diff mbox series

[v2,08/34] media: iris: introduce state machine for iris core

Message ID 1702899149-21321-9-git-send-email-quic_dikshita@quicinc.com (mailing list archive)
State New, archived
Headers show
Series Qualcomm video encoder and decoder driver | expand

Commit Message

Dikshita Agarwal Dec. 18, 2023, 11:32 a.m. UTC 
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

Comments

Dmitry Baryshkov Dec. 18, 2023, 6:46 p.m. UTC | #1 
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
Dikshita Agarwal Dec. 20, 2023, 8:05 a.m. UTC | #2 
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 mbox series

Patch

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