| Message ID | 1440332571-20761-6-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Aug 23, 2015 at 05:52:51PM +0530, Sagar Arun Kamble wrote: > From: Alex Dai <yu.dai@intel.com> > [TOR:] This commit message is inadequate. The needed information is in the cover letter but is lacking here. Please rebase with Alex's previous patch "drm/i915: Notify GuC rc6 state" > Signed-off-by: Alex Dai <yu.dai@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/i915_guc_submission.c | 18 ++++++++++++++++++ > drivers/gpu/drm/i915/intel_guc.h | 1 + > drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++---- > 3 files changed, 31 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > index ec70393..462c679 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -877,6 +877,24 @@ int i915_guc_submission_enable(struct drm_device *dev) > return 0; > } > > +void i915_guc_sample_forcewake(struct drm_device *dev, u32 fw_data) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; > + > + /* Notify GuC about CPG changes. */ > + if (guc_fw->guc_fw_fetch_status == GUC_FIRMWARE_SUCCESS) { > + u32 data[2]; > + > + data[0] = HOST2GUC_ACTION_SAMPLE_FORCEWAKE; > + data[1] = fw_data; > + > + if (host2guc_action(guc, data, 2)) > + DRM_ERROR("Unable to notify GuC of CPG change\n"); > + } > +} > + > void i915_guc_submission_disable(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h > index 4ec2d27..691574d 100644 > --- a/drivers/gpu/drm/i915/intel_guc.h > +++ b/drivers/gpu/drm/i915/intel_guc.h > @@ -118,5 +118,6 @@ int i915_guc_submit(struct i915_guc_client *client, > struct drm_i915_gem_request *rq); > void i915_guc_submission_disable(struct drm_device *dev); > void i915_guc_submission_fini(struct drm_device *dev); > +void i915_guc_sample_forcewake(struct drm_device *dev, u32 fw_data); > > #endif > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index c0345d2..4a0483c 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4616,6 +4616,9 @@ static void gen9_disable_rps(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > > I915_WRITE(GEN6_RC_CONTROL, 0); > + > + i915_guc_sample_forcewake(dev, 0); > + > I915_WRITE(GEN9_PG_ENABLE, 0); > } > > @@ -4804,6 +4807,7 @@ static void gen9_enable_rc6(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_engine_cs *ring; > uint32_t rc6_mask = 0; > + uint32_t cpg_data = 0; > int unused; > > /* 1a: Software RC state - RC0 */ > @@ -4843,11 +4847,15 @@ static void gen9_enable_rc6(struct drm_device *dev) > * WaRsDisableCoarsePowerGating:skl,bxt - Render/Media PG need to be disabled with RC6. > */ > if ((IS_BROXTON(dev) && (INTEL_REVID(dev) < BXT_REVID_B0)) || > - (IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0))) > + (IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0))) { > + i915_guc_sample_forcewake(dev, 0); > I915_WRITE(GEN9_PG_ENABLE, 0); > - else > - I915_WRITE(GEN9_PG_ENABLE, (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? > - (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE) : 0); > + } else { > + cpg_data = (rc6_mask & GEN6_RC_CTL_RC6_ENABLE)? > + (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE):0; > + i915_guc_sample_forcewake(dev, cpg_data); > + I915_WRITE(GEN9_PG_ENABLE, cpg_data); > + } > > intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); > > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Alex, Kindly incorporate changes in this patch in your patch at: http://lists.freedesktop.org/archives/intel-gfx/2015-September/075668.html - [PATCH 5/6] drm/i915/guc: Media domain bit needed when notify GuC rc6 state This is because GuC sample forcewake parameters depend on Coarse power gating configuration. Thanks Sagar On 8/28/2015 1:25 AM, O'Rourke, Tom wrote: > On Sun, Aug 23, 2015 at 05:52:51PM +0530, Sagar Arun Kamble wrote: >> From: Alex Dai <yu.dai@intel.com> >> > [TOR:] This commit message is inadequate. The > needed information is in the cover letter but > is lacking here. Please rebase with Alex's > previous patch "drm/i915: Notify GuC rc6 state" > >> Signed-off-by: Alex Dai <yu.dai@intel.com> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> --- >> drivers/gpu/drm/i915/i915_guc_submission.c | 18 ++++++++++++++++++ >> drivers/gpu/drm/i915/intel_guc.h | 1 + >> drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++---- >> 3 files changed, 31 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c >> index ec70393..462c679 100644 >> --- a/drivers/gpu/drm/i915/i915_guc_submission.c >> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c >> @@ -877,6 +877,24 @@ int i915_guc_submission_enable(struct drm_device *dev) >> return 0; >> } >> >> +void i915_guc_sample_forcewake(struct drm_device *dev, u32 fw_data) >> +{ >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + struct intel_guc *guc = &dev_priv->guc; >> + struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; >> + >> + /* Notify GuC about CPG changes. */ >> + if (guc_fw->guc_fw_fetch_status == GUC_FIRMWARE_SUCCESS) { >> + u32 data[2]; >> + >> + data[0] = HOST2GUC_ACTION_SAMPLE_FORCEWAKE; >> + data[1] = fw_data; >> + >> + if (host2guc_action(guc, data, 2)) >> + DRM_ERROR("Unable to notify GuC of CPG change\n"); >> + } >> +} >> + >> void i915_guc_submission_disable(struct drm_device *dev) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h >> index 4ec2d27..691574d 100644 >> --- a/drivers/gpu/drm/i915/intel_guc.h >> +++ b/drivers/gpu/drm/i915/intel_guc.h >> @@ -118,5 +118,6 @@ int i915_guc_submit(struct i915_guc_client *client, >> struct drm_i915_gem_request *rq); >> void i915_guc_submission_disable(struct drm_device *dev); >> void i915_guc_submission_fini(struct drm_device *dev); >> +void i915_guc_sample_forcewake(struct drm_device *dev, u32 fw_data); >> >> #endif >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index c0345d2..4a0483c 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -4616,6 +4616,9 @@ static void gen9_disable_rps(struct drm_device *dev) >> struct drm_i915_private *dev_priv = dev->dev_private; >> >> I915_WRITE(GEN6_RC_CONTROL, 0); >> + >> + i915_guc_sample_forcewake(dev, 0); >> + >> I915_WRITE(GEN9_PG_ENABLE, 0); >> } >> >> @@ -4804,6 +4807,7 @@ static void gen9_enable_rc6(struct drm_device *dev) >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_engine_cs *ring; >> uint32_t rc6_mask = 0; >> + uint32_t cpg_data = 0; >> int unused; >> >> /* 1a: Software RC state - RC0 */ >> @@ -4843,11 +4847,15 @@ static void gen9_enable_rc6(struct drm_device *dev) >> * WaRsDisableCoarsePowerGating:skl,bxt - Render/Media PG need to be disabled with RC6. >> */ >> if ((IS_BROXTON(dev) && (INTEL_REVID(dev) < BXT_REVID_B0)) || >> - (IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0))) >> + (IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0))) { >> + i915_guc_sample_forcewake(dev, 0); >> I915_WRITE(GEN9_PG_ENABLE, 0); >> - else >> - I915_WRITE(GEN9_PG_ENABLE, (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? >> - (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE) : 0); >> + } else { >> + cpg_data = (rc6_mask & GEN6_RC_CTL_RC6_ENABLE)? >> + (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE):0; >> + i915_guc_sample_forcewake(dev, cpg_data); >> + I915_WRITE(GEN9_PG_ENABLE, cpg_data); >> + } >> >> intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); >> >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 09/10/2015 11:16 PM, Kamble, Sagar A wrote: > Hi Alex, > > Kindly incorporate changes in this patch in your patch at: > http://lists.freedesktop.org/archives/intel-gfx/2015-September/075668.html > - [PATCH 5/6] drm/i915/guc: Media domain bit needed when notify GuC rc6 > state > > This is because GuC sample forcewake parameters depend on Coarse power > gating configuration. > > Thanks > Sagar > > > > On 8/28/2015 1:25 AM, O'Rourke, Tom wrote: > > On Sun, Aug 23, 2015 at 05:52:51PM +0530, Sagar Arun Kamble wrote: > >> From: Alex Dai <yu.dai@intel.com> > >> > > [TOR:] This commit message is inadequate. The > > needed information is in the cover letter but > > is lacking here. Please rebase with Alex's > > previous patch "drm/i915: Notify GuC rc6 state" > > > >> Signed-off-by: Alex Dai <yu.dai@intel.com> > >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_guc_submission.c | 18 ++++++++++++++++++ > >> drivers/gpu/drm/i915/intel_guc.h | 1 + > >> drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++---- > >> 3 files changed, 31 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > >> index ec70393..462c679 100644 > >> --- a/drivers/gpu/drm/i915/i915_guc_submission.c > >> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > >> @@ -877,6 +877,24 @@ int i915_guc_submission_enable(struct drm_device *dev) > >> return 0; > >> } > >> > >> +void i915_guc_sample_forcewake(struct drm_device *dev, u32 fw_data) > >> +{ > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + struct intel_guc *guc = &dev_priv->guc; > >> + struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; > >> + > >> + /* Notify GuC about CPG changes. */ > >> + if (guc_fw->guc_fw_fetch_status == GUC_FIRMWARE_SUCCESS) { This might not be safe to protect code below. In the case of suspend, the guc_fw_fetch_status and guc_fw_load_status both are SUCCESS. However, we need to reload fw before any host2guc calls. > >> + u32 data[2]; > >> + > >> + data[0] = HOST2GUC_ACTION_SAMPLE_FORCEWAKE; > >> + data[1] = fw_data; > >> + > >> + if (host2guc_action(guc, data, 2)) > >> + DRM_ERROR("Unable to notify GuC of CPG change\n"); > >> + } > >> +} > >> + Please call host2guc_sample_forcewake rather than a host2guc_action. > >> void i915_guc_submission_disable(struct drm_device *dev) > >> { > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h > >> index 4ec2d27..691574d 100644 > >> --- a/drivers/gpu/drm/i915/intel_guc.h > >> +++ b/drivers/gpu/drm/i915/intel_guc.h > >> @@ -118,5 +118,6 @@ int i915_guc_submit(struct i915_guc_client *client, > >> struct drm_i915_gem_request *rq); > >> void i915_guc_submission_disable(struct drm_device *dev); > >> void i915_guc_submission_fini(struct drm_device *dev); > >> +void i915_guc_sample_forcewake(struct drm_device *dev, u32 fw_data); > >> > >> #endif > >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > >> index c0345d2..4a0483c 100644 > >> --- a/drivers/gpu/drm/i915/intel_pm.c > >> +++ b/drivers/gpu/drm/i915/intel_pm.c > >> @@ -4616,6 +4616,9 @@ static void gen9_disable_rps(struct drm_device *dev) > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> > >> I915_WRITE(GEN6_RC_CONTROL, 0); > >> + > >> + i915_guc_sample_forcewake(dev, 0); > >> + > >> I915_WRITE(GEN9_PG_ENABLE, 0); > >> } > >> > >> @@ -4804,6 +4807,7 @@ static void gen9_enable_rc6(struct drm_device *dev) > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> struct intel_engine_cs *ring; > >> uint32_t rc6_mask = 0; > >> + uint32_t cpg_data = 0; > >> int unused; > >> > >> /* 1a: Software RC state - RC0 */ > >> @@ -4843,11 +4847,15 @@ static void gen9_enable_rc6(struct drm_device *dev) > >> * WaRsDisableCoarsePowerGating:skl,bxt - Render/Media PG need to be disabled with RC6. > >> */ > >> if ((IS_BROXTON(dev) && (INTEL_REVID(dev) < BXT_REVID_B0)) || > >> - (IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0))) > >> + (IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0))) { > >> + i915_guc_sample_forcewake(dev, 0); > >> I915_WRITE(GEN9_PG_ENABLE, 0); > >> - else > >> - I915_WRITE(GEN9_PG_ENABLE, (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? > >> - (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE) : 0); > >> + } else { > >> + cpg_data = (rc6_mask & GEN6_RC_CTL_RC6_ENABLE)? > >> + (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE):0; > >> + i915_guc_sample_forcewake(dev, cpg_data); > >> + I915_WRITE(GEN9_PG_ENABLE, cpg_data); > >> + } > >> > >> intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); > >> > >> -- Be note that calling guc function here actually happens in another thread. The host2guc action will fail if GuC is not loaded. So there is a potential racing issue here. I believe the failure will happen during suspend / resume. I believe calling i915_guc_sample_forcewake or host2guc_sample_forcewake after GuC is initialized is good enough.
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index ec70393..462c679 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -877,6 +877,24 @@ int i915_guc_submission_enable(struct drm_device *dev) return 0; } +void i915_guc_sample_forcewake(struct drm_device *dev, u32 fw_data) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_guc *guc = &dev_priv->guc; + struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; + + /* Notify GuC about CPG changes. */ + if (guc_fw->guc_fw_fetch_status == GUC_FIRMWARE_SUCCESS) { + u32 data[2]; + + data[0] = HOST2GUC_ACTION_SAMPLE_FORCEWAKE; + data[1] = fw_data; + + if (host2guc_action(guc, data, 2)) + DRM_ERROR("Unable to notify GuC of CPG change\n"); + } +} + void i915_guc_submission_disable(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h index 4ec2d27..691574d 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -118,5 +118,6 @@ int i915_guc_submit(struct i915_guc_client *client, struct drm_i915_gem_request *rq); void i915_guc_submission_disable(struct drm_device *dev); void i915_guc_submission_fini(struct drm_device *dev); +void i915_guc_sample_forcewake(struct drm_device *dev, u32 fw_data); #endif diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c0345d2..4a0483c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4616,6 +4616,9 @@ static void gen9_disable_rps(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; I915_WRITE(GEN6_RC_CONTROL, 0); + + i915_guc_sample_forcewake(dev, 0); + I915_WRITE(GEN9_PG_ENABLE, 0); } @@ -4804,6 +4807,7 @@ static void gen9_enable_rc6(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_engine_cs *ring; uint32_t rc6_mask = 0; + uint32_t cpg_data = 0; int unused; /* 1a: Software RC state - RC0 */ @@ -4843,11 +4847,15 @@ static void gen9_enable_rc6(struct drm_device *dev) * WaRsDisableCoarsePowerGating:skl,bxt - Render/Media PG need to be disabled with RC6. */ if ((IS_BROXTON(dev) && (INTEL_REVID(dev) < BXT_REVID_B0)) || - (IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0))) + (IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0))) { + i915_guc_sample_forcewake(dev, 0); I915_WRITE(GEN9_PG_ENABLE, 0); - else - I915_WRITE(GEN9_PG_ENABLE, (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? - (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE) : 0); + } else { + cpg_data = (rc6_mask & GEN6_RC_CTL_RC6_ENABLE)? + (GEN9_RENDER_PG_ENABLE | GEN9_MEDIA_PG_ENABLE):0; + i915_guc_sample_forcewake(dev, cpg_data); + I915_WRITE(GEN9_PG_ENABLE, cpg_data); + } intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);