All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format
@ 2016-04-26 23:21 Martin Pietryka
  2016-04-26 23:21 ` [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Martin Pietryka @ 2016-04-26 23:21 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette
  2016-04-26 23:21 [U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format Martin Pietryka
@ 2016-04-26 23:21 ` Martin Pietryka
  2016-04-27  6:43   ` Hannes Schmelzer
  2016-04-27  9:02   ` Hannes Schmelzer
  2016-04-27  9:16 ` [U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format Hannes Schmelzer
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Martin Pietryka @ 2016-04-26 23:21 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette
  2016-04-26 23:21 ` [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
@ 2016-04-27  6:43   ` Hannes Schmelzer
  2016-04-27  7:31     ` Hannes Schmelzer
  2016-04-27  8:36     ` Martin Pietryka
  2016-04-27  9:02   ` Hannes Schmelzer
  1 sibling, 2 replies; 16+ messages in thread
From: Hannes Schmelzer @ 2016-04-27  6:43 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette
  2016-04-27  6:43   ` Hannes Schmelzer
@ 2016-04-27  7:31     ` Hannes Schmelzer
  2016-04-27  8:36     ` Martin Pietryka
  1 sibling, 0 replies; 16+ messages in thread
From: Hannes Schmelzer @ 2016-04-27  7:31 UTC (permalink / raw)
  To: u-boot

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
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette
  2016-04-27  6:43   ` Hannes Schmelzer
  2016-04-27  7:31     ` Hannes Schmelzer
@ 2016-04-27  8:36     ` Martin Pietryka
  1 sibling, 0 replies; 16+ messages in thread
From: Martin Pietryka @ 2016-04-27  8:36 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette
  2016-04-26 23:21 ` [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
  2016-04-27  6:43   ` Hannes Schmelzer
@ 2016-04-27  9:02   ` Hannes Schmelzer
  2016-04-27  9:17     ` Martin Pietryka
  1 sibling, 1 reply; 16+ messages in thread
From: Hannes Schmelzer @ 2016-04-27  9:02 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format
  2016-04-26 23:21 [U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format Martin Pietryka
  2016-04-26 23:21 ` [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
@ 2016-04-27  9:16 ` Hannes Schmelzer
  2016-04-27  9:42 ` [U-Boot] [PATCH v2 " Martin Pietryka
  2016-04-27 19:39 ` [U-Boot] [PATCH v3 " Martin Pietryka
  3 siblings, 0 replies; 16+ messages in thread
From: Hannes Schmelzer @ 2016-04-27  9:16 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette
  2016-04-27  9:02   ` Hannes Schmelzer
@ 2016-04-27  9:17     ` Martin Pietryka
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Pietryka @ 2016-04-27  9:17 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2 1/2] drivers/video/am335x-fb: Add support for 16bpp format
  2016-04-26 23:21 [U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format Martin Pietryka
  2016-04-26 23:21 ` [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
  2016-04-27  9:16 ` [U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format Hannes Schmelzer
@ 2016-04-27  9:42 ` Martin Pietryka
  2016-04-27 10:07   ` Hannes Schmelzer
  2016-04-27 19:39 ` [U-Boot] [PATCH v3 " Martin Pietryka
  3 siblings, 1 reply; 16+ messages in thread
From: Martin Pietryka @ 2016-04-27  9:42 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2 1/2] drivers/video/am335x-fb: Add support for 16bpp format
  2016-04-27  9:42 ` [U-Boot] [PATCH v2 " Martin Pietryka
@ 2016-04-27 10:07   ` Hannes Schmelzer
  2016-04-27 19:45     ` Martin Pietryka
  0 siblings, 1 reply; 16+ messages in thread
From: Hannes Schmelzer @ 2016-04-27 10:07 UTC (permalink / raw)
  To: u-boot

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;

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3 1/2] drivers/video/am335x-fb: Add support for 16bpp format
  2016-04-26 23:21 [U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format Martin Pietryka
                   ` (2 preceding siblings ...)
  2016-04-27  9:42 ` [U-Boot] [PATCH v2 " Martin Pietryka
@ 2016-04-27 19:39 ` Martin Pietryka
  2016-04-27 19:39   ` [U-Boot] [PATCH v3 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
                     ` (2 more replies)
  3 siblings, 3 replies; 16+ messages in thread
From: Martin Pietryka @ 2016-04-27 19:39 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette
  2016-04-27 19:39 ` [U-Boot] [PATCH v3 " Martin Pietryka
@ 2016-04-27 19:39   ` Martin Pietryka
  2016-04-28 14:57     ` Anatolij Gustschin
  2016-04-28  4:59   ` [U-Boot] [PATCH v3 1/2] drivers/video/am335x-fb: Add support for 16bpp format Hannes Schmelzer
  2016-04-28 14:56   ` Anatolij Gustschin
  2 siblings, 1 reply; 16+ messages in thread
From: Martin Pietryka @ 2016-04-27 19:39 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2 1/2] drivers/video/am335x-fb: Add support for 16bpp format
  2016-04-27 10:07   ` Hannes Schmelzer
@ 2016-04-27 19:45     ` Martin Pietryka
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Pietryka @ 2016-04-27 19:45 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3 1/2] drivers/video/am335x-fb: Add support for 16bpp format
  2016-04-27 19:39 ` [U-Boot] [PATCH v3 " Martin Pietryka
  2016-04-27 19:39   ` [U-Boot] [PATCH v3 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
@ 2016-04-28  4:59   ` Hannes Schmelzer
  2016-04-28 14:56   ` Anatolij Gustschin
  2 siblings, 0 replies; 16+ messages in thread
From: Hannes Schmelzer @ 2016-04-28  4:59 UTC (permalink / raw)
  To: u-boot



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>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3 1/2] drivers/video/am335x-fb: Add support for 16bpp format
  2016-04-27 19:39 ` [U-Boot] [PATCH v3 " Martin Pietryka
  2016-04-27 19:39   ` [U-Boot] [PATCH v3 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
  2016-04-28  4:59   ` [U-Boot] [PATCH v3 1/2] drivers/video/am335x-fb: Add support for 16bpp format Hannes Schmelzer
@ 2016-04-28 14:56   ` Anatolij Gustschin
  2 siblings, 0 replies; 16+ messages in thread
From: Anatolij Gustschin @ 2016-04-28 14:56 UTC (permalink / raw)
  To: u-boot

On Wed, 27 Apr 2016 21:39:15 +0200
Martin Pietryka martin.pietryka at 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette
  2016-04-27 19:39   ` [U-Boot] [PATCH v3 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
@ 2016-04-28 14:57     ` Anatolij Gustschin
  0 siblings, 0 replies; 16+ messages in thread
From: Anatolij Gustschin @ 2016-04-28 14:57 UTC (permalink / raw)
  To: u-boot

On Wed, 27 Apr 2016 21:39:16 +0200
Martin Pietryka martin.pietryka at 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-04-28 14:57 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-26 23:21 [U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format Martin Pietryka
2016-04-26 23:21 ` [U-Boot] [PATCH 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
2016-04-27  6:43   ` Hannes Schmelzer
2016-04-27  7:31     ` Hannes Schmelzer
2016-04-27  8:36     ` Martin Pietryka
2016-04-27  9:02   ` Hannes Schmelzer
2016-04-27  9:17     ` Martin Pietryka
2016-04-27  9:16 ` [U-Boot] [PATCH 1/2] drivers/video/am335x-fb: Add support for 16bpp format Hannes Schmelzer
2016-04-27  9:42 ` [U-Boot] [PATCH v2 " Martin Pietryka
2016-04-27 10:07   ` Hannes Schmelzer
2016-04-27 19:45     ` Martin Pietryka
2016-04-27 19:39 ` [U-Boot] [PATCH v3 " Martin Pietryka
2016-04-27 19:39   ` [U-Boot] [PATCH v3 2/2] drivers/video/am335x-fb: Properly point framebuffer behind palette Martin Pietryka
2016-04-28 14:57     ` Anatolij Gustschin
2016-04-28  4:59   ` [U-Boot] [PATCH v3 1/2] drivers/video/am335x-fb: Add support for 16bpp format Hannes Schmelzer
2016-04-28 14:56   ` Anatolij Gustschin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.