[Intel-gfx] [PATCH v9 7/7] drm/i915: add a sysfs entry to let users set sseu configs

[Rogozhkin, Dmitry V]

On Tue, 2018-07-24 at 21:50 +0000, Bloomfield, Jon wrote:
> Gratuitous top posting to re-kick the thread.
> 
> For Gen11 we can't have an on/off switch anyway (media simply won't
> run
> with an oncompatible slice config), so let's agree on an api to allow
> userland
> to select the slice configuration for its contexts, for Gen11 only.
> I'd prefer a
> simple {MediaConfig/GeneralConfig} type context param but I really
> don't
> care that much.

Does anyone work on breaking the patch series for Gen11 and Gen8/9/10
with Gen11 preceding since this is required for the functional side? We
hope to have userspace media driver ICL patches available pretty soon
and would be really nice to have kernel side prepared as much as
possible... If no one is working on this, can you, Lionel, help,
please?

> 
> For gen9/10 it's arguable whether we need this at all. The effect on
> media
> workloads varies, but I'm guessing normal users (outside a transcode
> type environment) will never know they're missing anything. Either
> way,
> we can continue discussing while we progress the gen11 solution.
> 
> Jon
> 
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf
> > Of
> > Bloomfield, Jon
> > Sent: Wednesday, July 18, 2018 9:44 AM
> > To: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>; Joonas
> > Lahtinen
> > <joonas.lahtinen at linux.intel.com>; Chris Wilson <chris at chris-wilson
> > .co.uk>;
> > Landwerlin, Lionel G <lionel.g.landwerlin at intel.com>; intel-
> > gfx at lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH v9 7/7] drm/i915: add a sysfs entry
> > to let users
> > set sseu configs
> > 
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On
> > > Behalf Of
> > > Tvrtko Ursulin
> > > Sent: Thursday, June 14, 2018 1:29 AM
> > > To: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>; Chris
> > > Wilson
> > > <chris at chris-wilson.co.uk>; Landwerlin, Lionel G
> > > <lionel.g.landwerlin at intel.com>; intel-gfx at lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [PATCH v9 7/7] drm/i915: add a sysfs
> > > entry to let
> > 
> > users
> > > set sseu configs
> > > 
> > > 
> > > On 13/06/2018 13:49, Joonas Lahtinen wrote:
> > > > Quoting Tvrtko Ursulin (2018-06-12 15:02:07)
> > > > > 
> > > > > On 12/06/2018 11:52, Lionel Landwerlin wrote:
> > > > > > On 12/06/18 11:37, Chris Wilson wrote:
> > > > > > > Quoting Lionel Landwerlin (2018-06-12 11:33:34)
> > > > > > > > On 12/06/18 10:20, Joonas Lahtinen wrote:
> > > > > > > > > Quoting Chris Wilson (2018-06-11 18:02:37)
> > > > > > > > > > Quoting Lionel Landwerlin (2018-06-11 14:46:07)
> > > > > > > > > > > On 11/06/18 13:10, Tvrtko Ursulin wrote:
> > > > > > > > > > > > On 30/05/2018 15:33, Lionel Landwerlin wrote:
> > > > > > > > > > > > > There are concerns about denial of service
> > > > > > > > > > > > > around the per
> > > > > > > > > > > > > context sseu
> > > > > > > > > > > > > configuration capability. In a previous
> > > > > > > > > > > > > commit introducing the
> > > > > > > > > > > > > capability we allowed it only for capable
> > > > > > > > > > > > > users. This changes
> > > > > > > > > > > > > adds a
> > > > > > > > > > > > > new debugfs entry to let any user configure
> > > > > > > > > > > > > its own context
> > > > > > > > > > > > > powergating setup.
> > > > > > > > > > > > 
> > > > > > > > > > > > As far as I understood it, Joonas' concerns
> > > > > > > > > > > > here are:
> > > > > > > > > > > > 
> > > > > > > > > > > > 1) That in the containers use case individual
> > > > > > > > > > > > containers
> > 
> > wouldn't
> > > be
> > > > > > > > > > > > able to turn on the sysfs toggle for them.
> > > > > > > > > > > > 
> > > > > > > > > > > > 2) That also in the containers use case if box
> > > > > > > > > > > > admin turns on
> > 
> > the
> > > > > > > > > > > > feature, some containers would potentially
> > > > > > > > > > > > start negatively
> > > > > > > > > > > > affecting
> > > > > > > > > > > > the others (via the accumulated cost of slice
> > > > > > > > > > > > re-configuration
> > 
> > on
> > > > > > > > > > > > context switching).
> > > > > > > > > > > > 
> > > > > > > > > > > > I am not familiar with typical container setups
> > > > > > > > > > > > to be
> > 
> > authoritative
> > > > > > > > > > > > here, but intuitively I find it reasonable that
> > > > > > > > > > > > a low-level
> > 
> > hardware
> > > > > > > > > > > > switch like this would be under the control of
> > > > > > > > > > > > a master domain
> > > > > > > > > > > > administrator. ("If you are installing our
> > > > > > > > > > > > product in the
> > 
> > container
> > > > > > > > > > > > environment, make sure your system
> > > > > > > > > > > > administrator enables
> > 
> > this
> > > > > > > > > > > > hardware
> > > > > > > > > > > > feature.", "Note to system administrators:
> > > > > > > > > > > > Enabling this
> > 
> > features
> > > > > > > > > > > > may
> > > > > > > > > > > > negatively affect the performance of other
> > > > > > > > > > > > containers.")
> > > > > > > > > > > > 
> > > > > > > > > > > > Alternative proposal is for the i915 to apply
> > > > > > > > > > > > an "or" filter on all
> > > > > > > > > > > > requested masks and in that way ensure dynamic
> > > > > > > > > > > > re-
> > > 
> > > configuration
> > > > > > > > > > > > doesn't happen on context switches, but driven
> > > > > > > > > > > > from
> > 
> > userspace
> > > via
> > > > > > > > > > > > ioctls.
> > > > > > > > > > > > 
> > > > > > > > > > > > In other words, should _all_ userspace agree
> > > > > > > > > > > > between
> > > 
> > > themselves that
> > > > > > > > > > > > they want to turn off a slice, they would then
> > > > > > > > > > > > need to send out
> > 
> > a
> > > > > > > > > > > > concerted ioctl storm, where number of needed
> > > > > > > > > > > > ioctls equals
> > > 
> > > the
> > > > > > > > > > > > number
> > > > > > > > > > > > of currently active contexts. (This may have
> > > > > > > > > > > > its own
> > 
> > performance
> > > > > > > > > > > > consequences caused by the barriers needed to
> > > > > > > > > > > > modify all
> > > 
> > > context
> > > > > > > > > > > > images.)
> > > > > > > > > > > > 
> > > > > > > > > > > > This was deemed acceptable the the media use
> > > > > > > > > > > > case, but my
> > > 
> > > concern is
> > > > > > > > > > > > the approach is not elegant and will tie us
> > > > > > > > > > > > with the "or" policy
> > 
> > in
> > > > > > > > > > > > the ABI. (Performance concerns I haven't
> > > > > > > > > > > > evaluated yet, but
> > > 
> > > they
> > > > > > > > > > > > also
> > > > > > > > > > > > may be significant.)
> > > > > > > > > > > > 
> > > > > > > > > > > > If we go back thinking about the containers use
> > > > > > > > > > > > case, then it
> > > > > > > > > > > > transpires that even though the "or" policy
> > > > > > > > > > > > does prevent one
> > > > > > > > > > > > container
> > > > > > > > > > > > from affecting the other from one angle, it
> > > > > > > > > > > > also prevents one
> > > > > > > > > > > > container from exercising the feature unless
> > > > > > > > > > > > all containers
> > > > > > > > > > > > co-operate.
> > > > > > > > > > > > 
> > > > > > > > > > > > As such, we can view the original problem
> > > > > > > > > > > > statement where
> > 
> > we
> > > have an
> > > > > > > > > > > > issue if not everyone co-operates, as
> > > > > > > > > > > > conceptually the same
> > 
> > just
> > > > > > > > > > > > from
> > > > > > > > > > > > an opposite angle. (Rather than one container
> > > > > > > > > > > > incurring the
> > > > > > > > > > > > increased
> > > > > > > > > > > > cost of context switches to the rest, we would
> > > > > > > > > > > > have one
> > > 
> > > container
> > > > > > > > > > > > preventing the optimized slice configuration to
> > > > > > > > > > > > the other.)
> > > > > > > > > > > > 
> > > > > > > > > > > >    From this follows that both proposals
> > > > > > > > > > > > require complete
> > > > > > > > > > > > co-operation
> > > > > > > > > > > > from all running userspace to avoid complete
> > > > > > > > > > > > control of the
> > > 
> > > feature.
> > > > > > > > > > > > 
> > > > > > > > > > > > Since the balance between the benefit of
> > > > > > > > > > > > optimized slice
> > > > > > > > > > > > configuration
> > > > > > > > > > > > (or penalty of suboptimal one), versus the
> > > > > > > > > > > > penalty of increased
> > > > > > > > > > > > context switch times, cannot be know by the
> > > > > > > > > > > > driver (barring
> > > > > > > > > > > > venturing
> > > > > > > > > > > > into the heuristics territory), that is another
> > > > > > > > > > > > reason why I find
> > > > > > > > > > > > the
> > > > > > > > > > > > "or" policy in the driver questionable.
> > > > > > > > > > > > 
> > > > > > > > > > > > We can also ask a question of - If we go with
> > > > > > > > > > > > the "or" policy,
> > 
> > why
> > > > > > > > > > > > require N per-context ioctls to modify the
> > > > > > > > > > > > global GPU
> > > 
> > > configuration
> > > > > > > > > > > > and not instead add a global driver ioctl to
> > > > > > > > > > > > modify the state?
> > > > > > > > > > > > 
> > > > > > > > > > > > If a future hardware requires, or enables, the
> > > > > > > > > > > > per-context
> > > 
> > > behaviour
> > > > > > > > > > > > in a more efficient way, we could then revisit
> > > > > > > > > > > > the problem
> > 
> > space.
> > > > > > > > > > > > 
> > > > > > > > > > > > In the mean time I see the "or" policy solution
> > > > > > > > > > > > as adding some
> > > 
> > > ABI
> > > > > > > > > > > > which doesn't do anything for many use cases
> > > > > > > > > > > > without any way
> > > 
> > > for the
> > > > > > > > > > > > sysadmin to enable it. At the same time master
> > > > > > > > > > > > sysfs knob at
> > > 
> > > least
> > > > > > > > > > > > enables the sysadmin to make a decision. Here I
> > > > > > > > > > > > am thinking
> > > 
> > > about a
> > > > > > > > > > > > random client environment where not all
> > > > > > > > > > > > userspace co-
> > > 
> > > operates,
> > > > > > > > > > > > but for
> > > > > > > > > > > > instance user is running the feature aware
> > > > > > > > > > > > media stack, and
> > > > > > > > > > > > non-feature aware OpenCL/3d stack.
> > > > > > > > > > > > 
> > > > > > > > > > > > I guess the complete story boils down to - is
> > > > > > > > > > > > the master sysfs
> > > 
> > > knob
> > > > > > > > > > > > really a problem in container use cases.
> > > > > > > > > > > > 
> > > > > > > > > > > > Regards,
> > > > > > > > > > > > 
> > > > > > > > > > > > Tvrtko
> > > > > > > > > > > 
> > > > > > > > > > > Hey Tvrtko,
> > > > > > > > > > > 
> > > > > > > > > > > Thanks for summarizing a bunch of discussions.
> > > > > > > > > > > Essentially I agree with every you wrote above.
> > > > > > > > > > > 
> > > > > > > > > > > If we have a global setting (determined by the OR
> > > > > > > > > > > policy), what's
> > > 
> > > the
> > > > > > > > > > > point of per context settings?
> > > > > > > > > > > 
> > > > > > > > > > > In Dmitry's scenario, all userspace applications
> > > > > > > > > > > will work
> > > > > > > > > > > together to
> > > > > > > > > > > reach the consensus so it sounds like we're
> > > > > > > > > > > reimplementing the
> > > 
> > > policy
> > > > > > > > > > > that is already existing in userspace.
> > > > > > > > > > > 
> > > > > > > > > > > Anyway, I'm implementing Joonas' suggestion.
> > > > > > > > > > > Hopefully
> > > 
> > > somebody else
> > > > > > > > > > > than me pick one or the other :)
> > > > > > > > > > 
> > > > > > > > > > I'll just mention the voting/consensus approach to
> > > > > > > > > > see if anyone
> > > 
> > > else
> > > > > > > > > > likes it.
> > > > > > > > > > 
> > > > > > > > > > Each context has a CONTEXT_PARAM_HINT_SSEU { small,
> > > 
> > > dontcare, large }
> > > > > > > > > > (or some other abstract names).
> > > > > > > > > 
> > > > > > > > > Yeah, the param name should have the word _HINT_ in
> > > > > > > > > it when it's
> > > 
> > > not a
> > > > > > > > > definitive set.
> > > > > > > > > 
> > > > > > > > > There's no global setter across containers, only a
> > > > > > > > > scenario when
> > > > > > > > > everyone agrees or not. Tallying up the votes and
> > > > > > > > > going with a
> > > 
> > > majority
> > > > > > > > > vote might be an option, too.
> > > > > > > > > 
> > > > > > > > > Regards, Joonas
> > > > > > > > 
> > > > > > > > Trying to test the "everyone agrees" approach here.
> > > > > > > 
> > > > > > > It's not everyone agrees, but the greater good.
> > > > > > 
> > > > > > I'm looking forward to the definition of the greater good
> > > > > > :)
> > > > > > Tvrtko wanted to avoid the heuristic territory, it seems
> > > > > > like we're just
> > > > > > stepping into it.
> > > > > 
> > > > > I am not sure that "small, dontcare, large" models brings
> > > > > much. No one
> > > > > would probably set "dontcare" since we have to default new
> > > > > contexts
> > 
> > to
> > > > > large to be compatible.
> > > > > 
> > > > > Don't know, I still prefer the master knob option. Honestly
> > > > > don't yet
> > > > > see the containers use case as a problem. There is always a
> > > > > master
> > > > > domain in any system where the knob can be enabled if the
> > > > > customers
> > 
> > on
> > > > > the system are such to warrant it. On mixed systems enable it
> > > > > or not
> > > > > someone always suffers. And with the knob we are free to add
> > 
> > heuristics
> > > > > later, keep the uapi, and just default the knob to on.
> > > > 
> > > > Master knob effectively means dead code behind a switch, that's
> > > > not
> > 
> > very
> > > > upstream friendly.
> > > 
> > > Hey at least I wasn't proposing a modparam! :)))
> > > 
> > > Yes it is not the best software practice, upstream or not,
> > > however I am
> > > trying to be pragmatical here and choose the simplest, smallest,
> > > good
> > > enough, and least headache inducing in the future solution.
> > > 
> > > One way of of looking at the master switch could be like tune
> > > your
> > > system for XYZ - change CPU frequency governor, disable SATA link
> > > saving, allow i915 media optimized mode. Some live code out, some
> > > dead
> > > code in.
> > > 
> > > But perhaps discussion is moot since we don't have userspace
> > > anyway.
> > > 
> > > Regards,
> > > 
> > > Tvrtko
> > 
> > I'm surprised by the "no deadcode behind knobs comment". We do this
> > all
> > the time - "display=0" anyone? Or "enable_cmdparser=false"?
> > 
> > Allowing user space to reduce EU performance for the system as a
> > whole
> > is not a great idea imho. Only sysadmin should have the right to
> > let that
> > happen, and an admin switch (I WOULD go with a modparam personally)
> > is
> > a good way to ensure that we can get the optimal media
> > configuration for
> > dedicated media systems, while for general systems we get the best
> > overall
> > performance (not just favouring media).
> > 
> > Why would we want to over engineer this?
> > 
> > Jon
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx