Re: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC
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.g...@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: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC
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.g...@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: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC
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.g...@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: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC
On 7/3/2016 2:45 PM, Chris Wilson wrote:
On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.g...@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: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC
On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.g...@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
[Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC
From: Sagar Arun Kamble
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
Signed-off-by: Akash Goel
---
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 i
