Re: [Intel-gfx] [PATCH v8 3/6] drm/i915/gvt: Frame buffer decoder support for GVT-g
>-Original Message-
>From: Zhenyu Wang [mailto:zhen...@linux.intel.com]
>Sent: Wednesday, June 14, 2017 5:39 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: alex.william...@redhat.com; kra...@redhat.com; ch...@chris-wilson.co.uk;
>intel-gfx@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>
>Subject: Re: [PATCH v8 3/6] drm/i915/gvt: Frame buffer decoder support for GVT-
>g
>
>On 2017.06.09 14:50:39 +0800, Xiaoguang Chen wrote:
>> decode frambuffer attributes of primary, cursor and sprite plane
>>
>> Signed-off-by: Xiaoguang Chen <xiaoguang.c...@intel.com>
>
>...
>
>> +/**
>> + * 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 (!skl_pixel_formats[fmt].bpp && !bdw_pixel_formats[fmt].bpp) {
>> +gvt_vgpu_err("Non-supported pixel format (0x%x)\n", fmt);
>> +return -EINVAL;
>> +}
>
>Is this correct? shouldn't be plane->bpp as last time comment?
Yes. But use plane->bpp is more concisely. Will change.
>
>> diff --git a/include/uapi/drm/drm_fourcc.h
>> b/include/uapi/drm/drm_fourcc.h index 55e3010..400759f 100644
>> --- a/include/uapi/drm/drm_fourcc.h
>> +++ b/include/uapi/drm/drm_fourcc.h
>> @@ -161,6 +161,12 @@ extern "C" {
>> #define DRM_FORMAT_YUV444 fourcc_code('Y', 'U', '2', '4') /* non-
>subsampled Cb (1) and Cr (2) planes */
>> #define DRM_FORMAT_YVU444 fourcc_code('Y', 'V', '2', '4') /* non-
>subsampled Cr (1) and Cb (2) planes */
>>
>> +/*
>> + * Intel GVT-g plane format definition */ #define
>> +DRM_FORMAT_XRGB161616_GVT fourcc_code('X', 'R', '4', '8') /* [63:0]
>> +x:R:G:B 16:16:16:16 little endian */ #define
>> +DRM_FORMAT_XBGR161616_GVT fourcc_code('X', 'B', '4', '8') /* [63:0]
>> +x:B:G:R 16:16:16:16 little endian */
>> +
>>
>
>This should be a seperate patch and not need GVT postfix for format definition.
OK.
>
>--
>Open Source Technology Center, Intel ltd.
>
>$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf operations
>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Wednesday, June 14, 2017 11:46 AM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: Tian, Kevin <kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; ch...@chris-wilson.co.uk; Lv,
>Zhiyuan <zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi
>A <zhi.a.w...@intel.com>; kra...@redhat.com
>Subject: Re: [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf operations
>
>On Wed, 14 Jun 2017 03:18:31 +
>"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
>
>> >-Original Message-
>> >From: intel-gvt-dev
>> >[mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On Behalf Of
>> >Alex Williamson
>> >Sent: Wednesday, June 14, 2017 11:06 AM
>> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>> >Cc: Tian, Kevin <kevin.t...@intel.com>;
>> >intel-gfx@lists.freedesktop.org; linux- ker...@vger.kernel.org;
>> >zhen...@linux.intel.com; ch...@chris-wilson.co.uk; Lv, Zhiyuan
>> ><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang,
>> >Zhi A <zhi.a.w...@intel.com>; kra...@redhat.com
>> >Subject: Re: [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf
>> >operations
>> >
>> >On Wed, 14 Jun 2017 02:53:24 +
>> >"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
>> >
>> >> >-Original Message-
>> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >> >Sent: Wednesday, June 14, 2017 5:25 AM
>> >> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>> >> >Cc: kra...@redhat.com; ch...@chris-wilson.co.uk; intel-
>> >> >g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>> >> >zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>;
>> >> >intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A
>> >> ><zhi.a.w...@intel.com>; Tian, Kevin <kevin.t...@intel.com>
>> >> >Subject: Re: [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf
>> >> >operations
>> >> >
>> >> >On Fri, 9 Jun 2017 14:50:40 +0800 Xiaoguang Chen
>> >> ><xiaoguang.c...@intel.com> 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.
>> >> >>
>> >> >> Signed-off-by: Xiaoguang Chen <xiaoguang.c...@intel.com>
>> >> >> ---
>> >> >> include/uapi/linux/vfio.h | 58
>> >> >> +++
>> >> >> 1 file changed, 58 insertions(+)
>> >> >>
>> >> >> diff --git a/include/uapi/linux/vfio.h
>> >> >> b/include/uapi/linux/vfio.h index ae46105..24427b7 100644
>> >> >> --- a/include/uapi/linux/vfio.h
>> >> >> +++ b/include/uapi/linux/vfio.h
>> >> >> @@ -502,6 +502,64 @@ 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)
>> >> >> +
>> >> >> +struct vfio_vgpu_plane_info {
>> >> >> + __u64 start;
>> >> >> + __u64 drm_format_mod;
>> >> >> + __u32 drm_format;
>> >> >> + __u32 width;
>> >> >> + __u32 height;
>> >> >> + __u32 s
Re: [Intel-gfx] [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf operations
>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Alex Williamson
>Sent: Wednesday, June 14, 2017 11:06 AM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: Tian, Kevin <kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; ch...@chris-wilson.co.uk; Lv,
>Zhiyuan <zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi
>A <zhi.a.w...@intel.com>; kra...@redhat.com
>Subject: Re: [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf operations
>
>On Wed, 14 Jun 2017 02:53:24 +
>"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
>
>> >-Original Message-
>> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >Sent: Wednesday, June 14, 2017 5:25 AM
>> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>> >Cc: kra...@redhat.com; ch...@chris-wilson.co.uk; intel-
>> >g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>> >zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>;
>> >intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A
>> ><zhi.a.w...@intel.com>; Tian, Kevin <kevin.t...@intel.com>
>> >Subject: Re: [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf
>> >operations
>> >
>> >On Fri, 9 Jun 2017 14:50:40 +0800
>> >Xiaoguang Chen <xiaoguang.c...@intel.com> 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.
>> >>
>> >> Signed-off-by: Xiaoguang Chen <xiaoguang.c...@intel.com>
>> >> ---
>> >> include/uapi/linux/vfio.h | 58
>> >> +++
>> >> 1 file changed, 58 insertions(+)
>> >>
>> >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> >> index ae46105..24427b7 100644
>> >> --- a/include/uapi/linux/vfio.h
>> >> +++ b/include/uapi/linux/vfio.h
>> >> @@ -502,6 +502,64 @@ 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)
>> >> +
>> >> +struct vfio_vgpu_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;
>> >> +};
>> >> +
>> >> +#define VFIO_DEVICE_DMABUF_MGR_FD0 /* Supported fd types */
>> >
>> >Move this #define up above vfio_vgpu_plane_info to associate it with
>> >the VFIO_DEVICE_GET_FD ioctl.
>> OK.
>>
>> >
>> >> +
>> >> +/*
>> >> + * VFIO_DEVICE_QUERY_PLANE - _IO(VFIO_TYPE, VFIO_BASE + 15,
>> >> + * struct
>> >> vfio_vgpu_query_plane)
>> >> + * Query plane information
>> >> + */
>> >> +struct vfio_vgpu_query_plane {
>> >> + __u32 argsz;
>> >> + __u32 flags;
>> >> + struct vfio_vgpu_plane_info plane_info;
>> >> + __u32 plane_id;
>> >> + __u32 padding;
>> >
>> >This padding doesn't make sense.
>> This padding is still needed if we do not move the plane_id into
>vfio_vgpu_plane_info. Right?
>
>I don't see why this padding is ever needed, can you explain?
I thought we add the padding to make sure the structure layout is the same in
both 32bit and 64bit systems.
Am I right?
>Does the structure
>not being a multiple
Re: [Intel-gfx] [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf operations
>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Wednesday, June 14, 2017 5:25 AM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: kra...@redhat.com; ch...@chris-wilson.co.uk; intel-
>g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>
>Subject: Re: [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf operations
>
>On Fri, 9 Jun 2017 14:50:40 +0800
>Xiaoguang Chen <xiaoguang.c...@intel.com> 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.
>>
>> Signed-off-by: Xiaoguang Chen <xiaoguang.c...@intel.com>
>> ---
>> include/uapi/linux/vfio.h | 58
>> +++
>> 1 file changed, 58 insertions(+)
>>
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index ae46105..24427b7 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -502,6 +502,64 @@ 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)
>> +
>> +struct vfio_vgpu_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;
>> +};
>> +
>> +#define VFIO_DEVICE_DMABUF_MGR_FD 0 /* Supported fd types */
>
>Move this #define up above vfio_vgpu_plane_info to associate it with the
>VFIO_DEVICE_GET_FD ioctl.
OK.
>
>> +
>> +/*
>> + * VFIO_DEVICE_QUERY_PLANE - _IO(VFIO_TYPE, VFIO_BASE + 15,
>> + * struct vfio_vgpu_query_plane)
>> + * Query plane information
>> + */
>> +struct vfio_vgpu_query_plane {
>> +__u32 argsz;
>> +__u32 flags;
>> +struct vfio_vgpu_plane_info plane_info;
>> +__u32 plane_id;
>> +__u32 padding;
>
>This padding doesn't make sense.
This padding is still needed if we do not move the plane_id into
vfio_vgpu_plane_info. Right?
>
>> +};
>> +
>> +#define VFIO_DEVICE_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15)
>> +
>> +/*
>> + * VFIO_DEVICE_CREATE_DMABUF - _IO(VFIO, VFIO_BASE + 16,
>> + * struct
>vfio_vgpu_create_dmabuf)
>> + *
>> + * Create a dma-buf for a plane
>> + */
>> +struct vfio_vgpu_create_dmabuf {
>> +__u32 argsz;
>> +__u32 flags;
>> +struct vfio_vgpu_plane_info plane_info;
>> +__s32 fd;
>> +__u32 plane_id;
>> +};
>
>Both of these have a plane_id, should plane_id simply replace the padding in
>plane_info?
Precisely speaking plane_id does not belong to the plane info. All the other
information are decoded from plane except plane id.
>If not, let's at least put them in the same order so that plane_id is
>after plane_info for both structs.
Ok.
>
>> +
>> +#define VFIO_DEVICE_CREATE_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 16)
>
>I don't think these should be named just VFIO_DEVICE_foo, that implies they're
>ioctls on the vfio device fd, they're not. They need to be associated both in
>name
>and more complete descriptions as ioctls to the fd returned from a request for
>a
>VFIO_DEVICE_DMABUF_MGR_FD. Perhaps VFIO_DMABUF_MGR_QUERY_PLANE
>and VFIO_DMABUF_MGR_CREATE_DMABUF. I'm also not sure why we're using
>"vgpu" in the structure names here either, the ioctls aren't named after vgpus.
>Aren't these rather generic to graphics dmabufs, not specifically vgpus?
Make sense. I will change the names.
Thanks,
>
>Alex
>
>> +
>> /* API for Type1 VFIO IOMMU */
>>
>> /**
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 3/6] drm/i915/gvt: Frame buffer decoder support for GVT-g
>-Original Message-
>From: Zhang, Tina
>Sent: Wednesday, June 07, 2017 5:06 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>;
>alex.william...@redhat.com; kra...@redhat.com; ch...@chris-wilson.co.uk;
>intel-gfx@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>
>Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Subject: RE: [PATCH v7 3/6] drm/i915/gvt: Frame buffer decoder support for GVT-
>g
>
>
>
>> -Original Message-
>> From: intel-gvt-dev
>> [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On Behalf Of
>> Xiaoguang Chen
>> Sent: Wednesday, June 7, 2017 3:45 PM
>> To: alex.william...@redhat.com; kra...@redhat.com;
>> ch...@chris-wilson.co.uk; intel-gfx@lists.freedesktop.org;
>> linux-ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> <zhiyuan...@intel.com>; intel-gvt- d...@lists.freedesktop.org; Wang,
>> Zhi A <zhi.a.w...@intel.com>; Tian, Kevin <kevin.t...@intel.com>
>> Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>> Subject: [PATCH v7 3/6] 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.c...@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 | 439
>> ++
>> drivers/gpu/drm/i915/gvt/fb_decoder.h | 167 +
>> drivers/gpu/drm/i915/gvt/gvt.h| 1 +
>> include/uapi/drm/drm_fourcc.h | 6 +
>> 7 files changed, 618 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 000..0825949
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
>> @@ -0,0 +1,439 @@
>> +/*
>> + * 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,
Re: [Intel-gfx] [PATCH v6 6/6] drm/i915/gvt: Adding interface so user space can get the dma-buf
Hi, >-Original Message- >From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On >Behalf Of Gerd Hoffmann >Sent: Friday, June 02, 2017 11:24 PM >To: Alex Williamson <alex.william...@redhat.com>; Chen, Xiaoguang ><xiaoguang.c...@intel.com> >Cc: Tian, Kevin <kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux- >ker...@vger.kernel.org; zhen...@linux.intel.com; ch...@chris-wilson.co.uk; Lv, >Zhiyuan <zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi >A <zhi.a.w...@intel.com> >Subject: Re: [PATCH v6 6/6] drm/i915/gvt: Adding interface so user space can >get >the dma-buf > > Hi, > >> > When i915's dma-buf's release() callback is called it will try to >> > free the gem object associated with the dma-buf if its ref count is >> > 0. But in our case the ref count is 1 so no free callback is called >> > so we can not release allocations there. > >Why the ref count is one? The gem object is created by us while creating the dma-buf(the ref count of the gem object is initialized to 1). Later when user import the dma-buf the ref count of the gem object associate with the dma-buf will increased. When user finished using the dma-buf it will decrease the ref count. But the ref count of the gem object will become 1 when all the user finished using the dma-buf because we create the gem object(the test also showing this result). Typically user only export a dma-buf(no gem object yet) then when user import the dma-buf then a gem object will be created. But in our case we do not implement the dma-buf from scratch but calling the i915_gem_prime_export() where a gem object is an input parameter. Chenxg >Who holds a reference and why? >Maybe it should be the other way around, i.e. the dmabuf holds a reference on >the vgpu instance backing it, i.e. you can't delete the vgpu while dma-bufs >exist? > >> We cannot simply say that the user isn't allowed to release them in >> that order. > >Yep, not going to fly. Can happen even unintentionally because we can pass >around dmabufs to other processes. Example: qemu passes dmabuf to spice- >client, then qemu crashes. mgmt fd is closed before dmabuf fd then. The >kernel >must be able to handle that. > >cheers, > Gerd >___ >intel-gvt-dev mailing list >intel-gvt-...@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 6/6] drm/i915/gvt: Adding interface so user space can get the dma-buf
>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Friday, June 02, 2017 11:35 AM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: kra...@redhat.com; ch...@chris-wilson.co.uk; intel-
>g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>
>Subject: Re: [PATCH v6 6/6] drm/i915/gvt: Adding interface so user space can
>get
>the dma-buf
>
>On Fri, 2 Jun 2017 03:24:35 +
>"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
>
>> Hi Alex,
>>
>> >-Original Message-
>> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >Sent: Friday, June 02, 2017 2:08 AM
>> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>> >Cc: kra...@redhat.com; ch...@chris-wilson.co.uk; intel-
>> >g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>> >zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>;
>> >intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A
>> ><zhi.a.w...@intel.com>; Tian, Kevin <kevin.t...@intel.com>
>> >Subject: Re: [PATCH v6 6/6] drm/i915/gvt: Adding interface so user
>> >space can get the dma-buf
>> >
>> >On Sat, 27 May 2017 16:38:52 +0800
>> >Xiaoguang Chen <xiaoguang.c...@intel.com> wrote:
>> >
>> >> User space should create the management fd for the dma-buf operation
>> >> first.
>> >> Then user can query the plane information and create dma-buf if
>> >> necessary using the management fd.
>> >>
>> >> Signed-off-by: Xiaoguang Chen <xiaoguang.c...@intel.com>
>> >> ---
>> >> drivers/gpu/drm/i915/gvt/dmabuf.c | 12
>> >> drivers/gpu/drm/i915/gvt/dmabuf.h | 5 ++
>> >> drivers/gpu/drm/i915/gvt/gvt.c| 2 +
>> >> drivers/gpu/drm/i915/gvt/gvt.h| 5 ++
>> >> drivers/gpu/drm/i915/gvt/kvmgt.c | 144
>> >++
>> >> drivers/gpu/drm/i915/gvt/vgpu.c | 1 +
>> >> 6 files changed, 169 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
>> >> b/drivers/gpu/drm/i915/gvt/dmabuf.c
>> >> index c831e91..9759e9a 100644
>> >> --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
>> >> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
>> >> @@ -226,6 +226,7 @@ int intel_vgpu_create_dmabuf(struct intel_vgpu
>> >> *vgpu,
>> >void *args)
>> >> struct vfio_vgpu_dmabuf_info *gvt_dmabuf = args;
>> >> struct intel_vgpu_fb_info *fb_info;
>> >> int ret;
>> >> + struct intel_vgpu_dmabuf_obj *dmabuf_obj;
>> >>
>> >> ret = intel_vgpu_get_plane_info(dev, vgpu, _dmabuf->plane_info);
>> >> if (ret != 0)
>> >> @@ -263,6 +264,17 @@ int intel_vgpu_create_dmabuf(struct intel_vgpu
>> >> *vgpu,
>> >void *args)
>> >> gvt_vgpu_err("create dma-buf fd failed ret:%d\n", ret);
>> >> return ret;
>> >> }
>> >> + dmabuf_obj = kmalloc(sizeof(*dmabuf_obj), GFP_KERNEL);
>> >> + if (dmabuf_obj == NULL) {
>> >> + kfree(fb_info);
>> >> + i915_gem_object_put(obj);
>> >> + gvt_vgpu_err("alloc dmabuf_obj failed\n");
>> >> + return -ENOMEM;
>> >> + }
>> >> + dmabuf_obj->obj = obj;
>> >> + INIT_LIST_HEAD(_obj->list);
>> >> + list_add_tail(_obj->list, >dmabuf_obj_list_head);
>> >> +
>> >> gvt_dmabuf->fd = ret;
>> >>
>> >> return 0;
>> >> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.h
>> >> b/drivers/gpu/drm/i915/gvt/dmabuf.h
>> >> index 8be9979..cafa781 100644
>> >> --- a/drivers/gpu/drm/i915/gvt/dmabuf.h
>> >> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.h
>> >> @@ -31,6 +31,11 @@ struct intel_vgpu_fb_info {
>> >> uint32_t fb_size;
>> >> };
>> >>
>> >> +struct intel_vgpu_dmabuf_obj {
>> >> + struct drm_i915_gem_object *obj;
>> >> + struct list_head list;
>> >> +};
>> >> +
>> >> int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args);
>> >> int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu,
Re: [Intel-gfx] [PATCH v6 6/6] drm/i915/gvt: Adding interface so user space can get the dma-buf
Hi Alex,
>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Friday, June 02, 2017 2:08 AM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: kra...@redhat.com; ch...@chris-wilson.co.uk; intel-
>g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>
>Subject: Re: [PATCH v6 6/6] drm/i915/gvt: Adding interface so user space can
>get
>the dma-buf
>
>On Sat, 27 May 2017 16:38:52 +0800
>Xiaoguang Chen <xiaoguang.c...@intel.com> wrote:
>
>> User space should create the management fd for the dma-buf operation first.
>> Then user can query the plane information and create dma-buf if
>> necessary using the management fd.
>>
>> Signed-off-by: Xiaoguang Chen <xiaoguang.c...@intel.com>
>> ---
>> drivers/gpu/drm/i915/gvt/dmabuf.c | 12
>> drivers/gpu/drm/i915/gvt/dmabuf.h | 5 ++
>> drivers/gpu/drm/i915/gvt/gvt.c| 2 +
>> drivers/gpu/drm/i915/gvt/gvt.h| 5 ++
>> drivers/gpu/drm/i915/gvt/kvmgt.c | 144
>++
>> drivers/gpu/drm/i915/gvt/vgpu.c | 1 +
>> 6 files changed, 169 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
>> b/drivers/gpu/drm/i915/gvt/dmabuf.c
>> index c831e91..9759e9a 100644
>> --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
>> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
>> @@ -226,6 +226,7 @@ int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu,
>void *args)
>> struct vfio_vgpu_dmabuf_info *gvt_dmabuf = args;
>> struct intel_vgpu_fb_info *fb_info;
>> int ret;
>> +struct intel_vgpu_dmabuf_obj *dmabuf_obj;
>>
>> ret = intel_vgpu_get_plane_info(dev, vgpu, _dmabuf->plane_info);
>> if (ret != 0)
>> @@ -263,6 +264,17 @@ int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu,
>void *args)
>> gvt_vgpu_err("create dma-buf fd failed ret:%d\n", ret);
>> return ret;
>> }
>> +dmabuf_obj = kmalloc(sizeof(*dmabuf_obj), GFP_KERNEL);
>> +if (dmabuf_obj == NULL) {
>> +kfree(fb_info);
>> +i915_gem_object_put(obj);
>> +gvt_vgpu_err("alloc dmabuf_obj failed\n");
>> +return -ENOMEM;
>> +}
>> +dmabuf_obj->obj = obj;
>> +INIT_LIST_HEAD(_obj->list);
>> +list_add_tail(_obj->list, >dmabuf_obj_list_head);
>> +
>> gvt_dmabuf->fd = ret;
>>
>> return 0;
>> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.h
>> b/drivers/gpu/drm/i915/gvt/dmabuf.h
>> index 8be9979..cafa781 100644
>> --- a/drivers/gpu/drm/i915/gvt/dmabuf.h
>> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.h
>> @@ -31,6 +31,11 @@ struct intel_vgpu_fb_info {
>> uint32_t fb_size;
>> };
>>
>> +struct intel_vgpu_dmabuf_obj {
>> +struct drm_i915_gem_object *obj;
>> +struct list_head list;
>> +};
>> +
>> int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args); int
>> intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args);
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c
>> b/drivers/gpu/drm/i915/gvt/gvt.c index 2032917..dbc3f86 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.c
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
>> @@ -54,6 +54,8 @@ static const struct intel_gvt_ops intel_gvt_ops = {
>> .vgpu_reset = intel_gvt_reset_vgpu,
>> .vgpu_activate = intel_gvt_activate_vgpu,
>> .vgpu_deactivate = intel_gvt_deactivate_vgpu,
>> +.vgpu_query_plane = intel_vgpu_query_plane,
>> +.vgpu_create_dmabuf = intel_vgpu_create_dmabuf,
>> };
>>
>> /**
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h
>> b/drivers/gpu/drm/i915/gvt/gvt.h index 763a8c5..a855797 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.h
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>> @@ -185,8 +185,11 @@ struct intel_vgpu {
>> struct kvm *kvm;
>> struct work_struct release_work;
>> atomic_t released;
>> +struct vfio_device *vfio_device;
>> } vdev;
>> #endif
>> +int dmabuf_mgr_fd;
>> +struct list_head dmabuf_obj_list_head;
>> };
>>
>> struct intel_gvt_gm {
>> @@ -467,6 +470,8 @@ struct intel_gvt_ops {
>> void (*vgpu_reset)(struct intel_vgpu *);
>> void (*vgpu_activate)(struct intel_vgpu *);
>> voi
Re: [Intel-gfx] [PATCH v6 5/6] drm/i915/gvt: Dmabuf support for GVT-g
Hi Gerd,
>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Gerd Hoffmann
>Sent: Wednesday, May 31, 2017 8:05 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>;
>alex.william...@redhat.com; ch...@chris-wilson.co.uk; intel-
>g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>
>Subject: Re: [PATCH v6 5/6] drm/i915/gvt: Dmabuf support for GVT-g
>
>On Sat, 2017-05-27 at 16:38 +0800, Xiaoguang Chen wrote:
>> + if (plane_id == PLANE_PRIMARY) {
>
>Should be DRM_PLANE_TYPE_PRIMARY (likewise for the cursor).
OK. I will change in the next version.
>___
>intel-gvt-dev mailing list
>intel-gvt-...@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf operations
Hi Kirti,
>-Original Message-
>From: Kirti Wankhede [mailto:kwankh...@nvidia.com]
>Sent: Thursday, June 01, 2017 1:23 AM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Gerd Hoffmann
><kra...@redhat.com>; alex.william...@redhat.com; ch...@chris-wilson.co.uk;
>intel-gfx@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>
>Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf operations
>
>
>
>On 5/31/2017 11:48 AM, Chen, Xiaoguang wrote:
>> Hi,
>>
>>> -Original Message-
>>> From: Gerd Hoffmann [mailto:kra...@redhat.com]
>>> Sent: Monday, May 29, 2017 3:20 PM
>>> To: Chen, Xiaoguang <xiaoguang.c...@intel.com>;
>>> alex.william...@redhat.com; ch...@chris-wilson.co.uk; intel-
>>> g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>>> zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>;
>>> intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A
>>> <zhi.a.w...@intel.com>; Tian, Kevin <kevin.t...@intel.com>
>>> Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf
>>> operations
>>>
>>>> +struct vfio_vgpu_dmabuf_info {
>>>> + __u32 argsz;
>>>> + __u32 flags;
>>>> + struct vfio_vgpu_plane_info plane_info;
>>>> + __s32 fd;
>>>> + __u32 pad;
>>>> +};
>>>
>>> Hmm, now you have argsz and flags twice in vfio_vgpu_dmabuf_info ...
>>>
>>> I think we should have something like this:
>>>
>>> struct vfio_vgpu_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;
>>> };
>>>
>>> struct vfio_vgpu_query_plane {
>>> __u32 argsz;
>>> __u32 flags;
>>> struct vfio_vgpu_plane_info plane_info;
>>>__u32 plane_id;
>>>__u32 padding;
>>> };
>>>
>>> struct vfio_vgpu_create_dmabuf {
>>> __u32 argsz;
>>> __u32 flags;
>>> struct vfio_vgpu_plane_info plane_info;
>>>__u32 plane_id;
>>>__s32 fd;
>>> };
>> Good suggestion will apply in the next version.
>> Thanks for review :)
>>
>
>Can you define what are the expected values of 'flags' would be?
Flags is not used in this case. It is defined to follow the rules of vfio
ioctls.
>
>Thanks,
>Kirti
>
>> Chenxg.
>>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 0/6] drm/i915/gvt: Dma-buf support for GVT-g
Hi Gerd, I found this problem once I sent the patches :( I checked the uapi definitions and found it is usually called pad to do the aligning. So I changed the 'resv' to 'pad' in the patch but forgot to update it in the last patch and did not test after the "small" change. Next time I will test even when the change is very small. I will change this in the next version. Sorry for the mistake. >-Original Message- >From: Gerd Hoffmann [mailto:kra...@redhat.com] >Sent: Wednesday, May 31, 2017 4:59 PM >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>; >alex.william...@redhat.com; ch...@chris-wilson.co.uk; intel- >g...@lists.freedesktop.org; linux-ker...@vger.kernel.org; >zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt- >d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin ><kevin.t...@intel.com> >Subject: Re: [PATCH v6 0/6] drm/i915/gvt: Dma-buf support for GVT-g > >On Wed, 2017-05-31 at 02:29 +, Chen, Xiaoguang wrote: >> Hi Gerd, >> >> It is based on 4.12.0-rc1 > >Applies, good. >But then fails to build: > >error: ‘struct vfio_vgpu_dmabuf_info’ has no member named ‘resv’ > >gvt/kvmgt.c:611:11: note: in expansion of macro ‘offsetofend’ > minsz = offsetofend(struct vfio_vgpu_dmabuf_info, resv); > >/me wonders how this was tested ... > >cheers, > Gerd ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 2/6] drm/i915/gvt: OpRegion support for GVT-g
>-Original Message-
>From: Zhenyu Wang [mailto:zhen...@linux.intel.com]
>Sent: Wednesday, May 31, 2017 2:30 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: Tian, Kevin <kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; ch...@chris-wilson.co.uk; alex.william...@redhat.com;
>kra...@redhat.com; Niu, Bing <bing@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Lv, Zhiyuan
><zhiyuan...@intel.com>
>Subject: Re: [PATCH v6 2/6] drm/i915/gvt: OpRegion support for GVT-g
>
>On 2017.05.31 06:22:28 +, Chen, Xiaoguang wrote:
>> >> @@ -467,6 +555,15 @@ static int intel_vgpu_create(struct kobject
>> >> *kobj,
>> >struct mdev_device *mdev)
>> >> vgpu->vdev.mdev = mdev;
>> >> mdev_set_drvdata(mdev, vgpu);
>> >>
>> >> + ret = intel_vgpu_reg_init_opregion(vgpu);
>> >> + if (ret) {
>> >> + gvt_vgpu_err("create OpRegion failed\n");
>> >> + goto out;
>> >> + }
>> >
>> >Still need to handle error path for created vgpu.
>> Just checked the code, if initialize the opregion failed we should first
>> release
>vfio/mdev releated work(maybe call intel_vgpu_release function) and then
>destroy the vgpu. Will update in the next version.
>>
>
>Better to init opregion inside of create vgpu and do proper error handling
>there
>too.
Then we must add a new entry in intel_gvt_mpt interface something like
"create_opregion".
>
>--
>Open Source Technology Center, Intel ltd.
>
>$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 3/6] drm/i915/gvt: Frame buffer decoder support for GVT-g
>-Original Message-
>From: Zhenyu Wang [mailto:zhen...@linux.intel.com]
>Sent: Wednesday, May 31, 2017 1:12 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: alex.william...@redhat.com; kra...@redhat.com; ch...@chris-wilson.co.uk;
>intel-gfx@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>
>Subject: Re: [PATCH v6 3/6] drm/i915/gvt: Frame buffer decoder support for GVT-
>g
>
>On 2017.05.27 16:38:49 +0800, Xiaoguang Chen wrote:
>> decode frambuffer attributes of primary, cursor and sprite plane
>>
>> Signed-off-by: Xiaoguang Chen <xiaoguang.c...@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 | 479
>> ++
>> drivers/gpu/drm/i915/gvt/fb_decoder.h | 166
>> drivers/gpu/drm/i915/gvt/gvt.h| 1 +
>> 6 files changed, 651 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 000..d4404fd
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
>> @@ -0,0 +1,479 @@
>> +/*
>> + * 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
Re: [Intel-gfx] [PATCH v6 2/6] drm/i915/gvt: OpRegion support for GVT-g
Hi
>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Zhenyu Wang
>Sent: Wednesday, May 31, 2017 12:47 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: Tian, Kevin <kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; ch...@chris-wilson.co.uk;
>alex.william...@redhat.com; kra...@redhat.com; Niu, Bing
><bing@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
><zhi.a.w...@intel.com>; Lv, Zhiyuan <zhiyuan...@intel.com>
>Subject: Re: [PATCH v6 2/6] drm/i915/gvt: OpRegion support for GVT-g
>
>On 2017.05.27 16:38:48 +0800, Xiaoguang Chen wrote:
>> OpRegion is needed to support display related operation for intel
>> vgpu.
>>
>> A vfio device region is added to intel vgpu to deliver the host
>> OpRegion information to user space so user space can construct the
>> OpRegion for vgpu.
>>
>> Signed-off-by: Bing Niu <bing@intel.com>
>> Signed-off-by: Xiaoguang Chen <xiaoguang.c...@intel.com>
>> ---
>> drivers/gpu/drm/i915/gvt/kvmgt.c| 97
>+
>> drivers/gpu/drm/i915/gvt/opregion.c | 8 ++-
>> 2 files changed, 104 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 3c6a02b..389f072 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -53,6 +53,8 @@ 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, @@ -436,6 +438,92
>> @@ 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 intel_vgpu_reg_init_opregion(struct intel_vgpu *vgpu) {
>> +unsigned int addr;
>> +void *base;
>> +int ret;
>> +
>> +addr = vgpu->gvt->opregion.opregion_pa;
>> +if (!addr || !(~addr))
>> +return -ENODEV;
>> +
>> +base = memremap(addr, OPREGION_SIZE
Re: [Intel-gfx] [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf operations
Hi,
>-Original Message-
>From: Gerd Hoffmann [mailto:kra...@redhat.com]
>Sent: Monday, May 29, 2017 3:20 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>;
>alex.william...@redhat.com; ch...@chris-wilson.co.uk; intel-
>g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>
>Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf operations
>
>> +struct vfio_vgpu_dmabuf_info {
>> +__u32 argsz;
>> +__u32 flags;
>> +struct vfio_vgpu_plane_info plane_info;
>> +__s32 fd;
>> +__u32 pad;
>> +};
>
>Hmm, now you have argsz and flags twice in vfio_vgpu_dmabuf_info ...
>
>I think we should have something like this:
>
>struct vfio_vgpu_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;
>};
>
>struct vfio_vgpu_query_plane {
> __u32 argsz;
> __u32 flags;
> struct vfio_vgpu_plane_info plane_info;
>__u32 plane_id;
>__u32 padding;
>};
>
>struct vfio_vgpu_create_dmabuf {
> __u32 argsz;
> __u32 flags;
> struct vfio_vgpu_plane_info plane_info;
>__u32 plane_id;
>__s32 fd;
>};
Good suggestion will apply in the next version.
Thanks for review :)
Chenxg.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 0/6] drm/i915/gvt: Dma-buf support for GVT-g
Hi Gerd, It is based on 4.12.0-rc1 >-Original Message- >From: Gerd Hoffmann [mailto:kra...@redhat.com] >Sent: Tuesday, May 30, 2017 6:24 PM >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>; >alex.william...@redhat.com; ch...@chris-wilson.co.uk; intel- >g...@lists.freedesktop.org; linux-ker...@vger.kernel.org; >zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt- >d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin ><kevin.t...@intel.com> >Subject: Re: [PATCH v6 0/6] drm/i915/gvt: Dma-buf support for GVT-g > > Hi, > >> 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. > >Which branch is this based on? >Applying to gvt-stable-4.11 doesn't work. >Applying to drm-intel-next doesn't work either ... > >cheers, > Gerd ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf
Hi Alex,
>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Saturday, May 20, 2017 12:34 AM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: kra...@redhat.com; intel-gfx@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
><zhi.a.w...@intel.com>; Tian, Kevin <kevin.t...@intel.com>
>Subject: Re: [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can
>get
>the dma-buf
>
>On Thu, 18 May 2017 17:50:05 +0800
>Xiaoguang Chen <xiaoguang.c...@intel.com> wrote:
>
>> User space will try to create a management fd for the dma-buf operation.
>> Using this management fd user can query the plane information and
>> create a dma-buf fd if necessary.
>> GVT-g will handle the life cycle of the management fd and will align
>> the life cycle of the fd with the vfio device.
>> User space should handle the life cycle of the created dma-buf fd
>> close the dma-buf fd timely when finishing use.
>>
>> Signed-off-by: Xiaoguang Chen <xiaoguang.c...@intel.com>
>> ---
>> drivers/gpu/drm/i915/gvt/gvt.c | 2 +
>> drivers/gpu/drm/i915/gvt/gvt.h | 3 ++
>> drivers/gpu/drm/i915/gvt/kvmgt.c | 89
>
>> include/uapi/drm/i915_drm.h | 2 +
>> include/uapi/linux/vfio.h| 12 ++
>> 5 files changed, 108 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c
>> b/drivers/gpu/drm/i915/gvt/gvt.c index 2032917..48e04e6 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.c
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
>> @@ -54,6 +54,8 @@
>> .vgpu_reset = intel_gvt_reset_vgpu,
>> .vgpu_activate = intel_gvt_activate_vgpu,
>> .vgpu_deactivate = intel_gvt_deactivate_vgpu,
>> +.vgpu_query_dmabuf = intel_vgpu_query_dmabuf,
>> +.vgpu_create_dmabuf = intel_vgpu_create_dmabuf,
>> };
>>
>> /**
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h
>> b/drivers/gpu/drm/i915/gvt/gvt.h index a553120..b7fdfd5 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.h
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>> @@ -185,6 +185,7 @@ struct intel_vgpu {
>> struct kvm *kvm;
>> struct work_struct release_work;
>> atomic_t released;
>> +struct vfio_device *vfio_device;
>> } vdev;
>> #endif
>> struct intel_vgpu_plane_info *plane_info; @@ -469,6 +470,8 @@ struct
>> intel_gvt_ops {
>> void (*vgpu_reset)(struct intel_vgpu *);
>> void (*vgpu_activate)(struct intel_vgpu *);
>> void (*vgpu_deactivate)(struct intel_vgpu *);
>> +int (*vgpu_query_dmabuf)(struct intel_vgpu *, void *);
>> +int (*vgpu_create_dmabuf)(struct intel_vgpu *, void *);
>> };
>>
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 389f072..9a663df 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -41,6 +41,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "i915_drv.h"
>> #include "gvt.h"
>> @@ -524,6 +525,66 @@ static int intel_vgpu_reg_init_opregion(struct
>intel_vgpu *vgpu)
>> return ret;
>> }
>>
>> +static int intel_vgpu_dmabuf_mgr_fd_mmap(struct file *file,
>> +struct vm_area_struct *vma)
>> +{
>> +return -EPERM;
>> +}
>> +
>> +static int intel_vgpu_dmabuf_mgr_fd_release(struct inode *inode,
>> +struct file *filp)
>> +{
>> +struct intel_vgpu *vgpu = filp->private_data;
>> +
>> +if (vgpu->vdev.vfio_device != NULL)
>> +vfio_device_put(vgpu->vdev.vfio_device);
>> +else
>> +gvt_vgpu_err("intel vgpu dmabuf mgr fd is in a wrong state\n");
>
>You could do:
>
>if (WARN_ON(!vgpu->vdev.vfio_device))
> return -EINVAL;
OK.
>
>> +
>> +return 0;
>> +}
>> +
>> +static long intel_vgpu_dmabuf_mgr_fd_ioctl(struct file *filp,
>> +unsigned int ioctl, unsigned long arg) {
>> +struct intel_vgpu *vgpu = filp->private_data;
>> +int minsz;
>> +struct intel_vgpu_dmabuf dmabuf;
>> +int ret;
>> +struct fd f;
>> +
>> +minsz = offsetofend(struct intel_vgpu_dmabuf, tiled);
>> +if (copy_from_user(, (void __user *)arg, minsz))
>> +return -EFA
Re: [Intel-gfx] [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
Hi min,
>-Original Message-
>From: He, Min
>Sent: Thursday, May 18, 2017 11:44 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>;
>alex.william...@redhat.com; kra...@redhat.com; intel-
>g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>
>Cc: Niu, Bing <bing@intel.com>; Chen, Xiaoguang
><xiaoguang.c...@intel.com>
>Subject: RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
>
>Xiaoguang,
>I have some comments inline. Thanks.
>
>> -Original Message-
>> From: intel-gvt-dev
>> [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On Behalf Of
>> Xiaoguang Chen
>> Sent: Thursday, May 18, 2017 2:50 AM
>> To: alex.william...@redhat.com; kra...@redhat.com; intel-
>> g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
>> zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>;
>> intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A
>> <zhi.a.w...@intel.com>; Tian, Kevin <kevin.t...@intel.com>
>> Cc: Niu, Bing <bing@intel.com>; Chen, Xiaoguang
>> <xiaoguang.c...@intel.com>
>> Subject: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
>>
>> OpRegion is needed to support display related operation for intel
>> vgpu.
>>
>> A vfio device region is added to intel vgpu to deliver the host
>> OpRegion information to user space so user space can construct the
>> OpRegion for vgpu.
>>
>> Signed-off-by: Bing Niu <bing@intel.com>
>> Signed-off-by: Xiaoguang Chen <xiaoguang.c...@intel.com>
>> ---
>> drivers/gpu/drm/i915/gvt/kvmgt.c| 97
>> +
>> drivers/gpu/drm/i915/gvt/opregion.c | 12 -
>> 2 files changed, 107 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 3c6a02b..389f072 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -53,6 +53,8 @@
>> #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, @@ -436,6 +438,92
>> @@ 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->
Re: [Intel-gfx] [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
Hi Alex and Gerd, >-Original Message- >From: Alex Williamson [mailto:alex.william...@redhat.com] >Sent: Saturday, May 13, 2017 12:38 AM >To: Gerd Hoffmann <kra...@redhat.com> >Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Tian, Kevin ><kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux- >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan ><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A ><zhi.a.w...@intel.com> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf > >On Fri, 12 May 2017 11:12:05 +0200 >Gerd Hoffmann <kra...@redhat.com> wrote: > >> Hi, >> >> > If the contents of the framebuffer change or if the parameters of >> > the framebuffer change? I can't image that creating a new dmabuf fd >> > for every visual change within the framebuffer would be efficient, >> > but I don't have any concept of what a dmabuf actually does. >> >> Ok, some background: >> >> The drm subsystem has the concept of planes. The most important plane >> is the primary framebuffer (i.e. what gets scanned out to the physical >> display). The cursor is a plane too, and there can be additional >> overlay planes for stuff like video playback. >> >> Typically there are multiple planes in a system and only one of them >> gets scanned out to the crtc, i.e. the fbdev emulation creates one >> plane for the framebuffer console. The X-Server creates a plane too, >> and when you switch between X-Server and framebuffer console via >> ctrl-alt-fn the intel driver just reprograms the encoder to scan out >> the one or the other plane to the crtc. >> >> The dma-buf handed out by gvt is a reference to a plane. I think on >> the host side gvt can see only the active plane (from encoder/crtc >> register >> programming) not the inactive ones. >> >> The dma-buf can be imported as opengl texture and then be used to >> render the guest display to a host window. I think it is even >> possible to use the dma-buf as plane in the host drm driver and scan >> it out directly to a physical display. The actual framebuffer content >> stays in gpu memory all the time, the cpu never has to touch it. >> >> It is possible to cache the dma-buf handles, i.e. when the guest boots >> you'll get the first for the fbcon plane, when the x-server starts the >> second for the x-server framebuffer, and when the user switches to the >> text console via ctrl-alt-fn you can re-use the fbcon dma-buf you >> already have. >> >> The caching becomes more important for good performance when the guest >> uses pageflipping (wayland does): define two planes, render into one >> while displaying the other, then flip the two for a atomic display >> update. >> >> The caching also makes it a bit difficult to create a good interface. >> So, the current patch set creates: >> >> (a) A way to query the active planes (ioctl >> INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series). >> (b) A way to create a dma-buf for the active plane (ioctl >> INTEL_VGPU_GENERATE_DMABUF). >> >> Typical userspace workflow is to first query the plane, then check if >> it already has a dma-buf for it, and if not create one. > >Thank you! This is immensely helpful! > >> > What changes to the framebuffer require a new dmabuf fd? Shouldn't >> > the user query the parameters of the framebuffer through a dmabuf fd >> > and shouldn't the dmabuf fd have some signaling mechanism to the >> > user (eventfd perhaps) to notify the user to re-evaluate the parameters? >> >> dma-bufs don't support that, they are really just a handle to a piece >> of memory, all metadata (format, size) most be communicated by other means. >> >> > Otherwise are you imagining that the user polls the vfio region? >> >> Hmm, notification support would probably a good reason to have a >> separate file handle to manage the dma-bufs (instead of using >> driver-specific ioctls on the vfio fd), because the driver could also >> use the management fd for notifications then. > >I like this idea of a separate control fd for dmabufs, it provides not only a >central >management point, but also a nice abstraction for the vfio device specific >interface. We potentially only need a single >VFIO_DEVICE_GET_DMABUF_MGR_FD() ioctl to get a dmabuf management fd >(perhaps with a type parameter, ex. GFX) where maybe we could have vfio-core >incorporate this reference into the group lifecycle, so the vendor driver only >needs t
Re: [Intel-gfx] [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
Hi Gerd, >-Original Message- >From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On >Behalf Of Gerd Hoffmann >Sent: Thursday, May 11, 2017 9:28 PM >To: Chen, Xiaoguang <xiaoguang.c...@intel.com> >Cc: Tian, Kevin <kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux- >ker...@vger.kernel.org; zhen...@linux.intel.com; Alex Williamson ><alex.william...@redhat.com>; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt- >d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf > > Hi, > >> While read the framebuffer region we have to tell the vendor driver which >framebuffer we want to read? There are two framebuffers now in KVMGT that is >primary and cursor. >> There are two methods to implement this: >> 1) write the plane id first and then read the framebuffer. >> 2) create 2 vfio regions one for primary and one for cursor. > >(3) Place information for both planes into one vfio region. >Which allows to fetch both with a single read() syscall. That works too. Then using the ioctl to get the dmabuf fd if needed. And plane id can be ioctl's parameter. How about method 2 primary plane and cursor plane are different and should generate different dmabuf for each of them. > >The question is how you'll get the file descriptor then. If the ioctl returns >the >dma-buf fd only you have a racy interface: Things can change between >read(vfio- >region) and ioctl(need-dmabuf-fd). You are right. So when creating the dmabuf we may have to decode the framebuffer and create the dmabuf using the latest framebuffer information and we must return the framebuffer information together with the dmabuf fd. In the current implementation I saved the framebuffer information while user querying the framebuffer and generate the dmabuf using the saved information no error found yet but in theory there are sync problems. > >ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix the race, >but then it is easier to go with ioctl only interface (simliar to the orginal >one from >dec last year) I think. Yes. ioctl works for it. But based on the mail last week. If I understand correctly Alex hope to query the framebuffer information by reading the vfio device region and then get the dmabuf fd using ioctl. > >cheers, > Gerd > >___ >intel-gvt-dev mailing list >intel-gvt-...@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>-Original Message- >From: Alex Williamson [mailto:alex.william...@redhat.com] >Sent: Friday, May 12, 2017 10:58 AM >To: Chen, Xiaoguang <xiaoguang.c...@intel.com> >Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin <kevin.t...@intel.com>; >intel-gfx@lists.freedesktop.org; linux-ker...@vger.kernel.org; >zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt- >d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf > >On Fri, 12 May 2017 02:12:10 + >"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote: > >> Hi Alex and Gerd, >> >> >-Original Message- >> >From: intel-gvt-dev >> >[mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On Behalf Of >> >Alex Williamson >> >Sent: Thursday, May 11, 2017 11:45 PM >> >To: Gerd Hoffmann <kra...@redhat.com> >> >Cc: Tian, Kevin <kevin.t...@intel.com>; >> >intel-gfx@lists.freedesktop.org; linux- ker...@vger.kernel.org; >> >zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; Chen, >> >Xiaoguang <xiaoguang.c...@intel.com>; intel- >> >gvt-...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the >> >dmabuf >> > >> >On Thu, 11 May 2017 15:27:53 +0200 >> >Gerd Hoffmann <kra...@redhat.com> wrote: >> > >> >> Hi, >> >> >> >> > While read the framebuffer region we have to tell the vendor >> >> > driver which >> >framebuffer we want to read? There are two framebuffers now in KVMGT >> >that is primary and cursor. >> >> > There are two methods to implement this: >> >> > 1) write the plane id first and then read the framebuffer. >> >> > 2) create 2 vfio regions one for primary and one for cursor. >> >> >> >> (3) Place information for both planes into one vfio region. >> >> Which allows to fetch both with a single read() syscall. >> >> >> >> The question is how you'll get the file descriptor then. If the >> >> ioctl returns the dma-buf fd only you have a racy interface: >> >> Things can change between read(vfio-region) and ioctl(need-dmabuf-fd). >> >> >> >> ioctl(need-dma-buf) could return both dmabuf fd and plane info to >> >> fix the race, but then it is easier to go with ioctl only interface >> >> (simliar to the orginal one from dec last year) I think. >> > >> >If the dmabuf fd is provided by a separate mdev vendor driver >> >specific ioctl, I don't see how vfio regions should be involved. Selecting >> >which >framebuffer >> >should be an ioctl parameter. >> Based on your last mail. I think the implementation looks like this: >> 1) user query the framebuffer information by reading the vfio region. >> 2) if the framebuffer changed(such as framebuffer's graphics address changed, >size changed etc) we will need to create a new dmabuf fd. >> 3) create a new dmabuf fd using vfio device specific ioctl. >> >> >What sort of information needs to be conveyed >> >about each plane? >> Only plane id is needed. >> >> >Is it static information or something that needs to be read >> >repeatedly? >> It is static information. For our case plane id 1 represent primary plane >> and 3 for >cursor plane. 2 means sprite plane which will not be used in our case. >> >> >Do we need it before we get the dmabuf fd or can it be an ioctl on >> >the dmabuf fd? >> We need it while query the framebuffer. In kernel we need the plane id to >decide which plane we should decode. >> Below is my current implementation: >> 1) user first query the framebuffer(primary or cursor) and kernel decode the >framebuffer and return the framebuffer information to user and also save a copy >in kernel. >> 2) user compared the framebuffer and if the framebuffer changed creating a >new dmabuf fd. > >If the contents of the framebuffer change or if the parameters of the >framebuffer >change? If the parameters of the framebuffer change we need to create new dmabuf. >I can't image that creating a new dmabuf fd for every visual change >within the framebuffer would be efficient, but I don't have any concept of >what a >dmabuf actually does. > >> 3) kernel create a new dmabuf fd based on saved framebuffer information. >> >> So we need plane id in step 1. &g
Re: [Intel-gfx] [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
Hi Alex and Gerd, >-Original Message- >From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On >Behalf Of Alex Williamson >Sent: Thursday, May 11, 2017 11:45 PM >To: Gerd Hoffmann <kra...@redhat.com> >Cc: Tian, Kevin <kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux- >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan ><zhiyuan...@intel.com>; Chen, Xiaoguang <xiaoguang.c...@intel.com>; intel- >gvt-...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf > >On Thu, 11 May 2017 15:27:53 +0200 >Gerd Hoffmann <kra...@redhat.com> wrote: > >> Hi, >> >> > While read the framebuffer region we have to tell the vendor driver which >framebuffer we want to read? There are two framebuffers now in KVMGT that is >primary and cursor. >> > There are two methods to implement this: >> > 1) write the plane id first and then read the framebuffer. >> > 2) create 2 vfio regions one for primary and one for cursor. >> >> (3) Place information for both planes into one vfio region. >> Which allows to fetch both with a single read() syscall. >> >> The question is how you'll get the file descriptor then. If the ioctl >> returns the dma-buf fd only you have a racy interface: Things can >> change between read(vfio-region) and ioctl(need-dmabuf-fd). >> >> ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix >> the race, but then it is easier to go with ioctl only interface >> (simliar to the orginal one from dec last year) I think. > >If the dmabuf fd is provided by a separate mdev vendor driver specific ioctl, I >don't see how vfio regions should be involved. Selecting which framebuffer >should be an ioctl parameter. Based on your last mail. I think the implementation looks like this: 1) user query the framebuffer information by reading the vfio region. 2) if the framebuffer changed(such as framebuffer's graphics address changed, size changed etc) we will need to create a new dmabuf fd. 3) create a new dmabuf fd using vfio device specific ioctl. >What sort of information needs to be conveyed >about each plane? Only plane id is needed. >Is it static information or something that needs to be read >repeatedly? It is static information. For our case plane id 1 represent primary plane and 3 for cursor plane. 2 means sprite plane which will not be used in our case. >Do we need it before we get the dmabuf fd or can it be an ioctl on >the dmabuf fd? We need it while query the framebuffer. In kernel we need the plane id to decide which plane we should decode. Below is my current implementation: 1) user first query the framebuffer(primary or cursor) and kernel decode the framebuffer and return the framebuffer information to user and also save a copy in kernel. 2) user compared the framebuffer and if the framebuffer changed creating a new dmabuf fd. 3) kernel create a new dmabuf fd based on saved framebuffer information. So we need plane id in step 1. In step 3 we create a dmabuf fd only using saved framebuffer information(no other information is needed). Chenxg. >Thanks, > >Alex >___ >intel-gvt-dev mailing list >intel-gvt-...@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
Hi Alex, >-Original Message- >From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On >Behalf Of Alex Williamson >Sent: Friday, May 05, 2017 11:11 PM >To: Gerd Hoffmann <kra...@redhat.com> >Cc: Tian, Kevin <kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux- >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan ><zhiyuan...@intel.com>; Chen, Xiaoguang <xiaoguang.c...@intel.com>; intel- >gvt-...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf > >On Fri, 05 May 2017 08:55:31 +0200 >Gerd Hoffmann <kra...@redhat.com> wrote: > >> Hi, >> >> > > >>Hmm, that looks like a rather strange way to return a file descriptor. >> > > >> >> > > >>What is the reason to not use ioctls on the vfio file handle, like >> > > >>older version of these patches did? >> > > >If I understood correctly that Alex prefer not to change the >> > > >ioctls on the vfio file handle like the old version. >> > > >So I used this way the smallest change to general vfio framework >> > > >only adding a subregion definition. >> > >> > I think I was hoping we could avoid a separate file descriptor >> > altogether and use a vfio region instead. >> >> What exactly did you have in mind? Put the framebuffer information >> (struct intel_vgpu_dmabuf) into the vfio region, then access it using >> read/write/mmap? > >Yeah, that was my hope. Adding a new file descriptor means we have one more >reference floating around complicating the life cycle of the device, group, and >container. Furthermore this one is really only visible to the mdev vendor >driver, >so we can't rely on vfio-core, the vendor driver will need to consider the >reference when releasing the device. While read the framebuffer region we have to tell the vendor driver which framebuffer we want to read? There are two framebuffers now in KVMGT that is primary and cursor. There are two methods to implement this: 1) write the plane id first and then read the framebuffer. 2) create 2 vfio regions one for primary and one for cursor. Which method do you prefer? Or do you have other idea to handle this problem? chenxg ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 5/6] drm/i915/gvt: dmabuf support for GVT-g
Hi Chis, do you have any comments for this problem?
>> +static struct sg_table *
>> +intel_vgpu_create_sg_pages(struct drm_device *dev, u32 start, u32
>> +num_pages) {
>> +struct drm_i915_private *dev_priv = dev->dev_private;
>> +struct sg_table *st;
>> +struct scatterlist *sg;
>> +int i;
>> +gen8_pte_t __iomem *gtt_entries;
>> +
>> +st = kmalloc(sizeof(*st), GFP_KERNEL);
>> +if (st == NULL)
>> +return NULL;
>> +
>> +if (sg_alloc_table(st, num_pages, GFP_KERNEL)) {
>> +kfree(st);
>> +return NULL;
>> +}
>> +
>> +gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
>> +(start >> PAGE_SHIFT);
>> +for_each_sg(st->sgl, sg, num_pages, i) {
>> +sg->offset = 0;
>> +sg->length = PAGE_SIZE;
>> +sg_dma_address(sg) =
>> +GEN8_DECODE_PTE(readq(_entries[i]));
>> +sg_dma_len(sg) = PAGE_SIZE;
>> +}
>
>This should be get_pages.
The reason that I did not alloc pages to the gem obj in get_pages callback is
that there may be sync problems:
1) decode the vgpu's current frambuffer.
2) save the decoded information(we will use size and address of the framebuffer
later)
3) create a gem obj the size of the gem obj is the above framebuffer size.
4) associate the gem obj to a dmabuf and export the dmabuf.
..
5) while user access the gem obj get_pages callback of the gem obj will be
called to alloc pages for the gem obj.
6) use saved frambuffer's address to look up the gtt table to get the pages of
the frambuffer. Assign the pages to gem obj.
There may be interval between step 4 and step 5. The framebuffer may have
changed in step 5 but we are still using the old framebuffer's address(decoded
in step 1) in step 5.
>
>> +return st;
>> +}
>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Chen, Xiaoguang
>Sent: Tuesday, May 02, 2017 3:40 PM
>To: Chris Wilson <ch...@chris-wilson.co.uk>
>Cc: Tian, Kevin <kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; alex.william...@redhat.com;
>kra...@redhat.com; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
><zhi.a.w...@intel.com>; Lv, Zhiyuan <zhiyuan...@intel.com>
>Subject: RE: [Intel-gfx] [RFC PATCH 5/6] drm/i915/gvt: dmabuf support for GVT-g
>
>
>
>>-Original Message-
>>From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
>>Sent: Friday, April 28, 2017 6:09 PM
>>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>>Cc: kra...@redhat.com; alex.william...@redhat.com; intel-
>>g...@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org; Wang,
>>Zhi A <zhi.a.w...@intel.com>; zhen...@linux.intel.com; linux-
>>ker...@vger.kernel.org; Lv, Zhiyuan <zhiyuan...@intel.com>; Tian, Kevin
>><kevin.t...@intel.com>
>>Subject: Re: [Intel-gfx] [RFC PATCH 5/6] drm/i915/gvt: dmabuf support
>>for GVT-g
>>
>>On Fri, Apr 28, 2017 at 05:35:29PM +0800, Xiaoguang Chen wrote:
>>> 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). 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.c...@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gvt/Makefile | 2 +-
>>> drivers/gpu/drm/i915/gvt/dmabuf.c | 268
>>> ++
>>> drivers/gpu/drm/i915/gvt/dmabuf.h | 50 +++
>>
Re: [Intel-gfx] [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
Hi Alex, do you have any comments for this interface?
>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Chen, Xiaoguang
>Sent: Wednesday, May 03, 2017 9:39 AM
>To: Gerd Hoffmann <kra...@redhat.com>
>Cc: Tian, Kevin <kevin.t...@intel.com>; intel-gfx@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; alex.william...@redhat.com;
>Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang,
>Zhi A <zhi.a.w...@intel.com>
>Subject: RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>
>
>>-Original Message-
>>From: Gerd Hoffmann [mailto:kra...@redhat.com]
>>Sent: Tuesday, May 02, 2017 5:51 PM
>>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>>Cc: alex.william...@redhat.com; intel-gfx@lists.freedesktop.org;
>>intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A
>><zhi.a.w...@intel.com>; zhen...@linux.intel.com;
>>linux-ker...@vger.kernel.org; Lv, Zhiyuan <zhiyuan...@intel.com>; Tian,
>>Kevin <kevin.t...@intel.com>
>>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>>dmabuf
>>
>>On Fr, 2017-04-28 at 17:35 +0800, Xiaoguang Chen wrote:
>>> +static size_t intel_vgpu_reg_rw_gvtg(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;
>>> + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>>> + int fd;
>>> +
>>> + if (pos >= vgpu->vdev.region[i].size || iswrite) {
>>> + gvt_vgpu_err("invalid op or offset for Intel vgpu fd
>>> region\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + fd = anon_inode_getfd("gvtg", _vgpu_gvtg_ops, vgpu,
>>> + O_RDWR | O_CLOEXEC);
>>> + if (fd < 0) {
>>> + gvt_vgpu_err("create intel vgpu fd failed:%d\n", fd);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
>>> + memcpy(buf, , count);
>>> +
>>> + return count;
>>> +}
>>
>>Hmm, that looks like a rather strange way to return a file descriptor.
>>
>>What is the reason to not use ioctls on the vfio file handle, like
>>older version of these patches did?
>If I understood correctly that Alex prefer not to change the ioctls on the
>vfio file
>handle like the old version.
>So I used this way the smallest change to general vfio framework only adding a
>subregion definition.
>
>>
>>cheers,
>> Gerd
>
>___
>intel-gvt-dev mailing list
>intel-gvt-...@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>-Original Message-
>From: Gerd Hoffmann [mailto:kra...@redhat.com]
>Sent: Tuesday, May 02, 2017 5:51 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: alex.william...@redhat.com; intel-gfx@lists.freedesktop.org; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>;
>zhen...@linux.intel.com; linux-ker...@vger.kernel.org; Lv, Zhiyuan
><zhiyuan...@intel.com>; Tian, Kevin <kevin.t...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Fr, 2017-04-28 at 17:35 +0800, Xiaoguang Chen wrote:
>> +static size_t intel_vgpu_reg_rw_gvtg(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;
>> + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>> + int fd;
>> +
>> + if (pos >= vgpu->vdev.region[i].size || iswrite) {
>> + gvt_vgpu_err("invalid op or offset for Intel vgpu fd
>> region\n");
>> + return -EINVAL;
>> + }
>> +
>> + fd = anon_inode_getfd("gvtg", _vgpu_gvtg_ops, vgpu,
>> + O_RDWR | O_CLOEXEC);
>> + if (fd < 0) {
>> + gvt_vgpu_err("create intel vgpu fd failed:%d\n", fd);
>> + return -EINVAL;
>> + }
>> +
>> + count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
>> + memcpy(buf, , count);
>> +
>> + return count;
>> +}
>
>Hmm, that looks like a rather strange way to return a file descriptor.
>
>What is the reason to not use ioctls on the vfio file handle, like older
>version of
>these patches did?
If I understood correctly that Alex prefer not to change the ioctls on the vfio
file handle like the old version.
So I used this way the smallest change to general vfio framework only adding a
subregion definition.
>
>cheers,
> Gerd
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 5/6] drm/i915/gvt: dmabuf support for GVT-g
>-Original Message- >From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] >Sent: Friday, April 28, 2017 6:09 PM >To: Chen, Xiaoguang <xiaoguang.c...@intel.com> >Cc: kra...@redhat.com; alex.william...@redhat.com; intel- >g...@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A ><zhi.a.w...@intel.com>; zhen...@linux.intel.com; linux- >ker...@vger.kernel.org; Lv, Zhiyuan <zhiyuan...@intel.com>; Tian, Kevin ><kevin.t...@intel.com> >Subject: Re: [Intel-gfx] [RFC PATCH 5/6] drm/i915/gvt: dmabuf support for GVT-g > >On Fri, Apr 28, 2017 at 05:35:29PM +0800, Xiaoguang Chen wrote: >> 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). 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.c...@intel.com> >> --- >> drivers/gpu/drm/i915/gvt/Makefile | 2 +- >> drivers/gpu/drm/i915/gvt/dmabuf.c | 268 >> ++ >> drivers/gpu/drm/i915/gvt/dmabuf.h | 50 +++ >> drivers/gpu/drm/i915/gvt/gvt.h| 1 + >> 4 files changed, 320 insertions(+), 1 deletion(-) 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 000..d776dfa >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c >> @@ -0,0 +1,268 @@ >> +/* >> + * 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 L
