On 10/31/2023 1:45 PM, Jani Nikula wrote:
Hide gpu error specifics in i915_gpu_error.c. This is also cleaner wrt
conditional compilation, as i915_gpu_error.c is only built with
DRM_I915_CAPTURE_ERROR=y.

With this, we can also make i915_first_error_state() static.

Signed-off-by: Jani Nikula <jani.nik...@intel.com>
Reviewed-by: Nirmoy Das <nirmoy....@intel.com>

---
  drivers/gpu/drm/i915/i915_gpu_error.c | 75 ++++++++++++++++++++++++-
  drivers/gpu/drm/i915/i915_gpu_error.h | 17 +++---
  drivers/gpu/drm/i915/i915_sysfs.c     | 79 +--------------------------
  3 files changed, 86 insertions(+), 85 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
b/drivers/gpu/drm/i915/i915_gpu_error.c
index f195df91d9e6..00559a75b798 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -57,6 +57,7 @@
  #include "i915_memcpy.h"
  #include "i915_reg.h"
  #include "i915_scatterlist.h"
+#include "i915_sysfs.h"
  #include "i915_utils.h"
#define ALLOW_FAIL (__GFP_KSWAPD_RECLAIM | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
@@ -2211,7 +2212,7 @@ void i915_capture_error_state(struct intel_gt *gt,
        i915_gpu_coredump_put(error);
  }
-struct i915_gpu_coredump *
+static struct i915_gpu_coredump *
  i915_first_error_state(struct drm_i915_private *i915)
  {
        struct i915_gpu_coredump *error;
@@ -2487,3 +2488,75 @@ void i915_gpu_error_debugfs_register(struct 
drm_i915_private *i915)
        debugfs_create_file("i915_gpu_info", 0644, minor->debugfs_root, i915,
                            &i915_gpu_info_fops);
  }
+
+static ssize_t error_state_read(struct file *filp, struct kobject *kobj,
+                               struct bin_attribute *attr, char *buf,
+                               loff_t off, size_t count)
+{
+
+       struct device *kdev = kobj_to_dev(kobj);
+       struct drm_i915_private *i915 = kdev_minor_to_i915(kdev);
+       struct i915_gpu_coredump *gpu;
+       ssize_t ret = 0;
+
+       /*
+        * FIXME: Concurrent clients triggering resets and reading + clearing
+        * dumps can cause inconsistent sysfs reads when a user calls in with a
+        * non-zero offset to complete a prior partial read but the
+        * gpu_coredump has been cleared or replaced.
+        */
+
+       gpu = i915_first_error_state(i915);
+       if (IS_ERR(gpu)) {
+               ret = PTR_ERR(gpu);
+       } else if (gpu) {
+               ret = i915_gpu_coredump_copy_to_buffer(gpu, buf, off, count);
+               i915_gpu_coredump_put(gpu);
+       } else {
+               const char *str = "No error state collected\n";
+               size_t len = strlen(str);
+
+               if (off < len) {
+                       ret = min_t(size_t, count, len - off);
+                       memcpy(buf, str + off, ret);
+               }
+       }
+
+       return ret;
+}
+
+static ssize_t error_state_write(struct file *file, struct kobject *kobj,
+                                struct bin_attribute *attr, char *buf,
+                                loff_t off, size_t count)
+{
+       struct device *kdev = kobj_to_dev(kobj);
+       struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
+
+       drm_dbg(&dev_priv->drm, "Resetting error state\n");
+       i915_reset_error_state(dev_priv);
+
+       return count;
+}
+
+static const struct bin_attribute error_state_attr = {
+       .attr.name = "error",
+       .attr.mode = S_IRUSR | S_IWUSR,
+       .size = 0,
+       .read = error_state_read,
+       .write = error_state_write,
+};
+
+void i915_gpu_error_sysfs_setup(struct drm_i915_private *i915)
+{
+       struct device *kdev = i915->drm.primary->kdev;
+
+       if (sysfs_create_bin_file(&kdev->kobj, &error_state_attr))
+               drm_err(&i915->drm, "error_state sysfs setup failed\n");
+}
+
+void i915_gpu_error_sysfs_teardown(struct drm_i915_private *i915)
+{
+       struct device *kdev = i915->drm.primary->kdev;
+
+       sysfs_remove_bin_file(&kdev->kobj, &error_state_attr);
+}
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h 
b/drivers/gpu/drm/i915/i915_gpu_error.h
index f851189b0ff1..fa886620d3f8 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.h
+++ b/drivers/gpu/drm/i915/i915_gpu_error.h
@@ -325,11 +325,12 @@ static inline void i915_gpu_coredump_put(struct 
i915_gpu_coredump *gpu)
                kref_put(&gpu->ref, __i915_gpu_coredump_free);
  }
-struct i915_gpu_coredump *i915_first_error_state(struct drm_i915_private *i915);
  void i915_reset_error_state(struct drm_i915_private *i915);
  void i915_disable_error_state(struct drm_i915_private *i915, int err);
void i915_gpu_error_debugfs_register(struct drm_i915_private *i915);
+void i915_gpu_error_sysfs_setup(struct drm_i915_private *i915);
+void i915_gpu_error_sysfs_teardown(struct drm_i915_private *i915);
#else @@ -398,12 +399,6 @@ static inline void i915_gpu_coredump_put(struct i915_gpu_coredump *gpu)
  {
  }
-static inline struct i915_gpu_coredump *
-i915_first_error_state(struct drm_i915_private *i915)
-{
-       return ERR_PTR(-ENODEV);
-}
-
  static inline void i915_reset_error_state(struct drm_i915_private *i915)
  {
  }
@@ -417,6 +412,14 @@ static inline void i915_gpu_error_debugfs_register(struct 
drm_i915_private *i915
  {
  }
+static inline void i915_gpu_error_sysfs_setup(struct drm_i915_private *i915)
+{
+}
+
+static inline void i915_gpu_error_sysfs_teardown(struct drm_i915_private *i915)
+{
+}
+
  #endif /* IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) */
#endif /* _I915_GPU_ERROR_H_ */
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c 
b/drivers/gpu/drm/i915/i915_sysfs.c
index e88bb4f04305..613decd47760 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -155,81 +155,6 @@ static const struct bin_attribute dpf_attrs_1 = {
        .private = (void *)1
  };
-#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
-
-static ssize_t error_state_read(struct file *filp, struct kobject *kobj,
-                               struct bin_attribute *attr, char *buf,
-                               loff_t off, size_t count)
-{
-
-       struct device *kdev = kobj_to_dev(kobj);
-       struct drm_i915_private *i915 = kdev_minor_to_i915(kdev);
-       struct i915_gpu_coredump *gpu;
-       ssize_t ret = 0;
-
-       /*
-        * FIXME: Concurrent clients triggering resets and reading + clearing
-        * dumps can cause inconsistent sysfs reads when a user calls in with a
-        * non-zero offset to complete a prior partial read but the
-        * gpu_coredump has been cleared or replaced.
-        */
-
-       gpu = i915_first_error_state(i915);
-       if (IS_ERR(gpu)) {
-               ret = PTR_ERR(gpu);
-       } else if (gpu) {
-               ret = i915_gpu_coredump_copy_to_buffer(gpu, buf, off, count);
-               i915_gpu_coredump_put(gpu);
-       } else {
-               const char *str = "No error state collected\n";
-               size_t len = strlen(str);
-
-               if (off < len) {
-                       ret = min_t(size_t, count, len - off);
-                       memcpy(buf, str + off, ret);
-               }
-       }
-
-       return ret;
-}
-
-static ssize_t error_state_write(struct file *file, struct kobject *kobj,
-                                struct bin_attribute *attr, char *buf,
-                                loff_t off, size_t count)
-{
-       struct device *kdev = kobj_to_dev(kobj);
-       struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
-
-       drm_dbg(&dev_priv->drm, "Resetting error state\n");
-       i915_reset_error_state(dev_priv);
-
-       return count;
-}
-
-static const struct bin_attribute error_state_attr = {
-       .attr.name = "error",
-       .attr.mode = S_IRUSR | S_IWUSR,
-       .size = 0,
-       .read = error_state_read,
-       .write = error_state_write,
-};
-
-static void i915_setup_error_capture(struct device *kdev)
-{
-       if (sysfs_create_bin_file(&kdev->kobj, &error_state_attr))
-               drm_err(&kdev_minor_to_i915(kdev)->drm,
-                       "error_state sysfs setup failed\n");
-}
-
-static void i915_teardown_error_capture(struct device *kdev)
-{
-       sysfs_remove_bin_file(&kdev->kobj, &error_state_attr);
-}
-#else
-static void i915_setup_error_capture(struct device *kdev) {}
-static void i915_teardown_error_capture(struct device *kdev) {}
-#endif
-
  void i915_setup_sysfs(struct drm_i915_private *dev_priv)
  {
        struct device *kdev = dev_priv->drm.primary->kdev;
@@ -255,7 +180,7 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv)
                drm_warn(&dev_priv->drm,
                         "failed to register GT sysfs directory\n");
- i915_setup_error_capture(kdev);
+       i915_gpu_error_sysfs_setup(dev_priv);
intel_engines_add_sysfs(dev_priv);
  }
@@ -264,7 +189,7 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
  {
        struct device *kdev = dev_priv->drm.primary->kdev;
- i915_teardown_error_capture(kdev);
+       i915_gpu_error_sysfs_teardown(dev_priv);
device_remove_bin_file(kdev, &dpf_attrs_1);
        device_remove_bin_file(kdev,  &dpf_attrs);

Reply via email to