Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Laurentiu Palcu <laurentiu.palcu@nxp.com>
To:     =?iso-8859-1?Q?Ville_Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
CC:     Uma Shankar <uma.shankar@intel.com>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        Daniel Vetter <daniel.vetter@ffwll.ch>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        dl-linux-imx <linux-imx@nxp.com>
Subject: Re: [EXT] Re: [PATCH] drm: fix HDR static metadata type field
 numbering
Thread-Topic: [EXT] Re: [PATCH] drm: fix HDR static metadata type field
 numbering
Thread-Index: AQHVpTDonNbf5j/+IEiOiFgD+u80BaefIQ+AgAEjT4A=
Date:   Thu, 28 Nov 2019 08:39:41 +0000
Message-ID: <20191128083940.GC10251@fsr-ub1664-121>
References: <1574865719-24490-1-git-send-email-laurentiu.palcu@nxp.com>
 <20191127151703.GJ1208@intel.com>
In-Reply-To: <20191127151703.GJ1208@intel.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="iso-8859-1"
Content-ID: <09903457459C43408EEE994B2177CD2A@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 09e49c6e-afec-4cde-c5dc-08d773de8356
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Nov 2019 08:39:41.8401
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: CS22ezZ32Q5KAusHAkiI5CbG2jO6vgP59J8gxaH8/lzDH7fiazvaXwNIffJoBmmDBjJx2nAPR3XH+JUn3KncUw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB4880
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Nov 27, 2019 at 05:17:03PM +0200, Ville Syrj=E4l=E4 wrote:
> Caution: EXT Email
>=20
> On Wed, Nov 27, 2019 at 02:42:35PM +0000, Laurentiu Palcu wrote:
> > According to CTA-861 specification, HDR static metadata data block allo=
ws a
> > sink to indicate which HDR metadata types it supports by setting the SM=
_0 to
> > SM_7 bits. Currently, only Static Metadata Type 1 is supported and this=
 is
> > indicated by setting the SM_0 bit to 1.
> >
> > However, the connector->hdr_sink_metadata.hdmi_type1.metadata_type is a=
lways
> > 0, because hdr_metadata_type() in drm_edid.c checks the wrong bit.
> >
> > This patch corrects the HDMI_STATIC_METADATA_TYPE1 bit position.
>=20
> Was confused for a while why this has even been workning, but I guess
> that's due to userspace populating the metadata infoframe blob correctly
> even if we misreported the metadata types in the parsed EDID metadata
> blob.
>=20
> Hmm. Actually on further inspection this all seems to be dead code. The
> only thing we seem to use from the parsed EDID metadata stuff is
> eotf bitmask. We check that in drm_hdmi_infoframe_set_hdr_metadata()
> but we don't check the metadata type.
>=20
> Maybe we should just nuke this EDID parsing stuff entirely? Seems
> pretty much pointless.

I've been thinking about that but we may need the rest of the fields as
well, even though they're not currently used. I'm referring to sink's
min/max luminance data. Shouldn't we also check min/max cll, besides
eotf, to make sure the source does not pass higher/lower luminance
values, than the sink supports, for optimal content rendering?

However, CTA-861 is not very clear on how a sink should behave if
the CLL values exceed the allowed range... :/ Also, if the CLL range or
the FALL values passed in the DRM infoframe exceed the sink's advertised
min/max values, I guess the sink cannot go lower/higher than it can
anyway. In which case, we don't really need the rest of the HDR static
metadata block and nuking that part should be ok.

Thanks,
laurentiu


>=20
> >
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@nxp.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 9918a6c..216e25e 100644
> > --- a/include/linux/hdmi.h
> > +++ b/include/linux/hdmi.h
> > @@ -155,7 +155,7 @@ enum hdmi_content_type {
> >  };
> >
> >  enum hdmi_metadata_type {
> > -     HDMI_STATIC_METADATA_TYPE1 =3D 1,
> > +     HDMI_STATIC_METADATA_TYPE1 =3D 0,
> >  };
> >
> >  enum hdmi_eotf {
> > --
> > 2.7.4
>=20
> --
> Ville Syrj=E4l=E4
> Intel=