On Fri, Nov 18, 2022 at 01:20:45PM -0800, Lucas De Marchi wrote:
> On Thu, Nov 17, 2022 at 09:33:58AM -0800, Matt Roper wrote:
> > The GT MCR code currently relies on uncore->lock to avoid race
> > conditions on the steering control register during MCR operations. The
> > *_fw() versions of MCR operations expect the caller to already hold
> > uncore->lock, while the non-fw variants manage the lock internally.
> > However the sole callsite of intel_gt_mcr_wait_for_reg_fw() does not
> > currently obtain the forcewake lock, allowing a potential race condition
> > (and triggering an assertion on lockdep builds). Furthermore, since
> > 'wait for register value' requests may not return immediately, it is
> > undesirable to hold a fundamental lock like uncore->lock for the entire
> > wait and block all other MMIO for the duration; rather the lock is only
> > needed around the MCR read operations and can be released during the
> > delays.
> >
> > Convert intel_gt_mcr_wait_for_reg_fw() to a non-fw variant that will
> > manage uncore->lock internally. This does have the side effect of
> > causing an unnecessary lookup in the forcewake table on each read
> > operation, but since the caller is still holding the relevant forcewake
> > domain, this will ultimately just incremenent the reference count and
> > won't actually cause any additional MMIO traffic.
> >
> > In the future we plan to switch to a dedicated MCR lock to protect the
> > steering critical section rather than using the overloaded and
> > high-traffic uncore->lock; on MTL and beyond the new lock can be
> > implemented on top of the hardware-provided synchonization mechanism for
> > steering.
> >
> > Fixes: 3068bec83eea ("drm/i915/gt: Add intel_gt_mcr_wait_for_reg_fw()")
> > Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
>
>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Applied to drm-intel-gt-next. Thanks for the review.
Matt
>
> thanks
> Lucas De Marchi
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
