Re: [Mesa3d-dev] gallium: add blitter
On Tue, 2009-12-15 at 09:40 -0800, Marek Olšák wrote: > On Tue, Dec 15, 2009 at 11:27 AM, Keith Whitwell wrote: > > On Tue, 2009-12-15 at 01:49 -0800, Marek Olšák wrote: > >> On Tue, Dec 15, 2009 at 10:28 AM, Keith Whitwell wrote: > >> > On Mon, 2009-12-14 at 16:31 -0800, Marek Olšák wrote: > >> >> On Mon, Dec 14, 2009 at 5:42 PM, Corbin Simpson > >> >> wrote: > >> >> > As far as immediate verts, why don't we just add support to r300g to > >> >> > switch > >> >> > to immediate mode for small VBOs? > >> >> > > >> >> > Posting from a mobile, pardon my terseness. ~ C. > >> >> > > >> >> > >> >> Corbin, > >> >> > >> >> that seems reasonable, and it's the reason I killed the draw_quad > >> >> function. BTW immediate mode doubles the performance in glxgears. > >> > > >> > Shouldn't gears upload its vertices one time only and then leave them > >> > resident in VRAM? > >> > > >> > Keith > >> > > >> > > >> > >> I meant the clear path with a quad using immediate mode, that made the > >> difference in comparison with a straightforward VBO path. > > > > OK, understood. > > > > I'm interested to understand why the VBO path is slow. Is it the > > allocation of the VBO itself? Mapping & uploading the vertices? > > Command submission (the extra reloc)? > > > > It seems to me that none of these should be taking half your time. In > > fact, it makes me wonder if there wasn't something else going on -- eg. > > was the same buffer getting reused to hold those 4 vertices frame after > > frame, resulting in a sync when the driver tried to map the buffer? > > > > That could be happening in the state tracker or in the driver, I guess. > > > > Keith > > > > Yes, the same buffer was getting reused every frame causing sync when > mapping the buffer. However, it will no longer be an issue once > immediate mode gets implemented. Using DONT_WAIT would probably also > help here. > > Please, could you possibly look at the patches I sent to you > approximately 16 hours ago? (there are 7 of them) The patches look fine to me, Marek, thanks for fixing them up. Keith -- This SF.Net email is sponsored by the Verizon Developer Community Take advantage of Verizon's best-in-class app development support A streamlined, 14 day to market process makes app distribution fast and easy Join now and get one step closer to millions of Verizon customers http://p.sf.net/sfu/verizon-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Tue, Dec 15, 2009 at 11:27 AM, Keith Whitwell wrote: > On Tue, 2009-12-15 at 01:49 -0800, Marek Olšák wrote: >> On Tue, Dec 15, 2009 at 10:28 AM, Keith Whitwell wrote: >> > On Mon, 2009-12-14 at 16:31 -0800, Marek Olšák wrote: >> >> On Mon, Dec 14, 2009 at 5:42 PM, Corbin Simpson >> >> wrote: >> >> > As far as immediate verts, why don't we just add support to r300g to >> >> > switch >> >> > to immediate mode for small VBOs? >> >> > >> >> > Posting from a mobile, pardon my terseness. ~ C. >> >> > >> >> >> >> Corbin, >> >> >> >> that seems reasonable, and it's the reason I killed the draw_quad >> >> function. BTW immediate mode doubles the performance in glxgears. >> > >> > Shouldn't gears upload its vertices one time only and then leave them >> > resident in VRAM? >> > >> > Keith >> > >> > >> >> I meant the clear path with a quad using immediate mode, that made the >> difference in comparison with a straightforward VBO path. > > OK, understood. > > I'm interested to understand why the VBO path is slow. Is it the > allocation of the VBO itself? Mapping & uploading the vertices? > Command submission (the extra reloc)? > > It seems to me that none of these should be taking half your time. In > fact, it makes me wonder if there wasn't something else going on -- eg. > was the same buffer getting reused to hold those 4 vertices frame after > frame, resulting in a sync when the driver tried to map the buffer? > > That could be happening in the state tracker or in the driver, I guess. > > Keith > Yes, the same buffer was getting reused every frame causing sync when mapping the buffer. However, it will no longer be an issue once immediate mode gets implemented. Using DONT_WAIT would probably also help here. Please, could you possibly look at the patches I sent to you approximately 16 hours ago? (there are 7 of them) Marek -- This SF.Net email is sponsored by the Verizon Developer Community Take advantage of Verizon's best-in-class app development support A streamlined, 14 day to market process makes app distribution fast and easy Join now and get one step closer to millions of Verizon customers http://p.sf.net/sfu/verizon-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Tue, 2009-12-15 at 10:27 +, Keith Whitwell wrote: > On Tue, 2009-12-15 at 01:49 -0800, Marek Olšák wrote: > > On Tue, Dec 15, 2009 at 10:28 AM, Keith Whitwell wrote: > > > On Mon, 2009-12-14 at 16:31 -0800, Marek Olšák wrote: > > >> On Mon, Dec 14, 2009 at 5:42 PM, Corbin Simpson > > >> wrote: > > >> > As far as immediate verts, why don't we just add support to r300g to > > >> > switch > > >> > to immediate mode for small VBOs? > > >> > > > >> > Posting from a mobile, pardon my terseness. ~ C. > > >> > > > >> > > >> Corbin, > > >> > > >> that seems reasonable, and it's the reason I killed the draw_quad > > >> function. BTW immediate mode doubles the performance in glxgears. > > > > > > Shouldn't gears upload its vertices one time only and then leave them > > > resident in VRAM? > > > > > > Keith > > > > > > > > > > I meant the clear path with a quad using immediate mode, that made the > > difference in comparison with a straightforward VBO path. > > OK, understood. > > I'm interested to understand why the VBO path is slow. Is it the > allocation of the VBO itself? Mapping & uploading the vertices? > Command submission (the extra reloc)? > > It seems to me that none of these should be taking half your time. In > fact, it makes me wonder if there wasn't something else going on -- eg. > was the same buffer getting reused to hold those 4 vertices frame after > frame, resulting in a sync when the driver tried to map the buffer? > > That could be happening in the state tracker or in the driver, I guess. > > Keith -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 16:31 -0800, Marek Olšák wrote: > On Mon, Dec 14, 2009 at 5:42 PM, Corbin Simpson > wrote: > > As far as immediate verts, why don't we just add support to r300g to switch > > to immediate mode for small VBOs? > > > > Posting from a mobile, pardon my terseness. ~ C. > > > > Corbin, > > that seems reasonable, and it's the reason I killed the draw_quad > function. BTW immediate mode doubles the performance in glxgears. > > To others: > I noticed that there is a weird optimization in u_gen_mipmaps. It > allocates a large vertex buffer and uses small chunks of it to render > consecutive quads (one for each mipmap level and cubemap face). If we > implement switching to immediate mode, it would be nice for VBOs to be > as small as possible so that the driver can easily recognize the most > efficient path. The simplest solution (4 vertices in a VBO) may end up > being the fastest one here. Note that if we used user_buffers for this, the driver will be involved in uploading the data anyway and can make a decision at that point whether to use an immediate path. Keith -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 16:31 -0800, Marek Olšák wrote: > On Mon, Dec 14, 2009 at 5:42 PM, Corbin Simpson > wrote: > > As far as immediate verts, why don't we just add support to r300g to switch > > to immediate mode for small VBOs? > > > > Posting from a mobile, pardon my terseness. ~ C. > > > > Corbin, > > that seems reasonable, and it's the reason I killed the draw_quad > function. BTW immediate mode doubles the performance in glxgears. Shouldn't gears upload its vertices one time only and then leave them resident in VRAM? Keith -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 16:31 -0800, Marek Olšák wrote: > On Mon, Dec 14, 2009 at 5:42 PM, Corbin Simpson > wrote: > > As far as immediate verts, why don't we just add support to r300g to switch > > to immediate mode for small VBOs? > > > > Posting from a mobile, pardon my terseness. ~ C. > > > > Corbin, > > that seems reasonable, and it's the reason I killed the draw_quad > function. BTW immediate mode doubles the performance in glxgears. > > To others: > I noticed that there is a weird optimization in u_gen_mipmaps. It > allocates a large vertex buffer and uses small chunks of it to render > consecutive quads (one for each mipmap level and cubemap face). If we > implement switching to immediate mode, it would be nice for VBOs to be > as small as possible so that the driver can easily recognize the most > efficient path. The simplest solution (4 vertices in a VBO) may end up > being the fastest one here. I think it was a bit of a bad optimization -- to make it really work properly we'd want to map_buffer_range() with DONT_WAIT or similar. It was intended to avoid lots of buffer creations, but we ended up getting flushes/waits when we try to append vertices to a VBO which is already in flight. We don't get enough notification to avoid this, so the only way to fix is with map_buffer_range() using DONT_WAIT or similar. On balance, it would be better just to pass the vertices in a user buffer and let the driver decide how to deal with them. Keith -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, Dec 14, 2009 at 5:42 PM, Corbin Simpson wrote: > As far as immediate verts, why don't we just add support to r300g to switch > to immediate mode for small VBOs? > > Posting from a mobile, pardon my terseness. ~ C. > Corbin, that seems reasonable, and it's the reason I killed the draw_quad function. BTW immediate mode doubles the performance in glxgears. To others: I noticed that there is a weird optimization in u_gen_mipmaps. It allocates a large vertex buffer and uses small chunks of it to render consecutive quads (one for each mipmap level and cubemap face). If we implement switching to immediate mode, it would be nice for VBOs to be as small as possible so that the driver can easily recognize the most efficient path. The simplest solution (4 vertices in a VBO) may end up being the fastest one here. Marek -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 09:28 -0800, Keith Whitwell wrote:
> On Mon, 2009-12-14 at 08:51 -0800, José Fonseca wrote:
> > On Mon, 2009-12-14 at 08:22 -0800, Keith Whitwell wrote:
> > > On Mon, 2009-12-14 at 08:19 -0800, Keith Whitwell wrote:
> > > > On Mon, 2009-12-14 at 08:04 -0800, José Fonseca wrote:
> > > > > On Mon, 2009-12-14 at 05:39 -0800, Keith Whitwell wrote:
> > > > > > On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
> > > > > > >
> > > > > > > +static INLINE
> > > > > > > +void util_blitter_save_fragment_sampler_states(
> > > > > > > + struct blitter_context *blitter,
> > > > > > > + int num_sampler_states,
> > > > > > > + void **sampler_states)
> > > > > > > +{
> > > > > > > + assert(num_textures <= 32);
> > > > > > > +
> > > > > > > + blitter->saved_num_sampler_states = num_sampler_states;
> > > > > > > + memcpy(blitter->saved_sampler_states, sampler_states,
> > > > > > > + num_sampler_states * sizeof(void *));
> > > > > > > +}
> > > > > > > +
> > > > > >
> > > > > > Have you tried compiling with debug enabled? The assert above
> > > > > > fails to
> > > > > > compile. Also, can you use Elements() or similar instead of the
> > > > > > hard-coded 32?
> > > > > >
> > > > > > Maybe we can figure out how to go back to having asserts keep
> > > > > > exposing
> > > > > > their contents to the compiler even on non-debug builds. This used
> > > > > > to
> > > > > > work without problem on linux and helped a lot to avoid these type
> > > > > > of
> > > > > > problems.
> > > > >
> > > > > I wouldn't say without a problem: defining assert(expr) as (void)0
> > > > > instead of (void)(expr) on release builds yielded a non-negligible
> > > > > performance improvement. I don't recall the exact figure, but I
> > > > > believe
> > > > > it was the 3-5% for the driver I was benchmarking at the time. YMMV.
> > > > > Different drivers will give different results, but there's nothing
> > > > > platform specific about this.
> > > >
> > > > It's not hard to avoid excuting code... For instance we could always
> > > > have it translated to something like:
> > > >
> > > > if (0) {
> > > > (void)(expr);
> > > > }
> > > > (void)(0)
> > > >
> > >
> > > Obviously I would have meant to say something cleaner like:
> > >
> > > do {
> > >if (0) { (void)(expr); }
> > > }
> > > while (0)
> >
> > This only works if expr has no calls, or just inline calls. Using my
> > earlier example, if very_expensive_check() is in another file then the
> > compiler has to assume the function will have side effects, and the call
> > can't be removed.
>
> What call?!?
>
> if (0) do_something_with_side_effects();
>
> Has no side effects.
Nevermind. Don't know what I was thinking.
Jose
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 08:51 -0800, José Fonseca wrote:
> On Mon, 2009-12-14 at 08:22 -0800, Keith Whitwell wrote:
> > On Mon, 2009-12-14 at 08:19 -0800, Keith Whitwell wrote:
> > > On Mon, 2009-12-14 at 08:04 -0800, José Fonseca wrote:
> > > > On Mon, 2009-12-14 at 05:39 -0800, Keith Whitwell wrote:
> > > > > On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
> > > > > >
> > > > > > +static INLINE
> > > > > > +void util_blitter_save_fragment_sampler_states(
> > > > > > + struct blitter_context *blitter,
> > > > > > + int num_sampler_states,
> > > > > > + void **sampler_states)
> > > > > > +{
> > > > > > + assert(num_textures <= 32);
> > > > > > +
> > > > > > + blitter->saved_num_sampler_states = num_sampler_states;
> > > > > > + memcpy(blitter->saved_sampler_states, sampler_states,
> > > > > > + num_sampler_states * sizeof(void *));
> > > > > > +}
> > > > > > +
> > > > >
> > > > > Have you tried compiling with debug enabled? The assert above fails
> > > > > to
> > > > > compile. Also, can you use Elements() or similar instead of the
> > > > > hard-coded 32?
> > > > >
> > > > > Maybe we can figure out how to go back to having asserts keep exposing
> > > > > their contents to the compiler even on non-debug builds. This used to
> > > > > work without problem on linux and helped a lot to avoid these type of
> > > > > problems.
> > > >
> > > > I wouldn't say without a problem: defining assert(expr) as (void)0
> > > > instead of (void)(expr) on release builds yielded a non-negligible
> > > > performance improvement. I don't recall the exact figure, but I believe
> > > > it was the 3-5% for the driver I was benchmarking at the time. YMMV.
> > > > Different drivers will give different results, but there's nothing
> > > > platform specific about this.
> > >
> > > It's not hard to avoid excuting code... For instance we could always
> > > have it translated to something like:
> > >
> > > if (0) {
> > > (void)(expr);
> > > }
> > > (void)(0)
> > >
> >
> > Obviously I would have meant to say something cleaner like:
> >
> > do {
> >if (0) { (void)(expr); }
> > }
> > while (0)
>
> This only works if expr has no calls, or just inline calls. Using my
> earlier example, if very_expensive_check() is in another file then the
> compiler has to assume the function will have side effects, and the call
> can't be removed.
What call?!?
if (0) do_something_with_side_effects();
Has no side effects.
Keith
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
José Fonseca pisze:
> On Mon, 2009-12-14 at 08:58 -0800, michal wrote:
>
>> José Fonseca pisze:
>>
>>> On Mon, 2009-12-14 at 08:22 -0800, Keith Whitwell wrote:
>>>
>>>
On Mon, 2009-12-14 at 08:19 -0800, Keith Whitwell wrote:
> On Mon, 2009-12-14 at 08:04 -0800, José Fonseca wrote:
>
>
>> On Mon, 2009-12-14 at 05:39 -0800, Keith Whitwell wrote:
>>
>>
>>> On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
>>>
>>>
+static INLINE
+void util_blitter_save_fragment_sampler_states(
+ struct blitter_context *blitter,
+ int num_sampler_states,
+ void **sampler_states)
+{
+ assert(num_textures <= 32);
+
+ blitter->saved_num_sampler_states = num_sampler_states;
+ memcpy(blitter->saved_sampler_states, sampler_states,
+ num_sampler_states * sizeof(void *));
+}
+
>>> Have you tried compiling with debug enabled? The assert above fails to
>>> compile. Also, can you use Elements() or similar instead of the
>>> hard-coded 32?
>>>
>>> Maybe we can figure out how to go back to having asserts keep exposing
>>> their contents to the compiler even on non-debug builds. This used to
>>> work without problem on linux and helped a lot to avoid these type of
>>> problems.
>>>
>>>
>> I wouldn't say without a problem: defining assert(expr) as (void)0
>> instead of (void)(expr) on release builds yielded a non-negligible
>> performance improvement. I don't recall the exact figure, but I believe
>> it was the 3-5% for the driver I was benchmarking at the time. YMMV.
>> Different drivers will give different results, but there's nothing
>> platform specific about this.
>>
>>
> It's not hard to avoid excuting code... For instance we could always
> have it translated to something like:
>
> if (0) {
> (void)(expr);
> }
> (void)(0)
>
>
>
Obviously I would have meant to say something cleaner like:
do {
if (0) { (void)(expr); }
}
while (0)
>>> This only works if expr has no calls, or just inline calls. Using my
>>> earlier example, if very_expensive_check() is in another file then the
>>> compiler has to assume the function will have side effects, and the call
>>> can't be removed.
>>>
>>> I'm not sure __assume keyword that Michal mentioned helps. It's more a
>>> hint to the compiler to help him optimize code around the assertion, but
>>> perhaps it helps with the warnings too.
>>>
>>>
>>>
>> If I try to compile this:
>>
>> __assume(lalala);
>>
>> I get:
>>
>> error C2065: 'lalala' : undeclared identifier
>>
>> On the other side, the compiler is going to be serious about the
>> assumptions inside __assume(), and if they happen to be false, the
>> application can behave not as expected. This is against current gallium
>> paradigm, where we put assertions, but also do the same check in
>> non-debug builds to early out from a function or provide default values
>> (e.g. in switch-case statements).
>>
>
> Bummer... that's no good.
>
>
>
On the third hand, we could transform the following idiom
switch (foo) {
case 1:
bar = 22;
default:
assert(0);
bar = 11; /* Safe value. */
}
to use some flavour of assert() that doesn't get substituted with
__assume() on non-debug builds. Something like weak_assert() or
warning(). Then assert() could be used in places where there is no
backup plan and the app is going to crash anyway.
Or... do the opposite and introduce strong_assert() that translates to
__assume() and leave assert() as it is now.
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 08:58 -0800, michal wrote:
> José Fonseca pisze:
> > On Mon, 2009-12-14 at 08:22 -0800, Keith Whitwell wrote:
> >
> >> On Mon, 2009-12-14 at 08:19 -0800, Keith Whitwell wrote:
> >>
> >>> On Mon, 2009-12-14 at 08:04 -0800, José Fonseca wrote:
> >>>
> On Mon, 2009-12-14 at 05:39 -0800, Keith Whitwell wrote:
>
> > On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
> >
> >> +static INLINE
> >> +void util_blitter_save_fragment_sampler_states(
> >> + struct blitter_context *blitter,
> >> + int num_sampler_states,
> >> + void **sampler_states)
> >> +{
> >> + assert(num_textures <= 32);
> >> +
> >> + blitter->saved_num_sampler_states = num_sampler_states;
> >> + memcpy(blitter->saved_sampler_states, sampler_states,
> >> + num_sampler_states * sizeof(void *));
> >> +}
> >> +
> >>
> > Have you tried compiling with debug enabled? The assert above fails to
> > compile. Also, can you use Elements() or similar instead of the
> > hard-coded 32?
> >
> > Maybe we can figure out how to go back to having asserts keep exposing
> > their contents to the compiler even on non-debug builds. This used to
> > work without problem on linux and helped a lot to avoid these type of
> > problems.
> >
> I wouldn't say without a problem: defining assert(expr) as (void)0
> instead of (void)(expr) on release builds yielded a non-negligible
> performance improvement. I don't recall the exact figure, but I believe
> it was the 3-5% for the driver I was benchmarking at the time. YMMV.
> Different drivers will give different results, but there's nothing
> platform specific about this.
>
> >>> It's not hard to avoid excuting code... For instance we could always
> >>> have it translated to something like:
> >>>
> >>> if (0) {
> >>> (void)(expr);
> >>> }
> >>> (void)(0)
> >>>
> >>>
> >> Obviously I would have meant to say something cleaner like:
> >>
> >> do {
> >>if (0) { (void)(expr); }
> >> }
> >> while (0)
> >>
> >
> > This only works if expr has no calls, or just inline calls. Using my
> > earlier example, if very_expensive_check() is in another file then the
> > compiler has to assume the function will have side effects, and the call
> > can't be removed.
> >
> > I'm not sure __assume keyword that Michal mentioned helps. It's more a
> > hint to the compiler to help him optimize code around the assertion, but
> > perhaps it helps with the warnings too.
> >
> >
> If I try to compile this:
>
> __assume(lalala);
>
> I get:
>
> error C2065: 'lalala' : undeclared identifier
>
> On the other side, the compiler is going to be serious about the
> assumptions inside __assume(), and if they happen to be false, the
> application can behave not as expected. This is against current gallium
> paradigm, where we put assertions, but also do the same check in
> non-debug builds to early out from a function or provide default values
> (e.g. in switch-case statements).
Bummer... that's no good.
Jose
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
José Fonseca pisze:
> On Mon, 2009-12-14 at 08:22 -0800, Keith Whitwell wrote:
>
>> On Mon, 2009-12-14 at 08:19 -0800, Keith Whitwell wrote:
>>
>>> On Mon, 2009-12-14 at 08:04 -0800, José Fonseca wrote:
>>>
On Mon, 2009-12-14 at 05:39 -0800, Keith Whitwell wrote:
> On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
>
>> +static INLINE
>> +void util_blitter_save_fragment_sampler_states(
>> + struct blitter_context *blitter,
>> + int num_sampler_states,
>> + void **sampler_states)
>> +{
>> + assert(num_textures <= 32);
>> +
>> + blitter->saved_num_sampler_states = num_sampler_states;
>> + memcpy(blitter->saved_sampler_states, sampler_states,
>> + num_sampler_states * sizeof(void *));
>> +}
>> +
>>
> Have you tried compiling with debug enabled? The assert above fails to
> compile. Also, can you use Elements() or similar instead of the
> hard-coded 32?
>
> Maybe we can figure out how to go back to having asserts keep exposing
> their contents to the compiler even on non-debug builds. This used to
> work without problem on linux and helped a lot to avoid these type of
> problems.
>
I wouldn't say without a problem: defining assert(expr) as (void)0
instead of (void)(expr) on release builds yielded a non-negligible
performance improvement. I don't recall the exact figure, but I believe
it was the 3-5% for the driver I was benchmarking at the time. YMMV.
Different drivers will give different results, but there's nothing
platform specific about this.
>>> It's not hard to avoid excuting code... For instance we could always
>>> have it translated to something like:
>>>
>>> if (0) {
>>> (void)(expr);
>>> }
>>> (void)(0)
>>>
>>>
>> Obviously I would have meant to say something cleaner like:
>>
>> do {
>>if (0) { (void)(expr); }
>> }
>> while (0)
>>
>
> This only works if expr has no calls, or just inline calls. Using my
> earlier example, if very_expensive_check() is in another file then the
> compiler has to assume the function will have side effects, and the call
> can't be removed.
>
> I'm not sure __assume keyword that Michal mentioned helps. It's more a
> hint to the compiler to help him optimize code around the assertion, but
> perhaps it helps with the warnings too.
>
>
If I try to compile this:
__assume(lalala);
I get:
error C2065: 'lalala' : undeclared identifier
On the other side, the compiler is going to be serious about the
assumptions inside __assume(), and if they happen to be false, the
application can behave not as expected. This is against current gallium
paradigm, where we put assertions, but also do the same check in
non-debug builds to early out from a function or provide default values
(e.g. in switch-case statements).
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, Dec 14, 2009 at 11:42 AM, Corbin Simpson wrote: > As far as immediate verts, why don't we just add support to r300g to switch > to immediate mode for small VBOs? > > Posting from a mobile, pardon my terseness. ~ C. That was what I was thinking for Nouveau, silently create a user buffer for size < some threshold and when we get a draw call with a user vertex buffer submit it in immediate mode. -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 08:22 -0800, Keith Whitwell wrote:
> On Mon, 2009-12-14 at 08:19 -0800, Keith Whitwell wrote:
> > On Mon, 2009-12-14 at 08:04 -0800, José Fonseca wrote:
> > > On Mon, 2009-12-14 at 05:39 -0800, Keith Whitwell wrote:
> > > > On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
> > > > >
> > > > > +static INLINE
> > > > > +void util_blitter_save_fragment_sampler_states(
> > > > > + struct blitter_context *blitter,
> > > > > + int num_sampler_states,
> > > > > + void **sampler_states)
> > > > > +{
> > > > > + assert(num_textures <= 32);
> > > > > +
> > > > > + blitter->saved_num_sampler_states = num_sampler_states;
> > > > > + memcpy(blitter->saved_sampler_states, sampler_states,
> > > > > + num_sampler_states * sizeof(void *));
> > > > > +}
> > > > > +
> > > >
> > > > Have you tried compiling with debug enabled? The assert above fails to
> > > > compile. Also, can you use Elements() or similar instead of the
> > > > hard-coded 32?
> > > >
> > > > Maybe we can figure out how to go back to having asserts keep exposing
> > > > their contents to the compiler even on non-debug builds. This used to
> > > > work without problem on linux and helped a lot to avoid these type of
> > > > problems.
> > >
> > > I wouldn't say without a problem: defining assert(expr) as (void)0
> > > instead of (void)(expr) on release builds yielded a non-negligible
> > > performance improvement. I don't recall the exact figure, but I believe
> > > it was the 3-5% for the driver I was benchmarking at the time. YMMV.
> > > Different drivers will give different results, but there's nothing
> > > platform specific about this.
> >
> > It's not hard to avoid excuting code... For instance we could always
> > have it translated to something like:
> >
> > if (0) {
> > (void)(expr);
> > }
> > (void)(0)
> >
>
> Obviously I would have meant to say something cleaner like:
>
> do {
>if (0) { (void)(expr); }
> }
> while (0)
This only works if expr has no calls, or just inline calls. Using my
earlier example, if very_expensive_check() is in another file then the
compiler has to assume the function will have side effects, and the call
can't be removed.
I'm not sure __assume keyword that Michal mentioned helps. It's more a
hint to the compiler to help him optimize code around the assertion, but
perhaps it helps with the warnings too.
Jose
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
As far as immediate verts, why don't we just add support to r300g to switch to immediate mode for small VBOs? Posting from a mobile, pardon my terseness. ~ C. On Dec 13, 2009 3:28 PM, "Marek Olšák" wrote: Hi Keith, I've finished the blitter module. It fully implements the clear, surface_copy, and surface_fill functions. It properly fallbacks to software in case a surface cannot be sampled or rendered to according to usage. Copying a stencil buffer always fallbacks unless the ignore_stencil parameter (see util_blitter_copy) is set to TRUE. To my knowledge, GPUs cannot copy the stencil buffer (not sure if fiddling with texture formats can help). It's all documented in u_blitter.h. The pipe driver can optionally hook up a function to draw a quad (blitter_context::draw_quad). I realized that embedding 4 vertices into a command stream (AKA immediate mode) is much faster than writing them to a vertex buffer due to reduced driver overhead. It might be worth to consider adding the draw_quad function to pipe_context. When working on the blitter, I added the following things to util/u_simple_shaders: - util_make_fragment_tex_shader has a new parametr tex_target and the value should be one of TGSI_TEXTURE_* enums so that it can be used to sample from any kind of texture. - Added util_make_fragment_tex_shader_writedepth, which writes depth sampled from a texture. It's used for copying depth textures. - Added util_make_fragment_clonecolor_shader, which copies input COLOR[0] to a specified number of render targets. It's used to clear MRTs. Also, I moved the code for converting 2D texture coordinates into cubemap texture coordinates from u_gen_mipmap to a new function in util/u_texture. Please review/push. Once it gets approved, I will send patches with r300g blit support to Corbin. With this work, untiling a texture will be as easy as calling surface_copy whereas the driver state remains intact (theoretically). Cheers. Marek On Thu, Dec 10, 2009 at 6:23 PM, Keith Whitwell wrote: > On Thu, 2009-12-10 at 01:52 -0800, Marek Olšák wrote: >> Keith, >> >> I've taken your comment into consideration and started laying out a >> new simple driver module which I call Blitter. The idea is to provide >> acceleration for operations like clear, surface_copy, and >> surface_fill. The module doesn't depend on a CSO context, instead, a >> driver must call appropriate util_blitter_save* functions to save CSOs >> and a blit operation takes care of their restoration once it's done. >> >> I attached a patch illustrating the idea with the clear implemented >> and a working example of usage, but it's not ready to get pushed yet. >> >> Please tell me what you think about it. > > Marek, > > This looks good to me. It looks like this approach keeps the > implementation entirely on the driver side of the interface, which is > what I was hoping for. > > I had assumed that doing this type of operation in the driver would > require assistance "from above" for saving and restoring state. But it > seems like you've been able to do without that, which is nice. > > Let me know how it progresses. > > Keith > > -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 08:19 -0800, Keith Whitwell wrote:
> On Mon, 2009-12-14 at 08:04 -0800, José Fonseca wrote:
> > On Mon, 2009-12-14 at 05:39 -0800, Keith Whitwell wrote:
> > > On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
> > > >
> > > > +static INLINE
> > > > +void util_blitter_save_fragment_sampler_states(
> > > > + struct blitter_context *blitter,
> > > > + int num_sampler_states,
> > > > + void **sampler_states)
> > > > +{
> > > > + assert(num_textures <= 32);
> > > > +
> > > > + blitter->saved_num_sampler_states = num_sampler_states;
> > > > + memcpy(blitter->saved_sampler_states, sampler_states,
> > > > + num_sampler_states * sizeof(void *));
> > > > +}
> > > > +
> > >
> > > Have you tried compiling with debug enabled? The assert above fails to
> > > compile. Also, can you use Elements() or similar instead of the
> > > hard-coded 32?
> > >
> > > Maybe we can figure out how to go back to having asserts keep exposing
> > > their contents to the compiler even on non-debug builds. This used to
> > > work without problem on linux and helped a lot to avoid these type of
> > > problems.
> >
> > I wouldn't say without a problem: defining assert(expr) as (void)0
> > instead of (void)(expr) on release builds yielded a non-negligible
> > performance improvement. I don't recall the exact figure, but I believe
> > it was the 3-5% for the driver I was benchmarking at the time. YMMV.
> > Different drivers will give different results, but there's nothing
> > platform specific about this.
>
> It's not hard to avoid excuting code... For instance we could always
> have it translated to something like:
>
> if (0) {
> (void)(expr);
> }
> (void)(0)
>
Obviously I would have meant to say something cleaner like:
do {
if (0) { (void)(expr); }
}
while (0)
Keith
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 08:04 -0800, José Fonseca wrote:
> On Mon, 2009-12-14 at 05:39 -0800, Keith Whitwell wrote:
> > On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
> > >
> > > +static INLINE
> > > +void util_blitter_save_fragment_sampler_states(
> > > + struct blitter_context *blitter,
> > > + int num_sampler_states,
> > > + void **sampler_states)
> > > +{
> > > + assert(num_textures <= 32);
> > > +
> > > + blitter->saved_num_sampler_states = num_sampler_states;
> > > + memcpy(blitter->saved_sampler_states, sampler_states,
> > > + num_sampler_states * sizeof(void *));
> > > +}
> > > +
> >
> > Have you tried compiling with debug enabled? The assert above fails to
> > compile. Also, can you use Elements() or similar instead of the
> > hard-coded 32?
> >
> > Maybe we can figure out how to go back to having asserts keep exposing
> > their contents to the compiler even on non-debug builds. This used to
> > work without problem on linux and helped a lot to avoid these type of
> > problems.
>
> I wouldn't say without a problem: defining assert(expr) as (void)0
> instead of (void)(expr) on release builds yielded a non-negligible
> performance improvement. I don't recall the exact figure, but I believe
> it was the 3-5% for the driver I was benchmarking at the time. YMMV.
> Different drivers will give different results, but there's nothing
> platform specific about this.
It's not hard to avoid excuting code... For instance we could always
have it translated to something like:
if (0) {
(void)(expr);
}
(void)(0)
> I believe the problem is we sometimes have
>
> assert(very_expensive_check());
>
> and it should be really
>
> #ifdef DEBUG
> assert(very_expensive_check());
> #endf
I think the above translation is fine, without the extra ifdefs.
> We could go through the files with a fine-toothed comb and fix it, but
> it's quite likely this sort of checks creep back in unnoticed and the
> thing repeats again. Between having debug builds temporarily broken and
> slower release builds I personally I'm for the former.
>
> No suprise that (void)0 is the common practice: glibc, ms's headers,
> etc. all do that.
> Also, I don't understand why a developer wouldn't want to use a debug
> build unless he's profiling. I don't see why we should make easy for a
> developer not to test its code, and running a debug build is the bare
> minimum.
There are other issues as well, such as unused variable warnings for
vars used only in asserts, etc.
Keith
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 03:52 -0800, Keith Whitwell wrote: > On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote: > > Hi Keith, > > > > I've finished the blitter module. It fully implements the clear, > > surface_copy, and surface_fill functions. It properly fallbacks to > > software in case a surface cannot be sampled or rendered to according > > to usage. Copying a stencil buffer always fallbacks unless the > > ignore_stencil parameter (see util_blitter_copy) is set to TRUE. To my > > knowledge, GPUs cannot copy the stencil buffer (not sure if fiddling > > with texture formats can help). It's all documented in u_blitter.h. > > > > The pipe driver can optionally hook up a function to draw a quad > > (blitter_context::draw_quad). I realized that embedding 4 vertices > > into a command stream (AKA immediate mode) is much faster than writing > > them to a vertex buffer due to reduced driver overhead. It might be > > worth to consider adding the draw_quad function to pipe_context. > > > > When working on the blitter, I added the following things to > > util/u_simple_shaders: > > - util_make_fragment_tex_shader has a new parametr tex_target and the > > value should be one of TGSI_TEXTURE_* enums so that it can be used to > > sample from any kind of texture. > > - Added util_make_fragment_tex_shader_writedepth, which writes depth > > sampled from a texture. It's used for copying depth textures. > > - Added util_make_fragment_clonecolor_shader, which copies input > > COLOR[0] to a specified number of render targets. It's used to clear > > MRTs. > > > > Also, I moved the code for converting 2D texture coordinates into > > cubemap texture coordinates from u_gen_mipmap to a new function in > > util/u_texture. > > > > Please review/push. > > > > Once it gets approved, I will send patches with r300g blit support to > > Corbin. With this work, untiling a texture will be as easy as calling > > surface_copy whereas the driver state remains intact (theoretically). > > Marek, > > This all looks great. Many thanks for adding this functionality - I'm > sure we'll be building on it in many ways going forward. Nice stuff indeed. FWIW, I also think that putting a reasonable functionality bars instead querying the pipe for every little capability will benefit us in the long term. It worked well for vertex processing and hardware unsupported API quirks (via draw module); it's nice to see the same for blits; and I hope this becomes a trend. It not only makes things less complex, having all pipe drivers with similar capabilities is what allows us to plug'n'play pipe drivers; do things like replay a trace of one driver on top of another; perhaps in the future code a drivers that do differential analysis with a reference one, etc. Jose -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 03:52 -0800, Keith Whitwell wrote: > On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote: > > Hi Keith, > > > > I've finished the blitter module. It fully implements the clear, > > surface_copy, and surface_fill functions. It properly fallbacks to > > software in case a surface cannot be sampled or rendered to according > > to usage. Copying a stencil buffer always fallbacks unless the > > ignore_stencil parameter (see util_blitter_copy) is set to TRUE. To my > > knowledge, GPUs cannot copy the stencil buffer (not sure if fiddling > > with texture formats can help). It's all documented in u_blitter.h. > > > > The pipe driver can optionally hook up a function to draw a quad > > (blitter_context::draw_quad). I realized that embedding 4 vertices > > into a command stream (AKA immediate mode) is much faster than writing > > them to a vertex buffer due to reduced driver overhead. It might be > > worth to consider adding the draw_quad function to pipe_context. > > > > When working on the blitter, I added the following things to > > util/u_simple_shaders: > > - util_make_fragment_tex_shader has a new parametr tex_target and the > > value should be one of TGSI_TEXTURE_* enums so that it can be used to > > sample from any kind of texture. > > - Added util_make_fragment_tex_shader_writedepth, which writes depth > > sampled from a texture. It's used for copying depth textures. > > - Added util_make_fragment_clonecolor_shader, which copies input > > COLOR[0] to a specified number of render targets. It's used to clear > > MRTs. > > > > Also, I moved the code for converting 2D texture coordinates into > > cubemap texture coordinates from u_gen_mipmap to a new function in > > util/u_texture. > > > > Please review/push. > > > > Once it gets approved, I will send patches with r300g blit support to > > Corbin. With this work, untiling a texture will be as easy as calling > > surface_copy whereas the driver state remains intact (theoretically). > > Marek, > > This all looks great. Many thanks for adding this functionality - I'm > sure we'll be building on it in many ways going forward. > > I'll push the patches intact, but one thing we need to start thinking > about is the mix of code in the util/ directory -- there's some stuff in > there that's only legal/useful for state-trackers, some that's likewise > only legal for drivers, and a lot that is valid everywhere. At some > stage we want to split that up. I plan to split the os specific stuff out soon. I'm referring to memory allocation. debug printing, file abstraction, etc. All stuff that is not Gallium related and is needed everywhere. Jose -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Mon, 2009-12-14 at 05:39 -0800, Keith Whitwell wrote:
> On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
> >
> > +static INLINE
> > +void util_blitter_save_fragment_sampler_states(
> > + struct blitter_context *blitter,
> > + int num_sampler_states,
> > + void **sampler_states)
> > +{
> > + assert(num_textures <= 32);
> > +
> > + blitter->saved_num_sampler_states = num_sampler_states;
> > + memcpy(blitter->saved_sampler_states, sampler_states,
> > + num_sampler_states * sizeof(void *));
> > +}
> > +
>
> Have you tried compiling with debug enabled? The assert above fails to
> compile. Also, can you use Elements() or similar instead of the
> hard-coded 32?
>
> Maybe we can figure out how to go back to having asserts keep exposing
> their contents to the compiler even on non-debug builds. This used to
> work without problem on linux and helped a lot to avoid these type of
> problems.
I wouldn't say without a problem: defining assert(expr) as (void)0
instead of (void)(expr) on release builds yielded a non-negligible
performance improvement. I don't recall the exact figure, but I believe
it was the 3-5% for the driver I was benchmarking at the time. YMMV.
Different drivers will give different results, but there's nothing
platform specific about this.
I believe the problem is we sometimes have
assert(very_expensive_check());
and it should be really
#ifdef DEBUG
assert(very_expensive_check());
#endf
We could go through the files with a fine-toothed comb and fix it, but
it's quite likely this sort of checks creep back in unnoticed and the
thing repeats again. Between having debug builds temporarily broken and
slower release builds I personally I'm for the former.
No suprise that (void)0 is the common practice: glibc, ms's headers,
etc. all do that.
Also, I don't understand why a developer wouldn't want to use a debug
build unless he's profiling. I don't see why we should make easy for a
developer not to test its code, and running a debug build is the bare
minimum.
Jose
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
Keith Whitwell pisze:
> On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
>
>> +static INLINE
>> +void util_blitter_save_fragment_sampler_states(
>> + struct blitter_context *blitter,
>> + int num_sampler_states,
>> + void **sampler_states)
>> +{
>> + assert(num_textures <= 32);
>> +
>> + blitter->saved_num_sampler_states = num_sampler_states;
>> + memcpy(blitter->saved_sampler_states, sampler_states,
>> + num_sampler_states * sizeof(void *));
>> +}
>> +
>>
>
> Have you tried compiling with debug enabled? The assert above fails to
> compile. Also, can you use Elements() or similar instead of the
> hard-coded 32?
>
> Maybe we can figure out how to go back to having asserts keep exposing
> their contents to the compiler even on non-debug builds. This used to
> work without problem on linux and helped a lot to avoid these type of
> problems.
>
>
Precisely. Recently I've been thinking about mapping assert() to
__assume() for non-debug builds on windows and MSVC.
http://msdn.microsoft.com/en-us/library/1b3fsfxw%28VS.80%29.aspx
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
> -- /dev/null
> +++ b/src/gallium/auxiliary/util/u_blitter.c
> @@ -0,0 +1,605 @@
> +/**
> + *
> + * Copyright 2009 Marek Olšák
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> + * IN NO EVENT SHALL TUNGSTEN GRAPHICS AND/OR ITS SUPPLIERS BE LIABLE FOR
> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + **/
> +
> +/**
> + * @file
> + * Blitter utility to facilitate acceleration of the clear, surface_copy,
> + * and surface_fill functions.
> + *
> + * @author Marek Olšák
> + */
> +
> +#include "pipe/p_context.h"
> +#include "pipe/p_defines.h"
> +#include "pipe/p_inlines.h"
> +#include "pipe/p_shader_tokens.h"
> +#include "pipe/p_state.h"
> +
> +#include "util/u_memory.h"
> +#include "util/u_math.h"
> +#include "util/u_blitter.h"
> +#include "util/u_draw_quad.h"
> +#include "util/u_pack_color.h"
> +#include "util/u_rect.h"
> +#include "util/u_simple_shaders.h"
> +#include "util/u_texture.h"
> +
> +struct blitter_context_priv
> +{
> + struct blitter_context blitter;
> +
> + struct pipe_context *pipe; /**< pipe context */
> + struct pipe_buffer *vbuf; /**< quad */
> +
> + float vertices[4][2][4]; /**< {pos, color} or {pos, texcoord} */
> +
> + /* Constant state objects. */
> + /* Vertex shaders. */
> + void *vs_col; /**< Vertex shader which passes {pos, color} to the output
> */
> + void *vs_tex; /** output.*/
> +
> + /* Fragment shaders. */
> + void *fs_col[8]; /**< FS which outputs colors to 1-8 color buffers */
> + void *fs_texfetch_col[4]; /**< FS which outputs a color from a texture
> */
> + void *fs_texfetch_depth[4]; /**< FS which outputs a depth from a texture,
> + where the index is PIPE_TEXTURE_* to be
> sampled */
Please use PIPE_MAX_COLOR_BUFS or other defines to size these arrays.
> + /* Blend state. */
> + void *blend_write_color; /**< blend state with writemask of RGBA */
> + void *blend_keep_color;/**< blend state with writemask of 0 */
> +
> + /* Depth stencil alpha state. */
> + void *dsa_write_depth_stencil[0xff]; /**< indices are stencil clear
> values */
That's a lot of state objects...
> + void *dsa_write_depth_keep_stencil;
> + void *dsa_keep_depth_stencil;
> +
> + /* Other state. */
> + void *sampler_state[16]; /**< sampler state for clamping to a miplevel
> */
> + void *rs_state;/**< rasterizer state */
> +};
> +
> +struct blitter_context *util_blitter_create(struct pipe_context *pipe)
> +{
> + struct blitter_context_priv *ctx;
> + struct pipe_blend_state blend;
> + struct pipe_depth_stencil_alpha_state dsa;
> + struct pipe_rasterizer_state rs_state;
> + struct pipe_sampler_state sampler_state;
> + unsigned i, max_render_targets;
> +
> + ctx = CALLOC_STRUCT(blitter_context_priv);
> + if (!ctx)
> + return NULL;
> +
> + ctx->pipe = pipe;
> +
> + /* init state objects for them to be considered invalid */
> + ctx->blitter.saved_fb_state.nr_cbufs = ~0;
> + ctx->blitter.saved_num_textures = ~0;
> + ctx->blitter.saved_num_sampler_states = ~0;
> +
> + /* blend state objects */
> + memset(&blend, 0, sizeof(blend));
> + ctx->blend_keep_color = pipe->create_blend_state(pipe, &blend);
> +
> + blend.colormask = PIPE_MASK_RGBA;
> + ctx->blend_write_color = pipe->create_blend_state(pipe, &blend);
> +
> + /* depth stencil alpha state objects */
> + memset(&dsa, 0, sizeof(dsa));
> + ctx->dsa_keep_depth_stencil =
> + pipe->create_depth_stencil_alpha_state(pipe, &dsa);
> +
> + dsa.depth.enabled = 1;
> + dsa.depth.writemask = 1;
> + dsa.depth.func = PIPE_FUNC_ALWAYS;
> + ctx->dsa_write_depth_keep_stencil =
> + pipe->create_depth_stencil_alpha_state(pipe, &dsa);
> +
> + dsa.stencil[0].enabled = 1;
> + dsa.stencil[0].func = PIPE_FUNC_ALWAYS;
Re: [Mesa3d-dev] gallium: add blitter
On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote:
>
> +static INLINE
> +void util_blitter_save_fragment_sampler_states(
> + struct blitter_context *blitter,
> + int num_sampler_states,
> + void **sampler_states)
> +{
> + assert(num_textures <= 32);
> +
> + blitter->saved_num_sampler_states = num_sampler_states;
> + memcpy(blitter->saved_sampler_states, sampler_states,
> + num_sampler_states * sizeof(void *));
> +}
> +
Have you tried compiling with debug enabled? The assert above fails to
compile. Also, can you use Elements() or similar instead of the
hard-coded 32?
Maybe we can figure out how to go back to having asserts keep exposing
their contents to the compiler even on non-debug builds. This used to
work without problem on linux and helped a lot to avoid these type of
problems.
Keith
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] gallium: add blitter
On Sun, 2009-12-13 at 15:27 -0800, Marek Olšák wrote: > Hi Keith, > > I've finished the blitter module. It fully implements the clear, > surface_copy, and surface_fill functions. It properly fallbacks to > software in case a surface cannot be sampled or rendered to according > to usage. Copying a stencil buffer always fallbacks unless the > ignore_stencil parameter (see util_blitter_copy) is set to TRUE. To my > knowledge, GPUs cannot copy the stencil buffer (not sure if fiddling > with texture formats can help). It's all documented in u_blitter.h. > > The pipe driver can optionally hook up a function to draw a quad > (blitter_context::draw_quad). I realized that embedding 4 vertices > into a command stream (AKA immediate mode) is much faster than writing > them to a vertex buffer due to reduced driver overhead. It might be > worth to consider adding the draw_quad function to pipe_context. > > When working on the blitter, I added the following things to > util/u_simple_shaders: > - util_make_fragment_tex_shader has a new parametr tex_target and the > value should be one of TGSI_TEXTURE_* enums so that it can be used to > sample from any kind of texture. > - Added util_make_fragment_tex_shader_writedepth, which writes depth > sampled from a texture. It's used for copying depth textures. > - Added util_make_fragment_clonecolor_shader, which copies input > COLOR[0] to a specified number of render targets. It's used to clear > MRTs. > > Also, I moved the code for converting 2D texture coordinates into > cubemap texture coordinates from u_gen_mipmap to a new function in > util/u_texture. > > Please review/push. > > Once it gets approved, I will send patches with r300g blit support to > Corbin. With this work, untiling a texture will be as easy as calling > surface_copy whereas the driver state remains intact (theoretically). Marek, This all looks great. Many thanks for adding this functionality - I'm sure we'll be building on it in many ways going forward. I'll push the patches intact, but one thing we need to start thinking about is the mix of code in the util/ directory -- there's some stuff in there that's only legal/useful for state-trackers, some that's likewise only legal for drivers, and a lot that is valid everywhere. At some stage we want to split that up. Keith -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
