| Message ID | 20181001182249.129565-44-sjg@chromium.org |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | Various fixes and improvements | expand |
Hi Simon, On Mon, 1 Oct 2018 12:22:47 -0600 Simon Glass sjg@chromium.org wrote: > At present this function uses pixels but it seems more useful for it to > position in terms of characters on the screen. This also matches the > comment to the function. Update this. > > Unfortunately there is one user of this function (at91). Have a crack at > fixing this, since I cannot test it. > > Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Anatolij Gustschin <agust@denx.de> > --- > > board/atmel/common/video_display.c | 5 ++++- > drivers/video/vidconsole-uclass.c | 2 ++ > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/board/atmel/common/video_display.c b/board/atmel/common/video_display.c > index 7dd7b85556e..e02cb00f866 100644 > --- a/board/atmel/common/video_display.c > +++ b/board/atmel/common/video_display.c > @@ -18,6 +18,7 @@ DECLARE_GLOBAL_DATA_PTR; > > int at91_video_show_board_info(void) > { > + struct vidconsole_priv *priv; > ulong dram_size, nand_size; > int i; > u32 len = 0; > @@ -63,7 +64,9 @@ int at91_video_show_board_info(void) > if (ret) > return ret; > > - vidconsole_position_cursor(con, 0, logo_info.logo_height); > + priv = dev_get_uclass_priv(con); > + vidconsole_position_cursor(con, 0, (logo_info.logo_height + > + con->y_charsize - 1) / con->y_charsize); Shouldn't it be priv->y_charsize? 'con' is struct udevice * and doesn't have y_charsize. -- Anatolij
On 01.10.2018 23:22, Anatolij Gustschin wrote: > Hi Simon, > > On Mon, 1 Oct 2018 12:22:47 -0600 > Simon Glass sjg@chromium.org wrote: > >> At present this function uses pixels but it seems more useful for it to >> position in terms of characters on the screen. This also matches the >> comment to the function. Update this. >> >> Unfortunately there is one user of this function (at91). Have a crack at >> fixing this, since I cannot test it. Hello Simon, I will gladly test this for you on at91 board, but I am having some issues applying your patch series: Applying: binman: Move to three-digit test-file numbers error: patch failed: tools/binman/entry_test.py:25 error: tools/binman/entry_test.py: patch does not apply error: patch failed: tools/binman/ftest.py:712 error: tools/binman/ftest.py: patch does not apply Patch failed at 0026 binman: Move to three-digit test-file numbers Do you have them in some public tree I can pull from ? Also, any specific tests you would like except just checking the video console ? Eugen >> >> Signed-off-by: Simon Glass <sjg@chromium.org> > > Reviewed-by: Anatolij Gustschin <agust@denx.de> > >> --- >> >> board/atmel/common/video_display.c | 5 ++++- >> drivers/video/vidconsole-uclass.c | 2 ++ >> 2 files changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/board/atmel/common/video_display.c b/board/atmel/common/video_display.c >> index 7dd7b85556e..e02cb00f866 100644 >> --- a/board/atmel/common/video_display.c >> +++ b/board/atmel/common/video_display.c >> @@ -18,6 +18,7 @@ DECLARE_GLOBAL_DATA_PTR; >> >> int at91_video_show_board_info(void) >> { >> + struct vidconsole_priv *priv; >> ulong dram_size, nand_size; >> int i; >> u32 len = 0; >> @@ -63,7 +64,9 @@ int at91_video_show_board_info(void) >> if (ret) >> return ret; >> >> - vidconsole_position_cursor(con, 0, logo_info.logo_height); >> + priv = dev_get_uclass_priv(con); >> + vidconsole_position_cursor(con, 0, (logo_info.logo_height + >> + con->y_charsize - 1) / con->y_charsize); > > Shouldn't it be priv->y_charsize? 'con' is struct udevice * and doesn't > have y_charsize. > > > -- > Anatolij > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot >
Hi Eugen, On 2 October 2018 at 01:37, Eugen Hristev <eugen.hristev@microchip.com> wrote: > > > > On 01.10.2018 23:22, Anatolij Gustschin wrote: >> >> Hi Simon, >> >> On Mon, 1 Oct 2018 12:22:47 -0600 >> Simon Glass sjg@chromium.org wrote: >> >>> At present this function uses pixels but it seems more useful for it to >>> position in terms of characters on the screen. This also matches the >>> comment to the function. Update this. >>> >>> Unfortunately there is one user of this function (at91). Have a crack at >>> fixing this, since I cannot test it. > > > Hello Simon, > > I will gladly test this for you on at91 board, > but I am having some issues applying your patch series: > > Applying: binman: Move to three-digit test-file numbers > error: patch failed: tools/binman/entry_test.py:25 > error: tools/binman/entry_test.py: patch does not apply > error: patch failed: tools/binman/ftest.py:712 > error: tools/binman/ftest.py: patch does not apply > Patch failed at 0026 binman: Move to three-digit test-file numbers > > Do you have them in some public tree I can pull from ? Yes you can try u-boot-dm/testing > > Also, any specific tests you would like except just checking the video console ? It looks like this code runs when the board boots up, so just starting it should be enough. Thanks, Simon > > Eugen > > >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >> >> >> Reviewed-by: Anatolij Gustschin <agust@denx.de> >> >>> --- >>> >>> board/atmel/common/video_display.c | 5 ++++- >>> drivers/video/vidconsole-uclass.c | 2 ++ >>> 2 files changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/board/atmel/common/video_display.c b/board/atmel/common/video_display.c >>> index 7dd7b85556e..e02cb00f866 100644 >>> --- a/board/atmel/common/video_display.c >>> +++ b/board/atmel/common/video_display.c >>> @@ -18,6 +18,7 @@ DECLARE_GLOBAL_DATA_PTR; >>> int at91_video_show_board_info(void) >>> { >>> + struct vidconsole_priv *priv; >>> ulong dram_size, nand_size; >>> int i; >>> u32 len = 0; >>> @@ -63,7 +64,9 @@ int at91_video_show_board_info(void) >>> if (ret) >>> return ret; >>> - vidconsole_position_cursor(con, 0, logo_info.logo_height); >>> + priv = dev_get_uclass_priv(con); >>> + vidconsole_position_cursor(con, 0, (logo_info.logo_height + >>> + con->y_charsize - 1) / con->y_charsize); >> >> >> Shouldn't it be priv->y_charsize? 'con' is struct udevice * and doesn't >> have y_charsize. >> >> >> -- >> Anatolij >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> https://lists.denx.de/listinfo/u-boot >>
Hi Anatolij, On 1 October 2018 at 14:22, Anatolij Gustschin <agust@denx.de> wrote: > Hi Simon, > > On Mon, 1 Oct 2018 12:22:47 -0600 > Simon Glass sjg@chromium.org wrote: > >> At present this function uses pixels but it seems more useful for it to >> position in terms of characters on the screen. This also matches the >> comment to the function. Update this. >> >> Unfortunately there is one user of this function (at91). Have a crack at >> fixing this, since I cannot test it. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> > > Reviewed-by: Anatolij Gustschin <agust@denx.de> > >> --- >> >> board/atmel/common/video_display.c | 5 ++++- >> drivers/video/vidconsole-uclass.c | 2 ++ >> 2 files changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/board/atmel/common/video_display.c b/board/atmel/common/video_display.c >> index 7dd7b85556e..e02cb00f866 100644 >> --- a/board/atmel/common/video_display.c >> +++ b/board/atmel/common/video_display.c >> @@ -18,6 +18,7 @@ DECLARE_GLOBAL_DATA_PTR; >> >> int at91_video_show_board_info(void) >> { >> + struct vidconsole_priv *priv; >> ulong dram_size, nand_size; >> int i; >> u32 len = 0; >> @@ -63,7 +64,9 @@ int at91_video_show_board_info(void) >> if (ret) >> return ret; >> >> - vidconsole_position_cursor(con, 0, logo_info.logo_height); >> + priv = dev_get_uclass_priv(con); >> + vidconsole_position_cursor(con, 0, (logo_info.logo_height + >> + con->y_charsize - 1) / con->y_charsize); > > Shouldn't it be priv->y_charsize? 'con' is struct udevice * and doesn't > have y_charsize. Yes thanks, I will fix that when I apply this. Regards, Simon > > > -- > Anatolij
On 09.10.2018 06:40, Simon Glass wrote: > Hi Eugen, > > On 2 October 2018 at 01:37, Eugen Hristev <eugen.hristev@microchip.com> wrote: >> >> >> >> On 01.10.2018 23:22, Anatolij Gustschin wrote: >>> >>> Hi Simon, >>> >>> On Mon, 1 Oct 2018 12:22:47 -0600 >>> Simon Glass sjg@chromium.org wrote: >>> >>>> At present this function uses pixels but it seems more useful for it to >>>> position in terms of characters on the screen. This also matches the >>>> comment to the function. Update this. >>>> >>>> Unfortunately there is one user of this function (at91). Have a crack at >>>> fixing this, since I cannot test it. >> >> >> Hello Simon, >> >> I will gladly test this for you on at91 board, >> but I am having some issues applying your patch series: >> >> Applying: binman: Move to three-digit test-file numbers >> error: patch failed: tools/binman/entry_test.py:25 >> error: tools/binman/entry_test.py: patch does not apply >> error: patch failed: tools/binman/ftest.py:712 >> error: tools/binman/ftest.py: patch does not apply >> Patch failed at 0026 binman: Move to three-digit test-file numbers >> >> Do you have them in some public tree I can pull from ? > > Yes you can try u-boot-dm/testing > >> >> Also, any specific tests you would like except just checking the video console ? > > It looks like this code runs when the board boots up, so just starting > it should be enough. I made a build on your branch and tested it on at91 sama5d2_xplained board, and the logo and text appears on the display. Tested-by: Eugen Hristev <eugen.hristev@microchip.com> Let me know if you want me to do more tests. Eugen > > Thanks, > Simon > > >> >> Eugen >> >> >>>> >>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> >>> >>> Reviewed-by: Anatolij Gustschin <agust@denx.de> >>> >>>> --- >>>> >>>> board/atmel/common/video_display.c | 5 ++++- >>>> drivers/video/vidconsole-uclass.c | 2 ++ >>>> 2 files changed, 6 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/board/atmel/common/video_display.c b/board/atmel/common/video_display.c >>>> index 7dd7b85556e..e02cb00f866 100644 >>>> --- a/board/atmel/common/video_display.c >>>> +++ b/board/atmel/common/video_display.c >>>> @@ -18,6 +18,7 @@ DECLARE_GLOBAL_DATA_PTR; >>>> int at91_video_show_board_info(void) >>>> { >>>> + struct vidconsole_priv *priv; >>>> ulong dram_size, nand_size; >>>> int i; >>>> u32 len = 0; >>>> @@ -63,7 +64,9 @@ int at91_video_show_board_info(void) >>>> if (ret) >>>> return ret; >>>> - vidconsole_position_cursor(con, 0, logo_info.logo_height); >>>> + priv = dev_get_uclass_priv(con); >>>> + vidconsole_position_cursor(con, 0, (logo_info.logo_height + >>>> + con->y_charsize - 1) / con->y_charsize); >>> >>> >>> Shouldn't it be priv->y_charsize? 'con' is struct udevice * and doesn't >>> have y_charsize. >>> >>> >>> -- >>> Anatolij >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot@lists.denx.de >>> https://lists.denx.de/listinfo/u-boot >>>
On 09.10.2018 06:40, Simon Glass wrote: > Hi Eugen, > > On 2 October 2018 at 01:37, Eugen Hristev <eugen.hristev@microchip.com> wrote: >> >> >> >> On 01.10.2018 23:22, Anatolij Gustschin wrote: >>> >>> Hi Simon, >>> >>> On Mon, 1 Oct 2018 12:22:47 -0600 >>> Simon Glass sjg@chromium.org wrote: >>> >>>> At present this function uses pixels but it seems more useful for it to >>>> position in terms of characters on the screen. This also matches the >>>> comment to the function. Update this. >>>> >>>> Unfortunately there is one user of this function (at91). Have a crack at >>>> fixing this, since I cannot test it. >> >> >> Hello Simon, >> >> I will gladly test this for you on at91 board, >> but I am having some issues applying your patch series: >> >> Applying: binman: Move to three-digit test-file numbers >> error: patch failed: tools/binman/entry_test.py:25 >> error: tools/binman/entry_test.py: patch does not apply >> error: patch failed: tools/binman/ftest.py:712 >> error: tools/binman/ftest.py: patch does not apply >> Patch failed at 0026 binman: Move to three-digit test-file numbers >> >> Do you have them in some public tree I can pull from ? > > Yes you can try u-boot-dm/testing > >> >> Also, any specific tests you would like except just checking the video console ? > > It looks like this code runs when the board boots up, so just starting > it should be enough. I made a build on your branch and tested it on at91 sama5d2_xplained board, and the logo and text appears on the display. Tested-by: Eugen Hristev <eugen.hristev@microchip.com> Let me know if you want me to do more tests. Eugen > > Thanks, > Simon > > >> >> Eugen >> >> >>>> >>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> >>> >>> Reviewed-by: Anatolij Gustschin <agust@denx.de> >>> >>>> --- >>>> >>>> board/atmel/common/video_display.c | 5 ++++- >>>> drivers/video/vidconsole-uclass.c | 2 ++ >>>> 2 files changed, 6 insertions(+), 1 deletion(-) >>>> Applied to u-boot-dm
diff --git a/board/atmel/common/video_display.c b/board/atmel/common/video_display.c index 7dd7b85556e..e02cb00f866 100644 --- a/board/atmel/common/video_display.c +++ b/board/atmel/common/video_display.c @@ -18,6 +18,7 @@ DECLARE_GLOBAL_DATA_PTR; int at91_video_show_board_info(void) { + struct vidconsole_priv *priv; ulong dram_size, nand_size; int i; u32 len = 0; @@ -63,7 +64,9 @@ int at91_video_show_board_info(void) if (ret) return ret; - vidconsole_position_cursor(con, 0, logo_info.logo_height); + priv = dev_get_uclass_priv(con); + vidconsole_position_cursor(con, 0, (logo_info.logo_height + + con->y_charsize - 1) / con->y_charsize); for (s = buf, i = 0; i < len; s++, i++) vidconsole_put_char(con, *s); diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index 89ac8b3cc8f..1874887f2f3 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -511,6 +511,8 @@ void vidconsole_position_cursor(struct udevice *dev, unsigned col, unsigned row) struct udevice *vid_dev = dev->parent; struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev); + col *= priv->x_charsize; + row *= priv->y_charsize; priv->xcur_frac = VID_TO_POS(min_t(short, col, vid_priv->xsize - 1)); priv->ycur = min_t(short, row, vid_priv->ysize - 1); }
At present this function uses pixels but it seems more useful for it to position in terms of characters on the screen. This also matches the comment to the function. Update this. Unfortunately there is one user of this function (at91). Have a crack at fixing this, since I cannot test it. Signed-off-by: Simon Glass <sjg@chromium.org> --- board/atmel/common/video_display.c | 5 ++++- drivers/video/vidconsole-uclass.c | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-)