[PATCH] drm/i915: Move the irq_barrier for reset earlier into reset_prepare

[PATCH] drm/i915: Move the irq_barrier for reset earlier into reset_prepare

[Chris Wilson]

When updating the bookkeeping following the reset, we need the seqno to
be coherent on the CPU prior to trusting its result for deciding whether
any request is completed. We need the irq_barrier before we start making
these decisions, i.e. in reset_prepare.

References: https://bugs.freedesktop.org/show_bug.cgi?id=99733
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ed60d5881b40..3066f94da8f0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2677,6 +2677,9 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
 		tasklet_disable(&engine->irq_tasklet);
 		tasklet_kill(&engine->irq_tasklet);
 
+		if (engine->irq_seqno_barrier)
+			engine->irq_seqno_barrier(engine);
+
 		if (engine_stalled(engine)) {
 			request = i915_gem_find_active_request(engine);
 			if (request && request->fence.error == -EIO)
@@ -2773,9 +2776,6 @@ static void i915_gem_reset_engine(struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_request *request;
 
-	if (engine->irq_seqno_barrier)
-		engine->irq_seqno_barrier(engine);
-
 	request = i915_gem_find_active_request(engine);
 	if (request && i915_gem_reset_request(request)) {
 		DRM_DEBUG_DRIVER("resetting %s to restart from tail of request 0x%x\n",
-- 
2.11.0

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

Re: [PATCH] drm/i915: Move the irq_barrier for reset earlier into reset_prepare

[Michel Thierry]

On 10/02/17 10:52, Chris Wilson wrote:
> When updating the bookkeeping following the reset, we need the seqno to
> be coherent on the CPU prior to trusting its result for deciding whether
> any request is completed. We need the irq_barrier before we start making
> these decisions, i.e. in reset_prepare.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=99733
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>

I havent seen the issue in the platforms I have, but makes sense. If you 
need one,

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ed60d5881b40..3066f94da8f0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2677,6 +2677,9 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
>                 tasklet_disable(&engine->irq_tasklet);
>                 tasklet_kill(&engine->irq_tasklet);
>
> +               if (engine->irq_seqno_barrier)
> +                       engine->irq_seqno_barrier(engine);
> +
>                 if (engine_stalled(engine)) {
>                         request = i915_gem_find_active_request(engine);
>                         if (request && request->fence.error == -EIO)
> @@ -2773,9 +2776,6 @@ static void i915_gem_reset_engine(struct intel_engine_cs *engine)
>  {
>         struct drm_i915_gem_request *request;
>
> -       if (engine->irq_seqno_barrier)
> -               engine->irq_seqno_barrier(engine);
> -
>         request = i915_gem_find_active_request(engine);
>         if (request && i915_gem_reset_request(request)) {
>                 DRM_DEBUG_DRIVER("resetting %s to restart from tail of request 0x%x\n",
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Move the irq_barrier for reset earlier into reset_prepare

[Patchwork]

== Series Details ==

Series: drm/i915: Move the irq_barrier for reset earlier into reset_prepare
URL   : https://patchwork.freedesktop.org/series/19473/
State : failure

== Summary ==

Series 19473v1 drm/i915: Move the irq_barrier for reset earlier into reset_prepare
https://patchwork.freedesktop.org/api/1.0/series/19473/revisions/1/mbox/

Test gem_exec_parse:
        Subgroup basic-allowed:
                pass       -> INCOMPLETE (fi-byt-j1900)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (fi-ivb-3770)

fi-bdw-5557u     total:252  pass:241  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:252  pass:213  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:69   pass:59   dwarn:0   dfail:0   fail:0   skip:9  
fi-byt-n2820     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:252  pass:202  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:252  pass:235  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:252  pass:230  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:252  pass:224  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:252  pass:223  dwarn:0   dfail:0   fail:0   skip:29 

c02eede7256a86ccd8308612491aa5e350aad8db drm-tip: 2017y-02m-10d-19h-11m-09s UTC integration manifest
48a5eaf drm/i915: Move the irq_barrier for reset earlier into reset_prepare

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3773/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Move the irq_barrier for reset earlier into reset_prepare

[Chris Wilson]

On Fri, Feb 10, 2017 at 12:05:53PM -0800, Michel Thierry wrote:
> 
> On 10/02/17 10:52, Chris Wilson wrote:
> >When updating the bookkeeping following the reset, we need the seqno to
> >be coherent on the CPU prior to trusting its result for deciding whether
> >any request is completed. We need the irq_barrier before we start making
> >these decisions, i.e. in reset_prepare.
> >
> >References: https://bugs.freedesktop.org/show_bug.cgi?id=99733
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> >---
> > drivers/gpu/drm/i915/i915_gem.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> 
> I havent seen the issue in the platforms I have, but makes sense.

Ironlake is the most special of all. There's no flush control available
to order the posting of writes and the interrupt, so there's just a big
sleep and hope for the best. (Not that the story is any better for
gen6/7.)

> If you need one,
> 
> Reviewed-by: Michel Thierry <michel.thierry@intel.com>

Thanks,
-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