[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 23:35:45 UTC 2016
On Thu, Apr 14, 2016 at 7:26 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> 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).
Any solution for persistent/coherent mappings will have to require
that cpu and gpu agree on the format. However ARB_buffer_storage is
the only way to access that, and losing that on a LE/BE hybrid system
doesn't seem like the worst thing.
-ilia
More information about the mesa-dev
mailing list