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