[Mesa-dev] [PATCH 03/10] mesa/st: add endian_format field to struct pipe_resource
Oded Gabbay
oded.gabbay at gmail.com
Tue Apr 12 07:27:14 UTC 2016
On Tue, Apr 12, 2016 at 12:15 AM, Marek Olšák <maraeo at gmail.com> 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 <sroland at vmware.com> 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 <oded.gabbay at gmail.com>
>>> ---
>>> 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
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list