[PATCH v3] video: sunxi_display: Convert to DM_VIDEO
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(-)
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 + select DISPLAY imply VIDEO_DT_SIMPLEFB default y ---help--- - Say Y here to add support for using a cfb console on the HDMI, LCD - or VGA output found on most sunxi devices. See doc/README.video for - info on how to select the video output and mode. + Say Y here to add support for using a graphical console on the HDMI, + LCD or VGA output found on older sunxi devices. This will also provide + a simple_framebuffer device for Linux.
config VIDEO_HDMI bool "HDMI output support" diff --git a/drivers/video/sunxi/sunxi_display.c b/drivers/video/sunxi/sunxi_display.c index f52aba4d21c..61498d1642f 100644 --- a/drivers/video/sunxi/sunxi_display.c +++ b/drivers/video/sunxi/sunxi_display.c @@ -7,6 +7,8 @@ */
#include <common.h> +#include <display.h> +#include <dm.h> #include <cpu_func.h> #include <efi_loader.h> #include <init.h> @@ -28,7 +30,9 @@ #include <fdt_support.h> #include <i2c.h> #include <malloc.h> +#include <video.h> #include <video_fb.h> +#include <dm/uclass-internal.h> #include "../videomodes.h" #include "../anx9804.h" #include "../hitachi_tx18d42vm_lcd.h" @@ -45,6 +49,11 @@
DECLARE_GLOBAL_DATA_PTR;
+/* Maximum LCD size we support */ +#define LCD_MAX_WIDTH 3840 +#define LCD_MAX_HEIGHT 2160 +#define LCD_MAX_LOG2_BPP VIDEO_BPP32 + enum sunxi_monitor { sunxi_monitor_none, sunxi_monitor_dvi, @@ -59,12 +68,11 @@ enum sunxi_monitor { #define SUNXI_MONITOR_LAST sunxi_monitor_composite_pal_nc
struct sunxi_display { - GraphicDevice graphic_device; enum sunxi_monitor monitor; unsigned int depth; unsigned int fb_addr; unsigned int fb_size; -} sunxi_display; +};
const struct ctfb_res_modes composite_video_modes[2] = { /* x y hz pixclk ps/kHz le ri up lo hs vs s vmode */ @@ -214,7 +222,8 @@ static int sunxi_hdmi_edid_get_block(int block, u8 *buf) return r; }
-static int sunxi_hdmi_edid_get_mode(struct ctfb_res_modes *mode, +static int sunxi_hdmi_edid_get_mode(struct sunxi_display *sunxi_display, + struct ctfb_res_modes *mode, bool verbose_mode) { struct edid1_info edid1; @@ -291,14 +300,14 @@ static int sunxi_hdmi_edid_get_mode(struct ctfb_res_modes *mode, }
/* Check for basic audio support, if found enable hdmi output */ - sunxi_display.monitor = sunxi_monitor_dvi; + sunxi_display->monitor = sunxi_monitor_dvi; for (i = 0; i < ext_blocks; i++) { if (cea681[i].extension_tag != EDID_CEA861_EXTENSION_TAG || cea681[i].revision < 2) continue;
if (EDID_CEA861_SUPPORTS_BASIC_AUDIO(cea681[i])) - sunxi_display.monitor = sunxi_monitor_hdmi; + sunxi_display->monitor = sunxi_monitor_hdmi; }
return 0; @@ -414,9 +423,9 @@ static void sunxi_frontend_mode_set(const struct ctfb_res_modes *mode, static void sunxi_frontend_enable(void) {} #endif
-static bool sunxi_is_composite(void) +static bool sunxi_is_composite(enum sunxi_monitor monitor) { - switch (sunxi_display.monitor) { + switch (monitor) { case sunxi_monitor_none: case sunxi_monitor_dvi: case sunxi_monitor_hdmi: @@ -473,7 +482,8 @@ static const u32 sunxi_rgb2yuv_coef[12] = { };
static void sunxi_composer_mode_set(const struct ctfb_res_modes *mode, - unsigned int address) + unsigned int address, + enum sunxi_monitor monitor) { struct sunxi_de_be_reg * const de_be = (struct sunxi_de_be_reg *)SUNXI_DE_BE0_BASE; @@ -502,7 +512,7 @@ static void sunxi_composer_mode_set(const struct ctfb_res_modes *mode, #endif SUNXI_DE_BE_MODE_INTERLACE_ENABLE);
- if (sunxi_is_composite()) { + if (sunxi_is_composite(monitor)) { writel(SUNXI_DE_BE_OUTPUT_COLOR_CTRL_ENABLE, &de_be->output_color_ctrl); for (i = 0; i < 12; i++) @@ -616,7 +626,8 @@ static void sunxi_lcdc_backlight_enable(void) gpio_direction_output(pin, PWM_ON); }
-static void sunxi_lcdc_tcon0_mode_set(const struct ctfb_res_modes *mode, +static void sunxi_lcdc_tcon0_mode_set(struct sunxi_display *sunxi_display, + const struct ctfb_res_modes *mode, bool for_ext_vga_dac) { struct sunxi_lcdc_reg * const lcdc = @@ -643,17 +654,18 @@ static void sunxi_lcdc_tcon0_mode_set(const struct ctfb_res_modes *mode, }
lcdc_pll_set(ccm, 0, mode->pixclock_khz, &clk_div, &clk_double, - sunxi_is_composite()); + sunxi_is_composite(sunxi_display->monitor));
video_ctfb_mode_to_display_timing(mode, &timing); lcdc_tcon0_mode_set(lcdc, &timing, clk_div, for_ext_vga_dac, - sunxi_display.depth, CONFIG_VIDEO_LCD_DCLK_PHASE); + sunxi_display->depth, CONFIG_VIDEO_LCD_DCLK_PHASE); }
#if defined CONFIG_VIDEO_HDMI || defined CONFIG_VIDEO_VGA || defined CONFIG_VIDEO_COMPOSITE static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode, int *clk_div, int *clk_double, - bool use_portd_hvsync) + bool use_portd_hvsync, + enum sunxi_monitor monitor) { struct sunxi_lcdc_reg * const lcdc = (struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE; @@ -663,7 +675,7 @@ static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode,
video_ctfb_mode_to_display_timing(mode, &timing); lcdc_tcon1_mode_set(lcdc, &timing, use_portd_hvsync, - sunxi_is_composite()); + sunxi_is_composite(monitor));
if (use_portd_hvsync) { sunxi_gpio_set_cfgpin(SUNXI_GPD(26), SUNXI_GPD_LCD0); @@ -671,7 +683,7 @@ static void sunxi_lcdc_tcon1_mode_set(const struct ctfb_res_modes *mode, }
lcdc_pll_set(ccm, 1, mode->pixclock_khz, clk_div, clk_double, - sunxi_is_composite()); + sunxi_is_composite(monitor)); } #endif /* CONFIG_VIDEO_HDMI || defined CONFIG_VIDEO_VGA || CONFIG_VIDEO_COMPOSITE */
@@ -725,7 +737,8 @@ static void sunxi_hdmi_setup_info_frames(const struct ctfb_res_modes *mode) }
static void sunxi_hdmi_mode_set(const struct ctfb_res_modes *mode, - int clk_div, int clk_double) + int clk_div, int clk_double, + enum sunxi_monitor monitor) { struct sunxi_hdmi_reg * const hdmi = (struct sunxi_hdmi_reg *)SUNXI_HDMI_BASE; @@ -734,7 +747,7 @@ static void sunxi_hdmi_mode_set(const struct ctfb_res_modes *mode, /* Write clear interrupt status bits */ writel(SUNXI_HDMI_IRQ_STATUS_BITS, &hdmi->irq);
- if (sunxi_display.monitor == sunxi_monitor_hdmi) + if (monitor == sunxi_monitor_hdmi) sunxi_hdmi_setup_info_frames(mode);
/* Set input sync enable */ @@ -789,7 +802,7 @@ static void sunxi_hdmi_enable(void)
#if defined CONFIG_VIDEO_VGA || defined CONFIG_VIDEO_COMPOSITE
-static void sunxi_tvencoder_mode_set(void) +static void sunxi_tvencoder_mode_set(enum sunxi_monitor monitor) { struct sunxi_ccm_reg * const ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; @@ -801,7 +814,7 @@ static void sunxi_tvencoder_mode_set(void) /* Clock on */ setbits_le32(&ccm->ahb_gate1, 1 << AHB_GATE_OFFSET_TVE0);
- switch (sunxi_display.monitor) { + switch (monitor) { case sunxi_monitor_vga: tvencoder_mode_set(tve, tve_mode_vga); break; @@ -896,26 +909,28 @@ static void sunxi_engines_init(void) sunxi_drc_init(); }
-static void sunxi_mode_set(const struct ctfb_res_modes *mode, +static void sunxi_mode_set(struct sunxi_display *sunxi_display, + const struct ctfb_res_modes *mode, unsigned int address) { + enum sunxi_monitor monitor = sunxi_display->monitor; int __maybe_unused clk_div, clk_double; struct sunxi_lcdc_reg * const lcdc = (struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE; struct sunxi_tve_reg * __maybe_unused const tve = (struct sunxi_tve_reg *)SUNXI_TVE0_BASE;
- switch (sunxi_display.monitor) { + switch (sunxi_display->monitor) { case sunxi_monitor_none: break; case sunxi_monitor_dvi: case sunxi_monitor_hdmi: #ifdef CONFIG_VIDEO_HDMI - sunxi_composer_mode_set(mode, address); - sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 0); - sunxi_hdmi_mode_set(mode, clk_div, clk_double); + sunxi_composer_mode_set(mode, address, monitor); + sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 0, monitor); + sunxi_hdmi_mode_set(mode, clk_div, clk_double, monitor); sunxi_composer_enable(); - lcdc_enable(lcdc, sunxi_display.depth); + lcdc_enable(lcdc, sunxi_display->depth); sunxi_hdmi_enable(); #endif break; @@ -930,7 +945,7 @@ static void sunxi_mode_set(const struct ctfb_res_modes *mode, axp_set_eldo(3, 1800); anx9804_init(CONFIG_VIDEO_LCD_I2C_BUS, 4, ANX9804_DATA_RATE_1620M, - sunxi_display.depth); + sunxi_display->depth); } if (IS_ENABLED(CONFIG_VIDEO_LCD_HITACHI_TX18D42VM)) { mdelay(50); /* Wait for lcd controller power on */ @@ -942,10 +957,10 @@ static void sunxi_mode_set(const struct ctfb_res_modes *mode, i2c_reg_write(0x5c, 0x04, 0x42); /* Turn on the LCD */ i2c_set_bus_num(orig_i2c_bus); } - sunxi_composer_mode_set(mode, address); - sunxi_lcdc_tcon0_mode_set(mode, false); + sunxi_composer_mode_set(mode, address, monitor); + sunxi_lcdc_tcon0_mode_set(sunxi_display, mode, false); sunxi_composer_enable(); - lcdc_enable(lcdc, sunxi_display.depth); + lcdc_enable(lcdc, sunxi_display->depth); #ifdef CONFIG_VIDEO_LCD_SSD2828 sunxi_ssd2828_init(mode); #endif @@ -953,17 +968,17 @@ static void sunxi_mode_set(const struct ctfb_res_modes *mode, break; case sunxi_monitor_vga: #ifdef CONFIG_VIDEO_VGA - sunxi_composer_mode_set(mode, address); - sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 1); - sunxi_tvencoder_mode_set(); + sunxi_composer_mode_set(mode, address, monitor); + sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 1, monitor); + sunxi_tvencoder_mode_set(monitor); sunxi_composer_enable(); - lcdc_enable(lcdc, sunxi_display.depth); + lcdc_enable(lcdc, sunxi_display->depth); tvencoder_enable(tve); #elif defined CONFIG_VIDEO_VGA_VIA_LCD - sunxi_composer_mode_set(mode, address); - sunxi_lcdc_tcon0_mode_set(mode, true); + sunxi_composer_mode_set(mode, address, monitor); + sunxi_lcdc_tcon0_mode_set(sunxi_display, mode, true); sunxi_composer_enable(); - lcdc_enable(lcdc, sunxi_display.depth); + lcdc_enable(lcdc, sunxi_display->depth); sunxi_vga_external_dac_enable(); #endif break; @@ -972,11 +987,11 @@ static void sunxi_mode_set(const struct ctfb_res_modes *mode, case sunxi_monitor_composite_pal_m: case sunxi_monitor_composite_pal_nc: #ifdef CONFIG_VIDEO_COMPOSITE - sunxi_composer_mode_set(mode, address); - sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 0); - sunxi_tvencoder_mode_set(); + sunxi_composer_mode_set(mode, address, monitor); + sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 0, monitor); + sunxi_tvencoder_mode_set(monitor); sunxi_composer_enable(); - lcdc_enable(lcdc, sunxi_display.depth); + lcdc_enable(lcdc, sunxi_display->depth); tvencoder_enable(tve); #endif break; @@ -999,11 +1014,6 @@ static const char *sunxi_get_mon_desc(enum sunxi_monitor monitor) } }
-ulong board_get_usable_ram_top(ulong total_size) -{ - return gd->ram_top - CONFIG_SUNXI_MAX_FB_SIZE; -} - static bool sunxi_has_hdmi(void) { #ifdef CONFIG_VIDEO_HDMI @@ -1052,9 +1062,11 @@ static enum sunxi_monitor sunxi_get_default_mon(bool allow_hdmi) return sunxi_monitor_none; }
-void *video_hw_init(void) +static int sunxi_de_probe(struct udevice *dev) { - static GraphicDevice *graphic_device = &sunxi_display.graphic_device; + struct video_priv *uc_priv = dev_get_uclass_priv(dev); + struct video_uc_plat *plat = dev_get_uclass_plat(dev); + struct sunxi_display *sunxi_display = dev_get_priv(dev); const struct ctfb_res_modes *mode; struct ctfb_res_modes custom; const char *options; @@ -1067,10 +1079,8 @@ void *video_hw_init(void) char mon[16]; char *lcd_mode = CONFIG_VIDEO_LCD_MODE;
- memset(&sunxi_display, 0, sizeof(struct sunxi_display)); - video_get_ctfb_res_modes(RES_MODE_1024x768, 24, &mode, - &sunxi_display.depth, &options); + &sunxi_display->depth, &options); #ifdef CONFIG_VIDEO_HDMI hpd = video_get_option_int(options, "hpd", 1); hpd_delay = video_get_option_int(options, "hpd_delay", 500); @@ -1078,35 +1088,35 @@ void *video_hw_init(void) #endif overscan_x = video_get_option_int(options, "overscan_x", -1); overscan_y = video_get_option_int(options, "overscan_y", -1); - sunxi_display.monitor = sunxi_get_default_mon(true); + sunxi_display->monitor = sunxi_get_default_mon(true); video_get_option_string(options, "monitor", mon, sizeof(mon), - sunxi_get_mon_desc(sunxi_display.monitor)); + sunxi_get_mon_desc(sunxi_display->monitor)); for (i = 0; i <= SUNXI_MONITOR_LAST; i++) { if (strcmp(mon, sunxi_get_mon_desc(i)) == 0) { - sunxi_display.monitor = i; + sunxi_display->monitor = i; break; } } if (i > SUNXI_MONITOR_LAST) printf("Unknown monitor: '%s', falling back to '%s'\n", - mon, sunxi_get_mon_desc(sunxi_display.monitor)); + mon, sunxi_get_mon_desc(sunxi_display->monitor));
#ifdef CONFIG_VIDEO_HDMI /* If HDMI/DVI is selected do HPD & EDID, and handle fallback */ - if (sunxi_display.monitor == sunxi_monitor_dvi || - sunxi_display.monitor == sunxi_monitor_hdmi) { + if (sunxi_display->monitor == sunxi_monitor_dvi || + sunxi_display->monitor == sunxi_monitor_hdmi) { /* Always call hdp_detect, as it also enables clocks, etc. */ hdmi_present = (sunxi_hdmi_hpd_detect(hpd_delay) == 1); if (hdmi_present && edid) { printf("HDMI connected: "); - if (sunxi_hdmi_edid_get_mode(&custom, true) == 0) + if (sunxi_hdmi_edid_get_mode(sunxi_display, &custom, true) == 0) mode = &custom; else hdmi_present = false; } /* Fall back to EDID in case HPD failed */ if (edid && !hdmi_present) { - if (sunxi_hdmi_edid_get_mode(&custom, false) == 0) { + if (sunxi_hdmi_edid_get_mode(sunxi_display, &custom, false) == 0) { mode = &custom; hdmi_present = true; } @@ -1114,38 +1124,39 @@ void *video_hw_init(void) /* Shut down when display was not found */ if ((hpd || edid) && !hdmi_present) { sunxi_hdmi_shutdown(); - sunxi_display.monitor = sunxi_get_default_mon(false); + sunxi_display->monitor = sunxi_get_default_mon(false); } /* else continue with hdmi/dvi without a cable connected */ } #endif
- switch (sunxi_display.monitor) { + switch (sunxi_display->monitor) { case sunxi_monitor_none: - return NULL; + printf("Unknown monitor\n"); + return -EINVAL; case sunxi_monitor_dvi: case sunxi_monitor_hdmi: if (!sunxi_has_hdmi()) { printf("HDMI/DVI not supported on this board\n"); - sunxi_display.monitor = sunxi_monitor_none; - return NULL; + sunxi_display->monitor = sunxi_monitor_none; + return -EINVAL; } break; case sunxi_monitor_lcd: if (!sunxi_has_lcd()) { printf("LCD not supported on this board\n"); - sunxi_display.monitor = sunxi_monitor_none; - return NULL; + sunxi_display->monitor = sunxi_monitor_none; + return -EINVAL; } - sunxi_display.depth = video_get_params(&custom, lcd_mode); + sunxi_display->depth = video_get_params(&custom, lcd_mode); mode = &custom; break; case sunxi_monitor_vga: if (!sunxi_has_vga()) { printf("VGA not supported on this board\n"); - sunxi_display.monitor = sunxi_monitor_none; - return NULL; + sunxi_display->monitor = sunxi_monitor_none; + return -EINVAL; } - sunxi_display.depth = 18; + sunxi_display->depth = 18; break; case sunxi_monitor_composite_pal: case sunxi_monitor_composite_ntsc: @@ -1153,85 +1164,103 @@ void *video_hw_init(void) case sunxi_monitor_composite_pal_nc: if (!sunxi_has_composite()) { printf("Composite video not supported on this board\n"); - sunxi_display.monitor = sunxi_monitor_none; - return NULL; + sunxi_display->monitor = sunxi_monitor_none; + return -EINVAL; } - if (sunxi_display.monitor == sunxi_monitor_composite_pal || - sunxi_display.monitor == sunxi_monitor_composite_pal_nc) + if (sunxi_display->monitor == sunxi_monitor_composite_pal || + sunxi_display->monitor == sunxi_monitor_composite_pal_nc) mode = &composite_video_modes[0]; else mode = &composite_video_modes[1]; - sunxi_display.depth = 24; + sunxi_display->depth = 24; break; }
/* Yes these defaults are quite high, overscan on composite sucks... */ if (overscan_x == -1) - overscan_x = sunxi_is_composite() ? 32 : 0; + overscan_x = sunxi_is_composite(sunxi_display->monitor) ? 32 : 0; if (overscan_y == -1) - overscan_y = sunxi_is_composite() ? 20 : 0; + overscan_y = sunxi_is_composite(sunxi_display->monitor) ? 20 : 0;
- sunxi_display.fb_size = - (mode->xres * mode->yres * 4 + 0xfff) & ~0xfff; + sunxi_display->fb_size = plat->size; overscan_offset = (overscan_y * mode->xres + overscan_x) * 4; /* We want to keep the fb_base for simplefb page aligned, where as * the sunxi dma engines will happily accept an unaligned address. */ if (overscan_offset) - sunxi_display.fb_size += 0x1000; - - if (sunxi_display.fb_size > CONFIG_SUNXI_MAX_FB_SIZE) { - printf("Error need %dkB for fb, but only %dkB is reserved\n", - sunxi_display.fb_size >> 10, - CONFIG_SUNXI_MAX_FB_SIZE >> 10); - return NULL; - } + sunxi_display->fb_size += 0x1000;
printf("Setting up a %dx%d%s %s console (overscan %dx%d)\n", mode->xres, mode->yres, (mode->vmode == FB_VMODE_INTERLACED) ? "i" : "", - sunxi_get_mon_desc(sunxi_display.monitor), + sunxi_get_mon_desc(sunxi_display->monitor), overscan_x, overscan_y);
- gd->fb_base = gd->bd->bi_dram[0].start + - gd->bd->bi_dram[0].size - sunxi_display.fb_size; + sunxi_display->fb_addr = plat->base; sunxi_engines_init();
#ifdef CONFIG_EFI_LOADER - efi_add_memory_map(gd->fb_base, sunxi_display.fb_size, + efi_add_memory_map(sunxi_display->fb_addr, sunxi_display->fb_size, EFI_RESERVED_MEMORY_TYPE); #endif
- fb_dma_addr = gd->fb_base - CONFIG_SYS_SDRAM_BASE; - sunxi_display.fb_addr = gd->fb_base; + fb_dma_addr = sunxi_display->fb_addr - CONFIG_SYS_SDRAM_BASE; if (overscan_offset) { fb_dma_addr += 0x1000 - (overscan_offset & 0xfff); - sunxi_display.fb_addr += (overscan_offset + 0xfff) & ~0xfff; - memset((void *)gd->fb_base, 0, sunxi_display.fb_size); - flush_cache(gd->fb_base, sunxi_display.fb_size); + sunxi_display->fb_addr += (overscan_offset + 0xfff) & ~0xfff; + memset((void *)sunxi_display->fb_addr, 0, sunxi_display->fb_size); + flush_cache(sunxi_display->fb_addr, sunxi_display->fb_size); } - sunxi_mode_set(mode, fb_dma_addr); + sunxi_mode_set(sunxi_display, mode, fb_dma_addr);
/* * These are the only members of this structure that are used. All the * others are driver specific. The pitch is stored in plnSizeX. */ - graphic_device->frameAdrs = sunxi_display.fb_addr; - graphic_device->gdfIndex = GDF_32BIT_X888RGB; - graphic_device->gdfBytesPP = 4; - graphic_device->winSizeX = mode->xres - 2 * overscan_x; - graphic_device->winSizeY = mode->yres - 2 * overscan_y; - graphic_device->plnSizeX = mode->xres * graphic_device->gdfBytesPP; - - return graphic_device; + uc_priv->bpix = VIDEO_BPP32; + uc_priv->xsize = mode->xres; + uc_priv->ysize = mode->yres; + + video_set_flush_dcache(dev, 1); + + return 0; }
+static int sunxi_de_bind(struct udevice *dev) +{ + struct video_uc_plat *plat = dev_get_uclass_plat(dev); + + plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT * + (1 << LCD_MAX_LOG2_BPP) / 8; + + return 0; +} + +static const struct video_ops sunxi_de_ops = { +}; + +U_BOOT_DRIVER(sunxi_de) = { + .name = "sunxi_de", + .id = UCLASS_VIDEO, + .ops = &sunxi_de_ops, + .bind = sunxi_de_bind, + .probe = sunxi_de_probe, + .priv_auto = sizeof(struct sunxi_display), + .flags = DM_FLAG_PRE_RELOC, +}; + +U_BOOT_DRVINFO(sunxi_de) = { + .name = "sunxi_de" +}; + /* * Simplefb support. */ #if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_VIDEO_DT_SIMPLEFB) int sunxi_simplefb_setup(void *blob) { - static GraphicDevice *graphic_device = &sunxi_display.graphic_device; + struct sunxi_display *sunxi_display; + struct video_priv *uc_priv; + struct udevice *de; int offset, ret; u64 start, size; const char *pipeline = NULL; @@ -1242,7 +1271,19 @@ int sunxi_simplefb_setup(void *blob) #define PIPELINE_PREFIX #endif
- switch (sunxi_display.monitor) { + ret = uclass_find_device_by_name(UCLASS_VIDEO, "sunxi_de", &de); + if (ret) { + printf("DE not present\n"); + return 0; + } else if (!device_active(de)) { + printf("DE is present but not probed\n"); + return 0; + } + + uc_priv = dev_get_uclass_priv(de); + sunxi_display = dev_get_priv(de); + + switch (sunxi_display->monitor) { case sunxi_monitor_none: return 0; case sunxi_monitor_dvi: @@ -1280,16 +1321,17 @@ int sunxi_simplefb_setup(void *blob) * linux/arch/arm/mm/ioremap.c around line 301. */ start = gd->bd->bi_dram[0].start; - size = gd->bd->bi_dram[0].size - sunxi_display.fb_size; + size = sunxi_display->fb_addr - start; ret = fdt_fixup_memory_banks(blob, &start, &size, 1); if (ret) { eprintf("Cannot setup simplefb: Error reserving memory\n"); return ret; }
- ret = fdt_setup_simplefb_node(blob, offset, sunxi_display.fb_addr, - graphic_device->winSizeX, graphic_device->winSizeY, - graphic_device->plnSizeX, "x8r8g8b8"); + ret = fdt_setup_simplefb_node(blob, offset, sunxi_display->fb_addr, + uc_priv->xsize, uc_priv->ysize, + VNBYTES(uc_priv->bpix) * uc_priv->xsize, + "x8r8g8b8"); if (ret) eprintf("Cannot setup simplefb: Error setting properties\n");
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 000f3864702..97661329ba4 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -256,23 +256,6 @@ extern int soft_i2c_gpio_scl; #endif #endif /* ifdef CONFIG_REQUIRE_SERIAL_CONSOLE */
-#ifdef CONFIG_VIDEO_SUNXI -/* - * The amount of RAM to keep free at the top of RAM when relocating u-boot, - * to use as framebuffer. This must be a multiple of 4096. - */ -#define CONFIG_SUNXI_MAX_FB_SIZE (16 << 20) - -#define CONFIG_VIDEO_LOGO -#define CONFIG_VIDEO_STD_TIMINGS -#define CONFIG_I2C_EDID -#define VIDEO_LINE_LEN (pGD->plnSizeX) - -/* allow both serial and cfb console. */ -/* stop x86 thinking in cfbconsole from trying to init a pc keyboard */ - -#endif /* CONFIG_VIDEO_SUNXI */ - /* Ethernet support */
#ifdef CONFIG_USB_EHCI_HCD diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt index c6a83124956..ed469f74034 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -1661,7 +1661,6 @@ CONFIG_STV0991 CONFIG_STV0991_HZ CONFIG_STV0991_HZ_CLOCK CONFIG_ST_SMI -CONFIG_SUNXI_MAX_FB_SIZE CONFIG_SXNI855T CONFIG_SYSFLAGS_ADDR CONFIG_SYSFS
On Fri, Feb 05, 2021 at 01:07:48AM +0000, Andre Przywara 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
Acked-by: Maxime Ripard mripard@kernel.org
Thanks! Maxime
Dne petek, 05. februar 2021 ob 02:07:48 CET je Andre Przywara napisal(a):
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(-)
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
- select DISPLAY imply VIDEO_DT_SIMPLEFB default y ---help---
- Say Y here to add support for using a cfb console on the HDMI, LCD
- or VGA output found on most sunxi devices. See doc/README.video
for
- info on how to select the video output and mode.
- Say Y here to add support for using a graphical console on the
HDMI,
- LCD or VGA output found on older sunxi devices. This will also
provide
- a simple_framebuffer device for Linux.
config VIDEO_HDMI bool "HDMI output support"
<snip>
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 000f3864702..97661329ba4 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -256,23 +256,6 @@ extern int soft_i2c_gpio_scl; #endif #endif /* ifdef CONFIG_REQUIRE_SERIAL_CONSOLE */
-#ifdef CONFIG_VIDEO_SUNXI -/*
- The amount of RAM to keep free at the top of RAM when relocating u-boot,
- to use as framebuffer. This must be a multiple of 4096.
- */
-#define CONFIG_SUNXI_MAX_FB_SIZE (16 << 20)
-#define CONFIG_VIDEO_LOGO -#define CONFIG_VIDEO_STD_TIMINGS -#define CONFIG_I2C_EDID -#define VIDEO_LINE_LEN (pGD->plnSizeX)
-/* allow both serial and cfb console. */ -/* stop x86 thinking in cfbconsole from trying to init a pc keyboard */
-#endif /* CONFIG_VIDEO_SUNXI */
/* Ethernet support */
#ifdef CONFIG_USB_EHCI_HCD
This file has another ifdef which can be removed (#ifdef CONFIG_VIDEO).
After that, Reviewed-by: Jernej Skrabec jernej.skrabec@siol.net
Best regards, Jernej
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt index c6a83124956..ed469f74034 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -1661,7 +1661,6 @@ CONFIG_STV0991 CONFIG_STV0991_HZ CONFIG_STV0991_HZ_CLOCK CONFIG_ST_SMI -CONFIG_SUNXI_MAX_FB_SIZE CONFIG_SXNI855T CONFIG_SYSFLAGS_ADDR CONFIG_SYSFS -- 2.17.5
On Fri, 05 Feb 2021 17:27:25 +0100 Jernej Škrabec jernej.skrabec@siol.net wrote:
Dne petek, 05. februar 2021 ob 02:07:48 CET je Andre Przywara napisal(a):
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(-)
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
- select DISPLAY imply VIDEO_DT_SIMPLEFB default y ---help---
- Say Y here to add support for using a cfb console on the HDMI, LCD
- or VGA output found on most sunxi devices. See doc/README.video
for
- info on how to select the video output and mode.
- Say Y here to add support for using a graphical console on the
HDMI,
- LCD or VGA output found on older sunxi devices. This will also
provide
- a simple_framebuffer device for Linux.
config VIDEO_HDMI bool "HDMI output support"
<snip>
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 000f3864702..97661329ba4 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -256,23 +256,6 @@ extern int soft_i2c_gpio_scl; #endif #endif /* ifdef CONFIG_REQUIRE_SERIAL_CONSOLE */
-#ifdef CONFIG_VIDEO_SUNXI -/*
- The amount of RAM to keep free at the top of RAM when relocating u-boot,
- to use as framebuffer. This must be a multiple of 4096.
- */
-#define CONFIG_SUNXI_MAX_FB_SIZE (16 << 20)
-#define CONFIG_VIDEO_LOGO -#define CONFIG_VIDEO_STD_TIMINGS -#define CONFIG_I2C_EDID -#define VIDEO_LINE_LEN (pGD->plnSizeX)
-/* allow both serial and cfb console. */ -/* stop x86 thinking in cfbconsole from trying to init a pc keyboard */
-#endif /* CONFIG_VIDEO_SUNXI */
/* Ethernet support */
#ifdef CONFIG_USB_EHCI_HCD
This file has another ifdef which can be removed (#ifdef CONFIG_VIDEO).
Ah, indeed, thanks for pointing this out.
I think they are even more cleanups lurking in the shadows ;-)
After that, Reviewed-by: Jernej Skrabec jernej.skrabec@siol.net
Thanks! Andre
Best regards, Jernej
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt index c6a83124956..ed469f74034 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -1661,7 +1661,6 @@ CONFIG_STV0991 CONFIG_STV0991_HZ CONFIG_STV0991_HZ_CLOCK CONFIG_ST_SMI -CONFIG_SUNXI_MAX_FB_SIZE CONFIG_SXNI855T CONFIG_SYSFLAGS_ADDR CONFIG_SYSFS -- 2.17.5
Hi Andre,
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
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 ?
select DISPLAY imply VIDEO_DT_SIMPLEFB default y ---help---
Say Y here to add support for using a cfb console on the HDMI, LCDor VGA output found on most sunxi devices. See doc/README.video forinfo on how to select the video output and mode.
Say Y here to add support for using a graphical console on the HDMI,LCD or VGA output found on older sunxi devices. This will also providea simple_framebuffer device for Linux.config VIDEO_HDMI bool "HDMI output support" diff --git a/drivers/video/sunxi/sunxi_display.c b/drivers/video/sunxi/sunxi_display.c index f52aba4d21c..61498d1642f 100644 --- a/drivers/video/sunxi/sunxi_display.c +++ b/drivers/video/sunxi/sunxi_display.c @@ -7,6 +7,8 @@ */
#include <common.h> +#include <display.h> +#include <dm.h> #include <cpu_func.h> #include <efi_loader.h> #include <init.h> @@ -28,7 +30,9 @@ #include <fdt_support.h> #include <i2c.h> #include <malloc.h> +#include <video.h> #include <video_fb.h> +#include <dm/uclass-internal.h>
Do you need that? Internal things should be avoided if posssible.
#include "../videomodes.h" #include "../anx9804.h" #include "../hitachi_tx18d42vm_lcd.h" @@ -45,6 +49,11 @@
DECLARE_GLOBAL_DATA_PTR;
+/* Maximum LCD size we support */ +#define LCD_MAX_WIDTH 3840 +#define LCD_MAX_HEIGHT 2160 +#define LCD_MAX_LOG2_BPP VIDEO_BPP32
enum sunxi_monitor { sunxi_monitor_none, sunxi_monitor_dvi, @@ -59,12 +68,11 @@ enum sunxi_monitor { #define SUNXI_MONITOR_LAST sunxi_monitor_composite_pal_nc
struct sunxi_display {
GraphicDevice graphic_device; enum sunxi_monitor monitor; unsigned int depth; unsigned int fb_addr; unsigned int fb_size;
These last three are repeated from the uclass info. But fb_addr seems to sometimes be different from the ucass frame buffer, in which case I wonder how output actually works.
If you do actually need these, can you please document them here?
-} sunxi_display; +};
const struct ctfb_res_modes composite_video_modes[2] = { /* x y hz pixclk ps/kHz le ri up lo hs vs s vmode */
[..]
-static void sunxi_mode_set(const struct ctfb_res_modes *mode, +static void sunxi_mode_set(struct sunxi_display *sunxi_display,
const struct ctfb_res_modes *mode, unsigned int address){
enum sunxi_monitor monitor = sunxi_display->monitor; int __maybe_unused clk_div, clk_double; struct sunxi_lcdc_reg * const lcdc = (struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE; struct sunxi_tve_reg * __maybe_unused const tve = (struct sunxi_tve_reg *)SUNXI_TVE0_BASE;
switch (sunxi_display.monitor) {
switch (sunxi_display->monitor) { case sunxi_monitor_none: break; case sunxi_monitor_dvi: case sunxi_monitor_hdmi:#ifdef CONFIG_VIDEO_HDMI
I wonder if all of these can use IS_ENABLED()?
sunxi_composer_mode_set(mode, address);sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 0);sunxi_hdmi_mode_set(mode, clk_div, clk_double);
sunxi_composer_mode_set(mode, address, monitor);sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 0, monitor);sunxi_hdmi_mode_set(mode, clk_div, clk_double, monitor); sunxi_composer_enable();
lcdc_enable(lcdc, sunxi_display.depth);
lcdc_enable(lcdc, sunxi_display->depth); sunxi_hdmi_enable();#endif break;
[..]
-void *video_hw_init(void) +static int sunxi_de_probe(struct udevice *dev)
This function is way too long and would benefit from splitting into several parts IMO.
{
static GraphicDevice *graphic_device = &sunxi_display.graphic_device;
struct video_priv *uc_priv = dev_get_uclass_priv(dev);struct video_uc_plat *plat = dev_get_uclass_plat(dev);struct sunxi_display *sunxi_display = dev_get_priv(dev); const struct ctfb_res_modes *mode; struct ctfb_res_modes custom; const char *options;@@ -1067,10 +1079,8 @@ void *video_hw_init(void) char mon[16]; char *lcd_mode = CONFIG_VIDEO_LCD_MODE;
memset(&sunxi_display, 0, sizeof(struct sunxi_display));video_get_ctfb_res_modes(RES_MODE_1024x768, 24, &mode,&sunxi_display.depth, &options);
&sunxi_display->depth, &options);#ifdef CONFIG_VIDEO_HDMI hpd = video_get_option_int(options, "hpd", 1); hpd_delay = video_get_option_int(options, "hpd_delay", 500); @@ -1078,35 +1088,35 @@ void *video_hw_init(void) #endif overscan_x = video_get_option_int(options, "overscan_x", -1); overscan_y = video_get_option_int(options, "overscan_y", -1);
sunxi_display.monitor = sunxi_get_default_mon(true);
sunxi_display->monitor = sunxi_get_default_mon(true); video_get_option_string(options, "monitor", mon, sizeof(mon),
sunxi_get_mon_desc(sunxi_display.monitor));
sunxi_get_mon_desc(sunxi_display->monitor)); for (i = 0; i <= SUNXI_MONITOR_LAST; i++) { if (strcmp(mon, sunxi_get_mon_desc(i)) == 0) {
sunxi_display.monitor = i;
sunxi_display->monitor = i; break; } } if (i > SUNXI_MONITOR_LAST) printf("Unknown monitor: '%s', falling back to '%s'\n",
mon, sunxi_get_mon_desc(sunxi_display.monitor));
mon, sunxi_get_mon_desc(sunxi_display->monitor));#ifdef CONFIG_VIDEO_HDMI /* If HDMI/DVI is selected do HPD & EDID, and handle fallback */
if (sunxi_display.monitor == sunxi_monitor_dvi ||sunxi_display.monitor == sunxi_monitor_hdmi) {
if (sunxi_display->monitor == sunxi_monitor_dvi ||sunxi_display->monitor == sunxi_monitor_hdmi) { /* Always call hdp_detect, as it also enables clocks, etc. */ hdmi_present = (sunxi_hdmi_hpd_detect(hpd_delay) == 1); if (hdmi_present && edid) { printf("HDMI connected: ");
if (sunxi_hdmi_edid_get_mode(&custom, true) == 0)
if (sunxi_hdmi_edid_get_mode(sunxi_display, &custom, true) == 0) mode = &custom; else hdmi_present = false; } /* Fall back to EDID in case HPD failed */ if (edid && !hdmi_present) {
if (sunxi_hdmi_edid_get_mode(&custom, false) == 0) {
if (sunxi_hdmi_edid_get_mode(sunxi_display, &custom, false) == 0) { mode = &custom; hdmi_present = true; }@@ -1114,38 +1124,39 @@ void *video_hw_init(void) /* Shut down when display was not found */ if ((hpd || edid) && !hdmi_present) { sunxi_hdmi_shutdown();
sunxi_display.monitor = sunxi_get_default_mon(false);
sunxi_display->monitor = sunxi_get_default_mon(false); } /* else continue with hdmi/dvi without a cable connected */ }#endif
switch (sunxi_display.monitor) {
switch (sunxi_display->monitor) { case sunxi_monitor_none:
return NULL;
printf("Unknown monitor\n");return -EINVAL; case sunxi_monitor_dvi: case sunxi_monitor_hdmi: if (!sunxi_has_hdmi()) { printf("HDMI/DVI not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; } break; case sunxi_monitor_lcd: if (!sunxi_has_lcd()) { printf("LCD not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; }
sunxi_display.depth = video_get_params(&custom, lcd_mode);
sunxi_display->depth = video_get_params(&custom, lcd_mode); mode = &custom; break; case sunxi_monitor_vga: if (!sunxi_has_vga()) { printf("VGA not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; }
sunxi_display.depth = 18;
sunxi_display->depth = 18; break; case sunxi_monitor_composite_pal: case sunxi_monitor_composite_ntsc:@@ -1153,85 +1164,103 @@ void *video_hw_init(void) case sunxi_monitor_composite_pal_nc: if (!sunxi_has_composite()) { printf("Composite video not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; }
if (sunxi_display.monitor == sunxi_monitor_composite_pal ||sunxi_display.monitor == sunxi_monitor_composite_pal_nc)
if (sunxi_display->monitor == sunxi_monitor_composite_pal ||sunxi_display->monitor == sunxi_monitor_composite_pal_nc) mode = &composite_video_modes[0]; else mode = &composite_video_modes[1];
sunxi_display.depth = 24;
sunxi_display->depth = 24; break; } /* Yes these defaults are quite high, overscan on composite sucks... */ if (overscan_x == -1)
overscan_x = sunxi_is_composite() ? 32 : 0;
overscan_x = sunxi_is_composite(sunxi_display->monitor) ? 32 : 0; if (overscan_y == -1)
overscan_y = sunxi_is_composite() ? 20 : 0;
overscan_y = sunxi_is_composite(sunxi_display->monitor) ? 20 : 0;
sunxi_display.fb_size =(mode->xres * mode->yres * 4 + 0xfff) & ~0xfff;
sunxi_display->fb_size = plat->size; overscan_offset = (overscan_y * mode->xres + overscan_x) * 4; /* We want to keep the fb_base for simplefb page aligned, where as * the sunxi dma engines will happily accept an unaligned address. */ if (overscan_offset)
sunxi_display.fb_size += 0x1000;if (sunxi_display.fb_size > CONFIG_SUNXI_MAX_FB_SIZE) {printf("Error need %dkB for fb, but only %dkB is reserved\n",sunxi_display.fb_size >> 10,CONFIG_SUNXI_MAX_FB_SIZE >> 10);return NULL;}
sunxi_display->fb_size += 0x1000; printf("Setting up a %dx%d%s %s console (overscan %dx%d)\n", mode->xres, mode->yres, (mode->vmode == FB_VMODE_INTERLACED) ? "i" : "",
sunxi_get_mon_desc(sunxi_display.monitor),
sunxi_get_mon_desc(sunxi_display->monitor), overscan_x, overscan_y);
gd->fb_base = gd->bd->bi_dram[0].start +gd->bd->bi_dram[0].size - sunxi_display.fb_size;
sunxi_display->fb_addr = plat->base; sunxi_engines_init();#ifdef CONFIG_EFI_LOADER
efi_add_memory_map(gd->fb_base, sunxi_display.fb_size,
efi_add_memory_map(sunxi_display->fb_addr, sunxi_display->fb_size, EFI_RESERVED_MEMORY_TYPE);#endif
fb_dma_addr = gd->fb_base - CONFIG_SYS_SDRAM_BASE;sunxi_display.fb_addr = gd->fb_base;
fb_dma_addr = sunxi_display->fb_addr - CONFIG_SYS_SDRAM_BASE; if (overscan_offset) { fb_dma_addr += 0x1000 - (overscan_offset & 0xfff);
sunxi_display.fb_addr += (overscan_offset + 0xfff) & ~0xfff;memset((void *)gd->fb_base, 0, sunxi_display.fb_size);flush_cache(gd->fb_base, sunxi_display.fb_size);
sunxi_display->fb_addr += (overscan_offset + 0xfff) & ~0xfff;
ALIGN(overscan_offset, 0x100) ?
memset((void *)sunxi_display->fb_addr, 0, sunxi_display->fb_size);flush_cache(sunxi_display->fb_addr, sunxi_display->fb_size);
Driver model clears the frame buffer. Is this needed?
}
sunxi_mode_set(mode, fb_dma_addr);
sunxi_mode_set(sunxi_display, mode, fb_dma_addr); /* * These are the only members of this structure that are used. All the * others are driver specific. The pitch is stored in plnSizeX. */
graphic_device->frameAdrs = sunxi_display.fb_addr;graphic_device->gdfIndex = GDF_32BIT_X888RGB;graphic_device->gdfBytesPP = 4;graphic_device->winSizeX = mode->xres - 2 * overscan_x;graphic_device->winSizeY = mode->yres - 2 * overscan_y;graphic_device->plnSizeX = mode->xres * graphic_device->gdfBytesPP;return graphic_device;
uc_priv->bpix = VIDEO_BPP32;uc_priv->xsize = mode->xres;uc_priv->ysize = mode->yres;video_set_flush_dcache(dev, 1);
true
return 0;}
+static int sunxi_de_bind(struct udevice *dev) +{
struct video_uc_plat *plat = dev_get_uclass_plat(dev);plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT *(1 << LCD_MAX_LOG2_BPP) / 8;
Should use enum video_log2_bpp here. Also see VNBYTES().
return 0;+}
+static const struct video_ops sunxi_de_ops = { +};
+U_BOOT_DRIVER(sunxi_de) = {
.name = "sunxi_de",.id = UCLASS_VIDEO,.ops = &sunxi_de_ops,.bind = sunxi_de_bind,.probe = sunxi_de_probe,.priv_auto = sizeof(struct sunxi_display),
Should ideally have an _priv suffix
.flags = DM_FLAG_PRE_RELOC,+};
+U_BOOT_DRVINFO(sunxi_de) = {
.name = "sunxi_de"+};
[..]
Regards, Simon
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.
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?
Also: with CONFIG_VIDEO now dying, CONFIG_DM_VIDEO somewhat loses its purpose, doesn't it?
select DISPLAY imply VIDEO_DT_SIMPLEFB default y ---help---
Say Y here to add support for using a cfb console on the HDMI, LCDor VGA output found on most sunxi devices. See doc/README.video forinfo on how to select the video output and mode.
Say Y here to add support for using a graphical console on the HDMI,LCD or VGA output found on older sunxi devices. This will also providea simple_framebuffer device for Linux.config VIDEO_HDMI bool "HDMI output support" diff --git a/drivers/video/sunxi/sunxi_display.c b/drivers/video/sunxi/sunxi_display.c index f52aba4d21c..61498d1642f 100644 --- a/drivers/video/sunxi/sunxi_display.c +++ b/drivers/video/sunxi/sunxi_display.c @@ -7,6 +7,8 @@ */
#include <common.h> +#include <display.h> +#include <dm.h> #include <cpu_func.h> #include <efi_loader.h> #include <init.h> @@ -28,7 +30,9 @@ #include <fdt_support.h> #include <i2c.h> #include <malloc.h> +#include <video.h> #include <video_fb.h> +#include <dm/uclass-internal.h>
Do you need that? Internal things should be avoided if posssible.
#include "../videomodes.h" #include "../anx9804.h" #include "../hitachi_tx18d42vm_lcd.h" @@ -45,6 +49,11 @@
DECLARE_GLOBAL_DATA_PTR;
+/* Maximum LCD size we support */ +#define LCD_MAX_WIDTH 3840 +#define LCD_MAX_HEIGHT 2160 +#define LCD_MAX_LOG2_BPP VIDEO_BPP32
enum sunxi_monitor { sunxi_monitor_none, sunxi_monitor_dvi, @@ -59,12 +68,11 @@ enum sunxi_monitor { #define SUNXI_MONITOR_LAST sunxi_monitor_composite_pal_nc
struct sunxi_display {
GraphicDevice graphic_device; enum sunxi_monitor monitor; unsigned int depth; unsigned int fb_addr; unsigned int fb_size;These last three are repeated from the uclass info. But fb_addr seems to sometimes be different from the ucass frame buffer, in which case I wonder how output actually works.
If you do actually need these, can you please document them here?
-} sunxi_display; +};
const struct ctfb_res_modes composite_video_modes[2] = { /* x y hz pixclk ps/kHz le ri up lo hs vs s vmode */
[..]
-static void sunxi_mode_set(const struct ctfb_res_modes *mode, +static void sunxi_mode_set(struct sunxi_display *sunxi_display,
const struct ctfb_res_modes *mode, unsigned int address){
enum sunxi_monitor monitor = sunxi_display->monitor; int __maybe_unused clk_div, clk_double; struct sunxi_lcdc_reg * const lcdc = (struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE; struct sunxi_tve_reg * __maybe_unused const tve = (struct sunxi_tve_reg *)SUNXI_TVE0_BASE;
switch (sunxi_display.monitor) {
switch (sunxi_display->monitor) { case sunxi_monitor_none: break; case sunxi_monitor_dvi: case sunxi_monitor_hdmi:#ifdef CONFIG_VIDEO_HDMI
I wonder if all of these can use IS_ENABLED()?
Yes, in fact I have patches to replace all #ifdefs with IS_ENABLED(), except the sun6i reset gate ones (which would break compilation on non-sun6i). And those should be tackled by proper UCLASS_CLK usage. But I wanted to get this basic conversion out first, see above.
The rest of your comments make sense on the first glance, I will try to fix the easy things for a v4.
Cheers, Andre
sunxi_composer_mode_set(mode, address);sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 0);sunxi_hdmi_mode_set(mode, clk_div, clk_double);
sunxi_composer_mode_set(mode, address, monitor);sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 0, monitor);sunxi_hdmi_mode_set(mode, clk_div, clk_double, monitor); sunxi_composer_enable();
lcdc_enable(lcdc, sunxi_display.depth);
lcdc_enable(lcdc, sunxi_display->depth); sunxi_hdmi_enable();#endif break;
[..]
-void *video_hw_init(void) +static int sunxi_de_probe(struct udevice *dev)
This function is way too long and would benefit from splitting into several parts IMO.
{
static GraphicDevice *graphic_device = &sunxi_display.graphic_device;
struct video_priv *uc_priv = dev_get_uclass_priv(dev);struct video_uc_plat *plat = dev_get_uclass_plat(dev);struct sunxi_display *sunxi_display = dev_get_priv(dev); const struct ctfb_res_modes *mode; struct ctfb_res_modes custom; const char *options;@@ -1067,10 +1079,8 @@ void *video_hw_init(void) char mon[16]; char *lcd_mode = CONFIG_VIDEO_LCD_MODE;
memset(&sunxi_display, 0, sizeof(struct sunxi_display));video_get_ctfb_res_modes(RES_MODE_1024x768, 24, &mode,&sunxi_display.depth, &options);
&sunxi_display->depth, &options);#ifdef CONFIG_VIDEO_HDMI hpd = video_get_option_int(options, "hpd", 1); hpd_delay = video_get_option_int(options, "hpd_delay", 500); @@ -1078,35 +1088,35 @@ void *video_hw_init(void) #endif overscan_x = video_get_option_int(options, "overscan_x", -1); overscan_y = video_get_option_int(options, "overscan_y", -1);
sunxi_display.monitor = sunxi_get_default_mon(true);
sunxi_display->monitor = sunxi_get_default_mon(true); video_get_option_string(options, "monitor", mon, sizeof(mon),
sunxi_get_mon_desc(sunxi_display.monitor));
sunxi_get_mon_desc(sunxi_display->monitor)); for (i = 0; i <= SUNXI_MONITOR_LAST; i++) { if (strcmp(mon, sunxi_get_mon_desc(i)) == 0) {
sunxi_display.monitor = i;
sunxi_display->monitor = i; break; } } if (i > SUNXI_MONITOR_LAST) printf("Unknown monitor: '%s', falling back to '%s'\n",
mon, sunxi_get_mon_desc(sunxi_display.monitor));
mon, sunxi_get_mon_desc(sunxi_display->monitor));#ifdef CONFIG_VIDEO_HDMI /* If HDMI/DVI is selected do HPD & EDID, and handle fallback */
if (sunxi_display.monitor == sunxi_monitor_dvi ||sunxi_display.monitor == sunxi_monitor_hdmi) {
if (sunxi_display->monitor == sunxi_monitor_dvi ||sunxi_display->monitor == sunxi_monitor_hdmi) { /* Always call hdp_detect, as it also enables clocks, etc. */ hdmi_present = (sunxi_hdmi_hpd_detect(hpd_delay) == 1); if (hdmi_present && edid) { printf("HDMI connected: ");
if (sunxi_hdmi_edid_get_mode(&custom, true) == 0)
if (sunxi_hdmi_edid_get_mode(sunxi_display, &custom, true) == 0) mode = &custom; else hdmi_present = false; } /* Fall back to EDID in case HPD failed */ if (edid && !hdmi_present) {
if (sunxi_hdmi_edid_get_mode(&custom, false) == 0) {
if (sunxi_hdmi_edid_get_mode(sunxi_display, &custom, false) == 0) { mode = &custom; hdmi_present = true; }@@ -1114,38 +1124,39 @@ void *video_hw_init(void) /* Shut down when display was not found */ if ((hpd || edid) && !hdmi_present) { sunxi_hdmi_shutdown();
sunxi_display.monitor = sunxi_get_default_mon(false);
sunxi_display->monitor = sunxi_get_default_mon(false); } /* else continue with hdmi/dvi without a cable connected */ }#endif
switch (sunxi_display.monitor) {
switch (sunxi_display->monitor) { case sunxi_monitor_none:
return NULL;
printf("Unknown monitor\n");return -EINVAL; case sunxi_monitor_dvi: case sunxi_monitor_hdmi: if (!sunxi_has_hdmi()) { printf("HDMI/DVI not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; } break; case sunxi_monitor_lcd: if (!sunxi_has_lcd()) { printf("LCD not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; }
sunxi_display.depth = video_get_params(&custom, lcd_mode);
sunxi_display->depth = video_get_params(&custom, lcd_mode); mode = &custom; break; case sunxi_monitor_vga: if (!sunxi_has_vga()) { printf("VGA not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; }
sunxi_display.depth = 18;
sunxi_display->depth = 18; break; case sunxi_monitor_composite_pal: case sunxi_monitor_composite_ntsc:@@ -1153,85 +1164,103 @@ void *video_hw_init(void) case sunxi_monitor_composite_pal_nc: if (!sunxi_has_composite()) { printf("Composite video not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; }
if (sunxi_display.monitor == sunxi_monitor_composite_pal ||sunxi_display.monitor == sunxi_monitor_composite_pal_nc)
if (sunxi_display->monitor == sunxi_monitor_composite_pal ||sunxi_display->monitor == sunxi_monitor_composite_pal_nc) mode = &composite_video_modes[0]; else mode = &composite_video_modes[1];
sunxi_display.depth = 24;
sunxi_display->depth = 24; break; } /* Yes these defaults are quite high, overscan on composite sucks... */ if (overscan_x == -1)
overscan_x = sunxi_is_composite() ? 32 : 0;
overscan_x = sunxi_is_composite(sunxi_display->monitor) ? 32 : 0; if (overscan_y == -1)
overscan_y = sunxi_is_composite() ? 20 : 0;
overscan_y = sunxi_is_composite(sunxi_display->monitor) ? 20 : 0;
sunxi_display.fb_size =(mode->xres * mode->yres * 4 + 0xfff) & ~0xfff;
sunxi_display->fb_size = plat->size; overscan_offset = (overscan_y * mode->xres + overscan_x) * 4; /* We want to keep the fb_base for simplefb page aligned, where as * the sunxi dma engines will happily accept an unaligned address. */ if (overscan_offset)
sunxi_display.fb_size += 0x1000;if (sunxi_display.fb_size > CONFIG_SUNXI_MAX_FB_SIZE) {printf("Error need %dkB for fb, but only %dkB is reserved\n",sunxi_display.fb_size >> 10,CONFIG_SUNXI_MAX_FB_SIZE >> 10);return NULL;}
sunxi_display->fb_size += 0x1000; printf("Setting up a %dx%d%s %s console (overscan %dx%d)\n", mode->xres, mode->yres, (mode->vmode == FB_VMODE_INTERLACED) ? "i" : "",
sunxi_get_mon_desc(sunxi_display.monitor),
sunxi_get_mon_desc(sunxi_display->monitor), overscan_x, overscan_y);
gd->fb_base = gd->bd->bi_dram[0].start +gd->bd->bi_dram[0].size - sunxi_display.fb_size;
sunxi_display->fb_addr = plat->base; sunxi_engines_init();#ifdef CONFIG_EFI_LOADER
efi_add_memory_map(gd->fb_base, sunxi_display.fb_size,
efi_add_memory_map(sunxi_display->fb_addr, sunxi_display->fb_size, EFI_RESERVED_MEMORY_TYPE);#endif
fb_dma_addr = gd->fb_base - CONFIG_SYS_SDRAM_BASE;sunxi_display.fb_addr = gd->fb_base;
fb_dma_addr = sunxi_display->fb_addr - CONFIG_SYS_SDRAM_BASE; if (overscan_offset) { fb_dma_addr += 0x1000 - (overscan_offset & 0xfff);
sunxi_display.fb_addr += (overscan_offset + 0xfff) & ~0xfff;memset((void *)gd->fb_base, 0, sunxi_display.fb_size);flush_cache(gd->fb_base, sunxi_display.fb_size);
sunxi_display->fb_addr += (overscan_offset + 0xfff) & ~0xfff;ALIGN(overscan_offset, 0x100) ?
memset((void *)sunxi_display->fb_addr, 0, sunxi_display->fb_size);flush_cache(sunxi_display->fb_addr, sunxi_display->fb_size);Driver model clears the frame buffer. Is this needed?
}
sunxi_mode_set(mode, fb_dma_addr);
sunxi_mode_set(sunxi_display, mode, fb_dma_addr); /* * These are the only members of this structure that are used. All the * others are driver specific. The pitch is stored in plnSizeX. */
graphic_device->frameAdrs = sunxi_display.fb_addr;graphic_device->gdfIndex = GDF_32BIT_X888RGB;graphic_device->gdfBytesPP = 4;graphic_device->winSizeX = mode->xres - 2 * overscan_x;graphic_device->winSizeY = mode->yres - 2 * overscan_y;graphic_device->plnSizeX = mode->xres * graphic_device->gdfBytesPP;return graphic_device;
uc_priv->bpix = VIDEO_BPP32;uc_priv->xsize = mode->xres;uc_priv->ysize = mode->yres;video_set_flush_dcache(dev, 1);true
return 0;}
+static int sunxi_de_bind(struct udevice *dev) +{
struct video_uc_plat *plat = dev_get_uclass_plat(dev);plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT *(1 << LCD_MAX_LOG2_BPP) / 8;Should use enum video_log2_bpp here. Also see VNBYTES().
return 0;+}
+static const struct video_ops sunxi_de_ops = { +};
+U_BOOT_DRIVER(sunxi_de) = {
.name = "sunxi_de",.id = UCLASS_VIDEO,.ops = &sunxi_de_ops,.bind = sunxi_de_bind,.probe = sunxi_de_probe,.priv_auto = sizeof(struct sunxi_display),Should ideally have an _priv suffix
.flags = DM_FLAG_PRE_RELOC,+};
+U_BOOT_DRVINFO(sunxi_de) = {
.name = "sunxi_de"+};
[..]
Regards, Simon
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
On Sun, 7 Feb 2021 07:37:34 -0700 Simon Glass sjg@chromium.org wrote:
Hi Simon,
coming back to this patch, answering to the other comments I skipped over the last time.
In general this patch is the shortest way to get to some kind of DM driver, in many ways it still looks like a non-DM driver on the inside, which shows.
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
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 ?
select DISPLAY imply VIDEO_DT_SIMPLEFB default y ---help---
Say Y here to add support for using a cfb console on the HDMI, LCDor VGA output found on most sunxi devices. See doc/README.video forinfo on how to select the video output and mode.
Say Y here to add support for using a graphical console on the HDMI,LCD or VGA output found on older sunxi devices. This will also providea simple_framebuffer device for Linux.config VIDEO_HDMI bool "HDMI output support" diff --git a/drivers/video/sunxi/sunxi_display.c b/drivers/video/sunxi/sunxi_display.c index f52aba4d21c..61498d1642f 100644 --- a/drivers/video/sunxi/sunxi_display.c +++ b/drivers/video/sunxi/sunxi_display.c @@ -7,6 +7,8 @@ */
#include <common.h> +#include <display.h> +#include <dm.h> #include <cpu_func.h> #include <efi_loader.h> #include <init.h> @@ -28,7 +30,9 @@ #include <fdt_support.h> #include <i2c.h> #include <malloc.h> +#include <video.h> #include <video_fb.h> +#include <dm/uclass-internal.h>
Do you need that? Internal things should be avoided if posssible.
That's a good point. This is needed for uclass_find_device_by_name() down in the simplefb setup function at the very end. This function will be called from a different context (from ft_board_setup() in board.c), and tries to find the (only) instance of this very driver to populate the simplefb DT node accordingly. This is the approach the sunxi_de2.c uses, and back at the time this was seemingly the best way to achieve this. Alternatives I see: 1) Keep a global static variable, pointing to the struct udevice, set in probe(). 2) Use uclass_get_device_by_name() instead, but prevent the double probe by keeping an indication of the probe status. 3) Gather all data needed for the simplefb setup already in probe(), and store them in global variables, to be picked up by sunxi_simplefb_setup() later. 4) as 3), but store the extra data (just the pipeline name to find the right DT node to enable?) in generic uclass storage. This has the potential of making this function platform and board agnostic (so meson could benefit as well?). We could then setup the simplefb in common code.
Any ideas on this? Maybe something else entirely?
#include "../videomodes.h" #include "../anx9804.h" #include "../hitachi_tx18d42vm_lcd.h" @@ -45,6 +49,11 @@
DECLARE_GLOBAL_DATA_PTR;
+/* Maximum LCD size we support */ +#define LCD_MAX_WIDTH 3840 +#define LCD_MAX_HEIGHT 2160 +#define LCD_MAX_LOG2_BPP VIDEO_BPP32
So this resolution is not achievable with a maximum pixel clock of 165 MHz (not even at 24 Hz). Given that the framebuffer reservation is fixed, this wastes quite some memory. Realistically 165 MHz means 1920x1200 (at 60Hz), shall we use that one instead?
enum sunxi_monitor { sunxi_monitor_none, sunxi_monitor_dvi, @@ -59,12 +68,11 @@ enum sunxi_monitor { #define SUNXI_MONITOR_LAST sunxi_monitor_composite_pal_nc
struct sunxi_display {
GraphicDevice graphic_device; enum sunxi_monitor monitor; unsigned int depth; unsigned int fb_addr; unsigned int fb_size;These last three are repeated from the uclass info. But fb_addr seems to sometimes be different from the ucass frame buffer, in which case I wonder how output actually works.
If you do actually need these, can you please document them here?
I think this might be mostly non-DM legacy, but for the composite overscan we tweak the framebuffer address. I haven't fully wrapped my mind around this, though, and ideally we can indeed lose those extra members. I will try to find some device with composite input to test it.
-} sunxi_display; +};
const struct ctfb_res_modes composite_video_modes[2] = { /* x y hz pixclk ps/kHz le ri up lo hs vs s vmode */
[..]
-static void sunxi_mode_set(const struct ctfb_res_modes *mode, +static void sunxi_mode_set(struct sunxi_display *sunxi_display,
const struct ctfb_res_modes *mode, unsigned int address){
enum sunxi_monitor monitor = sunxi_display->monitor; int __maybe_unused clk_div, clk_double; struct sunxi_lcdc_reg * const lcdc = (struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE; struct sunxi_tve_reg * __maybe_unused const tve = (struct sunxi_tve_reg *)SUNXI_TVE0_BASE;
switch (sunxi_display.monitor) {
switch (sunxi_display->monitor) { case sunxi_monitor_none: break; case sunxi_monitor_dvi: case sunxi_monitor_hdmi:#ifdef CONFIG_VIDEO_HDMI
I wonder if all of these can use IS_ENABLED()?
sunxi_composer_mode_set(mode, address);sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 0);sunxi_hdmi_mode_set(mode, clk_div, clk_double);
sunxi_composer_mode_set(mode, address, monitor);sunxi_lcdc_tcon1_mode_set(mode, &clk_div, &clk_double, 0, monitor);sunxi_hdmi_mode_set(mode, clk_div, clk_double, monitor); sunxi_composer_enable();
lcdc_enable(lcdc, sunxi_display.depth);
lcdc_enable(lcdc, sunxi_display->depth); sunxi_hdmi_enable();#endif break;
[..]
-void *video_hw_init(void) +static int sunxi_de_probe(struct udevice *dev)
This function is way too long and would benefit from splitting into several parts IMO.
Yeah, indeed. I started working on this, but it probably won't be part of the next version already.
{
static GraphicDevice *graphic_device = &sunxi_display.graphic_device;
struct video_priv *uc_priv = dev_get_uclass_priv(dev);struct video_uc_plat *plat = dev_get_uclass_plat(dev);struct sunxi_display *sunxi_display = dev_get_priv(dev); const struct ctfb_res_modes *mode; struct ctfb_res_modes custom; const char *options;@@ -1067,10 +1079,8 @@ void *video_hw_init(void) char mon[16]; char *lcd_mode = CONFIG_VIDEO_LCD_MODE;
memset(&sunxi_display, 0, sizeof(struct sunxi_display));video_get_ctfb_res_modes(RES_MODE_1024x768, 24, &mode,&sunxi_display.depth, &options);
&sunxi_display->depth, &options);#ifdef CONFIG_VIDEO_HDMI hpd = video_get_option_int(options, "hpd", 1); hpd_delay = video_get_option_int(options, "hpd_delay", 500); @@ -1078,35 +1088,35 @@ void *video_hw_init(void) #endif overscan_x = video_get_option_int(options, "overscan_x", -1); overscan_y = video_get_option_int(options, "overscan_y", -1);
sunxi_display.monitor = sunxi_get_default_mon(true);
sunxi_display->monitor = sunxi_get_default_mon(true); video_get_option_string(options, "monitor", mon, sizeof(mon),
sunxi_get_mon_desc(sunxi_display.monitor));
sunxi_get_mon_desc(sunxi_display->monitor)); for (i = 0; i <= SUNXI_MONITOR_LAST; i++) { if (strcmp(mon, sunxi_get_mon_desc(i)) == 0) {
sunxi_display.monitor = i;
sunxi_display->monitor = i; break; } } if (i > SUNXI_MONITOR_LAST) printf("Unknown monitor: '%s', falling back to '%s'\n",
mon, sunxi_get_mon_desc(sunxi_display.monitor));
mon, sunxi_get_mon_desc(sunxi_display->monitor));#ifdef CONFIG_VIDEO_HDMI /* If HDMI/DVI is selected do HPD & EDID, and handle fallback */
if (sunxi_display.monitor == sunxi_monitor_dvi ||sunxi_display.monitor == sunxi_monitor_hdmi) {
if (sunxi_display->monitor == sunxi_monitor_dvi ||sunxi_display->monitor == sunxi_monitor_hdmi) { /* Always call hdp_detect, as it also enables clocks, etc. */ hdmi_present = (sunxi_hdmi_hpd_detect(hpd_delay) == 1); if (hdmi_present && edid) { printf("HDMI connected: ");
if (sunxi_hdmi_edid_get_mode(&custom, true) == 0)
if (sunxi_hdmi_edid_get_mode(sunxi_display, &custom, true) == 0) mode = &custom; else hdmi_present = false; } /* Fall back to EDID in case HPD failed */ if (edid && !hdmi_present) {
if (sunxi_hdmi_edid_get_mode(&custom, false) == 0) {
if (sunxi_hdmi_edid_get_mode(sunxi_display, &custom, false) == 0) { mode = &custom; hdmi_present = true; }@@ -1114,38 +1124,39 @@ void *video_hw_init(void) /* Shut down when display was not found */ if ((hpd || edid) && !hdmi_present) { sunxi_hdmi_shutdown();
sunxi_display.monitor = sunxi_get_default_mon(false);
sunxi_display->monitor = sunxi_get_default_mon(false); } /* else continue with hdmi/dvi without a cable connected */ }#endif
switch (sunxi_display.monitor) {
switch (sunxi_display->monitor) { case sunxi_monitor_none:
return NULL;
printf("Unknown monitor\n");return -EINVAL; case sunxi_monitor_dvi: case sunxi_monitor_hdmi: if (!sunxi_has_hdmi()) { printf("HDMI/DVI not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; } break; case sunxi_monitor_lcd: if (!sunxi_has_lcd()) { printf("LCD not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; }
sunxi_display.depth = video_get_params(&custom, lcd_mode);
sunxi_display->depth = video_get_params(&custom, lcd_mode); mode = &custom; break; case sunxi_monitor_vga: if (!sunxi_has_vga()) { printf("VGA not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; }
sunxi_display.depth = 18;
sunxi_display->depth = 18; break; case sunxi_monitor_composite_pal: case sunxi_monitor_composite_ntsc:@@ -1153,85 +1164,103 @@ void *video_hw_init(void) case sunxi_monitor_composite_pal_nc: if (!sunxi_has_composite()) { printf("Composite video not supported on this board\n");
sunxi_display.monitor = sunxi_monitor_none;return NULL;
sunxi_display->monitor = sunxi_monitor_none;return -EINVAL; }
if (sunxi_display.monitor == sunxi_monitor_composite_pal ||sunxi_display.monitor == sunxi_monitor_composite_pal_nc)
if (sunxi_display->monitor == sunxi_monitor_composite_pal ||sunxi_display->monitor == sunxi_monitor_composite_pal_nc) mode = &composite_video_modes[0]; else mode = &composite_video_modes[1];
sunxi_display.depth = 24;
sunxi_display->depth = 24; break; } /* Yes these defaults are quite high, overscan on composite sucks... */ if (overscan_x == -1)
overscan_x = sunxi_is_composite() ? 32 : 0;
overscan_x = sunxi_is_composite(sunxi_display->monitor) ? 32 : 0; if (overscan_y == -1)
overscan_y = sunxi_is_composite() ? 20 : 0;
overscan_y = sunxi_is_composite(sunxi_display->monitor) ? 20 : 0;
sunxi_display.fb_size =(mode->xres * mode->yres * 4 + 0xfff) & ~0xfff;
sunxi_display->fb_size = plat->size; overscan_offset = (overscan_y * mode->xres + overscan_x) * 4; /* We want to keep the fb_base for simplefb page aligned, where as * the sunxi dma engines will happily accept an unaligned address. */ if (overscan_offset)
sunxi_display.fb_size += 0x1000;if (sunxi_display.fb_size > CONFIG_SUNXI_MAX_FB_SIZE) {printf("Error need %dkB for fb, but only %dkB is reserved\n",sunxi_display.fb_size >> 10,CONFIG_SUNXI_MAX_FB_SIZE >> 10);return NULL;}
sunxi_display->fb_size += 0x1000; printf("Setting up a %dx%d%s %s console (overscan %dx%d)\n", mode->xres, mode->yres, (mode->vmode == FB_VMODE_INTERLACED) ? "i" : "",
sunxi_get_mon_desc(sunxi_display.monitor),
sunxi_get_mon_desc(sunxi_display->monitor), overscan_x, overscan_y);
gd->fb_base = gd->bd->bi_dram[0].start +gd->bd->bi_dram[0].size - sunxi_display.fb_size;
sunxi_display->fb_addr = plat->base; sunxi_engines_init();#ifdef CONFIG_EFI_LOADER
efi_add_memory_map(gd->fb_base, sunxi_display.fb_size,
efi_add_memory_map(sunxi_display->fb_addr, sunxi_display->fb_size, EFI_RESERVED_MEMORY_TYPE);#endif
fb_dma_addr = gd->fb_base - CONFIG_SYS_SDRAM_BASE;sunxi_display.fb_addr = gd->fb_base;
fb_dma_addr = sunxi_display->fb_addr - CONFIG_SYS_SDRAM_BASE; if (overscan_offset) { fb_dma_addr += 0x1000 - (overscan_offset & 0xfff);
sunxi_display.fb_addr += (overscan_offset + 0xfff) & ~0xfff;memset((void *)gd->fb_base, 0, sunxi_display.fb_size);flush_cache(gd->fb_base, sunxi_display.fb_size);
sunxi_display->fb_addr += (overscan_offset + 0xfff) & ~0xfff;ALIGN(overscan_offset, 0x100) ?
memset((void *)sunxi_display->fb_addr, 0, sunxi_display->fb_size);flush_cache(sunxi_display->fb_addr, sunxi_display->fb_size);Driver model clears the frame buffer. Is this needed?
This is just when overscan is enabled (for composite video). I *think* it aims to clear the unused overscan area?
}
sunxi_mode_set(mode, fb_dma_addr);
sunxi_mode_set(sunxi_display, mode, fb_dma_addr); /* * These are the only members of this structure that are used. All the * others are driver specific. The pitch is stored in plnSizeX. */
graphic_device->frameAdrs = sunxi_display.fb_addr;graphic_device->gdfIndex = GDF_32BIT_X888RGB;graphic_device->gdfBytesPP = 4;graphic_device->winSizeX = mode->xres - 2 * overscan_x;graphic_device->winSizeY = mode->yres - 2 * overscan_y;graphic_device->plnSizeX = mode->xres * graphic_device->gdfBytesPP;return graphic_device;
uc_priv->bpix = VIDEO_BPP32;uc_priv->xsize = mode->xres;uc_priv->ysize = mode->yres;video_set_flush_dcache(dev, 1);true
return 0;}
+static int sunxi_de_bind(struct udevice *dev) +{
struct video_uc_plat *plat = dev_get_uclass_plat(dev);plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT *(1 << LCD_MAX_LOG2_BPP) / 8;Should use enum video_log2_bpp here. Also see VNBYTES().
LCD_MAX_LOG2_BPP is defined as VIDEO_BPP32 at the very top. Sure will use VNBYTES.
On a related topic: IIUC this is called several times, for a start once before relocation, where it's supposed to give an upper bound? Are subsequent calls then expected to be more precise? Our 32MB frame buffer is *very* generous, for the usual FullHD display we just need 8MB. But we would only know this in probe(), when we have learned the output device and the video modes it supports. So is there a way to restrict (and possibly also move) the framebuffer in probe()?
Cheers, Andre
return 0;+}
+static const struct video_ops sunxi_de_ops = { +};
+U_BOOT_DRIVER(sunxi_de) = {
.name = "sunxi_de",.id = UCLASS_VIDEO,.ops = &sunxi_de_ops,.bind = sunxi_de_bind,.probe = sunxi_de_probe,.priv_auto = sizeof(struct sunxi_display),Should ideally have an _priv suffix
.flags = DM_FLAG_PRE_RELOC,+};
+U_BOOT_DRVINFO(sunxi_de) = {
.name = "sunxi_de"+};
[..]
Regards, Simon
On Sun, Feb 21, 2021 at 12:07:35AM +0000, Andre Przywara wrote:
On Sun, 7 Feb 2021 07:37:34 -0700 Simon Glass sjg@chromium.org wrote:
Hi Simon,
coming back to this patch, answering to the other comments I skipped over the last time.
In general this patch is the shortest way to get to some kind of DM driver, in many ways it still looks like a non-DM driver on the inside, which shows.
For the record, I'm OK with pulling this patch in as-is and improving the situation rather than requiring further changes right now.
On Sat, 20 Feb 2021 19:45:17 -0500 Tom Rini trini@konsulko.com wrote:
On Sun, Feb 21, 2021 at 12:07:35AM +0000, Andre Przywara wrote:
On Sun, 7 Feb 2021 07:37:34 -0700 Simon Glass sjg@chromium.org wrote:
Hi Simon,
coming back to this patch, answering to the other comments I skipped over the last time.
In general this patch is the shortest way to get to some kind of DM driver, in many ways it still looks like a non-DM driver on the inside, which shows.
For the record, I'm OK with pulling this patch in as-is and improving the situation rather than requiring further changes right now.
OK, thanks! I'll queue it and will submit a pull request soon.
-- Anatolij
On Sun, 21 Feb 2021 17:47:53 +0100 Anatolij Gustschin agust@denx.de wrote:
Hi Anatolij,
On Sat, 20 Feb 2021 19:45:17 -0500 Tom Rini trini@konsulko.com wrote:
On Sun, Feb 21, 2021 at 12:07:35AM +0000, Andre Przywara wrote:
On Sun, 7 Feb 2021 07:37:34 -0700 Simon Glass sjg@chromium.org wrote:
Hi Simon,
coming back to this patch, answering to the other comments I skipped over the last time.
In general this patch is the shortest way to get to some kind of DM driver, in many ways it still looks like a non-DM driver on the inside, which shows.
For the record, I'm OK with pulling this patch in as-is and improving the situation rather than requiring further changes right now.
OK, thanks! I'll queue it and will submit a pull request soon.
Thanks for that. I just sent a v4 with some small fixes and all the tags.
And just to be sure: with "queue it" you mean for -next, so the April merge window? Because I would feel less comfortable with merging it now, as this has not been widely tested yet (only on A10 and A20, but not on all those other SoCs, especially in tablets with native LCDs).
Thanks! Andre.
Hi Andre,
On Mon, 22 Feb 2021 00:17:05 +0000 Andre Przywara andre.przywara@arm.com wrote: ...
Thanks for that. I just sent a v4 with some small fixes and all the tags.
And just to be sure: with "queue it" you mean for -next, so the April merge window?
I meant to queue it for my build testing, to merge the patch for v2021.04-rc3.
Because I would feel less comfortable with merging it now, as this has not been widely tested yet (only on A10 and A20, but not on all those other SoCs, especially in tablets with native LCDs).
OK, then I'll postpone it and will merge in the April merge window.
Thanks!
-- Anatolij
Hi Andre,
On Sat, 20 Feb 2021 at 17:08, Andre Przywara andre.przywara@arm.com wrote:
On Sun, 7 Feb 2021 07:37:34 -0700 Simon Glass sjg@chromium.org wrote:
Hi Simon,
coming back to this patch, answering to the other comments I skipped over the last time.
In general this patch is the shortest way to get to some kind of DM driver, in many ways it still looks like a non-DM driver on the inside, which shows.
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
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 ?
select DISPLAY imply VIDEO_DT_SIMPLEFB default y ---help---
Say Y here to add support for using a cfb console on the HDMI, LCDor VGA output found on most sunxi devices. See doc/README.video forinfo on how to select the video output and mode.
Say Y here to add support for using a graphical console on the HDMI,LCD or VGA output found on older sunxi devices. This will also providea simple_framebuffer device for Linux.config VIDEO_HDMI bool "HDMI output support" diff --git a/drivers/video/sunxi/sunxi_display.c b/drivers/video/sunxi/sunxi_display.c index f52aba4d21c..61498d1642f 100644 --- a/drivers/video/sunxi/sunxi_display.c +++ b/drivers/video/sunxi/sunxi_display.c @@ -7,6 +7,8 @@ */
#include <common.h> +#include <display.h> +#include <dm.h> #include <cpu_func.h> #include <efi_loader.h> #include <init.h> @@ -28,7 +30,9 @@ #include <fdt_support.h> #include <i2c.h> #include <malloc.h> +#include <video.h> #include <video_fb.h> +#include <dm/uclass-internal.h>
Do you need that? Internal things should be avoided if posssible.
That's a good point. This is needed for uclass_find_device_by_name() down in the simplefb setup function at the very end. This function will be called from a different context (from ft_board_setup() in board.c), and tries to find the (only) instance of this very driver to populate the simplefb DT node accordingly. This is the approach the sunxi_de2.c uses, and back at the time this was seemingly the best way to achieve this. Alternatives I see:
- Keep a global static variable, pointing to the struct udevice, set in
probe().
Best avoided.
- Use uclass_get_device_by_name() instead, but prevent the double
probe by keeping an indication of the probe status.
Probing a probed device is a nop, so don't worry about that. We do it all the time.
- Gather all data needed for the simplefb setup already in probe(),
and store them in global variables, to be picked up by sunxi_simplefb_setup() later.
Again, best to avoid a global.
- as 3), but store the extra data (just the pipeline name to find the
right DT node to enable?) in generic uclass storage. This has the potential of making this function platform and board agnostic (so meson could benefit as well?). We could then setup the simplefb in common code.
Any ideas on this? Maybe something else entirely?
I think what you have is OK...at present those functions are not called in device context. I don't really have a good solution. You want to avoid probing it just to fill in the data.
BTW it is a bit faster to call device_get_by_driver_info(DM_DRIVER_GET(sunxi_de), &dev)
#include "../videomodes.h" #include "../anx9804.h" #include "../hitachi_tx18d42vm_lcd.h" @@ -45,6 +49,11 @@
DECLARE_GLOBAL_DATA_PTR;
+/* Maximum LCD size we support */ +#define LCD_MAX_WIDTH 3840 +#define LCD_MAX_HEIGHT 2160 +#define LCD_MAX_LOG2_BPP VIDEO_BPP32
So this resolution is not achievable with a maximum pixel clock of 165 MHz (not even at 24 Hz). Given that the framebuffer reservation is fixed, this wastes quite some memory. Realistically 165 MHz means 1920x1200 (at 60Hz), shall we use that one instead?
I'm not sure about that, but I suppose if the maximum is too high you can change it.
enum sunxi_monitor { sunxi_monitor_none, sunxi_monitor_dvi, @@ -59,12 +68,11 @@ enum sunxi_monitor { #define SUNXI_MONITOR_LAST sunxi_monitor_composite_pal_nc
struct sunxi_display {
GraphicDevice graphic_device; enum sunxi_monitor monitor; unsigned int depth; unsigned int fb_addr; unsigned int fb_size;These last three are repeated from the uclass info. But fb_addr seems to sometimes be different from the ucass frame buffer, in which case I wonder how output actually works.
If you do actually need these, can you please document them here?
I think this might be mostly non-DM legacy, but for the composite overscan we tweak the framebuffer address. I haven't fully wrapped my mind around this, though, and ideally we can indeed lose those extra members. I will try to find some device with composite input to test it.
Ah OK. Well yes it needs some docs for the next person :-)
[..]
printf("Setting up a %dx%d%s %s console (overscan %dx%d)\n", mode->xres, mode->yres, (mode->vmode == FB_VMODE_INTERLACED) ? "i" : "",
sunxi_get_mon_desc(sunxi_display.monitor),
sunxi_get_mon_desc(sunxi_display->monitor), overscan_x, overscan_y);
gd->fb_base = gd->bd->bi_dram[0].start +gd->bd->bi_dram[0].size - sunxi_display.fb_size;
sunxi_display->fb_addr = plat->base; sunxi_engines_init();#ifdef CONFIG_EFI_LOADER
efi_add_memory_map(gd->fb_base, sunxi_display.fb_size,
efi_add_memory_map(sunxi_display->fb_addr, sunxi_display->fb_size, EFI_RESERVED_MEMORY_TYPE);#endif
fb_dma_addr = gd->fb_base - CONFIG_SYS_SDRAM_BASE;sunxi_display.fb_addr = gd->fb_base;
fb_dma_addr = sunxi_display->fb_addr - CONFIG_SYS_SDRAM_BASE; if (overscan_offset) { fb_dma_addr += 0x1000 - (overscan_offset & 0xfff);
sunxi_display.fb_addr += (overscan_offset + 0xfff) & ~0xfff;memset((void *)gd->fb_base, 0, sunxi_display.fb_size);flush_cache(gd->fb_base, sunxi_display.fb_size);
sunxi_display->fb_addr += (overscan_offset + 0xfff) & ~0xfff;ALIGN(overscan_offset, 0x100) ?
memset((void *)sunxi_display->fb_addr, 0, sunxi_display->fb_size);flush_cache(sunxi_display->fb_addr, sunxi_display->fb_size);Driver model clears the frame buffer. Is this needed?
This is just when overscan is enabled (for composite video). I *think* it aims to clear the unused overscan area?
In that case I feel that the fb should be allocated large enough for the overscan area (how else could it be). But I'm not sure what is happening here, so perhaps this is needed.
}
sunxi_mode_set(mode, fb_dma_addr);
sunxi_mode_set(sunxi_display, mode, fb_dma_addr); /* * These are the only members of this structure that are used. All the * others are driver specific. The pitch is stored in plnSizeX. */
graphic_device->frameAdrs = sunxi_display.fb_addr;graphic_device->gdfIndex = GDF_32BIT_X888RGB;graphic_device->gdfBytesPP = 4;graphic_device->winSizeX = mode->xres - 2 * overscan_x;graphic_device->winSizeY = mode->yres - 2 * overscan_y;graphic_device->plnSizeX = mode->xres * graphic_device->gdfBytesPP;return graphic_device;
uc_priv->bpix = VIDEO_BPP32;uc_priv->xsize = mode->xres;uc_priv->ysize = mode->yres;video_set_flush_dcache(dev, 1);true
return 0;}
+static int sunxi_de_bind(struct udevice *dev) +{
struct video_uc_plat *plat = dev_get_uclass_plat(dev);plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT *(1 << LCD_MAX_LOG2_BPP) / 8;Should use enum video_log2_bpp here. Also see VNBYTES().
LCD_MAX_LOG2_BPP is defined as VIDEO_BPP32 at the very top. Sure will use VNBYTES.
On a related topic: IIUC this is called several times, for a start once before relocation, where it's supposed to give an upper bound? Are subsequent calls then expected to be more precise? Our 32MB frame buffer is *very* generous, for the usual FullHD display we just need 8MB. But we would only know this in probe(), when we have learned the output device and the video modes it supports. So is there a way to restrict (and possibly also move) the framebuffer in probe()?
I suppose you can, but that is not what is expected. You don't save memory for U-Boot (and it doesn't matter), but making it smaller so that linux uses less would be worthwhile. We just need to make sure this is documented in video.h and tested.
To be clear, you have my review thg, so these are things that can be done later.
Regards, Simon
On Sun, Feb 21, 2021 at 09:24:18AM -0700, Simon Glass wrote:
+static int sunxi_de_bind(struct udevice *dev) +{
struct video_uc_plat *plat = dev_get_uclass_plat(dev);plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT *(1 << LCD_MAX_LOG2_BPP) / 8;Should use enum video_log2_bpp here. Also see VNBYTES().
LCD_MAX_LOG2_BPP is defined as VIDEO_BPP32 at the very top. Sure will use VNBYTES.
On a related topic: IIUC this is called several times, for a start once before relocation, where it's supposed to give an upper bound? Are subsequent calls then expected to be more precise? Our 32MB frame buffer is *very* generous, for the usual FullHD display we just need 8MB. But we would only know this in probe(), when we have learned the output device and the video modes it supports. So is there a way to restrict (and possibly also move) the framebuffer in probe()?
I suppose you can, but that is not what is expected. You don't save memory for U-Boot (and it doesn't matter), but making it smaller so that linux uses less would be worthwhile. We just need to make sure this is documented in video.h and tested.
To be clear, you have my review thg, so these are things that can be done later.
Seems Andre seems to be motivated to rework that driver, I guess we could also rework how this is done.
Ideally, we should be describing the reserved buffer through a reserved-memory node in the DT instead of carving out some memory for Linux. That way, if we don't have simplefb support, or it's replaced by something else eventually, we have a chance a claiming that memory back at some point, instead of just ignoring it.
Maxime
Hi Andre,
On Sun, Feb 21, 2021 at 12:07:35AM +0000, Andre Przywara wrote:
enum sunxi_monitor { sunxi_monitor_none, sunxi_monitor_dvi, @@ -59,12 +68,11 @@ enum sunxi_monitor { #define SUNXI_MONITOR_LAST sunxi_monitor_composite_pal_nc
struct sunxi_display {
GraphicDevice graphic_device; enum sunxi_monitor monitor; unsigned int depth; unsigned int fb_addr; unsigned int fb_size;These last three are repeated from the uclass info. But fb_addr seems to sometimes be different from the ucass frame buffer, in which case I wonder how output actually works.
If you do actually need these, can you please document them here?
I think this might be mostly non-DM legacy, but for the composite overscan we tweak the framebuffer address. I haven't fully wrapped my mind around this, though, and ideally we can indeed lose those extra members. I will try to find some device with composite input to test it.
So the overscan will put the edges of any displayed image out of the visible area of the screen, right? The amount of content non-visible due to this isn't really standard and any display will behave differently there.
One way to deal with this is to rescale the composited image to a slightly smaller and center it in the middle of the screen.
We don't have that option though, so the code will instead render in a slightly smaller resolution, centered in the framebuffer.
Note that composite isn't the only output that you can test this on: HDMI TVs will also use overscan.
participants (6)
-
Anatolij Gustschin -
Andre Przywara -
Jernej Škrabec -
Maxime Ripard -
Simon Glass -
Tom Rini