drivers/video/am335x-fb: Add support for 16bpp format

PATCH Superseded Martin Pietryka 3 participants 16 messages Apr 26, 2016 – Apr 28, 2016 Framebuffer layer

Share & tools

Martin Pietryka

author

2016-04-26 23:21 UTC #0 lore raw

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 <[email protected]>

---
 drivers/video/am335x-fb.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

drivers/video/am335x-fb.c +23 -6

index 0ec843f..f2b4c78 100644
@@ -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 */

Signature · 1 line

2.8.0

Martin Pietryka

author

2016-04-26 23:21 UTC #1 lore raw

PATCH 2/2 drivers/video/am335x-fb: Properly point framebuffer 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.

"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 <[email protected]>

---
 drivers/video/am335x-fb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/video/am335x-fb.c +4 -4

index f2b4c78..982db98 100644
@@ -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);

Signature · 1 line

2.8.0

Hannes Schmelzer

2016-04-27 06:43 UTC #2 lore raw

PATCH 2/2 drivers/video/am335x-fb: Properly point framebuffer behind palette

Quoted 7 lines · Martin Pietryka

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.

Quoted 7 lines

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

Hannes Schmelzer

2016-04-27 07:31 UTC #3 lore raw

PATCH 2/2 drivers/video/am335x-fb: Properly point framebuffer behind palette

Quoted 33 lines · Hannes Schmelzer

Quoted 8 lines · Martin Pietryka
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.
Quoted 8 lines
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;

Quoted 3 lines

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 ;-)

Quoted 10 lines

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

Martin Pietryka

author

2016-04-27 08:36 UTC #4 lore raw

PATCH 2/2 drivers/video/am335x-fb: Properly point framebuffer behind palette

In reply to Hannes Schmelzer

Hi Hannes,

Quoted 37 lines · Hannes Schmelzer

Quoted 8 lines · Martin Pietryka
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.
Quoted 8 lines
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)

Quoted 10 lines

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

Hannes Schmelzer

2016-04-27 09:02 UTC #5 lore raw

PATCH 2/2 drivers/video/am335x-fb: Properly point framebuffer behind palette

In reply to Martin Pietryka

Quoted 3 lines · Martin Pietryka

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.

Quoted 10 lines

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.

Quoted 10 lines

Signed-off-by Martin Pietryka <[email protected]>
---
  drivers/video/am335x-fb.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)
drivers/video/am335x-fb.c +0 -0
index f2b4c78..982db98 100644

drivers/video/am335x-fb.c int am335xfb_init(struct am335x_lcdpanel *panel) · line 132

Quoted 5 lines

drivers/video/am335x-fb.c +2 -0

@@ -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.

Quoted 15 lines

drivers/video/am335x-fb.c +2 -2

  	/* 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) |

drivers/video/am335x-fb.c int am335xfb_init(struct am335x_lcdpanel *panel) · line 187

Quoted 8 lines

drivers/video/am335x-fb.c +0 -2

@@ -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 <[email protected]>

Tested-by Hannes Schmelzer <[email protected]>

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

Martin Pietryka

author

2016-04-27 09:17 UTC #6 lore raw

PATCH 2/2 drivers/video/am335x-fb: Properly point framebuffer behind palette

Quoted 92 lines · Hannes Schmelzer

Quoted 3 lines · Martin Pietryka
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.
Quoted 10 lines
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.
Quoted 10 lines
Signed-off-by Martin Pietryka<[email protected]>
---
  drivers/video/am335x-fb.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)
drivers/video/am335x-fb.c +0 -0
index f2b4c78..982db98 100644
drivers/video/am335x-fb.c int am335xfb_init(struct am335x_lcdpanel *panel) · line 132
Quoted 5 lines
drivers/video/am335x-fb.c +2 -0
@@ -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.
Quoted 15 lines
drivers/video/am335x-fb.c +2 -2
  	/* 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) |
drivers/video/am335x-fb.c int am335xfb_init(struct am335x_lcdpanel *panel) · line 187
Quoted 8 lines
drivers/video/am335x-fb.c +0 -2
@@ -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 <[email protected]>
Tested-by Hannes Schmelzer <[email protected]>

Quoted 2 lines

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

Hannes Schmelzer

2016-04-27 09:16 UTC #7 lore raw

PATCH 1/2 drivers/video/am335x-fb: Add support for 16bpp format

In reply to Martin Pietryka

Quoted 52 lines · Martin Pietryka

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 <[email protected]>

---
  drivers/video/am335x-fb.c | 29 +++++++++++++++++++++++------
  1 file changed, 23 insertions(+), 6 deletions(-)

drivers/video/am335x-fb.c +10 -6

index 0ec843f..f2b4c78 100644
@@ -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:

drivers/video/am335x-fb.c int am335xfb_init(struct am335x_lcdpanel *panel) · line 178

Quoted 10 lines

drivers/video/am335x-fb.c +10 -0

+		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.

drivers/video/am335x-fb.c int am335xfb_init(struct am335x_lcdpanel *panel) · line 184

Quoted 6 lines

drivers/video/am335x-fb.c +3 -0

+	}
+
+	lcdhw->raster_ctrl = raster_ctrl;
 
  	gd->fb_base += 0x20;	/* point fb behind palette */

best regards,
Hannes

Martin Pietryka

author

2016-04-27 09:42 UTC #8 lore raw

PATCH v2 1/2 drivers/video/am335x-fb: Add support for 16bpp format

In reply to Martin Pietryka

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 <[email protected]>

---
 drivers/video/am335x-fb.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

drivers/video/am335x-fb.c +19 -3

index 0ec843f..569ecb6 100644
@@ -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;

Signature · 1 line

2.8.0

Hannes Schmelzer

2016-04-27 10:07 UTC #9 lore raw

PATCH v2 1/2 drivers/video/am335x-fb: Add support for 16bpp format

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

Quoted 68 lines · Martin Pietryka

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 <[email protected]>

---
  drivers/video/am335x-fb.c | 22 +++++++++++++++++++---
  1 file changed, 19 insertions(+), 3 deletions(-)

drivers/video/am335x-fb.c +19 -3

index 0ec843f..569ecb6 100644
@@ -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 Pietryka

author

2016-04-27 19:45 UTC #10 lore raw

PATCH v2 1/2 drivers/video/am335x-fb: Add support for 16bpp format

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

Quoted 88 lines · Hannes Schmelzer

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
Quoted 71 lines · Martin Pietryka
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 <[email protected]>
---
  drivers/video/am335x-fb.c | 22 +++++++++++++++++++---
  1 file changed, 19 insertions(+), 3 deletions(-)
drivers/video/am335x-fb.c +0 -0
index 0ec843f..569ecb6 100644
@@ -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
*
drivers/video/am335x-fb.c +2 -0
@@ -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 */
drivers/video/am335x-fb.c +14 -0
+    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);
drivers/video/am335x-fb.c +1 -2
@@ -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 Pietryka

author

2016-04-27 19:39 UTC #11 lore raw

PATCH v3 1/2 drivers/video/am335x-fb: Add support for 16bpp format

In reply to Martin Pietryka

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 <[email protected]>

---

Changes in v3:
- Moved switch statement before changing any registers

 drivers/video/am335x-fb.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

drivers/video/am335x-fb.c +19 -3

index 0ec843f..d984435 100644
@@ -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;

Signature · 1 line

2.8.0

Martin Pietryka

author

2016-04-27 19:39 UTC #12 lore raw

PATCH v3 2/2 drivers/video/am335x-fb: Properly point framebuffer behind palette

NOTE: According to the TRM, even in 16/24bit mode a palette is required in the first 32 bytes of the framebuffer.

Signed-off-by Martin Pietryka <[email protected]>

Reviewed-by Hannes Schmelzer <[email protected]>

Tested-by Hannes Schmelzer <[email protected]>

---

Changes in v3: None

 drivers/video/am335x-fb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/video/am335x-fb.c +4 -4

index d984435..bb5cc97 100644
@@ -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);

Signature · 1 line

2.8.0

Anatolij Gustschin

2016-04-28 14:57 UTC #13 lore raw

PATCH v3 2/2 drivers/video/am335x-fb: Properly point framebuffer behind palette

Quoted 18 lines · On Wed, 27 Apr 2016 21:39:16 +0200 Martin Pietryka martin.pietryka at…

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 <[email protected]>
Reviewed-by Hannes Schmelzer <[email protected]>
Tested-by Hannes Schmelzer <[email protected]>

Quoted 6 lines

---

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

Hannes Schmelzer

2016-04-28 04:59 UTC #14 lore raw

PATCH v3 1/2 drivers/video/am335x-fb: Add support for 16bpp format

In reply to Martin Pietryka

Quoted 63 lines · Martin Pietryka

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 <[email protected]>

---

Changes in v3:
- Moved switch statement before changing any registers

  drivers/video/am335x-fb.c | 22 +++++++++++++++++++---
  1 file changed, 19 insertions(+), 3 deletions(-)

drivers/video/am335x-fb.c +18 -1

index 0ec843f..d984435 100644
@@ -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);

drivers/video/am335x-fb.c int am335xfb_init(struct am335x_lcdpanel *panel) · line 180

Quoted 9 lines

drivers/video/am335x-fb.c +1 -2

@@ -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 <[email protected]>

Tested-by Hannes Schmelzer <[email protected]>

Anatolij Gustschin

2016-04-28 14:56 UTC #15 lore raw

PATCH v3 1/2 drivers/video/am335x-fb: Add support for 16bpp format

In reply to Martin Pietryka

Quoted 9 lines · On Wed, 27 Apr 2016 21:39:15 +0200 Martin Pietryka martin.pietryka at…

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)

Quoted 8 lines

Signed-off-by Martin Pietryka <[email protected]>
---
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