Re: [PATCH RFC v3 07/11] clk: qcom: gcc-msm8939: mark Venus core GDSCs as hardware controlled

[Erikas Bitovtas]

On 4/29/26 12:18 PM, Konrad Dybcio wrote:
> On 4/29/26 6:14 AM, Bryan O'Donoghue wrote:
>> On 27/04/2026 18:58, 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.
>>>
>>> Venus core clock cannot be enabled if Venus core GDSCs are switched off.
>>> But since they are hardware controlled, 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@xxxxxxxxx>
>>> ---
>>>   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
>>>
>>
>> The downstream opts to put the GDSC under hw control, which is not the same thing as it being under hw control, its up to you to put it under hw control.
>>
>> So you might want to be more conservative especially given you have a problem getting the encoder and decoder to run simultaneously - I might try parking this patch and then see what happens.
> 
> i.e., Bryan is asking you to replace HW_CTRL with HW_CTRL_TRIGGER
> 
I tried to revert the patch and replace HW_CTRL with HW_CTRL_TRIGGER -
both result in power collapse fails.> Konrad