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

Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> · Thu, 9 Sep 2021 03:38:27 -0700

On 9/9/2021 1:17 AM, Teres Alexis, Alan Previn wrote:
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.

I went with "terminate_state", because the termination interrupt can 
come from the display but can also come from the ME and we handle both 
in the same way. What we want to test is the termination path that we 
enter when a termination interrupt is received, we don't really care who 
the source of the interrupt is.

Daniele

+	};
+	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








