[Mesa-dev] [PATCH] virgl: add initial shader_storage_buffer_object support. (v2)

[Dave Airlie]

On 24 July 2018 at 03:14, Gert Wollny <gert.wollny at collabora.com> wrote:
> Am Montag, den 23.07.2018, 21:17 +1000 schrieb Dave Airlie:
>> On 23 July 2018 at 17:28, Gert Wollny <gert.wollny at collabora.com>
>> wrote:
>> > Am Montag, den 23.07.2018, 17:13 +1000 schrieb Dave Airlie:
>> > > On 23 July 2018 at 16:46, Gert Wollny <gert.wollny at collabora.com>
>> > > wrote:
>> > > > Am Montag, den 23.07.2018, 09:04 +1000 schrieb Dave Airlie:
>> > > > >
>> > > > > > > +   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.
>> > > >
>> > > > I somehow suspected this, but then one should probably
>> > > > explicitely
>> > > > return zero instead of returning
>> > > > max_shader_buffer_other_stages.
>> > >
>> > > You can never reach this point of code with shader set to
>> > > PIPE_SHADER_COMPUTE.
>> > >
>> > > The outer switch statement defaults to return 0 for compute
>> > > shaders.
>> >
>> > I see, that was not visible in the patch, sorry for the noise.
>>
>> Is that an r-b in disguise? :-)
>
> Well, I had to take a closer look for this, and there is one thing that
>  I'm wondering about: Are the last five macros in virgl_protocol.h for
> the shader buffers actually used? I don't see it, but if they supposed
> to be used somewhere then the offsets for the last three entries seem
> to be off by one, because currently
> VIRGL_SET_SHADER_BUFFER_OFFSET(0) == VIRGL_SET_SHADER_BUFFER_START_SLOT
> == 2

Those macros are meant to be used on the decode side in virglrenderer,
and I just realised they weren't and are wrong, just sent a patch to fix them
up there. I'll fixup these for consistency

Dave.

>
> When this is clarified the patch is
> Reviewed-By: Gert Wollny <gert.wollny at collabora.com>
>
> Best,
> Gert
>>
>> Dave.
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev