[U-Boot] am335x-fb: don't override lcd_get_size()

From: David Rivshin <DRivshin@allworx.com>

am335x-fb overrides the default lcd_get_size() to add an extra 32 bytes
compared to the normal calculation. While the gd->fb_base is an extra 32
bytes larger than the logical framebuffer size, the first 32 bytes is
always skipped. Adding that extra 32 bytes in lcd_get_size() can cause
check_cache_range() (called from lcd_sync(), via flush_dcache_range()) to
output a warning because the end address is not cacheline aligned, even
if the start and real framebuffer size are.

When removing that "+ 0x20" the am335x-fb lcd_get_size() is identical
to the default one, so just delete it altogether.

Signed-off-by: David Rivshin <drivshin@allworx.com>
---
 drivers/video/am335x-fb.c | 6 ------
 1 file changed, 6 deletions(-)


base-commit: d3689267f92c5956e09cc7d1baa4700141662bff

On 2/21/19 5:05 PM, David Rivshin wrote:
> From: David Rivshin <DRivshin@allworx.com>
>
> am335x-fb overrides the default lcd_get_size() to add an extra 32 bytes
> compared to the normal calculation. While the gd->fb_base is an extra 32
> bytes larger than the logical framebuffer size, the first 32 bytes is
> always skipped. Adding that extra 32 bytes in lcd_get_size() can cause
> check_cache_range() (called from lcd_sync(), via flush_dcache_range()) to
> output a warning because the end address is not cacheline aligned, even
> if the start and real framebuffer size are.
>
> When removing that "+ 0x20" the am335x-fb lcd_get_size() is identical
> to the default one, so just delete it altogether.
>
> Signed-off-by: David Rivshin <drivshin@allworx.com>
> ---
>   drivers/video/am335x-fb.c | 6 ------
>   1 file changed, 6 deletions(-)
>
> diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c
> index 51c1af587f..3a510cf91c 100644
> --- a/drivers/video/am335x-fb.c
> +++ b/drivers/video/am335x-fb.c
> @@ -103,12 +103,6 @@ static struct am335x_lcdhw *lcdhw = (void *)LCD_CNTL_BASE;
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> -int lcd_get_size(int *line_length)
> -{
> -	*line_length = (panel_info.vl_col * NBITS(panel_info.vl_bpix)) / 8;
> -	return *line_length * panel_info.vl_row + 0x20;
> -}
> -
>   int am335xfb_init(struct am335x_lcdpanel *panel)
>   {
>   	u32 raster_ctrl = 0;
>
> base-commit: d3689267f92c5956e09cc7d1baa4700141662bff
I'll look into.
But it's not easy as it looks like.

Just downloaded latest TRM, looks like documentation had become better.

cheers,
Hannes

On Mon, 25 Feb 2019 12:54:57 +0100
Hannes Schmelzer <hannes@schmelzer.or.at> wrote:

> On 2/21/19 5:05 PM, David Rivshin wrote:
> > From: David Rivshin <DRivshin@allworx.com>
> >
> > am335x-fb overrides the default lcd_get_size() to add an extra 32 bytes
> > compared to the normal calculation. While the gd->fb_base is an extra 32
> > bytes larger than the logical framebuffer size, the first 32 bytes is
> > always skipped. Adding that extra 32 bytes in lcd_get_size() can cause
> > check_cache_range() (called from lcd_sync(), via flush_dcache_range()) to
> > output a warning because the end address is not cacheline aligned, even
> > if the start and real framebuffer size are.
> >
> > When removing that "+ 0x20" the am335x-fb lcd_get_size() is identical
> > to the default one, so just delete it altogether.
> >
> > Signed-off-by: David Rivshin <drivshin@allworx.com>
> > ---
> >   drivers/video/am335x-fb.c | 6 ------
> >   1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c
> > index 51c1af587f..3a510cf91c 100644
> > --- a/drivers/video/am335x-fb.c
> > +++ b/drivers/video/am335x-fb.c
> > @@ -103,12 +103,6 @@ static struct am335x_lcdhw *lcdhw = (void *)LCD_CNTL_BASE;
> >   
> >   DECLARE_GLOBAL_DATA_PTR;
> >   
> > -int lcd_get_size(int *line_length)
> > -{
> > -	*line_length = (panel_info.vl_col * NBITS(panel_info.vl_bpix)) / 8;
> > -	return *line_length * panel_info.vl_row + 0x20;
> > -}
> > -
> >   int am335xfb_init(struct am335x_lcdpanel *panel)
> >   {
> >   	u32 raster_ctrl = 0;
> >
> > base-commit: d3689267f92c5956e09cc7d1baa4700141662bff  
> I'll look into.
> But it's not easy as it looks like.
> 
> Just downloaded latest TRM, looks like documentation had become better.

Hi Hannes,

Have you had a chance to look further? Just for more explanation, my 
understanding is that lcd_get_size() should return the size of the pixel
array, starting at gd->fb_base. While gd->fb_base is allocated with an
extra 32 bytes at the start, it is updated in am335xfb_init:
	gd->fb_base += 0x20
to move past that. After that point, gd->fb_base has an effective size
of just the usual (bytes_per_pixel * pixels_per_line * num_lines). 

Specifically, my issue was with lcd_sync(), which does:
	flush_dcache_range((ulong)lcd_base,
		(ulong)(lcd_base + lcd_get_size(&line_length)));
and was flushing 32 bytes too much.

I do see that lcd_setmem() is also using lcd_get_size(), and if I'm 
reading correctly it is reserving memory for a buffer of (at least) 
that size. So perhaps in that place it would need the extra 32 bytes, 
whereas everything after am335xfb_init() is called would ignore the 
extra 32 bytes.

If that's the case, then perhaps we need two functions, one which 
returns the amount of memory that needs to be reserved (with the 
32bytes extra) and another which returns the size of the actual 
framebuffer? 

> 
> cheers,
> Hannes7
>