On Wed, Mar 20, 2019 at 04:18:19PM +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. > > 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. > > Signed-off-by: Uma Shankar <uma.shan...@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com> > Reviewed-by: Shashank Sharma <shashank.sha...@intel.com> > --- > drivers/gpu/drm/drm_edid.c | 56 ++++++++++++++++++++ > drivers/video/hdmi.c | 129 > +++++++++++++++++++++++++++++++++++++++++++++ > include/drm/drm_edid.h | 4 ++ > include/linux/hdmi.h | 22 ++++++++ > 4 files changed, 211 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 676569b..78c0b97 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4916,6 +4916,62 @@ 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, > + void *hdr_metadata)
I don't think this needs to be/should be a void *, you might as well us (struct hdr_static_metadata *). Looks like you do the cast in i915 before calling this function anyway. > +{ > + struct hdr_static_metadata *hdr_source_metadata; > + int err, i; > + > + 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); > + > + hdr_source_metadata = (struct hdr_static_metadata *)hdr_metadata; > + > + frame->length = sizeof(struct hdr_static_metadata); > + > + frame->eotf = hdr_source_metadata->eotf; > + frame->metadata_type = hdr_source_metadata->metadata_type; > + > + for (i = 0; i < 3; i++) { > + frame->display_primaries[i].x = > + hdr_source_metadata->display_primaries[i].x; > + frame->display_primaries[i].y = > + hdr_source_metadata->display_primaries[i].y; > + } > + > + frame->white_point.x = hdr_source_metadata->white_point.x; > + frame->white_point.y = hdr_source_metadata->white_point.y; > + > + frame->max_mastering_display_luminance = > + hdr_source_metadata->max_mastering_display_luminance; > + frame->min_mastering_display_luminance = > + hdr_source_metadata->min_mastering_display_luminance; > + > + frame->max_cll = hdr_source_metadata->max_cll; > + frame->max_fall = hdr_source_metadata->max_fall; Couldn't the infoframe definition embed the UAPI structure, making this a straight memcpy() ? > + > + hdmi_infoframe_log(KERN_CRIT, NULL, > + (union hdmi_infoframe *)frame); > + > + 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..80bb0ee 100644 > --- a/drivers/video/hdmi.c > +++ b/drivers/video/hdmi.c > @@ -650,6 +650,93 @@ 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); > + > +/** > + * 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(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_mastering_display_luminance; > + *ptr++ = frame->max_mastering_display_luminance >> 8; > + > + *ptr++ = frame->min_mastering_display_luminance; > + *ptr++ = frame->min_mastering_display_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; > +} > + > /* > * hdmi_vendor_any_infoframe_check() - check a vendor infoframe > */ > @@ -806,6 +893,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 +928,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 +1376,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_mastering_display_luminance: %d\n", > + frame->max_mastering_display_luminance); > + hdmi_log("min_mastering_display_luminance: %d\n", > + frame->min_mastering_display_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 +1498,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..973e43e 100644 > --- a/include/drm/drm_edid.h > +++ b/include/drm/drm_edid.h > @@ -370,6 +370,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, > + void *hdr_source_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 a065194..b925b24 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,32 @@ struct hdmi_avi_infoframe { > unsigned short right_bar; > }; > > +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_mastering_display_luminance; > + u16 min_mastering_display_luminance; > + u16 max_fall; > + u16 max_cll; Less confusing IMO if these are the same way around as in the spec. max_cll first, then max_fall. > + u16 min_cll; This guy isn't in the copy of I have (CTA-861-G). I'm probably out of date, but you also don't print or encode this value, so should it be dropped? Cheers, -Brian > +}; > + > 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); > > enum hdmi_spd_sdi { > HDMI_SPD_SDI_UNKNOWN, > @@ -365,6 +386,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