| Age | Commit message (Collapse) | Author |
|---|
|
We are required to call dev_pm_opp_put() from outside of the
opp_table->lock as debugfs removal needs to happen lock-less to avoid
circular dependency issues.
commit cf1fac943c63 ("opp: Reduce the size of critical section in
_opp_kref_release()") tried to fix that introducing a new routine
_opp_get_next() which keeps returning OPPs that can be freed by the
callers and this routine shall be called without holding the
opp_table->lock.
Though the commit overlooked the fact that the OPPs can be referenced by
other users as well and this routine will end up dropping references
which were taken by other users and hence freeing the OPPs prematurely.
In effect, other users of the OPPs will end up having invalid pointers
at hand. We didn't see any crash reports earlier as the exact situation
never happened, though it is certainly possible.
We need a way to mark which OPPs are no longer referenced by the OPP
core, so we don't drop extra references to them accidentally.
This commit adds another OPP flag, "removed", which is used to track
this. And now we should never end up dropping extra references to the
OPPs.
Cc: v5.11+ <stable@vger.kernel.org> # v5.11+
Fixes: cf1fac943c63 ("opp: Reduce the size of critical section in _opp_kref_release()")
Signed-off-by: Beata Michalska <beata.michalska@arm.com>
[ Viresh: Almost rewrote entire patch, added new "removed" field,
rewrote commit log and added the correct Fixes tag. ]
Co-developed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
We skip the OPP update if the current and target OPPs are same. This is
fine for the devices that don't support frequency but may cause issues
for the ones that need to program frequency.
An OPP entry doesn't really signify a single operating frequency but
rather the highest frequency at which the other properties of the OPP
entry apply. And we may reach here with different frequency values,
while all of them would point to the same OPP entry in the OPP table.
We just need to update the clock frequency in that case, though in order
to not add special exit points we reuse the code flow from a normal
path.
While at it, rearrange the conditionals in the 'if' statement to check
'enabled' flag at the end.
Fixes: 81c4d8a3c414 ("opp: Keep track of currently programmed OPP")
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
[ Viresh: Improved commit log and subject, rename current_freq as
current_rate, document it, remove local variable and rearrange
code. ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Add a function that allows looking up required OPPs given a source OPP
table, destination OPP table and the source OPP.
Signed-off-by: Saravana Kannan <saravanak@google.com>
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
[ Viresh: Rearranged code, fixed return errors ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Not all devices that need to use OPP core need to have clocks, a missing
clock is fine in which case -ENOENT shall be returned by clk_get().
Anything else is an error and must be handled properly.
Reported-by: Dmitry Osipenko <digetx@gmail.com>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
The bandwidth must be scaled at a different point in the code flow based
on if we are scaling up or down the frequency, otherwise this may cause
undesired effects as the device will try to use more of the memory
bandwidth which may be shared across several devices. Much like how
regulators and required-opps are programmed.
Reported-by: Dmitry Osipenko <digetx@gmail.com>
Reported-by: Akhil P Oommen <akhilpo@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
The OPP core currently requires the required opp tables to be available
before the dependent OPP table is added, as it needs to create links
from the dependent OPP table to the required ones. This may not be
convenient for all the platforms though, as this requires strict
ordering for probing the drivers.
This patch allows lazy-linking of the required-opps. The OPP tables for
which the required-opp-tables aren't available at the time of their
initialization, are added to a special list of OPP tables:
lazy_opp_tables. Later on, whenever a new OPP table is registered with
the OPP core, we check if it is required by an OPP table in the pending
list; if yes, then we complete the linking then and there.
An OPP table is marked unusable until the time all its required-opp
tables are available. And if lazy-linking fails for an OPP table, the
OPP core disables all of its OPPs to make sure no one can use them.
Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
All the users have migrated to dev_pm_opp_set_opp() now, get rid of the
duplicate API, dev_pm_opp_set_bw(), which only performs a part of the new API.
While at it, remove the unnecessary parameter to _set_opp_bw().
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
The new helper dev_pm_opp_set_opp() can be used for configuring the
devices for a particular OPP and can be used by different type of
devices, even the ones which don't change frequency (like power
domains).
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
Drop the unnecessary parameters and follow the pattern from
_generic_set_opp_regulator().
While at it, also remove the local variable old_freq.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
In order to avoid conditional statements at the caller site, this patch
updates _generic_set_opp_clk_only() to work for devices that don't
change frequency (like power domains, etc.). Return 0 if the clk pointer
passed to this routine is not valid.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
The _generic_set_opp_regulator() helper will be used for devices which
don't change frequency (like power domains, etc.) later on, prepare for
that by not relying on frequency for making decisions here.
While at it, update its parameters to pass only what is necessary.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
The _set_opp() helper will be used for devices which don't change frequency
(like power domains, etc.) later on, prepare for that by not relying on
frequency for making decisions here.
While at it, also update the debug print to contain all relevant
information.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
The _set_opp() helper will be used for devices which don't change their
frequency (like power domains, etc.) later on, prepare for that by
breaking the generic part out of dev_pm_opp_set_rate().
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
The dev_pm_opp_set_rate() helper needs to know the currently programmed
OPP to make few decisions and currently we try to find it on every
invocation of this routine.
Lets start keeping track of the current_opp programmed for the devices
of the opp table, that will be quite useful going forward.
If we fail to find the current OPP, we pick the first one available in
the list, as the list is in ascending order of frequencies, level, or
bandwidth and that's the best guess we can make anyway.
Note that we used to do the frequency comparison a bit early in
dev_pm_opp_set_rate() previously, and now instead we check the target
opp, which shall be more accurate anyway.
We need to make sure that current_opp's memory doesn't get freed while
it is being used and so we keep a reference of it until the time it is
used.
Now that current_opp will always be set, we can drop some unnecessary
checks as well.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
Clock is not optional for users who call into dev_pm_opp_set_rate().
Remove the unnecessary checks.
While at it also drop the local variable for clk and use opp_table->clk
instead.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
This routine has nothing to do with frequency, it just disables all the
resources previously enabled. Rename it to match its purpose.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
Check whether OPP table has regulators in _set_opp_custom() and set up
dev_pm_set_opp_data accordingly. Now _set_opp_custom() works properly,
i.e. it doesn't crash if OPP table doesn't have assigned regulators.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
[ Viresh: Rearrange the routine a bit ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Until now the ->set_opp() helper (i.e. special implementation for
setting the OPPs for platforms) was implemented only to take care of
multiple regulators case, but going forward we would need that for other
use cases as well.
This patch prepares for that by allocating the regulator specific part
from dev_pm_opp_set_regulators() and the opp helper part from
dev_pm_opp_register_set_opp_helper().
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
NVIDIA Tegra SoCs have a power domains topology such that child domains
only clamp a power rail, while parent domain controls shared performance
state of the multiple child domains. In this case child's domain doesn't
need to have OPP table. Hence we want to allow children power domains to
pass performance state to the parent domain if child's domain doesn't have
OPP table.
The dev_pm_opp_xlate_performance_state() gets src_table=NULL if a child
power domain doesn't have OPP table and in this case we should pass the
performance state to the parent domain.
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Matt Merhar <mattmerhar@protonmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Add resource-managed version of dev_pm_opp_attach_genpd().
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
[ Viresh: Manually apply the patch and relocate the routines ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Add resource-managed version of dev_pm_opp_register_set_opp_helper().
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Matt Merhar <mattmerhar@protonmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
[ Viresh: Manually apply the patch and relocate the routines ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
We acquire the clk at the time the OPP table is allocated, though it
works fine, it is not the best place to do so. One of the main reason
being we may need to acquire it again from dev_pm_opp_set_clkname() if
the platform wants another clock to be acquired instead.
There is also requirement from some of the platforms where they do not
want the OPP core to manage the clock at all.
This patch hence defers acquiring the clk until the time we are certain
about which clk we need to acquire and if we really need to acquire one.
With this commit, the clk will get acquired either from
dev_pm_opp_set_clkname() or while we initialize the OPPs within the
table.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
|
|
Extend OPP API with dev_pm_opp_sync_regulators() function, which syncs
voltage state of regulators.
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Matt Merhar <mattmerhar@protonmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
[ Viresh: Added unlikely() ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Add dev_pm_opp_get_required_pstate() which allows OPP users to retrieve
required performance state of a given OPP.
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Matt Merhar <mattmerhar@protonmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Add a ceil version of the dev_pm_opp_find_level(). It's handy to have if
levels don't start from 0 in OPP table and zero usually means a minimal
level.
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Matt Merhar <mattmerhar@protonmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
_add_opp_table() isn't used outside of core.c, mark it static.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
A required OPP may not be available, and thus, all OPPs which are using
this required OPP should be unavailable too.
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Matt Merhar <mattmerhar@protonmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Fix adding OPP entries in a wrong (opposite) order if OPP rate is
unavailable. The OPP comparison was erroneously skipped, thus OPPs
were left unsorted.
Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Tested-by: Matt Merhar <mattmerhar@protonmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Fix the clock reference counting by calling the missing clk_put() in the
error path.
Cc: v5.10 <stable@vger.kernel.org> # v5.10
Fixes: dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
In function _allocate_opp_table, opp_dev is allocated and referenced
by opp_table via _add_opp_dev. But in the case that the subsequent calls
return -EPROBE_DEFER, it will jump to err label and opp_table will be
freed. Then opp_dev becomes an unreferenced object to cause memory leak.
So let's call _remove_opp_dev to do the cleanup.
This fixes the following kmemleak report:
unreferenced object 0xffff000801524a00 (size 128):
comm "swapper/0", pid 1, jiffies 4294892465 (age 84.616s)
hex dump (first 32 bytes):
40 00 56 01 08 00 ff ff 40 00 56 01 08 00 ff ff @.V.....@.V.....
b8 52 77 7f 08 00 ff ff 00 3c 4c 00 08 00 ff ff .Rw......<L.....
backtrace:
[<00000000b1289fb1>] kmemleak_alloc+0x30/0x40
[<0000000056da48f0>] kmem_cache_alloc+0x3d4/0x588
[<00000000a84b3b0e>] _add_opp_dev+0x2c/0x88
[<0000000062a380cd>] _add_opp_table_indexed+0x124/0x268
[<000000008b4c8f1f>] dev_pm_opp_of_add_table+0x20/0x1d8
[<00000000e5316798>] dev_pm_opp_of_cpumask_add_table+0x48/0xf0
[<00000000db0a8ec2>] dt_cpufreq_probe+0x20c/0x448
[<0000000030a3a26c>] platform_probe+0x68/0xd8
[<00000000c618e78d>] really_probe+0xd0/0x3a0
[<00000000642e856f>] driver_probe_device+0x58/0xb8
[<00000000f10f5307>] device_driver_attach+0x74/0x80
[<0000000004f254b8>] __driver_attach+0x58/0xe0
[<0000000009d5d19e>] bus_for_each_dev+0x70/0xc8
[<0000000000d22e1c>] driver_attach+0x24/0x30
[<0000000001d4e952>] bus_add_driver+0x14c/0x1f0
[<0000000089928aaa>] driver_register+0x64/0x120
Cc: v5.10 <stable@vger.kernel.org> # v5.10
Fixes: dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER")
Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
[ Viresh: Added the stable tag ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
This allows the callers to drop the unnecessary checks.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
It has been found that some users (like cpufreq-dt and others on LKML)
have abused the helper dev_pm_opp_get_opp_table() to create the OPP
table instead of just finding it, which is the wrong thing to do. This
routine was meant for OPP core's internal working and exposed the whole
functionality by mistake.
Change the scope of dev_pm_opp_get_opp_table() to only finding the
table. The internal helpers _opp_get_opp_table*() are thus renamed to
_add_opp_table*(), dev_pm_opp_get_opp_table_indexed() is removed (as we
don't need the index field for finding the OPP table) and so the only
user, genpd, is updated.
Note that the prototype of _add_opp_table() was already left in opp.h by
mistake when it was removed earlier and so we weren't required to add it
now.
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
There is a lot of stuff here which can be done outside of the
opp_table->lock, do that. This helps avoiding a circular dependency
lockdeps around debugfs.
Reported-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
The caller of _find_opp_dev() only needs to know if the opp_dev is there
in the list or not.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
There is no critical section which needs protection with locks while
allocating an OPP table, except while adding it to the opp_tables list.
And taking the opp_table_lock for the entire duration causes circular
dependency issues like the one mentioned below. This patch takes
another approach to reduce the size of the critical section to avoid
such issues, the details of that are present within the patch.
======================================================
WARNING: possible circular locking dependency detected
5.4.72 #14 Not tainted
------------------------------------------------------
chrome/1865 is trying to acquire lock:
ffffffdd34921750 (opp_table_lock){+.+.}, at: _find_opp_table+0x34/0x74
but task is already holding lock:
ffffff81f0fc71a8 (reservation_ww_class_mutex){+.+.}, at: submit_lock_objects+0x70/0x1ec
fscrypt: AES-256-CTS-CBC using implementation "cts-cbc-aes-ce"
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #4 (reservation_ww_class_mutex){+.+.}:
__mutex_lock_common+0xec/0xc0c
ww_mutex_lock_interruptible+0x5c/0xc4
msm_gem_fault+0x2c/0x124
__do_fault+0x40/0x16c
handle_mm_fault+0x7cc/0xd98
do_page_fault+0x230/0x3b4
do_translation_fault+0x5c/0x78
do_mem_abort+0x4c/0xb4
el0_da+0x1c/0x20
-> #3 (&mm->mmap_sem){++++}:
__might_fault+0x70/0x98
compat_filldir+0xf8/0x48c
dcache_readdir+0x70/0x1dc
iterate_dir+0xd4/0x180
__arm64_compat_sys_getdents+0xa0/0x19c
el0_svc_common+0xa8/0x178
el0_svc_compat_handler+0x2c/0x40
el0_svc_compat+0x8/0x10
-> #2 (&sb->s_type->i_mutex_key#3){++++}:
down_write+0x54/0x16c
start_creating+0x68/0x128
debugfs_create_dir+0x28/0x114
opp_debug_register+0x8c/0xc0
_add_opp_dev_unlocked+0x5c/0x70
_add_opp_dev+0x38/0x58
_opp_get_opp_table+0xdc/0x1ac
dev_pm_opp_get_opp_table_indexed+0x24/0x30
dev_pm_opp_of_add_table_indexed+0x48/0x84
of_genpd_add_provider_onecell+0xc0/0x1b8
rpmhpd_probe+0x240/0x268
platform_drv_probe+0x90/0xb0
really_probe+0x134/0x2ec
driver_probe_device+0x64/0xfc
__device_attach_driver+0x8c/0xa4
bus_for_each_drv+0x90/0xd8
__device_attach+0xc0/0x148
device_initial_probe+0x20/0x2c
bus_probe_device+0x34/0x94
device_add+0x1fc/0x3b0
of_device_add+0x3c/0x4c
of_platform_device_create_pdata+0xb8/0xfc
of_platform_bus_create+0x1e4/0x368
of_platform_populate+0x70/0xbc
devm_of_platform_populate+0x58/0xa0
rpmh_rsc_probe+0x36c/0x3cc
platform_drv_probe+0x90/0xb0
really_probe+0x134/0x2ec
driver_probe_device+0x64/0xfc
__device_attach_driver+0x8c/0xa4
bus_for_each_drv+0x90/0xd8
__device_attach+0xc0/0x148
device_initial_probe+0x20/0x2c
bus_probe_device+0x34/0x94
device_add+0x1fc/0x3b0
of_device_add+0x3c/0x4c
of_platform_device_create_pdata+0xb8/0xfc
of_platform_bus_create+0x1e4/0x368
of_platform_bus_create+0x230/0x368
of_platform_populate+0x70/0xbc
of_platform_default_populate_init+0xa8/0xc0
do_one_initcall+0x1c8/0x3fc
do_initcall_level+0xb4/0x10c
do_basic_setup+0x30/0x48
kernel_init_freeable+0x124/0x1a4
kernel_init+0x14/0x104
ret_from_fork+0x10/0x18
-> #1 (&opp_table->lock){+.+.}:
__mutex_lock_common+0xec/0xc0c
mutex_lock_nested+0x40/0x50
_add_opp_dev+0x2c/0x58
_opp_get_opp_table+0xdc/0x1ac
dev_pm_opp_get_opp_table_indexed+0x24/0x30
dev_pm_opp_of_add_table_indexed+0x48/0x84
of_genpd_add_provider_onecell+0xc0/0x1b8
rpmhpd_probe+0x240/0x268
platform_drv_probe+0x90/0xb0
really_probe+0x134/0x2ec
driver_probe_device+0x64/0xfc
__device_attach_driver+0x8c/0xa4
bus_for_each_drv+0x90/0xd8
__device_attach+0xc0/0x148
device_initial_probe+0x20/0x2c
bus_probe_device+0x34/0x94
device_add+0x1fc/0x3b0
of_device_add+0x3c/0x4c
of_platform_device_create_pdata+0xb8/0xfc
of_platform_bus_create+0x1e4/0x368
of_platform_populate+0x70/0xbc
devm_of_platform_populate+0x58/0xa0
rpmh_rsc_probe+0x36c/0x3cc
platform_drv_probe+0x90/0xb0
really_probe+0x134/0x2ec
driver_probe_device+0x64/0xfc
__device_attach_driver+0x8c/0xa4
bus_for_each_drv+0x90/0xd8
__device_attach+0xc0/0x148
device_initial_probe+0x20/0x2c
bus_probe_device+0x34/0x94
device_add+0x1fc/0x3b0
of_device_add+0x3c/0x4c
of_platform_device_create_pdata+0xb8/0xfc
of_platform_bus_create+0x1e4/0x368
of_platform_populate+0x70/0xbc
devm_of_platform_populate+0x58/0xa0
rpmh_rsc_probe+0x36c/0x3cc
platform_drv_probe+0x90/0xb0
really_probe+0x134/0x2ec
driver_probe_device+0x64/0xfc
__device_attach_driver+0x8c/0xa4
bus_for_each_drv+0x90/0xd8
__device_attach+0xc0/0x148
device_initial_probe+0x20/0x2c
bus_probe_device+0x34/0x94
device_add+0x1fc/0x3b0
of_device_add+0x3c/0x4c
of_platform_device_create_pdata+0xb8/0xfc
of_platform_bus_create+0x1e4/0x368
of_platform_bus_create+0x230/0x368
of_platform_populate+0x70/0xbc
of_platform_default_populate_init+0xa8/0xc0
do_one_initcall+0x1c8/0x3fc
do_initcall_level+0xb4/0x10c
do_basic_setup+0x30/0x48
kernel_init_freeable+0x124/0x1a4
kernel_init+0x14/0x104
ret_from_fork+0x10/0x18
-> #0 (opp_table_lock){+.+.}:
__lock_acquire+0xee4/0x2450
lock_acquire+0x1cc/0x210
__mutex_lock_common+0xec/0xc0c
mutex_lock_nested+0x40/0x50
_find_opp_table+0x34/0x74
dev_pm_opp_find_freq_exact+0x2c/0xdc
a6xx_gmu_resume+0xc8/0xecc
a6xx_pm_resume+0x148/0x200
adreno_resume+0x28/0x34
pm_generic_runtime_resume+0x34/0x48
__rpm_callback+0x70/0x10c
rpm_callback+0x34/0x8c
rpm_resume+0x414/0x550
__pm_runtime_resume+0x7c/0xa0
msm_gpu_submit+0x60/0x1c0
msm_ioctl_gem_submit+0xadc/0xb60
drm_ioctl_kernel+0x9c/0x118
drm_ioctl+0x27c/0x408
drm_compat_ioctl+0xcc/0xdc
__se_compat_sys_ioctl+0x100/0x206c
__arm64_compat_sys_ioctl+0x20/0x2c
el0_svc_common+0xa8/0x178
el0_svc_compat_handler+0x2c/0x40
el0_svc_compat+0x8/0x10
other info that might help us debug this:
Chain exists of: opp_table_lock --> &mm->mmap_sem --> reservation_ww_class_mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(reservation_ww_class_mutex);
lock(&mm->mmap_sem);
lock(reservation_ww_class_mutex);
lock(opp_table_lock);
*** DEADLOCK ***
3 locks held by chrome/1865:
#0: ffffff81edecc0d8 (&dev->struct_mutex){+.+.}, at: msm_ioctl_gem_submit+0x264/0xb60
#1: ffffff81d0000870 (reservation_ww_class_acquire){+.+.}, at: msm_ioctl_gem_submit+0x8e8/0xb60
#2: ffffff81f0fc71a8 (reservation_ww_class_mutex){+.+.}, at: submit_lock_objects+0x70/0x1ec
stack backtrace:
CPU: 0 PID: 1865 Comm: chrome Not tainted 5.4.72 #14
Hardware name: Google Lazor (rev1+) with LTE (DT)
Call trace:
dump_backtrace+0x0/0x158
show_stack+0x20/0x2c
dump_stack+0xc8/0x160
print_circular_bug+0x2c4/0x2c8
check_noncircular+0x1a8/0x1b0
__lock_acquire+0xee4/0x2450
lock_acquire+0x1cc/0x210
__mutex_lock_common+0xec/0xc0c
mutex_lock_nested+0x40/0x50
_find_opp_table+0x34/0x74
dev_pm_opp_find_freq_exact+0x2c/0xdc
a6xx_gmu_resume+0xc8/0xecc
a6xx_pm_resume+0x148/0x200
adreno_resume+0x28/0x34
pm_generic_runtime_resume+0x34/0x48
__rpm_callback+0x70/0x10c
rpm_callback+0x34/0x8c
rpm_resume+0x414/0x550
__pm_runtime_resume+0x7c/0xa0
msm_gpu_submit+0x60/0x1c0
msm_ioctl_gem_submit+0xadc/0xb60
drm_ioctl_kernel+0x9c/0x118
drm_ioctl+0x27c/0x408
drm_compat_ioctl+0xcc/0xdc
__se_compat_sys_ioctl+0x100/0x206c
__arm64_compat_sys_ioctl+0x20/0x2c
el0_svc_common+0xa8/0x178
el0_svc_compat_handler+0x2c/0x40
el0_svc_compat+0x8/0x10
Reported-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
The readers of dev_list expect the updates to it to take place from
within the opp_table->lock and this is missing in the case where the
dev_list is updated for already managed OPPs.
Fix that by calling _add_opp_dev() from there and remove the now unused
_add_opp_dev_unlocked() callback. While at it, also reduce the length of
the critical section in _add_opp_dev().
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
There is a lot of stuff here which can be done outside of the big
opp_table_lock, do that. This helps avoiding few circular dependency
lockdeps around debugfs and interconnects.
Reported-by: Rob Clark <robdclark@gmail.com>
Reported-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
We returned earlier by mistake even when there were no failures. Fix it.
Fixes: dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER")
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.com>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm
Pull opertaing performance points (OPP) framework fixes for 5.10-rc1
from Viresh Kumar:
"- Return -EPROBE_DEFER properly from dev_pm_opp_get_opp_table()
(Stephan Gerhold).
- Minor cleanups around required-opps (Stephan Gerhold).
- Extends opp-supported-hw property to contain multiple versions
(Viresh Kumar).
- Multiple cleanups around dev_pm_opp_attach_genpd() (Viresh Kumar).
- Multiple fixes, cleanups in the OPP core for overall better design
(Viresh Kumar)."
* 'opp/linux-next' of git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm:
opp: Allow opp-level to be set to 0
opp: Prevent memory leak in dev_pm_opp_attach_genpd()
ARM: tegra: Pass multiple versions in opp-supported-hw property
opp: Allow opp-supported-hw to contain multiple versions
dt-bindings: opp: Allow opp-supported-hw to contain multiple versions
opp: Set required OPPs in reverse order when scaling down
opp: Reduce code duplication in _set_required_opps()
opp: Drop unnecessary check from dev_pm_opp_attach_genpd()
opp: Handle multiple calls for same OPP table in _of_add_opp_table_v1()
opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER
opp: Remove _dev_pm_opp_find_and_remove_table() wrapper
opp: Split out _opp_set_rate_zero()
opp: Reuse the enabled flag in !target_freq path
opp: Rename regulator_enabled and use it as status of all resources
|
|
The DT bindings don't put such a constraint, nor should the kernel. It
is perfectly fine for opp-level to be set to 0, if we need to put the
performance state votes for a domain for a particular OPP.
Reported-by: Stephan Gerhold <stephan@gerhold.net>
Tested-by: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
If dev_pm_opp_attach_genpd() is called multiple times (once for each CPU
sharing the table), then it would result in unwanted behavior like
memory leak, attaching the domain multiple times, etc.
Handle that by checking and returning earlier if the domains are already
attached. Now that dev_pm_opp_detach_genpd() can get called multiple
times as well, we need to protect that too.
Note that the virtual device pointers aren't returned in this case, as
they may become unavailable to some callers during the middle of the
operation.
Reported-by: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
The OPP core already has well-defined semantics to ensure required
OPPs/regulators are set before/after the frequency change, depending
on if we scale up or down.
Similar requirements might exist for the order of required OPPs
when multiple power domains need to be scaled for a frequency change.
For example, on Qualcomm platforms using CPR (Core Power Reduction),
we need to scale the VDDMX and CPR power domain. When scaling up,
MX should be scaled up before CPR. When scaling down, CPR should be
scaled down before MX.
In general, if there are multiple "required-opps" in the device tree
I would expect that the order is either irrelevant, or there is some
dependency between the power domains. In that case, the power domains
should be scaled down in reverse order.
This commit updates _set_required_opps() to set required OPPs in
reverse order when scaling down.
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[ Viresh: Fix rebase conflict and minor rearrangement of the code ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Move call to dev_pm_genpd_set_performance_state() to a separate
function so we can avoid duplicating the code for the single and
multiple genpd case.
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[ Viresh: Validate virtual device before use ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Since commit c0ab9e0812da ("opp: Allocate genpd_virt_devs from
dev_pm_opp_attach_genpd()"), the allocation of the virtual devices is
moved to dev_pm_opp_attach_genpd() and this check isn't required anymore
as it will always fail. Drop it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm
Pull operating performance points (OPP) framework fixes for 5.9-rc4 from
Viresh Kumar:
"This fixes reference counting for OPP tables. Few patches are getting
queued (for various subsystems) for 5.10 which depend on this to be
fixed first."
* 'opp/fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm:
opp: Don't drop reference for an OPP table that was never parsed
|
|
dev_pm_opp_remove_table() should drop a reference to the OPP table only
if the DT OPP table was parsed earlier with a call to
dev_pm_opp_of_add_table() earlier. Else it may end up dropping the
reference to the OPP table, which was added as a result of other calls
like dev_pm_opp_set_clkname(). And would hence result in undesirable
behavior later on when caller would try to free the resource again.
Fixes: 03758d60265c ("opp: Replace list_kref with a local counter")
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Reported-by: Anders Roxell <anders.roxell@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
The OPP core manages various resources, e.g. clocks or interconnect paths.
These resources are looked up when the OPP table is allocated once
dev_pm_opp_get_opp_table() is called the first time (either directly
or indirectly through one of the many helper functions).
At this point, the resources may not be available yet, i.e. looking them
up will result in -EPROBE_DEFER. Unfortunately, dev_pm_opp_get_opp_table()
is currently unable to propagate this error code since it only returns
the allocated OPP table or NULL.
This means that all consumers of the OPP core are required to make sure
that all necessary resources are available. Usually this happens by
requesting them, checking the result and releasing them immediately after.
For example, we have added "dev_pm_opp_of_find_icc_paths(dev, NULL)" to
several drivers now just to make sure the interconnect providers are
ready before the OPP table is allocated. If this call is missing,
the OPP core will only warn about this and then attempt to continue
without interconnect. This will eventually fail horribly, e.g.:
cpu cpu0: _allocate_opp_table: Error finding interconnect paths: -517
... later ...
of: _read_bw: Mismatch between opp-peak-kBps and paths (1 0)
cpu cpu0: _opp_add_static_v2: opp key field not found
cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22
This example happens when trying to use interconnects for a CPU OPP
table together with qcom-cpufreq-nvmem.c. qcom-cpufreq-nvmem calls
dev_pm_opp_set_supported_hw(), which ends up allocating the OPP table
early. To fix the problem with the current approach we would need to add
yet another call to dev_pm_opp_of_find_icc_paths(dev, NULL).
But actually qcom-cpufreq-nvmem.c has nothing to do with interconnects...
This commit attempts to make this more robust by allowing
dev_pm_opp_get_opp_table() to return an error pointer. Fixing all
the usages is trivial because the function is usually used indirectly
through another helper (e.g. dev_pm_opp_set_supported_hw() above).
These other helpers already return an error pointer.
The example above then works correctly because set_supported_hw() will
return -EPROBE_DEFER, and qcom-cpufreq-nvmem.c already propagates that
error. It should also be possible to remove the remaining usages of
"dev_pm_opp_of_find_icc_paths(dev, NULL)" from other drivers as well.
Note that this commit currently only handles -EPROBE_DEFER for the
clock/interconnects within _allocate_opp_table(). Other errors are just
ignored as before. Eventually those should be propagated as well.
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
[ Viresh: skip checking return value of dev_pm_opp_get_opp_table() for
EPROBE_DEFER in domain.c, fix NULL return value and reorder
code a bit in core.c, and update exynos-asv.c ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Remove the unnecessary wrapper and merge
_dev_pm_opp_find_and_remove_table() with dev_pm_opp_remove_table().
Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
|
Create separate routine _opp_set_rate_zero() to handle !target_freq
case.
Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
|
