From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH v3 3/4] drm/dp: Add DisplayPort link helpers Date: Tue, 21 Jan 2014 20:30:40 +0100 Message-ID: <20140121193039.GA12023@ulmo.nvidia.com> References: <1389711336-29702-1-git-send-email-treding@nvidia.com> <1389711336-29702-4-git-send-email-treding@nvidia.com> <87bnzabv5r.fsf@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1179069952==" Return-path: In-Reply-To: <87bnzabv5r.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1179069952== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="gKMricLos+KVdGMg" Content-Disposition: inline --gKMricLos+KVdGMg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 17, 2014 at 03:22:08PM +0200, Jani Nikula wrote: > On Tue, 14 Jan 2014, Thierry Reding wrote: [...] > > +int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link) > > +{ > > + u8 values[3]; > > + int err; > > + > > + memset(link, 0, sizeof(*link)); > > + > > + err =3D drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values)); > > + if (err < 0) > > + return err; > > + > > + link->revision =3D values[0]; > > + link->rate =3D drm_dp_bw_code_to_link_rate(values[1]); > > + link->num_lanes =3D values[2] & DP_MAX_LANE_COUNT_MASK; > > + > > + if (values[2] & DP_ENHANCED_FRAME_CAP) > > + link->capabilities |=3D DP_LINK_CAP_ENHANCED_FRAMING; >=20 > Since DP_DPCD_REV =3D=3D 0, you could use the #defines for the indexes (if > you're going to send another version anyway). Ditto below for > drm_dp_link_configure. We write to DP_LINK_BW_SET in drm_dp_link_configure() so I don't think we can apply the same trick there. Also I'm not sure if it's really worth having that here. Given that it only works if we actually read DP_DPCD_REV, the number of locations where it can be done is fairly small and they will look asymmetric with respect to other functions using the drm_dp_dpcd_read/write() helpers. So unless you feel strongly I'd prefer not to do this. > Other than that nitpick, the series looks good to me. If we face any > issues migrating i915 on top of this, we can iron them out later on. >=20 > On the series, >=20 > Reviewed-by: Jani Nikula Thanks! Thierry --gKMricLos+KVdGMg Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJS3srfAAoJEN0jrNd/PrOhVzIP/2a8rXAL58azQ55oUHgN+rWa ReUvqqFx3/+EGMq4NM3/wy2IMN5Xe5Uz2ysSJ9ysI7vAV2LYxe2WQRONnFFORqjj Th6vkm3z86VOA/fjon813ag/P3id6SvEqkMrqOj832PH3HCPw9JeIE/w5Xs/Vu3/ DLxIaz7ZAH6Vxorn7oGTlMsgsfQFFiXknZX8Pzn8dwzve/m2Fnna/Ry8hVkqbJcS 2+AvNII1nCei5/GE3aBLjzoQF+nL2LdFQFyY/wm66lpaEZzgQE8ZRWxuLmVIFoYH +WZypV1+lJbYwIvBU9CYFkSUIuJhl4y6DY6IVBKXdWPL34xBVAz/jfa/nRsYUTHx bOwlTrruVvihEfsugEvnQfmQVY1Lj1Ih27AoC9JKe0Y3rSSXHnzIlZPj+u+2Mdtd Cr8oQapkEIzjlu8uoOzuCt/Esgi92mdjaviGEyS3Ymvy6fmFjT39rwGYrQ6VFEX/ PnxttsiKzv2I27qi3pLJByQbE1Bc9zi10GFRXCbhIWkGtgnoJ3FkAz8N8AL6G/d2 kzLKmu1vP/Y+9HphlEJpitzaFUsw0EY2XVIHPMdznb6vgmGQZ8NZLUcVNmlvl1IC 5nswyVFZkSS1QtANPiM5ymdqpXm5hxmTZcTGuCB+lasrYzWMJtkVXqUQZ6A6cMIz mdZxLNW1alsWxfdpQz5z =0/Q1 -----END PGP SIGNATURE----- --gKMricLos+KVdGMg-- --===============1179069952== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1179069952==--