[Intel-gfx] [PATCH] drm/i915: Kick the rps worker when changing the boost frequency

[Chris Wilson]

Quoting Mika Kuoppala (2018-03-08 09:53:01)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > The boost frequency is only applied from the RPS worker while someone is
> > waiting on a request and requested a boost. As such, when the user
> > wishes to change the frequency, we have to kick the worker in order to
> > re-evaluate whether to apply the boost frequency.
> >
> > Fixes: 29ecd78d3b79 ("drm/i915: Define a separate variable and control for RPS waitboost frequency")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_sysfs.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> > index b33d2158c234..d97463bb69ad 100644
> > --- a/drivers/gpu/drm/i915/i915_sysfs.c
> > +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> > @@ -304,8 +304,9 @@ static ssize_t gt_boost_freq_mhz_store(struct device *kdev,
> >  {
> >       struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
> >       struct intel_rps *rps = &dev_priv->gt_pm.rps;
> > -     u32 val;
> > +     bool boost = false;
> >       ssize_t ret;
> > +     u32 val;
> >  
> >       ret = kstrtou32(buf, 0, &val);
> >       if (ret)
> > @@ -317,8 +318,13 @@ static ssize_t gt_boost_freq_mhz_store(struct device *kdev,
> >               return -EINVAL;
> >  
> >       mutex_lock(&dev_priv->pcu_lock);
> > -     rps->boost_freq = val;
> > +     if (val != rps->boost_freq) {
> > +             rps->boost_freq = val;
> > +             boost = true;
> > +     }
> >       mutex_unlock(&dev_priv->pcu_lock);
> > +     if (boost)
> > +             schedule_work(&rps->work);
> >
> 
> Can the rps work handler handle a situation
> where the powers are off? It bails out if we dont
> have iir nor boost, which might be enough of a
> safeguard.

Yes, we check whether the interrupts are enabled and do nothing if they
are not. We disable the interrupts when idling.
 
> If it can, why not just kick the rps work
> unconditionally?

That's the plan. I'm pushing this because I've a small series waiting
for the conflicts to resolve...
 
> Further, on all other freq store entries in sysfs
> update val and schedule unconditionally?

Coming soon :)
 
> Then the handler would be the single entry to
> re-evaluation and setting the hardware.

\o/
-Chris