On 2024-07-03 15:20, Boris Brezillon wrote:
On Wed, 03 Jul 2024 14:42:37 +0200 Dragan Simic dsimic@manjaro.org 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/co... [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=49d8e0b... [5] https://github.com/archlinux/mkinitcpio/commit/97ac4d37aae084a050be512f6d8f4...
Cc: Diederik de Haas didi.debian@cknow.org Cc: Furkan Kardame f.kardame@manjaro.org Cc: stable@vger.kernel.org Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver") Signed-off-by: Dragan Simic dsimic@manjaro.org
Just checking, could this patch be accepted, please?
Yes, sorry for the delay. Here's my
Reviewed-by: Boris Brezillon boris.brezillon@collabora.com
No worries, and thanks!
Steve, any objection?
The Lima counterpart has already been accepted. [6]
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]
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=0c94f58cef319ad054fd909... [7] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... [8] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/b...
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");