On Fri, 2015-02-06 at 15:48 +0000, Chris Wilson wrote:
> On Fri, Feb 06, 2015 at 08:26:34PM +0530, akash.goel@xxxxxxxxx wrote:
> > From: Akash Goel <akash.goel@xxxxxxxxx>
> >
> > Prior to SKL, the time period programmed in Up/Down EI & Up/Down
> > threshold registers was in units of 1.28 micro seconds. But for
> > SKL, the units have changed (1.333 micro seconds).
> > Have generalized the implementation of gen6_set_rps_thresholds function,
> > by removing the hard coding done in it as per 1.28 micro seconds.
> >
> > Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 70 ++++++++++++++++++++---------------------
> > 1 file changed, 34 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 58c8c0e..215b200 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3643,6 +3643,8 @@ static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 val)
> > static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
> > {
> > int new_power;
> > + u32 threshold_up_pct = 0, threshold_down_pct = 0;
>
> Drop the _pct, unrequired early initialisation, just comment that
> up/down are in %.
Fine, will remove the _pct suffix.
> > + u32 ei_up = 0, ei_down = 0;
> >
> > new_power = dev_priv->rps.power;
> > switch (dev_priv->rps.power) {
> > @@ -3675,59 +3677,55 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
> > switch (new_power) {
> > case LOW_POWER:
> > /* Upclock if more than 95% busy over 16ms */
> > - I915_WRITE(GEN6_RP_UP_EI, 12500);
> > - I915_WRITE(GEN6_RP_UP_THRESHOLD, 11800);
> > + ei_up = 16000;
> > + threshold_up_pct = 95; /* x% */
>
> Double comments that this is a %! Really doesn't seem to be required
> with the preceeding comment.
Fine, will remove the needless comment
> > + I915_WRITE(GEN6_RP_UP_EI,
> > + GT_FREQ_FROM_PERIOD(ei_up, dev_priv->dev));
>
> Just pass dev_priv. It's magic.
Will modify the macro to accept the dev_priv instead of dev.
>
> > + I915_WRITE(GEN6_RP_UP_THRESHOLD,
> > + GT_FREQ_FROM_PERIOD((ei_up * threshold_up_pct / 100),
>
> I wonder if it is worth using base 128 instead of 100%.
Sorry couldn't get this comment.
Actually the macro GT_FREQ_FROM_PERIOD has been used at other places,
where % conversion is not required. So not sure, how using a base of
128 would be better.
>
> Otherwise looks good and ties in with using it from vlv. Do you mind
> reviewing those patches?
Sure, will review that patch, please provide the patch link.
> They fix a bug in which the manual c0 counting
> keeps interrupts alive whilst idle.
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
