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 <[email protected]>
---
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
+
+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
--
2.7.4
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 <[email protected]>
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
--
With best wishes
Dmitry
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 <[email protected]>
>
> 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/[email protected]/
>> ---
>> 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
>