Re: [PATCH] drm/i915: Disable high-precision vblank timestamping

2011-01-23 Thread Mario Kleiner 

--

Message: 10
Date: Sun, 23 Jan 2011 10:50:01 +
From: Chris Wilson ch...@chris-wilson.co.uk
Subject: [PATCH] drm/i915: Disable high-precision vblank timestamping
for UMS
To: Chris Clayton chris2...@googlemail.com
Cc: linux-ker...@vger.kernel.org, dri-devel@lists.freedesktop.org
Message-ID:
1295779801-8118-1-git-send-email-ch...@chris-wilson.co.uk

We only have sufficient information for accurate (sub-frame)  
timestamping

when the modesetting is under our control.

Reported-by: Chris Clayton chris2...@googlemail.com
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_drv.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/ 
i915_drv.c

index 59eb19b..66796bb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -752,6 +752,9 @@ static int __init i915_init(void)
driver.driver_features = ~DRIVER_MODESET;
 #endif

+   if (!(driver.driver_features  DRIVER_MODESET))
+   driver.get_vblank_timestamp = NULL;
+
return drm_init(driver);
 }

--
1.7.2.3





Oops! Have missed that. Thanks for fixing it.

Reviewed-by: Mario Kleiner mario.klei...@tuebingen.mpg.de


*
Mario Kleiner
Max Planck Institute for Biological Cybernetics
Spemannstr. 38
72076 Tuebingen
Germany

e-mail: mario.klei...@tuebingen.mpg.de
office: +49 (0)7071/601-1623
fax:+49 (0)7071/601-616
www:http://www.kyb.tuebingen.mpg.de/~kleinerm
*
For a successful technology, reality must take precedence
over public relations, for Nature cannot be fooled.
(Richard Feynman)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Intel-gfx] [PATCH] drm: Aggressively disable vblanks

2010-12-27 Thread Mario Kleiner 

On Dec 27, 2010, at 12:16 PM, Ville Syrj?l? wrote:

> On Mon, Dec 27, 2010 at 12:58:10AM +0100, Mario Kleiner wrote:
>
>> 2. There are gpu's firing spurious vblank irq's as soon as you enable
>> irq's
>
> You're sure this isn't simply a matter of the driver forgetting to ack
> the irq just before enabling it?
>

Good point. This was on radeon. I can't remember for certain if it  
happened always, or only frequently. I can check that later this week  
when i'm back at the test machine.

Anyway, it's good to be robust against such problems, regardless if  
it is gpu quirks or driver bugs. The current implementation would  
filter the redundant vblank irq and DRM_DEBUG a message if the  
drm.debug parameter is set.

thanks,
-mario

> -- 
> Ville Syrj?l?
> syrjala at sci.fi
> http://www.sci.fi/~syrjala/

*****
Mario Kleiner
Max Planck Institute for Biological Cybernetics
Spemannstr. 38
72076 Tuebingen
Germany

e-mail: mario.kleiner at tuebingen.mpg.de
office: +49 (0)7071/601-1623
fax:+49 (0)7071/601-616
www:http://www.kyb.tuebingen.mpg.de/~kleinerm
*
"For a successful technology, reality must take precedence
over public relations, for Nature cannot be fooled."
(Richard Feynman)

[Intel-gfx] [PATCH] drm: Aggressively disable vblanks

2010-12-27 Thread Mario Kleiner 
On Dec 26, 2010, at 3:53 PM, Andrew Lutomirski wrote:

> On Wed, Dec 22, 2010 at 4:06 PM, Mario Kleiner
>  wrote:
>>
>> There's a new drm module parameter for selecting the timeout: echo  
>> 50 >
>> /sys/module/drm/parameters/vblankoffdelay
>> would set the timeout to 50 msecs. A setting of zero will disable  
>> the timer,
>> so vblank irq's would stay on all the time.
>>
>> The default setting is still 5000 msecs as before, but reducing  
>> this to 100
>> msecs wouldn't be a real problem imho. At least i didn't observe any
>> miscounting during extensive testing with 100 msecs.
>>
>> The patches in drm-next fix a couple of races that i observed on  
>> intel and
>> radeon during testing and a few that i didn't see but that i could  
>> imagine
>> happening. It tries to make sure that the saved final count at  
>> vblank irq
>> disable of the software vblank_count and the gpu counter are  
>> consistent - no
>> off by one errors. They also try to detect and filter out spurious  
>> vblank
>> interrupts at vblank enable time, e.g., on the radeon.
>>
>> There's still one possible race in the disable path which i will  
>> try to fix:
>> We don't know when exactly the hardware counter increments wrt. the
>> processing of the vblank interrupt - it could increment a few
>> (dozen/hundred) microseconds before or after the irq handler runs,  
>> so if you
>> happen to query the hardware counter while the gpu is inside the  
>> vblank you
>> can't be sure if you picked up the old count or the new count for  
>> that
>> vblank.
>
> That's disgusting.  Does this affect many GPUs?  (I can't imagine why
> any sensible implementation wouldn't guarantee that the counter
> increments just before the IRQ.)

;-). I don't know, but at least on the tested R500 and R600 class  
Radeon's, this was the case, so i assume it's at least this way on  
many radeon gpu's (probably all avivo parts?) out there. We don't  
have any evergreen gpu's yet in our lab so i don't know how the more  
recent parts behave. Also it doesn't matter

I guess it's also a matter of definition when a new video frame  
starts? Leading edge / trailing edge of vblank? Start of vsync?  
Something else?

>>
>> This only matters during vblank disable. For that reason it's not  
>> such a
>> good idea to disable vblank irq's from within the vblank irq  
>> handler. I
>> tried that and it didn't work well --> When doing it from within  
>> irq you
>> basically maximize the chance of hitting the time window when the  
>> race can
>> happen. Delaying within the irq handler by a millisecond would fix  
>> that, but
>> that's not what we want.
>>
>> Having the disable in a function triggered by a timer like now is  
>> the most
>> simple solution i could come up with. There we can burn a few dozen
>> microseconds if neccessary to remove this race.
>
> Maybe I'm missing something, but other than the race above (which
> seems like it shouldn't exist on sane hardware), the new code seems
> more complicated than necessary.

I don't think it's more complicated than necessary for what it tries  
to achieve, but of course i'm a bit biased. It also started off more  
simple and grew a bit when i found new issues with the tested gpu's.

The aim is to fix a couple of real races and to make vblank counts  
and timestamps as trustworthy and oml_sync_control spec-conformant  
(see http://www.opengl.org/registry/specs/OML/glx_sync_control.txt)  
as possible. It only consumes a few additional bytes of memory  
(approx. 40 bytes) per crtc, doesn't use excessive time inside the  
irq handler and tries to avoid taking locks  that are shared between  
irq and non-irq context to avoid delays in irq execution, also if  
used with a kernel with the preempt_rt patch applied (important for  
my use case and other hard realtime apps). It's pretty self-contained  
and because most of it is driver-independent it can handle similar  
issues on different gpu's and kms drivers without the need for us to  
check each single gpu + driver combo if it has such issues or not.

1. There's the hardware vblank counter race, and it's there on lots  
of existing hardware, regardless if this is sane or not, so it needs  
to be handled.

2. There are gpu's firing spurious vblank irq's as soon as you enable  
irq's -- you need to filter those out, otherwise counts and  
timestamps will be wrong for at least one refresh cycle after vblank  
irq enable. You don't know when these redundant ones get delivered or  
if they get delivered at all because a real vblank irq enable gets  
triggered by some userspace calls, not locked to the video refresh  
cy

Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks

2010-12-27 Thread Mario Kleiner 


On Dec 27, 2010, at 12:16 PM, Ville Syrjälä wrote:


On Mon, Dec 27, 2010 at 12:58:10AM +0100, Mario Kleiner wrote:


2. There are gpu's firing spurious vblank irq's as soon as you enable
irq's


You're sure this isn't simply a matter of the driver forgetting to ack
the irq just before enabling it?



Good point. This was on radeon. I can't remember for certain if it  
happened always, or only frequently. I can check that later this week  
when i'm back at the test machine.


Anyway, it's good to be robust against such problems, regardless if  
it is gpu quirks or driver bugs. The current implementation would  
filter the redundant vblank irq and DRM_DEBUG a message if the  
drm.debug parameter is set.


thanks,
-mario


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


*
Mario Kleiner
Max Planck Institute for Biological Cybernetics
Spemannstr. 38
72076 Tuebingen
Germany

e-mail: mario.klei...@tuebingen.mpg.de
office: +49 (0)7071/601-1623
fax:+49 (0)7071/601-616
www:http://www.kyb.tuebingen.mpg.de/~kleinerm
*
For a successful technology, reality must take precedence
over public relations, for Nature cannot be fooled.
(Richard Feynman)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks

2010-12-26 Thread Mario Kleiner 

On Dec 26, 2010, at 3:53 PM, Andrew Lutomirski wrote:


On Wed, Dec 22, 2010 at 4:06 PM, Mario Kleiner
mario.klei...@tuebingen.mpg.de wrote:


There's a new drm module parameter for selecting the timeout: echo  
50 

/sys/module/drm/parameters/vblankoffdelay
would set the timeout to 50 msecs. A setting of zero will disable  
the timer,

so vblank irq's would stay on all the time.

The default setting is still 5000 msecs as before, but reducing  
this to 100

msecs wouldn't be a real problem imho. At least i didn't observe any
miscounting during extensive testing with 100 msecs.

The patches in drm-next fix a couple of races that i observed on  
intel and
radeon during testing and a few that i didn't see but that i could  
imagine
happening. It tries to make sure that the saved final count at  
vblank irq
disable of the software vblank_count and the gpu counter are  
consistent - no
off by one errors. They also try to detect and filter out spurious  
vblank

interrupts at vblank enable time, e.g., on the radeon.

There's still one possible race in the disable path which i will  
try to fix:

We don't know when exactly the hardware counter increments wrt. the
processing of the vblank interrupt - it could increment a few
(dozen/hundred) microseconds before or after the irq handler runs,  
so if you
happen to query the hardware counter while the gpu is inside the  
vblank you
can't be sure if you picked up the old count or the new count for  
that

vblank.


That's disgusting.  Does this affect many GPUs?  (I can't imagine why
any sensible implementation wouldn't guarantee that the counter
increments just before the IRQ.)


;-). I don't know, but at least on the tested R500 and R600 class  
Radeon's, this was the case, so i assume it's at least this way on  
many radeon gpu's (probably all avivo parts?) out there. We don't  
have any evergreen gpu's yet in our lab so i don't know how the more  
recent parts behave. Also it doesn't matter


I guess it's also a matter of definition when a new video frame  
starts? Leading edge / trailing edge of vblank? Start of vsync?  
Something else?




This only matters during vblank disable. For that reason it's not  
such a
good idea to disable vblank irq's from within the vblank irq  
handler. I
tried that and it didn't work well -- When doing it from within  
irq you
basically maximize the chance of hitting the time window when the  
race can
happen. Delaying within the irq handler by a millisecond would fix  
that, but

that's not what we want.

Having the disable in a function triggered by a timer like now is  
the most

simple solution i could come up with. There we can burn a few dozen
microseconds if neccessary to remove this race.


Maybe I'm missing something, but other than the race above (which
seems like it shouldn't exist on sane hardware), the new code seems
more complicated than necessary.


I don't think it's more complicated than necessary for what it tries  
to achieve, but of course i'm a bit biased. It also started off more  
simple and grew a bit when i found new issues with the tested gpu's.


The aim is to fix a couple of real races and to make vblank counts  
and timestamps as trustworthy and oml_sync_control spec-conformant  
(see http://www.opengl.org/registry/specs/OML/glx_sync_control.txt)  
as possible. It only consumes a few additional bytes of memory  
(approx. 40 bytes) per crtc, doesn't use excessive time inside the  
irq handler and tries to avoid taking locks  that are shared between  
irq and non-irq context to avoid delays in irq execution, also if  
used with a kernel with the preempt_rt patch applied (important for  
my use case and other hard realtime apps). It's pretty self-contained  
and because most of it is driver-independent it can handle similar  
issues on different gpu's and kms drivers without the need for us to  
check each single gpu + driver combo if it has such issues or not.


1. There's the hardware vblank counter race, and it's there on lots  
of existing hardware, regardless if this is sane or not, so it needs  
to be handled.


2. There are gpu's firing spurious vblank irq's as soon as you enable  
irq's -- you need to filter those out, otherwise counts and  
timestamps will be wrong for at least one refresh cycle after vblank  
irq enable. You don't know when these redundant ones get delivered or  
if they get delivered at all because a real vblank irq enable gets  
triggered by some userspace calls, not locked to the video refresh  
cycle and because the enable code itself holds spin_lock_irqsave  
locks and may or may not (depending on number of cores and irq  
routing) prevent delivery of the vblank irq's concurrent to its own  
execution - a possible race between the drm_handle_vblank() routine  
and the drm_update_vblank_count() routine each time you call  
drm_vblank_get().


3. There's gpu's sometimes, but not on other times, firing the irq  
too early (1-3 scanlines observed on radeon's) so you get your

[Intel-gfx] [PATCH] drm: Aggressively disable vblanks

2010-12-22 Thread Mario Kleiner 
On Dec 22, 2010, at 6:23 PM, Jesse Barnes wrote:

> On Wed, 22 Dec 2010 05:36:13 +0100
> Mario Kleiner  wrote:
>
>>>  
>>> --
>>>
>>> Message: 1
>>> Date: Mon, 20 Dec 2010 19:23:40 -0800
>>> From: Keith Packard 
>>> Subject: Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks
>>> To: Andy Lutomirski , Jesse Barnes
>>> ,  Chris Wilson >> wilson.co.uk>,
>>> David Airlie 
>>> Cc: intel-gfx at lists.freedesktop.org, dri-devel at lists.freedesktop.org
>>> Message-ID: 
>>> Content-Type: text/plain; charset="us-ascii"
>>>
>>> On Mon, 20 Dec 2010 14:00:54 -0500, Andy Lutomirski 
>>> wrote:
>>>
>>>> Enabling and disabling the vblank interrupt (on devices that
>>>> support it) is cheap.  So disable it quickly after each
>>>> interrupt.
>>>
>>> So, the concern (and reason for the original design) was that you
>>> might
>>> lose count of vblank interrupts as vblanks are counted differently
>>> while
>>> off than while on. If vblank interrupts get enabled near the  
>>> interrupt
>>> time, is it possible that you'll end up off by one somehow?
>>>
>>> Leaving them enabled for 'a while' was supposed to reduce the
>>> impact of
>>> this potential error.
>>>
>>> -- 
>>> keith.packard at intel.com
>>> -- next part --
>>> A non-text attachment was scrubbed...
>>> Name: not available
>>> Type: application/pgp-signature
>>> Size: 189 bytes
>>> Desc: not available
>>> URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/
>>> 20101220/105a9fb6/attachment-0001.pgp>
>>>
>>> --
>>>
>>> Message: 2
>>> Date: Mon, 20 Dec 2010 22:34:12 -0500
>>> From: Andrew Lutomirski 
>>> Subject: Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks
>>> To: Keith Packard 
>>> Cc: intel-gfx at lists.freedesktop.org, dri-devel at lists.freedesktop.org
>>> Message-ID:
>>> <AANLkTik-1zni1DdS6i+1ARoenx6p=9jQAAiA6t5uGg_K at mail.gmail.com>
>>> Content-Type: text/plain; charset=ISO-8859-1
>>>
>>> On Mon, Dec 20, 2010 at 10:23 PM, Keith Packard 
>>> wrote:
>>>> On Mon, 20 Dec 2010 14:00:54 -0500, Andy Lutomirski 
>>>> wrote:
>>>>
>>>>> Enabling and disabling the vblank interrupt (on devices that
>>>>> support it) is cheap. ?So disable it quickly after each
>>>>> interrupt.
>>>>
>>>> So, the concern (and reason for the original design) was that you
>>>> might
>>>> lose count of vblank interrupts as vblanks are counted differently
>>>> while
>>>> off than while on. If vblank interrupts get enabled near the
>>>> interrupt
>>>> time, is it possible that you'll end up off by one somehow?
>>>
>>> So the race is:
>>>
>>> 1. Vblank happens.
>>> 2. vblank_get runs, reads hardware counter, and enables the  
>>> interrupt
>>> (and maybe not quite in that order)
>>> 3. Interrupt fires and increments the counter.  Now the counter is
>>> one too high.
>>>
>>> What if we read the hardware counter from the IRQ the first time  
>>> that
>>> it fires after being enabled?  That way, if the hardware and  
>>> software
>>> counters match (mod 2^23 or whatever the magic number is), we don't
>>> increment the counter.
>>>
>>>>
>>>> Leaving them enabled for 'a while' was supposed to reduce the
>>>> impact of
>>>> this potential error.
>>>>
>>>
>>> Fair enough,
>>>
>>> But five seconds is a *long* time, and anything short enough that  
>>> the
>>> interrupt actually gets turned off in normal use risks the same  
>>> race.
>>>
>>> --Andy
>>>
>>>
>>> --
>>>
>>> Message: 3
>>> Date: Mon, 20 Dec 2010 19:47:11 -0800
>>> From: Keith Packard 
>>> Subject: Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks
>>> To: Andrew Lutomirski 
>>> Cc: intel-gfx at lists.freedesktop.org, dri-devel at lists.freedesktop.org
>>> Message-ID: 
>>> Content-Type: text/plain; charset="us

Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks

2010-12-22 Thread Mario Kleiner 

On Dec 22, 2010, at 6:23 PM, Jesse Barnes wrote:


On Wed, 22 Dec 2010 05:36:13 +0100
Mario Kleiner mario.klei...@tuebingen.mpg.de wrote:

 
--


Message: 1
Date: Mon, 20 Dec 2010 19:23:40 -0800
From: Keith Packard kei...@keithp.com
Subject: Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks
To: Andy Lutomirski l...@mit.edu, Jesse Barnes
	jbar...@virtuousgeek.org,	Chris Wilson ch...@chris- 
wilson.co.uk,

David Airlie airl...@linux.ie
Cc: intel-...@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Message-ID: yunfwtrrepv@aiko.keithp.com
Content-Type: text/plain; charset=us-ascii

On Mon, 20 Dec 2010 14:00:54 -0500, Andy Lutomirski l...@mit.edu
wrote:


Enabling and disabling the vblank interrupt (on devices that
support it) is cheap.  So disable it quickly after each
interrupt.


So, the concern (and reason for the original design) was that you
might
lose count of vblank interrupts as vblanks are counted differently
while
off than while on. If vblank interrupts get enabled near the  
interrupt

time, is it possible that you'll end up off by one somehow?

Leaving them enabled for 'a while' was supposed to reduce the
impact of
this potential error.

--
keith.pack...@intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: http://lists.freedesktop.org/archives/dri-devel/attachments/
20101220/105a9fb6/attachment-0001.pgp

--

Message: 2
Date: Mon, 20 Dec 2010 22:34:12 -0500
From: Andrew Lutomirski l...@mit.edu
Subject: Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks
To: Keith Packard kei...@keithp.com
Cc: intel-...@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Message-ID:
aanlktik-1zni1dds6i+1aroenx6p=9jqaaia6t5ug...@mail.gmail.com
Content-Type: text/plain; charset=ISO-8859-1

On Mon, Dec 20, 2010 at 10:23 PM, Keith Packard kei...@keithp.com
wrote:

On Mon, 20 Dec 2010 14:00:54 -0500, Andy Lutomirski l...@mit.edu
wrote:


Enabling and disabling the vblank interrupt (on devices that
support it) is cheap. ?So disable it quickly after each
interrupt.


So, the concern (and reason for the original design) was that you
might
lose count of vblank interrupts as vblanks are counted differently
while
off than while on. If vblank interrupts get enabled near the
interrupt
time, is it possible that you'll end up off by one somehow?


So the race is:

1. Vblank happens.
2. vblank_get runs, reads hardware counter, and enables the  
interrupt

(and maybe not quite in that order)
3. Interrupt fires and increments the counter.  Now the counter is
one too high.

What if we read the hardware counter from the IRQ the first time  
that
it fires after being enabled?  That way, if the hardware and  
software

counters match (mod 2^23 or whatever the magic number is), we don't
increment the counter.



Leaving them enabled for 'a while' was supposed to reduce the
impact of
this potential error.



Fair enough,

But five seconds is a *long* time, and anything short enough that  
the
interrupt actually gets turned off in normal use risks the same  
race.


--Andy


--

Message: 3
Date: Mon, 20 Dec 2010 19:47:11 -0800
From: Keith Packard kei...@keithp.com
Subject: Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks
To: Andrew Lutomirski l...@mit.edu
Cc: intel-...@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Message-ID: yunbp4frdmo@aiko.keithp.com
Content-Type: text/plain; charset=us-ascii

On Mon, 20 Dec 2010 22:34:12 -0500, Andrew Lutomirski
l...@mit.edu wrote:

But five seconds is a *long* time, and anything short enough  
that the
interrupt actually gets turned off in normal use risks the same  
race.


Right, so eliminating any race seems like the basic requirement. Can
that be done?

--
keith.pack...@intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: http://lists.freedesktop.org/archives/dri-devel/attachments/
20101220/5ca3b674/attachment-0001.pgp

--

Message: 4
Date: Mon, 20 Dec 2010 22:55:46 -0500
From: Andrew Lutomirski l...@mit.edu
Subject: Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks
To: Keith Packard kei...@keithp.com
Cc: intel-...@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Message-ID:
aanlktimk6rlkr8lt76cr8nclw_3kax6dqa+=id9_g...@mail.gmail.com
Content-Type: text/plain; charset=ISO-8859-1

On Mon, Dec 20, 2010 at 10:47 PM, Keith Packard kei...@keithp.com
wrote:

On Mon, 20 Dec 2010 22:34:12 -0500, Andrew Lutomirski
l...@mit.edu wrote:


But five seconds is a *long* time, and anything short enough that
the
interrupt actually gets turned off in normal use risks the same
race.


Right, so eliminating any race seems like

Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks

2010-12-21 Thread Mario Kleiner 
 drm_wait_vblank from
userspace for testing?  I know approximately nothing about libdrm or
any userspace graphics stuff whatsoever.

--Andy



--
keith.pack...@intel.com




--

Message: 5
Date: Mon, 20 Dec 2010 20:03:53 -0800
From: Jesse Barnes jbar...@virtuousgeek.org
Subject: Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks
To: Andrew Lutomirski l...@mit.edu
Cc: intel-...@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Message-ID: 20101220200353.12479...@jbarnes-desktop
Content-Type: text/plain; charset=US-ASCII

On Mon, 20 Dec 2010 22:55:46 -0500
Andrew Lutomirski l...@mit.edu wrote:

On Mon, Dec 20, 2010 at 10:47 PM, Keith Packard  
kei...@keithp.com wrote:
On Mon, 20 Dec 2010 22:34:12 -0500, Andrew Lutomirski  
l...@mit.edu wrote:


But five seconds is a *long* time, and anything short enough  
that the
interrupt actually gets turned off in normal use risks the same  
race.


Right, so eliminating any race seems like the basic requirement. Can
that be done?


I'll give it a shot.

Do you know if there's an existing tool to call drm_wait_vblank from
userspace for testing?  I know approximately nothing about libdrm or
any userspace graphics stuff whatsoever.


Yeah, libdrm has a test program called vbltest; it should do what you
need.



Not so fast please! After a lot of review by Jesse, Dave and Chris  
just merged a set of my patches into the drm-next (and the intel and  
radeon kms drivers) to implement precise timestamping of vblank's and  
pageflip completion and vblank counting for DRI2 and the  
OML_sync_control extension. It also fixes (hopefully) almost all race- 
conditions (that i could find or think of) related to vblank irq on/ 
off, a few of them surprising and due to funny behaviour of some  
gpu's when you enable/disable vblanks (e.g., radeon's spontaneously  
firing a vblank irq in the middle of a scanout cycle when vblank  
irq's get enabled, or firing the irq sometimes shortly before a  
vblank instead of in the vblank).


There's one tiny race left in the vblank off path, which i wanted to  
address during the next weeks. Also i need to implement support for  
nouveau. After that we could simply reduce the vblank off timeout to  
something small like 50 msecs. Or use Andrew's heuristic on top of this.


In any case, please check against the drm-next branch. I think your  
patches touch/conflict with most of the areas in drm that are  
modified in drm-next. At least my users need a very high level of  
precision and robustness in vblank counting and timestamping for  
neuro-science applications and similar stuff.


thanks,
-mario


*
Mario Kleiner
Max Planck Institute for Biological Cybernetics
Spemannstr. 38
72076 Tuebingen
Germany

e-mail: mario.klei...@tuebingen.mpg.de
office: +49 (0)7071/601-1623
fax:+49 (0)7071/601-616
www:http://www.kyb.tuebingen.mpg.de/~kleinerm
*
For a successful technology, reality must take precedence
over public relations, for Nature cannot be fooled.
(Richard Feynman)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm-vblank: Always return true vblank count of scheduled vblank event.

2010-12-13 Thread Mario Kleiner 
On Dec 10, 2010, at 6:45 PM, Jesse Barnes wrote:

> On Fri, 10 Dec 2010 16:00:19 +0100
> Mario Kleiner  wrote:
>
>>
>>
>>  Original Message 
>> Subject: [PATCH] drm-vblank: Always return true vblank count of
>> scheduled vblank event.
>> Date: Fri, 10 Dec 2010 15:58:10 +0100
>> From: Mario Kleiner 
>> To: airlied at gmail.com
>> CC: jbarnes at virtuousgeek.org, krh at bitplanet.net,   Mario Kleiner
>> 
>>
>> This patch tries to make sure that the vbl.reply.sequence
>> vblank count for a queued or emitted vblank event always
>> corresponds to the true vblank count of queueing/emission, so
>> the ddx can rely on the returned target_msc for consistency
>> checks and implementation of swap_intervals in glXSwapBuffers().
>>
>> Without this there is a small race-condition between the
>> userspace ddx queueing a vblank event and the vblank
>> counter incrementing before the event gets queued in
>> the kernel.
>>
>> Signed-off-by: Mario Kleiner 
>> ---
>>   drivers/gpu/drm/drm_irq.c |3 +++
>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
>> index 4e82d0d..55160d7 100644
>> --- a/drivers/gpu/drm/drm_irq.c
>> +++ b/drivers/gpu/drm/drm_irq.c
>> @@ -1086,15 +1086,18 @@ static int drm_queue_vblank_event(struct
>> drm_device *dev, int pipe,
>>
>>  e->event.sequence = vblwait->request.sequence;
>>  if ((seq - vblwait->request.sequence) <= (1 << 23)) {
>> +e->event.sequence = seq;
>>  e->event.tv_sec = now.tv_sec;
>>  e->event.tv_usec = now.tv_usec;
>>  drm_vblank_put(dev, e->pipe);
>>  list_add_tail(>base.link, >base.file_priv->event_list);
>>  wake_up_interruptible(>base.file_priv->event_wait);
>> +vblwait->reply.sequence = seq;
>>  trace_drm_vblank_event_delivered(current->pid, pipe,
>>   vblwait->request.sequence);
>
> But this actually causes us to return the current count rather than  
> the
> requested count if the event requested is in the past, right?
>

Yes. But i think that's what the spec wants. The wording in the spec  
is usually "...and then will return the current values for UST, MSC,
 and SBC...". If it just returned what was passed to it, it  
wouldn't provide any new information to the calling client.

Currently there are three uses for queuing of vblank events:

a) Copy-Swaps: Queue a vblank event for target_msc to trigger a blit  
once the event is received. Because we set the  
DRM_VBLANK_NEXT_ON_MISS flag when queuing this event, the requested  
target_msc is automatically adapted by the drm to current_msc + 1 and  
this new adapted value is returned to the ddx. So for copy swaps the  
kernel returns the true expected count and time of the swap. The  
above path isn't ever taken.

b) Pageflip-Swaps: Queue a vblank event for target_msc - 1, even if  
target_msc -1 is already over. This will deliver the event  
immediately if current_msc >= target_msc - 1, but without the patch  
it would deliver it with a vbl.reply.sequence that has nothing to do  
with the real count at swap.

So a) and b) with the old code don't have a negative effect on  
scheduling a swap, but b) can have a a negative effect for the  
scheduling of the next pageflipped swap: The ddx uses the returned  
value as last_swap_target to implement swap_interval correctly, so  
the next swap may not obey the swap_interval correctly if case b)  
delivered an outdated vbl.reply.sequence number.

c) WaitForMscOML: Like b), but here it would return the "wrong" count  
for a target_msc in the past. The client would get the information  
back that it passed into the call, instead of the information that  
reflects reality.

d) Some correctness check for pageflipping in the ddx that can work  
more reliable / detect more possible errors if the returned value is  
always precise.

>>  } else {
>>  list_add_tail(>base.link, >vblank_event_list);
>> +vblwait->reply.sequence = vblwait->request.sequence;
>>  }
>
> This one makes sense; we want to make sure the returned sequence is  
> the
> one requested (assuming it was in the future at the time of the  
> request
> anyway).
>

Yes, but the requested number gets adapted by the function if  
DRM_VBLANK_NEXT_ON_MISS is set. Also this statement is a bit  
redundant, because vblwait is a union iirc, so vblwait- 
 >reply.sequence and vblwait->request.sequence are actually the same  
field. It's just added for clarity.

-mario

Re: [PATCH] drm-vblank: Always return true vblank count of scheduled vblank event.

2010-12-13 Thread Mario Kleiner 

On Dec 10, 2010, at 6:45 PM, Jesse Barnes wrote:


On Fri, 10 Dec 2010 16:00:19 +0100
Mario Kleiner mario.klei...@tuebingen.mpg.de wrote:




 Original Message 
Subject: [PATCH] drm-vblank: Always return true vblank count of
scheduled vblank event.
Date: Fri, 10 Dec 2010 15:58:10 +0100
From: Mario Kleiner mario.klei...@tuebingen.mpg.de
To: airl...@gmail.com
CC: jbar...@virtuousgeek.org,   k...@bitplanet.net, Mario Kleiner
mario.klei...@tuebingen.mpg.de

This patch tries to make sure that the vbl.reply.sequence
vblank count for a queued or emitted vblank event always
corresponds to the true vblank count of queueing/emission, so
the ddx can rely on the returned target_msc for consistency
checks and implementation of swap_intervals in glXSwapBuffers().

Without this there is a small race-condition between the
userspace ddx queueing a vblank event and the vblank
counter incrementing before the event gets queued in
the kernel.

Signed-off-by: Mario Kleiner mario.klei...@tuebingen.mpg.de
---
  drivers/gpu/drm/drm_irq.c |3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 4e82d0d..55160d7 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1086,15 +1086,18 @@ static int drm_queue_vblank_event(struct
drm_device *dev, int pipe,

e-event.sequence = vblwait-request.sequence;
if ((seq - vblwait-request.sequence) = (1  23)) {
+   e-event.sequence = seq;
e-event.tv_sec = now.tv_sec;
e-event.tv_usec = now.tv_usec;
drm_vblank_put(dev, e-pipe);
list_add_tail(e-base.link, e-base.file_priv-event_list);
wake_up_interruptible(e-base.file_priv-event_wait);
+   vblwait-reply.sequence = seq;
trace_drm_vblank_event_delivered(current-pid, pipe,
 vblwait-request.sequence);


But this actually causes us to return the current count rather than  
the

requested count if the event requested is in the past, right?



Yes. But i think that's what the spec wants. The wording in the spec  
is usually ...and then will return the current values for UST, MSC,
and SBC If it just returned what was passed to it, it  
wouldn't provide any new information to the calling client.


Currently there are three uses for queuing of vblank events:

a) Copy-Swaps: Queue a vblank event for target_msc to trigger a blit  
once the event is received. Because we set the  
DRM_VBLANK_NEXT_ON_MISS flag when queuing this event, the requested  
target_msc is automatically adapted by the drm to current_msc + 1 and  
this new adapted value is returned to the ddx. So for copy swaps the  
kernel returns the true expected count and time of the swap. The  
above path isn't ever taken.


b) Pageflip-Swaps: Queue a vblank event for target_msc - 1, even if  
target_msc -1 is already over. This will deliver the event  
immediately if current_msc = target_msc - 1, but without the patch  
it would deliver it with a vbl.reply.sequence that has nothing to do  
with the real count at swap.


So a) and b) with the old code don't have a negative effect on  
scheduling a swap, but b) can have a a negative effect for the  
scheduling of the next pageflipped swap: The ddx uses the returned  
value as last_swap_target to implement swap_interval correctly, so  
the next swap may not obey the swap_interval correctly if case b)  
delivered an outdated vbl.reply.sequence number.


c) WaitForMscOML: Like b), but here it would return the wrong count  
for a target_msc in the past. The client would get the information  
back that it passed into the call, instead of the information that  
reflects reality.


d) Some correctness check for pageflipping in the ddx that can work  
more reliable / detect more possible errors if the returned value is  
always precise.



} else {
list_add_tail(e-base.link, dev-vblank_event_list);
+   vblwait-reply.sequence = vblwait-request.sequence;
}


This one makes sense; we want to make sure the returned sequence is  
the
one requested (assuming it was in the future at the time of the  
request

anyway).



Yes, but the requested number gets adapted by the function if  
DRM_VBLANK_NEXT_ON_MISS is set. Also this statement is a bit  
redundant, because vblwait is a union iirc, so vblwait- 
reply.sequence and vblwait-request.sequence are actually the same  
field. It's just added for clarity.


-mario

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm-vblank: Always return true vblank count of scheduled vblank event.

2010-12-10 Thread Mario Kleiner 


 Original Message 
Subject: [PATCH] drm-vblank: Always return true vblank count of 
scheduled vblank event.
Date: Fri, 10 Dec 2010 15:58:10 +0100
From: Mario Kleiner <mario.klei...@tuebingen.mpg.de>
To: airlied at gmail.com
CC: jbarnes at virtuousgeek.org,krh at bitplanet.net,   Mario Kleiner 


This patch tries to make sure that the vbl.reply.sequence
vblank count for a queued or emitted vblank event always
corresponds to the true vblank count of queueing/emission, so
the ddx can rely on the returned target_msc for consistency
checks and implementation of swap_intervals in glXSwapBuffers().

Without this there is a small race-condition between the
userspace ddx queueing a vblank event and the vblank
counter incrementing before the event gets queued in
the kernel.

Signed-off-by: Mario Kleiner 
---
  drivers/gpu/drm/drm_irq.c |3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 4e82d0d..55160d7 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1086,15 +1086,18 @@ static int drm_queue_vblank_event(struct 
drm_device *dev, int pipe,

e->event.sequence = vblwait->request.sequence;
if ((seq - vblwait->request.sequence) <= (1 << 23)) {
+   e->event.sequence = seq;
e->event.tv_sec = now.tv_sec;
e->event.tv_usec = now.tv_usec;
drm_vblank_put(dev, e->pipe);
list_add_tail(>base.link, >base.file_priv->event_list);
wake_up_interruptible(>base.file_priv->event_wait);
+   vblwait->reply.sequence = seq;
trace_drm_vblank_event_delivered(current->pid, pipe,
 vblwait->request.sequence);
} else {
list_add_tail(>base.link, >vblank_event_list);
+   vblwait->reply.sequence = vblwait->request.sequence;
}

spin_unlock_irqrestore(>event_lock, flags);
-- 
1.7.0.4

[PATCH] drm-vblank: Always return true vblank count of scheduled vblank event.

2010-12-10 Thread Mario Kleiner 



 Original Message 
Subject: [PATCH] drm-vblank: Always return true vblank count of 
scheduled vblank event.

Date: Fri, 10 Dec 2010 15:58:10 +0100
From: Mario Kleiner mario.klei...@tuebingen.mpg.de
To: airl...@gmail.com
CC: jbar...@virtuousgeek.org,	k...@bitplanet.net,	Mario Kleiner 
mario.klei...@tuebingen.mpg.de


This patch tries to make sure that the vbl.reply.sequence
vblank count for a queued or emitted vblank event always
corresponds to the true vblank count of queueing/emission, so
the ddx can rely on the returned target_msc for consistency
checks and implementation of swap_intervals in glXSwapBuffers().

Without this there is a small race-condition between the
userspace ddx queueing a vblank event and the vblank
counter incrementing before the event gets queued in
the kernel.

Signed-off-by: Mario Kleiner mario.klei...@tuebingen.mpg.de
---
 drivers/gpu/drm/drm_irq.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 4e82d0d..55160d7 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1086,15 +1086,18 @@ static int drm_queue_vblank_event(struct 
drm_device *dev, int pipe,


e-event.sequence = vblwait-request.sequence;
if ((seq - vblwait-request.sequence) = (1  23)) {
+   e-event.sequence = seq;
e-event.tv_sec = now.tv_sec;
e-event.tv_usec = now.tv_usec;
drm_vblank_put(dev, e-pipe);
list_add_tail(e-base.link, e-base.file_priv-event_list);
wake_up_interruptible(e-base.file_priv-event_wait);
+   vblwait-reply.sequence = seq;
trace_drm_vblank_event_delivered(current-pid, pipe,
 vblwait-request.sequence);
} else {
list_add_tail(e-base.link, dev-vblank_event_list);
+   vblwait-reply.sequence = vblwait-request.sequence;
}

spin_unlock_irqrestore(dev-event_lock, flags);
--
1.7.0.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

DRM Vblank timestamping patches. 1st Try.

2010-10-24 Thread Mario Kleiner 
Oh btw., the patches are against the current drm-radeon-testing tree,
applied after commit...

21c74a8ea8b47eb6c3c621e36578f6e27f65c5c7
"drm, kdb, kms: Change mode_set_base_atomic() enter argument to be an enum"

... and should apply cleanly.

thanks,
-mario

[PATCH 3/3] kms/i915: Add support for precise vblank timestamping.

2010-10-24 Thread Mario Kleiner 
This patch adds new functions for use by the drm core:

.get_vblank_timestamp() provides a precise timestamp
for the end of the most recent (or current) vblank
interval of a given crtc, as needed for the DRI2
implementation of the OML_sync_control extension.
It is a thin wrapper around the drm function
drm_calc_vbltimestamp_from_scanoutpos() which does
almost all the work.

.get_scanout_position() provides the current horizontal
and vertical video scanout position and "in vblank"
status of a given crtc, as needed by the drm for use by
drm_calc_vbltimestamp_from_scanoutpos().

The patch modifies the pageflip completion routine
to use these precise vblank timestamps as the timestamps
for pageflip completion events.

This code has been only tested on a HP-Mini Netbook with
Atom processor and Intel 945GME gpu. The codepath for
(IS_G4X(dev) || IS_IRONLAKE(dev) || IS_GEN6(dev)) gpu's
has not been tested so far due to lack of hardware.

Signed-off-by: Mario Kleiner 
---
 drivers/gpu/drm/i915/i915_drv.c  |2 +
 drivers/gpu/drm/i915/i915_drv.h  |7 +++
 drivers/gpu/drm/i915/i915_irq.c  |   92 ++
 drivers/gpu/drm/i915/intel_display.c |   31 +---
 drivers/gpu/drm/i915/intel_drv.h |1 +
 5 files changed, 126 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c3decb2..4d8184a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -603,6 +603,8 @@ static struct drm_driver driver = {
.device_is_agp = i915_driver_device_is_agp,
.enable_vblank = i915_enable_vblank,
.disable_vblank = i915_disable_vblank,
+   .get_vblank_timestamp = i915_get_vblank_timestamp,
+   .get_scanout_position = i915_get_crtc_scanoutpos,
.irq_preinstall = i915_driver_irq_preinstall,
.irq_postinstall = i915_driver_irq_postinstall,
.irq_uninstall = i915_driver_irq_uninstall,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 73ad8bf..276fbdb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -948,6 +948,13 @@ void
 i915_disable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask);

 void intel_enable_asle (struct drm_device *dev);
+int i915_get_vblank_timestamp(struct drm_device *dev, int crtc,
+ int *max_error,
+ struct timeval *vblank_time,
+ unsigned flags);
+
+int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
+int *vpos, int *hpos);

 #ifdef CONFIG_DEBUG_FS
 extern void i915_destroy_error_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 64c07c2..c9c8d4a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -243,6 +243,98 @@ u32 gm45_get_vblank_counter(struct drm_device *dev, int 
pipe)
return I915_READ(reg);
 }

+int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
+int *vpos, int *hpos)
+{
+   drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+   int reg;
+   u32 vbl = 0, position = 0;
+   int vbl_start, vbl_end, htotal, vtotal;
+   bool in_vbl = true;
+   int ret = 0;
+
+   if (!i915_pipe_enabled(dev, pipe)) {
+   DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled "
+   "pipe %d\n", pipe);
+   return 0;
+   }
+
+   /* Get vtotal. */
+   reg = pipe ? VTOTAL_B : VTOTAL_A;
+   vtotal = 1 + ((I915_READ(reg) >> 16) & 0x1fff);
+
+   if (IS_G4X(dev) || IS_IRONLAKE(dev) || IS_GEN6(dev)) {
+   /* No obvious pixelcount register. Only query vertical
+* scanout position from Display scan line register.
+*/
+   reg = pipe ? PIPEBDSL : PIPEADSL;
+   position = I915_READ(reg);
+
+   /* Decode into vertical scanout position. Don't have
+* horizontal scanout position.
+*/
+   *vpos = position & 0x1fff;
+   *hpos = 0;
+   } else {
+   /* Have access to pixelcount since start of frame.
+* We can split this into vertical and horizontal
+* scanout position.
+*/
+   reg = pipe ? PIPEBFRAMEPIXEL : PIPEAFRAMEPIXEL;
+   position = (I915_READ(reg) & PIPE_PIXEL_MASK) >> 
PIPE_PIXEL_SHIFT;
+
+   reg = pipe ? HTOTAL_B : HTOTAL_A;
+   htotal = 1 + ((I915_READ(reg) >> 16) & 0x1fff);
+   *vpos = position / htotal;
+   *hpos = position - (*vpos * htotal);
+   }
+
+   /* Query vblank area. */
+   reg = pipe ? VBLANK_B : VBLANK_A;
+   vbl

[PATCH 2/3] kms/radeon: Add support for precise vblank timestamping.

2010-10-24 Thread Mario Kleiner 
This patch adds new functions for use by the drm core:

.get_vblank_timestamp() provides a precise timestamp
for the end of the most recent (or current) vblank
interval of a given crtc, as needed for the DRI2
implementation of the OML_sync_control extension.

It is a thin wrapper around the drm function
drm_calc_vbltimestamp_from_scanoutpos() which does
almost all the work and is shared across drivers.

.get_scanout_position() provides the current horizontal
and vertical video scanout position and "in vblank"
status of a given crtc, as needed by the drm for use by
drm_calc_vbltimestamp_from_scanoutpos().

The function is also used by the dynamic gpu reclocking
code to determine when it is safe to reclock inside vblank.

For that purpose radeon_pm_in_vbl() is modified to
accomodate a small change in the function prototype of
the radeon_get_crtc_scanoutpos() which is hooked up to
.get_scanout_position().

This code has been tested on AVIVO hardware, a RV530
(ATI Mobility Radeon X1600) in a Intel Core-2 Duo MacBookPro
and some R600 variant (FireGL V7600) in a single cpu
AMD Athlon 64 PC.

Signed-off-by: Mario Kleiner 
---
 drivers/gpu/drm/radeon/radeon_display.c |   40 --
 drivers/gpu/drm/radeon/radeon_drv.c |8 ++
 drivers/gpu/drm/radeon/radeon_kms.c |   21 
 drivers/gpu/drm/radeon/radeon_mode.h|7 +
 drivers/gpu/drm/radeon/radeon_pm.c  |6 ++--
 5 files changed, 55 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
b/drivers/gpu/drm/radeon/radeon_display.c
index 0383631..37bfc48 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -1015,7 +1015,7 @@ bool radeon_crtc_scaling_mode_fixup(struct drm_crtc *crtc,
 /*
  * Retrieve current video scanout position of crtc on a given gpu.
  *
- * \param rdev Device to query.
+ * \param dev Device to query.
  * \param crtc Crtc to query.
  * \param *vpos Location where vertical scanout position should be stored.
  * \param *hpos Location where horizontal scanout position should go.
@@ -1027,72 +1027,74 @@ bool radeon_crtc_scaling_mode_fixup(struct drm_crtc 
*crtc,
  *
  * \return Flags, or'ed together as follows:
  *
- * RADEON_SCANOUTPOS_VALID = Query successfull.
- * RADEON_SCANOUTPOS_INVBL = Inside vblank.
- * RADEON_SCANOUTPOS_ACCURATE = Returned position is accurate. A lack of
+ * DRM_SCANOUTPOS_VALID = Query successfull.
+ * DRM_SCANOUTPOS_INVBL = Inside vblank.
+ * DRM_SCANOUTPOS_ACCURATE = Returned position is accurate. A lack of
  * this flag means that returned position may be offset by a constant but
  * unknown small number of scanlines wrt. real scanout position.
  *
  */
-int radeon_get_crtc_scanoutpos(struct radeon_device *rdev, int crtc, int 
*vpos, int *hpos)
+int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, int *vpos, 
int *hpos)
 {
u32 stat_crtc = 0, vbl = 0, position = 0;
int vbl_start, vbl_end, vtotal, ret = 0;
bool in_vbl = true;

+   struct radeon_device *rdev = dev->dev_private;
+
if (ASIC_IS_DCE4(rdev)) {
if (crtc == 0) {
vbl = RREG32(EVERGREEN_CRTC_V_BLANK_START_END +
 EVERGREEN_CRTC0_REGISTER_OFFSET);
position = RREG32(EVERGREEN_CRTC_STATUS_POSITION +
  EVERGREEN_CRTC0_REGISTER_OFFSET);
-   ret |= RADEON_SCANOUTPOS_VALID;
+   ret |= DRM_SCANOUTPOS_VALID;
}
if (crtc == 1) {
vbl = RREG32(EVERGREEN_CRTC_V_BLANK_START_END +
 EVERGREEN_CRTC1_REGISTER_OFFSET);
position = RREG32(EVERGREEN_CRTC_STATUS_POSITION +
  EVERGREEN_CRTC1_REGISTER_OFFSET);
-   ret |= RADEON_SCANOUTPOS_VALID;
+   ret |= DRM_SCANOUTPOS_VALID;
}
if (crtc == 2) {
vbl = RREG32(EVERGREEN_CRTC_V_BLANK_START_END +
 EVERGREEN_CRTC2_REGISTER_OFFSET);
position = RREG32(EVERGREEN_CRTC_STATUS_POSITION +
  EVERGREEN_CRTC2_REGISTER_OFFSET);
-   ret |= RADEON_SCANOUTPOS_VALID;
+   ret |= DRM_SCANOUTPOS_VALID;
}
if (crtc == 3) {
vbl = RREG32(EVERGREEN_CRTC_V_BLANK_START_END +
 EVERGREEN_CRTC3_REGISTER_OFFSET);
position = RREG32(EVERGREEN_CRTC_STATUS_POSITION +
  EVERGREEN_CRTC3_REGISTER_OFFSET);
-   ret |= RADEON_SCANOUTPOS_VALID;
+   ret |= DRM_SCANOUTPOS_VALID;
}

[PATCH 1/3] drm/vblank: Add support for precise vblank timestamping.

2010-10-24 Thread Mario Kleiner 
The DRI2 swap & sync implementation needs precise
vblank counts and precise timestamps corresponding
to those vblank counts. For conformance to the OpenML
OML_sync_control extension specification the DRM
timestamp associated with a vblank count should
correspond to the start of video scanout of the first
scanline of the video frame following the vblank
interval for that vblank count.

Therefore we need to carry around precise timestamps
for vblanks. Currently the DRM and KMS drivers generate
timestamps ad-hoc via do_gettimeofday() in some
places. The resulting timestamps are sometimes not
very precise due to interrupt handling delays, they
don't conform to OML_sync_control and some are wrong,
as they aren't taken synchronized to the vblank.

This patch implements support inside the drm core
for precise and robust timestamping. It consists
of the following interrelated pieces.

1. Vblank timestamp caching:

A per-crtc ringbuffer stores the most recent vblank
timestamps corresponding to vblank counts.

The ringbuffer can be read out lock-free via the
accessor function:

struct timeval timestamp;
vblankcount = drm_vblank_count_and_time(dev, crtcid, ).

The function returns the current vblank count and
the corresponding timestamp for start of video
scanout following the vblank interval. It can be
used anywhere between enclosing drm_vblank_get(dev, crtcid)
and drm_vblank_put(dev,crtcid) statements. It is used
inside the drmWaitVblank ioctl and in the vblank event
queueing and handling. It should be used by kms drivers for
timestamping of bufferswap completion.

The timestamp ringbuffer is reinitialized each time
vblank irq's get reenabled in drm_vblank_get()/
drm_update_vblank_count(). It is invalidated when
vblank irq's get disabled.

The ringbuffer is updated inside drm_handle_vblank()
at each vblank irq.

2. Calculation of precise vblank timestamps:

drm_get_last_vbltimestamp() is used to compute the
timestamp for the end of the most recent vblank (if
inside active scanout), or the expected end of the
current vblank interval (if called inside a vblank
interval). The function calls into a new optional kms
driver entry point dev->driver->get_vblank_timestamp()
which is supposed to provide the precise timestamp.
If a kms driver doesn't implement the entry point or
if the call fails, a simple do_gettimeofday() timestamp
is returned as crude approximation of the true vblank time.

A new drm module parameter drm.timestamp_precision_usec
allows to disable high precision timestamps (if set to
zero) or to specify the maximum acceptable error in
the timestamps in microseconds.

Kms drivers could implement their get_vblank_timestamp()
function in a gpu specific way, as long as returned
timestamps conform to OML_sync_control, e.g., by use
of gpu specific hardware timestamps.

Optionally, kms drivers can simply wrap and use the new
utility function drm_calc_vbltimestamp_from_scanoutpos().
This function calls a new optional kms driver function
dev->driver->get_scanout_position() which returns the
current horizontal and vertical video scanout position
of the crtc. The scanout position together with the
drm_display_timing of the current video mode is used
to calculate elapsed time relative to start of active scanout
for the current video frame. This elapsed time is subtracted
from the current do_gettimeofday() time to get the timestamp
corresponding to start of video scanout. Currently
non-interlaced, non-doublescan video modes, with or
without panel scaling are handled correctly. Interlaced/
doublescan modes are tbd in a future patch.

3. Filtering of redundant vblank irq's and removal of
some race-conditions in the vblank irq enable/disable path:

Some gpu's (e.g., Radeon R500/R600) send spurious vblank
irq's outside the vblank if vblank irq's get reenabled.
These get detected by use of the vblank timestamps and
filtered out to avoid miscounting of vblanks.

Some race-conditions between the vblank irq enable/disable
functions, the vblank irq handler and the gpu itself (updating
its hardware vblank counter in the "wrong" moment) are
fixed inside vblank_disable_and_save() and
drm_update_vblank_count() by use of the vblank timestamps and
a new spinlock dev->vblank_time_lock.

The time until vblank irq disable is now configurable via
a new drm module parameter drm.vblankoffdelay to allow
experimentation with timeouts that are much shorter than
the current 5 seconds and should allow longer vblank off
periods for better power savings.

Followup patches will use these new functions to
implement precise timestamping for the intel and radeon
kms drivers.

Signed-off-by: Mario Kleiner 
---
 drivers/gpu/drm/drm_crtc_helper.c |   13 +-
 drivers/gpu/drm/drm_irq.c |  563 +++--
 drivers/gpu/drm/drm_stub.c|   10 +
 include/drm/drmP.h|   93 ++
 include/drm/drm_crtc.h|9 +
 5 files changed, 662 insertions(+), 26 deletions(-)

DRM Vblank timestamping patches. 1st Try.

2010-10-24 Thread Mario Kleiner 
Hi all,

after obsessing over these for another month after xds 2010,
this is the first version of my vblank timestamping patches that
i'm not totally ashamed off.

1st patch modifies the drm vblank handling to maintain precise
timestamps of when vblanks happen. It timestamps the end of
each vblank interval (= start of active scanout of the following
frame), as required by the OpenML OML_sync_control extension spec,
so the DRI2 swap & sync bits should return proper timestamps to
gl clients. It also takes care of keeping timestamps and vblcounts
consistent across vblank irq on/off and hopefully fixes a few
off-by-one vblank counts due to race conditions in those on/off bits.

There is still one small race condition between the vblank on/off
bits and the gpu (updating its hardware vblank counter at the
wrong moment) which can cause miscounting of vblanks across
enable/disable. I have an idea how to fix that, based on these
patches but will need to tinker around with it a bit more. After
that we could reduce the vblankoffdelay from 5000 msecs to something
smaller like 100 msecs to save more power on desktops with
desktop composition enabled, where already a blinking cursor
in a terminal window or a ticking clock in the menu bar
can keep vblanks turned on all the time with a 5000 msecs timeout.

All the timestamping inside the drmWaitVblank ioctl() and in
vblank event handling now uses the timestamps calculated in this
patch.

2nd and 3rd patches add the new timestamping and scanout position
functions to the i915 and radeon drivers.

Wrt. what i showed you at xds, i made the following changes:

I've moved the actual timestamping routine as a helper function
into the core. kms drivers now optionally export the crtc scanout
position query function in addition to the vblank timestamping function
to the drm core. Whenever the core has to recalculate timestamps it
goes like:

core -> kmsdriver-timestamping() -> core-timestampinghelper() ->
kmsdriver-getscanoutposition()

The kms driver can implement its own version of the timestamping function,
e.g., using special hardware timestamping registers in some more recent
gpu's, or it can simply export a getscanoutposition() function with
standardized behaviour across drivers and call the timestamping routine
implemented in the core. The routine in the core works in non-interlaced,
non-doublescan video modes, with or without panel scaling. It can
compensate for any random delay up to 1 video refresh duration. For
more robustness one would need to use a gpu specific hardware method.

I also moved the calculation of the constants needed for timetamping
into the drm_crtc_helper routine, so they only get recalculated after
a mode switch, not at each vblank. At modeset time i cache the
adjusted_mode after validation/adjustment by the crtc and encoders
inside the drm_crtc struct. Needed to account for panel scaling, and
i have a few other ideas that would require that cached value.

More details in the different commit messages and the code, just
some more remarks.

@Jesse: I replaced the n extra dynamically allocated spin-locks for n
crtc's by one statically allocated lock shared by all crtc's and renamed
it. I think i also worked all your other suggestions into the patches.
Thanks a lot for your first review of the earlier version.

The intel driver uses the timestamps inside the finish pageflip function
for timestamps of bufferswap completion. I've tested with an Atom
Mini netbook with Intel 945 GME gpu and there it works perfect. I don't
have any other intel test machines, so the finish pageflip and scanout
query functions for ironlake, gen6 and g4x aren't tested, only coded
based on some of the intel manuals at xorg. In the pageflip irq
handlers there is this uncertainty if pageflip irq's get delivered
before or after the vblank irq's for the vblank interval of swap
completion. The i945 delivers pagflip irq's before handling the
vblank irq's so the completion handler needs to account for that.
I guessed from the code, which ones do deliver before/after vblank,
but that guesses may be wrong, so this needs testing.

@Alex: Thanks for your feedback at xds. I think the radeon part
should be as we discussed. The radeon patch builds onto what you
committed already into drm-radeon-testing for the reclocking fixes.
It slightly changes the interface between the scanout query function and
radeon_pm_in_vbl(), but doesn't change the implementation.

Testing of the timestamps on the Intel 945GME, a Radeon R500 and a R600
against external measurement equipment shows that the timestamps are
essentially perfect. Error wrt. to the external measurement is less
than 20 microseconds (for the radeon's) and jitter is +/- 1 usec. The
intel error is less than 0.5 msecs, but i can't test how much better
because my photo-diode can't do better and my usec precise equipment only
works with DVI, but the intel test machine only has a vga output.

Now this swapbuffers timestamping is only trustworthy

DRM Vblank timestamping patches. 1st Try.

2010-10-24 Thread Mario Kleiner 

Hi all,

after obsessing over these for another month after xds 2010,
this is the first version of my vblank timestamping patches that
i'm not totally ashamed off.

1st patch modifies the drm vblank handling to maintain precise
timestamps of when vblanks happen. It timestamps the end of
each vblank interval (= start of active scanout of the following
frame), as required by the OpenML OML_sync_control extension spec,
so the DRI2 swap  sync bits should return proper timestamps to
gl clients. It also takes care of keeping timestamps and vblcounts
consistent across vblank irq on/off and hopefully fixes a few
off-by-one vblank counts due to race conditions in those on/off bits.

There is still one small race condition between the vblank on/off
bits and the gpu (updating its hardware vblank counter at the
wrong moment) which can cause miscounting of vblanks across
enable/disable. I have an idea how to fix that, based on these
patches but will need to tinker around with it a bit more. After
that we could reduce the vblankoffdelay from 5000 msecs to something
smaller like 100 msecs to save more power on desktops with
desktop composition enabled, where already a blinking cursor
in a terminal window or a ticking clock in the menu bar
can keep vblanks turned on all the time with a 5000 msecs timeout.

All the timestamping inside the drmWaitVblank ioctl() and in
vblank event handling now uses the timestamps calculated in this
patch.

2nd and 3rd patches add the new timestamping and scanout position
functions to the i915 and radeon drivers.

Wrt. what i showed you at xds, i made the following changes:

I've moved the actual timestamping routine as a helper function
into the core. kms drivers now optionally export the crtc scanout
position query function in addition to the vblank timestamping function
to the drm core. Whenever the core has to recalculate timestamps it
goes like:

core - kmsdriver-timestamping() - core-timestampinghelper() -
kmsdriver-getscanoutposition()

The kms driver can implement its own version of the timestamping function,
e.g., using special hardware timestamping registers in some more recent
gpu's, or it can simply export a getscanoutposition() function with
standardized behaviour across drivers and call the timestamping routine
implemented in the core. The routine in the core works in non-interlaced,
non-doublescan video modes, with or without panel scaling. It can
compensate for any random delay up to 1 video refresh duration. For
more robustness one would need to use a gpu specific hardware method.

I also moved the calculation of the constants needed for timetamping
into the drm_crtc_helper routine, so they only get recalculated after
a mode switch, not at each vblank. At modeset time i cache the
adjusted_mode after validation/adjustment by the crtc and encoders
inside the drm_crtc struct. Needed to account for panel scaling, and
i have a few other ideas that would require that cached value.

More details in the different commit messages and the code, just
some more remarks.

@Jesse: I replaced the n extra dynamically allocated spin-locks for n
crtc's by one statically allocated lock shared by all crtc's and renamed
it. I think i also worked all your other suggestions into the patches.
Thanks a lot for your first review of the earlier version.

The intel driver uses the timestamps inside the finish pageflip function
for timestamps of bufferswap completion. I've tested with an Atom
Mini netbook with Intel 945 GME gpu and there it works perfect. I don't
have any other intel test machines, so the finish pageflip and scanout
query functions for ironlake, gen6 and g4x aren't tested, only coded
based on some of the intel manuals at xorg. In the pageflip irq
handlers there is this uncertainty if pageflip irq's get delivered
before or after the vblank irq's for the vblank interval of swap
completion. The i945 delivers pagflip irq's before handling the
vblank irq's so the completion handler needs to account for that.
I guessed from the code, which ones do deliver before/after vblank,
but that guesses may be wrong, so this needs testing.

@Alex: Thanks for your feedback at xds. I think the radeon part
should be as we discussed. The radeon patch builds onto what you
committed already into drm-radeon-testing for the reclocking fixes.
It slightly changes the interface between the scanout query function and
radeon_pm_in_vbl(), but doesn't change the implementation.

Testing of the timestamps on the Intel 945GME, a Radeon R500 and a R600
against external measurement equipment shows that the timestamps are
essentially perfect. Error wrt. to the external measurement is less
than 20 microseconds (for the radeon's) and jitter is +/- 1 usec. The
intel error is less than 0.5 msecs, but i can't test how much better
because my photo-diode can't do better and my usec precise equipment only
works with DVI, but the intel test machine only has a vga output.

Now this swapbuffers timestamping is only trustworthy for

[Bug 28383] New: Cloned screens with different res/refresh cause problems with mesa demos since new dri2 vsync.

2010-06-08 Thread Mario Kleiner 
On Jun 4, 2010, at 4:02 PM, dri-devel-request at lists.freedesktop.org  
wrote:
> Message: 2
> Date: Fri,  4 Jun 2010 04:28:02 -0700 (PDT)
> From: bugzilla-daemon at freedesktop.org
> Subject: [Bug 28383] New: Cloned screens with different res/refresh
>   cause   problems with mesa demos since new dri2 vsync.
> To: dri-devel at lists.freedesktop.org
> Message-ID: 
> Content-Type: text/plain; charset="UTF-8"
>
> https://bugs.freedesktop.org/show_bug.cgi?id=28383
>
>Summary: Cloned screens with different res/refresh cause
> problems with mesa demos since new dri2 vsync.
>Product: Mesa
>Version: git
>   Platform: x86 (IA32)
> OS/Version: Linux (All)
> Status: NEW
>   Severity: normal
>   Priority: medium
>  Component: Drivers/DRI/R600
> AssignedTo: dri-devel at lists.freedesktop.org
> ReportedBy: lists at andyfurniss.entadsl.com
>
>
> I think this is linked to the new dri2 vsync but mat be wrong...
>
> rv670 mesa git, ddx git xorg git (2 weeks ago) drt kernels.
>
> Not just latest drt and unrelated to tiling patches.
>
> I am starting with a TV connected @ 1024x768 + vga monitor @  
> 1920x1080 so the
> top left section of the monitor screen gets tv(50Hz) vsync -  
> monitor is 60Hz.
>
> If I start a mesa demo and it starts in the TV area it will sync to  
> 50HZ if I
> move it outside the top left area it will sync to 60Hz.
>
> The problem is if I then move it back to the TV area it will stop  
> rendering and
> become unresponsive. I can  it but the window will remain.  
> If I the
> move the window around enough I can get xserver to segfault.
>
> This doesn't happen with UMS or a drt from 29th April (uses old  
> vsync).
> It doesn't happen if I disable TV with xrandr.

Diagnosis by treatment: Can you try if the attached patch fixes the  
problem for you?

-- next part --
A non-text attachment was scrubbed...
Name: xserver-dualheadvsyncfix.patch
Type: application/octet-stream
Size: 828 bytes
Desc: not available
URL: 

-- next part --


best,
-mario

>
> [   489.682] 0: /home/andy/Src/Xorg-git/modular/bin/Xorg  
> (xorg_backtrace+0x3b)
> [0x80da21b]
> [   489.682] 1: /home/andy/Src/Xorg-git/modular/bin/Xorg (0x8048000 
> +0x59ca5)
> [0x80a1ca5]
> [   489.682] 2: (vdso) (__kernel_rt_sigreturn+0x0) [0xe40c]
> [   489.682] 3: /home/andy/Src/Xorg-git/modular/bin/Xorg  
> (LocalClient+0x41)
> [0x809d8d1]
> [   489.682] 4:
> /home/andy/Src/Xorg-git/modular/lib/xorg/modules/extensions/libdri2.so
> (0xb73ce000+0x309b) [0xb73d109b]
> [   489.682] 5: /home/andy/Src/Xorg-git/modular/bin/Xorg (0x8048000 
> +0x25198)
> [0x806d198]
> [   489.682] 6: /home/andy/Src/Xorg-git/modular/bin/Xorg (0x8048000 
> +0x1a86a)
> [0x806286a]
> [   489.682] 7: /lib/libc.so.6 (__libc_start_main+0xd0) [0xb7495380]
> [   489.682] 8: /home/andy/Src/Xorg-git/modular/bin/Xorg (0x8048000 
> +0x1a481)
> [0x8062481]
> [   489.682] Segmentation fault at address 0x18
>
> -- 
> Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi? 
> tab=email
> --- You are receiving this mail because: ---
> You are the assignee for the bug.
>
>
> --

Re: [Bug 28383] New: Cloned screens with different res/refresh cause problems with mesa demos since new dri2 vsync.

2010-06-08 Thread Mario Kleiner 
On Jun 4, 2010, at 4:02 PM, dri-devel-requ...@lists.freedesktop.org  
wrote:

Message: 2
Date: Fri,  4 Jun 2010 04:28:02 -0700 (PDT)
From: bugzilla-dae...@freedesktop.org
Subject: [Bug 28383] New: Cloned screens with different res/refresh
cause   problems with mesa demos since new dri2 vsync.
To: dri-devel@lists.freedesktop.org
Message-ID: bug-28383-...@http.bugs.freedesktop.org/
Content-Type: text/plain; charset=UTF-8

https://bugs.freedesktop.org/show_bug.cgi?id=28383

   Summary: Cloned screens with different res/refresh cause
problems with mesa demos since new dri2 vsync.
   Product: Mesa
   Version: git
  Platform: x86 (IA32)
OS/Version: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/DRI/R600
AssignedTo: dri-devel@lists.freedesktop.org
ReportedBy: li...@andyfurniss.entadsl.com


I think this is linked to the new dri2 vsync but mat be wrong...

rv670 mesa git, ddx git xorg git (2 weeks ago) drt kernels.

Not just latest drt and unrelated to tiling patches.

I am starting with a TV connected @ 1024x768 + vga monitor @  
1920x1080 so the
top left section of the monitor screen gets tv(50Hz) vsync -  
monitor is 60Hz.


If I start a mesa demo and it starts in the TV area it will sync to  
50HZ if I

move it outside the top left area it will sync to 60Hz.

The problem is if I then move it back to the TV area it will stop  
rendering and
become unresponsive. I can ctrlc it but the window will remain.  
If I the

move the window around enough I can get xserver to segfault.

This doesn't happen with UMS or a drt from 29th April (uses old  
vsync).

It doesn't happen if I disable TV with xrandr.


Diagnosis by treatment: Can you try if the attached patch fixes the  
problem for you?




xserver-dualheadvsyncfix.patch
Description: Binary data



best,
-mario



[   489.682] 0: /home/andy/Src/Xorg-git/modular/bin/Xorg  
(xorg_backtrace+0x3b)

[0x80da21b]
[   489.682] 1: /home/andy/Src/Xorg-git/modular/bin/Xorg (0x8048000 
+0x59ca5)

[0x80a1ca5]
[   489.682] 2: (vdso) (__kernel_rt_sigreturn+0x0) [0xe40c]
[   489.682] 3: /home/andy/Src/Xorg-git/modular/bin/Xorg  
(LocalClient+0x41)

[0x809d8d1]
[   489.682] 4:
/home/andy/Src/Xorg-git/modular/lib/xorg/modules/extensions/libdri2.so
(0xb73ce000+0x309b) [0xb73d109b]
[   489.682] 5: /home/andy/Src/Xorg-git/modular/bin/Xorg (0x8048000 
+0x25198)

[0x806d198]
[   489.682] 6: /home/andy/Src/Xorg-git/modular/bin/Xorg (0x8048000 
+0x1a86a)

[0x806286a]
[   489.682] 7: /lib/libc.so.6 (__libc_start_main+0xd0) [0xb7495380]
[   489.682] 8: /home/andy/Src/Xorg-git/modular/bin/Xorg (0x8048000 
+0x1a481)

[0x8062481]
[   489.682] Segmentation fault at address 0x18

--
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi? 
tab=email

--- You are receiving this mail because: ---
You are the assignee for the bug.


--


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Flickering screen in Neverball on drm-radeon-testing

2010-06-06 Thread Mario Kleiner 
On Jun 4, 2010, at 7:27 PM, Michel D?nzer wrote:

> On Mit, 2010-06-02 at 16:38 +0200, Mario Kleiner wrote:
>
> I think your analysis is spot on.
>
> The ideal solution would probably be to make the kernel block in the
> command stream (CS) submission ioctl if the CS renders to (and  
> from?) a
> buffer object (BO) which is involved in a pending swap.
>
> Meanwhile, the attached hacks for xf86-video-ati and Mesa seem to help
> here, YMMV.

I think that is what the Intel drm does. Your dri2-flicker-mesa.diff  
patch makes sense to me as a newbie with limited understanding. If  
this ends up calling DRI2GetBuffers... in the server, it will block  
in DRI2ThrottleClient until the swap completes.

I doubt that the dri2-flicker-xf86-video-ati.diff patch has a real  
effect? For a regular glXSwapBuffers() command, the code-path isn't  
taken, where you placed the DRI2BlockClient() call, so it can't have  
an effect. If it would be taken as part of some call to the new  
glXSwapBuffersMscOML() function, it would likely do bad things(tm)  
which are against the purpose of this new extension.

-mario

*********
Mario Kleiner
Max Planck Institute for Biological Cybernetics
Spemannstr. 38
72076 Tuebingen
Germany

e-mail: mario.kleiner at tuebingen.mpg.de
office: +49 (0)7071/601-1623
fax:+49 (0)7071/601-616
www:http://www.kyb.tuebingen.mpg.de/~kleinerm
*
"For a successful technology, reality must take precedence
over public relations, for Nature cannot be fooled."
(Richard Feynman)

Re: Flickering screen in Neverball on drm-radeon-testing

2010-06-06 Thread Mario Kleiner 

On Jun 4, 2010, at 7:27 PM, Michel Dänzer wrote:


On Mit, 2010-06-02 at 16:38 +0200, Mario Kleiner wrote:

I think your analysis is spot on.

The ideal solution would probably be to make the kernel block in the
command stream (CS) submission ioctl if the CS renders to (and  
from?) a

buffer object (BO) which is involved in a pending swap.

Meanwhile, the attached hacks for xf86-video-ati and Mesa seem to help
here, YMMV.


I think that is what the Intel drm does. Your dri2-flicker-mesa.diff  
patch makes sense to me as a newbie with limited understanding. If  
this ends up calling DRI2GetBuffers... in the server, it will block  
in DRI2ThrottleClient until the swap completes.


I doubt that the dri2-flicker-xf86-video-ati.diff patch has a real  
effect? For a regular glXSwapBuffers() command, the code-path isn't  
taken, where you placed the DRI2BlockClient() call, so it can't have  
an effect. If it would be taken as part of some call to the new  
glXSwapBuffersMscOML() function, it would likely do bad things(tm)  
which are against the purpose of this new extension.


-mario

*
Mario Kleiner
Max Planck Institute for Biological Cybernetics
Spemannstr. 38
72076 Tuebingen
Germany

e-mail: mario.klei...@tuebingen.mpg.de
office: +49 (0)7071/601-1623
fax:+49 (0)7071/601-616
www:http://www.kyb.tuebingen.mpg.de/~kleinerm
*
For a successful technology, reality must take precedence
over public relations, for Nature cannot be fooled.
(Richard Feynman)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Flickering screen in Neverball on drm-radeon-testing

2010-06-02 Thread Mario Kleiner 
> --
>
> Message: 1
> Date: Tue,  1 Jun 2010 12:25:42 -0700 (PDT)
> From: bugzilla-daemon at freedesktop.org
> Subject: [Bug 28341] Flickering screen in Neverball on
>   drm-radeon-testing
> To: dri-devel at lists.freedesktop.org
> Message-ID: <20100601192542.ACE5B1300E8 at annarchy.freedesktop.org>
> Content-Type: text/plain; charset="UTF-8"
>
> https://bugs.freedesktop.org/show_bug.cgi?id=28341
>
> --- Comment #3 from Magnus Jensen   
> 2010-06-01 12:25:42 PDT ---
> Sorry. ignore my last attachment. These  errors continue to spawn  
> and i don't
> think they are related to neverball. i'm going to recompile mesa  
> and ddx, and
> post another dmesg soon.
> Just waiting for a kernel compile to finish.
>

I saw a similar flickering with latest Xorg stack (mesa master,  
xserver, ddx etc. master) and the 2.6.34 tree from radeon testing  
with my own toolkit on a R600 gpu. This setup uses the new dri sync &  
swap bits and changes how glXSwapbuffers works.

My best hunch would be that submission of new rendering commands by  
the direct rendering client to the gpu doesn't block after a  
glXSwapbuffers() call until the bufferswap completed, i.e., some  
synchronization is missing there.

This shows flickering, but load and timing dependent...

 glXXX rendering commands to draw image.
glXSwapBuffers();
glBegin(GL_POINTS);
glVertex2i(10,10);
glEnd();
glFinish();
Take a  swap completion timestamp here.
glClear();
...more rendering commands

-> I use this glVertex2i,  glFinish() sequence to wait for swap  
completion and get a timestamp. This works on any os/gpu combo ever  
tested, but doesn't seem to work reliably anymore with the new radeon  
sync & swap bits in place. Display flickers, presumably because the  
glClear() executes almost immediately after the glXSwapBuffers is  
scheduled, and before the bufferswap has actually taken place ->  
Clear the backbuffer before swapping.

This however:

 glXXX rendering commands to draw image.
glXSwapBuffers();
glXWaitForSbcOML(...);
glClear();
...

does work, because the new glXWaitForSbcOML() blocks the client until  
swap completion, so glClear() can only get submitted to the gpu after  
the swap completed.

[Except that glXWaitForSbcOML itself currently has a bug, for which  
i'll send a patch for xorg master/1.8.1 soon].

Didn't do detailed testing, but maybe this points into the right  
direction?

best,
-mario


*
Mario Kleiner
Max Planck Institute for Biological Cybernetics
Spemannstr. 38
72076 Tuebingen
Germany

e-mail: mario.kleiner at tuebingen.mpg.de
office: +49 (0)7071/601-1623
fax:+49 (0)7071/601-616
www:http://www.kyb.tuebingen.mpg.de/~kleinerm
*
"For a successful technology, reality must take precedence
over public relations, for Nature cannot be fooled."
(Richard Feynman)

Re: Flickering screen in Neverball on drm-radeon-testing

2010-06-02 Thread Mario Kleiner 

--

Message: 1
Date: Tue,  1 Jun 2010 12:25:42 -0700 (PDT)
From: bugzilla-dae...@freedesktop.org
Subject: [Bug 28341] Flickering screen in Neverball on
drm-radeon-testing
To: dri-devel@lists.freedesktop.org
Message-ID: 20100601192542.ace5b130...@annarchy.freedesktop.org
Content-Type: text/plain; charset=UTF-8

https://bugs.freedesktop.org/show_bug.cgi?id=28341

--- Comment #3 from Magnus Jensen mag...@jensenligan.se  
2010-06-01 12:25:42 PDT ---
Sorry. ignore my last attachment. These  errors continue to spawn  
and i don't
think they are related to neverball. i'm going to recompile mesa  
and ddx, and

post another dmesg soon.
Just waiting for a kernel compile to finish.



I saw a similar flickering with latest Xorg stack (mesa master,  
xserver, ddx etc. master) and the 2.6.34 tree from radeon testing  
with my own toolkit on a R600 gpu. This setup uses the new dri sync   
swap bits and changes how glXSwapbuffers works.


My best hunch would be that submission of new rendering commands by  
the direct rendering client to the gpu doesn't block after a  
glXSwapbuffers() call until the bufferswap completed, i.e., some  
synchronization is missing there.


This shows flickering, but load and timing dependent...

 glXXX rendering commands to draw image.
glXSwapBuffers();
glBegin(GL_POINTS);
glVertex2i(10,10);
glEnd();
glFinish();
Take a  swap completion timestamp here.
glClear();
...more rendering commands

- I use this glVertex2i,  glFinish() sequence to wait for swap  
completion and get a timestamp. This works on any os/gpu combo ever  
tested, but doesn't seem to work reliably anymore with the new radeon  
sync  swap bits in place. Display flickers, presumably because the  
glClear() executes almost immediately after the glXSwapBuffers is  
scheduled, and before the bufferswap has actually taken place -  
Clear the backbuffer before swapping.


This however:

 glXXX rendering commands to draw image.
glXSwapBuffers();
glXWaitForSbcOML(...);
glClear();
...

does work, because the new glXWaitForSbcOML() blocks the client until  
swap completion, so glClear() can only get submitted to the gpu after  
the swap completed.


[Except that glXWaitForSbcOML itself currently has a bug, for which  
i'll send a patch for xorg master/1.8.1 soon].


Didn't do detailed testing, but maybe this points into the right  
direction?


best,
-mario


*
Mario Kleiner
Max Planck Institute for Biological Cybernetics
Spemannstr. 38
72076 Tuebingen
Germany

e-mail: mario.klei...@tuebingen.mpg.de
office: +49 (0)7071/601-1623
fax:+49 (0)7071/601-616
www:http://www.kyb.tuebingen.mpg.de/~kleinerm
*
For a successful technology, reality must take precedence
over public relations, for Nature cannot be fooled.
(Richard Feynman)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

<    1   2   3   4   5   6