[Intel-gfx] [PATCH 17/27] drm/i915/guc: Flush G2H work queue during reset

2021-08-25 Thread Matthew Brost
It isn't safe to scrub for missing G2H or continue with the reset until
all G2H processing is complete. Flush the G2H work queue during reset to
ensure it is done running. No need to call the IRQ handler directly
either as the scrubbing code can deal with any missing G2H.

Fixes: eb5e7da736f3 ("drm/i915/guc: Reset implementation for new GuC interface")
Signed-off-by: Matthew Brost 
Reviewed-by: Daniele Ceraolo Spurio 
---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c  | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 2949dce57489..baf789f37d42 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -714,8 +714,6 @@ static void guc_flush_submissions(struct intel_guc *guc)
 
 void intel_guc_submission_reset_prepare(struct intel_guc *guc)
 {
-   int i;
-
if (unlikely(!guc_submission_initialized(guc))) {
/* Reset called during driver load? GuC not yet initialised! */
return;
@@ -731,20 +729,8 @@ void intel_guc_submission_reset_prepare(struct intel_guc 
*guc)
 
guc_flush_submissions(guc);
 
-   /*
-* Handle any outstanding G2Hs before reset. Call IRQ handler directly
-* each pass as interrupt have been disabled. We always scrub for
-* outstanding G2H as it is possible for outstanding_submission_g2h to
-* be incremented after the context state update.
-*/
-   for (i = 0; i < 4 && atomic_read(>outstanding_submission_g2h); 
++i) {
-   intel_guc_to_host_event_handler(guc);
-#define wait_for_reset(guc, wait_var) \
-   intel_guc_wait_for_pending_msg(guc, wait_var, false, (HZ / 20))
-   do {
-   wait_for_reset(guc, >outstanding_submission_g2h);
-   } while (!list_empty(>ct.requests.incoming));
-   }
+   flush_work(>ct.requests.worker);
+
scrub_guc_desc_for_outstanding_g2h(guc);
 }
 
-- 
2.32.0



Re: [Intel-gfx] [PATCH 17/27] drm/i915/guc: Flush G2H work queue during reset

2021-08-24 Thread Daniele Ceraolo Spurio




On 8/24/2021 8:44 AM, Matthew Brost wrote:

On Fri, Aug 20, 2021 at 05:25:41PM -0700, Daniele Ceraolo Spurio wrote:


On 8/18/2021 11:16 PM, Matthew Brost wrote:

It isn't safe to scrub for missing G2H or continue with the reset until
all G2H processing is complete. Flush the G2H work queue during reset to
ensure it is done running.

Might be worth moving this patch closer to "drm/i915/guc: Process all G2H
message at once in work queue".


Sure.


Fixes: eb5e7da736f3 ("drm/i915/guc: Reset implementation for new GuC interface")
Signed-off-by: Matthew Brost 
---
   .../gpu/drm/i915/gt/uc/intel_guc_submission.c  | 18 ++
   1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 4cf5a565f08e..9a53bae367b1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -714,8 +714,6 @@ static void guc_flush_submissions(struct intel_guc *guc)
   void intel_guc_submission_reset_prepare(struct intel_guc *guc)
   {
-   int i;
-
if (unlikely(!guc_submission_initialized(guc))) {
/* Reset called during driver load? GuC not yet initialised! */
return;
@@ -731,20 +729,8 @@ void intel_guc_submission_reset_prepare(struct intel_guc 
*guc)
guc_flush_submissions(guc);
-   /*
-* Handle any outstanding G2Hs before reset. Call IRQ handler directly
-* each pass as interrupt have been disabled. We always scrub for
-* outstanding G2H as it is possible for outstanding_submission_g2h to
-* be incremented after the context state update.
-*/
-   for (i = 0; i < 4 && atomic_read(>outstanding_submission_g2h); 
++i) {
-   intel_guc_to_host_event_handler(guc);
-#define wait_for_reset(guc, wait_var) \
-   intel_guc_wait_for_pending_msg(guc, wait_var, false, (HZ / 20))
-   do {
-   wait_for_reset(guc, >outstanding_submission_g2h);
-   } while (!list_empty(>ct.requests.incoming));
-   }
+   flush_work(>ct.requests.worker);
+

We're now not waiting in the requests anymore, just ensuring that the
processing of the ones we already received is done. Is this intended? We do
still handle the remaining oustanding submission in the scrub so it's
functionally correct, but the commit message doesn't state the change in
waiting behavior, so wanted to double check it was planned.


Yes, it is planned as scrub code should be able to cope with any missing
G2H. Will update the commit message to reflect that.


With the updated commit msg:

Reviewed-by: Daniele Ceraolo Spurio 

Daniele



Matt


Daniele


scrub_guc_desc_for_outstanding_g2h(guc);
   }




Re: [Intel-gfx] [PATCH 17/27] drm/i915/guc: Flush G2H work queue during reset

2021-08-24 Thread Matthew Brost
On Fri, Aug 20, 2021 at 05:25:41PM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 8/18/2021 11:16 PM, Matthew Brost wrote:
> > It isn't safe to scrub for missing G2H or continue with the reset until
> > all G2H processing is complete. Flush the G2H work queue during reset to
> > ensure it is done running.
> 
> Might be worth moving this patch closer to "drm/i915/guc: Process all G2H
> message at once in work queue".
> 

Sure.

> > Fixes: eb5e7da736f3 ("drm/i915/guc: Reset implementation for new GuC 
> > interface")
> > Signed-off-by: Matthew Brost 
> > ---
> >   .../gpu/drm/i915/gt/uc/intel_guc_submission.c  | 18 ++
> >   1 file changed, 2 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
> > b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > index 4cf5a565f08e..9a53bae367b1 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > @@ -714,8 +714,6 @@ static void guc_flush_submissions(struct intel_guc *guc)
> >   void intel_guc_submission_reset_prepare(struct intel_guc *guc)
> >   {
> > -   int i;
> > -
> > if (unlikely(!guc_submission_initialized(guc))) {
> > /* Reset called during driver load? GuC not yet initialised! */
> > return;
> > @@ -731,20 +729,8 @@ void intel_guc_submission_reset_prepare(struct 
> > intel_guc *guc)
> > guc_flush_submissions(guc);
> > -   /*
> > -* Handle any outstanding G2Hs before reset. Call IRQ handler directly
> > -* each pass as interrupt have been disabled. We always scrub for
> > -* outstanding G2H as it is possible for outstanding_submission_g2h to
> > -* be incremented after the context state update.
> > -*/
> > -   for (i = 0; i < 4 && atomic_read(>outstanding_submission_g2h); 
> > ++i) {
> > -   intel_guc_to_host_event_handler(guc);
> > -#define wait_for_reset(guc, wait_var) \
> > -   intel_guc_wait_for_pending_msg(guc, wait_var, false, (HZ / 20))
> > -   do {
> > -   wait_for_reset(guc, >outstanding_submission_g2h);
> > -   } while (!list_empty(>ct.requests.incoming));
> > -   }
> > +   flush_work(>ct.requests.worker);
> > +
> 
> We're now not waiting in the requests anymore, just ensuring that the
> processing of the ones we already received is done. Is this intended? We do
> still handle the remaining oustanding submission in the scrub so it's
> functionally correct, but the commit message doesn't state the change in
> waiting behavior, so wanted to double check it was planned.
> 

Yes, it is planned as scrub code should be able to cope with any missing
G2H. Will update the commit message to reflect that.

Matt

> Daniele
> 
> > scrub_guc_desc_for_outstanding_g2h(guc);
> >   }
> 


Re: [Intel-gfx] [PATCH 17/27] drm/i915/guc: Flush G2H work queue during reset

2021-08-20 Thread Daniele Ceraolo Spurio




On 8/18/2021 11:16 PM, Matthew Brost wrote:

It isn't safe to scrub for missing G2H or continue with the reset until
all G2H processing is complete. Flush the G2H work queue during reset to
ensure it is done running.


Might be worth moving this patch closer to "drm/i915/guc: Process all 
G2H message at once in work queue".



Fixes: eb5e7da736f3 ("drm/i915/guc: Reset implementation for new GuC interface")
Signed-off-by: Matthew Brost 
---
  .../gpu/drm/i915/gt/uc/intel_guc_submission.c  | 18 ++
  1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 4cf5a565f08e..9a53bae367b1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -714,8 +714,6 @@ static void guc_flush_submissions(struct intel_guc *guc)
  
  void intel_guc_submission_reset_prepare(struct intel_guc *guc)

  {
-   int i;
-
if (unlikely(!guc_submission_initialized(guc))) {
/* Reset called during driver load? GuC not yet initialised! */
return;
@@ -731,20 +729,8 @@ void intel_guc_submission_reset_prepare(struct intel_guc 
*guc)
  
  	guc_flush_submissions(guc);
  
-	/*

-* Handle any outstanding G2Hs before reset. Call IRQ handler directly
-* each pass as interrupt have been disabled. We always scrub for
-* outstanding G2H as it is possible for outstanding_submission_g2h to
-* be incremented after the context state update.
-*/
-   for (i = 0; i < 4 && atomic_read(>outstanding_submission_g2h); 
++i) {
-   intel_guc_to_host_event_handler(guc);
-#define wait_for_reset(guc, wait_var) \
-   intel_guc_wait_for_pending_msg(guc, wait_var, false, (HZ / 20))
-   do {
-   wait_for_reset(guc, >outstanding_submission_g2h);
-   } while (!list_empty(>ct.requests.incoming));
-   }
+   flush_work(>ct.requests.worker);
+


We're now not waiting in the requests anymore, just ensuring that the 
processing of the ones we already received is done. Is this intended? We 
do still handle the remaining oustanding submission in the scrub so it's 
functionally correct, but the commit message doesn't state the change in 
waiting behavior, so wanted to double check it was planned.


Daniele


scrub_guc_desc_for_outstanding_g2h(guc);
  }
  




[Intel-gfx] [PATCH 17/27] drm/i915/guc: Flush G2H work queue during reset

2021-08-19 Thread Matthew Brost
It isn't safe to scrub for missing G2H or continue with the reset until
all G2H processing is complete. Flush the G2H work queue during reset to
ensure it is done running.

Fixes: eb5e7da736f3 ("drm/i915/guc: Reset implementation for new GuC interface")
Signed-off-by: Matthew Brost 
---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c  | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 4cf5a565f08e..9a53bae367b1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -714,8 +714,6 @@ static void guc_flush_submissions(struct intel_guc *guc)
 
 void intel_guc_submission_reset_prepare(struct intel_guc *guc)
 {
-   int i;
-
if (unlikely(!guc_submission_initialized(guc))) {
/* Reset called during driver load? GuC not yet initialised! */
return;
@@ -731,20 +729,8 @@ void intel_guc_submission_reset_prepare(struct intel_guc 
*guc)
 
guc_flush_submissions(guc);
 
-   /*
-* Handle any outstanding G2Hs before reset. Call IRQ handler directly
-* each pass as interrupt have been disabled. We always scrub for
-* outstanding G2H as it is possible for outstanding_submission_g2h to
-* be incremented after the context state update.
-*/
-   for (i = 0; i < 4 && atomic_read(>outstanding_submission_g2h); 
++i) {
-   intel_guc_to_host_event_handler(guc);
-#define wait_for_reset(guc, wait_var) \
-   intel_guc_wait_for_pending_msg(guc, wait_var, false, (HZ / 20))
-   do {
-   wait_for_reset(guc, >outstanding_submission_g2h);
-   } while (!list_empty(>ct.requests.incoming));
-   }
+   flush_work(>ct.requests.worker);
+
scrub_guc_desc_for_outstanding_g2h(guc);
 }
 
-- 
2.32.0