Re: [Intel-gfx] [PATCH 10/12] video/hdmi: Hook the HDMI vendor infoframe with the generic _pack()

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

On Wed, Aug 14, 2013 at 12:17:26AM +0100, Damien Lespiau wrote:
> With this last bit, hdmi_infoframe_pack() is now able to pack any
> infoframe we support.
> 
> At the same time, because it's impractical to make two commits out of
> this, we get rid of the version that encourages the open coding of the
> vendor infoframe packing. We can do so because the only user of this API
> has been ported in:
> 
>   Author: Damien Lespiau <damien.lespiau@intel.com>
>   Date:   Mon Aug 12 18:08:37 2013 +0100
> 
>       gpu: host1x: Port the HDMI vendor infoframe code the common helpers
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/video/hdmi.c | 45 +++++++++------------------------------------
>  include/linux/hdmi.h | 24 ++++++++++++------------
>  2 files changed, 21 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index 2059f7b..073f005 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -300,6 +300,7 @@ int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)
>  	frame->length = 5; /* we can hardcode the size for now as we don't
>  			    support neither 3D_Ext_Data nor 3D_Metadata_* */
>  
> +	frame->oui = HDMI_IDENTIFIER;
>  	/* 0 is a valid value for s3d_struct, so we use a special "not set"
>  	 * value */
>  	frame->s3d_struct = HDMI_3D_STRUCTURE_INVALID;
> @@ -367,46 +368,18 @@ ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
>  }
>  EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
>  
> -/**
> - * hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
> - *                                buffer
> - * @frame: HDMI vendor 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.
> +/*
> + * hdmi_vendor_infoframe_pack() - write a vendor infoframe to binary buffer
>   */
> -ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
> -				   void *buffer, size_t size)
> +static ssize_t hdmi_vendor_infoframe_pack(union hdmi_vendor_infoframe *frame,
> +					  void *buffer, size_t size)
>  {
> -	u8 *ptr = buffer;
> -	size_t length;
> -
> -	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 */
> -
> -	memcpy(&ptr[HDMI_INFOFRAME_HEADER_SIZE], frame->data, frame->length);
> -
> -	hdmi_infoframe_checksum(buffer, length);
> +	/* we only know about HDMI vendor infoframes */
> +	if (frame->any.oui != HDMI_IDENTIFIER)
> +		return -EINVAL;
>  
> -	return length;
> +	return hdmi_hdmi_infoframe_pack(&frame->hdmi, buffer, size);
>  }
> -EXPORT_SYMBOL(hdmi_vendor_infoframe_pack);
>  
>  /**
>   * hdmi_infoframe_pack() - write a HDMI infoframe to binary buffer
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 53bbf0d..5c572e9 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -225,16 +225,6 @@ int hdmi_audio_infoframe_init(struct hdmi_audio_infoframe *frame);
>  ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
>  				  void *buffer, size_t size);
>  
> -struct hdmi_vendor_infoframe {
> -	enum hdmi_infoframe_type type;
> -	unsigned char version;
> -	unsigned char length;
> -	u8 data[27];
> -};
> -
> -ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
> -				   void *buffer, size_t size);
> -
>  enum hdmi_3d_structure {
>  	HDMI_3D_STRUCTURE_INVALID = -1,
>  	HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
> @@ -251,6 +241,7 @@ struct hdmi_hdmi_infoframe {
>  	enum hdmi_infoframe_type type;
>  	unsigned char version;
>  	unsigned char length;
> +	int oui;

unsigned perhaps? Same deal below in the 'any' struct.

Doesn't really matter I guess, so w/ or w/o that change:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	u8 vic;
>  	enum hdmi_3d_structure s3d_struct;
>  };
> @@ -259,12 +250,21 @@ int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame);
>  ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
>  				 void *buffer, size_t size);
>  
> +union hdmi_vendor_infoframe {
> +	struct {
> +		enum hdmi_infoframe_type type;
> +		unsigned char version;
> +		unsigned char length;
> +		int oui;
> +	} any;
> +	struct hdmi_hdmi_infoframe hdmi;
> +};
> +
>  union hdmi_infoframe {
>  	struct hdmi_any_infoframe any;
>  	struct hdmi_avi_infoframe avi;
>  	struct hdmi_spd_infoframe spd;
> -	struct hdmi_vendor_infoframe vendor;
> -	struct hdmi_hdmi_infoframe hdmi;
> +	union hdmi_vendor_infoframe vendor;
>  	struct hdmi_audio_infoframe audio;
>  };
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC