[Intel-xe] [PATCH 0/1] drm: Add a gpu page-table walker

[Thomas Hellström]

Hi, Daniel,

On 2/16/23 21:18, Daniel Vetter wrote:
> On Thu, Feb 16, 2023 at 05:27:28PM +0100, Thomas Hellström wrote:
>> A slightly unusual cover letter for a single patch.
>>
>> The page table walker is currently used by the xe driver only,
>> but the code is generic so we can be good citizens and add it to drm
>> as a helper, for possible use by other drivers,
>> If so we can merge the commit when we merge the xe driver.
>>
>> The question raised here is
>> *) Should it be a generic drm helper or xe-specific with changed
>>     prefixes?
> I think if there's some other drivers interested in using this, then this
> sounds like a good idea. Maybe more useful if it's also integrated into
> the vm/vma helpers that are being discussed as an optional part?
>
> Maybe some good old sales pitching here to convince people would be good.
>
> Maybe one of the new accel drivers is interested in this too?

Thanks for your thoughts on this. Yeah, I think it's a bit awkward to 
push for having code generic when there is only one user, and the 
prospect of having other drivers rewrite their page-table building code 
based on this helper in the near future is probably small. Perhaps more 
of interest to new drivers. I think what will happen otherwise is that 
during some future cleanup this will be pushed down to xe claiming it's 
the only user.

I wonder whether it might be an idea to maintain a small document where 
driver writers can list suggestions for code that could be lifted to 
core drm and be reused by others. That way both reviewers and writers of 
other drivers can keep an eye on that document and use it to avoid 
duplicating code. The procedure would then be to lift it to core drm and 
fix up prefixes as soon as we have two or more users.

Thoughts?

Thomas


>
>> *) If a drm helper, should we use a config option?
> I am no fan of Kconfig things tbh. Maybe just include it in the vma
> helpers, or perhaps we want to do a drm-accel-helpers with gem helpers,
> drm/sched, this one here, vm/vma helpers or whatever they will be and so
> on? Kinda like we have modeset helpers.
>
> I'd definitely not go for a Kconfig per individual file, that's just
> excessive.
> -Daniel
>
>> For usage examples, see xe_pt.c
>> https://gitlab.freedesktop.org/drm/xe/kernel/-/blob/drm-xe-next/drivers/gpu/drm/xe/xe_pt.c
>>
>> Thanks,
>> Thomas
>>
>> Thomas Hellström (1):
>>    drm: Add a gpu page-table walker helper
>>
>>   drivers/gpu/drm/Makefile      |   1 +
>>   drivers/gpu/drm/drm_pt_walk.c | 159 +++++++++++++++++++++++++++++++++
>>   include/drm/drm_pt_walk.h     | 161 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 321 insertions(+)
>>   create mode 100644 drivers/gpu/drm/drm_pt_walk.c
>>   create mode 100644 include/drm/drm_pt_walk.h
>>
>> -- 
>> 2.34.1
>>