[virglrenderer-devel] multiprocess model and GL

[Gurchetan Singh]

On Thu, Feb 13, 2020 at 1:40 AM Gerd Hoffmann <kraxel at redhat.com> wrote:
>
>   Hi,
>
> > > > The kernel will tell virglrenderer the resource id.
> > >
> > > Which implies the resource_create_3d isn't an execbuffer but something
> > > else.  It is not self-contained but depends on the given context,
> > > specifically the resid generated by the kernel.  You can't do the same
> > > via SUBMIT_3D.
> >
> > Typically, virgl_renderer_submit_cmd will take in
> > VIRGL_CMD(ALLOCATE_WITH_OBJ_ID, size) since it needs to establish the
> > [obj_id] --> [struct resource] mapping.
> >
> > virgl_renderer_resource_create_blob(resid, *resource_create_3d, ..)
> > can take in both VIRGL_CMD(ALLOCATE_WITH_OBJ_ID, size) and
> > VIRGL_CMD(ALLOCATE, size) [OpenGL/generic allocation contexts]).
> > Internally, they'll go to the same allocation functions.  User-space
> > will typically choose one or the other.
> >
> > Both VIRGL_CMD(ALLOCATE_WITH_OBJ_ID, size) and VIRGL_CMD(ALLOCATE,
> > size) can be defined in the same header and same protocol namespace,
> > if that's desired.  Does that address your concern about having two
> > different protocols?
>
> No.  IMHO every execbuffer command should be submittable via
> virgl_renderer_submit_cmd().  And ALLOCATE isn't, no matter in which
> header file we define it, because processing it needs additional
> information (the resid) which isn't in the execbuffer.

Ah I see, the issue is both VIRGL_CMD(ALLOCATE, size) and
VIRGL_CMD(GET_OBJ_ID, size) are only useful in the context of the
kernel generated resource id.  Agreed there.

I think it makes sense to ditch the detour to
virgl_renderer_submit_cmd() for both allocation based and object id
based flows?

With the following virglrenderer API:

virgl_renderer_resource_create_blob(resid, *void resource_create_3d, ..)

Here's what I'm thinking:

1) One unified protocol per context for both resource creation and
execbuffer.  Same protocol namespace, APIs.  This is different than
the current status quo where resource creation is defined in
virgl_hw.h and execbuffer is defined in virgl_protocol.h.

2) The capabilities define which commands are usable by
virgl_renderer_resource_create_blob(resid, *void resource_create_3d,
..) and  virgl_renderer_submit_cmd(...).  A bitmask should do the
trick.

3) VIRGL_CMD(ALLOCATE_WITH_OBJ_ID) will be usable by both,
VIRGL_CMD(ALLOCATE, size) and VIRGL_CMD(GET_OBJ_ID, size) will only be
usable by virgl_renderer_resource_create_blob(resid, *void
resource_create_3d, ...).

4) All complex 3D operations commands will be reserved for
virgl_renderer_submit_cmd(..).

5) Resource creation/management operations must be small (~ 64 bytes).

6) Only allow DRM_IOCTL_VIRTGPU_RESOURCE_CREATE_BLOB with the
predefined resource creation/management operations.

I think this should satisfy all use cases, allow allocation during
resource creation, avoid protocol collison, make sure
virgl_renderer_submit_cmd(..) is self-contained, not require object
ids for OpenGL/generic allocation contexts.  Style aside, is there any
other concerns about this*?

* for hostmem.  udmabuf TBD



>
> > > IMHO the workflow should be more like this:
> > >
> > >   (1) VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE
> >
> > Since resources will now be per context, doesn't
> > VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB imply
> > VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE for the creation context (if it's
> > successful)?
>
> Yes (for EXECBUFFER allocations, DUMB resources are a different story of
> course).
>
> > Does it make sense to only call
> > VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE on importing into another context?
>
> Yes.
>
> cheers,
>   Gerd
>