On Mon, Apr 04, 2022 at 05:11:49PM -0700, Lucas De Marchi wrote:
> Since gen6 we use FPGA_DBG register to detect unclaimed MMIO registers.
> This register is in the display engine IP and can only ever detect
> unclaimed accesses to registers in this area. However sometimes there
> are reports of this triggering for registers in other areas, which
> should not be possible.
>
> Right now we always warn after the read/write of registers going through
> unclaimed_reg_debug(). However places using __raw_uncore_* may be
> triggering the unclaimed access and those being later accounted to a
> different register. Let's warn both before and after the read/write
> with a slightly different message, so it's clear if the register
> reported in the warning is actually the culprit.
You should probably probably give an explicit mention of commit
dda960335e020 ("drm/i915: Just clear the mmiodebug before a register
access") in the commit message since we're reversing direction here.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 8b9caaaacc21..df59ec88459e 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1502,11 +1502,10 @@ ilk_dummy_write(struct intel_uncore *uncore)
> static void
> __unclaimed_reg_debug(struct intel_uncore *uncore,
> const i915_reg_t reg,
> - const bool read,
> - const bool before)
> + const bool read)
> {
> if (drm_WARN(&uncore->i915->drm,
> - check_for_unclaimed_mmio(uncore) && !before,
> + check_for_unclaimed_mmio(uncore),
> "Unclaimed %s register 0x%x\n",
Might be simpler to just keep it all in one function and do something
like a
before ? "MMIO operation *before* a " : ""
in the message? Up to you. Either way,
Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> read ? "read from" : "write to",
> i915_mmio_reg_offset(reg)))
> @@ -1514,6 +1513,20 @@ __unclaimed_reg_debug(struct intel_uncore *uncore,
> uncore->i915->params.mmio_debug--;
> }
>
> +static void
> +__unclaimed_previous_reg_debug(struct intel_uncore *uncore,
> + const i915_reg_t reg,
> + const bool read)
> +{
> + if (drm_WARN(&uncore->i915->drm,
> + check_for_unclaimed_mmio(uncore),
> + "Unclaimed access detected before %s register 0x%x\n",
> + read ? "read from" : "write to",
> + i915_mmio_reg_offset(reg)))
> + /* Only report the first N failures */
> + uncore->i915->params.mmio_debug--;
> +}
> +
> static inline void
> unclaimed_reg_debug(struct intel_uncore *uncore,
> const i915_reg_t reg,
> @@ -1526,13 +1539,13 @@ unclaimed_reg_debug(struct intel_uncore *uncore,
> /* interrupts are disabled and re-enabled around uncore->lock usage */
> lockdep_assert_held(&uncore->lock);
>
> - if (before)
> + if (before) {
> spin_lock(&uncore->debug->lock);
> -
> - __unclaimed_reg_debug(uncore, reg, read, before);
> -
> - if (!before)
> + __unclaimed_previous_reg_debug(uncore, reg, read);
> + } else {
> + __unclaimed_reg_debug(uncore, reg, read);
> spin_unlock(&uncore->debug->lock);
> + }
> }
>
> #define __vgpu_read(x) \
> --
> 2.35.1
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
