Skip to content
  1. Jun 14, 2018
  3. Jun 13, 2018
  5. Jun 12, 2018
  7. Jun 11, 2018
    • Chris Wilson's avatar
      drm/i915: Wrap around the tail offset before setting ring->tail · 41d37680
      Chris Wilson authored 
      

The HW only accepts offsets within ring->size, and fails peculiarly if
the RING_HEAD or RING_TAIL is set to ring->size. Therefore whenever we
set ring->head/ring->tail we want to make sure it is within value (using
intel_ring_wrap()).

v2: Double check execlists as well
v3: Remove redundancy with assert_ring_tail_valid()
v4: Just assert in intel_ring_reset() rather than be over-defensive.

Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v2
Link: https://patchwork.freedesktop.org/patch/msgid/20180611110845.31890-2-chris@chris-wilson.co.uk
      41d37680
    • Chris Wilson's avatar
      drm/i915/ringbuffer: Fix context restore upon reset · b3ee09a4
      Chris Wilson authored 
      

The discovery with trying to enable full-ppgtt was that we were
completely failing to the load both the mm and context following the
reset. Although we were performing mmio to set the PP_DIR (per-process
GTT) and CCID (context), these were taking no effect (the assumption was
that this would trigger reload of the context and restore the page
tables). It was not until we performed the LRI + MI_SET_CONTEXT in a
following context switch would anything occur.

Since we are then required to reset the context image and PP_DIR using
CS commands, we place those commands into every batch. The hardware
should recognise the no-ops and eliminate the expensive context loads,
but we still have to pay the cost of using cross-powerwell register
writes. In practice, this has no effect on actual context switch times,
and only adds a few hundred nanoseconds to no-op switches. We can improve
the latter by eliminating the w/a around known no-op switches, but there
is an ulterior motive to keeping them.

Always emitting the context switch at the beginning of the request (and
relying on HW to skip unneeded switches) does have one key advantage.
Should we implement request reordering on Haswell, we will not know in
advance what the previous executing context was on the GPU and so we
would not be able to elide the MI_SET_CONTEXT commands ourselves and
always have to emit them. Having our hand forced now actually prepares
us for later.

Now since that context and mm follow the request, we no longer (and not
for a long time since requests took over!) require a trace point to tell
when we write the switch into the ring, since it is always. (This is
even more important when you remember that simply writing into the ring
bears no relation to the current mm.)

v2: Sandybridge has to agree to use LRI as well.

Testcase: igt/drv_selftests/live_hangcheck
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180611110845.31890-1-chris@chris-wilson.co.uk
      b3ee09a4
    • Chris Wilson's avatar
      drm/i915/ringbuffer: Brute force context restore · 1fc719d1
      Chris Wilson authored 
      

An issue encountered with switching mm on gen7 is that the GPU likes to
hang (with the VS unit busy) when told to force restore the current
context. We can simply workaround this by substituting the
MI_FORCE_RESTORE flag with a round-trip through the kernel_context,
forcing the context to be saved and restored; thereby reloading the
PP_DIR registers and updating the modified page directory!

v2: Undo attempted optimisation in caller (Tvrtko)

Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: default avatarMika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180611104808.24295-1-chris@chris-wilson.co.uk
      1fc719d1
    • Imre Deak's avatar
      drm/i915/skl: Add warn about unsupported CDCLK rates · 602a9de5
      Imre Deak authored 
      

While checking workarounds related to the CDCLK PLL, I noticed that the
DMC firmware bits for WA#1183 are missing for SKL. After that I
clarified with HW people that it's not needed on SKL, since it doesn't
support eDP1.4 which would be the only thing requiring the problematic
CDCLK clock rates. So in theory we shouldn't ever choose these
frequencies, but add an assert in any case for catching such cases and
for documentation.

v2:
- Move the check to skl_set_cdclk and warn whenever using the
  corresponding VCO freq. (Ville)

