On Fri, Mar 01, 2024 at 10:17:35AM +0100, Sébastien Szymanski wrote: > 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.
So yes, this in conjunction with https://lore.kernel.org/linux-arm-kernel/20200115123401.2264293-4-oleksandr.suvo...@toradex.com/ type changes is what I would like to only support in U-Boot going forward (and our drivers/video/simple_panel.c will need an update). This should help make sure we don't have the problem of "new panel, more size growth for every platform". -- Tom
signature.asc
Description: PGP signature