From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 3/6] drm/edid: detect SCDC support in HF-VSDB Date: Wed, 1 Feb 2017 17:10:02 +0100 Message-ID: <20170201161002.GB18725@ulmo.ba.sec> References: <1485953081-7630-1-git-send-email-shashank.sharma@intel.com> <1485953081-7630-4-git-send-email-shashank.sharma@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1403287810==" Return-path: In-Reply-To: <1485953081-7630-4-git-send-email-shashank.sharma@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Shashank Sharma Cc: jose.abreu@synopsys.com, =daniel.vetter@intel.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1403287810== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="rS8CxjVDS/+yyDmU" Content-Disposition: inline --rS8CxjVDS/+yyDmU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > Signed-off-by: Shashank Sharma > --- > drivers/gpu/drm/drm_edid.c | 14 ++++++++++++++ > include/drm/drm_connector.h | 26 ++++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) >=20 > 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); > =20 > +static void drm_detect_hdmi_scdc(struct drm_connector *connector, > + const u8 *hf_vsdb) > +{ > + struct drm_hdmi_info *hdmi =3D &connector->display_info.hdmi_info; > + > + if (hf_vsdb[6] & 0x80) { > + hdmi->scdc_supported =3D true; > + if (hf_vsdb[6] & 0x40) > + hdmi->scdc_rr =3D true; > + } > +} > + > static void drm_parse_hdmi_deep_color_info(struct drm_connector *connect= or, > const u8 *hdmi) > { > @@ -3916,6 +3928,8 @@ static void drm_parse_cea_ext(struct drm_connector = *connector, > =20 > 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); > } > } > =20 > 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; > }; > =20 > /** > @@ -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 --rS8CxjVDS/+yyDmU Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAliSCFgACgkQ3SOs138+ s6EPWA/9HYV5ZIMjp+ZB23feiFWZSNIiJiiXWNLLEClbYlVd23HntqfLymBoLuxS JsKUEOj9KvszNrfWqilPRjVLL5o5RU5ekhEdSXl6GsbOYTQDiCZ1wD93UBIqmsTL ZToakLjVVwaK1e/dVjEr6xaBzX/8afTtOoHDAZW/Geyi27nXwUYPPO0L7l3gI+GP 0frp6xGQs31RTGbAxG+1SL82X2tBM9UTwCbgk080Xm2c0sGJbPQIKRP6FBZ06RWe H2dBeOvJv2fwcW9xulgwMwzLLxWE9aqWoPrIPnZAWXp57diC14j73IJv0SwbOy/+ kZf+jQL3vADGy3J+W0a1IPiwIyqWE221uIgPbQtPZLa4AXxmZ5lh68ntYreKsJWD ggIcoTZ7Juy59FmS6OEtz9wiBR+1x0UoiwPZQoYj50VyJDIPT+LrLMYbgwjgQ+3n wKQ3qhSPrz1Byz/AKhUMk6DGA6pp6Xbe881NFNe2WHQkhK9jHufuWcs/UtZHRW3X SA8pj3UTP1OWTGpf0fLfKONi43Lr+U1ORej8hQgMHy/e3L3DMGd2iiKYspOM169g fHTXC8I2BrfbwlKi7W8U9nFOrS5p1lBAVX/E3KUdKjblAWYT2GASWs4+k4I5yFZm 7KjCes1OafzW0Y+9X2PRvJY8Id6nBX2eiapz85eSwsChVXR7Kx4= =d9gE -----END PGP SIGNATURE----- --rS8CxjVDS/+yyDmU-- --===============1403287810== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1403287810==--