Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Lionel Landwerlin <lionel.g.landwerlin@intel.com>,
	intel-gfx@lists.freedesktop.org,
	Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v6 0/5] drm/i915: Expose more GPU properties through sysfs
Date: Mon, 11 Dec 2017 14:38:53 +0000	[thread overview]
Message-ID: <708a205b-9ab1-b332-6cdf-8183a44a02fa@linux.intel.com> (raw)
In-Reply-To: <1512989417.5315.38.camel@linux.intel.com>


On 11/12/2017 10:50, Joonas Lahtinen wrote:
> + Daniel, Chris
> 
> On Thu, 2017-12-07 at 09:21 +0000, Tvrtko Ursulin wrote:
>> On 04/12/2017 15:02, Lionel Landwerlin wrote:
>>> Hi,
>>>
>>> After discussion with Chris, Joonas & Tvrtko, this series adds an
>>> additional commit to link the render node back to the card through a
>>> symlink. Making it obvious from an application using a render node to
>>> know where to get the information it needs.
>>
>> Important thing to mention as well is that it is trivial to get from the
>> master drm fd to the sysfs root, via fstat and opendir
>> /sys/dev/char/<major>:<minor>. With the addition of the card symlink to
>> render nodes it is trivial for render node fd as well.
>>
>> I am happy with this approach - it is extensible, flexible and avoids
>> issues with ioctl versioning or whatnot. With one value per file it is
>> trivial for userspace to access.
>>
>> So for what I'm concerned, given how gputop would use all of this and so
>> be the userspace, if everyone else is happy, I think we could do a
>> detailed review and prehaps also think about including gputop in some
>> distribution to make the case 100% straightforward.
> 
> For the GPU topology I agree this is the right choice, it's going to be
> about topology after all, and directory tree is the perfect candidate.
> And if a new platform appears, then it's a new platform and may change
> the topology well the hardware topology has changed.
> 
> For the engine enumeration, I'm not equally sold for sysfs exposing it.
> It's a "linear list of engine instances with flags" how the userspace
> is going to be looking at them. And it's also information about what to
> pass to an IOCTL as arguments after decision has been made, and then
> you already have the FD you know you'll be dealing with, at hand. So
> another IOCTL for that seems more convenient.

Apart from more flexibility and easier to extend, sysfs might be a 
better fit for applications which do not otherwise need a drm fd. Say a 
top-like tool which shows engine utilization, or those patches I RFC-ed 
recently which do the same but per DRM client.

Okay, these stats are now available also via PMU so the argument is not 
the strongest I admit, but I still find it quite neat. It also might 
allow us to define our own policy with regards to needed privilege to 
access these stats, and not be governed by the perf API rules.

> So I'd say for the GPU topology part, we go forward with the review and
> make sure we don't expose driver internal bits that could break when
> refactoring code. If the exposed N bits of information are strictly
> tied to the underlying hardware, we should have no trouble maintaining
> that for the foreseeable future.
> 
> Then we can continue on about the engine discovery in parallel, not
> blocking GPU topology discovery.

I can live with that, but would like to keep the gt/engines/ namespace 
reserved for the eventuality with go with engine info in sysfs at a 
later stage then.

Also, Lionel, did you have plans to use the engine info straight away in 
gpu top, or you only needed topology? I think you were drawing a nice 
block diagram of a GPU so do you need it for that?

Regards,

Tvrtko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2017-12-11 14:38 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-04 15:02 [PATCH v6 0/5] drm/i915: Expose more GPU properties through sysfs Lionel Landwerlin
2017-12-04 15:02 ` [PATCH v6 1/5] drm: add card symlink in render sysfs directory Lionel Landwerlin
2017-12-04 15:02 ` [PATCH v6 2/5] drm/i915: store all subslice masks Lionel Landwerlin
2017-12-04 15:02 ` [PATCH v6 3/5] drm/i915/debugfs: reuse max slice/subslices already stored in sseu Lionel Landwerlin
2017-12-04 15:02 ` [PATCH v6 4/5] drm/i915: expose engine availability through sysfs Lionel Landwerlin
2017-12-04 15:02 ` [PATCH v6 5/5] drm/i915: expose EU topology " Lionel Landwerlin
2017-12-04 16:02 ` ✓ Fi.CI.BAT: success for drm/i915: Expose more GPU properties through sysfs (rev6) Patchwork
2017-12-04 18:48 ` ✗ Fi.CI.IGT: failure " Patchwork
2017-12-07  9:21 ` [Intel-gfx] [PATCH v6 0/5] drm/i915: Expose more GPU properties through sysfs Tvrtko Ursulin
2017-12-11 10:50   ` Joonas Lahtinen
2017-12-11 13:29     ` Lionel Landwerlin
2017-12-11 14:38     ` Tvrtko Ursulin [this message]
2017-12-11 14:47       ` Lionel Landwerlin
2017-12-11 21:05       ` [Intel-gfx] " Daniel Vetter
2017-12-12 11:19         ` Tvrtko Ursulin
2017-12-12 14:33           ` Lionel Landwerlin
2017-12-13  8:17             ` Daniel Vetter
2017-12-13 13:35               ` Chris Wilson
2017-12-13 15:09                 ` Lionel Landwerlin
2017-12-13 15:06               ` Lionel Landwerlin
2017-12-12 15:18           ` Daniel Vetter

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=708a205b-9ab1-b332-6cdf-8183a44a02fa@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=lionel.g.landwerlin@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