On Tue, Apr 12, 2016 at 12:15 AM, Marek Olšák <[email protected]> wrote: > I haven't read the following patches, but it looks like you can > override endian_format by passing that flag into transfer_map.
I think I see what you mean. I'll try to follow that approach. Oded > > 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
