From: Luca Coelho <luciano.coelho@intel.com> To: intel-gfx@lists.freedesktop.org Cc: intel-xe@lists.freedesktop.org, rodrigo.vivi@intel.com Subject: [Intel-gfx] [PATCH v4] drm/i915: handle uncore spinlock when not available Date: Wed, 29 Nov 2023 11:17:28 +0200 [thread overview] Message-ID: <20231129091728.1272876-1-luciano.coelho@intel.com> (raw) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 5198 bytes --] 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: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Luca Coelho <luciano.coelho@intel.com> --- In v2: * Renamed uncore_spin_*() to intel_spin_*() * Corrected the order: save, lock, unlock, restore In v3: * Undid the change to pass drm_i915_private instead of the lock itself, since we would have to include i915_drv.h and that pulls in a truckload of other includes. In v4: * After a brief attempt to replace this with a different patch, we're back to this one; * Pass drm_i195_private again, and move the functions to intel_vblank.c, so we don't need to include i915_drv.h in a header file and it's already included in intel_vblank.c; drivers/gpu/drm/i915/display/intel_display.h | 1 + drivers/gpu/drm/i915/display/intel_vblank.c | 45 +++++++++++++++----- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h index 8548f49e3972..5ff299bc4b87 100644 --- a/drivers/gpu/drm/i915/display/intel_display.h +++ b/drivers/gpu/drm/i915/display/intel_display.h @@ -29,6 +29,7 @@ #include "i915_reg_defs.h" #include "intel_display_limits.h" +#include "i915_drv.h" enum drm_scaling_filter; struct dpll; diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c index 2cec2abf9746..d9625db82681 100644 --- a/drivers/gpu/drm/i915/display/intel_vblank.c +++ b/drivers/gpu/drm/i915/display/intel_vblank.c @@ -265,6 +265,26 @@ 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. Keep the decision-making centralized + * here. + */ +static inline void intel_vblank_section_enter(struct drm_i915_private *i915) +{ +#ifdef I915 + spin_lock(&i915->uncore.lock); +#endif +} + +static inline void intel_vblank_section_exit(struct drm_i915_private *i915) +{ +#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, @@ -302,11 +322,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. */ @@ -374,7 +395,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 @@ -412,9 +434,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; } @@ -537,7 +563,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); @@ -546,7 +572,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); } -- 2.39.2
WARNING: multiple messages have this Message-ID (diff)
From: Luca Coelho <luciano.coelho@intel.com> To: intel-gfx@lists.freedesktop.org Cc: intel-xe@lists.freedesktop.org, rodrigo.vivi@intel.com Subject: [Intel-xe] [PATCH v4] drm/i915: handle uncore spinlock when not available Date: Wed, 29 Nov 2023 11:17:28 +0200 [thread overview] Message-ID: <20231129091728.1272876-1-luciano.coelho@intel.com> (raw) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 5198 bytes --] 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: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Luca Coelho <luciano.coelho@intel.com> --- In v2: * Renamed uncore_spin_*() to intel_spin_*() * Corrected the order: save, lock, unlock, restore In v3: * Undid the change to pass drm_i915_private instead of the lock itself, since we would have to include i915_drv.h and that pulls in a truckload of other includes. In v4: * After a brief attempt to replace this with a different patch, we're back to this one; * Pass drm_i195_private again, and move the functions to intel_vblank.c, so we don't need to include i915_drv.h in a header file and it's already included in intel_vblank.c; drivers/gpu/drm/i915/display/intel_display.h | 1 + drivers/gpu/drm/i915/display/intel_vblank.c | 45 +++++++++++++++----- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h index 8548f49e3972..5ff299bc4b87 100644 --- a/drivers/gpu/drm/i915/display/intel_display.h +++ b/drivers/gpu/drm/i915/display/intel_display.h @@ -29,6 +29,7 @@ #include "i915_reg_defs.h" #include "intel_display_limits.h" +#include "i915_drv.h" enum drm_scaling_filter; struct dpll; diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c index 2cec2abf9746..d9625db82681 100644 --- a/drivers/gpu/drm/i915/display/intel_vblank.c +++ b/drivers/gpu/drm/i915/display/intel_vblank.c @@ -265,6 +265,26 @@ 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. Keep the decision-making centralized + * here. + */ +static inline void intel_vblank_section_enter(struct drm_i915_private *i915) +{ +#ifdef I915 + spin_lock(&i915->uncore.lock); +#endif +} + +static inline void intel_vblank_section_exit(struct drm_i915_private *i915) +{ +#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, @@ -302,11 +322,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. */ @@ -374,7 +395,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 @@ -412,9 +434,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; } @@ -537,7 +563,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); @@ -546,7 +572,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); } -- 2.39.2
next reply other threads:[~2023-11-29 9:24 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-11-29 9:17 Luca Coelho [this message] 2023-11-29 9:17 ` [Intel-xe] [PATCH v4] drm/i915: handle uncore spinlock when not available Luca Coelho 2023-11-29 18:01 ` [Intel-gfx] " Rodrigo Vivi 2023-11-29 18:01 ` Rodrigo Vivi 2023-11-29 20:24 ` [Intel-gfx] " Coelho, Luciano 2023-11-29 20:24 ` [Intel-xe] [Intel-gfx] " Coelho, Luciano 2023-11-29 20:34 ` [Intel-gfx] [Intel-xe] " Rodrigo Vivi 2023-11-29 20:34 ` [Intel-xe] [Intel-gfx] " Rodrigo Vivi 2023-11-29 21:17 ` [Intel-gfx] [Intel-xe] " Coelho, Luciano 2023-11-29 21:17 ` [Intel-xe] [Intel-gfx] " Coelho, Luciano
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20231129091728.1272876-1-luciano.coelho@intel.com \ --to=luciano.coelho@intel.com \ --cc=intel-gfx@lists.freedesktop.org \ --cc=intel-xe@lists.freedesktop.org \ --cc=rodrigo.vivi@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.