public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Don't complain when there is no workaround.
@ 2014-12-17 16:34 Rodrigo Vivi
  2014-12-18  7:19 ` shuang.he
  2014-12-18  8:16 ` Daniel Vetter
  0 siblings, 2 replies; 6+ messages in thread
From: Rodrigo Vivi @ 2014-12-17 16:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

When we don't have any workaround to emit we should celebrate, not bother.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        | 2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 7670a0f..0bb50f6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1096,7 +1096,7 @@ static int intel_logical_ring_workarounds_emit(struct intel_engine_cs *ring,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct i915_workarounds *w = &dev_priv->workarounds;
 
-	if (WARN_ON_ONCE(w->count == 0))
+	if (w->count == 0)
 		return 0;
 
 	ring->gpu_caches_dirty = true;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 12a36f0..3d99bb1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -682,7 +682,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct i915_workarounds *w = &dev_priv->workarounds;
 
-	if (WARN_ON_ONCE(w->count == 0))
+	if (w->count == 0)
 		return 0;
 
 	ring->gpu_caches_dirty = true;
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Don't complain when there is no workaround.
  2014-12-17 16:34 [PATCH] drm/i915: Don't complain when there is no workaround Rodrigo Vivi
@ 2014-12-18  7:19 ` shuang.he
  2014-12-18  8:16 ` Daniel Vetter
  1 sibling, 0 replies; 6+ messages in thread
From: shuang.he @ 2014-12-18  7:19 UTC (permalink / raw)
  To: shuang.he, intel-gfx, rodrigo.vivi

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK              +1-3              364/366              362/366
SNB                 -1              448/450              447/450
IVB                                  497/498              497/498
BYT                                  289/289              289/289
HSW                                  563/564              563/564
BDW                                  416/417              416/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 ILK  igt_kms_flip_bcs-flip-vs-modeset-interruptible      DMESG_WARN(1, M26)PASS(3, M37M26)      PASS(1, M26)
*ILK  igt_kms_flip_busy-flip-interruptible      PASS(2, M26)      DMESG_WARN(1, M26)
*ILK  igt_kms_flip_wf_vblank-ts-check      PASS(2, M26)      DMESG_WARN(1, M26)
*ILK  igt_kms_flip_wf_vblank-vs-modeset-interruptible      PASS(2, M26)      DMESG_WARN(1, M26)
*SNB  igt_gem_concurrent_blit_gtt-rcs-early-read-forked      PASS(2, M35M22)      FAIL(1, M22)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Don't complain when there is no workaround.
  2014-12-17 16:34 [PATCH] drm/i915: Don't complain when there is no workaround Rodrigo Vivi
  2014-12-18  7:19 ` shuang.he
@ 2014-12-18  8:16 ` Daniel Vetter
  2014-12-18 16:26   ` Dave Gordon
  1 sibling, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2014-12-18  8:16 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Wed, Dec 17, 2014 at 08:34:41AM -0800, Rodrigo Vivi wrote:
> When we don't have any workaround to emit we should celebrate, not bother.

Well except that on the platforms where this fires we really should have
some, most likely. This is in a way similar to all the other WARN_ON cases
we have to make sure nothing gets lost in platform enabling.
-Daniel

> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c        | 2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 7670a0f..0bb50f6 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1096,7 +1096,7 @@ static int intel_logical_ring_workarounds_emit(struct intel_engine_cs *ring,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct i915_workarounds *w = &dev_priv->workarounds;
>  
> -	if (WARN_ON_ONCE(w->count == 0))
> +	if (w->count == 0)
>  		return 0;
>  
>  	ring->gpu_caches_dirty = true;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 12a36f0..3d99bb1 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -682,7 +682,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct i915_workarounds *w = &dev_priv->workarounds;
>  
> -	if (WARN_ON_ONCE(w->count == 0))
> +	if (w->count == 0)
>  		return 0;
>  
>  	ring->gpu_caches_dirty = true;
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Don't complain when there is no workaround.
  2014-12-18  8:16 ` Daniel Vetter
@ 2014-12-18 16:26   ` Dave Gordon
  2014-12-18 18:47     ` Rodrigo Vivi
  0 siblings, 1 reply; 6+ messages in thread
From: Dave Gordon @ 2014-12-18 16:26 UTC (permalink / raw)
  To: intel-gfx

On 18/12/14 08:16, Daniel Vetter wrote:
> On Wed, Dec 17, 2014 at 08:34:41AM -0800, Rodrigo Vivi wrote:
>> When we don't have any workaround to emit we should celebrate, not bother.
> 
> Well except that on the platforms where this fires we really should have
> some, most likely. This is in a way similar to all the other WARN_ON cases
> we have to make sure nothing gets lost in platform enabling.
> -Daniel

You could have a convention that
	(count == 0)
means "I haven't filled this in yet" (and generates a warning), whereas
	(count == I915_NO_WORKAROUNDS_REQUIRED)
(which can be defined as (~0)) means "I haven't forgotten, there really
aren't any workarounds" and suppresses the warning.

Of course, any /other/ value > I915_MAX_WA_REGS should definitely give a
warning, preferably at compile time!

.Dave

>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_lrc.c        | 2 +-
>>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index 7670a0f..0bb50f6 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -1096,7 +1096,7 @@ static int intel_logical_ring_workarounds_emit(struct intel_engine_cs *ring,
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct i915_workarounds *w = &dev_priv->workarounds;
>>  
>> -	if (WARN_ON_ONCE(w->count == 0))
>> +	if (w->count == 0)
>>  		return 0;
>>  
>>  	ring->gpu_caches_dirty = true;
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> index 12a36f0..3d99bb1 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -682,7 +682,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring,
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct i915_workarounds *w = &dev_priv->workarounds;
>>  
>> -	if (WARN_ON_ONCE(w->count == 0))
>> +	if (w->count == 0)
>>  		return 0;
>>  
>>  	ring->gpu_caches_dirty = true;
>> -- 
>> 1.9.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Don't complain when there is no workaround.
  2014-12-18 16:26   ` Dave Gordon
@ 2014-12-18 18:47     ` Rodrigo Vivi
  2014-12-18 19:17       ` Ville Syrjälä
  0 siblings, 1 reply; 6+ messages in thread
From: Rodrigo Vivi @ 2014-12-18 18:47 UTC (permalink / raw)
  To: Dave Gordon; +Cc: intel-gfx

Yeah, but Skylake is running and we didn't need any W/A here yet. The
warning is just disturbing dmesg with 3 ops and not forcing uns to
hunt for missing workarouinds.

We add W/A based on specific needs, not based on counts.

On Thu, Dec 18, 2014 at 8:26 AM, Dave Gordon <david.s.gordon@intel.com> wrote:
> On 18/12/14 08:16, Daniel Vetter wrote:
>> On Wed, Dec 17, 2014 at 08:34:41AM -0800, Rodrigo Vivi wrote:
>>> When we don't have any workaround to emit we should celebrate, not bother.
>>
>> Well except that on the platforms where this fires we really should have
>> some, most likely. This is in a way similar to all the other WARN_ON cases
>> we have to make sure nothing gets lost in platform enabling.
>> -Daniel
>
> You could have a convention that
>         (count == 0)
> means "I haven't filled this in yet" (and generates a warning), whereas
>         (count == I915_NO_WORKAROUNDS_REQUIRED)
> (which can be defined as (~0)) means "I haven't forgotten, there really
> aren't any workarounds" and suppresses the warning.
>
> Of course, any /other/ value > I915_MAX_WA_REGS should definitely give a
> warning, preferably at compile time!
>
> .Dave
>
>>>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_lrc.c        | 2 +-
>>>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>>> index 7670a0f..0bb50f6 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -1096,7 +1096,7 @@ static int intel_logical_ring_workarounds_emit(struct intel_engine_cs *ring,
>>>      struct drm_i915_private *dev_priv = dev->dev_private;
>>>      struct i915_workarounds *w = &dev_priv->workarounds;
>>>
>>> -    if (WARN_ON_ONCE(w->count == 0))
>>> +    if (w->count == 0)
>>>              return 0;
>>>
>>>      ring->gpu_caches_dirty = true;
>>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> index 12a36f0..3d99bb1 100644
>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> @@ -682,7 +682,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring,
>>>      struct drm_i915_private *dev_priv = dev->dev_private;
>>>      struct i915_workarounds *w = &dev_priv->workarounds;
>>>
>>> -    if (WARN_ON_ONCE(w->count == 0))
>>> +    if (w->count == 0)
>>>              return 0;
>>>
>>>      ring->gpu_caches_dirty = true;
>>> --
>>> 1.9.3
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Don't complain when there is no workaround.
  2014-12-18 18:47     ` Rodrigo Vivi
@ 2014-12-18 19:17       ` Ville Syrjälä
  0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2014-12-18 19:17 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Thu, Dec 18, 2014 at 10:47:11AM -0800, Rodrigo Vivi wrote:
> Yeah, but Skylake is running and we didn't need any W/A here yet. The
> warning is just disturbing dmesg with 3 ops and not forcing uns to
> hunt for missing workarouinds.
> 
> We add W/A based on specific needs, not based on counts.

I can immediately see two w/as in gen9_init_clock_gating() that need to be
moved over to the new way of doing things.

> 
> On Thu, Dec 18, 2014 at 8:26 AM, Dave Gordon <david.s.gordon@intel.com> wrote:
> > On 18/12/14 08:16, Daniel Vetter wrote:
> >> On Wed, Dec 17, 2014 at 08:34:41AM -0800, Rodrigo Vivi wrote:
> >>> When we don't have any workaround to emit we should celebrate, not bother.
> >>
> >> Well except that on the platforms where this fires we really should have
> >> some, most likely. This is in a way similar to all the other WARN_ON cases
> >> we have to make sure nothing gets lost in platform enabling.
> >> -Daniel
> >
> > You could have a convention that
> >         (count == 0)
> > means "I haven't filled this in yet" (and generates a warning), whereas
> >         (count == I915_NO_WORKAROUNDS_REQUIRED)
> > (which can be defined as (~0)) means "I haven't forgotten, there really
> > aren't any workarounds" and suppresses the warning.
> >
> > Of course, any /other/ value > I915_MAX_WA_REGS should definitely give a
> > warning, preferably at compile time!
> >
> > .Dave
> >
> >>>
> >>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_lrc.c        | 2 +-
> >>>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
> >>>  2 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >>> index 7670a0f..0bb50f6 100644
> >>> --- a/drivers/gpu/drm/i915/intel_lrc.c
> >>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> >>> @@ -1096,7 +1096,7 @@ static int intel_logical_ring_workarounds_emit(struct intel_engine_cs *ring,
> >>>      struct drm_i915_private *dev_priv = dev->dev_private;
> >>>      struct i915_workarounds *w = &dev_priv->workarounds;
> >>>
> >>> -    if (WARN_ON_ONCE(w->count == 0))
> >>> +    if (w->count == 0)
> >>>              return 0;
> >>>
> >>>      ring->gpu_caches_dirty = true;
> >>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >>> index 12a36f0..3d99bb1 100644
> >>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >>> @@ -682,7 +682,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring,
> >>>      struct drm_i915_private *dev_priv = dev->dev_private;
> >>>      struct i915_workarounds *w = &dev_priv->workarounds;
> >>>
> >>> -    if (WARN_ON_ONCE(w->count == 0))
> >>> +    if (w->count == 0)
> >>>              return 0;
> >>>
> >>>      ring->gpu_caches_dirty = true;
> >>> --
> >>> 1.9.3
> >>>
> >>> _______________________________________________
> >>> Intel-gfx mailing list
> >>> Intel-gfx@lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-12-18 19:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-17 16:34 [PATCH] drm/i915: Don't complain when there is no workaround Rodrigo Vivi
2014-12-18  7:19 ` shuang.he
2014-12-18  8:16 ` Daniel Vetter
2014-12-18 16:26   ` Dave Gordon
2014-12-18 18:47     ` Rodrigo Vivi
2014-12-18 19:17       ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox