Hi Andre, On Sun, 7 Feb 2021 at 18:37, Andre Przywara <andre.przyw...@arm.com> wrote: > > On Sun, 7 Feb 2021 07:37:34 -0700 > Simon Glass <s...@chromium.org> wrote: > > Hi Simon, > > > On Thu, 4 Feb 2021 at 18:08, Andre Przywara <andre.przyw...@arm.com> wrote: > > > > > > From: Jagan Teki <ja...@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 <ja...@amarulasolutions.com> > > > [Andre: rebase and smaller fixes] > > > Signed-off-by: Andre Przywara <andre.przyw...@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 <s...@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_VIDEO > > > > I 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