[Intel-gfx] [v3 6/8] drm/i915/display: Implement infoframes readback for LSPCON

[Shankar, Uma]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Thursday, June 11, 2020 9:31 PM
> To: Shankar, Uma <uma.shankar at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; jani.nikula at linux.intel.com; Mun, Gwan-
> gyeong <gwan-gyeong.mun at intel.com>
> Subject: Re: [v3 6/8] drm/i915/display: Implement infoframes readback for
> LSPCON
> 
> On Thu, Jun 11, 2020 at 06:46:50PM +0300, Ville Syrjälä wrote:
> > On Thu, Jun 11, 2020 at 12:42:30AM +0530, Uma Shankar wrote:
> > > Implemented Infoframes enabled readback for LSPCON devices.
> > > This will help align the implementation with state readback
> > > infrastructure.
> > >
> > > Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_lspcon.c | 63
> > > ++++++++++++++++++++-
> > >  1 file changed, 61 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c
> > > b/drivers/gpu/drm/i915/display/intel_lspcon.c
> > > index 9034ce6f20b9..0ebe9a700291 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
> > > @@ -576,11 +576,70 @@ void lspcon_set_infoframes(struct intel_encoder
> *encoder,
> > >  				  buf, ret);
> > >  }
> > >
> > > +static bool _lspcon_read_avi_infoframe_enabled_mca(struct
> > > +drm_dp_aux *aux) {
> > > +	int ret;
> > > +	u32 val = 0;
> > > +	u16 reg = LSPCON_MCA_AVI_IF_CTRL;
> > > +
> > > +	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
> > > +	if (ret < 0) {
> > > +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> > > +		return false;
> > > +	}
> > > +
> > > +	if (val & LSPCON_MCA_AVI_IF_KICKOFF)
> > > +		return true;
> > > +
> > > +	return false;
> >
> > return val & ...;
> >
> > > +}
> > > +
> > > +static bool _lspcon_read_avi_infoframe_enabled_parade(struct
> > > +drm_dp_aux *aux) {
> > > +	int ret;
> > > +	u32 val = 0;
> > > +	u16 reg = LSPCON_PARADE_AVI_IF_CTRL;
> > > +
> > > +	ret = drm_dp_dpcd_read(aux, reg, &val, 1);
> > > +	if (ret < 0) {
> > > +		DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> > > +		return false;
> > > +	}
> > > +
> > > +	if (val & LSPCON_PARADE_AVI_IF_KICKOFF)
> > > +		return true;
> > > +
> > > +	return false;
> > > +}
> > > +
> > >  u32 lspcon_infoframes_enabled(struct intel_encoder *encoder,
> > >  			      const struct intel_crtc_state *pipe_config)  {
> > > -	/* FIXME actually read this from the hw */
> > > -	return 0;
> > > +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > +	struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
> > > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > +	bool infoframes_enabled;
> > > +	u32 mask = 0;
> > > +	u32 val;
> > > +
> > > +	if (lspcon->vendor == LSPCON_VENDOR_MCA)
> > > +		infoframes_enabled =
> _lspcon_read_avi_infoframe_enabled_mca(&intel_dp->aux);
> > > +	else
> > > +		infoframes_enabled =
> > > +_lspcon_read_avi_infoframe_enabled_parade(&intel_dp->aux);
> > > +
> > > +	if (infoframes_enabled)
> > > +		return true;
> >
> > This is supposed to return a bitmask of all enabled infoframes.

> Actually since we're dealing with both the LSPCON specific stuff and DIP stuff for
> the DRM infoframe I think we should stop using using
> intel_hdmi_infoframes_enabled(), and instead provide a LSPCON specific
> replacement for it. That way we can directly return the abstract bitmask instead
> of pretending to return a bitmask of the DIP bits.

Sure,  will fix this and resend the next version.

> >
> > Also my question "how do we turn off infoframes once enabled?"
> > from
> > https://patchwork.freedesktop.org/patch/351719/?series=72928&rev=1
> > still remains unanswered...

For the AVI infoframe we generally compute and change the respective values. If no change is
requested and computed we can let the existing infoframes be transmitted. AFAIK there is no
mechanism called out, to explicitly disable this on Lspcon. Have not seen any issues due to this,
so hoping that it may be safe even if they are enabled.

I am planning to take your patch from the series and float along with this series, adding check for DRM
Infoframes also. Hope that is ok ?

Thanks Ville for your feedback.

Regards,
Uma Shankar

> > > +
> > > +	if (lspcon->hdr_supported) {
> > > +		val = intel_de_read(dev_priv,
> > > +				    HSW_TVIDEO_DIP_CTL(pipe_config-
> >cpu_transcoder));
> > > +		mask |= VIDEO_DIP_ENABLE_GMP_HSW;
> > > +
> > > +		if (val & mask)
> > > +			return val & mask;
> > > +	}
> > > +
> > > +	return false;
> > >  }
> > >
> > >  void lspcon_resume(struct intel_lspcon *lspcon)
> > > --
> > > 2.22.0
> >
> > --
> > Ville Syrjälä
> > Intel
> 
> --
> Ville Syrjälä
> Intel