Re: i915 | Bug in virtual PCH detection

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Sun, Sep 01, 2024 at 02:56:07PM +0500, Andrey Toloknev wrote:
> Hello!
> 
> I have 2 machines with Comet Lake CPUs on Tiger Lake PCH (500 series of
> Intel chipsets).
> For that configuration there was a patch for adding support for Tiger Lake
> PCH with CometLake CPU in 2021 -
> https://patchwork.freedesktop.org/patch/412664/
> This patch made possible correct detection of such chipset and cpu
> configuration for i915 kernel module. Without it there was no output to any
> display (HDMI/DP/DVI, even VGA).
> 
> But this patch doesn't touch intel_virt_detect_pch method, when you
> passthrough iGPU to a virtual machine.
> So, virtual PCH incorrectly detects as Cannon Lake and you have no output
> to a physical display with i915 driver:
> 
> [    2.933139] i915 0000:00:02.0: [drm:intel_virt_detect_pch [i915]]
> Assuming PCH ID a300
> [    2.933308] i915 0000:00:02.0: [drm:intel_pch_type [i915]] Found Cannon
> Lake PCH (CNP)
> 
> 
> The bug is on line 173 in drivers/gpu/drm/i915/soc/intel_pch.c in method
> intel_virt_detect_pch:
> 
> else if (IS_TIGERLAKE(dev_priv) || IS_ROCKETLAKE(dev_priv))
> 
> id = INTEL_PCH_TGP_DEVICE_ID_TYPE;
> 
> It must be:
> 
> else if (IS_TIGERLAKE(dev_priv) || IS_ROCKETLAKE(dev_priv) ||
> IS_GEN9_BC(dev_priv))
> 
> id = INTEL_PCH_TGP_DEVICE_ID_TYPE;
> 
> 
> After that small change you get correct detection of PCH and have output to
> a physical display in VM with passthrough iGPU:
> 
> [   16.139809] i915 0000:00:02.0: [drm:intel_virt_detect_pch [i915]]
> Assuming PCH ID a080
> [   16.261151] i915 0000:00:02.0: [drm:intel_pch_type [i915]] Found Tiger
> Lake LP PCH
> 
> 
> All kernel versions in any distro since 2021 are affected by this small bug.
> The patch for i915 module of the actual kernel version is in attachment.

You fix one CPU+PCH combo, but break the other. I don't think there is
any way to handle this mess in intel_virt_detect_pch(). The best thing
would be if the virtual machine would advertise the correct ISA/LPC
bridge, then the heiristic is not even invoked. If that's not possible
for some reason then I suppose we'd need a modparam/etc. so the user
can specify the PCH ID by hand.

-- 
Ville Syrjälä
Intel