| Message ID | 1402001374-2657-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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?
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
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
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
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.
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
>> 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
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 :)
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 :)
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,