| Message ID | aqoql4ri3vpe4larpkz4p6hxy76agq6pmn6gunt5xv56hxdbye@72ilwk7rpiu5 (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] i915/gt: Reapply workarounds in case the previous attempt failed. | expand |
On Thu, Dec 05, 2024 at 03:47:35PM +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`. > > This patch aims to resolve: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12668 It should be: 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 | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 570c91878189..4ee623448223 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -1761,6 +1761,17 @@ static void wa_list_apply(const struct i915_wa_list *wal) > intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : > intel_uncore_read_fw(uncore, wa->reg); > > + if ((val ^ wa->set) & wa->read) { > + if (wa->is_mcr) > + intel_gt_mcr_multicast_write_fw(gt, wa->mcr_reg, val); > + else > + intel_uncore_write_fw(uncore, wa->reg, val); > + } instead of duplicating the code you should extract that to an aux function to write it... > + > + val = wa->is_mcr ? > + intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : > + intel_uncore_read_fw(uncore, wa->reg); and another one to read it... > + > wa_verify(gt, wa, val, wal->name, "application"); However my biggest concern with this patch is the brute force solution and only on CONFIG_DRM_I915_DEBUG_GEM case... and as duplication because I see that the second write attempt is already happening above if (val != old || !wa->clr) So, something is not quite right in here and this deserves another alternative.. > } > } > -- > 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..4ee623448223 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1761,6 +1761,17 @@ static void wa_list_apply(const struct i915_wa_list *wal) intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : intel_uncore_read_fw(uncore, wa->reg); + if ((val ^ wa->set) & wa->read) { + if (wa->is_mcr) + intel_gt_mcr_multicast_write_fw(gt, wa->mcr_reg, val); + else + intel_uncore_write_fw(uncore, wa->reg, val); + } + + val = wa->is_mcr ? + intel_gt_mcr_read_any_fw(gt, wa->mcr_reg) : + intel_uncore_read_fw(uncore, wa->reg); + wa_verify(gt, wa, val, wal->name, "application"); } }
`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`. This patch aims to resolve: 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 | 11 +++++++++++ 1 file changed, 11 insertions(+)