Re: [Intel-gfx] [PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well
On Thu, 2018-12-13 at 15:09 -0800, Dhinakaran Pandiyan wrote: > 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ä > > > > > > > > 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ä > > > > --- > > > > 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 = > > > > + _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(, > > > >conn_sta > > > > te- > > > > > connector, > > > > > > > > - _co > > > > nfig- > > > > > 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(, > > > > + conn_state- > > > > >connector, > > > > + adjusted_mode, > > > > + pipe_config- > > > > > limited_color_range ? > > > > > > > > + rgb_quant_range_sele > > > > ctableTE > > > > D : > > > > + HDMI_QUANTIZATION_RA > > > > NGE_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. > Looks alright now that I went through the specs. With commit message updated to make note of the Q value changes Reviewed-by: Dhinakaran Pandiyan > > > > > > > > > > > > > > > > > len = hdmi_infoframe_pack(, sdvo_data, > > > > sizeof(sdvo_data)); > > > > if (len < 0) > > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well
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ä > > > > > > 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ä > > > --- > > > 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 = > > > + _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(, > > > conn_state- > > > > connector, > > > > > > -_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(, > > > +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(, sdvo_data, > > > sizeof(sdvo_data)); > > > if (len < 0) > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well
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ä > > > > 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ä > > --- > > 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 = > > + _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(, > >conn_state- > > >connector, > > - _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(, > > + 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 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. > > > > > > len = hdmi_infoframe_pack(, sdvo_data, > > sizeof(sdvo_data)); > > if (len < 0) -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well
On Tue, 2018-11-20 at 18:13 +0200, Ville Syrjala wrote: > From: Ville Syrjälä > > 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ä > --- > 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 = > + _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(, > conn_state- > >connector, > -_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(, > +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? > > len = hdmi_infoframe_pack(, sdvo_data, > sizeof(sdvo_data)); > if (len < 0) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel