Re: [Intel-gfx] [PATCH 06/12] drm/i915: Populate ctx ID for periodic OA reports

2017-07-31 Thread kbuild test robot
Hi Sourab,

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20170731]
[cannot apply to v4.13-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Sagar-Arun-Kamble/i915-perf-support-for-command-stream-based-OA-GPU-and-workload-metrics-capture/20170731-184412
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick 
(https://www.imagemagick.org)
   include/linux/init.h:1: warning: no structured comments found
   include/linux/mod_devicetable.h:687: warning: Excess 
struct/union/enum/typedef member 'ver_major' description in 'fsl_mc_device_id'
   include/linux/mod_devicetable.h:687: warning: Excess 
struct/union/enum/typedef member 'ver_minor' description in 'fsl_mc_device_id'
   kernel/sched/core.c:2080: warning: No description found for parameter 'rf'
   kernel/sched/core.c:2080: warning: Excess function parameter 'cookie' 
description in 'try_to_wake_up_local'
   include/linux/wait.h:555: warning: No description found for parameter 'wq'
   include/linux/wait.h:555: warning: Excess function parameter 'wq_head' 
description in 'wait_event_interruptible_hrtimeout'
   include/linux/wait.h:759: warning: No description found for parameter 
'wq_head'
   include/linux/wait.h:759: warning: Excess function parameter 'wq' 
description in 'wait_event_killable'
   include/linux/kthread.h:26: warning: Excess function parameter '...' 
description in 'kthread_create'
   kernel/sys.c:1: warning: no structured comments found
   include/linux/device.h:968: warning: No description found for parameter 
'dma_ops'
   drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
   include/linux/iio/iio.h:603: warning: No description found for parameter 
'trig_readonly'
   include/linux/iio/trigger.h:151: warning: No description found for parameter 
'indio_dev'
   include/linux/iio/trigger.h:151: warning: No description found for parameter 
'trig'
   include/linux/device.h:969: warning: No description found for parameter 
'dma_ops'
   drivers/ata/libata-eh.c:1449: warning: No description found for parameter 
'link'
   drivers/ata/libata-eh.c:1449: warning: Excess function parameter 'ap' 
description in 'ata_eh_done'
   drivers/ata/libata-eh.c:1590: warning: No description found for parameter 
'qc'
   drivers/ata/libata-eh.c:1590: warning: Excess function parameter 'dev' 
description in 'ata_eh_request_sense'
   drivers/mtd/nand/nand_base.c:2751: warning: Excess function parameter 
'cached' description in 'nand_write_page'
   drivers/mtd/nand/nand_base.c:2751: warning: Excess function parameter 
'cached' description in 'nand_write_page'
   arch/s390/include/asm/cmb.h:1: warning: no structured comments found
   drivers/scsi/scsi_lib.c:1116: warning: No description found for parameter 
'rq'
   drivers/scsi/constants.c:1: warning: no structured comments found
   include/linux/usb/gadget.h:230: warning: No description found for parameter 
'claimed'
   include/linux/usb/gadget.h:230: warning: No description found for parameter 
'enabled'
   include/linux/usb/gadget.h:412: warning: No description found for parameter 
'quirk_altset_not_supp'
   include/linux/usb/gadget.h:412: warning: No description found for parameter 
'quirk_stall_not_supp'
   include/linux/usb/gadget.h:412: warning: No description found for parameter 
'quirk_zlp_not_supp'
   fs/inode.c:1666: warning: No description found for parameter 'rcu'
   include/linux/jbd2.h:443: warning: No description found for parameter 
'i_transaction'
   include/linux/jbd2.h:443: warning: No description found for parameter 
'i_next_transaction'
   include/linux/jbd2.h:443: warning: No description found for parameter 
'i_list'
   include/linux/jbd2.h:443: warning: No description found for parameter 
'i_vfs_inode'
   include/linux/jbd2.h:443: warning: No description found for parameter 
'i_flags'
   include/linux/jbd2.h:497: warning: No description found for parameter 
'h_rsv_handle'
   include/linux/jbd2.h:497: warning: No description found for parameter 
'h_reserved'
   include/linux/jbd2.h:497: warning: No description found for parameter 
'h_type'
   include/linux/jbd2.h:497: warning: No description found for parameter 
'h_line_no'
   include/linux/jbd2.h:497: warning: No description found for parameter 
'h_start_jiffies'
   include/linux/jbd2.h:497: warning: No description found for parameter 
'h_requested_credits'
   include/linux/jbd2.h:497: warning: No description found for parameter 
'saved_alloc_context'
   include/linux/jbd2.h:1050: warning: No description found for parameter 
'j_chkpt_bhs'
   include/linux/jbd2.h:1050: warning: No description found for parameter 
'j_devname'
   include/linux/jbd2.h:1050: warning: No description found for parameter 
'j_average_commit_time'
   include

Re: [Intel-gfx] [PATCH 06/12] drm/i915: Populate ctx ID for periodic OA reports

2017-07-31 Thread Kamble, Sagar A
Ctx_id for first submission will be its corresponding context as CS sample for 
that is allocated during submission with ctx_id taken from ctx->hw_id.
For periodic reports, cs sample after those reports will have the ctx_id info 
as the timestamp of that CS sample's report is greater than periodic report.

With no CS samples, periodic reports can't be associated with last context 
hence that would need change in following patch to set last ctx id to INVALID
f5f73cf drm/i915: Flush periodic samples, in case of no pending CS sample 
requests

Timestamps of OA reports taken before and after batch are used to associate 
ctx_id information with OA reports.
So for e.g. for batches B1, B2 if following is the timeline:
B1.start -> P1 -> P2 -> B1.end -> P3 -> B2.start -> P4 -> B2.end

Then while reading CS samples will be read first interleaved with OA samples so
Read sequence will be
1. Read B1.start report
2. Read P1 and P2 and associate with B1's context
3. Read B1.end report
4. Read P3 and associate with B1 (this is incorrect - should not be tagged with 
any context)
5. Read B2.start report
6. Read P4 and associate with B2's context
7. Read B2.end report



-Original Message-
From: Landwerlin, Lionel G 
Sent: Monday, July 31, 2017 2:57 PM
To: Kamble, Sagar A ; intel-gfx@lists.freedesktop.org
Cc: Sourab Gupta 
Subject: Re: [Intel-gfx] [PATCH 06/12] drm/i915: Populate ctx ID for periodic 
OA reports

Hi Sagar,

I'm curious to what happens if 2 contexts submit requests which a time period 
smaller than the sampling OA period on Gen7.5.
My understanding is that with this change you'll only retain the last 
submission and then the ctx_id reported in the SAMPLE_CTX_ID field will be 
incorrect for the first workload.

Am I missing something?

-
Lionel

On 31/07/17 08:59, Sagar Arun Kamble wrote:
> From: Sourab Gupta 
>
> This adds support for populating the ctx id for the periodic OA 
> reports when requested through the corresponding property.
>
> For Gen8, the OA reports itself have the ctx ID and it is the one 
> programmed into HW while submitting workloads. Thus it's retrieved 
> from reports itself.
> For Gen7, the OA reports don't have any such field, and we can 
> populate this field with the last seen ctx ID while sending CS reports.
>
> Signed-off-by: Sourab Gupta 
> Signed-off-by: Sagar Arun Kamble 
> ---
>   drivers/gpu/drm/i915/i915_drv.h  |  8 ++
>   drivers/gpu/drm/i915/i915_perf.c | 58 
> +++-
>   2 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> b/drivers/gpu/drm/i915/i915_drv.h index fb81315..6c011f3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2077,6 +2077,8 @@ struct i915_perf_stream {
>   
>   wait_queue_head_t poll_wq;
>   bool pollin;
> +
> + u32 last_ctx_id;
>   };
>   
>   /**
> @@ -2151,6 +2153,12 @@ struct i915_oa_ops {
>* generations.
>*/
>   u32 (*oa_hw_tail_read)(struct drm_i915_private *dev_priv);
> +
> + /**
> +  * @get_ctx_id: Retrieve the ctx_id associated with the (periodic) OA
> +  * report.
> +  */
> + u32 (*get_ctx_id)(struct i915_perf_stream *stream, const u8 
> +*report);
>   };
>   
>   /*
> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
> b/drivers/gpu/drm/i915/i915_perf.c
> index 905c5bb..1f5ebdb 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -790,6 +790,45 @@ static u32 oa_buffer_num_reports_unlocked(
>   return aged_tail == INVALID_TAIL_PTR ? 0 : num_reports;
>   }
>   
> +static u32 gen7_oa_buffer_get_ctx_id(struct i915_perf_stream *stream,
> + const u8 *report)
> +{
> + if (!stream->cs_mode)
> + WARN_ONCE(1,
> + "CTX ID can't be retrieved if command stream mode not 
> enabled");
> +
> + /*
> +  * OA reports generated in Gen7 don't have the ctx ID information.
> +  * Therefore, just rely on the ctx ID information from the last CS
> +  * sample forwarded
> +  */
> + return stream->last_ctx_id;
> +}
> +
> +static u32 gen8_oa_buffer_get_ctx_id(struct i915_perf_stream *stream,
> + const u8 *report)
> +{
> + u32 ctx_id;
> +
> + /* The ctx ID present in the OA reports have intel_context::hw_id
> +  * present, since this is programmed into the ELSP in execlist mode.
> +  * In non-execlist mode, fall back to retrieving the ctx ID from the
> +  * last saved ctx ID from command stream mode.
> +  */
> + if (i915.enable_execlists) {
> 

Re: [Intel-gfx] [PATCH 06/12] drm/i915: Populate ctx ID for periodic OA reports

2017-07-31 Thread Lionel Landwerlin

Hi Sagar,

I'm curious to what happens if 2 contexts submit requests which a time 
period smaller than the sampling OA period on Gen7.5.
My understanding is that with this change you'll only retain the last 
submission and then the ctx_id reported in the SAMPLE_CTX_ID field will 
be incorrect for the first workload.


Am I missing something?

-
Lionel

On 31/07/17 08:59, Sagar Arun Kamble wrote:

From: Sourab Gupta 

This adds support for populating the ctx id for the periodic OA reports
when requested through the corresponding property.

For Gen8, the OA reports itself have the ctx ID and it is the one
programmed into HW while submitting workloads. Thus it's retrieved from
reports itself.
For Gen7, the OA reports don't have any such field, and we can populate
this field with the last seen ctx ID while sending CS reports.

Signed-off-by: Sourab Gupta 
Signed-off-by: Sagar Arun Kamble 
---
  drivers/gpu/drm/i915/i915_drv.h  |  8 ++
  drivers/gpu/drm/i915/i915_perf.c | 58 +++-
  2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fb81315..6c011f3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2077,6 +2077,8 @@ struct i915_perf_stream {
  
  	wait_queue_head_t poll_wq;

bool pollin;
+
+   u32 last_ctx_id;
  };
  
  /**

@@ -2151,6 +2153,12 @@ struct i915_oa_ops {
 * generations.
 */
u32 (*oa_hw_tail_read)(struct drm_i915_private *dev_priv);
+
+   /**
+* @get_ctx_id: Retrieve the ctx_id associated with the (periodic) OA
+* report.
+*/
+   u32 (*get_ctx_id)(struct i915_perf_stream *stream, const u8 *report);
  };
  
  /*

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 905c5bb..1f5ebdb 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -790,6 +790,45 @@ static u32 oa_buffer_num_reports_unlocked(
return aged_tail == INVALID_TAIL_PTR ? 0 : num_reports;
  }
  
+static u32 gen7_oa_buffer_get_ctx_id(struct i915_perf_stream *stream,

+   const u8 *report)
+{
+   if (!stream->cs_mode)
+   WARN_ONCE(1,
+   "CTX ID can't be retrieved if command stream mode not 
enabled");
+
+   /*
+* OA reports generated in Gen7 don't have the ctx ID information.
+* Therefore, just rely on the ctx ID information from the last CS
+* sample forwarded
+*/
+   return stream->last_ctx_id;
+}
+
+static u32 gen8_oa_buffer_get_ctx_id(struct i915_perf_stream *stream,
+   const u8 *report)
+{
+   u32 ctx_id;
+
+   /* The ctx ID present in the OA reports have intel_context::hw_id
+* present, since this is programmed into the ELSP in execlist mode.
+* In non-execlist mode, fall back to retrieving the ctx ID from the
+* last saved ctx ID from command stream mode.
+*/
+   if (i915.enable_execlists) {
+   u32 *report32 = (void *)report;
+
+   ctx_id = report32[2] & 0x1f;
+   } else {
+   if (!stream->cs_mode)
+   WARN_ONCE(1,
+   "CTX ID can't be retrieved if command stream mode 
not enabled");
+
+   ctx_id = stream->last_ctx_id;
+   }
+   return ctx_id;
+}
+
  /**
   * append_oa_status - Appends a status record to a userspace read() buffer.
   * @stream: An i915-perf stream opened for OA metrics
@@ -914,22 +953,12 @@ static int append_oa_buffer_sample(struct 
i915_perf_stream *stream,
struct drm_i915_private *dev_priv = stream->dev_priv;
u32 sample_flags = stream->sample_flags;
struct i915_perf_sample_data data = { 0 };
-   u32 *report32 = (u32 *)report;
  
  	if (sample_flags & SAMPLE_OA_SOURCE)

data.source = I915_PERF_SAMPLE_OA_SOURCE_OABUFFER;
  
  	if (sample_flags & SAMPLE_CTX_ID) {

-   if (INTEL_INFO(dev_priv)->gen < 8)
-   data.ctx_id = 0;
-   else {
-   /*
-* XXX: Just keep the lower 21 bits for now since I'm
-* not entirely sure if the HW touches any of the higher
-* bits in this field
-*/
-   data.ctx_id = report32[2] & 0x1f;
-   }
+   data.ctx_id = dev_priv->perf.oa.ops.get_ctx_id(stream, report);
}
  
  	if (sample_flags & SAMPLE_OA_REPORT)

@@ -1524,8 +1553,10 @@ static int append_cs_buffer_sample(struct 
i915_perf_stream *stream,
if (sample_flags & SAMPLE_OA_SOURCE)
data.source = I915_PERF_SAMPLE_OA_SOURCE_CS;
  
-	if (sample_flags & SAMPLE_CTX_ID)

+   if (sample_flags & SAMPLE_CTX_ID) {
data.ctx_id = node->ctx_id;
+   st

[Intel-gfx] [PATCH 06/12] drm/i915: Populate ctx ID for periodic OA reports

2017-07-31 Thread Sagar Arun Kamble
From: Sourab Gupta 

This adds support for populating the ctx id for the periodic OA reports
when requested through the corresponding property.

For Gen8, the OA reports itself have the ctx ID and it is the one
programmed into HW while submitting workloads. Thus it's retrieved from
reports itself.
For Gen7, the OA reports don't have any such field, and we can populate
this field with the last seen ctx ID while sending CS reports.

Signed-off-by: Sourab Gupta 
Signed-off-by: Sagar Arun Kamble 
---
 drivers/gpu/drm/i915/i915_drv.h  |  8 ++
 drivers/gpu/drm/i915/i915_perf.c | 58 +++-
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fb81315..6c011f3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2077,6 +2077,8 @@ struct i915_perf_stream {
 
wait_queue_head_t poll_wq;
bool pollin;
+
+   u32 last_ctx_id;
 };
 
 /**
@@ -2151,6 +2153,12 @@ struct i915_oa_ops {
 * generations.
 */
u32 (*oa_hw_tail_read)(struct drm_i915_private *dev_priv);
+
+   /**
+* @get_ctx_id: Retrieve the ctx_id associated with the (periodic) OA
+* report.
+*/
+   u32 (*get_ctx_id)(struct i915_perf_stream *stream, const u8 *report);
 };
 
 /*
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 905c5bb..1f5ebdb 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -790,6 +790,45 @@ static u32 oa_buffer_num_reports_unlocked(
return aged_tail == INVALID_TAIL_PTR ? 0 : num_reports;
 }
 
+static u32 gen7_oa_buffer_get_ctx_id(struct i915_perf_stream *stream,
+   const u8 *report)
+{
+   if (!stream->cs_mode)
+   WARN_ONCE(1,
+   "CTX ID can't be retrieved if command stream mode not 
enabled");
+
+   /*
+* OA reports generated in Gen7 don't have the ctx ID information.
+* Therefore, just rely on the ctx ID information from the last CS
+* sample forwarded
+*/
+   return stream->last_ctx_id;
+}
+
+static u32 gen8_oa_buffer_get_ctx_id(struct i915_perf_stream *stream,
+   const u8 *report)
+{
+   u32 ctx_id;
+
+   /* The ctx ID present in the OA reports have intel_context::hw_id
+* present, since this is programmed into the ELSP in execlist mode.
+* In non-execlist mode, fall back to retrieving the ctx ID from the
+* last saved ctx ID from command stream mode.
+*/
+   if (i915.enable_execlists) {
+   u32 *report32 = (void *)report;
+
+   ctx_id = report32[2] & 0x1f;
+   } else {
+   if (!stream->cs_mode)
+   WARN_ONCE(1,
+   "CTX ID can't be retrieved if command stream 
mode not enabled");
+
+   ctx_id = stream->last_ctx_id;
+   }
+   return ctx_id;
+}
+
 /**
  * append_oa_status - Appends a status record to a userspace read() buffer.
  * @stream: An i915-perf stream opened for OA metrics
@@ -914,22 +953,12 @@ static int append_oa_buffer_sample(struct 
i915_perf_stream *stream,
struct drm_i915_private *dev_priv = stream->dev_priv;
u32 sample_flags = stream->sample_flags;
struct i915_perf_sample_data data = { 0 };
-   u32 *report32 = (u32 *)report;
 
if (sample_flags & SAMPLE_OA_SOURCE)
data.source = I915_PERF_SAMPLE_OA_SOURCE_OABUFFER;
 
if (sample_flags & SAMPLE_CTX_ID) {
-   if (INTEL_INFO(dev_priv)->gen < 8)
-   data.ctx_id = 0;
-   else {
-   /*
-* XXX: Just keep the lower 21 bits for now since I'm
-* not entirely sure if the HW touches any of the higher
-* bits in this field
-*/
-   data.ctx_id = report32[2] & 0x1f;
-   }
+   data.ctx_id = dev_priv->perf.oa.ops.get_ctx_id(stream, report);
}
 
if (sample_flags & SAMPLE_OA_REPORT)
@@ -1524,8 +1553,10 @@ static int append_cs_buffer_sample(struct 
i915_perf_stream *stream,
if (sample_flags & SAMPLE_OA_SOURCE)
data.source = I915_PERF_SAMPLE_OA_SOURCE_CS;
 
-   if (sample_flags & SAMPLE_CTX_ID)
+   if (sample_flags & SAMPLE_CTX_ID) {
data.ctx_id = node->ctx_id;
+   stream->last_ctx_id = data.ctx_id;
+   }
 
return append_perf_sample(stream, buf, count, offset, &data);
 }
@@ -3838,6 +3869,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
dev_priv->perf.oa.ops.read = gen7_oa_read;
dev_priv->perf.oa.ops.oa_hw_tail_read =
gen7_oa_hw_tail_read;
+   dev_priv->perf.oa.ops.get_ctx_