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.somalapu...@amd.com>
> > 
> > [...]
> > 
> > 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

Reply via email to