Re: [Intel-gfx] [PATCH 10/11] drm/i915/perf: limit OA buffer size to minimum when unread

[Lionel Landwerlin]

On 26/03/18 10:08, Lionel Landwerlin wrote:

The way our hardware is designed doesn't seem to let us use the
MI_RECORD_PERF_COUNT command without setting up a circular buffer.

In the case where the user didn't request OA reports to be available
through the i915 perf stream, we can set the OA buffer to the minimum
size to avoid consuming memory which won't be used by the driver.

Signed-off-by: Lionel Landwerlin 
---
  drivers/gpu/drm/i915/i915_perf.c | 15 ++-
  1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 55c7631ad3c2..f8ec09e2d599 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1372,7 +1372,8 @@ static void gen7_init_oa_buffer(struct drm_i915_private 
*dev_priv)
 * the assumption that new reports are being written to zeroed
 * memory...
 */
There is an issue in both function programming the OA buffer. They still 
program the length of the buffer to 16Mb.

I'll send a v2.

-   memset(dev_priv->perf.oa.oa_buffer.vaddr, 0, OA_BUFFER_SIZE);
+   memset(dev_priv->perf.oa.oa_buffer.vaddr, 0,
+  dev_priv->perf.oa.oa_buffer.vma->size);
  
  	/* Maybe make ->pollin per-stream state if we support multiple

 * concurrent streams in the future.
@@ -1430,7 +1431,8 @@ static void gen8_init_oa_buffer(struct drm_i915_private 
*dev_priv)
 * the assumption that new reports are being written to zeroed
 * memory...
 */
-   memset(dev_priv->perf.oa.oa_buffer.vaddr, 0, OA_BUFFER_SIZE);
+   memset(dev_priv->perf.oa.oa_buffer.vaddr, 0,
+  dev_priv->perf.oa.oa_buffer.vma->size);
  
  	/*

 * Maybe make ->pollin per-stream state if we support multiple
@@ -1439,10 +1441,13 @@ static void gen8_init_oa_buffer(struct drm_i915_private 
*dev_priv)
dev_priv->perf.oa.pollin = false;
  }
  
-static int alloc_oa_buffer(struct drm_i915_private *dev_priv)

+static int alloc_oa_buffer(struct drm_i915_private *dev_priv,
+  struct i915_perf_stream *stream)
  {
struct drm_i915_gem_object *bo;
struct i915_vma *vma;
+   u32 buffer_size = (stream->sample_flags & SAMPLE_OA_REPORT) == 0 ?
+   SZ_128K : OA_BUFFER_SIZE;
int ret;
  
  	ret = i915_mutex_lock_interruptible(_priv->drm);

@@ -1457,7 +1462,7 @@ static int alloc_oa_buffer(struct drm_i915_private 
*dev_priv)
BUILD_BUG_ON_NOT_POWER_OF_2(OA_BUFFER_SIZE);
BUILD_BUG_ON(OA_BUFFER_SIZE < SZ_128K || OA_BUFFER_SIZE > SZ_16M);
  
-	bo = i915_gem_object_create(dev_priv, OA_BUFFER_SIZE);

+   bo = i915_gem_object_create(dev_priv, buffer_size);
if (IS_ERR(bo)) {
DRM_ERROR("Failed to allocate OA buffer\n");
ret = PTR_ERR(bo);
@@ -2148,7 +2153,7 @@ static int i915_oa_stream_init(struct i915_perf_stream 
*stream,
intel_runtime_pm_get(dev_priv);
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
  
-	ret = alloc_oa_buffer(dev_priv);

+   ret = alloc_oa_buffer(dev_priv, stream);
if (ret)
goto err_oa_buf_alloc;
  



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

[Intel-gfx] [PATCH 10/11] drm/i915/perf: limit OA buffer size to minimum when unread

[Lionel Landwerlin]

The way our hardware is designed doesn't seem to let us use the
MI_RECORD_PERF_COUNT command without setting up a circular buffer.

In the case where the user didn't request OA reports to be available
through the i915 perf stream, we can set the OA buffer to the minimum
size to avoid consuming memory which won't be used by the driver.

Signed-off-by: Lionel Landwerlin 
---
 drivers/gpu/drm/i915/i915_perf.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 55c7631ad3c2..f8ec09e2d599 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1372,7 +1372,8 @@ static void gen7_init_oa_buffer(struct drm_i915_private 
*dev_priv)
 * the assumption that new reports are being written to zeroed
 * memory...
 */
-   memset(dev_priv->perf.oa.oa_buffer.vaddr, 0, OA_BUFFER_SIZE);
+   memset(dev_priv->perf.oa.oa_buffer.vaddr, 0,
+  dev_priv->perf.oa.oa_buffer.vma->size);
 
/* Maybe make ->pollin per-stream state if we support multiple
 * concurrent streams in the future.
@@ -1430,7 +1431,8 @@ static void gen8_init_oa_buffer(struct drm_i915_private 
*dev_priv)
 * the assumption that new reports are being written to zeroed
 * memory...
 */
-   memset(dev_priv->perf.oa.oa_buffer.vaddr, 0, OA_BUFFER_SIZE);
+   memset(dev_priv->perf.oa.oa_buffer.vaddr, 0,
+  dev_priv->perf.oa.oa_buffer.vma->size);
 
/*
 * Maybe make ->pollin per-stream state if we support multiple
@@ -1439,10 +1441,13 @@ static void gen8_init_oa_buffer(struct drm_i915_private 
*dev_priv)
dev_priv->perf.oa.pollin = false;
 }
 
-static int alloc_oa_buffer(struct drm_i915_private *dev_priv)
+static int alloc_oa_buffer(struct drm_i915_private *dev_priv,
+  struct i915_perf_stream *stream)
 {
struct drm_i915_gem_object *bo;
struct i915_vma *vma;
+   u32 buffer_size = (stream->sample_flags & SAMPLE_OA_REPORT) == 0 ?
+   SZ_128K : OA_BUFFER_SIZE;
int ret;
 
ret = i915_mutex_lock_interruptible(_priv->drm);
@@ -1457,7 +1462,7 @@ static int alloc_oa_buffer(struct drm_i915_private 
*dev_priv)
BUILD_BUG_ON_NOT_POWER_OF_2(OA_BUFFER_SIZE);
BUILD_BUG_ON(OA_BUFFER_SIZE < SZ_128K || OA_BUFFER_SIZE > SZ_16M);
 
-   bo = i915_gem_object_create(dev_priv, OA_BUFFER_SIZE);
+   bo = i915_gem_object_create(dev_priv, buffer_size);
if (IS_ERR(bo)) {
DRM_ERROR("Failed to allocate OA buffer\n");
ret = PTR_ERR(bo);
@@ -2148,7 +2153,7 @@ static int i915_oa_stream_init(struct i915_perf_stream 
*stream,
intel_runtime_pm_get(dev_priv);
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
 
-   ret = alloc_oa_buffer(dev_priv);
+   ret = alloc_oa_buffer(dev_priv, stream);
if (ret)
goto err_oa_buf_alloc;
 
-- 
2.16.3

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