On Wed, 2016-04-13 at 13:26 +0000, Zanoni, Paulo R wrote:
> Em Ter, 2016-04-12 às 12:18 -0700, Alexandra Yates escreveu:
> > This interface allows an immediate enabling of PSR feature. What
> > allow us
> > to see immediately the PSR savings and will allow us to expose this
> > through sysfs interface for powertop to leverage its functionality.
> >
> > Signed-off-by: Alexandra Yates <alexandra.yates@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/i915_sysfs.c | 79
> > +++++++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_ddi.c | 6 ++-
> > drivers/gpu/drm/i915/intel_dp.c | 11 ++++--
> > drivers/gpu/drm/i915/intel_drv.h | 4 +-
> > drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++----
> > 6 files changed, 122 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index f5c91b0..c96da4d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -976,6 +976,7 @@ struct i915_psr {
> > bool psr2_support;
> > bool aux_frame_sync;
> > bool link_standby;
> > + bool sysfs_set;
> > };
> >
> > enum intel_pch {
> > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
> > b/drivers/gpu/drm/i915/i915_sysfs.c
> > index 2d576b7..208c6ec 100644
> > --- a/drivers/gpu/drm/i915/i915_sysfs.c
> > +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> > @@ -106,12 +106,84 @@ show_media_rc6_ms(struct device *kdev, struct
> > device_attribute *attr, char *buf)
> > return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency);
> > }
> >
> > +static ssize_t
> > +show_psr(struct device *kdev, struct device_attribute *attr, char
> > *buf)
> > +{
> > + struct drm_minor *dminor = dev_to_drm_minor(kdev);
> > + struct drm_device *dev = dminor->dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + ssize_t ret;
> > +
> > + mutex_lock(&dev_priv->psr.lock);
> > + ret = snprintf(buf, PAGE_SIZE, "%s\n", dev_priv
> > ->psr.enabled
> > ?
> > + "enabled":"disabled");
> > + mutex_unlock(&dev_priv->psr.lock);
> > + return ret;
> > +}
> > +
> > +static ssize_t
> > +toggle_psr(struct device *kdev, struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct drm_minor *dminor = dev_to_drm_minor(kdev);
> > + struct drm_device *dev = dminor->dev;
> > + struct intel_connector *connector;
> > + struct intel_encoder *encoder;
> > + struct intel_crtc *crtc = NULL;
> > + u32 val;
> > + ssize_t ret;
> > + struct intel_dp *intel_dp = NULL;
> > + bool sysfs_set = true;
> > +
> > + ret = kstrtou32(buf, 0, &val);
> > + if (ret)
> > + return ret;
> > +
> > + for_each_intel_connector(dev, connector) {
> > + if (!connector->base.encoder)
> > + continue;
> > + encoder = to_intel_encoder(connector
> > ->base.encoder);
> > + crtc = to_intel_crtc(encoder->base.crtc);
> > + intel_dp = enc_to_intel_dp(&encoder->base);
> > + }
>
> Same problem here: no guarantee that the encoder is DP, nor that it
> supports PSR, nor that it is enabled and has a mode set.
I agree. Also we have an issue with new platforms if we end up having
any laptop with 2 eDPs supporting PSR things could get crazy.
So we could actually create a
struct intel_dp *edp_with_psr_support; inside psr structure that we set
in the moment we find the first eDP panel that supports PSR
and change the enabled from intel_dp pointer to a boolean.
>
> > +
> > + if (!crtc)
> > + return -ENODEV;
> > + switch (val) {
> > + case 0:
> > + ret = intel_psr_disable(intel_dp, sysfs_set);
> > + if (ret)
> > + return ret;
> > + break;
> > + case 1:
> > + ret = intel_psr_enable(intel_dp, sysfs_set);
> > + if (ret)
> > + return ret;
> > + break;
> > + default:
> > + return -EINVAL;
> > +
> > + }
> > + return count;
> > +}
> > +
> > +static DEVICE_ATTR(psr_enable, S_IRUGO | S_IWUSR, show_psr,
> > toggle_psr);
> > static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL);
> > static DEVICE_ATTR(rc6_residency_ms, S_IRUGO, show_rc6_ms, NULL);
> > static DEVICE_ATTR(rc6p_residency_ms, S_IRUGO, show_rc6p_ms,
> > NULL);
> > static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms,
> > NULL);
> > static DEVICE_ATTR(media_rc6_residency_ms, S_IRUGO,
> > show_media_rc6_ms, NULL);
> >
> > +static struct attribute *psr_attrs[] = {
> > + &dev_attr_psr_enable.attr,
> > + NULL
> > +};
> > +
> > +static struct attribute_group psr_attr_group = {
> > + .name = power_group_name,
> > + .attrs = psr_attrs
> > +};
> > +
> > static struct attribute *rc6_attrs[] = {
> > &dev_attr_rc6_enable.attr,
> > &dev_attr_rc6_residency_ms.attr,
> > @@ -596,6 +668,12 @@ void i915_setup_sysfs(struct drm_device *dev)
> > int ret;
> >
> > #ifdef CONFIG_PM
> > + if (HAS_PSR(dev)) {
> > + ret = sysfs_merge_group(&dev->primary->kdev->kobj,
> > + &psr_attr_group);
> > + if (ret)
> > + DRM_ERROR("PSR sysfs setup failed\n");
> > + }
> > if (HAS_RC6(dev)) {
> > ret = sysfs_merge_group(&dev->primary->kdev->kobj,
> > &rc6_attr_group);
> > @@ -652,6 +730,7 @@ void i915_teardown_sysfs(struct drm_device
> > *dev)
> > device_remove_bin_file(dev->primary->kdev, &dpf_attrs_1);
> > device_remove_bin_file(dev->primary->kdev, &dpf_attrs);
> > #ifdef CONFIG_PM
> > + sysfs_unmerge_group(&dev->primary->kdev->kobj,
> > &psr_attr_group);
> > sysfs_unmerge_group(&dev->primary->kdev->kobj,
> > &rc6_attr_group);
> > sysfs_unmerge_group(&dev->primary->kdev->kobj,
> > &rc6p_attr_group);
> > #endif
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 921edf1..8e384e5 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1689,7 +1689,8 @@ static void intel_enable_ddi(struct
> > intel_encoder *intel_encoder)
> > intel_dp_stop_link_train(intel_dp);
> >
> > intel_edp_backlight_on(intel_dp);
> > - intel_psr_enable(intel_dp);
> > + if (dev_priv->psr.sysfs_set != true)
> > + intel_psr_enable(intel_dp, dev_priv-
> > > psr.sysfs_set);
> > intel_edp_drrs_enable(intel_dp);
> > }
> >
> > @@ -1717,7 +1718,8 @@ static void intel_disable_ddi(struct
> > intel_encoder *intel_encoder)
> > struct intel_dp *intel_dp =
> > enc_to_intel_dp(encoder);
> >
> > intel_edp_drrs_disable(intel_dp);
> > - intel_psr_disable(intel_dp);
> > + if (dev_priv->psr.sysfs_set != true)
> > + intel_psr_disable(intel_dp, dev_priv-
> > > psr.sysfs_set);
> > intel_edp_backlight_off(intel_dp);
> > }
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 7523558..183a60a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2421,13 +2421,16 @@ static void intel_disable_dp(struct
> > intel_encoder *encoder)
> > {
> > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder
> > ->base);
> > struct drm_device *dev = encoder->base.dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *crtc = to_intel_crtc(encoder
> > ->base.crtc);
> >
> > if (crtc->config->has_audio)
> > intel_audio_codec_disable(encoder);
> >
> > - if (HAS_PSR(dev) && !HAS_DDI(dev))
> > - intel_psr_disable(intel_dp);
> > + if (HAS_PSR(dev) && !HAS_DDI(dev)) {
> > + if (dev_priv->psr.sysfs_set != true)
> > + intel_psr_disable(intel_dp, dev_priv-
> > > psr.sysfs_set);
> > + }
> >
> > /* Make sure the panel is off before trying to change the
> > mode. But also
> > * ensure that we have vdd while we switch off the panel.
> > */
> > @@ -2689,9 +2692,11 @@ static void g4x_enable_dp(struct
> > intel_encoder
> > *encoder)
> > static void vlv_enable_dp(struct intel_encoder *encoder)
> > {
> > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder
> > ->base);
> > + struct drm_i915_private *dev_priv = to_i915(encoder-
> > > base.dev);
> >
> > intel_edp_backlight_on(intel_dp);
> > - intel_psr_enable(intel_dp);
> > + if (dev_priv->psr.sysfs_set != true)
> > + intel_psr_enable(intel_dp, dev_priv
> > ->psr.sysfs_set);
> > }
> >
> > static void g4x_pre_enable_dp(struct intel_encoder *encoder)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index e0fcfa1..199e8cc 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1446,8 +1446,8 @@ void intel_backlight_unregister(struct
> > drm_device *dev);
> >
> >
> > /* intel_psr.c */
> > -void intel_psr_enable(struct intel_dp *intel_dp);
> > -void intel_psr_disable(struct intel_dp *intel_dp);
> > +int intel_psr_enable(struct intel_dp *intel_dp, bool sysfs_set);
> > +int intel_psr_disable(struct intel_dp *intel_dp, bool sysfs_set);
> > void intel_psr_invalidate(struct drm_device *dev,
> > unsigned frontbuffer_bits);
> > void intel_psr_flush(struct drm_device *dev,
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index c3abae4..64cb714 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -378,34 +378,43 @@ static void intel_psr_activate(struct
> > intel_dp
> > *intel_dp)
> > /**
> > * intel_psr_enable - Enable PSR
> > * @intel_dp: Intel DP
> > + * @sysfs_set: Identifies if this feature is set from sysfs
> > *
> > * This function can only be called after the pipe is fully
> > trained
> > and enabled.
> > + *
> > + * Returns:
> > + * 0 on success and -errno otherwise.
> > */
> > -void intel_psr_enable(struct intel_dp *intel_dp)
> > +int intel_psr_enable(struct intel_dp *intel_dp, bool sysfs_set)
> > {
> > struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > struct drm_device *dev = intel_dig_port->base.base.dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *crtc = to_intel_crtc(intel_dig_port-
> > > base.base.crtc);
> > + int ret = 0;
> > +
> >
> > if (!HAS_PSR(dev)) {
> > DRM_DEBUG_KMS("PSR not supported on this
> > platform\n");
> > - return;
> > + return -EINVAL;
> > }
> >
> > if (!is_edp_psr(intel_dp)) {
> > DRM_DEBUG_KMS("PSR not supported by this
> > panel\n");
> > - return;
> > + return -EINVAL;
> > }
> >
> > mutex_lock(&dev_priv->psr.lock);
> > if (dev_priv->psr.enabled) {
> > DRM_DEBUG_KMS("PSR already in use\n");
> > + ret = -EALREADY;
> > goto unlock;
> > }
> >
> > - if (!intel_psr_match_conditions(intel_dp))
> > + if (!intel_psr_match_conditions(intel_dp)) {
> > + ret = -ENOTTY;
> > goto unlock;
> > + }
> >
> > dev_priv->psr.busy_frontbuffer_bits = 0;
> >
> > @@ -464,8 +473,12 @@ void intel_psr_enable(struct intel_dp
> > *intel_dp)
> > msecs_to_jiffies(intel_dp-
> > > panel_power_cycle_delay * 5));
> >
> > dev_priv->psr.enabled = intel_dp;
> > + if (sysfs_set)
> > + dev_priv->psr.sysfs_set = sysfs_set;
> > +
> > unlock:
> > mutex_unlock(&dev_priv->psr.lock);
> > + return ret;
> > }
> >
> > static void vlv_psr_disable(struct intel_dp *intel_dp)
> > @@ -520,10 +533,11 @@ static void hsw_psr_disable(struct intel_dp
> > *intel_dp)
> > /**
> > * intel_psr_disable - Disable PSR
> > * @intel_dp: Intel DP
> > + * @sysfs_set: Identifies if this feature is set from sysfs
> > *
> > * This function needs to be called before disabling pipe.
> > */
> > -void intel_psr_disable(struct intel_dp *intel_dp)
> > +int intel_psr_disable(struct intel_dp *intel_dp, bool sysfs_set)
> > {
> > struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > struct drm_device *dev = intel_dig_port->base.base.dev;
> > @@ -532,7 +546,7 @@ void intel_psr_disable(struct intel_dp
> > *intel_dp)
> > mutex_lock(&dev_priv->psr.lock);
> > if (!dev_priv->psr.enabled) {
> > mutex_unlock(&dev_priv->psr.lock);
> > - return;
> > + return -EALREADY;
> > }
> >
> > /* Disable PSR on Source */
> > @@ -545,9 +559,13 @@ void intel_psr_disable(struct intel_dp
> > *intel_dp)
> > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
> >
> > dev_priv->psr.enabled = NULL;
> > + if (sysfs_set)
> > + dev_priv->psr.sysfs_set = sysfs_set;
> > +
> > mutex_unlock(&dev_priv->psr.lock);
> >
> > cancel_delayed_work_sync(&dev_priv->psr.work);
> > + return 0;
> > }
> >
> > static void intel_psr_work(struct work_struct *work)
> > @@ -781,8 +799,10 @@ void intel_psr_init(struct drm_device *dev)
> >
> > /* Per platform default */
> > if (i915.enable_psr == -1) {
> > - if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > - i915.enable_psr = 1;
> > + if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> > + if (dev_priv->psr.sysfs_set != true)
> > + i915.enable_psr = 1;
> > + }
> > else
> > i915.enable_psr = 0;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
