Re: [PATCH v5 5/8] clk: qcom: gcc-msm8939: mark Venus core GDSCs as hardware controlled

[Bryan O'Donoghue <bod@kernel.org>]

On 07/05/2026 09:32, Erikas Bitovtas wrote:
> Since in downstream kernel VENUS_CORE0_GDSC and VENUS_CORE1_GDSC have a
> device tree property "qcom,supports-hw-trigger", add a HW_CTRL flag
> to these GDSCs to indicate that they are hardware controlled.

This is a misreading/misunderstanding - supports-hw-trigger means the 
GDSC is capable of being configured this way - not that it _must_ be 
configured this way.

Which leads me back to my previous point/feedback to you in the previous 
cycle.

If you _don't_ apply this patch, do you get different behaviour when 
trying to switch on the encoder ?

> Venus core clock cannot be enabled if Venus core GDSCs are switched off.

True.

> But since they are hardware controlled

False. Your patch makes it so.

The question is, is that change actually required and/or beneficial here ?

  they can be switched off at
> any moment. Vote for the Venus core clock to enable it later when GDSCs
> get turned on.


> 
> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
> ---
>   drivers/clk/qcom/gcc-msm8939.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gcc-msm8939.c b/drivers/clk/qcom/gcc-msm8939.c
> index 45193b3d714b..420997b00ae0 100644
> --- a/drivers/clk/qcom/gcc-msm8939.c
> +++ b/drivers/clk/qcom/gcc-msm8939.c
> @@ -3664,6 +3664,7 @@ static struct clk_branch gcc_venus0_vcodec0_clk = {
> 
>   static struct clk_branch gcc_venus0_core0_vcodec0_clk = {
>   	.halt_reg = 0x4c02c,
> +	.halt_check = BRANCH_HALT_SKIP,
>   	.clkr = {
>   		.enable_reg = 0x4c02c,
>   		.enable_mask = BIT(0),
> @@ -3681,6 +3682,7 @@ static struct clk_branch gcc_venus0_core0_vcodec0_clk = {
> 
>   static struct clk_branch gcc_venus0_core1_vcodec0_clk = {
>   	.halt_reg = 0x4c034,
> +	.halt_check = BRANCH_HALT_SKIP,
>   	.clkr = {
>   		.enable_reg = 0x4c034,
>   		.enable_mask = BIT(0),
> @@ -3753,6 +3755,7 @@ static struct gdsc venus_core0_gdsc = {
>   	.pd = {
>   		.name = "venus_core0",
>   	},
> +	.flags = HW_CTRL,
>   	.pwrsts = PWRSTS_OFF_ON,
>   };
> 
> @@ -3761,6 +3764,7 @@ static struct gdsc venus_core1_gdsc = {
>   	.pd = {
>   		.name = "venus_core1",
>   	},
> +	.flags = HW_CTRL,
>   	.pwrsts = PWRSTS_OFF_ON,
>   };
> 
> 
> --
> 2.54.0
>