u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2] common/lcd: use lcd_setcolreg() to setup CLUT
@ 2012-11-02 14:18 Andreas Bießmann
  2012-11-06 10:13 ` [U-Boot] [PATCH v3 0/2] rework common/lcd Andreas Bießmann
  0 siblings, 1 reply; 14+ messages in thread
From: Andreas Bießmann @ 2012-11-02 14:18 UTC (permalink / raw)
  To: u-boot

The lcd_setcolreg() is a API provided by the lcd driver and used to setup the
color lookup table. However the lcd driver setup the CLUT by using a lot of
ifdiffery and accessing the CLUT arrays directly instead. Remove that and use
the API.

Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Anatolij Gustschin <agust@denx.de>
---
since v1:
 * also use lcd_setcolreg() in lcd_display_bitmap()
 * remove RFC

This patch requires http://patchwork.ozlabs.org/patch/194861/ to compile all
arm boards cleanly!

Runtime tested on at91sam9263ek, please test and comment
Build tested for AVR32 and arm

 common/lcd.c |   79 ++++++++++++----------------------------------------------
 1 file changed, 16 insertions(+), 63 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index b6be800..6835e65 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -523,11 +523,6 @@ static inline ushort *configuration_get_cmap(void)
 #ifdef CONFIG_LCD_LOGO
 void bitmap_plot(int x, int y)
 {
-#ifdef CONFIG_ATMEL_LCD
-	uint *cmap = (uint *)bmp_logo_palette;
-#else
-	ushort *cmap = (ushort *)bmp_logo_palette;
-#endif
 	ushort i, j;
 	uchar *bmap;
 	uchar *fb;
@@ -545,44 +540,20 @@ void bitmap_plot(int x, int y)
 	fb   = (uchar *)(lcd_base + y * lcd_line_length + x);
 
 	if (NBITS(panel_info.vl_bpix) < 12) {
-		/* Leave room for default color map
-		 * default case: generic system with no cmap (most likely 16bpp)
-		 * cmap was set to the source palette, so no change is done.
-		 * This avoids even more ifdefs in the next stanza
-		 */
-#if defined(CONFIG_MPC823)
-		cmap = (ushort *) &(cp->lcd_cmap[BMP_LOGO_OFFSET * sizeof(ushort)]);
-#elif defined(CONFIG_ATMEL_LCD)
-		cmap = (uint *)configuration_get_cmap();
-#else
-		cmap = configuration_get_cmap();
-#endif
 
 		WATCHDOG_RESET();
 
 		/* Set color map */
 		for (i = 0; i < ARRAY_SIZE(bmp_logo_palette); ++i) {
-			ushort colreg = bmp_logo_palette[i];
-#ifdef CONFIG_ATMEL_LCD
-			uint lut_entry;
-#ifdef CONFIG_ATMEL_LCD_BGR555
-			lut_entry = ((colreg & 0x000F) << 11) |
-					((colreg & 0x00F0) <<  2) |
-					((colreg & 0x0F00) >>  7);
-#else /* CONFIG_ATMEL_LCD_RGB565 */
-			lut_entry = ((colreg & 0x000F) << 1) |
-					((colreg & 0x00F0) << 3) |
-					((colreg & 0x0F00) << 4);
-#endif
-			*(cmap + BMP_LOGO_OFFSET) = lut_entry;
-			cmap++;
-#else /* !CONFIG_ATMEL_LCD */
-#ifdef  CONFIG_SYS_INVERT_COLORS
-			*cmap++ = 0xffff - colreg;
-#else
-			*cmap++ = colreg;
-#endif
-#endif /* CONFIG_ATMEL_LCD */
+			/* use the most significant bits here */
+			uint8_t red   = ((bmp_logo_palette[i] >> 4) & 0xF0);
+			uint8_t green = ((bmp_logo_palette[i] >> 0) & 0xF0);
+			uint8_t blue  = ((bmp_logo_palette[i] << 4) & 0xF0);
+			debug("LCD: setup colreg %u with "
+			      "R: 0x%02x G: 0x%02x B: 0x%02x (0x%04x)\n",
+			      i+BMP_LOGO_OFFSET, red, green, blue, bmp_logo_palette[i]);
+			/* leave room for the default colormap here */
+			lcd_setcolreg(i+BMP_LOGO_OFFSET, red, green, blue);
 		}
 
 		WATCHDOG_RESET();
@@ -667,9 +638,6 @@ static inline void fb_put_word(uchar **fb, uchar **from)
 
 int lcd_display_bitmap(ulong bmp_image, int x, int y)
 {
-#if !defined(CONFIG_MCC200)
-	ushort *cmap = NULL;
-#endif
 	ushort *cmap_base = NULL;
 	ushort i, j;
 	uchar *fb;
@@ -716,34 +684,18 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 #if !defined(CONFIG_MCC200)
 	/* MCC200 LCD doesn't need CMAP, supports 1bpp b&w only */
 	if (bmp_bpix == 8) {
-		cmap = configuration_get_cmap();
-		cmap_base = cmap;
+		cmap_base = configuration_get_cmap();
 
 		/* Set color map */
 		for (i = 0; i < colors; ++i) {
 			bmp_color_table_entry_t cte = bmp->color_table[i];
-#if !defined(CONFIG_ATMEL_LCD)
-			ushort colreg =
-				( ((cte.red)   << 8) & 0xf800) |
-				( ((cte.green) << 3) & 0x07e0) |
-				( ((cte.blue)  >> 3) & 0x001f) ;
-#ifdef CONFIG_SYS_INVERT_COLORS
-			*cmap = 0xffff - colreg;
-#else
-			*cmap = colreg;
-#endif
-#if defined(CONFIG_MPC823)
-			cmap--;
-#else
-			cmap++;
-#endif
-#else /* CONFIG_ATMEL_LCD */
+			debug("LCD: setup colreg %u with "
+			      "R: 0x%02x G: 0x%02x B: 0x%02x\n",
+			      i, cte.red, cte.green, cte.blue);
 			lcd_setcolreg(i, cte.red, cte.green, cte.blue);
-#endif
 		}
 	}
-#endif
-
+#else
 	/*
 	 *  BMP format for Monochrome assumes that the state of a
 	 * pixel is described on a per Bit basis, not per Byte.
@@ -754,7 +706,6 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 	 * their own ways, so make the converting to be MCC200
 	 * specific.
 	 */
-#if defined(CONFIG_MCC200)
 	if (bpix == 1) {
 		width = ((width + 7) & ~7) >> 3;
 		x     = ((x + 7) & ~7) >> 3;
@@ -786,6 +737,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 		else
 			byte_width = width * 2;
 
+		debug("LCD: draw %i b/pix image on %i b/pix display\n", bmp_bpix, bpix);
+
 		for (i = 0; i < height; ++i) {
 			WATCHDOG_RESET();
 			for (j = 0; j < width; j++) {
-- 
1.7.10.4

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

* [U-Boot] [PATCH v3 0/2] rework common/lcd
  2012-11-02 14:18 [U-Boot] [PATCH v2] common/lcd: use lcd_setcolreg() to setup CLUT Andreas Bießmann
@ 2012-11-06 10:13 ` Andreas Bießmann
  2012-11-06 10:13   ` [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion Andreas Bießmann
  2012-11-06 10:13   ` [U-Boot] [PATCH v3 2/2] common/lcd: use lcd_setcolreg() to setup CLUT Andreas Bießmann
  0 siblings, 2 replies; 14+ messages in thread
From: Andreas Bießmann @ 2012-11-06 10:13 UTC (permalink / raw)
  To: u-boot

This series rework the CLUT management in common/lcd and use the driver API
lcd_setcolreg() for that. The change was first requested from Marek Vasut due to
a compiler warning (type-punned pointer stuff) encountered with eldk 4.2 and
introduced in 203c37b8c5556aad1901ce4954792afd718c7d42

BEWARE this change is only tested on atmel devices so far

Andreas Bie?mann (1):
  common/lcd: use lcd_setcolreg() to setup CLUT

Bo Shen (1):
  video: atmel: implement lcd_setcolreg funtion

 common/lcd.c                 |   79 +++++++++---------------------------------
 drivers/video/atmel_hlcdfb.c |    6 ++++
 2 files changed, 22 insertions(+), 63 deletions(-)

-- 
1.7.10.4

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

* [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion
  2012-11-06 10:13 ` [U-Boot] [PATCH v3 0/2] rework common/lcd Andreas Bießmann
@ 2012-11-06 10:13   ` Andreas Bießmann
  2012-11-06 22:54     ` Marek Vasut
                       ` (2 more replies)
  2012-11-06 10:13   ` [U-Boot] [PATCH v3 2/2] common/lcd: use lcd_setcolreg() to setup CLUT Andreas Bießmann
  1 sibling, 3 replies; 14+ messages in thread
From: Andreas Bießmann @ 2012-11-06 10:13 UTC (permalink / raw)
  To: u-boot

From: Bo Shen <voice.shen@atmel.com>

Signed-off-by: Bo Shen <voice.shen@atmel.com>
Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
---
since v2:
 * add this single patch

 drivers/video/atmel_hlcdfb.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
index beb7fa3..4110d4d 100644
--- a/drivers/video/atmel_hlcdfb.c
+++ b/drivers/video/atmel_hlcdfb.c
@@ -51,6 +51,12 @@ short console_row;
 #define lcdc_readl(reg)		__raw_readl((reg))
 #define lcdc_writel(reg, val)	__raw_writel((val), (reg))
 
+void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
+{
+	lcdc_writel((red << 16) | (green << 8) | blue,
+			panel_info.mmio + ATMEL_LCDC_LUT(regno));
+}
+
 void lcd_ctrl_init(void *lcdbase)
 {
 	unsigned long value;
-- 
1.7.10.4

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

* [U-Boot] [PATCH v3 2/2] common/lcd: use lcd_setcolreg() to setup CLUT
  2012-11-06 10:13 ` [U-Boot] [PATCH v3 0/2] rework common/lcd Andreas Bießmann
  2012-11-06 10:13   ` [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion Andreas Bießmann
@ 2012-11-06 10:13   ` Andreas Bießmann
  1 sibling, 0 replies; 14+ messages in thread
From: Andreas Bießmann @ 2012-11-06 10:13 UTC (permalink / raw)
  To: u-boot

The lcd_setcolreg() is a API provided by the lcd driver and used to setup the
color lookup table. However the lcd driver setup the CLUT by using a lot of
ifdiffery and accessing the CLUT arrays directly instead. Remove that and use
the API.

Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Anatolij Gustschin <agust@denx.de>
---
since v1:
 * also use lcd_setcolreg() in lcd_display_bitmap()
 * remove RFC

since v2:
 * no change

 common/lcd.c |   79 ++++++++++++----------------------------------------------
 1 file changed, 16 insertions(+), 63 deletions(-)

diff --git a/common/lcd.c b/common/lcd.c
index b6be800..6835e65 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -523,11 +523,6 @@ static inline ushort *configuration_get_cmap(void)
 #ifdef CONFIG_LCD_LOGO
 void bitmap_plot(int x, int y)
 {
-#ifdef CONFIG_ATMEL_LCD
-	uint *cmap = (uint *)bmp_logo_palette;
-#else
-	ushort *cmap = (ushort *)bmp_logo_palette;
-#endif
 	ushort i, j;
 	uchar *bmap;
 	uchar *fb;
@@ -545,44 +540,20 @@ void bitmap_plot(int x, int y)
 	fb   = (uchar *)(lcd_base + y * lcd_line_length + x);
 
 	if (NBITS(panel_info.vl_bpix) < 12) {
-		/* Leave room for default color map
-		 * default case: generic system with no cmap (most likely 16bpp)
-		 * cmap was set to the source palette, so no change is done.
-		 * This avoids even more ifdefs in the next stanza
-		 */
-#if defined(CONFIG_MPC823)
-		cmap = (ushort *) &(cp->lcd_cmap[BMP_LOGO_OFFSET * sizeof(ushort)]);
-#elif defined(CONFIG_ATMEL_LCD)
-		cmap = (uint *)configuration_get_cmap();
-#else
-		cmap = configuration_get_cmap();
-#endif
 
 		WATCHDOG_RESET();
 
 		/* Set color map */
 		for (i = 0; i < ARRAY_SIZE(bmp_logo_palette); ++i) {
-			ushort colreg = bmp_logo_palette[i];
-#ifdef CONFIG_ATMEL_LCD
-			uint lut_entry;
-#ifdef CONFIG_ATMEL_LCD_BGR555
-			lut_entry = ((colreg & 0x000F) << 11) |
-					((colreg & 0x00F0) <<  2) |
-					((colreg & 0x0F00) >>  7);
-#else /* CONFIG_ATMEL_LCD_RGB565 */
-			lut_entry = ((colreg & 0x000F) << 1) |
-					((colreg & 0x00F0) << 3) |
-					((colreg & 0x0F00) << 4);
-#endif
-			*(cmap + BMP_LOGO_OFFSET) = lut_entry;
-			cmap++;
-#else /* !CONFIG_ATMEL_LCD */
-#ifdef  CONFIG_SYS_INVERT_COLORS
-			*cmap++ = 0xffff - colreg;
-#else
-			*cmap++ = colreg;
-#endif
-#endif /* CONFIG_ATMEL_LCD */
+			/* use the most significant bits here */
+			uint8_t red   = ((bmp_logo_palette[i] >> 4) & 0xF0);
+			uint8_t green = ((bmp_logo_palette[i] >> 0) & 0xF0);
+			uint8_t blue  = ((bmp_logo_palette[i] << 4) & 0xF0);
+			debug("LCD: setup colreg %u with "
+			      "R: 0x%02x G: 0x%02x B: 0x%02x (0x%04x)\n",
+			      i+BMP_LOGO_OFFSET, red, green, blue, bmp_logo_palette[i]);
+			/* leave room for the default colormap here */
+			lcd_setcolreg(i+BMP_LOGO_OFFSET, red, green, blue);
 		}
 
 		WATCHDOG_RESET();
@@ -667,9 +638,6 @@ static inline void fb_put_word(uchar **fb, uchar **from)
 
 int lcd_display_bitmap(ulong bmp_image, int x, int y)
 {
-#if !defined(CONFIG_MCC200)
-	ushort *cmap = NULL;
-#endif
 	ushort *cmap_base = NULL;
 	ushort i, j;
 	uchar *fb;
@@ -716,34 +684,18 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 #if !defined(CONFIG_MCC200)
 	/* MCC200 LCD doesn't need CMAP, supports 1bpp b&w only */
 	if (bmp_bpix == 8) {
-		cmap = configuration_get_cmap();
-		cmap_base = cmap;
+		cmap_base = configuration_get_cmap();
 
 		/* Set color map */
 		for (i = 0; i < colors; ++i) {
 			bmp_color_table_entry_t cte = bmp->color_table[i];
-#if !defined(CONFIG_ATMEL_LCD)
-			ushort colreg =
-				( ((cte.red)   << 8) & 0xf800) |
-				( ((cte.green) << 3) & 0x07e0) |
-				( ((cte.blue)  >> 3) & 0x001f) ;
-#ifdef CONFIG_SYS_INVERT_COLORS
-			*cmap = 0xffff - colreg;
-#else
-			*cmap = colreg;
-#endif
-#if defined(CONFIG_MPC823)
-			cmap--;
-#else
-			cmap++;
-#endif
-#else /* CONFIG_ATMEL_LCD */
+			debug("LCD: setup colreg %u with "
+			      "R: 0x%02x G: 0x%02x B: 0x%02x\n",
+			      i, cte.red, cte.green, cte.blue);
 			lcd_setcolreg(i, cte.red, cte.green, cte.blue);
-#endif
 		}
 	}
-#endif
-
+#else
 	/*
 	 *  BMP format for Monochrome assumes that the state of a
 	 * pixel is described on a per Bit basis, not per Byte.
@@ -754,7 +706,6 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 	 * their own ways, so make the converting to be MCC200
 	 * specific.
 	 */
-#if defined(CONFIG_MCC200)
 	if (bpix == 1) {
 		width = ((width + 7) & ~7) >> 3;
 		x     = ((x + 7) & ~7) >> 3;
@@ -786,6 +737,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
 		else
 			byte_width = width * 2;
 
+		debug("LCD: draw %i b/pix image on %i b/pix display\n", bmp_bpix, bpix);
+
 		for (i = 0; i < height; ++i) {
 			WATCHDOG_RESET();
 			for (j = 0; j < width; j++) {
-- 
1.7.10.4

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

* [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion
  2012-11-06 10:13   ` [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion Andreas Bießmann
@ 2012-11-06 22:54     ` Marek Vasut
  2012-11-07  1:26       ` Bo Shen
  2012-11-08  9:10     ` [U-Boot] [PATCH v4] " Bo Shen
  2012-11-09  3:49     ` [U-Boot] [Patch v5] video: atmel: implement lcd_setcolreg function Bo Shen
  2 siblings, 1 reply; 14+ messages in thread
From: Marek Vasut @ 2012-11-06 22:54 UTC (permalink / raw)
  To: u-boot

Dear Andreas Bie?mann,

> From: Bo Shen <voice.shen@atmel.com>

Missing commit message

> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
> ---
> since v2:
>  * add this single patch
> 
>  drivers/video/atmel_hlcdfb.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
> index beb7fa3..4110d4d 100644
> --- a/drivers/video/atmel_hlcdfb.c
> +++ b/drivers/video/atmel_hlcdfb.c
> @@ -51,6 +51,12 @@ short console_row;
>  #define lcdc_readl(reg)		__raw_readl((reg))
>  #define lcdc_writel(reg, val)	__raw_writel((val), (reg))
> 
> +void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
> +{
> +	lcdc_writel((red << 16) | (green << 8) | blue,
> +			panel_info.mmio + ATMEL_LCDC_LUT(regno));

So this is RGB666? Or what are those magic numbers ?

> +}
> +
>  void lcd_ctrl_init(void *lcdbase)
>  {
>  	unsigned long value;

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion
  2012-11-06 22:54     ` Marek Vasut
@ 2012-11-07  1:26       ` Bo Shen
  2012-11-07 13:26         ` Marek Vasut
  0 siblings, 1 reply; 14+ messages in thread
From: Bo Shen @ 2012-11-07  1:26 UTC (permalink / raw)
  To: u-boot

Hi Marek,

On 11/7/2012 6:54, Marek Vasut wrote:
> Dear Andreas Bie?mann,
>
>> From: Bo Shen <voice.shen@atmel.com>
>
> Missing commit message
>
>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>> Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>> ---
>> since v2:
>>   * add this single patch
>>
>>   drivers/video/atmel_hlcdfb.c |    6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
>> index beb7fa3..4110d4d 100644
>> --- a/drivers/video/atmel_hlcdfb.c
>> +++ b/drivers/video/atmel_hlcdfb.c
>> @@ -51,6 +51,12 @@ short console_row;
>>   #define lcdc_readl(reg)		__raw_readl((reg))
>>   #define lcdc_writel(reg, val)	__raw_writel((val), (reg))
>>
>> +void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
>> +{
>> +	lcdc_writel((red << 16) | (green << 8) | blue,
>> +			panel_info.mmio + ATMEL_LCDC_LUT(regno));
>
> So this is RGB666? Or what are those magic numbers ?

This is a little different with the driver of atmel_lcdfb.c.
The register for LUT is layout as following:
RCLUT (24 ~ 16), GCLUT (15 ~ 8) and BCLUT (7 ~ 0).
So, use those magic numbers.

More information, you can get from [1] on page 1163.

1. http://www.atmel.com/Images/doc11053.pdf

BRs
Bo Shen

>> +}
>> +
>>   void lcd_ctrl_init(void *lcdbase)
>>   {
>>   	unsigned long value;
>
> Best regards,
> Marek Vasut
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

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

* [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion
  2012-11-07  1:26       ` Bo Shen
@ 2012-11-07 13:26         ` Marek Vasut
  2012-11-08  3:06           ` Bo Shen
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Vasut @ 2012-11-07 13:26 UTC (permalink / raw)
  To: u-boot

Dear Bo Shen,

> Hi Marek,
> 
> On 11/7/2012 6:54, Marek Vasut wrote:
> > Dear Andreas Bie?mann,
> > 
> >> From: Bo Shen <voice.shen@atmel.com>
> > 
> > Missing commit message
> > 
> >> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> >> Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
> >> ---
> >> 
> >> since v2:
> >>   * add this single patch
> >>   
> >>   drivers/video/atmel_hlcdfb.c |    6 ++++++
> >>   1 file changed, 6 insertions(+)
> >> 
> >> diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
> >> index beb7fa3..4110d4d 100644
> >> --- a/drivers/video/atmel_hlcdfb.c
> >> +++ b/drivers/video/atmel_hlcdfb.c
> >> @@ -51,6 +51,12 @@ short console_row;
> >> 
> >>   #define lcdc_readl(reg)		__raw_readl((reg))
> >>   #define lcdc_writel(reg, val)	__raw_writel((val), (reg))
> >> 
> >> +void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
> >> +{
> >> +	lcdc_writel((red << 16) | (green << 8) | blue,
> >> +			panel_info.mmio + ATMEL_LCDC_LUT(regno));
> > 
> > So this is RGB666? Or what are those magic numbers ?
> 
> This is a little different with the driver of atmel_lcdfb.c.
> The register for LUT is layout as following:
> RCLUT (24 ~ 16), GCLUT (15 ~ 8) and BCLUT (7 ~ 0).
> So, use those magic numbers.

Good, can you define those magic offsets then please?

> More information, you can get from [1] on page 1163.
> 
> 1. http://www.atmel.com/Images/doc11053.pdf

[...]

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion
  2012-11-07 13:26         ` Marek Vasut
@ 2012-11-08  3:06           ` Bo Shen
  2012-11-08  7:53             ` Andreas Bießmann
  0 siblings, 1 reply; 14+ messages in thread
From: Bo Shen @ 2012-11-08  3:06 UTC (permalink / raw)
  To: u-boot

Hi Andreas,

On 11/7/2012 21:26, Marek Vasut wrote:
> Dear Bo Shen,
>
>> Hi Marek,
>>
>> On 11/7/2012 6:54, Marek Vasut wrote:
>>> Dear Andreas Bie?mann,
>>>
>>>> From: Bo Shen <voice.shen@atmel.com>
>>>
>>> Missing commit message
>>>
>>>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>>>> Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>>>> ---
>>>>
>>>> since v2:
>>>>    * add this single patch
>>>>
>>>>    drivers/video/atmel_hlcdfb.c |    6 ++++++
>>>>    1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
>>>> index beb7fa3..4110d4d 100644
>>>> --- a/drivers/video/atmel_hlcdfb.c
>>>> +++ b/drivers/video/atmel_hlcdfb.c
>>>> @@ -51,6 +51,12 @@ short console_row;
>>>>
>>>>    #define lcdc_readl(reg)		__raw_readl((reg))
>>>>    #define lcdc_writel(reg, val)	__raw_writel((val), (reg))
>>>>
>>>> +void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
>>>> +{
>>>> +	lcdc_writel((red << 16) | (green << 8) | blue,
>>>> +			panel_info.mmio + ATMEL_LCDC_LUT(regno));
>>>
>>> So this is RGB666? Or what are those magic numbers ?
>>
>> This is a little different with the driver of atmel_lcdfb.c.
>> The register for LUT is layout as following:
>> RCLUT (24 ~ 16), GCLUT (15 ~ 8) and BCLUT (7 ~ 0).
>> So, use those magic numbers.
>
> Good, can you define those magic offsets then please?

Will you redo your patch series or do I need to implement this and send 
the patch to you again?

Which do you prefer?

Best Regards,
Bo Shen

>> More information, you can get from [1] on page 1163.
>>
>> 1. http://www.atmel.com/Images/doc11053.pdf
>
> [...]
>
> Best regards,
> Marek Vasut
>

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

* [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion
  2012-11-08  3:06           ` Bo Shen
@ 2012-11-08  7:53             ` Andreas Bießmann
  0 siblings, 0 replies; 14+ messages in thread
From: Andreas Bießmann @ 2012-11-08  7:53 UTC (permalink / raw)
  To: u-boot

Hi Bo,

On 08.11.2012 04:06, Bo Shen wrote:
> Hi Andreas,
> 
> On 11/7/2012 21:26, Marek Vasut wrote:
>> Dear Bo Shen,
>>
>>> Hi Marek,
>>>
>>> On 11/7/2012 6:54, Marek Vasut wrote:
>>>> Dear Andreas Bie?mann,
>>>>
>>>>> From: Bo Shen <voice.shen@atmel.com>
>>>>
>>>> Missing commit message
>>>>
>>>>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>>>>> Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>>>>> ---
>>>>>
>>>>> since v2:
>>>>>    * add this single patch
>>>>>
>>>>>    drivers/video/atmel_hlcdfb.c |    6 ++++++
>>>>>    1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/drivers/video/atmel_hlcdfb.c
>>>>> b/drivers/video/atmel_hlcdfb.c
>>>>> index beb7fa3..4110d4d 100644
>>>>> --- a/drivers/video/atmel_hlcdfb.c
>>>>> +++ b/drivers/video/atmel_hlcdfb.c
>>>>> @@ -51,6 +51,12 @@ short console_row;
>>>>>
>>>>>    #define lcdc_readl(reg)        __raw_readl((reg))
>>>>>    #define lcdc_writel(reg, val)    __raw_writel((val), (reg))
>>>>>
>>>>> +void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort
>>>>> blue)
>>>>> +{
>>>>> +    lcdc_writel((red << 16) | (green << 8) | blue,
>>>>> +            panel_info.mmio + ATMEL_LCDC_LUT(regno));
>>>>
>>>> So this is RGB666? Or what are those magic numbers ?
>>>
>>> This is a little different with the driver of atmel_lcdfb.c.
>>> The register for LUT is layout as following:
>>> RCLUT (24 ~ 16), GCLUT (15 ~ 8) and BCLUT (7 ~ 0).
>>> So, use those magic numbers.
>>
>> Good, can you define those magic offsets then please?
> 
> Will you redo your patch series or do I need to implement this and send
> the patch to you again?

can you please just send a v4 for this single patch? Add correct
in-reply-to header so we can see it belongs to the series. Thanks in
advance, I'm a bit busy currently.

Best regards

Andreas Bie?mann

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

* [U-Boot] [PATCH v4] video: atmel: implement lcd_setcolreg funtion
  2012-11-06 10:13   ` [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion Andreas Bießmann
  2012-11-06 22:54     ` Marek Vasut
@ 2012-11-08  9:10     ` Bo Shen
  2012-11-08 13:08       ` Marek Vasut
  2012-11-09  3:49     ` [U-Boot] [Patch v5] video: atmel: implement lcd_setcolreg function Bo Shen
  2 siblings, 1 reply; 14+ messages in thread
From: Bo Shen @ 2012-11-08  9:10 UTC (permalink / raw)
  To: u-boot

implement the common api lce_setcolreg in include/lcd.h

Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
since v3:
  * add magic number
since v2:
  * add this single patch
---
 drivers/video/atmel_hlcdfb.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
index beb7fa3..8ebde04 100644
--- a/drivers/video/atmel_hlcdfb.c
+++ b/drivers/video/atmel_hlcdfb.c
@@ -51,6 +51,16 @@ short console_row;
 #define lcdc_readl(reg)		__raw_readl((reg))
 #define lcdc_writel(reg, val)	__raw_writel((val), (reg))
 
+/*
+ * the CLUT register map as following
+ * RCLUT(24 ~ 16), GCLUT(15 ~ 8), BCLUT(7 ~ 0)
+ */
+void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
+{
+	lcdc_writel((red << 16) & 0xff0000 | (green << 8) & 0xff00 |
+		blue & 0xff, panel_info.mmio + ATMEL_LCDC_LUT(regno));
+}
+
 void lcd_ctrl_init(void *lcdbase)
 {
 	unsigned long value;
-- 
1.7.9.5

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

* [U-Boot] [PATCH v4] video: atmel: implement lcd_setcolreg funtion
  2012-11-08  9:10     ` [U-Boot] [PATCH v4] " Bo Shen
@ 2012-11-08 13:08       ` Marek Vasut
  2012-11-09  3:46         ` Bo Shen
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Vasut @ 2012-11-08 13:08 UTC (permalink / raw)
  To: u-boot

Dear Bo Shen,

[...]
> +/*
> + * the CLUT register map as following
> + * RCLUT(24 ~ 16), GCLUT(15 ~ 8), BCLUT(7 ~ 0)
> + */
> +void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
> +{
> +	lcdc_writel((red << 16) & 0xff0000 | (green << 8) & 0xff00 |
> +		blue & 0xff, panel_info.mmio + ATMEL_LCDC_LUT(regno));
> +}
> +

Why don't you #define these values instead?

>  void lcd_ctrl_init(void *lcdbase)
>  {
>  	unsigned long value;

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH v4] video: atmel: implement lcd_setcolreg funtion
  2012-11-08 13:08       ` Marek Vasut
@ 2012-11-09  3:46         ` Bo Shen
  0 siblings, 0 replies; 14+ messages in thread
From: Bo Shen @ 2012-11-09  3:46 UTC (permalink / raw)
  To: u-boot

Hi Marek Vasut,

On 11/8/2012 21:08, Marek Vasut wrote:
> Dear Bo Shen,
>
> [...]
>> +/*
>> + * the CLUT register map as following
>> + * RCLUT(24 ~ 16), GCLUT(15 ~ 8), BCLUT(7 ~ 0)
>> + */
>> +void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
>> +{
>> +	lcdc_writel((red << 16) & 0xff0000 | (green << 8) & 0xff00 |
>> +		blue & 0xff, panel_info.mmio + ATMEL_LCDC_LUT(regno));
>> +}
>> +
>
> Why don't you #define these values instead?

Ok, I will change to use #define.

Thanks,

Best Regards,
Bo Shen

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

* [U-Boot] [Patch v5] video: atmel: implement lcd_setcolreg function
  2012-11-06 10:13   ` [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion Andreas Bießmann
  2012-11-06 22:54     ` Marek Vasut
  2012-11-08  9:10     ` [U-Boot] [PATCH v4] " Bo Shen
@ 2012-11-09  3:49     ` Bo Shen
  2012-11-10 13:13       ` Anatolij Gustschin
  2 siblings, 1 reply; 14+ messages in thread
From: Bo Shen @ 2012-11-09  3:49 UTC (permalink / raw)
  To: u-boot

implement the common api lce_setcolreg in include/lcd.h

Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
since v4:
  * using define for these magic number
since v3:
  * add magic number
since v2:
  * add this single patch
---
 drivers/video/atmel_hlcdfb.c |   12 ++++++++++++
 include/atmel_hlcdc.h        |    7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
index beb7fa3..e04eaf6 100644
--- a/drivers/video/atmel_hlcdfb.c
+++ b/drivers/video/atmel_hlcdfb.c
@@ -51,6 +51,18 @@ short console_row;
 #define lcdc_readl(reg)		__raw_readl((reg))
 #define lcdc_writel(reg, val)	__raw_writel((val), (reg))
 
+/*
+ * the CLUT register map as following
+ * RCLUT(24 ~ 16), GCLUT(15 ~ 8), BCLUT(7 ~ 0)
+ */
+void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
+{
+	lcdc_writel((red << LCDC_BASECLUT_RCLUT_Pos) & LCDC_BASECLUT_RCLUT_Msk
+		| (green << LCDC_BASECLUT_GCLUT_Pos) & LCDC_BASECLUT_GCLUT_Msk
+		| (blue << LCDC_BASECLUT_BCLUT_Pos) & LCDC_BASECLUT_BCLUT_Msk,
+		panel_info.mmio + ATMEL_LCDC_LUT(regno));
+}
+
 void lcd_ctrl_init(void *lcdbase)
 {
 	unsigned long value;
diff --git a/include/atmel_hlcdc.h b/include/atmel_hlcdc.h
index 945b30a..fbd2f92 100644
--- a/include/atmel_hlcdc.h
+++ b/include/atmel_hlcdc.h
@@ -217,6 +217,13 @@ struct atmel_hlcd_regs {
 #define LCDC_BASECFG3_RDEF(value) \
 	((LCDC_BASECFG3_RDEF_Msk & ((value) << LCDC_BASECFG3_RDEF_Pos)))
 
+#define LCDC_BASECLUT_BCLUT_Pos 0
+#define LCDC_BASECLUT_BCLUT_Msk (0xff << LCDC_BASECLUT_BCLUT_Pos)
+#define LCDC_BASECLUT_GCLUT_Pos 8
+#define LCDC_BASECLUT_GCLUT_Msk (0xff << LCDC_BASECLUT_GCLUT_Pos)
+#define LCDC_BASECLUT_RCLUT_Pos 16
+#define LCDC_BASECLUT_RCLUT_Msk (0xff << LCDC_BASECLUT_RCLUT_Pos)
+
 #define LCDC_BASECFG4_DMA	(0x1 << 8)
 #define LCDC_BASECFG4_REP	(0x1 << 9)
 
-- 
1.7.9.5

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

* [U-Boot] [Patch v5] video: atmel: implement lcd_setcolreg function
  2012-11-09  3:49     ` [U-Boot] [Patch v5] video: atmel: implement lcd_setcolreg function Bo Shen
@ 2012-11-10 13:13       ` Anatolij Gustschin
  0 siblings, 0 replies; 14+ messages in thread
From: Anatolij Gustschin @ 2012-11-10 13:13 UTC (permalink / raw)
  To: u-boot

Hi,

On Fri,  9 Nov 2012 11:49:14 +0800
Bo Shen <voice.shen@atmel.com> wrote:

> implement the common api lce_setcolreg in include/lcd.h
> 
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> ---
> since v4:
>   * using define for these magic number
> since v3:
>   * add magic number
> since v2:
>   * add this single patch
> ---
>  drivers/video/atmel_hlcdfb.c |   12 ++++++++++++
>  include/atmel_hlcdc.h        |    7 +++++++
>  2 files changed, 19 insertions(+)

Applied to video/master after fixing commit log and gcc 4.6 warnings.

Thanks!

Anatolij

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

end of thread, other threads:[~2012-11-10 13:13 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-02 14:18 [U-Boot] [PATCH v2] common/lcd: use lcd_setcolreg() to setup CLUT Andreas Bießmann
2012-11-06 10:13 ` [U-Boot] [PATCH v3 0/2] rework common/lcd Andreas Bießmann
2012-11-06 10:13   ` [U-Boot] [PATCH v3 1/2] video: atmel: implement lcd_setcolreg funtion Andreas Bießmann
2012-11-06 22:54     ` Marek Vasut
2012-11-07  1:26       ` Bo Shen
2012-11-07 13:26         ` Marek Vasut
2012-11-08  3:06           ` Bo Shen
2012-11-08  7:53             ` Andreas Bießmann
2012-11-08  9:10     ` [U-Boot] [PATCH v4] " Bo Shen
2012-11-08 13:08       ` Marek Vasut
2012-11-09  3:46         ` Bo Shen
2012-11-09  3:49     ` [U-Boot] [Patch v5] video: atmel: implement lcd_setcolreg function Bo Shen
2012-11-10 13:13       ` Anatolij Gustschin
2012-11-06 10:13   ` [U-Boot] [PATCH v3 2/2] common/lcd: use lcd_setcolreg() to setup CLUT Andreas Bießmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).