[Intel-gfx] [PATCH v2] drm/i915: Try to guess PCH type even without ISA bridge

[Zhenyu Wang]

On 2020.12.24 02:54:18 +0000, Zhang, Xiong Y wrote:
> > On Fri, 18 Dec 2020, Xiong Zhang <xiong.y.zhang at intel.com> wrote:
> > > From: Zhenyu Wang <zhenyuw at linux.intel.com>
> > >
> > > Some vmm like hyperv and crosvm don't supply any ISA bridge to their
> > > guest, when igd passthrough is equipped on these vmm, guest i915
> > > display may couldn't work as guest i915 detects PCH_NONE pch type.
> > >
> > > When i915 runs as guest, this patch guess pch type through gpu type
> > > even without ISA bridge.
> > >
> > > v2: Fix CI warning
> > >
> > > Signed-off-by: Zhenyu Wang <zhenyuw at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h  |  7 +++++-
> > > drivers/gpu/drm/i915/intel_pch.c | 38 ++++++++++++++++++++++----------
> > >  2 files changed, 32 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h index 5a7df5621aa3..df0b8f9268b2
> > > 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1758,6 +1758,11 @@ tgl_revids_get(struct drm_i915_private
> > > *dev_priv)  #define INTEL_DISPLAY_ENABLED(dev_priv) \
> > >  	(drm_WARN_ON(&(dev_priv)->drm, !HAS_DISPLAY(dev_priv)),
> > > !(dev_priv)->params.disable_display)
> > >
> > > +static inline bool run_as_guest(void) {
> > > +	return !hypervisor_is_type(X86_HYPER_NATIVE);
> > > +}
> > > +
> > >  static inline bool intel_vtd_active(void)  {  #ifdef
> > > CONFIG_INTEL_IOMMU @@ -1766,7 +1771,7 @@ static inline bool
> > > intel_vtd_active(void)  #endif
> > >
> > >  	/* Running as a guest, we assume the host is enforcing VT'd */
> > > -	return !hypervisor_is_type(X86_HYPER_NATIVE);
> > > +	return run_as_guest();
> > >  }
> > >
> > >  static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private
> > > *dev_priv) diff --git a/drivers/gpu/drm/i915/intel_pch.c
> > > b/drivers/gpu/drm/i915/intel_pch.c
> > > index f31c0dabd0cc..a73c60bf349e 100644
> > > --- a/drivers/gpu/drm/i915/intel_pch.c
> > > +++ b/drivers/gpu/drm/i915/intel_pch.c
> > > @@ -184,6 +184,23 @@ intel_virt_detect_pch(const struct
> > drm_i915_private *dev_priv)
> > >  	return id;
> > >  }
> > >
> > > +static void intel_detect_pch_virt(struct drm_i915_private *dev_priv)
> > > +{
> > > +	unsigned short id;
> > > +	enum intel_pch pch_type;
> > > +
> > > +	id = intel_virt_detect_pch(dev_priv);
> > 
> > intel_detect_pch_virt() calls intel_virt_detect_pch()... the naming should be
> > clarified to make some difference.
>  [Zhang, Xiong Y] Indeed the naming is confusing. How about deleting this new added function, then move intel_pch_type() calling into original intel_virt_detect_pch(), and let intel_virt_detect_pch() return id and pch_type,  finally assign id and pch_type  in intel_detect_pch().
> 

May just put those in intel_virt_detect_pch() for all virt cases.

> > 
> > > +	pch_type = intel_pch_type(dev_priv, id);
> > > +
> > > +	/* Sanity check virtual PCH id */
> > > +	if (drm_WARN_ON(&dev_priv->drm,
> > > +			id && pch_type == PCH_NONE))
> > > +		id = 0;
> > > +
> > > +	dev_priv->pch_type = pch_type;
> > > +	dev_priv->pch_id = id;
> > 
> > Previously the pch type and id assignments were all done in
> > intel_detect_pch(), so moving this to a separate function in *some* but not
> > all cases reduces clarity too.
> > 
> > BR,
> > Jani.
> > 
> > > +}
> > > +
> > >  void intel_detect_pch(struct drm_i915_private *dev_priv)  {
> > >  	struct pci_dev *pch = NULL;
> > > @@ -221,16 +238,7 @@ 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);
> > > -			pch_type = intel_pch_type(dev_priv, id);
> > > -
> > > -			/* Sanity check virtual PCH id */
> > > -			if (drm_WARN_ON(&dev_priv->drm,
> > > -					id && pch_type == PCH_NONE))
> > > -				id = 0;
> > > -
> > > -			dev_priv->pch_type = pch_type;
> > > -			dev_priv->pch_id = id;
> > > +			intel_detect_pch_virt(dev_priv);
> > >  			break;
> > >  		}
> > >  	}
> > > @@ -246,8 +254,14 @@ void intel_detect_pch(struct drm_i915_private
> > *dev_priv)
> > >  		dev_priv->pch_id = 0;
> > >  	}
> > >
> > > -	if (!pch)
> > > -		drm_dbg_kms(&dev_priv->drm, "No PCH found.\n");
> > > +	if (!pch) {
> > > +		if (run_as_guest()) {
> > > +			drm_dbg_kms(&dev_priv->drm, "No PCH found in
> > vm, try guess..\n");
> > > +			intel_detect_pch_virt(dev_priv);
> > > +		} else {
> > > +			drm_dbg_kms(&dev_priv->drm, "No PCH found.\n");
> > > +		}
> > > +	}
> > >
> > >  	pci_dev_put(pch);
> > >  }
> > 
> > --
> > Jani Nikula, Intel Open Source Graphics Center

-- 

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20201224/5d738d1c/attachment.sig>