Hi Andre,
On Sun, 7 Feb 2021 at 18:37, Andre Przywara andre.przywara@arm.com wrote:
On Sun, 7 Feb 2021 07:37:34 -0700 Simon Glass sjg@chromium.org wrote:
Hi Simon,
On Thu, 4 Feb 2021 at 18:08, Andre Przywara andre.przywara@arm.com wrote:
From: Jagan Teki jagan@amarulasolutions.com
DM_VIDEO migration deadline is already expired, but around 80 Allwinner boards are still using video in a legacy way.
===================== WARNING ====================== This board does not use CONFIG_DM_VIDEO Please update the board to use CONFIG_DM_VIDEO before the v2019.07 release. Failure to update by the deadline may result in board removal. See doc/driver-model/migration.rst for more info. ====================================================
Convert the legacy video driver over to the DM_VIDEO framework. This is a minimal conversion: it doesn't use the DT for finding its resources, nor does it use DM clocks or DM devices for the outputs (LCD, HDMI, CVBS).
Tested in Bananapi M1+ Plus 1920x1200 HDMI out. (Jagan)
Signed-off-by: Jagan Teki jagan@amarulasolutions.com [Andre: rebase and smaller fixes] Signed-off-by: Andre Przywara andre.przywara@arm.com
Hi,
I picked this one up to get rid of the warnings. I dropped the BMP support for now (v2 1/3 and v2 2/3), I need to have a closer look, as I am not convinced this was the right solution.
Cheers, Andre
Changelog v2 .. v3:
- rebase against master, fixing up renamed variables and structs
- replace enum with #define
- remove BMP from Kconfig
- fix memory node size calculation in simplefb setup
arch/arm/mach-sunxi/Kconfig | 9 +- drivers/video/sunxi/sunxi_display.c | 262 ++++++++++++++++------------ include/configs/sunxi-common.h | 17 -- scripts/config_whitelist.txt | 1 - 4 files changed, 157 insertions(+), 132 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Some thoughts below
Many thanks for having a thorough look, much appreciated.
I will have a closer look at your comments which I didn't reply to below.
For the other points: To be honest, I haven't checked every line in this patch, my goal was merely to get it merged (this time), to prevent feature removal and drop the nasty buildman warnings. I see quite some cleanup potential here (some I already mentioned in the commit message above), but wanted to get the main DM_VIDEO conversion done first (as I think last time some discussion already prevented a merge). So my plan was to queue this for next ASAP, then send more cleanup patches on top, before the next merge window. I understand that it's typically done the other way around, but given this is dragging on for a while, is long overdue and works for me (TM) as it is, I would prefer a functional patch first, with cleanups on top.
That's fine...you have my review tag. My comments are just suggestions for improvement and much of it relates to the existing code.
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig index 0135575ca1e..a29d11505aa 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -816,13 +816,14 @@ config VIDEO_SUNXI depends on !MACH_SUN9I depends on !MACH_SUN50I depends on !SUN50I_GEN_H6
select VIDEO
select DM_VIDEOI wonder whether instead VIDEO_SUNXI should depend on DM_VIDEO ?
So I was always wondering about the logic behind that. My understanding would be: This driver is (now) implementing the DM_VIDEO interface. Any user (at board or SoC level) would just be selecting this driver, without caring about which driver interface it uses. So as this driver is (now) DM_VIDEO only, it would signal this via this select line.
If that is not how it's meant to be, can you explain the the idea behind this, please?
That sounds fine to me, but what happens when someone selects a DM and non-DM driver? That has always been the intent - to ensure that people are selecting DM for a subsystem on purpose.
Of course these days most stuff is DM, so perhaps that approach is obsolete and we should do what you say. I'm OK with that.
Also: with CONFIG_VIDEO now dying, CONFIG_DM_VIDEO somewhat loses its purpose, doesn't it?
All of the CONFIG_DM... things should be removed at some point, yet. Just waiting for the mythical deadlines...
[...]
Regards, Simon