[QUESTION] sdma_v5_2 updates address with an running async dma engine

[Haohui Mai]

Hi,

I'm looking at the initialization sequences in sdma_v5_2.c. I'm
confused on whether the DMA engine should be activated when updating
the MMIO registers. Some clarifications are highly appreciated.

Here is the background:
 * sdma_v5_2_enable() toggles the HALT bit to enable / disable the
async DMA engine
 * sdma_v5_2_resume() initializes MMIO registers (e.g., queue
addresses) of the DMA engine.
 * sdma_v5_2_start() is called when the kernel initializes the device.

However, the driver has two paths when updating the MMIO registers,
where the DMA engine is activated / deactivated respectively.

When amdgpu_sriov_vf(adev) is true:

   866         if (amdgpu_sriov_vf(adev)) {
   867                 sdma_v5_2_ctx_switch_enable(adev, false);
   868                 sdma_v5_2_enable(adev, false);
   869
   870                 /* set RB registers */
   871                 r = sdma_v5_2_gfx_resume(adev);
   872                 return r;
   873         }

When amdgpu_sriov_vf(adev) is false:

   893         sdma_v5_2_enable(adev, true);
   894         /* enable sdma ring preemption */
   895         sdma_v5_2_ctx_switch_enable(adev, true);
   896
   897         /* start the gfx rings and rlc compute queues */
   898         r = sdma_v5_2_gfx_resume(adev);

Furthermore, sdma_v5_2_gfx_resume() re-enables the already active DMA
engine when amdgpu_sriov_vf(adev) is false:

   728                         /* unhalt engine */
   729                         temp =
RREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_F32_CNTL));
   730                         temp = REG_SET_FIELD(temp,
SDMA0_F32_CNTL, HALT, 0);
   731                         WREG32(sdma_v5_2_get_reg_offset(adev,
i, mmSDMA0_F32_CNTL), temp);

The behavior seems inconsistent. Looking at the code that re-enables
the engine, it seems that the driver assumes a deactivated DMA engine
during initialization regardless whether the device is in vf mode or
not.

Just wondering, is the behavior expected or is it a bug?

Thanks,
Haohui