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

[Martin Pietryka]

On 04/27/2016 11:02 AM, Hannes Schmelzer wrote:
> 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 at 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 at oevsv.at>
> Tested-by: Hannes Schmelzer <oe5hpm at oevsv.at>
>
> I've done my test with 32bit framebuffer.
> Martin, i think your'e testing with 16bit - right?
Yes, I've done the testing and development with a 16bit 
display/framebuffer and it works as expected for me.

Martin