Re: [PATCH v2 6/6] drm/i915/gvt: decouple check_vgpu() from uncore_init()

[Zhenyu Wang <zhenyuw@linux.intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5694 bytes --]

On 2019.06.19 18:00:21 -0700, Daniele Ceraolo Spurio wrote:
> With multiple uncore to initialize (GT vs Display), it makes little
> sense to have the vgpu_check inside uncore_init(). We also have
> a catch-22 scenario where the uncore is required to read the vgpu
> capabilities while the vgpu capabilities are required to decide if
> we need to initialize forcewake support. To remove this circular
> dependency, we can perform the required MMIO access by mmapping just
> the vgtif shared page in mmio space and use raw accessors.
> 
> v2: rename check_vgpu to detect_vgpu (Chris)
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

Looks good to me.

Acked-by: Zhenyu Wang <zhenyuw@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c     |  2 ++
>  drivers/gpu/drm/i915/i915_pvinfo.h  |  5 +++--
>  drivers/gpu/drm/i915/i915_vgpu.c    | 35 ++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/i915_vgpu.h    |  2 +-
>  drivers/gpu/drm/i915/intel_uncore.c |  2 --
>  5 files changed, 33 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 00b6512cdee6..8f84ed26e972 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1899,6 +1899,8 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>  
> +	i915_detect_vgpu(dev_priv);
> +
>  	ret = i915_driver_init_mmio(dev_priv);
>  	if (ret < 0)
>  		goto out_runtime_pm_put;
> diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
> index 969e514916ab..ca4661e98f79 100644
> --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> @@ -110,8 +110,9 @@ struct vgt_if {
>  	u32  rsv7[0x200 - 24];    /* pad to one page */
>  } __packed;
>  
> -#define vgtif_reg(x) \
> -	_MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x)))
> +#define vgtif_offset(x) (offsetof(struct vgt_if, x))
> +
> +#define vgtif_reg(x) _MMIO(VGT_PVINFO_PAGE + vgtif_offset(x))
>  
>  /* vGPU display status to be used by the host side */
>  #define VGT_DRV_DISPLAY_NOT_READY 0
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index 94d3992b599d..1a8f7c731126 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -52,34 +52,53 @@
>   */
>  
>  /**
> - * i915_check_vgpu - detect virtual GPU
> + * i915_detect_vgpu - detect virtual GPU
>   * @dev_priv: i915 device private
>   *
>   * This function is called at the initialization stage, to detect whether
>   * running on a vGPU.
>   */
> -void i915_check_vgpu(struct drm_i915_private *dev_priv)
> +void i915_detect_vgpu(struct drm_i915_private *dev_priv)
>  {
> -	struct intel_uncore *uncore = &dev_priv->uncore;
> +	struct pci_dev *pdev = dev_priv->drm.pdev;
>  	u64 magic;
>  	u16 version_major;
> +	void __iomem *shared_area;
>  
>  	BUILD_BUG_ON(sizeof(struct vgt_if) != VGT_PVINFO_SIZE);
>  
> -	magic = __raw_uncore_read64(uncore, vgtif_reg(magic));
> -	if (magic != VGT_MAGIC)
> +	/*
> +	 * This is called before we setup the main MMIO BAR mappings used via
> +	 * the uncore structure, so we need to access the BAR directly. Since
> +	 * we do not support VGT on older gens, return early so we don't have
> +	 * to consider differently numbered or sized MMIO bars
> +	 */
> +	if (INTEL_GEN(dev_priv) < 6)
> +		return;
> +
> +	shared_area = pci_iomap_range(pdev, 0, VGT_PVINFO_PAGE, VGT_PVINFO_SIZE);
> +	if (!shared_area) {
> +		DRM_ERROR("failed to map MMIO bar to check for VGT\n");
>  		return;
> +	}
> +
> +	magic = readq(shared_area + vgtif_offset(magic));
> +	if (magic != VGT_MAGIC)
> +		goto out;
>  
> -	version_major = __raw_uncore_read16(uncore, vgtif_reg(version_major));
> +	version_major = readw(shared_area + vgtif_offset(version_major));
>  	if (version_major < VGT_VERSION_MAJOR) {
>  		DRM_INFO("VGT interface version mismatch!\n");
> -		return;
> +		goto out;
>  	}
>  
> -	dev_priv->vgpu.caps = __raw_uncore_read32(uncore, vgtif_reg(vgt_caps));
> +	dev_priv->vgpu.caps = readl(shared_area + vgtif_offset(vgt_caps));
>  
>  	dev_priv->vgpu.active = true;
>  	DRM_INFO("Virtual GPU for Intel GVT-g detected.\n");
> +
> +out:
> +	pci_iounmap(pdev, shared_area);
>  }
>  
>  bool intel_vgpu_has_full_ppgtt(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
> index ebe1b7bced98..a919735fb6ce 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -26,7 +26,7 @@
>  
>  #include "i915_pvinfo.h"
>  
> -void i915_check_vgpu(struct drm_i915_private *dev_priv);
> +void i915_detect_vgpu(struct drm_i915_private *dev_priv);
>  
>  bool intel_vgpu_has_full_ppgtt(struct drm_i915_private *dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 2bd602a41bb7..68d54e126d79 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1650,8 +1650,6 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
>  	if (ret)
>  		return ret;
>  
> -	i915_check_vgpu(i915);
> -
>  	if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
>  		uncore->flags |= UNCORE_HAS_FORCEWAKE;
>  
> -- 
> 2.20.1
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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