* [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