All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
@ 2021-01-24  4:40 Mario Kleiner
  2021-01-24  8:15 ` Simon Ser
  0 siblings, 1 reply; 19+ messages in thread
From: Mario Kleiner @ 2021-01-24  4:40 UTC (permalink / raw)
  To: dri-devel; +Cc: Uma Shankar, Shashank Sharma

According to the CTA 861.G spec, HDMI_STATIC_METADATA_TYPE1 is
not 1, but zero, so fix this enum.

While this doesn't cause problems in the kernel yet, as the
constant isn't actively used by drivers yet, it did create
confusion while debugging HDR problems in yours truly, and
also potential bugs in userspace components, as the wrong
enum propagates to components, e.g., like it did already
into intel-gpu-tools (tests/kms_hdr.c) or is used as wrong
reference when writing future new userspace HDR components
like compositors.

Fixes: fbb5d0353c62 ("drm: Add HDR source metadata property")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/linux/hdmi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 9850d59d6f1c..c8ec982ff498 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -156,7 +156,7 @@ enum hdmi_content_type {
 };
 
 enum hdmi_metadata_type {
-	HDMI_STATIC_METADATA_TYPE1 = 1,
+	HDMI_STATIC_METADATA_TYPE1 = 0,
 };
 
 enum hdmi_eotf {
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-24  4:40 [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant Mario Kleiner
@ 2021-01-24  8:15 ` Simon Ser
  2021-01-24 20:10   ` Mario Kleiner
  0 siblings, 1 reply; 19+ messages in thread
From: Simon Ser @ 2021-01-24  8:15 UTC (permalink / raw)
  To: Mario Kleiner; +Cc: Uma Shankar, dri-devel, Shashank Sharma

On Sunday, January 24th, 2021 at 5:40 AM, Mario Kleiner <mario.kleiner.de@gmail.com> wrote:

> According to the CTA 861.G spec, HDMI_STATIC_METADATA_TYPE1 is
> not 1, but zero, so fix this enum.
>
> While this doesn't cause problems in the kernel yet, as the
> constant isn't actively used by drivers yet, it did create
> confusion while debugging HDR problems in yours truly, and
> also potential bugs in userspace components, as the wrong
> enum propagates to components, e.g., like it did already
> into intel-gpu-tools (tests/kms_hdr.c) or is used as wrong
> reference when writing future new userspace HDR components
> like compositors.
>
> Fixes: fbb5d0353c62 ("drm: Add HDR source metadata property")
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Sadly I think it's too late for this. Some user-space (like Kodi)
already has copied over the kernel definitions, and libdrm already has
a release with the wrong value.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-24  8:15 ` Simon Ser
@ 2021-01-24 20:10   ` Mario Kleiner
  2021-01-24 20:24     ` Simon Ser
  0 siblings, 1 reply; 19+ messages in thread
From: Mario Kleiner @ 2021-01-24 20:10 UTC (permalink / raw)
  To: Simon Ser; +Cc: Uma Shankar, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1391 bytes --]

On Sun, Jan 24, 2021 at 9:15 AM Simon Ser <contact@emersion.fr> wrote:

> On Sunday, January 24th, 2021 at 5:40 AM, Mario Kleiner <
> mario.kleiner.de@gmail.com> wrote:
>
> > According to the CTA 861.G spec, HDMI_STATIC_METADATA_TYPE1 is
> > not 1, but zero, so fix this enum.
> >
> > While this doesn't cause problems in the kernel yet, as the
> > constant isn't actively used by drivers yet, it did create
> > confusion while debugging HDR problems in yours truly, and
> > also potential bugs in userspace components, as the wrong
> > enum propagates to components, e.g., like it did already
> > into intel-gpu-tools (tests/kms_hdr.c) or is used as wrong
> > reference when writing future new userspace HDR components
> > like compositors.
> >
> > Fixes: fbb5d0353c62 ("drm: Add HDR source metadata property")
> > Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> > Cc: Uma Shankar <uma.shankar@intel.com>
> > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Sadly I think it's too late for this. Some user-space (like Kodi)
> already has copied over the kernel definitions, and libdrm already has
> a release with the wrong value.
>

But it still needs to be fixed if we want working HDR. I thought libdrm
copies the definitions from the kernel periodically, so the fix should
propagate?

[-- Attachment #1.2: Type: text/html, Size: 2142 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-24 20:10   ` Mario Kleiner
@ 2021-01-24 20:24     ` Simon Ser
  2021-01-24 21:04       ` Mario Kleiner
  0 siblings, 1 reply; 19+ messages in thread
From: Simon Ser @ 2021-01-24 20:24 UTC (permalink / raw)
  To: Mario Kleiner; +Cc: Uma Shankar, dri-devel

On Sunday, January 24th, 2021 at 9:10 PM, Mario Kleiner <mario.kleiner.de@gmail.com> wrote:

> But it still needs to be fixed if we want working HDR. I thought
> libdrm copies the definitions from the kernel periodically, so the
> fix should propagate?

There will always be user-space that sends 1 instead of 0. This
shouldn't fail on more recent kernels or it will be a regression.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-24 20:24     ` Simon Ser
@ 2021-01-24 21:04       ` Mario Kleiner
  2021-01-25 12:09         ` Ville Syrjälä
  0 siblings, 1 reply; 19+ messages in thread
From: Mario Kleiner @ 2021-01-24 21:04 UTC (permalink / raw)
  To: Simon Ser; +Cc: Uma Shankar, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1999 bytes --]

On Sun, Jan 24, 2021 at 9:24 PM Simon Ser <contact@emersion.fr> wrote:

> On Sunday, January 24th, 2021 at 9:10 PM, Mario Kleiner <
> mario.kleiner.de@gmail.com> wrote:
>
> > But it still needs to be fixed if we want working HDR. I thought
> > libdrm copies the definitions from the kernel periodically, so the
> > fix should propagate?
>
> There will always be user-space that sends 1 instead of 0. This
> shouldn't fail on more recent kernels or it will be a regression.
>

Yes, i know, regressing user-space is bad, but in this very specific case a
"good" one, if ever. At the moment, it wouldn't regress userspace simply
because the kernel doesn't actually check for the correct value in its HDR
metadata handling. But the value itself is sent as HDMI HDR metadata to the
attached HDR display monitor, so if the monitors firmware checks, it will
classify the wrong value of 1 as invalid and disable HDR mode on the
display, which is certainly not what a HDR client application wants. And
future HDR standards which will actually allocate the value 1 to a
different mode of HDR operation will switch to the wrong mode /
misinterpret the sent HDR metadata with hillarious results, which is also
not in the interest of a HDR client application, or a HDR capable
compositor.

Iow. if clients continue to use the wrong value 1 then HDR display will
break in various ways on correctly implemented HDR displays, but in a
mystifying and hard to debug way. The kernel rejecting a wrong setting
explicitly and forcing a bug fix in the client would be a blessing in this
case.

I spent weeks last year, going in circles and hunting ghost bugs related to
HDR because much of the HDR stuff, both drivers and monitor firmware seems
to be in not a great shape. "Less wrong" would be a big step forward.
Especially with the cheaper HDR monitors it is difficult to see when things
go wrong, because we don't have good expectations on how proper HDR should
look and the lower end HDR displays don't help.

-mario

[-- Attachment #1.2: Type: text/html, Size: 2506 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-24 21:04       ` Mario Kleiner
@ 2021-01-25 12:09         ` Ville Syrjälä
  2021-01-25 12:14           ` Simon Ser
  2021-01-25 16:08           ` Mario Kleiner
  0 siblings, 2 replies; 19+ messages in thread
From: Ville Syrjälä @ 2021-01-25 12:09 UTC (permalink / raw)
  To: Mario Kleiner; +Cc: Uma Shankar, dri-devel

On Sun, Jan 24, 2021 at 10:04:54PM +0100, Mario Kleiner wrote:
> On Sun, Jan 24, 2021 at 9:24 PM Simon Ser <contact@emersion.fr> wrote:
> 
> > On Sunday, January 24th, 2021 at 9:10 PM, Mario Kleiner <
> > mario.kleiner.de@gmail.com> wrote:
> >
> > > But it still needs to be fixed if we want working HDR. I thought
> > > libdrm copies the definitions from the kernel periodically, so the
> > > fix should propagate?
> >
> > There will always be user-space that sends 1 instead of 0. This
> > shouldn't fail on more recent kernels or it will be a regression.
> >
> 
> Yes, i know, regressing user-space is bad, but in this very specific case a
> "good" one, if ever. At the moment, it wouldn't regress userspace simply
> because the kernel doesn't actually check for the correct value in its HDR
> metadata handling. But the value itself is sent as HDMI HDR metadata to the
> attached HDR display monitor, so if the monitors firmware checks, it will
> classify the wrong value of 1 as invalid and disable HDR mode on the
> display, which is certainly not what a HDR client application wants. And
> future HDR standards which will actually allocate the value 1 to a
> different mode of HDR operation will switch to the wrong mode /
> misinterpret the sent HDR metadata with hillarious results, which is also
> not in the interest of a HDR client application, or a HDR capable
> compositor.
> 
> Iow. if clients continue to use the wrong value 1 then HDR display will
> break in various ways on correctly implemented HDR displays, but in a
> mystifying and hard to debug way. The kernel rejecting a wrong setting
> explicitly and forcing a bug fix in the client would be a blessing in this
> case.
> 
> I spent weeks last year, going in circles and hunting ghost bugs related to
> HDR because much of the HDR stuff, both drivers and monitor firmware seems
> to be in not a great shape. "Less wrong" would be a big step forward.
> Especially with the cheaper HDR monitors it is difficult to see when things
> go wrong, because we don't have good expectations on how proper HDR should
> look and the lower end HDR displays don't help.

This is not an uapi defintion anyway so fixing should be fine.
I don't think we even check any of the client provided values, or do we?
EOTF I think we do check, but IMO that check should probably just be
nuked as well if we don't bother checking anything else.

I was in fact going to suggest nuking this entire hdr_sink_metadata
parsing as unused, but looks like amdgpu has started to use it for
some backlight stuff of all things.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-25 12:09         ` Ville Syrjälä
@ 2021-01-25 12:14           ` Simon Ser
  2021-01-25 16:00             ` Mario Kleiner
  2021-01-25 16:08           ` Mario Kleiner
  1 sibling, 1 reply; 19+ messages in thread
From: Simon Ser @ 2021-01-25 12:14 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Uma Shankar, dri-devel

> This is not an uapi defintion anyway so fixing should be fine.

Oh, my bad, I thought this was in drm_mode.h, but it's not. Then yeah
should be completely fine to fix it.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-25 12:14           ` Simon Ser
@ 2021-01-25 16:00             ` Mario Kleiner
  2021-01-25 16:05               ` Simon Ser
  0 siblings, 1 reply; 19+ messages in thread
From: Mario Kleiner @ 2021-01-25 16:00 UTC (permalink / raw)
  To: Simon Ser; +Cc: Uma Shankar, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 680 bytes --]

On Mon, Jan 25, 2021 at 1:14 PM Simon Ser <contact@emersion.fr> wrote:

> > This is not an uapi defintion anyway so fixing should be fine.
>
> Oh, my bad, I thought this was in drm_mode.h, but it's not. Then yeah
> should be completely fine to fix it.
>

Good! The beginning of the end of a sad story ;). So i guess i can get your
r-b's for it?

Will this fix propagate into igt and libdrm? Or are separate fixup patches
needed?

Simon, could you let the Kodi devs know in case you have a line to them? I
didn't know that there is even one more real-world HDR client for Linux,
apart from AMD's amdvlk Vulkan driver, which does things right and doesn't
need fixing.

thanks
mario

[-- Attachment #1.2: Type: text/html, Size: 1092 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-25 16:00             ` Mario Kleiner
@ 2021-01-25 16:05               ` Simon Ser
  2021-01-25 19:53                 ` Mario Kleiner
  0 siblings, 1 reply; 19+ messages in thread
From: Simon Ser @ 2021-01-25 16:05 UTC (permalink / raw)
  To: Mario Kleiner, Ville Syrjala; +Cc: Uma Shankar, dri-devel

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On Monday, January 25th, 2021 at 5:00 PM, Mario Kleiner <mario.kleiner.de@gmail.com> wrote:

> On Mon, Jan 25, 2021 at 1:14 PM Simon Ser <contact@emersion.fr> wrote:
>
> > > This is not an uapi defintion anyway so fixing should be fine.
> >
> > Oh, my bad, I thought this was in drm_mode.h, but it's not. Then yeah
> >
> > should be completely fine to fix it.
>
> Good! The beginning of the end of a sad story ;). So i guess i can
> get your r-b's for it?

Sorry, I haven't verified that this wouldn't break the world, so I'm
not comfortable giving a R-b.

> Will this fix propagate into igt and libdrm? Or are separate fixup patches needed?

No, since this is not part of UAPI.

