[PATCH] drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()

[PATCH] drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()

[Chris Wilson]

We indirectly hold the runtime-pm for the intel_lrc_irq_handler() by
virtue of dev_priv->gt.awake keeping a wakeref whilst the requests are
busy. As this is not obvious from the code, add a comment.

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

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0dc1cc4ad6e7..e16cc28dc783 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -515,6 +515,15 @@ static void intel_lrc_irq_handler(unsigned long data)
 	struct execlist_port *port = engine->execlist_port;
 	struct drm_i915_private *dev_priv = engine->i915;
 
+	/* We can skip acquiring intel_runtime_pm_get() here as it was taken
+	 * on our behalf by the request (see i915_gem_mark_busy ()) and it will
+	 * not be relinquished until the device is idle (see
+	 * i915_gem_idle_work_handler()). As a precaution, we make sure
+	 * that all ELSP are drained i.e. we have processed the CSB,
+	 * before allowing ourselves to idle and calling intel_runtime_pm_put().
+	 */
+	GEM_BUG_ON(!dev_priv->gt.awake);
+
 	intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
 
 	/* Prefer doing test_and_clear_bit() as a two stage operation to avoid
-- 
2.11.0

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

✗ Fi.CI.BAT: warning for drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()
URL   : https://patchwork.freedesktop.org/series/22790/
State : warning

== Summary ==

Series 22790v1 drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()
https://patchwork.freedesktop.org/api/1.0/series/22790/revisions/1/mbox/

Test gem_exec_basic:
        Subgroup basic-render:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup gtt-bsd:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup gtt-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup gtt-render:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
        Subgroup readonly-default:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
        Subgroup readonly-render:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
Test gem_exec_reloc:
        Subgroup basic-cpu-read-noreloc:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-gtt-read:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-gtt-read-noreloc:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup basic-write-cpu:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup basic-write-gtt-active:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
Test gem_exec_store:
        Subgroup basic-all:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
        Subgroup basic-blt:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup basic-default:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup basic-render:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
Test gem_mmap_gtt:
        Subgroup basic-read-write:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
        Subgroup basic-read-write-distinct:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup basic-small-bo:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
Test gem_ringfill:
        Subgroup basic-default-fd:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test kms_addfb_basic:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-x-tiled:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup framebuffer-vs-set-tiling:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup invalid-get-prop:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup size-max:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup small-bo:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup unused-modifier:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup unused-offsets:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (fi-kbl-7560u)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-kbl-7560u)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100445
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                pass       -> DMESG-WARN (fi-snb-2600)
Test prime_self_import:
        Subgroup basic-with_fd_dup:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-with_one_bo:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
Test prime_vgem:
        Subgroup basic-busy-default:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
        Subgroup basic-fence-flip:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup basic-fence-mmap:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
        Subgroup basic-fence-read:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup basic-read:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup basic-sync-default:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643

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

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:431s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:434s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:572s
WARNING: Long output truncated

e461ecfd413fb930d00f44f3de0019e528b4731f drm-tip: 2017y-04m-10d-14h-25m-24s UTC integration manifest
e51f5a0 drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()

== Logs ==

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

[PATCH] drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()

[Chris Wilson]

We indirectly hold the runtime-pm for the intel_lrc_irq_handler() by
virtue of dev_priv->gt.awake keeping a wakeref whilst the requests are
busy. As this is not obvious from the code, add a comment.

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

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0dc1cc4ad6e7..e16cc28dc783 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -515,6 +515,15 @@ static void intel_lrc_irq_handler(unsigned long data)
 	struct execlist_port *port = engine->execlist_port;
 	struct drm_i915_private *dev_priv = engine->i915;
 
+	/* We can skip acquiring intel_runtime_pm_get() here as it was taken
+	 * on our behalf by the request (see i915_gem_mark_busy ()) and it will
+	 * not be relinquished until the device is idle (see
+	 * i915_gem_idle_work_handler()). As a precaution, we make sure
+	 * that all ELSP are drained i.e. we have processed the CSB,
+	 * before allowing ourselves to idle and calling intel_runtime_pm_put().
+	 */
+	GEM_BUG_ON(!dev_priv->gt.awake);
+
 	intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
 
 	/* Prefer doing test_and_clear_bit() as a two stage operation to avoid
-- 
2.11.0

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

✓ Fi.CI.BAT: success for drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler() (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler() (rev2)
URL   : https://patchwork.freedesktop.org/series/22790/
State : success

== Summary ==

Series 22790v2 drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()
https://patchwork.freedesktop.org/api/1.0/series/22790/revisions/2/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:431s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:426s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:575s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:511s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:478s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:412s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:401s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:425s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:487s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:463s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:452s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:566s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:453s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:569s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:473s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:482s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:431s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:533s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:404s
fi-byt-j1900 failed to collect. IGT log at Patchwork_4479/fi-byt-j1900/igt.log

c77055e7cbbc6f925498f386c498bd0f2d7c6bdb drm-tip: 2017y-04m-11d-18h-46m-29s UTC integration manifest
9c6c070 drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()

== Logs ==

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

Re: [PATCH] drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()

[Tvrtko Ursulin]

On 11/04/2017 18:58, Chris Wilson wrote:
> We indirectly hold the runtime-pm for the intel_lrc_irq_handler() by
> virtue of dev_priv->gt.awake keeping a wakeref whilst the requests are
> busy. As this is not obvious from the code, add a comment.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0dc1cc4ad6e7..e16cc28dc783 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -515,6 +515,15 @@ static void intel_lrc_irq_handler(unsigned long data)
>  	struct execlist_port *port = engine->execlist_port;
>  	struct drm_i915_private *dev_priv = engine->i915;
>
> +	/* We can skip acquiring intel_runtime_pm_get() here as it was taken
> +	 * on our behalf by the request (see i915_gem_mark_busy ()) and it will
> +	 * not be relinquished until the device is idle (see
> +	 * i915_gem_idle_work_handler()). As a precaution, we make sure
> +	 * that all ELSP are drained i.e. we have processed the CSB,
> +	 * before allowing ourselves to idle and calling intel_runtime_pm_put().
> +	 */
> +	GEM_BUG_ON(!dev_priv->gt.awake);
> +
>  	intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
>
>  	/* Prefer doing test_and_clear_bit() as a two stage operation to avoid
>

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

It feels a big to c&p, but not sure if it worth putting a comment "see 
comment in intel_lrc_irq_handler on why we don't have to " to 
i915_guc_irq_handler or maybe dequeue?

Regards,

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

Re: [PATCH] drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()

[Chris Wilson]

On Wed, Apr 12, 2017 at 08:12:17AM +0100, Tvrtko Ursulin wrote:
> 
> On 11/04/2017 18:58, Chris Wilson wrote:
> >We indirectly hold the runtime-pm for the intel_lrc_irq_handler() by
> >virtue of dev_priv->gt.awake keeping a wakeref whilst the requests are
> >busy. As this is not obvious from the code, add a comment.
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >---
> > drivers/gpu/drm/i915/intel_lrc.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index 0dc1cc4ad6e7..e16cc28dc783 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -515,6 +515,15 @@ static void intel_lrc_irq_handler(unsigned long data)
> > 	struct execlist_port *port = engine->execlist_port;
> > 	struct drm_i915_private *dev_priv = engine->i915;
> >
> >+	/* We can skip acquiring intel_runtime_pm_get() here as it was taken
> >+	 * on our behalf by the request (see i915_gem_mark_busy ()) and it will
> >+	 * not be relinquished until the device is idle (see
> >+	 * i915_gem_idle_work_handler()). As a precaution, we make sure
> >+	 * that all ELSP are drained i.e. we have processed the CSB,
> >+	 * before allowing ourselves to idle and calling intel_runtime_pm_put().
> >+	 */
> >+	GEM_BUG_ON(!dev_priv->gt.awake);
> >+
> > 	intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
> >
> > 	/* Prefer doing test_and_clear_bit() as a two stage operation to avoid
> >
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> It feels a big to c&p, but not sure if it worth putting a comment
> "see comment in intel_lrc_irq_handler on why we don't have to " to
> i915_guc_irq_handler or maybe dequeue?

For guc, the requirement for rpm is much less strict - the interface
with the guc is through ordinary memory not device memory, so we don't
need rpm_get (aiui). There is the mmio readback for !llc to ensure that
all GTT writes are flushed to system memory prior to dispatching the
workqueue -- that technically doesn't require rpm either.

I'm a bit more hesistant about adding such a comment to the guc atm, as
I think I will be more misleading then helpful.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/execlists: Document runtime pm for intel_lrc_irq_handler()

[Tvrtko Ursulin]

On 12/04/2017 09:31, Chris Wilson wrote:
> On Wed, Apr 12, 2017 at 08:12:17AM +0100, Tvrtko Ursulin wrote:
>>
>> On 11/04/2017 18:58, Chris Wilson wrote:
>>> We indirectly hold the runtime-pm for the intel_lrc_irq_handler() by
>>> virtue of dev_priv->gt.awake keeping a wakeref whilst the requests are
>>> busy. As this is not obvious from the code, add a comment.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_lrc.c | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>>> index 0dc1cc4ad6e7..e16cc28dc783 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -515,6 +515,15 @@ static void intel_lrc_irq_handler(unsigned long data)
>>> 	struct execlist_port *port = engine->execlist_port;
>>> 	struct drm_i915_private *dev_priv = engine->i915;
>>>
>>> +	/* We can skip acquiring intel_runtime_pm_get() here as it was taken
>>> +	 * on our behalf by the request (see i915_gem_mark_busy ()) and it will
>>> +	 * not be relinquished until the device is idle (see
>>> +	 * i915_gem_idle_work_handler()). As a precaution, we make sure
>>> +	 * that all ELSP are drained i.e. we have processed the CSB,
>>> +	 * before allowing ourselves to idle and calling intel_runtime_pm_put().
>>> +	 */
>>> +	GEM_BUG_ON(!dev_priv->gt.awake);
>>> +
>>> 	intel_uncore_forcewake_get(dev_priv, engine->fw_domains);
>>>
>>> 	/* Prefer doing test_and_clear_bit() as a two stage operation to avoid
>>>
>>
>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> It feels a big to c&p, but not sure if it worth putting a comment
>> "see comment in intel_lrc_irq_handler on why we don't have to " to
>> i915_guc_irq_handler or maybe dequeue?
>
> For guc, the requirement for rpm is much less strict - the interface
> with the guc is through ordinary memory not device memory, so we don't
> need rpm_get (aiui). There is the mmio readback for !llc to ensure that
> all GTT writes are flushed to system memory prior to dispatching the
> workqueue -- that technically doesn't require rpm either.
>
> I'm a bit more hesistant about adding such a comment to the guc atm, as
> I think I will be more misleading then helpful.

I am not sure but was thinking that presumably device needs to be awake 
as we are adding stuff into the GuC's workqueue.

Regards,

Tvrtko


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