[Mesa-dev] [PATCH v2 04/11] gallium: add endian_format field to struct pipe_resource
Roland Scheidegger
sroland at vmware.com
Thu Apr 14 23:26:18 UTC 2016
Am 14.04.2016 um 14:18 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 or state trackers that 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.
>
> v2:
>
> This field is now initialized *only* at creation time, as pipe_resource
> is an immutable object. For this change I did the following:
>
> - st_create_texture now receives a pipe_endian parameter, so whenever
> a pipe_resource object is created using st_create_texture, the calling
> function can set its endian_format during creation, same as pipe_format.
>
> - When a pipe_resource is created using a template, I initialized the
> endian_format field of the template so the created object will contain
> the correct endian value.
>
> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
> ---
> src/gallium/include/pipe/p_state.h | 1 +
> src/mesa/state_tracker/st_cb_bitmap.c | 7 +++++--
> src/mesa/state_tracker/st_cb_drawpixels.c | 15 +++++++++------
> src/mesa/state_tracker/st_cb_readpixels.c | 4 ++++
> src/mesa/state_tracker/st_cb_texture.c | 18 ++++++++++++++----
> src/mesa/state_tracker/st_texture.c | 7 +++++--
> src/mesa/state_tracker/st_texture.h | 3 ++-
> 7 files changed, 40 insertions(+), 15 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_bitmap.c b/src/mesa/state_tracker/st_cb_bitmap.c
> index 4fd2dfe..a4f241c 100644
> --- a/src/mesa/state_tracker/st_cb_bitmap.c
> +++ b/src/mesa/state_tracker/st_cb_bitmap.c
> @@ -46,6 +46,7 @@
> #include "st_program.h"
> #include "st_cb_bitmap.h"
> #include "st_texture.h"
> +#include "st_format.h"
>
> #include "pipe/p_context.h"
> #include "pipe/p_defines.h"
> @@ -155,7 +156,8 @@ make_bitmap_texture(struct gl_context *ctx, GLsizei width, GLsizei height,
> */
> pt = st_texture_create(st, st->internal_target, st->bitmap.tex_format,
> 0, width, height, 1, 1, 0,
> - PIPE_BIND_SAMPLER_VIEW);
> + PIPE_BIND_SAMPLER_VIEW,
> + st_get_endian_by_format(st->bitmap.tex_format));
> if (!pt) {
> _mesa_unmap_pbo_source(ctx, unpack);
> return NULL;
> @@ -371,7 +373,8 @@ reset_cache(struct st_context *st)
> st->bitmap.tex_format, 0,
> BITMAP_CACHE_WIDTH, BITMAP_CACHE_HEIGHT,
> 1, 1, 0,
> - PIPE_BIND_SAMPLER_VIEW);
> + PIPE_BIND_SAMPLER_VIEW,
> + st_get_endian_by_format(st->bitmap.tex_format));
> }
>
>
> diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c
> index c3e05bb..96d97ca 100644
> --- a/src/mesa/state_tracker/st_cb_drawpixels.c
> +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
> @@ -360,12 +360,13 @@ internal_format(struct gl_context *ctx, GLenum format, GLenum type)
> */
> static struct pipe_resource *
> alloc_texture(struct st_context *st, GLsizei width, GLsizei height,
> - enum pipe_format texFormat, unsigned bind)
> + enum pipe_format texFormat, unsigned bind,
> + enum pipe_endian endian_format)
> {
> struct pipe_resource *pt;
>
> pt = st_texture_create(st, st->internal_target, texFormat, 0,
> - width, height, 1, 1, 0, bind);
> + width, height, 1, 1, 0, bind, endian_format);
>
> return pt;
> }
> @@ -453,7 +454,8 @@ make_texture(struct st_context *st,
> return NULL;
>
> /* alloc temporary texture */
> - pt = alloc_texture(st, width, height, pipeFormat, PIPE_BIND_SAMPLER_VIEW);
> + pt = alloc_texture(st, width, height, pipeFormat, PIPE_BIND_SAMPLER_VIEW,
> + st_get_endian_by_format(pipeFormat));
> if (!pt) {
> _mesa_unmap_pbo_source(ctx, unpack);
> return NULL;
> @@ -473,7 +475,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.
> @@ -1569,8 +1570,10 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy,
> readW = MAX2(0, readW);
> readH = MAX2(0, readH);
>
> - /* Allocate the temporary texture. */
> - pt = alloc_texture(st, width, height, srcFormat, srcBind);
> + /* Allocate the temporary texture, with the same endianess as that of the
> + * source texture endianess. */
> + pt = alloc_texture(st, width, height, srcFormat, srcBind,
> + rbRead->texture->endian_format);
> if (!pt)
> return;
>
> diff --git a/src/mesa/state_tracker/st_cb_readpixels.c b/src/mesa/state_tracker/st_cb_readpixels.c
> index 393b881..3d77bea 100644
> --- a/src/mesa/state_tracker/st_cb_readpixels.c
> +++ b/src/mesa/state_tracker/st_cb_readpixels.c
> @@ -183,6 +183,10 @@ st_ReadPixels(struct gl_context *ctx, GLint x, GLint y,
> dst_templ.format = dst_format;
> dst_templ.bind = bind;
> dst_templ.usage = PIPE_USAGE_STAGING;
> + /* Because we are reading from the GPU (LE) we want to make sure the result
> + * is in the host's endianess
> + */
> + dst_templ.endian_format = PIPE_ENDIAN_NATIVE;
>
> st_gl_texture_dims_to_pipe_dims(GL_TEXTURE_2D, width, height, 1,
> &dst_templ.width0, &dst_templ.height0,
> diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
> index a18b08b..307d8cb 100644
> --- a/src/mesa/state_tracker/st_cb_texture.c
> +++ b/src/mesa/state_tracker/st_cb_texture.c
> @@ -517,7 +517,8 @@ guess_and_alloc_texture(struct st_context *st,
> ptHeight,
> ptDepth,
> ptLayers, 0,
> - bindings);
> + bindings,
> + st_get_endian_by_format(fmt));
>
> stObj->lastLevel = lastLevel;
>
> @@ -604,7 +605,8 @@ st_AllocTextureImageBuffer(struct gl_context *ctx,
> ptHeight,
> ptDepth,
> ptLayers, 0,
> - bindings);
> + bindings,
> + st_get_endian_by_format(format));
> return stImage->pt != NULL;
> }
> }
> @@ -1791,6 +1793,8 @@ st_TexSubImage(struct gl_context *ctx, GLuint dims,
> src_templ.format = src_format;
> src_templ.bind = PIPE_BIND_SAMPLER_VIEW;
> src_templ.usage = PIPE_USAGE_STAGING;
> + /* source texture is created with host endianess */
> + src_templ.endian_format = PIPE_ENDIAN_NATIVE;
>
> st_gl_texture_dims_to_pipe_dims(gl_target, width, height, depth,
> &src_templ.width0, &src_templ.height0,
> @@ -2253,6 +2257,10 @@ st_GetTexSubImage(struct gl_context * ctx,
> dst_templ.format = dst_format;
> dst_templ.bind = bind;
> dst_templ.usage = PIPE_USAGE_STAGING;
> + /* Because we are reading from the GPU (LE) we want to make sure the result
> + * is in the host's endianess
> + */
> + dst_templ.endian_format = PIPE_ENDIAN_NATIVE;
>
> st_gl_texture_dims_to_pipe_dims(gl_target, width, height, depth,
> &dst_templ.width0, &dst_templ.height0,
> @@ -2868,7 +2876,8 @@ st_finalize_texture(struct gl_context *ctx,
> ptHeight,
> ptDepth,
> ptLayers, ptNumSamples,
> - bindings);
> + bindings,
> + st_get_endian_by_format(firstImageFormat));
>
> if (!stObj->pt) {
> _mesa_error(ctx, GL_OUT_OF_MEMORY, "glTexImage");
> @@ -2974,7 +2983,8 @@ st_AllocTextureStorage(struct gl_context *ctx,
> ptHeight,
> ptDepth,
> ptLayers, num_samples,
> - bindings);
> + bindings,
> + st_get_endian_by_format(fmt));
> if (!stObj->pt)
> return GL_FALSE;
>
> diff --git a/src/mesa/state_tracker/st_texture.c b/src/mesa/state_tracker/st_texture.c
> index 52b0943..9058b3d 100644
> --- a/src/mesa/state_tracker/st_texture.c
> +++ b/src/mesa/state_tracker/st_texture.c
> @@ -62,7 +62,8 @@ st_texture_create(struct st_context *st,
> GLuint depth0,
> GLuint layers,
> GLuint nr_samples,
> - GLuint bind )
> + GLuint bind,
> + enum pipe_endian endian_format )
> {
> struct pipe_resource pt, *newtex;
> struct pipe_screen *screen = st->pipe->screen;
> @@ -93,6 +94,7 @@ st_texture_create(struct st_context *st,
> pt.bind = bind;
> pt.flags = 0;
> pt.nr_samples = nr_samples;
> + pt.endian_format = endian_format;
>
> newtex = screen->resource_create(screen, &pt);
>
> @@ -412,7 +414,8 @@ st_create_color_map_texture(struct gl_context *ctx)
>
> /* create texture for color map/table */
> pt = st_texture_create(st, PIPE_TEXTURE_2D, format, 0,
> - texSize, texSize, 1, 1, 0, PIPE_BIND_SAMPLER_VIEW);
> + texSize, texSize, 1, 1, 0, PIPE_BIND_SAMPLER_VIEW,
> + st_get_endian_by_format(format));
> return pt;
> }
>
> diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h
> index d8cd7c7..6122622 100644
> --- a/src/mesa/state_tracker/st_texture.h
> +++ b/src/mesa/state_tracker/st_texture.h
> @@ -183,7 +183,8 @@ st_texture_create(struct st_context *st,
> GLuint depth0,
> GLuint layers,
> GLuint nr_samples,
> - GLuint tex_usage );
> + GLuint tex_usage,
> + enum pipe_endian endian_format );
>
>
> extern void
>
This is now better as it doesn't violate the contract of the immutable
objects.
However, I have to agree with Ilia, this doesn't really seem like a
clean solution. Having to create differently endianness flagged
resources depending on what they are intended to be used for just seems
wrong. That's just not how those resource objects are supposed to work.
Doing workarounds at transfer_map time would seem better. However, I
have no idea how either of these solutions is going to work with
coherent/persistent or whatnot mappings, I think there's still something
wrong (but I'm no expert on endianness issues, thinking about it too
much gives me a headache).
Roland
More information about the mesa-dev
mailing list