From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH] drm/dp: Use large transactions for I2C over AUX Date: Fri, 23 Jan 2015 22:21:09 +0100 Message-ID: <20150123212108.GA9472@mithrandir> References: <1422038438-9323-1-git-send-email-simon.farnsworth@onelan.co.uk> <20150123194629.GY19354@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1189152466==" Return-path: Received: from mail-wi0-f170.google.com (mail-wi0-f170.google.com [209.85.212.170]) by gabe.freedesktop.org (Postfix) with ESMTP id 5341D6E045 for ; Fri, 23 Jan 2015 13:21:13 -0800 (PST) Received: by mail-wi0-f170.google.com with SMTP id em10so5437630wid.1 for ; Fri, 23 Jan 2015 13:21:12 -0800 (PST) In-Reply-To: <20150123194629.GY19354@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Ville =?utf-8?B?U3lyasOkbMOk?= Cc: Thierry Reding , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1189152466== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="6TrnltStXW4iwmi0" Content-Disposition: inline --6TrnltStXW4iwmi0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 23, 2015 at 09:46:29PM +0200, Ville Syrj=C3=A4l=C3=A4 wrote: > On Fri, Jan 23, 2015 at 06:40:38PM +0000, Simon Farnsworth wrote: > > DisplayPort to DVI-D Dual Link adapters designed by Bizlink have bugs in > > their I2C over AUX implementation. They work fine with Windows, but fail > > with Linux. > >=20 > > It turns out that they cannot keep an I2C transaction open unless the > > previous read was 16 bytes; shorter reads can only be followed by a zero > > byte transfer ending the I2C transaction. > >=20 > > Copy Windows's behaviour, and read 16 bytes at a time. Analysis of the > > failure state was provided by Datapath Ltd. > >=20 > > Signed-off-by: Simon Farnsworth > > --- > > Thierry, > >=20 > > You put in the comment about "decreased performance", back in December = 2013; > > would you mind testing that this still works with the devices you teste= d? > >=20 > > Unfortunately, Bizlink are the only game in town for DP->DVI-DL adapter= s - > > and their firmware is prone to giving up on I2C if we look at it > > wrongly. Even Apple's device is Bizlink designed. > >=20 > > drivers/gpu/drm/drm_dp_helper.c | 13 +++++-------- > > 1 file changed, 5 insertions(+), 8 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_h= elper.c > > index 79968e3..b4a9d4a 100644 > > --- a/drivers/gpu/drm/drm_dp_helper.c > > +++ b/drivers/gpu/drm/drm_dp_helper.c > > @@ -507,16 +507,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *ad= apter, struct i2c_msg *msgs, > > err =3D drm_dp_i2c_do_msg(aux, &msg); > > if (err < 0) > > break; > > - /* > > - * Many hardware implementations support FIFOs larger than a > > - * single byte, but it has been empirically determined that > > - * transferring data in larger chunks can actually lead to > > - * decreased performance. Therefore each message is simply > > - * transferred byte-by-byte. > > + /* Bizlink designed DP->DVI-D Dual Link adapters require the > > + * I2C over AUX packets to be as large as possible. If= not, > > + * the I2C transactions never succeed. > > */ > > - for (j =3D 0; j < msgs[i].len; j++) { > > + for (j =3D 0; j < msgs[i].len; j+=3D16) { > > msg.buffer =3D msgs[i].buf + j; > > - msg.size =3D 1; > > + msg.size =3D min(16, msgs[i].len - 16); >=20 > I don't think it's quite this simple. The sink is allowed to ACK > partial data for multi-byte messages. The code doesn't handle that. Also not all hardware may support transferring 16 bytes at a time. How does that work with these adapters? Does it mean they can't work on DP hardware that can't do 16 byte block transfers? Thierry --6TrnltStXW4iwmi0 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJUwrs5AAoJEN0jrNd/PrOh4WoP/AmfBb417hv8+JhAo88P+OAE R2Fpo6xPppwD8CfLZzx1feO6Rfk9PTxq/Zq58KgbsPkNNRSh6/FV9Hc7igEwUk5K fyCXUsSh8tfhgA7+WQkHUuGVQ/l9EuDaZIGviVt13vOoeFYokMAOicsj7fCRrvdQ I4jx+509+f/Eu5AKm4UGPrtc+FxZnFy1SHc5VLofOFHOEZqWXcyPZlhYleBOg2NI 3BUul4Y8OkhZW8UGXCQvoga5bbe94/KzbZLwCvmEZXBPv/Y98GKZl2h85l2/QC6W wZcOHTe2xnAXnWbJgCgNVFeXQa7E2ZV1Kxj7Mt6JHxv2rbOviLcAHNy510eH0Fcf ZxMlPgzAxpGo08HE7PoxOfthWLY2VefQlLBITndqrfwqKPdj3/XegQ6G+dQLWRyD TsIJbD12oP/TLy5OlLu75ZfABP4qFhcMMGdrMwoydXbqAUyOyWImf/8KwUz7lXsE 3ik4sfGS/C57bD7Tk3+KMC4cn7JM2+GTAepxgnRCyggCyBliFRsRkaG0pBQkRyRS xF53Hijg9JlIFTJSxQdbU7NDd/8yjJrStx091d7sxNUIrTqp/nYHh6bDBvewU/mx f1UATU5GKs7Xs+2SME/TW7B5w9q5IW5wIQ/rq36S6MaIypH1sQyi1NB8jhY6+9tX l3J3SAvIF2+Q2uc2GM5v =vKpp -----END PGP SIGNATURE----- --6TrnltStXW4iwmi0-- --===============1189152466== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1189152466==--