[PATCH] Radeon driver fixes

[Michel Dänzer]

On Fri, 2004-11-12 at 16:03 +1100, Benjamin Herrenschmidt wrote: 
> > 
> > > @@ -223,6 +226,9 @@
> > >  #endif
> > >      { OPTION_SHOWCACHE,      "ShowCache",        OPTV_BOOLEAN, {0}, FALSE },
> > >      { OPTION_DYNAMIC_CLOCKS, "DynamicClocks",    OPTV_BOOLEAN, {0}, FALSE },
> > > +    { OPTION_NO_VGA,         "NoVGA",            OPTV_BOOLEAN, {0}, FALSE },
> > 
> > An option name that starts with "No" (or any other negative term, for
> > that matter) is a no no IMHO. :) The option parser handles this as well
> > as a number of other prefixes and other things.
> 
> Blame my inexperience with X option parser... you mean that if I call
> the option "SaveRestoreVGA" instead with a default to TRUE, and put
> Option "NoSaveRestorVGA" in my config file, the option parser will deal
> with it ?

Yes, as well as with

	Option	"SaveRestoreVGA" "off"
	Option	"SaveRestoreVGA" "false"
	...


> > > @@ -4237,6 +4331,24 @@
> > >      memcpy(info->Options, RADEONOptions, sizeof(RADEONOptions));
> > >      xf86ProcessOptions(pScrn->scrnIndex, pScrn->options, info->Options);
> > >  
> > > +    info->NoVGA = TRUE;
> > > +    if (!xf86ReturnOptValBool(info->Options, OPTION_NO_VGA, FALSE)) {
> > 
> > Please consider using xf86GetOptValBool() instead of
> > xf86ReturnOptValBool() and setting the second argument to xf86DrvMsg()
> > according to whether something is the default or set by the
> > configuration or not.
> 
> Hrm... care to rephrase ? Not sure I got what you meant about the
> default ... 

I mean something along the lines of

    from = xf86GetOptValBool(info->Options, OPTION_ACCESS_VGA,
                             &info->AccessVGA)
         ? X_CONFIG : X_INFO;

    xf86DrvMsg(pScrn->scrnIndex, from, "VGA access %sabled\n",
               info->AccessVGA);


-- 
Earthling Michel Dänzer      |     Debian (powerpc), X and DRI developer
Libre software enthusiast    |   http://svcs.affero.net/rm.php?r=daenzer