diff mbox

drm/i915: Detect virtual south bridge

Message ID 1440763822-11141-1-git-send-email-robert.beckett@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

robert.beckett@intel.com Aug. 28, 2015, 12:10 p.m. UTC
From: Robert Beckett <robert.beckett@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@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |   30 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h |    1 +
 2 files changed, 31 insertions(+)

Comments

Daniel Vetter Sept. 2, 2015, 10:11 a.m. UTC | #1
On Fri, Aug 28, 2015 at 01:10:22PM +0100, robert.beckett@intel.com wrote:
> From: Robert Beckett <robert.beckett@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@intel.com>

What changed here compared to your earlier submission a day before?
-Daniel

> ---
>  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)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
robert.beckett@intel.com Sept. 7, 2015, 10:38 a.m. UTC | #2
On 02/09/2015 11:11, Daniel Vetter wrote:
> On Fri, Aug 28, 2015 at 01:10:22PM +0100, robert.beckett@intel.com wrote:
>> From: Robert Beckett <robert.beckett@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@intel.com>
>
> What changed here compared to your earlier submission a day before?
> -Daniel

Nothing functional, just corrected the "[PATCH]" tag in the subject. My 
email client was doing weird things...

>
>> ---
>>   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)
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Jesse Barnes Sept. 24, 2015, 4:40 p.m. UTC | #3
On 08/28/2015 05:10 AM, robert.beckett@intel.com wrote:
> From: Robert Beckett <robert.beckett@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@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@virtuousgeek.org>
Tian, Kevin Sept. 25, 2015, 3:41 a.m. UTC | #4
> From: Jesse Barnes [mailto:jbarnes@virtuousgeek.org]

> Sent: Friday, September 25, 2015 12:41 AM

> 

> On 08/28/2015 05:10 AM, robert.beckett@intel.com wrote:

> > From: Robert Beckett <robert.beckett@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@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@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...

Thanks
Kevin
Jesse Barnes Sept. 25, 2015, 3:31 p.m. UTC | #5
On 09/24/2015 08:41 PM, Tian, Kevin wrote:
>> From: Jesse Barnes [mailto:jbarnes@virtuousgeek.org]
>> Sent: Friday, September 25, 2015 12:41 AM
>>
>> On 08/28/2015 05:10 AM, robert.beckett@intel.com wrote:
>>> From: Robert Beckett <robert.beckett@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@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@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.

Jesse
Jesse Barnes Sept. 25, 2015, 4:52 p.m. UTC | #6
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@virtuousgeek.org]
>>> Sent: Friday, September 25, 2015 12:41 AM
>>>
>>> On 08/28/2015 05:10 AM, robert.beckett@intel.com wrote:
>>>> From: Robert Beckett <robert.beckett@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@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@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.

Jesse
Daniel Vetter Sept. 28, 2015, 6:46 a.m. UTC | #7
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@virtuousgeek.org]
> >>> Sent: Friday, September 25, 2015 12:41 AM
> >>>
> >>> On 08/28/2015 05:10 AM, robert.beckett@intel.com wrote:
> >>>> From: Robert Beckett <robert.beckett@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@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@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
diff mbox

Patch

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)