On Wed, Sep 06, 2017 at 12:14:05PM +0100, Chris Wilson wrote:
> Ville Syrjälä spotted that PGETBL_CTL was losing its enable bit upon a
> reset. That was causing the display to show garbage on his 945gm. On my
> i915gm the effect was far more severe; re-enabling the display following
> the reset without PGETBL_CTL being enabled lead to an immediate hard
> hang.
>
> We do have a routine to re-enable PGETBL_CTL which is applicable to
> gen2-4, although on gen4 it is documented that a graphics reset doesn't
> alter the register (no such wording is given for gen3) and should be safe
> to call to punch back in the enable bit. However, that leaves the question
> of whether we need to completely re-initialise the register and the
> rest of the GSM. For g33/pnv/gen4+, where we do have a configurable
> page table, its contents do seem to be kept, and so we should be able to
> recover without having to reinitialise the GTT from scratch (as prior to
> g33, that register is configured by the BIOS and we leave alone except
> for the enable bit).
>
> This appears to have been broken by commit 5fbd0418eef2 ("drm/i915:
> Re-enable GGTT earlier during resume on pre-gen6 platforms"), which
> moved the intel_enable_gtt() from i915_gem_init_hw() (also used by
> reset) to add it earlier during hw init and resume, missing the reset
> path.
>
> v2: Find the culprit, rearrange ggtt_enable to be before gem_init_hw to
> match init/resume
>
> Reported-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Fixes: 5fbd0418eef2 ("drm/i915: Re-enable GGTT earlier during resume on pre-gen6 platforms")
Ouch. So it was me that broke this. Sorry about that folks.
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101852
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> Reviewed-by: Daniel Vetter <daniel@xxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f10a078e3a55..ff70fc45ba7c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1892,9 +1892,15 @@ void i915_reset(struct drm_i915_private *i915, unsigned int flags)
>
> /*
> * Everything depends on having the GTT running, so we need to start
> - * there. Fortunately we don't need to do this unless we reset the
> - * chip at a PCI level.
> - *
> + * there.
> + */
> + ret = i915_ggtt_enable_hw(i915);
> + if (ret) {
> + DRM_ERROR("Failed to re-enable GGTT following reset %d\n", ret);
> + goto error;
> + }
> +
> + /*
> * Next we need to restore the context, but we don't use those
> * yet either...
> *
> --
> 2.14.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
