On Thu, Apr 30, 2015 at 03:34:32PM +0530, Deepak S wrote: > > > On Wednesday 29 April 2015 03:56 PM, Ville Syrjälä wrote: > >On Wed, Apr 29, 2015 at 08:20:20AM +0530, Deepak S wrote: > >> > >>On Wednesday 29 April 2015 12:02 AM, Ville Syrjälä wrote: > >>>On Tue, Apr 28, 2015 at 11:16:29AM -0700, Jesse Barnes wrote: > >>>>On 03/04/2015 08:08 PM, [email protected] wrote: > >>>>>From: Deepak S <[email protected]> > >>>>> > >>>>>When GPU is idle on VLV, Request freq to punit should be good enough to > >>>>>get the voltage back to VNN. Also, make sure gfx clock force applies > >>>>>before requesting the freq fot vlv. > >>>>> > >>>>>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244 > >>>>>suggested-by: Jesse Barnes <[email protected]> > >>>>>Signed-off-by: Deepak S <[email protected]> > >>>>>--- > >>>>> drivers/gpu/drm/i915/intel_pm.c | 20 ++++---------------- > >>>>> 1 file changed, 4 insertions(+), 16 deletions(-) > >>>>> > >>>>>diff --git a/drivers/gpu/drm/i915/intel_pm.c > >>>>>b/drivers/gpu/drm/i915/intel_pm.c > >>>>>index e710b43..2e1ed07 100644 > >>>>>--- a/drivers/gpu/drm/i915/intel_pm.c > >>>>>+++ b/drivers/gpu/drm/i915/intel_pm.c > >>>>>@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device > >>>>>*dev, u8 val) > >>>>> * * If Gfx is Idle, then > >>>>> * 1. Mask Turbo interrupts > >>>>> * 2. Bring up Gfx clock > >>>>>- * 3. Change the freq to Rpn and wait till P-Unit updates freq > >>>>>+ * 3. Request the freq to Rpn. > >>>>> * 4. Clear the Force GFX CLK ON bit so that Gfx can down > >>>>> * 5. Unmask Turbo interrupts > >>>>> */ > >>>>>@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct > >>>>>drm_i915_private *dev_priv) > >>>>> { > >>>>> struct drm_device *dev = dev_priv->dev; > >>>>>- /* CHV and latest VLV don't need to force the gfx clock */ > >>>>>- if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) { > >>>>>+ /* CHV don't need to force the gfx clock */ > >>>>>+ if (IS_CHERRYVIEW(dev)) { > >>>>> valleyview_set_rps(dev_priv->dev, > >>>>> dev_priv->rps.min_freq_softlimit); > >>>>> return; > >>>>> } > >>>>>@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct > >>>>>drm_i915_private *dev_priv) > >>>>> gen6_sanitize_rps_pm_mask(dev_priv, ~0)); > >>>>> vlv_force_gfx_clock(dev_priv, true); > >>>>>- > >>>>>- dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit; > >>>>>- > >>>>>- 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, 100)) > >>>>>- DRM_ERROR("timed out waiting for Punit\n"); > >>>>>- > >>>>>+ valleyview_set_rps(dev_priv->dev, > >>>>>dev_priv->rps.min_freq_softlimit); > >>>>> vlv_force_gfx_clock(dev_priv, false); > >>>>>- > >>>>>- I915_WRITE(GEN6_PMINTRMSK, > >>>>>- gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq)); > >>>>> } > >>>>> void gen6_rps_idle(struct drm_i915_private *dev_priv) > >>>>> > >>>>Yeah I think this is fine (may need a rebase though, you can keep my r-b > >>>>if you do that in case Jani doesn't want to deal with the merge > >>>>conflicts). > >>>> > >>>>Reviewed-by: Jesse Barnes <[email protected]> > >>>The removal of the stepping check is still confusing me even if the > >>>rest would be OK. > >>> > >>Stepping check was added latest BYT release. On older BYT stepping, We used > >>to wait for punit to grant the freq in GT Idle case, (most of the cases > >>punit is timing out :( ) > >>We now make the gfx clock force apply to all VLV and then request the freq > >>to RPn this should be good enough to get voltage to Vnn. > >But we shouldn't need the gfx clock force for the latest VLV > >stepping(s), and we certainly didn't do it before. So why do > >it now? > > > Hi Ville, This is keep code common across all the VLV stepping. :)
Makes sense (at least to me) but please add this explanation to the commit message when resending so it won't get lost. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
