[PATCH] drm/edid: Don't send non-zero YQ in AVI infoframe for HDMI 1.x sinks

[PATCH] drm/edid: Don't send non-zero YQ in AVI infoframe for HDMI 1.x sinks

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Apparently some sinks look at the YQ bits even when receiving RGB,
and they get somehow confused when they see a non-zero YQ value.
So we can't just blindly follow CEA-861-F and set YQ to match the
RGB range.

Unfortunately there is no good way to tell whether the sink
designer claims to have read CEA-861-F. The CEA extension block
revision number has generally been stuck at 3 since forever,
and even a very recently manufactured sink might be based on
an old design so the manufacturing date doesn't seem like
something we can use. In lieu of better information let's
follow CEA-861-F only for HDMI 2.0 sinks, since HDMI 2.0 is
based on CEA-861-F. For HDMI 1.x sinks we'll always set YQ=0.

The alternative would of course be to always set YQ=0. And if
we ever encounter a HDMI 2.0+ sink with this bug that's what
we'll probably have to do.

Cc: stable@vger.kernel.org
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Neil Kownacki <njkkow@gmail.com>
Reported-by: Neil Kownacki <njkkow@gmail.com>
Fixes: fcc8a22cc905 ("drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101639
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c        | 12 ++++++++++--
 drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
 drivers/gpu/drm/vc4/vc4_hdmi.c    |  3 ++-
 include/drm/drm_edid.h            |  3 ++-
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 749d07a01772..8567890f47a7 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4838,7 +4838,8 @@ void
 drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
 				   const struct drm_display_mode *mode,
 				   enum hdmi_quantization_range rgb_quant_range,
-				   bool rgb_quant_range_selectable)
+				   bool rgb_quant_range_selectable,
+				   bool is_hdmi2_sink)
 {
 	/*
 	 * CEA-861:
@@ -4862,8 +4863,15 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
 	 *  YQ-field to match the RGB Quantization Range being transmitted
 	 *  (e.g., when Limited Range RGB, set YQ=0 or when Full Range RGB,
 	 *  set YQ=1) and the Sink shall ignore the YQ-field."
+	 *
+	 * Unfortunate certain sinks (eg. VIZ Model 67/E261VA) get confused
+	 * by non-zero YQ when receiving RGB. There doesn't seem to be any
+	 * good way to tell which version of CEA-861 the sink supports, so
+	 * we limit non-zero YQ to HDMI 2.0 sinks only as HDMI 2.0 is based
+	 * on on CEA-861-F.
 	 */
-	if (rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED)
+	if (!is_hdmi2_sink ||
+	    rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED)
 		frame->ycc_quantization_range =
 			HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
 	else
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index fa1c793a21ef..2b23d3662eec 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -487,7 +487,8 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 					   crtc_state->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
-					   intel_hdmi->rgb_quant_range_selectable);
+					   intel_hdmi->rgb_quant_range_selectable,
+					   is_hdmi2_sink);
 
 	/* TODO: handle pixel repetition for YCBCR420 outputs */
 	intel_write_infoframe(encoder, crtc_state, &frame);
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 9a9a6b4acccf..d645df0c6d15 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -423,7 +423,8 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 					   vc4_encoder->limited_rgb_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
-					   vc4_encoder->rgb_range_selectable);
+					   vc4_encoder->rgb_range_selectable,
+					   false);
 
 	vc4_hdmi_write_infoframe(encoder, &frame);
 }
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 9e4e23524840..58ffb68efdc5 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -361,7 +361,8 @@ void
 drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
 				   const struct drm_display_mode *mode,
 				   enum hdmi_quantization_range rgb_quant_range,
-				   bool rgb_quant_range_selectable);
+				   bool rgb_quant_range_selectable,
+				   bool is_hdmi2_sink);
 
 /**
  * drm_eld_mnl - Get ELD monitor name length in bytes.
-- 
2.13.6

Re: [PATCH] drm/edid: Don't send non-zero YQ in AVI infoframe for HDMI 1.x sinks

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1182 bytes --]

Ville Syrjala <ville.syrjala@linux.intel.com> writes:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Apparently some sinks look at the YQ bits even when receiving RGB,
> and they get somehow confused when they see a non-zero YQ value.
> So we can't just blindly follow CEA-861-F and set YQ to match the
> RGB range.
>
> Unfortunately there is no good way to tell whether the sink
> designer claims to have read CEA-861-F. The CEA extension block
> revision number has generally been stuck at 3 since forever,
> and even a very recently manufactured sink might be based on
> an old design so the manufacturing date doesn't seem like
> something we can use. In lieu of better information let's
> follow CEA-861-F only for HDMI 2.0 sinks, since HDMI 2.0 is
> based on CEA-861-F. For HDMI 1.x sinks we'll always set YQ=0.
>
> The alternative would of course be to always set YQ=0. And if
> we ever encounter a HDMI 2.0+ sink with this bug that's what
> we'll probably have to do.

Should vc4 be doing anything special for HDMI2 sinks, if it's an HDMI1.4
source?

That said, as far as vc4, this patch is

Acked-by: Eric Anholt <eric@anholt.net>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] drm/edid: Don't send non-zero YQ in AVI infoframe for HDMI 1.x sinks

[Ville Syrjälä]

On Wed, Nov 08, 2017 at 12:17:28PM -0800, Eric Anholt wrote:
> Ville Syrjala <ville.syrjala@linux.intel.com> writes:
> 
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> >
> > Apparently some sinks look at the YQ bits even when receiving RGB,
> > and they get somehow confused when they see a non-zero YQ value.
> > So we can't just blindly follow CEA-861-F and set YQ to match the
> > RGB range.
> >
> > Unfortunately there is no good way to tell whether the sink
> > designer claims to have read CEA-861-F. The CEA extension block
> > revision number has generally been stuck at 3 since forever,
> > and even a very recently manufactured sink might be based on
> > an old design so the manufacturing date doesn't seem like
> > something we can use. In lieu of better information let's
> > follow CEA-861-F only for HDMI 2.0 sinks, since HDMI 2.0 is
> > based on CEA-861-F. For HDMI 1.x sinks we'll always set YQ=0.
> >
> > The alternative would of course be to always set YQ=0. And if
> > we ever encounter a HDMI 2.0+ sink with this bug that's what
> > we'll probably have to do.
> 
> Should vc4 be doing anything special for HDMI2 sinks, if it's an HDMI1.4
> source?

As long as you stick to < 340 MHz modes you shouldn't have to do
anything. For >=340 MHz you'd need to use some new HDMI 2.0 features.

Looks like vc4 crtc .mode_valid() doesn't do much. I presume it's up
to bridges/encoders to filter out most things that aren't supported?

> 
> That said, as far as vc4, this patch is
> 
> Acked-by: Eric Anholt <eric@anholt.net>

Ta.

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH] drm/edid: Don't send non-zero YQ in AVI infoframe for HDMI 1.x sinks

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]

Ville Syrjälä <ville.syrjala@linux.intel.com> writes:

> On Wed, Nov 08, 2017 at 12:17:28PM -0800, Eric Anholt wrote:
>> Ville Syrjala <ville.syrjala@linux.intel.com> writes:
>> 
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Apparently some sinks look at the YQ bits even when receiving RGB,
>> > and they get somehow confused when they see a non-zero YQ value.
>> > So we can't just blindly follow CEA-861-F and set YQ to match the
>> > RGB range.
>> >
>> > Unfortunately there is no good way to tell whether the sink
>> > designer claims to have read CEA-861-F. The CEA extension block
>> > revision number has generally been stuck at 3 since forever,
>> > and even a very recently manufactured sink might be based on
>> > an old design so the manufacturing date doesn't seem like
>> > something we can use. In lieu of better information let's
>> > follow CEA-861-F only for HDMI 2.0 sinks, since HDMI 2.0 is
>> > based on CEA-861-F. For HDMI 1.x sinks we'll always set YQ=0.
>> >
>> > The alternative would of course be to always set YQ=0. And if
>> > we ever encounter a HDMI 2.0+ sink with this bug that's what
>> > we'll probably have to do.
>> 
>> Should vc4 be doing anything special for HDMI2 sinks, if it's an HDMI1.4
>> source?
>
> As long as you stick to < 340 MHz modes you shouldn't have to do
> anything. For >=340 MHz you'd need to use some new HDMI 2.0 features.
>
> Looks like vc4 crtc .mode_valid() doesn't do much. I presume it's up
> to bridges/encoders to filter out most things that aren't supported?

I had a patch for that at
https://patchwork.freedesktop.org/series/30680/ -- fedora folks had run
into trouble with 4k monitors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] drm/edid: Don't send non-zero YQ in AVI infoframe for HDMI 1.x sinks

[Daniel Vetter]

On Wed, Nov 08, 2017 at 02:21:08PM -0800, Eric Anholt wrote:
> Ville Syrj�l� <ville.syrjala@linux.intel.com> writes:
> 
> > On Wed, Nov 08, 2017 at 12:17:28PM -0800, Eric Anholt wrote:
> >> Ville Syrjala <ville.syrjala@linux.intel.com> writes:
> >> 
> >> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> >> >
> >> > Apparently some sinks look at the YQ bits even when receiving RGB,
> >> > and they get somehow confused when they see a non-zero YQ value.
> >> > So we can't just blindly follow CEA-861-F and set YQ to match the
> >> > RGB range.
> >> >
> >> > Unfortunately there is no good way to tell whether the sink
> >> > designer claims to have read CEA-861-F. The CEA extension block
> >> > revision number has generally been stuck at 3 since forever,
> >> > and even a very recently manufactured sink might be based on
> >> > an old design so the manufacturing date doesn't seem like
> >> > something we can use. In lieu of better information let's
> >> > follow CEA-861-F only for HDMI 2.0 sinks, since HDMI 2.0 is
> >> > based on CEA-861-F. For HDMI 1.x sinks we'll always set YQ=0.
> >> >
> >> > The alternative would of course be to always set YQ=0. And if
> >> > we ever encounter a HDMI 2.0+ sink with this bug that's what
> >> > we'll probably have to do.
> >> 
> >> Should vc4 be doing anything special for HDMI2 sinks, if it's an HDMI1.4
> >> source?
> >
> > As long as you stick to < 340 MHz modes you shouldn't have to do
> > anything. For >=340 MHz you'd need to use some new HDMI 2.0 features.
> >
> > Looks like vc4 crtc .mode_valid() doesn't do much. I presume it's up
> > to bridges/encoders to filter out most things that aren't supported?
> 
> I had a patch for that at
> https://patchwork.freedesktop.org/series/30680/ -- fedora folks had run
> into trouble with 4k monitors.

Ack on the clock limiting patch, silly that it's stuck. No idea about CEC,
better for Hans/Boris I guess.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/edid: Don't send non-zero YQ in AVI infoframe for HDMI 1.x sinks

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 2060 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> On Wed, Nov 08, 2017 at 02:21:08PM -0800, Eric Anholt wrote:
>> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
>> 
>> > On Wed, Nov 08, 2017 at 12:17:28PM -0800, Eric Anholt wrote:
>> >> Ville Syrjala <ville.syrjala@linux.intel.com> writes:
>> >> 
>> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> >
>> >> > Apparently some sinks look at the YQ bits even when receiving RGB,
>> >> > and they get somehow confused when they see a non-zero YQ value.
>> >> > So we can't just blindly follow CEA-861-F and set YQ to match the
>> >> > RGB range.
>> >> >
>> >> > Unfortunately there is no good way to tell whether the sink
>> >> > designer claims to have read CEA-861-F. The CEA extension block
>> >> > revision number has generally been stuck at 3 since forever,
>> >> > and even a very recently manufactured sink might be based on
>> >> > an old design so the manufacturing date doesn't seem like
>> >> > something we can use. In lieu of better information let's
>> >> > follow CEA-861-F only for HDMI 2.0 sinks, since HDMI 2.0 is
>> >> > based on CEA-861-F. For HDMI 1.x sinks we'll always set YQ=0.
>> >> >
>> >> > The alternative would of course be to always set YQ=0. And if
>> >> > we ever encounter a HDMI 2.0+ sink with this bug that's what
>> >> > we'll probably have to do.
>> >> 
>> >> Should vc4 be doing anything special for HDMI2 sinks, if it's an HDMI1.4
>> >> source?
>> >
>> > As long as you stick to < 340 MHz modes you shouldn't have to do
>> > anything. For >=340 MHz you'd need to use some new HDMI 2.0 features.
>> >
>> > Looks like vc4 crtc .mode_valid() doesn't do much. I presume it's up
>> > to bridges/encoders to filter out most things that aren't supported?
>> 
>> I had a patch for that at
>> https://patchwork.freedesktop.org/series/30680/ -- fedora folks had run
>> into trouble with 4k monitors.
>
> Ack on the clock limiting patch, silly that it's stuck. No idea about CEC,
> better for Hans/Boris I guess.

Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] drm/edid: Don't send non-zero YQ in AVI infoframe for HDMI 1.x sinks

[Ville Syrjälä]

On Tue, Nov 14, 2017 at 06:38:11PM -0500, neil k wrote:
> I tested the patch from Ville Syrj�l�'s email and it works fine on top of
> 4.14 for me.

Thanks for confirming.

Patch pushed to drm-misc-fixes.

> 
> Thanks,
> Neil
> 
> On Thu, Nov 9, 2017 at 1:16 PM, Eric Anholt <eric@anholt.net> wrote:
> 
> > Daniel Vetter <daniel@ffwll.ch> writes:
> >
> > > On Wed, Nov 08, 2017 at 02:21:08PM -0800, Eric Anholt wrote:
> > >> Ville Syrj�l� <ville.syrjala@linux.intel.com> writes:
> > >>
> > >> > On Wed, Nov 08, 2017 at 12:17:28PM -0800, Eric Anholt wrote:
> > >> >> Ville Syrjala <ville.syrjala@linux.intel.com> writes:
> > >> >>
> > >> >> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > >> >> >
> > >> >> > Apparently some sinks look at the YQ bits even when receiving RGB,
> > >> >> > and they get somehow confused when they see a non-zero YQ value.
> > >> >> > So we can't just blindly follow CEA-861-F and set YQ to match the
> > >> >> > RGB range.
> > >> >> >
> > >> >> > Unfortunately there is no good way to tell whether the sink
> > >> >> > designer claims to have read CEA-861-F. The CEA extension block
> > >> >> > revision number has generally been stuck at 3 since forever,
> > >> >> > and even a very recently manufactured sink might be based on
> > >> >> > an old design so the manufacturing date doesn't seem like
> > >> >> > something we can use. In lieu of better information let's
> > >> >> > follow CEA-861-F only for HDMI 2.0 sinks, since HDMI 2.0 is
> > >> >> > based on CEA-861-F. For HDMI 1.x sinks we'll always set YQ=0.
> > >> >> >
> > >> >> > The alternative would of course be to always set YQ=0. And if
> > >> >> > we ever encounter a HDMI 2.0+ sink with this bug that's what
> > >> >> > we'll probably have to do.
> > >> >>
> > >> >> Should vc4 be doing anything special for HDMI2 sinks, if it's an
> > HDMI1.4
> > >> >> source?
> > >> >
> > >> > As long as you stick to < 340 MHz modes you shouldn't have to do
> > >> > anything. For >=340 MHz you'd need to use some new HDMI 2.0 features.
> > >> >
> > >> > Looks like vc4 crtc .mode_valid() doesn't do much. I presume it's up
> > >> > to bridges/encoders to filter out most things that aren't supported?
> > >>
> > >> I had a patch for that at
> > >> https://patchwork.freedesktop.org/series/30680/ -- fedora folks had run
> > >> into trouble with 4k monitors.
> > >
> > > Ack on the clock limiting patch, silly that it's stuck. No idea about
> > CEC,
> > > better for Hans/Boris I guess.
> >
> > Thanks!
> >

-- 
Ville Syrj�l�
Intel OTC