On Tue, Feb 25, 2025 at 04:39:25AM +0000, Upadhyay, Tejas wrote:
-----Original Message-----
From: De Marchi, Lucas <lucas.demarchi@xxxxxxxxx>
Sent: Tuesday, February 25, 2025 12:42 AM
To: Upadhyay, Tejas <tejas.upadhyay@xxxxxxxxx>
Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
Belgaumkar, Vinay <vinay.belgaumkar@xxxxxxxxx>
Subject: Re: [PATCH V9] drm/xe/uapi: Use hint for guc to set GT frequency
On Mon, Feb 24, 2025 at 09:43:09AM -0600, Lucas De Marchi wrote:
>On Wed, Feb 12, 2025 at 05:08:30PM +0530, Tejas Upadhyay wrote:
>>Allow user to provide a low latency hint. When set, KMD sends a hint
>>to GuC which results in special handling for that process. SLPC will
>>ramp the GT frequency aggressively every time it switches to this
>>process.
>>
>>We need to enable the use of SLPC Compute strategy during init, but it
>>will apply only to processes that set this bit during process
>>creation.
>>
>>Improvement with this approach as below:
>>
>>Before,
>>
>>:~$ NEOReadDebugKeys=1 EnableDirectSubmission=0 clpeak
>>--kernel-latency
>>Platform: Intel(R) OpenCL Graphics
>> Device: Intel(R) Graphics [0xe20b]
>> Driver version : 24.52.0 (Linux x64)
>> Compute units : 160
>> Clock frequency : 2850 MHz
>> Kernel launch latency : 283.16 us
>>
>>After,
>>
>>:~$ NEOReadDebugKeys=1 EnableDirectSubmission=0 clpeak
>>--kernel-latency
>>Platform: Intel(R) OpenCL Graphics
>> Device: Intel(R) Graphics [0xe20b]
>> Driver version : 24.52.0 (Linux x64)
>> Compute units : 160
>> Clock frequency : 2850 MHz
>>
>> Kernel launch latency : 63.38 us
>>
>>UMD Compute PR : https://github.com/intel/compute-runtime/pull/794
>>UMD Mesa PR :
>>https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33214
>>
>>v9(Vinay):
>> - remove extra line, align commit message
>>v8(Vinay):
>> - Add separate example for using low latency hint
>>v7(Jose):
>> - Update UMD PR
>> - applicable to all gpus
>>V6:
>> - init flags, remove redundant flags check (MAuld)
>>V5:
>> - Move uapi doc to documentation and GuC ABI specific change
>>(Rodrigo)
>
>hmn... that doesn't look right.
>
>...
>
>>diff --git a/Documentation/gpu/drm-uapi.rst
>>b/Documentation/gpu/drm-uapi.rst index b75cc9a70d1f..7337d1be45ef
>>100644
>>--- a/Documentation/gpu/drm-uapi.rst
>>+++ b/Documentation/gpu/drm-uapi.rst
>>@@ -583,3 +583,21 @@ dma-buf interoperability
>>
>>Please see Documentation/userspace-api/dma-buf-alloc-exchange.rst for
>>information on how dma-buf is integrated and exposed within DRM.
>>+
>>+Low latency hint by user
>>+========================
>>+
>>+Allow users to provide a hint to kernel for cases demanding low
>>+latency profile. Please note it will have impact on power
>>+consumption. User can indicate low latency hint with flag while
>>+creating exec queue as mentioned below,
>>+
>>+ struct drm_xe_exec_queue_create exec_queue_create = {
>>+ .flags = DRM_XE_EXEC_QUEUE_LOW_LATENCY_HINT,
>>+ .extensions = 0,
>>+ .vm_id = vm,
>>+ .num_bb_per_exec = 1,
>>+ .num_eng_per_bb = 1,
>>+ .instances = to_user_pointer(&instance),
>>+ };
>>+ ioctl(fd, DRM_IOCTL_XE_EXEC_QUEUE_CREATE, &exec_queue_create);
>
>how does this driver-specific doc make sense in this file?
Thanks for looking at this.
There was suggestion to add this in documentation during review, @Vivi, Rodrigo Can we remove this? And if we remove this whats point in keeping documentation here for 4 line paragraph?
@De Marchi, Lucas Do you mean completely remove from here, just keep in xe_drm.h?
AFAICS in v4 the request was to add it to the documentation, not to add
it to a random documentation file. In v4 this example was only in the
commit message rather than in the documentation.
If you add it as kernel-doc you are adding it to the documentation. See
https://docs.kernel.org/gpu/driver-uapi.html#c.drm_xe_exec_queue_create
^^^^^^^^^^^
The xe uapi shows up here:
https://docs.kernel.org/gpu/driver-uapi.html#drm-xe-uapi because:
Documentation/gpu/index.rst:
...
driver-uapi
...
Documentation/gpu/driver-uapi:
drm/xe uAPI
===========
.. kernel-doc:: include/uapi/drm/xe_drm.h
In v9 this example is duplicated in Documentation/gpu/drm-uapi.rst
(wrong file) and xe_drm.h (right one).
I hope that clarifies.
Lucas De Marchi
Tejas
to avoid needless noise in the log, I zapped this commit from
drm-xe-next:
+ 8f78c0dfbe9d...8b4b3af869e9 drm-xe-next -> drm-xe-next (forced
update)
8b4b3af o [drm-xe-next] {drm-xe/drm-xe-next} {drm-xe/HEAD}
drm/xe/userptr: remove tmp_evict list
6b93cb9 o drm/xe/userptr: fix EFAULT handling
4e37e92 o drm/xe/userptr: restore invalidation list on error 8f78c0d │ o
drm/xe/userptr: remove tmp_evict list f2211aa │ o drm/xe/userptr: fix EFAULT
handling b9aaabb │ o drm/xe/userptr: restore invalidation list on error
bee37a8 │ o drm/xe/uapi: Use hint for guc to set GT frequency
this documentation is also in include/uapi/drm/xe_drm.h, so maybe the best
thing to do is probably to just drop this part that got included in the wrong
part. Once this is fixed we can add it back.
Lucas De Marchi
