[PATCH 0/4] Add runtime PM support for videocc on SM8150

[PATCH 0/4] Add runtime PM support for videocc on SM8150

[Satya Priya Kakitapalli]

Add runtime support for videocc on SM8150 and update the resets
and video_pll0_config configuration.

Satya Priya Kakitapalli (4):
  dt-bindings: clock: Update the videocc resets for sm8150
  clk: qcom: videocc-sm8150: Update the videocc resets
  clk: qcom: videocc-sm8150: Update the pll0 config
  clk: qcom: videocc-sm8150: Add runtime PM support

 drivers/clk/qcom/videocc-sm8150.c             | 26 +++++++++++++++++--
 .../dt-bindings/clock/qcom,videocc-sm8150.h   |  4 +++
 2 files changed, 28 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 1/4] dt-bindings: clock: Update the videocc resets for sm8150

[Satya Priya Kakitapalli]

Add all the available resets for the video clock controller
on sm8150.

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
 include/dt-bindings/clock/qcom,videocc-sm8150.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,videocc-sm8150.h b/include/dt-bindings/clock/qcom,videocc-sm8150.h
index e24ee840cfdb..c557b78dc572 100644
--- a/include/dt-bindings/clock/qcom,videocc-sm8150.h
+++ b/include/dt-bindings/clock/qcom,videocc-sm8150.h
@@ -16,6 +16,10 @@
 
 /* VIDEO_CC Resets */
 #define VIDEO_CC_MVSC_CORE_CLK_BCR	0
+#define VIDEO_CC_INTERFACE_BCR		1
+#define VIDEO_CC_MVS0_BCR		2
+#define VIDEO_CC_MVS1_BCR		3
+#define VIDEO_CC_MVSC_BCR		4
 
 /* VIDEO_CC GDSCRs */
 #define VENUS_GDSC			0
-- 
2.25.1

Re: [PATCH 1/4] dt-bindings: clock: Update the videocc resets for sm8150

[Krzysztof Kozlowski]

On 10/11/2023 07:50, Satya Priya Kakitapalli wrote:
> Add all the available resets for the video clock controller
> on sm8150.
> 
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH 2/4] clk: qcom: videocc-sm8150: Update the videocc resets

[Satya Priya Kakitapalli]

Add all the available resets for the video clock controller
on sm8150.

Fixes: 5658e8cf1a8a ("clk: qcom: add video clock controller driver for SM8150")
Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
 drivers/clk/qcom/videocc-sm8150.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
index 1afdbe4a249d..6a5f89f53da8 100644
--- a/drivers/clk/qcom/videocc-sm8150.c
+++ b/drivers/clk/qcom/videocc-sm8150.c
@@ -214,6 +214,10 @@ static const struct regmap_config video_cc_sm8150_regmap_config = {
 
 static const struct qcom_reset_map video_cc_sm8150_resets[] = {
 	[VIDEO_CC_MVSC_CORE_CLK_BCR] = { 0x850, 2 },
+	[VIDEO_CC_INTERFACE_BCR] = { 0x8f0 },
+	[VIDEO_CC_MVS0_BCR] = { 0x870 },
+	[VIDEO_CC_MVS1_BCR] = { 0x8b0 },
+	[VIDEO_CC_MVSC_BCR] = { 0x810 },
 };
 
 static const struct qcom_cc_desc video_cc_sm8150_desc = {
-- 
2.25.1

Re: [PATCH 2/4] clk: qcom: videocc-sm8150: Update the videocc resets

[Bryan O'Donoghue]

On 10/11/2023 06:50, Satya Priya Kakitapalli wrote:
> Add all the available resets for the video clock controller
> on sm8150.
> 
> Fixes: 5658e8cf1a8a ("clk: qcom: add video clock controller driver for SM8150")
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---
>   drivers/clk/qcom/videocc-sm8150.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
> index 1afdbe4a249d..6a5f89f53da8 100644
> --- a/drivers/clk/qcom/videocc-sm8150.c
> +++ b/drivers/clk/qcom/videocc-sm8150.c
> @@ -214,6 +214,10 @@ static const struct regmap_config video_cc_sm8150_regmap_config = {
>   
>   static const struct qcom_reset_map video_cc_sm8150_resets[] = {
>   	[VIDEO_CC_MVSC_CORE_CLK_BCR] = { 0x850, 2 },
> +	[VIDEO_CC_INTERFACE_BCR] = { 0x8f0 },
> +	[VIDEO_CC_MVS0_BCR] = { 0x870 },
> +	[VIDEO_CC_MVS1_BCR] = { 0x8b0 },
> +	[VIDEO_CC_MVSC_BCR] = { 0x810 },

Are you sure this shouldn't be

+	[VIDEO_CC_INTERFACE_BCR] = { 0x9ac },
+	[VIDEO_CC_MVS0_BCR] = { 0x870 },
+	[VIDEO_CC_MVS1_BCR] = { 0x8b0 },
+	[VIDEO_CC_MVSC_BCR] = { 0x810 },

?

---
bod

Re: [PATCH 2/4] clk: qcom: videocc-sm8150: Update the videocc resets

[Konrad Dybcio]

On 11/10/23 12:49, Bryan O'Donoghue wrote:
> On 10/11/2023 06:50, Satya Priya Kakitapalli wrote:
>> Add all the available resets for the video clock controller
>> on sm8150.
>>
>> Fixes: 5658e8cf1a8a ("clk: qcom: add video clock controller driver for SM8150")
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> ---
>>   drivers/clk/qcom/videocc-sm8150.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
>> index 1afdbe4a249d..6a5f89f53da8 100644
>> --- a/drivers/clk/qcom/videocc-sm8150.c
>> +++ b/drivers/clk/qcom/videocc-sm8150.c
>> @@ -214,6 +214,10 @@ static const struct regmap_config video_cc_sm8150_regmap_config = {
>>   static const struct qcom_reset_map video_cc_sm8150_resets[] = {
>>       [VIDEO_CC_MVSC_CORE_CLK_BCR] = { 0x850, 2 },
>> +    [VIDEO_CC_INTERFACE_BCR] = { 0x8f0 },
>> +    [VIDEO_CC_MVS0_BCR] = { 0x870 },
>> +    [VIDEO_CC_MVS1_BCR] = { 0x8b0 },
>> +    [VIDEO_CC_MVSC_BCR] = { 0x810 },
> 
> Are you sure this shouldn't be
> 
> +    [VIDEO_CC_INTERFACE_BCR] = { 0x9ac },
> +    [VIDEO_CC_MVS0_BCR] = { 0x870 },
> +    [VIDEO_CC_MVS1_BCR] = { 0x8b0 },
> +    [VIDEO_CC_MVSC_BCR] = { 0x810 },
FWIW this seems to be a copypaste from

https://git.codelinaro.org/clo/la/kernel/msm-5.4/-/blame/92b31370d31d22e910120f6a875bf0919b3f1773/drivers/clk/qcom/videocc-sm8150.c

so if it's an issue, it should probably be fixed downstream too

Konrad

Re: [PATCH 2/4] clk: qcom: videocc-sm8150: Update the videocc resets

[Bryan O'Donoghue]

On 15/11/2023 16:48, Konrad Dybcio wrote:
>> +    [VIDEO_CC_INTERFACE_BCR] = { 0x9ac },
>> +    [VIDEO_CC_MVS0_BCR] = { 0x870 },
>> +    [VIDEO_CC_MVS1_BCR] = { 0x8b0 },
>> +    [VIDEO_CC_MVSC_BCR] = { 0x810 },
> FWIW this seems to be a copypaste from
> 
> https://git.codelinaro.org/clo/la/kernel/msm-5.4/-/blame/92b31370d31d22e910120f6a875bf0919b3f1773/drivers/clk/qcom/videocc-sm8150.c
> 
> so if it's an issue, it should probably be fixed downstream too

More of a question than a gotcha - 0x9ac is valid for sm8250 so curious 
to me that its a different address on sm8150.

---
bod

Re: [PATCH 2/4] clk: qcom: videocc-sm8150: Update the videocc resets

[Satya Priya Kakitapalli (Temp)]

On 11/15/2023 10:39 PM, Bryan O'Donoghue wrote:
> On 15/11/2023 16:48, Konrad Dybcio wrote:
>>> +    [VIDEO_CC_INTERFACE_BCR] = { 0x9ac },
>>> +    [VIDEO_CC_MVS0_BCR] = { 0x870 },
>>> +    [VIDEO_CC_MVS1_BCR] = { 0x8b0 },
>>> +    [VIDEO_CC_MVSC_BCR] = { 0x810 },
>> FWIW this seems to be a copypaste from
>>
>> https://git.codelinaro.org/clo/la/kernel/msm-5.4/-/blame/92b31370d31d22e910120f6a875bf0919b3f1773/drivers/clk/qcom/videocc-sm8150.c 
>>
>>
>> so if it's an issue, it should probably be fixed downstream too
>
> More of a question than a gotcha - 0x9ac is valid for sm8250 so 
> curious to me that its a different address on sm8150.


I have re-checked the offset values, as per the HW plan for SM8150 below 
values are correct:

+    [VIDEO_CC_INTERFACE_BCR] = { 0x8f0 },
+    [VIDEO_CC_MVS0_BCR] = { 0x870 },
+    [VIDEO_CC_MVS1_BCR] = { 0x8b0 },
+    [VIDEO_CC_MVSC_BCR] = { 0x810 },


> ---
> bod

Re: [PATCH 2/4] clk: qcom: videocc-sm8150: Update the videocc resets

[Bryan O'Donoghue]

On 10/11/2023 06:50, Satya Priya Kakitapalli wrote:
> Add all the available resets for the video clock controller
> on sm8150.
> 
> Fixes: 5658e8cf1a8a ("clk: qcom: add video clock controller driver for SM8150")
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---
>   drivers/clk/qcom/videocc-sm8150.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
> index 1afdbe4a249d..6a5f89f53da8 100644
> --- a/drivers/clk/qcom/videocc-sm8150.c
> +++ b/drivers/clk/qcom/videocc-sm8150.c
> @@ -214,6 +214,10 @@ static const struct regmap_config video_cc_sm8150_regmap_config = {
>   
>   static const struct qcom_reset_map video_cc_sm8150_resets[] = {
>   	[VIDEO_CC_MVSC_CORE_CLK_BCR] = { 0x850, 2 },
> +	[VIDEO_CC_INTERFACE_BCR] = { 0x8f0 },
> +	[VIDEO_CC_MVS0_BCR] = { 0x870 },
> +	[VIDEO_CC_MVS1_BCR] = { 0x8b0 },
> +	[VIDEO_CC_MVSC_BCR] = { 0x810 },
>   };
>   
>   static const struct qcom_cc_desc video_cc_sm8150_desc = {

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

[PATCH 3/4] clk: qcom: videocc-sm8150: Update the video_pll0_config

[Satya Priya Kakitapalli]

Update the video_pll0_config to configure the test_ctl_hi and
test_ctl_hi1 values.

Fixes: 5658e8cf1a8a ("clk: qcom: add video clock controller driver for SM8150")
Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
 drivers/clk/qcom/videocc-sm8150.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
index 6a5f89f53da8..ddfe784fb9e1 100644
--- a/drivers/clk/qcom/videocc-sm8150.c
+++ b/drivers/clk/qcom/videocc-sm8150.c
@@ -33,6 +33,8 @@ static struct alpha_pll_config video_pll0_config = {
 	.config_ctl_val = 0x20485699,
 	.config_ctl_hi_val = 0x00002267,
 	.config_ctl_hi1_val = 0x00000024,
+	.test_ctl_hi_val = 0x00000002,
+	.test_ctl_hi1_val = 0x00000020,
 	.user_ctl_val = 0x00000000,
 	.user_ctl_hi_val = 0x00000805,
 	.user_ctl_hi1_val = 0x000000D0,
-- 
2.25.1

Re: [PATCH 3/4] clk: qcom: videocc-sm8150: Update the video_pll0_config

[Bryan O'Donoghue]

On 10/11/2023 06:50, Satya Priya Kakitapalli wrote:
> Update the video_pll0_config to configure the test_ctl_hi and
> test_ctl_hi1 values.
> 
> Fixes: 5658e8cf1a8a ("clk: qcom: add video clock controller driver for SM8150")
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---
>   drivers/clk/qcom/videocc-sm8150.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
> index 6a5f89f53da8..ddfe784fb9e1 100644
> --- a/drivers/clk/qcom/videocc-sm8150.c
> +++ b/drivers/clk/qcom/videocc-sm8150.c
> @@ -33,6 +33,8 @@ static struct alpha_pll_config video_pll0_config = {
>   	.config_ctl_val = 0x20485699,
>   	.config_ctl_hi_val = 0x00002267,
>   	.config_ctl_hi1_val = 0x00000024,
> +	.test_ctl_hi_val = 0x00000002,
> +	.test_ctl_hi1_val = 0x00000020,
>   	.user_ctl_val = 0x00000000,
>   	.user_ctl_hi_val = 0x00000805,
>   	.user_ctl_hi1_val = 0x000000D0,

It would be nice to get a better description of this change.

Reading this - I can see it does an update but, the commit log should 
contain what it is updating and why plus what effect the update will have.

Please update the commit log to capture that data and +cc me on the V2.

---
bod

Re: [PATCH 3/4] clk: qcom: videocc-sm8150: Update the video_pll0_config

[Satya Priya Kakitapalli (Temp)]

On 11/10/2023 4:46 PM, Bryan O'Donoghue wrote:
> On 10/11/2023 06:50, Satya Priya Kakitapalli wrote:
>> Update the video_pll0_config to configure the test_ctl_hi and
>> test_ctl_hi1 values.
>>
>> Fixes: 5658e8cf1a8a ("clk: qcom: add video clock controller driver 
>> for SM8150")
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> ---
>>   drivers/clk/qcom/videocc-sm8150.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/videocc-sm8150.c 
>> b/drivers/clk/qcom/videocc-sm8150.c
>> index 6a5f89f53da8..ddfe784fb9e1 100644
>> --- a/drivers/clk/qcom/videocc-sm8150.c
>> +++ b/drivers/clk/qcom/videocc-sm8150.c
>> @@ -33,6 +33,8 @@ static struct alpha_pll_config video_pll0_config = {
>>       .config_ctl_val = 0x20485699,
>>       .config_ctl_hi_val = 0x00002267,
>>       .config_ctl_hi1_val = 0x00000024,
>> +    .test_ctl_hi_val = 0x00000002,
>> +    .test_ctl_hi1_val = 0x00000020,
>>       .user_ctl_val = 0x00000000,
>>       .user_ctl_hi_val = 0x00000805,
>>       .user_ctl_hi1_val = 0x000000D0,
>
> It would be nice to get a better description of this change.
>
> Reading this - I can see it does an update but, the commit log should 
> contain what it is updating and why plus what effect the update will 
> have.
>
> Please update the commit log to capture that data and +cc me on the V2.
>

Sure will check and update.


Thanks,

Satya Priya

> ---
> bod

Re: [PATCH 3/4] clk: qcom: videocc-sm8150: Update the video_pll0_config

[Konrad Dybcio]

On 10.11.2023 07:50, Satya Priya Kakitapalli wrote:
> Update the video_pll0_config to configure the test_ctl_hi and
> test_ctl_hi1 values.
> 
> Fixes: 5658e8cf1a8a ("clk: qcom: add video clock controller driver for SM8150")
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---
These values are not present on LA.UM.7.1.r1. Can you confirm internally
that they're valid?

Konrad

Re: [PATCH 3/4] clk: qcom: videocc-sm8150: Update the video_pll0_config

[Satya Priya Kakitapalli (Temp)]

On 11/13/2023 5:37 PM, Konrad Dybcio wrote:
> On 10.11.2023 07:50, Satya Priya Kakitapalli wrote:
>> Update the video_pll0_config to configure the test_ctl_hi and
>> test_ctl_hi1 values.
>>
>> Fixes: 5658e8cf1a8a ("clk: qcom: add video clock controller driver for SM8150")
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> ---
> These values are not present on LA.UM.7.1.r1. Can you confirm internally
> that they're valid?


Sure, I'll re-check the values and update if needed while posting v2.


> Konrad

[PATCH 4/4] clk: qcom: videocc-sm8150: Add runtime PM support

[Satya Priya Kakitapalli]

Add runtime PM support to ensure the supply rails are enabled
when necessary.

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
 drivers/clk/qcom/videocc-sm8150.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
index ddfe784fb9e1..e56dfcea05c6 100644
--- a/drivers/clk/qcom/videocc-sm8150.c
+++ b/drivers/clk/qcom/videocc-sm8150.c
@@ -6,6 +6,7 @@
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,videocc-sm8150.h>
@@ -241,17 +242,32 @@ MODULE_DEVICE_TABLE(of, video_cc_sm8150_match_table);
 static int video_cc_sm8150_probe(struct platform_device *pdev)
 {
 	struct regmap *regmap;
+	int ret;
+
+	ret = devm_pm_runtime_enable(&pdev->dev);
+	if (ret)
+		return ret;
+
+	ret = pm_runtime_resume_and_get(&pdev->dev);
+	if (ret)
+		return ret;
 
 	regmap = qcom_cc_map(pdev, &video_cc_sm8150_desc);
-	if (IS_ERR(regmap))
+	if (IS_ERR(regmap)) {
+		pm_runtime_put(&pdev->dev);
 		return PTR_ERR(regmap);
+	}
 
 	clk_trion_pll_configure(&video_pll0, regmap, &video_pll0_config);
 
 	/* Keep VIDEO_CC_XO_CLK ALWAYS-ON */
 	regmap_update_bits(regmap, 0x984, 0x1, 0x1);
 
-	return qcom_cc_really_probe(pdev, &video_cc_sm8150_desc, regmap);
+	ret = qcom_cc_really_probe(pdev, &video_cc_sm8150_desc, regmap);
+
+	pm_runtime_put(&pdev->dev);
+
+	return ret;
 }
 
 static struct platform_driver video_cc_sm8150_driver = {
-- 
2.25.1