[PATCH] drm/xe: Modify minimum of various scheduler timeout to 0

Thu May 16 10:09:05 UTC 2024

On 5/14/2024 3:47 PM, Konda, SaiKishore wrote:
>
> Hi @Ghimiray, Himal Prasad <mailto:himal.prasad.ghimiray at intel.com>
>
> What was the conclusion ?  Will XE team support this ? Sysman 
> consumers use single KMD Config and they don’t have different configs 
> for different use cases.
>
I discussed with Saikishore and He will check with XPUM E2E team for 
usages of L0 API which are programming exclusive mode.

> Thanks
>
> SaiKishore
>
> *From:*Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>
> *Sent:* Friday, May 10, 2024 3:14 PM
> *To:* Joonas Lahtinen <joonas.lahtinen at linux.intel.com>; De Marchi, 
> Lucas <lucas.demarchi at intel.com>; Brost, Matthew 
> <matthew.brost at intel.com>; Konda, SaiKishore <saikishore.konda at intel.com>
> *Cc:* intel-xe at lists.freedesktop.org; Upadhyay, Tejas 
> <tejas.upadhyay at intel.com>; Siddiqui, Ayaz A <ayaz.siddiqui at intel.com>
> *Subject:* Re: [PATCH] drm/xe: Modify minimum of various scheduler 
> timeout to 0
>
> ------------------------------------------------------------------------
>
> *From:*Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> *Sent:* Friday, May 10, 2024 2:39:56 PM
> *To:* De Marchi, Lucas <lucas.demarchi at intel.com>; Brost, Matthew 
> <matthew.brost at intel.com>
> *Cc:* Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>; 
> intel-xe at lists.freedesktop.org <intel-xe at lists.freedesktop.org>; 
> Upadhyay, Tejas <tejas.upadhyay at intel.com>
> *Subject:* Re: [PATCH] drm/xe: Modify minimum of various scheduler 
> timeout to 0
>
> Quoting Matthew Brost (2024-05-09 19:00:43)
> > On Wed, May 08, 2024 at 08:06:52AM -0500, Lucas De Marchi wrote:
> > > On Mon, May 06, 2024 at 07:04:47PM GMT, Himal Prasad Ghimiray wrote:
> > > > Remove the min/max configs and change minimum values of 
> preemption and
> > > > timeslice to 0. The sysman teams must deactivate preemption and
> > > > timeslice to assess exclusivity mode. They depend on configuring
> > > > timeslice_duration_us and preempt_timeout_us to 0, ensuring these
> > > > values are set at their minimum.
> > >
> > > nack for now, based on this explanation. See below.
> > >
> > > >
> > > > Cc: Matthew Brost <matthew.brost at intel.com>
> > > > Cc: Tejas Upadhyay <tejas.upadhyay at intel.com>
> > > > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > > Signed-off-by: Himal Prasad Ghimiray 
> <himal.prasad.ghimiray at intel.com>
>
> <SNIP>
>
> > > > +++ b/drivers/gpu/drm/xe/xe_hw_engine.h
> > > > @@ -10,41 +10,17 @@
> > > >
> > > > struct drm_printer;
> > > >
> > > > -#ifdef CONFIG_DRM_XE_JOB_TIMEOUT_MIN
> > > > -#define XE_HW_ENGINE_JOB_TIMEOUT_MIN CONFIG_DRM_XE_JOB_TIMEOUT_MIN
> > > > -#else
> > > > #define XE_HW_ENGINE_JOB_TIMEOUT_MIN 1
> > > > -#endif
> > > > -#ifdef CONFIG_DRM_XE_JOB_TIMEOUT_MAX
> > > > -#define XE_HW_ENGINE_JOB_TIMEOUT_MAX CONFIG_DRM_XE_JOB_TIMEOUT_MAX
> > > > -#else
> > > > #define XE_HW_ENGINE_JOB_TIMEOUT_MAX (10 * 1000)
> > >
> > > previously one could configure a kernel with a different max.
> > > And now it's hardcoded. How is this better and related to the
> > > motivation in the commit message?
> > >
> > > is this commit doing 2 different things?
> > >
> > > 1) Removing the _MAX from config since nobody is or should change that
> > > 2) Modify the min value to 0... That seems weird as basically there is
> > >    no minimum.
> >
> > This is true.
>
> Maybe I've missed some discussion but what is wrong with the previously
> agreed approach where KConfig would determine hard _MIN/_MAX bounds
> (and maybe _DEFAULT) that the kernel can work with, then there would be
> sysfs min/max (+ default) which the sysadmin sets as comfortable with
> considering the expected use-cases on the system and application then
> choose within those bounds (if it wants expedited hang detection for an
> example)?
>
> > > I'd like to hear Matt Brost's opinion on that.
> >
> > I chatted with Lucas and second his nack. The requirements for this
> > change are coming from sysman and are unclear. If sysman needs to set
> > this to 0 (akin to no maximum), why can't sysman build the kernel with a
> > Kconfig to allow this? This use case appears to be an HPC/AI data center
> > scenario in which the kernel is probably built with a non-standard
> > Kconfig anyway. Why can't CONFIG_DRM_*_MIN be set to zero? This needs to
> > be addressed clearly and publicly.
>
> I also thought we agreed on doing different build for such scenario
> where the interactive desktop is not expected to be used, so larger
> delays are more allowable.
>
So you are proposing an server only build where it can be set to zero.

Regards

-Ayaz


>
> The default build simply can't allow unlimited blocking of the HW by
> certain workloads in order to make sure the desktop/compositor stays
> operational. That can only be enabled by default build when the 
> long-running
> workloads can be isolated so they won't interfere with the workloads
> using dma-fences. And I don't think that code exists yet.
>
> Sysman should be able to report a feature missing from the kernel if it
> detects that it can't set the limits wanted. This can be reported to
> user via error code and/or error message asking to install a different
> kernel build.
>
> Same strategy has been applied in the past for various kernel features
> like realtime/virtualization etc.
>
> Regards, Joonas
>
> >
> > Matt
> >
> > >
> > > Lucas De Marchi
> > >
> > > > -#endif
> > > > -#ifdef CONFIG_DRM_XE_TIMESLICE_MIN
> > > > -#define XE_HW_ENGINE_TIMESLICE_MIN CONFIG_DRM_XE_TIMESLICE_MIN
> > > > -#else
> > > > -#define XE_HW_ENGINE_TIMESLICE_MIN 1
> > > > -#endif
> > > > -#ifdef CONFIG_DRM_XE_TIMESLICE_MAX
> > > > -#define XE_HW_ENGINE_TIMESLICE_MAX CONFIG_DRM_XE_TIMESLICE_MAX
> > > > -#else
> > > > +#define XE_HW_ENGINE_TIMESLICE_MIN 0
> > > > #define XE_HW_ENGINE_TIMESLICE_MAX (10 * 1000 * 1000)
> > > > -#endif
> > > > #ifdef CONFIG_DRM_XE_PREEMPT_TIMEOUT
> > > > #define XE_HW_ENGINE_PREEMPT_TIMEOUT CONFIG_DRM_XE_PREEMPT_TIMEOUT
> > > > #else
> > > > #define XE_HW_ENGINE_PREEMPT_TIMEOUT (640 * 1000)
> > > > #endif
> > > > -#ifdef CONFIG_DRM_XE_PREEMPT_TIMEOUT_MIN
> > > > -#define XE_HW_ENGINE_PREEMPT_TIMEOUT_MIN 
> CONFIG_DRM_XE_PREEMPT_TIMEOUT_MIN
> > > > -#else
> > > > -#define XE_HW_ENGINE_PREEMPT_TIMEOUT_MIN 1
> > > > -#endif
> > > > -#ifdef CONFIG_DRM_XE_PREEMPT_TIMEOUT_MAX
> > > > -#define XE_HW_ENGINE_PREEMPT_TIMEOUT_MAX 
> CONFIG_DRM_XE_PREEMPT_TIMEOUT_MAX
> > > > -#else
> > > > +#define XE_HW_ENGINE_PREEMPT_TIMEOUT_MIN 0
> > > > #define XE_HW_ENGINE_PREEMPT_TIMEOUT_MAX (10 * 1000 * 1000)
> > > > -#endif
> > > >
> > > > int xe_hw_engines_init_early(struct xe_gt *gt);
> > > > int xe_hw_engines_init(struct xe_gt *gt);
> > > > --
> > > > 2.25.1
> > > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-xe/attachments/20240516/9552f8ef/attachment-0001.htm>