RE: [v9 04/13] drm: Enable HDR infoframe support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>-----Original Message-----
>From: Kazlauskas, Nicholas [mailto:Nicholas.Kazlauskas@xxxxxxx]
>Sent: Tuesday, May 14, 2019 6:06 PM
>To: Shankar, Uma <uma.shankar@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-
>devel@xxxxxxxxxxxxxxxxxxxxx
>Cc: dcastagna@xxxxxxxxxxxx; jonas@xxxxxxxxx; emil.l.velikov@xxxxxxxxx;
>seanpaul@xxxxxxxxxxxx; Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Lankhorst, Maarten
><maarten.lankhorst@xxxxxxxxx>
>Subject: Re: [v9 04/13] drm: Enable HDR infoframe support
>
>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) {
>> +       int err;
>> +
>> +       if (!frame || !hdr_metadata)
>> +               return 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.

I added this for debug and missed to drop it, it's really not needed. Will drop this log in the next version.

Regards,
Uma Shankar


>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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux