Re: [Intel-gfx] [PATCH v2] drm/i915/gvt: Fix cached atomics setting for Windows VM

[Colin Xu]

On Fri, 6 Aug 2021, Zhenyu Wang wrote:

Thanks for the fix! Otherwise Windows VM is unusable with recent kernel.

Reviewed-by: Colin Xu 


We've seen recent regression with host and windows VM running
simultaneously that cause gpu hang or even crash. Finally bisect to
commit 58586680ffad ("drm/i915: Disable atomics in L3 for gen9"),
which seems cached atomics behavior difference caused regression
issue.

This tries to add new scratch register handler and add those in mmio
save/restore list for context switch. No gpu hang produced with this one.

Cc: sta...@vger.kernel.org # 5.12+
Cc: "Xu, Terrence" 
Cc: "Bloomfield, Jon" 
Cc: "Ekstrand, Jason" 
Fixes: 58586680ffad ("drm/i915: Disable atomics in L3 for gen9")
Signed-off-by: Zhenyu Wang 
---
drivers/gpu/drm/i915/gvt/handlers.c | 1 +
drivers/gpu/drm/i915/gvt/mmio_context.c | 2 ++
2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
b/drivers/gpu/drm/i915/gvt/handlers.c
index 06024d321a1a..cde0a477fb49 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -3149,6 +3149,7 @@ static int init_bdw_mmio_info(struct intel_gvt *gvt)
MMIO_DFH(_MMIO(0xb100), D_BDW, F_CMD_ACCESS, NULL, NULL);
MMIO_DFH(_MMIO(0xb10c), D_BDW, F_CMD_ACCESS, NULL, NULL);
MMIO_D(_MMIO(0xb110), D_BDW);
+   MMIO_D(GEN9_SCRATCH_LNCF1, D_BDW_PLUS);

MMIO_F(_MMIO(0x24d0), 48, F_CMD_ACCESS | F_CMD_WRITE_PATCH, 0, 0,
D_BDW_PLUS, NULL, force_nonpriv_write);
diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c 
b/drivers/gpu/drm/i915/gvt/mmio_context.c
index b8ac80765461..f776c470914d 100644
--- a/drivers/gpu/drm/i915/gvt/mmio_context.c
+++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
@@ -105,6 +105,8 @@ static struct engine_mmio gen9_engine_mmio_list[] 
__cacheline_aligned = {
{RCS0, COMMON_SLICE_CHICKEN2, 0x, true}, /* 0x7014 */
{RCS0, GEN9_CS_DEBUG_MODE1, 0x, false}, /* 0x20ec */
{RCS0, GEN8_L3SQCREG4, 0, false}, /* 0xb118 */
+   {RCS0, GEN9_SCRATCH1, 0, false}, /* 0xb11c */
+   {RCS0, GEN9_SCRATCH_LNCF1, 0, false}, /* 0xb008 */
{RCS0, GEN7_HALF_SLICE_CHICKEN1, 0x, true}, /* 0xe100 */
{RCS0, HALF_SLICE_CHICKEN2, 0x, true}, /* 0xe180 */
{RCS0, HALF_SLICE_CHICKEN3, 0x, true}, /* 0xe184 */
--
2.32.0.rc2




--
Best Regards,
Colin Xu

Re: [Intel-gfx] [PATCH 3/3] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV"

[Colin Xu]

On Tue, 11 May 2021, Zhenyu Wang wrote:


This reverts commit 07e543f4f9d116d6b4240644191dee6388ef4a85.

As I915_GVT dependency issue is resolved, revert this.

Cc: Jason Gunthorpe 
Signed-off-by: Zhenyu Wang 
---
drivers/gpu/drm/i915/Kconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 8f15bfb5faac..93f4d059fc89 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -102,7 +102,6 @@ config DRM_I915_GVT
bool "Enable Intel GVT-g graphics virtualization host support"
depends on DRM_I915
depends on 64BIT
-   depends on VFIO_MDEV
default n
help
  Choose this option if you want to enable Intel GVT-g graphics
--
2.31.0

___
intel-gvt-dev mailing list
intel-gvt-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev



Reviewed-by: Colin Xu 

--
Best Regards,
Colin Xu
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/3] Revert "vfio/gvt: fix DRM_I915_GVT dependency on VFIO_MDEV"

[Colin Xu]

On Tue, 11 May 2021, Zhenyu Wang wrote:


This reverts commit adaeb718d46f6b42a3fc1dffd4f946f26b33779a.

As I915_GVT dependency is resolved, revert this temporary hack.

Cc: Arnd Bergmann 
Signed-off-by: Zhenyu Wang 
---
drivers/gpu/drm/i915/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 69f57ca9c68d..8f15bfb5faac 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -102,7 +102,7 @@ config DRM_I915_GVT
bool "Enable Intel GVT-g graphics virtualization host support"
depends on DRM_I915
depends on 64BIT
-   depends on VFIO_MDEV=y || VFIO_MDEV=DRM_I915
+   depends on VFIO_MDEV
default n
help
  Choose this option if you want to enable Intel GVT-g graphics
--
2.31.0

___
intel-gvt-dev mailing list
intel-gvt-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev



Reviewed-by: Colin Xu 

--
Best Regards,
Colin Xu
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915/gvt: Move mdev attribute groups into kvmgt module

[Colin Xu]

el_vgpu_type *type;
+   struct intel_gvt *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
+
+   type = &gvt->types[mtype_get_type_group_id(mtype)];
+   if (!type)
+   return 0;
+
+   return sprintf(buf, "low_gm_size: %dMB\nhigh_gm_size: %dMB\n"
+  "fence: %d\nresolution: %s\n"
+  "weight: %d\n",
+  BYTES_TO_MB(type->low_gm_size),
+  BYTES_TO_MB(type->high_gm_size),
+  type->fence, vgpu_edid_str(type->resolution),
+  type->weight);
+}
+
+static MDEV_TYPE_ATTR_RO(available_instances);
+static MDEV_TYPE_ATTR_RO(device_api);
+static MDEV_TYPE_ATTR_RO(description);
+
+static struct attribute *gvt_type_attrs[] = {
+   &mdev_type_attr_available_instances.attr,
+   &mdev_type_attr_device_api.attr,
+   &mdev_type_attr_description.attr,
+   NULL,
+};
+
+static struct attribute_group *gvt_vgpu_type_groups[] = {
+   [0 ... NR_MAX_INTEL_VGPU_TYPES - 1] = NULL,
+};
+
+static int intel_gvt_init_vgpu_type_groups(struct intel_gvt *gvt)
+{
+   int i, j;
+   struct intel_vgpu_type *type;
+   struct attribute_group *group;
+
+   for (i = 0; i < gvt->num_types; i++) {
+   type = &gvt->types[i];
+
+   group = kzalloc(sizeof(struct attribute_group), GFP_KERNEL);
+   if (!group)
+   goto unwind;
+
+   group->name = type->name;
+   group->attrs = gvt_type_attrs;
+   gvt_vgpu_type_groups[i] = group;
+   }
+
+   return 0;
+
+unwind:
+   for (j = 0; j < i; j++) {
+   group = gvt_vgpu_type_groups[j];
+   kfree(group);
+   }
+
+   return -ENOMEM;
+}
+
+static void intel_gvt_cleanup_vgpu_type_groups(struct intel_gvt *gvt)
+{
+   int i;
+   struct attribute_group *group;
+
+   for (i = 0; i < gvt->num_types; i++) {
+   group = gvt_vgpu_type_groups[i];
+   gvt_vgpu_type_groups[i] = NULL;
+   kfree(group);
+   }
+}
+
static int kvmgt_guest_init(struct mdev_device *mdev);
static void intel_vgpu_release_work(struct work_struct *work);
static bool kvmgt_guest_exit(struct kvmgt_guest_info *info);
@@ -694,14 +792,13 @@ static int intel_vgpu_create(struct mdev_device *mdev)
struct intel_vgpu *vgpu = NULL;
struct intel_vgpu_type *type;
struct device *pdev;
-   void *gvt;
+   struct intel_gvt *gvt;
int ret;

pdev = mdev_parent_dev(mdev);
gvt = kdev_to_i915(pdev)->gvt;

-   type = intel_gvt_ops->gvt_find_vgpu_type(gvt,
-mdev_get_type_group_id(mdev));
+   type = &gvt->types[mdev_get_type_group_id(mdev)];
if (!type) {
ret = -EINVAL;
goto out;
@@ -1667,19 +1764,26 @@ static struct mdev_parent_ops intel_vgpu_ops = {

static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
{
-   struct attribute_group **kvm_vgpu_type_groups;
+   int ret;
+
+   ret = intel_gvt_init_vgpu_type_groups((struct intel_gvt *)gvt);
+   if (ret)
+   return ret;

intel_gvt_ops = ops;
-   if (!intel_gvt_ops->get_gvt_attrs(&kvm_vgpu_type_groups))
-   return -EFAULT;
-   intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
+   intel_vgpu_ops.supported_type_groups = gvt_vgpu_type_groups;

-   return mdev_register_device(dev, &intel_vgpu_ops);
+   ret = mdev_register_device(dev, &intel_vgpu_ops);
+   if (ret)
+   intel_gvt_cleanup_vgpu_type_groups((struct intel_gvt *)gvt);
+
+   return ret;
}

-static void kvmgt_host_exit(struct device *dev)
+static void kvmgt_host_exit(struct device *dev, void *gvt)
{
mdev_unregister_device(dev);
+   intel_gvt_cleanup_vgpu_type_groups((struct intel_gvt *)gvt);
}

static int kvmgt_page_track_add(unsigned long handle, u64 gfn)
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index 550a456e936f..e6c5a792a49a 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -63,13 +63,13 @@ static inline int intel_gvt_hypervisor_host_init(struct 
device *dev,
/**
 * intel_gvt_hypervisor_host_exit - exit GVT-g host side
 */
-static inline void intel_gvt_hypervisor_host_exit(struct device *dev)
+static inline void intel_gvt_hypervisor_host_exit(struct device *dev, void 
*gvt)
{
/* optional to provide */
if (!intel_gvt_host.mpt->host_exit)
    return;

-   intel_gvt_host.mpt->host_exit(dev);
+   intel_gvt_host.mpt->host_exit(dev, gvt);
}

/**
--
2.31.0

___
intel-gvt-dev mailing list
intel-gvt-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev



Reviewed-by: Colin Xu 
--
Best Regards,
Colin Xu
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v7 2/2] drm/i915/gvt: Add GVT resume routine to i915

[Colin Xu]

This patch add gvt resume wrapper into i915_drm_resume().
GVT relies on i915 so resume gvt at last.

V2:
- Direct call into gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c.
The wrapper and implementation will check and call gvt routine. (zhenyu)

V3:
Refresh.

V4:
Rebase.

V5:
Fail intel_gvt_suspend() if fail to save GGTT.

V6:
Save host entry to per-vGPU gtt.ggtt_mm on each host_entry update so
only need the resume routine.

V7:
Refresh.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/i915_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d3237b0d821d..2c15c9440f8a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1270,6 +1270,8 @@ static int i915_drm_resume(struct drm_device *dev)
 
intel_power_domains_enable(dev_priv);
 
+   intel_gvt_resume(dev_priv);
+
enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
return 0;
-- 
2.29.1

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

[Intel-gfx] [PATCH v7 0/2] Enable GVT suspend/resume

[Colin Xu]

This patchset enables GVT suspend/resume so that GVT enabled VM can
continue running after host resuming from suspend state.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

V5:
Fail intel_gvt_pm_suspend if fail to save ggtt.

V6:
Save host entry to per-vGPU gtt.ggtt_mm on each host_entry update so
that no need to read from HW and save during suspend.

V7:
Restore GGTT entry based on present bit.
Split fence restore and mmio restore in different functions.

Colin Xu (2):
  drm/i915/gvt: Save/restore HW status to support GVT suspend/resume
  drm/i915/gvt: Add GVT resume routine to i915

 drivers/gpu/drm/i915/gvt/gtt.c  | 64 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  4 ++
 drivers/gpu/drm/i915/gvt/gvt.c  |  9 
 drivers/gpu/drm/i915/gvt/gvt.h  |  3 ++
 drivers/gpu/drm/i915/gvt/handlers.c | 44 ++--
 drivers/gpu/drm/i915/gvt/mmio.h |  4 ++
 drivers/gpu/drm/i915/i915_drv.c |  2 +
 drivers/gpu/drm/i915/intel_gvt.c| 15 +++
 drivers/gpu/drm/i915/intel_gvt.h|  5 +++
 9 files changed, 147 insertions(+), 3 deletions(-)

-- 
2.29.1

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

[Intel-gfx] [PATCH v7 1/2] drm/i915/gvt: Save/restore HW status to support GVT suspend/resume

[Colin Xu]

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries on each host_entry update, restore the saved dirty entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

V5:
Fail intel_gvt_save_ggtt as -ENOMEM if fail to alloc memory to save
ggtt. Free allocated ggtt_entries on failure.

V6:
Save host entry to per-vGPU gtt.ggtt_mm on each host_entry update.

V7:
Restore GGTT entry based on present bit.
Split fence restore and mmio restore in different functions.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/gtt.c  | 64 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  4 ++
 drivers/gpu/drm/i915/gvt/gvt.c  |  9 
 drivers/gpu/drm/i915/gvt/gvt.h  |  3 ++
 drivers/gpu/drm/i915/gvt/handlers.c | 44 ++--
 drivers/gpu/drm/i915/gvt/mmio.h |  4 ++
 drivers/gpu/drm/i915/intel_gvt.c| 15 +++
 drivers/gpu/drm/i915/intel_gvt.h|  5 +++
 8 files changed, 145 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index a3a4305eda01..897c007ea96a 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -636,9 +636,18 @@ static void ggtt_set_host_entry(struct intel_vgpu_mm *mm,
struct intel_gvt_gtt_entry *entry, unsigned long index)
 {
struct intel_gvt_gtt_pte_ops *pte_ops = mm->vgpu->gvt->gtt.pte_ops;
+   unsigned long offset = index;
 
GEM_BUG_ON(mm->type != INTEL_GVT_MM_GGTT);
 
+   if (vgpu_gmadr_is_aperture(mm->vgpu, index << I915_GTT_PAGE_SHIFT)) {
+   offset -= (vgpu_aperture_gmadr_base(mm->vgpu) >> PAGE_SHIFT);
+   mm->ggtt_mm.host_ggtt_aperture[offset] = entry->val64;
+   } else if (vgpu_gmadr_is_hidden(mm->vgpu, index << 
I915_GTT_PAGE_SHIFT)) {
+   offset -= (vgpu_hidden_gmadr_base(mm->vgpu) >> PAGE_SHIFT);
+   mm->ggtt_mm.host_ggtt_hidden[offset] = entry->val64;
+   }
+
pte_ops->set_entry(NULL, entry, index, false, 0, mm->vgpu);
 }
 
@@ -1944,6 +1953,21 @@ static struct intel_vgpu_mm 
*intel_vgpu_create_ggtt_mm(struct intel_vgpu *vgpu)
return ERR_PTR(-ENOMEM);
}
 
+   mm->ggtt_mm.host_ggtt_aperture = vzalloc((vgpu_aperture_sz(vgpu) >> 
PAGE_SHIFT) * sizeof(u64));
+   if (!mm->ggtt_mm.host_ggtt_aperture) {
+   vfree(mm->ggtt_mm.virtual_ggtt);
+   vgpu_free_mm(mm);
+   return ERR_PTR(-ENOMEM);
+   }
+
+   mm->ggtt_mm.host_ggtt_hidden = vzalloc((vgpu_hidden_sz(vgpu) >> 
PAGE_SHIFT) * sizeof(u64));
+   if (!mm->ggtt_mm.host_ggtt_hidden) {
+   vfree(mm->ggtt_mm.host_ggtt_aperture);
+   vfree(mm->ggtt_mm.virtual_ggtt);
+   vgpu_free_mm(mm);
+   return ERR_PTR(-ENOMEM);
+   }
+
return mm;
 }
 
@@ -1971,6 +1995,8 @@ void _intel_vgpu_mm_release(struct kref *mm_ref)
invalidate_ppgtt_mm(mm);
} else {
vfree(mm->ggtt_mm.virtual_ggtt);
+   vfree(mm->ggtt_mm.host_ggtt_aperture);
+   vfree(mm->ggtt_mm.host_ggtt_hidden);
}
 
vgpu_free_mm(mm);
@@ -2852,3 +2878,41 @@ void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu)
intel_vgpu_destroy_all_ppgtt_mm(vgpu);
intel_vgpu_reset_ggtt(vgpu, true);
 }
+
+/**
+ * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver resume stage to restore
+ * GGTT entries of every vGPU.
+ *
+ */
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   struct intel_vgpu_mm *mm;
+   int id;
+   gen8_pte_t pte;
+   u32 idx, num_low, num_hi, offset;
+
+   /* Restore dirty host ggtt for all vGPUs */
+   idr_for_each_entry(&(gvt)->vgpu_idr, vgpu, id) {
+   mm = vgpu->gtt.ggtt_mm;
+
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   offset = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   for (idx = 0; idx < num_low; idx++) {
+   pte = mm->ggtt_mm.host_ggtt_apertu

Re: [Intel-gfx] [PATCH v6 1/2] drm/i915/gvt: Save/restore HW status to support GVT suspend/resume

[Colin Xu]

On 2020-10-27 09:49, Zhenyu Wang wrote:

On 2020.10.27 08:42:40 +0800, Colin Xu wrote:

On 2020-10-26 17:19, Zhenyu Wang wrote:

On 2020.10.23 16:17:19 +0800, Colin Xu wrote:

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries on each host_entry update, restore the saved dirty entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

V5:
Fail intel_gvt_save_ggtt as -ENOMEM if fail to alloc memory to save
ggtt. Free allocated ggtt_entries on failure.

V6:
Save host entry to per-vGPU gtt.ggtt_mm on each host_entry update.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
   drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
   drivers/gpu/drm/i915/gvt/gtt.h  |  9 
   drivers/gpu/drm/i915/gvt/gvt.c  |  8 +++
   drivers/gpu/drm/i915/gvt/gvt.h  |  3 ++
   drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
   drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
   drivers/gpu/drm/i915/intel_gvt.c| 15 ++
   drivers/gpu/drm/i915/intel_gvt.h|  5 ++
   8 files changed, 154 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index a3a4305eda01..33385d640cb9 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -636,9 +636,20 @@ static void ggtt_set_host_entry(struct intel_vgpu_mm *mm,
struct intel_gvt_gtt_entry *entry, unsigned long index)
   {
struct intel_gvt_gtt_pte_ops *pte_ops = mm->vgpu->gvt->gtt.pte_ops;
+   unsigned long offset = index;
GEM_BUG_ON(mm->type != INTEL_GVT_MM_GGTT);
+   if (vgpu_gmadr_is_aperture(mm->vgpu, index << I915_GTT_PAGE_SHIFT)) {
+   offset -= (vgpu_aperture_gmadr_base(mm->vgpu) >> PAGE_SHIFT);
+   mm->ggtt_mm.host_aperture[offset].val64 = entry->val64;
+   mm->ggtt_mm.host_aperture[offset].dirty = true;
+   } else if (vgpu_gmadr_is_hidden(mm->vgpu, index << 
I915_GTT_PAGE_SHIFT)) {
+   offset -= (vgpu_hidden_gmadr_base(mm->vgpu) >> PAGE_SHIFT);
+   mm->ggtt_mm.host_hidden[offset].val64 = entry->val64;
+   mm->ggtt_mm.host_hidden[offset].dirty = true;
+   }

Looks 'dirty' flag is not needed at all, as what you need is to restore all 
present
entries, so can just check present bit of val64 for that.

Strictly it's different. Host could update a page which isn't present but
other PTE fields are still valid, this is "present". For dirty it's purpose
is to record whether or not host has update this entry, regardless the page
presence. If only check the present bit, there will be a case that previous
PTE points to region that shouldn't be walked by the new PDE, but new
PDE/PTE update only updates PDE w/ present, but PTE w/o present. That could
leak information by new PDE + old PTE. Hypervisor should follow guest
behavior to update all PDE/PTEs, and guest should make sure partial update
won't leak information. If hypervisor doesn't follow guest page update,
information could be leaked.

But this is only for GGTT which is just single level table right?


And actually, when vGPU created, all GGTT pages are updates so in practice,
all entries are dirty. So from real practice, it's true that dirty check can
be removed.

You mean guest would update all entries normally? That's ok which we just follow
guest's behavior. And we've also seen guest partial update of GGTT entry which
could be point for suspend/resume and we reset present bit at that time so can 
be
skipped in this case.
Yes guest would update all. So should I check present again, or just 
save/restore all?



+
pte_ops->set_entry(NULL, entry, index, false, 0, mm->vgpu);
   }
@@ -1928,6 +1939,7 @@ static struct intel_vgpu_mm 
*intel_vgpu_create_ggtt_mm(struct intel_vgpu *vgpu)
   {
struct intel_vgpu_mm *mm;
unsigned long nr_entries;
+   u64 size;
mm = vgpu_alloc_mm(vgpu);
if (!mm)
@@ -1944,6 +1956,25 @@ static struct intel_vgpu_mm 
*intel_vgpu_create_ggtt_mm(struct intel_vgpu *vgpu)
return ERR_PTR(-ENOMEM);
}
+   size = (vgpu_aperture_sz(vgpu) >> PAGE_SHIFT) *
+   sizeof(struct intel_vgpu_ho

Re: [Intel-gfx] [PATCH v6 1/2] drm/i915/gvt: Save/restore HW status to support GVT suspend/resume

[Colin Xu]

On 2020-10-26 17:19, Zhenyu Wang wrote:

On 2020.10.23 16:17:19 +0800, Colin Xu wrote:

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries on each host_entry update, restore the saved dirty entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

V5:
Fail intel_gvt_save_ggtt as -ENOMEM if fail to alloc memory to save
ggtt. Free allocated ggtt_entries on failure.

V6:
Save host entry to per-vGPU gtt.ggtt_mm on each host_entry update.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
  drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
  drivers/gpu/drm/i915/gvt/gtt.h  |  9 
  drivers/gpu/drm/i915/gvt/gvt.c  |  8 +++
  drivers/gpu/drm/i915/gvt/gvt.h  |  3 ++
  drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
  drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
  drivers/gpu/drm/i915/intel_gvt.c| 15 ++
  drivers/gpu/drm/i915/intel_gvt.h|  5 ++
  8 files changed, 154 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index a3a4305eda01..33385d640cb9 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -636,9 +636,20 @@ static void ggtt_set_host_entry(struct intel_vgpu_mm *mm,
struct intel_gvt_gtt_entry *entry, unsigned long index)
  {
struct intel_gvt_gtt_pte_ops *pte_ops = mm->vgpu->gvt->gtt.pte_ops;
+   unsigned long offset = index;
  
  	GEM_BUG_ON(mm->type != INTEL_GVT_MM_GGTT);
  
+	if (vgpu_gmadr_is_aperture(mm->vgpu, index << I915_GTT_PAGE_SHIFT)) {

+   offset -= (vgpu_aperture_gmadr_base(mm->vgpu) >> PAGE_SHIFT);
+   mm->ggtt_mm.host_aperture[offset].val64 = entry->val64;
+   mm->ggtt_mm.host_aperture[offset].dirty = true;
+   } else if (vgpu_gmadr_is_hidden(mm->vgpu, index << 
I915_GTT_PAGE_SHIFT)) {
+   offset -= (vgpu_hidden_gmadr_base(mm->vgpu) >> PAGE_SHIFT);
+   mm->ggtt_mm.host_hidden[offset].val64 = entry->val64;
+   mm->ggtt_mm.host_hidden[offset].dirty = true;
+   }

Looks 'dirty' flag is not needed at all, as what you need is to restore all 
present
entries, so can just check present bit of val64 for that.


Strictly it's different. Host could update a page which isn't present 
but other PTE fields are still valid, this is "present". For dirty it's 
purpose is to record whether or not host has update this entry, 
regardless the page presence. If only check the present bit, there will 
be a case that previous PTE points to region that shouldn't be walked by 
the new PDE, but new PDE/PTE update only updates PDE w/ present, but PTE 
w/o present. That could leak information by new PDE + old PTE. 
Hypervisor should follow guest behavior to update all PDE/PTEs, and 
guest should make sure partial update won't leak information. If 
hypervisor doesn't follow guest page update, information could be leaked.


And actually, when vGPU created, all GGTT pages are updates so in 
practice, all entries are dirty. So from real practice, it's true that 
dirty check can be removed.





+
pte_ops->set_entry(NULL, entry, index, false, 0, mm->vgpu);
  }
  
@@ -1928,6 +1939,7 @@ static struct intel_vgpu_mm *intel_vgpu_create_ggtt_mm(struct intel_vgpu *vgpu)

  {
struct intel_vgpu_mm *mm;
unsigned long nr_entries;
+   u64 size;
  
  	mm = vgpu_alloc_mm(vgpu);

if (!mm)
@@ -1944,6 +1956,25 @@ static struct intel_vgpu_mm 
*intel_vgpu_create_ggtt_mm(struct intel_vgpu *vgpu)
return ERR_PTR(-ENOMEM);
}
  
+	size = (vgpu_aperture_sz(vgpu) >> PAGE_SHIFT) *

+   sizeof(struct intel_vgpu_host_ggtt);
+   mm->ggtt_mm.host_aperture = vzalloc(size);

I think normally just write required size calculation in alloc parameter
instead of in extra variable.
Only for trim line length within 80 otherwise align to parentheses 
doesn't look clean. Since kernel has 80 columns as the 'strong preferred 
limit', if it's OK I'll put it into same line then no extra variable is 
required.



+   if (!mm->ggtt_mm.host_aperture) {
+   vfree(mm->ggtt_mm.virtual_ggtt);
+  

[Intel-gfx] [PATCH v6 2/2] drm/i915/gvt: Add GVT resume routine to i915

[Colin Xu]

This patch add gvt resume wrapper into i915_drm_resume().
GVT relies on i915 so resume gvt at last.

V2:
- Direct call into gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c.
The wrapper and implementation will check and call gvt routine. (zhenyu)

V3:
Refresh.

V4:
Rebase.

V5:
Fail intel_gvt_suspend() if fail to save GGTT.

V6:
Save host entry to per-vGPU gtt.ggtt_mm on each host_entry update so
only need the resume routine.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/i915_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d3237b0d821d..2c15c9440f8a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1270,6 +1270,8 @@ static int i915_drm_resume(struct drm_device *dev)
 
intel_power_domains_enable(dev_priv);
 
+   intel_gvt_resume(dev_priv);
+
enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
return 0;
-- 
2.29.0

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

[Intel-gfx] [PATCH v6 0/2] Enable GVT suspend/resume

[Colin Xu]

This patchset enables GVT suspend/resume so that GVT enabled VM can
continue running after host resuming from suspend state.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

V5:
Fail intel_gvt_pm_suspend if fail to save ggtt.

V6:
Save host entry to per-vGPU gtt.ggtt_mm on each host_entry update so
that no need to read from HW and save during suspend.

Colin Xu (2):
  drm/i915/gvt: Save/restore HW status to support GVT suspend/resume
  drm/i915/gvt: Add GVT resume routine to i915

 drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  9 
 drivers/gpu/drm/i915/gvt/gvt.c  |  8 +++
 drivers/gpu/drm/i915/gvt/gvt.h  |  3 ++
 drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
 drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
 drivers/gpu/drm/i915/i915_drv.c |  2 +
 drivers/gpu/drm/i915/intel_gvt.c| 15 ++
 drivers/gpu/drm/i915/intel_gvt.h|  5 ++
 9 files changed, 156 insertions(+), 3 deletions(-)

-- 
2.29.0

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

[Intel-gfx] [PATCH v6 1/2] drm/i915/gvt: Save/restore HW status to support GVT suspend/resume

[Colin Xu]

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries on each host_entry update, restore the saved dirty entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

V5:
Fail intel_gvt_save_ggtt as -ENOMEM if fail to alloc memory to save
ggtt. Free allocated ggtt_entries on failure.

V6:
Save host entry to per-vGPU gtt.ggtt_mm on each host_entry update.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  9 
 drivers/gpu/drm/i915/gvt/gvt.c  |  8 +++
 drivers/gpu/drm/i915/gvt/gvt.h  |  3 ++
 drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
 drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
 drivers/gpu/drm/i915/intel_gvt.c| 15 ++
 drivers/gpu/drm/i915/intel_gvt.h|  5 ++
 8 files changed, 154 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index a3a4305eda01..33385d640cb9 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -636,9 +636,20 @@ static void ggtt_set_host_entry(struct intel_vgpu_mm *mm,
struct intel_gvt_gtt_entry *entry, unsigned long index)
 {
struct intel_gvt_gtt_pte_ops *pte_ops = mm->vgpu->gvt->gtt.pte_ops;
+   unsigned long offset = index;
 
GEM_BUG_ON(mm->type != INTEL_GVT_MM_GGTT);
 
+   if (vgpu_gmadr_is_aperture(mm->vgpu, index << I915_GTT_PAGE_SHIFT)) {
+   offset -= (vgpu_aperture_gmadr_base(mm->vgpu) >> PAGE_SHIFT);
+   mm->ggtt_mm.host_aperture[offset].val64 = entry->val64;
+   mm->ggtt_mm.host_aperture[offset].dirty = true;
+   } else if (vgpu_gmadr_is_hidden(mm->vgpu, index << 
I915_GTT_PAGE_SHIFT)) {
+   offset -= (vgpu_hidden_gmadr_base(mm->vgpu) >> PAGE_SHIFT);
+   mm->ggtt_mm.host_hidden[offset].val64 = entry->val64;
+   mm->ggtt_mm.host_hidden[offset].dirty = true;
+   }
+
pte_ops->set_entry(NULL, entry, index, false, 0, mm->vgpu);
 }
 
@@ -1928,6 +1939,7 @@ static struct intel_vgpu_mm 
*intel_vgpu_create_ggtt_mm(struct intel_vgpu *vgpu)
 {
struct intel_vgpu_mm *mm;
unsigned long nr_entries;
+   u64 size;
 
mm = vgpu_alloc_mm(vgpu);
if (!mm)
@@ -1944,6 +1956,25 @@ static struct intel_vgpu_mm 
*intel_vgpu_create_ggtt_mm(struct intel_vgpu *vgpu)
return ERR_PTR(-ENOMEM);
}
 
+   size = (vgpu_aperture_sz(vgpu) >> PAGE_SHIFT) *
+   sizeof(struct intel_vgpu_host_ggtt);
+   mm->ggtt_mm.host_aperture = vzalloc(size);
+   if (!mm->ggtt_mm.host_aperture) {
+   vfree(mm->ggtt_mm.virtual_ggtt);
+   vgpu_free_mm(mm);
+   return ERR_PTR(-ENOMEM);
+   }
+
+   size = (vgpu_hidden_sz(vgpu) >> PAGE_SHIFT) *
+   sizeof(struct intel_vgpu_host_ggtt);
+   mm->ggtt_mm.host_hidden = vzalloc(size);
+   if (!mm->ggtt_mm.host_hidden) {
+   vfree(mm->ggtt_mm.host_aperture);
+   vfree(mm->ggtt_mm.virtual_ggtt);
+   vgpu_free_mm(mm);
+   return ERR_PTR(-ENOMEM);
+   }
+
return mm;
 }
 
@@ -1971,6 +2002,8 @@ void _intel_vgpu_mm_release(struct kref *mm_ref)
invalidate_ppgtt_mm(mm);
} else {
vfree(mm->ggtt_mm.virtual_ggtt);
+   vfree(mm->ggtt_mm.host_aperture);
+   vfree(mm->ggtt_mm.host_hidden);
}
 
vgpu_free_mm(mm);
@@ -2852,3 +2885,45 @@ void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu)
intel_vgpu_destroy_all_ppgtt_mm(vgpu);
intel_vgpu_reset_ggtt(vgpu, true);
 }
+
+/**
+ * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver resume stage to restore
+ * GGTT entries of every vGPU.
+ *
+ */
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   struct intel_vgpu_mm *mm;
+   struct intel_vgpu_host_ggtt *host_ggtt;
+   int id;
+   u32 idx, num_low, num_hi, offset;
+
+   /* Restore dirty host ggtt for all vGPUs */
+   idr_for_each_entry(&(gvt)->

Re: [Intel-gfx] [PATCH v5 2/2] drm/i915/gvt: Add GVT suspend/resume routine to i915

[Colin Xu]

On 2020-10-16 16:13, Colin Xu wrote:

This patch add gvt suspend/resume wrapper into i915: i915_drm_suspend()
and i915_drm_resume(). GVT relies on i915 so suspend gvt ahead of other
i915 sub-routine and resume gvt at last.

V2:
- Direct call into gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c.
The wrapper and implementation will check and call gvt routine. (zhenyu)

V3:
Refresh.

V4:
Rebase.

V5:
Fail intel_gvt_suspend() if fail to save GGTT.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
  drivers/gpu/drm/i915/i915_drv.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8bb7e2dcfaaa..b3203292b0ee 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1104,6 +1104,9 @@ static int i915_drm_suspend(struct drm_device *dev)
  
  	drm_kms_helper_poll_disable(dev);
  
+	if (intel_gvt_suspend(dev_priv))

+   drm_err(&dev_priv->drm, "failed to suspend GVT\n");
I'm not quite sure if it's OK to fail i915_drm_suspend() here if 
intel_gvt_suspend() fails. I saw intel_display_suspend() may also return 
failure but i915_drm_suspend() doesn't handle such case so I just follow 
the style for intel_gvt_suspend().

+
pci_save_state(pdev);
  
  	intel_display_suspend(dev);

@@ -1281,6 +1284,8 @@ static int i915_drm_resume(struct drm_device *dev)
  
  	intel_power_domains_enable(dev_priv);
  
+	intel_gvt_resume(dev_priv);

+
enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
  
  	return 0;


--
Best Regards,
Colin Xu

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

[Intel-gfx] [PATCH v5 2/2] drm/i915/gvt: Add GVT suspend/resume routine to i915

[Colin Xu]

This patch add gvt suspend/resume wrapper into i915: i915_drm_suspend()
and i915_drm_resume(). GVT relies on i915 so suspend gvt ahead of other
i915 sub-routine and resume gvt at last.

V2:
- Direct call into gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c.
The wrapper and implementation will check and call gvt routine. (zhenyu)

V3:
Refresh.

V4:
Rebase.

V5:
Fail intel_gvt_suspend() if fail to save GGTT.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/i915_drv.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8bb7e2dcfaaa..b3203292b0ee 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1104,6 +1104,9 @@ static int i915_drm_suspend(struct drm_device *dev)
 
drm_kms_helper_poll_disable(dev);
 
+   if (intel_gvt_suspend(dev_priv))
+   drm_err(&dev_priv->drm, "failed to suspend GVT\n");
+
pci_save_state(pdev);
 
intel_display_suspend(dev);
@@ -1281,6 +1284,8 @@ static int i915_drm_resume(struct drm_device *dev)
 
intel_power_domains_enable(dev_priv);
 
+   intel_gvt_resume(dev_priv);
+
enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
return 0;
-- 
2.28.0

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

[Intel-gfx] [PATCH v5 0/2] Enable GVT suspend/resume

[Colin Xu]

This patchset enables GVT suspend/resume so that GVT enabled VM can
continue running after host resuming from suspend state.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

V5:
Fail intel_gvt_pm_suspend if fail to save ggtt.

Colin Xu (2):
  drm/i915/gvt: Save/restore HW status for GVT during suspend/resume
  drm/i915/gvt: Add GVT suspend/resume routine to i915

 drivers/gpu/drm/i915/gvt/gtt.c  | 88 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
 drivers/gpu/drm/i915/gvt/gvt.c  | 14 +
 drivers/gpu/drm/i915/gvt/gvt.h  |  7 +++
 drivers/gpu/drm/i915/gvt/handlers.c | 39 -
 drivers/gpu/drm/i915/gvt/mmio.h |  3 +
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 drivers/gpu/drm/i915/i915_drv.c |  5 ++
 drivers/gpu/drm/i915/intel_gvt.c| 30 ++
 drivers/gpu/drm/i915/intel_gvt.h| 10 
 10 files changed, 196 insertions(+), 3 deletions(-)

-- 
2.28.0

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

[Intel-gfx] [PATCH v5 1/2] drm/i915/gvt: Save/restore HW status for GVT during suspend/resume

[Colin Xu]

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries into GVT in suspend routine, and restore the saved entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

V5:
Fail intel_gvt_save_ggtt as -ENOMEM if fail to alloc memory to save
ggtt. Free allocated ggtt_entries on failure.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/gtt.c  | 88 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
 drivers/gpu/drm/i915/gvt/gvt.c  | 14 +
 drivers/gpu/drm/i915/gvt/gvt.h  |  7 +++
 drivers/gpu/drm/i915/gvt/handlers.c | 39 -
 drivers/gpu/drm/i915/gvt/mmio.h |  3 +
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 drivers/gpu/drm/i915/intel_gvt.c| 30 ++
 drivers/gpu/drm/i915/intel_gvt.h| 10 
 9 files changed, 191 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index a3a4305eda01..3426d62c4f2c 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2533,6 +2533,11 @@ static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu 
*vgpu)
}
intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm);
vgpu->gtt.ggtt_mm = NULL;
+
+   if (vgpu->ggtt_entries) {
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
 }
 
 /**
@@ -2852,3 +2857,86 @@ void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu)
intel_vgpu_destroy_all_ppgtt_mm(vgpu);
intel_vgpu_reset_ggtt(vgpu, true);
 }
+
+/**
+ * intel_gvt_save_ggtt - save all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver suspend stage to save
+ * GGTT entries of every active vGPU.
+ *
+ */
+int intel_gvt_save_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+
+   vgpu->ggtt_entries = vzalloc((num_low + num_hi) * sizeof(u64));
+   if (!vgpu->ggtt_entries) {
+   gvt_vgpu_err("fail to alloc memory to save ggtt\n");
+   goto fail;
+   }
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries, addr, num_low * sizeof(u64));
+
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries + num_low,
+ addr, num_hi * sizeof(u64));
+   }
+
+   return 0;
+fail:
+   for_each_active_vgpu(gvt, vgpu, id) {
+   if (vgpu->ggtt_entries) {
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
+   }
+   return -ENOMEM;
+}
+
+/**
+ * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver resume stage to restore
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   if (!vgpu->ggtt_entries) {
+   gvt_vgpu_err("fail to get saved ggtt\n");
+   continue;
+   }
+
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_toio(addr, vgpu->ggtt_entries, num_low * sizeof(u64));
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/gvt: Save/restore HW status for GVT during suspend/resume

[Colin Xu]

On 2020-10-16 14:37, Zhenyu Wang wrote:

On 2020.10.16 14:20:29 +0800, Colin Xu wrote:

On 2020-10-16 13:54, Zhenyu Wang wrote:

On 2020.10.16 13:59:59 +0800, Colin Xu wrote:

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries into GVT in suspend routine, and restore the saved entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
   drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
   drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
   drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
   drivers/gpu/drm/i915/gvt/gvt.h  |  8 +++
   drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
   drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
   drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
   drivers/gpu/drm/i915/intel_gvt.c| 29 +++
   drivers/gpu/drm/i915/intel_gvt.h| 10 
   9 files changed, 179 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index a3a4305eda01..534bb2e08538 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2533,6 +2533,11 @@ static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu 
*vgpu)
}
intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm);
vgpu->gtt.ggtt_mm = NULL;
+
+   if (vgpu->ggtt_entries) {
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
   }
   /**
@@ -2852,3 +2857,73 @@ void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu)
intel_vgpu_destroy_all_ppgtt_mm(vgpu);
intel_vgpu_reset_ggtt(vgpu, true);
   }
+
+/**
+ * intel_gvt_save_ggtt - save all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver suspend stage to save
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_save_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+   vgpu->ggtt_entries = vzalloc((num_low + num_hi) * sizeof(u64));
+   if (!vgpu->ggtt_entries)
+   continue;

I think doing allocation in suspend is not good, that should prealloc
at vgpu create time and free at destroy.

  Yes Hang and I discussed about the two approaches. This buffer is only
useful during system suspend time for status backing store, but useless when
system in S0. If we move it to vgpu creation time, large piece of memory
could be wasted.

But looks error handling is problematic that just bypass possible failed vgpu,
which is not good...As gvt traps all vgpu ggtt access, could we just 
save/restore
trapped ones?


Yes that will be a problem. I'll add err handling here by fail save_ggtt 
and free the succeeded allocated ggtt_entries.


For only tracking trapped ggtt access, this looks unnecessary. 1st, gvt 
only have a list for partial update but doesn't have the full list of 
trapped ggtt access. 2nd, the memory is only taken during suspend state 
and will be freed after resume.





+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries, addr, num_low * sizeof(u64));
+
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries + num_low,
+ addr, num_hi * sizeof(u64));
+   }
+}
+
+/**
+ * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver resume stage to restore
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   if (!vgpu->ggtt_entries) {
+ 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/gvt: Save/restore HW status for GVT during suspend/resume

[Colin Xu]

On 2020-10-16 13:54, Zhenyu Wang wrote:

On 2020.10.16 13:59:59 +0800, Colin Xu wrote:

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries into GVT in suspend routine, and restore the saved entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
  drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
  drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
  drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
  drivers/gpu/drm/i915/gvt/gvt.h  |  8 +++
  drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
  drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
  drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
  drivers/gpu/drm/i915/intel_gvt.c| 29 +++
  drivers/gpu/drm/i915/intel_gvt.h| 10 
  9 files changed, 179 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index a3a4305eda01..534bb2e08538 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2533,6 +2533,11 @@ static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu 
*vgpu)
}
intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm);
vgpu->gtt.ggtt_mm = NULL;
+
+   if (vgpu->ggtt_entries) {
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
  }
  
  /**

@@ -2852,3 +2857,73 @@ void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu)
intel_vgpu_destroy_all_ppgtt_mm(vgpu);
intel_vgpu_reset_ggtt(vgpu, true);
  }
+
+/**
+ * intel_gvt_save_ggtt - save all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver suspend stage to save
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_save_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+   vgpu->ggtt_entries = vzalloc((num_low + num_hi) * sizeof(u64));
+   if (!vgpu->ggtt_entries)
+   continue;

I think doing allocation in suspend is not good, that should prealloc
at vgpu create time and free at destroy.
 Yes Hang and I discussed about the two approaches. This buffer is only 
useful during system suspend time for status backing store, but useless 
when system in S0. If we move it to vgpu creation time, large piece of 
memory could be wasted.



+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries, addr, num_low * sizeof(u64));
+
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries + num_low,
+ addr, num_hi * sizeof(u64));
+   }
+}
+
+/**
+ * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver resume stage to restore
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   if (!vgpu->ggtt_entries) {
+   gvt_vgpu_err("fail to get saved ggtt\n");
+   continue;
+   }
+
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_toio(addr, vgpu->ggtt_entries, num_low * sizeof(u64));
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memc

[Intel-gfx] [PATCH v4 2/2] drm/i915/gvt: Add GVT suspend/resume routine to i915

[Colin Xu]

This patch add gvt suspend/resume wrapper into i915: i915_drm_suspend()
and i915_drm_resume(). GVT relies on i915 so suspend gvt ahead of other
i915 sub-routine and resume gvt at last.

V2:
- Direct call into gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c.
The wrapper and implementation will check and call gvt routine. (zhenyu)

V3:
Refresh.

V4:
Rebase.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/i915_drv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8bb7e2dcfaaa..1fd05e15b54f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1104,6 +1104,8 @@ static int i915_drm_suspend(struct drm_device *dev)
 
drm_kms_helper_poll_disable(dev);
 
+   intel_gvt_suspend(dev_priv);
+
pci_save_state(pdev);
 
intel_display_suspend(dev);
@@ -1281,6 +1283,8 @@ static int i915_drm_resume(struct drm_device *dev)
 
intel_power_domains_enable(dev_priv);
 
+   intel_gvt_resume(dev_priv);
+
enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
return 0;
-- 
2.28.0

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

[Intel-gfx] [PATCH v4 1/2] drm/i915/gvt: Save/restore HW status for GVT during suspend/resume

[Colin Xu]

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries into GVT in suspend routine, and restore the saved entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
 drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
 drivers/gpu/drm/i915/gvt/gvt.h  |  8 +++
 drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
 drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 drivers/gpu/drm/i915/intel_gvt.c| 29 +++
 drivers/gpu/drm/i915/intel_gvt.h| 10 
 9 files changed, 179 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index a3a4305eda01..534bb2e08538 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2533,6 +2533,11 @@ static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu 
*vgpu)
}
intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm);
vgpu->gtt.ggtt_mm = NULL;
+
+   if (vgpu->ggtt_entries) {
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
 }
 
 /**
@@ -2852,3 +2857,73 @@ void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu)
intel_vgpu_destroy_all_ppgtt_mm(vgpu);
intel_vgpu_reset_ggtt(vgpu, true);
 }
+
+/**
+ * intel_gvt_save_ggtt - save all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver suspend stage to save
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_save_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+   vgpu->ggtt_entries = vzalloc((num_low + num_hi) * sizeof(u64));
+   if (!vgpu->ggtt_entries)
+   continue;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries, addr, num_low * sizeof(u64));
+
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries + num_low,
+ addr, num_hi * sizeof(u64));
+   }
+}
+
+/**
+ * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver resume stage to restore
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   if (!vgpu->ggtt_entries) {
+   gvt_vgpu_err("fail to get saved ggtt\n");
+   continue;
+   }
+
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_toio(addr, vgpu->ggtt_entries, num_low * sizeof(u64));
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_toio(addr, vgpu->ggtt_entries + num_low,
+   num_hi * sizeof(u64));
+
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
+}
diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
index 52d0d88abd86..141e0a41cd50 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.h
+++ b/drivers/gpu/drm/i915/gvt/gtt.h
@@ -280,5 +280,7 @@ int intel_vgpu_emulate_ggtt_mmio_write(stru

[Intel-gfx] [PATCH v4 0/2] Enable GVT suspend/resume

[Colin Xu]

This patchset enables GVT suspend/resume so that GVT enabled VM can
continue running after host resuming from suspend state.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

V4:
Rebase.

Colin Xu (2):
  drm/i915/gvt: Save/restore HW status for GVT during suspend/resume
  drm/i915/gvt: Add GVT suspend/resume routine to i915

 drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
 drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
 drivers/gpu/drm/i915/gvt/gvt.h  |  8 +++
 drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
 drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 drivers/gpu/drm/i915/i915_drv.c |  4 ++
 drivers/gpu/drm/i915/intel_gvt.c| 29 +++
 drivers/gpu/drm/i915/intel_gvt.h| 10 
 10 files changed, 183 insertions(+), 3 deletions(-)

-- 
2.28.0

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

Re: [Intel-gfx] [PATCH] drm/i915/gvt: Introduce per object locking in GVT scheduler.

[Colin Xu]

On 2020-09-09 10:06, Zhenyu Wang wrote:

On 2020.09.09 09:43:21 +0800, Colin Xu wrote:

I tested this patch on the suspend/resume case with vGPU created (no need
really activate), can still observer the system freeze issue as mentioned in
another patch I sent. So I suppose we still need decouple context pin/unpin
with submission setup/clean, but move to workload create/destroy?


yeah, I observed that conflict too. How about merging Zhi's ww lock fix first
then next for left suspend/resume fixes? And better if you could pack all fixes
in one series.

Thanks
No problem at all. And I would like to hear more comments if moving the 
context pin/unpin to workload lifecycle is reasonable.



After made similar changes based on this one, plus the suspend/resume
support patch, below tests can pass:

- Create vGPU then suspend/resume.

- Run VM w/ vGPU then suspend/resume.

https://lists.freedesktop.org/archives/intel-gvt-dev/2020-September/007061.html

On 2020-09-08 04:02, Zhi Wang wrote:

To support ww locking and per-object implemented in i915, GVT scheduler needs
to be refined. Most of the changes are located in shadow batch buffer, shadow
wa context in GVT-g, where use quite a lot of i915 gem object APIs.

Cc: Maarten Lankhorst 
Cc: Joonas Lahtinen 
Cc: Zhenyu Wang 
Signed-off-by: Zhi Wang 
---
   drivers/gpu/drm/i915/gvt/scheduler.c | 68 
++--
   1 file changed, 57 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
b/drivers/gpu/drm/i915/gvt/scheduler.c
index 1570eb8..fe7ee10 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -396,7 +396,9 @@ static void release_shadow_wa_ctx(struct 
intel_shadow_wa_ctx *wa_ctx)
if (!wa_ctx->indirect_ctx.obj)
return;
+   i915_gem_object_lock(wa_ctx->indirect_ctx.obj, NULL);
i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj);
+   i915_gem_object_unlock(wa_ctx->indirect_ctx.obj);
i915_gem_object_put(wa_ctx->indirect_ctx.obj);
wa_ctx->indirect_ctx.obj = NULL;
@@ -504,6 +506,7 @@ static int prepare_shadow_batch_buffer(struct 
intel_vgpu_workload *workload)
struct intel_gvt *gvt = workload->vgpu->gvt;
const int gmadr_bytes = gvt->device_info.gmadr_bytes_in_cmd;
struct intel_vgpu_shadow_bb *bb;
+   struct i915_gem_ww_ctx ww;
int ret;
list_for_each_entry(bb, &workload->shadow_bb, list) {
@@ -528,10 +531,19 @@ static int prepare_shadow_batch_buffer(struct 
intel_vgpu_workload *workload)
 * directly
 */
if (!bb->ppgtt) {
-   bb->vma = i915_gem_object_ggtt_pin(bb->obj,
-  NULL, 0, 0, 0);
+   i915_gem_ww_ctx_init(&ww, false);
+retry:
+   i915_gem_object_lock(bb->obj, &ww);
+
+   bb->vma = i915_gem_object_ggtt_pin_ww(bb->obj, &ww,
+ NULL, 0, 0, 0);
if (IS_ERR(bb->vma)) {
ret = PTR_ERR(bb->vma);
+   if (ret == -EDEADLK) {
+   ret = i915_gem_ww_ctx_backoff(&ww);
+   if (!ret)
+   goto retry;
+   }
goto err;
}
@@ -545,13 +557,18 @@ static int prepare_shadow_batch_buffer(struct 
intel_vgpu_workload *workload)
  0);
if (ret)
goto err;
+
+   /* No one is going to touch shadow bb from now on. */
+   i915_gem_object_flush_map(bb->obj);
+
+   i915_gem_object_unlock(bb->obj);
+   i915_gem_ww_ctx_fini(&ww);
}
-   /* No one is going to touch shadow bb from now on. */
-   i915_gem_object_flush_map(bb->obj);
}
return 0;
   err:
+   i915_gem_ww_ctx_fini(&ww);
release_shadow_batch_buffer(workload);
return ret;
   }
