[PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage v4

2019-01-09 Thread Russell, Kent
Add a sysfs file that reports the number of bytes transmitted and
received in the last second. This can be used to approximate the PCIe
bandwidth usage over the last second.

v2: Clarify use of mps as estimation of bandwidth
v3: Don't make the file on APUs
v4: Early exit for APUs in the read function, change output to
display "packets-received packets-sent mps"
Signed-off-by: Kent Russell 

Signed-off-by: Kent Russell 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 36 
 drivers/gpu/drm/amd/amdgpu/cik.c   | 47 
 drivers/gpu/drm/amd/amdgpu/si.c| 47 
 drivers/gpu/drm/amd/amdgpu/soc15.c | 50 ++
 drivers/gpu/drm/amd/amdgpu/vi.c| 47 
 6 files changed, 231 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e1b2c64..512b124 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -542,6 +542,9 @@ struct amdgpu_asic_funcs {
bool (*need_full_reset)(struct amdgpu_device *adev);
/* initialize doorbell layout for specific asic*/
void (*init_doorbell_index)(struct amdgpu_device *adev);
+   /* PCIe bandwidth usage */
+   void (*get_pcie_usage)(struct amdgpu_device *adev, uint64_t *count0,
+  uint64_t *count1);
 };
 
 /*
@@ -1045,6 +1048,7 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
 #define amdgpu_asic_invalidate_hdp(adev, r) 
(adev)->asic_funcs->invalidate_hdp((adev), (r))
 #define amdgpu_asic_need_full_reset(adev) 
(adev)->asic_funcs->need_full_reset((adev))
 #define amdgpu_asic_init_doorbell_index(adev) 
(adev)->asic_funcs->init_doorbell_index((adev))
+#define amdgpu_asic_get_pcie_usage(adev, cnt0, cnt1) 
((adev)->asic_funcs->get_pcie_usage((adev), (cnt0), (cnt1)))
 
 /* Common functions */
 bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 6896dec..b38c06f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -990,6 +990,31 @@ static ssize_t amdgpu_get_busy_percent(struct device *dev,
return snprintf(buf, PAGE_SIZE, "%d\n", value);
 }
 
+/**
+ * DOC: pcie_bw
+ *
+ * The amdgpu driver provides a sysfs API for estimating how much data
+ * has been received and sent by the GPU in the last second through PCIe.
+ * The file pcie_bw is used for this.
+ * The Perf counters count the number of received and sent messages and return
+ * those values, as well as the maximum payload size of a PCIe packet (mps).
+ * Note that it is not possible to easily and quickly obtain the size of each
+ * packet transmitted, so we output the max payload size (mps) to allow for
+ * quick estimation of the PCIe bandwidth usage
+ */
+static ssize_t amdgpu_get_pcie_bw(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+   uint64_t count0, count1;
+
+   amdgpu_asic_get_pcie_usage(adev, &count0, &count1);
+   return snprintf(buf, PAGE_SIZE, "%llu %llu %i\n",
+   count0, count1, pcie_get_mps(adev->pdev));
+}
+
 static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, 
amdgpu_set_dpm_state);
 static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
   amdgpu_get_dpm_forced_performance_level,
@@ -1025,6 +1050,7 @@ static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
amdgpu_set_pp_od_clk_voltage);
 static DEVICE_ATTR(gpu_busy_percent, S_IRUGO,
amdgpu_get_busy_percent, NULL);
+static DEVICE_ATTR(pcie_bw, S_IRUGO, amdgpu_get_pcie_bw, NULL);
 
 static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
  struct device_attribute *attr,
@@ -2108,6 +2134,14 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
"gpu_busy_level\n");
return ret;
}
+   /* PCIe Perf counters won't work on APU nodes */
+   if (adev->flags & !AMD_IS_APU) {
+   ret = device_create_file(adev->dev, &dev_attr_pcie_bw);
+   if (ret) {
+   DRM_ERROR("failed to create device file pcie_bw\n");
+   return ret;
+   }
+   }
ret = amdgpu_debugfs_pm_init(adev);
if (ret) {
DRM_ERROR("Failed to register debugfs file for dpm!\n");
@@ -2147,6 +2181,8 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
device_remove_file(adev->dev,
&dev_attr_pp_od_clk_voltage);
device_remove_file(adev->dev, &dev_attr_gpu

[PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage v2

2019-01-08 Thread Russell, Kent
Add a sysfs file that reports the number of bytes transmitted and
received in the last second. This can be used to approximate the PCIe
bandwidth usage over the last second.

v2: Clarify use of mps as estimation of bandwidth

Signed-off-by: Kent Russell 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 34 ++
 drivers/gpu/drm/amd/amdgpu/cik.c   | 41 +++
 drivers/gpu/drm/amd/amdgpu/si.c| 41 +++
 drivers/gpu/drm/amd/amdgpu/soc15.c | 44 ++
 drivers/gpu/drm/amd/amdgpu/vi.c| 41 +++
 6 files changed, 205 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e1b2c64..512b124 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -542,6 +542,9 @@ struct amdgpu_asic_funcs {
bool (*need_full_reset)(struct amdgpu_device *adev);
/* initialize doorbell layout for specific asic*/
void (*init_doorbell_index)(struct amdgpu_device *adev);
+   /* PCIe bandwidth usage */
+   void (*get_pcie_usage)(struct amdgpu_device *adev, uint64_t *count0,
+  uint64_t *count1);
 };
 
 /*
@@ -1045,6 +1048,7 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
 #define amdgpu_asic_invalidate_hdp(adev, r) 
(adev)->asic_funcs->invalidate_hdp((adev), (r))
 #define amdgpu_asic_need_full_reset(adev) 
(adev)->asic_funcs->need_full_reset((adev))
 #define amdgpu_asic_init_doorbell_index(adev) 
(adev)->asic_funcs->init_doorbell_index((adev))
+#define amdgpu_asic_get_pcie_usage(adev, cnt0, cnt1) 
((adev)->asic_funcs->get_pcie_usage((adev), (cnt0), (cnt1)))
 
 /* Common functions */
 bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 6896dec..9cf47ba 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -990,6 +990,33 @@ static ssize_t amdgpu_get_busy_percent(struct device *dev,
return snprintf(buf, PAGE_SIZE, "%d\n", value);
 }
 
+/**
+ * DOC: pcie_bw
+ *
+ * The amdgpu driver provides a sysfs API for reading how much data
+ * has been sent and received by the GPU in the last second through PCIe.
+ * The file pcie_bw is used for this.
+ * The Perf counters calculate this and return the number of sent and received
+ * messages, which we multiply by the maxsize of our PCIe packets (mps).
+ * Note that it is not possible to easily and quickly obtain the size of each
+ * packet transmitted, so we use the max payload size (mps) to estimate the
+ * PCIe bandwidth usage
+ */
+static ssize_t amdgpu_get_pcie_bw(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+   uint64_t mps = pcie_get_mps(adev->pdev);
+   uint64_t count0, count1;
+
+   amdgpu_asic_get_pcie_usage(adev, &count0, &count1);
+   return snprintf(buf, PAGE_SIZE,
+   "Bytes received: %llu\nBytes sent: %llu\n",
+   count0 * mps, count1 * mps);
+}
+
 static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, 
amdgpu_set_dpm_state);
 static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
   amdgpu_get_dpm_forced_performance_level,
@@ -1025,6 +1052,7 @@ static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
amdgpu_set_pp_od_clk_voltage);
 static DEVICE_ATTR(gpu_busy_percent, S_IRUGO,
amdgpu_get_busy_percent, NULL);
+static DEVICE_ATTR(pcie_bw, S_IRUGO, amdgpu_get_pcie_bw, NULL);
 
 static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
  struct device_attribute *attr,
@@ -2108,6 +2136,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
"gpu_busy_level\n");
return ret;
}
+   ret = device_create_file(adev->dev, &dev_attr_pcie_bw);
+   if (ret) {
+   DRM_ERROR("failed to create device file pcie_bw\n");
+   return ret;
+   }
ret = amdgpu_debugfs_pm_init(adev);
if (ret) {
DRM_ERROR("Failed to register debugfs file for dpm!\n");
@@ -2147,6 +2180,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
device_remove_file(adev->dev,
&dev_attr_pp_od_clk_voltage);
device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);
+   device_remove_file(adev->dev, &dev_attr_pcie_bw);
 }
 
 void amdgpu_pm_compute_clocks(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c
index 71c50d8..8681744 100644
--- a/drivers/gpu

RE: [PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage

2019-01-08 Thread Russell, Kent


> -Original Message-
> From: Kuehling, Felix
> Sent: Monday, January 07, 2019 2:54 PM
> To: Russell, Kent ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage
> 
> On 2019-01-04 7:35 a.m., Russell, Kent wrote:
> > Add a sysfs file that reports the number of bytes transmitted and
> > received in the last second. This can be used to approximate the PCIe
> > bandwidth usage over the last second.
> >
> > Change-Id: I6c82fe1cb17726825736512990fa64f5c1489861
> > Signed-off-by: Kent Russell 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 31
> 
> >  drivers/gpu/drm/amd/amdgpu/cik.c   | 41
> +++
> >  drivers/gpu/drm/amd/amdgpu/si.c| 41
> +++
> >  drivers/gpu/drm/amd/amdgpu/soc15.c | 44
> ++
> >  drivers/gpu/drm/amd/amdgpu/vi.c| 41
> +++
> >  6 files changed, 202 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index e1b2c64..512b124 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -542,6 +542,9 @@ struct amdgpu_asic_funcs {
> > bool (*need_full_reset)(struct amdgpu_device *adev);
> > /* initialize doorbell layout for specific asic*/
> > void (*init_doorbell_index)(struct amdgpu_device *adev);
> > +   /* PCIe bandwidth usage */
> > +   void (*get_pcie_usage)(struct amdgpu_device *adev, uint64_t
> *count0,
> > +  uint64_t *count1);
> >  };
> >
> >  /*
> > @@ -1045,6 +1048,7 @@ int emu_soc_asic_init(struct amdgpu_device
> > *adev);  #define amdgpu_asic_invalidate_hdp(adev, r)
> > (adev)->asic_funcs->invalidate_hdp((adev), (r))  #define
> > amdgpu_asic_need_full_reset(adev)
> > (adev)->asic_funcs->need_full_reset((adev))
> >  #define amdgpu_asic_init_doorbell_index(adev)
> > (adev)->asic_funcs->init_doorbell_index((adev))
> > +#define amdgpu_asic_get_pcie_usage(adev, cnt0, cnt1)
> > +((adev)->asic_funcs->get_pcie_usage((adev), (cnt0), (cnt1)))
> >
> >  /* Common functions */
> >  bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > index 1f61ed9..051307e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > @@ -990,6 +990,30 @@ static ssize_t amdgpu_get_busy_percent(struct
> device *dev,
> > return snprintf(buf, PAGE_SIZE, "%d\n", value);  }
> >
> > +/**
> > + * DOC: pcie_bw
> > + *
> > + * The amdgpu driver provides a sysfs API for reading how much data
> > + * has been sent and received by the GPU in the last second through PCIe.
> > + * The file pcie_bw is used for this.
> > + * The Perf counters calculate this and return the number of sent and
> > +received
> > + * messages, which we multiply by the size of our PCIe packets (mps).
> > + */
> > +static ssize_t amdgpu_get_pcie_bw(struct device *dev,
> > +   struct device_attribute *attr,
> > +   char *buf)
> > +{
> > +   struct drm_device *ddev = dev_get_drvdata(dev);
> > +   struct amdgpu_device *adev = ddev->dev_private;
> > +   uint64_t mps = pcie_get_mps(adev->pdev);
> > +   uint64_t count0, count1;
> > +
> > +   amdgpu_asic_get_pcie_usage(adev, &count0, &count1);
> > +   return snprintf(buf, PAGE_SIZE,
> > +   "Bytes received: %llu\nBytes sent: %llu\n",
> > +   count0 * mps, count1 * mps);
> 
> This quietly assumes that all transfers use the maximum payload size.
> 
> We should either state the assumption or report the count rather than a
> number in bytes.
> 
> Regards,
>   Felix

Agreed, I have expanded upon the comment to clarify this in the upcoming v2 
patch

> 
> 
> > +}
> > +
> >  static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR,
> > amdgpu_get_dpm_state, amdgpu_set_dpm_state);  static
> DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
> >amdgpu_get_dpm_forced_performance_level,
> > @@ -1025,6 +1049,7 @@ static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO
> | S_IWUSR,
> > amdgpu_set_pp_od_clk_voltage);
> >  static DEVICE_ATTR(gpu_busy_

Re: [PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage

2019-01-07 Thread Kuehling, Felix
On 2019-01-04 7:35 a.m., Russell, Kent wrote:
> Add a sysfs file that reports the number of bytes transmitted and
> received in the last second. This can be used to approximate the PCIe
> bandwidth usage over the last second.
>
> Change-Id: I6c82fe1cb17726825736512990fa64f5c1489861
> Signed-off-by: Kent Russell 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 31 
>  drivers/gpu/drm/amd/amdgpu/cik.c   | 41 +++
>  drivers/gpu/drm/amd/amdgpu/si.c| 41 +++
>  drivers/gpu/drm/amd/amdgpu/soc15.c | 44 
> ++
>  drivers/gpu/drm/amd/amdgpu/vi.c| 41 +++
>  6 files changed, 202 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e1b2c64..512b124 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -542,6 +542,9 @@ struct amdgpu_asic_funcs {
>   bool (*need_full_reset)(struct amdgpu_device *adev);
>   /* initialize doorbell layout for specific asic*/
>   void (*init_doorbell_index)(struct amdgpu_device *adev);
> + /* PCIe bandwidth usage */
> + void (*get_pcie_usage)(struct amdgpu_device *adev, uint64_t *count0,
> +uint64_t *count1);
>  };
>  
>  /*
> @@ -1045,6 +1048,7 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
>  #define amdgpu_asic_invalidate_hdp(adev, r) 
> (adev)->asic_funcs->invalidate_hdp((adev), (r))
>  #define amdgpu_asic_need_full_reset(adev) 
> (adev)->asic_funcs->need_full_reset((adev))
>  #define amdgpu_asic_init_doorbell_index(adev) 
> (adev)->asic_funcs->init_doorbell_index((adev))
> +#define amdgpu_asic_get_pcie_usage(adev, cnt0, cnt1) 
> ((adev)->asic_funcs->get_pcie_usage((adev), (cnt0), (cnt1)))
>  
>  /* Common functions */
>  bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 1f61ed9..051307e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -990,6 +990,30 @@ static ssize_t amdgpu_get_busy_percent(struct device 
> *dev,
>   return snprintf(buf, PAGE_SIZE, "%d\n", value);
>  }
>  
> +/**
> + * DOC: pcie_bw
> + *
> + * The amdgpu driver provides a sysfs API for reading how much data
> + * has been sent and received by the GPU in the last second through PCIe.
> + * The file pcie_bw is used for this.
> + * The Perf counters calculate this and return the number of sent and 
> received
> + * messages, which we multiply by the size of our PCIe packets (mps).
> + */
> +static ssize_t amdgpu_get_pcie_bw(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = ddev->dev_private;
> + uint64_t mps = pcie_get_mps(adev->pdev);
> + uint64_t count0, count1;
> +
> + amdgpu_asic_get_pcie_usage(adev, &count0, &count1);
> + return snprintf(buf, PAGE_SIZE,
> + "Bytes received: %llu\nBytes sent: %llu\n",
> + count0 * mps, count1 * mps);

This quietly assumes that all transfers use the maximum payload size.

We should either state the assumption or report the count rather than a
number in bytes.

Regards,
  Felix


> +}
> +
>  static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, 
> amdgpu_set_dpm_state);
>  static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
>  amdgpu_get_dpm_forced_performance_level,
> @@ -1025,6 +1049,7 @@ static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
>   amdgpu_set_pp_od_clk_voltage);
>  static DEVICE_ATTR(gpu_busy_percent, S_IRUGO,
>   amdgpu_get_busy_percent, NULL);
> +static DEVICE_ATTR(pcie_bw, S_IRUGO, amdgpu_get_pcie_bw, NULL);
>  
>  static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
> struct device_attribute *attr,
> @@ -2105,6 +2130,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>   "gpu_busy_level\n");
>   return ret;
>   }
> + ret = device_create_file(adev->dev, &dev_attr_pcie_bw);
> + if (ret) {
> + DRM_ERROR("failed to create device file pcie_bw\n");
> + return ret;
> + }
>   ret = amdgpu_debugfs_pm_init(adev);
>   if (ret) {
>   DRM_ERROR("Failed to register debugfs file for dpm!\n");
> @@ -2141,6 +2171,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
>   device_remove_file(adev->dev,
>   &dev_attr_pp_od_clk_voltage);
>   device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);
> + device_remove_file(adev->dev, &dev_attr_pcie_bw);
>  }
>  
>  void amdgp

Re: [PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage

2019-01-07 Thread Alex Deucher
On Mon, Jan 7, 2019 at 6:53 AM Russell, Kent  wrote:
>
> Did you mean on APU+dGPU, or just straight APU? The file currently returns 0s 
> on my Carrizo. The event values are the same on CZ as they are for the dGPU 
> ASICs, so it might just be that it's not supported since it's not going 
> through the physical PCIe bus on the board.  I have tested it on 
> Fiji/Vega10/Vega20 successfully, and they all give reasonable values, at 
> least. But I don't have access to Raven to see if it just returns 0s like CZ 
> does. Would you prefer that the sysfs file only be added for dGPUs, to save 
> confusion and wasted sysfs files that return no useful information?
>

On APUs.  If the registers don't exist or don't do anything on APUs,
then let's just not expose the file.  Otherwise, it just leads to
confusion.

Alex

>  Kent
>
> -Original Message-
> From: Alex Deucher 
> Sent: Friday, January 04, 2019 11:22 AM
> To: Russell, Kent 
> Cc: amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage
>
> On Fri, Jan 4, 2019 at 7:35 AM Russell, Kent  wrote:
> >
> > Add a sysfs file that reports the number of bytes transmitted and
> > received in the last second. This can be used to approximate the PCIe
> > bandwidth usage over the last second.
> >
> > Change-Id: I6c82fe1cb17726825736512990fa64f5c1489861
> > Signed-off-by: Kent Russell 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 31 
> >  drivers/gpu/drm/amd/amdgpu/cik.c   | 41 +++
> >  drivers/gpu/drm/amd/amdgpu/si.c| 41 +++
> >  drivers/gpu/drm/amd/amdgpu/soc15.c | 44 
> > ++
> >  drivers/gpu/drm/amd/amdgpu/vi.c| 41 +++
> >  6 files changed, 202 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index e1b2c64..512b124 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -542,6 +542,9 @@ struct amdgpu_asic_funcs {
> > bool (*need_full_reset)(struct amdgpu_device *adev);
> > /* initialize doorbell layout for specific asic*/
> > void (*init_doorbell_index)(struct amdgpu_device *adev);
> > +   /* PCIe bandwidth usage */
> > +   void (*get_pcie_usage)(struct amdgpu_device *adev, uint64_t *count0,
> > +  uint64_t *count1);
> >  };
> >
> >  /*
> > @@ -1045,6 +1048,7 @@ int emu_soc_asic_init(struct amdgpu_device
> > *adev);  #define amdgpu_asic_invalidate_hdp(adev, r)
> > (adev)->asic_funcs->invalidate_hdp((adev), (r))  #define
> > amdgpu_asic_need_full_reset(adev)
> > (adev)->asic_funcs->need_full_reset((adev))
> >  #define amdgpu_asic_init_doorbell_index(adev)
> > (adev)->asic_funcs->init_doorbell_index((adev))
> > +#define amdgpu_asic_get_pcie_usage(adev, cnt0, cnt1)
> > +((adev)->asic_funcs->get_pcie_usage((adev), (cnt0), (cnt1)))
> >
> >  /* Common functions */
> >  bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > index 1f61ed9..051307e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > @@ -990,6 +990,30 @@ static ssize_t amdgpu_get_busy_percent(struct device 
> > *dev,
> > return snprintf(buf, PAGE_SIZE, "%d\n", value);  }
> >
> > +/**
> > + * DOC: pcie_bw
> > + *
> > + * The amdgpu driver provides a sysfs API for reading how much data
> > + * has been sent and received by the GPU in the last second through PCIe.
> > + * The file pcie_bw is used for this.
> > + * The Perf counters calculate this and return the number of sent and
> > +received
> > + * messages, which we multiply by the size of our PCIe packets (mps).
> > + */
> > +static ssize_t amdgpu_get_pcie_bw(struct device *dev,
> > +   struct device_attribute *attr,
> > +   char *buf)
> > +{
> > +   struct drm_device *ddev = dev_get_drvdata(dev);
> > +   struct amdgpu_device *adev = ddev->dev_private;
> > +   uint64_t mps = pcie_get_mps(adev->pdev);
> > +   uint64_t count0, count1;
> > +
> > +   amdgpu_asic_get_pcie_usage(adev, &count0, &count1);
> > +   r

RE: [PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage

2019-01-07 Thread Russell, Kent
Did you mean on APU+dGPU, or just straight APU? The file currently returns 0s 
on my Carrizo. The event values are the same on CZ as they are for the dGPU 
ASICs, so it might just be that it's not supported since it's not going through 
the physical PCIe bus on the board.  I have tested it on Fiji/Vega10/Vega20 
successfully, and they all give reasonable values, at least. But I don't have 
access to Raven to see if it just returns 0s like CZ does. Would you prefer 
that the sysfs file only be added for dGPUs, to save confusion and wasted sysfs 
files that return no useful information?

 Kent

-Original Message-
From: Alex Deucher  
Sent: Friday, January 04, 2019 11:22 AM
To: Russell, Kent 
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage

On Fri, Jan 4, 2019 at 7:35 AM Russell, Kent  wrote:
>
> Add a sysfs file that reports the number of bytes transmitted and 
> received in the last second. This can be used to approximate the PCIe 
> bandwidth usage over the last second.
>
> Change-Id: I6c82fe1cb17726825736512990fa64f5c1489861
> Signed-off-by: Kent Russell 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 31 
>  drivers/gpu/drm/amd/amdgpu/cik.c   | 41 +++
>  drivers/gpu/drm/amd/amdgpu/si.c| 41 +++
>  drivers/gpu/drm/amd/amdgpu/soc15.c | 44 
> ++
>  drivers/gpu/drm/amd/amdgpu/vi.c| 41 +++
>  6 files changed, 202 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e1b2c64..512b124 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -542,6 +542,9 @@ struct amdgpu_asic_funcs {
> bool (*need_full_reset)(struct amdgpu_device *adev);
> /* initialize doorbell layout for specific asic*/
> void (*init_doorbell_index)(struct amdgpu_device *adev);
> +   /* PCIe bandwidth usage */
> +   void (*get_pcie_usage)(struct amdgpu_device *adev, uint64_t *count0,
> +  uint64_t *count1);
>  };
>
>  /*
> @@ -1045,6 +1048,7 @@ int emu_soc_asic_init(struct amdgpu_device 
> *adev);  #define amdgpu_asic_invalidate_hdp(adev, r) 
> (adev)->asic_funcs->invalidate_hdp((adev), (r))  #define 
> amdgpu_asic_need_full_reset(adev) 
> (adev)->asic_funcs->need_full_reset((adev))
>  #define amdgpu_asic_init_doorbell_index(adev) 
> (adev)->asic_funcs->init_doorbell_index((adev))
> +#define amdgpu_asic_get_pcie_usage(adev, cnt0, cnt1) 
> +((adev)->asic_funcs->get_pcie_usage((adev), (cnt0), (cnt1)))
>
>  /* Common functions */
>  bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev); 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 1f61ed9..051307e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -990,6 +990,30 @@ static ssize_t amdgpu_get_busy_percent(struct device 
> *dev,
> return snprintf(buf, PAGE_SIZE, "%d\n", value);  }
>
> +/**
> + * DOC: pcie_bw
> + *
> + * The amdgpu driver provides a sysfs API for reading how much data
> + * has been sent and received by the GPU in the last second through PCIe.
> + * The file pcie_bw is used for this.
> + * The Perf counters calculate this and return the number of sent and 
> +received
> + * messages, which we multiply by the size of our PCIe packets (mps).
> + */
> +static ssize_t amdgpu_get_pcie_bw(struct device *dev,
> +   struct device_attribute *attr,
> +   char *buf)
> +{
> +   struct drm_device *ddev = dev_get_drvdata(dev);
> +   struct amdgpu_device *adev = ddev->dev_private;
> +   uint64_t mps = pcie_get_mps(adev->pdev);
> +   uint64_t count0, count1;
> +
> +   amdgpu_asic_get_pcie_usage(adev, &count0, &count1);
> +   return snprintf(buf, PAGE_SIZE,
> +   "Bytes received: %llu\nBytes sent: %llu\n",
> +   count0 * mps, count1 * mps); }
> +
>  static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, 
> amdgpu_get_dpm_state, amdgpu_set_dpm_state);  static 
> DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
>amdgpu_get_dpm_forced_performance_level,
> @@ -1025,6 +1049,7 @@ static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
> amdgpu_set_pp_od_clk_voltage);  static 
> DEVICE_ATTR(gpu_busy_percent, S_IRUGO,
> amdgpu_get_busy_percent, NULL);
>

Re: [PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage

2019-01-04 Thread Alex Deucher
On Fri, Jan 4, 2019 at 7:35 AM Russell, Kent  wrote:
>
> Add a sysfs file that reports the number of bytes transmitted and
> received in the last second. This can be used to approximate the PCIe
> bandwidth usage over the last second.
>
> Change-Id: I6c82fe1cb17726825736512990fa64f5c1489861
> Signed-off-by: Kent Russell 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 31 
>  drivers/gpu/drm/amd/amdgpu/cik.c   | 41 +++
>  drivers/gpu/drm/amd/amdgpu/si.c| 41 +++
>  drivers/gpu/drm/amd/amdgpu/soc15.c | 44 
> ++
>  drivers/gpu/drm/amd/amdgpu/vi.c| 41 +++
>  6 files changed, 202 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e1b2c64..512b124 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -542,6 +542,9 @@ struct amdgpu_asic_funcs {
> bool (*need_full_reset)(struct amdgpu_device *adev);
> /* initialize doorbell layout for specific asic*/
> void (*init_doorbell_index)(struct amdgpu_device *adev);
> +   /* PCIe bandwidth usage */
> +   void (*get_pcie_usage)(struct amdgpu_device *adev, uint64_t *count0,
> +  uint64_t *count1);
>  };
>
>  /*
> @@ -1045,6 +1048,7 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
>  #define amdgpu_asic_invalidate_hdp(adev, r) 
> (adev)->asic_funcs->invalidate_hdp((adev), (r))
>  #define amdgpu_asic_need_full_reset(adev) 
> (adev)->asic_funcs->need_full_reset((adev))
>  #define amdgpu_asic_init_doorbell_index(adev) 
> (adev)->asic_funcs->init_doorbell_index((adev))
> +#define amdgpu_asic_get_pcie_usage(adev, cnt0, cnt1) 
> ((adev)->asic_funcs->get_pcie_usage((adev), (cnt0), (cnt1)))
>
>  /* Common functions */
>  bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 1f61ed9..051307e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -990,6 +990,30 @@ static ssize_t amdgpu_get_busy_percent(struct device 
> *dev,
> return snprintf(buf, PAGE_SIZE, "%d\n", value);
>  }
>
> +/**
> + * DOC: pcie_bw
> + *
> + * The amdgpu driver provides a sysfs API for reading how much data
> + * has been sent and received by the GPU in the last second through PCIe.
> + * The file pcie_bw is used for this.
> + * The Perf counters calculate this and return the number of sent and 
> received
> + * messages, which we multiply by the size of our PCIe packets (mps).
> + */
> +static ssize_t amdgpu_get_pcie_bw(struct device *dev,
> +   struct device_attribute *attr,
> +   char *buf)
> +{
> +   struct drm_device *ddev = dev_get_drvdata(dev);
> +   struct amdgpu_device *adev = ddev->dev_private;
> +   uint64_t mps = pcie_get_mps(adev->pdev);
> +   uint64_t count0, count1;
> +
> +   amdgpu_asic_get_pcie_usage(adev, &count0, &count1);
> +   return snprintf(buf, PAGE_SIZE,
> +   "Bytes received: %llu\nBytes sent: %llu\n",
> +   count0 * mps, count1 * mps);
> +}
> +
>  static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, 
> amdgpu_set_dpm_state);
>  static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
>amdgpu_get_dpm_forced_performance_level,
> @@ -1025,6 +1049,7 @@ static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
> amdgpu_set_pp_od_clk_voltage);
>  static DEVICE_ATTR(gpu_busy_percent, S_IRUGO,
> amdgpu_get_busy_percent, NULL);
> +static DEVICE_ATTR(pcie_bw, S_IRUGO, amdgpu_get_pcie_bw, NULL);
>
>  static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
>   struct device_attribute *attr,
> @@ -2105,6 +2130,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
> "gpu_busy_level\n");
> return ret;
> }
> +   ret = device_create_file(adev->dev, &dev_attr_pcie_bw);
> +   if (ret) {
> +   DRM_ERROR("failed to create device file pcie_bw\n");
> +   return ret;
> +   }

Should this file be added for APUs too or just dGPUs?

> ret = amdgpu_debugfs_pm_init(adev);
> if (ret) {
> DRM_ERROR("Failed to register debugfs file for dpm!\n");
> @@ -2141,6 +2171,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
> device_remove_file(adev->dev,
> &dev_attr_pp_od_clk_voltage);
> device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);
> +   device_remove_file(adev->dev, &dev_attr_pcie_bw);
>  }
>
>  void amdgpu_pm_compute_clocks(struct amdgpu_device *adev)
> diff

[PATCH 2/2] drm/amdgpu: Add sysfs file for PCIe usage

2019-01-04 Thread Russell, Kent
Add a sysfs file that reports the number of bytes transmitted and
received in the last second. This can be used to approximate the PCIe
bandwidth usage over the last second.

Change-Id: I6c82fe1cb17726825736512990fa64f5c1489861
Signed-off-by: Kent Russell 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 31 
 drivers/gpu/drm/amd/amdgpu/cik.c   | 41 +++
 drivers/gpu/drm/amd/amdgpu/si.c| 41 +++
 drivers/gpu/drm/amd/amdgpu/soc15.c | 44 ++
 drivers/gpu/drm/amd/amdgpu/vi.c| 41 +++
 6 files changed, 202 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e1b2c64..512b124 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -542,6 +542,9 @@ struct amdgpu_asic_funcs {
bool (*need_full_reset)(struct amdgpu_device *adev);
/* initialize doorbell layout for specific asic*/
void (*init_doorbell_index)(struct amdgpu_device *adev);
+   /* PCIe bandwidth usage */
+   void (*get_pcie_usage)(struct amdgpu_device *adev, uint64_t *count0,
+  uint64_t *count1);
 };
 
 /*
@@ -1045,6 +1048,7 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
 #define amdgpu_asic_invalidate_hdp(adev, r) 
(adev)->asic_funcs->invalidate_hdp((adev), (r))
 #define amdgpu_asic_need_full_reset(adev) 
(adev)->asic_funcs->need_full_reset((adev))
 #define amdgpu_asic_init_doorbell_index(adev) 
(adev)->asic_funcs->init_doorbell_index((adev))
+#define amdgpu_asic_get_pcie_usage(adev, cnt0, cnt1) 
((adev)->asic_funcs->get_pcie_usage((adev), (cnt0), (cnt1)))
 
 /* Common functions */
 bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 1f61ed9..051307e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -990,6 +990,30 @@ static ssize_t amdgpu_get_busy_percent(struct device *dev,
return snprintf(buf, PAGE_SIZE, "%d\n", value);
 }
 
+/**
+ * DOC: pcie_bw
+ *
+ * The amdgpu driver provides a sysfs API for reading how much data
+ * has been sent and received by the GPU in the last second through PCIe.
+ * The file pcie_bw is used for this.
+ * The Perf counters calculate this and return the number of sent and received
+ * messages, which we multiply by the size of our PCIe packets (mps).
+ */
+static ssize_t amdgpu_get_pcie_bw(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_device *ddev = dev_get_drvdata(dev);
+   struct amdgpu_device *adev = ddev->dev_private;
+   uint64_t mps = pcie_get_mps(adev->pdev);
+   uint64_t count0, count1;
+
+   amdgpu_asic_get_pcie_usage(adev, &count0, &count1);
+   return snprintf(buf, PAGE_SIZE,
+   "Bytes received: %llu\nBytes sent: %llu\n",
+   count0 * mps, count1 * mps);
+}
+
 static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, 
amdgpu_set_dpm_state);
 static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
   amdgpu_get_dpm_forced_performance_level,
@@ -1025,6 +1049,7 @@ static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
amdgpu_set_pp_od_clk_voltage);
 static DEVICE_ATTR(gpu_busy_percent, S_IRUGO,
amdgpu_get_busy_percent, NULL);
+static DEVICE_ATTR(pcie_bw, S_IRUGO, amdgpu_get_pcie_bw, NULL);
 
 static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
  struct device_attribute *attr,
@@ -2105,6 +2130,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
"gpu_busy_level\n");
return ret;
}
+   ret = device_create_file(adev->dev, &dev_attr_pcie_bw);
+   if (ret) {
+   DRM_ERROR("failed to create device file pcie_bw\n");
+   return ret;
+   }
ret = amdgpu_debugfs_pm_init(adev);
if (ret) {
DRM_ERROR("Failed to register debugfs file for dpm!\n");
@@ -2141,6 +2171,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
device_remove_file(adev->dev,
&dev_attr_pp_od_clk_voltage);
device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);
+   device_remove_file(adev->dev, &dev_attr_pcie_bw);
 }
 
 void amdgpu_pm_compute_clocks(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c
index 71c50d8..8681744 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik.c
@@ -1741,6 +1741,46 @@ static bool cik_need_full_reset(struct amdgpu_device 
*adev)
return true;
 }
 
+static void cik_get_pcie_usa