Sorry for replying twice. I thought about looking for the real PCH bridge, but I'm sure it will be a real headache in some situations, configurations and virtualizations. So, in my opinion, a better solution, as you noted in your first reply, is modparam.
ср, 4 сент. 2024 г. в 18:00, Andrey Toloknev <[email protected]>: > Hmm. I wonder how many systems we'd break if we make it look >> through all the bridges for a real match first, and only fall >> back to intel_virt_detect_pch() if nothing was found... > > > Yes, I definitely understand this, that's why I didn't touch this code at > all in the second patch. > I just add bool modparam force_tgp_vpch in i915_params and a bit modify > method intel_virt_detect_pch() in intel_pch.c > > > > ср, 4 сент. 2024 г. в 17:52, Ville Syrjälä <[email protected] > >: > >> On Wed, Sep 04, 2024 at 05:25:06PM +0500, Andrey Toloknev wrote: >> > Hello! >> > >> > Thanks for your reply, Ville. >> > >> > I looked at the code again and understood you are definitely right about >> > breaking other combinations of CPU+PCH with using IS_GEN9_BC in my >> patch. >> > >> > Using libvirt (kvm) I can passthrough ISA/LPC bridge to VM, but the >> problem >> > is connected with method intel_detect_pch(). It searches only for the >> first >> > ISA Bridge. >> >> Hmm. I wonder how many systems we'd break if we make it look >> through all the bridges for a real match first, and only fall >> back to intel_virt_detect_pch() if nothing was found... >> >> > And the virtual ISA/LPC Bridge PCI in libvirt is hardcoded to address >> > 00:01.0 - it's always first. >> > So, method intel_detect_pch() correctly detects that the first bridge is >> > virtual and then calls method intel_virt_detect_pch(), which just checks >> > the iGPU platform and doesn't take into account the possible >> combination of >> > Comet Lake CPU and Tiger Lake PCH. >> > >> > Of course, It would be nice if we can have a universal modparam to >> specify >> > PCH id by hand in future. >> > But as a fast fix of that small bug I think one more bool modparam may >> be >> > enough. >> > I wrote the second version of patch which adds that bool modparam - >> > force_tgp_vpch. It's false by default. >> > When this param is true, we also check that the CPU is Comet Lake and >> then >> > set PCH type as Tiger Lake in the method intel_virt_detect_pch(). >> > >> > The second version of patch is in attachment. >> > >> > >> > вт, 3 сент. 2024 г. в 18:38, Ville Syrjälä < >> [email protected]>: >> > >> > > 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 >> > > >> > >> > >> > -- >> > Best regards, Andrey Toloknev >> >> >> >> -- >> Ville Syrjälä >> Intel >> > > > -- > Best regards, Andrey Toloknev > -- Best regards, Andrey Toloknev
