Message ID | 20230927102237.30773-1-nirmoy.das@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/3] drm/i915: Introduce intel_gt_mcr_lock_reset() | expand |
Hi Nirmoy, [...] > +void intel_gt_mcr_lock_reset(struct intel_gt *gt) > +{ > + unsigned long __flags; > + > + lockdep_assert_not_held(>->uncore->lock); > + > + spin_lock_irqsave(>->mcr_lock, __flags); > + > + if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) > + intel_uncore_write_fw(gt->uncore, MTL_STEER_SEMAPHORE, 0x1); > + > + spin_unlock_irqrestore(>->mcr_lock, __flags); As we discussed offline, I don't think spinlocks are needed here. I don't expect the gt to be holding the lock somewhere else. Andi > +}
Hi Andi, On 9/27/2023 1:37 PM, Andi Shyti wrote: > Hi Nirmoy, > > [...] > >> +void intel_gt_mcr_lock_reset(struct intel_gt *gt) >> +{ >> + unsigned long __flags; >> + >> + lockdep_assert_not_held(>->uncore->lock); >> + >> + spin_lock_irqsave(>->mcr_lock, __flags); >> + >> + if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) >> + intel_uncore_write_fw(gt->uncore, MTL_STEER_SEMAPHORE, 0x1); >> + >> + spin_unlock_irqrestore(>->mcr_lock, __flags); > As we discussed offline, I don't think spinlocks are needed here. > I don't expect the gt to be holding the lock somewhere else. I can add a assert but I think it make sense to have the lock just to be 100% safe ? Regards, Nirmoy > > Andi > >> +}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c index bf4a933de03a..d98e0d2fc2ee 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.c @@ -419,6 +419,35 @@ void intel_gt_mcr_unlock(struct intel_gt *gt, unsigned long flags) intel_uncore_write_fw(gt->uncore, MTL_STEER_SEMAPHORE, 0x1); } +/** + * intel_gt_mcr_lock_reset - Reset MCR steering lock + * @gt: GT structure + * + * Performs a steer semaphore reset operation. On MTL and beyond, a hardware + * lock will also be taken to serialize access not only for the driver, + * but also for external hardware and firmware agents. + * However, there may be situations where the driver must reset the semaphore + * but only when it is absolutely certain that no other agent should own the + * lock at that given time. + * + * Context: Takes gt->mcr_lock. uncore->lock should *not* be held when this + * function is called, although it may be acquired after this + * function call. + */ +void intel_gt_mcr_lock_reset(struct intel_gt *gt) +{ + unsigned long __flags; + + lockdep_assert_not_held(>->uncore->lock); + + spin_lock_irqsave(>->mcr_lock, __flags); + + if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70)) + intel_uncore_write_fw(gt->uncore, MTL_STEER_SEMAPHORE, 0x1); + + spin_unlock_irqrestore(>->mcr_lock, __flags); +} + /** * intel_gt_mcr_read - read a specific instance of an MCR register * @gt: GT structure diff --git a/drivers/gpu/drm/i915/gt/intel_gt_mcr.h b/drivers/gpu/drm/i915/gt/intel_gt_mcr.h index 41684495b7da..485c7711f2e8 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_mcr.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_mcr.h @@ -11,6 +11,7 @@ void intel_gt_mcr_init(struct intel_gt *gt); void intel_gt_mcr_lock(struct intel_gt *gt, unsigned long *flags); void intel_gt_mcr_unlock(struct intel_gt *gt, unsigned long flags); +void intel_gt_mcr_lock_reset(struct intel_gt *gt); u32 intel_gt_mcr_read(struct intel_gt *gt, i915_mcr_reg_t reg,
Implement intel_gt_mcr_lock_reset() to provide a mechanism for resetting the steer semaphore when absolutely necessary. Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> --- drivers/gpu/drm/i915/gt/intel_gt_mcr.c | 29 ++++++++++++++++++++++++++ drivers/gpu/drm/i915/gt/intel_gt_mcr.h | 1 + 2 files changed, 30 insertions(+)