<div dir="ltr"><div dir="ltr">Hello!<div><br></div><div>Thanks for your reply, Ville.</div><div><br></div><div>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.</div><div><br></div><div>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.<br></div><div>And the virtual ISA/LPC Bridge PCI in libvirt is hardcoded to address 00:01.0 - it's always first.</div><div>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.</div><div><br></div><div>Of course, It would be nice if we can have a universal modparam to specify PCH id by hand in future. </div><div>But as a fast fix of that small bug I think one more bool modparam may be enough.<br></div><div>I wrote the second version of patch which adds that bool modparam - force_tgp_vpch. It's false by default.</div><div>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(). </div><div><br></div><div>The second version of patch is in attachment.</div><div> <br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">вт, 3 сент. 2024 г. в 18:38, Ville Syrjälä <<a href="mailto:ville.syrjala@linux.intel.com" target="_blank">ville.syrjala@linux.intel.com</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sun, Sep 01, 2024 at 02:56:07PM +0500, Andrey Toloknev wrote:<br>
> Hello!<br>
> <br>
> I have 2 machines with Comet Lake CPUs on Tiger Lake PCH (500 series of<br>
> Intel chipsets).<br>
> For that configuration there was a patch for adding support for Tiger Lake<br>
> PCH with CometLake CPU in 2021 -<br>
> <a href="https://patchwork.freedesktop.org/patch/412664/" rel="noreferrer" target="_blank">https://patchwork.freedesktop.org/patch/412664/</a><br>
> This patch made possible correct detection of such chipset and cpu<br>
> configuration for i915 kernel module. Without it there was no output to any<br>
> display (HDMI/DP/DVI, even VGA).<br>
> <br>
> But this patch doesn't touch intel_virt_detect_pch method, when you<br>
> passthrough iGPU to a virtual machine.<br>
> So, virtual PCH incorrectly detects as Cannon Lake and you have no output<br>
> to a physical display with i915 driver:<br>
> <br>
> [    2.933139] i915 0000:00:02.0: [drm:intel_virt_detect_pch [i915]]<br>
> Assuming PCH ID a300<br>
> [    2.933308] i915 0000:00:02.0: [drm:intel_pch_type [i915]] Found Cannon<br>
> Lake PCH (CNP)<br>
> <br>
> <br>
> The bug is on line 173 in drivers/gpu/drm/i915/soc/intel_pch.c in method<br>
> intel_virt_detect_pch:<br>
> <br>
> else if (IS_TIGERLAKE(dev_priv) || IS_ROCKETLAKE(dev_priv))<br>
> <br>
> id = INTEL_PCH_TGP_DEVICE_ID_TYPE;<br>
> <br>
> It must be:<br>
> <br>
> else if (IS_TIGERLAKE(dev_priv) || IS_ROCKETLAKE(dev_priv) ||<br>
> IS_GEN9_BC(dev_priv))<br>
> <br>
> id = INTEL_PCH_TGP_DEVICE_ID_TYPE;<br>
> <br>
> <br>
> After that small change you get correct detection of PCH and have output to<br>
> a physical display in VM with passthrough iGPU:<br>
> <br>
> [   16.139809] i915 0000:00:02.0: [drm:intel_virt_detect_pch [i915]]<br>
> Assuming PCH ID a080<br>
> [   16.261151] i915 0000:00:02.0: [drm:intel_pch_type [i915]] Found Tiger<br>
> Lake LP PCH<br>
> <br>
> <br>
> All kernel versions in any distro since 2021 are affected by this small bug.<br>
> The patch for i915 module of the actual kernel version is in attachment.<br>
<br>
You fix one CPU+PCH combo, but break the other. I don't think there is<br>
any way to handle this mess in intel_virt_detect_pch(). The best thing<br>
would be if the virtual machine would advertise the correct ISA/LPC<br>
bridge, then the heiristic is not even invoked. If that's not possible<br>
for some reason then I suppose we'd need a modparam/etc. so the user<br>
can specify the PCH ID by hand.<br>
<br>
-- <br>
Ville Syrjälä<br>
Intel<br>
</blockquote></div></div><br clear="all"><div><br></div><span class="gmail_signature_prefix">-- </span><br><div dir="ltr" class="gmail_signature">Best regards, Andrey Toloknev</div>
</div>