Re: [PATCH v5 10/14] media: iris: Add power sequence for Glymur

[Vishnu Reddy <busanna.reddy@oss.qualcomm.com>]

On 5/13/2026 7:19 PM, Dmitry Baryshkov wrote:
> On Mon, May 11, 2026 at 09:42:01PM +0530, Vishnu Reddy wrote:
>> On 5/9/2026 2:24 AM, Dmitry Baryshkov wrote:
>>> On Sat, May 09, 2026 at 12:29:59AM +0530, Vishnu Reddy wrote:
>>>> Glymur has a secondary video codec core (vcodec1), equivalent to the
>>>> primary core (vcodec0), but with independent power domains, clocks,
>>>> and reset lines. Reuse the existing code wherever possible and add
>>>> power sequence for vcodec1.
>>>>
>>>> Reviewed-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
>>>> Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>>>> ---
>>>>  .../platform/qcom/iris/iris_platform_common.h      |   4 +
>>>>  drivers/media/platform/qcom/iris/iris_vpu3x.c      | 141 ++++++++++++++++++++-
>>>>  drivers/media/platform/qcom/iris/iris_vpu_common.h |   1 +
>>>>  .../platform/qcom/iris/iris_vpu_register_defines.h |  10 ++
>>>>  4 files changed, 154 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> index 7d59e6364e9d..8995136ad29e 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> @@ -61,6 +61,9 @@ enum platform_clk_type {
>>>>  	IRIS_VPP0_HW_CLK,
>>>>  	IRIS_VPP1_HW_CLK,
>>>>  	IRIS_APV_HW_CLK,
>>>> +	IRIS_AXI_VCODEC1_CLK,
>>>> +	IRIS_VCODEC1_CLK,
>>>> +	IRIS_VCODEC1_FREERUN_CLK,
>>> I think I have asked the same question while reviewing some other code.
>>> When seeing such enums my expectation would be that the set of clocks is
>>> more or less generic, while the platform specifics should be
>>> encapsulated in platform-specific code. Instead these lists keep on
>>> growing to accomodate platform details.
>>>
>>> Can we stop that tradition? Adding a peculiarity of the platform should
>>> not require touching of the generic code.
>>>
>>>>  };
>>>>  
>>>>  struct platform_clk_data {
>>>> @@ -210,6 +213,7 @@ enum platform_pm_domain_type {
>>>>  	IRIS_VPP0_HW_POWER_DOMAIN,
>>>>  	IRIS_VPP1_HW_POWER_DOMAIN,
>>>>  	IRIS_APV_HW_POWER_DOMAIN,
>>>> +	IRIS_VCODEC1_POWER_DOMAIN,
>>>>  };
>>>>  
>>>>  struct platform_pd_data {
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
>>>> index 13fbb21c2182..ff90c375e805 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
>>>> +++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
>>>> @@ -22,9 +22,19 @@ static bool iris_vpu3x_hw_power_collapsed(struct iris_core *core)
>>>>  	u32 value, pwr_status;
>>>>  
>>>>  	value = readl(core->reg_base + WRAPPER_CORE_POWER_STATUS);
>>>> -	pwr_status = value & BIT(1);
>>>> +	pwr_status = value & VCODEC0_POWER_STATUS;
>>>>  
>>>> -	return pwr_status ? false : true;
>>>> +	return !pwr_status;
>>>> +}
>>>> +
>>>> +static bool iris_vpu36_hw1_power_collapsed(struct iris_core *core)
>>>> +{
>>>> +	u32 value, pwr_status;
>>>> +
>>>> +	value = readl(core->reg_base + WRAPPER_CORE_POWER_STATUS);
>>>> +	pwr_status = value & VCODEC1_POWER_STATUS;
>>>> +
>>>> +	return !pwr_status;
>>> Add core as an argument to the function instead of c&p'ing it.
>> Ack, will pass the vcodecx_power_status bit to this function.
> u32 core, please.

core is already used as a struct iris_core * pointer in this function. I'll
use a different name like pwr_status_bit for the u32 argument instead.

>>>>  }
>>>>  
>>>>  static void iris_vpu3_power_off_hardware(struct iris_core *core)
>>>> @@ -254,6 +264,124 @@ static void iris_vpu35_power_off_hw(struct iris_core *core)
>>>>  	iris_disable_unprepare_clock(core, IRIS_AXI_VCODEC_CLK);
>>>>  }
>>>>  
>>>> +static int iris_vpu36_power_on_hw1(struct iris_core *core)
>>> Hmmm... And if 3.7 gets 4 cores, will we have 4 copies of the function?
>> As of now, none of the near upcoming targets introduce a significantly higher
>> number of cores. If that changes in the future, we can revisit and optimize it
>> then.
> Okay....
>