[Mesa-dev] [PATCH 1/3] gallium/st: add pipe_context::generate_mipmap()
Brian Paul
brianp at vmware.com
Tue Jan 12 06:56:48 PST 2016
Just a few nit-picks below...
On 01/11/2016 11:18 PM, 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
> ---
> src/gallium/docs/source/context.rst | 10 ++++++++
> src/gallium/docs/source/screen.rst | 2 ++
> src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
> src/gallium/drivers/i915/i915_screen.c | 1 +
> src/gallium/drivers/ilo/ilo_screen.c | 1 +
> src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
> src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 +
> src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
> src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 1 +
> src/gallium/drivers/r300/r300_screen.c | 1 +
> src/gallium/drivers/r600/r600_pipe.c | 1 +
> src/gallium/drivers/radeonsi/si_pipe.c | 1 +
> src/gallium/drivers/softpipe/sp_screen.c | 1 +
> src/gallium/drivers/svga/svga_screen.c | 1 +
> src/gallium/drivers/trace/tr_context.c | 30 ++++++++++++++++++++++++
> src/gallium/drivers/vc4/vc4_screen.c | 1 +
> src/gallium/drivers/virgl/virgl_screen.c | 1 +
> src/gallium/include/pipe/p_context.h | 11 +++++++++
> src/gallium/include/pipe/p_defines.h | 1 +
> src/mesa/state_tracker/st_gen_mipmap.c | 17 ++++++++++----
> 20 files changed, 80 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/docs/source/context.rst b/src/gallium/docs/source/context.rst
> index 9a32716..e33e7a2 100644
> --- a/src/gallium/docs/source/context.rst
> +++ b/src/gallium/docs/source/context.rst
> @@ -648,3 +648,13 @@ In addition, normal texture sampling is allowed from the compute
> program: ``bind_sampler_states`` may be used to set up texture
> samplers for the compute stage and ``set_sampler_views`` may
> be used to bind a number of sampler views to it.
> +
> +Mipmap generation
> +^^^^^^^^^^^^^^^^^
> +
> +If PIPE_CAP_GENERATE_MIPMAP is true, ``generate_mipmap`` can be used
> +to generate mipmaps for the specified texture resource.
> +It replaces texel image levels base_level+1 through
> +last_level for layers range from first_layer through last_layer.
> +It returns TRUE if mipmap generation succeeds, otherwise it
> +returns FALSE.
You could elaborate on that comment by saying something like "mipmap
generation may fail when it's not supported for particular texture types
or formats."
> diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
> index c8f5f6a..b8d8de5 100644
> --- a/src/gallium/docs/source/screen.rst
> +++ b/src/gallium/docs/source/screen.rst
> @@ -301,6 +301,8 @@ The integer capabilities:
> alignment for pipe_shader_buffer::buffer_offset, in bytes. Maximum
> value allowed is 256 (for GL conformance). 0 is only allowed if
> shader buffers are not supported.
> +* ``PIPE_CAP_GENERATE_MIPMAP``: Whether `generate_mipmap` will be
> + available in contexts.
Or "indicates whether the pipe_context::generate_mipmap() function is
supported".
>
>
> .. _pipe_capf:
> diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c b/src/gallium/drivers/freedreno/freedreno_screen.c
> index 9d0cdd8..4969774 100644
> --- a/src/gallium/drivers/freedreno/freedreno_screen.c
> +++ b/src/gallium/drivers/freedreno/freedreno_screen.c
> @@ -245,6 +245,7 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
>
> case PIPE_CAP_MAX_VIEWPORTS:
> diff --git a/src/gallium/drivers/i915/i915_screen.c b/src/gallium/drivers/i915/i915_screen.c
> index e2a493b..5e6262b 100644
> --- a/src/gallium/drivers/i915/i915_screen.c
> +++ b/src/gallium/drivers/i915/i915_screen.c
> @@ -259,6 +259,7 @@ i915_get_param(struct pipe_screen *screen, enum pipe_cap cap)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
>
> case PIPE_CAP_MAX_DUAL_SOURCE_RENDER_TARGETS:
> diff --git a/src/gallium/drivers/ilo/ilo_screen.c b/src/gallium/drivers/ilo/ilo_screen.c
> index d5a82ce..d483cd6 100644
> --- a/src/gallium/drivers/ilo/ilo_screen.c
> +++ b/src/gallium/drivers/ilo/ilo_screen.c
> @@ -483,6 +483,7 @@ ilo_get_param(struct pipe_screen *screen, enum pipe_cap param)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
>
> case PIPE_CAP_VENDOR_ID:
> diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c b/src/gallium/drivers/llvmpipe/lp_screen.c
> index e29b008..ed10c46 100644
> --- a/src/gallium/drivers/llvmpipe/lp_screen.c
> +++ b/src/gallium/drivers/llvmpipe/lp_screen.c
> @@ -308,6 +308,7 @@ llvmpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
> }
> /* should only get here on unhandled cases */
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> index d9c9402..83ebd43 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> @@ -181,6 +181,7 @@ nv30_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
>
> case PIPE_CAP_VENDOR_ID:
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> index 56c67e0..e5ce955 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> @@ -224,6 +224,7 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
> case PIPE_CAP_TGSI_PACK_HALF_FLOAT:
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
>
> case PIPE_CAP_VENDOR_ID:
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> index 33dd17e..3e72295 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> @@ -213,6 +213,7 @@ nvc0_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
> case PIPE_CAP_DEVICE_RESET_STATUS_QUERY:
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
>
> case PIPE_CAP_VENDOR_ID:
> diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
> index d1b59ab..4514bcb 100644
> --- a/src/gallium/drivers/r300/r300_screen.c
> +++ b/src/gallium/drivers/r300/r300_screen.c
> @@ -207,6 +207,7 @@ static int r300_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
>
> /* SWTCL-only features. */
> diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
> index e61d928..28185ad 100644
> --- a/src/gallium/drivers/r600/r600_pipe.c
> +++ b/src/gallium/drivers/r600/r600_pipe.c
> @@ -355,6 +355,7 @@ static int r600_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
>
> case PIPE_CAP_MAX_SHADER_PATCH_VARYINGS:
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
> index c2ca943..fe1b423 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -347,6 +347,7 @@ static int si_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
>
> case PIPE_CAP_MAX_SHADER_PATCH_VARYINGS:
> diff --git a/src/gallium/drivers/softpipe/sp_screen.c b/src/gallium/drivers/softpipe/sp_screen.c
> index 29e392b..207b7cd 100644
> --- a/src/gallium/drivers/softpipe/sp_screen.c
> +++ b/src/gallium/drivers/softpipe/sp_screen.c
> @@ -258,6 +258,7 @@ softpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
> }
> /* should only get here on unhandled cases */
> diff --git a/src/gallium/drivers/svga/svga_screen.c b/src/gallium/drivers/svga/svga_screen.c
> index 0f41e4e..1770ff2 100644
> --- a/src/gallium/drivers/svga/svga_screen.c
> +++ b/src/gallium/drivers/svga/svga_screen.c
> @@ -391,6 +391,7 @@ svga_get_param(struct pipe_screen *screen, enum pipe_cap param)
> case PIPE_CAP_DRAW_PARAMETERS:
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
> }
>
> 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
As a follow-up, I'm going to remove most/all of the 'inline' qualifiers
in that file. They're pointless.
> trace_context_destroy(struct pipe_context *_pipe)
> {
> struct trace_context *tr_ctx = trace_context(_pipe);
> @@ -1620,6 +1649,7 @@ trace_context_create(struct trace_screen *tr_scr,
> TR_CTX_INIT(clear_render_target);
> TR_CTX_INIT(clear_depth_stencil);
> TR_CTX_INIT(flush);
> + TR_CTX_INIT(generate_mipmap);
> TR_CTX_INIT(texture_barrier);
> TR_CTX_INIT(memory_barrier);
> TR_CTX_INIT(set_tess_state);
> diff --git a/src/gallium/drivers/vc4/vc4_screen.c b/src/gallium/drivers/vc4/vc4_screen.c
> index 0e28943..b736efb 100644
> --- a/src/gallium/drivers/vc4/vc4_screen.c
> +++ b/src/gallium/drivers/vc4/vc4_screen.c
> @@ -196,6 +196,7 @@ vc4_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
>
> /* Stream output. */
> diff --git a/src/gallium/drivers/virgl/virgl_screen.c b/src/gallium/drivers/virgl/virgl_screen.c
> index e8d82b3..b11e2e3 100644
> --- a/src/gallium/drivers/virgl/virgl_screen.c
> +++ b/src/gallium/drivers/virgl/virgl_screen.c
> @@ -226,6 +226,7 @@ virgl_get_param(struct pipe_screen *screen, enum pipe_cap param)
> case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> + case PIPE_CAP_GENERATE_MIPMAP:
> return 0;
> case PIPE_CAP_VENDOR_ID:
> return 0x1af4;
> 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);
> };
>
>
> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
> index dd76fe5..40f114f 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -642,6 +642,7 @@ enum pipe_cap
> PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL,
> PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL,
> PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT,
> + PIPE_CAP_GENERATE_MIPMAP,
> };
>
> #define PIPE_QUIRK_TEXTURE_BORDER_COLOR_SWIZZLE_NV50 (1 << 0)
> 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);
> + }
> }
>
> /* Fill in the Mesa gl_texture_image fields */
>
The state_tracker changes could go into a separate patch, but not a big
deal.
Reviewed-by: Brian Paul <brianp at vmware.com>
More information about the mesa-dev
mailing list