On Fri, Mar 03, 2023 at 03:55:56PM +0100, Michel Dänzer wrote:
> On 3/3/23 08:16, Somalapuram Amaranath wrote:
> > Change the ttm_place structure member fpfn, lpfn, mem_type to
> > res_start, res_end, res_type.
> > Change the unsigned to u64.
> > Fix the dependence in all the DRM drivers and
> > clean up PAGE_SHIFT operation.
> >
> > Signed-off-by: Somalapuram Amaranath <Amaranath.Somalapuram@xxxxxxx>
> >
> > [...]
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> > index 44367f03316f..5b5104e724e3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> > @@ -131,11 +131,12 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> > goto err_free;
> > }
> >
> > - if (place->lpfn) {
> > + if (place->res_end) {
> > spin_lock(&mgr->lock);
> > r = drm_mm_insert_node_in_range(&mgr->mm, &node->mm_nodes[0],
> > - num_pages, tbo->page_alignment,
> > - 0, place->fpfn, place->lpfn,
> > + num_pages, tbo->page_alignment, 0,
> > + place->res_start << PAGE_SHIFT,
> > + place->res_end << PAGE_SHIFT,
> > DRM_MM_INSERT_BEST);
>
> This should be >> or no shift instead of <<, shouldn't it? Multiplying a value in bytes by the page size doesn't make sense.
>
>
> I didn't check the rest of the patch in detail, but it's easy introduce subtle regressions with this kind of change. It'll require a lot of review & testing scrutiny.
Also good justification. The changelog says only what is done, nothing about why the change is needed.
Regards
Stanislaw
