Menu
▾
▴
linux-fbdev-devel
|
From: Paul M. <pa...@sa...> - 2002-12-10 22:40:24
|
When I look at atyfb_check_var or aty128fb_check_var, I see that they will alter the contents of *info->par. Isn't this a bad thing? My understanding was that after calling check_var, you don't necessarily call set_par next (particularly if check_var returned an error). Also I notice that atyfb_set_par and aty128fb_set_par don't look at info->var, they simply set the hardware state based on the contents of *info->par. Looking at skeletonfb.c, it seems that this is the wrong behaviour. I had fixed the aty128fb.c driver in the linuxppc-2.5 tree. James, if you let me know whether the current behaviour is wrong or not, I'll fix them and send you the patch. Paul. |
|
From: Benjamin H. <be...@ke...> - 2002-12-10 23:11:13
|
On Tue, 2002-12-10 at 23:40, Paul Mackerras wrote: > When I look at atyfb_check_var or aty128fb_check_var, I see that they > will alter the contents of *info->par. Isn't this a bad thing? My Yes, this wrong, and afaik, it's your original port to 2.5 that did that ;) > understanding was that after calling check_var, you don't necessarily > call set_par next (particularly if check_var returned an error). > Also I notice that atyfb_set_par and aty128fb_set_par don't look at > info->var, they simply set the hardware state based on the contents of > *info->par. Which is wrong too indeed > Looking at skeletonfb.c, it seems that this is the wrong behaviour. I > had fixed the aty128fb.c driver in the linuxppc-2.5 tree. James, if > you let me know whether the current behaviour is wrong or not, I'll > fix them and send you the patch. I _think_ my radeonfb (in linuxppc-2.5) is right in this regard too. Look at the initialization too, iirc, you had some non necessary stuff in there (calling gen_set_disp, gen_set_var is plenty enough). Ben. > Paul. > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to maj...@vg... > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ |
|
From: James S. <jsi...@in...> - 2002-12-11 05:40:37
|
> > When I look at atyfb_check_var or aty128fb_check_var, I see that they > > will alter the contents of *info->par. Isn't this a bad thing? My > > Yes, this wrong, and afaik, it's your original port to 2.5 that did that > ;) Yeap. The idea of check_var is to validate a mode. Note modedb uses just check_var. It is okay to READ the values in your par. You shouldn't alter the values in par. > > understanding was that after calling check_var, you don't necessarily > > call set_par next (particularly if check_var returned an error). Correct. Check_var is always called. Now if you wanted to just test a mode via userland with the FB_ACTIVATE_TEST flag then only fb_check_var is called. If you pass in FB_ACTIVATE_NOW then both fb_check_var and fb_set_par will be called. Fb_set_par actually sets the hardware state. > > Also I notice that atyfb_set_par and aty128fb_set_par don't look at > > info->var, they simply set the hardware state based on the contents of > > *info->par. > > Which is wrong too indeed Actually you can look at info->var. Info->var has been validated so it can be trusted. You don't need to stuff everything into par. You DO need to change info->fix if the hardware state has changed. > > Looking at skeletonfb.c, it seems that this is the wrong behaviour. I > > had fixed the aty128fb.c driver in the linuxppc-2.5 tree. James, if > > you let me know whether the current behaviour is wrong or not, I'll > > fix them and send you the patch. I hope me input helped. BTW docs are on the way. I will work with Steven Luther on this the next couple of days. > I _think_ my radeonfb (in linuxppc-2.5) is right in this regard too. > Look at the initialization too, iirc, you had some non necessary stuff > in there (calling gen_set_disp, gen_set_var is plenty enough). You go the logic down. P.S For the pmu_sleep_notifier can you pass in a specific struct fb_info or do you need to make a list of all of them? |
|
From: Benjamin H. <be...@ke...> - 2002-12-11 09:06:36
|
On Wed, 2002-12-11 at 07:32, James Simmons wrote: > > For the pmu_sleep_notifier can you pass in a specific struct fb_info or > do you need to make a list of all of them? So far, I need to make that list. But sooner or later, those notifiers will go away and I'll use the PCI and/or new driver model PM features. Ben. |
|
From: Geert U. <ge...@li...> - 2002-12-22 12:20:12
|
On Tue, 10 Dec 2002, James Simmons wrote:
> > > When I look at atyfb_check_var or aty128fb_check_var, I see that they
> > > will alter the contents of *info->par. Isn't this a bad thing? My
> >
> > Yes, this wrong, and afaik, it's your original port to 2.5 that did that
> > ;)
>
> Yeap. The idea of check_var is to validate a mode. Note modedb uses just
> check_var. It is okay to READ the values in your par. You shouldn't alter
> the values in par.
Perhaps it makes sense to make the info parameter of fb_check_var() const to
prevent this from happening?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@li...
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
|
|
From: James S. <jsi...@in...> - 2003-01-10 02:32:08
|
> > > Yes, this wrong, and afaik, it's your original port to 2.5 that did that > > > ;) > > > > Yeap. The idea of check_var is to validate a mode. Note modedb uses just > > check_var. It is okay to READ the values in your par. You shouldn't alter > > the values in par. > > Perhaps it makes sense to make the info parameter of fb_check_var() const to > prevent this from happening? Easy fix. I can whip up a quick patch for that. |
Thanks for helping keep SourceForge clean.
X