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@xxxxxxxxx>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Reviewed-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
