* [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle @ 2014-06-05 20:49 Jesse Barnes 2014-06-06 8:29 ` Ville Syrjälä 0 siblings, 1 reply; 12+ messages in thread From: Jesse Barnes @ 2014-06-05 20:49 UTC (permalink / raw) To: intel-gfx This may take awhile (~10ms), and we don't need to make noise about it. References: https://bugs.freedesktop.org/show_bug.cgi?id=75244 Tested-by: huax.lu@intel.com Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/intel_pm.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ee27d74..4eebfd8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, dev_priv->rps.min_freq_softlimit); - if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) - & GENFREQSTATUS) == 0, 5)) - DRM_ERROR("timed out waiting for Punit\n"); - vlv_force_gfx_clock(dev_priv, false); I915_WRITE(GEN6_PMINTRMSK, -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-05 20:49 [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle Jesse Barnes @ 2014-06-06 8:29 ` Ville Syrjälä 2014-06-06 15:03 ` Jesse Barnes 0 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2014-06-06 8:29 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx On Thu, Jun 05, 2014 at 01:49:34PM -0700, Jesse Barnes wrote: > This may take awhile (~10ms), and we don't need to make noise about it. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=75244 > Tested-by: huax.lu@intel.com > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/intel_pm.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index ee27d74..4eebfd8 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) > vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, > dev_priv->rps.min_freq_softlimit); > > - if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) > - & GENFREQSTATUS) == 0, 5)) > - DRM_ERROR("timed out waiting for Punit\n"); > - As I recall the entire point of this was to make sure the frequency really drops before we allow turning off the clock. I'm not sure if we can trust that the frequency (and more importantly the voltage) will drop if we allow turning off the clock before the transition is complete. > vlv_force_gfx_clock(dev_priv, false); > > I915_WRITE(GEN6_PMINTRMSK, > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-06 8:29 ` Ville Syrjälä @ 2014-06-06 15:03 ` Jesse Barnes 2014-06-11 16:56 ` Ville Syrjälä 0 siblings, 1 reply; 12+ messages in thread From: Jesse Barnes @ 2014-06-06 15:03 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On Fri, 6 Jun 2014 11:29:24 +0300 Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Thu, Jun 05, 2014 at 01:49:34PM -0700, Jesse Barnes wrote: > > This may take awhile (~10ms), and we don't need to make noise about it. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=75244 > > Tested-by: huax.lu@intel.com > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 4 ---- > > 1 file changed, 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index ee27d74..4eebfd8 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) > > vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, > > dev_priv->rps.min_freq_softlimit); > > > > - if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) > > - & GENFREQSTATUS) == 0, 5)) > > - DRM_ERROR("timed out waiting for Punit\n"); > > - > > As I recall the entire point of this was to make sure the frequency > really drops before we allow turning off the clock. I'm not sure if we > can trust that the frequency (and more importantly the voltage) will > drop if we allow turning off the clock before the transition is > complete. well, we may need to increase the timeout then... let's see if that helps with the bug. -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-06 15:03 ` Jesse Barnes @ 2014-06-11 16:56 ` Ville Syrjälä 2014-06-12 2:44 ` S, Deepak 0 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2014-06-11 16:56 UTC (permalink / raw) To: Jesse Barnes; +Cc: S, Deepak, intel-gfx On Fri, Jun 06, 2014 at 08:03:20AM -0700, Jesse Barnes wrote: > On Fri, 6 Jun 2014 11:29:24 +0300 > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > On Thu, Jun 05, 2014 at 01:49:34PM -0700, Jesse Barnes wrote: > > > This may take awhile (~10ms), and we don't need to make noise about it. > > > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=75244 > > > Tested-by: huax.lu@intel.com > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > --- > > > drivers/gpu/drm/i915/intel_pm.c | 4 ---- > > > 1 file changed, 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > > index ee27d74..4eebfd8 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) > > > vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, > > > dev_priv->rps.min_freq_softlimit); > > > > > > - if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) > > > - & GENFREQSTATUS) == 0, 5)) > > > - DRM_ERROR("timed out waiting for Punit\n"); > > > - > > > > As I recall the entire point of this was to make sure the frequency > > really drops before we allow turning off the clock. I'm not sure if we > > can trust that the frequency (and more importantly the voltage) will > > drop if we allow turning off the clock before the transition is > > complete. > > well, we may need to increase the timeout then... let's see if that > helps with the bug. FYI I played around with my BYT a bit and it doesn't appear to require this "force gfx clock on" part of the workaround. I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make sure both render and media wells and the gfx clock remain off, and I was also monitoring vnn via svid. While that was going on I just rewrote PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure enough it did, and svid showed me that vnn had also changed. So it appears there's no need to have the gfx clock on to change its frequency on this BYT. I wonder if this part of the workaround was only needed on older parts. Deepak, any ideas? -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-11 16:56 ` Ville Syrjälä @ 2014-06-12 2:44 ` S, Deepak 2014-06-12 7:37 ` Lee, Chon Ming 2014-06-12 18:15 ` S, Deepak 0 siblings, 2 replies; 12+ messages in thread From: S, Deepak @ 2014-06-12 2:44 UTC (permalink / raw) To: Ville Syrjälä, Jesse Barnes; +Cc: intel-gfx On 6/11/2014 10:26 PM, Ville Syrjälä wrote: > On Fri, Jun 06, 2014 at 08:03:20AM -0700, Jesse Barnes wrote: >> On Fri, 6 Jun 2014 11:29:24 +0300 >> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> >>> On Thu, Jun 05, 2014 at 01:49:34PM -0700, Jesse Barnes wrote: >>>> This may take awhile (~10ms), and we don't need to make noise about it. >>>> >>>> References: https://bugs.freedesktop.org/show_bug.cgi?id=75244 >>>> Tested-by: huax.lu@intel.com >>>> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> >>>> --- >>>> drivers/gpu/drm/i915/intel_pm.c | 4 ---- >>>> 1 file changed, 4 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >>>> index ee27d74..4eebfd8 100644 >>>> --- a/drivers/gpu/drm/i915/intel_pm.c >>>> +++ b/drivers/gpu/drm/i915/intel_pm.c >>>> @@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) >>>> vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, >>>> dev_priv->rps.min_freq_softlimit); >>>> >>>> - if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) >>>> - & GENFREQSTATUS) == 0, 5)) >>>> - DRM_ERROR("timed out waiting for Punit\n"); >>>> - >>> >>> As I recall the entire point of this was to make sure the frequency >>> really drops before we allow turning off the clock. I'm not sure if we >>> can trust that the frequency (and more importantly the voltage) will >>> drop if we allow turning off the clock before the transition is >>> complete. >> >> well, we may need to increase the timeout then... let's see if that >> helps with the bug. > > FYI I played around with my BYT a bit and it doesn't appear to require > this "force gfx clock on" part of the workaround. > > I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make > sure both render and media wells and the gfx clock remain off, and I > was also monitoring vnn via svid. While that was going on I just rewrote > PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure > enough it did, and svid showed me that vnn had also changed. So it > appears there's no need to have the gfx clock on to change its frequency > on this BYT. > > I wonder if this part of the workaround was only needed on older parts. > Deepak, any ideas? Yes ville, this was added initial for older parts and force gfx clock was part of the workaround. We have not verified this on newer parts. Let me check with hw guys to see if workaround still exits and when this was fixed. Thanks Deepak ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-12 2:44 ` S, Deepak @ 2014-06-12 7:37 ` Lee, Chon Ming 2014-06-12 8:08 ` Daniel Vetter 2014-06-12 8:11 ` Ville Syrjälä 2014-06-12 18:15 ` S, Deepak 1 sibling, 2 replies; 12+ messages in thread From: Lee, Chon Ming @ 2014-06-12 7:37 UTC (permalink / raw) To: S, Deepak; +Cc: intel-gfx On 06/12 08:14, S, Deepak wrote: > > > On 6/11/2014 10:26 PM, Ville Syrjälä wrote: > >On Fri, Jun 06, 2014 at 08:03:20AM -0700, Jesse Barnes wrote: > >>On Fri, 6 Jun 2014 11:29:24 +0300 > >>Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > >> > >>>On Thu, Jun 05, 2014 at 01:49:34PM -0700, Jesse Barnes wrote: > >>>>This may take awhile (~10ms), and we don't need to make noise about it. > >>>> > >>>>References: https://bugs.freedesktop.org/show_bug.cgi?id=75244 > >>>>Tested-by: huax.lu@intel.com > >>>>Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > >>>>--- > >>>> drivers/gpu/drm/i915/intel_pm.c | 4 ---- > >>>> 1 file changed, 4 deletions(-) > >>>> > >>>>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > >>>>index ee27d74..4eebfd8 100644 > >>>>--- a/drivers/gpu/drm/i915/intel_pm.c > >>>>+++ b/drivers/gpu/drm/i915/intel_pm.c > >>>>@@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) > >>>> vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, > >>>> dev_priv->rps.min_freq_softlimit); > >>>> > >>>>- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) > >>>>- & GENFREQSTATUS) == 0, 5)) > >>>>- DRM_ERROR("timed out waiting for Punit\n"); > >>>>- > >>> > >>>As I recall the entire point of this was to make sure the frequency > >>>really drops before we allow turning off the clock. I'm not sure if we > >>>can trust that the frequency (and more importantly the voltage) will > >>>drop if we allow turning off the clock before the transition is > >>>complete. > >> > >>well, we may need to increase the timeout then... let's see if that > >>helps with the bug. > > > >FYI I played around with my BYT a bit and it doesn't appear to require > >this "force gfx clock on" part of the workaround. > > > >I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make > >sure both render and media wells and the gfx clock remain off, and I > >was also monitoring vnn via svid. While that was going on I just rewrote > >PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure > >enough it did, and svid showed me that vnn had also changed. So it > >appears there's no need to have the gfx clock on to change its frequency > >on this BYT. > > > >I wonder if this part of the workaround was only needed on older parts. > >Deepak, any ideas? > > Yes ville, this was added initial for older parts and force gfx > clock was part of the workaround. > We have not verified this on newer parts. Let me check with hw guys > to see if workaround still exits and when this was fixed. > > Thanks > Deepak This is a bit side topic. In the vlv_set_rps_idle, it try to mask the turbo interrupt by writing to GEN6_PMINTRMSK (offset 0xA168). But based on the bspec, the PMIMR for BYT/GEN6 should be 0x44024 (GEN6_PMIMR). Not sure I miss anything. If there offset is wrong, mean the turbo has not mask at all. Regards, Chon Ming > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-12 7:37 ` Lee, Chon Ming @ 2014-06-12 8:08 ` Daniel Vetter 2014-06-12 8:11 ` Ville Syrjälä 1 sibling, 0 replies; 12+ messages in thread From: Daniel Vetter @ 2014-06-12 8:08 UTC (permalink / raw) To: Lee, Chon Ming; +Cc: S, Deepak, intel-gfx On Thu, Jun 12, 2014 at 03:37:18PM +0800, Lee, Chon Ming wrote: > On 06/12 08:14, S, Deepak wrote: > > > > > > On 6/11/2014 10:26 PM, Ville Syrjälä wrote: > > >On Fri, Jun 06, 2014 at 08:03:20AM -0700, Jesse Barnes wrote: > > >>On Fri, 6 Jun 2014 11:29:24 +0300 > > >>Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > >> > > >>>On Thu, Jun 05, 2014 at 01:49:34PM -0700, Jesse Barnes wrote: > > >>>>This may take awhile (~10ms), and we don't need to make noise about it. > > >>>> > > >>>>References: https://bugs.freedesktop.org/show_bug.cgi?id=75244 > > >>>>Tested-by: huax.lu@intel.com > > >>>>Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > >>>>--- > > >>>> drivers/gpu/drm/i915/intel_pm.c | 4 ---- > > >>>> 1 file changed, 4 deletions(-) > > >>>> > > >>>>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > >>>>index ee27d74..4eebfd8 100644 > > >>>>--- a/drivers/gpu/drm/i915/intel_pm.c > > >>>>+++ b/drivers/gpu/drm/i915/intel_pm.c > > >>>>@@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) > > >>>> vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, > > >>>> dev_priv->rps.min_freq_softlimit); > > >>>> > > >>>>- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) > > >>>>- & GENFREQSTATUS) == 0, 5)) > > >>>>- DRM_ERROR("timed out waiting for Punit\n"); > > >>>>- > > >>> > > >>>As I recall the entire point of this was to make sure the frequency > > >>>really drops before we allow turning off the clock. I'm not sure if we > > >>>can trust that the frequency (and more importantly the voltage) will > > >>>drop if we allow turning off the clock before the transition is > > >>>complete. > > >> > > >>well, we may need to increase the timeout then... let's see if that > > >>helps with the bug. > > > > > >FYI I played around with my BYT a bit and it doesn't appear to require > > >this "force gfx clock on" part of the workaround. > > > > > >I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make > > >sure both render and media wells and the gfx clock remain off, and I > > >was also monitoring vnn via svid. While that was going on I just rewrote > > >PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure > > >enough it did, and svid showed me that vnn had also changed. So it > > >appears there's no need to have the gfx clock on to change its frequency > > >on this BYT. > > > > > >I wonder if this part of the workaround was only needed on older parts. > > >Deepak, any ideas? > > > > Yes ville, this was added initial for older parts and force gfx > > clock was part of the workaround. > > We have not verified this on newer parts. Let me check with hw guys > > to see if workaround still exits and when this was fixed. > > > > Thanks > > Deepak > > This is a bit side topic. In the vlv_set_rps_idle, it try to mask the turbo > interrupt by writing to GEN6_PMINTRMSK (offset 0xA168). But based on the bspec, > the PMIMR for BYT/GEN6 should be 0x44024 (GEN6_PMIMR). > > Not sure I miss anything. If there offset is wrong, mean the turbo has not mask at > all. Afaik PM interrupt handling has 3 levels: GEN6_PMINTRMSK is the lowest and only provides a mask to prevent irq event forwarding, GEN6_PMI[IEMS]R are the normal set of 4 registers and then there's the PM interrupt bit in the DE interrupt registers. Iirc masking in PMINTRMSK has power benefits, but I'm not sure. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-12 7:37 ` Lee, Chon Ming 2014-06-12 8:08 ` Daniel Vetter @ 2014-06-12 8:11 ` Ville Syrjälä 2014-06-12 8:19 ` Lee, Chon Ming 1 sibling, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2014-06-12 8:11 UTC (permalink / raw) To: Lee, Chon Ming; +Cc: S, Deepak, intel-gfx On Thu, Jun 12, 2014 at 03:37:18PM +0800, Lee, Chon Ming wrote: > On 06/12 08:14, S, Deepak wrote: > > > > > > On 6/11/2014 10:26 PM, Ville Syrjälä wrote: > > >On Fri, Jun 06, 2014 at 08:03:20AM -0700, Jesse Barnes wrote: > > >>On Fri, 6 Jun 2014 11:29:24 +0300 > > >>Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > >> > > >>>On Thu, Jun 05, 2014 at 01:49:34PM -0700, Jesse Barnes wrote: > > >>>>This may take awhile (~10ms), and we don't need to make noise about it. > > >>>> > > >>>>References: https://bugs.freedesktop.org/show_bug.cgi?id=75244 > > >>>>Tested-by: huax.lu@intel.com > > >>>>Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > >>>>--- > > >>>> drivers/gpu/drm/i915/intel_pm.c | 4 ---- > > >>>> 1 file changed, 4 deletions(-) > > >>>> > > >>>>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > >>>>index ee27d74..4eebfd8 100644 > > >>>>--- a/drivers/gpu/drm/i915/intel_pm.c > > >>>>+++ b/drivers/gpu/drm/i915/intel_pm.c > > >>>>@@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) > > >>>> vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, > > >>>> dev_priv->rps.min_freq_softlimit); > > >>>> > > >>>>- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) > > >>>>- & GENFREQSTATUS) == 0, 5)) > > >>>>- DRM_ERROR("timed out waiting for Punit\n"); > > >>>>- > > >>> > > >>>As I recall the entire point of this was to make sure the frequency > > >>>really drops before we allow turning off the clock. I'm not sure if we > > >>>can trust that the frequency (and more importantly the voltage) will > > >>>drop if we allow turning off the clock before the transition is > > >>>complete. > > >> > > >>well, we may need to increase the timeout then... let's see if that > > >>helps with the bug. > > > > > >FYI I played around with my BYT a bit and it doesn't appear to require > > >this "force gfx clock on" part of the workaround. > > > > > >I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make > > >sure both render and media wells and the gfx clock remain off, and I > > >was also monitoring vnn via svid. While that was going on I just rewrote > > >PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure > > >enough it did, and svid showed me that vnn had also changed. So it > > >appears there's no need to have the gfx clock on to change its frequency > > >on this BYT. > > > > > >I wonder if this part of the workaround was only needed on older parts. > > >Deepak, any ideas? > > > > Yes ville, this was added initial for older parts and force gfx > > clock was part of the workaround. > > We have not verified this on newer parts. Let me check with hw guys > > to see if workaround still exits and when this was fixed. > > > > Thanks > > Deepak > > This is a bit side topic. In the vlv_set_rps_idle, it try to mask the turbo > interrupt by writing to GEN6_PMINTRMSK (offset 0xA168). But based on the bspec, > the PMIMR for BYT/GEN6 should be 0x44024 (GEN6_PMIMR). > > Not sure I miss anything. If there offset is wrong, mean the turbo has not mask at > all. There are two mask registers for this stuff. PMINTRMSK is some kind of lower level mask, and GEN6_PMIMR is the regular interrupt mask. I believe masking the interrupt in PMINTRMSK prevents it from being visible in GEN6_PMISR even. -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-12 8:11 ` Ville Syrjälä @ 2014-06-12 8:19 ` Lee, Chon Ming 0 siblings, 0 replies; 12+ messages in thread From: Lee, Chon Ming @ 2014-06-12 8:19 UTC (permalink / raw) To: Ville Syrjälä; +Cc: S, Deepak, intel-gfx On 06/12 11:11, Ville Syrjälä wrote: > On Thu, Jun 12, 2014 at 03:37:18PM +0800, Lee, Chon Ming wrote: > > On 06/12 08:14, S, Deepak wrote: > > > > > > > > > On 6/11/2014 10:26 PM, Ville Syrjälä wrote: > > > >On Fri, Jun 06, 2014 at 08:03:20AM -0700, Jesse Barnes wrote: > > > >>On Fri, 6 Jun 2014 11:29:24 +0300 > > > >>Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > >> > > > >>>On Thu, Jun 05, 2014 at 01:49:34PM -0700, Jesse Barnes wrote: > > > >>>>This may take awhile (~10ms), and we don't need to make noise about it. > > > >>>> > > > >>>>References: https://bugs.freedesktop.org/show_bug.cgi?id=75244 > > > >>>>Tested-by: huax.lu@intel.com > > > >>>>Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > >>>>--- > > > >>>> drivers/gpu/drm/i915/intel_pm.c | 4 ---- > > > >>>> 1 file changed, 4 deletions(-) > > > >>>> > > > >>>>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > > >>>>index ee27d74..4eebfd8 100644 > > > >>>>--- a/drivers/gpu/drm/i915/intel_pm.c > > > >>>>+++ b/drivers/gpu/drm/i915/intel_pm.c > > > >>>>@@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) > > > >>>> vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, > > > >>>> dev_priv->rps.min_freq_softlimit); > > > >>>> > > > >>>>- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) > > > >>>>- & GENFREQSTATUS) == 0, 5)) > > > >>>>- DRM_ERROR("timed out waiting for Punit\n"); > > > >>>>- > > > >>> > > > >>>As I recall the entire point of this was to make sure the frequency > > > >>>really drops before we allow turning off the clock. I'm not sure if we > > > >>>can trust that the frequency (and more importantly the voltage) will > > > >>>drop if we allow turning off the clock before the transition is > > > >>>complete. > > > >> > > > >>well, we may need to increase the timeout then... let's see if that > > > >>helps with the bug. > > > > > > > >FYI I played around with my BYT a bit and it doesn't appear to require > > > >this "force gfx clock on" part of the workaround. > > > > > > > >I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make > > > >sure both render and media wells and the gfx clock remain off, and I > > > >was also monitoring vnn via svid. While that was going on I just rewrote > > > >PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure > > > >enough it did, and svid showed me that vnn had also changed. So it > > > >appears there's no need to have the gfx clock on to change its frequency > > > >on this BYT. > > > > > > > >I wonder if this part of the workaround was only needed on older parts. > > > >Deepak, any ideas? > > > > > > Yes ville, this was added initial for older parts and force gfx > > > clock was part of the workaround. > > > We have not verified this on newer parts. Let me check with hw guys > > > to see if workaround still exits and when this was fixed. > > > > > > Thanks > > > Deepak > > > > This is a bit side topic. In the vlv_set_rps_idle, it try to mask the turbo > > interrupt by writing to GEN6_PMINTRMSK (offset 0xA168). But based on the bspec, > > the PMIMR for BYT/GEN6 should be 0x44024 (GEN6_PMIMR). > > > > Not sure I miss anything. If there offset is wrong, mean the turbo has not mask at > > all. > > There are two mask registers for this stuff. PMINTRMSK is some kind > of lower level mask, and GEN6_PMIMR is the regular interrupt mask. > I believe masking the interrupt in PMINTRMSK prevents it from being > visible in GEN6_PMISR even. > Thanks for the info. Got that, we can't use GEN6PMIMR to mask interrupt, need to use PMINTRMSK. Regards, Chon Ming > -- > Ville Syrjälä > Intel OTC ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-12 2:44 ` S, Deepak 2014-06-12 7:37 ` Lee, Chon Ming @ 2014-06-12 18:15 ` S, Deepak 2014-06-12 18:40 ` Ville Syrjälä 1 sibling, 1 reply; 12+ messages in thread From: S, Deepak @ 2014-06-12 18:15 UTC (permalink / raw) To: Ville Syrjälä, Jesse Barnes; +Cc: intel-gfx >> I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make >> sure both render and media wells and the gfx clock remain off, and I >> was also monitoring vnn via svid. While that was going on I just rewrote >> PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure >> enough it did, and svid showed me that vnn had also changed. So it >> appears there's no need to have the gfx clock on to change its frequency >> on this BYT. >> >> I wonder if this part of the workaround was only needed on older parts. >> Deepak, any ideas? > > Yes ville, this was added initial for older parts and force gfx clock > was part of the workaround. > We have not verified this on newer parts. Let me check with hw guys to > see if workaround still exits and when this was fixed. Hi Ville, Got the confirmation from HW team, this WA as been fixed in latest stepping, we can remove the force gfx clock, mask and request only the min freq when we are idle. You will submit patch will fix or you want me to do it? Thanks Deepak ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-12 18:15 ` S, Deepak @ 2014-06-12 18:40 ` Ville Syrjälä 2014-06-12 18:41 ` S, Deepak 0 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2014-06-12 18:40 UTC (permalink / raw) To: S, Deepak; +Cc: intel-gfx On Thu, Jun 12, 2014 at 11:45:03PM +0530, S, Deepak wrote: > >> I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make > >> sure both render and media wells and the gfx clock remain off, and I > >> was also monitoring vnn via svid. While that was going on I just rewrote > >> PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure > >> enough it did, and svid showed me that vnn had also changed. So it > >> appears there's no need to have the gfx clock on to change its frequency > >> on this BYT. > >> > >> I wonder if this part of the workaround was only needed on older parts. > >> Deepak, any ideas? > > > > Yes ville, this was added initial for older parts and force gfx clock > > was part of the workaround. > > We have not verified this on newer parts. Let me check with hw guys to > > see if workaround still exits and when this was fixed. > > Hi Ville, Got the confirmation from HW team, this WA as been fixed in > latest stepping, What's latest here? Did we ever ship any of the steppings that still need the gfx clock force? > we can remove the force gfx clock, mask and request > only the min freq when we are idle. > > You will submit patch will fix or you want me to do it? Go ahead if you have time. I'm already juggling too many things :) -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle 2014-06-12 18:40 ` Ville Syrjälä @ 2014-06-12 18:41 ` S, Deepak 0 siblings, 0 replies; 12+ messages in thread From: S, Deepak @ 2014-06-12 18:41 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On 6/13/2014 12:10 AM, Ville Syrjälä wrote: > On Thu, Jun 12, 2014 at 11:45:03PM +0530, S, Deepak wrote: >> >> I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make >> >> sure both render and media wells and the gfx clock remain off, and I >> >> was also monitoring vnn via svid. While that was going on I just rewrote >> >> PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure >> >> enough it did, and svid showed me that vnn had also changed. So it >> >> appears there's no need to have the gfx clock on to change its frequency >> >> on this BYT. >> >> >> >> I wonder if this part of the workaround was only needed on older parts. >> >> Deepak, any ideas? >> > >> > Yes ville, this was added initial for older parts and force gfx clock >> > was part of the workaround. >> > We have not verified this on newer parts. Let me check with hw guys to >> > see if workaround still exits and when this was fixed. >> >> Hi Ville, Got the confirmation from HW team, this WA as been fixed in >> latest stepping, > > What's latest here? Did we ever ship any of the steppings that still > need the gfx clock force? > >> we can remove the force gfx clock, mask and request >> only the min freq when we are idle. >> >> You will submit patch will fix or you want me to do it? > > Go ahead if you have time. I'm already juggling too many things :) Ok. I will send the patch for review :) ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-06-12 18:41 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-05 20:49 [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle Jesse Barnes 2014-06-06 8:29 ` Ville Syrjälä 2014-06-06 15:03 ` Jesse Barnes 2014-06-11 16:56 ` Ville Syrjälä 2014-06-12 2:44 ` S, Deepak 2014-06-12 7:37 ` Lee, Chon Ming 2014-06-12 8:08 ` Daniel Vetter 2014-06-12 8:11 ` Ville Syrjälä 2014-06-12 8:19 ` Lee, Chon Ming 2014-06-12 18:15 ` S, Deepak 2014-06-12 18:40 ` Ville Syrjälä 2014-06-12 18:41 ` S, Deepak
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.