On 5/21/2024 12:46 PM, Jesse Zhang wrote:
> Since the type of data_size is uint32_t, adev->umsch_mm.data_size - 1 >> 16
> >> 16 is 0
> regardless of the values of its operands
>
> So removing the operations upper_32_bits and lower_32_bits.
>
> Signed-off-by: Jesse Zhang <jesse.zh...@amd.com>
> Suggested-by: Tim Huang <tim.hu...@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/umsch_mm_v4_0.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/umsch_mm_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/umsch_mm_v4_0.c
> index 2c5e7b0a73f9..ce3bb12e3572 100644
> --- a/drivers/gpu/drm/amd/amdgpu/umsch_mm_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/umsch_mm_v4_0.c
> @@ -116,9 +116,8 @@ static int umsch_mm_v4_0_load_microcode(struct
> amdgpu_umsch_mm *umsch)
> upper_32_bits(adev->umsch_mm.data_start_addr));
>
> WREG32_SOC15_UMSCH(regVCN_MES_LOCAL_MASK0_LO,
> - lower_32_bits(adev->umsch_mm.data_size - 1));
> - WREG32_SOC15_UMSCH(regVCN_MES_LOCAL_MASK0_HI,
> - upper_32_bits(adev->umsch_mm.data_size - 1));
> + adev->umsch_mm.data_size - 1);
> + WREG32_SOC15_UMSCH(regVCN_MES_LOCAL_MASK0_HI, 0);
cc: Lang
The original programming and the new one doesn't look correct.
I see the below field definitions as per the header. As per this, both
LO/HI are 16-bit fields.
vcn/vcn_4_0_5_sh_mask.h:#define VCN_MES_LOCAL_MASK0_HI__MASK0_HI__SHIFT
0x0
vcn/vcn_4_0_5_sh_mask.h:#define VCN_MES_LOCAL_MASK0_HI__MASK0_HI_MASK
0x0000FFFFL
vcn/vcn_4_0_5_sh_mask.h:#define VCN_MES_LOCAL_MASK0_LO__MASK0_LO__SHIFT
0x10
vcn/vcn_4_0_5_sh_mask.h:#define VCN_MES_LOCAL_MASK0_LO__MASK0_LO_MASK
0xFFFF0000L
Thanks,
Lijo
>
> data = adev->firmware.load_type == AMDGPU_FW_LOAD_PSP ?
> 0 : adev->umsch_mm.data_fw_gpu_addr;
