>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] >Sent: Thursday, May 16, 2019 12:45 AM >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; >maarten.lankhorst@xxxxxxxxxxxxxxx; Sharma, Shashank ><shashank.sharma@xxxxxxxxx>; emil.l.velikov@xxxxxxxxx; brian.starkey@xxxxxxx; >dcastagna@xxxxxxxxxxxx; seanpaul@xxxxxxxxxxxx; Roper, Matthew D ><matthew.d.roper@xxxxxxxxx>; jonas@xxxxxxxxx >Subject: Re: [v10 04/12] drm: Enable HDR infoframe support > >On Tue, May 14, 2019 at 11:06:26PM +0530, Uma Shankar wrote: >> 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. >> >> v9: 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 | 43 +++++++++++ >> drivers/video/hdmi.c | 187 >+++++++++++++++++++++++++++++++++++++++++++++ >> include/drm/drm_edid.h | 5 ++ >> include/linux/hdmi.h | 27 +++++++ >> 4 files changed, 262 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index 2e0b5be..73b7905 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -4903,6 +4903,49 @@ static bool is_hdmi2_sink(struct drm_connector >> *connector) } >> >> /** >> + * drm_hdmi_infoframe_set_hdr_metadata() - fill an HDMI DRM infoframe with >> + * HDR metadata from userspace >> + * @frame: HDMI DRM infoframe >> + * @hdr_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, >> + const struct hdr_output_metadata >*hdr_metadata) { >> + int err; >> + >> + if (!frame || !hdr_metadata) >> + return -EINVAL; >> + >> + err = hdmi_drm_infoframe_init(frame); >> + if (err < 0) >> + return err; >> + >> + DRM_DEBUG_KMS("type = %x\n", frame->type); > >This debug seems pointless. Ok, will drop this. >> + >> + 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); > >sizeof() can be used here. Sure. >Could also slap on a BUILD_BUG_ON() to make sure both are the same size. Ok, will addd it. > >> + >> + memcpy(&frame->white_point, >> + &hdr_metadata->hdmi_metadata_type1.white_point, 4); > >ditto. Sure. >> + >> + 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; >> + >> + 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..c5ecd16 100644 >> --- a/drivers/video/hdmi.c >> +++ b/drivers/video/hdmi.c >> @@ -650,6 +650,147 @@ 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; >> + frame->length = HDMI_DRM_INFOFRAME_SIZE; >> + >> + 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) > >Missing the length check. > >> + 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_only() - 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 +899,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 +951,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 +986,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 +1434,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 +1556,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..4ba56f5 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, >> + const 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 > >-- >Ville Syrjälä >Intel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
