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

Martin Pietryka
2016-04-27T09:17:07Z

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@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?
Yes, I've done the testing and development with a 16bit
display/framebuffer and it works as expected for me.

Martin