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

Ville Syrjälä
2013-08-14T11:14:42Z

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