[Mesa-dev] [PATCH 3/6] gallium/u_blitter: split out a helper for common clear state

[Rob Clark]

On Mon, Aug 15, 2016 at 10:44 AM, Marek Olšák <maraeo at gmail.com> wrote:
> On Sat, Aug 13, 2016 at 8:57 PM, Rob Clark <robdclark at gmail.com> wrote:
>> On Sat, Aug 13, 2016 at 2:23 PM, Rob Clark <robdclark at gmail.com> wrote:
>>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>> ---
>>>  src/gallium/auxiliary/util/u_blitter.c | 38 ++++++++++++++++++++++------------
>>>  src/gallium/auxiliary/util/u_blitter.h |  5 +++++
>>>  2 files changed, 30 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/src/gallium/auxiliary/util/u_blitter.c b/src/gallium/auxiliary/util/u_blitter.c
>>> index 9fbef9b..e008100 100644
>>> --- a/src/gallium/auxiliary/util/u_blitter.c
>>> +++ b/src/gallium/auxiliary/util/u_blitter.c
>>> @@ -1268,19 +1268,13 @@ static void *get_clear_blend_state(struct blitter_context_priv *ctx,
>>>     return ctx->blend_clear[index];
>>>  }
>>>
>>> -static void util_blitter_clear_custom(struct blitter_context *blitter,
>>> -                                      unsigned width, unsigned height,
>>> -                                      unsigned num_layers,
>>> -                                      unsigned clear_buffers,
>>> -                                      const union pipe_color_union *color,
>>> -                                      double depth, unsigned stencil,
>>> -                                      void *custom_blend, void *custom_dsa)
>>> +void util_blitter_common_clear_setup(struct blitter_context *blitter,
>>> +                                     unsigned width, unsigned height,
>>> +                                     unsigned clear_buffers,
>>> +                                     void *custom_blend, void *custom_dsa)
>>>  {
>>>     struct blitter_context_priv *ctx = (struct blitter_context_priv*)blitter;
>>>     struct pipe_context *pipe = ctx->base.pipe;
>>> -   struct pipe_stencil_ref sr = { { 0 } };
>>> -
>>> -   assert(ctx->has_layered || num_layers <= 1);
>>>
>>>     util_blitter_set_running_flag(blitter);
>>>     blitter_check_saved_vertex_states(ctx);
>>
>> so actually, I need to squash in:
>>
>> ----------
>> diff --git a/src/gallium/auxiliary/util/u_blitter.c
>> b/src/gallium/auxiliary/util/u_blitter.c
>> index e008100..c0d3808 100644
>> --- a/src/gallium/auxiliary/util/u_blitter.c
>> +++ b/src/gallium/auxiliary/util/u_blitter.c
>> @@ -1281,6 +1281,12 @@ void util_blitter_common_clear_setup(struct
>> blitter_context *blitter,
>>     blitter_check_saved_fragment_states(ctx);
>>     blitter_disable_render_cond(ctx);
>>
>> +   /* this will end up getting set again in
>> blitter_set_common_draw_rect_state()
>> +    * but needs to be here for drivers using util_blitter_common_clear_setup()
>> +    * but not util_blitter_clear_custom()
>> +    */
>> +   pipe->bind_rasterizer_state(pipe, ctx->rs_state);
>> +
>>     /* bind states */
>>     if (custom_blend) {
>>        pipe->bind_blend_state(pipe, custom_blend);
>> ----------
>>
>> I don't think there should be any harm in doing
>> pipe->bind_rasterizer_state() twice for drivers using
>> util_blitter_clear_custom(), and this is easier that adding a new
>> helper (since ctx->rs_state is not visible outside of u_blitter.c),
>> but if someone sees a problem with binding rasterizer state twice then
>> I could make it a new helper fxn instead.
>
> For clarity, it would be better to expose ctx->rs_state to drivers
> (e.g. via a function) instead of this hack.

since it only amounts to a few lines of code, I think I'll just create
my own rs_state in fd_context, and go back to the original patch here
without anything squashed in.  Was that your only objection to patch
3?

BR,
-R

> Patches 1-2:
>
> Reviewed-by: Marek Olšák <marek.olsak at amd.com>
>
> Marek