| Message ID | 20241210013010.81257-5-pgwipeout@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rockchip: rk3328 fixes in preparation for usb3-phy | expand |
Hello Peter, On 2024-12-10 02:30, Peter Geis wrote: > There is a race condition at startup between disabling power domains > not > used and disabling clocks not used on the rk3328. When the clocks are > disabled first, the hevc power domain fails to shut off leading to a > splat of failures. Add the hevc core clock to the rk3328 power domain > node to prevent this condition. > > rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 3-.... > } > 1087 jiffies s: 89 root: 0x8/. > rcu: blocking rcu_node structures (internal RCU debug): > Sending NMI from CPU 0 to CPUs 3: > NMI backtrace for cpu 3 > CPU: 3 UID: 0 PID: 86 Comm: kworker/3:3 Not tainted 6.12.0-rc5+ #53 > Hardware name: Firefly ROC-RK3328-CC (DT) > Workqueue: pm genpd_power_off_work_fn > pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : regmap_unlock_spinlock+0x18/0x30 > lr : regmap_read+0x60/0x88 > sp : ffff800081123c00 > x29: ffff800081123c00 x28: ffff2fa4c62cad80 x27: 0000000000000000 > x26: ffffd74e6e660eb8 x25: ffff2fa4c62cae00 x24: 0000000000000040 > x23: ffffd74e6d2f3ab8 x22: 0000000000000001 x21: ffff800081123c74 > x20: 0000000000000000 x19: ffff2fa4c0412000 x18: 0000000000000000 > x17: 77202c31203d2065 x16: 6c6469203a72656c x15: 6c6f72746e6f632d > x14: 7265776f703a6e6f x13: 2063766568206e69 x12: 616d6f64202c3431 > x11: 347830206f742030 x10: 3430303034783020 x9 : ffffd74e6c7369e0 > x8 : 3030316666206e69 x7 : 205d383738353733 x6 : 332e31202020205b > x5 : ffffd74e6c73fc88 x4 : ffffd74e6c73fcd4 x3 : ffffd74e6c740b40 > x2 : ffff800080015484 x1 : 0000000000000000 x0 : ffff2fa4c0412000 > Call trace: > regmap_unlock_spinlock+0x18/0x30 > rockchip_pmu_set_idle_request+0xac/0x2c0 > rockchip_pd_power+0x144/0x5f8 > rockchip_pd_power_off+0x1c/0x30 > _genpd_power_off+0x9c/0x180 > genpd_power_off.part.0.isra.0+0x130/0x2a8 > genpd_power_off_work_fn+0x6c/0x98 > process_one_work+0x170/0x3f0 > worker_thread+0x290/0x4a8 > kthread+0xec/0xf8 > ret_from_fork+0x10/0x20 > rockchip-pm-domain ff100000.syscon:power-controller: failed to get ack > on > domain 'hevc', val=0x88220 > > Fixes: 52e02d377a72 ("arm64: dts: rockchip: add core dtsi file for > RK3328 SoCs") > Signed-off-by: Peter Geis <pgwipeout@gmail.com> While I was unable to formally verify this clock assignment, i.e. by using the RK3328 TRM or the downstream kernel source from Rockchip, it makes perfect sense to me. Thanks for the patch, and please feel free to include: Reviewed-by: Dragan Simic <dsimic@manjaro.org> > --- > > arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi > b/arch/arm64/boot/dts/rockchip/rk3328.dtsi > index 0597de415fe0..7d992c3c01ce 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi > @@ -333,6 +333,7 @@ power: power-controller { > > power-domain@RK3328_PD_HEVC { > reg = <RK3328_PD_HEVC>; > + clocks = <&cru SCLK_VENC_CORE>; > #power-domain-cells = <0>; > }; > power-domain@RK3328_PD_VIDEO {
On Tue, Dec 10, 2024 at 5:04 AM Dragan Simic <dsimic@manjaro.org> wrote: > > Hello Peter, > > On 2024-12-10 02:30, Peter Geis wrote: > > There is a race condition at startup between disabling power domains > > not > > used and disabling clocks not used on the rk3328. When the clocks are > > disabled first, the hevc power domain fails to shut off leading to a > > splat of failures. Add the hevc core clock to the rk3328 power domain > > node to prevent this condition. > > > > rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 3-.... > > } > > 1087 jiffies s: 89 root: 0x8/. > > rcu: blocking rcu_node structures (internal RCU debug): > > Sending NMI from CPU 0 to CPUs 3: > > NMI backtrace for cpu 3 > > CPU: 3 UID: 0 PID: 86 Comm: kworker/3:3 Not tainted 6.12.0-rc5+ #53 > > Hardware name: Firefly ROC-RK3328-CC (DT) > > Workqueue: pm genpd_power_off_work_fn > > pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > pc : regmap_unlock_spinlock+0x18/0x30 > > lr : regmap_read+0x60/0x88 > > sp : ffff800081123c00 > > x29: ffff800081123c00 x28: ffff2fa4c62cad80 x27: 0000000000000000 > > x26: ffffd74e6e660eb8 x25: ffff2fa4c62cae00 x24: 0000000000000040 > > x23: ffffd74e6d2f3ab8 x22: 0000000000000001 x21: ffff800081123c74 > > x20: 0000000000000000 x19: ffff2fa4c0412000 x18: 0000000000000000 > > x17: 77202c31203d2065 x16: 6c6469203a72656c x15: 6c6f72746e6f632d > > x14: 7265776f703a6e6f x13: 2063766568206e69 x12: 616d6f64202c3431 > > x11: 347830206f742030 x10: 3430303034783020 x9 : ffffd74e6c7369e0 > > x8 : 3030316666206e69 x7 : 205d383738353733 x6 : 332e31202020205b > > x5 : ffffd74e6c73fc88 x4 : ffffd74e6c73fcd4 x3 : ffffd74e6c740b40 > > x2 : ffff800080015484 x1 : 0000000000000000 x0 : ffff2fa4c0412000 > > Call trace: > > regmap_unlock_spinlock+0x18/0x30 > > rockchip_pmu_set_idle_request+0xac/0x2c0 > > rockchip_pd_power+0x144/0x5f8 > > rockchip_pd_power_off+0x1c/0x30 > > _genpd_power_off+0x9c/0x180 > > genpd_power_off.part.0.isra.0+0x130/0x2a8 > > genpd_power_off_work_fn+0x6c/0x98 > > process_one_work+0x170/0x3f0 > > worker_thread+0x290/0x4a8 > > kthread+0xec/0xf8 > > ret_from_fork+0x10/0x20 > > rockchip-pm-domain ff100000.syscon:power-controller: failed to get ack > > on > > domain 'hevc', val=0x88220 > > > > Fixes: 52e02d377a72 ("arm64: dts: rockchip: add core dtsi file for > > RK3328 SoCs") > > Signed-off-by: Peter Geis <pgwipeout@gmail.com> > > While I was unable to formally verify this clock assignment, > i.e. by using the RK3328 TRM or the downstream kernel source > from Rockchip, it makes perfect sense to me. Thanks for the > patch, and please feel free to include: > > Reviewed-by: Dragan Simic <dsimic@manjaro.org> It is unfortunate the TRM doesn't include the clock maps, because they are extremely helpful when one can acquire them. It also doesn't help that the TRM register definition only referred to this as "pll". I was sent specifically the usb3 phy clock map for my work on the driver, which had the location of each switch and divider along with the register and bit that controlled it. That combined with the TRM register map allowed me to find this error. Thanks! Peter > > > --- > > > > arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi > > b/arch/arm64/boot/dts/rockchip/rk3328.dtsi > > index 0597de415fe0..7d992c3c01ce 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi > > +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi > > @@ -333,6 +333,7 @@ power: power-controller { > > > > power-domain@RK3328_PD_HEVC { > > reg = <RK3328_PD_HEVC>; > > + clocks = <&cru SCLK_VENC_CORE>; > > #power-domain-cells = <0>; > > }; > > power-domain@RK3328_PD_VIDEO {
On Tue, Dec 10, 2024 at 8:13 AM Peter Geis <pgwipeout@gmail.com> wrote: > > On Tue, Dec 10, 2024 at 5:04 AM Dragan Simic <dsimic@manjaro.org> wrote: > > > > Hello Peter, > > > > On 2024-12-10 02:30, Peter Geis wrote: > > > There is a race condition at startup between disabling power domains > > > not > > > used and disabling clocks not used on the rk3328. When the clocks are > > > disabled first, the hevc power domain fails to shut off leading to a > > > splat of failures. Add the hevc core clock to the rk3328 power domain > > > node to prevent this condition. > > > > > > rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 3-.... > > > } > > > 1087 jiffies s: 89 root: 0x8/. > > > rcu: blocking rcu_node structures (internal RCU debug): > > > Sending NMI from CPU 0 to CPUs 3: > > > NMI backtrace for cpu 3 > > > CPU: 3 UID: 0 PID: 86 Comm: kworker/3:3 Not tainted 6.12.0-rc5+ #53 > > > Hardware name: Firefly ROC-RK3328-CC (DT) > > > Workqueue: pm genpd_power_off_work_fn > > > pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > > pc : regmap_unlock_spinlock+0x18/0x30 > > > lr : regmap_read+0x60/0x88 > > > sp : ffff800081123c00 > > > x29: ffff800081123c00 x28: ffff2fa4c62cad80 x27: 0000000000000000 > > > x26: ffffd74e6e660eb8 x25: ffff2fa4c62cae00 x24: 0000000000000040 > > > x23: ffffd74e6d2f3ab8 x22: 0000000000000001 x21: ffff800081123c74 > > > x20: 0000000000000000 x19: ffff2fa4c0412000 x18: 0000000000000000 > > > x17: 77202c31203d2065 x16: 6c6469203a72656c x15: 6c6f72746e6f632d > > > x14: 7265776f703a6e6f x13: 2063766568206e69 x12: 616d6f64202c3431 > > > x11: 347830206f742030 x10: 3430303034783020 x9 : ffffd74e6c7369e0 > > > x8 : 3030316666206e69 x7 : 205d383738353733 x6 : 332e31202020205b > > > x5 : ffffd74e6c73fc88 x4 : ffffd74e6c73fcd4 x3 : ffffd74e6c740b40 > > > x2 : ffff800080015484 x1 : 0000000000000000 x0 : ffff2fa4c0412000 > > > Call trace: > > > regmap_unlock_spinlock+0x18/0x30 > > > rockchip_pmu_set_idle_request+0xac/0x2c0 > > > rockchip_pd_power+0x144/0x5f8 > > > rockchip_pd_power_off+0x1c/0x30 > > > _genpd_power_off+0x9c/0x180 > > > genpd_power_off.part.0.isra.0+0x130/0x2a8 > > > genpd_power_off_work_fn+0x6c/0x98 > > > process_one_work+0x170/0x3f0 > > > worker_thread+0x290/0x4a8 > > > kthread+0xec/0xf8 > > > ret_from_fork+0x10/0x20 > > > rockchip-pm-domain ff100000.syscon:power-controller: failed to get ack > > > on > > > domain 'hevc', val=0x88220 > > > > > > Fixes: 52e02d377a72 ("arm64: dts: rockchip: add core dtsi file for > > > RK3328 SoCs") > > > Signed-off-by: Peter Geis <pgwipeout@gmail.com> > > > > While I was unable to formally verify this clock assignment, > > i.e. by using the RK3328 TRM or the downstream kernel source > > from Rockchip, it makes perfect sense to me. Thanks for the > > patch, and please feel free to include: > > > > Reviewed-by: Dragan Simic <dsimic@manjaro.org> > > It is unfortunate the TRM doesn't include the clock maps, because they > are extremely helpful when one can acquire them. It also doesn't help > that the TRM register definition only referred to this as "pll". I was > sent specifically the usb3 phy clock map for my work on the driver, > which had the location of each switch and divider along with the > register and bit that controlled it. That combined with the TRM > register map allowed me to find this error. > > Thanks! > Peter Apologies, that's the wrong response for this one. This patch was the result of educated guess combined with testing. I grabbed all of the clocks that looked like they could affect things, then tested them one at a time until I isolated them to this clock. It lives alone with cpll as the parent and has no children according to the clock summary. (Though the writeup i mistakenly included above proves the clock map isn't always accurate). Thanks, Peter > > > > > > --- > > > > > > arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi > > > b/arch/arm64/boot/dts/rockchip/rk3328.dtsi > > > index 0597de415fe0..7d992c3c01ce 100644 > > > --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi > > > +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi > > > @@ -333,6 +333,7 @@ power: power-controller { > > > > > > power-domain@RK3328_PD_HEVC { > > > reg = <RK3328_PD_HEVC>; > > > + clocks = <&cru SCLK_VENC_CORE>; > > > #power-domain-cells = <0>; > > > }; > > > power-domain@RK3328_PD_VIDEO {
Hello Peter, On 2024-12-10 14:23, Peter Geis wrote: > On Tue, Dec 10, 2024 at 8:13 AM Peter Geis <pgwipeout@gmail.com> wrote: >> On Tue, Dec 10, 2024 at 5:04 AM Dragan Simic <dsimic@manjaro.org> >> wrote: >> > On 2024-12-10 02:30, Peter Geis wrote: >> > > There is a race condition at startup between disabling power domains >> > > not >> > > used and disabling clocks not used on the rk3328. When the clocks are >> > > disabled first, the hevc power domain fails to shut off leading to a >> > > splat of failures. Add the hevc core clock to the rk3328 power domain >> > > node to prevent this condition. >> > > >> > > rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 3-.... >> > > } >> > > 1087 jiffies s: 89 root: 0x8/. >> > > rcu: blocking rcu_node structures (internal RCU debug): >> > > Sending NMI from CPU 0 to CPUs 3: >> > > NMI backtrace for cpu 3 >> > > CPU: 3 UID: 0 PID: 86 Comm: kworker/3:3 Not tainted 6.12.0-rc5+ #53 >> > > Hardware name: Firefly ROC-RK3328-CC (DT) >> > > Workqueue: pm genpd_power_off_work_fn >> > > pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) >> > > pc : regmap_unlock_spinlock+0x18/0x30 >> > > lr : regmap_read+0x60/0x88 >> > > sp : ffff800081123c00 >> > > x29: ffff800081123c00 x28: ffff2fa4c62cad80 x27: 0000000000000000 >> > > x26: ffffd74e6e660eb8 x25: ffff2fa4c62cae00 x24: 0000000000000040 >> > > x23: ffffd74e6d2f3ab8 x22: 0000000000000001 x21: ffff800081123c74 >> > > x20: 0000000000000000 x19: ffff2fa4c0412000 x18: 0000000000000000 >> > > x17: 77202c31203d2065 x16: 6c6469203a72656c x15: 6c6f72746e6f632d >> > > x14: 7265776f703a6e6f x13: 2063766568206e69 x12: 616d6f64202c3431 >> > > x11: 347830206f742030 x10: 3430303034783020 x9 : ffffd74e6c7369e0 >> > > x8 : 3030316666206e69 x7 : 205d383738353733 x6 : 332e31202020205b >> > > x5 : ffffd74e6c73fc88 x4 : ffffd74e6c73fcd4 x3 : ffffd74e6c740b40 >> > > x2 : ffff800080015484 x1 : 0000000000000000 x0 : ffff2fa4c0412000 >> > > Call trace: >> > > regmap_unlock_spinlock+0x18/0x30 >> > > rockchip_pmu_set_idle_request+0xac/0x2c0 >> > > rockchip_pd_power+0x144/0x5f8 >> > > rockchip_pd_power_off+0x1c/0x30 >> > > _genpd_power_off+0x9c/0x180 >> > > genpd_power_off.part.0.isra.0+0x130/0x2a8 >> > > genpd_power_off_work_fn+0x6c/0x98 >> > > process_one_work+0x170/0x3f0 >> > > worker_thread+0x290/0x4a8 >> > > kthread+0xec/0xf8 >> > > ret_from_fork+0x10/0x20 >> > > rockchip-pm-domain ff100000.syscon:power-controller: failed to get ack >> > > on >> > > domain 'hevc', val=0x88220 >> > > >> > > Fixes: 52e02d377a72 ("arm64: dts: rockchip: add core dtsi file for >> > > RK3328 SoCs") >> > > Signed-off-by: Peter Geis <pgwipeout@gmail.com> >> > >> > While I was unable to formally verify this clock assignment, >> > i.e. by using the RK3328 TRM or the downstream kernel source >> > from Rockchip, it makes perfect sense to me. Thanks for the >> > patch, and please feel free to include: >> > >> > Reviewed-by: Dragan Simic <dsimic@manjaro.org> >> >> It is unfortunate the TRM doesn't include the clock maps, because they >> are extremely helpful when one can acquire them. It also doesn't help >> that the TRM register definition only referred to this as "pll". I was >> sent specifically the usb3 phy clock map for my work on the driver, >> which had the location of each switch and divider along with the >> register and bit that controlled it. That combined with the TRM >> register map allowed me to find this error. > > Apologies, that's the wrong response for this one. No worries. > This patch was the result of educated guess combined with testing. I > grabbed all of the clocks that looked like they could affect things, > then tested them one at a time until I isolated them to this clock. It > lives alone with cpll as the parent and has no children according to > the clock summary. (Though the writeup i mistakenly included above > proves the clock map isn't always accurate). I see, thanks for all your work on this patch! It surely took quite a lot of time to perform all the testing. >> > > --- >> > > >> > > arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + >> > > 1 file changed, 1 insertion(+) >> > > >> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi >> > > b/arch/arm64/boot/dts/rockchip/rk3328.dtsi >> > > index 0597de415fe0..7d992c3c01ce 100644 >> > > --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi >> > > +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi >> > > @@ -333,6 +333,7 @@ power: power-controller { >> > > >> > > power-domain@RK3328_PD_HEVC { >> > > reg = <RK3328_PD_HEVC>; >> > > + clocks = <&cru SCLK_VENC_CORE>; >> > > #power-domain-cells = <0>; >> > > }; >> > > power-domain@RK3328_PD_VIDEO {
Hi Peter, On 2024-12-10 14:53, Dragan Simic wrote: > Hello Peter, > > On 2024-12-10 14:23, Peter Geis wrote: >> On Tue, Dec 10, 2024 at 8:13 AM Peter Geis <pgwipeout@gmail.com> wrote: >>> On Tue, Dec 10, 2024 at 5:04 AM Dragan Simic <dsimic@manjaro.org> >>> wrote: >>>> On 2024-12-10 02:30, Peter Geis wrote: >>>>> There is a race condition at startup between disabling power domains >>>>> not >>>>> used and disabling clocks not used on the rk3328. When the clocks are >>>>> disabled first, the hevc power domain fails to shut off leading to a >>>>> splat of failures. Add the hevc core clock to the rk3328 power domain >>>>> node to prevent this condition. >>>>> >>>>> rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 3-.... >>>>> } >>>>> 1087 jiffies s: 89 root: 0x8/. >>>>> rcu: blocking rcu_node structures (internal RCU debug): >>>>> Sending NMI from CPU 0 to CPUs 3: >>>>> NMI backtrace for cpu 3 >>>>> CPU: 3 UID: 0 PID: 86 Comm: kworker/3:3 Not tainted 6.12.0-rc5+ #53 >>>>> Hardware name: Firefly ROC-RK3328-CC (DT) >>>>> Workqueue: pm genpd_power_off_work_fn >>>>> pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) >>>>> pc : regmap_unlock_spinlock+0x18/0x30 >>>>> lr : regmap_read+0x60/0x88 >>>>> sp : ffff800081123c00 >>>>> x29: ffff800081123c00 x28: ffff2fa4c62cad80 x27: 0000000000000000 >>>>> x26: ffffd74e6e660eb8 x25: ffff2fa4c62cae00 x24: 0000000000000040 >>>>> x23: ffffd74e6d2f3ab8 x22: 0000000000000001 x21: ffff800081123c74 >>>>> x20: 0000000000000000 x19: ffff2fa4c0412000 x18: 0000000000000000 >>>>> x17: 77202c31203d2065 x16: 6c6469203a72656c x15: 6c6f72746e6f632d >>>>> x14: 7265776f703a6e6f x13: 2063766568206e69 x12: 616d6f64202c3431 >>>>> x11: 347830206f742030 x10: 3430303034783020 x9 : ffffd74e6c7369e0 >>>>> x8 : 3030316666206e69 x7 : 205d383738353733 x6 : 332e31202020205b >>>>> x5 : ffffd74e6c73fc88 x4 : ffffd74e6c73fcd4 x3 : ffffd74e6c740b40 >>>>> x2 : ffff800080015484 x1 : 0000000000000000 x0 : ffff2fa4c0412000 >>>>> Call trace: >>>>> regmap_unlock_spinlock+0x18/0x30 >>>>> rockchip_pmu_set_idle_request+0xac/0x2c0 >>>>> rockchip_pd_power+0x144/0x5f8 >>>>> rockchip_pd_power_off+0x1c/0x30 >>>>> _genpd_power_off+0x9c/0x180 >>>>> genpd_power_off.part.0.isra.0+0x130/0x2a8 >>>>> genpd_power_off_work_fn+0x6c/0x98 >>>>> process_one_work+0x170/0x3f0 >>>>> worker_thread+0x290/0x4a8 >>>>> kthread+0xec/0xf8 >>>>> ret_from_fork+0x10/0x20 >>>>> rockchip-pm-domain ff100000.syscon:power-controller: failed to get ack >>>>> on >>>>> domain 'hevc', val=0x88220 >>>>> >>>>> Fixes: 52e02d377a72 ("arm64: dts: rockchip: add core dtsi file for >>>>> RK3328 SoCs") >>>>> Signed-off-by: Peter Geis <pgwipeout@gmail.com> >>>> >>>> While I was unable to formally verify this clock assignment, >>>> i.e. by using the RK3328 TRM or the downstream kernel source >>>> from Rockchip, it makes perfect sense to me. Thanks for the >>>> patch, and please feel free to include: >>>> >>>> Reviewed-by: Dragan Simic <dsimic@manjaro.org> >>> >>> It is unfortunate the TRM doesn't include the clock maps, because they >>> are extremely helpful when one can acquire them. It also doesn't help >>> that the TRM register definition only referred to this as "pll". I was >>> sent specifically the usb3 phy clock map for my work on the driver, >>> which had the location of each switch and divider along with the >>> register and bit that controlled it. That combined with the TRM >>> register map allowed me to find this error. >> >> Apologies, that's the wrong response for this one. > > No worries. > >> This patch was the result of educated guess combined with testing. I >> grabbed all of the clocks that looked like they could affect things, >> then tested them one at a time until I isolated them to this clock. It >> lives alone with cpll as the parent and has no children according to >> the clock summary. (Though the writeup i mistakenly included above >> proves the clock map isn't always accurate). > > I see, thanks for all your work on this patch! It surely took quite > a lot of time to perform all the testing. > >>>>> --- >>>>> >>>>> arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi >>>>> b/arch/arm64/boot/dts/rockchip/rk3328.dtsi >>>>> index 0597de415fe0..7d992c3c01ce 100644 >>>>> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi >>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi >>>>> @@ -333,6 +333,7 @@ power: power-controller { >>>>> >>>>> power-domain@RK3328_PD_HEVC { >>>>> reg = <RK3328_PD_HEVC>; >>>>> + clocks = <&cru SCLK_VENC_CORE>; Do we also need to include one or all of the following clocks? According to Fig. 3-6 RK3228H Clock Architecture Diagram 5 following clocks point to the H265 block: S51_6 (4PLL) / G6_3 / S51_0 (DivFree 1~32) / D4 ---- aclk_h265 \-- pclk_h265 S51_14 (4PLL) / G6_4 / S51_8 (DivFree 1~32) / D4 - clk_venc_core S52_14 (4PLL) / G6_7 / S52_8 (DivFree 1~32) / D4 - clk_venc_dsp Regards, Jonas >>>>> #power-domain-cells = <0>; >>>>> }; >>>>> power-domain@RK3328_PD_VIDEO {
On Tue, Dec 10, 2024 at 11:05 AM Jonas Karlman <jonas@kwiboo.se> wrote: > > Hi Peter, > > On 2024-12-10 14:53, Dragan Simic wrote: > > Hello Peter, > > > > On 2024-12-10 14:23, Peter Geis wrote: > >> On Tue, Dec 10, 2024 at 8:13 AM Peter Geis <pgwipeout@gmail.com> wrote: > >>> On Tue, Dec 10, 2024 at 5:04 AM Dragan Simic <dsimic@manjaro.org> > >>> wrote: > >>>> On 2024-12-10 02:30, Peter Geis wrote: > >>>>> There is a race condition at startup between disabling power domains > >>>>> not > >>>>> used and disabling clocks not used on the rk3328. When the clocks are > >>>>> disabled first, the hevc power domain fails to shut off leading to a > >>>>> splat of failures. Add the hevc core clock to the rk3328 power domain > >>>>> node to prevent this condition. > >>>>> > >>>>> rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 3-.... > >>>>> } > >>>>> 1087 jiffies s: 89 root: 0x8/. > >>>>> rcu: blocking rcu_node structures (internal RCU debug): > >>>>> Sending NMI from CPU 0 to CPUs 3: > >>>>> NMI backtrace for cpu 3 > >>>>> CPU: 3 UID: 0 PID: 86 Comm: kworker/3:3 Not tainted 6.12.0-rc5+ #53 > >>>>> Hardware name: Firefly ROC-RK3328-CC (DT) > >>>>> Workqueue: pm genpd_power_off_work_fn > >>>>> pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > >>>>> pc : regmap_unlock_spinlock+0x18/0x30 > >>>>> lr : regmap_read+0x60/0x88 > >>>>> sp : ffff800081123c00 > >>>>> x29: ffff800081123c00 x28: ffff2fa4c62cad80 x27: 0000000000000000 > >>>>> x26: ffffd74e6e660eb8 x25: ffff2fa4c62cae00 x24: 0000000000000040 > >>>>> x23: ffffd74e6d2f3ab8 x22: 0000000000000001 x21: ffff800081123c74 > >>>>> x20: 0000000000000000 x19: ffff2fa4c0412000 x18: 0000000000000000 > >>>>> x17: 77202c31203d2065 x16: 6c6469203a72656c x15: 6c6f72746e6f632d > >>>>> x14: 7265776f703a6e6f x13: 2063766568206e69 x12: 616d6f64202c3431 > >>>>> x11: 347830206f742030 x10: 3430303034783020 x9 : ffffd74e6c7369e0 > >>>>> x8 : 3030316666206e69 x7 : 205d383738353733 x6 : 332e31202020205b > >>>>> x5 : ffffd74e6c73fc88 x4 : ffffd74e6c73fcd4 x3 : ffffd74e6c740b40 > >>>>> x2 : ffff800080015484 x1 : 0000000000000000 x0 : ffff2fa4c0412000 > >>>>> Call trace: > >>>>> regmap_unlock_spinlock+0x18/0x30 > >>>>> rockchip_pmu_set_idle_request+0xac/0x2c0 > >>>>> rockchip_pd_power+0x144/0x5f8 > >>>>> rockchip_pd_power_off+0x1c/0x30 > >>>>> _genpd_power_off+0x9c/0x180 > >>>>> genpd_power_off.part.0.isra.0+0x130/0x2a8 > >>>>> genpd_power_off_work_fn+0x6c/0x98 > >>>>> process_one_work+0x170/0x3f0 > >>>>> worker_thread+0x290/0x4a8 > >>>>> kthread+0xec/0xf8 > >>>>> ret_from_fork+0x10/0x20 > >>>>> rockchip-pm-domain ff100000.syscon:power-controller: failed to get ack > >>>>> on > >>>>> domain 'hevc', val=0x88220 > >>>>> > >>>>> Fixes: 52e02d377a72 ("arm64: dts: rockchip: add core dtsi file for > >>>>> RK3328 SoCs") > >>>>> Signed-off-by: Peter Geis <pgwipeout@gmail.com> > >>>> > >>>> While I was unable to formally verify this clock assignment, > >>>> i.e. by using the RK3328 TRM or the downstream kernel source > >>>> from Rockchip, it makes perfect sense to me. Thanks for the > >>>> patch, and please feel free to include: > >>>> > >>>> Reviewed-by: Dragan Simic <dsimic@manjaro.org> > >>> > >>> It is unfortunate the TRM doesn't include the clock maps, because they > >>> are extremely helpful when one can acquire them. It also doesn't help > >>> that the TRM register definition only referred to this as "pll". I was > >>> sent specifically the usb3 phy clock map for my work on the driver, > >>> which had the location of each switch and divider along with the > >>> register and bit that controlled it. That combined with the TRM > >>> register map allowed me to find this error. > >> > >> Apologies, that's the wrong response for this one. > > > > No worries. > > > >> This patch was the result of educated guess combined with testing. I > >> grabbed all of the clocks that looked like they could affect things, > >> then tested them one at a time until I isolated them to this clock. It > >> lives alone with cpll as the parent and has no children according to > >> the clock summary. (Though the writeup i mistakenly included above > >> proves the clock map isn't always accurate). > > > > I see, thanks for all your work on this patch! It surely took quite > > a lot of time to perform all the testing. > > > >>>>> --- > >>>>> > >>>>> arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + > >>>>> 1 file changed, 1 insertion(+) > >>>>> > >>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi > >>>>> b/arch/arm64/boot/dts/rockchip/rk3328.dtsi > >>>>> index 0597de415fe0..7d992c3c01ce 100644 > >>>>> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi > >>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi > >>>>> @@ -333,6 +333,7 @@ power: power-controller { > >>>>> > >>>>> power-domain@RK3328_PD_HEVC { > >>>>> reg = <RK3328_PD_HEVC>; > >>>>> + clocks = <&cru SCLK_VENC_CORE>; > > Do we also need to include one or all of the following clocks? > > According to Fig. 3-6 RK3228H Clock Architecture Diagram 5 following > clocks point to the H265 block: > > S51_6 (4PLL) / G6_3 / S51_0 (DivFree 1~32) / D4 ---- aclk_h265 > \-- pclk_h265 > S51_14 (4PLL) / G6_4 / S51_8 (DivFree 1~32) / D4 - clk_venc_core > S52_14 (4PLL) / G6_7 / S52_8 (DivFree 1~32) / D4 - clk_venc_dsp Good Afternoon, Thanks for asking! If we were implementing the full encoder, probably. However even with all the clocks enabled currently the encoder hard locks the board if we touch it. For now adding just the SCLK_VENC_CORE is enough to enable control of the power domain. Very Respectfully, Peter Geis > > Regards, > Jonas > > >>>>> #power-domain-cells = <0>; > >>>>> }; > >>>>> power-domain@RK3328_PD_VIDEO { >
diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi index 0597de415fe0..7d992c3c01ce 100644 --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi @@ -333,6 +333,7 @@ power: power-controller { power-domain@RK3328_PD_HEVC { reg = <RK3328_PD_HEVC>; + clocks = <&cru SCLK_VENC_CORE>; #power-domain-cells = <0>; }; power-domain@RK3328_PD_VIDEO {
There is a race condition at startup between disabling power domains not used and disabling clocks not used on the rk3328. When the clocks are disabled first, the hevc power domain fails to shut off leading to a splat of failures. Add the hevc core clock to the rk3328 power domain node to prevent this condition. rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 3-.... } 1087 jiffies s: 89 root: 0x8/. rcu: blocking rcu_node structures (internal RCU debug): Sending NMI from CPU 0 to CPUs 3: NMI backtrace for cpu 3 CPU: 3 UID: 0 PID: 86 Comm: kworker/3:3 Not tainted 6.12.0-rc5+ #53 Hardware name: Firefly ROC-RK3328-CC (DT) Workqueue: pm genpd_power_off_work_fn pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : regmap_unlock_spinlock+0x18/0x30 lr : regmap_read+0x60/0x88 sp : ffff800081123c00 x29: ffff800081123c00 x28: ffff2fa4c62cad80 x27: 0000000000000000 x26: ffffd74e6e660eb8 x25: ffff2fa4c62cae00 x24: 0000000000000040 x23: ffffd74e6d2f3ab8 x22: 0000000000000001 x21: ffff800081123c74 x20: 0000000000000000 x19: ffff2fa4c0412000 x18: 0000000000000000 x17: 77202c31203d2065 x16: 6c6469203a72656c x15: 6c6f72746e6f632d x14: 7265776f703a6e6f x13: 2063766568206e69 x12: 616d6f64202c3431 x11: 347830206f742030 x10: 3430303034783020 x9 : ffffd74e6c7369e0 x8 : 3030316666206e69 x7 : 205d383738353733 x6 : 332e31202020205b x5 : ffffd74e6c73fc88 x4 : ffffd74e6c73fcd4 x3 : ffffd74e6c740b40 x2 : ffff800080015484 x1 : 0000000000000000 x0 : ffff2fa4c0412000 Call trace: regmap_unlock_spinlock+0x18/0x30 rockchip_pmu_set_idle_request+0xac/0x2c0 rockchip_pd_power+0x144/0x5f8 rockchip_pd_power_off+0x1c/0x30 _genpd_power_off+0x9c/0x180 genpd_power_off.part.0.isra.0+0x130/0x2a8 genpd_power_off_work_fn+0x6c/0x98 process_one_work+0x170/0x3f0 worker_thread+0x290/0x4a8 kthread+0xec/0xf8 ret_from_fork+0x10/0x20 rockchip-pm-domain ff100000.syscon:power-controller: failed to get ack on domain 'hevc', val=0x88220 Fixes: 52e02d377a72 ("arm64: dts: rockchip: add core dtsi file for RK3328 SoCs") Signed-off-by: Peter Geis <pgwipeout@gmail.com> --- arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 + 1 file changed, 1 insertion(+)