* [PATCH 02/15] drm/i915/selftests: Lock the drm_mm while modifying
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 9:50 ` Matthew Auld
2019-07-03 9:17 ` [PATCH 03/15] drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine Chris Wilson
` (17 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
Remember to lock the drm_mm as we modify it, lest it be modified in the
background by retire/free workers!
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index a1f0b235f56b..9b05bef15023 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -414,7 +414,9 @@ static int igt_mmap_offset_exhaustion(void *arg)
drm_mm_for_each_hole(hole, mm, hole_start, hole_end) {
resv.start = hole_start;
resv.size = hole_end - hole_start - 1; /* PAGE_SIZE units */
+ mutex_lock(&i915->drm.struct_mutex);
err = drm_mm_reserve_node(mm, &resv);
+ mutex_unlock(&i915->drm.struct_mutex);
if (err) {
pr_err("Failed to trim VMA manager, err=%d\n", err);
goto out_park;
@@ -478,7 +480,9 @@ static int igt_mmap_offset_exhaustion(void *arg)
}
out:
+ mutex_lock(&i915->drm.struct_mutex);
drm_mm_remove_node(&resv);
+ mutex_unlock(&i915->drm.struct_mutex);
out_park:
restore_retire_worker(i915);
return err;
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 03/15] drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
2019-07-03 9:17 ` [PATCH 02/15] drm/i915/selftests: Lock the drm_mm while modifying Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 11:23 ` Tvrtko Ursulin
2019-07-03 9:17 ` [PATCH 04/15] drm/i915/gt: Assume we hold forcewake for execlists resume Chris Wilson
` (16 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
During post-reset resume, we call intel_mocs_init_engine to reinitialise
the MOCS registers. Suprisingly, especially when enhanced by lockdep,
the acquisition of the forcewake lock around each register write takes a
substantial portion of the reset time. We don't need to use the
individual forcewake here as we can assume that the caller is holding a
blanket forcewake for the reset&resume and the resume is serialised.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
index ae6cbf0d517c..290a5e9b90b9 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -346,6 +346,9 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
unsigned int index;
u32 unused_value;
+ /* Called under a blanket forcewake */
+ assert_forcewakes_active(uncore, FORCEWAKE_ALL);
+
if (!get_mocs_settings(gt, &table))
return;
@@ -355,16 +358,16 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
for (index = 0; index < table.size; index++) {
u32 value = get_entry_control(&table, index);
- intel_uncore_write(uncore,
- mocs_register(engine->id, index),
- value);
+ intel_uncore_write_fw(uncore,
+ mocs_register(engine->id, index),
+ value);
}
/* All remaining entries are also unused */
for (; index < table.n_entries; index++)
- intel_uncore_write(uncore,
- mocs_register(engine->id, index),
- unused_value);
+ intel_uncore_write_fw(uncore,
+ mocs_register(engine->id, index),
+ unused_value);
}
/**
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 03/15] drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine
2019-07-03 9:17 ` [PATCH 03/15] drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine Chris Wilson
@ 2019-07-03 11:23 ` Tvrtko Ursulin
2019-07-03 11:47 ` Chris Wilson
0 siblings, 1 reply; 31+ messages in thread
From: Tvrtko Ursulin @ 2019-07-03 11:23 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: matthew.auld
On 03/07/2019 10:17, Chris Wilson wrote:
> During post-reset resume, we call intel_mocs_init_engine to reinitialise
> the MOCS registers. Suprisingly, especially when enhanced by lockdep,
> the acquisition of the forcewake lock around each register write takes a
> substantial portion of the reset time. We don't need to use the
> individual forcewake here as we can assume that the caller is holding a
> blanket forcewake for the reset&resume and the resume is serialised.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index ae6cbf0d517c..290a5e9b90b9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -346,6 +346,9 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
> unsigned int index;
> u32 unused_value;
>
> + /* Called under a blanket forcewake */
> + assert_forcewakes_active(uncore, FORCEWAKE_ALL);
> +
Assert is strictly speaking a bit weak since forcewake status can
theoretically change until the actual access below. But in our current
code we indeed hold it for the whole reset.
I don't have any actual ideas on how to fundamentally improve the
assert. Thought to have it after the writes is the only thing which came
to mind. Thoughts?
Regards,
Tvrtko
> if (!get_mocs_settings(gt, &table))
> return;
>
> @@ -355,16 +358,16 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
> for (index = 0; index < table.size; index++) {
> u32 value = get_entry_control(&table, index);
>
> - intel_uncore_write(uncore,
> - mocs_register(engine->id, index),
> - value);
> + intel_uncore_write_fw(uncore,
> + mocs_register(engine->id, index),
> + value);
> }
>
> /* All remaining entries are also unused */
> for (; index < table.n_entries; index++)
> - intel_uncore_write(uncore,
> - mocs_register(engine->id, index),
> - unused_value);
> + intel_uncore_write_fw(uncore,
> + mocs_register(engine->id, index),
> + unused_value);
> }
>
> /**
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 03/15] drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine
2019-07-03 11:23 ` Tvrtko Ursulin
@ 2019-07-03 11:47 ` Chris Wilson
2019-07-03 12:01 ` Tvrtko Ursulin
0 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 11:47 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: matthew.auld
Quoting Tvrtko Ursulin (2019-07-03 12:23:52)
>
> On 03/07/2019 10:17, Chris Wilson wrote:
> > During post-reset resume, we call intel_mocs_init_engine to reinitialise
> > the MOCS registers. Suprisingly, especially when enhanced by lockdep,
> > the acquisition of the forcewake lock around each register write takes a
> > substantial portion of the reset time. We don't need to use the
> > individual forcewake here as we can assume that the caller is holding a
> > blanket forcewake for the reset&resume and the resume is serialised.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
> > index ae6cbf0d517c..290a5e9b90b9 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> > @@ -346,6 +346,9 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
> > unsigned int index;
> > u32 unused_value;
> >
> > + /* Called under a blanket forcewake */
> > + assert_forcewakes_active(uncore, FORCEWAKE_ALL);
> > +
>
> Assert is strictly speaking a bit weak since forcewake status can
> theoretically change until the actual access below. But in our current
> code we indeed hold it for the whole reset.
You want to distinguish between an explicit hold by the caller and the
auto.
> I don't have any actual ideas on how to fundamentally improve the
> assert. Thought to have it after the writes is the only thing which came
> to mind. Thoughts?
I definitely prefer having it upfront to document the function
preconditions, and so would prefer if the assert actually did assert an
explicit forcewake as it is meant to do :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 03/15] drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine
2019-07-03 11:47 ` Chris Wilson
@ 2019-07-03 12:01 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2019-07-03 12:01 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: matthew.auld
On 03/07/2019 12:47, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-07-03 12:23:52)
>>
>> On 03/07/2019 10:17, Chris Wilson wrote:
>>> During post-reset resume, we call intel_mocs_init_engine to reinitialise
>>> the MOCS registers. Suprisingly, especially when enhanced by lockdep,
>>> the acquisition of the forcewake lock around each register write takes a
>>> substantial portion of the reset time. We don't need to use the
>>> individual forcewake here as we can assume that the caller is holding a
>>> blanket forcewake for the reset&resume and the resume is serialised.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>> drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++------
>>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
>>> index ae6cbf0d517c..290a5e9b90b9 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
>>> @@ -346,6 +346,9 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
>>> unsigned int index;
>>> u32 unused_value;
>>>
>>> + /* Called under a blanket forcewake */
>>> + assert_forcewakes_active(uncore, FORCEWAKE_ALL);
>>> +
>>
>> Assert is strictly speaking a bit weak since forcewake status can
>> theoretically change until the actual access below. But in our current
>> code we indeed hold it for the whole reset.
>
> You want to distinguish between an explicit hold by the caller and the
> auto.
Yes, we just don't have a way of distinguishing if the caller is holding
or they were held and about to be released.
>> I don't have any actual ideas on how to fundamentally improve the
>> assert. Thought to have it after the writes is the only thing which came
>> to mind. Thoughts?
>
> I definitely prefer having it upfront to document the function
> preconditions, and so would prefer if the assert actually did assert an
> explicit forcewake as it is meant to do :)
Probably better yes. I was just thinking out loud.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 04/15] drm/i915/gt: Assume we hold forcewake for execlists resume
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
2019-07-03 9:17 ` [PATCH 02/15] drm/i915/selftests: Lock the drm_mm while modifying Chris Wilson
2019-07-03 9:17 ` [PATCH 03/15] drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 12:03 ` Tvrtko Ursulin
2019-07-03 9:17 ` [PATCH 05/15] drm/i915/gt: Ignore forcewake acquisition for posting_reads Chris Wilson
` (15 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
We can assume the caller is holding a blanket forcewake for the
register writes during resume, and so we can skip taking individual
locks around each write inside execlists resume.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 953b3938a85f..497ca52381a7 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2076,22 +2076,23 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
static void enable_execlists(struct intel_engine_cs *engine)
{
+ u32 mode;
+
+ assert_forcewakes_active(engine->uncore, FORCEWAKE_ALL);
+
intel_engine_set_hwsp_writemask(engine, ~0u); /* HWSTAM */
if (INTEL_GEN(engine->i915) >= 11)
- ENGINE_WRITE(engine,
- RING_MODE_GEN7,
- _MASKED_BIT_ENABLE(GEN11_GFX_DISABLE_LEGACY_MODE));
+ mode = _MASKED_BIT_ENABLE(GEN11_GFX_DISABLE_LEGACY_MODE);
else
- ENGINE_WRITE(engine,
- RING_MODE_GEN7,
- _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE));
+ mode = _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE);
+ ENGINE_WRITE_FW(engine, RING_MODE_GEN7, mode);
- ENGINE_WRITE(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING));
+ ENGINE_WRITE_FW(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING));
- ENGINE_WRITE(engine,
- RING_HWS_PGA,
- i915_ggtt_offset(engine->status_page.vma));
+ ENGINE_WRITE_FW(engine,
+ RING_HWS_PGA,
+ i915_ggtt_offset(engine->status_page.vma));
ENGINE_POSTING_READ(engine, RING_HWS_PGA);
}
@@ -2099,7 +2100,7 @@ static bool unexpected_starting_state(struct intel_engine_cs *engine)
{
bool unexpected = false;
- if (ENGINE_READ(engine, RING_MI_MODE) & STOP_RING) {
+ if (ENGINE_READ_FW(engine, RING_MI_MODE) & STOP_RING) {
DRM_DEBUG_DRIVER("STOP_RING still set in RING_MI_MODE\n");
unexpected = true;
}
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 04/15] drm/i915/gt: Assume we hold forcewake for execlists resume
2019-07-03 9:17 ` [PATCH 04/15] drm/i915/gt: Assume we hold forcewake for execlists resume Chris Wilson
@ 2019-07-03 12:03 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2019-07-03 12:03 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: matthew.auld
On 03/07/2019 10:17, Chris Wilson wrote:
> We can assume the caller is holding a blanket forcewake for the
> register writes during resume, and so we can skip taking individual
> locks around each write inside execlists resume.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 953b3938a85f..497ca52381a7 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2076,22 +2076,23 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
>
> static void enable_execlists(struct intel_engine_cs *engine)
> {
> + u32 mode;
> +
> + assert_forcewakes_active(engine->uncore, FORCEWAKE_ALL);
> +
> intel_engine_set_hwsp_writemask(engine, ~0u); /* HWSTAM */
>
> if (INTEL_GEN(engine->i915) >= 11)
> - ENGINE_WRITE(engine,
> - RING_MODE_GEN7,
> - _MASKED_BIT_ENABLE(GEN11_GFX_DISABLE_LEGACY_MODE));
> + mode = _MASKED_BIT_ENABLE(GEN11_GFX_DISABLE_LEGACY_MODE);
> else
> - ENGINE_WRITE(engine,
> - RING_MODE_GEN7,
> - _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE));
> + mode = _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE);
> + ENGINE_WRITE_FW(engine, RING_MODE_GEN7, mode);
>
> - ENGINE_WRITE(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING));
> + ENGINE_WRITE_FW(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING));
>
> - ENGINE_WRITE(engine,
> - RING_HWS_PGA,
> - i915_ggtt_offset(engine->status_page.vma));
> + ENGINE_WRITE_FW(engine,
> + RING_HWS_PGA,
> + i915_ggtt_offset(engine->status_page.vma));
> ENGINE_POSTING_READ(engine, RING_HWS_PGA);
> }
>
> @@ -2099,7 +2100,7 @@ static bool unexpected_starting_state(struct intel_engine_cs *engine)
> {
> bool unexpected = false;
>
> - if (ENGINE_READ(engine, RING_MI_MODE) & STOP_RING) {
> + if (ENGINE_READ_FW(engine, RING_MI_MODE) & STOP_RING) {
> DRM_DEBUG_DRIVER("STOP_RING still set in RING_MI_MODE\n");
> unexpected = true;
> }
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 05/15] drm/i915/gt: Ignore forcewake acquisition for posting_reads
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (2 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 04/15] drm/i915/gt: Assume we hold forcewake for execlists resume Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 11:26 ` Tvrtko Ursulin
2019-07-03 9:17 ` [PATCH 06/15] drm/i915/gem: Free pages before rcu-freeing the object Chris Wilson
` (14 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
We don't care about the result of the read, so it may be garbage, we
only care that the mmio is flushed. As such, we can forgo using an
individual forcewake and lock around any posting-read for an engine.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_engine.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index 557b08b13feb..0331e9ac2485 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -51,7 +51,7 @@ struct drm_printer;
#define ENGINE_READ16(...) __ENGINE_READ_OP(read16, __VA_ARGS__)
#define ENGINE_READ(...) __ENGINE_READ_OP(read, __VA_ARGS__)
#define ENGINE_READ_FW(...) __ENGINE_READ_OP(read_fw, __VA_ARGS__)
-#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read, __VA_ARGS__)
+#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read_fw, __VA_ARGS__)
#define ENGINE_POSTING_READ16(...) __ENGINE_READ_OP(posting_read16, __VA_ARGS__)
#define ENGINE_READ64(engine__, lower_reg__, upper_reg__) \
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 05/15] drm/i915/gt: Ignore forcewake acquisition for posting_reads
2019-07-03 9:17 ` [PATCH 05/15] drm/i915/gt: Ignore forcewake acquisition for posting_reads Chris Wilson
@ 2019-07-03 11:26 ` Tvrtko Ursulin
2019-07-03 11:44 ` Chris Wilson
0 siblings, 1 reply; 31+ messages in thread
From: Tvrtko Ursulin @ 2019-07-03 11:26 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: matthew.auld
On 03/07/2019 10:17, Chris Wilson wrote:
> We don't care about the result of the read, so it may be garbage, we
> only care that the mmio is flushed. As such, we can forgo using an
> individual forcewake and lock around any posting-read for an engine.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/gt/intel_engine.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index 557b08b13feb..0331e9ac2485 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -51,7 +51,7 @@ struct drm_printer;
> #define ENGINE_READ16(...) __ENGINE_READ_OP(read16, __VA_ARGS__)
> #define ENGINE_READ(...) __ENGINE_READ_OP(read, __VA_ARGS__)
> #define ENGINE_READ_FW(...) __ENGINE_READ_OP(read_fw, __VA_ARGS__)
> -#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read, __VA_ARGS__)
> +#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read_fw, __VA_ARGS__)
> #define ENGINE_POSTING_READ16(...) __ENGINE_READ_OP(posting_read16, __VA_ARGS__)
>
> #define ENGINE_READ64(engine__, lower_reg__, upper_reg__) \
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Can we apply this to all posting reads? (intel_uncore_posting_read*)
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 05/15] drm/i915/gt: Ignore forcewake acquisition for posting_reads
2019-07-03 11:26 ` Tvrtko Ursulin
@ 2019-07-03 11:44 ` Chris Wilson
0 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 11:44 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: matthew.auld
Quoting Tvrtko Ursulin (2019-07-03 12:26:36)
>
> On 03/07/2019 10:17, Chris Wilson wrote:
> > We don't care about the result of the read, so it may be garbage, we
> > only care that the mmio is flushed. As such, we can forgo using an
> > individual forcewake and lock around any posting-read for an engine.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/gt/intel_engine.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> > index 557b08b13feb..0331e9ac2485 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> > @@ -51,7 +51,7 @@ struct drm_printer;
> > #define ENGINE_READ16(...) __ENGINE_READ_OP(read16, __VA_ARGS__)
> > #define ENGINE_READ(...) __ENGINE_READ_OP(read, __VA_ARGS__)
> > #define ENGINE_READ_FW(...) __ENGINE_READ_OP(read_fw, __VA_ARGS__)
> > -#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read, __VA_ARGS__)
> > +#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read_fw, __VA_ARGS__)
> > #define ENGINE_POSTING_READ16(...) __ENGINE_READ_OP(posting_read16, __VA_ARGS__)
> >
> > #define ENGINE_READ64(engine__, lower_reg__, upper_reg__) \
> >
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Can we apply this to all posting reads? (intel_uncore_posting_read*)
I briefly considered it, but was too lazy to think beyond the current
set. What gave me pause for concern was intel_gt_flush_ggtt_writes()
where we have to worry about overlapping mmio access causing gen7 to
explode, generalising the lock drop is tricky.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 06/15] drm/i915/gem: Free pages before rcu-freeing the object
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (3 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 05/15] drm/i915/gt: Ignore forcewake acquisition for posting_reads Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 10:37 ` Matthew Auld
2019-07-03 9:17 ` [PATCH 07/15] drm/i915: Markup potential lock for i915_active Chris Wilson
` (13 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
As we have dropped the final reference to the object, we do not need to
wait until after the rcu grace period to drop its pages. We still require
struct_mutex to completely unbind the object to release the pages, so we
still need a free-worker to manage that from process context. By
scheduling the release of pages before waiting for the rcu should mean
that we are not trapping those pages from beyond the reach of the
shrinker.
v2: Pass along the request to skip if the vma is busy to the underlying
unbind routine, to avoid checking the reservation underneath the
i915->mm.obj_lock which may be used from inside irq context.
v3: Flip the bit for unbinding while active, for later convenience.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111035
Fixes: a93615f900bd ("drm/i915: Throw away the active object retirement complexity")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 82 +++++++++-----------
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 18 +++--
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 +-
drivers/gpu/drm/i915/i915_drv.h | 15 ++--
drivers/gpu/drm/i915/i915_gem.c | 8 +-
6 files changed, 67 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 43194fbcbc2e..d3e96f09c6b7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -146,6 +146,18 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
}
}
+static void __i915_gem_free_object_rcu(struct rcu_head *head)
+{
+ struct drm_i915_gem_object *obj =
+ container_of(head, typeof(*obj), rcu);
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
+
+ i915_gem_object_free(obj);
+
+ GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
+ atomic_dec(&i915->mm.free_count);
+}
+
static void __i915_gem_free_objects(struct drm_i915_private *i915,
struct llist_node *freed)
{
@@ -168,22 +180,6 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
GEM_BUG_ON(!list_empty(&obj->vma.list));
GEM_BUG_ON(!RB_EMPTY_ROOT(&obj->vma.tree));
- /*
- * This serializes freeing with the shrinker. Since the free
- * is delayed, first by RCU then by the workqueue, we want the
- * shrinker to be able to free pages of unreferenced objects,
- * or else we may oom whilst there are plenty of deferred
- * freed objects.
- */
- if (i915_gem_object_has_pages(obj) &&
- i915_gem_object_is_shrinkable(obj)) {
- unsigned long flags;
-
- spin_lock_irqsave(&i915->mm.obj_lock, flags);
- list_del_init(&obj->mm.link);
- spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
- }
-
mutex_unlock(&i915->drm.struct_mutex);
GEM_BUG_ON(atomic_read(&obj->bind_count));
@@ -197,19 +193,15 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
atomic_set(&obj->mm.pages_pin_count, 0);
__i915_gem_object_put_pages(obj, I915_MM_NORMAL);
GEM_BUG_ON(i915_gem_object_has_pages(obj));
+ bitmap_free(obj->bit_17);
if (obj->base.import_attach)
drm_prime_gem_destroy(&obj->base, NULL);
drm_gem_object_release(&obj->base);
- bitmap_free(obj->bit_17);
- i915_gem_object_free(obj);
-
- GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
- atomic_dec(&i915->mm.free_count);
-
- cond_resched();
+ /* But keep the pointer alive for RCU-protected lookups */
+ call_rcu(&obj->rcu, __i915_gem_free_object_rcu);
}
intel_runtime_pm_put(&i915->runtime_pm, wakeref);
}
@@ -260,18 +252,34 @@ static void __i915_gem_free_work(struct work_struct *work)
spin_unlock(&i915->mm.free_lock);
}
-static void __i915_gem_free_object_rcu(struct rcu_head *head)
+void i915_gem_free_object(struct drm_gem_object *gem_obj)
{
- struct drm_i915_gem_object *obj =
- container_of(head, typeof(*obj), rcu);
+ struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
struct drm_i915_private *i915 = to_i915(obj->base.dev);
/*
- * We reuse obj->rcu for the freed list, so we had better not treat
- * it like a rcu_head from this point forwards. And we expect all
- * objects to be freed via this path.
+ * Before we free the object, make sure any pure RCU-only
+ * read-side critical sections are complete, e.g.
+ * i915_gem_busy_ioctl(). For the corresponding synchronized
+ * lookup see i915_gem_object_lookup_rcu().
*/
- destroy_rcu_head(&obj->rcu);
+ atomic_inc(&i915->mm.free_count);
+
+ /*
+ * This serializes freeing with the shrinker. Since the free
+ * is delayed, first by RCU then by the workqueue, we want the
+ * shrinker to be able to free pages of unreferenced objects,
+ * or else we may oom whilst there are plenty of deferred
+ * freed objects.
+ */
+ if (i915_gem_object_has_pages(obj) &&
+ i915_gem_object_is_shrinkable(obj)) {
+ unsigned long flags;
+
+ spin_lock_irqsave(&i915->mm.obj_lock, flags);
+ list_del_init(&obj->mm.link);
+ spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
+ }
/*
* Since we require blocking on struct_mutex to unbind the freed
@@ -287,20 +295,6 @@ static void __i915_gem_free_object_rcu(struct rcu_head *head)
queue_work(i915->wq, &i915->mm.free_work);
}
-void i915_gem_free_object(struct drm_gem_object *gem_obj)
-{
- struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
-
- /*
- * Before we free the object, make sure any pure RCU-only
- * read-side critical sections are complete, e.g.
- * i915_gem_busy_ioctl(). For the corresponding synchronized
- * lookup see i915_gem_object_lookup_rcu().
- */
- atomic_inc(&to_i915(obj->base.dev)->mm.free_count);
- call_rcu(&obj->rcu, __i915_gem_free_object_rcu);
-}
-
static inline enum fb_op_origin
fb_write_origin(struct drm_i915_gem_object *obj, unsigned int domain)
{
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 7b900ee4ed8d..b9fab22ada6f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -159,7 +159,7 @@ int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align)
if (obj->ops != &i915_gem_shmem_ops)
return -EINVAL;
- err = i915_gem_object_unbind(obj);
+ err = i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE);
if (err)
return err;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index d99f1a600b96..3f4c6bdcc3c3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -88,10 +88,18 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
return swap_available() || obj->mm.madv == I915_MADV_DONTNEED;
}
-static bool unsafe_drop_pages(struct drm_i915_gem_object *obj)
+static bool unsafe_drop_pages(struct drm_i915_gem_object *obj,
+ unsigned long shrink)
{
- if (i915_gem_object_unbind(obj) == 0)
+ unsigned long flags;
+
+ flags = 0;
+ if (shrink & I915_SHRINK_ACTIVE)
+ flags = I915_GEM_OBJECT_UNBIND_ACTIVE;
+
+ if (i915_gem_object_unbind(obj, flags) == 0)
__i915_gem_object_put_pages(obj, I915_MM_SHRINKER);
+
return !i915_gem_object_has_pages(obj);
}
@@ -229,9 +237,7 @@ i915_gem_shrink(struct drm_i915_private *i915,
continue;
if (!(shrink & I915_SHRINK_ACTIVE) &&
- (i915_gem_object_is_framebuffer(obj) ||
- !reservation_object_test_signaled_rcu(obj->base.resv,
- true)))
+ i915_gem_object_is_framebuffer(obj))
continue;
if (!(shrink & I915_SHRINK_BOUND) &&
@@ -246,7 +252,7 @@ i915_gem_shrink(struct drm_i915_private *i915,
spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
- if (unsafe_drop_pages(obj)) {
+ if (unsafe_drop_pages(obj, shrink)) {
/* May arrive from get_pages on another bo */
mutex_lock_nested(&obj->mm.lock,
I915_MM_SHRINKER);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 528b61678334..16ccec7fb7da 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -150,7 +150,8 @@ userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
}
}
- ret = i915_gem_object_unbind(obj);
+ ret = i915_gem_object_unbind(obj,
+ I915_GEM_OBJECT_UNBIND_ACTIVE);
if (ret == 0)
ret = __i915_gem_object_put_pages(obj, I915_MM_SHRINKER);
i915_gem_object_put(obj);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 09e09d26e67d..9d132c9d17b0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2444,18 +2444,17 @@ int i915_gem_freeze_late(struct drm_i915_private *dev_priv);
static inline void i915_gem_drain_freed_objects(struct drm_i915_private *i915)
{
- if (!atomic_read(&i915->mm.free_count))
- return;
-
- /* A single pass should suffice to release all the freed objects (along
+ /*
+ * A single pass should suffice to release all the freed objects (along
* most call paths) , but be a little more paranoid in that freeing
* the objects does take a little amount of time, during which the rcu
* callbacks could have added new objects into the freed list, and
* armed the work again.
*/
- do {
+ while (atomic_read(&i915->mm.free_count)) {
+ flush_work(&i915->mm.free_work);
rcu_barrier();
- } while (flush_work(&i915->mm.free_work));
+ }
}
static inline void i915_gem_drain_workqueue(struct drm_i915_private *i915)
@@ -2486,7 +2485,9 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
u64 alignment,
u64 flags);
-int i915_gem_object_unbind(struct drm_i915_gem_object *obj);
+int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
+ unsigned long flags);
+#define I915_GEM_OBJECT_UNBIND_ACTIVE BIT(0)
void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b7f290b77f8f..7ade42b8ec99 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -101,7 +101,8 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
return 0;
}
-int i915_gem_object_unbind(struct drm_i915_gem_object *obj)
+int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
+ unsigned long flags)
{
struct i915_vma *vma;
LIST_HEAD(still_in_list);
@@ -116,7 +117,10 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj)
list_move_tail(&vma->obj_link, &still_in_list);
spin_unlock(&obj->vma.lock);
- ret = i915_vma_unbind(vma);
+ ret = -EBUSY;
+ if (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
+ !i915_vma_is_active(vma))
+ ret = i915_vma_unbind(vma);
spin_lock(&obj->vma.lock);
}
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 06/15] drm/i915/gem: Free pages before rcu-freeing the object
2019-07-03 9:17 ` [PATCH 06/15] drm/i915/gem: Free pages before rcu-freeing the object Chris Wilson
@ 2019-07-03 10:37 ` Matthew Auld
0 siblings, 0 replies; 31+ messages in thread
From: Matthew Auld @ 2019-07-03 10:37 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 03/07/2019 10:17, Chris Wilson wrote:
> As we have dropped the final reference to the object, we do not need to
> wait until after the rcu grace period to drop its pages. We still require
> struct_mutex to completely unbind the object to release the pages, so we
> still need a free-worker to manage that from process context. By
> scheduling the release of pages before waiting for the rcu should mean
> that we are not trapping those pages from beyond the reach of the
> shrinker.
>
> v2: Pass along the request to skip if the vma is busy to the underlying
> unbind routine, to avoid checking the reservation underneath the
> i915->mm.obj_lock which may be used from inside irq context.
>
> v3: Flip the bit for unbinding while active, for later convenience.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111035
> Fixes: a93615f900bd ("drm/i915: Throw away the active object retirement complexity")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 07/15] drm/i915: Markup potential lock for i915_active
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (4 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 06/15] drm/i915/gem: Free pages before rcu-freeing the object Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 10:38 ` Matthew Auld
2019-07-03 9:17 ` [PATCH 08/15] drm/i915: Mark up vma->active as safe for use inside shrinkers Chris Wilson
` (12 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
Make the lockchains more deterministic via i915_active by flagging the
potential lock.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_active.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 584b247df9bc..13f304a29fc8 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -268,6 +268,8 @@ int i915_active_wait(struct i915_active *ref)
int err;
might_sleep();
+ might_lock(&ref->mutex);
+
if (RB_EMPTY_ROOT(&ref->tree))
return 0;
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 08/15] drm/i915: Mark up vma->active as safe for use inside shrinkers
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (5 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 07/15] drm/i915: Markup potential lock for i915_active Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 10:53 ` Matthew Auld
2019-07-03 9:17 ` [PATCH 09/15] drm/i915/execlists: Hesitate before slicing Chris Wilson
` (11 subsequent siblings)
18 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
Since a shrinker may be forced to wait on GPU activity,
i915_active_wait(&vma->active) must be safe for use inside a shrinker,
and so let's mark up the lock as being acquired by the shrinker to avoid
any nasty surprises creeping in.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_vma.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index c20a3022cd80..ee73baf29415 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -22,6 +22,7 @@
*
*/
+#include <linux/sched/mm.h>
#include <drm/drm_gem.h>
#include "display/intel_frontbuffer.h"
@@ -120,6 +121,13 @@ vma_create(struct drm_i915_gem_object *obj,
__i915_vma_active, __i915_vma_retire);
INIT_ACTIVE_REQUEST(&vma->last_fence);
+ /* Declare ourselves safe for use inside shrinkers */
+ if (IS_ENABLED(CONFIG_LOCKDEP)) {
+ fs_reclaim_acquire(GFP_KERNEL);
+ might_lock(&vma->active.mutex);
+ fs_reclaim_release(GFP_KERNEL);
+ }
+
INIT_LIST_HEAD(&vma->closed_link);
if (view && view->type != I915_GGTT_VIEW_NORMAL) {
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 09/15] drm/i915/execlists: Hesitate before slicing
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (6 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 08/15] drm/i915: Mark up vma->active as safe for use inside shrinkers Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 9:17 ` [PATCH 10/15] drm/i915: Teach execbuffer to take the engine wakeref not GT Chris Wilson
` (10 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
Be a little more hesitant before injecting a timeslice, and try to take
into account any change in priority that is due for the running task
before switching to another task. This will allow us to arbitrarily
prevent switching away from a request if we deem it necessarily to
disable preemption, for instance.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 497ca52381a7..f08afa26ba42 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -899,7 +899,7 @@ need_timeslice(struct intel_engine_cs *engine, const struct i915_request *rq)
hint = max(rq_prio(list_next_entry(rq, sched.link)),
engine->execlists.queue_priority_hint);
- return hint >= rq_prio(rq);
+ return hint >= effective_prio(rq);
}
static bool
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 10/15] drm/i915: Teach execbuffer to take the engine wakeref not GT
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (7 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 09/15] drm/i915/execlists: Hesitate before slicing Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 9:17 ` [PATCH 11/15] drm/i915/gt: Track timeline activeness in enter/exit Chris Wilson
` (9 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
In the next patch, we would like to couple into the engine wakeref to
free the batch pool on idling. The caveat here is that we therefore want
to track the engine wakeref more precisely and to hold it instead of the
broader GT wakeref as we process the ioctl.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 36 ++++++++++++-------
drivers/gpu/drm/i915/gt/intel_context.h | 7 ++++
2 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 1c5dfbfad71b..f43eaaa5db5f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2143,13 +2143,35 @@ static int eb_pin_context(struct i915_execbuffer *eb, struct intel_context *ce)
if (err)
return err;
+ /*
+ * Take a local wakeref for preparing to dispatch the execbuf as
+ * we expect to access the hardware fairly frequently in the
+ * process. Upon first dispatch, we acquire another prolonged
+ * wakeref that we hold until the GPU has been idle for at least
+ * 100ms.
+ */
+ err = intel_context_timeline_lock(ce);
+ if (err)
+ goto err_unpin;
+
+ intel_context_enter(ce);
+ intel_context_timeline_unlock(ce);
+
eb->engine = ce->engine;
eb->context = ce;
return 0;
+
+err_unpin:
+ intel_context_unpin(ce);
+ return err;
}
static void eb_unpin_context(struct i915_execbuffer *eb)
{
+ __intel_context_timeline_lock(eb->context);
+ intel_context_exit(eb->context);
+ intel_context_timeline_unlock(eb->context);
+
intel_context_unpin(eb->context);
}
@@ -2430,18 +2452,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
if (unlikely(err))
goto err_destroy;
- /*
- * Take a local wakeref for preparing to dispatch the execbuf as
- * we expect to access the hardware fairly frequently in the
- * process. Upon first dispatch, we acquire another prolonged
- * wakeref that we hold until the GPU has been idle for at least
- * 100ms.
- */
- intel_gt_pm_get(&eb.i915->gt);
-
err = i915_mutex_lock_interruptible(dev);
if (err)
- goto err_rpm;
+ goto err_context;
err = eb_select_engine(&eb, file, args);
if (unlikely(err))
@@ -2606,8 +2619,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
eb_unpin_context(&eb);
err_unlock:
mutex_unlock(&dev->struct_mutex);
-err_rpm:
- intel_gt_pm_put(&eb.i915->gt);
+err_context:
i915_gem_context_put(eb.gem_context);
err_destroy:
eb_destroy(&eb);
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 40cd8320fcc3..065ba4ac4e87 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -126,6 +126,13 @@ static inline void intel_context_put(struct intel_context *ce)
kref_put(&ce->ref, ce->ops->destroy);
}
+static inline void
+__intel_context_timeline_lock(struct intel_context *ce)
+ __acquires(&ce->ring->timeline->mutex)
+{
+ mutex_lock(&ce->ring->timeline->mutex);
+}
+
static inline int __must_check
intel_context_timeline_lock(struct intel_context *ce)
__acquires(&ce->ring->timeline->mutex)
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 11/15] drm/i915/gt: Track timeline activeness in enter/exit
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (8 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 10/15] drm/i915: Teach execbuffer to take the engine wakeref not GT Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 9:17 ` [PATCH 12/15] drm/i915/gt: Convert timeline tracking to spinlock Chris Wilson
` (8 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
Lift moving the timeline to/from the active_list on enter/exit in order
to shorten the active tracking span in comparison to the existing
pin/unpin.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gem/i915_gem_pm.c | 1 -
drivers/gpu/drm/i915/gt/intel_context.c | 2 +
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 1 +
drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +
drivers/gpu/drm/i915/gt/intel_timeline.c | 98 +++++++------------
drivers/gpu/drm/i915/gt/intel_timeline.h | 3 +-
.../gpu/drm/i915/gt/intel_timeline_types.h | 1 +
drivers/gpu/drm/i915/gt/selftest_timeline.c | 2 -
8 files changed, 46 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 4d774376f5b8..93d188526457 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -38,7 +38,6 @@ static void i915_gem_park(struct drm_i915_private *i915)
i915_gem_batch_pool_fini(&engine->batch_pool);
}
- intel_timelines_park(i915);
i915_vma_parked(i915);
i915_globals_park();
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 1110fc8f657a..0111a18c1f02 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -232,10 +232,12 @@ int __init i915_global_context_init(void)
void intel_context_enter_engine(struct intel_context *ce)
{
intel_engine_pm_get(ce->engine);
+ intel_timeline_enter(ce->ring->timeline);
}
void intel_context_exit_engine(struct intel_context *ce)
{
+ intel_timeline_exit(ce->ring->timeline);
intel_engine_pm_put(ce->engine);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 84e432abe8e0..9751a02d86bc 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -88,6 +88,7 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
/* Check again on the next retirement. */
engine->wakeref_serial = engine->serial + 1;
+ intel_timeline_enter(rq->timeline);
i915_request_add_barriers(rq);
__i915_request_commit(rq);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index f08afa26ba42..72c22b95cd9c 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -3191,6 +3191,8 @@ static void virtual_context_enter(struct intel_context *ce)
for (n = 0; n < ve->num_siblings; n++)
intel_engine_pm_get(ve->siblings[n]);
+
+ intel_timeline_enter(ce->ring->timeline);
}
static void virtual_context_exit(struct intel_context *ce)
@@ -3198,6 +3200,8 @@ static void virtual_context_exit(struct intel_context *ce)
struct virtual_engine *ve = container_of(ce, typeof(*ve), context);
unsigned int n;
+ intel_timeline_exit(ce->ring->timeline);
+
for (n = 0; n < ve->num_siblings; n++)
intel_engine_pm_put(ve->siblings[n]);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index 6daa9eb59e19..4af0b9801d91 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -278,64 +278,11 @@ void intel_timelines_init(struct drm_i915_private *i915)
timelines_init(&i915->gt);
}
-static void timeline_add_to_active(struct intel_timeline *tl)
-{
- struct intel_gt_timelines *gt = &tl->gt->timelines;
-
- mutex_lock(>->mutex);
- list_add(&tl->link, >->active_list);
- mutex_unlock(>->mutex);
-}
-
-static void timeline_remove_from_active(struct intel_timeline *tl)
-{
- struct intel_gt_timelines *gt = &tl->gt->timelines;
-
- mutex_lock(>->mutex);
- list_del(&tl->link);
- mutex_unlock(>->mutex);
-}
-
-static void timelines_park(struct intel_gt *gt)
-{
- struct intel_gt_timelines *timelines = >->timelines;
- struct intel_timeline *timeline;
-
- mutex_lock(&timelines->mutex);
- list_for_each_entry(timeline, &timelines->active_list, link) {
- /*
- * All known fences are completed so we can scrap
- * the current sync point tracking and start afresh,
- * any attempt to wait upon a previous sync point
- * will be skipped as the fence was signaled.
- */
- i915_syncmap_free(&timeline->sync);
- }
- mutex_unlock(&timelines->mutex);
-}
-
-/**
- * intel_timelines_park - called when the driver idles
- * @i915: the drm_i915_private device
- *
- * When the driver is completely idle, we know that all of our sync points
- * have been signaled and our tracking is then entirely redundant. Any request
- * to wait upon an older sync point will be completed instantly as we know
- * the fence is signaled and therefore we will not even look them up in the
- * sync point map.
- */
-void intel_timelines_park(struct drm_i915_private *i915)
-{
- timelines_park(&i915->gt);
-}
-
void intel_timeline_fini(struct intel_timeline *timeline)
{
GEM_BUG_ON(timeline->pin_count);
GEM_BUG_ON(!list_empty(&timeline->requests));
- i915_syncmap_free(&timeline->sync);
-
if (timeline->hwsp_cacheline)
cacheline_free(timeline->hwsp_cacheline);
else
@@ -370,6 +317,7 @@ int intel_timeline_pin(struct intel_timeline *tl)
if (tl->pin_count++)
return 0;
GEM_BUG_ON(!tl->pin_count);
+ GEM_BUG_ON(tl->active_count);
err = i915_vma_pin(tl->hwsp_ggtt, 0, 0, PIN_GLOBAL | PIN_HIGH);
if (err)
@@ -380,7 +328,6 @@ int intel_timeline_pin(struct intel_timeline *tl)
offset_in_page(tl->hwsp_offset);
cacheline_acquire(tl->hwsp_cacheline);
- timeline_add_to_active(tl);
return 0;
@@ -389,6 +336,40 @@ int intel_timeline_pin(struct intel_timeline *tl)
return err;
}
+void intel_timeline_enter(struct intel_timeline *tl)
+{
+ struct intel_gt_timelines *timelines = &tl->gt->timelines;
+
+ GEM_BUG_ON(!tl->pin_count);
+ if (tl->active_count++)
+ return;
+ GEM_BUG_ON(!tl->active_count); /* overflow? */
+
+ mutex_lock(&timelines->mutex);
+ list_add(&tl->link, &timelines->active_list);
+ mutex_unlock(&timelines->mutex);
+}
+
+void intel_timeline_exit(struct intel_timeline *tl)
+{
+ struct intel_gt_timelines *timelines = &tl->gt->timelines;
+
+ GEM_BUG_ON(!tl->active_count);
+ if (--tl->active_count)
+ return;
+
+ mutex_lock(&timelines->mutex);
+ list_del(&tl->link);
+ mutex_unlock(&timelines->mutex);
+
+ /*
+ * Since this timeline is idle, all bariers upon which we were waiting
+ * must also be complete and so we can discard the last used barriers
+ * without loss of information.
+ */
+ i915_syncmap_free(&tl->sync);
+}
+
static u32 timeline_advance(struct intel_timeline *tl)
{
GEM_BUG_ON(!tl->pin_count);
@@ -546,16 +527,9 @@ void intel_timeline_unpin(struct intel_timeline *tl)
if (--tl->pin_count)
return;
- timeline_remove_from_active(tl);
+ GEM_BUG_ON(tl->active_count);
cacheline_release(tl->hwsp_cacheline);
- /*
- * Since this timeline is idle, all bariers upon which we were waiting
- * must also be complete and so we can discard the last used barriers
- * without loss of information.
- */
- i915_syncmap_free(&tl->sync);
-
__i915_vma_unpin(tl->hwsp_ggtt);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.h b/drivers/gpu/drm/i915/gt/intel_timeline.h
index e08cebf64833..f583af1ba18d 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.h
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.h
@@ -77,9 +77,11 @@ static inline bool intel_timeline_sync_is_later(struct intel_timeline *tl,
}
int intel_timeline_pin(struct intel_timeline *tl);
+void intel_timeline_enter(struct intel_timeline *tl);
int intel_timeline_get_seqno(struct intel_timeline *tl,
struct i915_request *rq,
u32 *seqno);
+void intel_timeline_exit(struct intel_timeline *tl);
void intel_timeline_unpin(struct intel_timeline *tl);
int intel_timeline_read_hwsp(struct i915_request *from,
@@ -87,7 +89,6 @@ int intel_timeline_read_hwsp(struct i915_request *from,
u32 *hwsp_offset);
void intel_timelines_init(struct drm_i915_private *i915);
-void intel_timelines_park(struct drm_i915_private *i915);
void intel_timelines_fini(struct drm_i915_private *i915);
#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline_types.h b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
index 9a71aea7a338..b820ee76b7f5 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
@@ -58,6 +58,7 @@ struct intel_timeline {
*/
struct i915_syncmap *sync;
+ unsigned int active_count;
struct list_head link;
struct intel_gt *gt;
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index eae3b1963bf7..9f3100135590 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -815,8 +815,6 @@ static int live_hwsp_recycle(void *arg)
if (err)
goto out;
-
- intel_timelines_park(i915); /* Encourage recycling! */
} while (!__igt_timeout(end_time, NULL));
}
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 12/15] drm/i915/gt: Convert timeline tracking to spinlock
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (9 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 11/15] drm/i915/gt: Track timeline activeness in enter/exit Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 9:17 ` [PATCH 13/15] drm/i915/gt: Guard timeline pinning with its own mutex Chris Wilson
` (7 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
Convert the list manipulation of active to use spinlocks so that we can
perform the updates from underneath a quick interrupt callback.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_gt_types.h | 2 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 13 ++++++++++---
drivers/gpu/drm/i915/gt/intel_timeline.c | 12 +++++-------
drivers/gpu/drm/i915/i915_gem.c | 20 ++++++++++----------
4 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index c03e56628ee2..cfd41e6c54e1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -26,7 +26,7 @@ struct intel_gt {
struct i915_ggtt *ggtt;
struct intel_gt_timelines {
- struct mutex mutex; /* protects list */
+ spinlock_t lock; /* protects active_list */
struct list_head active_list;
/* Pack multiple timelines' seqnos into the same page */
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index adfdb908587f..72002c0f9698 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -858,6 +858,7 @@ void i915_gem_set_wedged(struct drm_i915_private *i915)
static bool __i915_gem_unset_wedged(struct drm_i915_private *i915)
{
struct i915_gpu_error *error = &i915->gpu_error;
+ struct intel_gt_timelines *timelines = &i915->gt.timelines;
struct intel_timeline *tl;
if (!test_bit(I915_WEDGED, &error->flags))
@@ -878,14 +879,16 @@ static bool __i915_gem_unset_wedged(struct drm_i915_private *i915)
*
* No more can be submitted until we reset the wedged bit.
*/
- mutex_lock(&i915->gt.timelines.mutex);
- list_for_each_entry(tl, &i915->gt.timelines.active_list, link) {
+ spin_lock(&timelines->lock);
+ list_for_each_entry(tl, &timelines->active_list, link) {
struct i915_request *rq;
rq = i915_active_request_get_unlocked(&tl->last_request);
if (!rq)
continue;
+ spin_unlock(&timelines->lock);
+
/*
* All internal dependencies (i915_requests) will have
* been flushed by the set-wedge, but we may be stuck waiting
@@ -895,8 +898,12 @@ static bool __i915_gem_unset_wedged(struct drm_i915_private *i915)
*/
dma_fence_default_wait(&rq->fence, false, MAX_SCHEDULE_TIMEOUT);
i915_request_put(rq);
+
+ /* Restart iteration after droping lock */
+ spin_lock(&timelines->lock);
+ tl = list_entry(&timelines->active_list, typeof(*tl), link);
}
- mutex_unlock(&i915->gt.timelines.mutex);
+ spin_unlock(&timelines->lock);
intel_gt_sanitize(&i915->gt, false);
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index 4af0b9801d91..355dfc52c804 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -266,7 +266,7 @@ static void timelines_init(struct intel_gt *gt)
{
struct intel_gt_timelines *timelines = >->timelines;
- mutex_init(&timelines->mutex);
+ spin_lock_init(&timelines->lock);
INIT_LIST_HEAD(&timelines->active_list);
spin_lock_init(&timelines->hwsp_lock);
@@ -345,9 +345,9 @@ void intel_timeline_enter(struct intel_timeline *tl)
return;
GEM_BUG_ON(!tl->active_count); /* overflow? */
- mutex_lock(&timelines->mutex);
+ spin_lock(&timelines->lock);
list_add(&tl->link, &timelines->active_list);
- mutex_unlock(&timelines->mutex);
+ spin_unlock(&timelines->lock);
}
void intel_timeline_exit(struct intel_timeline *tl)
@@ -358,9 +358,9 @@ void intel_timeline_exit(struct intel_timeline *tl)
if (--tl->active_count)
return;
- mutex_lock(&timelines->mutex);
+ spin_lock(&timelines->lock);
list_del(&tl->link);
- mutex_unlock(&timelines->mutex);
+ spin_unlock(&timelines->lock);
/*
* Since this timeline is idle, all bariers upon which we were waiting
@@ -548,8 +548,6 @@ static void timelines_fini(struct intel_gt *gt)
GEM_BUG_ON(!list_empty(&timelines->active_list));
GEM_BUG_ON(!list_empty(&timelines->hwsp_free_list));
-
- mutex_destroy(&timelines->mutex);
}
void intel_timelines_fini(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7ade42b8ec99..b6f3baa74da4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -909,20 +909,20 @@ static int wait_for_engines(struct drm_i915_private *i915)
static long
wait_for_timelines(struct drm_i915_private *i915,
- unsigned int flags, long timeout)
+ unsigned int wait, long timeout)
{
- struct intel_gt_timelines *gt = &i915->gt.timelines;
+ struct intel_gt_timelines *timelines = &i915->gt.timelines;
struct intel_timeline *tl;
- mutex_lock(>->mutex);
- list_for_each_entry(tl, >->active_list, link) {
+ spin_lock(&timelines->lock);
+ list_for_each_entry(tl, &timelines->active_list, link) {
struct i915_request *rq;
rq = i915_active_request_get_unlocked(&tl->last_request);
if (!rq)
continue;
- mutex_unlock(>->mutex);
+ spin_unlock(&timelines->lock);
/*
* "Race-to-idle".
@@ -933,19 +933,19 @@ wait_for_timelines(struct drm_i915_private *i915,
* want to complete as quickly as possible to avoid prolonged
* stalls, so allow the gpu to boost to maximum clocks.
*/
- if (flags & I915_WAIT_FOR_IDLE_BOOST)
+ if (wait & I915_WAIT_FOR_IDLE_BOOST)
gen6_rps_boost(rq);
- timeout = i915_request_wait(rq, flags, timeout);
+ timeout = i915_request_wait(rq, wait, timeout);
i915_request_put(rq);
if (timeout < 0)
return timeout;
/* restart after reacquiring the lock */
- mutex_lock(>->mutex);
- tl = list_entry(>->active_list, typeof(*tl), link);
+ spin_lock(&timelines->lock);
+ tl = list_entry(&timelines->active_list, typeof(*tl), link);
}
- mutex_unlock(>->mutex);
+ spin_unlock(&timelines->lock);
return timeout;
}
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 13/15] drm/i915/gt: Guard timeline pinning with its own mutex
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (10 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 12/15] drm/i915/gt: Convert timeline tracking to spinlock Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 9:17 ` [PATCH 14/15] drm/i915: Protect request retirement with timeline->mutex Chris Wilson
` (6 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
In preparation for removing struct_mutex from around context retirement,
we need to make timeline pinning safe. Since multiple engines/contexts
can share a single timeline, it needs to be protected by a mutex.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_timeline.c | 27 +++++++++----------
.../gpu/drm/i915/gt/intel_timeline_types.h | 2 +-
drivers/gpu/drm/i915/gt/mock_engine.c | 6 ++---
3 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index 355dfc52c804..7b476cd55dac 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -211,9 +211,9 @@ int intel_timeline_init(struct intel_timeline *timeline,
void *vaddr;
kref_init(&timeline->kref);
+ atomic_set(&timeline->pin_count, 0);
timeline->gt = gt;
- timeline->pin_count = 0;
timeline->has_initial_breadcrumb = !hwsp;
timeline->hwsp_cacheline = NULL;
@@ -280,7 +280,7 @@ void intel_timelines_init(struct drm_i915_private *i915)
void intel_timeline_fini(struct intel_timeline *timeline)
{
- GEM_BUG_ON(timeline->pin_count);
+ GEM_BUG_ON(atomic_read(&timeline->pin_count));
GEM_BUG_ON(!list_empty(&timeline->requests));
if (timeline->hwsp_cacheline)
@@ -314,33 +314,31 @@ int intel_timeline_pin(struct intel_timeline *tl)
{
int err;
- if (tl->pin_count++)
+ if (atomic_add_unless(&tl->pin_count, 1, 0))
return 0;
- GEM_BUG_ON(!tl->pin_count);
- GEM_BUG_ON(tl->active_count);
err = i915_vma_pin(tl->hwsp_ggtt, 0, 0, PIN_GLOBAL | PIN_HIGH);
if (err)
- goto unpin;
+ return err;
tl->hwsp_offset =
i915_ggtt_offset(tl->hwsp_ggtt) +
offset_in_page(tl->hwsp_offset);
cacheline_acquire(tl->hwsp_cacheline);
+ if (atomic_fetch_inc(&tl->pin_count)) {
+ cacheline_release(tl->hwsp_cacheline);
+ __i915_vma_unpin(tl->hwsp_ggtt);
+ }
return 0;
-
-unpin:
- tl->pin_count = 0;
- return err;
}
void intel_timeline_enter(struct intel_timeline *tl)
{
struct intel_gt_timelines *timelines = &tl->gt->timelines;
- GEM_BUG_ON(!tl->pin_count);
+ GEM_BUG_ON(!atomic_read(&tl->pin_count));
if (tl->active_count++)
return;
GEM_BUG_ON(!tl->active_count); /* overflow? */
@@ -372,7 +370,7 @@ void intel_timeline_exit(struct intel_timeline *tl)
static u32 timeline_advance(struct intel_timeline *tl)
{
- GEM_BUG_ON(!tl->pin_count);
+ GEM_BUG_ON(!atomic_read(&tl->pin_count));
GEM_BUG_ON(tl->seqno & tl->has_initial_breadcrumb);
return tl->seqno += 1 + tl->has_initial_breadcrumb;
@@ -523,11 +521,10 @@ int intel_timeline_read_hwsp(struct i915_request *from,
void intel_timeline_unpin(struct intel_timeline *tl)
{
- GEM_BUG_ON(!tl->pin_count);
- if (--tl->pin_count)
+ GEM_BUG_ON(!atomic_read(&tl->pin_count));
+ if (!atomic_dec_and_test(&tl->pin_count))
return;
- GEM_BUG_ON(tl->active_count);
cacheline_release(tl->hwsp_cacheline);
__i915_vma_unpin(tl->hwsp_ggtt);
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline_types.h b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
index b820ee76b7f5..8dd14a2b8781 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
@@ -25,7 +25,7 @@ struct intel_timeline {
struct mutex mutex; /* protects the flow of requests */
- unsigned int pin_count;
+ atomic_t pin_count;
const u32 *hwsp_seqno;
struct i915_vma *hwsp_ggtt;
u32 hwsp_offset;
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index 490ebd121f4c..a48b36d31e65 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -38,13 +38,13 @@ struct mock_ring {
static void mock_timeline_pin(struct intel_timeline *tl)
{
- tl->pin_count++;
+ atomic_inc(&tl->pin_count);
}
static void mock_timeline_unpin(struct intel_timeline *tl)
{
- GEM_BUG_ON(!tl->pin_count);
- tl->pin_count--;
+ GEM_BUG_ON(!atomic_read(&tl->pin_count));
+ atomic_dec(&tl->pin_count);
}
static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 14/15] drm/i915: Protect request retirement with timeline->mutex
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (11 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 13/15] drm/i915/gt: Guard timeline pinning with its own mutex Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 9:17 ` [PATCH 15/15] drm/i915: Replace struct_mutex for batch pool serialisation Chris Wilson
` (5 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
Forgo the struct_mutex requirement for request retirement as we have
been transitioning over to only using the timeline->mutex for
controlling the lifetime of a request on that timeline.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 192 ++++++++++--------
drivers/gpu/drm/i915/gt/intel_context.h | 25 +--
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 1 -
drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 -
drivers/gpu/drm/i915/gt/intel_gt.c | 1 -
drivers/gpu/drm/i915/gt/intel_gt_types.h | 2 -
drivers/gpu/drm/i915/gt/intel_lrc.c | 1 +
drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 13 +-
drivers/gpu/drm/i915/gt/mock_engine.c | 1 -
drivers/gpu/drm/i915/i915_request.c | 151 +++++++-------
drivers/gpu/drm/i915/i915_request.h | 3 -
11 files changed, 203 insertions(+), 189 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index f43eaaa5db5f..80c9c57a302f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -739,63 +739,6 @@ static int eb_select_context(struct i915_execbuffer *eb)
return 0;
}
-static struct i915_request *__eb_wait_for_ring(struct intel_ring *ring)
-{
- struct i915_request *rq;
-
- /*
- * Completely unscientific finger-in-the-air estimates for suitable
- * maximum user request size (to avoid blocking) and then backoff.
- */
- if (intel_ring_update_space(ring) >= PAGE_SIZE)
- return NULL;
-
- /*
- * Find a request that after waiting upon, there will be at least half
- * the ring available. The hysteresis allows us to compete for the
- * shared ring and should mean that we sleep less often prior to
- * claiming our resources, but not so long that the ring completely
- * drains before we can submit our next request.
- */
- list_for_each_entry(rq, &ring->request_list, ring_link) {
- if (__intel_ring_space(rq->postfix,
- ring->emit, ring->size) > ring->size / 2)
- break;
- }
- if (&rq->ring_link == &ring->request_list)
- return NULL; /* weird, we will check again later for real */
-
- return i915_request_get(rq);
-}
-
-static int eb_wait_for_ring(const struct i915_execbuffer *eb)
-{
- struct i915_request *rq;
- int ret = 0;
-
- /*
- * Apply a light amount of backpressure to prevent excessive hogs
- * from blocking waiting for space whilst holding struct_mutex and
- * keeping all of their resources pinned.
- */
-
- rq = __eb_wait_for_ring(eb->context->ring);
- if (rq) {
- mutex_unlock(&eb->i915->drm.struct_mutex);
-
- if (i915_request_wait(rq,
- I915_WAIT_INTERRUPTIBLE,
- MAX_SCHEDULE_TIMEOUT) < 0)
- ret = -EINTR;
-
- i915_request_put(rq);
-
- mutex_lock(&eb->i915->drm.struct_mutex);
- }
-
- return ret;
-}
-
static int eb_lookup_vmas(struct i915_execbuffer *eb)
{
struct radix_tree_root *handles_vma = &eb->gem_context->handles_vma;
@@ -2122,10 +2065,75 @@ static const enum intel_engine_id user_ring_map[] = {
[I915_EXEC_VEBOX] = VECS0
};
-static int eb_pin_context(struct i915_execbuffer *eb, struct intel_context *ce)
+static struct i915_request *eb_throttle(struct intel_context *ce)
+{
+ struct intel_ring *ring = ce->ring;
+ struct intel_timeline *tl = ring->timeline;
+ struct i915_request *rq;
+
+ /*
+ * Completely unscientific finger-in-the-air estimates for suitable
+ * maximum user request size (to avoid blocking) and then backoff.
+ */
+ if (intel_ring_update_space(ring) >= PAGE_SIZE)
+ return NULL;
+
+ /*
+ * Find a request that after waiting upon, there will be at least half
+ * the ring available. The hysteresis allows us to compete for the
+ * shared ring and should mean that we sleep less often prior to
+ * claiming our resources, but not so long that the ring completely
+ * drains before we can submit our next request.
+ */
+ list_for_each_entry(rq, &tl->requests, link) {
+ if (rq->ring != ring)
+ continue;
+
+ if (__intel_ring_space(rq->postfix,
+ ring->emit, ring->size) > ring->size / 2)
+ break;
+ }
+ if (&rq->link == &tl->requests)
+ return NULL; /* weird, we will check again later for real */
+
+ return i915_request_get(rq);
+}
+
+static int
+__eb_pin_context(struct i915_execbuffer *eb, struct intel_context *ce)
{
int err;
+ if (likely(atomic_inc_not_zero(&ce->pin_count)))
+ return 0;
+
+ err = mutex_lock_interruptible(&eb->i915->drm.struct_mutex);
+ if (err)
+ return err;
+
+ err = __intel_context_do_pin(ce);
+ mutex_unlock(&eb->i915->drm.struct_mutex);
+
+ return err;
+}
+
+static void
+__eb_unpin_context(struct i915_execbuffer *eb, struct intel_context *ce)
+{
+ if (likely(atomic_add_unless(&ce->pin_count, -1, 1)))
+ return;
+
+ mutex_lock(&eb->i915->drm.struct_mutex);
+ intel_context_unpin(ce);
+ mutex_unlock(&eb->i915->drm.struct_mutex);
+}
+
+static int __eb_pin_engine(struct i915_execbuffer *eb, struct intel_context *ce)
+{
+ struct intel_timeline *tl;
+ struct i915_request *rq;
+ int err;
+
/*
* ABI: Before userspace accesses the GPU (e.g. execbuffer), report
* EIO if the GPU is already wedged.
@@ -2139,7 +2147,7 @@ static int eb_pin_context(struct i915_execbuffer *eb, struct intel_context *ce)
* GGTT space, so do this first before we reserve a seqno for
* ourselves.
*/
- err = intel_context_pin(ce);
+ err = __eb_pin_context(eb, ce);
if (err)
return err;
@@ -2150,29 +2158,52 @@ static int eb_pin_context(struct i915_execbuffer *eb, struct intel_context *ce)
* wakeref that we hold until the GPU has been idle for at least
* 100ms.
*/
- err = intel_context_timeline_lock(ce);
- if (err)
+ tl = intel_context_timeline_lock(ce);
+ if (IS_ERR(tl)) {
+ err = PTR_ERR(tl);
goto err_unpin;
+ }
intel_context_enter(ce);
- intel_context_timeline_unlock(ce);
+ rq = eb_throttle(ce);
+
+ intel_context_timeline_unlock(tl);
+
+ if (rq) {
+ if (i915_request_wait(rq,
+ I915_WAIT_INTERRUPTIBLE,
+ MAX_SCHEDULE_TIMEOUT) < 0) {
+ i915_request_put(rq);
+ err = -EINTR;
+ goto err_exit;
+ }
+
+ i915_request_put(rq);
+ }
eb->engine = ce->engine;
eb->context = ce;
return 0;
+err_exit:
+ mutex_lock(&tl->mutex);
+ intel_context_exit(ce);
+ intel_context_timeline_unlock(tl);
err_unpin:
- intel_context_unpin(ce);
+ __eb_unpin_context(eb, ce);
return err;
}
-static void eb_unpin_context(struct i915_execbuffer *eb)
+static void eb_unpin_engine(struct i915_execbuffer *eb)
{
- __intel_context_timeline_lock(eb->context);
- intel_context_exit(eb->context);
- intel_context_timeline_unlock(eb->context);
+ struct intel_context *ce = eb->context;
+ struct intel_timeline *tl = ce->ring->timeline;
+
+ mutex_lock(&tl->mutex);
+ intel_context_exit(ce);
+ intel_context_timeline_unlock(tl);
- intel_context_unpin(eb->context);
+ __eb_unpin_context(eb, ce);
}
static unsigned int
@@ -2217,9 +2248,9 @@ eb_select_legacy_ring(struct i915_execbuffer *eb,
}
static int
-eb_select_engine(struct i915_execbuffer *eb,
- struct drm_file *file,
- struct drm_i915_gem_execbuffer2 *args)
+eb_pin_engine(struct i915_execbuffer *eb,
+ struct drm_file *file,
+ struct drm_i915_gem_execbuffer2 *args)
{
struct intel_context *ce;
unsigned int idx;
@@ -2234,7 +2265,7 @@ eb_select_engine(struct i915_execbuffer *eb,
if (IS_ERR(ce))
return PTR_ERR(ce);
- err = eb_pin_context(eb, ce);
+ err = __eb_pin_engine(eb, ce);
intel_context_put(ce);
return err;
@@ -2452,16 +2483,12 @@ i915_gem_do_execbuffer(struct drm_device *dev,
if (unlikely(err))
goto err_destroy;
- err = i915_mutex_lock_interruptible(dev);
- if (err)
- goto err_context;
-
- err = eb_select_engine(&eb, file, args);
+ err = eb_pin_engine(&eb, file, args);
if (unlikely(err))
- goto err_unlock;
+ goto err_context;
- err = eb_wait_for_ring(&eb); /* may temporarily drop struct_mutex */
- if (unlikely(err))
+ err = i915_mutex_lock_interruptible(dev);
+ if (err)
goto err_engine;
err = eb_relocate(&eb);
@@ -2615,10 +2642,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
err_vma:
if (eb.exec)
eb_release_vmas(&eb);
-err_engine:
- eb_unpin_context(&eb);
-err_unlock:
mutex_unlock(&dev->struct_mutex);
+err_engine:
+ eb_unpin_engine(&eb);
err_context:
i915_gem_context_put(eb.gem_context);
err_destroy:
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 065ba4ac4e87..38b60cbf2592 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -12,6 +12,7 @@
#include "i915_active.h"
#include "intel_context_types.h"
#include "intel_engine_types.h"
+#include "intel_timeline_types.h"
void intel_context_init(struct intel_context *ce,
struct i915_gem_context *ctx,
@@ -126,24 +127,24 @@ static inline void intel_context_put(struct intel_context *ce)
kref_put(&ce->ref, ce->ops->destroy);
}
-static inline void
-__intel_context_timeline_lock(struct intel_context *ce)
- __acquires(&ce->ring->timeline->mutex)
-{
- mutex_lock(&ce->ring->timeline->mutex);
-}
-
-static inline int __must_check
+static inline struct intel_timeline *__must_check
intel_context_timeline_lock(struct intel_context *ce)
__acquires(&ce->ring->timeline->mutex)
{
- return mutex_lock_interruptible(&ce->ring->timeline->mutex);
+ struct intel_timeline *tl = ce->ring->timeline;
+ int err;
+
+ err = mutex_lock_interruptible(&tl->mutex);
+ if (err)
+ return ERR_PTR(err);
+
+ return tl;
}
-static inline void intel_context_timeline_unlock(struct intel_context *ce)
- __releases(&ce->ring->timeline->mutex)
+static inline void intel_context_timeline_unlock(struct intel_timeline *tl)
+ __releases(&tl->mutex)
{
- mutex_unlock(&ce->ring->timeline->mutex);
+ mutex_unlock(&tl->mutex);
}
struct i915_request *intel_context_create_request(struct intel_context *ce);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index d1508f0b4c84..b27fc555fe09 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -745,7 +745,6 @@ static int measure_breadcrumb_dw(struct intel_engine_cs *engine)
engine->status_page.vma))
goto out_frame;
- INIT_LIST_HEAD(&frame->ring.request_list);
frame->ring.timeline = &frame->timeline;
frame->ring.vaddr = frame->cs;
frame->ring.size = sizeof(frame->cs);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 7e056114344e..0dde7e04b102 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -69,8 +69,6 @@ struct intel_ring {
void *vaddr;
struct intel_timeline *timeline;
- struct list_head request_list;
- struct list_head active_link;
/*
* As we have two types of rings, one global to the engine used
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 8cca6b22b386..46d24d9d62ac 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -14,7 +14,6 @@ void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
gt->i915 = i915;
gt->uncore = &i915->uncore;
- INIT_LIST_HEAD(>->active_rings);
INIT_LIST_HEAD(>->closed_vma);
spin_lock_init(>->closed_lock);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index cfd41e6c54e1..f43ea830b1e8 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -34,8 +34,6 @@ struct intel_gt {
struct list_head hwsp_free_list;
} timelines;
- struct list_head active_rings;
-
struct intel_wakeref wakeref;
struct list_head closed_vma;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 72c22b95cd9c..9ebd4f258227 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1535,6 +1535,7 @@ static void execlists_context_unpin(struct intel_context *ce)
{
i915_gem_context_unpin_hw_id(ce->gem_context);
i915_gem_object_unpin_map(ce->state->obj);
+ intel_ring_reset(ce->ring, ce->ring->tail);
}
static void
diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
index 81f9b0422e6a..b771170eb56a 100644
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -1227,7 +1227,7 @@ void intel_ring_unpin(struct intel_ring *ring)
GEM_TRACE("ring:%llx unpin\n", ring->timeline->fence_context);
/* Discard any unused bytes beyond that submitted to hw. */
- intel_ring_reset(ring, ring->tail);
+ intel_ring_reset(ring, ring->emit);
GEM_BUG_ON(!ring->vma);
i915_vma_unset_ggtt_write(ring->vma);
@@ -1293,7 +1293,6 @@ intel_engine_create_ring(struct intel_engine_cs *engine,
return ERR_PTR(-ENOMEM);
kref_init(&ring->ref);
- INIT_LIST_HEAD(&ring->request_list);
ring->timeline = intel_timeline_get(timeline);
ring->size = size;
@@ -1817,21 +1816,25 @@ static int ring_request_alloc(struct i915_request *request)
static noinline int wait_for_space(struct intel_ring *ring, unsigned int bytes)
{
+ struct intel_timeline *tl = ring->timeline;
struct i915_request *target;
long timeout;
if (intel_ring_update_space(ring) >= bytes)
return 0;
- GEM_BUG_ON(list_empty(&ring->request_list));
- list_for_each_entry(target, &ring->request_list, ring_link) {
+ GEM_BUG_ON(list_empty(&tl->requests));
+ list_for_each_entry(target, &tl->requests, link) {
+ if (target->ring != ring)
+ continue;
+
/* Would completion of this request free enough space? */
if (bytes <= __intel_ring_space(target->postfix,
ring->emit, ring->size))
break;
}
- if (WARN_ON(&target->ring_link == &ring->request_list))
+ if (GEM_WARN_ON(&target->link == &tl->requests))
return -ENOSPC;
timeout = i915_request_wait(target,
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index a48b36d31e65..5bcb461b8372 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -68,7 +68,6 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
ring->base.timeline = &ring->timeline;
atomic_set(&ring->base.pin_count, 1);
- INIT_LIST_HEAD(&ring->base.request_list);
intel_ring_update_space(&ring->base);
return &ring->base;
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 5ff87c4a0cd5..2aabed8ed594 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -180,40 +180,6 @@ i915_request_remove_from_client(struct i915_request *request)
spin_unlock(&file_priv->mm.lock);
}
-static void advance_ring(struct i915_request *request)
-{
- struct intel_ring *ring = request->ring;
- unsigned int tail;
-
- /*
- * We know the GPU must have read the request to have
- * sent us the seqno + interrupt, so use the position
- * of tail of the request to update the last known position
- * of the GPU head.
- *
- * Note this requires that we are always called in request
- * completion order.
- */
- GEM_BUG_ON(!list_is_first(&request->ring_link, &ring->request_list));
- if (list_is_last(&request->ring_link, &ring->request_list)) {
- /*
- * We may race here with execlists resubmitting this request
- * as we retire it. The resubmission will move the ring->tail
- * forwards (to request->wa_tail). We either read the
- * current value that was written to hw, or the value that
- * is just about to be. Either works, if we miss the last two
- * noops - they are safe to be replayed on a reset.
- */
- tail = READ_ONCE(request->tail);
- list_del(&ring->active_link);
- } else {
- tail = request->postfix;
- }
- list_del_init(&request->ring_link);
-
- ring->head = tail;
-}
-
static void free_capture_list(struct i915_request *request)
{
struct i915_capture_list *capture;
@@ -231,7 +197,7 @@ static bool i915_request_retire(struct i915_request *rq)
{
struct i915_active_request *active, *next;
- lockdep_assert_held(&rq->i915->drm.struct_mutex);
+ lockdep_assert_held(&rq->timeline->mutex);
if (!i915_request_completed(rq))
return false;
@@ -243,7 +209,17 @@ static bool i915_request_retire(struct i915_request *rq)
GEM_BUG_ON(!i915_sw_fence_signaled(&rq->submit));
trace_i915_request_retire(rq);
- advance_ring(rq);
+ /*
+ * We know the GPU must have read the request to have
+ * sent us the seqno + interrupt, so use the position
+ * of tail of the request to update the last known position
+ * of the GPU head.
+ *
+ * Note this requires that we are always called in request
+ * completion order.
+ */
+ GEM_BUG_ON(!list_is_first(&rq->link, &rq->timeline->requests));
+ rq->ring->head = rq->postfix;
/*
* Walk through the active list, calling retire on each. This allows
@@ -320,7 +296,7 @@ static bool i915_request_retire(struct i915_request *rq)
void i915_request_retire_upto(struct i915_request *rq)
{
- struct intel_ring *ring = rq->ring;
+ struct intel_timeline * const tl = rq->timeline;
struct i915_request *tmp;
GEM_TRACE("%s fence %llx:%lld, current %d\n",
@@ -328,15 +304,11 @@ void i915_request_retire_upto(struct i915_request *rq)
rq->fence.context, rq->fence.seqno,
hwsp_seqno(rq));
- lockdep_assert_held(&rq->i915->drm.struct_mutex);
+ lockdep_assert_held(&tl->mutex);
GEM_BUG_ON(!i915_request_completed(rq));
- if (list_empty(&rq->ring_link))
- return;
-
do {
- tmp = list_first_entry(&ring->request_list,
- typeof(*tmp), ring_link);
+ tmp = list_first_entry(&tl->requests, typeof(*tmp), link);
} while (i915_request_retire(tmp) && tmp != rq);
}
@@ -563,29 +535,28 @@ semaphore_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
return NOTIFY_DONE;
}
-static void ring_retire_requests(struct intel_ring *ring)
+static void retire_requests(struct intel_timeline *tl)
{
struct i915_request *rq, *rn;
- list_for_each_entry_safe(rq, rn, &ring->request_list, ring_link)
+ list_for_each_entry_safe(rq, rn, &tl->requests, link)
if (!i915_request_retire(rq))
break;
}
static noinline struct i915_request *
-request_alloc_slow(struct intel_context *ce, gfp_t gfp)
+request_alloc_slow(struct intel_timeline *tl, gfp_t gfp)
{
- struct intel_ring *ring = ce->ring;
struct i915_request *rq;
- if (list_empty(&ring->request_list))
+ if (list_empty(&tl->requests))
goto out;
if (!gfpflags_allow_blocking(gfp))
goto out;
/* Move our oldest request to the slab-cache (if not in use!) */
- rq = list_first_entry(&ring->request_list, typeof(*rq), ring_link);
+ rq = list_first_entry(&tl->requests, typeof(*rq), link);
i915_request_retire(rq);
rq = kmem_cache_alloc(global.slab_requests,
@@ -594,11 +565,11 @@ request_alloc_slow(struct intel_context *ce, gfp_t gfp)
return rq;
/* Ratelimit ourselves to prevent oom from malicious clients */
- rq = list_last_entry(&ring->request_list, typeof(*rq), ring_link);
+ rq = list_last_entry(&tl->requests, typeof(*rq), link);
cond_synchronize_rcu(rq->rcustate);
/* Retire our old requests in the hope that we free some */
- ring_retire_requests(ring);
+ retire_requests(tl);
out:
return kmem_cache_alloc(global.slab_requests, gfp);
@@ -649,7 +620,7 @@ __i915_request_create(struct intel_context *ce, gfp_t gfp)
rq = kmem_cache_alloc(global.slab_requests,
gfp | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
if (unlikely(!rq)) {
- rq = request_alloc_slow(ce, gfp);
+ rq = request_alloc_slow(tl, gfp);
if (!rq) {
ret = -ENOMEM;
goto err_unreserve;
@@ -741,15 +712,15 @@ struct i915_request *
i915_request_create(struct intel_context *ce)
{
struct i915_request *rq;
- int err;
+ struct intel_timeline *tl;
- err = intel_context_timeline_lock(ce);
- if (err)
- return ERR_PTR(err);
+ tl = intel_context_timeline_lock(ce);
+ if (IS_ERR(tl))
+ return ERR_CAST(tl);
/* Move our oldest request to the slab-cache (if not in use!) */
- rq = list_first_entry(&ce->ring->request_list, typeof(*rq), ring_link);
- if (!list_is_last(&rq->ring_link, &ce->ring->request_list))
+ rq = list_first_entry(&tl->requests, typeof(*rq), link);
+ if (!list_is_last(&rq->link, &tl->requests))
i915_request_retire(rq);
intel_context_enter(ce);
@@ -759,22 +730,22 @@ i915_request_create(struct intel_context *ce)
goto err_unlock;
/* Check that we do not interrupt ourselves with a new request */
- rq->cookie = lockdep_pin_lock(&ce->ring->timeline->mutex);
+ rq->cookie = lockdep_pin_lock(&tl->mutex);
return rq;
err_unlock:
- intel_context_timeline_unlock(ce);
+ intel_context_timeline_unlock(tl);
return rq;
}
static int
i915_request_await_start(struct i915_request *rq, struct i915_request *signal)
{
- if (list_is_first(&signal->ring_link, &signal->ring->request_list))
+ if (list_is_first(&signal->link, &signal->ring->timeline->requests))
return 0;
- signal = list_prev_entry(signal, ring_link);
+ signal = list_prev_entry(signal, link);
if (intel_timeline_sync_is_later(rq->timeline, &signal->fence))
return 0;
@@ -1167,6 +1138,7 @@ struct i915_request *__i915_request_commit(struct i915_request *rq)
*/
GEM_BUG_ON(rq->reserved_space > ring->space);
rq->reserved_space = 0;
+ rq->emitted_jiffies = jiffies;
/*
* Record the position of the start of the breadcrumb so that
@@ -1180,11 +1152,6 @@ struct i915_request *__i915_request_commit(struct i915_request *rq)
prev = __i915_request_add_to_timeline(rq);
- list_add_tail(&rq->ring_link, &ring->request_list);
- if (list_is_first(&rq->ring_link, &ring->request_list))
- list_add(&ring->active_link, &rq->i915->gt.active_rings);
- rq->emitted_jiffies = jiffies;
-
/*
* Let the backend know a new request has arrived that may need
* to adjust the existing execution schedule due to a high priority
@@ -1237,10 +1204,11 @@ struct i915_request *__i915_request_commit(struct i915_request *rq)
void i915_request_add(struct i915_request *rq)
{
+ struct intel_timeline * const tl = rq->timeline;
struct i915_request *prev;
- lockdep_assert_held(&rq->timeline->mutex);
- lockdep_unpin_lock(&rq->timeline->mutex, rq->cookie);
+ lockdep_assert_held(&tl->mutex);
+ lockdep_unpin_lock(&tl->mutex, rq->cookie);
trace_i915_request_add(rq);
@@ -1263,10 +1231,10 @@ void i915_request_add(struct i915_request *rq)
* work on behalf of others -- but instead we should benefit from
* improved resource management. (Well, that's the theory at least.)
*/
- if (prev && i915_request_completed(prev))
+ if (prev && i915_request_completed(prev) && prev->timeline == tl)
i915_request_retire_upto(prev);
- mutex_unlock(&rq->timeline->mutex);
+ mutex_unlock(&tl->mutex);
}
static unsigned long local_clock_us(unsigned int *cpu)
@@ -1487,18 +1455,43 @@ long i915_request_wait(struct i915_request *rq,
bool i915_retire_requests(struct drm_i915_private *i915)
{
- struct intel_ring *ring, *tmp;
+ struct intel_gt_timelines *timelines = &i915->gt.timelines;
+ struct intel_timeline *tl, *tn;
+ LIST_HEAD(free);
+
+ spin_lock(&timelines->lock);
+ list_for_each_entry_safe(tl, tn, &timelines->active_list, link) {
+ if (!mutex_trylock(&tl->mutex))
+ continue;
+
+ intel_timeline_get(tl);
+ GEM_BUG_ON(!tl->active_count);
+ tl->active_count++; /* pin the list element */
+ spin_unlock(&timelines->lock);
- lockdep_assert_held(&i915->drm.struct_mutex);
+ retire_requests(tl);
- list_for_each_entry_safe(ring, tmp,
- &i915->gt.active_rings, active_link) {
- intel_ring_get(ring); /* last rq holds reference! */
- ring_retire_requests(ring);
- intel_ring_put(ring);
+ spin_lock(&timelines->lock);
+
+ /* Restart iteration after dropping lock */
+ list_safe_reset_next(tl, tn, link);
+ if (!--tl->active_count)
+ list_del(&tl->link);
+
+ mutex_unlock(&tl->mutex);
+
+ /* Defer the final release to after the spinlock */
+ if (refcount_dec_and_test(&tl->kref.refcount)) {
+ GEM_BUG_ON(tl->active_count);
+ list_add(&tl->link, &free);
+ }
}
+ spin_unlock(&timelines->lock);
+
+ list_for_each_entry_safe(tl, tn, &free, link)
+ __intel_timeline_free(&tl->kref);
- return !list_empty(&i915->gt.active_rings);
+ return !list_empty(&timelines->active_list);
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index b58ceef92e20..a6b1e5f43949 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -221,9 +221,6 @@ struct i915_request {
/** timeline->request entry for this request */
struct list_head link;
- /** ring->request_list entry for this request */
- struct list_head ring_link;
-
struct drm_i915_file_private *file_priv;
/** file_priv list entry for this request */
struct list_head client_link;
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 15/15] drm/i915: Replace struct_mutex for batch pool serialisation
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (12 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 14/15] drm/i915: Protect request retirement with timeline->mutex Chris Wilson
@ 2019-07-03 9:17 ` Chris Wilson
2019-07-03 9:41 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/15] drm/i915/selftests: Common live setup/teardown Patchwork
` (4 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
Switch to tracking activity via i915_active on individual nodes, only
keeping a list of retired objects in the cache, and reaping the cache
when the engine itself idles.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/Makefile | 2 +-
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 58 +++---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 1 -
.../gpu/drm/i915/gem/i915_gem_object_types.h | 1 -
drivers/gpu/drm/i915/gem/i915_gem_pm.c | 4 +-
drivers/gpu/drm/i915/gt/intel_engine.h | 1 -
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 11 +-
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 2 +
drivers/gpu/drm/i915/gt/intel_engine_pool.c | 166 ++++++++++++++++++
drivers/gpu/drm/i915/gt/intel_engine_pool.h | 34 ++++
.../gpu/drm/i915/gt/intel_engine_pool_types.h | 29 +++
drivers/gpu/drm/i915/gt/intel_engine_types.h | 6 +-
drivers/gpu/drm/i915/gt/mock_engine.c | 3 +
drivers/gpu/drm/i915/i915_debugfs.c | 68 -------
drivers/gpu/drm/i915/i915_gem_batch_pool.c | 132 --------------
drivers/gpu/drm/i915/i915_gem_batch_pool.h | 26 ---
16 files changed, 279 insertions(+), 265 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_pool.c
create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_pool.h
create mode 100644 drivers/gpu/drm/i915/gt/intel_engine_pool_types.h
delete mode 100644 drivers/gpu/drm/i915/i915_gem_batch_pool.c
delete mode 100644 drivers/gpu/drm/i915/i915_gem_batch_pool.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 82c49ad16361..ce8db7b019f9 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -72,6 +72,7 @@ obj-y += gt/
gt-y += \
gt/intel_breadcrumbs.o \
gt/intel_context.o \
+ gt/intel_engine_pool.o \
gt/intel_engine_cs.o \
gt/intel_engine_pm.o \
gt/intel_gt.o \
@@ -118,7 +119,6 @@ i915-y += \
$(gem-y) \
i915_active.o \
i915_cmd_parser.o \
- i915_gem_batch_pool.o \
i915_gem_evict.o \
i915_gem_fence_reg.o \
i915_gem_gtt.o \
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 80c9c57a302f..0ea2d49bc8b9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -16,6 +16,7 @@
#include "gem/i915_gem_ioctls.h"
#include "gt/intel_context.h"
+#include "gt/intel_engine_pool.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_pm.h"
@@ -1145,25 +1146,26 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
unsigned int len)
{
struct reloc_cache *cache = &eb->reloc_cache;
- struct drm_i915_gem_object *obj;
+ struct intel_engine_pool_node *pool;
struct i915_request *rq;
struct i915_vma *batch;
u32 *cmd;
int err;
- obj = i915_gem_batch_pool_get(&eb->engine->batch_pool, PAGE_SIZE);
- if (IS_ERR(obj))
- return PTR_ERR(obj);
+ pool = intel_engine_pool_get(&eb->engine->pool, PAGE_SIZE);
+ if (IS_ERR(pool))
+ return PTR_ERR(pool);
- cmd = i915_gem_object_pin_map(obj,
+ cmd = i915_gem_object_pin_map(pool->obj,
cache->has_llc ?
I915_MAP_FORCE_WB :
I915_MAP_FORCE_WC);
- i915_gem_object_unpin_pages(obj);
- if (IS_ERR(cmd))
- return PTR_ERR(cmd);
+ if (IS_ERR(cmd)) {
+ err = PTR_ERR(cmd);
+ goto out_pool;
+ }
- batch = i915_vma_instance(obj, vma->vm, NULL);
+ batch = i915_vma_instance(pool->obj, vma->vm, NULL);
if (IS_ERR(batch)) {
err = PTR_ERR(batch);
goto err_unmap;
@@ -1179,6 +1181,10 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
goto err_unpin;
}
+ err = intel_engine_pool_mark_active(pool, rq);
+ if (err)
+ goto err_request;
+
err = reloc_move_to_gpu(rq, vma);
if (err)
goto err_request;
@@ -1204,7 +1210,7 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
cache->rq_size = 0;
/* Return with batch mapping (cmd) still pinned */
- return 0;
+ goto out_pool;
skip_request:
i915_request_skip(rq, err);
@@ -1213,7 +1219,9 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
err_unpin:
i915_vma_unpin(batch);
err_unmap:
- i915_gem_object_unpin_map(obj);
+ i915_gem_object_unpin_map(pool->obj);
+out_pool:
+ intel_engine_pool_put(pool);
return err;
}
@@ -1957,18 +1965,17 @@ static int i915_reset_gen7_sol_offsets(struct i915_request *rq)
static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
{
- struct drm_i915_gem_object *shadow_batch_obj;
+ struct intel_engine_pool_node *pool;
struct i915_vma *vma;
int err;
- shadow_batch_obj = i915_gem_batch_pool_get(&eb->engine->batch_pool,
- PAGE_ALIGN(eb->batch_len));
- if (IS_ERR(shadow_batch_obj))
- return ERR_CAST(shadow_batch_obj);
+ pool = intel_engine_pool_get(&eb->engine->pool, eb->batch_len);
+ if (IS_ERR(pool))
+ return ERR_CAST(pool);
err = intel_engine_cmd_parser(eb->engine,
eb->batch->obj,
- shadow_batch_obj,
+ pool->obj,
eb->batch_start_offset,
eb->batch_len,
is_master);
@@ -1977,12 +1984,12 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
vma = NULL;
else
vma = ERR_PTR(err);
- goto out;
+ goto err;
}
- vma = i915_gem_object_ggtt_pin(shadow_batch_obj, NULL, 0, 0, 0);
+ vma = i915_gem_object_ggtt_pin(pool->obj, NULL, 0, 0, 0);
if (IS_ERR(vma))
- goto out;
+ goto err;
eb->vma[eb->buffer_count] = i915_vma_get(vma);
eb->flags[eb->buffer_count] =
@@ -1990,8 +1997,11 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
vma->exec_flags = &eb->flags[eb->buffer_count];
eb->buffer_count++;
-out:
- i915_gem_object_unpin_pages(shadow_batch_obj);
+ vma->private = pool;
+ return vma;
+
+err:
+ intel_engine_pool_put(pool);
return vma;
}
@@ -2615,6 +2625,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
* to explicitly hold another reference here.
*/
eb.request->batch = eb.batch;
+ if (eb.batch->private)
+ intel_engine_pool_mark_active(eb.batch->private, eb.request);
trace_i915_request_queue(eb.request, eb.batch_flags);
err = eb_submit(&eb);
@@ -2639,6 +2651,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
err_batch_unpin:
if (eb.batch_flags & I915_DISPATCH_SECURE)
i915_vma_unpin(eb.batch);
+ if (eb.batch->private)
+ intel_engine_pool_put(eb.batch->private);
err_vma:
if (eb.exec)
eb_release_vmas(&eb);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index d3e96f09c6b7..e580ea4c734d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -64,7 +64,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
INIT_LIST_HEAD(&obj->vma.list);
INIT_LIST_HEAD(&obj->lut_list);
- INIT_LIST_HEAD(&obj->batch_pool_link);
init_rcu_head(&obj->rcu);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 34b51fad02de..d474c6ac4100 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -114,7 +114,6 @@ struct drm_i915_gem_object {
unsigned int userfault_count;
struct list_head userfault_link;
- struct list_head batch_pool_link;
I915_SELFTEST_DECLARE(struct list_head st_link);
/*
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 93d188526457..bf085b0cb7c6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -33,10 +33,8 @@ static void i915_gem_park(struct drm_i915_private *i915)
lockdep_assert_held(&i915->drm.struct_mutex);
- for_each_engine(engine, i915, id) {
+ for_each_engine(engine, i915, id)
call_idle_barriers(engine); /* cleanup after wedging */
- i915_gem_batch_pool_fini(&engine->batch_pool);
- }
i915_vma_parked(i915);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index 0331e9ac2485..faaa164267f4 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -9,7 +9,6 @@
#include <linux/random.h>
#include <linux/seqlock.h>
-#include "i915_gem_batch_pool.h"
#include "i915_pmu.h"
#include "i915_reg.h"
#include "i915_request.h"
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index b27fc555fe09..49439cf2fd1f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -32,6 +32,7 @@
#include "intel_engine.h"
#include "intel_engine_pm.h"
+#include "intel_engine_pool.h"
#include "intel_context.h"
#include "intel_lrc.h"
#include "intel_reset.h"
@@ -498,11 +499,6 @@ int intel_engines_init(struct drm_i915_private *i915)
return err;
}
-static void intel_engine_init_batch_pool(struct intel_engine_cs *engine)
-{
- i915_gem_batch_pool_init(&engine->batch_pool, engine);
-}
-
void intel_engine_init_execlists(struct intel_engine_cs *engine)
{
struct intel_engine_execlists * const execlists = &engine->execlists;
@@ -628,10 +624,11 @@ static int intel_engine_setup_common(struct intel_engine_cs *engine)
intel_engine_init_breadcrumbs(engine);
intel_engine_init_execlists(engine);
intel_engine_init_hangcheck(engine);
- intel_engine_init_batch_pool(engine);
intel_engine_init_cmd_parser(engine);
intel_engine_init__pm(engine);
+ intel_engine_pool_init(&engine->pool);
+
/* Use the whole device by default */
engine->sseu =
intel_sseu_from_device_info(&RUNTIME_INFO(engine->i915)->sseu);
@@ -880,9 +877,9 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
cleanup_status_page(engine);
+ intel_engine_pool_fini(&engine->pool);
intel_engine_fini_breadcrumbs(engine);
intel_engine_cleanup_cmd_parser(engine);
- i915_gem_batch_pool_fini(&engine->batch_pool);
if (engine->default_state)
i915_gem_object_put(engine->default_state);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 9751a02d86bc..fe9f9eaffe88 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -7,6 +7,7 @@
#include "i915_drv.h"
#include "intel_engine.h"
+#include "intel_engine_pool.h"
#include "intel_engine_pm.h"
#include "intel_gt_pm.h"
@@ -116,6 +117,7 @@ static int __engine_park(struct intel_wakeref *wf)
GEM_TRACE("%s\n", engine->name);
intel_engine_disarm_breadcrumbs(engine);
+ intel_engine_pool_park(&engine->pool);
/* Must be reset upon idling, or we may miss the busy wakeup. */
GEM_BUG_ON(engine->execlists.queue_priority_hint != INT_MIN);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pool.c b/drivers/gpu/drm/i915/gt/intel_engine_pool.c
new file mode 100644
index 000000000000..32688ca379ef
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pool.c
@@ -0,0 +1,166 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2014-2018 Intel Corporation
+ */
+
+#include "gem/i915_gem_object.h"
+
+#include "i915_drv.h"
+#include "intel_engine_pm.h"
+#include "intel_engine_pool.h"
+
+static struct intel_engine_cs *to_engine(struct intel_engine_pool *pool)
+{
+ return container_of(pool, struct intel_engine_cs, pool);
+}
+
+static struct list_head *
+bucket_for_size(struct intel_engine_pool *pool, size_t sz)
+{
+ int n;
+
+ /*
+ * Compute a power-of-two bucket, but throw everything greater than
+ * 16KiB into the same bucket: i.e. the buckets hold objects of
+ * (1 page, 2 pages, 4 pages, 8+ pages).
+ */
+ n = fls(sz >> PAGE_SHIFT) - 1;
+ if (n >= ARRAY_SIZE(pool->cache_list))
+ n = ARRAY_SIZE(pool->cache_list) - 1;
+
+ return &pool->cache_list[n];
+}
+
+static void node_free(struct intel_engine_pool_node *node)
+{
+ i915_gem_object_put(node->obj);
+ i915_active_fini(&node->active);
+ kfree(node);
+}
+
+static int pool_active(struct i915_active *ref)
+{
+ struct intel_engine_pool_node *node =
+ container_of(ref, typeof(*node), active);
+ struct reservation_object *resv = node->obj->base.resv;
+
+ if (reservation_object_trylock(resv)) {
+ reservation_object_add_excl_fence(resv, NULL);
+ reservation_object_unlock(resv);
+ }
+
+ return i915_gem_object_pin_pages(node->obj);
+}
+
+static void pool_retire(struct i915_active *ref)
+{
+ struct intel_engine_pool_node *node =
+ container_of(ref, typeof(*node), active);
+ struct intel_engine_pool *pool = node->pool;
+ struct list_head *list = bucket_for_size(pool, node->obj->base.size);
+ unsigned long flags;
+
+ GEM_BUG_ON(!intel_engine_pm_is_awake(to_engine(pool)));
+
+ i915_gem_object_unpin_pages(node->obj);
+
+ spin_lock_irqsave(&pool->lock, flags);
+ list_add(&node->link, list);
+ spin_unlock_irqrestore(&pool->lock, flags);
+}
+
+static struct intel_engine_pool_node *
+node_create(struct intel_engine_pool *pool, size_t sz)
+{
+ struct intel_engine_cs *engine = to_engine(pool);
+ struct intel_engine_pool_node *node;
+ struct drm_i915_gem_object *obj;
+
+ node = kmalloc(sizeof(*node),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
+ if (!node)
+ return ERR_PTR(-ENOMEM);
+
+ node->pool = pool;
+ i915_active_init(engine->i915, &node->active, pool_active, pool_retire);
+
+ obj = i915_gem_object_create_internal(engine->i915, sz);
+ if (IS_ERR(obj)) {
+ i915_active_fini(&node->active);
+ kfree(node);
+ return ERR_CAST(obj);
+ }
+
+ node->obj = obj;
+ return node;
+}
+
+struct intel_engine_pool_node *
+intel_engine_pool_get(struct intel_engine_pool *pool, size_t size)
+{
+ struct intel_engine_pool_node *node;
+ struct list_head *list;
+ unsigned long flags;
+ int ret;
+
+ GEM_BUG_ON(!intel_engine_pm_is_awake(to_engine(pool)));
+
+ size = PAGE_ALIGN(size);
+ list = bucket_for_size(pool, size);
+
+ spin_lock_irqsave(&pool->lock, flags);
+ list_for_each_entry(node, list, link) {
+ if (node->obj->base.size < size)
+ continue;
+ list_del(&node->link);
+ break;
+ }
+ spin_unlock_irqrestore(&pool->lock, flags);
+
+ if (&node->link == list) {
+ node = node_create(pool, size);
+ if (IS_ERR(node))
+ return node;
+ }
+
+ ret = i915_active_acquire(&node->active);
+ if (ret) {
+ node_free(node);
+ return ERR_PTR(ret);
+ }
+
+ return node;
+}
+
+void intel_engine_pool_init(struct intel_engine_pool *pool)
+{
+ int n;
+
+ spin_lock_init(&pool->lock);
+ for (n = 0; n < ARRAY_SIZE(pool->cache_list); n++)
+ INIT_LIST_HEAD(&pool->cache_list[n]);
+}
+
+void intel_engine_pool_park(struct intel_engine_pool *pool)
+{
+ int n;
+
+ for (n = 0; n < ARRAY_SIZE(pool->cache_list); n++) {
+ struct list_head *list = &pool->cache_list[n];
+ struct intel_engine_pool_node *node, *nn;
+
+ list_for_each_entry_safe(node, nn, list, link)
+ node_free(node);
+
+ INIT_LIST_HEAD(list);
+ }
+}
+
+void intel_engine_pool_fini(struct intel_engine_pool *pool)
+{
+ int n;
+
+ for (n = 0; n < ARRAY_SIZE(pool->cache_list); n++)
+ GEM_BUG_ON(!list_empty(&pool->cache_list[n]));
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pool.h b/drivers/gpu/drm/i915/gt/intel_engine_pool.h
new file mode 100644
index 000000000000..f7a0a660c1c9
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pool.h
@@ -0,0 +1,34 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2014-2018 Intel Corporation
+ */
+
+#ifndef INTEL_ENGINE_POOL_H
+#define INTEL_ENGINE_POOL_H
+
+#include "intel_engine_pool_types.h"
+#include "i915_active.h"
+#include "i915_request.h"
+
+struct intel_engine_pool_node *
+intel_engine_pool_get(struct intel_engine_pool *pool, size_t size);
+
+static inline int
+intel_engine_pool_mark_active(struct intel_engine_pool_node *node,
+ struct i915_request *rq)
+{
+ return i915_active_ref(&node->active, rq->fence.context, rq);
+}
+
+static inline void
+intel_engine_pool_put(struct intel_engine_pool_node *node)
+{
+ i915_active_release(&node->active);
+}
+
+void intel_engine_pool_init(struct intel_engine_pool *pool);
+void intel_engine_pool_park(struct intel_engine_pool *pool);
+void intel_engine_pool_fini(struct intel_engine_pool *pool);
+
+#endif /* INTEL_ENGINE_POOL_H */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pool_types.h b/drivers/gpu/drm/i915/gt/intel_engine_pool_types.h
new file mode 100644
index 000000000000..e31ee361b76f
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pool_types.h
@@ -0,0 +1,29 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2014-2018 Intel Corporation
+ */
+
+#ifndef INTEL_ENGINE_POOL_TYPES_H
+#define INTEL_ENGINE_POOL_TYPES_H
+
+#include <linux/list.h>
+#include <linux/spinlock.h>
+
+#include "i915_active_types.h"
+
+struct drm_i915_gem_object;
+
+struct intel_engine_pool {
+ spinlock_t lock;
+ struct list_head cache_list[4];
+};
+
+struct intel_engine_pool_node {
+ struct i915_active active;
+ struct drm_i915_gem_object *obj;
+ struct list_head link;
+ struct intel_engine_pool *pool;
+};
+
+#endif /* INTEL_ENGINE_POOL_TYPES_H */
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 0dde7e04b102..6d2f3e11da1c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -16,12 +16,12 @@
#include <linux/types.h>
#include "i915_gem.h"
-#include "i915_gem_batch_pool.h"
#include "i915_pmu.h"
#include "i915_priolist_types.h"
#include "i915_selftest.h"
-#include "gt/intel_timeline_types.h"
+#include "intel_engine_pool_types.h"
#include "intel_sseu.h"
+#include "intel_timeline_types.h"
#include "intel_wakeref.h"
#include "intel_workarounds_types.h"
@@ -353,7 +353,7 @@ struct intel_engine_cs {
* when the command parser is enabled. Prevents the client from
* modifying the batch contents after software parsing.
*/
- struct i915_gem_batch_pool batch_pool;
+ struct intel_engine_pool pool;
struct intel_hw_status_page status_page;
struct i915_ctx_workarounds wa_ctx;
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index 5bcb461b8372..b94d57bf2c48 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -27,6 +27,7 @@
#include "i915_drv.h"
#include "intel_context.h"
#include "intel_engine_pm.h"
+#include "intel_engine_pool.h"
#include "mock_engine.h"
#include "selftests/mock_request.h"
@@ -291,6 +292,8 @@ int mock_engine_init(struct intel_engine_cs *engine)
intel_engine_init_execlists(engine);
intel_engine_init__pm(engine);
+ intel_engine_pool_init(&engine->pool);
+
engine->kernel_context =
i915_gem_context_get_engine(i915->kernel_context, engine->id);
if (IS_ERR(engine->kernel_context))
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index eeecdad0e3ca..253e86868061 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -295,27 +295,6 @@ static int per_file_stats(int id, void *ptr, void *data)
stats.closed); \
} while (0)
-static void print_batch_pool_stats(struct seq_file *m,
- struct drm_i915_private *dev_priv)
-{
- struct drm_i915_gem_object *obj;
- struct intel_engine_cs *engine;
- struct file_stats stats = {};
- enum intel_engine_id id;
- int j;
-
- for_each_engine(engine, dev_priv, id) {
- for (j = 0; j < ARRAY_SIZE(engine->batch_pool.cache_list); j++) {
- list_for_each_entry(obj,
- &engine->batch_pool.cache_list[j],
- batch_pool_link)
- per_file_stats(0, obj, &stats);
- }
- }
-
- print_file_stats(m, "[k]batch pool", stats);
-}
-
static void print_context_stats(struct seq_file *m,
struct drm_i915_private *i915)
{
@@ -373,58 +352,12 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
if (ret)
return ret;
- print_batch_pool_stats(m, i915);
print_context_stats(m, i915);
mutex_unlock(&i915->drm.struct_mutex);
return 0;
}
-static int i915_gem_batch_pool_info(struct seq_file *m, void *data)
-{
- struct drm_i915_private *dev_priv = node_to_i915(m->private);
- struct drm_device *dev = &dev_priv->drm;
- struct drm_i915_gem_object *obj;
- struct intel_engine_cs *engine;
- enum intel_engine_id id;
- int total = 0;
- int ret, j;
-
- ret = mutex_lock_interruptible(&dev->struct_mutex);
- if (ret)
- return ret;
-
- for_each_engine(engine, dev_priv, id) {
- for (j = 0; j < ARRAY_SIZE(engine->batch_pool.cache_list); j++) {
- int count;
-
- count = 0;
- list_for_each_entry(obj,
- &engine->batch_pool.cache_list[j],
- batch_pool_link)
- count++;
- seq_printf(m, "%s cache[%d]: %d objects\n",
- engine->name, j, count);
-
- list_for_each_entry(obj,
- &engine->batch_pool.cache_list[j],
- batch_pool_link) {
- seq_puts(m, " ");
- describe_obj(m, obj);
- seq_putc(m, '\n');
- }
-
- total += count;
- }
- }
-
- seq_printf(m, "total: %d\n", total);
-
- mutex_unlock(&dev->struct_mutex);
-
- return 0;
-}
-
static void gen8_display_interrupt_info(struct seq_file *m)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4364,7 +4297,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_gem_objects", i915_gem_object_info, 0},
{"i915_gem_fence_regs", i915_gem_fence_regs_info, 0},
{"i915_gem_interrupt", i915_interrupt_info, 0},
- {"i915_gem_batch_pool", i915_gem_batch_pool_info, 0},
{"i915_guc_info", i915_guc_info, 0},
{"i915_guc_load_status", i915_guc_load_status_info, 0},
{"i915_guc_log_dump", i915_guc_log_dump, 0},
diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
deleted file mode 100644
index b17f23991253..000000000000
--- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c
+++ /dev/null
@@ -1,132 +0,0 @@
-/*
- * SPDX-License-Identifier: MIT
- *
- * Copyright © 2014-2018 Intel Corporation
- */
-
-#include "i915_gem_batch_pool.h"
-#include "i915_drv.h"
-
-/**
- * DOC: batch pool
- *
- * In order to submit batch buffers as 'secure', the software command parser
- * must ensure that a batch buffer cannot be modified after parsing. It does
- * this by copying the user provided batch buffer contents to a kernel owned
- * buffer from which the hardware will actually execute, and by carefully
- * managing the address space bindings for such buffers.
- *
- * The batch pool framework provides a mechanism for the driver to manage a
- * set of scratch buffers to use for this purpose. The framework can be
- * extended to support other uses cases should they arise.
- */
-
-/**
- * i915_gem_batch_pool_init() - initialize a batch buffer pool
- * @pool: the batch buffer pool
- * @engine: the associated request submission engine
- */
-void i915_gem_batch_pool_init(struct i915_gem_batch_pool *pool,
- struct intel_engine_cs *engine)
-{
- int n;
-
- pool->engine = engine;
-
- for (n = 0; n < ARRAY_SIZE(pool->cache_list); n++)
- INIT_LIST_HEAD(&pool->cache_list[n]);
-}
-
-/**
- * i915_gem_batch_pool_fini() - clean up a batch buffer pool
- * @pool: the pool to clean up
- *
- * Note: Callers must hold the struct_mutex.
- */
-void i915_gem_batch_pool_fini(struct i915_gem_batch_pool *pool)
-{
- int n;
-
- lockdep_assert_held(&pool->engine->i915->drm.struct_mutex);
-
- for (n = 0; n < ARRAY_SIZE(pool->cache_list); n++) {
- struct drm_i915_gem_object *obj, *next;
-
- list_for_each_entry_safe(obj, next,
- &pool->cache_list[n],
- batch_pool_link)
- i915_gem_object_put(obj);
-
- INIT_LIST_HEAD(&pool->cache_list[n]);
- }
-}
-
-/**
- * i915_gem_batch_pool_get() - allocate a buffer from the pool
- * @pool: the batch buffer pool
- * @size: the minimum desired size of the returned buffer
- *
- * Returns an inactive buffer from @pool with at least @size bytes,
- * with the pages pinned. The caller must i915_gem_object_unpin_pages()
- * on the returned object.
- *
- * Note: Callers must hold the struct_mutex
- *
- * Return: the buffer object or an error pointer
- */
-struct drm_i915_gem_object *
-i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool,
- size_t size)
-{
- struct drm_i915_gem_object *obj;
- struct list_head *list;
- int n, ret;
-
- lockdep_assert_held(&pool->engine->i915->drm.struct_mutex);
-
- /* Compute a power-of-two bucket, but throw everything greater than
- * 16KiB into the same bucket: i.e. the the buckets hold objects of
- * (1 page, 2 pages, 4 pages, 8+ pages).
- */
- n = fls(size >> PAGE_SHIFT) - 1;
- if (n >= ARRAY_SIZE(pool->cache_list))
- n = ARRAY_SIZE(pool->cache_list) - 1;
- list = &pool->cache_list[n];
-
- list_for_each_entry(obj, list, batch_pool_link) {
- struct reservation_object *resv = obj->base.resv;
-
- /* The batches are strictly LRU ordered */
- if (!reservation_object_test_signaled_rcu(resv, true))
- break;
-
- /*
- * The object is now idle, clear the array of shared
- * fences before we add a new request. Although, we
- * remain on the same engine, we may be on a different
- * timeline and so may continually grow the array,
- * trapping a reference to all the old fences, rather
- * than replace the existing fence.
- */
- if (rcu_access_pointer(resv->fence)) {
- reservation_object_lock(resv, NULL);
- reservation_object_add_excl_fence(resv, NULL);
- reservation_object_unlock(resv);
- }
-
- if (obj->base.size >= size)
- goto found;
- }
-
- obj = i915_gem_object_create_internal(pool->engine->i915, size);
- if (IS_ERR(obj))
- return obj;
-
-found:
- ret = i915_gem_object_pin_pages(obj);
- if (ret)
- return ERR_PTR(ret);
-
- list_move_tail(&obj->batch_pool_link, list);
- return obj;
-}
diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.h b/drivers/gpu/drm/i915/i915_gem_batch_pool.h
deleted file mode 100644
index feeeeeaa54d8..000000000000
--- a/drivers/gpu/drm/i915/i915_gem_batch_pool.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * SPDX-License-Identifier: MIT
- *
- * Copyright © 2014-2018 Intel Corporation
- */
-
-#ifndef I915_GEM_BATCH_POOL_H
-#define I915_GEM_BATCH_POOL_H
-
-#include <linux/types.h>
-
-struct drm_i915_gem_object;
-struct intel_engine_cs;
-
-struct i915_gem_batch_pool {
- struct intel_engine_cs *engine;
- struct list_head cache_list[4];
-};
-
-void i915_gem_batch_pool_init(struct i915_gem_batch_pool *pool,
- struct intel_engine_cs *engine);
-void i915_gem_batch_pool_fini(struct i915_gem_batch_pool *pool);
-struct drm_i915_gem_object *
-i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, size_t size);
-
-#endif /* I915_GEM_BATCH_POOL_H */
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/15] drm/i915/selftests: Common live setup/teardown
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (13 preceding siblings ...)
2019-07-03 9:17 ` [PATCH 15/15] drm/i915: Replace struct_mutex for batch pool serialisation Chris Wilson
@ 2019-07-03 9:41 ` Patchwork
2019-07-03 9:47 ` ✗ Fi.CI.SPARSE: " Patchwork
` (3 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2019-07-03 9:41 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/15] drm/i915/selftests: Common live setup/teardown
URL : https://patchwork.freedesktop.org/series/63114/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
f3a7eda391b2 drm/i915/selftests: Common live setup/teardown
-:239: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'T' - possible side-effects?
#239: FILE: drivers/gpu/drm/i915/i915_selftest.h:85:
+#define i915_live_subtests(T, data) ({ \
+ typecheck(struct drm_i915_private *, data); \
+ __i915_subtests(__func__, \
+ __i915_live_setup, __i915_live_teardown, \
+ T, ARRAY_SIZE(T), data); \
+})
total: 0 errors, 0 warnings, 1 checks, 273 lines checked
e3344fd5f501 drm/i915/selftests: Lock the drm_mm while modifying
98aef619b577 drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine
dd411766aecd drm/i915/gt: Assume we hold forcewake for execlists resume
54651a5c97e5 drm/i915/gt: Ignore forcewake acquisition for posting_reads
d85fcd7ec654 drm/i915/gem: Free pages before rcu-freeing the object
746cdb378dcd drm/i915: Markup potential lock for i915_active
8094b6f9bbd7 drm/i915: Mark up vma->active as safe for use inside shrinkers
094947492d58 drm/i915/execlists: Hesitate before slicing
c70ab2f0eeaf drm/i915: Teach execbuffer to take the engine wakeref not GT
cbf288ea92dd drm/i915/gt: Track timeline activeness in enter/exit
fec91822fa4b drm/i915/gt: Convert timeline tracking to spinlock
ccb245053e23 drm/i915/gt: Guard timeline pinning with its own mutex
d2991ee34d19 drm/i915: Protect request retirement with timeline->mutex
c74efbfd4cb8 drm/i915: Replace struct_mutex for batch pool serialisation
-:305: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#305:
new file mode 100644
-:310: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#310: FILE: drivers/gpu/drm/i915/gt/intel_engine_pool.c:1:
+/*
-:311: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#311: FILE: drivers/gpu/drm/i915/gt/intel_engine_pool.c:2:
+ * SPDX-License-Identifier: MIT
-:482: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#482: FILE: drivers/gpu/drm/i915/gt/intel_engine_pool.h:1:
+/*
-:483: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#483: FILE: drivers/gpu/drm/i915/gt/intel_engine_pool.h:2:
+ * SPDX-License-Identifier: MIT
-:522: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#522: FILE: drivers/gpu/drm/i915/gt/intel_engine_pool_types.h:1:
+/*
-:523: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#523: FILE: drivers/gpu/drm/i915/gt/intel_engine_pool_types.h:2:
+ * SPDX-License-Identifier: MIT
-:539: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment
#539: FILE: drivers/gpu/drm/i915/gt/intel_engine_pool_types.h:18:
+ spinlock_t lock;
total: 0 errors, 7 warnings, 1 checks, 595 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* ✗ Fi.CI.SPARSE: warning for series starting with [01/15] drm/i915/selftests: Common live setup/teardown
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (14 preceding siblings ...)
2019-07-03 9:41 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/15] drm/i915/selftests: Common live setup/teardown Patchwork
@ 2019-07-03 9:47 ` Patchwork
2019-07-03 9:48 ` [PATCH 01/15] " Matthew Auld
` (2 subsequent siblings)
18 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2019-07-03 9:47 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/15] drm/i915/selftests: Common live setup/teardown
URL : https://patchwork.freedesktop.org/series/63114/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/selftests: Common live setup/teardown
Okay!
Commit: drm/i915/selftests: Lock the drm_mm while modifying
Okay!
Commit: drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine
Okay!
Commit: drm/i915/gt: Assume we hold forcewake for execlists resume
Okay!
Commit: drm/i915/gt: Ignore forcewake acquisition for posting_reads
Okay!
Commit: drm/i915/gem: Free pages before rcu-freeing the object
Okay!
Commit: drm/i915: Markup potential lock for i915_active
Okay!
Commit: drm/i915: Mark up vma->active as safe for use inside shrinkers
Okay!
Commit: drm/i915/execlists: Hesitate before slicing
-O:drivers/gpu/drm/i915/gt/intel_lrc.c:899:16: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/gt/intel_lrc.c:899:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/gt/intel_lrc.c:899:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/gt/intel_lrc.c:899:16: warning: expression using sizeof(void)
Commit: drm/i915: Teach execbuffer to take the engine wakeref not GT
Okay!
Commit: drm/i915/gt: Track timeline activeness in enter/exit
Okay!
Commit: drm/i915/gt: Convert timeline tracking to spinlock
Okay!
Commit: drm/i915/gt: Guard timeline pinning with its own mutex
Okay!
Commit: drm/i915: Protect request retirement with timeline->mutex
Okay!
Commit: drm/i915: Replace struct_mutex for batch pool serialisation
+./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 01/15] drm/i915/selftests: Common live setup/teardown
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (15 preceding siblings ...)
2019-07-03 9:47 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-07-03 9:48 ` Matthew Auld
2019-07-03 10:04 ` Chris Wilson
2019-07-03 9:59 ` ✓ Fi.CI.BAT: success for series starting with [01/15] " Patchwork
2019-07-04 3:22 ` ✗ Fi.CI.IGT: failure " Patchwork
18 siblings, 1 reply; 31+ messages in thread
From: Matthew Auld @ 2019-07-03 9:48 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 03/07/2019 10:17, Chris Wilson wrote:
> We frequently, and not frequently enough, remember to flush residual
> openations and objects at the end of a live subtest. The purpose is to
operations
> cleanup after every subtest, leaving a clean slate for the next subtest,
> and perform early detection of leaky state.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
A variant with separate arguments for data and i915? Meh.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 01/15] drm/i915/selftests: Common live setup/teardown
2019-07-03 9:48 ` [PATCH 01/15] " Matthew Auld
@ 2019-07-03 10:04 ` Chris Wilson
0 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2019-07-03 10:04 UTC (permalink / raw)
To: Matthew Auld, intel-gfx
Quoting Matthew Auld (2019-07-03 10:48:28)
> On 03/07/2019 10:17, Chris Wilson wrote:
> > We frequently, and not frequently enough, remember to flush residual
> > openations and objects at the end of a live subtest. The purpose is to
>
> operations
>
> > cleanup after every subtest, leaving a clean slate for the next subtest,
> > and perform early detection of leaky state.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>
> A variant with separate arguments for data and i915? Meh.
My first attempt was to use
struct i915_live_data {
struct drm_i915_private *i915;
};
but for this pass everything is basically using i915 as the data
parameter, so went with that for a smaller patch.
It may also be interesting to a do a i915_mock_subtests() that creates
and destroy a mock_gem_device around each subtest. I am sure we will
start using setup/teardown more creatively in future :)
Also remind me to pay attention to kunit and see if we can have a smooth
transition over to a central framework.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [01/15] drm/i915/selftests: Common live setup/teardown
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (16 preceding siblings ...)
2019-07-03 9:48 ` [PATCH 01/15] " Matthew Auld
@ 2019-07-03 9:59 ` Patchwork
2019-07-04 3:22 ` ✗ Fi.CI.IGT: failure " Patchwork
18 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2019-07-03 9:59 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/15] drm/i915/selftests: Common live setup/teardown
URL : https://patchwork.freedesktop.org/series/63114/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6399 -> Patchwork_13503
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/
Known issues
------------
Here are the changes found in Patchwork_13503 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_exec@basic:
- fi-icl-guc: [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-icl-guc/igt@gem_ctx_exec@basic.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-icl-guc/igt@gem_ctx_exec@basic.html
* igt@gem_exec_reloc@basic-write-gtt-noreloc:
- fi-icl-u3: [PASS][3] -> [DMESG-WARN][4] ([fdo#107724])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-icl-u3/igt@gem_exec_reloc@basic-write-gtt-noreloc.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-icl-u3/igt@gem_exec_reloc@basic-write-gtt-noreloc.html
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-kbl-r: [PASS][5] -> [DMESG-WARN][6] ([fdo#111012])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-kbl-r/igt@i915_pm_rpm@basic-pci-d3-state.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-kbl-r/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_selftest@live_reset:
- fi-icl-u3: [PASS][7] -> [INCOMPLETE][8] ([fdo#107713])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-icl-u3/igt@i915_selftest@live_reset.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-icl-u3/igt@i915_selftest@live_reset.html
#### Possible fixes ####
* igt@gem_basic@create-close:
- fi-icl-u3: [DMESG-WARN][9] ([fdo#107724]) -> [PASS][10] +1 similar issue
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-icl-u3/igt@gem_basic@create-close.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-icl-u3/igt@gem_basic@create-close.html
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: [INCOMPLETE][11] ([fdo#107718]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
* igt@i915_selftest@live_contexts:
- fi-bdw-gvtdvm: [INCOMPLETE][13] ([fdo#110976]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
- fi-skl-gvtdvm: [INCOMPLETE][15] ([fdo#110976]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
* igt@kms_busy@basic-flip-c:
- fi-skl-6770hq: [SKIP][17] ([fdo#109271] / [fdo#109278]) -> [PASS][18] +2 similar issues
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-skl-6770hq/igt@kms_busy@basic-flip-c.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-skl-6770hq/igt@kms_busy@basic-flip-c.html
* igt@kms_chamelium@dp-crc-fast:
- fi-icl-u2: [FAIL][19] ([fdo#109635 ] / [fdo#110387]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html
* igt@kms_flip@basic-flip-vs-dpms:
- fi-skl-6770hq: [SKIP][21] ([fdo#109271]) -> [PASS][22] +23 similar issues
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html
* igt@kms_frontbuffer_tracking@basic:
- fi-icl-u3: [FAIL][23] ([fdo#103167]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635
[fdo#110387]: https://bugs.freedesktop.org/show_bug.cgi?id=110387
[fdo#110976]: https://bugs.freedesktop.org/show_bug.cgi?id=110976
[fdo#111012]: https://bugs.freedesktop.org/show_bug.cgi?id=111012
Participating hosts (54 -> 47)
------------------------------
Additional (2): fi-hsw-peppy fi-pnv-d510
Missing (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6399 -> Patchwork_13503
CI_DRM_6399: c15e6447c0c506c058671d59082fcd710e42d6d4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5079: 873df2fa9e8f5fd02d4532b30ef2579f4fe4f27f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13503: c74efbfd4cb8759cb423245dc64a05039ab283f9 @ git://anongit.freedesktop.org/gfx-ci/linux
== Kernel 32bit build ==
Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/build_32bit.log
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
CHK include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready (#1)
Building modules, stage 2.
MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2
== Linux commits ==
c74efbfd4cb8 drm/i915: Replace struct_mutex for batch pool serialisation
d2991ee34d19 drm/i915: Protect request retirement with timeline->mutex
ccb245053e23 drm/i915/gt: Guard timeline pinning with its own mutex
fec91822fa4b drm/i915/gt: Convert timeline tracking to spinlock
cbf288ea92dd drm/i915/gt: Track timeline activeness in enter/exit
c70ab2f0eeaf drm/i915: Teach execbuffer to take the engine wakeref not GT
094947492d58 drm/i915/execlists: Hesitate before slicing
8094b6f9bbd7 drm/i915: Mark up vma->active as safe for use inside shrinkers
746cdb378dcd drm/i915: Markup potential lock for i915_active
d85fcd7ec654 drm/i915/gem: Free pages before rcu-freeing the object
54651a5c97e5 drm/i915/gt: Ignore forcewake acquisition for posting_reads
dd411766aecd drm/i915/gt: Assume we hold forcewake for execlists resume
98aef619b577 drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine
e3344fd5f501 drm/i915/selftests: Lock the drm_mm while modifying
f3a7eda391b2 drm/i915/selftests: Common live setup/teardown
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread* ✗ Fi.CI.IGT: failure for series starting with [01/15] drm/i915/selftests: Common live setup/teardown
2019-07-03 9:17 [PATCH 01/15] drm/i915/selftests: Common live setup/teardown Chris Wilson
` (17 preceding siblings ...)
2019-07-03 9:59 ` ✓ Fi.CI.BAT: success for series starting with [01/15] " Patchwork
@ 2019-07-04 3:22 ` Patchwork
18 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2019-07-04 3:22 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/15] drm/i915/selftests: Common live setup/teardown
URL : https://patchwork.freedesktop.org/series/63114/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6399_full -> Patchwork_13503_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_13503_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_13503_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_13503_full:
### IGT changes ###
#### Possible regressions ####
* igt@gem_busy@close-race:
- shard-snb: [PASS][1] -> [DMESG-FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-snb4/igt@gem_busy@close-race.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-snb5/igt@gem_busy@close-race.html
- shard-apl: [PASS][3] -> [DMESG-FAIL][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-apl5/igt@gem_busy@close-race.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-apl3/igt@gem_busy@close-race.html
- shard-iclb: [PASS][5] -> [DMESG-FAIL][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb5/igt@gem_busy@close-race.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb4/igt@gem_busy@close-race.html
* igt@runner@aborted:
- shard-iclb: NOTRUN -> [FAIL][7]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb4/igt@runner@aborted.html
- shard-apl: NOTRUN -> [FAIL][8]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-apl3/igt@runner@aborted.html
- shard-snb: NOTRUN -> [FAIL][9]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-snb5/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_13503_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_tiled_swapping@non-threaded:
- shard-kbl: [PASS][10] -> [DMESG-WARN][11] ([fdo#108686] / [fdo#110853])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-kbl6/igt@gem_tiled_swapping@non-threaded.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-kbl3/igt@gem_tiled_swapping@non-threaded.html
* igt@kms_flip@dpms-vs-vblank-race-interruptible:
- shard-glk: [PASS][12] -> [FAIL][13] ([fdo#103060])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-glk3/igt@kms_flip@dpms-vs-vblank-race-interruptible.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-glk3/igt@kms_flip@dpms-vs-vblank-race-interruptible.html
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-apl: [PASS][14] -> [DMESG-WARN][15] ([fdo#108566]) +1 similar issue
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-apl8/igt@kms_frontbuffer_tracking@fbc-suspend.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-apl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
- shard-skl: [PASS][16] -> [INCOMPLETE][17] ([fdo#104108]) +1 similar issue
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-skl10/igt@kms_frontbuffer_tracking@fbc-suspend.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-skl8/igt@kms_frontbuffer_tracking@fbc-suspend.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt:
- shard-iclb: [PASS][18] -> [FAIL][19] ([fdo#103167]) +1 similar issue
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-cpu:
- shard-skl: [PASS][20] -> [FAIL][21] ([fdo#103167])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-skl8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-cpu.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-skl8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-cpu.html
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: [PASS][22] -> [FAIL][23] ([fdo#108145])
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
* igt@kms_plane_lowres@pipe-a-tiling-x:
- shard-iclb: [PASS][24] -> [FAIL][25] ([fdo#103166])
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-x.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-x.html
* igt@kms_psr@psr2_suspend:
- shard-iclb: [PASS][26] -> [SKIP][27] ([fdo#109441])
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb2/igt@kms_psr@psr2_suspend.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb8/igt@kms_psr@psr2_suspend.html
* igt@kms_setmode@basic:
- shard-apl: [PASS][28] -> [FAIL][29] ([fdo#99912])
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-apl6/igt@kms_setmode@basic.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-apl2/igt@kms_setmode@basic.html
- shard-kbl: [PASS][30] -> [FAIL][31] ([fdo#99912])
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-kbl1/igt@kms_setmode@basic.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-kbl7/igt@kms_setmode@basic.html
* igt@perf@blocking:
- shard-skl: [PASS][32] -> [FAIL][33] ([fdo#110728])
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-skl5/igt@perf@blocking.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-skl7/igt@perf@blocking.html
#### Possible fixes ####
* igt@gem_eio@in-flight-suspend:
- shard-iclb: [INCOMPLETE][34] ([fdo#107713]) -> [PASS][35]
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb3/igt@gem_eio@in-flight-suspend.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb6/igt@gem_eio@in-flight-suspend.html
* igt@gem_exec_balancer@smoke:
- shard-iclb: [SKIP][36] ([fdo#110854]) -> [PASS][37]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb6/igt@gem_exec_balancer@smoke.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb1/igt@gem_exec_balancer@smoke.html
* igt@gem_softpin@noreloc-s3:
- shard-skl: [INCOMPLETE][38] ([fdo#104108]) -> [PASS][39]
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-skl10/igt@gem_softpin@noreloc-s3.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-skl7/igt@gem_softpin@noreloc-s3.html
* igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
- shard-glk: [FAIL][40] ([fdo#105363]) -> [PASS][41]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-glk8/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-glk5/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
* igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render:
- shard-snb: [SKIP][42] ([fdo#109271]) -> [PASS][43] +2 similar issues
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-snb4/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-snb5/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
- shard-iclb: [FAIL][44] ([fdo#103167]) -> [PASS][45] +1 similar issue
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
- shard-skl: [FAIL][46] ([fdo#108145]) -> [PASS][47]
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
* igt@kms_psr2_su@frontbuffer:
- shard-iclb: [SKIP][48] ([fdo#109642]) -> [PASS][49]
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb1/igt@kms_psr2_su@frontbuffer.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
* igt@kms_psr@no_drrs:
- shard-iclb: [FAIL][50] ([fdo#108341]) -> [PASS][51]
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb1/igt@kms_psr@no_drrs.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb7/igt@kms_psr@no_drrs.html
* igt@kms_psr@psr2_primary_mmap_cpu:
- shard-iclb: [SKIP][52] ([fdo#109441]) -> [PASS][53] +1 similar issue
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb3/igt@kms_psr@psr2_primary_mmap_cpu.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
* igt@kms_setmode@basic:
- shard-skl: [FAIL][54] ([fdo#99912]) -> [PASS][55]
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-skl9/igt@kms_setmode@basic.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-skl9/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-b-ts-continuation-suspend:
- shard-apl: [DMESG-WARN][56] ([fdo#108566]) -> [PASS][57] +4 similar issues
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-apl4/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-apl6/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
#### Warnings ####
* igt@kms_dp_dsc@basic-dsc-enable-edp:
- shard-iclb: [SKIP][58] ([fdo#109349]) -> [DMESG-WARN][59] ([fdo#107724])
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb3/igt@kms_dp_dsc@basic-dsc-enable-edp.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13503/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
[fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
[fdo#110853]: https://bugs.freedesktop.org/show_bug.cgi?id=110853
[fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 10)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_6399 -> Patchwork_13503
CI_DRM_6399: c15e6447c0c506c058671d59082fcd710e42d6d4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5079: 873df2fa9e8f5fd02d4532b30ef2579f4fe4f27f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13503: c74efbfd4cb8759cb423245dc64a05039ab283f9 @ 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_13503/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread