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

[Jani Nikula]

On Thu, 29 Jun 2017, Daniel Vetter <daniel at 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 at 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 at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center