Re: [PATCH 5/5] drm/i915: Move psr.link_standby setup to intel_psr_match_conditions()

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, May 18, 2016 at 09:34:22PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Determine the value of psr.link_standby at runtime rather than at init
> time. This helps in testing since you can change between link-off and
> link-standby at runtime.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 41 ++++++++++++++++++++--------------------
>  1 file changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 6cab66b1b26a..c814a4443749 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -339,6 +339,27 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
>  	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>  	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
>  
> +	/* 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;
> +	else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
> +		/* On VLV and CHV only standby mode is supported. */
> +		dev_priv->psr.link_standby = true;
> +	else
> +		/* For new platforms let's respect VBT back again */
> +		dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;

One thing we discussed here (well Rodrigo raised it) that by default we
shouldn't override the platform wa settings, in case the vbt is broken.
Instead when they're incompatible we should just disable PSR.

But that's definitely for another patch, and has a good chance to break
working setups. On this one:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +
> +	/* 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;
> +	}
> +
>  	dev_priv->psr.source_ok = false;
>  
>  	/*
> @@ -831,26 +852,6 @@ void intel_psr_init(struct drm_device *dev)
>  			i915.enable_psr = 0;
>  	}
>  
> -	/* 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;
> -	else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
> -		/* On VLV and CHV only standby mode is supported. */
> -		dev_priv->psr.link_standby = true;
> -	else
> -		/* 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);
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx