Re: [Intel-gfx] [PATCH v7 15/17] drm/i915/pxp: add pxp debugfs

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@xxxxxxxxx>]

I dont see any issues except a couple of nits. 

Reviewed-by : Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>

...alan

On Fri, 2021-08-27 at 18:27 -0700, Daniele Ceraolo Spurio wrote:
> 2 debugfs files, one to query the current status of the pxp session and one
> to trigger an invalidation for testing.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/Makefile                |  1 +
>  drivers/gpu/drm/i915/gt/debugfs_gt.c         |  2 +
>  drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c | 78 ++++++++++++++++++++
>  drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h | 21 ++++++
>  4 files changed, 102 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
>  create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 6f6cbbe98b96..9a44d6f01e3b 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -284,6 +284,7 @@ i915-y += i915_perf.o
>  i915-$(CONFIG_DRM_I915_PXP) += \
>  	pxp/intel_pxp.o \
>  	pxp/intel_pxp_cmd.o \
> +	pxp/intel_pxp_debugfs.o \
>  	pxp/intel_pxp_irq.o \
>  	pxp/intel_pxp_pm.o \
>  	pxp/intel_pxp_session.o \
> diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.c b/drivers/gpu/drm/i915/gt/debugfs_gt.c
> index 591eb60785db..c27847ddb796 100644
> --- a/drivers/gpu/drm/i915/gt/debugfs_gt.c
> +++ b/drivers/gpu/drm/i915/gt/debugfs_gt.c
> @@ -9,6 +9,7 @@
>  #include "debugfs_gt.h"
>  #include "debugfs_gt_pm.h"
>  #include "intel_sseu_debugfs.h"
> +#include "pxp/intel_pxp_debugfs.h"
>  #include "uc/intel_uc_debugfs.h"
>  #include "i915_drv.h"
>  
> @@ -28,6 +29,7 @@ void debugfs_gt_register(struct intel_gt *gt)
>  	intel_sseu_debugfs_register(gt, root);
>  
>  	intel_uc_debugfs_register(&gt->uc, root);
> +	intel_pxp_debugfs_register(&gt->pxp, root);
>  }
>  
>  void intel_gt_debugfs_register_files(struct dentry *root,
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
> new file mode 100644
> index 000000000000..a26e4396ba6c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +
> +#include <linux/debugfs.h>
> +#include <drm/drm_print.h>
> +
> +#include "gt/debugfs_gt.h"
> +#include "pxp/intel_pxp.h"
> +#include "pxp/intel_pxp_irq.h"
> +#include "i915_drv.h"
> +
> +static int pxp_info_show(struct seq_file *m, void *data)
> +{
> +	struct intel_pxp *pxp = m->private;
> +	struct drm_printer p = drm_seq_file_printer(m);
> +	bool enabled = intel_pxp_is_enabled(pxp);
> +
> +	if (!enabled) {
> +		drm_printf(&p, "pxp disabled\n");
> +		return 0;
> +	}
> +
> +	drm_printf(&p, "active: %s\n", yesno(intel_pxp_is_active(pxp)));
> +	drm_printf(&p, "instance counter: %u\n", pxp->key_instance);
> +
> +	return 0;
> +}
> +DEFINE_GT_DEBUGFS_ATTRIBUTE(pxp_info);
> +
> +static int pxp_inval_get(void *data, u64 *val)
> +{
> +	/* nothing to read */
> +	return -EPERM;
> +}
> +
> +static int pxp_inval_set(void *data, u64 val)
> +{
> +	struct intel_pxp *pxp = data;
> +	struct intel_gt *gt = pxp_to_gt(pxp);
> +
> +	if (!intel_pxp_is_active(pxp))
> +		return -ENODEV;
> +
> +	/* simulate an invalidation interrupt */
> +	spin_lock_irq(&gt->irq_lock);
> +	intel_pxp_irq_handler(pxp, GEN12_DISPLAY_PXP_STATE_TERMINATED_INTERRUPT);
> +	spin_unlock_irq(&gt->irq_lock);
> +
> +	if (!wait_for_completion_timeout(&pxp->termination,
> +					 msecs_to_jiffies(100)))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(pxp_inval_fops, pxp_inval_get, pxp_inval_set, "%llx\n");
> +void intel_pxp_debugfs_register(struct intel_pxp *pxp, struct dentry *gt_root)
> +{
> +	static const struct debugfs_gt_file files[] = {
> +		{ "info", &pxp_info_fops, NULL },
> +		{ "invalidate", &pxp_inval_fops, NULL },
NIT only: consider naming to "invalidate_display" or "display_inval" since we are using this to trigger
display pxp teardown specific irq code path.
> +	};
> +	struct dentry *root;
> +
> +	if (!gt_root)
> +		return;
> +
> +	if (!HAS_PXP((pxp_to_gt(pxp)->i915)))
> +		return;
> +
> +	root = debugfs_create_dir("pxp", gt_root);
> +	if (IS_ERR(root))
> +		return;
> +
> +	intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), pxp);
> +}
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h
> new file mode 100644
> index 000000000000..3b7454d838e9
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2020 Intel Corporation
NIT - 2021
> + */
> +
> +#ifndef __INTEL_PXP_DEBUGFS_H__
> +#define __INTEL_PXP_DEBUGFS_H__
> +
> +struct intel_pxp;
> +struct dentry;
> +
> +#ifdef CONFIG_DRM_I915_PXP
> +void intel_pxp_debugfs_register(struct intel_pxp *pxp, struct dentry *root);
> +#else
> +static inline void
> +intel_pxp_debugfs_register(struct intel_pxp *pxp, struct dentry *root)
> +{
> +}
> +#endif
> +
> +#endif /* __INTEL_PXP_DEBUGFS_H__ */
> -- 
> 2.25.1
>