The comment is very old and quite misleading now.
drivers/gpu/drm/i915/i915_gem_context.c:349: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/i915_gem_context.c:349: warning: No description found for parameter 'file_priv'
Signed-off-by: Chris Wilson <[email protected]>
Reviewed-by: Lionel Landwerlin <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
drivers/gpu/drm/i915/i915_gem_userptr.c:761: warning: No description found for parameter 'dev'
drivers/gpu/drm/i915/i915_gem_userptr.c:761: warning: No description found for parameter 'data'
drivers/gpu/drm/i915/i915_gem_userptr.c:761: warning: No description found for parameter 'file'
Signed-off-by: Chris Wilson <[email protected]>
Reviewed-by: Lionel Landwerlin <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
drivers/gpu/drm/i915/intel_ringbuffer.c:179: warning: No description found for parameter 'req'
drivers/gpu/drm/i915/intel_ringbuffer.c:741: warning: No description found for parameter 'req'
drivers/gpu/drm/i915/intel_ringbuffer.c:741: warning: No description found for parameter 'cs'
Signed-off-by: Chris Wilson <[email protected]>
Reviewed-by: Lionel Landwerlin <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
drivers/gpu/drm/i915/i915_gpu_error.c:1815: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/i915_gpu_error.c:1815: warning: No description found for parameter 'engine_mask'
drivers/gpu/drm/i915/i915_gpu_error.c:1815: warning: No description found for parameter 'error_msg'
drivers/gpu/drm/i915/i915_gpu_error.c:1815: warning: Excess function parameter 'dev' description in 'i915_capture_error_state'
Signed-off-by: Chris Wilson <[email protected]>
Reviewed-by: Lionel Landwerlin <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
After we assert the reset request (and wait for 20us), when the device
has been fully reset it asserts the reset-status bit. Before we stop
requesting the reset and allow the device to return to normal, we should
wait for the reset to be completed. (Similar to how we wait for the
device to return to normal after deasserting the reset request.)
v2: Rename i915_reset_completed() probe to not cause as much confusion.
Signed-off-by: Chris Wilson <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Mika Kuoppala <[email protected]>
Reduce the window of opportunity for set-wedged being called
concurrently with reset (after i915_reset() has performed the
i915_gem_unset_wedged()) by moving the set_bit(I915_WEDGED) to before we
complete the inflight requests. When i915_reset() is being blocked on a
request, such completion may allow it to start and beginning resetting
the GPU before i915_gem_set_wedged() has finished (and so before
set-wedge will have marked the device as wedged). As such,
i915_gem_init_hw() may see a wedged device even from inside
i915_reset().
References: 36703e79a9 ("drm/i915: Break modeset deadlocks on reset")
Signed-off-by: Chris Wilson <[email protected]>
Cc: Mika Kuoppala <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Reviewed-by: Joonas Lahtinen <[email protected]>
Reviewed-by: Mika Kuoppala <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
drivers/gpu/drm/i915/i915_oa_cnl.c: In function ‘i915_perf_load_test_config_cnl’:
drivers/gpu/drm/i915/i915_oa_cnl.c:99:2: error: ‘strncpy’ output truncated before terminating nul copying 36 bytes from a string of the same length [-Werror=stringop-truncation]
v2: strlcpy
Fixes: 95690a02fb ("drm/i915/perf: enable perf support on CNL")
Signed-off-by: Chris Wilson <[email protected]>
Cc: Lionel Landwerlin <[email protected]>
Cc: Matthew Auld <[email protected]>
Reviewed-by: Lionel Landwerlin <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
drivers/gpu/drm/i915/i915_oa_cflgt3.c: In function ‘i915_perf_load_test_config_cflgt3’:
drivers/gpu/drm/i915/i915_oa_cflgt3.c:87:2: error: ‘strncpy’ output truncated before terminating nul copying 36 bytes from a string of the same length [-Werror=stringop-truncation]
v2: strlcpy
Fixes: 4407eaa9b0 ("drm/i915/perf: add support for Coffeelake GT3")
Signed-off-by: Chris Wilson <[email protected]>
Cc: Lionel Landwerlin <[email protected]>
Cc: Matthew Auld <[email protected]>
Reviewed-by: Lionel Landwerlin <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Since commit 5896a5c8c9 (drm/i915: Always stop the rings before a
missing GPU reset) we attempt to stop the engines during gem_sanitize
even if reset=0 and nothing bad happened on the gpu.
The specs says that the STOP_RINGS bit needs to be cleared to resume
normal operation, but for some reason the value of the bit seems to be
changing without us writing to it (maybe rc6 entry/exit?), so normal
operation resumes correctly. However, it still feels incorrect to stop
the engines if there hasn't been any issue so skip the whole reset
call in gem_sanitize if i915.reset=0
Cc: Chris Wilson <[email protected]>
Cc: Mika Kuoppala <[email protected]>
Signed-off-by: Daniele Ceraolo Spurio <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Chris Wilson <[email protected]>
Signed-off-by: Chris Wilson <[email protected]>
Rather than having the high level ioctl interface guess the underlying
implementation details, having the implementation declare what
capabilities it exports. We define an intel_driver_caps, similar to the
intel_device_info, which instead of trying to describe the HW gives
details on what the driver itself supports. This is then populated by
the engine backend for the new scheduler capability field for use
elsewhere.
v2: Use caps.scheduler for validating CONTEXT_PARAM_SET_PRIORITY (Mika)
One less assumption of engine[RCS] \o/
Signed-off-by: Chris Wilson <[email protected]>
Cc: Tomasz Lis <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: Michal Wajdeczko <[email protected]>
Cc: Mika Kuoppala <[email protected]>
Reviewed-by: Mika Kuoppala <[email protected]>
Reviewed-by: Tomasz Lis <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Michel Thierry <[email protected]>
We are not allowed to call intel_runtime_pm_get from the PMU counter read
callback since the former can sleep, and the latter is running under IRQ
context.
To workaround this, we record the last known RC6 and while runtime
suspended estimate its increase by querying the runtime PM core
timestamps.
Downside of this approach is that we can temporarily lose a chunk of RC6
time, from the last PMU read-out to runtime suspend entry, but that will
eventually catch up, once device comes back online and in the presence of
PMU queries.
Also, we have to be careful not to overshoot the RC6 estimate, so once
resumed after a period of approximation, we only update the counter once
it catches up. With the observation that RC6 is increasing while the
device is suspended, this should not pose a problem and can only cause
slight inaccuracies due clock base differences.
v2: Simplify by estimating on top of PM core counters. (Imre)
Signed-off-by: Tvrtko Ursulin <[email protected]>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104943
Fixes: 6060b6aec0 ("drm/i915/pmu: Add RC6 residency metrics")
Testcase: igt/perf_pmu/rc6-runtime-pm
Cc: Tvrtko Ursulin <[email protected]>
Cc: Chris Wilson <[email protected]>
Cc: Imre Deak <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: David Airlie <[email protected]>
Cc: [email protected]
Cc: [email protected]
Reviewed-by: Chris Wilson <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
On blb and pnv, we are seeing sporadic
i915 0000:00:02.0: Resetting chip after gpu hang
[drm:intel_gpu_reset [i915]] rcs0: timed out on STOP_RING
[drm:i915_reset [i915]] *ERROR* Failed hw init on reset -5
which notably lack the actual root cause of the error. Ostensibly it
should be the init_ring_common() that failed, but it's error paths are
covered by DRM_ERROR.
Signed-off-by: Chris Wilson <[email protected]>
Reviewed-by: Tvrtko Ursulin <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
If we submit a request and see that the previous request on this
timeline was already signaled, we first do not need to add the
dependency tracker for that completed request and secondly we know that
we there is then a large backlog in retiring requests affecting this
timeline. Given that we just submitted more work to the HW, now would be
a good time to catch up on those retirements.
v2: Try to sum up the compromises involved in flushing the retirement
queue after submission.
Signed-off-by: Chris Wilson <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Reviewed-by: Tvrtko Ursulin <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
According to bspec, result_lines > 31 is only a maximum for latency
level 1 through 7.
For level 0 the number of lines is ignored, so always write 0 there
to prevent overflowing the 5 bits value.
This is required to make NV12 work.
Changes since v1:
- Rebase on top of GEN11 wm changes. It seems to use res_lines for
level 0 limit calculations, but still doesn't appear to program it.
Signed-off-by: Maarten Lankhorst <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Reviewed-by: Ville Syrjälä <[email protected]> #v1
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
When a request is preempted, it is unsubmitted from the HW queue and
removed from the active list of breadcrumbs. In the process, this
however triggers the signaler and it may see the clear rbtree with the
old, and still valid, seqno, or it may match the cleared seqno with the
now zero rq->global_seqno. This confuses the signaler into action and
signaling the fence.
Fixes: d6a2289d9d ("drm/i915: Remove the preempted request from the execution queue")
Signed-off-by: Chris Wilson <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: <[email protected]> # v4.12+
Reviewed-by: Tvrtko Ursulin <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Commit 99e48bf98d ("drm/i915: Lock out execlist tasklet while peeking
inside for busy-stats") added a tasklet_disable call in busy stats
enabling, but we failed to understand that the PMU enable callback runs
as an hard IRQ (IPI).
Consequence of this is that the PMU enable callback can interrupt the
execlists tasklet, and will then deadlock when it calls
intel_engine_stats_enable->tasklet_disable.
To fix this, I realized it is possible to move the engine stats enablement
and disablement to PMU event init and destroy hooks. This allows for much
simpler implementation since those hooks run in normal context (can
sleep).
v2: Extract engine_event_destroy. (Chris Wilson)
Signed-off-by: Tvrtko Ursulin <[email protected]>
Fixes: 99e48bf98d ("drm/i915: Lock out execlist tasklet while peeking inside for busy-stats")
Testcase: igt/perf_pmu/enable-race-*
Cc: Chris Wilson <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: [email protected]
Reviewed-by: Chris Wilson <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
This workaround should prevent a bug that can be hit on a context
restore. To avoid the issue, we must emit a PIPE_CONTROL with CS stall
(0x7a000004 0x00100000 0x00000000 0x00000000) followed by 12DW's of
NOOP(0x0) in the indirect context batch buffer, to ensure the engine is
idle prior to programming 3DSTATE_SAMPLE_PATTERN.
It's also not clear whether we should add those extra dwords because of
the workaround itself, or if that's just padding for the WA BB (and next
commands could come right after the PIPE_CONTROL). We keep them for now.
References: HSD#1939868
v2: More descriptive changelog and comments.
v3: Explain that PIPE_CONTROL is actually 6 dwords, and that we advance
10 more dwords because of that.
Signed-off-by: Rafael Antognolli <[email protected]>
Cc: Chris Wilson <[email protected]>
Acked-by: Chris Wilson <[email protected]>
Signed-off-by: Chris Wilson <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
The find_reg function was assuming that there is always at least one table in
reg_tables. It is not always true.
In case of VCS or VECS, the reg_tables is NULL and reg_table_count is 0,
implying that no register-accessing commands are allowed. However, the command
tables include commands such as MI_STORE_REGISTER_MEM. When trying to check
such command, the find_reg would dereference NULL pointer.
Now it will just return NULL meaning that the register was not found and the
command will be rejected.
Fixes: 76ff480ec9 ("drm/i915/cmdparser: Use binary search for faster register lookup")
Signed-off-by: Michal Srb <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Cc: Chris Wilson <[email protected]>
Cc: Matthew Auld <[email protected]>
Reviewed-by: Chris Wilson <[email protected]>
Signed-off-by: Chris Wilson <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
register lookup")
Deprecate the silly I915_SET_COLORKEY_NONE flag. The obvious
way to disable colorkey is to just set flags to 0, which is
exactly what the intel ddx has been doing all along.
Currently when userspace sets the flags to 0, we end up in a
funny state where colorkey is disabled, but various colorkey
vs. scaling checks still consider colorkey to be enabled, and
thus we don't allow plane scaling to kick in.
In case there is some other userspace out there that actually
uses this flag (unlikely as this is an i915 specific uapi)
we'll keep on accepting it.
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Chris Wilson <[email protected]>
Since commit 7b6da818d8 ("drm/i915: Restore the kernel context after a
GPU reset on an idle engine") we submit a request following the engine
reset. The intent is that we don't submit a request if the engine is
busy (as it will restart active by itself) but we only checked to see if
there were remaining requests in flight on the hardware and skipped
checking to see if there were any ready requests that would be
immediately submitted on restart (the same time as our new request would
be). Having convinced the engine to appear idle in the previous patch,
we can use intel_engine_is_idle() as a better test to only submit a new
request if there are no pending requests.
As it happens, this is tripping up igt/drv_selftest/live_hangcheck in CI
as we overfill the kernel_context ringbuffer trigger an infinite
recursion from within the reset.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104786
References: 7b6da818d8 ("drm/i915: Restore the kernel context after a GPU reset on an idle engine")
Signed-off-by: Chris Wilson <[email protected]>
Cc: Mika Kuoppala <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Michel Thierry <[email protected]>
Reviewed-by: Joonas Lahtinen <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
In preparation for the next patch, we want the engine to appear idle
after a reset (if there are no requests in flight). For execlists, this
entails clearing the active status on reset, it will be regenerated on
restarting the engine after the reset. In the process, note that a
couple of other status flags and checks could be moved into the
describing function.
Signed-off-by: Chris Wilson <[email protected]>
Reviewed-by: Mika Kuoppala <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Avoid injecting hangs in to the i915->kernel_context in case the GPU
reset leaves corruption in the context image in its wake (leading to
continual failures and system hangs after the selftests are ostensibly
complete). Use a sacrificial kernel_context instead.
v2: Closing a context is tricky; export a function (for selftests) from
i915_gem_context.c to get it right.
Signed-off-by: Chris Wilson <[email protected]>
Cc: Mika Kuoppala <[email protected]>
Cc: Michel Thierry <[email protected]>
Reviewed-by: Mika Kuoppala <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
When injecting rapid resets, we must be careful to at least wait for the
previous reset to have taken effect and the engine restarted. If we
perform a second reset before that has happened, we will notice that the
engine hasn't recovered and declare it lost, wedging the device and
failing. In practice, since we wait for each hanging batch to start
before injecting the reset, this too-fast-reset condition can only be
triggered when moving onto the next engine in the test, so we need only
wait for the existing reset to complete before switching engines.
v2: Wrap up the wait inside a safety net to bail out in case of angry hw.
Signed-off-by: Chris Wilson <[email protected]>
Cc: Mika Kuoppala <[email protected]>
Cc: Michel Thierry <[email protected]>
Reviewed-by: Mika Kuoppala <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
If we remember to cancel the signaler on a request when retiring it
(after we know that the request has been signaled), we do not need to
carry an additional request in the signaler itself. This prevents an
issue whereby the signaler threads may be delayed and hold on to
thousands of request references, causing severe memory fragmentation and
premature oom (most noticeable on 32b snb due to the limited GFP_KERNEL
and frequent use of inter-engine fences).
v2: Rename first_signal(), document reads outside of locks.
Signed-off-by: Chris Wilson <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Tvrtko Ursulin <[email protected]>
During testing, we trigger a lot of resets on an unbannable context
leading to massive amounts of irrelevant debug spam. Remove the
ban_score accounting and message for the unbannable context so that we
improve the signal:noise in the log messages for when the unexpected
occurs.
Signed-off-by: Chris Wilson <[email protected]>
Cc: Mika Kuoppala <[email protected]>
Reviewed-by: Mika Kuoppala <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Since unbannable contexts are special and supposed not to be causing GPU
hangs in the first place, make it clear when they are implicated in said
hang. In practice, most unbannable contexts are those created by igt
for the express purpose of throwing untold thousands of hangs at the GPU
and wish to keep doing so to finish the test. Normally they are cleaned
up, but it's when they or the other unbannable kernel contexts stay
stuck in an erroneous state that we need to worry and so need
highlighting.
Suggested-by: Mika Kuoppala <[email protected]>
Signed-off-by: Chris Wilson <[email protected]>
Cc: Mika Kuoppala <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Mika Kuoppala <[email protected]>
We're using i915_inject_load_failure() to inject dummy
faults during driver load, but since this is debug utility
we shouldn't expose it in default config as it consumes
both code and data.
add/remove: 0/1 grow/shrink: 0/2 up/down: 0/-302 (-302)
Function old new delta
__i915_inject_load_failure 61 - -61
i915_gem_init 1331 1268 -63
i915_driver_load 5923 5745 -178
Total: Before=1177454, After=1177152, chg -0.03%
add/remove: 0/1 grow/shrink: 0/0 up/down: 0/-4 (-4)
Data old new delta
i915_load_fail_count 4 - -4
Total: Before=56762, After=56758, chg -0.01%
add/remove: 4/8 grow/shrink: 0/1 up/down: 245/-591 (-346)
RO Data old new delta
__param_str_inject_load_failure 20 - -20
__UNIQUE_ID_inject_load_failuretype200 34 - -34
__param_inject_load_failure 40 - -40
__func__ 4998 4896 -102
__UNIQUE_ID_inject_load_failure201 150 - -150
Total: Before=119095, After=118749, chg -0.29%
Signed-off-by: Michal Wajdeczko <[email protected]>
Cc: Chris Wilson <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Chris Wilson <[email protected]>
Signed-off-by: Chris Wilson <[email protected]>
There is no requirement for doing the PCODE request polling atomically,
so do that only for a short time switching to sleeping poll afterwards.
The specification requires a 150usec timeout for the change notification,
so let's use that for the atomic poll. Do the extra 2ms poll - needed as
a workaround on BXT/GLK - in sleeping mode.
v2:
- rebase on v2 of patchset dropping the sandybridge_pcode_read/write
refactoring (Chris)
Cc: Chris Wilson <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Signed-off-by: Imre Deak <[email protected]>
Reviewed-by: Chris Wilson <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Currently we see sporadic timeouts during CDCLK changing both on BXT and
GLK as reported by the Bugzilla: ticket. It's easy to reproduce this by
changing the frequency in a tight loop after blanking the display. The
upper bound for the completion time is 800us based on my tests, so
increase it from the current 500us to 2ms; with that I couldn't trigger
the problem either on BXT or GLK.
Note that timeouts happened during both the change notification and the
voltage level setting PCODE request. (For the latter one BSpec doesn't
require us to wait for completion before further HW programming.)
This issue is similar to
commit 2c7d0602c8 ("drm/i915/gen9: Fix PCODE polling during CDCLK
change notification")
but there the PCODE request does complete (as shown by the mbox
busy flag), only the reply we get from PCODE indicates a failure.
So there we keep resending the request until a success reply, here we
just have to increase the timeout for the one PCODE request we send.
v2:
- s/snb_pcode_request/sandybridge_pcode_write_timeout/ (Ville)
Cc: Chris Wilson <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Cc: <[email protected]> # v4.4+
Acked-by: Chris Wilson <[email protected]> (v1)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103326
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Imre Deak <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
