Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> MI_STORE_DWORD_IMM just doesn't work on the video decode engine under
> Sandybridge, so refrain from using it. Then switch the selftests over to
> using the now common test prior to using MI_STORE_DWORD_IMM.
>
> Fixes: 7dd4f6729f92 ("drm/i915: Async GPU relocation processing")
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: <drm-intel-fixes@xxxxxxxxxxxxxxxxxxxxx> # v4.13-rc1+
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 ++++----
> drivers/gpu/drm/i915/i915_selftest.h | 2 --
> drivers/gpu/drm/i915/intel_ringbuffer.h | 12 ++++++++++++
> drivers/gpu/drm/i915/selftests/i915_gem_coherency.c | 2 +-
> drivers/gpu/drm/i915/selftests/i915_gem_context.c | 6 ++++--
> drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 18 ++++++++++++------
> 7 files changed, 40 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6c25c8520c87..620e53bd705a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -4327,4 +4327,11 @@ int remap_io_mapping(struct vm_area_struct *vma,
> unsigned long addr, unsigned long pfn, unsigned long size,
> struct io_mapping *iomap);
>
> +static inline bool
> +intel_engine_can_store_dword(struct intel_engine_cs *engine)
> +{
> + return __intel_engine_can_store_dword(INTEL_GEN(engine->i915),
> + engine->class);
> +}
> +
> #endif
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 8e8bc7aefd9c..359d5dc6d8df 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1268,7 +1268,9 @@ relocate_entry(struct i915_vma *vma,
>
> if (!eb->reloc_cache.vaddr &&
> (DBG_FORCE_RELOC == FORCE_GPU_RELOC ||
> - !reservation_object_test_signaled_rcu(vma->resv, true))) {
> + !reservation_object_test_signaled_rcu(vma->resv, true)) &&
> + __intel_engine_can_store_dword(eb->reloc_cache.gen,
> + eb->engine->class)) {
> const unsigned int gen = eb->reloc_cache.gen;
If you lift this to upper scope, you can make the check little
shorter. But incase you are avoiding the assignment to the latest,
i am not insisting.
There is engine in the eb so could you elaborate that
do we get by not doig intel_engine_can_store_dword(eb->engine)?
-Mika
> unsigned int len;
> u32 *batch;
> @@ -1278,10 +1280,8 @@ relocate_entry(struct i915_vma *vma,
> len = offset & 7 ? 8 : 5;
> else if (gen >= 4)
> len = 4;
> - else if (gen >= 3)
> + else
> len = 3;
> - else /* On gen2 MI_STORE_DWORD_IMM uses a physical address */
> - goto repeat;
>
> batch = reloc_gpu(eb, vma, len);
> if (IS_ERR(batch))
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> index 9d7d86f1733d..78e1a1b168ff 100644
> --- a/drivers/gpu/drm/i915/i915_selftest.h
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -101,6 +101,4 @@ bool __igt_timeout(unsigned long timeout, const char *fmt, ...);
> #define igt_timeout(t, fmt, ...) \
> __igt_timeout((t), KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
>
> -#define igt_can_mi_store_dword_imm(D) (INTEL_GEN(D) > 2)
> -
> #endif /* !__I915_SELFTEST_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index d33c93444c0d..02d8974bf9ab 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -735,4 +735,16 @@ bool intel_engines_are_idle(struct drm_i915_private *dev_priv);
> void intel_engines_mark_idle(struct drm_i915_private *i915);
> void intel_engines_reset_default_submission(struct drm_i915_private *i915);
>
> +static inline bool
> +__intel_engine_can_store_dword(unsigned int gen, unsigned int class)
> +{
> + if (gen <= 2)
> + return false; /* uses physical not virtual addresses */
> +
> + if (gen == 6 && class == VIDEO_DECODE_CLASS)
> + return false; /* b0rked */
> +
> + return true;
> +}
> +
> #endif /* _INTEL_RINGBUFFER_H_ */
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c b/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c
> index 95d4aebc0181..35d778d70626 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c
> @@ -241,7 +241,7 @@ static bool always_valid(struct drm_i915_private *i915)
>
> static bool needs_mi_store_dword(struct drm_i915_private *i915)
> {
> - return igt_can_mi_store_dword_imm(i915);
> + return intel_engine_can_store_dword(i915->engine[RCS]);
> }
>
> static const struct igt_coherency_mode {
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index 12b85b3278cd..fb0a58fc8348 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -38,8 +38,6 @@ gpu_fill_dw(struct i915_vma *vma, u64 offset, unsigned long count, u32 value)
> u32 *cmd;
> int err;
>
> - GEM_BUG_ON(!igt_can_mi_store_dword_imm(vma->vm->i915));
> -
> size = (4 * count + 1) * sizeof(u32);
> size = round_up(size, PAGE_SIZE);
> obj = i915_gem_object_create_internal(vma->vm->i915, size);
> @@ -123,6 +121,7 @@ static int gpu_fill(struct drm_i915_gem_object *obj,
> int err;
>
> GEM_BUG_ON(obj->base.size > vm->total);
> + GEM_BUG_ON(!intel_engine_can_store_dword(engine));
>
> vma = i915_vma_instance(obj, vm, NULL);
> if (IS_ERR(vma))
> @@ -359,6 +358,9 @@ static int igt_ctx_exec(void *arg)
> }
>
> for_each_engine(engine, i915, id) {
> + if (!intel_engine_can_store_dword(engine))
> + continue;
> +
> if (!obj) {
> obj = create_test_object(ctx, file, &objects);
> if (IS_ERR(obj)) {
> diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> index 208b34e864fb..02e52a146ed8 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> @@ -253,9 +253,6 @@ static int igt_hang_sanitycheck(void *arg)
>
> /* Basic check that we can execute our hanging batch */
>
> - if (!igt_can_mi_store_dword_imm(i915))
> - return 0;
> -
> mutex_lock(&i915->drm.struct_mutex);
> err = hang_init(&h, i915);
> if (err)
> @@ -264,6 +261,9 @@ static int igt_hang_sanitycheck(void *arg)
> for_each_engine(engine, i915, id) {
> long timeout;
>
> + if (!intel_engine_can_store_dword(engine))
> + continue;
> +
> rq = hang_create_request(&h, engine, i915->kernel_context);
> if (IS_ERR(rq)) {
> err = PTR_ERR(rq);
> @@ -599,6 +599,9 @@ static int igt_wait_reset(void *arg)
> long timeout;
> int err;
>
> + if (!intel_engine_can_store_dword(i915->engine[RCS]))
> + return 0;
> +
> /* Check that we detect a stuck waiter and issue a reset */
>
> global_reset_lock(i915);
> @@ -664,9 +667,6 @@ static int igt_reset_queue(void *arg)
>
> /* Check that we replay pending requests following a hang */
>
> - if (!igt_can_mi_store_dword_imm(i915))
> - return 0;
> -
> global_reset_lock(i915);
>
> mutex_lock(&i915->drm.struct_mutex);
> @@ -679,6 +679,9 @@ static int igt_reset_queue(void *arg)
> IGT_TIMEOUT(end_time);
> unsigned int count;
>
> + if (!intel_engine_can_store_dword(engine))
> + continue;
> +
> prev = hang_create_request(&h, engine, i915->kernel_context);
> if (IS_ERR(prev)) {
> err = PTR_ERR(prev);
> @@ -784,6 +787,9 @@ static int igt_handle_error(void *arg)
> if (!intel_has_reset_engine(i915))
> return 0;
>
> + if (!intel_engine_can_store_dword(i915->engine[RCS]))
> + return 0;
> +
> mutex_lock(&i915->drm.struct_mutex);
>
> err = hang_init(&h, i915);
> --
> 2.13.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
