Re: [PATCH] drm/amd/amdgpu: Merge the smc/pcie/didt debugfs read/write methods

Wed, 04 Apr 2018 07:04:11 -0700 

Hi Christian,
I was thinking of merging the didt/smc/pcie ops into one themselves. I'll see what I can come up with without spending too much time on it. 

Cheers,
Tom

On 04/04/2018 10:02 AM, Christian König wrote:

Am 04.04.2018 um 15:35 schrieb Tom St Denis:

Fold the read/write methods of the various register access
debugfs entries into op functions.

Signed-off-by: Tom St Denis <[email protected]>


I would rather split out a dword helper. E.g. something like:


amdgpu_debugfs_regs_dword_op(struct file *f, char __user *buf, size_t 
size, loff_t *pos,
                                                         int 
(*op)(struct amdgpu_device *adev, char __user *buf))

{
...
}


And then you have op callbacks for PCIE, didt, SMC each in a read/write 
variant.


Would save quite a bunch more of loc if I'm not completely mistaken.

Christian.


---

  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 140 
+++++++++++-----------------

  1 file changed, 57 insertions(+), 83 deletions(-)


diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c

index c98e59721444..b80e18469d25 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c

@@ -177,8 +177,8 @@ static ssize_t amdgpu_debugfs_regs_write(struct 
file *f, const char __user *buf,
      return amdgpu_debugfs_process_reg_op(false, f, (char __user 
*)buf, size, pos);

  }

-static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char 
__user *buf,

-                    size_t size, loff_t *pos)
+static ssize_t amdgpu_debugfs_regs_pcie_op(bool read, struct file *f,
+                    char __user *buf, size_t size, loff_t *pos)
  {
      struct amdgpu_device *adev = file_inode(f)->i_private;
      ssize_t result = 0;

@@ -190,11 +190,18 @@ static ssize_t 
amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf,

      while (size) {
          uint32_t value;
-        value = RREG32_PCIE(*pos >> 2);
-        r = put_user(value, (uint32_t *)buf);
-        if (r)
-            return r;
+        if (read) {
+            value = RREG32_PCIE(*pos >> 2);
+            r = put_user(value, (uint32_t *)buf);
+            if (r)
+                return r;
+        } else {
+            r = get_user(value, (uint32_t *)buf);
+            if (r)
+                return r;
+            WREG32_PCIE(*pos >> 2, value);
+        }
          result += 4;
          buf += 4;
          *pos += 4;

@@ -204,8 +211,20 @@ static ssize_t 
amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf,

      return result;
  }

+static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char 
__user *buf,

+                    size_t size, loff_t *pos)
+{
+    return amdgpu_debugfs_regs_pcie_op(true, f, buf, size, pos);
+}
+

  static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const 
char __user *buf,

                       size_t size, loff_t *pos)
+{

+    return amdgpu_debugfs_regs_pcie_op(false, f, (char __user *)buf, 
size, pos);

+}
+
+static ssize_t amdgpu_debugfs_regs_didt_op(bool read, struct file *f,
+                    char __user *buf, size_t size, loff_t *pos)
  {
      struct amdgpu_device *adev = file_inode(f)->i_private;
      ssize_t result = 0;

@@ -217,12 +236,18 @@ static ssize_t 
amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user

      while (size) {
          uint32_t value;
-        r = get_user(value, (uint32_t *)buf);
-        if (r)
-            return r;
-
-        WREG32_PCIE(*pos >> 2, value);
+        if (read) {
+            value = RREG32_DIDT(*pos >> 2);
+            r = put_user(value, (uint32_t *)buf);
+            if (r)
+                return r;
+        } else {
+            r = get_user(value, (uint32_t *)buf);
+            if (r)
+                return r;
+            WREG32_DIDT(*pos >> 2, value);
+        }
          result += 4;
          buf += 4;
          *pos += 4;

@@ -235,32 +260,18 @@ static ssize_t 
amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user
  static ssize_t amdgpu_debugfs_regs_didt_read(struct file *f, char 
__user *buf,

                      size_t size, loff_t *pos)
  {
-    struct amdgpu_device *adev = file_inode(f)->i_private;
-    ssize_t result = 0;
-    int r;
-
-    if (size & 0x3 || *pos & 0x3)
-        return -EINVAL;
-
-    while (size) {
-        uint32_t value;
-
-        value = RREG32_DIDT(*pos >> 2);
-        r = put_user(value, (uint32_t *)buf);
-        if (r)
-            return r;
-
-        result += 4;
-        buf += 4;
-        *pos += 4;
-        size -= 4;
-    }
-
-    return result;
+    return amdgpu_debugfs_regs_didt_op(true, f, buf, size, pos);
  }

  static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const 
char __user *buf,

                       size_t size, loff_t *pos)
+{

+    return amdgpu_debugfs_regs_didt_op(false, f, (char __user *)buf, 
size, pos);

+}
+
+static ssize_t amdgpu_debugfs_regs_smc_op(bool read,
+                    struct file *f, char __user *buf,
+                    size_t size, loff_t *pos)
  {
      struct amdgpu_device *adev = file_inode(f)->i_private;
      ssize_t result = 0;

@@ -272,11 +283,17 @@ static ssize_t 
amdgpu_debugfs_regs_didt_write(struct file *f, const char __user

      while (size) {
          uint32_t value;
-        r = get_user(value, (uint32_t *)buf);
-        if (r)
-            return r;
-
-        WREG32_DIDT(*pos >> 2, value);
+        if (read) {
+            value = RREG32_SMC(*pos);
+            r = put_user(value, (uint32_t *)buf);
+            if (r)
+                return r;
+        } else {
+            r = get_user(value, (uint32_t *)buf);
+            if (r)
+                return r;
+            WREG32_SMC(*pos, value);
+        }
          result += 4;
          buf += 4;

@@ -290,56 +307,13 @@ static ssize_t 
amdgpu_debugfs_regs_didt_write(struct file *f, const char __user
  static ssize_t amdgpu_debugfs_regs_smc_read(struct file *f, char 
__user *buf,

                      size_t size, loff_t *pos)
  {
-    struct amdgpu_device *adev = file_inode(f)->i_private;
-    ssize_t result = 0;
-    int r;
-
-    if (size & 0x3 || *pos & 0x3)
-        return -EINVAL;
-
-    while (size) {
-        uint32_t value;
-
-        value = RREG32_SMC(*pos);
-        r = put_user(value, (uint32_t *)buf);
-        if (r)
-            return r;
-
-        result += 4;
-        buf += 4;
-        *pos += 4;
-        size -= 4;
-    }
-
-    return result;
+    return amdgpu_debugfs_regs_smc_op(true, f, buf, size, pos);
  }

  static ssize_t amdgpu_debugfs_regs_smc_write(struct file *f, const 
char __user *buf,

                       size_t size, loff_t *pos)
  {
-    struct amdgpu_device *adev = file_inode(f)->i_private;
-    ssize_t result = 0;
-    int r;
-
-    if (size & 0x3 || *pos & 0x3)
-        return -EINVAL;
-
-    while (size) {
-        uint32_t value;
-
-        r = get_user(value, (uint32_t *)buf);
-        if (r)
-            return r;
-
-        WREG32_SMC(*pos, value);
-
-        result += 4;
-        buf += 4;
-        *pos += 4;
-        size -= 4;
-    }
-
-    return result;

+    return amdgpu_debugfs_regs_smc_op(false, f, (char __user *)buf, 
size, pos);

  }

  static ssize_t amdgpu_debugfs_gca_config_read(struct file *f, char 
__user *buf,



_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx






















					Reply via email to