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

2019-01-08 Thread Ville Syrjala
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.

This changes the behaviour slightly as
drm_hdmi_avi_infoframe_quant_range() will set a non-zero Q bit
even when QS==0 iff the Q bit matched the default quantization
range for the given mode. This matches the recommendation in
HDMI 2.0 and is allowed even before that.

v2: Pimp commit msg (DK)

Signed-off-by: Ville Syrjälä 
Reviewed-by: Dhinakaran Pandiyan 
---
 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 ?
+  HDMI_QUANTIZATION_RANGE_LIMITED :
+  HDMI_QUANTIZATION_RANGE_FULL,
+  
intel_sdvo->rgb_quant_range_selectable);
 
len = hdmi_infoframe_pack(, sdvo_data, sizeof(sdvo_data));
if (len < 0)
-- 
2.19.2

___
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

2018-12-17 Thread Dhinakaran Pandiyan
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

2018-12-13 Thread 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ä 
> > > 
> > > 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

2018-12-12 Thread Ville Syrjälä
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

2018-12-12 Thread Dhinakaran Pandiyan
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


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

2018-11-20 Thread Ville Syrjala
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 ?
+  HDMI_QUANTIZATION_RANGE_LIMITED :
+  HDMI_QUANTIZATION_RANGE_FULL,
+  
intel_sdvo->rgb_quant_range_selectable);
 
len = hdmi_infoframe_pack(, sdvo_data, sizeof(sdvo_data));
if (len < 0)
-- 
2.18.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel