[PATCH] drm/i915: Attach a PSR property on eDP

[PATCH] drm/i915: Attach a PSR property on eDP

[Chris Wilson]

Let userspace know the status of Panel Self-Refresh by virtue of a
property on the appropriate connector.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_dp.c    | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_modes.c | 29 +++++++++++++++++++++++++++++
 4 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bdb419c48016..92561bdde201 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -642,6 +642,7 @@ struct i915_psr {
 	bool enabled;
 	bool active;
 	struct delayed_work work;
+	struct drm_property *property;
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e3ad8a7c9fd0..583b30c68b70 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1820,6 +1820,12 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 	return true;
 }
 
+static void intel_edp_set_psr_property(struct intel_connector *connector, uint64_t val)
+{
+	drm_object_property_set_value(&connector->base.base,
+				      to_i915(connector->base.dev)->psr.property, val);
+}
+
 static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -1835,6 +1841,8 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 	/* Enable PSR on the host */
 	intel_edp_psr_enable_source(intel_dp);
 
+	intel_edp_set_psr_property(intel_dp->attached_connector, 1);
+
 	dev_priv->psr.enabled = true;
 	dev_priv->psr.active = true;
 }
@@ -1931,6 +1939,7 @@ void intel_edp_psr_inactivate(struct drm_device *dev)
 			intel_dp = enc_to_intel_dp(&encoder->base);
 			intel_crtc = to_intel_crtc(encoder->base.crtc);
 
+			intel_edp_set_psr_property(connector, 0);
 			dev_priv->psr.active = false;
 
 			I915_WRITE(EDP_PSR_CTL(dev), I915_READ(EDP_PSR_CTL(dev))
@@ -3349,6 +3358,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 			dev_priv->psr.sink_support = true;
 			DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
 		}
+		if (HAS_PSR(dev))
+			intel_edp_set_psr_property(intel_dp->attached_connector,
+						   dev_priv->psr.sink_support ? dev_priv->psr.active : -1);
 	}
 
 	/* Training Pattern 3 support */
@@ -3977,6 +3989,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	intel_dp->color_range_auto = true;
 
 	if (is_edp(intel_dp)) {
+		intel_attach_psr_property(connector);
 		drm_mode_create_scaling_mode_property(connector->dev);
 		drm_object_attach_property(
 			&connector->base,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cfe140a74266..9c8524016e4b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -905,6 +905,7 @@ int intel_connector_update_modes(struct drm_connector *connector,
 int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
 void intel_attach_force_audio_property(struct drm_connector *connector);
 void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
+void intel_attach_psr_property(struct drm_connector *connector);
 
 
 /* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index 0e860f39933d..6c14c97fa8df 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -126,3 +126,32 @@ intel_attach_broadcast_rgb_property(struct drm_connector *connector)
 
 	drm_object_attach_property(&connector->base, prop, 0);
 }
+
+static const struct drm_prop_enum_list psr_names[] = {
+	{ -1, "Unsupported" },
+	{  0, "Idle" },
+	{  1, "Active" },
+};
+
+void
+intel_attach_psr_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property *prop;
+
+	prop = dev_priv->psr.property;
+	if (prop == NULL) {
+		prop = drm_property_create_enum(dev,
+						DRM_MODE_PROP_ENUM | DRM_MODE_PROP_IMMUTABLE,
+						"Panel Self-Refresh",
+						psr_names,
+						ARRAY_SIZE(psr_names));
+		if (prop == NULL)
+			return;
+
+		dev_priv->psr.property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop, 0);
+}
-- 
2.0.0

[PATCH] drm/i915: Attach a PSR property on eDP

[Chris Wilson]

Let userspace know the status of Panel Self-Refresh by virtue of a
property on the appropriate connector.

v2: Only attach the property if the driver is capable of PSR.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_dp.c    | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_modes.c | 29 +++++++++++++++++++++++++++++
 4 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bdb419c48016..92561bdde201 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -642,6 +642,7 @@ struct i915_psr {
 	bool enabled;
 	bool active;
 	struct delayed_work work;
+	struct drm_property *property;
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e3ad8a7c9fd0..51c0799ea83d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1820,6 +1820,12 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 	return true;
 }
 
+static void intel_edp_set_psr_property(struct intel_connector *connector, uint64_t val)
+{
+	drm_object_property_set_value(&connector->base.base,
+				      to_i915(connector->base.dev)->psr.property, val);
+}
+
 static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -1835,6 +1841,8 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 	/* Enable PSR on the host */
 	intel_edp_psr_enable_source(intel_dp);
 
+	intel_edp_set_psr_property(intel_dp->attached_connector, 1);
+
 	dev_priv->psr.enabled = true;
 	dev_priv->psr.active = true;
 }
@@ -1931,6 +1939,7 @@ void intel_edp_psr_inactivate(struct drm_device *dev)
 			intel_dp = enc_to_intel_dp(&encoder->base);
 			intel_crtc = to_intel_crtc(encoder->base.crtc);
 
+			intel_edp_set_psr_property(connector, 0);
 			dev_priv->psr.active = false;
 
 			I915_WRITE(EDP_PSR_CTL(dev), I915_READ(EDP_PSR_CTL(dev))
@@ -3349,6 +3358,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 			dev_priv->psr.sink_support = true;
 			DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
 		}
+		intel_edp_set_psr_property(intel_dp->attached_connector,
+					   dev_priv->psr.sink_support ? dev_priv->psr.active : -1);
 	}
 
 	/* Training Pattern 3 support */
@@ -3977,6 +3988,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	intel_dp->color_range_auto = true;
 
 	if (is_edp(intel_dp)) {
+		if (HAS_PSR(connector->dev))
+			intel_attach_psr_property(connector);
 		drm_mode_create_scaling_mode_property(connector->dev);
 		drm_object_attach_property(
 			&connector->base,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cfe140a74266..9c8524016e4b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -905,6 +905,7 @@ int intel_connector_update_modes(struct drm_connector *connector,
 int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
 void intel_attach_force_audio_property(struct drm_connector *connector);
 void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
+void intel_attach_psr_property(struct drm_connector *connector);
 
 
 /* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index 0e860f39933d..6c14c97fa8df 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -126,3 +126,32 @@ intel_attach_broadcast_rgb_property(struct drm_connector *connector)
 
 	drm_object_attach_property(&connector->base, prop, 0);
 }
+
+static const struct drm_prop_enum_list psr_names[] = {
+	{ -1, "Unsupported" },
+	{  0, "Idle" },
+	{  1, "Active" },
+};
+
+void
+intel_attach_psr_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property *prop;
+
+	prop = dev_priv->psr.property;
+	if (prop == NULL) {
+		prop = drm_property_create_enum(dev,
+						DRM_MODE_PROP_ENUM | DRM_MODE_PROP_IMMUTABLE,
+						"Panel Self-Refresh",
+						psr_names,
+						ARRAY_SIZE(psr_names));
+		if (prop == NULL)
+			return;
+
+		dev_priv->psr.property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop, 0);
+}
-- 
2.0.0

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[Daniel Vetter]

On Fri, Jun 13, 2014 at 10:32 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Let userspace know the status of Panel Self-Refresh by virtue of a
> property on the appropriate connector.
>
> v2: Only attach the property if the driver is capable of PSR.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Shouldn't we attach this to the crtc? Imo makes semantically more
sense. Or maybe to each plane and then we could label it a generic
"frontbuffer rendering discourage" option.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[Chris Wilson]

On Sat, Jun 14, 2014 at 01:39:45AM +0200, Daniel Vetter wrote:
> On Fri, Jun 13, 2014 at 10:32 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Let userspace know the status of Panel Self-Refresh by virtue of a
> > property on the appropriate connector.
> >
> > v2: Only attach the property if the driver is capable of PSR.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Shouldn't we attach this to the crtc? Imo makes semantically more
> sense. Or maybe to each plane and then we could label it a generic
> "frontbuffer rendering discourage" option.

It depends on the eDP panel itself, so it ties quite neatly into a
connector property. I'd also like a generic please don't render to the
directly into this CRTC property as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[Damien Lespiau]

On Fri, Jun 13, 2014 at 09:32:47PM +0100, Chris Wilson wrote:
> Let userspace know the status of Panel Self-Refresh by virtue of a
> property on the appropriate connector.
> 
> v2: Only attach the property if the driver is capable of PSR.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Can we have an updated property table in Documentation/DocBook/drm.tmpl
as well?

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_drv.h    |  1 +
>  drivers/gpu/drm/i915/intel_dp.c    | 13 +++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_modes.c | 29 +++++++++++++++++++++++++++++
>  4 files changed, 44 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bdb419c48016..92561bdde201 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -642,6 +642,7 @@ struct i915_psr {
>  	bool enabled;
>  	bool active;
>  	struct delayed_work work;
> +	struct drm_property *property;
>  };
>  
>  enum intel_pch {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e3ad8a7c9fd0..51c0799ea83d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1820,6 +1820,12 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
>  	return true;
>  }
>  
> +static void intel_edp_set_psr_property(struct intel_connector *connector, uint64_t val)
> +{
> +	drm_object_property_set_value(&connector->base.base,
> +				      to_i915(connector->base.dev)->psr.property, val);
> +}
> +
>  static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -1835,6 +1841,8 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
>  	/* Enable PSR on the host */
>  	intel_edp_psr_enable_source(intel_dp);
>  
> +	intel_edp_set_psr_property(intel_dp->attached_connector, 1);
> +
>  	dev_priv->psr.enabled = true;
>  	dev_priv->psr.active = true;
>  }
> @@ -1931,6 +1939,7 @@ void intel_edp_psr_inactivate(struct drm_device *dev)
>  			intel_dp = enc_to_intel_dp(&encoder->base);
>  			intel_crtc = to_intel_crtc(encoder->base.crtc);
>  
> +			intel_edp_set_psr_property(connector, 0);
>  			dev_priv->psr.active = false;
>  
>  			I915_WRITE(EDP_PSR_CTL(dev), I915_READ(EDP_PSR_CTL(dev))
> @@ -3349,6 +3358,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  			dev_priv->psr.sink_support = true;
>  			DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
>  		}
> +		intel_edp_set_psr_property(intel_dp->attached_connector,
> +					   dev_priv->psr.sink_support ? dev_priv->psr.active : -1);
>  	}
>  
>  	/* Training Pattern 3 support */
> @@ -3977,6 +3988,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  	intel_dp->color_range_auto = true;
>  
>  	if (is_edp(intel_dp)) {
> +		if (HAS_PSR(connector->dev))
> +			intel_attach_psr_property(connector);
>  		drm_mode_create_scaling_mode_property(connector->dev);
>  		drm_object_attach_property(
>  			&connector->base,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cfe140a74266..9c8524016e4b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -905,6 +905,7 @@ int intel_connector_update_modes(struct drm_connector *connector,
>  int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
>  void intel_attach_force_audio_property(struct drm_connector *connector);
>  void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
> +void intel_attach_psr_property(struct drm_connector *connector);
>  
>  
>  /* intel_overlay.c */
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index 0e860f39933d..6c14c97fa8df 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -126,3 +126,32 @@ intel_attach_broadcast_rgb_property(struct drm_connector *connector)
>  
>  	drm_object_attach_property(&connector->base, prop, 0);
>  }
> +
> +static const struct drm_prop_enum_list psr_names[] = {
> +	{ -1, "Unsupported" },
> +	{  0, "Idle" },
> +	{  1, "Active" },
> +};
> +
> +void
> +intel_attach_psr_property(struct drm_connector *connector)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_property *prop;
> +
> +	prop = dev_priv->psr.property;
> +	if (prop == NULL) {
> +		prop = drm_property_create_enum(dev,
> +						DRM_MODE_PROP_ENUM | DRM_MODE_PROP_IMMUTABLE,
> +						"Panel Self-Refresh",
> +						psr_names,
> +						ARRAY_SIZE(psr_names));
> +		if (prop == NULL)
> +			return;
> +
> +		dev_priv->psr.property = prop;
> +	}
> +
> +	drm_object_attach_property(&connector->base, prop, 0);
> +}
> -- 
> 2.0.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[Chris Wilson]

On Sat, Jun 14, 2014 at 09:17:34AM +0100, Damien Lespiau wrote:
> On Fri, Jun 13, 2014 at 09:32:47PM +0100, Chris Wilson wrote:
> > Let userspace know the status of Panel Self-Refresh by virtue of a
> > property on the appropriate connector.
> > 
> > v2: Only attach the property if the driver is capable of PSR.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Can we have an updated property table in Documentation/DocBook/drm.tmpl
> as well?

If you care to give me a taylor made example... ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[Damien Lespiau]

On Sat, Jun 14, 2014 at 09:26:20AM +0100, Chris Wilson wrote:
> On Sat, Jun 14, 2014 at 09:17:34AM +0100, Damien Lespiau wrote:
> > On Fri, Jun 13, 2014 at 09:32:47PM +0100, Chris Wilson wrote:
> > > Let userspace know the status of Panel Self-Refresh by virtue of a
> > > property on the appropriate connector.
> > > 
> > > v2: Only attach the property if the driver is capable of PSR.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > Can we have an updated property table in Documentation/DocBook/drm.tmpl
> > as well?
> 
> If you care to give me a taylor made example... ;-)

That prompted me wondering if it should be a DRM property instead, but there's
no harm in keeping it i915 specific until there's a second user.

$ git diff
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7df3134..3bb7ecb 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2648,7 +2648,7 @@ void intel_crt_init(struct drm_device *dev)
	<td valign="top" >TBD</td>
	</tr>
	<tr>
-	<td rowspan="21" valign="top" >i915</td>
+	<td rowspan="22" valign="top" >i915</td>
	<td rowspan="3" valign="top" >Generic</td>
	<td valign="top" >"Broadcast RGB"</td>
	<td valign="top" >ENUM</td>
@@ -2799,6 +2799,14 @@ void intel_crt_init(struct drm_device *dev)
	<td valign="top" >TBD</td>
	</tr>
	<tr>
+	<td valign="top" >eDP</td>
+	<td valign="top" >"PSR"</td>
+	<td valign="top" >ENUM | IMMUTABLE</td>
+	<td valign="top" >{ "Idle", "Active" }</td>
+	<td valign="top" >Connector</td>
+	<td valign="top" >Whether PSR is currently active or not</td>
+	</tr>
+	<tr>
	<td rowspan="3" valign="top" >CDV gma-500</td>
	<td rowspan="3" valign="top" >Generic</td>
	<td valign="top" >"Broadcast RGB"</td>

(It even renders correctly!)

-- 
Damien

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[Damien Lespiau]

On Fri, Jun 13, 2014 at 09:32:47PM +0100, Chris Wilson wrote:
> Let userspace know the status of Panel Self-Refresh by virtue of a
> property on the appropriate connector.
> 
> v2: Only attach the property if the driver is capable of PSR.

You probably want to expose it only if the panel is PSR-capable as well?
(is_edp_psr())

-- 
Damien

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[Chris Wilson]

On Sat, Jun 14, 2014 at 09:39:56AM +0100, Damien Lespiau wrote:
> On Fri, Jun 13, 2014 at 09:32:47PM +0100, Chris Wilson wrote:
> > Let userspace know the status of Panel Self-Refresh by virtue of a
> > property on the appropriate connector.
> > 
> > v2: Only attach the property if the driver is capable of PSR.
> 
> You probably want to expose it only if the panel is PSR-capable as well?
> (is_edp_psr())

I wanted to expose it nevertheless as there was hints of supporting
interchangeable panels.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm/i915: Attach a PSR property on eDP

[Chris Wilson]

Let userspace know the status of Panel Self-Refresh by virtue of a
property on the appropriate connector.

v2: Only attach the property if the driver is capable of PSR.
v3: Add docbook courtesy of Damien.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Damien Lespiau <damien.lespiau@intel.com>
---
 Documentation/DocBook/drm.tmpl     | 10 +++++++++-
 drivers/gpu/drm/i915/i915_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_dp.c    | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_modes.c | 29 +++++++++++++++++++++++++++++
 5 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7df3134ebc0e..e05841760c45 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2648,7 +2648,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="21" valign="top" >i915</td>
+	<td rowspan="22" valign="top" >i915</td>
 	<td rowspan="3" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
 	<td valign="top" >ENUM</td>
@@ -2799,6 +2799,14 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td valign="top" >eDP</td>
+	<td valign="top" >“Panel Self-Refresh”</td>
+	<td valign="top" >ENUM | IMMUTABLE</td>
+	<td valign="top" >{ "Unsupported", "Idle", "Active" }</td>
+	<td valign="top" >Connector</td>
+	<td valign="top" >Whether the eDP panel supports using self-refresh, which is a power saving mode for static displays as the panel is able to read from its own buffer rather than require the host to send the same frame on every vertical refresh, and whether it is idle or active</td>
+	</tr>
+	<tr>
 	<td rowspan="3" valign="top" >CDV gma-500</td>
 	<td rowspan="3" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bdb419c48016..92561bdde201 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -642,6 +642,7 @@ struct i915_psr {
 	bool enabled;
 	bool active;
 	struct delayed_work work;
+	struct drm_property *property;
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e3ad8a7c9fd0..51c0799ea83d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1820,6 +1820,12 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 	return true;
 }
 
+static void intel_edp_set_psr_property(struct intel_connector *connector, uint64_t val)
+{
+	drm_object_property_set_value(&connector->base.base,
+				      to_i915(connector->base.dev)->psr.property, val);
+}
+
 static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -1835,6 +1841,8 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 	/* Enable PSR on the host */
 	intel_edp_psr_enable_source(intel_dp);
 
+	intel_edp_set_psr_property(intel_dp->attached_connector, 1);
+
 	dev_priv->psr.enabled = true;
 	dev_priv->psr.active = true;
 }
@@ -1931,6 +1939,7 @@ void intel_edp_psr_inactivate(struct drm_device *dev)
 			intel_dp = enc_to_intel_dp(&encoder->base);
 			intel_crtc = to_intel_crtc(encoder->base.crtc);
 
+			intel_edp_set_psr_property(connector, 0);
 			dev_priv->psr.active = false;
 
 			I915_WRITE(EDP_PSR_CTL(dev), I915_READ(EDP_PSR_CTL(dev))
@@ -3349,6 +3358,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 			dev_priv->psr.sink_support = true;
 			DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
 		}
+		intel_edp_set_psr_property(intel_dp->attached_connector,
+					   dev_priv->psr.sink_support ? dev_priv->psr.active : -1);
 	}
 
 	/* Training Pattern 3 support */
@@ -3977,6 +3988,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	intel_dp->color_range_auto = true;
 
 	if (is_edp(intel_dp)) {
+		if (HAS_PSR(connector->dev))
+			intel_attach_psr_property(connector);
 		drm_mode_create_scaling_mode_property(connector->dev);
 		drm_object_attach_property(
 			&connector->base,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cfe140a74266..9c8524016e4b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -905,6 +905,7 @@ int intel_connector_update_modes(struct drm_connector *connector,
 int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
 void intel_attach_force_audio_property(struct drm_connector *connector);
 void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
+void intel_attach_psr_property(struct drm_connector *connector);
 
 
 /* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index 0e860f39933d..6c14c97fa8df 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -126,3 +126,32 @@ intel_attach_broadcast_rgb_property(struct drm_connector *connector)
 
 	drm_object_attach_property(&connector->base, prop, 0);
 }
+
+static const struct drm_prop_enum_list psr_names[] = {
+	{ -1, "Unsupported" },
+	{  0, "Idle" },
+	{  1, "Active" },
+};
+
+void
+intel_attach_psr_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property *prop;
+
+	prop = dev_priv->psr.property;
+	if (prop == NULL) {
+		prop = drm_property_create_enum(dev,
+						DRM_MODE_PROP_ENUM | DRM_MODE_PROP_IMMUTABLE,
+						"Panel Self-Refresh",
+						psr_names,
+						ARRAY_SIZE(psr_names));
+		if (prop == NULL)
+			return;
+
+		dev_priv->psr.property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop, 0);
+}
-- 
2.0.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Attach a PSR property on eDP

[Chris Wilson]

Let userspace know the status of Panel Self-Refresh by virtue of a
property on the appropriate connector.

v2: Only attach the property if the driver is capable of PSR.
v3: Add docbook courtesy of Damien.
v4: Mark the initial value as 'unsupported' - it will be determined
    correctly when we later read the DCPD from the panel.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Damien Lespiau <damien.lespiau@intel.com>
---
 Documentation/DocBook/drm.tmpl     | 10 +++++++++-
 drivers/gpu/drm/i915/i915_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_dp.c    | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_modes.c | 29 +++++++++++++++++++++++++++++
 5 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7df3134ebc0e..e05841760c45 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2648,7 +2648,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="21" valign="top" >i915</td>
+	<td rowspan="22" valign="top" >i915</td>
 	<td rowspan="3" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
 	<td valign="top" >ENUM</td>
@@ -2799,6 +2799,14 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td valign="top" >eDP</td>
+	<td valign="top" >“Panel Self-Refresh”</td>
+	<td valign="top" >ENUM | IMMUTABLE</td>
+	<td valign="top" >{ "Unsupported", "Idle", "Active" }</td>
+	<td valign="top" >Connector</td>
+	<td valign="top" >Whether the eDP panel supports using self-refresh, which is a power saving mode for static displays as the panel is able to read from its own buffer rather than require the host to send the same frame on every vertical refresh, and whether it is idle or active</td>
+	</tr>
+	<tr>
 	<td rowspan="3" valign="top" >CDV gma-500</td>
 	<td rowspan="3" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bdb419c48016..92561bdde201 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -642,6 +642,7 @@ struct i915_psr {
 	bool enabled;
 	bool active;
 	struct delayed_work work;
+	struct drm_property *property;
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e3ad8a7c9fd0..51c0799ea83d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1820,6 +1820,12 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 	return true;
 }
 
+static void intel_edp_set_psr_property(struct intel_connector *connector, uint64_t val)
+{
+	drm_object_property_set_value(&connector->base.base,
+				      to_i915(connector->base.dev)->psr.property, val);
+}
+
 static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -1835,6 +1841,8 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 	/* Enable PSR on the host */
 	intel_edp_psr_enable_source(intel_dp);
 
+	intel_edp_set_psr_property(intel_dp->attached_connector, 1);
+
 	dev_priv->psr.enabled = true;
 	dev_priv->psr.active = true;
 }
@@ -1931,6 +1939,7 @@ void intel_edp_psr_inactivate(struct drm_device *dev)
 			intel_dp = enc_to_intel_dp(&encoder->base);
 			intel_crtc = to_intel_crtc(encoder->base.crtc);
 
+			intel_edp_set_psr_property(connector, 0);
 			dev_priv->psr.active = false;
 
 			I915_WRITE(EDP_PSR_CTL(dev), I915_READ(EDP_PSR_CTL(dev))
@@ -3349,6 +3358,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 			dev_priv->psr.sink_support = true;
 			DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
 		}
+		intel_edp_set_psr_property(intel_dp->attached_connector,
+					   dev_priv->psr.sink_support ? dev_priv->psr.active : -1);
 	}
 
 	/* Training Pattern 3 support */
@@ -3977,6 +3988,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	intel_dp->color_range_auto = true;
 
 	if (is_edp(intel_dp)) {
+		if (HAS_PSR(connector->dev))
+			intel_attach_psr_property(connector);
 		drm_mode_create_scaling_mode_property(connector->dev);
 		drm_object_attach_property(
 			&connector->base,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cfe140a74266..9c8524016e4b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -905,6 +905,7 @@ int intel_connector_update_modes(struct drm_connector *connector,
 int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
 void intel_attach_force_audio_property(struct drm_connector *connector);
 void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
+void intel_attach_psr_property(struct drm_connector *connector);
 
 
 /* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index 0e860f39933d..a50174ecaaff 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -126,3 +126,32 @@ intel_attach_broadcast_rgb_property(struct drm_connector *connector)
 
 	drm_object_attach_property(&connector->base, prop, 0);
 }
+
+static const struct drm_prop_enum_list psr_names[] = {
+	{ -1, "Unsupported" },
+	{  0, "Idle" },
+	{  1, "Active" },
+};
+
+void
+intel_attach_psr_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property *prop;
+
+	prop = dev_priv->psr.property;
+	if (prop == NULL) {
+		prop = drm_property_create_enum(dev,
+						DRM_MODE_PROP_ENUM | DRM_MODE_PROP_IMMUTABLE,
+						"Panel Self-Refresh",
+						psr_names,
+						ARRAY_SIZE(psr_names));
+		if (prop == NULL)
+			return;
+
+		dev_priv->psr.property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop, -1);
+}
-- 
2.0.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Attach a PSR property on eDP

[Rodrigo Vivi]

From: Chris Wilson <chris@chris-wilson.co.uk>

Let userspace know the status of Panel Self-Refresh by virtue of a
property on the appropriate connector.

v2: Only attach the property if the driver is capable of PSR.
v3: Add docbook courtesy of Damien.
v4: Mark the initial value as 'unsupported' - it will be determined
    correctly when we later read the DCPD from the panel.
v5: Done by Rodrigo:
    	 - Add disabled state to match all cases
	 - Attach it anyway to eDP since it is started as unsupported
	 - Change prop name to PSR
	 - Add enum to make states more clear
	 - Rebased on intel_psr.c changing func name and fixing states

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 Documentation/DocBook/drm.tmpl   | 10 +++++++-
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_dp.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 53 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index f4976cd..bda3948 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2834,7 +2834,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="21" valign="top" >i915</td>
+	<td rowspan="22" valign="top" >i915</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
 	<td valign="top" >ENUM</td>
@@ -2986,6 +2986,14 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td valign="top" >eDP</td>
+	<td valign="top" >“Panel Self-Refresh”</td>
+	<td valign="top" >ENUM | IMMUTABLE</td>
+	<td valign="top" >{ "Unsupported", "Idle", "Active" }</td>
+	<td valign="top" >Connector</td>
+	<td valign="top" >Whether the eDP panel supports using self-refresh, which is a power saving mode for static displays as the panel is able to read from its own buffer rather than require the host to send the same frame on every vertical refresh, and whether it is idle or active</td>
+	</tr>
+	<tr>
 	<td rowspan="2" valign="top" >CDV gma-500</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 92a9e85..afb9741 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -878,6 +878,7 @@ struct i915_psr {
 	bool active;
 	struct delayed_work work;
 	unsigned busy_frontbuffer_bits;
+	struct drm_property *property;
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index af28833..2aa9a8b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4774,6 +4774,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	intel_dp->color_range_auto = true;
 
 	if (is_edp(intel_dp)) {
+		intel_attach_psr_property(connector);
 		drm_mode_create_scaling_mode_property(connector->dev);
 		drm_object_attach_property(
 			&connector->base,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d79852f..0910782 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1207,6 +1207,7 @@ void intel_backlight_unregister(struct drm_device *dev);
 
 
 /* intel_psr.c */
+void intel_attach_psr_property(struct drm_connector *connector);
 bool intel_psr_match_conditions(struct intel_digital_port *dig_port);
 void intel_psr_enable(struct intel_dp *intel_dp);
 void intel_psr_disable(struct intel_dp *intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 34854af..a1f67c6 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,6 +56,52 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
+enum psr_state {
+	PSR_UNSUPPORTED = -1,
+	PSR_DISABLED,
+	PSR_IDLE,
+	PSR_ACTIVE,
+};
+
+static const struct drm_prop_enum_list psr_names[] = {
+	{ PSR_UNSUPPORTED, "Unsupported" },
+	{ PSR_DISABLED, "Disabled" },
+	{ PSR_IDLE, "Idle" },
+	{ PSR_ACTIVE, "Active" },
+};
+
+void intel_attach_psr_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property *prop;
+
+	prop = dev_priv->psr.property;
+	if (prop == NULL) {
+		prop = drm_property_create_enum(dev,
+						DRM_MODE_PROP_ENUM |
+						DRM_MODE_PROP_IMMUTABLE,
+						"PSR",
+						psr_names,
+						ARRAY_SIZE(psr_names));
+		if (prop == NULL)
+			return;
+
+		dev_priv->psr.property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop, PSR_UNSUPPORTED);
+}
+
+static void intel_psr_set_property(struct intel_connector *connector,
+				   uint64_t val)
+{
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+
+	drm_object_property_set_value(&connector->base.base,
+				      dev_priv->psr.property, val);
+}
+
 static bool is_edp_psr(struct intel_dp *intel_dp)
 {
 	return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
@@ -316,6 +362,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
 		vlv_psr_activate(intel_dp);
 
 	dev_priv->psr.active = true;
+	intel_psr_set_property(intel_dp->attached_connector, PSR_ACTIVE);
 }
 
 /**
@@ -371,6 +418,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	}
 
 	dev_priv->psr.enabled = intel_dp;
+	intel_psr_set_property(intel_dp->attached_connector, PSR_IDLE);
 unlock:
 	mutex_unlock(&dev_priv->psr.lock);
 }
@@ -397,6 +445,8 @@ static void vlv_psr_disable(struct intel_dp *intel_dp)
 		I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), val);
 
 		dev_priv->psr.active = false;
+		intel_psr_set_property(intel_dp->attached_connector,
+				       PSR_DISABLED);
 	} else {
 		WARN_ON(vlv_is_psr_active_on_pipe(dev, intel_crtc->pipe));
 	}
@@ -418,6 +468,8 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
 			DRM_ERROR("Timed out waiting for PSR Idle State\n");
 
 		dev_priv->psr.active = false;
+		intel_psr_set_property(intel_dp->attached_connector,
+				       PSR_DISABLED);
 	} else {
 		WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
 	}
@@ -541,6 +593,7 @@ static void intel_psr_exit(struct drm_device *dev)
 	}
 
 	dev_priv->psr.active = false;
+	intel_psr_set_property(intel_dp->attached_connector, PSR_IDLE);
 }
 
 /**
-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[Chris Wilson]

On Wed, Mar 25, 2015 at 01:11:57PM -0700, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Let userspace know the status of Panel Self-Refresh by virtue of a
> property on the appropriate connector.
> 
> v2: Only attach the property if the driver is capable of PSR.
> v3: Add docbook courtesy of Damien.
> v4: Mark the initial value as 'unsupported' - it will be determined
>     correctly when we later read the DCPD from the panel.
> v5: Done by Rodrigo:
>     	 - Add disabled state to match all cases
> 	 - Attach it anyway to eDP since it is started as unsupported
> 	 - Change prop name to PSR
> 	 - Add enum to make states more clear
> 	 - Rebased on intel_psr.c changing func name and fixing states
> 
> +void intel_attach_psr_property(struct drm_connector *connector)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_property *prop;
> +
> +	prop = dev_priv->psr.property;
> +	if (prop == NULL) {
> +		prop = drm_property_create_enum(dev,
> +						DRM_MODE_PROP_ENUM |
> +						DRM_MODE_PROP_IMMUTABLE,
> +						"PSR",
> +						psr_names,
> +						ARRAY_SIZE(psr_names));

Are you sure you want the user visible string to be "PSR"? Googling
prefers "Panel Self-Refresh".
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[Vivi, Rodrigo]

On Wed, 2015-03-25 at 20:31 +0000, Chris Wilson wrote:
> On Wed, Mar 25, 2015 at 01:11:57PM -0700, Rodrigo Vivi wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > Let userspace know the status of Panel Self-Refresh by virtue of a
> > property on the appropriate connector.
> > 
> > v2: Only attach the property if the driver is capable of PSR.
> > v3: Add docbook courtesy of Damien.
> > v4: Mark the initial value as 'unsupported' - it will be determined
> >     correctly when we later read the DCPD from the panel.
> > v5: Done by Rodrigo:
> >     	 - Add disabled state to match all cases
> > 	 - Attach it anyway to eDP since it is started as unsupported
> > 	 - Change prop name to PSR
> > 	 - Add enum to make states more clear
> > 	 - Rebased on intel_psr.c changing func name and fixing states
> > 
> > +void intel_attach_psr_property(struct drm_connector *connector)
> > +{
> > +	struct drm_device *dev = connector->dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_property *prop;
> > +
> > +	prop = dev_priv->psr.property;
> > +	if (prop == NULL) {
> > +		prop = drm_property_create_enum(dev,
> > +						DRM_MODE_PROP_ENUM |
> > +						DRM_MODE_PROP_IMMUTABLE,
> > +						"PSR",
> > +						psr_names,
> > +						ARRAY_SIZE(psr_names));
> 
> Are you sure you want the user visible string to be "PSR"? Googling
> prefers "Panel Self-Refresh".

To be honest, no I'm not sure... something I regretted already... Sorry!

Also talking about visible names I'm not sure about "Idle" as well...
Every time I read it get confused... I believe it is because PSR active
needs Idle usage...

What do you think about changing to Idle to Enable-Exit and Active to
Enable-Active?


> -Chris
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[chris]

On Wed, Mar 25, 2015 at 08:37:29PM +0000, Vivi, Rodrigo wrote:
> Also talking about visible names I'm not sure about "Idle" as well...
> Every time I read it get confused... I believe it is because PSR active
> needs Idle usage...
> 
> What do you think about changing to Idle to Enable-Exit and Active to
> Enable-Active?

If we switch states rapidly, we don't want to change the output property
(as it will trigger uevents, or at least it should).

So just "Enabled" ?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Attach a PSR property on eDP

[Rodrigo Vivi]

Let userspace know the status of Panel Self-Refresh by virtue of a
property on the appropriate connector.

v2: Only attach the property if the driver is capable of PSR.
v3: Add docbook courtesy of Damien.
v4: Mark the initial value as 'unsupported' - it will be determined
    correctly when we later read the DCPD from the panel.
v5: Done by Rodrigo:
    	 - Add disabled state to match all cases
	 - Attach it anyway to eDP since it is started as unsupported
	 - Change prop name to PSR
	 - Add enum to make states more clear
	 - Rebased on intel_psr.c changing func name and fixing states
v6: Done by Rodrigo:
    	 - Revert name to Panel Self-Refresh
	 - Only report Enable/Disable since Active/Exit change so rapidily
	   triggering many uevents as Chris pointed out.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 Documentation/DocBook/drm.tmpl   | 10 ++++++++-
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_dp.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 47 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index f4976cd..bda3948 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2834,7 +2834,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="21" valign="top" >i915</td>
+	<td rowspan="22" valign="top" >i915</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
 	<td valign="top" >ENUM</td>
@@ -2986,6 +2986,14 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td valign="top" >eDP</td>
+	<td valign="top" >“Panel Self-Refresh”</td>
+	<td valign="top" >ENUM | IMMUTABLE</td>
+	<td valign="top" >{ "Unsupported", "Idle", "Active" }</td>
+	<td valign="top" >Connector</td>
+	<td valign="top" >Whether the eDP panel supports using self-refresh, which is a power saving mode for static displays as the panel is able to read from its own buffer rather than require the host to send the same frame on every vertical refresh, and whether it is idle or active</td>
+	</tr>
+	<tr>
 	<td rowspan="2" valign="top" >CDV gma-500</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 92a9e85..afb9741 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -878,6 +878,7 @@ struct i915_psr {
 	bool active;
 	struct delayed_work work;
 	unsigned busy_frontbuffer_bits;
+	struct drm_property *property;
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5551b3c..cfd64bd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4776,6 +4776,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	intel_dp->color_range_auto = true;
 
 	if (is_edp(intel_dp)) {
+		intel_attach_psr_property(connector);
 		drm_mode_create_scaling_mode_property(connector->dev);
 		drm_object_attach_property(
 			&connector->base,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 250d70c..d286ba3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1207,6 +1207,7 @@ void intel_backlight_unregister(struct drm_device *dev);
 
 
 /* intel_psr.c */
+void intel_attach_psr_property(struct drm_connector *connector);
 bool intel_psr_ready(struct intel_dp *intel_dp);
 void intel_psr_enable(struct intel_dp *intel_dp);
 void intel_psr_disable(struct intel_dp *intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 72940d8..44742fc 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,6 +56,50 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
+enum psr_state {
+	PSR_UNSUPPORTED = -1,
+	PSR_DISABLED,
+	PSR_ENABLED,
+};
+
+static const struct drm_prop_enum_list psr_names[] = {
+	{ PSR_UNSUPPORTED, "Unsupported" },
+	{ PSR_DISABLED, "Disabled" },
+	{ PSR_ENABLED, "Enabled" },
+};
+
+void intel_attach_psr_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property *prop;
+
+	prop = dev_priv->psr.property;
+	if (prop == NULL) {
+		prop = drm_property_create_enum(dev,
+						DRM_MODE_PROP_ENUM |
+						DRM_MODE_PROP_IMMUTABLE,
+						"Panel Self-Refresh",
+						psr_names,
+						ARRAY_SIZE(psr_names));
+		if (prop == NULL)
+			return;
+
+		dev_priv->psr.property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop, PSR_UNSUPPORTED);
+}
+
+static void intel_psr_set_property(struct intel_connector *connector,
+				   uint64_t val)
+{
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+
+	drm_object_property_set_value(&connector->base.base,
+				      dev_priv->psr.property, val);
+}
+
 static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -390,6 +434,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	}
 
 	dev_priv->psr.enabled = intel_dp;
+	intel_psr_set_property(intel_dp->attached_connector, PSR_ENABLED);
 unlock:
 	mutex_unlock(&dev_priv->psr.lock);
 }
@@ -466,6 +511,8 @@ void intel_psr_disable(struct intel_dp *intel_dp)
 		vlv_psr_disable(intel_dp);
 
 	dev_priv->psr.enabled = NULL;
+	intel_psr_set_property(intel_dp->attached_connector, PSR_DISABLED);
+
 	mutex_unlock(&dev_priv->psr.lock);
 
 	cancel_delayed_work_sync(&dev_priv->psr.work);
-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Attach a PSR property on eDP

[Chris Wilson]

On Thu, Mar 26, 2015 at 12:22:07PM -0700, Rodrigo Vivi wrote:
> Let userspace know the status of Panel Self-Refresh by virtue of a
> property on the appropriate connector.
> 
> v2: Only attach the property if the driver is capable of PSR.
> v3: Add docbook courtesy of Damien.
> v4: Mark the initial value as 'unsupported' - it will be determined
>     correctly when we later read the DCPD from the panel.
> v5: Done by Rodrigo:
>     	 - Add disabled state to match all cases
> 	 - Attach it anyway to eDP since it is started as unsupported
> 	 - Change prop name to PSR
> 	 - Add enum to make states more clear
> 	 - Rebased on intel_psr.c changing func name and fixing states
> v6: Done by Rodrigo:
>     	 - Revert name to Panel Self-Refresh
> 	 - Only report Enable/Disable since Active/Exit change so rapidily
> 	   triggering many uevents as Chris pointed out.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Looks good to me,

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

We probably want to teach

  drm_object_set_value() { if (property == NULL) return -EINVAL; }

but it is safe enough to call with a NULL property, that guard would
just short-circuit the search for an impossible property.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx