* [PATCH v2 1/5] drm/i915/gvt: Extend the GVT-g architecture to support vfio device region
2017-05-18 9:50 [PATCH v2 0/5] drm/i915/gvt: Dma-buf support for GVT-g Xiaoguang Chen
@ 2017-05-18 9:50 ` Xiaoguang Chen
2017-05-18 9:50 ` [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g Xiaoguang Chen
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Xiaoguang Chen @ 2017-05-18 9:50 UTC (permalink / raw)
To: alex.williamson, kraxel, intel-gfx, linux-kernel, zhenyuw,
zhiyuan.lv, intel-gvt-dev, zhi.a.wang, kevin.tian
Cc: Xiaoguang Chen
Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 1ae0b40..3c6a02b 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -53,11 +53,21 @@
#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)
+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 {
@@ -642,7 +652,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;
}
@@ -676,8 +686,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;
@@ -940,7 +953,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) ?
@@ -1061,6 +1075,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;
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
2017-05-18 9:50 [PATCH v2 0/5] drm/i915/gvt: Dma-buf support for GVT-g Xiaoguang Chen
2017-05-18 9:50 ` [PATCH v2 1/5] drm/i915/gvt: Extend the GVT-g architecture to support vfio device region Xiaoguang Chen
@ 2017-05-18 9:50 ` Xiaoguang Chen
2017-05-18 15:43 ` He, Min
2017-05-18 9:50 ` [PATCH v2 3/5] drm/i915/gvt: Frame buffer decoder " Xiaoguang Chen
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Xiaoguang Chen @ 2017-05-18 9:50 UTC (permalink / raw)
To: alex.williamson, kraxel, intel-gfx, linux-kernel, zhenyuw,
zhiyuan.lv, intel-gvt-dev, zhi.a.wang, kevin.tian
Cc: Xiaoguang Chen, Bing Niu
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.niu@intel.com>
Signed-off-by: Xiaoguang Chen <xiaoguang.chen@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->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, 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;
+ }
+
+ return ret;
+}
+
static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
{
struct intel_vgpu *vgpu = NULL;
@@ -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;
+ }
+
+ gvt_dbg_core("create OpRegion succeeded for mdev:%s\n",
+ dev_name(mdev_dev(mdev)));
+
gvt_dbg_core("intel_vgpu_create succeeded for mdev: %s\n",
dev_name(mdev_dev(mdev)));
ret = 0;
diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c
index 3117991..99591bc 100644
--- a/drivers/gpu/drm/i915/gvt/opregion.c
+++ b/drivers/gpu/drm/i915/gvt/opregion.c
@@ -283,14 +283,22 @@ int intel_vgpu_emulate_opregion_request(struct intel_vgpu *vgpu, u32 swsci)
{
u32 *scic, *parm;
u32 func, subfunc;
+ u32 scic_val, parm_val;
+ u32 gpa;
- scic = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_SCIC;
- parm = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_PARM;
+ memcpy(&gpa, vgpu_cfg_space(vgpu) + INTEL_GVT_PCI_OPREGION,
+ sizeof(gpa));
+ intel_gvt_hypervisor_read_gpa(vgpu, gpa + INTEL_GVT_OPREGION_SCIC,
+ &scic_val, sizeof(scic_val));
+ intel_gvt_hypervisor_read_gpa(vgpu, gpa + INTEL_GVT_OPREGION_PARM,
+ &parm_val, sizeof(parm_val));
if (!(swsci & SWSCI_SCI_SELECT)) {
gvt_vgpu_err("requesting SMI service\n");
return 0;
}
+ scic = &scic_val;
+ parm = &parm_val;
/* ignore non 0->1 trasitions */
if ((vgpu_cfg_space(vgpu)[INTEL_GVT_PCI_SWSCI]
& SWSCI_SCI_TRIGGER) ||
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
2017-05-18 9:50 ` [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g Xiaoguang Chen
@ 2017-05-18 15:43 ` He, Min
2017-05-19 2:20 ` Chen, Xiaoguang
0 siblings, 1 reply; 13+ messages in thread
From: He, Min @ 2017-05-18 15:43 UTC (permalink / raw)
To: Chen, Xiaoguang, alex.williamson@redhat.com, kraxel@redhat.com,
intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
zhenyuw@linux.intel.com, Lv, Zhiyuan,
intel-gvt-dev@lists.freedesktop.org, Wang, Zhi A, Tian, Kevin
Cc: Niu, Bing, Chen, Xiaoguang
Xiaoguang,
I have some comments inline. Thanks.
> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Xiaoguang Chen
> Sent: Thursday, May 18, 2017 2:50 AM
> To: alex.williamson@redhat.com; kraxel@redhat.com; intel-
> gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>; intel-gvt-
> dev@lists.freedesktop.org; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, Kevin
> <kevin.tian@intel.com>
> Cc: Niu, Bing <bing.niu@intel.com>; Chen, Xiaoguang
> <xiaoguang.chen@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.niu@intel.com>
> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@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->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, 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);
In current GVT code, we already have a function init_vgpu_opregion,
which will copy the physical opregion into a per vgpu structure:
vgpu_opregion(vgpu)->va, why don’t you reuse this function and set the
vgpu_opregion(vgpu)->va to the intel_vgpu_register_reg function?
> + if (ret) {
> + memunmap(base);
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
> {
> struct intel_vgpu *vgpu = NULL;
> @@ -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;
> + }
> +
> + gvt_dbg_core("create OpRegion succeeded for mdev:%s\n",
> + dev_name(mdev_dev(mdev)));
> +
> gvt_dbg_core("intel_vgpu_create succeeded for mdev: %s\n",
> dev_name(mdev_dev(mdev)));
> ret = 0;
> diff --git a/drivers/gpu/drm/i915/gvt/opregion.c
> b/drivers/gpu/drm/i915/gvt/opregion.c
> index 3117991..99591bc 100644
> --- a/drivers/gpu/drm/i915/gvt/opregion.c
> +++ b/drivers/gpu/drm/i915/gvt/opregion.c
> @@ -283,14 +283,22 @@ int intel_vgpu_emulate_opregion_request(struct
> intel_vgpu *vgpu, u32 swsci)
> {
> u32 *scic, *parm;
> u32 func, subfunc;
> + u32 scic_val, parm_val;
> + u32 gpa;
>
> - scic = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_SCIC;
> - parm = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_PARM;
> + memcpy(&gpa, vgpu_cfg_space(vgpu) + INTEL_GVT_PCI_OPREGION,
> + sizeof(gpa));
> + intel_gvt_hypervisor_read_gpa(vgpu, gpa +
> INTEL_GVT_OPREGION_SCIC,
> + &scic_val, sizeof(scic_val));
> + intel_gvt_hypervisor_read_gpa(vgpu, gpa +
> INTEL_GVT_OPREGION_PARM,
> + &parm_val, sizeof(parm_val));
As I said, you can call init_vgpu_opregion(), then vgpu_opregion(vgpu)->va will
be valid and you don’t need to change here.
>
> if (!(swsci & SWSCI_SCI_SELECT)) {
> gvt_vgpu_err("requesting SMI service\n");
> return 0;
> }
> + scic = &scic_val;
> + parm = &parm_val;
> /* ignore non 0->1 trasitions */
> if ((vgpu_cfg_space(vgpu)[INTEL_GVT_PCI_SWSCI]
> & SWSCI_SCI_TRIGGER) ||
> --
> 1.9.1
>
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
2017-05-18 15:43 ` He, Min
@ 2017-05-19 2:20 ` Chen, Xiaoguang
2017-05-22 2:28 ` He, Min
0 siblings, 1 reply; 13+ messages in thread
From: Chen, Xiaoguang @ 2017-05-19 2:20 UTC (permalink / raw)
To: He, Min, alex.williamson@redhat.com, kraxel@redhat.com,
intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
zhenyuw@linux.intel.com, Lv, Zhiyuan,
intel-gvt-dev@lists.freedesktop.org, Wang, Zhi A, Tian, Kevin
Cc: Niu, Bing
Hi min,
>-----Original Message-----
>From: He, Min
>Sent: Thursday, May 18, 2017 11:44 PM
>To: Chen, Xiaoguang <xiaoguang.chen@intel.com>;
>alex.williamson@redhat.com; kraxel@redhat.com; intel-
>gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
>zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>; intel-gvt-
>dev@lists.freedesktop.org; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, Kevin
><kevin.tian@intel.com>
>Cc: Niu, Bing <bing.niu@intel.com>; Chen, Xiaoguang
><xiaoguang.chen@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-bounces@lists.freedesktop.org] On Behalf Of
>> Xiaoguang Chen
>> Sent: Thursday, May 18, 2017 2:50 AM
>> To: alex.williamson@redhat.com; kraxel@redhat.com; intel-
>> gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
>> zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>;
>> intel-gvt- dev@lists.freedesktop.org; Wang, Zhi A
>> <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
>> Cc: Niu, Bing <bing.niu@intel.com>; Chen, Xiaoguang
>> <xiaoguang.chen@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.niu@intel.com>
>> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@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->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, 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);
>In current GVT code, we already have a function init_vgpu_opregion, which will
>copy the physical opregion into a per vgpu structure:
>vgpu_opregion(vgpu)->va, why don’t you reuse this function and set the
>vgpu_opregion(vgpu)->va to the intel_vgpu_register_reg function?
The reason is we do not the know the gpa of the OpRegion.
For KVMGT the gpa of the OpRegion is allocated by seabios.
>
>> + if (ret) {
>> + memunmap(base);
>> + return ret;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static int intel_vgpu_create(struct kobject *kobj, struct mdev_device
>> *mdev) {
>> struct intel_vgpu *vgpu = NULL;
>> @@ -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;
>> + }
>> +
>> + gvt_dbg_core("create OpRegion succeeded for mdev:%s\n",
>> + dev_name(mdev_dev(mdev)));
>> +
>> gvt_dbg_core("intel_vgpu_create succeeded for mdev: %s\n",
>> dev_name(mdev_dev(mdev)));
>> ret = 0;
>> diff --git a/drivers/gpu/drm/i915/gvt/opregion.c
>> b/drivers/gpu/drm/i915/gvt/opregion.c
>> index 3117991..99591bc 100644
>> --- a/drivers/gpu/drm/i915/gvt/opregion.c
>> +++ b/drivers/gpu/drm/i915/gvt/opregion.c
>> @@ -283,14 +283,22 @@ int intel_vgpu_emulate_opregion_request(struct
>> intel_vgpu *vgpu, u32 swsci)
>> {
>> u32 *scic, *parm;
>> u32 func, subfunc;
>> + u32 scic_val, parm_val;
>> + u32 gpa;
>>
>> - scic = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_SCIC;
>> - parm = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_PARM;
>> + memcpy(&gpa, vgpu_cfg_space(vgpu) + INTEL_GVT_PCI_OPREGION,
>> + sizeof(gpa));
>> + intel_gvt_hypervisor_read_gpa(vgpu, gpa +
>> INTEL_GVT_OPREGION_SCIC,
>> + &scic_val, sizeof(scic_val));
>> + intel_gvt_hypervisor_read_gpa(vgpu, gpa +
>> INTEL_GVT_OPREGION_PARM,
>> + &parm_val, sizeof(parm_val));
>As I said, you can call init_vgpu_opregion(), then vgpu_opregion(vgpu)->va will be
>valid and you don’t need to change here.
>
>>
>> if (!(swsci & SWSCI_SCI_SELECT)) {
>> gvt_vgpu_err("requesting SMI service\n");
>> return 0;
>> }
>> + scic = &scic_val;
>> + parm = &parm_val;
>> /* ignore non 0->1 trasitions */
>> if ((vgpu_cfg_space(vgpu)[INTEL_GVT_PCI_SWSCI]
>> & SWSCI_SCI_TRIGGER) ||
>> --
>> 1.9.1
>>
>> _______________________________________________
>> intel-gvt-dev mailing list
>> intel-gvt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
2017-05-19 2:20 ` Chen, Xiaoguang
@ 2017-05-22 2:28 ` He, Min
0 siblings, 0 replies; 13+ messages in thread
From: He, Min @ 2017-05-22 2:28 UTC (permalink / raw)
To: Chen, Xiaoguang, alex.williamson@redhat.com, kraxel@redhat.com,
intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
zhenyuw@linux.intel.com, Lv, Zhiyuan,
intel-gvt-dev@lists.freedesktop.org, Wang, Zhi A, Tian, Kevin
Cc: Niu, Bing
> -----Original Message-----
> From: Chen, Xiaoguang
> Sent: Thursday, May 18, 2017 7:20 PM
> To: He, Min <min.he@intel.com>; alex.williamson@redhat.com;
> kraxel@redhat.com; intel-gfx@lists.freedesktop.org; linux-
> kernel@vger.kernel.org; zhenyuw@linux.intel.com; Lv, Zhiyuan
> <zhiyuan.lv@intel.com>; intel-gvt-dev@lists.freedesktop.org; Wang, Zhi A
> <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
> Cc: Niu, Bing <bing.niu@intel.com>
> Subject: RE: [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.chen@intel.com>;
> >alex.williamson@redhat.com; kraxel@redhat.com; intel-
> >gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> >zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>; intel-gvt-
> >dev@lists.freedesktop.org; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, Kevin
> ><kevin.tian@intel.com>
> >Cc: Niu, Bing <bing.niu@intel.com>; Chen, Xiaoguang
> ><xiaoguang.chen@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-bounces@lists.freedesktop.org] On Behalf Of
> >> Xiaoguang Chen
> >> Sent: Thursday, May 18, 2017 2:50 AM
> >> To: alex.williamson@redhat.com; kraxel@redhat.com; intel-
> >> gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> >> zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>;
> >> intel-gvt- dev@lists.freedesktop.org; Wang, Zhi A
> >> <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
> >> Cc: Niu, Bing <bing.niu@intel.com>; Chen, Xiaoguang
> >> <xiaoguang.chen@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.niu@intel.com>
> >> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@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->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, 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);
> >In current GVT code, we already have a function init_vgpu_opregion, which will
> >copy the physical opregion into a per vgpu structure:
> >vgpu_opregion(vgpu)->va, why don’t you reuse this function and set the
> >vgpu_opregion(vgpu)->va to the intel_vgpu_register_reg function?
> The reason is we do not the know the gpa of the OpRegion.
> For KVMGT the gpa of the OpRegion is allocated by seabios.
Maybe you can consolidate them into one function to remove duplicate code.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/5] drm/i915/gvt: Frame buffer decoder support for GVT-g
2017-05-18 9:50 [PATCH v2 0/5] drm/i915/gvt: Dma-buf support for GVT-g Xiaoguang Chen
2017-05-18 9:50 ` [PATCH v2 1/5] drm/i915/gvt: Extend the GVT-g architecture to support vfio device region Xiaoguang Chen
2017-05-18 9:50 ` [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g Xiaoguang Chen
@ 2017-05-18 9:50 ` Xiaoguang Chen
2017-05-18 9:50 ` [PATCH v2 4/5] drm/i915/gvt: Dmabuf " Xiaoguang Chen
2017-05-18 9:50 ` [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf Xiaoguang Chen
4 siblings, 0 replies; 13+ messages in thread
From: Xiaoguang Chen @ 2017-05-18 9:50 UTC (permalink / raw)
To: alex.williamson, kraxel, intel-gfx, linux-kernel, zhenyuw,
zhiyuan.lv, intel-gvt-dev, zhi.a.wang, kevin.tian
Cc: Xiaoguang Chen
decode frambuffer attributes of primary, cursor and sprite plane
Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
---
drivers/gpu/drm/i915/gvt/Makefile | 3 +-
drivers/gpu/drm/i915/gvt/display.c | 2 +-
drivers/gpu/drm/i915/gvt/display.h | 2 +
drivers/gpu/drm/i915/gvt/fb_decoder.c | 487 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/gvt/fb_decoder.h | 166 ++++++++++++
drivers/gpu/drm/i915/gvt/gvt.h | 1 +
include/uapi/drm/i915_drm.h | 6 +
7 files changed, 665 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 @@ static inline char *vgpu_edid_str(enum intel_vgpu_edid id)
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..954f047
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
@@ -0,0 +1,487 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ * Kevin Tian <kevin.tian@intel.com>
+ *
+ * Contributors:
+ * Bing Niu <bing.niu@intel.com>
+ * Xu Han <xu.han@intel.com>
+ * Ping Gao <ping.a.gao@intel.com>
+ * Xiaoguang Chen <xiaoguang.chen@intel.com>
+ * Yang Liu <yang2.liu@intel.com>
+ *
+ */
+
+#include <uapi/drm/drm_fourcc.h>
+#include "i915_drv.h"
+#include "gvt.h"
+
+/* The below definitions are required by guest. */
+// [63:0] x:R:G:B 16:16:16:16 little endian
+#define DRM_FORMAT_XRGB161616_GVT fourcc_code('X', 'R', '4', '8')
+// [63:0] x:B:G:R 16:16:16:16 little endian
+#define DRM_FORMAT_XBGR161616_GVT fourcc_code('X', 'B', '4', '8')
+
+#define 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 primary_pixel_formats[FORMAT_NUM] = {
+ [0x2] = {DRM_FORMAT_C8, 8, "8-bit Indexed"},
+ [0x5] = {DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"},
+ [0x6] = {DRM_FORMAT_XRGB8888, 32,
+ "32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"},
+ [0x8] = {DRM_FORMAT_XBGR2101010, 32,
+ "32-bit RGBX (2:10:10:10 MSB-X:B:G:R)"},
+ [0xa] = {DRM_FORMAT_XRGB2101010, 32,
+ "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
+ [0xc] = {DRM_FORMAT_XRGB161616_GVT, 64,
+ "64-bit RGBX Floating Point(16:16:16:16 MSB-X:B:G:R)"},
+ [0xe] = {DRM_FORMAT_XBGR8888, 32,
+ "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"},
+};
+
+/* non-supported format has bpp default to 0 */
+static struct pixel_format skl_pixel_formats[] = {
+ {DRM_FORMAT_YUYV, 16, "16-bit packed YUYV (8:8:8:8 MSB-V:Y2:U:Y1)"},
+ {DRM_FORMAT_UYVY, 16, "16-bit packed UYVY (8:8:8:8 MSB-Y2:V:Y1:U)"},
+ {DRM_FORMAT_YVYU, 16, "16-bit packed YVYU (8:8:8:8 MSB-U:Y2:V:Y1)"},
+ {DRM_FORMAT_VYUY, 16, "16-bit packed VYUY (8:8:8:8 MSB-Y2:U:Y1:V)"},
+
+ {DRM_FORMAT_C8, 8, "8-bit Indexed"},
+ {DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"},
+ {DRM_FORMAT_ABGR8888, 32, "32-bit RGBA (8:8:8:8 MSB-A:B:G:R)"},
+ {DRM_FORMAT_XBGR8888, 32, "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"},
+
+ {DRM_FORMAT_ARGB8888, 32, "32-bit BGRA (8:8:8:8 MSB-A:R:G:B)"},
+ {DRM_FORMAT_XRGB8888, 32, "32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"},
+ {DRM_FORMAT_XBGR2101010, 32, "32-bit RGBX (2:10:10:10 MSB-X:B:G:R)"},
+ {DRM_FORMAT_XRGB2101010, 32, "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
+
+ {DRM_FORMAT_XRGB161616_GVT, 64,
+ "64-bit XRGB (16:16:16:16 MSB-X:R:G:B)"},
+ {DRM_FORMAT_XBGR161616_GVT, 64,
+ "64-bit XBGR (16:16:16:16 MSB-X:B:G:R)"},
+
+ /* non-supported format has bpp default to 0 */
+ {0, 0, NULL},
+};
+
+static int skl_format_to_drm(int format, bool rgb_order, bool alpha,
+ int yuv_order)
+{
+ int skl_pixel_formats_index = 14;
+
+ switch (format) {
+ case PLANE_CTL_FORMAT_INDEXED:
+ skl_pixel_formats_index = 4;
+ break;
+ case PLANE_CTL_FORMAT_RGB_565:
+ skl_pixel_formats_index = 5;
+ break;
+ case PLANE_CTL_FORMAT_XRGB_8888:
+ if (rgb_order)
+ skl_pixel_formats_index = alpha ? 6 : 7;
+ else
+ skl_pixel_formats_index = alpha ? 8 : 9;
+ break;
+ case PLANE_CTL_FORMAT_XRGB_2101010:
+ skl_pixel_formats_index = rgb_order ? 10 : 11;
+ break;
+
+ case PLANE_CTL_FORMAT_XRGB_16161616F:
+ skl_pixel_formats_index = rgb_order ? 12 : 13;
+ break;
+
+ case PLANE_CTL_FORMAT_YUV422:
+ skl_pixel_formats_index = yuv_order >> 16;
+ if (skl_pixel_formats_index > 3)
+ return -EINVAL;
+ break;
+
+ default:
+ break;
+ }
+
+ return skl_pixel_formats_index;
+}
+
+static u32 intel_vgpu_get_stride(struct intel_vgpu *vgpu, int pipe,
+ u32 tiled, int stride_mask, int bpp)
+{
+ struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+
+ u32 stride_reg = vgpu_vreg(vgpu, DSPSTRIDE(pipe)) & stride_mask;
+ u32 stride = stride_reg;
+
+ if (IS_SKYLAKE(dev_priv)) {
+ switch (tiled) {
+ case PLANE_CTL_TILED_LINEAR:
+ stride = stride_reg * 64;
+ break;
+ case PLANE_CTL_TILED_X:
+ stride = stride_reg * 512;
+ break;
+ case PLANE_CTL_TILED_Y:
+ stride = stride_reg * 128;
+ break;
+ case PLANE_CTL_TILED_YF:
+ if (bpp == 8)
+ stride = stride_reg * 64;
+ else if (bpp == 16 || bpp == 32 || bpp == 64)
+ stride = stride_reg * 128;
+ else
+ gvt_dbg_core("skl: unsupported bpp:%d\n", bpp);
+ break;
+ default:
+ gvt_dbg_core("skl: unsupported tile format:%x\n",
+ tiled);
+ }
+ }
+
+ return stride;
+}
+
+static int intel_vgpu_decode_primary_plane_format(struct intel_vgpu *vgpu,
+ int pipe, struct intel_vgpu_primary_plane_format *plane)
+{
+ u32 val, fmt;
+ struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+
+ val = vgpu_vreg(vgpu, DSPCNTR(pipe));
+ plane->enabled = !!(val & DISPLAY_PLANE_ENABLE);
+ if (!plane->enabled)
+ return 0;
+
+ 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 = primary_pixel_formats[fmt].bpp;
+ plane->drm_format = primary_pixel_formats[fmt].drm_format;
+ }
+
+ if ((IS_SKYLAKE(dev_priv) && !skl_pixel_formats[fmt].bpp)
+ || (!IS_SKYLAKE(dev_priv) &&
+ !primary_pixel_formats[fmt].bpp)) {
+ gvt_vgpu_err("Non-supported pixel format (0x%x)\n", fmt);
+ return -EINVAL;
+ }
+
+ plane->hw_format = fmt;
+
+ plane->base = vgpu_vreg(vgpu, DSPSURF(pipe)) & GTT_PAGE_MASK;
+
+ plane->stride = intel_vgpu_get_stride(vgpu, pipe, (plane->tiled << 10),
+ (IS_SKYLAKE(dev_priv)) ? (_PRI_PLANE_STRIDE_MASK >> 6) :
+ _PRI_PLANE_STRIDE_MASK, plane->bpp);
+
+ plane->width = (vgpu_vreg(vgpu, PIPESRC(pipe)) & _PIPE_H_SRCSZ_MASK) >>
+ _PIPE_H_SRCSZ_SHIFT;
+ plane->width += 1;
+ plane->height = (vgpu_vreg(vgpu, PIPESRC(pipe)) &
+ _PIPE_V_SRCSZ_MASK) >> _PIPE_V_SRCSZ_SHIFT;
+ plane->height += 1; /* raw height is one minus the real value */
+
+ val = vgpu_vreg(vgpu, DSPTILEOFF(pipe));
+ plane->x_offset = (val & _PRI_PLANE_X_OFF_MASK) >>
+ _PRI_PLANE_X_OFF_SHIFT;
+ plane->y_offset = (val & _PRI_PLANE_Y_OFF_MASK) >>
+ _PRI_PLANE_Y_OFF_SHIFT;
+
+ return 0;
+}
+
+#define CURSOR_MODE_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_MODE_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"},
+};
+
+static int intel_vgpu_decode_cursor_plane_format(struct intel_vgpu *vgpu,
+ int pipe, 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;
+
+ val = vgpu_vreg(vgpu, CURCNTR(pipe));
+ mode = val & CURSOR_MODE;
+ plane->enabled = (mode != CURSOR_MODE_DISABLE);
+ if (!plane->enabled)
+ return 0;
+
+ if (!cursor_pixel_formats[mode].bpp) {
+ gvt_vgpu_err("Non-supported cursor mode (0x%x)\n", mode);
+ return -EINVAL;
+ }
+ plane->mode = mode;
+ plane->bpp = cursor_pixel_formats[mode].bpp;
+ plane->drm_format = cursor_pixel_formats[mode].drm_format;
+ plane->width = cursor_pixel_formats[mode].width;
+ plane->height = cursor_pixel_formats[mode].height;
+
+ alpha_plane = (val & _CURSOR_ALPHA_PLANE_MASK) >>
+ _CURSOR_ALPHA_PLANE_SHIFT;
+ alpha_force = (val & _CURSOR_ALPHA_FORCE_MASK) >>
+ _CURSOR_ALPHA_FORCE_SHIFT;
+ if (alpha_plane || alpha_force)
+ gvt_dbg_core("alpha_plane=0x%x, alpha_force=0x%x\n",
+ alpha_plane, alpha_force);
+
+ plane->base = vgpu_vreg(vgpu, CURBASE(pipe)) & GTT_PAGE_MASK;
+
+ val = vgpu_vreg(vgpu, CURPOS(pipe));
+ plane->x_pos = (val & _CURSOR_POS_X_MASK) >> _CURSOR_POS_X_SHIFT;
+ plane->x_sign = (val & _CURSOR_SIGN_X_MASK) >> _CURSOR_SIGN_X_SHIFT;
+ plane->y_pos = (val & _CURSOR_POS_Y_MASK) >> _CURSOR_POS_Y_SHIFT;
+ plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >> _CURSOR_SIGN_Y_SHIFT;
+
+ return 0;
+}
+
+#define FORMAT_NUM_SRRITE (1 << 3)
+
+static struct pixel_format sprite_pixel_formats[FORMAT_NUM_SRRITE] = {
+ [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_GVT, 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)"},
+};
+
+static int intel_vgpu_decode_sprite_plane_format(struct intel_vgpu *vgpu,
+ int pipe, struct intel_vgpu_sprite_plane_format *plane)
+{
+ u32 val, fmt;
+ u32 width;
+ u32 color_order, yuv_order;
+ int drm_format;
+
+ val = vgpu_vreg(vgpu, SPRCTL(pipe));
+ plane->enabled = !!(val & SPRITE_ENABLE);
+ if (!plane->enabled)
+ return 0;
+
+ 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->width = vgpu_vreg(vgpu, SPRSTRIDE(pipe)) &
+ _SPRITE_STRIDE_MASK;
+ plane->width /= plane->bpp / 8; /* raw width in bytes */
+
+ val = vgpu_vreg(vgpu, SPRSIZE(pipe));
+ plane->height = (val & _SPRITE_SIZE_HEIGHT_MASK) >>
+ _SPRITE_SIZE_HEIGHT_SHIFT;
+ width = (val & _SPRITE_SIZE_WIDTH_MASK) >> _SPRITE_SIZE_WIDTH_SHIFT;
+ plane->height += 1; /* raw height is one minus the real value */
+ width += 1; /* raw width is one minus the real value */
+ if (plane->width != width)
+ gvt_dbg_core("sprite_plane: plane->width=%d, width=%d\n",
+ plane->width, width);
+
+ 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;
+}
+
+static int intel_vgpu_decode_fb_format(struct intel_gvt *gvt, int id,
+ struct intel_vgpu_fb_format *fb)
+{
+ int i;
+ struct intel_vgpu *vgpu = NULL;
+ int ret = 0;
+ struct drm_i915_private *dev_priv = gvt->dev_priv;
+
+ if (!fb)
+ return -EINVAL;
+
+ /* TODO: use fine-grained refcnt later */
+ mutex_lock(&gvt->lock);
+
+ for_each_active_vgpu(gvt, vgpu, i)
+ if (vgpu->id == id)
+ break;
+
+ if (!vgpu) {
+ gvt_vgpu_err("Invalid vgpu ID (%d)\n", id);
+ mutex_unlock(&gvt->lock);
+ return -ENODEV;
+ }
+
+ for (i = 0; i < I915_MAX_PIPES; i++) {
+ struct intel_vgpu_pipe_format *pipe = &fb->pipes[i];
+ u32 ddi_func_ctl = vgpu_vreg(vgpu, TRANS_DDI_FUNC_CTL(i));
+
+ if (!(ddi_func_ctl & TRANS_DDI_FUNC_ENABLE)) {
+ pipe->ddi_port = DDI_PORT_NONE;
+ } else {
+ u32 port = (ddi_func_ctl & TRANS_DDI_PORT_MASK) >>
+ TRANS_DDI_PORT_SHIFT;
+ if (port <= DDI_PORT_E)
+ pipe->ddi_port = port;
+ else
+ pipe->ddi_port = DDI_PORT_NONE;
+ }
+
+ ret |= intel_vgpu_decode_primary_plane_format(vgpu,
+ i, &pipe->primary);
+ ret |= intel_vgpu_decode_sprite_plane_format(vgpu,
+ i, &pipe->sprite);
+ ret |= intel_vgpu_decode_cursor_plane_format(vgpu,
+ i, &pipe->cursor);
+
+ if (ret) {
+ gvt_vgpu_err("Decode format error for pipe(%d)\n", i);
+ ret = -EINVAL;
+ break;
+ }
+ }
+
+ mutex_unlock(&gvt->lock);
+
+ return ret;
+}
+
+/**
+ * intel_vgpu_decode_plane - Decode plane based on plane id
+ * @dev: drm device
+ * @vgpu: input vgpu
+ * @plane_id: plane id to be decoded
+ * This function is called for decoding plane
+ *
+ * Returns:
+ * decoded plane on success, NULL if failed.
+ */
+void *intel_vgpu_decode_plane(struct drm_device *dev,
+ struct intel_vgpu *vgpu, u32 plane_id)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_vgpu_fb_format fb;
+ struct intel_vgpu_pipe_format *pipe;
+ int i;
+
+ if (intel_vgpu_decode_fb_format(dev_priv->gvt, vgpu->id, &fb))
+ return NULL;
+
+ for (i = 0; i < I915_MAX_PIPES; i++)
+ if (pipe_is_enabled(vgpu, i))
+ break;
+ if (i >= I915_MAX_PIPES) {
+ gvt_dbg_core("No enabled pipes\n");
+ return NULL;
+ }
+ pipe = &fb.pipes[i];
+
+ if (!pipe || !pipe->primary.enabled) {
+ gvt_dbg_core("Invalid pipe_id :%d\n", i);
+ return NULL;
+ }
+ if (plane_id == INTEL_GVT_PLANE_PRIMARY)
+ return &pipe->primary;
+
+ if (plane_id == INTEL_GVT_PLANE_CURSOR)
+ return &pipe->cursor;
+
+ gvt_dbg_core("Invalid plane_id: %d\n", plane_id);
+
+ return NULL;
+}
+
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..d7d5fc4
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.h
@@ -0,0 +1,166 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ * Kevin Tian <kevin.tian@intel.com>
+ *
+ * Contributors:
+ * Bing Niu <bing.niu@intel.com>
+ * Xu Han <xu.han@intel.com>
+ * Ping Gao <ping.a.gao@intel.com>
+ * Xiaoguang Chen <xiaoguang.chen@intel.com>
+ * Yang Liu <yang2.liu@intel.com>
+ *
+ */
+
+#ifndef _GVT_FB_DECODER_H_
+#define _GVT_FB_DECODER_H_
+
+#define _PLANE_CTL_FORMAT_SHIFT 24
+#define _PLANE_CTL_TILED_SHIFT 10
+#define _PIPE_V_SRCSZ_SHIFT 0
+#define _PIPE_V_SRCSZ_MASK (0xfff << _PIPE_V_SRCSZ_SHIFT)
+#define _PIPE_H_SRCSZ_SHIFT 16
+#define _PIPE_H_SRCSZ_MASK (0x1fff << _PIPE_H_SRCSZ_SHIFT)
+
+#define _PRI_PLANE_FMT_SHIFT 26
+#define _PRI_PLANE_STRIDE_MASK (0x3ff << 6)
+#define _PRI_PLANE_X_OFF_SHIFT 0
+#define _PRI_PLANE_X_OFF_MASK (0x1fff << _PRI_PLANE_X_OFF_SHIFT)
+#define _PRI_PLANE_Y_OFF_SHIFT 16
+#define _PRI_PLANE_Y_OFF_MASK (0xfff << _PRI_PLANE_Y_OFF_SHIFT)
+
+#define _CURSOR_MODE 0x3f
+#define _CURSOR_ALPHA_FORCE_SHIFT 8
+#define _CURSOR_ALPHA_FORCE_MASK (0x3 << _CURSOR_ALPHA_FORCE_SHIFT)
+#define _CURSOR_ALPHA_PLANE_SHIFT 10
+#define _CURSOR_ALPHA_PLANE_MASK (0x3 << _CURSOR_ALPHA_PLANE_SHIFT)
+#define _CURSOR_POS_X_SHIFT 0
+#define _CURSOR_POS_X_MASK (0x1fff << _CURSOR_POS_X_SHIFT)
+#define _CURSOR_SIGN_X_SHIFT 15
+#define _CURSOR_SIGN_X_MASK (1 << _CURSOR_SIGN_X_SHIFT)
+#define _CURSOR_POS_Y_SHIFT 16
+#define _CURSOR_POS_Y_MASK (0xfff << _CURSOR_POS_Y_SHIFT)
+#define _CURSOR_SIGN_Y_SHIFT 31
+#define _CURSOR_SIGN_Y_MASK (1 << _CURSOR_SIGN_Y_SHIFT)
+
+#define _SPRITE_FMT_SHIFT 25
+#define _SPRITE_COLOR_ORDER_SHIFT 20
+#define _SPRITE_YUV_ORDER_SHIFT 16
+#define _SPRITE_STRIDE_SHIFT 6
+#define _SPRITE_STRIDE_MASK (0x1ff << _SPRITE_STRIDE_SHIFT)
+#define _SPRITE_SIZE_WIDTH_SHIFT 0
+#define _SPRITE_SIZE_HEIGHT_SHIFT 16
+#define _SPRITE_SIZE_WIDTH_MASK (0x1fff << _SPRITE_SIZE_WIDTH_SHIFT)
+#define _SPRITE_SIZE_HEIGHT_MASK (0xfff << _SPRITE_SIZE_HEIGHT_SHIFT)
+#define _SPRITE_POS_X_SHIFT 0
+#define _SPRITE_POS_Y_SHIFT 16
+#define _SPRITE_POS_X_MASK (0x1fff << _SPRITE_POS_X_SHIFT)
+#define _SPRITE_POS_Y_MASK (0xfff << _SPRITE_POS_Y_SHIFT)
+#define _SPRITE_OFFSET_START_X_SHIFT 0
+#define _SPRITE_OFFSET_START_Y_SHIFT 16
+#define _SPRITE_OFFSET_START_X_MASK (0x1fff << _SPRITE_OFFSET_START_X_SHIFT)
+#define _SPRITE_OFFSET_START_Y_MASK (0xfff << _SPRITE_OFFSET_START_Y_SHIFT)
+
+enum GVT_FB_EVENT {
+ FB_MODE_SET_START = 1,
+ FB_MODE_SET_END,
+ FB_DISPLAY_FLIP,
+};
+
+enum DDI_PORT {
+ DDI_PORT_NONE = 0,
+ DDI_PORT_B = 1,
+ DDI_PORT_C = 2,
+ DDI_PORT_D = 3,
+ DDI_PORT_E = 4
+};
+
+struct intel_gvt;
+
+struct gvt_fb_notify_msg {
+ unsigned int vm_id;
+ unsigned int pipe_id; /* id starting from 0 */
+ unsigned int plane_id; /* primary, cursor, or sprite */
+};
+
+/* color space conversion and gamma correction are not included */
+struct intel_vgpu_primary_plane_format {
+ u8 enabled; /* plane is enabled */
+ u8 tiled; /* X-tiled */
+ u8 bpp; /* bits per pixel */
+ u32 hw_format; /* format field in the PRI_CTL register */
+ u32 drm_format; /* format in DRM definition */
+ u32 base; /* framebuffer base in graphics memory */
+ u32 x_offset; /* in pixels */
+ u32 y_offset; /* in lines */
+ u32 width; /* in pixels */
+ u32 height; /* in lines */
+ u32 stride; /* in bytes */
+};
+
+struct intel_vgpu_sprite_plane_format {
+ u8 enabled; /* plane is enabled */
+ u8 tiled; /* X-tiled */
+ u8 bpp; /* bits per pixel */
+ u32 hw_format; /* format field in the SPR_CTL register */
+ u32 drm_format; /* format in DRM definition */
+ u32 base; /* sprite base in graphics memory */
+ u32 x_pos; /* in pixels */
+ u32 y_pos; /* in lines */
+ u32 x_offset; /* in pixels */
+ u32 y_offset; /* in lines */
+ u32 width; /* in pixels */
+ u32 height; /* in lines */
+};
+
+struct intel_vgpu_cursor_plane_format {
+ u8 enabled;
+ u8 mode; /* cursor mode select */
+ u8 bpp; /* bits per pixel */
+ u32 drm_format; /* format in DRM definition */
+ u32 base; /* cursor base in graphics memory */
+ u32 x_pos; /* in pixels */
+ u32 y_pos; /* in lines */
+ u8 x_sign; /* X Position Sign */
+ u8 y_sign; /* Y Position Sign */
+ u32 width; /* in pixels */
+ u32 height; /* in lines */
+ u32 x_hot; /* in pixels */
+ u32 y_hot; /* in pixels */
+};
+
+struct intel_vgpu_pipe_format {
+ struct intel_vgpu_primary_plane_format primary;
+ struct intel_vgpu_sprite_plane_format sprite;
+ struct intel_vgpu_cursor_plane_format cursor;
+ enum DDI_PORT ddi_port; /* the DDI port that pipe is connected to */
+};
+
+struct intel_vgpu_fb_format {
+ struct intel_vgpu_pipe_format pipes[4];
+};
+
+void *intel_vgpu_decode_plane(struct drm_device *dev,
+ struct intel_vgpu *vgpu, u32 plane_id);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 930732e..c42266c 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -46,6 +46,7 @@
#include "sched_policy.h"
#include "render.h"
#include "cmd_parser.h"
+#include "fb_decoder.h"
#define GVT_MAX_VGPU 8
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index f24a80d..ece8e2b 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1439,6 +1439,12 @@ enum drm_i915_perf_record_type {
DRM_I915_PERF_RECORD_MAX /* non-ABI */
};
+/** Intel GVT-g specific definitions
+ */
+#define INTEL_GVT_PLANE_PRIMARY 1
+#define INTEL_GVT_PLANE_SPRITE 2
+#define INTEL_GVT_PLANE_CURSOR 3
+
#if defined(__cplusplus)
}
#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 4/5] drm/i915/gvt: Dmabuf support for GVT-g
2017-05-18 9:50 [PATCH v2 0/5] drm/i915/gvt: Dma-buf support for GVT-g Xiaoguang Chen
` (2 preceding siblings ...)
2017-05-18 9:50 ` [PATCH v2 3/5] drm/i915/gvt: Frame buffer decoder " Xiaoguang Chen
@ 2017-05-18 9:50 ` Xiaoguang Chen
2017-05-18 10:27 ` Chris Wilson
2017-05-19 11:28 ` Gerd Hoffmann
2017-05-18 9:50 ` [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf Xiaoguang Chen
4 siblings, 2 replies; 13+ messages in thread
From: Xiaoguang Chen @ 2017-05-18 9:50 UTC (permalink / raw)
To: alex.williamson, kraxel, intel-gfx, linux-kernel, zhenyuw,
zhiyuan.lv, intel-gvt-dev, zhi.a.wang, kevin.tian
Cc: Xiaoguang Chen
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.chen@intel.com>
---
drivers/gpu/drm/i915/gvt/Makefile | 2 +-
drivers/gpu/drm/i915/gvt/dmabuf.c | 321 ++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/gvt/dmabuf.h | 44 ++++++
drivers/gpu/drm/i915/gvt/gvt.h | 3 +
include/uapi/drm/i915_drm.h | 21 +++
5 files changed, 390 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 0000000..3358e6f
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
@@ -0,0 +1,321 @@
+/*
+ * Copyright 2017 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Zhiyuan Lv <zhiyuan.lv@intel.com>
+ *
+ * Contributors:
+ * Xiaoguang Chen <xiaoguang.chen@intel.com>
+ */
+
+#include <linux/dma-buf.h>
+#include <drm/drmP.h>
+
+#include "i915_drv.h"
+#include "gvt.h"
+
+#define GEN8_DECODE_PTE(pte) \
+ ((dma_addr_t)(((((u64)pte) >> 12) & 0x7ffffffULL) << 12))
+
+static struct sg_table *intel_vgpu_gem_get_pages(
+ struct drm_i915_gem_object *obj)
+{
+ struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+ struct intel_gvt *gvt = dev_priv->gvt;
+ struct sg_table *st;
+ struct scatterlist *sg;
+ int i, ret;
+ gen8_pte_t __iomem *gtt_entries;
+ struct intel_vgpu *vgpu;
+ unsigned int fb_gma = 0, fb_size = 0;
+ bool found = false;
+
+ mutex_lock(&gvt->lock);
+ for_each_active_vgpu(gvt, vgpu, i) {
+ if (vgpu->obj_dmabuf == obj) {
+ fb_gma = vgpu->plane_info->start;
+ fb_size = vgpu->plane_info->size;
+ found = true;
+ break;
+ }
+ }
+ mutex_unlock(&gvt->lock);
+
+ if (!found) {
+ gvt_vgpu_err("no vgpu found\n");
+ return NULL;
+ }
+
+ st = kmalloc(sizeof(*st), GFP_KERNEL);
+ if (!st) {
+ ret = -ENOMEM;
+ return ERR_PTR(ret);
+ }
+
+ ret = sg_alloc_table(st, fb_size, GFP_KERNEL);
+ if (ret) {
+ kfree(st);
+ return ERR_PTR(ret);
+ }
+ gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
+ (fb_gma >> PAGE_SHIFT);
+ for_each_sg(st->sgl, sg, fb_size, i) {
+ sg->offset = 0;
+ sg->length = PAGE_SIZE;
+ sg_dma_address(sg) =
+ GEN8_DECODE_PTE(readq(>t_entries[i]));
+ sg_dma_len(sg) = PAGE_SIZE;
+ }
+
+ return st;
+}
+
+static void intel_vgpu_gem_put_pages(struct drm_i915_gem_object *obj,
+ struct sg_table *pages)
+{
+ struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+ struct intel_gvt *gvt = dev_priv->gvt;
+ int i;
+ struct intel_vgpu *vgpu;
+
+ mutex_lock(&gvt->lock);
+ for_each_active_vgpu(gvt, vgpu, i) {
+ if (vgpu->obj_dmabuf == obj) {
+ kfree(vgpu->plane_info);
+ break;
+ }
+ }
+ mutex_unlock(&gvt->lock);
+
+ sg_free_table(pages);
+ kfree(pages);
+
+ i915_gem_object_unpin_pages(obj);
+}
+
+static const struct drm_i915_gem_object_ops intel_vgpu_gem_ops = {
+ .get_pages = intel_vgpu_gem_get_pages,
+ .put_pages = intel_vgpu_gem_put_pages,
+};
+
+static struct drm_i915_gem_object *intel_vgpu_create_gem(struct drm_device *dev,
+ struct intel_vgpu *vgpu)
+{
+ struct drm_i915_private *pri = dev->dev_private;
+ struct drm_i915_gem_object *obj;
+ struct intel_vgpu_plane_info *info = vgpu->plane_info;
+
+ obj = i915_gem_object_alloc(pri);
+ if (obj == NULL)
+ return NULL;
+
+ drm_gem_private_object_init(dev, &obj->base,
+ info->size << PAGE_SHIFT);
+ i915_gem_object_init(obj, &intel_vgpu_gem_ops);
+
+ obj->base.read_domains = I915_GEM_DOMAIN_GTT;
+ obj->base.write_domain = 0;
+
+ if (IS_SKYLAKE(pri)) {
+ unsigned int tiling_mode = 0;
+
+ switch (info->tiled << 10) {
+ case PLANE_CTL_TILED_LINEAR:
+ tiling_mode = I915_TILING_NONE;
+ break;
+ case PLANE_CTL_TILED_X:
+ tiling_mode = I915_TILING_X;
+ break;
+ case PLANE_CTL_TILED_Y:
+ tiling_mode = I915_TILING_Y;
+ break;
+ default:
+ gvt_vgpu_err("tile %d not supported\n", info->tiled);
+ }
+ obj->tiling_and_stride = tiling_mode | info->stride;
+ } else {
+ obj->tiling_and_stride = info->tiled ? I915_TILING_X : 0;
+ }
+
+ return obj;
+}
+
+static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(
+ struct drm_device *dev,
+ struct intel_vgpu *vgpu, int plane_id)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_vgpu_primary_plane_format *p;
+ struct intel_vgpu_cursor_plane_format *c;
+ struct intel_vgpu_plane_info *info;
+
+ info = kmalloc(sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return NULL;
+
+ if (plane_id == INTEL_GVT_PLANE_PRIMARY) {
+ p = (struct intel_vgpu_primary_plane_format *)
+ intel_vgpu_decode_plane(dev, vgpu, plane_id);
+ if (p != NULL) {
+ info->start = p->base;
+ info->width = p->width;
+ info->height = p->height;
+ info->stride = p->stride;
+ info->drm_format = p->drm_format;
+ info->tiled = p->tiled;
+ info->size = (((p->stride * p->height * p->bpp) / 8) +
+ (PAGE_SIZE - 1)) >> PAGE_SHIFT;
+ } else {
+ kfree(info);
+ gvt_vgpu_err("invalid primary plane\n");
+ return NULL;
+ }
+ } else if (plane_id == INTEL_GVT_PLANE_CURSOR) {
+ c = (struct intel_vgpu_cursor_plane_format *)
+ intel_vgpu_decode_plane(dev, vgpu, plane_id);
+ if (c != NULL) {
+ info->start = c->base;
+ info->width = c->width;
+ info->height = c->height;
+ info->stride = c->width * (c->bpp / 8);
+ info->tiled = 0;
+ info->x_pos = c->x_pos;
+ info->y_pos = c->y_pos;
+ info->size = (((info->stride * c->height * c->bpp) / 8)
+ + (PAGE_SIZE - 1)) >> PAGE_SHIFT;
+ } else {
+ kfree(info);
+ gvt_vgpu_err("invalid cursor plane\n");
+ return NULL;
+ }
+ } else {
+ kfree(info);
+ gvt_vgpu_err("invalid plane id:%d\n", plane_id);
+ return NULL;
+ }
+
+ if (info->size == 0) {
+ kfree(info);
+ gvt_vgpu_err("fb size is zero\n");
+ return NULL;
+ }
+
+ if (info->start & (PAGE_SIZE - 1)) {
+ kfree(info);
+ gvt_vgpu_err("Not aligned fb address:0x%x\n", info->start);
+ return NULL;
+ }
+ if (((info->start >> PAGE_SHIFT) + info->size) >
+ ggtt_total_entries(&dev_priv->ggtt)) {
+ kfree(info);
+ gvt_vgpu_err("Invalid GTT offset or size\n");
+ return NULL;
+ }
+
+ if (!intel_gvt_ggtt_validate_range(vgpu, info->start, info->size)) {
+ kfree(info);
+ gvt_vgpu_err("invalid gma addr\n");
+ return NULL;
+ }
+
+ return info;
+}
+
+int intel_vgpu_query_dmabuf(struct intel_vgpu *vgpu, void *args)
+{
+ struct drm_device *dev = &vgpu->gvt->dev_priv->drm;
+ struct intel_vgpu_dmabuf *gvt_dmabuf = args;
+ struct intel_vgpu_plane_info *info;
+
+ info = intel_vgpu_get_plane_info(dev, vgpu, gvt_dmabuf->plane_id);
+ if (info == NULL)
+ return -EINVAL;
+
+ vgpu->plane_info = info;
+
+ gvt_dmabuf->drm_format = info->drm_format;
+ gvt_dmabuf->width = info->width;
+ gvt_dmabuf->height = info->height;
+ gvt_dmabuf->stride = info->stride;
+ gvt_dmabuf->start = info->start;
+ gvt_dmabuf->x_pos = info->x_pos;
+ gvt_dmabuf->y_pos = info->y_pos;
+ gvt_dmabuf->size = info->size;
+ gvt_dmabuf->tiled = info->tiled;
+
+ return 0;
+}
+
+int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args)
+{
+ struct dma_buf *dmabuf;
+ struct drm_i915_gem_object *obj;
+ struct drm_device *dev = &vgpu->gvt->dev_priv->drm;
+ struct intel_vgpu_dmabuf *gvt_dmabuf = args;
+ struct intel_vgpu_plane_info *info;
+ int ret;
+
+ info = intel_vgpu_get_plane_info(dev, vgpu, gvt_dmabuf->plane_id);
+ if (info == NULL)
+ return -EINVAL;
+
+ vgpu->plane_info = info;
+ obj = intel_vgpu_create_gem(dev, vgpu);
+ if (obj == NULL) {
+ gvt_vgpu_err("create gvt gem obj failed:%d\n", vgpu->id);
+ return -EINVAL;
+ }
+ vgpu->obj_dmabuf = obj;
+
+ ret = i915_gem_object_pin_pages(obj);
+ if (ret) {
+ i915_gem_object_free(obj);
+ gvt_vgpu_err("pin pages failed\n");
+ return -EINVAL;
+ }
+
+ dmabuf = i915_gem_prime_export(dev, &obj->base, DRM_CLOEXEC | DRM_RDWR);
+
+ if (IS_ERR(dmabuf)) {
+ gvt_vgpu_err("export dma-buf failed\n");
+ return -EINVAL;
+ }
+
+ ret = dma_buf_fd(dmabuf, DRM_CLOEXEC | DRM_RDWR);
+ if (ret < 0) {
+ gvt_vgpu_err("create dma-buf fd failed ret:%d\n", ret);
+ return -EINVAL;
+ }
+ gvt_dmabuf->fd = ret;
+
+ gvt_dmabuf->drm_format = info->drm_format;
+ gvt_dmabuf->width = info->width;
+ gvt_dmabuf->height = info->height;
+ gvt_dmabuf->stride = info->stride;
+ gvt_dmabuf->start = info->start;
+ gvt_dmabuf->x_pos = info->x_pos;
+ gvt_dmabuf->y_pos = info->y_pos;
+ gvt_dmabuf->size = info->size;
+ gvt_dmabuf->tiled = info->tiled;
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.h b/drivers/gpu/drm/i915/gvt/dmabuf.h
new file mode 100644
index 0000000..a411fc6
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.h
@@ -0,0 +1,44 @@
+
+/*
+ * Copyright(c) 2017 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ */
+
+#ifndef _GVT_DMABUF_H_
+#define _GVT_DMABUF_H_
+
+struct intel_vgpu_plane_info {
+ unsigned int drm_format;
+ unsigned int width;
+ unsigned int height;
+ unsigned int stride;
+ unsigned int start;
+ unsigned int x_pos;
+ unsigned int y_pos;
+ unsigned int size;
+ unsigned int tiled;
+};
+
+int intel_vgpu_query_dmabuf(struct intel_vgpu *vgpu, void *args);
+int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index c42266c..a553120 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -47,6 +47,7 @@
#include "render.h"
#include "cmd_parser.h"
#include "fb_decoder.h"
+#include "dmabuf.h"
#define GVT_MAX_VGPU 8
@@ -186,6 +187,8 @@ struct intel_vgpu {
atomic_t released;
} vdev;
#endif
+ struct intel_vgpu_plane_info *plane_info;
+ struct drm_i915_gem_object *obj_dmabuf;
};
struct intel_gvt_gm {
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index ece8e2b..cde4f8e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1445,6 +1445,27 @@ enum drm_i915_perf_record_type {
#define INTEL_GVT_PLANE_SPRITE 2
#define INTEL_GVT_PLANE_CURSOR 3
+/**
+ * Ioctl to query plane info or create dma-buf
+ */
+#define INTEL_VGPU_QUERY_DMABUF 0
+#define INTEL_VGPU_GENERATE_DMABUF 1
+
+struct intel_vgpu_dmabuf {
+ __u32 plane_id;
+ /* out */
+ __u32 fd;
+ __u32 drm_format;
+ __u32 width;
+ __u32 height;
+ __u32 stride;
+ __u32 start;
+ __u32 x_pos;
+ __u32 y_pos;
+ __u32 size;
+ __u32 tiled;
+};
+
#if defined(__cplusplus)
}
#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 4/5] drm/i915/gvt: Dmabuf support for GVT-g
2017-05-18 9:50 ` [PATCH v2 4/5] drm/i915/gvt: Dmabuf " Xiaoguang Chen
@ 2017-05-18 10:27 ` Chris Wilson
2017-05-19 11:28 ` Gerd Hoffmann
1 sibling, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2017-05-18 10:27 UTC (permalink / raw)
To: Xiaoguang Chen
Cc: alex.williamson, kraxel, intel-gfx, linux-kernel, zhenyuw,
zhiyuan.lv, intel-gvt-dev, zhi.a.wang, kevin.tian
On Thu, May 18, 2017 at 05:50:04PM +0800, Xiaoguang Chen wrote:
> +#define GEN8_DECODE_PTE(pte) \
> + ((dma_addr_t)(((((u64)pte) >> 12) & 0x7ffffffULL) << 12))
> +
> +static struct sg_table *intel_vgpu_gem_get_pages(
> + struct drm_i915_gem_object *obj)
> +{
> + struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
> + struct intel_gvt *gvt = dev_priv->gvt;
> + struct sg_table *st;
> + struct scatterlist *sg;
> + int i, ret;
> + gen8_pte_t __iomem *gtt_entries;
> + struct intel_vgpu *vgpu;
> + unsigned int fb_gma = 0, fb_size = 0;
> + bool found = false;
> +
> + mutex_lock(&gvt->lock);
> + for_each_active_vgpu(gvt, vgpu, i) {
> + if (vgpu->obj_dmabuf == obj) {
> + fb_gma = vgpu->plane_info->start;
> + fb_size = vgpu->plane_info->size;
> + found = true;
> + break;
> + }
> + }
> + mutex_unlock(&gvt->lock);
> +
> + if (!found) {
> + gvt_vgpu_err("no vgpu found\n");
> + return NULL;
> + }
> +
> + st = kmalloc(sizeof(*st), GFP_KERNEL);
> + if (!st) {
> + ret = -ENOMEM;
> + return ERR_PTR(ret);
> + }
> +
> + ret = sg_alloc_table(st, fb_size, GFP_KERNEL);
> + if (ret) {
> + kfree(st);
> + return ERR_PTR(ret);
> + }
> + gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
> + (fb_gma >> PAGE_SHIFT);
> + for_each_sg(st->sgl, sg, fb_size, i) {
> + sg->offset = 0;
> + sg->length = PAGE_SIZE;
> + sg_dma_address(sg) =
> + GEN8_DECODE_PTE(readq(>t_entries[i]));
> + sg_dma_len(sg) = PAGE_SIZE;
> + }
> +
> + return st;
> +}
> +
> +static void intel_vgpu_gem_put_pages(struct drm_i915_gem_object *obj,
> + struct sg_table *pages)
> +{
> + struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
> + struct intel_gvt *gvt = dev_priv->gvt;
> + int i;
> + struct intel_vgpu *vgpu;
> +
> + mutex_lock(&gvt->lock);
> + for_each_active_vgpu(gvt, vgpu, i) {
> + if (vgpu->obj_dmabuf == obj) {
> + kfree(vgpu->plane_info);
> + break;
> + }
We have a union for private data in i915_gem_object that you can use to
store the backpointer.
> + }
> + mutex_unlock(&gvt->lock);
> +
> + sg_free_table(pages);
> + kfree(pages);
> +
> + i915_gem_object_unpin_pages(obj);
That's confused and should be triggering at least 2 WARNs when the
object is freed.
> +}
> +
> +static const struct drm_i915_gem_object_ops intel_vgpu_gem_ops = {
> + .get_pages = intel_vgpu_gem_get_pages,
> + .put_pages = intel_vgpu_gem_put_pages,
> +};
> +
> +static struct drm_i915_gem_object *intel_vgpu_create_gem(struct drm_device *dev,
> + struct intel_vgpu *vgpu)
> +{
> + struct drm_i915_private *pri = dev->dev_private;
> + struct drm_i915_gem_object *obj;
> + struct intel_vgpu_plane_info *info = vgpu->plane_info;
> +
> + obj = i915_gem_object_alloc(pri);
> + if (obj == NULL)
> + return NULL;
> +
> + drm_gem_private_object_init(dev, &obj->base,
> + info->size << PAGE_SHIFT);
> + i915_gem_object_init(obj, &intel_vgpu_gem_ops);
> +
> + obj->base.read_domains = I915_GEM_DOMAIN_GTT;
> + obj->base.write_domain = 0;
> +
> + if (IS_SKYLAKE(pri)) {
> + unsigned int tiling_mode = 0;
> +
> + switch (info->tiled << 10) {
> + case PLANE_CTL_TILED_LINEAR:
> + tiling_mode = I915_TILING_NONE;
> + break;
> + case PLANE_CTL_TILED_X:
> + tiling_mode = I915_TILING_X;
> + break;
> + case PLANE_CTL_TILED_Y:
> + tiling_mode = I915_TILING_Y;
> + break;
> + default:
> + gvt_vgpu_err("tile %d not supported\n", info->tiled);
> + }
> + obj->tiling_and_stride = tiling_mode | info->stride;
> + } else {
> + obj->tiling_and_stride = info->tiled ? I915_TILING_X : 0;
> + }
How good a first class object is this? If I understand this correctly,
we have a valid DMA mapping for the object, so we can access it via GTT
mmaps and rendering on the GPU. We just can't use CPU mmaps.
However, we should reject things like tiling changes, cache level
changes, set-domain is also problematic.
> +
> + return obj;
> +}
> +int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args)
> +{
> + struct dma_buf *dmabuf;
> + struct drm_i915_gem_object *obj;
> + struct drm_device *dev = &vgpu->gvt->dev_priv->drm;
> + struct intel_vgpu_dmabuf *gvt_dmabuf = args;
> + struct intel_vgpu_plane_info *info;
> + int ret;
> +
> + info = intel_vgpu_get_plane_info(dev, vgpu, gvt_dmabuf->plane_id);
> + if (info == NULL)
> + return -EINVAL;
> +
> + vgpu->plane_info = info;
> + obj = intel_vgpu_create_gem(dev, vgpu);
> + if (obj == NULL) {
> + gvt_vgpu_err("create gvt gem obj failed:%d\n", vgpu->id);
> + return -EINVAL;
> + }
> + vgpu->obj_dmabuf = obj;
> +
> + ret = i915_gem_object_pin_pages(obj);
Why do you need to pin the pages? It has no relevance to the backing
storage, that can be discard freely by the other end of vgpu. get_pages
is just allocating an sg_table...
-chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 4/5] drm/i915/gvt: Dmabuf support for GVT-g
2017-05-18 9:50 ` [PATCH v2 4/5] drm/i915/gvt: Dmabuf " Xiaoguang Chen
2017-05-18 10:27 ` Chris Wilson
@ 2017-05-19 11:28 ` Gerd Hoffmann
1 sibling, 0 replies; 13+ messages in thread
From: Gerd Hoffmann @ 2017-05-19 11:28 UTC (permalink / raw)
To: Xiaoguang Chen
Cc: alex.williamson, intel-gfx, linux-kernel, zhenyuw, zhiyuan.lv,
intel-gvt-dev, zhi.a.wang, kevin.tian
> +/**
> + * Ioctl to query plane info or create dma-buf
> + */
> +#define INTEL_VGPU_QUERY_DMABUF 0
> +#define INTEL_VGPU_GENERATE_DMABUF 1
This should use _IO* #defines.
> +struct intel_vgpu_dmabuf {
> + __u32 plane_id;
> + /* out */
> + __u32 fd;
> + __u32 drm_format;
> + __u32 width;
> + __u32 height;
> + __u32 stride;
> + __u32 start;
> + __u32 x_pos;
> + __u32 y_pos;
> + __u32 size;
> + __u32 tiled;
I'd suggest to drop tiled, and add drm_format_mod (for
fourcc_mod_code()) instead.
You can also move intel_vgpu_plane_info to this place and just use it as
sub-struct for intel_vgpu_dmabuf.
cheers,
Gerd
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf
2017-05-18 9:50 [PATCH v2 0/5] drm/i915/gvt: Dma-buf support for GVT-g Xiaoguang Chen
` (3 preceding siblings ...)
2017-05-18 9:50 ` [PATCH v2 4/5] drm/i915/gvt: Dmabuf " Xiaoguang Chen
@ 2017-05-18 9:50 ` Xiaoguang Chen
2017-05-19 16:33 ` Alex Williamson
4 siblings, 1 reply; 13+ messages in thread
From: Xiaoguang Chen @ 2017-05-18 9:50 UTC (permalink / raw)
To: alex.williamson, kraxel, intel-gfx, linux-kernel, zhenyuw,
zhiyuan.lv, intel-gvt-dev, zhi.a.wang, kevin.tian
Cc: Xiaoguang Chen
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.chen@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 <linux/kvm_host.h>
#include <linux/vfio.h>
#include <linux/mdev.h>
+#include <linux/anon_inodes.h>
#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");
+
+ 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(&dmabuf, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ f = fdget(dmabuf.fd);
+
+ if (ioctl == INTEL_VGPU_QUERY_DMABUF)
+ ret = intel_gvt_ops->vgpu_query_dmabuf(vgpu, &dmabuf);
+ else if (ioctl == INTEL_VGPU_GENERATE_DMABUF)
+ ret = intel_gvt_ops->vgpu_create_dmabuf(vgpu, &dmabuf);
+ else {
+ fdput(f);
+ gvt_vgpu_err("unsupported dmabuf operation\n");
+ return -EINVAL;
+ }
+
+ if (ret != 0) {
+ fdput(f);
+ gvt_vgpu_err("gvt-g get dmabuf failed:%d\n", ret);
+ return -EINVAL;
+ }
+ fdput(f);
+
+ return copy_to_user((void __user *)arg, &dmabuf, minsz) ? -EFAULT : 0;
+}
+
+static const struct file_operations intel_vgpu_dmabuf_mgr_fd_ops = {
+ .release = intel_vgpu_dmabuf_mgr_fd_release,
+ .unlocked_ioctl = intel_vgpu_dmabuf_mgr_fd_ioctl,
+ .mmap = intel_vgpu_dmabuf_mgr_fd_mmap,
+ .llseek = noop_llseek,
+};
static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
{
struct intel_vgpu *vgpu = NULL;
@@ -1259,6 +1320,34 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
} else if (cmd == VFIO_DEVICE_RESET) {
intel_gvt_ops->vgpu_reset(vgpu);
return 0;
+ } else if (cmd == VFIO_DEVICE_GET_FD) {
+ int fd;
+ u32 type;
+ struct vfio_device *device;
+
+ if (copy_from_user(&type, (void __user *)arg, sizeof(type)))
+ return -EINVAL;
+ if (type != INTEL_VGPU_DMABUF_MGR_FD) {
+ gvt_vgpu_err("not supported fd type:%d\n", type);
+ return -EINVAL;
+ }
+
+ fd = anon_inode_getfd("intel-vgpu-dmabuf-mgr-fd",
+ &intel_vgpu_dmabuf_mgr_fd_ops,
+ vgpu, O_RDWR | O_CLOEXEC);
+ if (fd < 0) {
+ gvt_vgpu_err("create dmabuf mgr fd failed\n");
+ return -EINVAL;
+ }
+
+ device = vfio_device_get_from_dev(mdev_dev(mdev));
+ if (device == NULL) {
+ gvt_vgpu_err("kvmgt: vfio device is null\n");
+ return -EINVAL;
+ }
+ vgpu->vdev.vfio_device = device;
+
+ return fd;
}
return 0;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index cde4f8e..9d28433 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1466,6 +1466,8 @@ struct intel_vgpu_dmabuf {
__u32 tiled;
};
+#define INTEL_VGPU_DMABUF_MGR_FD 0
+
#if defined(__cplusplus)
}
#endif
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index ae46105..c81500b 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -502,6 +502,18 @@ 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)
+
/* -------- API for Type1 VFIO IOMMU -------- */
/**
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf
2017-05-18 9:50 ` [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf Xiaoguang Chen
@ 2017-05-19 16:33 ` Alex Williamson
2017-05-22 2:00 ` Chen, Xiaoguang
0 siblings, 1 reply; 13+ messages in thread
From: Alex Williamson @ 2017-05-19 16:33 UTC (permalink / raw)
To: Xiaoguang Chen
Cc: kraxel, intel-gfx, linux-kernel, zhenyuw, zhiyuan.lv,
intel-gvt-dev, zhi.a.wang, kevin.tian
On Thu, 18 May 2017 17:50:05 +0800
Xiaoguang Chen <xiaoguang.chen@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.chen@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 <linux/kvm_host.h>
> #include <linux/vfio.h>
> #include <linux/mdev.h>
> +#include <linux/anon_inodes.h>
>
> #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;
> +
> + 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(&dmabuf, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + f = fdget(dmabuf.fd);
> +
> + if (ioctl == INTEL_VGPU_QUERY_DMABUF)
> + ret = intel_gvt_ops->vgpu_query_dmabuf(vgpu, &dmabuf);
> + else if (ioctl == INTEL_VGPU_GENERATE_DMABUF)
> + ret = intel_gvt_ops->vgpu_create_dmabuf(vgpu, &dmabuf);
Why are these still Intel specific?
> + else {
> + fdput(f);
> + gvt_vgpu_err("unsupported dmabuf operation\n");
> + return -EINVAL;
> + }
> +
> + if (ret != 0) {
> + fdput(f);
> + gvt_vgpu_err("gvt-g get dmabuf failed:%d\n", ret);
> + return -EINVAL;
> + }
> + fdput(f);
> +
> + return copy_to_user((void __user *)arg, &dmabuf, minsz) ? -EFAULT : 0;
> +}
> +
> +static const struct file_operations intel_vgpu_dmabuf_mgr_fd_ops = {
> + .release = intel_vgpu_dmabuf_mgr_fd_release,
> + .unlocked_ioctl = intel_vgpu_dmabuf_mgr_fd_ioctl,
> + .mmap = intel_vgpu_dmabuf_mgr_fd_mmap,
> + .llseek = noop_llseek,
> +};
> static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
> {
> struct intel_vgpu *vgpu = NULL;
> @@ -1259,6 +1320,34 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
> } else if (cmd == VFIO_DEVICE_RESET) {
> intel_gvt_ops->vgpu_reset(vgpu);
> return 0;
> + } else if (cmd == VFIO_DEVICE_GET_FD) {
> + int fd;
> + u32 type;
> + struct vfio_device *device;
> +
> + if (copy_from_user(&type, (void __user *)arg, sizeof(type)))
> + return -EINVAL;
> + if (type != INTEL_VGPU_DMABUF_MGR_FD) {
Yet more unnecessary Intel specific code. Userspace doesn't want to
handle a dmabuf differently between Intel, AMD, and NVIDIA, so if we
have no reason to need vendor specific ioctl and parameters, then don't.
> + gvt_vgpu_err("not supported fd type:%d\n", type);
Just return error, users can abuse any logging they can get to.
> + return -EINVAL;
> + }
> +
> + fd = anon_inode_getfd("intel-vgpu-dmabuf-mgr-fd",
> + &intel_vgpu_dmabuf_mgr_fd_ops,
> + vgpu, O_RDWR | O_CLOEXEC);
And this ordering is still wrong, do this last so you don't need to
worry about getting the vfio_device reference below failing and calling
release unnecessarily (though it seems this fd is leaked in that case
anyway).
> + if (fd < 0) {
> + gvt_vgpu_err("create dmabuf mgr fd failed\n");
> + return -EINVAL;
> + }
> +
> + device = vfio_device_get_from_dev(mdev_dev(mdev));
> + if (device == NULL) {
> + gvt_vgpu_err("kvmgt: vfio device is null\n");
> + return -EINVAL;
> + }
> + vgpu->vdev.vfio_device = device;
> +
> + return fd;
> }
>
> return 0;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index cde4f8e..9d28433 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1466,6 +1466,8 @@ struct intel_vgpu_dmabuf {
> __u32 tiled;
> };
>
> +#define INTEL_VGPU_DMABUF_MGR_FD 0
> +
> #if defined(__cplusplus)
> }
> #endif
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index ae46105..c81500b 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -502,6 +502,18 @@ 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)
> +
The known input types would need to be defined here too. Definition of
the ioctls available on that type should also be documented here.
> /* -------- API for Type1 VFIO IOMMU -------- */
>
> /**
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf
2017-05-19 16:33 ` Alex Williamson
@ 2017-05-22 2:00 ` Chen, Xiaoguang
0 siblings, 0 replies; 13+ messages in thread
From: Chen, Xiaoguang @ 2017-05-22 2:00 UTC (permalink / raw)
To: Alex Williamson
Cc: kraxel@redhat.com, intel-gfx@lists.freedesktop.org,
linux-kernel@vger.kernel.org, zhenyuw@linux.intel.com,
Lv, Zhiyuan, intel-gvt-dev@lists.freedesktop.org, Wang, Zhi A,
Tian, Kevin
Hi Alex,
>-----Original Message-----
>From: Alex Williamson [mailto:alex.williamson@redhat.com]
>Sent: Saturday, May 20, 2017 12:34 AM
>To: Chen, Xiaoguang <xiaoguang.chen@intel.com>
>Cc: kraxel@redhat.com; intel-gfx@lists.freedesktop.org; linux-
>kernel@vger.kernel.org; zhenyuw@linux.intel.com; Lv, Zhiyuan
><zhiyuan.lv@intel.com>; intel-gvt-dev@lists.freedesktop.org; Wang, Zhi A
><zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@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.chen@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.chen@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 <linux/kvm_host.h>
>> #include <linux/vfio.h>
>> #include <linux/mdev.h>
>> +#include <linux/anon_inodes.h>
>>
>> #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(&dmabuf, (void __user *)arg, minsz))
>> + return -EFAULT;
>> +
>> + f = fdget(dmabuf.fd);
>> +
>> + if (ioctl == INTEL_VGPU_QUERY_DMABUF)
>> + ret = intel_gvt_ops->vgpu_query_dmabuf(vgpu, &dmabuf);
>> + else if (ioctl == INTEL_VGPU_GENERATE_DMABUF)
>> + ret = intel_gvt_ops->vgpu_create_dmabuf(vgpu, &dmabuf);
>
>Why are these still Intel specific?
Ah, the v2 patch series were sent before our discussion in other thread. Will change it in v3.
>
>> + else {
>> + fdput(f);
>> + gvt_vgpu_err("unsupported dmabuf operation\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (ret != 0) {
>> + fdput(f);
>> + gvt_vgpu_err("gvt-g get dmabuf failed:%d\n", ret);
>> + return -EINVAL;
>> + }
>> + fdput(f);
>> +
>> + return copy_to_user((void __user *)arg, &dmabuf, minsz) ? -EFAULT :
>> +0; }
>> +
>> +static const struct file_operations intel_vgpu_dmabuf_mgr_fd_ops = {
>> + .release = intel_vgpu_dmabuf_mgr_fd_release,
>> + .unlocked_ioctl = intel_vgpu_dmabuf_mgr_fd_ioctl,
>> + .mmap = intel_vgpu_dmabuf_mgr_fd_mmap,
>> + .llseek = noop_llseek,
>> +};
>> static int intel_vgpu_create(struct kobject *kobj, struct mdev_device
>> *mdev) {
>> struct intel_vgpu *vgpu = NULL;
>> @@ -1259,6 +1320,34 @@ static long intel_vgpu_ioctl(struct mdev_device
>*mdev, unsigned int cmd,
>> } else if (cmd == VFIO_DEVICE_RESET) {
>> intel_gvt_ops->vgpu_reset(vgpu);
>> return 0;
>> + } else if (cmd == VFIO_DEVICE_GET_FD) {
>> + int fd;
>> + u32 type;
>> + struct vfio_device *device;
>> +
>> + if (copy_from_user(&type, (void __user *)arg, sizeof(type)))
>> + return -EINVAL;
>> + if (type != INTEL_VGPU_DMABUF_MGR_FD) {
>
>Yet more unnecessary Intel specific code. Userspace doesn't want to handle a
>dmabuf differently between Intel, AMD, and NVIDIA, so if we have no reason to
>need vendor specific ioctl and parameters, then don't.
Yes. I will change in v3.
>
>> + gvt_vgpu_err("not supported fd type:%d\n", type);
>
>Just return error, users can abuse any logging they can get to.
OK.
>
>> + return -EINVAL;
>> + }
>> +
>> + fd = anon_inode_getfd("intel-vgpu-dmabuf-mgr-fd",
>> + &intel_vgpu_dmabuf_mgr_fd_ops,
>> + vgpu, O_RDWR | O_CLOEXEC);
>
>And this ordering is still wrong, do this last so you don't need to worry about
>getting the vfio_device reference below failing and calling release unnecessarily
>(though it seems this fd is leaked in that case anyway).
Will change in v3.
>
>> + if (fd < 0) {
>> + gvt_vgpu_err("create dmabuf mgr fd failed\n");
>> + return -EINVAL;
>> + }
>> +
>> + device = vfio_device_get_from_dev(mdev_dev(mdev));
>> + if (device == NULL) {
>> + gvt_vgpu_err("kvmgt: vfio device is null\n");
>> + return -EINVAL;
>> + }
>> + vgpu->vdev.vfio_device = device;
>> +
>> + return fd;
>> }
>>
>> return 0;
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index cde4f8e..9d28433 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -1466,6 +1466,8 @@ struct intel_vgpu_dmabuf {
>> __u32 tiled;
>> };
>>
>> +#define INTEL_VGPU_DMABUF_MGR_FD 0
>> +
>> #if defined(__cplusplus)
>> }
>> #endif
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index ae46105..c81500b 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -502,6 +502,18 @@ 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)
>> +
>
>The known input types would need to be defined here too. Definition of the
>ioctls available on that type should also be documented here.
Yes. Will do it in v3.
Thanks.
>
>> /* -------- API for Type1 VFIO IOMMU -------- */
>>
>> /**
^ permalink raw reply [flat|nested] 13+ messages in thread