On Mon, Jan 23, 2017 at 1:36 PM, Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote: > On Mon, 23 Jan 2017 13:12:12 +0100 > Andrea Merello <andrea.merello@xxxxxxxxx> wrote: > >> On Mon, Jan 23, 2017 at 12:32 PM, Jani Nikula >> <jani.nikula@xxxxxxxxxxxxxxx> wrote: >> > On Mon, 23 Jan 2017, Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote: >> >> Hi Andrea, >> >> >> >> On Mon, 23 Jan 2017 11:00:02 +0100 >> >> Andrea Merello <andrea.merello@xxxxxxxxx> wrote: >> >> >> >>> From: Andrea Merello <andrea.merello@xxxxxxxxx> >> >>> >> >>> The standard DRM function to get the edid from the i2c bus performs >> >>> (at least) two transfers. >> >>> >> >>> By experiments it seems that the sii9022a have problems with the >> >>> 2nd I2C start, at least unless a wait is introduced detween the >> >> >> >> ^ between >> >> >> >>> two transfers. >> >>> >> >>> So we perform one single I2C transfer, and if the transfer must be >> >>> split, then we introduce a delay. >> >> >> >> That's not exactly what this patch does: you're introducing a delay >> >> between each retry. So, if the transceiver really requires a delay >> >> between each transfer, you'll have to retry at least once on the 2nd >> >> transfer. >> >> >> >> I guess a better solution would be to add a delay even in case of >> >> success, or maybe modify drm_do_get_edid() to optionally wait for a >> >> specified time between each transfer. >> > >> > Is the problem related specifically to EDID reads, or generally to I2C >> > transfers? Perhaps this should be fixed at the adapter master_xfer layer >> > instead? Does the master_xfer perhaps return -EAGAIN, have you looked >> > into i2c_adapter timeout member? (See __i2c_transfer in i2c-core.c.) >> > >> > The intention of drm_do_get_edid() is to facilitate *alternative* >> > methods to doing I2C, not to workaround quirks like this. > > Yes, I tend to agree with that. > >> >> This problem seems not related to the I2C master itself. I saw the I2C >> master to TX correctly by looking at the I2C bus before the sii9022a >> with the scope, and I saw the same I2C transfer to be corrupted after >> the sii9022a; actually the sii9022a seems to gate the bus damaging the >> I2C start of the 2nd transfer, unless we place this delay. >> >> This happened with two different I2C master (the one on the SoC as >> well as another different one implemented on FPGA). > > Actually, I faced the same problem on an at91-based platform, and came > up with pretty much the same fix (add a delay after each transfer), > except mine was uglier and I never took the time to clean it up and > submit it. > >> >> So IMHO this quirk should be done in the sii902x dirver. Said that, I >> admit that I've not looked at i2c_adapter timeout member or other >> details in upper layers. > > Well, let's sum up the situation: the sii902x device is acting both as > a regular i2c device and as an i2c 'pass-through' device. In the > pass-through mode, the si902x has a new constraint: the i2c transfers > have to be separated by an idle period. > > Maybe we should expose the pass-through mode as separate i2c adapter, > that would be registered when entering pass-through mode and > de-registered when leaving this mode. > This way, we can implement the ->master_xfer() as we need (add a delay > after each xfer), and still use the generic drm_get_edid(). > > What do you think? IMHO this could be a good way to go.. Although honestly the current quirk doesn't seem so much bad to me :) _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
