<div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">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...</blockquote><div> </div><div>Yes, I definitely understand this, that's why I didn't touch this code at all in the second patch. </div><div>I just add bool modparam force_tgp_vpch in i915_params and a bit modify method intel_virt_detect_pch() in intel_pch.c</div><div> </div><div> </div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">ср, 4 сент. 2024 г. в 17:52, Ville Syrjälä <<a href="mailto:ville.syrjala@linux.intel.com">ville.syrjala@linux.intel.com</a>>: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">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ä <<a href="mailto:ville.syrjala@linux.intel.com" target="_blank">ville.syrjala@linux.intel.com</a>>: > > > 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 - > > > <a href="https://patchwork.freedesktop.org/patch/412664/" rel="noreferrer" target="_blank">https://patchwork.freedesktop.org/patch/412664/</a> > > > 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 </blockquote></div> <div> </div>-- <div dir="ltr" class="gmail_signature">С Уважением, Толокнев Андрей</div>