[Intel-gfx] [PATCH v5 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf

[Kirti Wankhede]

On 5/23/2017 7:39 PM, Gerd Hoffmann wrote:
>   Hi,
> 
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index ae46105..285dc16 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -502,10 +502,58 @@ struct vfio_pci_hot_reset {
>>  
>>  #define VFIO_DEVICE_PCI_HOT_RESET	_IO(VFIO_TYPE, VFIO_BASE +
>> 13)
>>  
>> +/**
>> + * VFIO_DEVICE_GET_FD - _IO(VFIO_TYPE, VFIO_BASE + 14, __u32)
>> + *
>> + * Create a fd for a vfio device based on the input type
>> + * Vendor driver should handle this ioctl to create a fd and manage
>> the
>> + * life cycle of this fd.
>> + *
>> + * Return: a fd if vendor support that type, -errno if not supported
>> + */
>> +
>> +#define VFIO_DEVICE_GET_FD	_IO(VFIO_TYPE, VFIO_BASE + 14)
>> +
>> +#define VFIO_DEVICE_DMABUF_MGR_FD	0 /* Supported fd types */
>> +
>> +/*
>> + * VFIO_DEVICE_QUERY_PLANE - _IO(VFIO_TYPE, VFIO_BASE + 15, struct
>> plane_info)
>> + * Query plane information for a plane
>> + */
>> +struct plane_info {
> 
> That is a pretty generic name.  vfio_vgpu_plane_info?  Or
> vfio_dmabuf_plane_info?
> 

Agree with Gerd, another suggestion vfio_vgpu_surface_info since all
solutions might not always use dmabuf.
Another way to provide surface is by adding a VGA region for vGPU device
using region capability which would be mmaped by QEMU and then QEMU can
directly use that region to get surface. This structure could be made
generic such that user can either use it using dmabuf or a separate
region.

I also back Gerd's comment on 4/5 patch, change in
include/uapi/linux/vfio.h should be a seperate patch.

Thanks,
Kirti

>> +	__u32 plane_id;
>> +	__u32 drm_format;
>> +	__u32 width;
>> +	__u32 height;
>> +	__u32 stride;
>> +	__u32 start;
>> +	__u32 x_pos;
>> +	__u32 y_pos;
>> +	__u32 size;
>> +	__u64 drm_format_mod;
>> +};
>> +
>> +#define VFIO_PRIMARY_PLANE		1
>> +#define VFIO_CURSOR_PLANE		2
> 
> I think we should use "enum drm_plane_type" values instead of creating
> something new.
> 
> cheers,
>   Gerd
>