[snip]
>> +static unsigned
>> +tc_improve_map_buffer_flags(struct threaded_context *tc,
>> + struct threaded_resource *tres, unsigned
>> usage,
>> + unsigned offset, unsigned size)
>> +{
>> + /* Handle CPU reads trivially. */
>> + if (usage & PIPE_TRANSFER_READ) {
>> + /* Driver aren't allowed to do buffer invalidations. */
>> + return (usage & ~PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE) |
>> + TC_TRANSFER_MAP_NO_INVALIDATE |
>> + TC_TRANSFER_MAP_IGNORE_VALID_RANGE;
>> + }
>> +
>> + /* Sparse buffers can't be mapped directly. Use a staging buffer. */
>> + if (tres->b.flags & PIPE_RESOURCE_FLAG_SPARSE) {
>> + return (usage & ~(PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE |
>> + PIPE_TRANSFER_UNSYNCHRONIZED)) |
>> + PIPE_TRANSFER_DISCARD_RANGE |
>
>
> Why are we allowed to discard here? Even when a range is mapped only for
> writing, we can't just assume that the whole range will be written by the
> application.
>
> Also, why do we clear the unsynchronized flag? As I understand the code,
we
> do need to synchronize the threads because we're going to use a staging
> buffer. But theoretically, if the driver had a way to do the staging copy
> without waiting for previously submitted draws, it could do so. So... I
> don't think it has a visible effect right now, but I'd rather not remove
the
> unsynchronized flag here.
>
> This may need a bit of clarification in the big comment in the header
file.
That code was indeed completely wrong. I've changed it to this and I moved
this block to the beginning of the function (before the READ handling):
/* Sparse buffers can't be mapped directly and can't be reallocated
* (fully invalidated). That may just be a radeonsi limitation, but
* the threaded context must obey it with radeonsi.
*/
if (tres->b.flags & PIPE_RESOURCE_FLAG_SPARSE) {
/* We can use DISCARD_RANGE instead of full discard. This is the only
* fast path for sparse buffers that doesn't need thread
synchronization.
*/
if (usage & PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE)
usage |= PIPE_TRANSFER_DISCARD_RANGE;
/* Allow DISCARD_WHOLE_RESOURCE and infering UNSYNCHRONIZED in
drivers.
* The threaded context doesn't do unsychronized mappings and
invalida-
* tions of sparse buffers, therefore a correct driver behavior won't
* result in an incorrect behavior with the threaded context.
*/
return usage;
}
>
>
>
>> + TC_TRANSFER_MAP_NO_INVALIDATE |
>> + TC_TRANSFER_MAP_IGNORE_VALID_RANGE;
>> + }
[snip]
>> + ttrans->b.level = 0;
>> + ttrans->b.usage = usage;
>> + ttrans->b.box = *box;
>> + ttrans->b.stride = 0;
>> + ttrans->b.layer_stride = 0;
>> + *transfer = &ttrans->b;
>> + return map + (box->x % tc->map_buffer_alignment);
>> + }
>> + }
>> +
>> + /* Unsychronized buffer mappings don't have to synchronize the
thread.
>> */
>> + if (!(usage & TC_TRANSFER_MAP_THREADED_UNSYNC))
>> + tc_sync_msg(tc, resource->target != PIPE_BUFFER ? " texture" :
>> + usage & PIPE_TRANSFER_DISCARD_RANGE ? "
>> discard_range" :
>> + usage & PIPE_TRANSFER_READ ? " read" : " ??");
>> +
>> + return pipe->transfer_map(pipe, tres->latest ? tres->latest :
>> resource,
>> + level, usage, box, transfer);
>
>
> The ternary operator here should be unnecessary -- tres->latest should
> always be non-NULL.
It's necessary because this codepath is also used by textures where
tres->latest == NULL because it's not initialized explicitly.
[snip]
>> +
>> +static boolean
>> +tc_generate_mipmap(struct pipe_context *_pipe,
>> + struct pipe_resource *res,
>> + enum pipe_format format,
>> + unsigned base_level,
>> + unsigned last_level,
>> + unsigned first_layer,
>> + unsigned last_layer)
>> +{
>> + struct threaded_context *tc = threaded_context(_pipe);
>> + struct pipe_context *pipe = tc->pipe;
>> + struct pipe_screen *screen = pipe->screen;
>> + unsigned bind = PIPE_BIND_SAMPLER_VIEW;
>> +
>> + if (util_format_is_depth_or_stencil(format))
>> + bind = PIPE_BIND_DEPTH_STENCIL;
>> + else
>> + bind = PIPE_BIND_RENDER_TARGET;
>> +
>> + if (!screen->is_format_supported(screen, format, res->target,
>> + res->nr_samples, bind))
>> + return false;
>
>
> This feels like the kind of thing the state tracker should be checking
> before it calls this function...
True, but the Gallium interface is currently defined such that drivers do
it.
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
