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

[Ville Syrjälä]

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.

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."

-- 
Ville Syrjälä
Intel OTC