Re: [Intel-gfx] [PATCH] drm/i915/lrc: Remove superfluous WARN_ON

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-01-26 11:49:01)
> 
> On 26/01/2018 09:35, Chris Wilson wrote:
> > Remove the WARN_ON(ce->state) inside the static function only called
> > when ce->state == NULL and downgrade the w/a batch setup warning into a
> > developer only mode (GEM_WARN_ON).
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Tvrtko Ursulin 
> > ---
> >   drivers/gpu/drm/i915/intel_lrc.c | 5 ++---
> >   1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index 1a3174371c8e..50b37bf3682a 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -1419,7 +1419,8 @@ static int intel_init_workaround_bb(struct 
> > intel_engine_cs *engine)
> >*/
> >   for (i = 0; i < ARRAY_SIZE(wa_bb_fn); i++) {
> >   wa_bb[i]->offset = batch_ptr - batch;
> > - if (WARN_ON(!IS_ALIGNED(wa_bb[i]->offset, CACHELINE_BYTES))) {
> > + if (GEM_WARN_ON(!IS_ALIGNED(wa_bb[i]->offset,
> > + CACHELINE_BYTES))) {
> >   ret = -EINVAL;
> >   break;
> >   }
> > @@ -2271,8 +2272,6 @@ static int execlists_context_deferred_alloc(struct 
> > i915_gem_context *ctx,
> >   struct intel_ring *ring;
> >   int ret;
> >   
> > - WARN_ON(ce->state);
> > -
> 
> Call path is a bit messy, maybe this would be better left as 
> GEM_WARN_ON, return -Esomething?

Caller is a straight if (!ce->state) deferred_alloc().

GEM_BUG_ON() at most, but since this is all static, it just looks
superfluous. Maybe just move the guard here and let the compiler sort it
out.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/lrc: Remove superfluous WARN_ON

[Tvrtko Ursulin]

On 26/01/2018 09:35, Chris Wilson wrote:

Remove the WARN_ON(ce->state) inside the static function only called
when ce->state == NULL and downgrade the w/a batch setup warning into a
developer only mode (GEM_WARN_ON).

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
---
  drivers/gpu/drm/i915/intel_lrc.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 1a3174371c8e..50b37bf3682a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1419,7 +1419,8 @@ static int intel_init_workaround_bb(struct 
intel_engine_cs *engine)
 */
for (i = 0; i < ARRAY_SIZE(wa_bb_fn); i++) {
wa_bb[i]->offset = batch_ptr - batch;
-   if (WARN_ON(!IS_ALIGNED(wa_bb[i]->offset, CACHELINE_BYTES))) {
+   if (GEM_WARN_ON(!IS_ALIGNED(wa_bb[i]->offset,
+   CACHELINE_BYTES))) {
ret = -EINVAL;
break;
}
@@ -2271,8 +2272,6 @@ static int execlists_context_deferred_alloc(struct 
i915_gem_context *ctx,
struct intel_ring *ring;
int ret;
  
-	WARN_ON(ce->state);

-


Call path is a bit messy, maybe this would be better left as 
GEM_WARN_ON, return -Esomething?


Regards,

Tvrtko


context_size = round_up(engine->context_size, I915_GTT_PAGE_SIZE);
  
  	/*



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

[Intel-gfx] [PATCH] drm/i915/lrc: Remove superfluous WARN_ON

[Chris Wilson]

Remove the WARN_ON(ce->state) inside the static function only called
when ce->state == NULL and downgrade the w/a batch setup warning into a
developer only mode (GEM_WARN_ON).

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/intel_lrc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 1a3174371c8e..50b37bf3682a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1419,7 +1419,8 @@ static int intel_init_workaround_bb(struct 
intel_engine_cs *engine)
 */
for (i = 0; i < ARRAY_SIZE(wa_bb_fn); i++) {
wa_bb[i]->offset = batch_ptr - batch;
-   if (WARN_ON(!IS_ALIGNED(wa_bb[i]->offset, CACHELINE_BYTES))) {
+   if (GEM_WARN_ON(!IS_ALIGNED(wa_bb[i]->offset,
+   CACHELINE_BYTES))) {
ret = -EINVAL;
break;
}
@@ -2271,8 +2272,6 @@ static int execlists_context_deferred_alloc(struct 
i915_gem_context *ctx,
struct intel_ring *ring;
int ret;
 
-   WARN_ON(ce->state);
-
context_size = round_up(engine->context_size, I915_GTT_PAGE_SIZE);
 
/*
-- 
2.15.1

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