On 2017-11-27 06:12 PM, Christian König wrote:
> Am 27.11.2017 um 17:40 schrieb Felix Kuehling:
>> On 2017-11-27 11:02 AM, Christian König wrote:
>>> The block size only affects the leave nodes, everything else is fixed.
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>> Â drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 28
>>> +++++++++++++++++++++++-----
>>> Â 1 file changed, 23 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 122379dfc7d8..f1e541e9b514 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -139,6 +139,24 @@ struct amdgpu_prt_cb {
>>> Â };
>>> Â Â /**
>>> + * amdgpu_vm_level_shift - return the addr shift for each level
>>> + *
>>> + * @adev: amdgpu_device pointer
>>> + *
>>> + * Returns the number of bits the pfn needs to be right shifted for
>>> a level.
>>> + */
>>> +static unsigned amdgpu_vm_level_shift(struct amdgpu_device *adev,
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â unsigned level)
>>> +{
>>> +Â Â Â if (level != adev->vm_manager.num_level)
>>> +Â Â Â Â Â Â Â return 9 * (adev->vm_manager.num_level - level - 1) +
>>> +Â Â Â Â Â Â Â Â Â Â Â adev->vm_manager.block_size;
>>> +Â Â Â else
>>> +Â Â Â Â Â Â Â /* For the page tables on the leaves */
>>> +Â Â Â Â Â Â Â return 0;
>>> +}
>>> +
>>> +/**
>>> Â Â * amdgpu_vm_num_entries - return the number of entries in a PD/PT
>>> Â Â *
>>> Â Â * @adev: amdgpu_device pointer
>>> @@ -288,8 +306,7 @@ static int amdgpu_vm_alloc_levels(struct
>>> amdgpu_device *adev,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â uint64_t saddr, uint64_t eaddr,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â unsigned level)
>>> Â {
>>> -Â Â Â unsigned shift = (adev->vm_manager.num_level - level) *
>>> -Â Â Â Â Â Â Â adev->vm_manager.block_size;
>>> +Â Â Â unsigned shift = amdgpu_vm_level_shift(adev, level);
>>> Â Â Â Â Â unsigned pt_idx, from, to;
>>> Â Â Â Â Â int r;
>>> Â Â Â Â Â u64 flags;
>>> @@ -1302,18 +1319,19 @@ void amdgpu_vm_get_entry(struct
>>> amdgpu_pte_update_params *p, uint64_t addr,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct amdgpu_vm_pt **entry,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct amdgpu_vm_pt **parent)
>>> Â {
>>> -Â Â Â unsigned idx, level = p->adev->vm_manager.num_level;
>>> +Â Â Â unsigned level = 0;
>> This generates a checkpatch.pl warning.
>>
>> General question: Do you have a policy for running your patches through
>> checkpatch.pl before/during/after code review?
>
> No, I'm using editor settings which generate coding style compliant code
> most of the time and complains/shows when it isn't compliant.
>
>> I've noticed before that some of your patches aren't always 100%
>> compliant.
>
> Yeah, amdgpu inherited a lot of code as well as coding style from radeon
> which isn't compliant.
>
> For example in this case checkpatch.pl most likely complains that we
> should use "unsigned int" instead of just "unsigned".
>
> I've tried to clean this up for years, but simply not enough time to
> handle everything.
Moreover, checkpatch.pl cannot always be 100% satisfied, let alone in a
way that actually makes sense for the code in question. Its reports
should be considered guidelines, not the law.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
