Re: [PATCH 4/7] drm/i915/display: Fix vsc_sdp computation

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Dec 14, 2023 at 01:48:35PM +0200, Jouni Högander wrote:
> Currently colorimetry data is not added for psr1 or non-psr case.
> Fix this by adding it as needed.
> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 48 ++++++++++---------------
>  1 file changed, 19 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 3550cebb44f2..635790ec2fb7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2628,36 +2628,26 @@ static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp,
>  	crtc_state->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_VSC);
>  	vsc->sdp_type = DP_SDP_VSC;
>  
> -	if (crtc_state->has_psr2) {
> -		if (intel_dp->colorimetry_support &&
> -		    intel_dp_needs_vsc_sdp(crtc_state, conn_state)) {
> -			/* [PSR2, +Colorimetry] */
> -			intel_dp_compute_vsc_colorimetry(crtc_state, conn_state,
> -							 vsc);
> -		} else {
> -			/*
> -			 * [PSR2, -Colorimetry]

could we please spell this out like below...
I got confused for a while thinking that - was a typo or a hyphen, not a minus.
only after checking the table in spec and vsc->revision = 5 inside
intel_dp_compute_vsc_colorimetry then I understood that this is for
'PSR2 without colorimetry'.

with that changed or at least clarified:

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


> -			 * Prepare VSC Header for SU as per eDP 1.4 spec, Table 6-11
> -			 * 3D stereo + PSR/PSR2 + Y-coordinate.
> -			 */
> -			vsc->revision = 0x4;
> -			vsc->length = 0xe;
> -		}
> +	/* Needs colorimetry */
> +	if (intel_dp_needs_vsc_sdp(crtc_state, conn_state)) {
> +		intel_dp_compute_vsc_colorimetry(crtc_state, conn_state,
> +						 vsc);
> +	} else if (crtc_state->has_psr2) {
> +		/*
> +		 * [PSR2, -Colorimetry]
> +		 * Prepare VSC Header for SU as per eDP 1.4 spec, Table 6-11
> +		 * 3D stereo + PSR/PSR2 + Y-coordinate.
> +		 */
> +		vsc->revision = 0x4;
> +		vsc->length = 0xe;
>  	} else if (crtc_state->has_panel_replay) {
> -		if (intel_dp->colorimetry_support &&
> -		    intel_dp_needs_vsc_sdp(crtc_state, conn_state)) {
> -			/* [Panel Replay with colorimetry info] */
> -			intel_dp_compute_vsc_colorimetry(crtc_state, conn_state,
> -							 vsc);
> -		} else {
> -			/*
> -			 * [Panel Replay without colorimetry info]
> -			 * Prepare VSC Header for SU as per DP 2.0 spec, Table 2-223
> -			 * VSC SDP supporting 3D stereo + Panel Replay.
> -			 */
> -			vsc->revision = 0x6;
> -			vsc->length = 0x10;
> -		}
> +		/*
> +		 * [Panel Replay without colorimetry info]
> +		 * Prepare VSC Header for SU as per DP 2.0 spec, Table 2-223
> +		 * VSC SDP supporting 3D stereo + Panel Replay.
> +		 */
> +		vsc->revision = 0x6;
> +		vsc->length = 0x10;
>  	} else {
>  		/*
>  		 * [PSR1]
> -- 
> 2.34.1
>