| Message ID | 20240827-iris_v3-v3-8-c5fdbbe65e70@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Qualcomm iris video decoder driver | expand |
On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote: > From: Dikshita Agarwal <quic_dikshita@quicinc.com> > > Set memory region to firmware and implement boot sequence. > > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- > drivers/media/platform/qcom/iris/Makefile | 1 + > drivers/media/platform/qcom/iris/iris_core.c | 7 ++ > .../platform/qcom/iris/iris_platform_common.h | 1 + > .../platform/qcom/iris/iris_platform_sm8550.c | 3 + > drivers/media/platform/qcom/iris/iris_vpu_common.c | 87 ++++++++++++++++++++++ > drivers/media/platform/qcom/iris/iris_vpu_common.h | 13 ++++ > 6 files changed, 112 insertions(+) > > diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile > index ddd4c994a0b9..95f4e92fe085 100644 > --- a/drivers/media/platform/qcom/iris/Makefile > +++ b/drivers/media/platform/qcom/iris/Makefile > @@ -8,5 +8,6 @@ iris-objs += iris_core.o \ > iris_probe.o \ > iris_resources.o \ > iris_vidc.o \ > + iris_vpu_common.o \ > > obj-$(CONFIG_VIDEO_QCOM_IRIS) += iris.o > diff --git a/drivers/media/platform/qcom/iris/iris_core.c b/drivers/media/platform/qcom/iris/iris_core.c > index 8c7d53c57086..5ad66ac113ae 100644 > --- a/drivers/media/platform/qcom/iris/iris_core.c > +++ b/drivers/media/platform/qcom/iris/iris_core.c > @@ -6,6 +6,7 @@ > #include "iris_core.h" > #include "iris_firmware.h" > #include "iris_state.h" > +#include "iris_vpu_common.h" > > void iris_core_deinit(struct iris_core *core) > { > @@ -39,10 +40,16 @@ int iris_core_init(struct iris_core *core) > if (ret) > goto error_queue_deinit; > > + ret = iris_vpu_boot_firmware(core); > + if (ret) > + goto error_unload_fw; > + > mutex_unlock(&core->lock); > > return 0; > > +error_unload_fw: > + iris_fw_unload(core); > error_queue_deinit: > iris_hfi_queues_deinit(core); > error: > diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h > index 9c919367f9d7..47fdebd8135c 100644 > --- a/drivers/media/platform/qcom/iris/iris_platform_common.h > +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h > @@ -45,6 +45,7 @@ struct iris_platform_data { > const char *fwname; > u32 pas_id; > struct tz_cp_config *tz_cp_config_data; > + u32 core_arch; > }; > > #endif > diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c > index 1bb34c3e6e18..a559e095fefc 100644 > --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c > +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c > @@ -8,6 +8,8 @@ > #include "iris_platform_common.h" > #include "iris_resources.h" > > +#define VIDEO_ARCH_LX 1 > + > static const struct icc_info sm8550_icc_table[] = { > { "cpu-cfg", 1000, 1000 }, > { "video-mem", 1000, 15000000 }, > @@ -48,4 +50,5 @@ struct iris_platform_data sm8550_data = { > .fwname = "qcom/vpu/vpu30_p4.mbn", > .pas_id = IRIS_PAS_ID, > .tz_cp_config_data = &tz_cp_config_sm8550, > + .core_arch = VIDEO_ARCH_LX, > }; > diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c > new file mode 100644 > index 000000000000..df87b1b719a9 > --- /dev/null > +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c > @@ -0,0 +1,87 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/iopoll.h> > + > +#include "iris_core.h" > +#include "iris_vpu_common.h" > + > +#define CPU_BASE_OFFS 0x000A0000 > + > +#define CPU_CS_BASE_OFFS (CPU_BASE_OFFS) > + > +#define CTRL_INIT (CPU_CS_BASE_OFFS + 0x48) > +#define CTRL_STATUS (CPU_CS_BASE_OFFS + 0x4C) > + > +#define CTRL_ERROR_STATUS__M 0xfe > + > +#define QTBL_INFO (CPU_CS_BASE_OFFS + 0x50) > +#define QTBL_ADDR (CPU_CS_BASE_OFFS + 0x54) > +#define CPU_CS_SCIACMDARG3 (CPU_CS_BASE_OFFS + 0x58) > +#define SFR_ADDR (CPU_CS_BASE_OFFS + 0x5C) > +#define UC_REGION_ADDR (CPU_CS_BASE_OFFS + 0x64) > +#define UC_REGION_SIZE (CPU_CS_BASE_OFFS + 0x68) > + > +#define CPU_CS_H2XSOFTINTEN (CPU_CS_BASE_OFFS + 0x148) > +#define CPU_CS_X2RPMH (CPU_CS_BASE_OFFS + 0x168) > + > +static void iris_vpu_setup_ucregion_memory_map(struct iris_core *core) > +{ > + u32 queue_size, value; > + > + /* Iris hardware requires 4K queue alignment */ > + queue_size = ALIGN(sizeof(struct iris_hfi_queue_table_header) + > + (IFACEQ_QUEUE_SIZE * IFACEQ_NUMQ), SZ_4K); > + > + value = (u32)core->iface_q_table_daddr; > + writel(value, core->reg_base + UC_REGION_ADDR); > + > + /* Iris hardware requires 1M queue alignment */ > + value = ALIGN(SFR_SIZE + queue_size, SZ_1M); > + writel(value, core->reg_base + UC_REGION_SIZE); > + > + value = (u32)core->iface_q_table_daddr; > + writel(value, core->reg_base + QTBL_ADDR); > + > + writel(0x01, core->reg_base + QTBL_INFO); A general comment I have is instead of writing hard-coded values to registers we should define at a minimum the bit-fields we use if not the entire set of bits-fields for the register. The only exception to this is when we don't know what those values are - for example receiving a magic write sequence for a camera sensor. In this case though we have full access to enumerate the register bit-fields. Without looking at the register descriptions I guess this bit is an enable or a startup bit => #define QTBL_INFO_EN BIT(0) I'll not go through this series reiterating this comment but, it certainly applies to any bit-field/register in the same => please define at least the bits used if not the full set of bits for register writes instead of using magic numbers. --- bod
Hi Bryan, On 9/5/2024 6:04 PM, Bryan O'Donoghue wrote: > On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote: >> From: Dikshita Agarwal <quic_dikshita@quicinc.com> >> >> Set memory region to firmware and implement boot sequence. >> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> >> --- >> drivers/media/platform/qcom/iris/Makefile | 1 + >> drivers/media/platform/qcom/iris/iris_core.c | 7 ++ >> .../platform/qcom/iris/iris_platform_common.h | 1 + >> .../platform/qcom/iris/iris_platform_sm8550.c | 3 + >> drivers/media/platform/qcom/iris/iris_vpu_common.c | 87 ++++++++++++++++++++++ >> drivers/media/platform/qcom/iris/iris_vpu_common.h | 13 ++++ >> 6 files changed, 112 insertions(+) >> >> diff --git a/drivers/media/platform/qcom/iris/Makefile >> b/drivers/media/platform/qcom/iris/Makefile >> index ddd4c994a0b9..95f4e92fe085 100644 >> --- a/drivers/media/platform/qcom/iris/Makefile >> +++ b/drivers/media/platform/qcom/iris/Makefile >> @@ -8,5 +8,6 @@ iris-objs += iris_core.o \ >> iris_probe.o \ >> iris_resources.o \ >> iris_vidc.o \ >> + iris_vpu_common.o \ >> obj-$(CONFIG_VIDEO_QCOM_IRIS) += iris.o >> diff --git a/drivers/media/platform/qcom/iris/iris_core.c >> b/drivers/media/platform/qcom/iris/iris_core.c >> index 8c7d53c57086..5ad66ac113ae 100644 >> --- a/drivers/media/platform/qcom/iris/iris_core.c >> +++ b/drivers/media/platform/qcom/iris/iris_core.c >> @@ -6,6 +6,7 @@ >> #include "iris_core.h" >> #include "iris_firmware.h" >> #include "iris_state.h" >> +#include "iris_vpu_common.h" >> void iris_core_deinit(struct iris_core *core) >> { >> @@ -39,10 +40,16 @@ int iris_core_init(struct iris_core *core) >> if (ret) >> goto error_queue_deinit; >> + ret = iris_vpu_boot_firmware(core); >> + if (ret) >> + goto error_unload_fw; >> + >> mutex_unlock(&core->lock); >> return 0; >> +error_unload_fw: >> + iris_fw_unload(core); >> error_queue_deinit: >> iris_hfi_queues_deinit(core); >> error: >> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h >> b/drivers/media/platform/qcom/iris/iris_platform_common.h >> index 9c919367f9d7..47fdebd8135c 100644 >> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h >> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h >> @@ -45,6 +45,7 @@ struct iris_platform_data { >> const char *fwname; >> u32 pas_id; >> struct tz_cp_config *tz_cp_config_data; >> + u32 core_arch; >> }; >> #endif >> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c >> b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c >> index 1bb34c3e6e18..a559e095fefc 100644 >> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c >> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c >> @@ -8,6 +8,8 @@ >> #include "iris_platform_common.h" >> #include "iris_resources.h" >> +#define VIDEO_ARCH_LX 1 >> + >> static const struct icc_info sm8550_icc_table[] = { >> { "cpu-cfg", 1000, 1000 }, >> { "video-mem", 1000, 15000000 }, >> @@ -48,4 +50,5 @@ struct iris_platform_data sm8550_data = { >> .fwname = "qcom/vpu/vpu30_p4.mbn", >> .pas_id = IRIS_PAS_ID, >> .tz_cp_config_data = &tz_cp_config_sm8550, >> + .core_arch = VIDEO_ARCH_LX, >> }; >> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c >> b/drivers/media/platform/qcom/iris/iris_vpu_common.c >> new file mode 100644 >> index 000000000000..df87b1b719a9 >> --- /dev/null >> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c >> @@ -0,0 +1,87 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. >> + */ >> + >> +#include <linux/iopoll.h> >> + >> +#include "iris_core.h" >> +#include "iris_vpu_common.h" >> + >> +#define CPU_BASE_OFFS 0x000A0000 >> + >> +#define CPU_CS_BASE_OFFS (CPU_BASE_OFFS) >> + >> +#define CTRL_INIT (CPU_CS_BASE_OFFS + 0x48) >> +#define CTRL_STATUS (CPU_CS_BASE_OFFS + 0x4C) >> + >> +#define CTRL_ERROR_STATUS__M 0xfe >> + >> +#define QTBL_INFO (CPU_CS_BASE_OFFS + 0x50) >> +#define QTBL_ADDR (CPU_CS_BASE_OFFS + 0x54) >> +#define CPU_CS_SCIACMDARG3 (CPU_CS_BASE_OFFS + 0x58) >> +#define SFR_ADDR (CPU_CS_BASE_OFFS + 0x5C) >> +#define UC_REGION_ADDR (CPU_CS_BASE_OFFS + 0x64) >> +#define UC_REGION_SIZE (CPU_CS_BASE_OFFS + 0x68) >> + >> +#define CPU_CS_H2XSOFTINTEN (CPU_CS_BASE_OFFS + 0x148) >> +#define CPU_CS_X2RPMH (CPU_CS_BASE_OFFS + 0x168) >> + >> +static void iris_vpu_setup_ucregion_memory_map(struct iris_core *core) >> +{ >> + u32 queue_size, value; >> + >> + /* Iris hardware requires 4K queue alignment */ >> + queue_size = ALIGN(sizeof(struct iris_hfi_queue_table_header) + >> + (IFACEQ_QUEUE_SIZE * IFACEQ_NUMQ), SZ_4K); >> + >> + value = (u32)core->iface_q_table_daddr; >> + writel(value, core->reg_base + UC_REGION_ADDR); >> + >> + /* Iris hardware requires 1M queue alignment */ >> + value = ALIGN(SFR_SIZE + queue_size, SZ_1M); >> + writel(value, core->reg_base + UC_REGION_SIZE); >> + >> + value = (u32)core->iface_q_table_daddr; >> + writel(value, core->reg_base + QTBL_ADDR); >> + >> + writel(0x01, core->reg_base + QTBL_INFO); > > A general comment I have is instead of writing hard-coded values to registers we > should define at a minimum the bit-fields we use if not the entire set of > bits-fields for the register. > > The only exception to this is when we don't know what those values are - for > example receiving a magic write sequence for a camera sensor. > > In this case though we have full access to enumerate the register bit-fields. > > Without looking at the register descriptions I guess this bit is an enable or a > startup bit => > > #define QTBL_INFO_EN BIT(0) Yes, we should expand the bits being set to provide better context to it. There are few other places as well, primarily the power on/off sequences, will update it. > > I'll not go through this series reiterating this comment but, it certainly > applies to any bit-field/register in the same => please define at least the bits > used if not the full set of bits for register writes instead of using magic > numbers. > > --- > bod Regards, Vikash
diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile index ddd4c994a0b9..95f4e92fe085 100644 --- a/drivers/media/platform/qcom/iris/Makefile +++ b/drivers/media/platform/qcom/iris/Makefile @@ -8,5 +8,6 @@ iris-objs += iris_core.o \ iris_probe.o \ iris_resources.o \ iris_vidc.o \ + iris_vpu_common.o \ obj-$(CONFIG_VIDEO_QCOM_IRIS) += iris.o diff --git a/drivers/media/platform/qcom/iris/iris_core.c b/drivers/media/platform/qcom/iris/iris_core.c index 8c7d53c57086..5ad66ac113ae 100644 --- a/drivers/media/platform/qcom/iris/iris_core.c +++ b/drivers/media/platform/qcom/iris/iris_core.c @@ -6,6 +6,7 @@ #include "iris_core.h" #include "iris_firmware.h" #include "iris_state.h" +#include "iris_vpu_common.h" void iris_core_deinit(struct iris_core *core) { @@ -39,10 +40,16 @@ int iris_core_init(struct iris_core *core) if (ret) goto error_queue_deinit; + ret = iris_vpu_boot_firmware(core); + if (ret) + goto error_unload_fw; + mutex_unlock(&core->lock); return 0; +error_unload_fw: + iris_fw_unload(core); error_queue_deinit: iris_hfi_queues_deinit(core); error: diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h index 9c919367f9d7..47fdebd8135c 100644 --- a/drivers/media/platform/qcom/iris/iris_platform_common.h +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h @@ -45,6 +45,7 @@ struct iris_platform_data { const char *fwname; u32 pas_id; struct tz_cp_config *tz_cp_config_data; + u32 core_arch; }; #endif diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c index 1bb34c3e6e18..a559e095fefc 100644 --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c @@ -8,6 +8,8 @@ #include "iris_platform_common.h" #include "iris_resources.h" +#define VIDEO_ARCH_LX 1 + static const struct icc_info sm8550_icc_table[] = { { "cpu-cfg", 1000, 1000 }, { "video-mem", 1000, 15000000 }, @@ -48,4 +50,5 @@ struct iris_platform_data sm8550_data = { .fwname = "qcom/vpu/vpu30_p4.mbn", .pas_id = IRIS_PAS_ID, .tz_cp_config_data = &tz_cp_config_sm8550, + .core_arch = VIDEO_ARCH_LX, }; diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c new file mode 100644 index 000000000000..df87b1b719a9 --- /dev/null +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#include <linux/iopoll.h> + +#include "iris_core.h" +#include "iris_vpu_common.h" + +#define CPU_BASE_OFFS 0x000A0000 + +#define CPU_CS_BASE_OFFS (CPU_BASE_OFFS) + +#define CTRL_INIT (CPU_CS_BASE_OFFS + 0x48) +#define CTRL_STATUS (CPU_CS_BASE_OFFS + 0x4C) + +#define CTRL_ERROR_STATUS__M 0xfe + +#define QTBL_INFO (CPU_CS_BASE_OFFS + 0x50) +#define QTBL_ADDR (CPU_CS_BASE_OFFS + 0x54) +#define CPU_CS_SCIACMDARG3 (CPU_CS_BASE_OFFS + 0x58) +#define SFR_ADDR (CPU_CS_BASE_OFFS + 0x5C) +#define UC_REGION_ADDR (CPU_CS_BASE_OFFS + 0x64) +#define UC_REGION_SIZE (CPU_CS_BASE_OFFS + 0x68) + +#define CPU_CS_H2XSOFTINTEN (CPU_CS_BASE_OFFS + 0x148) +#define CPU_CS_X2RPMH (CPU_CS_BASE_OFFS + 0x168) + +static void iris_vpu_setup_ucregion_memory_map(struct iris_core *core) +{ + u32 queue_size, value; + + /* Iris hardware requires 4K queue alignment */ + queue_size = ALIGN(sizeof(struct iris_hfi_queue_table_header) + + (IFACEQ_QUEUE_SIZE * IFACEQ_NUMQ), SZ_4K); + + value = (u32)core->iface_q_table_daddr; + writel(value, core->reg_base + UC_REGION_ADDR); + + /* Iris hardware requires 1M queue alignment */ + value = ALIGN(SFR_SIZE + queue_size, SZ_1M); + writel(value, core->reg_base + UC_REGION_SIZE); + + value = (u32)core->iface_q_table_daddr; + writel(value, core->reg_base + QTBL_ADDR); + + writel(0x01, core->reg_base + QTBL_INFO); + + if (core->sfr_daddr) { + value = (u32)core->sfr_daddr + core->iris_platform_data->core_arch; + writel(value, core->reg_base + SFR_ADDR); + } +} + +int iris_vpu_boot_firmware(struct iris_core *core) +{ + u32 ctrl_init = 0, ctrl_status = 0, count = 0, max_tries = 1000; + + iris_vpu_setup_ucregion_memory_map(core); + + ctrl_init = BIT(0); + + writel(ctrl_init, core->reg_base + CTRL_INIT); + writel(0x1, core->reg_base + CPU_CS_SCIACMDARG3); + + while (!ctrl_status && count < max_tries) { + ctrl_status = readl(core->reg_base + CTRL_STATUS); + if ((ctrl_status & CTRL_ERROR_STATUS__M) == 0x4) { + dev_err(core->dev, "invalid setting for uc_region\n"); + break; + } + + usleep_range(50, 100); + count++; + } + + if (count >= max_tries) { + dev_err(core->dev, "error booting up iris firmware\n"); + return -ETIME; + } + + writel(0x1, core->reg_base + CPU_CS_H2XSOFTINTEN); + writel(0x0, core->reg_base + CPU_CS_X2RPMH); + + return 0; +} diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.h b/drivers/media/platform/qcom/iris/iris_vpu_common.h new file mode 100644 index 000000000000..d9b8df6e3f80 --- /dev/null +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#ifndef _IRIS_VPU_COMMON_H_ +#define _IRIS_VPU_COMMON_H_ + +struct iris_core; + +int iris_vpu_boot_firmware(struct iris_core *core); + +#endif