Re: [PATCH 1/2] video/hdmi: Allow "empty" HDMI infoframes

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jul 04, 2017 at 11:41:27AM +0200, Andrzej Hajda wrote:
> On 03.07.2017 21:19, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@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.
> >
> > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/video/hdmi.c | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> > index 1cf907ecded4..af0c8dad29eb 100644
> > --- a/drivers/video/hdmi.c
> > +++ b/drivers/video/hdmi.c
> > @@ -341,10 +341,6 @@ 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;
> > @@ -352,8 +348,10 @@ ssize_t hdmi_vendor_infoframe_pack(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)
> >  		frame->length = 6;
> > -	else
> > +	else if (frame->vic != 0 || frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID)
> >  		frame->length = 5;
> > +	else
> > +		frame->length = 4;
> >  
> >  	length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
> >  
> > @@ -372,14 +370,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) {
> > +		ptr[7] = 0x1 << 5;	/* video format */
> > +		ptr[8] = frame->vic;
> > +	} else {
> > +		ptr[7] = 0x0 << 5;	/* video format */
> 
> Changed order of conditionals (vic,s3d versus s3d,vic) could result in
> different output, but only in case of incorrect VSIFs, so probably harmless.

Such an infoframe would have been rejected earlier, so can't happen.

I did the reordering because I wanted the order to be the same
here as it's where the length is determined. easier to cross check that
way.

> 
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

Thanks.

> 
>  --
> Regards
> Andrzej
> 
> >  	}
> >  
> >  	hdmi_infoframe_set_checksum(buffer, length);
> 

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel