Re: [PATCH 3/6] drm/edid: detect SCDC support in HF-VSDB

[Thierry Reding <thierry.reding@gmail.com>]

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

On Wed, Feb 01, 2017 at 06:14:38PM +0530, Shashank Sharma wrote:
> This patch does following:
> - Adds a new structure (drm_hdmi_info) in drm_display_info.
>   This structure will be used to save and indicate if sink
>   supports advance HDMI 2.0 features

"advanced"

> - Checks the HF-VSDB block for presence of SCDC, and marks it
>   in hdmi_info structure.

"drm_hdmi_info structure"?

> - If SCDC is present, checks if sink is capable of generating
>   scdc read request, and marks it in hdmi_info structure.

"SCDC" to be consistent and because it's an abbreviation.

> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c  | 14 ++++++++++++++
>  include/drm/drm_connector.h | 26 ++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 96d3e47..37902e5 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3802,6 +3802,18 @@ enum hdmi_quantization_range
>  }
>  EXPORT_SYMBOL(drm_default_rgb_quant_range);
>  
> +static void drm_detect_hdmi_scdc(struct drm_connector *connector,
> +				 const u8 *hf_vsdb)
> +{
> +	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi_info;
> +
> +	if (hf_vsdb[6] & 0x80) {
> +		hdmi->scdc_supported = true;
> +		if (hf_vsdb[6] & 0x40)
> +			hdmi->scdc_rr = true;
> +	}
> +}
> +
>  static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>  					   const u8 *hdmi)
>  {
> @@ -3916,6 +3928,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_hdmi_forum_vsdb(db))
> +			drm_detect_hdmi_scdc(connector, db);
>  	}
>  }
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index e5e1edd..2435598 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -87,6 +87,27 @@ enum subpixel_order {
>  	SubPixelVerticalRGB,
>  	SubPixelVerticalBGR,
>  	SubPixelNone,
> +
> +};
> +
> +/**
> + * struct drm_hdmi_info - runtime data about the connected sink

Maybe "connected HDMI sink"?

> + *
> + * Describes if a given hdmi display supports advance HDMI 2.0 featutes.

"HDMI", "advanced", "features"

> + * This information is available in CEA-861-F extension blocks (like
> + * HF-VSDB)

This should be terminated by a full-stop.

> + * For sinks which provide an EDID this can be filled out by calling
> + * drm_add_edid_modes().

And maybe make this sentence start right after the one above rather than
breaking it to the next line. I'm not sure how useful this line is. Most
driver will be calling drm_add_edid_modes() anyway, but the above makes
it sound like drm_add_edid_modes() is something you have to explicitly
call to get these fields parsed.

> + */
> +struct drm_hdmi_info {
> +	/**
> +	 * @scdc_supported: status control & data channel present.
> +	 */
> +	bool scdc_supported;
> +	/**
> +	 * @scdc_rr: sink is capable of generating scdc read request.
> +	 */
> +	bool scdc_rr;
>  };
>  
>  /**
> @@ -188,6 +209,11 @@ struct drm_display_info {
>  	 * @cea_rev: CEA revision of the HDMI sink.
>  	 */
>  	u8 cea_rev;
> +
> +	/**
> +	 * @hdmi_info: advance features of a HDMI sink.
> +	 */
> +	struct drm_hdmi_info hdmi_info;

I think we can safely drop the _info suffix on the field name. It's
already inside a structure that carries this suffix.

Other than that:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

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