| Message ID | 1702899149-21321-9-git-send-email-quic_dikshita@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Qualcomm video encoder and decoder driver | expand |
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@quicinc.com> Can we see users for this API please? Otherwise it's just a dead code. > --- > 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. > +{ > + 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
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@quicinc.com> > > 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@quicinc.com/ >> --- >> 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 >
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 + +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) +{ + 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
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@quicinc.com> --- 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