[Intel-xe] [PATCH 0/3] drm/xe: Add sysfs entry to report per gt memory size

Tue Jun 6 21:46:11 UTC 2023

On Mon, Jun 05, 2023 at 06:38:22AM -0700, Upadhyay, Tejas wrote:
> 
> 
> > -----Original Message-----
> > From: Roper, Matthew D <matthew.d.roper at intel.com>
> > Sent: Friday, June 2, 2023 8:48 PM
> > To: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> > Cc: Iddamsetty, Aravind <aravind.iddamsetty at intel.com>; intel-
> > xe at lists.freedesktop.org; Vivi, Rodrigo <rodrigo.vivi at intel.com>; Dixit,
> > Ashutosh <ashutosh.dixit at intel.com>
> > Subject: Re: [PATCH 0/3] drm/xe: Add sysfs entry to report per gt memory
> > size
> > 
> > On Thu, Jun 01, 2023 at 11:51:51PM -0700, Upadhyay, Tejas wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Iddamsetty, Aravind <aravind.iddamsetty at intel.com>
> > > > Sent: Tuesday, May 30, 2023 12:22 PM
> > > > To: Upadhyay, Tejas <tejas.upadhyay at intel.com>; intel-
> > > > xe at lists.freedesktop.org; Roper, Matthew D
> > > > <matthew.d.roper at intel.com>
> > > > Cc: Vivi, Rodrigo <rodrigo.vivi at intel.com>; Dixit, Ashutosh
> > > > <ashutosh.dixit at intel.com>
> > > > Subject: RE: [PATCH 0/3] drm/xe: Add sysfs entry to report per gt
> > > > memory size
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Iddamsetty, Aravind
> > > > > Sent: Monday, May 29, 2023 2:15 PM
> > > > > To: Upadhyay, Tejas <tejas.upadhyay at intel.com>; intel-
> > > > > xe at lists.freedesktop.org
> > > > > Cc: Vivi, Rodrigo <rodrigo.vivi at intel.com>; Dixit, Ashutosh
> > > > > <ashutosh.dixit at intel.com>; Roper, Matthew D
> > > > > <matthew.d.roper at intel.com>
> > > > > Subject: RE: [PATCH 0/3] drm/xe: Add sysfs entry to report per gt
> > > > > memory size
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> > > > > > Sent: Monday, May 29, 2023 2:10 PM
> > > > > > To: intel-xe at lists.freedesktop.org
> > > > > > Cc: Iddamsetty, Aravind <aravind.iddamsetty at intel.com>; Vivi,
> > > > > > Rodrigo <rodrigo.vivi at intel.com>; Dixit, Ashutosh
> > > > > > <ashutosh.dixit at intel.com>; Roper, Matthew D
> > > > > > <matthew.d.roper at intel.com>
> > > > > > Subject: RE: [PATCH 0/3] drm/xe: Add sysfs entry to report per
> > > > > > gt memory size
> > > > > >
> > > > > > Initiating discussion on sysfs path for this inline below :
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> > > > > > > Sent: Thursday, May 4, 2023 4:50 PM
> > > > > > > To: intel-xe at lists.freedesktop.org
> > > > > > > Cc: Upadhyay, Tejas <tejas.upadhyay at intel.com>; Iddamsetty,
> > > > > > > Aravind <aravind.iddamsetty at intel.com>
> > > > > > > Subject: [PATCH 0/3] drm/xe: Add sysfs entry to report per gt
> > > > > > > memory size
> > > > > > >
> > > > > > > Add sysfs entry to read per gt physical memory in total
> > > > > > > including stolen memory.
> > > > > > >
> > > > > > > The sysfs entry added can be read using:
> > > > > > > cat /sys/class/drm/card1/gt/gt0/addr_range
> > > > > > > cat /sys/class/drm/card1/gt/gt1/addr_range
> > > > > >
> > > > > > We already have gt#num directory created under device folder,
> > > > > > should we create addr_range under that only. I think that would
> > > > > > be straight forward to start with.  For example,
> > > > > >
> > > > > >
> > > >
> > /devices/pci0000:a7/0000:a7:01.0/0000:a8:00.0/0000:a9:01.0/0000:aa:00.
> > > > > > 0/
> > > > > > drm/card1/gt0/addr_range
> > > > > >
> > > > > > Initiating discussion there to conclude on path on this sysfs
> > > > > > entry as well as scheduler interface entries as well as under "engine"
> > > > > > folder under
> > > > > card1.
> > > >
> > > > Also, now with Matt's " Separate GT and tile" series addr_range is
> > > > associated to a tile rather than gt.  We might have to introduce
> > > > "tile" in sysfs
> > >
> > > Hi @Roper, Matthew D, @Vivi, Rodrigo,
> > >
> > > Should we expose tile0/1->addr_range to user, with sysfs like
> > device/tile0/gt0, device/tile1/gt1? Looking for inputs here to move forward
> > with second version of this patch.
> > 
> > Since memory is a tile-specific concept and isn't related to GTs, I'd imagine
> > you probably want something like
> > 
> >         /sys/class/drm/cardX/tileY/addr_range
> > 
> > Since this is sysfs rather than debugfs, that means we have to follow all the
> > strict uapi requirements, including having an opensource userspace
> > consumer and preserving backward compatibility forever.  Which userspace
> > component plans to use this?  It's unclear to me how this is going to get used,
> > but you should make sure a single general addr_range is actually what they
> > want, rather than a more general interface that can report details about
> > multiple ranges of memory per tile (e.g., stolen memory, generally available
> > vram, etc.).
> > 
> 
> Also, we have gt0 and gt1 sysfs dirs created under device folder as
> device/gt0 and device/gt1 right, do we need to establish relation
> between new tile dir (/sys/class/drm/cardX/tileY/addr_range)  and
> gt0/gt1 dir which we already have in xe? Or we should keep them
> independent!

