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

[Zanoni, Paulo R]

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 at intel.com>

> 
> Cc: Jani Nikula <jani.nikula at intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at 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);
>  }