Re: [PATCH] drm/i915: Initialize HWS page address after GPU reset

["Siluvery, Arun" <arun.siluvery@linux.intel.com>]

On 15/06/2015 06:20, Daniel Vetter wrote:
> On Wed, Jun 3, 2015 at 6:14 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
>> I was going to suggest removing the same thing from the
>> lrc_setup_hardware_status_page(), but after another look it seems we
>> sometimes call .init_hw() before the context setup. Would be nice to
>> have a more consistent sequence for init and reset. But anyway the patch
>> looks OK to me. I verified that we indeed lose this register on GPU
>> reset.
>
> Yep, this is a mess. And historically _any_ difference between driver
> load and gpu reset (or resume fwiw) has lead to hilarious bugs, so
> this difference is really troubling to me. Arun, can you please work
> on a patch to unify the setup sequence here, so that both driver load
> gpu resets work the same way? By the time we're calling gem_init_hw
> the default context should have been created already, and hence we
> should be able to write to HWS_PGA in ring->init_hw only.
>

Hi Daniel,

I think the problem in this case was the code to init HWS page after 
reset was missing for Gen8+. For Gen7 we are doing this as part of 
ring->init_hw.

Gen7:
i915_reset()
+--> i915_gem_init_hw()
+------> ring->init_hw() which is init_render_ring()
+----------> init_ring_common()
+------------> intel_ring_setup_status_page()

Gen8:
i915_reset()
+--> i915_gem_init_hw()
+------> ring->init_hw() which is gen8_init_render_ring()
+--------> gen8_init_common_ring() - I added changes in this function.

We could probably use intel_ring_setup_status_page() for both cases, 
does it have to be Gen7 specific?

> Also I wonder about resume, where's the HWS_PGA restore for that case?
It is covered.

i915_drm_resume()
+-->i915_gem_init_hw

regards
Arun

> -Daniel
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx