On 2024-07-03 15:20, Steven Price wrote:
On 03/07/2024 13:42, Dragan Simic wrote:
On 2024-06-17 22:17, Dragan Simic wrote:
Panfrost DRM driver uses devfreq to perform DVFS, while using
simple_ondemand
devfreq governor by default. This causes driver initialization to
fail on
boot when simple_ondemand governor isn't built into the kernel
statically,
as a result of the missing module dependency and, consequently, the
required
governor module not being included in the initial ramdisk. Thus,
let's mark
simple_ondemand governor as a softdep for Panfrost, to have its
kernel
module
included in the initial ramdisk.
This is a rather longstanding issue that has forced distributions to
build
devfreq governors statically into their kernels, [1][2] or has forced
users
to introduce some unnecessary workarounds. [3]
For future reference, not having support for the simple_ondemand
governor in
the initial ramdisk produces errors in the kernel log similar to
these
below,
which were taken from a Pine64 RockPro64:
panfrost ff9a0000.gpu: [drm:panfrost_devfreq_init [panfrost]]
*ERROR* Couldn't initialize GPU devfreq
panfrost ff9a0000.gpu: Fatal error during GPU init
panfrost: probe of ff9a0000.gpu failed with error -22
Having simple_ondemand marked as a softdep for Panfrost may not
resolve this
issue for all Linux distributions. In particular, it will remain
unresolved
for the distributions whose utilities for the initial ramdisk
generation do
not handle the available softdep information [4] properly yet.
However, some
Linux distributions already handle softdeps properly while generating
their
initial ramdisks, [5] and this is a prerequisite step in the right
direction
for the distributions that don't handle them properly yet.
[1]
https://gitlab.manjaro.org/manjaro-arm/packages/core/linux/-/blob/linux61/config?ref_type=heads#L8180
[2] https://salsa.debian.org/kernel-team/linux/-/merge_requests/1066
[3] https://forum.pine64.org/showthread.php?tid=15458
[4]
https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/commit/?id=49d8e0b59052999de577ab732b719cfbeb89504d
[5]
https://github.com/archlinux/mkinitcpio/commit/97ac4d37aae084a050be512f6d8f4489054668ad
Cc: Diederik de Haas <didi.debian@xxxxxxxxx>
Cc: Furkan Kardame <f.kardame@xxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver")
Signed-off-by: Dragan Simic <dsimic@xxxxxxxxxxx>
Reviewed-by: Steven Price <steven.price@xxxxxxx>
Thanks!
Just checking, could this patch be accepted, please? The Lima
counterpart
has already been accepted. [6]
Thanks for the prod - I have to admit I saw there was discussion about
the Lima patch and so just put this on my list to look again later
after
the discussion had reached a conclusion.
The approach in this patch is 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 introducing harddeps, which would hopefully
replace quite a few instances of the softdep (ab)use that already
extend
rather far. For example, have a look at the commit d5178578bcd4
(btrfs:
directly call into crypto framework for checksumming) [7] and the
lines
containing MODULE_SOFTDEP() at the very end of fs/btrfs/super.c. [8]
I agree - it's not perfect, but it's the best we have for now. I hope
sometime we'll have a cleaner solution to express dependencies like
this
(good luck! ;) ).
Thanks. :) Implementing harddeps is _relatively_ straightforward, but
getting full support for harddeps reach various Linux distributions is
going to be an uphill battle without doubt. :)
If a filesystem driver can rely on the (ab)use of softdeps, which may
be
fragile or seen as a bit wrong, I think we can follow the same
approach,
at least until a better solution is available.
[6]
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=0c94f58cef319ad054fd909b3bf4b7d09c03e11c
[7]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d5178578bcd4
[8]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/super.c#n2593
---
drivers/gpu/drm/panfrost/panfrost_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index ef9f6c0716d5..149737d7a07e 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -828,3 +828,4 @@ module_platform_driver(panfrost_driver);
MODULE_AUTHOR("Panfrost Project Developers");
MODULE_DESCRIPTION("Panfrost DRM Driver");
MODULE_LICENSE("GPL v2");
+MODULE_SOFTDEP("pre: governor_simpleondemand");
