On 2/21/22 10:12, Lucas Stach wrote:
Am Samstag, dem 19.02.2022 um 03:39 +0100 schrieb Marek Vasut:
On 2/18/22 18:49, Lucas Stach wrote:
Am Freitag, dem 18.02.2022 um 02:00 +0100 schrieb Marek Vasut:
The TC358767/TC358867/TC9595 are all capable of operating either from
attached Xtal or from DSI clock lane clock. In case the later is used,
all I2C accesses will fail until the DSI clock lane is running and
supplying clock to the chip.
Move all hardware initialization to enable callback to guarantee the
DSI clock lane is running before accessing the hardware. In case Xtal
is attached to the chip, this change has no effect.
I'm not sure if that last statement is correct. When the chip is
bridging to eDP, the aux channel and HPD handling is needed to be
functional way before the atomic enable happen. I have no idea how this
would interact with the clock supplied from the DSI lanes. Maybe it
doesn't work at all and proper eDP support always needs a external
reference clock?
The driver currently assumes the TC358767 always gets RefClk from Xtal.
There is subsequent series which adds support for deriving clock which
drive the TC358767 PLLs from DSI HS clock instead of Xtal in case the
bridge operates in DSI-to-DPI mode. That needs additional plumbing, as
the TC358767 must be able to select specific clock frequency on the DSI
HS clock lane, because its PLLs need specific frequencies, see:
[RFC][PATCH 0/7] drm/bridge: Add support for selecting DSI host HS clock
from DSI bridge
If someone needs to implement DSI-to-(e)DP mode without Xtal, ugh, that
would likely need to have a way to figure out the DSI HS clock frequency
already in probe and then enable those DSI HS clock very early on too ?
Probably, but that was really just something I wondered about while
passing by.
The real issue is that I think _this_ patch breaks eDP operation, as
now the chip is initialized way too late, as the AUX channel
functionality needs to be available long before the atomic bridge
enable callback is called.
I don't think that's correct -- look at the tc_hardware_init() function,
all it does is it reads the chip ID, optionally does software reset if
there is no reset GPIO connected, and then sets up hotplug detect IRQ.
There is nothing specific to bringing up the AUX channel in that
function, so the AUX channel should work even before tc_hardware_init()
is called.
The AUX channel is used to fetch the display
EDID, so before you can even set a mode it needs to be functional.
Unconditionally moving the chip init is probably (I haven't tested it
yet) going to break this.
If you have such a setup with eDP, please test it, I don't think this is
going to break it.
