Re: [PATCH v16 5/6] vfio: ABI for mdev display dma-buf operation

From: Gerd Hoffmann <kraxel@redhat.com>
To: "Zhang, Tina" <tina.zhang@intel.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"chris@chris-wilson.co.uk" <chris@chris-wilson.co.uk>,
	"joonas.lahtinen@linux.intel.com"
	<joonas.lahtinen@linux.intel.com>,
	"zhenyuw@linux.intel.com" <zhenyuw@linux.intel.com>,
	"Lv, Zhiyuan" <zhiyuan.lv@intel.com>,
	"Wang, Zhi A" <zhi.a.wang@intel.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"daniel@ffwll.ch" <daniel@ffwll.ch>,
	"kwankhede@nvidia.com" <kwankhede@nvidia.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"intel-gvt-dev@lists.freedesktop.org"
	<intel-gvt-dev@lists.freedesktop.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v16 5/6] vfio: ABI for mdev display dma-buf operation
Date: Tue, 07 Nov 2017 09:03:21 +0100	[thread overview]
Message-ID: <1510041801.24424.5.camel@redhat.com> (raw)
In-Reply-To: <237F54289DF84E4997F34151298ABEBC7C626C77@SHSMSX101.ccr.corp.intel.com>

  Hi,

> > Add a head field here?  People asked @ kvm forum about multihead
> > support.
> > Even if the initial driver version doesn't support it we could add
> > a field so it
> > becomes easier to add it at some point in the future.
> > 
> > Probing for available heads could be done with the PROBE flag, i.e.
> > flags = PROBE | DMABUF, plane_type = PRIMARY, head = 0, 1, ...
> 
> Does the multihead mean multiple display monitor? So each head can
> have its own one primary plane, one cursor plane and maybe some
> sprite planes if supported.

Yes and yes.

> And the flow could be like this:
> 1) flags = PROBE | DMABUF, then return the number of heads in
> vfio_device_gfx_plane_info.head.

I'd suggest to use { .flags = PROBE | DMABUF, .head = 0 } to check
whenever dmabuf is supported for head 0, then { .flags = PROBE |
DMABUF, .head = 1 } to check for head 1 support, ...

Driver returns success if the head is supported and -EINVAL otherwise,
simliar to the dmabuf vs. region probing.

It is less confusing because .head is always an input field then.

It is also a bit more flexible because different heads can support
different modes (I don't expect drivers will actually use that though).

> 2) flags = DMABUF with plane_type = PRIMARY, head = 0, 1, ..., then
> return the id of the exposed framebuffer of that head.

Yes.

> Another question is if the sprite plane is supported, how we're going
> to identify them, as there could be several sprite planes for one
> display monitor.

Do you mean DRM_PLANE_TYPE_OVERLAY?

> Add another field "num_plane" for it? Then, I guess the flow could be
> like this:
> 1) flags = PROBE | DMABUF, then return the number of heads in
> vfio_device_gfx_plane_info.head.
> 2) flags = PROBE | DMABUF and head = 0, 1, ..., and plane_type =
> PRIMARY/CURSOR/SPRITE, then return the num_plane.

I'd suggest to do it simliar to the head probe outlined above, i.e.
userspace calls { .flags = PROBE | DMABUF, .head = 0, .plane_type =
OVERLAY, plane_num = 0, 1, 2, ... } and the driver returns -EINVAL or 0
depending on whenever it is supported or not.

cheers,
  Gerd

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx