[U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format
To support 16bpp we just need to change the raster_ctrl register accordingly. Also 32bpp mode should work as well, but was not tested. According to the TRM the uppermost byte will be ignored when LCD_TFT_24BPP_UNPACK is set.
The switch logic is based on the Liunx kernel tilcdc driver: drivers/gpu/drm/tilcdc/tilcdc_crtc.c: lines 407 through 419 (kernel was checked out at commit: bcc981e9ed8)
Signed-off-by: Martin Pietryka martin.pietryka@chello.at --- drivers/video/am335x-fb.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c index 0ec843f..f2b4c78 100644 --- a/drivers/video/am335x-fb.c +++ b/drivers/video/am335x-fb.c @@ -5,7 +5,7 @@ * minimal framebuffer driver for TI's AM335x SoC to be compatible with * Wolfgang Denk's LCD-Framework (CONFIG_LCD, common/lcd.c) * - * - supporting only 24bit RGB/TFT raster Mode (not using palette) + * - supporting 16/24/32bit RGB/TFT raster Mode (not using palette) * - sets up LCD controller as in 'am335x_lcdpanel' struct given * - starts output DMA from gd->fb_base buffer * @@ -106,6 +106,8 @@ int lcd_get_size(int *line_length)
int am335xfb_init(struct am335x_lcdpanel *panel) { + u32 raster_ctrl = 0; + if (0 == gd->fb_base) { printf("ERROR: no valid fb_base stored in GLOBAL_DATA_PTR!\n"); return -1; @@ -157,11 +159,26 @@ int am335xfb_init(struct am335x_lcdpanel *panel) LCD_HBPMSB(panel->hbp) | LCD_HFPMSB(panel->hfp) | 0x0000FF00; /* clk cycles for ac-bias */ - lcdhw->raster_ctrl = LCD_TFT_24BPP_MODE | - LCD_TFT_24BPP_UNPACK | - LCD_PALMODE_RAWDATA | - LCD_TFT_MODE | - LCD_RASTER_ENABLE; + + raster_ctrl = LCD_PALMODE_RAWDATA | + LCD_TFT_MODE | + LCD_RASTER_ENABLE; + + switch (panel->bpp) { + case 16: + break; + case 32: + raster_ctrl |= LCD_TFT_24BPP_UNPACK; + /* fallthrough */ + case 24: + raster_ctrl |= LCD_TFT_24BPP_MODE; + break; + default: + error("am335x-fb: invalid bpp value: %d\n", panel->bpp); + return 1; + } + + lcdhw->raster_ctrl = raster_ctrl;
gd->fb_base += 0x20; /* point fb behind palette */
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.
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."
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;
/* 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);
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.
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."
Hi Martin, many thanks for working on this.
I'm just reviewing the stuff, you're right it shouldn't work right now. But it does ?! Perhaps it does because there is another issue.
DMA today fetches palette also, but the Bit 21..20 in RASTER_CTRL aren't set correctly.
We do: lcdhw->raster_ctrl = LCD_TFT_24BPP_MODE | LCD_TFT_24BPP_UNPACK | LCD_PALMODE_RAWDATA | LCD_TFT_MODE | LCD_RASTER_ENABLE;
with
#define LCD_PALMODE_RAWDATA (0x10 << 20)
this doesn't set palmode as excepted to raw data, instead this sets 'stn565, bit24'.
I will investigate a bit on this now, testing your patches and fixup my mistakes.
best regards, Hannes
On 04/27/2016 08:43 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.
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."
Hi Martin, many thanks for working on this.
I'm just reviewing the stuff, you're right it shouldn't work right now. But it does ?! Perhaps it does because there is another issue.
DMA today fetches palette also, but the Bit 21..20 in RASTER_CTRL aren't set correctly.
We do: lcdhw->raster_ctrl = LCD_TFT_24BPP_MODE | LCD_TFT_24BPP_UNPACK | LCD_PALMODE_RAWDATA | LCD_TFT_MODE | LCD_RASTER_ENABLE;
with
#define LCD_PALMODE_RAWDATA (0x10 << 20)
sorry, i've missed recent changes applied to master, there this was already fixed. I also reviewed this ;-)
this doesn't set palmode as excepted to raw data, instead this sets 'stn565, bit24'.
I will investigate a bit on this now, testing your patches and fixup my mistakes.
best regards, Hannes
Hi Hannes,
On 04/27/2016 08:43 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.
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."
Hi Martin, many thanks for working on this.
I'm just reviewing the stuff, you're right it shouldn't work right now. But it does ?! Perhaps it does because there is another issue.
DMA today fetches palette also, but the Bit 21..20 in RASTER_CTRL aren't set correctly.
We do: lcdhw->raster_ctrl = LCD_TFT_24BPP_MODE | LCD_TFT_24BPP_UNPACK | LCD_PALMODE_RAWDATA | LCD_TFT_MODE | LCD_RASTER_ENABLE;
with
#define LCD_PALMODE_RAWDATA (0x10 << 20)
this doesn't set palmode as excepted to raw data, instead this sets 'stn565, bit24'.
I will investigate a bit on this now, testing your patches and fixup my mistakes.
best regards, Hannes
regarding the bits in the palmode in RASTER_CTRL, I already made a patch yesterday which fixes this:
ac5c61b drivers/video/am335x-fb: Fix bits for LCD_PALMODE_RAWDATA definition
That might explain why no one saw DMA issues before because setting it to stn565 mode might have not show any issues with the DMA (but that's just a guess).
But I'm sure it was the wrong setting, I also crosschecked the RASTER_CTRL register value with a running kernel (4.4) and the stn565 bit is also not set for 16bpp mode.
I tested it only on a 16bpp screen, it would be nice if someone could confirm that it works on a 24/32bpp screen.
Martin
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
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 Pietrykamartin.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
On 04/27/2016 01:21 AM, Martin Pietryka wrote:
To support 16bpp we just need to change the raster_ctrl register accordingly. Also 32bpp mode should work as well, but was not tested. According to the TRM the uppermost byte will be ignored when LCD_TFT_24BPP_UNPACK is set.
The switch logic is based on the Liunx kernel tilcdc driver: drivers/gpu/drm/tilcdc/tilcdc_crtc.c: lines 407 through 419 (kernel was checked out at commit: bcc981e9ed8)
Signed-off-by: Martin Pietryka martin.pietryka@chello.at
drivers/video/am335x-fb.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c index 0ec843f..f2b4c78 100644 --- a/drivers/video/am335x-fb.c +++ b/drivers/video/am335x-fb.c @@ -5,7 +5,7 @@
- minimal framebuffer driver for TI's AM335x SoC to be compatible with
- Wolfgang Denk's LCD-Framework (CONFIG_LCD, common/lcd.c)
- supporting only 24bit RGB/TFT raster Mode (not using palette)
- supporting 16/24/32bit RGB/TFT raster Mode (not using palette)
- sets up LCD controller as in 'am335x_lcdpanel' struct given
- starts output DMA from gd->fb_base buffer
@@ -106,6 +106,8 @@ int lcd_get_size(int *line_length)
int am335xfb_init(struct am335x_lcdpanel *panel) {
- u32 raster_ctrl = 0;
- if (0 == gd->fb_base) { printf("ERROR: no valid fb_base stored in GLOBAL_DATA_PTR!\n"); return -1;
@@ -157,11 +159,26 @@ int am335xfb_init(struct am335x_lcdpanel *panel) LCD_HBPMSB(panel->hbp) | LCD_HFPMSB(panel->hfp) | 0x0000FF00; /* clk cycles for ac-bias */
- lcdhw->raster_ctrl = LCD_TFT_24BPP_MODE |
LCD_TFT_24BPP_UNPACK |LCD_PALMODE_RAWDATA |LCD_TFT_MODE |LCD_RASTER_ENABLE;
- raster_ctrl = LCD_PALMODE_RAWDATA |
LCD_TFT_MODE |LCD_RASTER_ENABLE;- switch (panel->bpp) {
- case 16:
break;- case 32:
raster_ctrl |= LCD_TFT_24BPP_UNPACK;/* fallthrough */- case 24:
raster_ctrl |= LCD_TFT_24BPP_MODE;break;- default:
error("am335x-fb: invalid bpp value: %d\n", panel->bpp);return 1;
please return -1 on error; I would also suggest to put this switch case above the first hw-access or doing anything on lcd, so the controller and display leaves untouched on wrong parameters.
}
lcdhw->raster_ctrl = raster_ctrl;
gd->fb_base += 0x20; /* point fb behind palette */
best regards, Hannes
To support 16bpp we just need to change the raster_ctrl register accordingly. Also 32bpp mode should work as well, but was not tested. According to the TRM the uppermost byte will be ignored when LCD_TFT_24BPP_UNPACK is set.
The switch logic is based on the Liunx kernel tilcdc driver: drivers/gpu/drm/tilcdc/tilcdc_crtc.c: lines 407 through 419 (kernel was checked out at commit: bcc981e9ed8)
Signed-off-by: Martin Pietryka martin.pietryka@chello.at --- drivers/video/am335x-fb.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c index 0ec843f..569ecb6 100644 --- a/drivers/video/am335x-fb.c +++ b/drivers/video/am335x-fb.c @@ -5,7 +5,7 @@ * minimal framebuffer driver for TI's AM335x SoC to be compatible with * Wolfgang Denk's LCD-Framework (CONFIG_LCD, common/lcd.c) * - * - supporting only 24bit RGB/TFT raster Mode (not using palette) + * - supporting 16/24/32bit RGB/TFT raster Mode (not using palette) * - sets up LCD controller as in 'am335x_lcdpanel' struct given * - starts output DMA from gd->fb_base buffer * @@ -106,6 +106,8 @@ int lcd_get_size(int *line_length)
int am335xfb_init(struct am335x_lcdpanel *panel) { + u32 raster_ctrl = 0; + if (0 == gd->fb_base) { printf("ERROR: no valid fb_base stored in GLOBAL_DATA_PTR!\n"); return -1; @@ -115,6 +117,21 @@ int am335xfb_init(struct am335x_lcdpanel *panel) return -1; }
+ /* We can already set the bits for the raster_ctrl in this check */ + switch (panel->bpp) { + case 16: + break; + case 32: + raster_ctrl |= LCD_TFT_24BPP_UNPACK; + /* fallthrough */ + case 24: + raster_ctrl |= LCD_TFT_24BPP_MODE; + break; + default: + error("am335x-fb: invalid bpp value: %d\n", panel->bpp); + return -1; + } + debug("setting up LCD-Controller for %dx%dx%d (hfp=%d,hbp=%d,hsw=%d / ", panel->hactive, panel->vactive, panel->bpp, panel->hfp, panel->hbp, panel->hsw); @@ -157,8 +174,7 @@ int am335xfb_init(struct am335x_lcdpanel *panel) LCD_HBPMSB(panel->hbp) | LCD_HFPMSB(panel->hfp) | 0x0000FF00; /* clk cycles for ac-bias */ - lcdhw->raster_ctrl = LCD_TFT_24BPP_MODE | - LCD_TFT_24BPP_UNPACK | + lcdhw->raster_ctrl = raster_ctrl | /* contains the bits for the pixel format */ LCD_PALMODE_RAWDATA | LCD_TFT_MODE | LCD_RASTER_ENABLE;
Martin,
you have to send also a v2 from "2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette". v1 doesn't apply after your v2 patch below has been applied.
how do you manage your patch-generation and emailing?
have a look to: tools/patman/README, a really great tool for managing patch-series.
best regards, Hannes
On 04/27/2016 11:42 AM, Martin Pietryka wrote:
To support 16bpp we just need to change the raster_ctrl register accordingly. Also 32bpp mode should work as well, but was not tested. According to the TRM the uppermost byte will be ignored when LCD_TFT_24BPP_UNPACK is set.
The switch logic is based on the Liunx kernel tilcdc driver: drivers/gpu/drm/tilcdc/tilcdc_crtc.c: lines 407 through 419 (kernel was checked out at commit: bcc981e9ed8)
Signed-off-by: Martin Pietryka martin.pietryka@chello.at
drivers/video/am335x-fb.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c index 0ec843f..569ecb6 100644 --- a/drivers/video/am335x-fb.c +++ b/drivers/video/am335x-fb.c @@ -5,7 +5,7 @@
- minimal framebuffer driver for TI's AM335x SoC to be compatible with
- Wolfgang Denk's LCD-Framework (CONFIG_LCD, common/lcd.c)
- supporting only 24bit RGB/TFT raster Mode (not using palette)
- supporting 16/24/32bit RGB/TFT raster Mode (not using palette)
- sets up LCD controller as in 'am335x_lcdpanel' struct given
- starts output DMA from gd->fb_base buffer
@@ -106,6 +106,8 @@ int lcd_get_size(int *line_length)
int am335xfb_init(struct am335x_lcdpanel *panel) {
- u32 raster_ctrl = 0;
- if (0 == gd->fb_base) { printf("ERROR: no valid fb_base stored in GLOBAL_DATA_PTR!\n"); return -1;
@@ -115,6 +117,21 @@ int am335xfb_init(struct am335x_lcdpanel *panel) return -1; }
- /* We can already set the bits for the raster_ctrl in this check */
- switch (panel->bpp) {
- case 16:
break;- case 32:
raster_ctrl |= LCD_TFT_24BPP_UNPACK;/* fallthrough */- case 24:
raster_ctrl |= LCD_TFT_24BPP_MODE;break;- default:
error("am335x-fb: invalid bpp value: %d\n", panel->bpp);return -1;- }
- debug("setting up LCD-Controller for %dx%dx%d (hfp=%d,hbp=%d,hsw=%d / ", panel->hactive, panel->vactive, panel->bpp, panel->hfp, panel->hbp, panel->hsw);
@@ -157,8 +174,7 @@ int am335xfb_init(struct am335x_lcdpanel *panel) LCD_HBPMSB(panel->hbp) | LCD_HFPMSB(panel->hfp) | 0x0000FF00; /* clk cycles for ac-bias */
- lcdhw->raster_ctrl = LCD_TFT_24BPP_MODE |
LCD_TFT_24BPP_UNPACK |
- lcdhw->raster_ctrl = raster_ctrl | /* contains the bits for the pixel format */ LCD_PALMODE_RAWDATA | LCD_TFT_MODE | LCD_RASTER_ENABLE;
Hi Hannes,
I used git send-email and git format-patch manually, which was a bit of a hassle.
Now I used patman which seems to be really handy, thanks for the hint.
Martin
On 04/27/2016 12:07 PM, Hannes Schmelzer wrote:
Martin,
you have to send also a v2 from "2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette". v1 doesn't apply after your v2 patch below has been applied.
how do you manage your patch-generation and emailing?
have a look to: tools/patman/README, a really great tool for managing patch-series.
best regards, Hannes
On 04/27/2016 11:42 AM, Martin Pietryka wrote:
To support 16bpp we just need to change the raster_ctrl register accordingly. Also 32bpp mode should work as well, but was not tested. According to the TRM the uppermost byte will be ignored when LCD_TFT_24BPP_UNPACK is set.
The switch logic is based on the Liunx kernel tilcdc driver: drivers/gpu/drm/tilcdc/tilcdc_crtc.c: lines 407 through 419 (kernel was checked out at commit: bcc981e9ed8)
Signed-off-by: Martin Pietryka martin.pietryka@chello.at
drivers/video/am335x-fb.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c index 0ec843f..569ecb6 100644 --- a/drivers/video/am335x-fb.c +++ b/drivers/video/am335x-fb.c @@ -5,7 +5,7 @@
- minimal framebuffer driver for TI's AM335x SoC to be compatible
with
- Wolfgang Denk's LCD-Framework (CONFIG_LCD, common/lcd.c)
- supporting only 24bit RGB/TFT raster Mode (not using palette)
- supporting 16/24/32bit RGB/TFT raster Mode (not using palette)
- sets up LCD controller as in 'am335x_lcdpanel' struct given
- starts output DMA from gd->fb_base buffer
@@ -106,6 +106,8 @@ int lcd_get_size(int *line_length) int am335xfb_init(struct am335x_lcdpanel *panel) {
- u32 raster_ctrl = 0;
if (0 == gd->fb_base) { printf("ERROR: no valid fb_base stored inGLOBAL_DATA_PTR!\n"); return -1; @@ -115,6 +117,21 @@ int am335xfb_init(struct am335x_lcdpanel *panel) return -1; }
- /* We can already set the bits for the raster_ctrl in this
check */
- switch (panel->bpp) {
- case 16:
break;- case 32:
raster_ctrl |= LCD_TFT_24BPP_UNPACK;/* fallthrough */- case 24:
raster_ctrl |= LCD_TFT_24BPP_MODE;break;- default:
error("am335x-fb: invalid bpp value: %d\n", panel->bpp);return -1;- }
debug("setting up LCD-Controller for %dx%dx%d(hfp=%d,hbp=%d,hsw=%d / ", panel->hactive, panel->vactive, panel->bpp, panel->hfp, panel->hbp, panel->hsw); @@ -157,8 +174,7 @@ int am335xfb_init(struct am335x_lcdpanel *panel) LCD_HBPMSB(panel->hbp) | LCD_HFPMSB(panel->hfp) | 0x0000FF00; /* clk cycles for ac-bias */
- lcdhw->raster_ctrl = LCD_TFT_24BPP_MODE |
LCD_TFT_24BPP_UNPACK |
- lcdhw->raster_ctrl = raster_ctrl | /* contains the bits
for the pixel format */ LCD_PALMODE_RAWDATA | LCD_TFT_MODE | LCD_RASTER_ENABLE;
To support 16bpp we just need to change the raster_ctrl register accordingly. Also 32bpp mode should work as well, but was not tested. According to the TRM the uppermost byte will be ignored when LCD_TFT_24BPP_UNPACK is set.
The switch logic is based on the Liunx kernel tilcdc driver: drivers/gpu/drm/tilcdc/tilcdc_crtc.c: lines 407 through 419 (kernel was checked out at commit: bcc981e9ed8)
Signed-off-by: Martin Pietryka martin.pietryka@chello.at ---
Changes in v3: - Moved switch statement before changing any registers
drivers/video/am335x-fb.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c index 0ec843f..d984435 100644 --- a/drivers/video/am335x-fb.c +++ b/drivers/video/am335x-fb.c @@ -5,7 +5,7 @@ * minimal framebuffer driver for TI's AM335x SoC to be compatible with * Wolfgang Denk's LCD-Framework (CONFIG_LCD, common/lcd.c) * - * - supporting only 24bit RGB/TFT raster Mode (not using palette) + * - supporting 16/24/32bit RGB/TFT raster Mode (not using palette) * - sets up LCD controller as in 'am335x_lcdpanel' struct given * - starts output DMA from gd->fb_base buffer * @@ -106,6 +106,8 @@ int lcd_get_size(int *line_length)
int am335xfb_init(struct am335x_lcdpanel *panel) { + u32 raster_ctrl = 0; + if (0 == gd->fb_base) { printf("ERROR: no valid fb_base stored in GLOBAL_DATA_PTR!\n"); return -1; @@ -115,6 +117,21 @@ int am335xfb_init(struct am335x_lcdpanel *panel) return -1; }
+ /* We can already set the bits for the raster_ctrl in this check */ + switch (panel->bpp) { + case 16: + break; + case 32: + raster_ctrl |= LCD_TFT_24BPP_UNPACK; + /* fallthrough */ + case 24: + raster_ctrl |= LCD_TFT_24BPP_MODE; + break; + default: + error("am335x-fb: invalid bpp value: %d\n", panel->bpp); + return -1; + } + debug("setting up LCD-Controller for %dx%dx%d (hfp=%d,hbp=%d,hsw=%d / ", panel->hactive, panel->vactive, panel->bpp, panel->hfp, panel->hbp, panel->hsw); @@ -157,8 +174,7 @@ int am335xfb_init(struct am335x_lcdpanel *panel) LCD_HBPMSB(panel->hbp) | LCD_HFPMSB(panel->hfp) | 0x0000FF00; /* clk cycles for ac-bias */ - lcdhw->raster_ctrl = LCD_TFT_24BPP_MODE | - LCD_TFT_24BPP_UNPACK | + lcdhw->raster_ctrl = raster_ctrl | LCD_PALMODE_RAWDATA | LCD_TFT_MODE | LCD_RASTER_ENABLE;
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.
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."
Signed-off-by: Martin Pietryka martin.pietryka@chello.at Reviewed-by: Hannes Schmelzer oe5hpm@oevsv.at Tested-by: Hannes Schmelzer oe5hpm@oevsv.at ---
Changes in v3: None
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 d984435..bb5cc97 100644 --- a/drivers/video/am335x-fb.c +++ b/drivers/video/am335x-fb.c @@ -143,6 +143,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;
/* turn ON display through powercontrol function if accessible */ if (0 != panel->panel_power_ctrl) @@ -154,9 +156,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) | @@ -179,8 +181,6 @@ int am335xfb_init(struct am335x_lcdpanel *panel) LCD_TFT_MODE | LCD_RASTER_ENABLE;
- gd->fb_base += 0x20; /* point fb behind palette */ - debug("am335x-fb: waiting picture to be stable.\n."); mdelay(panel->pon_delay);
On Wed, 27 Apr 2016 21:39:16 +0200 Martin Pietryka martin.pietryka@chello.at 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.
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."
Signed-off-by: Martin Pietryka martin.pietryka@chello.at Reviewed-by: Hannes Schmelzer oe5hpm@oevsv.at Tested-by: Hannes Schmelzer oe5hpm@oevsv.at
Changes in v3: None
drivers/video/am335x-fb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
appied to u-boot-video/master, thanks!
-- Anatolij
On 27.04.2016 21:39, Martin Pietryka wrote:
To support 16bpp we just need to change the raster_ctrl register accordingly. Also 32bpp mode should work as well, but was not tested. According to the TRM the uppermost byte will be ignored when LCD_TFT_24BPP_UNPACK is set.
The switch logic is based on the Liunx kernel tilcdc driver: drivers/gpu/drm/tilcdc/tilcdc_crtc.c: lines 407 through 419 (kernel was checked out at commit: bcc981e9ed8)
Signed-off-by: Martin Pietryka martin.pietryka@chello.at
Changes in v3:
Moved switch statement before changing any registers
drivers/video/am335x-fb.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c index 0ec843f..d984435 100644 --- a/drivers/video/am335x-fb.c +++ b/drivers/video/am335x-fb.c @@ -5,7 +5,7 @@
- minimal framebuffer driver for TI's AM335x SoC to be compatible with
- Wolfgang Denk's LCD-Framework (CONFIG_LCD, common/lcd.c)
- supporting only 24bit RGB/TFT raster Mode (not using palette)
- supporting 16/24/32bit RGB/TFT raster Mode (not using palette)
- sets up LCD controller as in 'am335x_lcdpanel' struct given
- starts output DMA from gd->fb_base buffer
@@ -106,6 +106,8 @@ int lcd_get_size(int *line_length)
int am335xfb_init(struct am335x_lcdpanel *panel) {
- u32 raster_ctrl = 0;
- if (0 == gd->fb_base) { printf("ERROR: no valid fb_base stored in GLOBAL_DATA_PTR!\n"); return -1;
@@ -115,6 +117,21 @@ int am335xfb_init(struct am335x_lcdpanel *panel) return -1; }
- /* We can already set the bits for the raster_ctrl in this check */
- switch (panel->bpp) {
- case 16:
break;- case 32:
raster_ctrl |= LCD_TFT_24BPP_UNPACK;/* fallthrough */- case 24:
raster_ctrl |= LCD_TFT_24BPP_MODE;break;- default:
error("am335x-fb: invalid bpp value: %d\n", panel->bpp);return -1;- }
- debug("setting up LCD-Controller for %dx%dx%d (hfp=%d,hbp=%d,hsw=%d / ", panel->hactive, panel->vactive, panel->bpp, panel->hfp, panel->hbp, panel->hsw);
@@ -157,8 +174,7 @@ int am335xfb_init(struct am335x_lcdpanel *panel) LCD_HBPMSB(panel->hbp) | LCD_HFPMSB(panel->hfp) | 0x0000FF00; /* clk cycles for ac-bias */
- lcdhw->raster_ctrl = LCD_TFT_24BPP_MODE |
LCD_TFT_24BPP_UNPACK |
- lcdhw->raster_ctrl = raster_ctrl | LCD_PALMODE_RAWDATA | LCD_TFT_MODE | LCD_RASTER_ENABLE;
Reviewed-by: Hannes Schmelzer oe5hpm@oevsv.at Tested-by: Hannes Schmelzer oe5hpm@oevsv.at
On Wed, 27 Apr 2016 21:39:15 +0200 Martin Pietryka martin.pietryka@chello.at wrote:
To support 16bpp we just need to change the raster_ctrl register accordingly. Also 32bpp mode should work as well, but was not tested. According to the TRM the uppermost byte will be ignored when LCD_TFT_24BPP_UNPACK is set.
The switch logic is based on the Liunx kernel tilcdc driver: drivers/gpu/drm/tilcdc/tilcdc_crtc.c: lines 407 through 419 (kernel was checked out at commit: bcc981e9ed8)
Signed-off-by: Martin Pietryka martin.pietryka@chello.at
Changes in v3:
- Moved switch statement before changing any registers
drivers/video/am335x-fb.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
appied to u-boot-video/master, thanks!
-- Anatolij
participants (3)
-
Anatolij Gustschin -
Hannes Schmelzer -
Martin Pietryka