[U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette

[Hannes Schmelzer <hannes@schmelzer.or.at>]

On 04/27/2016 01:21 AM, Martin Pietryka wrote:
> The DMA was outputting the palette on the screen because the base
> for the DMA was not after the palette. In addition to that, the ceiling was
> also too high, this led that the output on the screen was shifted.
This "fault" was masked in the past due to wrong handling of 
LCD_PALMODE_RAWDATA bit,
which was fixed with commit ac5c61bfa6ad24a5384b9b89902e024a994f715f 
(drivers/video/am335x-fb: Fix bits for LCD_PALMODE_RAWDATA definition).

So now the things are coming out and screen output is shifted.
> NOTE: According to the TRM, even in 16/24bit mode a palette is required
> in the first 32 bytes of the framebuffer.
>
> See also:
> https://e2e.ti.com/support/arm/sitara_arm/f/791/p/234967/834483#834483
>
> "In this mode, the LCDC will assume all information is data and thus you
> need to ensure that the DMA points to the first pixel of data and not the
> first entry in the frame buffer which is the beginning of the 512 byte
> palette."
Correct.
> Signed-off-by: Martin Pietryka <martin.pietryka@chello.at>
> ---
>   drivers/video/am335x-fb.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c
> index f2b4c78..982db98 100644
> --- a/drivers/video/am335x-fb.c
> +++ b/drivers/video/am335x-fb.c
> @@ -128,6 +128,8 @@ int am335xfb_init(struct am335x_lcdpanel *panel)
>   	/* palette default entry */
>   	memset((void *)gd->fb_base, 0, 0x20);
>   	*(unsigned int *)gd->fb_base = 0x4000;
> +	/* point fb behind palette */
> +	gd->fb_base += 0x20;
In theory we don't need anymore this palette offset of 0x20 byte, 
because DMA is fetching startign behind this offset.

But in praxis we have to use the offset also in future for compatibility 
reasons, because there may some OS which is writing to our 
bare-framebuffer starting at 32byte offset.
For example, my vxWorks is doing so ;-(

Also the wording within the TRM (spruh73l.pdf) about this topic isn't 
fully clear to me, on page 1842 they say:
"
12-, 16-, and 24-BPP modes do not need a palette; i.e., the pixel data 
is the desired RGB value.
However, the first 32 bytes are still considered a palette. The first 
entry should be 4000h (bit 14 is 1) while the remaining entries must be 
filled with 0.
"

Exactly as we do today.

Few pages later, 1874, description from the RASTER_CTRL register they 
write for our case:
"Data loading only For Raw Data (12/16/24 bpp) framebuffers, no palette 
lookup is employed."

I've tested removing this offset and it works as expected:
- fine display in u-boot
- garbage within my vxWorks :-)

Finally i think we have to leave it as it is, 0x20 bytes offset.
>   	/* turn ON display through powercontrol function if accessible */
>   	if (0 != panel->panel_power_ctrl)
> @@ -139,9 +141,9 @@ int am335xfb_init(struct am335x_lcdpanel *panel)
>   	lcdhw->raster_ctrl = 0;
>   	lcdhw->ctrl = LCD_CLK_DIVISOR(panel->pxl_clk_div) | LCD_RASTER_MODE;
>   	lcdhw->lcddma_fb0_base = gd->fb_base;
> -	lcdhw->lcddma_fb0_ceiling = gd->fb_base + FBSIZE(panel) + 0x20;
> +	lcdhw->lcddma_fb0_ceiling = gd->fb_base + FBSIZE(panel);
>   	lcdhw->lcddma_fb1_base = gd->fb_base;
> -	lcdhw->lcddma_fb1_ceiling = gd->fb_base + FBSIZE(panel) + 0x20;
> +	lcdhw->lcddma_fb1_ceiling = gd->fb_base + FBSIZE(panel);
>   	lcdhw->lcddma_ctrl = LCD_DMA_BURST_SIZE(LCD_DMA_BURST_16);
>   
>   	lcdhw->raster_timing0 = LCD_HORLSB(panel->hactive) |
> @@ -180,8 +182,6 @@ int am335xfb_init(struct am335x_lcdpanel *panel)
>   
>   	lcdhw->raster_ctrl = raster_ctrl;
>   
> -	gd->fb_base += 0x20;	/* point fb behind palette */
> -
>   	debug("am335x-fb: waiting picture to be stable.\n.");
>   	mdelay(panel->pon_delay);
>   
Reviewd-by: Hannes Schmelzer <oe5hpm@oevsv.at>
Tested-by: Hannes Schmelzer <oe5hpm@oevsv.at>

I've done my test with 32bit framebuffer.
Martin, i think your'e testing with 16bit - right?

many thanks for picking up this and
best regards,
Hannes - OE5HPM