Re: [PATCH v2 2/2] drm: parse hf-vsdb

[Jose Abreu <Jose.Abreu@synopsys.com>]

Hi Shashank,


On 20-12-2016 13:47, Shashank Sharma wrote:
> HDMI 2.0 / CEA-861-F specs define a new CEA extension data block,
> called hdmi-forum vendor specific data block (HF-VSDB). This block
> contains information about sink's support for HDMI 2.0 compliant
> features. These features are:
>     - Deep color YUV 420 support and BPC
>     - 3D flags for
>         - OSD Displarity
>         - Dual view signaling
>         - independent view signaling
>     - SCDC support
>     - Max TMDS char rate
>     - Scrambling support
>
> This patch adds a parser function for this block, and add flags to
> indicate support for new features, in drm_display_info structure
>
> V2:
> - Addressed review comments from Thierry
> 	- remove len > 31 check
> 	- remove version check
> 	- fix duplicate values for macros of 36 and 30-bit depths
> - Added a sub-class for HDMI related information within drm_display_info
>   (Thierry, Daniel) and populated it with HF-VSDB specific info.
>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_edid.h     |  5 ++++
>  include/linux/hdmi.h       |  1 +
>  3 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index b552197..59e04fb 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3224,6 +3224,23 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
>  	return 0;
>  }
>  
> +static bool cea_db_is_hf_vsdb(const u8 *db)
> +{
> +	u8 len;
> +	int hfvsdb_id;
> +
> +	if (cea_db_tag(db) != VENDOR_BLOCK)
> +		return false;
> +
> +	len = cea_db_payload_len(db);
> +	if (len < 7)
> +		return false;
> +
> +	hfvsdb_id = db[1] | (db[2] << 8) | (db[3] << 16);
> +
> +	return hfvsdb_id == HDMI_IEEE_OUI_HFVSDB;
> +}
> +
>  static bool cea_db_is_hdmi_vsdb(const u8 *db)
>  {
>  	int hdmi_id;
> @@ -3768,6 +3785,57 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
>  }
>  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
>  
> +static void drm_parse_yuv420_deep_color_info(struct drm_connector *connector,
> +						const u8 *db)
> +{
> +	struct drm_hdmi_info *info = &connector->display_info.hdmi_info;
> +
> +	if (db[7] & DRM_EDID_YUV420_DC_48)
> +		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_48;
> +	if (db[7] & DRM_EDID_YUV420_DC_36)
> +		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_36;
> +	if (db[7] & DRM_EDID_YUV420_DC_30)
> +		info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_30;
> +
> +	if (!info->edid_yuv420_dc_modes) {
> +		DRM_DEBUG("%s: No YUV 420 deep color support in sink.\n",
> +			  connector->name);
> +		return;
> +	}
> +}
> +
> +static void
> +drm_parse_hf_vsdb(struct drm_connector *connector, const u8 *db)
> +{
> +	struct drm_display_info *info = &connector->display_info;
> +	struct drm_hdmi_info *hdmi_info = &info->hdmi_info;
> +
> +	if (db[5]) {
> +		/*
> +		 * If the sink supplies max tmds char rate in db,
> +		 * the actual max tmds rate = db[5] * 5Mhz.
> +		 */
> +		info->max_tmds_clock = db[5] * 5000;
> +		DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
> +		info->max_tmds_clock);
> +	}
> +
> +	if (db[6] & DRM_HFVSDB_SCDC_SUPPORT)
> +		hdmi_info->scdc_supported = true;
> +	if (db[6] & DRM_HFVSDB_SCDC_RR_CAP)
> +		hdmi_info->scdc_rr_cap = true;
> +	if (db[6] & DRM_HFVSDB_SCRAMBLING)
> +		hdmi_info->scrambling = true;

According to spec you should also check if scdc is supported in
order to support scrambling.

> +	if (db[6] & DRM_HFVSDB_INDEPENDENT_VIEW)
> +		hdmi_info->independent_view_3d = true;
> +	if (db[6] & DRM_HFVSDB_DUAL_VIEW)
> +		hdmi_info->dual_view_3d = true;
> +	if (db[6] & DRM_HFVSDB_3D_OSD)
> +		hdmi_info->osd_disparity_3d = true;
> +
> +	drm_parse_yuv420_deep_color_info(connector, db);
> +}
> +
>  static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>  					   const u8 *hdmi)
>  {
> @@ -3882,6 +3950,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>  
>  		if (cea_db_is_hdmi_vsdb(db))
>  			drm_parse_hdmi_vsdb_video(connector, db);
> +		if (cea_db_is_hf_vsdb(db))
> +			drm_parse_hf_vsdb(connector, db);
>  	}
>  }
>  
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 38eabf6..df606e3 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -212,6 +212,11 @@ struct detailed_timing {
>  #define DRM_EDID_HDMI_DC_30               (1 << 4)
>  #define DRM_EDID_HDMI_DC_Y444             (1 << 3)
>  
> +/* YUV 420 deep color modes */
> +#define DRM_EDID_YUV420_DC_48		  (1 << 6)
> +#define DRM_EDID_YUV420_DC_36		  (1 << 5)
> +#define DRM_EDID_YUV420_DC_30		  (1 << 4)

Hmm, I think this is wrong. According to spec DC 48 is at
position 2, DC 36 at position 1 and DC 30 at position 0.

> +
>  /* ELD Header Block */
>  #define DRM_ELD_HEADER_BLOCK_SIZE	4
>  
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index edbb4fc..2d4e9ef 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -35,6 +35,7 @@ enum hdmi_infoframe_type {
>  };
>  
>  #define HDMI_IEEE_OUI 0x000c03
> +#define HDMI_IEEE_OUI_HFVSDB 0xC45DD8

Lower case, because HDMI_IEEE_OUI is also lower case.

>  #define HDMI_INFOFRAME_HEADER_SIZE  4
>  #define HDMI_AVI_INFOFRAME_SIZE    13
>  #define HDMI_SPD_INFOFRAME_SIZE    25

Best regards,
Jose Miguel Abreu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx