| Message ID | 20241118-b4-linux-next-24-11-18-clock-multiple-power-domains-v1-0-b7a2bd82ba37@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | clk: qcom: Add support for multiple power-domains for a clock controller. | expand |
On Mon, Nov 18, 2024 at 02:24:31AM +0000, Bryan O'Donoghue wrote: > On x1e80100 and it's SKUs the Camera Clock Controller - CAMCC has > multiple power-domains which power it. Usually with a single power-domain > the core platform code will automatically switch on the singleton > power-domain for you. If you have multiple power-domains for a device, in > this case the clock controller, you need to switch those power-domains > on/off yourself. I think the series misses the platform-specific part. It is hard to understand what kind of power relationship do you need to express. Is it actually the whole CC being powered by several domains? Or are some of those domains used to power up PLLs? Or as parents to some of GDSCs? > > The clock controllers can also contain Global Distributed > Switch Controllers - GDSCs which themselves can be referenced from dtsi > nodes ultimately triggering a gdsc_en() in drivers/clk/qcom/gdsc.c. > > As an example: > > cci0: cci@ac4a000 { > power-domains = <&camcc TITAN_TOP_GDSC>; > }; > > This series adds the support to attach a power-domain list to the > clock-controllers and the GDSCs those controllers provide so that in the > case of the above example gdsc_toggle_logic() will trigger the power-domain > list with pm_runtime_resume_and_get() and pm_runtime_put_sync() > respectively. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > Bryan O'Donoghue (2): > clk: qcom: common: Add support for power-domain attachment > clk: qcom: gdsc: Add pm_runtime hooks > > drivers/clk/qcom/common.c | 24 ++++++++++++++++++++++++ > drivers/clk/qcom/gdsc.c | 26 ++++++++++++++++++-------- > drivers/clk/qcom/gdsc.h | 2 ++ > 3 files changed, 44 insertions(+), 8 deletions(-) > --- > base-commit: 744cf71b8bdfcdd77aaf58395e068b7457634b2c > change-id: 20241118-b4-linux-next-24-11-18-clock-multiple-power-domains-a5f994dc452a > > Best regards, > -- > Bryan O'Donoghue <bryan.odonoghue@linaro.org> >
On 18/11/2024 13:15, Dmitry Baryshkov wrote: >> On x1e80100 and it's SKUs the Camera Clock Controller - CAMCC has >> multiple power-domains which power it. Usually with a single power-domain >> the core platform code will automatically switch on the singleton >> power-domain for you. If you have multiple power-domains for a device, in >> this case the clock controller, you need to switch those power-domains >> on/off yourself. > I think the series misses the platform-specific part. I don't think I understand what you mean by that. It is hard to > understand what kind of power relationship do you need to express. Is it > actually the whole CC being powered by several domains? Or are some of > those domains used to power up PLLs? Or as parents to some of GDSCs? Well for example the TITAN_TOP_GDSC needs both PDs to be switched on. We _could_ do this just for the CAMCC on x1e80100 - i.e. make it just for the camcc device but then, the next clock controller that needs more than one PD will have to implement the same fix. --- bod
On 18/11/2024 13:22, Bryan O'Donoghue wrote: > On 18/11/2024 13:15, Dmitry Baryshkov wrote: >>> On x1e80100 and it's SKUs the Camera Clock Controller - CAMCC has >>> multiple power-domains which power it. Usually with a single power- >>> domain >>> the core platform code will automatically switch on the singleton >>> power-domain for you. If you have multiple power-domains for a >>> device, in >>> this case the clock controller, you need to switch those power-domains >>> on/off yourself. > >> I think the series misses the platform-specific part. > > I don't think I understand what you mean by that. > > It is hard to >> understand what kind of power relationship do you need to express. Is it >> actually the whole CC being powered by several domains? Or are some of >> those domains used to power up PLLs? Or as parents to some of GDSCs? > > Well for example the TITAN_TOP_GDSC needs both PDs to be switched on. > > We _could_ do this just for the CAMCC on x1e80100 - i.e. make it just > for the camcc device but then, the next clock controller that needs more > than one PD will have to implement the same fix. Can some of the PLLs run with one PD or the other ? Perhaps, but without _both_ PDs on the GDSC will stay stuck @ off for my test case on x1e80100 and looking at other places we manage PDs directly - drivers/media/platform/venus.c for example its generally just all or nothing. There may be more granularity to extract but, I don't think there's more granularity for the first case here. Both PDs are needed or the GDSC will remain stuck @ off. As I see it we can either address 1. At the drivers/clk/qcom/camcc-somesoc.c level 2. At drivers/clk/qcom/[gdsc.c|common.c] level I think option 2 is more how you'd expect this to work though. Its not particularly obvious but ATM with singleton power-domains for the clock controllers the singletons just get turned on and left that way. So managing the PD on/off inside of common.c/gdsc.c means the calling clock controller can stay as a simple probe() type driver and also means we can reuse the generic common.c/gdsc.c approach. --- bod
On 11/18/2024 7:54 AM, Bryan O'Donoghue wrote: > On x1e80100 and it's SKUs the Camera Clock Controller - CAMCC has > multiple power-domains which power it. Usually with a single power-domain > the core platform code will automatically switch on the singleton > power-domain for you. If you have multiple power-domains for a device, in > this case the clock controller, you need to switch those power-domains > on/off yourself. > > The clock controllers can also contain Global Distributed > Switch Controllers - GDSCs which themselves can be referenced from dtsi > nodes ultimately triggering a gdsc_en() in drivers/clk/qcom/gdsc.c. > > As an example: > > cci0: cci@ac4a000 { > power-domains = <&camcc TITAN_TOP_GDSC>; > }; > > This series adds the support to attach a power-domain list to the > clock-controllers and the GDSCs those controllers provide so that in the > case of the above example gdsc_toggle_logic() will trigger the power-domain > list with pm_runtime_resume_and_get() and pm_runtime_put_sync() > respectively. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- Bryan, as we were already in discussion with Bjorn to post the patches which take care of Multi GDSC and PLL requirements, I would request to kindly hold this series posting. I am in the final discussions with Bjorn to handle it gracefully to post the series. > Bryan O'Donoghue (2): > clk: qcom: common: Add support for power-domain attachment > clk: qcom: gdsc: Add pm_runtime hooks > > drivers/clk/qcom/common.c | 24 ++++++++++++++++++++++++ > drivers/clk/qcom/gdsc.c | 26 ++++++++++++++++++-------- > drivers/clk/qcom/gdsc.h | 2 ++ > 3 files changed, 44 insertions(+), 8 deletions(-) > --- > base-commit: 744cf71b8bdfcdd77aaf58395e068b7457634b2c > change-id: 20241118-b4-linux-next-24-11-18-clock-multiple-power-domains-a5f994dc452a > > Best regards,
On 19/11/2024 06:08, Taniya Das wrote: > > Bryan, as we were already in discussion with Bjorn to post the patches > which take care of Multi GDSC and PLL requirements, I would request to > kindly hold this series posting. I am in the final discussions with > Bjorn to handle it gracefully to post the series. Is this not 'graceful' ? Hmm. What specifically don't you like about this series ? --- bod
On Tue, Nov 19, 2024 at 11:38:34AM +0530, Taniya Das wrote: > > > On 11/18/2024 7:54 AM, Bryan O'Donoghue wrote: > > On x1e80100 and it's SKUs the Camera Clock Controller - CAMCC has > > multiple power-domains which power it. Usually with a single power-domain > > the core platform code will automatically switch on the singleton > > power-domain for you. If you have multiple power-domains for a device, in > > this case the clock controller, you need to switch those power-domains > > on/off yourself. > > > > The clock controllers can also contain Global Distributed > > Switch Controllers - GDSCs which themselves can be referenced from dtsi > > nodes ultimately triggering a gdsc_en() in drivers/clk/qcom/gdsc.c. > > > > As an example: > > > > cci0: cci@ac4a000 { > > power-domains = <&camcc TITAN_TOP_GDSC>; > > }; > > > > This series adds the support to attach a power-domain list to the > > clock-controllers and the GDSCs those controllers provide so that in the > > case of the above example gdsc_toggle_logic() will trigger the power-domain > > list with pm_runtime_resume_and_get() and pm_runtime_put_sync() > > respectively. > > > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > --- > > Bryan, as we were already in discussion with Bjorn to post the patches which > take care of Multi GDSC and PLL requirements, I would request to kindly hold > this series posting. There's no "hold before posting", this series is already posted. Please review it. > I am in the final discussions with Bjorn to handle it > gracefully to post the series. > You may in such discussion (the review) say "you're missing X, Y, Z, and here is my patches that covers these aspects", but not "I'll ignore this until we're done preparing our patches". Regards, Bjorn
On x1e80100 and it's SKUs the Camera Clock Controller - CAMCC has multiple power-domains which power it. Usually with a single power-domain the core platform code will automatically switch on the singleton power-domain for you. If you have multiple power-domains for a device, in this case the clock controller, you need to switch those power-domains on/off yourself. The clock controllers can also contain Global Distributed Switch Controllers - GDSCs which themselves can be referenced from dtsi nodes ultimately triggering a gdsc_en() in drivers/clk/qcom/gdsc.c. As an example: cci0: cci@ac4a000 { power-domains = <&camcc TITAN_TOP_GDSC>; }; This series adds the support to attach a power-domain list to the clock-controllers and the GDSCs those controllers provide so that in the case of the above example gdsc_toggle_logic() will trigger the power-domain list with pm_runtime_resume_and_get() and pm_runtime_put_sync() respectively. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- Bryan O'Donoghue (2): clk: qcom: common: Add support for power-domain attachment clk: qcom: gdsc: Add pm_runtime hooks drivers/clk/qcom/common.c | 24 ++++++++++++++++++++++++ drivers/clk/qcom/gdsc.c | 26 ++++++++++++++++++-------- drivers/clk/qcom/gdsc.h | 2 ++ 3 files changed, 44 insertions(+), 8 deletions(-) --- base-commit: 744cf71b8bdfcdd77aaf58395e068b7457634b2c change-id: 20241118-b4-linux-next-24-11-18-clock-multiple-power-domains-a5f994dc452a Best regards,