> Simon, could you let the Kodi devs know in case you have a line to
> them? I didn't know that there is even one more real-world HDR client
> for Linux, apart from AMD's amdvlk Vulkan driver, which does things
> right and doesn't need fixing.

Seems like Kodi hardcodes the bad version:

https://github.com/xbmc/xbmc/blob/aa5c2e79c069ba7d0ab1d8ad930e4294bf554680/xbmc/cores/VideoPlayer/Buffers/VideoBufferDRMPRIME.h#L24

Maybe we should add the good version in UAPI header?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-25 12:09         ` Ville Syrjälä
  2021-01-25 12:14           ` Simon Ser
@ 2021-01-25 16:08           ` Mario Kleiner
  2021-01-25 16:11             ` Simon Ser
  1 sibling, 1 reply; 19+ messages in thread
From: Mario Kleiner @ 2021-01-25 16:08 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Uma Shankar, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 3745 bytes --]

On Mon, Jan 25, 2021 at 1:09 PM Ville Syrjälä <ville.syrjala@linux.intel.com>
wrote:

> On Sun, Jan 24, 2021 at 10:04:54PM +0100, Mario Kleiner wrote:
> > On Sun, Jan 24, 2021 at 9:24 PM Simon Ser <contact@emersion.fr> wrote:
> >
> > > On Sunday, January 24th, 2021 at 9:10 PM, Mario Kleiner <
> > > mario.kleiner.de@gmail.com> wrote:
> > >
> > > > But it still needs to be fixed if we want working HDR. I thought
> > > > libdrm copies the definitions from the kernel periodically, so the
> > > > fix should propagate?
> > >
> > > There will always be user-space that sends 1 instead of 0. This
> > > shouldn't fail on more recent kernels or it will be a regression.
> > >
> >
> > Yes, i know, regressing user-space is bad, but in this very specific
> case a
> > "good" one, if ever. At the moment, it wouldn't regress userspace simply
> > because the kernel doesn't actually check for the correct value in its
> HDR
> > metadata handling. But the value itself is sent as HDMI HDR metadata to
> the
> > attached HDR display monitor, so if the monitors firmware checks, it will
> > classify the wrong value of 1 as invalid and disable HDR mode on the
> > display, which is certainly not what a HDR client application wants. And
> > future HDR standards which will actually allocate the value 1 to a
> > different mode of HDR operation will switch to the wrong mode /
> > misinterpret the sent HDR metadata with hillarious results, which is also
> > not in the interest of a HDR client application, or a HDR capable
> > compositor.
> >
> > Iow. if clients continue to use the wrong value 1 then HDR display will
> > break in various ways on correctly implemented HDR displays, but in a
> > mystifying and hard to debug way. The kernel rejecting a wrong setting
> > explicitly and forcing a bug fix in the client would be a blessing in
> this
> > case.
> >
> > I spent weeks last year, going in circles and hunting ghost bugs related
> to
> > HDR because much of the HDR stuff, both drivers and monitor firmware
> seems
> > to be in not a great shape. "Less wrong" would be a big step forward.
> > Especially with the cheaper HDR monitors it is difficult to see when
> things
> > go wrong, because we don't have good expectations on how proper HDR
> should
> > look and the lower end HDR displays don't help.
>
> This is not an uapi defintion anyway so fixing should be fine.
> I don't think we even check any of the client provided values, or do we?
> EOTF I think we do check, but IMO that check should probably just be
> nuked as well if we don't bother checking anything else.
>
>
I think we check only EOTF atm. That check does make sense though, as
userspace getting that wrong will definitely knock out even low-end HDR
monitors. My tests with a - supposed to be pretty good according to tests -
DisplayHDR-600 monitor suggest that that's pretty much the only thing the
monitor actually uses, apart from CRC checking the data packet.


> I was in fact going to suggest nuking this entire hdr_sink_metadata
> parsing as unused, but looks like amdgpu has started to use it for
> some backlight stuff of all things.
>

My gut feeling says we will need this info in the kernel in the future,
independent of current users. Probably especially if one wants to do
interesting things which combine HDR with VRR/DP-Adaptive sync, or future
HDR standards (dynamic HDR metadata?), those infos in the kernel may become
quite useful.
 In some way it would even be nice to have all that info exposed in parsed
form as a connector property or such, so all clients can use the same
parsed data instead of reinventing the wheel. So I'd vote against nuking it.

