From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 08/15] drm/dp: Use drm_dp_aux_rd_interval() Date: Mon, 21 Oct 2019 10:08:08 +0200 Message-ID: <20191021080808.GC1118266@ulmo> References: <20191015143509.1104985-1-thierry.reding@gmail.com> <20191015143509.1104985-9-thierry.reding@gmail.com> <46be9f324facaa8afb8dae4bd5fdb16227ff8c67.camel@redhat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1092413775==" Return-path: Received: from mail-wr1-x441.google.com (mail-wr1-x441.google.com [IPv6:2a00:1450:4864:20::441]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5A2B889B01 for ; Mon, 21 Oct 2019 08:08:13 +0000 (UTC) Received: by mail-wr1-x441.google.com with SMTP id r1so2920039wrs.9 for ; Mon, 21 Oct 2019 01:08:13 -0700 (PDT) In-Reply-To: <46be9f324facaa8afb8dae4bd5fdb16227ff8c67.camel@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Lyude Paul Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1092413775== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="da4uJneut+ArUgXk" Content-Disposition: inline --da4uJneut+ArUgXk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 18, 2019 at 05:33:12PM -0400, Lyude Paul wrote: > This also seems like maybe it should just go into the previous patch? I suppose they could both be merged, but I think it's better to keep them separate. In fact, I'm having second thoughts about the new helper because it doesn't really take into account all the special cases. For example, the patch below will use the value returned from the helper independent of context, whereas according to the specification the value is different if used for clock recovery (100 us) or if it is used for channel equalization (400 us). Perhaps a better order would be for the "do not busy loop" patch to go first and then introduce the new helper and finally use the new helper (along with the signed -> unsigned change) in a third patch while taking care of using the right values all the time. I'll respin these patches and send out the fixes in a v2. Thierry > On Tue, 2019-10-15 at 16:35 +0200, Thierry Reding wrote: > > From: Thierry Reding > >=20 > > Make use of the newly added drm_dp_aux_rd_interval() helper in existing > > DP link training helpers. > >=20 > > v2: drop stale sentence from commit message (Philipp Zabel) > >=20 > > Signed-off-by: Thierry Reding > > --- > > drivers/gpu/drm/drm_dp_helper.c | 26 +++----------------------- > > 1 file changed, 3 insertions(+), 23 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/drm_dp_helper.c > > b/drivers/gpu/drm/drm_dp_helper.c > > index ad2671d2ee8f..4b66010771fa 100644 > > --- a/drivers/gpu/drm/drm_dp_helper.c > > +++ b/drivers/gpu/drm/drm_dp_helper.c > > @@ -122,17 +122,7 @@ EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphas= is); > > =20 > > void drm_dp_link_train_clock_recovery_delay(const u8 > > dpcd[DP_RECEIVER_CAP_SIZE]) > > { > > - unsigned int rd_interval =3D dpcd[DP_TRAINING_AUX_RD_INTERVAL] & > > - DP_TRAINING_AUX_RD_MASK; > > - > > - if (rd_interval > 4) > > - DRM_DEBUG_KMS("AUX interval %u, out of range (max 4)\n", > > - rd_interval); > > - > > - if (rd_interval =3D=3D 0 || dpcd[DP_DPCD_REV] >=3D DP_DPCD_REV_14) > > - rd_interval =3D 100; > > - else > > - rd_interval *=3D 4 * USEC_PER_MSEC; > > + unsigned int rd_interval =3D drm_dp_aux_rd_interval(dpcd); > > =20 > > usleep_range(rd_interval, rd_interval * 2); > > } > > @@ -140,19 +130,9 @@ EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_del= ay); > > =20 > > void drm_dp_link_train_channel_eq_delay(const u8 > > dpcd[DP_RECEIVER_CAP_SIZE]) > > { > > - unsigned int rd_interval =3D dpcd[DP_TRAINING_AUX_RD_INTERVAL] & > > - DP_TRAINING_AUX_RD_MASK; > > - > > - if (rd_interval > 4) > > - DRM_DEBUG_KMS("AUX interval %u, out of range (max 4)\n", > > - rd_interval); > > + unsigned int min =3D drm_dp_aux_rd_interval(dpcd); > > =20 > > - if (rd_interval =3D=3D 0) > > - rd_interval =3D 400; > > - else > > - rd_interval *=3D 4 * USEC_PER_MSEC; > > - > > - usleep_range(rd_interval, rd_interval * 2); > > + usleep_range(min, min * 2); > > } > > EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay); > > =20 > --=20 > Cheers, > Lyude Paul >=20 --da4uJneut+ArUgXk Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAl2tZ2gACgkQ3SOs138+ s6FG5w//Tk+FcHUqNsmYAhqfYQpXd0G9n6dYND6L22eNREldoP1hLGn8d32kNf2O z3wETLOvG2yChUSEYGiF3VUvjtHMVvQIxvcLwHJCtSNduRAiUw4Y1Qk7sNextwRi 0VJYO647pItiQHpuSp/Uc8OLAmfAcTLFMoew/tqrGW3/wv2zFrw3BEsJFUK853/6 bBv+F4Kgx2Pi70UjdFnyW6Xag/g6sAOLmfDhJt/rRD7/S8Mb4FVPgbwlkGOwLiKt 9GrKGIDJZLWRevDWBUUX/ytIIK4xIXFzMAjZPgveB3MpPLOTKR2lFX9ojIS6yyr2 fJtEykAYVfGFaSY+eZsprHehciIzACo/c1lpOt+X/G1g/dyJQZZAWcPc9+ohWuy5 0NykFaaI49vPq8EGDzrlxe5FCVG1HLLtnKqTorv7cay56MCwZUPH2llqggzLo0VP 49zP6caiROI61F044ClI7HkQX3aixYSC2DpyvgeQKAJUPeCG0vBAlAMIdOBMmyKH hu9TbYtDihAo/2b3rQqgWQ2+YJsxYmOpiadk+44PjAQgHSy2UF/P6c/1SgUpGMYc t2XPi0xkvrlHtt4O/SrImnkJGR8N6m8SMUYXCiH8YYEEJftRkObxECKNnbYIqMc/ /XdznxzoWYSiOJjQ7XQvHkRYDBYJN1FL1jppvjOw0bcfVnPXGuk= =PlmK -----END PGP SIGNATURE----- --da4uJneut+ArUgXk-- --===============1092413775== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs --===============1092413775==--