[Intel-gfx] [PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well

[Dhinakaran Pandiyan]

On Thu, 2018-12-13 at 07:18 +0200, Ville Syrjälä wrote:
> On Wed, Dec 12, 2018 at 04:32:02PM -0800, Dhinakaran Pandiyan wrote:
> > On Tue, 2018-11-20 at 18:13 +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > Fill out the AVI infoframe quantization range bits using
> > > drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI encoder as
> > > well.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_sdvo.c | 19 ++++++++++---------
> > >  1 file changed, 10 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> > > b/drivers/gpu/drm/i915/intel_sdvo.c
> > > index 1277d31adb54..9c16e273fb8d 100644
> > > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > > @@ -984,6 +984,8 @@ static bool
> > > intel_sdvo_set_avi_infoframe(struct
> > > intel_sdvo *intel_sdvo,
> > >  					 const struct intel_crtc_state
> > > *pipe_config,
> > >  					 const struct
> > > drm_connector_state *conn_state)
> > >  {
> > > +	const struct drm_display_mode *adjusted_mode =
> > > +		&pipe_config->base.adjusted_mode;
> > >  	uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
> > >  	union hdmi_infoframe frame;
> > >  	int ret;
> > > @@ -991,20 +993,19 @@ static bool
> > > intel_sdvo_set_avi_infoframe(struct
> > > intel_sdvo *intel_sdvo,
> > >  
> > >  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> > >  						       conn_state-
> > > > connector,
> > > 
> > > -						       &pipe_config-
> > > > base.adjusted_mode);
> > > 
> > > +						       adjusted_mode);
> > >  	if (ret < 0) {
> > >  		DRM_ERROR("couldn't fill AVI infoframe\n");
> > >  		return false;
> > >  	}
> > >  
> > > -	if (intel_sdvo->rgb_quant_range_selectable) {
> > > -		if (pipe_config->limited_color_range)
> > > -			frame.avi.quantization_range =
> > > -				HDMI_QUANTIZATION_RANGE_LIMITED;
> > > -		else
> > > -			frame.avi.quantization_range =
> > > -				HDMI_QUANTIZATION_RANGE_FULL;
> > > -	}
> > > +	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> > > +					   conn_state->connector,
> > > +					   adjusted_mode,
> > > +					   pipe_config-
> > > > limited_color_range ?
> > > 
> > > +					   rgb_quant_range_selectableTE
> > > D :
> > > +					   HDMI_QUANTIZATION_RANGE_FULL
> > > ,
> > > +					   intel_sdvo-
> > > > rgb_quant_range_selectable);
> > 
> > Seems like avi.quantization_range can now get set to _LIMITED or
> > _FULL
> > even when ->rgb_quant_range_selectable == false, i.e., it is not
> > _DEFAULT anymore. Is that change in behavior intended?
> 
> ->quant_range_selectable will be passed to
> drm_hdmi_avi_infoframe_quant_range() which will do the right thing
> with
> it.
> 
> That said, there is a slight behavioural change in that it will set
Okay, I was indeed referring to this case.

> the Q bit even with QS==1 iff the quantization range matches the
> default quantization range for the mode. I noted this in the radeon
> patch but forgot to mention it here.
I'll let someone else with knowledge of HDMI to review this behavioral 
change. I'm trying to get hold of the HDMI spec now and will review if
this hasn't been looked at by then.


> 
> > 
> > 
> > >  
> > >  	len = hdmi_infoframe_pack(&frame, sdvo_data,
> > > sizeof(sdvo_data));
> > >  	if (len < 0)
> 
>