On 31 May 2018 at 20:02, Shirish S <shirish.s at amd.com> wrote:
> mutex's lead to sleeps which should be avoided in
> atomic context.
> Hence this patch replaces it with the spin_locks.
>
> Below is the stack trace:
I'm not sure I really like this series of patches, going around
replacing ATOMIC and mutex with spinlocks
isn't something that should be done lightly,
In all the patches I haven't seen what spin lock or what causes us to
be in an atomic state in the first
place and why it is necessary that we are in an atomic state for such
long sequences of code.
Thanks,
Dave.
>
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:**
> in_atomic(): 1, irqs_disabled(): 1, pid: 89, name: kworker/u4:3
> CPU: 1 PID: 89 Comm: kworker/u4:3 Tainted: G W 4.14.43 #8
> Workqueue: events_unbound commit_work
> Call Trace:
> dump_stack+0x4d/0x63
> ___might_sleep+0x11f/0x12e
> mutex_lock+0x20/0x42
> amdgpu_atom_execute_table+0x26/0x72
> enable_disp_power_gating_v2_1+0x85/0xae
> dce110_enable_display_power_gating+0x83/0x1b1
> dce110_power_down_fe+0x4a/0x6d
> dc_post_update_surfaces_to_stream+0x59/0x87
> amdgpu_dm_do_flip+0x239/0x298
> amdgpu_dm_commit_planes.isra.23+0x379/0x54b
> ? drm_calc_timestamping_constants+0x14b/0x15c
> amdgpu_dm_atomic_commit_tail+0x4fc/0x5d2
> ? wait_for_common+0x5b/0x69
> commit_tail+0x42/0x64
> process_one_work+0x1b0/0x314
> worker_thread+0x1cb/0x2c1
> ? create_worker+0x1da/0x1da
> kthread+0x156/0x15e
> ? kthread_flush_work+0xea/0xea
> ret_from_fork+0x22/0x40
>
> V2: Added stack trace in commit message.
>
> Signed-off-by: Shirish S <shirish.s at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/atom.c | 4 ++--
> drivers/gpu/drm/amd/amdgpu/atom.h | 3 ++-
> 3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> index bf872f6..ba3d4b9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> @@ -2033,7 +2033,7 @@ int amdgpu_atombios_init(struct amdgpu_device *adev)
> return -ENOMEM;
> }
>
> - mutex_init(&adev->mode_info.atom_context->mutex);
> + spin_lock_init(&adev->mode_info.atom_context->lock);
> if (adev->is_atom_fw) {
> amdgpu_atomfirmware_scratch_regs_init(adev);
> amdgpu_atomfirmware_allocate_fb_scratch(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
> index 69500a8..bfd98f0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
> @@ -1261,7 +1261,7 @@ int amdgpu_atom_execute_table(struct atom_context *ctx, int index, uint32_t * pa
> {
> int r;
>
> - mutex_lock(&ctx->mutex);
> + spin_lock(&ctx->lock);
> /* reset data block */
> ctx->data_block = 0;
> /* reset reg block */
> @@ -1274,7 +1274,7 @@ int amdgpu_atom_execute_table(struct atom_context *ctx, int index, uint32_t * pa
> ctx->divmul[0] = 0;
> ctx->divmul[1] = 0;
> r = amdgpu_atom_execute_table_locked(ctx, index, params);
> - mutex_unlock(&ctx->mutex);
> + spin_unlock(&ctx->lock);
> return r;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.h b/drivers/gpu/drm/amd/amdgpu/atom.h
> index a391709..54063e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atom.h
> +++ b/drivers/gpu/drm/amd/amdgpu/atom.h
> @@ -26,6 +26,7 @@
> #define ATOM_H
>
> #include <linux/types.h>
> +#include <linux/spinlock_types.h>
> #include <drm/drmP.h>
>
> #define ATOM_BIOS_MAGIC 0xAA55
> @@ -125,7 +126,7 @@ struct card_info {
>
> struct atom_context {
> struct card_info *card;
> - struct mutex mutex;
> + spinlock_t lock;
> void *bios;
> uint32_t cmd_table, data_table;
> uint16_t *iio;
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
