RE: [PATCH v10] vfio: ABI for mdev display dma-buf operation

From: Zhang, Tina
Date: Tue Jul 11 2017 - 22:43:56 EST




> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@xxxxxxxxxx]
> Sent: Tuesday, July 11, 2017 2:08 PM
> To: Zhang, Tina <tina.zhang@xxxxxxxxx>; alex.williamson@xxxxxxxxxx;
> chris@xxxxxxxxxxxxxxxxxx; zhenyuw@xxxxxxxxxxxxxxx; Lv, Zhiyuan
> <zhiyuan.lv@xxxxxxxxx>; Wang, Zhi A <zhi.a.wang@xxxxxxxxx>; Tian, Kevin
> <kevin.tian@xxxxxxxxx>; daniel@xxxxxxxx; kwankhede@xxxxxxxxxx
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v10] vfio: ABI for mdev display dma-buf operation
>
> > +/**
> > + * VFIO_DEVICE_QUERY_GFX_PLANE - _IOW(VFIO_TYPE, VFIO_BASE + 14,
> > + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂstruct vfio_device_query_gfx_plane)
> > + * Return: 0 on success, -errno on failure.
> > + */
> > +
> > +struct vfio_device_gfx_plane_info {
> > + __u64 start;
> > + __u64 drm_format_mod;
> > + __u32 drm_format;
> > + __u32 width;
> > + __u32 height;
> > + __u32 stride;
> > + __u32 size;
> > + __u32 x_pos;
> > + __u32 y_pos;
> > +};
>
> Do we want keep that as separate struct? Given we now have only a single
> struct using that as sub-struct it looks pointless, at least from a API point of view.
> Does the driver use the struct internally?
Driver has another struct which is super-set of these fields. Yes, we can move all these fields into struct vfio_device_query_gfx_plane. Also, we can remove some of these fields which may seem useless for user mode.
Thanks.

Tina
>
> > +
> > +struct vfio_device_query_gfx_plane {
> > + __u32 argsz;
> > + __u32 flags;
> > + struct vfio_device_gfx_plane_info plane_info;
> > + __u32 plane_type;
> > + __s32 fd; /* dma-buf fd */
> > + __u32 plane_id;
>
> What is plane_id?
I cannot figure out the mean of plane_id either. If I remember correctly, it was asked by region usage. Of course, if no one needs it, I'd like to remove it.
Thanks.

Tina

>
> cheers,
> Gerd