[PATCH] drm/dp: Use large transactions for I2C over AUX

[Simon Farnsworth]

On Monday 26 January 2015 18:11:01 Ville Syrjälä wrote:
> On Mon, Jan 26, 2015 at 03:47:13PM +0000, Simon Farnsworth wrote:
> > On Monday 26 January 2015 17:33:35 Ville Syrjälä wrote:
> > > On Mon, Jan 26, 2015 at 03:22:48PM +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.
> > > > 
> > > > 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.
> > > > 
> > > > Copy Windows's behaviour, and read 16 bytes at a time. If we get a short
> > > > reply, assume that there's a hardware bottleneck, and shrink our read size
> > > > to match.
> > > > 
> > > > Signed-off-by: Simon Farnsworth <simon.farnsworth at onelan.co.uk>
> > > > ---
> > > > 
> > > > v2 changes, after feedback from Thierry and Ville:
> > > > 
> > > >  * Handle short replies. I've decided (arbitrarily) that a short 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 will see the
> > > >    following AUX transfers for the EDID read (after address is set):
> > > > 
> > > >    <set address, block etc>
> > > >    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
> > > >    <end I2C transaction>
> > > 
> > > I think that's agaisnt the spec. IIRC you have to keep repeating the
> > > same transaction (meaning address/len are unchanged) until all the data
> > > was transferred.
> > >
> > Do you have a spec reference against the DisplayPort 1.1a (last public
> > version) spec? My chosen behaviour matches Table 2-50 in the 1.1a spec.
> 
> 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 are 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 Rules
> 
>  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 Either to 0 or 1
> 
>  The Source device must:
>  o Repeat the identical I2C-read-over-AUX transaction with the updated
>    LEN value equal to the original LEN value minus the total number of data
>    bytes received so far,
>  o Repeat the identical I2C-read-over-AUX transaction with the same LEN
>    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."
> 
So what would be the best implementation strategy for Linux? Bear in mind
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 solely
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 have
suggested that that's a performance problem waiting to happen, hence the
effort to find a better strategy.

My strategy was intended to spot that there's probably a FIFO in the middle
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 was 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 its
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 complies
with DP 1.2?
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150126/1b4825c0/attachment.sig>