[Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT

All of lore.kernel.org
 help / color / mirror / Atom feed

* [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT
@ 2020-02-28  2:28 Daniele Ceraolo Spurio
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 1/6] drm/i915/guc: drop stage_pool debugfs Daniele Ceraolo Spurio
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-02-28  2:28 UTC (permalink / raw)
  To: intel-gfx

Move printing functions to their respective feature files, squash the 2
guc status debugfs files (load_status and info) and move them under the
gt/ folder. A fix for keeping the log error around, to be dumped in
debugfs after a wedge, is also included at the end.

Cc: Andi Shyti <andi.shyti@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>

Daniele Ceraolo Spurio (6):
  drm/i915/guc: drop stage_pool debugfs
  drm/i915/uc: mark structure passed to checker functions as const
  drm/i915/huc: make "support huc" reflect HW capabilities
  drm/i915/debugfs: move uC printers and update debugfs file names
  drm/i915/uc: Move uC debugfs to its own folder under GT
  drm/i915/uc: do not free err log on uc_fini

 drivers/gpu/drm/i915/Makefile                 |   7 +-
 drivers/gpu/drm/i915/gt/debugfs_gt.c          |   3 +
 drivers/gpu/drm/i915/gt/intel_gt.c            |   3 +-
 drivers/gpu/drm/i915/gt/intel_gt.h            |   6 +-
 drivers/gpu/drm/i915/gt/uc/debugfs_guc.c      |  42 +++
 drivers/gpu/drm/i915/gt/uc/debugfs_guc.h      |  14 +
 drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c  | 123 ++++++++
 drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h  |  14 +
 drivers/gpu/drm/i915/gt/uc/debugfs_huc.c      |  36 +++
 drivers/gpu/drm/i915/gt/uc/debugfs_huc.h      |  14 +
 drivers/gpu/drm/i915/gt/uc/debugfs_uc.c       |  31 ++
 drivers/gpu/drm/i915/gt/uc/debugfs_uc.h       |  43 +++
 drivers/gpu/drm/i915/gt/uc/intel_guc.c        |  46 ++-
 drivers/gpu/drm/i915/gt/uc/intel_guc.h        |  17 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h     |   2 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c     |  14 -
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.h     |   1 -
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.c    |  97 +++++-
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.h    |   4 +
 .../gpu/drm/i915/gt/uc/intel_guc_submission.h |   6 +-
 drivers/gpu/drm/i915/gt/uc/intel_huc.c        |  31 +-
 drivers/gpu/drm/i915/gt/uc/intel_huc.h        |  10 +-
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c     |  17 --
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h     |   1 -
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         |   9 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc.h         |   3 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  25 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h      |  21 +-
 drivers/gpu/drm/i915/i915_debugfs.c           | 289 ------------------
 29 files changed, 559 insertions(+), 370 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc.h
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_huc.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_huc.h
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_uc.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_uc.h

-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH 1/6] drm/i915/guc: drop stage_pool debugfs
  2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
@ 2020-02-28  2:28 ` Daniele Ceraolo Spurio
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 2/6] drm/i915/uc: mark structure passed to checker functions as const Daniele Ceraolo Spurio
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-02-28  2:28 UTC (permalink / raw)
  To: intel-gfx

The pool will be private to GuC in the new submission scheme, so we
won't be able to print it and we can just drop the current legacy code.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 53 -----------------------------
 1 file changed, 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 8f2525e4ce0f..37cb8b4bf4dc 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1570,58 +1570,6 @@ static int i915_guc_info(struct seq_file *m, void *data)
 	return 0;
 }
 
-static int i915_guc_stage_pool(struct seq_file *m, void *data)
-{
-	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	struct intel_uc *uc = &dev_priv->gt.uc;
-	struct guc_stage_desc *desc = uc->guc.stage_desc_pool_vaddr;
-	int index;
-
-	if (!intel_uc_uses_guc_submission(uc))
-		return -ENODEV;
-
-	for (index = 0; index < GUC_MAX_STAGE_DESCRIPTORS; index++, desc++) {
-		struct intel_engine_cs *engine;
-
-		if (!(desc->attribute & GUC_STAGE_DESC_ATTR_ACTIVE))
-			continue;
-
-		seq_printf(m, "GuC stage descriptor %u:\n", index);
-		seq_printf(m, "\tIndex: %u\n", desc->stage_id);
-		seq_printf(m, "\tAttribute: 0x%x\n", desc->attribute);
-		seq_printf(m, "\tPriority: %d\n", desc->priority);
-		seq_printf(m, "\tDoorbell id: %d\n", desc->db_id);
-		seq_printf(m, "\tEngines used: 0x%x\n",
-			   desc->engines_used);
-		seq_printf(m, "\tDoorbell trigger phy: 0x%llx, cpu: 0x%llx, uK: 0x%x\n",
-			   desc->db_trigger_phy,
-			   desc->db_trigger_cpu,
-			   desc->db_trigger_uk);
-		seq_printf(m, "\tProcess descriptor: 0x%x\n",
-			   desc->process_desc);
-		seq_printf(m, "\tWorkqueue address: 0x%x, size: 0x%x\n",
-			   desc->wq_addr, desc->wq_size);
-		seq_putc(m, '\n');
-
-		for_each_uabi_engine(engine, dev_priv) {
-			u32 guc_engine_id = engine->guc_id;
-			struct guc_execlist_context *lrc =
-						&desc->lrc[guc_engine_id];
-
-			seq_printf(m, "\t%s LRC:\n", engine->name);
-			seq_printf(m, "\t\tContext desc: 0x%x\n",
-				   lrc->context_desc);
-			seq_printf(m, "\t\tContext id: 0x%x\n", lrc->context_id);
-			seq_printf(m, "\t\tLRCA: 0x%x\n", lrc->ring_lrca);
-			seq_printf(m, "\t\tRing begin: 0x%x\n", lrc->ring_begin);
-			seq_printf(m, "\t\tRing end: 0x%x\n", lrc->ring_end);
-			seq_putc(m, '\n');
-		}
-	}
-
-	return 0;
-}
-
 static int i915_guc_log_dump(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = m->private;
@@ -2357,7 +2305,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_guc_load_status", i915_guc_load_status_info, 0},
 	{"i915_guc_log_dump", i915_guc_log_dump, 0},
 	{"i915_guc_load_err_log_dump", i915_guc_log_dump, 0, (void *)1},
-	{"i915_guc_stage_pool", i915_guc_stage_pool, 0},
 	{"i915_huc_load_status", i915_huc_load_status_info, 0},
 	{"i915_frequency_info", i915_frequency_info, 0},
 	{"i915_drpc_info", i915_drpc_info, 0},
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH 2/6] drm/i915/uc: mark structure passed to checker functions as const
  2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 1/6] drm/i915/guc: drop stage_pool debugfs Daniele Ceraolo Spurio
@ 2020-02-28  2:28 ` Daniele Ceraolo Spurio
  2020-02-28  9:18   ` Jani Nikula
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 3/6] drm/i915/huc: make "support huc" reflect HW capabilities Daniele Ceraolo Spurio
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-02-28  2:28 UTC (permalink / raw)
  To: intel-gfx

Follow-up patches will pass const objects from debugfs to some those
functions, so we need to be ready.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt.h             |  6 +++---
 drivers/gpu/drm/i915/gt/uc/intel_guc.h         | 10 +++++-----
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h      |  2 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.h  |  6 +++---
 drivers/gpu/drm/i915/gt/uc/intel_huc.h         |  8 ++++----
 drivers/gpu/drm/i915/gt/uc/intel_uc.h          |  2 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h       | 18 +++++++++---------
 7 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
index 4fac043750aa..f9fbe645478d 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -18,17 +18,17 @@ struct drm_i915_private;
 		  ##__VA_ARGS__);					\
 } while (0)
 
-static inline struct intel_gt *uc_to_gt(struct intel_uc *uc)
+static inline struct intel_gt *uc_to_gt(const struct intel_uc *uc)
 {
 	return container_of(uc, struct intel_gt, uc);
 }
 
-static inline struct intel_gt *guc_to_gt(struct intel_guc *guc)
+static inline struct intel_gt *guc_to_gt(const struct intel_guc *guc)
 {
 	return container_of(guc, struct intel_gt, uc.guc);
 }
 
-static inline struct intel_gt *huc_to_gt(struct intel_huc *huc)
+static inline struct intel_gt *huc_to_gt(const struct intel_huc *huc)
 {
 	return container_of(huc, struct intel_gt, uc.huc);
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 4594ccbeaa34..969147bd9973 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -138,28 +138,28 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
 int intel_guc_allocate_and_map_vma(struct intel_guc *guc, u32 size,
 				   struct i915_vma **out_vma, void **out_vaddr);
 
-static inline bool intel_guc_is_supported(struct intel_guc *guc)
+static inline bool intel_guc_is_supported(const struct intel_guc *guc)
 {
 	return intel_uc_fw_is_supported(&guc->fw);
 }
 
-static inline bool intel_guc_is_wanted(struct intel_guc *guc)
+static inline bool intel_guc_is_wanted(const struct intel_guc *guc)
 {
 	return intel_uc_fw_is_enabled(&guc->fw);
 }
 
-static inline bool intel_guc_is_used(struct intel_guc *guc)
+static inline bool intel_guc_is_used(const struct intel_guc *guc)
 {
 	GEM_BUG_ON(__intel_uc_fw_status(&guc->fw) == INTEL_UC_FIRMWARE_SELECTED);
 	return intel_uc_fw_is_available(&guc->fw);
 }
 
-static inline bool intel_guc_is_fw_running(struct intel_guc *guc)
+static inline bool intel_guc_is_fw_running(const struct intel_guc *guc)
 {
 	return intel_uc_fw_is_running(&guc->fw);
 }
 
-static inline bool intel_guc_is_ready(struct intel_guc *guc)
+static inline bool intel_guc_is_ready(const struct intel_guc *guc)
 {
 	return intel_guc_is_fw_running(guc) && intel_guc_ct_enabled(&guc->ct);
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
index 494a51a5200f..15e41a194544 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
@@ -70,7 +70,7 @@ static inline void intel_guc_ct_sanitize(struct intel_guc_ct *ct)
 	ct->enabled = false;
 }
 
-static inline bool intel_guc_ct_enabled(struct intel_guc_ct *ct)
+static inline bool intel_guc_ct_enabled(const struct intel_guc_ct *ct)
 {
 	return ct->enabled;
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index 4cf9d3e50263..be96a476550b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -21,18 +21,18 @@ int intel_guc_preempt_work_create(struct intel_guc *guc);
 void intel_guc_preempt_work_destroy(struct intel_guc *guc);
 bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine);
 
-static inline bool intel_guc_submission_is_supported(struct intel_guc *guc)
+static inline bool intel_guc_submission_is_supported(const struct intel_guc *guc)
 {
 	/* XXX: GuC submission is unavailable for now */
 	return false;
 }
 
-static inline bool intel_guc_submission_is_wanted(struct intel_guc *guc)
+static inline bool intel_guc_submission_is_wanted(const struct intel_guc *guc)
 {
 	return guc->submission_selected;
 }
 
-static inline bool intel_guc_submission_is_used(struct intel_guc *guc)
+static inline bool intel_guc_submission_is_used(const struct intel_guc *guc)
 {
 	return intel_guc_is_used(guc) && intel_guc_submission_is_wanted(guc);
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index a40b9cfc6c22..19651b46d6a4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -36,23 +36,23 @@ static inline int intel_huc_sanitize(struct intel_huc *huc)
 	return 0;
 }
 
-static inline bool intel_huc_is_supported(struct intel_huc *huc)
+static inline bool intel_huc_is_supported(const struct intel_huc *huc)
 {
 	return intel_uc_fw_is_supported(&huc->fw);
 }
 
-static inline bool intel_huc_is_wanted(struct intel_huc *huc)
+static inline bool intel_huc_is_wanted(const struct intel_huc *huc)
 {
 	return intel_uc_fw_is_enabled(&huc->fw);
 }
 
-static inline bool intel_huc_is_used(struct intel_huc *huc)
+static inline bool intel_huc_is_used(const struct intel_huc *huc)
 {
 	GEM_BUG_ON(__intel_uc_fw_status(&huc->fw) == INTEL_UC_FIRMWARE_SELECTED);
 	return intel_uc_fw_is_available(&huc->fw);
 }
 
-static inline bool intel_huc_is_authenticated(struct intel_huc *huc)
+static inline bool intel_huc_is_authenticated(const struct intel_huc *huc)
 {
 	return intel_uc_fw_is_running(&huc->fw);
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 5ae7b50b7dc1..2f7d3028af08 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -63,7 +63,7 @@ int intel_uc_runtime_resume(struct intel_uc *uc);
  */
 
 #define __uc_state_checker(x, func, state, required) \
-static inline bool intel_uc_##state##_##func(struct intel_uc *uc) \
+static inline bool intel_uc_##state##_##func(const struct intel_uc *uc) \
 { \
 	return intel_##func##_is_##required(&uc->x); \
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 888ff0de0244..704b7b0fd710 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -169,39 +169,39 @@ static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
 }
 
 static inline enum intel_uc_fw_status
-__intel_uc_fw_status(struct intel_uc_fw *uc_fw)
+__intel_uc_fw_status(const struct intel_uc_fw *uc_fw)
 {
 	/* shouldn't call this before checking hw/blob availability */
 	GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_UNINITIALIZED);
 	return uc_fw->status;
 }
 
-static inline bool intel_uc_fw_is_supported(struct intel_uc_fw *uc_fw)
+static inline bool intel_uc_fw_is_supported(const struct intel_uc_fw *uc_fw)
 {
 	return __intel_uc_fw_status(uc_fw) != INTEL_UC_FIRMWARE_NOT_SUPPORTED;
 }
 
-static inline bool intel_uc_fw_is_enabled(struct intel_uc_fw *uc_fw)
+static inline bool intel_uc_fw_is_enabled(const struct intel_uc_fw *uc_fw)
 {
 	return __intel_uc_fw_status(uc_fw) > INTEL_UC_FIRMWARE_DISABLED;
 }
 
-static inline bool intel_uc_fw_is_available(struct intel_uc_fw *uc_fw)
+static inline bool intel_uc_fw_is_available(const struct intel_uc_fw *uc_fw)
 {
 	return __intel_uc_fw_status(uc_fw) >= INTEL_UC_FIRMWARE_AVAILABLE;
 }
 
-static inline bool intel_uc_fw_is_loadable(struct intel_uc_fw *uc_fw)
+static inline bool intel_uc_fw_is_loadable(const struct intel_uc_fw *uc_fw)
 {
 	return __intel_uc_fw_status(uc_fw) >= INTEL_UC_FIRMWARE_LOADABLE;
 }
 
-static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
+static inline bool intel_uc_fw_is_loaded(const struct intel_uc_fw *uc_fw)
 {
 	return __intel_uc_fw_status(uc_fw) >= INTEL_UC_FIRMWARE_TRANSFERRED;
 }
 
-static inline bool intel_uc_fw_is_running(struct intel_uc_fw *uc_fw)
+static inline bool intel_uc_fw_is_running(const struct intel_uc_fw *uc_fw)
 {
 	return __intel_uc_fw_status(uc_fw) == INTEL_UC_FIRMWARE_RUNNING;
 }
@@ -217,7 +217,7 @@ static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
 		intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_LOADABLE);
 }
 
-static inline u32 __intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
+static inline u32 __intel_uc_fw_get_upload_size(const struct intel_uc_fw *uc_fw)
 {
 	return sizeof(struct uc_css_header) + uc_fw->ucode_size;
 }
@@ -230,7 +230,7 @@ static inline u32 __intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
  *
  * Return: Upload firmware size, or zero on firmware fetch failure.
  */
-static inline u32 intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
+static inline u32 intel_uc_fw_get_upload_size(const struct intel_uc_fw *uc_fw)
 {
 	if (!intel_uc_fw_is_available(uc_fw))
 		return 0;
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH 3/6] drm/i915/huc: make "support huc" reflect HW capabilities
  2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 1/6] drm/i915/guc: drop stage_pool debugfs Daniele Ceraolo Spurio
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 2/6] drm/i915/uc: mark structure passed to checker functions as const Daniele Ceraolo Spurio
@ 2020-02-28  2:28 ` Daniele Ceraolo Spurio
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 4/6] drm/i915/debugfs: move uC printers and update debugfs file names Daniele Ceraolo Spurio
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-02-28  2:28 UTC (permalink / raw)
  To: intel-gfx

We currently initialize HuC support based on GuC being enabled in
modparam; this means that huc_is_supported() can return false on HW that
does have a HuC when enable_guc=0. The rationale for this behavior is
that HuC requires GuC for authentication and therefore is not supported
by itself. However, we do not allow defining HuC fw wthout GuC fw and
selecting HuC in modparam implicitly selects GuC as well, so we can't
actually hit a scenario where HuC is selected alone. Therefore, we can
flip the support check to reflect the HW capabilities and fw
availability, which is more intuitive and will make it cleaner to log
HuC the difference between not supported in HW and not selected.

Removing the difference between GuC and HuC also allows us to simplify
the init_early, since we don't need to differentiate the support based
on the type of uC.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c    |  2 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 14 -------------
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.h |  1 -
 drivers/gpu/drm/i915/gt/uc/intel_huc.c    |  2 +-
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 17 ---------------
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h |  1 -
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  | 25 +++++++++++++++--------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  |  3 +--
 8 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 819f09ef51fc..827d75073879 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -169,7 +169,7 @@ void intel_guc_init_early(struct intel_guc *guc)
 {
 	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
 
-	intel_guc_fw_init_early(guc);
+	intel_uc_fw_init_early(&guc->fw, INTEL_UC_FW_TYPE_GUC);
 	intel_guc_ct_init_early(&guc->ct);
 	intel_guc_log_init_early(&guc->log);
 	intel_guc_submission_init_early(guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 3a1c47d600ea..d4a87f4c9421 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -13,20 +13,6 @@
 #include "intel_guc_fw.h"
 #include "i915_drv.h"
 
-/**
- * intel_guc_fw_init_early() - initializes GuC firmware struct
- * @guc: intel_guc struct
- *
- * On platforms with GuC selects firmware for uploading
- */
-void intel_guc_fw_init_early(struct intel_guc *guc)
-{
-	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
-
-	intel_uc_fw_init_early(&guc->fw, INTEL_UC_FW_TYPE_GUC, HAS_GT_UC(i915),
-			       INTEL_INFO(i915)->platform, INTEL_REVID(i915));
-}
-
 static void guc_prepare_xfer(struct intel_uncore *uncore)
 {
 	u32 shim_flags = GUC_DISABLE_SRAM_INIT_TO_ZEROES |
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.h
index b5ab639d7259..0b4d2a9c9435 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.h
@@ -8,7 +8,6 @@
 
 struct intel_guc;
 
-void intel_guc_fw_init_early(struct intel_guc *guc);
 int intel_guc_fw_upload(struct intel_guc *guc);
 
 #endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index a74b65694512..d73dc21686e7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -41,7 +41,7 @@ void intel_huc_init_early(struct intel_huc *huc)
 {
 	struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
 
-	intel_huc_fw_init_early(huc);
+	intel_uc_fw_init_early(&huc->fw, INTEL_UC_FW_TYPE_HUC);
 
 	if (INTEL_GEN(i915) >= 11) {
 		huc->status.reg = GEN11_HUC_KERNEL_LOAD_INFO;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 9cdf4cbe691c..e5ef509c70e8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -7,23 +7,6 @@
 #include "intel_huc_fw.h"
 #include "i915_drv.h"
 
-/**
- * intel_huc_fw_init_early() - initializes HuC firmware struct
- * @huc: intel_huc struct
- *
- * On platforms with HuC selects firmware for uploading
- */
-void intel_huc_fw_init_early(struct intel_huc *huc)
-{
-	struct intel_gt *gt = huc_to_gt(huc);
-	struct intel_uc *uc = &gt->uc;
-	struct drm_i915_private *i915 = gt->i915;
-
-	intel_uc_fw_init_early(&huc->fw, INTEL_UC_FW_TYPE_HUC,
-			       intel_uc_wants_guc(uc),
-			       INTEL_INFO(i915)->platform, INTEL_REVID(i915));
-}
-
 /**
  * intel_huc_fw_upload() - load HuC uCode to device
  * @huc: intel_huc structure
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
index b791269ce923..12f264ee3e0b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h
@@ -8,7 +8,6 @@
 
 struct intel_huc;
 
-void intel_huc_fw_init_early(struct intel_huc *huc);
 int intel_huc_fw_upload(struct intel_huc *huc);
 
 #endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 5434c07aefa1..1e689e2fe089 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -11,16 +11,22 @@
 #include "intel_uc_fw_abi.h"
 #include "i915_drv.h"
 
-static inline struct intel_gt *__uc_fw_to_gt(struct intel_uc_fw *uc_fw)
+static inline struct intel_gt *
+____uc_fw_to_gt(struct intel_uc_fw *uc_fw, enum intel_uc_fw_type type)
 {
-	GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_UNINITIALIZED);
-	if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
+	if (type == INTEL_UC_FW_TYPE_GUC)
 		return container_of(uc_fw, struct intel_gt, uc.guc.fw);
 
-	GEM_BUG_ON(uc_fw->type != INTEL_UC_FW_TYPE_HUC);
+	GEM_BUG_ON(type != INTEL_UC_FW_TYPE_HUC);
 	return container_of(uc_fw, struct intel_gt, uc.huc.fw);
 }
 
+static inline struct intel_gt *__uc_fw_to_gt(struct intel_uc_fw *uc_fw)
+{
+	GEM_BUG_ON(uc_fw->status == INTEL_UC_FIRMWARE_UNINITIALIZED);
+	return ____uc_fw_to_gt(uc_fw, uc_fw->type);
+}
+
 #ifdef CONFIG_DRM_I915_DEBUG_GUC
 void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
 			       enum intel_uc_fw_status status)
@@ -195,9 +201,10 @@ static void __uc_fw_user_override(struct intel_uc_fw *uc_fw)
  * firmware to fetch and load.
  */
 void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
-			    enum intel_uc_fw_type type, bool supported,
-			    enum intel_platform platform, u8 rev)
+			    enum intel_uc_fw_type type)
 {
+	struct drm_i915_private *i915 = ____uc_fw_to_gt(uc_fw, type)->i915;
+
 	/*
 	 * we use FIRMWARE_UNINITIALIZED to detect checks against uc_fw->status
 	 * before we're looked at the HW caps to see if we have uc support
@@ -208,8 +215,10 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
 
 	uc_fw->type = type;
 
-	if (supported) {
-		__uc_fw_auto_select(uc_fw, platform, rev);
+	if (HAS_GT_UC(i915)) {
+		__uc_fw_auto_select(uc_fw,
+				    INTEL_INFO(i915)->platform,
+				    INTEL_REVID(i915));
 		__uc_fw_user_override(uc_fw);
 	}
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 704b7b0fd710..9f66c058fd23 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -239,8 +239,7 @@ static inline u32 intel_uc_fw_get_upload_size(const struct intel_uc_fw *uc_fw)
 }
 
 void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
-			    enum intel_uc_fw_type type, bool supported,
-			    enum intel_platform platform, u8 rev);
+			    enum intel_uc_fw_type type);
 int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw);
 void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw);
 int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, u32 offset, u32 dma_flags);
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH 4/6] drm/i915/debugfs: move uC printers and update debugfs file names
  2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
                   ` (2 preceding siblings ...)
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 3/6] drm/i915/huc: make "support huc" reflect HW capabilities Daniele Ceraolo Spurio
@ 2020-02-28  2:28 ` Daniele Ceraolo Spurio
       [not found]   ` <be6dd380-e87b-9ac7-0185-c3c46e40240b@intel.com>
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT Daniele Ceraolo Spurio
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-02-28  2:28 UTC (permalink / raw)
  To: intel-gfx

Move the printers to the respective files for clarity. The
guc_load_status debugfs has been squashed in the guc_info one, has
having separate ones wasn't very useful. The HuC debugfs has been
renamed huc_info to match.

While at it, fix the register printed in the HuC debugfs for gen11+.

Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Tony Ye <tony.ye@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c     |  44 +++++++
 drivers/gpu/drm/i915/gt/uc/intel_guc.h     |   2 +
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.c |  94 ++++++++++++++-
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.h |   4 +
 drivers/gpu/drm/i915/gt/uc/intel_huc.c     |  29 +++++
 drivers/gpu/drm/i915/gt/uc/intel_huc.h     |   2 +
 drivers/gpu/drm/i915/i915_debugfs.c        | 131 +++------------------
 7 files changed, 190 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 827d75073879..9647002b0e2f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -723,3 +723,47 @@ int intel_guc_allocate_and_map_vma(struct intel_guc *guc, u32 size,
 
 	return 0;
 }
+
+/**
+ * intel_guc_load_status - dump information about GuC load status
+ * @guc: the GuC
+ * @p: the &drm_printer
+ *
+ * Pretty printer for GuC load status.
+ */
+void intel_guc_load_status(const struct intel_guc *guc, struct drm_printer *p)
+{
+	struct intel_gt *gt = guc_to_gt(guc);
+	struct intel_uncore *uncore = gt->uncore;
+	intel_wakeref_t wakeref;
+
+	if (!intel_guc_is_supported(guc)) {
+		drm_printf(p, "GuC not supported\n");
+		return;
+	}
+
+	if (!intel_guc_is_wanted(guc)) {
+		drm_printf(p, "GuC disabled\n");
+		return;
+	}
+
+	intel_uc_fw_dump(&guc->fw, p);
+
+	with_intel_runtime_pm(uncore->rpm, wakeref) {
+		u32 status = intel_uncore_read(uncore, GUC_STATUS);
+		u32 i;
+
+		drm_printf(p, "\nGuC status 0x%08x:\n", status);
+		drm_printf(p, "\tBootrom status = 0x%x\n",
+			   (status & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
+		drm_printf(p, "\tuKernel status = 0x%x\n",
+			   (status & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
+		drm_printf(p, "\tMIA Core status = 0x%x\n",
+			   (status & GS_MIA_MASK) >> GS_MIA_SHIFT);
+		drm_puts(p, "\nScratch registers:\n");
+		for (i = 0; i < 16; i++) {
+			drm_printf(p, "\t%2d: \t0x%x\n",
+				   i, intel_uncore_read(uncore, SOFT_SCRATCH(i)));
+		}
+	}
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 969147bd9973..7d1ae9879b94 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -190,4 +190,6 @@ static inline void intel_guc_disable_msg(struct intel_guc *guc, u32 mask)
 int intel_guc_reset_engine(struct intel_guc *guc,
 			   struct intel_engine_cs *engine);
 
+void intel_guc_load_status(const struct intel_guc *guc, struct drm_printer *p);
+
 #endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
index caed0d57e704..fbb73b8d9178 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
@@ -55,7 +55,7 @@ static int guc_action_control_log(struct intel_guc *guc, bool enable,
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
 
-static inline struct intel_guc *log_to_guc(struct intel_guc_log *log)
+static inline struct intel_guc *log_to_guc(const struct intel_guc_log *log)
 {
 	return container_of(log, struct intel_guc, log);
 }
@@ -672,3 +672,95 @@ void intel_guc_log_handle_flush_event(struct intel_guc_log *log)
 {
 	queue_work(system_highpri_wq, &log->relay.flush_work);
 }
+
+static const char *
+stringify_guc_log_type(enum guc_log_buffer_type type)
+{
+	switch (type) {
+	case GUC_ISR_LOG_BUFFER:
+		return "ISR";
+	case GUC_DPC_LOG_BUFFER:
+		return "DPC";
+	case GUC_CRASH_DUMP_LOG_BUFFER:
+		return "CRASH";
+	default:
+		MISSING_CASE(type);
+	}
+
+	return "";
+}
+
+/**
+ * intel_guc_log_info - dump information about GuC log relay
+ * @guc: the GuC
+ * @p: the &drm_printer
+ *
+ * Pretty printer for GuC log info
+ */
+void intel_guc_log_info(struct intel_guc_log *log, struct drm_printer *p)
+{
+	enum guc_log_buffer_type type;
+
+	if (!intel_guc_log_relay_created(log)) {
+		drm_puts(p, "GuC log relay not created\n");
+		return;
+	}
+
+	drm_puts(p, "GuC logging stats:\n");
+
+	drm_printf(p, "\tRelay full count: %u\n", log->relay.full_count);
+
+	for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
+		drm_printf(p, "\t%s:\tflush count %10u, overflow count %10u\n",
+			   stringify_guc_log_type(type),
+			   log->stats[type].flush,
+			   log->stats[type].sampled_overflow);
+	}
+}
+
+/**
+ * intel_guc_log_dump - dump the contents of the GuC log
+ * @log: the GuC log
+ * @p: the &drm_printer
+ * @dump_load_err: dump the log saved on GuC load error
+ *
+ * Pretty printer for the GuC log
+ */
+int intel_guc_log_dump(const struct intel_guc_log *log, struct drm_printer *p,
+		       bool dump_load_err)
+{
+	struct intel_guc *guc = log_to_guc(log);
+	struct intel_uc *uc = container_of(guc, struct intel_uc, guc);
+	struct drm_i915_gem_object *obj = NULL;
+	u32 *map;
+	int i = 0;
+
+	if (!intel_guc_is_supported(guc))
+		return -ENODEV;
+
+	if (dump_load_err)
+		obj = uc->load_err_log;
+	else if (guc->log.vma)
+		obj = guc->log.vma->obj;
+
+	if (!obj)
+		return 0;
+
+	map = i915_gem_object_pin_map(obj, I915_MAP_WC);
+	if (IS_ERR(map)) {
+		DRM_DEBUG("Failed to pin object\n");
+		drm_puts(p, "(log data unaccessible)\n");
+		return PTR_ERR(map);
+	}
+
+	for (i = 0; i < obj->base.size / sizeof(u32); i += 4)
+		drm_printf(p, "0x%08x 0x%08x 0x%08x 0x%08x\n",
+			   *(map + i), *(map + i + 1),
+			   *(map + i + 2), *(map + i + 3));
+
+	drm_puts(p, "\n");
+
+	i915_gem_object_unpin_map(obj);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
index c252c022c5fc..e57b94a1badd 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
@@ -79,4 +79,8 @@ static inline u32 intel_guc_log_get_level(struct intel_guc_log *log)
 	return log->level;
 }
 
+void intel_guc_log_info(struct intel_guc_log *log, struct drm_printer *p);
+int intel_guc_log_dump(const struct intel_guc_log *log, struct drm_printer *p,
+		       bool dump_load_err);
+
 #endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index d73dc21686e7..093d6988cfb1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -218,3 +218,32 @@ int intel_huc_check_status(struct intel_huc *huc)
 
 	return (status & huc->status.mask) == huc->status.value;
 }
+
+/**
+ * intel_huc_load_status - dump information about HuC load status
+ * @huc: the HuC
+ * @p: the &drm_printer
+ *
+ * Pretty printer for HuC load status.
+ */
+void intel_huc_load_status(const struct intel_huc *huc, struct drm_printer *p)
+{
+	struct intel_gt *gt = huc_to_gt(huc);
+	intel_wakeref_t wakeref;
+
+	if (!intel_huc_is_supported(huc)) {
+		drm_printf(p, "HuC not supported\n");
+		return;
+	}
+
+	if (!intel_huc_is_wanted(huc)) {
+		drm_printf(p, "HuC disabled\n");
+		return;
+	}
+
+	intel_uc_fw_dump(&huc->fw, p);
+
+	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+		drm_printf(p, "\nHuC status 0x%08x:\n",
+			   intel_uncore_read(gt->uncore, huc->status.reg));
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 19651b46d6a4..f1299c0138e5 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -57,4 +57,6 @@ static inline bool intel_huc_is_authenticated(const struct intel_huc *huc)
 	return intel_uc_fw_is_running(&huc->fw);
 }
 
+void intel_huc_load_status(const struct intel_huc *huc, struct drm_printer *p);
+
 #endif
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 37cb8b4bf4dc..1bec4cdeb92f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1465,105 +1465,32 @@ static int i915_llc(struct seq_file *m, void *data)
 	return 0;
 }
 
-static int i915_huc_load_status_info(struct seq_file *m, void *data)
+static int i915_huc_info(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	intel_wakeref_t wakeref;
-	struct drm_printer p;
-
-	if (!HAS_GT_UC(dev_priv))
-		return -ENODEV;
-
-	p = drm_seq_file_printer(m);
-	intel_uc_fw_dump(&dev_priv->gt.uc.huc.fw, &p);
-
-	with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref)
-		seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2));
-
-	return 0;
-}
-
-static int i915_guc_load_status_info(struct seq_file *m, void *data)
-{
-	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	intel_wakeref_t wakeref;
-	struct drm_printer p;
+	struct intel_huc *huc = &dev_priv->gt.uc.huc;
+	struct drm_printer p = drm_seq_file_printer(m);
 
-	if (!HAS_GT_UC(dev_priv))
+	if (!intel_huc_is_supported(huc))
 		return -ENODEV;
 
-	p = drm_seq_file_printer(m);
-	intel_uc_fw_dump(&dev_priv->gt.uc.guc.fw, &p);
-
-	with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) {
-		u32 tmp = I915_READ(GUC_STATUS);
-		u32 i;
-
-		seq_printf(m, "\nGuC status 0x%08x:\n", tmp);
-		seq_printf(m, "\tBootrom status = 0x%x\n",
-			   (tmp & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
-		seq_printf(m, "\tuKernel status = 0x%x\n",
-			   (tmp & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
-		seq_printf(m, "\tMIA Core status = 0x%x\n",
-			   (tmp & GS_MIA_MASK) >> GS_MIA_SHIFT);
-		seq_puts(m, "\nScratch registers:\n");
-		for (i = 0; i < 16; i++) {
-			seq_printf(m, "\t%2d: \t0x%x\n",
-				   i, I915_READ(SOFT_SCRATCH(i)));
-		}
-	}
+	intel_huc_load_status(huc, &p);
 
 	return 0;
 }
 
-static const char *
-stringify_guc_log_type(enum guc_log_buffer_type type)
-{
-	switch (type) {
-	case GUC_ISR_LOG_BUFFER:
-		return "ISR";
-	case GUC_DPC_LOG_BUFFER:
-		return "DPC";
-	case GUC_CRASH_DUMP_LOG_BUFFER:
-		return "CRASH";
-	default:
-		MISSING_CASE(type);
-	}
-
-	return "";
-}
-
-static void i915_guc_log_info(struct seq_file *m, struct intel_guc_log *log)
-{
-	enum guc_log_buffer_type type;
-
-	if (!intel_guc_log_relay_created(log)) {
-		seq_puts(m, "GuC log relay not created\n");
-		return;
-	}
-
-	seq_puts(m, "GuC logging stats:\n");
-
-	seq_printf(m, "\tRelay full count: %u\n",
-		   log->relay.full_count);
-
-	for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
-		seq_printf(m, "\t%s:\tflush count %10u, overflow count %10u\n",
-			   stringify_guc_log_type(type),
-			   log->stats[type].flush,
-			   log->stats[type].sampled_overflow);
-	}
-}
-
 static int i915_guc_info(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	struct intel_uc *uc = &dev_priv->gt.uc;
+	struct intel_guc *guc = &dev_priv->gt.uc.guc;
+	struct drm_printer p = drm_seq_file_printer(m);
 
-	if (!intel_uc_uses_guc(uc))
+	if (!intel_guc_is_supported(guc))
 		return -ENODEV;
 
-	i915_guc_log_info(m, &uc->guc.log);
+	intel_guc_load_status(guc, &p);
+	drm_puts(&p, "\n");
+	intel_guc_log_info(&guc->log, &p);
 
 	/* Add more as required ... */
 
@@ -1574,39 +1501,14 @@ static int i915_guc_log_dump(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = m->private;
 	struct drm_i915_private *dev_priv = node_to_i915(node);
+	struct intel_guc *guc = &dev_priv->gt.uc.guc;
 	bool dump_load_err = !!node->info_ent->data;
-	struct drm_i915_gem_object *obj = NULL;
-	u32 *log;
-	int i = 0;
+	struct drm_printer p = drm_seq_file_printer(m);
 
-	if (!HAS_GT_UC(dev_priv))
+	if (!intel_guc_is_supported(guc))
 		return -ENODEV;
 
-	if (dump_load_err)
-		obj = dev_priv->gt.uc.load_err_log;
-	else if (dev_priv->gt.uc.guc.log.vma)
-		obj = dev_priv->gt.uc.guc.log.vma->obj;
-
-	if (!obj)
-		return 0;
-
-	log = i915_gem_object_pin_map(obj, I915_MAP_WC);
-	if (IS_ERR(log)) {
-		DRM_DEBUG("Failed to pin object\n");
-		seq_puts(m, "(log data unaccessible)\n");
-		return PTR_ERR(log);
-	}
-
-	for (i = 0; i < obj->base.size / sizeof(u32); i += 4)
-		seq_printf(m, "0x%08x 0x%08x 0x%08x 0x%08x\n",
-			   *(log + i), *(log + i + 1),
-			   *(log + i + 2), *(log + i + 3));
-
-	seq_putc(m, '\n');
-
-	i915_gem_object_unpin_map(obj);
-
-	return 0;
+	return intel_guc_log_dump(&guc->log, &p, dump_load_err);
 }
 
 static int i915_guc_log_level_get(void *data, u64 *val)
@@ -2302,10 +2204,9 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_gem_fence_regs", i915_gem_fence_regs_info, 0},
 	{"i915_gem_interrupt", i915_interrupt_info, 0},
 	{"i915_guc_info", i915_guc_info, 0},
-	{"i915_guc_load_status", i915_guc_load_status_info, 0},
 	{"i915_guc_log_dump", i915_guc_log_dump, 0},
 	{"i915_guc_load_err_log_dump", i915_guc_log_dump, 0, (void *)1},
-	{"i915_huc_load_status", i915_huc_load_status_info, 0},
+	{"i915_huc_info", i915_huc_info, 0},
 	{"i915_frequency_info", i915_frequency_info, 0},
 	{"i915_drpc_info", i915_drpc_info, 0},
 	{"i915_ring_freq_table", i915_ring_freq_table, 0},
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT
  2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
                   ` (3 preceding siblings ...)
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 4/6] drm/i915/debugfs: move uC printers and update debugfs file names Daniele Ceraolo Spurio
@ 2020-02-28  2:28 ` Daniele Ceraolo Spurio
  2020-02-28  9:24   ` Jani Nikula
  2020-03-03  1:52   ` Andi Shyti
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 6/6] drm/i915/uc: do not free err log on uc_fini Daniele Ceraolo Spurio
                   ` (4 subsequent siblings)
  9 siblings, 2 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-02-28  2:28 UTC (permalink / raw)
  To: intel-gfx

uC is a component of the GT, so it makes sense for the uC debugfs files
to be in the GT folder. A subfolder has been used to keep the same
structure we have for the code.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Andi Shyti <andi.shyti@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Tony Ye <tony.ye@intel.com>
---
 drivers/gpu/drm/i915/Makefile                |   7 +-
 drivers/gpu/drm/i915/gt/debugfs_gt.c         |   3 +
 drivers/gpu/drm/i915/gt/uc/debugfs_guc.c     |  42 ++++++
 drivers/gpu/drm/i915/gt/uc/debugfs_guc.h     |  14 ++
 drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c | 123 +++++++++++++++++
 drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h |  14 ++
 drivers/gpu/drm/i915/gt/uc/debugfs_huc.c     |  36 +++++
 drivers/gpu/drm/i915/gt/uc/debugfs_huc.h     |  14 ++
 drivers/gpu/drm/i915/gt/uc/debugfs_uc.c      |  31 +++++
 drivers/gpu/drm/i915/gt/uc/debugfs_uc.h      |  43 ++++++
 drivers/gpu/drm/i915/gt/uc/intel_guc.h       |   5 +
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.c   |   5 -
 drivers/gpu/drm/i915/i915_debugfs.c          | 137 -------------------
 13 files changed, 331 insertions(+), 143 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc.h
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_huc.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_huc.h
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_uc.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_uc.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index fe6d580869d7..c5cd9b1390e7 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -164,7 +164,12 @@ i915-y += \
 	  intel_wopcm.o
 
 # general-purpose microcontroller (GuC) support
-i915-y += gt/uc/intel_uc.o \
+i915-y += \
+	  gt/uc/debugfs_guc.o \
+	  gt/uc/debugfs_guc_log.o \
+	  gt/uc/debugfs_huc.o \
+	  gt/uc/debugfs_uc.o \
+	  gt/uc/intel_uc.o \
 	  gt/uc/intel_uc_fw.o \
 	  gt/uc/intel_guc.o \
 	  gt/uc/intel_guc_ads.o \
diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.c b/drivers/gpu/drm/i915/gt/debugfs_gt.c
index 75255aaacaed..eb403cc3c48d 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_engines.h"
 #include "debugfs_gt.h"
 #include "debugfs_gt_pm.h"
+#include "uc/debugfs_uc.h"
 #include "i915_drv.h"
 
 void debugfs_gt_register(struct intel_gt *gt)
@@ -24,6 +25,8 @@ void debugfs_gt_register(struct intel_gt *gt)
 
 	debugfs_engines_register(gt, root);
 	debugfs_gt_pm_register(gt, root);
+
+	debugfs_uc_register(&gt->uc, root);
 }
 
 void debugfs_gt_register_files(struct intel_gt *gt,
diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_guc.c b/drivers/gpu/drm/i915/gt/uc/debugfs_guc.c
new file mode 100644
index 000000000000..1adac42e1596
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/debugfs_guc.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#include <drm/drm_print.h>
+
+#include "debugfs_guc_log.h"
+#include "debugfs_uc.h"
+#include "intel_guc.h"
+
+static int guc_info_show(struct seq_file *m, void *data)
+{
+	struct intel_guc *guc = m->private;
+	struct drm_printer p = drm_seq_file_printer(m);
+
+	if (!intel_guc_is_supported(guc))
+		return -ENODEV;
+
+	intel_guc_load_status(guc, &p);
+	drm_puts(&p, "\n");
+	intel_guc_log_info(&guc->log, &p);
+
+	/* Add more as required ... */
+
+	return 0;
+}
+DEFINE_UC_DEBUGFS_ATTRIBUTE(guc_info);
+
+void debugfs_guc_register(struct intel_guc *guc, struct dentry *root)
+{
+	static const struct debugfs_uc_file files[] = {
+		{ "guc_info", &guc_info_fops },
+	};
+
+	if (!intel_guc_is_supported(guc))
+		return;
+
+	debugfs_uc_register_files(files, root, guc);
+	debugfs_guc_log_register(&guc->log, root);
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_guc.h b/drivers/gpu/drm/i915/gt/uc/debugfs_guc.h
new file mode 100644
index 000000000000..d9188def7d12
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/debugfs_guc.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#ifndef DEBUGFS_GUC_H
+#define DEBUGFS_GUC_H
+
+struct intel_guc;
+struct dentry;
+
+void debugfs_guc_register(struct intel_guc *guc, struct dentry *root);
+
+#endif /* DEBUGFS_GUC_H */
diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c b/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c
new file mode 100644
index 000000000000..a560a392aa09
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#include <linux/fs.h>
+#include <drm/drm_print.h>
+
+#include "debugfs_uc.h"
+#include "intel_guc.h"
+#include "intel_guc_log.h"
+
+static int guc_log_dump_show(struct seq_file *m, void *data)
+{
+	struct drm_printer p = drm_seq_file_printer(m);
+
+	return intel_guc_log_dump(m->private, &p, false);
+}
+DEFINE_UC_DEBUGFS_ATTRIBUTE(guc_log_dump);
+
+static int guc_load_err_log_dump_show(struct seq_file *m, void *data)
+{
+	struct drm_printer p = drm_seq_file_printer(m);
+
+	return intel_guc_log_dump(m->private, &p, true);
+}
+DEFINE_UC_DEBUGFS_ATTRIBUTE(guc_load_err_log_dump);
+
+static int guc_log_level_get(void *data, u64 *val)
+{
+	struct intel_guc_log *log = data;
+
+	if (!intel_guc_is_used(log_to_guc(log)))
+		return -ENODEV;
+
+	*val = intel_guc_log_get_level(log);
+
+	return 0;
+}
+
+static int guc_log_level_set(void *data, u64 val)
+{
+	struct intel_guc_log *log = data;
+
+	if (!intel_guc_is_used(log_to_guc(log)))
+		return -ENODEV;
+
+	return intel_guc_log_set_level(log, val);
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(guc_log_level_fops,
+			guc_log_level_get, guc_log_level_set,
+			"%lld\n");
+
+static int guc_log_relay_open(struct inode *inode, struct file *file)
+{
+	struct intel_guc_log *log = inode->i_private;
+
+	if (!intel_guc_is_ready(log_to_guc(log)))
+		return -ENODEV;
+
+	file->private_data = log;
+
+	return intel_guc_log_relay_open(log);
+}
+
+static ssize_t
+guc_log_relay_write(struct file *filp,
+		    const char __user *ubuf,
+		    size_t cnt,
+		    loff_t *ppos)
+{
+	struct intel_guc_log *log = filp->private_data;
+	int val;
+	int ret;
+
+	ret = kstrtoint_from_user(ubuf, cnt, 0, &val);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Enable and start the guc log relay on value of 1.
+	 * Flush log relay for any other value.
+	 */
+	if (val == 1)
+		ret = intel_guc_log_relay_start(log);
+	else
+		intel_guc_log_relay_flush(log);
+
+	return ret ?: cnt;
+}
+
+static int guc_log_relay_release(struct inode *inode, struct file *file)
+{
+	struct intel_guc_log *log = inode->i_private;
+
+	intel_guc_log_relay_close(log);
+	return 0;
+}
+
+static const struct file_operations guc_log_relay_fops = {
+	.owner = THIS_MODULE,
+	.open = guc_log_relay_open,
+	.write = guc_log_relay_write,
+	.release = guc_log_relay_release,
+};
+
+void debugfs_guc_log_register(struct intel_guc_log *log, struct dentry *root)
+{
+	static const struct debugfs_uc_file files[] = {
+		{ "guc_log_dump", &guc_log_dump_fops },
+		{ "guc_load_err_log_dump", &guc_load_err_log_dump_fops },
+		{ "guc_log_level", &guc_log_level_fops },
+		{ "guc_log_relay", &guc_log_relay_fops },
+	};
+
+	if (!intel_guc_is_supported(log_to_guc(log)))
+		return;
+
+	debugfs_uc_register_files(files, root, log);
+}
+
diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h b/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h
new file mode 100644
index 000000000000..bc3ce784667e
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#ifndef DEBUGFS_GUC_LOG_H
+#define DEBUGFS_GUC_LOG_H
+
+struct intel_guc_log;
+struct dentry;
+
+void debugfs_guc_log_register(struct intel_guc_log *log, struct dentry *root);
+
+#endif /* DEBUGFS_GUC_LOG_H */
diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_huc.c b/drivers/gpu/drm/i915/gt/uc/debugfs_huc.c
new file mode 100644
index 000000000000..b58740872333
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/debugfs_huc.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#include <drm/drm_print.h>
+
+#include "debugfs_uc.h"
+#include "intel_huc.h"
+
+static int huc_info_show(struct seq_file *m, void *data)
+{
+	struct intel_huc *huc = m->private;
+	struct drm_printer p = drm_seq_file_printer(m);
+
+	if (!intel_huc_is_supported(huc))
+		return -ENODEV;
+
+	intel_huc_load_status(huc, &p);
+
+	return 0;
+}
+DEFINE_UC_DEBUGFS_ATTRIBUTE(huc_info);
+
+void debugfs_huc_register(struct intel_huc *huc, struct dentry *root)
+{
+	static const struct debugfs_uc_file files[] = {
+		{ "huc_info", &huc_info_fops },
+	};
+
+	if (!intel_huc_is_supported(huc))
+		return;
+
+	debugfs_uc_register_files(files, root, huc);
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_huc.h b/drivers/gpu/drm/i915/gt/uc/debugfs_huc.h
new file mode 100644
index 000000000000..761f3ee3ed6a
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/debugfs_huc.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#ifndef DEBUGFS_HUC_H
+#define DEBUGFS_HUC_H
+
+struct intel_huc;
+struct dentry;
+
+void debugfs_huc_register(struct intel_huc *huc, struct dentry *root);
+
+#endif /* DEBUGFS_HUC_H */
diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_uc.c b/drivers/gpu/drm/i915/gt/uc/debugfs_uc.c
new file mode 100644
index 000000000000..e925443183be
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/debugfs_uc.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#include <linux/debugfs.h>
+
+#include "debugfs_guc.h"
+#include "debugfs_huc.h"
+#include "debugfs_uc.h"
+#include "intel_uc.h"
+
+void debugfs_uc_register(struct intel_uc *uc, struct dentry *gt_root)
+{
+	struct dentry *root;
+
+	if (!gt_root)
+		return;
+
+	/* GuC and HuC go always in pair, no need to check both */
+	if (!intel_uc_supports_guc(uc))
+		return;
+
+	root = debugfs_create_dir("uc", gt_root);
+	if (IS_ERR(root))
+		return;
+
+	debugfs_guc_register(&uc->guc, root);
+	debugfs_huc_register(&uc->huc, root);
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_uc.h b/drivers/gpu/drm/i915/gt/uc/debugfs_uc.h
new file mode 100644
index 000000000000..9aa4b7e52770
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/debugfs_uc.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#ifndef DEBUGFS_UC_H
+#define DEBUGFS_UC_H
+
+#include <linux/file.h>
+
+struct intel_uc;
+
+#define DEFINE_UC_DEBUGFS_ATTRIBUTE(__name)				\
+	static int __name ## _open(struct inode *inode, struct file *file) \
+{									\
+	return single_open(file, __name ## _show, inode->i_private);	\
+}									\
+static const struct file_operations __name ## _fops = {			\
+	.owner = THIS_MODULE,						\
+	.open = __name ## _open,					\
+	.read = seq_read,						\
+	.llseek = seq_lseek,						\
+	.release = single_release,					\
+}
+
+void debugfs_uc_register(struct intel_uc *uc, struct dentry *gt_root);
+
+struct debugfs_uc_file {
+	const char *name;
+	const struct file_operations *fops;
+};
+
+#define debugfs_uc_register_files(files__, root__, data__) \
+do { \
+	int i__ = 0; \
+	for (i__ = 0; i__ < ARRAY_SIZE(files__); i__++) { \
+		debugfs_create_file(files__[i__].name, \
+				    0444, root__, data__, \
+				    files__[i__].fops); \
+	} \
+} while (0)
+
+#endif /* DEBUGFS_UC_H */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 7d1ae9879b94..2b79a8bf8d27 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -74,6 +74,11 @@ struct intel_guc {
 	struct mutex send_mutex;
 };
 
+static inline struct intel_guc *log_to_guc(const struct intel_guc_log *log)
+{
+	return container_of(log, struct intel_guc, log);
+}
+
 static
 inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
 {
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
index fbb73b8d9178..cfe2f9321680 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
@@ -55,11 +55,6 @@ static int guc_action_control_log(struct intel_guc *guc, bool enable,
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
 
-static inline struct intel_guc *log_to_guc(const struct intel_guc_log *log)
-{
-	return container_of(log, struct intel_guc, log);
-}
-
 static void guc_log_enable_flush_events(struct intel_guc_log *log)
 {
 	intel_guc_enable_msg(log_to_guc(log),
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1bec4cdeb92f..a8b6f0fd9439 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -37,7 +37,6 @@
 #include "gt/intel_reset.h"
 #include "gt/intel_rc6.h"
 #include "gt/intel_rps.h"
-#include "gt/uc/intel_guc_submission.h"
 
 #include "i915_debugfs.h"
 #include "i915_debugfs_params.h"
@@ -1465,136 +1464,6 @@ static int i915_llc(struct seq_file *m, void *data)
 	return 0;
 }
 
-static int i915_huc_info(struct seq_file *m, void *data)
-{
-	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	struct intel_huc *huc = &dev_priv->gt.uc.huc;
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	if (!intel_huc_is_supported(huc))
-		return -ENODEV;
-
-	intel_huc_load_status(huc, &p);
-
-	return 0;
-}
-
-static int i915_guc_info(struct seq_file *m, void *data)
-{
-	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	struct intel_guc *guc = &dev_priv->gt.uc.guc;
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	if (!intel_guc_is_supported(guc))
-		return -ENODEV;
-
-	intel_guc_load_status(guc, &p);
-	drm_puts(&p, "\n");
-	intel_guc_log_info(&guc->log, &p);
-
-	/* Add more as required ... */
-
-	return 0;
-}
-
-static int i915_guc_log_dump(struct seq_file *m, void *data)
-{
-	struct drm_info_node *node = m->private;
-	struct drm_i915_private *dev_priv = node_to_i915(node);
-	struct intel_guc *guc = &dev_priv->gt.uc.guc;
-	bool dump_load_err = !!node->info_ent->data;
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	if (!intel_guc_is_supported(guc))
-		return -ENODEV;
-
-	return intel_guc_log_dump(&guc->log, &p, dump_load_err);
-}
-
-static int i915_guc_log_level_get(void *data, u64 *val)
-{
-	struct drm_i915_private *dev_priv = data;
-	struct intel_uc *uc = &dev_priv->gt.uc;
-
-	if (!intel_uc_uses_guc(uc))
-		return -ENODEV;
-
-	*val = intel_guc_log_get_level(&uc->guc.log);
-
-	return 0;
-}
-
-static int i915_guc_log_level_set(void *data, u64 val)
-{
-	struct drm_i915_private *dev_priv = data;
-	struct intel_uc *uc = &dev_priv->gt.uc;
-
-	if (!intel_uc_uses_guc(uc))
-		return -ENODEV;
-
-	return intel_guc_log_set_level(&uc->guc.log, val);
-}
-
-DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_level_fops,
-			i915_guc_log_level_get, i915_guc_log_level_set,
-			"%lld\n");
-
-static int i915_guc_log_relay_open(struct inode *inode, struct file *file)
-{
-	struct drm_i915_private *i915 = inode->i_private;
-	struct intel_guc *guc = &i915->gt.uc.guc;
-	struct intel_guc_log *log = &guc->log;
-
-	if (!intel_guc_is_ready(guc))
-		return -ENODEV;
-
-	file->private_data = log;
-
-	return intel_guc_log_relay_open(log);
-}
-
-static ssize_t
-i915_guc_log_relay_write(struct file *filp,
-			 const char __user *ubuf,
-			 size_t cnt,
-			 loff_t *ppos)
-{
-	struct intel_guc_log *log = filp->private_data;
-	int val;
-	int ret;
-
-	ret = kstrtoint_from_user(ubuf, cnt, 0, &val);
-	if (ret < 0)
-		return ret;
-
-	/*
-	 * Enable and start the guc log relay on value of 1.
-	 * Flush log relay for any other value.
-	 */
-	if (val == 1)
-		ret = intel_guc_log_relay_start(log);
-	else
-		intel_guc_log_relay_flush(log);
-
-	return ret ?: cnt;
-}
-
-static int i915_guc_log_relay_release(struct inode *inode, struct file *file)
-{
-	struct drm_i915_private *i915 = inode->i_private;
-	struct intel_guc *guc = &i915->gt.uc.guc;
-
-	intel_guc_log_relay_close(&guc->log);
-	return 0;
-}
-
-static const struct file_operations i915_guc_log_relay_fops = {
-	.owner = THIS_MODULE,
-	.open = i915_guc_log_relay_open,
-	.write = i915_guc_log_relay_write,
-	.release = i915_guc_log_relay_release,
-};
-
 static int i915_runtime_pm_status(struct seq_file *m, void *unused)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -2203,10 +2072,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_gem_objects", i915_gem_object_info, 0},
 	{"i915_gem_fence_regs", i915_gem_fence_regs_info, 0},
 	{"i915_gem_interrupt", i915_interrupt_info, 0},
-	{"i915_guc_info", i915_guc_info, 0},
-	{"i915_guc_log_dump", i915_guc_log_dump, 0},
-	{"i915_guc_load_err_log_dump", i915_guc_log_dump, 0, (void *)1},
-	{"i915_huc_info", i915_huc_info, 0},
 	{"i915_frequency_info", i915_frequency_info, 0},
 	{"i915_drpc_info", i915_drpc_info, 0},
 	{"i915_ring_freq_table", i915_ring_freq_table, 0},
@@ -2236,8 +2101,6 @@ static const struct i915_debugfs_files {
 	{"i915_error_state", &i915_error_state_fops},
 	{"i915_gpu_info", &i915_gpu_info_fops},
 #endif
-	{"i915_guc_log_level", &i915_guc_log_level_fops},
-	{"i915_guc_log_relay", &i915_guc_log_relay_fops},
 };
 
 int i915_debugfs_register(struct drm_i915_private *dev_priv)
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH 6/6] drm/i915/uc: do not free err log on uc_fini
  2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
                   ` (4 preceding siblings ...)
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT Daniele Ceraolo Spurio
@ 2020-02-28  2:28 ` Daniele Ceraolo Spurio
  2020-02-28  5:54 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Re-org uC debugfs files and move them under GT Patchwork
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-02-28  2:28 UTC (permalink / raw)
  To: intel-gfx

we do call uc_fini if there is an issue while loading the GuC, so we
can't delete in there the logs we need to debug the load failure.
Moving the log free to driver remove ensures the logs stick around ong
enough for us to dump them.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt.c    | 3 +--
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 9 +++++++--
 drivers/gpu/drm/i915/gt/uc/intel_uc.h | 1 +
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 3dea8881e915..eda66b0d44bd 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -635,8 +635,7 @@ void intel_gt_driver_remove(struct intel_gt *gt)
 {
 	__intel_gt_disable(gt);
 
-	intel_uc_fini_hw(&gt->uc);
-	intel_uc_fini(&gt->uc);
+	intel_uc_driver_remove(&gt->uc);
 
 	intel_engines_release(gt);
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index a4cbe06e06bd..b11e564ef22e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -131,6 +131,13 @@ static void __uc_free_load_err_log(struct intel_uc *uc)
 		i915_gem_object_put(log);
 }
 
+void intel_uc_driver_remove(struct intel_uc *uc)
+{
+	intel_uc_fini_hw(uc);
+	intel_uc_fini(uc);
+	__uc_free_load_err_log(uc);
+}
+
 static inline bool guc_communication_enabled(struct intel_guc *guc)
 {
 	return intel_guc_ct_enabled(&guc->ct);
@@ -311,8 +318,6 @@ static void __uc_fini(struct intel_uc *uc)
 {
 	intel_huc_fini(&uc->huc);
 	intel_guc_fini(&uc->guc);
-
-	__uc_free_load_err_log(uc);
 }
 
 static int __uc_sanitize(struct intel_uc *uc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 2f7d3028af08..52b6e4e64ced 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -34,6 +34,7 @@ struct intel_uc {
 
 void intel_uc_init_early(struct intel_uc *uc);
 void intel_uc_driver_late_release(struct intel_uc *uc);
+void intel_uc_driver_remove(struct intel_uc *uc);
 void intel_uc_init_mmio(struct intel_uc *uc);
 void intel_uc_reset_prepare(struct intel_uc *uc);
 void intel_uc_suspend(struct intel_uc *uc);
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Re-org uC debugfs files and move them under GT
  2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
                   ` (5 preceding siblings ...)
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 6/6] drm/i915/uc: do not free err log on uc_fini Daniele Ceraolo Spurio
@ 2020-02-28  5:54 ` Patchwork
  2020-02-28  5:58 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2020-02-28  5:54 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: Re-org uC debugfs files and move them under GT
URL   : https://patchwork.freedesktop.org/series/74051/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
342d7bf407c2 drm/i915/guc: drop stage_pool debugfs
dfe41cbafaee drm/i915/uc: mark structure passed to checker functions as const
4d8f194b0199 drm/i915/huc: make "support huc" reflect HW capabilities
930ae359ffad drm/i915/debugfs: move uC printers and update debugfs file names
a4b27a2cdac1 drm/i915/uc: Move uC debugfs to its own folder under GT
-:57: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#57: 
new file mode 100644

-:410: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'files__' - possible side-effects?
#410: FILE: drivers/gpu/drm/i915/gt/uc/debugfs_uc.h:33:
+#define debugfs_uc_register_files(files__, root__, data__) \
+do { \
+	int i__ = 0; \
+	for (i__ = 0; i__ < ARRAY_SIZE(files__); i__++) { \
+		debugfs_create_file(files__[i__].name, \
+				    0444, root__, data__, \
+				    files__[i__].fops); \
+	} \
+} while (0)

total: 0 errors, 1 warnings, 1 checks, 528 lines checked
2919c449e528 drm/i915/uc: do not free err log on uc_fini

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Re-org uC debugfs files and move them under GT
  2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
                   ` (6 preceding siblings ...)
  2020-02-28  5:54 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Re-org uC debugfs files and move them under GT Patchwork
@ 2020-02-28  5:58 ` Patchwork
  2020-02-28  6:20 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2020-02-29 16:42 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2020-02-28  5:58 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: Re-org uC debugfs files and move them under GT
URL   : https://patchwork.freedesktop.org/series/74051/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915/guc: drop stage_pool debugfs
Okay!

Commit: drm/i915/uc: mark structure passed to checker functions as const
Okay!

Commit: drm/i915/huc: make "support huc" reflect HW capabilities
Okay!

Commit: drm/i915/debugfs: move uC printers and update debugfs file names
Okay!

Commit: drm/i915/uc: Move uC debugfs to its own folder under GT
+drivers/gpu/drm/i915/gt/uc/debugfs_guc.c:31:6: warning: symbol 'debugfs_guc_register' was not declared. Should it be static?
+drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c:109:6: warning: symbol 'debugfs_guc_log_register' was not declared. Should it be static?
+drivers/gpu/drm/i915/gt/uc/debugfs_huc.c:26:6: warning: symbol 'debugfs_huc_register' was not declared. Should it be static?

Commit: drm/i915/uc: do not free err log on uc_fini
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for Re-org uC debugfs files and move them under GT
  2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
                   ` (7 preceding siblings ...)
  2020-02-28  5:58 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2020-02-28  6:20 ` Patchwork
  2020-02-29 16:42 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2020-02-28  6:20 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: Re-org uC debugfs files and move them under GT
URL   : https://patchwork.freedesktop.org/series/74051/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8024 -> Patchwork_16755
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_16755:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@runner@aborted:
    - {fi-tgl-u}:         NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/fi-tgl-u/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_16755 that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [FAIL][2] ([i915#262]) -> [PASS][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-kbl-8809g:       [FAIL][4] ([i915#192] / [i915#193] / [i915#194]) -> [FAIL][5] ([i915#1209])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/fi-kbl-8809g/igt@runner@aborted.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/fi-kbl-8809g/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1209]: https://gitlab.freedesktop.org/drm/intel/issues/1209
  [i915#1233]: https://gitlab.freedesktop.org/drm/intel/issues/1233
  [i915#192]: https://gitlab.freedesktop.org/drm/intel/issues/192
  [i915#193]: https://gitlab.freedesktop.org/drm/intel/issues/193
  [i915#194]: https://gitlab.freedesktop.org/drm/intel/issues/194
  [i915#262]: https://gitlab.freedesktop.org/drm/intel/issues/262


Participating hosts (41 -> 39)
------------------------------

  Additional (4): fi-hsw-peppy fi-gdg-551 fi-bwr-2160 fi-bsw-n3050 
  Missing    (6): fi-ilk-650 fi-ctg-p8600 fi-blb-e6850 fi-byt-clapper fi-bsw-nick fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8024 -> Patchwork_16755

  CI-20190529: 20190529
  CI_DRM_8024: 3290680f9735978238a1d3df1efa83326a843327 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5474: 1be610f852de155cd915e7cda65cb2737adf04d4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16755: 2919c449e528a523e408e4c810f274d023dae2de @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2919c449e528 drm/i915/uc: do not free err log on uc_fini
a4b27a2cdac1 drm/i915/uc: Move uC debugfs to its own folder under GT
930ae359ffad drm/i915/debugfs: move uC printers and update debugfs file names
4d8f194b0199 drm/i915/huc: make "support huc" reflect HW capabilities
dfe41cbafaee drm/i915/uc: mark structure passed to checker functions as const
342d7bf407c2 drm/i915/guc: drop stage_pool debugfs

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 2/6] drm/i915/uc: mark structure passed to checker functions as const
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 2/6] drm/i915/uc: mark structure passed to checker functions as const Daniele Ceraolo Spurio
@ 2020-02-28  9:18   ` Jani Nikula
  2020-02-29  0:20     ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2020-02-28  9:18 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio, intel-gfx

On Thu, 27 Feb 2020, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
> Follow-up patches will pass const objects from debugfs to some those
> functions, so we need to be ready.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt.h             |  6 +++---
>  drivers/gpu/drm/i915/gt/uc/intel_guc.h         | 10 +++++-----
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h      |  2 +-
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.h  |  6 +++---
>  drivers/gpu/drm/i915/gt/uc/intel_huc.h         |  8 ++++----
>  drivers/gpu/drm/i915/gt/uc/intel_uc.h          |  2 +-
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h       | 18 +++++++++---------
>  7 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
> index 4fac043750aa..f9fbe645478d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
> @@ -18,17 +18,17 @@ struct drm_i915_private;
>  		  ##__VA_ARGS__);					\
>  } while (0)
>  
> -static inline struct intel_gt *uc_to_gt(struct intel_uc *uc)
> +static inline struct intel_gt *uc_to_gt(const struct intel_uc *uc)
>  {
>  	return container_of(uc, struct intel_gt, uc);
>  }
>  
> -static inline struct intel_gt *guc_to_gt(struct intel_guc *guc)
> +static inline struct intel_gt *guc_to_gt(const struct intel_guc *guc)
>  {
>  	return container_of(guc, struct intel_gt, uc.guc);
>  }
>  
> -static inline struct intel_gt *huc_to_gt(struct intel_huc *huc)
> +static inline struct intel_gt *huc_to_gt(const struct intel_huc *huc)
>  {
>  	return container_of(huc, struct intel_gt, uc.huc);
>  }

Not fond of the fact that these cast the const away. If you can return
const also, fine, but const in, non-const out is not fine.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT Daniele Ceraolo Spurio
@ 2020-02-28  9:24   ` Jani Nikula
  2020-03-03  1:52   ` Andi Shyti
  1 sibling, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2020-02-28  9:24 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio, intel-gfx

On Thu, 27 Feb 2020, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
> uC is a component of the GT, so it makes sense for the uC debugfs files
> to be in the GT folder. A subfolder has been used to keep the same
> structure we have for the code.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Andi Shyti <andi.shyti@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Tony Ye <tony.ye@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                |   7 +-
>  drivers/gpu/drm/i915/gt/debugfs_gt.c         |   3 +
>  drivers/gpu/drm/i915/gt/uc/debugfs_guc.c     |  42 ++++++
>  drivers/gpu/drm/i915/gt/uc/debugfs_guc.h     |  14 ++
>  drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c | 123 +++++++++++++++++
>  drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h |  14 ++
>  drivers/gpu/drm/i915/gt/uc/debugfs_huc.c     |  36 +++++
>  drivers/gpu/drm/i915/gt/uc/debugfs_huc.h     |  14 ++
>  drivers/gpu/drm/i915/gt/uc/debugfs_uc.c      |  31 +++++
>  drivers/gpu/drm/i915/gt/uc/debugfs_uc.h      |  43 ++++++
>  drivers/gpu/drm/i915/gt/uc/intel_guc.h       |   5 +
>  drivers/gpu/drm/i915/gt/uc/intel_guc_log.c   |   5 -
>  drivers/gpu/drm/i915/i915_debugfs.c          | 137 -------------------
>  13 files changed, 331 insertions(+), 143 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc.c
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc.h
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_huc.c
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_huc.h
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_uc.c
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/debugfs_uc.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index fe6d580869d7..c5cd9b1390e7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -164,7 +164,12 @@ i915-y += \
>  	  intel_wopcm.o
>  
>  # general-purpose microcontroller (GuC) support
> -i915-y += gt/uc/intel_uc.o \
> +i915-y += \
> +	  gt/uc/debugfs_guc.o \
> +	  gt/uc/debugfs_guc_log.o \
> +	  gt/uc/debugfs_huc.o \
> +	  gt/uc/debugfs_uc.o \
> +	  gt/uc/intel_uc.o \
>  	  gt/uc/intel_uc_fw.o \
>  	  gt/uc/intel_guc.o \
>  	  gt/uc/intel_guc_ads.o \
> diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.c b/drivers/gpu/drm/i915/gt/debugfs_gt.c
> index 75255aaacaed..eb403cc3c48d 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_engines.h"
>  #include "debugfs_gt.h"
>  #include "debugfs_gt_pm.h"
> +#include "uc/debugfs_uc.h"
>  #include "i915_drv.h"
>  
>  void debugfs_gt_register(struct intel_gt *gt)
> @@ -24,6 +25,8 @@ void debugfs_gt_register(struct intel_gt *gt)
>  
>  	debugfs_engines_register(gt, root);
>  	debugfs_gt_pm_register(gt, root);
> +
> +	debugfs_uc_register(&gt->uc, root);


I've complained about the two above, and I'll also complain about this
one: please don't use debugfs_ prefix for non-static functions. It's for
fs/debugfs/* not us.

BR,
Jani.



>  }
>  
>  void debugfs_gt_register_files(struct intel_gt *gt,
> diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_guc.c b/drivers/gpu/drm/i915/gt/uc/debugfs_guc.c
> new file mode 100644
> index 000000000000..1adac42e1596
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/debugfs_guc.c
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: MIT
> +
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#include <drm/drm_print.h>
> +
> +#include "debugfs_guc_log.h"
> +#include "debugfs_uc.h"
> +#include "intel_guc.h"
> +
> +static int guc_info_show(struct seq_file *m, void *data)
> +{
> +	struct intel_guc *guc = m->private;
> +	struct drm_printer p = drm_seq_file_printer(m);
> +
> +	if (!intel_guc_is_supported(guc))
> +		return -ENODEV;
> +
> +	intel_guc_load_status(guc, &p);
> +	drm_puts(&p, "\n");
> +	intel_guc_log_info(&guc->log, &p);
> +
> +	/* Add more as required ... */
> +
> +	return 0;
> +}
> +DEFINE_UC_DEBUGFS_ATTRIBUTE(guc_info);
> +
> +void debugfs_guc_register(struct intel_guc *guc, struct dentry *root)
> +{
> +	static const struct debugfs_uc_file files[] = {
> +		{ "guc_info", &guc_info_fops },
> +	};
> +
> +	if (!intel_guc_is_supported(guc))
> +		return;
> +
> +	debugfs_uc_register_files(files, root, guc);
> +	debugfs_guc_log_register(&guc->log, root);
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_guc.h b/drivers/gpu/drm/i915/gt/uc/debugfs_guc.h
> new file mode 100644
> index 000000000000..d9188def7d12
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/debugfs_guc.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#ifndef DEBUGFS_GUC_H
> +#define DEBUGFS_GUC_H
> +
> +struct intel_guc;
> +struct dentry;
> +
> +void debugfs_guc_register(struct intel_guc *guc, struct dentry *root);
> +
> +#endif /* DEBUGFS_GUC_H */
> diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c b/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c
> new file mode 100644
> index 000000000000..a560a392aa09
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: MIT
> +
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#include <linux/fs.h>
> +#include <drm/drm_print.h>
> +
> +#include "debugfs_uc.h"
> +#include "intel_guc.h"
> +#include "intel_guc_log.h"
> +
> +static int guc_log_dump_show(struct seq_file *m, void *data)
> +{
> +	struct drm_printer p = drm_seq_file_printer(m);
> +
> +	return intel_guc_log_dump(m->private, &p, false);
> +}
> +DEFINE_UC_DEBUGFS_ATTRIBUTE(guc_log_dump);
> +
> +static int guc_load_err_log_dump_show(struct seq_file *m, void *data)
> +{
> +	struct drm_printer p = drm_seq_file_printer(m);
> +
> +	return intel_guc_log_dump(m->private, &p, true);
> +}
> +DEFINE_UC_DEBUGFS_ATTRIBUTE(guc_load_err_log_dump);
> +
> +static int guc_log_level_get(void *data, u64 *val)
> +{
> +	struct intel_guc_log *log = data;
> +
> +	if (!intel_guc_is_used(log_to_guc(log)))
> +		return -ENODEV;
> +
> +	*val = intel_guc_log_get_level(log);
> +
> +	return 0;
> +}
> +
> +static int guc_log_level_set(void *data, u64 val)
> +{
> +	struct intel_guc_log *log = data;
> +
> +	if (!intel_guc_is_used(log_to_guc(log)))
> +		return -ENODEV;
> +
> +	return intel_guc_log_set_level(log, val);
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(guc_log_level_fops,
> +			guc_log_level_get, guc_log_level_set,
> +			"%lld\n");
> +
> +static int guc_log_relay_open(struct inode *inode, struct file *file)
> +{
> +	struct intel_guc_log *log = inode->i_private;
> +
> +	if (!intel_guc_is_ready(log_to_guc(log)))
> +		return -ENODEV;
> +
> +	file->private_data = log;
> +
> +	return intel_guc_log_relay_open(log);
> +}
> +
> +static ssize_t
> +guc_log_relay_write(struct file *filp,
> +		    const char __user *ubuf,
> +		    size_t cnt,
> +		    loff_t *ppos)
> +{
> +	struct intel_guc_log *log = filp->private_data;
> +	int val;
> +	int ret;
> +
> +	ret = kstrtoint_from_user(ubuf, cnt, 0, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Enable and start the guc log relay on value of 1.
> +	 * Flush log relay for any other value.
> +	 */
> +	if (val == 1)
> +		ret = intel_guc_log_relay_start(log);
> +	else
> +		intel_guc_log_relay_flush(log);
> +
> +	return ret ?: cnt;
> +}
> +
> +static int guc_log_relay_release(struct inode *inode, struct file *file)
> +{
> +	struct intel_guc_log *log = inode->i_private;
> +
> +	intel_guc_log_relay_close(log);
> +	return 0;
> +}
> +
> +static const struct file_operations guc_log_relay_fops = {
> +	.owner = THIS_MODULE,
> +	.open = guc_log_relay_open,
> +	.write = guc_log_relay_write,
> +	.release = guc_log_relay_release,
> +};
> +
> +void debugfs_guc_log_register(struct intel_guc_log *log, struct dentry *root)
> +{
> +	static const struct debugfs_uc_file files[] = {
> +		{ "guc_log_dump", &guc_log_dump_fops },
> +		{ "guc_load_err_log_dump", &guc_load_err_log_dump_fops },
> +		{ "guc_log_level", &guc_log_level_fops },
> +		{ "guc_log_relay", &guc_log_relay_fops },
> +	};
> +
> +	if (!intel_guc_is_supported(log_to_guc(log)))
> +		return;
> +
> +	debugfs_uc_register_files(files, root, log);
> +}
> +
> diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h b/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h
> new file mode 100644
> index 000000000000..bc3ce784667e
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/debugfs_guc_log.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#ifndef DEBUGFS_GUC_LOG_H
> +#define DEBUGFS_GUC_LOG_H
> +
> +struct intel_guc_log;
> +struct dentry;
> +
> +void debugfs_guc_log_register(struct intel_guc_log *log, struct dentry *root);
> +
> +#endif /* DEBUGFS_GUC_LOG_H */
> diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_huc.c b/drivers/gpu/drm/i915/gt/uc/debugfs_huc.c
> new file mode 100644
> index 000000000000..b58740872333
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/debugfs_huc.c
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: MIT
> +
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#include <drm/drm_print.h>
> +
> +#include "debugfs_uc.h"
> +#include "intel_huc.h"
> +
> +static int huc_info_show(struct seq_file *m, void *data)
> +{
> +	struct intel_huc *huc = m->private;
> +	struct drm_printer p = drm_seq_file_printer(m);
> +
> +	if (!intel_huc_is_supported(huc))
> +		return -ENODEV;
> +
> +	intel_huc_load_status(huc, &p);
> +
> +	return 0;
> +}
> +DEFINE_UC_DEBUGFS_ATTRIBUTE(huc_info);
> +
> +void debugfs_huc_register(struct intel_huc *huc, struct dentry *root)
> +{
> +	static const struct debugfs_uc_file files[] = {
> +		{ "huc_info", &huc_info_fops },
> +	};
> +
> +	if (!intel_huc_is_supported(huc))
> +		return;
> +
> +	debugfs_uc_register_files(files, root, huc);
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_huc.h b/drivers/gpu/drm/i915/gt/uc/debugfs_huc.h
> new file mode 100644
> index 000000000000..761f3ee3ed6a
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/debugfs_huc.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#ifndef DEBUGFS_HUC_H
> +#define DEBUGFS_HUC_H
> +
> +struct intel_huc;
> +struct dentry;
> +
> +void debugfs_huc_register(struct intel_huc *huc, struct dentry *root);
> +
> +#endif /* DEBUGFS_HUC_H */
> diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_uc.c b/drivers/gpu/drm/i915/gt/uc/debugfs_uc.c
> new file mode 100644
> index 000000000000..e925443183be
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/debugfs_uc.c
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: MIT
> +
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#include <linux/debugfs.h>
> +
> +#include "debugfs_guc.h"
> +#include "debugfs_huc.h"
> +#include "debugfs_uc.h"
> +#include "intel_uc.h"
> +
> +void debugfs_uc_register(struct intel_uc *uc, struct dentry *gt_root)
> +{
> +	struct dentry *root;
> +
> +	if (!gt_root)
> +		return;
> +
> +	/* GuC and HuC go always in pair, no need to check both */
> +	if (!intel_uc_supports_guc(uc))
> +		return;
> +
> +	root = debugfs_create_dir("uc", gt_root);
> +	if (IS_ERR(root))
> +		return;
> +
> +	debugfs_guc_register(&uc->guc, root);
> +	debugfs_huc_register(&uc->huc, root);
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/debugfs_uc.h b/drivers/gpu/drm/i915/gt/uc/debugfs_uc.h
> new file mode 100644
> index 000000000000..9aa4b7e52770
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/debugfs_uc.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#ifndef DEBUGFS_UC_H
> +#define DEBUGFS_UC_H
> +
> +#include <linux/file.h>
> +
> +struct intel_uc;
> +
> +#define DEFINE_UC_DEBUGFS_ATTRIBUTE(__name)				\
> +	static int __name ## _open(struct inode *inode, struct file *file) \
> +{									\
> +	return single_open(file, __name ## _show, inode->i_private);	\
> +}									\
> +static const struct file_operations __name ## _fops = {			\
> +	.owner = THIS_MODULE,						\
> +	.open = __name ## _open,					\
> +	.read = seq_read,						\
> +	.llseek = seq_lseek,						\
> +	.release = single_release,					\
> +}
> +
> +void debugfs_uc_register(struct intel_uc *uc, struct dentry *gt_root);
> +
> +struct debugfs_uc_file {
> +	const char *name;
> +	const struct file_operations *fops;
> +};
> +
> +#define debugfs_uc_register_files(files__, root__, data__) \
> +do { \
> +	int i__ = 0; \
> +	for (i__ = 0; i__ < ARRAY_SIZE(files__); i__++) { \
> +		debugfs_create_file(files__[i__].name, \
> +				    0444, root__, data__, \
> +				    files__[i__].fops); \
> +	} \
> +} while (0)
> +
> +#endif /* DEBUGFS_UC_H */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index 7d1ae9879b94..2b79a8bf8d27 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -74,6 +74,11 @@ struct intel_guc {
>  	struct mutex send_mutex;
>  };
>  
> +static inline struct intel_guc *log_to_guc(const struct intel_guc_log *log)
> +{
> +	return container_of(log, struct intel_guc, log);
> +}
> +
>  static
>  inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
>  {
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> index fbb73b8d9178..cfe2f9321680 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> @@ -55,11 +55,6 @@ static int guc_action_control_log(struct intel_guc *guc, bool enable,
>  	return intel_guc_send(guc, action, ARRAY_SIZE(action));
>  }
>  
> -static inline struct intel_guc *log_to_guc(const struct intel_guc_log *log)
> -{
> -	return container_of(log, struct intel_guc, log);
> -}
> -
>  static void guc_log_enable_flush_events(struct intel_guc_log *log)
>  {
>  	intel_guc_enable_msg(log_to_guc(log),
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1bec4cdeb92f..a8b6f0fd9439 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -37,7 +37,6 @@
>  #include "gt/intel_reset.h"
>  #include "gt/intel_rc6.h"
>  #include "gt/intel_rps.h"
> -#include "gt/uc/intel_guc_submission.h"
>  
>  #include "i915_debugfs.h"
>  #include "i915_debugfs_params.h"
> @@ -1465,136 +1464,6 @@ static int i915_llc(struct seq_file *m, void *data)
>  	return 0;
>  }
>  
> -static int i915_huc_info(struct seq_file *m, void *data)
> -{
> -	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> -	struct intel_huc *huc = &dev_priv->gt.uc.huc;
> -	struct drm_printer p = drm_seq_file_printer(m);
> -
> -	if (!intel_huc_is_supported(huc))
> -		return -ENODEV;
> -
> -	intel_huc_load_status(huc, &p);
> -
> -	return 0;
> -}
> -
> -static int i915_guc_info(struct seq_file *m, void *data)
> -{
> -	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> -	struct intel_guc *guc = &dev_priv->gt.uc.guc;
> -	struct drm_printer p = drm_seq_file_printer(m);
> -
> -	if (!intel_guc_is_supported(guc))
> -		return -ENODEV;
> -
> -	intel_guc_load_status(guc, &p);
> -	drm_puts(&p, "\n");
> -	intel_guc_log_info(&guc->log, &p);
> -
> -	/* Add more as required ... */
> -
> -	return 0;
> -}
> -
> -static int i915_guc_log_dump(struct seq_file *m, void *data)
> -{
> -	struct drm_info_node *node = m->private;
> -	struct drm_i915_private *dev_priv = node_to_i915(node);
> -	struct intel_guc *guc = &dev_priv->gt.uc.guc;
> -	bool dump_load_err = !!node->info_ent->data;
> -	struct drm_printer p = drm_seq_file_printer(m);
> -
> -	if (!intel_guc_is_supported(guc))
> -		return -ENODEV;
> -
> -	return intel_guc_log_dump(&guc->log, &p, dump_load_err);
> -}
> -
> -static int i915_guc_log_level_get(void *data, u64 *val)
> -{
> -	struct drm_i915_private *dev_priv = data;
> -	struct intel_uc *uc = &dev_priv->gt.uc;
> -
> -	if (!intel_uc_uses_guc(uc))
> -		return -ENODEV;
> -
> -	*val = intel_guc_log_get_level(&uc->guc.log);
> -
> -	return 0;
> -}
> -
> -static int i915_guc_log_level_set(void *data, u64 val)
> -{
> -	struct drm_i915_private *dev_priv = data;
> -	struct intel_uc *uc = &dev_priv->gt.uc;
> -
> -	if (!intel_uc_uses_guc(uc))
> -		return -ENODEV;
> -
> -	return intel_guc_log_set_level(&uc->guc.log, val);
> -}
> -
> -DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_level_fops,
> -			i915_guc_log_level_get, i915_guc_log_level_set,
> -			"%lld\n");
> -
> -static int i915_guc_log_relay_open(struct inode *inode, struct file *file)
> -{
> -	struct drm_i915_private *i915 = inode->i_private;
> -	struct intel_guc *guc = &i915->gt.uc.guc;
> -	struct intel_guc_log *log = &guc->log;
> -
> -	if (!intel_guc_is_ready(guc))
> -		return -ENODEV;
> -
> -	file->private_data = log;
> -
> -	return intel_guc_log_relay_open(log);
> -}
> -
> -static ssize_t
> -i915_guc_log_relay_write(struct file *filp,
> -			 const char __user *ubuf,
> -			 size_t cnt,
> -			 loff_t *ppos)
> -{
> -	struct intel_guc_log *log = filp->private_data;
> -	int val;
> -	int ret;
> -
> -	ret = kstrtoint_from_user(ubuf, cnt, 0, &val);
> -	if (ret < 0)
> -		return ret;
> -
> -	/*
> -	 * Enable and start the guc log relay on value of 1.
> -	 * Flush log relay for any other value.
> -	 */
> -	if (val == 1)
> -		ret = intel_guc_log_relay_start(log);
> -	else
> -		intel_guc_log_relay_flush(log);
> -
> -	return ret ?: cnt;
> -}
> -
> -static int i915_guc_log_relay_release(struct inode *inode, struct file *file)
> -{
> -	struct drm_i915_private *i915 = inode->i_private;
> -	struct intel_guc *guc = &i915->gt.uc.guc;
> -
> -	intel_guc_log_relay_close(&guc->log);
> -	return 0;
> -}
> -
> -static const struct file_operations i915_guc_log_relay_fops = {
> -	.owner = THIS_MODULE,
> -	.open = i915_guc_log_relay_open,
> -	.write = i915_guc_log_relay_write,
> -	.release = i915_guc_log_relay_release,
> -};
> -
>  static int i915_runtime_pm_status(struct seq_file *m, void *unused)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> @@ -2203,10 +2072,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
>  	{"i915_gem_objects", i915_gem_object_info, 0},
>  	{"i915_gem_fence_regs", i915_gem_fence_regs_info, 0},
>  	{"i915_gem_interrupt", i915_interrupt_info, 0},
> -	{"i915_guc_info", i915_guc_info, 0},
> -	{"i915_guc_log_dump", i915_guc_log_dump, 0},
> -	{"i915_guc_load_err_log_dump", i915_guc_log_dump, 0, (void *)1},
> -	{"i915_huc_info", i915_huc_info, 0},
>  	{"i915_frequency_info", i915_frequency_info, 0},
>  	{"i915_drpc_info", i915_drpc_info, 0},
>  	{"i915_ring_freq_table", i915_ring_freq_table, 0},
> @@ -2236,8 +2101,6 @@ static const struct i915_debugfs_files {
>  	{"i915_error_state", &i915_error_state_fops},
>  	{"i915_gpu_info", &i915_gpu_info_fops},
>  #endif
> -	{"i915_guc_log_level", &i915_guc_log_level_fops},
> -	{"i915_guc_log_relay", &i915_guc_log_relay_fops},
>  };
>  
>  int i915_debugfs_register(struct drm_i915_private *dev_priv)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 2/6] drm/i915/uc: mark structure passed to checker functions as const
  2020-02-28  9:18   ` Jani Nikula
@ 2020-02-29  0:20     ` Daniele Ceraolo Spurio
  2020-03-04  9:56       ` Jani Nikula
  0 siblings, 1 reply; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-02-29  0:20 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx



On 2/28/20 1:18 AM, Jani Nikula wrote:
> On Thu, 27 Feb 2020, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
>> Follow-up patches will pass const objects from debugfs to some those
>> functions, so we need to be ready.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_gt.h             |  6 +++---
>>   drivers/gpu/drm/i915/gt/uc/intel_guc.h         | 10 +++++-----
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h      |  2 +-
>>   .../gpu/drm/i915/gt/uc/intel_guc_submission.h  |  6 +++---
>>   drivers/gpu/drm/i915/gt/uc/intel_huc.h         |  8 ++++----
>>   drivers/gpu/drm/i915/gt/uc/intel_uc.h          |  2 +-
>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h       | 18 +++++++++---------
>>   7 files changed, 26 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
>> index 4fac043750aa..f9fbe645478d 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
>> @@ -18,17 +18,17 @@ struct drm_i915_private;
>>   		  ##__VA_ARGS__);					\
>>   } while (0)
>>   
>> -static inline struct intel_gt *uc_to_gt(struct intel_uc *uc)
>> +static inline struct intel_gt *uc_to_gt(const struct intel_uc *uc)
>>   {
>>   	return container_of(uc, struct intel_gt, uc);
>>   }
>>   
>> -static inline struct intel_gt *guc_to_gt(struct intel_guc *guc)
>> +static inline struct intel_gt *guc_to_gt(const struct intel_guc *guc)
>>   {
>>   	return container_of(guc, struct intel_gt, uc.guc);
>>   }
>>   
>> -static inline struct intel_gt *huc_to_gt(struct intel_huc *huc)
>> +static inline struct intel_gt *huc_to_gt(const struct intel_huc *huc)
>>   {
>>   	return container_of(huc, struct intel_gt, uc.huc);
>>   }
> 
> Not fond of the fact that these cast the const away. If you can return
> const also, fine, but const in, non-const out is not fine.
> 

fair point. We usually use those functions for non-const->non-const 
conversions, but in debugfs the objects are marked as const hence why 
the need to add it here (the output in that case can also be marked as 
const).

What's the favorite alternative, add a guc_to_gt_const() variant, do a 
straight container_of in the debugfs function or simply avoid marking 
the objects as const to begin with, even if they're treated as such?

Thanks,
Daniele

> BR,
> Jani.
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for Re-org uC debugfs files and move them under GT
  2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
                   ` (8 preceding siblings ...)
  2020-02-28  6:20 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-02-29 16:42 ` Patchwork
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2020-02-29 16:42 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

== Series Details ==

Series: Re-org uC debugfs files and move them under GT
URL   : https://patchwork.freedesktop.org/series/74051/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8024_full -> Patchwork_16755_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_16755_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@implicit-read-write-bsd1:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#109276] / [i915#677]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb2/igt@gem_exec_schedule@implicit-read-write-bsd1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb3/igt@gem_exec_schedule@implicit-read-write-bsd1.html

  * igt@gem_exec_schedule@pi-common-bsd1:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276]) +17 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb1/igt@gem_exec_schedule@pi-common-bsd1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb7/igt@gem_exec_schedule@pi-common-bsd1.html

  * igt@gem_exec_schedule@pi-userfault-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([i915#677]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb6/igt@gem_exec_schedule@pi-userfault-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb2/igt@gem_exec_schedule@pi-userfault-bsd.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#112146]) +4 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb6/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb4/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_exec_suspend@basic-s3:
    - shard-kbl:          [PASS][9] -> [DMESG-WARN][10] ([i915#180]) +3 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-kbl2/igt@gem_exec_suspend@basic-s3.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-kbl7/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([i915#644])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb5/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb7/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@i915_pm_rps@waitboost:
    - shard-tglb:         [PASS][13] -> [FAIL][14] ([i915#413])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-tglb8/igt@i915_pm_rps@waitboost.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-tglb5/igt@i915_pm_rps@waitboost.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x256-random:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([i915#54])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl9/igt@kms_cursor_crc@pipe-a-cursor-256x256-random.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl8/igt@kms_cursor_crc@pipe-a-cursor-256x256-random.html

  * igt@kms_flip@flip-vs-absolute-wf_vblank:
    - shard-tglb:         [PASS][17] -> [FAIL][18] ([i915#488])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-tglb2/igt@kms_flip@flip-vs-absolute-wf_vblank.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-tglb8/igt@kms_flip@flip-vs-absolute-wf_vblank.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([i915#79])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([i915#49]) +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl9/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-gtt.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl8/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-gtt.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([i915#1188])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl2/igt@kms_hdr@bpc-switch-dpms.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl6/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane@plane-panning-bottom-right-pipe-c-planes:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([i915#1036])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl2/igt@kms_plane@plane-panning-bottom-right-pipe-c-planes.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl9/igt@kms_plane@plane-panning-bottom-right-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [PASS][27] -> [FAIL][28] ([fdo#108145])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][29] -> [FAIL][30] ([fdo#108145] / [i915#265])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-skl:          [PASS][31] -> [DMESG-WARN][32] ([IGT#6])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl6/igt@kms_plane_multiple@atomic-pipe-b-tiling-yf.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl1/igt@kms_plane_multiple@atomic-pipe-b-tiling-yf.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [PASS][33] -> [SKIP][34] ([fdo#109441]) +3 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb4/igt@kms_psr@psr2_cursor_render.html

  * igt@perf_pmu@busy-accuracy-98-vcs1:
    - shard-iclb:         [PASS][35] -> [SKIP][36] ([fdo#112080]) +3 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb2/igt@perf_pmu@busy-accuracy-98-vcs1.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb6/igt@perf_pmu@busy-accuracy-98-vcs1.html

  
#### Possible fixes ####

  * {igt@gem_ctx_ringsize@active@bcs0}:
    - shard-apl:          [INCOMPLETE][37] ([fdo#103927]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-apl2/igt@gem_ctx_ringsize@active@bcs0.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-apl4/igt@gem_ctx_ringsize@active@bcs0.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][39] ([fdo#110841]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb3/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_async@concurrent-writes-bsd:
    - shard-iclb:         [SKIP][41] ([fdo#112146]) -> [PASS][42] +3 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb4/igt@gem_exec_async@concurrent-writes-bsd.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb3/igt@gem_exec_async@concurrent-writes-bsd.html

  * igt@gem_exec_schedule@implicit-write-read-bsd1:
    - shard-iclb:         [SKIP][43] ([fdo#109276] / [i915#677]) -> [PASS][44] +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb6/igt@gem_exec_schedule@implicit-write-read-bsd1.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb1/igt@gem_exec_schedule@implicit-write-read-bsd1.html

  * igt@gem_exec_schedule@out-order-bsd2:
    - shard-iclb:         [SKIP][45] ([fdo#109276]) -> [PASS][46] +8 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb6/igt@gem_exec_schedule@out-order-bsd2.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb1/igt@gem_exec_schedule@out-order-bsd2.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-kbl:          [FAIL][47] ([i915#644]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-kbl6/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-kbl7/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-skl:          [DMESG-FAIL][49] ([fdo#112406]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl10/igt@i915_selftest@live@gt_heartbeat.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl5/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-apl:          [DMESG-WARN][51] ([i915#180]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-apl4/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-apl2/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][53] ([i915#96]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-hsw4/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-hsw1/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][55] ([i915#72]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-glk2/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-glk4/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_cursor_legacy@all-pipes-forked-bo:
    - shard-iclb:         [INCOMPLETE][57] -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb7/igt@kms_cursor_legacy@all-pipes-forked-bo.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb5/igt@kms_cursor_legacy@all-pipes-forked-bo.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-render:
    - shard-tglb:         [SKIP][59] ([i915#668]) -> [PASS][60] +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-tglb2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-render.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-tglb8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-render.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [FAIL][61] ([i915#1188]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl8/igt@kms_hdr@bpc-switch-suspend.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl10/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][63] ([fdo#108145]) -> [PASS][64] +1 similar issue
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-glk:          [FAIL][65] ([i915#899]) -> [PASS][66] +1 similar issue
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-glk9/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-glk6/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_sprite_blt:
    - shard-iclb:         [SKIP][67] ([fdo#109441]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb4/igt@kms_psr@psr2_sprite_blt.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][69] ([i915#31]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-apl3/igt@kms_setmode@basic.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-apl4/igt@kms_setmode@basic.html
    - shard-skl:          [FAIL][71] ([i915#31]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-skl9/igt@kms_setmode@basic.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-skl2/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [DMESG-WARN][73] ([i915#180]) -> [PASS][74] +4 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-kbl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@perf_pmu@busy-start-vcs1:
    - shard-iclb:         [SKIP][75] ([fdo#112080]) -> [PASS][76] +12 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb6/igt@perf_pmu@busy-start-vcs1.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb1/igt@perf_pmu@busy-start-vcs1.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc5-psr:
    - shard-snb:          [INCOMPLETE][77] ([i915#82]) -> [SKIP][78] ([fdo#109271])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-snb5/igt@i915_pm_dc@dc5-psr.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-snb5/igt@i915_pm_dc@dc5-psr.html

  * igt@i915_pm_dc@dc6-dpms:
    - shard-tglb:         [SKIP][79] ([i915#468]) -> [FAIL][80] ([i915#454])
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-tglb2/igt@i915_pm_dc@dc6-dpms.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-tglb1/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-tglb:         [FAIL][81] ([i915#454]) -> [SKIP][82] ([i915#468])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-tglb7/igt@i915_pm_dc@dc6-psr.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-tglb2/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rpm@legacy-planes-dpms:
    - shard-iclb:         [TIMEOUT][83] ([i915#1281]) -> [INCOMPLETE][84] ([fdo#109960] / [i915#189])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-iclb5/igt@i915_pm_rpm@legacy-planes-dpms.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-iclb2/igt@i915_pm_rpm@legacy-planes-dpms.html

  * igt@i915_selftest@live@gt_lrc:
    - shard-tglb:         [INCOMPLETE][85] ([i915#1233]) -> [DMESG-FAIL][86] ([i915#1233])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-tglb2/igt@i915_selftest@live@gt_lrc.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-tglb6/igt@i915_selftest@live@gt_lrc.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-apl:          [DMESG-FAIL][87] -> [DMESG-WARN][88] ([i915#1297])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8024/shard-apl4/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/shard-apl2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [IGT#6]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/6
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109960]: https://bugs.freedesktop.org/show_bug.cgi?id=109960
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [fdo#112406]: https://bugs.freedesktop.org/show_bug.cgi?id=112406
  [i915#1036]: https://gitlab.freedesktop.org/drm/intel/issues/1036
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1233]: https://gitlab.freedesktop.org/drm/intel/issues/1233
  [i915#1281]: https://gitlab.freedesktop.org/drm/intel/issues/1281
  [i915#1297]: https://gitlab.freedesktop.org/drm/intel/issues/1297
  [i915#1333]: https://gitlab.freedesktop.org/drm/intel/issues/1333
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#189]: https://gitlab.freedesktop.org/drm/intel/issues/189
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#413]: https://gitlab.freedesktop.org/drm/intel/issues/413
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#488]: https://gitlab.freedesktop.org/drm/intel/issues/488
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#668]: https://gitlab.freedesktop.org/drm/intel/issues/668
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#899]: https://gitlab.freedesktop.org/drm/intel/issues/899
  [i915#96]: https://gitlab.freedesktop.org/drm/intel/issues/96


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8024 -> Patchwork_16755

  CI-20190529: 20190529
  CI_DRM_8024: 3290680f9735978238a1d3df1efa83326a843327 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5474: 1be610f852de155cd915e7cda65cb2737adf04d4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16755: 2919c449e528a523e408e4c810f274d023dae2de @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16755/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT
  2020-02-28  2:28 ` [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT Daniele Ceraolo Spurio
  2020-02-28  9:24   ` Jani Nikula
@ 2020-03-03  1:52   ` Andi Shyti
  2020-03-03 22:13     ` Daniele Ceraolo Spurio
  1 sibling, 1 reply; 20+ messages in thread
From: Andi Shyti @ 2020-03-03  1:52 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

Hi Daniele,

I'm sorry I missed this patch,

On Thu, Feb 27, 2020 at 06:28:42PM -0800, Daniele Ceraolo Spurio wrote:
> uC is a component of the GT, so it makes sense for the uC debugfs files
> to be in the GT folder. A subfolder has been used to keep the same
> structure we have for the code.

Can we please document the interface changes. I see there are
some differences between the original and the new interfaces.

> +#define DEFINE_UC_DEBUGFS_ATTRIBUTE(__name)				\
> +	static int __name ## _open(struct inode *inode, struct file *file) \
> +{									\
> +	return single_open(file, __name ## _show, inode->i_private);	\
> +}									\
> +static const struct file_operations __name ## _fops = {			\
> +	.owner = THIS_MODULE,						\
> +	.open = __name ## _open,					\
> +	.read = seq_read,						\
> +	.llseek = seq_lseek,						\
> +	.release = single_release,					\
> +}

Why do we need DEFINE_UC_DEBUGFS_ATTRIBUTE()?

DEFINE_GT_DEBUGFS_ATTRIBUTE() was meant to be common to all gt
debugfs. I there any reason we need a new one?

> +struct debugfs_uc_file {
> +	const char *name;
> +	const struct file_operations *fops;
> +};
> +
> +#define debugfs_uc_register_files(files__, root__, data__) \
> +do { \
> +	int i__ = 0; \
> +	for (i__ = 0; i__ < ARRAY_SIZE(files__); i__++) { \
> +		debugfs_create_file(files__[i__].name, \
> +				    0444, root__, data__, \
> +				    files__[i__].fops); \
> +	} \
> +} while (0)

You want to define your own debugfs_uc_register_files() instead
of using debugfs_gt_register_files() because you want "data__"
to be void, right?

I think we can achieve that by adding a wrapper in debugfs_gt.c,
perhaps we can do something like:

void __debugfs_gt_register_files(struct intel_gt *gt,
                                 struct dentry *root,
                                 const struct debugfs_gt_file *files,
                                 void *data,
                                 unsigned long count)
{
      ......
}

and 

#define debugfs_gt_register_files(...) __debugfs_gt_register_files(...)
#define debugfs_uc_register_files(...) __debugfs_gt_register_files(...)

so that we can keep everything in a library. What do you think.

Thanks,
Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT
  2020-03-03  1:52   ` Andi Shyti
@ 2020-03-03 22:13     ` Daniele Ceraolo Spurio
  2020-03-05 18:02       ` Andi Shyti
  0 siblings, 1 reply; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-03-03 22:13 UTC (permalink / raw)
  To: Andi Shyti; +Cc: intel-gfx



On 3/2/20 5:52 PM, Andi Shyti wrote:
> Hi Daniele,
> 
> I'm sorry I missed this patch,
> 
> On Thu, Feb 27, 2020 at 06:28:42PM -0800, Daniele Ceraolo Spurio wrote:
>> uC is a component of the GT, so it makes sense for the uC debugfs files
>> to be in the GT folder. A subfolder has been used to keep the same
>> structure we have for the code.
> 
> Can we please document the interface changes. I see there are
> some differences between the original and the new interfaces.
> 

What differences are you referring to? there aren't supposed to be any, 
aside from the path change.

>> +#define DEFINE_UC_DEBUGFS_ATTRIBUTE(__name)				\
>> +	static int __name ## _open(struct inode *inode, struct file *file) \
>> +{									\
>> +	return single_open(file, __name ## _show, inode->i_private);	\
>> +}									\
>> +static const struct file_operations __name ## _fops = {			\
>> +	.owner = THIS_MODULE,						\
>> +	.open = __name ## _open,					\
>> +	.read = seq_read,						\
>> +	.llseek = seq_lseek,						\
>> +	.release = single_release,					\
>> +}
> 
> Why do we need DEFINE_UC_DEBUGFS_ATTRIBUTE()?
> 
> DEFINE_GT_DEBUGFS_ATTRIBUTE() was meant to be common to all gt
> debugfs. I there any reason we need a new one?
> 

Just wanted to avoid including the other header just for this macro.

>> +struct debugfs_uc_file {
>> +	const char *name;
>> +	const struct file_operations *fops;
>> +};
>> +
>> +#define debugfs_uc_register_files(files__, root__, data__) \
>> +do { \
>> +	int i__ = 0; \
>> +	for (i__ = 0; i__ < ARRAY_SIZE(files__); i__++) { \
>> +		debugfs_create_file(files__[i__].name, \
>> +				    0444, root__, data__, \
>> +				    files__[i__].fops); \
>> +	} \
>> +} while (0)
> 
> You want to define your own debugfs_uc_register_files() instead
> of using debugfs_gt_register_files() because you want "data__"
> to be void, right?
> 
> I think we can achieve that by adding a wrapper in debugfs_gt.c,
> perhaps we can do something like:
> 
> void __debugfs_gt_register_files(struct intel_gt *gt,
>                                   struct dentry *root,
>                                   const struct debugfs_gt_file *files,
>                                   void *data,
>                                   unsigned long count)
> {
>        ......
> }
> 
> and
> 
> #define debugfs_gt_register_files(...) __debugfs_gt_register_files(...)
> #define debugfs_uc_register_files(...) __debugfs_gt_register_files(...)
> 
> so that we can keep everything in a library. What do you think.
> 

LGTM. Mind if I rename to:

intel_gt_debugfs_register(...)
intel_uc_debugfs_register(...)

to avoid the debugfs_* prefix, as pointed out by Jani?

Thanks,
Daniele

> Thanks,
> Andi
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 2/6] drm/i915/uc: mark structure passed to checker functions as const
  2020-02-29  0:20     ` Daniele Ceraolo Spurio
@ 2020-03-04  9:56       ` Jani Nikula
  0 siblings, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2020-03-04  9:56 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio, intel-gfx

On Fri, 28 Feb 2020, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
> On 2/28/20 1:18 AM, Jani Nikula wrote:
>> On Thu, 27 Feb 2020, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
>>> Follow-up patches will pass const objects from debugfs to some those
>>> functions, so we need to be ready.
>>>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>> Cc: Matthew Brost <matthew.brost@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/gt/intel_gt.h             |  6 +++---
>>>   drivers/gpu/drm/i915/gt/uc/intel_guc.h         | 10 +++++-----
>>>   drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h      |  2 +-
>>>   .../gpu/drm/i915/gt/uc/intel_guc_submission.h  |  6 +++---
>>>   drivers/gpu/drm/i915/gt/uc/intel_huc.h         |  8 ++++----
>>>   drivers/gpu/drm/i915/gt/uc/intel_uc.h          |  2 +-
>>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h       | 18 +++++++++---------
>>>   7 files changed, 26 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
>>> index 4fac043750aa..f9fbe645478d 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
>>> @@ -18,17 +18,17 @@ struct drm_i915_private;
>>>   		  ##__VA_ARGS__);					\
>>>   } while (0)
>>>   
>>> -static inline struct intel_gt *uc_to_gt(struct intel_uc *uc)
>>> +static inline struct intel_gt *uc_to_gt(const struct intel_uc *uc)
>>>   {
>>>   	return container_of(uc, struct intel_gt, uc);
>>>   }
>>>   
>>> -static inline struct intel_gt *guc_to_gt(struct intel_guc *guc)
>>> +static inline struct intel_gt *guc_to_gt(const struct intel_guc *guc)
>>>   {
>>>   	return container_of(guc, struct intel_gt, uc.guc);
>>>   }
>>>   
>>> -static inline struct intel_gt *huc_to_gt(struct intel_huc *huc)
>>> +static inline struct intel_gt *huc_to_gt(const struct intel_huc *huc)
>>>   {
>>>   	return container_of(huc, struct intel_gt, uc.huc);
>>>   }
>> 
>> Not fond of the fact that these cast the const away. If you can return
>> const also, fine, but const in, non-const out is not fine.
>> 
>
> fair point. We usually use those functions for non-const->non-const 
> conversions, but in debugfs the objects are marked as const hence why 
> the need to add it here (the output in that case can also be marked as 
> const).
>
> What's the favorite alternative, add a guc_to_gt_const() variant, do a 
> straight container_of in the debugfs function or simply avoid marking 
> the objects as const to begin with, even if they're treated as such?

In a later patch, this seems wrong to me:

 +void intel_guc_load_status(const struct intel_guc *guc, struct drm_printer *p)
 +{
 +	struct intel_gt *gt = guc_to_gt(guc);

You get const * in, and convert it to non-const. The container_of you
suggest would do the same I believe.

How many _const() conversion macros would you get? If a lot, I'd not
bother with it.

BR,
Jani.


>
> Thanks,
> Daniele
>
>> BR,
>> Jani.
>> 
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT
  2020-03-03 22:13     ` Daniele Ceraolo Spurio
@ 2020-03-05 18:02       ` Andi Shyti
  2020-03-05 23:10         ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 20+ messages in thread
From: Andi Shyti @ 2020-03-05 18:02 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: intel-gfx

Hi Daniele,

> > On Thu, Feb 27, 2020 at 06:28:42PM -0800, Daniele Ceraolo Spurio wrote:
> > > uC is a component of the GT, so it makes sense for the uC debugfs files
> > > to be in the GT folder. A subfolder has been used to keep the same
> > > structure we have for the code.
> > 
> > Can we please document the interface changes. I see there are
> > some differences between the original and the new interfaces.
> > 
> 
> What differences are you referring to? there aren't supposed to be any,
> aside from the path change.

Have I seen it wrong or there are new files in this patch?
In any case, maybe we need to have the new structure.

> > > +#define DEFINE_UC_DEBUGFS_ATTRIBUTE(__name)				\
> > > +	static int __name ## _open(struct inode *inode, struct file *file) \
> > > +{									\
> > > +	return single_open(file, __name ## _show, inode->i_private);	\
> > > +}									\
> > > +static const struct file_operations __name ## _fops = {			\
> > > +	.owner = THIS_MODULE,						\
> > > +	.open = __name ## _open,					\
> > > +	.read = seq_read,						\
> > > +	.llseek = seq_lseek,						\
> > > +	.release = single_release,					\
> > > +}
> > 
> > Why do we need DEFINE_UC_DEBUGFS_ATTRIBUTE()?
> > 
> > DEFINE_GT_DEBUGFS_ATTRIBUTE() was meant to be common to all gt
> > debugfs. I there any reason we need a new one?
> > 
> 
> Just wanted to avoid including the other header just for this macro.

well that was supposed to be a library for all the gem/debugfs
files and avoid duplicated code, I don't see anything wrong with
including the file.

> > > +struct debugfs_uc_file {
> > > +	const char *name;
> > > +	const struct file_operations *fops;
> > > +};
> > > +
> > > +#define debugfs_uc_register_files(files__, root__, data__) \
> > > +do { \
> > > +	int i__ = 0; \
> > > +	for (i__ = 0; i__ < ARRAY_SIZE(files__); i__++) { \
> > > +		debugfs_create_file(files__[i__].name, \
> > > +				    0444, root__, data__, \
> > > +				    files__[i__].fops); \
> > > +	} \
> > > +} while (0)
> > 
> > You want to define your own debugfs_uc_register_files() instead
> > of using debugfs_gt_register_files() because you want "data__"
> > to be void, right?
> > 
> > I think we can achieve that by adding a wrapper in debugfs_gt.c,
> > perhaps we can do something like:
> > 
> > void __debugfs_gt_register_files(struct intel_gt *gt,
> >                                   struct dentry *root,
> >                                   const struct debugfs_gt_file *files,
> >                                   void *data,
> >                                   unsigned long count)
> > {
> >        ......
> > }
> > 
> > and
> > 
> > #define debugfs_gt_register_files(...) __debugfs_gt_register_files(...)
> > #define debugfs_uc_register_files(...) __debugfs_gt_register_files(...)
> > 
> > so that we can keep everything in a library. What do you think.
> > 
> 
> LGTM. Mind if I rename to:
> 
> intel_gt_debugfs_register(...)
> intel_uc_debugfs_register(...)
> 
> to avoid the debugfs_* prefix, as pointed out by Jani?

I have a patch for it, can you please hold a little, unless, of
course, yours is already ready.

Obvously, the naming you propose makes sense.

Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT
  2020-03-05 18:02       ` Andi Shyti
@ 2020-03-05 23:10         ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-03-05 23:10 UTC (permalink / raw)
  To: Andi Shyti; +Cc: intel-gfx



On 3/5/20 10:02 AM, Andi Shyti wrote:
> Hi Daniele,
> 
>>> On Thu, Feb 27, 2020 at 06:28:42PM -0800, Daniele Ceraolo Spurio wrote:
>>>> uC is a component of the GT, so it makes sense for the uC debugfs files
>>>> to be in the GT folder. A subfolder has been used to keep the same
>>>> structure we have for the code.
>>>
>>> Can we please document the interface changes. I see there are
>>> some differences between the original and the new interfaces.
>>>
>>
>> What differences are you referring to? there aren't supposed to be any,
>> aside from the path change.
> 
> Have I seen it wrong or there are new files in this patch?

No, no new debugfs files, only the old ones moved across.

> In any case, maybe we need to have the new structure.
> 
>>>> +#define DEFINE_UC_DEBUGFS_ATTRIBUTE(__name)				\
>>>> +	static int __name ## _open(struct inode *inode, struct file *file) \
>>>> +{									\
>>>> +	return single_open(file, __name ## _show, inode->i_private);	\
>>>> +}									\
>>>> +static const struct file_operations __name ## _fops = {			\
>>>> +	.owner = THIS_MODULE,						\
>>>> +	.open = __name ## _open,					\
>>>> +	.read = seq_read,						\
>>>> +	.llseek = seq_lseek,						\
>>>> +	.release = single_release,					\
>>>> +}
>>>
>>> Why do we need DEFINE_UC_DEBUGFS_ATTRIBUTE()?
>>>
>>> DEFINE_GT_DEBUGFS_ATTRIBUTE() was meant to be common to all gt
>>> debugfs. I there any reason we need a new one?
>>>
>>
>> Just wanted to avoid including the other header just for this macro.
> 
> well that was supposed to be a library for all the gem/debugfs
> files and avoid duplicated code, I don't see anything wrong with
> including the file.
> 
>>>> +struct debugfs_uc_file {
>>>> +	const char *name;
>>>> +	const struct file_operations *fops;
>>>> +};
>>>> +
>>>> +#define debugfs_uc_register_files(files__, root__, data__) \
>>>> +do { \
>>>> +	int i__ = 0; \
>>>> +	for (i__ = 0; i__ < ARRAY_SIZE(files__); i__++) { \
>>>> +		debugfs_create_file(files__[i__].name, \
>>>> +				    0444, root__, data__, \
>>>> +				    files__[i__].fops); \
>>>> +	} \
>>>> +} while (0)
>>>
>>> You want to define your own debugfs_uc_register_files() instead
>>> of using debugfs_gt_register_files() because you want "data__"
>>> to be void, right?
>>>
>>> I think we can achieve that by adding a wrapper in debugfs_gt.c,
>>> perhaps we can do something like:
>>>
>>> void __debugfs_gt_register_files(struct intel_gt *gt,
>>>                                    struct dentry *root,
>>>                                    const struct debugfs_gt_file *files,
>>>                                    void *data,
>>>                                    unsigned long count)
>>> {
>>>         ......
>>> }
>>>
>>> and
>>>
>>> #define debugfs_gt_register_files(...) __debugfs_gt_register_files(...)
>>> #define debugfs_uc_register_files(...) __debugfs_gt_register_files(...)
>>>
>>> so that we can keep everything in a library. What do you think.
>>>
>>
>> LGTM. Mind if I rename to:
>>
>> intel_gt_debugfs_register(...)
>> intel_uc_debugfs_register(...)
>>
>> to avoid the debugfs_* prefix, as pointed out by Jani?
> 
> I have a patch for it, can you please hold a little, unless, of
> course, yours is already ready.
> 

Sure, I'll wait for your patch to land first.

Daniele

> Obvously, the naming you propose makes sense.
> 
> Andi
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 4/6] drm/i915/debugfs: move uC printers and update debugfs file names
       [not found]   ` <be6dd380-e87b-9ac7-0185-c3c46e40240b@intel.com>
@ 2020-03-11 23:38     ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-03-11 23:38 UTC (permalink / raw)
  To: Ye, Tony, intel-gfx

<snip>

>> +/**
>> + * intel_huc_load_status - dump information about HuC load status
>> + * @huc: the HuC
>> + * @p: the &drm_printer
>> + *
>> + * Pretty printer for HuC load status.
>> + */
>> +void intel_huc_load_status(const struct intel_huc *huc, struct 
>> drm_printer *p)
>> +{
>> +    struct intel_gt *gt = huc_to_gt(huc);
>> +    intel_wakeref_t wakeref;
>> +
>> +    if (!intel_huc_is_supported(huc)) {
>> +        drm_printf(p, "HuC not supported\n");
>> +        return;
>> +    }
>> +
>> +    if (!intel_huc_is_wanted(huc)) {
>> +        drm_printf(p, "HuC disabled\n");
>> +        return;
>> +    }
>> +
>> +    intel_uc_fw_dump(&huc->fw, p);
>> +
>> +    with_intel_runtime_pm(gt->uncore->rpm, wakeref)
>> +        drm_printf(p, "\nHuC status 0x%08x:\n",
>> +               intel_uncore_read(gt->uncore, huc->status.reg));
> 
> HUC_STATUS register is still available on Gen11+. But if the purpose 
> here is to print the status of huc load/auth, then the change makes sense.
> 

The load/auth status is already dumped as part of intel_uc_fw_dump(). I 
though all the state had transferred over to the new register, but since 
I was wrong I'll keep using HUC_STATUS.

Thanks for catching this,
Daniele

> Thanks,
> Tony
> 
>> +}
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
>> index 19651b46d6a4..f1299c0138e5 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
>> @@ -57,4 +57,6 @@ static inline bool intel_huc_is_authenticated(const 
>> struct intel_huc *huc)
>>       return intel_uc_fw_is_running(&huc->fw);
>>   }
>> +void intel_huc_load_status(const struct intel_huc *huc, struct 
>> drm_printer *p);
>> +
>>   #endif
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 37cb8b4bf4dc..1bec4cdeb92f 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1465,105 +1465,32 @@ static int i915_llc(struct seq_file *m, void 
>> *data)
>>       return 0;
>>   }
>> -static int i915_huc_load_status_info(struct seq_file *m, void *data)
>> +static int i915_huc_info(struct seq_file *m, void *data)
>>   {
>>       struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> -    intel_wakeref_t wakeref;
>> -    struct drm_printer p;
>> -
>> -    if (!HAS_GT_UC(dev_priv))
>> -        return -ENODEV;
>> -
>> -    p = drm_seq_file_printer(m);
>> -    intel_uc_fw_dump(&dev_priv->gt.uc.huc.fw, &p);
>> -
>> -    with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref)
>> -        seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2));
>> -
>> -    return 0;
>> -}
>> -
>> -static int i915_guc_load_status_info(struct seq_file *m, void *data)
>> -{
>> -    struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> -    intel_wakeref_t wakeref;
>> -    struct drm_printer p;
>> +    struct intel_huc *huc = &dev_priv->gt.uc.huc;
>> +    struct drm_printer p = drm_seq_file_printer(m);
>> -    if (!HAS_GT_UC(dev_priv))
>> +    if (!intel_huc_is_supported(huc))
>>           return -ENODEV;
>> -    p = drm_seq_file_printer(m);
>> -    intel_uc_fw_dump(&dev_priv->gt.uc.guc.fw, &p);
>> -
>> -    with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) {
>> -        u32 tmp = I915_READ(GUC_STATUS);
>> -        u32 i;
>> -
>> -        seq_printf(m, "\nGuC status 0x%08x:\n", tmp);
>> -        seq_printf(m, "\tBootrom status = 0x%x\n",
>> -               (tmp & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT);
>> -        seq_printf(m, "\tuKernel status = 0x%x\n",
>> -               (tmp & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT);
>> -        seq_printf(m, "\tMIA Core status = 0x%x\n",
>> -               (tmp & GS_MIA_MASK) >> GS_MIA_SHIFT);
>> -        seq_puts(m, "\nScratch registers:\n");
>> -        for (i = 0; i < 16; i++) {
>> -            seq_printf(m, "\t%2d: \t0x%x\n",
>> -                   i, I915_READ(SOFT_SCRATCH(i)));
>> -        }
>> -    }
>> +    intel_huc_load_status(huc, &p);
>>       return 0;
>>   }
>> -static const char *
>> -stringify_guc_log_type(enum guc_log_buffer_type type)
>> -{
>> -    switch (type) {
>> -    case GUC_ISR_LOG_BUFFER:
>> -        return "ISR";
>> -    case GUC_DPC_LOG_BUFFER:
>> -        return "DPC";
>> -    case GUC_CRASH_DUMP_LOG_BUFFER:
>> -        return "CRASH";
>> -    default:
>> -        MISSING_CASE(type);
>> -    }
>> -
>> -    return "";
>> -}
>> -
>> -static void i915_guc_log_info(struct seq_file *m, struct 
>> intel_guc_log *log)
>> -{
>> -    enum guc_log_buffer_type type;
>> -
>> -    if (!intel_guc_log_relay_created(log)) {
>> -        seq_puts(m, "GuC log relay not created\n");
>> -        return;
>> -    }
>> -
>> -    seq_puts(m, "GuC logging stats:\n");
>> -
>> -    seq_printf(m, "\tRelay full count: %u\n",
>> -           log->relay.full_count);
>> -
>> -    for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
>> -        seq_printf(m, "\t%s:\tflush count %10u, overflow count %10u\n",
>> -               stringify_guc_log_type(type),
>> -               log->stats[type].flush,
>> -               log->stats[type].sampled_overflow);
>> -    }
>> -}
>> -
>>   static int i915_guc_info(struct seq_file *m, void *data)
>>   {
>>       struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> -    struct intel_uc *uc = &dev_priv->gt.uc;
>> +    struct intel_guc *guc = &dev_priv->gt.uc.guc;
>> +    struct drm_printer p = drm_seq_file_printer(m);
>> -    if (!intel_uc_uses_guc(uc))
>> +    if (!intel_guc_is_supported(guc))
>>           return -ENODEV;
>> -    i915_guc_log_info(m, &uc->guc.log);
>> +    intel_guc_load_status(guc, &p);
>> +    drm_puts(&p, "\n");
>> +    intel_guc_log_info(&guc->log, &p);
>>       /* Add more as required ... */
>> @@ -1574,39 +1501,14 @@ static int i915_guc_log_dump(struct seq_file 
>> *m, void *data)
>>   {
>>       struct drm_info_node *node = m->private;
>>       struct drm_i915_private *dev_priv = node_to_i915(node);
>> +    struct intel_guc *guc = &dev_priv->gt.uc.guc;
>>       bool dump_load_err = !!node->info_ent->data;
>> -    struct drm_i915_gem_object *obj = NULL;
>> -    u32 *log;
>> -    int i = 0;
>> +    struct drm_printer p = drm_seq_file_printer(m);
>> -    if (!HAS_GT_UC(dev_priv))
>> +    if (!intel_guc_is_supported(guc))
>>           return -ENODEV;
>> -    if (dump_load_err)
>> -        obj = dev_priv->gt.uc.load_err_log;
>> -    else if (dev_priv->gt.uc.guc.log.vma)
>> -        obj = dev_priv->gt.uc.guc.log.vma->obj;
>> -
>> -    if (!obj)
>> -        return 0;
>> -
>> -    log = i915_gem_object_pin_map(obj, I915_MAP_WC);
>> -    if (IS_ERR(log)) {
>> -        DRM_DEBUG("Failed to pin object\n");
>> -        seq_puts(m, "(log data unaccessible)\n");
>> -        return PTR_ERR(log);
>> -    }
>> -
>> -    for (i = 0; i < obj->base.size / sizeof(u32); i += 4)
>> -        seq_printf(m, "0x%08x 0x%08x 0x%08x 0x%08x\n",
>> -               *(log + i), *(log + i + 1),
>> -               *(log + i + 2), *(log + i + 3));
>> -
>> -    seq_putc(m, '\n');
>> -
>> -    i915_gem_object_unpin_map(obj);
>> -
>> -    return 0;
>> +    return intel_guc_log_dump(&guc->log, &p, dump_load_err);
>>   }
>>   static int i915_guc_log_level_get(void *data, u64 *val)
>> @@ -2302,10 +2204,9 @@ static const struct drm_info_list 
>> i915_debugfs_list[] = {
>>       {"i915_gem_fence_regs", i915_gem_fence_regs_info, 0},
>>       {"i915_gem_interrupt", i915_interrupt_info, 0},
>>       {"i915_guc_info", i915_guc_info, 0},
>> -    {"i915_guc_load_status", i915_guc_load_status_info, 0},
>>       {"i915_guc_log_dump", i915_guc_log_dump, 0},
>>       {"i915_guc_load_err_log_dump", i915_guc_log_dump, 0, (void *)1},
>> -    {"i915_huc_load_status", i915_huc_load_status_info, 0},
>> +    {"i915_huc_info", i915_huc_info, 0},
>>       {"i915_frequency_info", i915_frequency_info, 0},
>>       {"i915_drpc_info", i915_drpc_info, 0},
>>       {"i915_ring_freq_table", i915_ring_freq_table, 0},
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-03-11 23:39 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28  2:28 [Intel-gfx] [PATCH 0/6] Re-org uC debugfs files and move them under GT Daniele Ceraolo Spurio
2020-02-28  2:28 ` [Intel-gfx] [PATCH 1/6] drm/i915/guc: drop stage_pool debugfs Daniele Ceraolo Spurio
2020-02-28  2:28 ` [Intel-gfx] [PATCH 2/6] drm/i915/uc: mark structure passed to checker functions as const Daniele Ceraolo Spurio
2020-02-28  9:18   ` Jani Nikula
2020-02-29  0:20     ` Daniele Ceraolo Spurio
2020-03-04  9:56       ` Jani Nikula
2020-02-28  2:28 ` [Intel-gfx] [PATCH 3/6] drm/i915/huc: make "support huc" reflect HW capabilities Daniele Ceraolo Spurio
2020-02-28  2:28 ` [Intel-gfx] [PATCH 4/6] drm/i915/debugfs: move uC printers and update debugfs file names Daniele Ceraolo Spurio
     [not found]   ` <be6dd380-e87b-9ac7-0185-c3c46e40240b@intel.com>
2020-03-11 23:38     ` Daniele Ceraolo Spurio
2020-02-28  2:28 ` [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT Daniele Ceraolo Spurio
2020-02-28  9:24   ` Jani Nikula
2020-03-03  1:52   ` Andi Shyti
2020-03-03 22:13     ` Daniele Ceraolo Spurio
2020-03-05 18:02       ` Andi Shyti
2020-03-05 23:10         ` Daniele Ceraolo Spurio
2020-02-28  2:28 ` [Intel-gfx] [PATCH 6/6] drm/i915/uc: do not free err log on uc_fini Daniele Ceraolo Spurio
2020-02-28  5:54 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Re-org uC debugfs files and move them under GT Patchwork
2020-02-28  5:58 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-02-28  6:20 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-02-29 16:42 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.