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

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Thierry Reding <treding@nvidia.com>, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/dp: Use large transactions for I2C over AUX
Date: Wed, 28 Jan 2015 11:33:34 +0200	[thread overview]
Message-ID: <87y4onqmfl.fsf@intel.com> (raw)
In-Reply-To: <20150128091022.GF4764@phenom.ffwll.local>

On Wed, 28 Jan 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Jan 28, 2015 at 10:59:06AM +0200, Jani Nikula wrote:
>> On Tue, 27 Jan 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > So I've been experimenting a bit with various dongles here, and sadly I've
>> > not managed to get any one of them to return short reads :(
>> >
>> > I did find one that allows changing the speed of the i2c bus, but even if
>> > I reduce it to 1khz there are no short reads, just a lot more defers. The
>> > dongle in question has OUI 001cf8.
>> >
>> > However the good news is that EDID reads seem to get faster across the
>> > board with 16 byte messages. How much faster depends on the dongle.
>> >
>> > Here are my measurements how long it took to read a single EDID block:
>> >  DP->DVI (OUI 001cf8):	40ms -> 35ms
>> >  DP->VGA (OUI 0022b9):	45ms -> 38ms
>> >  Zotac DP->2xHDMI:	25ms ->  4ms
>> >
>> >
>> > Oh and this is how I mangled my drm_dp_i2c_xfer():
>> > transferred = 0;
>> > while (msgs[i].len > transferred) {
>> > 	msg.buffer = msgs[i].buf + transferred;
>> > 	msg.size = min_t(unsigned int, drm_dp_i2c_msg_size,
>> > 			 msgs[i].len - transferred);
>> > 	err = drm_dp_i2c_do_msg(aux, &msg);
>> > 	if (err < 0)
>> > 		break;
>> > 	WARN_ON(err == 0);
>> > 	transferred += err;
>> > }
>> >
>> > I made the msg size configurable via a module param just to help me test
>> > this stuff, but I'm thinking we might want to upstream that just to make
>> > it easier to try smaller message sizes if/when people encounter problematic
>> > sinks/dongles.
>> 
>> How about just letting that happen first, to see if and how the problems
>> occur? If there's a pattern, maybe we can fall back to 1-byte transfers
>> in those cases (or even add OUI based quirks). I've grown really
>> hesitant about adding new module parameters, they are ABI we can't
>> easily remove/regress once added.
>
> module_param_debug takes care of any such risks imo.

No such thing, maybe you mean module_param_unsafe?

Jani.

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel