From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Farnsworth Subject: Re: [PATCH] drm/dp: Use large transactions for I2C over AUX Date: Mon, 26 Jan 2015 09:50:17 +0000 Message-ID: <1451253.iOrOxgl147@f19simon> References: <1422038438-9323-1-git-send-email-simon.farnsworth@onelan.co.uk> <20150123194629.GY19354@intel.com> <20150123212108.GA9472@mithrandir> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1761863093==" Return-path: Received: from claranet-outbound-smtp08.uk.clara.net (claranet-outbound-smtp08.uk.clara.net [195.8.89.41]) by gabe.freedesktop.org (Postfix) with ESMTP id A61726E09D for ; Mon, 26 Jan 2015 01:50:30 -0800 (PST) In-Reply-To: <20150123212108.GA9472@mithrandir> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Thierry Reding Cc: Thierry Reding , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1761863093== Content-Type: multipart/signed; boundary="nextPart5718051.dgao9FBlec"; micalg="pgp-sha1"; protocol="application/pgp-signature" --nextPart5718051.dgao9FBlec Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" On Friday 23 January 2015 22:21:09 Thierry Reding wrote: > On Fri, Jan 23, 2015 at 09:46:29PM +0200, Ville Syrj=E4l=E4 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, b= ut 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 o= f 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 Dec= ember 2013; > > > would you mind testing that this still works with the devices you= tested? > > >=20 > > > Unfortunately, Bizlink are the only game in town for DP->DVI-DL a= dapters - > > > 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/dr= m_dp_helper.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_adapt= er *adapter, struct i2c_msg *msgs, > > > =09=09err =3D drm_dp_i2c_do_msg(aux, &msg); > > > =09=09if (err < 0) > > > =09=09=09break; > > > -=09=09/* > > > -=09=09 * Many hardware implementations support FIFOs larger than= a > > > -=09=09 * single byte, but it has been empirically determined tha= t > > > -=09=09 * transferring data in larger chunks can actually lead to= > > > -=09=09 * decreased performance. Therefore each message is simply= > > > -=09=09 * transferred byte-by-byte. > > > +=09=09/* Bizlink designed DP->DVI-D Dual Link adapters require t= he > > > + * I2C over AUX packets to be as large as possib= le. If not, > > > + * the I2C transactions never succeed. > > > =09=09 */ > > > -=09=09for (j =3D 0; j < msgs[i].len; j++) { > > > +=09=09for (j =3D 0; j < msgs[i].len; j+=3D16) { > > > =09=09=09msg.buffer =3D msgs[i].buf + j; > > > -=09=09=09msg.size =3D 1; > > > +=09=09=09msg.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.= > It doesn't look challenging to fix that, though - I'll do a v2 with tha= t fixed. I don't have hardware to test against that I know of; is there a= nyone who's got a sink that does partial ACKs that could test for me? > Also not all hardware may support transferring 16 bytes at a time. Ho= w > does that work with these adapters? Does it mean they can't work on D= P > hardware that can't do 16 byte block transfers? >=20 Correct. 16 bytes or go home, based on testing done at Datapath. Not entirely coincidentally, this is the behaviour of NVIDIA, AMD and Intel= graphics on Windows and Mac OS X - I suspect that testing was dominated= by "what does Windows do", not by "what does the spec say". Datapath Ltd tested with a non-Linux source that's only capable of transferring one byte at a time, and the adapter failed in exactly the = same way as it does with Linux. =2D-=20 Simon Farnsworth Software Engineer ONELAN Ltd http://www.onelan.com --nextPart5718051.dgao9FBlec Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABAgAGBQJUxg3eAAoJEOsKZy3xM+c7a1UH/0269s5B76G+KPXwLKdbzIry r/MvS0UHirVhE1VS+d10V5xKkCoMdNdAx48RYOLEFe9UBWY+yLe830CoAyOnQ+2e E6c4pXR4v7BJNb86N4/X9XfXZTgw8R7DDRF//dqP8TFjW8aAbDuuIjRVKXEX5/VD d4wVyLdG+nVHByAixJL3ubjwmJxob72QiS3T7oc+3C3otR5vS9kbRpHQureA4qrn K/N02Iey4zDk32cfStzLH9lGu4q5rFWMWSWwbk39UvyDlV+mg5v2T8AyRyNPbVly 3QGqBEVx0arp4Zi6WTF1zny56iBfNJberqcWyVUq/IXNMeGhwliod8P8/jj+xcQ= =jMqX -----END PGP SIGNATURE----- --nextPart5718051.dgao9FBlec-- --===============1761863093== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1761863093==--