clk: mediatek: clk-mt8195-vdo0: Set rate on vdo0_dp_intf0_dp_intf's parent

[PATCH] clk: mediatek: clk-mt8195-vdo0: Set rate on vdo0_dp_intf0_dp_intf's parent

Posted by AngeloGioacchino Del Regno 2 years, 3 months ago

Add the CLK_SET_RATE_PARENT flag to the CLK_VDO0_DP_INTF0_DP_INTF
clock: this is required to trigger clock source selection on
CLK_TOP_EDP, while avoiding to manage the enablement of the former
separately from the latter in the displayport driver.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/clk/mediatek/clk-mt8195-vdo0.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/mediatek/clk-mt8195-vdo0.c b/drivers/clk/mediatek/clk-mt8195-vdo0.c
index 261a7f76dd3c..07b46bfd5040 100644
--- a/drivers/clk/mediatek/clk-mt8195-vdo0.c
+++ b/drivers/clk/mediatek/clk-mt8195-vdo0.c
@@ -37,6 +37,10 @@ static const struct mtk_gate_regs vdo0_2_cg_regs = {
 #define GATE_VDO0_2(_id, _name, _parent, _shift)			\
 	GATE_MTK(_id, _name, _parent, &vdo0_2_cg_regs, _shift, &mtk_clk_gate_ops_setclr)
 
+#define GATE_VDO0_2_FLAGS(_id, _name, _parent, _shift, _flags)		\
+	GATE_MTK_FLAGS(_id, _name, _parent, &vdo0_2_cg_regs, _shift,	\
+		       &mtk_clk_gate_ops_setclr, _flags)
+
 static const struct mtk_gate vdo0_clks[] = {
 	/* VDO0_0 */
 	GATE_VDO0_0(CLK_VDO0_DISP_OVL0, "vdo0_disp_ovl0", "top_vpp", 0),
@@ -85,7 +89,8 @@ static const struct mtk_gate vdo0_clks[] = {
 	/* VDO0_2 */
 	GATE_VDO0_2(CLK_VDO0_DSI0_DSI, "vdo0_dsi0_dsi", "top_dsi_occ", 0),
 	GATE_VDO0_2(CLK_VDO0_DSI1_DSI, "vdo0_dsi1_dsi", "top_dsi_occ", 8),
-	GATE_VDO0_2(CLK_VDO0_DP_INTF0_DP_INTF, "vdo0_dp_intf0_dp_intf", "top_edp", 16),
+	GATE_VDO0_2_FLAGS(CLK_VDO0_DP_INTF0_DP_INTF, "vdo0_dp_intf0_dp_intf",
+			  "top_edp", 16, CLK_SET_RATE_PARENT),
 };
 
 static int clk_mt8195_vdo0_probe(struct platform_device *pdev)
-- 
2.35.1

Re: [PATCH] clk: mediatek: clk-mt8195-vdo0: Set rate on vdo0_dp_intf0_dp_intf's parent

Posted by Rex-BC Chen 2 years, 3 months ago

On Tue, 2022-06-14 at 17:10 +0800, AngeloGioacchino Del Regno wrote:
> Add the CLK_SET_RATE_PARENT flag to the CLK_VDO0_DP_INTF0_DP_INTF
> clock: this is required to trigger clock source selection on
> CLK_TOP_EDP, while avoiding to manage the enablement of the former
> separately from the latter in the displayport driver.
> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  drivers/clk/mediatek/clk-mt8195-vdo0.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt8195-vdo0.c
> b/drivers/clk/mediatek/clk-mt8195-vdo0.c
> index 261a7f76dd3c..07b46bfd5040 100644
> --- a/drivers/clk/mediatek/clk-mt8195-vdo0.c
> +++ b/drivers/clk/mediatek/clk-mt8195-vdo0.c
> @@ -37,6 +37,10 @@ static const struct mtk_gate_regs vdo0_2_cg_regs =
> {
>  #define GATE_VDO0_2(_id, _name, _parent, _shift)			
> \
>  	GATE_MTK(_id, _name, _parent, &vdo0_2_cg_regs, _shift,
> &mtk_clk_gate_ops_setclr)
>  
> +#define GATE_VDO0_2_FLAGS(_id, _name, _parent, _shift, _flags)	
> 	\
> +	GATE_MTK_FLAGS(_id, _name, _parent, &vdo0_2_cg_regs, _shift,	
> \
> +		       &mtk_clk_gate_ops_setclr, _flags)
> +
>  static const struct mtk_gate vdo0_clks[] = {
>  	/* VDO0_0 */
>  	GATE_VDO0_0(CLK_VDO0_DISP_OVL0, "vdo0_disp_ovl0", "top_vpp",
> 0),
> @@ -85,7 +89,8 @@ static const struct mtk_gate vdo0_clks[] = {
>  	/* VDO0_2 */
>  	GATE_VDO0_2(CLK_VDO0_DSI0_DSI, "vdo0_dsi0_dsi", "top_dsi_occ",
> 0),
>  	GATE_VDO0_2(CLK_VDO0_DSI1_DSI, "vdo0_dsi1_dsi", "top_dsi_occ",
> 8),
> -	GATE_VDO0_2(CLK_VDO0_DP_INTF0_DP_INTF, "vdo0_dp_intf0_dp_intf",
> "top_edp", 16),
> +	GATE_VDO0_2_FLAGS(CLK_VDO0_DP_INTF0_DP_INTF,
> "vdo0_dp_intf0_dp_intf",
> +			  "top_edp", 16, CLK_SET_RATE_PARENT),
>  };
>  
>  static int clk_mt8195_vdo0_probe(struct platform_device *pdev)
> -- 
> 2.35.1
> 

Hello Angelo,

Thanks for this patch.
Another dp clock should also be fix.
After confirming with Jitao who is our dp expert.
The parent of CLK_VDO1_DPINTF should be top_dp instead of top_vpp.

Thanks!

--- a/drivers/clk/mediatek/clk-mt8195-vdo1.c
+++ b/drivers/clk/mediatek/clk-mt8195-vdo1.c
@@ -43,6 +43,9 @@ static const struct mtk_gate_regs vdo1_3_cg_regs = {
 #define GATE_VDO1_2(_id, _name, _parent,
_shift)                       \
        GATE_MTK(_id, _name, _parent, &vdo1_2_cg_regs, _shift,
&mtk_clk_gate_ops_setclr)

+#define GATE_VDO1_2_FLAGS(_id, _name, _parent, _shift,
_flags)                 \
+       GATE_MTK_FLAGS(_id, _name, _parent, &vdo1_2_cg_regs, _shift,
&mtk_clk_gate_ops_setclr, _flags)
+
 #define GATE_VDO1_3(_id, _name, _parent,
_shift)                       \
        GATE_MTK(_id, _name, _parent, &vdo1_3_cg_regs, _shift,
&mtk_clk_gate_ops_setclr)

@@ -99,7 +102,7 @@ static const struct mtk_gate vdo1_clks[] = {
        GATE_VDO1_2(CLK_VDO1_DISP_MONITOR_DPI0,
"vdo1_disp_monitor_dpi0", "top_vpp", 1),
        GATE_VDO1_2(CLK_VDO1_DPI1, "vdo1_dpi1", "top_vpp", 8),
        GATE_VDO1_2(CLK_VDO1_DISP_MONITOR_DPI1,
"vdo1_disp_monitor_dpi1", "top_vpp", 9),
-       GATE_VDO1_2(CLK_VDO1_DPINTF, "vdo1_dpintf", "top_vpp", 16),
+       GATE_VDO1_2_FLAGS(CLK_VDO1_DPINTF, "vdo1_dpintf", "top_dp", 16,
CLK_SET_RATE_PARENT),

BRs,
Bo-Chen

Re: [PATCH] clk: mediatek: clk-mt8195-vdo0: Set rate on vdo0_dp_intf0_dp_intf's parent

Posted by AngeloGioacchino Del Regno 2 years, 3 months ago

Il 16/06/22 09:12, Rex-BC Chen ha scritto:
> On Tue, 2022-06-14 at 17:10 +0800, AngeloGioacchino Del Regno wrote:
>> Add the CLK_SET_RATE_PARENT flag to the CLK_VDO0_DP_INTF0_DP_INTF
>> clock: this is required to trigger clock source selection on
>> CLK_TOP_EDP, while avoiding to manage the enablement of the former
>> separately from the latter in the displayport driver.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <
>> angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/clk/mediatek/clk-mt8195-vdo0.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/mediatek/clk-mt8195-vdo0.c
>> b/drivers/clk/mediatek/clk-mt8195-vdo0.c
>> index 261a7f76dd3c..07b46bfd5040 100644
>> --- a/drivers/clk/mediatek/clk-mt8195-vdo0.c
>> +++ b/drivers/clk/mediatek/clk-mt8195-vdo0.c
>> @@ -37,6 +37,10 @@ static const struct mtk_gate_regs vdo0_2_cg_regs =
>> {
>>   #define GATE_VDO0_2(_id, _name, _parent, _shift)			
>> \
>>   	GATE_MTK(_id, _name, _parent, &vdo0_2_cg_regs, _shift,
>> &mtk_clk_gate_ops_setclr)
>>   
>> +#define GATE_VDO0_2_FLAGS(_id, _name, _parent, _shift, _flags)	
>> 	\
>> +	GATE_MTK_FLAGS(_id, _name, _parent, &vdo0_2_cg_regs, _shift,	
>> \
>> +		       &mtk_clk_gate_ops_setclr, _flags)
>> +
>>   static const struct mtk_gate vdo0_clks[] = {
>>   	/* VDO0_0 */
>>   	GATE_VDO0_0(CLK_VDO0_DISP_OVL0, "vdo0_disp_ovl0", "top_vpp",
>> 0),
>> @@ -85,7 +89,8 @@ static const struct mtk_gate vdo0_clks[] = {
>>   	/* VDO0_2 */
>>   	GATE_VDO0_2(CLK_VDO0_DSI0_DSI, "vdo0_dsi0_dsi", "top_dsi_occ",
>> 0),
>>   	GATE_VDO0_2(CLK_VDO0_DSI1_DSI, "vdo0_dsi1_dsi", "top_dsi_occ",
>> 8),
>> -	GATE_VDO0_2(CLK_VDO0_DP_INTF0_DP_INTF, "vdo0_dp_intf0_dp_intf",
>> "top_edp", 16),
>> +	GATE_VDO0_2_FLAGS(CLK_VDO0_DP_INTF0_DP_INTF,
>> "vdo0_dp_intf0_dp_intf",
>> +			  "top_edp", 16, CLK_SET_RATE_PARENT),
>>   };
>>   
>>   static int clk_mt8195_vdo0_probe(struct platform_device *pdev)
>> -- 
>> 2.35.1
>>
> 
> Hello Angelo,
> 
> Thanks for this patch.
> Another dp clock should also be fix.
> After confirming with Jitao who is our dp expert.
> The parent of CLK_VDO1_DPINTF should be top_dp instead of top_vpp.
> 
> Thanks!

Hey.

Yes, we should really fix that, I can send a separated series with:
* A first commit fixing the clock parent (with a Fixes: tag)
* A second commit adding the CLK_SET_RATE_PARENT

...and I'll do that ASAP.

Thank you!
Angelo

Re: [PATCH] clk: mediatek: clk-mt8195-vdo0: Set rate on vdo0_dp_intf0_dp_intf's parent

Posted by Stephen Boyd 2 years, 3 months ago

Quoting AngeloGioacchino Del Regno (2022-06-14 02:10:20)
> Add the CLK_SET_RATE_PARENT flag to the CLK_VDO0_DP_INTF0_DP_INTF
> clock: this is required to trigger clock source selection on
> CLK_TOP_EDP, while avoiding to manage the enablement of the former
> separately from the latter in the displayport driver.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---

Any Fixes tag?

Re: [PATCH] clk: mediatek: clk-mt8195-vdo0: Set rate on vdo0_dp_intf0_dp_intf's parent

Posted by AngeloGioacchino Del Regno 2 years, 3 months ago

Il 16/06/22 04:44, Stephen Boyd ha scritto:
> Quoting AngeloGioacchino Del Regno (2022-06-14 02:10:20)
>> Add the CLK_SET_RATE_PARENT flag to the CLK_VDO0_DP_INTF0_DP_INTF
>> clock: this is required to trigger clock source selection on
>> CLK_TOP_EDP, while avoiding to manage the enablement of the former
>> separately from the latter in the displayport driver.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
> 
> Any Fixes tag?

Backporting is useless because there's no DisplayPort driver that supports MT8195
in the previous kernel versions, so this clock (and whatever logic behind it) is
unused.

Though, if you think that's going to be useful in any way, I can add one?

Regards,
Angelo

Re: [PATCH] clk: mediatek: clk-mt8195-vdo0: Set rate on vdo0_dp_intf0_dp_intf's parent

Posted by Stephen Boyd 2 years, 3 months ago

Quoting AngeloGioacchino Del Regno (2022-06-16 01:48:44)
> Il 16/06/22 04:44, Stephen Boyd ha scritto:
> > Quoting AngeloGioacchino Del Regno (2022-06-14 02:10:20)
> >> Add the CLK_SET_RATE_PARENT flag to the CLK_VDO0_DP_INTF0_DP_INTF
> >> clock: this is required to trigger clock source selection on
> >> CLK_TOP_EDP, while avoiding to manage the enablement of the former
> >> separately from the latter in the displayport driver.
> >>
> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >> ---
> > 
> > Any Fixes tag?
> 
> Backporting is useless because there's no DisplayPort driver that supports MT8195
> in the previous kernel versions, so this clock (and whatever logic behind it) is
> unused.
> 
> Though, if you think that's going to be useful in any way, I can add one?
> 

It's always useful. A Fixes tag doesn't mean anything for backporting to
stable kernels.