diff mbox series

tbs2910: migrate to DM_VIDEO

Message ID eb4bf791-04dd-6597-6b6d-0e8893cd28dd@web.de
State New
Headers show
Series tbs2910: migrate to DM_VIDEO | expand

Commit Message

Soeren Moch May 24, 2020, 3:46 p.m. UTC 
On 23.05.20 01:24, Anatolij Gustschin wrote:
> Migration to DM_VIDEO driver is long overdue, configure it in
> board config files. To enable the display set stdout like:
>
>   setenv stdout serial,vidconsole
>
> Signed-off-by: Anatolij Gustschin <agust at denx.de>
> ---
> This is only build tested.
I tested this on top of v2020.07-rc2 and your patch
"video: extend stdout video console work-around for 'vga'"

Works great. However, some comments below.
>
>  configs/tbs2910_defconfig | 7 ++++++-
>  include/configs/tbs2910.h | 2 --
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
> index 2ff0e160f7..b40641c17b 100644
> --- a/configs/tbs2910_defconfig
> +++ b/configs/tbs2910_defconfig
> @@ -1,6 +1,7 @@
>  CONFIG_ARM=y
>  CONFIG_ARCH_MX6=y
>  CONFIG_SYS_TEXT_BASE=0x17800000
> +CONFIG_SYS_MALLOC_F_LEN=0x4000
This is not necessary. The default 0x2000 also works fine.
>  CONFIG_ENV_SIZE=0x2000
>  CONFIG_ENV_OFFSET=0x60000
>  CONFIG_TARGET_TBS2910=y
> @@ -84,7 +85,11 @@ CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5
>  CONFIG_CI_UDC=y
>  CONFIG_USB_GADGET_DOWNLOAD=y
>  CONFIG_I2C_EDID=y
> +CONFIG_DM_VIDEO=y
> +# CONFIG_VIDEO_BPP8 is not set
> +# CONFIG_VIDEO_BPP32 is not set
> +# CONFIG_VIDEO_ANSI is not set
>  CONFIG_VIDEO_IPUV3=y
> -CONFIG_VIDEO=y
> +CONFIG_SYS_WHITE_ON_BLACK=y
>  CONFIG_OF_LIBFDT_ASSUME_MASK=0xff
>  # CONFIG_EFI_LOADER is not set
> diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
> index 7376b91f55..8ab3fcfe3e 100644
> --- a/include/configs/tbs2910.h
> +++ b/include/configs/tbs2910.h
> @@ -37,11 +37,9 @@
>  #define CONFIG_MXC_UART_BASE		UART1_BASE /* select UART1/UART2 */
>
>  /* Framebuffer */
> -#ifdef CONFIG_VIDEO
>  #define CONFIG_VIDEO_BMP_RLE8
>  #define CONFIG_IMX_HDMI
>  #define CONFIG_IMX_VIDEO_SKIP
> -#endif
>
>  /* PCI */
>  #ifdef CONFIG_CMD_PCI
Since DM_VIDEO requires vidconsole as output device, please also add

---8<---
---8<---

to avoid warnings for users with default environment.(not sure if
whitespace is still correct in this snippet)

With this DM_VIDEO conversion the board comes very close to the size
limit. While it works with my toolchain, there might be problems with
others. So maybe not a good idea to merge this as fix immediately.

Since this depends on your above mentioned patch, do you plan to merge
this via u-boot-video, or should this go as usual through the imx tree?

Thank you very much for finishing this DM_VIDEO migration,
Soeren

Comments

Anatolij Gustschin May 26, 2020, 8:13 p.m. UTC | #1 
On Sun, 24 May 2020 17:46:22 +0200
Soeren Moch smoch at web.de wrote:

> On 23.05.20 01:24, Anatolij Gustschin wrote:
> > Migration to DM_VIDEO driver is long overdue, configure it in
> > board config files. To enable the display set stdout like:
> >
> >   setenv stdout serial,vidconsole
> >
> > Signed-off-by: Anatolij Gustschin <agust at denx.de>
> > ---
> > This is only build tested.  
> I tested this on top of v2020.07-rc2 and your patch
> "video: extend stdout video console work-around for 'vga'"
> 
> Works great. However, some comments below.

Thanks for testing!

...
> > +CONFIG_SYS_MALLOC_F_LEN=0x4000  
> This is not necessary. The default 0x2000 also works fine.

Dropped in v2.

...
> Since DM_VIDEO requires vidconsole as output device, please also add
> 
> ---8<---
> diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
> index 8ab3fcfe3e..82165a9ffe 100644
> --- a/include/configs/tbs2910.h
> +++ b/include/configs/tbs2910.h
> @@ -92,11 +92,11 @@
> ??????? "fan=gpio set 92\0" \
> ??????? "set_con_serial=setenv stdout serial; " \
> ??????????????????????? "setenv stderr serial\0" \
> -?????? "set_con_hdmi=setenv stdout serial,vga; " \
> -?????????????????????? "setenv stderr serial,vga\0" \
> -?????? "stderr=serial,vga\0" \
> +?????? "set_con_hdmi=setenv stdout serial,vidconsole; " \
> +?????????????????????? "setenv stderr serial,vidconsole\0" \
> +?????? "stderr=serial,vidconsole\0" \
> ??????? "stdin=serial,usbkbd\0" \
> -?????? "stdout=serial,vga\0"
> +?????? "stdout=serial,vidconsole\0"
> ?
> ?#define CONFIG_BOOTCOMMAND \
> ??????? "mmc rescan; " \
> ---8<---
> 
> to avoid warnings for users with default environment.(not sure if
> whitespace is still correct in this snippet)

Done in v2.

> With this DM_VIDEO conversion the board comes very close to the size
> limit. While it works with my toolchain, there might be problems with
> others. So maybe not a good idea to merge this as fix immediately.

Yes, with GCC 9.2 it didn't work any more. I've submitted some DM_VIDEO
patches to disable more unused code to fix this.
 
> Since this depends on your above mentioned patch, do you plan to merge
> this via u-boot-video, or should this go as usual through the imx tree?

I'll merge this via u-boot-video tree. v2 patch depends on a few more
video patches which are under review/built-test currently.

--
Anatolij
Soeren Moch May 28, 2020, 7:54 a.m. UTC | #2 
On 26.05.20 22:13, Anatolij Gustschin wrote:
> On Sun, 24 May 2020 17:46:22 +0200
> Soeren Moch smoch at web.de wrote:
>
>> On 23.05.20 01:24, Anatolij Gustschin wrote:
>>> Migration to DM_VIDEO driver is long overdue, configure it in
>>> board config files. To enable the display set stdout like:
>>>
>>>   setenv stdout serial,vidconsole
>>>
>>> Signed-off-by: Anatolij Gustschin <agust at denx.de>
>>> ---
>>> This is only build tested.
>> I tested this on top of v2020.07-rc2 and your patch
>> "video: extend stdout video console work-around for 'vga'"
>>
>> Works great. However, some comments below.
> Thanks for testing!
>
> ...
>>> +CONFIG_SYS_MALLOC_F_LEN=0x4000
>> This is not necessary. The default 0x2000 also works fine.
> Dropped in v2.
>
> ...
>> Since DM_VIDEO requires vidconsole as output device, please also add
>>
>> ---8<---
>> diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
>> index 8ab3fcfe3e..82165a9ffe 100644
>> --- a/include/configs/tbs2910.h
>> +++ b/include/configs/tbs2910.h
>> @@ -92,11 +92,11 @@
>> ??????? "fan=gpio set 92\0" \
>> ??????? "set_con_serial=setenv stdout serial; " \
>> ??????????????????????? "setenv stderr serial\0" \
>> -?????? "set_con_hdmi=setenv stdout serial,vga; " \
>> -?????????????????????? "setenv stderr serial,vga\0" \
>> -?????? "stderr=serial,vga\0" \
>> +?????? "set_con_hdmi=setenv stdout serial,vidconsole; " \
>> +?????????????????????? "setenv stderr serial,vidconsole\0" \
>> +?????? "stderr=serial,vidconsole\0" \
>> ??????? "stdin=serial,usbkbd\0" \
>> -?????? "stdout=serial,vga\0"
>> +?????? "stdout=serial,vidconsole\0"
>> ?
>> ?#define CONFIG_BOOTCOMMAND \
>> ??????? "mmc rescan; " \
>> ---8<---
>>
>> to avoid warnings for users with default environment.(not sure if
>> whitespace is still correct in this snippet)
> Done in v2.
>
>> With this DM_VIDEO conversion the board comes very close to the size
>> limit. While it works with my toolchain, there might be problems with
>> others. So maybe not a good idea to merge this as fix immediately.
> Yes, with GCC 9.2 it didn't work any more. I've submitted some DM_VIDEO
> patches to disable more unused code to fix this.
>
>> Since this depends on your above mentioned patch, do you plan to merge
>> this via u-boot-video, or should this go as usual through the imx tree?
> I'll merge this via u-boot-video tree. v2 patch depends on a few more
> video patches which are under review/built-test currently.
OK, I will try to test this new version again on top of your other patches.