v3:
- Actually check for the platform. (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180608144137.7943-1-imre.deak@intel.com
      602a9de5
    • Michel Thierry's avatar
      drm/i915/perf: fix gen11 engine class shift · 2b9a8203
      Michel Thierry authored 
      Use the correct engine class shift value while storing the ctx hw id.
Fixes the copy+paste error from commit 61d5676b ("drm/i915/perf: fix
ctx_id read with GuC & ICL").

Apologies for not spotting this in the original review, the
specific_ctx_id_mask is correct, only the specific_ctx_id had this
problem.

v2: Just use the upper 32 bits of lrc_desc (Chris)
v3: If we use the lrc_desc, we must apply the ctx_id_mask too (Lionel)

Fixes: 61d5676b

 ("drm/i915/perf: fix ctx_id read with GuC & ICL")
Signed-off-by: default avatarMichel Thierry <michel.thierry@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Matthew Auld <matthew.auld@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: default avatarLionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: default avatarLionel Landwerlin <lionel.g.landwerlin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180604233250.609-2-michel.thierry@intel.com
      2b9a8203
    • Michel Thierry's avatar
      drm/i915/perf: use the lrc_desc to get the ctx hw id in gen8-10 · 9904b156
      Michel Thierry authored 
      

The upper 32 bits of the lrc_desc (bits 52-32 to be precise) are the
context hw id in GEN8-10, so use them and have one less thing to
maintain in the unlikely case we change the descriptor sw fields.

v2: If we use the lrc_desc, we must apply the ctx_id_mask too (Lionel)

Signed-off-by: default avatarMichel Thierry <michel.thierry@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarLionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: default avatarLionel Landwerlin <lionel.g.landwerlin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180604233250.609-1-michel.thierry@intel.com
      9904b156
    • Chris Wilson's avatar
      drm/i915: Apply batch location restrictions before pinning · 746c8f14
      Chris Wilson authored 
      We special case the position of the batch within the GTT to prevent
negative self-relocation deltas from underflowing. However, that
restriction is being applied after a trial pin of the batch in its
current position. Thus we are not rejecting an invalid location if the
batch has been used before, leading to an assertion if we happen to need
to rearrange the entire payload. In the worst case, this may cause a GPU
hang on gen7 or perhaps missing state.

References: https://bugs.freedesktop.org/show_bug.cgi?id=105720
Fixes: 2889caa9

 ("drm/i915: Eliminate lots of iterations over the execobjects array")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Martin Peres <martin.peres@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180610194325.13467-2-chris@chris-wilson.co.uk
Reviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
      746c8f14
    • Chris Wilson's avatar
      drm/i915: Refactor unsettting obj->mm.pages · acd1c1e6
      Chris Wilson authored 
      

As i915_gem_object_phys_attach() wants to play dirty and mess around
with obj->mm.pages itself (replacing the shmemfs with a DMA allocation),
refactor the gubbins so into i915_gem_object_unset_pages() that we don't
have to duplicate all the secrets.

Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180611075532.26534-1-chris@chris-wilson.co.uk
Link: https://patchwork.freedesktop.org/patch/msgid/152871104647.1718.8796913290418060204@jlahtine-desk.ger.corp.intel.com
      acd1c1e6
    • Chris Wilson's avatar
      drm/i915: Squash GEM load failure message (again) · 51c18bf7
      Chris Wilson authored 
      Due to a silent conflict (silent because we are trying to fix the CI
test that is meant to exercising these failures!) between commit
51e645b6 ("drm/i915: Mark the GPU as wedged without error on fault
injection") and commit 8571a05a

 ("drm/i915: Use GEM suspend when
aborting initialisation"), we failed to actually squash the error
message after injecting the load failure.

Rearrange the code to export i915_load_failure() for better logging of
real errors (and quiet logging of injected errors).

Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: default avatarMichał Winiarski <michal.winiarski@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180609111058.2660-1-chris@chris-wilson.co.uk
      51c18bf7
    • Jani Nikula's avatar
      drm/i915: fix PCH_NOP setting for non-PCH platforms · 07ba0a82
      Jani Nikula authored 
      

Setting PCH type to PCH_NOP before checking whether we actually have a
PCH ends up returning true for HAS_PCH_SPLIT() on all non-PCH split
platforms. Fix this by using PCH_NOP only for platforms that actually
have a PCH.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180608123330.31003-6-jani.nikula@intel.com
      07ba0a82
    • Jani Nikula's avatar
      drm/i915: be more strict about HAS_PCH_NOP() usage · 13d0464b
      Jani Nikula authored 
      

HAS_PCH_NOP() implies a PCH platform without south display, not generic
disabled display. Prefer num_pipes == 0 for PCH independent checks.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180608123330.31003-5-jani.nikula@intel.com
      13d0464b
    • Jani Nikula's avatar
      drm/i915: clean up virtual PCH special case handling · 85b17e6e
      Jani Nikula authored 
      

Use intel_pch_type() also for mapping the no PCH case (PCH id 0) to
PCH_NONE to simplify code.

Also make sure that intel_pch_type() knows all the PCH ids returned by
intel_virt_detect_pch(). Loudly fail if this isn't the case; this
shouldn't happen anyway.

Cc: Colin Xu <Colin.Xu@intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Tested-by: default avatarColin Xu <Colin.Xu@intel.com>
Reviewed-by: default avatarColin Xu <Colin.Xu@intel.com>
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180608123330.31003-4-jani.nikula@intel.com
      85b17e6e
    • Lucas De Marchi's avatar
      drm/i915: document PCH_NOP · b8bf31d8
      Lucas De Marchi authored 
      

There's a difference between PCH_NONE and PCH_NOP: the former means we
don't have a PCH while in the latter we do, but it doesn't have the
south display.

Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180608123330.31003-3-jani.nikula@intel.com
      b8bf31d8
    • Jani Nikula's avatar
      drm/i915: fix guest virtual PCH detection on non-PCH systems · 78ef3faf
      Jani Nikula authored 
      

Virtualized non-PCH systems such as Broxton or Geminilake should use
PCH_NONE to indicate no PCH rather than PCH_NOP. The latter is a
specific case to indicate a PCH system without south display.

Reported-by: default avatarColin Xu <Colin.Xu@intel.com>
Cc: Colin Xu <Colin.Xu@intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Tested-by: default avatarColin Xu <Colin.Xu@intel.com>
Reviewed-by: default avatarColin Xu <Colin.Xu@intel.com>
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180608123330.31003-2-jani.nikula@intel.com
      78ef3faf
  9. Jun 09, 2018