| Message ID | 20210413135248.1266-1-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] drm/nouveau: use bo->base.size instead of mem->num_pages | expand |
On Tue, 13 Apr 2021 at 14:52, Christian König <ckoenig.leichtzumerken@gmail.com> wrote: > > Change a couple of cases where it makes more sense to use the base size > instead of the number of pages in the resource. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/nouveau/nouveau_bo.c | 9 ++++----- > drivers/gpu/drm/nouveau/nouveau_fbcon.c | 4 ++-- > drivers/gpu/drm/nouveau/nouveau_gem.c | 4 ++-- > 3 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c > index 2d5d68fc15c2..6dbcbe2fa55f 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > @@ -302,7 +302,6 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 domain, > int type = sg ? ttm_bo_type_sg : ttm_bo_type_device; > int ret; > > - nvbo->bo.mem.num_pages = size >> PAGE_SHIFT; So this was redundant, since ttm_bo_init_reserved() already did this for us? > nouveau_bo_placement_set(nvbo, domain, 0); > INIT_LIST_HEAD(&nvbo->io_reserve_lru); > > @@ -364,12 +363,12 @@ static void > set_placement_range(struct nouveau_bo *nvbo, uint32_t domain) > { > struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev); > - u32 vram_pages = drm->client.device.info.ram_size >> PAGE_SHIFT; > + u64 vram_size = drm->client.device.info.ram_size; > unsigned i, fpfn, lpfn; > > if (drm->client.device.info.family == NV_DEVICE_INFO_V0_CELSIUS && > nvbo->mode && (domain & NOUVEAU_GEM_DOMAIN_VRAM) && > - nvbo->bo.mem.num_pages < vram_pages / 4) { > + nvbo->bo.base.size < vram_size / 4) { > /* > * Make sure that the color and depth buffers are handled > * by independent memory controller units. Up to a 9x > @@ -377,11 +376,11 @@ set_placement_range(struct nouveau_bo *nvbo, uint32_t domain) > * at the same time. > */ > if (nvbo->zeta) { > - fpfn = vram_pages / 2; > + fpfn = (vram_size / 2) >> PAGE_SHIFT; > lpfn = ~0; > } else { > fpfn = 0; > - lpfn = vram_pages / 2; > + lpfn = (vram_size / 2) >> PAGE_SHIFT; > } > for (i = 0; i < nvbo->placement.num_placement; ++i) { > nvbo->placements[i].fpfn = fpfn; > diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c > index 4fc0fa696461..93ac78bda750 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c > +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c > @@ -379,10 +379,10 @@ nouveau_fbcon_create(struct drm_fb_helper *helper, > FBINFO_HWACCEL_IMAGEBLIT; > info->fbops = &nouveau_fbcon_sw_ops; > info->fix.smem_start = nvbo->bo.mem.bus.offset; > - info->fix.smem_len = nvbo->bo.mem.num_pages << PAGE_SHIFT; > + info->fix.smem_len = nvbo->bo.base.size; Is byte level granularity a thing in general? I would have assumed that base.size is always aligned to PAGE_SIZE or whatever? At least in ttm_bo_init_reserved() we first align the size and then calculate the num_pages, so not sure. Hopefully this is not a concern, and should be equivalent. Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Am 13.04.21 um 17:54 schrieb Matthew Auld: > On Tue, 13 Apr 2021 at 14:52, Christian König > <ckoenig.leichtzumerken@gmail.com> wrote: >> Change a couple of cases where it makes more sense to use the base size >> instead of the number of pages in the resource. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/nouveau/nouveau_bo.c | 9 ++++----- >> drivers/gpu/drm/nouveau/nouveau_fbcon.c | 4 ++-- >> drivers/gpu/drm/nouveau/nouveau_gem.c | 4 ++-- >> 3 files changed, 8 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c >> index 2d5d68fc15c2..6dbcbe2fa55f 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c >> @@ -302,7 +302,6 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 domain, >> int type = sg ? ttm_bo_type_sg : ttm_bo_type_device; >> int ret; >> >> - nvbo->bo.mem.num_pages = size >> PAGE_SHIFT; > So this was redundant, since ttm_bo_init_reserved() already did this for us? No, this is redundant since we now use base.size in the functions called by nouveau_bo_placement_set(). Since base.size is already initialized here we should be able to safely drop this. > >> nouveau_bo_placement_set(nvbo, domain, 0); >> INIT_LIST_HEAD(&nvbo->io_reserve_lru); >> >> @@ -364,12 +363,12 @@ static void >> set_placement_range(struct nouveau_bo *nvbo, uint32_t domain) >> { >> struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev); >> - u32 vram_pages = drm->client.device.info.ram_size >> PAGE_SHIFT; >> + u64 vram_size = drm->client.device.info.ram_size; >> unsigned i, fpfn, lpfn; >> >> if (drm->client.device.info.family == NV_DEVICE_INFO_V0_CELSIUS && >> nvbo->mode && (domain & NOUVEAU_GEM_DOMAIN_VRAM) && >> - nvbo->bo.mem.num_pages < vram_pages / 4) { >> + nvbo->bo.base.size < vram_size / 4) { >> /* >> * Make sure that the color and depth buffers are handled >> * by independent memory controller units. Up to a 9x >> @@ -377,11 +376,11 @@ set_placement_range(struct nouveau_bo *nvbo, uint32_t domain) >> * at the same time. >> */ >> if (nvbo->zeta) { >> - fpfn = vram_pages / 2; >> + fpfn = (vram_size / 2) >> PAGE_SHIFT; >> lpfn = ~0; >> } else { >> fpfn = 0; >> - lpfn = vram_pages / 2; >> + lpfn = (vram_size / 2) >> PAGE_SHIFT; >> } >> for (i = 0; i < nvbo->placement.num_placement; ++i) { >> nvbo->placements[i].fpfn = fpfn; >> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c >> index 4fc0fa696461..93ac78bda750 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c >> @@ -379,10 +379,10 @@ nouveau_fbcon_create(struct drm_fb_helper *helper, >> FBINFO_HWACCEL_IMAGEBLIT; >> info->fbops = &nouveau_fbcon_sw_ops; >> info->fix.smem_start = nvbo->bo.mem.bus.offset; >> - info->fix.smem_len = nvbo->bo.mem.num_pages << PAGE_SHIFT; >> + info->fix.smem_len = nvbo->bo.base.size; > Is byte level granularity a thing in general? Yes, on AMD GPUs we have resources which must be managed in bytes or even arbitrary units like blocks of registers for example. > I would have assumed > that base.size is always aligned to PAGE_SIZE or whatever? At least in > ttm_bo_init_reserved() we first align the size and then calculate the > num_pages, so not sure. Hopefully this is not a concern, and should be > equivalent. Yes currently the size should always be aligned to a page, but that is exactly what I want to get away from. > Reviewed-by: Matthew Auld <matthew.auld@intel.com> Thanks, Christian.
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 2d5d68fc15c2..6dbcbe2fa55f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -302,7 +302,6 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 domain, int type = sg ? ttm_bo_type_sg : ttm_bo_type_device; int ret; - nvbo->bo.mem.num_pages = size >> PAGE_SHIFT; nouveau_bo_placement_set(nvbo, domain, 0); INIT_LIST_HEAD(&nvbo->io_reserve_lru); @@ -364,12 +363,12 @@ static void set_placement_range(struct nouveau_bo *nvbo, uint32_t domain) { struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev); - u32 vram_pages = drm->client.device.info.ram_size >> PAGE_SHIFT; + u64 vram_size = drm->client.device.info.ram_size; unsigned i, fpfn, lpfn; if (drm->client.device.info.family == NV_DEVICE_INFO_V0_CELSIUS && nvbo->mode && (domain & NOUVEAU_GEM_DOMAIN_VRAM) && - nvbo->bo.mem.num_pages < vram_pages / 4) { + nvbo->bo.base.size < vram_size / 4) { /* * Make sure that the color and depth buffers are handled * by independent memory controller units. Up to a 9x @@ -377,11 +376,11 @@ set_placement_range(struct nouveau_bo *nvbo, uint32_t domain) * at the same time. */ if (nvbo->zeta) { - fpfn = vram_pages / 2; + fpfn = (vram_size / 2) >> PAGE_SHIFT; lpfn = ~0; } else { fpfn = 0; - lpfn = vram_pages / 2; + lpfn = (vram_size / 2) >> PAGE_SHIFT; } for (i = 0; i < nvbo->placement.num_placement; ++i) { nvbo->placements[i].fpfn = fpfn; diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 4fc0fa696461..93ac78bda750 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -379,10 +379,10 @@ nouveau_fbcon_create(struct drm_fb_helper *helper, FBINFO_HWACCEL_IMAGEBLIT; info->fbops = &nouveau_fbcon_sw_ops; info->fix.smem_start = nvbo->bo.mem.bus.offset; - info->fix.smem_len = nvbo->bo.mem.num_pages << PAGE_SHIFT; + info->fix.smem_len = nvbo->bo.base.size; info->screen_base = nvbo_kmap_obj_iovirtual(nvbo); - info->screen_size = nvbo->bo.mem.num_pages << PAGE_SHIFT; + info->screen_size = nvbo->bo.base.size; drm_fb_helper_fill_info(info, &fbcon->helper, sizes); diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index c88cbb85f101..a70e82413fa7 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -253,7 +253,7 @@ nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem, rep->offset = vma->addr; } - rep->size = nvbo->bo.mem.num_pages << PAGE_SHIFT; + rep->size = nvbo->bo.base.size; rep->map_handle = drm_vma_node_offset_addr(&nvbo->bo.base.vma_node); rep->tile_mode = nvbo->mode; rep->tile_flags = nvbo->contig ? 0 : NOUVEAU_GEM_TILE_NONCONTIG; @@ -638,7 +638,7 @@ nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli, nvbo = (void *)(unsigned long)bo[r->reloc_bo_index].user_priv; if (unlikely(r->reloc_bo_offset + 4 > - nvbo->bo.mem.num_pages << PAGE_SHIFT)) { + nvbo->bo.base.size)) { NV_PRINTK(err, cli, "reloc outside of bo\n"); ret = -EINVAL; break;
Change a couple of cases where it makes more sense to use the base size instead of the number of pages in the resource. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/nouveau/nouveau_bo.c | 9 ++++----- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 4 ++-- drivers/gpu/drm/nouveau/nouveau_gem.c | 4 ++-- 3 files changed, 8 insertions(+), 9 deletions(-)