[PATCH] ARM: Parse ATAG_VIDEOLFB to screen_info

[PATCH] ARM: Parse ATAG_VIDEOLFB to screen_info

[Tomi Valkeinen]

Parse ATAG_VIDEOLFB received from bootloader to screen_info struct, so that video
drivers can use the information.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@nokia.com>
---
 arch/arm/kernel/setup.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 1f1eecc..11dafe5 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -576,6 +576,31 @@ static int __init parse_tag_videotext(const struct tag *tag)
 }
 
 __tagtable(ATAG_VIDEOTEXT, parse_tag_videotext);
+
+static int __init parse_tag_videolfb(const struct tag *tag)
+{
+	screen_info.lfb_width = tag->u.videolfb.lfb_width;
+	screen_info.lfb_height = tag->u.videolfb.lfb_height;
+	screen_info.lfb_depth = tag->u.videolfb.lfb_depth;
+	screen_info.lfb_linelength = tag->u.videolfb.lfb_linelength;
+	screen_info.lfb_base = tag->u.videolfb.lfb_base;
+	screen_info.lfb_size = tag->u.videolfb.lfb_size;
+
+	screen_info.red_size = tag->u.videolfb.red_size;
+	screen_info.red_pos = tag->u.videolfb.red_pos;
+	screen_info.green_size = tag->u.videolfb.green_size;
+	screen_info.green_pos = tag->u.videolfb.green_pos;
+	screen_info.blue_size = tag->u.videolfb.blue_size;
+	screen_info.blue_pos = tag->u.videolfb.blue_pos;
+	screen_info.rsvd_size = tag->u.videolfb.rsvd_size;
+	screen_info.rsvd_pos = tag->u.videolfb.rsvd_pos;
+
+	screen_info.orig_video_isVGA = VIDEO_TYPE_VLFB;
+
+	return 0;
+}
+
+__tagtable(ATAG_VIDEOLFB, parse_tag_videolfb);
 #endif
 
 static int __init parse_tag_ramdisk(const struct tag *tag)
-- 
1.6.0.3

Re: [PATCH] ARM: Parse ATAG_VIDEOLFB to screen_info

[Russell King - ARM Linux]

On Fri, Nov 21, 2008 at 12:25:36PM +0200, Tomi Valkeinen wrote:
> Parse ATAG_VIDEOLFB received from bootloader to screen_info struct,
> so that video drivers can use the information.

Boot loaders tend to be absolutely diabolical about passing correct
information to the kernel, so I really don't want to see such platform
specific stuff being passed.  Please put it in the board specific file
instead, just like everyone else does.

Re: [PATCH] ARM: Parse ATAG_VIDEOLFB to screen_info

[Tomi Valkeinen]

Hi,

On Mon, 2008-11-24 at 12:12 +0000, ext Russell King - ARM Linux wrote:
> On Fri, Nov 21, 2008 at 12:25:36PM +0200, Tomi Valkeinen wrote:
> > Parse ATAG_VIDEOLFB received from bootloader to screen_info struct,
> > so that video drivers can use the information.
> 
> Boot loaders tend to be absolutely diabolical about passing correct
> information to the kernel, so I really don't want to see such platform
> specific stuff being passed.  Please put it in the board specific file
> instead, just like everyone else does.

I though a general file would be better, because:

a) the tag contains only generic stuff and it is parsed directly to
screen_info, which is also generic

b) ATAG_VIDEOTEXT, which is very similar to ATAG_VIDEOLFB, is parsed in
setup.c also.

c) it is not used anywhere currently

So I don't see anything platform or board specific there. Or do you mean
that there are broken bootloaders out there and this would mess up
screen_info?

 Tomi

Re: [PATCH] ARM: Parse ATAG_VIDEOLFB to screen_info

[Russell King - ARM Linux]

On Tue, Nov 25, 2008 at 10:17:43AM +0200, Tomi Valkeinen wrote:
> Hi,
> 
> On Mon, 2008-11-24 at 12:12 +0000, ext Russell King - ARM Linux wrote:
> > On Fri, Nov 21, 2008 at 12:25:36PM +0200, Tomi Valkeinen wrote:
> > > Parse ATAG_VIDEOLFB received from bootloader to screen_info struct,
> > > so that video drivers can use the information.
> > 
> > Boot loaders tend to be absolutely diabolical about passing correct
> > information to the kernel, so I really don't want to see such platform
> > specific stuff being passed.  Please put it in the board specific file
> > instead, just like everyone else does.
> 
> I though a general file would be better, because:
> 
> a) the tag contains only generic stuff and it is parsed directly to
> screen_info, which is also generic
> 
> b) ATAG_VIDEOTEXT, which is very similar to ATAG_VIDEOLFB, is parsed in
> setup.c also.
> 
> c) it is not used anywhere currently

I was meaning "don't do it this way at all" - instead, do it like every
other platform does, and keep the framebuffer settings in the platform
specific files.  Don't encode them into the boot loader and pass them
to the kernel.

Boot loaders are fragile and buggy.  No one likes fixing them.  It takes
_years_ to get problems with them fixed.  It may seem like it's a good
idea to pass lots of platform specific data from the boot loader to the
kernel, but it's just not worth the hastle of getting people to fix that
data when it's not 100% correct.

We've seen this many times.  We've seen it with simple things like passing
the machine number to the kernel.  Just don't go there - it's a pain in
the back side and causes years of grief.