amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] LSDMA support
@ 2022-05-05 20:03 Alex Deucher
  2022-05-05 20:04 ` [PATCH 2/8] drm/amdgpu: add lsdma block Alex Deucher
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Alex Deucher @ 2022-05-05 20:03 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher

Light SDMA (LSDMA) is a supplimental SDMA block on SDMA 6.x
for use by the kernel driver.  Patch 1 adds large header
updates so was not sent to the list.

Hawking Zhang (1):
  drm/amdgpu: add lsdma v6_0_0 ip headers

Likun Gao (7):
  drm/amdgpu: add lsdma block
  drm/amdgpu: support mem copy for LSDMA
  drm/amdgpu: support fill mem for LSDMA
  drm/amdgpu: add LSDMA block for LSDMA v6.0.0
  drm/amdgpu: add LSDMA block for LSDMA v6.0.2
  drm/amdgpu: support memory power gating for lsdma
  drm/amdgpu: support memory power gating for lsdma 6.0.2

 drivers/gpu/drm/amd/amdgpu/Makefile           |    4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |    5 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c |   12 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c     |   91 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h     |   47 +
 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c       |  121 ++
 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.h       |   31 +
 drivers/gpu/drm/amd/amdgpu/soc21.c            |   13 +-
 .../asic_reg/lsdma/lsdma_6_0_0_offset.h       |  391 +++++
 .../asic_reg/lsdma/lsdma_6_0_0_sh_mask.h      | 1439 +++++++++++++++++
 drivers/gpu/drm/amd/include/soc15_hw_ip.h     |    1 +
 11 files changed, 2152 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.h
 create mode 100644 drivers/gpu/drm/amd/include/asic_reg/lsdma/lsdma_6_0_0_offset.h
 create mode 100644 drivers/gpu/drm/amd/include/asic_reg/lsdma/lsdma_6_0_0_sh_mask.h

-- 
2.35.1


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

* [PATCH 2/8] drm/amdgpu: add lsdma block
  2022-05-05 20:03 [PATCH 0/8] LSDMA support Alex Deucher
@ 2022-05-05 20:04 ` Alex Deucher
  2022-05-05 20:04 ` [PATCH 3/8] drm/amdgpu: support mem copy for LSDMA Alex Deucher
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2022-05-05 20:04 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Likun Gao, Hawking Zhang

From: Likun Gao <Likun.Gao@amd.com>

Add Light SDMA (LSDMA) block and related function. LSDMA
is a small instance of SDMA mainly for kernel driver use.

Signed-off-by: Likun Gao <Likun.Gao@amd.com>
Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/Makefile           |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  5 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c     | 25 +++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h     | 35 +++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c       | 33 +++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.h       | 31 ++++++++++++++++
 drivers/gpu/drm/amd/include/soc15_hw_ip.h     |  1 +
 8 files changed, 135 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index de7abc7c371f..9ca9f86e04c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -58,7 +58,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 	amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
 	amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
 	amdgpu_fw_attestation.o amdgpu_securedisplay.o \
-	amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o
+	amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o
 
 amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
 
@@ -75,7 +75,7 @@ amdgpu-y += \
 	vi.o mxgpu_vi.o nbio_v6_1.o soc15.o emu_soc.o mxgpu_ai.o nbio_v7_0.o vega10_reg_init.o \
 	vega20_reg_init.o nbio_v7_4.o nbio_v2_3.o nv.o arct_reg_init.o mxgpu_nv.o \
 	nbio_v7_2.o hdp_v4_0.o hdp_v5_0.o aldebaran_reg_init.o aldebaran.o soc21.o \
-	nbio_v4_3.o hdp_v6_0.o nbio_v7_7.o hdp_v5_2.o
+	nbio_v4_3.o hdp_v6_0.o nbio_v7_7.o hdp_v5_2.o lsdma_v6_0.o
 
 # add DF block
 amdgpu-y += \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 578a405f777c..058f272e40ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -86,6 +86,7 @@
 #include "amdgpu_gmc.h"
 #include "amdgpu_gfx.h"
 #include "amdgpu_sdma.h"
+#include "amdgpu_lsdma.h"
 #include "amdgpu_nbio.h"
 #include "amdgpu_hdp.h"
 #include "amdgpu_dm.h"
@@ -643,6 +644,7 @@ enum amd_hw_ip_block_type {
 	SDMA5_HWIP,
 	SDMA6_HWIP,
 	SDMA7_HWIP,
+	LSDMA_HWIP,
 	MMHUB_HWIP,
 	ATHUB_HWIP,
 	NBIO_HWIP,
@@ -909,6 +911,9 @@ struct amdgpu_device {
 	/* sdma */
 	struct amdgpu_sdma		sdma;
 
+	/* lsdma */
+	struct amdgpu_lsdma		lsdma;
+
 	/* uvd */
 	struct amdgpu_uvd		uvd;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 263fedadbf5b..3f9f2d6cc6c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -66,6 +66,7 @@
 #include "sdma_v5_0.h"
 #include "sdma_v5_2.h"
 #include "sdma_v6_0.h"
+#include "lsdma_v6_0.h"
 #include "vcn_v2_0.h"
 #include "jpeg_v2_0.h"
 #include "vcn_v3_0.h"
@@ -124,6 +125,7 @@ static const char *hw_id_names[HW_ID_MAX] = {
 	[SDMA1_HWID]		= "SDMA1",
 	[SDMA2_HWID]		= "SDMA2",
 	[SDMA3_HWID]		= "SDMA3",
+	[LSDMA_HWID]		= "LSDMA",
 	[ISP_HWID]		= "ISP",
 	[DBGU_IO_HWID]		= "DBGU_IO",
 	[DF_HWID]		= "DF",
@@ -173,6 +175,7 @@ static int hw_id_map[MAX_HWIP] = {
 	[SDMA1_HWIP]	= SDMA1_HWID,
 	[SDMA2_HWIP]    = SDMA2_HWID,
 	[SDMA3_HWIP]    = SDMA3_HWID,
+	[LSDMA_HWIP]    = LSDMA_HWID,
 	[MMHUB_HWIP]	= MMHUB_HWID,
 	[ATHUB_HWIP]	= ATHUB_HWID,
 	[NBIO_HWIP]	= NBIF_HWID,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
new file mode 100644
index 000000000000..af00a66f8282
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2022 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include "amdgpu.h"
+#include "amdgpu_lsdma.h"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
new file mode 100644
index 000000000000..4df4da423181
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright 2022 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __AMDGPU_LSDMA_H__
+#define __AMDGPU_LSDMA_H__
+
+struct amdgpu_lsdma {
+	const struct amdgpu_lsdma_funcs      *funcs;
+};
+
+struct amdgpu_lsdma_funcs
+{
+};
+
+#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
new file mode 100644
index 000000000000..b611ade53be2
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2022 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include <linux/delay.h>
+#include "amdgpu.h"
+#include "lsdma_v6_0.h"
+#include "amdgpu_lsdma.h"
+
+#include "lsdma/lsdma_6_0_0_offset.h"
+#include "lsdma/lsdma_6_0_0_sh_mask.h"
+
+const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
+};
diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.h b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.h
new file mode 100644
index 000000000000..3ef79be1a9bf
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2022 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __LSDMA_V6_0_H__
+#define __LSDMA_V6_0_H__
+
+#include "soc15_common.h"
+
+extern const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs;
+
+#endif /* __LSDMA_V6_0_H__ */
diff --git a/drivers/gpu/drm/amd/include/soc15_hw_ip.h b/drivers/gpu/drm/amd/include/soc15_hw_ip.h
index c1519d20596a..bc9783da7e96 100644
--- a/drivers/gpu/drm/amd/include/soc15_hw_ip.h
+++ b/drivers/gpu/drm/amd/include/soc15_hw_ip.h
@@ -86,6 +86,7 @@
 #define PCS_HWID                                          80
 #define DDCL_HWID                                         89
 #define SST_HWID                                          90
+#define LSDMA_HWID                                        91
 #define IOAGR_HWID                                       100
 #define NBIF_HWID                                        108
 #define IOAPIC_HWID                                      124
-- 
2.35.1


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

* [PATCH 3/8] drm/amdgpu: support mem copy for LSDMA
  2022-05-05 20:03 [PATCH 0/8] LSDMA support Alex Deucher
  2022-05-05 20:04 ` [PATCH 2/8] drm/amdgpu: add lsdma block Alex Deucher
@ 2022-05-05 20:04 ` Alex Deucher
  2022-05-06  5:02   ` Luben Tuikov
  2022-05-05 20:04 ` [PATCH 4/8] drm/amdgpu: support fill mem " Alex Deucher
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Alex Deucher @ 2022-05-05 20:04 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Likun Gao, Christian König

From: Likun Gao <Likun.Gao@amd.com>

Support memory to memory linear copy in PIO mode for LSDMA.

Signed-off-by: Likun Gao <Likun.Gao@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c | 26 ++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h |  5 +++
 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c   | 44 +++++++++++++++++++++++
 3 files changed, 75 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
index af00a66f8282..3f1c674afe41 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
@@ -23,3 +23,29 @@
 
 #include "amdgpu.h"
 #include "amdgpu_lsdma.h"
+
+#define AMDGPU_LSDMA_MAX_SIZE	0x2000000ULL
+
+int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev,
+			  uint64_t src_addr,
+			  uint64_t dst_addr,
+			  uint64_t mem_size)
+{
+	int ret;
+
+	if (mem_size == 0)
+		return -EINVAL;
+
+	while(mem_size > 0) {
+		uint64_t current_copy_size = min(mem_size, AMDGPU_LSDMA_MAX_SIZE);
+
+		ret = adev->lsdma.funcs->copy_mem(adev, src_addr, dst_addr, current_copy_size);
+		if (ret)
+			return ret;
+		src_addr += current_copy_size;
+		dst_addr += current_copy_size;
+		mem_size -= current_copy_size;
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
index 4df4da423181..be397666e4c1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
@@ -30,6 +30,11 @@ struct amdgpu_lsdma {
 
 struct amdgpu_lsdma_funcs
 {
+	int (*copy_mem)(struct amdgpu_device *adev, uint64_t src_addr,
+                    uint64_t dst_addr, uint64_t size);
 };
 
+int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev, uint64_t src_addr,
+                          uint64_t dst_addr, uint64_t mem_size);
+
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
index b611ade53be2..0d2bdd04bd76 100644
--- a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
@@ -29,5 +29,49 @@
 #include "lsdma/lsdma_6_0_0_offset.h"
 #include "lsdma/lsdma_6_0_0_sh_mask.h"
 
+static int lsdma_v6_0_copy_mem(struct amdgpu_device *adev,
+			       uint64_t src_addr,
+			       uint64_t dst_addr,
+			       uint64_t size)
+{
+	uint32_t usec_timeout = 5000;  /* wait for 5ms */
+	uint32_t tmp, expect_val;
+	int i;
+
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_LO, lower_32_bits(src_addr));
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_HI, upper_32_bits(src_addr));
+
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_LO, lower_32_bits(dst_addr));
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_HI, upper_32_bits(dst_addr));
+
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_CONTROL, 0x0);
+
+	tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, BYTE_COUNT, size);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_LOCATION, 0);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_LOCATION, 0);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_ADDR_INC, 0);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_ADDR_INC, 0);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, OVERLAP_DISABLE, 0);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, CONSTANT_FILL, 0);
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND, tmp);
+
+	expect_val = LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK;
+	for (i = 0; i < usec_timeout; i++) {
+		tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_STATUS);
+		if ((tmp & expect_val) == expect_val)
+			break;
+		udelay(1);
+	}
+
+	if (i >= usec_timeout) {
+		dev_err(adev->dev, "LSDMA PIO failed to copy memory!\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
 const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
+	.copy_mem = lsdma_v6_0_copy_mem
 };
-- 
2.35.1


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

* [PATCH 4/8] drm/amdgpu: support fill mem for LSDMA
  2022-05-05 20:03 [PATCH 0/8] LSDMA support Alex Deucher
  2022-05-05 20:04 ` [PATCH 2/8] drm/amdgpu: add lsdma block Alex Deucher
  2022-05-05 20:04 ` [PATCH 3/8] drm/amdgpu: support mem copy for LSDMA Alex Deucher
@ 2022-05-05 20:04 ` Alex Deucher
  2022-05-06  5:06   ` Luben Tuikov
  2022-05-05 20:04 ` [PATCH 5/8] drm/amdgpu: add LSDMA block for LSDMA v6.0.0 Alex Deucher
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Alex Deucher @ 2022-05-05 20:04 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Likun Gao, Christian König

From: Likun Gao <Likun.Gao@amd.com>

Support constant data filling in PIO mode for LSDMA.

Signed-off-by: Likun Gao <Likun.Gao@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c | 40 +++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h |  6 +++
 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c   | 62 +++++++++++++++++------
 3 files changed, 92 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
index 3f1c674afe41..223c47d1cc1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
@@ -26,6 +26,23 @@
 
 #define AMDGPU_LSDMA_MAX_SIZE	0x2000000ULL
 
+int amdgpu_lsdma_wait_for(struct amdgpu_device *adev,
+			  uint32_t reg_index, uint32_t reg_val,
+			  uint32_t mask)
+{
+	uint32_t val;
+	int i;
+
+	for (i = 0; i < adev->usec_timeout; i++) {
+		val = RREG32(reg_index);
+		if ((val & mask) == reg_val)
+			return 0;
+		udelay(1);
+	}
+
+	return -ETIME;
+}
+
 int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev,
 			  uint64_t src_addr,
 			  uint64_t dst_addr,
@@ -49,3 +66,26 @@ int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev,
 
 	return 0;
 }
+
+int amdgpu_lsdma_fill_mem(struct amdgpu_device *adev,
+			  uint64_t dst_addr,
+			  uint32_t data,
+			  uint64_t mem_size)
+{
+	int ret;
+
+	if (mem_size == 0)
+		return -EINVAL;
+
+	while(mem_size > 0) {
+		uint64_t current_fill_size = min(mem_size, AMDGPU_LSDMA_MAX_SIZE);
+
+		ret = adev->lsdma.funcs->fill_mem(adev, dst_addr, data, current_fill_size);
+		if (ret)
+			return ret;
+		dst_addr += current_fill_size;
+		mem_size -= current_fill_size;
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
index be397666e4c1..9a29f18407b8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
@@ -32,9 +32,15 @@ struct amdgpu_lsdma_funcs
 {
 	int (*copy_mem)(struct amdgpu_device *adev, uint64_t src_addr,
                     uint64_t dst_addr, uint64_t size);
+	int (*fill_mem)(struct amdgpu_device *adev, uint64_t dst_addr,
+                    uint32_t data, uint64_t size);
 };
 
 int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev, uint64_t src_addr,
                           uint64_t dst_addr, uint64_t mem_size);
+int amdgpu_lsdma_fill_mem(struct amdgpu_device *adev, uint64_t dst_addr,
+                          uint32_t data, uint64_t mem_size);
+int amdgpu_lsdma_wait_for(struct amdgpu_device *adev, uint32_t reg_index,
+                          uint32_t reg_val, uint32_t mask);
 
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
index 0d2bdd04bd76..b4adb94a080b 100644
--- a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
@@ -29,14 +29,20 @@
 #include "lsdma/lsdma_6_0_0_offset.h"
 #include "lsdma/lsdma_6_0_0_sh_mask.h"
 
+static int lsdma_v6_0_wait_pio_status(struct amdgpu_device *adev)
+{
+	return amdgpu_lsdma_wait_for(adev, SOC15_REG_OFFSET(LSDMA, 0, regLSDMA_PIO_STATUS),
+			LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK,
+			LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK);
+}
+
 static int lsdma_v6_0_copy_mem(struct amdgpu_device *adev,
 			       uint64_t src_addr,
 			       uint64_t dst_addr,
 			       uint64_t size)
 {
-	uint32_t usec_timeout = 5000;  /* wait for 5ms */
-	uint32_t tmp, expect_val;
-	int i;
+	int ret;
+	uint32_t tmp;
 
 	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_LO, lower_32_bits(src_addr));
 	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_HI, upper_32_bits(src_addr));
@@ -56,22 +62,46 @@ static int lsdma_v6_0_copy_mem(struct amdgpu_device *adev,
 	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, CONSTANT_FILL, 0);
 	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND, tmp);
 
-	expect_val = LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK;
-	for (i = 0; i < usec_timeout; i++) {
-		tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_STATUS);
-		if ((tmp & expect_val) == expect_val)
-			break;
-		udelay(1);
-	}
-
-	if (i >= usec_timeout) {
+	ret = lsdma_v6_0_wait_pio_status(adev);
+	if (ret)
 		dev_err(adev->dev, "LSDMA PIO failed to copy memory!\n");
-		return -ETIMEDOUT;
-	}
 
-	return 0;
+	return ret;
+}
+
+static int lsdma_v6_0_fill_mem(struct amdgpu_device *adev,
+			       uint64_t dst_addr,
+			       uint32_t data,
+			       uint64_t size)
+{
+	int ret;
+	uint32_t tmp;
+
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_CONSTFILL_DATA, data);
+
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_LO, lower_32_bits(dst_addr));
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_HI, upper_32_bits(dst_addr));
+
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_CONTROL, 0x0);
+
+	tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, BYTE_COUNT, size);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_LOCATION, 0);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_LOCATION, 0);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_ADDR_INC, 0);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_ADDR_INC, 0);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, OVERLAP_DISABLE, 0);
+	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, CONSTANT_FILL, 1);
+	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND, tmp);
+
+	ret = lsdma_v6_0_wait_pio_status(adev);
+	if (ret)
+		dev_err(adev->dev, "LSDMA PIO failed to fill memory!\n");
+
+	return ret;
 }
 
 const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
-	.copy_mem = lsdma_v6_0_copy_mem
+	.copy_mem = lsdma_v6_0_copy_mem,
+	.fill_mem = lsdma_v6_0_fill_mem
 };
-- 
2.35.1


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

* [PATCH 5/8] drm/amdgpu: add LSDMA block for LSDMA v6.0.0
  2022-05-05 20:03 [PATCH 0/8] LSDMA support Alex Deucher
                   ` (2 preceding siblings ...)
  2022-05-05 20:04 ` [PATCH 4/8] drm/amdgpu: support fill mem " Alex Deucher
@ 2022-05-05 20:04 ` Alex Deucher
  2022-05-05 20:04 ` [PATCH 6/8] drm/amdgpu: add LSDMA block for LSDMA v6.0.2 Alex Deucher
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2022-05-05 20:04 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Likun Gao, Hawking Zhang

From: Likun Gao <Likun.Gao@amd.com>

Add LSDMA ip block for LSDMA v6.0.0.

Signed-off-by: Likun Gao <Likun.Gao@amd.com>
Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 3f9f2d6cc6c2..a3494131a946 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -2307,6 +2307,14 @@ int amdgpu_discovery_set_ip_blocks(struct amdgpu_device *adev)
 		break;
 	}
 
+	switch (adev->ip_versions[LSDMA_HWIP][0]) {
+	case IP_VERSION(6, 0, 0):
+		adev->lsdma.funcs = &lsdma_v6_0_funcs;
+		break;
+	default:
+		break;
+	}
+
 	r = amdgpu_discovery_set_common_ip_blocks(adev);
 	if (r)
 		return r;
-- 
2.35.1


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

* [PATCH 6/8] drm/amdgpu: add LSDMA block for LSDMA v6.0.2
  2022-05-05 20:03 [PATCH 0/8] LSDMA support Alex Deucher
                   ` (3 preceding siblings ...)
  2022-05-05 20:04 ` [PATCH 5/8] drm/amdgpu: add LSDMA block for LSDMA v6.0.0 Alex Deucher
@ 2022-05-05 20:04 ` Alex Deucher
  2022-05-05 20:04 ` [PATCH 7/8] drm/amdgpu: support memory power gating for lsdma Alex Deucher
  2022-05-05 20:04 ` [PATCH 8/8] drm/amdgpu: support memory power gating for lsdma 6.0.2 Alex Deucher
  6 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2022-05-05 20:04 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Likun Gao, Hawking Zhang

From: Likun Gao <Likun.Gao@amd.com>

Add LSDMA ip block for LSDMA v6.0.2.

Signed-off-by: Likun Gao <Likun.Gao@amd.com>
Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index a3494131a946..d7b0fd1cad62 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -2309,6 +2309,7 @@ int amdgpu_discovery_set_ip_blocks(struct amdgpu_device *adev)
 
 	switch (adev->ip_versions[LSDMA_HWIP][0]) {
 	case IP_VERSION(6, 0, 0):
+	case IP_VERSION(6, 0, 2):
 		adev->lsdma.funcs = &lsdma_v6_0_funcs;
 		break;
 	default:
-- 
2.35.1


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

* [PATCH 7/8] drm/amdgpu: support memory power gating for lsdma
  2022-05-05 20:03 [PATCH 0/8] LSDMA support Alex Deucher
                   ` (4 preceding siblings ...)
  2022-05-05 20:04 ` [PATCH 6/8] drm/amdgpu: add LSDMA block for LSDMA v6.0.2 Alex Deucher
@ 2022-05-05 20:04 ` Alex Deucher
  2022-05-06  5:10   ` Luben Tuikov
  2022-05-05 20:04 ` [PATCH 8/8] drm/amdgpu: support memory power gating for lsdma 6.0.2 Alex Deucher
  6 siblings, 1 reply; 16+ messages in thread
From: Alex Deucher @ 2022-05-05 20:04 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Likun Gao, Hawking Zhang

From: Likun Gao <Likun.Gao@amd.com>

Support memory power gating control for LSDMA.

Signed-off-by: Likun Gao <Likun.Gao@amd.com>
Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h |  1 +
 drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c   | 16 +++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/soc21.c        | 12 +++++++++++-
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
index 9a29f18407b8..7ec7598e7dec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
@@ -34,6 +34,7 @@ struct amdgpu_lsdma_funcs
                     uint64_t dst_addr, uint64_t size);
 	int (*fill_mem)(struct amdgpu_device *adev, uint64_t dst_addr,
                     uint32_t data, uint64_t size);
+	void (*update_memory_power_gating)(struct amdgpu_device *adev, bool enable);
 };
 
 int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev, uint64_t src_addr,
diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
index b4adb94a080b..1a285b531881 100644
--- a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
@@ -101,7 +101,21 @@ static int lsdma_v6_0_fill_mem(struct amdgpu_device *adev,
 	return ret;
 }
 
+static void lsdma_v6_0_update_memory_power_gating(struct amdgpu_device *adev,
+						 bool enable)
+{
+	uint32_t tmp;
+
+	tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL);
+	tmp = REG_SET_FIELD(tmp, LSDMA_MEM_POWER_CTRL, MEM_POWER_CTRL_EN, 0);
+	WREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL, tmp);
+
+	tmp = REG_SET_FIELD(tmp, LSDMA_MEM_POWER_CTRL, MEM_POWER_CTRL_EN, enable);
+	WREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL, tmp);
+}
+
 const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
 	.copy_mem = lsdma_v6_0_copy_mem,
-	.fill_mem = lsdma_v6_0_fill_mem
+	.fill_mem = lsdma_v6_0_fill_mem,
+	.update_memory_power_gating = lsdma_v6_0_update_memory_power_gating
 };
diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
index 741ed3ba84d6..3303e02f85d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc21.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
@@ -676,7 +676,17 @@ static int soc21_common_set_clockgating_state(void *handle,
 static int soc21_common_set_powergating_state(void *handle,
 					   enum amd_powergating_state state)
 {
-	/* TODO */
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+	switch (adev->ip_versions[LSDMA_HWIP][0]) {
+	case IP_VERSION(6, 0, 0):
+		adev->lsdma.funcs->update_memory_power_gating(adev,
+				state == AMD_PG_STATE_GATE);
+		break;
+	default:
+		break;
+	}
+
 	return 0;
 }
 
-- 
2.35.1


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

* [PATCH 8/8] drm/amdgpu: support memory power gating for lsdma 6.0.2
  2022-05-05 20:03 [PATCH 0/8] LSDMA support Alex Deucher
                   ` (5 preceding siblings ...)
  2022-05-05 20:04 ` [PATCH 7/8] drm/amdgpu: support memory power gating for lsdma Alex Deucher
@ 2022-05-05 20:04 ` Alex Deucher
  6 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2022-05-05 20:04 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Likun Gao, Hawking Zhang

From: Likun Gao <Likun.Gao@amd.com>

Support memory power gating control for lsdma 6.0.2.

Signed-off-by: Likun Gao <Likun.Gao@amd.com>
Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/soc21.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
index 3303e02f85d1..c6a8520053bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc21.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
@@ -680,6 +680,7 @@ static int soc21_common_set_powergating_state(void *handle,
 
 	switch (adev->ip_versions[LSDMA_HWIP][0]) {
 	case IP_VERSION(6, 0, 0):
+	case IP_VERSION(6, 0, 2):
 		adev->lsdma.funcs->update_memory_power_gating(adev,
 				state == AMD_PG_STATE_GATE);
 		break;
-- 
2.35.1


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

* Re: [PATCH 3/8] drm/amdgpu: support mem copy for LSDMA
  2022-05-05 20:04 ` [PATCH 3/8] drm/amdgpu: support mem copy for LSDMA Alex Deucher
@ 2022-05-06  5:02   ` Luben Tuikov
  2022-05-06 14:22     ` Alex Deucher
  0 siblings, 1 reply; 16+ messages in thread
From: Luben Tuikov @ 2022-05-06  5:02 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx; +Cc: Likun Gao, Christian König



On 2022-05-05 16:04, Alex Deucher wrote:
> From: Likun Gao <Likun.Gao@amd.com>
> 
> Support memory to memory linear copy in PIO mode for LSDMA.
> 
> Signed-off-by: Likun Gao <Likun.Gao@amd.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c | 26 ++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h |  5 +++
>  drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c   | 44 +++++++++++++++++++++++
>  3 files changed, 75 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
> index af00a66f8282..3f1c674afe41 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
> @@ -23,3 +23,29 @@
>  
>  #include "amdgpu.h"
>  #include "amdgpu_lsdma.h"
> +
> +#define AMDGPU_LSDMA_MAX_SIZE	0x2000000ULL
> +
> +int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev,
> +			  uint64_t src_addr,
> +			  uint64_t dst_addr,
> +			  uint64_t mem_size)
> +{
> +	int ret;
> +
> +	if (mem_size == 0)
> +		return -EINVAL;
> +
> +	while(mem_size > 0) {

Kernel style requires a space after the "while" and before the "(".

> +		uint64_t current_copy_size = min(mem_size, AMDGPU_LSDMA_MAX_SIZE);
> +
> +		ret = adev->lsdma.funcs->copy_mem(adev, src_addr, dst_addr, current_copy_size);
> +		if (ret)
> +			return ret;
> +		src_addr += current_copy_size;
> +		dst_addr += current_copy_size;
> +		mem_size -= current_copy_size;
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> index 4df4da423181..be397666e4c1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> @@ -30,6 +30,11 @@ struct amdgpu_lsdma {
>  
>  struct amdgpu_lsdma_funcs
>  {
> +	int (*copy_mem)(struct amdgpu_device *adev, uint64_t src_addr,
> +                    uint64_t dst_addr, uint64_t size);
>  };
>  
> +int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev, uint64_t src_addr,
> +                          uint64_t dst_addr, uint64_t mem_size);
> +
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> index b611ade53be2..0d2bdd04bd76 100644
> --- a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> @@ -29,5 +29,49 @@
>  #include "lsdma/lsdma_6_0_0_offset.h"
>  #include "lsdma/lsdma_6_0_0_sh_mask.h"
>  
> +static int lsdma_v6_0_copy_mem(struct amdgpu_device *adev,
> +			       uint64_t src_addr,
> +			       uint64_t dst_addr,
> +			       uint64_t size)
> +{
> +	uint32_t usec_timeout = 5000;  /* wait for 5ms */
> +	uint32_t tmp, expect_val;
> +	int i;
> +
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_LO, lower_32_bits(src_addr));
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_HI, upper_32_bits(src_addr));
> +
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_LO, lower_32_bits(dst_addr));
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_HI, upper_32_bits(dst_addr));
> +
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_CONTROL, 0x0);
> +
> +	tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, BYTE_COUNT, size);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_LOCATION, 0);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_LOCATION, 0);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_ADDR_INC, 0);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_ADDR_INC, 0);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, OVERLAP_DISABLE, 0);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, CONSTANT_FILL, 0);
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND, tmp);
> +
> +	expect_val = LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK;
> +	for (i = 0; i < usec_timeout; i++) {
> +		tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_STATUS);
> +		if ((tmp & expect_val) == expect_val)
> +			break;
> +		udelay(1);
> +	}

Does the hardware specify a minimum delay after the write to the LSDMA_PIO_COMMAND,
and before we check the LSDMA_PIO_STATUS?

At the moment the code above checks the status immediately after writing to
LSDMA_PIO_COMMAND, and the copy wouldn't be completed.

If the poll timeout is 1 us, as the loop shows us, maybe the command completion
is larger than that (the time after writing to LSDMA_PIO_COMMAND and before
checking LSDMA_PIO_STATUS)?

> +
> +	if (i >= usec_timeout) {
> +		dev_err(adev->dev, "LSDMA PIO failed to copy memory!\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
>  const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
> +	.copy_mem = lsdma_v6_0_copy_mem
>  };

Regards,
-- 
Luben

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

* Re: [PATCH 4/8] drm/amdgpu: support fill mem for LSDMA
  2022-05-05 20:04 ` [PATCH 4/8] drm/amdgpu: support fill mem " Alex Deucher
@ 2022-05-06  5:06   ` Luben Tuikov
  0 siblings, 0 replies; 16+ messages in thread
From: Luben Tuikov @ 2022-05-06  5:06 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx; +Cc: Likun Gao, Christian König



On 2022-05-05 16:04, Alex Deucher wrote:
> From: Likun Gao <Likun.Gao@amd.com>
> 
> Support constant data filling in PIO mode for LSDMA.
> 
> Signed-off-by: Likun Gao <Likun.Gao@amd.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c | 40 +++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h |  6 +++
>  drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c   | 62 +++++++++++++++++------
>  3 files changed, 92 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
> index 3f1c674afe41..223c47d1cc1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
> @@ -26,6 +26,23 @@
>  
>  #define AMDGPU_LSDMA_MAX_SIZE	0x2000000ULL
>  
> +int amdgpu_lsdma_wait_for(struct amdgpu_device *adev,
> +			  uint32_t reg_index, uint32_t reg_val,
> +			  uint32_t mask)
> +{
> +	uint32_t val;
> +	int i;
> +
> +	for (i = 0; i < adev->usec_timeout; i++) {
> +		val = RREG32(reg_index);
> +		if ((val & mask) == reg_val)
> +			return 0;
> +		udelay(1);
> +	}
> +
> +	return -ETIME;
> +}
> +
>  int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev,
>  			  uint64_t src_addr,
>  			  uint64_t dst_addr,
> @@ -49,3 +66,26 @@ int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev,
>  
>  	return 0;
>  }
> +
> +int amdgpu_lsdma_fill_mem(struct amdgpu_device *adev,
> +			  uint64_t dst_addr,
> +			  uint32_t data,
> +			  uint64_t mem_size)
> +{
> +	int ret;
> +
> +	if (mem_size == 0)
> +		return -EINVAL;
> +
> +	while(mem_size > 0) {

checkpatch.pl complains here for style.

> +		uint64_t current_fill_size = min(mem_size, AMDGPU_LSDMA_MAX_SIZE);
> +
> +		ret = adev->lsdma.funcs->fill_mem(adev, dst_addr, data, current_fill_size);
> +		if (ret)
> +			return ret;
> +		dst_addr += current_fill_size;
> +		mem_size -= current_fill_size;
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> index be397666e4c1..9a29f18407b8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> @@ -32,9 +32,15 @@ struct amdgpu_lsdma_funcs
>  {
>  	int (*copy_mem)(struct amdgpu_device *adev, uint64_t src_addr,
>                      uint64_t dst_addr, uint64_t size);
> +	int (*fill_mem)(struct amdgpu_device *adev, uint64_t dst_addr,
> +                    uint32_t data, uint64_t size);
>  };
>  
>  int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev, uint64_t src_addr,
>                            uint64_t dst_addr, uint64_t mem_size);
> +int amdgpu_lsdma_fill_mem(struct amdgpu_device *adev, uint64_t dst_addr,
> +                          uint32_t data, uint64_t mem_size);
> +int amdgpu_lsdma_wait_for(struct amdgpu_device *adev, uint32_t reg_index,
> +                          uint32_t reg_val, uint32_t mask);
>  
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> index 0d2bdd04bd76..b4adb94a080b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> @@ -29,14 +29,20 @@
>  #include "lsdma/lsdma_6_0_0_offset.h"
>  #include "lsdma/lsdma_6_0_0_sh_mask.h"
>  
> +static int lsdma_v6_0_wait_pio_status(struct amdgpu_device *adev)
> +{
> +	return amdgpu_lsdma_wait_for(adev, SOC15_REG_OFFSET(LSDMA, 0, regLSDMA_PIO_STATUS),
> +			LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK,
> +			LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK);
> +}
> +
>  static int lsdma_v6_0_copy_mem(struct amdgpu_device *adev,
>  			       uint64_t src_addr,
>  			       uint64_t dst_addr,
>  			       uint64_t size)
>  {
> -	uint32_t usec_timeout = 5000;  /* wait for 5ms */
> -	uint32_t tmp, expect_val;
> -	int i;
> +	int ret;
> +	uint32_t tmp;
>  
>  	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_LO, lower_32_bits(src_addr));
>  	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_HI, upper_32_bits(src_addr));
> @@ -56,22 +62,46 @@ static int lsdma_v6_0_copy_mem(struct amdgpu_device *adev,
>  	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, CONSTANT_FILL, 0);
>  	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND, tmp);
>  
> -	expect_val = LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK;
> -	for (i = 0; i < usec_timeout; i++) {
> -		tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_STATUS);
> -		if ((tmp & expect_val) == expect_val)
> -			break;
> -		udelay(1);
> -	}
> -
> -	if (i >= usec_timeout) {
> +	ret = lsdma_v6_0_wait_pio_status(adev);

Similarly here. Shouldn't we wait a minimum command completion time
before starting to (immediatly) poll? (perhaps not, but I've not seen the HW
spec, if it is specified a minimum command wating time before polling
for compeltion (whose poll time interval would be different (smaller)).

> +	if (ret)
>  		dev_err(adev->dev, "LSDMA PIO failed to copy memory!\n");
> -		return -ETIMEDOUT;
> -	}
>  
> -	return 0;
> +	return ret;
> +}
> +
> +static int lsdma_v6_0_fill_mem(struct amdgpu_device *adev,
> +			       uint64_t dst_addr,
> +			       uint32_t data,
> +			       uint64_t size)
> +{
> +	int ret;
> +	uint32_t tmp;
> +
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_CONSTFILL_DATA, data);
> +
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_LO, lower_32_bits(dst_addr));
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_HI, upper_32_bits(dst_addr));
> +
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_CONTROL, 0x0);
> +
> +	tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, BYTE_COUNT, size);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_LOCATION, 0);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_LOCATION, 0);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_ADDR_INC, 0);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_ADDR_INC, 0);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, OVERLAP_DISABLE, 0);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, CONSTANT_FILL, 1);
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND, tmp);
> +
> +	ret = lsdma_v6_0_wait_pio_status(adev);
> +	if (ret)
> +		dev_err(adev->dev, "LSDMA PIO failed to fill memory!\n");
> +
> +	return ret;
>  }
>  
>  const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
> -	.copy_mem = lsdma_v6_0_copy_mem
> +	.copy_mem = lsdma_v6_0_copy_mem,
> +	.fill_mem = lsdma_v6_0_fill_mem
>  };

Regards,
-- 
Luben

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

* Re: [PATCH 7/8] drm/amdgpu: support memory power gating for lsdma
  2022-05-05 20:04 ` [PATCH 7/8] drm/amdgpu: support memory power gating for lsdma Alex Deucher
@ 2022-05-06  5:10   ` Luben Tuikov
  2022-05-06 15:26     ` Alex Deucher
  0 siblings, 1 reply; 16+ messages in thread
From: Luben Tuikov @ 2022-05-06  5:10 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx; +Cc: Likun Gao, Hawking Zhang



On 2022-05-05 16:04, Alex Deucher wrote:
> From: Likun Gao <Likun.Gao@amd.com>
> 
> Support memory power gating control for LSDMA.
> 
> Signed-off-by: Likun Gao <Likun.Gao@amd.com>
> Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h |  1 +
>  drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c   | 16 +++++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/soc21.c        | 12 +++++++++++-
>  3 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> index 9a29f18407b8..7ec7598e7dec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> @@ -34,6 +34,7 @@ struct amdgpu_lsdma_funcs
>                      uint64_t dst_addr, uint64_t size);
>  	int (*fill_mem)(struct amdgpu_device *adev, uint64_t dst_addr,
>                      uint32_t data, uint64_t size);
> +	void (*update_memory_power_gating)(struct amdgpu_device *adev, bool enable);
>  };
>  
>  int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev, uint64_t src_addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> index b4adb94a080b..1a285b531881 100644
> --- a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> @@ -101,7 +101,21 @@ static int lsdma_v6_0_fill_mem(struct amdgpu_device *adev,
>  	return ret;
>  }
>  
> +static void lsdma_v6_0_update_memory_power_gating(struct amdgpu_device *adev,
> +						 bool enable)
> +{
> +	uint32_t tmp;
> +
> +	tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL);
> +	tmp = REG_SET_FIELD(tmp, LSDMA_MEM_POWER_CTRL, MEM_POWER_CTRL_EN, 0);
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL, tmp);
> +
> +	tmp = REG_SET_FIELD(tmp, LSDMA_MEM_POWER_CTRL, MEM_POWER_CTRL_EN, enable);
> +	WREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL, tmp);
> +}
> +

Do we need to disable it first before updating?

What if we're updating it to the same value it currently is (on the first read)?
Shouldn't we just skip the subsequent writes?

>  const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
>  	.copy_mem = lsdma_v6_0_copy_mem,
> -	.fill_mem = lsdma_v6_0_fill_mem
> +	.fill_mem = lsdma_v6_0_fill_mem,
> +	.update_memory_power_gating = lsdma_v6_0_update_memory_power_gating
>  };
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
> index 741ed3ba84d6..3303e02f85d1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> @@ -676,7 +676,17 @@ static int soc21_common_set_clockgating_state(void *handle,
>  static int soc21_common_set_powergating_state(void *handle,
>  					   enum amd_powergating_state state)
>  {
> -	/* TODO */
> +	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> +	switch (adev->ip_versions[LSDMA_HWIP][0]) {
> +	case IP_VERSION(6, 0, 0):
> +		adev->lsdma.funcs->update_memory_power_gating(adev,
> +				state == AMD_PG_STATE_GATE);
> +		break;
> +	default:
> +		break;
> +	}
> +
>  	return 0;
>  }
>  

Regards,
-- 
Luben

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

* Re: [PATCH 3/8] drm/amdgpu: support mem copy for LSDMA
  2022-05-06  5:02   ` Luben Tuikov
@ 2022-05-06 14:22     ` Alex Deucher
  2022-05-07  0:03       ` Luben Tuikov
  0 siblings, 1 reply; 16+ messages in thread
From: Alex Deucher @ 2022-05-06 14:22 UTC (permalink / raw)
  To: Luben Tuikov; +Cc: Alex Deucher, Likun Gao, Christian König, amd-gfx list

On Fri, May 6, 2022 at 1:02 AM Luben Tuikov <luben.tuikov@amd.com> wrote:
>
>
>
> On 2022-05-05 16:04, Alex Deucher wrote:
> > From: Likun Gao <Likun.Gao@amd.com>
> >
> > Support memory to memory linear copy in PIO mode for LSDMA.
> >
> > Signed-off-by: Likun Gao <Likun.Gao@amd.com>
> > Reviewed-by: Christian König <christian.koenig@amd.com>
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c | 26 ++++++++++++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h |  5 +++
> >  drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c   | 44 +++++++++++++++++++++++
> >  3 files changed, 75 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
> > index af00a66f8282..3f1c674afe41 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
> > @@ -23,3 +23,29 @@
> >
> >  #include "amdgpu.h"
> >  #include "amdgpu_lsdma.h"
> > +
> > +#define AMDGPU_LSDMA_MAX_SIZE        0x2000000ULL
> > +
> > +int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev,
> > +                       uint64_t src_addr,
> > +                       uint64_t dst_addr,
> > +                       uint64_t mem_size)
> > +{
> > +     int ret;
> > +
> > +     if (mem_size == 0)
> > +             return -EINVAL;
> > +
> > +     while(mem_size > 0) {
>
> Kernel style requires a space after the "while" and before the "(".
>
> > +             uint64_t current_copy_size = min(mem_size, AMDGPU_LSDMA_MAX_SIZE);
> > +
> > +             ret = adev->lsdma.funcs->copy_mem(adev, src_addr, dst_addr, current_copy_size);
> > +             if (ret)
> > +                     return ret;
> > +             src_addr += current_copy_size;
> > +             dst_addr += current_copy_size;
> > +             mem_size -= current_copy_size;
> > +     }
> > +
> > +     return 0;
> > +}
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> > index 4df4da423181..be397666e4c1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> > @@ -30,6 +30,11 @@ struct amdgpu_lsdma {
> >
> >  struct amdgpu_lsdma_funcs
> >  {
> > +     int (*copy_mem)(struct amdgpu_device *adev, uint64_t src_addr,
> > +                    uint64_t dst_addr, uint64_t size);
> >  };
> >
> > +int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev, uint64_t src_addr,
> > +                          uint64_t dst_addr, uint64_t mem_size);
> > +
> >  #endif
> > diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> > index b611ade53be2..0d2bdd04bd76 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> > @@ -29,5 +29,49 @@
> >  #include "lsdma/lsdma_6_0_0_offset.h"
> >  #include "lsdma/lsdma_6_0_0_sh_mask.h"
> >
> > +static int lsdma_v6_0_copy_mem(struct amdgpu_device *adev,
> > +                            uint64_t src_addr,
> > +                            uint64_t dst_addr,
> > +                            uint64_t size)
> > +{
> > +     uint32_t usec_timeout = 5000;  /* wait for 5ms */
> > +     uint32_t tmp, expect_val;
> > +     int i;
> > +
> > +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_LO, lower_32_bits(src_addr));
> > +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_HI, upper_32_bits(src_addr));
> > +
> > +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_LO, lower_32_bits(dst_addr));
> > +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_HI, upper_32_bits(dst_addr));
> > +
> > +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_CONTROL, 0x0);
> > +
> > +     tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND);
> > +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, BYTE_COUNT, size);
> > +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_LOCATION, 0);
> > +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_LOCATION, 0);
> > +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_ADDR_INC, 0);
> > +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_ADDR_INC, 0);
> > +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, OVERLAP_DISABLE, 0);
> > +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, CONSTANT_FILL, 0);
> > +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND, tmp);
> > +
> > +     expect_val = LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK;
> > +     for (i = 0; i < usec_timeout; i++) {
> > +             tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_STATUS);
> > +             if ((tmp & expect_val) == expect_val)
> > +                     break;
> > +             udelay(1);
> > +     }
>
> Does the hardware specify a minimum delay after the write to the LSDMA_PIO_COMMAND,
> and before we check the LSDMA_PIO_STATUS?

I'm not sure, but it should be pretty minimal.

>
> At the moment the code above checks the status immediately after writing to
> LSDMA_PIO_COMMAND, and the copy wouldn't be completed.
>
> If the poll timeout is 1 us, as the loop shows us, maybe the command completion
> is larger than that (the time after writing to LSDMA_PIO_COMMAND and before
> checking LSDMA_PIO_STATUS)?

The time it takes for the copy will be dependent on the amount of data
there is to copy.  While the command is probably not complete on the
first read of the status register, it may be required to make sure the
previous write goes through.

Alex

>
> > +
> > +     if (i >= usec_timeout) {
> > +             dev_err(adev->dev, "LSDMA PIO failed to copy memory!\n");
> > +             return -ETIMEDOUT;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
> > +     .copy_mem = lsdma_v6_0_copy_mem
> >  };
>
> Regards,
> --
> Luben

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

* Re: [PATCH 7/8] drm/amdgpu: support memory power gating for lsdma
  2022-05-06  5:10   ` Luben Tuikov
@ 2022-05-06 15:26     ` Alex Deucher
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2022-05-06 15:26 UTC (permalink / raw)
  To: Luben Tuikov; +Cc: Alex Deucher, Likun Gao, amd-gfx list, Hawking Zhang

On Fri, May 6, 2022 at 1:10 AM Luben Tuikov <luben.tuikov@amd.com> wrote:
>
>
>
> On 2022-05-05 16:04, Alex Deucher wrote:
> > From: Likun Gao <Likun.Gao@amd.com>
> >
> > Support memory power gating control for LSDMA.
> >
> > Signed-off-by: Likun Gao <Likun.Gao@amd.com>
> > Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h |  1 +
> >  drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c   | 16 +++++++++++++++-
> >  drivers/gpu/drm/amd/amdgpu/soc21.c        | 12 +++++++++++-
> >  3 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> > index 9a29f18407b8..7ec7598e7dec 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> > @@ -34,6 +34,7 @@ struct amdgpu_lsdma_funcs
> >                      uint64_t dst_addr, uint64_t size);
> >       int (*fill_mem)(struct amdgpu_device *adev, uint64_t dst_addr,
> >                      uint32_t data, uint64_t size);
> > +     void (*update_memory_power_gating)(struct amdgpu_device *adev, bool enable);
> >  };
> >
> >  int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev, uint64_t src_addr,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> > index b4adb94a080b..1a285b531881 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> > @@ -101,7 +101,21 @@ static int lsdma_v6_0_fill_mem(struct amdgpu_device *adev,
> >       return ret;
> >  }
> >
> > +static void lsdma_v6_0_update_memory_power_gating(struct amdgpu_device *adev,
> > +                                              bool enable)
> > +{
> > +     uint32_t tmp;
> > +
> > +     tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL);
> > +     tmp = REG_SET_FIELD(tmp, LSDMA_MEM_POWER_CTRL, MEM_POWER_CTRL_EN, 0);
> > +     WREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL, tmp);
> > +
> > +     tmp = REG_SET_FIELD(tmp, LSDMA_MEM_POWER_CTRL, MEM_POWER_CTRL_EN, enable);
> > +     WREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL, tmp);
> > +}
> > +
>
> Do we need to disable it first before updating?
>
> What if we're updating it to the same value it currently is (on the first read)?
> Shouldn't we just skip the subsequent writes?

I'll let Likun comment, but I suspect this sequence came from the hardware team.

Alex

>
> >  const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
> >       .copy_mem = lsdma_v6_0_copy_mem,
> > -     .fill_mem = lsdma_v6_0_fill_mem
> > +     .fill_mem = lsdma_v6_0_fill_mem,
> > +     .update_memory_power_gating = lsdma_v6_0_update_memory_power_gating
> >  };
> > diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
> > index 741ed3ba84d6..3303e02f85d1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> > @@ -676,7 +676,17 @@ static int soc21_common_set_clockgating_state(void *handle,
> >  static int soc21_common_set_powergating_state(void *handle,
> >                                          enum amd_powergating_state state)
> >  {
> > -     /* TODO */
> > +     struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> > +
> > +     switch (adev->ip_versions[LSDMA_HWIP][0]) {
> > +     case IP_VERSION(6, 0, 0):
> > +             adev->lsdma.funcs->update_memory_power_gating(adev,
> > +                             state == AMD_PG_STATE_GATE);
> > +             break;
> > +     default:
> > +             break;
> > +     }
> > +
> >       return 0;
> >  }
> >
>
> Regards,
> --
> Luben

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

* Re: [PATCH 3/8] drm/amdgpu: support mem copy for LSDMA
  2022-05-06 14:22     ` Alex Deucher
@ 2022-05-07  0:03       ` Luben Tuikov
  2022-05-09  6:06         ` Christian König
  0 siblings, 1 reply; 16+ messages in thread
From: Luben Tuikov @ 2022-05-07  0:03 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Alex Deucher, Likun Gao, Christian König, amd-gfx list



On 2022-05-06 10:22, Alex Deucher wrote:
> On Fri, May 6, 2022 at 1:02 AM Luben Tuikov <luben.tuikov@amd.com> wrote:
>>
>>
>>
>> On 2022-05-05 16:04, Alex Deucher wrote:
>>> From: Likun Gao <Likun.Gao@amd.com>
>>>
>>> Support memory to memory linear copy in PIO mode for LSDMA.
>>>
>>> Signed-off-by: Likun Gao <Likun.Gao@amd.com>
>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c | 26 ++++++++++++++
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h |  5 +++
>>>  drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c   | 44 +++++++++++++++++++++++
>>>  3 files changed, 75 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
>>> index af00a66f8282..3f1c674afe41 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.c
>>> @@ -23,3 +23,29 @@
>>>
>>>  #include "amdgpu.h"
>>>  #include "amdgpu_lsdma.h"
>>> +
>>> +#define AMDGPU_LSDMA_MAX_SIZE        0x2000000ULL
>>> +
>>> +int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev,
>>> +                       uint64_t src_addr,
>>> +                       uint64_t dst_addr,
>>> +                       uint64_t mem_size)
>>> +{
>>> +     int ret;
>>> +
>>> +     if (mem_size == 0)
>>> +             return -EINVAL;
>>> +
>>> +     while(mem_size > 0) {
>>
>> Kernel style requires a space after the "while" and before the "(".
>>
>>> +             uint64_t current_copy_size = min(mem_size, AMDGPU_LSDMA_MAX_SIZE);
>>> +
>>> +             ret = adev->lsdma.funcs->copy_mem(adev, src_addr, dst_addr, current_copy_size);
>>> +             if (ret)
>>> +                     return ret;
>>> +             src_addr += current_copy_size;
>>> +             dst_addr += current_copy_size;
>>> +             mem_size -= current_copy_size;
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
>>> index 4df4da423181..be397666e4c1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
>>> @@ -30,6 +30,11 @@ struct amdgpu_lsdma {
>>>
>>>  struct amdgpu_lsdma_funcs
>>>  {
>>> +     int (*copy_mem)(struct amdgpu_device *adev, uint64_t src_addr,
>>> +                    uint64_t dst_addr, uint64_t size);
>>>  };
>>>
>>> +int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev, uint64_t src_addr,
>>> +                          uint64_t dst_addr, uint64_t mem_size);
>>> +
>>>  #endif
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
>>> index b611ade53be2..0d2bdd04bd76 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
>>> @@ -29,5 +29,49 @@
>>>  #include "lsdma/lsdma_6_0_0_offset.h"
>>>  #include "lsdma/lsdma_6_0_0_sh_mask.h"
>>>
>>> +static int lsdma_v6_0_copy_mem(struct amdgpu_device *adev,
>>> +                            uint64_t src_addr,
>>> +                            uint64_t dst_addr,
>>> +                            uint64_t size)
>>> +{
>>> +     uint32_t usec_timeout = 5000;  /* wait for 5ms */
>>> +     uint32_t tmp, expect_val;
>>> +     int i;
>>> +
>>> +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_LO, lower_32_bits(src_addr));
>>> +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_SRC_ADDR_HI, upper_32_bits(src_addr));
>>> +
>>> +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_LO, lower_32_bits(dst_addr));
>>> +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_DST_ADDR_HI, upper_32_bits(dst_addr));
>>> +
>>> +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_CONTROL, 0x0);
>>> +
>>> +     tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND);
>>> +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, BYTE_COUNT, size);
>>> +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_LOCATION, 0);
>>> +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_LOCATION, 0);
>>> +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, SRC_ADDR_INC, 0);
>>> +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, DST_ADDR_INC, 0);
>>> +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, OVERLAP_DISABLE, 0);
>>> +     tmp = REG_SET_FIELD(tmp, LSDMA_PIO_COMMAND, CONSTANT_FILL, 0);
>>> +     WREG32_SOC15(LSDMA, 0, regLSDMA_PIO_COMMAND, tmp);
>>> +
>>> +     expect_val = LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK;
>>> +     for (i = 0; i < usec_timeout; i++) {
>>> +             tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_STATUS);
>>> +             if ((tmp & expect_val) == expect_val)
>>> +                     break;
>>> +             udelay(1);
>>> +     }
>>
>> Does the hardware specify a minimum delay after the write to the LSDMA_PIO_COMMAND,
>> and before we check the LSDMA_PIO_STATUS?
> 
> I'm not sure, but it should be pretty minimal.

My point is that there should be a delay at least as large as the polling delay,
after writing to the command register and before reading the status register.
So that should be at least a 1 us.

There should be a udelay(1) after writing the command to the LSDMA_PIO_COMMAND
register, before the for () loop begins.

Regards,
Luben


> 
>>
>> At the moment the code above checks the status immediately after writing to
>> LSDMA_PIO_COMMAND, and the copy wouldn't be completed.
>>
>> If the poll timeout is 1 us, as the loop shows us, maybe the command completion
>> is larger than that (the time after writing to LSDMA_PIO_COMMAND and before
>> checking LSDMA_PIO_STATUS)?
> 
> The time it takes for the copy will be dependent on the amount of data
> there is to copy.  While the command is probably not complete on the
> first read of the status register, it may be required to make sure the
> previous write goes through.
> 
> Alex
> 
>>
>>> +
>>> +     if (i >= usec_timeout) {
>>> +             dev_err(adev->dev, "LSDMA PIO failed to copy memory!\n");
>>> +             return -ETIMEDOUT;
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>>  const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
>>> +     .copy_mem = lsdma_v6_0_copy_mem
>>>  };
>>
>> Regards,
>> --
>> Luben

Regards,
-- 
Luben

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

* Re: [PATCH 3/8] drm/amdgpu: support mem copy for LSDMA
  2022-05-07  0:03       ` Luben Tuikov
@ 2022-05-09  6:06         ` Christian König
  2022-05-09 14:59           ` Luben Tuikov
  0 siblings, 1 reply; 16+ messages in thread
From: Christian König @ 2022-05-09  6:06 UTC (permalink / raw)
  To: Luben Tuikov, Alex Deucher
  Cc: Alex Deucher, Likun Gao, Christian König, amd-gfx list

Am 07.05.22 um 02:03 schrieb Luben Tuikov:
> [SNIP]
> +     expect_val = LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK;
> +     for (i = 0; i < usec_timeout; i++) {
> +             tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_STATUS);
> +             if ((tmp & expect_val) == expect_val)
> +                     break;
> +             udelay(1);
> +     }
>>> Does the hardware specify a minimum delay after the write to the LSDMA_PIO_COMMAND,
>>> and before we check the LSDMA_PIO_STATUS?
>> I'm not sure, but it should be pretty minimal.
> My point is that there should be a delay at least as large as the polling delay,
> after writing to the command register and before reading the status register.
> So that should be at least a 1 us.
>
> There should be a udelay(1) after writing the command to the LSDMA_PIO_COMMAND
> register, before the for () loop begins.

I can't count how often I had to reject that approach. This is exactly 
what we should *not* do.

The SRBM (or whatever that's called on newer hardware) is the one who 
inserts the delay here. The driver should not explicitly do that 
additionally.

The background is that a) the read is often used to commit the write 
operations, both for the PCIe as well as internally in the GPU and b) 
the read only comes back when the SRBM has synced up between the bus 
interface and the hardware block in question.

So when the SRBM has synced between the two clock domains the operation 
has usually been completed or at least started. The minimal delay we 
enter between reads is just to avoid hammering on the register bus when 
the hardware block in question is (for example) power gated or otherwise 
not reachable.

Regards,
Christian.

>
> Regards,
> Luben
>
>
>>> At the moment the code above checks the status immediately after writing to
>>> LSDMA_PIO_COMMAND, and the copy wouldn't be completed.
>>>
>>> If the poll timeout is 1 us, as the loop shows us, maybe the command completion
>>> is larger than that (the time after writing to LSDMA_PIO_COMMAND and before
>>> checking LSDMA_PIO_STATUS)?
>> The time it takes for the copy will be dependent on the amount of data
>> there is to copy.  While the command is probably not complete on the
>> first read of the status register, it may be required to make sure the
>> previous write goes through.
>>
>> Alex
>>
>>>> +
>>>> +     if (i >= usec_timeout) {
>>>> +             dev_err(adev->dev, "LSDMA PIO failed to copy memory!\n");
>>>> +             return -ETIMEDOUT;
>>>> +     }
>>>> +
>>>> +     return 0;
>>>> +}
>>>> +
>>>>   const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
>>>> +     .copy_mem = lsdma_v6_0_copy_mem
>>>>   };
>>> Regards,
>>> --
>>> Luben
> Regards,


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

* Re: [PATCH 3/8] drm/amdgpu: support mem copy for LSDMA
  2022-05-09  6:06         ` Christian König
@ 2022-05-09 14:59           ` Luben Tuikov
  0 siblings, 0 replies; 16+ messages in thread
From: Luben Tuikov @ 2022-05-09 14:59 UTC (permalink / raw)
  To: Christian König, Alex Deucher
  Cc: Alex Deucher, Likun Gao, Christian König, amd-gfx list



On 2022-05-09 02:06, Christian König wrote:
> Am 07.05.22 um 02:03 schrieb Luben Tuikov:
>> [SNIP]
>> +     expect_val = LSDMA_PIO_STATUS__PIO_IDLE_MASK | LSDMA_PIO_STATUS__PIO_FIFO_EMPTY_MASK;
>> +     for (i = 0; i < usec_timeout; i++) {
>> +             tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_PIO_STATUS);
>> +             if ((tmp & expect_val) == expect_val)
>> +                     break;
>> +             udelay(1);
>> +     }
>>>> Does the hardware specify a minimum delay after the write to the LSDMA_PIO_COMMAND,
>>>> and before we check the LSDMA_PIO_STATUS?
>>> I'm not sure, but it should be pretty minimal.
>> My point is that there should be a delay at least as large as the polling delay,
>> after writing to the command register and before reading the status register.
>> So that should be at least a 1 us.
>>
>> There should be a udelay(1) after writing the command to the LSDMA_PIO_COMMAND
>> register, before the for () loop begins.
> 
> I can't count how often I had to reject that approach. This is exactly 
> what we should *not* do.
> 
> The SRBM (or whatever that's called on newer hardware) is the one who 
> inserts the delay here. The driver should not explicitly do that 
> additionally.
> 
> The background is that a) the read is often used to commit the write 
> operations, both for the PCIe as well as internally in the GPU and b) 
> the read only comes back when the SRBM has synced up between the bus 
> interface and the hardware block in question.
> 
> So when the SRBM has synced between the two clock domains the operation 
> has usually been completed or at least started. The minimal delay we 
> enter between reads is just to avoid hammering on the register bus when 
> the hardware block in question is (for example) power gated or otherwise 
> not reachable.

Yeah, that's what I wanted to avoid, but if you say that we need it to
make sure that the write has committed, then it's okay.

Regards,
Luben

> 
> Regards,
> Christian.
> 
>>
>> Regards,
>> Luben
>>
>>
>>>> At the moment the code above checks the status immediately after writing to
>>>> LSDMA_PIO_COMMAND, and the copy wouldn't be completed.
>>>>
>>>> If the poll timeout is 1 us, as the loop shows us, maybe the command completion
>>>> is larger than that (the time after writing to LSDMA_PIO_COMMAND and before
>>>> checking LSDMA_PIO_STATUS)?
>>> The time it takes for the copy will be dependent on the amount of data
>>> there is to copy.  While the command is probably not complete on the
>>> first read of the status register, it may be required to make sure the
>>> previous write goes through.
>>>
>>> Alex
>>>
>>>>> +
>>>>> +     if (i >= usec_timeout) {
>>>>> +             dev_err(adev->dev, "LSDMA PIO failed to copy memory!\n");
>>>>> +             return -ETIMEDOUT;
>>>>> +     }
>>>>> +
>>>>> +     return 0;
>>>>> +}
>>>>> +
>>>>>   const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
>>>>> +     .copy_mem = lsdma_v6_0_copy_mem
>>>>>   };
>>>> Regards,
>>>> --
>>>> Luben
>> Regards,
> 

Regards,
-- 
Luben

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

end of thread, other threads:[~2022-05-09 14:59 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-05 20:03 [PATCH 0/8] LSDMA support Alex Deucher
2022-05-05 20:04 ` [PATCH 2/8] drm/amdgpu: add lsdma block Alex Deucher
2022-05-05 20:04 ` [PATCH 3/8] drm/amdgpu: support mem copy for LSDMA Alex Deucher
2022-05-06  5:02   ` Luben Tuikov
2022-05-06 14:22     ` Alex Deucher
2022-05-07  0:03       ` Luben Tuikov
2022-05-09  6:06         ` Christian König
2022-05-09 14:59           ` Luben Tuikov
2022-05-05 20:04 ` [PATCH 4/8] drm/amdgpu: support fill mem " Alex Deucher
2022-05-06  5:06   ` Luben Tuikov
2022-05-05 20:04 ` [PATCH 5/8] drm/amdgpu: add LSDMA block for LSDMA v6.0.0 Alex Deucher
2022-05-05 20:04 ` [PATCH 6/8] drm/amdgpu: add LSDMA block for LSDMA v6.0.2 Alex Deucher
2022-05-05 20:04 ` [PATCH 7/8] drm/amdgpu: support memory power gating for lsdma Alex Deucher
2022-05-06  5:10   ` Luben Tuikov
2022-05-06 15:26     ` Alex Deucher
2022-05-05 20:04 ` [PATCH 8/8] drm/amdgpu: support memory power gating for lsdma 6.0.2 Alex Deucher

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