All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Shankar, Uma" <uma.shankar@intel.com>
To: "Sharma, Shashank" <shashank.sharma@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Cc: "Syrjala, Ville" <ville.syrjala@intel.com>,
	"Lankhorst, Maarten" <maarten.lankhorst@intel.com>
Subject: Re: [v2 08/14] drm: Enable HDR infoframe support
Date: Tue, 8 Jan 2019 06:53:47 +0000	[thread overview]
Message-ID: <E7C9878FBA1C6D42A1CA3F62AEB6945F81E9B986@BGSMSX104.gar.corp.intel.com> (raw)
In-Reply-To: <c01c48c1-6863-4003-c88f-5670fe756cf5@intel.com>



>-----Original Message-----
>From: Sharma, Shashank
>Sent: Friday, December 21, 2018 2:11 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville
><ville.syrjala@intel.com>; Brian.Starkey@arm.com
>Subject: Re: [v2 08/14] drm: Enable HDR infoframe support
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, 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.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c |  58 ++++++++++++++++++++
>>   drivers/video/hdmi.c       | 129
>+++++++++++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_edid.h     |   4 ++
>>   include/linux/hdmi.h       |  22 ++++++++
>>   4 files changed, 213 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 344d8c1..5a7fc9b 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4916,6 +4916,64 @@ void drm_set_preferred_mode(struct
>drm_connector *connector,
>>   EXPORT_SYMBOL(drm_set_preferred_mode);
>>
>>   /**
>> + * 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)
>> +{
>> +	struct hdr_static_metadata *hdr_source_metadata;
>> +	int err, i;
>> +
>> +	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);
>> +
>> +	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;
>> +
>> +	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..0937c8c 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 & 0xff;
>As ptr is a u8 ptr, & 0xff is not required, it will take lower 8 bits only.

I feel keeping the mask will make it clearer and avoid any ambiguity.
I can update and drop this if you suggest that way.

Regards,
Uma Shankar

>> +		*ptr++ = frame->display_primaries[i].x >> 8;
>> +		*ptr++ = frame->display_primaries[i].y & 0xff;
>Same here and below all the places.
>> +		*ptr++ = frame->display_primaries[i].y >> 8;
>> +	}
>> +
>> +	*ptr++ = frame->white_point.x & 0xff;
>> +	*ptr++ = frame->white_point.x >> 8;
>> +
>> +	*ptr++ = frame->white_point.y & 0xff;
>> +	*ptr++ = frame->white_point.y >> 8;
>> +
>> +	*ptr++ = frame->max_mastering_display_luminance & 0xff;
>> +	*ptr++ = frame->max_mastering_display_luminance >> 8;
>> +
>> +	*ptr++ = frame->min_mastering_display_luminance & 0xff;
>> +	*ptr++ = frame->min_mastering_display_luminance >> 8;
>> +
>> +	*ptr++ = frame->max_cll & 0xff;
>> +	*ptr++ = frame->max_cll >> 8;
>> +
>> +	*ptr++ = frame->max_fall & 0xff;
>> +	*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,
>> +                                  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
>> e3c4048..0511607 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -365,6 +365,10 @@ int drm_av_sync_delay(struct drm_connector
>*connector,
>>   				   bool rgb_quant_range_selectable,
>>   				   bool is_hdmi2_sink);
>>
>> +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 bed3e08..ce00e1e
>> 100644
>> --- a/include/linux/hdmi.h
>> +++ b/include/linux/hdmi.h
>> @@ -32,6 +32,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
>> @@ -170,12 +171,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 {
>> +		uint16_t x, y;
>> +	} display_primaries[3];
>> +	struct {
>> +		uint16_t x, y;
>> +	} white_point;
>> +	uint16_t max_mastering_display_luminance;
>> +	uint16_t min_mastering_display_luminance;
>> +	uint16_t max_fall;
>> +	uint16_t max_cll;
>> +	uint16_t min_cll;
>> +};
>> +
>>   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,
>> @@ -350,6 +371,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,

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-01-08  6:53 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-11 20:38 [v2 00/14] Add HDR Metadata Parsing and handling in DRM layer Uma Shankar
2018-12-11 20:19 ` ✗ Fi.CI.BAT: failure for Add HDR Metadata Parsing and handling in DRM layer (rev2) Patchwork
2018-12-11 20:38 ` [v2 01/14] drm: Add HDR source metadata property Uma Shankar
2018-12-20 17:50   ` Sharma, Shashank
2019-01-08  5:29     ` Shankar, Uma
2018-12-11 20:38 ` [v2 02/14] drm: Add CEA extended tag blocks and HDR bitfield macros Uma Shankar
2018-12-20 18:07   ` Sharma, Shashank
2019-01-08  5:35     ` Shankar, Uma
2018-12-11 20:38 ` [v2 03/14] drm: Parse HDR metadata info from EDID Uma Shankar
2018-12-20 18:17   ` Sharma, Shashank
2019-01-08  5:40     ` Shankar, Uma
2019-01-08  5:56       ` Sharma, Shashank
2019-01-08  5:59         ` Shankar, Uma
2018-12-11 20:38 ` [v2 04/14] drm: Parse Colorimetry data block " Uma Shankar
2018-12-20 18:23   ` Sharma, Shashank
2019-01-08  6:40     ` Shankar, Uma
2019-01-08  6:56       ` Sharma, Shashank
2019-01-08  7:12         ` Shankar, Uma
2018-12-11 20:38 ` [v2 05/14] drm/i915: Attach HDR metadata property to connector Uma Shankar
2018-12-20 18:25   ` Sharma, Shashank
2019-01-08  6:42     ` Shankar, Uma
2018-12-11 20:38 ` [v2 06/14] drm: Add HDR capability field to plane structure Uma Shankar
2018-12-11 20:38 ` [v2 07/14] drm: Implement HDR source metadata set and get property handling Uma Shankar
2018-12-20 18:33   ` Sharma, Shashank
2019-01-08  6:48     ` Shankar, Uma
2018-12-11 20:38 ` [v2 08/14] drm: Enable HDR infoframe support Uma Shankar
2018-12-21  8:40   ` Sharma, Shashank
2019-01-08  6:53     ` Shankar, Uma [this message]
2018-12-11 20:38 ` [v2 09/14] drm/i915: Write HDR infoframe and send to panel Uma Shankar
2018-12-21  8:43   ` Sharma, Shashank
2019-01-08  6:55     ` Shankar, Uma
2018-12-11 20:38 ` [v2 10/14] drm/i915: [DO NOT MERGE] hack for glk board outputs Uma Shankar
2018-12-11 20:38 ` [v2 11/14] drm/i915: Add HLG EOTF Uma Shankar
2018-12-21  8:47   ` Sharma, Shashank
2019-01-08  6:56     ` Shankar, Uma
2018-12-11 20:38 ` [v2 12/14] drm/i915: Enable infoframes on GLK+ for HDR Uma Shankar
2018-12-21  8:53   ` Sharma, Shashank
2018-12-11 20:38 ` [v2 13/14] drm/i915:Enabled Modeset when HDR Infoframe changes Uma Shankar
2018-12-11 20:38 ` [v2 14/14] drivers/video: Constantify function argument for HDMI infoframe log Uma Shankar
2018-12-21  8:58   ` Sharma, Shashank
2019-01-08  7:29     ` 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=E7C9878FBA1C6D42A1CA3F62AEB6945F81E9B986@BGSMSX104.gar.corp.intel.com \
    --to=uma.shankar@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=maarten.lankhorst@intel.com \
    --cc=shashank.sharma@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.