i915 | Bug in virtual PCH detection

Wed Sep 4 13:37:06 UTC 2024

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 <andreyhack at gmail.com>:

> 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ä <ville.syrjala at linux.intel.com
> >:
>
>> 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ä <
>> ville.syrjala at 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
>> > >
>> >
>> >
>> > --
>> > Best regards, Andrey Toloknev
>>
>>
>>
>> --
>> Ville Syrjälä
>> Intel
>>
>
>
> --
> Best regards, Andrey Toloknev
>

-- 
Best regards, Andrey Toloknev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20240904/964107ef/attachment-0001.htm>