Skip to content

Commit

Permalink
drm/i915: handle uncore spinlock when not available
Browse files Browse the repository at this point in the history
The uncore code may not always be available (e.g. when we build the
display code with Xe), so we can't always rely on having the uncore's
spinlock.

To handle this, split the spin_lock/unlock_irqsave/restore() into
spin_lock/unlock() followed by a call to local_irq_save/restore() and
create wrapper functions for locking and unlocking the uncore's
spinlock.  In these functions, we have a condition check and only
actually try to lock/unlock the spinlock when I915 is defined, and
thus uncore is available.

This keeps the ifdefs contained in these new functions and all such
logic inside the display code.

Cc: Tvrtko Ursulin <tvrto.ursulin@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20231201100032.1367589-1-luciano.coelho@intel.com
  • Loading branch information
Luca Coelho authored and Jouni Högander committed Dec 7, 2023
1 parent 01a39f1 commit dcdf1bb
Showing 1 changed file with 41 additions and 10 deletions.
51 changes: 41 additions & 10 deletions drivers/gpu/drm/i915/display/intel_vblank.c
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,32 @@ int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int scanline)
return (scanline + vtotal - crtc->scanline_offset) % vtotal;
}

/*
* The uncore version of the spin lock functions is used to decide
* whether we need to lock the uncore lock or not. This is only
* needed in i915, not in Xe.
*
* This lock in i915 is needed because some old platforms (at least
* IVB and possibly HSW as well), which are not supported in Xe, need
* all register accesses to the same cacheline to be serialized,
* otherwise they may hang.
*/
static void intel_vblank_section_enter(struct drm_i915_private *i915)
__acquires(i915->uncore.lock)
{
#ifdef I915
spin_lock(&i915->uncore.lock);
#endif
}

static void intel_vblank_section_exit(struct drm_i915_private *i915)
__releases(i915->uncore.lock)
{
#ifdef I915
spin_unlock(&i915->uncore.lock);
#endif
}

static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
bool in_vblank_irq,
int *vpos, int *hpos,
Expand Down Expand Up @@ -302,11 +328,12 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
}

/*
* Lock uncore.lock, as we will do multiple timing critical raw
* register reads, potentially with preemption disabled, so the
* following code must not block on uncore.lock.
* Enter vblank critical section, as we will do multiple
* timing critical raw register reads, potentially with
* preemption disabled, so the following code must not block.
*/
spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
local_irq_save(irqflags);
intel_vblank_section_enter(dev_priv);

/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */

Expand Down Expand Up @@ -374,7 +401,8 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,

/* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */

spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
intel_vblank_section_exit(dev_priv);
local_irq_restore(irqflags);

/*
* While in vblank, position will be negative
Expand Down Expand Up @@ -412,9 +440,13 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
unsigned long irqflags;
int position;

spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
local_irq_save(irqflags);
intel_vblank_section_enter(dev_priv);

position = __intel_get_crtc_scanline(crtc);
spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);

intel_vblank_section_exit(dev_priv);
local_irq_restore(irqflags);

return position;
}
Expand Down Expand Up @@ -537,7 +569,7 @@ void intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state,
* Need to audit everything to make sure it's safe.
*/
spin_lock_irqsave(&i915->drm.vblank_time_lock, irqflags);
spin_lock(&i915->uncore.lock);
intel_vblank_section_enter(i915);

drm_calc_timestamping_constants(&crtc->base, &adjusted_mode);

Expand All @@ -546,7 +578,6 @@ void intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state,
crtc->mode_flags = mode_flags;

crtc->scanline_offset = intel_crtc_scanline_offset(crtc_state);

spin_unlock(&i915->uncore.lock);
intel_vblank_section_exit(i915);
spin_unlock_irqrestore(&i915->drm.vblank_time_lock, irqflags);
}

0 comments on commit dcdf1bb

Please sign in to comment.