Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Tvrtko Ursulin <tvrtko.ursulin@linux.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: [PATCH v6 0/5] drm/i915: Expose more GPU properties through sysfs
Date: Mon, 11 Dec 2017 13:29:42 +0000	[thread overview]
Message-ID: <f3f38b3e-687e-0546-b6ae-65f43e2586bf@intel.com> (raw)
In-Reply-To: <1512989417.5315.38.camel@linux.intel.com>

On 11/12/17 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.

We'll be working with topology from the render FD too.
This last series added the ability to find the card from the fd's 
major/minor just for that case.

>
> 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.

If we're going with just topology through sysfs, what should the layout 
look like?
card/gt/rcs/topology?

>
> Then we can continue on about the engine discovery in parallel, not
> blocking GPU topology discovery.
>
> Regards, Joonas
>
>> Regards,
>>
>> Tvrtko
>>
>>> Cheers,
>>>
>>> Lionel Landwerlin (5):
>>>     drm: add card symlink in render sysfs directory
>>>     drm/i915: store all subslice masks
>>>     drm/i915/debugfs: reuse max slice/subslices already stored in sseu
>>>     drm/i915: expose engine availability through sysfs
>>>     drm/i915: expose EU topology through sysfs
>>>
>>>    drivers/gpu/drm/drm_drv.c                |  11 +
>>>    drivers/gpu/drm/i915/i915_debugfs.c      |  50 ++--
>>>    drivers/gpu/drm/i915/i915_drv.c          |   2 +-
>>>    drivers/gpu/drm/i915/i915_drv.h          |  56 ++++-
>>>    drivers/gpu/drm/i915/i915_sysfs.c        | 386 +++++++++++++++++++++++++++++++
>>>    drivers/gpu/drm/i915/intel_device_info.c | 169 ++++++++++----
>>>    drivers/gpu/drm/i915/intel_engine_cs.c   |  12 +
>>>    drivers/gpu/drm/i915/intel_lrc.c         |   2 +-
>>>    drivers/gpu/drm/i915/intel_ringbuffer.h  |   6 +-
>>>    9 files changed, 617 insertions(+), 77 deletions(-)
>>>
>>> --
>>> 2.15.1
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2017-12-11 13:29 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 [this message]
2017-12-11 14:38     ` Tvrtko Ursulin
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=f3f38b3e-687e-0546-b6ae-65f43e2586bf@intel.com \
    --to=lionel.g.landwerlin@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=tvrtko.ursulin@linux.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