[Mesa-dev] [PATCH] st/mesa: fix pbo uploads

[Ilia Mirkin]

On Thu, Feb 18, 2016 at 11:36 AM, Nicolai Hähnle
<nicolai.haehnle at amd.com> wrote:
> On 18.02.2016 01:26, Ilia Mirkin wrote:
>>
>> On Thu, Feb 18, 2016 at 1:07 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>
>>>   - LOD must be provided in .w for TXF (even for buffer textures)
>>>   - User buffer must be valid at draw time
>
>
> Good catch.
>
>>>   - Must have a sampler associated with the sampler view
>
>
> Fine by me. Honestly, I'd prefer to relax the requirements for TXF/buffer
> textures, but if that's difficult for Nouveau I can accept it.

I have no clue why nouveau breaks without it. It should totally work.
Even in the instruction decoding by nvdisasm, the sampler reference
isn't even listed. And yet... it doesn't work, and I just get all
black -- on both a GT215 (tesla, dx10) and a GF108 (fermi, dx11).
Perhaps the sampler reference encoded within the op is required to
exist, even if it's not used? Perhaps the TIC (tex descriptor) somehow
refers to the TSC (sampler descriptor) and could be made to somehow
not refer to it? Unfortunately I don't know, and am not extremely
inclined to RE it -- the blob driver always uses "linked tsc" mode,
which means that there's a 1:1 correspondence between textures and
samplers; I assume that this mode is not used in DX10/DX11, but I have
no way of tracing that. And this situation never comes up in st/mesa
otherwise...

>
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

Thanks!

>
>
>>>
>>> This makes PBO uploads work again on nouveau.
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>
>>
>> Just realized that I also need to add CSO_BIT_FRAGMENT_SAMPLERS to the
>> save state. And also fixed a minor warning in my tree. Too minor to
>> resend, but final version available here:
>> https://github.com/imirkin/mesa/commit/c246a0e.patch
>>
>>>   src/mesa/state_tracker/st_cb_texture.c | 26 ++++++++++++++++----------
>>>   1 file changed, 16 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/src/mesa/state_tracker/st_cb_texture.c
>>> b/src/mesa/state_tracker/st_cb_texture.c
>>> index d09c360..eb09f3d 100644
>>> --- a/src/mesa/state_tracker/st_cb_texture.c
>>> +++ b/src/mesa/state_tracker/st_cb_texture.c
>>> @@ -1272,10 +1272,11 @@ create_pbo_upload_fs(struct st_context *st)
>>>                         ureg_scalar(ureg_src(temp0), TGSI_SWIZZLE_X));
>>>      }
>>>
>>> +   /* temp0.w = 0 */
>>> +   ureg_MOV(ureg, ureg_writemask(temp0, TGSI_WRITEMASK_W),
>>> ureg_imm1u(ureg, 0));
>>> +
>>>      /* out = txf(sampler, temp0.x) */
>>> -   ureg_TXF(ureg, out, TGSI_TEXTURE_BUFFER,
>>> -                  ureg_scalar(ureg_src(temp0), TGSI_SWIZZLE_X),
>>> -                  sampler);
>>> +   ureg_TXF(ureg, out, TGSI_TEXTURE_BUFFER, ureg_src(temp0), sampler);
>>>
>>>      ureg_release_temporary(ureg, temp0);
>>>
>>> @@ -1353,6 +1354,7 @@ try_pbo_upload_common(struct gl_context *ctx,
>>>            + (upload_height - 1 + (depth - 1) * image_height) * stride;
>>>         struct pipe_sampler_view templ;
>>>         struct pipe_sampler_view *sampler_view;
>>> +      struct pipe_sampler_state sampler = {0}, *samplers[1] =
>>> {&sampler};
>>>
>>>         /* This should be ensured by Mesa before calling our callbacks */
>>>         assert((last_element + 1) * bytes_per_pixel <= buffer->width0);
>>> @@ -1361,6 +1363,7 @@ try_pbo_upload_common(struct gl_context *ctx,
>>>            goto fail;
>>>
>>>         memset(&templ, 0, sizeof(templ));
>>> +      templ.target = PIPE_BUFFER;
>>>         templ.format = src_format;
>>>         templ.u.buf.first_element = first_element;
>>>         templ.u.buf.last_element = last_element;
>>> @@ -1376,6 +1379,8 @@ try_pbo_upload_common(struct gl_context *ctx,
>>>         cso_set_sampler_views(cso, PIPE_SHADER_FRAGMENT, 1,
>>> &sampler_view);
>>>
>>>         pipe_sampler_view_reference(&sampler_view, NULL);
>>> +
>>> +      cso_set_samplers(cso, PIPE_SHADER_FRAGMENT, 1, samplers);
>>>      }
>>>
>>>      /* Upload vertices */
>>> @@ -1423,16 +1428,17 @@ try_pbo_upload_common(struct gl_context *ctx,
>>>      }
>>>
>>>      /* Upload constants */
>>> +   /* Note: the user buffer must be valid until draw time */
>>> +   struct {
>>> +      int32_t xoffset;
>>> +      int32_t yoffset;
>>> +      int32_t stride;
>>> +      int32_t image_size;
>>> +   } constants;
>>> +
>>>      {
>>>         struct pipe_constant_buffer cb;
>>>
>>> -      struct {
>>> -         int32_t xoffset;
>>> -         int32_t yoffset;
>>> -         int32_t stride;
>>> -         int32_t image_size;
>>> -      } constants;
>>> -
>>>         constants.xoffset = -xoffset + skip_pixels;
>>>         constants.yoffset = -yoffset;
>>>         constants.stride = stride;
>>> --
>>> 2.4.10
>>>
>