On Tue, 11 Mar 2025, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Tue, Mar 11, 2025 at 08:00:41PM +0200, Jani Nikula wrote:
>> static void
>> ivb_primary_enable_flip_done(struct intel_plane *plane)
>> {
>> + struct intel_display *display = to_intel_display(plane);
>> struct drm_i915_private *i915 = to_i915(plane->base.dev);
>>
>> spin_lock_irq(&i915->irq_lock);
>> - ilk_enable_display_irq(i915, DE_PLANE_FLIP_DONE_IVB(plane->i9xx_plane));
>> + ilk_enable_display_irq(display, DE_PLANE_FLIP_DONE_IVB(plane->i9xx_plane));
>> spin_unlock_irq(&i915->irq_lock);
>
> I was pondering if we could just suck the lock into these
> guys. But at least the fifo underrun reporting code is using
> some of these things and there there the lock is taken
> further out. So sadly not as trivial as I was hoping.
The whole i915->irq_lock (*) is a looming problem for display separation
from i915/xe core.
How do you abstract a spin lock? Is it okay to pass a spinlock_t * from
i915 core code to display, which would then store this pointer and use
it locally?
Wrapping it in a pair of function calls certainly sucks in a lot of
ways.
BR,
Jani.
*) xe->irq.lock on xe, with an ugly #define irq_lock irq.lock
--
Jani Nikula, Intel
