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

[Kahola, Mika]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Rodrigo Vivi
> Sent: Friday, December 15, 2023 7:26 PM
> To: Hogander, Jouni <jouni.hogander at intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 7/7] drm/i915/display: Take care of VSC select field in video dip ctl register
> 
> 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 at 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 at intel.com>

Since we are dealing with bit 26 (BSpec 70002) and there we set bits 25 and 26 to 10b i.e. data only. To me it looks like, this change doesn't interfere with the workaround.

Reviewed-by: Mika Kahola <mika.kahola at 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
> >