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

2020-06-22 Thread Shankar, Uma



> > > > > > > > > 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 
> > > > > > > > > > > ---
> > > > > > > > > > >  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, , 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, , 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(_dp->aux);
> > > > > > > > > > > + else
> > > > > > > > > > > + infoframes_enabled =
> > > > > > > > > > > +_lspcon_read_avi_infoframe_enabled_parade(_dp
> > > > > > > > > > > +->au
> > > > > > > > > > > +x)
> > > > > > > > > > > +;
> > > > > > > > > > > +
> > > > > > > > > > > + 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.
> > > > >
> > > > > We have DP (VSC etc) packets also managed as HDMI infoframes only.
> > > > > We can keep the same with bitmask as VIDEO_DIP_ENABLE_AVI_HSW

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

2020-06-22 Thread Ville Syrjälä
On Mon, Jun 22, 2020 at 05:21:50PM +, Shankar, Uma wrote:
> 
> > > > > > > > -Original Message-
> > > > > > > > From: Ville Syrjälä 
> > > > > > > > Sent: Thursday, June 11, 2020 9:31 PM
> > > > > > > > To: Shankar, Uma 
> > > > > > > > Cc: intel-gfx@lists.freedesktop.org;
> > > > > > > > jani.nik...@linux.intel.com; Mun, Gwan- gyeong
> > > > > > > > 
> > > > > > > > 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 
> > > > > > > > > > ---
> > > > > > > > > >  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, , 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, , 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(_dp->aux);
> > > > > > > > > > +   else
> > > > > > > > > > +   infoframes_enabled =
> > > > > > > > > > +_lspcon_read_avi_infoframe_enabled_parade(_dp->au
> > > > > > > > > > +x)
> > > > > > > > > > +;
> > > > > > > > > > +
> > > > > > > > > > +   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 

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

2020-06-22 Thread Shankar, Uma



> -Original Message-
> From: Ville Syrjälä 
> Sent: Monday, June 22, 2020 11:10 PM
> To: Shankar, Uma 
> Cc: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com; Mun, Gwan-
> gyeong 
> Subject: Re: [v3 6/8] drm/i915/display: Implement infoframes readback for
> LSPCON
> 
> On Mon, Jun 22, 2020 at 05:17:50PM +, Shankar, Uma wrote:
> >
> > > > > > > > -Original Message-
> > > > > > > > From: Ville Syrjälä 
> > > > > > > > Sent: Thursday, June 11, 2020 9:31 PM
> > > > > > > > To: Shankar, Uma 
> > > > > > > > Cc: intel-gfx@lists.freedesktop.org;
> > > > > > > > jani.nik...@linux.intel.com; Mun, Gwan- gyeong
> > > > > > > > 
> > > > > > > > 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 
> > > > > > > > > > ---
> > > > > > > > > >  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, , 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, , 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(_dp->aux);
> > > > > > > > > > +   else
> > > > > > > > > > +   infoframes_enabled =
> > > > > > > > > > +_lspcon_read_avi_infoframe_enabled_parade(_dp->
> > > > > > > > > > +aux)
> > > > > > > > > > +;
> > > > > > > > > > +
> > > > > > > > > > +   if (infoframes_enabled)
> > > > > > > > > > +   return true;
> > > > > > > > >
> > > > > > > > > This is supposed to return a bitmask of all enabled 
> > > > > > > > > 

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

2020-06-22 Thread Ville Syrjälä
On Mon, Jun 22, 2020 at 05:17:50PM +, Shankar, Uma wrote:
> 
> > > > > > > -Original Message-
> > > > > > > From: Ville Syrjälä 
> > > > > > > Sent: Thursday, June 11, 2020 9:31 PM
> > > > > > > To: Shankar, Uma 
> > > > > > > Cc: intel-gfx@lists.freedesktop.org;
> > > > > > > jani.nik...@linux.intel.com; Mun, Gwan- gyeong
> > > > > > > 
> > > > > > > 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 
> > > > > > > > > ---
> > > > > > > > >  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, , 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, , 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(_dp->aux);
> > > > > > > > > + else
> > > > > > > > > + infoframes_enabled =
> > > > > > > > > +_lspcon_read_avi_infoframe_enabled_parade(_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
> > > 

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

2020-06-22 Thread Shankar, Uma


> > > > > > > -Original Message-
> > > > > > > From: Ville Syrjälä 
> > > > > > > Sent: Thursday, June 11, 2020 9:31 PM
> > > > > > > To: Shankar, Uma 
> > > > > > > Cc: intel-gfx@lists.freedesktop.org;
> > > > > > > jani.nik...@linux.intel.com; Mun, Gwan- gyeong
> > > > > > > 
> > > > > > > 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 
> > > > > > > > > ---
> > > > > > > > >  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, , 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, , 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(_dp->aux);
> > > > > > > > > + else
> > > > > > > > > + infoframes_enabled =
> > > > > > > > > +_lspcon_read_avi_infoframe_enabled_parade(_dp->au
> > > > > > > > > +x)
> > > > > > > > > +;
> > > > > > > > > +
> > > > > > > > > + 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.
> > >
> 

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

2020-06-22 Thread Shankar, Uma


> > > > > > -Original Message-
> > > > > > From: Ville Syrjälä 
> > > > > > Sent: Thursday, June 11, 2020 9:31 PM
> > > > > > To: Shankar, Uma 
> > > > > > Cc: intel-gfx@lists.freedesktop.org;
> > > > > > jani.nik...@linux.intel.com; Mun, Gwan- gyeong
> > > > > > 
> > > > > > 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 
> > > > > > > > ---
> > > > > > > >  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, , 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, , 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(_dp->aux);
> > > > > > > > +   else
> > > > > > > > +   infoframes_enabled =
> > > > > > > > +_lspcon_read_avi_infoframe_enabled_parade(_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.
> >
> > We have DP (VSC etc) packets also managed as HDMI infoframes only. We
> > can keep the same with bitmask as VIDEO_DIP_ENABLE_AVI_HSW for AVI and
> > similarly 

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

2020-06-22 Thread Ville Syrjälä
On Mon, Jun 22, 2020 at 11:44:58AM +, Shankar, Uma wrote:
> 
> > > > > -Original Message-
> > > > > From: Ville Syrjälä 
> > > > > Sent: Thursday, June 11, 2020 9:31 PM
> > > > > To: Shankar, Uma 
> > > > > Cc: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com;
> > > > > Mun, Gwan- gyeong 
> > > > > 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 
> > > > > > > ---
> > > > > > >  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, , 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, , 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(_dp->aux);
> > > > > > > + else
> > > > > > > + infoframes_enabled =
> > > > > > > +_lspcon_read_avi_infoframe_enabled_parade(_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.
> 
> We have DP (VSC etc) packets also managed as HDMI infoframes only. We can 
> keep the
> same with bitmask as VIDEO_DIP_ENABLE_AVI_HSW for AVI and similarly 
> VIDEO_DIP_ENABLE_GMP_HSW
> for DRM (HDR metadata). This will help all the helper align appropriately 
> even in the intel_dump_pipe_config.

intel_dump_infoframe() does not use any platform specific bitmasks.
So I don't understand what you're talking about here.

> 
> Will fix this accordingly and send the next version. Hope this is ok.
> 
> > > > Sure,  will fix this and resend the next version.
> > > >
> > > > > >
> > > > > > Also my question "how do we turn off infoframes once enabled?"
> > > > > > 

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

2020-06-22 Thread Shankar, Uma


> > > > -Original Message-
> > > > From: Ville Syrjälä 
> > > > Sent: Thursday, June 11, 2020 9:31 PM
> > > > To: Shankar, Uma 
> > > > Cc: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com;
> > > > Mun, Gwan- gyeong 
> > > > 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 
> > > > > > ---
> > > > > >  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, , 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, , 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(_dp->aux);
> > > > > > +   else
> > > > > > +   infoframes_enabled =
> > > > > > +_lspcon_read_avi_infoframe_enabled_parade(_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.

We have DP (VSC etc) packets also managed as HDMI infoframes only. We can keep 
the
same with bitmask as VIDEO_DIP_ENABLE_AVI_HSW for AVI and similarly 
VIDEO_DIP_ENABLE_GMP_HSW
for DRM (HDR metadata). This will help all the helper align appropriately even 
in the intel_dump_pipe_config.

Will fix this accordingly and send the next version. Hope this is ok.

> > > 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
> > > > > =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
> > > 

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

2020-06-15 Thread Shankar, Uma



> > > -Original Message-
> > > From: Ville Syrjälä 
> > > Sent: Thursday, June 11, 2020 9:31 PM
> > > To: Shankar, Uma 
> > > Cc: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com;
> > > Mun, Gwan- gyeong 
> > > 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 
> > > > > ---
> > > > >  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, , 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, , 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(_dp->aux);
> > > > > + else
> > > > > + infoframes_enabled =
> > > > > +_lspcon_read_avi_infoframe_enabled_parade(_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=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.
> 
> It's not valid to transmit infoframes to DVI sinks.

With your fix, we won't be enabling or setting the infoframe on DVI sinks.
If I understand correctly, we may have issue if we connect HDMI (where we would 
have
sent the infoframe) and later unplug and plug a DVI sink. With unplug if Lspcon 
is not
resetting this internally then this will be a problem. I 

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

2020-06-15 Thread Ville Syrjälä
On Mon, Jun 15, 2020 at 08:39:55PM +, Shankar, Uma wrote:
> 
> 
> > -Original Message-
> > From: Ville Syrjälä 
> > Sent: Thursday, June 11, 2020 9:31 PM
> > To: Shankar, Uma 
> > Cc: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com; Mun, Gwan-
> > gyeong 
> > 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 
> > > > ---
> > > >  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, , 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, , 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(_dp->aux);
> > > > +   else
> > > > +   infoframes_enabled =
> > > > +_lspcon_read_avi_infoframe_enabled_parade(_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=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.

It's not valid to transmit infoframes to DVI sinks.

> 
> 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,
> > > > +   

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

2020-06-15 Thread Shankar, Uma



> -Original Message-
> From: Ville Syrjälä 
> Sent: Thursday, June 11, 2020 9:31 PM
> To: Shankar, Uma 
> Cc: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com; Mun, Gwan-
> gyeong 
> 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 
> > > ---
> > >  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, , 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, , 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(_dp->aux);
> > > + else
> > > + infoframes_enabled =
> > > +_lspcon_read_avi_infoframe_enabled_parade(_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=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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2020-06-11 Thread Ville Syrjälä
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 
> > ---
> >  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, , 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, , 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(_dp->aux);
> > +   else
> > +   infoframes_enabled = 
> > _lspcon_read_avi_infoframe_enabled_parade(_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.

> 
> Also my question "how do we turn off infoframes once enabled?"
> from https://patchwork.freedesktop.org/patch/351719/?series=72928=1
> still remains unanswered...
> 
> > +
> > +   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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2020-06-11 Thread Ville Syrjälä
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 
> ---
>  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, , 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, , 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(_dp->aux);
> + else
> + infoframes_enabled = 
> _lspcon_read_avi_infoframe_enabled_parade(_dp->aux);
> +
> + if (infoframes_enabled)
> + return true;

This is supposed to return a bitmask of all enabled infoframes.

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

> +
> + 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2020-06-10 Thread Uma Shankar
Implemented Infoframes enabled readback for LSPCON devices.
This will help align the implementation with state readback
infrastructure.

Signed-off-by: Uma Shankar 
---
 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, , 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;
+}
+
+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, , 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(_dp->aux);
+   else
+   infoframes_enabled = 
_lspcon_read_avi_infoframe_enabled_parade(_dp->aux);
+
+   if (infoframes_enabled)
+   return true;
+
+   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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx