[U-Boot] [PATCH 0/5] db410c: updates for grub + gfxterm

[U-Boot] [PATCH 0/5] db410c: updates for grub + gfxterm

[Rob Clark]

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

-- 
2.9.4

[U-Boot] [PATCH 1/5] board/db410c: use fdt passed from lk

[Rob Clark]

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)
 {
-- 
2.9.4

[U-Boot] [PATCH 2/5] dm: core: also parse chosen node

[Rob Clark]

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),
-- 
2.9.4

[U-Boot] [PATCH 3/5] video: simplefb

[Rob Clark]

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,
+};
-- 
2.9.4

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Rob Clark]

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);
-- 
2.9.4

[U-Boot] [PATCH 5/5] configs: db410c: config updates

[Rob Clark]

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
-- 
2.9.4

[U-Boot] [PATCH 0/5] db410c: updates for grub + gfxterm

[Mateusz Kulikowski]

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 at 07824000: 0, sdhci at 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
> 

[U-Boot] [PATCH 1/5] board/db410c: use fdt passed from lk

[Tom Rini]

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170707/f7701db4/attachment.sig>

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Alexander Graf]

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);
> 

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Simon Glass]

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;
>>   };
>>     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);
>>
>

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Alexander Graf]

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

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Simon Glass]

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

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Alexander Graf]

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

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Tom Rini]

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...

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170718/25d86f21/attachment.sig>

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Wenyou.Yang at microchip.com]

Hi

> -----Original Message-----
> From: Tom Rini [mailto:trini at 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

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Rob Clark]

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

[U-Boot] [PATCH 0/5] db410c: updates for grub + gfxterm

[Rob Clark]

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_linux-79-u-boot.tgz

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 at 07824000: 0, sdhci at 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
>>
>

[U-Boot] [PATCH 0/5] db410c: updates for grub + gfxterm

[Rob Clark]

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_linux-79-u-boot.tgz
>
> 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 at 07824000: 0, sdhci at 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

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Rob Clark]

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
>

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Simon Glass]

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

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Tom Rini]

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170721/ba50e5f4/attachment.sig>

[U-Boot] [PATCH 4/5] efi_loader: gop: fixes for CONFIG_DM_VIDEO without CONFIG_LCD

[Rob Clark]

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