Re: [PATCH] drm/amdgpu: replace mutex with spin_lock

[S, Shirish]

On 5/30/2018 9:10 PM, Christian König wrote:

Keep in mind that under SRIOV you can read registers while in atomic context, e.g. while holding a spinlock.

Please double check if that won't bite us.

Apart from that the change looks good to me,

Thanks Christian, i verified boot, s3, s5 on Stoney with this patch.
Have re-spun V2 which has the exact trace in which scenario this BUG is hit.
Regards,
Shirish S

Christian.

Am 30.05.2018 um 12:19 schrieb Shirish S:

mutex's lead to sleeps which should be avoided in
atomic context.
Hence this patch replaces it with the spin_locks.

Signed-off-by: Shirish S <shiris...@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..cdfb0d0 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;

--
Regards,
Shirish S

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx