Re: [igt-dev] [PATCH i-g-t 2/5] lib/igt_edid: add hdmi_vsd

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

On Tue, Jul 02, 2019 at 03:50:35PM +0300, Simon Ser wrote:
> The HDMI Vendor-Specific Data block, defined as an opaque blob in the EDID
> spec, is described in the HDMI spec.

You may want to clarify that this is the HDMI 1.4 thing, not the
HDMI 2.0 "HDMI Forum" thing.

> 
> Most of the extension fields are optional, so it doesn't translate well to a C
> struct. For now callers need to manually fill the hdmi_vsd.data field.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  lib/igt_edid.c | 40 ++++++++++++++++++++++++++++++++--------
>  lib/igt_edid.h | 49 +++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 77 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/igt_edid.c b/lib/igt_edid.c
> index cbb7eefff70d..c93d8fa8bca0 100644
> --- a/lib/igt_edid.c
> +++ b/lib/igt_edid.c
> @@ -314,6 +314,14 @@ void cea_sad_init_pcm(struct cea_sad *sad, int channels,
>  	sad->bitrate = sample_sizes;
>  }
>  
> +static void cea_vsd_set_hdmi(struct cea_vsd *vsd)

vsdb is the acronym used by the specs. I would suggest a sed job
over the whole thing to align with that. The name of the function
is a bit vague. ..._set_ieee_oui_hdmi() ?

> +{
> +	/* HDMI's IEEE Registration Identifier */
> +	vsd->ieee_oui[0] = 0x03;
> +	vsd->ieee_oui[1] = 0x0C;
> +	vsd->ieee_oui[2] = 0x00;
> +}
> +
>  /**
>   * cea_vsd_get_hdmi_default:
>   *
> @@ -321,21 +329,22 @@ void cea_sad_init_pcm(struct cea_sad *sad, int channels,
>   */
>  const struct cea_vsd *cea_vsd_get_hdmi_default(size_t *size)
>  {
> -	static char raw[sizeof(struct cea_vsd) + 4] = {0};
> +	static char raw[CEA_VSD_HDMI_MIN_SIZE + 2] = {0};

A comment explaining the 2 might be helful.

Otherwise
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	struct cea_vsd *vsd;
> +	struct hdmi_vsd *hdmi;
>  
>  	*size = sizeof(raw);
>  
>  	/* Magic incantation. Works better if you orient your screen in the
>  	 * direction of the VESA headquarters. */
>  	vsd = (struct cea_vsd *) raw;
> -	vsd->ieee_oui[0] = 0x03;
> -	vsd->ieee_oui[1] = 0x0C;
> -	vsd->ieee_oui[2] = 0x00;
> -	vsd->data[0] = 0x10;
> -	vsd->data[1] = 0x00;
> -	vsd->data[2] = 0x38;
> -	vsd->data[3] = 0x2D;
> +	cea_vsd_set_hdmi(vsd);
> +	hdmi = &vsd->data.hdmi;
> +	hdmi->src_phy_addr[0] = 0x10;
> +	hdmi->src_phy_addr[1] = 0x00;
> +	/* 2 VSD extension fields */
> +	hdmi->flags1 = 0x38;
> +	hdmi->max_tdms_clock = 0x2D;
>  
>  	return vsd;
>  }
> @@ -371,6 +380,21 @@ size_t edid_cea_data_block_set_vsd(struct edid_cea_data_block *block,
>  	return sizeof(struct edid_cea_data_block) + vsd_size;
>  }
>  
> +size_t edid_cea_data_block_set_hdmi_vsd(struct edid_cea_data_block *block,
> +					const struct hdmi_vsd *hdmi,
> +					size_t hdmi_size)
> +{
> +	char raw_vsd[CEA_VSD_HDMI_MAX_SIZE] = {0};
> +	struct cea_vsd *vsd = (struct cea_vsd *) raw_vsd;
> +
> +	assert(hdmi_size >= HDMI_VSD_MIN_SIZE && hdmi_size <= HDMI_VSD_MAX_SIZE);
> +	cea_vsd_set_hdmi(vsd);
> +	memcpy(&vsd->data.hdmi, hdmi, hdmi_size);
> +
> +	return edid_cea_data_block_set_vsd(block, vsd,
> +					   CEA_VSD_HEADER_SIZE + hdmi_size);
> +}
> +
>  size_t edid_cea_data_block_set_speaker_alloc(struct edid_cea_data_block *block,
>  					     const struct cea_speaker_alloc *speakers)
>  {
> diff --git a/lib/igt_edid.h b/lib/igt_edid.h
> index 47581bb778b0..0ac851c0d74a 100644
> --- a/lib/igt_edid.h
> +++ b/lib/igt_edid.h
> @@ -191,12 +191,50 @@ struct cea_sad {
>  	uint8_t bitrate;
>  } __attribute__((packed));
>  
> -/* Vendor Specific Data */
> -struct cea_vsd {
> -	uint8_t ieee_oui[3];
> -	char data[];
> +enum hdmi_vsd_flags1 {
> +	HDMI_VSD_DVI_DUAL = 1 << 0,
> +	HDMI_VSD_DC_Y444 = 1 << 3, /* supports YCbCr 4:4:4 */
> +	HDMI_VSD_DC_30BIT = 1 << 4, /* 30 bits per pixel */
> +	HDMI_VSD_DC_36BIT = 1 << 5, /* 36 bits per pixel */
> +	HDMI_VSD_DC_48BIT = 1 << 6, /* 48 bits per pixel */
> +	HDMI_VSD_SUPPORTS_AI = 1 << 7, /* supports ACP, ISRC1 or ISRC2 packets */
> +};
> +
> +enum hdmi_vsd_flags2 {
> +	HDMI_VSD_CNC_GRAPHICS = 1 << 0,
> +	HDMI_VSD_CNC_PHOTO = 1 << 1,
> +	HDMI_VSD_CNC_CINEMA = 1 << 2,
> +	HDMI_VSD_CNC_GAME = 1 << 3,
> +	HDMI_VSD_VIDEO_PRESENT = 1 << 5,
> +	HDMI_VSD_INTERLACED_LATENCY_PRESENT = 1 << 6,
> +	HDMI_VSD_LATENCY_PRESENT = 1 << 7,
>  };
>  
> +/* HDMI Vendor-Specific Data Block (defined in the HDMI spec) */
> +struct hdmi_vsd {
> +	uint8_t src_phy_addr[2]; /* source physical address */
> +
> +	/* Extension fields */
> +	uint8_t flags1; /* enum hdmi_vsd_flags1 */
> +	uint8_t max_tdms_clock; /* multiply by 5MHz */
> +	uint8_t flags2; /* enum hdmi_vsd_flags2 */
> +	char data[]; /* latency, misc, VIC, 3D */
> +} __attribute__((packed));
> +
> +#define HDMI_VSD_MIN_SIZE 2 /* just the source physical address */
> +#define HDMI_VSD_MAX_SIZE 28
> +#define CEA_VSD_HEADER_SIZE 3 /* IEEE OUI */
> +#define CEA_VSD_HDMI_MIN_SIZE (CEA_VSD_HEADER_SIZE + HDMI_VSD_MIN_SIZE)
> +#define CEA_VSD_HDMI_MAX_SIZE (CEA_VSD_HEADER_SIZE + HDMI_VSD_MAX_SIZE)
> +
> +/* Vendor-Specific Data */
> +struct cea_vsd {
> +	uint8_t ieee_oui[3]; /* 24-bit IEEE Registration Identifier, LSB */
> +	union {
> +		struct hdmi_vsd hdmi;
> +	} data;
> +} __attribute__((packed));
> +
>  enum cea_speaker_alloc_item {
>  	CEA_SPEAKER_FRONT_LEFT_RIGHT = 1 << 0,
>  	CEA_SPEAKER_LFE = 1 << 1,
> @@ -315,6 +353,9 @@ size_t edid_cea_data_block_set_sad(struct edid_cea_data_block *block,
>  				   const struct cea_sad *sads, size_t sads_len);
>  size_t edid_cea_data_block_set_vsd(struct edid_cea_data_block *block,
>  				   const struct cea_vsd *vsd, size_t vsd_size);
> +size_t edid_cea_data_block_set_hdmi_vsd(struct edid_cea_data_block *block,
> +					const struct hdmi_vsd *hdmi,
> +					size_t hdmi_size);
>  size_t edid_cea_data_block_set_speaker_alloc(struct edid_cea_data_block *block,
>  					     const struct cea_speaker_alloc *speakers);
>  void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
> -- 
> 2.22.0
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev