Re: [PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 07.09.22 um 19:00 schrieb Daniel Vetter:
[SNIP]
I'm a bit confused why the bloat here ...
Drivers do have specialized implementations of the backend, e.g. VMWGFX have
his handle backend, amdgpu the VRAM backend with special placements, i915 is
completely special as well.

Here we only move the decision if resources intersect or are compatible into
those specialized backends. Previously we had all this in a centralized
callback for all backends of a driver.

See the switch in amdgpu_ttm_bo_eviction_valuable() for an example. Final
goal is to move all this stuff into the specialized backends and remove this
callback.

The only driver where I couldn't figure out why we have duplicated all this
from the standard implementation is Nouveau.
Yeah I didn't read this too carefully, apologies.

Also please document new callbacks precisely with inline kerneldoc. I know
ttm docs aren't great yet, but they don't get better if we don't start
somewhere. I think the in-depth comments for modeset vfuncs (e.g. in
drm_modeset_helper_vtables.h) are a good standard here.
I thought we already did that. Please be a bit more specific.
Yeah rushed this too, but the kerneldoc isn't too great yet. It's
definitely not formatted correctly (you can't do a full function
definition in a struct unfortunately, see the examples I linked). And it
would be good to specificy what the default implementation is, that there
is one (i.e. the hook is optional) and when exactly a driver would want to
overwrite this. Atm it's a one-liner that explains exactly as much as you
can guess from the function interface anyway, that's not super userful.
-Daniel

Arun can you take care of improving this?

Thanks,
Christian.




Thanks,
Christian.

-Daniel

Thanks,
Arun
Regards,
Christian.





[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux