RE: [RFC] drm/bridge/sii902x: Fix EDID readback

[Fabrizio Castro <fabrizio.castro@bp.renesas.com>]

Hello Peter,

Thank you for your feedback!

> > The "mux-locked" implementation was the one I first tried and I discovered
> > it doesn't work for me, as other traffic on the parent adapter can get in the
> > way. What we need for this device is no other traffic on the bus during the
> > "select transaction deselect" procedure, that's why I went with the parent
> > locking. Also this device needs a delay between stop and start conditions
> > while addressing the monitor.
>
> Ok, I thought the problem was that a delay was needed between the STOP
> of the command opening the gate and the START of the edid eeprom xfer, and
> that everything else worked normally. Too bad this wasn't the actual problem.
>
> Hmm. If it is indeed true that other xfers must never creep into the "select
> xfer deselect" procedure then you are indeed stuck with a parent-locking.
> But is that really the case? Could it be that the extra delay between
> STOP-START is only needed after the absolutely last xfer before the
> command closing the gate?
>
> If that problem description is correct, it should be possible to go back to
> a mux-locked gate, but then in your deselect implementation grab the i2c adapter
> lock manually - with i2c_lock_bus(adapter, I2C_LOCK_ROOT_ADAPTER) - before the
> 30 us delay, then open code the command to close the gate with an unlocked i2c
> access, and finally release the i2c bus lock. That way you have ensured silence
> on the bus for the required time before closing the gate. You would still need
> to bypass regmap, but only in this one place (but maybe you should bypass
> regmap for opening the gate too, for symmetry).

To further detail the problem, the system is vulnerable from before the last write
performed by sii902x_i2c_bypass_select to after we confirm we need the switch
to be closed within sii902x_i2c_bypass_deselect, that's the minimum amount of time
we could keep the parent adapter locked for, I guess I am stuck with a parent-locking
architecture, aren't I? But I guess I could release it when it's not actually needed,
or is this going to be a pain to maintain? Shall I just keep going with the parent-locking
but using bare i2c transactions only within select and deselect and leave regmap
to deal with everything else?

snip

> >
> > Ah, do you think the following is going to cause any issue in the future?
> > -edid = drm_get_edid(connector, sii902x->i2c->adapter);
> > +edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
>
> No, that was a critical part of the idea to use a gate to introduce the
> needed delay.

Again, thank you very much for spending your time looking into this, your
feedback  is very much appreciated.

Fab

>
> Cheers,
> Peter



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel