All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v12 0/6] drm/i915/gvt: Dma-buf support for GVT-g
@ 2017-07-19 10:58 Tina Zhang
  2017-07-19 10:59 ` [PATCH v12 1/6] drm/i915/gvt: Add framebuffer decoder support Tina Zhang
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Tina Zhang @ 2017-07-19 10:58 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev, dri-devel, ville.syrjala, zhenyuw,
	zhiyuan.lv, zhi.a.wang, alex.williamson, kraxel, chris, daniel,
	kwankhede, kevin.tian

v11->v12:
1) add drm_format_mod back. (Gerd and Zhenyu)
2) add region_index. (Gerd)
3) refine the lifecycle of dmabuf.
4) send to dri-devel@lists.freedesktop.org. (Ville) 

v10->v11:
1) rename plane_type to drm_plane_type. (Gerd)
2) move fields of vfio_device_query_gfx_plane to
   vfio_device_gfx_plane_info. (Gerd)
3) remove drm_format_mod, start fields. (Daniel)
4) remove plane_id.

v9->v10:
1) remove dma-buf management
2) refine the ABI API VFIO_DEVICE_QUERY_GFX_PLANE
3) track the dma-buf create and release in kernel mode

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.

Tina Zhang (6):
  drm/i915/gvt: Add framebuffer decoder support
  drm: Introduce RGB 64-bit 16:16:16:16 float format
  drm/i915/gvt: Add RGB 64-bit 16:16:16:16 float format support
  drm/i915/gvt: add opregion support
  vfio: ABI for mdev display dma-buf operation
  drm/i915: Introduce GEM proxy

 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  | 440 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/fb_decoder.h  | 175 +++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.h         |   1 +
 drivers/gpu/drm/i915/gvt/hypercall.h   |   1 +
 drivers/gpu/drm/i915/gvt/kvmgt.c       | 109 +++++++-
 drivers/gpu/drm/i915/gvt/mpt.h         |  15 ++
 drivers/gpu/drm/i915/gvt/opregion.c    |  26 +-
 drivers/gpu/drm/i915/gvt/vgpu.c        |   4 +
 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              |  28 +++
 16 files changed, 838 insertions(+), 12 deletions(-)
 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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v12 1/6] drm/i915/gvt: Add framebuffer decoder support
  2017-07-19 10:58 [PATCH v12 0/6] drm/i915/gvt: Dma-buf support for GVT-g Tina Zhang
@ 2017-07-19 10:59 ` Tina Zhang
  2017-07-19 10:59 ` [PATCH v12 2/6] drm: Introduce RGB 64-bit 16:16:16:16 float format Tina Zhang
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Tina Zhang @ 2017-07-19 10:59 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev, dri-devel, ville.syrjala, zhenyuw,
	zhiyuan.lv, zhi.a.wang, alex.williamson, kraxel, chris, daniel,
	kwankhede, kevin.tian
  Cc: Xiaoguang Chen

Framebuffer decoder returns guest framebuffer information.
Guest framebuffer includes primary, cursor and sprite plane.

Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
Signed-off-by: Tina Zhang <tina.zhang@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 | 429 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/fb_decoder.h | 175 ++++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.h        |   1 +
 6 files changed, 610 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 f5486cb9..019d596 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)
 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 2deb05f..58d90cf 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..2bd5b3c
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
@@ -0,0 +1,429 @@
+/*
+ * 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>
+ *    Tina Zhang <tina.zhang@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)"},
+	[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)"},
+
+
+	/* 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_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->base_gpa = intel_vgpu_gma_to_gpa(vgpu->gtt.ggtt_mm, plane->base);
+	if (plane->base_gpa == INTEL_GVT_INVALID_ADDR) {
+		gvt_vgpu_err("invalid gma address: %lx\n",
+				(unsigned long)plane->base);
+	}
+
+	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;
+	plane->base_gpa = intel_vgpu_gma_to_gpa(vgpu->gtt.ggtt_mm, plane->base);
+	if (plane->base_gpa == INTEL_GVT_INVALID_ADDR) {
+		gvt_vgpu_err("invalid gma address: %lx\n",
+				(unsigned long)plane->base);
+	}
+
+	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"},
+	[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->base_gpa = intel_vgpu_gma_to_gpa(vgpu->gtt.ggtt_mm, plane->base);
+	if (plane->base_gpa == INTEL_GVT_INVALID_ADDR) {
+		gvt_vgpu_err("invalid gma address: %lx\n",
+				(unsigned long)plane->base);
+	}
+
+	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..51a7e97
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.h
@@ -0,0 +1,175 @@
+/*
+ * 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>
+ *    Tina Zhang <tina.zhang@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 */
+	u64     base_gpa;
+	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 */
+	u64     base_gpa;
+	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 */
+	u64     base_gpa;
+	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 ba53ad1..d0fe4d0 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

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v12 2/6] drm: Introduce RGB 64-bit 16:16:16:16 float format
  2017-07-19 10:58 [PATCH v12 0/6] drm/i915/gvt: Dma-buf support for GVT-g Tina Zhang
  2017-07-19 10:59 ` [PATCH v12 1/6] drm/i915/gvt: Add framebuffer decoder support Tina Zhang
@ 2017-07-19 10:59 ` Tina Zhang
  2017-07-19 10:59 ` [PATCH v12 3/6] drm/i915/gvt: Add RGB 64-bit 16:16:16:16 float format support Tina Zhang
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Tina Zhang @ 2017-07-19 10:59 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev, dri-devel, ville.syrjala, zhenyuw,
	zhiyuan.lv, zhi.a.wang, alex.williamson, kraxel, chris, daniel,
	kwankhede, kevin.tian
  Cc: Xiaoguang Chen

The RGB 64-bit 16:16:16:16 float pixel format is needed by windows
guest VM. This patch is to introduce the format to drm.

v1:
Suggested by Ville to submit this patch to dri-devel.

Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
Signed-off-by: Tina Zhang <tina.zhang@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 7586c46..3e002e3 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

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v12 3/6] drm/i915/gvt: Add RGB 64-bit 16:16:16:16 float format support
  2017-07-19 10:58 [PATCH v12 0/6] drm/i915/gvt: Dma-buf support for GVT-g Tina Zhang
  2017-07-19 10:59 ` [PATCH v12 1/6] drm/i915/gvt: Add framebuffer decoder support Tina Zhang
  2017-07-19 10:59 ` [PATCH v12 2/6] drm: Introduce RGB 64-bit 16:16:16:16 float format Tina Zhang
@ 2017-07-19 10:59 ` Tina Zhang
  2017-07-19 10:59 ` [PATCH v12 4/6] drm/i915/gvt: add opregion support Tina Zhang
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Tina Zhang @ 2017-07-19 10:59 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev, dri-devel, ville.syrjala, zhenyuw,
	zhiyuan.lv, zhi.a.wang, alex.williamson, kraxel, chris, daniel,
	kwankhede, kevin.tian
  Cc: Xiaoguang Chen

The RGB 64-bit 16:16:16:16 float pixel format is needed by windows 10
guest VM. This patch is to add this pixel format support to gvt device
model. Without this patch, some Apps, e.g. "DXGIGammaVM.exe", will crash
and make guest screen black.

Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/gpu/drm/i915/gvt/fb_decoder.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c b/drivers/gpu/drm/i915/gvt/fb_decoder.c
index 2bd5b3c..739ca81 100644
--- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
@@ -54,6 +54,8 @@ static struct pixel_format bdw_pixel_formats[PRIMARY_FORMAT_NUM] = {
 		"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)"},
 };
@@ -75,6 +77,10 @@ static struct pixel_format skl_pixel_formats[] = {
 	{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},
@@ -101,6 +107,9 @@ static int skl_format_to_drm(int format, bool rgb_order, bool alpha,
 	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)
@@ -321,6 +330,8 @@ 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)"},
 };
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v12 4/6] drm/i915/gvt: add opregion support
  2017-07-19 10:58 [PATCH v12 0/6] drm/i915/gvt: Dma-buf support for GVT-g Tina Zhang
                   ` (2 preceding siblings ...)
  2017-07-19 10:59 ` [PATCH v12 3/6] drm/i915/gvt: Add RGB 64-bit 16:16:16:16 float format support Tina Zhang
@ 2017-07-19 10:59 ` Tina Zhang
  2017-07-19 10:59 ` [PATCH v12 5/6] vfio: ABI for mdev display dma-buf operation Tina Zhang
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Tina Zhang @ 2017-07-19 10:59 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev, dri-devel, ville.syrjala, zhenyuw,
	zhiyuan.lv, zhi.a.wang, alex.williamson, kraxel, chris, daniel,
	kwankhede, kevin.tian
  Cc: Xiaoguang Chen

Windows guest UPT driver can use operegion to configure the setting
for display. Without the opregion support, the display registers won't
be set and this blocks display model to get the correct information
of the guest display plane.

Signed-off-by: Bing Niu <bing.niu@intel.com>
Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/gpu/drm/i915/gvt/hypercall.h |   1 +
 drivers/gpu/drm/i915/gvt/kvmgt.c     | 109 ++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/gvt/mpt.h       |  15 +++++
 drivers/gpu/drm/i915/gvt/opregion.c  |  26 +++++++--
 drivers/gpu/drm/i915/gvt/vgpu.c      |   4 ++
 5 files changed, 146 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
index df7f33a..32c345c 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -55,6 +55,7 @@ struct intel_gvt_mpt {
 			      unsigned long mfn, unsigned int nr, bool map);
 	int (*set_trap_area)(unsigned long handle, u64 start, u64 end,
 			     bool map);
+	int (*set_opregion)(void *vgpu);
 };
 
 extern struct intel_gvt_mpt xengt_mpt;
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index fd0c85f..6b0a330 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -53,11 +53,23 @@ static const struct intel_gvt_ops *intel_gvt_ops;
 #define VFIO_PCI_INDEX_TO_OFFSET(index) ((u64)(index) << VFIO_PCI_OFFSET_SHIFT)
 #define VFIO_PCI_OFFSET_MASK    (((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1)
 
+#define OPREGION_SIGNATURE "IntelGraphicsMem"
+
+struct vfio_region;
+struct intel_vgpu_regops {
+	size_t (*rw)(struct intel_vgpu *vgpu, char *buf,
+			size_t count, loff_t *ppos, bool iswrite);
+	void (*release)(struct intel_vgpu *vgpu,
+			struct vfio_region *region);
+};
+
 struct vfio_region {
 	u32				type;
 	u32				subtype;
 	size_t				size;
 	u32				flags;
+	const struct intel_vgpu_regops	*ops;
+	void				*data;
 };
 
 struct kvmgt_pgfn {
@@ -430,6 +442,91 @@ static void kvmgt_protect_table_del(struct kvmgt_guest_info *info,
 	}
 }
 
+static size_t intel_vgpu_reg_rw_opregion(struct intel_vgpu *vgpu, char *buf,
+		size_t count, loff_t *ppos, bool iswrite)
+{
+	unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
+			VFIO_PCI_NUM_REGIONS;
+	void *base = vgpu->vdev.region[i].data;
+	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
+
+	if (pos >= vgpu->vdev.region[i].size || iswrite) {
+		gvt_vgpu_err("invalid op or offset for Intel vgpu OpRegion\n");
+		return -EINVAL;
+	}
+	count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
+	memcpy(buf, base + pos, count);
+
+	return count;
+}
+
+static void intel_vgpu_reg_release_opregion(struct intel_vgpu *vgpu,
+		struct vfio_region *region)
+{
+	memunmap(region->data);
+}
+
+static const struct intel_vgpu_regops intel_vgpu_regops_opregion = {
+	.rw = intel_vgpu_reg_rw_opregion,
+	.release = intel_vgpu_reg_release_opregion,
+};
+
+static int intel_vgpu_register_reg(struct intel_vgpu *vgpu,
+		unsigned int type, unsigned int subtype,
+		const struct intel_vgpu_regops *ops,
+		size_t size, u32 flags, void *data)
+{
+	struct vfio_region *region;
+
+	region = krealloc(vgpu->vdev.region,
+			(vgpu->vdev.num_regions + 1) * sizeof(*region),
+			GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+
+	vgpu->vdev.region = region;
+	vgpu->vdev.region[vgpu->vdev.num_regions].type = type;
+	vgpu->vdev.region[vgpu->vdev.num_regions].subtype = subtype;
+	vgpu->vdev.region[vgpu->vdev.num_regions].ops = ops;
+	vgpu->vdev.region[vgpu->vdev.num_regions].size = size;
+	vgpu->vdev.region[vgpu->vdev.num_regions].flags = flags;
+	vgpu->vdev.region[vgpu->vdev.num_regions].data = data;
+	vgpu->vdev.num_regions++;
+
+	return 0;
+}
+
+static int kvmgt_set_opregion(void *p_vgpu)
+{
+	struct intel_vgpu *vgpu = (struct intel_vgpu *)p_vgpu;
+	unsigned int addr;
+	void *base;
+	int ret;
+
+	addr = vgpu->gvt->opregion.opregion_pa;
+	if (!addr || !(~addr))
+		return -ENODEV;
+
+	base = memremap(addr, OPREGION_SIZE, MEMREMAP_WB);
+	if (!base)
+		return -ENOMEM;
+
+	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
+		memunmap(base);
+		return -EINVAL;
+	}
+
+	ret = intel_vgpu_register_reg(vgpu,
+			PCI_VENDOR_ID_INTEL | VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
+			VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION,
+			&intel_vgpu_regops_opregion, OPREGION_SIZE,
+			VFIO_REGION_INFO_FLAG_READ, base);
+	if (ret)
+		memunmap(base);
+
+	return ret;
+}
+
 static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
 {
 	struct intel_vgpu *vgpu = NULL;
@@ -646,7 +743,7 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf,
 	int ret = -EINVAL;
 
 
-	if (index >= VFIO_PCI_NUM_REGIONS) {
+	if (index >= VFIO_PCI_NUM_REGIONS + vgpu->vdev.num_regions) {
 		gvt_vgpu_err("invalid index: %u\n", index);
 		return -EINVAL;
 	}
@@ -680,8 +777,11 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf,
 	case VFIO_PCI_BAR5_REGION_INDEX:
 	case VFIO_PCI_VGA_REGION_INDEX:
 	case VFIO_PCI_ROM_REGION_INDEX:
+		break;
 	default:
-		gvt_vgpu_err("unsupported region: %u\n", index);
+		index -= VFIO_PCI_NUM_REGIONS;
+		return vgpu->vdev.region[index].ops->rw(vgpu, buf, count,
+				ppos, is_write);
 	}
 
 	return ret == 0 ? count : ret;
@@ -944,7 +1044,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 
 		info.flags = VFIO_DEVICE_FLAGS_PCI;
 		info.flags |= VFIO_DEVICE_FLAGS_RESET;
-		info.num_regions = VFIO_PCI_NUM_REGIONS;
+		info.num_regions = VFIO_PCI_NUM_REGIONS +
+				vgpu->vdev.num_regions;
 		info.num_irqs = VFIO_PCI_NUM_IRQS;
 
 		return copy_to_user((void __user *)arg, &info, minsz) ?
@@ -1065,6 +1166,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 		}
 
 		if (caps.size) {
+			info.flags |= VFIO_REGION_INFO_FLAG_CAPS;
 			if (info.argsz < sizeof(info) + caps.size) {
 				info.argsz = sizeof(info) + caps.size;
 				info.cap_offset = 0;
@@ -1513,6 +1615,7 @@ struct intel_gvt_mpt kvmgt_mpt = {
 	.read_gpa = kvmgt_read_gpa,
 	.write_gpa = kvmgt_write_gpa,
 	.gfn_to_mfn = kvmgt_gfn_to_pfn,
+	.set_opregion = kvmgt_set_opregion,
 };
 EXPORT_SYMBOL_GPL(kvmgt_mpt);
 
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index f0e5487..9e73f2e 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -292,4 +292,19 @@ static inline int intel_gvt_hypervisor_set_trap_area(
 	return intel_gvt_host.mpt->set_trap_area(vgpu->handle, start, end, map);
 }
 
+/**
+ * intel_gvt_hypervisor_set_opregion - Set opregion for guest
+ * @vgpu: a vGPU
+ *
+ * Returns:
+ * Zero on success, negative error code if failed.
+ */
+static inline int intel_gvt_hypervisor_set_opregion(struct intel_vgpu *vgpu)
+{
+	if (!intel_gvt_host.mpt->set_opregion)
+		return 0;
+
+	return intel_gvt_host.mpt->set_opregion(vgpu);
+}
+
 #endif /* _GVT_MPT_H_ */
diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c
index 3117991..04c0452 100644
--- a/drivers/gpu/drm/i915/gvt/opregion.c
+++ b/drivers/gpu/drm/i915/gvt/opregion.c
@@ -113,23 +113,37 @@ void intel_vgpu_clean_opregion(struct intel_vgpu *vgpu)
  */
 int intel_vgpu_init_opregion(struct intel_vgpu *vgpu, u32 gpa)
 {
-	int ret;
+	int ret = 0;
+	unsigned long pfn;
 
 	gvt_dbg_core("vgpu%d: init vgpu opregion\n", vgpu->id);
 
-	if (intel_gvt_host.hypervisor_type == INTEL_GVT_HYPERVISOR_XEN) {
+	switch (intel_gvt_host.hypervisor_type) {
+	case INTEL_GVT_HYPERVISOR_KVM:
+		pfn = intel_gvt_hypervisor_gfn_to_mfn(vgpu, gpa >> PAGE_SHIFT);
+		vgpu_opregion(vgpu)->va = memremap(pfn << PAGE_SHIFT,
+						INTEL_GVT_OPREGION_SIZE,
+						MEMREMAP_WB);
+		if (!vgpu_opregion(vgpu)->va) {
+			gvt_vgpu_err("failed to map guest opregion\n");
+			ret = -EFAULT;
+		}
+		break;
+	case INTEL_GVT_HYPERVISOR_XEN:
 		gvt_dbg_core("emulate opregion from kernel\n");
 
 		ret = init_vgpu_opregion(vgpu, gpa);
 		if (ret)
-			return ret;
+			break;
 
 		ret = map_vgpu_opregion(vgpu, true);
-		if (ret)
-			return ret;
+		break;
+	default:
+		ret = -EINVAL;
+		gvt_vgpu_err("not supported hypervisor\n");
 	}
 
-	return 0;
+	return ret;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 90c14e6..03f9539 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -383,6 +383,10 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
 	if (ret)
 		goto out_clean_shadow_ctx;
 
+	ret = intel_gvt_hypervisor_set_opregion(vgpu);
+	if (ret)
+		goto out_clean_shadow_ctx;
+
 	mutex_unlock(&gvt->lock);
 
 	return vgpu;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v12 5/6] vfio: ABI for mdev display dma-buf operation
  2017-07-19 10:58 [PATCH v12 0/6] drm/i915/gvt: Dma-buf support for GVT-g Tina Zhang
                   ` (3 preceding siblings ...)
  2017-07-19 10:59 ` [PATCH v12 4/6] drm/i915/gvt: add opregion support Tina Zhang
@ 2017-07-19 10:59 ` Tina Zhang
  2017-07-19 10:59 ` [PATCH v12 6/6] drm/i915: Introduce GEM proxy Tina Zhang
  2017-07-19 12:06 ` ✓ Fi.CI.BAT: success for drm/i915/gvt: Dma-buf support for GVT-g Patchwork
  6 siblings, 0 replies; 10+ messages in thread
From: Tina Zhang @ 2017-07-19 10:59 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev, dri-devel, ville.syrjala, zhenyuw,
	zhiyuan.lv, zhi.a.wang, alex.williamson, kraxel, chris, daniel,
	kwankhede, kevin.tian

Add VFIO_DEVICE_QUERY_GFX_PLANE ioctl command to let user mode query and
get the plan and its related information.

The dma-buf's life cycle is handled by user mode and tracked by kernel.
The returned fd in struct vfio_device_query_gfx_plane can be a new
fd or an old fd of a re-exported dma-buf. Host User mode can check the
value of fd and to see if it needs to create new resource according to
the new fd or just use the existed resource related to the old fd.

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 include/uapi/linux/vfio.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index ae46105..827a230 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -502,6 +502,34 @@ struct vfio_pci_hot_reset {
 
 #define VFIO_DEVICE_PCI_HOT_RESET	_IO(VFIO_TYPE, VFIO_BASE + 13)
 
+/**
+ * VFIO_DEVICE_QUERY_GFX_PLANE - _IOW(VFIO_TYPE, VFIO_BASE + 14, struct vfio_device_query_gfx_plane)
+ *
+ * Set the drm_plane_type and retrieve information about the gfx plane.
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+struct vfio_device_gfx_plane_info {
+	__u32 argsz;
+	__u32 flags;
+	/* in */
+	__u32 drm_plane_type;	/* type of plane: DRM_PLANE_TYPE_* */
+	/* out */
+	__u32 drm_format;	/* drm format of plane */
+	__u64 drm_format_mod;   /* tiled mode */
+	__u32 width;	/* width of plane */
+	__u32 height;	/* height of plane */
+	__u32 stride;	/* stride of plane */
+	__u32 size;	/* size of plane in bytes, align on page*/
+	__u32 x_pos;	/* horizontal position of cursor plane, upper left corner in pixels */
+	__u32 y_pos;	/* vertical position of cursor plane, upper left corner in lines*/
+	__u32 region_index;
+	__s32 fd;	/* dma-buf fd */
+};
+
+#define VFIO_DEVICE_QUERY_GFX_PLANE _IO(VFIO_TYPE, VFIO_BASE + 14)
+
+
 /* -------- API for Type1 VFIO IOMMU -------- */
 
 /**
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v12 6/6] drm/i915: Introduce GEM proxy
  2017-07-19 10:58 [PATCH v12 0/6] drm/i915/gvt: Dma-buf support for GVT-g Tina Zhang
                   ` (4 preceding siblings ...)
  2017-07-19 10:59 ` [PATCH v12 5/6] vfio: ABI for mdev display dma-buf operation Tina Zhang
@ 2017-07-19 10:59 ` Tina Zhang
  2017-07-19 11:20   ` Chris Wilson
  2017-07-19 12:06 ` ✓ Fi.CI.BAT: success for drm/i915/gvt: Dma-buf support for GVT-g Patchwork
  6 siblings, 1 reply; 10+ messages in thread
From: Tina Zhang @ 2017-07-19 10:59 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev, dri-devel, ville.syrjala, zhenyuw,
	zhiyuan.lv, zhi.a.wang, alex.williamson, kraxel, chris, daniel,
	kwankhede, kevin.tian
  Cc: Tina Zhang

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 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 +++++
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1b2dfa8..ebacc04 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1612,6 +1612,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.
@@ -1680,6 +1686,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);
@@ -1730,7 +1742,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,
@@ -3764,6 +3776,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;
 
@@ -4210,6 +4228,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 5b19a49..c91e336 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 BIT(0)
 #define I915_GEM_OBJECT_IS_SHRINKABLE   BIT(1)
+#define I915_GEM_OBJECT_IS_PROXY	BIT(2)
 
 	/* Interface between the GEM object and its backing storage.
 	 * get_pages() is called once prior to the use of the associated set
@@ -198,6 +199,8 @@ struct drm_i915_gem_object {
 		} userptr;
 
 		unsigned long scratch;
+
+		void *gvt_info;
 	};
 
 	/** for phys allocated objects */
@@ -300,6 +303,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 fb5231f..21ec066 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v12 6/6] drm/i915: Introduce GEM proxy
  2017-07-19 10:59 ` [PATCH v12 6/6] drm/i915: Introduce GEM proxy Tina Zhang
@ 2017-07-19 11:20   ` Chris Wilson
  2017-07-21  4:49     ` Zhang, Tina
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2017-07-19 11:20 UTC (permalink / raw)
  To: Tina Zhang, intel-gfx, intel-gvt-dev, dri-devel, ville.syrjala,
	zhenyuw, zhiyuan.lv, zhi.a.wang, alex.williamson, kraxel, daniel,
	kwankhede, kevin.tian

s/GEM proxy/a GEM proxy object/

Quoting Tina Zhang (2017-07-19 11:59:05)

Rationale goes here.

> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> ---
>  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 +++++
>  3 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1b2dfa8..ebacc04 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1612,6 +1612,12 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>         if (!obj)
>                 return -ENOENT;
>  
> +       /* proxy gem object does not support setting domain */

Yes, this is what the code is doing. The comment tells us why.

/*
 * Proxy objects do not control access to the backing storage, ergo
 * they cannot be used as a means to manipulate the cache domain
 * tracking for that backing storage. The proxy object is always
 * considered to be outside of any cache domain.
 */

However, set-domain does have some other side-effects that includes
waiting which should still be performed, i.e. this check should be after
the lockless wait.

> +       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.
> @@ -1680,6 +1686,12 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
>         if (!obj)
>                 return -ENOENT;
>  

A comment could explain that since proxy objects are barred from CPU
access, we do not need to ban sw_finish as it is a nop.

> +       /* 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);
> @@ -1730,7 +1742,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,
> @@ -3764,6 +3776,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 */

More rationale. Also is the proxied object (its target) also banned from
changing the PTE bits or do we inherit all such changes automatically?

> +       if (i915_gem_object_is_proxy(obj)) {
> +               ret = -EPERM;
> +               goto out;
> +       }
> +
>         if (obj->cache_level == level)
>                 goto out;
>  
> @@ -4210,6 +4228,12 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
>         if (!obj)
>                 return -ENOENT;
>  
> +       /* proxy gem obj does not support changing backding storage */

This one could be generalised to I915_GEM_OBJECT_IS_SHRINKABLE?

> +       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 5b19a49..c91e336 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 BIT(0)
>  #define I915_GEM_OBJECT_IS_SHRINKABLE   BIT(1)
> +#define I915_GEM_OBJECT_IS_PROXY       BIT(2)
>  
>         /* Interface between the GEM object and its backing storage.
>          * get_pages() is called once prior to the use of the associated set
> @@ -198,6 +199,8 @@ struct drm_i915_gem_object {
>                 } userptr;
>  
>                 unsigned long scratch;
> +
> +               void *gvt_info;

Unrelated chunk, this should go into the gvt patch to use the object.

>         };
>  
>         /** for phys allocated objects */
> @@ -300,6 +303,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 fb5231f..21ec066 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;
> +       }

Changing the tiling mode may well be justifiable, even for a proxy since
the tiling is local to the view. The ban on GVT behalf should be done via
obj->framebuffer_references, and a good rationale given here on why the
proxy should be banned.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/gvt: Dma-buf support for GVT-g
  2017-07-19 10:58 [PATCH v12 0/6] drm/i915/gvt: Dma-buf support for GVT-g Tina Zhang
                   ` (5 preceding siblings ...)
  2017-07-19 10:59 ` [PATCH v12 6/6] drm/i915: Introduce GEM proxy Tina Zhang
@ 2017-07-19 12:06 ` Patchwork
  6 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-07-19 12:06 UTC (permalink / raw)
  To: Tina Zhang; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gvt: Dma-buf support for GVT-g
URL   : https://patchwork.freedesktop.org/series/27572/
State : success

== Summary ==

Series 27572v1 drm/i915/gvt: Dma-buf support for GVT-g
https://patchwork.freedesktop.org/api/1.0/series/27572/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:453s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:438s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:356s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:545s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:512s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:496s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:485s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:606s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:439s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:417s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:417s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:496s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:472s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:469s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:576s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:590s
fi-pnv-d510      total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  time:560s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:463s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:582s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:472s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:474s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:433s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:474s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:546s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:404s

541e75cb2ac9f85388e9e6f228d98f266506eebd drm-tip: 2017y-07m-19d-10h-10m-36s UTC integration manifest
2cd6b00 drm/i915: Introduce GEM proxy
6ca8e14 vfio: ABI for mdev display dma-buf operation
fd3a206 drm/i915/gvt: add opregion support
11cb619 drm/i915/gvt: Add RGB 64-bit 16:16:16:16 float format support
38501de drm: Introduce RGB 64-bit 16:16:16:16 float format
879bb65 drm/i915/gvt: Add framebuffer decoder support

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5233/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v12 6/6] drm/i915: Introduce GEM proxy
  2017-07-19 11:20   ` Chris Wilson
@ 2017-07-21  4:49     ` Zhang, Tina
  0 siblings, 0 replies; 10+ messages in thread
From: Zhang, Tina @ 2017-07-21  4:49 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, intel-gvt-dev, dri-devel, ville.syrjala,
	zhenyuw, Lv, Zhiyuan, Wang, Zhi A, alex.williamson, kraxel,
	daniel, kwankhede, Tian, Kevin



> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Wednesday, July 19, 2017 7:20 PM
> To: Zhang, Tina <tina.zhang@intel.com>; intel-gfx@lists.freedesktop.org; intel-
> gvt-dev@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> ville.syrjala@linux.intel.com; zhenyuw@linux.intel.com; Lv, Zhiyuan
> <zhiyuan.lv@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>;
> alex.williamson@redhat.com; kraxel@redhat.com; daniel@ffwll.ch;
> kwankhede@nvidia.com; Tian, Kevin <kevin.tian@intel.com>
> Cc: Zhang, Tina <tina.zhang@intel.com>
> Subject: Re: [PATCH v12 6/6] drm/i915: Introduce GEM proxy
> 
> s/GEM proxy/a GEM proxy object/
> 
> Quoting Tina Zhang (2017-07-19 11:59:05)
> 
> Rationale goes here.
Thanks for the comments. The idea behind the GEM proxy is that we want to propose a kind of GEM which content is produced by the guest VM and used by host. So, to host, this kind of GEM should be read only (both for CPU and GPU), and host cannot use ioctls to change or modify the GEM.
I will add more comments in the next version. Thanks.

> 
> > Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> > ---
> >  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 +++++
> >  3 files changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c index 1b2dfa8..ebacc04 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1612,6 +1612,12 @@ i915_gem_set_domain_ioctl(struct drm_device
> *dev, void *data,
> >         if (!obj)
> >                 return -ENOENT;
> >
> > +       /* proxy gem object does not support setting domain */
> 
> Yes, this is what the code is doing. The comment tells us why.
> 
> /*
>  * Proxy objects do not control access to the backing storage, ergo
>  * they cannot be used as a means to manipulate the cache domain
>  * tracking for that backing storage. The proxy object is always
>  * considered to be outside of any cache domain.
>  */
> 
> However, set-domain does have some other side-effects that includes waiting
> which should still be performed, i.e. this check should be after the lockless wait.
Is it the requirement of this ioctl? Other ioctls here in this patch, won't need it?

> 
> > +       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.
> > @@ -1680,6 +1686,12 @@ i915_gem_sw_finish_ioctl(struct drm_device
> *dev, void *data,
> >         if (!obj)
> >                 return -ENOENT;
> >
> 
> A comment could explain that since proxy objects are barred from CPU access,
> we do not need to ban sw_finish as it is a nop.
Agree.

> 
> > +       /* 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);
> > @@ -1730,7 +1742,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, @@ -3764,6
> > +3776,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 */
> 
> More rationale. Also is the proxied object (its target) also banned from changing
> the PTE bits or do we inherit all such changes automatically?
From v2, cache_level isn't be set during the GEM generation, i.e. cache_level=0. And the PTE bits which is set by guest, won't be modified by host.

> 
> > +       if (i915_gem_object_is_proxy(obj)) {
> > +               ret = -EPERM;
> > +               goto out;
> > +       }
> > +
> >         if (obj->cache_level == level)
> >                 goto out;
> >
> > @@ -4210,6 +4228,12 @@ i915_gem_madvise_ioctl(struct drm_device
> *dev, void *data,
> >         if (!obj)
> >                 return -ENOENT;
> >
> > +       /* proxy gem obj does not support changing backding storage */
> 
> This one could be generalised to I915_GEM_OBJECT_IS_SHRINKABLE?
I suppose no. The backend pages should always be the guest framebuffer. And when to release it is up to user mode. If the kernel part senses the guest framebuffer is changed, it will generate a new GEM/dma-buf with the new framebuffer backend, to user mode.


> 
> > +       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 5b19a49..c91e336 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 BIT(0)
> >  #define I915_GEM_OBJECT_IS_SHRINKABLE   BIT(1)
> > +#define I915_GEM_OBJECT_IS_PROXY       BIT(2)
> >
> >         /* Interface between the GEM object and its backing storage.
> >          * get_pages() is called once prior to the use of the
> > associated set @@ -198,6 +199,8 @@ struct drm_i915_gem_object {
> >                 } userptr;
> >
> >                 unsigned long scratch;
> > +
> > +               void *gvt_info;
> 
> Unrelated chunk, this should go into the gvt patch to use the object.
> 
> >         };
> >
> >         /** for phys allocated objects */ @@ -300,6 +303,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 fb5231f..21ec066 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;
> > +       }
> 
> Changing the tiling mode may well be justifiable, even for a proxy since the tiling
> is local to the view. The ban on GVT behalf should be done via
> obj->framebuffer_references, and a good rationale given here on why the
> proxy should be banned.
The GEM's content and tiling mode is produced by guest, in host, how could we guarantee the correction of the view if we allow the tiling modification through this ioctl?
And I guess we tried the obj->framebuffer_references before, based on the discussion in v6 (https://lists.freedesktop.org/archives/intel-gvt-dev/2017-June/001146.html)
Thanks.

Tina
> -Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-07-21  4:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-19 10:58 [PATCH v12 0/6] drm/i915/gvt: Dma-buf support for GVT-g Tina Zhang
2017-07-19 10:59 ` [PATCH v12 1/6] drm/i915/gvt: Add framebuffer decoder support Tina Zhang
2017-07-19 10:59 ` [PATCH v12 2/6] drm: Introduce RGB 64-bit 16:16:16:16 float format Tina Zhang
2017-07-19 10:59 ` [PATCH v12 3/6] drm/i915/gvt: Add RGB 64-bit 16:16:16:16 float format support Tina Zhang
2017-07-19 10:59 ` [PATCH v12 4/6] drm/i915/gvt: add opregion support Tina Zhang
2017-07-19 10:59 ` [PATCH v12 5/6] vfio: ABI for mdev display dma-buf operation Tina Zhang
2017-07-19 10:59 ` [PATCH v12 6/6] drm/i915: Introduce GEM proxy Tina Zhang
2017-07-19 11:20   ` Chris Wilson
2017-07-21  4:49     ` Zhang, Tina
2017-07-19 12:06 ` ✓ Fi.CI.BAT: success for drm/i915/gvt: Dma-buf support for GVT-g Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.