diff mbox series

[v2,4/6] clk: qcom: Use HW_CTRL_TRIGGER flag to switch video GDSC to HW mode

Message ID 20230816145741.1472721-5-abel.vesa@linaro.org
State New
Headers show
Series PM: domains: Add control for switching back and forth to HW control | expand

Commit Message

Abel Vesa Aug. 16, 2023, 2:57 p.m. UTC
From: Jagadeesh Kona <quic_jkona@quicinc.com>

The current HW_CTRL flag switches the video GDSC to HW control mode as
part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
give consumer drivers more control and switch the GDSC mode as and when
required.

HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/clk/qcom/videocc-sc7180.c | 2 +-
 drivers/clk/qcom/videocc-sc7280.c | 2 +-
 drivers/clk/qcom/videocc-sdm845.c | 4 ++--
 drivers/clk/qcom/videocc-sm8250.c | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

Comments

Abel Vesa Aug. 23, 2023, 10:41 a.m. UTC | #1
On 23-08-16 19:56:46, Konrad Dybcio wrote:
> On 16.08.2023 16:57, Abel Vesa wrote:
> > From: Jagadeesh Kona <quic_jkona@quicinc.com>
> > 
> > The current HW_CTRL flag switches the video GDSC to HW control mode as
> > part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
> > give consumer drivers more control and switch the GDSC mode as and when
> > required.
> > 
> > HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
> > HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
> > 
> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> Do we have any use for the HW_CTRL flag?
> 
> Perhaps it should be renamed to HW_CTRL_ALWAYS?
> 
> Or even better, *if and only if* that is necessary, add a common
> property like "always_hw_managed" to the genpd code?

The HW_CTRL flag is still needed for the consumers that expect the GDSC
to be have the HW control bit set right after it gets enabled.

AFAIU, for the HW_CTRL_TRIGGER, the switch to HW control will only be
done by the consumer (not on enable).

> 
> Konrad
Konrad Dybcio Aug. 26, 2023, 10:47 a.m. UTC | #2
On 23.08.2023 12:41, Abel Vesa wrote:
> On 23-08-16 19:56:46, Konrad Dybcio wrote:
>> On 16.08.2023 16:57, Abel Vesa wrote:
>>> From: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>
>>> The current HW_CTRL flag switches the video GDSC to HW control mode as
>>> part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
>>> give consumer drivers more control and switch the GDSC mode as and when
>>> required.
>>>
>>> HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
>>> HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
>>>
>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>> ---
>> Do we have any use for the HW_CTRL flag?
>>
>> Perhaps it should be renamed to HW_CTRL_ALWAYS?
>>
>> Or even better, *if and only if* that is necessary, add a common
>> property like "always_hw_managed" to the genpd code?
> 
> The HW_CTRL flag is still needed for the consumers that expect the GDSC
> to be have the HW control bit set right after it gets enabled.
Guess the correct question here would be.. Are there any?

Konrad
Jagadeesh Kona Aug. 28, 2023, 6:48 a.m. UTC | #3
On 8/26/2023 4:17 PM, Konrad Dybcio wrote:
> On 23.08.2023 12:41, Abel Vesa wrote:
>> On 23-08-16 19:56:46, Konrad Dybcio wrote:
>>> On 16.08.2023 16:57, Abel Vesa wrote:
>>>> From: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>
>>>> The current HW_CTRL flag switches the video GDSC to HW control mode as
>>>> part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
>>>> give consumer drivers more control and switch the GDSC mode as and when
>>>> required.
>>>>
>>>> HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
>>>> HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
>>>>
>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>>> ---
>>> Do we have any use for the HW_CTRL flag?
>>>
>>> Perhaps it should be renamed to HW_CTRL_ALWAYS?
>>>
>>> Or even better, *if and only if* that is necessary, add a common
>>> property like "always_hw_managed" to the genpd code?
>>
>> The HW_CTRL flag is still needed for the consumers that expect the GDSC
>> to be have the HW control bit set right after it gets enabled.
> Guess the correct question here would be.. Are there any?
> 

Yes, Display GDSC(mdss_gdsc) is required to be controlled always in HW 
control mode when it is enabled.

Thanks,
Jagadeesh

> Konrad
Abel Vesa Aug. 28, 2023, 7:30 a.m. UTC | #4
On 23-08-28 12:18:30, Jagadeesh Kona wrote:
> 
> 
> On 8/26/2023 4:17 PM, Konrad Dybcio wrote:
> > On 23.08.2023 12:41, Abel Vesa wrote:
> > > On 23-08-16 19:56:46, Konrad Dybcio wrote:
> > > > On 16.08.2023 16:57, Abel Vesa wrote:
> > > > > From: Jagadeesh Kona <quic_jkona@quicinc.com>
> > > > > 
> > > > > The current HW_CTRL flag switches the video GDSC to HW control mode as
> > > > > part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
> > > > > give consumer drivers more control and switch the GDSC mode as and when
> > > > > required.
> > > > > 
> > > > > HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
> > > > > HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
> > > > > 
> > > > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > > > ---
> > > > Do we have any use for the HW_CTRL flag?
> > > > 
> > > > Perhaps it should be renamed to HW_CTRL_ALWAYS?
> > > > 
> > > > Or even better, *if and only if* that is necessary, add a common
> > > > property like "always_hw_managed" to the genpd code?
> > > 
> > > The HW_CTRL flag is still needed for the consumers that expect the GDSC
> > > to be have the HW control bit set right after it gets enabled.
> > Guess the correct question here would be.. Are there any?
> > 
> 
> Yes, Display GDSC(mdss_gdsc) is required to be controlled always in HW
> control mode when it is enabled.
> 

Actually, since all the GDSCs that support HW control are by default
switched to HW mode after they are enabled, we can't make any changes
with respect to that since we risk breaking consumers. Therefore, the
new flag makes perfect sense since we can switch GDSCs from HW_CTRL to
HW_CTRL_TRIGGER per platform/consumer.


> Thanks,
> Jagadeesh
> 
> > Konrad
Konrad Dybcio Sept. 2, 2023, 11:59 a.m. UTC | #5
On 28.08.2023 09:30, Abel Vesa wrote:
> On 23-08-28 12:18:30, Jagadeesh Kona wrote:
>>
>>
>> On 8/26/2023 4:17 PM, Konrad Dybcio wrote:
>>> On 23.08.2023 12:41, Abel Vesa wrote:
>>>> On 23-08-16 19:56:46, Konrad Dybcio wrote:
>>>>> On 16.08.2023 16:57, Abel Vesa wrote:
>>>>>> From: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>
>>>>>> The current HW_CTRL flag switches the video GDSC to HW control mode as
>>>>>> part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
>>>>>> give consumer drivers more control and switch the GDSC mode as and when
>>>>>> required.
>>>>>>
>>>>>> HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
>>>>>> HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
>>>>>>
>>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>>>>> ---
>>>>> Do we have any use for the HW_CTRL flag?
>>>>>
>>>>> Perhaps it should be renamed to HW_CTRL_ALWAYS?
>>>>>
>>>>> Or even better, *if and only if* that is necessary, add a common
>>>>> property like "always_hw_managed" to the genpd code?
>>>>
>>>> The HW_CTRL flag is still needed for the consumers that expect the GDSC
>>>> to be have the HW control bit set right after it gets enabled.
>>> Guess the correct question here would be.. Are there any?
>>>
>>
>> Yes, Display GDSC(mdss_gdsc) is required to be controlled always in HW
>> control mode when it is enabled.
>>
> 
> Actually, since all the GDSCs that support HW control are by default
> switched to HW mode after they are enabled, we can't make any changes
> with respect to that since we risk breaking consumers. Therefore, the
> new flag makes perfect sense since we can switch GDSCs from HW_CTRL to
> HW_CTRL_TRIGGER per platform/consumer.
Ok, I can get behind this reasoning.

The flag name gives me a little 'eeh' feeling, but I can't
think of anything much better either..

Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
Konrad Dybcio Sept. 2, 2023, 12:03 p.m. UTC | #6
On 28.08.2023 08:48, Jagadeesh Kona wrote:
> 
> 
> On 8/26/2023 4:17 PM, Konrad Dybcio wrote:
>> On 23.08.2023 12:41, Abel Vesa wrote:
>>> On 23-08-16 19:56:46, Konrad Dybcio wrote:
>>>> On 16.08.2023 16:57, Abel Vesa wrote:
>>>>> From: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>
>>>>> The current HW_CTRL flag switches the video GDSC to HW control mode as
>>>>> part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
>>>>> give consumer drivers more control and switch the GDSC mode as and when
>>>>> required.
>>>>>
>>>>> HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
>>>>> HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
>>>>>
>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>>>> ---
>>>> Do we have any use for the HW_CTRL flag?
>>>>
>>>> Perhaps it should be renamed to HW_CTRL_ALWAYS?
>>>>
>>>> Or even better, *if and only if* that is necessary, add a common
>>>> property like "always_hw_managed" to the genpd code?
>>>
>>> The HW_CTRL flag is still needed for the consumers that expect the GDSC
>>> to be have the HW control bit set right after it gets enabled.
>> Guess the correct question here would be.. Are there any?
>>
> 
> Yes, Display GDSC(mdss_gdsc) is required to be controlled always in HW control mode when it is enabled.
Oh really?

Looking at msm-5.10 techpack, only the SDE RSC driver seems to
trigger regulator fast mode (so, enabling gdsc hw_ctrl on downstream).

Konrad
Jagadeesh Kona Sept. 4, 2023, 9:27 a.m. UTC | #7
On 9/2/2023 5:33 PM, Konrad Dybcio wrote:
> On 28.08.2023 08:48, Jagadeesh Kona wrote:
>>
>>
>> On 8/26/2023 4:17 PM, Konrad Dybcio wrote:
>>> On 23.08.2023 12:41, Abel Vesa wrote:
>>>> On 23-08-16 19:56:46, Konrad Dybcio wrote:
>>>>> On 16.08.2023 16:57, Abel Vesa wrote:
>>>>>> From: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>
>>>>>> The current HW_CTRL flag switches the video GDSC to HW control mode as
>>>>>> part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
>>>>>> give consumer drivers more control and switch the GDSC mode as and when
>>>>>> required.
>>>>>>
>>>>>> HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
>>>>>> HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
>>>>>>
>>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>>>>> ---
>>>>> Do we have any use for the HW_CTRL flag?
>>>>>
>>>>> Perhaps it should be renamed to HW_CTRL_ALWAYS?
>>>>>
>>>>> Or even better, *if and only if* that is necessary, add a common
>>>>> property like "always_hw_managed" to the genpd code?
>>>>
>>>> The HW_CTRL flag is still needed for the consumers that expect the GDSC
>>>> to be have the HW control bit set right after it gets enabled.
>>> Guess the correct question here would be.. Are there any?
>>>
>>
>> Yes, Display GDSC(mdss_gdsc) is required to be controlled always in HW control mode when it is enabled.
> Oh really?
> 
> Looking at msm-5.10 techpack, only the SDE RSC driver seems to
> trigger regulator fast mode (so, enabling gdsc hw_ctrl on downstream).
> 

Yes, on downstream, display GDSC has only one consumer(SDE RSC driver) 
and there are no other consumers. SDE RSC driver switches the GDSC to hw 
control mode once GDSC is enabled and leaves it in hw control mode. Thanks!

Regards,
Jagadeesh

> Konrad
Konrad Dybcio Sept. 4, 2023, 4:02 p.m. UTC | #8
On 4.09.2023 11:27, Jagadeesh Kona wrote:
> 
> 
> On 9/2/2023 5:33 PM, Konrad Dybcio wrote:
>> On 28.08.2023 08:48, Jagadeesh Kona wrote:
>>>
>>>
>>> On 8/26/2023 4:17 PM, Konrad Dybcio wrote:
>>>> On 23.08.2023 12:41, Abel Vesa wrote:
>>>>> On 23-08-16 19:56:46, Konrad Dybcio wrote:
>>>>>> On 16.08.2023 16:57, Abel Vesa wrote:
>>>>>>> From: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>>
>>>>>>> The current HW_CTRL flag switches the video GDSC to HW control mode as
>>>>>>> part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
>>>>>>> give consumer drivers more control and switch the GDSC mode as and when
>>>>>>> required.
>>>>>>>
>>>>>>> HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
>>>>>>> HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
>>>>>>>
>>>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>>>>>> ---
>>>>>> Do we have any use for the HW_CTRL flag?
>>>>>>
>>>>>> Perhaps it should be renamed to HW_CTRL_ALWAYS?
>>>>>>
>>>>>> Or even better, *if and only if* that is necessary, add a common
>>>>>> property like "always_hw_managed" to the genpd code?
>>>>>
>>>>> The HW_CTRL flag is still needed for the consumers that expect the GDSC
>>>>> to be have the HW control bit set right after it gets enabled.
>>>> Guess the correct question here would be.. Are there any?
>>>>
>>>
>>> Yes, Display GDSC(mdss_gdsc) is required to be controlled always in HW control mode when it is enabled.
>> Oh really?
>>
>> Looking at msm-5.10 techpack, only the SDE RSC driver seems to
>> trigger regulator fast mode (so, enabling gdsc hw_ctrl on downstream).
>>
> 
> Yes, on downstream, display GDSC has only one consumer(SDE RSC driver) and there are no other consumers. SDE RSC driver switches the GDSC to hw control mode once GDSC is enabled and leaves it in hw control mode. Thanks!
Sorry for pulling your tongue here a bit, but would it only concern
RPMh SoCs? Designs like SM6115 don't implement RSCs, should they not
have HW_CTRL enabled at all times?

Konrad
Jagadeesh Kona Sept. 7, 2023, 5:55 a.m. UTC | #9
On 9/4/2023 9:32 PM, Konrad Dybcio wrote:
> On 4.09.2023 11:27, Jagadeesh Kona wrote:
>>
>>
>> On 9/2/2023 5:33 PM, Konrad Dybcio wrote:
>>> On 28.08.2023 08:48, Jagadeesh Kona wrote:
>>>>
>>>>
>>>> On 8/26/2023 4:17 PM, Konrad Dybcio wrote:
>>>>> On 23.08.2023 12:41, Abel Vesa wrote:
>>>>>> On 23-08-16 19:56:46, Konrad Dybcio wrote:
>>>>>>> On 16.08.2023 16:57, Abel Vesa wrote:
>>>>>>>> From: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>>>
>>>>>>>> The current HW_CTRL flag switches the video GDSC to HW control mode as
>>>>>>>> part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
>>>>>>>> give consumer drivers more control and switch the GDSC mode as and when
>>>>>>>> required.
>>>>>>>>
>>>>>>>> HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
>>>>>>>> HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
>>>>>>>>
>>>>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>>>>>>> ---
>>>>>>> Do we have any use for the HW_CTRL flag?
>>>>>>>
>>>>>>> Perhaps it should be renamed to HW_CTRL_ALWAYS?
>>>>>>>
>>>>>>> Or even better, *if and only if* that is necessary, add a common
>>>>>>> property like "always_hw_managed" to the genpd code?
>>>>>>
>>>>>> The HW_CTRL flag is still needed for the consumers that expect the GDSC
>>>>>> to be have the HW control bit set right after it gets enabled.
>>>>> Guess the correct question here would be.. Are there any?
>>>>>
>>>>
>>>> Yes, Display GDSC(mdss_gdsc) is required to be controlled always in HW control mode when it is enabled.
>>> Oh really?
>>>
>>> Looking at msm-5.10 techpack, only the SDE RSC driver seems to
>>> trigger regulator fast mode (so, enabling gdsc hw_ctrl on downstream).
>>>
>>
>> Yes, on downstream, display GDSC has only one consumer(SDE RSC driver) and there are no other consumers. SDE RSC driver switches the GDSC to hw control mode once GDSC is enabled and leaves it in hw control mode. Thanks!
> Sorry for pulling your tongue here a bit, but would it only concern
> RPMh SoCs? Designs like SM6115 don't implement RSCs, should they not
> have HW_CTRL enabled at all times?
> 

Yes, for RPMh SoCs which have display RSC block, GDSC is switched to HW 
control mode. For SoCs which doesn't have display RSC block, display 
driver controls the GDSC in SW mode on downstream. Thanks!

Regards,
Jagadeesh

> Konrad
Konrad Dybcio Sept. 7, 2023, 7:36 a.m. UTC | #10
On 7.09.2023 07:55, Jagadeesh Kona wrote:
> 
> 
> On 9/4/2023 9:32 PM, Konrad Dybcio wrote:
>> On 4.09.2023 11:27, Jagadeesh Kona wrote:
>>>
>>>
>>> On 9/2/2023 5:33 PM, Konrad Dybcio wrote:
>>>> On 28.08.2023 08:48, Jagadeesh Kona wrote:
>>>>>
>>>>>
>>>>> On 8/26/2023 4:17 PM, Konrad Dybcio wrote:
>>>>>> On 23.08.2023 12:41, Abel Vesa wrote:
>>>>>>> On 23-08-16 19:56:46, Konrad Dybcio wrote:
>>>>>>>> On 16.08.2023 16:57, Abel Vesa wrote:
>>>>>>>>> From: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>>>>
>>>>>>>>> The current HW_CTRL flag switches the video GDSC to HW control mode as
>>>>>>>>> part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
>>>>>>>>> give consumer drivers more control and switch the GDSC mode as and when
>>>>>>>>> required.
>>>>>>>>>
>>>>>>>>> HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
>>>>>>>>> HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>>>>>>>> ---
>>>>>>>> Do we have any use for the HW_CTRL flag?
>>>>>>>>
>>>>>>>> Perhaps it should be renamed to HW_CTRL_ALWAYS?
>>>>>>>>
>>>>>>>> Or even better, *if and only if* that is necessary, add a common
>>>>>>>> property like "always_hw_managed" to the genpd code?
>>>>>>>
>>>>>>> The HW_CTRL flag is still needed for the consumers that expect the GDSC
>>>>>>> to be have the HW control bit set right after it gets enabled.
>>>>>> Guess the correct question here would be.. Are there any?
>>>>>>
>>>>>
>>>>> Yes, Display GDSC(mdss_gdsc) is required to be controlled always in HW control mode when it is enabled.
>>>> Oh really?
>>>>
>>>> Looking at msm-5.10 techpack, only the SDE RSC driver seems to
>>>> trigger regulator fast mode (so, enabling gdsc hw_ctrl on downstream).
>>>>
>>>
>>> Yes, on downstream, display GDSC has only one consumer(SDE RSC driver) and there are no other consumers. SDE RSC driver switches the GDSC to hw control mode once GDSC is enabled and leaves it in hw control mode. Thanks!
>> Sorry for pulling your tongue here a bit, but would it only concern
>> RPMh SoCs? Designs like SM6115 don't implement RSCs, should they not
>> have HW_CTRL enabled at all times?
>>
> 
> Yes, for RPMh SoCs which have display RSC block, GDSC is switched to HW control mode. For SoCs which doesn't have display RSC block, display driver controls the GDSC in SW mode on downstream. Thanks!
Thanks for explaining!

One last question, I promise.. Should we switch the MDSS GDSC to
HW_CTRL mode only after we start controlling the DISP RSC from Linux,
or should it be done regardless (because of the RPMh solving algos)?

Konrad
Jagadeesh Kona Sept. 11, 2023, 12:47 p.m. UTC | #11
On 9/7/2023 1:06 PM, Konrad Dybcio wrote:
> On 7.09.2023 07:55, Jagadeesh Kona wrote:
>>
>>
>> On 9/4/2023 9:32 PM, Konrad Dybcio wrote:
>>> On 4.09.2023 11:27, Jagadeesh Kona wrote:
>>>>
>>>>
>>>> On 9/2/2023 5:33 PM, Konrad Dybcio wrote:
>>>>> On 28.08.2023 08:48, Jagadeesh Kona wrote:
>>>>>>
>>>>>>
>>>>>> On 8/26/2023 4:17 PM, Konrad Dybcio wrote:
>>>>>>> On 23.08.2023 12:41, Abel Vesa wrote:
>>>>>>>> On 23-08-16 19:56:46, Konrad Dybcio wrote:
>>>>>>>>> On 16.08.2023 16:57, Abel Vesa wrote:
>>>>>>>>>> From: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>>>>>
>>>>>>>>>> The current HW_CTRL flag switches the video GDSC to HW control mode as
>>>>>>>>>> part of GDSC enable itself, instead of that use HW_CTRL_TRIGGER flag to
>>>>>>>>>> give consumer drivers more control and switch the GDSC mode as and when
>>>>>>>>>> required.
>>>>>>>>>>
>>>>>>>>>> HW_CTRL_TRIGGER flag allows consumer drivers to switch the video GDSC to
>>>>>>>>>> HW/SW control modes at runtime using dev_pm_genpd_set_hwmode API.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>>>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>>>>>>>>> ---
>>>>>>>>> Do we have any use for the HW_CTRL flag?
>>>>>>>>>
>>>>>>>>> Perhaps it should be renamed to HW_CTRL_ALWAYS?
>>>>>>>>>
>>>>>>>>> Or even better, *if and only if* that is necessary, add a common
>>>>>>>>> property like "always_hw_managed" to the genpd code?
>>>>>>>>
>>>>>>>> The HW_CTRL flag is still needed for the consumers that expect the GDSC
>>>>>>>> to be have the HW control bit set right after it gets enabled.
>>>>>>> Guess the correct question here would be.. Are there any?
>>>>>>>
>>>>>>
>>>>>> Yes, Display GDSC(mdss_gdsc) is required to be controlled always in HW control mode when it is enabled.
>>>>> Oh really?
>>>>>
>>>>> Looking at msm-5.10 techpack, only the SDE RSC driver seems to
>>>>> trigger regulator fast mode (so, enabling gdsc hw_ctrl on downstream).
>>>>>
>>>>
>>>> Yes, on downstream, display GDSC has only one consumer(SDE RSC driver) and there are no other consumers. SDE RSC driver switches the GDSC to hw control mode once GDSC is enabled and leaves it in hw control mode. Thanks!
>>> Sorry for pulling your tongue here a bit, but would it only concern
>>> RPMh SoCs? Designs like SM6115 don't implement RSCs, should they not
>>> have HW_CTRL enabled at all times?
>>>
>>
>> Yes, for RPMh SoCs which have display RSC block, GDSC is switched to HW control mode. For SoCs which doesn't have display RSC block, display driver controls the GDSC in SW mode on downstream. Thanks!
> Thanks for explaining!
> 
> One last question, I promise.. Should we switch the MDSS GDSC to
> HW_CTRL mode only after we start controlling the DISP RSC from Linux,
> or should it be done regardless (because of the RPMh solving algos)?
> 

 From GDSC driver, MDSS GDSC can be switched to HW_CTRL mode regardless. 
Thanks!

Regards,
Jagadeesh

> Konrad
diff mbox series

Patch

diff --git a/drivers/clk/qcom/videocc-sc7180.c b/drivers/clk/qcom/videocc-sc7180.c
index 5b9b54f616b8..51439f7ba70c 100644
--- a/drivers/clk/qcom/videocc-sc7180.c
+++ b/drivers/clk/qcom/videocc-sc7180.c
@@ -166,7 +166,7 @@  static struct gdsc vcodec0_gdsc = {
 	.pd = {
 		.name = "vcodec0_gdsc",
 	},
-	.flags = HW_CTRL,
+	.flags = HW_CTRL_TRIGGER,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 
diff --git a/drivers/clk/qcom/videocc-sc7280.c b/drivers/clk/qcom/videocc-sc7280.c
index 615695d82319..3d07b1e95986 100644
--- a/drivers/clk/qcom/videocc-sc7280.c
+++ b/drivers/clk/qcom/videocc-sc7280.c
@@ -236,7 +236,7 @@  static struct gdsc mvs0_gdsc = {
 		.name = "mvs0_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
-	.flags = HW_CTRL | RETAIN_FF_ENABLE,
+	.flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc mvsc_gdsc = {
diff --git a/drivers/clk/qcom/videocc-sdm845.c b/drivers/clk/qcom/videocc-sdm845.c
index c77a4dd5d39c..dad011c48973 100644
--- a/drivers/clk/qcom/videocc-sdm845.c
+++ b/drivers/clk/qcom/videocc-sdm845.c
@@ -260,7 +260,7 @@  static struct gdsc vcodec0_gdsc = {
 	},
 	.cxcs = (unsigned int []){ 0x890, 0x930 },
 	.cxc_count = 2,
-	.flags = HW_CTRL | POLL_CFG_GDSCR,
+	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 
@@ -271,7 +271,7 @@  static struct gdsc vcodec1_gdsc = {
 	},
 	.cxcs = (unsigned int []){ 0x8d0, 0x950 },
 	.cxc_count = 2,
-	.flags = HW_CTRL | POLL_CFG_GDSCR,
+	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 
diff --git a/drivers/clk/qcom/videocc-sm8250.c b/drivers/clk/qcom/videocc-sm8250.c
index ad46c4014a40..c1b73d852f1c 100644
--- a/drivers/clk/qcom/videocc-sm8250.c
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -293,7 +293,7 @@  static struct gdsc mvs0_gdsc = {
 	.pd = {
 		.name = "mvs0_gdsc",
 	},
-	.flags = HW_CTRL,
+	.flags = HW_CTRL_TRIGGER,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 
@@ -302,7 +302,7 @@  static struct gdsc mvs1_gdsc = {
 	.pd = {
 		.name = "mvs1_gdsc",
 	},
-	.flags = HW_CTRL,
+	.flags = HW_CTRL_TRIGGER,
 	.pwrsts = PWRSTS_OFF_ON,
 };