[PATCH] drm/amdgpu: Fix 32bit x86 compilation warning

[Alex Deucher]

On Wed, Mar 29, 2017 at 3:07 AM, Christian König
<deathsimple at vodafone.de> wrote:
> 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.

The problem is these are dma_addr_t not 64 bit addresses.

Alex

>
>
>>>
>>> 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
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx