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
