On Friday 23 January 2015 22:21:09 Thierry Reding wrote:
> On Fri, Jan 23, 2015 at 09:46:29PM +0200, Ville Syrjälä 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.
> > >
> > > 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. Analysis of the
> > > failure state was provided by Datapath Ltd.
> > >
> > > Signed-off-by: Simon Farnsworth <simon.farnsworth@xxxxxxxxxxxx>
> > > ---
> > > Thierry,
> > >
> > > You put in the comment about "decreased performance", back in December 2013;
> > > would you mind testing that this still works with the devices you tested?
> > >
> > > Unfortunately, Bizlink are the only game in town for DP->DVI-DL adapters -
> > > and their firmware is prone to giving up on I2C if we look at it
> > > wrongly. Even Apple's device is Bizlink designed.
> > >
> > > drivers/gpu/drm/drm_dp_helper.c | 13 +++++--------
> > > 1 file changed, 5 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_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_adapter *adapter, struct i2c_msg *msgs,
> > > err = 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 = 0; j < msgs[i].len; j++) {
> > > + for (j = 0; j < msgs[i].len; j+=16) {
> > > msg.buffer = msgs[i].buf + j;
> > > - msg.size = 1;
> > > + msg.size = min(16, msgs[i].len - 16);
> >
> > 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 that
fixed. I don't have hardware to test against that I know of; is there anyone
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. 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?
>
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.
--
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com
Attachment:
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
