Re: [v16 3/4] drm: Add colorspace info to AVI Infoframe

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

 




>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Ville
>Syrjälä
>Sent: Tuesday, February 19, 2019 1:37 AM
>To: Shankar, Uma <uma.shankar@xxxxxxxxx>
>Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Lankhorst,
>Maarten <maarten.lankhorst@xxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx
>Subject: Re:  [v16 3/4] drm: Add colorspace info to AVI Infoframe
>
>On Wed, Feb 13, 2019 at 12:35:10PM +0530, Uma Shankar wrote:
>> This adds colorspace information to HDMI AVI infoframe.
>> A helper function is added to program the same.
>>
>> v2: Moved this to drm core instead of i915 driver.
>>
>> v3: Exported the helper function.
>>
>> v4: Added separate HDMI specific macro as per CTA spec.
>> This is separate from user exposed enum values. This is as per Ville's
>> suggestion.
>>
>> v5: Appended BT709 and SMPTE 170M with YCC information as per Ville's
>> review comment to be clear and not to be confused with RGB.
>>
>> v6: Added bit wise macro for various fields of colorimetry for easier
>> understanding and review as per Ville's comments. Moved the same out
>> of header file to avoid any namespace issues.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx>
>> ---
>>  drivers/gpu/drm/drm_edid.c | 66
>++++++++++++++++++++++++++++++++++++++++++++++
>>  include/drm/drm_edid.h     |  6 +++++
>>  2 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 990b190..64816c0 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4924,6 +4924,72 @@ static bool is_hdmi2_sink(struct drm_connector
>> *connector)  }
>> EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
>>
>> +/* HDMI Colorspace Spec Definitions */
>> +#define FULL_COLORIMETRY_MASK		0x1FF
>> +#define NORMAL_COLORIMETRY_MASK		0x3
>> +#define EXTENDED_COLORIMETRY_MASK	0x7
>> +#define EXTENDED_ACE_COLORIMETRY_MASK	0xF
>> +
>> +#define C(x) ((x) << 0)
>> +#define EC(x) ((x) << 2)
>> +#define ACE(x) ((x) << 5)
>> +
>> +#define HDMI_COLORIMETRY_NO_DATA		0x0
>> +#define HDMI_COLORIMETRY_SMPTE_170M_YCC		(C(1) | EC(0) |
>ACE(0))
>> +#define HDMI_COLORIMETRY_BT709_YCC		(C(2) | EC(0) | ACE(0))
>> +#define HDMI_COLORIMETRY_XVYCC_601		(C(3) | EC(0) | ACE(0))
>> +#define HDMI_COLORIMETRY_XVYCC_709		(C(3) | EC(1) | ACE(0))
>> +#define HDMI_COLORIMETRY_SYCC_601		(C(3) | EC(2) | ACE(0))
>> +#define HDMI_COLORIMETRY_OPYCC_601		(C(3) | EC(3) | ACE(0))
>> +#define HDMI_COLORIMETRY_OPRGB			(C(3) | EC(4) |
>ACE(0))
>> +#define HDMI_COLORIMETRY_BT2020_CYCC		(C(3) | EC(5) | ACE(0))
>> +#define HDMI_COLORIMETRY_BT2020_RGB		(C(3) | EC(6) | ACE(0))
>> +#define HDMI_COLORIMETRY_BT2020_YCC		(C(3) | EC(6) | ACE(0))
>> +#define HDMI_COLORIMETRY_DCI_P3_RGB_D65		(C(3) | EC(7) |
>ACE(0))
>> +#define HDMI_COLORIMETRY_DCI_P3_RGB_THEATER	(C(3) | EC(7) |
>ACE(1))
>> +
>> +static const u32 hdmi_colorimetry_val[] = {
>> +	[DRM_MODE_COLORIMETRY_NO_DATA] =
>HDMI_COLORIMETRY_NO_DATA,
>> +	[DRM_MODE_COLORIMETRY_SMPTE_170M_YCC] =
>HDMI_COLORIMETRY_SMPTE_170M_YCC,
>> +	[DRM_MODE_COLORIMETRY_BT709_YCC] =
>HDMI_COLORIMETRY_BT709_YCC,
>> +	[DRM_MODE_COLORIMETRY_XVYCC_601] =
>HDMI_COLORIMETRY_XVYCC_601,
>> +	[DRM_MODE_COLORIMETRY_XVYCC_709] =
>HDMI_COLORIMETRY_XVYCC_709,
>> +	[DRM_MODE_COLORIMETRY_SYCC_601] =
>HDMI_COLORIMETRY_SYCC_601,
>> +	[DRM_MODE_COLORIMETRY_OPYCC_601] =
>HDMI_COLORIMETRY_OPYCC_601,
>> +	[DRM_MODE_COLORIMETRY_OPRGB] = HDMI_COLORIMETRY_OPRGB,
>> +	[DRM_MODE_COLORIMETRY_BT2020_CYCC] =
>HDMI_COLORIMETRY_BT2020_CYCC,
>> +	[DRM_MODE_COLORIMETRY_BT2020_RGB] =
>HDMI_COLORIMETRY_BT2020_RGB,
>> +	[DRM_MODE_COLORIMETRY_BT2020_YCC] =
>HDMI_COLORIMETRY_BT2020_YCC, };
>
>Probably best to
>
>#undef C
>#undef EC
>#undef ACE
>
>here to avoid accidents.

Sure will add that. So with this fixed and DP stuff dropped, can I mark your RB on all the remaining 2 patches
(DP patch will get dropped). If you confirm will resend with your RB for merge.

Thanks & Regards,
Uma Shankar

>With that
>Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>


>> +
>> +/**
>> + * drm_hdmi_avi_infoframe_colorspace() - fill the HDMI AVI infoframe
>> + *                                       colorspace information
>> + * @frame: HDMI AVI infoframe
>> + * @conn_state: connector state
>> + */
>> +void
>> +drm_hdmi_avi_infoframe_colorspace(struct hdmi_avi_infoframe *frame,
>> +				  const struct drm_connector_state *conn_state) {
>> +	u32 colorimetry_val;
>> +	u32 colorimetry_index = conn_state->colorspace &
>> +FULL_COLORIMETRY_MASK;
>> +
>> +	if (colorimetry_index >= ARRAY_SIZE(hdmi_colorimetry_val))
>> +		colorimetry_val = HDMI_COLORIMETRY_NO_DATA;
>> +	else
>> +		colorimetry_val = hdmi_colorimetry_val[colorimetry_index];
>> +
>> +	frame->colorimetry = colorimetry_val & NORMAL_COLORIMETRY_MASK;
>> +	/*
>> +	 * ToDo: Extend it for ACE formats as well. Modify the infoframe
>> +	 * structure and extend it in drivers/video/hdmi
>> +	 */
>> +	frame->extended_colorimetry = (colorimetry_val >> 2) &
>> +					EXTENDED_COLORIMETRY_MASK;
>> +}
>> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_colorspace);
>> +
>>  /**
>>   * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe
>>   *                                        quantization range information
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index
>> 8dc1a08..9d3b5b9 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -331,6 +331,7 @@ struct cea_sad {
>>
>>  struct drm_encoder;
>>  struct drm_connector;
>> +struct drm_connector_state;
>>  struct drm_display_mode;
>>
>>  int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads); @@
>> -358,6 +359,11 @@ int drm_av_sync_delay(struct drm_connector
>> *connector,  drm_hdmi_vendor_infoframe_from_display_mode(struct
>hdmi_vendor_infoframe *frame,
>>  					    struct drm_connector *connector,
>>  					    const struct drm_display_mode *mode);
>> +
>> +void
>> +drm_hdmi_avi_infoframe_colorspace(struct hdmi_avi_infoframe *frame,
>> +				  const struct drm_connector_state *conn_state);
>> +
>>  void
>>  drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
>>  				   struct drm_connector *connector,
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel
>_______________________________________________
>dri-devel mailing list
>dri-devel@xxxxxxxxxxxxxxxxxxxxx
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux