Re: [Intel-gfx] [PATCH v3] drm/i915: handle uncore spinlock when not available

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 03/11/2023 08:58, Coelho, Luciano wrote:
> On Fri, 2023-11-03 at 03:31 +0000, Vivi, Rodrigo wrote:
>>>>
>>>> Any other suggestions?
>>>
>>> I think it will boil down to the reason uncore lock is held over
>>> the
>>> respective sections ie. the comment in i915_get_crtc_scanoutpos.
>>>
>>> If it is timing sensitive to the extent irq off was needed it may
>>> apply
>>> to Xe as well. Likewise the need to use mmio helpers which rely on
>>> the
>>> uncore lock already held. Question of whether there is conceptual
>>> commonality, will probably drive the best name, or the best
>>> approach
>>> in
>>> general.
>>
>> yeap, this is how I'm seeing this. If i915-display needs this global
>> lock around mmio operations, then we wound need to add it to the
>> xe_mmio as well and then solve the name, etc.
>>
>> However, I don't believe that other users of the mmio would need
>> this lock. So I believe the right thing to do is to create a i915-
>> display only spin_lock, around the intel_de_mmio calls and here.
>>
>> With this we entirely kill the dependency on someone-else's lock
>> and have something that is entirely inside display code so it
>> doesn't need to be ported to one or another driver core components.
> 
> Right, I agree too.
> 
> My patch was just trying to address the quick hack made for Xe, not the
> actual implementation in Xe's side.  But it makes sense to implement
> this new lock internally in the display so there are no dependencies or
> wrappers needed.
> 
> I'll respin.

You could also make sure it needs to be a lock and not just say a 
preempt off or irq section?

Regards,

Tvrtko