[PATCH] video: Get the default mode from the right database

[PATCH] video: Get the default mode from the right database

[Jordan Crouse]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

fb_find_mode() is behaving in an non-intuitive way.  When I specify my
own video mode database, and no default mode, I would have expected it
to assume the first mode in my database as the default mode.  Instead, it
uses the built in database:

> if (!db) {
>     db = modedb;
>     dbsize = ARRAY_SIZE(modedb);
> }
> if (!default_mode)
>     default_mode = &modedb[DEFAULT_MODEDB_INDEX];

Personally, I think this is incorrect - if an alternate database is
specified, it should be always using that.  Patch is attached.

Regards,
Jordan

-- 
Jordan Crouse
Senior Linux Engineer
Advanced Micro Devices, Inc.
<www.amd.com/embeddedprocessors>

[-- Attachment #2: modedb-fix.patch --]
[-- Type: text/plain, Size: 763 bytes --]

[PATCH] video: Get the default mode from the right database

From: Jordan Crouse <jordan.crouse@amd.com>

If no default mode is specified, it should be grabbed from the supplied
database, not the default one.  

Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
---

 drivers/video/modedb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
index d126790..e068f52 100644
--- a/drivers/video/modedb.c
+++ b/drivers/video/modedb.c
@@ -506,7 +506,7 @@ int fb_find_mode(struct fb_var_screeninf
 	dbsize = ARRAY_SIZE(modedb);
     }
     if (!default_mode)
-	default_mode = &modedb[DEFAULT_MODEDB_INDEX];
+	default_mode = &db[DEFAULT_MODEDB_INDEX];
     if (!default_bpp)
 	default_bpp = 8;
 

[-- Attachment #3: Type: text/plain, Size: 133 bytes --]

_______________________________________________
Devel mailing list
Devel@laptop.org
http://mailman.laptop.org/mailman/listinfo/devel

Re: [PATCH] video: Get the default mode from the right database

[Geert Uytterhoeven]

On Mon, 2 Oct 2006, Jordan Crouse wrote:
> fb_find_mode() is behaving in an non-intuitive way.  When I specify my
> own video mode database, and no default mode, I would have expected it
> to assume the first mode in my database as the default mode.  Instead, it
> uses the built in database:
> 
> > if (!db) {
> >     db = modedb;
> >     dbsize = ARRAY_SIZE(modedb);
> > }
> > if (!default_mode)
> >     default_mode = &modedb[DEFAULT_MODEDB_INDEX];
> 
> Personally, I think this is incorrect - if an alternate database is
> specified, it should be always using that.  Patch is attached.

> --- a/drivers/video/modedb.c
> +++ b/drivers/video/modedb.c
> @@ -506,7 +506,7 @@ int fb_find_mode(struct fb_var_screeninf
>  	dbsize = ARRAY_SIZE(modedb);
>      }
>      if (!default_mode)
> -	default_mode = &modedb[DEFAULT_MODEDB_INDEX];
> +	default_mode = &db[DEFAULT_MODEDB_INDEX];
>      if (!default_bpp)
>  	default_bpp = 8;

Although currently DEFAULT_MODEDB_INDEX is defined to be 0, perhaps we need a
more rigorous check now it may apply to the custom video mode database?
Probably you always want the first mode of your custom video mode database to
be the default?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: video: Get the default mode from the right database

[Jordan Crouse]

[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]

On 03/10/06 09:04 +0200, Geert Uytterhoeven wrote:
> >      if (!default_mode)
> > -	default_mode = &modedb[DEFAULT_MODEDB_INDEX];
> > +	default_mode = &db[DEFAULT_MODEDB_INDEX];
> >      if (!default_bpp)
> >  	default_bpp = 8;
> 
> Although currently DEFAULT_MODEDB_INDEX is defined to be 0, perhaps we need a
> more rigorous check now it may apply to the custom video mode database?
> Probably you always want the first mode of your custom video mode database to
> be the default?

Indeed.  I'm not sure how many people out there actually change
DEFAULT_MODEDB_INDEX to be non zero, but can't think of a reason why the
default shouldn't just always use the first index in the database.  

At least, thats the way I thought fb_find_mode() worked before I looked into 
the internals. Still, there might be some people attached to 
DEFAULT_MODEDB_INDEX, so I've attached a new patch that should make everybody 
happy.

Jordan

-- 
Jordan Crouse
Senior Linux Engineer
Advanced Micro Devices, Inc.
<www.amd.com/embeddedprocessors>

[-- Attachment #2: modedb-fix.patch --]
[-- Type: text/plain, Size: 815 bytes --]

[PATCH] video: Get the default mode from the right database

From: Jordan Crouse <jordan.crouse@amd.com>

If no default mode is specified, it should be grabbed from the supplied
database, not the default one.  

Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
---

 drivers/video/modedb.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
index d126790..4c04413 100644
--- a/drivers/video/modedb.c
+++ b/drivers/video/modedb.c
@@ -505,8 +505,11 @@ int fb_find_mode(struct fb_var_screeninf
 	db = modedb;
 	dbsize = ARRAY_SIZE(modedb);
     }
-    if (!default_mode)
+    if (!default_mode && db != modedb)
+	default_mode = &db[0];
+    else
 	default_mode = &modedb[DEFAULT_MODEDB_INDEX];
+
     if (!default_bpp)
 	default_bpp = 8;
 

[-- Attachment #3: Type: text/plain, Size: 133 bytes --]

_______________________________________________
Devel mailing list
Devel@laptop.org
http://mailman.laptop.org/mailman/listinfo/devel