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 16:39:29 +0000 Message-ID: <3675886.CKNMQZRBBc@f19simon> References: <1422285768-1655-1-git-send-email-simon.farnsworth@onelan.co.uk> <1540451.drLVq8Y4Zh@f19simon> <20150126161101.GC19354@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0179182558==" Return-path: Received: from claranet-outbound-smtp07.uk.clara.net (claranet-outbound-smtp07.uk.clara.net [195.8.89.40]) by gabe.freedesktop.org (Postfix) with ESMTP id 2B90B6E171 for ; Mon, 26 Jan 2015 08:39:38 -0800 (PST) In-Reply-To: <20150126161101.GC19354@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 =?ISO-8859-1?Q?Syrj=E4l=E4?= Cc: Thierry Reding , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0179182558== Content-Type: multipart/signed; boundary="nextPart1878520.LkRi71Xk3i"; micalg="pgp-sha1"; protocol="application/pgp-signature" --nextPart1878520.LkRi71Xk3i Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" On Monday 26 January 2015 18:11:01 Ville Syrj=E4l=E4 wrote: > On Mon, Jan 26, 2015 at 03:47:13PM +0000, Simon Farnsworth wrote: > > On Monday 26 January 2015 17:33:35 Ville Syrj=E4l=E4 wrote: > > > On Mon, Jan 26, 2015 at 03:22:48PM +0000, Simon Farnsworth wrote:= > > > > DisplayPort to DVI-D Dual Link adapters designed by Bizlink hav= e 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 unle= ss 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. If we ge= t a short > > > > reply, assume that there's a hardware bottleneck, and shrink ou= r read size > > > > to match. > > > >=20 > > > > Signed-off-by: Simon Farnsworth = > > > > --- > > > >=20 > > > > v2 changes, after feedback from Thierry and Ville: > > > >=20 > > > > * Handle short replies. I've decided (arbitrarily) that a shor= t reply > > > > results in us dropping back to the newly chosen size for the= rest of this > > > > I2C transaction. Thus, given an attempt to read the first 16= bytes of > > > > EDID, and a sink that only does 4 bytes of buffering, we wil= l see the > > > > following AUX transfers for the EDID read (after address is = set): > > > >=20 > > > > > > > > Read 16 bytes from I2C over AUX. > > > > Reply with 4 bytes > > > > Read 4 bytes > > > > Reply with 4 bytes > > > > Read 4 bytes > > > > Reply with 4 bytes > > > > Read 4 bytes > > > > Reply with 4 bytes > > > > > > >=20 > > > I think that's agaisnt the spec. IIRC you have to keep repeating = the > > > same transaction (meaning address/len are unchanged) until all th= e data > > > was transferred. > > > > > Do you have a spec reference against the DisplayPort 1.1a (last pub= lic > > version) spec? My chosen behaviour matches Table 2-50 in the 1.1a s= pec. >=20 > In my copy if DP v1.1 the example in 2-50 just keeps repeating w/ 16 = bytes. > So doesn't match what you do. And that's unchanged in v1.2. > Yes, looks like I misread the table; I was more thinking about "what ar= e the semantics of a 4 byte reply to a 16 byte read?" when I read that bit of= the spec. > DP v1.2 has some extra clarifications for this stuff: > "2.7.7 I2C-overAUX Transaction Clarifications and Implementation Rule= s >=20 > 2.7.7.1.6.4 Upon the Reply of I2C_ACK|AUX_ACK Followed by the Total = Number of Data > Bytes Fewer than LEN+1, to a Request Transaction with MOT Bit Set Ei= ther to 0 or 1 >=20 > The Source device must: > o Repeat the identical I2C-read-over-AUX transaction with the update= d > LEN value equal to the original LEN value minus the total number o= f data > bytes received so far, > o Repeat the identical I2C-read-over-AUX transaction with the same L= EN > value as the original value, or, > o Issue an address-only I2C-over-AUX with MOT bit set to 0 to prompt= I2C STOP > to terminate the current I2C-read-over-AUX transaction. > It should be noted that when the Source device repeats the same I2C-= read-over-AUX > transaction with the same LEN value as the original value, the Sink = device is > likely to read more data bytes than the Source device needs." >=20 So what would be the best implementation strategy for Linux? Bear in mi= nd that I don't have the 1.2 spec, nor do I have any devices which give a partial response to a 16 byte read, so I'm coding blind here, based sol= ely on the information people are giving me. The simplest strategy would be to keep repeating with 16 byte reads all= the time, but Thierry's original comment and response to the first patch ha= ve suggested that that's a performance problem waiting to happen, hence th= e effort to find a better strategy. My strategy was intended to spot that there's probably a FIFO in the mi= ddle that's too small, and reduce the transfer size to avoid overflowing the= FIFO. I could make it more complex by remembering that the last read wa= s a partial read, finishing it off by reducing the read size, then sticking= to the new smaller size, but that's added complexity and I'm not sure of i= ts value. Should I simply stick to 16 byte reads, and cope with getting back less= data than I want, or do you think we need the more complex strategy that com= plies with DP 1.2? =2D-=20 Simon Farnsworth Software Engineer ONELAN Ltd http://www.onelan.com --nextPart1878520.LkRi71Xk3i 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 iQEcBAABAgAGBQJUxm3BAAoJEOsKZy3xM+c7iSMH/1NfSaTWUw6i6WxsVtWMeBfd cUTcCsK9n+DQhSaEhawYTT0KwiyMJHpsTjJW3JkgT7GMEeWrLNYn9vWU3ugQ85e/ y0a7uGI9x/BT0VcsoEChDsKGVqz/QvE/1DoBJqXi9VW+etiKH4BwtHiTZHhkHtai nJHh4S3Dl690/nhGNwTLm1mbZZXqusve7Genidvm+zPRh223FWbsiyOJB08opX+e FihRYsUBinOVFuoOlS0QlMNDQl8SriF2/DKt9ITBv/8W6ohJHHiu0x7JyrB58FKO pJnrualNyO4VTTV2ZbdzJFv5frTOfXZ881EpUTzfeG/uMQgGpABH80e+Rl3bO+U= =N/Ni -----END PGP SIGNATURE----- --nextPart1878520.LkRi71Xk3i-- --===============0179182558== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============0179182558==--