devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/7] drm/v3d: Fix GPU reset issues on the Raspberry Pi 5
@ 2025-03-13 14:43 Maíra Canal
  2025-03-13 14:43 ` [PATCH v4 1/7] drm/v3d: Don't run jobs that have errors flagged in its fence Maíra Canal
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Maíra Canal @ 2025-03-13 14:43 UTC (permalink / raw)
  To: Melissa Wen, Iago Toral, Krzysztof Kozlowski, Conor Dooley,
	Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, stable,
	Maíra Canal, Emma Anholt, Rob Herring (Arm)

This series addresses GPU reset issues reported in [1], where running a
long compute job would trigger repeated GPU resets, leading to a UI
freeze.

Patches #1 and #2 prevent the same faulty job from being resubmitted in a
loop, mitigating the first cause of the issue.

However, the issue isn't entirely solved. Even with only a single GPU
reset, the UI still freezes on the Raspberry Pi 5, indicating a GPU hang.
Patches #3 to #6 address this by properly configuring the V3D_SMS
registers, which are required for power management and resets in V3D 7.1.

Patch #7 updates the DT maintainership, replacing Emma with the current
v3d driver maintainer.

[1] https://github.com/raspberrypi/linux/issues/6660

Best Regards,
- Maíra

---
v1 -> v2:
- [1/6, 2/6, 5/6] Add Iago's R-b (Iago Toral)
- [3/6] Use V3D_GEN_* macros consistently throughout the driver (Phil Elwell)
- [3/6] Don't add Iago's R-b in 3/6 due to changes in the patch
- [4/6] Add per-compatible restrictions to enforce per‐SoC register rules (Conor Dooley)
- [6/6] Add Emma's A-b, collected through IRC (Emma Anholt)
- [6/6] Add Rob's A-b (Rob Herring)
- Link to v1: https://lore.kernel.org/r/20250226-v3d-gpu-reset-fixes-v1-0-83a969fdd9c1@igalia.com

v2 -> v3:
- [3/7] Add Iago's R-b (Iago Toral)
- [4/7, 5/7] Separate the patches to ease the reviewing process -> Now,
  PATCH 4/7 only adds the per-compatible rules and PATCH 5/7 adds the
  SMS registers
- [4/7] `allOf` goes above `additionalProperties` (Krzysztof Kozlowski)
- [4/7, 5/7] Sync `reg` and `reg-names` items (Krzysztof Kozlowski)
- Link to v2: https://lore.kernel.org/r/20250308-v3d-gpu-reset-fixes-v2-0-2939c30f0cc4@igalia.com

v3 -> v4:
- [4/7] BCM2712 has an external reset controller, therefore the "bridge"
	register is not needed (Krzysztof Kozlowski)
- [4/7] Remove the word "required" from the reg descriptions (Rob Herring)
- [5/7] Improve commit message (Rob Herring)
- Link to v3: https://lore.kernel.org/r/20250311-v3d-gpu-reset-fixes-v3-0-64f7a4247ec0@igalia.com

---
Maíra Canal (7):
      drm/v3d: Don't run jobs that have errors flagged in its fence
      drm/v3d: Set job pointer to NULL when the job's fence has an error
      drm/v3d: Associate a V3D tech revision to all supported devices
      dt-bindings: gpu: v3d: Add per-compatible register restrictions
      dt-bindings: gpu: v3d: Add SMS register to BCM2712 compatible
      drm/v3d: Use V3D_SMS registers for power on/off and reset on V3D 7.x
      dt-bindings: gpu: Add V3D driver maintainer as DT maintainer

 .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      |  77 ++++++++++---
 drivers/gpu/drm/v3d/v3d_debugfs.c                  | 126 ++++++++++-----------
 drivers/gpu/drm/v3d/v3d_drv.c                      |  62 +++++++++-
 drivers/gpu/drm/v3d/v3d_drv.h                      |  22 +++-
 drivers/gpu/drm/v3d/v3d_gem.c                      |  27 ++++-
 drivers/gpu/drm/v3d/v3d_irq.c                      |   6 +-
 drivers/gpu/drm/v3d/v3d_perfmon.c                  |   4 +-
 drivers/gpu/drm/v3d/v3d_regs.h                     |  26 +++++
 drivers/gpu/drm/v3d/v3d_sched.c                    |  29 ++++-
 9 files changed, 279 insertions(+), 100 deletions(-)
---
base-commit: 10646ddac2917b31c985ceff0e4982c42a9c924b
change-id: 20250224-v3d-gpu-reset-fixes-2d21fc70711d


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

* [PATCH v4 1/7] drm/v3d: Don't run jobs that have errors flagged in its fence
  2025-03-13 14:43 [PATCH v4 0/7] drm/v3d: Fix GPU reset issues on the Raspberry Pi 5 Maíra Canal
@ 2025-03-13 14:43 ` Maíra Canal
  2025-03-13 20:20   ` Maíra Canal
  2025-03-13 14:43 ` [PATCH v4 2/7] drm/v3d: Set job pointer to NULL when the job's fence has an error Maíra Canal
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Maíra Canal @ 2025-03-13 14:43 UTC (permalink / raw)
  To: Melissa Wen, Iago Toral, Krzysztof Kozlowski, Conor Dooley,
	Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, stable,
	Maíra Canal

The V3D driver still relies on `drm_sched_increase_karma()` and
`drm_sched_resubmit_jobs()` for resubmissions when a timeout occurs.
The function `drm_sched_increase_karma()` marks the job as guilty, while
`drm_sched_resubmit_jobs()` sets an error (-ECANCELED) in the DMA fence of
that guilty job.

Because of this, we must check whether the job’s DMA fence has been
flagged with an error before executing the job. Otherwise, the same guilty
job may be resubmitted indefinitely, causing repeated GPU resets.

This patch adds a check for an error on the job's fence to prevent running
a guilty job that was previously flagged when the GPU timed out.

Note that the CPU and CACHE_CLEAN queues do not require this check, as
their jobs are executed synchronously once the DRM scheduler starts them.

Cc: stable@vger.kernel.org
Fixes: d223f98f0209 ("drm/v3d: Add support for compute shader dispatch.")
Fixes: 1584f16ca96e ("drm/v3d: Add support for submitting jobs to the TFU.")
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_sched.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 80466ce8c7df669280e556c0793490b79e75d2c7..c2010ecdb08f4ba3b54f7783ed33901552d0eba1 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -327,11 +327,15 @@ v3d_tfu_job_run(struct drm_sched_job *sched_job)
 	struct drm_device *dev = &v3d->drm;
 	struct dma_fence *fence;
 
+	if (unlikely(job->base.base.s_fence->finished.error))
+		return NULL;
+
+	v3d->tfu_job = job;
+
 	fence = v3d_fence_create(v3d, V3D_TFU);
 	if (IS_ERR(fence))
 		return NULL;
 
-	v3d->tfu_job = job;
 	if (job->base.irq_fence)
 		dma_fence_put(job->base.irq_fence);
 	job->base.irq_fence = dma_fence_get(fence);
@@ -369,6 +373,9 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
 	struct dma_fence *fence;
 	int i, csd_cfg0_reg;
 
+	if (unlikely(job->base.base.s_fence->finished.error))
+		return NULL;
+
 	v3d->csd_job = job;
 
 	v3d_invalidate_caches(v3d);

-- 
Git-154)


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

* [PATCH v4 2/7] drm/v3d: Set job pointer to NULL when the job's fence has an error
  2025-03-13 14:43 [PATCH v4 0/7] drm/v3d: Fix GPU reset issues on the Raspberry Pi 5 Maíra Canal
  2025-03-13 14:43 ` [PATCH v4 1/7] drm/v3d: Don't run jobs that have errors flagged in its fence Maíra Canal
@ 2025-03-13 14:43 ` Maíra Canal
  2025-03-13 14:43 ` [PATCH v4 3/7] drm/v3d: Associate a V3D tech revision to all supported devices Maíra Canal
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Maíra Canal @ 2025-03-13 14:43 UTC (permalink / raw)
  To: Melissa Wen, Iago Toral, Krzysztof Kozlowski, Conor Dooley,
	Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, Maíra Canal

Similar to commit e4b5ccd392b9 ("drm/v3d: Ensure job pointer is set to
NULL after job completion"), ensure the job pointer is set to `NULL` when
a job's fence has an error. Failing to do so can trigger kernel warnings
in specific scenarios, such as:

1. v3d_csd_job_run() assigns `v3d->csd_job = job`
2. CSD job exceeds hang limit, causing a timeout → v3d_gpu_reset_for_timeout()
3. GPU reset
4. drm_sched_resubmit_jobs() sets the job's fence to `-ECANCELED`.
5. v3d_csd_job_run() detects the fence error and returns NULL, not
   submitting the job to the GPU
6. User-space runs `modprobe -r v3d`
7. v3d_gem_destroy()

v3d_gem_destroy() triggers a warning indicating that the CSD job never
ended, as we didn't set `v3d->csd_job` to NULL after the timeout. The same
can also happen to BIN, RENDER, and TFU jobs.

Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_sched.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index c2010ecdb08f4ba3b54f7783ed33901552d0eba1..34c42d6e12cde656d3b51a18be324976199eceae 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -226,8 +226,12 @@ static struct dma_fence *v3d_bin_job_run(struct drm_sched_job *sched_job)
 	struct dma_fence *fence;
 	unsigned long irqflags;
 
-	if (unlikely(job->base.base.s_fence->finished.error))
+	if (unlikely(job->base.base.s_fence->finished.error)) {
+		spin_lock_irqsave(&v3d->job_lock, irqflags);
+		v3d->bin_job = NULL;
+		spin_unlock_irqrestore(&v3d->job_lock, irqflags);
 		return NULL;
+	}
 
 	/* Lock required around bin_job update vs
 	 * v3d_overflow_mem_work().
@@ -281,8 +285,10 @@ static struct dma_fence *v3d_render_job_run(struct drm_sched_job *sched_job)
 	struct drm_device *dev = &v3d->drm;
 	struct dma_fence *fence;
 
-	if (unlikely(job->base.base.s_fence->finished.error))
+	if (unlikely(job->base.base.s_fence->finished.error)) {
+		v3d->render_job = NULL;
 		return NULL;
+	}
 
 	v3d->render_job = job;
 
@@ -327,8 +333,10 @@ v3d_tfu_job_run(struct drm_sched_job *sched_job)
 	struct drm_device *dev = &v3d->drm;
 	struct dma_fence *fence;
 
-	if (unlikely(job->base.base.s_fence->finished.error))
+	if (unlikely(job->base.base.s_fence->finished.error)) {
+		v3d->tfu_job = NULL;
 		return NULL;
+	}
 
 	v3d->tfu_job = job;
 
@@ -373,8 +381,10 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
 	struct dma_fence *fence;
 	int i, csd_cfg0_reg;
 
-	if (unlikely(job->base.base.s_fence->finished.error))
+	if (unlikely(job->base.base.s_fence->finished.error)) {
+		v3d->csd_job = NULL;
 		return NULL;
+	}
 
 	v3d->csd_job = job;
 

-- 
Git-154)


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

* [PATCH v4 3/7] drm/v3d: Associate a V3D tech revision to all supported devices
  2025-03-13 14:43 [PATCH v4 0/7] drm/v3d: Fix GPU reset issues on the Raspberry Pi 5 Maíra Canal
  2025-03-13 14:43 ` [PATCH v4 1/7] drm/v3d: Don't run jobs that have errors flagged in its fence Maíra Canal
  2025-03-13 14:43 ` [PATCH v4 2/7] drm/v3d: Set job pointer to NULL when the job's fence has an error Maíra Canal
@ 2025-03-13 14:43 ` Maíra Canal
  2025-03-13 14:43 ` [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions Maíra Canal
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Maíra Canal @ 2025-03-13 14:43 UTC (permalink / raw)
  To: Melissa Wen, Iago Toral, Krzysztof Kozlowski, Conor Dooley,
	Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, Maíra Canal

The V3D driver currently determines the GPU tech version (33, 41...)
by reading a register. This approach has worked so far since this
information wasn’t needed before powering on the GPU.

V3D 7.1 introduces new registers that must be written to power on the
GPU, requiring us to know the V3D version beforehand. To address this,
associate each supported SoC with the corresponding VideoCore GPU version
as part of the device data.

To prevent possible mistakes, add an assertion to verify that the version
specified in the device data matches the one reported by the hardware.
If there is a mismatch, the kernel will trigger a warning.

Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_debugfs.c | 126 +++++++++++++++++++-------------------
 drivers/gpu/drm/v3d/v3d_drv.c     |  22 +++++--
 drivers/gpu/drm/v3d/v3d_drv.h     |  11 +++-
 drivers/gpu/drm/v3d/v3d_gem.c     |  10 +--
 drivers/gpu/drm/v3d/v3d_irq.c     |   6 +-
 drivers/gpu/drm/v3d/v3d_perfmon.c |   4 +-
 drivers/gpu/drm/v3d/v3d_sched.c   |   6 +-
 7 files changed, 101 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c
index 76816f2551c10026a775e4331ad7eb2f008cfb0a..7e789e181af0ac138044f194a29555c30ab01836 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -21,74 +21,74 @@ struct v3d_reg_def {
 };
 
 static const struct v3d_reg_def v3d_hub_reg_defs[] = {
-	REGDEF(33, 42, V3D_HUB_AXICFG),
-	REGDEF(33, 71, V3D_HUB_UIFCFG),
-	REGDEF(33, 71, V3D_HUB_IDENT0),
-	REGDEF(33, 71, V3D_HUB_IDENT1),
-	REGDEF(33, 71, V3D_HUB_IDENT2),
-	REGDEF(33, 71, V3D_HUB_IDENT3),
-	REGDEF(33, 71, V3D_HUB_INT_STS),
-	REGDEF(33, 71, V3D_HUB_INT_MSK_STS),
-
-	REGDEF(33, 71, V3D_MMU_CTL),
-	REGDEF(33, 71, V3D_MMU_VIO_ADDR),
-	REGDEF(33, 71, V3D_MMU_VIO_ID),
-	REGDEF(33, 71, V3D_MMU_DEBUG_INFO),
-
-	REGDEF(71, 71, V3D_GMP_STATUS(71)),
-	REGDEF(71, 71, V3D_GMP_CFG(71)),
-	REGDEF(71, 71, V3D_GMP_VIO_ADDR(71)),
+	REGDEF(V3D_GEN_33, V3D_GEN_42, V3D_HUB_AXICFG),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_HUB_UIFCFG),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_HUB_IDENT0),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_HUB_IDENT1),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_HUB_IDENT2),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_HUB_IDENT3),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_HUB_INT_STS),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_HUB_INT_MSK_STS),
+
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_MMU_CTL),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_MMU_VIO_ADDR),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_MMU_VIO_ID),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_MMU_DEBUG_INFO),
+
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_GMP_STATUS(71)),
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_GMP_CFG(71)),
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_GMP_VIO_ADDR(71)),
 };
 
 static const struct v3d_reg_def v3d_gca_reg_defs[] = {
-	REGDEF(33, 33, V3D_GCA_SAFE_SHUTDOWN),
-	REGDEF(33, 33, V3D_GCA_SAFE_SHUTDOWN_ACK),
+	REGDEF(V3D_GEN_33, V3D_GEN_33, V3D_GCA_SAFE_SHUTDOWN),
+	REGDEF(V3D_GEN_33, V3D_GEN_33, V3D_GCA_SAFE_SHUTDOWN_ACK),
 };
 
 static const struct v3d_reg_def v3d_core_reg_defs[] = {
-	REGDEF(33, 71, V3D_CTL_IDENT0),
-	REGDEF(33, 71, V3D_CTL_IDENT1),
-	REGDEF(33, 71, V3D_CTL_IDENT2),
-	REGDEF(33, 71, V3D_CTL_MISCCFG),
-	REGDEF(33, 71, V3D_CTL_INT_STS),
-	REGDEF(33, 71, V3D_CTL_INT_MSK_STS),
-	REGDEF(33, 71, V3D_CLE_CT0CS),
-	REGDEF(33, 71, V3D_CLE_CT0CA),
-	REGDEF(33, 71, V3D_CLE_CT0EA),
-	REGDEF(33, 71, V3D_CLE_CT1CS),
-	REGDEF(33, 71, V3D_CLE_CT1CA),
-	REGDEF(33, 71, V3D_CLE_CT1EA),
-
-	REGDEF(33, 71, V3D_PTB_BPCA),
-	REGDEF(33, 71, V3D_PTB_BPCS),
-
-	REGDEF(33, 42, V3D_GMP_STATUS(33)),
-	REGDEF(33, 42, V3D_GMP_CFG(33)),
-	REGDEF(33, 42, V3D_GMP_VIO_ADDR(33)),
-
-	REGDEF(33, 71, V3D_ERR_FDBGO),
-	REGDEF(33, 71, V3D_ERR_FDBGB),
-	REGDEF(33, 71, V3D_ERR_FDBGS),
-	REGDEF(33, 71, V3D_ERR_STAT),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CTL_IDENT0),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CTL_IDENT1),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CTL_IDENT2),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CTL_MISCCFG),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CTL_INT_STS),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CTL_INT_MSK_STS),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CLE_CT0CS),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CLE_CT0CA),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CLE_CT0EA),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CLE_CT1CS),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CLE_CT1CA),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_CLE_CT1EA),
+
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_PTB_BPCA),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_PTB_BPCS),
+
+	REGDEF(V3D_GEN_33, V3D_GEN_42, V3D_GMP_STATUS(33)),
+	REGDEF(V3D_GEN_33, V3D_GEN_42, V3D_GMP_CFG(33)),
+	REGDEF(V3D_GEN_33, V3D_GEN_42, V3D_GMP_VIO_ADDR(33)),
+
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_ERR_FDBGO),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_ERR_FDBGB),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_ERR_FDBGS),
+	REGDEF(V3D_GEN_33, V3D_GEN_71, V3D_ERR_STAT),
 };
 
 static const struct v3d_reg_def v3d_csd_reg_defs[] = {
-	REGDEF(41, 71, V3D_CSD_STATUS),
-	REGDEF(41, 42, V3D_CSD_CURRENT_CFG0(41)),
-	REGDEF(41, 42, V3D_CSD_CURRENT_CFG1(41)),
-	REGDEF(41, 42, V3D_CSD_CURRENT_CFG2(41)),
-	REGDEF(41, 42, V3D_CSD_CURRENT_CFG3(41)),
-	REGDEF(41, 42, V3D_CSD_CURRENT_CFG4(41)),
-	REGDEF(41, 42, V3D_CSD_CURRENT_CFG5(41)),
-	REGDEF(41, 42, V3D_CSD_CURRENT_CFG6(41)),
-	REGDEF(71, 71, V3D_CSD_CURRENT_CFG0(71)),
-	REGDEF(71, 71, V3D_CSD_CURRENT_CFG1(71)),
-	REGDEF(71, 71, V3D_CSD_CURRENT_CFG2(71)),
-	REGDEF(71, 71, V3D_CSD_CURRENT_CFG3(71)),
-	REGDEF(71, 71, V3D_CSD_CURRENT_CFG4(71)),
-	REGDEF(71, 71, V3D_CSD_CURRENT_CFG5(71)),
-	REGDEF(71, 71, V3D_CSD_CURRENT_CFG6(71)),
-	REGDEF(71, 71, V3D_V7_CSD_CURRENT_CFG7),
+	REGDEF(V3D_GEN_41, V3D_GEN_71, V3D_CSD_STATUS),
+	REGDEF(V3D_GEN_41, V3D_GEN_42, V3D_CSD_CURRENT_CFG0(41)),
+	REGDEF(V3D_GEN_41, V3D_GEN_42, V3D_CSD_CURRENT_CFG1(41)),
+	REGDEF(V3D_GEN_41, V3D_GEN_42, V3D_CSD_CURRENT_CFG2(41)),
+	REGDEF(V3D_GEN_41, V3D_GEN_42, V3D_CSD_CURRENT_CFG3(41)),
+	REGDEF(V3D_GEN_41, V3D_GEN_42, V3D_CSD_CURRENT_CFG4(41)),
+	REGDEF(V3D_GEN_41, V3D_GEN_42, V3D_CSD_CURRENT_CFG5(41)),
+	REGDEF(V3D_GEN_41, V3D_GEN_42, V3D_CSD_CURRENT_CFG6(41)),
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_CSD_CURRENT_CFG0(71)),
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_CSD_CURRENT_CFG1(71)),
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_CSD_CURRENT_CFG2(71)),
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_CSD_CURRENT_CFG3(71)),
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_CSD_CURRENT_CFG4(71)),
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_CSD_CURRENT_CFG5(71)),
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_CSD_CURRENT_CFG6(71)),
+	REGDEF(V3D_GEN_71, V3D_GEN_71, V3D_V7_CSD_CURRENT_CFG7),
 };
 
 static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
@@ -164,7 +164,7 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
 		   str_yes_no(ident2 & V3D_HUB_IDENT2_WITH_MMU));
 	seq_printf(m, "TFU:        %s\n",
 		   str_yes_no(ident1 & V3D_HUB_IDENT1_WITH_TFU));
-	if (v3d->ver <= 42) {
+	if (v3d->ver <= V3D_GEN_42) {
 		seq_printf(m, "TSY:        %s\n",
 			   str_yes_no(ident1 & V3D_HUB_IDENT1_WITH_TSY));
 	}
@@ -196,11 +196,11 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
 		seq_printf(m, "  QPUs:         %d\n", nslc * qups);
 		seq_printf(m, "  Semaphores:   %d\n",
 			   V3D_GET_FIELD(ident1, V3D_IDENT1_NSEM));
-		if (v3d->ver <= 42) {
+		if (v3d->ver <= V3D_GEN_42) {
 			seq_printf(m, "  BCG int:      %d\n",
 				   (ident2 & V3D_IDENT2_BCG_INT) != 0);
 		}
-		if (v3d->ver < 40) {
+		if (v3d->ver < V3D_GEN_41) {
 			seq_printf(m, "  Override TMU: %d\n",
 				   (misccfg & V3D_MISCCFG_OVRTMUOUT) != 0);
 		}
@@ -234,7 +234,7 @@ static int v3d_measure_clock(struct seq_file *m, void *unused)
 	int core = 0;
 	int measure_ms = 1000;
 
-	if (v3d->ver >= 40) {
+	if (v3d->ver >= V3D_GEN_41) {
 		int cycle_count_reg = V3D_PCTR_CYCLE_COUNT(v3d->ver);
 		V3D_CORE_WRITE(core, V3D_V4_PCTR_0_SRC_0_3,
 			       V3D_SET_FIELD_VER(cycle_count_reg,
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 852015214e971c60f9939d34d893d8d8cb4e9b01..c63f0ed1bd8a3d5511085e76ed2fbd6ee7df6f80 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -17,6 +17,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/sched/clock.h>
@@ -92,7 +93,7 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data,
 		args->value = 1;
 		return 0;
 	case DRM_V3D_PARAM_SUPPORTS_PERFMON:
-		args->value = (v3d->ver >= 40);
+		args->value = (v3d->ver >= V3D_GEN_41);
 		return 0;
 	case DRM_V3D_PARAM_SUPPORTS_MULTISYNC_EXT:
 		args->value = 1;
@@ -254,10 +255,10 @@ static const struct drm_driver v3d_drm_driver = {
 };
 
 static const struct of_device_id v3d_of_match[] = {
-	{ .compatible = "brcm,2711-v3d" },
-	{ .compatible = "brcm,2712-v3d" },
-	{ .compatible = "brcm,7268-v3d" },
-	{ .compatible = "brcm,7278-v3d" },
+	{ .compatible = "brcm,2711-v3d", .data = (void *)V3D_GEN_42 },
+	{ .compatible = "brcm,2712-v3d", .data = (void *)V3D_GEN_71 },
+	{ .compatible = "brcm,7268-v3d", .data = (void *)V3D_GEN_33 },
+	{ .compatible = "brcm,7278-v3d", .data = (void *)V3D_GEN_41 },
 	{},
 };
 MODULE_DEVICE_TABLE(of, v3d_of_match);
@@ -274,6 +275,7 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct drm_device *drm;
 	struct v3d_dev *v3d;
+	enum v3d_gen gen;
 	int ret;
 	u32 mmu_debug;
 	u32 ident1, ident3;
@@ -287,6 +289,9 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, drm);
 
+	gen = (enum v3d_gen)of_device_get_match_data(dev);
+	v3d->ver = gen;
+
 	ret = map_regs(v3d, &v3d->hub_regs, "hub");
 	if (ret)
 		return ret;
@@ -316,6 +321,11 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 	ident1 = V3D_READ(V3D_HUB_IDENT1);
 	v3d->ver = (V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_TVER) * 10 +
 		    V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_REV));
+	/* Make sure that the V3D tech version retrieved from the HW is equal
+	 * to the one advertised by the device tree.
+	 */
+	WARN_ON(v3d->ver != gen);
+
 	v3d->cores = V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_NCORES);
 	WARN_ON(v3d->cores > 1); /* multicore not yet implemented */
 
@@ -340,7 +350,7 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (v3d->ver < 41) {
+	if (v3d->ver < V3D_GEN_41) {
 		ret = map_regs(v3d, &v3d->gca_regs, "gca");
 		if (ret)
 			goto clk_disable;
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 9deaefa0f95b71b842f1c5bef2c6a8a8ffc21fe2..de4a9e18f6a9039edf57f406ab1cee9dad4c0a49 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -94,11 +94,18 @@ struct v3d_perfmon {
 	u64 values[] __counted_by(ncounters);
 };
 
+enum v3d_gen {
+	V3D_GEN_33 = 33,
+	V3D_GEN_41 = 41,
+	V3D_GEN_42 = 42,
+	V3D_GEN_71 = 71,
+};
+
 struct v3d_dev {
 	struct drm_device drm;
 
 	/* Short representation (e.g. 33, 41) of the V3D tech version */
-	int ver;
+	enum v3d_gen ver;
 
 	/* Short representation (e.g. 5, 6) of the V3D tech revision */
 	int rev;
@@ -199,7 +206,7 @@ to_v3d_dev(struct drm_device *dev)
 static inline bool
 v3d_has_csd(struct v3d_dev *v3d)
 {
-	return v3d->ver >= 41;
+	return v3d->ver >= V3D_GEN_41;
 }
 
 #define v3d_to_pdev(v3d) to_platform_device((v3d)->drm.dev)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index b1e681630ded098de8aee691884368a959443812..1ea6d3832c2212d9cbbd90236478d18491f0ff14 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -25,7 +25,7 @@ v3d_init_core(struct v3d_dev *v3d, int core)
 	 * type.  If you want the default behavior, you can still put
 	 * "2" in the indirect texture state's output_type field.
 	 */
-	if (v3d->ver < 40)
+	if (v3d->ver < V3D_GEN_41)
 		V3D_CORE_WRITE(core, V3D_CTL_MISCCFG, V3D_MISCCFG_OVRTMUOUT);
 
 	/* Whenever we flush the L2T cache, we always want to flush
@@ -58,7 +58,7 @@ v3d_idle_axi(struct v3d_dev *v3d, int core)
 static void
 v3d_idle_gca(struct v3d_dev *v3d)
 {
-	if (v3d->ver >= 41)
+	if (v3d->ver >= V3D_GEN_41)
 		return;
 
 	V3D_GCA_WRITE(V3D_GCA_SAFE_SHUTDOWN, V3D_GCA_SAFE_SHUTDOWN_EN);
@@ -132,13 +132,13 @@ v3d_reset(struct v3d_dev *v3d)
 static void
 v3d_flush_l3(struct v3d_dev *v3d)
 {
-	if (v3d->ver < 41) {
+	if (v3d->ver < V3D_GEN_41) {
 		u32 gca_ctrl = V3D_GCA_READ(V3D_GCA_CACHE_CTRL);
 
 		V3D_GCA_WRITE(V3D_GCA_CACHE_CTRL,
 			      gca_ctrl | V3D_GCA_CACHE_CTRL_FLUSH);
 
-		if (v3d->ver < 33) {
+		if (v3d->ver < V3D_GEN_33) {
 			V3D_GCA_WRITE(V3D_GCA_CACHE_CTRL,
 				      gca_ctrl & ~V3D_GCA_CACHE_CTRL_FLUSH);
 		}
@@ -151,7 +151,7 @@ v3d_flush_l3(struct v3d_dev *v3d)
 static void
 v3d_invalidate_l2c(struct v3d_dev *v3d, int core)
 {
-	if (v3d->ver > 32)
+	if (v3d->ver >= V3D_GEN_33)
 		return;
 
 	V3D_CORE_WRITE(core, V3D_CTL_L2CACTL,
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index 72b6a119412fa74f8771308e228305678a19ba43..29f63f572d35b7217e346f82b9afb0957a42bd39 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -143,7 +143,7 @@ v3d_irq(int irq, void *arg)
 	/* We shouldn't be triggering these if we have GMP in
 	 * always-allowed mode.
 	 */
-	if (v3d->ver < 71 && (intsts & V3D_INT_GMPV))
+	if (v3d->ver < V3D_GEN_71 && (intsts & V3D_INT_GMPV))
 		dev_err(v3d->drm.dev, "GMP violation\n");
 
 	/* V3D 4.2 wires the hub and core IRQs together, so if we &
@@ -200,7 +200,7 @@ v3d_hub_irq(int irq, void *arg)
 
 		V3D_WRITE(V3D_MMU_CTL, V3D_READ(V3D_MMU_CTL));
 
-		if (v3d->ver >= 41) {
+		if (v3d->ver >= V3D_GEN_41) {
 			axi_id = axi_id >> 5;
 			if (axi_id < ARRAY_SIZE(v3d41_axi_ids))
 				client = v3d41_axi_ids[axi_id];
@@ -217,7 +217,7 @@ v3d_hub_irq(int irq, void *arg)
 		status = IRQ_HANDLED;
 	}
 
-	if (v3d->ver >= 71 && (intsts & V3D_V7_HUB_INT_GMPV)) {
+	if (v3d->ver >= V3D_GEN_71 && (intsts & V3D_V7_HUB_INT_GMPV)) {
 		dev_err(v3d->drm.dev, "GMP Violation\n");
 		status = IRQ_HANDLED;
 	}
diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
index 3ebda2fa46fc4775c67d13d8f8131160ff6ca09d..9a3fe52558746e8523d4cf4ee433a90d94bffdbf 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -200,10 +200,10 @@ void v3d_perfmon_init(struct v3d_dev *v3d)
 	const struct v3d_perf_counter_desc *counters = NULL;
 	unsigned int max = 0;
 
-	if (v3d->ver >= 71) {
+	if (v3d->ver >= V3D_GEN_71) {
 		counters = v3d_v71_performance_counters;
 		max = ARRAY_SIZE(v3d_v71_performance_counters);
-	} else if (v3d->ver >= 42) {
+	} else if (v3d->ver >= V3D_GEN_42) {
 		counters = v3d_v42_performance_counters;
 		max = ARRAY_SIZE(v3d_v42_performance_counters);
 	}
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 34c42d6e12cde656d3b51a18be324976199eceae..b3be08b0ca9188564f9fb6aa32694940a5fadc9d 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -357,11 +357,11 @@ v3d_tfu_job_run(struct drm_sched_job *sched_job)
 	V3D_WRITE(V3D_TFU_ICA(v3d->ver), job->args.ica);
 	V3D_WRITE(V3D_TFU_IUA(v3d->ver), job->args.iua);
 	V3D_WRITE(V3D_TFU_IOA(v3d->ver), job->args.ioa);
-	if (v3d->ver >= 71)
+	if (v3d->ver >= V3D_GEN_71)
 		V3D_WRITE(V3D_V7_TFU_IOC, job->args.v71.ioc);
 	V3D_WRITE(V3D_TFU_IOS(v3d->ver), job->args.ios);
 	V3D_WRITE(V3D_TFU_COEF0(v3d->ver), job->args.coef[0]);
-	if (v3d->ver >= 71 || (job->args.coef[0] & V3D_TFU_COEF0_USECOEF)) {
+	if (v3d->ver >= V3D_GEN_71 || (job->args.coef[0] & V3D_TFU_COEF0_USECOEF)) {
 		V3D_WRITE(V3D_TFU_COEF1(v3d->ver), job->args.coef[1]);
 		V3D_WRITE(V3D_TFU_COEF2(v3d->ver), job->args.coef[2]);
 		V3D_WRITE(V3D_TFU_COEF3(v3d->ver), job->args.coef[3]);
@@ -412,7 +412,7 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
 	 *
 	 * XXX: Set the CFG7 register
 	 */
-	if (v3d->ver >= 71)
+	if (v3d->ver >= V3D_GEN_71)
 		V3D_CORE_WRITE(0, V3D_V7_CSD_QUEUED_CFG7, 0);
 
 	/* CFG0 write kicks off the job. */

-- 
Git-154)


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

* [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions
  2025-03-13 14:43 [PATCH v4 0/7] drm/v3d: Fix GPU reset issues on the Raspberry Pi 5 Maíra Canal
                   ` (2 preceding siblings ...)
  2025-03-13 14:43 ` [PATCH v4 3/7] drm/v3d: Associate a V3D tech revision to all supported devices Maíra Canal
@ 2025-03-13 14:43 ` Maíra Canal
  2025-03-13 15:03   ` Krzysztof Kozlowski
  2025-03-13 14:43 ` [PATCH v4 5/7] dt-bindings: gpu: v3d: Add SMS register to BCM2712 compatible Maíra Canal
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Maíra Canal @ 2025-03-13 14:43 UTC (permalink / raw)
  To: Melissa Wen, Iago Toral, Krzysztof Kozlowski, Conor Dooley,
	Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, Maíra Canal

In order to enforce per-SoC register rules, add per-compatible
restrictions. V3D 3.3 (represented by brcm,7268-v3d) has a cache
controller (GCA), which is not present in other V3D generations.
Declaring these differences helps ensure the DTB accurately reflect
the hardware design.

While not ideal, this commit keeps the register order flexible for
brcm,7268-v3d with the goal to keep the ABI backwards compatible.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 73 ++++++++++++++++++----
 1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
index dc078ceeca9ac3447ba54a7c8830821f0b2a7f9f..9867b617c60c6fe34a0f88a3ee2f581a94b69a5c 100644
--- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
+++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
@@ -22,20 +22,10 @@ properties:
       - brcm,7278-v3d
 
   reg:
-    items:
-      - description: hub register (required)
-      - description: core0 register (required)
-      - description: GCA cache controller register (if GCA controller present)
-      - description: bridge register (if no external reset controller)
-    minItems: 2
+    maxItems: 4
 
   reg-names:
-    items:
-      - const: hub
-      - const: core0
-      - enum: [ bridge, gca ]
-      - enum: [ bridge, gca ]
-    minItems: 2
+    maxItems: 4
 
   interrupts:
     items:
@@ -58,6 +48,65 @@ required:
   - reg-names
   - interrupts
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - brcm,2711-v3d
+              - brcm,7278-v3d
+    then:
+      properties:
+        reg:
+          items:
+            - description: hub register
+            - description: core0 register
+            - description: bridge register (if no external reset controller)
+          minItems: 2
+        reg-names:
+          items:
+            - const: hub
+            - const: core0
+            - const: bridge
+          minItems: 2
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: brcm,2712-v3d
+    then:
+      properties:
+        reg:
+          items:
+            - description: hub register
+            - description: core0 register
+        reg-names:
+          items:
+            - const: hub
+            - const: core0
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: brcm,7268-v3d
+    then:
+      properties:
+        reg:
+          items:
+            - description: hub register
+            - description: core0 register
+            - description: GCA cache controller register
+            - description: bridge register (if no external reset controller)
+          minItems: 3
+        reg-names:
+          items:
+            - const: hub
+            - const: core0
+            - enum: [ bridge, gca ]
+            - enum: [ bridge, gca ]
+          minItems: 3
+
 additionalProperties: false
 
 examples:

-- 
Git-154)


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

* [PATCH v4 5/7] dt-bindings: gpu: v3d: Add SMS register to BCM2712 compatible
  2025-03-13 14:43 [PATCH v4 0/7] drm/v3d: Fix GPU reset issues on the Raspberry Pi 5 Maíra Canal
                   ` (3 preceding siblings ...)
  2025-03-13 14:43 ` [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions Maíra Canal
@ 2025-03-13 14:43 ` Maíra Canal
  2025-03-13 15:03   ` Krzysztof Kozlowski
  2025-03-13 14:43 ` [PATCH v4 6/7] drm/v3d: Use V3D_SMS registers for power on/off and reset on V3D 7.x Maíra Canal
  2025-03-13 14:43 ` [PATCH v4 7/7] dt-bindings: gpu: Add V3D driver maintainer as DT maintainer Maíra Canal
  6 siblings, 1 reply; 15+ messages in thread
From: Maíra Canal @ 2025-03-13 14:43 UTC (permalink / raw)
  To: Melissa Wen, Iago Toral, Krzysztof Kozlowski, Conor Dooley,
	Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, Maíra Canal

V3D 7.1 exposes a new register block, called V3D_SMS. As BCM2712 has a
V3D 7.1 core, add a new register item to its compatible. Similar to the
GCA, which is specific for V3D 3.3, SMS should only be added for V3D 7.1
variants (such as brcm,2712-v3d).

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
index 9867b617c60c6fe34a0f88a3ee2f581a94b69a5c..739e8afcacebe63ac6cd8853a58750f4f83146d3 100644
--- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
+++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
@@ -81,10 +81,12 @@ allOf:
           items:
             - description: hub register
             - description: core0 register
+            - description: SMS register
         reg-names:
           items:
             - const: hub
             - const: core0
+            - const: sms
   - if:
       properties:
         compatible:

-- 
Git-154)


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

* [PATCH v4 6/7] drm/v3d: Use V3D_SMS registers for power on/off and reset on V3D 7.x
  2025-03-13 14:43 [PATCH v4 0/7] drm/v3d: Fix GPU reset issues on the Raspberry Pi 5 Maíra Canal
                   ` (4 preceding siblings ...)
  2025-03-13 14:43 ` [PATCH v4 5/7] dt-bindings: gpu: v3d: Add SMS register to BCM2712 compatible Maíra Canal
@ 2025-03-13 14:43 ` Maíra Canal
  2025-03-13 14:43 ` [PATCH v4 7/7] dt-bindings: gpu: Add V3D driver maintainer as DT maintainer Maíra Canal
  6 siblings, 0 replies; 15+ messages in thread
From: Maíra Canal @ 2025-03-13 14:43 UTC (permalink / raw)
  To: Melissa Wen, Iago Toral, Krzysztof Kozlowski, Conor Dooley,
	Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, Maíra Canal

In addition to the standard reset controller, V3D 7.x requires configuring
the V3D_SMS registers for proper power on/off and reset. Add the new
registers to `v3d_regs.h` and ensure they are properly configured during
device probing, removal, and reset.

This change fixes GPU reset issues on the Raspberry Pi 5 (BCM2712).
Without exposing these registers, a GPU reset causes the GPU to hang,
stopping any further job execution and freezing the desktop GUI. The same
issue occurs when unloading and loading the v3d driver.

Link: https://github.com/raspberrypi/linux/issues/6660
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_drv.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/v3d/v3d_drv.h  | 11 +++++++++++
 drivers/gpu/drm/v3d/v3d_gem.c  | 17 +++++++++++++++++
 drivers/gpu/drm/v3d/v3d_regs.h | 26 ++++++++++++++++++++++++++
 4 files changed, 94 insertions(+)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index c63f0ed1bd8a3d5511085e76ed2fbd6ee7df6f80..122848cdccc4a02039d9ea2e77aa2f377886b5d6 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -263,6 +263,36 @@ static const struct of_device_id v3d_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, v3d_of_match);
 
+static void
+v3d_idle_sms(struct v3d_dev *v3d)
+{
+	if (v3d->ver < V3D_GEN_71)
+		return;
+
+	V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_CLEAR_POWER_OFF);
+
+	if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
+				    V3D_SMS_STATE) == V3D_SMS_IDLE), 100)) {
+		DRM_ERROR("Failed to power up SMS\n");
+	}
+
+	v3d_reset_sms(v3d);
+}
+
+static void
+v3d_power_off_sms(struct v3d_dev *v3d)
+{
+	if (v3d->ver < V3D_GEN_71)
+		return;
+
+	V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_POWER_OFF);
+
+	if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
+				    V3D_SMS_STATE) == V3D_SMS_POWER_OFF_STATE), 100)) {
+		DRM_ERROR("Failed to power off SMS\n");
+	}
+}
+
 static int
 map_regs(struct v3d_dev *v3d, void __iomem **regs, const char *name)
 {
@@ -300,6 +330,12 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	if (v3d->ver >= V3D_GEN_71) {
+		ret = map_regs(v3d, &v3d->sms_regs, "sms");
+		if (ret)
+			return ret;
+	}
+
 	v3d->clk = devm_clk_get_optional(dev, NULL);
 	if (IS_ERR(v3d->clk))
 		return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D clock\n");
@@ -310,6 +346,8 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	v3d_idle_sms(v3d);
+
 	mmu_debug = V3D_READ(V3D_MMU_DEBUG_INFO);
 	mask = DMA_BIT_MASK(30 + V3D_GET_FIELD(mmu_debug, V3D_MMU_PA_WIDTH));
 	ret = dma_set_mask_and_coherent(dev, mask);
@@ -410,6 +448,8 @@ static void v3d_platform_drm_remove(struct platform_device *pdev)
 	dma_free_wc(v3d->drm.dev, 4096, v3d->mmu_scratch,
 		    v3d->mmu_scratch_paddr);
 
+	v3d_power_off_sms(v3d);
+
 	clk_disable_unprepare(v3d->clk);
 }
 
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index de4a9e18f6a9039edf57f406ab1cee9dad4c0a49..b51f0b648a08011f737317ec1841d5ab316355b2 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -118,6 +118,7 @@ struct v3d_dev {
 	void __iomem *core_regs[3];
 	void __iomem *bridge_regs;
 	void __iomem *gca_regs;
+	void __iomem *sms_regs;
 	struct clk *clk;
 	struct reset_control *reset;
 
@@ -268,6 +269,15 @@ to_v3d_fence(struct dma_fence *fence)
 #define V3D_GCA_READ(offset) readl(v3d->gca_regs + offset)
 #define V3D_GCA_WRITE(offset, val) writel(val, v3d->gca_regs + offset)
 
+#define V3D_SMS_IDLE				0x0
+#define V3D_SMS_ISOLATING_FOR_RESET		0xa
+#define V3D_SMS_RESETTING			0xb
+#define V3D_SMS_ISOLATING_FOR_POWER_OFF	0xc
+#define V3D_SMS_POWER_OFF_STATE		0xd
+
+#define V3D_SMS_READ(offset) readl(v3d->sms_regs + (offset))
+#define V3D_SMS_WRITE(offset, val) writel(val, v3d->sms_regs + (offset))
+
 #define V3D_CORE_READ(core, offset) readl(v3d->core_regs[core] + offset)
 #define V3D_CORE_WRITE(core, offset, val) writel(val, v3d->core_regs[core] + offset)
 
@@ -546,6 +556,7 @@ struct dma_fence *v3d_fence_create(struct v3d_dev *v3d, enum v3d_queue queue);
 /* v3d_gem.c */
 int v3d_gem_init(struct drm_device *dev);
 void v3d_gem_destroy(struct drm_device *dev);
+void v3d_reset_sms(struct v3d_dev *v3d);
 void v3d_reset(struct v3d_dev *v3d);
 void v3d_invalidate_caches(struct v3d_dev *v3d);
 void v3d_clean_caches(struct v3d_dev *v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 1ea6d3832c2212d9cbbd90236478d18491f0ff14..d7d16da78db328f004d1d702731d1a1b5437a394 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -104,6 +104,22 @@ v3d_reset_v3d(struct v3d_dev *v3d)
 	v3d_init_hw_state(v3d);
 }
 
+void
+v3d_reset_sms(struct v3d_dev *v3d)
+{
+	if (v3d->ver < V3D_GEN_71)
+		return;
+
+	V3D_SMS_WRITE(V3D_SMS_REE_CS, V3D_SET_FIELD(0x4, V3D_SMS_STATE));
+
+	if (wait_for(!(V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_REE_CS),
+				     V3D_SMS_STATE) == V3D_SMS_ISOLATING_FOR_RESET) &&
+		     !(V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_REE_CS),
+				     V3D_SMS_STATE) == V3D_SMS_RESETTING), 100)) {
+		DRM_ERROR("Failed to wait for SMS reset\n");
+	}
+}
+
 void
 v3d_reset(struct v3d_dev *v3d)
 {
@@ -119,6 +135,7 @@ v3d_reset(struct v3d_dev *v3d)
 		v3d_idle_axi(v3d, 0);
 
 	v3d_idle_gca(v3d);
+	v3d_reset_sms(v3d);
 	v3d_reset_v3d(v3d);
 
 	v3d_mmu_set_page_table(v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h
index 6da3c69082bd6d5954bf88bd9ff2543a5e4e04c4..c1870265eaeecc188afc4f09cf13a5201d3aa1c6 100644
--- a/drivers/gpu/drm/v3d/v3d_regs.h
+++ b/drivers/gpu/drm/v3d/v3d_regs.h
@@ -515,4 +515,30 @@
 # define V3D_ERR_VPAERGS                               BIT(1)
 # define V3D_ERR_VPAEABB                               BIT(0)
 
+#define V3D_SMS_REE_CS                                 0x00000
+#define V3D_SMS_TEE_CS                                 0x00400
+# define V3D_SMS_INTERRUPT                             BIT(31)
+# define V3D_SMS_POWER_OFF                             BIT(30)
+# define V3D_SMS_CLEAR_POWER_OFF                       BIT(29)
+# define V3D_SMS_LOCK                                  BIT(28)
+# define V3D_SMS_CLEAR_LOCK                            BIT(27)
+# define V3D_SMS_SVP_MODE_EXIT                         BIT(26)
+# define V3D_SMS_CLEAR_SVP_MODE_EXIT                   BIT(25)
+# define V3D_SMS_SVP_MODE_ENTER                        BIT(24)
+# define V3D_SMS_CLEAR_SVP_MODE_ENTER                  BIT(23)
+# define V3D_SMS_THEIR_MODE_EXIT                       BIT(22)
+# define V3D_SMS_THEIR_MODE_ENTER                      BIT(21)
+# define V3D_SMS_OUR_MODE_EXIT                         BIT(20)
+# define V3D_SMS_CLEAR_OUR_MODE_EXIT                   BIT(19)
+# define V3D_SMS_SEQ_PC_MASK                           V3D_MASK(16, 10)
+# define V3D_SMS_SEQ_PC_SHIFT                          10
+# define V3D_SMS_HUBCORE_STATUS_MASK                   V3D_MASK(9, 8)
+# define V3D_SMS_HUBCORE_STATUS_SHIFT                  8
+# define V3D_SMS_NEW_MODE_MASK                         V3D_MASK(7, 6)
+# define V3D_SMS_NEW_MODE_SHIFT                        6
+# define V3D_SMS_OLD_MODE_MASK                         V3D_MASK(5, 4)
+# define V3D_SMS_OLD_MODE_SHIFT                        4
+# define V3D_SMS_STATE_MASK                            V3D_MASK(3, 0)
+# define V3D_SMS_STATE_SHIFT                           0
+
 #endif /* V3D_REGS_H */

-- 
Git-154)


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

* [PATCH v4 7/7] dt-bindings: gpu: Add V3D driver maintainer as DT maintainer
  2025-03-13 14:43 [PATCH v4 0/7] drm/v3d: Fix GPU reset issues on the Raspberry Pi 5 Maíra Canal
                   ` (5 preceding siblings ...)
  2025-03-13 14:43 ` [PATCH v4 6/7] drm/v3d: Use V3D_SMS registers for power on/off and reset on V3D 7.x Maíra Canal
@ 2025-03-13 14:43 ` Maíra Canal
  6 siblings, 0 replies; 15+ messages in thread
From: Maíra Canal @ 2025-03-13 14:43 UTC (permalink / raw)
  To: Melissa Wen, Iago Toral, Krzysztof Kozlowski, Conor Dooley,
	Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, Emma Anholt,
	Rob Herring (Arm), Maíra Canal

As established in commit 89d04995f76c ("MAINTAINERS: Drop Emma Anholt
from all M lines."), Emma is no longer active in the Linux kernel and
dropped the V3D maintainership. Therefore, remove Emma as one of the DT
maintainers and add the current V3D driver maintainer.

Acked-by: Emma Anholt <emma@anholt.net>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
index 739e8afcacebe63ac6cd8853a58750f4f83146d3..82bc2f7c4006055df1031ddd4c64432c5ed3a14f 100644
--- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
+++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: Broadcom V3D GPU
 
 maintainers:
-  - Eric Anholt <eric@anholt.net>
+  - Maíra Canal <mcanal@igalia.com>
   - Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
 
 properties:

-- 
Git-154)


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

* Re: [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions
  2025-03-13 14:43 ` [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions Maíra Canal
@ 2025-03-13 15:03   ` Krzysztof Kozlowski
  2025-03-13 19:04     ` Maíra Canal
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-13 15:03 UTC (permalink / raw)
  To: Maíra Canal, Melissa Wen, Iago Toral, Krzysztof Kozlowski,
	Conor Dooley, Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev

On 13/03/2025 15:43, Maíra Canal wrote:
> In order to enforce per-SoC register rules, add per-compatible
> restrictions. V3D 3.3 (represented by brcm,7268-v3d) has a cache
> controller (GCA), which is not present in other V3D generations.
> Declaring these differences helps ensure the DTB accurately reflect
> the hardware design.
> 
> While not ideal, this commit keeps the register order flexible for
> brcm,7268-v3d with the goal to keep the ABI backwards compatible.
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 73 ++++++++++++++++++----
>  1 file changed, 61 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
> index dc078ceeca9ac3447ba54a7c8830821f0b2a7f9f..9867b617c60c6fe34a0f88a3ee2f581a94b69a5c 100644
> --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
> @@ -22,20 +22,10 @@ properties:
>        - brcm,7278-v3d
>  
>    reg:
> -    items:
> -      - description: hub register (required)
> -      - description: core0 register (required)
> -      - description: GCA cache controller register (if GCA controller present)
> -      - description: bridge register (if no external reset controller)
> -    minItems: 2

Widest constraints always stay here, so you cannot remove minItems.

> +    maxItems: 4

>  
>    reg-names:
> -    items:
> -      - const: hub
> -      - const: core0
> -      - enum: [ bridge, gca ]
> -      - enum: [ bridge, gca ]
> -    minItems: 2

Same problem.

> +    maxItems: 4
>  
>    interrupts:
>      items:
> @@ -58,6 +48,65 @@ required:
>    - reg-names
>    - interrupts

...

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: brcm,7268-v3d
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - description: hub register
> +            - description: core0 register
> +            - description: GCA cache controller register
> +            - description: bridge register (if no external reset controller)
> +          minItems: 3
> +        reg-names:
> +          items:
> +            - const: hub
> +            - const: core0
> +            - enum: [ bridge, gca ]

So GCA is always there? Then this should be just 'gca'. Your list for
'reg' already says that third item must be GCA. I understand that you do
not want to affect the ABI, but it already kind of is with enforcing GCA
in 'reg'.

I anyway do not understand why 'gca' or 'bridge' are supposed to be
optional. Does the given SoC differ between boards? What is the external
reset controller here? External like outside of SoC?

Best regards,
Krzysztof

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

* Re: [PATCH v4 5/7] dt-bindings: gpu: v3d: Add SMS register to BCM2712 compatible
  2025-03-13 14:43 ` [PATCH v4 5/7] dt-bindings: gpu: v3d: Add SMS register to BCM2712 compatible Maíra Canal
@ 2025-03-13 15:03   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-13 15:03 UTC (permalink / raw)
  To: Maíra Canal, Melissa Wen, Iago Toral, Krzysztof Kozlowski,
	Conor Dooley, Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev

On 13/03/2025 15:43, Maíra Canal wrote:
> V3D 7.1 exposes a new register block, called V3D_SMS. As BCM2712 has a
> V3D 7.1 core, add a new register item to its compatible. Similar to the
> GCA, which is specific for V3D 3.3, SMS should only be added for V3D 7.1
> variants (such as brcm,2712-v3d).
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

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

* Re: [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions
  2025-03-13 15:03   ` Krzysztof Kozlowski
@ 2025-03-13 19:04     ` Maíra Canal
  2025-03-15  9:52       ` Stefan Wahren
  0 siblings, 1 reply; 15+ messages in thread
From: Maíra Canal @ 2025-03-13 19:04 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Melissa Wen, Iago Toral, Krzysztof Kozlowski,
	Conor Dooley, Nicolas Saenz Julienne, Stefan Wahren
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev

+Cc Stefan

Hi Krzysztof,

On 13/03/25 12:03, Krzysztof Kozlowski wrote:
> On 13/03/2025 15:43, Maíra Canal wrote:
>> In order to enforce per-SoC register rules, add per-compatible
>> restrictions. V3D 3.3 (represented by brcm,7268-v3d) has a cache
>> controller (GCA), which is not present in other V3D generations.
>> Declaring these differences helps ensure the DTB accurately reflect
>> the hardware design.
>>
>> While not ideal, this commit keeps the register order flexible for
>> brcm,7268-v3d with the goal to keep the ABI backwards compatible.
>>
>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>> ---
>>   .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 73 ++++++++++++++++++----
>>   1 file changed, 61 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> index dc078ceeca9ac3447ba54a7c8830821f0b2a7f9f..9867b617c60c6fe34a0f88a3ee2f581a94b69a5c 100644
>> --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> @@ -22,20 +22,10 @@ properties:
>>         - brcm,7278-v3d
>>   
>>     reg:
>> -    items:
>> -      - description: hub register (required)
>> -      - description: core0 register (required)
>> -      - description: GCA cache controller register (if GCA controller present)
>> -      - description: bridge register (if no external reset controller)
>> -    minItems: 2
> 
> Widest constraints always stay here, so you cannot remove minItems.
> 
>> +    maxItems: 4
> 
>>   
>>     reg-names:
>> -    items:
>> -      - const: hub
>> -      - const: core0
>> -      - enum: [ bridge, gca ]
>> -      - enum: [ bridge, gca ]
>> -    minItems: 2
> 
> Same problem.
> 
>> +    maxItems: 4
>>   
>>     interrupts:
>>       items:
>> @@ -58,6 +48,65 @@ required:
>>     - reg-names
>>     - interrupts
> 
> ...
> 
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: brcm,7268-v3d
>> +    then:
>> +      properties:
>> +        reg:
>> +          items:
>> +            - description: hub register
>> +            - description: core0 register
>> +            - description: GCA cache controller register
>> +            - description: bridge register (if no external reset controller)
>> +          minItems: 3
>> +        reg-names:
>> +          items:
>> +            - const: hub
>> +            - const: core0
>> +            - enum: [ bridge, gca ]
> 
 > So GCA is always there? Then this should be just 'gca'. Your list for

GCA is always there for V3D 3.3, therefore it is always there for
brcm,7268-v3d.

> 'reg' already says that third item must be GCA. I understand that you do
> not want to affect the ABI, but it already kind of is with enforcing GCA
> in 'reg'.

I'm adding Stefan to the loop as he was the one that converted this DT
binding to YAML. Stefan, could you share your thoughts about breaking
the ABI for BCM7268? We would enforce the following order: hub, core0,
bridge, and gca.

> 
> I anyway do not understand why 'gca' or 'bridge' are supposed to be
> optional. Does the given SoC differ between boards? What is the external
> reset controller here? External like outside of SoC?

TBH I never saw BCM7268 or BCM7278 in the wild, but you are correct,
"bridge" shouldn't change for the same SoC. I'll do my diligence and
research more about those SoCs.

Best Regards,
- Maíra

> 
> Best regards,
> Krzysztof


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

* Re: [PATCH v4 1/7] drm/v3d: Don't run jobs that have errors flagged in its fence
  2025-03-13 14:43 ` [PATCH v4 1/7] drm/v3d: Don't run jobs that have errors flagged in its fence Maíra Canal
@ 2025-03-13 20:20   ` Maíra Canal
  0 siblings, 0 replies; 15+ messages in thread
From: Maíra Canal @ 2025-03-13 20:20 UTC (permalink / raw)
  To: Melissa Wen, Iago Toral, Krzysztof Kozlowski, Conor Dooley,
	Nicolas Saenz Julienne
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, stable

On 13/03/25 11:43, Maíra Canal wrote:
> The V3D driver still relies on `drm_sched_increase_karma()` and
> `drm_sched_resubmit_jobs()` for resubmissions when a timeout occurs.
> The function `drm_sched_increase_karma()` marks the job as guilty, while
> `drm_sched_resubmit_jobs()` sets an error (-ECANCELED) in the DMA fence of
> that guilty job.
> 
> Because of this, we must check whether the job’s DMA fence has been
> flagged with an error before executing the job. Otherwise, the same guilty
> job may be resubmitted indefinitely, causing repeated GPU resets.
> 
> This patch adds a check for an error on the job's fence to prevent running
> a guilty job that was previously flagged when the GPU timed out.
> 
> Note that the CPU and CACHE_CLEAN queues do not require this check, as
> their jobs are executed synchronously once the DRM scheduler starts them.
> 
> Cc: stable@vger.kernel.org
> Fixes: d223f98f0209 ("drm/v3d: Add support for compute shader dispatch.")
> Fixes: 1584f16ca96e ("drm/v3d: Add support for submitting jobs to the TFU.")
> Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>

As patches 1/7 and 2/7 prevent the same faulty job from being
resubmitted in a loop, I just applied them to misc/kernel.git (drm-misc-
fixes).

Best Regards,
- Maíra

> ---
>   drivers/gpu/drm/v3d/v3d_sched.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
> index 80466ce8c7df669280e556c0793490b79e75d2c7..c2010ecdb08f4ba3b54f7783ed33901552d0eba1 100644
> --- a/drivers/gpu/drm/v3d/v3d_sched.c
> +++ b/drivers/gpu/drm/v3d/v3d_sched.c
> @@ -327,11 +327,15 @@ v3d_tfu_job_run(struct drm_sched_job *sched_job)
>   	struct drm_device *dev = &v3d->drm;
>   	struct dma_fence *fence;
>   
> +	if (unlikely(job->base.base.s_fence->finished.error))
> +		return NULL;
> +
> +	v3d->tfu_job = job;
> +
>   	fence = v3d_fence_create(v3d, V3D_TFU);
>   	if (IS_ERR(fence))
>   		return NULL;
>   
> -	v3d->tfu_job = job;
>   	if (job->base.irq_fence)
>   		dma_fence_put(job->base.irq_fence);
>   	job->base.irq_fence = dma_fence_get(fence);
> @@ -369,6 +373,9 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
>   	struct dma_fence *fence;
>   	int i, csd_cfg0_reg;
>   
> +	if (unlikely(job->base.base.s_fence->finished.error))
> +		return NULL;
> +
>   	v3d->csd_job = job;
>   
>   	v3d_invalidate_caches(v3d);
> 


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

* Re: [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions
  2025-03-13 19:04     ` Maíra Canal
@ 2025-03-15  9:52       ` Stefan Wahren
  2025-03-15 12:17         ` Maíra Canal
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Wahren @ 2025-03-15  9:52 UTC (permalink / raw)
  To: Maíra Canal, Krzysztof Kozlowski, Melissa Wen, Iago Toral,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, Maxime Ripard,
	Florian Fainelli

Hello,

Am 13.03.25 um 20:04 schrieb Maíra Canal:
> +Cc Stefan
>
> Hi Krzysztof,
>
> On 13/03/25 12:03, Krzysztof Kozlowski wrote:
>> On 13/03/2025 15:43, Maíra Canal wrote:
>>> In order to enforce per-SoC register rules, add per-compatible
>>> restrictions. V3D 3.3 (represented by brcm,7268-v3d) has a cache
>>> controller (GCA), which is not present in other V3D generations.
>>> Declaring these differences helps ensure the DTB accurately reflect
>>> the hardware design.
>>>
>>> While not ideal, this commit keeps the register order flexible for
>>> brcm,7268-v3d with the goal to keep the ABI backwards compatible.
>>>
>>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>>> ---
>>>   .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 73
>>> ++++++++++++++++++----
>>>   1 file changed, 61 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>> b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>> index
>>> dc078ceeca9ac3447ba54a7c8830821f0b2a7f9f..9867b617c60c6fe34a0f88a3ee2f581a94b69a5c
>>> 100644
>>> --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>> @@ -22,20 +22,10 @@ properties:
>>>         - brcm,7278-v3d
>>>       reg:
>>> -    items:
>>> -      - description: hub register (required)
>>> -      - description: core0 register (required)
>>> -      - description: GCA cache controller register (if GCA
>>> controller present)
>>> -      - description: bridge register (if no external reset controller)
>>> -    minItems: 2
>>
>> Widest constraints always stay here, so you cannot remove minItems.
>>
>>> +    maxItems: 4
>>
>>>       reg-names:
>>> -    items:
>>> -      - const: hub
>>> -      - const: core0
>>> -      - enum: [ bridge, gca ]
>>> -      - enum: [ bridge, gca ]
>>> -    minItems: 2
>>
>> Same problem.
>>
>>> +    maxItems: 4
>>>       interrupts:
>>>       items:
>>> @@ -58,6 +48,65 @@ required:
>>>     - reg-names
>>>     - interrupts
>>
>> ...
>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: brcm,7268-v3d
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          items:
>>> +            - description: hub register
>>> +            - description: core0 register
>>> +            - description: GCA cache controller register
>>> +            - description: bridge register (if no external reset
>>> controller)
>>> +          minItems: 3
>>> +        reg-names:
>>> +          items:
>>> +            - const: hub
>>> +            - const: core0
>>> +            - enum: [ bridge, gca ]
>>
> > So GCA is always there? Then this should be just 'gca'. Your list for
>
> GCA is always there for V3D 3.3, therefore it is always there for
> brcm,7268-v3d.
>
>> 'reg' already says that third item must be GCA. I understand that you do
>> not want to affect the ABI, but it already kind of is with enforcing GCA
>> in 'reg'.
>
> I'm adding Stefan to the loop as he was the one that converted this DT
> binding to YAML. Stefan, could you share your thoughts about breaking
> the ABI for BCM7268? We would enforce the following order: hub, core0,
> bridge, and gca.
Phew, that was over 4 years ago. To be honest, my only motivation back
then was to prepare support for the Raspberry Pi 4 (BCM2711). I did it
all in my spare time and never had access to any Broadcom documents. I
have no idea about all the other BCM chips, so a possible break of the
ABI for the BCM7268 was an accident. I don't know if Florian Fainelli or
Maxime Ripard can help here.

By the way the two schema maintainers have not been active at V3D for a
long time, so it would be good if someone could take over.

Regards
>
>>
>> I anyway do not understand why 'gca' or 'bridge' are supposed to be
>> optional. Does the given SoC differ between boards? What is the external
>> reset controller here? External like outside of SoC?
>
> TBH I never saw BCM7268 or BCM7278 in the wild, but you are correct,
> "bridge" shouldn't change for the same SoC. I'll do my diligence and
> research more about those SoCs.
>
> Best Regards,
> - Maíra
>
>>
>> Best regards,
>> Krzysztof
>


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

* Re: [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions
  2025-03-15  9:52       ` Stefan Wahren
@ 2025-03-15 12:17         ` Maíra Canal
  2025-03-15 14:44           ` Florian Fainelli
  0 siblings, 1 reply; 15+ messages in thread
From: Maíra Canal @ 2025-03-15 12:17 UTC (permalink / raw)
  To: Stefan Wahren, Krzysztof Kozlowski, Melissa Wen, Iago Toral,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, Maxime Ripard,
	Florian Fainelli

Hi Stefan,

On 15/03/25 06:52, Stefan Wahren wrote:
> Hello,
> 
> Am 13.03.25 um 20:04 schrieb Maíra Canal:
>> +Cc Stefan
>>
>> Hi Krzysztof,
>>
>> On 13/03/25 12:03, Krzysztof Kozlowski wrote:
>>> On 13/03/2025 15:43, Maíra Canal wrote:
>>>> In order to enforce per-SoC register rules, add per-compatible
>>>> restrictions. V3D 3.3 (represented by brcm,7268-v3d) has a cache
>>>> controller (GCA), which is not present in other V3D generations.
>>>> Declaring these differences helps ensure the DTB accurately reflect
>>>> the hardware design.
>>>>
>>>> While not ideal, this commit keeps the register order flexible for
>>>> brcm,7268-v3d with the goal to keep the ABI backwards compatible.
>>>>
>>>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>>>> ---
>>>>   .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 73
>>>> ++++++++++++++++++----
>>>>   1 file changed, 61 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>>> b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>>> index
>>>> dc078ceeca9ac3447ba54a7c8830821f0b2a7f9f..9867b617c60c6fe34a0f88a3ee2f581a94b69a5c
>>>> 100644
>>>> --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml

[...]

>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: brcm,7268-v3d
>>>> +    then:
>>>> +      properties:
>>>> +        reg:
>>>> +          items:
>>>> +            - description: hub register
>>>> +            - description: core0 register
>>>> +            - description: GCA cache controller register
>>>> +            - description: bridge register (if no external reset
>>>> controller)
>>>> +          minItems: 3
>>>> +        reg-names:
>>>> +          items:
>>>> +            - const: hub
>>>> +            - const: core0
>>>> +            - enum: [ bridge, gca ]
>>>
>> > So GCA is always there? Then this should be just 'gca'. Your list for
>>
>> GCA is always there for V3D 3.3, therefore it is always there for
>> brcm,7268-v3d.
>>
>>> 'reg' already says that third item must be GCA. I understand that you do
>>> not want to affect the ABI, but it already kind of is with enforcing GCA
>>> in 'reg'.
>>
>> I'm adding Stefan to the loop as he was the one that converted this DT
>> binding to YAML. Stefan, could you share your thoughts about breaking
>> the ABI for BCM7268? We would enforce the following order: hub, core0,
>> bridge, and gca.
> Phew, that was over 4 years ago. To be honest, my only motivation back
> then was to prepare support for the Raspberry Pi 4 (BCM2711). I did it
> all in my spare time and never had access to any Broadcom documents. I
> have no idea about all the other BCM chips, so a possible break of the
> ABI for the BCM7268 was an accident. I don't know if Florian Fainelli or
> Maxime Ripard can help here.

Thanks for providing your feedback! I did my diligence and now I know
which SoCs have each register bank. For BCM2711, BCM2712, and BCM7278,
the ABI will be preserved. As for BCM7268, I plan to enforce the order
specified in the current DT binding example: hub, core0, bridge, and
gca.

Florian, it would be great to hear your feedback about BCM7268.

> 
> By the way the two schema maintainers have not been active at V3D for a
> long time, so it would be good if someone could take over.

I sent a patch [1] to step in as maintainer.

[1] 
https://lore.kernel.org/dri-devel/20250313-v3d-gpu-reset-fixes-v4-0-c1e780d8e096@igalia.com/T/#mf8ffc3dc7a216efc1842d773787394c3506814cd

Best Regards,
- Maíra

> 
> Regards
>>
>>>
>>> I anyway do not understand why 'gca' or 'bridge' are supposed to be
>>> optional. Does the given SoC differ between boards? What is the external
>>> reset controller here? External like outside of SoC?
>>
>> TBH I never saw BCM7268 or BCM7278 in the wild, but you are correct,
>> "bridge" shouldn't change for the same SoC. I'll do my diligence and
>> research more about those SoCs.
>>
>> Best Regards,
>> - Maíra
>>
>>>
>>> Best regards,
>>> Krzysztof
>>
> 


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

* Re: [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions
  2025-03-15 12:17         ` Maíra Canal
@ 2025-03-15 14:44           ` Florian Fainelli
  0 siblings, 0 replies; 15+ messages in thread
From: Florian Fainelli @ 2025-03-15 14:44 UTC (permalink / raw)
  To: Maíra Canal, Stefan Wahren, Krzysztof Kozlowski, Melissa Wen,
	Iago Toral, Krzysztof Kozlowski, Conor Dooley
  Cc: Phil Elwell, dri-devel, devicetree, kernel-dev, Maxime Ripard



On 3/15/2025 5:17 AM, Maíra Canal wrote:
> Hi Stefan,
> 
> On 15/03/25 06:52, Stefan Wahren wrote:
>> Hello,
>>
>> Am 13.03.25 um 20:04 schrieb Maíra Canal:
>>> +Cc Stefan
>>>
>>> Hi Krzysztof,
>>>
>>> On 13/03/25 12:03, Krzysztof Kozlowski wrote:
>>>> On 13/03/2025 15:43, Maíra Canal wrote:
>>>>> In order to enforce per-SoC register rules, add per-compatible
>>>>> restrictions. V3D 3.3 (represented by brcm,7268-v3d) has a cache
>>>>> controller (GCA), which is not present in other V3D generations.
>>>>> Declaring these differences helps ensure the DTB accurately reflect
>>>>> the hardware design.
>>>>>
>>>>> While not ideal, this commit keeps the register order flexible for
>>>>> brcm,7268-v3d with the goal to keep the ABI backwards compatible.
>>>>>
>>>>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>>>>> ---
>>>>>   .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 73
>>>>> ++++++++++++++++++----
>>>>>   1 file changed, 61 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>>>> b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>>>> index
>>>>> dc078ceeca9ac3447ba54a7c8830821f0b2a7f9f..9867b617c60c6fe34a0f88a3ee2f581a94b69a5c
>>>>> 100644
>>>>> --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>>>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
> 
> [...]
> 
>>>>> +  - if:
>>>>> +      properties:
>>>>> +        compatible:
>>>>> +          contains:
>>>>> +            const: brcm,7268-v3d
>>>>> +    then:
>>>>> +      properties:
>>>>> +        reg:
>>>>> +          items:
>>>>> +            - description: hub register
>>>>> +            - description: core0 register
>>>>> +            - description: GCA cache controller register
>>>>> +            - description: bridge register (if no external reset
>>>>> controller)
>>>>> +          minItems: 3
>>>>> +        reg-names:
>>>>> +          items:
>>>>> +            - const: hub
>>>>> +            - const: core0
>>>>> +            - enum: [ bridge, gca ]
>>>>
>>> > So GCA is always there? Then this should be just 'gca'. Your list for
>>>
>>> GCA is always there for V3D 3.3, therefore it is always there for
>>> brcm,7268-v3d.
>>>
>>>> 'reg' already says that third item must be GCA. I understand that 
>>>> you do
>>>> not want to affect the ABI, but it already kind of is with enforcing 
>>>> GCA
>>>> in 'reg'.
>>>
>>> I'm adding Stefan to the loop as he was the one that converted this DT
>>> binding to YAML. Stefan, could you share your thoughts about breaking
>>> the ABI for BCM7268? We would enforce the following order: hub, core0,
>>> bridge, and gca.
>> Phew, that was over 4 years ago. To be honest, my only motivation back
>> then was to prepare support for the Raspberry Pi 4 (BCM2711). I did it
>> all in my spare time and never had access to any Broadcom documents. I
>> have no idea about all the other BCM chips, so a possible break of the
>> ABI for the BCM7268 was an accident. I don't know if Florian Fainelli or
>> Maxime Ripard can help here.
> 
> Thanks for providing your feedback! I did my diligence and now I know
> which SoCs have each register bank. For BCM2711, BCM2712, and BCM7278,
> the ABI will be preserved. As for BCM7268, I plan to enforce the order
> specified in the current DT binding example: hub, core0, bridge, and
> gca.

For 7268, if we were to enforce by ascending address/offset, the order 
ought to be hub, bridge, gca, and core0 (all are present). In practice I 
don't think this matters at all because the upstream v3d driver is not 
used on those STB chips at all.
-- 
Florian


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

end of thread, other threads:[~2025-03-15 14:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-13 14:43 [PATCH v4 0/7] drm/v3d: Fix GPU reset issues on the Raspberry Pi 5 Maíra Canal
2025-03-13 14:43 ` [PATCH v4 1/7] drm/v3d: Don't run jobs that have errors flagged in its fence Maíra Canal
2025-03-13 20:20   ` Maíra Canal
2025-03-13 14:43 ` [PATCH v4 2/7] drm/v3d: Set job pointer to NULL when the job's fence has an error Maíra Canal
2025-03-13 14:43 ` [PATCH v4 3/7] drm/v3d: Associate a V3D tech revision to all supported devices Maíra Canal
2025-03-13 14:43 ` [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions Maíra Canal
2025-03-13 15:03   ` Krzysztof Kozlowski
2025-03-13 19:04     ` Maíra Canal
2025-03-15  9:52       ` Stefan Wahren
2025-03-15 12:17         ` Maíra Canal
2025-03-15 14:44           ` Florian Fainelli
2025-03-13 14:43 ` [PATCH v4 5/7] dt-bindings: gpu: v3d: Add SMS register to BCM2712 compatible Maíra Canal
2025-03-13 15:03   ` Krzysztof Kozlowski
2025-03-13 14:43 ` [PATCH v4 6/7] drm/v3d: Use V3D_SMS registers for power on/off and reset on V3D 7.x Maíra Canal
2025-03-13 14:43 ` [PATCH v4 7/7] dt-bindings: gpu: Add V3D driver maintainer as DT maintainer Maíra Canal

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).