Re: [PATCH 5/7] drm/i915/display: Ignore only psr specific part of vsc sdp

[Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>]

On Thu, Dec 14, 2023 at 01:48:36PM +0200, Jouni Högander wrote:
> Pipe config check is currently ignoring vsc sdp changes completely
> if psr is enabled. We want to ignore only PSR part of it as there
> might be changes in colorimetry data. Also read back vsc_sdp when psr is
> used.
> 
> Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 9 ++++++---
>  drivers/gpu/drm/i915/display/intel_dp.c      | 4 ----
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f1e58163277d..0faf84d3680a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4764,7 +4764,11 @@ static bool
>  intel_compare_dp_vsc_sdp(const struct drm_dp_vsc_sdp *a,
>  			 const struct drm_dp_vsc_sdp *b)
>  {
> -	return memcmp(a, b, sizeof(*a)) == 0;
> +	return a->pixelformat == b->pixelformat &&
> +		a->colorimetry == b->colorimetry &&
> +		a->bpc == b->bpc &&
> +		a->dynamic_range == b->dynamic_range &&
> +		a->content_type == b->content_type;
>  }
>  
>  static bool
> @@ -5045,8 +5049,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  } while (0)
>  
>  #define PIPE_CONF_CHECK_DP_VSC_SDP(name) do { \
> -	if (!current_config->has_psr && !pipe_config->has_psr && \
> -	    !intel_compare_dp_vsc_sdp(&current_config->infoframes.name, \
> +	if (!intel_compare_dp_vsc_sdp(&current_config->infoframes.name, \
>  				      &pipe_config->infoframes.name)) { \
>  		pipe_config_dp_vsc_sdp_mismatch(dev_priv, fastset, __stringify(name), \
>  						&current_config->infoframes.name, \
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 635790ec2fb7..b7e9b6eb6f66 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4425,10 +4425,6 @@ static void intel_read_dp_vsc_sdp(struct intel_encoder *encoder,
>  	struct dp_sdp sdp = {};
>  	int ret;
>  
> -	/* When PSR is enabled, VSC SDP is handled by PSR routine */
> -	if (crtc_state->has_psr)
> -		return;

I almost got confused by this one, but checking the patch 3 again,
it makes total sense.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>

> -
>  	if ((crtc_state->infoframes.enable &
>  	     intel_hdmi_infoframe_enable(type)) == 0)
>  		return;
> -- 
> 2.34.1
>