On Wed, Mar 12, 2025 at 11:52:33AM +0200, Jani Nikula wrote:
> 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?
I think we should be able to just switch to our own display->irq_lock.
I don't think we're really protecting anything shared between gt and
display with this.
--
Ville Syrjälä
Intel
