From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [RFC] drm: Add utility function to check for edp1.4 Date: Wed, 29 Oct 2014 14:42:29 +0100 Message-ID: <20141029134227.GA17727@ulmo.nvidia.com> References: <1413958523-32742-1-git-send-email-sonika.jindal@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1347011419==" Return-path: In-Reply-To: <1413958523-32742-1-git-send-email-sonika.jindal@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: sonika.jindal@intel.com Cc: alexdeucher@gmail.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, daniel.vetter@ffwll.ch List-Id: intel-gfx@lists.freedesktop.org --===============1347011419== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="6c2NcOVqGQ03X4Wi" Content-Disposition: inline --6c2NcOVqGQ03X4Wi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Oct 22, 2014 at 11:45:23AM +0530, sonika.jindal@intel.com wrote: > From: Sonika Jindal >=20 > v2: Reading DP_EDP_REV, only when DISPLAY_CONTROL_CAPABLE field is set (S= atheesh) >=20 > v3: Moving the utility function to drm_dp_helper (Daniel) >=20 > Signed-off-by: Sonika Jindal > --- > drivers/gpu/drm/drm_dp_helper.c | 15 +++++++++++++++ > include/drm/drm_dp_helper.h | 2 ++ > 2 files changed, 17 insertions(+) >=20 > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_hel= per.c > index 08e33b8..a54a760 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -768,3 +768,18 @@ void drm_dp_aux_unregister(struct drm_dp_aux *aux) > i2c_del_adapter(&aux->ddc); > } > EXPORT_SYMBOL(drm_dp_aux_unregister); > + > +bool drm_dp_is_edp_v1_4(struct drm_dp_aux *aux, const u8 dpcd[DP_RECEIVE= R_CAP_SIZE]) I'd prefer if this didn't take a dpcd argument but rather directly accessed the DP_EDP_CONFIGURATION_CAP register so that it can be used directly rather than rely on the driver to have read a dpcd block in the appropriate format. > +{ > + uint8_t reg; > + > + if (dpcd[DP_EDP_CONFIGURATION_CAP] & > + DP_DPCD_DISPLAY_CONTROL_CAPABLE) { > + > + if (drm_dp_dpcd_read(aux, DP_EDP_REV, ®, 1)) > + if (reg =3D=3D 0x03) > + return true; > + } > + return false; > +} > +EXPORT_SYMBOL(drm_dp_is_edp_v1_4); Does it make sense to have a function that checks for a specific version? Why not add one that returns the revision so that it can be compared, something like: u8 value; drm_dp_dpcd_read(aux, DP_EDP_REV, &value, 1); return value; Then we can do something like: #define DP_EDP_REV_1_1 0x00 #define DP_EDP_REV_1_2 0x01 #define DP_EDP_REV_1_3 0x02 #define DP_EDP_REV_1_4 0x03 And code can simply compare against that: drm_dp_get_edp_revision(aux, &rev); if (rev >=3D DP_EDP_REV_1_4) { ... } The check in your variant will only match v1.4 exactly, but presumably v1.5 will be backwards compatible. Having a direct check on the revision code will allow code to continue to work with future, backwards- compatible revisions. > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 8edeed0..b017e1e 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -102,6 +102,8 @@ > =20 > #define DP_EDP_CONFIGURATION_CAP 0x00d /* XXX 1.2? */ > #define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */ > +#define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) This seems to be a field in the DP_EDP_CONFIGURATION_CAP register, so it should be sorted below that register, not DP_TRAINING_AUX_RD_INTERVAL. > +#define DP_EDP_REV 0x700 And this belongs further down, so it properly sorts into the list of registers. Thierry --6c2NcOVqGQ03X4Wi Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJUUO7DAAoJEN0jrNd/PrOhkVUP+wWEWNt6q+MpGS7/pywJrzKr vTqD20E6Kh15mCqSg1cwvm8D6T5/L3k0A5QHETPflxrgqIw1nyzJYCWVWC1teUJx aTAIYWJ6cdgaieRyt7jqDl7M2hlf48/UwPWeSnSO3qiOUMwMqkwK78CF1E7Wv+us /XVPjq8Ayj3RAurlVmZl7tuQVfrD5onKPvoymoVT76F3eualrOZHGVzzeyV379lQ 40kTZjsJacRq7QQXabdfMa8Tbwc3A2wjiAbmOiytP2pEnqzGmJ9JPcOqE00K4/7p i4fHo2qfq9CuuU6e4ZafRtHldFypT0BNGLqAnCp31O8F8fvVFu5A8+e2kZrEfAnu FUAFJ1fzDGE1CtwMMKjtXmucbh+xP0JKQWwFNvxWdRUilM81iR++DOeOdBsOzRwQ /96TvhqPde9nz0u9PLx0sFLRC4b5T3cZ7opVsCWguOrI+JxAyemBBrHZx82evsll 9HAYbUKcOxnOfpDigH9LfA8AqZGL+z1Yf3pxsHPz1iDQf03c1bNI/dS1seCF3ar+ Sp/NcTTfnSTM7Tou11INqY5jk1zQvnmpKNEdGuWMVtcu+HFqCKcobg9cjKGw+uUK Jl8IPCj0BGcJqCgAAnESkk4yNxc4V+SUrtByW8dWVpyv7L8ek5yUKiXKqXXLuvHK p7sFMAwxTKG7DUnvt2iM =j59/ -----END PGP SIGNATURE----- --6c2NcOVqGQ03X4Wi-- --===============1347011419== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============1347011419==--