Re: [Qemu-devel] [PATCH v6 08/11] contrib: add vhost-user-gpu

[Marc-André Lureau]

Hi

On Fri, Apr 26, 2019 at 10:03 AM Gerd Hoffmann <address@hidden> wrote:
>
>   Hi,
>
> > +#ifdef CONFIG_LIBDRM_INTEL
> > +static bool
> > +intel_alloc_bo(struct drm_buffer *buf)
> > +{
> > +    uint32_t tiling = I915_TILING_NONE;
> > +
> > +    buf->intel_bo = drm_intel_bo_alloc_tiled(buf->dev->bufmgr, 
> > "vhost-user-gpu",
> > +                                             buf->width, buf->height,
> > +                                             (buf->bpp / 8), &tiling,
> > +                                             &buf->stride, 0);
>
> Why do you go for intel specific code here?
>

No idea, most likely some functions were lacking when I looked at it.
GBM is fine now.

> Can't you do the same with gbm_bo_create() or
> gbm_bo_create_with_modifiers() ?

It's not clear to me how to properly use
gbm_bo_create_with_modifiers(). I went with gbm_bo_create() for now,
it "works for me".

>
> > +static bool
> > +intel_map_bo(struct drm_buffer *buf)
> > +{
> > +    if (drm_intel_gem_bo_map_gtt(buf->intel_bo) != 0) {
> > +        return false;
> > +    }
>
> gbm_bo_map()
>
> > +static bool
> > +intel_export_bo_to_prime(struct drm_buffer *buffer, int *fd)
> > +{
> > +    if (drm_intel_bo_gem_export_to_prime(buffer->intel_bo, fd) < 0) {
> > +        return false;
> > +    }
>
> gbm_bo_get_fd()
>
> > +get_pixman_format(uint32_t virtio_gpu_format)
> > +{
> > +    switch (virtio_gpu_format) {
> > +#ifdef HOST_WORDS_BIGENDIAN
> > +    case VIRTIO_GPU_FORMAT_B8G8R8X8_UNORM:
> > +        return PIXMAN_b8g8r8x8;
>
> Move that to a header for sharing?  Simliar to the bswap helpers?

ack

>
> > +static void
> > +virgl_cmd_create_resource_3d(VuGpu *g,
> > +                             struct virtio_gpu_ctrl_command *cmd)
> > +{
> > +    struct virtio_gpu_resource_create_3d c3d;
> > +    struct virgl_renderer_resource_create_args args;
> > +
> > +    VUGPU_FILL_CMD(c3d);
> > +
> > +    args.handle = c3d.resource_id;
> > +    args.target = c3d.target;
> > +    args.format = c3d.format;
> > +    args.bind = c3d.bind;
> > +    args.width = c3d.width;
> > +    args.height = c3d.height;
> > +    args.depth = c3d.depth;
> > +    args.array_size = c3d.array_size;
> > +    args.last_level = c3d.last_level;
> > +    args.nr_samples = c3d.nr_samples;
> > +    args.flags = c3d.flags;
> > +    virgl_renderer_resource_create(&args, NULL, 0);
> > +}
>
> The virtio_gpu_resource_create_3d -> virgl_renderer_resource_create_args
> mapping looks like an opportunity for code sharing too.
>
> Hmm, but virgl_renderer_resource_create seems to be the only call with
> an args struct, so maybe not worth the effort for this single case.

yeah, I will not bother for now

thanks!

-- 
Marc-André Lureau