| Message ID | 20230822152859.1586761-3-oak.zeng@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: Create a blitter context for GGTT updates | expand |
On Tue, Aug 22, 2023 at 11:28:59AM -0400, Oak Zeng wrote: > From: Nirmoy Das <nirmoy.das@intel.com> > > MTL can hang because of a HW bug while parallel reading/writing > from/to LMEM/GTTMMADR BAR so try to reduce GGTT update > related pci transactions with blitter command as recommended > for Wa_22018444074. Drive-by comment: this isn't a valid workaround number. 22018444074 is a per-platform record number, whereas workarounds should always be identified by their cross-platform lineage number, which will stay constant if the workaround winds up extending to future platforms as well. So in this case, the workaround should be referred to as Wa_13010847436. Matt > > To issue blitter commands, the driver must be primed to receive > requests. Maintain blitter-based GGTT update disablement until driver > probing completes. Moreover, implement a temporary disablement > of blitter prior to entering suspend, followed by re-enablement > post-resume. This is acceptable as those transition periods are > mostly single threaded. > > v2: Disable GGTT blitter prior to runtime suspend and re-enable > after runtime resume. (Oak) > > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> > Signed-off-by: Oak Zeng <oak.zeng@intel.com> > --- > drivers/gpu/drm/i915/i915_driver.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > index f8dbee7a5af7..6afe0adc8ddb 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -815,6 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > i915_welcome_messages(i915); > > i915->do_release = true; > + intel_engine_blitter_context_set_ready(to_gt(i915), true); > > return 0; > > @@ -855,6 +856,7 @@ void i915_driver_remove(struct drm_i915_private *i915) > { > intel_wakeref_t wakeref; > > + intel_engine_blitter_context_set_ready(to_gt(i915), false); > wakeref = intel_runtime_pm_get(&i915->runtime_pm); > > i915_driver_unregister(i915); > @@ -1077,6 +1079,8 @@ static int i915_drm_suspend(struct drm_device *dev) > struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); > pci_power_t opregion_target_state; > > + intel_engine_blitter_context_set_ready(to_gt(dev_priv), false); > + > disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > /* We do a lot of poking in a lot of registers, make sure they work > @@ -1264,6 +1268,7 @@ static int i915_drm_resume(struct drm_device *dev) > intel_gvt_resume(dev_priv); > > enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > + intel_engine_blitter_context_set_ready(to_gt(dev_priv), true); > > return 0; > } > @@ -1515,6 +1520,7 @@ static int intel_runtime_suspend(struct device *kdev) > if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv))) > return -ENODEV; > > + intel_engine_blitter_context_set_ready(to_gt(dev_priv), false); > drm_dbg(&dev_priv->drm, "Suspending device\n"); > > disable_rpm_wakeref_asserts(rpm); > @@ -1669,6 +1675,8 @@ static int intel_runtime_resume(struct device *kdev) > else > drm_dbg(&dev_priv->drm, "Device resumed\n"); > > + intel_engine_blitter_context_set_ready(to_gt(dev_priv), true); > + > return ret; > } > > -- > 2.26.3 >
Thanks, Oak > -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@intel.com> > Sent: August 24, 2023 11:54 AM > To: Zeng, Oak <oak.zeng@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Shyti, Andi <andi.shyti@intel.com>; > chris.p.wilson@linux.intel.com; Das, Nirmoy <nirmoy.das@intel.com> > Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Enable GGTT blitting in MTL > > On Tue, Aug 22, 2023 at 11:28:59AM -0400, Oak Zeng wrote: > > From: Nirmoy Das <nirmoy.das@intel.com> > > > > MTL can hang because of a HW bug while parallel reading/writing > > from/to LMEM/GTTMMADR BAR so try to reduce GGTT update > > related pci transactions with blitter command as recommended > > for Wa_22018444074. > > Drive-by comment: this isn't a valid workaround number. 22018444074 is > a per-platform record number, whereas workarounds should always be > identified by their cross-platform lineage number, which will stay > constant if the workaround winds up extending to future platforms as > well. So in this case, the workaround should be referred to as > Wa_13010847436. Spoke with Matt offline. Will keep lineage number 13010847436. Also put a soc HSD number 14019519902 Thanks Matt for explanation! Oak > > > Matt > > > > > To issue blitter commands, the driver must be primed to receive > > requests. Maintain blitter-based GGTT update disablement until driver > > probing completes. Moreover, implement a temporary disablement > > of blitter prior to entering suspend, followed by re-enablement > > post-resume. This is acceptable as those transition periods are > > mostly single threaded. > > > > v2: Disable GGTT blitter prior to runtime suspend and re-enable > > after runtime resume. (Oak) > > > > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> > > Signed-off-by: Oak Zeng <oak.zeng@intel.com> > > --- > > drivers/gpu/drm/i915/i915_driver.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_driver.c > b/drivers/gpu/drm/i915/i915_driver.c > > index f8dbee7a5af7..6afe0adc8ddb 100644 > > --- a/drivers/gpu/drm/i915/i915_driver.c > > +++ b/drivers/gpu/drm/i915/i915_driver.c > > @@ -815,6 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct > pci_device_id *ent) > > i915_welcome_messages(i915); > > > > i915->do_release = true; > > + intel_engine_blitter_context_set_ready(to_gt(i915), true); > > > > return 0; > > > > @@ -855,6 +856,7 @@ void i915_driver_remove(struct drm_i915_private *i915) > > { > > intel_wakeref_t wakeref; > > > > + intel_engine_blitter_context_set_ready(to_gt(i915), false); > > wakeref = intel_runtime_pm_get(&i915->runtime_pm); > > > > i915_driver_unregister(i915); > > @@ -1077,6 +1079,8 @@ static int i915_drm_suspend(struct drm_device *dev) > > struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); > > pci_power_t opregion_target_state; > > > > + intel_engine_blitter_context_set_ready(to_gt(dev_priv), false); > > + > > disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > > > /* We do a lot of poking in a lot of registers, make sure they work > > @@ -1264,6 +1268,7 @@ static int i915_drm_resume(struct drm_device *dev) > > intel_gvt_resume(dev_priv); > > > > enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > + intel_engine_blitter_context_set_ready(to_gt(dev_priv), true); > > > > return 0; > > } > > @@ -1515,6 +1520,7 @@ static int intel_runtime_suspend(struct device *kdev) > > if (drm_WARN_ON_ONCE(&dev_priv- > >drm, !HAS_RUNTIME_PM(dev_priv))) > > return -ENODEV; > > > > + intel_engine_blitter_context_set_ready(to_gt(dev_priv), false); > > drm_dbg(&dev_priv->drm, "Suspending device\n"); > > > > disable_rpm_wakeref_asserts(rpm); > > @@ -1669,6 +1675,8 @@ static int intel_runtime_resume(struct device *kdev) > > else > > drm_dbg(&dev_priv->drm, "Device resumed\n"); > > > > + intel_engine_blitter_context_set_ready(to_gt(dev_priv), true); > > + > > return ret; > > } > > > > -- > > 2.26.3 > > > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index f8dbee7a5af7..6afe0adc8ddb 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -815,6 +815,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) i915_welcome_messages(i915); i915->do_release = true; + intel_engine_blitter_context_set_ready(to_gt(i915), true); return 0; @@ -855,6 +856,7 @@ void i915_driver_remove(struct drm_i915_private *i915) { intel_wakeref_t wakeref; + intel_engine_blitter_context_set_ready(to_gt(i915), false); wakeref = intel_runtime_pm_get(&i915->runtime_pm); i915_driver_unregister(i915); @@ -1077,6 +1079,8 @@ static int i915_drm_suspend(struct drm_device *dev) struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); pci_power_t opregion_target_state; + intel_engine_blitter_context_set_ready(to_gt(dev_priv), false); + disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); /* We do a lot of poking in a lot of registers, make sure they work @@ -1264,6 +1268,7 @@ static int i915_drm_resume(struct drm_device *dev) intel_gvt_resume(dev_priv); enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); + intel_engine_blitter_context_set_ready(to_gt(dev_priv), true); return 0; } @@ -1515,6 +1520,7 @@ static int intel_runtime_suspend(struct device *kdev) if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv))) return -ENODEV; + intel_engine_blitter_context_set_ready(to_gt(dev_priv), false); drm_dbg(&dev_priv->drm, "Suspending device\n"); disable_rpm_wakeref_asserts(rpm); @@ -1669,6 +1675,8 @@ static int intel_runtime_resume(struct device *kdev) else drm_dbg(&dev_priv->drm, "Device resumed\n"); + intel_engine_blitter_context_set_ready(to_gt(dev_priv), true); + return ret; }