Alex Deucher wrote:

On Sun, Feb 8, 2009 at 9:22 AM, Christian König wrote:

...

Hi Alex,

the attached patch implements a simple wait_vblank function, and utilize those in R600DisplayTexturedVideo just before draw_auto as Matthias suggested.

I am assuming that the Xv output goes to CRTC1 and redefining the DxCRTC_STATUS registers in r6xx_accel is very ugly, but it's at least a start and seems to work quite fine for me, reducing tearing in the output under a (for me) noticeable level.

This looks good, but you might want to look at stalling on a particular vline range rather than vblank in general like we did in the radeon driver. I suppose it's worth comparing which method works better.

...

Can i start cleaning up this mess a little bit? For example removing "#define uint32_t CARD32" or the double register definition in rhd_reg.h and r600_reg*.h.

I'm fine with cleaning up. I wanted to move to using std int types originally, but Egbert and others wanted to use CARD* types instead for compatibility with really old compilers that don't support standard int types.

On a side note, what is the policy with C++ comments in C code? I made a start in manually converting the // C++ comments in the AtomBIOS headers to /* C */ style, largely to get familiar with the code and reduce compile noise. But then I noticed that some contributors are adding new C++ comments to the driver source. Even though GCC tolerates C++ comments, this generates needless warnings, is technically wrong, and could also break on older compilers. I'd happily convert all such comments on a one-off basis, but there's no point if others insist on putting them in afresh. Tony.

Alex

-- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org

Am Sonntag, den 08.02.2009, 18:50 -0500 schrieb Alex Deucher:

This looks good, but you might want to look at stalling on a particular vline range rather than vblank in general like we did in the radeon driver. I suppose it's worth comparing which method works better. I thought about that also, but came to the conclusion that an simpler implementation is easier to do right now.

...

Can i start cleaning up this mess a little bit? For example removing "#define uint32_t CARD32" or the double register definition in rhd_reg.h and r600_reg*.h.

I'm fine with cleaning up. I wanted to move to using std int types originally, but Egbert and others wanted to use CARD* types instead for compatibility with really old compilers that don't support standard int types. AFAIK int*_t and uint*_t are from ISO C99, the problem with this standard is that it's not included in some C++ compiling environments (most noticeable with Microsofts Visual C++, which gives me a constant

My idea was creating a general function with a simple adaptive logic, something like: i want to do an operation which draws (linear) from vertical line y1 to y2, which takes x ms or to be more specific x lines (worse), please time that with the vertical scan out... The problem is that i don't have any idea how to measure how long it takes to make the complete drawing to framebuffer. Any idea for that? headache). Bye, Christian. -- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org

On Feb 08, 09 18:50:33 -0500, Alex Deucher wrote:

This looks good, but you might want to look at stalling on a particular vline range rather than vblank in general like we did in the radeon driver. I suppose it's worth comparing which method works better.

Comparing for a particular line is definitely better, because you can wait for the first line after the visible area, which might be earlier than vblank due to overscan.

I'm fine with cleaning up. I wanted to move to using std int types originally, but Egbert and others wanted to use CARD* types instead for compatibility with really old compilers that don't support standard int types.

I, personally, am perfectly fine with stdint.h types (though inttypes.h is supposed to behave identically and be supported on more architectures). If Egbert was against that, however, he probably has a reason for it. Matthias -- Matthias Hopf __ __ __ Maxfeldstr. 5 / 90409 Nuernberg (_ | | (_ |__ mat@mshopf.de Phone +49-911-74053-715 __) |_| __) |__ R & D www.mshopf.de -- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org

On Tue, Feb 10, 2009 at 4:43 PM, Christian König wrote:

Am Dienstag, den 10.02.2009, 19:01 +0100 schrieb Matthias Hopf:

...

Comparing for a particular line is definitely better, because you can wait for the first line after the visible area, which might be earlier than vblank due to overscan. Ok, implemented it as you suggested. Patch is attached.

...

You can push a type-3 packet that writes the current GPU 64bit counter into some memory address as soon as the pipeline is finished (and this actually even doesn't stall the engine). Is there some documentation of the type-3 packets available for r6xx? I've found the docs for r5xx, but can't find any thing mentioned about it in the r6xx isa and register documentations. Am i missing something?

The opcodes are in the headers we released, but the descriptions are part of the soon to be released programming guide.

What i had in mind was reading out the horizontal and vertical count register before draw_auto and after wait_3d_idle, so i could calc how much time a specific operation needs in terms of pixel clock and/or vertical scanout. For this i need copy operation from a register to a memory address, but i don't know if that is possible with an cp opcode.

You're still waiting more than you need to. You should set the start and stop vlines and just wait for the scanout be be outside of the that range. Alex -- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org

On Tue, Feb 10, 2009 at 5:59 PM, Christian König wrote:

Am Dienstag, den 10.02.2009, 17:32 -0500 schrieb Alex Deucher:

...

The opcodes are in the headers we released, but the descriptions are part of the soon to be released programming guide. Ok, so i can stop searching for this.

...

...

What i had in mind was reading out the horizontal and vertical count register before draw_auto and after wait_3d_idle, so i could calc how much time a specific operation needs in terms of pixel clock and/or vertical scanout. For this i need copy operation from a register to a memory address, but i don't know if that is possible with an cp opcode.

You're still waiting more than you need to. You should set the start and stop vlines and just wait for the scanout be be outside of the that range. Yeah, i know. The problem is that without documentation it's impossible for me to make IT_WAIT_REG_MEM use another compare operation than equal (the only one that i could figure out from other parts of the code).

You should have said so :) ALWAYS (0<<0) LT (1<<0) LE (2<<0) EQ (3<<0) NE (4<<0) GE (5<<0) GT (6<<0) here's ossman's patch for radeon as a reference: http://cgit.freedesktop.org/xorg/driver/xf86-video-ati/commit/?h=r6xx-r7xx-support&id=8e9ef8ff581892cbe1b7ea56d48b9a1abd70179d Alex -- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org

On Wed, Feb 11, 2009 at 5:35 PM, Christian König wrote:

Am Dienstag, den 10.02.2009, 18:13 -0500 schrieb Alex Deucher:

...

You should have said so :)

ALWAYS (0<<0) LT (1<<0) LE (2<<0) EQ (3<<0) NE (4<<0) GE (5<<0) GT (6<<0)

here's ossman's patch for radeon as a reference: http://cgit.freedesktop.org/xorg/driver/xf86-video-ati/commit/?h=r6xx-r7xx-support&id=8e9ef8ff581892cbe1b7ea56d48b9a1abd70179d

Not knowing that there is a (undocumented) register to wait for a vline range instead of a single line was also some part of the problem :-/

Attached are three patches: 1. The definition of the proper IT_WAIT_REG_MEM compare operators.

2. Implementation of wait_vline_range, also including a fix for a nasty bug in "ereg", in the radeon driver source the num parameter of the CP_PACKET0 macro isn't decremented by one, so the ereg function have to do it, but in the radeonhd source CP_PACKET0 decrements the parameter.

3. A little bit cleanup of the register definitions in rhd_reg.h and r600_reg_(r6xx|r7xx).h. So both files can be included at the same time.

Looks good. Pushed! thanks, Alex -- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org