From: "Konda, SaiKishore" <saikishore.konda@intel.com>
To: "Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>,
"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
"De Marchi, Lucas" <lucas.demarchi@intel.com>,
"Brost, Matthew" <matthew.brost@intel.com>
Cc: "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
"Upadhyay, Tejas" <tejas.upadhyay@intel.com>,
"Siddiqui, Ayaz A" <ayaz.siddiqui@intel.com>
Subject: RE: [PATCH] drm/xe: Modify minimum of various scheduler timeout to 0
Date: Tue, 14 May 2024 10:17:25 +0000 [thread overview]
Message-ID: <PH0PR11MB5674E8EF849E5B47A847B70594E32@PH0PR11MB5674.namprd11.prod.outlook.com> (raw)
In-Reply-To: <MW4PR11MB705636A5456E4D03F69005E0B3E72@MW4PR11MB7056.namprd11.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 6588 bytes --]
Hi @Ghimiray, Himal Prasad<mailto:himal.prasad.ghimiray@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.
Thanks
SaiKishore
From: Ghimiray, Himal Prasad <himal.prasad.ghimiray@intel.com>
Sent: Friday, May 10, 2024 3:14 PM
To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>; Brost, Matthew <matthew.brost@intel.com>; Konda, SaiKishore <saikishore.konda@intel.com>
Cc: intel-xe@lists.freedesktop.org; Upadhyay, Tejas <tejas.upadhyay@intel.com>; Siddiqui, Ayaz A <ayaz.siddiqui@intel.com>
Subject: Re: [PATCH] drm/xe: Modify minimum of various scheduler timeout to 0
________________________________
From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com<mailto:joonas.lahtinen@linux.intel.com>>
Sent: Friday, May 10, 2024 2:39:56 PM
To: De Marchi, Lucas <lucas.demarchi@intel.com<mailto:lucas.demarchi@intel.com>>; Brost, Matthew <matthew.brost@intel.com<mailto:matthew.brost@intel.com>>
Cc: Ghimiray, Himal Prasad <himal.prasad.ghimiray@intel.com<mailto:himal.prasad.ghimiray@intel.com>>; intel-xe@lists.freedesktop.org<mailto:intel-xe@lists.freedesktop.org> <intel-xe@lists.freedesktop.org<mailto:intel-xe@lists.freedesktop.org>>; Upadhyay, Tejas <tejas.upadhyay@intel.com<mailto:tejas.upadhyay@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@intel.com<mailto:matthew.brost@intel.com>>
> > > Cc: Tejas Upadhyay <tejas.upadhyay@intel.com<mailto:tejas.upadhyay@intel.com>>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com<mailto:joonas.lahtinen@linux.intel.com>>
> > > Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com<mailto:himal.prasad.ghimiray@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.
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
> > >
[-- Attachment #2: Type: text/html, Size: 11161 bytes --]
next prev parent reply other threads:[~2024-05-14 10:17 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-06 13:34 [PATCH] drm/xe: Modify minimum of various scheduler timeout to 0 Himal Prasad Ghimiray
2024-05-06 14:11 ` ✓ CI.Patch_applied: success for " Patchwork
2024-05-06 14:11 ` ✓ CI.checkpatch: " Patchwork
2024-05-06 14:13 ` ✓ CI.KUnit: " Patchwork
2024-05-06 14:29 ` ✓ CI.Build: " Patchwork
2024-05-06 14:35 ` ✓ CI.Hooks: " Patchwork
2024-05-06 14:37 ` ✓ CI.checksparse: " Patchwork
2024-05-06 15:10 ` ✓ CI.BAT: " Patchwork
2024-05-06 16:47 ` ✓ CI.FULL: " Patchwork
2024-05-08 5:35 ` [PATCH] " Upadhyay, Tejas
2024-05-08 13:06 ` Lucas De Marchi
2024-05-09 16:00 ` Matthew Brost
2024-05-10 9:09 ` Joonas Lahtinen
2024-05-10 9:44 ` Ghimiray, Himal Prasad
2024-05-14 10:17 ` Konda, SaiKishore [this message]
2024-05-16 10:09 ` Ayaz A Siddiqui
-- strict thread matches above, loose matches on Subject: below --
2024-03-08 11:00 Himal Prasad Ghimiray
2024-04-05 4:53 ` Upadhyay, Tejas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=PH0PR11MB5674E8EF849E5B47A847B70594E32@PH0PR11MB5674.namprd11.prod.outlook.com \
--to=saikishore.konda@intel.com \
--cc=ayaz.siddiqui@intel.com \
--cc=himal.prasad.ghimiray@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=joonas.lahtinen@linux.intel.com \
--cc=lucas.demarchi@intel.com \
--cc=matthew.brost@intel.com \
--cc=tejas.upadhyay@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).