On Wed, Dec 6, 2017 at 4:48 PM, Eric Anholt <e...@anholt.net> wrote:
> Rob Clark <robdcl...@gmail.com> writes:
>
>> Add a new helper that drivers can use to emulate various things that
>> need special handling in particular in transfer_map:
>>
>> 1) z32_s8x24.. gl/gallium treats this as a single buffer with depth
>> and stencil interleaved but hardware frequently treats this as
>> separate z32 and s8 buffers. Special pack/unpack handling is
>> needed in transfer_map/unmap to pack/unpack the exposed buffer
>>
>> 2) fake RGTC.. GPUs designed with GLES in mind, but which can other-
>> wise do GL3, if native RGTC is not supported it can be emulated
>> by converting to uncompressed internally, but needs pack/unpack
>> in transfer_map/unmap
>>
>> 3) MSAA resolves in the transfer_map() case
>>
>> v2: add MSAA resolve based on Eric's "gallium: Add helpers for MSAA
>> resolves in pipe_transfer_map()/unmap()." patch; avoid wrapping
>> pipe_resource, to make it possible for drivers to use both this
>> and threaded_context.
>
> The driver side is clean enough with this layer that I'm pretty happy
> now. Just one significant review comment, then I think we'll be
> ready...
>
>> +static void
>> +flush_region(struct pipe_context *pctx, struct pipe_transfer *ptrans,
>> + const struct pipe_box *box)
>> +{
>> + struct u_transfer *trans = u_transfer(ptrans);
>> + enum pipe_format format = ptrans->resource->format;
>> + unsigned width = ptrans->box.width;
>> + unsigned height = ptrans->box.height;
>
> It seems silly to be implementing flush_region and ignoring the box
> argument to flush the entire mapped region on every call. We should
> either drop this implementation in favor of the no-op and flush at
> unmap, or actually use the box in the flushes.
oh, whoops.. and I guess in theory I should use those dimensions for
the MSAA blit too..
> That said, I don't think you can reach flush_region with explicit flush
> for non-buffer resources?
hmm, not 100% sure about the APIs on the GL side of things, but I
think if that were the case mesa/st would dtrt. (I guess it could be
different w/ gallium9, not that I have a big collection of windows arm
games to play :-P)
>> +
>> + if (!(ptrans->usage & PIPE_TRANSFER_WRITE))
>> + return;
>> +
>> + if (trans->ss) {
>> + struct pipe_blit_info blit;
>> + memset(&blit, 0, sizeof(blit));
>> +
>> + blit.src.resource = trans->ss;
>> + blit.src.format = trans->ss->format;
>> + blit.src.box.width = ptrans->box.width;
>> + blit.src.box.height = ptrans->box.height;
>> + blit.src.box.depth = 1;
>> +
>> + blit.dst.resource = ptrans->resource;
>> + blit.dst.format = ptrans->resource->format;
>> + blit.dst.level = ptrans->level;
>> + blit.dst.box = ptrans->box;
>> +
>> + blit.mask = util_format_get_mask(ptrans->resource->format);
>> + blit.filter = PIPE_TEX_FILTER_NEAREST;
>> +
>> + pctx->blit(pctx, &blit);
>> +
>> + return;
>> + }
>> +
>> + switch (format) {
>> + case PIPE_FORMAT_Z32_FLOAT_S8X24_UINT:
>> + util_format_z32_float_s8x24_uint_unpack_z_float(trans->ptr,
>> + trans->trans->stride,
>> + trans->staging,
>> + ptrans->stride,
>> + width, height);
>> + /* fallthru */
>> + case PIPE_FORMAT_X32_S8X24_UINT:
>> + util_format_z32_float_s8x24_uint_unpack_s_8uint(trans->ptr2,
>> + trans->trans2->stride,
>> + trans->staging,
>> + ptrans->stride,
>> + width, height);
>> + break;
>> + case PIPE_FORMAT_RGTC1_UNORM:
>> + case PIPE_FORMAT_RGTC1_SNORM:
>> + case PIPE_FORMAT_LATC1_UNORM:
>> + case PIPE_FORMAT_LATC1_SNORM:
>> + util_format_rgtc1_unorm_unpack_rgba_8unorm(trans->ptr,
>> + trans->trans->stride,
>> + trans->staging,
>> + ptrans->stride,
>> + width, height);
>> + break;
>> + case PIPE_FORMAT_RGTC2_UNORM:
>> + case PIPE_FORMAT_RGTC2_SNORM:
>> + case PIPE_FORMAT_LATC2_UNORM:
>> + case PIPE_FORMAT_LATC2_SNORM:
>> + util_format_rgtc2_unorm_unpack_rgba_8unorm(trans->ptr,
>> + trans->trans->stride,
>> + trans->staging,
>> + ptrans->stride,
>> + width, height);
>> + break;
>> + default:
>> + assert(!"Unexpected staging transfer type");
>> + break;
>> + }
>> +}
>
>> diff --git a/src/gallium/auxiliary/util/u_transfer_helper.h
>> b/src/gallium/auxiliary/util/u_transfer_helper.h
>> new file mode 100644
>> index 00000000000..392b34f0697
>> --- /dev/null
>> +++ b/src/gallium/auxiliary/util/u_transfer_helper.h
>> @@ -0,0 +1,132 @@
>> +/*
>> + * Copyright © 2017 Red Hat
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> IN THE
>> + * SOFTWARE.
>> + */
>> +
>> +#ifndef _U_TRANSFER_HELPER_H
>> +#define _U_TRANSFER_HELPER_H
>> +
>> +#include "pipe/p_state.h"
>> +#include "pipe/p_context.h"
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +/* A helper to implement various "lowering" for transfers:
>> + *
>> + * - exposing separate z32 and s8 as z32x24s8
>> + * - fake RGTC support for GLES class hardware which needs it to expose
>> GL3+
>> + * - MSAA resolves
>> + *
>> + * To use this, drivers should:
>> + *
>> + * 1) populate u_transfer_vtbl and plug that into
>> pipe_screen::transfer_helper
>> + * 2) plug the the transfer helpers into pipe_screen/pipe_context
>
> s/the the/the/
opps
>> + *
>> + * To avoid subclassing pipe_resource (and conflicting with
>> threaded_context)
>> + * the vtbl contains setter/getter methods used for fake_rgct &
>> separate_stencil
>> + * to access the internal_format and separate stencil buffer.
>> + */
>> +
>> +struct u_transfer_vtbl {
>> + /* NOTE I am not expecting resource_create_from_handle() or
>> + * resource_create_with_modifiers() paths to be creating any
>> + * resources that need special handling. Otherwise they would
>> + * need to be wrapped too.
>> + */
>> + struct pipe_resource * (*resource_create)(struct pipe_screen *pscreen,
>> + const struct pipe_resource
>> *templ);
>> +
>> + void (*resource_destroy)(struct pipe_screen *pscreen,
>> + struct pipe_resource *prsc);
>> +
>> + void *(*transfer_map)(struct pipe_context *pctx,
>> + struct pipe_resource *prsc,
>> + unsigned level,
>> + unsigned usage,
>> + const struct pipe_box *box,
>> + struct pipe_transfer **pptrans);
>> +
>> +
>> + void (*transfer_flush_region)(struct pipe_context *pctx,
>> + struct pipe_transfer *ptrans,
>> + const struct pipe_box *box);
>> +
>> + void (*transfer_unmap)(struct pipe_context *pctx,
>> + struct pipe_transfer *ptrans);
>> +
>> + /*
>> + * auxiliary methods to access internal format, stencil:
>> + */
>> +
>> + /**
>> + * Must be implemented if separate_z32s8 or fake_rgtc is used. The
>> + * internal_format is the format the resource was created with. In
>> + * the case of separate_z32s8 or fake_rgtc, prsc->format is set back
>> + * to the state tracker visible format (Z32_FLOAT_S8X24_UINT or
>> + * PIPE_FORMAT_{RTGC,LATC}* after the resource is created.
>> + */
>> + enum pipe_format (*get_internal_format)(struct pipe_resource *prsc);
>> +
>> + /**
>> + * Must be implemented if separate_z32s8 is used. Used to set/get
>> + * the separate s8 stencil buffer.
>> + */
>> + void (*set_stencil)(struct pipe_resource *prsc, struct pipe_resource
>> *stencil);
>> + struct pipe_resource *(*get_stencil)(struct pipe_resource *prsc);
>
> Maybe we should note that these two ops are intended to be pointer
> assignments, not refcounted?
Yeah, probably.. and I guess document who destroys the stencil
resource. I'm pretty sure I was leaking those before in freedreno..
BR,
-R
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
