From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH v2 2/3] drm/edid: Implement SCDC support detection Date: Mon, 5 Dec 2016 12:11:46 +0100 Message-ID: <20161205111146.GC19891@ulmo.ba.sec> References: <20161202192415.16110-1-thierry.reding@gmail.com> <20161202192415.16110-2-thierry.reding@gmail.com> <20161205075743.GE18069@ulmo.ba.sec> <20161205081627.eadgg2y4epyrhp4a@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0578254033==" Return-path: Received: from mail-pf0-x244.google.com (mail-pf0-x244.google.com [IPv6:2607:f8b0:400e:c00::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 222496E1E3 for ; Mon, 5 Dec 2016 11:11:51 +0000 (UTC) Received: by mail-pf0-x244.google.com with SMTP id 144so16954219pfv.0 for ; Mon, 05 Dec 2016 03:11:51 -0800 (PST) In-Reply-To: <20161205081627.eadgg2y4epyrhp4a@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Jose Abreu , "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org --===============0578254033== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Md/poaVZ8hnGTzuv" Content-Disposition: inline --Md/poaVZ8hnGTzuv Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Dec 05, 2016 at 09:16:27AM +0100, Daniel Vetter wrote: > On Mon, Dec 05, 2016 at 08:57:43AM +0100, Thierry Reding wrote: > > On Sat, Dec 03, 2016 at 04:35:24AM +0000, Sharma, Shashank wrote: > > > Hi Thierry,=20 > > >=20 > > > If you can please have a look on this patch, I had written one to par= se HF-VSDB, which was covering SCDC detection too.=20 > > > https://patchwork.kernel.org/patch/9452259/=20 > >=20 > > I think there had been pushback before about caching capabilities from > > EDID, so from that point of view my patch is more inline with existing > > EDID parsing API. >=20 > Hm, where was that pushback? We do have a bit a mess between explicitly > parsing stuff (e.g. eld) and stuffing parsed data into drm_display_info. You did object to a very similar patch some time ago that did a similar thing for DPCD stuff. And also Villa had commented on an earlier patch =66rom Jose about concerns of bloating core structures: https://patchwork.freedesktop.org/patch/104806/ > I think long-term stuffing it into drm_display_info is probably better, > since then we only have 1 interaction point between the probe code and the > atomic_check code. That should be useful for eventually fixing the lack of > locking between the two, if I ever get around to that ;-) I don't really have objections to caching the results of parsing, it's what I had proposed and what seemed most natural back when I was working on the DPCD helpers. But if we now agree that this is the preferred way to do things, then we should at least agree that it applies to all areas for the sake of consistency. Also, it might be worth looking into improving the structures, and maybe adding new ones to order things more conveniently or at least group them in some logical way. In my opinion some of our data structures are becoming somewhat... unwieldy. > > Other than that the patches are mostly equivalent, except yours parses > > more information than just the SCDC bits. >=20 > So merge patch 1 from your series + Shashank's parsing patch? Everyone > agrees and can you pls cross-r-b stamp so I can apply it all? I think I spotted a mistake in Shashank's parsing patch. Let me take another look. If we can agree on a common way forward on how to deal with this kind of static data, I have no objections to caching data for the duration of a hotplug period. Thierry > > > -----Original Message----- > > > From: Thierry Reding [mailto:thierry.reding@gmail.com]=20 > > > Sent: Saturday, December 3, 2016 12:54 AM > > > To: dri-devel@lists.freedesktop.org > > > Cc: Jani Nikula ; Sharma, Shashank ; Ville Syrj=C3=A4l=C3=A4 ; Jose Abreu > > > Subject: [PATCH v2 2/3] drm/edid: Implement SCDC support detection > > >=20 > > > From: Thierry Reding > > >=20 > > > Sinks compliant with the HDMI 2.0 specification may support SCDC, a m= echanism for the source and the sink to communicate. Sinks advertise suppor= t for this feature in the HDMI Forum Vendor Specific Data Block as defined = in the HDMI 2.0 specification, section 10.4 "Status and Control Data Channe= l". Implement a small helper that find the HF-VSDB and parses it to check i= f the sink supports SCDC. > > >=20 > > > Signed-off-by: Thierry Reding > > > --- > > > Changes in v2: > > > - rename internal function to cea_db_is_hdmi_forum_vsdb() for more > > > clarity (Ville Syrj=C3=A4l=C3=A4) > > >=20 > > > drivers/gpu/drm/drm_edid.c | 49 ++++++++++++++++++++++++++++++++++++= ++++++++++ > > > include/drm/drm_edid.h | 1 + > > > include/linux/hdmi.h | 1 + > > > 3 files changed, 51 insertions(+) > > >=20 > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c = index 336be31ff3de..369961597ee5 100644 > > > --- a/drivers/gpu/drm/drm_edid.c > > > +++ b/drivers/gpu/drm/drm_edid.c > > > @@ -3238,6 +3238,21 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db) > > > return hdmi_id =3D=3D HDMI_IEEE_OUI; > > > } > > > =20 > > > +static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) { > > > + unsigned int oui; > > > + > > > + if (cea_db_tag(db) !=3D VENDOR_BLOCK) > > > + return false; > > > + > > > + if (cea_db_payload_len(db) < 7) > > > + return false; > > > + > > > + oui =3D db[3] << 16 | db[2] << 8 | db[1]; > > > + > > > + return oui =3D=3D HDMI_FORUM_IEEE_OUI; > > > +} > > > + > > > #define for_each_cea_db(cea, i, start, end) \ > > > for ((i) =3D (start); (i) < (end) && (i) + cea_db_payload_len(&(cea= )[(i)]) < (end); (i) +=3D cea_db_payload_len(&(cea)[(i)]) + 1) > > > =20 > > > @@ -3687,6 +3702,40 @@ bool drm_detect_hdmi_monitor(struct edid *edid= ) EXPORT_SYMBOL(drm_detect_hdmi_monitor); > > > =20 > > > /** > > > + * drm_detect_hdmi_scdc - detect whether an HDMI sink supports SCDC > > > + * @edid: sink EDID information > > > + * > > > + * Parse the CEA extension according to CEA-861-B to find an HF-VSDB= as > > > + * defined in HDMI 2.0, section 10.3.2 "HDMI Forum Vendor Specific D= ata > > > + * Block" and checks if the SCDC_Present bit (bit 7 of byte 6) is se= t. > > > + * > > > + * Returns: > > > + * True if the sink supports SCDC, false otherwise. > > > + */ > > > +bool drm_detect_hdmi_scdc(struct edid *edid) { > > > + unsigned int start, end, i; > > > + const u8 *cea; > > > + > > > + cea =3D drm_find_cea_extension(edid); > > > + if (!cea) > > > + return false; > > > + > > > + if (cea_db_offsets(cea, &start, &end)) > > > + return false; > > > + > > > + for_each_cea_db(cea, i, start, end) { > > > + if (cea_db_is_hdmi_forum_vsdb(&cea[i])) { > > > + if (cea[i + 6] & 0x80) > > > + return true; > > > + } > > > + } > > > + > > > + return false; > > > +} > > > +EXPORT_SYMBOL(drm_detect_hdmi_scdc); > > > + > > > +/** > > > * drm_detect_monitor_audio - check monitor audio capability > > > * @edid: EDID block to scan > > > * > > > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 38= eabf65f19d..7ea7e90846d8 100644 > > > --- a/include/drm/drm_edid.h > > > +++ b/include/drm/drm_edid.h > > > @@ -440,6 +440,7 @@ int drm_add_edid_modes(struct drm_connector *conn= ector, struct edid *edid); > > > u8 drm_match_cea_mode(const struct drm_display_mode *to_match); enu= m hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code); bool = drm_detect_hdmi_monitor(struct edid *edid); > > > +bool drm_detect_hdmi_scdc(struct edid *edid); > > > bool drm_detect_monitor_audio(struct edid *edid); bool drm_rgb_quan= t_range_selectable(struct edid *edid); int drm_add_modes_noedid(struct drm= _connector *connector, diff --git a/include/linux/hdmi.h b/include/linux/hd= mi.h index edbb4fc674ed..d271ff23984f 100644 > > > --- a/include/linux/hdmi.h > > > +++ b/include/linux/hdmi.h > > > @@ -35,6 +35,7 @@ enum hdmi_infoframe_type { }; > > > =20 > > > #define HDMI_IEEE_OUI 0x000c03 > > > +#define HDMI_FORUM_IEEE_OUI 0xc45dd8 > > > #define HDMI_INFOFRAME_HEADER_SIZE 4 > > > #define HDMI_AVI_INFOFRAME_SIZE 13 > > > #define HDMI_SPD_INFOFRAME_SIZE 25 > > > -- > > > 2.10.2 > > >=20 >=20 >=20 >=20 > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel >=20 >=20 > --=20 > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch --Md/poaVZ8hnGTzuv Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAABCAAGBQJYRUtwAAoJEN0jrNd/PrOhRd0P/2Xt9Q0aazelCHkpQPa05dG8 DAFKrkB76F/FehoprGSZ+uPBiO9W+6tELAaG7Iv67qoJ0jBFtCz+mz03to+Rkstb fLPkOUBn8opeRnXk/8SPAcsUrfyIl1XxGJuFizFDmKQLEVZR20zAgFTchWZ3JOm4 OZGfqLZCvspWzu8ocIu0QxH4Ekqd64u7Jus+ZM1NCwdx20v7wxz0tQWd+5QGQWA4 33lWX9iJpGMldaSJ6N8CETN5Qb3C7ktlhvAwqIdd0DQ48HDEq6W8qrZc08ONAoH8 C18dCFtvu1i2NfVdeUdJ3VWVWJdTjU3mO7gMJv13dyKa5s9xT6qxcQpr34M2UdEw 7PjUyVDIGfWbYXkawAhTQkjTuEYHF2Yx8B4MvjnKk6OQyrhx+ORdCAaHFa5FTsQ/ un+7/zCM21qaB+Pn7BXdg0ZnFYC9Y2/lnPkSL7B4Z5e6EO/f8ZS9wIgZ8BbdGthC RqIkh5fR84QdE5UbELJY4CvEuj6aihrl/kgUlSZ3+s4mwEqL2u2qGyFYQ06v0bNZ WcZxi5Hn1jpHoNm7QJIDLdPmtZ/q1XxXRPd9m2ziZIIuyBIuIMFQq0Jyq1j21DIl toX2bTc7+Zq5wTOxAiw2Bwb+G4r96ERPaa9L8C2qv+8+0ZKvMLGkBgvPgLx9bKeS w7DXgO2khb8vZcFyPkrL =GT47 -----END PGP SIGNATURE----- --Md/poaVZ8hnGTzuv-- --===============0578254033== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0578254033==--