@@ -578,14 +595,30 @@ static int prepare_shadow_wa_ctx(struct 
intel_shadow_wa_ctx *wa_ctx)
unsigned char *per_ctx_va =
(unsigned char *)wa_ctx->indirect_ctx.shadow_va +
wa_ctx->indirect_ctx.size;
+   struct i915_gem_ww_ctx ww;
+   int ret;
if (wa_ctx->indirect_ctx.size == 0)
return 0;
-   vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL,
-  0, CACHELINE_BYTES, 0);
-   if (IS_ERR(vma))
-   return PTR_ERR(vma);
+   i915_gem_ww_ctx_init(&ww, false);

Re: [Intel-gfx] [PATCH] drm/i915/gvt: Introduce per object locking in GVT scheduler.

[Colin Xu]

);
+   if (!ret)
+   goto retry;
+   }
+   return ret;
+   }
+
+   i915_gem_object_unlock(wa_ctx->indirect_ctx.obj);
+   i915_gem_ww_ctx_fini(&ww);
  
  	/* FIXME: we are not tracking our pinned VMA leaving it

 * up to the core to fix up the stray pin_count upon
@@ -619,12 +652,14 @@ static void release_shadow_batch_buffer(struct 
intel_vgpu_workload *workload)
  
  	list_for_each_entry_safe(bb, pos, &workload->shadow_bb, list) {

if (bb->obj) {
+   i915_gem_object_lock(bb->obj, NULL);
if (bb->va && !IS_ERR(bb->va))
i915_gem_object_unpin_map(bb->obj);
  
  			if (bb->vma && !IS_ERR(bb->vma))

i915_vma_unpin(bb->vma);
  
+			i915_gem_object_unlock(bb->obj);

i915_gem_object_put(bb->obj);
}
list_del(&bb->list);
@@ -1337,6 +1372,7 @@ int intel_vgpu_setup_submission(struct intel_vgpu *vgpu)
struct intel_vgpu_submission *s = &vgpu->submission;
struct intel_engine_cs *engine;
struct i915_ppgtt *ppgtt;
+   struct i915_gem_ww_ctx ww;
enum intel_engine_id i;
int ret;
  
@@ -1368,11 +1404,20 @@ int intel_vgpu_setup_submission(struct intel_vgpu *vgpu)
  
  			ce->ring = __intel_context_ring_size(ring_size);

}

Cut here

+   i915_gem_ww_ctx_init(&ww, false);
+retry:
+   ret = intel_context_pin_ww(ce, &ww);
+   if (ret) {
+   if (ret == -EDEADLK) {
+   ret = i915_gem_ww_ctx_backoff(&ww);
+   if (!ret)
+   goto retry;
+   }
+   goto out_shadow_ctx;
+   }
I move the piece to create_workload. Similar to the change I made in my 
patch sent.
  
-		ret = intel_context_pin(ce);

intel_context_put(ce);
-   if (ret)
-   goto out_shadow_ctx;
+   i915_gem_ww_ctx_fini(&ww);
  
  		s->shadow[i] = ce;

}
@@ -1400,6 +1445,7 @@ int intel_vgpu_setup_submission(struct intel_vgpu *vgpu)
return 0;
  
  out_shadow_ctx:

+   i915_gem_ww_ctx_fini(&ww);
i915_context_ppgtt_root_restore(s, ppgtt);
for_each_engine(engine, vgpu->gvt->gt, i) {
if (IS_ERR(s->shadow[i]))


--
Best Regards,
Colin Xu

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

[Intel-gfx] [PATCH v3 1/2] drm/i915/gvt: Save/restore HW status for GVT during suspend/resume

[Colin Xu]

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries into GVT in suspend routine, and restore the saved entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
 drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
 drivers/gpu/drm/i915/gvt/gvt.h  |  8 +++
 drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
 drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 drivers/gpu/drm/i915/intel_gvt.c| 29 +++
 drivers/gpu/drm/i915/intel_gvt.h| 10 
 9 files changed, 179 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 04bf018ecc34..817095dd221b 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2533,6 +2533,11 @@ static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu 
*vgpu)
}
intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm);
vgpu->gtt.ggtt_mm = NULL;
+
+   if (vgpu->ggtt_entries) {
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
 }
 
 /**
@@ -2834,3 +2839,73 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu, bool 
invalidate_old)
 
ggtt_invalidate(gvt->gt);
 }
+
+/**
+ * intel_gvt_save_ggtt - save all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver suspend stage to save
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_save_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+   vgpu->ggtt_entries = vzalloc((num_low + num_hi) * sizeof(u64));
+   if (!vgpu->ggtt_entries)
+   continue;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries, addr, num_low * sizeof(u64));
+
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_fromio(vgpu->ggtt_entries + num_low,
+ addr, num_hi * sizeof(u64));
+   }
+}
+
+/**
+ * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver resume stage to restore
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   if (!vgpu->ggtt_entries) {
+   gvt_vgpu_err("fail to get saved ggtt\n");
+   continue;
+   }
+
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_toio(addr, vgpu->ggtt_entries, num_low * sizeof(u64));
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy_toio(addr, vgpu->ggtt_entries + num_low,
+   num_hi * sizeof(u64));
+
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
+}
diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
index b76a262dd9bc..0d2fb2714852 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.h
+++ b/drivers/gpu/drm/i915/gvt/gtt.h
@@ -279,5 +279,7 @@ int intel_vgpu_emulate_ggtt_mmio_write(struct intel_vgpu 
*vgpu,
unsigned int

[Intel-gfx] [PATCH v3 2/2] drm/i915/gvt: Add GVT suspend/resume routine to i915

[Colin Xu]

This patch add gvt suspend/resume wrapper into i915: i915_drm_suspend()
and i915_drm_resume(). GVT relies on i915 so suspend gvt ahead of other
i915 sub-routine and resume gvt at last.

V2:
- Direct call into gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c.
The wrapper and implementation will check and call gvt routine. (zhenyu)

V3:
Refresh.

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/i915_drv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d66fe09d337e..28055dc65ecd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1090,6 +1090,8 @@ static int i915_drm_suspend(struct drm_device *dev)
 
drm_kms_helper_poll_disable(dev);
 
+   intel_gvt_suspend(dev_priv);
+
pci_save_state(pdev);
 
intel_display_suspend(dev);
@@ -1267,6 +1269,8 @@ static int i915_drm_resume(struct drm_device *dev)
 
intel_power_domains_enable(dev_priv);
 
+   intel_gvt_resume(dev_priv);
+
enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
return 0;
-- 
2.28.0

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

[Intel-gfx] [PATCH v3 0/2] Enable GVT suspend/resume

[Colin Xu]

This patchset enables GVT suspend/resume so that GVT enabled VM can
continue running after host resuming from suspend state.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

V3: (zhenyu)
- Incorrect copy length. Should be num entries * entry size.
- Use memcpy_toio()/memcpy_fromio() instead of memcpy for iomem.
- Add F_PM_SAVE flags to indicate which MMIOs to save/restore for PM.

Colin Xu (2):
  drm/i915/gvt: Save/restore HW status for GVT during suspend/resume
  drm/i915/gvt: Add GVT suspend/resume routine to i915

 drivers/gpu/drm/i915/gvt/gtt.c  | 75 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
 drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
 drivers/gpu/drm/i915/gvt/gvt.h  |  8 +++
 drivers/gpu/drm/i915/gvt/handlers.c | 39 +--
 drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 drivers/gpu/drm/i915/i915_drv.c |  4 ++
 drivers/gpu/drm/i915/intel_gvt.c| 29 +++
 drivers/gpu/drm/i915/intel_gvt.h| 10 
 10 files changed, 183 insertions(+), 3 deletions(-)

-- 
2.28.0

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

Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/gvt: Save/restore HW status for GVT during suspend/resume

[Colin Xu]

On 2020-08-26 17:10, Zhenyu Wang wrote:

On 2020.08.26 14:35:05 +0800, Colin Xu wrote:

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries into GVT in suspend routine, and restore the saved entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
  drivers/gpu/drm/i915/gvt/gtt.c  | 73 +
  drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
  drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
  drivers/gpu/drm/i915/gvt/gvt.h  |  6 +++
  drivers/gpu/drm/i915/gvt/handlers.c | 20 
  drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
  drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
  drivers/gpu/drm/i915/intel_gvt.c| 29 
  drivers/gpu/drm/i915/intel_gvt.h| 10 
  9 files changed, 159 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 04bf018ecc34..7907a535d49f 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2533,6 +2533,11 @@ static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu 
*vgpu)
}
intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm);
vgpu->gtt.ggtt_mm = NULL;
+
+   if (vgpu->ggtt_entries) {
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
  }
  
  /**

@@ -2834,3 +2839,71 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu, bool 
invalidate_old)
  
  	ggtt_invalidate(gvt->gt);

  }
+
+/**
+ * intel_gvt_save_ggtt - save all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver suspend stage to save
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_save_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+   vgpu->ggtt_entries = vzalloc((num_low + num_hi) * sizeof(u64));
+   if (!vgpu->ggtt_entries)
+   continue;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy(vgpu->ggtt_entries, addr, num_low);

Should use memcpy_fromio() and is the size right? It's the number of entries
instead of bytes count?
Indeed this is a mistake. ggtt_entries is allocated num_entries * 8bytes 
(sizeof(u64)) and copy should also count on bytes instead of num entries.



+
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy((u64 *)vgpu->ggtt_entries + num_low, addr, num_hi);
+   }

ditto


+}
+
+/**
+ * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver resume stage to restore
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   if (!vgpu->ggtt_entries) {
+   gvt_vgpu_err("fail to get saved ggtt\n");
+   continue;
+   }
+
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy(addr, vgpu->ggtt_entries, num_low);

memcpy_toio()


+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy(addr, (u64 *)vgpu->ggtt_entries + num_low, num_hi);
+
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
+}
diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
index b76a262dd9bc..0d2fb2714852 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.h
+++ b/drivers/gpu/drm/i915/gvt/gtt.h
@@ -279,5 +279,7 @@ int intel_vgpu_emulate_ggtt_mmio_write(struct 

[Intel-gfx] [PATCH v2 2/2] drm/i915/gvt: Add GVT suspend/resume routine to i915

[Colin Xu]

This patch add gvt suspend/resume wrapper into i915: i915_drm_suspend()
and i915_drm_resume(). GVT relies on i915 so suspend gvt ahead of other
i915 sub-routine and resume gvt at last.

V2:
- Direct call into gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c.
The wrapper and implementation will check and call gvt routine. (zhenyu)

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/i915_drv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 00292a849c34..99c15a9183c2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1177,6 +1177,8 @@ static int i915_drm_suspend(struct drm_device *dev)
 
drm_kms_helper_poll_disable(dev);
 
+   intel_gvt_suspend(dev_priv);
+
pci_save_state(pdev);
 
intel_display_suspend(dev);
@@ -1354,6 +1356,8 @@ static int i915_drm_resume(struct drm_device *dev)
 
intel_power_domains_enable(dev_priv);
 
+   intel_gvt_resume(dev_priv);
+
enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
 
return 0;
-- 
2.28.0

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

[Intel-gfx] [PATCH v2 1/2] drm/i915/gvt: Save/restore HW status for GVT during suspend/resume

[Colin Xu]

This patch save/restore necessary GVT info during i915 suspend/resume so
that GVT enabled QEMU VM can continue running.

Only GGTT and fence regs are saved/restored now. GVT will save GGTT
entries into GVT in suspend routine, and restore the saved entries
and re-init fence regs in resume routine.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

Signed-off-by: Hang Yuan 
Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/gtt.c  | 73 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
 drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
 drivers/gpu/drm/i915/gvt/gvt.h  |  6 +++
 drivers/gpu/drm/i915/gvt/handlers.c | 20 
 drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 drivers/gpu/drm/i915/intel_gvt.c| 29 
 drivers/gpu/drm/i915/intel_gvt.h| 10 
 9 files changed, 159 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 04bf018ecc34..7907a535d49f 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2533,6 +2533,11 @@ static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu 
*vgpu)
}
intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm);
vgpu->gtt.ggtt_mm = NULL;
+
+   if (vgpu->ggtt_entries) {
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
 }
 
 /**
@@ -2834,3 +2839,71 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu, bool 
invalidate_old)
 
ggtt_invalidate(gvt->gt);
 }
+
+/**
+ * intel_gvt_save_ggtt - save all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver suspend stage to save
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_save_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+   vgpu->ggtt_entries = vzalloc((num_low + num_hi) * sizeof(u64));
+   if (!vgpu->ggtt_entries)
+   continue;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy(vgpu->ggtt_entries, addr, num_low);
+
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy((u64 *)vgpu->ggtt_entries + num_low, addr, num_hi);
+   }
+}
+
+/**
+ * intel_gvt_restore_ggtt - restore all vGPU's ggtt entries
+ * @gvt: intel gvt device
+ *
+ * This function is called at driver resume stage to restore
+ * GGTT entries of every active vGPU.
+ *
+ */
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt)
+{
+   struct intel_vgpu *vgpu;
+   int id;
+   u32 index, num_low, num_hi;
+   void __iomem *addr;
+
+   for_each_active_vgpu(gvt, vgpu, id) {
+   if (!vgpu->ggtt_entries) {
+   gvt_vgpu_err("fail to get saved ggtt\n");
+   continue;
+   }
+
+   num_low = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
+   num_hi = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
+
+   index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy(addr, vgpu->ggtt_entries, num_low);
+   index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
+   addr = (gen8_pte_t __iomem *)gvt->gt->i915->ggtt.gsm + index;
+   memcpy(addr, (u64 *)vgpu->ggtt_entries + num_low, num_hi);
+
+   vfree(vgpu->ggtt_entries);
+   vgpu->ggtt_entries = NULL;
+   }
+}
diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
index b76a262dd9bc..0d2fb2714852 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.h
+++ b/drivers/gpu/drm/i915/gvt/gtt.h
@@ -279,5 +279,7 @@ int intel_vgpu_emulate_ggtt_mmio_write(struct intel_vgpu 
*vgpu,
unsigned int off, void *p_data, unsigned int bytes);
 
 void intel_vgpu_destroy_all_ppgtt_mm(struct intel_vgpu *vgpu);
+void intel_gvt_save_ggtt(struct intel_gvt *gvt);
+void intel_gvt_restore_ggtt(struct intel_gvt *gvt);
 
 #endif /* _GVT_GTT_H_ */
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index c7c561237883..3de740fa0911 100644
-

[Intel-gfx] [PATCH v2 0/2] Enable GVT suspend/resume

[Colin Xu]

This patchset enables GVT suspend/resume so that GVT enabled VM can
continue running after host resuming from suspend state.

V2:
- Change kzalloc/kfree to vzalloc/vfree since the space allocated
from kmalloc may not enough for all saved GGTT entries.
- Keep gvt suspend/resume wrapper in intel_gvt.h/intel_gvt.c and
move the actual implementation to gvt.h/gvt.c. (zhenyu)
- Check gvt config on and active with intel_gvt_active(). (zhenyu)

Colin Xu (2):
  drm/i915/gvt: Save/restore HW status for GVT during suspend/resume
  drm/i915/gvt: Add GVT suspend/resume routine to i915

 drivers/gpu/drm/i915/gvt/gtt.c  | 73 +
 drivers/gpu/drm/i915/gvt/gtt.h  |  2 +
 drivers/gpu/drm/i915/gvt/gvt.c  | 15 ++
 drivers/gpu/drm/i915/gvt/gvt.h  |  6 +++
 drivers/gpu/drm/i915/gvt/handlers.c | 20 
 drivers/gpu/drm/i915/gvt/mmio.h |  3 ++
 drivers/gpu/drm/i915/gvt/vgpu.c |  1 +
 drivers/gpu/drm/i915/i915_drv.c |  4 ++
 drivers/gpu/drm/i915/intel_gvt.c| 29 
 drivers/gpu/drm/i915/intel_gvt.h| 10 
 10 files changed, 163 insertions(+)

-- 
2.28.0

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

[Intel-gfx] [PATCH v3 12/12] drm/i915: Enable KVMGT for BXT.

[Colin Xu]

Enable KVMGT for BXT.
is_supported_device() acting as the gatekeeper of GVT-g init.
If all supported platforms share the same configurations for some
specific feature, platform check will rely on this check only.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/intel_gvt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index a2fe7c8d4477..a6291f60545b 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -47,6 +47,8 @@ static bool is_supported_device(struct drm_i915_private 
*dev_priv)
return true;
if (IS_KABYLAKE(dev_priv))
return true;
+   if (IS_BROXTON(dev_priv))
+   return true;
return false;
 }
 
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 11/12] drm/i915/gvt: Add mmio handler for for BXT.

[Colin Xu]

Leverage most SKL/KBL mmio init info and add different mmio to
BXT specific function init_bxt_mmio_info().

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/handlers.c | 389 
 1 file changed, 344 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
b/drivers/gpu/drm/i915/gvt/handlers.c
index 48e337977cf6..e39492aaff6c 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -257,7 +257,8 @@ static int mul_force_wake_write(struct intel_vgpu *vgpu,
new = CALC_MODE_MASK_REG(old, *(u32 *)p_data);
 
if (IS_SKYLAKE(vgpu->gvt->dev_priv)
-   || IS_KABYLAKE(vgpu->gvt->dev_priv)) {
+   || IS_KABYLAKE(vgpu->gvt->dev_priv)
+   || IS_BROXTON(vgpu->gvt->dev_priv)) {
switch (offset) {
case FORCEWAKE_RENDER_GEN9_REG:
ack_reg_offset = FORCEWAKE_ACK_RENDER_GEN9_REG;
@@ -863,7 +864,8 @@ static int dp_aux_ch_ctl_mmio_write(struct intel_vgpu *vgpu,
data = vgpu_vreg(vgpu, offset);
 
if ((IS_SKYLAKE(vgpu->gvt->dev_priv)
-   || IS_KABYLAKE(vgpu->gvt->dev_priv))
+   || IS_KABYLAKE(vgpu->gvt->dev_priv)
+   || IS_BROXTON(vgpu->gvt->dev_priv))
&& offset != _REG_SKL_DP_AUX_CH_CTL(port_index)) {
/* SKL DPB/C/D aux ctl register changed */
return 0;
@@ -1375,6 +1377,16 @@ static int mailbox_write(struct intel_vgpu *vgpu, 
unsigned int offset,
*data0 = 0x1e1a1100;
else
*data0 = 0x61514b3d;
+   } else if (IS_BROXTON(vgpu->gvt->dev_priv)) {
+   /**
+* "Read memory latency" command on gen9.
+* Below memory latency values are read
+* from Broxton MRB.
+*/
+   if (!*data0)
+   *data0 = 0x16080707;
+   else
+   *data0 = 0x16161616;
}
break;
case SKL_PCODE_CDCLK_CONTROL:
@@ -1432,8 +1444,11 @@ static int skl_power_well_ctl_write(struct intel_vgpu 
*vgpu,
 {
u32 v = *(u32 *)p_data;
 
-   v &= (1 << 31) | (1 << 29) | (1 << 9) |
-(1 << 7) | (1 << 5) | (1 << 3) | (1 << 1);
+   if (IS_BROXTON(vgpu->gvt->dev_priv))
+   v &= (1 << 31) | (1 << 29);
+   else
+   v &= (1 << 31) | (1 << 29) | (1 << 9) |
+   (1 << 7) | (1 << 5) | (1 << 3) | (1 << 1);
v |= (v >> 1);
 
return intel_vgpu_default_mmio_write(vgpu, offset, &v, bytes);
@@ -1453,6 +1468,102 @@ static int skl_lcpll_write(struct intel_vgpu *vgpu, 
unsigned int offset,
return 0;
 }
 
+static int bxt_de_pll_enable_write(struct intel_vgpu *vgpu,
+   unsigned int offset, void *p_data, unsigned int bytes)
+{
+   u32 v = *(u32 *)p_data;
+
+   if (v & BXT_DE_PLL_PLL_ENABLE)
+   v |= BXT_DE_PLL_LOCK;
+
+   vgpu_vreg(vgpu, offset) = v;
+
+   return 0;
+}
+
+static int bxt_port_pll_enable_write(struct intel_vgpu *vgpu,
+   unsigned int offset, void *p_data, unsigned int bytes)
+{
+   u32 v = *(u32 *)p_data;
+
+   if (v & PORT_PLL_ENABLE)
+   v |= PORT_PLL_LOCK;
+
+   vgpu_vreg(vgpu, offset) = v;
+
+   return 0;
+}
+
+static int bxt_phy_ctl_family_write(struct intel_vgpu *vgpu,
+   unsigned int offset, void *p_data, unsigned int bytes)
+{
+   u32 v = *(u32 *)p_data;
+   u32 data = v & COMMON_RESET_DIS ? BXT_PHY_LANE_ENABLED : 0;
+
+   vgpu_vreg(vgpu, _BXT_PHY_CTL_DDI_A) = data;
+   vgpu_vreg(vgpu, _BXT_PHY_CTL_DDI_B) = data;
+   vgpu_vreg(vgpu, _BXT_PHY_CTL_DDI_C) = data;
+
+   vgpu_vreg(vgpu, offset) = v;
+
+   return 0;
+}
+
+static int bxt_port_tx_dw3_read(struct intel_vgpu *vgpu,
+   unsigned int offset, void *p_data, unsigned int bytes)
+{
+   u32 v = vgpu_vreg(vgpu, offset);
+
+   v &= ~UNIQUE_TRANGE_EN_METHOD;
+
+   vgpu_vreg(vgpu, offset) = v;
+
+   return intel_vgpu_default_mmio_read(vgpu, offset, p_data, bytes);
+}
+
+static int bxt_pcs_dw12_grp_write(struct intel_vgpu *vgpu,
+   unsigned int offset, void *p_data, unsigned int bytes)
+{
+   u32 v = *(u32 *)p_data;
+
+   if (offset == _PORT_PCS_DW12_GRP_A || offset == _PORT_PCS_DW12_GRP_B) {
+   vgpu_vreg(vgpu, offset - 0x600) = v;
+   vgpu_vreg(vgpu, offset - 0x800) = v;
+   } else {
+   vgpu_vreg(vgpu, offset - 0x400) = v;
+   vgpu_vreg(vgpu, offset - 0x600) = v;
+   }

[Intel-gfx] [PATCH v3 09/12] drm/i915/gvt: Enable virtual display support for BXT.

[Colin Xu]

Virtual monitor on BXT start from port B.
Unlike SKL/KBL, digital display port connectivity is detected via
GEN8_DE_PORT_ISR so emulate monitor state change by setting it.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/display.c | 23 +++
 drivers/gpu/drm/i915/gvt/edid.c| 20 +++-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gvt/display.c 
b/drivers/gpu/drm/i915/gvt/display.c
index 38521fa81bf9..6ee50cb328f8 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -171,6 +171,29 @@ static void emulate_monitor_status_change(struct 
intel_vgpu *vgpu)
struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
int pipe;
 
+   if (IS_BROXTON(dev_priv)) {
+   vgpu_vreg_t(vgpu, GEN8_DE_PORT_ISR) &= ~(BXT_DE_PORT_HP_DDIA |
+   BXT_DE_PORT_HP_DDIB |
+   BXT_DE_PORT_HP_DDIC);
+
+   if (intel_vgpu_has_monitor_on_port(vgpu, PORT_A)) {
+   vgpu_vreg_t(vgpu, GEN8_DE_PORT_ISR) |=
+   BXT_DE_PORT_HP_DDIA;
+   }
+
+   if (intel_vgpu_has_monitor_on_port(vgpu, PORT_B)) {
+   vgpu_vreg_t(vgpu, GEN8_DE_PORT_ISR) |=
+   BXT_DE_PORT_HP_DDIB;
+   }
+
+   if (intel_vgpu_has_monitor_on_port(vgpu, PORT_C)) {
+   vgpu_vreg_t(vgpu, GEN8_DE_PORT_ISR) |=
+   BXT_DE_PORT_HP_DDIC;
+   }
+
+   return;
+   }
+
vgpu_vreg_t(vgpu, SDEISR) &= ~(SDE_PORTB_HOTPLUG_CPT |
SDE_PORTC_HOTPLUG_CPT |
SDE_PORTD_HOTPLUG_CPT);
diff --git a/drivers/gpu/drm/i915/gvt/edid.c b/drivers/gpu/drm/i915/gvt/edid.c
index f61337632969..4b98539025c5 100644
--- a/drivers/gpu/drm/i915/gvt/edid.c
+++ b/drivers/gpu/drm/i915/gvt/edid.c
@@ -77,6 +77,20 @@ static unsigned char edid_get_byte(struct intel_vgpu *vgpu)
return chr;
 }
 
+static inline int bxt_get_port_from_gmbus0(u32 gmbus0)
+{
+   int port_select = gmbus0 & _GMBUS_PIN_SEL_MASK;
+   int port = -EINVAL;
+
+   if (port_select == 1)
+   port = PORT_B;
+   else if (port_select == 2)
+   port = PORT_C;
+   else if (port_select == 3)
+   port = PORT_D;
+   return port;
+}
+
 static inline int get_port_from_gmbus0(u32 gmbus0)
 {
int port_select = gmbus0 & _GMBUS_PIN_SEL_MASK;
@@ -105,6 +119,7 @@ static void reset_gmbus_controller(struct intel_vgpu *vgpu)
 static int gmbus0_mmio_write(struct intel_vgpu *vgpu,
unsigned int offset, void *p_data, unsigned int bytes)
 {
+   struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
int port, pin_select;
 
memcpy(&vgpu_vreg(vgpu, offset), p_data, bytes);
@@ -116,7 +131,10 @@ static int gmbus0_mmio_write(struct intel_vgpu *vgpu,
if (pin_select == 0)
return 0;
 
-   port = get_port_from_gmbus0(pin_select);
+   if (IS_BROXTON(dev_priv))
+   port = bxt_get_port_from_gmbus0(pin_select);
+   else
+   port = get_port_from_gmbus0(pin_select);
if (WARN_ON(port < 0))
return 0;
 
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 10/12] drm/i915/gvt: Enable dma_buf support for BXT.

[Colin Xu]

Handle dma_buf on BXT as SKL and KBL.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/dmabuf.c |  4 +++-
 drivers/gpu/drm/i915/gvt/fb_decoder.c | 12 +---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c 
b/drivers/gpu/drm/i915/gvt/dmabuf.c
index d2eb2f7754b9..6e3f56684f4e 100644
--- a/drivers/gpu/drm/i915/gvt/dmabuf.c
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
@@ -164,7 +164,9 @@ static struct drm_i915_gem_object *vgpu_create_gem(struct 
drm_device *dev,
 
obj->read_domains = I915_GEM_DOMAIN_GTT;
obj->write_domain = 0;
-   if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
+   if (IS_SKYLAKE(dev_priv)
+   || IS_KABYLAKE(dev_priv)
+   || IS_BROXTON(dev_priv)) {
unsigned int tiling_mode = 0;
unsigned int stride = 0;
 
diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c 
b/drivers/gpu/drm/i915/gvt/fb_decoder.c
index 20b502c44eae..face664be3e8 100644
--- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
@@ -151,7 +151,9 @@ static u32 intel_vgpu_get_stride(struct intel_vgpu *vgpu, 
int pipe,
u32 stride_reg = vgpu_vreg_t(vgpu, DSPSTRIDE(pipe)) & stride_mask;
u32 stride = stride_reg;
 
-   if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
+   if (IS_SKYLAKE(dev_priv)
+   || IS_KABYLAKE(dev_priv)
+   || IS_BROXTON(dev_priv)) {
switch (tiled) {
case PLANE_CTL_TILED_LINEAR:
stride = stride_reg * 64;
@@ -215,7 +217,9 @@ int intel_vgpu_decode_primary_plane(struct intel_vgpu *vgpu,
if (!plane->enabled)
return -ENODEV;
 
-   if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
+   if (IS_SKYLAKE(dev_priv)
+   || IS_KABYLAKE(dev_priv)
+   || IS_BROXTON(dev_priv)) {
plane->tiled = (val & PLANE_CTL_TILED_MASK) >>
_PLANE_CTL_TILED_SHIFT;
fmt = skl_format_to_drm(
@@ -257,7 +261,9 @@ int intel_vgpu_decode_primary_plane(struct intel_vgpu *vgpu,
}
 
plane->stride = intel_vgpu_get_stride(vgpu, pipe, (plane->tiled << 10),
-   (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) ?
+   (IS_SKYLAKE(dev_priv)
+   || IS_KABYLAKE(dev_priv)
+   || IS_BROXTON(dev_priv)) ?
(_PRI_PLANE_STRIDE_MASK >> 6) :
_PRI_PLANE_STRIDE_MASK, plane->bpp);
 
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 08/12] drm/i915/gvt: Enable force wake support for BXT.

[Colin Xu]

BXT forcewake is handled in the same way as SKL/KBL.

v2: Add missing inhibit_context restore for BXT.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/scheduler.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
b/drivers/gpu/drm/i915/gvt/scheduler.c
index cf5a22cb6e06..bbdb118b36ec 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -298,7 +298,8 @@ static int copy_workload_to_ring_buffer(struct 
intel_vgpu_workload *workload)
void *shadow_ring_buffer_va;
u32 *cs;
 
-   if (IS_KABYLAKE(req->i915) && is_inhibit_context(req->hw_context))
+   if ((IS_KABYLAKE(req->i915) || IS_BROXTON(req->i915))
+   && is_inhibit_context(req->hw_context))
intel_vgpu_restore_inhibit_context(vgpu, req);
 
/* allocate shadow ring buffer */
@@ -905,7 +906,8 @@ static int workload_thread(void *priv)
struct intel_vgpu *vgpu = NULL;
int ret;
bool need_force_wake = IS_SKYLAKE(gvt->dev_priv)
-   || IS_KABYLAKE(gvt->dev_priv);
+   || IS_KABYLAKE(gvt->dev_priv)
+   || IS_BROXTON(gvt->dev_priv);
DEFINE_WAIT_FUNC(wait, woken_wake_function);
 
kfree(p);
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 07/12] drm/i915/gvt: Enable cmd_parser support for BXT.

[Colin Xu]

Handle BXT cmd_parser as SKL/KBL.

v2: All supported platforms share the same routines.
Remove the platform check by now and let is_supported_device()
be the gate keeper.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/cmd_parser.c | 39 ++-
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c 
b/drivers/gpu/drm/i915/gvt/cmd_parser.c
index f65cf4515783..0651e63b25fb 100644
--- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
+++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
@@ -1257,7 +1257,9 @@ static int gen8_check_mi_display_flip(struct 
parser_exec_state *s,
if (!info->async_flip)
return 0;
 
-   if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
+   if (IS_SKYLAKE(dev_priv)
+   || IS_KABYLAKE(dev_priv)
+   || IS_BROXTON(dev_priv)) {
stride = vgpu_vreg_t(s->vgpu, info->stride_reg) & GENMASK(9, 0);
tile = (vgpu_vreg_t(s->vgpu, info->ctrl_reg) &
GENMASK(12, 10)) >> 10;
@@ -1285,7 +1287,9 @@ static int gen8_update_plane_mmio_from_mi_display_flip(
 
set_mask_bits(&vgpu_vreg_t(vgpu, info->surf_reg), GENMASK(31, 12),
  info->surf_val << 12);
-   if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
+   if (IS_SKYLAKE(dev_priv)
+   || IS_KABYLAKE(dev_priv)
+   || IS_BROXTON(dev_priv)) {
set_mask_bits(&vgpu_vreg_t(vgpu, info->stride_reg), GENMASK(9, 
0),
  info->stride_val);
set_mask_bits(&vgpu_vreg_t(vgpu, info->ctrl_reg), GENMASK(12, 
10),
@@ -1309,7 +1313,9 @@ static int decode_mi_display_flip(struct 
parser_exec_state *s,
 
if (IS_BROADWELL(dev_priv))
return gen8_decode_mi_display_flip(s, info);
-   if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
+   if (IS_SKYLAKE(dev_priv)
+   || IS_KABYLAKE(dev_priv)
+   || IS_BROXTON(dev_priv))
return skl_decode_mi_display_flip(s, info);
 
return -ENODEV;
@@ -1318,26 +1324,14 @@ static int decode_mi_display_flip(struct 
parser_exec_state *s,
 static int check_mi_display_flip(struct parser_exec_state *s,
struct mi_display_flip_command_info *info)
 {
-   struct drm_i915_private *dev_priv = s->vgpu->gvt->dev_priv;
-
-   if (IS_BROADWELL(dev_priv)
-   || IS_SKYLAKE(dev_priv)
-   || IS_KABYLAKE(dev_priv))
-   return gen8_check_mi_display_flip(s, info);
-   return -ENODEV;
+   return gen8_check_mi_display_flip(s, info);
 }
 
 static int update_plane_mmio_from_mi_display_flip(
struct parser_exec_state *s,
struct mi_display_flip_command_info *info)
 {
-   struct drm_i915_private *dev_priv = s->vgpu->gvt->dev_priv;
-
-   if (IS_BROADWELL(dev_priv)
-   || IS_SKYLAKE(dev_priv)
-   || IS_KABYLAKE(dev_priv))
-   return gen8_update_plane_mmio_from_mi_display_flip(s, info);
-   return -ENODEV;
+   return gen8_update_plane_mmio_from_mi_display_flip(s, info);
 }
 
 static int cmd_handler_mi_display_flip(struct parser_exec_state *s)
@@ -1616,15 +1610,10 @@ static int copy_gma_to_hva(struct intel_vgpu *vgpu, 
struct intel_vgpu_mm *mm,
  */
 static int batch_buffer_needs_scan(struct parser_exec_state *s)
 {
-   struct intel_gvt *gvt = s->vgpu->gvt;
-
-   if (IS_BROADWELL(gvt->dev_priv) || IS_SKYLAKE(gvt->dev_priv)
-   || IS_KABYLAKE(gvt->dev_priv)) {
-   /* BDW decides privilege based on address space */
-   if (cmd_val(s, 0) & (1 << 8) &&
+   /* Decide privilege based on address space */
+   if (cmd_val(s, 0) & (1 << 8) &&
!(s->vgpu->scan_nonprivbb & (1 << s->ring_id)))
-   return 0;
-   }
+   return 0;
return 1;
 }
 
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 06/12] drm/i915/gvt: Enable mmio context init and switch for BXT.

[Colin Xu]

Handle pending tlb flush, mocs/mmio switch and context as KBL.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/mmio_context.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c 
b/drivers/gpu/drm/i915/gvt/mmio_context.c
index 708170e61625..20be9a92600f 100644
--- a/drivers/gpu/drm/i915/gvt/mmio_context.c
+++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
@@ -364,7 +364,8 @@ static void handle_tlb_pending_event(struct intel_vgpu 
*vgpu, int ring_id)
 */
fw = intel_uncore_forcewake_for_reg(dev_priv, reg,
FW_REG_READ | FW_REG_WRITE);
-   if (ring_id == RCS && (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)))
+   if (ring_id == RCS && (IS_SKYLAKE(dev_priv) ||
+   IS_KABYLAKE(dev_priv) || IS_BROXTON(dev_priv)))
fw |= FORCEWAKE_RENDER;
 
intel_uncore_forcewake_get(dev_priv, fw);
@@ -401,7 +402,7 @@ static void switch_mocs(struct intel_vgpu *pre, struct 
intel_vgpu *next,
if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
return;
 
-   if (IS_KABYLAKE(dev_priv) && ring_id == RCS)
+   if ((IS_KABYLAKE(dev_priv) || IS_BROXTON(dev_priv)) && ring_id == RCS)
return;
 
if (!pre && !gen9_render_mocs.initialized)
@@ -467,7 +468,9 @@ static void switch_mmio(struct intel_vgpu *pre,
u32 old_v, new_v;
 
dev_priv = pre ? pre->gvt->dev_priv : next->gvt->dev_priv;
-   if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
+   if (IS_SKYLAKE(dev_priv)
+   || IS_KABYLAKE(dev_priv)
+   || IS_BROXTON(dev_priv))
switch_mocs(pre, next, ring_id);
 
for (mmio = dev_priv->gvt->engine_mmio_list.mmio;
@@ -479,7 +482,8 @@ static void switch_mmio(struct intel_vgpu *pre,
 * state image on kabylake, it's initialized by lri command and
 * save or restore with context together.
 */
-   if (IS_KABYLAKE(dev_priv) && mmio->in_context)
+   if ((IS_KABYLAKE(dev_priv) || IS_BROXTON(dev_priv))
+   && mmio->in_context)
continue;
 
// save
@@ -574,7 +578,9 @@ void intel_gvt_init_engine_mmio_context(struct intel_gvt 
*gvt)
 {
struct engine_mmio *mmio;
 
-   if (IS_SKYLAKE(gvt->dev_priv) || IS_KABYLAKE(gvt->dev_priv))
+   if (IS_SKYLAKE(gvt->dev_priv) ||
+   IS_KABYLAKE(gvt->dev_priv) ||
+   IS_BROXTON(gvt->dev_priv))
gvt->engine_mmio_list.mmio = gen9_engine_mmio_list;
else
gvt->engine_mmio_list.mmio = gen8_engine_mmio_list;
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 05/12] drm/i915/gvt: Enable irq initialization for BXT.

[Colin Xu]

Initialize BXT irq handler as SKL/KBL.

v2: All supported platforms share the same irq ops and map.
Remove the platform check by now and let is_supported_device()
be the gate keeper.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/interrupt.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c 
b/drivers/gpu/drm/i915/gvt/interrupt.c
index 7a041b368f68..84883ebe5824 100644
--- a/drivers/gpu/drm/i915/gvt/interrupt.c
+++ b/drivers/gpu/drm/i915/gvt/interrupt.c
@@ -580,7 +580,9 @@ static void gen8_init_irq(
 
SET_BIT_INFO(irq, 4, PRIMARY_C_FLIP_DONE, 
INTEL_GVT_IRQ_INFO_DE_PIPE_C);
SET_BIT_INFO(irq, 5, SPRITE_C_FLIP_DONE, 
INTEL_GVT_IRQ_INFO_DE_PIPE_C);
-   } else if (IS_SKYLAKE(gvt->dev_priv) || IS_KABYLAKE(gvt->dev_priv)) {
+   } else if (IS_SKYLAKE(gvt->dev_priv)
+   || IS_KABYLAKE(gvt->dev_priv)
+   || IS_BROXTON(gvt->dev_priv)) {
SET_BIT_INFO(irq, 25, AUX_CHANNEL_B, 
INTEL_GVT_IRQ_INFO_DE_PORT);
SET_BIT_INFO(irq, 26, AUX_CHANNEL_C, 
INTEL_GVT_IRQ_INFO_DE_PORT);
SET_BIT_INFO(irq, 27, AUX_CHANNEL_D, 
INTEL_GVT_IRQ_INFO_DE_PORT);
@@ -690,14 +692,8 @@ int intel_gvt_init_irq(struct intel_gvt *gvt)
 
gvt_dbg_core("init irq framework\n");
 
-   if (IS_BROADWELL(gvt->dev_priv) || IS_SKYLAKE(gvt->dev_priv)
-   || IS_KABYLAKE(gvt->dev_priv)) {
-   irq->ops = &gen8_irq_ops;
-   irq->irq_map = gen8_irq_map;
-   } else {
-   WARN_ON(1);
-   return -ENODEV;
-   }
+   irq->ops = &gen8_irq_ops;
+   irq->irq_map = gen8_irq_map;
 
/* common event initialization */
init_events(irq);
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 01/12] drm/i915/gvt: Add D_BXT device type define for BXT.

[Colin Xu]

Broxton belongs to GEN9 family so add to SKL and GEN9 plus.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/handlers.c |  2 ++
 drivers/gpu/drm/i915/gvt/mmio.h | 11 ++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/handlers.c 
b/drivers/gpu/drm/i915/gvt/handlers.c
index 386ada117aa5..48e337977cf6 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -55,6 +55,8 @@ unsigned long intel_gvt_get_device_type(struct intel_gvt *gvt)
return D_SKL;
else if (IS_KABYLAKE(gvt->dev_priv))
return D_KBL;
+   else if (IS_BROXTON(gvt->dev_priv))
+   return D_BXT;
 
return 0;
 }
diff --git a/drivers/gpu/drm/i915/gvt/mmio.h b/drivers/gpu/drm/i915/gvt/mmio.h
index 71b620875943..e474188b46d2 100644
--- a/drivers/gpu/drm/i915/gvt/mmio.h
+++ b/drivers/gpu/drm/i915/gvt/mmio.h
@@ -42,15 +42,16 @@ struct intel_vgpu;
 #define D_BDW   (1 << 0)
 #define D_SKL  (1 << 1)
 #define D_KBL  (1 << 2)
+#define D_BXT  (1 << 3)
 
-#define D_GEN9PLUS (D_SKL | D_KBL)
-#define D_GEN8PLUS (D_BDW | D_SKL | D_KBL)
+#define D_GEN9PLUS (D_SKL | D_KBL | D_BXT)
+#define D_GEN8PLUS (D_BDW | D_SKL | D_KBL | D_BXT)
 
-#define D_SKL_PLUS (D_SKL | D_KBL)
-#define D_BDW_PLUS (D_BDW | D_SKL | D_KBL)
+#define D_SKL_PLUS (D_SKL | D_KBL | D_BXT)
+#define D_BDW_PLUS (D_BDW | D_SKL | D_KBL | D_BXT)
 
 #define D_PRE_SKL  (D_BDW)
-#define D_ALL  (D_BDW | D_SKL | D_KBL)
+#define D_ALL  (D_BDW | D_SKL | D_KBL | D_BXT)
 
 typedef int (*gvt_mmio_func)(struct intel_vgpu *, unsigned int, void *,
 unsigned int);
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 02/12] drm/i915/gvt: Add MEDIA_POOL_STATE for BXT.

[Colin Xu]

As referred in PRM for Broxton Graphics on 01.org

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/cmd_parser.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c 
b/drivers/gpu/drm/i915/gvt/cmd_parser.c
index b51c05d03f14..f65cf4515783 100644
--- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
+++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
@@ -172,6 +172,7 @@ struct decode_info {
 #define OP_MEDIA_INTERFACE_DESCRIPTOR_LOAD  OP_3D_MEDIA(0x2, 0x0, 0x2)
 #define OP_MEDIA_GATEWAY_STATE  OP_3D_MEDIA(0x2, 0x0, 0x3)
 #define OP_MEDIA_STATE_FLUSHOP_3D_MEDIA(0x2, 0x0, 0x4)
+#define OP_MEDIA_POOL_STATE OP_3D_MEDIA(0x2, 0x0, 0x5)
 
 #define OP_MEDIA_OBJECT OP_3D_MEDIA(0x2, 0x1, 0x0)
 #define OP_MEDIA_OBJECT_PRT OP_3D_MEDIA(0x2, 0x1, 0x2)
@@ -2349,6 +2350,9 @@ static struct cmd_info cmd_info[] = {
{"MEDIA_STATE_FLUSH", OP_MEDIA_STATE_FLUSH, F_LEN_VAR, R_RCS, D_ALL,
0, 16, NULL},
 
+   {"MEDIA_POOL_STATE", OP_MEDIA_POOL_STATE, F_LEN_VAR, R_RCS, D_ALL,
+   0, 16, NULL},
+
{"MEDIA_OBJECT", OP_MEDIA_OBJECT, F_LEN_VAR, R_RCS, D_ALL, 0, 16, NULL},
 
{"MEDIA_CURBE_LOAD", OP_MEDIA_CURBE_LOAD, F_LEN_VAR, R_RCS, D_ALL,
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 03/12] drm/i915/gvt: Enable device info initialization for BXT.

[Colin Xu]

Initialize BXT device info as SKL/KBL.

v2: All supported platforms share the same device configuration.
Remove the platform check by now and let is_supported_device()
be the gate keeper.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/gvt.c | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 22a3ddff38a3..4e65266e7b95 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -238,18 +238,15 @@ static void init_device_info(struct intel_gvt *gvt)
struct intel_gvt_device_info *info = &gvt->device_info;
struct pci_dev *pdev = gvt->dev_priv->drm.pdev;
 
-   if (IS_BROADWELL(gvt->dev_priv) || IS_SKYLAKE(gvt->dev_priv)
-   || IS_KABYLAKE(gvt->dev_priv)) {
-   info->max_support_vgpus = 8;
-   info->cfg_space_size = PCI_CFG_SPACE_EXP_SIZE;
-   info->mmio_size = 2 * 1024 * 1024;
-   info->mmio_bar = 0;
-   info->gtt_start_offset = 8 * 1024 * 1024;
-   info->gtt_entry_size = 8;
-   info->gtt_entry_size_shift = 3;
-   info->gmadr_bytes_in_cmd = 8;
-   info->max_surface_size = 36 * 1024 * 1024;
-   }
+   info->max_support_vgpus = 8;
+   info->cfg_space_size = PCI_CFG_SPACE_EXP_SIZE;
+   info->mmio_size = 2 * 1024 * 1024;
+   info->mmio_bar = 0;
+   info->gtt_start_offset = 8 * 1024 * 1024;
+   info->gtt_entry_size = 8;
+   info->gtt_entry_size_shift = 3;
+   info->gmadr_bytes_in_cmd = 8;
+   info->max_surface_size = 36 * 1024 * 1024;
info->msi_cap_offset = pdev->msi_cap;
 }
 
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 04/12] drm/i915/gvt: Enable gtt initialization for BXT.

[Colin Xu]

Initialize BXT gtt as SKL/KBL.

v2: All supported platforms share the same gtt ops.
Remove the platform check by now and let is_supported_device()
be the gate keeper.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/gvt/gtt.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 78e55aafc8bc..22cc5d4c63bb 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -2256,13 +2256,8 @@ int intel_gvt_init_gtt(struct intel_gvt *gvt)
 
gvt_dbg_core("init gtt\n");
 
-   if (IS_BROADWELL(gvt->dev_priv) || IS_SKYLAKE(gvt->dev_priv)
-   || IS_KABYLAKE(gvt->dev_priv)) {
-   gvt->gtt.pte_ops = &gen8_gtt_pte_ops;
-   gvt->gtt.gma_ops = &gen8_gtt_gma_ops;
-   } else {
-   return -ENODEV;
-   }
+   gvt->gtt.pte_ops = &gen8_gtt_pte_ops;
+   gvt->gtt.gma_ops = &gen8_gtt_gma_ops;
 
page = (void *)get_zeroed_page(GFP_KERNEL);
if (!page) {
-- 
2.17.1

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

[Intel-gfx] [PATCH v3 00/11] drm/i915/gvt: Enable KVMGT for BXT

[Colin Xu]

The patch set will enable KVMGT on Broxton platform.
Device type define will be added first then per-functional enablement.
The overall switch will be enabled in intel_gvt.c at last.

v3:
  - Add global enablement is_supported_device() into patch series.

v2:
  - Remove unnecessary platform detect if all supported platforms share the
same configuration set. is_supported_device() will be the gatekeeper.
  - Enable dma_buf for BXT.
  - Add inhibit_context restore for BXT.

Colin Xu (12):
  drm/i915/gvt: Add D_BXT device type define for BXT.
  drm/i915/gvt: Add MEDIA_POOL_STATE for BXT.
  drm/i915/gvt: Enable device info initialization for BXT.
  drm/i915/gvt: Enable gtt initialization for BXT.
  drm/i915/gvt: Enable irq initialization for BXT.
  drm/i915/gvt: Enable mmio context init and switch for BXT.
  drm/i915/gvt: Enable cmd_parser support for BXT.
  drm/i915/gvt: Enable force wake support for BXT.
  drm/i915/gvt: Enable virtual display support for BXT.
  drm/i915/gvt: Enable dma_buf support for BXT.
  drm/i915/gvt: Add mmio handler for for BXT.
  drm/i915: Enable KVMGT for BXT.

 drivers/gpu/drm/i915/gvt/cmd_parser.c   |  43 ++-
 drivers/gpu/drm/i915/gvt/display.c  |  23 ++
 drivers/gpu/drm/i915/gvt/dmabuf.c   |   4 +-
 drivers/gpu/drm/i915/gvt/edid.c |  20 +-
 drivers/gpu/drm/i915/gvt/fb_decoder.c   |  12 +-
 drivers/gpu/drm/i915/gvt/gtt.c  |   9 +-
 drivers/gpu/drm/i915/gvt/gvt.c  |  21 +-
 drivers/gpu/drm/i915/gvt/handlers.c | 391 +---
 drivers/gpu/drm/i915/gvt/interrupt.c|  14 +-
 drivers/gpu/drm/i915/gvt/mmio.h |  11 +-
 drivers/gpu/drm/i915/gvt/mmio_context.c |  16 +-
 drivers/gpu/drm/i915/gvt/scheduler.c|   6 +-
 drivers/gpu/drm/i915/intel_gvt.c|   2 +
 13 files changed, 457 insertions(+), 115 deletions(-)

-- 
2.17.1

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

Re: [Intel-gfx] [PATCH 2/4] drm/i915: clean up virtual PCH special case handling

[Colin Xu]

On 05/31/2018 07:56 PM, Jani Nikula wrote:

Use intel_pch_type() also for mapping the no PCH case (PCH id 0) to
PCH_NONE to simplify code.

Also make sure that intel_pch_type() knows all the PCH ids returned by
intel_virt_detect_pch(). Loudly fail if this isn't the case; this
shouldn't happen anyway.

Cc: Colin Xu 
Reviewed-by: Ville Syrjälä 
Signed-off-by: Jani Nikula 
---
  drivers/gpu/drm/i915/i915_drv.c | 13 ++---
  1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c42e389a27f3..1842a067a604 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -282,13 +282,12 @@ static void intel_detect_pch(struct drm_i915_private 
*dev_priv)
} else if (intel_is_virt_pch(id, pch->subsystem_vendor,
 pch->subsystem_device)) {
id = intel_virt_detect_pch(dev_priv);
-   if (id) {
-   pch_type = intel_pch_type(dev_priv, id);
-   if (WARN_ON(pch_type == PCH_NONE))
-   pch_type = PCH_NOP;
-   } else {
-   pch_type = PCH_NONE;
-   }
+   pch_type = intel_pch_type(dev_priv, id);
+
+   /* Sanity check virtual PCH id */
+   if (WARN_ON(id && pch_type == PCH_NONE))
+   id = 0;
+
dev_priv->pch_type = pch_type;
dev_priv->pch_id = id;
break;


Tested on BXT gvt-g and got expected behaviour.

Tested-by: Colin Xu 
Reviewed-by: Colin Xu 

--
Best Regards,
Colin Xu

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

Re: [Intel-gfx] [PATCH 1/4] drm/i915: fix guest virtual PCH detection on non-PCH systems

[Colin Xu]

On 05/31/2018 07:56 PM, Jani Nikula wrote:

Virtualized non-PCH systems such as Broxton or Geminilake should use
PCH_NONE to indicate no PCH rather than PCH_NOP. The latter is a
specific case to indicate a PCH system without south display.

Reported-by: Colin Xu 
Cc: Colin Xu 
Reviewed-by: Ville Syrjälä 
Signed-off-by: Jani Nikula 
---
  drivers/gpu/drm/i915/i915_drv.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8f002ae22e62..c42e389a27f3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -287,7 +287,7 @@ static void intel_detect_pch(struct drm_i915_private 
*dev_priv)
if (WARN_ON(pch_type == PCH_NONE))
pch_type = PCH_NOP;
} else {
-   pch_type = PCH_NOP;
+   pch_type = PCH_NONE;
}
dev_priv->pch_type = pch_type;
dev_priv->pch_id = id;


Tested on BXT gvt-g and got expected behaviour.

Tested-by: Colin Xu 
Reviewed-by: Colin Xu 

--
Best Regards,
Colin Xu

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Update virtual PCH in single function

[Colin Xu]

On 05/29/2018 01:45 PM, Jani Nikula wrote:


On Wed, 30 May 2018, Colin Xu  wrote:

On 05/28/2018 09:42 PM, Jani Nikula wrote:

On Mon, 28 May 2018, Jani Nikula  wrote:

On Mon, 28 May 2018, Jani Nikula  wrote:

On Tue, 29 May 2018, colin...@intel.com wrote:

From: Colin Xu 

The existing way to update virtual PCH will return wrong PCH type
in case the host doesn't have PCH:
- intel_virt_detect_pch returns guessed PCH id 0
- id 0 maps to PCH_NOP. >> should be PCH_NONE.
Since PCH_NONE and PCH_NOP are different types, mixing them up
will break vbt initialization logic.

In addition, to add new none/nop PCH override for a specific
platform, branching need to be added to intel_virt_detect_pch(),
intel_pch_type() and the caller since none/nop PCH is not always
mapping to the same predefined PCH id.

This patch merges the virtual PCH update/sanity check logic into
single function intel_virt_update_pch(), which still keeps using
existing intel_pch_type() to do the sanity check, while making it
clean to override virtual PCH id for a specific platform for future
platform enablement.

Please keep the assignment out of intel_virt_{detect,update}_pch like. I
think the patch here is unnecessarily complicated.

To elaborate, intel_pch_type() should *always* be able to map pch id to
pch type. There should not be combinations that aren't covered by
that. If the sanity checks there need to accept Broxton as well, perhaps
pass a parameter to indicate virtualization, and accept certain pch ids
for Broxton as well.

If you're faking a pch for Broxton, I don't think there's a case where
pch id should be 0 and pch type should be something else. Either both
are zero, or both are non-zero.

-ENOCOFFEE in the morning. Is the fix you're looking for simply:

Yes this is the most simply way.
The reason I didn't craft the patch like this in the beginning is that
I'm not sure after your refactoring patch, if the case exists that pch
id 0 maps to type either nop or none.
As you said there is no such case, the simply change should work well.
Will you made the change sometime or I need update my patch set?

I was trying to look at which part of my refactoring broke this, but it
seems to me it was already setting pch_type to PCH_NOP before that.

Do you have a bisect result?


It doesnt' seem being broken by the refactoring. Since Broxton isn't supported
in virtualization environemtn before, the issue is covered up.
In native case, intel_pch_type() returns none since there is no PCH hardware
and it works correctly. In virutalization, we expect the same.
--
Best Regards,
Colin Xu



BR,
Jani.





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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Update virtual PCH in single function

[Colin Xu]

On 05/28/2018 09:42 PM, Jani Nikula wrote:

On Mon, 28 May 2018, Jani Nikula  wrote:

On Mon, 28 May 2018, Jani Nikula  wrote:

On Tue, 29 May 2018, colin...@intel.com wrote:

From: Colin Xu 

The existing way to update virtual PCH will return wrong PCH type
in case the host doesn't have PCH:
   - intel_virt_detect_pch returns guessed PCH id 0
   - id 0 maps to PCH_NOP. >> should be PCH_NONE.
Since PCH_NONE and PCH_NOP are different types, mixing them up
will break vbt initialization logic.

In addition, to add new none/nop PCH override for a specific
platform, branching need to be added to intel_virt_detect_pch(),
intel_pch_type() and the caller since none/nop PCH is not always
mapping to the same predefined PCH id.

This patch merges the virtual PCH update/sanity check logic into
single function intel_virt_update_pch(), which still keeps using
existing intel_pch_type() to do the sanity check, while making it
clean to override virtual PCH id for a specific platform for future
platform enablement.

Please keep the assignment out of intel_virt_{detect,update}_pch like. I
think the patch here is unnecessarily complicated.

To elaborate, intel_pch_type() should *always* be able to map pch id to
pch type. There should not be combinations that aren't covered by
that. If the sanity checks there need to accept Broxton as well, perhaps
pass a parameter to indicate virtualization, and accept certain pch ids
for Broxton as well.

If you're faking a pch for Broxton, I don't think there's a case where
pch id should be 0 and pch type should be something else. Either both
are zero, or both are non-zero.

-ENOCOFFEE in the morning. Is the fix you're looking for simply:


Yes this is the most simply way.
The reason I didn't craft the patch like this in the beginning is that
I'm not sure after your refactoring patch, if the case exists that pch
id 0 maps to type either nop or none.
As you said there is no such case, the simply change should work well.
Will you made the change sometime or I need update my patch set?
--
Best Regards,
Colin Xu


diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9c449b8d8eab..ae07e36e364c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -287,7 +287,7 @@ static void intel_detect_pch(struct drm_i915_private 
*dev_priv)
if (WARN_ON(pch_type == PCH_NONE))
pch_type = PCH_NOP;
} else {
-   pch_type = PCH_NOP;
+   pch_type = PCH_NONE;
}
dev_priv->pch_type = pch_type;
dev_priv->pch_id = id;

---

BR,
Jani.




BR,
Jani






BR,
Jani.



Signed-off-by: Colin Xu 
---
  drivers/gpu/drm/i915/i915_drv.c | 56 ++---
  1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index fb39e40c0847..637ba86104be 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -209,10 +209,11 @@ static bool intel_is_virt_pch(unsigned short id,
 sdevice == PCI_SUBDEVICE_ID_QEMU));
  }
  
-static unsigned short

-intel_virt_detect_pch(const struct drm_i915_private *dev_priv)
+static void
+intel_virt_update_pch(struct drm_i915_private *dev_priv)
  {
unsigned short id = 0;
+   enum intel_pch pch_type = PCH_NONE;
  
  	/*

 * In a virtualized passthrough environment we can be in a
@@ -221,25 +222,37 @@ intel_virt_detect_pch(const struct drm_i915_private 
*dev_priv)
 * make an educated guess as to which PCH is really there.
 */
  
-	if (IS_GEN5(dev_priv))

+   if (IS_GEN5(dev_priv)) {
id = INTEL_PCH_IBX_DEVICE_ID_TYPE;
-   else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
+   pch_type = intel_pch_type(dev_priv, id);
+   DRM_DEBUG_KMS("Assuming Ibex Peak PCH id %04x\n", id);
+   } else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv)) {
id = INTEL_PCH_CPT_DEVICE_ID_TYPE;
-   else if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
-   id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
-   else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-   id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
-   else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
+   pch_type = intel_pch_type(dev_priv, id);
+   DRM_DEBUG_KMS("Assuming CougarPoint PCH id %04x\n", id);
+   } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
+   if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
+   id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
+   else
+   id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
+   pch_type = intel_pch_type(dev_priv, i

[Intel-gfx] [PATCH 2/2] drm/i915: Assign PCH_NONE for BXT in virtualization

[colin . xu]

From: Colin Xu 

On BXT platform, guest kernel request PCH_NONE to initialize display
correctly.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/i915_drv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 637ba86104be..a09516a348e1 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -245,6 +245,10 @@ intel_virt_update_pch(struct drm_i915_private *dev_priv)
id = INTEL_PCH_CNP_DEVICE_ID_TYPE;
pch_type = intel_pch_type(dev_priv, id);
DRM_DEBUG_KMS("Assuming CannonPoint PCH id %04x\n", id);
+   } else if (IS_BROXTON(dev_priv)) {
+   id = 0;
+   pch_type = intel_pch_type(dev_priv, id);
+   DRM_DEBUG_KMS("Assuming no PCH\n");
} else {
id = 0;
pch_type = PCH_NOP;
-- 
2.17.0

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

[Intel-gfx] [PATCH 0/2] drm/i915: Fix incorrect virtual PCH_NONE/PCH_NOP assignment

[colin . xu]

From: Colin Xu 

In recent virtual PCH detection refactoring patch, virtual pch id
is guessed and sanity checked from dev_priv. However, on platform that
has no PCH like BXT, the guessing is wrong by mixing up PCH_NONE with
PCH_NOP.
The patch set handles such situation so that correct virtual PCH_NONE or
PCH_NOP type could be assigned accordingly. And it also add BXT to virtual
PCH detection logic.

Colin Xu (2):
  drm/i915: Update virtual PCH in single function
  drm/i915: Assign PCH_NONE for BXT in virtualization

 drivers/gpu/drm/i915/i915_drv.c | 58 +++--
 1 file changed, 33 insertions(+), 25 deletions(-)

-- 
2.17.0

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

[Intel-gfx] [PATCH 1/2] drm/i915: Update virtual PCH in single function

[colin . xu]

From: Colin Xu 

The existing way to update virtual PCH will return wrong PCH type
in case the host doesn't have PCH:
  - intel_virt_detect_pch returns guessed PCH id 0
  - id 0 maps to PCH_NOP. >> should be PCH_NONE.
Since PCH_NONE and PCH_NOP are different types, mixing them up
will break vbt initialization logic.

In addition, to add new none/nop PCH override for a specific
platform, branching need to be added to intel_virt_detect_pch(),
intel_pch_type() and the caller since none/nop PCH is not always
mapping to the same predefined PCH id.

This patch merges the virtual PCH update/sanity check logic into
single function intel_virt_update_pch(), which still keeps using
existing intel_pch_type() to do the sanity check, while making it
clean to override virtual PCH id for a specific platform for future
platform enablement.

Signed-off-by: Colin Xu 
---
 drivers/gpu/drm/i915/i915_drv.c | 56 ++---
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index fb39e40c0847..637ba86104be 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -209,10 +209,11 @@ static bool intel_is_virt_pch(unsigned short id,
 sdevice == PCI_SUBDEVICE_ID_QEMU));
 }
 
-static unsigned short
-intel_virt_detect_pch(const struct drm_i915_private *dev_priv)
+static void
+intel_virt_update_pch(struct drm_i915_private *dev_priv)
 {
unsigned short id = 0;
+   enum intel_pch pch_type = PCH_NONE;
 
/*
 * In a virtualized passthrough environment we can be in a
@@ -221,25 +222,37 @@ intel_virt_detect_pch(const struct drm_i915_private 
*dev_priv)
 * make an educated guess as to which PCH is really there.
 */
 
-   if (IS_GEN5(dev_priv))
+   if (IS_GEN5(dev_priv)) {
id = INTEL_PCH_IBX_DEVICE_ID_TYPE;
-   else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
+   pch_type = intel_pch_type(dev_priv, id);
+   DRM_DEBUG_KMS("Assuming Ibex Peak PCH id %04x\n", id);
+   } else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv)) {
id = INTEL_PCH_CPT_DEVICE_ID_TYPE;
-   else if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
-   id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
-   else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-   id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
-   else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
+   pch_type = intel_pch_type(dev_priv, id);
+   DRM_DEBUG_KMS("Assuming CougarPoint PCH id %04x\n", id);
+   } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
+   if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
+   id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
+   else
+   id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
+   pch_type = intel_pch_type(dev_priv, id);
+   DRM_DEBUG_KMS("Assuming LynxPoint PCH id %04x\n", id);
+   } else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
id = INTEL_PCH_SPT_DEVICE_ID_TYPE;
-   else if (IS_COFFEELAKE(dev_priv) || IS_CANNONLAKE(dev_priv))
+   pch_type = intel_pch_type(dev_priv, id);
+   DRM_DEBUG_KMS("Assuming SunrisePoint PCH id %04x\n", id);
+   } else if (IS_COFFEELAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
id = INTEL_PCH_CNP_DEVICE_ID_TYPE;
+   pch_type = intel_pch_type(dev_priv, id);
+   DRM_DEBUG_KMS("Assuming CannonPoint PCH id %04x\n", id);
+   } else {
+   id = 0;
+   pch_type = PCH_NOP;
+   DRM_DEBUG_KMS("Assuming NOP PCH\n");
+   }
 
-   if (id)
-   DRM_DEBUG_KMS("Assuming PCH ID %04x\n", id);
-   else
-   DRM_DEBUG_KMS("Assuming no PCH\n");
-
-   return id;
+   dev_priv->pch_type = pch_type;
+   dev_priv->pch_id = id;
 }
 
 static void intel_detect_pch(struct drm_i915_private *dev_priv)
@@ -281,16 +294,7 @@ static void intel_detect_pch(struct drm_i915_private 
*dev_priv)
break;
} else if (intel_is_virt_pch(id, pch->subsystem_vendor,
 pch->subsystem_device)) {
-   id = intel_virt_detect_pch(dev_priv);
-   if (id) {
-   pch_type = intel_pch_type(dev_priv, id);
-   if (WARN_ON(pch_type == PCH_NONE))
-   pch_type = PCH_NOP;
-   } else {
-   pch_type = PCH_NOP;
-   }
-   dev_priv->pch_type = pch_type;
-   dev_priv->pch_id = id;
+