[PATCH v3 00/14] Support for sustained capturing of GuC firmware logs

[PATCH v3 00/14] Support for sustained capturing of GuC firmware logs

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

GuC firmware log its debug messages into a Host-GuC shared memory buffer
and when the buffer is half full it sends a Flush interrupt to Host.
GuC firmware expects that while it is writing to 2nd half of the buffer,
first half would get consumed by Host and then get a flush completed
acknowledgement from Host, so that it does not end up doing any overwrite
causing loss of logs.
So far Flush interrupt wasn't enabled on Host side & User could capture the
contents/snapshot of log buffer through 'i915_guc_log_dump' debugfs iface.
But this couldn't meet couple of key requirements, especially of Validation,
first is to ensure capturing of all boot time logs even with high verbosity
level and second is to enable capturing of logs in a sustained manner like
for the entire duration of a workload.
Now Driver will enable Flush interrupt and on receving it, would copy the
contents of log buffer into its local buffer. The size of local buffer would
be big enough to contain multiple snapshots of the log buffer giving ample
time to User to pull boot time messages.
Have added a debugfs interface '/sys/kernel/debug/dri/guc_log' for User to
collect the logs. Availed relay framework to implement this interface, where
Driver will have to just use a relay API to store snapshots of GuC log buffer
in a buffer managed by relay. The relay buffer will be operated in a mode
where the old data, not yet collected by User, will be overwritten if buffer
becomes full. So the behavior exhibited will be equivalent to 'dmesg -c'.
Besides mmap method, through which User can directly access the relay
buffer contents, relay also supports the 'poll' method. Through the 'poll'
call on log file, User can come to know whenever a new snapshot of the log
buffer is taken by Driver, so can run in tandem with the Driver and thus
capture logs in a sustained/streaming manner, without any loss of data.

v2: Rebased to the latest drm-intel-nightly.

v3: Aligned with the modification of late debugfs registration, at the end of
    i915 Driver load. Did cleanup as per Tvrtko's review comments, added 3
    new patches to optimize the log-buffer flush interrupt handling, gather
    and report the logging related stats.

Akash Goel (8):
  drm/i915: New structure to contain GuC logging related fields
  drm/i915: Add low level set of routines for programming PM IER/IIR/IMR
    register set
  drm/i915: Add a relay backed debugfs interface for capturing GuC logs
  drm/i915: New module param to control the size of buffer used for
    storing GuC firmware logs
  drm/i915: Use uncached(WC) mapping for acessing the GuC log buffer
  drm/i915: New lock to serialize the Host2GuC actions
  drm/i915: Add stats for GuC log buffer flush interrupts
  drm/i915: Optimization to reduce the sampling time of GuC log buffer

Chris Wilson (1):
  drm/i915: Support to create write combined type vmaps

Sagar Arun Kamble (5):
  drm/i915: Add GuC ukernel logging related fields to fw interface file
  drm/i915: Support for GuC interrupts
  drm/i915: Handle log buffer flush interrupt event from GuC
  drm/i915: Forcefully flush GuC log buffer on reset
  drm/i915: Debugfs support for GuC logging control

 drivers/gpu/drm/i915/i915_debugfs.c        |  60 +++-
 drivers/gpu/drm/i915/i915_drv.c            |  15 +
 drivers/gpu/drm/i915/i915_drv.h            |   6 +-
 drivers/gpu/drm/i915/i915_gem.c            |  57 +++-
 drivers/gpu/drm/i915/i915_gem_dmabuf.c     |   2 +-
 drivers/gpu/drm/i915/i915_guc_submission.c | 434 ++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_irq.c            | 175 ++++++++++--
 drivers/gpu/drm/i915/i915_params.c         |   5 +
 drivers/gpu/drm/i915/i915_params.h         |   1 +
 drivers/gpu/drm/i915/i915_reg.h            |  11 +
 drivers/gpu/drm/i915/intel_drv.h           |   6 +
 drivers/gpu/drm/i915/intel_guc.h           |  28 +-
 drivers/gpu/drm/i915/intel_guc_fwif.h      |  64 +++++
 drivers/gpu/drm/i915/intel_guc_loader.c    |   6 +-
 drivers/gpu/drm/i915/intel_lrc.c           |   8 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c    |   6 +-
 16 files changed, 832 insertions(+), 52 deletions(-)

-- 
1.9.2

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

[PATCH 01/14] drm/i915: Add GuC ukernel logging related fields to fw interface file

[akash.goel]

From: Sagar Arun Kamble <sagar.a.kamble@intel.com>

The first page of the GuC log buffer contains state info or meta data
which is required to parse the logs contained in the subsequent pages.
The structure representing the state info is added to interface file
as Driver would need to handle log buffer flush interrupts from GuC.
Added an enum for the different message/event types that can be send
by the GuC ukernel to Host.
Also added 2 new Host to GuC action types to inform GuC when Host has
flushed the log buffer and forcefuly cause the GuC to send a new
log buffer flush interrupt.

v2: Make documentation of log buffer state structure more elaborate. (Tvrtko)
    Rename LOGBUFFERFLUSH action to LOG_BUFFER_FLUSH for consistency. (Tvrtko)

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h | 64 +++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 944786d..ad916b5 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -418,15 +418,73 @@ struct guc_ads {
 	u32 reserved2[4];
 } __packed;
 
+/* GuC logging structures */
+
+enum guc_log_buffer_type {
+	GUC_ISR_LOG_BUFFER,
+	GUC_DPC_LOG_BUFFER,
+	GUC_CRASH_DUMP_LOG_BUFFER,
+	GUC_MAX_LOG_BUFFER
+};
+
+/*
+ * Below state is used for coordination of retrieval of GuC firmware logs.
+ * read_ptr points to the location where i915 read last in log buffer and
+ * read only for GuC firmware. write_ptr is incremented by GuC with number
+ * of bytes written for each log entry and is read only for i915.
+ * When the log buffer becomes half full, GuC sends a flush interrupt.
+ * GuC firmware expects that while it is writing to 2nd half of the buffer,
+ * first half would get consumed by Host and then get a flush completed
+ * acknowledgement from Host, so that it does not end up doing any overwrite
+ * causing loss of logs. So when buffer gets half filled & i915 has requested
+ * for interrupt, GuC will set flush_to_file field, set the sampled_write_ptr
+ * to the value of write_ptr and raise the interrupt.
+ * On receving the interrupt i915 should read the buffer, clear flush_to_file
+ * field and also update read_ptr with the value of sample_write_ptr, before
+ * sending an acknowledgement to GuC. marker & version fields are for internal
+ * usage of GuC and opaque to i915. buffer_full_cnt field is incremented every
+ * time GuC detects the log buffer overflow.
+ * Separate state is maintained for each log buffer type.
+ */
+struct guc_log_buffer_state {
+	u32 marker[2];
+	u32 read_ptr;
+	u32 write_ptr;
+	u32 size;
+	u32 sampled_write_ptr;
+	union {
+		struct {
+			u32 flush_to_file:1;
+			u32 buffer_full_cnt:4;
+			u32 reserved:27;
+		};
+		u32 flags;
+	};
+	u32 version;
+} __packed;
+
+union guc_log_control {
+	struct {
+		u32 logging_enabled:1;
+		u32 reserved1:3;
+		u32 verbosity:4;
+		u32 reserved2:24;
+	};
+	u32 value;
+} __packed;
+
 /* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */
 enum host2guc_action {
 	HOST2GUC_ACTION_DEFAULT = 0x0,
 	HOST2GUC_ACTION_SAMPLE_FORCEWAKE = 0x6,
 	HOST2GUC_ACTION_ALLOCATE_DOORBELL = 0x10,
 	HOST2GUC_ACTION_DEALLOCATE_DOORBELL = 0x20,
+	HOST2GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE = 0x30,
+	HOST2GUC_ACTION_FORCE_LOG_BUFFER_FLUSH = 0x302,
 	HOST2GUC_ACTION_ENTER_S_STATE = 0x501,
 	HOST2GUC_ACTION_EXIT_S_STATE = 0x502,
 	HOST2GUC_ACTION_SLPC_REQUEST = 0x3003,
+	HOST2GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000,
 	HOST2GUC_ACTION_LIMIT
 };
 
@@ -448,4 +506,10 @@ enum guc2host_status {
 	GUC2HOST_STATUS_GENERIC_FAIL = GUC2HOST_STATUS(0x0000F000)
 };
 
+/* This action will be programmed in C1BC - SOFT_SCRATCH_15_REG */
+enum guc2host_message {
+	GUC2HOST_MSG_CRASH_DUMP_POSTED = (1 << 1),
+	GUC2HOST_MSG_FLUSH_LOG_BUFFER = (1 << 3)
+};
+
 #endif
-- 
1.9.2

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

[PATCH 02/14] drm/i915: New structure to contain GuC logging related fields

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

So far there were 2 fields related to GuC logs in 'intel_guc' structure.
For the support of capturing GuC logs & storing them in a local buffer,
multiple new fields would have to be added. This warrants a separate
structure to contain the fields related to GuC logging state.
Added a new structure 'intel_guc_log' and instance of it inside
'intel_guc' structure.

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |  2 +-
 drivers/gpu/drm/i915/i915_guc_submission.c | 10 +++++-----
 drivers/gpu/drm/i915/intel_guc.h           |  8 ++++++--
 drivers/gpu/drm/i915/intel_guc_loader.c    |  2 +-
 4 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5185e02..de197535 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2625,7 +2625,7 @@ static int i915_guc_log_dump(struct seq_file *m, void *data)
 	struct drm_info_node *node = m->private;
 	struct drm_device *dev = node->minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_i915_gem_object *log_obj = dev_priv->guc.log_obj;
+	struct drm_i915_gem_object *log_obj = dev_priv->guc.log.obj;
 	u32 *log;
 	int i = 0, pg;
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 28a810f..fedf82b 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -835,7 +835,7 @@ static void guc_create_log(struct intel_guc *guc)
 		GUC_LOG_ISR_PAGES + 1 +
 		GUC_LOG_CRASH_PAGES + 1) << PAGE_SHIFT;
 
-	obj = guc->log_obj;
+	obj = guc->log.obj;
 	if (!obj) {
 		obj = gem_allocate_guc_obj(dev_priv, size);
 		if (!obj) {
@@ -844,7 +844,7 @@ static void guc_create_log(struct intel_guc *guc)
 			return;
 		}
 
-		guc->log_obj = obj;
+		guc->log.obj = obj;
 	}
 
 	/* each allocated unit is a page */
@@ -854,7 +854,7 @@ static void guc_create_log(struct intel_guc *guc)
 		(GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT);
 
 	offset = i915_gem_obj_ggtt_offset(obj) >> PAGE_SHIFT; /* in pages */
-	guc->log_flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags;
+	guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags;
 }
 
 static void init_guc_policies(struct guc_policies *policies)
@@ -1015,8 +1015,8 @@ void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
 	gem_release_guc_obj(dev_priv->guc.ads_obj);
 	guc->ads_obj = NULL;
 
-	gem_release_guc_obj(dev_priv->guc.log_obj);
-	guc->log_obj = NULL;
+	gem_release_guc_obj(dev_priv->guc.log.obj);
+	guc->log.obj = NULL;
 
 	if (guc->ctx_pool_obj)
 		ida_destroy(&guc->ctx_ids);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 3e3e743..d52eca3 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -122,10 +122,14 @@ struct intel_guc_fw {
 	uint32_t ucode_offset;
 };
 
+struct intel_guc_log {
+	uint32_t flags;
+	struct drm_i915_gem_object *obj;
+};
+
 struct intel_guc {
 	struct intel_guc_fw guc_fw;
-	uint32_t log_flags;
-	struct drm_i915_gem_object *log_obj;
+	struct intel_guc_log log;
 
 	struct drm_i915_gem_object *ads_obj;
 
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index db3c897..8ef9b7f 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -173,7 +173,7 @@ static void set_guc_init_params(struct drm_i915_private *dev_priv)
 	params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
 			GUC_CTL_VCS2_ENABLED;
 
-	params[GUC_CTL_LOG_PARAMS] = guc->log_flags;
+	params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
 
 	if (i915.guc_log_level >= 0)
 		params[GUC_CTL_DEBUG] =
-- 
1.9.2

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

[PATCH 03/14] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

So far PM IER/IIR/IMR registers were being used only for Turbo related
interrupts. But interrupts coming from GuC also use the same set.
As a precursor to supporting GuC interrupts, added new low level routines
so as to allow sharing the programming of PM IER/IIR/IMR registers between
Turbo & GuC.
Also similar to PM IMR, maintaining a bitmask for PM IER register, to allow
easy sharing of it between Turbo & GuC without involving a rmw operation.

v2:
- For appropriateness & avoid any ambiguity, rename old functions
  enable/disable pm_irq to mask/unmask pm_irq and rename new functions
  enable/disable pm_interrupts to enable/disable pm_irq. (Tvrtko)
- Use u32 in place of uint32_t. (Tvrtko)

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/i915_irq.c         | 63 ++++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_drv.h        |  3 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c |  4 +--
 4 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9ef4919..85a7103 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1806,6 +1806,7 @@ struct drm_i915_private {
 	};
 	u32 gt_irq_mask;
 	u32 pm_irq_mask;
+	u32 pm_ier_mask;
 	u32 pm_rps_events;
 	u32 pipestat_irq_mask[I915_MAX_PIPES];
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4378a65..dd5ae6d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -314,7 +314,7 @@ static void snb_update_pm_irq(struct drm_i915_private *dev_priv,
 	}
 }
 
-void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+void gen6_unmask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
 {
 	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
 		return;
@@ -322,28 +322,62 @@ void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
 	snb_update_pm_irq(dev_priv, mask, mask);
 }
 
-static void __gen6_disable_pm_irq(struct drm_i915_private *dev_priv,
-				  uint32_t mask)
+static void __gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
 {
 	snb_update_pm_irq(dev_priv, mask, 0);
 }
 
-void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
 {
 	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
 		return;
 
-	__gen6_disable_pm_irq(dev_priv, mask);
+	__gen6_mask_pm_irq(dev_priv, mask);
 }
 