Someone needs to make a policy decision about exactly what we want the
GT vs tile uapi to look like for the Xe driver.  Since Xe isn't part of
upstream yet, we still have time to change the uapi before it's all set
in stone; we just need to make sure that the kernel driver and userspace
developers are all on the same page.

In my mind, it would make sense to move the GT directories underneath
the tile that contains them rather than having them directly under the
device.  But if/when we do that there are still decisions that need to
be made about whether to use device-wide IDs like:

        /tile0/gt0
        /tile0/gt1
        /tile1/gt2
        /tile2/gt3

or whether to use per-tile IDs like:

        /tile0/gt0
        /tile0/gt1
        /tile1/gt0
        /tile2/gt0

or whether to use symbolic names instead of IDS:

        /tile0/gt-primary
        /tile0/gt-media
        /tile1/gt-primary
        /tile2/gt-primary

Those kind of decisions are something that need to be discussed with our
userspace partners so that a direction can be set.

Matt

> 
> Thanks,
> Tejas
> > 
> > Matt
> > 
> > >
> > > Thanks,
> > > Tejas
> > > >
> > > > Thanks,
> > > > Aravind.
> > > >
> > > > >
> > > > > Scheduler/engine entries are not related to this feature let's
> > > > > discuss them separately.
> > > > >
> > > > > Thanks,
> > > > > Aravind.
> > > > >
> > > > > >
> > > > > > Please provide your inputs.
> > > > > >
> > > > > > Thanks,
> > > > > > Tejas
> > > > > > >
> > > > > > > As it involves drm_class, it required movement in xe_device
> > > > > > > init flow, patch for same is part of this series.
> > > > > > >
> > > > > > > Actual physical memory per GT was dependent on infra patch
> > > > > > > from Ruhl, Michael J, so I have added it as part of this
> > > > > > > series to pass through pre-
> > > > > > merge.
> > > > > > > Patchwork:
> > > > > > > https://patchwork.freedesktop.org/series/117126/#rev3
> > > > > > >
> > > > > > > Cc: Aravind Iddamsetty <aravind.iddamsetty at intel.com>
> > > > > > > Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> > > > > > >
> > > > > > > Tejas Upadhyay (3):
> > > > > > >   drm/xe: Rework multi tile device memory initialization
> > > > > > >   drm/xe: Register drm device prior to gt init
> > > > > > >   drm/xe: Add sysfs entry to report per gt memory size
> > > > > > >
> > > > > > >  drivers/gpu/drm/xe/regs/xe_gt_regs.h   |   2 +-
> > > > > > >  drivers/gpu/drm/xe/xe_bo.c             |   8 +-
> > > > > > >  drivers/gpu/drm/xe/xe_bo.h             |   2 +-
> > > > > > >  drivers/gpu/drm/xe/xe_device.c         |   8 +-
> > > > > > >  drivers/gpu/drm/xe/xe_device_types.h   |  20 +-
> > > > > > >  drivers/gpu/drm/xe/xe_gt_sysfs.c       |  74 ++++++-
> > > > > > >  drivers/gpu/drm/xe/xe_gt_sysfs.h       |   1 -
> > > > > > >  drivers/gpu/drm/xe/xe_gt_types.h       |  21 +-
> > > > > > >  drivers/gpu/drm/xe/xe_migrate.c        |   6 +-
> > > > > > >  drivers/gpu/drm/xe/xe_mmio.c           | 279 ++++++++++++++---------
> > --
> > > > > > >  drivers/gpu/drm/xe/xe_mmio.h           |   4 +-
> > > > > > >  drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c |  30 +--
> > > > > > >  12 files changed, 291 insertions(+), 164 deletions(-)
> > > > > > >
> > > > > > > --
> > > > > > > 2.25.1
> > >
> > 
> > --
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation