[PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task

[PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task

[Sagar Arun Kamble]

Releasing GuC doorbell involves stopping the doorbell unit snooping and
executing doorbell deallocation flow in GuC firmware. When GuC is in sane
state, both steps will be necessary to release doorbell.
If GuC is hung (possibly leading to i915 reset), only doorbell unit
snooping is to be stopped. destroy_doorbell will be called in suspend and
reset flows differently in upcoming patches.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index d14c134..b6486dc 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -207,7 +207,7 @@ static int __create_doorbell(struct i915_guc_client *client)
 	return err;
 }
 
-static int __destroy_doorbell(struct i915_guc_client *client)
+static int __destroy_doorbell(struct i915_guc_client *client, bool notify_guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(client->guc);
 	struct guc_doorbell_info *doorbell;
@@ -225,7 +225,10 @@ static int __destroy_doorbell(struct i915_guc_client *client)
 	if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID), 10))
 		WARN_ONCE(true, "Doorbell never became invalid after disable\n");
 
-	return __guc_deallocate_doorbell(client->guc, client->stage_id);
+	if (notify_guc)
+		return __guc_deallocate_doorbell(client->guc, client->stage_id);
+
+	return 0;
 }
 
 static int create_doorbell(struct i915_guc_client *client)
@@ -250,7 +253,11 @@ static int create_doorbell(struct i915_guc_client *client)
 	return ret;
 }
 
-static int destroy_doorbell(struct i915_guc_client *client)
+/*
+ * This function deactivates doorbell monitoring through doorbell unit and
+ * optionally notifies GuC to deallocate the doorbell.
+ */
+static int destroy_doorbell(struct i915_guc_client *client, bool notify_guc)
 {
 	int err;
 
@@ -259,7 +266,7 @@ static int destroy_doorbell(struct i915_guc_client *client)
 	/* XXX: wait for any interrupts */
 	/* XXX: wait for workqueue to drain */
 
-	err = __destroy_doorbell(client);
+	err = __destroy_doorbell(client, notify_guc);
 	if (err)
 		return err;
 
@@ -873,7 +880,7 @@ static int __reset_doorbell(struct i915_guc_client* client, u16 db_id)
 	__update_doorbell_desc(client, db_id);
 	err = __create_doorbell(client);
 	if (!err)
-		err = __destroy_doorbell(client);
+		err = __destroy_doorbell(client, true);
 
 	return err;
 }
@@ -899,7 +906,7 @@ static int guc_init_doorbell_hw(struct intel_guc *guc)
 
 		if (has_doorbell(client)) {
 			/* Borrow execbuf_client (we will recreate it later) */
-			destroy_doorbell(client);
+			destroy_doorbell(client, true);
 			recreate_first_client = true;
 		}
 
@@ -925,7 +932,7 @@ static int guc_init_doorbell_hw(struct intel_guc *guc)
 
 	ret = __create_doorbell(guc->preempt_client);
 	if (ret) {
-		__destroy_doorbell(guc->execbuf_client);
+		__destroy_doorbell(guc->execbuf_client, true);
 		return ret;
 	}
 
@@ -1043,7 +1050,7 @@ static void guc_client_free(struct i915_guc_client *client)
 	/* FIXME: in many cases, by the time we get here the GuC has been
 	 * reset, so we cannot destroy the doorbell properly. Ignore the
 	 * error message for now */
-	destroy_doorbell(client);
+	destroy_doorbell(client, true);
 	guc_stage_desc_fini(client->guc, client);
 	i915_gem_object_unpin_map(client->vma->obj);
 	i915_vma_unpin_and_release(&client->vma);
-- 
1.9.1

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

[PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume

[Sagar Arun Kamble]

Before GT device suspend, i915 should release GuC client doorbells by
stopping doorbell controller snooping and doorbell deallocation through
GuC. They need to be acquired again on resume. This will also resolve
the driver unload issue with GuC, where doorbell deallocation was being
done post GEM suspend.
Release function is called from guc_suspend prior to sending sleep action
during runtime and drm suspend. Acquiral is different in runtime and drm
resume paths as on drm resume we are currently doing full reinit. Release
should be done in synchronization with acquire hence GuC suspend/resume
along with doorbell release/acquire should be under struct_mutex. Upcoming
suspend and resume restructuring for GuC will update these changes.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c            |  3 +++
 drivers/gpu/drm/i915/i915_gem.c            |  5 +++--
 drivers/gpu/drm/i915/i915_guc_submission.c | 20 ++++++++++++++++----
 drivers/gpu/drm/i915/i915_guc_submission.h |  2 ++
 drivers/gpu/drm/i915/intel_guc.c           |  2 ++
 5 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e7e9e06..3df8a7d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -47,6 +47,7 @@
 #include <drm/i915_drm.h>
 
 #include "i915_drv.h"
+#include "i915_guc_submission.h"
 #include "i915_trace.h"
 #include "i915_vgpu.h"
 #include "intel_drv.h"
@@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device *kdev)
 
 	intel_guc_resume(dev_priv);
 
+	i915_guc_clients_acquire_doorbells(&dev_priv->guc);
+
 	if (IS_GEN9_LP(dev_priv)) {
 		bxt_disable_dc9(dev_priv);
 		bxt_display_core_init(dev_priv, true);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6a71805..cbce714 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -29,13 +29,14 @@
 #include <drm/drm_vma_manager.h>
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "i915_gemfs.h"
 #include "i915_gem_clflush.h"
-#include "i915_vgpu.h"
+#include "i915_guc_submission.h"
 #include "i915_trace.h"
+#include "i915_vgpu.h"
 #include "intel_drv.h"
 #include "intel_frontbuffer.h"
 #include "intel_mocs.h"
-#include "i915_gemfs.h"
 #include <linux/dma-fence-array.h>
 #include <linux/kthread.h>
 #include <linux/reservation.h>
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b6486dc..b989edf 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1047,10 +1047,6 @@ static void guc_client_free(struct i915_guc_client *client)
 	 * Be sure to drop any locks
 	 */
 
-	/* FIXME: in many cases, by the time we get here the GuC has been
-	 * reset, so we cannot destroy the doorbell properly. Ignore the
-	 * error message for now */
-	destroy_doorbell(client, true);
 	guc_stage_desc_fini(client->guc, client);
 	i915_gem_object_unpin_map(client->vma->obj);
 	i915_vma_unpin_and_release(&client->vma);
@@ -1102,6 +1098,21 @@ static void guc_clients_destroy(struct intel_guc *guc)
 	guc_client_free(client);
 }
 
+void i915_guc_clients_acquire_doorbells(struct intel_guc *guc)
+{
+	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return;
+
+	create_doorbell(guc->execbuf_client);
+	create_doorbell(guc->preempt_client);
+}
+
+void i915_guc_clients_release_doorbells(struct intel_guc *guc)
+{
+	destroy_doorbell(guc->preempt_client, true);
+	destroy_doorbell(guc->execbuf_client, true);
+}
+
 static void guc_policy_init(struct guc_policy *policy)
 {
 	policy->execution_quantum = POLICY_DEFAULT_EXECUTION_QUANTUM_US;
@@ -1423,6 +1434,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 	} else {
 		guc_reset_wq(guc->execbuf_client);
 		guc_reset_wq(guc->preempt_client);
+		i915_guc_clients_acquire_doorbells(guc);
 	}
 
 	err = intel_guc_sample_forcewake(guc);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.h b/drivers/gpu/drm/i915/i915_guc_submission.h
index cb4353b..1d5cf22 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.h
+++ b/drivers/gpu/drm/i915/i915_guc_submission.h
@@ -72,6 +72,8 @@ struct i915_guc_client {
 	u64 submissions[I915_NUM_ENGINES];
 };
 
+void i915_guc_clients_acquire_doorbells(struct intel_guc *guc);
+void i915_guc_clients_release_doorbells(struct intel_guc *guc);
 int i915_guc_submission_init(struct drm_i915_private *dev_priv);
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 9678630..b6eb5ac 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -274,6 +274,8 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
 	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return 0;
 
+	i915_guc_clients_release_doorbells(&dev_priv->guc);
+
 	gen9_disable_guc_interrupts(dev_priv);
 
 	data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
-- 
1.9.1

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

Re: [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume

[Chris Wilson]

Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
> Before GT device suspend, i915 should release GuC client doorbells by
> stopping doorbell controller snooping and doorbell deallocation through
> GuC. They need to be acquired again on resume. This will also resolve
> the driver unload issue with GuC, where doorbell deallocation was being
> done post GEM suspend.
> Release function is called from guc_suspend prior to sending sleep action
> during runtime and drm suspend. Acquiral is different in runtime and drm
> resume paths as on drm resume we are currently doing full reinit. Release
> should be done in synchronization with acquire hence GuC suspend/resume
> along with doorbell release/acquire should be under struct_mutex. Upcoming
> suspend and resume restructuring for GuC will update these changes.
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c            |  3 +++
>  drivers/gpu/drm/i915/i915_gem.c            |  5 +++--
>  drivers/gpu/drm/i915/i915_guc_submission.c | 20 ++++++++++++++++----
>  drivers/gpu/drm/i915/i915_guc_submission.h |  2 ++
>  drivers/gpu/drm/i915/intel_guc.c           |  2 ++
>  5 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e7e9e06..3df8a7d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -47,6 +47,7 @@
>  #include <drm/i915_drm.h>
>  
>  #include "i915_drv.h"
> +#include "i915_guc_submission.h"
>  #include "i915_trace.h"
>  #include "i915_vgpu.h"
>  #include "intel_drv.h"
> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device *kdev)
>  
>         intel_guc_resume(dev_priv);
>  
> +       i915_guc_clients_acquire_doorbells(&dev_priv->guc);

intel_guc_acquire_doorbells();

Though, if we need to specialise between resume and runtime_resume, I
suggest adding intel_guc_resume() and intel_guc_runtime_resume() entry
points. (We probably should find a better way to distinguish those two
paths, system_resume vs runtime_resume?)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume

[Sagar Arun Kamble]

On 11/6/2017 5:43 PM, Chris Wilson wrote:
> Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
>> Before GT device suspend, i915 should release GuC client doorbells by
>> stopping doorbell controller snooping and doorbell deallocation through
>> GuC. They need to be acquired again on resume. This will also resolve
>> the driver unload issue with GuC, where doorbell deallocation was being
>> done post GEM suspend.
>> Release function is called from guc_suspend prior to sending sleep action
>> during runtime and drm suspend. Acquiral is different in runtime and drm
>> resume paths as on drm resume we are currently doing full reinit. Release
>> should be done in synchronization with acquire hence GuC suspend/resume
>> along with doorbell release/acquire should be under struct_mutex. Upcoming
>> suspend and resume restructuring for GuC will update these changes.
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.c            |  3 +++
>>   drivers/gpu/drm/i915/i915_gem.c            |  5 +++--
>>   drivers/gpu/drm/i915/i915_guc_submission.c | 20 ++++++++++++++++----
>>   drivers/gpu/drm/i915/i915_guc_submission.h |  2 ++
>>   drivers/gpu/drm/i915/intel_guc.c           |  2 ++
>>   5 files changed, 26 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index e7e9e06..3df8a7d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -47,6 +47,7 @@
>>   #include <drm/i915_drm.h>
>>   
>>   #include "i915_drv.h"
>> +#include "i915_guc_submission.h"
>>   #include "i915_trace.h"
>>   #include "i915_vgpu.h"
>>   #include "intel_drv.h"
>> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device *kdev)
>>   
>>          intel_guc_resume(dev_priv);
>>   
>> +       i915_guc_clients_acquire_doorbells(&dev_priv->guc);
> intel_guc_acquire_doorbells();
Prefixed "i915_guc_clients" since this modifies submission state 
(clients/doorbells). Should have kept dev_priv as parameter.
what should be the correct option here: intel_guc*(guc) or 
i915_guc*(dev_priv)
>
> Though, if we need to specialise between resume and runtime_resume, I
> suggest adding intel_guc_resume() and intel_guc_runtime_resume() entry
> points. (We probably should find a better way to distinguish those two
> paths, system_resume vs runtime_resume?)
Yes. these will be added in the upcoming GuC suspend/resume 
restructuring series.
Functionality along runtime/system suspend path will be same for GuC. 
Will defer in
the resume path since we are doing full gem reinit on resume from system 
suspend.
> -Chris

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

Re: [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume

[Chris Wilson]

Quoting Sagar Arun Kamble (2017-11-07 06:05:01)
> 
> 
> On 11/6/2017 5:43 PM, Chris Wilson wrote:
> > Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
> >> Before GT device suspend, i915 should release GuC client doorbells by
> >> stopping doorbell controller snooping and doorbell deallocation through
> >> GuC. They need to be acquired again on resume. This will also resolve
> >> the driver unload issue with GuC, where doorbell deallocation was being
> >> done post GEM suspend.
> >> Release function is called from guc_suspend prior to sending sleep action
> >> during runtime and drm suspend. Acquiral is different in runtime and drm
> >> resume paths as on drm resume we are currently doing full reinit. Release
> >> should be done in synchronization with acquire hence GuC suspend/resume
> >> along with doorbell release/acquire should be under struct_mutex. Upcoming
> >> suspend and resume restructuring for GuC will update these changes.
> >>
> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Michał Winiarski <michal.winiarski@intel.com>
> >> Cc: Michel Thierry <michel.thierry@intel.com>
> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/i915_drv.c            |  3 +++
> >>   drivers/gpu/drm/i915/i915_gem.c            |  5 +++--
> >>   drivers/gpu/drm/i915/i915_guc_submission.c | 20 ++++++++++++++++----
> >>   drivers/gpu/drm/i915/i915_guc_submission.h |  2 ++
> >>   drivers/gpu/drm/i915/intel_guc.c           |  2 ++
> >>   5 files changed, 26 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index e7e9e06..3df8a7d 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -47,6 +47,7 @@
> >>   #include <drm/i915_drm.h>
> >>   
> >>   #include "i915_drv.h"
> >> +#include "i915_guc_submission.h"
> >>   #include "i915_trace.h"
> >>   #include "i915_vgpu.h"
> >>   #include "intel_drv.h"
> >> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device *kdev)
> >>   
> >>          intel_guc_resume(dev_priv);
> >>   
> >> +       i915_guc_clients_acquire_doorbells(&dev_priv->guc);
> > intel_guc_acquire_doorbells();
> Prefixed "i915_guc_clients" since this modifies submission state 
> (clients/doorbells). Should have kept dev_priv as parameter.
> what should be the correct option here: intel_guc*(guc) or 
> i915_guc*(dev_priv)

GuC submission is not i915. It is not part of the user facing api.
Operate on intel_guc as you were.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume

[Michal Wajdeczko]

On Tue, 07 Nov 2017 10:21:17 +0100, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Sagar Arun Kamble (2017-11-07 06:05:01)
>>
>>
>> On 11/6/2017 5:43 PM, Chris Wilson wrote:
>> > Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
>> >> Before GT device suspend, i915 should release GuC client doorbells by
>> >> stopping doorbell controller snooping and doorbell deallocation  
>> through
>> >> GuC. They need to be acquired again on resume. This will also resolve
>> >> the driver unload issue with GuC, where doorbell deallocation was  
>> being
>> >> done post GEM suspend.
>> >> Release function is called from guc_suspend prior to sending sleep  
>> action
>> >> during runtime and drm suspend. Acquiral is different in runtime and  
>> drm
>> >> resume paths as on drm resume we are currently doing full reinit.  
>> Release
>> >> should be done in synchronization with acquire hence GuC  
>> suspend/resume
>> >> along with doorbell release/acquire should be under struct_mutex.  
>> Upcoming
>> >> suspend and resume restructuring for GuC will update these changes.
>> >>
>> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> >> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> >> Cc: Michel Thierry <michel.thierry@intel.com>
>> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> >> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> >> ---
>> >>   drivers/gpu/drm/i915/i915_drv.c            |  3 +++
>> >>   drivers/gpu/drm/i915/i915_gem.c            |  5 +++--
>> >>   drivers/gpu/drm/i915/i915_guc_submission.c | 20  
>> ++++++++++++++++----
>> >>   drivers/gpu/drm/i915/i915_guc_submission.h |  2 ++
>> >>   drivers/gpu/drm/i915/intel_guc.c           |  2 ++
>> >>   5 files changed, 26 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c  
>> b/drivers/gpu/drm/i915/i915_drv.c
>> >> index e7e9e06..3df8a7d 100644
>> >> --- a/drivers/gpu/drm/i915/i915_drv.c
>> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> >> @@ -47,6 +47,7 @@
>> >>   #include <drm/i915_drm.h>
>> >>
>> >>   #include "i915_drv.h"
>> >> +#include "i915_guc_submission.h"
>> >>   #include "i915_trace.h"
>> >>   #include "i915_vgpu.h"
>> >>   #include "intel_drv.h"
>> >> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct device  
>> *kdev)
>> >>
>> >>          intel_guc_resume(dev_priv);
>> >>
>> >> +       i915_guc_clients_acquire_doorbells(&dev_priv->guc);
>> > intel_guc_acquire_doorbells();
>> Prefixed "i915_guc_clients" since this modifies submission state
>> (clients/doorbells). Should have kept dev_priv as parameter.
>> what should be the correct option here: intel_guc*(guc) or
>> i915_guc*(dev_priv)
>
> GuC submission is not i915. It is not part of the user facing api.
> Operate on intel_guc as you were.

So if GuC submission is not i915 than maybe to avoid confusion we should
rename i915_guc_submission.c to intel_guc_submission.c ?

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

Re: [PATCH 2/5] drm/i915/guc: Release all client doorbells on suspend and acquire on resume

[Sagar Arun Kamble]

On 11/7/2017 3:57 PM, Michal Wajdeczko wrote:
> On Tue, 07 Nov 2017 10:21:17 +0100, Chris Wilson 
> <chris@chris-wilson.co.uk> wrote:
>
>> Quoting Sagar Arun Kamble (2017-11-07 06:05:01)
>>>
>>>
>>> On 11/6/2017 5:43 PM, Chris Wilson wrote:
>>> > Quoting Sagar Arun Kamble (2017-11-05 13:39:40)
>>> >> Before GT device suspend, i915 should release GuC client 
>>> doorbells by
>>> >> stopping doorbell controller snooping and doorbell deallocation 
>>> through
>>> >> GuC. They need to be acquired again on resume. This will also 
>>> resolve
>>> >> the driver unload issue with GuC, where doorbell deallocation was 
>>> being
>>> >> done post GEM suspend.
>>> >> Release function is called from guc_suspend prior to sending 
>>> sleep action
>>> >> during runtime and drm suspend. Acquiral is different in runtime 
>>> and drm
>>> >> resume paths as on drm resume we are currently doing full reinit. 
>>> Release
>>> >> should be done in synchronization with acquire hence GuC 
>>> suspend/resume
>>> >> along with doorbell release/acquire should be under struct_mutex. 
>>> Upcoming
>>> >> suspend and resume restructuring for GuC will update these changes.
>>> >>
>>> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> >> Cc: Michał Winiarski <michal.winiarski@intel.com>
>>> >> Cc: Michel Thierry <michel.thierry@intel.com>
>>> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> >> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>>> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> >> ---
>>> >>   drivers/gpu/drm/i915/i915_drv.c            |  3 +++
>>> >>   drivers/gpu/drm/i915/i915_gem.c            |  5 +++--
>>> >>   drivers/gpu/drm/i915/i915_guc_submission.c | 20 
>>> ++++++++++++++++----
>>> >>   drivers/gpu/drm/i915/i915_guc_submission.h |  2 ++
>>> >>   drivers/gpu/drm/i915/intel_guc.c           |  2 ++
>>> >>   5 files changed, 26 insertions(+), 6 deletions(-)
>>> >>
>>> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c 
>>> b/drivers/gpu/drm/i915/i915_drv.c
>>> >> index e7e9e06..3df8a7d 100644
>>> >> --- a/drivers/gpu/drm/i915/i915_drv.c
>>> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>> >> @@ -47,6 +47,7 @@
>>> >>   #include <drm/i915_drm.h>
>>> >>
>>> >>   #include "i915_drv.h"
>>> >> +#include "i915_guc_submission.h"
>>> >>   #include "i915_trace.h"
>>> >>   #include "i915_vgpu.h"
>>> >>   #include "intel_drv.h"
>>> >> @@ -2615,6 +2616,8 @@ static int intel_runtime_resume(struct 
>>> device *kdev)
>>> >>
>>> >>          intel_guc_resume(dev_priv);
>>> >>
>>> >> + i915_guc_clients_acquire_doorbells(&dev_priv->guc);
>>> > intel_guc_acquire_doorbells();
>>> Prefixed "i915_guc_clients" since this modifies submission state
>>> (clients/doorbells). Should have kept dev_priv as parameter.
>>> what should be the correct option here: intel_guc*(guc) or
>>> i915_guc*(dev_priv)
>>
>> GuC submission is not i915. It is not part of the user facing api.
>> Operate on intel_guc as you were.
>
> So if GuC submission is not i915 than maybe to avoid confusion we should
> rename i915_guc_submission.c to intel_guc_submission.c ?
Yes. Will need to update functions that are i915_guc* as well. (spotted 
some in guc_log.c as well.)
>
> Michal

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

[PATCH 3/5] drm/i915/guc: Deactivate all client doorbells before reset

[Sagar Arun Kamble]

Prior to i915 reset, we need to deactivate all client doorbells
as they will need to be reacquired again post reset. We should
not execute GuC doorbell deallocation flow as GuC might be in
bad state (possibly reason for i915 reset).

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c            |  2 ++
 drivers/gpu/drm/i915/i915_guc_submission.c | 14 ++++++++++++++
 drivers/gpu/drm/i915/i915_guc_submission.h |  1 +
 3 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cbce714..879e318 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2963,6 +2963,8 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
 
 	i915_gem_revoke_fences(dev_priv);
 
+	i915_guc_clients_reset_prepare(&dev_priv->guc);
+
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b989edf..21f7fa7 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1113,6 +1113,20 @@ void i915_guc_clients_release_doorbells(struct intel_guc *guc)
 	destroy_doorbell(guc->execbuf_client, true);
 }
 
+void i915_guc_clients_reset_prepare(struct intel_guc *guc)
+{
+	if (!i915_modparams.enable_guc_submission)
+		return;
+
+	/*
+	 * We are only deactivating doorbell unit monitoring. GuC might
+	 * be in bad state and will be reset hence don't execute GuC
+	 * doorbell deallocation flow.
+	 */
+	destroy_doorbell(guc->preempt_client, false);
+	destroy_doorbell(guc->execbuf_client, false);
+}
+
 static void guc_policy_init(struct guc_policy *policy)
 {
 	policy->execution_quantum = POLICY_DEFAULT_EXECUTION_QUANTUM_US;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.h b/drivers/gpu/drm/i915/i915_guc_submission.h
index 1d5cf22..91ad505 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.h
+++ b/drivers/gpu/drm/i915/i915_guc_submission.h
@@ -74,6 +74,7 @@ struct i915_guc_client {
 
 void i915_guc_clients_acquire_doorbells(struct intel_guc *guc);
 void i915_guc_clients_release_doorbells(struct intel_guc *guc);
+void i915_guc_clients_reset_prepare(struct intel_guc *guc);
 int i915_guc_submission_init(struct drm_i915_private *dev_priv);
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
-- 
1.9.1

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

[PATCH 4/5] drm/i915/guc: Increase wait timeout for doorbell release status update

[Sagar Arun Kamble]

Current wait timeout of 10us is very tight as seen on SKL/KBL randomly
for pm_rpm@basic-pci-d3-state. Increase it to 50us.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 21f7fa7..3914415 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -222,7 +222,8 @@ static int __destroy_doorbell(struct i915_guc_client *client, bool notify_guc)
 	/* Doorbell release flow requires that we wait for GEN8_DRB_VALID bit
 	 * to go to zero after updating db_status before we call the GuC to
 	 * release the doorbell */
-	if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID), 10))
+	if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID),
+			50))
 		WARN_ONCE(true, "Doorbell never became invalid after disable\n");
 
 	if (notify_guc)
-- 
1.9.1

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

Re: [PATCH 4/5] drm/i915/guc: Increase wait timeout for doorbell release status update

[Michal Wajdeczko]

On Sun, 05 Nov 2017 14:39:42 +0100, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> Current wait timeout of 10us is very tight as seen on SKL/KBL randomly
> for pm_rpm@basic-pci-d3-state. Increase it to 50us.
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_guc_submission.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c  
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 21f7fa7..3914415 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -222,7 +222,8 @@ static int __destroy_doorbell(struct i915_guc_client  
> *client, bool notify_guc)
>  	/* Doorbell release flow requires that we wait for GEN8_DRB_VALID bit
>  	 * to go to zero after updating db_status before we call the GuC to
>  	 * release the doorbell */
> -	if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID),  
> 10))
> +	if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID),
> +			50))

Can we use intel_wait_for_register() here ?

>  		WARN_ONCE(true, "Doorbell never became invalid after disable\n");
> 	if (notify_guc)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] drm/i915/guc: Increase wait timeout for doorbell release status update

[Sagar Arun Kamble]

On 11/5/2017 9:25 PM, Michal Wajdeczko wrote:
> On Sun, 05 Nov 2017 14:39:42 +0100, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>> Current wait timeout of 10us is very tight as seen on SKL/KBL randomly
>> for pm_rpm@basic-pci-d3-state. Increase it to 50us.
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_guc_submission.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index 21f7fa7..3914415 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -222,7 +222,8 @@ static int __destroy_doorbell(struct 
>> i915_guc_client *client, bool notify_guc)
>>      /* Doorbell release flow requires that we wait for 
>> GEN8_DRB_VALID bit
>>       * to go to zero after updating db_status before we call the GuC to
>>       * release the doorbell */
>> -    if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & 
>> GEN8_DRB_VALID), 10))
>> +    if (wait_for_us(!(I915_READ(GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID),
>> +            50))
>
> Can we use intel_wait_for_register() here ?
Yes. Since wait timeout is expected to be in the order of uS (50us for 
now) will be using __intel_wait_for_register_fw.

+        if (__intel_wait_for_register_fw(dev_priv, GEN8_DRBREGL(db_id), 
GEN8_DRB_VALID, 0,
+ 50, 1, NULL))

Is this fine (with slow timeout of 1ms)?

>>          WARN_ONCE(true, "Doorbell never became invalid after 
>> disable\n");
>>     if (notify_guc)

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

[PATCH 5/5] HAX enable GuC submission for CI

[Sagar Arun Kamble]

Also revert ("drm/i915/guc: Assert that we switch between
known ggtt->invalidate functions")
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.h  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0684d5d..a351ddf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3562,17 +3562,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c729226..c38cef0 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -44,8 +44,8 @@
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc_loading, 0) \
-	param(int, enable_guc_submission, 0) \
+	param(int, enable_guc_loading, 1) \
+	param(int, enable_guc_submission, 1) \
 	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
-- 
1.9.1

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

✗ Fi.CI.BAT: warning for series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task

[Patchwork]

== Series Details ==

Series: series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
URL   : https://patchwork.freedesktop.org/series/33209/
State : warning

== Summary ==

Series 33209v1 series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
https://patchwork.freedesktop.org/api/1.0/series/33209/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c:
                dmesg-warn -> PASS       (fi-bsw-n3050)
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-cnl-y)
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-cnl-y)
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (fi-cnl-y)

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:444s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:454s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:380s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:538s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:276s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:510s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:512s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:503s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:486s
fi-cnl-y         total:289  pass:259  dwarn:3   dfail:0   fail:0   skip:27  time:626s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:425s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:265s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:588s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:435s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:430s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:428s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:497s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:466s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:574s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:590s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:577s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:450s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:592s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:649s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:517s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:502s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:464s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:570s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:428s

8b0ae6b50a229dc661a02f4034252ee854cc9b83 drm-tip: 2017y-11m-03d-17h-15m-57s UTC integration manifest
a91c31b1a24c HAX enable GuC submission for CI
32d332ce5a44 drm/i915/guc: Increase wait timeout for doorbell release status update
be93da66d7df drm/i915/guc: Deactivate all client doorbells before reset
b6515b105ab5 drm/i915/guc: Release all client doorbells on suspend and acquire on resume
9e190e74bacd drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6966/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task

[Patchwork]

== Series Details ==

Series: series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
URL   : https://patchwork.freedesktop.org/series/33209/
State : success

== Summary ==

Series 33209v1 series starting with [1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task
https://patchwork.freedesktop.org/api/1.0/series/33209/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c:
                dmesg-warn -> PASS       (fi-bsw-n3050) fdo#103479

fdo#103479 https://bugs.freedesktop.org/show_bug.cgi?id=103479

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:444s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:454s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:380s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:538s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:276s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:510s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:512s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:503s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:486s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:425s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:265s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:588s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:435s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:430s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:428s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:497s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:466s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:574s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:590s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:577s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:450s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:592s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:649s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:517s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:502s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:464s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:570s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:428s
Blacklisted hosts:
fi-cnl-y         total:289  pass:259  dwarn:3   dfail:0   fail:0   skip:27  time:626s

8b0ae6b50a229dc661a02f4034252ee854cc9b83 drm-tip: 2017y-11m-03d-17h-15m-57s UTC integration manifest
a91c31b1a24c HAX enable GuC submission for CI
32d332ce5a44 drm/i915/guc: Increase wait timeout for doorbell release status update
be93da66d7df drm/i915/guc: Deactivate all client doorbells before reset
b6515b105ab5 drm/i915/guc: Release all client doorbells on suspend and acquire on resume
9e190e74bacd drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6966/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable

[Michał Winiarski]

Originally removed in:
c1adab970348 ("drm/i915/guc: Remove failed doorbell stat from debugfs")
f1448a62a103 ("drm/i915/guc: Remove last submission result from debugfs")

Were accidentally restored in:
925344ccc91d ("BackMerge tag 'v4.12-rc5' into drm-next")

We can also remove unused variable and replace it with a WARN.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 3 +--
 drivers/gpu/drm/i915/intel_uc.h            | 4 ----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b28677e5a4f2..c180ff1423fd 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -601,7 +601,6 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	struct intel_guc *guc = &rq->i915->guc;
 	struct i915_guc_client *client = guc->execbuf_client;
 	unsigned long flags;
-	int b_ret;
 
 	/* WA to flush out the pending GMADR writes to ring buffer. */
 	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
@@ -610,7 +609,7 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	spin_lock_irqsave(&client->wq_lock, flags);
 
 	guc_wq_item_append(client, rq);
-	b_ret = guc_ring_doorbell(client);
+	WARN_ON(guc_ring_doorbell(client));
 
 	client->submissions[engine_id] += 1;
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b17b0f..69daf4c01cd0 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -59,10 +59,6 @@ struct drm_i915_gem_request;
  *                available in the work queue (note, the queue is shared,
  *                not per-engine). It is OK for this to be nonzero, but
  *                it should not be huge!
- *   b_fail: failed to ring the doorbell. This should never happen, unless
- *           somehow the hardware misbehaves, or maybe if the GuC firmware
- *           crashes? We probably need to reset the GPU to recover.
- *   retcode: errno from last guc_submit()
  */
 struct i915_guc_client {
 	struct i915_vma *vma;
-- 
2.13.5

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

[PATCH 5/5] HAX Enable GuC Submission for CI

[Michał Winiarski]

Also:
Revert "drm/i915/guc: Assert that we switch between known ggtt->invalidate functions"

This reverts commit 04f7b24eccdfae680a36e9825fe0d61dcd5ed528.
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.c  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 09e524dbc090..478a8d42aeb0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3189,17 +3189,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8ab003dca113..c9d72f1b8383 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 2,
+	.enable_guc_submission = 2,
 	.guc_log_level = -1,
 	.guc_firmware_path = NULL,
 	.huc_firmware_path = NULL,
-- 
2.13.5

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

[PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable

[Michał Winiarski]

Originally removed in:
c1adab970348 ("drm/i915/guc: Remove failed doorbell stat from debugfs")
f1448a62a103 ("drm/i915/guc: Remove last submission result from debugfs")

Were accidentally restored in:
925344ccc91d ("BackMerge tag 'v4.12-rc5' into drm-next")

We can also remove unused variable and replace it with a WARN.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 3 +--
 drivers/gpu/drm/i915/intel_uc.h            | 4 ----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 48a1e9349a2c..8a550785b257 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -602,7 +602,6 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	struct intel_guc *guc = &rq->i915->guc;
 	struct i915_guc_client *client = guc->execbuf_client;
 	unsigned long flags;
-	int b_ret;
 
 	/* WA to flush out the pending GMADR writes to ring buffer. */
 	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
@@ -611,7 +610,7 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	spin_lock_irqsave(&client->wq_lock, flags);
 
 	guc_wq_item_append(client, rq);
-	b_ret = guc_ring_doorbell(client);
+	WARN_ON(guc_ring_doorbell(client));
 
 	client->submissions[engine_id] += 1;
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b17b0f..69daf4c01cd0 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -59,10 +59,6 @@ struct drm_i915_gem_request;
  *                available in the work queue (note, the queue is shared,
  *                not per-engine). It is OK for this to be nonzero, but
  *                it should not be huge!
- *   b_fail: failed to ring the doorbell. This should never happen, unless
- *           somehow the hardware misbehaves, or maybe if the GuC firmware
- *           crashes? We probably need to reset the GPU to recover.
- *   retcode: errno from last guc_submit()
  */
 struct i915_guc_client {
 	struct i915_vma *vma;
-- 
2.13.5

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

[PATCH 5/5] HAX Enable GuC Submission for CI

[Michał Winiarski]

Also:
Revert "drm/i915/guc: Assert that we switch between known ggtt->invalidate functions"

This reverts commit 04f7b24eccdfae680a36e9825fe0d61dcd5ed528.
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.c  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 09e524dbc090..478a8d42aeb0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3189,17 +3189,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8ab003dca113..c9d72f1b8383 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 2,
+	.enable_guc_submission = 2,
 	.guc_log_level = -1,
 	.guc_firmware_path = NULL,
 	.huc_firmware_path = NULL,
-- 
2.13.5

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

[PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable

[Chris Wilson]

From: Michał Winiarski <michal.winiarski@intel.com>

Originally removed in:
c1adab970348 ("drm/i915/guc: Remove failed doorbell stat from debugfs")
f1448a62a103 ("drm/i915/guc: Remove last submission result from debugfs")

Were accidentaly restored in:
925344ccc91d ("BackMerge tag 'v4.12-rc5' into drm-next")

We can also remove unused variable and replace it with a WARN.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20170912124726.19689-1-michal.winiarski@intel.com
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 3 +--
 drivers/gpu/drm/i915/intel_uc.h            | 4 ----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 48a1e9349a2c..8a550785b257 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -602,7 +602,6 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	struct intel_guc *guc = &rq->i915->guc;
 	struct i915_guc_client *client = guc->execbuf_client;
 	unsigned long flags;
-	int b_ret;
 
 	/* WA to flush out the pending GMADR writes to ring buffer. */
 	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
@@ -611,7 +610,7 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	spin_lock_irqsave(&client->wq_lock, flags);
 
 	guc_wq_item_append(client, rq);
-	b_ret = guc_ring_doorbell(client);
+	WARN_ON(guc_ring_doorbell(client));
 
 	client->submissions[engine_id] += 1;
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b17b0f..69daf4c01cd0 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -59,10 +59,6 @@ struct drm_i915_gem_request;
  *                available in the work queue (note, the queue is shared,
  *                not per-engine). It is OK for this to be nonzero, but
  *                it should not be huge!
- *   b_fail: failed to ring the doorbell. This should never happen, unless
- *           somehow the hardware misbehaves, or maybe if the GuC firmware
- *           crashes? We probably need to reset the GPU to recover.
- *   retcode: errno from last guc_submit()
  */
 struct i915_guc_client {
 	struct i915_vma *vma;
-- 
2.14.1

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

[PATCH 5/5] HAX Enable GuC Submission for CI

[Chris Wilson]

From: Michał Winiarski <michal.winiarski@intel.com>

Also:
Revert "drm/i915/guc: Assert that we switch between known ggtt->invalidate functions"

This reverts commit 04f7b24eccdfae680a36e9825fe0d61dcd5ed528.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20170912132226.25629-1-michal.winiarski@intel.com
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.c  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 09e524dbc090..478a8d42aeb0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3189,17 +3189,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8ab003dca113..c9d72f1b8383 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 2,
+	.enable_guc_submission = 2,
 	.guc_log_level = -1,
 	.guc_firmware_path = NULL,
 	.huc_firmware_path = NULL,
-- 
2.14.1

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