Nokia RX-51: Convert to CONFIG_DM_VIDEO

Mechanically convert video_hw_init() function to UCLASS_VIDEO probe
callback and replace CONFIG_CFB_CONSOLE by CONFIG_DM_VIDEO.

As framebuffer base address is setup by the bootloader which loads U-Boot,
set it into plat->base. And do not allocate framebuffer in video_post_bind
function when base address is already specified.

This change was tested in qemu n900 machine and is working fine.

What does not work is CONFIG_VIDEO_LOGO, seems to be buggy.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 board/nokia/rx51/rx51.c      | 40 +++++++++++++++++++++---------------
 configs/nokia_rx51_defconfig |  7 +++++--
 drivers/video/video-uclass.c |  3 +++
 include/configs/nokia_rx51.h | 11 ++--------
 4 files changed, 34 insertions(+), 27 deletions(-)

Hi Pali,


On Sun, 6 Mar 2022 at 08:09, Pali Rohár <pali@kernel.org> wrote:
>
> Mechanically convert video_hw_init() function to UCLASS_VIDEO probe
> callback and replace CONFIG_CFB_CONSOLE by CONFIG_DM_VIDEO.
>
> As framebuffer base address is setup by the bootloader which loads U-Boot,
> set it into plat->base. And do not allocate framebuffer in video_post_bind
> function when base address is already specified.
>
> This change was tested in qemu n900 machine and is working fine.
>
> What does not work is CONFIG_VIDEO_LOGO, seems to be buggy.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  board/nokia/rx51/rx51.c      | 40 +++++++++++++++++++++---------------
>  configs/nokia_rx51_defconfig |  7 +++++--
>  drivers/video/video-uclass.c |  3 +++
>  include/configs/nokia_rx51.h | 11 ++--------
>  4 files changed, 34 insertions(+), 27 deletions(-)
>
> diff --git a/board/nokia/rx51/rx51.c b/board/nokia/rx51/rx51.c
> index a52691509da4..621cff095619 100644
> --- a/board/nokia/rx51/rx51.c
> +++ b/board/nokia/rx51/rx51.c
> @@ -30,7 +30,7 @@
>  #include <malloc.h>
>  #include <twl4030.h>
>  #include <i2c.h>
> -#include <video_fb.h>
> +#include <video.h>
>  #include <keyboard.h>
>  #include <asm/global_data.h>
>  #include <asm/io.h>
> @@ -62,8 +62,6 @@ struct emu_hal_params_rx51 {
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> -GraphicDevice gdev;
> -
>  const omap3_sysinfo sysinfo = {
>         DDR_STACKED,
>         "Nokia RX-51",
> @@ -342,22 +340,28 @@ void setup_board_tags(struct tag **in_params)
>         *in_params = params;
>  }
>
> -/*
> - * Routine: video_hw_init
> - * Description: Set up the GraphicDevice depending on sys_boot.
> - */
> -void *video_hw_init(void)
> +static int rx51_video_probe(struct udevice *dev)
>  {
> -       /* fill in Graphic Device */
> -       gdev.frameAdrs = 0x8f9c0000;
> -       gdev.winSizeX = 800;
> -       gdev.winSizeY = 480;
> -       gdev.gdfBytesPP = 2;
> -       gdev.gdfIndex = GDF_16BIT_565RGB;
> -       memset((void *)gdev.frameAdrs, 0, 0xbb800);
> -       return (void *) &gdev;
> +       struct video_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> +       struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> +
> +       uc_plat->base = 0x8f9c0000;

How is this calculated?

> +       uc_plat->size = 800 * 480 * sizeof(u16);

More correctly this should be in the bind() method, but in fact I
don't think it matters. We can always adjust it later if needed.

> +       uc_priv->xsize = 800;
> +       uc_priv->ysize = 480;
> +       uc_priv->bpix = VIDEO_BPP16;
> +
> +       video_set_flush_dcache(dev, true);
> +
> +       return 0;
>  }
>
> +U_BOOT_DRIVER(rx51_video) = {
> +       .name = "rx51_video",
> +       .id = UCLASS_VIDEO,
> +       .probe = rx51_video_probe,
> +};
> +
>  /*
>   * Routine: twl4030_regulator_set_mode
>   * Description: Set twl4030 regulator mode over i2c powerbus.
> @@ -777,6 +781,10 @@ U_BOOT_DRVINFOS(rx51_watchdog) = {
>         { "rx51_watchdog" },
>  };
>
> +U_BOOT_DRVINFOS(rx51_video) = {
> +       { "rx51_video" },
> +};
> +
>  U_BOOT_DRVINFOS(rx51_kp) = {
>         { "rx51_kp" },
>  };
> diff --git a/configs/nokia_rx51_defconfig b/configs/nokia_rx51_defconfig
> index 47b7bc3b4f03..1d64981afc46 100644
> --- a/configs/nokia_rx51_defconfig
> +++ b/configs/nokia_rx51_defconfig
> @@ -77,8 +77,11 @@ CONFIG_SPI=y
>  CONFIG_USB=y
>  CONFIG_USB_MUSB_UDC=y
>  CONFIG_USB_OMAP3=y
> -CONFIG_CFB_CONSOLE=y
> -CONFIG_CFB_CONSOLE_ANSI=y
> +CONFIG_DM_VIDEO=y
> +CONFIG_VIDEO_LOGO=y
> +# CONFIG_VIDEO_BPP8 is not set
> +# CONFIG_VIDEO_BPP32 is not set
> +CONFIG_SYS_WHITE_ON_BLACK=y
>  CONFIG_SPLASH_SCREEN=y
>  CONFIG_WATCHDOG_TIMEOUT_MSECS=31000
>  CONFIG_WDT=y
> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> index 7d499bcec51d..c2f7d97e93e3 100644
> --- a/drivers/video/video-uclass.c
> +++ b/drivers/video/video-uclass.c
> @@ -78,6 +78,9 @@ static ulong alloc_fb(struct udevice *dev, ulong *addrp)
>         if (!plat->size)
>                 return 0;
>
> +       if (plat->base)
> +               return 0;
> +

How about a comment there like:

/* Allow drivers to allocate the frame buffer themselves */

Please put this change in its own video: patch and update the comment
at the top of video-uclass.c and the comment for @base in video.h so
the next person can figure this out.

>         align = plat->align ? plat->align : 1 << 20;
>         base = *addrp - plat->size;
>         base &= ~(align - 1);
> diff --git a/include/configs/nokia_rx51.h b/include/configs/nokia_rx51.h
> index 9be64c3d3f87..e837b12b568f 100644
> --- a/include/configs/nokia_rx51.h
> +++ b/include/configs/nokia_rx51.h
> @@ -70,19 +70,12 @@
>
>  #define CONFIG_SYS_ONENAND_BASE                ONENAND_MAP
>
> -/*
> - * Framebuffer
> - */
> -/* Video console */
> -#define VIDEO_FB_16BPP_PIXEL_SWAP
> -#define VIDEO_FB_16BPP_WORD_SWAP
> -
>  /* Environment information */
>  #define CONFIG_EXTRA_ENV_SETTINGS \
>         "usbtty=cdc_acm\0" \
>         "stdin=usbtty,serial,keyboard\0" \
> -       "stdout=usbtty,serial,vga\0" \
> -       "stderr=usbtty,serial,vga\0" \
> +       "stdout=usbtty,serial,vidconsole\0" \
> +       "stderr=usbtty,serial,vidconsole\0" \
>         "slide=gpio input " __stringify(GPIO_SLIDE) "\0" \
>         "switchmmc=mmc dev ${mmcnum}\0" \
>         "kernaddr=0x82008000\0" \
> --
> 2.20.1
>

Regards,
Simon

On Sunday 06 March 2022 11:43:21 Simon Glass wrote:
> On Sun, 6 Mar 2022 at 08:09, Pali Rohár <pali@kernel.org> wrote:
> > -/*
> > - * Routine: video_hw_init
> > - * Description: Set up the GraphicDevice depending on sys_boot.
> > - */
> > -void *video_hw_init(void)
> > +static int rx51_video_probe(struct udevice *dev)
> >  {
> > -       /* fill in Graphic Device */
> > -       gdev.frameAdrs = 0x8f9c0000;
> > -       gdev.winSizeX = 800;
> > -       gdev.winSizeY = 480;
> > -       gdev.gdfBytesPP = 2;
> > -       gdev.gdfIndex = GDF_16BIT_565RGB;
> > -       memset((void *)gdev.frameAdrs, 0, 0xbb800);
> > -       return (void *) &gdev;
> > +       struct video_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> > +       struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> > +
> > +       uc_plat->base = 0x8f9c0000;
> 
> How is this calculated?

I do not remember. This seems to be fixed address set by the code which
loads U-Boot. I do not have any notes about this stuff.

> > +       uc_plat->size = 800 * 480 * sizeof(u16);
> 
> More correctly this should be in the bind() method, but in fact I
> don't think it matters. We can always adjust it later if needed.

Ok. I let it in probe, so all initialization is done at one place. If
something is needed to adjust, it can be done later.