Re: [PATCH] drm/i915: Introduce i915_gem_fini_hw for symmetry with i915_gem_init_hw

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 12/10/18 06:50, Chris Wilson wrote:
> Quoting Michal Wajdeczko (2018-10-12 14:26:09)
>> In function i915_gem_init_hw we are initializing some uC code that
> 
> i915_gem_init_hw really shouldn't be called such, at least it doesn't
> fit in with the global init_hw phase. Suggestions for a clearer name
> welcome.
> 
>> requires some cleanup. Then during unwind we call this uC cleanup
>> function directly which breaks symmetry and layering. Fix that by
>> adding i915_gem_fini_hw for symmetry with i915_gem_init_hw.
> 
> There's a lot that we do in init_hw that we presume will be undone by
> reset. The asymmetry looks inherent, but I wonder if it would be better
> expressed as we are missing a global reset along the error path.
> 
> It also has to be said that we cannot support intel_uc_init_hw() from
> within a struct_mutex-less i915_reset. (I have a patch to disable global
> reset while guc is enabled to avoid the deadlock, forcing us to rely on
> per-engine resets in that case).
> 

Can we really not have global reset with GuC? Or is your patch just a 
temporary WA? I know per-engine reset should work for 99% of cases, but 
what if, for example, the GuC itself hangs? There is also some other HW 
blocks in GT that are not covered by engine reset.

Thanks,
Daniele

> Anyway, I'm hesitant to call the suggested i915_gem_fini_hw() as the
> compliment to i915_gem_init_hw() and feel a little more convincing is in
> order.
> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx