Re: [PATCH] drm/Makefile: Move tiny drivers before native drivers

[Huacai Chen <chenhuacai@kernel.org>]

Hi, Thomas,

On Wed, Jan 24, 2024 at 5:44 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 24.01.24 um 10:24 schrieb Huacai Chen:
> > Hi, Thomas,
> >
> > On Wed, Jan 24, 2024 at 4:16 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> Hi
> >>
> >> Am 24.01.24 um 04:00 schrieb Huacai Chen:
> >>> Hi, Javier and Thomas,
> >>>
> >>> On Wed, Jan 24, 2024 at 5:21 AM Jaak Ristioja <jaak@ristioja.ee> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> I apologize for not finding the time to test this earlier.
> >>>>
> >>>> On 11.12.23 05:08, Huacai Chen wrote:
> >>>>> And Jaak, could you please test with the below patch (but keep the
> >>>>> original order in Makefile) and then give me the dmesg output?
> >>>>>
> >>>>> diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
> >>>>> index 561be8feca96..cc2e39fb98f5 100644
> >>>>> --- a/drivers/video/aperture.c
> >>>>> +++ b/drivers/video/aperture.c
> >>>>> @@ -350,21 +350,29 @@ int
> >>>>> aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const
> >>>>> char *na
> >>>>>            resource_size_t base, size;
> >>>>>            int bar, ret = 0;
> >>>>>
> >>>>> -       if (pdev == vga_default_device())
> >>>>> +       printk("DEBUG: remove 1\n");
> >>>>> +
> >>>>> +       if (pdev == vga_default_device()) {
> >>>>> +               printk("DEBUG: primary = true\n");
> >>>>>                    primary = true;
> >>>>> +       }
> >>>>>
> >>>>> -       if (primary)
> >>>>> +       if (primary) {
> >>>>> +               printk("DEBUG: disable sysfb\n");
> >>>>>                    sysfb_disable();
> >>>>> +       }
> >>>>>
> >>>>>            for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
> >>>>>                    if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
> >>>>>                            continue;
> >>>>>
> >>>>> +               printk("DEBUG: remove 2\n");
> >>>>>                    base = pci_resource_start(pdev, bar);
> >>>>>                    size = pci_resource_len(pdev, bar);
> >>>>>                    aperture_detach_devices(base, size);
> >>>>>            }
> >>>>>
> >>>>> +       printk("DEBUG: remove 3\n");
> >>>>>            /*
> >>>>>             * If this is the primary adapter, there could be a VGA device
> >>>>>             * that consumes the VGA framebuffer I/O range. Remove this
> >>>>>
> >>>>> [1]  https://lore.kernel.org/lkml/170222766284.86103.11020060769330721008@leemhuis.info/T/#u
> >>>>
> >>>> Copy-pasting this from the e-mail body didn't work well, but I applied
> >>>> the changes manually to a 6.5.9 kernel without any of the other patches.
> >>>> Here's the relevant dmesg output on the Lenovo L570:
> >>>>
> >>>> ...
> >>>> [    2.953405] ACPI: bus type drm_connector registered
> >>>> [    2.954014] i915 0000:00:02.0: [drm] VT-d active for gfx access
> >>>> [    2.954018] DEBUG: remove 1
> >>>> [    2.954020] DEBUG: remove 2
> >>>> [    2.954021] DEBUG: remove 2
> >>>> [    2.954023] DEBUG: remove 3
> >>>
> >>> My tmp patch is as follows:
> >>>
> >>> diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
> >>> index 561be8feca96..cc2e39fb98f5 100644
> >>> --- a/drivers/video/aperture.c
> >>> +++ b/drivers/video/aperture.c
> >>> @@ -350,21 +350,29 @@ int
> >>> aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const
> >>> char *na
> >>>           resource_size_t base, size;
> >>>           int bar, ret = 0;
> >>>
> >>> -       if (pdev == vga_default_device())
> >>> +       printk("DEBUG: remove 1\n");
> >>> +
> >>> +       if (pdev == vga_default_device()) {
> >>> +               printk("DEBUG: primary = true\n");
> >>>                   primary = true;
> >>> +       }
> >>>
> >>> -       if (primary)
> >>> +       if (primary) {
> >>> +               printk("DEBUG: disable sysfb\n");
> >>>                   sysfb_disable();
> >>> +       }
> >>>
> >>>           for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
> >>>                   if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
> >>>                           continue;
> >>>
> >>> +               printk("DEBUG: remove 2\n");
> >>>                   base = pci_resource_start(pdev, bar);
> >>>                   size = pci_resource_len(pdev, bar);
> >>>                   aperture_detach_devices(base, size);
> >>>           }
> >>>
> >>> +       printk("DEBUG: remove 3\n");
> >>>           /*
> >>>            * If this is the primary adapter, there could be a VGA device
> >>>            * that consumes the VGA framebuffer I/O range. Remove this
> >>>
> >>>   From the Jaak's dmesg, it is obvious that "pdev ==
> >>> vga_default_device()" is false, which causes sysfb_disable() to be not
> >>> called. And I think the simple-drm device is not provided by the i915
> >>> device in this case. So, can we unconditionally call sysfb_disable()
> >>> here, which is the same as aperture_remove_conflicting_devices()?
> >>
> >> Unfortunately, we cannot call sysfb_disable() unconditionally.
> >> Otherwise, we'd possibly remove simpledrm et al on the primary driver
> >> even pdev is not the primary device.
> >>
> >> Both, sysfb and vgaarb, are initialized with subsys_initcall_sync() and
> >> the order of initialization is most likely wrong in the broken builds.
> >> Hence, reordering the linking mitigates the problem.
> > OK, sysfb_init() should be after vgaarb, so I think we  can move
> > sysfb_init to be fs_initcall(). Though sysfb has nothing to do with
> > "file system", I found that there are already a lot of init functions
> > using fs_initcall().
> >
> > Hi, Jaak, could you please make sysfb_initcall from
> > subsys_initcall_sync to be fs_initcall and see if your problem can be
> > fixed? Thank you very much.
>
> Thanks for helping. I already supplied a patch to fix sysfb. No further
> action is required.
Do you mean reverting "drivers/firmware: Move sysfb_init() from
device_initcall to subsys_initcall_sync"? I still want to keep that,
since we can probably solve both the original problem and Jaak's
problem now.

And if you mean others, please ignore what I said. :)

Huacai

>
> Best regards
> Thomas
>
> >
> > Huacai
> >
> >>
> >> I've long been thinking about how the graphics init code could be
> >> streamlined into something more manageable. But it's a larger effort.
> >>
> >> Best regards
> >> Thomas
> >>
> >>>
> >>> Huacai
> >>>
> >>>> [    2.954029] resource: resource sanity check: requesting [mem
> >>>> 0x00000000e0000000-0x00000000efffffff], which spans more than BOOTFB
> >>>> [mem 0xe0000000-0xe012bfff]
> >>>> [    2.954035] caller i915_ggtt_init_hw+0x88/0x120 mapping multiple BARs
> >>>> [    2.954061] i915 0000:00:02.0: [drm] Using Transparent Hugepages
> >>>> [    2.955103] Loading firmware: i915/kbl_dmc_ver1_04.bin
> >>>> [    2.955384] i915 0000:00:02.0: [drm] Finished loading DMC firmware
> >>>> i915/kbl_dmc_ver1_04.bin (v1.4)
> >>>> ...
> >>>> [    4.145013] [drm] Initialized i915 1.6.0 20201103 for 0000:00:02.0 on
> >>>> minor 0
> >>>> [    4.147101] ACPI: video: Video Device [GFX0] (multi-head: yes  rom:
> >>>> no  post: no)
> >>>> [    4.147244] input: Video Bus as
> >>>> /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:00/input/input4
> >>>> [    4.147410] [drm] Initialized vgem 1.0.0 20120112 for vgem on minor 1
> >>>> [    4.147420] usbcore: registered new interface driver udl
> >>>> [    4.147500] [drm] Initialized simpledrm 1.0.0 20200625 for
> >>>> simple-framebuffer.0 on minor 2
> >>>> [    4.148643] Console: switching to colour frame buffer device 80x30
> >>>> [    4.153216] simple-framebuffer simple-framebuffer.0: [drm] fb0:
> >>>> simpledrmdrmfb frame buffer device
> >>>> [    4.154043] loop: module loaded
> >>>> [    4.156017] ahci 0000:00:17.0: version 3.0
> >>>> [    4.157373] i915 0000:00:02.0: [drm] fb1: i915drmfb frame buffer device
> >>>> ...
> >>>>
> >>>> J
> >>>>
> >>
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Frankenstrasse 146, 90461 Nuernberg, Germany
> >> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> >> HRB 36809 (AG Nuernberg)
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)