Do you plan to merge this for v2020.07, or for -next?

Soeren
Anatolij Gustschin May 28, 2020, 8:07 a.m. UTC | #3 
On Thu, 28 May 2020 09:54:42 +0200
Soeren Moch smoch at web.de wrote:
...
> > I'll merge this via u-boot-video tree. v2 patch depends on a few more
> > video patches which are under review/built-test currently.  
> OK, I will try to test this new version again on top of your other patches.

OK, you could use 'dm_video-imx6' branch in my repo:
 https://gitlab.denx.de/u-boot/custodians/u-boot-video/-/commits/dm_video-imx6

All required patches are already integrated there.

> Do you plan to merge this for v2020.07, or for -next?

For v2020.07 it is too late, Tom suggested to merge this for -next.

--
Anatolij
Soeren Moch May 30, 2020, 4:01 p.m. UTC | #4 
On 28.05.20 10:07, Anatolij Gustschin wrote:
> On Thu, 28 May 2020 09:54:42 +0200
> Soeren Moch smoch at web.de wrote:
> ...
>>> I'll merge this via u-boot-video tree. v2 patch depends on a few more
>>> video patches which are under review/built-test currently.
>> OK, I will try to test this new version again on top of your other patches.
> OK, you could use 'dm_video-imx6' branch in my repo:
>  https://gitlab.denx.de/u-boot/custodians/u-boot-video/-/commits/dm_video-imx6
>
> All required patches are already integrated there.
Still works as expected.
The tbs2910-patch already has my tested-by, I could not find this series
as a whole to send a tag for this.
>> Do you plan to merge this for v2020.07, or for -next?
> For v2020.07 it is too late, Tom suggested to merge this for -next.

OK, thanks,
Soeren
diff mbox series

Patch

diff --git a/include/configs/tbs2910.h b/include/configs/tbs2910.h
index 8ab3fcfe3e..82165a9ffe 100644
--- a/include/configs/tbs2910.h
+++ b/include/configs/tbs2910.h
@@ -92,11 +92,11 @@ 
??????? "fan=gpio set 92\0" \
??????? "set_con_serial=setenv stdout serial; " \
??????????????????????? "setenv stderr serial\0" \
-?????? "set_con_hdmi=setenv stdout serial,vga; " \
-?????????????????????? "setenv stderr serial,vga\0" \
-?????? "stderr=serial,vga\0" \
+?????? "set_con_hdmi=setenv stdout serial,vidconsole; " \
+?????????????????????? "setenv stderr serial,vidconsole\0" \
+?????? "stderr=serial,vidconsole\0" \
??????? "stdin=serial,usbkbd\0" \
-?????? "stdout=serial,vga\0"
+?????? "stdout=serial,vidconsole\0"
?
?#define CONFIG_BOOTCOMMAND \
??????? "mmc rescan; " \