Am 09.08.19 um 06:27 schrieb Tao Zhou:
> implement 64 bits operations via 32 bits interface
>
> Signed-off-by: Tao Zhou <tao.zh...@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_umc.h | 9 +++++++++
> drivers/gpu/drm/amd/amdgpu/umc_v6_1.c | 10 +++++-----
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.h
> index 9efdd66279e5..a617dcc9d257 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_umc.h
> @@ -21,6 +21,15 @@
> #ifndef __AMDGPU_UMC_H__
> #define __AMDGPU_UMC_H__
>
> +/* implement 64 bits REG operations via 32 bits interface */
> +#define RREG64_UMC(reg) (RREG32(reg) | \
> + ((uint64_t)RREG32((reg) + 1) << 32))
> +#define WREG64_UMC(reg, v) \
Maybe call this WREG_LO_HI and RREG_LO_HI to explicitly note what they
are doing. This way we can probably keep them in amdgpu.h as well.
> + do { \
> + WREG32((reg), (uint32_t)((v) & 0xffffffff)); \
> + WREG32((reg) + 1, (uint32_t)((v) >> 32)); \
I think I now understand why you wanted to have this macro.
We have explicit lower_32_bits() and upper_32_bits() functions to avoid
the masking and shifting and make the code more readable.
For example see the UVD code:
> WREG32_SOC15(UVD, 0, mmUVD_LMI_VCPU_CACHE_64BIT_BAR_LOW,
> lower_32_bits(adev->vcn.gpu_addr));
> WREG32_SOC15(UVD, 0, mmUVD_LMI_VCPU_CACHE_64BIT_BAR_HIGH,
> upper_32_bits(adev->vcn.gpu_addr));
I suggest to use those in your macro here as well, apart from that the
patch looks good to me.
Thanks for taking care of this,
Christian.
> + } while (0)
> +
> /*
> * void (*func)(struct amdgpu_device *adev, struct ras_err_data *err_data,
> * uint32_t umc_reg_offset, uint32_t channel_index)
> diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
> b/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
> index 64df37b860dd..8502e736f721 100644
> --- a/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/umc_v6_1.c
> @@ -116,7 +116,7 @@ static void umc_v6_1_query_correctable_error_count(struct
> amdgpu_device *adev,
>
> /* check for SRAM correctable error
> MCUMC_STATUS is a 64 bit register */
> - mc_umc_status = RREG64(mc_umc_status_addr + umc_reg_offset);
> + mc_umc_status = RREG64_UMC(mc_umc_status_addr + umc_reg_offset);
> if (REG_GET_FIELD(mc_umc_status, MCA_UMC_UMC0_MCUMC_STATUST0,
> ErrorCodeExt) == 6 &&
> REG_GET_FIELD(mc_umc_status, MCA_UMC_UMC0_MCUMC_STATUST0, Val) == 1
> &&
> REG_GET_FIELD(mc_umc_status, MCA_UMC_UMC0_MCUMC_STATUST0, CECC) ==
> 1)
> @@ -134,7 +134,7 @@ static void
> umc_v6_1_querry_uncorrectable_error_count(struct amdgpu_device *adev
> SOC15_REG_OFFSET(UMC, 0, mmMCA_UMC_UMC0_MCUMC_STATUST0);
>
> /* check the MCUMC_STATUS */
> - mc_umc_status = RREG64(mc_umc_status_addr + umc_reg_offset);
> + mc_umc_status = RREG64_UMC(mc_umc_status_addr + umc_reg_offset);
> if ((REG_GET_FIELD(mc_umc_status, MCA_UMC_UMC0_MCUMC_STATUST0, Val) ==
> 1) &&
> (REG_GET_FIELD(mc_umc_status, MCA_UMC_UMC0_MCUMC_STATUST0,
> Deferred) == 1 ||
> REG_GET_FIELD(mc_umc_status, MCA_UMC_UMC0_MCUMC_STATUST0, UECC) ==
> 1 ||
> @@ -173,11 +173,11 @@ static void umc_v6_1_query_error_address(struct
> amdgpu_device *adev,
> /* skip error address process if -ENOMEM */
> if (!err_data->err_addr) {
> /* clear umc status */
> - WREG64(mc_umc_status_addr + umc_reg_offset, 0x0ULL);
> + WREG64_UMC(mc_umc_status_addr + umc_reg_offset, 0x0ULL);
> return;
> }
>
> - mc_umc_status = RREG64(mc_umc_status_addr + umc_reg_offset);
> + mc_umc_status = RREG64_UMC(mc_umc_status_addr + umc_reg_offset);
>
> /* calculate error address if ue/ce error is detected */
> if (REG_GET_FIELD(mc_umc_status, MCA_UMC_UMC0_MCUMC_STATUST0, Val) == 1
> &&
> @@ -200,7 +200,7 @@ static void umc_v6_1_query_error_address(struct
> amdgpu_device *adev,
> }
>
> /* clear umc status */
> - WREG64(mc_umc_status_addr + umc_reg_offset, 0x0ULL);
> + WREG64_UMC(mc_umc_status_addr + umc_reg_offset, 0x0ULL);
> }
>
> static void umc_v6_1_query_ras_error_address(struct amdgpu_device *adev,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
