On 12/10/2022 22:30, Dmitry Torokhov wrote:
On Wed, Oct 12, 2022 at 01:58:15PM +0300, Tomi Valkeinen wrote:
Hi,
On 11/10/2022 17:47, Tony Lindgren wrote:
* Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> [221011 14:30]:
On Tue, Oct 11, 2022 at 05:30:12PM +0300, Tony Lindgren wrote:
* Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> [221011 13:57]:
Hi Sebastian,
On Tue, Oct 11, 2022 at 02:37:26PM +0200, Sebastian Reichel wrote:
Hi,
On Tue, Oct 11, 2022 at 08:45:54AM +0300, Tony Lindgren wrote:
* Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> [221004 21:26]:
The LCD driver (panel-sony-acx565akm), when probing, starts with line
driven low, and then toggles it to high and keeps it there. Also, the
line is driven low when powering off the device, and ls released when
powering it back on. This means that the reset line should be described
as "active low" in DTS. This will be important when the driver is
converted to gpiod API which respects the polarity declared in DTS.
We should ensure these patches get merged together with the driver
change to avoid breaking LCD for booting. Probably no need to have
the driver quirk handling for inverted polartity in this case.
It's probably easiest to have an immutable branch for the driver
changes I can base the dts changes on. Or I can ack the dts changes
if they get merged with the driver.
Both drivers are already using gpiod API:
drivers/gpu/drm/panel/panel-sony-acx565akm.c
drivers/gpu/drm/panel/panel-dsi-cm.c
I was looking at
drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
Ah OK that explains :)
which are not using gpiod. Should they be retired?
Yes we should just get rid of them with omapdrm working just fine.
Will you be submitting such patches? I'd like to get rid of
of_get_named_gpio() and friends if I can...
Adding Tomi to Cc, my guess is he already has such patches and knows
better which ones can go :)
To be honest, I haven't really even had a glance towards fbdev for a long
time.
There is one thing that omapdrm doesn't support, which is VRFB rotation. I
cannot say if the users of those above-mentioned panels require VRFB.
So this just breaks things.
I missed the drivers in drivers/gpu/... and I see that they essentially
abuse gpiod API as gpiod_set_value() operates on logical level
(active/inactive) and not absolute (high/low). They should either use
the gpiod_*_raw() variants, or they should be adjusted to do the proper
thing together with the accompanying DTS change.
What are your preferences?
Seems like high/low at the connected device end is what we should use,
right? Otherwise things will misbehave if the panel is connected to
some other SoC possibly.
It is exactly because of this case the driver should use active/inactive
and follow polarity described in DTS. If the driver does:
gpiod_set_value_cansleep(d->reset, 1);
then if DTS is saying that the reset line is active low, under the wraps
the line will be driven to "0", but if DTS is saying that the line is
active high, then the very same call will drive the line to "1".
This allows accommodating different designs without having to change the
driver code.
Isn't breaking an old dts file quite a bad thing? Why not just add a comment
to the .dts and to the driver about the situation. I don't quite see that
the fixing the dts (And, if done properly, adding a boot time fixup for old
dtbs) and changing the drivers is worth the hassle.
Unless we see new users for these drivers, which would require the new users
to write broken dts files.
Or maybe there are devices with fixed DTSes and fixed up kernels but the
fixes have not been contributed upstream. I don't know...
My personal opinion is that we pay too much attention to DTS
compatibility in cases when it is not totally clear if there are devices
that use DTSes that are not bundled with the kernel and also have a
chance to have their kernel updated (and be lucky enough for the
upstream kernel to work on such device without extensive work).
Anyway, my goal is to stop exposing of_get_named_gpio() and its
derivatives, so please let me know your preference. Should I:
- mirror in omapfb drivers what gpu drivers do and use inverted
polarity
I would just go with the above for the time being. It should be an easy
change, and as these omapfb and drm panel drivers are kind of copies of
each other, I think it makes sense to use the same code in both.
That said, I personally don't mind fixing the dts files and the drivers,
and even dropping the omapfb panel drivers. However, as I don't know if
someone needs the omapfb drivers or has to use an old dtb, I don't want
to step on that possible mine field. If someone else wants to go there
(without my involvement), fine for me =).
Tomi
