On Wed, Mar 16, 2016 at 08:07:03PM +0200, Imre Deak wrote:
> On Fri, 2016-03-04 at 21:43 +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> >
> > cur_freq, min/max_freq_softlimit, efficient_freq are just single
> > values
> > stored under dev_priv.rps, so there's no real point in locking,
> > resuming
> > the devices and flushing the delayed resume work just to print them
> > out.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> This makes things clearer, so would it make sense to still keep the
> flush wq and leave out patch 4 until somebody benchmarks things? That
> would address Tom's concern as well.
Yeah, at least that would get rid of the mutex + rpm. And I suppose I
should look at the debugfs side too.
>
> > ---
> > drivers/gpu/drm/i915/i915_sysfs.c | 84 ++++++-----------------------
> > ----------
> > 1 file changed, 13 insertions(+), 71 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
> > b/drivers/gpu/drm/i915/i915_sysfs.c
> > index 2d576b7ff299..9e39d7a18fdb 100644
> > --- a/drivers/gpu/drm/i915/i915_sysfs.c
> > +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> > @@ -305,55 +305,6 @@ static ssize_t gt_act_freq_mhz_show(struct
> > device *kdev,
> > return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> > }
> >
> > -static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
> > - struct device_attribute *attr,
> > char *buf)
> > -{
> > - struct drm_minor *minor = dev_to_drm_minor(kdev);
> > - struct drm_device *dev = minor->dev;
> > - struct drm_i915_private *dev_priv = dev->dev_private;
> > - int ret;
> > -
> > - flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> > -
> > - intel_runtime_pm_get(dev_priv);
> > -
> > - mutex_lock(&dev_priv->rps.hw_lock);
> > - ret = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
> > - mutex_unlock(&dev_priv->rps.hw_lock);
> > -
> > - intel_runtime_pm_put(dev_priv);
> > -
> > - return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> > -}
> > -
> > -static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev,
> > - struct device_attribute *attr,
> > char *buf)
> > -{
> > - struct drm_minor *minor = dev_to_drm_minor(kdev);
> > - struct drm_device *dev = minor->dev;
> > - struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> > - return snprintf(buf, PAGE_SIZE,
> > - "%d\n",
> > - intel_gpu_freq(dev_priv, dev_priv-
> > >rps.efficient_freq));
> > -}
> > -
> > -static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct
> > device_attribute *attr, char *buf)
> > -{
> > - struct drm_minor *minor = dev_to_drm_minor(kdev);
> > - struct drm_device *dev = minor->dev;
> > - struct drm_i915_private *dev_priv = dev->dev_private;
> > - int ret;
> > -
> > - flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> > -
> > - mutex_lock(&dev_priv->rps.hw_lock);
> > - ret = intel_gpu_freq(dev_priv, dev_priv-
> > >rps.max_freq_softlimit);
> > - mutex_unlock(&dev_priv->rps.hw_lock);
> > -
> > - return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> > -}
> > -
> > static ssize_t gt_max_freq_mhz_store(struct device *kdev,
> > struct device_attribute *attr,
> > const char *buf, size_t count)
> > @@ -406,22 +357,6 @@ static ssize_t gt_max_freq_mhz_store(struct
> > device *kdev,
> > return count;
> > }
> >
> > -static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct
> > device_attribute *attr, char *buf)
> > -{
> > - struct drm_minor *minor = dev_to_drm_minor(kdev);
> > - struct drm_device *dev = minor->dev;
> > - struct drm_i915_private *dev_priv = dev->dev_private;
> > - int ret;
> > -
> > - flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> > -
> > - mutex_lock(&dev_priv->rps.hw_lock);
> > - ret = intel_gpu_freq(dev_priv, dev_priv-
> > >rps.min_freq_softlimit);
> > - mutex_unlock(&dev_priv->rps.hw_lock);
> > -
> > - return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> > -}
> > -
> > static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> > struct device_attribute *attr,
> > const char *buf, size_t count)
> > @@ -472,16 +407,15 @@ static ssize_t gt_min_freq_mhz_store(struct
> > device *kdev,
> > }
> >
> > static DEVICE_ATTR(gt_act_freq_mhz, S_IRUGO, gt_act_freq_mhz_show,
> > NULL);
> > -static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show,
> > NULL);
> > -static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR,
> > gt_max_freq_mhz_show, gt_max_freq_mhz_store);
> > -static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR,
> > gt_min_freq_mhz_show, gt_min_freq_mhz_store);
> > -
> > -static DEVICE_ATTR(vlv_rpe_freq_mhz, S_IRUGO, vlv_rpe_freq_mhz_show,
> > NULL);
> >
> > static ssize_t gt_rp_mhz_show(struct device *kdev, struct
> > device_attribute *attr, char *buf);
> > +static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> > +static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR,
> > gt_rp_mhz_show, gt_max_freq_mhz_store);
> > +static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR,
> > gt_rp_mhz_show, gt_min_freq_mhz_store);
> > static DEVICE_ATTR(gt_RP0_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> > static DEVICE_ATTR(gt_RP1_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> > static DEVICE_ATTR(gt_RPn_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> > +static DEVICE_ATTR(vlv_rpe_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL);
> >
> > /* For now we have a static number of RP states */
> > static ssize_t gt_rp_mhz_show(struct device *kdev, struct
> > device_attribute *attr, char *buf)
> > @@ -491,12 +425,20 @@ static ssize_t gt_rp_mhz_show(struct device
> > *kdev, struct device_attribute *attr
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > u32 val;
> >
> > - if (attr == &dev_attr_gt_RP0_freq_mhz)
> > + if (attr == &dev_attr_gt_cur_freq_mhz)
> > + val = intel_gpu_freq(dev_priv, dev_priv-
> > >rps.cur_freq);
> > + else if (attr == &dev_attr_gt_max_freq_mhz)
> > + val = intel_gpu_freq(dev_priv, dev_priv-
> > >rps.max_freq_softlimit);
> > + else if (attr == &dev_attr_gt_min_freq_mhz)
> > + val = intel_gpu_freq(dev_priv, dev_priv-
> > >rps.min_freq_softlimit);
> > + else if (attr == &dev_attr_gt_RP0_freq_mhz)
> > val = intel_gpu_freq(dev_priv, dev_priv-
> > >rps.rp0_freq);
> > else if (attr == &dev_attr_gt_RP1_freq_mhz)
> > val = intel_gpu_freq(dev_priv, dev_priv-
> > >rps.rp1_freq);
> > else if (attr == &dev_attr_gt_RPn_freq_mhz)
> > val = intel_gpu_freq(dev_priv, dev_priv-
> > >rps.min_freq);
> > + else if (attr == &dev_attr_vlv_rpe_freq_mhz)
> > + val = intel_gpu_freq(dev_priv, dev_priv-
> > >rps.efficient_freq);
> > else
> > BUG();
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
