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

[Rodrigo Vivi]

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.hogan...@intel.com>
> ---
>  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.v...@intel.com>

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