-void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+void gen6_reset_pm_irq(struct drm_i915_private *dev_priv, u32 reset_mask)
 {
 	i915_reg_t reg = gen6_pm_iir(dev_priv);
 
-	spin_lock_irq(&dev_priv->irq_lock);
-	I915_WRITE(reg, dev_priv->pm_rps_events);
-	I915_WRITE(reg, dev_priv->pm_rps_events);
+	assert_spin_locked(&dev_priv->irq_lock);
+
+	I915_WRITE(reg, reset_mask);
+	I915_WRITE(reg, reset_mask);
 	POSTING_READ(reg);
+}
+
+void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, u32 enable_mask)
+{
+	u32 new_val;
+
+	assert_spin_locked(&dev_priv->irq_lock);
+
+	new_val = dev_priv->pm_ier_mask;
+	new_val |= enable_mask;
+
+	dev_priv->pm_ier_mask = new_val;
+	I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+	gen6_unmask_pm_irq(dev_priv, enable_mask);
+}
+
+void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 disable_mask)
+{
+	u32 new_val;
+
+	assert_spin_locked(&dev_priv->irq_lock);
+
+	new_val = dev_priv->pm_ier_mask;
+	new_val &= ~disable_mask;
+
+	dev_priv->pm_ier_mask = new_val;
+	__gen6_mask_pm_irq(dev_priv, disable_mask);
+	I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+}
+
+void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+{
+	spin_lock_irq(&dev_priv->irq_lock);
+	gen6_reset_pm_irq(dev_priv, dev_priv->pm_rps_events);
 	dev_priv->rps.pm_iir = 0;
 	spin_unlock_irq(&dev_priv->irq_lock);
 }
@@ -355,8 +389,6 @@ void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv)
 	WARN_ON(dev_priv->rps.pm_iir);
 	WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) & dev_priv->pm_rps_events);
 	dev_priv->rps.interrupts_enabled = true;
-	I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) |
-				dev_priv->pm_rps_events);
 	gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
 
 	spin_unlock_irq(&dev_priv->irq_lock);
@@ -379,9 +411,7 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
 
 	I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
-	__gen6_disable_pm_irq(dev_priv, dev_priv->pm_rps_events);
-	I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) &
-				~dev_priv->pm_rps_events);
+	gen6_disable_pm_irq(dev_priv, dev_priv->pm_rps_events);
 
 	spin_unlock_irq(&dev_priv->irq_lock);
 
@@ -1094,7 +1124,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
 	pm_iir = dev_priv->rps.pm_iir;
 	dev_priv->rps.pm_iir = 0;
 	/* Make sure not to corrupt PMIMR state used by ringbuffer on GEN6 */
-	gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
+	gen6_unmask_pm_irq(dev_priv, dev_priv->pm_rps_events);
 	client_boost = dev_priv->rps.client_boost;
 	dev_priv->rps.client_boost = false;
 	spin_unlock_irq(&dev_priv->irq_lock);
@@ -1599,7 +1629,7 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
 {
 	if (pm_iir & dev_priv->pm_rps_events) {
 		spin_lock(&dev_priv->irq_lock);
-		gen6_disable_pm_irq(dev_priv, pm_iir & dev_priv->pm_rps_events);
+		gen6_mask_pm_irq(dev_priv, pm_iir & dev_priv->pm_rps_events);
 		if (dev_priv->rps.interrupts_enabled) {
 			dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events;
 			queue_work(dev_priv->wq, &dev_priv->rps.work);
@@ -3770,6 +3800,7 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 		gt_interrupts[0] |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
 
 	dev_priv->pm_irq_mask = 0xffffffff;
+	dev_priv->pm_ier_mask = 0x0;
 	GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
 	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
 	/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3156d8d..cfbc08d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1055,11 +1055,14 @@ void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
 /* i915_irq.c */
 void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
+void gen6_reset_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
 void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv);
 void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv);
 void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv);
+void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
+void gen6_unmask_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
 u32 gen6_sanitize_rps_pm_mask(struct drm_i915_private *dev_priv, u32 mask);
 void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable_interrupts(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 04a2d14..13544f8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1912,7 +1912,7 @@ hsw_vebox_get_irq(struct intel_engine_cs *engine)
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
 	if (engine->irq_refcount++ == 0) {
 		I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
-		gen6_enable_pm_irq(dev_priv, engine->irq_enable_mask);
+		gen6_unmask_pm_irq(dev_priv, engine->irq_enable_mask);
 	}
 	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
 
@@ -1928,7 +1928,7 @@ hsw_vebox_put_irq(struct intel_engine_cs *engine)
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
 	if (--engine->irq_refcount == 0) {
 		I915_WRITE_IMR(engine, ~0);
-		gen6_disable_pm_irq(dev_priv, engine->irq_enable_mask);
+		gen6_mask_pm_irq(dev_priv, engine->irq_enable_mask);
 	}
 	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
 }
-- 
1.9.2

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

[PATCH 04/14] drm/i915: Support for GuC interrupts

[akash.goel]

From: Sagar Arun Kamble <sagar.a.kamble@intel.com>

There are certain types of interrupts which Host can recieve from GuC.
GuC ukernel sends an interrupt to Host for certain events, like for
example retrieve/consume the logs generated by ukernel.
This patch adds support to receive interrupts from GuC but currently
enables & partially handles only the interrupt sent by GuC ukernel.
Future patches will add support for handling other interrupt types.

v2:
- Use common low level routines for PM IER/IIR programming (Chris)
- Rename interrupt functions to gen9_xxx from gen8_xxx (Chris)
- Replace disabling of wake ref asserts with rpm get/put (Chris)

v3:
- Update comments for more clarity. (Tvrtko)
- Remove the masking of GuC interrupt, which was kept masked till the
  start of bottom half, its not really needed as there is only a
  single instance of work item. (Tvrtko)

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |   1 +
 drivers/gpu/drm/i915/i915_guc_submission.c |   5 ++
 drivers/gpu/drm/i915/i915_irq.c            | 108 +++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h            |  11 +++
 drivers/gpu/drm/i915/intel_drv.h           |   3 +
 drivers/gpu/drm/i915/intel_guc.h           |   4 ++
 drivers/gpu/drm/i915/intel_guc_loader.c    |   4 ++
 7 files changed, 132 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 85a7103..20c701c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1808,6 +1808,7 @@ struct drm_i915_private {
 	u32 pm_irq_mask;
 	u32 pm_ier_mask;
 	u32 pm_rps_events;
+	u32 guc_events;
 	u32 pipestat_irq_mask[I915_MAX_PIPES];
 
 	struct i915_hotplug hotplug;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index fedf82b..b441951 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1038,6 +1038,8 @@ int intel_guc_suspend(struct drm_device *dev)
 	if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
 		return 0;
 
+	gen9_disable_guc_interrupts(dev_priv);
+
 	ctx = dev_priv->kernel_context;
 
 	data[0] = HOST2GUC_ACTION_ENTER_S_STATE;
@@ -1064,6 +1066,9 @@ int intel_guc_resume(struct drm_device *dev)
 	if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
 		return 0;
 
+	if (i915.guc_log_level >= 0)
+		gen9_enable_guc_interrupts(dev_priv);
+
 	ctx = dev_priv->kernel_context;
 
 	data[0] = HOST2GUC_ACTION_EXIT_S_STATE;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index dd5ae6d..aefe1a1 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -170,6 +170,7 @@ static void gen5_assert_iir_is_zero(struct drm_i915_private *dev_priv,
 } while (0)
 
 static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
+static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
 
 /* For display hotplug interrupt */
 static inline void
@@ -418,6 +419,42 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
 	synchronize_irq(dev_priv->dev->irq);
 }
 
+void gen9_reset_guc_interrupts(struct drm_i915_private *dev_priv)
+{
+	spin_lock_irq(&dev_priv->irq_lock);
+	gen6_reset_pm_irq(dev_priv, dev_priv->guc_events);
+	spin_unlock_irq(&dev_priv->irq_lock);
+}
+
+void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv)
+{
+	spin_lock_irq(&dev_priv->irq_lock);
+	if (!dev_priv->guc.interrupts_enabled) {
+		WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) &
+					dev_priv->guc_events);
+		dev_priv->guc.interrupts_enabled = true;
+		gen6_enable_pm_irq(dev_priv, dev_priv->guc_events);
+	}
+	spin_unlock_irq(&dev_priv->irq_lock);
+}
+
+void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv)
+{
+	spin_lock_irq(&dev_priv->irq_lock);
+	dev_priv->guc.interrupts_enabled = false;
+	spin_unlock_irq(&dev_priv->irq_lock);
+
+	cancel_work_sync(&dev_priv->guc.events_work);
+
+	spin_lock_irq(&dev_priv->irq_lock);
+
+	gen6_disable_pm_irq(dev_priv, dev_priv->guc_events);
+
+	spin_unlock_irq(&dev_priv->irq_lock);
+
+	synchronize_irq(dev_priv->dev->irq);
+}
+
 /**
  * bdw_update_port_irq - update DE port interrupt
  * @dev_priv: driver private
@@ -1192,6 +1229,48 @@ out:
 	ENABLE_RPM_WAKEREF_ASSERTS(dev_priv);
 }
 
+static void gen9_guc2host_events_work(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, struct drm_i915_private, guc.events_work);
+	u32 msg;
+
+	/* Though this work item gets synced during rpm suspend, but still need
+	 * a rpm get/put to avoid the warning, as it could get executed in a
+	 * window, where rpm ref count has dropped to zero but rpm suspend has
+	 * not kicked in. Generally device is expected to be active only at this
+	 * time so get/put should be really quick.
+	 */
+	intel_runtime_pm_get(dev_priv);
+
+	spin_lock_irq(&dev_priv->irq_lock);
+	/* Speed up work cancellation during disabling guc interrupts. */
+	if (!dev_priv->guc.interrupts_enabled) {
+		spin_unlock_irq(&dev_priv->irq_lock);
+		intel_runtime_pm_put(dev_priv);
+		return;
+	}
+	spin_unlock_irq(&dev_priv->irq_lock);
+
+	msg = I915_READ(SOFT_SCRATCH(15));
+	if (msg & (GUC2HOST_MSG_CRASH_DUMP_POSTED |
+		   GUC2HOST_MSG_FLUSH_LOG_BUFFER)) {
+		/* TODO: Handle the events for which GuC interrupted host */
+
+		/* Clear GuC to Host msg bits that are handled */
+		if (msg & GUC2HOST_MSG_FLUSH_LOG_BUFFER)
+			I915_WRITE(SOFT_SCRATCH(15),
+				I915_READ(SOFT_SCRATCH(15)) &
+				~GUC2HOST_MSG_FLUSH_LOG_BUFFER);
+
+		if (msg & GUC2HOST_MSG_CRASH_DUMP_POSTED)
+			I915_WRITE(SOFT_SCRATCH(15),
+				I915_READ(SOFT_SCRATCH(15)) &
+				~GUC2HOST_MSG_CRASH_DUMP_POSTED);
+	}
+
+	intel_runtime_pm_put(dev_priv);
+}
 
 /**
  * ivybridge_parity_work - Workqueue called when a parity error interrupt
@@ -1367,11 +1446,13 @@ static irqreturn_t gen8_gt_irq_ack(struct drm_i915_private *dev_priv,
 			DRM_ERROR("The master control interrupt lied (GT3)!\n");
 	}
 
-	if (master_ctl & GEN8_GT_PM_IRQ) {
+	if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) {
 		gt_iir[2] = I915_READ_FW(GEN8_GT_IIR(2));
-		if (gt_iir[2] & dev_priv->pm_rps_events) {
+		if (gt_iir[2] & (dev_priv->pm_rps_events |
+				 dev_priv->guc_events)) {
 			I915_WRITE_FW(GEN8_GT_IIR(2),
-				      gt_iir[2] & dev_priv->pm_rps_events);
+				      gt_iir[2] & (dev_priv->pm_rps_events |
+						   dev_priv->guc_events));
 			ret = IRQ_HANDLED;
 		} else
 			DRM_ERROR("The master control interrupt lied (PM)!\n");
@@ -1403,6 +1484,9 @@ static void gen8_gt_irq_handler(struct drm_i915_private *dev_priv,
 
 	if (gt_iir[2] & dev_priv->pm_rps_events)
 		gen6_rps_irq_handler(dev_priv, gt_iir[2]);
+
+	if (gt_iir[2] & dev_priv->guc_events)
+		gen9_guc_irq_handler(dev_priv, gt_iir[2]);
 }
 
 static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
@@ -1649,6 +1733,18 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
 	}
 }
 
+static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 gt_iir)
+{
+	if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT) {
+		spin_lock(&dev_priv->irq_lock);
+		if (dev_priv->guc.interrupts_enabled) {
+			/* Process all the GuC to Host events in bottom half. */
+			queue_work(dev_priv->wq, &dev_priv->guc.events_work);
+		}
+		spin_unlock(&dev_priv->irq_lock);
+	}
+}
+
 static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv,
 				     enum pipe pipe)
 {
@@ -3805,7 +3901,7 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
 	/*
 	 * RPS interrupts will get enabled/disabled on demand when RPS itself
-	 * is enabled/disabled.
+	 * is enabled/disabled. Same wil be the case for GuC interrupts.
 	 */
 	GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_irq_mask, 0);
 	GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
@@ -4590,6 +4686,10 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 
 	INIT_WORK(&dev_priv->rps.work, gen6_pm_rps_work);
 	INIT_WORK(&dev_priv->l3_parity.error_work, ivybridge_parity_work);
+	INIT_WORK(&dev_priv->guc.events_work, gen9_guc2host_events_work);
+
+	if (HAS_GUC_UCODE(dev))
+		dev_priv->guc_events = GEN9_GUC_TO_HOST_INT_EVENT;
 
 	/* Let's track the enabled rps events */
 	if (IS_VALLEYVIEW(dev_priv))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1c8d029..157458f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5963,6 +5963,7 @@ enum {
 #define  GEN8_DE_PIPE_A_IRQ		(1<<16)
 #define  GEN8_DE_PIPE_IRQ(pipe)		(1<<(16+(pipe)))
 #define  GEN8_GT_VECS_IRQ		(1<<6)
+#define  GEN8_GT_GUC_IRQ		(1<<5)
 #define  GEN8_GT_PM_IRQ			(1<<4)
 #define  GEN8_GT_VCS2_IRQ		(1<<3)
 #define  GEN8_GT_VCS1_IRQ		(1<<2)
@@ -5974,6 +5975,16 @@ enum {
 #define GEN8_GT_IIR(which) _MMIO(0x44308 + (0x10 * (which)))
 #define GEN8_GT_IER(which) _MMIO(0x4430c + (0x10 * (which)))
 
+#define GEN9_GUC_TO_HOST_INT_EVENT	(1<<31)
+#define GEN9_GUC_EXEC_ERROR_EVENT	(1<<30)
+#define GEN9_GUC_DISPLAY_EVENT		(1<<29)
+#define GEN9_GUC_SEMA_SIGNAL_EVENT	(1<<28)
+#define GEN9_GUC_IOMMU_MSG_EVENT	(1<<27)
+#define GEN9_GUC_DB_RING_EVENT		(1<<26)
+#define GEN9_GUC_DMA_DONE_EVENT		(1<<25)
+#define GEN9_GUC_FATAL_ERROR_EVENT	(1<<24)
+#define GEN9_GUC_NOTIFICATION_EVENT	(1<<23)
+
 #define GEN8_RCS_IRQ_SHIFT 0
 #define GEN8_BCS_IRQ_SHIFT 16
 #define GEN8_VCS1_IRQ_SHIFT 0
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cfbc08d..0feef3b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1080,6 +1080,9 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
 				     unsigned int pipe_mask);
 void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
 				     unsigned int pipe_mask);
+void gen9_reset_guc_interrupts(struct drm_i915_private *dev_priv);
+void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv);
+void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv);
 
 /* intel_crt.c */
 void intel_crt_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index d52eca3..2663b41 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -131,6 +131,10 @@ struct intel_guc {
 	struct intel_guc_fw guc_fw;
 	struct intel_guc_log log;
 
+	/* GuC2Host interrupt related state */
+	struct work_struct events_work;
+	bool interrupts_enabled;
+
 	struct drm_i915_gem_object *ads_obj;
 
 	struct drm_i915_gem_object *ctx_pool_obj;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 8ef9b7f..43e7c1c 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -448,6 +448,7 @@ int intel_guc_setup(struct drm_device *dev)
 	}
 
 	direct_interrupts_to_host(dev_priv);
+	gen9_reset_guc_interrupts(dev_priv);
 
 	guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING;
 
@@ -494,6 +495,9 @@ int intel_guc_setup(struct drm_device *dev)
 		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
 
 	if (i915.enable_guc_submission) {
+		if (i915.guc_log_level >= 0)
+			gen9_enable_guc_interrupts(dev_priv);
+
 		err = i915_guc_submission_enable(dev_priv);
 		if (err)
 			goto fail;
-- 
1.9.2

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

[PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

[akash.goel]

From: Sagar Arun Kamble <sagar.a.kamble@intel.com>

GuC ukernel sends an interrupt to Host to flush the log buffer
and expects Host to correspondingly update the read pointer
information in the state structure, once it has consumed the
log buffer contents by copying them to a file or buffer.
Even if Host couldn't copy the contents, it can still update the
read pointer so that logging state is not disturbed on GuC side.

v2:
- Use a dedicated workqueue for handling flush interrupt. (Tvrtko)
- Reduce the overall log buffer copying time by skipping the copy of
  crash buffer area for regular cases and copying only the state
  structure data in first page.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c            | 13 +++++
 drivers/gpu/drm/i915/i915_guc_submission.c | 87 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_irq.c            |  6 ++-
 drivers/gpu/drm/i915/intel_guc.h           |  2 +
 4 files changed, 106 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b98afbd..ec3721c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1174,8 +1174,20 @@ static int i915_workqueues_init(struct drm_i915_private *dev_priv)
 	if (dev_priv->gpu_error.hangcheck_wq == NULL)
 		goto out_free_dp_wq;
 
+	if (HAS_GUC_SCHED(dev_priv)) {
+		/* Need a dedicated wq to process log buffer flush interrupts
+		 * from GuC without much delay so as to avoid any loss of logs.
+		 */
+		dev_priv->guc.log.wq =
+			alloc_ordered_workqueue("i915-guc_log", 0);
+		if (dev_priv->guc.log.wq == NULL)
+			goto out_free_hangcheck_wq;
+	}
+
 	return 0;
 
+out_free_hangcheck_wq:
+	destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
 out_free_dp_wq:
 	destroy_workqueue(dev_priv->hotplug.dp_wq);
 out_free_wq:
@@ -1188,6 +1200,7 @@ out_err:
 
 static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
 {
+	destroy_workqueue(dev_priv->guc.log.wq);
 	destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
 	destroy_workqueue(dev_priv->hotplug.dp_wq);
 	destroy_workqueue(dev_priv->wq);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b441951..425226e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -166,6 +166,15 @@ static int host2guc_sample_forcewake(struct intel_guc *guc,
 	return host2guc_action(guc, data, ARRAY_SIZE(data));
 }
 
+static int host2guc_logbuffer_flush_complete(struct intel_guc *guc)
+{
+	u32 data[1];
+
+	data[0] = HOST2GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE;
+
+	return host2guc_action(guc, data, 1);
+}
+
 /*
  * Initialise, update, or clear doorbell data shared with the GuC
  *
@@ -819,6 +828,75 @@ err:
 	return NULL;
 }
 
+static void* guc_get_write_buffer(struct intel_guc *guc)
+{
+	return NULL;
+}
+
+static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_guc *guc = &dev_priv->guc;
+	struct guc_log_buffer_state *log_buffer_state;
+	struct guc_log_buffer_state *log_buffer_copy_state;
+	void *src_ptr, *dst_ptr;
+	u32 num_pages_to_copy;
+	int i;
+
+	if (!guc->log.obj)
+		return;
+
+	num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
+	/* Don't really need to copy crash buffer area in regular cases as there
+	 * won't be any unread data there.
+	 */
+	if (!capture_all)
+		num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
+
+	log_buffer_state = src_ptr =
+		kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));
+
+	/* Get the pointer to local buffer to store the logs */
+	dst_ptr = log_buffer_copy_state = guc_get_write_buffer(guc);
+
+	for (i = 0; i < GUC_MAX_LOG_BUFFER; i++) {
+		if (log_buffer_copy_state) {
+			memcpy(log_buffer_copy_state, log_buffer_state,
+					sizeof(struct guc_log_buffer_state));
+
+			/* The write pointer could have been updated by the GuC
+			 * firmware, after sending the flush interrupt to Host,
+			 * for consistency set the write pointer value to same
+			 * value of sampled_write_ptr in the snapshot buffer.
+			 */
+			log_buffer_copy_state->write_ptr =
+				log_buffer_copy_state->sampled_write_ptr;
+
+			log_buffer_copy_state++;
+		}
+
+		/* FIXME: invalidate/flush for log buffer needed */
+
+		/* Update the read pointer in the shared log buffer */
+		log_buffer_state->read_ptr =
+			log_buffer_state->sampled_write_ptr;
+
+		/* Clear the 'flush to file' flag */
+		log_buffer_state->flush_to_file = 0;
+		log_buffer_state++;
+	}
+
+	kunmap_atomic(src_ptr);
+
+	/* Now copy the actual logs */
+	for (i=1; (i < num_pages_to_copy) && dst_ptr; i++) {
+		dst_ptr += PAGE_SIZE;
+		src_ptr = kmap_atomic(i915_gem_object_get_page(guc->log.obj, i));
+		memcpy(dst_ptr, src_ptr, PAGE_SIZE);
+		kunmap_atomic(src_ptr);
+	}
+}
+
 static void guc_create_log(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -1078,3 +1156,12 @@ int intel_guc_resume(struct drm_device *dev)
 
 	return host2guc_action(guc, data, ARRAY_SIZE(data));
 }
+
+void i915_guc_capture_logs(struct drm_device *dev, bool capture_all)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	guc_read_update_log_buffer(dev, capture_all);
+
+	host2guc_logbuffer_flush_complete(&dev_priv->guc);
+}
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index aefe1a1..7b8894c1 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1255,7 +1255,8 @@ static void gen9_guc2host_events_work(struct work_struct *work)
 	msg = I915_READ(SOFT_SCRATCH(15));
 	if (msg & (GUC2HOST_MSG_CRASH_DUMP_POSTED |
 		   GUC2HOST_MSG_FLUSH_LOG_BUFFER)) {
-		/* TODO: Handle the events for which GuC interrupted host */
+		i915_guc_capture_logs(dev_priv->dev,
+			msg & GUC2HOST_MSG_CRASH_DUMP_POSTED);
 
 		/* Clear GuC to Host msg bits that are handled */
 		if (msg & GUC2HOST_MSG_FLUSH_LOG_BUFFER)
@@ -1739,7 +1740,8 @@ static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 gt_iir)
 		spin_lock(&dev_priv->irq_lock);
 		if (dev_priv->guc.interrupts_enabled) {
 			/* Process all the GuC to Host events in bottom half. */
-			queue_work(dev_priv->wq, &dev_priv->guc.events_work);
+			queue_work(dev_priv->guc.log.wq,
+					&dev_priv->guc.events_work);
 		}
 		spin_unlock(&dev_priv->irq_lock);
 	}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 2663b41..71aab85 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -125,6 +125,7 @@ struct intel_guc_fw {
 struct intel_guc_log {
 	uint32_t flags;
 	struct drm_i915_gem_object *obj;
+	struct workqueue_struct *wq;
 };
 
 struct intel_guc {
@@ -171,5 +172,6 @@ int i915_guc_wq_check_space(struct drm_i915_gem_request *rq);
 int i915_guc_submit(struct drm_i915_gem_request *rq);
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
+void i915_guc_capture_logs(struct drm_device *dev, bool capture_all);
 
 #endif
-- 
1.9.2

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

[PATCH 06/14] drm/i915: Add a relay backed debugfs interface for capturing GuC logs

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

Added a new debugfs interface '/sys/kernel/debug/dri/guc_log' for the
User to capture GuC firmware logs. Availed relay framework to implement
the interface, where Driver will have to just use a relay API to store
snapshots of the GuC log buffer in the buffer managed by relay.
The snapshot will be taken when GuC firmware sends a log buffer flush
interrupt and up to four snaphots could be stored in the relay buffer.
The relay buffer will be operated in a mode where it will overwrite the
data not yet collected by User.
Besides mmap method, through which User can directly access the relay
buffer contents, relay also supports the 'poll' method. Through the 'poll'
call on log file, User can come to know whenever a new snapshot of the
log buffer is taken by Driver, so can run in tandem with the Driver and
capture the logs in a sustained/streaming manner, without any loss of data.

v2: Defer the creation of relay channel & associated debugfs file, as
    debugfs setup is now done at the end of i915 Driver load. (Chris)

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Sourab Gupta <sourab.gupta@intel.com>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c            |   2 +
 drivers/gpu/drm/i915/i915_guc_submission.c | 179 ++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_guc.h           |   3 +
 3 files changed, 183 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ec3721c..7a3d821 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1544,6 +1544,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	/* Reveal our presence to userspace */
 	if (drm_dev_register(dev, 0) == 0) {
 		i915_debugfs_register(dev_priv);
+		i915_guc_register(dev);
 		i915_setup_sysfs(dev);
 	} else
 		DRM_ERROR("Failed to register driver for userspace access!\n");
@@ -1582,6 +1583,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	intel_opregion_unregister(dev_priv);
 
 	i915_teardown_sysfs(dev_priv->dev);
+	i915_guc_unregister(dev_priv->dev);
 	i915_debugfs_unregister(dev_priv);
 	drm_dev_unregister(dev_priv->dev);
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 425226e..f3831b9 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -23,6 +23,8 @@
  */
 #include <linux/firmware.h>
 #include <linux/circ_buf.h>
+#include <linux/debugfs.h>
+#include <linux/relay.h>
 #include "i915_drv.h"
 #include "intel_guc.h"
 
@@ -830,7 +832,16 @@ err:
 
 static void* guc_get_write_buffer(struct intel_guc *guc)
 {
-	return NULL;
+	/* FIXME: Cover the check under a lock ? */
+	if (!guc->log.relay_chan)
+		return NULL;
+
+	/* Get the pointer to relay sub buffer and copy data into it ourselves.
+	 * Could have used the relay_write() to indirectly copy the data, but
+	 * that would have been bit convoluted, as we also need to update the
+	 * first page containing state data.
+	 */
+	return relay_reserve(guc->log.relay_chan, guc->log.obj->base.size);
 }
 
 static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
@@ -897,6 +908,152 @@ static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
 	}
 }
 
+/*
+ * Sub buffer switch callback. If this callback is not implemented
+ * relay will operate in non-overwrite mode so will stop accepting
+ * new data if there are no empty sub buffers left.
+ */
+static int subbuf_start_callback(struct rchan_buf *buf,
+				 void *subbuf,
+				 void *prev_subbuf,
+				 size_t prev_padding)
+{
+	/* Always switch to next sub buffer as we don't mind overwriting of
+	 * old data/logs.
+	 */
+	return 1;
+}
+
+/*
+ * file_create() callback. Creates relay file in debugfs.
+ */
+static struct dentry *create_buf_file_callback(const char *filename,
+					       struct dentry *parent,
+					       umode_t mode,
+					       struct rchan_buf *buf,
+					       int *is_global)
+{
+	/*
+	 * Not using the channel filename passed as an argument, since for each
+	 * channel relay appends the corresponding CPU number to the filename
+	 * passed in relay_open(). This should be fine as relay just needs a
+	 * dentry of the file associated with the channel buffer and that file's
+	 * name need not be same as the filename passed as an argument.
+	 */
+	struct dentry *buf_file = debugfs_create_file("guc_log", mode,
+			parent, buf, &relay_file_operations);
+
+	/* This to enable the use of a single buffer for the relay channel and
+	 * correspondingly have a single file exposed to User, through which
+	 * it can pull the logs in order without any post-processing.
+	 */
+	*is_global = 1;
+
+	return buf_file;
+}
+
+/*
+ * file_remove() default callback. Removes relay file in debugfs.
+ */
+static int remove_buf_file_callback(struct dentry *dentry)
+{
+	debugfs_remove(dentry);
+	return 0;
+}
+
+/* relay channel callbacks */
+static struct rchan_callbacks relay_callbacks = {
+	.subbuf_start = subbuf_start_callback,
+	.create_buf_file = create_buf_file_callback,
+	.remove_buf_file = remove_buf_file_callback,
+};
+
+static void guc_remove_log_relay_file(struct intel_guc *guc)
+{
+	relay_close(guc->log.relay_chan);
+}
+
+static int guc_create_log_relay_file(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct drm_device *dev = dev_priv->dev;
+	struct dentry *log_dir;
+	struct rchan *guc_log_relay_chan;
+	size_t n_subbufs, subbuf_size;
+
+	if (guc->log.relay_chan)
+		return 0;
+
+	/* If /sys/kernel/debug/dri/0 location do not exist, then debugfs is
+	 * not mounted and so can't create the relay file.
+	 * The relay API seems to fit well with debugfs only.
+	 */
+	if (!dev->primary->debugfs_root)
+		return -ENODEV;
+
+	/* For now create the log file in /sys/kernel/debug/dri/0 dir */
+	log_dir = dev->primary->debugfs_root;
+
+	/* Keep the size of sub buffers same as shared log buffer */
+	subbuf_size = guc->log.obj->base.size;
+	/* TODO: Decide based on the User's input */
+	n_subbufs = 4;
+
+	guc_log_relay_chan = relay_open("guc_log", log_dir,
+			subbuf_size, n_subbufs, &relay_callbacks, dev);
+
+	if (!guc_log_relay_chan) {
+		DRM_DEBUG_DRIVER("Couldn't create relay chan for guc logs\n");
+		return -ENOMEM;
+	}
+
+	/* FIXME: Cover the update under a lock ? */
+	guc->log.relay_chan = guc_log_relay_chan;
+	return 0;
+}
+
+static void guc_log_cleanup(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_guc *guc = &dev_priv->guc;
+
+	lockdep_assert_held(&dev->struct_mutex);
+
+	if (i915.guc_log_level < 0)
+		return;
+
+	/* First disable the flush interrupt */
+	gen9_disable_guc_interrupts(dev_priv);
+
+	guc_remove_log_relay_file(guc);
+}
+
+static int guc_log_late_setup(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_guc *guc = &dev_priv->guc;
+	int ret;
+
+	lockdep_assert_held(&dev->struct_mutex);
+
+	if (i915.guc_log_level < 0)
+		return -EINVAL;
+
+	if (WARN_ON(guc->log.relay_chan))
+		return -EINVAL;
+
+	ret = guc_create_log_relay_file(guc);
+	if (ret)
+		goto err;
+
+	return 0;
+err:
+	/* logging will remain off */
+	i915.guc_log_level = -1;
+	gen9_disable_guc_interrupts(dev_priv);
+	return ret;
+}
+
 static void guc_create_log(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -1165,3 +1322,23 @@ void i915_guc_capture_logs(struct drm_device *dev, bool capture_all)
 
 	host2guc_logbuffer_flush_complete(&dev_priv->guc);
 }
+
+void i915_guc_unregister(struct drm_device *dev)
+{
+	if (!i915.enable_guc_submission)
+		return;
+
+	mutex_lock(&dev->struct_mutex);
+	guc_log_cleanup(dev);
+	mutex_unlock(&dev->struct_mutex);
+}
+
+void i915_guc_register(struct drm_device *dev)
+{
+	if (!i915.enable_guc_submission)
+		return;
+
+	mutex_lock(&dev->struct_mutex);
+	guc_log_late_setup(dev);
+	mutex_unlock(&dev->struct_mutex);
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 71aab85..73c9fdf 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -126,6 +126,7 @@ struct intel_guc_log {
 	uint32_t flags;
 	struct drm_i915_gem_object *obj;
 	struct workqueue_struct *wq;
+	struct rchan *relay_chan;
 };
 
 struct intel_guc {
@@ -173,5 +174,7 @@ int i915_guc_submit(struct drm_i915_gem_request *rq);
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
 void i915_guc_capture_logs(struct drm_device *dev, bool capture_all);
+void i915_guc_register(struct drm_device *dev);
+void i915_guc_unregister(struct drm_device *dev);
 
 #endif
-- 
1.9.2

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

[PATCH 07/14] drm/i915: Forcefully flush GuC log buffer on reset

[akash.goel]

From: Sagar Arun Kamble <sagar.a.kamble@intel.com>

If GuC logs are being captured, there should be a force log buffer flush
action sent to GuC before proceeding with GPU reset and re-initializing
GUC. Those logs would be useful to understand why the GPU reset was
initiated.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 32 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_irq.c            |  2 ++
 drivers/gpu/drm/i915/intel_guc.h           |  1 +
 3 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index f3831b9..86c2381 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -177,6 +177,16 @@ static int host2guc_logbuffer_flush_complete(struct intel_guc *guc)
 	return host2guc_action(guc, data, 1);
 }
 
+static int host2guc_force_logbuffer_flush(struct intel_guc *guc)
+{
+	u32 data[2];
+
+	data[0] = HOST2GUC_ACTION_FORCE_LOG_BUFFER_FLUSH;
+	data[1] = 0;
+
+	return host2guc_action(guc, data, 2);
+}
+
 /*
  * Initialise, update, or clear doorbell data shared with the GuC
  *
@@ -1323,6 +1333,28 @@ void i915_guc_capture_logs(struct drm_device *dev, bool capture_all)
 	host2guc_logbuffer_flush_complete(&dev_priv->guc);
 }
 
+void i915_guc_capture_logs_on_reset(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	mutex_lock(&dev->struct_mutex);
+
+	if (!i915.enable_guc_submission || (i915.guc_log_level < 0))
+		goto end;
+
+	/* First disable the interrupts, will be renabled after reset */
+	gen9_disable_guc_interrupts(dev_priv);
+
+	/* Ask GuC to update the log buffer state */
+	host2guc_force_logbuffer_flush(&dev_priv->guc);
+
+	/* GuC would have updated the log buffer by now, so capture it */
+	i915_guc_capture_logs(dev, true);
+
+end:
+	mutex_unlock(&dev->struct_mutex);
+}
+
 void i915_guc_unregister(struct drm_device *dev)
 {
 	if (!i915.enable_guc_submission)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7b8894c1..f1cf36e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2683,6 +2683,8 @@ static void i915_reset_and_wakeup(struct drm_i915_private *dev_priv)
 		 */
 		intel_runtime_pm_get(dev_priv);
 
+		i915_guc_capture_logs_on_reset(dev_priv->dev);
+
 		intel_prepare_reset(dev_priv);
 
 		/*
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 73c9fdf..445c9e8 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -174,6 +174,7 @@ int i915_guc_submit(struct drm_i915_gem_request *rq);
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
 void i915_guc_capture_logs(struct drm_device *dev, bool capture_all);
+void i915_guc_capture_logs_on_reset(struct drm_device *dev);
 void i915_guc_register(struct drm_device *dev);
 void i915_guc_unregister(struct drm_device *dev);
 
-- 
1.9.2

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

[PATCH 08/14] drm/i915: Debugfs support for GuC logging control

[akash.goel]

From: Sagar Arun Kamble <sagar.a.kamble@intel.com>

This patch provides debugfs interface i915_guc_output_control for
on the fly enabling/disabling of logging in GuC firmware and controlling
the verbosity level of logs.
The value written to the file, should have bit 0 set to enable logging and
bits 4-7 should contain the verbosity info.

v2: Add a forceful flush, to collect left over logs, on disabling logging.
    Useful for Validation.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 32 ++++++++++++++++-
 drivers/gpu/drm/i915/i915_guc_submission.c | 57 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h           |  1 +
 3 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index de197535..16aeab2 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2648,6 +2648,35 @@ static int i915_guc_log_dump(struct seq_file *m, void *data)
 	return 0;
 }
 
+static int
+i915_guc_log_control_set(void *data, u64 val)
+{
+	struct drm_device *dev = data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	if (!i915.enable_guc_submission || !dev_priv->guc.log.obj) {
+		ret = -EINVAL;
+		goto end;
+	}
+
+	intel_runtime_pm_get(dev_priv);
+	ret = i915_guc_log_control(dev, val);
+	intel_runtime_pm_put(dev_priv);
+
+end:
+	mutex_unlock(&dev->struct_mutex);
+	return ret;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_control_fops,
+			NULL, i915_guc_log_control_set,
+			"0x%08llx\n");
+
 static int i915_edp_psr_status(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = m->private;
@@ -5510,7 +5539,8 @@ static const struct i915_debugfs_files {
 	{"i915_fbc_false_color", &i915_fbc_fc_fops},
 	{"i915_dp_test_data", &i915_displayport_test_data_fops},
 	{"i915_dp_test_type", &i915_displayport_test_type_fops},
-	{"i915_dp_test_active", &i915_displayport_test_active_fops}
+	{"i915_dp_test_active", &i915_displayport_test_active_fops},
+	{"i915_guc_log_control", &i915_guc_log_control_fops}
 };
 
 void intel_display_crc_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 86c2381..5b44976 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -187,6 +187,16 @@ static int host2guc_force_logbuffer_flush(struct intel_guc *guc)
 	return host2guc_action(guc, data, 2);
 }
 
+static int host2guc_logging_control(struct intel_guc *guc, u32 control_val)
+{
+	u32 data[2];
+
+	data[0] = HOST2GUC_ACTION_UK_LOG_ENABLE_LOGGING;
+	data[1] = control_val;
+
+	return host2guc_action(guc, data, 2);
+}
+
 /*
  * Initialise, update, or clear doorbell data shared with the GuC
  *
@@ -1374,3 +1384,50 @@ void i915_guc_register(struct drm_device *dev)
 	guc_log_late_setup(dev);
 	mutex_unlock(&dev->struct_mutex);
 }
+
+int i915_guc_log_control(struct drm_device *dev, uint64_t control_val)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	union guc_log_control log_param;
+	int ret;
+
+	log_param.logging_enabled = control_val & 0x1;
+	log_param.verbosity = (control_val >> 4) & 0xF;
+
+	if (log_param.verbosity < GUC_LOG_VERBOSITY_MIN ||
+	    log_param.verbosity > GUC_LOG_VERBOSITY_MAX)
+		return -EINVAL;
+
+	/* This combination doesn't make sense & won't have any effect */
+	if (!log_param.logging_enabled && (i915.guc_log_level < 0))
+		return -EINVAL;
+
+	ret = host2guc_logging_control(&dev_priv->guc, log_param.value);
+	if (ret < 0) {
+		DRM_DEBUG_DRIVER("host2guc action failed\n");
+		return ret;
+	}
+
+	i915.guc_log_level = log_param.verbosity;
+
+	/* If log_level was set as -1 at boot time, then the relay channel file
+	 * wouldn't have been created by now and interrupts also would not have
+	 * been enabled.
+	 */
+	if (!dev_priv->guc.log.relay_chan) {
+		ret = guc_log_late_setup(dev_priv->dev);
+		if (!ret)
+			gen9_enable_guc_interrupts(dev_priv);
+	} else if (!log_param.logging_enabled) {
+		/* Once logging is disabled, GuC won't generate logs & send an
+		 * interrupt. But there could be some data in the log buffer
+		 * which is yet to be captured. So request GuC to update the log
+		 * buffer state and send the flush interrupt so that Host can
+		 * collect the left over logs also.
+		 */
+		flush_work(&dev_priv->guc.events_work);
+		host2guc_force_logbuffer_flush(&dev_priv->guc);
+	}
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 445c9e8..3e90279 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -177,5 +177,6 @@ void i915_guc_capture_logs(struct drm_device *dev, bool capture_all);
 void i915_guc_capture_logs_on_reset(struct drm_device *dev);
 void i915_guc_register(struct drm_device *dev);
 void i915_guc_unregister(struct drm_device *dev);
+int i915_guc_log_control(struct drm_device *dev, uint64_t control_val);
 
 #endif
-- 
1.9.2

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

[PATCH 09/14] drm/i915: New module param to control the size of buffer used for storing GuC firmware logs

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

On recieving the log buffer flush interrupt from GuC firmware, Driver
stores the snapshot of the log buffer in a local buffer, from which
Userspace can pull the logs. By default Driver store, up to, 4 snapshots
of the log buffer in a local buffer (managed by relay).
Added a new module (read only) param, 'guc_log_size', through which User
can specify the number of snapshots of log buffer to be stored in local
buffer. This can be used to ensure capturing of all boot time logs even
with high verbosity level.

v2: Rename module param to more apt name 'guc_log_buffer_nr'. (Nikula)

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 3 +--
 drivers/gpu/drm/i915/i915_params.c         | 5 +++++
 drivers/gpu/drm/i915/i915_params.h         | 1 +
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 5b44976..ec68a6d 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1016,8 +1016,7 @@ static int guc_create_log_relay_file(struct intel_guc *guc)
 
 	/* Keep the size of sub buffers same as shared log buffer */
 	subbuf_size = guc->log.obj->base.size;
-	/* TODO: Decide based on the User's input */
-	n_subbufs = 4;
+	n_subbufs = i915.guc_log_buffer_nr
 
 	guc_log_relay_chan = relay_open("guc_log", log_dir,
 			subbuf_size, n_subbufs, &relay_callbacks, dev);
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8b13bfa..d30c972 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -57,6 +57,7 @@ struct i915_params i915 __read_mostly = {
 	.enable_guc_loading = -1,
 	.enable_guc_submission = -1,
 	.guc_log_level = -1,
+	.guc_log_buffer_nr = 4,
 	.enable_dp_mst = true,
 	.inject_load_failure = 0,
 	.enable_dpcd_backlight = false,
@@ -214,6 +215,10 @@ module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
 MODULE_PARM_DESC(guc_log_level,
 	"GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
 
+module_param_named(guc_log_buffer_nr, i915.guc_log_buffer_nr, int, 0400);
+MODULE_PARM_DESC(guc_log_buffer_nr,
+	"Number of sub buffers to store GuC firmware logs (default: 4)");
+
 module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, 0600);
 MODULE_PARM_DESC(enable_dp_mst,
 	"Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 0ad020b..14ca855 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -48,6 +48,7 @@ struct i915_params {
 	int enable_guc_loading;
 	int enable_guc_submission;
 	int guc_log_level;
+	int guc_log_buffer_nr;
 	int use_mmio_flip;
 	int mmio_debug;
 	int edp_vswing;
-- 
1.9.2

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

[PATCH 10/14] drm/i915: Support to create write combined type vmaps

[akash.goel]

From: Chris Wilson <chris@chris-wilson.co.uk>

vmaps has a provision for controlling the page protection bits, with which
we can use to control the mapping type, e.g. WB, WC, UC or even WT.
To allow the caller to choose their mapping type, we add a parameter to
i915_gem_object_pin_map - but we still only allow one vmap to be cached
per object. If the object is currently not pinned, then we recreate the
previous vmap with the new access type, but if it was pinned we report an
error. This effectively limits the access via i915_gem_object_pin_map to a
single mapping type for the lifetime of the object. Not usually a problem,
but something to be aware of when setting up the object's vmap.

We will want to vary the access type to enable WC mappings of ringbuffer
and context objects on !llc platforms, as well as other objects where we
need coherent access to the GPU's pages without going through the GTT

v2: Remove the redundant braces around pin count check and fix the marker
    in documentation (Chris)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  4 ++-
 drivers/gpu/drm/i915/i915_gem.c         | 57 +++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/i915_gem_dmabuf.c  |  2 +-
 drivers/gpu/drm/i915/intel_lrc.c        |  8 ++---
 drivers/gpu/drm/i915/intel_ringbuffer.c |  2 +-
 5 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 20c701c..7aa11de 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3197,6 +3197,7 @@ static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
 /**
  * i915_gem_object_pin_map - return a contiguous mapping of the entire object
  * @obj - the object to map into kernel address space
+ * @use_wc - whether the mapping should be using WC or WB pgprot_t
  *
  * Calls i915_gem_object_pin_pages() to prevent reaping of the object's
  * pages and then returns a contiguous mapping of the backing storage into
@@ -3208,7 +3209,8 @@ static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
  * Returns the pointer through which to access the mapped object, or an
  * ERR_PTR() on error.
  */
-void *__must_check i915_gem_object_pin_map(struct drm_i915_gem_object *obj);
+void *__must_check i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
+					 bool use_wc);
 
 /**
  * i915_gem_object_unpin_map - releases an earlier mapping
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 51191b87..2ab7e8f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2469,10 +2469,11 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
 	list_del(&obj->global_list);
 
 	if (obj->mapping) {
-		if (is_vmalloc_addr(obj->mapping))
-			vunmap(obj->mapping);
+		void *ptr = (void *)((uintptr_t)obj->mapping & ~1);
+		if (is_vmalloc_addr(ptr))
+			vunmap(ptr);
 		else
-			kunmap(kmap_to_page(obj->mapping));
+			kunmap(kmap_to_page(ptr));
 		obj->mapping = NULL;
 	}
 
@@ -2645,7 +2646,8 @@ i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
 }
 
 /* The 'mapping' part of i915_gem_object_pin_map() below */
-static void *i915_gem_object_map(const struct drm_i915_gem_object *obj)
+static void *i915_gem_object_map(const struct drm_i915_gem_object *obj,
+				bool use_wc)
 {
 	unsigned long n_pages = obj->base.size >> PAGE_SHIFT;
 	struct sg_table *sgt = obj->pages;
@@ -2657,7 +2659,7 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj)
 	void *addr;
 
 	/* A single page can always be kmapped */
-	if (n_pages == 1)
+	if (n_pages == 1 && !use_wc)
 		return kmap(sg_page(sgt->sgl));
 
 	if (n_pages > ARRAY_SIZE(stack_pages)) {
@@ -2673,7 +2675,8 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj)
 	/* Check that we have the expected number of pages */
 	GEM_BUG_ON(i != n_pages);
 
-	addr = vmap(pages, n_pages, 0, PAGE_KERNEL);
+	addr = vmap(pages, n_pages, VM_NO_GUARD,
+		    use_wc ? pgprot_writecombine(PAGE_KERNEL_IO) : PAGE_KERNEL);
 
 	if (pages != stack_pages)
 		drm_free_large(pages);
@@ -2682,27 +2685,55 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj)
 }
 
 /* get, pin, and map the pages of the object into kernel space */
