drm/msm: make msm_disp_state transient data struct

Instead of allocating snapshotting structure at the driver probe time
and later handling concurrent access, actual state, etc, make
msm_disp_state transient struct. Allocate one when snapshotting happens
and free it after coredump data is read by userspace.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
Link: https://lore.kernel.org/r/20210427001828.2375555-3-dmitry.baryshkov@linaro.org
Signed-off-by: Rob Clark <robdclark@chromium.org>

diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c b/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c
index 70fd5a1..a4a7cb0 100644 (file)
--- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c
+++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot.c
@@ -7,8 +7,7 @@
 
 #include "msm_disp_snapshot.h"
 
-#ifdef CONFIG_DEV_COREDUMP
-static ssize_t disp_devcoredump_read(char *buffer, loff_t offset,
+static ssize_t __maybe_unused disp_devcoredump_read(char *buffer, loff_t offset,
                size_t count, void *data, size_t datalen)
 {
        struct drm_print_iterator iter;
@@ -29,52 +28,47 @@ static ssize_t disp_devcoredump_read(char *buffer, loff_t offset,
        return count - iter.remain;
 }
 
-static void disp_devcoredump_free(void *data)
+static void _msm_disp_snapshot_work(struct kthread_work *work)
 {
+       struct msm_kms *kms = container_of(work, struct msm_kms, dump_work);
+       struct drm_device *drm_dev = kms->dev;
        struct msm_disp_state *disp_state;
+       struct drm_printer p;
 
-       disp_state = data;
-
-       msm_disp_state_free(disp_state);
+       disp_state = kzalloc(sizeof(struct msm_disp_state), GFP_KERNEL);
+       if (!disp_state)
+               return;
 
-       disp_state->coredump_pending = false;
-}
-#endif /* CONFIG_DEV_COREDUMP */
+       disp_state->dev = drm_dev->dev;
+       disp_state->drm_dev = drm_dev;
 
-static void _msm_disp_snapshot_work(struct kthread_work *work)
-{
-       struct msm_disp_state *disp_state = container_of(work, struct msm_disp_state, dump_work);
-       struct drm_printer p;
+       INIT_LIST_HEAD(&disp_state->blocks);
 
-       mutex_lock(&disp_state->mutex);
+       /* Serialize dumping here */
+       mutex_lock(&kms->dump_mutex);
 
        msm_disp_snapshot_capture_state(disp_state);
 
+       mutex_unlock(&kms->dump_mutex);
+
        if (MSM_DISP_SNAPSHOT_DUMP_IN_CONSOLE) {
                p = drm_info_printer(disp_state->drm_dev->dev);
                msm_disp_state_print(disp_state, &p);
        }
 
        /*
-        * if devcoredump is not defined free the state immediately
-        * otherwise it will be freed in the free handler.
+        * If COREDUMP is disabled, the stub will call the free function.
+        * If there is a codedump pending for the device, the dev_coredumpm()
+        * will also free new coredump state.
         */
-#ifdef CONFIG_DEV_COREDUMP
        dev_coredumpm(disp_state->dev, THIS_MODULE, disp_state, 0, GFP_KERNEL,
-                       disp_devcoredump_read, disp_devcoredump_free);
-       disp_state->coredump_pending = true;
-#else
-       msm_disp_state_free(disp_state);
-#endif
-
-       mutex_unlock(&disp_state->mutex);
+                       disp_devcoredump_read, msm_disp_state_free);
 }
 
 void msm_disp_snapshot_state(struct drm_device *drm_dev)
 {
        struct msm_drm_private *priv;
        struct msm_kms *kms;
-       struct msm_disp_state *disp_state;
 
        if (!drm_dev) {
                DRM_ERROR("invalid params\n");
@@ -83,30 +77,13 @@ void msm_disp_snapshot_state(struct drm_device *drm_dev)
 
        priv = drm_dev->dev_private;
        kms = priv->kms;
-       disp_state = kms->disp_state;
-
-       if (!disp_state) {
-               DRM_ERROR("invalid params\n");
-               return;
-       }
 
-       /*
-        * if there is a coredump pending return immediately till dump
-        * if read by userspace or timeout happens
-        */
-       if (disp_state->coredump_pending) {
-               DRM_DEBUG("coredump is pending read\n");
-               return;
-       }
-
-       kthread_queue_work(disp_state->dump_worker,
-                       &disp_state->dump_work);
+       kthread_queue_work(kms->dump_worker, &kms->dump_work);
 }
 
 int msm_disp_snapshot_init(struct drm_device *drm_dev)
 {
        struct msm_drm_private *priv;
-       struct msm_disp_state *disp_state;
        struct msm_kms *kms;
 
        if (!drm_dev) {
@@ -117,22 +94,13 @@ int msm_disp_snapshot_init(struct drm_device *drm_dev)
        priv = drm_dev->dev_private;
        kms = priv->kms;
 
-       disp_state = devm_kzalloc(drm_dev->dev, sizeof(struct msm_disp_state), GFP_KERNEL);
-
-       mutex_init(&disp_state->mutex);
+       mutex_init(&kms->dump_mutex);
 
-       disp_state->dev = drm_dev->dev;
-       disp_state->drm_dev = drm_dev;
-
-       INIT_LIST_HEAD(&disp_state->blocks);
-
-       disp_state->dump_worker = kthread_create_worker(0, "%s", "disp_snapshot");
-       if (IS_ERR(disp_state->dump_worker))
+       kms->dump_worker = kthread_create_worker(0, "%s", "disp_snapshot");
+       if (IS_ERR(kms->dump_worker))
                DRM_ERROR("failed to create disp state task\n");
 
-       kthread_init_work(&disp_state->dump_work, _msm_disp_snapshot_work);
-
-       kms->disp_state = disp_state;
+       kthread_init_work(&kms->dump_work, _msm_disp_snapshot_work);
 
        return 0;
 }
@@ -141,7 +109,6 @@ void msm_disp_snapshot_destroy(struct drm_device *drm_dev)
 {
        struct msm_kms *kms;
        struct msm_drm_private *priv;
-       struct msm_disp_state *disp_state;
 
        if (!drm_dev) {
                DRM_ERROR("invalid params\n");
@@ -150,12 +117,9 @@ void msm_disp_snapshot_destroy(struct drm_device *drm_dev)
 
        priv = drm_dev->dev_private;
        kms = priv->kms;
-       disp_state = kms->disp_state;
-
-       if (disp_state->dump_worker)
-               kthread_destroy_worker(disp_state->dump_worker);
 
-       list_del(&disp_state->blocks);
+       if (kms->dump_worker)
+               kthread_destroy_worker(kms->dump_worker);
 
-       mutex_destroy(&disp_state->mutex);
+       mutex_destroy(&kms->dump_mutex);
 }

diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot.h b/drivers/gpu/drm/msm/disp/msm_disp_snapshot.h
index 32f5279..c6174a3 100644 (file)
--- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot.h
+++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot.h
@@ -41,26 +41,17 @@
  * struct msm_disp_state - structure to store current dpu state
  * @dev: device pointer
  * @drm_dev: drm device pointer
- * @mutex: mutex to serialize access to serialze dumps, debugfs access
- * @coredump_pending: coredump is pending read from userspace
  * @atomic_state: atomic state duplicated at the time of the error
- * @dump_worker: kworker thread which runs the dump work
- * @dump_work: kwork which dumps the registers and drm state
  * @timestamp: timestamp at which the coredump was captured
  */
 struct msm_disp_state {
        struct device *dev;
        struct drm_device *drm_dev;
-       struct mutex mutex;
-
-       bool coredump_pending;
 
        struct list_head blocks;
 
        struct drm_atomic_state *atomic_state;
 
-       struct kthread_worker *dump_worker;
-       struct kthread_work dump_work;
        ktime_t timestamp;
 };
 
@@ -123,11 +114,11 @@ void msm_disp_snapshot_capture_state(struct msm_disp_state *disp_state);
 
 /**
  * msm_disp_state_free - free the memory after the coredump has been read
- * @disp_state:            handle to struct msm_disp_state
+ * @data:          handle to struct msm_disp_state
 
  * Returns: none
  */
-void msm_disp_state_free(struct msm_disp_state *disp_state);
+void msm_disp_state_free(void *data);
 
 /**
  * msm_disp_snapshot_add_block - add a hardware block with its register dump

diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
index ca66325..cabe151 100644 (file)
--- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
+++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
@@ -142,8 +142,9 @@ void msm_disp_snapshot_capture_state(struct msm_disp_state *disp_state)
        msm_disp_capture_atomic_state(disp_state);
 }
 
-void msm_disp_state_free(struct msm_disp_state *disp_state)
+void msm_disp_state_free(void *data)
 {
+       struct msm_disp_state *disp_state = data;
        struct msm_disp_state_block *block, *tmp;
 
        if (disp_state->atomic_state) {
@@ -156,6 +157,8 @@ void msm_disp_state_free(struct msm_disp_state *disp_state)
                kfree(block->state);
                kfree(block);
        }
+
+       kfree(disp_state);
 }
 
 void msm_disp_snapshot_add_block(struct msm_disp_state *disp_state, u32 len,

diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 146dcab..086a2d5 100644 (file)
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -156,8 +156,10 @@ struct msm_kms {
        /* mapper-id used to request GEM buffer mapped for scanout: */
        struct msm_gem_address_space *aspace;
 
-       /* handle to disp snapshot state */
-       struct msm_disp_state *disp_state;
+       /* disp snapshot support */
+       struct kthread_worker *dump_worker;
+       struct kthread_work dump_work;
+       struct mutex dump_mutex;
 
        /*
         * For async commit, where ->flush_commit() and later happens