[Intel-gfx] [PATCH 01/10] video/hdmi: Allow "empty" HDMI infoframes

[Sharma, Shashank]

Regards

Shashank


On 11/16/2017 9:46 PM, Ville Syrjälä wrote:
> On Thu, Nov 16, 2017 at 08:06:18PM +0530, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 11/13/2017 10:34 PM, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>
>>> HDMI 2.0 Appendix F suggest that we should keep sending the infoframe
>>> when switching from 3D to 2D mode, even if the infoframe isn't strictly
>>> necessary (ie. not needed to transmit the VIC or stereo information).
>>> This is a workaround against some sinks that fail to realize that they
>>> should switch from 3D to 2D mode when the source stop transmitting
>>> the infoframe.
>>>
>>> v2: Handle unpack() as well
>>>       Pull the length calculation into a helper
>>>
>>> Cc: Shashank Sharma <shashank.sharma at intel.com>
>>> Cc: Andrzej Hajda <a.hajda at samsung.com>
>>> Cc: Thierry Reding <thierry.reding at gmail.com>
>>> Cc: Hans Verkuil <hans.verkuil at cisco.com>
>>> Cc: linux-media at vger.kernel.org
>>> Reviewed-by: Andrzej Hajda <a.hajda at samsung.com> #v1
>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> ---
>>>    drivers/video/hdmi.c | 51 +++++++++++++++++++++++++++++++--------------------
>>>    1 file changed, 31 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
>>> index 1cf907ecded4..111a0ab6280a 100644
>>> --- a/drivers/video/hdmi.c
>>> +++ b/drivers/video/hdmi.c
>>> @@ -321,6 +321,17 @@ int hdmi_vendor_infoframe_init(struct hdmi_vendor_infoframe *frame)
>>>    }
>>>    EXPORT_SYMBOL(hdmi_vendor_infoframe_init);
>>>    
>>> +static int hdmi_vendor_infoframe_length(const struct hdmi_vendor_infoframe *frame)
>>> +{
>>> +	/* for side by side (half) we also need to provide 3D_Ext_Data */
>>> +	if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
>>> +		return 6;
>>> +	else if (frame->vic != 0 || frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID)
>>> +		return 5;
>>> +	else
>>> +		return 4;
>>> +}
>>> +
>>>    /**
>>>     * hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary buffer
>>>     * @frame: HDMI infoframe
>>> @@ -341,19 +352,11 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
>>>    	u8 *ptr = buffer;
>>>    	size_t length;
>>>    
>>> -	/* empty info frame */
>>> -	if (frame->vic == 0 && frame->s3d_struct == HDMI_3D_STRUCTURE_INVALID)
>>> -		return -EINVAL;
>>> -
>>>    	/* only one of those can be supplied */
>>>    	if (frame->vic != 0 && frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID)
>>>    		return -EINVAL;
>>>    
>>> -	/* for side by side (half) we also need to provide 3D_Ext_Data */
>>> -	if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
>>> -		frame->length = 6;
>>> -	else
>>> -		frame->length = 5;
>>> +	frame->length = hdmi_vendor_infoframe_length(frame);
>>>    
>>>    	length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
>>>    
>>> @@ -372,14 +375,16 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
>>>    	ptr[5] = 0x0c;
>>>    	ptr[6] = 0x00;
>>>    
>>> -	if (frame->vic) {
>>> -		ptr[7] = 0x1 << 5;	/* video format */
>>> -		ptr[8] = frame->vic;
>>> -	} else {
>>> +	if (frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID) {
>>>    		ptr[7] = 0x2 << 5;	/* video format */
>>>    		ptr[8] = (frame->s3d_struct & 0xf) << 4;
>>>    		if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
>>>    			ptr[9] = (frame->s3d_ext_data & 0xf) << 4;
>>> +	} else if (frame->vic) {
>> Please correct me if I dint understand this properly, but for a HDMI 2.0
>> sink + 3D transmission, wouldn't I be sending
>> HDMI 2.0 VIC = 94 as well as some valid s3d flag (like side by side) ?
> That vic will be in the AVI infoframe. Here we're concerned about the
> VIC in the HDMI vendor infoframe.
Yep, I was thinking if there is any way we can cross check that there is 
a valid HDMI 2 vic
before we do anything here, but seems like a long shot.

Reviewed-by: Shashank Sharma <shashank.sharma at intel.com>
>> - Shashank
>>> +		ptr[7] = 0x1 << 5;	/* video format */
>>> +		ptr[8] = frame->vic;
>>> +	} else {
>>> +		ptr[7] = 0x0 << 5;	/* video format */
>>>    	}
>>>    
>>>    	hdmi_infoframe_set_checksum(buffer, length);
>>> @@ -1165,7 +1170,7 @@ hdmi_vendor_any_infoframe_unpack(union hdmi_vendor_any_infoframe *frame,
>>>    
>>>    	if (ptr[0] != HDMI_INFOFRAME_TYPE_VENDOR ||
>>>    	    ptr[1] != 1 ||
>>> -	    (ptr[2] != 5 && ptr[2] != 6))
>>> +	    (ptr[2] != 4 && ptr[2] != 5 && ptr[2] != 6))
>>>    		return -EINVAL;
>>>    
>>>    	length = ptr[2];
>>> @@ -1193,16 +1198,22 @@ hdmi_vendor_any_infoframe_unpack(union hdmi_vendor_any_infoframe *frame,
>>>    
>>>    	hvf->length = length;
>>>    
>>> -	if (hdmi_video_format == 0x1) {
>>> -		hvf->vic = ptr[4];
>>> -	} else if (hdmi_video_format == 0x2) {
>>> +	if (hdmi_video_format == 0x2) {
>>> +		if (length != 5 && length != 6)
>>> +			return -EINVAL;
>>>    		hvf->s3d_struct = ptr[4] >> 4;
>>>    		if (hvf->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF) {
>>> -			if (length == 6)
>>> -				hvf->s3d_ext_data = ptr[5] >> 4;
>>> -			else
>>> +			if (length != 6)
>>>    				return -EINVAL;
>>> +			hvf->s3d_ext_data = ptr[5] >> 4;
>>>    		}
>>> +	} else if (hdmi_video_format == 0x1) {
>>> +		if (length != 5)
>>> +			return -EINVAL;
>>> +		hvf->vic = ptr[4];
>>> +	} else {
>>> +		if (length != 4)
>>> +			return -EINVAL;
>>>    	}
>>>    
>>>    	return 0;