From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 11/12] drm/dp: Read AUX read interval from DPCD Date: Mon, 29 Feb 2016 07:38:21 +0100 Message-ID: <20160229063821.GB23745@ulmo> References: <1450097764-30063-1-git-send-email-thierry.reding@gmail.com> <1450097764-30063-12-git-send-email-thierry.reding@gmail.com> <20151215103810.GK3189@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1381805855==" Return-path: Received: from mail-wm0-x233.google.com (mail-wm0-x233.google.com [IPv6:2a00:1450:400c:c09::233]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3F3416E114 for ; Mon, 29 Feb 2016 06:38:24 +0000 (UTC) Received: by mail-wm0-x233.google.com with SMTP id l68so47250281wml.0 for ; Sun, 28 Feb 2016 22:38:24 -0800 (PST) In-Reply-To: <20151215103810.GK3189@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: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1381805855== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="cvVnyQ+4j833TQvp" Content-Disposition: inline --cvVnyQ+4j833TQvp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Dec 15, 2015 at 11:38:10AM +0100, Daniel Vetter wrote: > On Mon, Dec 14, 2015 at 01:56:03PM +0100, Thierry Reding wrote: > > From: Thierry Reding > >=20 > > Store the AUX read interval from DPCD, so that it can be used to wait > > for the durations given in the specification during link training. > >=20 > > Signed-off-by: Thierry Reding > > --- > > drivers/gpu/drm/drm_dp_helper.c | 4 ++++ > > include/drm/drm_dp_helper.h | 17 +++++++++++++++++ > > 2 files changed, 21 insertions(+) > >=20 > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_h= elper.c > > index 76ac68bc1042..da519acfeba7 100644 > > --- a/drivers/gpu/drm/drm_dp_helper.c > > +++ b/drivers/gpu/drm/drm_dp_helper.c > > @@ -339,6 +339,7 @@ static void drm_dp_link_reset(struct drm_dp_link *l= ink) > > link->max_lanes =3D 0; > > =20 > > drm_dp_link_caps_reset(&link->caps); > > + link->aux_rd_interval =3D 0; > > link->edp =3D 0; > > =20 > > link->rate =3D 0; > > @@ -392,6 +393,9 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struc= t drm_dp_link *link) > > link->edp =3D edp_revs[value]; > > } > > =20 > > + /* DP_TRAINING_AUX_RD_INTERVAL is in units of 4 milliseconds */ > > + link->aux_rd_interval =3D drm_dp_aux_rd_interval(values); >=20 > Hm, just wondering a bit of the relationship between link and cap. Is link > all about sink really, and not the source? At least in my experience it > makes a lot of sense to strictly keep these two separate, since otherwise > you'll have lots of fun aligning things in generic code. Anyway, just a > thougth. The idea is that the link is the intersection between sink and source capabilities. Drivers are supposed to call drm_dp_link_probe() to obtain the capabilities of the sink and then adjust the struct drm_dp_link according to their limitations (e.g. decrease the maximum rate if they don't support 5.4 GHz, reduce the number of lanes if they only support two, ...). Once that's done the drivers can call drm_dp_link_choose() to select the "best" set of configuration parameters given the link capabilities. Note that this is strictly deriven from reading the specification under the assumption that this is how things work in real life. My, arguably limited, experience with Tegra shows that this is true. But perhaps that is overly naive. But I'd like to better understand what other drivers require so that these helpers can be improved and be useful by more than a single driver. Currently every driver implements their own DP stack, which I think is rather unfortunate because we end up with vastly different behaviour depending on which driver is in use. Of course if that's what's desired, I'm more than happy to move this code into the Tegra driver. I might have to duplicate the code that's shared with MSM, but it's really not a lot compared to what's coming up. > > + > > link->rate =3D link->max_rate; > > link->lanes =3D link->max_lanes; > > =20 > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > > index 825aaf4e8c71..20ae0e413b64 100644 > > --- a/include/drm/drm_dp_helper.h > > +++ b/include/drm/drm_dp_helper.h > > @@ -678,6 +678,22 @@ drm_dp_alternate_scrambler_reset_cap(const u8 dpcd= [DP_RECEIVER_CAP_SIZE]) > > DP_ALTERNATE_SCRAMBLER_RESET_CAP; > > } > > =20 > > +/** > > + * drm_dp_read_aux_interval() - read the AUX read interval from the DP= CD > > + * @dpcd: receiver capacity buffer > > + * > > + * Reads the AUX read interval (in microseconds) from the DPCD. Note t= hat the > > + * TRAINING_AUX_RD_INTERVAL stores the value in units of 4 millisecond= s. > > + * > > + * Returns: > > + * The read AUX interval in microseconds. > > + */ > > +static inline unsigned int > > +drm_dp_aux_rd_interval(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) >=20 > We should use this one here in the 2 delay helpers for channel_eq and > clock_recovery imo. Agreed. Thierry --cvVnyQ+4j833TQvp Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJW0+ddAAoJEN0jrNd/PrOhTDgQALVPrw1HTMHLvH1c5GQkSGmn bQ6iYh1xKpRUuY5pYaZqq395EFpKJfeXdIYqrrSFVfeerh07lIuN6bUeC7OlM1Ti A484uOZykAyC1hf/0Krq76iOgrg77kHqFjssbv1Ltt57gw++uRAvQoC65lbzekHR O4pOiFIy0B5eVTXWL9te+Cg+uSUMaW9KXtIWoxgp/rqcWQcZZB2qv8EPee/GVrH7 sh9CZ47+pApPkVEQSdjjhdBEyvtyMzY0XR6kaYL9J1etk9I01yHyrhbK4HYSWyRu I5/BlGHxqYmuGkowR2Ff33VR2JM9bh8VyXbNyHMSvFg4667uLHviVKjezeC2ymS2 UG7wY8IyaMnF3PEOiwz4Aas4ixdqAwzUJKLfCC1SBoL8vJpRMazfryVK4aKOerCf 4wDInoKWk/ok+/ZidQ6ibgVuMXWKXtMgBay/zbJemSa0+phv/m5Slv4d0hjJeIXw AwbnyBTshVAbq7079OfHkOt9EFUnI4GYNbClyKrFfhL+b8UqJV3uwxyFZmT9qhDB Gg8QjypoTwLiRNvN0YHwb0nKibN/sEoWKf0FRRca3kqDbz7ZS9Yoz5kvk6Kbf96B x5+WD4lClpMFisa0/escYtzOSslo9x1tRcDYFfi21lQZfazxPDAtfkfx/XCdk/69 OBLAPQqOsTYjBz8/99KE =PaxQ -----END PGP SIGNATURE----- --cvVnyQ+4j833TQvp-- --===============1381805855== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1381805855==--