[Intel-gfx] [PATCH 3/6] drm/i915: Reorder intel_psr2_config_valid()

[Mun, Gwan-gyeong]

Looks good to me.

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>

On Tue, 2020-05-26 at 15:14 -0700, José Roberto de Souza wrote:
> Future patches will bring PSR2 selective fetch configuration
> validation but most of the configuration checks will be used for HW
> tracking and selective fetch so the reoder was necessary.
> 
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 50 ++++++++++++--------
> ----
>  1 file changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 714c590b39f5..0c86e9e341a2 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -646,21 +646,6 @@ static bool intel_psr2_config_valid(struct
> intel_dp *intel_dp,
>  		return false;
>  	}
>  
> -	/*
> -	 * Some platforms lack PSR2 HW tracking and instead require
> manual
> -	 * tracking by software.  In this case, the driver is required
> to track
> -	 * the areas that need updates and program hardware to send
> selective
> -	 * updates.
> -	 *
> -	 * So until the software tracking is implemented, PSR2 needs to
> be
> -	 * disabled for platforms without PSR2 HW tracking.
> -	 */
> -	if (!HAS_PSR_HW_TRACKING(dev_priv)) {
> -		drm_dbg_kms(&dev_priv->drm,
> -			    "No PSR2 HW tracking in the platform\n");
> -		return false;
> -	}
> -
>  	/*
>  	 * DSC and PSR2 cannot be enabled simultaneously. If a
> requested
>  	 * resolution requires DSC to be enabled, priority is given to
> DSC
> @@ -672,6 +657,12 @@ static bool intel_psr2_config_valid(struct
> intel_dp *intel_dp,
>  		return false;
>  	}
>  
> +	if (crtc_state->crc_enabled) {
> +		drm_dbg_kms(&dev_priv->drm,
> +			    "PSR2 not enabled because it would inhibit
> pipe CRC calculation\n");
> +		return false;
> +	}
> +
>  	if (INTEL_GEN(dev_priv) >= 12) {
>  		psr_max_h = 5120;
>  		psr_max_v = 3200;
> @@ -686,14 +677,6 @@ static bool intel_psr2_config_valid(struct
> intel_dp *intel_dp,
>  		max_bpp = 24;
>  	}
>  
> -	if (crtc_hdisplay > psr_max_h || crtc_vdisplay > psr_max_v) {
> -		drm_dbg_kms(&dev_priv->drm,
> -			    "PSR2 not enabled, resolution %dx%d > max
> supported %dx%d\n",
> -			    crtc_hdisplay, crtc_vdisplay,
> -			    psr_max_h, psr_max_v);
> -		return false;
> -	}
> -
>  	if (crtc_state->pipe_bpp > max_bpp) {
>  		drm_dbg_kms(&dev_priv->drm,
>  			    "PSR2 not enabled, pipe bpp %d > max
> supported %d\n",
> @@ -714,9 +697,26 @@ static bool intel_psr2_config_valid(struct
> intel_dp *intel_dp,
>  		return false;
>  	}
>  
> -	if (crtc_state->crc_enabled) {
> +	/*
> +	 * Some platforms lack PSR2 HW tracking and instead require
> manual
> +	 * tracking by software.  In this case, the driver is required
> to track
> +	 * the areas that need updates and program hardware to send
> selective
> +	 * updates.
> +	 *
> +	 * So until the software tracking is implemented, PSR2 needs to
> be
> +	 * disabled for platforms without PSR2 HW tracking.
> +	 */
> +	if (!HAS_PSR_HW_TRACKING(dev_priv)) {
>  		drm_dbg_kms(&dev_priv->drm,
> -			    "PSR2 not enabled because it would inhibit
> pipe CRC calculation\n");
> +			    "No PSR2 HW tracking in the platform\n");
> +		return false;
> +	}
> +
> +	if (crtc_hdisplay > psr_max_h || crtc_vdisplay > psr_max_v) {
> +		drm_dbg_kms(&dev_priv->drm,
> +			    "PSR2 not enabled, resolution %dx%d > max
> supported %dx%d\n",
> +			    crtc_hdisplay, crtc_vdisplay,
> +			    psr_max_h, psr_max_v);
>  		return false;
>  	}
>