[Mesa-dev] [PATCH 03/10] mesa/st: add endian_format field to struct pipe_resource
Roland Scheidegger
sroland at vmware.com
Mon Apr 11 15:34:02 UTC 2016
Am 11.04.2016 um 16:34 schrieb Oded Gabbay:
> This patch adds a new field, called "endian_format", to
> "struct pipe_resource". The new field is of type "enum pipe_endian" and
> can receive one of two values:
> - PIPE_ENDIAN_LITTLE
> - PIPE_ENDIAN_NATIVE
>
> PIPE_ENDIAN_NATIVE is initialized to either PIPE_ENDIAN_LITTLE or
> PIPE_ENDIAN_BIG during build time.
>
> This field is needed to provide information to the H/W drivers about the
> endianess current state or desired state of the resource. In other words,
> for resources that are the source of the operation, this field indicates
> the resource's current memory layout endianess (big or little endian).
> For resources that are the destination of the operation, this field
> indicates the resource's desired memory layout endianess.
>
> This field is mandatory because of how mesa works. When we get into the
> H/W driver functions, the driver *ususally* doesn't know if it is doing a
> CPU->GPU, a GPU->CPU, a CPU->CPU or a GPU->GPU operation, as this
> information is "hidden" by the fact we go through common code
> paths (state tracker layer). This isn't an issue in little-endian
> architecture because both the CPU and GPU works in little-endian mode.
> However, when the CPU is working in big-endian mode, the GPU needs to be
> configured according to the requested operation's direction, because the
> GPU *always* works in little-endian mode.
>
> There are two guidelines for setting this field:
>
> - This field must *never* be checked at the state tracker layer. It can
> only be set in that layer. That way, drivers which don't use this field
> won't be effected at all.
>
> - The values that this field can be set to must be only
> PIPE_ENDIAN_LITTLE or PIPE_ENDIAN_NATIVE. It should never be set
> to PIPE_ENDIAN_BIG directly for the code to work correctly in both endian
> modes.
>
> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
> ---
> src/gallium/include/pipe/p_state.h | 1 +
> src/mesa/state_tracker/st_cb_drawpixels.c | 11 ++++++-
> src/mesa/state_tracker/st_cb_fbo.c | 16 ++++++++++-
> src/mesa/state_tracker/st_cb_readpixels.c | 5 ++++
> src/mesa/state_tracker/st_cb_texture.c | 13 +++++++++
> src/mesa/state_tracker/st_texture.c | 48 +++++++++++++++++++++++++++++++
> 6 files changed, 92 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
> index 9e466ce..a669b3b 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -447,6 +447,7 @@ struct pipe_resource
> struct pipe_screen *screen; /**< screen that this texture belongs to */
> enum pipe_texture_target target; /**< PIPE_TEXTURE_x */
> enum pipe_format format; /**< PIPE_FORMAT_x */
> + enum pipe_endian endian_format; /**< PIPE_ENDIAN_x */
>
> unsigned width0;
> unsigned height0;
> diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c
> index 01ed544..5eafdc0 100644
> --- a/src/mesa/state_tracker/st_cb_drawpixels.c
> +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
> @@ -465,7 +465,6 @@ make_texture(struct st_context *st,
> PIPE_TRANSFER_WRITE, 0, 0,
> width, height, &transfer);
>
> -
> /* Put image into texture transfer.
> * Note that the image is actually going to be upside down in
> * the texture. We deal with that with texcoords.
> @@ -1222,6 +1221,11 @@ copy_stencil_pixels(struct gl_context *ctx, GLint srcx, GLint srcy,
> assert(util_format_get_blockwidth(rbDraw->texture->format) == 1);
> assert(util_format_get_blockheight(rbDraw->texture->format) == 1);
>
> + /* We are going to upload texture to GPU, so mark texture according
> + * to the host's endianess
> + */
> + rbDraw->texture->endian_format = PIPE_ENDIAN_NATIVE;
> +
> /* map the stencil buffer */
> drawMap = pipe_transfer_map(pipe,
> rbDraw->texture,
> @@ -1561,6 +1565,11 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy,
> if (!pt)
> return;
>
> + /* Temporary texture needs to be in GPU format as the swapping happens
> + * when blitting to it
> + */
> + pt->endian_format = PIPE_ENDIAN_LITTLE;
> +
> sv[0] = st_create_texture_sampler_view(st->pipe, pt);
> if (!sv[0]) {
> pipe_resource_reference(&pt, NULL);
> diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c
> index 456ad83..0fcadb2 100644
> --- a/src/mesa/state_tracker/st_cb_fbo.c
> +++ b/src/mesa/state_tracker/st_cb_fbo.c
> @@ -754,6 +754,7 @@ st_MapRenderbuffer(struct gl_context *ctx,
> unsigned usage;
> GLuint y2;
> GLubyte *map;
> + enum pipe_endian saved_endian_format;
>
> if (strb->software) {
> /* software-allocated renderbuffer (probably an accum buffer) */
> @@ -771,9 +772,17 @@ st_MapRenderbuffer(struct gl_context *ctx,
> return;
> }
>
> + /* save current endian format of renderbuffer in case we change it */
> + saved_endian_format = strb->texture->endian_format;
> +
> usage = 0x0;
> - if (mode & GL_MAP_READ_BIT)
> + if (mode & GL_MAP_READ_BIT) {
> usage |= PIPE_TRANSFER_READ;
> + /* reading from the GPU into the texture of the renderbuffer so we
> + * need to do swapping according to host's endianess
> + */
> + strb->texture->endian_format = PIPE_ENDIAN_NATIVE;
> + }
> if (mode & GL_MAP_WRITE_BIT)
> usage |= PIPE_TRANSFER_WRITE;
> if (mode & GL_MAP_INVALIDATE_RANGE_BIT)
> @@ -807,6 +816,11 @@ st_MapRenderbuffer(struct gl_context *ctx,
> *mapOut = NULL;
> *rowStrideOut = 0;
> }
> +
> + /* Need to restore original endian format for all cases renderbuffer is
> + * used as source
> + */
> + strb->texture->endian_format = saved_endian_format;
> }
>
>
> diff --git a/src/mesa/state_tracker/st_cb_readpixels.c b/src/mesa/state_tracker/st_cb_readpixels.c
> index 5153c4b..7686c42 100644
> --- a/src/mesa/state_tracker/st_cb_readpixels.c
> +++ b/src/mesa/state_tracker/st_cb_readpixels.c
> @@ -193,6 +193,11 @@ st_ReadPixels(struct gl_context *ctx, GLint x, GLint y,
> goto fallback;
> }
>
> + /* Because we are reading from the GPU (LE) we want to make sure the result
> + * is in the host's endianess
> + */
> + dst->endian_format = PIPE_ENDIAN_NATIVE;
> +
> memset(&blit, 0, sizeof(blit));
> blit.src.resource = src;
> blit.src.level = strb->surface->u.tex.level;
> diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
> index 3980f5d..8bd3707 100644
> --- a/src/mesa/state_tracker/st_cb_texture.c
> +++ b/src/mesa/state_tracker/st_cb_texture.c
> @@ -1809,6 +1809,14 @@ st_TexSubImage(struct gl_context *ctx, GLuint dims,
> goto fallback;
> }
>
> + /* source texture is created with host endianess */
> + src->endian_format = PIPE_ENDIAN_NATIVE;
> +
> + /* Destination image will be in GPU and all subsequent actions will
> + * be done inside/from GPU, so we need to mark it as LE as we don't want
> + * unnecessary swaps. The only swap we want is from the src to dst */
> + dst->endian_format = PIPE_ENDIAN_LITTLE;
> +
> /* Map source pixels. */
> pixels = _mesa_validate_pbo_teximage(ctx, dims, width, height, depth,
> format, type, pixels, unpack,
> @@ -2262,6 +2270,11 @@ st_GetTexSubImage(struct gl_context * ctx,
> goto fallback;
> }
>
> + /* Because we are reading from the GPU (LE) we want to make sure the result
> + * is in the host's endianess
> + */
> + dst->endian_format = PIPE_ENDIAN_NATIVE;
> +
> /* From now on, we need the gallium representation of dimensions. */
> if (gl_target == GL_TEXTURE_1D_ARRAY) {
> zoffset = yoffset;
> diff --git a/src/mesa/state_tracker/st_texture.c b/src/mesa/state_tracker/st_texture.c
> index 52b0943..a780c80 100644
> --- a/src/mesa/state_tracker/st_texture.c
> +++ b/src/mesa/state_tracker/st_texture.c
> @@ -94,6 +94,54 @@ st_texture_create(struct st_context *st,
> pt.flags = 0;
> pt.nr_samples = nr_samples;
>
> + /* set it to Host endianess by default, except for a couple of gallium
> + * formats, where gallium provides different formats based on endianess */
> + switch (format)
> + {
> + case PIPE_FORMAT_R8G8B8A8_UNORM:
> + case PIPE_FORMAT_R8G8B8X8_UNORM:
> + case PIPE_FORMAT_B8G8R8A8_UNORM:
> + case PIPE_FORMAT_B8G8R8X8_UNORM:
> + case PIPE_FORMAT_A8R8G8B8_UNORM:
> + case PIPE_FORMAT_X8R8G8B8_UNORM:
> + case PIPE_FORMAT_A8B8G8R8_UNORM:
> + case PIPE_FORMAT_X8B8G8R8_UNORM:
> + case PIPE_FORMAT_R8G8B8A8_SNORM:
> + case PIPE_FORMAT_R8G8B8X8_SNORM:
> + case PIPE_FORMAT_A8B8G8R8_SNORM:
> + case PIPE_FORMAT_X8B8G8R8_SNORM:
> + case PIPE_FORMAT_R8G8B8A8_SRGB:
> + case PIPE_FORMAT_R8G8B8X8_SRGB:
> + case PIPE_FORMAT_B8G8R8A8_SRGB:
> + case PIPE_FORMAT_B8G8R8X8_SRGB:
> + case PIPE_FORMAT_A8R8G8B8_SRGB:
> + case PIPE_FORMAT_X8R8G8B8_SRGB:
> + case PIPE_FORMAT_A8B8G8R8_SRGB:
> + case PIPE_FORMAT_X8B8G8R8_SRGB:
> + case PIPE_FORMAT_A8L8_UNORM:
> + case PIPE_FORMAT_L8A8_UNORM:
> + case PIPE_FORMAT_A8L8_SNORM:
> + case PIPE_FORMAT_L8A8_SNORM:
> + case PIPE_FORMAT_A8L8_SRGB:
> + case PIPE_FORMAT_L8A8_SRGB:
> + case PIPE_FORMAT_A16L16_UNORM:
> + case PIPE_FORMAT_L16A16_UNORM:
> + case PIPE_FORMAT_G8R8_UNORM:
> + case PIPE_FORMAT_R8G8_UNORM:
> + case PIPE_FORMAT_G8R8_SNORM:
> + case PIPE_FORMAT_R8G8_SNORM:
> + case PIPE_FORMAT_G16R16_UNORM:
> + case PIPE_FORMAT_R16G16_UNORM:
> + case PIPE_FORMAT_G16R16_SNORM:
> + case PIPE_FORMAT_R16G16_SNORM:
> + pt.endian_format = PIPE_ENDIAN_LITTLE;
> + break;
> +
> + default:
> + pt.endian_format = PIPE_ENDIAN_NATIVE;
> + break;
> + }
> +
> newtex = screen->resource_create(screen, &pt);
>
> assert(!newtex || pipe_is_referenced(&newtex->reference));
>
That would be a gallium: change not mesa/st:
NAK though. First, you're modifying the pipe_resource fields after
creation for instance in st_TexSubImage - these are immutable, so no,
you absolutely can't do that, no way.
I don't claim I know how to solve it properly but it sort of sounds to
me like some more places need to know about endianness than they do
know? Be it state tracker or driver.
Roland
More information about the mesa-dev
mailing list