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

From: Thierry Reding <thierry.reding@gmail.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Thierry Reding <treding@nvidia.com>, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/dp: Use large transactions for I2C over AUX
Date: Fri, 23 Jan 2015 22:21:09 +0100	[thread overview]
Message-ID: <20150123212108.GA9472@mithrandir> (raw)
In-Reply-To: <20150123194629.GY19354@intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 2726 bytes --]

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@onelan.co.uk>
> > ---
> > 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.

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?

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel