[PATCH] drm/i915/breadcrumbs: Drop assertion that we've already enabled irqs

[PATCH] drm/i915/breadcrumbs: Drop assertion that we've already enabled irqs

[Chris Wilson]

The motivation for introducing the check that we only enable breadcrumb
irqs if the device's irq was installed was once upon a time we waited
during suspend after disabling interrupts (which was quite slow until
the bug was discovered). Since then we have the notion of pinning the
breadcrumb irq, broadening it from the sole purpose of user interrupt
notification and waiting, and more importantly decoupling it from a very
defined time period during which enabling the irq was expected. So stop
insisting the irq is installed before we setup our IMR masks, if the IER
isn't yet enabled, nothing will happen and we will timeout instead,
revealing the lack of irq in the hang debug messages.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_breadcrumbs.c | 28 ++++++++++--------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 4ed7105d7ff5..bfbff04c16aa 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -158,30 +158,24 @@ static void intel_breadcrumbs_fake_irq(struct timer_list *t)
 
 static void irq_enable(struct intel_engine_cs *engine)
 {
-	/*
-	 * FIXME: Ideally we want this on the API boundary, but for the
-	 * sake of testing with mock breadcrumbs (no HW so unable to
-	 * enable irqs) we place it deep within the bowels, at the point
-	 * of no return.
-	 */
-	GEM_BUG_ON(!intel_irqs_enabled(engine->i915));
+	if (!engine->irq_enable)
+		return;
 
 	/* Caller disables interrupts */
-	if (engine->irq_enable) {
-		spin_lock(&engine->i915->irq_lock);
-		engine->irq_enable(engine);
-		spin_unlock(&engine->i915->irq_lock);
-	}
+	spin_lock(&engine->i915->irq_lock);
+	engine->irq_enable(engine);
+	spin_unlock(&engine->i915->irq_lock);
 }
 
 static void irq_disable(struct intel_engine_cs *engine)
 {
+	if (!engine->irq_disable)
+		return;
+
 	/* Caller disables interrupts */
-	if (engine->irq_disable) {
-		spin_lock(&engine->i915->irq_lock);
-		engine->irq_disable(engine);
-		spin_unlock(&engine->i915->irq_lock);
-	}
+	spin_lock(&engine->i915->irq_lock);
+	engine->irq_disable(engine);
+	spin_unlock(&engine->i915->irq_lock);
 }
 
 void __intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
-- 
2.20.1

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

✓ Fi.CI.BAT: success for drm/i915/breadcrumbs: Drop assertion that we've already enabled irqs

[Patchwork]

== Series Details ==

Series: drm/i915/breadcrumbs: Drop assertion that we've already enabled irqs
URL   : https://patchwork.freedesktop.org/series/55388/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5443 -> Patchwork_11977
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55388/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_11977 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       PASS -> DMESG-WARN [fdo#108965]

  * igt@gem_exec_suspend@basic-s3:
    - fi-skl-6700k2:      PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       PASS -> FAIL [fdo#108767]

  
#### Possible fixes ####

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       {SKIP} [fdo#109271] / [fdo#109278] -> PASS +2

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@pm_rpm@basic-pci-d3-state:
    - fi-byt-j1900:       {SKIP} [fdo#109271] -> PASS

  * igt@pm_rpm@basic-rte:
    - fi-byt-j1900:       FAIL [fdo#108800] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (48 -> 44)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (5): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5443 -> Patchwork_11977

  CI_DRM_5443: 62c660c0385bee9e07ef59265f95e66fb536753e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4777: 8614d5eb114a660c3bd7ff77eab8bed53424cd30 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11977: 5cc8b2907dce7e07ef6d6dc7692e1953ca4f244d @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5cc8b2907dce drm/i915/breadcrumbs: Drop assertion that we've already enabled irqs

== Logs ==

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

Re: [PATCH] drm/i915/breadcrumbs: Drop assertion that we've already enabled irqs

[Tvrtko Ursulin]

On 17/01/2019 23:31, Chris Wilson wrote:
> The motivation for introducing the check that we only enable breadcrumb
> irqs if the device's irq was installed was once upon a time we waited
> during suspend after disabling interrupts (which was quite slow until
> the bug was discovered). Since then we have the notion of pinning the
> breadcrumb irq, broadening it from the sole purpose of user interrupt
> notification and waiting, and more importantly decoupling it from a very
> defined time period during which enabling the irq was expected. So stop
> insisting the irq is installed before we setup our IMR masks, if the IER
> isn't yet enabled, nothing will happen and we will timeout instead,
> revealing the lack of irq in the hang debug messages.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_breadcrumbs.c | 28 ++++++++++--------------
>   1 file changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> index 4ed7105d7ff5..bfbff04c16aa 100644
> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> @@ -158,30 +158,24 @@ static void intel_breadcrumbs_fake_irq(struct timer_list *t)
>   
>   static void irq_enable(struct intel_engine_cs *engine)
>   {
> -	/*
> -	 * FIXME: Ideally we want this on the API boundary, but for the
> -	 * sake of testing with mock breadcrumbs (no HW so unable to
> -	 * enable irqs) we place it deep within the bowels, at the point
> -	 * of no return.
> -	 */
> -	GEM_BUG_ON(!intel_irqs_enabled(engine->i915));
> +	if (!engine->irq_enable)
> +		return;
>   
>   	/* Caller disables interrupts */
> -	if (engine->irq_enable) {
> -		spin_lock(&engine->i915->irq_lock);
> -		engine->irq_enable(engine);
> -		spin_unlock(&engine->i915->irq_lock);
> -	}
> +	spin_lock(&engine->i915->irq_lock);
> +	engine->irq_enable(engine);
> +	spin_unlock(&engine->i915->irq_lock);
>   }
>   
>   static void irq_disable(struct intel_engine_cs *engine)
>   {
> +	if (!engine->irq_disable)
> +		return;
> +
>   	/* Caller disables interrupts */
> -	if (engine->irq_disable) {
> -		spin_lock(&engine->i915->irq_lock);
> -		engine->irq_disable(engine);
> -		spin_unlock(&engine->i915->irq_lock);
> -	}
> +	spin_lock(&engine->i915->irq_lock);
> +	engine->irq_disable(engine);
> +	spin_unlock(&engine->i915->irq_lock);
>   }
>   
>   void __intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

✓ Fi.CI.IGT: success for drm/i915/breadcrumbs: Drop assertion that we've already enabled irqs

[Patchwork]

== Series Details ==

Series: drm/i915/breadcrumbs: Drop assertion that we've already enabled irqs
URL   : https://patchwork.freedesktop.org/series/55388/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5443_full -> Patchwork_11977_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_11977_full:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@runner@aborted}:
    - shard-apl:          ( 3 FAIL ) -> ( 11 FAIL )

  
Known issues
------------

  Here are the changes found in Patchwork_11977_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#103558] / [fdo#105602] / [fdo#107956] +1

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-apl:          PASS -> FAIL [fdo#106510] / [fdo#108145]

  * igt@kms_color@pipe-a-ctm-max:
    - shard-apl:          PASS -> FAIL [fdo#108147]

  * igt@kms_color@pipe-a-degamma:
    - shard-apl:          PASS -> FAIL [fdo#104782] / [fdo#108145]

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-glk:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions:
    - shard-apl:          NOTRUN -> INCOMPLETE [fdo#103927]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-apl:          PASS -> DMESG-FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move:
    - shard-glk:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#103558] / [fdo#105602] +26

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1

  
#### Possible fixes ####

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-glk:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-glk:          FAIL [fdo#103060] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-glk:          FAIL [fdo#103167] -> PASS +4

  * igt@kms_frontbuffer_tracking@fbc-2p-rte:
    - shard-glk:          FAIL [fdo#103167] / [fdo#105682] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-glk:          FAIL [fdo#103166] -> PASS

  * igt@kms_setmode@basic:
    - shard-glk:          FAIL [fdo#99912] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * Linux: CI_DRM_5443 -> Patchwork_11977

  CI_DRM_5443: 62c660c0385bee9e07ef59265f95e66fb536753e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4777: 8614d5eb114a660c3bd7ff77eab8bed53424cd30 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11977: 5cc8b2907dce7e07ef6d6dc7692e1953ca4f244d @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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