Re: [PATCH] drm: edid: HDMI 2.0 HF-VSDB block parsing

From: Ville Syrjälä
Date: Fri Aug 12 2016 - 09:46:58 EST


On Wed, Aug 10, 2016 at 04:29:21PM +0100, Jose Abreu wrote:
> Adds parsing for HDMI 2.0 'HDMI Forum Vendor
> Specific Data Block'. This block is present in
> some HDMI 2.0 EDID's and gives information about
> scrambling support, SCDC, 3D Views, and others.
>
> Parsed parameters are stored in drm_connector
> structure.
>
> Signed-off-by: Jose Abreu <joabreu@xxxxxxxxxxxx>
> Cc: Carlos Palminha <palminha@xxxxxxxxxxxx>
> Cc: David Airlie <airlied@xxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx

Thierry not in cc? Is this based on his SCDC work [1] or did we have
multiple people implementing the same thing, partially at least?

[1] https://lists.freedesktop.org/archives/dri-devel/2015-September/090929.html

> ---
> drivers/gpu/drm/drm_edid.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
> include/drm/drm_crtc.h | 12 ++++++++++++
> include/drm/drm_edid.h | 5 +++++
> include/linux/hdmi.h | 1 +
> 4 files changed, 67 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7df26d4..bcacf11 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3160,6 +3160,21 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db)
> return hdmi_id == HDMI_IEEE_OUI;
> }
>
> +static bool cea_db_is_hdmi_hf_vsdb(const u8 *db)

IIRC I asked Thierry to spell out the hdmi forum to avoid having this
look like alphabet soup. But now that I've read the spec, I do see it
being referred as hdmi-hf there as well, so I suppose it's fine.

> +{
> + int hdmi_id;
> +
> + if (cea_db_tag(db) != VENDOR_BLOCK)
> + return false;
> +
> + if (cea_db_payload_len(db) < 7)
> + return false;
> +
> + hdmi_id = db[1] | (db[2] << 8) | (db[3] << 16);
> +
> + return hdmi_id == HDMI_IEEE_OUI_HF;
> +}
> +
> #define for_each_cea_db(cea, i, start, end) \
> for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
>
> @@ -3287,6 +3302,37 @@ parse_hdmi_vsdb(struct drm_connector *connector, const u8 *db)
> }
>
> static void
> +parse_hdmi_hf_vsdb(struct drm_connector *connector, const u8 *db)
> +{
> + u8 len = cea_db_payload_len(db);
> +
> + if (len < 7)
> + return;
> +
> + if (db[4] != 1)
> + return; /* invalid version */
> +
> + connector->max_tmds_char = db[5] * 5;

We already have the max_tmds_clock. Also I was just trying move some of
the junk out from the connector into display_info [2].

[2] https://lists.freedesktop.org/archives/dri-devel/2016-August/114634.html

> + connector->scdc_present = db[6] & (1 << 7);
> + connector->rr_capable = db[6] & (1 << 6);
> + connector->flags_3d = db[6] & 0x7;
> + connector->supports_scramble = connector->scdc_present &&
> + (db[6] & (1 << 3));

Do we have actual users for all these? I don't like adding stuff into
core structures just for the purposes of immediately printing it out.

For the stereo flags, I guess someone would have to figure put how they
relate to the 3d flags in the other vsdb, and how to convert to drm mode
flags.

> +
> + DRM_DEBUG_KMS("HDMI v2: max TMDS char %d, "
> + "scdc %s, "
> + "rr %s, "
> + "3D flags 0x%x, "
> + "scramble %s\n",
> + connector->max_tmds_char,
> + connector->scdc_present ? "available" : "not available",
> + connector->rr_capable ? "capable" : "not capable",
> + connector->flags_3d,
> + connector->supports_scramble ?
> + "supported" : "not supported");
> +}
> +
> +static void
> monitor_name(struct detailed_timing *t, void *data)
> {
> if (t->data.other_data.type == EDID_DETAIL_MONITOR_NAME)
> @@ -3403,6 +3449,9 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> /* HDMI Vendor-Specific Data Block */
> if (cea_db_is_hdmi_vsdb(db))
> parse_hdmi_vsdb(connector, db);
> + /* HDMI Forum Vendor-Specific Data Block */
> + else if (cea_db_is_hdmi_hf_vsdb(db))
> + parse_hdmi_hf_vsdb(connector, db);

Wasn't there some priority scheme between these two? Or was it just
about the infoframes? I'd have to dig through the spec to find out.

> break;
> default:
> break;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index b618b50..eca57d2 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1279,6 +1279,11 @@ struct drm_encoder {
> * @audio_latency: audio latency info from ELD, if found
> * @null_edid_counter: track sinks that give us all zeros for the EDID
> * @bad_edid_counter: track sinks that give us an EDID with invalid checksum
> + * @max_tmds_char: indicates the maximum TMDS Character Rate supported
> + * @scdc_present: when set the sink supports SCDC functionality
> + * @rr_capable: when set the sink is capable of initiating an SCDC read request
> + * @supports_scramble: when set the sink supports less than 340Mcsc scrambling
> + * @flags_3d: 3D view(s) supported by the sink, see drm_edid.h (DRM_EDID_3D_*)
> * @edid_corrupt: indicates whether the last read EDID was corrupt
> * @debugfs_entry: debugfs directory for this connector
> * @state: current atomic state for this connector
> @@ -1377,6 +1382,13 @@ struct drm_connector {
> int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
> unsigned bad_edid_counter;
>
> + /* EDID bits HDMI 2.0 */
> + int max_tmds_char; /* in Mcsc */
> + bool scdc_present;
> + bool rr_capable;
> + bool supports_scramble;
> + int flags_3d;
> +
> /* Flag for raw EDID header corruption - used in Displayport
> * compliance testing - * Displayport Link CTS Core 1.2 rev1.1 4.2.2.6
> */
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 919933d..35d39f0 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -266,6 +266,11 @@ struct detailed_timing {
>
> #define DRM_ELD_CEA_SAD(mnl, sad) (20 + (mnl) + 3 * (sad))
>
> +/* HDMI 2.0 */
> +#define DRM_EDID_3D_INDEPENDENT_VIEW (1 << 2)
> +#define DRM_EDID_3D_DUAL_VIEW (1 << 1)
> +#define DRM_EDID_3D_OSD_DISPARITY (1 << 0)
> +
> struct edid {
> u8 header[8];
> /* Vendor & product info */
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index e974420..6e23409 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_HF 0xc45dd8
> #define HDMI_INFOFRAME_HEADER_SIZE 4
> #define HDMI_AVI_INFOFRAME_SIZE 13
> #define HDMI_SPD_INFOFRAME_SIZE 25
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Ville Syrjälä
Intel OTC