[virglrenderer-devel] multiprocess model and GL

[Chia-I Wu]

On Thu, Feb 6, 2020 at 7:48 PM Gurchetan Singh
<gurchetansingh at chromium.org> wrote:
>
> On Wed, Feb 5, 2020 at 10:35 PM Gerd Hoffmann <kraxel at redhat.com> wrote:
> >
> >   Hi,
> >
> > > > > Multiple sources are perfectly valid.  The virglrenderer protocol is a
> > > > > superset of all capset defined protocols.  capset 1 and capset 2
> > > > > (virgl_hw.h) only apply to virgl (context type 0).
> > > > >
> > > > > capset 3 version 1 can apply to context type 0
> > > > > capset 3 version 2 can apply to context type 1
> >
> > Hmm?  I don't think version is meant to be used like this ...
>
> Yeah, I think we structure this in various ways.
>
> >
> > > The questions in my mind with memory-v4 where:
> > >
> > > 1) VIRTGPU_RESOURCE_FLAG_ALLOC_EXECBUFFER vs.
> > > VIRTGPU_RESOURCE_FLAG_ALLOC_{3D, ARGS} -- since in Virgl, execbuffer
> > > is used for (pipe context) rendering commands commands and not (pipe
> > > screen) allocations, I'd tend to support the latter.  Do you
> > > agree/disagree?
> >
> > Latest memory-v4 has just EXECBUFFER (and DUMB, for virtio only, it's
> > pointless at ioctl level).
> >
> > Didn't we agree on using execbuffer for all allocations, so the VMM just
> > passes on the execbuffer and virglrenderer handles the details?
>
> I agree with passing opaque 3d creation arguments to virglrenderer.  I
> wouldn't call it execbuffer, since currently resource creation and
> execbuffer are different in virglrenderer.
>
> In particular, I think the ioctl should go to
> virgl_renderer_resource_create_blob(...) only and not take a detour at
> virgl_renderer_submit_cmd.  I think Chia's latest version is the same
> in that regard.
Ah, I always advocate execbuffer+resource_create_blob.  Let me comment
out VIRTIO_GPU_RESOURCE_FLAG_ALLOC_VIRGL from my branch to avoid the
confusion.


> For object id based flows, I would favor:
>
> 3d_resource_create_cmd[0] = header
> 3d_resource_create_cmd[1] = object_id.
>
> That way, the fact the resource was created by a prior
> DRM_VIRTGPU_EXECBUFFER ioctl can also be an implementation detail.
> For allocation based flows, I would favor:
>
> 3d_resource_create_cmd[0] = header
> 3d_resource_create_cmd[1] = gltarget
> ..
>
> VMM/kernel wouldn't know, such it's a bit like execbuffer.  Here's the
> latest version:
>
> https://gitlab.freedesktop.org/virgl/drm-misc-next/commit/5bf2a0f9938cbb8b616c224bda6bb4c1bfa02900
>
> >
> > > 2) SHARED_ALLOW vs SHARED_REQUIRE --  SHARED_REQUIRE is the way to go.
> > > Maybe just HOSTVMM_3D since VMM creates the dma-buf?
> >
> > It's named just SHARED in latest v4, but yes the plan is that the host
> > can't choose but must use a shared mapping.
> >
> > > 3) {GET_HOST_BACKING, RELEASE_HOST_BACKING} vs. {ATTACH_HOST_BACKING,
> > > DETACH_HOST_BACKING} vs. {RESOURCE_MAP, RESOURCE_UNMAP} -- something
> > > like the first two seem nice, since they mirror what we have for guest
> > > memory and we want to do want to batch hypercalls.  Thoughts?
> >
> > Well, the plan is to *not* use ATTACH_BACKING for blob resources.
> > What is bad with MAP / UNMAP?
>
> No strong opinions on the subject.  MAP/UNMAP should be fine.
>
> >
> > > 2) How to define protocols -- I think the answer here is the capabilities set.
> >
> > Agree.
> >
> > > > This will be the core resource creation command going forward. It can
> > > > create resources from context-local objects.  Once virgl_protocol.h is
> > > > extended to support local allocations, it can be used with
> > > > virgl_protocol.h to replace the v1 resource creation command.
> > >
> > > As someone (the only one?) who's looked into the virglrenderer
> > > implementation, I think it's easier to extend the current model rather
> > > than requiring object IDs for Virgl.  It's just the way the code is
> > > structured, and it's structured that way on based the Gallium model.
> >
> > I don't think you have to put everything upside down just to use object
> > ids as temporary cookie in virglrenderer.  Just use u64 for the id, then
> > you can use ...
> >
> >     vrend_resource->id = (context_id << 32) | object_id
> >
> > ... when creating the object via execbuffer.  When creating the resource
> > you can lookup the object using context_id+object_id and replace the id
> > with the (global) resource_id.
>
> With the flow described above, we don't have to do this trick.
>
> >
> > cheers,
> >   Gerd
> >