[Intel-gfx] [PATCH] drm/i915: Detect virtual south bridge

[Daniel Vetter]

On Fri, Sep 25, 2015 at 09:52:48AM -0700, Jesse Barnes wrote:
> On 09/25/2015 08:31 AM, Jesse Barnes wrote:
> > On 09/24/2015 08:41 PM, Tian, Kevin wrote:
> >>> From: Jesse Barnes [mailto:jbarnes at virtuousgeek.org]
> >>> Sent: Friday, September 25, 2015 12:41 AM
> >>>
> >>> On 08/28/2015 05:10 AM, robert.beckett at intel.com wrote:
> >>>> From: Robert Beckett <robert.beckett at intel.com>
> >>>>
> >>>> Virtualized systems often use a virtual P2X4 south bridge.
> >>>> Detect this in intel_detect_pch and make a best guess as to which PCH
> >>>> we should be using.
> >>>>
> >>>> This was seen on vmware esxi hypervisor. When passing the graphics device
> >>>> through to a guest, it can not pass through the PCH. Instead it simulates
> >>>> a P2X4 southbridge.
> >>>>
> >>>> Signed-off-by: Robert Beckett <robert.beckett at intel.com>
> >>>> ---
> >>>>  drivers/gpu/drm/i915/i915_drv.c |   30
> >>> ++++++++++++++++++++++++++++++
> >>>>  drivers/gpu/drm/i915/i915_drv.h |    1 +
> >>>>  2 files changed, 31 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >>>> index ce3bd0c..8e158b3 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_drv.c
> >>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >>>> @@ -443,6 +443,34 @@ static const struct pci_device_id pciidlist[] = {		/* aka
> >>> */
> >>>>
> >>>>  MODULE_DEVICE_TABLE(pci, pciidlist);
> >>>>
> >>>> +static enum intel_pch intel_virt_detect_pch(struct drm_device *dev)
> >>>> +{
> >>>> +	enum intel_pch ret = PCH_NOP;
> >>>> +
> >>>> +	/*
> >>>> +	 * In a virtualized passthrough environment we can be in a
> >>>> +	 * setup where the ISA bridge is not able to be passed through.
> >>>> +	 * In this case, a south bridge can be emulated and we have to
> >>>> +	 * make an educated guess as to which PCH is really there.
> >>>> +	 */
> >>>> +
> >>>> +	if (IS_GEN5(dev)) {
> >>>> +		ret = PCH_IBX;
> >>>> +		DRM_DEBUG_KMS("Assuming Ibex Peak PCH\n");
> >>>> +	} else if (IS_GEN6(dev) || IS_IVYBRIDGE(dev)) {
> >>>> +		ret = PCH_CPT;
> >>>> +		DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
> >>>> +	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> >>>> +		ret = PCH_LPT;
> >>>> +		DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
> >>>> +	} else if (IS_SKYLAKE(dev)) {
> >>>> +		ret = PCH_SPT;
> >>>> +		DRM_DEBUG_KMS("Assuming SunrisePoint PCH\n");
> >>>> +	}
> >>>> +
> >>>> +	return ret;
> >>>> +}
> >>>> +
> >>>>  void intel_detect_pch(struct drm_device *dev)
> >>>>  {
> >>>>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>>> @@ -503,6 +531,8 @@ void intel_detect_pch(struct drm_device *dev)
> >>>>  				dev_priv->pch_type = PCH_SPT;
> >>>>  				DRM_DEBUG_KMS("Found SunrisePoint LP PCH\n");
> >>>>  				WARN_ON(!IS_SKYLAKE(dev));
> >>>> +			} else if (id == INTEL_PCH_P2X_DEVICE_ID_TYPE) {
> >>>> +				dev_priv->pch_type = intel_virt_detect_pch(dev);
> >>>>  			} else
> >>>>  				continue;
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>> index 8c93845..6eb0230 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>> @@ -2584,6 +2584,7 @@ struct drm_i915_cmd_table {
> >>>>  #define INTEL_PCH_LPT_LP_DEVICE_ID_TYPE		0x9c00
> >>>>  #define INTEL_PCH_SPT_DEVICE_ID_TYPE		0xA100
> >>>>  #define INTEL_PCH_SPT_LP_DEVICE_ID_TYPE		0x9D00
> >>>> +#define INTEL_PCH_P2X_DEVICE_ID_TYPE		0x7100
> >>>>
> >>>>  #define INTEL_PCH_TYPE(dev) (__I915__(dev)->pch_type)
> >>>>  #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT)
> >>>>
> >>>
> >>> Assuming Kevin is ok with this:
> >>> Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> >>
> >> Yes, I'm OK with this change. Just one comment. Does it make
> >> sense to always have the guess as the fallback, instead of only
> >> for P2X? If native side is OK with this, it will remove the need 
> >> to add more IDs for different hypervisors in the future...
> > 
> > Yeah, at this point I don't think we have mix & match cases to worry
> > about, so we could guess based on the GPU type across the board.
> 
> On second thought, let's get this version merged first, then play with
> assuming a bridge type in a later patch, in case that breaks something.

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch