Am 29.03.2017 um 03:50 schrieb Zhang, Jerry (Junwei): > > On 03/29/2017 02:37 AM, Alex Xie wrote: >> drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c:187:2: warning: right shift >> count >= width of type [enabled by default] >> drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c:173:2: warning: right shift >> count >= width of type [enabled by default] >> drivers/gpu/drm/amd/amdgpu/vega10_ih.c:106:3: warning: right shift >> count >= width of type [enabled by default] Usually we try to use upper_32_bits() for this: > 164 <http://lxr.free-electrons.com/source/include/linux/kernel.h#L164> */* A basic shift-right of a 64- or 32-bit quantity. Use this to suppress/* > 165 <http://lxr.free-electrons.com/source/include/linux/kernel.h#L165> */* the "right shift count >= width of type" warning when that quantity > is/* > 166 <http://lxr.free-electrons.com/source/include/linux/kernel.h#L166> */* 32-bits./* > 167 <http://lxr.free-electrons.com/source/include/linux/kernel.h#L167> */*//* > 168 <http://lxr.free-electrons.com/source/include/linux/kernel.h#L168> #defineupper_32_bits <http://lxr.free-electrons.com/ident?i=upper_32_bits>(n <http://lxr.free-electrons.com/ident?i=n>) ((u32 <http://lxr.free-electrons.com/ident?i=u32>)(((n <http://lxr.free-electrons.com/ident?i=n>) >> 16) >> 16)) But since the we don't shift by 32 here it's probably ok to code this manually. >> >> Reported by: kbuild-all at 01.org >> >> Change-Id: I0c3e52f7fd1026d07ac1042b5e8796fbdf09afff >> Signed-off-by: Alex Xie <AlexBin.Xie at amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +- >> drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 2 +- >> drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> index 5604a53..5d4d999 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c >> @@ -172,7 +172,7 @@ int gfxhub_v1_0_gart_enable(struct amdgpu_device >> *adev) >> (u32)(adev->dummy_page.addr >> 12)); >> WREG32(SOC15_REG_OFFSET(GC, 0, >> mmVM_L2_PROTECTION_FAULT_DEFAULT_ADDR_HI32), >> - (u32)(adev->dummy_page.addr >> 44)); >> + (u32)((u64)adev->dummy_page.addr >> 44)); >> >> tmp = RREG32(SOC15_REG_OFFSET(GC, 0, >> mmVM_L2_PROTECTION_FAULT_CNTL2)); >> tmp = REG_SET_FIELD(tmp, VM_L2_PROTECTION_FAULT_CNTL2, >> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c >> b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c >> index 5903bb0..d41e765 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c >> @@ -186,7 +186,7 @@ int mmhub_v1_0_gart_enable(struct amdgpu_device >> *adev) >> (u32)(adev->dummy_page.addr >> 12)); >> WREG32(SOC15_REG_OFFSET(MMHUB, 0, >> mmVM_L2_PROTECTION_FAULT_DEFAULT_ADDR_HI32), >> - (u32)(adev->dummy_page.addr >> 44)); >> + (u32)((u64)adev->dummy_page.addr >> 44)); >> >> tmp = RREG32(SOC15_REG_OFFSET(MMHUB, 0, >> mmVM_L2_PROTECTION_FAULT_CNTL2)); >> tmp = REG_SET_FIELD(tmp, VM_L2_PROTECTION_FAULT_CNTL2, >> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c >> b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c >> index 23371e1..041ed0b 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c >> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c >> @@ -103,7 +103,7 @@ static int vega10_ih_irq_init(struct >> amdgpu_device *adev) >> /* Ring Buffer base. [39:8] of 40-bit address of the beginning >> of the ring buffer*/ >> if (adev->irq.ih.use_bus_addr) { >> WREG32(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_BASE), >> adev->irq.ih.rb_dma_addr >> 8); >> - WREG32(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_BASE_HI), >> (adev->irq.ih.rb_dma_addr >> 40) &0xff); >> + WREG32(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_BASE_HI), >> ((u64)adev->irq.ih.rb_dma_addr >> 40) &0xff); > Could you adding a space between "&" and "0xff" during this fix? With that fixed the patch is Reviewed-by: Christian König <christian.koenig at amd.com>. > >> ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, >> MC_SPACE, 1); >> } else { >> WREG32(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_BASE), >> adev->irq.ih.gpu_addr >> 8); > > Maybe miss the "else" case, like below: > WREG32(SOC15_REG_OFFSET(OSSSYS, 0, mmIH_RB_BASE_HI), > (adev->irq.ih.gpu_addr >> 40) & 0xff); The gpu_addr should be 64bit anyway. Regards, Christian. > > Jerry > >> > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx