diff mbox

[3/3] drm/i915: Instrument PSR parameter for debuging with link standby x link off.

Message ID 1454352549-2411-4-git-send-email-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi Feb. 1, 2016, 6:49 p.m. UTC
Unfortunately we don't know all panels and platforms out there and we
found internal prototypes without VBT proper set but where only
link in standby worked well.

So, before enable PSR by default let's instrument the PSR parameter
in a way that we can identify different panels out there that might
require or work better with link standby mode.

It is also useful to say that for backward compatibility I'm not
changing the meaning of this flag. So "0" still means disabled
and "1" means enabled with full support and maximum power savings.

v2: Use positive value instead of negative for different operation mode
    as suggested by Daniel.

v3: As Paulo suggested use 2 to force link standby and 3 to force link
    fully on. Also split the link_standby introduction in a separated patch.

v4: Use DRM_ERROR for link off request on platfors that don't support and
    Remove the quirk promisse.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c |  3 ++-
 drivers/gpu/drm/i915/intel_psr.c   | 17 +++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

Comments

Zanoni, Paulo R Feb. 1, 2016, 7:47 p.m. UTC | #1
Em Seg, 2016-02-01 às 10:49 -0800, Rodrigo Vivi escreveu:
> Unfortunately we don't know all panels and platforms out there and we

> found internal prototypes without VBT proper set but where only

> link in standby worked well.

> 

> So, before enable PSR by default let's instrument the PSR parameter

> in a way that we can identify different panels out there that might

> require or work better with link standby mode.

> 

> It is also useful to say that for backward compatibility I'm not

> changing the meaning of this flag. So "0" still means disabled

> and "1" means enabled with full support and maximum power savings.

> 

> v2: Use positive value instead of negative for different operation

> mode

>     as suggested by Daniel.

> 

> v3: As Paulo suggested use 2 to force link standby and 3 to force

> link

>     fully on. Also split the link_standby introduction in a separated

> patch.

> 

> v4: Use DRM_ERROR for link off request on platfors that don't support

> and

>     Remove the quirk promisse.


"platforms" and "promise"

For the 3 patches:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>


> 

> Cc: Jani Nikula <jani.nikula@intel.com>

> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>

> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---

>  drivers/gpu/drm/i915/i915_params.c |  3 ++-

>  drivers/gpu/drm/i915/intel_psr.c   | 17 +++++++++++++++++

>  2 files changed, 19 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/gpu/drm/i915/i915_params.c

> b/drivers/gpu/drm/i915/i915_params.c

> index 8d90c25..8b9f368 100644

> --- a/drivers/gpu/drm/i915/i915_params.c

> +++ b/drivers/gpu/drm/i915/i915_params.c

> @@ -127,7 +127,8 @@ MODULE_PARM_DESC(enable_execlists,

>  	"(-1=auto [default], 0=disabled, 1=enabled)");

>  

>  module_param_named_unsafe(enable_psr, i915.enable_psr, int, 0600);

> -MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)");

> +MODULE_PARM_DESC(enable_psr, "Enable PSR "

> +		 "(0=disabled [default], 1=enabled - link mode

> chosen per-platform, 2=force link-standby mode, 3=force link-off

> mode)");

>  

>  module_param_named_unsafe(preliminary_hw_support,

> i915.preliminary_hw_support, int, 0600);

>  MODULE_PARM_DESC(preliminary_hw_support,

> diff --git a/drivers/gpu/drm/i915/intel_psr.c

> b/drivers/gpu/drm/i915/intel_psr.c

> index b99a105..4ab7579 100644

> --- a/drivers/gpu/drm/i915/intel_psr.c

> +++ b/drivers/gpu/drm/i915/intel_psr.c

> @@ -329,6 +329,12 @@ static bool intel_psr_match_conditions(struct

> intel_dp *intel_dp)

>  		return false;

>  	}

>  

> +	if ((IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) &&

> +	    !dev_priv->psr.link_standby) {

> +		DRM_ERROR("PSR condition failed: Link off requested

> but not supported on this platform\n");

> +		return false;

> +	}

> +

>  	if (IS_HASWELL(dev) &&

>  	    I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config-

> >cpu_transcoder)) &

>  		      S3D_ENABLE) {

> @@ -772,6 +778,7 @@ void intel_psr_init(struct drm_device *dev)

>  	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?

>  		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;

>  

> +	/* Set link_standby x link_off defaults */

>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev))

>  		/* HSW and BDW require workarounds that we don't

> implement. */

>  		dev_priv->psr.link_standby = false;

> @@ -782,6 +789,16 @@ void intel_psr_init(struct drm_device *dev)

>  		/* For new platforms let's respect VBT back again */

>  		dev_priv->psr.link_standby = dev_priv-

> >vbt.psr.full_link;

>  

> +	/* Override link_standby x link_off defaults */

> +	if (i915.enable_psr == 2 && !dev_priv->psr.link_standby) {

> +		DRM_DEBUG_KMS("PSR: Forcing link standby\n");

> +		dev_priv->psr.link_standby = true;

> +	}

> +	if (i915.enable_psr == 3 && dev_priv->psr.link_standby) {

> +		DRM_DEBUG_KMS("PSR: Forcing main link off\n");

> +		dev_priv->psr.link_standby = false;

> +	}

> +

>  	INIT_DELAYED_WORK(&dev_priv->psr.work, intel_psr_work);

>  	mutex_init(&dev_priv->psr.lock);

>  }
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8d90c25..8b9f368 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -127,7 +127,8 @@  MODULE_PARM_DESC(enable_execlists,
 	"(-1=auto [default], 0=disabled, 1=enabled)");
 
 module_param_named_unsafe(enable_psr, i915.enable_psr, int, 0600);
-MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)");
+MODULE_PARM_DESC(enable_psr, "Enable PSR "
+		 "(0=disabled [default], 1=enabled - link mode chosen per-platform, 2=force link-standby mode, 3=force link-off mode)");
 
 module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, 0600);
 MODULE_PARM_DESC(preliminary_hw_support,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index b99a105..4ab7579 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -329,6 +329,12 @@  static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 		return false;
 	}
 
+	if ((IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) &&
+	    !dev_priv->psr.link_standby) {
+		DRM_ERROR("PSR condition failed: Link off requested but not supported on this platform\n");
+		return false;
+	}
+
 	if (IS_HASWELL(dev) &&
 	    I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config->cpu_transcoder)) &
 		      S3D_ENABLE) {
@@ -772,6 +778,7 @@  void intel_psr_init(struct drm_device *dev)
 	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
 		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
 
+	/* Set link_standby x link_off defaults */
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 		/* HSW and BDW require workarounds that we don't implement. */
 		dev_priv->psr.link_standby = false;
@@ -782,6 +789,16 @@  void intel_psr_init(struct drm_device *dev)
 		/* For new platforms let's respect VBT back again */
 		dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;
 
+	/* Override link_standby x link_off defaults */
+	if (i915.enable_psr == 2 && !dev_priv->psr.link_standby) {
+		DRM_DEBUG_KMS("PSR: Forcing link standby\n");
+		dev_priv->psr.link_standby = true;
+	}
+	if (i915.enable_psr == 3 && dev_priv->psr.link_standby) {
+		DRM_DEBUG_KMS("PSR: Forcing main link off\n");
+		dev_priv->psr.link_standby = false;
+	}
+
 	INIT_DELAYED_WORK(&dev_priv->psr.work, intel_psr_work);
 	mutex_init(&dev_priv->psr.lock);
 }