[Mesa-dev] [PATCH v2 04/11] gallium: add endian_format field to struct pipe_resource
Ilia Mirkin
imirkin at alum.mit.edu
Thu Apr 14 14:01:37 UTC 2016
This feels like an incredibly confused interface to me... you talk
about CPU <-> GPU, but I think that's a misnomer. What happens if you
use up too much VRAM and something gets evicted to GART? Does it get
byteswapped to the CPU-endianness? Are you treating everything as
32-bit packed integers implicitly? What if you have, e.g., a R8 format
(without the GBA bits)? Or even worse, a R16 format, where cpu bytes 0
1 2 3 would have to come out as 1 0 3 2.
This seems like it might be enough for GL 1.3, but this sort of
interface has to account for everything, IMO.
I was also struggling with such issues when getting the nv30 driver to
work on big endian. This was compounded by the issue that the NVIDIA
GPUs have a kinda-sorta-but-not-really big-endian mode which swaps
some things but not others. Basically you need to intelligently be
able to say what "endian" the data is in where, and in order to do
that, you need to know what format it's in. And that's where we hit
some fails, because all the transfer_map/unmap stuff is formatless.
And so there's no way to know whether you should be swapping or not.
[I left these issues unresolved... nv30 on BE sorta works but not
really.]
-ilia
On Thu, Apr 14, 2016 at 8:18 AM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
> 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
> --
> 2.5.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list