Re: [PATCH 08/11] media: iris: Add power sequence for Glymur

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

On 4/14/2026 3:19 PM, Konrad Dybcio wrote:
> On 4/14/26 7:00 AM, Vishnu Reddy wrote:
>> Add power sequence hooks for controller, vcodec and vcodec1. reuse the
>> existing code where ever is possible. add vcodec1 power on and off code
>> separately which has different power domains and clocks.
>>
>> Signed-off-by: Vishnu Reddy<busanna.reddy@oss.qualcomm.com>
>> ---
>>   .../platform/qcom/iris/iris_platform_common.h      |   9 ++
>>   drivers/media/platform/qcom/iris/iris_vpu3x.c      | 123 +++++++++++++++++++++
>>   drivers/media/platform/qcom/iris/iris_vpu_common.h |   1 +
>>   .../platform/qcom/iris/iris_vpu_register_defines.h |   7 ++
>>   4 files changed, 140 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> index 30e9d4d288c6..e3c1aff770dd 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,
>>   };
>>   
>>   struct platform_clk_data {
>> @@ -208,6 +211,12 @@ enum platform_pm_domain_type {
>>   	IRIS_CTRL_POWER_DOMAIN,
>>   	IRIS_VCODEC_POWER_DOMAIN,
>>   	IRIS_VPP0_HW_POWER_DOMAIN,
>> +	/*
>> +	 * On Glymur, vcodec1 power domain is at the same index in pd_devs[]
>> +	 * as IRIS_VPP0_HW_POWER_DOMAIN. Alias it so that the Glymur power
>> +	 * domain table is indexed correctly.
>> +	 */
>> +	IRIS_VCODEC1_POWER_DOMAIN = IRIS_VPP0_HW_POWER_DOMAIN,
> This feels really fragile..

I'm thinking to add wrapper and use the power domain name to find the 
index from the
platform data instead of using the enum values. I'll try this if that works.

> [...]
>
>> +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 & BIT(4);
>> +
>> +	return pwr_status ? false : true;
> return !pwr_status

Ack.

Thanks,
Vishnu Reddy.

> Konrad