Re: [PATCH 07/12] video/hdmi: Introduce helpers for the HDMI vendor specific infoframe

From: Thierry Reding <thierry.reding@gmail.com>
To: Damien Lespiau <damien.lespiau@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 07/12] video/hdmi: Introduce helpers for the HDMI vendor specific infoframe
Date: Thu, 15 Aug 2013 16:59:04 +0200	[thread overview]
Message-ID: <20130815145904.GC16951@ulmo> (raw)
In-Reply-To: <1376500755-30227-8-git-send-email-damien.lespiau@intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 1746 bytes --]

On Wed, Aug 14, 2013 at 06:19:10PM +0100, Damien Lespiau wrote:
[...]
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index ac84215..59c4748 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -286,6 +286,94 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
>  EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
>  
>  /**
> + * hdmi_hdmi_infoframe_init() - initialize an HDMI vendor infoframe
> + * @frame: HDMI vendor infoframe
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)

The hdmi_hdmi_ prefix is weird. Can't we come up with a better prefix?
You refer to it as "HDMI vendor infoframe" in the comments, yet we
already have struct hdmi_vendor_infoframe. Perhaps hdmi_3d_infoframe or
hdmi_vendor_3d_infoframe would be better choices?

> +{
> +	memset(frame, 0, sizeof(*frame));
> +
> +	frame->type = HDMI_INFOFRAME_TYPE_VENDOR;
> +	frame->version = 1;
> +
> +	/* 0 is a valid value for s3d_struct, so we use a special "not set"
> +	 * value */

Nit: The block comment style is inconsistent again.

> +/**
> + * hdmi_hdmi_infoframe_pack() - write a HDMI vendor infoframe to binary buffer
> + * @frame: HDMI 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.

I need to dig up that version of the specification. This infoframe
doesn't seem to exist in 1.3.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel