[Intel-gfx] [PATCH] drm/i915: Setting pch_id for Gen7.5+ in virtual environment

[Martin Peres]

On 31/03/17 12:37, Martin Peres wrote:
>
>
> On 30/03/17 08:39, Zhang, Xiong Y wrote:
>>> On Wed, Mar 29, 2017 at 05:02:47PM +0800, Xiong Zhang wrote:
>>>> In a virtual passthrough environment, the ISA bridge isn't able to
>>>> be passed through. So pch_id couldn't be gotten from ISA bridge, but
>>>> pch_id is used to identify LPT_H and LPT_LP, this patch set pch_id
>>>> according to IGD type.
>>>>
>>>> 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 | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>>> b/drivers/gpu/drm/i915/i915_drv.c
>>>> index 8b807a9..32a9bff 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.c
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>>> @@ -135,9 +135,17 @@ 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;
>>>> +        if (IS_SKL_ULT(dev_priv) || IS_KBL_ULT(dev_priv))
>>>> +            dev_priv->pch_id = INTEL_PCH_SPT_LP_DEVICE_ID_TYPE;
>>>> +        else
>>>> +            dev_priv->pch_id = INTEL_PCH_SPT_DEVICE_ID_TYPE;
>>>
>>> I'm not 100% sure the ULT/ULX <=> LP thing always holds. I *think* it
>>> should but I've never been able to convince myself totally.
>> [Zhang, Xiong Y] For BDW ULT/ULX, it should be LP. A picture from
>> https://gfxspecs.intel.com/Predator/Home/Index/4216 could confirm this.
>> For HSW ULT/ULX, I couldn't find a material to confirm this.
>> Anyway I copy this condition from the WARN_ON() in intel_detect_pch()
>>
>> For SKL/KBL ULT/ULX, I couldn't find a material to confirm this neither.
>>
>>> As far as KBL goes, maybe we want PCH_KBP for it? Although I don't
>>> actually
>>> know why we even make the distinction between the two since they seem
>>> identical for us, and we don't distinguish LPT vs. WPT either. Rodrigo?
>> [Zhang, Xiong Y] For PCH_KBP, I couldn't find out which KBL type will
>> co-work with it.
>> As the real ISA Bridge doesn't exist in a emulated guest machine, we
>> couldn't get the real and accurate pch_id in guest. In such
>> passthrough virtual environment, the only real HW is IGD which is
>> passed through to guest, we could only assume the pch_id from this IGD.
>> Fortunately pch_id is only used to identify LPT_LP and LPT_H currently.
>> Without this patch, pch_id is totally wrong. And on LPT_LP platform
>> without this patch, the local HDMI/DP display couldn't lightup when
>> guest boot up. But this patch could only remedy part of pch_id.
>> Currently I have two plans to improve this patch:
>> 1) only remain pch_id assuming for HSW and BDW, as this is really we
>> need to fix the bug.
>>   Delete pch_id assuming for SKL and KBL, as currently i915 driver
>> don't use it.
>> 2) Claim this patch's limitation in commit message.
>> Any suggestion ?
>
> Guys, any updates on this?
>
> This blocks the testing of the following tests on fi-bdw-gvtdvm:
> igt at drv_module_reload@basic-reload
> igt at drv_module_reload@basic-reload-final
> igt at drv_module_reload@basic-reload-inject
> igt at gem_exec_suspend@basic-s3
> igt at gem_exec_suspend@basic-s4-devices
> igt at kms_pipe_crc_basic@suspend-read-crc-pipe-a
> igt at kms_pipe_crc_basic@suspend-read-crc-pipe-b
> igt at kms_pipe_crc_basic@suspend-read-crc-pipe-c

Ping, testing in virtual environment is seriously hindered by this bug. 
Please take this seriously.