| Message ID | 20210922125419.4125779-3-maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: hdmi: Remove CPU hangs, take 2 | expand |
On Wed, 2021-09-22 at 14:54 +0200, Maxime Ripard wrote: > The driver, once it found a divider, tries to round it up by increasing > the least significant bit of the fractional part by one when the > round_up argument is set and there's a remainder. > > However, since it increases the divider it will actually reduce the > clock rate below what we were asking for, leading to issues with > clk_set_min_rate() that will complain that our rounded clock rate is > below the minimum of the rate. > > Since the dividers are fairly precise already, let's remove that part so > that we can have clk_set_min_rate() working. > > This is effectively a revert of 9c95b32ca093 ("clk: bcm2835: add a round > up ability to the clock divisor"). > > Fixes: 9c95b32ca093 ("clk: bcm2835: add a round up ability to the clock divisor") > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- Reviewed-by: Nicolas Saenz Julienne <nsaenz@kernel.org> Tested-by: Nicolas Saenz Julienne <nsaenz@kernel.org> # boot and basic functionality Regards, Nicolas
On Tue, Sep 28, 2021 at 06:26:55PM +0200, nicolas saenz julienne wrote: > On Wed, 2021-09-22 at 14:54 +0200, Maxime Ripard wrote: > > The driver, once it found a divider, tries to round it up by increasing > > the least significant bit of the fractional part by one when the > > round_up argument is set and there's a remainder. > > > > However, since it increases the divider it will actually reduce the > > clock rate below what we were asking for, leading to issues with > > clk_set_min_rate() that will complain that our rounded clock rate is > > below the minimum of the rate. > > > > Since the dividers are fairly precise already, let's remove that part so > > that we can have clk_set_min_rate() working. > > > > This is effectively a revert of 9c95b32ca093 ("clk: bcm2835: add a round > > up ability to the clock divisor"). > > > > Fixes: 9c95b32ca093 ("clk: bcm2835: add a round up ability to the clock divisor") > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > --- > > Reviewed-by: Nicolas Saenz Julienne <nsaenz@kernel.org> > Tested-by: Nicolas Saenz Julienne <nsaenz@kernel.org> # boot and basic functionality Does that mean you're ok with merging it through the DRM-misc tree? Florian, Mike, Stephen, any objection? Maxime
Quoting Maxime Ripard (2021-09-22 05:54:16) > The driver, once it found a divider, tries to round it up by increasing > the least significant bit of the fractional part by one when the > round_up argument is set and there's a remainder. > > However, since it increases the divider it will actually reduce the > clock rate below what we were asking for, leading to issues with > clk_set_min_rate() that will complain that our rounded clock rate is > below the minimum of the rate. > > Since the dividers are fairly precise already, let's remove that part so > that we can have clk_set_min_rate() working. > > This is effectively a revert of 9c95b32ca093 ("clk: bcm2835: add a round > up ability to the clock divisor"). > > Fixes: 9c95b32ca093 ("clk: bcm2835: add a round up ability to the clock divisor") > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- Acked-by: Stephen Boyd <sboyd@kernel.org>
On Wed, 22 Sep 2021 14:54:16 +0200, Maxime Ripard wrote: > The driver, once it found a divider, tries to round it up by increasing > the least significant bit of the fractional part by one when the > round_up argument is set and there's a remainder. > > However, since it increases the divider it will actually reduce the > clock rate below what we were asking for, leading to issues with > clk_set_min_rate() that will complain that our rounded clock rate is > below the minimum of the rate. > > [...] Applied to drm/drm-misc (drm-misc-fixes). Thanks! Maxime
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index bf97b2b2a63f..3667b4d731e7 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -932,8 +932,7 @@ static int bcm2835_clock_is_on(struct clk_hw *hw) static u32 bcm2835_clock_choose_div(struct clk_hw *hw, unsigned long rate, - unsigned long parent_rate, - bool round_up) + unsigned long parent_rate) { struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw); const struct bcm2835_clock_data *data = clock->data; @@ -945,10 +944,6 @@ static u32 bcm2835_clock_choose_div(struct clk_hw *hw, rem = do_div(temp, rate); div = temp; - - /* Round up and mask off the unused bits */ - if (round_up && ((div & unused_frac_mask) != 0 || rem != 0)) - div += unused_frac_mask + 1; div &= ~unused_frac_mask; /* different clamping limits apply for a mash clock */ @@ -1079,7 +1074,7 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw, struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw); struct bcm2835_cprman *cprman = clock->cprman; const struct bcm2835_clock_data *data = clock->data; - u32 div = bcm2835_clock_choose_div(hw, rate, parent_rate, false); + u32 div = bcm2835_clock_choose_div(hw, rate, parent_rate); u32 ctl; spin_lock(&cprman->regs_lock); @@ -1130,7 +1125,7 @@ static unsigned long bcm2835_clock_choose_div_and_prate(struct clk_hw *hw, if (!(BIT(parent_idx) & data->set_rate_parent)) { *prate = clk_hw_get_rate(parent); - *div = bcm2835_clock_choose_div(hw, rate, *prate, true); + *div = bcm2835_clock_choose_div(hw, rate, *prate); *avgrate = bcm2835_clock_rate_from_divisor(clock, *prate, *div);
The driver, once it found a divider, tries to round it up by increasing the least significant bit of the fractional part by one when the round_up argument is set and there's a remainder. However, since it increases the divider it will actually reduce the clock rate below what we were asking for, leading to issues with clk_set_min_rate() that will complain that our rounded clock rate is below the minimum of the rate. Since the dividers are fairly precise already, let's remove that part so that we can have clk_set_min_rate() working. This is effectively a revert of 9c95b32ca093 ("clk: bcm2835: add a round up ability to the clock divisor"). Fixes: 9c95b32ca093 ("clk: bcm2835: add a round up ability to the clock divisor") Signed-off-by: Maxime Ripard <maxime@cerno.tech> --- drivers/clk/bcm/clk-bcm2835.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)