dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32.
@ 2019-02-20 21:03 Eric Anholt
  2019-02-20 21:03 ` [PATCH 2/7] drm/vc4: Use drm_print_regset32() for our debug register dumping Eric Anholt
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Eric Anholt @ 2019-02-20 21:03 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-kernel, Paul Kocialkowski, Maxime Ripard, Eric Anholt

The debugfs_regset32 is nice to use for reducing boilerplate in
dumping a bunch of regs in debugfs, but we also want to be able to
print to dmesg them at runtime for driver debugging.  drm_printer lets
us format debugfs and the printk the same way.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/drm_print.c | 16 ++++++++++++++++
 include/drm/drm_print.h     |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 0e7fc3e7dfb4..5ecc0f04cd0c 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -253,3 +253,19 @@ void drm_err(const char *format, ...)
 	va_end(args);
 }
 EXPORT_SYMBOL(drm_err);
+
+void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
+{
+	int namelen = 0;
+	int i;
+
+	for (i = 0; i < regset->nregs; i++)
+		namelen = max(namelen, (int)strlen(regset->regs[i].name));
+
+	for (i = 0; i < regset->nregs; i++) {
+		drm_printf(p, "%*s = 0x%08x\n",
+			   namelen, regset->regs[i].name,
+			   readl(regset->base + regset->regs[i].offset));
+	}
+}
+EXPORT_SYMBOL(drm_print_regset32);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index afbc3beef089..3a4247319e63 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -30,6 +30,7 @@
 #include <linux/printk.h>
 #include <linux/seq_file.h>
 #include <linux/device.h>
+#include <linux/debugfs.h>
 
 /**
  * DOC: print
@@ -84,6 +85,7 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
 __printf(2, 3)
 void drm_printf(struct drm_printer *p, const char *f, ...);
 void drm_puts(struct drm_printer *p, const char *str);
+void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset);
 
 __printf(2, 0)
 /**
-- 
2.20.1

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

* [PATCH 2/7] drm/vc4: Use drm_print_regset32() for our debug register dumping.
  2019-02-20 21:03 [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Eric Anholt
@ 2019-02-20 21:03 ` Eric Anholt
  2019-03-22 10:32   ` Paul Kocialkowski
  2019-02-20 21:03 ` [PATCH 3/7] drm/vc4: Use common helpers for debugfs setup by the driver components Eric Anholt
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Eric Anholt @ 2019-02-20 21:03 UTC (permalink / raw)
  To: dri-devel; +Cc: Paul Kocialkowski, Maxime Ripard, linux-kernel

This removes a bunch of duplicated boilerplate for the debugfs vs
runtime printk debug dumping.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_crtc.c |  68 ++++++-------
 drivers/gpu/drm/vc4/vc4_dpi.c  |  23 ++---
 drivers/gpu/drm/vc4/vc4_drv.h  |   7 ++
 drivers/gpu/drm/vc4/vc4_dsi.c  | 155 +++++++++++++----------------
 drivers/gpu/drm/vc4/vc4_hdmi.c | 156 +++++++++++++----------------
 drivers/gpu/drm/vc4/vc4_hvs.c  |  87 ++++++++---------
 drivers/gpu/drm/vc4/vc4_txp.c  |  28 +++---
 drivers/gpu/drm/vc4/vc4_v3d.c  | 173 ++++++++++++++++-----------------
 drivers/gpu/drm/vc4/vc4_vec.c  |  67 ++++++-------
 9 files changed, 350 insertions(+), 414 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 730008d3da76..6d7072026056 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -35,6 +35,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_atomic_uapi.h>
+#include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 #include <linux/clk.h>
 #include <drm/drm_fb_cma_helper.h>
@@ -67,43 +68,29 @@ to_vc4_crtc_state(struct drm_crtc_state *crtc_state)
 #define CRTC_WRITE(offset, val) writel(val, vc4_crtc->regs + (offset))
 #define CRTC_READ(offset) readl(vc4_crtc->regs + (offset))
 
-#define CRTC_REG(reg) { reg, #reg }
-static const struct {
-	u32 reg;
-	const char *name;
-} crtc_regs[] = {
-	CRTC_REG(PV_CONTROL),
-	CRTC_REG(PV_V_CONTROL),
-	CRTC_REG(PV_VSYNCD_EVEN),
-	CRTC_REG(PV_HORZA),
-	CRTC_REG(PV_HORZB),
-	CRTC_REG(PV_VERTA),
-	CRTC_REG(PV_VERTB),
-	CRTC_REG(PV_VERTA_EVEN),
-	CRTC_REG(PV_VERTB_EVEN),
-	CRTC_REG(PV_INTEN),
-	CRTC_REG(PV_INTSTAT),
-	CRTC_REG(PV_STAT),
-	CRTC_REG(PV_HACT_ACT),
+static const struct debugfs_reg32 crtc_regs[] = {
+	VC4_REG32(PV_CONTROL),
+	VC4_REG32(PV_V_CONTROL),
+	VC4_REG32(PV_VSYNCD_EVEN),
+	VC4_REG32(PV_HORZA),
+	VC4_REG32(PV_HORZB),
+	VC4_REG32(PV_VERTA),
+	VC4_REG32(PV_VERTB),
+	VC4_REG32(PV_VERTA_EVEN),
+	VC4_REG32(PV_VERTB_EVEN),
+	VC4_REG32(PV_INTEN),
+	VC4_REG32(PV_INTSTAT),
+	VC4_REG32(PV_STAT),
+	VC4_REG32(PV_HACT_ACT),
 };
 
-static void vc4_crtc_dump_regs(struct vc4_crtc *vc4_crtc)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(crtc_regs); i++) {
-		DRM_INFO("0x%04x (%s): 0x%08x\n",
-			 crtc_regs[i].reg, crtc_regs[i].name,
-			 CRTC_READ(crtc_regs[i].reg));
-	}
-}
-
 #ifdef CONFIG_DEBUG_FS
 int vc4_crtc_debugfs_regs(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *dev = node->minor->dev;
 	int crtc_index = (uintptr_t)node->info_ent->data;
+	struct drm_printer p = drm_seq_file_printer(m);
 	struct drm_crtc *crtc;
 	struct vc4_crtc *vc4_crtc;
 	int i;
@@ -118,11 +105,7 @@ int vc4_crtc_debugfs_regs(struct seq_file *m, void *unused)
 		return 0;
 	vc4_crtc = to_vc4_crtc(crtc);
 
-	for (i = 0; i < ARRAY_SIZE(crtc_regs); i++) {
-		seq_printf(m, "%s (0x%04x): 0x%08x\n",
-			   crtc_regs[i].name, crtc_regs[i].reg,
-			   CRTC_READ(crtc_regs[i].reg));
-	}
+	drm_print_regset32(&p, &vc4_crtc->regset);
 
 	return 0;
 }
@@ -434,8 +417,10 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	bool debug_dump_regs = false;
 
 	if (debug_dump_regs) {
-		DRM_INFO("CRTC %d regs before:\n", drm_crtc_index(crtc));
-		vc4_crtc_dump_regs(vc4_crtc);
+		struct drm_printer p = drm_info_printer(&vc4_crtc->pdev->dev);
+		dev_info(&vc4_crtc->pdev->dev, "CRTC %d regs before:\n",
+			 drm_crtc_index(crtc));
+		drm_print_regset32(&p, &vc4_crtc->regset);
 	}
 
 	if (vc4_crtc->channel == 2) {
@@ -476,8 +461,10 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	vc4_crtc_lut_load(crtc);
 
 	if (debug_dump_regs) {
-		DRM_INFO("CRTC %d regs after:\n", drm_crtc_index(crtc));
-		vc4_crtc_dump_regs(vc4_crtc);
+		struct drm_printer p = drm_info_printer(&vc4_crtc->pdev->dev);
+		dev_info(&vc4_crtc->pdev->dev, "CRTC %d regs after:\n",
+			 drm_crtc_index(crtc));
+		drm_print_regset32(&p, &vc4_crtc->regset);
 	}
 }
 
@@ -1169,11 +1156,16 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data)
 	if (!match)
 		return -ENODEV;
 	vc4_crtc->data = match->data;
+	vc4_crtc->pdev = pdev;
 
 	vc4_crtc->regs = vc4_ioremap_regs(pdev, 0);
 	if (IS_ERR(vc4_crtc->regs))
 		return PTR_ERR(vc4_crtc->regs);
 
+	vc4_crtc->regset.base = vc4_crtc->regs;
+	vc4_crtc->regset.regs = crtc_regs;
+	vc4_crtc->regset.nregs = ARRAY_SIZE(crtc_regs);
+
 	/* For now, we create just the primary and the legacy cursor
 	 * planes.  We should be able to stack more planes on easily,
 	 * but to do that we would need to compute the bandwidth
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 169521e547ba..8be2264f496a 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -101,6 +101,8 @@ struct vc4_dpi {
 
 	struct clk *pixel_clock;
 	struct clk *core_clock;
+
+	struct debugfs_regset32 regset;
 };
 
 #define DPI_READ(offset) readl(dpi->regs + (offset))
@@ -118,13 +120,9 @@ to_vc4_dpi_encoder(struct drm_encoder *encoder)
 	return container_of(encoder, struct vc4_dpi_encoder, base.base);
 }
 
-#define DPI_REG(reg) { reg, #reg }
-static const struct {
-	u32 reg;
-	const char *name;
-} dpi_regs[] = {
-	DPI_REG(DPI_C),
-	DPI_REG(DPI_ID),
+static const struct debugfs_reg32 dpi_regs[] = {
+	VC4_REG32(DPI_C),
+	VC4_REG32(DPI_ID),
 };
 
 #ifdef CONFIG_DEBUG_FS
@@ -134,16 +132,12 @@ int vc4_dpi_debugfs_regs(struct seq_file *m, void *unused)
 	struct drm_device *dev = node->minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct vc4_dpi *dpi = vc4->dpi;
-	int i;
+	struct drm_printer p = drm_seq_file_printer(m);
 
 	if (!dpi)
 		return 0;
 
-	for (i = 0; i < ARRAY_SIZE(dpi_regs); i++) {
-		seq_printf(m, "%s (0x%04x): 0x%08x\n",
-			   dpi_regs[i].name, dpi_regs[i].reg,
-			   DPI_READ(dpi_regs[i].reg));
-	}
+	drm_print_regset32(&p, &dpi->regset);
 
 	return 0;
 }
@@ -314,6 +308,9 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data)
 	dpi->regs = vc4_ioremap_regs(pdev, 0);
 	if (IS_ERR(dpi->regs))
 		return PTR_ERR(dpi->regs);
+	dpi->regset.base = dpi->regs;
+	dpi->regset.regs = dpi_regs;
+	dpi->regset.nregs = ARRAY_SIZE(dpi_regs);
 
 	if (DPI_READ(DPI_ID) != DPI_ID_VALUE) {
 		dev_err(dev, "Port returned 0x%08x for ID instead of 0x%08x\n",
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index ce1af06cc197..511ad884d9c3 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -285,6 +285,7 @@ struct vc4_v3d {
 	struct platform_device *pdev;
 	void __iomem *regs;
 	struct clk *clk;
+	struct debugfs_regset32 regset;
 };
 
 struct vc4_hvs {
@@ -301,6 +302,7 @@ struct vc4_hvs {
 	spinlock_t mm_lock;
 
 	struct drm_mm_node mitchell_netravali_filter;
+	struct debugfs_regset32 regset;
 };
 
 struct vc4_plane {
@@ -410,6 +412,7 @@ struct vc4_crtc_data {
 
 struct vc4_crtc {
 	struct drm_crtc base;
+	struct platform_device *pdev;
 	const struct vc4_crtc_data *data;
 	void __iomem *regs;
 
@@ -426,6 +429,8 @@ struct vc4_crtc {
 	u32 cob_size;
 
 	struct drm_pending_vblank_event *event;
+
+	struct debugfs_regset32 regset;
 };
 
 static inline struct vc4_crtc *
@@ -439,6 +444,8 @@ to_vc4_crtc(struct drm_crtc *crtc)
 #define HVS_READ(offset) readl(vc4->hvs->regs + offset)
 #define HVS_WRITE(offset, val) writel(val, vc4->hvs->regs + offset)
 
+#define VC4_REG32(reg) { .name = #reg, .offset = reg }
+
 struct vc4_exec_info {
 	/* Sequence number for this bin/render job. */
 	uint64_t seqno;
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 11702e1d9011..806cfaa2a6a7 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -545,6 +545,8 @@ struct vc4_dsi {
 
 	struct completion xfer_completion;
 	int xfer_result;
+
+	struct debugfs_regset32 regset;
 };
 
 #define host_to_dsi(host) container_of(host, struct vc4_dsi, dsi_host)
@@ -605,82 +607,56 @@ to_vc4_dsi_encoder(struct drm_encoder *encoder)
 	return container_of(encoder, struct vc4_dsi_encoder, base.base);
 }
 
-#define DSI_REG(reg) { reg, #reg }
-static const struct {
-	u32 reg;
-	const char *name;
-} dsi0_regs[] = {
-	DSI_REG(DSI0_CTRL),
-	DSI_REG(DSI0_STAT),
-	DSI_REG(DSI0_HSTX_TO_CNT),
-	DSI_REG(DSI0_LPRX_TO_CNT),
-	DSI_REG(DSI0_TA_TO_CNT),
-	DSI_REG(DSI0_PR_TO_CNT),
-	DSI_REG(DSI0_DISP0_CTRL),
-	DSI_REG(DSI0_DISP1_CTRL),
-	DSI_REG(DSI0_INT_STAT),
-	DSI_REG(DSI0_INT_EN),
-	DSI_REG(DSI0_PHYC),
-	DSI_REG(DSI0_HS_CLT0),
-	DSI_REG(DSI0_HS_CLT1),
-	DSI_REG(DSI0_HS_CLT2),
-	DSI_REG(DSI0_HS_DLT3),
-	DSI_REG(DSI0_HS_DLT4),
-	DSI_REG(DSI0_HS_DLT5),
-	DSI_REG(DSI0_HS_DLT6),
-	DSI_REG(DSI0_HS_DLT7),
-	DSI_REG(DSI0_PHY_AFEC0),
-	DSI_REG(DSI0_PHY_AFEC1),
-	DSI_REG(DSI0_ID),
+static const struct debugfs_reg32 dsi0_regs[] = {
+	VC4_REG32(DSI0_CTRL),
+	VC4_REG32(DSI0_STAT),
+	VC4_REG32(DSI0_HSTX_TO_CNT),
+	VC4_REG32(DSI0_LPRX_TO_CNT),
+	VC4_REG32(DSI0_TA_TO_CNT),
+	VC4_REG32(DSI0_PR_TO_CNT),
+	VC4_REG32(DSI0_DISP0_CTRL),
+	VC4_REG32(DSI0_DISP1_CTRL),
+	VC4_REG32(DSI0_INT_STAT),
+	VC4_REG32(DSI0_INT_EN),
+	VC4_REG32(DSI0_PHYC),
+	VC4_REG32(DSI0_HS_CLT0),
+	VC4_REG32(DSI0_HS_CLT1),
+	VC4_REG32(DSI0_HS_CLT2),
+	VC4_REG32(DSI0_HS_DLT3),
+	VC4_REG32(DSI0_HS_DLT4),
+	VC4_REG32(DSI0_HS_DLT5),
+	VC4_REG32(DSI0_HS_DLT6),
+	VC4_REG32(DSI0_HS_DLT7),
+	VC4_REG32(DSI0_PHY_AFEC0),
+	VC4_REG32(DSI0_PHY_AFEC1),
+	VC4_REG32(DSI0_ID),
 };
 
-static const struct {
-	u32 reg;
-	const char *name;
-} dsi1_regs[] = {
-	DSI_REG(DSI1_CTRL),
-	DSI_REG(DSI1_STAT),
-	DSI_REG(DSI1_HSTX_TO_CNT),
-	DSI_REG(DSI1_LPRX_TO_CNT),
-	DSI_REG(DSI1_TA_TO_CNT),
-	DSI_REG(DSI1_PR_TO_CNT),
-	DSI_REG(DSI1_DISP0_CTRL),
-	DSI_REG(DSI1_DISP1_CTRL),
-	DSI_REG(DSI1_INT_STAT),
-	DSI_REG(DSI1_INT_EN),
-	DSI_REG(DSI1_PHYC),
-	DSI_REG(DSI1_HS_CLT0),
-	DSI_REG(DSI1_HS_CLT1),
-	DSI_REG(DSI1_HS_CLT2),
-	DSI_REG(DSI1_HS_DLT3),
-	DSI_REG(DSI1_HS_DLT4),
-	DSI_REG(DSI1_HS_DLT5),
-	DSI_REG(DSI1_HS_DLT6),
-	DSI_REG(DSI1_HS_DLT7),
-	DSI_REG(DSI1_PHY_AFEC0),
-	DSI_REG(DSI1_PHY_AFEC1),
-	DSI_REG(DSI1_ID),
+static const struct debugfs_reg32 dsi1_regs[] = {
+	VC4_REG32(DSI1_CTRL),
+	VC4_REG32(DSI1_STAT),
+	VC4_REG32(DSI1_HSTX_TO_CNT),
+	VC4_REG32(DSI1_LPRX_TO_CNT),
+	VC4_REG32(DSI1_TA_TO_CNT),
+	VC4_REG32(DSI1_PR_TO_CNT),
+	VC4_REG32(DSI1_DISP0_CTRL),
+	VC4_REG32(DSI1_DISP1_CTRL),
+	VC4_REG32(DSI1_INT_STAT),
+	VC4_REG32(DSI1_INT_EN),
+	VC4_REG32(DSI1_PHYC),
+	VC4_REG32(DSI1_HS_CLT0),
+	VC4_REG32(DSI1_HS_CLT1),
+	VC4_REG32(DSI1_HS_CLT2),
+	VC4_REG32(DSI1_HS_DLT3),
+	VC4_REG32(DSI1_HS_DLT4),
+	VC4_REG32(DSI1_HS_DLT5),
+	VC4_REG32(DSI1_HS_DLT6),
+	VC4_REG32(DSI1_HS_DLT7),
+	VC4_REG32(DSI1_PHY_AFEC0),
+	VC4_REG32(DSI1_PHY_AFEC1),
+	VC4_REG32(DSI1_ID),
 };
 
-static void vc4_dsi_dump_regs(struct vc4_dsi *dsi)
-{
-	int i;
-
-	if (dsi->port == 0) {
-		for (i = 0; i < ARRAY_SIZE(dsi0_regs); i++) {
-			DRM_INFO("0x%04x (%s): 0x%08x\n",
-				 dsi0_regs[i].reg, dsi0_regs[i].name,
-				 DSI_READ(dsi0_regs[i].reg));
-		}
-	} else {
-		for (i = 0; i < ARRAY_SIZE(dsi1_regs); i++) {
-			DRM_INFO("0x%04x (%s): 0x%08x\n",
-				 dsi1_regs[i].reg, dsi1_regs[i].name,
-				 DSI_READ(dsi1_regs[i].reg));
-		}
-	}
-}
-
 #ifdef CONFIG_DEBUG_FS
 int vc4_dsi_debugfs_regs(struct seq_file *m, void *unused)
 {
@@ -689,24 +665,12 @@ int vc4_dsi_debugfs_regs(struct seq_file *m, void *unused)
 	struct vc4_dev *vc4 = to_vc4_dev(drm);
 	int dsi_index = (uintptr_t)node->info_ent->data;
 	struct vc4_dsi *dsi = (dsi_index == 1 ? vc4->dsi1 : NULL);
-	int i;
+	struct drm_printer p = drm_seq_file_printer(m);
 
 	if (!dsi)
 		return 0;
 
-	if (dsi->port == 0) {
-		for (i = 0; i < ARRAY_SIZE(dsi0_regs); i++) {
-			seq_printf(m, "0x%04x (%s): 0x%08x\n",
-				   dsi0_regs[i].reg, dsi0_regs[i].name,
-				   DSI_READ(dsi0_regs[i].reg));
-		}
-	} else {
-		for (i = 0; i < ARRAY_SIZE(dsi1_regs); i++) {
-			seq_printf(m, "0x%04x (%s): 0x%08x\n",
-				   dsi1_regs[i].reg, dsi1_regs[i].name,
-				   DSI_READ(dsi1_regs[i].reg));
-		}
-	}
+	drm_print_regset32(&p, &dsi->regset);
 
 	return 0;
 }
@@ -900,8 +864,9 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
 	}
 
 	if (debug_dump_regs) {
-		DRM_INFO("DSI regs before:\n");
-		vc4_dsi_dump_regs(dsi);
+		struct drm_printer p = drm_info_printer(&dsi->pdev->dev);
+		dev_info(&dsi->pdev->dev, "DSI regs before:\n");
+		drm_print_regset32(&p, &dsi->regset);
 	}
 
 	/* Round up the clk_set_rate() request slightly, since
@@ -1135,8 +1100,9 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
 	drm_bridge_enable(dsi->bridge);
 
 	if (debug_dump_regs) {
-		DRM_INFO("DSI regs after:\n");
-		vc4_dsi_dump_regs(dsi);
+		struct drm_printer p = drm_info_printer(&dsi->pdev->dev);
+		dev_info(&dsi->pdev->dev, "DSI regs after:\n");
+		drm_print_regset32(&p, &dsi->regset);
 	}
 }
 
@@ -1527,6 +1493,15 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
 	if (IS_ERR(dsi->regs))
 		return PTR_ERR(dsi->regs);
 
+	dsi->regset.base = dsi->regs;
+	if (dsi->port == 0) {
+		dsi->regset.regs = dsi0_regs;
+		dsi->regset.nregs = ARRAY_SIZE(dsi0_regs);
+	} else {
+		dsi->regset.regs = dsi1_regs;
+		dsi->regset.nregs = ARRAY_SIZE(dsi1_regs);
+	}
+
 	if (DSI_PORT_READ(ID) != DSI_ID_VALUE) {
 		dev_err(dev, "Port returned 0x%08x for ID instead of 0x%08x\n",
 			DSI_PORT_READ(ID), DSI_ID_VALUE);
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 88fd5df7e7dc..38c9172cfe52 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -97,6 +97,9 @@ struct vc4_hdmi {
 
 	struct clk *pixel_clock;
 	struct clk *hsm_clock;
+
+	struct debugfs_regset32 hdmi_regset;
+	struct debugfs_regset32 hd_regset;
 };
 
 #define HDMI_READ(offset) readl(vc4->hdmi->hdmicore_regs + offset)
@@ -134,61 +137,54 @@ to_vc4_hdmi_connector(struct drm_connector *connector)
 	return container_of(connector, struct vc4_hdmi_connector, base);
 }
 
-#define HDMI_REG(reg) { reg, #reg }
-static const struct {
-	u32 reg;
-	const char *name;
-} hdmi_regs[] = {
-	HDMI_REG(VC4_HDMI_CORE_REV),
-	HDMI_REG(VC4_HDMI_SW_RESET_CONTROL),
-	HDMI_REG(VC4_HDMI_HOTPLUG_INT),
-	HDMI_REG(VC4_HDMI_HOTPLUG),
-	HDMI_REG(VC4_HDMI_MAI_CHANNEL_MAP),
-	HDMI_REG(VC4_HDMI_MAI_CONFIG),
-	HDMI_REG(VC4_HDMI_MAI_FORMAT),
-	HDMI_REG(VC4_HDMI_AUDIO_PACKET_CONFIG),
-	HDMI_REG(VC4_HDMI_RAM_PACKET_CONFIG),
-	HDMI_REG(VC4_HDMI_HORZA),
-	HDMI_REG(VC4_HDMI_HORZB),
-	HDMI_REG(VC4_HDMI_FIFO_CTL),
-	HDMI_REG(VC4_HDMI_SCHEDULER_CONTROL),
-	HDMI_REG(VC4_HDMI_VERTA0),
-	HDMI_REG(VC4_HDMI_VERTA1),
-	HDMI_REG(VC4_HDMI_VERTB0),
-	HDMI_REG(VC4_HDMI_VERTB1),
-	HDMI_REG(VC4_HDMI_TX_PHY_RESET_CTL),
-	HDMI_REG(VC4_HDMI_TX_PHY_CTL0),
-
-	HDMI_REG(VC4_HDMI_CEC_CNTRL_1),
-	HDMI_REG(VC4_HDMI_CEC_CNTRL_2),
-	HDMI_REG(VC4_HDMI_CEC_CNTRL_3),
-	HDMI_REG(VC4_HDMI_CEC_CNTRL_4),
-	HDMI_REG(VC4_HDMI_CEC_CNTRL_5),
-	HDMI_REG(VC4_HDMI_CPU_STATUS),
-	HDMI_REG(VC4_HDMI_CPU_MASK_STATUS),
-
-	HDMI_REG(VC4_HDMI_CEC_RX_DATA_1),
-	HDMI_REG(VC4_HDMI_CEC_RX_DATA_2),
-	HDMI_REG(VC4_HDMI_CEC_RX_DATA_3),
-	HDMI_REG(VC4_HDMI_CEC_RX_DATA_4),
-	HDMI_REG(VC4_HDMI_CEC_TX_DATA_1),
-	HDMI_REG(VC4_HDMI_CEC_TX_DATA_2),
-	HDMI_REG(VC4_HDMI_CEC_TX_DATA_3),
-	HDMI_REG(VC4_HDMI_CEC_TX_DATA_4),
+static const struct debugfs_reg32 hdmi_regs[] = {
+	VC4_REG32(VC4_HDMI_CORE_REV),
+	VC4_REG32(VC4_HDMI_SW_RESET_CONTROL),
+	VC4_REG32(VC4_HDMI_HOTPLUG_INT),
+	VC4_REG32(VC4_HDMI_HOTPLUG),
+	VC4_REG32(VC4_HDMI_MAI_CHANNEL_MAP),
+	VC4_REG32(VC4_HDMI_MAI_CONFIG),
+	VC4_REG32(VC4_HDMI_MAI_FORMAT),
+	VC4_REG32(VC4_HDMI_AUDIO_PACKET_CONFIG),
+	VC4_REG32(VC4_HDMI_RAM_PACKET_CONFIG),
+	VC4_REG32(VC4_HDMI_HORZA),
+	VC4_REG32(VC4_HDMI_HORZB),
+	VC4_REG32(VC4_HDMI_FIFO_CTL),
+	VC4_REG32(VC4_HDMI_SCHEDULER_CONTROL),
+	VC4_REG32(VC4_HDMI_VERTA0),
+	VC4_REG32(VC4_HDMI_VERTA1),
+	VC4_REG32(VC4_HDMI_VERTB0),
+	VC4_REG32(VC4_HDMI_VERTB1),
+	VC4_REG32(VC4_HDMI_TX_PHY_RESET_CTL),
+	VC4_REG32(VC4_HDMI_TX_PHY_CTL0),
+
+	VC4_REG32(VC4_HDMI_CEC_CNTRL_1),
+	VC4_REG32(VC4_HDMI_CEC_CNTRL_2),
+	VC4_REG32(VC4_HDMI_CEC_CNTRL_3),
+	VC4_REG32(VC4_HDMI_CEC_CNTRL_4),
+	VC4_REG32(VC4_HDMI_CEC_CNTRL_5),
+	VC4_REG32(VC4_HDMI_CPU_STATUS),
+	VC4_REG32(VC4_HDMI_CPU_MASK_STATUS),
+
+	VC4_REG32(VC4_HDMI_CEC_RX_DATA_1),
+	VC4_REG32(VC4_HDMI_CEC_RX_DATA_2),
+	VC4_REG32(VC4_HDMI_CEC_RX_DATA_3),
+	VC4_REG32(VC4_HDMI_CEC_RX_DATA_4),
+	VC4_REG32(VC4_HDMI_CEC_TX_DATA_1),
+	VC4_REG32(VC4_HDMI_CEC_TX_DATA_2),
+	VC4_REG32(VC4_HDMI_CEC_TX_DATA_3),
+	VC4_REG32(VC4_HDMI_CEC_TX_DATA_4),
 };
 
-static const struct {
-	u32 reg;
-	const char *name;
-} hd_regs[] = {
-	HDMI_REG(VC4_HD_M_CTL),
-	HDMI_REG(VC4_HD_MAI_CTL),
-	HDMI_REG(VC4_HD_MAI_THR),
-	HDMI_REG(VC4_HD_MAI_FMT),
-	HDMI_REG(VC4_HD_MAI_SMP),
-	HDMI_REG(VC4_HD_VID_CTL),
-	HDMI_REG(VC4_HD_CSC_CTL),
-	HDMI_REG(VC4_HD_FRAME_COUNT),
+static const struct debugfs_reg32 hd_regs[] = {
+	VC4_REG32(VC4_HD_M_CTL),
+	VC4_REG32(VC4_HD_MAI_CTL),
+	VC4_REG32(VC4_HD_MAI_THR),
+	VC4_REG32(VC4_HD_MAI_FMT),
+	VC4_REG32(VC4_HD_MAI_SMP),
+	VC4_REG32(VC4_HD_VID_CTL),
+	VC4_REG32(VC4_HD_CSC_CTL),
+	VC4_REG32(VC4_HD_FRAME_COUNT),
 };
 
 #ifdef CONFIG_DEBUG_FS
@@ -197,41 +193,16 @@ int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *dev = node->minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(hdmi_regs); i++) {
-		seq_printf(m, "%s (0x%04x): 0x%08x\n",
-			   hdmi_regs[i].name, hdmi_regs[i].reg,
-			   HDMI_READ(hdmi_regs[i].reg));
-	}
+	struct vc4_hdmi *hdmi = vc4->hdmi;
+	struct drm_printer p = drm_seq_file_printer(m);
 
-	for (i = 0; i < ARRAY_SIZE(hd_regs); i++) {
-		seq_printf(m, "%s (0x%04x): 0x%08x\n",
-			   hd_regs[i].name, hd_regs[i].reg,
-			   HD_READ(hd_regs[i].reg));
-	}
+	drm_print_regset32(&p, &hdmi->hdmi_regset);
+	drm_print_regset32(&p, &hdmi->hd_regset);
 
 	return 0;
 }
 #endif /* CONFIG_DEBUG_FS */
 
-static void vc4_hdmi_dump_regs(struct drm_device *dev)
-{
-	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(hdmi_regs); i++) {
-		DRM_INFO("0x%04x (%s): 0x%08x\n",
-			 hdmi_regs[i].reg, hdmi_regs[i].name,
-			 HDMI_READ(hdmi_regs[i].reg));
-	}
-	for (i = 0; i < ARRAY_SIZE(hd_regs); i++) {
-		DRM_INFO("0x%04x (%s): 0x%08x\n",
-			 hd_regs[i].reg, hd_regs[i].name,
-			 HD_READ(hd_regs[i].reg));
-	}
-}
-
 static enum drm_connector_status
 vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
 {
@@ -561,8 +532,11 @@ static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder)
 	HDMI_WRITE(VC4_HDMI_TX_PHY_RESET_CTL, 0);
 
 	if (debug_dump_regs) {
-		DRM_INFO("HDMI regs before:\n");
-		vc4_hdmi_dump_regs(dev);
+		struct drm_printer p = drm_info_printer(&hdmi->pdev->dev);
+
+		dev_info(&hdmi->pdev->dev, "HDMI regs before:\n");
+		drm_print_regset32(&p, &hdmi->hdmi_regset);
+		drm_print_regset32(&p, &hdmi->hd_regset);
 	}
 
 	HD_WRITE(VC4_HD_VID_CTL, 0);
@@ -637,8 +611,11 @@ static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder)
 	HDMI_WRITE(VC4_HDMI_FIFO_CTL, VC4_HDMI_FIFO_CTL_MASTER_SLAVE_N);
 
 	if (debug_dump_regs) {
-		DRM_INFO("HDMI regs after:\n");
-		vc4_hdmi_dump_regs(dev);
+		struct drm_printer p = drm_info_printer(&hdmi->pdev->dev);
+
+		dev_info(&hdmi->pdev->dev, "HDMI regs after:\n");
+		drm_print_regset32(&p, &hdmi->hdmi_regset);
+		drm_print_regset32(&p, &hdmi->hd_regset);
 	}
 
 	HD_WRITE(VC4_HD_VID_CTL,
@@ -1333,6 +1310,13 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 	if (IS_ERR(hdmi->hd_regs))
 		return PTR_ERR(hdmi->hd_regs);
 
+	hdmi->hdmi_regset.base = hdmi->hdmicore_regs;
+	hdmi->hdmi_regset.regs = hdmi_regs;
+	hdmi->hdmi_regset.nregs = ARRAY_SIZE(hdmi_regs);
+	hdmi->hd_regset.base = hdmi->hd_regs;
+	hdmi->hd_regset.regs = hd_regs;
+	hdmi->hd_regset.nregs = ARRAY_SIZE(hd_regs);
+
 	hdmi->pixel_clock = devm_clk_get(dev, "pixel");
 	if (IS_ERR(hdmi->pixel_clock)) {
 		DRM_ERROR("Failed to get pixel clock\n");
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 5d8c749c9749..45405f76cd40 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -26,54 +26,47 @@
 #include "vc4_drv.h"
 #include "vc4_regs.h"
 
-#define HVS_REG(reg) { reg, #reg }
-static const struct {
-	u32 reg;
-	const char *name;
-} hvs_regs[] = {
-	HVS_REG(SCALER_DISPCTRL),
-	HVS_REG(SCALER_DISPSTAT),
-	HVS_REG(SCALER_DISPID),
-	HVS_REG(SCALER_DISPECTRL),
-	HVS_REG(SCALER_DISPPROF),
-	HVS_REG(SCALER_DISPDITHER),
-	HVS_REG(SCALER_DISPEOLN),
-	HVS_REG(SCALER_DISPLIST0),
-	HVS_REG(SCALER_DISPLIST1),
-	HVS_REG(SCALER_DISPLIST2),
-	HVS_REG(SCALER_DISPLSTAT),
-	HVS_REG(SCALER_DISPLACT0),
-	HVS_REG(SCALER_DISPLACT1),
-	HVS_REG(SCALER_DISPLACT2),
-	HVS_REG(SCALER_DISPCTRL0),
-	HVS_REG(SCALER_DISPBKGND0),
-	HVS_REG(SCALER_DISPSTAT0),
-	HVS_REG(SCALER_DISPBASE0),
-	HVS_REG(SCALER_DISPCTRL1),
-	HVS_REG(SCALER_DISPBKGND1),
-	HVS_REG(SCALER_DISPSTAT1),
-	HVS_REG(SCALER_DISPBASE1),
-	HVS_REG(SCALER_DISPCTRL2),
-	HVS_REG(SCALER_DISPBKGND2),
-	HVS_REG(SCALER_DISPSTAT2),
-	HVS_REG(SCALER_DISPBASE2),
-	HVS_REG(SCALER_DISPALPHA2),
-	HVS_REG(SCALER_OLEDOFFS),
-	HVS_REG(SCALER_OLEDCOEF0),
-	HVS_REG(SCALER_OLEDCOEF1),
-	HVS_REG(SCALER_OLEDCOEF2),
+static const struct debugfs_reg32 hvs_regs[] = {
+	VC4_REG32(SCALER_DISPCTRL),
+	VC4_REG32(SCALER_DISPSTAT),
+	VC4_REG32(SCALER_DISPID),
+	VC4_REG32(SCALER_DISPECTRL),
+	VC4_REG32(SCALER_DISPPROF),
+	VC4_REG32(SCALER_DISPDITHER),
+	VC4_REG32(SCALER_DISPEOLN),
+	VC4_REG32(SCALER_DISPLIST0),
+	VC4_REG32(SCALER_DISPLIST1),
+	VC4_REG32(SCALER_DISPLIST2),
+	VC4_REG32(SCALER_DISPLSTAT),
+	VC4_REG32(SCALER_DISPLACT0),
+	VC4_REG32(SCALER_DISPLACT1),
+	VC4_REG32(SCALER_DISPLACT2),
+	VC4_REG32(SCALER_DISPCTRL0),
+	VC4_REG32(SCALER_DISPBKGND0),
+	VC4_REG32(SCALER_DISPSTAT0),
+	VC4_REG32(SCALER_DISPBASE0),
+	VC4_REG32(SCALER_DISPCTRL1),
+	VC4_REG32(SCALER_DISPBKGND1),
+	VC4_REG32(SCALER_DISPSTAT1),
+	VC4_REG32(SCALER_DISPBASE1),
+	VC4_REG32(SCALER_DISPCTRL2),
+	VC4_REG32(SCALER_DISPBKGND2),
+	VC4_REG32(SCALER_DISPSTAT2),
+	VC4_REG32(SCALER_DISPBASE2),
+	VC4_REG32(SCALER_DISPALPHA2),
+	VC4_REG32(SCALER_OLEDOFFS),
+	VC4_REG32(SCALER_OLEDCOEF0),
+	VC4_REG32(SCALER_OLEDCOEF1),
+	VC4_REG32(SCALER_OLEDCOEF2),
 };
 
 void vc4_hvs_dump_state(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	struct drm_printer p = drm_info_printer(&vc4->hvs->pdev->dev);
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(hvs_regs); i++) {
-		DRM_INFO("0x%04x (%s): 0x%08x\n",
-			 hvs_regs[i].reg, hvs_regs[i].name,
-			 HVS_READ(hvs_regs[i].reg));
-	}
+	drm_print_regset32(&p, &vc4->hvs->regset);
 
 	DRM_INFO("HVS ctx:\n");
 	for (i = 0; i < 64; i += 4) {
@@ -92,13 +85,9 @@ int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused)
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *dev = node->minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	int i;
+	struct drm_printer p = drm_seq_file_printer(m);
 
-	for (i = 0; i < ARRAY_SIZE(hvs_regs); i++) {
-		seq_printf(m, "%s (0x%04x): 0x%08x\n",
-			   hvs_regs[i].name, hvs_regs[i].reg,
-			   HVS_READ(hvs_regs[i].reg));
-	}
+	drm_print_regset32(&p, &vc4->hvs->regset);
 
 	return 0;
 }
@@ -185,6 +174,10 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 	if (IS_ERR(hvs->regs))
 		return PTR_ERR(hvs->regs);
 
+	hvs->regset.base = hvs->regs;
+	hvs->regset.regs = hvs_regs;
+	hvs->regset.nregs = ARRAY_SIZE(hvs_regs);
+
 	hvs->dlist = hvs->regs + SCALER_DLIST_START;
 
 	spin_lock_init(&hvs->mm_lock);
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index afb1c4ec4f18..c8f80064c179 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -148,6 +148,7 @@ struct vc4_txp {
 	struct drm_writeback_connector connector;
 
 	void __iomem *regs;
+	struct debugfs_regset32 regset;
 };
 
 static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder *encoder)
@@ -160,16 +161,12 @@ static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector *conn)
 	return container_of(conn, struct vc4_txp, connector.base);
 }
 
-#define TXP_REG(reg) { reg, #reg }
-static const struct {
-	u32 reg;
-	const char *name;
-} txp_regs[] = {
-	TXP_REG(TXP_DST_PTR),
-	TXP_REG(TXP_DST_PITCH),
-	TXP_REG(TXP_DIM),
-	TXP_REG(TXP_DST_CTRL),
-	TXP_REG(TXP_PROGRESS),
+static const struct debugfs_reg32 txp_regs[] = {
+	VC4_REG32(TXP_DST_PTR),
+	VC4_REG32(TXP_DST_PITCH),
+	VC4_REG32(TXP_DIM),
+	VC4_REG32(TXP_DST_CTRL),
+	VC4_REG32(TXP_PROGRESS),
 };
 
 #ifdef CONFIG_DEBUG_FS
@@ -179,16 +176,12 @@ int vc4_txp_debugfs_regs(struct seq_file *m, void *unused)
 	struct drm_device *dev = node->minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct vc4_txp *txp = vc4->txp;
-	int i;
+	struct drm_printer p = drm_seq_file_printer(m);
 
 	if (!txp)
 		return 0;
 
-	for (i = 0; i < ARRAY_SIZE(txp_regs); i++) {
-		seq_printf(m, "%s (0x%04x): 0x%08x\n",
-			   txp_regs[i].name, txp_regs[i].reg,
-			   TXP_READ(txp_regs[i].reg));
-	}
+	drm_print_regset32(&p, &txp->regset);
 
 	return 0;
 }
@@ -410,6 +403,9 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data)
 	txp->regs = vc4_ioremap_regs(pdev, 0);
 	if (IS_ERR(txp->regs))
 		return PTR_ERR(txp->regs);
+	txp->regset.base = txp->regs;
+	txp->regset.regs = txp_regs;
+	txp->regset.nregs = ARRAY_SIZE(txp_regs);
 
 	drm_connector_helper_add(&txp->connector.base,
 				 &vc4_txp_connector_helper_funcs);
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index e47e29426078..e1e728e95562 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -23,89 +23,85 @@
 #include "vc4_regs.h"
 
 #ifdef CONFIG_DEBUG_FS
-#define REGDEF(reg) { reg, #reg }
-static const struct {
-	uint32_t reg;
-	const char *name;
-} vc4_reg_defs[] = {
-	REGDEF(V3D_IDENT0),
-	REGDEF(V3D_IDENT1),
-	REGDEF(V3D_IDENT2),
-	REGDEF(V3D_SCRATCH),
-	REGDEF(V3D_L2CACTL),
-	REGDEF(V3D_SLCACTL),
-	REGDEF(V3D_INTCTL),
-	REGDEF(V3D_INTENA),
-	REGDEF(V3D_INTDIS),
-	REGDEF(V3D_CT0CS),
-	REGDEF(V3D_CT1CS),
-	REGDEF(V3D_CT0EA),
-	REGDEF(V3D_CT1EA),
-	REGDEF(V3D_CT0CA),
-	REGDEF(V3D_CT1CA),
-	REGDEF(V3D_CT00RA0),
-	REGDEF(V3D_CT01RA0),
-	REGDEF(V3D_CT0LC),
-	REGDEF(V3D_CT1LC),
-	REGDEF(V3D_CT0PC),
-	REGDEF(V3D_CT1PC),
-	REGDEF(V3D_PCS),
-	REGDEF(V3D_BFC),
-	REGDEF(V3D_RFC),
-	REGDEF(V3D_BPCA),
-	REGDEF(V3D_BPCS),
-	REGDEF(V3D_BPOA),
-	REGDEF(V3D_BPOS),
-	REGDEF(V3D_BXCF),
-	REGDEF(V3D_SQRSV0),
-	REGDEF(V3D_SQRSV1),
-	REGDEF(V3D_SQCNTL),
-	REGDEF(V3D_SRQPC),
-	REGDEF(V3D_SRQUA),
-	REGDEF(V3D_SRQUL),
-	REGDEF(V3D_SRQCS),
-	REGDEF(V3D_VPACNTL),
-	REGDEF(V3D_VPMBASE),
-	REGDEF(V3D_PCTRC),
-	REGDEF(V3D_PCTRE),
-	REGDEF(V3D_PCTR(0)),
-	REGDEF(V3D_PCTRS(0)),
-	REGDEF(V3D_PCTR(1)),
-	REGDEF(V3D_PCTRS(1)),
-	REGDEF(V3D_PCTR(2)),
-	REGDEF(V3D_PCTRS(2)),
-	REGDEF(V3D_PCTR(3)),
-	REGDEF(V3D_PCTRS(3)),
-	REGDEF(V3D_PCTR(4)),
-	REGDEF(V3D_PCTRS(4)),
-	REGDEF(V3D_PCTR(5)),
-	REGDEF(V3D_PCTRS(5)),
-	REGDEF(V3D_PCTR(6)),
-	REGDEF(V3D_PCTRS(6)),
-	REGDEF(V3D_PCTR(7)),
-	REGDEF(V3D_PCTRS(7)),
-	REGDEF(V3D_PCTR(8)),
-	REGDEF(V3D_PCTRS(8)),
-	REGDEF(V3D_PCTR(9)),
-	REGDEF(V3D_PCTRS(9)),
-	REGDEF(V3D_PCTR(10)),
-	REGDEF(V3D_PCTRS(10)),
-	REGDEF(V3D_PCTR(11)),
-	REGDEF(V3D_PCTRS(11)),
-	REGDEF(V3D_PCTR(12)),
-	REGDEF(V3D_PCTRS(12)),
-	REGDEF(V3D_PCTR(13)),
-	REGDEF(V3D_PCTRS(13)),
-	REGDEF(V3D_PCTR(14)),
-	REGDEF(V3D_PCTRS(14)),
-	REGDEF(V3D_PCTR(15)),
-	REGDEF(V3D_PCTRS(15)),
-	REGDEF(V3D_DBGE),
-	REGDEF(V3D_FDBGO),
-	REGDEF(V3D_FDBGB),
-	REGDEF(V3D_FDBGR),
-	REGDEF(V3D_FDBGS),
-	REGDEF(V3D_ERRSTAT),
+static const struct debugfs_reg32 v3d_regs[] = {
+	VC4_REG32(V3D_IDENT0),
+	VC4_REG32(V3D_IDENT1),
+	VC4_REG32(V3D_IDENT2),
+	VC4_REG32(V3D_SCRATCH),
+	VC4_REG32(V3D_L2CACTL),
+	VC4_REG32(V3D_SLCACTL),
+	VC4_REG32(V3D_INTCTL),
+	VC4_REG32(V3D_INTENA),
+	VC4_REG32(V3D_INTDIS),
+	VC4_REG32(V3D_CT0CS),
+	VC4_REG32(V3D_CT1CS),
+	VC4_REG32(V3D_CT0EA),
+	VC4_REG32(V3D_CT1EA),
+	VC4_REG32(V3D_CT0CA),
+	VC4_REG32(V3D_CT1CA),
+	VC4_REG32(V3D_CT00RA0),
+	VC4_REG32(V3D_CT01RA0),
+	VC4_REG32(V3D_CT0LC),
+	VC4_REG32(V3D_CT1LC),
+	VC4_REG32(V3D_CT0PC),
+	VC4_REG32(V3D_CT1PC),
+	VC4_REG32(V3D_PCS),
+	VC4_REG32(V3D_BFC),
+	VC4_REG32(V3D_RFC),
+	VC4_REG32(V3D_BPCA),
+	VC4_REG32(V3D_BPCS),
+	VC4_REG32(V3D_BPOA),
+	VC4_REG32(V3D_BPOS),
+	VC4_REG32(V3D_BXCF),
+	VC4_REG32(V3D_SQRSV0),
+	VC4_REG32(V3D_SQRSV1),
+	VC4_REG32(V3D_SQCNTL),
+	VC4_REG32(V3D_SRQPC),
+	VC4_REG32(V3D_SRQUA),
+	VC4_REG32(V3D_SRQUL),
+	VC4_REG32(V3D_SRQCS),
+	VC4_REG32(V3D_VPACNTL),
+	VC4_REG32(V3D_VPMBASE),
+	VC4_REG32(V3D_PCTRC),
+	VC4_REG32(V3D_PCTRE),
+	VC4_REG32(V3D_PCTR(0)),
+	VC4_REG32(V3D_PCTRS(0)),
+	VC4_REG32(V3D_PCTR(1)),
+	VC4_REG32(V3D_PCTRS(1)),
+	VC4_REG32(V3D_PCTR(2)),
+	VC4_REG32(V3D_PCTRS(2)),
+	VC4_REG32(V3D_PCTR(3)),
+	VC4_REG32(V3D_PCTRS(3)),
+	VC4_REG32(V3D_PCTR(4)),
+	VC4_REG32(V3D_PCTRS(4)),
+	VC4_REG32(V3D_PCTR(5)),
+	VC4_REG32(V3D_PCTRS(5)),
+	VC4_REG32(V3D_PCTR(6)),
+	VC4_REG32(V3D_PCTRS(6)),
+	VC4_REG32(V3D_PCTR(7)),
+	VC4_REG32(V3D_PCTRS(7)),
+	VC4_REG32(V3D_PCTR(8)),
+	VC4_REG32(V3D_PCTRS(8)),
+	VC4_REG32(V3D_PCTR(9)),
+	VC4_REG32(V3D_PCTRS(9)),
+	VC4_REG32(V3D_PCTR(10)),
+	VC4_REG32(V3D_PCTRS(10)),
+	VC4_REG32(V3D_PCTR(11)),
+	VC4_REG32(V3D_PCTRS(11)),
+	VC4_REG32(V3D_PCTR(12)),
+	VC4_REG32(V3D_PCTRS(12)),
+	VC4_REG32(V3D_PCTR(13)),
+	VC4_REG32(V3D_PCTRS(13)),
+	VC4_REG32(V3D_PCTR(14)),
+	VC4_REG32(V3D_PCTRS(14)),
+	VC4_REG32(V3D_PCTR(15)),
+	VC4_REG32(V3D_PCTRS(15)),
+	VC4_REG32(V3D_DBGE),
+	VC4_REG32(V3D_FDBGO),
+	VC4_REG32(V3D_FDBGB),
+	VC4_REG32(V3D_FDBGR),
+	VC4_REG32(V3D_FDBGS),
+	VC4_REG32(V3D_ERRSTAT),
 };
 
 int vc4_v3d_debugfs_regs(struct seq_file *m, void *unused)
@@ -113,13 +109,9 @@ int vc4_v3d_debugfs_regs(struct seq_file *m, void *unused)
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *dev = node->minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	int i;
+	struct drm_printer p = drm_seq_file_printer(m);
 
-	for (i = 0; i < ARRAY_SIZE(vc4_reg_defs); i++) {
-		seq_printf(m, "%s (0x%04x): 0x%08x\n",
-			   vc4_reg_defs[i].name, vc4_reg_defs[i].reg,
-			   V3D_READ(vc4_reg_defs[i].reg));
-	}
+	drm_print_regset32(&p, &vc4->v3d->regset);
 
 	return 0;
 }
@@ -354,6 +346,9 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
 	v3d->regs = vc4_ioremap_regs(pdev, 0);
 	if (IS_ERR(v3d->regs))
 		return PTR_ERR(v3d->regs);
+	v3d->regset.base = v3d->regs;
+	v3d->regset.regs = v3d_regs;
+	v3d->regset.nregs = ARRAY_SIZE(v3d_regs);
 
 	vc4->v3d = v3d;
 	v3d->vc4 = vc4;
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index 858c3a483229..83227d2440aa 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -176,6 +176,8 @@ struct vc4_vec {
 	struct clk *clock;
 
 	const struct vc4_vec_tv_mode *tv_mode;
+
+	struct debugfs_regset32 regset;
 };
 
 #define VEC_READ(offset) readl(vec->regs + (offset))
@@ -223,35 +225,31 @@ struct vc4_vec_tv_mode {
 	void (*mode_set)(struct vc4_vec *vec);
 };
 
-#define VEC_REG(reg) { reg, #reg }
-static const struct {
-	u32 reg;
-	const char *name;
-} vec_regs[] = {
-	VEC_REG(VEC_WSE_CONTROL),
-	VEC_REG(VEC_WSE_WSS_DATA),
-	VEC_REG(VEC_WSE_VPS_DATA1),
-	VEC_REG(VEC_WSE_VPS_CONTROL),
-	VEC_REG(VEC_REVID),
-	VEC_REG(VEC_CONFIG0),
-	VEC_REG(VEC_SCHPH),
-	VEC_REG(VEC_CLMP0_START),
-	VEC_REG(VEC_CLMP0_END),
-	VEC_REG(VEC_FREQ3_2),
-	VEC_REG(VEC_FREQ1_0),
-	VEC_REG(VEC_CONFIG1),
-	VEC_REG(VEC_CONFIG2),
-	VEC_REG(VEC_INTERRUPT_CONTROL),
-	VEC_REG(VEC_INTERRUPT_STATUS),
-	VEC_REG(VEC_FCW_SECAM_B),
-	VEC_REG(VEC_SECAM_GAIN_VAL),
-	VEC_REG(VEC_CONFIG3),
-	VEC_REG(VEC_STATUS0),
-	VEC_REG(VEC_MASK0),
-	VEC_REG(VEC_CFG),
-	VEC_REG(VEC_DAC_TEST),
-	VEC_REG(VEC_DAC_CONFIG),
-	VEC_REG(VEC_DAC_MISC),
+static const struct debugfs_reg32 vec_regs[] = {
+	VC4_REG32(VEC_WSE_CONTROL),
+	VC4_REG32(VEC_WSE_WSS_DATA),
+	VC4_REG32(VEC_WSE_VPS_DATA1),
+	VC4_REG32(VEC_WSE_VPS_CONTROL),
+	VC4_REG32(VEC_REVID),
+	VC4_REG32(VEC_CONFIG0),
+	VC4_REG32(VEC_SCHPH),
+	VC4_REG32(VEC_CLMP0_START),
+	VC4_REG32(VEC_CLMP0_END),
+	VC4_REG32(VEC_FREQ3_2),
+	VC4_REG32(VEC_FREQ1_0),
+	VC4_REG32(VEC_CONFIG1),
+	VC4_REG32(VEC_CONFIG2),
+	VC4_REG32(VEC_INTERRUPT_CONTROL),
+	VC4_REG32(VEC_INTERRUPT_STATUS),
+	VC4_REG32(VEC_FCW_SECAM_B),
+	VC4_REG32(VEC_SECAM_GAIN_VAL),
+	VC4_REG32(VEC_CONFIG3),
+	VC4_REG32(VEC_STATUS0),
+	VC4_REG32(VEC_MASK0),
+	VC4_REG32(VEC_CFG),
+	VC4_REG32(VEC_DAC_TEST),
+	VC4_REG32(VEC_DAC_CONFIG),
+	VC4_REG32(VEC_DAC_MISC),
 };
 
 #ifdef CONFIG_DEBUG_FS
@@ -261,16 +259,12 @@ int vc4_vec_debugfs_regs(struct seq_file *m, void *unused)
 	struct drm_device *dev = node->minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct vc4_vec *vec = vc4->vec;
-	int i;
+	struct drm_printer p = drm_seq_file_printer(m);
 
 	if (!vec)
 		return 0;
 
-	for (i = 0; i < ARRAY_SIZE(vec_regs); i++) {
-		seq_printf(m, "%s (0x%04x): 0x%08x\n",
-			   vec_regs[i].name, vec_regs[i].reg,
-			   VEC_READ(vec_regs[i].reg));
-	}
+	drm_print_regset32(&p, &vec->regset);
 
 	return 0;
 }
@@ -587,6 +581,9 @@ static int vc4_vec_bind(struct device *dev, struct device *master, void *data)
 	vec->regs = vc4_ioremap_regs(pdev, 0);
 	if (IS_ERR(vec->regs))
 		return PTR_ERR(vec->regs);
+	vec->regset.base = vec->regs;
+	vec->regset.regs = vec_regs;
+	vec->regset.nregs = ARRAY_SIZE(vec_regs);
 
 	vec->clock = devm_clk_get(dev, NULL);
 	if (IS_ERR(vec->clock)) {
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 3/7] drm/vc4: Use common helpers for debugfs setup by the driver components.
  2019-02-20 21:03 [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Eric Anholt
  2019-02-20 21:03 ` [PATCH 2/7] drm/vc4: Use drm_print_regset32() for our debug register dumping Eric Anholt
@ 2019-02-20 21:03 ` Eric Anholt
  2019-02-20 21:03 ` [PATCH 4/7] drm/vc4: Use drm_printer for the debugfs and runtime bo stats output Eric Anholt
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Eric Anholt @ 2019-02-20 21:03 UTC (permalink / raw)
  To: dri-devel; +Cc: Paul Kocialkowski, Maxime Ripard, linux-kernel

The global list of all debugfs entries for the driver was painful: the
list couldn't see into the components' structs, so each component had
its own debugs show function to find the component, then find the
regset and dump it.  The components also had to be careful to check
that they were actually registered in vc4 before dereferencing
themselves, in case they weren't probed on a particular platform.
They routinely failed at that.

Instead, we can have the components add their debugfs callbacks to a
little list in vc4 to be registered at drm_dev_register() time, which
gets vc4_debugfs.c out of the business of knowing the whole list of
components.

Thanks to this change, dsi0 (if it existed) would register its node.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_bo.c      |  6 +--
 drivers/gpu/drm/vc4/vc4_crtc.c    | 33 +++---------
 drivers/gpu/drm/vc4/vc4_debugfs.c | 84 ++++++++++++++++++++++++-------
 drivers/gpu/drm/vc4/vc4_dpi.c     | 20 +-------
 drivers/gpu/drm/vc4/vc4_drv.c     |  1 +
 drivers/gpu/drm/vc4/vc4_drv.h     | 38 ++++++++++----
 drivers/gpu/drm/vc4/vc4_dsi.c     | 24 ++-------
 drivers/gpu/drm/vc4/vc4_hdmi.c    |  6 +--
 drivers/gpu/drm/vc4/vc4_hvs.c     | 16 +-----
 drivers/gpu/drm/vc4/vc4_txp.c     | 20 +-------
 drivers/gpu/drm/vc4/vc4_v3d.c     | 19 ++-----
 drivers/gpu/drm/vc4/vc4_vec.c     | 20 +-------
 12 files changed, 125 insertions(+), 162 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 92e3f98d8478..8c509d560bf0 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -66,8 +66,7 @@ static void vc4_bo_stats_dump(struct vc4_dev *vc4)
 	mutex_unlock(&vc4->purgeable.lock);
 }
 
-#ifdef CONFIG_DEBUG_FS
-int vc4_bo_stats_debugfs(struct seq_file *m, void *unused)
+static int vc4_bo_stats_debugfs(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *dev = node->minor->dev;
@@ -99,7 +98,6 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *unused)
 
 	return 0;
 }
-#endif
 
 /* Takes ownership of *name and returns the appropriate slot for it in
  * the bo_labels[] array, extending it as necessary.
@@ -1025,6 +1023,8 @@ int vc4_bo_cache_init(struct drm_device *dev)
 
 	mutex_init(&vc4->bo_lock);
 
+	vc4_debugfs_add_file(dev, "bo_stats", vc4_bo_stats_debugfs, NULL);
+
 	INIT_LIST_HEAD(&vc4->bo_cache.time_list);
 
 	INIT_WORK(&vc4->bo_cache.time_work, vc4_bo_cache_time_work);
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 6d7072026056..9a62f12aca54 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -84,33 +84,6 @@ static const struct debugfs_reg32 crtc_regs[] = {
 	VC4_REG32(PV_HACT_ACT),
 };
 
-#ifdef CONFIG_DEBUG_FS
-int vc4_crtc_debugfs_regs(struct seq_file *m, void *unused)
-{
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
-	int crtc_index = (uintptr_t)node->info_ent->data;
-	struct drm_printer p = drm_seq_file_printer(m);
-	struct drm_crtc *crtc;
-	struct vc4_crtc *vc4_crtc;
-	int i;
-
-	i = 0;
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-		if (i == crtc_index)
-			break;
-		i++;
-	}
-	if (!crtc)
-		return 0;
-	vc4_crtc = to_vc4_crtc(crtc);
-
-	drm_print_regset32(&p, &vc4_crtc->regset);
-
-	return 0;
-}
-#endif
-
 bool vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
 			     bool in_vblank_irq, int *vpos, int *hpos,
 			     ktime_t *stime, ktime_t *etime,
@@ -1062,6 +1035,7 @@ static const struct drm_crtc_helper_funcs vc4_crtc_helper_funcs = {
 
 static const struct vc4_crtc_data pv0_data = {
 	.hvs_channel = 0,
+	.debugfs_name = "crtc0_regs",
 	.encoder_types = {
 		[PV_CONTROL_CLK_SELECT_DSI] = VC4_ENCODER_TYPE_DSI0,
 		[PV_CONTROL_CLK_SELECT_DPI_SMI_HDMI] = VC4_ENCODER_TYPE_DPI,
@@ -1070,6 +1044,7 @@ static const struct vc4_crtc_data pv0_data = {
 
 static const struct vc4_crtc_data pv1_data = {
 	.hvs_channel = 2,
+	.debugfs_name = "crtc1_regs",
 	.encoder_types = {
 		[PV_CONTROL_CLK_SELECT_DSI] = VC4_ENCODER_TYPE_DSI1,
 		[PV_CONTROL_CLK_SELECT_DPI_SMI_HDMI] = VC4_ENCODER_TYPE_SMI,
@@ -1078,6 +1053,7 @@ static const struct vc4_crtc_data pv1_data = {
 
 static const struct vc4_crtc_data pv2_data = {
 	.hvs_channel = 1,
+	.debugfs_name = "crtc2_regs",
 	.encoder_types = {
 		[PV_CONTROL_CLK_SELECT_DPI_SMI_HDMI] = VC4_ENCODER_TYPE_HDMI,
 		[PV_CONTROL_CLK_SELECT_VEC] = VC4_ENCODER_TYPE_VEC,
@@ -1239,6 +1215,9 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data)
 
 	platform_set_drvdata(pdev, vc4_crtc);
 
+	vc4_debugfs_add_regset32(drm, vc4_crtc->data->debugfs_name,
+				 &vc4_crtc->regset);
+
 	return 0;
 
 err_destroy_planes:
diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c
index 7a0003de71ab..ead7271b4363 100644
--- a/drivers/gpu/drm/vc4/vc4_debugfs.c
+++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
@@ -15,26 +15,76 @@
 #include "vc4_drv.h"
 #include "vc4_regs.h"
 
-static const struct drm_info_list vc4_debugfs_list[] = {
-	{"bo_stats", vc4_bo_stats_debugfs, 0},
-	{"dpi_regs", vc4_dpi_debugfs_regs, 0},
-	{"dsi1_regs", vc4_dsi_debugfs_regs, 0, (void *)(uintptr_t)1},
-	{"hdmi_regs", vc4_hdmi_debugfs_regs, 0},
-	{"vec_regs", vc4_vec_debugfs_regs, 0},
-	{"txp_regs", vc4_txp_debugfs_regs, 0},
-	{"hvs_regs", vc4_hvs_debugfs_regs, 0},
-	{"crtc0_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)0},
-	{"crtc1_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)1},
-	{"crtc2_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)2},
-	{"v3d_ident", vc4_v3d_debugfs_ident, 0},
-	{"v3d_regs", vc4_v3d_debugfs_regs, 0},
+struct vc4_debugfs_info_entry {
+	struct list_head link;
+	struct drm_info_list info;
 };
 
-#define VC4_DEBUGFS_ENTRIES ARRAY_SIZE(vc4_debugfs_list)
-
+/**
+ * Called at drm_dev_register() time on each of the minors registered
+ * by the DRM device, to attach the debugfs files.
+ */
 int
 vc4_debugfs_init(struct drm_minor *minor)
 {
-	return drm_debugfs_create_files(vc4_debugfs_list, VC4_DEBUGFS_ENTRIES,
-					minor->debugfs_root, minor);
+	struct drm_device *dev = minor->dev;
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	struct vc4_debugfs_info_entry *entry;
+
+	list_for_each_entry(entry, &vc4->debugfs_list, link) {
+		int ret = drm_debugfs_create_files(&entry->info, 1,
+						   minor->debugfs_root, minor);
+
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+int vc4_debugfs_regset32(struct seq_file *m, void *unused)
+{
+	struct drm_info_node *node = (struct drm_info_node *)m->private;
+	struct debugfs_regset32 *regset = node->info_ent->data;
+	struct drm_printer p = drm_seq_file_printer(m);
+
+	drm_print_regset32(&p, regset);
+
+	return 0;
+}
+
+/**
+ * Registers a debugfs file with a callback function for a vc4 component.
+ *
+ * This is like drm_debugfs_create_files(), but that can only be
+ * called a given DRM minor, while the various VC4 components want to
+ * register their debugfs files during the component bind process.  We
+ * track the request and delay it to be called on each minor during
+ * vc4_debugfs_init().
+ */
+void vc4_debugfs_add_file(struct drm_device *dev,
+			  const char *name,
+			  int (*show)(struct seq_file*, void*),
+			  void *data)
+{
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+
+	struct vc4_debugfs_info_entry *entry =
+		devm_kzalloc(dev->dev, sizeof(*entry), GFP_KERNEL);
+
+	if (!entry)
+		return;
+
+	entry->info.name = name;
+	entry->info.show = show;
+	entry->info.data = data;
+
+	list_add(&entry->link, &vc4->debugfs_list);
+}
+
+void vc4_debugfs_add_regset32(struct drm_device *drm,
+			      const char *name,
+			      struct debugfs_regset32 *regset)
+{
+	vc4_debugfs_add_file(drm, name, vc4_debugfs_regset32, regset);
 }
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 8be2264f496a..34f90ca8f479 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -125,24 +125,6 @@ static const struct debugfs_reg32 dpi_regs[] = {
 	VC4_REG32(DPI_ID),
 };
 
-#ifdef CONFIG_DEBUG_FS
-int vc4_dpi_debugfs_regs(struct seq_file *m, void *unused)
-{
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
-	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	struct vc4_dpi *dpi = vc4->dpi;
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	if (!dpi)
-		return 0;
-
-	drm_print_regset32(&p, &dpi->regset);
-
-	return 0;
-}
-#endif
-
 static const struct drm_encoder_funcs vc4_dpi_encoder_funcs = {
 	.destroy = drm_encoder_cleanup,
 };
@@ -349,6 +331,8 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data)
 
 	vc4->dpi = dpi;
 
+	vc4_debugfs_add_regset32(drm, "dpi_regs", &dpi->regset);
+
 	return 0;
 
 err_destroy_encoder:
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 52576dee809e..1927d1b756f5 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -265,6 +265,7 @@ static int vc4_drm_bind(struct device *dev)
 	platform_set_drvdata(pdev, drm);
 	vc4->dev = drm;
 	drm->dev_private = vc4;
+	INIT_LIST_HEAD(&vc4->debugfs_list);
 
 	ret = vc4_bo_cache_init(drm);
 	if (ret)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 511ad884d9c3..495c5a13a948 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -200,6 +200,11 @@ struct vc4_dev {
 
 	struct drm_modeset_lock ctm_state_lock;
 	struct drm_private_obj ctm_manager;
+
+	/* List of vc4_debugfs_info_entry for adding to debugfs once
+	 * the minor is available (after drm_dev_register()).
+	 */
+	struct list_head debugfs_list;
 };
 
 static inline struct vc4_dev *
@@ -408,6 +413,7 @@ struct vc4_crtc_data {
 	int hvs_channel;
 
 	enum vc4_encoder_type encoder_types[4];
+	const char *debugfs_name;
 };
 
 struct vc4_crtc {
@@ -694,7 +700,6 @@ struct drm_gem_object *vc4_prime_import_sg_table(struct drm_device *dev,
 void *vc4_prime_vmap(struct drm_gem_object *obj);
 int vc4_bo_cache_init(struct drm_device *dev);
 void vc4_bo_cache_destroy(struct drm_device *dev);
-int vc4_bo_stats_debugfs(struct seq_file *m, void *arg);
 int vc4_bo_inc_usecnt(struct vc4_bo *bo);
 void vc4_bo_dec_usecnt(struct vc4_bo *bo);
 void vc4_bo_add_to_purgeable_pool(struct vc4_bo *bo);
@@ -702,7 +707,6 @@ void vc4_bo_remove_from_purgeable_pool(struct vc4_bo *bo);
 
 /* vc4_crtc.c */
 extern struct platform_driver vc4_crtc_driver;
-int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg);
 bool vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
 			     bool in_vblank_irq, int *vpos, int *hpos,
 			     ktime_t *stime, ktime_t *etime,
@@ -715,17 +719,37 @@ void vc4_crtc_get_margins(struct drm_crtc_state *state,
 
 /* vc4_debugfs.c */
 int vc4_debugfs_init(struct drm_minor *minor);
+#ifdef CONFIG_DEBUG_FS
+void vc4_debugfs_add_file(struct drm_device *drm,
+			  const char *filename,
+			  int (*show)(struct seq_file*, void*),
+			  void *data);
+void vc4_debugfs_add_regset32(struct drm_device *drm,
+			      const char *filename,
+			      struct debugfs_regset32 *regset);
+#else
+static inline void vc4_debugfs_add_file(struct drm_device *drm,
+					const char *filename,
+					int (*show)(struct seq_file*, void*),
+					void *data)
+{
+}
+
+static inline void vc4_debugfs_add_regset32(struct drm_device *drm,
+					    const char *filename,
+					    struct debugfs_regset32 *regset)
+{
+}
+#endif
 
 /* vc4_drv.c */
 void __iomem *vc4_ioremap_regs(struct platform_device *dev, int index);
 
 /* vc4_dpi.c */
 extern struct platform_driver vc4_dpi_driver;
-int vc4_dpi_debugfs_regs(struct seq_file *m, void *unused);
 
 /* vc4_dsi.c */
 extern struct platform_driver vc4_dsi_driver;
-int vc4_dsi_debugfs_regs(struct seq_file *m, void *unused);
 
 /* vc4_fence.c */
 extern const struct dma_fence_ops vc4_fence_ops;
@@ -753,15 +777,12 @@ int vc4_gem_madvise_ioctl(struct drm_device *dev, void *data,
 
 /* vc4_hdmi.c */
 extern struct platform_driver vc4_hdmi_driver;
-int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused);
 
 /* vc4_vec.c */
 extern struct platform_driver vc4_vec_driver;
-int vc4_vec_debugfs_regs(struct seq_file *m, void *unused);
 
 /* vc4_txp.c */
 extern struct platform_driver vc4_txp_driver;
-int vc4_txp_debugfs_regs(struct seq_file *m, void *unused);
 
 /* vc4_irq.c */
 irqreturn_t vc4_irq(int irq, void *arg);
@@ -773,7 +794,6 @@ void vc4_irq_reset(struct drm_device *dev);
 /* vc4_hvs.c */
 extern struct platform_driver vc4_hvs_driver;
 void vc4_hvs_dump_state(struct drm_device *dev);
-int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused);
 
 /* vc4_kms.c */
 int vc4_kms_load(struct drm_device *dev);
@@ -788,8 +808,6 @@ void vc4_plane_async_set_fb(struct drm_plane *plane,
 
 /* vc4_v3d.c */
 extern struct platform_driver vc4_v3d_driver;
-int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused);
-int vc4_v3d_debugfs_regs(struct seq_file *m, void *unused);
 int vc4_v3d_get_bin_slot(struct vc4_dev *vc4);
 
 /* vc4_validate.c */
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 806cfaa2a6a7..9412709067f5 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -657,25 +657,6 @@ static const struct debugfs_reg32 dsi1_regs[] = {
 	VC4_REG32(DSI1_ID),
 };
 
-#ifdef CONFIG_DEBUG_FS
-int vc4_dsi_debugfs_regs(struct seq_file *m, void *unused)
-{
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *drm = node->minor->dev;
-	struct vc4_dev *vc4 = to_vc4_dev(drm);
-	int dsi_index = (uintptr_t)node->info_ent->data;
-	struct vc4_dsi *dsi = (dsi_index == 1 ? vc4->dsi1 : NULL);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	if (!dsi)
-		return 0;
-
-	drm_print_regset32(&p, &dsi->regset);
-
-	return 0;
-}
-#endif
-
 static void vc4_dsi_encoder_destroy(struct drm_encoder *encoder)
 {
 	drm_encoder_cleanup(encoder);
@@ -1637,6 +1618,11 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
 	 */
 	dsi->encoder->bridge = NULL;
 
+	if (dsi->port == 0)
+		vc4_debugfs_add_regset32(drm, "dsi0_regs", &dsi->regset);
+	else
+		vc4_debugfs_add_regset32(drm, "dsi1_regs", &dsi->regset);
+
 	pm_runtime_enable(dev);
 
 	return 0;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 38c9172cfe52..99fc8569e0f5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -187,8 +187,7 @@ static const struct debugfs_reg32 hd_regs[] = {
 	VC4_REG32(VC4_HD_FRAME_COUNT),
 };
 
-#ifdef CONFIG_DEBUG_FS
-int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
+static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *dev = node->minor->dev;
@@ -201,7 +200,6 @@ int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
 
 	return 0;
 }
-#endif /* CONFIG_DEBUG_FS */
 
 static enum drm_connector_status
 vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
@@ -1432,6 +1430,8 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 	if (ret)
 		goto err_destroy_encoder;
 
+	vc4_debugfs_add_file(drm, "hdmi_regs", vc4_hdmi_debugfs_regs, hdmi);
+
 	return 0;
 
 #ifdef CONFIG_DRM_VC4_HDMI_CEC
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 45405f76cd40..7a8d71fbf04d 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -79,20 +79,6 @@ void vc4_hvs_dump_state(struct drm_device *dev)
 	}
 }
 
-#ifdef CONFIG_DEBUG_FS
-int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused)
-{
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
-	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	drm_print_regset32(&p, &vc4->hvs->regset);
-
-	return 0;
-}
-#endif
-
 /* The filter kernel is composed of dwords each containing 3 9-bit
  * signed integers packed next to each other.
  */
@@ -221,6 +207,8 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 
 	HVS_WRITE(SCALER_DISPCTRL, dispctrl);
 
+	vc4_debugfs_add_regset32(drm, "hvs_regs", &hvs->regset);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index c8f80064c179..1220f4a5aac4 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -169,24 +169,6 @@ static const struct debugfs_reg32 txp_regs[] = {
 	VC4_REG32(TXP_PROGRESS),
 };
 
-#ifdef CONFIG_DEBUG_FS
-int vc4_txp_debugfs_regs(struct seq_file *m, void *unused)
-{
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
-	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	struct vc4_txp *txp = vc4->txp;
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	if (!txp)
-		return 0;
-
-	drm_print_regset32(&p, &txp->regset);
-
-	return 0;
-}
-#endif
-
 static int vc4_txp_connector_get_modes(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
@@ -424,6 +406,8 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data)
 	dev_set_drvdata(dev, txp);
 	vc4->txp = txp;
 
+	vc4_debugfs_add_regset32(drm, "txp_regs", &txp->regset);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index e1e728e95562..f008584eb739 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -22,7 +22,6 @@
 #include "vc4_drv.h"
 #include "vc4_regs.h"
 
-#ifdef CONFIG_DEBUG_FS
 static const struct debugfs_reg32 v3d_regs[] = {
 	VC4_REG32(V3D_IDENT0),
 	VC4_REG32(V3D_IDENT1),
@@ -104,19 +103,7 @@ static const struct debugfs_reg32 v3d_regs[] = {
 	VC4_REG32(V3D_ERRSTAT),
 };
 
-int vc4_v3d_debugfs_regs(struct seq_file *m, void *unused)
-{
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
-	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	drm_print_regset32(&p, &vc4->v3d->regset);
-
-	return 0;
-}
-
-int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
+static int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *dev = node->minor->dev;
@@ -136,7 +123,6 @@ int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
 
 	return 0;
 }
-#endif /* CONFIG_DEBUG_FS */
 
 static void vc4_v3d_init_hw(struct drm_device *dev)
 {
@@ -404,6 +390,9 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
 	pm_runtime_set_autosuspend_delay(dev, 40); /* a little over 2 frames. */
 	pm_runtime_enable(dev);
 
+	vc4_debugfs_add_file(drm, "v3d_ident", vc4_v3d_debugfs_ident, NULL);
+	vc4_debugfs_add_regset32(drm, "v3d_regs", &v3d->regset);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index 83227d2440aa..0a27e48fab31 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -252,24 +252,6 @@ static const struct debugfs_reg32 vec_regs[] = {
 	VC4_REG32(VEC_DAC_MISC),
 };
 
-#ifdef CONFIG_DEBUG_FS
-int vc4_vec_debugfs_regs(struct seq_file *m, void *unused)
-{
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
-	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	struct vc4_vec *vec = vc4->vec;
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	if (!vec)
-		return 0;
-
-	drm_print_regset32(&p, &vec->regset);
-
-	return 0;
-}
-#endif
-
 static void vc4_vec_ntsc_mode_set(struct vc4_vec *vec)
 {
 	VEC_WRITE(VEC_CONFIG0, VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN);
@@ -609,6 +591,8 @@ static int vc4_vec_bind(struct device *dev, struct device *master, void *data)
 
 	vc4->vec = vec;
 
+	vc4_debugfs_add_regset32(drm, "vec_regs", &vec->regset);
+
 	return 0;
 
 err_destroy_encoder:
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 4/7] drm/vc4: Use drm_printer for the debugfs and runtime bo stats output.
  2019-02-20 21:03 [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Eric Anholt
  2019-02-20 21:03 ` [PATCH 2/7] drm/vc4: Use drm_print_regset32() for our debug register dumping Eric Anholt
  2019-02-20 21:03 ` [PATCH 3/7] drm/vc4: Use common helpers for debugfs setup by the driver components Eric Anholt
@ 2019-02-20 21:03 ` Eric Anholt
  2019-03-22 10:41   ` Paul Kocialkowski
  2019-02-20 21:03 ` [PATCH 5/7] drm/vc4: Disable V3D interactions if the v3d component didn't probe Eric Anholt
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Eric Anholt @ 2019-02-20 21:03 UTC (permalink / raw)
  To: dri-devel; +Cc: Paul Kocialkowski, Maxime Ripard, linux-kernel

Now I can extend the stats without more copy and pasting between the
two.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_bo.c | 48 +++++++++++-------------------------
 1 file changed, 14 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 8c509d560bf0..88ebd681d7eb 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -40,7 +40,7 @@ static bool is_user_label(int label)
 	return label >= VC4_BO_TYPE_COUNT;
 }
 
-static void vc4_bo_stats_dump(struct vc4_dev *vc4)
+static void vc4_bo_stats_print(struct drm_printer *p, struct vc4_dev *vc4)
 {
 	int i;
 
@@ -48,21 +48,21 @@ static void vc4_bo_stats_dump(struct vc4_dev *vc4)
 		if (!vc4->bo_labels[i].num_allocated)
 			continue;
 
-		DRM_INFO("%30s: %6dkb BOs (%d)\n",
-			 vc4->bo_labels[i].name,
-			 vc4->bo_labels[i].size_allocated / 1024,
-			 vc4->bo_labels[i].num_allocated);
+		drm_printf(p, "%30s: %6dkb BOs (%d)\n",
+			   vc4->bo_labels[i].name,
+			   vc4->bo_labels[i].size_allocated / 1024,
+			   vc4->bo_labels[i].num_allocated);
 	}
 
 	mutex_lock(&vc4->purgeable.lock);
 	if (vc4->purgeable.num)
-		DRM_INFO("%30s: %6zdkb BOs (%d)\n", "userspace BO cache",
-			 vc4->purgeable.size / 1024, vc4->purgeable.num);
+		drm_printf(p, "%30s: %6zdkb BOs (%d)\n", "userspace BO cache",
+			   vc4->purgeable.size / 1024, vc4->purgeable.num);
 
 	if (vc4->purgeable.purged_num)
-		DRM_INFO("%30s: %6zdkb BOs (%d)\n", "total purged BO",
-			 vc4->purgeable.purged_size / 1024,
-			 vc4->purgeable.purged_num);
+		drm_printf(p, "%30s: %6zdkb BOs (%d)\n", "total purged BO",
+			   vc4->purgeable.purged_size / 1024,
+			   vc4->purgeable.purged_num);
 	mutex_unlock(&vc4->purgeable.lock);
 }
 
@@ -71,30 +71,9 @@ static int vc4_bo_stats_debugfs(struct seq_file *m, void *unused)
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *dev = node->minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	int i;
-
-	mutex_lock(&vc4->bo_lock);
-	for (i = 0; i < vc4->num_labels; i++) {
-		if (!vc4->bo_labels[i].num_allocated)
-			continue;
-
-		seq_printf(m, "%30s: %6dkb BOs (%d)\n",
-			   vc4->bo_labels[i].name,
-			   vc4->bo_labels[i].size_allocated / 1024,
-			   vc4->bo_labels[i].num_allocated);
-	}
-	mutex_unlock(&vc4->bo_lock);
+	struct drm_printer p = drm_seq_file_printer(m);
 
-	mutex_lock(&vc4->purgeable.lock);
-	if (vc4->purgeable.num)
-		seq_printf(m, "%30s: %6zdkb BOs (%d)\n", "userspace BO cache",
-			   vc4->purgeable.size / 1024, vc4->purgeable.num);
-
-	if (vc4->purgeable.purged_num)
-		seq_printf(m, "%30s: %6zdkb BOs (%d)\n", "total purged BO",
-			   vc4->purgeable.purged_size / 1024,
-			   vc4->purgeable.purged_num);
-	mutex_unlock(&vc4->purgeable.lock);
+	vc4_bo_stats_print(&p, vc4);
 
 	return 0;
 }
@@ -473,8 +452,9 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size,
 	}
 
 	if (IS_ERR(cma_obj)) {
+		struct drm_printer p = drm_info_printer(vc4->dev->dev);
 		DRM_ERROR("Failed to allocate from CMA:\n");
-		vc4_bo_stats_dump(vc4);
+		vc4_bo_stats_print(&p, vc4);
 		return ERR_PTR(-ENOMEM);
 	}
 	bo = to_vc4_bo(&cma_obj->base);
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 5/7] drm/vc4: Disable V3D interactions if the v3d component didn't probe.
  2019-02-20 21:03 [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Eric Anholt
                   ` (2 preceding siblings ...)
  2019-02-20 21:03 ` [PATCH 4/7] drm/vc4: Use drm_printer for the debugfs and runtime bo stats output Eric Anholt
@ 2019-02-20 21:03 ` Eric Anholt
  2019-03-25 13:30   ` Paul Kocialkowski
  2019-02-20 21:03 ` [PATCH 6/7] drm/vc4: Add helpers for pm get/put Eric Anholt
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Eric Anholt @ 2019-02-20 21:03 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-kernel, Paul Kocialkowski, Maxime Ripard, Eric Anholt

One might want to use the VC4 display stack without using Mesa.
Similar to the debugfs fixes for not having all of the possible
display bits enabled, make sure you can't oops in vc4 if v3d isn't
enabled.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_drv.c     | 11 +++++++++++
 drivers/gpu/drm/vc4/vc4_gem.c     | 10 ++++++++++
 drivers/gpu/drm/vc4/vc4_irq.c     |  9 +++++++++
 drivers/gpu/drm/vc4/vc4_perfmon.c | 18 ++++++++++++++++++
 4 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 1927d1b756f5..de1d0ce11831 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -72,6 +72,9 @@ static int vc4_get_param_ioctl(struct drm_device *dev, void *data,
 	if (args->pad != 0)
 		return -EINVAL;
 
+	if (!vc4->v3d)
+		return -EINVAL;
+
 	switch (args->param) {
 	case DRM_VC4_PARAM_V3D_IDENT0:
 		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
@@ -251,6 +254,7 @@ static int vc4_drm_bind(struct device *dev)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct drm_device *drm;
 	struct vc4_dev *vc4;
+	struct device_node *node;
 	int ret = 0;
 
 	dev->coherent_dma_mask = DMA_BIT_MASK(32);
@@ -259,6 +263,13 @@ static int vc4_drm_bind(struct device *dev)
 	if (!vc4)
 		return -ENOMEM;
 
+	/* If VC4 V3D is missing, don't advertise render nodes. */
+	node = of_find_compatible_node(NULL, NULL, "brcm,bcm2835-v3d");
+	if (node)
+		of_node_put(node);
+	else
+		vc4_drm_driver.driver_features &= ~DRIVER_RENDER;
+
 	drm = drm_dev_alloc(&vc4_drm_driver, dev);
 	if (IS_ERR(drm))
 		return PTR_ERR(drm);
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 5ee5bf7fedf7..8d816e5ed762 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -74,6 +74,11 @@ vc4_get_hang_state_ioctl(struct drm_device *dev, void *data,
 	u32 i;
 	int ret = 0;
 
+	if (!vc4->v3d) {
+		DRM_DEBUG("VC4_GET_HANG_STATE with no VC4 V3D probed\n");
+		return -EINVAL;
+	}
+
 	spin_lock_irqsave(&vc4->job_lock, irqflags);
 	kernel_state = vc4->hang_state;
 	if (!kernel_state) {
@@ -1124,6 +1129,11 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
 	struct dma_fence *in_fence;
 	int ret = 0;
 
+	if (!vc4->v3d) {
+		DRM_DEBUG("VC4_SUBMIT_CL with no VC4 V3D probed\n");
+		return -EINVAL;
+	}
+
 	if ((args->flags & ~(VC4_SUBMIT_CL_USE_CLEAR_COLOR |
 			     VC4_SUBMIT_CL_FIXED_RCL_ORDER |
 			     VC4_SUBMIT_CL_RCL_ORDER_INCREASING_X |
diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
index 4cd2ccfe15f4..ffd0a4388752 100644
--- a/drivers/gpu/drm/vc4/vc4_irq.c
+++ b/drivers/gpu/drm/vc4/vc4_irq.c
@@ -229,6 +229,9 @@ vc4_irq_preinstall(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 
+	if (!vc4->v3d)
+		return;
+
 	init_waitqueue_head(&vc4->job_wait_queue);
 	INIT_WORK(&vc4->overflow_mem_work, vc4_overflow_mem_work);
 
@@ -243,6 +246,9 @@ vc4_irq_postinstall(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 
+	if (!vc4->v3d)
+		return 0;
+
 	/* Enable both the render done and out of memory interrupts. */
 	V3D_WRITE(V3D_INTENA, V3D_DRIVER_IRQS);
 
@@ -254,6 +260,9 @@ vc4_irq_uninstall(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 
+	if (!vc4->v3d)
+		return;
+
 	/* Disable sending interrupts for our driver's IRQs. */
 	V3D_WRITE(V3D_INTDIS, V3D_DRIVER_IRQS);
 
diff --git a/drivers/gpu/drm/vc4/vc4_perfmon.c b/drivers/gpu/drm/vc4/vc4_perfmon.c
index 495150415020..6562d3d30b20 100644
--- a/drivers/gpu/drm/vc4/vc4_perfmon.c
+++ b/drivers/gpu/drm/vc4/vc4_perfmon.c
@@ -100,12 +100,18 @@ void vc4_perfmon_close_file(struct vc4_file *vc4file)
 int vc4_perfmon_create_ioctl(struct drm_device *dev, void *data,
 			     struct drm_file *file_priv)
 {
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct vc4_file *vc4file = file_priv->driver_priv;
 	struct drm_vc4_perfmon_create *req = data;
 	struct vc4_perfmon *perfmon;
 	unsigned int i;
 	int ret;
 
+	if (!vc4->v3d) {
+		DRM_DEBUG("Creating perfmon no VC4 V3D probed\n");
+		return -EINVAL;
+	}
+
 	/* Number of monitored counters cannot exceed HW limits. */
 	if (req->ncounters > DRM_VC4_MAX_PERF_COUNTERS ||
 	    !req->ncounters)
@@ -146,10 +152,16 @@ int vc4_perfmon_create_ioctl(struct drm_device *dev, void *data,
 int vc4_perfmon_destroy_ioctl(struct drm_device *dev, void *data,
 			      struct drm_file *file_priv)
 {
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct vc4_file *vc4file = file_priv->driver_priv;
 	struct drm_vc4_perfmon_destroy *req = data;
 	struct vc4_perfmon *perfmon;
 
+	if (!vc4->v3d) {
+		DRM_DEBUG("Destroying perfmon no VC4 V3D probed\n");
+		return -EINVAL;
+	}
+
 	mutex_lock(&vc4file->perfmon.lock);
 	perfmon = idr_remove(&vc4file->perfmon.idr, req->id);
 	mutex_unlock(&vc4file->perfmon.lock);
@@ -164,11 +176,17 @@ int vc4_perfmon_destroy_ioctl(struct drm_device *dev, void *data,
 int vc4_perfmon_get_values_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv)
 {
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct vc4_file *vc4file = file_priv->driver_priv;
 	struct drm_vc4_perfmon_get_values *req = data;
 	struct vc4_perfmon *perfmon;
 	int ret;
 
+	if (!vc4->v3d) {
+		DRM_DEBUG("Getting perfmon no VC4 V3D probed\n");
+		return -EINVAL;
+	}
+
 	mutex_lock(&vc4file->perfmon.lock);
 	perfmon = idr_find(&vc4file->perfmon.idr, req->id);
 	vc4_perfmon_get(perfmon);
-- 
2.20.1

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

* [PATCH 6/7] drm/vc4: Add helpers for pm get/put.
  2019-02-20 21:03 [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Eric Anholt
                   ` (3 preceding siblings ...)
  2019-02-20 21:03 ` [PATCH 5/7] drm/vc4: Disable V3D interactions if the v3d component didn't probe Eric Anholt
@ 2019-02-20 21:03 ` Eric Anholt
  2019-03-22 10:48   ` Paul Kocialkowski
  2019-02-20 21:03 ` [PATCH 7/7] drm/vc4: Make sure that the v3d ident debugfs has vc4's power on Eric Anholt
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Eric Anholt @ 2019-02-20 21:03 UTC (permalink / raw)
  To: dri-devel; +Cc: Paul Kocialkowski, Maxime Ripard, linux-kernel

This makes sure the vc4_reset doesn't hit an obscure race with the
GET_PARAM ioctl, fixes a decrement outside of the lock, and prevents
future code from making mistakes with the weird return value of
pm_runtime_get_sync().

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 21 +++++++++------------
 drivers/gpu/drm/vc4/vc4_drv.h |  2 ++
 drivers/gpu/drm/vc4/vc4_gem.c | 21 +++++----------------
 drivers/gpu/drm/vc4/vc4_v3d.c | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index de1d0ce11831..f95891a61d52 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -77,28 +77,25 @@ static int vc4_get_param_ioctl(struct drm_device *dev, void *data,
 
 	switch (args->param) {
 	case DRM_VC4_PARAM_V3D_IDENT0:
-		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
-		if (ret < 0)
+		ret = vc4_v3d_pm_get(vc4);
+		if (ret)
 			return ret;
 		args->value = V3D_READ(V3D_IDENT0);
-		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
-		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
+		vc4_v3d_pm_put(vc4);
 		break;
 	case DRM_VC4_PARAM_V3D_IDENT1:
-		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
-		if (ret < 0)
+		ret = vc4_v3d_pm_get(vc4);
+		if (ret)
 			return ret;
 		args->value = V3D_READ(V3D_IDENT1);
-		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
-		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
+		vc4_v3d_pm_put(vc4);
 		break;
 	case DRM_VC4_PARAM_V3D_IDENT2:
-		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
-		if (ret < 0)
+		ret = vc4_v3d_pm_get(vc4);
+		if (ret)
 			return ret;
 		args->value = V3D_READ(V3D_IDENT2);
-		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
-		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
+		vc4_v3d_pm_put(vc4);
 		break;
 	case DRM_VC4_PARAM_SUPPORTS_BRANCHES:
 	case DRM_VC4_PARAM_SUPPORTS_ETC1:
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 495c5a13a948..c71988b270bc 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -809,6 +809,8 @@ void vc4_plane_async_set_fb(struct drm_plane *plane,
 /* vc4_v3d.c */
 extern struct platform_driver vc4_v3d_driver;
 int vc4_v3d_get_bin_slot(struct vc4_dev *vc4);
+int vc4_v3d_pm_get(struct vc4_dev *vc4);
+void vc4_v3d_pm_put(struct vc4_dev *vc4);
 
 /* vc4_validate.c */
 int
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 8d816e5ed762..4544a478f106 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -969,12 +969,7 @@ vc4_complete_exec(struct drm_device *dev, struct vc4_exec_info *exec)
 	/* Release the reference we had on the perf monitor. */
 	vc4_perfmon_put(exec->perfmon);
 
-	mutex_lock(&vc4->power_lock);
-	if (--vc4->power_refcount == 0) {
-		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
-		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
-	}
-	mutex_unlock(&vc4->power_lock);
+	vc4_v3d_pm_put(vc4);
 
 	kfree(exec);
 }
@@ -1153,17 +1148,11 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
 		return -ENOMEM;
 	}
 
-	mutex_lock(&vc4->power_lock);
-	if (vc4->power_refcount++ == 0) {
-		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
-		if (ret < 0) {
-			mutex_unlock(&vc4->power_lock);
-			vc4->power_refcount--;
-			kfree(exec);
-			return ret;
-		}
+	ret = vc4_v3d_pm_get(vc4);
+	if (ret) {
+		kfree(exec);
+		return ret;
 	}
-	mutex_unlock(&vc4->power_lock);
 
 	exec->args = args;
 	INIT_LIST_HEAD(&exec->unref_list);
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index f008584eb739..7820b8eaaa98 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -124,6 +124,39 @@ static int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
 	return 0;
 }
 
+/**
+ * Wraps pm_runtime_get_sync() in a refcount, so that we can reliably
+ * get the pm_runtime refcount to 0 in vc4_reset().
+ */
+int
+vc4_v3d_pm_get(struct vc4_dev *vc4)
+{
+	mutex_lock(&vc4->power_lock);
+	if (vc4->power_refcount++ == 0) {
+		int ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
+
+		if (ret < 0) {
+			vc4->power_refcount--;
+			mutex_unlock(&vc4->power_lock);
+			return ret;
+		}
+	}
+	mutex_unlock(&vc4->power_lock);
+
+	return 0;
+}
+
+void
+vc4_v3d_pm_put(struct vc4_dev *vc4)
+{
+	mutex_lock(&vc4->power_lock);
+	if (--vc4->power_refcount == 0) {
+		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
+		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
+	}
+	mutex_unlock(&vc4->power_lock);
+}
+
 static void vc4_v3d_init_hw(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 7/7] drm/vc4: Make sure that the v3d ident debugfs has vc4's power on.
  2019-02-20 21:03 [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Eric Anholt
                   ` (4 preceding siblings ...)
  2019-02-20 21:03 ` [PATCH 6/7] drm/vc4: Add helpers for pm get/put Eric Anholt
@ 2019-02-20 21:03 ` Eric Anholt
  2019-03-22 10:43   ` Paul Kocialkowski
  2019-02-21  9:37 ` [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Daniel Vetter
  2019-03-22 10:30 ` Paul Kocialkowski
  7 siblings, 1 reply; 16+ messages in thread
From: Eric Anholt @ 2019-02-20 21:03 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-kernel, Paul Kocialkowski, Maxime Ripard, Eric Anholt

Otherwise, you sometimes decode the ident fields based on 0xdeadbeef
register reads.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_v3d.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 7820b8eaaa98..36e6c7086ecf 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -108,18 +108,23 @@ static int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *dev = node->minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
-	uint32_t ident1 = V3D_READ(V3D_IDENT1);
-	uint32_t nslc = VC4_GET_FIELD(ident1, V3D_IDENT1_NSLC);
-	uint32_t tups = VC4_GET_FIELD(ident1, V3D_IDENT1_TUPS);
-	uint32_t qups = VC4_GET_FIELD(ident1, V3D_IDENT1_QUPS);
-
-	seq_printf(m, "Revision:   %d\n",
-		   VC4_GET_FIELD(ident1, V3D_IDENT1_REV));
-	seq_printf(m, "Slices:     %d\n", nslc);
-	seq_printf(m, "TMUs:       %d\n", nslc * tups);
-	seq_printf(m, "QPUs:       %d\n", nslc * qups);
-	seq_printf(m, "Semaphores: %d\n",
-		   VC4_GET_FIELD(ident1, V3D_IDENT1_NSEM));
+	int ret = vc4_v3d_pm_get(vc4);
+
+	if (ret == 0) {
+		uint32_t ident1 = V3D_READ(V3D_IDENT1);
+		uint32_t nslc = VC4_GET_FIELD(ident1, V3D_IDENT1_NSLC);
+		uint32_t tups = VC4_GET_FIELD(ident1, V3D_IDENT1_TUPS);
+		uint32_t qups = VC4_GET_FIELD(ident1, V3D_IDENT1_QUPS);
+
+		seq_printf(m, "Revision:   %d\n",
+			   VC4_GET_FIELD(ident1, V3D_IDENT1_REV));
+		seq_printf(m, "Slices:     %d\n", nslc);
+		seq_printf(m, "TMUs:       %d\n", nslc * tups);
+		seq_printf(m, "QPUs:       %d\n", nslc * qups);
+		seq_printf(m, "Semaphores: %d\n",
+			   VC4_GET_FIELD(ident1, V3D_IDENT1_NSEM));
+		vc4_v3d_pm_put(vc4);
+	}
 
 	return 0;
 }
-- 
2.20.1

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

* Re: [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32.
  2019-02-20 21:03 [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Eric Anholt
                   ` (5 preceding siblings ...)
  2019-02-20 21:03 ` [PATCH 7/7] drm/vc4: Make sure that the v3d ident debugfs has vc4's power on Eric Anholt
@ 2019-02-21  9:37 ` Daniel Vetter
  2019-03-25 16:43   ` Eric Anholt
  2019-03-22 10:30 ` Paul Kocialkowski
  7 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2019-02-21  9:37 UTC (permalink / raw)
  To: Eric Anholt; +Cc: Paul Kocialkowski, Maxime Ripard, linux-kernel, dri-devel

On Wed, Feb 20, 2019 at 01:03:37PM -0800, Eric Anholt wrote:
> The debugfs_regset32 is nice to use for reducing boilerplate in
> dumping a bunch of regs in debugfs, but we also want to be able to
> print to dmesg them at runtime for driver debugging.  drm_printer lets
> us format debugfs and the printk the same way.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/drm_print.c | 16 ++++++++++++++++
>  include/drm/drm_print.h     |  2 ++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 0e7fc3e7dfb4..5ecc0f04cd0c 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -253,3 +253,19 @@ void drm_err(const char *format, ...)
>  	va_end(args);
>  }
>  EXPORT_SYMBOL(drm_err);
> +

A bit of kerneldoc would be nice. With that:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
> +{
> +	int namelen = 0;
> +	int i;
> +
> +	for (i = 0; i < regset->nregs; i++)
> +		namelen = max(namelen, (int)strlen(regset->regs[i].name));
> +
> +	for (i = 0; i < regset->nregs; i++) {
> +		drm_printf(p, "%*s = 0x%08x\n",
> +			   namelen, regset->regs[i].name,
> +			   readl(regset->base + regset->regs[i].offset));
> +	}
> +}
> +EXPORT_SYMBOL(drm_print_regset32);
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index afbc3beef089..3a4247319e63 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -30,6 +30,7 @@
>  #include <linux/printk.h>
>  #include <linux/seq_file.h>
>  #include <linux/device.h>
> +#include <linux/debugfs.h>
>  
>  /**
>   * DOC: print
> @@ -84,6 +85,7 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
>  __printf(2, 3)
>  void drm_printf(struct drm_printer *p, const char *f, ...);
>  void drm_puts(struct drm_printer *p, const char *str);
> +void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset);
>  
>  __printf(2, 0)
>  /**
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32.
  2019-02-20 21:03 [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Eric Anholt
                   ` (6 preceding siblings ...)
  2019-02-21  9:37 ` [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Daniel Vetter
@ 2019-03-22 10:30 ` Paul Kocialkowski
  2019-04-01 17:52   ` Eric Anholt
  7 siblings, 1 reply; 16+ messages in thread
From: Paul Kocialkowski @ 2019-03-22 10:30 UTC (permalink / raw)
  To: Eric Anholt, dri-devel; +Cc: linux-kernel, Maxime Ripard

Hi,

Le mercredi 20 février 2019 à 13:03 -0800, Eric Anholt a écrit :
> The debugfs_regset32 is nice to use for reducing boilerplate in
> dumping a bunch of regs in debugfs, but we also want to be able to
> print to dmesg them at runtime for driver debugging.  drm_printer lets
> us format debugfs and the printk the same way.

Looks good to me!

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/drm_print.c | 16 ++++++++++++++++
>  include/drm/drm_print.h     |  2 ++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 0e7fc3e7dfb4..5ecc0f04cd0c 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -253,3 +253,19 @@ void drm_err(const char *format, ...)
>  	va_end(args);
>  }
>  EXPORT_SYMBOL(drm_err);
> +
> +void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
> +{
> +	int namelen = 0;
> +	int i;
> +
> +	for (i = 0; i < regset->nregs; i++)
> +		namelen = max(namelen, (int)strlen(regset->regs[i].name));
> +
> +	for (i = 0; i < regset->nregs; i++) {
> +		drm_printf(p, "%*s = 0x%08x\n",
> +			   namelen, regset->regs[i].name,
> +			   readl(regset->base + regset->regs[i].offset));
> +	}
> +}
> +EXPORT_SYMBOL(drm_print_regset32);
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index afbc3beef089..3a4247319e63 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -30,6 +30,7 @@
>  #include <linux/printk.h>
>  #include <linux/seq_file.h>
>  #include <linux/device.h>
> +#include <linux/debugfs.h>
>  
>  /**
>   * DOC: print
> @@ -84,6 +85,7 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
>  __printf(2, 3)
>  void drm_printf(struct drm_printer *p, const char *f, ...);
>  void drm_puts(struct drm_printer *p, const char *str);
> +void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset);
>  
>  __printf(2, 0)
>  /**
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

* Re: [PATCH 2/7] drm/vc4: Use drm_print_regset32() for our debug register dumping.
  2019-02-20 21:03 ` [PATCH 2/7] drm/vc4: Use drm_print_regset32() for our debug register dumping Eric Anholt
@ 2019-03-22 10:32   ` Paul Kocialkowski
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Kocialkowski @ 2019-03-22 10:32 UTC (permalink / raw)
  To: Eric Anholt, dri-devel; +Cc: Maxime Ripard, linux-kernel

Hi,

Le mercredi 20 février 2019 à 13:03 -0800, Eric Anholt a écrit :
> This removes a bunch of duplicated boilerplate for the debugfs vs
> runtime printk debug dumping.

This is:

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/vc4/vc4_crtc.c |  68 ++++++-------
>  drivers/gpu/drm/vc4/vc4_dpi.c  |  23 ++---
>  drivers/gpu/drm/vc4/vc4_drv.h  |   7 ++
>  drivers/gpu/drm/vc4/vc4_dsi.c  | 155 +++++++++++++----------------
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 156 +++++++++++++----------------
>  drivers/gpu/drm/vc4/vc4_hvs.c  |  87 ++++++++---------
>  drivers/gpu/drm/vc4/vc4_txp.c  |  28 +++---
>  drivers/gpu/drm/vc4/vc4_v3d.c  | 173 ++++++++++++++++-----------------
>  drivers/gpu/drm/vc4/vc4_vec.c  |  67 ++++++-------
>  9 files changed, 350 insertions(+), 414 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 730008d3da76..6d7072026056 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -35,6 +35,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_atomic_uapi.h>
> +#include <drm/drm_print.h>
>  #include <drm/drm_probe_helper.h>
>  #include <linux/clk.h>
>  #include <drm/drm_fb_cma_helper.h>
> @@ -67,43 +68,29 @@ to_vc4_crtc_state(struct drm_crtc_state *crtc_state)
>  #define CRTC_WRITE(offset, val) writel(val, vc4_crtc->regs + (offset))
>  #define CRTC_READ(offset) readl(vc4_crtc->regs + (offset))
>  
> -#define CRTC_REG(reg) { reg, #reg }
> -static const struct {
> -	u32 reg;
> -	const char *name;
> -} crtc_regs[] = {
> -	CRTC_REG(PV_CONTROL),
> -	CRTC_REG(PV_V_CONTROL),
> -	CRTC_REG(PV_VSYNCD_EVEN),
> -	CRTC_REG(PV_HORZA),
> -	CRTC_REG(PV_HORZB),
> -	CRTC_REG(PV_VERTA),
> -	CRTC_REG(PV_VERTB),
> -	CRTC_REG(PV_VERTA_EVEN),
> -	CRTC_REG(PV_VERTB_EVEN),
> -	CRTC_REG(PV_INTEN),
> -	CRTC_REG(PV_INTSTAT),
> -	CRTC_REG(PV_STAT),
> -	CRTC_REG(PV_HACT_ACT),
> +static const struct debugfs_reg32 crtc_regs[] = {
> +	VC4_REG32(PV_CONTROL),
> +	VC4_REG32(PV_V_CONTROL),
> +	VC4_REG32(PV_VSYNCD_EVEN),
> +	VC4_REG32(PV_HORZA),
> +	VC4_REG32(PV_HORZB),
> +	VC4_REG32(PV_VERTA),
> +	VC4_REG32(PV_VERTB),
> +	VC4_REG32(PV_VERTA_EVEN),
> +	VC4_REG32(PV_VERTB_EVEN),
> +	VC4_REG32(PV_INTEN),
> +	VC4_REG32(PV_INTSTAT),
> +	VC4_REG32(PV_STAT),
> +	VC4_REG32(PV_HACT_ACT),
>  };
>  
> -static void vc4_crtc_dump_regs(struct vc4_crtc *vc4_crtc)
> -{
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(crtc_regs); i++) {
> -		DRM_INFO("0x%04x (%s): 0x%08x\n",
> -			 crtc_regs[i].reg, crtc_regs[i].name,
> -			 CRTC_READ(crtc_regs[i].reg));
> -	}
> -}
> -
>  #ifdef CONFIG_DEBUG_FS
>  int vc4_crtc_debugfs_regs(struct seq_file *m, void *unused)
>  {
>  	struct drm_info_node *node = (struct drm_info_node *)m->private;
>  	struct drm_device *dev = node->minor->dev;
>  	int crtc_index = (uintptr_t)node->info_ent->data;
> +	struct drm_printer p = drm_seq_file_printer(m);
>  	struct drm_crtc *crtc;
>  	struct vc4_crtc *vc4_crtc;
>  	int i;
> @@ -118,11 +105,7 @@ int vc4_crtc_debugfs_regs(struct seq_file *m, void *unused)
>  		return 0;
>  	vc4_crtc = to_vc4_crtc(crtc);
>  
> -	for (i = 0; i < ARRAY_SIZE(crtc_regs); i++) {
> -		seq_printf(m, "%s (0x%04x): 0x%08x\n",
> -			   crtc_regs[i].name, crtc_regs[i].reg,
> -			   CRTC_READ(crtc_regs[i].reg));
> -	}
> +	drm_print_regset32(&p, &vc4_crtc->regset);
>  
>  	return 0;
>  }
> @@ -434,8 +417,10 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	bool debug_dump_regs = false;
>  
>  	if (debug_dump_regs) {
> -		DRM_INFO("CRTC %d regs before:\n", drm_crtc_index(crtc));
> -		vc4_crtc_dump_regs(vc4_crtc);
> +		struct drm_printer p = drm_info_printer(&vc4_crtc->pdev->dev);
> +		dev_info(&vc4_crtc->pdev->dev, "CRTC %d regs before:\n",
> +			 drm_crtc_index(crtc));
> +		drm_print_regset32(&p, &vc4_crtc->regset);
>  	}
>  
>  	if (vc4_crtc->channel == 2) {
> @@ -476,8 +461,10 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	vc4_crtc_lut_load(crtc);
>  
>  	if (debug_dump_regs) {
> -		DRM_INFO("CRTC %d regs after:\n", drm_crtc_index(crtc));
> -		vc4_crtc_dump_regs(vc4_crtc);
> +		struct drm_printer p = drm_info_printer(&vc4_crtc->pdev->dev);
> +		dev_info(&vc4_crtc->pdev->dev, "CRTC %d regs after:\n",
> +			 drm_crtc_index(crtc));
> +		drm_print_regset32(&p, &vc4_crtc->regset);
>  	}
>  }
>  
> @@ -1169,11 +1156,16 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data)
>  	if (!match)
>  		return -ENODEV;
>  	vc4_crtc->data = match->data;
> +	vc4_crtc->pdev = pdev;
>  
>  	vc4_crtc->regs = vc4_ioremap_regs(pdev, 0);
>  	if (IS_ERR(vc4_crtc->regs))
>  		return PTR_ERR(vc4_crtc->regs);
>  
> +	vc4_crtc->regset.base = vc4_crtc->regs;
> +	vc4_crtc->regset.regs = crtc_regs;
> +	vc4_crtc->regset.nregs = ARRAY_SIZE(crtc_regs);
> +
>  	/* For now, we create just the primary and the legacy cursor
>  	 * planes.  We should be able to stack more planes on easily,
>  	 * but to do that we would need to compute the bandwidth
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index 169521e547ba..8be2264f496a 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -101,6 +101,8 @@ struct vc4_dpi {
>  
>  	struct clk *pixel_clock;
>  	struct clk *core_clock;
> +
> +	struct debugfs_regset32 regset;
>  };
>  
>  #define DPI_READ(offset) readl(dpi->regs + (offset))
> @@ -118,13 +120,9 @@ to_vc4_dpi_encoder(struct drm_encoder *encoder)
>  	return container_of(encoder, struct vc4_dpi_encoder, base.base);
>  }
>  
> -#define DPI_REG(reg) { reg, #reg }
> -static const struct {
> -	u32 reg;
> -	const char *name;
> -} dpi_regs[] = {
> -	DPI_REG(DPI_C),
> -	DPI_REG(DPI_ID),
> +static const struct debugfs_reg32 dpi_regs[] = {
> +	VC4_REG32(DPI_C),
> +	VC4_REG32(DPI_ID),
>  };
>  
>  #ifdef CONFIG_DEBUG_FS
> @@ -134,16 +132,12 @@ int vc4_dpi_debugfs_regs(struct seq_file *m, void *unused)
>  	struct drm_device *dev = node->minor->dev;
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  	struct vc4_dpi *dpi = vc4->dpi;
> -	int i;
> +	struct drm_printer p = drm_seq_file_printer(m);
>  
>  	if (!dpi)
>  		return 0;
>  
> -	for (i = 0; i < ARRAY_SIZE(dpi_regs); i++) {
> -		seq_printf(m, "%s (0x%04x): 0x%08x\n",
> -			   dpi_regs[i].name, dpi_regs[i].reg,
> -			   DPI_READ(dpi_regs[i].reg));
> -	}
> +	drm_print_regset32(&p, &dpi->regset);
>  
>  	return 0;
>  }
> @@ -314,6 +308,9 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data)
>  	dpi->regs = vc4_ioremap_regs(pdev, 0);
>  	if (IS_ERR(dpi->regs))
>  		return PTR_ERR(dpi->regs);
> +	dpi->regset.base = dpi->regs;
> +	dpi->regset.regs = dpi_regs;
> +	dpi->regset.nregs = ARRAY_SIZE(dpi_regs);
>  
>  	if (DPI_READ(DPI_ID) != DPI_ID_VALUE) {
>  		dev_err(dev, "Port returned 0x%08x for ID instead of 0x%08x\n",
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index ce1af06cc197..511ad884d9c3 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -285,6 +285,7 @@ struct vc4_v3d {
>  	struct platform_device *pdev;
>  	void __iomem *regs;
>  	struct clk *clk;
> +	struct debugfs_regset32 regset;
>  };
>  
>  struct vc4_hvs {
> @@ -301,6 +302,7 @@ struct vc4_hvs {
>  	spinlock_t mm_lock;
>  
>  	struct drm_mm_node mitchell_netravali_filter;
> +	struct debugfs_regset32 regset;
>  };
>  
>  struct vc4_plane {
> @@ -410,6 +412,7 @@ struct vc4_crtc_data {
>  
>  struct vc4_crtc {
>  	struct drm_crtc base;
> +	struct platform_device *pdev;
>  	const struct vc4_crtc_data *data;
>  	void __iomem *regs;
>  
> @@ -426,6 +429,8 @@ struct vc4_crtc {
>  	u32 cob_size;
>  
>  	struct drm_pending_vblank_event *event;
> +
> +	struct debugfs_regset32 regset;
>  };
>  
>  static inline struct vc4_crtc *
> @@ -439,6 +444,8 @@ to_vc4_crtc(struct drm_crtc *crtc)
>  #define HVS_READ(offset) readl(vc4->hvs->regs + offset)
>  #define HVS_WRITE(offset, val) writel(val, vc4->hvs->regs + offset)
>  
> +#define VC4_REG32(reg) { .name = #reg, .offset = reg }
> +
>  struct vc4_exec_info {
>  	/* Sequence number for this bin/render job. */
>  	uint64_t seqno;
> diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
> index 11702e1d9011..806cfaa2a6a7 100644
> --- a/drivers/gpu/drm/vc4/vc4_dsi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dsi.c
> @@ -545,6 +545,8 @@ struct vc4_dsi {
>  
>  	struct completion xfer_completion;
>  	int xfer_result;
> +
> +	struct debugfs_regset32 regset;
>  };
>  
>  #define host_to_dsi(host) container_of(host, struct vc4_dsi, dsi_host)
> @@ -605,82 +607,56 @@ to_vc4_dsi_encoder(struct drm_encoder *encoder)
>  	return container_of(encoder, struct vc4_dsi_encoder, base.base);
>  }
>  
> -#define DSI_REG(reg) { reg, #reg }
> -static const struct {
> -	u32 reg;
> -	const char *name;
> -} dsi0_regs[] = {
> -	DSI_REG(DSI0_CTRL),
> -	DSI_REG(DSI0_STAT),
> -	DSI_REG(DSI0_HSTX_TO_CNT),
> -	DSI_REG(DSI0_LPRX_TO_CNT),
> -	DSI_REG(DSI0_TA_TO_CNT),
> -	DSI_REG(DSI0_PR_TO_CNT),
> -	DSI_REG(DSI0_DISP0_CTRL),
> -	DSI_REG(DSI0_DISP1_CTRL),
> -	DSI_REG(DSI0_INT_STAT),
> -	DSI_REG(DSI0_INT_EN),
> -	DSI_REG(DSI0_PHYC),
> -	DSI_REG(DSI0_HS_CLT0),
> -	DSI_REG(DSI0_HS_CLT1),
> -	DSI_REG(DSI0_HS_CLT2),
> -	DSI_REG(DSI0_HS_DLT3),
> -	DSI_REG(DSI0_HS_DLT4),
> -	DSI_REG(DSI0_HS_DLT5),
> -	DSI_REG(DSI0_HS_DLT6),
> -	DSI_REG(DSI0_HS_DLT7),
> -	DSI_REG(DSI0_PHY_AFEC0),
> -	DSI_REG(DSI0_PHY_AFEC1),
> -	DSI_REG(DSI0_ID),
> +static const struct debugfs_reg32 dsi0_regs[] = {
> +	VC4_REG32(DSI0_CTRL),
> +	VC4_REG32(DSI0_STAT),
> +	VC4_REG32(DSI0_HSTX_TO_CNT),
> +	VC4_REG32(DSI0_LPRX_TO_CNT),
> +	VC4_REG32(DSI0_TA_TO_CNT),
> +	VC4_REG32(DSI0_PR_TO_CNT),
> +	VC4_REG32(DSI0_DISP0_CTRL),
> +	VC4_REG32(DSI0_DISP1_CTRL),
> +	VC4_REG32(DSI0_INT_STAT),
> +	VC4_REG32(DSI0_INT_EN),
> +	VC4_REG32(DSI0_PHYC),
> +	VC4_REG32(DSI0_HS_CLT0),
> +	VC4_REG32(DSI0_HS_CLT1),
> +	VC4_REG32(DSI0_HS_CLT2),
> +	VC4_REG32(DSI0_HS_DLT3),
> +	VC4_REG32(DSI0_HS_DLT4),
> +	VC4_REG32(DSI0_HS_DLT5),
> +	VC4_REG32(DSI0_HS_DLT6),
> +	VC4_REG32(DSI0_HS_DLT7),
> +	VC4_REG32(DSI0_PHY_AFEC0),
> +	VC4_REG32(DSI0_PHY_AFEC1),
> +	VC4_REG32(DSI0_ID),
>  };
>  
> -static const struct {
> -	u32 reg;
> -	const char *name;
> -} dsi1_regs[] = {
> -	DSI_REG(DSI1_CTRL),
> -	DSI_REG(DSI1_STAT),
> -	DSI_REG(DSI1_HSTX_TO_CNT),
> -	DSI_REG(DSI1_LPRX_TO_CNT),
> -	DSI_REG(DSI1_TA_TO_CNT),
> -	DSI_REG(DSI1_PR_TO_CNT),
> -	DSI_REG(DSI1_DISP0_CTRL),
> -	DSI_REG(DSI1_DISP1_CTRL),
> -	DSI_REG(DSI1_INT_STAT),
> -	DSI_REG(DSI1_INT_EN),
> -	DSI_REG(DSI1_PHYC),
> -	DSI_REG(DSI1_HS_CLT0),
> -	DSI_REG(DSI1_HS_CLT1),
> -	DSI_REG(DSI1_HS_CLT2),
> -	DSI_REG(DSI1_HS_DLT3),
> -	DSI_REG(DSI1_HS_DLT4),
> -	DSI_REG(DSI1_HS_DLT5),
> -	DSI_REG(DSI1_HS_DLT6),
> -	DSI_REG(DSI1_HS_DLT7),
> -	DSI_REG(DSI1_PHY_AFEC0),
> -	DSI_REG(DSI1_PHY_AFEC1),
> -	DSI_REG(DSI1_ID),
> +static const struct debugfs_reg32 dsi1_regs[] = {
> +	VC4_REG32(DSI1_CTRL),
> +	VC4_REG32(DSI1_STAT),
> +	VC4_REG32(DSI1_HSTX_TO_CNT),
> +	VC4_REG32(DSI1_LPRX_TO_CNT),
> +	VC4_REG32(DSI1_TA_TO_CNT),
> +	VC4_REG32(DSI1_PR_TO_CNT),
> +	VC4_REG32(DSI1_DISP0_CTRL),
> +	VC4_REG32(DSI1_DISP1_CTRL),
> +	VC4_REG32(DSI1_INT_STAT),
> +	VC4_REG32(DSI1_INT_EN),
> +	VC4_REG32(DSI1_PHYC),
> +	VC4_REG32(DSI1_HS_CLT0),
> +	VC4_REG32(DSI1_HS_CLT1),
> +	VC4_REG32(DSI1_HS_CLT2),
> +	VC4_REG32(DSI1_HS_DLT3),
> +	VC4_REG32(DSI1_HS_DLT4),
> +	VC4_REG32(DSI1_HS_DLT5),
> +	VC4_REG32(DSI1_HS_DLT6),
> +	VC4_REG32(DSI1_HS_DLT7),
> +	VC4_REG32(DSI1_PHY_AFEC0),
> +	VC4_REG32(DSI1_PHY_AFEC1),
> +	VC4_REG32(DSI1_ID),
>  };
>  
> -static void vc4_dsi_dump_regs(struct vc4_dsi *dsi)
> -{
> -	int i;
> -
> -	if (dsi->port == 0) {
> -		for (i = 0; i < ARRAY_SIZE(dsi0_regs); i++) {
> -			DRM_INFO("0x%04x (%s): 0x%08x\n",
> -				 dsi0_regs[i].reg, dsi0_regs[i].name,
> -				 DSI_READ(dsi0_regs[i].reg));
> -		}
> -	} else {
> -		for (i = 0; i < ARRAY_SIZE(dsi1_regs); i++) {
> -			DRM_INFO("0x%04x (%s): 0x%08x\n",
> -				 dsi1_regs[i].reg, dsi1_regs[i].name,
> -				 DSI_READ(dsi1_regs[i].reg));
> -		}
> -	}
> -}
> -
>  #ifdef CONFIG_DEBUG_FS
>  int vc4_dsi_debugfs_regs(struct seq_file *m, void *unused)
>  {
> @@ -689,24 +665,12 @@ int vc4_dsi_debugfs_regs(struct seq_file *m, void *unused)
>  	struct vc4_dev *vc4 = to_vc4_dev(drm);
>  	int dsi_index = (uintptr_t)node->info_ent->data;
>  	struct vc4_dsi *dsi = (dsi_index == 1 ? vc4->dsi1 : NULL);
> -	int i;
> +	struct drm_printer p = drm_seq_file_printer(m);
>  
>  	if (!dsi)
>  		return 0;
>  
> -	if (dsi->port == 0) {
> -		for (i = 0; i < ARRAY_SIZE(dsi0_regs); i++) {
> -			seq_printf(m, "0x%04x (%s): 0x%08x\n",
> -				   dsi0_regs[i].reg, dsi0_regs[i].name,
> -				   DSI_READ(dsi0_regs[i].reg));
> -		}
> -	} else {
> -		for (i = 0; i < ARRAY_SIZE(dsi1_regs); i++) {
> -			seq_printf(m, "0x%04x (%s): 0x%08x\n",
> -				   dsi1_regs[i].reg, dsi1_regs[i].name,
> -				   DSI_READ(dsi1_regs[i].reg));
> -		}
> -	}
> +	drm_print_regset32(&p, &dsi->regset);
>  
>  	return 0;
>  }
> @@ -900,8 +864,9 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
>  	}
>  
>  	if (debug_dump_regs) {
> -		DRM_INFO("DSI regs before:\n");
> -		vc4_dsi_dump_regs(dsi);
> +		struct drm_printer p = drm_info_printer(&dsi->pdev->dev);
> +		dev_info(&dsi->pdev->dev, "DSI regs before:\n");
> +		drm_print_regset32(&p, &dsi->regset);
>  	}
>  
>  	/* Round up the clk_set_rate() request slightly, since
> @@ -1135,8 +1100,9 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
>  	drm_bridge_enable(dsi->bridge);
>  
>  	if (debug_dump_regs) {
> -		DRM_INFO("DSI regs after:\n");
> -		vc4_dsi_dump_regs(dsi);
> +		struct drm_printer p = drm_info_printer(&dsi->pdev->dev);
> +		dev_info(&dsi->pdev->dev, "DSI regs after:\n");
> +		drm_print_regset32(&p, &dsi->regset);
>  	}
>  }
>  
> @@ -1527,6 +1493,15 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
>  	if (IS_ERR(dsi->regs))
>  		return PTR_ERR(dsi->regs);
>  
> +	dsi->regset.base = dsi->regs;
> +	if (dsi->port == 0) {
> +		dsi->regset.regs = dsi0_regs;
> +		dsi->regset.nregs = ARRAY_SIZE(dsi0_regs);
> +	} else {
> +		dsi->regset.regs = dsi1_regs;
> +		dsi->regset.nregs = ARRAY_SIZE(dsi1_regs);
> +	}
> +
>  	if (DSI_PORT_READ(ID) != DSI_ID_VALUE) {
>  		dev_err(dev, "Port returned 0x%08x for ID instead of 0x%08x\n",
>  			DSI_PORT_READ(ID), DSI_ID_VALUE);
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 88fd5df7e7dc..38c9172cfe52 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -97,6 +97,9 @@ struct vc4_hdmi {
>  
>  	struct clk *pixel_clock;
>  	struct clk *hsm_clock;
> +
> +	struct debugfs_regset32 hdmi_regset;
> +	struct debugfs_regset32 hd_regset;
>  };
>  
>  #define HDMI_READ(offset) readl(vc4->hdmi->hdmicore_regs + offset)
> @@ -134,61 +137,54 @@ to_vc4_hdmi_connector(struct drm_connector *connector)
>  	return container_of(connector, struct vc4_hdmi_connector, base);
>  }
>  
> -#define HDMI_REG(reg) { reg, #reg }
> -static const struct {
> -	u32 reg;
> -	const char *name;
> -} hdmi_regs[] = {
> -	HDMI_REG(VC4_HDMI_CORE_REV),
> -	HDMI_REG(VC4_HDMI_SW_RESET_CONTROL),
> -	HDMI_REG(VC4_HDMI_HOTPLUG_INT),
> -	HDMI_REG(VC4_HDMI_HOTPLUG),
> -	HDMI_REG(VC4_HDMI_MAI_CHANNEL_MAP),
> -	HDMI_REG(VC4_HDMI_MAI_CONFIG),
> -	HDMI_REG(VC4_HDMI_MAI_FORMAT),
> -	HDMI_REG(VC4_HDMI_AUDIO_PACKET_CONFIG),
> -	HDMI_REG(VC4_HDMI_RAM_PACKET_CONFIG),
> -	HDMI_REG(VC4_HDMI_HORZA),
> -	HDMI_REG(VC4_HDMI_HORZB),
> -	HDMI_REG(VC4_HDMI_FIFO_CTL),
> -	HDMI_REG(VC4_HDMI_SCHEDULER_CONTROL),
> -	HDMI_REG(VC4_HDMI_VERTA0),
> -	HDMI_REG(VC4_HDMI_VERTA1),
> -	HDMI_REG(VC4_HDMI_VERTB0),
> -	HDMI_REG(VC4_HDMI_VERTB1),
> -	HDMI_REG(VC4_HDMI_TX_PHY_RESET_CTL),
> -	HDMI_REG(VC4_HDMI_TX_PHY_CTL0),
> -
> -	HDMI_REG(VC4_HDMI_CEC_CNTRL_1),
> -	HDMI_REG(VC4_HDMI_CEC_CNTRL_2),
> -	HDMI_REG(VC4_HDMI_CEC_CNTRL_3),
> -	HDMI_REG(VC4_HDMI_CEC_CNTRL_4),
> -	HDMI_REG(VC4_HDMI_CEC_CNTRL_5),
> -	HDMI_REG(VC4_HDMI_CPU_STATUS),
> -	HDMI_REG(VC4_HDMI_CPU_MASK_STATUS),
> -
> -	HDMI_REG(VC4_HDMI_CEC_RX_DATA_1),
> -	HDMI_REG(VC4_HDMI_CEC_RX_DATA_2),
> -	HDMI_REG(VC4_HDMI_CEC_RX_DATA_3),
> -	HDMI_REG(VC4_HDMI_CEC_RX_DATA_4),
> -	HDMI_REG(VC4_HDMI_CEC_TX_DATA_1),
> -	HDMI_REG(VC4_HDMI_CEC_TX_DATA_2),
> -	HDMI_REG(VC4_HDMI_CEC_TX_DATA_3),
> -	HDMI_REG(VC4_HDMI_CEC_TX_DATA_4),
> +static const struct debugfs_reg32 hdmi_regs[] = {
> +	VC4_REG32(VC4_HDMI_CORE_REV),
> +	VC4_REG32(VC4_HDMI_SW_RESET_CONTROL),
> +	VC4_REG32(VC4_HDMI_HOTPLUG_INT),
> +	VC4_REG32(VC4_HDMI_HOTPLUG),
> +	VC4_REG32(VC4_HDMI_MAI_CHANNEL_MAP),
> +	VC4_REG32(VC4_HDMI_MAI_CONFIG),
> +	VC4_REG32(VC4_HDMI_MAI_FORMAT),
> +	VC4_REG32(VC4_HDMI_AUDIO_PACKET_CONFIG),
> +	VC4_REG32(VC4_HDMI_RAM_PACKET_CONFIG),
> +	VC4_REG32(VC4_HDMI_HORZA),
> +	VC4_REG32(VC4_HDMI_HORZB),
> +	VC4_REG32(VC4_HDMI_FIFO_CTL),
> +	VC4_REG32(VC4_HDMI_SCHEDULER_CONTROL),
> +	VC4_REG32(VC4_HDMI_VERTA0),
> +	VC4_REG32(VC4_HDMI_VERTA1),
> +	VC4_REG32(VC4_HDMI_VERTB0),
> +	VC4_REG32(VC4_HDMI_VERTB1),
> +	VC4_REG32(VC4_HDMI_TX_PHY_RESET_CTL),
> +	VC4_REG32(VC4_HDMI_TX_PHY_CTL0),
> +
> +	VC4_REG32(VC4_HDMI_CEC_CNTRL_1),
> +	VC4_REG32(VC4_HDMI_CEC_CNTRL_2),
> +	VC4_REG32(VC4_HDMI_CEC_CNTRL_3),
> +	VC4_REG32(VC4_HDMI_CEC_CNTRL_4),
> +	VC4_REG32(VC4_HDMI_CEC_CNTRL_5),
> +	VC4_REG32(VC4_HDMI_CPU_STATUS),
> +	VC4_REG32(VC4_HDMI_CPU_MASK_STATUS),
> +
> +	VC4_REG32(VC4_HDMI_CEC_RX_DATA_1),
> +	VC4_REG32(VC4_HDMI_CEC_RX_DATA_2),
> +	VC4_REG32(VC4_HDMI_CEC_RX_DATA_3),
> +	VC4_REG32(VC4_HDMI_CEC_RX_DATA_4),
> +	VC4_REG32(VC4_HDMI_CEC_TX_DATA_1),
> +	VC4_REG32(VC4_HDMI_CEC_TX_DATA_2),
> +	VC4_REG32(VC4_HDMI_CEC_TX_DATA_3),
> +	VC4_REG32(VC4_HDMI_CEC_TX_DATA_4),
>  };
>  
> -static const struct {
> -	u32 reg;
> -	const char *name;
> -} hd_regs[] = {
> -	HDMI_REG(VC4_HD_M_CTL),
> -	HDMI_REG(VC4_HD_MAI_CTL),
> -	HDMI_REG(VC4_HD_MAI_THR),
> -	HDMI_REG(VC4_HD_MAI_FMT),
> -	HDMI_REG(VC4_HD_MAI_SMP),
> -	HDMI_REG(VC4_HD_VID_CTL),
> -	HDMI_REG(VC4_HD_CSC_CTL),
> -	HDMI_REG(VC4_HD_FRAME_COUNT),
> +static const struct debugfs_reg32 hd_regs[] = {
> +	VC4_REG32(VC4_HD_M_CTL),
> +	VC4_REG32(VC4_HD_MAI_CTL),
> +	VC4_REG32(VC4_HD_MAI_THR),
> +	VC4_REG32(VC4_HD_MAI_FMT),
> +	VC4_REG32(VC4_HD_MAI_SMP),
> +	VC4_REG32(VC4_HD_VID_CTL),
> +	VC4_REG32(VC4_HD_CSC_CTL),
> +	VC4_REG32(VC4_HD_FRAME_COUNT),
>  };
>  
>  #ifdef CONFIG_DEBUG_FS
> @@ -197,41 +193,16 @@ int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
>  	struct drm_info_node *node = (struct drm_info_node *)m->private;
>  	struct drm_device *dev = node->minor->dev;
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(hdmi_regs); i++) {
> -		seq_printf(m, "%s (0x%04x): 0x%08x\n",
> -			   hdmi_regs[i].name, hdmi_regs[i].reg,
> -			   HDMI_READ(hdmi_regs[i].reg));
> -	}
> +	struct vc4_hdmi *hdmi = vc4->hdmi;
> +	struct drm_printer p = drm_seq_file_printer(m);
>  
> -	for (i = 0; i < ARRAY_SIZE(hd_regs); i++) {
> -		seq_printf(m, "%s (0x%04x): 0x%08x\n",
> -			   hd_regs[i].name, hd_regs[i].reg,
> -			   HD_READ(hd_regs[i].reg));
> -	}
> +	drm_print_regset32(&p, &hdmi->hdmi_regset);
> +	drm_print_regset32(&p, &hdmi->hd_regset);
>  
>  	return 0;
>  }
>  #endif /* CONFIG_DEBUG_FS */
>  
> -static void vc4_hdmi_dump_regs(struct drm_device *dev)
> -{
> -	struct vc4_dev *vc4 = to_vc4_dev(dev);
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(hdmi_regs); i++) {
> -		DRM_INFO("0x%04x (%s): 0x%08x\n",
> -			 hdmi_regs[i].reg, hdmi_regs[i].name,
> -			 HDMI_READ(hdmi_regs[i].reg));
> -	}
> -	for (i = 0; i < ARRAY_SIZE(hd_regs); i++) {
> -		DRM_INFO("0x%04x (%s): 0x%08x\n",
> -			 hd_regs[i].reg, hd_regs[i].name,
> -			 HD_READ(hd_regs[i].reg));
> -	}
> -}
> -
>  static enum drm_connector_status
>  vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
>  {
> @@ -561,8 +532,11 @@ static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder)
>  	HDMI_WRITE(VC4_HDMI_TX_PHY_RESET_CTL, 0);
>  
>  	if (debug_dump_regs) {
> -		DRM_INFO("HDMI regs before:\n");
> -		vc4_hdmi_dump_regs(dev);
> +		struct drm_printer p = drm_info_printer(&hdmi->pdev->dev);
> +
> +		dev_info(&hdmi->pdev->dev, "HDMI regs before:\n");
> +		drm_print_regset32(&p, &hdmi->hdmi_regset);
> +		drm_print_regset32(&p, &hdmi->hd_regset);
>  	}
>  
>  	HD_WRITE(VC4_HD_VID_CTL, 0);
> @@ -637,8 +611,11 @@ static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder)
>  	HDMI_WRITE(VC4_HDMI_FIFO_CTL, VC4_HDMI_FIFO_CTL_MASTER_SLAVE_N);
>  
>  	if (debug_dump_regs) {
> -		DRM_INFO("HDMI regs after:\n");
> -		vc4_hdmi_dump_regs(dev);
> +		struct drm_printer p = drm_info_printer(&hdmi->pdev->dev);
> +
> +		dev_info(&hdmi->pdev->dev, "HDMI regs after:\n");
> +		drm_print_regset32(&p, &hdmi->hdmi_regset);
> +		drm_print_regset32(&p, &hdmi->hd_regset);
>  	}
>  
>  	HD_WRITE(VC4_HD_VID_CTL,
> @@ -1333,6 +1310,13 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
>  	if (IS_ERR(hdmi->hd_regs))
>  		return PTR_ERR(hdmi->hd_regs);
>  
> +	hdmi->hdmi_regset.base = hdmi->hdmicore_regs;
> +	hdmi->hdmi_regset.regs = hdmi_regs;
> +	hdmi->hdmi_regset.nregs = ARRAY_SIZE(hdmi_regs);
> +	hdmi->hd_regset.base = hdmi->hd_regs;
> +	hdmi->hd_regset.regs = hd_regs;
> +	hdmi->hd_regset.nregs = ARRAY_SIZE(hd_regs);
> +
>  	hdmi->pixel_clock = devm_clk_get(dev, "pixel");
>  	if (IS_ERR(hdmi->pixel_clock)) {
>  		DRM_ERROR("Failed to get pixel clock\n");
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index 5d8c749c9749..45405f76cd40 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -26,54 +26,47 @@
>  #include "vc4_drv.h"
>  #include "vc4_regs.h"
>  
> -#define HVS_REG(reg) { reg, #reg }
> -static const struct {
> -	u32 reg;
> -	const char *name;
> -} hvs_regs[] = {
> -	HVS_REG(SCALER_DISPCTRL),
> -	HVS_REG(SCALER_DISPSTAT),
> -	HVS_REG(SCALER_DISPID),
> -	HVS_REG(SCALER_DISPECTRL),
> -	HVS_REG(SCALER_DISPPROF),
> -	HVS_REG(SCALER_DISPDITHER),
> -	HVS_REG(SCALER_DISPEOLN),
> -	HVS_REG(SCALER_DISPLIST0),
> -	HVS_REG(SCALER_DISPLIST1),
> -	HVS_REG(SCALER_DISPLIST2),
> -	HVS_REG(SCALER_DISPLSTAT),
> -	HVS_REG(SCALER_DISPLACT0),
> -	HVS_REG(SCALER_DISPLACT1),
> -	HVS_REG(SCALER_DISPLACT2),
> -	HVS_REG(SCALER_DISPCTRL0),
> -	HVS_REG(SCALER_DISPBKGND0),
> -	HVS_REG(SCALER_DISPSTAT0),
> -	HVS_REG(SCALER_DISPBASE0),
> -	HVS_REG(SCALER_DISPCTRL1),
> -	HVS_REG(SCALER_DISPBKGND1),
> -	HVS_REG(SCALER_DISPSTAT1),
> -	HVS_REG(SCALER_DISPBASE1),
> -	HVS_REG(SCALER_DISPCTRL2),
> -	HVS_REG(SCALER_DISPBKGND2),
> -	HVS_REG(SCALER_DISPSTAT2),
> -	HVS_REG(SCALER_DISPBASE2),
> -	HVS_REG(SCALER_DISPALPHA2),
> -	HVS_REG(SCALER_OLEDOFFS),
> -	HVS_REG(SCALER_OLEDCOEF0),
> -	HVS_REG(SCALER_OLEDCOEF1),
> -	HVS_REG(SCALER_OLEDCOEF2),
> +static const struct debugfs_reg32 hvs_regs[] = {
> +	VC4_REG32(SCALER_DISPCTRL),
> +	VC4_REG32(SCALER_DISPSTAT),
> +	VC4_REG32(SCALER_DISPID),
> +	VC4_REG32(SCALER_DISPECTRL),
> +	VC4_REG32(SCALER_DISPPROF),
> +	VC4_REG32(SCALER_DISPDITHER),
> +	VC4_REG32(SCALER_DISPEOLN),
> +	VC4_REG32(SCALER_DISPLIST0),
> +	VC4_REG32(SCALER_DISPLIST1),
> +	VC4_REG32(SCALER_DISPLIST2),
> +	VC4_REG32(SCALER_DISPLSTAT),
> +	VC4_REG32(SCALER_DISPLACT0),
> +	VC4_REG32(SCALER_DISPLACT1),
> +	VC4_REG32(SCALER_DISPLACT2),
> +	VC4_REG32(SCALER_DISPCTRL0),
> +	VC4_REG32(SCALER_DISPBKGND0),
> +	VC4_REG32(SCALER_DISPSTAT0),
> +	VC4_REG32(SCALER_DISPBASE0),
> +	VC4_REG32(SCALER_DISPCTRL1),
> +	VC4_REG32(SCALER_DISPBKGND1),
> +	VC4_REG32(SCALER_DISPSTAT1),
> +	VC4_REG32(SCALER_DISPBASE1),
> +	VC4_REG32(SCALER_DISPCTRL2),
> +	VC4_REG32(SCALER_DISPBKGND2),
> +	VC4_REG32(SCALER_DISPSTAT2),
> +	VC4_REG32(SCALER_DISPBASE2),
> +	VC4_REG32(SCALER_DISPALPHA2),
> +	VC4_REG32(SCALER_OLEDOFFS),
> +	VC4_REG32(SCALER_OLEDCOEF0),
> +	VC4_REG32(SCALER_OLEDCOEF1),
> +	VC4_REG32(SCALER_OLEDCOEF2),
>  };
>  
>  void vc4_hvs_dump_state(struct drm_device *dev)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> +	struct drm_printer p = drm_info_printer(&vc4->hvs->pdev->dev);
>  	int i;
>  
> -	for (i = 0; i < ARRAY_SIZE(hvs_regs); i++) {
> -		DRM_INFO("0x%04x (%s): 0x%08x\n",
> -			 hvs_regs[i].reg, hvs_regs[i].name,
> -			 HVS_READ(hvs_regs[i].reg));
> -	}
> +	drm_print_regset32(&p, &vc4->hvs->regset);
>  
>  	DRM_INFO("HVS ctx:\n");
>  	for (i = 0; i < 64; i += 4) {
> @@ -92,13 +85,9 @@ int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused)
>  	struct drm_info_node *node = (struct drm_info_node *)m->private;
>  	struct drm_device *dev = node->minor->dev;
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> -	int i;
> +	struct drm_printer p = drm_seq_file_printer(m);
>  
> -	for (i = 0; i < ARRAY_SIZE(hvs_regs); i++) {
> -		seq_printf(m, "%s (0x%04x): 0x%08x\n",
> -			   hvs_regs[i].name, hvs_regs[i].reg,
> -			   HVS_READ(hvs_regs[i].reg));
> -	}
> +	drm_print_regset32(&p, &vc4->hvs->regset);
>  
>  	return 0;
>  }
> @@ -185,6 +174,10 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
>  	if (IS_ERR(hvs->regs))
>  		return PTR_ERR(hvs->regs);
>  
> +	hvs->regset.base = hvs->regs;
> +	hvs->regset.regs = hvs_regs;
> +	hvs->regset.nregs = ARRAY_SIZE(hvs_regs);
> +
>  	hvs->dlist = hvs->regs + SCALER_DLIST_START;
>  
>  	spin_lock_init(&hvs->mm_lock);
> diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
> index afb1c4ec4f18..c8f80064c179 100644
> --- a/drivers/gpu/drm/vc4/vc4_txp.c
> +++ b/drivers/gpu/drm/vc4/vc4_txp.c
> @@ -148,6 +148,7 @@ struct vc4_txp {
>  	struct drm_writeback_connector connector;
>  
>  	void __iomem *regs;
> +	struct debugfs_regset32 regset;
>  };
>  
>  static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder *encoder)
> @@ -160,16 +161,12 @@ static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector *conn)
>  	return container_of(conn, struct vc4_txp, connector.base);
>  }
>  
> -#define TXP_REG(reg) { reg, #reg }
> -static const struct {
> -	u32 reg;
> -	const char *name;
> -} txp_regs[] = {
> -	TXP_REG(TXP_DST_PTR),
> -	TXP_REG(TXP_DST_PITCH),
> -	TXP_REG(TXP_DIM),
> -	TXP_REG(TXP_DST_CTRL),
> -	TXP_REG(TXP_PROGRESS),
> +static const struct debugfs_reg32 txp_regs[] = {
> +	VC4_REG32(TXP_DST_PTR),
> +	VC4_REG32(TXP_DST_PITCH),
> +	VC4_REG32(TXP_DIM),
> +	VC4_REG32(TXP_DST_CTRL),
> +	VC4_REG32(TXP_PROGRESS),
>  };
>  
>  #ifdef CONFIG_DEBUG_FS
> @@ -179,16 +176,12 @@ int vc4_txp_debugfs_regs(struct seq_file *m, void *unused)
>  	struct drm_device *dev = node->minor->dev;
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  	struct vc4_txp *txp = vc4->txp;
> -	int i;
> +	struct drm_printer p = drm_seq_file_printer(m);
>  
>  	if (!txp)
>  		return 0;
>  
> -	for (i = 0; i < ARRAY_SIZE(txp_regs); i++) {
> -		seq_printf(m, "%s (0x%04x): 0x%08x\n",
> -			   txp_regs[i].name, txp_regs[i].reg,
> -			   TXP_READ(txp_regs[i].reg));
> -	}
> +	drm_print_regset32(&p, &txp->regset);
>  
>  	return 0;
>  }
> @@ -410,6 +403,9 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data)
>  	txp->regs = vc4_ioremap_regs(pdev, 0);
>  	if (IS_ERR(txp->regs))
>  		return PTR_ERR(txp->regs);
> +	txp->regset.base = txp->regs;
> +	txp->regset.regs = txp_regs;
> +	txp->regset.nregs = ARRAY_SIZE(txp_regs);
>  
>  	drm_connector_helper_add(&txp->connector.base,
>  				 &vc4_txp_connector_helper_funcs);
> diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
> index e47e29426078..e1e728e95562 100644
> --- a/drivers/gpu/drm/vc4/vc4_v3d.c
> +++ b/drivers/gpu/drm/vc4/vc4_v3d.c
> @@ -23,89 +23,85 @@
>  #include "vc4_regs.h"
>  
>  #ifdef CONFIG_DEBUG_FS
> -#define REGDEF(reg) { reg, #reg }
> -static const struct {
> -	uint32_t reg;
> -	const char *name;
> -} vc4_reg_defs[] = {
> -	REGDEF(V3D_IDENT0),
> -	REGDEF(V3D_IDENT1),
> -	REGDEF(V3D_IDENT2),
> -	REGDEF(V3D_SCRATCH),
> -	REGDEF(V3D_L2CACTL),
> -	REGDEF(V3D_SLCACTL),
> -	REGDEF(V3D_INTCTL),
> -	REGDEF(V3D_INTENA),
> -	REGDEF(V3D_INTDIS),
> -	REGDEF(V3D_CT0CS),
> -	REGDEF(V3D_CT1CS),
> -	REGDEF(V3D_CT0EA),
> -	REGDEF(V3D_CT1EA),
> -	REGDEF(V3D_CT0CA),
> -	REGDEF(V3D_CT1CA),
> -	REGDEF(V3D_CT00RA0),
> -	REGDEF(V3D_CT01RA0),
> -	REGDEF(V3D_CT0LC),
> -	REGDEF(V3D_CT1LC),
> -	REGDEF(V3D_CT0PC),
> -	REGDEF(V3D_CT1PC),
> -	REGDEF(V3D_PCS),
> -	REGDEF(V3D_BFC),
> -	REGDEF(V3D_RFC),
> -	REGDEF(V3D_BPCA),
> -	REGDEF(V3D_BPCS),
> -	REGDEF(V3D_BPOA),
> -	REGDEF(V3D_BPOS),
> -	REGDEF(V3D_BXCF),
> -	REGDEF(V3D_SQRSV0),
> -	REGDEF(V3D_SQRSV1),
> -	REGDEF(V3D_SQCNTL),
> -	REGDEF(V3D_SRQPC),
> -	REGDEF(V3D_SRQUA),
> -	REGDEF(V3D_SRQUL),
> -	REGDEF(V3D_SRQCS),
> -	REGDEF(V3D_VPACNTL),
> -	REGDEF(V3D_VPMBASE),
> -	REGDEF(V3D_PCTRC),
> -	REGDEF(V3D_PCTRE),
> -	REGDEF(V3D_PCTR(0)),
> -	REGDEF(V3D_PCTRS(0)),
> -	REGDEF(V3D_PCTR(1)),
> -	REGDEF(V3D_PCTRS(1)),
> -	REGDEF(V3D_PCTR(2)),
> -	REGDEF(V3D_PCTRS(2)),
> -	REGDEF(V3D_PCTR(3)),
> -	REGDEF(V3D_PCTRS(3)),
> -	REGDEF(V3D_PCTR(4)),
> -	REGDEF(V3D_PCTRS(4)),
> -	REGDEF(V3D_PCTR(5)),
> -	REGDEF(V3D_PCTRS(5)),
> -	REGDEF(V3D_PCTR(6)),
> -	REGDEF(V3D_PCTRS(6)),
> -	REGDEF(V3D_PCTR(7)),
> -	REGDEF(V3D_PCTRS(7)),
> -	REGDEF(V3D_PCTR(8)),
> -	REGDEF(V3D_PCTRS(8)),
> -	REGDEF(V3D_PCTR(9)),
> -	REGDEF(V3D_PCTRS(9)),
> -	REGDEF(V3D_PCTR(10)),
> -	REGDEF(V3D_PCTRS(10)),
> -	REGDEF(V3D_PCTR(11)),
> -	REGDEF(V3D_PCTRS(11)),
> -	REGDEF(V3D_PCTR(12)),
> -	REGDEF(V3D_PCTRS(12)),
> -	REGDEF(V3D_PCTR(13)),
> -	REGDEF(V3D_PCTRS(13)),
> -	REGDEF(V3D_PCTR(14)),
> -	REGDEF(V3D_PCTRS(14)),
> -	REGDEF(V3D_PCTR(15)),
> -	REGDEF(V3D_PCTRS(15)),
> -	REGDEF(V3D_DBGE),
> -	REGDEF(V3D_FDBGO),
> -	REGDEF(V3D_FDBGB),
> -	REGDEF(V3D_FDBGR),
> -	REGDEF(V3D_FDBGS),
> -	REGDEF(V3D_ERRSTAT),
> +static const struct debugfs_reg32 v3d_regs[] = {
> +	VC4_REG32(V3D_IDENT0),
> +	VC4_REG32(V3D_IDENT1),
> +	VC4_REG32(V3D_IDENT2),
> +	VC4_REG32(V3D_SCRATCH),
> +	VC4_REG32(V3D_L2CACTL),
> +	VC4_REG32(V3D_SLCACTL),
> +	VC4_REG32(V3D_INTCTL),
> +	VC4_REG32(V3D_INTENA),
> +	VC4_REG32(V3D_INTDIS),
> +	VC4_REG32(V3D_CT0CS),
> +	VC4_REG32(V3D_CT1CS),
> +	VC4_REG32(V3D_CT0EA),
> +	VC4_REG32(V3D_CT1EA),
> +	VC4_REG32(V3D_CT0CA),
> +	VC4_REG32(V3D_CT1CA),
> +	VC4_REG32(V3D_CT00RA0),
> +	VC4_REG32(V3D_CT01RA0),
> +	VC4_REG32(V3D_CT0LC),
> +	VC4_REG32(V3D_CT1LC),
> +	VC4_REG32(V3D_CT0PC),
> +	VC4_REG32(V3D_CT1PC),
> +	VC4_REG32(V3D_PCS),
> +	VC4_REG32(V3D_BFC),
> +	VC4_REG32(V3D_RFC),
> +	VC4_REG32(V3D_BPCA),
> +	VC4_REG32(V3D_BPCS),
> +	VC4_REG32(V3D_BPOA),
> +	VC4_REG32(V3D_BPOS),
> +	VC4_REG32(V3D_BXCF),
> +	VC4_REG32(V3D_SQRSV0),
> +	VC4_REG32(V3D_SQRSV1),
> +	VC4_REG32(V3D_SQCNTL),
> +	VC4_REG32(V3D_SRQPC),
> +	VC4_REG32(V3D_SRQUA),
> +	VC4_REG32(V3D_SRQUL),
> +	VC4_REG32(V3D_SRQCS),
> +	VC4_REG32(V3D_VPACNTL),
> +	VC4_REG32(V3D_VPMBASE),
> +	VC4_REG32(V3D_PCTRC),
> +	VC4_REG32(V3D_PCTRE),
> +	VC4_REG32(V3D_PCTR(0)),
> +	VC4_REG32(V3D_PCTRS(0)),
> +	VC4_REG32(V3D_PCTR(1)),
> +	VC4_REG32(V3D_PCTRS(1)),
> +	VC4_REG32(V3D_PCTR(2)),
> +	VC4_REG32(V3D_PCTRS(2)),
> +	VC4_REG32(V3D_PCTR(3)),
> +	VC4_REG32(V3D_PCTRS(3)),
> +	VC4_REG32(V3D_PCTR(4)),
> +	VC4_REG32(V3D_PCTRS(4)),
> +	VC4_REG32(V3D_PCTR(5)),
> +	VC4_REG32(V3D_PCTRS(5)),
> +	VC4_REG32(V3D_PCTR(6)),
> +	VC4_REG32(V3D_PCTRS(6)),
> +	VC4_REG32(V3D_PCTR(7)),
> +	VC4_REG32(V3D_PCTRS(7)),
> +	VC4_REG32(V3D_PCTR(8)),
> +	VC4_REG32(V3D_PCTRS(8)),
> +	VC4_REG32(V3D_PCTR(9)),
> +	VC4_REG32(V3D_PCTRS(9)),
> +	VC4_REG32(V3D_PCTR(10)),
> +	VC4_REG32(V3D_PCTRS(10)),
> +	VC4_REG32(V3D_PCTR(11)),
> +	VC4_REG32(V3D_PCTRS(11)),
> +	VC4_REG32(V3D_PCTR(12)),
> +	VC4_REG32(V3D_PCTRS(12)),
> +	VC4_REG32(V3D_PCTR(13)),
> +	VC4_REG32(V3D_PCTRS(13)),
> +	VC4_REG32(V3D_PCTR(14)),
> +	VC4_REG32(V3D_PCTRS(14)),
> +	VC4_REG32(V3D_PCTR(15)),
> +	VC4_REG32(V3D_PCTRS(15)),
> +	VC4_REG32(V3D_DBGE),
> +	VC4_REG32(V3D_FDBGO),
> +	VC4_REG32(V3D_FDBGB),
> +	VC4_REG32(V3D_FDBGR),
> +	VC4_REG32(V3D_FDBGS),
> +	VC4_REG32(V3D_ERRSTAT),
>  };
>  
>  int vc4_v3d_debugfs_regs(struct seq_file *m, void *unused)
> @@ -113,13 +109,9 @@ int vc4_v3d_debugfs_regs(struct seq_file *m, void *unused)
>  	struct drm_info_node *node = (struct drm_info_node *)m->private;
>  	struct drm_device *dev = node->minor->dev;
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> -	int i;
> +	struct drm_printer p = drm_seq_file_printer(m);
>  
> -	for (i = 0; i < ARRAY_SIZE(vc4_reg_defs); i++) {
> -		seq_printf(m, "%s (0x%04x): 0x%08x\n",
> -			   vc4_reg_defs[i].name, vc4_reg_defs[i].reg,
> -			   V3D_READ(vc4_reg_defs[i].reg));
> -	}
> +	drm_print_regset32(&p, &vc4->v3d->regset);
>  
>  	return 0;
>  }
> @@ -354,6 +346,9 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
>  	v3d->regs = vc4_ioremap_regs(pdev, 0);
>  	if (IS_ERR(v3d->regs))
>  		return PTR_ERR(v3d->regs);
> +	v3d->regset.base = v3d->regs;
> +	v3d->regset.regs = v3d_regs;
> +	v3d->regset.nregs = ARRAY_SIZE(v3d_regs);
>  
>  	vc4->v3d = v3d;
>  	v3d->vc4 = vc4;
> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> index 858c3a483229..83227d2440aa 100644
> --- a/drivers/gpu/drm/vc4/vc4_vec.c
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> @@ -176,6 +176,8 @@ struct vc4_vec {
>  	struct clk *clock;
>  
>  	const struct vc4_vec_tv_mode *tv_mode;
> +
> +	struct debugfs_regset32 regset;
>  };
>  
>  #define VEC_READ(offset) readl(vec->regs + (offset))
> @@ -223,35 +225,31 @@ struct vc4_vec_tv_mode {
>  	void (*mode_set)(struct vc4_vec *vec);
>  };
>  
> -#define VEC_REG(reg) { reg, #reg }
> -static const struct {
> -	u32 reg;
> -	const char *name;
> -} vec_regs[] = {
> -	VEC_REG(VEC_WSE_CONTROL),
> -	VEC_REG(VEC_WSE_WSS_DATA),
> -	VEC_REG(VEC_WSE_VPS_DATA1),
> -	VEC_REG(VEC_WSE_VPS_CONTROL),
> -	VEC_REG(VEC_REVID),
> -	VEC_REG(VEC_CONFIG0),
> -	VEC_REG(VEC_SCHPH),
> -	VEC_REG(VEC_CLMP0_START),
> -	VEC_REG(VEC_CLMP0_END),
> -	VEC_REG(VEC_FREQ3_2),
> -	VEC_REG(VEC_FREQ1_0),
> -	VEC_REG(VEC_CONFIG1),
> -	VEC_REG(VEC_CONFIG2),
> -	VEC_REG(VEC_INTERRUPT_CONTROL),
> -	VEC_REG(VEC_INTERRUPT_STATUS),
> -	VEC_REG(VEC_FCW_SECAM_B),
> -	VEC_REG(VEC_SECAM_GAIN_VAL),
> -	VEC_REG(VEC_CONFIG3),
> -	VEC_REG(VEC_STATUS0),
> -	VEC_REG(VEC_MASK0),
> -	VEC_REG(VEC_CFG),
> -	VEC_REG(VEC_DAC_TEST),
> -	VEC_REG(VEC_DAC_CONFIG),
> -	VEC_REG(VEC_DAC_MISC),
> +static const struct debugfs_reg32 vec_regs[] = {
> +	VC4_REG32(VEC_WSE_CONTROL),
> +	VC4_REG32(VEC_WSE_WSS_DATA),
> +	VC4_REG32(VEC_WSE_VPS_DATA1),
> +	VC4_REG32(VEC_WSE_VPS_CONTROL),
> +	VC4_REG32(VEC_REVID),
> +	VC4_REG32(VEC_CONFIG0),
> +	VC4_REG32(VEC_SCHPH),
> +	VC4_REG32(VEC_CLMP0_START),
> +	VC4_REG32(VEC_CLMP0_END),
> +	VC4_REG32(VEC_FREQ3_2),
> +	VC4_REG32(VEC_FREQ1_0),
> +	VC4_REG32(VEC_CONFIG1),
> +	VC4_REG32(VEC_CONFIG2),
> +	VC4_REG32(VEC_INTERRUPT_CONTROL),
> +	VC4_REG32(VEC_INTERRUPT_STATUS),
> +	VC4_REG32(VEC_FCW_SECAM_B),
> +	VC4_REG32(VEC_SECAM_GAIN_VAL),
> +	VC4_REG32(VEC_CONFIG3),
> +	VC4_REG32(VEC_STATUS0),
> +	VC4_REG32(VEC_MASK0),
> +	VC4_REG32(VEC_CFG),
> +	VC4_REG32(VEC_DAC_TEST),
> +	VC4_REG32(VEC_DAC_CONFIG),
> +	VC4_REG32(VEC_DAC_MISC),
>  };
>  
>  #ifdef CONFIG_DEBUG_FS
> @@ -261,16 +259,12 @@ int vc4_vec_debugfs_regs(struct seq_file *m, void *unused)
>  	struct drm_device *dev = node->minor->dev;
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  	struct vc4_vec *vec = vc4->vec;
> -	int i;
> +	struct drm_printer p = drm_seq_file_printer(m);
>  
>  	if (!vec)
>  		return 0;
>  
> -	for (i = 0; i < ARRAY_SIZE(vec_regs); i++) {
> -		seq_printf(m, "%s (0x%04x): 0x%08x\n",
> -			   vec_regs[i].name, vec_regs[i].reg,
> -			   VEC_READ(vec_regs[i].reg));
> -	}
> +	drm_print_regset32(&p, &vec->regset);
>  
>  	return 0;
>  }
> @@ -587,6 +581,9 @@ static int vc4_vec_bind(struct device *dev, struct device *master, void *data)
>  	vec->regs = vc4_ioremap_regs(pdev, 0);
>  	if (IS_ERR(vec->regs))
>  		return PTR_ERR(vec->regs);
> +	vec->regset.base = vec->regs;
> +	vec->regset.regs = vec_regs;
> +	vec->regset.nregs = ARRAY_SIZE(vec_regs);
>  
>  	vec->clock = devm_clk_get(dev, NULL);
>  	if (IS_ERR(vec->clock)) {
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 4/7] drm/vc4: Use drm_printer for the debugfs and runtime bo stats output.
  2019-02-20 21:03 ` [PATCH 4/7] drm/vc4: Use drm_printer for the debugfs and runtime bo stats output Eric Anholt
@ 2019-03-22 10:41   ` Paul Kocialkowski
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Kocialkowski @ 2019-03-22 10:41 UTC (permalink / raw)
  To: Eric Anholt, dri-devel; +Cc: linux-kernel, Maxime Ripard

Hi,

Le mercredi 20 février 2019 à 13:03 -0800, Eric Anholt a écrit :
> Now I can extend the stats without more copy and pasting between the
> two.

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/vc4/vc4_bo.c | 48 +++++++++++-------------------------
>  1 file changed, 14 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
> index 8c509d560bf0..88ebd681d7eb 100644
> --- a/drivers/gpu/drm/vc4/vc4_bo.c
> +++ b/drivers/gpu/drm/vc4/vc4_bo.c
> @@ -40,7 +40,7 @@ static bool is_user_label(int label)
>  	return label >= VC4_BO_TYPE_COUNT;
>  }
>  
> -static void vc4_bo_stats_dump(struct vc4_dev *vc4)
> +static void vc4_bo_stats_print(struct drm_printer *p, struct vc4_dev *vc4)
>  {
>  	int i;
>  
> @@ -48,21 +48,21 @@ static void vc4_bo_stats_dump(struct vc4_dev *vc4)
>  		if (!vc4->bo_labels[i].num_allocated)
>  			continue;
>  
> -		DRM_INFO("%30s: %6dkb BOs (%d)\n",
> -			 vc4->bo_labels[i].name,
> -			 vc4->bo_labels[i].size_allocated / 1024,
> -			 vc4->bo_labels[i].num_allocated);
> +		drm_printf(p, "%30s: %6dkb BOs (%d)\n",
> +			   vc4->bo_labels[i].name,
> +			   vc4->bo_labels[i].size_allocated / 1024,
> +			   vc4->bo_labels[i].num_allocated);
>  	}
>  
>  	mutex_lock(&vc4->purgeable.lock);
>  	if (vc4->purgeable.num)
> -		DRM_INFO("%30s: %6zdkb BOs (%d)\n", "userspace BO cache",
> -			 vc4->purgeable.size / 1024, vc4->purgeable.num);
> +		drm_printf(p, "%30s: %6zdkb BOs (%d)\n", "userspace BO cache",
> +			   vc4->purgeable.size / 1024, vc4->purgeable.num);
>  
>  	if (vc4->purgeable.purged_num)
> -		DRM_INFO("%30s: %6zdkb BOs (%d)\n", "total purged BO",
> -			 vc4->purgeable.purged_size / 1024,
> -			 vc4->purgeable.purged_num);
> +		drm_printf(p, "%30s: %6zdkb BOs (%d)\n", "total purged BO",
> +			   vc4->purgeable.purged_size / 1024,
> +			   vc4->purgeable.purged_num);
>  	mutex_unlock(&vc4->purgeable.lock);
>  }
>  
> @@ -71,30 +71,9 @@ static int vc4_bo_stats_debugfs(struct seq_file *m, void *unused)
>  	struct drm_info_node *node = (struct drm_info_node *)m->private;
>  	struct drm_device *dev = node->minor->dev;
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> -	int i;
> -
> -	mutex_lock(&vc4->bo_lock);
> -	for (i = 0; i < vc4->num_labels; i++) {
> -		if (!vc4->bo_labels[i].num_allocated)
> -			continue;
> -
> -		seq_printf(m, "%30s: %6dkb BOs (%d)\n",
> -			   vc4->bo_labels[i].name,
> -			   vc4->bo_labels[i].size_allocated / 1024,
> -			   vc4->bo_labels[i].num_allocated);
> -	}
> -	mutex_unlock(&vc4->bo_lock);
> +	struct drm_printer p = drm_seq_file_printer(m);
>  
> -	mutex_lock(&vc4->purgeable.lock);
> -	if (vc4->purgeable.num)
> -		seq_printf(m, "%30s: %6zdkb BOs (%d)\n", "userspace BO cache",
> -			   vc4->purgeable.size / 1024, vc4->purgeable.num);
> -
> -	if (vc4->purgeable.purged_num)
> -		seq_printf(m, "%30s: %6zdkb BOs (%d)\n", "total purged BO",
> -			   vc4->purgeable.purged_size / 1024,
> -			   vc4->purgeable.purged_num);
> -	mutex_unlock(&vc4->purgeable.lock);
> +	vc4_bo_stats_print(&p, vc4);
>  
>  	return 0;
>  }
> @@ -473,8 +452,9 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size,
>  	}
>  
>  	if (IS_ERR(cma_obj)) {
> +		struct drm_printer p = drm_info_printer(vc4->dev->dev);
>  		DRM_ERROR("Failed to allocate from CMA:\n");
> -		vc4_bo_stats_dump(vc4);
> +		vc4_bo_stats_print(&p, vc4);
>  		return ERR_PTR(-ENOMEM);
>  	}
>  	bo = to_vc4_bo(&cma_obj->base);
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

* Re: [PATCH 7/7] drm/vc4: Make sure that the v3d ident debugfs has vc4's power on.
  2019-02-20 21:03 ` [PATCH 7/7] drm/vc4: Make sure that the v3d ident debugfs has vc4's power on Eric Anholt
@ 2019-03-22 10:43   ` Paul Kocialkowski
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Kocialkowski @ 2019-03-22 10:43 UTC (permalink / raw)
  To: Eric Anholt, dri-devel; +Cc: linux-kernel, Maxime Ripard

Hi,

Le mercredi 20 février 2019 à 13:03 -0800, Eric Anholt a écrit :
> Otherwise, you sometimes decode the ident fields based on 0xdeadbeef
> register reads.

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/vc4/vc4_v3d.c | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
> index 7820b8eaaa98..36e6c7086ecf 100644
> --- a/drivers/gpu/drm/vc4/vc4_v3d.c
> +++ b/drivers/gpu/drm/vc4/vc4_v3d.c
> @@ -108,18 +108,23 @@ static int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
>  	struct drm_info_node *node = (struct drm_info_node *)m->private;
>  	struct drm_device *dev = node->minor->dev;
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> -	uint32_t ident1 = V3D_READ(V3D_IDENT1);
> -	uint32_t nslc = VC4_GET_FIELD(ident1, V3D_IDENT1_NSLC);
> -	uint32_t tups = VC4_GET_FIELD(ident1, V3D_IDENT1_TUPS);
> -	uint32_t qups = VC4_GET_FIELD(ident1, V3D_IDENT1_QUPS);
> -
> -	seq_printf(m, "Revision:   %d\n",
> -		   VC4_GET_FIELD(ident1, V3D_IDENT1_REV));
> -	seq_printf(m, "Slices:     %d\n", nslc);
> -	seq_printf(m, "TMUs:       %d\n", nslc * tups);
> -	seq_printf(m, "QPUs:       %d\n", nslc * qups);
> -	seq_printf(m, "Semaphores: %d\n",
> -		   VC4_GET_FIELD(ident1, V3D_IDENT1_NSEM));
> +	int ret = vc4_v3d_pm_get(vc4);
> +
> +	if (ret == 0) {
> +		uint32_t ident1 = V3D_READ(V3D_IDENT1);
> +		uint32_t nslc = VC4_GET_FIELD(ident1, V3D_IDENT1_NSLC);
> +		uint32_t tups = VC4_GET_FIELD(ident1, V3D_IDENT1_TUPS);
> +		uint32_t qups = VC4_GET_FIELD(ident1, V3D_IDENT1_QUPS);
> +
> +		seq_printf(m, "Revision:   %d\n",
> +			   VC4_GET_FIELD(ident1, V3D_IDENT1_REV));
> +		seq_printf(m, "Slices:     %d\n", nslc);
> +		seq_printf(m, "TMUs:       %d\n", nslc * tups);
> +		seq_printf(m, "QPUs:       %d\n", nslc * qups);
> +		seq_printf(m, "Semaphores: %d\n",
> +			   VC4_GET_FIELD(ident1, V3D_IDENT1_NSEM));
> +		vc4_v3d_pm_put(vc4);
> +	}
>  
>  	return 0;
>  }
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

* Re: [PATCH 6/7] drm/vc4: Add helpers for pm get/put.
  2019-02-20 21:03 ` [PATCH 6/7] drm/vc4: Add helpers for pm get/put Eric Anholt
@ 2019-03-22 10:48   ` Paul Kocialkowski
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Kocialkowski @ 2019-03-22 10:48 UTC (permalink / raw)
  To: Eric Anholt, dri-devel; +Cc: linux-kernel, Maxime Ripard

Hi,

Le mercredi 20 février 2019 à 13:03 -0800, Eric Anholt a écrit :
> This makes sure the vc4_reset doesn't hit an obscure race with the
> GET_PARAM ioctl, fixes a decrement outside of the lock, and prevents
> future code from making mistakes with the weird return value of
> pm_runtime_get_sync().

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/vc4/vc4_drv.c | 21 +++++++++------------
>  drivers/gpu/drm/vc4/vc4_drv.h |  2 ++
>  drivers/gpu/drm/vc4/vc4_gem.c | 21 +++++----------------
>  drivers/gpu/drm/vc4/vc4_v3d.c | 33 +++++++++++++++++++++++++++++++++
>  4 files changed, 49 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index de1d0ce11831..f95891a61d52 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -77,28 +77,25 @@ static int vc4_get_param_ioctl(struct drm_device *dev, void *data,
>  
>  	switch (args->param) {
>  	case DRM_VC4_PARAM_V3D_IDENT0:
> -		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
> -		if (ret < 0)
> +		ret = vc4_v3d_pm_get(vc4);
> +		if (ret)
>  			return ret;
>  		args->value = V3D_READ(V3D_IDENT0);
> -		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
> -		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
> +		vc4_v3d_pm_put(vc4);
>  		break;
>  	case DRM_VC4_PARAM_V3D_IDENT1:
> -		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
> -		if (ret < 0)
> +		ret = vc4_v3d_pm_get(vc4);
> +		if (ret)
>  			return ret;
>  		args->value = V3D_READ(V3D_IDENT1);
> -		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
> -		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
> +		vc4_v3d_pm_put(vc4);
>  		break;
>  	case DRM_VC4_PARAM_V3D_IDENT2:
> -		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
> -		if (ret < 0)
> +		ret = vc4_v3d_pm_get(vc4);
> +		if (ret)
>  			return ret;
>  		args->value = V3D_READ(V3D_IDENT2);
> -		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
> -		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
> +		vc4_v3d_pm_put(vc4);
>  		break;
>  	case DRM_VC4_PARAM_SUPPORTS_BRANCHES:
>  	case DRM_VC4_PARAM_SUPPORTS_ETC1:
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 495c5a13a948..c71988b270bc 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -809,6 +809,8 @@ void vc4_plane_async_set_fb(struct drm_plane *plane,
>  /* vc4_v3d.c */
>  extern struct platform_driver vc4_v3d_driver;
>  int vc4_v3d_get_bin_slot(struct vc4_dev *vc4);
> +int vc4_v3d_pm_get(struct vc4_dev *vc4);
> +void vc4_v3d_pm_put(struct vc4_dev *vc4);
>  
>  /* vc4_validate.c */
>  int
> diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
> index 8d816e5ed762..4544a478f106 100644
> --- a/drivers/gpu/drm/vc4/vc4_gem.c
> +++ b/drivers/gpu/drm/vc4/vc4_gem.c
> @@ -969,12 +969,7 @@ vc4_complete_exec(struct drm_device *dev, struct vc4_exec_info *exec)
>  	/* Release the reference we had on the perf monitor. */
>  	vc4_perfmon_put(exec->perfmon);
>  
> -	mutex_lock(&vc4->power_lock);
> -	if (--vc4->power_refcount == 0) {
> -		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
> -		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
> -	}
> -	mutex_unlock(&vc4->power_lock);
> +	vc4_v3d_pm_put(vc4);
>  
>  	kfree(exec);
>  }
> @@ -1153,17 +1148,11 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
>  		return -ENOMEM;
>  	}
>  
> -	mutex_lock(&vc4->power_lock);
> -	if (vc4->power_refcount++ == 0) {
> -		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
> -		if (ret < 0) {
> -			mutex_unlock(&vc4->power_lock);
> -			vc4->power_refcount--;
> -			kfree(exec);
> -			return ret;
> -		}
> +	ret = vc4_v3d_pm_get(vc4);
> +	if (ret) {
> +		kfree(exec);
> +		return ret;
>  	}
> -	mutex_unlock(&vc4->power_lock);
>  
>  	exec->args = args;
>  	INIT_LIST_HEAD(&exec->unref_list);
> diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
> index f008584eb739..7820b8eaaa98 100644
> --- a/drivers/gpu/drm/vc4/vc4_v3d.c
> +++ b/drivers/gpu/drm/vc4/vc4_v3d.c
> @@ -124,6 +124,39 @@ static int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
>  	return 0;
>  }
>  
> +/**
> + * Wraps pm_runtime_get_sync() in a refcount, so that we can reliably
> + * get the pm_runtime refcount to 0 in vc4_reset().
> + */
> +int
> +vc4_v3d_pm_get(struct vc4_dev *vc4)
> +{
> +	mutex_lock(&vc4->power_lock);
> +	if (vc4->power_refcount++ == 0) {
> +		int ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
> +
> +		if (ret < 0) {
> +			vc4->power_refcount--;
> +			mutex_unlock(&vc4->power_lock);
> +			return ret;
> +		}
> +	}
> +	mutex_unlock(&vc4->power_lock);
> +
> +	return 0;
> +}
> +
> +void
> +vc4_v3d_pm_put(struct vc4_dev *vc4)
> +{
> +	mutex_lock(&vc4->power_lock);
> +	if (--vc4->power_refcount == 0) {
> +		pm_runtime_mark_last_busy(&vc4->v3d->pdev->dev);
> +		pm_runtime_put_autosuspend(&vc4->v3d->pdev->dev);
> +	}
> +	mutex_unlock(&vc4->power_lock);
> +}
> +
>  static void vc4_v3d_init_hw(struct drm_device *dev)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

* Re: [PATCH 5/7] drm/vc4: Disable V3D interactions if the v3d component didn't probe.
  2019-02-20 21:03 ` [PATCH 5/7] drm/vc4: Disable V3D interactions if the v3d component didn't probe Eric Anholt
@ 2019-03-25 13:30   ` Paul Kocialkowski
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Kocialkowski @ 2019-03-25 13:30 UTC (permalink / raw)
  To: Eric Anholt, dri-devel; +Cc: linux-kernel, Maxime Ripard

Hi,

On Wed, 2019-02-20 at 13:03 -0800, Eric Anholt wrote:
> One might want to use the VC4 display stack without using Mesa.
> Similar to the debugfs fixes for not having all of the possible
> display bits enabled, make sure you can't oops in vc4 if v3d isn't
> enabled.

See a comment below.

> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/vc4/vc4_drv.c     | 11 +++++++++++
>  drivers/gpu/drm/vc4/vc4_gem.c     | 10 ++++++++++
>  drivers/gpu/drm/vc4/vc4_irq.c     |  9 +++++++++
>  drivers/gpu/drm/vc4/vc4_perfmon.c | 18 ++++++++++++++++++
>  4 files changed, 48 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 1927d1b756f5..de1d0ce11831 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -72,6 +72,9 @@ static int vc4_get_param_ioctl(struct drm_device *dev, void *data,
>  	if (args->pad != 0)
>  		return -EINVAL;
>  
> +	if (!vc4->v3d)
> +		return -EINVAL;
> +
>  	switch (args->param) {
>  	case DRM_VC4_PARAM_V3D_IDENT0:
>  		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
> @@ -251,6 +254,7 @@ static int vc4_drm_bind(struct device *dev)
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct drm_device *drm;
>  	struct vc4_dev *vc4;
> +	struct device_node *node;
>  	int ret = 0;
>  
>  	dev->coherent_dma_mask = DMA_BIT_MASK(32);
> @@ -259,6 +263,13 @@ static int vc4_drm_bind(struct device *dev)
>  	if (!vc4)
>  		return -ENOMEM;
>  
> +	/* If VC4 V3D is missing, don't advertise render nodes. */
> +	node = of_find_compatible_node(NULL, NULL, "brcm,bcm2835-v3d");

It looks like we support more compatibles than this one, so it could be
worth having a separate helper to check if we can find a v3d-compatible 
node. It could certainly reuse the platform driver's of_match_table;

With that fixed, this is:
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> +	if (node)
> +		of_node_put(node);
> +	else
> +		vc4_drm_driver.driver_features &= ~DRIVER_RENDER;
> +
>  	drm = drm_dev_alloc(&vc4_drm_driver, dev);
>  	if (IS_ERR(drm))
>  		return PTR_ERR(drm);
> diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
> index 5ee5bf7fedf7..8d816e5ed762 100644
> --- a/drivers/gpu/drm/vc4/vc4_gem.c
> +++ b/drivers/gpu/drm/vc4/vc4_gem.c
> @@ -74,6 +74,11 @@ vc4_get_hang_state_ioctl(struct drm_device *dev, void *data,
>  	u32 i;
>  	int ret = 0;
>  
> +	if (!vc4->v3d) {
> +		DRM_DEBUG("VC4_GET_HANG_STATE with no VC4 V3D probed\n");
> +		return -EINVAL;
> +	}
> +
>  	spin_lock_irqsave(&vc4->job_lock, irqflags);
>  	kernel_state = vc4->hang_state;
>  	if (!kernel_state) {
> @@ -1124,6 +1129,11 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data,
>  	struct dma_fence *in_fence;
>  	int ret = 0;
>  
> +	if (!vc4->v3d) {
> +		DRM_DEBUG("VC4_SUBMIT_CL with no VC4 V3D probed\n");
> +		return -EINVAL;
> +	}
> +
>  	if ((args->flags & ~(VC4_SUBMIT_CL_USE_CLEAR_COLOR |
>  			     VC4_SUBMIT_CL_FIXED_RCL_ORDER |
>  			     VC4_SUBMIT_CL_RCL_ORDER_INCREASING_X |
> diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
> index 4cd2ccfe15f4..ffd0a4388752 100644
> --- a/drivers/gpu/drm/vc4/vc4_irq.c
> +++ b/drivers/gpu/drm/vc4/vc4_irq.c
> @@ -229,6 +229,9 @@ vc4_irq_preinstall(struct drm_device *dev)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  
> +	if (!vc4->v3d)
> +		return;
> +
>  	init_waitqueue_head(&vc4->job_wait_queue);
>  	INIT_WORK(&vc4->overflow_mem_work, vc4_overflow_mem_work);
>  
> @@ -243,6 +246,9 @@ vc4_irq_postinstall(struct drm_device *dev)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  
> +	if (!vc4->v3d)
> +		return 0;
> +
>  	/* Enable both the render done and out of memory interrupts. */
>  	V3D_WRITE(V3D_INTENA, V3D_DRIVER_IRQS);
>  
> @@ -254,6 +260,9 @@ vc4_irq_uninstall(struct drm_device *dev)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  
> +	if (!vc4->v3d)
> +		return;
> +
>  	/* Disable sending interrupts for our driver's IRQs. */
>  	V3D_WRITE(V3D_INTDIS, V3D_DRIVER_IRQS);
>  
> diff --git a/drivers/gpu/drm/vc4/vc4_perfmon.c b/drivers/gpu/drm/vc4/vc4_perfmon.c
> index 495150415020..6562d3d30b20 100644
> --- a/drivers/gpu/drm/vc4/vc4_perfmon.c
> +++ b/drivers/gpu/drm/vc4/vc4_perfmon.c
> @@ -100,12 +100,18 @@ void vc4_perfmon_close_file(struct vc4_file *vc4file)
>  int vc4_perfmon_create_ioctl(struct drm_device *dev, void *data,
>  			     struct drm_file *file_priv)
>  {
> +	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  	struct vc4_file *vc4file = file_priv->driver_priv;
>  	struct drm_vc4_perfmon_create *req = data;
>  	struct vc4_perfmon *perfmon;
>  	unsigned int i;
>  	int ret;
>  
> +	if (!vc4->v3d) {
> +		DRM_DEBUG("Creating perfmon no VC4 V3D probed\n");
> +		return -EINVAL;
> +	}
> +
>  	/* Number of monitored counters cannot exceed HW limits. */
>  	if (req->ncounters > DRM_VC4_MAX_PERF_COUNTERS ||
>  	    !req->ncounters)
> @@ -146,10 +152,16 @@ int vc4_perfmon_create_ioctl(struct drm_device *dev, void *data,
>  int vc4_perfmon_destroy_ioctl(struct drm_device *dev, void *data,
>  			      struct drm_file *file_priv)
>  {
> +	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  	struct vc4_file *vc4file = file_priv->driver_priv;
>  	struct drm_vc4_perfmon_destroy *req = data;
>  	struct vc4_perfmon *perfmon;
>  
> +	if (!vc4->v3d) {
> +		DRM_DEBUG("Destroying perfmon no VC4 V3D probed\n");
> +		return -EINVAL;
> +	}
> +
>  	mutex_lock(&vc4file->perfmon.lock);
>  	perfmon = idr_remove(&vc4file->perfmon.idr, req->id);
>  	mutex_unlock(&vc4file->perfmon.lock);
> @@ -164,11 +176,17 @@ int vc4_perfmon_destroy_ioctl(struct drm_device *dev, void *data,
>  int vc4_perfmon_get_values_ioctl(struct drm_device *dev, void *data,
>  				 struct drm_file *file_priv)
>  {
> +	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  	struct vc4_file *vc4file = file_priv->driver_priv;
>  	struct drm_vc4_perfmon_get_values *req = data;
>  	struct vc4_perfmon *perfmon;
>  	int ret;
>  
> +	if (!vc4->v3d) {
> +		DRM_DEBUG("Getting perfmon no VC4 V3D probed\n");
> +		return -EINVAL;
> +	}
> +
>  	mutex_lock(&vc4file->perfmon.lock);
>  	perfmon = idr_find(&vc4file->perfmon.idr, req->id);
>  	vc4_perfmon_get(perfmon);
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

* Re: [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32.
  2019-02-21  9:37 ` [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Daniel Vetter
@ 2019-03-25 16:43   ` Eric Anholt
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Anholt @ 2019-03-25 16:43 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel, Paul Kocialkowski, Maxime Ripard, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1436 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> On Wed, Feb 20, 2019 at 01:03:37PM -0800, Eric Anholt wrote:
>> The debugfs_regset32 is nice to use for reducing boilerplate in
>> dumping a bunch of regs in debugfs, but we also want to be able to
>> print to dmesg them at runtime for driver debugging.  drm_printer lets
>> us format debugfs and the printk the same way.
>> 
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>>  drivers/gpu/drm/drm_print.c | 16 ++++++++++++++++
>>  include/drm/drm_print.h     |  2 ++
>>  2 files changed, 18 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
>> index 0e7fc3e7dfb4..5ecc0f04cd0c 100644
>> --- a/drivers/gpu/drm/drm_print.c
>> +++ b/drivers/gpu/drm/drm_print.c
>> @@ -253,3 +253,19 @@ void drm_err(const char *format, ...)
>>  	va_end(args);
>>  }
>>  EXPORT_SYMBOL(drm_err);
>> +
>
> A bit of kerneldoc would be nice. With that:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

+/**
+ * drm_print_regset32 - print the contents of registers to a
+ * &drm_printer stream.
+ *
+ * @p: the &drm printer
+ * @regset: the list of registers to print.
+ *
+ * Often in driver debug, it's useful to be able to either capture the
+ * contents of registers in the steady state using debugfs or at
+ * specific points during operation.  This lets the driver have a
+ * single list of registers for both.
+ */

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

* Re: [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32.
  2019-03-22 10:30 ` Paul Kocialkowski
@ 2019-04-01 17:52   ` Eric Anholt
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Anholt @ 2019-04-01 17:52 UTC (permalink / raw)
  To: Paul Kocialkowski, dri-devel; +Cc: linux-kernel, Maxime Ripard

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

Paul Kocialkowski <paul.kocialkowski@bootlin.com> writes:

> Hi,
>
> Le mercredi 20 février 2019 à 13:03 -0800, Eric Anholt a écrit :
>> The debugfs_regset32 is nice to use for reducing boilerplate in
>> dumping a bunch of regs in debugfs, but we also want to be able to
>> print to dmesg them at runtime for driver debugging.  drm_printer lets
>> us format debugfs and the printk the same way.
>
> Looks good to me!
>
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Thanks!  Merged all but patch 3 (unreviewed) and patch 5 (needs patch 3
to avoid oopsing).  Sending out a rebased version shortly.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

end of thread, other threads:[~2019-04-01 17:52 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-20 21:03 [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Eric Anholt
2019-02-20 21:03 ` [PATCH 2/7] drm/vc4: Use drm_print_regset32() for our debug register dumping Eric Anholt
2019-03-22 10:32   ` Paul Kocialkowski
2019-02-20 21:03 ` [PATCH 3/7] drm/vc4: Use common helpers for debugfs setup by the driver components Eric Anholt
2019-02-20 21:03 ` [PATCH 4/7] drm/vc4: Use drm_printer for the debugfs and runtime bo stats output Eric Anholt
2019-03-22 10:41   ` Paul Kocialkowski
2019-02-20 21:03 ` [PATCH 5/7] drm/vc4: Disable V3D interactions if the v3d component didn't probe Eric Anholt
2019-03-25 13:30   ` Paul Kocialkowski
2019-02-20 21:03 ` [PATCH 6/7] drm/vc4: Add helpers for pm get/put Eric Anholt
2019-03-22 10:48   ` Paul Kocialkowski
2019-02-20 21:03 ` [PATCH 7/7] drm/vc4: Make sure that the v3d ident debugfs has vc4's power on Eric Anholt
2019-03-22 10:43   ` Paul Kocialkowski
2019-02-21  9:37 ` [PATCH 1/7] drm: Add a helper function for printing a debugfs_regset32 Daniel Vetter
2019-03-25 16:43   ` Eric Anholt
2019-03-22 10:30 ` Paul Kocialkowski
2019-04-01 17:52   ` Eric Anholt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).