[Mesa-dev] [PATCH 01/10] gallium: cleanup set_tess_state
Rob Clark
robdclark at gmail.com
Tue Jun 14 17:15:24 UTC 2016
On Tue, Jun 14, 2016 at 12:30 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 14.06.2016 18:02, Ilia Mirkin wrote:
>>
>> Can you explain the motivation behind this change? I'm adding a
>> ->set_window_rectangles thing which also takes multiple parameters.
>> What's the advantage of stuffing things into a struct first?
>
>
> FWIW, I tend to be mildly supportive of changes like this. At least, the
> other extreme where functions grow multiple bool or int parameters over time
> is much worse. But in this particular case, changing this around might be
> too eager.
I'd have to think about how it would work to deal w/ variants that
have params not wrapped in a struct. It at least sounds annoying, and
I tended to think the benefits of using a struct where enough of a
justification to change this. (Plus there are not many usages of this
API yet, so seemed like the perfect time to cleanup.)
> Perhaps teaching the script to deal with slightly more complicated cases
> will help elsewhere, too.
*maybe*, but I can't think of anything.. right now it is only the
sampler_view and stream_output_target state that I handle "manually"..
but those are also kind of different from the rest since they are
already refcnt'd. And I figured it was easier to just deal w/ those
manually than implement a 3rd type of state (CSO vs Param) in
rsq_state.py..
BR,
-R
> Nicolai
>
>>
>> -ilia
>>
>> On Tue, Jun 14, 2016 at 11:57 AM, Rob Clark <robdclark at gmail.com> wrote:
>>>
>>> From: Rob Clark <robclark at freedesktop.org>
>>>
>>> The reset of the state APIs take state structs, rather than inline
>>> parameters (with the exception of a couple which just amount to a single
>>> uint).
>>>
>>> This makes the API more regular and simplifies autogeneration of the
>>> gallium state related APIs.
>>>
>>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>>> ---
>>> src/gallium/drivers/ddebug/dd_context.c | 9 ++++-----
>>> src/gallium/drivers/nouveau/nvc0/nvc0_state.c | 7 +++----
>>> src/gallium/drivers/r600/evergreen_state.c | 7 +++----
>>> src/gallium/drivers/radeonsi/si_state.c | 7 +++----
>>> src/gallium/drivers/trace/tr_context.c | 9 ++++-----
>>> src/gallium/include/pipe/p_context.h | 4 ++--
>>> src/gallium/include/pipe/p_state.h | 8 ++++++++
>>> src/mesa/state_tracker/st_atom_tess.c | 13 ++++++++++---
>>> 8 files changed, 37 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/ddebug/dd_context.c
>>> b/src/gallium/drivers/ddebug/dd_context.c
>>> index 0f8ef18..06b7c91 100644
>>> --- a/src/gallium/drivers/ddebug/dd_context.c
>>> +++ b/src/gallium/drivers/ddebug/dd_context.c
>>> @@ -380,15 +380,14 @@ dd_context_set_viewport_states(struct pipe_context
>>> *_pipe,
>>> }
>>>
>>> static void dd_context_set_tess_state(struct pipe_context *_pipe,
>>> - const float
>>> default_outer_level[4],
>>> - const float
>>> default_inner_level[2])
>>> + const struct pipe_tess_state
>>> *state)
>>> {
>>> struct dd_context *dctx = dd_context(_pipe);
>>> struct pipe_context *pipe = dctx->pipe;
>>>
>>> - memcpy(dctx->tess_default_levels, default_outer_level, sizeof(float)
>>> * 4);
>>> - memcpy(dctx->tess_default_levels+4, default_inner_level,
>>> sizeof(float) * 2);
>>> - pipe->set_tess_state(pipe, default_outer_level, default_inner_level);
>>> + memcpy(dctx->tess_default_levels, state->default_outer_level,
>>> sizeof(float) * 4);
>>> + memcpy(dctx->tess_default_levels+4, state->default_inner_level,
>>> sizeof(float) * 2);
>>> + pipe->set_tess_state(pipe, state);
>>> }
>>>
>>>
>>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
>>> b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
>>> index 92161ec..a9c1830 100644
>>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
>>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
>>> @@ -1001,13 +1001,12 @@ nvc0_set_viewport_states(struct pipe_context
>>> *pipe,
>>>
>>> static void
>>> nvc0_set_tess_state(struct pipe_context *pipe,
>>> - const float default_tess_outer[4],
>>> - const float default_tess_inner[2])
>>> + const struct pipe_tess_state *state)
>>> {
>>> struct nvc0_context *nvc0 = nvc0_context(pipe);
>>>
>>> - memcpy(nvc0->default_tess_outer, default_tess_outer, 4 *
>>> sizeof(float));
>>> - memcpy(nvc0->default_tess_inner, default_tess_inner, 2 *
>>> sizeof(float));
>>> + memcpy(nvc0->default_tess_outer, state->default_tess_outer, 4 *
>>> sizeof(float));
>>> + memcpy(nvc0->default_tess_inner, state->default_tess_inner, 2 *
>>> sizeof(float));
>>> nvc0->dirty_3d |= NVC0_NEW_3D_TESSFACTOR;
>>> }
>>>
>>> diff --git a/src/gallium/drivers/r600/evergreen_state.c
>>> b/src/gallium/drivers/r600/evergreen_state.c
>>> index 1ac8914..2a424f5 100644
>>> --- a/src/gallium/drivers/r600/evergreen_state.c
>>> +++ b/src/gallium/drivers/r600/evergreen_state.c
>>> @@ -3569,13 +3569,12 @@ fallback:
>>> }
>>>
>>> static void evergreen_set_tess_state(struct pipe_context *ctx,
>>> - const float default_outer_level[4],
>>> - const float default_inner_level[2])
>>> + const struct pipe_tess_state *state)
>>> {
>>> struct r600_context *rctx = (struct r600_context *)ctx;
>>>
>>> - memcpy(rctx->tess_state, default_outer_level, sizeof(float) * 4);
>>> - memcpy(rctx->tess_state+4, default_inner_level, sizeof(float) *
>>> 2);
>>> + memcpy(rctx->tess_state, state->default_outer_level,
>>> sizeof(float) * 4);
>>> + memcpy(rctx->tess_state+4, state->default_inner_level,
>>> sizeof(float) * 2);
>>> rctx->tess_state_dirty = true;
>>> }
>>>
>>> diff --git a/src/gallium/drivers/radeonsi/si_state.c
>>> b/src/gallium/drivers/radeonsi/si_state.c
>>> index 0c52eee..6ef3fe5 100644
>>> --- a/src/gallium/drivers/radeonsi/si_state.c
>>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>>> @@ -3238,15 +3238,14 @@ static void si_set_index_buffer(struct
>>> pipe_context *ctx,
>>> */
>>>
>>> static void si_set_tess_state(struct pipe_context *ctx,
>>> - const float default_outer_level[4],
>>> - const float default_inner_level[2])
>>> + const struct pipe_tess_state *state)
>>> {
>>> struct si_context *sctx = (struct si_context *)ctx;
>>> struct pipe_constant_buffer cb;
>>> float array[8];
>>>
>>> - memcpy(array, default_outer_level, sizeof(float) * 4);
>>> - memcpy(array+4, default_inner_level, sizeof(float) * 2);
>>> + memcpy(array, state->default_outer_level, sizeof(float) * 4);
>>> + memcpy(array+4, state->default_inner_level, sizeof(float) * 2);
>>>
>>> cb.buffer = NULL;
>>> cb.user_buffer = NULL;
>>> diff --git a/src/gallium/drivers/trace/tr_context.c
>>> b/src/gallium/drivers/trace/tr_context.c
>>> index 18c5c43..0dd07e9 100644
>>> --- a/src/gallium/drivers/trace/tr_context.c
>>> +++ b/src/gallium/drivers/trace/tr_context.c
>>> @@ -1652,19 +1652,18 @@ trace_context_memory_barrier(struct pipe_context
>>> *_context,
>>>
>>> static void
>>> trace_context_set_tess_state(struct pipe_context *_context,
>>> - const float default_outer_level[4],
>>> - const float default_inner_level[2])
>>> + const struct pipe_tess_state *state)
>>> {
>>> struct trace_context *tr_context = trace_context(_context);
>>> struct pipe_context *context = tr_context->pipe;
>>>
>>> trace_dump_call_begin("pipe_context", "set_tess_state");
>>> trace_dump_arg(ptr, context);
>>> - trace_dump_arg_array(float, default_outer_level, 4);
>>> - trace_dump_arg_array(float, default_inner_level, 2);
>>> + trace_dump_arg_array(float, state->default_outer_level, 4);
>>> + trace_dump_arg_array(float, state->default_inner_level, 2);
>>> trace_dump_call_end();
>>>
>>> - context->set_tess_state(context, default_outer_level,
>>> default_inner_level);
>>> + context->set_tess_state(context, state);
>>> }
>>>
>>>
>>> diff --git a/src/gallium/include/pipe/p_context.h
>>> b/src/gallium/include/pipe/p_context.h
>>> index 9d7a8eb..9eb4a87 100644
>>> --- a/src/gallium/include/pipe/p_context.h
>>> +++ b/src/gallium/include/pipe/p_context.h
>>> @@ -60,6 +60,7 @@ struct pipe_resolve_info;
>>> struct pipe_resource;
>>> struct pipe_sampler_state;
>>> struct pipe_sampler_view;
>>> +struct pipe_tess_state;
>>> struct pipe_scissor_state;
>>> struct pipe_shader_buffer;
>>> struct pipe_shader_state;
>>> @@ -284,8 +285,7 @@ struct pipe_context {
>>> struct pipe_sampler_view **);
>>>
>>> void (*set_tess_state)(struct pipe_context *,
>>> - const float default_outer_level[4],
>>> - const float default_inner_level[2]);
>>> + const struct pipe_tess_state *);
>>>
>>> /**
>>> * Sets the debug callback. If the pointer is null, then no callback
>>> is
>>> diff --git a/src/gallium/include/pipe/p_state.h
>>> b/src/gallium/include/pipe/p_state.h
>>> index 396f563..f2ebc53 100644
>>> --- a/src/gallium/include/pipe/p_state.h
>>> +++ b/src/gallium/include/pipe/p_state.h
>>> @@ -453,6 +453,14 @@ struct pipe_image_view
>>> } u;
>>> };
>>>
>>> +/**
>>> + * Tessellation state.
>>> + */
>>> +struct pipe_tess_state
>>> +{
>>> + float default_outer_level[4];
>>> + float default_inner_level[2];
>>> +};
>>>
>>> /**
>>> * Subregion of 1D/2D/3D image resource.
>>> diff --git a/src/mesa/state_tracker/st_atom_tess.c
>>> b/src/mesa/state_tracker/st_atom_tess.c
>>> index 8e6287a..61aa92f 100644
>>> --- a/src/mesa/state_tracker/st_atom_tess.c
>>> +++ b/src/mesa/state_tracker/st_atom_tess.c
>>> @@ -34,6 +34,7 @@
>>> #include "main/macros.h"
>>> #include "st_context.h"
>>> #include "pipe/p_context.h"
>>> +#include "pipe/p_state.h"
>>> #include "st_atom.h"
>>>
>>>
>>> @@ -42,13 +43,19 @@ update_tess(struct st_context *st)
>>> {
>>> const struct gl_context *ctx = st->ctx;
>>> struct pipe_context *pipe = st->pipe;
>>> + struct pipe_tess_state state;
>>>
>>> if (!pipe->set_tess_state)
>>> return;
>>>
>>> - pipe->set_tess_state(pipe,
>>> - ctx->TessCtrlProgram.patch_default_outer_level,
>>> - ctx->TessCtrlProgram.patch_default_inner_level);
>>> + memcpy(state.default_outer_level,
>>> + ctx->TessCtrlProgram.patch_default_outer_level,
>>> + sizeof(state.default_outer_level));
>>> + memcpy(state.default_inner_level,
>>> + ctx->TessCtrlProgram.patch_default_inner_level,
>>> + sizeof(state.default_inner_level));
>>> +
>>> + pipe->set_tess_state(pipe, &state);
>>> }
>>>
>>>
>>> --
>>> 2.5.5
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list