* [PATCH] drm/amdgpu: Keep reset handlers shared
@ 2023-08-10 11:44 Lijo Lazar
2023-08-10 15:11 ` Christian König
2023-08-16 5:38 ` Lazar, Lijo
0 siblings, 2 replies; 6+ messages in thread
From: Lijo Lazar @ 2023-08-10 11:44 UTC (permalink / raw)
To: amd-gfx; +Cc: Alexander.Deucher, Asad.Kamal, Hawking.Zhang
Instead of maintaining a list per device, keep the reset handlers common
per ASIC family. A pointer to the list of handlers is maintained in
reset control.
Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
---
drivers/gpu/drm/amd/amdgpu/aldebaran.c | 19 +++++++++++--------
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 8 --------
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 16 ++++++++++++----
drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c | 20 +++++++++++---------
drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c | 19 +++++++++++--------
5 files changed, 45 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/aldebaran.c b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
index 2b97b8a96fb4..82e1c83a7ccc 100644
--- a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
+++ b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
@@ -48,20 +48,19 @@ aldebaran_get_reset_handler(struct amdgpu_reset_control *reset_ctl,
{
struct amdgpu_reset_handler *handler;
struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+ int i;
if (reset_context->method != AMD_RESET_METHOD_NONE) {
dev_dbg(adev->dev, "Getting reset handler for method %d\n",
reset_context->method);
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_context->method)
return handler;
}
}
if (aldebaran_is_mode2_default(reset_ctl)) {
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == AMD_RESET_METHOD_MODE2) {
reset_context->method = AMD_RESET_METHOD_MODE2;
return handler;
@@ -124,9 +123,9 @@ static void aldebaran_async_reset(struct work_struct *work)
struct amdgpu_reset_control *reset_ctl =
container_of(work, struct amdgpu_reset_control, reset_work);
struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+ int i;
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_ctl->active_reset) {
dev_dbg(adev->dev, "Resetting device\n");
handler->do_reset(adev);
@@ -395,6 +394,11 @@ static struct amdgpu_reset_handler aldebaran_mode2_handler = {
.do_reset = aldebaran_mode2_reset,
};
+static struct amdgpu_reset_handler
+ *aldebaran_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
+ &aldebaran_mode2_handler,
+ };
+
int aldebaran_reset_init(struct amdgpu_device *adev)
{
struct amdgpu_reset_control *reset_ctl;
@@ -408,10 +412,9 @@ int aldebaran_reset_init(struct amdgpu_device *adev)
reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
reset_ctl->get_reset_handler = aldebaran_get_reset_handler;
- INIT_LIST_HEAD(&reset_ctl->reset_handlers);
INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
/* Only mode2 is handled through reset control now */
- amdgpu_reset_add_handler(reset_ctl, &aldebaran_mode2_handler);
+ reset_ctl->reset_handlers = &aldebaran_rst_handlers;
adev->reset_cntl = reset_ctl;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
index 5fed06ffcc6b..02d874799c16 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
@@ -26,14 +26,6 @@
#include "sienna_cichlid.h"
#include "smu_v13_0_10.h"
-int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
- struct amdgpu_reset_handler *handler)
-{
- /* TODO: Check if handler exists? */
- list_add_tail(&handler->handler_list, &reset_ctl->reset_handlers);
- return 0;
-}
-
int amdgpu_reset_init(struct amdgpu_device *adev)
{
int ret = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
index f4a501ff87d9..471d789b33a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
@@ -26,6 +26,8 @@
#include "amdgpu.h"
+#define AMDGPU_RESET_MAX_HANDLERS 5
+
enum AMDGPU_RESET_FLAGS {
AMDGPU_NEED_FULL_RESET = 0,
@@ -44,7 +46,6 @@ struct amdgpu_reset_context {
struct amdgpu_reset_handler {
enum amd_reset_method reset_method;
- struct list_head handler_list;
int (*prepare_env)(struct amdgpu_reset_control *reset_ctl,
struct amdgpu_reset_context *context);
int (*prepare_hwcontext)(struct amdgpu_reset_control *reset_ctl,
@@ -63,7 +64,8 @@ struct amdgpu_reset_control {
void *handle;
struct work_struct reset_work;
struct mutex reset_lock;
- struct list_head reset_handlers;
+ struct amdgpu_reset_handler *(
+ *reset_handlers)[AMDGPU_RESET_MAX_HANDLERS];
atomic_t in_reset;
enum amd_reset_method active_reset;
struct amdgpu_reset_handler *(*get_reset_handler)(
@@ -97,8 +99,10 @@ int amdgpu_reset_prepare_hwcontext(struct amdgpu_device *adev,
int amdgpu_reset_perform_reset(struct amdgpu_device *adev,
struct amdgpu_reset_context *reset_context);
-int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
- struct amdgpu_reset_handler *handler);
+int amdgpu_reset_prepare_env(struct amdgpu_device *adev,
+ struct amdgpu_reset_context *reset_context);
+int amdgpu_reset_restore_env(struct amdgpu_device *adev,
+ struct amdgpu_reset_context *reset_context);
struct amdgpu_reset_domain *amdgpu_reset_create_reset_domain(enum amdgpu_reset_domain_type type,
char *wq_name);
@@ -126,4 +130,8 @@ void amdgpu_device_lock_reset_domain(struct amdgpu_reset_domain *reset_domain);
void amdgpu_device_unlock_reset_domain(struct amdgpu_reset_domain *reset_domain);
+#define for_each_handler(i, handler, reset_ctl) \
+ for (i = 0; (i < AMDGPU_RESET_MAX_HANDLERS) && \
+ (handler = (*reset_ctl->reset_handlers)[i]); \
+ ++i)
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
index 8b8086d5c864..07ded70f4df9 100644
--- a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
+++ b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
@@ -48,18 +48,17 @@ sienna_cichlid_get_reset_handler(struct amdgpu_reset_control *reset_ctl,
struct amdgpu_reset_context *reset_context)
{
struct amdgpu_reset_handler *handler;
+ int i;
if (reset_context->method != AMD_RESET_METHOD_NONE) {
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_context->method)
return handler;
}
}
if (sienna_cichlid_is_mode2_default(reset_ctl)) {
- list_for_each_entry (handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == AMD_RESET_METHOD_MODE2)
return handler;
}
@@ -120,9 +119,9 @@ static void sienna_cichlid_async_reset(struct work_struct *work)
struct amdgpu_reset_control *reset_ctl =
container_of(work, struct amdgpu_reset_control, reset_work);
struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+ int i;
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_ctl->active_reset) {
dev_dbg(adev->dev, "Resetting device\n");
handler->do_reset(adev);
@@ -281,6 +280,11 @@ static struct amdgpu_reset_handler sienna_cichlid_mode2_handler = {
.do_reset = sienna_cichlid_mode2_reset,
};
+static struct amdgpu_reset_handler
+ *sienna_cichlid_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
+ &sienna_cichlid_mode2_handler,
+ };
+
int sienna_cichlid_reset_init(struct amdgpu_device *adev)
{
struct amdgpu_reset_control *reset_ctl;
@@ -294,11 +298,9 @@ int sienna_cichlid_reset_init(struct amdgpu_device *adev)
reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
reset_ctl->get_reset_handler = sienna_cichlid_get_reset_handler;
- INIT_LIST_HEAD(&reset_ctl->reset_handlers);
INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
/* Only mode2 is handled through reset control now */
- amdgpu_reset_add_handler(reset_ctl, &sienna_cichlid_mode2_handler);
-
+ reset_ctl->reset_handlers = &sienna_cichlid_rst_handlers;
adev->reset_cntl = reset_ctl;
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
index ae29620b1ea4..04c797d54511 100644
--- a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
+++ b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
@@ -44,10 +44,10 @@ smu_v13_0_10_get_reset_handler(struct amdgpu_reset_control *reset_ctl,
{
struct amdgpu_reset_handler *handler;
struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+ int i;
if (reset_context->method != AMD_RESET_METHOD_NONE) {
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_context->method)
return handler;
}
@@ -55,8 +55,7 @@ smu_v13_0_10_get_reset_handler(struct amdgpu_reset_control *reset_ctl,
if (smu_v13_0_10_is_mode2_default(reset_ctl) &&
amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_MODE2) {
- list_for_each_entry (handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == AMD_RESET_METHOD_MODE2)
return handler;
}
@@ -119,9 +118,9 @@ static void smu_v13_0_10_async_reset(struct work_struct *work)
struct amdgpu_reset_control *reset_ctl =
container_of(work, struct amdgpu_reset_control, reset_work);
struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+ int i;
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_ctl->active_reset) {
dev_dbg(adev->dev, "Resetting device\n");
handler->do_reset(adev);
@@ -272,6 +271,11 @@ static struct amdgpu_reset_handler smu_v13_0_10_mode2_handler = {
.do_reset = smu_v13_0_10_mode2_reset,
};
+static struct amdgpu_reset_handler
+ *smu_v13_0_10_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
+ &smu_v13_0_10_mode2_handler,
+ };
+
int smu_v13_0_10_reset_init(struct amdgpu_device *adev)
{
struct amdgpu_reset_control *reset_ctl;
@@ -285,10 +289,9 @@ int smu_v13_0_10_reset_init(struct amdgpu_device *adev)
reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
reset_ctl->get_reset_handler = smu_v13_0_10_get_reset_handler;
- INIT_LIST_HEAD(&reset_ctl->reset_handlers);
INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
/* Only mode2 is handled through reset control now */
- amdgpu_reset_add_handler(reset_ctl, &smu_v13_0_10_mode2_handler);
+ reset_ctl->reset_handlers = &smu_v13_0_10_rst_handlers;
adev->reset_cntl = reset_ctl;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Keep reset handlers shared
2023-08-10 11:44 [PATCH] drm/amdgpu: Keep reset handlers shared Lijo Lazar
@ 2023-08-10 15:11 ` Christian König
2023-08-10 17:13 ` Lazar, Lijo
2023-08-16 5:38 ` Lazar, Lijo
1 sibling, 1 reply; 6+ messages in thread
From: Christian König @ 2023-08-10 15:11 UTC (permalink / raw)
To: Lijo Lazar, amd-gfx; +Cc: Alexander.Deucher, Asad.Kamal, Hawking.Zhang
Am 10.08.23 um 13:44 schrieb Lijo Lazar:
> Instead of maintaining a list per device, keep the reset handlers common
> per ASIC family. A pointer to the list of handlers is maintained in
> reset control.
Why should this be beneficial?
Christian.
>
> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/aldebaran.c | 19 +++++++++++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 8 --------
> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 16 ++++++++++++----
> drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c | 20 +++++++++++---------
> drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c | 19 +++++++++++--------
> 5 files changed, 45 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/aldebaran.c b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> index 2b97b8a96fb4..82e1c83a7ccc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> +++ b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> @@ -48,20 +48,19 @@ aldebaran_get_reset_handler(struct amdgpu_reset_control *reset_ctl,
> {
> struct amdgpu_reset_handler *handler;
> struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
> + int i;
>
> if (reset_context->method != AMD_RESET_METHOD_NONE) {
> dev_dbg(adev->dev, "Getting reset handler for method %d\n",
> reset_context->method);
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_context->method)
> return handler;
> }
> }
>
> if (aldebaran_is_mode2_default(reset_ctl)) {
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == AMD_RESET_METHOD_MODE2) {
> reset_context->method = AMD_RESET_METHOD_MODE2;
> return handler;
> @@ -124,9 +123,9 @@ static void aldebaran_async_reset(struct work_struct *work)
> struct amdgpu_reset_control *reset_ctl =
> container_of(work, struct amdgpu_reset_control, reset_work);
> struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
> + int i;
>
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_ctl->active_reset) {
> dev_dbg(adev->dev, "Resetting device\n");
> handler->do_reset(adev);
> @@ -395,6 +394,11 @@ static struct amdgpu_reset_handler aldebaran_mode2_handler = {
> .do_reset = aldebaran_mode2_reset,
> };
>
> +static struct amdgpu_reset_handler
> + *aldebaran_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
> + &aldebaran_mode2_handler,
> + };
> +
> int aldebaran_reset_init(struct amdgpu_device *adev)
> {
> struct amdgpu_reset_control *reset_ctl;
> @@ -408,10 +412,9 @@ int aldebaran_reset_init(struct amdgpu_device *adev)
> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
> reset_ctl->get_reset_handler = aldebaran_get_reset_handler;
>
> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
> /* Only mode2 is handled through reset control now */
> - amdgpu_reset_add_handler(reset_ctl, &aldebaran_mode2_handler);
> + reset_ctl->reset_handlers = &aldebaran_rst_handlers;
>
> adev->reset_cntl = reset_ctl;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> index 5fed06ffcc6b..02d874799c16 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> @@ -26,14 +26,6 @@
> #include "sienna_cichlid.h"
> #include "smu_v13_0_10.h"
>
> -int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
> - struct amdgpu_reset_handler *handler)
> -{
> - /* TODO: Check if handler exists? */
> - list_add_tail(&handler->handler_list, &reset_ctl->reset_handlers);
> - return 0;
> -}
> -
> int amdgpu_reset_init(struct amdgpu_device *adev)
> {
> int ret = 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> index f4a501ff87d9..471d789b33a5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> @@ -26,6 +26,8 @@
>
> #include "amdgpu.h"
>
> +#define AMDGPU_RESET_MAX_HANDLERS 5
> +
> enum AMDGPU_RESET_FLAGS {
>
> AMDGPU_NEED_FULL_RESET = 0,
> @@ -44,7 +46,6 @@ struct amdgpu_reset_context {
>
> struct amdgpu_reset_handler {
> enum amd_reset_method reset_method;
> - struct list_head handler_list;
> int (*prepare_env)(struct amdgpu_reset_control *reset_ctl,
> struct amdgpu_reset_context *context);
> int (*prepare_hwcontext)(struct amdgpu_reset_control *reset_ctl,
> @@ -63,7 +64,8 @@ struct amdgpu_reset_control {
> void *handle;
> struct work_struct reset_work;
> struct mutex reset_lock;
> - struct list_head reset_handlers;
> + struct amdgpu_reset_handler *(
> + *reset_handlers)[AMDGPU_RESET_MAX_HANDLERS];
> atomic_t in_reset;
> enum amd_reset_method active_reset;
> struct amdgpu_reset_handler *(*get_reset_handler)(
> @@ -97,8 +99,10 @@ int amdgpu_reset_prepare_hwcontext(struct amdgpu_device *adev,
> int amdgpu_reset_perform_reset(struct amdgpu_device *adev,
> struct amdgpu_reset_context *reset_context);
>
> -int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
> - struct amdgpu_reset_handler *handler);
> +int amdgpu_reset_prepare_env(struct amdgpu_device *adev,
> + struct amdgpu_reset_context *reset_context);
> +int amdgpu_reset_restore_env(struct amdgpu_device *adev,
> + struct amdgpu_reset_context *reset_context);
>
> struct amdgpu_reset_domain *amdgpu_reset_create_reset_domain(enum amdgpu_reset_domain_type type,
> char *wq_name);
> @@ -126,4 +130,8 @@ void amdgpu_device_lock_reset_domain(struct amdgpu_reset_domain *reset_domain);
>
> void amdgpu_device_unlock_reset_domain(struct amdgpu_reset_domain *reset_domain);
>
> +#define for_each_handler(i, handler, reset_ctl) \
> + for (i = 0; (i < AMDGPU_RESET_MAX_HANDLERS) && \
> + (handler = (*reset_ctl->reset_handlers)[i]); \
> + ++i)
> #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> index 8b8086d5c864..07ded70f4df9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> @@ -48,18 +48,17 @@ sienna_cichlid_get_reset_handler(struct amdgpu_reset_control *reset_ctl,
> struct amdgpu_reset_context *reset_context)
> {
> struct amdgpu_reset_handler *handler;
> + int i;
>
> if (reset_context->method != AMD_RESET_METHOD_NONE) {
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_context->method)
> return handler;
> }
> }
>
> if (sienna_cichlid_is_mode2_default(reset_ctl)) {
> - list_for_each_entry (handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == AMD_RESET_METHOD_MODE2)
> return handler;
> }
> @@ -120,9 +119,9 @@ static void sienna_cichlid_async_reset(struct work_struct *work)
> struct amdgpu_reset_control *reset_ctl =
> container_of(work, struct amdgpu_reset_control, reset_work);
> struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
> + int i;
>
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_ctl->active_reset) {
> dev_dbg(adev->dev, "Resetting device\n");
> handler->do_reset(adev);
> @@ -281,6 +280,11 @@ static struct amdgpu_reset_handler sienna_cichlid_mode2_handler = {
> .do_reset = sienna_cichlid_mode2_reset,
> };
>
> +static struct amdgpu_reset_handler
> + *sienna_cichlid_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
> + &sienna_cichlid_mode2_handler,
> + };
> +
> int sienna_cichlid_reset_init(struct amdgpu_device *adev)
> {
> struct amdgpu_reset_control *reset_ctl;
> @@ -294,11 +298,9 @@ int sienna_cichlid_reset_init(struct amdgpu_device *adev)
> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
> reset_ctl->get_reset_handler = sienna_cichlid_get_reset_handler;
>
> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
> /* Only mode2 is handled through reset control now */
> - amdgpu_reset_add_handler(reset_ctl, &sienna_cichlid_mode2_handler);
> -
> + reset_ctl->reset_handlers = &sienna_cichlid_rst_handlers;
> adev->reset_cntl = reset_ctl;
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> index ae29620b1ea4..04c797d54511 100644
> --- a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> +++ b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> @@ -44,10 +44,10 @@ smu_v13_0_10_get_reset_handler(struct amdgpu_reset_control *reset_ctl,
> {
> struct amdgpu_reset_handler *handler;
> struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
> + int i;
>
> if (reset_context->method != AMD_RESET_METHOD_NONE) {
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_context->method)
> return handler;
> }
> @@ -55,8 +55,7 @@ smu_v13_0_10_get_reset_handler(struct amdgpu_reset_control *reset_ctl,
>
> if (smu_v13_0_10_is_mode2_default(reset_ctl) &&
> amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_MODE2) {
> - list_for_each_entry (handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == AMD_RESET_METHOD_MODE2)
> return handler;
> }
> @@ -119,9 +118,9 @@ static void smu_v13_0_10_async_reset(struct work_struct *work)
> struct amdgpu_reset_control *reset_ctl =
> container_of(work, struct amdgpu_reset_control, reset_work);
> struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
> + int i;
>
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_ctl->active_reset) {
> dev_dbg(adev->dev, "Resetting device\n");
> handler->do_reset(adev);
> @@ -272,6 +271,11 @@ static struct amdgpu_reset_handler smu_v13_0_10_mode2_handler = {
> .do_reset = smu_v13_0_10_mode2_reset,
> };
>
> +static struct amdgpu_reset_handler
> + *smu_v13_0_10_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
> + &smu_v13_0_10_mode2_handler,
> + };
> +
> int smu_v13_0_10_reset_init(struct amdgpu_device *adev)
> {
> struct amdgpu_reset_control *reset_ctl;
> @@ -285,10 +289,9 @@ int smu_v13_0_10_reset_init(struct amdgpu_device *adev)
> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
> reset_ctl->get_reset_handler = smu_v13_0_10_get_reset_handler;
>
> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
> /* Only mode2 is handled through reset control now */
> - amdgpu_reset_add_handler(reset_ctl, &smu_v13_0_10_mode2_handler);
> + reset_ctl->reset_handlers = &smu_v13_0_10_rst_handlers;
>
> adev->reset_cntl = reset_ctl;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Keep reset handlers shared
2023-08-10 15:11 ` Christian König
@ 2023-08-10 17:13 ` Lazar, Lijo
0 siblings, 0 replies; 6+ messages in thread
From: Lazar, Lijo @ 2023-08-10 17:13 UTC (permalink / raw)
To: Christian König, amd-gfx
Cc: Alexander.Deucher, Asad.Kamal, Hawking.Zhang
On 8/10/2023 8:41 PM, Christian König wrote:
> Am 10.08.23 um 13:44 schrieb Lijo Lazar:
>> Instead of maintaining a list per device, keep the reset handlers common
>> per ASIC family. A pointer to the list of handlers is maintained in
>> reset control.
>
> Why should this be beneficial?
> There is a global reset handler object for each type of reset for a
particular ASIC family. Each device has a reset control which holds a
reference to these handlers.
Earlier, the handler used to be a list object. This creates trouble when
there are multiple devices of the same ASIC family - the same global
object gets added to reset control of each device and that corrupts list.
Keeping an array of reset handlers and having the reset control holding
a reference to the array of handlers makes it simpler.
Thanks,
Lijo
> Christian.
>
>>
>> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/aldebaran.c | 19 +++++++++++--------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 8 --------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 16 ++++++++++++----
>> drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c | 20 +++++++++++---------
>> drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c | 19 +++++++++++--------
>> 5 files changed, 45 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
>> b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
>> index 2b97b8a96fb4..82e1c83a7ccc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
>> @@ -48,20 +48,19 @@ aldebaran_get_reset_handler(struct
>> amdgpu_reset_control *reset_ctl,
>> {
>> struct amdgpu_reset_handler *handler;
>> struct amdgpu_device *adev = (struct amdgpu_device
>> *)reset_ctl->handle;
>> + int i;
>> if (reset_context->method != AMD_RESET_METHOD_NONE) {
>> dev_dbg(adev->dev, "Getting reset handler for method %d\n",
>> reset_context->method);
>> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
>> - handler_list) {
>> + for_each_handler(i, handler, reset_ctl) {
>> if (handler->reset_method == reset_context->method)
>> return handler;
>> }
>> }
>> if (aldebaran_is_mode2_default(reset_ctl)) {
>> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
>> - handler_list) {
>> + for_each_handler(i, handler, reset_ctl) {
>> if (handler->reset_method == AMD_RESET_METHOD_MODE2) {
>> reset_context->method = AMD_RESET_METHOD_MODE2;
>> return handler;
>> @@ -124,9 +123,9 @@ static void aldebaran_async_reset(struct
>> work_struct *work)
>> struct amdgpu_reset_control *reset_ctl =
>> container_of(work, struct amdgpu_reset_control, reset_work);
>> struct amdgpu_device *adev = (struct amdgpu_device
>> *)reset_ctl->handle;
>> + int i;
>> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
>> - handler_list) {
>> + for_each_handler(i, handler, reset_ctl) {
>> if (handler->reset_method == reset_ctl->active_reset) {
>> dev_dbg(adev->dev, "Resetting device\n");
>> handler->do_reset(adev);
>> @@ -395,6 +394,11 @@ static struct amdgpu_reset_handler
>> aldebaran_mode2_handler = {
>> .do_reset = aldebaran_mode2_reset,
>> };
>> +static struct amdgpu_reset_handler
>> + *aldebaran_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
>> + &aldebaran_mode2_handler,
>> + };
>> +
>> int aldebaran_reset_init(struct amdgpu_device *adev)
>> {
>> struct amdgpu_reset_control *reset_ctl;
>> @@ -408,10 +412,9 @@ int aldebaran_reset_init(struct amdgpu_device *adev)
>> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
>> reset_ctl->get_reset_handler = aldebaran_get_reset_handler;
>> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
>> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
>> /* Only mode2 is handled through reset control now */
>> - amdgpu_reset_add_handler(reset_ctl, &aldebaran_mode2_handler);
>> + reset_ctl->reset_handlers = &aldebaran_rst_handlers;
>> adev->reset_cntl = reset_ctl;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> index 5fed06ffcc6b..02d874799c16 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
>> @@ -26,14 +26,6 @@
>> #include "sienna_cichlid.h"
>> #include "smu_v13_0_10.h"
>> -int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
>> - struct amdgpu_reset_handler *handler)
>> -{
>> - /* TODO: Check if handler exists? */
>> - list_add_tail(&handler->handler_list, &reset_ctl->reset_handlers);
>> - return 0;
>> -}
>> -
>> int amdgpu_reset_init(struct amdgpu_device *adev)
>> {
>> int ret = 0;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>> index f4a501ff87d9..471d789b33a5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>> @@ -26,6 +26,8 @@
>> #include "amdgpu.h"
>> +#define AMDGPU_RESET_MAX_HANDLERS 5
>> +
>> enum AMDGPU_RESET_FLAGS {
>> AMDGPU_NEED_FULL_RESET = 0,
>> @@ -44,7 +46,6 @@ struct amdgpu_reset_context {
>> struct amdgpu_reset_handler {
>> enum amd_reset_method reset_method;
>> - struct list_head handler_list;
>> int (*prepare_env)(struct amdgpu_reset_control *reset_ctl,
>> struct amdgpu_reset_context *context);
>> int (*prepare_hwcontext)(struct amdgpu_reset_control *reset_ctl,
>> @@ -63,7 +64,8 @@ struct amdgpu_reset_control {
>> void *handle;
>> struct work_struct reset_work;
>> struct mutex reset_lock;
>> - struct list_head reset_handlers;
>> + struct amdgpu_reset_handler *(
>> + *reset_handlers)[AMDGPU_RESET_MAX_HANDLERS];
>> atomic_t in_reset;
>> enum amd_reset_method active_reset;
>> struct amdgpu_reset_handler *(*get_reset_handler)(
>> @@ -97,8 +99,10 @@ int amdgpu_reset_prepare_hwcontext(struct
>> amdgpu_device *adev,
>> int amdgpu_reset_perform_reset(struct amdgpu_device *adev,
>> struct amdgpu_reset_context *reset_context);
>> -int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
>> - struct amdgpu_reset_handler *handler);
>> +int amdgpu_reset_prepare_env(struct amdgpu_device *adev,
>> + struct amdgpu_reset_context *reset_context);
>> +int amdgpu_reset_restore_env(struct amdgpu_device *adev,
>> + struct amdgpu_reset_context *reset_context);
>> struct amdgpu_reset_domain *amdgpu_reset_create_reset_domain(enum
>> amdgpu_reset_domain_type type,
>> char *wq_name);
>> @@ -126,4 +130,8 @@ void amdgpu_device_lock_reset_domain(struct
>> amdgpu_reset_domain *reset_domain);
>> void amdgpu_device_unlock_reset_domain(struct amdgpu_reset_domain
>> *reset_domain);
>> +#define for_each_handler(i, handler, reset_ctl) \
>> + for (i = 0; (i < AMDGPU_RESET_MAX_HANDLERS) && \
>> + (handler = (*reset_ctl->reset_handlers)[i]); \
>> + ++i)
>> #endif
>> diff --git a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
>> b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
>> index 8b8086d5c864..07ded70f4df9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
>> @@ -48,18 +48,17 @@ sienna_cichlid_get_reset_handler(struct
>> amdgpu_reset_control *reset_ctl,
>> struct amdgpu_reset_context *reset_context)
>> {
>> struct amdgpu_reset_handler *handler;
>> + int i;
>> if (reset_context->method != AMD_RESET_METHOD_NONE) {
>> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
>> - handler_list) {
>> + for_each_handler(i, handler, reset_ctl) {
>> if (handler->reset_method == reset_context->method)
>> return handler;
>> }
>> }
>> if (sienna_cichlid_is_mode2_default(reset_ctl)) {
>> - list_for_each_entry (handler, &reset_ctl->reset_handlers,
>> - handler_list) {
>> + for_each_handler(i, handler, reset_ctl) {
>> if (handler->reset_method == AMD_RESET_METHOD_MODE2)
>> return handler;
>> }
>> @@ -120,9 +119,9 @@ static void sienna_cichlid_async_reset(struct
>> work_struct *work)
>> struct amdgpu_reset_control *reset_ctl =
>> container_of(work, struct amdgpu_reset_control, reset_work);
>> struct amdgpu_device *adev = (struct amdgpu_device
>> *)reset_ctl->handle;
>> + int i;
>> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
>> - handler_list) {
>> + for_each_handler(i, handler, reset_ctl) {
>> if (handler->reset_method == reset_ctl->active_reset) {
>> dev_dbg(adev->dev, "Resetting device\n");
>> handler->do_reset(adev);
>> @@ -281,6 +280,11 @@ static struct amdgpu_reset_handler
>> sienna_cichlid_mode2_handler = {
>> .do_reset = sienna_cichlid_mode2_reset,
>> };
>> +static struct amdgpu_reset_handler
>> + *sienna_cichlid_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
>> + &sienna_cichlid_mode2_handler,
>> + };
>> +
>> int sienna_cichlid_reset_init(struct amdgpu_device *adev)
>> {
>> struct amdgpu_reset_control *reset_ctl;
>> @@ -294,11 +298,9 @@ int sienna_cichlid_reset_init(struct
>> amdgpu_device *adev)
>> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
>> reset_ctl->get_reset_handler = sienna_cichlid_get_reset_handler;
>> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
>> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
>> /* Only mode2 is handled through reset control now */
>> - amdgpu_reset_add_handler(reset_ctl, &sienna_cichlid_mode2_handler);
>> -
>> + reset_ctl->reset_handlers = &sienna_cichlid_rst_handlers;
>> adev->reset_cntl = reset_ctl;
>> return 0;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
>> b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
>> index ae29620b1ea4..04c797d54511 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
>> @@ -44,10 +44,10 @@ smu_v13_0_10_get_reset_handler(struct
>> amdgpu_reset_control *reset_ctl,
>> {
>> struct amdgpu_reset_handler *handler;
>> struct amdgpu_device *adev = (struct amdgpu_device
>> *)reset_ctl->handle;
>> + int i;
>> if (reset_context->method != AMD_RESET_METHOD_NONE) {
>> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
>> - handler_list) {
>> + for_each_handler(i, handler, reset_ctl) {
>> if (handler->reset_method == reset_context->method)
>> return handler;
>> }
>> @@ -55,8 +55,7 @@ smu_v13_0_10_get_reset_handler(struct
>> amdgpu_reset_control *reset_ctl,
>> if (smu_v13_0_10_is_mode2_default(reset_ctl) &&
>> amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_MODE2) {
>> - list_for_each_entry (handler, &reset_ctl->reset_handlers,
>> - handler_list) {
>> + for_each_handler(i, handler, reset_ctl) {
>> if (handler->reset_method == AMD_RESET_METHOD_MODE2)
>> return handler;
>> }
>> @@ -119,9 +118,9 @@ static void smu_v13_0_10_async_reset(struct
>> work_struct *work)
>> struct amdgpu_reset_control *reset_ctl =
>> container_of(work, struct amdgpu_reset_control, reset_work);
>> struct amdgpu_device *adev = (struct amdgpu_device
>> *)reset_ctl->handle;
>> + int i;
>> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
>> - handler_list) {
>> + for_each_handler(i, handler, reset_ctl) {
>> if (handler->reset_method == reset_ctl->active_reset) {
>> dev_dbg(adev->dev, "Resetting device\n");
>> handler->do_reset(adev);
>> @@ -272,6 +271,11 @@ static struct amdgpu_reset_handler
>> smu_v13_0_10_mode2_handler = {
>> .do_reset = smu_v13_0_10_mode2_reset,
>> };
>> +static struct amdgpu_reset_handler
>> + *smu_v13_0_10_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
>> + &smu_v13_0_10_mode2_handler,
>> + };
>> +
>> int smu_v13_0_10_reset_init(struct amdgpu_device *adev)
>> {
>> struct amdgpu_reset_control *reset_ctl;
>> @@ -285,10 +289,9 @@ int smu_v13_0_10_reset_init(struct amdgpu_device
>> *adev)
>> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
>> reset_ctl->get_reset_handler = smu_v13_0_10_get_reset_handler;
>> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
>> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
>> /* Only mode2 is handled through reset control now */
>> - amdgpu_reset_add_handler(reset_ctl, &smu_v13_0_10_mode2_handler);
>> + reset_ctl->reset_handlers = &smu_v13_0_10_rst_handlers;
>> adev->reset_cntl = reset_ctl;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: Keep reset handlers shared
2023-08-10 11:44 [PATCH] drm/amdgpu: Keep reset handlers shared Lijo Lazar
2023-08-10 15:11 ` Christian König
@ 2023-08-16 5:38 ` Lazar, Lijo
2023-08-16 10:46 ` Ma, Le
1 sibling, 1 reply; 6+ messages in thread
From: Lazar, Lijo @ 2023-08-16 5:38 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander, Kamal, Asad, Zhang, Hawking
[AMD Official Use Only - General]
<ping>
Thanks,
Lijo
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Lijo Lazar
Sent: Thursday, August 10, 2023 5:14 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
Subject: [PATCH] drm/amdgpu: Keep reset handlers shared
Instead of maintaining a list per device, keep the reset handlers common per ASIC family. A pointer to the list of handlers is maintained in reset control.
Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
---
drivers/gpu/drm/amd/amdgpu/aldebaran.c | 19 +++++++++++--------
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 8 --------
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 16 ++++++++++++----
drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c | 20 +++++++++++---------
drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c | 19 +++++++++++--------
5 files changed, 45 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/aldebaran.c b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
index 2b97b8a96fb4..82e1c83a7ccc 100644
--- a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
+++ b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
@@ -48,20 +48,19 @@ aldebaran_get_reset_handler(struct amdgpu_reset_control *reset_ctl, {
struct amdgpu_reset_handler *handler;
struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+ int i;
if (reset_context->method != AMD_RESET_METHOD_NONE) {
dev_dbg(adev->dev, "Getting reset handler for method %d\n",
reset_context->method);
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_context->method)
return handler;
}
}
if (aldebaran_is_mode2_default(reset_ctl)) {
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == AMD_RESET_METHOD_MODE2) {
reset_context->method = AMD_RESET_METHOD_MODE2;
return handler;
@@ -124,9 +123,9 @@ static void aldebaran_async_reset(struct work_struct *work)
struct amdgpu_reset_control *reset_ctl =
container_of(work, struct amdgpu_reset_control, reset_work);
struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+ int i;
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_ctl->active_reset) {
dev_dbg(adev->dev, "Resetting device\n");
handler->do_reset(adev);
@@ -395,6 +394,11 @@ static struct amdgpu_reset_handler aldebaran_mode2_handler = {
.do_reset = aldebaran_mode2_reset,
};
+static struct amdgpu_reset_handler
+ *aldebaran_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
+ &aldebaran_mode2_handler,
+ };
+
int aldebaran_reset_init(struct amdgpu_device *adev) {
struct amdgpu_reset_control *reset_ctl; @@ -408,10 +412,9 @@ int aldebaran_reset_init(struct amdgpu_device *adev)
reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
reset_ctl->get_reset_handler = aldebaran_get_reset_handler;
- INIT_LIST_HEAD(&reset_ctl->reset_handlers);
INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
/* Only mode2 is handled through reset control now */
- amdgpu_reset_add_handler(reset_ctl, &aldebaran_mode2_handler);
+ reset_ctl->reset_handlers = &aldebaran_rst_handlers;
adev->reset_cntl = reset_ctl;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
index 5fed06ffcc6b..02d874799c16 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
@@ -26,14 +26,6 @@
#include "sienna_cichlid.h"
#include "smu_v13_0_10.h"
-int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
- struct amdgpu_reset_handler *handler)
-{
- /* TODO: Check if handler exists? */
- list_add_tail(&handler->handler_list, &reset_ctl->reset_handlers);
- return 0;
-}
-
int amdgpu_reset_init(struct amdgpu_device *adev) {
int ret = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
index f4a501ff87d9..471d789b33a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
@@ -26,6 +26,8 @@
#include "amdgpu.h"
+#define AMDGPU_RESET_MAX_HANDLERS 5
+
enum AMDGPU_RESET_FLAGS {
AMDGPU_NEED_FULL_RESET = 0,
@@ -44,7 +46,6 @@ struct amdgpu_reset_context {
struct amdgpu_reset_handler {
enum amd_reset_method reset_method;
- struct list_head handler_list;
int (*prepare_env)(struct amdgpu_reset_control *reset_ctl,
struct amdgpu_reset_context *context);
int (*prepare_hwcontext)(struct amdgpu_reset_control *reset_ctl, @@ -63,7 +64,8 @@ struct amdgpu_reset_control {
void *handle;
struct work_struct reset_work;
struct mutex reset_lock;
- struct list_head reset_handlers;
+ struct amdgpu_reset_handler *(
+ *reset_handlers)[AMDGPU_RESET_MAX_HANDLERS];
atomic_t in_reset;
enum amd_reset_method active_reset;
struct amdgpu_reset_handler *(*get_reset_handler)( @@ -97,8 +99,10 @@ int amdgpu_reset_prepare_hwcontext(struct amdgpu_device *adev, int amdgpu_reset_perform_reset(struct amdgpu_device *adev,
struct amdgpu_reset_context *reset_context);
-int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
- struct amdgpu_reset_handler *handler);
+int amdgpu_reset_prepare_env(struct amdgpu_device *adev,
+ struct amdgpu_reset_context *reset_context); int
+amdgpu_reset_restore_env(struct amdgpu_device *adev,
+ struct amdgpu_reset_context *reset_context);
struct amdgpu_reset_domain *amdgpu_reset_create_reset_domain(enum amdgpu_reset_domain_type type,
char *wq_name);
@@ -126,4 +130,8 @@ void amdgpu_device_lock_reset_domain(struct amdgpu_reset_domain *reset_domain);
void amdgpu_device_unlock_reset_domain(struct amdgpu_reset_domain *reset_domain);
+#define for_each_handler(i, handler, reset_ctl) \
+ for (i = 0; (i < AMDGPU_RESET_MAX_HANDLERS) && \
+ (handler = (*reset_ctl->reset_handlers)[i]); \
+ ++i)
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
index 8b8086d5c864..07ded70f4df9 100644
--- a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
+++ b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
@@ -48,18 +48,17 @@ sienna_cichlid_get_reset_handler(struct amdgpu_reset_control *reset_ctl,
struct amdgpu_reset_context *reset_context) {
struct amdgpu_reset_handler *handler;
+ int i;
if (reset_context->method != AMD_RESET_METHOD_NONE) {
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_context->method)
return handler;
}
}
if (sienna_cichlid_is_mode2_default(reset_ctl)) {
- list_for_each_entry (handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == AMD_RESET_METHOD_MODE2)
return handler;
}
@@ -120,9 +119,9 @@ static void sienna_cichlid_async_reset(struct work_struct *work)
struct amdgpu_reset_control *reset_ctl =
container_of(work, struct amdgpu_reset_control, reset_work);
struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+ int i;
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_ctl->active_reset) {
dev_dbg(adev->dev, "Resetting device\n");
handler->do_reset(adev);
@@ -281,6 +280,11 @@ static struct amdgpu_reset_handler sienna_cichlid_mode2_handler = {
.do_reset = sienna_cichlid_mode2_reset,
};
+static struct amdgpu_reset_handler
+ *sienna_cichlid_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
+ &sienna_cichlid_mode2_handler,
+ };
+
int sienna_cichlid_reset_init(struct amdgpu_device *adev) {
struct amdgpu_reset_control *reset_ctl; @@ -294,11 +298,9 @@ int sienna_cichlid_reset_init(struct amdgpu_device *adev)
reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
reset_ctl->get_reset_handler = sienna_cichlid_get_reset_handler;
- INIT_LIST_HEAD(&reset_ctl->reset_handlers);
INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
/* Only mode2 is handled through reset control now */
- amdgpu_reset_add_handler(reset_ctl, &sienna_cichlid_mode2_handler);
-
+ reset_ctl->reset_handlers = &sienna_cichlid_rst_handlers;
adev->reset_cntl = reset_ctl;
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
index ae29620b1ea4..04c797d54511 100644
--- a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
+++ b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
@@ -44,10 +44,10 @@ smu_v13_0_10_get_reset_handler(struct amdgpu_reset_control *reset_ctl, {
struct amdgpu_reset_handler *handler;
struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+ int i;
if (reset_context->method != AMD_RESET_METHOD_NONE) {
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_context->method)
return handler;
}
@@ -55,8 +55,7 @@ smu_v13_0_10_get_reset_handler(struct amdgpu_reset_control *reset_ctl,
if (smu_v13_0_10_is_mode2_default(reset_ctl) &&
amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_MODE2) {
- list_for_each_entry (handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == AMD_RESET_METHOD_MODE2)
return handler;
}
@@ -119,9 +118,9 @@ static void smu_v13_0_10_async_reset(struct work_struct *work)
struct amdgpu_reset_control *reset_ctl =
container_of(work, struct amdgpu_reset_control, reset_work);
struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+ int i;
- list_for_each_entry(handler, &reset_ctl->reset_handlers,
- handler_list) {
+ for_each_handler(i, handler, reset_ctl) {
if (handler->reset_method == reset_ctl->active_reset) {
dev_dbg(adev->dev, "Resetting device\n");
handler->do_reset(adev);
@@ -272,6 +271,11 @@ static struct amdgpu_reset_handler smu_v13_0_10_mode2_handler = {
.do_reset = smu_v13_0_10_mode2_reset,
};
+static struct amdgpu_reset_handler
+ *smu_v13_0_10_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
+ &smu_v13_0_10_mode2_handler,
+ };
+
int smu_v13_0_10_reset_init(struct amdgpu_device *adev) {
struct amdgpu_reset_control *reset_ctl; @@ -285,10 +289,9 @@ int smu_v13_0_10_reset_init(struct amdgpu_device *adev)
reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
reset_ctl->get_reset_handler = smu_v13_0_10_get_reset_handler;
- INIT_LIST_HEAD(&reset_ctl->reset_handlers);
INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
/* Only mode2 is handled through reset control now */
- amdgpu_reset_add_handler(reset_ctl, &smu_v13_0_10_mode2_handler);
+ reset_ctl->reset_handlers = &smu_v13_0_10_rst_handlers;
adev->reset_cntl = reset_ctl;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: Keep reset handlers shared
2023-08-16 5:38 ` Lazar, Lijo
@ 2023-08-16 10:46 ` Ma, Le
2023-08-16 13:20 ` Kamal, Asad
0 siblings, 1 reply; 6+ messages in thread
From: Ma, Le @ 2023-08-16 10:46 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander, Kamal, Asad, Zhang, Hawking
[AMD Official Use Only - General]
Reviewed-by: Le Ma <le.ma@amd.com>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Lazar,
> Lijo
> Sent: Wednesday, August 16, 2023 1:38 PM
> To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Kamal, Asad
> <Asad.Kamal@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
> Subject: RE: [PATCH] drm/amdgpu: Keep reset handlers shared
>
> [AMD Official Use Only - General]
>
> [AMD Official Use Only - General]
>
> <ping>
>
> Thanks,
> Lijo
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Lijo
> Lazar
> Sent: Thursday, August 10, 2023 5:14 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Kamal, Asad
> <Asad.Kamal@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
> Subject: [PATCH] drm/amdgpu: Keep reset handlers shared
>
> Instead of maintaining a list per device, keep the reset handlers common per
> ASIC family. A pointer to the list of handlers is maintained in reset control.
>
> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/aldebaran.c | 19 +++++++++++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 8 --------
> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 16 ++++++++++++----
> drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c | 20 +++++++++++---------
> drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c | 19 +++++++++++--------
> 5 files changed, 45 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> index 2b97b8a96fb4..82e1c83a7ccc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> +++ b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> @@ -48,20 +48,19 @@ aldebaran_get_reset_handler(struct
> amdgpu_reset_control *reset_ctl, {
> struct amdgpu_reset_handler *handler;
> struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
> + int i;
>
> if (reset_context->method != AMD_RESET_METHOD_NONE) {
> dev_dbg(adev->dev, "Getting reset handler for method %d\n",
> reset_context->method);
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_context->method)
> return handler;
> }
> }
>
> if (aldebaran_is_mode2_default(reset_ctl)) {
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == AMD_RESET_METHOD_MODE2) {
> reset_context->method = AMD_RESET_METHOD_MODE2;
> return handler; @@ -124,9 +123,9 @@ static void
> aldebaran_async_reset(struct work_struct *work)
> struct amdgpu_reset_control *reset_ctl =
> container_of(work, struct amdgpu_reset_control, reset_work);
> struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
> + int i;
>
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_ctl->active_reset) {
> dev_dbg(adev->dev, "Resetting device\n");
> handler->do_reset(adev); @@ -395,6 +394,11 @@ static struct
> amdgpu_reset_handler aldebaran_mode2_handler = {
> .do_reset = aldebaran_mode2_reset,
> };
>
> +static struct amdgpu_reset_handler
> + *aldebaran_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
> + &aldebaran_mode2_handler,
> + };
> +
> int aldebaran_reset_init(struct amdgpu_device *adev) {
> struct amdgpu_reset_control *reset_ctl; @@ -408,10 +412,9 @@ int
> aldebaran_reset_init(struct amdgpu_device *adev)
> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
> reset_ctl->get_reset_handler = aldebaran_get_reset_handler;
>
> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
> /* Only mode2 is handled through reset control now */
> - amdgpu_reset_add_handler(reset_ctl, &aldebaran_mode2_handler);
> + reset_ctl->reset_handlers = &aldebaran_rst_handlers;
>
> adev->reset_cntl = reset_ctl;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> index 5fed06ffcc6b..02d874799c16 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> @@ -26,14 +26,6 @@
> #include "sienna_cichlid.h"
> #include "smu_v13_0_10.h"
>
> -int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
> - struct amdgpu_reset_handler *handler)
> -{
> - /* TODO: Check if handler exists? */
> - list_add_tail(&handler->handler_list, &reset_ctl->reset_handlers);
> - return 0;
> -}
> -
> int amdgpu_reset_init(struct amdgpu_device *adev) {
> int ret = 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> index f4a501ff87d9..471d789b33a5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> @@ -26,6 +26,8 @@
>
> #include "amdgpu.h"
>
> +#define AMDGPU_RESET_MAX_HANDLERS 5
> +
> enum AMDGPU_RESET_FLAGS {
>
> AMDGPU_NEED_FULL_RESET = 0,
> @@ -44,7 +46,6 @@ struct amdgpu_reset_context {
>
> struct amdgpu_reset_handler {
> enum amd_reset_method reset_method;
> - struct list_head handler_list;
> int (*prepare_env)(struct amdgpu_reset_control *reset_ctl,
> struct amdgpu_reset_context *context);
> int (*prepare_hwcontext)(struct amdgpu_reset_control *reset_ctl, @@ -
> 63,7 +64,8 @@ struct amdgpu_reset_control {
> void *handle;
> struct work_struct reset_work;
> struct mutex reset_lock;
> - struct list_head reset_handlers;
> + struct amdgpu_reset_handler *(
> + *reset_handlers)[AMDGPU_RESET_MAX_HANDLERS];
> atomic_t in_reset;
> enum amd_reset_method active_reset;
> struct amdgpu_reset_handler *(*get_reset_handler)( @@ -97,8 +99,10 @@
> int amdgpu_reset_prepare_hwcontext(struct amdgpu_device *adev, int
> amdgpu_reset_perform_reset(struct amdgpu_device *adev,
> struct amdgpu_reset_context *reset_context);
>
> -int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
> - struct amdgpu_reset_handler *handler);
> +int amdgpu_reset_prepare_env(struct amdgpu_device *adev,
> + struct amdgpu_reset_context
> +*reset_context); int amdgpu_reset_restore_env(struct amdgpu_device *adev,
> + struct amdgpu_reset_context
> +*reset_context);
>
> struct amdgpu_reset_domain *amdgpu_reset_create_reset_domain(enum
> amdgpu_reset_domain_type type,
> char *wq_name); @@ -126,4 +130,8 @@ void
> amdgpu_device_lock_reset_domain(struct amdgpu_reset_domain
> *reset_domain);
>
> void amdgpu_device_unlock_reset_domain(struct amdgpu_reset_domain
> *reset_domain);
>
> +#define for_each_handler(i, handler, reset_ctl) \
> + for (i = 0; (i < AMDGPU_RESET_MAX_HANDLERS) && \
> + (handler = (*reset_ctl->reset_handlers)[i]); \
> + ++i)
> #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> index 8b8086d5c864..07ded70f4df9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> @@ -48,18 +48,17 @@ sienna_cichlid_get_reset_handler(struct
> amdgpu_reset_control *reset_ctl,
> struct amdgpu_reset_context *reset_context) {
> struct amdgpu_reset_handler *handler;
> + int i;
>
> if (reset_context->method != AMD_RESET_METHOD_NONE) {
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_context->method)
> return handler;
> }
> }
>
> if (sienna_cichlid_is_mode2_default(reset_ctl)) {
> - list_for_each_entry (handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == AMD_RESET_METHOD_MODE2)
> return handler;
> }
> @@ -120,9 +119,9 @@ static void sienna_cichlid_async_reset(struct
> work_struct *work)
> struct amdgpu_reset_control *reset_ctl =
> container_of(work, struct amdgpu_reset_control, reset_work);
> struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
> + int i;
>
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_ctl->active_reset) {
> dev_dbg(adev->dev, "Resetting device\n");
> handler->do_reset(adev); @@ -281,6 +280,11 @@ static struct
> amdgpu_reset_handler sienna_cichlid_mode2_handler = {
> .do_reset = sienna_cichlid_mode2_reset,
> };
>
> +static struct amdgpu_reset_handler
> + *sienna_cichlid_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
> + &sienna_cichlid_mode2_handler,
> + };
> +
> int sienna_cichlid_reset_init(struct amdgpu_device *adev) {
> struct amdgpu_reset_control *reset_ctl; @@ -294,11 +298,9 @@ int
> sienna_cichlid_reset_init(struct amdgpu_device *adev)
> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
> reset_ctl->get_reset_handler = sienna_cichlid_get_reset_handler;
>
> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
> /* Only mode2 is handled through reset control now */
> - amdgpu_reset_add_handler(reset_ctl, &sienna_cichlid_mode2_handler);
> -
> + reset_ctl->reset_handlers = &sienna_cichlid_rst_handlers;
> adev->reset_cntl = reset_ctl;
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> index ae29620b1ea4..04c797d54511 100644
> --- a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> +++ b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> @@ -44,10 +44,10 @@ smu_v13_0_10_get_reset_handler(struct
> amdgpu_reset_control *reset_ctl, {
> struct amdgpu_reset_handler *handler;
> struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
> + int i;
>
> if (reset_context->method != AMD_RESET_METHOD_NONE) {
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_context->method)
> return handler;
> }
> @@ -55,8 +55,7 @@ smu_v13_0_10_get_reset_handler(struct
> amdgpu_reset_control *reset_ctl,
>
> if (smu_v13_0_10_is_mode2_default(reset_ctl) &&
> amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_MODE2)
> {
> - list_for_each_entry (handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == AMD_RESET_METHOD_MODE2)
> return handler;
> }
> @@ -119,9 +118,9 @@ static void smu_v13_0_10_async_reset(struct
> work_struct *work)
> struct amdgpu_reset_control *reset_ctl =
> container_of(work, struct amdgpu_reset_control, reset_work);
> struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
> + int i;
>
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_ctl->active_reset) {
> dev_dbg(adev->dev, "Resetting device\n");
> handler->do_reset(adev); @@ -272,6 +271,11 @@ static struct
> amdgpu_reset_handler smu_v13_0_10_mode2_handler = {
> .do_reset = smu_v13_0_10_mode2_reset,
> };
>
> +static struct amdgpu_reset_handler
> + *smu_v13_0_10_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
> + &smu_v13_0_10_mode2_handler,
> + };
> +
> int smu_v13_0_10_reset_init(struct amdgpu_device *adev) {
> struct amdgpu_reset_control *reset_ctl; @@ -285,10 +289,9 @@ int
> smu_v13_0_10_reset_init(struct amdgpu_device *adev)
> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
> reset_ctl->get_reset_handler = smu_v13_0_10_get_reset_handler;
>
> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
> /* Only mode2 is handled through reset control now */
> - amdgpu_reset_add_handler(reset_ctl, &smu_v13_0_10_mode2_handler);
> + reset_ctl->reset_handlers = &smu_v13_0_10_rst_handlers;
>
> adev->reset_cntl = reset_ctl;
>
> --
> 2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: Keep reset handlers shared
2023-08-16 10:46 ` Ma, Le
@ 2023-08-16 13:20 ` Kamal, Asad
0 siblings, 0 replies; 6+ messages in thread
From: Kamal, Asad @ 2023-08-16 13:20 UTC (permalink / raw)
To: Ma, Le, Lazar, Lijo, amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander, Zhang, Hawking
[AMD Official Use Only - General]
Reviewed-by: Asad Kamal <asad.kamal@amd.com>
Tested-by: Asad Kamal <asad.kamal@amd.com>
Thanks & Regards
Asad
-----Original Message-----
From: Ma, Le <Le.Ma@amd.com>
Sent: Wednesday, August 16, 2023 4:17 PM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
Subject: RE: [PATCH] drm/amdgpu: Keep reset handlers shared
[AMD Official Use Only - General]
Reviewed-by: Le Ma <le.ma@amd.com>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Lazar, Lijo
> Sent: Wednesday, August 16, 2023 1:38 PM
> To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Kamal, Asad
> <Asad.Kamal@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
> Subject: RE: [PATCH] drm/amdgpu: Keep reset handlers shared
>
> [AMD Official Use Only - General]
>
> [AMD Official Use Only - General]
>
> <ping>
>
> Thanks,
> Lijo
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Lijo Lazar
> Sent: Thursday, August 10, 2023 5:14 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Kamal, Asad
> <Asad.Kamal@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
> Subject: [PATCH] drm/amdgpu: Keep reset handlers shared
>
> Instead of maintaining a list per device, keep the reset handlers
> common per ASIC family. A pointer to the list of handlers is maintained in reset control.
>
> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/aldebaran.c | 19 +++++++++++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c | 8 --------
> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 16 ++++++++++++----
> drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c | 20 +++++++++++---------
> drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c | 19 +++++++++++--------
> 5 files changed, 45 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> index 2b97b8a96fb4..82e1c83a7ccc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> +++ b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
> @@ -48,20 +48,19 @@ aldebaran_get_reset_handler(struct
> amdgpu_reset_control *reset_ctl, {
> struct amdgpu_reset_handler *handler;
> struct amdgpu_device *adev = (struct amdgpu_device
> *)reset_ctl->handle;
> + int i;
>
> if (reset_context->method != AMD_RESET_METHOD_NONE) {
> dev_dbg(adev->dev, "Getting reset handler for method %d\n",
> reset_context->method);
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_context->method)
> return handler;
> }
> }
>
> if (aldebaran_is_mode2_default(reset_ctl)) {
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == AMD_RESET_METHOD_MODE2) {
> reset_context->method = AMD_RESET_METHOD_MODE2;
> return handler; @@ -124,9 +123,9 @@
> static void aldebaran_async_reset(struct work_struct *work)
> struct amdgpu_reset_control *reset_ctl =
> container_of(work, struct amdgpu_reset_control, reset_work);
> struct amdgpu_device *adev = (struct amdgpu_device
> *)reset_ctl->handle;
> + int i;
>
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_ctl->active_reset) {
> dev_dbg(adev->dev, "Resetting device\n");
> handler->do_reset(adev); @@ -395,6 +394,11 @@
> static struct amdgpu_reset_handler aldebaran_mode2_handler = {
> .do_reset = aldebaran_mode2_reset,
> };
>
> +static struct amdgpu_reset_handler
> + *aldebaran_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
> + &aldebaran_mode2_handler,
> + };
> +
> int aldebaran_reset_init(struct amdgpu_device *adev) {
> struct amdgpu_reset_control *reset_ctl; @@ -408,10 +412,9 @@
> int aldebaran_reset_init(struct amdgpu_device *adev)
> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
> reset_ctl->get_reset_handler = aldebaran_get_reset_handler;
>
> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
> /* Only mode2 is handled through reset control now */
> - amdgpu_reset_add_handler(reset_ctl, &aldebaran_mode2_handler);
> + reset_ctl->reset_handlers = &aldebaran_rst_handlers;
>
> adev->reset_cntl = reset_ctl;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> index 5fed06ffcc6b..02d874799c16 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
> @@ -26,14 +26,6 @@
> #include "sienna_cichlid.h"
> #include "smu_v13_0_10.h"
>
> -int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
> - struct amdgpu_reset_handler *handler)
> -{
> - /* TODO: Check if handler exists? */
> - list_add_tail(&handler->handler_list, &reset_ctl->reset_handlers);
> - return 0;
> -}
> -
> int amdgpu_reset_init(struct amdgpu_device *adev) {
> int ret = 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> index f4a501ff87d9..471d789b33a5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> @@ -26,6 +26,8 @@
>
> #include "amdgpu.h"
>
> +#define AMDGPU_RESET_MAX_HANDLERS 5
> +
> enum AMDGPU_RESET_FLAGS {
>
> AMDGPU_NEED_FULL_RESET = 0,
> @@ -44,7 +46,6 @@ struct amdgpu_reset_context {
>
> struct amdgpu_reset_handler {
> enum amd_reset_method reset_method;
> - struct list_head handler_list;
> int (*prepare_env)(struct amdgpu_reset_control *reset_ctl,
> struct amdgpu_reset_context *context);
> int (*prepare_hwcontext)(struct amdgpu_reset_control
> *reset_ctl, @@ -
> 63,7 +64,8 @@ struct amdgpu_reset_control {
> void *handle;
> struct work_struct reset_work;
> struct mutex reset_lock;
> - struct list_head reset_handlers;
> + struct amdgpu_reset_handler *(
> + *reset_handlers)[AMDGPU_RESET_MAX_HANDLERS];
> atomic_t in_reset;
> enum amd_reset_method active_reset;
> struct amdgpu_reset_handler *(*get_reset_handler)( @@ -97,8
> +99,10 @@ int amdgpu_reset_prepare_hwcontext(struct amdgpu_device
> *adev, int amdgpu_reset_perform_reset(struct amdgpu_device *adev,
> struct amdgpu_reset_context
> *reset_context);
>
> -int amdgpu_reset_add_handler(struct amdgpu_reset_control *reset_ctl,
> - struct amdgpu_reset_handler *handler);
> +int amdgpu_reset_prepare_env(struct amdgpu_device *adev,
> + struct amdgpu_reset_context
> +*reset_context); int amdgpu_reset_restore_env(struct amdgpu_device *adev,
> + struct amdgpu_reset_context
> +*reset_context);
>
> struct amdgpu_reset_domain *amdgpu_reset_create_reset_domain(enum
> amdgpu_reset_domain_type type,
> char
> *wq_name); @@ -126,4 +130,8 @@ void
> amdgpu_device_lock_reset_domain(struct amdgpu_reset_domain
> *reset_domain);
>
> void amdgpu_device_unlock_reset_domain(struct amdgpu_reset_domain
> *reset_domain);
>
> +#define for_each_handler(i, handler, reset_ctl) \
> + for (i = 0; (i < AMDGPU_RESET_MAX_HANDLERS) && \
> + (handler = (*reset_ctl->reset_handlers)[i]); \
> + ++i)
> #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> index 8b8086d5c864..07ded70f4df9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
> @@ -48,18 +48,17 @@ sienna_cichlid_get_reset_handler(struct
> amdgpu_reset_control *reset_ctl,
> struct amdgpu_reset_context *reset_context) {
> struct amdgpu_reset_handler *handler;
> + int i;
>
> if (reset_context->method != AMD_RESET_METHOD_NONE) {
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_context->method)
> return handler;
> }
> }
>
> if (sienna_cichlid_is_mode2_default(reset_ctl)) {
> - list_for_each_entry (handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == AMD_RESET_METHOD_MODE2)
> return handler;
> }
> @@ -120,9 +119,9 @@ static void sienna_cichlid_async_reset(struct
> work_struct *work)
> struct amdgpu_reset_control *reset_ctl =
> container_of(work, struct amdgpu_reset_control, reset_work);
> struct amdgpu_device *adev = (struct amdgpu_device
> *)reset_ctl->handle;
> + int i;
>
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_ctl->active_reset) {
> dev_dbg(adev->dev, "Resetting device\n");
> handler->do_reset(adev); @@ -281,6 +280,11 @@
> static struct amdgpu_reset_handler sienna_cichlid_mode2_handler = {
> .do_reset = sienna_cichlid_mode2_reset,
> };
>
> +static struct amdgpu_reset_handler
> + *sienna_cichlid_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
> + &sienna_cichlid_mode2_handler,
> + };
> +
> int sienna_cichlid_reset_init(struct amdgpu_device *adev) {
> struct amdgpu_reset_control *reset_ctl; @@ -294,11 +298,9 @@
> int sienna_cichlid_reset_init(struct amdgpu_device *adev)
> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
> reset_ctl->get_reset_handler =
> sienna_cichlid_get_reset_handler;
>
> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
> /* Only mode2 is handled through reset control now */
> - amdgpu_reset_add_handler(reset_ctl, &sienna_cichlid_mode2_handler);
> -
> + reset_ctl->reset_handlers = &sienna_cichlid_rst_handlers;
> adev->reset_cntl = reset_ctl;
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> index ae29620b1ea4..04c797d54511 100644
> --- a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> +++ b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
> @@ -44,10 +44,10 @@ smu_v13_0_10_get_reset_handler(struct
> amdgpu_reset_control *reset_ctl, {
> struct amdgpu_reset_handler *handler;
> struct amdgpu_device *adev = (struct amdgpu_device
> *)reset_ctl->handle;
> + int i;
>
> if (reset_context->method != AMD_RESET_METHOD_NONE) {
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_context->method)
> return handler;
> }
> @@ -55,8 +55,7 @@ smu_v13_0_10_get_reset_handler(struct
> amdgpu_reset_control *reset_ctl,
>
> if (smu_v13_0_10_is_mode2_default(reset_ctl) &&
> amdgpu_asic_reset_method(adev) ==
> AMD_RESET_METHOD_MODE2) {
> - list_for_each_entry (handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == AMD_RESET_METHOD_MODE2)
> return handler;
> }
> @@ -119,9 +118,9 @@ static void smu_v13_0_10_async_reset(struct
> work_struct *work)
> struct amdgpu_reset_control *reset_ctl =
> container_of(work, struct amdgpu_reset_control, reset_work);
> struct amdgpu_device *adev = (struct amdgpu_device
> *)reset_ctl->handle;
> + int i;
>
> - list_for_each_entry(handler, &reset_ctl->reset_handlers,
> - handler_list) {
> + for_each_handler(i, handler, reset_ctl) {
> if (handler->reset_method == reset_ctl->active_reset) {
> dev_dbg(adev->dev, "Resetting device\n");
> handler->do_reset(adev); @@ -272,6 +271,11 @@
> static struct amdgpu_reset_handler smu_v13_0_10_mode2_handler = {
> .do_reset = smu_v13_0_10_mode2_reset,
> };
>
> +static struct amdgpu_reset_handler
> + *smu_v13_0_10_rst_handlers[AMDGPU_RESET_MAX_HANDLERS] = {
> + &smu_v13_0_10_mode2_handler,
> + };
> +
> int smu_v13_0_10_reset_init(struct amdgpu_device *adev) {
> struct amdgpu_reset_control *reset_ctl; @@ -285,10 +289,9 @@
> int smu_v13_0_10_reset_init(struct amdgpu_device *adev)
> reset_ctl->active_reset = AMD_RESET_METHOD_NONE;
> reset_ctl->get_reset_handler = smu_v13_0_10_get_reset_handler;
>
> - INIT_LIST_HEAD(&reset_ctl->reset_handlers);
> INIT_WORK(&reset_ctl->reset_work, reset_ctl->async_reset);
> /* Only mode2 is handled through reset control now */
> - amdgpu_reset_add_handler(reset_ctl, &smu_v13_0_10_mode2_handler);
> + reset_ctl->reset_handlers = &smu_v13_0_10_rst_handlers;
>
> adev->reset_cntl = reset_ctl;
>
> --
> 2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-16 13:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-10 11:44 [PATCH] drm/amdgpu: Keep reset handlers shared Lijo Lazar
2023-08-10 15:11 ` Christian König
2023-08-10 17:13 ` Lazar, Lijo
2023-08-16 5:38 ` Lazar, Lijo
2023-08-16 10:46 ` Ma, Le
2023-08-16 13:20 ` Kamal, Asad
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox