All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ
@ 2020-01-16 10:00 chen gong
  2020-01-16 10:00 ` [PATCH 2/3] drm/amdgpu: add kiq version interface for RREG32/WREG32 chen gong
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: chen gong @ 2020-01-16 10:00 UTC (permalink / raw)
  To: amd-gfx; +Cc: chen gong

Move amdgpu_virt_kiq_rreg/amdgpu_virt_kiq_wreg function to amdgpu_gfx.c,
and rename them to amdgpu_kiq_rreg/amdgpu_kiq_wreg.Make it generic and
flexible.

Signed-off-by: chen gong <curry.gong@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 96 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 92 ----------------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   |  2 -
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c     |  5 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      |  5 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      |  5 +-
 8 files changed, 108 insertions(+), 104 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2c64d2a..963ea46 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -218,7 +218,7 @@ uint32_t amdgpu_mm_rreg(struct amdgpu_device *adev, uint32_t reg,
 	uint32_t ret;
 
 	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
-		return amdgpu_virt_kiq_rreg(adev, reg);
+		return amdgpu_kiq_rreg(adev, reg);
 
 	if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
 		ret = readl(((void __iomem *)adev->rmmio) + (reg * 4));
@@ -296,7 +296,7 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
 	}
 
 	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
-		return amdgpu_virt_kiq_wreg(adev, reg, v);
+		return amdgpu_kiq_wreg(adev, reg, v);
 
 	if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
 		writel(v, ((void __iomem *)adev->rmmio) + (reg * 4));
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index b88b8b8..0f960b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -296,7 +296,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
 
 	spin_lock_init(&kiq->ring_lock);
 
-	r = amdgpu_device_wb_get(adev, &adev->virt.reg_val_offs);
+	r = amdgpu_device_wb_get(adev, &kiq->reg_val_offs);
 	if (r)
 		return r;
 
@@ -321,7 +321,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
 
 void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring)
 {
-	amdgpu_device_wb_free(ring->adev, ring->adev->virt.reg_val_offs);
+	amdgpu_device_wb_free(ring->adev, ring->adev->gfx.kiq.reg_val_offs);
 	amdgpu_ring_fini(ring);
 }
 
@@ -658,3 +658,95 @@ int amdgpu_gfx_cp_ecc_error_irq(struct amdgpu_device *adev,
 	amdgpu_ras_interrupt_dispatch(adev, &ih_data);
 	return 0;
 }
+
+uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
+{
+	signed long r, cnt = 0;
+	unsigned long flags;
+	uint32_t seq;
+	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
+	struct amdgpu_ring *ring = &kiq->ring;
+
+	BUG_ON(!ring->funcs->emit_rreg);
+
+	spin_lock_irqsave(&kiq->ring_lock, flags);
+	amdgpu_ring_alloc(ring, 32);
+	amdgpu_ring_emit_rreg(ring, reg);
+	amdgpu_fence_emit_polling(ring, &seq);
+	amdgpu_ring_commit(ring);
+	spin_unlock_irqrestore(&kiq->ring_lock, flags);
+
+	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
+
+	/* don't wait anymore for gpu reset case because this way may
+	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
+	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
+	 * never return if we keep waiting in virt_kiq_rreg, which cause
+	 * gpu_recover() hang there.
+	 *
+	 * also don't wait anymore for IRQ context
+	 * */
+	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
+		goto failed_kiq_read;
+
+	might_sleep();
+	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
+		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
+		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
+	}
+
+	if (cnt > MAX_KIQ_REG_TRY)
+		goto failed_kiq_read;
+
+	return adev->wb.wb[kiq->reg_val_offs];
+
+failed_kiq_read:
+	pr_err("failed to read reg:%x\n", reg);
+	return ~0;
+}
+
+void amdgpu_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
+{
+	signed long r, cnt = 0;
+	unsigned long flags;
+	uint32_t seq;
+	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
+	struct amdgpu_ring *ring = &kiq->ring;
+
+	BUG_ON(!ring->funcs->emit_wreg);
+
+	spin_lock_irqsave(&kiq->ring_lock, flags);
+	amdgpu_ring_alloc(ring, 32);
+	amdgpu_ring_emit_wreg(ring, reg, v);
+	amdgpu_fence_emit_polling(ring, &seq);
+	amdgpu_ring_commit(ring);
+	spin_unlock_irqrestore(&kiq->ring_lock, flags);
+
+	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
+
+	/* don't wait anymore for gpu reset case because this way may
+	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
+	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
+	 * never return if we keep waiting in virt_kiq_rreg, which cause
+	 * gpu_recover() hang there.
+	 *
+	 * also don't wait anymore for IRQ context
+	 * */
+	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
+		goto failed_kiq_write;
+
+	might_sleep();
+	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
+
+		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
+		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
+	}
+
+	if (cnt > MAX_KIQ_REG_TRY)
+		goto failed_kiq_write;
+
+	return;
+
+failed_kiq_write:
+	pr_err("failed to write reg:%x\n", reg);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index af4bd27..ca17ffb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -94,6 +94,7 @@ struct amdgpu_kiq {
 	struct amdgpu_ring	ring;
 	struct amdgpu_irq_src	irq;
 	const struct kiq_pm4_funcs *pmf;
+	uint32_t			reg_val_offs;
 };
 
 /*
@@ -375,4 +376,6 @@ int amdgpu_gfx_process_ras_data_cb(struct amdgpu_device *adev,
 int amdgpu_gfx_cp_ecc_error_irq(struct amdgpu_device *adev,
 				  struct amdgpu_irq_src *source,
 				  struct amdgpu_iv_entry *entry);
+uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
+void amdgpu_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 103033f..adc813c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -45,98 +45,6 @@ void amdgpu_virt_init_setting(struct amdgpu_device *adev)
 	adev->pg_flags = 0;
 }
 
-uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
-{
-	signed long r, cnt = 0;
-	unsigned long flags;
-	uint32_t seq;
-	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
-	struct amdgpu_ring *ring = &kiq->ring;
-
-	BUG_ON(!ring->funcs->emit_rreg);
-
-	spin_lock_irqsave(&kiq->ring_lock, flags);
-	amdgpu_ring_alloc(ring, 32);
-	amdgpu_ring_emit_rreg(ring, reg);
-	amdgpu_fence_emit_polling(ring, &seq);
-	amdgpu_ring_commit(ring);
-	spin_unlock_irqrestore(&kiq->ring_lock, flags);
-
-	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
-
-	/* don't wait anymore for gpu reset case because this way may
-	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
-	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
-	 * never return if we keep waiting in virt_kiq_rreg, which cause
-	 * gpu_recover() hang there.
-	 *
-	 * also don't wait anymore for IRQ context
-	 * */
-	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
-		goto failed_kiq_read;
-
-	might_sleep();
-	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
-		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
-		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
-	}
-
-	if (cnt > MAX_KIQ_REG_TRY)
-		goto failed_kiq_read;
-
-	return adev->wb.wb[adev->virt.reg_val_offs];
-
-failed_kiq_read:
-	pr_err("failed to read reg:%x\n", reg);
-	return ~0;
-}
-
-void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
-{
-	signed long r, cnt = 0;
-	unsigned long flags;
-	uint32_t seq;
-	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
-	struct amdgpu_ring *ring = &kiq->ring;
-
-	BUG_ON(!ring->funcs->emit_wreg);
-
-	spin_lock_irqsave(&kiq->ring_lock, flags);
-	amdgpu_ring_alloc(ring, 32);
-	amdgpu_ring_emit_wreg(ring, reg, v);
-	amdgpu_fence_emit_polling(ring, &seq);
-	amdgpu_ring_commit(ring);
-	spin_unlock_irqrestore(&kiq->ring_lock, flags);
-
-	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
-
-	/* don't wait anymore for gpu reset case because this way may
-	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
-	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
-	 * never return if we keep waiting in virt_kiq_rreg, which cause
-	 * gpu_recover() hang there.
-	 *
-	 * also don't wait anymore for IRQ context
-	 * */
-	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
-		goto failed_kiq_write;
-
-	might_sleep();
-	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
-
-		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
-		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
-	}
-
-	if (cnt > MAX_KIQ_REG_TRY)
-		goto failed_kiq_write;
-
-	return;
-
-failed_kiq_write:
-	pr_err("failed to write reg:%x\n", reg);
-}
-
 void amdgpu_virt_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
 					uint32_t reg0, uint32_t reg1,
 					uint32_t ref, uint32_t mask)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
index 4d1ac76..daaf909 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
@@ -287,8 +287,6 @@ static inline bool is_virtual_machine(void)
 
 bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev);
 void amdgpu_virt_init_setting(struct amdgpu_device *adev);
-uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
-void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
 void amdgpu_virt_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
 					uint32_t reg0, uint32_t rreg1,
 					uint32_t ref, uint32_t mask);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 1cfc508..e74ed06 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -4747,6 +4747,7 @@ static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
 static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
 {
 	struct amdgpu_device *adev = ring->adev;
+	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
 
 	amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
 	amdgpu_ring_write(ring, 0 |	/* src: register*/
@@ -4755,9 +4756,9 @@ static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
 	amdgpu_ring_write(ring, reg);
 	amdgpu_ring_write(ring, 0);
 	amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
-				adev->virt.reg_val_offs * 4));
+				kiq->reg_val_offs * 4));
 	amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
-				adev->virt.reg_val_offs * 4));
+				kiq->reg_val_offs * 4));
 }
 
 static void gfx_v10_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 306ee61..a46ec1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6450,6 +6450,7 @@ static void gfx_v8_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne
 static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
 {
 	struct amdgpu_device *adev = ring->adev;
+	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
 
 	amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
 	amdgpu_ring_write(ring, 0 |	/* src: register*/
@@ -6458,9 +6459,9 @@ static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
 	amdgpu_ring_write(ring, reg);
 	amdgpu_ring_write(ring, 0);
 	amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
-				adev->virt.reg_val_offs * 4));
+				kiq->reg_val_offs * 4));
 	amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
-				adev->virt.reg_val_offs * 4));
+				kiq->reg_val_offs * 4));
 }
 
 static void gfx_v8_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 0dfdc86..d9d7ee9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -5221,6 +5221,7 @@ static void gfx_v9_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne
 static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
 {
 	struct amdgpu_device *adev = ring->adev;
+	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
 
 	amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
 	amdgpu_ring_write(ring, 0 |	/* src: register*/
@@ -5229,9 +5230,9 @@ static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
 	amdgpu_ring_write(ring, reg);
 	amdgpu_ring_write(ring, 0);
 	amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
-				adev->virt.reg_val_offs * 4));
+				kiq->reg_val_offs * 4));
 	amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
-				adev->virt.reg_val_offs * 4));
+				kiq->reg_val_offs * 4));
 }
 
 static void gfx_v9_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
-- 
2.7.4

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

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

* [PATCH 2/3] drm/amdgpu: add kiq version interface for RREG32/WREG32
  2020-01-16 10:00 [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ chen gong
@ 2020-01-16 10:00 ` chen gong
  2020-01-16 10:00 ` [PATCH 3/3] drm/amdgpu: read gfx register using RREG32_KIQ macro chen gong
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: chen gong @ 2020-01-16 10:00 UTC (permalink / raw)
  To: amd-gfx; +Cc: chen gong

Reading some registers by mmio will result in hang when GPU is in
"gfxoff" state.This problem can be solved by GPU in "ring command
packages" way.

Signed-off-by: chen gong <curry.gong@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 4 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 63eab0c..b4b00e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1016,10 +1016,14 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
 
 #define AMDGPU_REGS_IDX       (1<<0)
 #define AMDGPU_REGS_NO_KIQ    (1<<1)
+#define AMDGPU_REGS_KIQ       (1<<2)
 
 #define RREG32_NO_KIQ(reg) amdgpu_mm_rreg(adev, (reg), AMDGPU_REGS_NO_KIQ)
 #define WREG32_NO_KIQ(reg, v) amdgpu_mm_wreg(adev, (reg), (v), AMDGPU_REGS_NO_KIQ)
 
+#define RREG32_KIQ(reg) amdgpu_mm_rreg(adev, (reg), AMDGPU_REGS_KIQ)
+#define WREG32_KIQ(reg, v) amdgpu_mm_wreg(adev, (reg), (v), AMDGPU_REGS_KIQ)
+
 #define RREG8(reg) amdgpu_mm_rreg8(adev, (reg))
 #define WREG8(reg, v) amdgpu_mm_wreg8(adev, (reg), (v))
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 963ea46..36c0f9d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -217,7 +217,7 @@ uint32_t amdgpu_mm_rreg(struct amdgpu_device *adev, uint32_t reg,
 {
 	uint32_t ret;
 
-	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
+	if ((acc_flags & AMDGPU_REGS_KIQ) || (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev)))
 		return amdgpu_kiq_rreg(adev, reg);
 
 	if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
@@ -295,7 +295,7 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
 		adev->last_mm_index = v;
 	}
 
-	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
+	if ((acc_flags & AMDGPU_REGS_KIQ) || (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev)))
 		return amdgpu_kiq_wreg(adev, reg, v);
 
 	if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
-- 
2.7.4

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

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

* [PATCH 3/3] drm/amdgpu: read gfx register using RREG32_KIQ macro
  2020-01-16 10:00 [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ chen gong
  2020-01-16 10:00 ` [PATCH 2/3] drm/amdgpu: add kiq version interface for RREG32/WREG32 chen gong
@ 2020-01-16 10:00 ` chen gong
  2020-01-16 12:44 ` [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ Christian König
  2020-01-17  3:01 ` Huang Rui
  3 siblings, 0 replies; 6+ messages in thread
From: chen gong @ 2020-01-16 10:00 UTC (permalink / raw)
  To: amd-gfx; +Cc: chen gong

Reading CP_MEM_SLP_CNTL register with RREG32_SOC15 macro will lead to
hang when GPU is in "gfxoff" state.
I do a uniform substitution here.

Signed-off-by: chen gong <curry.gong@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index d9d7ee9..03f4dbe 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -4717,12 +4717,12 @@ static void gfx_v9_0_get_clockgating_state(void *handle, u32 *flags)
 		*flags = 0;
 
 	/* AMD_CG_SUPPORT_GFX_MGCG */
-	data = RREG32_SOC15(GC, 0, mmRLC_CGTT_MGCG_OVERRIDE);
+	data = RREG32_KIQ(SOC15_REG_OFFSET(GC, 0, mmRLC_CGTT_MGCG_OVERRIDE));
 	if (!(data & RLC_CGTT_MGCG_OVERRIDE__GFXIP_MGCG_OVERRIDE_MASK))
 		*flags |= AMD_CG_SUPPORT_GFX_MGCG;
 
 	/* AMD_CG_SUPPORT_GFX_CGCG */
-	data = RREG32_SOC15(GC, 0, mmRLC_CGCG_CGLS_CTRL);
+	data = RREG32_KIQ(SOC15_REG_OFFSET(GC, 0, mmRLC_CGCG_CGLS_CTRL));
 	if (data & RLC_CGCG_CGLS_CTRL__CGCG_EN_MASK)
 		*flags |= AMD_CG_SUPPORT_GFX_CGCG;
 
@@ -4731,18 +4731,18 @@ static void gfx_v9_0_get_clockgating_state(void *handle, u32 *flags)
 		*flags |= AMD_CG_SUPPORT_GFX_CGLS;
 
 	/* AMD_CG_SUPPORT_GFX_RLC_LS */
-	data = RREG32_SOC15(GC, 0, mmRLC_MEM_SLP_CNTL);
+	data = RREG32_KIQ(SOC15_REG_OFFSET(GC, 0, mmRLC_MEM_SLP_CNTL));
 	if (data & RLC_MEM_SLP_CNTL__RLC_MEM_LS_EN_MASK)
 		*flags |= AMD_CG_SUPPORT_GFX_RLC_LS | AMD_CG_SUPPORT_GFX_MGLS;
 
 	/* AMD_CG_SUPPORT_GFX_CP_LS */
-	data = RREG32_SOC15(GC, 0, mmCP_MEM_SLP_CNTL);
+	data = RREG32_KIQ(SOC15_REG_OFFSET(GC, 0, mmCP_MEM_SLP_CNTL));
 	if (data & CP_MEM_SLP_CNTL__CP_MEM_LS_EN_MASK)
 		*flags |= AMD_CG_SUPPORT_GFX_CP_LS | AMD_CG_SUPPORT_GFX_MGLS;
 
 	if (adev->asic_type != CHIP_ARCTURUS) {
 		/* AMD_CG_SUPPORT_GFX_3D_CGCG */
-		data = RREG32_SOC15(GC, 0, mmRLC_CGCG_CGLS_CTRL_3D);
+		data = RREG32_KIQ(SOC15_REG_OFFSET(GC, 0, mmRLC_CGCG_CGLS_CTRL_3D));
 		if (data & RLC_CGCG_CGLS_CTRL_3D__CGCG_EN_MASK)
 			*flags |= AMD_CG_SUPPORT_GFX_3D_CGCG;
 
-- 
2.7.4

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

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

* Re: [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ
  2020-01-16 10:00 [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ chen gong
  2020-01-16 10:00 ` [PATCH 2/3] drm/amdgpu: add kiq version interface for RREG32/WREG32 chen gong
  2020-01-16 10:00 ` [PATCH 3/3] drm/amdgpu: read gfx register using RREG32_KIQ macro chen gong
@ 2020-01-16 12:44 ` Christian König
  2020-01-16 19:21   ` Alex Deucher
  2020-01-17  3:01 ` Huang Rui
  3 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2020-01-16 12:44 UTC (permalink / raw)
  To: chen gong, amd-gfx

Am 16.01.20 um 11:00 schrieb chen gong:
> Move amdgpu_virt_kiq_rreg/amdgpu_virt_kiq_wreg function to amdgpu_gfx.c,
> and rename them to amdgpu_kiq_rreg/amdgpu_kiq_wreg.Make it generic and
> flexible.
>
> Signed-off-by: chen gong <curry.gong@amd.com>

Alex has the last word on this since I'm not so deep into the KIQ, but 
at least to me that looks like a rather nice cleanup.

Feel free to add an Acked-by: Christian König <christian.koenig@amd.com> 
to the series.

Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 96 +++++++++++++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 92 ----------------------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   |  2 -
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c     |  5 +-
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      |  5 +-
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      |  5 +-
>   8 files changed, 108 insertions(+), 104 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 2c64d2a..963ea46 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -218,7 +218,7 @@ uint32_t amdgpu_mm_rreg(struct amdgpu_device *adev, uint32_t reg,
>   	uint32_t ret;
>   
>   	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
> -		return amdgpu_virt_kiq_rreg(adev, reg);
> +		return amdgpu_kiq_rreg(adev, reg);
>   
>   	if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
>   		ret = readl(((void __iomem *)adev->rmmio) + (reg * 4));
> @@ -296,7 +296,7 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
>   	}
>   
>   	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
> -		return amdgpu_virt_kiq_wreg(adev, reg, v);
> +		return amdgpu_kiq_wreg(adev, reg, v);
>   
>   	if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
>   		writel(v, ((void __iomem *)adev->rmmio) + (reg * 4));
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index b88b8b8..0f960b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -296,7 +296,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>   
>   	spin_lock_init(&kiq->ring_lock);
>   
> -	r = amdgpu_device_wb_get(adev, &adev->virt.reg_val_offs);
> +	r = amdgpu_device_wb_get(adev, &kiq->reg_val_offs);
>   	if (r)
>   		return r;
>   
> @@ -321,7 +321,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>   
>   void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring)
>   {
> -	amdgpu_device_wb_free(ring->adev, ring->adev->virt.reg_val_offs);
> +	amdgpu_device_wb_free(ring->adev, ring->adev->gfx.kiq.reg_val_offs);
>   	amdgpu_ring_fini(ring);
>   }
>   
> @@ -658,3 +658,95 @@ int amdgpu_gfx_cp_ecc_error_irq(struct amdgpu_device *adev,
>   	amdgpu_ras_interrupt_dispatch(adev, &ih_data);
>   	return 0;
>   }
> +
> +uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
> +{
> +	signed long r, cnt = 0;
> +	unsigned long flags;
> +	uint32_t seq;
> +	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> +	struct amdgpu_ring *ring = &kiq->ring;
> +
> +	BUG_ON(!ring->funcs->emit_rreg);
> +
> +	spin_lock_irqsave(&kiq->ring_lock, flags);
> +	amdgpu_ring_alloc(ring, 32);
> +	amdgpu_ring_emit_rreg(ring, reg);
> +	amdgpu_fence_emit_polling(ring, &seq);
> +	amdgpu_ring_commit(ring);
> +	spin_unlock_irqrestore(&kiq->ring_lock, flags);
> +
> +	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +
> +	/* don't wait anymore for gpu reset case because this way may
> +	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> +	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> +	 * never return if we keep waiting in virt_kiq_rreg, which cause
> +	 * gpu_recover() hang there.
> +	 *
> +	 * also don't wait anymore for IRQ context
> +	 * */
> +	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> +		goto failed_kiq_read;
> +
> +	might_sleep();
> +	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> +		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> +		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +	}
> +
> +	if (cnt > MAX_KIQ_REG_TRY)
> +		goto failed_kiq_read;
> +
> +	return adev->wb.wb[kiq->reg_val_offs];
> +
> +failed_kiq_read:
> +	pr_err("failed to read reg:%x\n", reg);
> +	return ~0;
> +}
> +
> +void amdgpu_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
> +{
> +	signed long r, cnt = 0;
> +	unsigned long flags;
> +	uint32_t seq;
> +	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> +	struct amdgpu_ring *ring = &kiq->ring;
> +
> +	BUG_ON(!ring->funcs->emit_wreg);
> +
> +	spin_lock_irqsave(&kiq->ring_lock, flags);
> +	amdgpu_ring_alloc(ring, 32);
> +	amdgpu_ring_emit_wreg(ring, reg, v);
> +	amdgpu_fence_emit_polling(ring, &seq);
> +	amdgpu_ring_commit(ring);
> +	spin_unlock_irqrestore(&kiq->ring_lock, flags);
> +
> +	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +
> +	/* don't wait anymore for gpu reset case because this way may
> +	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> +	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> +	 * never return if we keep waiting in virt_kiq_rreg, which cause
> +	 * gpu_recover() hang there.
> +	 *
> +	 * also don't wait anymore for IRQ context
> +	 * */
> +	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> +		goto failed_kiq_write;
> +
> +	might_sleep();
> +	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> +
> +		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> +		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +	}
> +
> +	if (cnt > MAX_KIQ_REG_TRY)
> +		goto failed_kiq_write;
> +
> +	return;
> +
> +failed_kiq_write:
> +	pr_err("failed to write reg:%x\n", reg);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index af4bd27..ca17ffb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -94,6 +94,7 @@ struct amdgpu_kiq {
>   	struct amdgpu_ring	ring;
>   	struct amdgpu_irq_src	irq;
>   	const struct kiq_pm4_funcs *pmf;
> +	uint32_t			reg_val_offs;
>   };
>   
>   /*
> @@ -375,4 +376,6 @@ int amdgpu_gfx_process_ras_data_cb(struct amdgpu_device *adev,
>   int amdgpu_gfx_cp_ecc_error_irq(struct amdgpu_device *adev,
>   				  struct amdgpu_irq_src *source,
>   				  struct amdgpu_iv_entry *entry);
> +uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
> +void amdgpu_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
>   #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 103033f..adc813c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -45,98 +45,6 @@ void amdgpu_virt_init_setting(struct amdgpu_device *adev)
>   	adev->pg_flags = 0;
>   }
>   
> -uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
> -{
> -	signed long r, cnt = 0;
> -	unsigned long flags;
> -	uint32_t seq;
> -	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> -	struct amdgpu_ring *ring = &kiq->ring;
> -
> -	BUG_ON(!ring->funcs->emit_rreg);
> -
> -	spin_lock_irqsave(&kiq->ring_lock, flags);
> -	amdgpu_ring_alloc(ring, 32);
> -	amdgpu_ring_emit_rreg(ring, reg);
> -	amdgpu_fence_emit_polling(ring, &seq);
> -	amdgpu_ring_commit(ring);
> -	spin_unlock_irqrestore(&kiq->ring_lock, flags);
> -
> -	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> -
> -	/* don't wait anymore for gpu reset case because this way may
> -	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> -	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> -	 * never return if we keep waiting in virt_kiq_rreg, which cause
> -	 * gpu_recover() hang there.
> -	 *
> -	 * also don't wait anymore for IRQ context
> -	 * */
> -	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> -		goto failed_kiq_read;
> -
> -	might_sleep();
> -	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> -		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> -		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> -	}
> -
> -	if (cnt > MAX_KIQ_REG_TRY)
> -		goto failed_kiq_read;
> -
> -	return adev->wb.wb[adev->virt.reg_val_offs];
> -
> -failed_kiq_read:
> -	pr_err("failed to read reg:%x\n", reg);
> -	return ~0;
> -}
> -
> -void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
> -{
> -	signed long r, cnt = 0;
> -	unsigned long flags;
> -	uint32_t seq;
> -	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> -	struct amdgpu_ring *ring = &kiq->ring;
> -
> -	BUG_ON(!ring->funcs->emit_wreg);
> -
> -	spin_lock_irqsave(&kiq->ring_lock, flags);
> -	amdgpu_ring_alloc(ring, 32);
> -	amdgpu_ring_emit_wreg(ring, reg, v);
> -	amdgpu_fence_emit_polling(ring, &seq);
> -	amdgpu_ring_commit(ring);
> -	spin_unlock_irqrestore(&kiq->ring_lock, flags);
> -
> -	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> -
> -	/* don't wait anymore for gpu reset case because this way may
> -	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> -	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> -	 * never return if we keep waiting in virt_kiq_rreg, which cause
> -	 * gpu_recover() hang there.
> -	 *
> -	 * also don't wait anymore for IRQ context
> -	 * */
> -	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> -		goto failed_kiq_write;
> -
> -	might_sleep();
> -	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> -
> -		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> -		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> -	}
> -
> -	if (cnt > MAX_KIQ_REG_TRY)
> -		goto failed_kiq_write;
> -
> -	return;
> -
> -failed_kiq_write:
> -	pr_err("failed to write reg:%x\n", reg);
> -}
> -
>   void amdgpu_virt_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
>   					uint32_t reg0, uint32_t reg1,
>   					uint32_t ref, uint32_t mask)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index 4d1ac76..daaf909 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -287,8 +287,6 @@ static inline bool is_virtual_machine(void)
>   
>   bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev);
>   void amdgpu_virt_init_setting(struct amdgpu_device *adev);
> -uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
> -void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
>   void amdgpu_virt_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
>   					uint32_t reg0, uint32_t rreg1,
>   					uint32_t ref, uint32_t mask);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 1cfc508..e74ed06 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -4747,6 +4747,7 @@ static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
>   static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>   {
>   	struct amdgpu_device *adev = ring->adev;
> +	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>   
>   	amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
>   	amdgpu_ring_write(ring, 0 |	/* src: register*/
> @@ -4755,9 +4756,9 @@ static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>   	amdgpu_ring_write(ring, reg);
>   	amdgpu_ring_write(ring, 0);
>   	amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>   	amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>   }
>   
>   static void gfx_v10_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 306ee61..a46ec1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6450,6 +6450,7 @@ static void gfx_v8_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne
>   static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>   {
>   	struct amdgpu_device *adev = ring->adev;
> +	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>   
>   	amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
>   	amdgpu_ring_write(ring, 0 |	/* src: register*/
> @@ -6458,9 +6459,9 @@ static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>   	amdgpu_ring_write(ring, reg);
>   	amdgpu_ring_write(ring, 0);
>   	amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>   	amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>   }
>   
>   static void gfx_v8_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 0dfdc86..d9d7ee9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -5221,6 +5221,7 @@ static void gfx_v9_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne
>   static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>   {
>   	struct amdgpu_device *adev = ring->adev;
> +	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>   
>   	amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
>   	amdgpu_ring_write(ring, 0 |	/* src: register*/
> @@ -5229,9 +5230,9 @@ static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>   	amdgpu_ring_write(ring, reg);
>   	amdgpu_ring_write(ring, 0);
>   	amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>   	amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>   }
>   
>   static void gfx_v9_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,

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

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

* Re: [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ
  2020-01-16 12:44 ` [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ Christian König
@ 2020-01-16 19:21   ` Alex Deucher
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Deucher @ 2020-01-16 19:21 UTC (permalink / raw)
  To: Christian Koenig; +Cc: amd-gfx list, chen gong

On Thu, Jan 16, 2020 at 7:44 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 16.01.20 um 11:00 schrieb chen gong:
> > Move amdgpu_virt_kiq_rreg/amdgpu_virt_kiq_wreg function to amdgpu_gfx.c,
> > and rename them to amdgpu_kiq_rreg/amdgpu_kiq_wreg.Make it generic and
> > flexible.
> >
> > Signed-off-by: chen gong <curry.gong@amd.com>
>
> Alex has the last word on this since I'm not so deep into the KIQ, but
> at least to me that looks like a rather nice cleanup.
>
> Feel free to add an Acked-by: Christian König <christian.koenig@amd.com>
> to the series.

Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
> Christian.
>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 96 +++++++++++++++++++++++++++++-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 92 ----------------------------
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   |  2 -
> >   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c     |  5 +-
> >   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      |  5 +-
> >   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      |  5 +-
> >   8 files changed, 108 insertions(+), 104 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 2c64d2a..963ea46 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -218,7 +218,7 @@ uint32_t amdgpu_mm_rreg(struct amdgpu_device *adev, uint32_t reg,
> >       uint32_t ret;
> >
> >       if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
> > -             return amdgpu_virt_kiq_rreg(adev, reg);
> > +             return amdgpu_kiq_rreg(adev, reg);
> >
> >       if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
> >               ret = readl(((void __iomem *)adev->rmmio) + (reg * 4));
> > @@ -296,7 +296,7 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
> >       }
> >
> >       if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
> > -             return amdgpu_virt_kiq_wreg(adev, reg, v);
> > +             return amdgpu_kiq_wreg(adev, reg, v);
> >
> >       if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
> >               writel(v, ((void __iomem *)adev->rmmio) + (reg * 4));
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > index b88b8b8..0f960b4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > @@ -296,7 +296,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
> >
> >       spin_lock_init(&kiq->ring_lock);
> >
> > -     r = amdgpu_device_wb_get(adev, &adev->virt.reg_val_offs);
> > +     r = amdgpu_device_wb_get(adev, &kiq->reg_val_offs);
> >       if (r)
> >               return r;
> >
> > @@ -321,7 +321,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
> >
> >   void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring)
> >   {
> > -     amdgpu_device_wb_free(ring->adev, ring->adev->virt.reg_val_offs);
> > +     amdgpu_device_wb_free(ring->adev, ring->adev->gfx.kiq.reg_val_offs);
> >       amdgpu_ring_fini(ring);
> >   }
> >
> > @@ -658,3 +658,95 @@ int amdgpu_gfx_cp_ecc_error_irq(struct amdgpu_device *adev,
> >       amdgpu_ras_interrupt_dispatch(adev, &ih_data);
> >       return 0;
> >   }
> > +
> > +uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
> > +{
> > +     signed long r, cnt = 0;
> > +     unsigned long flags;
> > +     uint32_t seq;
> > +     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> > +     struct amdgpu_ring *ring = &kiq->ring;
> > +
> > +     BUG_ON(!ring->funcs->emit_rreg);
> > +
> > +     spin_lock_irqsave(&kiq->ring_lock, flags);
> > +     amdgpu_ring_alloc(ring, 32);
> > +     amdgpu_ring_emit_rreg(ring, reg);
> > +     amdgpu_fence_emit_polling(ring, &seq);
> > +     amdgpu_ring_commit(ring);
> > +     spin_unlock_irqrestore(&kiq->ring_lock, flags);
> > +
> > +     r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > +
> > +     /* don't wait anymore for gpu reset case because this way may
> > +      * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> > +      * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> > +      * never return if we keep waiting in virt_kiq_rreg, which cause
> > +      * gpu_recover() hang there.
> > +      *
> > +      * also don't wait anymore for IRQ context
> > +      * */
> > +     if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> > +             goto failed_kiq_read;
> > +
> > +     might_sleep();
> > +     while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> > +             msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> > +             r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > +     }
> > +
> > +     if (cnt > MAX_KIQ_REG_TRY)
> > +             goto failed_kiq_read;
> > +
> > +     return adev->wb.wb[kiq->reg_val_offs];
> > +
> > +failed_kiq_read:
> > +     pr_err("failed to read reg:%x\n", reg);
> > +     return ~0;
> > +}
> > +
> > +void amdgpu_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
> > +{
> > +     signed long r, cnt = 0;
> > +     unsigned long flags;
> > +     uint32_t seq;
> > +     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> > +     struct amdgpu_ring *ring = &kiq->ring;
> > +
> > +     BUG_ON(!ring->funcs->emit_wreg);
> > +
> > +     spin_lock_irqsave(&kiq->ring_lock, flags);
> > +     amdgpu_ring_alloc(ring, 32);
> > +     amdgpu_ring_emit_wreg(ring, reg, v);
> > +     amdgpu_fence_emit_polling(ring, &seq);
> > +     amdgpu_ring_commit(ring);
> > +     spin_unlock_irqrestore(&kiq->ring_lock, flags);
> > +
> > +     r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > +
> > +     /* don't wait anymore for gpu reset case because this way may
> > +      * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> > +      * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> > +      * never return if we keep waiting in virt_kiq_rreg, which cause
> > +      * gpu_recover() hang there.
> > +      *
> > +      * also don't wait anymore for IRQ context
> > +      * */
> > +     if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> > +             goto failed_kiq_write;
> > +
> > +     might_sleep();
> > +     while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> > +
> > +             msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> > +             r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > +     }
> > +
> > +     if (cnt > MAX_KIQ_REG_TRY)
> > +             goto failed_kiq_write;
> > +
> > +     return;
> > +
> > +failed_kiq_write:
> > +     pr_err("failed to write reg:%x\n", reg);
> > +}
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > index af4bd27..ca17ffb 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > @@ -94,6 +94,7 @@ struct amdgpu_kiq {
> >       struct amdgpu_ring      ring;
> >       struct amdgpu_irq_src   irq;
> >       const struct kiq_pm4_funcs *pmf;
> > +     uint32_t                        reg_val_offs;
> >   };
> >
> >   /*
> > @@ -375,4 +376,6 @@ int amdgpu_gfx_process_ras_data_cb(struct amdgpu_device *adev,
> >   int amdgpu_gfx_cp_ecc_error_irq(struct amdgpu_device *adev,
> >                                 struct amdgpu_irq_src *source,
> >                                 struct amdgpu_iv_entry *entry);
> > +uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
> > +void amdgpu_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
> >   #endif
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > index 103033f..adc813c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > @@ -45,98 +45,6 @@ void amdgpu_virt_init_setting(struct amdgpu_device *adev)
> >       adev->pg_flags = 0;
> >   }
> >
> > -uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
> > -{
> > -     signed long r, cnt = 0;
> > -     unsigned long flags;
> > -     uint32_t seq;
> > -     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> > -     struct amdgpu_ring *ring = &kiq->ring;
> > -
> > -     BUG_ON(!ring->funcs->emit_rreg);
> > -
> > -     spin_lock_irqsave(&kiq->ring_lock, flags);
> > -     amdgpu_ring_alloc(ring, 32);
> > -     amdgpu_ring_emit_rreg(ring, reg);
> > -     amdgpu_fence_emit_polling(ring, &seq);
> > -     amdgpu_ring_commit(ring);
> > -     spin_unlock_irqrestore(&kiq->ring_lock, flags);
> > -
> > -     r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > -
> > -     /* don't wait anymore for gpu reset case because this way may
> > -      * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> > -      * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> > -      * never return if we keep waiting in virt_kiq_rreg, which cause
> > -      * gpu_recover() hang there.
> > -      *
> > -      * also don't wait anymore for IRQ context
> > -      * */
> > -     if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> > -             goto failed_kiq_read;
> > -
> > -     might_sleep();
> > -     while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> > -             msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> > -             r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > -     }
> > -
> > -     if (cnt > MAX_KIQ_REG_TRY)
> > -             goto failed_kiq_read;
> > -
> > -     return adev->wb.wb[adev->virt.reg_val_offs];
> > -
> > -failed_kiq_read:
> > -     pr_err("failed to read reg:%x\n", reg);
> > -     return ~0;
> > -}
> > -
> > -void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
> > -{
> > -     signed long r, cnt = 0;
> > -     unsigned long flags;
> > -     uint32_t seq;
> > -     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> > -     struct amdgpu_ring *ring = &kiq->ring;
> > -
> > -     BUG_ON(!ring->funcs->emit_wreg);
> > -
> > -     spin_lock_irqsave(&kiq->ring_lock, flags);
> > -     amdgpu_ring_alloc(ring, 32);
> > -     amdgpu_ring_emit_wreg(ring, reg, v);
> > -     amdgpu_fence_emit_polling(ring, &seq);
> > -     amdgpu_ring_commit(ring);
> > -     spin_unlock_irqrestore(&kiq->ring_lock, flags);
> > -
> > -     r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > -
> > -     /* don't wait anymore for gpu reset case because this way may
> > -      * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> > -      * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> > -      * never return if we keep waiting in virt_kiq_rreg, which cause
> > -      * gpu_recover() hang there.
> > -      *
> > -      * also don't wait anymore for IRQ context
> > -      * */
> > -     if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> > -             goto failed_kiq_write;
> > -
> > -     might_sleep();
> > -     while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> > -
> > -             msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> > -             r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> > -     }
> > -
> > -     if (cnt > MAX_KIQ_REG_TRY)
> > -             goto failed_kiq_write;
> > -
> > -     return;
> > -
> > -failed_kiq_write:
> > -     pr_err("failed to write reg:%x\n", reg);
> > -}
> > -
> >   void amdgpu_virt_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
> >                                       uint32_t reg0, uint32_t reg1,
> >                                       uint32_t ref, uint32_t mask)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > index 4d1ac76..daaf909 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > @@ -287,8 +287,6 @@ static inline bool is_virtual_machine(void)
> >
> >   bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev);
> >   void amdgpu_virt_init_setting(struct amdgpu_device *adev);
> > -uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
> > -void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
> >   void amdgpu_virt_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
> >                                       uint32_t reg0, uint32_t rreg1,
> >                                       uint32_t ref, uint32_t mask);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > index 1cfc508..e74ed06 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > @@ -4747,6 +4747,7 @@ static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
> >   static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >   {
> >       struct amdgpu_device *adev = ring->adev;
> > +     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> >
> >       amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
> >       amdgpu_ring_write(ring, 0 |     /* src: register*/
> > @@ -4755,9 +4756,9 @@ static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >       amdgpu_ring_write(ring, reg);
> >       amdgpu_ring_write(ring, 0);
> >       amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >       amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >   }
> >
> >   static void gfx_v10_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 306ee61..a46ec1c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -6450,6 +6450,7 @@ static void gfx_v8_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne
> >   static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >   {
> >       struct amdgpu_device *adev = ring->adev;
> > +     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> >
> >       amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
> >       amdgpu_ring_write(ring, 0 |     /* src: register*/
> > @@ -6458,9 +6459,9 @@ static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >       amdgpu_ring_write(ring, reg);
> >       amdgpu_ring_write(ring, 0);
> >       amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >       amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >   }
> >
> >   static void gfx_v8_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > index 0dfdc86..d9d7ee9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > @@ -5221,6 +5221,7 @@ static void gfx_v9_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne
> >   static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >   {
> >       struct amdgpu_device *adev = ring->adev;
> > +     struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> >
> >       amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
> >       amdgpu_ring_write(ring, 0 |     /* src: register*/
> > @@ -5229,9 +5230,9 @@ static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> >       amdgpu_ring_write(ring, reg);
> >       amdgpu_ring_write(ring, 0);
> >       amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >       amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> > -                             adev->virt.reg_val_offs * 4));
> > +                             kiq->reg_val_offs * 4));
> >   }
> >
> >   static void gfx_v9_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ
  2020-01-16 10:00 [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ chen gong
                   ` (2 preceding siblings ...)
  2020-01-16 12:44 ` [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ Christian König
@ 2020-01-17  3:01 ` Huang Rui
  3 siblings, 0 replies; 6+ messages in thread
From: Huang Rui @ 2020-01-17  3:01 UTC (permalink / raw)
  To: chen gong; +Cc: amd-gfx

On Thu, Jan 16, 2020 at 06:00:54PM +0800, chen gong wrote:
> Move amdgpu_virt_kiq_rreg/amdgpu_virt_kiq_wreg function to amdgpu_gfx.c,
> and rename them to amdgpu_kiq_rreg/amdgpu_kiq_wreg.Make it generic and
> flexible.
> 

Thanks.

Series are Reviewed-by: Huang Rui <ray.huang@amd.com>

> Signed-off-by: chen gong <curry.gong@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 96 +++++++++++++++++++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 92 ----------------------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   |  2 -
>  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c     |  5 +-
>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      |  5 +-
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      |  5 +-
>  8 files changed, 108 insertions(+), 104 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 2c64d2a..963ea46 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -218,7 +218,7 @@ uint32_t amdgpu_mm_rreg(struct amdgpu_device *adev, uint32_t reg,
>  	uint32_t ret;
>  
>  	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
> -		return amdgpu_virt_kiq_rreg(adev, reg);
> +		return amdgpu_kiq_rreg(adev, reg);
>  
>  	if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
>  		ret = readl(((void __iomem *)adev->rmmio) + (reg * 4));
> @@ -296,7 +296,7 @@ void amdgpu_mm_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v,
>  	}
>  
>  	if (!(acc_flags & AMDGPU_REGS_NO_KIQ) && amdgpu_sriov_runtime(adev))
> -		return amdgpu_virt_kiq_wreg(adev, reg, v);
> +		return amdgpu_kiq_wreg(adev, reg, v);
>  
>  	if ((reg * 4) < adev->rmmio_size && !(acc_flags & AMDGPU_REGS_IDX))
>  		writel(v, ((void __iomem *)adev->rmmio) + (reg * 4));
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index b88b8b8..0f960b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -296,7 +296,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>  
>  	spin_lock_init(&kiq->ring_lock);
>  
> -	r = amdgpu_device_wb_get(adev, &adev->virt.reg_val_offs);
> +	r = amdgpu_device_wb_get(adev, &kiq->reg_val_offs);
>  	if (r)
>  		return r;
>  
> @@ -321,7 +321,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>  
>  void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring)
>  {
> -	amdgpu_device_wb_free(ring->adev, ring->adev->virt.reg_val_offs);
> +	amdgpu_device_wb_free(ring->adev, ring->adev->gfx.kiq.reg_val_offs);
>  	amdgpu_ring_fini(ring);
>  }
>  
> @@ -658,3 +658,95 @@ int amdgpu_gfx_cp_ecc_error_irq(struct amdgpu_device *adev,
>  	amdgpu_ras_interrupt_dispatch(adev, &ih_data);
>  	return 0;
>  }
> +
> +uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
> +{
> +	signed long r, cnt = 0;
> +	unsigned long flags;
> +	uint32_t seq;
> +	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> +	struct amdgpu_ring *ring = &kiq->ring;
> +
> +	BUG_ON(!ring->funcs->emit_rreg);
> +
> +	spin_lock_irqsave(&kiq->ring_lock, flags);
> +	amdgpu_ring_alloc(ring, 32);
> +	amdgpu_ring_emit_rreg(ring, reg);
> +	amdgpu_fence_emit_polling(ring, &seq);
> +	amdgpu_ring_commit(ring);
> +	spin_unlock_irqrestore(&kiq->ring_lock, flags);
> +
> +	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +
> +	/* don't wait anymore for gpu reset case because this way may
> +	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> +	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> +	 * never return if we keep waiting in virt_kiq_rreg, which cause
> +	 * gpu_recover() hang there.
> +	 *
> +	 * also don't wait anymore for IRQ context
> +	 * */
> +	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> +		goto failed_kiq_read;
> +
> +	might_sleep();
> +	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> +		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> +		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +	}
> +
> +	if (cnt > MAX_KIQ_REG_TRY)
> +		goto failed_kiq_read;
> +
> +	return adev->wb.wb[kiq->reg_val_offs];
> +
> +failed_kiq_read:
> +	pr_err("failed to read reg:%x\n", reg);
> +	return ~0;
> +}
> +
> +void amdgpu_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
> +{
> +	signed long r, cnt = 0;
> +	unsigned long flags;
> +	uint32_t seq;
> +	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> +	struct amdgpu_ring *ring = &kiq->ring;
> +
> +	BUG_ON(!ring->funcs->emit_wreg);
> +
> +	spin_lock_irqsave(&kiq->ring_lock, flags);
> +	amdgpu_ring_alloc(ring, 32);
> +	amdgpu_ring_emit_wreg(ring, reg, v);
> +	amdgpu_fence_emit_polling(ring, &seq);
> +	amdgpu_ring_commit(ring);
> +	spin_unlock_irqrestore(&kiq->ring_lock, flags);
> +
> +	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +
> +	/* don't wait anymore for gpu reset case because this way may
> +	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> +	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> +	 * never return if we keep waiting in virt_kiq_rreg, which cause
> +	 * gpu_recover() hang there.
> +	 *
> +	 * also don't wait anymore for IRQ context
> +	 * */
> +	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> +		goto failed_kiq_write;
> +
> +	might_sleep();
> +	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> +
> +		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> +		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> +	}
> +
> +	if (cnt > MAX_KIQ_REG_TRY)
> +		goto failed_kiq_write;
> +
> +	return;
> +
> +failed_kiq_write:
> +	pr_err("failed to write reg:%x\n", reg);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index af4bd27..ca17ffb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -94,6 +94,7 @@ struct amdgpu_kiq {
>  	struct amdgpu_ring	ring;
>  	struct amdgpu_irq_src	irq;
>  	const struct kiq_pm4_funcs *pmf;
> +	uint32_t			reg_val_offs;
>  };
>  
>  /*
> @@ -375,4 +376,6 @@ int amdgpu_gfx_process_ras_data_cb(struct amdgpu_device *adev,
>  int amdgpu_gfx_cp_ecc_error_irq(struct amdgpu_device *adev,
>  				  struct amdgpu_irq_src *source,
>  				  struct amdgpu_iv_entry *entry);
> +uint32_t amdgpu_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
> +void amdgpu_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 103033f..adc813c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -45,98 +45,6 @@ void amdgpu_virt_init_setting(struct amdgpu_device *adev)
>  	adev->pg_flags = 0;
>  }
>  
> -uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
> -{
> -	signed long r, cnt = 0;
> -	unsigned long flags;
> -	uint32_t seq;
> -	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> -	struct amdgpu_ring *ring = &kiq->ring;
> -
> -	BUG_ON(!ring->funcs->emit_rreg);
> -
> -	spin_lock_irqsave(&kiq->ring_lock, flags);
> -	amdgpu_ring_alloc(ring, 32);
> -	amdgpu_ring_emit_rreg(ring, reg);
> -	amdgpu_fence_emit_polling(ring, &seq);
> -	amdgpu_ring_commit(ring);
> -	spin_unlock_irqrestore(&kiq->ring_lock, flags);
> -
> -	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> -
> -	/* don't wait anymore for gpu reset case because this way may
> -	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> -	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> -	 * never return if we keep waiting in virt_kiq_rreg, which cause
> -	 * gpu_recover() hang there.
> -	 *
> -	 * also don't wait anymore for IRQ context
> -	 * */
> -	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> -		goto failed_kiq_read;
> -
> -	might_sleep();
> -	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> -		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> -		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> -	}
> -
> -	if (cnt > MAX_KIQ_REG_TRY)
> -		goto failed_kiq_read;
> -
> -	return adev->wb.wb[adev->virt.reg_val_offs];
> -
> -failed_kiq_read:
> -	pr_err("failed to read reg:%x\n", reg);
> -	return ~0;
> -}
> -
> -void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
> -{
> -	signed long r, cnt = 0;
> -	unsigned long flags;
> -	uint32_t seq;
> -	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
> -	struct amdgpu_ring *ring = &kiq->ring;
> -
> -	BUG_ON(!ring->funcs->emit_wreg);
> -
> -	spin_lock_irqsave(&kiq->ring_lock, flags);
> -	amdgpu_ring_alloc(ring, 32);
> -	amdgpu_ring_emit_wreg(ring, reg, v);
> -	amdgpu_fence_emit_polling(ring, &seq);
> -	amdgpu_ring_commit(ring);
> -	spin_unlock_irqrestore(&kiq->ring_lock, flags);
> -
> -	r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> -
> -	/* don't wait anymore for gpu reset case because this way may
> -	 * block gpu_recover() routine forever, e.g. this virt_kiq_rreg
> -	 * is triggered in TTM and ttm_bo_lock_delayed_workqueue() will
> -	 * never return if we keep waiting in virt_kiq_rreg, which cause
> -	 * gpu_recover() hang there.
> -	 *
> -	 * also don't wait anymore for IRQ context
> -	 * */
> -	if (r < 1 && (adev->in_gpu_reset || in_interrupt()))
> -		goto failed_kiq_write;
> -
> -	might_sleep();
> -	while (r < 1 && cnt++ < MAX_KIQ_REG_TRY) {
> -
> -		msleep(MAX_KIQ_REG_BAILOUT_INTERVAL);
> -		r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
> -	}
> -
> -	if (cnt > MAX_KIQ_REG_TRY)
> -		goto failed_kiq_write;
> -
> -	return;
> -
> -failed_kiq_write:
> -	pr_err("failed to write reg:%x\n", reg);
> -}
> -
>  void amdgpu_virt_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
>  					uint32_t reg0, uint32_t reg1,
>  					uint32_t ref, uint32_t mask)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index 4d1ac76..daaf909 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -287,8 +287,6 @@ static inline bool is_virtual_machine(void)
>  
>  bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev);
>  void amdgpu_virt_init_setting(struct amdgpu_device *adev);
> -uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
> -void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
>  void amdgpu_virt_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
>  					uint32_t reg0, uint32_t rreg1,
>  					uint32_t ref, uint32_t mask);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 1cfc508..e74ed06 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -4747,6 +4747,7 @@ static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
>  static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>  {
>  	struct amdgpu_device *adev = ring->adev;
> +	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>  
>  	amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
>  	amdgpu_ring_write(ring, 0 |	/* src: register*/
> @@ -4755,9 +4756,9 @@ static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>  	amdgpu_ring_write(ring, reg);
>  	amdgpu_ring_write(ring, 0);
>  	amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>  	amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>  }
>  
>  static void gfx_v10_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 306ee61..a46ec1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6450,6 +6450,7 @@ static void gfx_v8_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne
>  static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>  {
>  	struct amdgpu_device *adev = ring->adev;
> +	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>  
>  	amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
>  	amdgpu_ring_write(ring, 0 |	/* src: register*/
> @@ -6458,9 +6459,9 @@ static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>  	amdgpu_ring_write(ring, reg);
>  	amdgpu_ring_write(ring, 0);
>  	amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>  	amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>  }
>  
>  static void gfx_v8_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 0dfdc86..d9d7ee9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -5221,6 +5221,7 @@ static void gfx_v9_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne
>  static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>  {
>  	struct amdgpu_device *adev = ring->adev;
> +	struct amdgpu_kiq *kiq = &adev->gfx.kiq;
>  
>  	amdgpu_ring_write(ring, PACKET3(PACKET3_COPY_DATA, 4));
>  	amdgpu_ring_write(ring, 0 |	/* src: register*/
> @@ -5229,9 +5230,9 @@ static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>  	amdgpu_ring_write(ring, reg);
>  	amdgpu_ring_write(ring, 0);
>  	amdgpu_ring_write(ring, lower_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>  	amdgpu_ring_write(ring, upper_32_bits(adev->wb.gpu_addr +
> -				adev->virt.reg_val_offs * 4));
> +				kiq->reg_val_offs * 4));
>  }
>  
>  static void gfx_v9_0_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> -- 
> 2.7.4
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cray.huang%40amd.com%7C5af923f51c4f4ef15d7708d79a6b02f5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637147656726826661&amp;sdata=IDIyfAQYOOevkv4LTO05%2BuEdtYzowncU%2BraBH4fFFEw%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

end of thread, other threads:[~2020-01-17  3:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-16 10:00 [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ chen gong
2020-01-16 10:00 ` [PATCH 2/3] drm/amdgpu: add kiq version interface for RREG32/WREG32 chen gong
2020-01-16 10:00 ` [PATCH 3/3] drm/amdgpu: read gfx register using RREG32_KIQ macro chen gong
2020-01-16 12:44 ` [PATCH 1/3] drm/amdgpu: provide a generic function interface for reading/writing register by KIQ Christian König
2020-01-16 19:21   ` Alex Deucher
2020-01-17  3:01 ` Huang Rui

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