[PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Xiong Zhang]

In a IGD passthrough environment, the real ISA bridge may doesn't exist.
then pch_id couldn't be correctly gotten from ISA bridge, but pch_id is
used to identify LPT_H and LPT_LP. Currently i915 treat all LPT pch as
LPT_H,then errors occur when i915 runs on LPT_LP machines with igd
passthrough.

This patch set pch_id for HSW/BDW according to IGD type and isn't fully
correct. But it solves such issue on HSW/BDW ult/ulx machines.
QA CI system is blocked by this issue for a long time, it's better that
we could merge it to unblock QA CI system.

We know the root cause is in device model of virtual passthrough, and
will resolve it in the future with several parts cooperation in kernel,
qemu and xen.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99938

Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
---
 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 1f802de..2e664c5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -135,6 +135,10 @@ static enum intel_pch intel_virt_detect_pch(struct drm_i915_private *dev_priv)
 		DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
 	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		ret = PCH_LPT;
+		if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
+			dev_priv->pch_id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
+		else
+			dev_priv->pch_id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
 		DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
 	} else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
 		ret = PCH_SPT;
-- 
2.7.4

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

✓ Fi.CI.BAT: success for drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Patchwork]

== Series Details ==

Series: drm/i915: Setting pch_id for HSW/BDW in virtual environment
URL   : https://patchwork.freedesktop.org/series/25772/
State : success

== Summary ==

Series 25772v1 drm/i915: Setting pch_id for HSW/BDW in virtual environment
https://patchwork.freedesktop.org/api/1.0/series/25772/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-warn -> PASS       (fi-bdw-gvtdvm) fdo#99938 +7
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125

fdo#99938 https://bugs.freedesktop.org/show_bug.cgi?id=99938
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:447s
fi-bdw-gvtdvm    total:278  pass:264  dwarn:0   dfail:0   fail:0   skip:14  time:432s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:576s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:511s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:481s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:480s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:591s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:432s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:412s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:420s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:491s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:471s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:467s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:570s
fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:586s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:471s
fi-skl-6700hq    total:278  pass:228  dwarn:1   dfail:0   fail:27  skip:22  time:408s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:468s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:476s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:431s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:529s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:400s

66555696fc0f33fc19eca5068783b95ecc8a91d5 drm-tip: 2017y-06m-14d-09h-56m-45s UTC integration manifest
f30e349 drm/i915: Setting pch_id for HSW/BDW in virtual environment

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4950/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Saarinen, Jani]

HI, 
> -----Original Message-----
> == Series Details ==
> 
> Series: drm/i915: Setting pch_id for HSW/BDW in virtual environment
> URL   : https://patchwork.freedesktop.org/series/25772/
> State : success
> 
> == Summary ==
> 
> Series 25772v1 drm/i915: Setting pch_id for HSW/BDW in virtual
> environment
> https://patchwork.freedesktop.org/api/1.0/series/25772/revisions/1/mbox/
> 
> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 dmesg-warn -> PASS       (fi-bdw-gvtdvm) fdo#99938 +7
>         Subgroup basic-s4-devices:
>                 pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
> 
> fdo#99938 https://bugs.freedesktop.org/show_bug.cgi?id=99938
> fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
> 
Does what is expected:
This: fi-bdw-gvtdvm    total:278  pass:264  dwarn:0   dfail:0   fail:0   skip:14  time:432s
Old:  fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:432s
So with this fixes issues at least. 

> 
> 66555696fc0f33fc19eca5068783b95ecc8a91d5 drm-tip: 2017y-06m-14d-
> 09h-56m-45s UTC integration manifest
> f30e349 drm/i915: Setting pch_id for HSW/BDW in virtual environment
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4950/


Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo


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

Re: ✓ Fi.CI.BAT: success for drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Joonas Lahtinen]

On ke, 2017-06-14 at 11:27 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Setting pch_id for HSW/BDW in virtual environment
> URL   : https://patchwork.freedesktop.org/series/25772/
> State : success

Janie & Daniel,

This patch could be considered for the CI topic branch, because it'll
do the detection right for the machines in the CI, but might worsen
situation for others.

Regards, Joonas

> 
> == Summary ==
> 
> Series 25772v1 drm/i915: Setting pch_id for HSW/BDW in virtual environment
> https://patchwork.freedesktop.org/api/1.0/series/25772/revisions/1/mbox/
> 
> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 dmesg-warn -> PASS       (fi-bdw-gvtdvm) fdo#99938 +7
>         Subgroup basic-s4-devices:
>                 pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
> 
> fdo#99938 https://bugs.freedesktop.org/show_bug.cgi?id=99938
> fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
> 
> fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:447s
> fi-bdw-gvtdvm    total:278  pass:264  dwarn:0   dfail:0   fail:0   skip:14  time:432s
> fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:576s
> fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:511s
> fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:481s
> fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:480s
> fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:591s
> fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:432s
> fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:412s
> fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:420s
> fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:491s
> fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:471s
> fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:467s
> fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:570s
> fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:586s
> fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:471s
> fi-skl-6700hq    total:278  pass:228  dwarn:1   dfail:0   fail:27  skip:22  time:408s
> fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:468s
> fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:476s
> fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:431s
> fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:529s
> fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:400s
> 
> 66555696fc0f33fc19eca5068783b95ecc8a91d5 drm-tip: 2017y-06m-14d-09h-56m-45s UTC integration manifest
> f30e349 drm/i915: Setting pch_id for HSW/BDW in virtual environment
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4950/
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Martin Peres]

On 14/06/17 16:18, Joonas Lahtinen wrote:
> On ke, 2017-06-14 at 11:27 +0000, Patchwork wrote:
>> == Series Details ==
>>
>> Series: drm/i915: Setting pch_id for HSW/BDW in virtual environment
>> URL   : https://patchwork.freedesktop.org/series/25772/
>> State : success
> 
> Janie & Daniel,
> 
> This patch could be considered for the CI topic branch, because it'll
> do the detection right for the machines in the CI, but might worsen
> situation for others.
> 
> Regards, Joonas

I agree. I could merge that now to fix our CI while they are reworking 
the world not to have this problem.

Daniel, what's your take on this?

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

Re: [PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Imre Deak]

On Thu, Jun 15, 2017 at 11:11:45AM +0800, Xiong Zhang wrote:
> In a IGD passthrough environment, the real ISA bridge may doesn't exist.
> then pch_id couldn't be correctly gotten from ISA bridge, but pch_id is
> used to identify LPT_H and LPT_LP. Currently i915 treat all LPT pch as
> LPT_H,then errors occur when i915 runs on LPT_LP machines with igd
> passthrough.
> 
> This patch set pch_id for HSW/BDW according to IGD type and isn't fully
> correct. But it solves such issue on HSW/BDW ult/ulx machines.
> QA CI system is blocked by this issue for a long time, it's better that
> we could merge it to unblock QA CI system.
> 
> We know the root cause is in device model of virtual passthrough, and
> will resolve it in the future with several parts cooperation in kernel,
> qemu and xen.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99938
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>

Looks ok to me, this is the assumption of the non-passthrough case
anyway:
Reviewed-by: Imre Deak <imre.deak@intel.com>

I noticed a few issues in the surrounding code: SPT/SPT_LP and
CNP/CNP_LP have the same problem, although we don't need to distinguish
between them atm. I think a cleaner way would be to adjust id and ext_id
in the pass-through case in intel_detect_pch() upfront and set
dev_priv->pch_type accordingly the same way as it's done in the
non-passthrough case.

Also, unrelated to the pass-through case, the HAS_PCH_CNP_LP() macro
looks bogus, it checks only 8 bits of the PCI device ID instead of 9, so
it'll be false on CNP_LP platforms (and true on SPT_LP).

--Imre

> ---
>  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 1f802de..2e664c5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -135,6 +135,10 @@ static enum intel_pch intel_virt_detect_pch(struct drm_i915_private *dev_priv)
>  		DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
>  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		ret = PCH_LPT;
> +		if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
> +			dev_priv->pch_id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
> +		else
> +			dev_priv->pch_id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
>  		DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
>  	} else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
>  		ret = PCH_SPT;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Zhang, Xiong Y]

> On Thu, Jun 15, 2017 at 11:11:45AM +0800, Xiong Zhang wrote:
> > In a IGD passthrough environment, the real ISA bridge may doesn't exist.
> > then pch_id couldn't be correctly gotten from ISA bridge, but pch_id is
> > used to identify LPT_H and LPT_LP. Currently i915 treat all LPT pch as
> > LPT_H,then errors occur when i915 runs on LPT_LP machines with igd
> > passthrough.
> >
> > This patch set pch_id for HSW/BDW according to IGD type and isn't fully
> > correct. But it solves such issue on HSW/BDW ult/ulx machines.
> > QA CI system is blocked by this issue for a long time, it's better that
> > we could merge it to unblock QA CI system.
> >
> > We know the root cause is in device model of virtual passthrough, and
> > will resolve it in the future with several parts cooperation in kernel,
> > qemu and xen.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99938
> >
> > Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> 
> Looks ok to me, this is the assumption of the non-passthrough case
> anyway:
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> I noticed a few issues in the surrounding code: SPT/SPT_LP and
> CNP/CNP_LP have the same problem, although we don't need to distinguish
> between them atm. I think a cleaner way would be to adjust id and ext_id
> in the pass-through case in intel_detect_pch() upfront and set
> dev_priv->pch_type accordingly the same way as it's done in the
> non-passthrough case.
[Zhang, Xiong Y] Thanks for your suggestion.
pch_id in passthrough is guessed and isn't fully accurate,  so I don't add it to
SPT and CNP. Currently we are seeking acceptable method to pass pch id from
host to guest, once this decided, we will follow your suggestion.
> 
> Also, unrelated to the pass-through case, the HAS_PCH_CNP_LP() macro
> looks bogus, it checks only 8 bits of the PCI device ID instead of 9, so
> it'll be false on CNP_LP platforms (and true on SPT_LP).
[Zhang, Xiong Y] Please help fix it although HAS_PCH_CNP_LP() isn't used atm.
> 
> --Imre
> 
> > ---
> >  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 1f802de..2e664c5 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -135,6 +135,10 @@ static enum intel_pch intel_virt_detect_pch(struct
> drm_i915_private *dev_priv)
> >  		DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
> >  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> >  		ret = PCH_LPT;
> > +		if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
> > +			dev_priv->pch_id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
> > +		else
> > +			dev_priv->pch_id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
> >  		DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
> >  	} else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> >  		ret = PCH_SPT;
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Imre Deak]

On Thu, Jun 15, 2017 at 05:21:23AM +0300, Zhang, Xiong Y wrote:
> [...]
> > Also, unrelated to the pass-through case, the HAS_PCH_CNP_LP() macro
> > looks bogus, it checks only 8 bits of the PCI device ID instead of 9, so
> > it'll be false on CNP_LP platforms (and true on SPT_LP).
> [Zhang, Xiong Y] Please help fix it although HAS_PCH_CNP_LP() isn't
> > used atm

Adding more people to look into this.

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

Re: [PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Jani Nikula]

On Thu, 15 Jun 2017, Xiong Zhang <xiong.y.zhang@intel.com> wrote:
> In a IGD passthrough environment, the real ISA bridge may doesn't exist.
> then pch_id couldn't be correctly gotten from ISA bridge, but pch_id is
> used to identify LPT_H and LPT_LP. Currently i915 treat all LPT pch as
> LPT_H,then errors occur when i915 runs on LPT_LP machines with igd
> passthrough.
>
> This patch set pch_id for HSW/BDW according to IGD type and isn't fully
> correct. But it solves such issue on HSW/BDW ult/ulx machines.
> QA CI system is blocked by this issue for a long time, it's better that
> we could merge it to unblock QA CI system.
>
> We know the root cause is in device model of virtual passthrough, and
> will resolve it in the future with several parts cooperation in kernel,
> qemu and xen.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99938
>
> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>

Please always indicate when you're sending a new version of a patch. For
example, there's plenty of old discussion in the thread starting at [1].


BR,
Jani.


[1] http://mid.mail-archive.com/1490778167-21424-1-git-send-email-xiong.y.zhang@intel.com


> ---
>  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 1f802de..2e664c5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -135,6 +135,10 @@ static enum intel_pch intel_virt_detect_pch(struct drm_i915_private *dev_priv)
>  		DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
>  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		ret = PCH_LPT;
> +		if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
> +			dev_priv->pch_id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
> +		else
> +			dev_priv->pch_id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
>  		DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
>  	} else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
>  		ret = PCH_SPT;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Daniel Vetter]

On Thu, Jun 15, 2017 at 11:11:45AM +0800, Xiong Zhang wrote:
> In a IGD passthrough environment, the real ISA bridge may doesn't exist.
> then pch_id couldn't be correctly gotten from ISA bridge, but pch_id is
> used to identify LPT_H and LPT_LP. Currently i915 treat all LPT pch as
> LPT_H,then errors occur when i915 runs on LPT_LP machines with igd
> passthrough.
> 
> This patch set pch_id for HSW/BDW according to IGD type and isn't fully
> correct. But it solves such issue on HSW/BDW ult/ulx machines.
> QA CI system is blocked by this issue for a long time, it's better that
> we could merge it to unblock QA CI system.
> 
> We know the root cause is in device model of virtual passthrough, and
> will resolve it in the future with several parts cooperation in kernel,
> qemu and xen.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99938
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> ---
>  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 1f802de..2e664c5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -135,6 +135,10 @@ static enum intel_pch intel_virt_detect_pch(struct drm_i915_private *dev_priv)
>  		DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
>  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		ret = PCH_LPT;
> +		if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
> +			dev_priv->pch_id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
> +		else
> +			dev_priv->pch_id = INTEL_PCH_LPT_DEVICE_ID_TYPE;

So it's imo silly that we're leaking the pch_id to our code when we
already have pch_type, but oh well.

Anyway, this is all about the physical pch on the chip, nothing to do with
the virtualized one, and we've been hard-coding these forever.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Afaiui if this isn't true on a machine, someone used a solder iron to
build something that Intel doesn't sell.
-Daniel

>  		DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
>  	} else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
>  		ret = PCH_SPT;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Jani Nikula]

On Thu, 29 Jun 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Jun 15, 2017 at 11:11:45AM +0800, Xiong Zhang wrote:
>> In a IGD passthrough environment, the real ISA bridge may doesn't exist.
>> then pch_id couldn't be correctly gotten from ISA bridge, but pch_id is
>> used to identify LPT_H and LPT_LP. Currently i915 treat all LPT pch as
>> LPT_H,then errors occur when i915 runs on LPT_LP machines with igd
>> passthrough.
>> 
>> This patch set pch_id for HSW/BDW according to IGD type and isn't fully
>> correct. But it solves such issue on HSW/BDW ult/ulx machines.
>> QA CI system is blocked by this issue for a long time, it's better that
>> we could merge it to unblock QA CI system.
>> 
>> We know the root cause is in device model of virtual passthrough, and
>> will resolve it in the future with several parts cooperation in kernel,
>> qemu and xen.
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99938
>> 
>> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
>> ---
>>  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 1f802de..2e664c5 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -135,6 +135,10 @@ static enum intel_pch intel_virt_detect_pch(struct drm_i915_private *dev_priv)
>>  		DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
>>  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>>  		ret = PCH_LPT;
>> +		if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
>> +			dev_priv->pch_id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
>> +		else
>> +			dev_priv->pch_id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
>
> So it's imo silly that we're leaking the pch_id to our code when we
> already have pch_type, but oh well.

It's for distinguishing between LP and non-LP, which we need in the
code, and why we have this patch in the first place.

> Anyway, this is all about the physical pch on the chip, nothing to do with
> the virtualized one, and we've been hard-coding these forever.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Afaiui if this isn't true on a machine, someone used a solder iron to
> build something that Intel doesn't sell.

Bspec says there are (at least) non-ULT/ULX Broadwells with LP PCH. We
do seem to warn about the combo in the bare metal PCH detection, so I
guess it's safe to assume they are rare. But strictly speaking this
change just moves problems from one setup to another.

This has all been covered in the thread that I linked to in my previous
reply, and all of that discussion has been conveniently overlooked and
ignored in this patch submission and the commit message.

I'm not opposed to merging the patch, but this needs to be documented
for posterity.

Of course, this gets *much* more complicated from SKL/SPT on, where the
combos can be mixed even more freely (e.g. SKL+KBP and KBL+SPT).


BR,
Jani.



> -Daniel
>
>>  		DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
>>  	} else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
>>  		ret = PCH_SPT;
>> -- 
>> 2.7.4
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Ville Syrjälä]

On Thu, Jun 29, 2017 at 06:22:45PM +0300, Jani Nikula wrote:
> On Thu, 29 Jun 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Jun 15, 2017 at 11:11:45AM +0800, Xiong Zhang wrote:
> >> In a IGD passthrough environment, the real ISA bridge may doesn't exist.
> >> then pch_id couldn't be correctly gotten from ISA bridge, but pch_id is
> >> used to identify LPT_H and LPT_LP. Currently i915 treat all LPT pch as
> >> LPT_H,then errors occur when i915 runs on LPT_LP machines with igd
> >> passthrough.
> >> 
> >> This patch set pch_id for HSW/BDW according to IGD type and isn't fully
> >> correct. But it solves such issue on HSW/BDW ult/ulx machines.
> >> QA CI system is blocked by this issue for a long time, it's better that
> >> we could merge it to unblock QA CI system.
> >> 
> >> We know the root cause is in device model of virtual passthrough, and
> >> will resolve it in the future with several parts cooperation in kernel,
> >> qemu and xen.
> >> 
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99938
> >> 
> >> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> >> ---
> >>  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 1f802de..2e664c5 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -135,6 +135,10 @@ static enum intel_pch intel_virt_detect_pch(struct drm_i915_private *dev_priv)
> >>  		DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
> >>  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> >>  		ret = PCH_LPT;
> >> +		if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
> >> +			dev_priv->pch_id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
> >> +		else
> >> +			dev_priv->pch_id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
> >
> > So it's imo silly that we're leaking the pch_id to our code when we
> > already have pch_type, but oh well.
> 
> It's for distinguishing between LP and non-LP, which we need in the
> code, and why we have this patch in the first place.

I guess we could add separate pch_types for the LP variants. But I think
that'll just slightly shift the complexity between the
HAS_PCH/HAS_PCH_H/HAS_PCH_LP macros.

Or maybe we should just nuke pch_type?

> 
> > Anyway, this is all about the physical pch on the chip, nothing to do with
> > the virtualized one, and we've been hard-coding these forever.
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > Afaiui if this isn't true on a machine, someone used a solder iron to
> > build something that Intel doesn't sell.
> 
> Bspec says there are (at least) non-ULT/ULX Broadwells with LP PCH. We
> do seem to warn about the combo in the bare metal PCH detection, so I
> guess it's safe to assume they are rare. But strictly speaking this
> change just moves problems from one setup to another.
> 
> This has all been covered in the thread that I linked to in my previous
> reply, and all of that discussion has been conveniently overlooked and
> ignored in this patch submission and the commit message.
> 
> I'm not opposed to merging the patch, but this needs to be documented
> for posterity.
> 
> Of course, this gets *much* more complicated from SKL/SPT on, where the
> combos can be mixed even more freely (e.g. SKL+KBP and KBL+SPT).

Actually I'm not sure we have PCH_KBP since it seems to be identical
to SPT? We don't have PCH_PPT or PCH_WPT either. So IMO we should either
remove PCH_KBP or add PCH_PPT+PCH_WPT.

> 
> 
> BR,
> Jani.
> 
> 
> 
> > -Daniel
> >
> >>  		DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
> >>  	} else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> >>  		ret = PCH_SPT;
> >> -- 
> >> 2.7.4
> >> 
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Zhang, Xiong Y]

> On Thu, 29 Jun 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Jun 15, 2017 at 11:11:45AM +0800, Xiong Zhang wrote:
> >> In a IGD passthrough environment, the real ISA bridge may doesn't exist.
> >> then pch_id couldn't be correctly gotten from ISA bridge, but pch_id is
> >> used to identify LPT_H and LPT_LP. Currently i915 treat all LPT pch as
> >> LPT_H,then errors occur when i915 runs on LPT_LP machines with igd
> >> passthrough.
> >>
> >> This patch set pch_id for HSW/BDW according to IGD type and isn't fully
> >> correct. But it solves such issue on HSW/BDW ult/ulx machines.
> >> QA CI system is blocked by this issue for a long time, it's better that
> >> we could merge it to unblock QA CI system.
> >>
> >> We know the root cause is in device model of virtual passthrough, and
> >> will resolve it in the future with several parts cooperation in kernel,
> >> qemu and xen.
> >>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99938
> >>
> >> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
> >> ---
> >>  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 1f802de..2e664c5 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -135,6 +135,10 @@ static enum intel_pch intel_virt_detect_pch(struct
> drm_i915_private *dev_priv)
> >>  		DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
> >>  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> >>  		ret = PCH_LPT;
> >> +		if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
> >> +			dev_priv->pch_id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
> >> +		else
> >> +			dev_priv->pch_id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
> >
> > So it's imo silly that we're leaking the pch_id to our code when we
> > already have pch_type, but oh well.
> 
> It's for distinguishing between LP and non-LP, which we need in the
> code, and why we have this patch in the first place.
> 
> > Anyway, this is all about the physical pch on the chip, nothing to do with
> > the virtualized one, and we've been hard-coding these forever.
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > Afaiui if this isn't true on a machine, someone used a solder iron to
> > build something that Intel doesn't sell.
> 
> Bspec says there are (at least) non-ULT/ULX Broadwells with LP PCH. We
> do seem to warn about the combo in the bare metal PCH detection, so I
> guess it's safe to assume they are rare. But strictly speaking this
> change just moves problems from one setup to another.
> 
> This has all been covered in the thread that I linked to in my previous
> reply, and all of that discussion has been conveniently overlooked and
> ignored in this patch submission and the commit message.
[Zhang, Xiong Y] Yes, I remembered your concern and have been seeking
the accurate map between IGD and PCH in the past months. Unfortunately
no one could give us such document. 
Then as Jonnas suggested, we try to pass real pch id from host to guest.
But we couldn't find a secure and acceptable register to pass such info.
Finally this issue blocks QA CI system for a long time and QA complained 
this. So we decided to send this again and resolve part of issue.

This is a limitation of IGD passthrough and need a spec which define
how to pass real pch id from host to guest in virtual environment. But this
need the top level design and much more time.
thanks
> 
> I'm not opposed to merging the patch, but this needs to be documented
> for posterity.
> 
> Of course, this gets *much* more complicated from SKL/SPT on, where the
> combos can be mixed even more freely (e.g. SKL+KBP and KBL+SPT).
> 
> 
> BR,
> Jani.
> 
> 
> 
> > -Daniel
> >
> >>  		DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
> >>  	} else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> >>  		ret = PCH_SPT;
> >> --
> >> 2.7.4
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Setting pch_id for HSW/BDW in virtual environment

[Daniel Vetter]

On Thu, Jun 15, 2017 at 11:11:45AM +0800, Xiong Zhang wrote:
> In a IGD passthrough environment, the real ISA bridge may doesn't exist.
> then pch_id couldn't be correctly gotten from ISA bridge, but pch_id is
> used to identify LPT_H and LPT_LP. Currently i915 treat all LPT pch as
> LPT_H,then errors occur when i915 runs on LPT_LP machines with igd
> passthrough.
> 
> This patch set pch_id for HSW/BDW according to IGD type and isn't fully
> correct. But it solves such issue on HSW/BDW ult/ulx machines.
> QA CI system is blocked by this issue for a long time, it's better that
> we could merge it to unblock QA CI system.
> 
> We know the root cause is in device model of virtual passthrough, and
> will resolve it in the future with several parts cooperation in kernel,
> qemu and xen.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99938
> 
> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>

Applied, thanks for the patch.

(aka CI wins).
-Daniel

> ---
>  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 1f802de..2e664c5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -135,6 +135,10 @@ static enum intel_pch intel_virt_detect_pch(struct drm_i915_private *dev_priv)
>  		DRM_DEBUG_KMS("Assuming CouarPoint PCH\n");
>  	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		ret = PCH_LPT;
> +		if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
> +			dev_priv->pch_id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
> +		else
> +			dev_priv->pch_id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
>  		DRM_DEBUG_KMS("Assuming LynxPoint PCH\n");
>  	} else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
>  		ret = PCH_SPT;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx