[U-Boot] [PATCH 0/5] db410c: updates for grub + gfxterm
In particular, support for display setup by lk. This introduces a simplefb display driver that uses the framebuffer fdt node populated by the firmware[1] for u-boot display support (and, at least for what I am working on, more interestingly, EFI GOP support).
In general, most snapdragon devices have a lk firmware which can light up the display. And mdp5 is a rather complex display controller block (not to mention different mdp4 block for older generations, and hdmi vs dsi blocks). So rather than duplicate all of that in u-boot, add a video driver similar to linux's simplefb, which simply takes over a display setup by an earlier boot stage.
The simplefb video driver is so far only used on snapdragon, but the implementation is totally generic so probably useful on other devices in the future.
A few related patches so that on db410c we actually use the fdt passed by lk (and so that dm/core is clever enough to notice fdt nodes under "chosen"), config updates, and related fixes.
[1] https://github.com/robclark/lk/commits/db410c-display
Rob Clark (5): board/db410c: use fdt passed from lk dm: core: also parse chosen node video: simplefb efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD configs: db410c: config updates
arch/arm/Kconfig | 2 +- board/qualcomm/dragonboard410c/dragonboard410c.c | 16 ++++++ cmd/bootefi.c | 2 +- configs/dragonboard410c_defconfig | 7 +++ drivers/core/root.c | 22 +++++++- drivers/video/Kconfig | 10 ++++ drivers/video/Makefile | 2 +- drivers/video/simplefb.c | 68 ++++++++++++++++++++++++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_gop.c | 7 ++- 10 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 drivers/video/simplefb.c
lk patches the fdt to set some device's MAC addresses and more importantly to patch in the simple-framebuffer node that we want u-boot to see.
Signed-off-by: Rob Clark robdclark@gmail.com --- board/qualcomm/dragonboard410c/dragonboard410c.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/board/qualcomm/dragonboard410c/dragonboard410c.c b/board/qualcomm/dragonboard410c/dragonboard410c.c index 37d0b85..6657e14 100644 --- a/board/qualcomm/dragonboard410c/dragonboard410c.c +++ b/board/qualcomm/dragonboard410c/dragonboard410c.c @@ -27,6 +27,22 @@ int dram_init_banksize(void) return 0; }
+unsigned long fw_dtb_pointer; + +void save_boot_params_ret(void); + +void save_boot_params(u64 x0, u64 x1, u64 x2, u64 x3) +{ + fw_dtb_pointer = x0; + save_boot_params_ret(); +} + +void *board_fdt_blob_setup(void) +{ + if (fdt_magic(fw_dtb_pointer) != FDT_MAGIC) + return NULL; + return (void *)fw_dtb_pointer; +}
int board_prepare_usb(enum usb_init_type type) {
On Sat, Jun 24, 2017 at 07:05:19PM -0400, Rob Clark wrote:
lk patches the fdt to set some device's MAC addresses and more importantly to patch in the simple-framebuffer node that we want u-boot to see.
Signed-off-by: Rob Clark robdclark@gmail.com
board/qualcomm/dragonboard410c/dragonboard410c.c | 16 ++++++++++++++++
This is too early for C, please re-do this in an asm file.
This is the node that would contain, for example, the framebuffer setup by an earlier stage.
Signed-off-by: Rob Clark robdclark@gmail.com --- drivers/core/root.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c index d691d6f..5e6b2da 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -266,6 +266,26 @@ static int dm_scan_fdt_node(struct udevice *parent, const void *blob, for (offset = fdt_first_subnode(blob, offset); offset > 0; offset = fdt_next_subnode(blob, offset)) { + ofnode node = offset_to_ofnode(offset); + + /* "chosen" node isn't a device itself but may contain some: */ + if (strcmp(ofnode_get_name(node), "chosen") == 0) { + dm_dbg("parsing subnodes of "chosen"\n"); + + for (node = ofnode_first_subnode(node); + ofnode_valid(node); + node = ofnode_next_subnode(node)) { + dm_dbg("subnode: %s\n", ofnode_get_name(node)); + err = lists_bind_fdt(parent, node, NULL); + if (err && !ret) { + ret = err; + dm_dbg("%s: ret=%d\n", ofnode_get_name(node), ret); + } + } + + continue; + } + if (pre_reloc_only && !dm_fdt_pre_reloc(blob, offset)) continue; @@ -273,7 +293,7 @@ static int dm_scan_fdt_node(struct udevice *parent, const void *blob, dm_dbg(" - ignoring disabled device\n"); continue; } - err = lists_bind_fdt(parent, offset_to_ofnode(offset), NULL); + err = lists_bind_fdt(parent, node, NULL); if (err && !ret) { ret = err; debug("%s: ret=%d\n", fdt_get_name(blob, offset, NULL),
Not really qcom specific, but for now qcom/lk is the one firmware that is (afaiu) setting up the appropriate dt node for pre-configured display. Uses the generic simple-framebuffer DT bindings so this should be useful on other platforms.
Signed-off-by: Rob Clark robdclark@gmail.com --- drivers/video/Kconfig | 10 +++++++ drivers/video/Makefile | 2 +- drivers/video/simplefb.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 drivers/video/simplefb.c
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 61dfed8..8eb0359 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -628,4 +628,14 @@ config VIDEO_DW_HDMI rather requires a SoC-specific glue driver to call it), it can not be enabled from the configuration menu.
+config VIDEO_SIMPLE + bool "Simple display driver for preconfigured display" + help + Enables a simple generic display driver which utilizes the + simple-framebuffer devicetree bindings. + + This driver assumes that the display hardware has been initialized + before u-boot starts, and u-boot will simply render to the pre- + allocated frame buffer surface. + endmenu diff --git a/drivers/video/Makefile b/drivers/video/Makefile index ac5371f..52f50f6 100644 --- a/drivers/video/Makefile +++ b/drivers/video/Makefile @@ -57,7 +57,7 @@ obj-$(CONFIG_FORMIKE) += formike.o obj-$(CONFIG_LG4573) += lg4573.o obj-$(CONFIG_AM335X_LCD) += am335x-fb.o obj-$(CONFIG_VIDEO_DW_HDMI) += dw_hdmi.o - +obj-$(CONFIG_VIDEO_SIMPLE) += simplefb.o obj-${CONFIG_VIDEO_TEGRA124} += tegra124/ obj-${CONFIG_EXYNOS_FB} += exynos/ obj-${CONFIG_VIDEO_ROCKCHIP} += rockchip/ diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c new file mode 100644 index 0000000..7c3e8e5 --- /dev/null +++ b/drivers/video/simplefb.c @@ -0,0 +1,68 @@ +/* + * (C) Copyright 2017 Rob Clark + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <fdtdec.h> +#include <fdt_support.h> +#include <video.h> + +static int simple_video_probe(struct udevice *dev) +{ + struct video_uc_platdata *plat = dev_get_uclass_platdata(dev); + struct video_priv *uc_priv = dev_get_uclass_priv(dev); + const void *blob = gd->fdt_blob; + const int node = dev_of_offset(dev); + const char *format; + fdt_addr_t base, size; + + if (fdtdec_decode_region(blob, node, "reg", &base, &size)) { + debug("%s: Failed to decode memory region\n", __func__); + return -EINVAL; + } + + debug("%s: base=%llx, size=%llu\n", __func__, base, size); + + // TODO is there some way to reserve the framebuffer + // region so it isn't clobbered? + plat->base = base; + plat->size = size; + + video_set_flush_dcache(dev, true); + + debug("%s: Query resolution...\n", __func__); + + uc_priv->xsize = fdtdec_get_uint(blob, node, "width", 0); + uc_priv->ysize = fdtdec_get_uint(blob, node, "height", 0); + uc_priv->rot = 0; + + format = fdt_getprop(blob, node, "format", NULL); + debug("%s: %dx%d@%s\n", __func__, uc_priv->xsize, uc_priv->ysize, format); + + if (strcmp(format, "r5g6b5") == 0) { + uc_priv->bpix = VIDEO_BPP16; + } else if (strcmp(format, "a8b8g8r8") == 0) { + uc_priv->bpix = VIDEO_BPP32; + } else { + printf("%s: invalid format: %s\n", __func__, format); + return -EINVAL; + } + + return 0; +} + +static const struct udevice_id simple_video_ids[] = { + { .compatible = "simple-framebuffer" }, + { } +}; + +U_BOOT_DRIVER(simple_video) = { + .name = "simple_video", + .id = UCLASS_VIDEO, + .of_match = simple_video_ids, + .probe = simple_video_probe, + .flags = DM_FLAG_PRE_RELOC, +};
Signed-off-by: Rob Clark robdclark@gmail.com Cc: Alexander Graf agraf@suse.de --- cmd/bootefi.c | 2 +- lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_gop.c | 7 ++++++- 3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index a6598df..4f11682 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -280,7 +280,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt) #ifdef CONFIG_PARTITIONS efi_disk_register(); #endif -#ifdef CONFIG_LCD +#if defined(CONFIG_LCD) || defined(CONFIG_DM_VIDEO) efi_gop_register(); #endif #ifdef CONFIG_NET diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index fa8b91a..3c230ac 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o obj-y += efi_memory.o obj-$(CONFIG_LCD) += efi_gop.o +obj-$(CONFIG_DM_VIDEO) += efi_gop.o obj-$(CONFIG_PARTITIONS) += efi_disk.o obj-$(CONFIG_NET) += efi_net.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 286ad83..b8b0d5e 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -28,6 +28,7 @@ struct efi_gop_obj { struct efi_gop_mode mode; /* Fields we only have acces to during init */ u32 bpix; + void *fb; };
static efi_status_t EFIAPI gop_query_mode(struct efi_gop *this, u32 mode_number, @@ -71,7 +72,7 @@ static efi_status_t EFIAPI gop_blt(struct efi_gop *this, void *buffer, if (operation != EFI_BLT_BUFFER_TO_VIDEO) return EFI_EXIT(EFI_INVALID_PARAMETER);
- fb = (void*)gd->fb_base; + fb = gopobj->fb; line_len16 = gopobj->info.width * sizeof(u16); line_len32 = gopobj->info.width * sizeof(u32);
@@ -130,6 +131,7 @@ int efi_gop_register(void) struct efi_gop_obj *gopobj; u32 bpix, col, row; u64 fb_base, fb_size; + void *fb;
#ifdef CONFIG_DM_VIDEO struct udevice *vdev; @@ -144,6 +146,7 @@ int efi_gop_register(void) row = video_get_ysize(vdev); fb_base = (uintptr_t)priv->fb; fb_size = priv->fb_size; + fb = priv->fb; #else int line_len;
@@ -152,6 +155,7 @@ int efi_gop_register(void) row = panel_info.vl_row; fb_base = gd->fb_base; fb_size = lcd_get_size(&line_len); + fb = gd->fb_base; #endif
switch (bpix) { @@ -200,6 +204,7 @@ int efi_gop_register(void) gopobj->info.pixels_per_scanline = col;
gopobj->bpix = bpix; + gopobj->fb = fb;
/* Hook up to the device list */ list_add_tail(&gopobj->parent.link, &efi_obj_list);
On 25.06.17 01:05, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com Cc: Alexander Graf agraf@suse.de
Looks reasonable to me, but could probably use a commit message ;). Also please make sure to CC Simon on all things DM.
Alex
cmd/bootefi.c | 2 +- lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_gop.c | 7 ++++++- 3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index a6598df..4f11682 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -280,7 +280,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt) #ifdef CONFIG_PARTITIONS efi_disk_register(); #endif -#ifdef CONFIG_LCD +#if defined(CONFIG_LCD) || defined(CONFIG_DM_VIDEO) efi_gop_register(); #endif #ifdef CONFIG_NET diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index fa8b91a..3c230ac 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o obj-y += efi_memory.o obj-$(CONFIG_LCD) += efi_gop.o +obj-$(CONFIG_DM_VIDEO) += efi_gop.o obj-$(CONFIG_PARTITIONS) += efi_disk.o obj-$(CONFIG_NET) += efi_net.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 286ad83..b8b0d5e 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -28,6 +28,7 @@ struct efi_gop_obj { struct efi_gop_mode mode; /* Fields we only have acces to during init */ u32 bpix;
void *fb; };
static efi_status_t EFIAPI gop_query_mode(struct efi_gop *this, u32 mode_number,
@@ -71,7 +72,7 @@ static efi_status_t EFIAPI gop_blt(struct efi_gop *this, void *buffer, if (operation != EFI_BLT_BUFFER_TO_VIDEO) return EFI_EXIT(EFI_INVALID_PARAMETER);
- fb = (void*)gd->fb_base;
- fb = gopobj->fb; line_len16 = gopobj->info.width * sizeof(u16); line_len32 = gopobj->info.width * sizeof(u32);
@@ -130,6 +131,7 @@ int efi_gop_register(void) struct efi_gop_obj *gopobj; u32 bpix, col, row; u64 fb_base, fb_size;
void *fb;
#ifdef CONFIG_DM_VIDEO struct udevice *vdev;
@@ -144,6 +146,7 @@ int efi_gop_register(void) row = video_get_ysize(vdev); fb_base = (uintptr_t)priv->fb; fb_size = priv->fb_size;
- fb = priv->fb; #else int line_len;
@@ -152,6 +155,7 @@ int efi_gop_register(void) row = panel_info.vl_row; fb_base = gd->fb_base; fb_size = lcd_get_size(&line_len);
fb = gd->fb_base; #endif
switch (bpix) {
@@ -200,6 +204,7 @@ int efi_gop_register(void) gopobj->info.pixels_per_scanline = col;
gopobj->bpix = bpix;
gopobj->fb = fb;
/* Hook up to the device list */ list_add_tail(&gopobj->parent.link, &efi_obj_list);
Hi,
On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote:
On 25.06.17 01:05, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com Cc: Alexander Graf agraf@suse.de
Looks reasonable to me, but could probably use a commit message ;). Also please make sure to CC Simon on all things DM.
Can we drop the CONFIG_LCD support entirely? This is legacy code at this point. What boards use it?
- Simon
Alex
cmd/bootefi.c | 2 +- lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_gop.c | 7 ++++++- 3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index a6598df..4f11682 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -280,7 +280,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt) #ifdef CONFIG_PARTITIONS efi_disk_register(); #endif -#ifdef CONFIG_LCD +#if defined(CONFIG_LCD) || defined(CONFIG_DM_VIDEO) efi_gop_register(); #endif #ifdef CONFIG_NET diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index fa8b91a..3c230ac 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o obj-y += efi_memory.o obj-$(CONFIG_LCD) += efi_gop.o +obj-$(CONFIG_DM_VIDEO) += efi_gop.o obj-$(CONFIG_PARTITIONS) += efi_disk.o obj-$(CONFIG_NET) += efi_net.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 286ad83..b8b0d5e 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -28,6 +28,7 @@ struct efi_gop_obj { struct efi_gop_mode mode; /* Fields we only have acces to during init */ u32 bpix;
void *fb;mode_number, @@ -71,7 +72,7 @@ static efi_status_t EFIAPI gop_blt(struct efi_gop *this, void *buffer, if (operation != EFI_BLT_BUFFER_TO_VIDEO) return EFI_EXIT(EFI_INVALID_PARAMETER);
fb = (void*)gd->fb_base;
fb = gopobj->fb; line_len16 = gopobj->info.width * sizeof(u16); line_len32 = gopobj->info.width * sizeof(u32);void *fb;@@ -144,6 +146,7 @@ int efi_gop_register(void) row = video_get_ysize(vdev); fb_base = (uintptr_t)priv->fb; fb_size = priv->fb_size;
fb = priv->fb;fb = gd->fb_base;@@ -200,6 +204,7 @@ int efi_gop_register(void) gopobj->info.pixels_per_scanline = col; gopobj->bpix = bpix;
gopobj->fb = fb; /* Hook up to the device list */ list_add_tail(&gopobj->parent.link, &efi_obj_list);
On 07/18/2017 04:00 PM, Simon Glass wrote:
Hi,
On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote:
On 25.06.17 01:05, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com Cc: Alexander Graf agraf@suse.de
Looks reasonable to me, but could probably use a commit message ;). Also please make sure to CC Simon on all things DM.
Can we drop the CONFIG_LCD support entirely? This is legacy code at this point. What boards use it?
Sounds like someone would first need to convert a bunch of boards :).
$ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q DM_VIDEO $i || echo $i; done configs/at91sam9261ek_dataflash_cs0_defconfig configs/at91sam9261ek_dataflash_cs3_defconfig configs/at91sam9261ek_nandflash_defconfig configs/at91sam9263ek_dataflash_cs0_defconfig configs/at91sam9263ek_dataflash_defconfig configs/at91sam9263ek_nandflash_defconfig configs/at91sam9263ek_norflash_boot_defconfig configs/at91sam9263ek_norflash_defconfig configs/at91sam9g10ek_dataflash_cs0_defconfig configs/at91sam9g10ek_dataflash_cs3_defconfig configs/at91sam9g10ek_nandflash_defconfig configs/at91sam9m10g45ek_mmc_defconfig configs/at91sam9m10g45ek_nandflash_defconfig configs/at91sam9n12ek_mmc_defconfig configs/at91sam9n12ek_nandflash_defconfig configs/at91sam9n12ek_spiflash_defconfig configs/at91sam9rlek_dataflash_defconfig configs/at91sam9rlek_mmc_defconfig configs/at91sam9rlek_nandflash_defconfig configs/at91sam9x5ek_dataflash_defconfig configs/at91sam9x5ek_mmc_defconfig configs/at91sam9x5ek_nandflash_defconfig configs/at91sam9x5ek_spiflash_defconfig configs/brppt1_mmc_defconfig configs/brppt1_nand_defconfig configs/brppt1_spi_defconfig configs/brxre1_defconfig configs/cm_t3517_defconfig configs/cm_t35_defconfig configs/picosam9g45_defconfig configs/pm9261_defconfig configs/pm9263_defconfig configs/sama5d36ek_cmp_mmc_defconfig configs/sama5d36ek_cmp_nandflash_defconfig configs/sama5d36ek_cmp_spiflash_defconfig configs/sama5d3xek_mmc_defconfig configs/sama5d3xek_nandflash_defconfig configs/sama5d3xek_spiflash_defconfig configs/sama5d4ek_mmc_defconfig configs/sama5d4ek_nandflash_defconfig configs/sama5d4ek_spiflash_defconfig configs/zipitz2_defconfig
Alex
Hi Alex,
On 18 July 2017 at 07:47, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:00 PM, Simon Glass wrote:
Hi,
On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote:
On 25.06.17 01:05, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com Cc: Alexander Graf agraf@suse.de
Looks reasonable to me, but could probably use a commit message ;). Also please make sure to CC Simon on all things DM.
Can we drop the CONFIG_LCD support entirely? This is legacy code at this point. What boards use it?
Sounds like someone would first need to convert a bunch of boards :).
$ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q DM_VIDEO $i || echo $i; done configs/at91sam9261ek_dataflash_cs0_defconfig configs/at91sam9261ek_dataflash_cs3_defconfig configs/at91sam9261ek_nandflash_defconfig configs/at91sam9263ek_dataflash_cs0_defconfig configs/at91sam9263ek_dataflash_defconfig configs/at91sam9263ek_nandflash_defconfig configs/at91sam9263ek_norflash_boot_defconfig configs/at91sam9263ek_norflash_defconfig configs/at91sam9g10ek_dataflash_cs0_defconfig configs/at91sam9g10ek_dataflash_cs3_defconfig configs/at91sam9g10ek_nandflash_defconfig configs/at91sam9m10g45ek_mmc_defconfig configs/at91sam9m10g45ek_nandflash_defconfig configs/at91sam9n12ek_mmc_defconfig configs/at91sam9n12ek_nandflash_defconfig configs/at91sam9n12ek_spiflash_defconfig configs/at91sam9rlek_dataflash_defconfig configs/at91sam9rlek_mmc_defconfig configs/at91sam9rlek_nandflash_defconfig configs/at91sam9x5ek_dataflash_defconfig configs/at91sam9x5ek_mmc_defconfig configs/at91sam9x5ek_nandflash_defconfig configs/at91sam9x5ek_spiflash_defconfig configs/brppt1_mmc_defconfig configs/brppt1_nand_defconfig configs/brppt1_spi_defconfig configs/brxre1_defconfig configs/cm_t3517_defconfig configs/cm_t35_defconfig configs/picosam9g45_defconfig configs/pm9261_defconfig configs/pm9263_defconfig configs/sama5d36ek_cmp_mmc_defconfig configs/sama5d36ek_cmp_nandflash_defconfig configs/sama5d36ek_cmp_spiflash_defconfig configs/sama5d3xek_mmc_defconfig configs/sama5d3xek_nandflash_defconfig configs/sama5d3xek_spiflash_defconfig configs/sama5d4ek_mmc_defconfig configs/sama5d4ek_nandflash_defconfig configs/sama5d4ek_spiflash_defconfig configs/zipitz2_defconfig
Not really. I suspect none of those uses EFI_LOADER
There is video driver for atmel which is most of the boards in that list, but we can disable EFI_LOADER until they are converted.
We should avoid adding new features to legacy code paths as it makes DM conversion harder and less likely to complete.
Regards, Simon
On 07/18/2017 04:54 PM, Simon Glass wrote:
Hi Alex,
On 18 July 2017 at 07:47, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:00 PM, Simon Glass wrote:
Hi,
On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote:
On 25.06.17 01:05, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com Cc: Alexander Graf agraf@suse.de
Looks reasonable to me, but could probably use a commit message ;). Also please make sure to CC Simon on all things DM.
Can we drop the CONFIG_LCD support entirely? This is legacy code at this point. What boards use it?
Sounds like someone would first need to convert a bunch of boards :).
$ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q DM_VIDEO $i || echo $i; done configs/at91sam9261ek_dataflash_cs0_defconfig configs/at91sam9261ek_dataflash_cs3_defconfig configs/at91sam9261ek_nandflash_defconfig configs/at91sam9263ek_dataflash_cs0_defconfig configs/at91sam9263ek_dataflash_defconfig configs/at91sam9263ek_nandflash_defconfig configs/at91sam9263ek_norflash_boot_defconfig configs/at91sam9263ek_norflash_defconfig configs/at91sam9g10ek_dataflash_cs0_defconfig configs/at91sam9g10ek_dataflash_cs3_defconfig configs/at91sam9g10ek_nandflash_defconfig configs/at91sam9m10g45ek_mmc_defconfig configs/at91sam9m10g45ek_nandflash_defconfig configs/at91sam9n12ek_mmc_defconfig configs/at91sam9n12ek_nandflash_defconfig configs/at91sam9n12ek_spiflash_defconfig configs/at91sam9rlek_dataflash_defconfig configs/at91sam9rlek_mmc_defconfig configs/at91sam9rlek_nandflash_defconfig configs/at91sam9x5ek_dataflash_defconfig configs/at91sam9x5ek_mmc_defconfig configs/at91sam9x5ek_nandflash_defconfig configs/at91sam9x5ek_spiflash_defconfig configs/brppt1_mmc_defconfig configs/brppt1_nand_defconfig configs/brppt1_spi_defconfig configs/brxre1_defconfig configs/cm_t3517_defconfig configs/cm_t35_defconfig configs/picosam9g45_defconfig configs/pm9261_defconfig configs/pm9263_defconfig configs/sama5d36ek_cmp_mmc_defconfig configs/sama5d36ek_cmp_nandflash_defconfig configs/sama5d36ek_cmp_spiflash_defconfig configs/sama5d3xek_mmc_defconfig configs/sama5d3xek_nandflash_defconfig configs/sama5d3xek_spiflash_defconfig configs/sama5d4ek_mmc_defconfig configs/sama5d4ek_nandflash_defconfig configs/sama5d4ek_spiflash_defconfig configs/zipitz2_defconfig
Not really. I suspect none of those uses EFI_LOADER
Why not? I really don't want to limit EFI_LOADER to something I consider good. It's supposed to be the *one* interface that just works for everyone.
There is video driver for atmel which is most of the boards in that list, but we can disable EFI_LOADER until they are converted.
No, I won't disable EFI_LOADER on any board because it's not converted. I'd rather add support to EFI_LOADER to support more boards that are not converted to DM ;).
We should avoid adding new features to legacy code paths as it makes DM conversion harder and less likely to complete.
I agree, but the solution is not to disable EFI_LOADER, it's to convert boards.
Alex
On Tue, Jul 18, 2017 at 05:06:49PM +0200, Alexander Graf wrote:
On 07/18/2017 04:54 PM, Simon Glass wrote:
Hi Alex,
On 18 July 2017 at 07:47, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:00 PM, Simon Glass wrote:
Hi,
On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote:
On 25.06.17 01:05, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com Cc: Alexander Graf agraf@suse.de
Looks reasonable to me, but could probably use a commit message ;). Also please make sure to CC Simon on all things DM.
Can we drop the CONFIG_LCD support entirely? This is legacy code at this point. What boards use it?
Sounds like someone would first need to convert a bunch of boards :).
$ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q DM_VIDEO $i || echo $i; done configs/at91sam9261ek_dataflash_cs0_defconfig configs/at91sam9261ek_dataflash_cs3_defconfig configs/at91sam9261ek_nandflash_defconfig configs/at91sam9263ek_dataflash_cs0_defconfig configs/at91sam9263ek_dataflash_defconfig configs/at91sam9263ek_nandflash_defconfig configs/at91sam9263ek_norflash_boot_defconfig configs/at91sam9263ek_norflash_defconfig configs/at91sam9g10ek_dataflash_cs0_defconfig configs/at91sam9g10ek_dataflash_cs3_defconfig configs/at91sam9g10ek_nandflash_defconfig configs/at91sam9m10g45ek_mmc_defconfig configs/at91sam9m10g45ek_nandflash_defconfig configs/at91sam9n12ek_mmc_defconfig configs/at91sam9n12ek_nandflash_defconfig configs/at91sam9n12ek_spiflash_defconfig configs/at91sam9rlek_dataflash_defconfig configs/at91sam9rlek_mmc_defconfig configs/at91sam9rlek_nandflash_defconfig configs/at91sam9x5ek_dataflash_defconfig configs/at91sam9x5ek_mmc_defconfig configs/at91sam9x5ek_nandflash_defconfig configs/at91sam9x5ek_spiflash_defconfig configs/brppt1_mmc_defconfig configs/brppt1_nand_defconfig configs/brppt1_spi_defconfig configs/brxre1_defconfig configs/cm_t3517_defconfig configs/cm_t35_defconfig configs/picosam9g45_defconfig configs/pm9261_defconfig configs/pm9263_defconfig configs/sama5d36ek_cmp_mmc_defconfig configs/sama5d36ek_cmp_nandflash_defconfig configs/sama5d36ek_cmp_spiflash_defconfig configs/sama5d3xek_mmc_defconfig configs/sama5d3xek_nandflash_defconfig configs/sama5d3xek_spiflash_defconfig configs/sama5d4ek_mmc_defconfig configs/sama5d4ek_nandflash_defconfig configs/sama5d4ek_spiflash_defconfig configs/zipitz2_defconfig
Not really. I suspect none of those uses EFI_LOADER
Why not? I really don't want to limit EFI_LOADER to something I consider good. It's supposed to be the *one* interface that just works for everyone.
There is video driver for atmel which is most of the boards in that list, but we can disable EFI_LOADER until they are converted.
No, I won't disable EFI_LOADER on any board because it's not converted. I'd rather add support to EFI_LOADER to support more boards that are not converted to DM ;).
We should avoid adding new features to legacy code paths as it makes DM conversion harder and less likely to complete.
I agree, but the solution is not to disable EFI_LOADER, it's to convert boards.
Lets CC some Atmel folks...
Hi
-----Original Message----- From: Tom Rini [mailto:trini@konsulko.com] Sent: 2017年7月19日 1:08 To: Alexander Graf agraf@suse.de; Wenyou Yang - A41535 Wenyou.Yang@microchip.com; Andreas Bießmann andreas.devel@googlemail.com Cc: Simon Glass sjg@chromium.org; Rob Clark robdclark@gmail.com; U- Boot Mailing List u-boot@lists.denx.de; Mateusz Kulikowski mateusz.kulikowski@gmail.com Subject: Re: [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD
On Tue, Jul 18, 2017 at 05:06:49PM +0200, Alexander Graf wrote:
On 07/18/2017 04:54 PM, Simon Glass wrote:
Hi Alex,
On 18 July 2017 at 07:47, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:00 PM, Simon Glass wrote:
Hi,
On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote:
On 25.06.17 01:05, Rob Clark wrote: >Signed-off-by: Rob Clark robdclark@gmail.com >Cc: Alexander Graf agraf@suse.de
Looks reasonable to me, but could probably use a commit message ;). Also please make sure to CC Simon on all things DM.
Can we drop the CONFIG_LCD support entirely? This is legacy code at this point. What boards use it?
Sounds like someone would first need to convert a bunch of boards :).
$ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q DM_VIDEO $i || echo $i; done configs/at91sam9261ek_dataflash_cs0_defconfig configs/at91sam9261ek_dataflash_cs3_defconfig configs/at91sam9261ek_nandflash_defconfig configs/at91sam9263ek_dataflash_cs0_defconfig configs/at91sam9263ek_dataflash_defconfig configs/at91sam9263ek_nandflash_defconfig configs/at91sam9263ek_norflash_boot_defconfig configs/at91sam9263ek_norflash_defconfig configs/at91sam9g10ek_dataflash_cs0_defconfig configs/at91sam9g10ek_dataflash_cs3_defconfig configs/at91sam9g10ek_nandflash_defconfig configs/at91sam9m10g45ek_mmc_defconfig configs/at91sam9m10g45ek_nandflash_defconfig configs/at91sam9n12ek_mmc_defconfig configs/at91sam9n12ek_nandflash_defconfig configs/at91sam9n12ek_spiflash_defconfig configs/at91sam9rlek_dataflash_defconfig configs/at91sam9rlek_mmc_defconfig configs/at91sam9rlek_nandflash_defconfig configs/at91sam9x5ek_dataflash_defconfig configs/at91sam9x5ek_mmc_defconfig configs/at91sam9x5ek_nandflash_defconfig configs/at91sam9x5ek_spiflash_defconfig configs/brppt1_mmc_defconfig configs/brppt1_nand_defconfig configs/brppt1_spi_defconfig configs/brxre1_defconfig configs/cm_t3517_defconfig configs/cm_t35_defconfig configs/picosam9g45_defconfig configs/pm9261_defconfig configs/pm9263_defconfig configs/sama5d36ek_cmp_mmc_defconfig configs/sama5d36ek_cmp_nandflash_defconfig configs/sama5d36ek_cmp_spiflash_defconfig configs/sama5d3xek_mmc_defconfig configs/sama5d3xek_nandflash_defconfig configs/sama5d3xek_spiflash_defconfig configs/sama5d4ek_mmc_defconfig configs/sama5d4ek_nandflash_defconfig configs/sama5d4ek_spiflash_defconfig configs/zipitz2_defconfig
Not really. I suspect none of those uses EFI_LOADER
Why not? I really don't want to limit EFI_LOADER to something I consider good. It's supposed to be the *one* interface that just works for everyone.
There is video driver for atmel which is most of the boards in that list, but we can disable EFI_LOADER until they are converted.
No, I won't disable EFI_LOADER on any board because it's not converted. I'd rather add support to EFI_LOADER to support more boards that are not converted to DM ;).
We should avoid adding new features to legacy code paths as it makes DM conversion harder and less likely to complete.
I agree, but the solution is not to disable EFI_LOADER, it's to convert boards.
Lets CC some Atmel folks...
We will send patches to convert the board config to DM_VIDEO, remove CONFIG_LCD.
Best Regards, Wenyou Yang
On Tue, Jul 18, 2017 at 11:06 AM, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:54 PM, Simon Glass wrote:
Hi Alex,
On 18 July 2017 at 07:47, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:00 PM, Simon Glass wrote:
Hi,
On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote:
On 25.06.17 01:05, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com Cc: Alexander Graf agraf@suse.de
Looks reasonable to me, but could probably use a commit message ;). Also please make sure to CC Simon on all things DM.
Can we drop the CONFIG_LCD support entirely? This is legacy code at this point. What boards use it?
Sounds like someone would first need to convert a bunch of boards :).
$ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q DM_VIDEO $i || echo $i; done configs/at91sam9261ek_dataflash_cs0_defconfig configs/at91sam9261ek_dataflash_cs3_defconfig configs/at91sam9261ek_nandflash_defconfig configs/at91sam9263ek_dataflash_cs0_defconfig configs/at91sam9263ek_dataflash_defconfig configs/at91sam9263ek_nandflash_defconfig configs/at91sam9263ek_norflash_boot_defconfig configs/at91sam9263ek_norflash_defconfig configs/at91sam9g10ek_dataflash_cs0_defconfig configs/at91sam9g10ek_dataflash_cs3_defconfig configs/at91sam9g10ek_nandflash_defconfig configs/at91sam9m10g45ek_mmc_defconfig configs/at91sam9m10g45ek_nandflash_defconfig configs/at91sam9n12ek_mmc_defconfig configs/at91sam9n12ek_nandflash_defconfig configs/at91sam9n12ek_spiflash_defconfig configs/at91sam9rlek_dataflash_defconfig configs/at91sam9rlek_mmc_defconfig configs/at91sam9rlek_nandflash_defconfig configs/at91sam9x5ek_dataflash_defconfig configs/at91sam9x5ek_mmc_defconfig configs/at91sam9x5ek_nandflash_defconfig configs/at91sam9x5ek_spiflash_defconfig configs/brppt1_mmc_defconfig configs/brppt1_nand_defconfig configs/brppt1_spi_defconfig configs/brxre1_defconfig configs/cm_t3517_defconfig configs/cm_t35_defconfig configs/picosam9g45_defconfig configs/pm9261_defconfig configs/pm9263_defconfig configs/sama5d36ek_cmp_mmc_defconfig configs/sama5d36ek_cmp_nandflash_defconfig configs/sama5d36ek_cmp_spiflash_defconfig configs/sama5d3xek_mmc_defconfig configs/sama5d3xek_nandflash_defconfig configs/sama5d3xek_spiflash_defconfig configs/sama5d4ek_mmc_defconfig configs/sama5d4ek_nandflash_defconfig configs/sama5d4ek_spiflash_defconfig configs/zipitz2_defconfig
Not really. I suspect none of those uses EFI_LOADER
Why not? I really don't want to limit EFI_LOADER to something I consider good. It's supposed to be the *one* interface that just works for everyone.
There is video driver for atmel which is most of the boards in that list, but we can disable EFI_LOADER until they are converted.
No, I won't disable EFI_LOADER on any board because it's not converted. I'd rather add support to EFI_LOADER to support more boards that are not converted to DM ;).
We should avoid adding new features to legacy code paths as it makes DM conversion harder and less likely to complete.
I agree, but the solution is not to disable EFI_LOADER, it's to convert boards.
So what is the conclusion on this patch? I can re-send with a commit msg (although there is not much to say beyond $subject).. I kinda think we should merge this for now, unless dropping CONFIG_LCD is imminent in which case I can re-work it to drop the CONFIG_LCD and add CONFIG_DM_VIDEO.. EFI_LOADER should definitely support the non-legacy case and probably should not remove the legacy case until it does not exist anymore.
btw, I had a bit of time today to rebase my u-boot patches and start cleaning up some of the other patches I have for (re)posting.. so expect some more patches soon(ish)
BR, -R
Hi Rob,
On 19 July 2017 at 09:24, Rob Clark robdclark@gmail.com wrote:
On Tue, Jul 18, 2017 at 11:06 AM, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:54 PM, Simon Glass wrote:
Hi Alex,
On 18 July 2017 at 07:47, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:00 PM, Simon Glass wrote:
Hi,
On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote:
On 25.06.17 01:05, Rob Clark wrote: > > Signed-off-by: Rob Clark robdclark@gmail.com > Cc: Alexander Graf agraf@suse.de
Looks reasonable to me, but could probably use a commit message ;). Also please make sure to CC Simon on all things DM.
Can we drop the CONFIG_LCD support entirely? This is legacy code at this point. What boards use it?
Sounds like someone would first need to convert a bunch of boards :).
$ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q DM_VIDEO $i || echo $i; done configs/at91sam9261ek_dataflash_cs0_defconfig configs/at91sam9261ek_dataflash_cs3_defconfig configs/at91sam9261ek_nandflash_defconfig configs/at91sam9263ek_dataflash_cs0_defconfig configs/at91sam9263ek_dataflash_defconfig configs/at91sam9263ek_nandflash_defconfig configs/at91sam9263ek_norflash_boot_defconfig configs/at91sam9263ek_norflash_defconfig configs/at91sam9g10ek_dataflash_cs0_defconfig configs/at91sam9g10ek_dataflash_cs3_defconfig configs/at91sam9g10ek_nandflash_defconfig configs/at91sam9m10g45ek_mmc_defconfig configs/at91sam9m10g45ek_nandflash_defconfig configs/at91sam9n12ek_mmc_defconfig configs/at91sam9n12ek_nandflash_defconfig configs/at91sam9n12ek_spiflash_defconfig configs/at91sam9rlek_dataflash_defconfig configs/at91sam9rlek_mmc_defconfig configs/at91sam9rlek_nandflash_defconfig configs/at91sam9x5ek_dataflash_defconfig configs/at91sam9x5ek_mmc_defconfig configs/at91sam9x5ek_nandflash_defconfig configs/at91sam9x5ek_spiflash_defconfig configs/brppt1_mmc_defconfig configs/brppt1_nand_defconfig configs/brppt1_spi_defconfig configs/brxre1_defconfig configs/cm_t3517_defconfig configs/cm_t35_defconfig configs/picosam9g45_defconfig configs/pm9261_defconfig configs/pm9263_defconfig configs/sama5d36ek_cmp_mmc_defconfig configs/sama5d36ek_cmp_nandflash_defconfig configs/sama5d36ek_cmp_spiflash_defconfig configs/sama5d3xek_mmc_defconfig configs/sama5d3xek_nandflash_defconfig configs/sama5d3xek_spiflash_defconfig configs/sama5d4ek_mmc_defconfig configs/sama5d4ek_nandflash_defconfig configs/sama5d4ek_spiflash_defconfig configs/zipitz2_defconfig
Not really. I suspect none of those uses EFI_LOADER
Why not? I really don't want to limit EFI_LOADER to something I consider good. It's supposed to be the *one* interface that just works for everyone.
There is video driver for atmel which is most of the boards in that list, but we can disable EFI_LOADER until they are converted.
No, I won't disable EFI_LOADER on any board because it's not converted. I'd rather add support to EFI_LOADER to support more boards that are not converted to DM ;).
We should avoid adding new features to legacy code paths as it makes DM conversion harder and less likely to complete.
I agree, but the solution is not to disable EFI_LOADER, it's to convert boards.
So what is the conclusion on this patch? I can re-send with a commit msg (although there is not much to say beyond $subject).. I kinda think we should merge this for now, unless dropping CONFIG_LCD is imminent in which case I can re-work it to drop the CONFIG_LCD and add CONFIG_DM_VIDEO.. EFI_LOADER should definitely support the non-legacy case and probably should not remove the legacy case until it does not exist anymore.
Yes I think resend with a commit message and apply it.
I'll look at a patch to make EFI_LOADER depend on DM, which I think should have been done at the start. Supporting legacy code paths with new features is just not a good idea.
btw, I had a bit of time today to rebase my u-boot patches and start cleaning up some of the other patches I have for (re)posting.. so expect some more patches soon(ish)
BR, -R
Regards, Simon
On Fri, Jul 21, 2017 at 04:48:23AM -0600, Simon Glass wrote:
Hi Rob,
On 19 July 2017 at 09:24, Rob Clark robdclark@gmail.com wrote:
On Tue, Jul 18, 2017 at 11:06 AM, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:54 PM, Simon Glass wrote:
Hi Alex,
On 18 July 2017 at 07:47, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:00 PM, Simon Glass wrote:
Hi,
On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote: > > > On 25.06.17 01:05, Rob Clark wrote: >> >> Signed-off-by: Rob Clark robdclark@gmail.com >> Cc: Alexander Graf agraf@suse.de > > > Looks reasonable to me, but could probably use a commit message ;). > Also > please make sure to CC Simon on all things DM. > Can we drop the CONFIG_LCD support entirely? This is legacy code at this point. What boards use it?
Sounds like someone would first need to convert a bunch of boards :).
$ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q DM_VIDEO $i || echo $i; done configs/at91sam9261ek_dataflash_cs0_defconfig configs/at91sam9261ek_dataflash_cs3_defconfig configs/at91sam9261ek_nandflash_defconfig configs/at91sam9263ek_dataflash_cs0_defconfig configs/at91sam9263ek_dataflash_defconfig configs/at91sam9263ek_nandflash_defconfig configs/at91sam9263ek_norflash_boot_defconfig configs/at91sam9263ek_norflash_defconfig configs/at91sam9g10ek_dataflash_cs0_defconfig configs/at91sam9g10ek_dataflash_cs3_defconfig configs/at91sam9g10ek_nandflash_defconfig configs/at91sam9m10g45ek_mmc_defconfig configs/at91sam9m10g45ek_nandflash_defconfig configs/at91sam9n12ek_mmc_defconfig configs/at91sam9n12ek_nandflash_defconfig configs/at91sam9n12ek_spiflash_defconfig configs/at91sam9rlek_dataflash_defconfig configs/at91sam9rlek_mmc_defconfig configs/at91sam9rlek_nandflash_defconfig configs/at91sam9x5ek_dataflash_defconfig configs/at91sam9x5ek_mmc_defconfig configs/at91sam9x5ek_nandflash_defconfig configs/at91sam9x5ek_spiflash_defconfig configs/brppt1_mmc_defconfig configs/brppt1_nand_defconfig configs/brppt1_spi_defconfig configs/brxre1_defconfig configs/cm_t3517_defconfig configs/cm_t35_defconfig configs/picosam9g45_defconfig configs/pm9261_defconfig configs/pm9263_defconfig configs/sama5d36ek_cmp_mmc_defconfig configs/sama5d36ek_cmp_nandflash_defconfig configs/sama5d36ek_cmp_spiflash_defconfig configs/sama5d3xek_mmc_defconfig configs/sama5d3xek_nandflash_defconfig configs/sama5d3xek_spiflash_defconfig configs/sama5d4ek_mmc_defconfig configs/sama5d4ek_nandflash_defconfig configs/sama5d4ek_spiflash_defconfig configs/zipitz2_defconfig
Not really. I suspect none of those uses EFI_LOADER
Why not? I really don't want to limit EFI_LOADER to something I consider good. It's supposed to be the *one* interface that just works for everyone.
There is video driver for atmel which is most of the boards in that list, but we can disable EFI_LOADER until they are converted.
No, I won't disable EFI_LOADER on any board because it's not converted. I'd rather add support to EFI_LOADER to support more boards that are not converted to DM ;).
We should avoid adding new features to legacy code paths as it makes DM conversion harder and less likely to complete.
I agree, but the solution is not to disable EFI_LOADER, it's to convert boards.
So what is the conclusion on this patch? I can re-send with a commit msg (although there is not much to say beyond $subject).. I kinda think we should merge this for now, unless dropping CONFIG_LCD is imminent in which case I can re-work it to drop the CONFIG_LCD and add CONFIG_DM_VIDEO.. EFI_LOADER should definitely support the non-legacy case and probably should not remove the legacy case until it does not exist anymore.
Yes I think resend with a commit message and apply it.
I'll look at a patch to make EFI_LOADER depend on DM, which I think should have been done at the start. Supporting legacy code paths with new features is just not a good idea.
Well, that's very much not how the EFI_LOADER maintainer wants go :) I think I'd rather go with disabling the video side of the various boards, until they're converted.
On Fri, Jul 21, 2017 at 11:58 AM, Tom Rini trini@konsulko.com wrote:
On Fri, Jul 21, 2017 at 04:48:23AM -0600, Simon Glass wrote:
Hi Rob,
On 19 July 2017 at 09:24, Rob Clark robdclark@gmail.com wrote:
On Tue, Jul 18, 2017 at 11:06 AM, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:54 PM, Simon Glass wrote:
Hi Alex,
On 18 July 2017 at 07:47, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:00 PM, Simon Glass wrote: > > Hi, > > On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote: >> >> >> On 25.06.17 01:05, Rob Clark wrote: >>> >>> Signed-off-by: Rob Clark robdclark@gmail.com >>> Cc: Alexander Graf agraf@suse.de >> >> >> Looks reasonable to me, but could probably use a commit message ;). >> Also >> please make sure to CC Simon on all things DM. >> > Can we drop the CONFIG_LCD support entirely? This is legacy code at > this point. What boards use it?
Sounds like someone would first need to convert a bunch of boards :).
$ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q DM_VIDEO $i || echo $i; done configs/at91sam9261ek_dataflash_cs0_defconfig configs/at91sam9261ek_dataflash_cs3_defconfig configs/at91sam9261ek_nandflash_defconfig configs/at91sam9263ek_dataflash_cs0_defconfig configs/at91sam9263ek_dataflash_defconfig configs/at91sam9263ek_nandflash_defconfig configs/at91sam9263ek_norflash_boot_defconfig configs/at91sam9263ek_norflash_defconfig configs/at91sam9g10ek_dataflash_cs0_defconfig configs/at91sam9g10ek_dataflash_cs3_defconfig configs/at91sam9g10ek_nandflash_defconfig configs/at91sam9m10g45ek_mmc_defconfig configs/at91sam9m10g45ek_nandflash_defconfig configs/at91sam9n12ek_mmc_defconfig configs/at91sam9n12ek_nandflash_defconfig configs/at91sam9n12ek_spiflash_defconfig configs/at91sam9rlek_dataflash_defconfig configs/at91sam9rlek_mmc_defconfig configs/at91sam9rlek_nandflash_defconfig configs/at91sam9x5ek_dataflash_defconfig configs/at91sam9x5ek_mmc_defconfig configs/at91sam9x5ek_nandflash_defconfig configs/at91sam9x5ek_spiflash_defconfig configs/brppt1_mmc_defconfig configs/brppt1_nand_defconfig configs/brppt1_spi_defconfig configs/brxre1_defconfig configs/cm_t3517_defconfig configs/cm_t35_defconfig configs/picosam9g45_defconfig configs/pm9261_defconfig configs/pm9263_defconfig configs/sama5d36ek_cmp_mmc_defconfig configs/sama5d36ek_cmp_nandflash_defconfig configs/sama5d36ek_cmp_spiflash_defconfig configs/sama5d3xek_mmc_defconfig configs/sama5d3xek_nandflash_defconfig configs/sama5d3xek_spiflash_defconfig configs/sama5d4ek_mmc_defconfig configs/sama5d4ek_nandflash_defconfig configs/sama5d4ek_spiflash_defconfig configs/zipitz2_defconfig
Not really. I suspect none of those uses EFI_LOADER
Why not? I really don't want to limit EFI_LOADER to something I consider good. It's supposed to be the *one* interface that just works for everyone.
There is video driver for atmel which is most of the boards in that list, but we can disable EFI_LOADER until they are converted.
No, I won't disable EFI_LOADER on any board because it's not converted. I'd rather add support to EFI_LOADER to support more boards that are not converted to DM ;).
We should avoid adding new features to legacy code paths as it makes DM conversion harder and less likely to complete.
I agree, but the solution is not to disable EFI_LOADER, it's to convert boards.
So what is the conclusion on this patch? I can re-send with a commit msg (although there is not much to say beyond $subject).. I kinda think we should merge this for now, unless dropping CONFIG_LCD is imminent in which case I can re-work it to drop the CONFIG_LCD and add CONFIG_DM_VIDEO.. EFI_LOADER should definitely support the non-legacy case and probably should not remove the legacy case until it does not exist anymore.
Yes I think resend with a commit message and apply it.
I'll look at a patch to make EFI_LOADER depend on DM, which I think should have been done at the start. Supporting legacy code paths with new features is just not a good idea.
Well, that's very much not how the EFI_LOADER maintainer wants go :) I think I'd rather go with disabling the video side of the various boards, until they're converted.
removing just the legacy video part of things would clean up a small bit of the mess.. but fixing the efi block devices (disks) stuff depends somewhat on DM.. I'm leaving the legacy case in place (just as broken as it currently is), but it is messy.
BR, -R
On Tue, Jul 18, 2017 at 11:06 AM, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:54 PM, Simon Glass wrote:
Hi Alex,
On 18 July 2017 at 07:47, Alexander Graf agraf@suse.de wrote:
On 07/18/2017 04:00 PM, Simon Glass wrote:
Hi,
On 12 July 2017 at 05:52, Alexander Graf agraf@suse.de wrote:
On 25.06.17 01:05, Rob Clark wrote:
Signed-off-by: Rob Clark robdclark@gmail.com Cc: Alexander Graf agraf@suse.de
Looks reasonable to me, but could probably use a commit message ;). Also please make sure to CC Simon on all things DM.
Can we drop the CONFIG_LCD support entirely? This is legacy code at this point. What boards use it?
Sounds like someone would first need to convert a bunch of boards :).
$ for i in $(grep CONFIG_LCD configs/* | cut -d : -f 1); do grep -q DM_VIDEO $i || echo $i; done configs/at91sam9261ek_dataflash_cs0_defconfig configs/at91sam9261ek_dataflash_cs3_defconfig configs/at91sam9261ek_nandflash_defconfig configs/at91sam9263ek_dataflash_cs0_defconfig configs/at91sam9263ek_dataflash_defconfig configs/at91sam9263ek_nandflash_defconfig configs/at91sam9263ek_norflash_boot_defconfig configs/at91sam9263ek_norflash_defconfig configs/at91sam9g10ek_dataflash_cs0_defconfig configs/at91sam9g10ek_dataflash_cs3_defconfig configs/at91sam9g10ek_nandflash_defconfig configs/at91sam9m10g45ek_mmc_defconfig configs/at91sam9m10g45ek_nandflash_defconfig configs/at91sam9n12ek_mmc_defconfig configs/at91sam9n12ek_nandflash_defconfig configs/at91sam9n12ek_spiflash_defconfig configs/at91sam9rlek_dataflash_defconfig configs/at91sam9rlek_mmc_defconfig configs/at91sam9rlek_nandflash_defconfig configs/at91sam9x5ek_dataflash_defconfig configs/at91sam9x5ek_mmc_defconfig configs/at91sam9x5ek_nandflash_defconfig configs/at91sam9x5ek_spiflash_defconfig configs/brppt1_mmc_defconfig configs/brppt1_nand_defconfig configs/brppt1_spi_defconfig configs/brxre1_defconfig configs/cm_t3517_defconfig configs/cm_t35_defconfig configs/picosam9g45_defconfig configs/pm9261_defconfig configs/pm9263_defconfig configs/sama5d36ek_cmp_mmc_defconfig configs/sama5d36ek_cmp_nandflash_defconfig configs/sama5d36ek_cmp_spiflash_defconfig configs/sama5d3xek_mmc_defconfig configs/sama5d3xek_nandflash_defconfig configs/sama5d3xek_spiflash_defconfig configs/sama5d4ek_mmc_defconfig configs/sama5d4ek_nandflash_defconfig configs/sama5d4ek_spiflash_defconfig configs/zipitz2_defconfig
Not really. I suspect none of those uses EFI_LOADER
Why not? I really don't want to limit EFI_LOADER to something I consider good. It's supposed to be the *one* interface that just works for everyone.
So, thinking about this a bit.. I'd argue that EFI_LOADER doesn't actually work (for everyone.. or perhaps anyone) in it's current state (I assume opensuse must have some grub patches, grub master fails to grok the devicepaths EFI_LOADER creates).
In particular, we currently get devicepaths that look like:
/File(usb_mass_storage.lun0)/EndEntire
But they should really include parent devices / bus in the path. Currently this prevents grub from finding and automatically loading grub.cfg. (It "works" if you manually enter some grub commands.. but I don't really consider that "working".)
(If you have a board where it actually works with mainline grub, please apply that hack/workaround patch I sent yesterday to not crash grub's lsefi command and send me the output of lsefi.. I'm actually curious how this worked for anyone.)
I have some wip patches to map u-boots device model to more complete EFI devicepaths. The legacy case is somewhat in the way. (Currently I'm just trying to make sure the legacy case doesn't suck worse than it currently does.. but it would be nice to be able to blow it away.)
And tbh, my personal opinion (as someone who spends most time working on stuff other than u-boot), the legacy cases make the code much harder to understand for a newbie.. if it isn't going to go away real soon now, I'd say there is an argument for forking a legacy u-boot branch, then stripping out the core legacy bits and marking drivers/boards that aren't converted as depends on BROKEN on master.
BR, -R
There is video driver for atmel which is most of the boards in that list, but we can disable EFI_LOADER until they are converted.
No, I won't disable EFI_LOADER on any board because it's not converted. I'd rather add support to EFI_LOADER to support more boards that are not converted to DM ;).
We should avoid adding new features to legacy code paths as it makes DM conversion harder and less likely to complete.
I agree, but the solution is not to disable EFI_LOADER, it's to convert boards.
Alex
Signed-off-by: Rob Clark robdclark@gmail.com --- arch/arm/Kconfig | 2 +- configs/dragonboard410c_defconfig | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index dce4105..879dbc5 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -633,7 +633,7 @@ config ARCH_SNAPDRAGON select DM_SERIAL select SPMI select OF_CONTROL - select OF_SEPARATE + select OF_BOARD
config ARCH_SOCFPGA bool "Altera SOCFPGA family" diff --git a/configs/dragonboard410c_defconfig b/configs/dragonboard410c_defconfig index 90c145a..45a1218 100644 --- a/configs/dragonboard410c_defconfig +++ b/configs/dragonboard410c_defconfig @@ -37,3 +37,10 @@ CONFIG_USB_EHCI_MSM=y CONFIG_USB_ULPI_VIEWPORT=y CONFIG_USB_ULPI=y CONFIG_USB_STORAGE=y +CONFIG_DM_VIDEO=y +# CONFIG_VIDEO_BPP8 is not set +CONFIG_VIDEO_BPP16=y +CONFIG_VIDEO_BPP32=y +CONFIG_CONSOLE_NORMAL=y +CONFIG_VIDEO_SIMPLE=y +CONFIG_OF_BOARD=y
Hi Rob,
On 25.06.2017 01:05, Rob Clark wrote:
In particular, support for display setup by lk. This introduces a simplefb display driver that uses the framebuffer fdt node populated by the firmware[1] for u-boot display support (and, at least for what I am working on, more interestingly, EFI GOP support).
[...]
A few related patches so that on db410c we actually use the fdt passed by lk (and so that dm/core is clever enough to notice fdt nodes under "chosen"), config updates, and related fixes.
[...]
I finally had time to look at patches (this and the earlier series).
Enabling the output is pretty cool - I thought of that myself some time ago.
Question: Do you have non-standard partition layout? I have flashed most recent 96boards debian image, but my device doesn't have splash partition. (It initializes screen properly, just doesn't display anything)
Question2: Are you sure you have included all the patches?
I've applied this series (also included patches from 2017-06-20), but u-boot doesn't parse device tree properly and things like usb cease to work - both with your and official lk:
<log>
U-Boot 2017.07-rc3-00015-ga2592ee (Jul 06 2017 - 13:48:40 +0200) Qualcomm-DragonBoard 410C
DRAM: 986 MiB MMC: sdhci@07824000: 0, sdhci@07864000: 1 Using default environment
In: serial Out: serial Err: serial Failed to find PMIC pon node. Check device tree Net: Net Initialization Skipped No ethernet found. Hit any key to stop autoboot: 0
</log>
On the other hand it works properly with your pre-built binary [1].
I assume the difference may be that you have added proper (instead of fake) device tree into mkbootimg - is that the case?
Thanks, Mateusz
[1] https://github.com/robclark/lk/commits/db410c-display
Rob Clark (5): board/db410c: use fdt passed from lk dm: core: also parse chosen node video: simplefb efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD configs: db410c: config updates
arch/arm/Kconfig | 2 +- board/qualcomm/dragonboard410c/dragonboard410c.c | 16 ++++++ cmd/bootefi.c | 2 +- configs/dragonboard410c_defconfig | 7 +++ drivers/core/root.c | 22 +++++++- drivers/video/Kconfig | 10 ++++ drivers/video/Makefile | 2 +- drivers/video/simplefb.c | 68 ++++++++++++++++++++++++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_gop.c | 7 ++- 10 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 drivers/video/simplefb.c
On Thu, Jul 6, 2017 at 8:02 AM, Mateusz Kulikowski mateusz.kulikowski@gmail.com wrote:
Hi Rob,
On 25.06.2017 01:05, Rob Clark wrote:
In particular, support for display setup by lk. This introduces a simplefb display driver that uses the framebuffer fdt node populated by the firmware[1] for u-boot display support (and, at least for what I am working on, more interestingly, EFI GOP support).
[...]
A few related patches so that on db410c we actually use the fdt passed by lk (and so that dm/core is clever enough to notice fdt nodes under "chosen"), config updates, and related fixes.
[...]
I finally had time to look at patches (this and the earlier series).
Enabling the output is pretty cool - I thought of that myself some time ago.
Question: Do you have non-standard partition layout? I have flashed most recent 96boards debian image, but my device doesn't have splash partition. (It initializes screen properly, just doesn't display anything)
yes, I have a modified version of bootloader package from linaro:
https://people.freedesktop.org/~robclark/tmp/dragonboard410c_bootloader_emmc...
I split the boot partition (which was originally big enough for kernel+initrd which is somewhat larger than u-boot.img) to make a splash partition. I probably should write up instructions for how to do that and generate the splash.img (but it isn't too hard to find with a bit of googling)..
It isn't the latest lk + u-boot, on the lk side I have some fixes I still need to push to make it deal properly with #address/size-cells and a few other misc things. (And on the u-boot side, adding a reserved-memory node in the dragonboard410c.dts)..
Question2: Are you sure you have included all the patches?
I've applied this series (also included patches from 2017-06-20), but u-boot doesn't parse device tree properly and things like usb cease to work - both with your and official lk:
<log>
U-Boot 2017.07-rc3-00015-ga2592ee (Jul 06 2017 - 13:48:40 +0200) Qualcomm-DragonBoard 410C
DRAM: 986 MiB MMC: sdhci@07824000: 0, sdhci@07864000: 1 Using default environment
In: serial Out: serial Err: serial Failed to find PMIC pon node. Check device tree Net: Net Initialization Skipped No ethernet found. Hit any key to stop autoboot: 0
</log>
On the other hand it works properly with your pre-built binary [1].
I assume the difference may be that you have added proper (instead of fake) device tree into mkbootimg - is that the case?
I've (mostly) been using the dragonboard410c.dtb from u-boot build, since there seem to be some small problems in the other direction.. u-boot isn't completely compatible with upstream linux dtb.
I did find a few things that were only working after I shifted to CONFIG_OF_BOARD because I hadn't done a clean build (ie. still picking up old .dtb)
I'm working on cleaning up things, and I'll try to write up some better instructions and push things to github branch this afternoon, to make it a bit more reproducible
BR, -R
Thanks, Mateusz
[1] https://github.com/robclark/lk/commits/db410c-display
Rob Clark (5): board/db410c: use fdt passed from lk dm: core: also parse chosen node video: simplefb efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD configs: db410c: config updates
arch/arm/Kconfig | 2 +- board/qualcomm/dragonboard410c/dragonboard410c.c | 16 ++++++ cmd/bootefi.c | 2 +- configs/dragonboard410c_defconfig | 7 +++ drivers/core/root.c | 22 +++++++- drivers/video/Kconfig | 10 ++++ drivers/video/Makefile | 2 +- drivers/video/simplefb.c | 68 ++++++++++++++++++++++++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_gop.c | 7 ++- 10 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 drivers/video/simplefb.c
On Wed, Jul 19, 2017 at 12:49 PM, Rob Clark robdclark@gmail.com wrote:
On Thu, Jul 6, 2017 at 8:02 AM, Mateusz Kulikowski mateusz.kulikowski@gmail.com wrote:
Hi Rob,
On 25.06.2017 01:05, Rob Clark wrote:
In particular, support for display setup by lk. This introduces a simplefb display driver that uses the framebuffer fdt node populated by the firmware[1] for u-boot display support (and, at least for what I am working on, more interestingly, EFI GOP support).
[...]
A few related patches so that on db410c we actually use the fdt passed by lk (and so that dm/core is clever enough to notice fdt nodes under "chosen"), config updates, and related fixes.
[...]
I finally had time to look at patches (this and the earlier series).
Enabling the output is pretty cool - I thought of that myself some time ago.
Question: Do you have non-standard partition layout? I have flashed most recent 96boards debian image, but my device doesn't have splash partition. (It initializes screen properly, just doesn't display anything)
yes, I have a modified version of bootloader package from linaro:
https://people.freedesktop.org/~robclark/tmp/dragonboard410c_bootloader_emmc...
I split the boot partition (which was originally big enough for kernel+initrd which is somewhat larger than u-boot.img) to make a splash partition. I probably should write up instructions for how to do that and generate the splash.img (but it isn't too hard to find with a bit of googling)..
It isn't the latest lk + u-boot, on the lk side I have some fixes I still need to push to make it deal properly with #address/size-cells and a few other misc things. (And on the u-boot side, adding a reserved-memory node in the dragonboard410c.dts)..
Question2: Are you sure you have included all the patches?
I've applied this series (also included patches from 2017-06-20), but u-boot doesn't parse device tree properly and things like usb cease to work - both with your and official lk:
<log>
U-Boot 2017.07-rc3-00015-ga2592ee (Jul 06 2017 - 13:48:40 +0200) Qualcomm-DragonBoard 410C
DRAM: 986 MiB MMC: sdhci@07824000: 0, sdhci@07864000: 1 Using default environment
In: serial Out: serial Err: serial Failed to find PMIC pon node. Check device tree Net: Net Initialization Skipped No ethernet found. Hit any key to stop autoboot: 0
</log>
On the other hand it works properly with your pre-built binary [1].
I assume the difference may be that you have added proper (instead of fake) device tree into mkbootimg - is that the case?
I've (mostly) been using the dragonboard410c.dtb from u-boot build, since there seem to be some small problems in the other direction.. u-boot isn't completely compatible with upstream linux dtb.
I did find a few things that were only working after I shifted to CONFIG_OF_BOARD because I hadn't done a clean build (ie. still picking up old .dtb)
I'm working on cleaning up things, and I'll try to write up some better instructions and push things to github branch this afternoon, to make it a bit more reproducible
jfyi, I've pushed latest of what I'm using (both with lk -> kernel(simplefb) and with lk -> u-boot -> grub -> kernel(efifb) here:
https://github.com/robclark/lk/commits/db410c-display https://github.com/robclark/u-boot/commits/wip
BR, -R
participants (6)
-
Alexander Graf -
Mateusz Kulikowski -
Rob Clark -
Simon Glass -
Tom Rini -
Wenyou.Yang＠microchip.com