| Message ID | 20180308093058.17496-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Chris Wilson <chris@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@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@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. If it can, why not just kick the rps work unconditionally? Further, on all other freq store entries in sysfs update val and schedule unconditionally? Then the handler would be the single entry to re-evaluation and setting the hardware. -Mika > return count; > } > -- > 2.16.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2018-03-08 09:53:01) > Chris Wilson <chris@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@chris-wilson.co.uk> > > Cc: Mika Kuoppala <mika.kuoppala@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
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); return count; }
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@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_sysfs.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)