Re: [PATCH 5/8] media: iris: Move vpu register defines to common header file

Vikash Garodia
2025-10-16T18:37:11Z

On 10/16/2025 7:17 PM, Dmitry Baryshkov wrote:
>> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_register_defines.h b/drivers/media/platform/qcom/iris/iris_vpu_register_defines.h
>> index fe8a39e5e5a3fc68dc3a706ffdba07a5558163cf..6474f561c8dc29d1975bb44792595d86f16b6cff 100644
>> --- a/drivers/media/platform/qcom/iris/iris_vpu_register_defines.h
>> +++ b/drivers/media/platform/qcom/iris/iris_vpu_register_defines.h
>> @@ -9,9 +9,38 @@
>> #define VCODEC_BASE_OFFS 0x00000000
>> #define CPU_BASE_OFFS 0x000A0000
>> #define WRAPPER_BASE_OFFS 0x000B0000
>> +#define AON_BASE_OFFS 0x000E0000
>> +#define WRAPPER_TZ_BASE_OFFS 0x000C0000
>> +#define AON_MVP_NOC_RESET 0x0001F000
>>
>> #define CPU_CS_BASE_OFFS (CPU_BASE_OFFS)
>>
>> #define WRAPPER_CORE_POWER_STATUS (WRAPPER_BASE_OFFS + 0x80)
>> +#define WRAPPER_CORE_CLOCK_CONFIG (WRAPPER_BASE_OFFS + 0x88)
>> +#define AON_WRAPPER_MVP_NOC_LPI_CONTROL (AON_BASE_OFFS)
>> +#define WRAPPER_DEBUG_BRIDGE_LPI_CONTROL (WRAPPER_BASE_OFFS + 0x54)
>> +#define WRAPPER_DEBUG_BRIDGE_LPI_STATUS (WRAPPER_BASE_OFFS + 0x58)
>> +#define WRAPPER_IRIS_CPU_NOC_LPI_CONTROL (WRAPPER_BASE_OFFS + 0x5C)
>> +#define WRAPPER_IRIS_CPU_NOC_LPI_STATUS (WRAPPER_BASE_OFFS + 0x60)
>> +#define WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG (WRAPPER_TZ_BASE_OFFS + 0x14)
>> +#define CPU_CS_AHB_BRIDGE_SYNC_RESET (CPU_CS_BASE_OFFS + 0x160)
>> +#define CPU_CS_X2RPMH (CPU_CS_BASE_OFFS + 0x168)
>> +#define AON_WRAPPER_MVP_NOC_RESET_REQ (AON_MVP_NOC_RESET + 0x000)
>> +#define AON_WRAPPER_MVP_NOC_RESET_ACK (AON_MVP_NOC_RESET + 0x004)
>> +#define VCODEC_SS_IDLE_STATUSN (VCODEC_BASE_OFFS + 0x70)
>> +#define AON_WRAPPER_MVP_NOC_LPI_STATUS (AON_BASE_OFFS + 0x4)
>> +#define WRAPPER_TZ_QNS4PDXFIFO_RESET (WRAPPER_TZ_BASE_OFFS + 0x18)
> Registers here got totally unsorted (they were in the original source
> file). Seeing this makes me sad.
>

Sure, i will be improving this part in v2.

>> +
>> +#define CORE_BRIDGE_SW_RESET BIT(0)
>> +#define CORE_BRIDGE_HW_RESET_DISABLE BIT(1)
>> +#define MSK_SIGNAL_FROM_TENSILICA BIT(0)
>> +#define MSK_CORE_POWER_ON BIT(1)
>> +#define CTL_AXI_CLK_HALT BIT(0)
>> +#define CTL_CLK_HALT BIT(1)
>> +#define REQ_POWER_DOWN_PREP BIT(0)
>> +#define RESET_HIGH BIT(0)
>> +#define NOC_LPI_STATUS_DONE BIT(0) /* Indicates the NOC handshake is complete */
>> +#define NOC_LPI_STATUS_DENY BIT(1) /* Indicates the NOC handshake is denied */
>> +#define NOC_LPI_STATUS_ACTIVE BIT(2) /* Indicates the NOC is active */
> Ugh. This mixed all the bits, loosing connection between the register
> and the corresponding bits. I'm going to pick up this patch into the
> sc7280 series and I will improve it there, keeping the link between
> registers and bit fields.
>

Ok, not updating this part in the next revision of my series. Do you mean
something like

#define CORE_BRIDGE_SW_RESET_BIT0 BIT(0)
#define CORE_BRIDGE_HW_RESET_DISABLE_BIT1 BIT(1)

Regards,
Vikash