Re: [Intel-gfx] [PATCH] drm/i915: Disable MI_STORE_DATA_IMM for i915g/i915gm

[Chris Wilson]

Quoting Ville Syrjälä (2017-09-06 16:00:49)
> On Wed, Sep 06, 2017 at 03:40:08PM +0100, Chris Wilson wrote:
> > The early gen3 machines (i915g/Grantsdale and i915gm/Alviso) share a lot
> > of characteristics in their MI/GTT blocks with gen2, and in particular
> > can only use physical addresses in MI_STORE_DATA_IMM. This makes it
> > incompatible with our usage, so include those two machines in the
> > blacklist to prevent usage.
> > 
> > Signed-off-by: Chris Wilson 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h|  7 ---
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  7 ---
> >  drivers/gpu/drm/i915/intel_engine_cs.c | 16 
> >  drivers/gpu/drm/i915/intel_ringbuffer.h| 12 +---
> >  4 files changed, 21 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 789f7502cd1f..6020a94daf81 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -4360,11 +4360,4 @@ 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 53718a245ded..7687483ff218 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -1168,6 +1168,9 @@ static u32 *reloc_gpu(struct i915_execbuffer *eb,
> >   if (eb_use_cmdparser(eb))
> >   return ERR_PTR(-EWOULDBLOCK);
> >  
> > + if (!intel_engine_can_store_dword(eb->engine))
> > + return ERR_PTR(-ENODEV);
> > +
> >   err = __reloc_gpu_alloc(eb, vma, len);
> >   if (unlikely(err))
> >   return ERR_PTR(err);
> > @@ -1192,9 +1195,7 @@ 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)) &&
> > - __intel_engine_can_store_dword(eb->reloc_cache.gen,
> > -eb->engine->class)) {
> > +  !reservation_object_test_signaled_rcu(vma->resv, true))) {
> >   const unsigned int gen = eb->reloc_cache.gen;
> >   unsigned int len;
> >   u32 *batch;
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
> > b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index ae668340620e..ff56de4816cb 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -1378,6 +1378,22 @@ void intel_engines_mark_idle(struct drm_i915_private 
> > *i915)
> >   }
> >  }
> >  
> > +bool intel_engine_can_store_dword(struct intel_engine_cs *engine)
> > +{
> > + int gen = INTEL_GEN(engine->i915);
> > +
> > + if (gen <= 2)
> > + return false; /* uses physical not virtual addresses */
> > +
> > + if (gen == 3 && (IS_I915G(engine->i915) || IS_I915GM(engine->i915)))
> > + return false; /* uses physical not virtual addresses */
> 
> nit: 'gen' check seems redundant.

I was optimistic that it would make gcc smarter. :)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Disable MI_STORE_DATA_IMM for i915g/i915gm

[Ville Syrjälä]

On Wed, Sep 06, 2017 at 03:40:08PM +0100, Chris Wilson wrote:
> The early gen3 machines (i915g/Grantsdale and i915gm/Alviso) share a lot
> of characteristics in their MI/GTT blocks with gen2, and in particular
> can only use physical addresses in MI_STORE_DATA_IMM. This makes it
> incompatible with our usage, so include those two machines in the
> blacklist to prevent usage.
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_drv.h|  7 ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  7 ---
>  drivers/gpu/drm/i915/intel_engine_cs.c | 16 
>  drivers/gpu/drm/i915/intel_ringbuffer.h| 12 +---
>  4 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 789f7502cd1f..6020a94daf81 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -4360,11 +4360,4 @@ 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 53718a245ded..7687483ff218 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1168,6 +1168,9 @@ static u32 *reloc_gpu(struct i915_execbuffer *eb,
>   if (eb_use_cmdparser(eb))
>   return ERR_PTR(-EWOULDBLOCK);
>  
> + if (!intel_engine_can_store_dword(eb->engine))
> + return ERR_PTR(-ENODEV);
> +
>   err = __reloc_gpu_alloc(eb, vma, len);
>   if (unlikely(err))
>   return ERR_PTR(err);
> @@ -1192,9 +1195,7 @@ 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)) &&
> - __intel_engine_can_store_dword(eb->reloc_cache.gen,
> -eb->engine->class)) {
> +  !reservation_object_test_signaled_rcu(vma->resv, true))) {
>   const unsigned int gen = eb->reloc_cache.gen;
>   unsigned int len;
>   u32 *batch;
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
> b/drivers/gpu/drm/i915/intel_engine_cs.c
> index ae668340620e..ff56de4816cb 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1378,6 +1378,22 @@ void intel_engines_mark_idle(struct drm_i915_private 
> *i915)
>   }
>  }
>  
> +bool intel_engine_can_store_dword(struct intel_engine_cs *engine)
> +{
> + int gen = INTEL_GEN(engine->i915);
> +
> + if (gen <= 2)
> + return false; /* uses physical not virtual addresses */
> +
> + if (gen == 3 && (IS_I915G(engine->i915) || IS_I915GM(engine->i915)))
> + return false; /* uses physical not virtual addresses */

nit: 'gen' check seems redundant.

I suppose trying to do the relocs using physical addresses would be
quite a bit of work for little gain.

Patch lgtm
Reviewed-by: Ville Syrjälä 

> +
> + if (gen == 6 && engine->class == VIDEO_DECODE_CLASS)
> + return false; /* b0rked */
> +
> + return true;
> +}
> +
>  #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>  #include "selftests/mock_engine.c"
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
> b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 02d8974bf9ab..79c0021f3700 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -735,16 +735,6 @@ 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;
> -}
> +bool intel_engine_can_store_dword(struct intel_engine_cs *engine);
>  
>  #endif /* _INTEL_RINGBUFFER_H_ */
> -- 
> 2.14.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list

Re: [Intel-gfx] [PATCH] drm/i915: Disable MI_STORE_DATA_IMM for i915g/i915gm

[Chris Wilson]

Quoting Chris Wilson (2017-09-06 15:40:08)
> The early gen3 machines (i915g/Grantsdale and i915gm/Alviso) share a lot
> of characteristics in their MI/GTT blocks with gen2, and in particular
> can only use physical addresses in MI_STORE_DATA_IMM. This makes it
> incompatible with our usage, so include those two machines in the
> blacklist to prevent usage.
 
Fixes: 7dd4f6729f92 ("drm/i915: Async GPU relocation processing")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102562
> Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Disable MI_STORE_DATA_IMM for i915g/i915gm

[Chris Wilson]

The early gen3 machines (i915g/Grantsdale and i915gm/Alviso) share a lot
of characteristics in their MI/GTT blocks with gen2, and in particular
can only use physical addresses in MI_STORE_DATA_IMM. This makes it
incompatible with our usage, so include those two machines in the
blacklist to prevent usage.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_drv.h|  7 ---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  7 ---
 drivers/gpu/drm/i915/intel_engine_cs.c | 16 
 drivers/gpu/drm/i915/intel_ringbuffer.h| 12 +---
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 789f7502cd1f..6020a94daf81 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -4360,11 +4360,4 @@ 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 53718a245ded..7687483ff218 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1168,6 +1168,9 @@ static u32 *reloc_gpu(struct i915_execbuffer *eb,
if (eb_use_cmdparser(eb))
return ERR_PTR(-EWOULDBLOCK);
 
+   if (!intel_engine_can_store_dword(eb->engine))
+   return ERR_PTR(-ENODEV);
+
err = __reloc_gpu_alloc(eb, vma, len);
if (unlikely(err))
return ERR_PTR(err);
@@ -1192,9 +1195,7 @@ 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)) &&
-   __intel_engine_can_store_dword(eb->reloc_cache.gen,
-  eb->engine->class)) {
+!reservation_object_test_signaled_rcu(vma->resv, true))) {
const unsigned int gen = eb->reloc_cache.gen;
unsigned int len;
u32 *batch;
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index ae668340620e..ff56de4816cb 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1378,6 +1378,22 @@ void intel_engines_mark_idle(struct drm_i915_private 
*i915)
}
 }
 
+bool intel_engine_can_store_dword(struct intel_engine_cs *engine)
+{
+   int gen = INTEL_GEN(engine->i915);
+
+   if (gen <= 2)
+   return false; /* uses physical not virtual addresses */
+
+   if (gen == 3 && (IS_I915G(engine->i915) || IS_I915GM(engine->i915)))
+   return false; /* uses physical not virtual addresses */
+
+   if (gen == 6 && engine->class == VIDEO_DECODE_CLASS)
+   return false; /* b0rked */
+
+   return true;
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/mock_engine.c"
 #endif
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 02d8974bf9ab..79c0021f3700 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -735,16 +735,6 @@ 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;
-}
+bool intel_engine_can_store_dword(struct intel_engine_cs *engine);
 
 #endif /* _INTEL_RINGBUFFER_H_ */
-- 
2.14.1

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