[Mesa-dev] [PATCH] virgl: add shader offset alignment to to v2 caps struct

[Gert Wollny]

Am Montag, den 04.06.2018, 09:07 -0700 schrieb Gurchetan Singh:
> The tgsi_invariant code never landed in the Mesa side.  Is the caps
> code is resilient enough to handle this case?  From my understanding
> the guest caps and host caps don't have to match exactly, and the
> "right thing" will be done.

I don't think so. When I implemented what I sent as RFC to the virgl
list, the tgsi_invariant and shader_buffer_offset_alignment were in the
virglrenderer caps.v2 struct but not in the guest side version of the
struct, and since I added the new values after these two in 
virglrenderer the output was garbage, but after adding these two values
also in the guest side struct the output was okay.

Brousing through the qemu code it seems to me that the caps structure 
is simply send as a blob, and both sides have to know the layout to
make sense of it. 

Best, 
Gert

> On Mon, Jun 4, 2018 at 6:03 AM Gert Wollny <gw.fossdev at gmail.com>
> wrote:
> > 
> > Am Donnerstag, den 12.04.2018, 18:11 -0700 schrieb
> > gurchetansingh at chromium.org:
> > > This is the SSBO analogue to fe0647. User supplied data must
> > > be a multiple of GL_SHADER_STORAGE_BUFFER_OFFSET_ALIGNMENT.
> > > 
> > > This fixes 44 GLES31 tests on airlied@'s GLES31 sketch branches
> > > with
> > > Nvidia hardware, but this patch standalone can applied to master.
> > > The
> > > alignment restriction on Nvidia is 32, hence the default value.
> > > 
> > > Example tests:
> > >    dEQP-GLES31.functional.ssbo.layout.random.all_shared_buffer.0
> > >    dEQP-
> > > GLES31.functional.ssbo.layout.multi_basic_types.single_buffer.std
> > > 430
> > > 
> > > v2: Move to a better place in case statement
> > > ---
> > >  src/gallium/drivers/virgl/virgl_hw.h     | 1 +
> > >  src/gallium/drivers/virgl/virgl_screen.c | 3 ++-
> > >  src/gallium/drivers/virgl/virgl_winsys.h | 1 +
> > >  3 files changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/gallium/drivers/virgl/virgl_hw.h
> > > b/src/gallium/drivers/virgl/virgl_hw.h
> > > index 93849c03dd..624be3b271 100644
> > > --- a/src/gallium/drivers/virgl/virgl_hw.h
> > > +++ b/src/gallium/drivers/virgl/virgl_hw.h
> > > @@ -286,6 +286,7 @@ struct virgl_caps_v2 {
> > >          int32_t max_texture_gather_offset;
> > >          uint32_t texture_buffer_offset_alignment;
> > >          uint32_t uniform_buffer_offset_alignment;
> > > +        uint32_t shader_buffer_offset_alignment;
> > >  };
> > 
> > This conflicts with
> > 
> >    https://patchwork.freedesktop.org/patch/216081/
> > 
> > (Dave, you wrote there that this landed,  but I don't see it on the
> > mesa side).
> > 
> > In virglrenderer the order is
> >     ...
> >     uint32_t uniform_buffer_offset_alignment;
> >     uint32_t tgsi_invariant;
> >     uint32_t shader_buffer_offset_alignment;
> > 
> > Best,
> > Gert
> > 
> > > 
> > >  union virgl_caps {
> > > diff --git a/src/gallium/drivers/virgl/virgl_screen.c
> > > b/src/gallium/drivers/virgl/virgl_screen.c
> > > index 02613f1866..f183752d63 100644
> > > --- a/src/gallium/drivers/virgl/virgl_screen.c
> > > +++ b/src/gallium/drivers/virgl/virgl_screen.c
> > > @@ -198,6 +198,8 @@ virgl_get_param(struct pipe_screen *screen,
> > > enum
> > > pipe_cap param)
> > >        return vscreen->caps.caps.v1.bset.has_sample_shading;
> > >     case PIPE_CAP_CULL_DISTANCE:
> > >        return vscreen->caps.caps.v1.bset.has_cull;
> > > +   case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> > > +      return vscreen-
> > > >caps.caps.v2.shader_buffer_offset_alignment;
> > >     case PIPE_CAP_TEXTURE_GATHER_SM5:
> > >     case PIPE_CAP_BUFFER_MAP_PERSISTENT_COHERENT:
> > >     case PIPE_CAP_FAKE_SW_MSAA:
> > > @@ -227,7 +229,6 @@ virgl_get_param(struct pipe_screen *screen,
> > > enum
> > > pipe_cap param)
> > >     case PIPE_CAP_TGSI_PACK_HALF_FLOAT:
> > >     case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
> > >     case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
> > > -   case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
> > >     case PIPE_CAP_INVALIDATE_BUFFER:
> > >     case PIPE_CAP_GENERATE_MIPMAP:
> > >     case PIPE_CAP_SURFACE_REINTERPRET_BLOCKS:
> > > diff --git a/src/gallium/drivers/virgl/virgl_winsys.h
> > > b/src/gallium/drivers/virgl/virgl_winsys.h
> > > index 99e98ad9c9..c36b957aba 100644
> > > --- a/src/gallium/drivers/virgl/virgl_winsys.h
> > > +++ b/src/gallium/drivers/virgl/virgl_winsys.h
> > > @@ -134,5 +134,6 @@ static inline void
> > > virgl_ws_fill_new_caps_defaults(struct virgl_drm_caps *caps)
> > >     caps->caps.v2.max_texture_gather_offset = 7;
> > >     caps->caps.v2.texture_buffer_offset_alignment = 32;
> > >     caps->caps.v2.uniform_buffer_offset_alignment = 256;
> > > +   caps->caps.v2.shader_buffer_offset_alignment = 32;
> > >  }
> > >  #endif