Re: [Intel-gfx] [PATCH 2/2] drm/i915/tracepoints: Remove DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option

[Kukanova, Svetlana]

OK, so no end-user queries matter, just the queries from the tools for 
end-users do, right?

And with making the open-source tool (shipped by distros, etc.) suitable for 
negotiations we need to hurry while at least the trace-point mechanism is not 
yet completely broken and can be used to show usefulness and to have at least 
something that can be taken to distro?

If the new tool and kernel changes required by it are developed in parallel - 
you don't have that "shipped by a distro" condition, BTW, right? Or in case of 
parallel discussion you're deciding if the suggested tool  has rights to exist?

-Original Message-
From: Joonas Lahtinen [mailto:joonas.lahti...@linux.intel.com] 
Sent: Wednesday, August 29, 2018 5:52 PM
To: Intel-gfx@lists.freedesktop.org; Kukanova, Svetlana 
; Chris Wilson ; Tvrtko 
Ursulin ; Tvrtko Ursulin 
Subject: RE: [Intel-gfx] [PATCH 2/2] drm/i915/tracepoints: Remove 
DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option

Quoting Kukanova, Svetlana (2018-08-27 16:37:14)
> > Once there is an actual request to have some metrics from vanilla kernels
> > through some end-user tools (not a developer tool, like here), I'll be glad
> > to discuss about how to provide the information the best for them in a
> > stable manner.
> 
> Sorry for my ignorance, but looks like I don't understand what developer vs.
> end-user means here.
> With regard to GPU profiling VTune's end-user is somebody who develops gfx or
> media applications basing on MediaSDK, OpenCL, C for Media, etc.
> Or, more often it's an intel application engineer working with those people's
> code.
> AE in his\her turn may contact e.g. Dmitry's team if judging by VTune data
> he\she decides that the problem is on the deeper level of the gfx stack, not
> in the customer's code.
> Then Dmitry's team would be experimenting with VTune and deciding if the
> problem is in their code or it's deeper in i915.
> Don't think that i915 people use VTune (sadly:)) so here the chain is broken.
> Otherwise they could e.g. blame HW based on the same data.
> I'm wondering who in this chain (app developer, AE, Dmitry, i915) is an
> "end-user" and who's a "developer"?
> Or is a "developer" a kernel developer only?
> And e.g. Dmitry is an end-user and thus he is not supposed to use tools like
> gpuvis or VTune?
> Looks like all the chain before i915 is annoyed by the kernel-rebuilding
> requirement.

With end-user tool I'm referring to something that would have interest
in being packaged and shipped by a distro.

gpuvis team seems to be doing fine with the application being built from source
and being run against a specially configured kernel for their purposes. I would
assume there to be some queries about a enabling the tracepoints by default if
there was demand. At the same time I would assume them to try to get the
application packaged and into distros.

And then we would commence discussing how to provide the information in a stable
manner (most likely outside tracepoints). So far I'm not seeing such queries
from gpuvis direction.

> > The interface discussion would probably start from a DRM subsystem level, so
> > that the tool would have an equivalent level of base experience from all
> > drivers.
>
> That sounds like a solution from an ideal world. I mean if DRM had a uAPI for
> scheduling observability and all the drivers had to implement this. And the
> drivers would require info from HW like GuC pointing to the necessity of uAPI
> support...
> Would be just great for all the tools (, developers and end-users).
> But I have no idea what kind of impulse should it be to bring this to reality.
> And if all the energy available to human kind at the given evolution point
> would be enough to at least start this. 
> Or am I just too pessimistic? Are there some simple defined steps to be done
> to make it? Can we build a realistic plan?

Step is "1. Have the tool" :) There seem to be three options: 1) open sourcing
VTune 2) contributing to gpuvis project to drive the project into the above
mentioned direction. 3) writing a new project from scratch (not encouraged,
unless you have something differentiating to bring to the table).

Unless somebody actively drives the feature to some Open Source userspace
consumer, there won't be an interface for the information from kernel.
Demand from an Open Source application is a hard requirement for kickstarting
the interface discussion.

> E.g. is this the first step? -
> > There's just no Open Source tool to first design and then validate the
> > interfaces against. There's just the debugging tool which happens to work
> > currently, without any guarantees that next kernel version would not cause a
> > substantial rework of the interfacing code.
> 
> How does it usually work, I mean you 

Re: [Intel-gfx] [PATCH 2/2] drm/i915/tracepoints: Remove DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option

[Kukanova, Svetlana]

> Once there is an actual request to have some metrics from vanilla kernels 
> through some end-user tools (not a developer tool, like here), I'll be glad 
> to discuss about how to provide the information the best for them in a stable 
> manner.

Sorry for my ignorance, but looks like I don't understand what developer vs. 
end-user means here.
With regard to GPU profiling VTune's end-user is somebody who develops gfx or 
media applications basing on MediaSDK, OpenCL, C for Media, etc.
Or, more often it's an intel application engineer working with those people's 
code. 
AE in his\her turn may contact e.g. Dmitry's team if judging by VTune data 
he\she decides that the problem is on the deeper level of the gfx stack, not in 
the customer's code.
Then Dmitry's team would be experimenting with VTune and deciding if the 
problem is in their code or it's deeper in i915.
Don't think that i915 people use VTune (sadly:)) so here the chain is broken. 
Otherwise they could e.g. blame HW based on the same data.
I'm wondering who in this chain (app developer, AE, Dmitry, i915) is an 
"end-user" and who's a "developer"? 
Or is a "developer" a kernel developer only? 
And e.g. Dmitry is an end-user and thus he is not supposed to use tools like 
gpuvis or VTune?
Looks like all the chain before i915 is annoyed by the kernel-rebuilding 
requirement.

>The interface discussion would probably start from a DRM subsystem level, so 
>that the tool would have an equivalent level of base experience from all 
>drivers.

That sounds like a solution from an ideal world. I mean if DRM had a uAPI for 
scheduling observability and all the drivers had to implement this. And the 
drivers would require info from HW like GuC pointing to the necessity of uAPI 
support... 
Would be just great for all the tools (, developers and end-users). 
But I have no idea what kind of impulse should it be to bring this to reality. 
And if all the energy available to human kind at the given evolution point 
would be enough to at least start this. 
Or am I just too pessimistic? Are there some simple defined steps to be done to 
make it? Can we build a realistic plan?

E.g. is this the first step? -
> There's just no Open Source tool to first design and then validate the 
> interfaces against. There's just the debugging tool which happens to work 
> currently, without any guarantees that next kernel version would not cause a 
> substantial rework of the interfacing code.

How does it usually work, I mean you can't have a widely shipped open-source 
consumer already using a non-existent feature that is to be requested? 
And I can't imagine what kind of existing tool should it be to decide suddenly 
that it needs to add GPU scheduling tracing to the list of its features.
If you want to have a new tool for GPU scheduling timeline - and it sounds like 
a sane idea, looks like we agree on the use cases etc. - how can you make it 
open source first and then get the API to be based on from i915?
Or am I just missing the point completely?
If the open-sourced MediaSDK was shipped with some distro (isn't it, btw?) - 
would Dmitry be eligible to request observability features for tools?

Thank you,
Svetlana

-Original Message-
From: Joonas Lahtinen [mailto:joonas.lahti...@linux.intel.com] 
Sent: Tuesday, August 21, 2018 3:07 PM
To: Intel-gfx@lists.freedesktop.org; Kukanova, Svetlana 
; Chris Wilson ; Tvrtko 
Ursulin ; Tvrtko Ursulin 
Subject: RE: [Intel-gfx] [PATCH 2/2] drm/i915/tracepoints: Remove 
DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option

Quoting Kukanova, Svetlana (2018-08-13 16:44:49)
> Joonas, sorry for interfering; could you please explain more regarding 
> the options for tracing scheduling events better than tracepoints?
> After scheduling moves to GuC tools will have to switch to something 
> like GuC-logging; but while kmd does scheduling isn't kernel-tracing the best 
> solution?
> I know gpuvis is not the only attempt to use tracepoints for the same purpose.
> (there're trace.pl and S.E.A. and of course VTune though it probably 
> is not considered to be existing as it's not open source).
> And assuming this movement towards GuC is it not too late to invent a 
> completely new way to provide tools with scheduling info from kmd?
> Could we just improve the existing way and let it live its last years\months? 

Hi,

You actually mentioned the prime reason why we should not go and hastily make 
tracepoints a stable uAPI with regards to scheduling information.

The scheduler's nature will be evolving when some of the scheduling decisions 
are moved to GuC and the way how we get the information will be changing at 
that point, so tracepoints will indeed be a very bad mechanism for providing 
the information.

The kernel scheduler is definitely not going anywhere with the introduction of 
more hardware scheduling capabilities, so it is a misconce

Re: [Intel-gfx] [PATCH 2/2] drm/i915/tracepoints: Remove DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option

[Kukanova, Svetlana]

Joonas, sorry for interfering; could you please explain more regarding the 
options for tracing scheduling events better than tracepoints?
After scheduling moves to GuC tools will have to switch to something like 
GuC-logging; but while kmd does scheduling isn't kernel-tracing the best 
solution?
I know gpuvis is not the only attempt to use tracepoints for the same purpose. 
(there're trace.pl and S.E.A. and of course VTune though it probably is not 
considered to be existing as it's not open source). 
And assuming this movement towards GuC is it not too late to invent a 
completely new way to provide tools with scheduling info from kmd? 
Could we just improve the existing way and let it live its last years\months? 

gpuvis works w\o modifying kernel for AMDgpu showing HW queue and HW execution; 
it cosplays Microsoft GPUView which works out-of-the-box on Windows too.
Thus it appears that intel gfx on linux is the most closed platform, not 
bothering of observability (or even bothering about how to forbid 
observability).

Not long ago the MediaSDK team diagnosed a problem with their workloads looking 
at VTune timelines - seeing the difference between the time request came to kmd 
and time it went runnable & comparing the queues on 2 engines they understood 
that their requests have dependencies that were definitely unexpected. MediaSDK 
reported the problem to driver people and it was fixed.

I can add Dmitry Rogozhkin to discussion if the usefulness of scheduling 
timeline in tools is questionable, as far as I remember this wasn't the only 
use case they had, I'm sure he can add more.

Thank you,
Svetlana

-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Joonas Lahtinen
Sent: Monday, August 13, 2018 12:55 PM
To: Chris Wilson ; Intel-gfx@lists.freedesktop.org; 
Tvrtko Ursulin ; Tvrtko Ursulin 

Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/tracepoints: Remove 
DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option

Quoting Tvrtko Ursulin (2018-08-08 15:56:01)
> On 08/08/2018 13:42, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-08-08 13:13:08)
> This is true, no disagreement. My point simply was that we can provide 
> this info easily to anyone. There is a little bit of analogy with perf 
> scheduler tracing/map etc.
> 
> > I don't see any value in giving the information away, just the cost. 
> > If you can convince Joonas of its merit, and if we can define just 
> > exactly what ABI it constitutes, then I'd be happy to be the one who 
> > says "I told you so" in the future for a change.
> 
> I think Joonas was okay in principle that we soft-commit to _trying_ 
> to keep _some_ tracepoint stable-ish (where it makes sense and after 
> some discussion for each) if IGT also materializes which auto-pings us 
> (via
> CI) when we break one of them. But I may be misremembering so Joonas 
> please comment.

Currently gpuvis, using these, seems to be only packaged in one AUR repo, and 
they do make a not in the wiki how you need to configure kernel for debugging. 
And there's been no apparent demand for them to have it in stock kernel.

And even when we do get demand for having gpuvis or another tool working from 
vanilla kernel, tracepoints being a rather tricky subject, I would start the 
discussion by going through alternative means of providing the information the 
tool needs and considering those.

So lets still keep this option as it was introduced. The whole "tracepoints as 
stable uAPI" idea is a can of worms which is only dug into when other options 
are exhausted.

Regards, Joonas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park,
17 Krylatskaya Str., Bldg 4, Moscow 121614,
Russian Federation

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Decouple execbuf uAPI from internal implementation

[Kukanova, Svetlana]

Hi everyone,

Yes, this breaks userspace ABI and in particular it broke VTune work. 
Ring ids are seen via i915 tracepoints, and VTune Amplifier uses them.
We were relying on the old ring ids, and assuming that the new rings would be 
added to the end of the enum.

I'm objecting just now because now this driver change reached our internal 
users and they complained that VTune is reporting DMA packets on wrong engines.

I would request this change (the enum intel_ring_id change) to be rolled back. 
Hope, it's still possible.

Regards,
Svetlana

-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Chris Wilson
Sent: Friday, January 15, 2016 1:09 AM
To: Tvrtko Ursulin 
Cc: Daniel Vetter ; Intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Decouple execbuf uAPI from internal 
implementation

On Thu, Jan 14, 2016 at 03:02:59PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> At the moment execbuf ring selection is fully coupled to internal ring 
> ids which is not a good thing on its own.
> 
> This dependency is also spread between two source files and not 
> spelled out at either side which makes it hidden and fragile.
> 
> This patch decouples this dependency by introducing an explicit 
> translation table of execbuf uAPI to ring id close to the only call 
> site (i915_gem_do_execbuffer).
> 
> This way we are free to change driver internal implementation details 
> without breaking userspace. All state relating to the uAPI is now 
> contained in, or next to, i915_gem_do_execbuffer.

I was hesistant at first, since "why?" isn't fully developed, but the code won 
me over.

> +#define I915_USER_RINGS (4)
> +
> +static const enum intel_ring_id user_ring_map[I915_USER_RINGS + 1] = {
> + [I915_EXEC_DEFAULT] = RCS,
> + [I915_EXEC_RENDER]  = RCS,
> + [I915_EXEC_BLT] = BCS,
> + [I915_EXEC_BSD] = VCS,
> + [I915_EXEC_VEBOX]   = VECS
> +};

I was wondering whether packing here mattered at all, but we're under a 
cacheline so highly unlikely.

>  static int
>  i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  struct drm_file *file,
> @@ -1393,6 +1393,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void 
> *data,
>   struct i915_execbuffer_params params_master; /* XXX: will be removed 
> later */
>   struct i915_execbuffer_params *params = _master;
>   const u32 ctx_id = i915_execbuffer2_get_context_id(*args);
> + unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK;
>   u32 dispatch_flags;
>   int ret;
>   bool need_relocs;
> @@ -1414,49 +1415,39 @@ i915_gem_do_execbuffer(struct drm_device *dev, void 
> *data,
>   if (args->flags & I915_EXEC_IS_PINNED)
>   dispatch_flags |= I915_DISPATCH_PINNED;
>  
> - if ((args->flags & I915_EXEC_RING_MASK) > LAST_USER_RING) {
> - DRM_DEBUG("execbuf with unknown ring: %d\n",
> -   (int)(args->flags & I915_EXEC_RING_MASK));
> + if (user_ring_id > I915_USER_RINGS) {
> + DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id);
>   return -EINVAL;
>   }
>  
> - if (((args->flags & I915_EXEC_RING_MASK) != I915_EXEC_BSD) &&
> + if ((user_ring_id != I915_EXEC_BSD) &&
>   ((args->flags & I915_EXEC_BSD_MASK) != 0)) {
>   DRM_DEBUG("execbuf with non bsd ring but with invalid "
>   "bsd dispatch flags: %d\n", (int)(args->flags));
>   return -EINVAL;
> - } 
> -
> - if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT)
> - ring = _priv->ring[RCS];
> - else if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
> - if (HAS_BSD2(dev)) {
> - int ring_id;
> -
> - switch (args->flags & I915_EXEC_BSD_MASK) {
> - case I915_EXEC_BSD_DEFAULT:
> - ring_id = gen8_dispatch_bsd_ring(dev, file);
> - ring = _priv->ring[ring_id];
> - break;
> - case I915_EXEC_BSD_RING1:
> - ring = _priv->ring[VCS];
> - break;
> - case I915_EXEC_BSD_RING2:
> - ring = _priv->ring[VCS2];
> - break;
> - default:
> - DRM_DEBUG("execbuf with unknown bsd ring: %d\n",
> -   (int)(args->flags & 
> I915_EXEC_BSD_MASK));
> - return -EINVAL;
> - }
> - } else
> - ring = _priv->ring[VCS];
> - } else
> - ring = _priv->ring[(args->flags & I915_EXEC_RING_MASK) - 1];
> + }
> +
> + if (user_ring_id