From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "Kazlauskas, Nicholas" <Nicholas.Kazlauskas@amd.com>
Cc: "ville.syrjala@intel.com" <ville.syrjala@intel.com>,
"jonas@kwiboo.se" <jonas@kwiboo.se>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"emil.l.velikov@gmail.com" <emil.l.velikov@gmail.com>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
Uma Shankar <uma.shankar@intel.com>,
"seanpaul@chromium.org" <seanpaul@chromium.org>,
"dcastagna@chromium.org" <dcastagna@chromium.org>,
"maarten.lankhorst@intel.com" <maarten.lankhorst@intel.com>
Subject: Re: [v9 04/13] drm: Enable HDR infoframe support
Date: Tue, 14 May 2019 15:44:39 +0300 [thread overview]
Message-ID: <20190514124439.GZ24299@intel.com> (raw)
In-Reply-To: <070c4187-42d5-8db1-973f-682ccc2f420b@amd.com>
On Tue, May 14, 2019 at 12:36:25PM +0000, Kazlauskas, Nicholas wrote:
> On 5/8/19 2:38 PM, Uma Shankar wrote:
> > [CAUTION: External Email]
> >
> > Enable Dynamic Range and Mastering Infoframe for HDR
> > content, which is defined in CEA 861.3 spec.
> >
> > The metadata will be computed based on blending
> > policy in userspace compositors and passed as a connector
> > property blob to driver. The same will be sent as infoframe
> > to panel which support HDR.
> >
> > Added the const version of infoframe for DRM metadata
> > for HDR.
> >
> > v2: Rebase and added Ville's POC changes.
> >
> > v3: No Change
> >
> > v4: Addressed Shashank's review comments and merged the
> > patch making drm infoframe function arguments as constant.
> >
> > v5: Rebase
> >
> > v6: Fixed checkpatch warnings with --strict option. Addressed
> > Shashank's review comments and added his RB.
> >
> > v7: Addressed Brian Starkey's review comments. Merged 2 patches
> > into one.
> >
> > v8: Addressed Jonas Karlman review comments.
> >
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> > ---
> > drivers/gpu/drm/drm_edid.c | 48 ++++++++++++
> > drivers/video/hdmi.c | 186 +++++++++++++++++++++++++++++++++++++++++++++
> > include/drm/drm_edid.h | 5 ++
> > include/linux/hdmi.h | 27 +++++++
> > 4 files changed, 266 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index fe2c29b..5f48965 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -4906,6 +4906,54 @@ static bool is_hdmi2_sink(struct drm_connector *connector)
> > }
> >
> > /**
> > + * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI AVI infoframe with
> > + * HDR metadata from userspace
> > + * @frame: HDMI AVI infoframe
> > + * @hdr_source_metadata: hdr_source_metadata info from userspace
> > + *
> > + * Return: 0 on success or a negative error code on failure.
> > + */
> > +int
> > +drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
> > + struct hdr_output_metadata *hdr_metadata)
hdr_metadata can be const.
> > +{
> > + int err;
> > +
> > + if (!frame || !hdr_metadata)
> > + return true;
true?
> > +
> > + err = hdmi_drm_infoframe_init(frame);
> > + if (err < 0)
> > + return err;
> > +
> > + DRM_DEBUG_KMS("type = %x\n", frame->type);
> > +
> > + frame->length = sizeof(struct hdr_metadata_infoframe);
> > +
> > + frame->eotf = hdr_metadata->hdmi_metadata_type1.eotf;
> > + frame->metadata_type = hdr_metadata->hdmi_metadata_type1.metadata_type;
> > +
> > + memcpy(&frame->display_primaries,
> > + &hdr_metadata->hdmi_metadata_type1.display_primaries, 12);
> > +
> > + memcpy(&frame->white_point,
> > + &hdr_metadata->hdmi_metadata_type1.white_point, 4);
> > +
> > + frame->max_display_mastering_luminance =
> > + hdr_metadata->hdmi_metadata_type1.max_display_mastering_luminance;
> > + frame->min_display_mastering_luminance =
> > + hdr_metadata->hdmi_metadata_type1.min_display_mastering_luminance;
> > + frame->max_fall = hdr_metadata->hdmi_metadata_type1.max_fall;
> > + frame->max_cll = hdr_metadata->hdmi_metadata_type1.max_cll;
> > +
> > + hdmi_infoframe_log(KERN_CRIT, NULL,
> > + (union hdmi_infoframe *)frame);
>
> This shouldn't really be KERN_CRIT for debug output. Maybe KERN_INFO or
> KERN_DEBUG or just drop the log altogether since it can't actually tag
> it to DRM or the driver itself in this function.
Yeah, this shouldn't be here. It's up to the driver to dump it when it
wants to.
>
> Nicholas Kazlauskas
>
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata);
> > +
> > +/**
> > * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
> > * data from a DRM display mode
> > * @frame: HDMI AVI infoframe
> > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> > index 799ae49..717ed7fb 100644
> > --- a/drivers/video/hdmi.c
> > +++ b/drivers/video/hdmi.c
> > @@ -650,6 +650,146 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
> > return 0;
> > }
> >
> > +/**
> > + * hdmi_drm_infoframe_init() - initialize an HDMI Dynaminc Range and
> > + * mastering infoframe
> > + * @frame: HDMI DRM infoframe
> > + *
> > + * Returns 0 on success or a negative error code on failure.
> > + */
> > +int hdmi_drm_infoframe_init(struct hdmi_drm_infoframe *frame)
> > +{
> > + memset(frame, 0, sizeof(*frame));
> > +
> > + frame->type = HDMI_INFOFRAME_TYPE_DRM;
> > + frame->version = 1;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(hdmi_drm_infoframe_init);
> > +
> > +static int hdmi_drm_infoframe_check_only(const struct hdmi_drm_infoframe *frame)
> > +{
> > + if (frame->type != HDMI_INFOFRAME_TYPE_DRM ||
> > + frame->version != 1)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * hdmi_drm_infoframe_check() - check a HDMI DRM infoframe
> > + * @frame: HDMI DRM infoframe
> > + *
> > + * Validates that the infoframe is consistent.
> > + * Returns 0 on success or a negative error code on failure.
> > + */
> > +int hdmi_drm_infoframe_check(struct hdmi_drm_infoframe *frame)
> > +{
> > + return hdmi_drm_infoframe_check_only(frame);
> > +}
> > +EXPORT_SYMBOL(hdmi_drm_infoframe_check);
> > +
> > +/**
> > + * hdmi_drm_infoframe_pack() - write HDMI DRM infoframe to binary buffer
> > + * @frame: HDMI DRM infoframe
> > + * @buffer: destination buffer
> > + * @size: size of buffer
> > + *
> > + * Packs the information contained in the @frame structure into a binary
> > + * representation that can be written into the corresponding controller
> > + * registers. Also computes the checksum as required by section 5.3.5 of
> > + * the HDMI 1.4 specification.
> > + *
> > + * Returns the number of bytes packed into the binary buffer or a negative
> > + * error code on failure.
> > + */
> > +ssize_t hdmi_drm_infoframe_pack_only(const struct hdmi_drm_infoframe *frame,
> > + void *buffer, size_t size)
> > +{
> > + u8 *ptr = buffer;
> > + size_t length;
> > + int i;
> > +
> > + length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
> > +
> > + if (size < length)
> > + return -ENOSPC;
> > +
> > + memset(buffer, 0, size);
> > +
> > + ptr[0] = frame->type;
> > + ptr[1] = frame->version;
> > + ptr[2] = frame->length;
> > + ptr[3] = 0; /* checksum */
> > +
> > + /* start infoframe payload */
> > + ptr += HDMI_INFOFRAME_HEADER_SIZE;
> > +
> > + *ptr++ = frame->eotf;
> > + *ptr++ = frame->metadata_type;
> > +
> > + for (i = 0; i < 3; i++) {
> > + *ptr++ = frame->display_primaries[i].x;
> > + *ptr++ = frame->display_primaries[i].x >> 8;
> > + *ptr++ = frame->display_primaries[i].y;
> > + *ptr++ = frame->display_primaries[i].y >> 8;
> > + }
> > +
> > + *ptr++ = frame->white_point.x;
> > + *ptr++ = frame->white_point.x >> 8;
> > +
> > + *ptr++ = frame->white_point.y;
> > + *ptr++ = frame->white_point.y >> 8;
> > +
> > + *ptr++ = frame->max_display_mastering_luminance;
> > + *ptr++ = frame->max_display_mastering_luminance >> 8;
> > +
> > + *ptr++ = frame->min_display_mastering_luminance;
> > + *ptr++ = frame->min_display_mastering_luminance >> 8;
> > +
> > + *ptr++ = frame->max_cll;
> > + *ptr++ = frame->max_cll >> 8;
> > +
> > + *ptr++ = frame->max_fall;
> > + *ptr++ = frame->max_fall >> 8;
> > +
> > + hdmi_infoframe_set_checksum(buffer, length);
> > +
> > + return length;
> > +}
> > +EXPORT_SYMBOL(hdmi_drm_infoframe_pack_only);
> > +
> > +/**
> > + * hdmi_drm_infoframe_pack() - check a HDMI DRM infoframe,
> > + * and write it to binary buffer
> > + * @frame: HDMI DRM infoframe
> > + * @buffer: destination buffer
> > + * @size: size of buffer
> > + *
> > + * Validates that the infoframe is consistent and updates derived fields
> > + * (eg. length) based on other fields, after which it packs the information
> > + * contained in the @frame structure into a binary representation that
> > + * can be written into the corresponding controller registers. This function
> > + * also computes the checksum as required by section 5.3.5 of the HDMI 1.4
> > + * specification.
> > + *
> > + * Returns the number of bytes packed into the binary buffer or a negative
> > + * error code on failure.
> > + */
> > +ssize_t hdmi_drm_infoframe_pack(struct hdmi_drm_infoframe *frame,
> > + void *buffer, size_t size)
> > +{
> > + int ret;
> > +
> > + ret = hdmi_drm_infoframe_check(frame);
> > + if (ret)
> > + return ret;
> > +
> > + return hdmi_drm_infoframe_pack_only(frame, buffer, size);
> > +}
> > +EXPORT_SYMBOL(hdmi_drm_infoframe_pack);
> > +
> > /*
> > * hdmi_vendor_any_infoframe_check() - check a vendor infoframe
> > */
> > @@ -758,6 +898,10 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
> > length = hdmi_avi_infoframe_pack_only(&frame->avi,
> > buffer, size);
> > break;
> > + case HDMI_INFOFRAME_TYPE_DRM:
> > + length = hdmi_drm_infoframe_pack_only(&frame->drm,
> > + buffer, size);
> > + break;
> > case HDMI_INFOFRAME_TYPE_SPD:
> > length = hdmi_spd_infoframe_pack_only(&frame->spd,
> > buffer, size);
> > @@ -806,6 +950,9 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
> > case HDMI_INFOFRAME_TYPE_AVI:
> > length = hdmi_avi_infoframe_pack(&frame->avi, buffer, size);
> > break;
> > + case HDMI_INFOFRAME_TYPE_DRM:
> > + length = hdmi_drm_infoframe_pack(&frame->drm, buffer, size);
> > + break;
> > case HDMI_INFOFRAME_TYPE_SPD:
> > length = hdmi_spd_infoframe_pack(&frame->spd, buffer, size);
> > break;
> > @@ -838,6 +985,8 @@ static const char *hdmi_infoframe_type_get_name(enum hdmi_infoframe_type type)
> > return "Source Product Description (SPD)";
> > case HDMI_INFOFRAME_TYPE_AUDIO:
> > return "Audio";
> > + case HDMI_INFOFRAME_TYPE_DRM:
> > + return "Dynamic Range and Mastering";
> > }
> > return "Reserved";
> > }
> > @@ -1284,6 +1433,40 @@ static void hdmi_audio_infoframe_log(const char *level,
> > frame->downmix_inhibit ? "Yes" : "No");
> > }
> >
> > +/**
> > + * hdmi_drm_infoframe_log() - log info of HDMI DRM infoframe
> > + * @level: logging level
> > + * @dev: device
> > + * @frame: HDMI DRM infoframe
> > + */
> > +static void hdmi_drm_infoframe_log(const char *level,
> > + struct device *dev,
> > + const struct hdmi_drm_infoframe *frame)
> > +{
> > + int i;
> > +
> > + hdmi_infoframe_log_header(level, dev,
> > + (struct hdmi_any_infoframe *)frame);
> > + hdmi_log("length: %d\n", frame->length);
> > + hdmi_log("metadata type: %d\n", frame->metadata_type);
> > + hdmi_log("eotf: %d\n", frame->eotf);
> > + for (i = 0; i < 3; i++) {
> > + hdmi_log("x[%d]: %d\n", i, frame->display_primaries[i].x);
> > + hdmi_log("y[%d]: %d\n", i, frame->display_primaries[i].y);
> > + }
> > +
> > + hdmi_log("white point x: %d\n", frame->white_point.x);
> > + hdmi_log("white point y: %d\n", frame->white_point.y);
> > +
> > + hdmi_log("max_display_mastering_luminance: %d\n",
> > + frame->max_display_mastering_luminance);
> > + hdmi_log("min_display_mastering_luminance: %d\n",
> > + frame->min_display_mastering_luminance);
> > +
> > + hdmi_log("max_cll: %d\n", frame->max_cll);
> > + hdmi_log("max_fall: %d\n", frame->max_fall);
> > +}
> > +
> > static const char *
> > hdmi_3d_structure_get_name(enum hdmi_3d_structure s3d_struct)
> > {
> > @@ -1372,6 +1555,9 @@ void hdmi_infoframe_log(const char *level,
> > case HDMI_INFOFRAME_TYPE_VENDOR:
> > hdmi_vendor_any_infoframe_log(level, dev, &frame->vendor);
> > break;
> > + case HDMI_INFOFRAME_TYPE_DRM:
> > + hdmi_drm_infoframe_log(level, dev, &frame->drm);
> > + break;
> > }
> > }
> > EXPORT_SYMBOL(hdmi_infoframe_log);
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index 9d3b5b9..d81c5c2 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -25,6 +25,7 @@
> >
> > #include <linux/types.h>
> > #include <linux/hdmi.h>
> > +#include <drm/drm_mode.h>
> >
> > struct drm_device;
> > struct i2c_adapter;
> > @@ -370,6 +371,10 @@ int drm_av_sync_delay(struct drm_connector *connector,
> > const struct drm_display_mode *mode,
> > enum hdmi_quantization_range rgb_quant_range);
> >
> > +int
> > +drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
> > + struct hdr_output_metadata *hdr_metadata);
> > +
> > /**
> > * drm_eld_mnl - Get ELD monitor name length in bytes.
> > * @eld: pointer to an eld memory structure with mnl set
> > diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> > index 6780476..7edafcf 100644
> > --- a/include/linux/hdmi.h
> > +++ b/include/linux/hdmi.h
> > @@ -47,6 +47,7 @@ enum hdmi_infoframe_type {
> > HDMI_INFOFRAME_TYPE_AVI = 0x82,
> > HDMI_INFOFRAME_TYPE_SPD = 0x83,
> > HDMI_INFOFRAME_TYPE_AUDIO = 0x84,
> > + HDMI_INFOFRAME_TYPE_DRM = 0x87,
> > };
> >
> > #define HDMI_IEEE_OUI 0x000c03
> > @@ -185,12 +186,37 @@ struct hdmi_avi_infoframe {
> > unsigned short right_bar;
> > };
> >
> > +/* DRM Infoframe as per CTA 861.G spec */
> > +struct hdmi_drm_infoframe {
> > + enum hdmi_infoframe_type type;
> > + unsigned char version;
> > + unsigned char length;
> > + enum hdmi_eotf eotf;
> > + enum hdmi_metadata_type metadata_type;
> > + struct {
> > + u16 x, y;
> > + } display_primaries[3];
> > + struct {
> > + u16 x, y;
> > + } white_point;
> > + u16 max_display_mastering_luminance;
> > + u16 min_display_mastering_luminance;
> > + u16 max_cll;
> > + u16 max_fall;
> > +};
> > +
> > int hdmi_avi_infoframe_init(struct hdmi_avi_infoframe *frame);
> > ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer,
> > size_t size);
> > ssize_t hdmi_avi_infoframe_pack_only(const struct hdmi_avi_infoframe *frame,
> > void *buffer, size_t size);
> > int hdmi_avi_infoframe_check(struct hdmi_avi_infoframe *frame);
> > +int hdmi_drm_infoframe_init(struct hdmi_drm_infoframe *frame);
> > +ssize_t hdmi_drm_infoframe_pack(struct hdmi_drm_infoframe *frame, void *buffer,
> > + size_t size);
> > +ssize_t hdmi_drm_infoframe_pack_only(const struct hdmi_drm_infoframe *frame,
> > + void *buffer, size_t size);
> > +int hdmi_drm_infoframe_check(struct hdmi_drm_infoframe *frame);
> >
> > enum hdmi_spd_sdi {
> > HDMI_SPD_SDI_UNKNOWN,
> > @@ -381,6 +407,7 @@ ssize_t hdmi_vendor_infoframe_pack_only(const struct hdmi_vendor_infoframe *fram
> > struct hdmi_spd_infoframe spd;
> > union hdmi_vendor_any_infoframe vendor;
> > struct hdmi_audio_infoframe audio;
> > + struct hdmi_drm_infoframe drm;
> > };
> >
> > ssize_t hdmi_infoframe_pack(union hdmi_infoframe *frame, void *buffer,
> > --
> > 1.9.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-05-14 12:44 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-08 18:38 [v9 00/13] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
2019-05-08 18:34 ` ✗ Fi.CI.CHECKPATCH: warning for Add HDR Metadata Parsing and handling in DRM layer (rev9) Patchwork
2019-05-08 18:38 ` [v9 01/13] drm: Add HDR source metadata property Uma Shankar
2019-05-08 18:38 ` [v9 02/13] drm: Add reference counting on HDR metadata blob Uma Shankar
2019-05-08 18:38 ` [v9 03/13] drm: Parse HDR metadata info from EDID Uma Shankar
2019-05-13 19:19 ` Ville Syrjälä
2019-05-14 9:49 ` Shankar, Uma
2019-05-14 12:40 ` Ville Syrjälä
2019-05-14 13:26 ` Shankar, Uma
2019-05-08 18:38 ` [v9 04/13] drm: Enable HDR infoframe support Uma Shankar
2019-05-14 12:36 ` Kazlauskas, Nicholas
2019-05-14 12:39 ` Shankar, Uma
2019-05-14 12:44 ` Ville Syrjälä [this message]
2019-05-08 18:38 ` [v9 05/13] drm/i915: Attach HDR metadata property to connector Uma Shankar
2019-05-08 18:38 ` [v9 06/13] drm/i915: Write HDR infoframe and send to panel Uma Shankar
2019-05-13 19:35 ` Ville Syrjälä
2019-05-14 10:22 ` Shankar, Uma
2019-05-08 18:38 ` [v9 07/13] drm: Add HLG EOTF Uma Shankar
2019-05-08 18:38 ` [v9 08/13] drm/i915: Enable infoframes on GLK+ for HDR Uma Shankar
2019-05-13 19:37 ` Ville Syrjälä
2019-05-08 18:38 ` [v9 09/13] drm/i915:Enabled Modeset when HDR Infoframe changes Uma Shankar
2019-05-08 18:38 ` [v9 10/13] drm/i915: Set Infoframe for non modeset case for HDR Uma Shankar
2019-05-13 19:39 ` Ville Syrjälä
2019-05-14 16:22 ` Shankar, Uma
2019-05-08 18:38 ` [v9 11/13] drm/i915: Added DRM Infoframe handling for BYT/CHT Uma Shankar
2019-05-08 18:38 ` [v9 12/13] video/hdmi: Add Unpack function for DRM infoframe Uma Shankar
2019-05-13 19:49 ` Ville Syrjälä
2019-05-14 16:18 ` Shankar, Uma
2019-05-08 18:38 ` [v9 13/13] drm/i915: Add state readout " Uma Shankar
2019-05-13 19:50 ` Ville Syrjälä
2019-05-08 18:54 ` ✓ Fi.CI.BAT: success for Add HDR Metadata Parsing and handling in DRM layer (rev9) Patchwork
2019-05-08 22:13 ` ✓ Fi.CI.IGT: " Patchwork
2019-05-12 20:12 ` [v9 00/13] Add HDR Metadata Parsing and handling in DRM layer Jonas Karlman
2019-05-13 15:48 ` Shankar, Uma
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190514124439.GZ24299@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=Nicholas.Kazlauskas@amd.com \
--cc=dcastagna@chromium.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=emil.l.velikov@gmail.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jonas@kwiboo.se \
--cc=maarten.lankhorst@intel.com \
--cc=seanpaul@chromium.org \
--cc=uma.shankar@intel.com \
--cc=ville.syrjala@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.