On 12/19/2023 12:16 AM, Dmitry Baryshkov wrote: > On 18/12/2023 13:32, Dikshita Agarwal wrote: >> Introduces different states for core. State transitions >> are defined, based on which they would be allowed/rejected/ignored. >> >> IRIS_CORE_DEINIT - default state. >> IRIS_CORE_INIT_WAIT - wait state for video core to complete >> initialization. >> IRIS_CORE_INIT - core state with core initialized. FW loaded and >> HW brought out of reset, shared queues established between host >> driver and firmware. >> IRIS_CORE_ERROR - error state. >> >> | >> v >> ----------- >> +--->| DEINIT |<---+ >> | ----------- | >> | | | >> | v | >> | ----------- | >> | |INIT_WAIT| | >> | ----------- | >> | / \ | >> | / \ | >> | / \ | >> | v v v >> ----------- ----------. >> | INIT |-->| ERROR | >> ----------- ----------. >> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx> > > Can we see users for this API please? Otherwise it's just a dead code. > This is like a preparation patch to add the core state. These APIs are used in patch-10[1] onward. [1]: https://patchwork.kernel.org/project/linux-media/patch/1702899149-21321-11-git-send-email-quic_dikshita@xxxxxxxxxxx/ >> --- >> drivers/media/platform/qcom/vcodec/iris/Makefile | 3 +- >> .../media/platform/qcom/vcodec/iris/iris_core.h | 4 ++ >> .../media/platform/qcom/vcodec/iris/iris_state.c | 64 >> ++++++++++++++++++++++ >> .../media/platform/qcom/vcodec/iris/iris_state.h | 22 ++++++++ >> 4 files changed, 92 insertions(+), 1 deletion(-) >> create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_state.c >> create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_state.h >> >> diff --git a/drivers/media/platform/qcom/vcodec/iris/Makefile >> b/drivers/media/platform/qcom/vcodec/iris/Makefile >> index 0748819..12a74de 100644 >> --- a/drivers/media/platform/qcom/vcodec/iris/Makefile >> +++ b/drivers/media/platform/qcom/vcodec/iris/Makefile >> @@ -1,4 +1,5 @@ >> iris-objs += iris_probe.o \ >> - resources.o >> + resources.o \ >> + iris_state.o >> obj-$(CONFIG_VIDEO_QCOM_IRIS) += iris.o >> diff --git a/drivers/media/platform/qcom/vcodec/iris/iris_core.h >> b/drivers/media/platform/qcom/vcodec/iris/iris_core.h >> index c2bc95d..56a5b7d 100644 >> --- a/drivers/media/platform/qcom/vcodec/iris/iris_core.h >> +++ b/drivers/media/platform/qcom/vcodec/iris/iris_core.h >> @@ -9,6 +9,8 @@ >> #include <linux/types.h> >> #include <media/v4l2-device.h> >> +#include "iris_state.h" >> + >> /** >> * struct iris_core - holds core parameters valid for all instances >> * >> @@ -27,6 +29,7 @@ >> * @clk_count: count of iris clocks >> * @reset_tbl: table of iris reset clocks >> * @reset_count: count of iris reset clocks >> + * @state: current state of core >> */ >> struct iris_core { >> @@ -45,6 +48,7 @@ struct iris_core { >> u32 clk_count; >> struct reset_info *reset_tbl; >> u32 reset_count; >> + enum iris_core_state state; >> }; >> #endif >> diff --git a/drivers/media/platform/qcom/vcodec/iris/iris_state.c >> b/drivers/media/platform/qcom/vcodec/iris/iris_state.c >> new file mode 100644 >> index 0000000..22557af >> --- /dev/null >> +++ b/drivers/media/platform/qcom/vcodec/iris/iris_state.c >> @@ -0,0 +1,64 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights >> reserved. >> + */ >> + >> +#include "iris_core.h" >> +#include "iris_state.h" >> + >> +#define IRIS_STATE(name)[IRIS_CORE_##name] = "CORE_"#name > > Inline this please. > >> + >> +static const char * const core_state_names[] = { >> + IRIS_STATE(DEINIT), >> + IRIS_STATE(INIT_WAIT), >> + IRIS_STATE(INIT), >> + IRIS_STATE(ERROR), >> +}; >> + >> +#undef IRIS_STATE >> + >> +bool core_in_valid_state(struct iris_core *core) > > So, even in your driver you have global names? That's really ugh. Please > fix them. > Sure will fix these names. >> +{ >> + return core->state == IRIS_CORE_INIT || >> + core->state == IRIS_CORE_INIT_WAIT; >> +} >> + >> +static const char *core_state_name(enum iris_core_state state) >> +{ >> + if ((unsigned int)state < ARRAY_SIZE(core_state_names)) >> + return core_state_names[state]; >> + >> + return "UNKNOWN_STATE"; >> +} >> + >> +static bool iris_allow_core_state_change(struct iris_core *core, >> + enum iris_core_state req_state) >> +{ >> + if (core->state == IRIS_CORE_DEINIT) >> + return req_state == IRIS_CORE_INIT_WAIT || req_state == >> IRIS_CORE_ERROR; >> + else if (core->state == IRIS_CORE_INIT_WAIT) >> + return req_state == IRIS_CORE_INIT || req_state == IRIS_CORE_ERROR; >> + else if (core->state == IRIS_CORE_INIT) >> + return req_state == IRIS_CORE_DEINIT || req_state == >> IRIS_CORE_ERROR; >> + else if (core->state == IRIS_CORE_ERROR) >> + return req_state == IRIS_CORE_DEINIT; >> + >> + dev_warn(core->dev, "core state change %s -> %s is not allowed\n", >> + core_state_name(core->state), core_state_name(req_state)); >> + >> + return false; >> +} >> + >> +int iris_change_core_state(struct iris_core *core, >> + enum iris_core_state request_state) >> +{ >> + if (core->state == request_state) >> + return 0; >> + >> + if (!iris_allow_core_state_change(core, request_state)) >> + return -EINVAL; >> + >> + core->state = request_state; >> + >> + return 0; >> +} >> diff --git a/drivers/media/platform/qcom/vcodec/iris/iris_state.h >> b/drivers/media/platform/qcom/vcodec/iris/iris_state.h >> new file mode 100644 >> index 0000000..ee20842 >> --- /dev/null >> +++ b/drivers/media/platform/qcom/vcodec/iris/iris_state.h >> @@ -0,0 +1,22 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights >> reserved. >> + */ >> + >> +#ifndef _IRIS_STATE_H_ >> +#define _IRIS_STATE_H_ >> + >> +struct iris_core; >> + >> +enum iris_core_state { >> + IRIS_CORE_DEINIT, >> + IRIS_CORE_INIT_WAIT, >> + IRIS_CORE_INIT, >> + IRIS_CORE_ERROR, >> +}; >> + >> +bool core_in_valid_state(struct iris_core *core); >> +int iris_change_core_state(struct iris_core *core, >> + enum iris_core_state request_state); >> + >> +#endif >