Applied to drm-misc-next. On Wed, Jun 26, 2024 at 2:49 PM Dragan Simic <dsimic@xxxxxxxxxxx> wrote: > > Hello Qiang, > > On 2024-06-26 03:11, Qiang Yu wrote: > > On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@xxxxxxxxxxx> > > wrote: > >> > >> Hello everyone, > >> > >> Just checking, any further thoughts about this patch? > >> > > I'm OK with this as a temp workaround because it's simple and do no > > harm > > even it's not perfect. If no other better suggestion for short term, > > I'll submit > > this at weekend. > > Thanks. Just as you described it, it's far from perfect, but it's still > fine until there's a better solution, such as harddeps. I'll continue > my > research about the possibility for adding harddeps, which would > hopefully > replace quite a few instances of the softdep (ab)use. > > >> On 2024-06-18 21:22, Dragan Simic wrote: > >> > On 2024-06-18 12:33, Dragan Simic wrote: > >> >> On 2024-06-18 10:13, Maxime Ripard wrote: > >> >>> On Tue, Jun 18, 2024 at 04:01:26PM GMT, Qiang Yu wrote: > >> >>>> On Tue, Jun 18, 2024 at 12:33 PM Qiang Yu <yuq825@xxxxxxxxx> wrote: > >> >>>> > > >> >>>> > I see the problem that initramfs need to build a module dependency chain, > >> >>>> > but lima does not call any symbol from simpleondemand governor module. > >> >>>> > softdep module seems to be optional while our dependency is hard one, > >> >>>> > can we just add MODULE_INFO(depends, _depends), or create a new > >> >>>> > macro called MODULE_DEPENDS()? > >> >> > >> >> I had the same thoughts, because softdeps are for optional module > >> >> dependencies, while in this case it's a hard dependency. Though, > >> >> I went with adding a softdep, simply because I saw no better option > >> >> available. > >> >> > >> >>>> This doesn't work on my side because depmod generates modules.dep > >> >>>> by symbol lookup instead of modinfo section. So softdep may be our > >> >>>> only > >> >>>> choice to add module dependency manually. I can accept the softdep > >> >>>> first, then make PM optional later. > >> >> > >> >> I also thought about making devfreq optional in the Lima driver, > >> >> which would make this additional softdep much more appropriate. > >> >> Though, I'm not really sure that's a good approach, because not > >> >> having working devfreq for Lima might actually cause issues on > >> >> some devices, such as increased power consumption. > >> >> > >> >> In other words, it might be better to have Lima probing fail if > >> >> devfreq can't be initialized, rather than having probing succeed > >> >> with no working devfreq. Basically, failed probing is obvious, > >> >> while a warning in the kernel log about no devfreq might easily > >> >> be overlooked, causing regressions on some devices. > >> >> > >> >>> It's still super fragile, and depends on the user not changing the > >> >>> policy. It should be solved in some other, more robust way. > >> >> > >> >> I see, but I'm not really sure how to make it more robust? In > >> >> the end, some user can blacklist the simple_ondemand governor > >> >> module, and we can't do much about it. > >> >> > >> >> Introducing harddeps alongside softdeps would make sense from > >> >> the design standpoint, but the amount of required changes wouldn't > >> >> be trivial at all, on various levels. > >> > > >> > After further investigation, it seems that the softdeps have > >> > already seen a fair amount of abuse for what they actually aren't > >> > intended, i.e. resolving hard dependencies. For example, have > >> > a look at the commit d5178578bcd4 (btrfs: directly call into > >> > crypto framework for checksumming) [1] and the lines containing > >> > MODULE_SOFTDEP() at the very end of fs/btrfs/super.c. [2] > >> > > >> > If a filesystem driver can rely on the abuse of softdeps, which > >> > admittedly are a bit fragile, I think we can follow the same > >> > approach, at least for now. > >> > > >> > With all that in mind, I think that accepting this patch, as well > >> > as the related Panfrost patch, [3] should be warranted. I'd keep > >> > investigating the possibility of introducing harddeps in form > >> > of MODULE_HARDDEP() and the related support in kmod project, > >> > similar to the already existing softdep support, [4] but that > >> > will inevitably take a lot of time, both for implementing it > >> > and for reaching various Linux distributions, which is another > >> > reason why accepting these patches seems reasonable. > >> > > >> > [1] > >> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d5178578bcd4 > >> > [2] > >> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/super.c#n2593 > >> > [3] > >> > https://lore.kernel.org/dri-devel/4e1e00422a14db4e2a80870afb704405da16fd1b.1718655077.git.dsimic@xxxxxxxxxxx/ > >> > [4] > >> > https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/commit/?id=49d8e0b59052999de577ab732b719cfbeb89504d