Re: [PATCH 3/3] drm/msm/debugfs: Add display/kms state snapshot

[Dmitry Baryshkov]

On Tue, 14 Dec 2021 at 20:34, Rob Clark <robdclark@xxxxxxxxx> wrote:
>
> From: Rob Clark <robdclark@xxxxxxxxxxxx>
>
> Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/msm/msm_debugfs.c | 90 +++++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
> index 956b1efc3721..088f1160c892 100644
> --- a/drivers/gpu/drm/msm/msm_debugfs.c
> +++ b/drivers/gpu/drm/msm/msm_debugfs.c
> @@ -15,6 +15,11 @@
>  #include "msm_gpu.h"
>  #include "msm_kms.h"
>  #include "msm_debugfs.h"
> +#include "disp/msm_disp_snapshot.h"
> +
> +/*
> + * GPU Snapshot:
> + */
>
>  struct msm_gpu_show_priv {
>         struct msm_gpu_state *state;
> @@ -109,6 +114,88 @@ static const struct file_operations msm_gpu_fops = {
>         .release = msm_gpu_release,
>  };
>
> +/*
> + * Display Snapshot:
> + */
> +
> +struct msm_kms_show_priv {
> +       struct msm_disp_state *state;
> +       struct drm_device *dev;

I don't see ->dev being used outside of _open() callback. So, I'd
suggest removing it and using struct msm_disp_state instead of struct
msm_kms_show_priv.

> +};
> +
> +static int msm_kms_show(struct seq_file *m, void *arg)
> +{
> +       struct drm_printer p = drm_seq_file_printer(m);
> +       struct msm_kms_show_priv *show_priv = m->private;
> +
> +       msm_disp_state_print(show_priv->state, &p);
> +
> +       return 0;
> +}
> +
> +static int msm_kms_release(struct inode *inode, struct file *file)
> +{
> +       struct seq_file *m = file->private_data;
> +       struct msm_kms_show_priv *show_priv = m->private;
> +
> +       msm_disp_state_free(show_priv->state);
> +       kfree(show_priv);
> +
> +       return single_release(inode, file);
> +}
> +
> +static int msm_kms_open(struct inode *inode, struct file *file)
> +{
> +       struct drm_device *dev = inode->i_private;
> +       struct msm_drm_private *priv = dev->dev_private;
> +       struct msm_kms_show_priv *show_priv;
> +       int ret;
> +
> +       if (!priv->kms)
> +               return -ENODEV;
> +
> +       show_priv = kmalloc(sizeof(*show_priv), GFP_KERNEL);
> +       if (!show_priv)
> +               return -ENOMEM;
> +
> +       ret = mutex_lock_interruptible(&priv->kms->dump_mutex);
> +       if (ret)
> +               goto free_priv;
> +
> +       show_priv->state = msm_disp_snapshot_state_sync(priv->kms);
> +
> +       mutex_unlock(&priv->kms->dump_mutex);
> +
> +       if (IS_ERR(show_priv->state)) {
> +               ret = PTR_ERR(show_priv->state);
> +               goto free_priv;
> +       }
> +
> +       show_priv->dev = dev;
> +
> +       ret = single_open(file, msm_kms_show, show_priv);
> +       if (ret)
> +               goto free_priv;
> +
> +       return 0;
> +
> +free_priv:
> +       kfree(show_priv);
> +       return ret;
> +}
> +
> +static const struct file_operations msm_kms_fops = {
> +       .owner = THIS_MODULE,
> +       .open = msm_kms_open,
> +       .read = seq_read,
> +       .llseek = seq_lseek,
> +       .release = msm_kms_release,
> +};
> +
> +/*
> + * Other debugfs:
> + */
> +
>  static unsigned long last_shrink_freed;
>
>  static int
> @@ -239,6 +326,9 @@ void msm_debugfs_init(struct drm_minor *minor)
>         debugfs_create_file("gpu", S_IRUSR, minor->debugfs_root,
>                 dev, &msm_gpu_fops);
>
> +       debugfs_create_file("kms", S_IRUSR, minor->debugfs_root,
> +               dev, &msm_kms_fops);
> +
>         debugfs_create_u32("hangcheck_period_ms", 0600, minor->debugfs_root,
>                 &priv->hangcheck_period);
>
> --
> 2.33.1
>


-- 
With best wishes
Dmitry