Re: [Intel-gfx] [PATCH 08/20] drm/i915: Add a relay backed debugfs interface for capturing GuC logs

2016-08-12 Thread Goel, Akash 



On 8/12/2016 7:23 PM, Tvrtko Ursulin wrote:


On 12/08/16 07:25, akash.g...@intel.com wrote:

From: Akash Goel 

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.


snapshots


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)

v3:
- Switch to no-overwrite mode for relay.
- Fix the relay sub buffer switching sequence.

v4:
- Update i915 Kconfig to select RELAY config. (TvrtKo)
- Log a message when there is no sub buffer available to capture
   the GuC log buffer. (Tvrtko)
- Increase the number of relay sub buffers to 8 from 4, to have
   sufficient buffering for boot time logs

Suggested-by: Chris Wilson 
Signed-off-by: Sourab Gupta 
Signed-off-by: Akash Goel 
---
  drivers/gpu/drm/i915/Kconfig   |   1 +
  drivers/gpu/drm/i915/i915_drv.c|   2 +
  drivers/gpu/drm/i915/i915_guc_submission.c | 206
-
  drivers/gpu/drm/i915/intel_guc.h   |   3 +
  4 files changed, 209 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 7769e46..fc900d2 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -11,6 +11,7 @@ config DRM_I915
  select DRM_KMS_HELPER
  select DRM_PANEL
  select DRM_MIPI_DSI
+select RELAY
  # i915 depends on ACPI_VIDEO when ACPI is enabled
  # but for select to work, need to select ACPI_VIDEO's
dependencies, ick
  select BACKLIGHT_LCD_SUPPORT if ACPI
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index fc2da32..cb8c943 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1145,6 +1145,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_priv);
  i915_setup_sysfs(dev);
  } else
  DRM_ERROR("Failed to register driver for userspace access!\n");
@@ -1183,6 +1184,7 @@ static void i915_driver_unregister(struct
drm_i915_private *dev_priv)
  intel_opregion_unregister(dev_priv);

  i915_teardown_sysfs(&dev_priv->drm);
+i915_guc_unregister(dev_priv);
  i915_debugfs_unregister(dev_priv);
  drm_dev_unregister(&dev_priv->drm);

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 2635b67..1a2d648 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -23,6 +23,8 @@
   */
  #include 
  #include 
+#include 
+#include 
  #include "i915_drv.h"
  #include "intel_guc.h"

@@ -851,12 +853,33 @@ err:

  static void guc_move_to_next_buf(struct intel_guc *guc)
  {
-return;
+/* Make sure the updates made in the sub buffer are visible when
+ * Consumer sees the following update to offset inside the sub
buffer.
+ */
+smp_wmb();
+
+/* All data has been written, so now move the offset of sub
buffer. */
+relay_reserve(guc->log.relay_chan, guc->log.obj->base.size);
+
+/* Switch to the next sub buffer */
+relay_flush(guc->log.relay_chan);
  }

  static void* guc_get_write_buffer(struct intel_guc *guc)
  {
-return NULL;
+/* FIXME: Cover the check under a lock ? */


Need to resolve before r-b in any case.
After the last patch in this series, where relay channel will be created 
before enabling the GuC interrupts, the need of lock will not be there 
so will remove these comments in that patch.





+if (!guc->log.relay_chan)
+return NULL;
+
+/* Just get the base address of a new sub buffer and copy data
into it
+ * ourselves. NULL will be returned in no-overwrite mode, if all sub
+ * buffers are full. Could have used the relay_write() to indirectly
+ * copy the data, but that would have been bit convoluted, as we
need to
+ * write to only certain locations inside a sub buffer which
cannot be
+ * done without using relay_reserve() along with relay_write().
So its
+ * better to use relay_

Re: [Intel-gfx] [PATCH 08/20] drm/i915: Add a relay backed debugfs interface for capturing GuC logs

2016-08-12 Thread Tvrtko Ursulin 


On 12/08/16 07:25, akash.g...@intel.com wrote:

From: Akash Goel 

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.


snapshots


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)

v3:
- Switch to no-overwrite mode for relay.
- Fix the relay sub buffer switching sequence.

v4:
- Update i915 Kconfig to select RELAY config. (TvrtKo)
- Log a message when there is no sub buffer available to capture
   the GuC log buffer. (Tvrtko)
- Increase the number of relay sub buffers to 8 from 4, to have
   sufficient buffering for boot time logs

Suggested-by: Chris Wilson 
Signed-off-by: Sourab Gupta 
Signed-off-by: Akash Goel 
---
  drivers/gpu/drm/i915/Kconfig   |   1 +
  drivers/gpu/drm/i915/i915_drv.c|   2 +
  drivers/gpu/drm/i915/i915_guc_submission.c | 206 -
  drivers/gpu/drm/i915/intel_guc.h   |   3 +
  4 files changed, 209 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 7769e46..fc900d2 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -11,6 +11,7 @@ config DRM_I915
select DRM_KMS_HELPER
select DRM_PANEL
select DRM_MIPI_DSI
+   select RELAY
# i915 depends on ACPI_VIDEO when ACPI is enabled
# but for select to work, need to select ACPI_VIDEO's dependencies, ick
select BACKLIGHT_LCD_SUPPORT if ACPI
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index fc2da32..cb8c943 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1145,6 +1145,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_priv);
i915_setup_sysfs(dev);
} else
DRM_ERROR("Failed to register driver for userspace access!\n");
@@ -1183,6 +1184,7 @@ static void i915_driver_unregister(struct 
drm_i915_private *dev_priv)
intel_opregion_unregister(dev_priv);

i915_teardown_sysfs(&dev_priv->drm);
+   i915_guc_unregister(dev_priv);
i915_debugfs_unregister(dev_priv);
drm_dev_unregister(&dev_priv->drm);

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 2635b67..1a2d648 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -23,6 +23,8 @@
   */
  #include 
  #include 
+#include 
+#include 
  #include "i915_drv.h"
  #include "intel_guc.h"

@@ -851,12 +853,33 @@ err:

  static void guc_move_to_next_buf(struct intel_guc *guc)
  {
-   return;
+   /* Make sure the updates made in the sub buffer are visible when
+* Consumer sees the following update to offset inside the sub buffer.
+*/
+   smp_wmb();
+
+   /* All data has been written, so now move the offset of sub buffer. */
+   relay_reserve(guc->log.relay_chan, guc->log.obj->base.size);
+
+   /* Switch to the next sub buffer */
+   relay_flush(guc->log.relay_chan);
  }

  static void* guc_get_write_buffer(struct intel_guc *guc)
  {
-   return NULL;
+   /* FIXME: Cover the check under a lock ? */


Need to resolve before r-b in any case.


+   if (!guc->log.relay_chan)
+   return NULL;
+
+   /* Just get the base address of a new sub buffer and copy data into it
+* ourselves. NULL will be returned in no-overwrite mode, if all sub
+* buffers are full. Could have used the relay_write() to indirectly
+* copy the data, but that would have been bit convoluted, as we need to
+* write to only certain locations inside a sub buffer which cannot be
+* done without using relay_reserve() along with relay_write(). So its
+* better to use relay_reserve() alone.
+*/
+   return relay_reserve(guc->log.relay_chan, 0);
  }

  static void guc_read_update

[Intel-gfx] [PATCH 08/20] drm/i915: Add a relay backed debugfs interface for capturing GuC logs

2016-08-11 Thread akash . goel 
From: Akash Goel 

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)

v3:
- Switch to no-overwrite mode for relay.
- Fix the relay sub buffer switching sequence.

v4:
- Update i915 Kconfig to select RELAY config. (TvrtKo)
- Log a message when there is no sub buffer available to capture
  the GuC log buffer. (Tvrtko)
- Increase the number of relay sub buffers to 8 from 4, to have
  sufficient buffering for boot time logs

Suggested-by: Chris Wilson 
Signed-off-by: Sourab Gupta 
Signed-off-by: Akash Goel 
---
 drivers/gpu/drm/i915/Kconfig   |   1 +
 drivers/gpu/drm/i915/i915_drv.c|   2 +
 drivers/gpu/drm/i915/i915_guc_submission.c | 206 -
 drivers/gpu/drm/i915/intel_guc.h   |   3 +
 4 files changed, 209 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 7769e46..fc900d2 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -11,6 +11,7 @@ config DRM_I915
select DRM_KMS_HELPER
select DRM_PANEL
select DRM_MIPI_DSI
+   select RELAY
# i915 depends on ACPI_VIDEO when ACPI is enabled
# but for select to work, need to select ACPI_VIDEO's dependencies, ick
select BACKLIGHT_LCD_SUPPORT if ACPI
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index fc2da32..cb8c943 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1145,6 +1145,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_priv);
i915_setup_sysfs(dev);
} else
DRM_ERROR("Failed to register driver for userspace access!\n");
@@ -1183,6 +1184,7 @@ static void i915_driver_unregister(struct 
drm_i915_private *dev_priv)
intel_opregion_unregister(dev_priv);
 
i915_teardown_sysfs(&dev_priv->drm);
+   i915_guc_unregister(dev_priv);
i915_debugfs_unregister(dev_priv);
drm_dev_unregister(&dev_priv->drm);
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 2635b67..1a2d648 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -23,6 +23,8 @@
  */
 #include 
 #include 
+#include 
+#include 
 #include "i915_drv.h"
 #include "intel_guc.h"
 
@@ -851,12 +853,33 @@ err:
 
 static void guc_move_to_next_buf(struct intel_guc *guc)
 {
-   return;
+   /* Make sure the updates made in the sub buffer are visible when
+* Consumer sees the following update to offset inside the sub buffer.
+*/
+   smp_wmb();
+
+   /* All data has been written, so now move the offset of sub buffer. */
+   relay_reserve(guc->log.relay_chan, guc->log.obj->base.size);
+
+   /* Switch to the next sub buffer */
+   relay_flush(guc->log.relay_chan);
 }
 
 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;
+
+   /* Just get the base address of a new sub buffer and copy data into it
+* ourselves. NULL will be returned in no-overwrite mode, if all sub
+* buffers are full. Could have used the relay_write() to indirectly
+* copy the data, but that would have been bit convoluted, as we need to
+* write to only certain locations inside a sub buffer which cannot be
+* done without using relay_reserve() along with relay_write(). So its
+* better to use relay_reserve() alone.
+*/
+   return relay_reserve(guc->log.relay_chan, 0);
 }
 
 static void guc_read_update_log_buffer(struct intel_guc *guc)
@@ -923,6 +946,130 @@ static void guc_read_update_log_buffer(struct intel_guc 
*guc)