[PATCH 1/7] drm/nouveau: use bo->base.size instead of mem->num_pages

[Christian König]

Am 13.04.21 um 17:54 schrieb Matthew Auld:
> On Tue, 13 Apr 2021 at 14:52, Christian König
> <ckoenig.leichtzumerken at 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 at 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 at intel.com>

Thanks,
Christian.