While the comment singles out Port A or B, the code says Port A or *D*.
Looking at the history it seems that the comment was added after the code,
so it seems likely that the code is correct, not the comment.
CC: Jani Nikula <jani.nikula@intel.com>
CC: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
Reviewed-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180209130755.11893-1-david.weinehall@linux.intel.com
When initialising the page directories, we set the GTT entries and the
tree to the scratch page. We have already replaced the DMA fill with
memset64(), but we can similarly use memset_p() to set the pointer array.
References: 4dd504f7d9 ("drm/i915: Use memset64() to prefill the GTT page")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180212133118.16443-1-chris@chris-wilson.co.uk
On BXT/GLK GEN6_PCODE_READ_RC6VIDS fails with
MAILBOX_P24C_CC_ILLEGAL_CMD, so don't try to do the query on these
platforms. Do it only on SNB, IVB and HSW, where we use this command
anyway for RC6 enabling.
Based on my tests the command also succeeds on all LLC platforms, but
it's not clear if it's really supported on those (it returns 0 aka 245mv
for all RC6 states everywhere except on SNB). BSpec lists the command as
supported on SKL+ (see P24C_PCODE_MAILBOX_INTERFACE) but that's clearly
incorrect, since on SKL/KBL the same command ID is used for
SKL_PCODE_LOAD_HDCP_KEYS. Since the command fails on BXT/GLK, the BSpec
command list is also incorrect for those platforms (see
P_CR_P24C_PCODE_MAILBOX_INTERFACE_0_2_0_GTTMMADR).
I filed a request to update that info in Bspec, but for now let's
assume a minimal set of platforms where the command is supported.
References: https://bugs.freedesktop.org/show_bug.cgi?id=103337
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208174102.10240-1-imre.deak@intel.com
When dumping the engine, we print out the current register values. This
requires the rpm wakeref. If the device is alseep, we can assume the
engine is asleep (and the register state is uninteresting) so skip and
only acquire the rpm wakeref if the device is already awake.
Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180212102415.24246-1-chris@chris-wilson.co.uk
If the entire device is powered off, we can safely assume that the
engine is also asleep (and idle).
Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: a091d4ee93 ("drm/i915: Hold a wakeref for probing the ring registers")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180212093928.6005-1-chris@chris-wilson.co.uk
If we fail to reset the GPU (i915_reset()), we do one final
intel_gpu_reset() attempt as we mark the device wedged. The idea here is
even though the GPU has proven unreliable (and so we want to stop using
it for the time being), we don't want it spinning away in the background
whilst the driver idles so we try to reset it one more time. However, we
want to dump the i915_gem_set_wedged() debugging info before we do, so
that we can see the accurate state of the GPU when it failed.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180209114056.9957-1-chris@chris-wilson.co.uk
Most of our ioctl functions have an _ioctl suffix in the name. I like
that idea since it makes it easy to figure out how the function is
going to get called. Rename the handful of exceptions to follow the
same pattern.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207164841.19431-1-ville.syrjala@linux.intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
FORCEWAKE_ACK is depricated by BSpec at least starting from BDW,
referring to the multi-threaded version of it instead. Accessing
FORCEWAKE_ACK triggers an unclaimed register access error - at
least on GLK - see the Reference: below.
The correct registers to use would be FORCEWAKE_MT_ACK on IVB+ and
FORCEWAKE_ACK_RENDER_GEN9 on SKL+ like it's done elsewhere in the
driver.
The forcewake check itself is inconsistent and redundant, since there
could be other forcewake requesters besides the kernel (being the
multithreaded version of the register) and the kernel's per-domain
forcewake counters are shown anyway at the end of the file. So let's
just remove the check.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=103337
Signed-off-by: Imre Deak <imre.deak@intel.com>
Tested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208112331.12986-1-imre.deak@intel.com
Since commit 4a118ecbe9 ("drm/i915: Filter out spurious execlists
context-switch interrupts") we probe execlists->active, and no longer
have to peek at the execlist interrupt to determine if the tasklet still
needs to be run to drain the ELSP.
References: 4a118ecbe9 ("drm/i915: Filter out spurious execlists context-switch interrupts")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208151224.16285-1-chris@chris-wilson.co.uk
clang is confused by our if-else-chain that abruptly exits before a
final else:
drivers/gpu/drm/i915/intel_crt.c:821:11: warning: variable 'status' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
else if (ret < 0)
^~~~~~~
drivers/gpu/drm/i915/intel_crt.c:826:9: note: uninitialized use occurs here
return status;
^~~~~~
drivers/gpu/drm/i915/intel_crt.c:821:7: note: remove the 'if' if its condition is always true
else if (ret < 0)
^~~~~~~~~~~~
drivers/gpu/drm/i915/intel_crt.c:761:12: note: initialize the variable 'status' to silence this warning
int status, ret;
In this case, we can reduce the final else-if clause to an unconditional
else.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208163939.27030-1-chris@chris-wilson.co.uk
The unused-but-set warning enabled by W=1 catches out a lot of the
atomic helper iterator macros and drown us in their noise (or trip over
Werror and die). Path of least resistance is to ignore the warning.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208161639.27511-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
drivers/gpu/drm/i915/i915_gem_internal.c:183: warning: No description found for parameter 'i915'
drivers/gpu/drm/i915/i915_gem_internal.c:183: warning: No description found for parameter 'size'
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208114224.27271-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
drivers/gpu/drm/i915/i915_gem_execbuffer.c:1983: warning: No description found for parameter 'dev_priv'
drivers/gpu/drm/i915/i915_gem_execbuffer.c:1983: warning: No description found for parameter 'file'
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208113917.8439-1-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/i915_syncmap.c:92: warning: No description found for parameter 'root'
drivers/gpu/drm/i915/i915_syncmap.c:155: warning: No description found for parameter 'root'
drivers/gpu/drm/i915/i915_syncmap.c:155: warning: No description found for parameter 'id'
drivers/gpu/drm/i915/i915_syncmap.c:155: warning: No description found for parameter 'seqno'
drivers/gpu/drm/i915/i915_syncmap.c:354: warning: No description found for parameter 'root'
drivers/gpu/drm/i915/i915_syncmap.c:354: warning: No description found for parameter 'id'
drivers/gpu/drm/i915/i915_syncmap.c:354: warning: No description found for parameter 'seqno'
drivers/gpu/drm/i915/i915_syncmap.c:396: warning: No description found for parameter 'root'
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208105449.29880-2-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
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 <chris@chris-wilson.co.uk>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208111559.32663-1-chris@chris-wilson.co.uk
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 <chris@chris-wilson.co.uk>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208111328.32422-1-chris@chris-wilson.co.uk
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 <chris@chris-wilson.co.uk>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208111220.32293-1-chris@chris-wilson.co.uk
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 <chris@chris-wilson.co.uk>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208111105.32149-1-chris@chris-wilson.co.uk
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 <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207222824.29864-1-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Although the mmio are uncached and so should be flushed on every write,
be paranoid and do a mmio read after setting the ring head/tail to be
sure they have taken effect before moving on.
v2: post tail to be pleasing to the eye
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208072800.595-1-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
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 <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207151350.20883-1-chris@chris-wilson.co.uk
Userspace provides a 64b value for the priority, we need to be careful
to preserve the full range before validation to prevent truncation (and
letting an illegal value pass).
Reported-by: Antonio Argenziano <antonio.argenziano@intel.com>
Fixes: ac14fbd460 ("drm/i915/scheduler: Support user-defined priorities")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208085151.11480-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
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 <chris@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208102403.5587-2-chris@chris-wilson.co.uk
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 <chris@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208102403.5587-1-chris@chris-wilson.co.uk
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 <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207212440.13438-1-daniele.ceraolospurio@intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
If we remove some hardcoded assumptions about the preempt context having
a fixed id, reserved from use by normal user contexts, we may only
allocate the i915_gem_context when required. Then the subsequent
decisions on using preemption reduce to having the preempt context
available.
v2: Include an assert that we don't allocate the preempt context twice.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207210544.26351-3-chris@chris-wilson.co.uk
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
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 <chris@chris-wilson.co.uk>
Cc: Tomasz Lis <tomasz.lis@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Tomasz Lis <tomasz.lis@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207210544.26351-2-chris@chris-wilson.co.uk
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
In the next patch, we may only conditionally allocate the preempt-client
if there is a global preempt context and so we need to be prepared in
case the preempt-client itself is NULL.
v2: Grep for more preempt_client.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207210544.26351-1-chris@chris-wilson.co.uk
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
As we peek inside struct device to query members guarded by CONFIG_PM,
so must be the code.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Fixes: 1fe699e301 ("drm/i915/pmu: Fix sleep under atomic in RC6 readout")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207160428.17015-1-chris@chris-wilson.co.uk
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 <tvrtko.ursulin@intel.com>
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 <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180206183311.17924-1-tvrtko.ursulin@linux.intel.com
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 <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207111545.17078-1-chris@chris-wilson.co.uk
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 <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207084350.3929-1-chris@chris-wilson.co.uk
If the last request on the timeline is already complete, we do not need
to emit the serialisation barriers.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180207084350.3929-2-chris@chris-wilson.co.uk
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 <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> #v1
Link: https://patchwork.freedesktop.org/patch/msgid/20180205105841.31634-1-maarten.lankhorst@linux.intel.com
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 <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: <stable@vger.kernel.org> # v4.12+
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180206094633.30181-1-chris@chris-wilson.co.uk
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 <tvrtko.ursulin@intel.com>
Fixes: 99e48bf98d ("drm/i915: Lock out execlist tasklet while peeking inside for busy-stats")
Testcase: igt/perf_pmu/enable-race-*
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180205093448.13877-1-tvrtko.ursulin@linux.intel.com
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 <rafael.antognolli@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180205233330.14973-1-rafael.antognolli@intel.com
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 <msrb@suse.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180205142916.27092-2-msrb@suse.com
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180205160438.3267-1-chris@chris-wilson.co.uk
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ä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180202204231.27905-1-ville.syrjala@linux.intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
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 <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180205152431.12163-4-chris@chris-wilson.co.uk
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 <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180205152431.12163-3-chris@chris-wilson.co.uk
