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