[Intel-gfx] [PATCH] drm/i915/gt: Manage uncore->lock while waiting on MCR register

[Lucas De Marchi]

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 at intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

thanks
Lucas De Marchi