[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit
== Series Details == Series: drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit URL : https://patchwork.freedesktop.org/series/48048/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4645_full -> Patchwork_9923_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_9923_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9923_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9923_full: === IGT changes === Warnings igt@kms_chv_cursor_fail@pipe-a-128x128-top-edge: shard-snb: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_9923_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@fence-restore-untiled: shard-glk: PASS -> FAIL (fdo#103375) igt@kms_cursor_legacy@all-pipes-torture-move: shard-snb: PASS -> DMESG-WARN (fdo#107122) fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#107122 https://bugs.freedesktop.org/show_bug.cgi?id=107122 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4645 -> Patchwork_9923 CI_DRM_4645: 37a3cb069a7116ab9b04d3020c54557633dd180b @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9923: 6a0549d5bcab0290e4ff6c2c22b63f9bbcedc147 @ 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_9923/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit
== Series Details ==
Series: drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit
URL : https://patchwork.freedesktop.org/series/48048/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4645 -> Patchwork_9923 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48048/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9923 that come from known issues:
=== IGT changes ===
Issues hit
{igt@amdgpu/amd_basic@userptr}:
{fi-kbl-8809g}: PASS -> INCOMPLETE (fdo#107402)
igt@drv_selftest@live_workarounds:
{fi-cfl-8109u}: PASS -> DMESG-FAIL (fdo#107292)
fi-kbl-x1275: PASS -> DMESG-FAIL (fdo#107292)
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
Possible fixes
igt@debugfs_test@read_all_entries:
fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS
igt@drv_selftest@live_hangcheck:
fi-kbl-7500u: DMESG-FAIL (fdo#106560, fdo#106947) -> PASS
fi-kbl-guc: DMESG-FAIL (fdo#106947) -> PASS
igt@drv_selftest@live_requests:
{fi-bsw-kefka}: INCOMPLETE (fdo#105876) -> PASS
igt@drv_selftest@live_workarounds:
{fi-bsw-kefka}: DMESG-FAIL (fdo#107292) -> PASS
igt@kms_flip@basic-flip-vs-dpms:
fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS
igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
fi-skl-guc: FAIL (fdo#103191) -> PASS
igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
{fi-byt-clapper}: FAIL (fdo#103191, fdo#107362) -> 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#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#105876 https://bugs.freedesktop.org/show_bug.cgi?id=105876
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402
== Participating hosts (53 -> 48) ==
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4645 -> Patchwork_9923
CI_DRM_4645: 37a3cb069a7116ab9b04d3020c54557633dd180b @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9923: 6a0549d5bcab0290e4ff6c2c22b63f9bbcedc147 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
6a0549d5bcab drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9923/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/psr: Add missing check for I915_PSR_DEBUG_IRQ bit
We print the last attempted entry and last exit timestamps only when
IRQ debug is requested. This check was missed when new debug flags were
added in 'commit c44301fce614 ("drm/i915: Allow control of PSR at
runtime through debugfs, v6")
Cc: Maarten Lankhorst
Signed-off-by: Dhinakaran Pandiyan
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 26b7e5276b15..374b550d9a4f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2735,7 +2735,7 @@ static int i915_edp_psr_status(struct seq_file *m, void
*data)
psr_source_status(dev_priv, m);
mutex_unlock(_priv->psr.lock);
- if (READ_ONCE(dev_priv->psr.debug)) {
+ if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) {
seq_printf(m, "Last attempted entry at: %lld\n",
dev_priv->psr.last_entry_attempt);
seq_printf(m, "Last exit at: %lld\n",
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/icl: account for context save/restore removed bits (rev2)
== Series Details == Series: drm/i915/icl: account for context save/restore removed bits (rev2) URL : https://patchwork.freedesktop.org/series/47976/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4643_full -> Patchwork_9922_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_9922_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9922_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9922_full: === IGT changes === Warnings igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a: shard-snb: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_9922_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@shrink: shard-apl: PASS -> FAIL (fdo#106886) Possible fixes igt@kms_mmap_write_crc: shard-hsw: DMESG-WARN (fdo#102614) -> PASS igt@perf@polling: shard-hsw: FAIL (fdo#102252) -> PASS fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4643 -> Patchwork_9922 CI_DRM_4643: 2fab0affe5a9f87309773bc4cbef828f4dde7acd @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9922: afe5704847eb9b9ba8ac1ef72d44a3ed9b1db04f @ 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_9922/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Expose retry count to per gen reset logic
== Series Details == Series: series starting with [1/2] drm/i915: Expose retry count to per gen reset logic URL : https://patchwork.freedesktop.org/series/48019/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4643_full -> Patchwork_9921_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9921_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@fence-restore-untiled: shard-glk: PASS -> FAIL (fdo#103375) Possible fixes igt@gem_ppgtt@blt-vs-render-ctxn: shard-kbl: INCOMPLETE (fdo#106023, fdo#103665) -> PASS igt@kms_mmap_write_crc: shard-hsw: DMESG-WARN (fdo#102614) -> PASS igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS igt@perf@polling: shard-hsw: FAIL (fdo#102252) -> PASS fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4643 -> Patchwork_9921 CI_DRM_4643: 2fab0affe5a9f87309773bc4cbef828f4dde7acd @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9921: 8d198b5b328dcebb6f57255fe4089dc739d51ff7 @ 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_9921/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/3] igt/gem_sync: Exercising waiting while keeping the GPU busy
On 10/08/18 10:51, Chris Wilson wrote:
Quoting Antonio Argenziano (2018-08-10 18:41:22)
On 10/08/18 04:01, Chris Wilson wrote:
Normally we wait on the last request, but that overlooks any
difficulties in waiting on a request while the next is being qeued.
/s/qeued/queued
Check those.
Signed-off-by: Chris Wilson
---
tests/gem_sync.c | 72
1 file changed, 72 insertions(+)
diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index c697220ad..fb209977d 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -294,6 +294,74 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
}
+ intel_detect_and_clear_missed_interrupts(fd);
+ igt_fork(child, num_engines) {
+ double start, end, elapsed;
+ unsigned long cycles;
+ igt_spin_t *spin[2];
+ uint32_t cmd;
+
+ spin[0] = __igt_spin_batch_new(fd,
+.engine = ring,
+.flags = IGT_SPIN_FAST);
+ cmd = *spin[0]->batch;
+
+ spin[1] = __igt_spin_batch_new(fd,
+.engine = ring,
+.flags = IGT_SPIN_FAST);
+ igt_assert(*spin[1]->batch == cmd);
+
+ start = gettime();
+ end = start + timeout;
+ cycles = 0;
+ do {
+ for (int loop = 0; loop < 1024; loop++) {
+ igt_spin_t *s = spin[loop & 1];
+
+ igt_spin_batch_end(s);
+ gem_sync(fd, s->handle);
How does the test fail if the sync goes wrong? Hang detector on the
queued batch?
We have a hang detector for both missed wakeups and GPU hangs. As tests
goes it's fairly tame, but in essence this entire file is about trying
to trick the HW+driver into not sending an interrupt back to userspace.
Just a very narrow stress test, over and over again from slightly
different angles.
I see.
Reviewed-by: Antonio Argenziano
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 1/2] drm: Introduce new DRM_FORMAT_XYUV
Hi Stanislav,
FYI, we are trying to add same format under a slightly different name.
See https://lists.freedesktop.org/archives/dri-devel/2018-July/184598.html
On Fri, Aug 10, 2018 at 04:19:03PM +0300, StanLis wrote:
> From: Stanislav Lisovskiy
>
> v5: This is YUV444 packed format same as AYUV, but without alpha,
> as supported by i915.
>
> Signed-off-by: Stanislav Lisovskiy
> ---
> drivers/gpu/drm/drm_fourcc.c | 1 +
> include/uapi/drm/drm_fourcc.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 5ca6395cd4d3..5bde1b20a098 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -173,6 +173,7 @@ const struct drm_format_info *__drm_format_info(u32
> format)
> { .format = DRM_FORMAT_UYVY,.depth = 0,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_VYUY,.depth = 0,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_AYUV,.depth = 0,
> .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true
> },
> + { .format = DRM_FORMAT_XYUV,.depth = 0,
> .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = false
> },
I would rather drop the .has_alpha = false, since these lines are long
enough already.
> };
>
> unsigned int i;
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index d5e52350a3aa..c4eb69468eda 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -112,6 +112,7 @@ extern "C" {
> #define DRM_FORMAT_VYUY fourcc_code('V', 'Y', 'U', 'Y') /*
> [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */
>
> #define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') /*
> [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
> +#define DRM_FORMAT_XYUV fourcc_code('X', 'Y', 'U', 'V') /*
> [31:0] X:Y:Cb:Cr 8:8:8:8 little endian */
>
> /*
> * 2 plane RGB + A
> --
> 2.17.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Cheers,
Alex G
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/3] igt/gem_sync: Exercising waiting while keeping the GPU busy
Quoting Antonio Argenziano (2018-08-10 18:41:22)
>
>
> On 10/08/18 04:01, Chris Wilson wrote:
> > Normally we wait on the last request, but that overlooks any
> > difficulties in waiting on a request while the next is being qeued.
>
> /s/qeued/queued
>
> > Check those.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > tests/gem_sync.c | 72
> > 1 file changed, 72 insertions(+)
> >
> > diff --git a/tests/gem_sync.c b/tests/gem_sync.c
> > index c697220ad..fb209977d 100644
> > --- a/tests/gem_sync.c
> > +++ b/tests/gem_sync.c
> > @@ -294,6 +294,74 @@ wakeup_ring(int fd, unsigned ring, int timeout, int
> > wlen)
> > igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
> > }
> >
>
> > + intel_detect_and_clear_missed_interrupts(fd);
> > + igt_fork(child, num_engines) {
> > + double start, end, elapsed;
> > + unsigned long cycles;
> > + igt_spin_t *spin[2];
> > + uint32_t cmd;
> > +
> > + spin[0] = __igt_spin_batch_new(fd,
> > +.engine = ring,
> > +.flags = IGT_SPIN_FAST);
> > + cmd = *spin[0]->batch;
> > +
> > + spin[1] = __igt_spin_batch_new(fd,
> > +.engine = ring,
> > +.flags = IGT_SPIN_FAST);
> > + igt_assert(*spin[1]->batch == cmd);
> > +
> > + start = gettime();
> > + end = start + timeout;
> > + cycles = 0;
> > + do {
> > + for (int loop = 0; loop < 1024; loop++) {
> > + igt_spin_t *s = spin[loop & 1];
> > +
> > + igt_spin_batch_end(s);
> > + gem_sync(fd, s->handle);
>
> How does the test fail if the sync goes wrong? Hang detector on the
> queued batch?
We have a hang detector for both missed wakeups and GPU hangs. As tests
goes it's fairly tame, but in essence this entire file is about trying
to trick the HW+driver into not sending an interrupt back to userspace.
Just a very narrow stress test, over and over again from slightly
different angles.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/3] igt/gem_sync: Exercising waiting while keeping the GPU busy
On 10/08/18 04:01, Chris Wilson wrote:
Normally we wait on the last request, but that overlooks any
difficulties in waiting on a request while the next is being qeued.
/s/qeued/queued
Check those.
Signed-off-by: Chris Wilson
---
tests/gem_sync.c | 72
1 file changed, 72 insertions(+)
diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index c697220ad..fb209977d 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -294,6 +294,74 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
}
+ intel_detect_and_clear_missed_interrupts(fd);
+ igt_fork(child, num_engines) {
+ double start, end, elapsed;
+ unsigned long cycles;
+ igt_spin_t *spin[2];
+ uint32_t cmd;
+
+ spin[0] = __igt_spin_batch_new(fd,
+ .engine = ring,
+ .flags = IGT_SPIN_FAST);
+ cmd = *spin[0]->batch;
+
+ spin[1] = __igt_spin_batch_new(fd,
+ .engine = ring,
+ .flags = IGT_SPIN_FAST);
+ igt_assert(*spin[1]->batch == cmd);
+
+ start = gettime();
+ end = start + timeout;
+ cycles = 0;
+ do {
+ for (int loop = 0; loop < 1024; loop++) {
+ igt_spin_t *s = spin[loop & 1];
+
+ igt_spin_batch_end(s);
+ gem_sync(fd, s->handle);
How does the test fail if the sync goes wrong? Hang detector on the
queued batch?
Antonio
+
+ *s->batch = cmd;
+ gem_execbuf(fd, >execbuf);
+ }
+ cycles += 1024;
+ } while ((elapsed = gettime()) < end);
+ igt_spin_batch_free(fd, spin[1]);
+ igt_spin_batch_free(fd, spin[0]);
+
+ igt_info("%s%sompleted %ld cycles: %.3f us\n",
+names[child % num_engines] ?: "",
+names[child % num_engines] ? " c" : "C",
+cycles, (elapsed - start)*1e6/cycles);
+ }
+ igt_waitchildren_timeout(2*timeout, NULL);
+ igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
+}
+
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/icl: account for context save/restore removed bits (rev2)
== Series Details ==
Series: drm/i915/icl: account for context save/restore removed bits (rev2)
URL : https://patchwork.freedesktop.org/series/47976/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4643 -> Patchwork_9922 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/47976/revisions/2/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9922:
=== IGT changes ===
Possible regressions
igt@gem_exec_suspend@basic-s3:
{fi-kbl-soraka}:NOTRUN -> INCOMPLETE
== Known issues ==
Here are the changes found in Patchwork_9922 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_module_reload@basic-reload-inject:
fi-hsw-4770r: PASS -> DMESG-WARN (fdo#107425)
igt@drv_selftest@live_hangcheck:
fi-skl-guc: PASS -> DMESG-FAIL (fdo#107174)
igt@drv_selftest@live_workarounds:
fi-cnl-psr: PASS -> DMESG-FAIL (fdo#107292)
igt@kms_frontbuffer_tracking@basic:
fi-hsw-peppy: PASS -> DMESG-FAIL (fdo#106103, fdo#102614)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
{fi-byt-clapper}: PASS -> FAIL (fdo#107362, fdo#103191)
igt@prime_vgem@basic-fence-flip:
fi-ilk-650: PASS -> FAIL (fdo#104008)
Possible fixes
igt@drv_module_reload@basic-reload:
fi-glk-j4005: DMESG-WARN (fdo#106725, fdo#106248) -> PASS
igt@drv_selftest@live_hangcheck:
{fi-icl-u}: INCOMPLETE (fdo#107399) -> PASS
igt@drv_selftest@live_workarounds:
{fi-bsw-kefka}: DMESG-FAIL (fdo#107292) -> PASS
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-glk-j4005: FAIL (fdo#100368) -> PASS
Warnings
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248
fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
fdo#107399 https://bugs.freedesktop.org/show_bug.cgi?id=107399
fdo#107425 https://bugs.freedesktop.org/show_bug.cgi?id=107425
== Participating hosts (53 -> 49) ==
Additional (1): fi-kbl-soraka
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4643 -> Patchwork_9922
CI_DRM_4643: 2fab0affe5a9f87309773bc4cbef828f4dde7acd @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9922: afe5704847eb9b9ba8ac1ef72d44a3ed9b1db04f @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
afe5704847eb drm/i915/icl: account for context save/restore removed bits
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9922/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/icl: account for context save/restore removed bits (rev2)
== Series Details ==
Series: drm/i915/icl: account for context save/restore removed bits (rev2)
URL : https://patchwork.freedesktop.org/series/47976/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
afe5704847eb drm/i915/icl: account for context save/restore removed bits
-:20: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+
chars of sha1> ("")' - ie: 'commit 05f0addd9b10 ("drm/i915/icl:
Enhanced execution list support")'
#20:
References: 05f0addd9b10 ("drm/i915/icl: Enhanced execution list support")
total: 1 errors, 0 warnings, 0 checks, 27 lines checked
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI] drm/i915/icl: account for context save/restore removed bits
From: Paulo Zanoni
The RS_CTX_ENABLE and CTX_SAVE_INHIBIT bits are not present on ICL
anymore, but we still try to set them and then check them with
GEM_BUG_ON, resulting in a BUG() call. The bug can be reproduced by
igt/drv_selftest/live_hangcheck/others-priority and our CI was able
to catch it.
It is worth noticing that commit 05f0addd9b10 ("drm/i915/icl: Enhanced
execution list support") already tried to avoid the save bits
on ICL, but only inside populate_lr_context().
Cc: Chris Wilson
Cc: Mika Kuoppala
Testcase: igt/drv_selftest/live_hangcheck/others-priority
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107399
References: 05f0addd9b10 ("drm/i915/icl: Enhanced execution list support")
Signed-off-by: Paulo Zanoni
Link:
https://patchwork.freedesktop.org/patch/msgid/20180809235852.24516-1-paulo.r.zan...@intel.com
Reviewed-by: Chris Wilson
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_lrc.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index e5385dbfcdda..3f90c74038ef 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -541,11 +541,6 @@ static void inject_preempt_context(struct intel_engine_cs
*engine)
GEM_BUG_ON(execlists->preempt_complete_status !=
upper_32_bits(ce->lrc_desc));
- GEM_BUG_ON((ce->lrc_reg_state[CTX_CONTEXT_CONTROL + 1] &
- _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
- CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT)) !=
- _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
- CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT));
/*
* Switch to our empty preempt context so
@@ -2582,10 +2577,13 @@ static void execlists_init_reg_state(u32 *regs,
MI_LRI_FORCE_POSTED;
CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(engine),
- _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
- CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT |
- CTX_CTRL_RS_CTX_ENABLE) |
+ _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT) |
_MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH));
+ if (INTEL_GEN(dev_priv) < 11) {
+ regs[CTX_CONTEXT_CONTROL + 1] |=
+ _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT |
+ CTX_CTRL_RS_CTX_ENABLE);
+ }
CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0);
CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0);
CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0);
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/icl: account for context save/restore removed bits
== Series Details == Series: drm/i915/icl: account for context save/restore removed bits URL : https://patchwork.freedesktop.org/series/47976/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4643_full -> Patchwork_9919_full = == Summary - SUCCESS == No regressions found. == Known issues == Here are the changes found in Patchwork_9919_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@fence-restore-untiled: shard-glk: PASS -> FAIL (fdo#103375) igt@gem_ctx_isolation@rcs0-s3: shard-snb: PASS -> INCOMPLETE (fdo#105411) igt@kms_universal_plane@cursor-fb-leak-pipe-a: shard-apl: PASS -> FAIL (fdo#107241) Possible fixes igt@kms_cursor_legacy@all-pipes-torture-move: shard-glk: DMESG-WARN (fdo#107122) -> PASS shard-hsw: DMESG-WARN (fdo#107122) -> PASS igt@kms_mmap_write_crc: shard-hsw: DMESG-WARN (fdo#102614) -> PASS igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS igt@kms_vblank@pipe-a-ts-continuation-suspend: shard-kbl: INCOMPLETE (fdo#103665) -> PASS igt@perf@polling: shard-hsw: FAIL (fdo#102252) -> PASS fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#107122 https://bugs.freedesktop.org/show_bug.cgi?id=107122 fdo#107241 https://bugs.freedesktop.org/show_bug.cgi?id=107241 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 5) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4643 -> Patchwork_9919 CI_DRM_4643: 2fab0affe5a9f87309773bc4cbef828f4dde7acd @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9919: 51a3d4c0faefb9faf09007614069ea315d0ffb57 @ 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_9919/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Expose retry count to per gen reset logic
== Series Details ==
Series: series starting with [1/2] drm/i915: Expose retry count to per gen
reset logic
URL : https://patchwork.freedesktop.org/series/48019/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4643 -> Patchwork_9921 =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9921 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9921, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/48019/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9921:
=== IGT changes ===
Possible regressions
igt@gem_exec_suspend@basic-s3:
{fi-kbl-soraka}:NOTRUN -> INCOMPLETE
Warnings
igt@pm_rpm@basic-pci-d3-state:
fi-glk-j4005: PASS -> SKIP
== Known issues ==
Here are the changes found in Patchwork_9921 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_selftest@live_requests:
fi-skl-guc: PASS -> DMESG-FAIL (fdo#106685)
{fi-bsw-kefka}: PASS -> INCOMPLETE (fdo#105876)
igt@drv_selftest@live_workarounds:
{fi-cfl-8109u}: PASS -> DMESG-FAIL (fdo#107292)
fi-cnl-psr: PASS -> DMESG-FAIL (fdo#107292)
Possible fixes
igt@drv_module_reload@basic-reload:
fi-glk-j4005: DMESG-WARN (fdo#106725, fdo#106248) -> PASS
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-glk-j4005: FAIL (fdo#100368) -> PASS
Warnings
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#105876 https://bugs.freedesktop.org/show_bug.cgi?id=105876
fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248
fdo#106685 https://bugs.freedesktop.org/show_bug.cgi?id=106685
fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725
fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
== Participating hosts (53 -> 49) ==
Additional (1): fi-kbl-soraka
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4643 -> Patchwork_9921
CI_DRM_4643: 2fab0affe5a9f87309773bc4cbef828f4dde7acd @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9921: 8d198b5b328dcebb6f57255fe4089dc739d51ff7 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
8d198b5b328d drm/i915: Force reset on unready engine
1848f1c440b6 drm/i915: Expose retry count to per gen reset logic
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9921/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] RFC: Add write flag to reservation object fences
Am 10.08.2018 um 11:21 schrieb Daniel Vetter: [SNIP] Then don't track _any_ of the amdgpu internal fences in the reservation object: - 1 reservation object that you hand to ttm, for use internally within amdgpu - 1 reservation object that you attach to the dma-buf (or get from the imported dma-buf), where you play all the tricks to fake fences. Well that is an interesting idea. Going to try that one out. Regards, Christian. -Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Expose retry count to per gen reset logic
Quoting Mika Kuoppala (2018-08-10 15:00:35) > There is a possibility for per gen reset logic to > be more nasty if the softer approach on resetting does > not bear fruit. > > Expose retry count to per gen reset logic if it > wants to take such tough measures. > > Cc: Chris Wilson > Signed-off-by: Mika Kuoppala Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
Quoting Mika Kuoppala (2018-08-10 15:00:36)
> If engine reports that it is not ready for reset, we
> give up. Evidence shows that forcing a per engine reset
> on an engine which is not reporting to be ready for reset,
> can bring it back into a working order. There is risk that
> we corrupt the context image currently executing on that
> engine. But that is a risk worth taking as if we unblock
> the engine, we prevent a whole device wedging in a case
> of full gpu reset.
>
> Reset individual engine even if it reports that it is not
> prepared for reset, but only if we aim for full gpu reset
> and not on first reset attempt.
>
> v2: force reset only on later attempts, readability (Chris)
>
> Cc: Chris Wilson
> Signed-off-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 49 +++--
> 1 file changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 027d14574bfa..d24026839b17 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -2099,9 +2099,6 @@ static int gen8_reset_engine_start(struct
> intel_engine_cs *engine)
>RESET_CTL_READY_TO_RESET,
>700, 0,
>NULL);
> - if (ret)
> - DRM_ERROR("%s: reset request timeout\n", engine->name);
> -
> return ret;
> }
>
> @@ -2113,6 +2110,42 @@ static void gen8_reset_engine_cancel(struct
> intel_engine_cs *engine)
> _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
> }
>
> +static int reset_engines(struct drm_i915_private *i915,
> +unsigned int engine_mask,
> +unsigned int retry)
> +{
> + if (INTEL_GEN(i915) >= 11)
> + return gen11_reset_engines(i915, engine_mask);
> + else
> + return gen6_reset_engines(i915, engine_mask, retry);
> +}
> +
> +static int gen8_prepare_engine_for_reset(struct intel_engine_cs *engine,
> +unsigned int retry)
> +{
> + const bool force_reset_on_non_ready = retry >= 1;
> + int ret;
> +
> + ret = gen8_reset_engine_start(engine);
> +
> + if (ret && force_reset_on_non_ready) {
> + /*
> +* Try to unblock a single non-ready engine by risking
> +* context corruption.
> +*/
> + ret = reset_engines(engine->i915,
> + intel_engine_flag(engine),
> + retry);
> + if (!ret)
> + ret = gen8_reset_engine_start(engine);
> +
> + DRM_ERROR("%s: reset request timeout, forcing reset (%d)\n",
> + engine->name, ret);
This looks dubious now ;)
After the force you then do a reset in the caller. Twice the reset for
twice the unpreparedness.
> + }
> +
> + return ret;
> +}
> +
> static int gen8_reset_engines(struct drm_i915_private *dev_priv,
> unsigned int engine_mask,
> unsigned int retry)
> @@ -2122,16 +2155,12 @@ static int gen8_reset_engines(struct drm_i915_private
> *dev_priv,
> int ret;
>
> for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
> - if (gen8_reset_engine_start(engine)) {
> - ret = -EIO;
> + ret = gen8_prepare_engine_for_reset(engine, retry);
> + if (ret)
if (ret && !force_reset_unread)
> goto not_ready;
> - }
> }
>
> - if (INTEL_GEN(dev_priv) >= 11)
> - ret = gen11_reset_engines(dev_priv, engine_mask);
> - else
> - ret = gen6_reset_engines(dev_priv, engine_mask, retry);
> + ret = reset_engines(dev_priv, engine_mask, retry);
>
> not_ready:
> for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
> --
> 2.17.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 1/8] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
On Fri, Aug 10, 2018 at 01:33:19PM +0100, Chris Wilson wrote: > Quoting Imre Deak (2018-08-10 13:22:43) > > On Fri, Aug 10, 2018 at 08:11:31AM +0100, Chris Wilson wrote: > > > @@ -1474,6 +1476,8 @@ void i915_driver_unload(struct drm_device *dev) > > > i915_driver_cleanup_mmio(dev_priv); > > > > > > intel_display_power_put(dev_priv, POWER_DOMAIN_INIT); > > > + > > > + WARN_ON(atomic_read(_priv->runtime_pm.wakeref_count)); > > > > This probably WARNs atm at least due to having a low-level > > pm_runtime_put_autosuspend() in intel_runtime_pm_enable(); but a > > corresponding intel_runtime_pm_get() in intel_power_domains_fini_hw() (via > > intel_display_set_init_power) which is i915 specific. The following > > would fix that: > > It gets caught out by the very last display_set_init_power indeed. I > mean to have a word with you about that function ;) > > The issue we have with intel_display_set_init_power() at the moment is > that we don't always clean up it correctly as it not obvious who is > responsible for it at any time. Would it be possible to make that into > local POWER_DOMAIN_INIT grabs so the onion is always clear? (It wasn't > obvious to me why ownership was being transferred fairly arbitrary.) Yes, would be much clearer. One thing that depends on it is driver loading / resume expecting any power wells enabled by BIOS to stay enabled until the end of display HW readout. I can take a look at the exact dependencies there and remove the use of intel_display_set_init_power(). > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index e209edbc561d..1623c0d2cf57 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -3806,6 +3806,10 @@ void intel_power_domains_fini_hw(struct > > drm_i915_private *dev_priv) > > */ > > intel_display_set_init_power(dev_priv, true); > > In this case this would be much clearer as a raw > intel_power_domain_get(POWER_DOMAIN_INIT) > I think. Yes, but would have to change it in sync with the intel_display_set_init_power() in intel_power_domains_init_hw(). An error somewhere after that and before the end of HW readout would make the above intel_display_set_init_power() a nop. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Expose retry count to per gen reset logic
There is a possibility for per gen reset logic to
be more nasty if the softer approach on resetting does
not bear fruit.
Expose retry count to per gen reset logic if it
wants to take such tough measures.
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_uncore.c | 40 +++--
1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index c2fcb51fc58a..027d14574bfa 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1739,7 +1739,7 @@ static void gen3_stop_engine(struct intel_engine_cs
*engine)
}
static void i915_stop_engines(struct drm_i915_private *dev_priv,
- unsigned engine_mask)
+ unsigned int engine_mask)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;
@@ -1759,7 +1759,9 @@ static bool i915_in_reset(struct pci_dev *pdev)
return gdrst & GRDOM_RESET_STATUS;
}
-static int i915_do_reset(struct drm_i915_private *dev_priv, unsigned
engine_mask)
+static int i915_do_reset(struct drm_i915_private *dev_priv,
+unsigned int engine_mask,
+unsigned int retry)
{
struct pci_dev *pdev = dev_priv->drm.pdev;
int err;
@@ -1786,7 +1788,9 @@ static bool g4x_reset_complete(struct pci_dev *pdev)
return (gdrst & GRDOM_RESET_ENABLE) == 0;
}
-static int g33_do_reset(struct drm_i915_private *dev_priv, unsigned
engine_mask)
+static int g33_do_reset(struct drm_i915_private *dev_priv,
+ unsigned int engine_mask,
+ unsigned int retry)
{
struct pci_dev *pdev = dev_priv->drm.pdev;
@@ -1794,7 +1798,9 @@ static int g33_do_reset(struct drm_i915_private
*dev_priv, unsigned engine_mask)
return wait_for(g4x_reset_complete(pdev), 500);
}
-static int g4x_do_reset(struct drm_i915_private *dev_priv, unsigned
engine_mask)
+static int g4x_do_reset(struct drm_i915_private *dev_priv,
+ unsigned int engine_mask,
+ unsigned int retry)
{
struct pci_dev *pdev = dev_priv->drm.pdev;
int ret;
@@ -1831,7 +1837,8 @@ static int g4x_do_reset(struct drm_i915_private
*dev_priv, unsigned engine_mask)
}
static int ironlake_do_reset(struct drm_i915_private *dev_priv,
-unsigned engine_mask)
+unsigned int engine_mask,
+unsigned int retry)
{
int ret;
@@ -1887,6 +1894,7 @@ static int gen6_hw_domain_reset(struct drm_i915_private
*dev_priv,
* gen6_reset_engines - reset individual engines
* @dev_priv: i915 device
* @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full
reset
+ * @retry: the count of of previous attempts to reset.
*
* This function will reset the individual engines that are set in engine_mask.
* If you provide ALL_ENGINES as mask, full global domain reset will be issued.
@@ -1897,7 +1905,8 @@ static int gen6_hw_domain_reset(struct drm_i915_private
*dev_priv,
* Returns 0 on success, nonzero on error.
*/
static int gen6_reset_engines(struct drm_i915_private *dev_priv,
- unsigned engine_mask)
+ unsigned int engine_mask,
+ unsigned int retry)
{
struct intel_engine_cs *engine;
const u32 hw_engine_mask[I915_NUM_ENGINES] = {
@@ -1936,7 +1945,7 @@ static int gen6_reset_engines(struct drm_i915_private
*dev_priv,
* Returns 0 on success, nonzero on error.
*/
static int gen11_reset_engines(struct drm_i915_private *dev_priv,
- unsigned engine_mask)
+ unsigned int engine_mask)
{
struct intel_engine_cs *engine;
const u32 hw_engine_mask[I915_NUM_ENGINES] = {
@@ -2105,7 +2114,8 @@ static void gen8_reset_engine_cancel(struct
intel_engine_cs *engine)
}
static int gen8_reset_engines(struct drm_i915_private *dev_priv,
- unsigned engine_mask)
+ unsigned int engine_mask,
+ unsigned int retry)
{
struct intel_engine_cs *engine;
unsigned int tmp;
@@ -2121,7 +2131,7 @@ static int gen8_reset_engines(struct drm_i915_private
*dev_priv,
if (INTEL_GEN(dev_priv) >= 11)
ret = gen11_reset_engines(dev_priv, engine_mask);
else
- ret = gen6_reset_engines(dev_priv, engine_mask);
+ ret = gen6_reset_engines(dev_priv, engine_mask, retry);
not_ready:
for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
@@ -2130,7 +2140,8 @@ static int gen8_reset_engines(struct drm_i915_private
*dev_priv,
return ret;
}
-typedef int (*reset_func)(struct drm_i915_private *, unsigned engine_mask);
+typedef int (*reset_func)(struct
[Intel-gfx] [PATCH 2/2] drm/i915: Force reset on unready engine
If engine reports that it is not ready for reset, we
give up. Evidence shows that forcing a per engine reset
on an engine which is not reporting to be ready for reset,
can bring it back into a working order. There is risk that
we corrupt the context image currently executing on that
engine. But that is a risk worth taking as if we unblock
the engine, we prevent a whole device wedging in a case
of full gpu reset.
Reset individual engine even if it reports that it is not
prepared for reset, but only if we aim for full gpu reset
and not on first reset attempt.
v2: force reset only on later attempts, readability (Chris)
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_uncore.c | 49 +++--
1 file changed, 39 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 027d14574bfa..d24026839b17 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -2099,9 +2099,6 @@ static int gen8_reset_engine_start(struct intel_engine_cs
*engine)
RESET_CTL_READY_TO_RESET,
700, 0,
NULL);
- if (ret)
- DRM_ERROR("%s: reset request timeout\n", engine->name);
-
return ret;
}
@@ -2113,6 +2110,42 @@ static void gen8_reset_engine_cancel(struct
intel_engine_cs *engine)
_MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
}
+static int reset_engines(struct drm_i915_private *i915,
+unsigned int engine_mask,
+unsigned int retry)
+{
+ if (INTEL_GEN(i915) >= 11)
+ return gen11_reset_engines(i915, engine_mask);
+ else
+ return gen6_reset_engines(i915, engine_mask, retry);
+}
+
+static int gen8_prepare_engine_for_reset(struct intel_engine_cs *engine,
+unsigned int retry)
+{
+ const bool force_reset_on_non_ready = retry >= 1;
+ int ret;
+
+ ret = gen8_reset_engine_start(engine);
+
+ if (ret && force_reset_on_non_ready) {
+ /*
+* Try to unblock a single non-ready engine by risking
+* context corruption.
+*/
+ ret = reset_engines(engine->i915,
+ intel_engine_flag(engine),
+ retry);
+ if (!ret)
+ ret = gen8_reset_engine_start(engine);
+
+ DRM_ERROR("%s: reset request timeout, forcing reset (%d)\n",
+ engine->name, ret);
+ }
+
+ return ret;
+}
+
static int gen8_reset_engines(struct drm_i915_private *dev_priv,
unsigned int engine_mask,
unsigned int retry)
@@ -2122,16 +2155,12 @@ static int gen8_reset_engines(struct drm_i915_private
*dev_priv,
int ret;
for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
- if (gen8_reset_engine_start(engine)) {
- ret = -EIO;
+ ret = gen8_prepare_engine_for_reset(engine, retry);
+ if (ret)
goto not_ready;
- }
}
- if (INTEL_GEN(dev_priv) >= 11)
- ret = gen11_reset_engines(dev_priv, engine_mask);
- else
- ret = gen6_reset_engines(dev_priv, engine_mask, retry);
+ ret = reset_engines(dev_priv, engine_mask, retry);
not_ready:
for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] igt/perf_pmu: Aim for a fixed number of iterations for calibrating accuracy
Quoting Tvrtko Ursulin (2018-08-09 12:54:41)
>
> On 08/08/2018 15:59, Chris Wilson wrote:
> > Our observation is that the systematic error is proportional to the
> > number of iterations we perform; the suspicion is that it directly
> > correlates with the number of sleeps. Reduce the number of iterations,
> > to try and keep the error in check.
> >
> > Signed-off-by: Chris Wilson
> > Cc: Tvrtko Ursulin
> > ---
> > tests/perf_pmu.c | 34 +-
> > 1 file changed, 21 insertions(+), 13 deletions(-)
> >
> > diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> > index 9a20abb6b..5a26d5272 100644
> > --- a/tests/perf_pmu.c
> > +++ b/tests/perf_pmu.c
> > @@ -1521,14 +1521,13 @@ static void __rearm_spin_batch(igt_spin_t *spin)
> >
> > static void
> > accuracy(int gem_fd, const struct intel_execution_engine2 *e,
> > - unsigned long target_busy_pct)
> > + unsigned long target_busy_pct,
> > + unsigned long target_iters)
> > {
> > - unsigned long busy_us = 1 - 100 * (1 + abs(50 - target_busy_pct));
> > - unsigned long idle_us = 100 * (busy_us - target_busy_pct *
> > - busy_us / 100) / target_busy_pct;
> > const unsigned long min_test_us = 1e6;
> > - const unsigned long pwm_calibration_us = min_test_us;
> > - const unsigned long test_us = min_test_us;
> > + unsigned long pwm_calibration_us;
> > + unsigned long test_us;
> > + unsigned long cycle_us, busy_us, idle_us;
> > double busy_r, expected;
> > uint64_t val[2];
> > uint64_t ts[2];
> > @@ -1538,18 +1537,27 @@ accuracy(int gem_fd, const struct
> > intel_execution_engine2 *e,
> > /* Sampling platforms cannot reach the high accuracy criteria. */
> > igt_require(gem_has_execlists(gem_fd));
> >
> > - while (idle_us < 2500) {
> > + /* Aim for approximately 100 iterations for calibration */
> > + cycle_us = min_test_us / target_iters;
> > + busy_us = cycle_us * target_busy_pct / 100;
> > + idle_us = cycle_us - busy_us;
>
> 2% load, 1s / 10 iters
> cycles_us = 100ms
> busy_us = 2ms
> idle_us = 98ms
> ...
>
> > +
> > + while (idle_us < 2500 || busy_us < 2500) {
> > busy_us *= 2;
> > idle_us *= 2;
>
> ...
>
> busy_us = 4ms
> idle_us = 196ms
Currently it is 250ms per 98:2 cycle and about 20ms per 50:50 cycle. So
we are only doing 4 and 50 iterations respectively.
10 cycles is strictly an improvement :-p
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 2/2] drm/i915: Adding YUV444 packed format(DRM_FORMAT_XYUV) support.
From: Stanislav Lisovskiy
PLANE_CTL_FORMAT_AYUV is already supported, according to hardware
specification.
v2: Edited commit message, removed redundant whitespaces.
v3: Fixed fallthrough logic for the format switch cases.
v4: Yet again fixed fallthrough logic, to reuse code from other case
labels.
v5: Started to use XYUV instead of AYUV, as we don't use alpha.
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/intel_display.c | 7 +++
drivers/gpu/drm/i915/intel_sprite.c | 1 +
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 56818a45181c..8a4f763c7ca3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_YVYU,
DRM_FORMAT_UYVY,
DRM_FORMAT_VYUY,
+ DRM_FORMAT_XYUV,
};
static const uint32_t skl_pri_planar_formats[] = {
@@ -102,6 +103,7 @@ static const uint32_t skl_pri_planar_formats[] = {
DRM_FORMAT_UYVY,
DRM_FORMAT_VYUY,
DRM_FORMAT_NV12,
+ DRM_FORMAT_XYUV,
};
static const uint64_t skl_format_modifiers_noccs[] = {
@@ -3497,6 +3499,8 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
return PLANE_CTL_FORMAT_XRGB_2101010;
case DRM_FORMAT_XBGR2101010:
return PLANE_CTL_ORDER_RGBX | PLANE_CTL_FORMAT_XRGB_2101010;
+ case DRM_FORMAT_XYUV:
+ return PLANE_CTL_FORMAT_AYUV;
case DRM_FORMAT_YUYV:
return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YUYV;
case DRM_FORMAT_YVYU:
@@ -13371,6 +13375,7 @@ static bool skl_plane_format_mod_supported(struct
drm_plane *_plane,
}
switch (format) {
+
case DRM_FORMAT_XRGB:
case DRM_FORMAT_XBGR:
case DRM_FORMAT_ARGB:
@@ -13387,6 +13392,7 @@ static bool skl_plane_format_mod_supported(struct
drm_plane *_plane,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_NV12:
+ case DRM_FORMAT_XYUV:
if (modifier == I915_FORMAT_MOD_Yf_TILED)
return true;
/* fall through */
@@ -14510,6 +14516,7 @@ static int intel_framebuffer_init(struct
intel_framebuffer *intel_fb,
goto err;
}
break;
+ case DRM_FORMAT_XYUV:
case DRM_FORMAT_YUYV:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_YVYU:
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 344c0e709b19..812e4e8afe2b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1421,6 +1421,7 @@ static bool skl_plane_format_mod_supported(struct
drm_plane *_plane,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_NV12:
+ case DRM_FORMAT_XYUV:
if (modifier == I915_FORMAT_MOD_Yf_TILED)
return true;
/* fall through */
--
2.17.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 1/2] drm: Introduce new DRM_FORMAT_XYUV
From: Stanislav Lisovskiy
v5: This is YUV444 packed format same as AYUV, but without alpha,
as supported by i915.
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/drm_fourcc.c | 1 +
include/uapi/drm/drm_fourcc.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 5ca6395cd4d3..5bde1b20a098 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -173,6 +173,7 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_UYVY,.depth = 0,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
{ .format = DRM_FORMAT_VYUY,.depth = 0,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
{ .format = DRM_FORMAT_AYUV,.depth = 0,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_XYUV,.depth = 0,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = false },
};
unsigned int i;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index d5e52350a3aa..c4eb69468eda 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -112,6 +112,7 @@ extern "C" {
#define DRM_FORMAT_VYUYfourcc_code('V', 'Y', 'U', 'Y') /*
[31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */
#define DRM_FORMAT_AYUVfourcc_code('A', 'Y', 'U', 'V') /*
[31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
+#define DRM_FORMAT_XYUVfourcc_code('X', 'Y', 'U', 'V') /*
[31:0] X:Y:Cb:Cr 8:8:8:8 little endian */
/*
* 2 plane RGB + A
--
2.17.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 0/2] Add XYUV format support
From: Stanislav Lisovskiy Introduced new XYUV scan-in format for framebuffer and added support for it to i915 driver. Stanislav Lisovskiy (2): drm: Introduce new DRM_FORMAT_XYUV drm/i915: Adding YUV444 packed format(DRM_FORMAT_XYUV) support. drivers/gpu/drm/drm_fourcc.c | 1 + drivers/gpu/drm/i915/intel_display.c | 7 +++ drivers/gpu/drm/i915/intel_sprite.c | 1 + include/uapi/drm/drm_fourcc.h| 1 + 4 files changed, 10 insertions(+) -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [v3] drm/i915: Add detection of changing of edid on between suspend and resume
On Thu, 2018-08-09 at 14:13 +0300, Gwan-gyeong Mun wrote:
> The hotplug detection routine of i915 uses
> drm_helper_hpd_irq_event(). This
> helper can detect changing of status of connector, but it can not
> detect
> changing of edid.
>
> Following scenario requires detection of changing of edid.
>
> 1) plug display device to a connector
> 2) system suspend
> 3) unplug 1)'s display device and plug the other display device to a
> connector
> 4) system resume
>
> It adds edid check routine when a connector status still remains as
> "connector_status_connected".
>
> v2: Add NULL check before comparing of EDIDs.
>
> Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
> Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
> Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
> Testcase: igt/kms_chamelium/dp-edid-change-during-suspend
>
> Signed-off-by: Gwan-gyeong Mun
> ---
> drivers/gpu/drm/i915/intel_hotplug.c | 84
> +++-
> 1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 648a13c6043c..965f2d771fc0 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -507,6 +507,88 @@ void intel_hpd_init(struct drm_i915_private
> *dev_priv)
> }
> }
>
> +/**
> + * intel_hpd_irq_event - hotplug processing
> + * @dev: drm_device
> + *
> + * Drivers can use this function to run a detect cycle on all
> connectors which
> + * have the DRM_CONNECTOR_POLL_HPD flag set in their member.
> All other
> + * connectors are ignored, which is useful to avoid reprobing fixed
> panels.
> + *
> + * This function is useful for drivers which can't or don't track
> hotplug interrupts
> + * for each connector. This function is based on
> drm_helper_hpd_irq_event() helper
> + * function and besides it adds edid check routine when a connector
> status still
> + * remains as "connector_status_connected".
> + *
> + * Following scenario requires detection of changing of edid.
> + * 1) plug display device to a connector
> + * 2) system suspend
> + * 3) unplug 1)'s display device and plug the other display device
> to a connector
> + * 4) system resume
> +
> + * This function must be called from process context with no mode
> + * setting locks held.
> + *
> + * Note that a connector can be both polled and probed from the
> hotplug handler,
> + * in case the hotplug interrupt is known to be unreliable.
> + */
> +static bool intel_hpd_irq_event(struct drm_device *dev)
> +{
> + struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> + enum drm_connector_status old_status, cur_status;
> + struct edid *old_edid;
> + bool changed = false;
> +
> + if (!dev->mode_config.poll_enabled)
> + return false;
> +
> + mutex_lock(>mode_config.mutex);
> + drm_connector_list_iter_begin(dev, _iter);
> + drm_for_each_connector_iter(connector, _iter) {
> + /* Only handle HPD capable connectors. */
> + if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
> + continue;
> +
> + old_status = connector->status;
> + old_edid = to_intel_connector(connector)-
> >detect_edid;
> +
> + cur_status = drm_helper_probe_detect(connector,
> NULL, false);
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from
> %s to %s\n",
> + connector->base.id, connector->name,
> + drm_get_connector_status_name(old_stat
> us),
> + drm_get_connector_status_name(cur_stat
> us));
> +
> + if (old_status != cur_status)
> + changed = true;
> +
> + /* Check changing of edid when a connector status
> still remains
> + * as "connector_status_connected".
> + */
> + if (old_status == cur_status &&
> + cur_status == connector_status_connected) {
> + struct edid *cur_edid =
> to_intel_connector(connector)->detect_edid;
> +
> + if (!old_edid || !cur_edid)
> + continue;
> +
> + if (memcmp(old_edid, cur_edid,
> sizeof(*cur_edid))) {
> + changed = true;
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]
> edid updated\n",
> + connector->base.id,
> + connector->name);
> + }
> + }
> + }
> + drm_connector_list_iter_end(_iter);
> + mutex_unlock(>mode_config.mutex);
> +
> + if (changed)
> + drm_kms_helper_hotplug_event(dev);
> +
> + return changed;
> +}
> +
> static void i915_hpd_poll_init_work(struct work_struct *work)
> {
> struct drm_i915_private *dev_priv =
> @@ -552,7 +634,7 @@ static
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [RFC,1/8] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable (rev2)
== Series Details == Series: series starting with [RFC,1/8] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable (rev2) URL : https://patchwork.freedesktop.org/series/47989/ State : failure == Summary == Applying: drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable Applying: drm/i915: Track all held rpm wakerefs Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/i915_drv.c M drivers/gpu/drm/i915/i915_drv.h M drivers/gpu/drm/i915/intel_drv.h M drivers/gpu/drm/i915/intel_runtime_pm.c Falling back to patching base and 3-way merge... Auto-merging drivers/gpu/drm/i915/intel_runtime_pm.c CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_runtime_pm.c Auto-merging drivers/gpu/drm/i915/intel_drv.h CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_drv.h Auto-merging drivers/gpu/drm/i915/i915_drv.h Auto-merging drivers/gpu/drm/i915/i915_drv.c CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_drv.c error: Failed to merge in the changes. Patch failed at 0002 drm/i915: Track all held rpm wakerefs Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 1/8] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
Quoting Imre Deak (2018-08-10 13:22:43) > On Fri, Aug 10, 2018 at 08:11:31AM +0100, Chris Wilson wrote: > > @@ -1474,6 +1476,8 @@ void i915_driver_unload(struct drm_device *dev) > > i915_driver_cleanup_mmio(dev_priv); > > > > intel_display_power_put(dev_priv, POWER_DOMAIN_INIT); > > + > > + WARN_ON(atomic_read(_priv->runtime_pm.wakeref_count)); > > This probably WARNs atm at least due to having a low-level > pm_runtime_put_autosuspend() in intel_runtime_pm_enable(); but a > corresponding intel_runtime_pm_get() in intel_power_domains_fini_hw() (via > intel_display_set_init_power) which is i915 specific. The following > would fix that: It gets caught out by the very last display_set_init_power indeed. I mean to have a word with you about that function ;) The issue we have with intel_display_set_init_power() at the moment is that we don't always clean up it correctly as it not obvious who is responsible for it at any time. Would it be possible to make that into local POWER_DOMAIN_INIT grabs so the onion is always clear? (It wasn't obvious to me why ownership was being transferred fairly arbitrary.) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index e209edbc561d..1623c0d2cf57 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -3806,6 +3806,10 @@ void intel_power_domains_fini_hw(struct > drm_i915_private *dev_priv) > */ > intel_display_set_init_power(dev_priv, true); In this case this would be much clearer as a raw intel_power_domain_get(POWER_DOMAIN_INIT) I think. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 1/8] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
On Fri, Aug 10, 2018 at 08:11:31AM +0100, Chris Wilson wrote:
> Currently, we cancel the extra wakeref we have for !runtime-pm devices
> inside power_wells_fini_hw. However, this is not strictly paired with
> the acquisition of that wakeref in runtime_pm_enable (as the fini_hw may
> be called on errors paths before we even call runtime_pm_enable). Make
> the symmetry more explicit and include a check that we do release all of
> our rpm wakerefs.
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_drv.c | 8 ++--
> drivers/gpu/drm/i915/intel_drv.h| 1 +
> drivers/gpu/drm/i915/intel_runtime_pm.c | 24 +++-
> 3 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9dce55182c3a..62ef105a241d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1281,6 +1281,8 @@ static void i915_driver_register(struct
> drm_i915_private *dev_priv)
>*/
> if (INTEL_INFO(dev_priv)->num_pipes)
> drm_kms_helper_poll_init(dev);
> +
> + intel_runtime_pm_enable(dev_priv);
> }
>
> /**
> @@ -1289,6 +1291,8 @@ static void i915_driver_register(struct
> drm_i915_private *dev_priv)
> */
> static void i915_driver_unregister(struct drm_i915_private *dev_priv)
> {
> + intel_runtime_pm_disable(dev_priv);
> +
> intel_fbdev_unregister(dev_priv);
> intel_audio_deinit(dev_priv);
>
> @@ -1408,8 +1412,6 @@ int i915_driver_load(struct pci_dev *pdev, const struct
> pci_device_id *ent)
>
> i915_driver_register(dev_priv);
>
> - intel_runtime_pm_enable(dev_priv);
> -
> intel_init_ipc(dev_priv);
>
> intel_runtime_pm_put(dev_priv);
> @@ -1474,6 +1476,8 @@ void i915_driver_unload(struct drm_device *dev)
> i915_driver_cleanup_mmio(dev_priv);
>
> intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> +
> + WARN_ON(atomic_read(_priv->runtime_pm.wakeref_count));
This probably WARNs atm at least due to having a low-level
pm_runtime_put_autosuspend() in intel_runtime_pm_enable(); but a
corresponding intel_runtime_pm_get() in intel_power_domains_fini_hw() (via
intel_display_set_init_power) which is i915 specific. The following
would fix that:
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index e209edbc561d..1623c0d2cf57 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3806,6 +3806,10 @@ void intel_power_domains_fini_hw(struct drm_i915_private
*dev_priv)
*/
intel_display_set_init_power(dev_priv, true);
+ /* Hand over RPM reference to PM core */
+ pm_runtime_get_noresume(kdev);
+ intel_runtime_pm_put(dev_priv);
+
/* Remove the refcount we took to keep power well support disabled. */
if (!i915_modparams.disable_power_well)
intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> }
>
> static void i915_driver_release(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 0601abb8c71f..dc6c0cec9b36 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1956,6 +1956,7 @@ void intel_power_domains_verify_state(struct
> drm_i915_private *dev_priv);
> void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
> void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
> +void intel_runtime_pm_disable(struct drm_i915_private *dev_priv);
> const char *
> intel_display_power_domain_str(enum intel_display_power_domain domain);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index e209edbc561d..b78c3b48aa62 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3793,8 +3793,6 @@ void intel_power_domains_init_hw(struct
> drm_i915_private *dev_priv, bool resume)
> */
> void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv)
> {
> - struct device *kdev = _priv->drm.pdev->dev;
> -
> /*
>* The i915.ko module is still not prepared to be loaded when
>* the power well is not enabled, so just enable it in case
> @@ -3809,13 +3807,6 @@ void intel_power_domains_fini_hw(struct
> drm_i915_private *dev_priv)
> /* Remove the refcount we took to keep power well support disabled. */
> if (!i915_modparams.disable_power_well)
> intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
> -
> - /*
> - * Remove the refcount we took in intel_runtime_pm_enable() in case
> - * the platform doesn't support runtime PM.
> - */
> - if (!HAS_RUNTIME_PM(dev_priv))
> - pm_runtime_put(kdev);
> }
>
> /**
> @@ -4074,3 +4065,18 @@ void
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/icl: account for context save/restore removed bits
== Series Details ==
Series: drm/i915/icl: account for context save/restore removed bits
URL : https://patchwork.freedesktop.org/series/47976/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4643 -> Patchwork_9919 =
== Summary - SUCCESS ==
No regressions found.
External URL:
https://patchwork.freedesktop.org/api/1.0/series/47976/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_9919 that come from known issues:
=== IGT changes ===
Issues hit
igt@drv_module_reload@basic-reload-inject:
fi-glk-j4005: PASS -> DMESG-WARN (fdo#106248, fdo#106725)
igt@drv_selftest@live_coherency:
fi-gdg-551: PASS -> DMESG-FAIL (fdo#107164)
igt@drv_selftest@live_hangcheck:
fi-skl-guc: PASS -> DMESG-FAIL (fdo#107174)
fi-kbl-7567u: PASS -> DMESG-FAIL (fdo#106947, fdo#106560)
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
{fi-byt-clapper}: PASS -> FAIL (fdo#103191, fdo#107362)
Possible fixes
igt@drv_module_reload@basic-reload:
fi-glk-j4005: DMESG-WARN (fdo#106248, fdo#106725) -> PASS
igt@drv_selftest@live_hangcheck:
{fi-icl-u}: INCOMPLETE (fdo#107399) -> PASS
igt@drv_selftest@live_workarounds:
{fi-bsw-kefka}: DMESG-FAIL (fdo#107292) -> PASS
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-glk-j4005: FAIL (fdo#100368) -> PASS
Warnings
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725
fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
fdo#107399 https://bugs.freedesktop.org/show_bug.cgi?id=107399
== Participating hosts (53 -> 48) ==
Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4643 -> Patchwork_9919
CI_DRM_4643: 2fab0affe5a9f87309773bc4cbef828f4dde7acd @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_4592: fce9638b2e60afce872b3056c19a729b1b3708be @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9919: 51a3d4c0faefb9faf09007614069ea315d0ffb57 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
51a3d4c0faef drm/i915/icl: account for context save/restore removed bits
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9919/issues.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6
Op 09-08-18 om 16:21 schreef Maarten Lankhorst: > Currently tests modify i915.enable_psr and then do a modeset cycle > to change PSR. We can write a value to i915_edp_psr_debug to force > a certain PSR mode without a modeset. > > To retain compatibility with older userspace, we also still allow > the override through the module parameter, and add some tracking > to check whether a debugfs mode is specified. > > Changes since v1: > - Rename dev_priv->psr.enabled to .dp, and .hw_configured to .enabled. > - Fix i915_psr_debugfs_mode to match the writes to debugfs. > - Rename __i915_edp_psr_write to intel_psr_set_debugfs_mode, simplify > it and move it to intel_psr.c. This keeps all internals in intel_psr.c > - Perform an interruptible wait for hw completion outside of the psr > lock, instead of being forced to trywait and return -EBUSY. > Changes since v2: > - Rebase on top of intel_psr changes. > Changes since v3: > - Assign psr.dp during init. (dhnkrn) > - Add prepared bool, which should be used instead of relying on psr.dp. > (dhnkrn) > - Fix -EDEADLK handling in debugfs. (dhnkrn) > - Clean up waiting for idle in intel_psr_set_debugfs_mode. > - Print PSR mode when trying to enable PSR. (dhnkrn) > - Move changing psr debug setting to i915_edp_psr_debug_set. (dhnkrn) > Changes since v4: > - Return error in _set() function. > - Change flag values to make them easier to remember. (dhnkrn) > - Only assign psr.dp once. (dhnkrn) > - Only set crtc_state->has_psr on the crtc with psr.dp. > - Fix typo. (dhnkrn) > Changes since v5: > - Only wait for PSR idle on the PSR connector correctly. (dhnkrn) > - Reinstate WARN_ON(drrs.dp) in intel_psr_enable. (dhnkrn) > - Remove stray comment. (dhnkrn) > - Be silent in intel_psr_compute_config on wrong connector. (dhnkrn) > > Signed-off-by: Maarten Lankhorst > Cc: Rodrigo Vivi > Cc: Dhinakaran Pandiyan > --- > drivers/gpu/drm/i915/i915_debugfs.c | 23 - > drivers/gpu/drm/i915/i915_drv.h | 12 ++- > drivers/gpu/drm/i915/i915_irq.c | 2 +- > drivers/gpu/drm/i915/intel_drv.h| 3 + > drivers/gpu/drm/i915/intel_psr.c| 134 +++- > 5 files changed, 146 insertions(+), 28 deletions(-) Pushed with dhnkrn's irc r-b ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [RFC,1/8] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
== Series Details == Series: series starting with [RFC,1/8] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable URL : https://patchwork.freedesktop.org/series/47989/ State : failure == Summary == Applying: drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable Applying: drm/i915: Track all held rpm wakerefs Applying: drm/i915: Markup paired operations on wakerefs Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/i915_debugfs.c M drivers/gpu/drm/i915/i915_drv.h M drivers/gpu/drm/i915/i915_irq.c M drivers/gpu/drm/i915/intel_drv.h Falling back to patching base and 3-way merge... Auto-merging drivers/gpu/drm/i915/intel_drv.h Auto-merging drivers/gpu/drm/i915/i915_irq.c Auto-merging drivers/gpu/drm/i915/i915_drv.h Auto-merging drivers/gpu/drm/i915/i915_debugfs.c CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_debugfs.c error: Failed to merge in the changes. Patch failed at 0003 drm/i915: Markup paired operations on wakerefs Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/3] igt/gem_sync: Exercising waiting while keeping the GPU busy
Normally we wait on the last request, but that overlooks any
difficulties in waiting on a request while the next is being qeued.
Check those.
Signed-off-by: Chris Wilson
---
tests/gem_sync.c | 72
1 file changed, 72 insertions(+)
diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index c697220ad..fb209977d 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -294,6 +294,74 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
}
+static void active_ring(int fd, unsigned ring, int timeout)
+{
+ unsigned engines[16];
+ const char *names[16];
+ int num_engines = 0;
+
+ if (ring == ALL_ENGINES) {
+ for_each_physical_engine(fd, ring) {
+ if (!gem_can_store_dword(fd, ring))
+ continue;
+
+ names[num_engines] = e__->name;
+ engines[num_engines++] = ring;
+ if (num_engines == ARRAY_SIZE(engines))
+ break;
+ }
+ igt_require(num_engines);
+ } else {
+ gem_require_ring(fd, ring);
+ igt_require(gem_can_store_dword(fd, ring));
+ names[num_engines] = NULL;
+ engines[num_engines++] = ring;
+ }
+
+ intel_detect_and_clear_missed_interrupts(fd);
+ igt_fork(child, num_engines) {
+ double start, end, elapsed;
+ unsigned long cycles;
+ igt_spin_t *spin[2];
+ uint32_t cmd;
+
+ spin[0] = __igt_spin_batch_new(fd,
+ .engine = ring,
+ .flags = IGT_SPIN_FAST);
+ cmd = *spin[0]->batch;
+
+ spin[1] = __igt_spin_batch_new(fd,
+ .engine = ring,
+ .flags = IGT_SPIN_FAST);
+ igt_assert(*spin[1]->batch == cmd);
+
+ start = gettime();
+ end = start + timeout;
+ cycles = 0;
+ do {
+ for (int loop = 0; loop < 1024; loop++) {
+ igt_spin_t *s = spin[loop & 1];
+
+ igt_spin_batch_end(s);
+ gem_sync(fd, s->handle);
+
+ *s->batch = cmd;
+ gem_execbuf(fd, >execbuf);
+ }
+ cycles += 1024;
+ } while ((elapsed = gettime()) < end);
+ igt_spin_batch_free(fd, spin[1]);
+ igt_spin_batch_free(fd, spin[0]);
+
+ igt_info("%s%sompleted %ld cycles: %.3f us\n",
+names[child % num_engines] ?: "",
+names[child % num_engines] ? " c" : "C",
+cycles, (elapsed - start)*1e6/cycles);
+ }
+ igt_waitchildren_timeout(2*timeout, NULL);
+ igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
+}
+
static void
active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
{
@@ -1154,6 +1222,8 @@ igt_main
sync_ring(fd, e->exec_id | e->flags, 1, 150);
igt_subtest_f("idle-%s", e->name)
idle_ring(fd, e->exec_id | e->flags, 150);
+ igt_subtest_f("active-%s", e->name)
+ active_ring(fd, e->exec_id | e->flags, 150);
igt_subtest_f("wakeup-%s", e->name)
wakeup_ring(fd, e->exec_id | e->flags, 150, 1);
igt_subtest_f("active-wakeup-%s", e->name)
@@ -1188,6 +1258,8 @@ igt_main
sync_ring(fd, ALL_ENGINES, ncpus, 150);
igt_subtest("forked-store-each")
store_ring(fd, ALL_ENGINES, ncpus, 150);
+ igt_subtest("active-each")
+ active_ring(fd, ALL_ENGINES, 150);
igt_subtest("wakeup-each")
wakeup_ring(fd, ALL_ENGINES, 150, 1);
igt_subtest("active-wakeup-each")
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/3] gem_sync: Measure wakeup latency while also scheduling the next batch
More variants on stress waits to serve the dual purpose of investigating
different aspects of the latency (this time while also serving
execlists interrupts) while also checking that we never miss the wakeup.
Signed-off-by: Chris Wilson
---
tests/gem_sync.c | 142 +++
1 file changed, 142 insertions(+)
diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index 495ca3b53..c697220ad 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -294,6 +294,144 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
}
+static void
+active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
+{
+ unsigned engines[16];
+ const char *names[16];
+ int num_engines = 0;
+
+ if (ring == ALL_ENGINES) {
+ for_each_physical_engine(fd, ring) {
+ if (!gem_can_store_dword(fd, ring))
+ continue;
+
+ names[num_engines] = e__->name;
+ engines[num_engines++] = ring;
+ if (num_engines == ARRAY_SIZE(engines))
+ break;
+ }
+ igt_require(num_engines);
+ } else {
+ gem_require_ring(fd, ring);
+ igt_require(gem_can_store_dword(fd, ring));
+ names[num_engines] = NULL;
+ engines[num_engines++] = ring;
+ }
+
+ intel_detect_and_clear_missed_interrupts(fd);
+ igt_fork(child, num_engines) {
+ const uint32_t bbe = MI_BATCH_BUFFER_END;
+ struct drm_i915_gem_exec_object2 object;
+ struct drm_i915_gem_execbuffer2 execbuf;
+ double end, this, elapsed, now, baseline;
+ unsigned long cycles;
+ igt_spin_t *spin[2];
+ uint32_t cmd;
+
+ memset(, 0, sizeof(object));
+ object.handle = gem_create(fd, 4096);
+ gem_write(fd, object.handle, 0, , sizeof(bbe));
+
+ memset(, 0, sizeof(execbuf));
+ execbuf.buffers_ptr = to_user_pointer();
+ execbuf.buffer_count = 1;
+ execbuf.flags = engines[child % num_engines];
+
+ spin[0] = __igt_spin_batch_new(fd,
+ .engine = execbuf.flags,
+ .flags = (IGT_SPIN_POLL_RUN |
+IGT_SPIN_FAST));
+ igt_assert(spin[0]->running);
+ cmd = *spin[0]->batch;
+
+ spin[1] = __igt_spin_batch_new(fd,
+ .engine = execbuf.flags,
+ .flags = (IGT_SPIN_POLL_RUN |
+IGT_SPIN_FAST));
+
+ gem_execbuf(fd, );
+
+ igt_spin_batch_end(spin[1]);
+ igt_spin_batch_end(spin[0]);
+ gem_sync(fd, object.handle);
+
+ for (int warmup = 0; warmup <= 1; warmup++) {
+ *spin[0]->batch = cmd;
+ *spin[0]->running = 0;
+ gem_execbuf(fd, [0]->execbuf);
+
+ end = gettime() + timeout/10.;
+ elapsed = 0;
+ cycles = 0;
+ do {
+ while (!READ_ONCE(*spin[0]->running))
+ ;
+
+ *spin[1]->batch = cmd;
+ *spin[1]->running = 0;
+ gem_execbuf(fd, [1]->execbuf);
+
+ this = gettime();
+ igt_spin_batch_end(spin[0]);
+ gem_sync(fd, spin[0]->handle);
+ now = gettime();
+
+ elapsed += now - this;
+ cycles++;
+ igt_swap(spin[0], spin[1]);
+ } while (now < end);
+ igt_spin_batch_end(spin[0]);
+ baseline = elapsed / cycles;
+ }
+ igt_info("%s%saseline %ld cycles: %.3f us\n",
+names[child % num_engines] ?: "",
+names[child % num_engines] ? " b" : "B",
+cycles, elapsed*1e6/cycles);
+
+ *spin[0]->batch = cmd;
+ *spin[0]->running = 0;
+ gem_execbuf(fd, [0]->execbuf);
+
+ end = gettime() + timeout;
+ elapsed = 0;
+ cycles = 0;
+ do {
+ while (!READ_ONCE(*spin[0]->running))
+ ;
+
+ for (int n = 0; n < wlen; n++)
+
[Intel-gfx] [PATCH i-g-t 1/3] igt/gem_sync: Exercise sync after context switch
This exercises a special case that may be of interest, waiting for a
context that may be preempted in order to reduce the wait.
Signed-off-by: Chris Wilson
---
tests/gem_sync.c | 146 +++
1 file changed, 146 insertions(+)
diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index 493ae61df..495ca3b53 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -409,6 +409,144 @@ store_ring(int fd, unsigned ring, int num_children, int
timeout)
igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
}
+static void
+switch_ring(int fd, unsigned ring, int num_children, int timeout)
+{
+ const int gen = intel_gen(intel_get_drm_devid(fd));
+ unsigned engines[16];
+ const char *names[16];
+ int num_engines = 0;
+
+ gem_require_contexts(fd);
+
+ if (ring == ALL_ENGINES) {
+ for_each_physical_engine(fd, ring) {
+ if (!gem_can_store_dword(fd, ring))
+ continue;
+
+ names[num_engines] = e__->name;
+ engines[num_engines++] = ring;
+ if (num_engines == ARRAY_SIZE(engines))
+ break;
+ }
+
+ num_children *= num_engines;
+ } else {
+ gem_require_ring(fd, ring);
+ igt_require(gem_can_store_dword(fd, ring));
+ names[num_engines] = NULL;
+ engines[num_engines++] = ring;
+ }
+
+ intel_detect_and_clear_missed_interrupts(fd);
+ igt_fork(child, num_children) {
+ struct context {
+ struct drm_i915_gem_exec_object2 object[2];
+ struct drm_i915_gem_relocation_entry reloc[1024];
+ struct drm_i915_gem_execbuffer2 execbuf;
+ } contexts[2];
+ double start, elapsed;
+ unsigned long cycles;
+
+ for (int i = 0; i < ARRAY_SIZE(contexts); i++) {
+ const uint32_t bbe = MI_BATCH_BUFFER_END;
+ const uint32_t sz = 32 << 10;
+ struct context *c = [i];
+ uint32_t *batch, *b;
+
+ memset(>execbuf, 0, sizeof(c->execbuf));
+ c->execbuf.buffers_ptr = to_user_pointer(c->object);
+ c->execbuf.flags = engines[child % num_engines];
+ c->execbuf.flags |= LOCAL_I915_EXEC_NO_RELOC;
+ c->execbuf.flags |= LOCAL_I915_EXEC_HANDLE_LUT;
+ if (gen < 6)
+ c->execbuf.flags |= I915_EXEC_SECURE;
+ c->execbuf.rsvd1 = gem_context_create(fd);
+
+ memset(c->object, 0, sizeof(c->object));
+ c->object[0].handle = gem_create(fd, 4096);
+ gem_write(fd, c->object[0].handle, 0, ,
sizeof(bbe));
+ c->execbuf.buffer_count = 1;
+ gem_execbuf(fd, >execbuf);
+
+ c->object[0].flags |= EXEC_OBJECT_WRITE;
+ c->object[1].handle = gem_create(fd, sz);
+
+ c->object[1].relocs_ptr = to_user_pointer(c->reloc);
+ c->object[1].relocation_count = 1024;
+
+ batch = gem_mmap__cpu(fd, c->object[1].handle, 0, sz,
+ PROT_WRITE | PROT_READ);
+ gem_set_domain(fd, c->object[1].handle,
+ I915_GEM_DOMAIN_CPU,
I915_GEM_DOMAIN_CPU);
+
+ memset(c->reloc, 0, sizeof(c->reloc));
+ b = batch;
+ for (int r = 0; r < 1024; r++) {
+ uint64_t offset;
+
+ c->reloc[r].presumed_offset =
c->object[0].offset;
+ c->reloc[r].offset = (b - batch + 1) *
sizeof(*batch);
+ c->reloc[r].delta = r * sizeof(uint32_t);
+ c->reloc[r].read_domains =
I915_GEM_DOMAIN_INSTRUCTION;
+ c->reloc[r].write_domain =
I915_GEM_DOMAIN_INSTRUCTION;
+
+ offset = c->object[0].offset +
c->reloc[r].delta;
+ *b++ = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22
: 0);
+ if (gen >= 8) {
+ *b++ = offset;
+ *b++ = offset >> 32;
+ } else if (gen >= 4) {
+ *b++ = 0;
+ *b++ = offset;
+ c->reloc[r].offset += sizeof(*batch);
+ } else {
+ b[-1] -= 1;
+
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] intel-ci: Skip module reloads
Quoting Petri Latvala (2018-08-10 10:55:06) > On Fri, Aug 10, 2018 at 10:41:45AM +0100, Chris Wilson wrote: > > Reloading the module may impact subsequent tests by destabilising the > > system. As we do for BAT, if we want to test reloads, it should be > > handled explicitly at the end of the run, rather than placed at random > > in the middle of the test list. > > > > v2: Commentary > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=106539 > > Signed-off-by: Chris Wilson > > Cc: Tomi Sarvela > > > The series is > > Acked-by: Petri Latvala I've pushed this to see if this does indeed stabilise our test results as currently we have random failures that change on every shardlist rebuild. I think we need an explicit test list to check after reload. I think BAT? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] intel-ci: Skip module reloads
On Fri, Aug 10, 2018 at 10:41:45AM +0100, Chris Wilson wrote: > Reloading the module may impact subsequent tests by destabilising the > system. As we do for BAT, if we want to test reloads, it should be > handled explicitly at the end of the run, rather than placed at random > in the middle of the test list. > > v2: Commentary > > References: https://bugs.freedesktop.org/show_bug.cgi?id=106539 > Signed-off-by: Chris Wilson > Cc: Tomi Sarvela The series is Acked-by: Petri Latvala > --- > tests/intel-ci/blacklist.txt | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt > index 7700a60b7..c93554a37 100644 > --- a/tests/intel-ci/blacklist.txt > +++ b/tests/intel-ci/blacklist.txt > @@ -5,6 +5,15 @@ igt@meta_test(@.*)? > igt@drv_selftest(@.*)? > igt@drm_mm(@.*)? > ### > +# Handle module reloads with great care! > +# > +# Reloading a module is more likely to leave > +# the machine in an unexpected state than other > +# self-contained tests, leading to random > +# failures in tests run afterwards. > +### > +igt@drv_module_reload(@.*)? > +### > # GEM > ### > igt@gem_busy@(?!.*basic).*hang.* > -- > 2.18.0 > > ___ > igt-dev mailing list > igt-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/igt-dev ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/2] intel-ci: Comment on kernel selftest exclusion
The kernel selftests are excluded from the shard lists as those lists are compiled separately. Signed-off-by: Chris Wilson --- tests/intel-ci/blacklist.txt | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt index 1d1652964..7700a60b7 100644 --- a/tests/intel-ci/blacklist.txt +++ b/tests/intel-ci/blacklist.txt @@ -1,9 +1,12 @@ igt@meta_test(@.*)? ### -# GEM +# Kernel selftests (run separately) ### igt@drv_selftest(@.*)? igt@drm_mm(@.*)? +### +# GEM +### igt@gem_busy@(?!.*basic).*hang.* igt@gem_close_race@(?!.*basic).* igt@gem_concurrent_blit(@.*)? -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/2] intel-ci: Skip module reloads
Reloading the module may impact subsequent tests by destabilising the system. As we do for BAT, if we want to test reloads, it should be handled explicitly at the end of the run, rather than placed at random in the middle of the test list. v2: Commentary References: https://bugs.freedesktop.org/show_bug.cgi?id=106539 Signed-off-by: Chris Wilson Cc: Tomi Sarvela --- tests/intel-ci/blacklist.txt | 9 + 1 file changed, 9 insertions(+) diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt index 7700a60b7..c93554a37 100644 --- a/tests/intel-ci/blacklist.txt +++ b/tests/intel-ci/blacklist.txt @@ -5,6 +5,15 @@ igt@meta_test(@.*)? igt@drv_selftest(@.*)? igt@drm_mm(@.*)? ### +# Handle module reloads with great care! +# +# Reloading a module is more likely to leave +# the machine in an unexpected state than other +# self-contained tests, leading to random +# failures in tests run afterwards. +### +igt@drv_module_reload(@.*)? +### # GEM ### igt@gem_busy@(?!.*basic).*hang.* -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] intel-ci: Skip module reloads
On Fri, Aug 10, 2018 at 10:21:59AM +0100, Chris Wilson wrote: > Quoting Petri Latvala (2018-08-10 10:11:29) > > On Fri, Aug 10, 2018 at 10:02:46AM +0100, Chris Wilson wrote: > > > Reloading the module may impact subsequent tests by destabilising the > > > system. As we do for BAT, if we want to test reloads, it should be > > > handled explicitly at the end of the run, rather than placed at random > > > in the middle of the test list. > > > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=106539 > > > Signed-off-by: Chris Wilson > > > Cc: Tomi Sarvela > > > --- > > > tests/intel-ci/blacklist.txt | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt > > > index 1d1652964..066a5c014 100644 > > > --- a/tests/intel-ci/blacklist.txt > > > +++ b/tests/intel-ci/blacklist.txt > > > @@ -2,6 +2,7 @@ igt@meta_test(@.*)? > > > ### > > > # GEM > > > ### > > > +igt@drv_module_reload(@.*)? > > > > "GEM"? > > That didn't apply to the selftests or drm_mm, so I thought it was the > usual misleading comment. > > > Comment the line with the explanation from the commit message. > > Where are comments allowed? I believe Tomi's handling of blacklist.txt considers anything beginning with # a comment. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] RFC: Add write flag to reservation object fences
On Fri, Aug 10, 2018 at 11:14 AM, Christian König wrote: > Am 10.08.2018 um 10:29 schrieb Daniel Vetter: >> >> [SNIP] >> I'm only interested in the case of shared buffers. And for those you >> _do_ pessimistically assume that all access must be implicitly synced. >> Iirc amdgpu doesn't support EGL_ANDROID_native_fence_sync, so this >> makes sense that you don't bother with it. > > > See flag AMDGPU_GEM_CREATE_EXPLICIT_SYNC. That's for radv. Won't be enough for EGL_ANDROID_native_fence_sync, because you cannot know at buffer allocation time how the fencing will be done in all cases. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] RFC: Add write flag to reservation object fences
On Fri, Aug 10, 2018 at 10:51 AM, Christian König wrote: > Am 10.08.2018 um 10:32 schrieb Daniel Vetter: >> >> On Fri, Aug 10, 2018 at 10:24 AM, Christian König >> wrote: >>> >>> Am 10.08.2018 um 09:51 schrieb Chris Wilson: Quoting Christian König (2018-08-09 15:54:31) > > Am 09.08.2018 um 16:22 schrieb Daniel Vetter: >> >> On Thu, Aug 9, 2018 at 3:58 PM, Christian König >> wrote: >>> >>> Am 09.08.2018 um 15:38 schrieb Daniel Vetter: On Thu, Aug 09, 2018 at 01:37:07PM +0200, Christian König wrote: [SNIP] >>> >>> See to me the explicit fence in the reservation object is not even >>> remotely >>> related to implicit or explicit synchronization. >> >> Hm, I guess that's the confusion then. The only reason we have the >> exclusive fence is to implement cross-driver implicit syncing. What >> else you do internally in your driver doesn't matter, as long as you >> keep up that contract. >> >> And it's intentionally not called write_fence or anything like that, >> because that's not what it tracks. >> >> Of course any buffer moves the kernel does also must be tracked in the >> exclusive fence, because userspace cannot know about these. So you >> might have an exclusive fence set and also an explicit fence passed in >> through the atomic ioctl. Aside: Right now all drivers only observe >> one or the other, not both, so will break as soon as we start moving >> shared buffers around. At least on Android or anything else using >> explicit fencing. > > Actually both radeon and nouveau use the approach that shared fences > need to wait on as well when they don't come from the current driver. nouveau has write hazard tracking in its api , and is using the excl fence for it was well. As far as I can tell, this is all about fixing the lack of acknowledgement of the requirement for implicit fence tracking in amdgpu's (and its radeon predecessor) ABI. >>> >>> >>> Well I agree that implicit fencing was a bad idea to begin with, but the >>> solution you guys came up with is not going to work in all cases either. >>> Which is fine so long as you get userspace to only use explicit fence passing to the compositor. >>> >>> >>> Well exactly that's the problem. >>> >>> I can't pass exactly one explicit fence to the compositor because I have >>> multiple command submissions which run asynchronously and need to finish >>> before the compositor can start to use the surface. >>> >>> So the whole concept of using a single exclusive fence doesn't work in >>> the >>> case of amdgpu. And to be honest I think all drivers with multiple >>> engines >>> could benefit of that as well. >> >> Fences can be merge. This works, really :-) In amdgpu's implementation >> of EGL_ANDROID_native_fence_sync you will need to do that before >> passing the result to the caller. > > Yeah, but that is for the userspace API. Not for any internal handling in > the driver. dma_fence_array? Or how do you think this fence merging for userspace is implemented? The only trouble is that amdgpu doesn't have an explicit hand-over point in the uapi where an explicitly synced buffer (all of them really) gets its fences for implicit syncing attached. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] intel-ci: Skip module reloads
Quoting Petri Latvala (2018-08-10 10:11:29) > On Fri, Aug 10, 2018 at 10:02:46AM +0100, Chris Wilson wrote: > > Reloading the module may impact subsequent tests by destabilising the > > system. As we do for BAT, if we want to test reloads, it should be > > handled explicitly at the end of the run, rather than placed at random > > in the middle of the test list. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=106539 > > Signed-off-by: Chris Wilson > > Cc: Tomi Sarvela > > --- > > tests/intel-ci/blacklist.txt | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt > > index 1d1652964..066a5c014 100644 > > --- a/tests/intel-ci/blacklist.txt > > +++ b/tests/intel-ci/blacklist.txt > > @@ -2,6 +2,7 @@ igt@meta_test(@.*)? > > ### > > # GEM > > ### > > +igt@drv_module_reload(@.*)? > > "GEM"? That didn't apply to the selftests or drm_mm, so I thought it was the usual misleading comment. > Comment the line with the explanation from the commit message. Where are comments allowed? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] RFC: Add write flag to reservation object fences
On Fri, Aug 10, 2018 at 11:14 AM, Christian König wrote: > Am 10.08.2018 um 10:29 schrieb Daniel Vetter: >> >> [SNIP] >> I'm only interested in the case of shared buffers. And for those you >> _do_ pessimistically assume that all access must be implicitly synced. >> Iirc amdgpu doesn't support EGL_ANDROID_native_fence_sync, so this >> makes sense that you don't bother with it. > > > See flag AMDGPU_GEM_CREATE_EXPLICIT_SYNC. > > >> - as a consequence, amdgpu needs to pessimistically assume that all writes to shared buffer need to obey implicit fencing rules. - for shared buffers (across process or drivers) implicit fencing does _not_ allow concurrent writers. That limitation is why people want to do explicit fencing, and it's the reason why there's only 1 slot for an exclusive. Note I really mean concurrent here, a queue of in-flight writes by different batches is perfectly fine. But it's a fully ordered queue of writes. - but as a consequence of amdgpu's lack of implicit fencing and hence need to pessimistically assume there's multiple write fences amdgpu needs to put multiple fences behind the single exclusive slot. This is a limitation imposed by by the amdgpu stack, not something inherit to how implicit fencing works. - Chris Wilson's patch implements all this (and afaics with a bit more coffee, correctly). If you want to be less pessimistic in amdgpu for shared buffers, you need to start tracking which shared buffer access need implicit and which explicit sync. What you can't do is suddenly create more than 1 exclusive fence, that's not how implicit fencing works. Another thing you cannot do is force everyone else (in non-amdgpu or core code) to sync against _all_ writes, because that forces implicit syncing. Which people very much don't want. >>> >>> >>> I also do see the problem that most other hardware doesn't need that >>> functionality, because it is driven by a single engine. That's why I >>> tried >>> to keep the overhead as low as possible. >>> >>> But at least for amdgpu (and I strongly suspect for nouveau as well) it >>> is >>> absolutely vital in a number of cases to allow concurrent accesses from >>> the >>> same client even when the BO is then later used with implicit >>> synchronization. >>> >>> This is also the reason why the current workaround is so problematic for >>> us. >>> Cause as soon as the BO is shared with another (non-amdgpu) device all >>> command submissions to it will be serialized even when they come from the >>> same client. >>> >>> Would it be an option extend the concept of the "owner" of the BO amdpgu >>> uses to other drivers as well? >>> >>> When you already have explicit synchronization insider your client, but >>> not >>> between clients (e.g. still uses DRI2 or DRI3), this could also be rather >>> beneficial for others as well. >> >> Again: How you synchronize your driver internal rendering is totally >> up to you. If you see an exclusive fence by amdgpu, and submit new >> rendering by amdgpu, you can totally ignore the exclusive fence. The >> only api contracts for implicit fencing are between drivers for shared >> buffers. If you submit rendering to a shared buffer in parallel, all >> without attaching an exclusive fence that's perfectly fine, but >> somewhen later on, depending upon protocol (glFlush or glxSwapBuffers >> or whatever) you have to collect all those concurrent write hazards >> and bake them into 1 single exclusive fence for implicit fencing. >> >> Atm (and Chris seems to concur) the amdgpu uapi doesn't allow you to >> do that, so for anything shared you have to be super pessimistic. >> Adding a HAND_OFF_FOR_IMPLICIT_FENCING flag/ioctl would probably fix >> that. Only when that flag is set would you take all shared write >> hazards and bake them into one exclusive fence for hand-off to the >> next driver. You'd also need the same when receiving an implicitly >> fenced buffer, to make sure that your concurrent writes do synchronize >> with reading (aka shared fences) done by other drivers. With a bunch >> of trickery and hacks it might be possible to infer this from current >> ioctls even, but you need to be really careful. > > > A new uapi is out of question because we need to be backward compatible. Since when is new uapi out of the question for a performance improvement? >> And you're right that amdgpu seems to be the only (or one of the only) >> drivers which do truly concurrent rendering to the same buffer (not >> just concurrent rendering to multiple buffers all suballocated from >> the same bo). But we can't fix this in the kernel with the tricks you >> propose, because without such an uapi extension (or inference) we >> can't tell the implicit fencing from the explicit fencing case. > > > Sure we can. As I said for amdgpu that is absolutely no problem at all. > > In your terminology all rendering from the same client to a BO
Re: [Intel-gfx] RFC: Add write flag to reservation object fences
Am 10.08.2018 um 10:29 schrieb Daniel Vetter: [SNIP] I'm only interested in the case of shared buffers. And for those you _do_ pessimistically assume that all access must be implicitly synced. Iirc amdgpu doesn't support EGL_ANDROID_native_fence_sync, so this makes sense that you don't bother with it. See flag AMDGPU_GEM_CREATE_EXPLICIT_SYNC. - as a consequence, amdgpu needs to pessimistically assume that all writes to shared buffer need to obey implicit fencing rules. - for shared buffers (across process or drivers) implicit fencing does _not_ allow concurrent writers. That limitation is why people want to do explicit fencing, and it's the reason why there's only 1 slot for an exclusive. Note I really mean concurrent here, a queue of in-flight writes by different batches is perfectly fine. But it's a fully ordered queue of writes. - but as a consequence of amdgpu's lack of implicit fencing and hence need to pessimistically assume there's multiple write fences amdgpu needs to put multiple fences behind the single exclusive slot. This is a limitation imposed by by the amdgpu stack, not something inherit to how implicit fencing works. - Chris Wilson's patch implements all this (and afaics with a bit more coffee, correctly). If you want to be less pessimistic in amdgpu for shared buffers, you need to start tracking which shared buffer access need implicit and which explicit sync. What you can't do is suddenly create more than 1 exclusive fence, that's not how implicit fencing works. Another thing you cannot do is force everyone else (in non-amdgpu or core code) to sync against _all_ writes, because that forces implicit syncing. Which people very much don't want. I also do see the problem that most other hardware doesn't need that functionality, because it is driven by a single engine. That's why I tried to keep the overhead as low as possible. But at least for amdgpu (and I strongly suspect for nouveau as well) it is absolutely vital in a number of cases to allow concurrent accesses from the same client even when the BO is then later used with implicit synchronization. This is also the reason why the current workaround is so problematic for us. Cause as soon as the BO is shared with another (non-amdgpu) device all command submissions to it will be serialized even when they come from the same client. Would it be an option extend the concept of the "owner" of the BO amdpgu uses to other drivers as well? When you already have explicit synchronization insider your client, but not between clients (e.g. still uses DRI2 or DRI3), this could also be rather beneficial for others as well. Again: How you synchronize your driver internal rendering is totally up to you. If you see an exclusive fence by amdgpu, and submit new rendering by amdgpu, you can totally ignore the exclusive fence. The only api contracts for implicit fencing are between drivers for shared buffers. If you submit rendering to a shared buffer in parallel, all without attaching an exclusive fence that's perfectly fine, but somewhen later on, depending upon protocol (glFlush or glxSwapBuffers or whatever) you have to collect all those concurrent write hazards and bake them into 1 single exclusive fence for implicit fencing. Atm (and Chris seems to concur) the amdgpu uapi doesn't allow you to do that, so for anything shared you have to be super pessimistic. Adding a HAND_OFF_FOR_IMPLICIT_FENCING flag/ioctl would probably fix that. Only when that flag is set would you take all shared write hazards and bake them into one exclusive fence for hand-off to the next driver. You'd also need the same when receiving an implicitly fenced buffer, to make sure that your concurrent writes do synchronize with reading (aka shared fences) done by other drivers. With a bunch of trickery and hacks it might be possible to infer this from current ioctls even, but you need to be really careful. A new uapi is out of question because we need to be backward compatible. And you're right that amdgpu seems to be the only (or one of the only) drivers which do truly concurrent rendering to the same buffer (not just concurrent rendering to multiple buffers all suballocated from the same bo). But we can't fix this in the kernel with the tricks you propose, because without such an uapi extension (or inference) we can't tell the implicit fencing from the explicit fencing case. Sure we can. As I said for amdgpu that is absolutely no problem at all. In your terminology all rendering from the same client to a BO is explicitly fenced, while all rendering from different clients are implicit fenced. And for shared buffers with explicit fencing we _must_ _not_ sync against all writes. owner won't help here, because it's still not tracking whether something is explicit or implicit synced. Implicit syncing can be disable by giving the AMDGPU_GEM_CREATE_EXPLICIT_SYNC flag while creating the BO. We've cheated a bit with most other drivers in this area, also
Re: [Intel-gfx] [PATCH i-g-t] intel-ci: Skip module reloads
On Fri, Aug 10, 2018 at 10:02:46AM +0100, Chris Wilson wrote: > Reloading the module may impact subsequent tests by destabilising the > system. As we do for BAT, if we want to test reloads, it should be > handled explicitly at the end of the run, rather than placed at random > in the middle of the test list. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=106539 > Signed-off-by: Chris Wilson > Cc: Tomi Sarvela > --- > tests/intel-ci/blacklist.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt > index 1d1652964..066a5c014 100644 > --- a/tests/intel-ci/blacklist.txt > +++ b/tests/intel-ci/blacklist.txt > @@ -2,6 +2,7 @@ igt@meta_test(@.*)? > ### > # GEM > ### > +igt@drv_module_reload(@.*)? "GEM"? Comment the line with the explanation from the commit message. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] intel-ci: Skip module reloads
Reloading the module may impact subsequent tests by destabilising the system. As we do for BAT, if we want to test reloads, it should be handled explicitly at the end of the run, rather than placed at random in the middle of the test list. References: https://bugs.freedesktop.org/show_bug.cgi?id=106539 Signed-off-by: Chris Wilson Cc: Tomi Sarvela --- tests/intel-ci/blacklist.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt index 1d1652964..066a5c014 100644 --- a/tests/intel-ci/blacklist.txt +++ b/tests/intel-ci/blacklist.txt @@ -2,6 +2,7 @@ igt@meta_test(@.*)? ### # GEM ### +igt@drv_module_reload(@.*)? igt@drv_selftest(@.*)? igt@drm_mm(@.*)? igt@gem_busy@(?!.*basic).*hang.* -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] RFC: Add write flag to reservation object fences
Am 10.08.2018 um 10:32 schrieb Daniel Vetter: On Fri, Aug 10, 2018 at 10:24 AM, Christian König wrote: Am 10.08.2018 um 09:51 schrieb Chris Wilson: Quoting Christian König (2018-08-09 15:54:31) Am 09.08.2018 um 16:22 schrieb Daniel Vetter: On Thu, Aug 9, 2018 at 3:58 PM, Christian König wrote: Am 09.08.2018 um 15:38 schrieb Daniel Vetter: On Thu, Aug 09, 2018 at 01:37:07PM +0200, Christian König wrote: [SNIP] See to me the explicit fence in the reservation object is not even remotely related to implicit or explicit synchronization. Hm, I guess that's the confusion then. The only reason we have the exclusive fence is to implement cross-driver implicit syncing. What else you do internally in your driver doesn't matter, as long as you keep up that contract. And it's intentionally not called write_fence or anything like that, because that's not what it tracks. Of course any buffer moves the kernel does also must be tracked in the exclusive fence, because userspace cannot know about these. So you might have an exclusive fence set and also an explicit fence passed in through the atomic ioctl. Aside: Right now all drivers only observe one or the other, not both, so will break as soon as we start moving shared buffers around. At least on Android or anything else using explicit fencing. Actually both radeon and nouveau use the approach that shared fences need to wait on as well when they don't come from the current driver. nouveau has write hazard tracking in its api , and is using the excl fence for it was well. As far as I can tell, this is all about fixing the lack of acknowledgement of the requirement for implicit fence tracking in amdgpu's (and its radeon predecessor) ABI. Well I agree that implicit fencing was a bad idea to begin with, but the solution you guys came up with is not going to work in all cases either. Which is fine so long as you get userspace to only use explicit fence passing to the compositor. Well exactly that's the problem. I can't pass exactly one explicit fence to the compositor because I have multiple command submissions which run asynchronously and need to finish before the compositor can start to use the surface. So the whole concept of using a single exclusive fence doesn't work in the case of amdgpu. And to be honest I think all drivers with multiple engines could benefit of that as well. Fences can be merge. This works, really :-) In amdgpu's implementation of EGL_ANDROID_native_fence_sync you will need to do that before passing the result to the caller. Yeah, but that is for the userspace API. Not for any internal handling in the driver. Christian. -Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] RFC: Add write flag to reservation object fences
On Fri, Aug 10, 2018 at 10:24 AM, Christian König wrote: > Am 10.08.2018 um 09:51 schrieb Chris Wilson: >> >> Quoting Christian König (2018-08-09 15:54:31) >>> >>> Am 09.08.2018 um 16:22 schrieb Daniel Vetter: On Thu, Aug 9, 2018 at 3:58 PM, Christian König wrote: > > Am 09.08.2018 um 15:38 schrieb Daniel Vetter: >> >> On Thu, Aug 09, 2018 at 01:37:07PM +0200, Christian König wrote: >> [SNIP] > > See to me the explicit fence in the reservation object is not even > remotely > related to implicit or explicit synchronization. Hm, I guess that's the confusion then. The only reason we have the exclusive fence is to implement cross-driver implicit syncing. What else you do internally in your driver doesn't matter, as long as you keep up that contract. And it's intentionally not called write_fence or anything like that, because that's not what it tracks. Of course any buffer moves the kernel does also must be tracked in the exclusive fence, because userspace cannot know about these. So you might have an exclusive fence set and also an explicit fence passed in through the atomic ioctl. Aside: Right now all drivers only observe one or the other, not both, so will break as soon as we start moving shared buffers around. At least on Android or anything else using explicit fencing. >>> >>> Actually both radeon and nouveau use the approach that shared fences >>> need to wait on as well when they don't come from the current driver. >> >> nouveau has write hazard tracking in its api , and is using the >> excl fence for it was well. >> >> As far as I can tell, this is all about fixing the lack of >> acknowledgement of the requirement for implicit fence tracking in >> amdgpu's (and its radeon predecessor) ABI. > > > Well I agree that implicit fencing was a bad idea to begin with, but the > solution you guys came up with is not going to work in all cases either. > >> Which is fine so long as you >> get userspace to only use explicit fence passing to the compositor. > > > Well exactly that's the problem. > > I can't pass exactly one explicit fence to the compositor because I have > multiple command submissions which run asynchronously and need to finish > before the compositor can start to use the surface. > > So the whole concept of using a single exclusive fence doesn't work in the > case of amdgpu. And to be honest I think all drivers with multiple engines > could benefit of that as well. Fences can be merge. This works, really :-) In amdgpu's implementation of EGL_ANDROID_native_fence_sync you will need to do that before passing the result to the caller. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] RFC: Add write flag to reservation object fences
On Thu, Aug 9, 2018 at 4:54 PM, Christian König wrote: > Am 09.08.2018 um 16:22 schrieb Daniel Vetter: >> >> On Thu, Aug 9, 2018 at 3:58 PM, Christian König >> wrote: >>> >>> Am 09.08.2018 um 15:38 schrieb Daniel Vetter: On Thu, Aug 09, 2018 at 01:37:07PM +0200, Christian König wrote: [SNIP] >>> >>> See to me the explicit fence in the reservation object is not even >>> remotely >>> related to implicit or explicit synchronization. >> >> Hm, I guess that's the confusion then. The only reason we have the >> exclusive fence is to implement cross-driver implicit syncing. What >> else you do internally in your driver doesn't matter, as long as you >> keep up that contract. >> >> And it's intentionally not called write_fence or anything like that, >> because that's not what it tracks. >> >> Of course any buffer moves the kernel does also must be tracked in the >> exclusive fence, because userspace cannot know about these. So you >> might have an exclusive fence set and also an explicit fence passed in >> through the atomic ioctl. Aside: Right now all drivers only observe >> one or the other, not both, so will break as soon as we start moving >> shared buffers around. At least on Android or anything else using >> explicit fencing. > > > Actually both radeon and nouveau use the approach that shared fences need to > wait on as well when they don't come from the current driver. > >> >> So here's my summary, as I understanding things right now: >> - for non-shared buffers at least, amdgpu uses explicit fencing, and >> hence all fences caused by userspace end up as shared fences, whether >> that's writes or reads. This means you end up with possibly multiple >> write fences, but never any exclusive fences. >> - for non-shared buffers the only exclusive fences amdgpu sets are for >> buffer moves done by the kernel. >> - amgpu (kernel + userspace combo here) does not seem to have a >> concept/tracking for when a buffer is used with implicit or explicit >> fencing. It does however track all writes. > > > No, that is incorrect. It tracks all accesses to a buffer object in the form > of shared fences, we don't care if it is a write or not. > > What we track as well is which client uses a BO last and as long as the same > client uses the BO we don't add any implicit synchronization. > > Only when a BO is used by another client we have implicit synchronization > for all command submissions. This behavior can be disable with a flag during > BO creation. I'm only interested in the case of shared buffers. And for those you _do_ pessimistically assume that all access must be implicitly synced. Iirc amdgpu doesn't support EGL_ANDROID_native_fence_sync, so this makes sense that you don't bother with it. >> - as a consequence, amdgpu needs to pessimistically assume that all >> writes to shared buffer need to obey implicit fencing rules. >> - for shared buffers (across process or drivers) implicit fencing does >> _not_ allow concurrent writers. That limitation is why people want to >> do explicit fencing, and it's the reason why there's only 1 slot for >> an exclusive. Note I really mean concurrent here, a queue of in-flight >> writes by different batches is perfectly fine. But it's a fully >> ordered queue of writes. >> - but as a consequence of amdgpu's lack of implicit fencing and hence >> need to pessimistically assume there's multiple write fences amdgpu >> needs to put multiple fences behind the single exclusive slot. This is >> a limitation imposed by by the amdgpu stack, not something inherit to >> how implicit fencing works. >> - Chris Wilson's patch implements all this (and afaics with a bit more >> coffee, correctly). >> >> If you want to be less pessimistic in amdgpu for shared buffers, you >> need to start tracking which shared buffer access need implicit and >> which explicit sync. What you can't do is suddenly create more than 1 >> exclusive fence, that's not how implicit fencing works. Another thing >> you cannot do is force everyone else (in non-amdgpu or core code) to >> sync against _all_ writes, because that forces implicit syncing. Which >> people very much don't want. > > > I also do see the problem that most other hardware doesn't need that > functionality, because it is driven by a single engine. That's why I tried > to keep the overhead as low as possible. > > But at least for amdgpu (and I strongly suspect for nouveau as well) it is > absolutely vital in a number of cases to allow concurrent accesses from the > same client even when the BO is then later used with implicit > synchronization. > > This is also the reason why the current workaround is so problematic for us. > Cause as soon as the BO is shared with another (non-amdgpu) device all > command submissions to it will be serialized even when they come from the > same client. > > Would it be an option extend the concept of the "owner" of the BO amdpgu > uses to other drivers as well? > > When you already have explicit synchronization
Re: [Intel-gfx] RFC: Add write flag to reservation object fences
Am 10.08.2018 um 09:51 schrieb Chris Wilson: Quoting Christian König (2018-08-09 15:54:31) Am 09.08.2018 um 16:22 schrieb Daniel Vetter: On Thu, Aug 9, 2018 at 3:58 PM, Christian König wrote: Am 09.08.2018 um 15:38 schrieb Daniel Vetter: On Thu, Aug 09, 2018 at 01:37:07PM +0200, Christian König wrote: [SNIP] See to me the explicit fence in the reservation object is not even remotely related to implicit or explicit synchronization. Hm, I guess that's the confusion then. The only reason we have the exclusive fence is to implement cross-driver implicit syncing. What else you do internally in your driver doesn't matter, as long as you keep up that contract. And it's intentionally not called write_fence or anything like that, because that's not what it tracks. Of course any buffer moves the kernel does also must be tracked in the exclusive fence, because userspace cannot know about these. So you might have an exclusive fence set and also an explicit fence passed in through the atomic ioctl. Aside: Right now all drivers only observe one or the other, not both, so will break as soon as we start moving shared buffers around. At least on Android or anything else using explicit fencing. Actually both radeon and nouveau use the approach that shared fences need to wait on as well when they don't come from the current driver. nouveau has write hazard tracking in its api , and is using the excl fence for it was well. As far as I can tell, this is all about fixing the lack of acknowledgement of the requirement for implicit fence tracking in amdgpu's (and its radeon predecessor) ABI. Well I agree that implicit fencing was a bad idea to begin with, but the solution you guys came up with is not going to work in all cases either. Which is fine so long as you get userspace to only use explicit fence passing to the compositor. Well exactly that's the problem. I can't pass exactly one explicit fence to the compositor because I have multiple command submissions which run asynchronously and need to finish before the compositor can start to use the surface. So the whole concept of using a single exclusive fence doesn't work in the case of amdgpu. And to be honest I think all drivers with multiple engines could benefit of that as well. Christian. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] RFC: Add write flag to reservation object fences
Quoting Christian König (2018-08-09 15:54:31) > Am 09.08.2018 um 16:22 schrieb Daniel Vetter: > > On Thu, Aug 9, 2018 at 3:58 PM, Christian König > > wrote: > >> Am 09.08.2018 um 15:38 schrieb Daniel Vetter: > >>> On Thu, Aug 09, 2018 at 01:37:07PM +0200, Christian König wrote: > >>> [SNIP] > >> See to me the explicit fence in the reservation object is not even remotely > >> related to implicit or explicit synchronization. > > Hm, I guess that's the confusion then. The only reason we have the > > exclusive fence is to implement cross-driver implicit syncing. What > > else you do internally in your driver doesn't matter, as long as you > > keep up that contract. > > > > And it's intentionally not called write_fence or anything like that, > > because that's not what it tracks. > > > > Of course any buffer moves the kernel does also must be tracked in the > > exclusive fence, because userspace cannot know about these. So you > > might have an exclusive fence set and also an explicit fence passed in > > through the atomic ioctl. Aside: Right now all drivers only observe > > one or the other, not both, so will break as soon as we start moving > > shared buffers around. At least on Android or anything else using > > explicit fencing. > > Actually both radeon and nouveau use the approach that shared fences > need to wait on as well when they don't come from the current driver. nouveau has write hazard tracking in its api , and is using the excl fence for it was well. As far as I can tell, this is all about fixing the lack of acknowledgement of the requirement for implicit fence tracking in amdgpu's (and its radeon predecessor) ABI. Which is fine so long as you get userspace to only use explicit fence passing to the compositor. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] dma-buf: add reservation object shared fence accessor
On Thu, Aug 09, 2018 at 01:37:09PM +0200, Christian König wrote:
> Add a helper to access the shared fences in an reservation object.
>
> Signed-off-by: Christian König
Reviewed-by: Huang Rui
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 7 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 3 +--
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++--
> drivers/gpu/drm/msm/msm_gem.c| 4 ++--
> drivers/gpu/drm/nouveau/nouveau_fence.c | 3 +--
> drivers/gpu/drm/radeon/radeon_sync.c | 3 +--
> drivers/gpu/drm/ttm/ttm_bo.c | 4 +---
> include/linux/reservation.h | 19 +++
> 8 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index fa38a960ce00..989932234160 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -238,9 +238,7 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct
> amdgpu_bo *bo,
> for (i = 0; i < shared_count; ++i) {
> struct dma_fence *f;
>
> - f = rcu_dereference_protected(fobj->shared[i],
> - reservation_object_held(resv));
> -
> + f = reservation_object_get_shared_fence(resv, fobj, i);
> if (ef) {
> if (f->context == ef->base.context) {
> dma_fence_put(f);
> @@ -273,8 +271,7 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct
> amdgpu_bo *bo,
> struct dma_fence *f;
> struct amdgpu_amdkfd_fence *efence;
>
> - f = rcu_dereference_protected(fobj->shared[i],
> - reservation_object_held(resv));
> + f = reservation_object_get_shared_fence(resv, fobj, i);
>
> efence = to_amdgpu_amdkfd_fence(f);
> if (efence) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> index 2d6f5ec77a68..dbfd62ab67e4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> @@ -212,8 +212,7 @@ int amdgpu_sync_resv(struct amdgpu_device *adev,
> return r;
>
> for (i = 0; i < flist->shared_count; ++i) {
> - f = rcu_dereference_protected(flist->shared[i],
> - reservation_object_held(resv));
> + f = reservation_object_get_shared_fence(resv, flist, i);
> /* We only want to trigger KFD eviction fences on
>* evict or move jobs. Skip KFD fences otherwise.
>*/
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index c6611cff64c8..22896a398eab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1482,8 +1482,8 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct
> ttm_buffer_object *bo,
> flist = reservation_object_get_list(bo->resv);
> if (flist) {
> for (i = 0; i < flist->shared_count; ++i) {
> - f = rcu_dereference_protected(flist->shared[i],
> - reservation_object_held(bo->resv));
> + f = reservation_object_get_shared_fence(bo->resv,
> + flist, i);
> if (amdkfd_fence_check_mm(f, current->mm))
> return false;
> }
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index f583bb4222f9..95d25dbfde2b 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -651,8 +651,8 @@ int msm_gem_sync_object(struct drm_gem_object *obj,
> return 0;
>
> for (i = 0; i < fobj->shared_count; i++) {
> - fence = rcu_dereference_protected(fobj->shared[i],
> -
> reservation_object_held(msm_obj->resv));
> + fence = reservation_object_get_shared_fence(msm_obj->resv,
> + fobj, i);
> if (fence->context != fctx->context) {
> ret = dma_fence_wait(fence, true);
> if (ret)
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
> b/drivers/gpu/drm/nouveau/nouveau_fence.c
> index 412d49bc6e56..3ce921c276c1 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> @@ -376,8 +376,7 @@ nouveau_fence_sync(struct nouveau_bo *nvbo, struct
> nouveau_channel *chan, bool e
> struct nouveau_channel *prev = NULL;
> bool must_wait = true;
>
> - fence =
[Intel-gfx] [RFC 2/8] drm/i915: Track all held rpm wakerefs
Everytime we take a wakeref, record the stack trace of where it was
taken; clearing the set if we ever drop back to no owners. For debugging
a rpm leak, we can look at all the current wakerefs and check if they
have a matching rpm_put.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/Kconfig.debug | 13 ++
drivers/gpu/drm/i915/i915_drv.c | 8 +-
drivers/gpu/drm/i915/i915_drv.h | 7 +
drivers/gpu/drm/i915/intel_drv.h| 32 ++--
drivers/gpu/drm/i915/intel_runtime_pm.c | 217 +---
5 files changed, 233 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig.debug
b/drivers/gpu/drm/i915/Kconfig.debug
index 459f8f88a34c..ed572a454e01 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -39,6 +39,19 @@ config DRM_I915_DEBUG
If in doubt, say "N".
+config DRM_I915_DEBUG_RPM
+bool "Insert extra checks into the runtime pm internals"
+depends on DRM_I915
+default n
+select STACKDEPOT
+help
+ Enable extra sanity checks (including BUGs) along the runtime pm
+ paths that may slow the system down and if hit hang the machine.
+
+ Recommended for driver developers only.
+
+ If in doubt, say "N".
+
config DRM_I915_DEBUG_GEM
bool "Insert extra checks into the GEM internals"
default n
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 62ef105a241d..b6e83043ac9d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -908,6 +908,7 @@ static int i915_driver_init_early(struct drm_i915_private
*dev_priv,
mutex_init(_priv->pps_mutex);
i915_memcpy_init_early(dev_priv);
+ intel_runtime_pm_init_early(dev_priv);
ret = i915_workqueues_init(dev_priv);
if (ret < 0)
@@ -1329,6 +1330,8 @@ static void i915_welcome_messages(struct drm_i915_private
*dev_priv)
DRM_INFO("DRM_I915_DEBUG enabled\n");
if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
DRM_INFO("DRM_I915_DEBUG_GEM enabled\n");
+ if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_RPM))
+ DRM_INFO("DRM_I915_DEBUG_RPM enabled\n");
}
/**
@@ -1477,7 +1480,7 @@ void i915_driver_unload(struct drm_device *dev)
intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
- WARN_ON(atomic_read(_priv->runtime_pm.wakeref_count));
+ intel_runtime_pm_cleanup(dev_priv);
}
static void i915_driver_release(struct drm_device *dev)
@@ -1685,6 +1688,7 @@ static int i915_drm_suspend_late(struct drm_device *dev,
bool hibernation)
out:
enable_rpm_wakeref_asserts(dev_priv);
+ intel_runtime_pm_cleanup(dev_priv);
return ret;
}
@@ -2644,7 +2648,7 @@ static int intel_runtime_suspend(struct device *kdev)
}
enable_rpm_wakeref_asserts(dev_priv);
- WARN_ON_ONCE(atomic_read(_priv->runtime_pm.wakeref_count));
+ intel_runtime_pm_cleanup(dev_priv);
if (intel_uncore_arm_unclaimed_mmio_detection(dev_priv))
DRM_ERROR("Unclaimed access detected prior to suspending\n");
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0b10a30b7d96..c8d5b896b155 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -45,6 +45,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1278,6 +1279,12 @@ struct i915_runtime_pm {
atomic_t wakeref_count;
bool suspended;
bool irqs_enabled;
+
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RPM)
+ spinlock_t debug_lock;
+ depot_stack_handle_t *debug_owners;
+ unsigned long debug_count;
+#endif
};
enum intel_pipe_crc_source {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dc6c0cec9b36..823db4ed47c8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1947,6 +1947,7 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp);
int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state);
/* intel_runtime_pm.c */
+void intel_runtime_pm_init_early(struct drm_i915_private *dev_priv);
int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool
resume);
@@ -1957,6 +1958,7 @@ void bxt_display_core_init(struct drm_i915_private
*dev_priv, bool resume);
void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
void intel_runtime_pm_disable(struct drm_i915_private *dev_priv);
+void intel_runtime_pm_cleanup(struct drm_i915_private *dev_priv);
const char *
intel_display_power_domain_str(enum intel_display_power_domain domain);
@@ -1974,23 +1976,23 @@ void icl_dbuf_slices_update(struct drm_i915_private
[Intel-gfx] [RFC 4/8] drm/i915: Syntatic sugar for using intel_runtime_pm
Frequently, we use intel_runtime_pm_get/_put around a small block.
Formalise that usage by providing a macro to define such a block with an
automatic closure to scope the intel_runtime_pm wakeref to that block,
i.e. macro abuse smelling of python.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_debugfs.c | 155 ++-
drivers/gpu/drm/i915/i915_gem.c | 10 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 23 ++--
drivers/gpu/drm/i915/i915_gem_shrinker.c | 44 +++
drivers/gpu/drm/i915/i915_pmu.c | 7 +-
drivers/gpu/drm/i915/i915_sysfs.c| 7 +-
drivers/gpu/drm/i915/intel_drv.h | 8 ++
drivers/gpu/drm/i915/intel_guc_log.c | 26 ++--
drivers/gpu/drm/i915/intel_huc.c | 7 +-
drivers/gpu/drm/i915/intel_panel.c | 18 +--
drivers/gpu/drm/i915/intel_uncore.c | 30 ++---
11 files changed, 160 insertions(+), 175 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index d70401f10f8c..529bdca2fd25 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -983,9 +983,9 @@ static int i915_gpu_info_open(struct inode *inode, struct
file *file)
struct i915_gpu_state *gpu;
intel_wakeref_t wakeref;
- wakeref = intel_runtime_pm_get(i915);
- gpu = i915_capture_gpu_state(i915);
- intel_runtime_pm_put(i915, wakeref);
+ gpu = NULL;
+ with_intel_runtime_pm(i915, wakeref)
+ gpu = i915_capture_gpu_state(i915);
if (!gpu)
return -ENOMEM;
@@ -1046,9 +1046,8 @@ i915_next_seqno_set(void *data, u64 val)
if (ret)
return ret;
- wakeref = intel_runtime_pm_get(dev_priv);
- ret = i915_gem_set_global_seqno(dev, val);
- intel_runtime_pm_put(dev_priv, wakeref);
+ with_intel_runtime_pm(dev_priv, wakeref)
+ ret = i915_gem_set_global_seqno(dev, val);
mutex_unlock(>struct_mutex);
@@ -1336,17 +1335,15 @@ static int i915_hangcheck_info(struct seq_file *m, void
*unused)
return 0;
}
- wakeref = intel_runtime_pm_get(dev_priv);
+ with_intel_runtime_pm(dev_priv, wakeref) {
+ for_each_engine(engine, dev_priv, id) {
+ acthd[id] = intel_engine_get_active_head(engine);
+ seqno[id] = intel_engine_get_seqno(engine);
+ }
- for_each_engine(engine, dev_priv, id) {
- acthd[id] = intel_engine_get_active_head(engine);
- seqno[id] = intel_engine_get_seqno(engine);
+ intel_engine_get_instdone(dev_priv->engine[RCS], );
}
- intel_engine_get_instdone(dev_priv->engine[RCS], );
-
- intel_runtime_pm_put(dev_priv, wakeref);
-
if (timer_pending(_priv->gpu_error.hangcheck_work.timer))
seq_printf(m, "Hangcheck active, timer fires in %dms\n",
jiffies_to_msecs(dev_priv->gpu_error.hangcheck_work.timer.expires -
@@ -1622,18 +1619,16 @@ static int i915_drpc_info(struct seq_file *m, void
*unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
intel_wakeref_t wakeref;
- int err;
+ int err = -ENODEV;
- wakeref = intel_runtime_pm_get(dev_priv);
-
- if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- err = vlv_drpc_info(m);
- else if (INTEL_GEN(dev_priv) >= 6)
- err = gen6_drpc_info(m);
- else
- err = ironlake_drpc_info(m);
-
- intel_runtime_pm_put(dev_priv, wakeref);
+ with_intel_runtime_pm(dev_priv, wakeref) {
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ err = vlv_drpc_info(m);
+ else if (INTEL_GEN(dev_priv) >= 6)
+ err = gen6_drpc_info(m);
+ else
+ err = ironlake_drpc_info(m);
+ }
return err;
}
@@ -2314,9 +2309,8 @@ static int i915_huc_load_status_info(struct seq_file *m,
void *data)
p = drm_seq_file_printer(m);
intel_uc_fw_dump(_priv->huc.fw, );
- wakeref = intel_runtime_pm_get(dev_priv);
- seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2));
- intel_runtime_pm_put(dev_priv, wakeref);
+ with_intel_runtime_pm(dev_priv, wakeref)
+ seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2));
return 0;
}
@@ -2326,7 +2320,6 @@ static int i915_guc_load_status_info(struct seq_file *m,
void *data)
struct drm_i915_private *dev_priv = node_to_i915(m->private);
intel_wakeref_t wakeref;
struct drm_printer p;
- u32 tmp, i;
if (!HAS_GUC(dev_priv))
return -ENODEV;
@@ -2334,22 +2327,23 @@ static int i915_guc_load_status_info(struct seq_file
*m, void *data)
p = drm_seq_file_printer(m);
[Intel-gfx] [RFC 6/8] drm/i915/dp: Markup pps lock power well
Track where and when we acquire and release the power well for pps
access along the dp aux link, with a view to detecting if we leak any
wakerefs.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_dp.c | 226 +---
1 file changed, 117 insertions(+), 109 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b4c22c0bc595..c7f75f96cc37 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -659,28 +659,34 @@ intel_dp_init_panel_power_sequencer_registers(struct
intel_dp *intel_dp,
static void
intel_dp_pps_init(struct intel_dp *intel_dp);
-static void pps_lock(struct intel_dp *intel_dp)
+static intel_wakeref_t pps_lock(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
+ intel_wakeref_t wakeref;
/*
* See intel_power_sequencer_reset() why we need
* a power domain reference here.
*/
- intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
+ wakeref = intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
mutex_lock(_priv->pps_mutex);
+
+ return wakeref;
}
-static void pps_unlock(struct intel_dp *intel_dp)
+static void pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t wakeref)
{
struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
mutex_unlock(_priv->pps_mutex);
- intel_display_power_put_unchecked(dev_priv, intel_dp->aux_power_domain);
+ intel_display_power_put(dev_priv, intel_dp->aux_power_domain, wakeref);
}
+#define with_pps_lock(dp, wf) \
+ for (wf = pps_lock(dp); wf; pps_unlock(dp, wf), wf = 0)
+
static void
vlv_power_sequencer_kick(struct intel_dp *intel_dp)
{
@@ -1026,33 +1032,33 @@ _pp_stat_reg(struct intel_dp *intel_dp)
static int edp_notify_handler(struct notifier_block *this, unsigned long code,
void *unused)
{
- struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
-edp_notifier);
+ struct intel_dp *intel_dp =
+ container_of(this, typeof(* intel_dp), edp_notifier);
struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
+ intel_wakeref_t wakeref;
if (!intel_dp_is_edp(intel_dp) || code != SYS_RESTART)
return 0;
- pps_lock(intel_dp);
-
- if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
- enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
- i915_reg_t pp_ctrl_reg, pp_div_reg;
- u32 pp_div;
-
- pp_ctrl_reg = PP_CONTROL(pipe);
- pp_div_reg = PP_DIVISOR(pipe);
- pp_div = I915_READ(pp_div_reg);
- pp_div &= PP_REFERENCE_DIVIDER_MASK;
-
- /* 0x1F write to PP_DIV_REG sets max cycle delay */
- I915_WRITE(pp_div_reg, pp_div | 0x1F);
- I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
- msleep(intel_dp->panel_power_cycle_delay);
+ with_pps_lock(intel_dp, wakeref) {
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+ i915_reg_t pp_ctrl_reg, pp_div_reg;
+ u32 pp_div;
+
+ pp_ctrl_reg = PP_CONTROL(pipe);
+ pp_div_reg = PP_DIVISOR(pipe);
+ pp_div = I915_READ(pp_div_reg);
+ pp_div &= PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg,
+ PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp->panel_power_cycle_delay);
+ }
}
- pps_unlock(intel_dp);
-
return 0;
}
@@ -1238,16 +1244,17 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
to_i915(intel_dig_port->base.base.dev);
i915_reg_t ch_ctl, ch_data[5];
uint32_t aux_clock_divider;
+ intel_wakeref_t wakeref;
int i, ret, recv_bytes;
- uint32_t status;
int try, clock = 0;
+ uint32_t status;
bool vdd;
ch_ctl = intel_dp->aux_ch_ctl_reg(intel_dp);
for (i = 0; i < ARRAY_SIZE(ch_data); i++)
ch_data[i] = intel_dp->aux_ch_data_reg(intel_dp, i);
- pps_lock(intel_dp);
+ wakeref = pps_lock(intel_dp);
/*
* We will be called with VDD already enabled for dpcd/edid/oui reads.
@@ -1391,7 +1398,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
if (vdd)
edp_panel_vdd_off(intel_dp, false);
- pps_unlock(intel_dp);
+
[Intel-gfx] [RFC 1/8] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
Currently, we cancel the extra wakeref we have for !runtime-pm devices
inside power_wells_fini_hw. However, this is not strictly paired with
the acquisition of that wakeref in runtime_pm_enable (as the fini_hw may
be called on errors paths before we even call runtime_pm_enable). Make
the symmetry more explicit and include a check that we do release all of
our rpm wakerefs.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.c | 8 ++--
drivers/gpu/drm/i915/intel_drv.h| 1 +
drivers/gpu/drm/i915/intel_runtime_pm.c | 24 +++-
3 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9dce55182c3a..62ef105a241d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1281,6 +1281,8 @@ static void i915_driver_register(struct drm_i915_private
*dev_priv)
*/
if (INTEL_INFO(dev_priv)->num_pipes)
drm_kms_helper_poll_init(dev);
+
+ intel_runtime_pm_enable(dev_priv);
}
/**
@@ -1289,6 +1291,8 @@ static void i915_driver_register(struct drm_i915_private
*dev_priv)
*/
static void i915_driver_unregister(struct drm_i915_private *dev_priv)
{
+ intel_runtime_pm_disable(dev_priv);
+
intel_fbdev_unregister(dev_priv);
intel_audio_deinit(dev_priv);
@@ -1408,8 +1412,6 @@ int i915_driver_load(struct pci_dev *pdev, const struct
pci_device_id *ent)
i915_driver_register(dev_priv);
- intel_runtime_pm_enable(dev_priv);
-
intel_init_ipc(dev_priv);
intel_runtime_pm_put(dev_priv);
@@ -1474,6 +1476,8 @@ void i915_driver_unload(struct drm_device *dev)
i915_driver_cleanup_mmio(dev_priv);
intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
+
+ WARN_ON(atomic_read(_priv->runtime_pm.wakeref_count));
}
static void i915_driver_release(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0601abb8c71f..dc6c0cec9b36 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1956,6 +1956,7 @@ void intel_power_domains_verify_state(struct
drm_i915_private *dev_priv);
void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume);
void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
+void intel_runtime_pm_disable(struct drm_i915_private *dev_priv);
const char *
intel_display_power_domain_str(enum intel_display_power_domain domain);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index e209edbc561d..b78c3b48aa62 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3793,8 +3793,6 @@ void intel_power_domains_init_hw(struct drm_i915_private
*dev_priv, bool resume)
*/
void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv)
{
- struct device *kdev = _priv->drm.pdev->dev;
-
/*
* The i915.ko module is still not prepared to be loaded when
* the power well is not enabled, so just enable it in case
@@ -3809,13 +3807,6 @@ void intel_power_domains_fini_hw(struct drm_i915_private
*dev_priv)
/* Remove the refcount we took to keep power well support disabled. */
if (!i915_modparams.disable_power_well)
intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
-
- /*
-* Remove the refcount we took in intel_runtime_pm_enable() in case
-* the platform doesn't support runtime PM.
-*/
- if (!HAS_RUNTIME_PM(dev_priv))
- pm_runtime_put(kdev);
}
/**
@@ -4074,3 +4065,18 @@ void intel_runtime_pm_enable(struct drm_i915_private
*dev_priv)
*/
pm_runtime_put_autosuspend(kdev);
}
+
+void intel_runtime_pm_disable(struct drm_i915_private *dev_priv)
+{
+ struct pci_dev *pdev = dev_priv->drm.pdev;
+ struct device *kdev = >dev;
+
+ pm_runtime_dont_use_autosuspend(kdev);
+
+ /*
+* Remove the refcount we took in intel_runtime_pm_enable() in case
+* the platform doesn't support runtime PM.
+*/
+ if (!HAS_RUNTIME_PM(dev_priv))
+ pm_runtime_put(kdev);
+}
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 8/8] drm/i915: Track crtc wakerefs during modesetting
Modesetting is one of the more complex interactions with power wells as
it acquires multiple wells to do its business. Fortunately, we do not
have to track every one individually as they are all called from the
same location and thus will have matching wakerefs (being a hash of its
stacktrace).
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_display.c | 26 ++
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6606e57f9265..9efbb930da9c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5953,7 +5953,8 @@ static u64 get_crtc_power_domains(struct drm_crtc *crtc,
static u64
modeset_get_crtc_power_domains(struct drm_crtc *crtc,
- struct intel_crtc_state *crtc_state)
+ struct intel_crtc_state *crtc_state,
+ intel_wakeref_t *wakeref)
{
struct drm_i915_private *dev_priv = to_i915(crtc->dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -5967,18 +5968,18 @@ modeset_get_crtc_power_domains(struct drm_crtc *crtc,
domains = new_domains & ~old_domains;
for_each_power_domain(domain, domains)
- intel_display_power_get(dev_priv, domain);
+ *wakeref = intel_display_power_get(dev_priv, domain);
return old_domains & ~new_domains;
}
static void modeset_put_power_domains(struct drm_i915_private *dev_priv,
- u64 domains)
+ u64 domains, intel_wakeref_t wakeref)
{
enum intel_display_power_domain domain;
for_each_power_domain(domain, domains)
- intel_display_power_put_unchecked(dev_priv, domain);
+ intel_display_power_put(dev_priv, domain, wakeref);
}
static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
@@ -12598,6 +12599,7 @@ static void intel_atomic_commit_tail(struct
drm_atomic_state *state)
struct intel_crtc_state *intel_cstate;
u64 put_domains[I915_MAX_PIPES] = {};
intel_wakeref_t wakeref = 0;
+ intel_wakeref_t crtc_wakeref = 0;
int i;
intel_atomic_commit_fence_wait(intel_state);
@@ -12615,7 +12617,8 @@ static void intel_atomic_commit_tail(struct
drm_atomic_state *state)
put_domains[to_intel_crtc(crtc)->pipe] =
modeset_get_crtc_power_domains(crtc,
- to_intel_crtc_state(new_crtc_state));
+ to_intel_crtc_state(new_crtc_state),
+ _wakeref);
}
if (!needs_modeset(new_crtc_state))
@@ -12725,7 +12728,9 @@ static void intel_atomic_commit_tail(struct
drm_atomic_state *state)
intel_post_plane_update(to_intel_crtc_state(old_crtc_state));
if (put_domains[i])
- modeset_put_power_domains(dev_priv, put_domains[i]);
+ modeset_put_power_domains(dev_priv,
+ put_domains[i],
+ crtc_wakeref);
intel_modeset_verify_crtc(crtc, state, old_crtc_state,
new_crtc_state);
}
@@ -15920,11 +15925,16 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
}
for_each_intel_crtc(dev, crtc) {
+ intel_wakeref_t wakeref;
u64 put_domains;
- put_domains = modeset_get_crtc_power_domains(>base,
crtc->config);
+ put_domains = modeset_get_crtc_power_domains(>base,
+crtc->config,
+);
if (WARN_ON(put_domains))
- modeset_put_power_domains(dev_priv, put_domains);
+ modeset_put_power_domains(dev_priv,
+ put_domains,
+ wakeref);
}
intel_display_set_init_power(dev_priv, false);
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 5/8] drm/i915: Markup paired operations on display power domains
The majority of runtime-pm operations are bounded and scoped within a
function; these are easy to verify that the wakeref are handled
correctly. We can employ the compiler to help us, and reduce the number
of wakerefs tracked when debugging, by passing around cookies provided
by the various rpm_get functions to their rpm_put counterpart. This
makes the pairing explicit, and given the required wakeref cookie the
compiler can verify that we pass an initialised value to the rpm_put
(quite handy for double checking error paths).
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_debugfs.c | 35 +++--
drivers/gpu/drm/i915/i915_drv.c | 5 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_gem.c | 4 +-
drivers/gpu/drm/i915/intel_audio.c | 3 +-
drivers/gpu/drm/i915/intel_cdclk.c | 10 ++--
drivers/gpu/drm/i915/intel_crt.c| 25 +
drivers/gpu/drm/i915/intel_csr.c| 17 --
drivers/gpu/drm/i915/intel_ddi.c| 36 -
drivers/gpu/drm/i915/intel_display.c| 61 ++
drivers/gpu/drm/i915/intel_dp.c | 29 ++-
drivers/gpu/drm/i915/intel_dpll_mgr.c | 66 +++
drivers/gpu/drm/i915/intel_drv.h| 17 --
drivers/gpu/drm/i915/intel_hdmi.c | 20 ---
drivers/gpu/drm/i915/intel_i2c.c| 20 +++
drivers/gpu/drm/i915/intel_lvds.c | 8 +--
drivers/gpu/drm/i915/intel_pipe_crc.c | 6 ++-
drivers/gpu/drm/i915/intel_pm.c | 7 ++-
drivers/gpu/drm/i915/intel_runtime_pm.c | 69 -
drivers/gpu/drm/i915/intel_sprite.c | 24 ++---
drivers/gpu/drm/i915/vlv_dsi.c | 8 +--
21 files changed, 303 insertions(+), 169 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 529bdca2fd25..06e49a423599 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -656,10 +656,12 @@ static void gen8_display_interrupt_info(struct seq_file
*m)
for_each_pipe(dev_priv, pipe) {
enum intel_display_power_domain power_domain;
+ intel_wakeref_t wakeref;
power_domain = POWER_DOMAIN_PIPE(pipe);
- if (!intel_display_power_get_if_enabled(dev_priv,
- power_domain)) {
+ wakeref = intel_display_power_get_if_enabled(dev_priv,
+ power_domain);
+ if (!wakeref) {
seq_printf(m, "Pipe %c power disabled\n",
pipe_name(pipe));
continue;
@@ -674,7 +676,7 @@ static void gen8_display_interrupt_info(struct seq_file *m)
pipe_name(pipe),
I915_READ(GEN8_DE_PIPE_IER(pipe)));
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, power_domain, wakeref);
}
seq_printf(m, "Display Engine port interrupt mask:\t%08x\n",
@@ -710,6 +712,8 @@ static int i915_interrupt_info(struct seq_file *m, void
*data)
wakeref = intel_runtime_pm_get(dev_priv);
if (IS_CHERRYVIEW(dev_priv)) {
+ intel_wakeref_t pref;
+
seq_printf(m, "Master Interrupt Control:\t%08x\n",
I915_READ(GEN8_MASTER_IRQ));
@@ -725,8 +729,9 @@ static int i915_interrupt_info(struct seq_file *m, void
*data)
enum intel_display_power_domain power_domain;
power_domain = POWER_DOMAIN_PIPE(pipe);
- if (!intel_display_power_get_if_enabled(dev_priv,
- power_domain)) {
+ pref = intel_display_power_get_if_enabled(dev_priv,
+ power_domain);
+ if (!pref) {
seq_printf(m, "Pipe %c power disabled\n",
pipe_name(pipe));
continue;
@@ -736,17 +741,17 @@ static int i915_interrupt_info(struct seq_file *m, void
*data)
pipe_name(pipe),
I915_READ(PIPESTAT(pipe)));
- intel_display_power_put(dev_priv, power_domain);
+ intel_display_power_put(dev_priv, power_domain, pref);
}
- intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
+ pref = intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
seq_printf(m, "Port hotplug:\t%08x\n",
I915_READ(PORT_HOTPLUG_EN));
seq_printf(m, "DPFLIPSTAT:\t%08x\n",
I915_READ(VLV_DPFLIPSTAT));
[Intel-gfx] [RFC 7/8] drm/i915: Complain is hsw_get_pipe_config acquires the same power well twice
As we only release each power well once, we assume that each transcoder
maps to a different domain. Complain if this is not so.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_display.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 93d4b569c3a0..6606e57f9265 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9388,6 +9388,8 @@ static bool hsw_get_transcoder_state(struct intel_crtc
*crtc,
power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
return false;
+
+ WARN_ON(*power_domain_mask & BIT_ULL(power_domain));
*power_domain_mask |= BIT_ULL(power_domain);
tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
@@ -9415,6 +9417,8 @@ static bool bxt_get_dsi_transcoder_state(struct
intel_crtc *crtc,
power_domain = POWER_DOMAIN_TRANSCODER(cpu_transcoder);
if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
continue;
+
+ WARN_ON(*power_domain_mask & BIT_ULL(power_domain));
*power_domain_mask |= BIT_ULL(power_domain);
/*
@@ -9546,7 +9550,9 @@ static bool haswell_get_pipe_config(struct intel_crtc
*crtc,
power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
+ WARN_ON(power_domain_mask & BIT_ULL(power_domain));
power_domain_mask |= BIT_ULL(power_domain);
+
if (INTEL_GEN(dev_priv) >= 9)
skylake_get_pfit_config(crtc, pipe_config);
else
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 3/8] drm/i915: Markup paired operations on wakerefs
The majority of runtime-pm operations are bounded and scoped within a function; these are easy to verify that the wakeref are handled correctly. We can employ the compiler to help us, and reduce the number of wakerefs tracked when debugging, by passing around cookies provided by the various rpm_get functions to their rpm_put counterpart. This makes the pairing explicit, and given the required wakeref cookie the compiler can verify that we pass an initialised value to the rpm_put (quite handy for double checking error paths). For regular builds, the compiler should be able to eliminate the unused local variables and the program growth should be minimal. Fwiw, it came out as a net improvement as gcc was able to refactor rpm_get and rpm_get_if_in_use together, add/remove: 1/1 grow/shrink: 20/9 up/down: 191/-268 (-77) Function old new delta intel_runtime_pm_put_unchecked - 136+136 i915_gem_unpark 396 406 +10 intel_runtime_pm_get 135 141 +6 intel_runtime_pm_get_noresume136 141 +5 i915_perf_open_ioctl43754379 +4 i915_gpu_busy 72 76 +4 i915_gem_idle_work_handler 954 958 +4 capture 68146818 +4 mock_gem_device 14331436 +3 __execlists_submission_tasklet 25732576 +3 i915_sample 756 758 +2 intel_guc_submission_disable 364 365 +1 igt_mmap_offset_exhaustion 10351036 +1 i915_runtime_pm_status 257 258 +1 i915_rps_boost_info 13581359 +1 i915_hangcheck_info 12291230 +1 i915_gem_switch_to_kernel_context682 683 +1 i915_gem_suspend 410 411 +1 i915_gem_resume 254 255 +1 i915_gem_park190 191 +1 i915_engine_info 279 280 +1 intel_rps_mark_interactive 194 193 -1 i915_hangcheck_elapsed 15261525 -1 i915_gem_wait_for_idle 298 297 -1 i915_drop_caches_set 555 554 -1 execlists_submission_tasklet 126 125 -1 aliasing_gtt_bind_vma235 234 -1 i915_gem_retire_work_handler 144 142 -2 igt_evict_contexts.part 916 910 -6 intel_runtime_pm_get_if_in_use 141 23-118 intel_runtime_pm_put 136 --136 Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/gvt/aperture_gm.c| 8 +- drivers/gpu/drm/i915/gvt/gvt.h| 2 +- drivers/gpu/drm/i915/gvt/mmio_context.c | 2 +- drivers/gpu/drm/i915/gvt/scheduler.c | 4 +- drivers/gpu/drm/i915/i915_debugfs.c | 127 +++--- drivers/gpu/drm/i915/i915_drv.c | 7 +- drivers/gpu/drm/i915/i915_drv.h | 6 +- drivers/gpu/drm/i915/i915_gem.c | 57 +--- drivers/gpu/drm/i915/i915_gem_execbuffer.c| 5 +- drivers/gpu/drm/i915/i915_gem_fence_reg.c | 6 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 22 +-- drivers/gpu/drm/i915/i915_gem_shrinker.c | 27 ++-- drivers/gpu/drm/i915/i915_irq.c | 5 +- drivers/gpu/drm/i915/i915_perf.c | 10 +- drivers/gpu/drm/i915/i915_pmu.c | 26 ++-- drivers/gpu/drm/i915/i915_sysfs.c | 24 ++-- drivers/gpu/drm/i915/intel_display.c | 5 +- drivers/gpu/drm/i915/intel_drv.h | 15 ++- drivers/gpu/drm/i915/intel_engine_cs.c| 12 +- drivers/gpu/drm/i915/intel_fbdev.c| 7 +- drivers/gpu/drm/i915/intel_guc_log.c | 15 ++- drivers/gpu/drm/i915/intel_hotplug.c | 5 +- drivers/gpu/drm/i915/intel_huc.c | 5 +- drivers/gpu/drm/i915/intel_panel.c| 5 +- drivers/gpu/drm/i915/intel_pm.c | 2 +- drivers/gpu/drm/i915/intel_runtime_pm.c | 85 +--- drivers/gpu/drm/i915/intel_uncore.c | 5 +- drivers/gpu/drm/i915/selftests/huge_pages.c | 5 +- .../gpu/drm/i915/selftests/i915_gem_context.c | 12 +- .../gpu/drm/i915/selftests/i915_gem_evict.c | 11 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 10 +- .../gpu/drm/i915/selftests/i915_gem_object.c | 18 ++- .../gpu/drm/i915/selftests/intel_hangcheck.c | 5 +- 33 files changed, 362 insertions(+), 198 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/aperture_gm.c b/drivers/gpu/drm/i915/gvt/aperture_gm.c index
[Intel-gfx] [PATCH i-g-t 1/2] igt/pm_rpm: Test incomplete(debug) suspends vs rpm
Check that we restore runtime pm around debug suspends and hibernates.
v2: Differentiate between external test setup failure and one of
interest
Signed-off-by: Chris Wilson
---
tests/pm_rpm.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index 4268bb19a..1fbdda4ed 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -707,8 +707,10 @@ static void setup_environment(void)
igt_info("Runtime PM support: %d\n", has_runtime_pm);
igt_info("PC8 residency support: %d\n", has_pc8);
-
igt_require(has_runtime_pm);
+
+ disable_all_screens(_data);
+ igt_require(wait_for_suspended());
}
static void restore_environment(void)
@@ -1372,10 +1374,11 @@ static void __attribute__((noreturn)) stay_subtest(void)
sleep(600);
}
-static void system_suspend_subtest(void)
+static void system_suspend_subtest(int state, int debug)
{
disable_all_screens_and_wait(_data);
- igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
+
+ igt_system_suspend_autoresume(state, debug);
igt_assert(wait_for_suspended());
}
@@ -1992,12 +1995,19 @@ int main(int argc, char *argv[])
WAIT_STATUS | WAIT_EXTRA);
/* System suspend */
+ igt_subtest("system-suspend-devices")
+ system_suspend_subtest(SUSPEND_STATE_MEM, SUSPEND_TEST_DEVICES);
igt_subtest("system-suspend")
- system_suspend_subtest();
+ system_suspend_subtest(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
igt_subtest("system-suspend-execbuf")
system_suspend_execbuf_subtest();
igt_subtest("system-suspend-modeset")
system_suspend_modeset_subtest();
+ igt_subtest("system-hibernate-devices")
+ system_suspend_subtest(SUSPEND_STATE_DISK,
+ SUSPEND_TEST_DEVICES);
+ igt_subtest("system-hibernate")
+ system_suspend_subtest(SUSPEND_STATE_DISK, SUSPEND_TEST_NONE);
/* GEM stress */
igt_subtest("gem-execbuf-stress")
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/2] igt/pm_rpm: Test reaquisition of runtime-pm after module reload
It doesn't work right now and desperately needs to be fixed...
Signed-off-by: Chris Wilson
---
tests/intel-ci/fast-feedback.testlist | 1 +
tests/pm_rpm.c| 28 ---
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/tests/intel-ci/fast-feedback.testlist
b/tests/intel-ci/fast-feedback.testlist
index 1f3b95357..c625904d5 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -272,3 +272,4 @@ igt@vgem_basic@unload
igt@drv_module_reload@basic-reload
igt@drv_module_reload@basic-no-display
igt@drv_module_reload@basic-reload-inject
+igt@pm_rpm@module-reload
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index 1fbdda4ed..79cdf969a 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -692,7 +692,7 @@ static void setup_pc8(void)
has_pc8 = true;
}
-static void setup_environment(void)
+static bool setup_environment(void)
{
drm_fd = drm_open_driver_master(DRIVER_INTEL);
debugfs = igt_debugfs_dir(drm_fd);
@@ -710,7 +710,7 @@ static void setup_environment(void)
igt_require(has_runtime_pm);
disable_all_screens(_data);
- igt_require(wait_for_suspended());
+ return wait_for_suspended();
}
static void restore_environment(void)
@@ -1905,7 +1905,7 @@ int main(int argc, char *argv[])
* PC8+. We don't want bug reports from cases where the machine is just
* not properly configured. */
igt_fixture
- setup_environment();
+ igt_require(setup_environment());
if (stay)
igt_subtest("stay")
@@ -2026,5 +2026,27 @@ int main(int argc, char *argv[])
igt_fixture
teardown_environment();
+ igt_subtest("module-reload") {
+ igt_debug("Reload w/o display\n");
+ igt_i915_driver_unload();
+ igt_assert_eq(igt_i915_driver_load("disable_display=1"), 0);
+
+ igt_assert(setup_environment());
+ basic_subtest();
+ drm_resources_equal_subtest();
+ pci_d3_state_subtest();
+ teardown_environment();
+
+ igt_debug("Reload as normal\n");
+ igt_i915_driver_unload();
+ igt_assert_eq(igt_i915_driver_load(NULL), 0);
+
+ igt_assert(setup_environment());
+ basic_subtest();
+ drm_resources_equal_subtest();
+ pci_d3_state_subtest();
+ teardown_environment();
+ }
+
igt_exit();
}
--
2.18.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] dma-buf: Remove requirement for ops->map() from dma_buf_export
On Tue, Aug 07, 2018 at 07:36:47PM +0100, Chris Wilson wrote:
> Since commit 9ea0dfbf972 ("dma-buf: make map_atomic and map function
> pointers optional"), the core provides the no-op functions when map and
> map_atomic are not provided, so we no longer need assert that are
> supplied by a dma-buf exporter.
Pushed to drm-misc-next.
thanks,
Gerd
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
