On 3/1/24 07:02, Sumit Garg wrote:
On Thu, 29 Feb 2024 at 19:31, Tom Rini <tr...@konsulko.com> wrote:

On Thu, Feb 29, 2024 at 08:42:42AM -0500, Tom Rini wrote:
On Thu, Feb 29, 2024 at 11:17:28AM +0530, Sumit Garg wrote:
On Wed, 28 Feb 2024 at 20:50, Tom Rini <tr...@konsulko.com> wrote:

On Wed, Feb 28, 2024 at 07:44:42PM +0530, Sumit Garg wrote:
+ Shawn, Krzysztof, Conor

Hi Tom,

On Wed, 28 Feb 2024 at 18:40, Tom Rini <tr...@konsulko.com> wrote:

On Wed, Feb 28, 2024 at 10:09:13AM +0300, Dan Carpenter wrote:
On Tue, Feb 27, 2024 at 04:40:01PM +0100, Sébastien Szymanski wrote:
Commit 5d7a95f49999 ("imx6ul/imx6ull: synchronise device trees with
linux") removed the display timings from the board device tree whereas
they are still needed by the mxsfb driver.
Add the timings back (the correct ones) in the
imx6ul-opos6uldev-u-boot.dtsi file and remove them from the
opos6uldev.env file.

Update the opos6uldev_defconfig file so that the LCD turns on at boot.

Fixes: 5d7a95f49999 ("imx6ul/imx6ull: synchronise device trees with linux")
Signed-off-by: Sébastien Szymanski <sebastien.szyman...@armadeus.com>

Huh.  This is the commit that did that upstream.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d9aa4d4fca67823838fe9861456201430c545e69

It's interesting how the timings in linux were always slightly different
from in u-boot.

Thanks for tracking that down, Dan. I'm adding in Sumit and Rob here
because this is a recent (rather than ancient) example of one of the
concerns about OF_UPSTREAM.

I rather think about this as an opportunity to improve with
OF_UPSTREAM. We can feed these kinds of DT ABI breakages to
corresponding Linux kernel sub-arch maintainers. Especially once we
move to OF_UPSTREAM and a sub-arch maintainer profile in Linux kernel
to keep them aware that U-Boot should be considered too.

Yes, a more extensive check around when removing information from dts
files would be good.

I think the commit in question can be
summarized as "remove a bunch of explicit HW information because there's
now a Linux Kernel driver that determines that dynamically". What do we
do next? The old information is in a presumably valid binding still, can
we just put it back and comment that consumers outside of Linux use this
still so it's not removed again later? Or am I just missing where we can
instead get this information from the DT still and not need to come up
with a new driver and subsystems?

I can see following two paths forward:

1) Partially revert the Linux kernel commit to add back the display
timings in DT.
2) Extend drivers/video/simple_panel.c in U-Boot to add support for
compatible: "armadeus,st0700-adapt".

If possible then I would be in favour of (2) rather than the current
patch to do this properly.

Well, looking at the kernel drivers/gpu/drm/panel/panel-simple.c driver
and then our drivers/video/simple_panel.c it sure would be nice if it's
just a matter of adding a compatible but I wouldn't be surprised if it
ends up needing more information being passed along too?

Although I am not a LCD panel expert but looking at the kernel driver
code [1], the display timings are rather taken from a static data
structure matching the compatible "armadeus,st0700-adapt".

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/panel/panel-simple.c#n901

Yes. My point is that it seems like the situation changed from "device
tree provides timings for the platform" to "driver has timing
information for N displays" and so we'll need to do something clever to
avoid including the structs for 5 panels when we'll only ever
(likely...) see one. And that also yes, we'll probably need to add data
for this panel rather than re-use the PANASONIC_VVX10F004B00 data.

And I'm going
assume there's good reasons for the design change in how the drivers
work in Linux now and note that it might make things more challenging
for us when we do care about space.

I agree it is always going to be challenging to use DT during SPL
stage which is mostly constrained by limited on-chip RAM.

Well, no. The DT way handled this more efficiently, I think I wasn't
clear enough in my reply.

And it's not just SPL, full U-Boot needs to stay small and within flash
partition considerations and I become cranky and question people when
non-generic changes impact platforms that don't need the change.


Okay I can see your point. I suppose this leads us to option (1) to
partially revert the Linux kernel commit [1] to add back the display
timings in DT. Ironically all the folks (developer, U-Boot and Linux
kernel iMX maintainers) were involved in the upstream process for the
Linux kernel commit [1] under question. So I will let them chime in
too.

It is also now possible to have the display timings under the panel node:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/panel/panel-simple.c?h=v6.8-rc6&id=4a1d0dbc8332231d1d500d7a1d13c45457262a97

Not sure if that could help here.

Regards,


[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d9aa4d4fca67823838fe9861456201430c545e69

-Sumit

--
Tom

--
Sébastien Szymanski, Armadeus Systems
Software engineer

Reply via email to