[PATCH v7 4/4] drm/amdgpu: track bo memory stats at runtime

[Li, Yunxiang (Teddy)]

[Public]

> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: Wednesday, November 13, 2024 9:22
> Am 13.11.24 um 14:53 schrieb Li, Yunxiang (Teddy):
> > [Public]
> >
> >> From: Koenig, Christian <Christian.Koenig at amd.com>
> >> Sent: Wednesday, November 13, 2024 6:39 Am 13.11.24 um 11:25 schrieb
> >> Tvrtko Ursulin:
> >>> On 13/11/2024 08:49, Christian König wrote:
> >>>> Am 12.11.24 um 19:16 schrieb Li, Yunxiang (Teddy):
> >>>>> [SNIP]
> >>>>>>> +   size = sign * amdgpu_bo_size(bo);
> >>>>>>> +   res = bo->tbo.resource;
> >>>>>>> +   type = res ? res->mem_type :
> >>>>>>> amdgpu_bo_get_preferred_placement(bo);
> >>>>>> Again, it's a clear NAK from my side to do stuff like that.
> >>>>>>
> >>>>>> When there isn't any backing store the BO should *not* be
> >>>>>> accounted to anything.
> >>>>> I don't have a preference either way, but I think it should be a
> >>>>> separate discussion to properly define what drm-total- means.
> >>> Total must show the total size of all BOs which exist even if they
> >>> don't currently have a backing store. That's how drm-usage-stats.rst
> >>> defines the field and that is how all the other drivers work.
> >> In that case we should only look at the preferred placement and not
> >> the backing store at all.
> >>
> >> But that makes the total identical to the requested value, doesn't it?
> > Yes, the issue is not which BO needs to be counted but where they should be
> counted. This gets more complicated if we consider BOs to prefer multiple
> placements.
> >
> > IMO it makes sense to have drm-total- to work like the legacy amd-requested-
> where we look at BO's preferred placement. For multiple preferred placements we
> say that the implementation needs to pick one of them to avoid double counting, but
> which one is up to the implementation as long as it's done in a consistent manner.
> Does that sound reasonable?
>
> Yeah that works for me. Just don't look at both bo->preferred_placement and bo-
> >tbo.resource because that will certainly be inconsistent in some use cases.

oof, from the commit message i915/xe is doing the exact opposite, BO gets counted in the totals for all the possible(preferred?) regions.

Teddy