Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Benjamin Herrenschmidt]

On Sat, 2005-03-12 at 17:29 -0800, Jon Smirl wrote:
> --- Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> > 
> > > When we get merged my thought was to let fbdev catch the vsync IRQ
> > and
> > > then chain into DRM. In that model fbdev becomes the mini-driver.
> > > Right now I don't think there is any other solution than merging
> > > fbdev/DRM that doesn't result in some kind of dead on VT switch
> > > scenario.
> > 
> > A VT switch should stop all asynchronous activity (interrupt, DRM,
> > etc...). X already stops the DRM on switch-out, this could be
> > enforced
> > by the kernel.
> 
> The stopping interrupts on VT switch doesn't really work in a multiuser
> system. That's what burnt me with radeonfb and X. My radeonfb was
> essentially running a different user. X caught a VT switch from the
> first user and stomped on the second.
> 
> fbdev doesn't know about VT switch and shouldn't. It's fbcon that knows
> about it.

Currently, we still need a few hacks to make things work, including that
sort of thing. In fact, the problem at this point is that once the VT
subsystem has lost ownership of the console (because X switch it to
KD_GRAPHICS), we should assume that all video cards, including the ones
not used by fbcon or whatever, have lost ownership.

That sucks, but we don't have a choice until we have proper arbitration.

> Why don't we just make X/DRM/fbdev all play nice with each other, then
> nothing has to be stopped on VT switch.

We do and are working toward that goal, remember ? 

Ben.




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Michel Dänzer]

On Wed, 2005-03-16 at 01:28 +0000, Torgeir Veimo wrote:
> On Sat, 2005-03-12 at 17:13 +0200, Ville Syrjälä wrote:
> > On Sat, Mar 12, 2005 at 02:06:45PM +0000, Torgeir Veimo wrote:
> 
> > > +		/* clear interrupt */
> > > +		//OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_STAT_ACK);
> > 
> > Why is this commented out?
> 
> Well, first of all is it really important to clear the interrupt when
> enabling it? 
> 
> From what I've seen the rage128 acknowledges an interrupt by writing to
> the int_cntl register, while the radeon does it by writing to the
> int_status register, so I think the above code is not correct.. 

That's right, so the int_cntl stuff in the interrupt handler is
superfluous?


> I based my code on the matrox fb driver and the atyfb driver. I don't
> have any documentation on the radeon chipsets, so am working on what I
> can gather on the net.

Let me point out again that the DRM has perfectly working code for
this...


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


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Ville Syrjälä]

On Wed, Mar 16, 2005 at 01:28:50AM +0000, Torgeir Veimo wrote:
> On Sat, 2005-03-12 at 17:13 +0200, Ville Syrjälä wrote:
> > On Sat, Mar 12, 2005 at 02:06:45PM +0000, Torgeir Veimo wrote:
> > > This is an implementation of the FB_WAITFORVSYNC ioctl for the radeonfb.
> > > A small test application is attached at the end. This patch is against
> > > vanilla 2.6.11.
> 
> > > +		/* clear interrupt */
> > > +		//OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_STAT_ACK);
> > 
> > Why is this commented out?
> 
> Well, first of all is it really important to clear the interrupt when
> enabling it?

Well, this being the vblank interrupt it's not really critical but 
probably a good idea anyway.

> >From what I've seen the rage128 acknowledges an interrupt by writing to
> the int_cntl register, while the radeon does it by writing to the
> int_status register, so I think the above code is not correct.. I might
> want to clear any vblank interrupts by writing to the int_status
> register though.

mach64 chips both enable and ack interrupts using the INT_CNTL register 
(hence the spinlock in atyfb). Apparently they weren't really thinking 
straight when they designed it.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Torgeir Veimo]

On Sun, 2005-03-13 at 10:30 +1100, Benjamin Herrenschmidt wrote:
> On Sat, 2005-03-12 at 14:06 +0000, Torgeir Veimo wrote:
> > This is an implementation of the FB_WAITFORVSYNC ioctl for the radeonfb.
> > A small test application is attached at the end. This patch is against
> > vanilla 2.6.11. 

> The patch definitely needs a lot of cleanups. A few things are wrong
> too, like using test_and_set_* as a mean of locking. This doesn't work
> on relaxed ordering architectures.
> 
> Besides, the entire fbdev subsystem is protected by the console
> semaphore, so you shouldn't need anything else. You need to acquire it
> yourself in the driver ioctl() routine though.
> 
> In order to avoid conflicting with other apps, you should probably
> "abort" the operation if a console switch happens while you are waiting.
> That is, a set_var() and/or a blank(). Return -EINTR or something like
> that.

Do you have any example code I can look at for this? From what I can see
neither the atyfb or matroxfb drivers do this. Not that I'm claiming
they're correct, I guess the current vanilla kernel source is a bit
behind the latest and greatest fb development. 

-- 
Torgeir Veimo <torgeir@pobox.com>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Torgeir Veimo]

On Sat, 2005-03-12 at 17:13 +0200, Ville Syrjälä wrote:
> On Sat, Mar 12, 2005 at 02:06:45PM +0000, Torgeir Veimo wrote:
> > This is an implementation of the FB_WAITFORVSYNC ioctl for the radeonfb.
> > A small test application is attached at the end. This patch is against
> > vanilla 2.6.11.
> 
> Some comements below.
> 
> > +	spinlock_t		int_lock;
> 
> Unused spinlock.

Removed. Something leftover as I experimented to get it to work
properly..


> > +		/* clear interrupt */
> > +		//OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_STAT_ACK);
> 
> Why is this commented out?

Well, first of all is it really important to clear the interrupt when
enabling it? 

From what I've seen the rage128 acknowledges an interrupt by writing to
the int_cntl register, while the radeon does it by writing to the
int_status register, so I think the above code is not correct.. I might
want to clear any vblank interrupts by writing to the int_status
register though.

I based my code on the matrox fb driver and the atyfb driver. I don't
have any documentation on the radeon chipsets, so am working on what I
can gather on the net.

-- 
Torgeir Veimo <torgeir@pobox.com>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Torgeir Veimo]

On Sun, 2005-03-13 at 10:14 +1100, Benjamin Herrenschmidt wrote:
> On Sat, 2005-03-12 at 14:06 +0000, Torgeir Veimo wrote:
> > This is an implementation of the FB_WAITFORVSYNC ioctl for the radeonfb.
> > A small test application is attached at the end. This patch is against
> > vanilla 2.6.11.
> > 
> > Signed-off-by: Torgeir Veimo <torgeir@pobox.com>
> > 
> 
> Please, CC me further radeonfb patches.

Sure! Am mostly driven by what I need myself, not by any agenda... : 

-- 
Torgeir Veimo <torgeir@pobox.com>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Benjamin Herrenschmidt]

> When we get merged my thought was to let fbdev catch the vsync IRQ and
> then chain into DRM. In that model fbdev becomes the mini-driver.
> Right now I don't think there is any other solution than merging
> fbdev/DRM that doesn't result in some kind of dead on VT switch
> scenario.

A VT switch should stop all asynchronous activity (interrupt, DRM,
etc...). X already stops the DRM on switch-out, this could be enforced
by the kernel.

> Also, I have the hotplug monitor change interrupt hookup in my local
> code but I'm waiting for BenH to finish his dual head radeonfb first.
> He has said it will probably be ready this week, patches are starting
> to appear on lkml from him.

Those are just fixes to the existing code base. The new one isn't ready
yet. I'll give it one more week unfortunately, I've been pressured by
other issues those past few days.

Ben.




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Benjamin Herrenschmidt]

On Sat, 2005-03-12 at 14:06 +0000, Torgeir Veimo wrote:
> This is an implementation of the FB_WAITFORVSYNC ioctl for the radeonfb.
> A small test application is attached at the end. This patch is against
> vanilla 2.6.11.
> 
> Signed-off-by: Torgeir Veimo <torgeir@pobox.com>

Please, put patches inline in the mail, not as attachment, it makes
dealing with them simpler and possible to quote them with any mailer.

The patch definitely needs a lot of cleanups. A few things are wrong
too, like using test_and_set_* as a mean of locking. This doesn't work
on relaxed ordering architectures.

Besides, the entire fbdev subsystem is protected by the console
semaphore, so you shouldn't need anything else. You need to acquire it
yourself in the driver ioctl() routine though.

In order to avoid conflicting with other apps, you should probably
"abort" the operation if a console switch happens while you are waiting.
That is, a set_var() and/or a blank(). Return -EINTR or something like
that.

Ben.






-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Benjamin Herrenschmidt]

On Sat, 2005-03-12 at 11:21 -0500, Jon Smirl wrote:
> On Sat, 12 Mar 2005 18:10:40 +0200, Ville Syrjälä <syrjala@sci.fi> wrote:
> > On Sat, Mar 12, 2005 at 11:00:56AM -0500, Jon Smirl wrote:
> > > On Sat, 12 Mar 2005 17:51:39 +0200, Ville Syrjälä <syrjala@sci.fi> wrote:
> > > > It's not a problem if one doesn't use X.Org/XFree86. Also this patch
> > > > should not affect X.Org/XFree86 users unless they manage to call the
> > > > FBIO_WAITFORVSYNC ioctl. matroxfb and atyfb both have similar interrupt
> > > > support and X can still be used with both.
> > >
> > > Almost all PPC Linux boxes are running radeonfb with Xorg.
> > 
> > What's your point? I doubt X.Org uses FBIO_WAITFORSYNC so the patch
> > should not interfere with anything.
> 
> Run something on one VT that uses radeonfb with FBIO_WAITFORSYNC.
> Switch to VT with xorg.
> Reboot box.

Not necessarily. We have the concept of active VT. FBIO_WAITFORSYNC only
need to enable interrupts during the ioctl execution, and mail stop
waiting and return an error if blank/set_var is called (and thus a VT
switch happens).

Ben.




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Benjamin Herrenschmidt]

On Sat, 2005-03-12 at 12:56 -0500, Jon Smirl wrote:

> 
> 1) where is the list of valid modes computed? in-kernel or user space.
> Technically either will work but each has plus and minuses. Right now
> radeonfb can do both.
> 
> 1a) #1 is computing the mode list, not setting the mode. The mode is
> always set in the kernel.

At first, I would keep the current mecanism. After all, parsing the EDID
and building the modelist is quite simple. Let the kernel driver do it,
pick a default mode, and let userland optionally change that. That gives
us at least a working boot mode for splash/console before userland is
available. fbdev now has an API for userland to add/remove entries to
modelists, we can extend on that.

> 2) multiheads implies buffer management, this needs to be coordinated with DRM

Yes. That's the main issue with the merge imho. My initial version will
do like MacOS, and cut the framebuffer in 2 apertures split in the
middle. That's the simplest way to get 2 independant access swappers.
This is necessary if we want completely independant framebuffer accesses
to /dev/fbN and /dev/fbN+1 with different bit depth. It may not stay
necessary in the long run if we have some lock for fb access, like X
has, in which case we can just switch the swapper settings before each
access.
 
> 3) fbdev needs to implement hardware cursors

That isn't terribly difficult.

> 4) The fb_info struct doesn't work too well for multiple heads. It
> really should be split up into fb_device/fb_head.

Well, when I look at other OSes, they don't do that neither. They
basically behave like if there was one device per head. All we need, I
think, is the necessary infos to let userland know which heads are
"linked" to a given device, but I don't think the fbdev layer has to
care about the concept of "device". All it cares about is a "head" which
is what fb_info represents.

> 5) driver initiated secondary card reset needs to be implement via a
> user space helper.
> 
> 6) fbcon is tied way too closely to fb_info. There should be an
> interface instead of just letting it muck around in fbdev data
> structures.

It's been very significantly split already, but yes, it could probably
be separated a bit more, though there again, we can go step by step,
there is no need to rewrite the whole world at once.

Ben.




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Benjamin Herrenschmidt]

On Sat, 2005-03-12 at 14:06 +0000, Torgeir Veimo wrote:
> This is an implementation of the FB_WAITFORVSYNC ioctl for the radeonfb.
> A small test application is attached at the end. This patch is against
> vanilla 2.6.11.
> 
> Signed-off-by: Torgeir Veimo <torgeir@pobox.com>
> 

Please, CC me further radeonfb patches.

Ben.




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Jon Smirl]

On Sat, 12 Mar 2005 14:20:24 -0500, Michel Dänzer <michel@daenzer.net> wrote:
> On Sat, 2005-03-12 at 17:12 +0000, Torgeir Veimo wrote:
> > On Sat, 2005-03-12 at 12:10 -0500, Michel Dänzer wrote:
> > > On Sat, 2005-03-12 at 17:13 +0200, Ville Syrjälä wrote:
> > > >
> > > > Also adding support for FB_ACTIVATE_VBL should be quite simple now.
> > >
> > > You don't need an interrupt for that. You can choose via the
> > > CRTC_OFFSET_CNTL:CRTC_OFFSET_FLIP_CNTL bit whether the update of the
> > > CRTC offset should take effect immediately or on the next vertical
> > > blank.
> > >
> > > Personally, I'm not sure why something that needs this functionality
> > > can't use the DRM...
> >
> > My interest for adding this is video playback. It's kind of hard to do
> > through the DRM.
> 
> You don't have to use the DRM for anything but the wait for vsync; I can
> see the effort of initializing the DRM to the point where this is
> possible being prohibitive though.
> 
> The real point is that the usage of the interrupt needs to be
> coordinated between the framebuffer device and the DRM. Something like
> the first one to request the IRQ setting up a callback mechanism where
> the other can hook into might be a possibility? Or the shared
> mini-driver that has been proposed for years, but isn't liked by some...

When we get merged my thought was to let fbdev catch the vsync IRQ and
then chain into DRM. In that model fbdev becomes the mini-driver.
Right now I don't think there is any other solution than merging
fbdev/DRM that doesn't result in some kind of dead on VT switch
scenario.

Also, I have the hotplug monitor change interrupt hookup in my local
code but I'm waiting for BenH to finish his dual head radeonfb first.
He has said it will probably be ready this week, patches are starting
to appear on lkml from him.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Michel Dänzer]

On Sat, 2005-03-12 at 17:12 +0000, Torgeir Veimo wrote:
> On Sat, 2005-03-12 at 12:10 -0500, Michel D채nzer wrote:
> > On Sat, 2005-03-12 at 17:13 +0200, Ville Syrj채l채 wrote:
> > > 
> > > Also adding support for FB_ACTIVATE_VBL should be quite simple now.
> > 
> > You don't need an interrupt for that. You can choose via the
> > CRTC_OFFSET_CNTL:CRTC_OFFSET_FLIP_CNTL bit whether the update of the
> > CRTC offset should take effect immediately or on the next vertical
> > blank.
> > 
> > Personally, I'm not sure why something that needs this functionality
> > can't use the DRM...
> 
> My interest for adding this is video playback. It's kind of hard to do
> through the DRM.

You don't have to use the DRM for anything but the wait for vsync; I can
see the effort of initializing the DRM to the point where this is
possible being prohibitive though.

The real point is that the usage of the interrupt needs to be
coordinated between the framebuffer device and the DRM. Something like
the first one to request the IRQ setting up a callback mechanism where
the other can hook into might be a possibility? Or the shared
mini-driver that has been proposed for years, but isn't liked by some...


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


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Jon Smirl]

On Sat, 12 Mar 2005 17:21:58 +0000, Torgeir Veimo <torgeir@pobox.com> wrote:
> On Sat, 2005-03-12 at 17:03 +0000, Torgeir Veimo wrote:
> > On Sat, 2005-03-12 at 10:33 -0500, Jon Smirl wrote:
> > > You can't do this without coordination from X and DRM.
> >
> > > Because of these issues WAITFORVSYNC in radeondb has to wait for
> > > merged DRM/fbdev to be implemented.
> >
> > It's a bit hard to track the merge-dri-and-db work,
> 
> ^^ merge-dri-and-fb

Merging DRM and fbdev is happening in bits and pieces in both DRM and
fbdev. I've already tried once at building it as a separate project
and the political hassles of getting it merged made me stop. The new
approach is to add the various bits to both drivers until they will
merge with each other. A big piece of getting fbdev ready for DRM was
getting the extended sysfs and module support that just went in.

BenH is working on a new version of radeonfb with support for both
heads. I've stopped working on radeonfb until this lands since it
changes 90% of the code.

Once the foundation is in place I'll modify DRM to attach to fbdev
instead of loading as a standalone driver. At that point we can work
on shared interrupt and buffer management code.

There are still some major unsettled issues:

1) where is the list of valid modes computed? in-kernel or user space.
Technically either will work but each has plus and minuses. Right now
radeonfb can do both.

1a) #1 is computing the mode list, not setting the mode. The mode is
always set in the kernel.

2) multiheads implies buffer management, this needs to be coordinated with DRM

3) fbdev needs to implement hardware cursors

4) The fb_info struct doesn't work too well for multiple heads. It
really should be split up into fb_device/fb_head.

5) driver initiated secondary card reset needs to be implement via a
user space helper.

6) fbcon is tied way too closely to fb_info. There should be an
interface instead of just letting it muck around in fbdev data
structures.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Ville Syrjälä]

On Sat, Mar 12, 2005 at 12:10:22PM -0500, Michel Dänzer wrote:
> On Sat, 2005-03-12 at 17:13 +0200, Ville Syrjälä wrote:
> > 
> > Also adding support for FB_ACTIVATE_VBL should be quite simple now.
> 
> You don't need an interrupt for that. You can choose via the
> CRTC_OFFSET_CNTL:CRTC_OFFSET_FLIP_CNTL bit whether the update of the
> CRTC offset should take effect immediately or on the next vertical
> blank.

Great. Matrox and mach64 chips don't have such a bit so they need 
to perform the flip/pan from the interrupt handler.

> Personally, I'm not sure why something that needs this functionality
> can't use the DRM...

I think the DRM is a real bitch to use. It requires a lot of hw specific 
init code and AFAIK it needs root privilidges to get to the state where 
you can actually use it. It's much easier to use fbdev. And IMO logically 
that's where the vblank interrupt belongs since fbdev is the one 
programming the CRTC.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Torgeir Veimo]

On Sat, 2005-03-12 at 17:03 +0000, Torgeir Veimo wrote:
> On Sat, 2005-03-12 at 10:33 -0500, Jon Smirl wrote:
> > You can't do this without coordination from X and DRM. 
> 
> > Because of these issues WAITFORVSYNC in radeondb has to wait for
> > merged DRM/fbdev to be implemented.
> 
> It's a bit hard to track the merge-dri-and-db work, 

^^ merge-dri-and-fb

-- 
Torgeir Veimo <torgeir@pobox.com>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Torgeir Veimo]

On Sat, 2005-03-12 at 12:10 -0500, Michel Dänzer wrote:
> On Sat, 2005-03-12 at 17:13 +0200, Ville Syrjälä wrote:
> > 
> > Also adding support for FB_ACTIVATE_VBL should be quite simple now.
> 
> You don't need an interrupt for that. You can choose via the
> CRTC_OFFSET_CNTL:CRTC_OFFSET_FLIP_CNTL bit whether the update of the
> CRTC offset should take effect immediately or on the next vertical
> blank.
> 
> Personally, I'm not sure why something that needs this functionality
> can't use the DRM...

My interest for adding this is video playback. It's kind of hard to do
through the DRM.

-- 
Torgeir Veimo <torgeir@pobox.com>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Michel Dänzer]

On Sat, 2005-03-12 at 17:13 +0200, Ville Syrjälä wrote:
> 
> Also adding support for FB_ACTIVATE_VBL should be quite simple now.

You don't need an interrupt for that. You can choose via the
CRTC_OFFSET_CNTL:CRTC_OFFSET_FLIP_CNTL bit whether the update of the
CRTC offset should take effect immediately or on the next vertical
blank.

Personally, I'm not sure why something that needs this functionality
can't use the DRM...


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


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Torgeir Veimo]

On Sat, 2005-03-12 at 10:33 -0500, Jon Smirl wrote:
> You can't do this without coordination from X and DRM. 

> Because of these issues WAITFORVSYNC in radeondb has to wait for
> merged DRM/fbdev to be implemented.

It's a bit hard to track the merge-dri-and-db work, so I'm basing it on
mainline kernel. Is there a common repository for this work somewhere?

I think it would be a good idea to settle on a common set of register
locations and values. Right now the #defines names used by framebuffers
differ quite a lot from those used by dri.

-- 
Torgeir Veimo <torgeir@pobox.com>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Torgeir Veimo]

On Sat, 2005-03-12 at 17:13 +0200, Ville Syrjälä wrote:
> On Sat, Mar 12, 2005 at 02:06:45PM +0000, Torgeir Veimo wrote:
> > This is an implementation of the FB_WAITFORVSYNC ioctl for the radeonfb.
> > A small test application is attached at the end. This patch is against
> > vanilla 2.6.11.
> 
> Some comements below.

Ok, I'll provide a new patch with fixes for these..

> Also adding support for FB_ACTIVATE_VBL should be quite simple now.

I guess it would require storing values, and apply the at the next vsync
irq. What operations would be interesting to do this way?


-- 
Torgeir Veimo <torgeir@pobox.com>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Jon Smirl]

On Sat, 12 Mar 2005 18:10:40 +0200, Ville Syrjälä <syrjala@sci.fi> wrote:
> On Sat, Mar 12, 2005 at 11:00:56AM -0500, Jon Smirl wrote:
> > On Sat, 12 Mar 2005 17:51:39 +0200, Ville Syrjälä <syrjala@sci.fi> wrote:
> > > It's not a problem if one doesn't use X.Org/XFree86. Also this patch
> > > should not affect X.Org/XFree86 users unless they manage to call the
> > > FBIO_WAITFORVSYNC ioctl. matroxfb and atyfb both have similar interrupt
> > > support and X can still be used with both.
> >
> > Almost all PPC Linux boxes are running radeonfb with Xorg.
> 
> What's your point? I doubt X.Org uses FBIO_WAITFORSYNC so the patch
> should not interfere with anything.

Run something on one VT that uses radeonfb with FBIO_WAITFORSYNC.
Switch to VT with xorg.
Reboot box.

> 
> --
> Ville Syrjälä
> syrjala@sci.fi
> http://www.sci.fi/~syrjala/
> 
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://ads.osdn.com/?ad_ide95&alloc_id\x14396&opclick
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 


-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Ville Syrjälä]

On Sat, Mar 12, 2005 at 11:00:56AM -0500, Jon Smirl wrote:
> On Sat, 12 Mar 2005 17:51:39 +0200, Ville Syrjälä <syrjala@sci.fi> wrote:
> > It's not a problem if one doesn't use X.Org/XFree86. Also this patch
> > should not affect X.Org/XFree86 users unless they manage to call the
> > FBIO_WAITFORVSYNC ioctl. matroxfb and atyfb both have similar interrupt
> > support and X can still be used with both.
> 
> Almost all PPC Linux boxes are running radeonfb with Xorg.

What's your point? I doubt X.Org uses FBIO_WAITFORSYNC so the patch 
should not interfere with anything.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Jon Smirl]

On Sat, 12 Mar 2005 17:51:39 +0200, Ville Syrjälä <syrjala@sci.fi> wrote:
> It's not a problem if one doesn't use X.Org/XFree86. Also this patch
> should not affect X.Org/XFree86 users unless they manage to call the
> FBIO_WAITFORVSYNC ioctl. matroxfb and atyfb both have similar interrupt
> support and X can still be used with both.

Almost all PPC Linux boxes are running radeonfb with Xorg.

> 
> --
> Ville Syrjälä
> syrjala@sci.fi
> http://www.sci.fi/~syrjala/
> 
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://ads.osdn.com/?ad_ide95&alloc_id\x14396&opclick
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 


-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Ville Syrjälä]

On Sat, Mar 12, 2005 at 10:33:45AM -0500, Jon Smirl wrote:
> You can't do this without coordination from X and DRM. If either of
> those are in use you will lock the machine with this patch.  DRM also
> implements WAITFORVSYNC as well as other interrupts. If you clear the
> ISR DRM is going to stop working.
> 
> X is a bigger problem. If the radeonfb adapter is not the primary X
> adapter, X is going to disable its IO/MEM access at the PCI level on
> VT switch. Your radeonfb will then take an interrupt and not be able
> to acknowledge it. This results in an interrupt loop and you rebooting
> your machine.
> 
> See the "Who is stomping PCI config space?" thread in the x.org xserver list.
> 
> Because of these issues WAITFORVSYNC in radeondb has to wait for
> merged DRM/fbdev to be implemented.

It's not a problem if one doesn't use X.Org/XFree86. Also this patch 
should not affect X.Org/XFree86 users unless they manage to call the 
FBIO_WAITFORVSYNC ioctl. matroxfb and atyfb both have similar interrupt 
support and X can still be used with both.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Jon Smirl]

You can't do this without coordination from X and DRM. If either of
those are in use you will lock the machine with this patch.  DRM also
implements WAITFORVSYNC as well as other interrupts. If you clear the
ISR DRM is going to stop working.

X is a bigger problem. If the radeonfb adapter is not the primary X
adapter, X is going to disable its IO/MEM access at the PCI level on
VT switch. Your radeonfb will then take an interrupt and not be able
to acknowledge it. This results in an interrupt loop and you rebooting
your machine.

See the "Who is stomping PCI config space?" thread in the x.org xserver list.

Because of these issues WAITFORVSYNC in radeondb has to wait for
merged DRM/fbdev to be implemented.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

[Ville Syrjälä]

On Sat, Mar 12, 2005 at 02:06:45PM +0000, Torgeir Veimo wrote:
> This is an implementation of the FB_WAITFORVSYNC ioctl for the radeonfb.
> A small test application is attached at the end. This patch is against
> vanilla 2.6.11.

Some comements below.

Also adding support for FB_ACTIVATE_VBL should be quite simple now.

> --- linux-2.6.11-orig/drivers/video/aty/radeonfb.h	2005-03-02 07:38:37.000000000 +0000
> +++ linux-2.6.11/drivers/video/aty/radeonfb.h	2005-03-03 17:09:23.000000000 +0000
<snip>
> @@ -344,6 +349,11 @@
>  	struct timer_list	lvds_timer;
>  	u32			pending_lvds_gen_cntl;
>  
> +	int			open;
> +	struct radeon_interrupt	vblank;
> +	unsigned long		irq_flags;
> +	spinlock_t		int_lock;
> +

Unused spinlock.

>  #ifdef CONFIG_FB_RADEON_I2C
>  	struct radeon_i2c_chan 	i2c[4];
>  #endif
> --- linux-2.6.11-orig/drivers/video/aty/radeon_base.c	2005-03-02 07:37:54.000000000 +0000
> +++ linux-2.6.11/drivers/video/aty/radeon_base.c	2005-03-12 13:53:32.000000000 +0000
> @@ -68,6 +68,9 @@
>  #include <linux/ioport.h>
>  #include <linux/init.h>
>  #include <linux/pci.h>
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>

Unused spinlock so no need for the header.

> +#include <linux/wait.h>
>  #include <linux/vmalloc.h>
>  #include <linux/device.h>
>  #include <linux/i2c.h>
> @@ -863,6 +866,97 @@
>  }
>  
>  
> +static irqreturn_t radeon_irq(int irq, void *dev_id, struct pt_regs *fp)
> +{
> +	struct radeonfb_info *rinfo = dev_id;
> +	u32 int_cntl = 0;
> +	u32 stat = 0;
> +
> +	int_cntl = INREG(GEN_INT_CNTL);
> +	stat = INREG(GEN_INT_STATUS) & (CRTC_VBLANK_MASK); 
> +	if (!stat)
> +		return IRQ_NONE;
> +	if (stat) {
> +		/* clear interrupt */
> +		OUTREG(GEN_INT_STATUS, stat);
> +
> +		(rinfo->vblank.count)++;

Nitpick: Drop the parentheses.

> +		wake_up_interruptible(&rinfo->vblank.wait);
> +	}
> +	return IRQ_HANDLED;
> +}
> +
> +static int radeon_enable_irq(struct radeonfb_info *rinfo, int reenable)
> +{
> +	u32 int_cntl;
> +
> +	if (!test_and_set_bit(0, &rinfo->irq_flags)) {
> +		if (request_irq(rinfo->pdev->irq, radeon_irq, SA_SHIRQ, 
> +			"radeonfb", rinfo)) {
> +			printk("radeonfb: request_irq failed..\n");
> +			clear_bit(0, &rinfo->irq_flags);
> +			return -EINVAL;
> +		}
> +		int_cntl = INREG(GEN_INT_CNTL) & CRTC_VBLANK_MASK;
> +		/* clear interrupt */
> +		//OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_STAT_ACK);

Why is this commented out?

> +		/* enable interrupt */
> +		OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_INT_EN);
> +		printk("radeonfb: enabled IRQ\n");
> +	} else if (reenable) {
> +		int_cntl = INREG(GEN_INT_CNTL) & CRTC_VBLANK_MASK;
> +		if (!(int_cntl & CRTC_VBLANK_INT_EN)) {
> +			printk("radeonfb: someone disabled IRQ [%08x]\n", int_cntl);
> +			/* re-enable interrupt */
> +			OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_INT_EN);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
<snip>
> @@ -2338,6 +2460,10 @@
>  	radeon_create_i2c_busses(rinfo);
>  #endif
>  
> +	rinfo->irq_flags = 0;

Nitpick: No need to set to 0. rinfo is memset() to 0 by 
framebuffer_alloc().

> +	init_waitqueue_head(&rinfo->vblank.wait);
> +	spin_lock_init(&(rinfo->int_lock));
> +

Unused spinlock.

>  	/* set all the vital stuff */
>  	radeon_set_fbinfo (rinfo);

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

[patch] radeonfb: FB_WAITFORVSYNC implementation

[Torgeir Veimo]

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

This is an implementation of the FB_WAITFORVSYNC ioctl for the radeonfb.
A small test application is attached at the end. This patch is against
vanilla 2.6.11.

Signed-off-by: Torgeir Veimo <torgeir@pobox.com>


-- 
Torgeir Veimo <torgeir@pobox.com>

[-- Attachment #2: vsynctest.c --]
[-- Type: text/x-csrc, Size: 1342 bytes --]

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <strings.h>
#include <fcntl.h>
#include <sys/time.h>
#include <sys/ioctl.h>

#define __u32 u_int32_t

#ifndef FBIO_WAITFORVSYNC
#define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
#endif

/* check functionality of VSYNC ioctl & determine approximate vertical refresh frequency */
/* compile with gcc -o vsynctest vsynctest.c */
/* March 12th, 2005, torgeir at nothome dot com. */

int main(int argc, char** argv) {
    int retval = 0;
    int fd = 0;
    int zero = 0;
	int i;
    struct timeval start;
	struct timeval end;
	long total; float secs; float rate;
	
    printf("testing refresh rate using FB_WAITFORVSYNC ioctl, standby while i loop 500 times..\n");
    if ((fd = open( "/dev/fb0", O_RDWR )) < 0) {
		printf("unable to access /dev/fb0, exiting..\n");
		exit(1);
	}
	
	gettimeofday(&start, NULL);

	for (i = 0; i < 500; i++) {
		retval = ioctl(fd, FBIO_WAITFORVSYNC, &zero);	
		if (retval != 0) {
			printf("ioctl failed, aborting...\n");
			exit(1);
		}			
	}
	gettimeofday(&end, NULL);
	
	total = (end.tv_sec - start.tv_sec) * 1000000 + (end.tv_usec - start.tv_usec);
	secs = total / 1000000.0;
	rate = 500000000.0 / total;
    printf("time elapsed: %2.2f secs, refresh rate is %2.2f Hz\n", secs, rate);

    close( fd );
}

[-- Attachment #3: irq.patch --]
[-- Type: text/x-patch, Size: 5518 bytes --]

--- linux-2.6.11-orig/include/video/radeon.h	2005-03-02 07:38:33.000000000 +0000
+++ linux-2.6.11/include/video/radeon.h	2005-03-12 13:50:35.000000000 +0000
@@ -541,6 +541,14 @@
 #define CRTC_EN					   (1 << 25)
 #define CRTC_DISP_REQ_EN_B                         (1 << 26)
 
+/* GEN_INT_CNTL bit constants */
+#define CRTC_VBLANK_MASK          (1 << 0)
+#define CRTC_VBLANK_INT_EN        (1 << 0)
+
+/* GEN_INT_STATUS bit constants */
+#define CRTC_VBLANK_STAT          (1 << 0)
+#define CRTC_VBLANK_STAT_ACK      (1 << 0)
+
 /* CRTC_STATUS bit constants */
 #define CRTC_VBLANK                                0x00000001
 
--- linux-2.6.11-orig/drivers/video/aty/radeonfb.h	2005-03-02 07:38:37.000000000 +0000
+++ linux-2.6.11/drivers/video/aty/radeonfb.h	2005-03-03 17:09:23.000000000 +0000
@@ -269,6 +269,11 @@
 	radeon_pm_off	= 0x00000002,	/* Can resume from D3 cold */
 };
 
+struct radeon_interrupt {
+	wait_queue_head_t wait;
+	unsigned int count;
+};
+
 struct radeonfb_info {
 	struct fb_info		*info;
 
@@ -344,6 +349,11 @@
 	struct timer_list	lvds_timer;
 	u32			pending_lvds_gen_cntl;
 
+	int			open;
+	struct radeon_interrupt	vblank;
+	unsigned long		irq_flags;
+	spinlock_t		int_lock;
+
 #ifdef CONFIG_FB_RADEON_I2C
 	struct radeon_i2c_chan 	i2c[4];
 #endif
--- linux-2.6.11-orig/drivers/video/aty/radeon_base.c	2005-03-02 07:37:54.000000000 +0000
+++ linux-2.6.11/drivers/video/aty/radeon_base.c	2005-03-12 13:53:32.000000000 +0000
@@ -68,6 +68,9 @@
 #include <linux/ioport.h>
 #include <linux/init.h>
 #include <linux/pci.h>
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/wait.h>
 #include <linux/vmalloc.h>
 #include <linux/device.h>
 #include <linux/i2c.h>
@@ -863,6 +866,97 @@
 }
 
 
+static irqreturn_t radeon_irq(int irq, void *dev_id, struct pt_regs *fp)
+{
+	struct radeonfb_info *rinfo = dev_id;
+	u32 int_cntl = 0;
+	u32 stat = 0;
+
+	int_cntl = INREG(GEN_INT_CNTL);
+	stat = INREG(GEN_INT_STATUS) & (CRTC_VBLANK_MASK); 
+	if (!stat)
+		return IRQ_NONE;
+	if (stat) {
+		/* clear interrupt */
+		OUTREG(GEN_INT_STATUS, stat);
+
+		(rinfo->vblank.count)++;
+		wake_up_interruptible(&rinfo->vblank.wait);
+	}
+	return IRQ_HANDLED;
+}
+
+static int radeon_enable_irq(struct radeonfb_info *rinfo, int reenable)
+{
+	u32 int_cntl;
+
+	if (!test_and_set_bit(0, &rinfo->irq_flags)) {
+		if (request_irq(rinfo->pdev->irq, radeon_irq, SA_SHIRQ, 
+			"radeonfb", rinfo)) {
+			printk("radeonfb: request_irq failed..\n");
+			clear_bit(0, &rinfo->irq_flags);
+			return -EINVAL;
+		}
+		int_cntl = INREG(GEN_INT_CNTL) & CRTC_VBLANK_MASK;
+		/* clear interrupt */
+		//OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_STAT_ACK);
+		/* enable interrupt */
+		OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_INT_EN);
+		printk("radeonfb: enabled IRQ\n");
+	} else if (reenable) {
+		int_cntl = INREG(GEN_INT_CNTL) & CRTC_VBLANK_MASK;
+		if (!(int_cntl & CRTC_VBLANK_INT_EN)) {
+			printk("radeonfb: someone disabled IRQ [%08x]\n", int_cntl);
+			/* re-enable interrupt */
+			OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_INT_EN);
+		}
+	}
+
+	return 0;
+}
+
+static void radeon_disable_irq(struct radeonfb_info *rinfo)
+{
+	u32 int_cntl;
+
+	if (test_and_clear_bit(0, &rinfo->irq_flags)) {
+		int_cntl = INREG(GEN_INT_CNTL) & CRTC_VBLANK_MASK;
+		/* disable interrupt */
+		OUTREG(GEN_INT_CNTL, int_cntl & ~CRTC_VBLANK_INT_EN);
+		free_irq(rinfo->pdev->irq, rinfo);
+		printk("radeonfb: disabled IRQ\n");
+	}
+}
+
+static int radeon_waitforvblank(struct radeonfb_info *rinfo)
+{
+	struct radeon_interrupt *vbl;
+	unsigned int count;
+	int ret;
+
+	vbl = &rinfo->vblank;
+
+	ret = radeon_enable_irq(rinfo, 0);
+	if (ret) {
+		return ret;
+	}
+
+	count = vbl->count;
+	ret = wait_event_interruptible_timeout(vbl->wait, count != vbl->count, HZ/10);
+	if (ret < 0) {
+		return ret;
+	}
+	if (ret == 0) {
+		radeon_enable_irq(rinfo, 1);
+		return -ETIMEDOUT;
+	}
+	return 0;
+}
+
+#ifndef FBIO_WAITFORVSYNC
+#define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
+#endif
+
 static int radeonfb_ioctl (struct inode *inode, struct file *file, unsigned int cmd,
                            unsigned long arg, struct fb_info *info)
 {
@@ -927,6 +1021,8 @@
 				value |= 0x02;
 
 			return put_user(value, (__u32 __user *)arg);
+		case FBIO_WAITFORVSYNC:
+			return radeon_waitforvblank(rinfo);
 		default:
 			return -EINVAL;
 	}
@@ -1464,6 +1560,30 @@
 	RTRACE("ppll_div_3 = 0x%x\n", regs->ppll_div_3);
 }
 
+static int radeonfb_open(struct fb_info *info, int user)
+{
+	struct radeonfb_info *rinfo = (struct radeonfb_info *) info->par;
+	if (user) {
+		rinfo->open++;
+	}
+	return (0);
+}
+
+static int radeonfb_release(struct fb_info *info, int user)
+{
+	struct radeonfb_info *rinfo = (struct radeonfb_info *) info->par;
+	if (user) {
+		rinfo->open--;
+		mdelay(1);
+		//wait_for_idle(rinfo);
+		if (!rinfo->open) {
+			radeon_disable_irq(rinfo);
+		}
+	}
+	return (0);
+}
+
+
 static int radeonfb_set_par(struct fb_info *info)
 {
 	struct radeonfb_info *rinfo = info->par;
@@ -1788,6 +1908,8 @@
 
 static struct fb_ops radeonfb_ops = {
 	.owner			= THIS_MODULE,
+	.fb_open	        = radeonfb_open,
+	.fb_release	        = radeonfb_release,        
 	.fb_check_var		= radeonfb_check_var,
 	.fb_set_par		= radeonfb_set_par,
 	.fb_setcolreg		= radeonfb_setcolreg,
@@ -2338,6 +2460,10 @@
 	radeon_create_i2c_busses(rinfo);
 #endif
 
+	rinfo->irq_flags = 0;
+	init_waitqueue_head(&rinfo->vblank.wait);
+	spin_lock_init(&(rinfo->int_lock));
+
 	/* set all the vital stuff */
 	radeon_set_fbinfo (rinfo);