| Message ID | 752zde6fl47boamqiccdhz2wmkxoee5rmzqucphvglhs53bclz@jlv5clqnxngh (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] i915/gt: Reapply workarounds in case the previous attempt failed. | expand |
On Mon, Dec 16, 2024 at 03:46:49PM +0000, Sebastian Brzezinka wrote: > `wa_verify`sporadically detects lost workaround on application; this > is unusual behavior since wa are applied at `intel_gt_init_hw` and > verified right away by `intel_gt_verify_workarounds`, and `wa_verify` > doesn't fail on initialization as one might suspect would happen. > > One approach that may be somewhat beneficial is to reapply workarounds > in the event of failure, or even get rid of verify on application, > since it's redundant to `intel_gt_verify_workarounds`. > > v2: Remove duplicate code, move workarounds read/write > to separated functions. I responded on v1 before I saw that a v2 had been sent here. https://lore.kernel.org/all/20241216212751.GZ5725@mdroper-desk1.amr.corp.intel.com/ Matt > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12668 > > Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 60 ++++++++++++--------- > 1 file changed, 36 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 570c91878189..c0bf909afe8e 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -1722,6 +1722,30 @@ wa_verify(struct intel_gt *gt, const struct i915_wa *wa, u32 cur, > return true; > } > > +static u32 wa_read_fw(struct intel_gt *gt, struct i915_wa *wa) > +{ > + return wa->is_mcr ? intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : > + intel_uncore_read_fw(gt->uncore, wa->reg); > + > +} > + > +static void wa_write_fw(struct intel_gt *gt, struct i915_wa *wa) > +{ > + u32 val, old = 0; > + > + /* open-coded rmw due to steering */ > + if (wa->clr) > + old = wa_read_fw(gt, wa); > + > + val = (old & ~wa->clr) | wa->set; > + if (val != old || !wa->clr) { > + if (wa->is_mcr) > + intel_gt_mcr_multicast_write_fw(gt, wa->mcr_reg, val); > + else > + intel_uncore_write_fw(gt->uncore, wa->reg, val); > + } > +} > + > static void wa_list_apply(const struct i915_wa_list *wal) > { > struct intel_gt *gt = wal->gt; > @@ -1741,28 +1765,17 @@ static void wa_list_apply(const struct i915_wa_list *wal) > intel_uncore_forcewake_get__locked(uncore, fw); > > for (i = 0, wa = wal->list; i < wal->count; i++, wa++) { > - u32 val, old = 0; > - > - /* open-coded rmw due to steering */ > - if (wa->clr) > - old = wa->is_mcr ? > - intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : > - intel_uncore_read_fw(uncore, wa->reg); > - val = (old & ~wa->clr) | wa->set; > - if (val != old || !wa->clr) { > - if (wa->is_mcr) > - intel_gt_mcr_multicast_write_fw(gt, wa->mcr_reg, val); > - else > - intel_uncore_write_fw(uncore, wa->reg, val); > - } > - > - if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) { > - u32 val = wa->is_mcr ? > - intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : > - intel_uncore_read_fw(uncore, wa->reg); > + /* > + * Writing workarounds can sporadically fail, > + * in which case try to apply it again. > + */ > + uint repeat = 1; > > - wa_verify(gt, wa, val, wal->name, "application"); > - } > + do { > + wa_write_fw(gt, wa); > + } while (!wa_verify(gt, wa, wa_read_fw(gt, wa), wal->name, > + "application") > + && repeat--); > } > > intel_uncore_forcewake_put__locked(uncore, fw); > @@ -1793,9 +1806,8 @@ static bool wa_list_verify(struct intel_gt *gt, > intel_uncore_forcewake_get__locked(uncore, fw); > > for (i = 0, wa = wal->list; i < wal->count; i++, wa++) > - ok &= wa_verify(wal->gt, wa, wa->is_mcr ? > - intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : > - intel_uncore_read_fw(uncore, wa->reg), > + ok &= wa_verify(wal->gt, wa, > + wa_read_fw(wal->gt, wa), > wal->name, from); > > intel_uncore_forcewake_put__locked(uncore, fw); > -- > 2.34.1 >
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 570c91878189..c0bf909afe8e 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1722,6 +1722,30 @@ wa_verify(struct intel_gt *gt, const struct i915_wa *wa, u32 cur, return true; } +static u32 wa_read_fw(struct intel_gt *gt, struct i915_wa *wa) +{ + return wa->is_mcr ? intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : + intel_uncore_read_fw(gt->uncore, wa->reg); + +} + +static void wa_write_fw(struct intel_gt *gt, struct i915_wa *wa) +{ + u32 val, old = 0; + + /* open-coded rmw due to steering */ + if (wa->clr) + old = wa_read_fw(gt, wa); + + val = (old & ~wa->clr) | wa->set; + if (val != old || !wa->clr) { + if (wa->is_mcr) + intel_gt_mcr_multicast_write_fw(gt, wa->mcr_reg, val); + else + intel_uncore_write_fw(gt->uncore, wa->reg, val); + } +} + static void wa_list_apply(const struct i915_wa_list *wal) { struct intel_gt *gt = wal->gt; @@ -1741,28 +1765,17 @@ static void wa_list_apply(const struct i915_wa_list *wal) intel_uncore_forcewake_get__locked(uncore, fw); for (i = 0, wa = wal->list; i < wal->count; i++, wa++) { - u32 val, old = 0; - - /* open-coded rmw due to steering */ - if (wa->clr) - old = wa->is_mcr ? - intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : - intel_uncore_read_fw(uncore, wa->reg); - val = (old & ~wa->clr) | wa->set; - if (val != old || !wa->clr) { - if (wa->is_mcr) - intel_gt_mcr_multicast_write_fw(gt, wa->mcr_reg, val); - else - intel_uncore_write_fw(uncore, wa->reg, val); - } - - if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) { - u32 val = wa->is_mcr ? - intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : - intel_uncore_read_fw(uncore, wa->reg); + /* + * Writing workarounds can sporadically fail, + * in which case try to apply it again. + */ + uint repeat = 1; - wa_verify(gt, wa, val, wal->name, "application"); - } + do { + wa_write_fw(gt, wa); + } while (!wa_verify(gt, wa, wa_read_fw(gt, wa), wal->name, + "application") + && repeat--); } intel_uncore_forcewake_put__locked(uncore, fw); @@ -1793,9 +1806,8 @@ static bool wa_list_verify(struct intel_gt *gt, intel_uncore_forcewake_get__locked(uncore, fw); for (i = 0, wa = wal->list; i < wal->count; i++, wa++) - ok &= wa_verify(wal->gt, wa, wa->is_mcr ? - intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : - intel_uncore_read_fw(uncore, wa->reg), + ok &= wa_verify(wal->gt, wa, + wa_read_fw(wal->gt, wa), wal->name, from); intel_uncore_forcewake_put__locked(uncore, fw);
`wa_verify`sporadically detects lost workaround on application; this is unusual behavior since wa are applied at `intel_gt_init_hw` and verified right away by `intel_gt_verify_workarounds`, and `wa_verify` doesn't fail on initialization as one might suspect would happen. One approach that may be somewhat beneficial is to reapply workarounds in the event of failure, or even get rid of verify on application, since it's redundant to `intel_gt_verify_workarounds`. v2: Remove duplicate code, move workarounds read/write to separated functions. Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12668 Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 60 ++++++++++++--------- 1 file changed, 36 insertions(+), 24 deletions(-)