-void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj)
+void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, bool use_wc)
 {
+	void *ptr;
+	bool has_wc;
+	bool pinned;
 	int ret;
 
 	lockdep_assert_held(&obj->base.dev->struct_mutex);
+	GEM_BUG_ON((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0);
 
 	ret = i915_gem_object_get_pages(obj);
 	if (ret)
 		return ERR_PTR(ret);
 
+	GEM_BUG_ON(obj->pages == NULL);
 	i915_gem_object_pin_pages(obj);
 
-	if (!obj->mapping) {
-		obj->mapping = i915_gem_object_map(obj);
-		if (!obj->mapping) {
-			i915_gem_object_unpin_pages(obj);
-			return ERR_PTR(-ENOMEM);
+	pinned = obj->pages_pin_count > 1;
+	ptr = (void *)((uintptr_t)obj->mapping & ~1);
+	has_wc = (uintptr_t)obj->mapping & 1;
+
+	if (ptr && has_wc != use_wc) {
+		if (pinned) {
+			ret = -EBUSY;
+			goto err;
+		}
+
+		if (is_vmalloc_addr(ptr))
+			vunmap(ptr);
+		else
+			kunmap(kmap_to_page(ptr));
+		ptr = obj->mapping = NULL;
+	}
+
+	if (!ptr) {
+		ptr = i915_gem_object_map(obj, use_wc);
+		if (!ptr) {
+			ret = -ENOMEM;
+			goto err;
 		}
+
+		obj->mapping = (void *)((uintptr_t)ptr | use_wc);
 	}
 
-	return obj->mapping;
+	return ptr;
+
+err:
+	i915_gem_object_unpin_pages(obj);
+	return ERR_PTR(ret);
 }
 
 void i915_vma_move_to_active(struct i915_vma *vma,
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 80bbe43..edcadce 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -115,7 +115,7 @@ static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
 	if (ret)
 		return ERR_PTR(ret);
 
-	addr = i915_gem_object_pin_map(obj);
+	addr = i915_gem_object_pin_map(obj, false);
 	mutex_unlock(&dev->struct_mutex);
 
 	return addr;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 62b0dc6..b6397a7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -971,7 +971,7 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
 	if (ret)
 		goto err;
 
-	vaddr = i915_gem_object_pin_map(ce->state);
+	vaddr = i915_gem_object_pin_map(ce->state, false);
 	if (IS_ERR(vaddr)) {
 		ret = PTR_ERR(vaddr);
 		goto unpin_ctx_obj;
@@ -2007,7 +2007,7 @@ lrc_setup_hws(struct intel_engine_cs *engine,
 	/* The HWSP is part of the default context object in LRC mode. */
 	engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(dctx_obj) +
 				       LRC_PPHWSP_PN * PAGE_SIZE;
-	hws = i915_gem_object_pin_map(dctx_obj);
+	hws = i915_gem_object_pin_map(dctx_obj, false);
 	if (IS_ERR(hws))
 		return PTR_ERR(hws);
 	engine->status_page.page_addr = hws + LRC_PPHWSP_PN * PAGE_SIZE;
@@ -2334,7 +2334,7 @@ populate_lr_context(struct i915_gem_context *ctx,
 		return ret;
 	}
 
-	vaddr = i915_gem_object_pin_map(ctx_obj);
+	vaddr = i915_gem_object_pin_map(ctx_obj, false);
 	if (IS_ERR(vaddr)) {
 		ret = PTR_ERR(vaddr);
 		DRM_DEBUG_DRIVER("Could not map object pages! (%d)\n", ret);
@@ -2568,7 +2568,7 @@ void intel_lr_context_reset(struct drm_i915_private *dev_priv,
 		if (!ctx_obj)
 			continue;
 
-		vaddr = i915_gem_object_pin_map(ctx_obj);
+		vaddr = i915_gem_object_pin_map(ctx_obj, false);
 		if (WARN_ON(IS_ERR(vaddr)))
 			continue;
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 13544f8..59df3da 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2212,7 +2212,7 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_i915_private *dev_priv,
 		if (ret)
 			goto err_unpin;
 
-		addr = i915_gem_object_pin_map(obj);
+		addr = i915_gem_object_pin_map(obj, false);
 		if (IS_ERR(addr)) {
 			ret = PTR_ERR(addr);
 			goto err_unpin;
-- 
1.9.2

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

[PATCH 11/14] drm/i915: Use uncached(WC) mapping for acessing the GuC log buffer

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

Host needs to sample the GuC log buffer on every flush interrupt from GuC.
To ensure that we always get the up-to-date data from log buffer, its
better to access the buffer through an uncached CPU mapping. Also the way
buffer is accessed from GuC & Host side, manually doing cache flush may
not be effective always if cached CPU mapping is used.
Since the data to be read is less than 50 KB, there would not be any
performance implications.

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 53 +++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_guc.h           |  1 +
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index ec68a6d..4d76588 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -874,7 +874,7 @@ static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
 	u32 num_pages_to_copy;
 	int i;
 
-	if (!guc->log.obj)
+	if (!guc->log.obj || !guc->log.buf_addr)
 		return;
 
 	num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
@@ -884,8 +884,7 @@ static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
 	if (!capture_all)
 		num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
 
-	log_buffer_state = src_ptr =
-		kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));
+	log_buffer_state = src_ptr = guc->log.buf_addr;
 
 	/* Get the pointer to local buffer to store the logs */
 	dst_ptr = log_buffer_copy_state = guc_get_write_buffer(guc);
@@ -906,8 +905,6 @@ static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
 			log_buffer_copy_state++;
 		}
 
-		/* FIXME: invalidate/flush for log buffer needed */
-
 		/* Update the read pointer in the shared log buffer */
 		log_buffer_state->read_ptr =
 			log_buffer_state->sampled_write_ptr;
@@ -917,14 +914,11 @@ static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
 		log_buffer_state++;
 	}
 
-	kunmap_atomic(src_ptr);
-
 	/* Now copy the actual logs */
 	for (i=1; (i < num_pages_to_copy) && dst_ptr; i++) {
 		dst_ptr += PAGE_SIZE;
-		src_ptr = kmap_atomic(i915_gem_object_get_page(guc->log.obj, i));
+		src_ptr += PAGE_SIZE;
 		memcpy(dst_ptr, src_ptr, PAGE_SIZE);
-		kunmap_atomic(src_ptr);
 	}
 }
 
@@ -1016,7 +1010,7 @@ static int guc_create_log_relay_file(struct intel_guc *guc)
 
 	/* Keep the size of sub buffers same as shared log buffer */
 	subbuf_size = guc->log.obj->base.size;
-	n_subbufs = i915.guc_log_buffer_nr
+	n_subbufs = i915.guc_log_buffer_nr;
 
 	guc_log_relay_chan = relay_open("guc_log", log_dir,
 			subbuf_size, n_subbufs, &relay_callbacks, dev);
@@ -1051,6 +1045,7 @@ static int guc_log_late_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
+	void *vaddr;
 	int ret;
 
 	lockdep_assert_held(&dev->struct_mutex);
@@ -1061,9 +1056,26 @@ static int guc_log_late_setup(struct drm_device *dev)
 	if (WARN_ON(guc->log.relay_chan))
 		return -EINVAL;
 
+	/* If log_level was set as -1 at boot time, then vmalloc mapping would
+	 * not have been created for the log buffer, so create one now.
+	 */
+	if (!guc->log.buf_addr) {
+		vaddr = i915_gem_object_pin_map(guc->log.obj, true);
+		if (IS_ERR(vaddr)) {
+			ret = PTR_ERR(vaddr);
+			DRM_DEBUG_DRIVER("Couldn't map log buffer %d\n", ret);
+			goto err;
+		}
+		guc->log.buf_addr = vaddr;
+	}
+
+
 	ret = guc_create_log_relay_file(guc);
-	if (ret)
+	if (ret) {
+		guc->log.buf_addr = NULL;
+		i915_gem_object_unpin_map(guc->log.obj);
 		goto err;
+	}
 
 	return 0;
 err:
@@ -1098,6 +1110,25 @@ static void guc_create_log(struct intel_guc *guc)
 			return;
 		}
 
+		if (i915.guc_log_level >= 0) {
+			/* Create a WC (Uncached for read) mapping so that we
+			 * can directly get the data (up-to-date) from memory.
+			 * Also create the mapping now only to properly handle
+			 * the flush interrupts which can come before we create
+			 * a relay channel at the end of Driver load.
+			 */
+			void *vaddr = i915_gem_object_pin_map(obj, true);
+			if (IS_ERR(vaddr)) {
+				DRM_DEBUG_DRIVER("Couldn't map log buffer %ld\n",
+				PTR_ERR(vaddr));
+				gem_release_guc_obj(obj);
+				i915.guc_log_level = -1;
+				return;
+			}
+
+			guc->log.buf_addr = vaddr;
+		}
+
 		guc->log.obj = obj;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 3e90279..5e6accb 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -127,6 +127,7 @@ struct intel_guc_log {
 	struct drm_i915_gem_object *obj;
 	struct workqueue_struct *wq;
 	struct rchan *relay_chan;
+	void *buf_addr;
 };
 
 struct intel_guc {
-- 
1.9.2

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

[PATCH 12/14] drm/i915: New lock to serialize the Host2GuC actions

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

With the addition of new Host2GuC actions related to GuC logging, there
is a need of a lock to serialize them, as they can execute concurrently
with each other and also with other existing actions.

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++
 drivers/gpu/drm/i915/intel_guc.h           | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 4d76588..d20df8d 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -88,6 +88,7 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len)
 		return -EINVAL;
 
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+	spin_lock(&guc->action_lock);
 
 	dev_priv->guc.action_count += 1;
 	dev_priv->guc.action_cmd = data[0];
@@ -120,6 +121,7 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len)
 	}
 	dev_priv->guc.action_status = status;
 
+	spin_unlock(&guc->action_lock);
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 
 	return ret;
@@ -1258,6 +1260,7 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 		return -ENOMEM;
 
 	ida_init(&guc->ctx_ids);
+	spin_lock_init(&guc->action_lock);
 	guc_create_log(guc);
 	guc_create_ads(guc);
 
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 5e6accb..7cefd81 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -157,6 +157,9 @@ struct intel_guc {
 
 	uint64_t submissions[I915_NUM_ENGINES];
 	uint32_t last_seqno[I915_NUM_ENGINES];
+
+	/* To serialize the Host2GuC actions */
+	spinlock_t action_lock;
 };
 
 /* intel_guc_loader.c */
-- 
1.9.2

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

[PATCH 13/14] drm/i915: Add stats for GuC log buffer flush interrupts

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

GuC firmware sends an interrupt to flush the log buffer when it
becomes half full. GuC firmware also tracks how many times the
buffer overflowed.
It would be useful to maintain a statistics of how many flush
interrupts were received and for which type of log buffer,
along with the overflow count of each buffer type.
Augmented i915_log_info debugfs to report back these statistics.

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_guc_submission.c |  3 +++
 drivers/gpu/drm/i915/i915_irq.c            |  1 +
 drivers/gpu/drm/i915/intel_guc.h           |  5 +++++
 4 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 16aeab2..857ce9d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2542,6 +2542,30 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
 	return 0;
 }
 
+static void i915_guc_log_info(struct seq_file *m,
+				 struct drm_i915_private *dev_priv)
+{
+	struct intel_guc *guc = &dev_priv->guc;
+
+	seq_printf(m, "\nGuC logging stats:\n");
+
+	seq_printf(m, "\tISR:   flush count %10u, overflow count %8u\n",
+		guc->log.flush_count[GUC_ISR_LOG_BUFFER],
+		guc->log.overflow_count[GUC_ISR_LOG_BUFFER]);
+
+	seq_printf(m, "\tDPC:   flush count %10u, overflow count %8u\n",
+		guc->log.flush_count[GUC_DPC_LOG_BUFFER],
+		guc->log.overflow_count[GUC_DPC_LOG_BUFFER]);
+
+	seq_printf(m, "\tCRASH: flush count %10u, overflow count %8u\n",
+		guc->log.flush_count[GUC_CRASH_DUMP_LOG_BUFFER],
+		guc->log.overflow_count[GUC_CRASH_DUMP_LOG_BUFFER]);
+
+	seq_printf(m, "\tTotal flush interrupt count: %u\n",
+			guc->log.flush_interrupt_count);
+
+}
+
 static void i915_guc_client_info(struct seq_file *m,
 				 struct drm_i915_private *dev_priv,
 				 struct i915_guc_client *client)
@@ -2615,6 +2639,8 @@ static int i915_guc_info(struct seq_file *m, void *data)
 	seq_printf(m, "\nGuC execbuf client @ %p:\n", guc.execbuf_client);
 	i915_guc_client_info(m, dev_priv, &client);
 
+	i915_guc_log_info(m, dev_priv);
+
 	/* Add more as required ... */
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index d20df8d..2a192d4 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -907,6 +907,9 @@ static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
 			log_buffer_copy_state++;
 		}
 
+		guc->log.overflow_count[i] = log_buffer_state->buffer_full_cnt;
+		guc->log.flush_count[i] += log_buffer_state->flush_to_file;
+
 		/* Update the read pointer in the shared log buffer */
 		log_buffer_state->read_ptr =
 			log_buffer_state->sampled_write_ptr;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f1cf36e..e2cbe1e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1743,6 +1743,7 @@ static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 gt_iir)
 			queue_work(dev_priv->guc.log.wq,
 					&dev_priv->guc.events_work);
 		}
+		dev_priv->guc.log.flush_interrupt_count++;
 		spin_unlock(&dev_priv->irq_lock);
 	}
 }
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 7cefd81..bd299d3 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -128,6 +128,11 @@ struct intel_guc_log {
 	struct workqueue_struct *wq;
 	struct rchan *relay_chan;
 	void *buf_addr;
+
+	/* logging related stats */
+	u32 flush_interrupt_count;
+	u32 overflow_count[GUC_MAX_LOG_BUFFER];
+	u32 flush_count[GUC_MAX_LOG_BUFFER];
 };
 
 struct intel_guc {
-- 
1.9.2

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

[PATCH 14/14] drm/i915: Optimization to reduce the sampling time of GuC log buffer

[akash.goel]

From: Akash Goel <akash.goel@intel.com>

GuC firmware sends an interrupt to flush the log buffer when it becomes
half full, so Driver doesn't really need to sample the complete buffer
and can just copy only the newly written data by GuC into the local
buffer, i.e. as per the read & write pointer values.
Moreover the flush interrupt would generally come for one type of log
buffer, when it becomes half full, so at that time the other 2 types of
log buffer would comparatively have much lesser unread data in them.
In case of overflow reported by GuC, Driver do need to copy the entire
buffer as the whole buffer would contain the unread data.

Signed-off-by: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 68 +++++++++++++++++++++---------
 drivers/gpu/drm/i915/i915_irq.c            |  3 +-
 drivers/gpu/drm/i915/intel_guc.h           |  2 +-
 3 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 2a192d4..9514645 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -866,32 +866,35 @@ static void* guc_get_write_buffer(struct intel_guc *guc)
 	return relay_reserve(guc->log.relay_chan, guc->log.obj->base.size);
 }
 
-static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
+static void guc_read_update_log_buffer(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
 	struct guc_log_buffer_state *log_buffer_state;
 	struct guc_log_buffer_state *log_buffer_copy_state;
 	void *src_ptr, *dst_ptr;
-	u32 num_pages_to_copy;
-	int i;
+	bool new_overflow;
+	u32 i, buffer_size, read_offset, write_offset, bytes_to_copy;
 
 	if (!guc->log.obj || !guc->log.buf_addr)
 		return;
 
-	num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
-	/* Don't really need to copy crash buffer area in regular cases as there
-	 * won't be any unread data there.
-	 */
-	if (!capture_all)
-		num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
-
 	log_buffer_state = src_ptr = guc->log.buf_addr;
 
 	/* Get the pointer to local buffer to store the logs */
 	dst_ptr = log_buffer_copy_state = guc_get_write_buffer(guc);
 
+	/* Actual logs are present from the 2nd page */
+	src_ptr += PAGE_SIZE;
+	dst_ptr += PAGE_SIZE;
+
 	for (i = 0; i < GUC_MAX_LOG_BUFFER; i++) {
+		buffer_size = log_buffer_state->size;
+		read_offset = log_buffer_state->read_ptr;
+		write_offset = log_buffer_state->sampled_write_ptr;
+		new_overflow = 0;
+
+		/* First copy the state structure */
 		if (log_buffer_copy_state) {
 			memcpy(log_buffer_copy_state, log_buffer_state,
 					sizeof(struct guc_log_buffer_state));
@@ -907,6 +910,10 @@ static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
 			log_buffer_copy_state++;
 		}
 
+		if (log_buffer_state->buffer_full_cnt !=
+					guc->log.overflow_count[i])
+			new_overflow = 1;
+
 		guc->log.overflow_count[i] = log_buffer_state->buffer_full_cnt;
 		guc->log.flush_count[i] += log_buffer_state->flush_to_file;
 
@@ -917,13 +924,36 @@ static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
 		/* Clear the 'flush to file' flag */
 		log_buffer_state->flush_to_file = 0;
 		log_buffer_state++;
-	}
 
-	/* Now copy the actual logs */
-	for (i=1; (i < num_pages_to_copy) && dst_ptr; i++) {
-		dst_ptr += PAGE_SIZE;
-		src_ptr += PAGE_SIZE;
-		memcpy(dst_ptr, src_ptr, PAGE_SIZE);
+		if (!log_buffer_copy_state)
+			continue;
+
+		/* Now copy the actual logs */
+		if ((read_offset > buffer_size) || (write_offset > buffer_size)) {
+			DRM_ERROR("invalid log buffer state\n");
+			/* copy the whole buffer, as offsets are unreliable */
+			memcpy(dst_ptr, src_ptr, buffer_size);
+		} else if (new_overflow) {
+			/* Need to copy the whole buffer in case of overflow */
+			memcpy(dst_ptr, src_ptr, buffer_size);
+		} else {
+			/* Just copy the new data */
+			if (read_offset <= write_offset) {
+				bytes_to_copy = write_offset - read_offset;
+				memcpy(dst_ptr + read_offset,
+					src_ptr + read_offset, bytes_to_copy);
+			} else {
+				bytes_to_copy = buffer_size - read_offset;
+				memcpy(dst_ptr + read_offset,
+					src_ptr + read_offset, bytes_to_copy);
+
+				bytes_to_copy = write_offset;
+				memcpy(dst_ptr, src_ptr, bytes_to_copy);
+			}
+		}
+
+		src_ptr += buffer_size;
+		dst_ptr += buffer_size;
 	}
 }
 
@@ -1370,11 +1400,11 @@ int intel_guc_resume(struct drm_device *dev)
 	return host2guc_action(guc, data, ARRAY_SIZE(data));
 }
 
-void i915_guc_capture_logs(struct drm_device *dev, bool capture_all)
+void i915_guc_capture_logs(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	guc_read_update_log_buffer(dev, capture_all);
+	guc_read_update_log_buffer(dev);
 
 	host2guc_logbuffer_flush_complete(&dev_priv->guc);
 }
@@ -1395,7 +1425,7 @@ void i915_guc_capture_logs_on_reset(struct drm_device *dev)
 	host2guc_force_logbuffer_flush(&dev_priv->guc);
 
 	/* GuC would have updated the log buffer by now, so capture it */
-	i915_guc_capture_logs(dev, true);
+	i915_guc_capture_logs(dev);
 
 end:
 	mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e2cbe1e..99af3b5 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1255,8 +1255,7 @@ static void gen9_guc2host_events_work(struct work_struct *work)
 	msg = I915_READ(SOFT_SCRATCH(15));
 	if (msg & (GUC2HOST_MSG_CRASH_DUMP_POSTED |
 		   GUC2HOST_MSG_FLUSH_LOG_BUFFER)) {
-		i915_guc_capture_logs(dev_priv->dev,
-			msg & GUC2HOST_MSG_CRASH_DUMP_POSTED);
+		i915_guc_capture_logs(dev_priv->dev);
 
 		/* Clear GuC to Host msg bits that are handled */
 		if (msg & GUC2HOST_MSG_FLUSH_LOG_BUFFER)
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index bd299d3..d9f50be 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -182,7 +182,7 @@ int i915_guc_wq_check_space(struct drm_i915_gem_request *rq);
 int i915_guc_submit(struct drm_i915_gem_request *rq);
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
-void i915_guc_capture_logs(struct drm_device *dev, bool capture_all);
+void i915_guc_capture_logs(struct drm_device *dev);
 void i915_guc_capture_logs_on_reset(struct drm_device *dev);
 void i915_guc_register(struct drm_device *dev);
 void i915_guc_unregister(struct drm_device *dev);
-- 
1.9.2

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

✗ Ro.CI.BAT: failure for Support for sustained capturing of GuC firmware logs (rev4)

[Patchwork]

== Series Details ==

Series: Support for sustained capturing of GuC firmware logs (rev4)
URL   : https://patchwork.freedesktop.org/series/7910/
State : failure

== Summary ==

Applying: drm/i915: Add GuC ukernel logging related fields to fw interface file
Applying: drm/i915: New structure to contain GuC logging related fields
fatal: sha1 information is lacking or useless (drivers/gpu/drm/i915/intel_guc_loader.c).
error: could not build fake ancestor
Patch failed at 0002 drm/i915: New structure to contain GuC logging related fields
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

[Chris Wilson]

On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.goel@intel.com wrote:
> +static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_guc *guc = &dev_priv->guc;
> +	struct guc_log_buffer_state *log_buffer_state;
> +	struct guc_log_buffer_state *log_buffer_copy_state;
> +	void *src_ptr, *dst_ptr;
> +	u32 num_pages_to_copy;
> +	int i;
> +
> +	if (!guc->log.obj)
> +		return;
> +
> +	num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
> +	/* Don't really need to copy crash buffer area in regular cases as there
> +	 * won't be any unread data there.
> +	 */
> +	if (!capture_all)
> +		num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
> +
> +	log_buffer_state = src_ptr =
> +		kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));

So why not use i915_gem_object_pin_map() from the start?

That will cut down on the churn later.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/14] drm/i915: Support for GuC interrupts

[Chris Wilson]

On Sun, Jul 03, 2016 at 12:21:21AM +0530, akash.goel@intel.com wrote:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 85a7103..20c701c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1808,6 +1808,7 @@ struct drm_i915_private {
>  	u32 pm_irq_mask;
>  	u32 pm_ier_mask;
>  	u32 pm_rps_events;
> +	u32 guc_events;

pm_guc_events so we have some clue as to which register/control block it
is associated within the massive drm_i915_private.

>  	u32 pipestat_irq_mask[I915_MAX_PIPES];
>  
>  	struct i915_hotplug hotplug;
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index fedf82b..b441951 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1038,6 +1038,8 @@ int intel_guc_suspend(struct drm_device *dev)
>  	if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
>  		return 0;
>  
> +	gen9_disable_guc_interrupts(dev_priv);

Also upon sanitize?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/14] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

[Chris Wilson]

On Sun, Jul 03, 2016 at 12:21:20AM +0530, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
> 
> So far PM IER/IIR/IMR registers were being used only for Turbo related
> interrupts. But interrupts coming from GuC also use the same set.
> As a precursor to supporting GuC interrupts, added new low level routines
> so as to allow sharing the programming of PM IER/IIR/IMR registers between
> Turbo & GuC.
> Also similar to PM IMR, maintaining a bitmask for PM IER register, to allow
> easy sharing of it between Turbo & GuC without involving a rmw operation.
> 
> v2:
> - For appropriateness & avoid any ambiguity, rename old functions
>   enable/disable pm_irq to mask/unmask pm_irq and rename new functions
>   enable/disable pm_interrupts to enable/disable pm_irq. (Tvrtko)
> - Use u32 in place of uint32_t. (Tvrtko)
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>  drivers/gpu/drm/i915/i915_irq.c         | 63 ++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_drv.h        |  3 ++
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  4 +--
>  4 files changed, 53 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9ef4919..85a7103 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1806,6 +1806,7 @@ struct drm_i915_private {
>  	};
>  	u32 gt_irq_mask;
>  	u32 pm_irq_mask;
> +	u32 pm_ier_mask;

Oops. u32 pm_imr; and u32 pm_ier;

>  	u32 pm_rps_events;
>  	u32 pipestat_irq_mask[I915_MAX_PIPES];
>  
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 4378a65..dd5ae6d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -314,7 +314,7 @@ static void snb_update_pm_irq(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> -void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
> +void gen6_unmask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
>  {
>  	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>  		return;
> @@ -322,28 +322,62 @@ void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
>  	snb_update_pm_irq(dev_priv, mask, mask);
>  }
>  
> -static void __gen6_disable_pm_irq(struct drm_i915_private *dev_priv,
> -				  uint32_t mask)
> +static void __gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
>  {
>  	snb_update_pm_irq(dev_priv, mask, 0);
>  }
>  
> -void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
> +void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
>  {
>  	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>  		return;
>  
> -	__gen6_disable_pm_irq(dev_priv, mask);
> +	__gen6_mask_pm_irq(dev_priv, mask);
>  }
>  
> -void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
> +void gen6_reset_pm_irq(struct drm_i915_private *dev_priv, u32 reset_mask)

reset_pm_iir

>  {
>  	i915_reg_t reg = gen6_pm_iir(dev_priv);
>  
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	I915_WRITE(reg, dev_priv->pm_rps_events);
> -	I915_WRITE(reg, dev_priv->pm_rps_events);
> +	assert_spin_locked(&dev_priv->irq_lock);
> +
> +	I915_WRITE(reg, reset_mask);
> +	I915_WRITE(reg, reset_mask);
>  	POSTING_READ(reg);
> +}
> +
> +void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, u32 enable_mask)
> +{
> +	u32 new_val;
> +
> +	assert_spin_locked(&dev_priv->irq_lock);
> +
> +	new_val = dev_priv->pm_ier_mask;
> +	new_val |= enable_mask;
> +
> +	dev_priv->pm_ier_mask = new_val;

dev_priv->pm_ier |= new_val;

> +	I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
> +	gen6_unmask_pm_irq(dev_priv, enable_mask);

What barrier do you need between the hw and the caller? I presume there
is a POSTING_READ in this callchain, would be nice to document it.

/* unmask_pm_irq provides a POSTING_READ */

> +}
> +
> +void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 disable_mask)
> +{
> +	u32 new_val;
> +
> +	assert_spin_locked(&dev_priv->irq_lock);
> +
> +	new_val = dev_priv->pm_ier_mask;
> +	new_val &= ~disable_mask;
> +
> +	dev_priv->pm_ier_mask = new_val;

dev_priv->pm_ier &= ~disable_mask;

> +	__gen6_mask_pm_irq(dev_priv, disable_mask);
> +	I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);

Do we need a barrier upon disabling? (Usually we need a stronger barrier
on enabling to ensure we don't miss an interrupt when enabling, but for
disabling we don't care.)

> +}
> +
> +void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
> +{
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	gen6_reset_pm_irq(dev_priv, dev_priv->pm_rps_events);
>  	dev_priv->rps.pm_iir = 0;

Hmm. That's slightly confusing, but passable since it is really the set
of bits in pm_iir for rps.

>  	spin_unlock_irq(&dev_priv->irq_lock);
>  }
> @@ -1599,7 +1629,7 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
>  {
>  	if (pm_iir & dev_priv->pm_rps_events) {
>  		spin_lock(&dev_priv->irq_lock);
> -		gen6_disable_pm_irq(dev_priv, pm_iir & dev_priv->pm_rps_events);
> +		gen6_mask_pm_irq(dev_priv, pm_iir & dev_priv->pm_rps_events);
>  		if (dev_priv->rps.interrupts_enabled) {
>  			dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events;
>  			queue_work(dev_priv->wq, &dev_priv->rps.work);
> @@ -3770,6 +3800,7 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>  		gt_interrupts[0] |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
>  
>  	dev_priv->pm_irq_mask = 0xffffffff;
> +	dev_priv->pm_ier_mask = 0x0;

dev_priv->pm_ier = 0;
dev_priv->pm_imr = ~dev_priv->pm_ier;

Make the initial relationship explicit.

>  	GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
>  	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);

Missed changing
GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_imr, dev_priv->pm_ier);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 13/14] drm/i915: Add stats for GuC log buffer flush interrupts

[Chris Wilson]

On Sun, Jul 03, 2016 at 12:21:30AM +0530, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
> 
> GuC firmware sends an interrupt to flush the log buffer when it
> becomes half full. GuC firmware also tracks how many times the
> buffer overflowed.
> It would be useful to maintain a statistics of how many flush

For what purpose? Would not tracepoints be even more useful?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

[Goel, Akash]

On 7/3/2016 2:45 PM, Chris Wilson wrote:
> On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.goel@intel.com wrote:
>> +static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
>> +{
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct intel_guc *guc = &dev_priv->guc;
>> +	struct guc_log_buffer_state *log_buffer_state;
>> +	struct guc_log_buffer_state *log_buffer_copy_state;
>> +	void *src_ptr, *dst_ptr;
>> +	u32 num_pages_to_copy;
>> +	int i;
>> +
>> +	if (!guc->log.obj)
>> +		return;
>> +
>> +	num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
>> +	/* Don't really need to copy crash buffer area in regular cases as there
>> +	 * won't be any unread data there.
>> +	 */
>> +	if (!capture_all)
>> +		num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
>> +
>> +	log_buffer_state = src_ptr =
>> +		kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));
>
> So why not use i915_gem_object_pin_map() from the start?
>
> That will cut down on the churn later.

Fine, will reorder the series and squash the other patch 'drm/i915: Use 
uncached(WC) mapping for accessing the GuC log buffer' with this patch.

Best regards
Akash
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

[Goel, Akash]

On 7/3/2016 5:51 PM, Goel, Akash wrote:
>
>
> On 7/3/2016 2:45 PM, Chris Wilson wrote:
>> On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.goel@intel.com wrote:
>>> +static void guc_read_update_log_buffer(struct drm_device *dev, bool
>>> capture_all)
>>> +{
>>> +    struct drm_i915_private *dev_priv = dev->dev_private;
>>> +    struct intel_guc *guc = &dev_priv->guc;
>>> +    struct guc_log_buffer_state *log_buffer_state;
>>> +    struct guc_log_buffer_state *log_buffer_copy_state;
>>> +    void *src_ptr, *dst_ptr;
>>> +    u32 num_pages_to_copy;
>>> +    int i;
>>> +
>>> +    if (!guc->log.obj)
>>> +        return;
>>> +
>>> +    num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
>>> +    /* Don't really need to copy crash buffer area in regular cases
>>> as there
>>> +     * won't be any unread data there.
>>> +     */
>>> +    if (!capture_all)
>>> +        num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
>>> +
>>> +    log_buffer_state = src_ptr =
>>> +        kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));
>>
>> So why not use i915_gem_object_pin_map() from the start?
>>
>> That will cut down on the churn later.
>
> Fine, will reorder the series and squash the other patch 'drm/i915: Use
> uncached(WC) mapping for accessing the GuC log buffer' with this patch.
>
Sorry got confused, will use the i915_gem_object_pin_map() here instead 
of kmap and keep the WC mapping patch at the end of series only. Then 
will just have to modify the call to i915_gem_object_pin_map() to pass 
the WC flag.


> Best regards
> Akash
>> -Chris
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 13/14] drm/i915: Add stats for GuC log buffer flush interrupts

[Goel, Akash]

On 7/3/2016 3:14 PM, Chris Wilson wrote:
> On Sun, Jul 03, 2016 at 12:21:30AM +0530, akash.goel@intel.com wrote:
>> From: Akash Goel <akash.goel@intel.com>
>>
>> GuC firmware sends an interrupt to flush the log buffer when it
>> becomes half full. GuC firmware also tracks how many times the
>> buffer overflowed.
>> It would be useful to maintain a statistics of how many flush
>
> For what purpose? Would not tracepoints be even more useful?
Having a stats would be useful to get an idea of the volume & the rate 
at which logs are being generated from GuC side and whether Driver is 
quick enough to capture all of them.

Yes tracepoint would also be very useful.

Please see below the logging related stats, in the output of
‘i915_guc_info’ on execution of ‘gem_exec_nop’ IGT.

GuC total action count: 623531
GuC action failure count: 0
GuC last action command: 0x30
GuC last action status: 0xf0000000
GuC last action error code: 0

GuC submissions:
         render ring             :    9019910, last seqno 0x01a4390b
         blitter ring            :    6188291, last seqno 0x01a4390d
         bsd ring                :    6179075, last seqno 0x01a4390c
         video enhancement ring  :    6156547, last seqno 0x01a4390e
         Total: 27543823

GuC execbuf client @ ffff8801659fb100:
         Priority 2, GuC ctx index: 0, PD offset 0x800
         Doorbell id 0, offset: 0x0, cookie 0x1a4490f
         WQ size 8192, offset: 0x1000, tail 4336
         Work queue full: 0
         Failed to queue: 0
         Failed doorbell: 0
         Last submission result: 0
         Submissions: 9019910 render ring
         Submissions: 6188291 blitter ring
         Submissions: 6179075 bsd ring
         Submissions: 6156547 video enhancement ring
         Total: 27543823

GuC logging stats:
         ISR:     flush count     321718, overflow count        0
         DPC:     flush count     303788, overflow count        1
         CRASH:   flush count          0, overflow count        0
         Total flush interrupt count: 625511


Best regards
Akash
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

[Chris Wilson]

On Sun, Jul 03, 2016 at 05:51:35PM +0530, Goel, Akash wrote:
> 
> 
> On 7/3/2016 2:45 PM, Chris Wilson wrote:
> >On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.goel@intel.com wrote:
> >>+static void guc_read_update_log_buffer(struct drm_device *dev, bool capture_all)
> >>+{
> >>+	struct drm_i915_private *dev_priv = dev->dev_private;
> >>+	struct intel_guc *guc = &dev_priv->guc;
> >>+	struct guc_log_buffer_state *log_buffer_state;
> >>+	struct guc_log_buffer_state *log_buffer_copy_state;
> >>+	void *src_ptr, *dst_ptr;
> >>+	u32 num_pages_to_copy;
> >>+	int i;
> >>+
> >>+	if (!guc->log.obj)
> >>+		return;
> >>+
> >>+	num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
> >>+	/* Don't really need to copy crash buffer area in regular cases as there
> >>+	 * won't be any unread data there.
> >>+	 */
> >>+	if (!capture_all)
> >>+		num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
> >>+
> >>+	log_buffer_state = src_ptr =
> >>+		kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));
> >
> >So why not use i915_gem_object_pin_map() from the start?
> >
> >That will cut down on the churn later.
> 
> Fine, will reorder the series and squash the other patch 'drm/i915:
> Use uncached(WC) mapping for accessing the GuC log buffer' with this
> patch.

I would keep the pin_map(false -> true) as a separate step so that it is
clearly documented (and reversible).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

[Chris Wilson]

On Sun, Jul 03, 2016 at 05:55:14PM +0530, Goel, Akash wrote:
> 
> 
> On 7/3/2016 5:51 PM, Goel, Akash wrote:
> >
> >
> >On 7/3/2016 2:45 PM, Chris Wilson wrote:
> >>On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.goel@intel.com wrote:
> >>>+static void guc_read_update_log_buffer(struct drm_device *dev, bool
> >>>capture_all)
> >>>+{
> >>>+    struct drm_i915_private *dev_priv = dev->dev_private;
> >>>+    struct intel_guc *guc = &dev_priv->guc;
> >>>+    struct guc_log_buffer_state *log_buffer_state;
> >>>+    struct guc_log_buffer_state *log_buffer_copy_state;
> >>>+    void *src_ptr, *dst_ptr;
> >>>+    u32 num_pages_to_copy;
> >>>+    int i;
> >>>+
> >>>+    if (!guc->log.obj)
> >>>+        return;
> >>>+
> >>>+    num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
> >>>+    /* Don't really need to copy crash buffer area in regular cases
> >>>as there
> >>>+     * won't be any unread data there.
> >>>+     */
> >>>+    if (!capture_all)
> >>>+        num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
> >>>+
> >>>+    log_buffer_state = src_ptr =
> >>>+        kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));
> >>
> >>So why not use i915_gem_object_pin_map() from the start?
> >>
> >>That will cut down on the churn later.
> >
> >Fine, will reorder the series and squash the other patch 'drm/i915: Use
> >uncached(WC) mapping for accessing the GuC log buffer' with this patch.
> >
> Sorry got confused, will use the i915_gem_object_pin_map() here
> instead of kmap and keep the WC mapping patch at the end of series
> only. Then will just have to modify the call to
> i915_gem_object_pin_map() to pass the WC flag.

Yup.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/14] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

[Goel, Akash]

On 7/3/2016 3:08 PM, Chris Wilson wrote:
> On Sun, Jul 03, 2016 at 12:21:20AM +0530, akash.goel@intel.com wrote:
>> From: Akash Goel <akash.goel@intel.com>
>>
>> So far PM IER/IIR/IMR registers were being used only for Turbo related
>> interrupts. But interrupts coming from GuC also use the same set.
>> As a precursor to supporting GuC interrupts, added new low level routines
>> so as to allow sharing the programming of PM IER/IIR/IMR registers between
>> Turbo & GuC.
>> Also similar to PM IMR, maintaining a bitmask for PM IER register, to allow
>> easy sharing of it between Turbo & GuC without involving a rmw operation.
>>
>> v2:
>> - For appropriateness & avoid any ambiguity, rename old functions
>>   enable/disable pm_irq to mask/unmask pm_irq and rename new functions
>>   enable/disable pm_interrupts to enable/disable pm_irq. (Tvrtko)
>> - Use u32 in place of uint32_t. (Tvrtko)
>>
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Akash Goel <akash.goel@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>>  drivers/gpu/drm/i915/i915_irq.c         | 63 ++++++++++++++++++++++++---------
>>  drivers/gpu/drm/i915/intel_drv.h        |  3 ++
>>  drivers/gpu/drm/i915/intel_ringbuffer.c |  4 +--
>>  4 files changed, 53 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 9ef4919..85a7103 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1806,6 +1806,7 @@ struct drm_i915_private {
>>  	};
>>  	u32 gt_irq_mask;
>>  	u32 pm_irq_mask;
>> +	u32 pm_ier_mask;
>
> Oops. u32 pm_imr; and u32 pm_ier;
>
Fine, will rename.

>>  	u32 pm_rps_events;
>>  	u32 pipestat_irq_mask[I915_MAX_PIPES];
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 4378a65..dd5ae6d 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -314,7 +314,7 @@ static void snb_update_pm_irq(struct drm_i915_private *dev_priv,
>>  	}
>>  }
>>
>> -void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
>> +void gen6_unmask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
>>  {
>>  	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>>  		return;
>> @@ -322,28 +322,62 @@ void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
>>  	snb_update_pm_irq(dev_priv, mask, mask);
>>  }
>>
>> -static void __gen6_disable_pm_irq(struct drm_i915_private *dev_priv,
>> -				  uint32_t mask)
>> +static void __gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
>>  {
>>  	snb_update_pm_irq(dev_priv, mask, 0);
>>  }
>>
>> -void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
>> +void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
>>  {
>>  	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>>  		return;
>>
>> -	__gen6_disable_pm_irq(dev_priv, mask);
>> +	__gen6_mask_pm_irq(dev_priv, mask);
>>  }
>>
>> -void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
>> +void gen6_reset_pm_irq(struct drm_i915_private *dev_priv, u32 reset_mask)
>
> reset_pm_iir
>
Thanks, will update.

>>  {
>>  	i915_reg_t reg = gen6_pm_iir(dev_priv);
>>
>> -	spin_lock_irq(&dev_priv->irq_lock);
>> -	I915_WRITE(reg, dev_priv->pm_rps_events);
>> -	I915_WRITE(reg, dev_priv->pm_rps_events);
>> +	assert_spin_locked(&dev_priv->irq_lock);
>> +
>> +	I915_WRITE(reg, reset_mask);
>> +	I915_WRITE(reg, reset_mask);
>>  	POSTING_READ(reg);
>> +}
>> +
>> +void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, u32 enable_mask)
>> +{
>> +	u32 new_val;
>> +
>> +	assert_spin_locked(&dev_priv->irq_lock);
>> +
>> +	new_val = dev_priv->pm_ier_mask;
>> +	new_val |= enable_mask;
>> +
>> +	dev_priv->pm_ier_mask = new_val;
>
> dev_priv->pm_ier |= new_val;

Sorry, my bad.
>
>> +	I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
>> +	gen6_unmask_pm_irq(dev_priv, enable_mask);
>
> What barrier do you need between the hw and the caller? I presume there
> is a POSTING_READ in this callchain, would be nice to document it.
>
> /* unmask_pm_irq provides a POSTING_READ */
>
Thanks, will add the comment.
So will assume that POSTING_READ is good enough here.

>> +}
>> +
>> +void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 disable_mask)
>> +{
>> +	u32 new_val;
>> +
>> +	assert_spin_locked(&dev_priv->irq_lock);
>> +
>> +	new_val = dev_priv->pm_ier_mask;
>> +	new_val &= ~disable_mask;
>> +
>> +	dev_priv->pm_ier_mask = new_val;
>
> dev_priv->pm_ier &= ~disable_mask;
>
>> +	__gen6_mask_pm_irq(dev_priv, disable_mask);
>> +	I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
>
> Do we need a barrier upon disabling? (Usually we need a stronger barrier
> on enabling to ensure we don't miss an interrupt when enabling, but for
> disabling we don't care.)
>
So no modification needed here, as you mentioned that we don't need to 
care about the register update getting completed in the disabling case.

>> +}
>> +
>> +void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
>> +{
>> +	spin_lock_irq(&dev_priv->irq_lock);
>> +	gen6_reset_pm_irq(dev_priv, dev_priv->pm_rps_events);
>>  	dev_priv->rps.pm_iir = 0;
>
> Hmm. That's slightly confusing, but passable since it is really the set
> of bits in pm_iir for rps.
>
>>  	spin_unlock_irq(&dev_priv->irq_lock);
>>  }
>> @@ -1599,7 +1629,7 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
>>  {
>>  	if (pm_iir & dev_priv->pm_rps_events) {
>>  		spin_lock(&dev_priv->irq_lock);
>> -		gen6_disable_pm_irq(dev_priv, pm_iir & dev_priv->pm_rps_events);
>> +		gen6_mask_pm_irq(dev_priv, pm_iir & dev_priv->pm_rps_events);
>>  		if (dev_priv->rps.interrupts_enabled) {
>>  			dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events;
>>  			queue_work(dev_priv->wq, &dev_priv->rps.work);
>> @@ -3770,6 +3800,7 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>>  		gt_interrupts[0] |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
>>
>>  	dev_priv->pm_irq_mask = 0xffffffff;
>> +	dev_priv->pm_ier_mask = 0x0;
>
> dev_priv->pm_ier = 0;
> dev_priv->pm_imr = ~dev_priv->pm_ier;
>
> Make the initial relationship explicit.
Thanks, will modify.

>
>>  	GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
>>  	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
>
> Missed changing
> GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_imr, dev_priv->pm_ier);

Thanks, will modify.

Best regards
Akash
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/14] drm/i915: Add GuC ukernel logging related fields to fw interface file

[Tvrtko Ursulin]

On 02/07/16 19:51, akash.goel@intel.com wrote:
> From: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>
> The first page of the GuC log buffer contains state info or meta data
> which is required to parse the logs contained in the subsequent pages.
> The structure representing the state info is added to interface file
> as Driver would need to handle log buffer flush interrupts from GuC.
> Added an enum for the different message/event types that can be send
> by the GuC ukernel to Host.
> Also added 2 new Host to GuC action types to inform GuC when Host has
> flushed the log buffer and forcefuly cause the GuC to send a new
> log buffer flush interrupt.
>
> v2: Make documentation of log buffer state structure more elaborate. (Tvrtko)
>      Rename LOGBUFFERFLUSH action to LOG_BUFFER_FLUSH for consistency. (Tvrtko)
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_fwif.h | 64 +++++++++++++++++++++++++++++++++++
>   1 file changed, 64 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 944786d..ad916b5 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -418,15 +418,73 @@ struct guc_ads {
>   	u32 reserved2[4];
>   } __packed;
>
> +/* GuC logging structures */
> +
> +enum guc_log_buffer_type {
> +	GUC_ISR_LOG_BUFFER,
> +	GUC_DPC_LOG_BUFFER,
> +	GUC_CRASH_DUMP_LOG_BUFFER,
> +	GUC_MAX_LOG_BUFFER
> +};
> +
> +/*
> + * Below state is used for coordination of retrieval of GuC firmware logs.
> + * read_ptr points to the location where i915 read last in log buffer and
> + * read only for GuC firmware. write_ptr is incremented by GuC with number

is read-only

> + * of bytes written for each log entry and is read only for i915.
> + * When the log buffer becomes half full, GuC sends a flush interrupt.
> + * GuC firmware expects that while it is writing to 2nd half of the buffer,
> + * first half would get consumed by Host and then get a flush completed
> + * acknowledgement from Host, so that it does not end up doing any overwrite
> + * causing loss of logs. So when buffer gets half filled & i915 has requested
> + * for interrupt, GuC will set flush_to_file field, set the sampled_write_ptr
> + * to the value of write_ptr and raise the interrupt.
> + * On receving the interrupt i915 should read the buffer, clear flush_to_file
> + * field and also update read_ptr with the value of sample_write_ptr, before
> + * sending an acknowledgement to GuC. marker & version fields are for internal
> + * usage of GuC and opaque to i915. buffer_full_cnt field is incremented every
> + * time GuC detects the log buffer overflow.
> + * Separate state is maintained for each log buffer type.
> + */
> +struct guc_log_buffer_state {
> +	u32 marker[2];
> +	u32 read_ptr;
> +	u32 write_ptr;
> +	u32 size;
> +	u32 sampled_write_ptr;
> +	union {
> +		struct {
> +			u32 flush_to_file:1;
> +			u32 buffer_full_cnt:4;
> +			u32 reserved:27;
> +		};
> +		u32 flags;
> +	};
> +	u32 version;
> +} __packed;
> +
> +union guc_log_control {
> +	struct {
> +		u32 logging_enabled:1;
> +		u32 reserved1:3;
> +		u32 verbosity:4;
> +		u32 reserved2:24;
> +	};
> +	u32 value;
> +} __packed;
> +
>   /* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */
>   enum host2guc_action {
>   	HOST2GUC_ACTION_DEFAULT = 0x0,
>   	HOST2GUC_ACTION_SAMPLE_FORCEWAKE = 0x6,
>   	HOST2GUC_ACTION_ALLOCATE_DOORBELL = 0x10,
>   	HOST2GUC_ACTION_DEALLOCATE_DOORBELL = 0x20,
> +	HOST2GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE = 0x30,
> +	HOST2GUC_ACTION_FORCE_LOG_BUFFER_FLUSH = 0x302,
>   	HOST2GUC_ACTION_ENTER_S_STATE = 0x501,
>   	HOST2GUC_ACTION_EXIT_S_STATE = 0x502,
>   	HOST2GUC_ACTION_SLPC_REQUEST = 0x3003,
> +	HOST2GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000,
>   	HOST2GUC_ACTION_LIMIT
>   };
>
> @@ -448,4 +506,10 @@ enum guc2host_status {
>   	GUC2HOST_STATUS_GENERIC_FAIL = GUC2HOST_STATUS(0x0000F000)
>   };
>
> +/* This action will be programmed in C1BC - SOFT_SCRATCH_15_REG */
> +enum guc2host_message {
> +	GUC2HOST_MSG_CRASH_DUMP_POSTED = (1 << 1),
> +	GUC2HOST_MSG_FLUSH_LOG_BUFFER = (1 << 3)
> +};
> +
>   #endif
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

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

Re: [PATCH 02/14] drm/i915: New structure to contain GuC logging related fields

[Tvrtko Ursulin]

On 02/07/16 19:51, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
>
> So far there were 2 fields related to GuC logs in 'intel_guc' structure.
> For the support of capturing GuC logs & storing them in a local buffer,
> multiple new fields would have to be added. This warrants a separate
> structure to contain the fields related to GuC logging state.
> Added a new structure 'intel_guc_log' and instance of it inside
> 'intel_guc' structure.
>
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c        |  2 +-
>   drivers/gpu/drm/i915/i915_guc_submission.c | 10 +++++-----
>   drivers/gpu/drm/i915/intel_guc.h           |  8 ++++++--
>   drivers/gpu/drm/i915/intel_guc_loader.c    |  2 +-
>   4 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5185e02..de197535 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2625,7 +2625,7 @@ static int i915_guc_log_dump(struct seq_file *m, void *data)
>   	struct drm_info_node *node = m->private;
>   	struct drm_device *dev = node->minor->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_i915_gem_object *log_obj = dev_priv->guc.log_obj;
> +	struct drm_i915_gem_object *log_obj = dev_priv->guc.log.obj;
>   	u32 *log;
>   	int i = 0, pg;
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 28a810f..fedf82b 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -835,7 +835,7 @@ static void guc_create_log(struct intel_guc *guc)
>   		GUC_LOG_ISR_PAGES + 1 +
>   		GUC_LOG_CRASH_PAGES + 1) << PAGE_SHIFT;
>
> -	obj = guc->log_obj;
> +	obj = guc->log.obj;
>   	if (!obj) {
>   		obj = gem_allocate_guc_obj(dev_priv, size);
>   		if (!obj) {
> @@ -844,7 +844,7 @@ static void guc_create_log(struct intel_guc *guc)
>   			return;
>   		}
>
> -		guc->log_obj = obj;
> +		guc->log.obj = obj;
>   	}
>
>   	/* each allocated unit is a page */
> @@ -854,7 +854,7 @@ static void guc_create_log(struct intel_guc *guc)
>   		(GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT);
>
>   	offset = i915_gem_obj_ggtt_offset(obj) >> PAGE_SHIFT; /* in pages */
> -	guc->log_flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags;
> +	guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags;
>   }
>
>   static void init_guc_policies(struct guc_policies *policies)
> @@ -1015,8 +1015,8 @@ void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
>   	gem_release_guc_obj(dev_priv->guc.ads_obj);
>   	guc->ads_obj = NULL;
>
> -	gem_release_guc_obj(dev_priv->guc.log_obj);
> -	guc->log_obj = NULL;
> +	gem_release_guc_obj(dev_priv->guc.log.obj);
> +	guc->log.obj = NULL;
>
>   	if (guc->ctx_pool_obj)
>   		ida_destroy(&guc->ctx_ids);
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 3e3e743..d52eca3 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -122,10 +122,14 @@ struct intel_guc_fw {
>   	uint32_t ucode_offset;
>   };
>
> +struct intel_guc_log {
> +	uint32_t flags;
> +	struct drm_i915_gem_object *obj;
> +};
> +
>   struct intel_guc {
>   	struct intel_guc_fw guc_fw;
> -	uint32_t log_flags;
> -	struct drm_i915_gem_object *log_obj;
> +	struct intel_guc_log log;
>
>   	struct drm_i915_gem_object *ads_obj;
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index db3c897..8ef9b7f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -173,7 +173,7 @@ static void set_guc_init_params(struct drm_i915_private *dev_priv)
>   	params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
>   			GUC_CTL_VCS2_ENABLED;
>
> -	params[GUC_CTL_LOG_PARAMS] = guc->log_flags;
> +	params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
>
>   	if (i915.guc_log_level >= 0)
>   		params[GUC_CTL_DEBUG] =
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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