diff mbox

[U-Boot,2/2] video: bcm2835: fix various output modes

Message ID 1382473641-20614-2-git-send-email-a.heider@gmail.com
State Rejected
Delegated to: Anatolij Gustschin
Headers show

Commit Message

Andre Heider Oct. 22, 2013, 8:27 p.m. UTC
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.

To always archive the required contiguousness for the used 16bpp, round
the framebuffer width down so its aligned to a width of 16.

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

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 drivers/video/bcm2835.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stephen Warren Oct. 23, 2013, 4:57 p.m. UTC | #1
On 10/22/2013 09:27 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.
> 
> To always archive the required contiguousness for the used 16bpp, round
> the framebuffer width down so its aligned to a width of 16.

This doesn't sound like the correct approach. By doing this, either the
SET_PHYSICAL_W_H request will fail since the requested mode doesn't
match the connected display device, or perhaps it'll work, but end up
with a frame-buffer that's a different resolution than the video signal,
so the HW will scale the image slightly, which will reduce quality.

Instead, can't you obtain the buffer width and stride separately, and
then configure the LCD core based on both those values, rather than
assuming they're the same?
Andre Heider Oct. 23, 2013, 8:06 p.m. UTC | #2
On Wed, Oct 23, 2013 at 05:57:53PM +0100, Stephen Warren wrote:
> On 10/22/2013 09:27 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.
> > 
> > To always archive the required contiguousness for the used 16bpp, round
> > the framebuffer width down so its aligned to a width of 16.
> 
> This doesn't sound like the correct approach. By doing this, either the
> SET_PHYSICAL_W_H request will fail since the requested mode doesn't
> match the connected display device, or perhaps it'll work, but end up
> with a frame-buffer that's a different resolution than the video signal,
> so the HW will scale the image slightly, which will reduce quality.

SET_PHYSICAL_W_H works in any case, but yeah, it does get "funky"
resolutions in some cases.

The thing is, the firmware is stupid to begin with (duh). In my case, I do
have both outputs connected. For the analog one, I have to set sdtv_*
and overscan_* config settings.
When the HDMI connection is active, the analog output is inactive, but
yet the firmware applies the sdtv_* and overscan_* settings to the HDMI
output too, so the clean 1:1 resolution is already gone...

> Instead, can't you obtain the buffer width and stride separately, and
> then configure the LCD core based on both those values, rather than
> assuming they're the same?

What I did try is to get the overscan values and add those to the
requested resolution (that's where patch 1 comes from). That gives saner
resolutions, but some are still broken for me.

I agree that getting the pitch value would be the right thing to do, I
do some digging to find a spot where to shove that into.

Thanks,
Andre
Andre Heider Oct. 24, 2013, 6:14 p.m. UTC | #3
Hi Stephen,

On Wed, Oct 23, 2013 at 05:57:53PM +0100, Stephen Warren wrote:
> Instead, can't you obtain the buffer width and stride separately, and
> then configure the LCD core based on both those values, rather than
> assuming they're the same?

I just sent a v2 patch to do exactly that.

Regarding overscan, I wonder why you zero out the initial overscan
settings.

If it was just to gain a contiguous frame buffer, can we keep the
overscan settings now (assuming my v2 patch is accepted)?
That way the framebuffer doesn't get cut off on the sides for old
analog SDTV display devices.
For HDTV, that means there's a border - but only if the user set the
overscan_* values.

Thanks,
Andre
Stephen Warren Oct. 24, 2013, 10:08 p.m. UTC | #4
On 10/24/2013 07:14 PM, Andre Heider wrote:
> Hi Stephen,
> 
> On Wed, Oct 23, 2013 at 05:57:53PM +0100, Stephen Warren wrote:
>> Instead, can't you obtain the buffer width and stride separately, and
>> then configure the LCD core based on both those values, rather than
>> assuming they're the same?
> 
> I just sent a v2 patch to do exactly that.
> 
> Regarding overscan, I wonder why you zero out the initial overscan
> settings.

I think it was because the firmware incorrectly decided by default that
my HDMI device needed overscan, when in fact it can display the entire
signal it's sent. If there's a way to control that in the config file,
then perhaps that might be possible instead, or perhaps I should file a
bug against the firmware, assuming I'm remembering correctly...
diff mbox

Patch

diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c
index 58a6163..1404340 100644
--- a/drivers/video/bcm2835.c
+++ b/drivers/video/bcm2835.c
@@ -52,7 +52,7 @@  void lcd_ctrl_init(void *lcdbase)
 		return;
 	}
 
-	w = msg_query->physical_w_h.body.resp.width;
+	w = ROUND(msg_query->physical_w_h.body.resp.width, 16);
 	h = msg_query->physical_w_h.body.resp.height;
 
 	debug("bcm2835: Setting up display for %d x %d\n", w, h);