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

[Arunpravin Paneer Selvam <arunpravin.paneerselvam@amd.com>]

On 9/8/2022 11:34 AM, Christian König wrote:
> 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?

Hi Christian,
Yes, I will check on this.

Thanks,
Arun
>
> Thanks,
> Christian.
>
>>
>>
>>
>>> Thanks,
>>> Christian.
>>>
>>>> -Daniel
>>>>
>>>>> Thanks,
>>>>> Arun
>>>>>> Regards,
>>>>>> Christian.
>>>>>>
>