[Intel-gfx] [PATCH 08/20] drm: set output colorspace in AVI infoframe

[Sharma, Shashank]

Regards

Shashank


On 7/12/2017 10:47 PM, Ville Syrjälä wrote:
> On Mon, Jul 10, 2017 at 04:48:36PM +0530, Shashank Sharma wrote:
>> A source must set output colorspace information in AVI
>> infoframes, so that the sink can decode upcoming frames
>> accordingly.
>>
>> This patch adds a function to add the output colorspace
>> information in the AVI infoframes.
>>
>> V2: Rebase
>> V3: Rebase
>> V4: Rebase
>> V5: Rebase
>> V6: Made patch independent of HDMI output type.
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c | 29 +++++++++++++++++++++++++++++
>>   include/drm/drm_edid.h     |  5 +++++
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 944a28f..cede86e 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4796,6 +4796,35 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>>   EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
>>   
>>   /**
>> + * drm_hdmi_avi_infoframe_set_colorspace - fill an HDMI AVI infoframe with
>> + * colorspace data of the output type
>> + *
>> + * @frame: HDMI AVI infoframe
>> + * @mode: DRM display mode
>> + * @hdmi_output: HDMI output colorspace
>> + *
>> + * Return: 0 on success or a negative error code on failure.
>> + */
>> +int
>> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame,
>> +				      const struct drm_display_mode *mode,
>> +				      enum hdmi_colorspace colorspace)
>> +{
>> +	if (colorspace > HDMI_COLORSPACE_YUV420 ||
>> +		colorspace < HDMI_COLORSPACE_RGB) {
>> +		DRM_ERROR("Invalid color space type\n");
>> +		return -EINVAL;
>> +	}
> Seems overly defensive. I'd say that if someone insists on writing
> buggy code just let them do it.
:) yep can be done, you know, its a new implementation, don't want to 
create unnecessary noise so being
a bit defensive :)
>> +
>> +	frame->colorspace = colorspace;
>> +	if (colorspace == HDMI_COLORSPACE_YUV420)
>> +		frame->pixel_repeat = 0;
> Most VICs don't allow pixel repeat in 444/etc. either, and we don't
> protect against that. So this looks like pretty pointless check in
> this form.
>
> So IMO just drop this entire patch and just assign frame->colorspace in
> the driver.
Actually YCBCR420 section of spec specifically calls out for not 
allowing repetition, also, when I tested this on a
HDMI 2.0 analyzer, if was giving a AVI IF failure on pixel_repeat not 0, 
so IMHO it would be a good idea to keep
this and get the tests passing.

- Shashank
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_set_colorspace);
>> +
>> +/**
>>    * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe
>>    *                                        quantization range information
>>    * @frame: HDMI AVI infoframe
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>> index aa58146..b79e0cb 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -332,6 +332,7 @@ struct cea_sad {
>>   struct drm_encoder;
>>   struct drm_connector;
>>   struct drm_display_mode;
>> +enum drm_hdmi_output_type;
>>   
>>   void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid);
>>   int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
>> @@ -354,6 +355,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>>   					 const struct drm_display_mode *mode,
>>   					 bool is_hdmi2_sink);
>>   int
>> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame,
>> +					 const struct drm_display_mode *mode,
>> +					 enum hdmi_colorspace colorspace);
>> +int
>>   drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
>>   					    const struct drm_display_mode *mode);
>>   void
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel