Re: [PATCH v2 1/2] drm/amdgpu/mes: Support 64-bit register reads

[Lazar, Lijo]

On 28-Sep-26 10:13 AM, Lazar, Lijo wrote:


On 24-Sep-26 12:05 PM, Victor Zhao wrote:
[why]
Some register values span two consecutive dwords and must be sampled
with one MES operation.

[how]
Add amdgpu_mes_rreg64(), set READ_REG read64Bits for MES 11 and
MES 12, and combine the two returned dwords into a 64-bit value.

v2: add MES 11 support with the same read64Bits option

Signed-off-by: Victor Zhao <victor.z...@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c       | 42 +++++++++++++++++++
  drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h       |  3 ++
  drivers/gpu/drm/amd/amdgpu/mes_v11_0.c        |  2 +
  drivers/gpu/drm/amd/amdgpu/mes_v12_0.c        |  2 +
  drivers/gpu/drm/amd/amdgpu/mes_v12_1.c        |  2 +
  drivers/gpu/drm/amd/include/mes_v11_api_def.h |  7 ++++
  6 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/ 
drm/amd/amdgpu/amdgpu_mes.c
index 5bebe74de01ad..52b05dc7b74e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -582,6 +582,7 @@ uint32_t amdgpu_mes_rreg(struct amdgpu_device 
*adev, uint32_t reg,
      op_input.op = MES_MISC_OP_READ_REG;
      op_input.read_reg.reg_offset = reg;
      op_input.read_reg.buffer_addr = read_val_gpu_addr;
+    op_input.read_reg.read64_bits = false;
      if (!adev->mes.funcs->misc_op) {
          dev_err(adev->dev, "mes rreg is not supported!\n");
@@ -602,6 +603,47 @@ uint32_t amdgpu_mes_rreg(struct amdgpu_device 
*adev, uint32_t reg,
      return val;
  }
+uint64_t amdgpu_mes_rreg64(struct amdgpu_device *adev, uint32_t reg,
+               uint32_t xcc_id)
+{
+    struct mes_misc_op_input op_input = { 0 };
+    uint64_t val = 0;
+    uint32_t addr_offset;
+    uint32_t *read_val_ptr;
+    int r;
+
+    if (amdgpu_wb_get(adev, &addr_offset)) {
+        dev_err(adev->dev, "critical bug! too many mes readers\n");
+        return 0;

For the comment about patch 2, this API should return an error code. 0 
could be misinterpreted as a valid register value.

Thanks,
Lijo
+    }
+
+    read_val_ptr = (uint32_t *)&adev->wb.wb[addr_offset];
+    op_input.xcc_id = xcc_id;
+    op_input.op = MES_MISC_OP_READ_REG;
+    op_input.read_reg.reg_offset = reg;
+    op_input.read_reg.buffer_addr = adev->wb.gpu_addr + addr_offset * 4;
+    op_input.read_reg.read64_bits = true;
+
+    if (!adev->mes.funcs->misc_op) {
+        dev_err(adev->dev, "mes rreg is not supported!\n");
+        goto error;
+    }
+
+    amdgpu_mes_lock(&adev->mes);
+    r = adev->mes.funcs->misc_op(&adev->mes, &op_input);
+    amdgpu_mes_unlock(&adev->mes);
+    if (r) {
+        dev_err(adev->dev, "failed to read reg (0x%x)\n", reg);
+    } else {
+        val = READ_ONCE(read_val_ptr[0]);
+        val |= (uint64_t)READ_ONCE(read_val_ptr[1]) << 32;
+    }
+
+error:
+    amdgpu_wb_free(adev, addr_offset);
+    return val;
+}
+
  int amdgpu_mes_wreg(struct amdgpu_device *adev, uint32_t reg,
              uint32_t val, uint32_t xcc_id)
  {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h b/drivers/gpu/ 
drm/amd/amdgpu/amdgpu_mes.h
index 33a426f58e723..d67814124f9e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
@@ -373,6 +373,7 @@ struct mes_misc_op_input {
          struct {
              uint32_t                  reg_offset;
              uint64_t                  buffer_addr;
+            bool                      read64_bits;
          } read_reg;
          struct {
@@ -545,6 +546,8 @@ int amdgpu_mes_detect_and_reset_hung_queues(struct 
amdgpu_device *adev,
  uint32_t amdgpu_mes_rreg(struct amdgpu_device *adev, uint32_t reg,
               uint32_t xcc_id);
+uint64_t amdgpu_mes_rreg64(struct amdgpu_device *adev, uint32_t reg,
+               uint32_t xcc_id);
  int amdgpu_mes_wreg(struct amdgpu_device *adev,
              uint32_t reg, uint32_t val, uint32_t xcc_id);
  int amdgpu_mes_reg_write_reg_wait(struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/ 
amd/amdgpu/mes_v11_0.c
index d8b4d3deff605..304087b851644 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -905,6 +905,8 @@ static int mes_v11_0_misc_op(struct amdgpu_mes *mes,
          misc_pkt.opcode = MESAPI_MISC__READ_REG;
          misc_pkt.read_reg.reg_offset = input->read_reg.reg_offset;
          misc_pkt.read_reg.buffer_addr = input->read_reg.buffer_addr;
+        misc_pkt.read_reg.option.bits.read64Bits =
+            input->read_reg.read64_bits;


Is this only a missing interface definition or does this need supported 
FW version check?

Thanks,
Lijo

          break;
      case MES_MISC_OP_WRITE_REG:
          misc_pkt.opcode = MESAPI_MISC__WRITE_REG;
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c b/drivers/gpu/drm/ 
amd/amdgpu/mes_v12_0.c
index 04465804c2544..0d2a60528802e 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
@@ -858,6 +858,8 @@ static int mes_v12_0_misc_op(struct amdgpu_mes *mes,
          misc_pkt.opcode = MESAPI_MISC__READ_REG;
          misc_pkt.read_reg.reg_offset = input->read_reg.reg_offset;
          misc_pkt.read_reg.buffer_addr = input->read_reg.buffer_addr;
+        misc_pkt.read_reg.option.bits.read64Bits =
+            input->read_reg.read64_bits;
          break;
      case MES_MISC_OP_WRITE_REG:
          misc_pkt.opcode = MESAPI_MISC__WRITE_REG;
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v12_1.c b/drivers/gpu/drm/ 
amd/amdgpu/mes_v12_1.c
index d17ced46d5f73..e0a488e516c09 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v12_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v12_1.c
@@ -581,6 +581,8 @@ static int mes_v12_1_misc_op(struct amdgpu_mes *mes,
      case MES_MISC_OP_READ_REG:
          misc_pkt.opcode = MESAPI_MISC__READ_REG;
          misc_pkt.read_reg.buffer_addr = input->read_reg.buffer_addr;
+        misc_pkt.read_reg.option.bits.read64Bits =
+            input->read_reg.read64_bits;
          mes_v12_1_get_rrmt(input->read_reg.reg_offset,
                     GET_INST(GC, input->xcc_id),
                     &misc_pkt.read_reg.rrmt_opt,
diff --git a/drivers/gpu/drm/amd/include/mes_v11_api_def.h b/drivers/ 
gpu/drm/amd/include/mes_v11_api_def.h
index 1fe824b41f2a5..b27ec78da3b08 100644
--- a/drivers/gpu/drm/amd/include/mes_v11_api_def.h
+++ b/drivers/gpu/drm/amd/include/mes_v11_api_def.h
@@ -635,6 +635,13 @@ struct WRITE_REG {
  struct READ_REG {
      uint32_t                  reg_offset;
      uint64_t                  buffer_addr;
+    union {
+        struct {
+            uint32_t read64Bits : 1;
+            uint32_t reserved : 31;
+        } bits;
+        uint32_t all;
+    } option;
  };
  enum WRM_OPERATION {