[Mesa-dev] [PATCH 1/3] gallium/st: add pipe_context::generate_mipmap()

[Charmaine Lee]

>From: Michel Dänzer <michel at daenzer.net>
>Sent: Monday, January 11, 2016 11:37 PM
>To: Charmaine Lee
>Cc: mesa-dev at lists.freedesktop.org
>Subject: Re: [Mesa-dev] [PATCH 1/3] gallium/st: add pipe_context::generate_mipmap()

>On 12.01.2016 15:18, Charmaine Lee wrote:
>> This patch adds a new interface to support hardware mipmap generation.
>> PIPE_CAP_GENERATE_MIPMAP is added to allow a driver to specify
>> if this new interface is supported; if not supported, the state tracker will
>> fallback to mipmap generation by rendering/texturing.
>>
>> v2: add PIPE_CAP_GENERATE_MIPMAP to the disabled section for all drivers

>[...]

>> diff --git a/src/gallium/drivers/trace/tr_context.c b/src/gallium/drivers/trace/tr_context.c
>> index d4c88c9..17da64e 100644
>> --- a/src/gallium/drivers/trace/tr_context.c
>> +++ b/src/gallium/drivers/trace/tr_context.c
>> @@ -1292,6 +1292,35 @@ trace_context_flush(struct pipe_context *_pipe,
>>
>>
>>  static inline void
>> +trace_context_generate_mipmap(struct pipe_context *_pipe,
>> +                              struct pipe_resource *res,
>> +                              unsigned base_level,
>> +                              unsigned last_level,
>> +                              unsigned first_layer,
>> +                              unsigned last_layer)
>> +{
>> +   struct trace_context *tr_ctx = trace_context(_pipe);
>> +   struct pipe_context *pipe = tr_ctx->pipe;
>> +
>> +   res = trace_resource_unwrap(tr_ctx, res);
>> +
>> +   trace_dump_call_begin("pipe_context", "generate_mipmap");
>> +
>> +   trace_dump_arg(ptr, pipe);
>> +   trace_dump_arg(ptr, res);
>> +
>> +   trace_dump_arg(uint, base_level);
>> +   trace_dump_arg(uint, last_level);
>> +   trace_dump_arg(uint, first_layer);
>> +   trace_dump_arg(uint, last_layer);
>> +
>> +   pipe->generate_mipmap(pipe, res, base_level, last_level, first_layer, last_layer);
>> +
>> +   trace_dump_call_end();
>> +}
>> +
>> +
>> +static inline void
>>  trace_context_destroy(struct pipe_context *_pipe)
>>  {
>>     struct trace_context *tr_ctx = trace_context(_pipe);

>[...]

>> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
>> index be7447d..a0774c7 100644
>> --- a/src/gallium/include/pipe/p_context.h
>> +++ b/src/gallium/include/pipe/p_context.h
>> @@ -673,6 +673,17 @@ struct pipe_context {
>>      */
>>     void (*dump_debug_state)(struct pipe_context *ctx, FILE *stream,
>>                              unsigned flags);
>> +
>> +   /**
>> +    * Generate mipmap.
>> +    * \return TRUE if mipmap generation succeeds, FALSE otherwise
>> +    */
>> +   boolean (*generate_mipmap)(struct pipe_context *ctx,
>> +                              struct pipe_resource *resource,
>> +                              unsigned base_level,
>> +                              unsigned last_level,
>> +                              unsigned first_layer,
>> +                              unsigned last_layer);
>>  };

>Maybe I'm missing something, but the prototypes of the pipe_context
>field and trace driver implementation don't match.

Hmm. I don't know what you mean. They look correct to me.

>> diff --git a/src/mesa/state_tracker/st_gen_mipmap.c b/src/mesa/state_tracker/st_gen_mipmap.c
>> index b370040..94c1171 100644
>> --- a/src/mesa/state_tracker/st_gen_mipmap.c
>> +++ b/src/mesa/state_tracker/st_gen_mipmap.c
>> @@ -149,12 +149,19 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
>>        last_layer = util_max_layer(pt, baseLevel);
>>     }
>>
>> -   /* Try to generate the mipmap by rendering/texturing.  If that fails,
>> -    * use the software fallback.
>> +   /* First see if the driver supports hardware mipmap generation,
>> +    * if not then generate the mipmap by rendering/texturing.
>> +    * If that fails, use the software fallback.
>>      */
>> -   if (!util_gen_mipmap(st->pipe, pt, pt->format, baseLevel, lastLevel,
>> -                        first_layer, last_layer, PIPE_TEX_FILTER_LINEAR)) {
>> -      _mesa_generate_mipmap(ctx, target, texObj);
>> +   if (!st->pipe->screen->get_param(st->pipe->screen,
>> +                                    PIPE_CAP_GENERATE_MIPMAP) ||
>> +       !st->pipe->generate_mipmap(st->pipe, pt, baseLevel, lastLevel,
>> +                                  first_layer, last_layer)) {
>> +
>> +      if (!util_gen_mipmap(st->pipe, pt, pt->format, baseLevel, lastLevel,
>> +                           first_layer, last_layer, PIPE_TEX_FILTER_LINEAR)) {
>> +         _mesa_generate_mipmap(ctx, target, texObj);
>> +      }
>>     }

>Do we really need a cap for this? Drivers not implementing this hook can
>just leave it at NULL, then state trackers can fall back and wrapper
>drivers can return false in that case.

My initial attempt was to check for NULL too, but Jose pointed out to me that we need a cap
for this capability. This makes trace/svga implementation a little simpler.

-Charmaine