Thanks,
-mario

[-- Attachment #1.2: Type: text/html, Size: 4652 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-25 16:08           ` Mario Kleiner
@ 2021-01-25 16:11             ` Simon Ser
  0 siblings, 0 replies; 19+ messages in thread
From: Simon Ser @ 2021-01-25 16:11 UTC (permalink / raw)
  To: Mario Kleiner; +Cc: Uma Shankar, dri-devel

On Monday, January 25th, 2021 at 5:08 PM, Mario Kleiner <mario.kleiner.de@gmail.com> wrote:

> In some way it would even be nice to have all that info exposed in
> parsed form as a connector property or such, so all clients can use
> the same parsed data instead of reinventing the wheel.

So far the policy has more or less been: if the kernel doesn't need to
understand it and user-space can grab it from the EDID, don't expose
it as a property. Maybe a user-space library would be better to avoid
reinventing the wheel.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-25 16:05               ` Simon Ser
@ 2021-01-25 19:53                 ` Mario Kleiner
  2021-02-10 21:04                   ` Mario Kleiner
  0 siblings, 1 reply; 19+ messages in thread
From: Mario Kleiner @ 2021-01-25 19:53 UTC (permalink / raw)
  To: Simon Ser; +Cc: Uma Shankar, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2050 bytes --]

On Mon, Jan 25, 2021 at 5:05 PM Simon Ser <contact@emersion.fr> wrote:

> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
>
> On Monday, January 25th, 2021 at 5:00 PM, Mario Kleiner <
> mario.kleiner.de@gmail.com> wrote:
>
> > On Mon, Jan 25, 2021 at 1:14 PM Simon Ser <contact@emersion.fr> wrote:
> >
> > > > This is not an uapi defintion anyway so fixing should be fine.
> > >
> > > Oh, my bad, I thought this was in drm_mode.h, but it's not. Then yeah
> > >
> > > should be completely fine to fix it.
> >
> > Good! The beginning of the end of a sad story ;). So i guess i can
> > get your r-b's for it?
>
> Sorry, I haven't verified that this wouldn't break the world, so I'm
> not comfortable giving a R-b.
>
>
Breaking the world is pretty unlikely for an unused #define, but I
understand.

I guess Ville will have access to the relevant spec to verify: It is the
CTA-861-G spec, table 44 in section 6.9 and also specifically section 6.9.1.


> > Will this fix propagate into igt and libdrm? Or are separate fixup
> patches needed?
>
> No, since this is not part of UAPI.
>

Ok. I'll submit patches once this one landed in the kernel.


>
> > Simon, could you let the Kodi devs know in case you have a line to
> > them? I didn't know that there is even one more real-world HDR client
> > for Linux, apart from AMD's amdvlk Vulkan driver, which does things
> > right and doesn't need fixing.
>
> Seems like Kodi hardcodes the bad version:
>
>
> https://github.com/xbmc/xbmc/blob/aa5c2e79c069ba7d0ab1d8ad930e4294bf554680/xbmc/cores/VideoPlayer/Buffers/VideoBufferDRMPRIME.h#L24
>
>
Thanks. I've filed an issue to them under:

https://github.com/xbmc/xbmc/issues/19122


> Maybe we should add the good version in UAPI header?
>

I'm scared that future HDR definitions would be as carefully done and
reviewed as this one, given how much harder it would be to fix them :/
But maybe that's just exhausted me who spent too many weeks dealing with
HDR bugs everywhere.

-mario

[-- Attachment #1.2: Type: text/html, Size: 3497 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-01-25 19:53                 ` Mario Kleiner
@ 2021-02-10 21:04                   ` Mario Kleiner
  2021-02-10 21:14                     ` Simon Ser
  0 siblings, 1 reply; 19+ messages in thread
From: Mario Kleiner @ 2021-02-10 21:04 UTC (permalink / raw)
  To: Simon Ser; +Cc: Uma Shankar, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2576 bytes --]

Ping! Anybody wanna review the most trivial, obviously correct, and
non-intrusive patch ever to fix a real bug? The Kodi devs changed their
broken constant and as if by magic, some LG TV now has working HDR support,
so we now know the constant is actually expected to be correct by at least
some real display hw out there.

thanks,
-mario


On Mon, Jan 25, 2021 at 8:53 PM Mario Kleiner <mario.kleiner.de@gmail.com>
wrote:

>
>
> On Mon, Jan 25, 2021 at 5:05 PM Simon Ser <contact@emersion.fr> wrote:
>
>> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
>>
>> On Monday, January 25th, 2021 at 5:00 PM, Mario Kleiner <
>> mario.kleiner.de@gmail.com> wrote:
>>
>> > On Mon, Jan 25, 2021 at 1:14 PM Simon Ser <contact@emersion.fr> wrote:
>> >
>> > > > This is not an uapi defintion anyway so fixing should be fine.
>> > >
>> > > Oh, my bad, I thought this was in drm_mode.h, but it's not. Then yeah
>> > >
>> > > should be completely fine to fix it.
>> >
>> > Good! The beginning of the end of a sad story ;). So i guess i can
>> > get your r-b's for it?
>>
>> Sorry, I haven't verified that this wouldn't break the world, so I'm
>> not comfortable giving a R-b.
>>
>>
> Breaking the world is pretty unlikely for an unused #define, but I
> understand.
>
> I guess Ville will have access to the relevant spec to verify: It is the
> CTA-861-G spec, table 44 in section 6.9 and also specifically section 6.9.1.
>
>
>> > Will this fix propagate into igt and libdrm? Or are separate fixup
>> patches needed?
>>
>> No, since this is not part of UAPI.
>>
>
> Ok. I'll submit patches once this one landed in the kernel.
>
>
>>
>> > Simon, could you let the Kodi devs know in case you have a line to
>> > them? I didn't know that there is even one more real-world HDR client
>> > for Linux, apart from AMD's amdvlk Vulkan driver, which does things
>> > right and doesn't need fixing.
>>
>> Seems like Kodi hardcodes the bad version:
>>
>>
>> https://github.com/xbmc/xbmc/blob/aa5c2e79c069ba7d0ab1d8ad930e4294bf554680/xbmc/cores/VideoPlayer/Buffers/VideoBufferDRMPRIME.h#L24
>>
>>
> Thanks. I've filed an issue to them under:
>
> https://github.com/xbmc/xbmc/issues/19122
>
>
>> Maybe we should add the good version in UAPI header?
>>
>
> I'm scared that future HDR definitions would be as carefully done and
> reviewed as this one, given how much harder it would be to fix them :/
> But maybe that's just exhausted me who spent too many weeks dealing with
> HDR bugs everywhere.
>
> -mario
>
>

[-- Attachment #1.2: Type: text/html, Size: 4288 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-02-10 21:04                   ` Mario Kleiner
@ 2021-02-10 21:14                     ` Simon Ser
  2021-02-10 22:02                       ` Mario Kleiner
  2021-02-23 17:52                       ` Mario Kleiner
  0 siblings, 2 replies; 19+ messages in thread
From: Simon Ser @ 2021-02-10 21:14 UTC (permalink / raw)
  To: Mario Kleiner; +Cc: Uma Shankar, dri-devel

On Wednesday, February 10th, 2021 at 10:04 PM, Mario Kleiner <mario.kleiner.de@gmail.com> wrote:

> Ping!

I now understand the problem better.

Reviewed-by: Simon Ser <contact@emersion.fr>

I'll push to drm-misc-next in a few days if no-one complains. Ping me
again if I forget.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-02-10 21:14                     ` Simon Ser
@ 2021-02-10 22:02                       ` Mario Kleiner
  2021-02-11 10:44                         ` Simon Ser
  2021-02-23 17:52                       ` Mario Kleiner
  1 sibling, 1 reply; 19+ messages in thread
From: Mario Kleiner @ 2021-02-10 22:02 UTC (permalink / raw)
  To: Simon Ser; +Cc: Uma Shankar, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 462 bytes --]

On Wed, Feb 10, 2021 at 10:14 PM Simon Ser <contact@emersion.fr> wrote:

> On Wednesday, February 10th, 2021 at 10:04 PM, Mario Kleiner <
> mario.kleiner.de@gmail.com> wrote:
>
> > Ping!
>
> I now understand the problem better.
>
> Reviewed-by: Simon Ser <contact@emersion.fr>
>
> I'll push to drm-misc-next in a few days if no-one complains. Ping me
> again if I forget.
>

Thanks! I'll prepare patches with the same fix for libdrm and igt as well
soon.
-mario

[-- Attachment #1.2: Type: text/html, Size: 938 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-02-10 22:02                       ` Mario Kleiner
@ 2021-02-11 10:44                         ` Simon Ser
  2021-02-12  5:49                           ` Mario Kleiner
  0 siblings, 1 reply; 19+ messages in thread
From: Simon Ser @ 2021-02-11 10:44 UTC (permalink / raw)
  To: Mario Kleiner; +Cc: Uma Shankar, dri-devel

On Wednesday, February 10th, 2021 at 11:02 PM, Mario Kleiner <mario.kleiner.de@gmail.com> wrote:

> I'll prepare patches with the same fix for libdrm and igt as well soon.

Please don't submit patches for drm_fourcc.h to downstream if they vendor the
whole file. Instead, update it from the kernel once your patch is merged to
drm-next. For instance, see [1] for libdrm.

Simon

[1]: https://gitlab.freedesktop.org/mesa/drm/-/blob/master/include/drm/README
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-02-11 10:44                         ` Simon Ser
@ 2021-02-12  5:49                           ` Mario Kleiner
  0 siblings, 0 replies; 19+ messages in thread
From: Mario Kleiner @ 2021-02-12  5:49 UTC (permalink / raw)
  To: Simon Ser; +Cc: Uma Shankar, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 590 bytes --]

On Thu, Feb 11, 2021 at 11:44 AM Simon Ser <contact@emersion.fr> wrote:

> On Wednesday, February 10th, 2021 at 11:02 PM, Mario Kleiner <
> mario.kleiner.de@gmail.com> wrote:
>
> > I'll prepare patches with the same fix for libdrm and igt as well soon.
>
> Please don't submit patches for drm_fourcc.h to downstream if they vendor
> the
> whole file. Instead, update it from the kernel once your patch is merged to
> drm-next. For instance, see [1] for libdrm.
>
> Simon
>
> [1]:
> https://gitlab.freedesktop.org/mesa/drm/-/blob/master/include/drm/README
>

Ok. Thanks for the info,
-mario

[-- Attachment #1.2: Type: text/html, Size: 1124 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-02-10 21:14                     ` Simon Ser
  2021-02-10 22:02                       ` Mario Kleiner
@ 2021-02-23 17:52                       ` Mario Kleiner
  2021-02-23 18:31                         ` Simon Ser
  1 sibling, 1 reply; 19+ messages in thread
From: Mario Kleiner @ 2021-02-23 17:52 UTC (permalink / raw)
  To: Simon Ser; +Cc: Uma Shankar, dri-devel

On Wed, Feb 10, 2021 at 10:14 PM Simon Ser <contact@emersion.fr> wrote:
>
> On Wednesday, February 10th, 2021 at 10:04 PM, Mario Kleiner <mario.kleiner.de@gmail.com> wrote:
>
> > Ping!
>
> I now understand the problem better.
>
> Reviewed-by: Simon Ser <contact@emersion.fr>
>
> I'll push to drm-misc-next in a few days if no-one complains. Ping me
> again if I forget.

Hi Simon,

ping! :)

thanks,
-mario
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.
  2021-02-23 17:52                       ` Mario Kleiner
@ 2021-02-23 18:31                         ` Simon Ser
  0 siblings, 0 replies; 19+ messages in thread
From: Simon Ser @ 2021-02-23 18:31 UTC (permalink / raw)
  To: Mario Kleiner; +Cc: Uma Shankar, dri-devel

Pushed, thanks!
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-02-23 18:31 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-24  4:40 [PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant Mario Kleiner
2021-01-24  8:15 ` Simon Ser
2021-01-24 20:10   ` Mario Kleiner
2021-01-24 20:24     ` Simon Ser
2021-01-24 21:04       ` Mario Kleiner
2021-01-25 12:09         ` Ville Syrjälä
2021-01-25 12:14           ` Simon Ser
2021-01-25 16:00             ` Mario Kleiner
2021-01-25 16:05               ` Simon Ser
2021-01-25 19:53                 ` Mario Kleiner
2021-02-10 21:04                   ` Mario Kleiner
2021-02-10 21:14                     ` Simon Ser
2021-02-10 22:02                       ` Mario Kleiner
2021-02-11 10:44                         ` Simon Ser
2021-02-12  5:49                           ` Mario Kleiner
2021-02-23 17:52                       ` Mario Kleiner
2021-02-23 18:31                         ` Simon Ser
2021-01-25 16:08           ` Mario Kleiner
2021-01-25 16:11             ` Simon Ser

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.