[PATCH] clk: qcom: gfx3d: add parent to parent request map

Dmitry Baryshkov posted 1 patch 3 weeks, 1 day ago
drivers/clk/qcom/clk-rcg2.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] clk: qcom: gfx3d: add parent to parent request map
Posted by Dmitry Baryshkov 3 weeks, 1 day ago
After commit d228ece36345 ("clk: divider: remove round_rate() in favor
of determine_rate()") determining GFX3D clock rate crashes, because the
passed parent map doesn't provide the expected best_parent_hw clock
(with the roundd_rate path before the offending commit the
best_parent_hw was ignored).

Set the field in parent_req in addition to setting it in the req,
fixing the crash.

 clk_hw_round_rate (drivers/clk/clk.c:1764) (P)
 clk_divider_bestdiv (drivers/clk/clk-divider.c:336)
 divider_determine_rate (drivers/clk/clk-divider.c:358)
 clk_alpha_pll_postdiv_determine_rate (drivers/clk/qcom/clk-alpha-pll.c:1275)
 clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
 clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
 __clk_determine_rate (drivers/clk/clk.c:1741)
 clk_gfx3d_determine_rate (drivers/clk/qcom/clk-rcg2.c:1268)
 clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
 clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
 clk_core_round_rate_nolock (drivers/clk/clk.c:1710)
 clk_round_rate (drivers/clk/clk.c:1804)
 dev_pm_opp_set_rate (drivers/opp/core.c:1440 (discriminator 1))
 msm_devfreq_target (drivers/gpu/drm/msm/msm_gpu_devfreq.c:51)
 devfreq_set_target (drivers/devfreq/devfreq.c:360)
 devfreq_update_target (drivers/devfreq/devfreq.c:426)
 devfreq_monitor (drivers/devfreq/devfreq.c:458)
 process_one_work (arch/arm64/include/asm/jump_label.h:36 include/trace/events/workqueue.h:110 kernel/workqueue.c:3284)
 worker_thread (kernel/workqueue.c:3356 (discriminator 2) kernel/workqueue.c:3443 (discriminator 2))
 kthread (kernel/kthread.c:467)
 ret_from_fork (arch/arm64/kernel/entry.S:861)

Fixes: 55213e1acec9 ("clk: qcom: Add gfx3d ping-pong PLL frequency switching")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/clk/qcom/clk-rcg2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index 2838d4cb2d58..d0a5847f9111 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -1264,6 +1264,7 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
 	if (req->max_rate < parent_req.max_rate)
 		parent_req.max_rate = req->max_rate;
 
+	parent_req.best_parent_hw = req->best_parent_hw;
 	ret = __clk_determine_rate(req->best_parent_hw, &parent_req);
 	if (ret)
 		return ret;

---
base-commit: b775e489bec70895b7ef6b66927886bbac79598f
change-id: 20260117-db820-fix-gfx3d-e61329023c8a

Best regards,
-- 
With best wishes
Dmitry
Re: [PATCH] clk: qcom: gfx3d: add parent to parent request map
Posted by Bjorn Andersson 2 weeks, 4 days ago
On Sat, 17 Jan 2026 05:54:47 +0200, Dmitry Baryshkov wrote:
> After commit d228ece36345 ("clk: divider: remove round_rate() in favor
> of determine_rate()") determining GFX3D clock rate crashes, because the
> passed parent map doesn't provide the expected best_parent_hw clock
> (with the roundd_rate path before the offending commit the
> best_parent_hw was ignored).
> 
> Set the field in parent_req in addition to setting it in the req,
> fixing the crash.
> 
> [...]

Applied, thanks!

[1/1] clk: qcom: gfx3d: add parent to parent request map
      commit: 2583cb925ca1ce450aa5d74a05a67448db970193

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>
Re: [PATCH] clk: qcom: gfx3d: add parent to parent request map
Posted by Bjorn Andersson 2 weeks, 5 days ago
On Sat, Jan 17, 2026 at 05:54:47AM +0200, Dmitry Baryshkov wrote:
> After commit d228ece36345 ("clk: divider: remove round_rate() in favor
> of determine_rate()") determining GFX3D clock rate crashes, because the
> passed parent map doesn't provide the expected best_parent_hw clock
> (with the roundd_rate path before the offending commit the
> best_parent_hw was ignored).
> 
> Set the field in parent_req in addition to setting it in the req,
> fixing the crash.
> 
>  clk_hw_round_rate (drivers/clk/clk.c:1764) (P)
>  clk_divider_bestdiv (drivers/clk/clk-divider.c:336)
>  divider_determine_rate (drivers/clk/clk-divider.c:358)
>  clk_alpha_pll_postdiv_determine_rate (drivers/clk/qcom/clk-alpha-pll.c:1275)
>  clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
>  clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
>  __clk_determine_rate (drivers/clk/clk.c:1741)
>  clk_gfx3d_determine_rate (drivers/clk/qcom/clk-rcg2.c:1268)
>  clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
>  clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
>  clk_core_round_rate_nolock (drivers/clk/clk.c:1710)
>  clk_round_rate (drivers/clk/clk.c:1804)
>  dev_pm_opp_set_rate (drivers/opp/core.c:1440 (discriminator 1))
>  msm_devfreq_target (drivers/gpu/drm/msm/msm_gpu_devfreq.c:51)
>  devfreq_set_target (drivers/devfreq/devfreq.c:360)
>  devfreq_update_target (drivers/devfreq/devfreq.c:426)
>  devfreq_monitor (drivers/devfreq/devfreq.c:458)
>  process_one_work (arch/arm64/include/asm/jump_label.h:36 include/trace/events/workqueue.h:110 kernel/workqueue.c:3284)
>  worker_thread (kernel/workqueue.c:3356 (discriminator 2) kernel/workqueue.c:3443 (discriminator 2))
>  kthread (kernel/kthread.c:467)
>  ret_from_fork (arch/arm64/kernel/entry.S:861)
> 
> Fixes: 55213e1acec9 ("clk: qcom: Add gfx3d ping-pong PLL frequency switching")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Why didn't Brian get Cc'ed on this patch? I'd love to have his input.

Regards,
Bjorn

> ---
>  drivers/clk/qcom/clk-rcg2.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 2838d4cb2d58..d0a5847f9111 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -1264,6 +1264,7 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
>  	if (req->max_rate < parent_req.max_rate)
>  		parent_req.max_rate = req->max_rate;
>  
> +	parent_req.best_parent_hw = req->best_parent_hw;
>  	ret = __clk_determine_rate(req->best_parent_hw, &parent_req);
>  	if (ret)
>  		return ret;
> 
> ---
> base-commit: b775e489bec70895b7ef6b66927886bbac79598f
> change-id: 20260117-db820-fix-gfx3d-e61329023c8a
> 
> Best regards,
> -- 
> With best wishes
> Dmitry
> 
>
Re: [PATCH] clk: qcom: gfx3d: add parent to parent request map
Posted by Brian Masney 2 weeks, 4 days ago
On Mon, Jan 19, 2026 at 09:39:30AM -0600, Bjorn Andersson wrote:
> On Sat, Jan 17, 2026 at 05:54:47AM +0200, Dmitry Baryshkov wrote:
> > After commit d228ece36345 ("clk: divider: remove round_rate() in favor
> > of determine_rate()") determining GFX3D clock rate crashes, because the
> > passed parent map doesn't provide the expected best_parent_hw clock
> > (with the roundd_rate path before the offending commit the
> > best_parent_hw was ignored).
> > 
> > Set the field in parent_req in addition to setting it in the req,
> > fixing the crash.
> > 
> >  clk_hw_round_rate (drivers/clk/clk.c:1764) (P)
> >  clk_divider_bestdiv (drivers/clk/clk-divider.c:336)
> >  divider_determine_rate (drivers/clk/clk-divider.c:358)
> >  clk_alpha_pll_postdiv_determine_rate (drivers/clk/qcom/clk-alpha-pll.c:1275)
> >  clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
> >  clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
> >  __clk_determine_rate (drivers/clk/clk.c:1741)
> >  clk_gfx3d_determine_rate (drivers/clk/qcom/clk-rcg2.c:1268)
> >  clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
> >  clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
> >  clk_core_round_rate_nolock (drivers/clk/clk.c:1710)
> >  clk_round_rate (drivers/clk/clk.c:1804)
> >  dev_pm_opp_set_rate (drivers/opp/core.c:1440 (discriminator 1))
> >  msm_devfreq_target (drivers/gpu/drm/msm/msm_gpu_devfreq.c:51)
> >  devfreq_set_target (drivers/devfreq/devfreq.c:360)
> >  devfreq_update_target (drivers/devfreq/devfreq.c:426)
> >  devfreq_monitor (drivers/devfreq/devfreq.c:458)
> >  process_one_work (arch/arm64/include/asm/jump_label.h:36 include/trace/events/workqueue.h:110 kernel/workqueue.c:3284)
> >  worker_thread (kernel/workqueue.c:3356 (discriminator 2) kernel/workqueue.c:3443 (discriminator 2))
> >  kthread (kernel/kthread.c:467)
> >  ret_from_fork (arch/arm64/kernel/entry.S:861)
> > 
> > Fixes: 55213e1acec9 ("clk: qcom: Add gfx3d ping-pong PLL frequency switching")
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> 
> Why didn't Brian get Cc'ed on this patch? I'd love to have his input.

Reviewed-by: Brian Masney <bmasney@redhat.com>

clk_alpha_pll_postdiv_round_rate() previously had this code:

        return divider_round_rate(hw, rate, prate, table,
                                  pll->width, CLK_DIVIDER_POWER_OF_TWO);

divider_round_rate() called divider_round_rate_parent(), which had this
code that set the best_parent_hw:

long divider_round_rate_parent(struct clk_hw *hw, struct clk_hw *parent,
                               unsigned long rate, unsigned long *prate,
                               const struct clk_div_table *table,
                               u8 width, unsigned long flags)
{
        struct clk_rate_request req;
        int ret;

        clk_hw_init_rate_request(hw, &req, rate);
        req.best_parent_rate = *prate;
        req.best_parent_hw = parent;

        ret = divider_determine_rate(hw, &req, table, width, flags);
        if (ret)
                return ret;

        *prate = req.best_parent_rate;

        return req.rate;
}

I coverted clk_alpha_pll_postdiv_round_rate() to
clk_alpha_pll_postdiv_determine_rate(), and that now directly calls
divider_determine_rate().

I'll look through the other cases where divider_round_rate_parent() was
called just to be sure there's no other cases of this.

Brian
Re: [PATCH] clk: qcom: gfx3d: add parent to parent request map
Posted by Bjorn Andersson 2 weeks, 4 days ago
On Tue, Jan 20, 2026 at 10:14:32AM -0500, Brian Masney wrote:
> On Mon, Jan 19, 2026 at 09:39:30AM -0600, Bjorn Andersson wrote:
> > On Sat, Jan 17, 2026 at 05:54:47AM +0200, Dmitry Baryshkov wrote:
> > > After commit d228ece36345 ("clk: divider: remove round_rate() in favor
> > > of determine_rate()") determining GFX3D clock rate crashes, because the
> > > passed parent map doesn't provide the expected best_parent_hw clock
> > > (with the roundd_rate path before the offending commit the
> > > best_parent_hw was ignored).
> > > 
> > > Set the field in parent_req in addition to setting it in the req,
> > > fixing the crash.
> > > 
> > >  clk_hw_round_rate (drivers/clk/clk.c:1764) (P)
> > >  clk_divider_bestdiv (drivers/clk/clk-divider.c:336)
> > >  divider_determine_rate (drivers/clk/clk-divider.c:358)
> > >  clk_alpha_pll_postdiv_determine_rate (drivers/clk/qcom/clk-alpha-pll.c:1275)
> > >  clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
> > >  clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
> > >  __clk_determine_rate (drivers/clk/clk.c:1741)
> > >  clk_gfx3d_determine_rate (drivers/clk/qcom/clk-rcg2.c:1268)
> > >  clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
> > >  clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
> > >  clk_core_round_rate_nolock (drivers/clk/clk.c:1710)
> > >  clk_round_rate (drivers/clk/clk.c:1804)
> > >  dev_pm_opp_set_rate (drivers/opp/core.c:1440 (discriminator 1))
> > >  msm_devfreq_target (drivers/gpu/drm/msm/msm_gpu_devfreq.c:51)
> > >  devfreq_set_target (drivers/devfreq/devfreq.c:360)
> > >  devfreq_update_target (drivers/devfreq/devfreq.c:426)
> > >  devfreq_monitor (drivers/devfreq/devfreq.c:458)
> > >  process_one_work (arch/arm64/include/asm/jump_label.h:36 include/trace/events/workqueue.h:110 kernel/workqueue.c:3284)
> > >  worker_thread (kernel/workqueue.c:3356 (discriminator 2) kernel/workqueue.c:3443 (discriminator 2))
> > >  kthread (kernel/kthread.c:467)
> > >  ret_from_fork (arch/arm64/kernel/entry.S:861)
> > > 
> > > Fixes: 55213e1acec9 ("clk: qcom: Add gfx3d ping-pong PLL frequency switching")
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > 
> > Why didn't Brian get Cc'ed on this patch? I'd love to have his input.
> 
> Reviewed-by: Brian Masney <bmasney@redhat.com>
> 
> clk_alpha_pll_postdiv_round_rate() previously had this code:
> 
>         return divider_round_rate(hw, rate, prate, table,
>                                   pll->width, CLK_DIVIDER_POWER_OF_TWO);
> 
> divider_round_rate() called divider_round_rate_parent(), which had this
> code that set the best_parent_hw:
> 
> long divider_round_rate_parent(struct clk_hw *hw, struct clk_hw *parent,
>                                unsigned long rate, unsigned long *prate,
>                                const struct clk_div_table *table,
>                                u8 width, unsigned long flags)
> {
>         struct clk_rate_request req;
>         int ret;
> 
>         clk_hw_init_rate_request(hw, &req, rate);
>         req.best_parent_rate = *prate;
>         req.best_parent_hw = parent;
> 
>         ret = divider_determine_rate(hw, &req, table, width, flags);
>         if (ret)
>                 return ret;
> 
>         *prate = req.best_parent_rate;
> 
>         return req.rate;
> }
> 
> I coverted clk_alpha_pll_postdiv_round_rate() to
> clk_alpha_pll_postdiv_determine_rate(), and that now directly calls
> divider_determine_rate().
> 
> I'll look through the other cases where divider_round_rate_parent() was
> called just to be sure there's no other cases of this.
> 

Thank you, Brian!

> Brian
>
Re: [PATCH] clk: qcom: gfx3d: add parent to parent request map
Posted by Abel Vesa 2 weeks, 5 days ago
On 26-01-17 05:54:47, Dmitry Baryshkov wrote:
> After commit d228ece36345 ("clk: divider: remove round_rate() in favor
> of determine_rate()") determining GFX3D clock rate crashes, because the
> passed parent map doesn't provide the expected best_parent_hw clock
> (with the roundd_rate path before the offending commit the
> best_parent_hw was ignored).
> 
> Set the field in parent_req in addition to setting it in the req,
> fixing the crash.
> 
>  clk_hw_round_rate (drivers/clk/clk.c:1764) (P)
>  clk_divider_bestdiv (drivers/clk/clk-divider.c:336)
>  divider_determine_rate (drivers/clk/clk-divider.c:358)
>  clk_alpha_pll_postdiv_determine_rate (drivers/clk/qcom/clk-alpha-pll.c:1275)
>  clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
>  clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
>  __clk_determine_rate (drivers/clk/clk.c:1741)
>  clk_gfx3d_determine_rate (drivers/clk/qcom/clk-rcg2.c:1268)
>  clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
>  clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
>  clk_core_round_rate_nolock (drivers/clk/clk.c:1710)
>  clk_round_rate (drivers/clk/clk.c:1804)
>  dev_pm_opp_set_rate (drivers/opp/core.c:1440 (discriminator 1))
>  msm_devfreq_target (drivers/gpu/drm/msm/msm_gpu_devfreq.c:51)
>  devfreq_set_target (drivers/devfreq/devfreq.c:360)
>  devfreq_update_target (drivers/devfreq/devfreq.c:426)
>  devfreq_monitor (drivers/devfreq/devfreq.c:458)
>  process_one_work (arch/arm64/include/asm/jump_label.h:36 include/trace/events/workqueue.h:110 kernel/workqueue.c:3284)
>  worker_thread (kernel/workqueue.c:3356 (discriminator 2) kernel/workqueue.c:3443 (discriminator 2))
>  kthread (kernel/kthread.c:467)
>  ret_from_fork (arch/arm64/kernel/entry.S:861)
> 
> Fixes: 55213e1acec9 ("clk: qcom: Add gfx3d ping-pong PLL frequency switching")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Re: [PATCH] clk: qcom: gfx3d: add parent to parent request map
Posted by Konrad Dybcio 2 weeks, 5 days ago
On 1/17/26 4:54 AM, Dmitry Baryshkov wrote:
> After commit d228ece36345 ("clk: divider: remove round_rate() in favor
> of determine_rate()") determining GFX3D clock rate crashes, because the
> passed parent map doesn't provide the expected best_parent_hw clock
> (with the roundd_rate path before the offending commit the
> best_parent_hw was ignored).
> 
> Set the field in parent_req in addition to setting it in the req,
> fixing the crash.
> 
>  clk_hw_round_rate (drivers/clk/clk.c:1764) (P)
>  clk_divider_bestdiv (drivers/clk/clk-divider.c:336)
>  divider_determine_rate (drivers/clk/clk-divider.c:358)
>  clk_alpha_pll_postdiv_determine_rate (drivers/clk/qcom/clk-alpha-pll.c:1275)
>  clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
>  clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
>  __clk_determine_rate (drivers/clk/clk.c:1741)
>  clk_gfx3d_determine_rate (drivers/clk/qcom/clk-rcg2.c:1268)
>  clk_core_determine_round_nolock (drivers/clk/clk.c:1606)
>  clk_core_round_rate_nolock (drivers/clk/clk.c:1701)
>  clk_core_round_rate_nolock (drivers/clk/clk.c:1710)
>  clk_round_rate (drivers/clk/clk.c:1804)
>  dev_pm_opp_set_rate (drivers/opp/core.c:1440 (discriminator 1))
>  msm_devfreq_target (drivers/gpu/drm/msm/msm_gpu_devfreq.c:51)
>  devfreq_set_target (drivers/devfreq/devfreq.c:360)
>  devfreq_update_target (drivers/devfreq/devfreq.c:426)
>  devfreq_monitor (drivers/devfreq/devfreq.c:458)
>  process_one_work (arch/arm64/include/asm/jump_label.h:36 include/trace/events/workqueue.h:110 kernel/workqueue.c:3284)
>  worker_thread (kernel/workqueue.c:3356 (discriminator 2) kernel/workqueue.c:3443 (discriminator 2))
>  kthread (kernel/kthread.c:467)
>  ret_from_fork (arch/arm64/kernel/entry.S:861)
> 
> Fixes: 55213e1acec9 ("clk: qcom: Add gfx3d ping-pong PLL frequency switching")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---

I'm still not fully understanding the logic of this code, but I
feel even less comfortable having a board crash on boot and I can
attest to there being a dereference of best_parent_hw..

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad


>  drivers/clk/qcom/clk-rcg2.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 2838d4cb2d58..d0a5847f9111 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -1264,6 +1264,7 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
>  	if (req->max_rate < parent_req.max_rate)
>  		parent_req.max_rate = req->max_rate;
>  
> +	parent_req.best_parent_hw = req->best_parent_hw;
>  	ret = __clk_determine_rate(req->best_parent_hw, &parent_req);
>  	if (ret)
>  		return ret;
> 
> ---
> base-commit: b775e489bec70895b7ef6b66927886bbac79598f
> change-id: 20260117-db820-fix-gfx3d-e61329023c8a
> 
> Best regards,