Menu
▾
▴
linux-fbdev-devel
|
From: Antonino D. <ad...@po...> - 2002-05-03 09:51:26
|
Hi,
I have a few observations on fbgen and fbcon-accel.
1. fbcon_accel_clear_margins may not work correctly with fbgen since
fbcon_accel will use the xoffset and yoffset values from info->var.
void fbcon_accel_clear_margins(struct vc_data *vc, struct display
*p,
int bottom_only)
{
<<<snip>>>
if (bh) {
region.dx = info->var.xoffset;
region.dy = info->var.yoffset + bs;
region.width = rs;
region.height = bh;
info->fbops->fb_fillrect(info, ®ion);
}
}
However fbgen_pan_display updates the xoffset and yoffset in
fb_display[con].var. So margins don't get cleared if the driver supports
y-panning or y-wrapping.
int fbgen_pan_display(struct fb_var_screeninfo *var, int con,
struct fb_info *info)
{
<<< snip >>>
if (con == info->currcon) {
if (fbhw->pan_display) {
if ((err = fbhw->pan_display(var, info2)))
return err;
} else
return -EINVAL;
}
fb_display[con].var.xoffset = var->xoffset;
fb_display[con].var.yoffset = var->yoffset;
<<< snip >>>
}
2. Also, fbgen_switch basically just do an fbgen_do_set_var()
(decode_var(), followed by set_par()). This is okay most times, but
it's probably better if fbgen_switch also does an encode_fix() since
fbcon's drawing functions also rely on fix->line_length.
If an fb_fix_screeninfo is not updated, display corruption occurs when
switching to another display with a different pixelformat.
Tony
|
|
From: James S. <jsi...@tr...> - 2002-05-03 22:15:21
|
> I have a few observations on fbgen and fbcon-accel.
Don't mix fbgen with fbcon-accel. The new gen_* stuff in fbgen.c is meant
to replace the old fbgen_* stuff. That is why the below doesn't work.
> 1. fbcon_accel_clear_margins may not work correctly with fbgen since
> fbcon_accel will use the xoffset and yoffset values from info->var.
>
> void fbcon_accel_clear_margins(struct vc_data *vc, struct display
> *p,
> int bottom_only)
> {
> <<<snip>>>
>
> if (bh) {
> region.dx = info->var.xoffset;
> region.dy = info->var.yoffset + bs;
> region.width = rs;
> region.height = bh;
> info->fbops->fb_fillrect(info, ®ion);
> }
> }
>
> However fbgen_pan_display updates the xoffset and yoffset in
> fb_display[con].var. So margins don't get cleared if the driver supports
> y-panning or y-wrapping.
>
> int fbgen_pan_display(struct fb_var_screeninfo *var, int con,
> struct fb_info *info)
> {
> <<< snip >>>
>
> if (con == info->currcon) {
> if (fbhw->pan_display) {
> if ((err = fbhw->pan_display(var, info2)))
> return err;
> } else
> return -EINVAL;
> }
> fb_display[con].var.xoffset = var->xoffset;
> fb_display[con].var.yoffset = var->yoffset;
>
> <<< snip >>>
> }
> 2. Also, fbgen_switch basically just do an fbgen_do_set_var()
> (decode_var(), followed by set_par()). This is okay most times, but
> it's probably better if fbgen_switch also does an encode_fix() since
> fbcon's drawing functions also rely on fix->line_length.
Most likely that is also broken. I haven't thought about it since I plan
to make all the old fbgen_* functions go away.
> If an fb_fix_screeninfo is not updated, display corruption occurs when
> switching to another display with a different pixelformat.
Correct. That is why I require info->fix to be updated when set_par is
called.
|
|
From: Antonino D. <ad...@po...> - 2002-05-04 00:39:40
|
On Sat, 2002-05-04 at 05:47, James Simmons wrote: > > > I have a few observations on fbgen and fbcon-accel. > > Don't mix fbgen with fbcon-accel. The new gen_* stuff in fbgen.c is meant > to replace the old fbgen_* stuff. That is why the below doesn't work. > I see now :) I'll incorport all gen_* functions as you along adding them and let you know. Thanks! Tony |
|
From: Antonino D. <ad...@po...> - 2002-05-04 15:49:29
|
On Sat, 2002-05-04 at 05:47, James Simmons wrote: > > > I have a few observations on fbgen and fbcon-accel. > > Don't mix fbgen with fbcon-accel. The new gen_* stuff in fbgen.c is meant > to replace the old fbgen_* stuff. That is why the below doesn't work. > Okay, I've succeeded in rewriting the i810/i815 driver to use the gen_* stuff instead of fbgen_*. As far as I can tell everything works :) -- y-panning, accel, etc -- although gen_update_var() may not work properly. I'm still getting incorrect cursor colors in 8 bpp, but that's probably my fault. And you're right, it's actually easier to write the driver using the gen_* stuff. > > > 2. Also, fbgen_switch basically just do an fbgen_do_set_var() > > (decode_var(), followed by set_par()). This is okay most times, but > > it's probably better if fbgen_switch also does an encode_fix() since > > fbcon's drawing functions also rely on fix->line_length. > > Most likely that is also broken. I haven't thought about it since I plan > to make all the old fbgen_* functions go away. > fb_gen_switch may be broken, but I think gen_switch works just okay as long as info->fix is updated in set_par(). > > If an fb_fix_screeninfo is not updated, display corruption occurs when > > switching to another display with a different pixelformat. > > Correct. That is why I require info->fix to be updated when set_par is > called. > Right. The i810fb patch is at http://prdownloads.sourceforge.net/i810fb/linux-2.5.13-i810fb.tar.bz2. Tony --- fbgen.c.orig Sat May 4 14:35:32 2002 +++ fbgen.c Sat May 4 15:02:37 2002 @@ -514,7 +514,8 @@ if (con == info->currcon) { if (info->fbops->fb_pan_display) { - if ((err = info->fbops->fb_pan_display(&info->var, con, info))) + /* Tony: offsets are still in disp->var, not info->var */ + if ((err = info->fbops->fb_pan_display(&fb_display[con].var, con, info))) return err; } } |
|
From: Antonino D. <ad...@po...> - 2002-05-04 18:02:43
|
On Sat, 2002-05-04 at 05:47, James Simmons wrote:
>
> > I have a few observations on fbgen and fbcon-accel.
>
One more thing I've noticed with gen_set_var. Basically, gen_set_var
will proceed if it satisfies 2 conditions -- during initialization (con
< 0) and if the new var is different from the old var.
The above is fine if everything is done in the console. However,
problems may arise if an app that touches the graphics hardware (ie X)
is launched. From the point of view of fbcon, the hardware state hasn't
changed (compare of newvar with oldvar is false) when display is
switched back to console. And if that app did not fully restore the
hardware state, we will be left with a corrupted display.
So, it's probably better if set_par() and pan_display() are allowed to
proceed unconditionally within gen_set_var. It might take a few more
milliseconds to switch consoles each time, but we are assured that the
hardware state is always coherent with the current var.
What do you think?
Tony
--- fbgen.c.orig Sun May 5 01:38:57 2002
+++ fbgen.c Sun May 5 01:38:55 2002
@@ -172,14 +172,8 @@
if ((var->activate & FB_ACTIVATE_MASK) == FB_ACTIVATE_NOW) {
info->var = *var;
-
- if (con == info->currcon) {
- if (info->fbops->fb_set_par)
- info->fbops->fb_set_par(info);
-
- if (info->fbops->fb_pan_display)
- info->fbops->fb_pan_display(&info->var, con, info);
+ if (con == info->currcon) {
gen_set_disp(con, info);
fb_set_cmap(&info->cmap, 1, info);
}
@@ -187,6 +181,13 @@
if (info->changevar)
info->changevar(con);
}
+ }
+ if ((var->activate & FB_ACTIVATE_MASK) == FB_ACTIVATE_NOW &&
+ con == info->currcon) {
+ if (info->fbops->fb_set_par)
+ info->fbops->fb_set_par(info);
+ if (info->fbops->fb_pan_display)
+ info->fbops->fb_pan_display(&info->var, con, info);
}
return 0;
}
|
|
From: James S. <jsi...@tr...> - 2002-05-31 20:45:11
|
> The above is fine if everything is done in the console. However, > problems may arise if an app that touches the graphics hardware (ie X) > is launched. From the point of view of fbcon, the hardware state hasn't > changed (compare of newvar with oldvar is false) when display is > switched back to console. And if that app did not fully restore the > hardware state, we will be left with a corrupted display. I'm aware of this flaw. I plan to fix this when the rewrite of the console system starts. The console system should handle restore ths video mode. |
|
From: Michel <mi...@da...> - 2002-05-06 22:25:22
|
On Sat, 2002-05-04 at 20:01, Antonino Daplas wrote:=20 > On Sat, 2002-05-04 at 05:47, James Simmons wrote: > >=20 > > > I have a few observations on fbgen and fbcon-accel. >=20 > One more thing I've noticed with gen_set_var. Basically, gen_set_var > will proceed if it satisfies 2 conditions -- during initialization (con > < 0) and if the new var is different from the old var. =20 >=20 > The above is fine if everything is done in the console. However, > problems may arise if an app that touches the graphics hardware (ie X) > is launched. From the point of view of fbcon, the hardware state hasn't > changed (compare of newvar with oldvar is false) when display is > switched back to console. And if that app did not fully restore the > hardware state, we will be left with a corrupted display. >=20 > So, it's probably better if set_par() and pan_display() are allowed to > proceed unconditionally within gen_set_var. It might take a few more > milliseconds to switch consoles each time, but we are assured that the > hardware state is always coherent with the current var. >=20 > What do you think? I think this is giving away an advantage. The X server is a bad example as it can use the framebuffer device fine. If it's really a problem, maybe we could figure out a way to detect when it's safe to optimize stuff away or as a last resort make it an option? --=20 Earthling Michel D=E4nzer (MrCooper)/ Debian GNU/Linux (powerpc) developer XFree86 and DRI project member / CS student, Free Software enthusiast |
|
From: Antonino D. <ad...@po...> - 2002-05-07 01:35:27
|
On Tue, 2002-05-07 at 06:24, Michel D=E4nzer wrote: > On Sat, 2002-05-04 at 20:01, Antonino Daplas wrote:=20 > > On Sat, 2002-05-04 at 05:47, James Simmons wrote: > > >=20 > > > > I have a few observations on fbgen and fbcon-accel. > >=20 > > One more thing I've noticed with gen_set_var. Basically, gen_set_var > > will proceed if it satisfies 2 conditions -- during initialization (con > > < 0) and if the new var is different from the old var. =20 > >=20 > > The above is fine if everything is done in the console. However, > > problems may arise if an app that touches the graphics hardware (ie X) > > is launched. From the point of view of fbcon, the hardware state hasn'= t > > changed (compare of newvar with oldvar is false) when display is > > switched back to console. And if that app did not fully restore the > > hardware state, we will be left with a corrupted display. > >=20 > > So, it's probably better if set_par() and pan_display() are allowed to > > proceed unconditionally within gen_set_var. It might take a few more > > milliseconds to switch consoles each time, but we are assured that the > > hardware state is always coherent with the current var. > >=20 > > What do you think? >=20 > I think this is giving away an advantage. The X server is a bad example > as it can use the framebuffer device fine. I was talking of X running with it's own accelerated drivers. I have checked most of the X accelerated drivers, and most will just attempt to restore the VGA registers and some when switching to the console. I think this is not just enough, since fb drivers require more than that.=20 Simiarly, I have also looked at some of the fbdev drivers that are not using the gen_* interface (nvidia, ati, matrox), and they will also unconditially set the hardware during switches/set_var. The fbgen_switch() function also calls fbhw->set_par() within do_set_var(). >=20 > If it's really a problem, maybe we could figure out a way to detect when > it's safe to optimize stuff away or as a last resort make it an option? >=20 I think if the gen_* interface is to be adopted, it will become a problem. Detection is the best solution, but right now X and DRI do not know that fb even exist so we can't get X to detect fb unless we persuade the X people to do that. I have tried X detection before by checking the previous console number. If the previous number is not a valid console, we can presume that a non-console app used that. But this is not clean and there are too many conditions where this check will fail. But then, I really don't understand the underlying console interface, so an easier and more effective way may exist that I don't know about. Tony |
|
From: Geert U. <ge...@li...> - 2002-05-07 08:00:52
|
On 7 May 2002, Antonino Daplas wrote:
> On Tue, 2002-05-07 at 06:24, Michel D=E4nzer wrote:
> > On Sat, 2002-05-04 at 20:01, Antonino Daplas wrote:=20
> > > On Sat, 2002-05-04 at 05:47, James Simmons wrote:
> > > >=20
> > > > > I have a few observations on fbgen and fbcon-accel.
> > >=20
> > > One more thing I've noticed with gen_set_var. Basically, gen_set_v=
ar
> > > will proceed if it satisfies 2 conditions -- during initialization =
(con
> > > < 0) and if the new var is different from the old var. =20
> > >=20
> > > The above is fine if everything is done in the console. However,
> > > problems may arise if an app that touches the graphics hardware (ie=
X)
> > > is launched. From the point of view of fbcon, the hardware state h=
asn't
> > > changed (compare of newvar with oldvar is false) when display is
> > > switched back to console. And if that app did not fully restore th=
e
> > > hardware state, we will be left with a corrupted display.
> > >=20
> > > So, it's probably better if set_par() and pan_display() are allowed=
to
> > > proceed unconditionally within gen_set_var. It might take a few mo=
re
> > > milliseconds to switch consoles each time, but we are assured that =
the
> > > hardware state is always coherent with the current var.
Which can be visible, especially if you reprogram the PLL...
> > > What do you think?
> >=20
> > I think this is giving away an advantage. The X server is a bad examp=
le
> > as it can use the framebuffer device fine.
Indeed.
> I was talking of X running with it's own accelerated drivers. I have
> checked most of the X accelerated drivers, and most will just attempt t=
o
> restore the VGA registers and some when switching to the console. I
> think this is not just enough, since fb drivers require more than that.=
=20
If you run an application that's not fbdev aware, behavior has always bee=
n
undefined.
> Simiarly, I have also looked at some of the fbdev drivers that are not
> using the gen_* interface (nvidia, ati, matrox), and they will also
> unconditially set the hardware during switches/set_var. The
> fbgen_switch() function also calls fbhw->set_par() within do_set_var().
And set_par() could do some optimizations based on a shadow map of the re=
gister
contents, also to avoid artefacts when switching to a different VC with t=
he
same video timings. Like:
write_reg(reg, val)
{
if (shadow[reg] !=3D val) {
shadow[reg] =3D val;
hardware[reg] =3D val;
}
}
> > If it's really a problem, maybe we could figure out a way to detect w=
hen
> > it's safe to optimize stuff away or as a last resort make it an optio=
n?
> >=20
> I think if the gen_* interface is to be adopted, it will become a
> problem. Detection is the best solution, but right now X and DRI do not
> know that fb even exist so we can't get X to detect fb unless we
> persuade the X people to do that. I have tried X detection before by
> checking the previous console number. If the previous number is not a
> valid console, we can presume that a non-console app used that. But
> this is not clean and there are too many conditions where this check
> will fail. But then, I really don't understand the underlying console
> interface, so an easier and more effective way may exist that I don't
> know about.
The problem is that most drivers in XFree86 don't _want_ to be fbdev comp=
liant.
The solution is to convince the hardcode anti-fbdev XFree86 guys to use t=
he
fbdev if it's present. Fbdev is part of the kernel API. Circumventing the=
API
is bad behavior.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m6=
8k.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
|
|
From: Antonino D. <ad...@po...> - 2002-05-07 13:26:53
|
On Tue, 2002-05-07 at 16:00, Geert Uytterhoeven wrote:
> > I was talking of X running with it's own accelerated drivers. I have
> > checked most of the X accelerated drivers, and most will just attempt to
> > restore the VGA registers and some when switching to the console. I
> > think this is not just enough, since fb drivers require more than that.
>
> If you run an application that's not fbdev aware, behavior has always been
> undefined.
>
In the same token, then the kernel should be made as robust as
possible. I would rather have a user app crash than have it lock-up the
hardware or corrupt the display.
> > Simiarly, I have also looked at some of the fbdev drivers that are not
> > using the gen_* interface (nvidia, ati, matrox), and they will also
> > unconditially set the hardware during switches/set_var. The
> > fbgen_switch() function also calls fbhw->set_par() within do_set_var().
>
> And set_par() could do some optimizations based on a shadow map of the register
> contents, also to avoid artefacts when switching to a different VC with the
> same video timings. Like:
>
> write_reg(reg, val)
> {
> if (shadow[reg] != val) {
> shadow[reg] = val;
> hardware[reg] = val;
> }
> }
>
Actually, What I'm trying to point out is that gen_set_var() is not even
given a chance to call set_par(). If the generic interface is to be
adopted, it has to satisfy the requirements of most, if not all cards.
My current thinking is to let gen_set_var() call set_par()
unconditionally and at the same time pass a parameter or set a flag.
This flag or parameter can be something like, "yes, you really need to
set the hardware" or "no, var has not changed but you have the option to
set the hardware or not." Something to this effect:
if (memcmp(oldvar, newvar, sizeof(struct fb_var_screeninfo)))
info->fbops->fb_set_par(info, 1);
else
info->fbops->fb_set_par(info, 0);
> > I think if the gen_* interface is to be adopted, it will become a
> > problem. Detection is the best solution, but right now X and DRI do not
> > know that fb even exist so we can't get X to detect fb unless we
> > persuade the X people to do that. I have tried X detection before by
> > checking the previous console number. If the previous number is not a
> > valid console, we can presume that a non-console app used that. But
> > this is not clean and there are too many conditions where this check
> > will fail. But then, I really don't understand the underlying console
> > interface, so an easier and more effective way may exist that I don't
> > know about.
>
> The problem is that most drivers in XFree86 don't _want_ to be fbdev compliant.
> The solution is to convince the hardcode anti-fbdev XFree86 guys to use the
I can't agree with you more. When I was writing the i810/i815 fb
driver, I also modified the XFree86 i810 driver so it is fbdev-aware.
The "fbdev-awareness" is not a requirement, but it solves a lot of
problems and lessened code bloat.
But as you've said, they're hardcore :)
> fbdev if it's present. Fbdev is part of the kernel API. Circumventing the API
> is bad behavior.
>
Which is why I abandoned that idea.
Tony
|
|
From: Michel <mi...@da...> - 2002-05-07 22:51:09
|
On Tue, 2002-05-07 at 10:00, Geert Uytterhoeven wrote:=20 > > > If it's really a problem, maybe we could figure out a way to detect w= hen > > > it's safe to optimize stuff away or as a last resort make it an optio= n? > > >=20 > > I think if the gen_* interface is to be adopted, it will become a > > problem. Detection is the best solution, but right now X and DRI do not > > know that fb even exist so we can't get X to detect fb unless we > > persuade the X people to do that. I have tried X detection before by > > checking the previous console number. If the previous number is not a > > valid console, we can presume that a non-console app used that. But > > this is not clean and there are too many conditions where this check > > will fail. But then, I really don't understand the underlying console > > interface, so an easier and more effective way may exist that I don't > > know about. >=20 > The problem is that most drivers in XFree86 don't _want_ to be fbdev comp= liant. > The solution is to convince the hardcode anti-fbdev XFree86 guys to use t= he > fbdev if it's present. Fbdev is part of the kernel API. Circumventing the= API > is bad behavior. The opposition seems to be mostly against the fbdev support being spread over the drivers, which is hard to maintain. If we could move it into an common layer, there should be no problem. I do have the basic idea how to do it but I suspect it would require changes to the driver interface so it might have to wait for XFree86 5.x (provided anyone actually tries :). --=20 Earthling Michel D=E4nzer (MrCooper)/ Debian GNU/Linux (powerpc) developer XFree86 and DRI project member / CS student, Free Software enthusiast |
Thanks for helping keep SourceForge clean.
X