Met vriendelijke groet / kind regards,
Mike Looijmans
System Expert
TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands
T: +31 (0) 499 33 69 69
E: mike.looijmans@xxxxxxxxxxxxxxxxx
W: www.topic.nl
Please consider the environment before printing this e-mail
On 25-05-2021 16:42, Marek Vasut wrote:
On 5/25/21 4:23 PM, Mike Looijmans wrote:
[...]
Alternatively, one can modify the RPi DSI code to start sending
data after the enable calls. That also works on my setup, with
everything in enable.
The major point here is that you should pick one and only one
callback: pre-enable or enable. The GPIO reset code as well as
writing the registers should be done in that one method.
Why , please elaborate ? It seems to be if there was no need for
those two callbacks, there would be no two callbacks in the API in
the first place. There is a chance you will get disable()->enable()
sequence without going through post_disable()->pre_enable() as far
as I can tell.
The datasheet states that "the DSI CLK lanes MUST be in HS state and
the DSI data lanes MUST be driven to LP11 state" when the reset
de-asserts (goes high) and when the CSR registers are being written.
Your driver now de-asserts the reset in the pre_enable and writes the
CSR registers in enable. This is the "least likely to work" option.
Understood. However, it seems to work on iMX8MM and MN just fine.
Is there a problem on the RPi, that the driver does not work on it ?
On the RPi, without any changes, the board won't output anything. Only
the test pattern works (using i2cset to enable it). There are two ways
to make it work:
- Use the current RPi DSI driver as is, and move all initialization code
in the sn65dsi83 driver to the "pre_enable" callback.
- Change the RPi DSI driver to not send any video data until after
"enable" and move all initialization code in the sn65dsi83 driver to the
"enable" callback.
There's also a bug in the RPi DSI driver that it does not call the
mode_set and mode_valid methods, so it needed workarounds for that too.
But that should be fixed in the RPi DSI driver and not in your code.
It's a surprise to me then that it works on the iMX. On the RPi it
absolutely won't work if the DSI is sending video data when the CSR
registers are being written. It does not seem to matter whether the data
lanes are in LP00 or LP11 state though, either mode appears to work.
Maybe the iMX is still in a command mode and doesn't actually start
sending video until after the "enable" callback?
Given that the test-pattern mode does work regardless of the data lane
state, it appears that the DSI input of the sn65dsi83 chip goes berserk
when the video data arrives before the CSR registers were written. It
won't recover from that without a full reset.
Both the reset and the CSR writing are to be done in the same
context. So either everything in "pre_enable", or everything in
"enable". Which one is correct is up to the maintainers, I also don't
know which is best. The other callback can just remain unused.
If the choice is to do the chip initialization in "pre_enable" then
do all the de-initialization in "post_disable". If the choice is to
do the chip initialization in "enable" then do all the
de-initialization in "disable".
If for some platform the choice happens to be wrong, it's a very
simple patch (just change the "ops" pointers) to change it and make
it work for that platform.
Alternatively, it's possible to make it a runtime choice through
devicetree or so as to whether the CSR initializes at "enable" or
"pre-enable".
That would mean you encode policy in DT, so not an option.
I would suggest we stop this discussion until there is input from the
maintainers. It could even be there is an API missing for configuring
the clock/data/LP/HS modes which needs to be added.
Agree...
There's no easy way out here. Adding a "post_pre_enable" or
"pre_enable_clock_up_data_down" callback is not going to make it I guess...
[...]
--
Mike Looijmans
