On Thu, Apr 30, 2015 at 02:19:07PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 30, 2015 at 03:42:42PM +0530, Deepak S wrote:
> > As you suggested it would be better to extend the VLV WA to
> > CHV also to make sure we drop the voltage when idle.
> >
> > Below is the sequence I think we should follow (based on your comments).
> > 1. forcewake power wells
> > 2. do gfx force clock on
> > 3. request freq to punit
> > 4. release gfx force clock on
> > 5. release forcewake of power wells.
> >
> > Please share your thoughts?
>
> I'm thinking we shouldn't need the gfx clock force since forcewake
> should already cause the clock to be enabled.
>
> I've also not verified what happens if we drop the forcewake before
> Punit has actually finished the frequency change. I'll try to hack
> up some kind of test to see if I can make that happen.
OK, after some hacking I see that the forcewake is enough, and also
we don't seem to need to wait for the Punit to finish the frequency
change before dropping forcewake. It seems to finish the change even
after forcewake has been dropped.
Also for a bit of extra micro optimization we should perhaps wake up
the media well only, as that takes a lot less power than the render
well.
>
> >
> > Thanks
> > Deepak
> >
> > >> So based on my tests this patch feels a bit wrong.
> > >>
> > >>> Since Punit validates the rps range [RPe, RP0]. This patch
> > >>> removes unused cherryview_rps_min_freq function.
> > >>>
> > >>> v2: Change commit message
> > >>>
> > >>> v3: set min_freq before idle_freq (chris)
> > >>>
> > >>> v4: Squash 'Remove unused rps min function' patch
> > >>>
> > >>> Signed-off-by: Deepak S <deepak.s@xxxxxxxxxxxxxxx>
> > >>> Acked-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > >>> ---
> > >>> drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
> > >>> 1 file changed, 2 insertions(+), 19 deletions(-)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > >>> index a7516ed..78c89ff 100644
> > >>> --- a/drivers/gpu/drm/i915/intel_pm.c
> > >>> +++ b/drivers/gpu/drm/i915/intel_pm.c
> > >>> @@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
> > >>> return rp1;
> > >>> }
> > >>>
> > >>> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> > >>> -{
> > >>> - struct drm_device *dev = dev_priv->dev;
> > >>> - u32 val, rpn;
> > >>> -
> > >>> - if (dev->pdev->revision >= 0x20) {
> > >>> - val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
> > >>> - rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
> > >>> - FB_GFX_FREQ_FUSE_MASK);
> > >>> - } else { /* For pre-production hardware */
> > >>> - val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
> > >>> - rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
> > >>> - PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
> > >>> - }
> > >>> -
> > >>> - return rpn;
> > >>> -}
> > >>> -
> > >>> static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
> > >>> {
> > >>> u32 val, rp1;
> > >>> @@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
> > >>> intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
> > >>> dev_priv->rps.rp1_freq);
> > >>>
> > >>> - dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
> > >>> + /* PUnit validated range is only [RPe, RP0] */
> > >>> + dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
> > >>> DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
> > >>> intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
> > >>> dev_priv->rps.min_freq);
> > >>> --
> > >>> 1.9.1
> > >> --
> > >> Ville Syrjälä
> > >> Intel OTC
> > >> _______________________________________________
> > >> Intel-gfx mailing list
> > >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
