> -----Original Message----- > From: De Marchi, Lucas <lucas.demarchi@xxxxxxxxx> > Sent: Tuesday, February 25, 2025 8:18 PM > To: Upadhyay, Tejas <tejas.upadhyay@xxxxxxxxx> > Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; 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 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 get what you suggested earlier I changed according to that only. But later there was suggestion from Rodrigo to move/add to Documentation/gpu/drm-uapi.rst. @Vivi, Rodrigo Are you ok if we remove from here and keep in xe_drm.h only? Tejas > > 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