I haven't read the following patches, but it looks like you can override endian_format by passing that flag into transfer_map.
Alternatively, you could do the byteswapping in transfer_map by always using the blit there. That means all resources would be in LE in hw and the pipe_resource flag could be moved to pipe_transfer. Marek On Mon, Apr 11, 2016 at 5:34 PM, Roland Scheidegger <[email protected]> wrote: > 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 <[email protected]> >> --- >> 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 > > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
