Re: [PATCH 7/7] drm/i915/display: Take care of VSC select field in video dip ctl register

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

On Thu, Dec 14, 2023 at 01:48:38PM +0200, Jouni Högander wrote:
> We need to configure VSC Select field in video dip ctl if we want to have
> e.g. colorimetry date in our VSC SDP.
> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 39e4f5f7c817..eedef8121ff7 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -523,10 +523,12 @@ void hsw_write_infoframe(struct intel_encoder *encoder,
>  			       0);
>  
>  	/* Wa_14013475917 */

For a moment I thought that your change in the logic below would bypass this w/a.
But then I read its description and notice that it is only about the bit 20, while
your new case below you set bit 26. So we should be good.

I even wonder if we shouldn't move this w/a below. let us to calculate the bits, but
then if wa condition val &= ~VIDEO_DIP_ENABLE_VSC_HSW;

> -	if (IS_DISPLAY_VER(dev_priv, 13, 14) && crtc_state->has_psr && type == DP_SDP_VSC)
> -		return;
> +	if (!(IS_DISPLAY_VER(dev_priv, 13, 14) && crtc_state->has_psr && type == DP_SDP_VSC))
> +		val |= hsw_infoframe_enable(type);
> +
> +	if (type == DP_SDP_VSC)
> +		val |= VSC_DIP_HW_DATA_SW_HEA;

for the part of need to set this bit 26 I confess that I'm not 100% sure.
What register this is in the spec?

but if someone else check these bits, I have nothing against this patch:

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

>  
> -	val |= hsw_infoframe_enable(type);
>  	intel_de_write(dev_priv, ctl_reg, val);
>  	intel_de_posting_read(dev_priv, ctl_reg);
>  }
> -- 
> 2.34.1
>