Re: [Nouveau] [Patches][nouveau/kms]: Precise Vblank and pageflip timestamping v2

2012-02-21 Thread Mario Kleiner

On Feb 20, 2012, at 8:24 AM, Lucas Stach wrote:

Just updated versions of the patches send by Mario Kleiner. This  
ones are

rebased on top of the nouveau tree and updated according to the review
feedback.

This time hopefully the right ones.

Regards,
Lucas




Ben, thanks for the feedback. I've resteted the v2 of Lucas new  
reworked and rebased patches, everything fine and precise wrt. to  
timing precision.


For my personal education: Is my understanding correct that these  
registers are basically double-buffered, and the  
NV50_PDISPLAY_CRTC_C macro accesses the current state, whereas the  
NV50_PDISPLAY_CRTC_P macro provides/sets the programmed values that  
will be latched into the _C registers at some later time, e.g., modeset?


thanks,
-mario

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [Patches][nouveau/kms]: Precise Vblank and pageflip timestamping v2

2012-02-21 Thread Maarten Maathuis
On Tue, Feb 21, 2012 at 8:04 PM, Mario Kleiner
mario.klei...@tuebingen.mpg.de wrote:
 On Feb 20, 2012, at 8:24 AM, Lucas Stach wrote:

 Just updated versions of the patches send by Mario Kleiner. This ones are
 rebased on top of the nouveau tree and updated according to the review
 feedback.

 This time hopefully the right ones.

 Regards,
 Lucas



 Ben, thanks for the feedback. I've resteted the v2 of Lucas new reworked and
 rebased patches, everything fine and precise wrt. to timing precision.

 For my personal education: Is my understanding correct that these registers
 are basically double-buffered, and the NV50_PDISPLAY_CRTC_C macro accesses
 the current state, whereas the NV50_PDISPLAY_CRTC_P macro provides/sets the
 programmed values that will be latched into the _C registers at some later
 time, e.g., modeset?

 thanks,
 -mario


 ___
 Nouveau mailing list
 Nouveau@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/nouveau

The proposed registers show the value that has submitted (via the
modesetting/display fifo), but they haven't been put into effect yet
by the update command. The registers (both the current and proposed
ones) are only intended for reading. At least that is my
understanding, perhaps Ben can tell you more.

-- 
Far away from the primal instinct, the song seems to fade away, the
river get wider between your thoughts and the things we do and say.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [Patches][nouveau/kms]: Precise Vblank and pageflip timestamping v2

2012-02-21 Thread Maarten Maathuis
On Tue, Feb 21, 2012 at 8:26 PM, Maarten Maathuis madman2...@gmail.com wrote:
 On Tue, Feb 21, 2012 at 8:04 PM, Mario Kleiner
 mario.klei...@tuebingen.mpg.de wrote:
 On Feb 20, 2012, at 8:24 AM, Lucas Stach wrote:

 Just updated versions of the patches send by Mario Kleiner. This ones are
 rebased on top of the nouveau tree and updated according to the review
 feedback.

 This time hopefully the right ones.

 Regards,
 Lucas



 Ben, thanks for the feedback. I've resteted the v2 of Lucas new reworked and
 rebased patches, everything fine and precise wrt. to timing precision.

 For my personal education: Is my understanding correct that these registers
 are basically double-buffered, and the NV50_PDISPLAY_CRTC_C macro accesses
 the current state, whereas the NV50_PDISPLAY_CRTC_P macro provides/sets the
 programmed values that will be latched into the _C registers at some later
 time, e.g., modeset?

 thanks,
 -mario


 ___
 Nouveau mailing list
 Nouveau@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/nouveau

 The proposed registers show the value that has submitted (via the
 modesetting/display fifo), but they haven't been put into effect yet
 by the update command. The registers (both the current and proposed
 ones) are only intended for reading. At least that is my
 understanding, perhaps Ben can tell you more.

 --
 Far away from the primal instinct, the song seems to fade away, the
 river get wider between your thoughts and the things we do and say.

That should be have been submitted not has submitted.

-- 
Far away from the primal instinct, the song seems to fade away, the
river get wider between your thoughts and the things we do and say.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH 1/9] dri2: Fix can_exchange() to allow page-flipping on new servers.

2012-02-21 Thread Mario Kleiner

On 02/20/2012 11:27 AM, Michel Dänzer wrote:

On Mon, 2012-02-20 at 05:59 +0100, Mario Kleiner wrote:

On 02/16/2012 11:04 AM, Michel Dänzer wrote:

On Don, 2012-02-16 at 00:45 +0100, Mario Kleiner wrote:

can_exchange() fails on at least Xorg 1.12+. This fixes
it in the same way it was fixed in the ati   intel ddx.

Signed-off-by: Mario Kleinermario.klei...@tuebingen.mpg.de
---
   src/nouveau_dri2.c |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index 3aa5ec5..5b62425 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -160,7 +160,7 @@ can_exchange(DrawablePtr draw, PixmapPtr dst_pix, PixmapPtr 
src_pix)
return ((DRI2CanFlip(draw)   pNv-has_pageflip))
dst_pix-drawable.width == src_pix-drawable.width
dst_pix-drawable.height == src_pix-drawable.height
-   dst_pix-drawable.depth == src_pix-drawable.depth
+   dst_pix-drawable.bitsPerPixel == 
src_pix-drawable.bitsPerPixel
dst_pix-devKind == src_pix-devKind;
   }



Actually, it seems like the pixmap depths really should match, otherwise
one could end up with the front pixmap depth not matching the window
depth. Not sure that's a real problem right now, but it seems wonky at
least...

Have you investigated why the depths don't match?


Depends on the meaning of investigated: One of the pixmaps has depth
24 bits (the pixmap of the root window) the other 32 bits (as requested
from the client via DRI2GetBuffersWithFormat for RGBA8 visuals). Both
have 32 bpp. I checked what the intel and ati ddx do. The ati ddx always
checked for matching drawable.bitsPerPixel since kms pageflip support
was implemented. The intel ddx does the same, but the code and comments
suggests they tried both and checking for matching depths probably
didn't work:

cd xorg/drivers/xf86-video-intel/
git log -p e2615cdeef078dbd2e834b68c437f098a92b941d

So everybody does it like this currently, and it seems to work.


Right. I must have been incorrectly thinking of flipping as changing the
window pixmap.

I still have some doubts — e.g. why is an RGBA visual chosen for a
fullscreen window, and does this really have anything to do with changes
in xserver 1.12, or rather in Mesa — but I think the change itself is
okay.



You are right, it is not the xserver, but more likely Mesa. I tried the 
patch again with some debug output against xserver 1.10.4 (Ubuntu 11.10 
standard x-server) and the same happens. The FRONT_LEFT buffer is taken 
from the windows pixmap, which is depth 24, the BACK_LEFT buffer is 
allocated as a pixmap of 32 bits. So now i'm surprised it ever worked 
before...


Original testing of this nouveau patchset was around August 2011, since 
then i've also updated mesa to 7.11 as part of a distribution upgrade 
from Ubuntu 11.04 - 11.10.


Mesa requests 32 bit buffers as part of the BACK_LEFT attachment in 
DRI2GetBuffersWithFormat(). As far as i could trace it back, the code in 
src/gallium/state_trackers/dri/drm/dri2.c, dri2_drawable_get_buffers() 
mapped the PIPE_FORMAT_B8G8R8A8_UNORM (as requested by my toolkit) and 
also PIPE_FORMAT_B8G8R8X8_UNORM (from glxgears) to a DRI2 buffer format 
of 32 bits and it seems that that is used as .depth for the pixmaps 
drawable as well.


There's this related commit in mesa:

git log -p 576161289df68eedade591fbca4013329c9e5ded

which changed the logic a bit after Mesa 7.12. Also this one:

git log -p c72d7df16879e3210946ba92a7edc823815b6f16

So to support page flipping on different mesa versions i guess 
can_exchange needs to be lenient and check for the matching bitsPerPixel 
instead.


I will change the commit message and title of this patch to be less 
misleading about the cause of the problem.


I wonder if the depth field is still a good way to describe the buffer 
format, especially with things like depth 30 RGB10 scanout.


-mario
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau