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

["Christian König" <christian.koenig@amd.com>]

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.
>>>>>