Quoting Sagar Arun Kamble (2018-03-17 15:10:08)
>
>
> On 3/14/2018 3:07 PM, Chris Wilson wrote:
> > When choosing the initial frequency in intel_gt_pm_busy() we also need
> > to calculate the current min/max bounds. As this calculation is going to
> > become more complex with the intersection of several different limits,
> > refactor it to a common function. The alternative wold be to feed the
> typo
> > initial reclocking through the RPS worker, but the latency in this case
> > is undesirable.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_gt_pm.c | 58 +++++++++++++++-----------------------
> > 1 file changed, 22 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_gt_pm.c b/drivers/gpu/drm/i915/intel_gt_pm.c
> > index 8630c30a7e48..f8e029b4a8a7 100644
> > --- a/drivers/gpu/drm/i915/intel_gt_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_gt_pm.c
> > @@ -382,15 +382,25 @@ static int __intel_set_rps(struct drm_i915_private *dev_priv, u8 val)
> > return 0;
> > }
> >
> > -static int intel_set_rps(struct drm_i915_private *dev_priv, u8 val)
> > +static int adjust_rps(struct drm_i915_private *dev_priv, int freq, int adj)
> > {
> > struct intel_rps *rps = &dev_priv->gt_pm.rps;
> > + int min, max, val;
> Can we move to u8 type in this patch itself
No. Check the math, that presumes int clamping, so just use native
register types until after the clamping.
> > int err;
> >
> > lockdep_assert_held(&rps->lock);
> > GEM_BUG_ON(!rps->active);
> > - GEM_BUG_ON(val > rps->max_freq);
> > - GEM_BUG_ON(val < rps->min_freq);
> > +
> > + min = rps->min_freq_softlimit;
> > + max = rps->max_freq_softlimit;
> > + if (atomic_read(&rps->num_waiters) && max < rps->boost_freq)
> > + max = rps->boost_freq;
> > +
> > + GEM_BUG_ON(min < rps->min_freq);
> > + GEM_BUG_ON(max > rps->max_freq);
> > + GEM_BUG_ON(max < min);
> > +
> > + val = clamp(freq + adj, min, max);
> >
> > err = __intel_set_rps(dev_priv, val);
> > if (err)
> > @@ -401,6 +411,8 @@ static int intel_set_rps(struct drm_i915_private *dev_priv, u8 val)
> > rps->cur_freq = val;
> > }
> >
> > + rps->last_adj = val == freq ? adj : 0;
> > +
> I think this should be:
> rps->last_adj = val == freq ? 0 : adj;
If we make the adjustment, store the new adj; if we overrule the
selection, then cancel the adj so that we don't keep on accumulating adj
upon hitting the bounds.
Hmm, should have been val == freq + adj.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
