Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1666536 > unrolled thread

[PATCH v9 0/7] drm/i915/gvt: Dma-buf support for GVT-g

Back to article view | Back to linux.kernel

Contents

  [PATCH v9 0/7] drm/i915/gvt: Dma-buf support for GVT-g Xiaoguang Chen <xiaoguang.chen@intel.com> - 2017-06-15 10:10 +0200
    [PATCH v9 6/7] drm/i915/gvt: Dmabuf support for GVT-g Xiaoguang Chen <xiaoguang.chen@intel.com> - 2017-06-15 10:10 +0200
    [PATCH v9 4/7] drm/i915/gvt: Frame buffer decoder support for GVT-g Xiaoguang Chen <xiaoguang.chen@intel.com> - 2017-06-15 10:10 +0200
    [PATCH v9 3/7] drm: Extend the drm format Xiaoguang Chen <xiaoguang.chen@intel.com> - 2017-06-15 10:10 +0200
      Re: [Intel-gfx] [PATCH v9 3/7] drm: Extend the drm format Ville Syrjälä <ville.syrjala@linux.intel.com> - 2017-06-15 12:30 +0200
    [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Xiaoguang Chen <xiaoguang.chen@intel.com> - 2017-06-15 10:10 +0200
      Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Kirti Wankhede <kwankhede@nvidia.com> - 2017-06-15 17:00 +0200
        Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-15 18:10 +0200
          Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-15 22:40 +0200
            Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-16 12:30 +0200
              Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-16 15:00 +0200
            Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Kirti Wankhede <kwankhede@nvidia.com> - 2017-06-16 15:40 +0200
              Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-16 18:50 +0200
                Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Kirti Wankhede <kwankhede@nvidia.com> - 2017-06-16 20:30 +0200
                Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-19 08:40 +0200
                  Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-19 17:00 +0200
              Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-19 08:40 +0200
                Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-19 17:00 +0200

#1666536 — [PATCH v9 0/7] drm/i915/gvt: Dma-buf support for GVT-g

v8->v9:
1) refine the dma-buf ioctl definition
2) add a lock to protect the dmabuf list
3) move drm format change to a separate patch
4) codes cleanup

v7->v8:
1) refine framebuffer decoder code
2) fix a bug in decoding primary plane

v6->v7:
1) release dma-buf related allocations in dma-buf's associated release
function.
2) refine ioctl interface for querying plane info or create dma-buf
3) refine framebuffer decoder code
4) the patch series is based on 4.12.0-rc1

v5->v6:
1) align the dma-buf life cycle with the vfio device.
2) add the dma-buf releated operations in a separate patch.
3) i915 releated changes.

v4->v5:
1) fix bug while checking whether the gem obj is gvt's dma-buf when user
change caching mode or domains. Add a helper function to do it.
2) add definition for the query plane and create dma-buf.

v3->v4:
1) fix bug while checking whether the gem obj is gvt's dma-buf when set
caching mode or doamins.

v2->v3:
1) add a field gvt_plane_info in the drm_i915_gem_obj structure to save
the decoded plane information to avoid look up while need the plane info.
2) declare a new flag I915_GEM_OBJECT_IS_GVT_DMABUF in drm_i915_gem_object
to represent the gem obj for gvt's dma-buf. The tiling mode, caching mode
and domains can not be changed for this kind of gem object.
3) change dma-buf related information to be more generic. So other vendor
can use the same interface.

v1->v2:
1) create a management fd for dma-buf operations.
2) alloc gem object's backing storage in gem obj's get_pages() callback.

This patch set adds the dma-buf support for intel GVT-g.
dma-buf is a uniform mechanism to share DMA buffers across different
devices and sub-systems.
dma-buf for intel GVT-g is mainly used to share the vgpu's framebuffer
to other users or sub-systems so they can use the dma-buf to show the
desktop of a vm which uses intel vgpu.

The main idea is we create a gem object and set vgpu's framebuffer as
the backing storage of this gem object. And associate this gem obj
to a dma-buf object then export this dma-buf at the meantime
generate a file descriptor for this dma-buf. Finally deliver this file
descriptor to user space. And user can use this dma-buf fd to do render
or other operations.
User need to create a fd(for intel GVT-g dma-buf support it is a:dma-buf
management fd) then user can use this fd to query the plane information
or create a dma-buf. The life cycle of this fd is managed by GVT-g user
do not need to care about that.

We have an example program on how to use the dma-buf. You can download
the program to have a try. Good luck :)
git repo: https://github.com/01org/igvtg-qemu branch:kvmgt_dmabuf_example

Xiaoguang Chen (7):
  drm/i915/gvt: Extend the GVT-g architecture
  drm/i915/gvt: OpRegion support for GVT-g
  drm: Extend the drm format
  drm/i915/gvt: Frame buffer decoder support for GVT-g
  vfio: Define vfio based dma-buf operations
  drm/i915/gvt: Dmabuf support for GVT-g
  drm/i915/gvt: Adding user interface for dma-buf

 drivers/gpu/drm/i915/gvt/Makefile      |   3 +-
 drivers/gpu/drm/i915/gvt/display.c     |   2 +-
 drivers/gpu/drm/i915/gvt/display.h     |   2 +
 drivers/gpu/drm/i915/gvt/dmabuf.c      | 307 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/dmabuf.h      |  42 ++++
 drivers/gpu/drm/i915/gvt/fb_decoder.c  | 425 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/fb_decoder.h  | 171 +++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.c         |   3 +
 drivers/gpu/drm/i915/gvt/gvt.h         |   8 +
 drivers/gpu/drm/i915/gvt/hypercall.h   |   4 +
 drivers/gpu/drm/i915/gvt/kvmgt.c       | 245 ++++++++++++++++++-
 drivers/gpu/drm/i915/gvt/mpt.h         |  45 ++++
 drivers/gpu/drm/i915/gvt/opregion.c    |  26 +-
 drivers/gpu/drm/i915/gvt/vgpu.c        |   6 +
 drivers/gpu/drm/i915/i915_gem.c        |  26 +-
 drivers/gpu/drm/i915/i915_gem_object.h |   9 +
 drivers/gpu/drm/i915/i915_gem_tiling.c |   5 +
 include/uapi/drm/drm_fourcc.h          |   4 +
 include/uapi/linux/vfio.h              |  57 +++++
 19 files changed, 1378 insertions(+), 12 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.c
 create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.h
 create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.c
 create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.h

-- 
2.7.4

[toc] | [next] | [standalone]

#1666537 — [PATCH v9 6/7] drm/i915/gvt: Dmabuf support for GVT-g

dmabuf for GVT-g can be exported to users who can use the dmabuf to show
the desktop of vm which use intel vgpu.

Currently we provide query and create new dmabuf operations.

Users of dmabuf can cache some created dmabufs and related information
such as the framebuffer's address, size, tiling mode, width, height etc.
When refresh the screen first query the currnet vgpu's frambuffer and
compare with the cached ones(address, size, tiling, width, height etc)
if found one then reuse the found dmabuf to gain performance improvment.

If there is no dmabuf created yet or not found in the cached dmabufs then
need to create a new dmabuf. To create a dmabuf first a gem object will
be created and the backing storage of this gem object is the vgpu's
framebuffer(primary/cursor).
Set caching mode, change tiling mode and set domains of this gem object
is not supported.
Then associate this gem object to a dmabuf and export this dmabuf.
A file descriptor will be generated for this dmabuf and this file
descriptor can be sent to user space to do display.

Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
Tested-by: Kechen Lu <kechen.lu@intel.com>
---
 drivers/gpu/drm/i915/gvt/Makefile      |   2 +-
 drivers/gpu/drm/i915/gvt/dmabuf.c      | 264 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/dmabuf.h      |  37 +++++
 drivers/gpu/drm/i915/gvt/gvt.h         |   1 +
 drivers/gpu/drm/i915/i915_gem.c        |  26 +++-
 drivers/gpu/drm/i915/i915_gem_object.h |   9 ++
 drivers/gpu/drm/i915/i915_gem_tiling.c |   5 +
 7 files changed, 342 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.c
 create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.h

diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
index 192ca26..e480f7d 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -2,7 +2,7 @@ GVT_DIR := gvt
 GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
 	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
 	execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \
-	fb_decoder.o
+	fb_decoder.o dmabuf.o
 
 ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR) -Wall
 i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c
new file mode 100644
index 0000000..6ef4f60
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
@@ -0,0 +1,264 @@
+/*
+ * Copyright 2017 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Zhiyuan Lv <zhiyuan.lv@intel.com>
+ *
+ * Contributors:
+ *    Xiaoguang Chen <xiaoguang.chen@intel.com>
+ */
+
+#include <linux/dma-buf.h>
+#include <drm/drmP.h>
+#include <linux/vfio.h>
+
+#include "i915_drv.h"
+#include "gvt.h"
+
+#define GEN8_DECODE_PTE(pte) (pte & GENMASK_ULL(63, 12))
+
+static struct sg_table *intel_vgpu_gem_get_pages(
+		struct drm_i915_gem_object *obj)
+{
+	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+	struct sg_table *st;
+	struct scatterlist *sg;
+	int i, ret;
+	gen8_pte_t __iomem *gtt_entries;
+	struct intel_vgpu_fb_info *fb_info;
+
+	fb_info = (struct intel_vgpu_fb_info *)obj->gvt_info;
+	if (WARN_ON(!fb_info))
+		return ERR_PTR(-ENODEV);
+
+	st = kmalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return ERR_PTR(-ENOMEM);
+
+	ret = sg_alloc_table(st, fb_info->fb_size, GFP_KERNEL);
+	if (ret) {
+		kfree(st);
+		return ERR_PTR(ret);
+	}
+	gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
+		(fb_info->fb_addr >> PAGE_SHIFT);
+	for_each_sg(st->sgl, sg, fb_info->fb_size, i) {
+		sg->offset = 0;
+		sg->length = PAGE_SIZE;
+		sg_dma_address(sg) =
+			GEN8_DECODE_PTE(readq(&gtt_entries[i]));
+		sg_dma_len(sg) = PAGE_SIZE;
+	}
+
+	return st;
+}
+
+static void intel_vgpu_gem_put_pages(struct drm_i915_gem_object *obj,
+		struct sg_table *pages)
+{
+	sg_free_table(pages);
+	kfree(pages);
+}
+
+static void intel_vgpu_gem_release(struct drm_i915_gem_object *obj)
+{
+	kfree(obj->gvt_info);
+}
+
+static const struct drm_i915_gem_object_ops intel_vgpu_gem_ops = {
+	.flags = I915_GEM_OBJECT_IS_PROXY,
+	.get_pages = intel_vgpu_gem_get_pages,
+	.put_pages = intel_vgpu_gem_put_pages,
+	.release = intel_vgpu_gem_release,
+};
+
+static struct drm_i915_gem_object *intel_vgpu_create_gem(struct drm_device *dev,
+		struct vfio_dmabuf_mgr_plane_info *info)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_gem_object *obj;
+
+	obj = i915_gem_object_alloc(dev_priv);
+	if (obj == NULL)
+		return NULL;
+
+	drm_gem_private_object_init(dev, &obj->base,
+		info->size << PAGE_SHIFT);
+	i915_gem_object_init(obj, &intel_vgpu_gem_ops);
+
+	obj->base.read_domains = I915_GEM_DOMAIN_GTT;
+	obj->base.write_domain = 0;
+	if (IS_SKYLAKE(dev_priv)) {
+		unsigned int tiling_mode = 0;
+		unsigned int stride = 0;
+
+		switch (info->drm_format_mod << 10) {
+		case PLANE_CTL_TILED_LINEAR:
+			tiling_mode = I915_TILING_NONE;
+			break;
+		case PLANE_CTL_TILED_X:
+			tiling_mode = I915_TILING_X;
+			stride = info->stride;
+			break;
+		case PLANE_CTL_TILED_Y:
+			tiling_mode = I915_TILING_Y;
+			stride = info->stride;
+			break;
+		default:
+			gvt_dbg_core("not supported tiling mode\n");
+		}
+		obj->tiling_and_stride = tiling_mode | stride;
+	} else {
+		obj->tiling_and_stride = info->drm_format_mod ?
+					I915_TILING_X : 0;
+	}
+
+	return obj;
+}
+
+static int intel_vgpu_get_plane_info(struct drm_device *dev,
+		struct intel_vgpu *vgpu,
+		struct vfio_dmabuf_mgr_plane_info *info,
+		int plane_id)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_vgpu_primary_plane_format p;
+	struct intel_vgpu_cursor_plane_format c;
+	int ret;
+
+	if (plane_id == DRM_PLANE_TYPE_PRIMARY) {
+		ret = intel_vgpu_decode_primary_plane(vgpu, &p);
+		if (ret)
+			return ret;
+		info->start = p.base;
+		info->width = p.width;
+		info->height = p.height;
+		info->stride = p.stride;
+		info->drm_format = p.drm_format;
+		info->drm_format_mod = p.tiled;
+		info->size = (((p.stride * p.height * p.bpp) / 8) +
+				(PAGE_SIZE - 1)) >> PAGE_SHIFT;
+	} else if (plane_id == DRM_PLANE_TYPE_CURSOR) {
+		ret = intel_vgpu_decode_cursor_plane(vgpu, &c);
+		if (ret)
+			return ret;
+		info->start = c.base;
+		info->width = c.width;
+		info->height = c.height;
+		info->stride = c.width * (c.bpp / 8);
+		info->drm_format = c.drm_format;
+		info->drm_format_mod = 0;
+		info->x_pos = c.x_pos;
+		info->y_pos = c.y_pos;
+		info->size = (((info->stride * c.height * c.bpp) / 8)
+				+ (PAGE_SIZE - 1)) >> PAGE_SHIFT;
+	} else {
+		gvt_vgpu_err("invalid plane id:%d\n", plane_id);
+		return -EINVAL;
+	}
+
+	if (info->size == 0) {
+		gvt_vgpu_err("fb size is zero\n");
+		return -EINVAL;
+	}
+
+	if (info->start & (PAGE_SIZE - 1)) {
+		gvt_vgpu_err("Not aligned fb address:0x%llx\n", info->start);
+		return -EFAULT;
+	}
+	if (((info->start >> PAGE_SHIFT) + info->size) >
+		ggtt_total_entries(&dev_priv->ggtt)) {
+		gvt_vgpu_err("Invalid GTT offset or size\n");
+		return -EFAULT;
+	}
+
+	if (!intel_gvt_ggtt_validate_range(vgpu, info->start, info->size)) {
+		gvt_vgpu_err("invalid gma addr\n");
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
+int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args)
+{
+	struct drm_device *dev = &vgpu->gvt->dev_priv->drm;
+	struct vfio_dmabuf_mgr_query_plane *plane = args;
+
+	return intel_vgpu_get_plane_info(dev, vgpu, &plane->plane_info,
+					plane->plane_id);
+}
+
+int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args)
+{
+	struct dma_buf *dmabuf;
+	struct drm_i915_gem_object *obj;
+	struct drm_device *dev = &vgpu->gvt->dev_priv->drm;
+	struct vfio_dmabuf_mgr_create_dmabuf *gvt_dmabuf = args;
+	struct intel_vgpu_fb_info *fb_info;
+	int ret;
+
+	ret = intel_vgpu_get_plane_info(dev, vgpu, &gvt_dmabuf->plane_info,
+					gvt_dmabuf->plane_id);
+	if (ret != 0)
+		return ret;
+
+	obj = intel_vgpu_create_gem(dev, &gvt_dmabuf->plane_info);
+	if (obj == NULL) {
+		gvt_vgpu_err("create gvt gem obj failed:%d\n", vgpu->id);
+		return -ENOMEM;
+	}
+	fb_info = kmalloc(sizeof(*fb_info), GFP_KERNEL);
+	if (!fb_info) {
+		gvt_vgpu_err("allocate intel vgpu fb info failed\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+	fb_info->fb_addr = gvt_dmabuf->plane_info.start;
+	fb_info->fb_size = gvt_dmabuf->plane_info.size;
+	fb_info->vgpu = vgpu;
+	obj->gvt_info = fb_info;
+
+	dmabuf = i915_gem_prime_export(dev, &obj->base, DRM_CLOEXEC | DRM_RDWR);
+
+	if (IS_ERR(dmabuf)) {
+		gvt_vgpu_err("export dma-buf failed\n");
+		ret = PTR_ERR(dmabuf);
+		goto out_free;
+	}
+
+	ret = dma_buf_fd(dmabuf, DRM_CLOEXEC | DRM_RDWR);
+	if (ret < 0) {
+		gvt_vgpu_err("create dma-buf fd failed ret:%d\n", ret);
+		goto out_free;
+	}
+
+	gvt_dmabuf->fd = ret;
+
+	return 0;
+out_free:
+	kfree(fb_info);
+out:
+	i915_gem_object_put(obj);
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.h b/drivers/gpu/drm/i915/gvt/dmabuf.h
new file mode 100644
index 0000000..8be9979
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.h
@@ -0,0 +1,37 @@
+/*
+ * Copyright(c) 2017 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ */
+
+#ifndef _GVT_DMABUF_H_
+#define _GVT_DMABUF_H_
+
+struct intel_vgpu_fb_info {
+	struct intel_vgpu *vgpu;
+	uint32_t fb_addr;
+	uint32_t fb_size;
+};
+
+int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args);
+int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index c42266c..763a8c5 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -47,6 +47,7 @@
 #include "render.h"
 #include "cmd_parser.h"
 #include "fb_decoder.h"
+#include "dmabuf.h"
 
 #define GVT_MAX_VGPU 8
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0c1cbe9..f91342b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1609,6 +1609,12 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
+	/* proxy gem object does not support setting domain */
+	if (i915_gem_object_is_proxy(obj)) {
+		err = -EPERM;
+		goto out;
+	}
+
 	/* Try to flush the object off the GPU without holding the lock.
 	 * We will repeat the flush holding the lock in the normal manner
 	 * to catch cases where we are gazumped.
@@ -1677,6 +1683,12 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
+	/* proxy gem obj does not support this operation */
+	if (i915_gem_object_is_proxy(obj)) {
+		i915_gem_object_put(obj);
+		return -EPERM;
+	}
+
 	/* Pinned buffers may be scanout, so flush the cache */
 	i915_gem_object_flush_if_display(obj);
 	i915_gem_object_put(obj);
@@ -1727,7 +1739,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 	 */
 	if (!obj->base.filp) {
 		i915_gem_object_put(obj);
-		return -EINVAL;
+		return -EPERM;
 	}
 
 	addr = vm_mmap(obj->base.filp, 0, args->size,
@@ -3717,6 +3729,12 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
+	/* proxy gem obj does not support setting caching mode */
+	if (i915_gem_object_is_proxy(obj)) {
+		ret = -EPERM;
+		goto out;
+	}
+
 	if (obj->cache_level == level)
 		goto out;
 
@@ -4168,6 +4186,12 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
+	/* proxy gem obj does not support changing backding storage */
+	if (i915_gem_object_is_proxy(obj)) {
+		err = -EPERM;
+		goto out;
+	}
+
 	err = mutex_lock_interruptible(&obj->mm.lock);
 	if (err)
 		goto out;
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 174cf92..be18a43 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -39,6 +39,7 @@ struct drm_i915_gem_object_ops {
 	unsigned int flags;
 #define I915_GEM_OBJECT_HAS_STRUCT_PAGE 0x1
 #define I915_GEM_OBJECT_IS_SHRINKABLE   0x2
+#define I915_GEM_OBJECT_IS_PROXY	0x4
 
 	/* Interface between the GEM object and its backing storage.
 	 * get_pages() is called once prior to the use of the associated set
@@ -184,6 +185,8 @@ struct drm_i915_gem_object {
 		} userptr;
 
 		unsigned long scratch;
+
+		void *gvt_info;
 	};
 
 	/** for phys allocated objects */
@@ -286,6 +289,12 @@ i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj)
 }
 
 static inline bool
+i915_gem_object_is_proxy(const struct drm_i915_gem_object *obj)
+{
+	return obj->ops->flags & I915_GEM_OBJECT_IS_PROXY;
+}
+
+static inline bool
 i915_gem_object_is_active(const struct drm_i915_gem_object *obj)
 {
 	return obj->active_count;
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index a0d6d43..2ecf5d7 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -345,6 +345,11 @@ i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
+	if (i915_gem_object_is_proxy(obj)) {
+		err = -EPERM;
+		goto err;
+	}
+
 	if (!i915_tiling_ok(obj, args->tiling_mode, args->stride)) {
 		err = -EINVAL;
 		goto err;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1666538 — [PATCH v9 4/7] drm/i915/gvt: Frame buffer decoder support for GVT-g

decode frambuffer attributes of primary, cursor and sprite plane

Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
---
 drivers/gpu/drm/i915/gvt/Makefile     |   3 +-
 drivers/gpu/drm/i915/gvt/display.c    |   2 +-
 drivers/gpu/drm/i915/gvt/display.h    |   2 +
 drivers/gpu/drm/i915/gvt/fb_decoder.c | 425 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/fb_decoder.h | 171 ++++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.h        |   1 +
 6 files changed, 602 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.c
 create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.h

diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
index b123c20..192ca26 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -1,7 +1,8 @@
 GVT_DIR := gvt
 GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
 	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
-	execlist.o scheduler.o sched_policy.o render.o cmd_parser.o
+	execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \
+	fb_decoder.o
 
 ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR) -Wall
 i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
index e0261fc..f5f63c5 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -67,7 +67,7 @@ static int edp_pipe_is_enabled(struct intel_vgpu *vgpu)
 	return 1;
 }
 
-static int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe)
+int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe)
 {
 	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
 
diff --git a/drivers/gpu/drm/i915/gvt/display.h b/drivers/gpu/drm/i915/gvt/display.h
index d73de22..b46b868 100644
--- a/drivers/gpu/drm/i915/gvt/display.h
+++ b/drivers/gpu/drm/i915/gvt/display.h
@@ -179,4 +179,6 @@ int intel_vgpu_init_display(struct intel_vgpu *vgpu, u64 resolution);
 void intel_vgpu_reset_display(struct intel_vgpu *vgpu);
 void intel_vgpu_clean_display(struct intel_vgpu *vgpu);
 
+int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe);
+
 #endif
diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c b/drivers/gpu/drm/i915/gvt/fb_decoder.c
new file mode 100644
index 0000000..a4614b5
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
@@ -0,0 +1,425 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ *    Kevin Tian <kevin.tian@intel.com>
+ *
+ * Contributors:
+ *    Bing Niu <bing.niu@intel.com>
+ *    Xu Han <xu.han@intel.com>
+ *    Ping Gao <ping.a.gao@intel.com>
+ *    Xiaoguang Chen <xiaoguang.chen@intel.com>
+ *    Yang Liu <yang2.liu@intel.com>
+ *
+ */
+
+#include <uapi/drm/drm_fourcc.h>
+#include "i915_drv.h"
+#include "gvt.h"
+
+#define PRIMARY_FORMAT_NUM	16
+struct pixel_format {
+	int	drm_format;	/* Pixel format in DRM definition */
+	int	bpp;		/* Bits per pixel, 0 indicates invalid */
+	char *desc;		/* The description */
+};
+
+/* non-supported format has bpp default to 0 */
+static struct pixel_format bdw_pixel_formats[PRIMARY_FORMAT_NUM] = {
+	[0x2] = {DRM_FORMAT_C8, 8, "8-bit Indexed"},
+	[0x5] = {DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"},
+	[0x6] = {DRM_FORMAT_XRGB8888, 32,
+		"32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"},
+	[0x8] = {DRM_FORMAT_XBGR2101010, 32,
+		"32-bit RGBX (2:10:10:10 MSB-X:B:G:R)"},
+	[0xa] = {DRM_FORMAT_XRGB2101010, 32,
+		"32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
+	[0xc] = {DRM_FORMAT_XRGB161616, 64,
+		"64-bit RGBX Floating Point(16:16:16:16 MSB-X:B:G:R)"},
+	[0xe] = {DRM_FORMAT_XBGR8888, 32,
+		"32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"},
+};
+
+/* non-supported format has bpp default to 0 */
+static struct pixel_format skl_pixel_formats[] = {
+	{DRM_FORMAT_YUYV, 16, "16-bit packed YUYV (8:8:8:8 MSB-V:Y2:U:Y1)"},
+	{DRM_FORMAT_UYVY, 16, "16-bit packed UYVY (8:8:8:8 MSB-Y2:V:Y1:U)"},
+	{DRM_FORMAT_YVYU, 16, "16-bit packed YVYU (8:8:8:8 MSB-U:Y2:V:Y1)"},
+	{DRM_FORMAT_VYUY, 16, "16-bit packed VYUY (8:8:8:8 MSB-Y2:U:Y1:V)"},
+
+	{DRM_FORMAT_C8, 8, "8-bit Indexed"},
+	{DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"},
+	{DRM_FORMAT_ABGR8888, 32, "32-bit RGBA (8:8:8:8 MSB-A:B:G:R)"},
+	{DRM_FORMAT_XBGR8888, 32, "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"},
+
+	{DRM_FORMAT_ARGB8888, 32, "32-bit BGRA (8:8:8:8 MSB-A:R:G:B)"},
+	{DRM_FORMAT_XRGB8888, 32, "32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"},
+	{DRM_FORMAT_XBGR2101010, 32, "32-bit RGBX (2:10:10:10 MSB-X:B:G:R)"},
+	{DRM_FORMAT_XRGB2101010, 32, "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
+
+	{DRM_FORMAT_XRGB161616, 64,
+		"64-bit XRGB (16:16:16:16 MSB-X:R:G:B)"},
+	{DRM_FORMAT_XBGR161616, 64,
+		"64-bit XBGR (16:16:16:16 MSB-X:B:G:R)"},
+
+	/* non-supported format has bpp default to 0 */
+	{0, 0, NULL},
+};
+
+static int skl_format_to_drm(int format, bool rgb_order, bool alpha,
+	int yuv_order)
+{
+	int skl_pixel_formats_index = 14;
+
+	switch (format) {
+	case PLANE_CTL_FORMAT_INDEXED:
+		skl_pixel_formats_index = 4;
+		break;
+	case PLANE_CTL_FORMAT_RGB_565:
+		skl_pixel_formats_index = 5;
+		break;
+	case PLANE_CTL_FORMAT_XRGB_8888:
+		if (rgb_order)
+			skl_pixel_formats_index = alpha ? 6 : 7;
+		else
+			skl_pixel_formats_index = alpha ? 8 : 9;
+		break;
+	case PLANE_CTL_FORMAT_XRGB_2101010:
+		skl_pixel_formats_index = rgb_order ? 10 : 11;
+		break;
+
+	case PLANE_CTL_FORMAT_XRGB_16161616F:
+		skl_pixel_formats_index = rgb_order ? 12 : 13;
+		break;
+
+	case PLANE_CTL_FORMAT_YUV422:
+		skl_pixel_formats_index = yuv_order >> 16;
+		if (skl_pixel_formats_index > 3)
+			return -EINVAL;
+		break;
+
+	default:
+		break;
+	}
+
+	return skl_pixel_formats_index;
+}
+
+static u32 intel_vgpu_get_stride(struct intel_vgpu *vgpu, int pipe,
+	u32 tiled, int stride_mask, int bpp)
+{
+	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+
+	u32 stride_reg = vgpu_vreg(vgpu, DSPSTRIDE(pipe)) & stride_mask;
+	u32 stride = stride_reg;
+
+	if (IS_SKYLAKE(dev_priv)) {
+		switch (tiled) {
+		case PLANE_CTL_TILED_LINEAR:
+			stride = stride_reg * 64;
+			break;
+		case PLANE_CTL_TILED_X:
+			stride = stride_reg * 512;
+			break;
+		case PLANE_CTL_TILED_Y:
+			stride = stride_reg * 128;
+			break;
+		case PLANE_CTL_TILED_YF:
+			if (bpp == 8)
+				stride = stride_reg * 64;
+			else if (bpp == 16 || bpp == 32 || bpp == 64)
+				stride = stride_reg * 128;
+			else
+				gvt_dbg_core("skl: unsupported bpp:%d\n", bpp);
+			break;
+		default:
+			gvt_dbg_core("skl: unsupported tile format:%x\n",
+				tiled);
+		}
+	}
+
+	return stride;
+}
+
+static int get_active_pipe(struct intel_vgpu *vgpu)
+{
+	int i;
+
+	for (i = 0; i < I915_MAX_PIPES; i++)
+		if (pipe_is_enabled(vgpu, i))
+			break;
+
+	return i;
+}
+
+/**
+ * intel_vgpu_decode_primary_plane - Decode primary plane
+ * @vgpu: input vgpu
+ * @plane: primary plane to save decoded info
+ * This function is called for decoding plane
+ *
+ * Returns:
+ * 0 on success, non-zero if failed.
+ */
+int intel_vgpu_decode_primary_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_primary_plane_format *plane)
+{
+	u32 val, fmt;
+	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+	int pipe;
+
+	pipe = get_active_pipe(vgpu);
+	if (pipe >= I915_MAX_PIPES)
+		return -ENODEV;
+
+	val = vgpu_vreg(vgpu, DSPCNTR(pipe));
+	plane->enabled = !!(val & DISPLAY_PLANE_ENABLE);
+	if (!plane->enabled)
+		return -ENODEV;
+
+	if (IS_SKYLAKE(dev_priv)) {
+		plane->tiled = (val & PLANE_CTL_TILED_MASK) >>
+		_PLANE_CTL_TILED_SHIFT;
+		fmt = skl_format_to_drm(
+			val & PLANE_CTL_FORMAT_MASK,
+			val & PLANE_CTL_ORDER_RGBX,
+			val & PLANE_CTL_ALPHA_MASK,
+			val & PLANE_CTL_YUV422_ORDER_MASK);
+		plane->bpp = skl_pixel_formats[fmt].bpp;
+		plane->drm_format = skl_pixel_formats[fmt].drm_format;
+	} else {
+		plane->tiled = !!(val & DISPPLANE_TILED);
+		fmt = (val & DISPPLANE_PIXFORMAT_MASK) >> _PRI_PLANE_FMT_SHIFT;
+		plane->bpp = bdw_pixel_formats[fmt].bpp;
+		plane->drm_format = bdw_pixel_formats[fmt].drm_format;
+	}
+
+	if (!plane->bpp) {
+		gvt_vgpu_err("Non-supported pixel format (0x%x)\n", fmt);
+		return -EINVAL;
+	}
+
+	plane->hw_format = fmt;
+
+	plane->base = vgpu_vreg(vgpu, DSPSURF(pipe)) & GTT_PAGE_MASK;
+
+	plane->stride = intel_vgpu_get_stride(vgpu, pipe, (plane->tiled << 10),
+		(IS_SKYLAKE(dev_priv)) ? (_PRI_PLANE_STRIDE_MASK >> 6) :
+		_PRI_PLANE_STRIDE_MASK, plane->bpp);
+
+	plane->width = (vgpu_vreg(vgpu, PIPESRC(pipe)) & _PIPE_H_SRCSZ_MASK) >>
+		_PIPE_H_SRCSZ_SHIFT;
+	plane->width += 1;
+	plane->height = (vgpu_vreg(vgpu, PIPESRC(pipe)) &
+			_PIPE_V_SRCSZ_MASK) >> _PIPE_V_SRCSZ_SHIFT;
+	plane->height += 1;	/* raw height is one minus the real value */
+
+	val = vgpu_vreg(vgpu, DSPTILEOFF(pipe));
+	plane->x_offset = (val & _PRI_PLANE_X_OFF_MASK) >>
+		_PRI_PLANE_X_OFF_SHIFT;
+	plane->y_offset = (val & _PRI_PLANE_Y_OFF_MASK) >>
+		_PRI_PLANE_Y_OFF_SHIFT;
+
+	return 0;
+}
+
+#define CURSOR_FORMAT_NUM	(1 << 6)
+struct cursor_mode_format {
+	int	drm_format;	/* Pixel format in DRM definition */
+	u8	bpp;		/* Bits per pixel; 0 indicates invalid */
+	u32	width;		/* In pixel */
+	u32	height;		/* In lines */
+	char	*desc;		/* The description */
+};
+
+/* non-supported format has bpp default to 0 */
+static struct cursor_mode_format cursor_pixel_formats[CURSOR_FORMAT_NUM] = {
+	[0x22]  = {DRM_FORMAT_ARGB8888, 32, 128, 128, "128x128 32bpp ARGB"},
+	[0x23]  = {DRM_FORMAT_ARGB8888, 32, 256, 256, "256x256 32bpp ARGB"},
+	[0x27]  = {DRM_FORMAT_ARGB8888, 32, 64, 64, "64x64 32bpp ARGB"},
+	[0x7]  = {DRM_FORMAT_ARGB8888, 32, 64, 64, "64x64 32bpp ARGB"},
+};
+
+/**
+ * intel_vgpu_decode_cursor_plane - Decode sprite plane
+ * @vgpu: input vgpu
+ * @plane: cursor plane to save decoded info
+ * This function is called for decoding plane
+ *
+ * Returns:
+ * 0 on success, non-zero if failed.
+ */
+int intel_vgpu_decode_cursor_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_cursor_plane_format *plane)
+{
+	u32 val, mode;
+	u32 alpha_plane, alpha_force;
+	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+	int pipe;
+
+	pipe = get_active_pipe(vgpu);
+	if (pipe >= I915_MAX_PIPES)
+		return -ENODEV;
+
+	val = vgpu_vreg(vgpu, CURCNTR(pipe));
+	mode = val & CURSOR_MODE;
+	plane->enabled = (mode != CURSOR_MODE_DISABLE);
+	if (!plane->enabled)
+		return -ENODEV;
+
+	if (!cursor_pixel_formats[mode].bpp) {
+		gvt_vgpu_err("Non-supported cursor mode (0x%x)\n", mode);
+		return -EINVAL;
+	}
+	plane->mode = mode;
+	plane->bpp = cursor_pixel_formats[mode].bpp;
+	plane->drm_format = cursor_pixel_formats[mode].drm_format;
+	plane->width = cursor_pixel_formats[mode].width;
+	plane->height = cursor_pixel_formats[mode].height;
+
+	alpha_plane = (val & _CURSOR_ALPHA_PLANE_MASK) >>
+				_CURSOR_ALPHA_PLANE_SHIFT;
+	alpha_force = (val & _CURSOR_ALPHA_FORCE_MASK) >>
+				_CURSOR_ALPHA_FORCE_SHIFT;
+	if (alpha_plane || alpha_force)
+		gvt_dbg_core("alpha_plane=0x%x, alpha_force=0x%x\n",
+			alpha_plane, alpha_force);
+
+	plane->base = vgpu_vreg(vgpu, CURBASE(pipe)) & GTT_PAGE_MASK;
+
+	val = vgpu_vreg(vgpu, CURPOS(pipe));
+	plane->x_pos = (val & _CURSOR_POS_X_MASK) >> _CURSOR_POS_X_SHIFT;
+	plane->x_sign = (val & _CURSOR_SIGN_X_MASK) >> _CURSOR_SIGN_X_SHIFT;
+	plane->y_pos = (val & _CURSOR_POS_Y_MASK) >> _CURSOR_POS_Y_SHIFT;
+	plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >> _CURSOR_SIGN_Y_SHIFT;
+
+	return 0;
+}
+
+#define SPRITE_FORMAT_NUM	(1 << 3)
+
+static struct pixel_format sprite_pixel_formats[SPRITE_FORMAT_NUM] = {
+	[0x0]  = {DRM_FORMAT_YUV422, 16, "YUV 16-bit 4:2:2 packed"},
+	[0x1]  = {DRM_FORMAT_XRGB2101010, 32, "RGB 32-bit 2:10:10:10"},
+	[0x2]  = {DRM_FORMAT_XRGB8888, 32, "RGB 32-bit 8:8:8:8"},
+	[0x3]  = {DRM_FORMAT_XRGB161616, 64,
+		    "RGB 64-bit 16:16:16:16 Floating Point"},
+	[0x4] = {DRM_FORMAT_AYUV, 32,
+		"YUV 32-bit 4:4:4 packed (8:8:8:8 MSB-X:Y:U:V)"},
+};
+
+/**
+ * intel_vgpu_decode_sprite_plane - Decode sprite plane
+ * @vgpu: input vgpu
+ * @plane: sprite plane to save decoded info
+ * This function is called for decoding plane
+ *
+ * Returns:
+ * 0 on success, non-zero if failed.
+ */
+int intel_vgpu_decode_sprite_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_sprite_plane_format *plane)
+{
+	u32 val, fmt;
+	u32 color_order, yuv_order;
+	int drm_format;
+	int pipe;
+
+	pipe = get_active_pipe(vgpu);
+	if (pipe >= I915_MAX_PIPES)
+		return -ENODEV;
+
+	val = vgpu_vreg(vgpu, SPRCTL(pipe));
+	plane->enabled = !!(val & SPRITE_ENABLE);
+	if (!plane->enabled)
+		return -ENODEV;
+
+	plane->tiled = !!(val & SPRITE_TILED);
+	color_order = !!(val & SPRITE_RGB_ORDER_RGBX);
+	yuv_order = (val & SPRITE_YUV_BYTE_ORDER_MASK) >>
+				_SPRITE_YUV_ORDER_SHIFT;
+
+	fmt = (val & SPRITE_PIXFORMAT_MASK) >> _SPRITE_FMT_SHIFT;
+	if (!sprite_pixel_formats[fmt].bpp) {
+		gvt_vgpu_err("Non-supported pixel format (0x%x)\n", fmt);
+		return -EINVAL;
+	}
+	plane->hw_format = fmt;
+	plane->bpp = sprite_pixel_formats[fmt].bpp;
+	drm_format = sprite_pixel_formats[fmt].drm_format;
+
+	/* Order of RGB values in an RGBxxx buffer may be ordered RGB or
+	 * BGR depending on the state of the color_order field
+	 */
+	if (!color_order) {
+		if (drm_format == DRM_FORMAT_XRGB2101010)
+			drm_format = DRM_FORMAT_XBGR2101010;
+		else if (drm_format == DRM_FORMAT_XRGB8888)
+			drm_format = DRM_FORMAT_XBGR8888;
+	}
+
+	if (drm_format == DRM_FORMAT_YUV422) {
+		switch (yuv_order) {
+		case 0:
+			drm_format = DRM_FORMAT_YUYV;
+			break;
+		case 1:
+			drm_format = DRM_FORMAT_UYVY;
+			break;
+		case 2:
+			drm_format = DRM_FORMAT_YVYU;
+			break;
+		case 3:
+			drm_format = DRM_FORMAT_VYUY;
+			break;
+		default:
+			/* yuv_order has only 2 bits */
+			break;
+		}
+	}
+
+	plane->drm_format = drm_format;
+
+	plane->base = vgpu_vreg(vgpu, SPRSURF(pipe)) & GTT_PAGE_MASK;
+	plane->stride = vgpu_vreg(vgpu, SPRSTRIDE(pipe)) &
+				_SPRITE_STRIDE_MASK;
+
+	val = vgpu_vreg(vgpu, SPRSIZE(pipe));
+	plane->height = (val & _SPRITE_SIZE_HEIGHT_MASK) >>
+		_SPRITE_SIZE_HEIGHT_SHIFT;
+	plane->width = (val & _SPRITE_SIZE_WIDTH_MASK) >>
+		_SPRITE_SIZE_WIDTH_SHIFT;
+	plane->height += 1;	/* raw height is one minus the real value */
+	plane->width += 1;	/* raw width is one minus the real value */
+
+	val = vgpu_vreg(vgpu, SPRPOS(pipe));
+	plane->x_pos = (val & _SPRITE_POS_X_MASK) >> _SPRITE_POS_X_SHIFT;
+	plane->y_pos = (val & _SPRITE_POS_Y_MASK) >> _SPRITE_POS_Y_SHIFT;
+
+	val = vgpu_vreg(vgpu, SPROFFSET(pipe));
+	plane->x_offset = (val & _SPRITE_OFFSET_START_X_MASK) >>
+			   _SPRITE_OFFSET_START_X_SHIFT;
+	plane->y_offset = (val & _SPRITE_OFFSET_START_Y_MASK) >>
+			   _SPRITE_OFFSET_START_Y_SHIFT;
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.h b/drivers/gpu/drm/i915/gvt/fb_decoder.h
new file mode 100644
index 0000000..08d4d8a
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.h
@@ -0,0 +1,171 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ *    Kevin Tian <kevin.tian@intel.com>
+ *
+ * Contributors:
+ *    Bing Niu <bing.niu@intel.com>
+ *    Xu Han <xu.han@intel.com>
+ *    Ping Gao <ping.a.gao@intel.com>
+ *    Xiaoguang Chen <xiaoguang.chen@intel.com>
+ *    Yang Liu <yang2.liu@intel.com>
+ *
+ */
+
+#ifndef _GVT_FB_DECODER_H_
+#define _GVT_FB_DECODER_H_
+
+#define _PLANE_CTL_FORMAT_SHIFT		24
+#define _PLANE_CTL_TILED_SHIFT		10
+#define _PIPE_V_SRCSZ_SHIFT		0
+#define _PIPE_V_SRCSZ_MASK		(0xfff << _PIPE_V_SRCSZ_SHIFT)
+#define _PIPE_H_SRCSZ_SHIFT		16
+#define _PIPE_H_SRCSZ_MASK		(0x1fff << _PIPE_H_SRCSZ_SHIFT)
+
+#define _PRI_PLANE_FMT_SHIFT		26
+#define _PRI_PLANE_STRIDE_MASK		(0x3ff << 6)
+#define _PRI_PLANE_X_OFF_SHIFT		0
+#define _PRI_PLANE_X_OFF_MASK		(0x1fff << _PRI_PLANE_X_OFF_SHIFT)
+#define _PRI_PLANE_Y_OFF_SHIFT		16
+#define _PRI_PLANE_Y_OFF_MASK		(0xfff << _PRI_PLANE_Y_OFF_SHIFT)
+
+#define _CURSOR_MODE			0x3f
+#define _CURSOR_ALPHA_FORCE_SHIFT	8
+#define _CURSOR_ALPHA_FORCE_MASK	(0x3 << _CURSOR_ALPHA_FORCE_SHIFT)
+#define _CURSOR_ALPHA_PLANE_SHIFT	10
+#define _CURSOR_ALPHA_PLANE_MASK	(0x3 << _CURSOR_ALPHA_PLANE_SHIFT)
+#define _CURSOR_POS_X_SHIFT		0
+#define _CURSOR_POS_X_MASK		(0x1fff << _CURSOR_POS_X_SHIFT)
+#define _CURSOR_SIGN_X_SHIFT		15
+#define _CURSOR_SIGN_X_MASK		(1 << _CURSOR_SIGN_X_SHIFT)
+#define _CURSOR_POS_Y_SHIFT		16
+#define _CURSOR_POS_Y_MASK		(0xfff << _CURSOR_POS_Y_SHIFT)
+#define _CURSOR_SIGN_Y_SHIFT		31
+#define _CURSOR_SIGN_Y_MASK		(1 << _CURSOR_SIGN_Y_SHIFT)
+
+#define _SPRITE_FMT_SHIFT		25
+#define _SPRITE_COLOR_ORDER_SHIFT	20
+#define _SPRITE_YUV_ORDER_SHIFT		16
+#define _SPRITE_STRIDE_SHIFT		6
+#define _SPRITE_STRIDE_MASK		(0x1ff << _SPRITE_STRIDE_SHIFT)
+#define _SPRITE_SIZE_WIDTH_SHIFT	0
+#define _SPRITE_SIZE_HEIGHT_SHIFT	16
+#define _SPRITE_SIZE_WIDTH_MASK		(0x1fff << _SPRITE_SIZE_WIDTH_SHIFT)
+#define _SPRITE_SIZE_HEIGHT_MASK	(0xfff << _SPRITE_SIZE_HEIGHT_SHIFT)
+#define _SPRITE_POS_X_SHIFT		0
+#define _SPRITE_POS_Y_SHIFT		16
+#define _SPRITE_POS_X_MASK		(0x1fff << _SPRITE_POS_X_SHIFT)
+#define _SPRITE_POS_Y_MASK		(0xfff << _SPRITE_POS_Y_SHIFT)
+#define _SPRITE_OFFSET_START_X_SHIFT	0
+#define _SPRITE_OFFSET_START_Y_SHIFT	16
+#define _SPRITE_OFFSET_START_X_MASK	(0x1fff << _SPRITE_OFFSET_START_X_SHIFT)
+#define _SPRITE_OFFSET_START_Y_MASK	(0xfff << _SPRITE_OFFSET_START_Y_SHIFT)
+
+enum GVT_FB_EVENT {
+	FB_MODE_SET_START = 1,
+	FB_MODE_SET_END,
+	FB_DISPLAY_FLIP,
+};
+
+enum DDI_PORT {
+	DDI_PORT_NONE	= 0,
+	DDI_PORT_B	= 1,
+	DDI_PORT_C	= 2,
+	DDI_PORT_D	= 3,
+	DDI_PORT_E	= 4
+};
+
+struct intel_gvt;
+
+struct gvt_fb_notify_msg {
+	unsigned int vm_id;
+	unsigned int pipe_id; /* id starting from 0 */
+	unsigned int plane_id; /* primary, cursor, or sprite */
+};
+
+/* color space conversion and gamma correction are not included */
+struct intel_vgpu_primary_plane_format {
+	u8	enabled;	/* plane is enabled */
+	u8	tiled;		/* X-tiled */
+	u8	bpp;		/* bits per pixel */
+	u32	hw_format;	/* format field in the PRI_CTL register */
+	u32	drm_format;	/* format in DRM definition */
+	u32	base;		/* framebuffer base in graphics memory */
+	u32	x_offset;	/* in pixels */
+	u32	y_offset;	/* in lines */
+	u32	width;		/* in pixels */
+	u32	height;		/* in lines */
+	u32	stride;		/* in bytes */
+};
+
+struct intel_vgpu_sprite_plane_format {
+	u8	enabled;	/* plane is enabled */
+	u8	tiled;		/* X-tiled */
+	u8	bpp;		/* bits per pixel */
+	u32	hw_format;	/* format field in the SPR_CTL register */
+	u32	drm_format;	/* format in DRM definition */
+	u32	base;		/* sprite base in graphics memory */
+	u32	x_pos;		/* in pixels */
+	u32	y_pos;		/* in lines */
+	u32	x_offset;	/* in pixels */
+	u32	y_offset;	/* in lines */
+	u32	width;		/* in pixels */
+	u32	height;		/* in lines */
+	u32	stride;		/* in bytes */
+};
+
+struct intel_vgpu_cursor_plane_format {
+	u8	enabled;
+	u8	mode;		/* cursor mode select */
+	u8	bpp;		/* bits per pixel */
+	u32	drm_format;	/* format in DRM definition */
+	u32	base;		/* cursor base in graphics memory */
+	u32	x_pos;		/* in pixels */
+	u32	y_pos;		/* in lines */
+	u8	x_sign;		/* X Position Sign */
+	u8	y_sign;		/* Y Position Sign */
+	u32	width;		/* in pixels */
+	u32	height;		/* in lines */
+	u32	x_hot;		/* in pixels */
+	u32	y_hot;		/* in pixels */
+};
+
+struct intel_vgpu_pipe_format {
+	struct intel_vgpu_primary_plane_format	primary;
+	struct intel_vgpu_sprite_plane_format	sprite;
+	struct intel_vgpu_cursor_plane_format	cursor;
+	enum DDI_PORT ddi_port;  /* the DDI port that pipe is connected to */
+};
+
+struct intel_vgpu_fb_format {
+	struct intel_vgpu_pipe_format	pipes[I915_MAX_PIPES];
+};
+
+int intel_vgpu_decode_primary_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_primary_plane_format *plane);
+int intel_vgpu_decode_cursor_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_cursor_plane_format *plane);
+int intel_vgpu_decode_sprite_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_sprite_plane_format *plane);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 930732e..c42266c 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -46,6 +46,7 @@
 #include "sched_policy.h"
 #include "render.h"
 #include "cmd_parser.h"
+#include "fb_decoder.h"
 
 #define GVT_MAX_VGPU 8
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1666540 — [PATCH v9 3/7] drm: Extend the drm format

Add new drm format which will be used by GVT-g.

Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
---
 include/uapi/drm/drm_fourcc.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 55e3010..2681862 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -113,6 +113,10 @@ extern "C" {
 
 #define DRM_FORMAT_AYUV		fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
 
+/* 64 bpp RGB */
+#define DRM_FORMAT_XRGB161616  fourcc_code('X', 'R', '4', '8') /* [63:0] x:R:G:B 16:16:16:16 little endian */
+#define DRM_FORMAT_XBGR161616  fourcc_code('X', 'B', '4', '8') /* [63:0] x:B:G:R 16:16:16:16 little endian */
+
 /*
  * 2 plane RGB + A
  * index 0 = RGB plane, same format as the corresponding non _A8 format has
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1666626 — Re: [Intel-gfx] [PATCH v9 3/7] drm: Extend the drm format

On Thu, Jun 15, 2017 at 04:00:07PM +0800, Xiaoguang Chen wrote:
> Add new drm format which will be used by GVT-g.
> 
> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
> ---
>  include/uapi/drm/drm_fourcc.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 55e3010..2681862 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -113,6 +113,10 @@ extern "C" {
>  
>  #define DRM_FORMAT_AYUV		fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
>  
> +/* 64 bpp RGB */
> +#define DRM_FORMAT_XRGB161616  fourcc_code('X', 'R', '4', '8') /* [63:0] x:R:G:B 16:16:16:16 little endian */
> +#define DRM_FORMAT_XBGR161616  fourcc_code('X', 'B', '4', '8') /* [63:0] x:B:G:R 16:16:16:16 little endian */

Are these supposed to be the half float formats? If so the docs are
lacking. Also this sort of stuff must be sent to dri-devel for everyone
to see.

That said, I don't really like having them in any gvt code until
they're handled by the driver proper.

> +
>  /*
>   * 2 plane RGB + A
>   * index 0 = RGB plane, same format as the corresponding non _A8 format has
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[toc] | [prev] | [next] | [standalone]

#1666541 — [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

Here we defined a new ioctl to create a fd for a vfio device based on
the input type. Now only one type is supported that is a dma-buf
management fd.
Two ioctls are defined for the dma-buf management fd: query the vfio
vgpu's plane information and create a dma-buf for a plane.

Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
---
 include/uapi/linux/vfio.h | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index ae46105..7d86101 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -502,6 +502,63 @@ 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 */
+
+struct vfio_dmabuf_mgr_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;
+	__u32 padding;
+};
+
+/*
+ * VFIO_DMABUF_MGR_QUERY_PLANE - _IO(VFIO_TYPE, VFIO_BASE + 15,
+ *					struct vfio_dmabuf_mgr_query_plane)
+ * Query plane information
+ */
+struct vfio_dmabuf_mgr_query_plane {
+	__u32 argsz;
+	__u32 flags;
+	struct vfio_dmabuf_mgr_plane_info plane_info;
+	__u32 plane_id;
+};
+
+#define VFIO_DMABUF_MGR_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15)
+
+/*
+ * VFIO_DMABUF_MGR_CREATE_DMABUF - _IO(VFIO, VFIO_BASE + 16,
+ *					struct vfio_dmabuf_mgr_create_dmabuf)
+ *
+ * Create a dma-buf for a plane
+ */
+struct vfio_dmabuf_mgr_create_dmabuf {
+	__u32 argsz;
+	__u32 flags;
+	struct vfio_dmabuf_mgr_plane_info plane_info;
+	__u32 plane_id;
+	__s32 fd;
+};
+
+#define VFIO_DMABUF_MGR_CREATE_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 16)
+
 /* -------- API for Type1 VFIO IOMMU -------- */
 
 /**
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1666781 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

On 6/15/2017 1:30 PM, Xiaoguang Chen wrote:
> Here we defined a new ioctl to create a fd for a vfio device based on
> the input type. Now only one type is supported that is a dma-buf
> management fd.
> Two ioctls are defined for the dma-buf management fd: query the vfio
> vgpu's plane information and create a dma-buf for a plane.
> 

I had suggested how we can use common structures for both type of ways
to query surface on v6 version of your patch,
https://lkml.org/lkml/2017/6/1/890


> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
> ---
>  include/uapi/linux/vfio.h | 57 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index ae46105..7d86101 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -502,6 +502,63 @@ 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 */
> +
> +struct vfio_dmabuf_mgr_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;
> +	__u32 padding;
> +};
> +

This structure is generic, can remove dmabuf from its name,
vfio_plane_info or vfio_vgpu_surface_info since this will only be used
by vgpu.

> +/*
> + * VFIO_DMABUF_MGR_QUERY_PLANE - _IO(VFIO_TYPE, VFIO_BASE + 15,
> + *					struct vfio_dmabuf_mgr_query_plane)
> + * Query plane information
> + */
> +struct vfio_dmabuf_mgr_query_plane {
> +	__u32 argsz;
> +	__u32 flags;
> +	struct vfio_dmabuf_mgr_plane_info plane_info;
> +	__u32 plane_id;
> +};
> +
> +#define VFIO_DMABUF_MGR_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15)
> +

This same interface can be used to query surface/plane information for
both, dmabuf and region, case. Here also 'DMABUF' can be removed and
define flags if you want to differentiate query for 'dmabuf' and 'region'.

Thanks,
Kirti

> +/*
> + * VFIO_DMABUF_MGR_CREATE_DMABUF - _IO(VFIO, VFIO_BASE + 16,
> + *					struct vfio_dmabuf_mgr_create_dmabuf)
> + *
> + * Create a dma-buf for a plane
> + */
> +struct vfio_dmabuf_mgr_create_dmabuf {
> +	__u32 argsz;
> +	__u32 flags;
> +	struct vfio_dmabuf_mgr_plane_info plane_info;
> +	__u32 plane_id;
> +	__s32 fd;
> +};
> +
> +#define VFIO_DMABUF_MGR_CREATE_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 16)
> +
>  /* -------- API for Type1 VFIO IOMMU -------- */
>  /**
> 

[toc] | [prev] | [next] | [standalone]

#1666826 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

  Hi,

> > +struct vfio_dmabuf_mgr_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;
> > +	__u32 padding;
> > +};
> > +
> 
> This structure is generic, can remove dmabuf from its name,
> vfio_plane_info or vfio_vgpu_surface_info since this will only be
> used
> by vgpu.

Agree.

> > +struct vfio_dmabuf_mgr_query_plane {
> > +	__u32 argsz;
> > +	__u32 flags;
> > +	struct vfio_dmabuf_mgr_plane_info plane_info;
> > +	__u32 plane_id;
> > +};
> > +
> > +#define VFIO_DMABUF_MGR_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15)
> > +
> 
> This same interface can be used to query surface/plane information
> for
> both, dmabuf and region, case. Here also 'DMABUF' can be removed and
> define flags if you want to differentiate query for 'dmabuf' and
> 'region'.

Hmm, any specific reason why you want use a ioctl for that?  I would
simply place a "struct vfio_dmabuf_mgr_plane_info" (or whatever the
final name will be) at the start of the region.

cheers,
  Gerd

[toc] | [prev] | [next] | [standalone]

#1667246 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

On Thu, 15 Jun 2017 18:00:38 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > > +struct vfio_dmabuf_mgr_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;
> > > +	__u32 padding;
> > > +};
> > > +  
> > 
> > This structure is generic, can remove dmabuf from its name,
> > vfio_plane_info or vfio_vgpu_surface_info since this will only be
> > used
> > by vgpu.  
> 
> Agree.

I'm not sure I agree regarding the vgpu statement, maybe this is not
dmabuf specific, but what makes it vgpu specific?  We need to separate
our current usage plans from what it's actually describing and I don't
see that it describes anything vgpu specific.
 
> > > +struct vfio_dmabuf_mgr_query_plane {
> > > +	__u32 argsz;
> > > +	__u32 flags;
> > > +	struct vfio_dmabuf_mgr_plane_info plane_info;
> > > +	__u32 plane_id;
> > > +};
> > > +
> > > +#define VFIO_DMABUF_MGR_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15)
> > > +  
> > 
> > This same interface can be used to query surface/plane information
> > for
> > both, dmabuf and region, case. Here also 'DMABUF' can be removed and
> > define flags if you want to differentiate query for 'dmabuf' and
> > 'region'.  
> 
> Hmm, any specific reason why you want use a ioctl for that?  I would
> simply place a "struct vfio_dmabuf_mgr_plane_info" (or whatever the
> final name will be) at the start of the region.

Right, these are ioctls on the dmabuf mgr fd, not the vfio device fd,
if you're exposing a region with the info I wouldn't think you'd want
the hassle of managing this separate fd when you could do something
like Gerd suggests with defining the first page of the regions as
containing the structure.  Maybe you could even allow mmap of that page
to reduce the overhead of getting the current state.  For the sake of
userspace, I'd hope we'd still use the same structure for either the
ioctl or region mapping.  I'm not really in favor of declaring that
this particular ioctl might exist on the device fd when such-and-such
region is present otherwise it might exist on a dmabuf manager fd.
Thanks,

Alex

[toc] | [prev] | [next] | [standalone]

#1667616 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

  Hi,

> I'm not sure I agree regarding the vgpu statement, maybe this is not
> dmabuf specific, but what makes it vgpu specific?  We need to
> separate
> our current usage plans from what it's actually describing and I
> don't
> see that it describes anything vgpu specific.

Well, it describes a framebuffer, what non-graphic device would need
that?

cheers,
  Gerd

[toc] | [prev] | [next] | [standalone]

#1667736 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

On Fri, 16 Jun 2017 12:24:42 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > I'm not sure I agree regarding the vgpu statement, maybe this is not
> > dmabuf specific, but what makes it vgpu specific?  We need to
> > separate
> > our current usage plans from what it's actually describing and I
> > don't
> > see that it describes anything vgpu specific.  
> 
> Well, it describes a framebuffer, what non-graphic device would need
> that?

Graphics is not necessarily vgpu though, which is my point.  It should
not be named after our intended use case (vgpu), it should be named
after what it's describing (a framebuffer, or graphics plane).  Thanks,

Alex

[toc] | [prev] | [next] | [standalone]

#1667752 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

On 6/16/2017 2:08 AM, Alex Williamson wrote:
> On Thu, 15 Jun 2017 18:00:38 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
>>   Hi,
>>
>>>> +struct vfio_dmabuf_mgr_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;
>>>> +	__u32 padding;
>>>> +};
>>>> +  
>>>
>>> This structure is generic, can remove dmabuf from its name,
>>> vfio_plane_info or vfio_vgpu_surface_info since this will only be
>>> used
>>> by vgpu.  
>>
>> Agree.
> 
> I'm not sure I agree regarding the vgpu statement, maybe this is not
> dmabuf specific, but what makes it vgpu specific?  We need to separate
> our current usage plans from what it's actually describing and I don't
> see that it describes anything vgpu specific.
>  
>>>> +struct vfio_dmabuf_mgr_query_plane {
>>>> +	__u32 argsz;
>>>> +	__u32 flags;
>>>> +	struct vfio_dmabuf_mgr_plane_info plane_info;
>>>> +	__u32 plane_id;
>>>> +};
>>>> +
>>>> +#define VFIO_DMABUF_MGR_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15)
>>>> +  
>>>
>>> This same interface can be used to query surface/plane information
>>> for
>>> both, dmabuf and region, case. Here also 'DMABUF' can be removed and
>>> define flags if you want to differentiate query for 'dmabuf' and
>>> 'region'.  
>>
>> Hmm, any specific reason why you want use a ioctl for that?  I would
>> simply place a "struct vfio_dmabuf_mgr_plane_info" (or whatever the
>> final name will be) at the start of the region.
> 
> Right, these are ioctls on the dmabuf mgr fd, not the vfio device fd,
> if you're exposing a region with the info I wouldn't think you'd want
> the hassle of managing this separate fd when you could do something
> like Gerd suggests with defining the first page of the regions as
> containing the structure.

My suggestion was to use vfio device fd for this ioctl and have dmabuf
mgr fd as member in above query_plane structure, for region type it
would be set to 0.
Yes there is other way to query surface information as Gerd suggested,
but my point is: if a ioctl is being add, it could be used for both
types, dmabuf and region.

>  Maybe you could even allow mmap of that page
> to reduce the overhead of getting the current state.  

Can't mmap that page to get surface information. There is no way to
synchronize between QEMU reading this mmapped region and vendor driver
writing it. There could be race condition in these two operations. Read
on this page should be trapped and blocking, so that surface in that
region is only updated when its asked for.

> For the sake of
> userspace, I'd hope we'd still use the same structure for either the
> ioctl or region mapping.  I'm not really in favor of declaring that
> this particular ioctl might exist on the device fd when such-and-such
> region is present otherwise it might exist on a dmabuf manager fd.

Userspace will always use vfio device fd for this ioctl, it only have to
set proper arguments to its structure based on type.

Thanks,
Kirti

> Thanks,
> 
> Alex
> 

[toc] | [prev] | [next] | [standalone]

#1667894 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

On Fri, 16 Jun 2017 19:02:30 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> On 6/16/2017 2:08 AM, Alex Williamson wrote:
> > On Thu, 15 Jun 2017 18:00:38 +0200
> > Gerd Hoffmann <kraxel@redhat.com> wrote:
> >   
> >>   Hi,
> >>  
> >>>> +struct vfio_dmabuf_mgr_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;
> >>>> +	__u32 padding;
> >>>> +};
> >>>> +    
> >>>
> >>> This structure is generic, can remove dmabuf from its name,
> >>> vfio_plane_info or vfio_vgpu_surface_info since this will only be
> >>> used
> >>> by vgpu.    
> >>
> >> Agree.  
> > 
> > I'm not sure I agree regarding the vgpu statement, maybe this is not
> > dmabuf specific, but what makes it vgpu specific?  We need to separate
> > our current usage plans from what it's actually describing and I don't
> > see that it describes anything vgpu specific.
> >    
> >>>> +struct vfio_dmabuf_mgr_query_plane {
> >>>> +	__u32 argsz;
> >>>> +	__u32 flags;
> >>>> +	struct vfio_dmabuf_mgr_plane_info plane_info;
> >>>> +	__u32 plane_id;
> >>>> +};
> >>>> +
> >>>> +#define VFIO_DMABUF_MGR_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15)
> >>>> +    
> >>>
> >>> This same interface can be used to query surface/plane information
> >>> for
> >>> both, dmabuf and region, case. Here also 'DMABUF' can be removed and
> >>> define flags if you want to differentiate query for 'dmabuf' and
> >>> 'region'.    
> >>
> >> Hmm, any specific reason why you want use a ioctl for that?  I would
> >> simply place a "struct vfio_dmabuf_mgr_plane_info" (or whatever the
> >> final name will be) at the start of the region.  
> > 
> > Right, these are ioctls on the dmabuf mgr fd, not the vfio device fd,
> > if you're exposing a region with the info I wouldn't think you'd want
> > the hassle of managing this separate fd when you could do something
> > like Gerd suggests with defining the first page of the regions as
> > containing the structure.  
> 
> My suggestion was to use vfio device fd for this ioctl and have dmabuf
> mgr fd as member in above query_plane structure, for region type it
> would be set to 0.
> Yes there is other way to query surface information as Gerd suggested,
> but my point is: if a ioctl is being add, it could be used for both
> types, dmabuf and region.

I think this suggests abandoning the dmabuf manager fd entirely.  That's
not necessarily a bad thing, but I don't think the idea of the dmabuf
manager fd stands if we push one of its primary reasons for existing
back to the device fd.  Reading though previous posts, I think we
embraced the dmabuf manager as a separate fd primarily for
consolidation and the potential to use it as a notification point, the
latter being only theoretically useful.

So perhaps this becomes:

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;
};

struct vfio_device_query_gfx_plane {
	__u32 argsz;
	__u32 flags;
#define VFIO_GFX_PLANE_FLAGS_REGION_ID		(1 << 0)
#define VFIO_GFX_PLANE_FLAGS_PLANE_ID		(1 << 1)
	struct vfio_device_gfx_plane_info plane_info;
	__u32 id; 
};

The flag defines the data in the id field as either referring to a
region (perhaps there could be multiple regions with only one active)
or a plane ID, which is acquired separately, such as via a dmabuf fd.
This would be retrieved via an optional VFIO_DEVICE_QUERY_GFX_PLANE
ioctl on the vfio device, implemented in the vendor driver.

Would the above, along with the already defined mechanism for defining
device specific regions, account for NVIDIA's needs?

For dmabuf users, we'd still need a new ioctl to get the dmabuf fd.  We
could either create a specific ioctl for this (ex.
VFIO_DEVICE_GET_DMABUF_FD) or we could create a shared, generic GET_FD
interface on the device.

> >  Maybe you could even allow mmap of that page
> > to reduce the overhead of getting the current state.    
> 
> Can't mmap that page to get surface information. There is no way to
> synchronize between QEMU reading this mmapped region and vendor driver
> writing it. There could be race condition in these two operations. Read
> on this page should be trapped and blocking, so that surface in that
> region is only updated when its asked for.
> 
> > For the sake of
> > userspace, I'd hope we'd still use the same structure for either the
> > ioctl or region mapping.  I'm not really in favor of declaring that
> > this particular ioctl might exist on the device fd when such-and-such
> > region is present otherwise it might exist on a dmabuf manager fd.  
> 
> Userspace will always use vfio device fd for this ioctl, it only have to
> set proper arguments to its structure based on type.

Then we should kill off the manager fd unless there are arguments that
still give it value.  Thanks,

Alex

[toc] | [prev] | [next] | [standalone]

#1667991 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

On 6/16/2017 10:09 PM, Alex Williamson wrote:
> On Fri, 16 Jun 2017 19:02:30 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> 
>> On 6/16/2017 2:08 AM, Alex Williamson wrote:
>>> On Thu, 15 Jun 2017 18:00:38 +0200
>>> Gerd Hoffmann <kraxel@redhat.com> wrote:
>>>   
>>>>   Hi,
>>>>  
>>>>>> +struct vfio_dmabuf_mgr_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;
>>>>>> +	__u32 padding;
>>>>>> +};
>>>>>> +    
>>>>>
>>>>> This structure is generic, can remove dmabuf from its name,
>>>>> vfio_plane_info or vfio_vgpu_surface_info since this will only be
>>>>> used
>>>>> by vgpu.    
>>>>
>>>> Agree.  
>>>
>>> I'm not sure I agree regarding the vgpu statement, maybe this is not
>>> dmabuf specific, but what makes it vgpu specific?  We need to separate
>>> our current usage plans from what it's actually describing and I don't
>>> see that it describes anything vgpu specific.
>>>    
>>>>>> +struct vfio_dmabuf_mgr_query_plane {
>>>>>> +	__u32 argsz;
>>>>>> +	__u32 flags;
>>>>>> +	struct vfio_dmabuf_mgr_plane_info plane_info;
>>>>>> +	__u32 plane_id;
>>>>>> +};
>>>>>> +
>>>>>> +#define VFIO_DMABUF_MGR_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15)
>>>>>> +    
>>>>>
>>>>> This same interface can be used to query surface/plane information
>>>>> for
>>>>> both, dmabuf and region, case. Here also 'DMABUF' can be removed and
>>>>> define flags if you want to differentiate query for 'dmabuf' and
>>>>> 'region'.    
>>>>
>>>> Hmm, any specific reason why you want use a ioctl for that?  I would
>>>> simply place a "struct vfio_dmabuf_mgr_plane_info" (or whatever the
>>>> final name will be) at the start of the region.  
>>>
>>> Right, these are ioctls on the dmabuf mgr fd, not the vfio device fd,
>>> if you're exposing a region with the info I wouldn't think you'd want
>>> the hassle of managing this separate fd when you could do something
>>> like Gerd suggests with defining the first page of the regions as
>>> containing the structure.  
>>
>> My suggestion was to use vfio device fd for this ioctl and have dmabuf
>> mgr fd as member in above query_plane structure, for region type it
>> would be set to 0.
>> Yes there is other way to query surface information as Gerd suggested,
>> but my point is: if a ioctl is being add, it could be used for both
>> types, dmabuf and region.
> 
> I think this suggests abandoning the dmabuf manager fd entirely.  That's
> not necessarily a bad thing, but I don't think the idea of the dmabuf
> manager fd stands if we push one of its primary reasons for existing
> back to the device fd.  Reading though previous posts, I think we
> embraced the dmabuf manager as a separate fd primarily for
> consolidation and the potential to use it as a notification point, the
> latter being only theoretically useful.
> 
> So perhaps this becomes:
> 
> 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;
> };
> 
> struct vfio_device_query_gfx_plane {
> 	__u32 argsz;
> 	__u32 flags;
> #define VFIO_GFX_PLANE_FLAGS_REGION_ID		(1 << 0)
> #define VFIO_GFX_PLANE_FLAGS_PLANE_ID		(1 << 1)
> 	struct vfio_device_gfx_plane_info plane_info;
> 	__u32 id; 
> };
> 
> The flag defines the data in the id field as either referring to a
> region (perhaps there could be multiple regions with only one active)
> or a plane ID, which is acquired separately, such as via a dmabuf fd.
> This would be retrieved via an optional VFIO_DEVICE_QUERY_GFX_PLANE
> ioctl on the vfio device, implemented in the vendor driver.
> 
> Would the above, along with the already defined mechanism for defining
> device specific regions, account for NVIDIA's needs?
> 

Yes, works for region type solution that we would go with.

Thanks,
Kirti

> For dmabuf users, we'd still need a new ioctl to get the dmabuf fd.  We
> could either create a specific ioctl for this (ex.
> VFIO_DEVICE_GET_DMABUF_FD) or we could create a shared, generic GET_FD
> interface on the device.
> 
>>>  Maybe you could even allow mmap of that page
>>> to reduce the overhead of getting the current state.    
>>
>> Can't mmap that page to get surface information. There is no way to
>> synchronize between QEMU reading this mmapped region and vendor driver
>> writing it. There could be race condition in these two operations. Read
>> on this page should be trapped and blocking, so that surface in that
>> region is only updated when its asked for.
>>
>>> For the sake of
>>> userspace, I'd hope we'd still use the same structure for either the
>>> ioctl or region mapping.  I'm not really in favor of declaring that
>>> this particular ioctl might exist on the device fd when such-and-such
>>> region is present otherwise it might exist on a dmabuf manager fd.  
>>
>> Userspace will always use vfio device fd for this ioctl, it only have to
>> set proper arguments to its structure based on type.
> 
> Then we should kill off the manager fd unless there are arguments that
> still give it value.  Thanks,
> 
> Alex
> 

[toc] | [prev] | [next] | [standalone]

#1668815 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

  Hi,

> So perhaps this becomes:
> 
> 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;
> };

Looks good.

> struct vfio_device_query_gfx_plane {
> 	__u32 argsz;
> 	__u32 flags;
> #define VFIO_GFX_PLANE_FLAGS_REGION_ID		(1 << 0)
> #define VFIO_GFX_PLANE_FLAGS_PLANE_ID		(1 << 1)
> 	struct vfio_device_gfx_plane_info plane_info;
> 	__u32 id; 
> };

Hmm, plane isn't really an ID, it is a type, with type being either
DRM_PLANE_TYPE_PRIMARY or DRM_PLANE_TYPE_CURSOR, so I don't think the
flage above make sense.

What are the nvidia plane for cursor support btw?

> The flag defines the data in the id field as either referring to a
> region (perhaps there could be multiple regions with only one active)

Well, we have a "start" field in vfio_device_gfx_plane_info (maybe we
should rename that to "offset"), which can be used to place multiple
planes into a single, fixed region.

Also I think it would be useful to have some way to figure the device
capabilities as the userspace workflow will look quite different for
the two cases.

cheers,
  Gerd

[toc] | [prev] | [next] | [standalone]

#1669165 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

On Mon, 19 Jun 2017 08:34:13 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > So perhaps this becomes:
> > 
> > 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;
> > };  
> 
> Looks good.
> 
> > struct vfio_device_query_gfx_plane {
> > 	__u32 argsz;
> > 	__u32 flags;
> > #define VFIO_GFX_PLANE_FLAGS_REGION_ID		(1 << 0)
> > #define VFIO_GFX_PLANE_FLAGS_PLANE_ID		(1 << 1)
> > 	struct vfio_device_gfx_plane_info plane_info;
> > 	__u32 id; 
> > };  
> 
> Hmm, plane isn't really an ID, it is a type, with type being either
> DRM_PLANE_TYPE_PRIMARY or DRM_PLANE_TYPE_CURSOR, so I don't think the
> flage above make sense.

The intention was that ..._REGION_ID and ...PLANE_ID are describing
what the vfio_device_query_gfx_plane.id field represents, either a
region index or a plane identifier.  The type of plane would be
represented within the vfio_device_gfx_plane_info struct.

> What are the nvidia plane for cursor support btw?
> 
> > The flag defines the data in the id field as either referring to a
> > region (perhaps there could be multiple regions with only one active)  
> 
> Well, we have a "start" field in vfio_device_gfx_plane_info (maybe we
> should rename that to "offset"), which can be used to place multiple
> planes into a single, fixed region.

That seems reasonable.
 
> Also I think it would be useful to have some way to figure the device
> capabilities as the userspace workflow will look quite different for
> the two cases.

In the region case, VFIO_DEVICE_GET_REGION_INFO would include a device
specific region with a hopefully common identifier to identify it as a
graphics framebuffer.  VFIO_DEVICE_QUERY_GFX_PLANE would indicate the
plane as a region index.

In the dmabuf case, VFIO_DEVICE_QUERY_GFX_PLANE would indicate the
plane as a "plane ID" and some sort of
VFIO_DEVICE_GET_GFX_PLANE(VFIO_GFX_TYPE_DMABUF) ioctl would be
necessary to get a file descriptor to that plane.

What else are you thinking we need?  Thanks,

Alex

[toc] | [prev] | [next] | [standalone]

#1668814 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

  Hi,

> My suggestion was to use vfio device fd for this ioctl and have
> dmabuf
> mgr fd as member in above query_plane structure, for region type it
> would be set to 0.

Region type should be DRM_PLANE_TYPE_PRIMARY

> Can't mmap that page to get surface information. There is no way to
> synchronize between QEMU reading this mmapped region and vendor
> driver
> writing it. There could be race condition in these two operations.
> Read
> on this page should be trapped and blocking, so that surface in that
> region is only updated when its asked for.

Does it make sense to have a "generation" field in the plane_info
struct (which gets increased each time the struct changes) ?

cheers,
  Gerd

[toc] | [prev] | [next] | [standalone]

#1669148 — Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

On Mon, 19 Jun 2017 08:38:32 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > My suggestion was to use vfio device fd for this ioctl and have
> > dmabuf
> > mgr fd as member in above query_plane structure, for region type it
> > would be set to 0.  
> 
> Region type should be DRM_PLANE_TYPE_PRIMARY
> 
> > Can't mmap that page to get surface information. There is no way to
> > synchronize between QEMU reading this mmapped region and vendor
> > driver
> > writing it. There could be race condition in these two operations.
> > Read
> > on this page should be trapped and blocking, so that surface in that
> > region is only updated when its asked for.  
> 
> Does it make sense to have a "generation" field in the plane_info
> struct (which gets increased each time the struct changes) ?

It seems less cumbersome than checking each field to see if it has
changed.  Thanks,

Alex

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web