On 20 July 2018 at 21:13, Gert Wollny <gert.wol...@collabora.com> wrote:
> There still seem to be a few quirks (see below)
>
> Best,
>>
>> diff --git a/src/gallium/drivers/virgl/virgl_buffer.c
>>
>> +static void virgl_set_shader_buffers(struct pipe_context *ctx,
>> + enum pipe_shader_type shader,
>> + unsigned start_slot, unsigned
>> count,
>> + const struct pipe_shader_buffer
>> *buffers)
>> +{
>> + struct virgl_context *vctx = virgl_context(ctx);
>> + struct virgl_screen *rs = virgl_screen(ctx->screen);
>> +
>> + for (unsigned i = 0; i < count; i++) {
>> + unsigned idx = start_slot + i;
>> +
>> + if (buffers) {
>> + if (buffers[i].buffer) {
>> + pipe_resource_reference(&vctx->ssbos[shader][idx],
>> buffers[i].buffer);
>> + continue;
>> + }
>> + }
>> + pipe_resource_reference(&vctx->ssbos[shader][idx], NULL);
>> + }
>> +
>> + uint32_t max_shader_buffer = shader == PIPE_SHADER_FRAGMENT ?
>> + rs->caps.caps.v2.max_shader_buffer_frag_compute :
> Shouldn't max_shader_buffer_frag_compute also be used for
> PIPE_SHADER_COMPUTE?
virgl doesn't support compute shaders yet. Adding support for them is in
the future patches.
>> + struct pipe_resource
>> *ssbos[PIPE_SHADER_TYPES][PIPE_MAX_SHADER_BUFFERS];
>> int num_transfers;
>> int num_draws;
>> struct list_head to_flush_bufs;
>> diff --git a/src/gallium/drivers/virgl/virgl_encode.c
>> b/src/gallium/drivers/virgl/virgl_encode.c
>> index c1af01b6fdf..b09366dcee6 100644
>> --- a/src/gallium/drivers/virgl/virgl_encode.c
>> +++ b/src/gallium/drivers/virgl/virgl_encode.c
>> @@ -918,3 +918,28 @@ int virgl_encode_set_tess_state(struct
>> virgl_context *ctx,
>> virgl_encoder_write_dword(ctx->cbuf, fui(inner[i]));
>> return 0;
>> }
>> +
>> +int virgl_encode_set_shader_buffers(struct virgl_context *ctx,
>> + enum pipe_shader_type shader,
>> + unsigned start_slot, unsigned
>> count,
>> + const struct pipe_shader_buffer
>> *buffers)
>> +{
>> + int i;
>> + virgl_encoder_write_cmd_dword(ctx,
>> VIRGL_CMD0(VIRGL_CCMD_SET_SHADER_BUFFERS, 0,
>> VIRGL_SET_SHADER_BUFFER_SIZE(count)));
>> +
>> + virgl_encoder_write_dword(ctx->cbuf, shader);
>> + virgl_encoder_write_dword(ctx->cbuf, start_slot);
>> + for (i = 0; i < count; i++) {
>> + if (buffers) {
> Is it legal to have (count > 0) but buffers= NULL?
Yes, it appears we get 0, 16, NULL to reset all the buffers,
at least I definitely remember adding this because it was possible.
>
>> @@ -302,6 +303,8 @@ struct virgl_caps_v2 {
>> uint32_t capability_bits;
>> uint32_t msaa_sample_positions[8];
>> uint32_t max_vertex_attrib_stride;
>> + uint32_t max_shader_buffer_frag_compute;
>> + uint32_t max_shader_buffer_other_stages;
> On the host you use a combined value
> uint32_t shader_storage_buffer_object_maximums;
> Maybe using two uint16_t on both sides would be better?
The final git version on those host side looks like this.
>> + case PIPE_SHADER_CAP_MAX_SHADER_BUFFERS:
>> + if (shader == PIPE_SHADER_FRAGMENT)
>> + return vscreen-
>> >caps.caps.v2.max_shader_buffer_frag_compute;
> Same like above: PIPE_SHADER_COMPUTE?
We don't support COMPUTE shaders yet.
Dave.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
