[Intel-gfx] [PATCH 1/1] drm/i915: Export LMEM max memory bandwidth via sysfs.

[Ghimiray, Himal Prasad]

> -----Original Message-----
> From: Ghimiray, Himal Prasad
> Sent: 16 November 2022 16:18
> To: Gupta, Anshuman <Anshuman.Gupta at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: Iddamsetty, Aravind <Aravind.Iddamsetty at intel.com>
> Subject: RE: [Intel-gfx] [PATCH 1/1] drm/i915: Export LMEM max memory
> bandwidth via sysfs.
> 
> 
> 
> > -----Original Message-----
> > From: Gupta, Anshuman <anshuman.gupta at intel.com>
> > Sent: 16 November 2022 15:38
> > To: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>; intel-
> > gfx at lists.freedesktop.org
> > Cc: Iddamsetty, Aravind <aravind.iddamsetty at intel.com>
> > Subject: RE: [Intel-gfx] [PATCH 1/1] drm/i915: Export LMEM max memory
> > bandwidth via sysfs.
> >
> >
> >
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf
> > > Of Himal Prasad Ghimiray
> > > Sent: Tuesday, November 15, 2022 1:39 PM
> > > To: intel-gfx at lists.freedesktop.org
> > > Subject: [Intel-gfx] [PATCH 1/1] drm/i915: Export LMEM max memory
> > > bandwidth via sysfs.
> > >
> > > Export lmem maximum memory bandwidth to the userspace via sysfs.
> > >
> > > Signed-off-by: Himal Prasad Ghimiray
> > > <himal.prasad.ghimiray at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h   |  2 ++
> > >  drivers/gpu/drm/i915/i915_sysfs.c | 27 +++++++++++++++++++++++++++
> > >  2 files changed, 29 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index
> c4921c9a60770..3ba1efa995ca9
> > > 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6603,6 +6603,8 @@
> > >  #define	    POWER_SETUP_I1_WATTS		REG_BIT(31)
> > >  #define	    POWER_SETUP_I1_SHIFT		6	/* 10.6 fixed
> > > point format */
> > >  #define	    POWER_SETUP_I1_DATA_MASK
> > > 	REG_GENMASK(15, 0)
> > > +#define	  PCODE_MEMORY_CONFIG			0x70
> > Please re-arrange the macros in  increasing order of pcode command.
> > > +#define
> > > MEMORY_CONFIG_SUBCOMMAND_READ_MAX_BANDWIDTH 0x0
> > >  #define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US	0x23
> > >  #define   XEHP_PCODE_FREQUENCY_CONFIG		0x6e	/*
> > xehpsdv,
> > > pvc */
> > >  /* XEHP_PCODE_FREQUENCY_CONFIG sub-commands (param1) */ diff --
> > git
> > > a/drivers/gpu/drm/i915/i915_sysfs.c
> > > b/drivers/gpu/drm/i915/i915_sysfs.c
> > > index 595e8b5749907..0a6efc300998b 100644
> > > --- a/drivers/gpu/drm/i915/i915_sysfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> > > @@ -37,7 +37,10 @@
> > >
> > >  #include "i915_drv.h"
> > >  #include "i915_sysfs.h"
> > > +#include "i915_reg.h"
> > >  #include "intel_pm.h"
> > > +#include "intel_pcode.h"
> > > +
> > >
> > >  struct drm_i915_private *kdev_minor_to_i915(struct device *kdev)  {
> > > @@ -
> > > 231,11 +234,35 @@ static void i915_setup_error_capture(struct device
> > > *kdev) {}  static void i915_teardown_error_capture(struct device
> > > *kdev) {} #endif
> > >
> > > +static ssize_t
> > > +prelim_lmem_max_bw_Mbps_show(struct device *dev, struct
> > Please don't use mixed case here,
> > How about i915_lmem_max_bw_mbps_show ?
> [Ghimiray, Himal Prasad]
> We need to differentiate between Mb (Mega bit) vs MB(MegaByte) ,Hence I
> used camelCase.
> Using i915_lmem_max_bw_mbps_show will not give clarity whether the
> value is in Mb or MB.
> 
> BR
> Himal Ghimiray
> > > +device_attribute *attr, char *buff) {
> > > +	struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
> > > +	u32 val;
> > > +	int err;
> > > +
> > > +	err = snb_pcode_read_p(&i915->uncore,
> > > PCODE_MEMORY_CONFIG,
> > > +
> > > MEMORY_CONFIG_SUBCOMMAND_READ_MAX_BANDWIDTH,
> > > +			       0x0, &val);
> > > +	if (err)
> > > +		return err;
> > > +
> > > +	return sysfs_emit(buff, "%u\n", val); }
> > > +
> > > +static DEVICE_ATTR_RO(prelim_lmem_max_bw_Mbps);
> > > +
> > >  void i915_setup_sysfs(struct drm_i915_private *dev_priv)  {
> > >  	struct device *kdev = dev_priv->drm.primary->kdev;
> > >  	int ret;
> > >
> > > +	if (IS_DG1(dev_priv) || IS_DG2(dev_priv)) {
> > This seems to discrete agnostic.
> > How about HAS_LMEM ?
[Ghimiray, Himal Prasad] 
The changes are not applicable to all the discrete platforms. Example PVC. 
> > > +		ret = sysfs_create_file(&kdev->kobj,
> > > &dev_attr_prelim_lmem_max_bw_Mbps.attr);
> >
> > > +		if (ret)
> > > +			drm_err(&dev_priv->drm, "Setting up sysfs to read
> > > max B/W failed\n");
> > Why this sys fs is outside gt directory ?
> > Thanks,
> > Anshuman.
[Ghimiray, Himal Prasad] 
Had put up the query in https://jira.devtools.intel.com/browse/VLK-39793 before implementation.
Query: sysfs entry should be associated with gt or  the card ?
As per answer: Individual tiles can't currently have a different frequency so would be talking about card level entry for now, next to the other frequency items.

> > > +	}
> > > +
> > >  	if (HAS_L3_DPF(dev_priv)) {
> > >  		ret = device_create_bin_file(kdev, &dpf_attrs);
> > >  		if (ret)
> > > --
> > > 2.25.1