Re: [U-Boot] [PATCH v2 2/2] video: bcm2835: respect the pitch value

2013-11-07 Thread Anatolij Gustschin
On Thu, 24 Oct 2013 20:00:41 +0200
Andre Heider a.hei...@gmail.com wrote:
...
 @@ -90,6 +94,7 @@ void lcd_ctrl_init(void *lcdbase)
  
   w = msg_setup-physical_w_h.body.resp.width;
   h = msg_setup-physical_w_h.body.resp.height;
 + bcm2835_pitch = msg_setup-pitch.body.resp.pitch;
  
   debug(bcm2835: Final resolution is %d x %d\n, w, h);
  
 @@ -102,4 +107,6 @@ void lcd_ctrl_init(void *lcdbase)
  
  void lcd_enable(void)
  {
 + if (bcm2835_pitch)
 + lcd_line_length = bcm2835_pitch;
  }

setting lcd_line_length in lcd_enable() is wrong, it should
happen earlier, before lcd_clear() is called in the lcd.c
driver. I suggest making the lcd_get_size() in lcd.c a weak
function and then provide bcm2835 specific lcd_get_size()
which sets the pitch as needed:

diff --git a/common/lcd.c b/common/lcd.c
index 5dd7948..60faa62 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -386,8 +386,13 @@ static void test_pattern(void)
 //
 /* ** GENERIC Initialization Routines  */
 //
-
-int lcd_get_size(int *line_length)
+/*
+ * Implement a weak default function for getting the length/size
+ * from panel_info parameters. With some boards/drivers the
+ * length might need adjustments, so allow defining the driver
+ * specific lcd_get_size() function.
+ */
+__weak int lcd_get_size(int *line_length)
 {
*line_length = (panel_info.vl_col * NBITS(panel_info.vl_bpix)) / 8;
return *line_length * panel_info.vl_row;

and

diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c
index 58a6163..45b470a 100644
--- a/drivers/video/bcm2835.c
+++ b/drivers/video/bcm2835.c
@@ -103,3 +103,9 @@ void lcd_ctrl_init(void *lcdbase)
 void lcd_enable(void)
 {
 }
+
+void lcd_get_size(int *line_length)
+{
+   *line_length = bcm2835_pitch;
+   return *line_length * panel_info.vl_row;
+}

Anatolij
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 2/2] video: bcm2835: respect the pitch value

2013-10-24 Thread Andre Heider
Depending on the firmware's video options [1] the active SDTV or
HDTV mode can yield a framebuffer with noncontiguous horizontal lines,
giving a messed up display, for both, u-boot and the loaded kernel.

Fix this by setting lcd_line_length to the pitch value of the configured
framebuffer.

[1] http://elinux.org/RPiconfig#Video_mode_options

Signed-off-by: Andre Heider a.hei...@gmail.com
---
 drivers/video/bcm2835.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c
index 58a6163..431b8a8 100644
--- a/drivers/video/bcm2835.c
+++ b/drivers/video/bcm2835.c
@@ -14,6 +14,8 @@ DECLARE_GLOBAL_DATA_PTR;
 /* Global variables that lcd.c expects to exist */
 vidinfo_t panel_info;
 
+static u32 bcm2835_pitch;
+
 struct msg_query {
struct bcm2835_mbox_hdr hdr;
struct bcm2835_mbox_tag_physical_w_h physical_w_h;
@@ -30,6 +32,7 @@ struct msg_setup {
struct bcm2835_mbox_tag_virtual_offset virtual_offset;
struct bcm2835_mbox_tag_overscan overscan;
struct bcm2835_mbox_tag_allocate_buffer allocate_buffer;
+   struct bcm2835_mbox_tag_pitch pitch;
u32 end_tag;
 };
 
@@ -80,6 +83,7 @@ void lcd_ctrl_init(void *lcdbase)
msg_setup-overscan.body.req.right = 0;
BCM2835_MBOX_INIT_TAG(msg_setup-allocate_buffer, ALLOCATE_BUFFER);
msg_setup-allocate_buffer.body.req.alignment = 0x100;
+   BCM2835_MBOX_INIT_TAG_NO_REQ(msg_setup-pitch, GET_PITCH);
 
ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, msg_setup-hdr);
if (ret) {
@@ -90,6 +94,7 @@ void lcd_ctrl_init(void *lcdbase)
 
w = msg_setup-physical_w_h.body.resp.width;
h = msg_setup-physical_w_h.body.resp.height;
+   bcm2835_pitch = msg_setup-pitch.body.resp.pitch;
 
debug(bcm2835: Final resolution is %d x %d\n, w, h);
 
@@ -102,4 +107,6 @@ void lcd_ctrl_init(void *lcdbase)
 
 void lcd_enable(void)
 {
+   if (bcm2835_pitch)
+   lcd_line_length = bcm2835_pitch;
 }
-- 
1.8.3.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/2] video: bcm2835: respect the pitch value

2013-10-24 Thread Stephen Warren
On 10/24/2013 07:00 PM, Andre Heider wrote:
 Depending on the firmware's video options [1] the active SDTV or
 HDTV mode can yield a framebuffer with noncontiguous horizontal lines,
 giving a messed up display, for both, u-boot and the loaded kernel.
 
 Fix this by setting lcd_line_length to the pitch value of the configured
 framebuffer.

This sounds like the right concept.

 diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c

  void lcd_enable(void)
  {
 + if (bcm2835_pitch)

Why make this conditional? Does the firmware sometimes not return the
correct pitch and/or does lcd_enable() get called before lcd_ctrl_init()
so the global hasn't been set up? Either of those seem like nasty bugs
that should be fixed...

 + lcd_line_length = bcm2835_pitch;

Why set lcd_line_length at a different time than the mailbox message is
executed? Can't lcd_line_length be set at the end of lcd_ctrl_init(),
thus avoiding the global variable bcm2835_pitch?

  }

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot