* [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
@ 2017-09-06 21:12 Oscar Mateo
2017-09-06 21:17 ` Oscar Mateo
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Oscar Mateo @ 2017-09-06 21:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi, Mika Kuoppala
Afaict, GEN9_GAMT_ECO_REG_RW_IA does not live in the context, so writing
it on every context creation is overkill (and wrong).
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
drivers/gpu/drm/i915/intel_engine_cs.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 23812ec..9f01a5c 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -985,8 +985,9 @@ static int skl_init_workarounds(struct intel_engine_cs *engine)
/* WaInPlaceDecompressionHang:skl */
if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
- WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
- GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+ I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
+ (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
+ GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
/* WaDisableLSQCROPERFforOCL:skl */
ret = wa_ring_whitelist_reg(engine, GEN8_L3SQCREG4);
@@ -1059,8 +1060,9 @@ static int bxt_init_workarounds(struct intel_engine_cs *engine)
/* WaInPlaceDecompressionHang:bxt */
if (IS_BXT_REVID(dev_priv, BXT_REVID_C0, REVID_FOREVER))
- WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
- GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+ I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
+ (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
+ GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
return 0;
}
@@ -1089,8 +1091,9 @@ static int cnl_init_workarounds(struct intel_engine_cs *engine)
GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE);
/* WaInPlaceDecompressionHang:cnl */
- WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
- GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+ I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
+ (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
+ GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
/* WaPushConstantDereferenceHoldDisable:cnl */
WA_SET_BIT(GEN7_ROW_CHICKEN2, PUSH_CONSTANT_DEREF_DISABLE);
@@ -1143,8 +1146,9 @@ static int kbl_init_workarounds(struct intel_engine_cs *engine)
GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
/* WaInPlaceDecompressionHang:kbl */
- WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
- GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+ I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
+ (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
+ GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
/* WaDisableLSQCROPERFforOCL:kbl */
ret = wa_ring_whitelist_reg(engine, GEN8_L3SQCREG4);
@@ -1196,8 +1200,9 @@ static int cfl_init_workarounds(struct intel_engine_cs *engine)
GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
/* WaInPlaceDecompressionHang:cfl */
- WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
- GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+ I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
+ (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
+ GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
return 0;
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
2017-09-06 21:12 [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write Oscar Mateo
@ 2017-09-06 21:17 ` Oscar Mateo
2017-09-07 9:30 ` Mika Kuoppala
2017-09-06 21:19 ` Chris Wilson
2017-09-06 21:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
2 siblings, 1 reply; 11+ messages in thread
From: Oscar Mateo @ 2017-09-06 21:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi, Mika Kuoppala
Hey Mika,
Regarding this patch: is there a consensus on where is the most
appropriate place to apply workarounds? My understanding is that
per-context workarounds (WAS_SET_BIT, etc...) go in
xxx_init_workarounds, while those that are needed only during
initialization (I915_WRITE) go in xxx_init_clock_gating. But it doesn't
look like this general rule is being followed (probably because
xxx_init_clock_gating is a very misleading name?).
This has probably been discussed before, so it would be good if we could
document the answer somewhere (maybe it already is?).
Thanks,
Oscar
On 09/06/2017 02:12 PM, Oscar Mateo wrote:
> Afaict, GEN9_GAMT_ECO_REG_RW_IA does not live in the context, so writing
> it on every context creation is overkill (and wrong).
>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
> drivers/gpu/drm/i915/intel_engine_cs.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 23812ec..9f01a5c 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -985,8 +985,9 @@ static int skl_init_workarounds(struct intel_engine_cs *engine)
>
> /* WaInPlaceDecompressionHang:skl */
> if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>
> /* WaDisableLSQCROPERFforOCL:skl */
> ret = wa_ring_whitelist_reg(engine, GEN8_L3SQCREG4);
> @@ -1059,8 +1060,9 @@ static int bxt_init_workarounds(struct intel_engine_cs *engine)
>
> /* WaInPlaceDecompressionHang:bxt */
> if (IS_BXT_REVID(dev_priv, BXT_REVID_C0, REVID_FOREVER))
> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>
> return 0;
> }
> @@ -1089,8 +1091,9 @@ static int cnl_init_workarounds(struct intel_engine_cs *engine)
> GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE);
>
> /* WaInPlaceDecompressionHang:cnl */
> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>
> /* WaPushConstantDereferenceHoldDisable:cnl */
> WA_SET_BIT(GEN7_ROW_CHICKEN2, PUSH_CONSTANT_DEREF_DISABLE);
> @@ -1143,8 +1146,9 @@ static int kbl_init_workarounds(struct intel_engine_cs *engine)
> GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
>
> /* WaInPlaceDecompressionHang:kbl */
> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>
> /* WaDisableLSQCROPERFforOCL:kbl */
> ret = wa_ring_whitelist_reg(engine, GEN8_L3SQCREG4);
> @@ -1196,8 +1200,9 @@ static int cfl_init_workarounds(struct intel_engine_cs *engine)
> GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
>
> /* WaInPlaceDecompressionHang:cfl */
> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>
> return 0;
> }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
2017-09-06 21:12 [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write Oscar Mateo
2017-09-06 21:17 ` Oscar Mateo
@ 2017-09-06 21:19 ` Chris Wilson
2017-09-06 21:27 ` Oscar Mateo
2017-09-06 21:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
2 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2017-09-06 21:19 UTC (permalink / raw)
To: Oscar Mateo, intel-gfx; +Cc: Mika Kuoppala, Rodrigo Vivi
Quoting Oscar Mateo (2017-09-06 22:12:11)
> Afaict, GEN9_GAMT_ECO_REG_RW_IA does not live in the context, so writing
> it on every context creation is overkill (and wrong).
>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
> drivers/gpu/drm/i915/intel_engine_cs.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 23812ec..9f01a5c 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -985,8 +985,9 @@ static int skl_init_workarounds(struct intel_engine_cs *engine)
>
> /* WaInPlaceDecompressionHang:skl */
> if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
Anything using a precalculated RMW value for a ctx register is indeed
fishy. Whilst you are checking this register, can you check whether the
other users of WA_SET_BIT/WA_CLR_BIT are indeed context bound?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
2017-09-06 21:19 ` Chris Wilson
@ 2017-09-06 21:27 ` Oscar Mateo
2017-09-06 21:43 ` Chris Wilson
0 siblings, 1 reply; 11+ messages in thread
From: Oscar Mateo @ 2017-09-06 21:27 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Mika Kuoppala, Rodrigo Vivi
On 09/06/2017 02:19 PM, Chris Wilson wrote:
> Quoting Oscar Mateo (2017-09-06 22:12:11)
>> Afaict, GEN9_GAMT_ECO_REG_RW_IA does not live in the context, so writing
>> it on every context creation is overkill (and wrong).
>>
>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_engine_cs.c | 25 +++++++++++++++----------
>> 1 file changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
>> index 23812ec..9f01a5c 100644
>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
>> @@ -985,8 +985,9 @@ static int skl_init_workarounds(struct intel_engine_cs *engine)
>>
>> /* WaInPlaceDecompressionHang:skl */
>> if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> Anything using a precalculated RMW value for a ctx register is indeed
> fishy. Whilst you are checking this register, can you check whether the
> other users of WA_SET_BIT/WA_CLR_BIT are indeed context bound?
> -Chris
Sure, I'll try to go through all of them (but I'd like to clarify first
if I should also be moving those I find to xxx_init_clock_gating).
-- Oscar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
2017-09-06 21:27 ` Oscar Mateo
@ 2017-09-06 21:43 ` Chris Wilson
2017-09-06 21:51 ` Oscar Mateo
0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2017-09-06 21:43 UTC (permalink / raw)
To: Oscar Mateo, intel-gfx; +Cc: Mika Kuoppala, Rodrigo Vivi
Quoting Oscar Mateo (2017-09-06 22:27:47)
>
>
> On 09/06/2017 02:19 PM, Chris Wilson wrote:
> > Quoting Oscar Mateo (2017-09-06 22:12:11)
> >> Afaict, GEN9_GAMT_ECO_REG_RW_IA does not live in the context, so writing
> >> it on every context creation is overkill (and wrong).
> >>
> >> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_engine_cs.c | 25 +++++++++++++++----------
> >> 1 file changed, 15 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> >> index 23812ec..9f01a5c 100644
> >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> >> @@ -985,8 +985,9 @@ static int skl_init_workarounds(struct intel_engine_cs *engine)
> >>
> >> /* WaInPlaceDecompressionHang:skl */
> >> if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
> >> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
> >> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> > Anything using a precalculated RMW value for a ctx register is indeed
> > fishy. Whilst you are checking this register, can you check whether the
> > other users of WA_SET_BIT/WA_CLR_BIT are indeed context bound?
> > -Chris
>
> Sure, I'll try to go through all of them (but I'd like to clarify first
> if I should also be moving those I find to xxx_init_clock_gating).
The short answer is probably not, init_clock_gating we expect to be
targetting display w/a. There's not always a clear divide between GT and
display, but we keep on muttering that we should keep them them as
cleanly separated as possible so that we know where to look when
different IP blocks are updated. (And yes the name is one of those
things that we keep on waiting for someone else to fix.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
2017-09-06 21:43 ` Chris Wilson
@ 2017-09-06 21:51 ` Oscar Mateo
2017-09-07 11:54 ` Ville Syrjälä
0 siblings, 1 reply; 11+ messages in thread
From: Oscar Mateo @ 2017-09-06 21:51 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Mika Kuoppala, Rodrigo Vivi
On 09/06/2017 02:43 PM, Chris Wilson wrote:
> Quoting Oscar Mateo (2017-09-06 22:27:47)
>>
>> On 09/06/2017 02:19 PM, Chris Wilson wrote:
>>> Quoting Oscar Mateo (2017-09-06 22:12:11)
>>>> Afaict, GEN9_GAMT_ECO_REG_RW_IA does not live in the context, so writing
>>>> it on every context creation is overkill (and wrong).
>>>>
>>>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/intel_engine_cs.c | 25 +++++++++++++++----------
>>>> 1 file changed, 15 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
>>>> index 23812ec..9f01a5c 100644
>>>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
>>>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
>>>> @@ -985,8 +985,9 @@ static int skl_init_workarounds(struct intel_engine_cs *engine)
>>>>
>>>> /* WaInPlaceDecompressionHang:skl */
>>>> if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
>>>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>>>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>> Anything using a precalculated RMW value for a ctx register is indeed
>>> fishy. Whilst you are checking this register, can you check whether the
>>> other users of WA_SET_BIT/WA_CLR_BIT are indeed context bound?
>>> -Chris
>> Sure, I'll try to go through all of them (but I'd like to clarify first
>> if I should also be moving those I find to xxx_init_clock_gating).
> The short answer is probably not, init_clock_gating we expect to be
> targetting display w/a. There's not always a clear divide between GT and
> display, but we keep on muttering that we should keep them them as
> cleanly separated as possible so that we know where to look when
> different IP blocks are updated. (And yes the name is one of those
> things that we keep on waiting for someone else to fix.)
> -Chris
It's not only the name, there is even a comment saying non-context,
non-WABB GT workarounds go here:
/**
* intel_init_clock_gating_hooks - setup the clock gating hooks
* @dev_priv: device private
*
* Setup the hooks that configure which clocks of a given platform can be
* gated and also apply various GT and display specific workarounds for
these
* platforms. Note that some GT specific workarounds are applied separately
* when GPU contexts or batchbuffers start their execution.
*/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
2017-09-06 21:12 [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write Oscar Mateo
2017-09-06 21:17 ` Oscar Mateo
2017-09-06 21:19 ` Chris Wilson
@ 2017-09-06 21:58 ` Patchwork
2 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-09-06 21:58 UTC (permalink / raw)
To: Oscar Mateo; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
URL : https://patchwork.freedesktop.org/series/29906/
State : failure
== Summary ==
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
CHK include/generated/bounds.h
CHK include/generated/timeconst.h
CHK include/generated/asm-offsets.h
CALL scripts/checksyscalls.sh
CHK scripts/mod/devicetable-offsets.h
CHK include/generated/compile.h
CHK kernel/config_data.h
CC [M] drivers/gpu/drm/i915/intel_engine_cs.o
In file included from drivers/gpu/drm/i915/selftests/mock_engine.h:32:0,
from drivers/gpu/drm/i915/selftests/mock_engine.c:25,
from drivers/gpu/drm/i915/intel_engine_cs.c:1402:
drivers/gpu/drm/i915/selftests/../intel_ringbuffer.h: In function ‘skl_init_workarounds’:
drivers/gpu/drm/i915/selftests/../intel_ringbuffer.h:740:0: error: unterminated argument list invoking macro "I915_WRITE"
#endif /* _INTEL_RINGBUFFER_H_ */
drivers/gpu/drm/i915/intel_engine_cs.c:988:3: error: ‘I915_WRITE’ undeclared (first use in this function)
I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
^~~~~~~~~~
drivers/gpu/drm/i915/intel_engine_cs.c:988:3: note: each undeclared identifier is reported only once for each function it appears in
In file included from drivers/gpu/drm/i915/selftests/mock_engine.c:25:0,
from drivers/gpu/drm/i915/intel_engine_cs.c:1402:
drivers/gpu/drm/i915/selftests/mock_engine.h:34:1: error: expected ‘;’ before ‘struct’
struct mock_engine {
^~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.h:42:1: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
^~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.h:49:20: error: invalid storage class for function ‘mock_seqno_advance’
static inline void mock_seqno_advance(struct intel_engine_cs *engine, u32 seqno)
^~~~~~~~~~~~~~~~~~
In file included from drivers/gpu/drm/i915/intel_engine_cs.c:1402:0:
drivers/gpu/drm/i915/selftests/mock_engine.c:28:50: error: ‘struct mock_engine’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
static struct mock_request *first_request(struct mock_engine *engine)
^~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:28:29: error: invalid storage class for function ‘first_request’
static struct mock_request *first_request(struct mock_engine *engine)
^~~~~~~~~~~~~
In file included from ./include/linux/preempt.h:10:0,
from ./include/linux/spinlock.h:50,
from ./include/linux/mmzone.h:7,
from ./include/linux/gfp.h:5,
from ./include/linux/slab.h:14,
from ./include/linux/io-mapping.h:22,
from drivers/gpu/drm/i915/i915_drv.h:36,
from drivers/gpu/drm/i915/intel_engine_cs.c:25:
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘first_request’:
drivers/gpu/drm/i915/selftests/mock_engine.c:30:41: error: dereferencing pointer to incomplete type ‘struct mock_engine’
return list_first_entry_or_null(&engine->hw_queue,
^
./include/linux/list.h:398:30: note: in definition of macro ‘list_first_entry_or_null’
struct list_head *head__ = (ptr); \
^~~
In file included from drivers/gpu/drm/i915/intel_engine_cs.c:1402:0:
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘skl_init_workarounds’:
drivers/gpu/drm/i915/selftests/mock_engine.c:35:13: error: invalid storage class for function ‘hw_delay_complete’
static void hw_delay_complete(unsigned long data)
^~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘hw_delay_complete’:
drivers/gpu/drm/i915/selftests/mock_engine.c:40:19: error: dereferencing pointer to incomplete type ‘struct mock_engine’
spin_lock(&engine->hw_lock);
^~
drivers/gpu/drm/i915/selftests/mock_engine.c:42:26: error: passing argument 1 of ‘first_request’ from incompatible pointer type [-Werror=incompatible-pointer-types]
request = first_request(engine);
^~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:28:29: note: expected ‘struct mock_engine *’ but argument is of type ‘struct mock_engine *’
static struct mock_request *first_request(struct mock_engine *engine)
^~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:48:26: error: passing argument 1 of ‘first_request’ from incompatible pointer type [-Werror=incompatible-pointer-types]
request = first_request(engine);
^~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:28:29: note: expected ‘struct mock_engine *’ but argument is of type ‘struct mock_engine *’
static struct mock_request *first_request(struct mock_engine *engine)
^~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘skl_init_workarounds’:
drivers/gpu/drm/i915/selftests/mock_engine.c:56:1: error: invalid storage class for function ‘mock_context_pin’
mock_context_pin(struct intel_engine_cs *engine,
^~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:63:13: error: invalid storage class for function ‘mock_context_unpin’
static void mock_context_unpin(struct intel_engine_cs *engine,
^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:69:12: error: invalid storage class for function ‘mock_request_alloc’
static int mock_request_alloc(struct drm_i915_gem_request *request)
^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:79:12: error: invalid storage class for function ‘mock_emit_flush’
static int mock_emit_flush(struct drm_i915_gem_request *request,
^~~~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:85:13: error: invalid storage class for function ‘mock_emit_breadcrumb’
static void mock_emit_breadcrumb(struct drm_i915_gem_request *request,
^~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:90:13: error: invalid storage class for function ‘mock_submit_request’
static void mock_submit_request(struct drm_i915_gem_request *request)
^~~~~~~~~~~~~~~~~~~
In file included from ./include/uapi/linux/stddef.h:1:0,
from ./include/linux/stddef.h:4,
from ./include/uapi/linux/posix_types.h:4,
from ./include/uapi/linux/types.h:13,
from ./include/linux/types.h:5,
from ./include/uapi/drm/drm.h:41,
from ./include/uapi/drm/i915_drm.h:30,
from drivers/gpu/drm/i915/i915_drv.h:33,
from drivers/gpu/drm/i915/intel_engine_cs.c:25:
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘mock_submit_request’:
drivers/gpu/drm/i915/selftests/mock_engine.c:94:40: error: dereferencing pointer to incomplete type ‘struct mock_engine’
container_of(request->engine, typeof(*engine), base);
^~
./include/linux/compiler.h:523:19: note: in definition of macro ‘__compiletime_assert’
bool __cond = !(condition); \
^~~~~~~~~
./include/linux/compiler.h:546:2: note: in expansion of macro ‘_compiletime_assert’
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:46:37: note: in expansion of macro ‘compiletime_assert’
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
./include/linux/kernel.h:860:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
./include/linux/kernel.h:860:20: note: in expansion of macro ‘__same_type’
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:94:3: note: in expansion of macro ‘container_of’
container_of(request->engine, typeof(*engine), base);
^~~~~~~~~~~~
In file included from drivers/gpu/drm/i915/intel_engine_cs.c:1402:0:
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘skl_init_workarounds’:
drivers/gpu/drm/i915/selftests/mock_engine.c:106:27: error: invalid storage class for function ‘mock_ring’
static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
^~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:125:25: error: static declaration of ‘mock_engine’ follows non-static declaration
struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
^~~~~~~~~~~
In file included from drivers/gpu/drm/i915/selftests/mock_engine.c:25:0,
from drivers/gpu/drm/i915/intel_engine_cs.c:1402:
drivers/gpu/drm/i915/selftests/mock_engine.h:42:25: note: previous declaration of ‘mock_engine’ was here
struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
^~~~~~~~~~~
In file included from drivers/gpu/drm/i915/intel_engine_cs.c:1402:0:
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘mock_engine’:
drivers/gpu/drm/i915/selftests/mock_engine.c:133:26: error: dereferencing pointer to incomplete type ‘struct mock_engine’
engine = kzalloc(sizeof(*engine) + PAGE_SIZE, GFP_KERNEL);
^~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:147:2: error: invalid use of undefined type ‘struct mock_engine’
engine->base.status_page.page_addr = (void *)(engine + 1);
^~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘skl_init_workarounds’:
drivers/gpu/drm/i915/selftests/mock_engine.c:172:6: error: static declaration of ‘mock_engine_flush’ follows non-static declaration
void mock_engine_flush(struct intel_engine_cs *engine)
^~~~~~~~~~~~~~~~~
In file included from drivers/gpu/drm/i915/selftests/mock_engine.c:25:0,
from drivers/gpu/drm/i915/intel_engine_cs.c:1402:
drivers/gpu/drm/i915/selftests/mock_engine.h:45:6: note: previous declaration of ‘mock_engine_flush’ was here
void mock_engine_flush(struct intel_engine_cs *engine);
^~~~~~~~~~~~~~~~~
In file included from ./include/uapi/linux/stddef.h:1:0,
from ./include/linux/stddef.h:4,
from ./include/uapi/linux/posix_types.h:4,
from ./include/uapi/linux/types.h:13,
from ./include/linux/types.h:5,
from ./include/uapi/drm/drm.h:41,
from ./include/uapi/drm/i915_drm.h:30,
from drivers/gpu/drm/i915/i915_drv.h:33,
from drivers/gpu/drm/i915/intel_engine_cs.c:25:
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘mock_engine_flush’:
drivers/gpu/drm/i915/selftests/mock_engine.c:175:31: error: dereferencing pointer to incomplete type ‘struct mock_engine’
container_of(engine, typeof(*mock), base);
^~
./include/linux/compiler.h:523:19: note: in definition of macro ‘__compiletime_assert’
bool __cond = !(condition); \
^~~~~~~~~
./include/linux/compiler.h:546:2: note: in expansion of macro ‘_compiletime_assert’
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:46:37: note: in expansion of macro ‘compiletime_assert’
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
./include/linux/kernel.h:860:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
./include/linux/kernel.h:860:20: note: in expansion of macro ‘__same_type’
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:175:3: note: in expansion of macro ‘container_of’
container_of(engine, typeof(*mock), base);
^~~~~~~~~~~~
In file included from drivers/gpu/drm/i915/intel_engine_cs.c:1402:0:
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘skl_init_workarounds’:
drivers/gpu/drm/i915/selftests/mock_engine.c:188:6: error: static declaration of ‘mock_engine_reset’ follows non-static declaration
void mock_engine_reset(struct intel_engine_cs *engine)
^~~~~~~~~~~~~~~~~
In file included from drivers/gpu/drm/i915/selftests/mock_engine.c:25:0,
from drivers/gpu/drm/i915/intel_engine_cs.c:1402:
drivers/gpu/drm/i915/selftests/mock_engine.h:46:6: note: previous declaration of ‘mock_engine_reset’ was here
void mock_engine_reset(struct intel_engine_cs *engine);
^~~~~~~~~~~~~~~~~
In file included from drivers/gpu/drm/i915/intel_engine_cs.c:1402:0:
drivers/gpu/drm/i915/selftests/mock_engine.c:193:6: error: static declaration of ‘mock_engine_free’ follows non-static declaration
void mock_engine_free(struct intel_engine_cs *engine)
^~~~~~~~~~~~~~~~
In file included from drivers/gpu/drm/i915/selftests/mock_engine.c:25:0,
from drivers/gpu/drm/i915/intel_engine_cs.c:1402:
drivers/gpu/drm/i915/selftests/mock_engine.h:47:6: note: previous declaration of ‘mock_engine_free’ was here
void mock_engine_free(struct intel_engine_cs *engine);
^~~~~~~~~~~~~~~~
In file included from ./include/uapi/linux/stddef.h:1:0,
from ./include/linux/stddef.h:4,
from ./include/uapi/linux/posix_types.h:4,
from ./include/uapi/linux/types.h:13,
from ./include/linux/types.h:5,
from ./include/uapi/drm/drm.h:41,
from ./include/uapi/drm/i915_drm.h:30,
from drivers/gpu/drm/i915/i915_drv.h:33,
from drivers/gpu/drm/i915/intel_engine_cs.c:25:
drivers/gpu/drm/i915/selftests/mock_engine.c: In function ‘mock_engine_free’:
drivers/gpu/drm/i915/selftests/mock_engine.c:196:31: error: dereferencing pointer to incomplete type ‘struct mock_engine’
container_of(engine, typeof(*mock), base);
^~
./include/linux/compiler.h:523:19: note: in definition of macro ‘__compiletime_assert’
bool __cond = !(condition); \
^~~~~~~~~
./include/linux/compiler.h:546:2: note: in expansion of macro ‘_compiletime_assert’
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:46:37: note: in expansion of macro ‘compiletime_assert’
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
./include/linux/kernel.h:860:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
./include/linux/kernel.h:860:20: note: in expansion of macro ‘__same_type’
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:196:3: note: in expansion of macro ‘container_of’
container_of(engine, typeof(*mock), base);
^~~~~~~~~~~~
drivers/gpu/drm/i915/intel_engine_cs.c: In function ‘skl_init_workarounds’:
drivers/gpu/drm/i915/intel_engine_cs.c:1403:0: error: expected declaration or statement at end of input
#endif
In file included from drivers/gpu/drm/i915/intel_engine_cs.c:1402:0:
At top level:
drivers/gpu/drm/i915/selftests/mock_engine.c:193:6: error: ‘mock_engine_free’ defined but not used [-Werror=unused-function]
void mock_engine_free(struct intel_engine_cs *engine)
^~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:188:6: error: ‘mock_engine_reset’ defined but not used [-Werror=unused-function]
void mock_engine_reset(struct intel_engine_cs *engine)
^~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:172:6: error: ‘mock_engine_flush’ defined but not used [-Werror=unused-function]
void mock_engine_flush(struct intel_engine_cs *engine)
^~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_engine.c:125:25: error: ‘mock_engine’ defined but not used [-Werror=unused-function]
struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
^~~~~~~~~~~
drivers/gpu/drm/i915/intel_engine_cs.c:962:12: error: ‘skl_init_workarounds’ defined but not used [-Werror=unused-function]
static int skl_init_workarounds(struct intel_engine_cs *engine)
^~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/intel_engine_cs.c:921:12: error: ‘skl_tune_iz_hashing’ defined but not used [-Werror=unused-function]
static int skl_tune_iz_hashing(struct intel_engine_cs *engine)
^~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/intel_engine_cs.c:780:12: error: ‘chv_init_workarounds’ defined but not used [-Werror=unused-function]
static int chv_init_workarounds(struct intel_engine_cs *engine)
^~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/intel_engine_cs.c:748:12: error: ‘bdw_init_workarounds’ defined but not used [-Werror=unused-function]
static int bdw_init_workarounds(struct intel_engine_cs *engine)
^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
scripts/Makefile.build:302: recipe for target 'drivers/gpu/drm/i915/intel_engine_cs.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_engine_cs.o] Error 1
scripts/Makefile.build:561: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:561: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:561: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1019: recipe for target 'drivers' failed
make: *** [drivers] Error 2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
2017-09-06 21:17 ` Oscar Mateo
@ 2017-09-07 9:30 ` Mika Kuoppala
2017-09-07 15:50 ` Oscar Mateo
0 siblings, 1 reply; 11+ messages in thread
From: Mika Kuoppala @ 2017-09-07 9:30 UTC (permalink / raw)
To: Oscar Mateo, intel-gfx; +Cc: Rodrigo Vivi
Oscar Mateo <oscar.mateo@intel.com> writes:
> Hey Mika,
>
> Regarding this patch: is there a consensus on where is the most
> appropriate place to apply workarounds? My understanding is that
> per-context workarounds (WAS_SET_BIT, etc...) go in
> xxx_init_workarounds, while those that are needed only during
> initialization (I915_WRITE) go in xxx_init_clock_gating. But it doesn't
> look like this general rule is being followed (probably because
> xxx_init_clock_gating is a very misleading name?).
>
> This has probably been discussed before, so it would be good if we could
> document the answer somewhere (maybe it already is?).
>
Yep, xxx_init_workarounds for per context. init_clock_gating for
globals.
If I recall init_clock_gating was a just suitable spot wrt
to init/reset to inject globals.
Have we reached the pain treshold and move to genx_workarounds.c
where there are per context and global entries?
-Mika
> Thanks,
>
> Oscar
>
>
>
> On 09/06/2017 02:12 PM, Oscar Mateo wrote:
>> Afaict, GEN9_GAMT_ECO_REG_RW_IA does not live in the context, so writing
>> it on every context creation is overkill (and wrong).
>>
>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_engine_cs.c | 25 +++++++++++++++----------
>> 1 file changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
>> index 23812ec..9f01a5c 100644
>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
>> @@ -985,8 +985,9 @@ static int skl_init_workarounds(struct intel_engine_cs *engine)
>>
>> /* WaInPlaceDecompressionHang:skl */
>> if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>
>> /* WaDisableLSQCROPERFforOCL:skl */
>> ret = wa_ring_whitelist_reg(engine, GEN8_L3SQCREG4);
>> @@ -1059,8 +1060,9 @@ static int bxt_init_workarounds(struct intel_engine_cs *engine)
>>
>> /* WaInPlaceDecompressionHang:bxt */
>> if (IS_BXT_REVID(dev_priv, BXT_REVID_C0, REVID_FOREVER))
>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>
>> return 0;
>> }
>> @@ -1089,8 +1091,9 @@ static int cnl_init_workarounds(struct intel_engine_cs *engine)
>> GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE);
>>
>> /* WaInPlaceDecompressionHang:cnl */
>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>
>> /* WaPushConstantDereferenceHoldDisable:cnl */
>> WA_SET_BIT(GEN7_ROW_CHICKEN2, PUSH_CONSTANT_DEREF_DISABLE);
>> @@ -1143,8 +1146,9 @@ static int kbl_init_workarounds(struct intel_engine_cs *engine)
>> GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
>>
>> /* WaInPlaceDecompressionHang:kbl */
>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>
>> /* WaDisableLSQCROPERFforOCL:kbl */
>> ret = wa_ring_whitelist_reg(engine, GEN8_L3SQCREG4);
>> @@ -1196,8 +1200,9 @@ static int cfl_init_workarounds(struct intel_engine_cs *engine)
>> GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
>>
>> /* WaInPlaceDecompressionHang:cfl */
>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>
>> return 0;
>> }
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
2017-09-06 21:51 ` Oscar Mateo
@ 2017-09-07 11:54 ` Ville Syrjälä
2017-09-08 7:15 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2017-09-07 11:54 UTC (permalink / raw)
To: Oscar Mateo; +Cc: Rodrigo Vivi, intel-gfx, Mika Kuoppala
On Wed, Sep 06, 2017 at 02:51:10PM -0700, Oscar Mateo wrote:
>
>
> On 09/06/2017 02:43 PM, Chris Wilson wrote:
> > Quoting Oscar Mateo (2017-09-06 22:27:47)
> >>
> >> On 09/06/2017 02:19 PM, Chris Wilson wrote:
> >>> Quoting Oscar Mateo (2017-09-06 22:12:11)
> >>>> Afaict, GEN9_GAMT_ECO_REG_RW_IA does not live in the context, so writing
> >>>> it on every context creation is overkill (and wrong).
> >>>>
> >>>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> >>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >>>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> >>>> ---
> >>>> drivers/gpu/drm/i915/intel_engine_cs.c | 25 +++++++++++++++----------
> >>>> 1 file changed, 15 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> >>>> index 23812ec..9f01a5c 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> >>>> @@ -985,8 +985,9 @@ static int skl_init_workarounds(struct intel_engine_cs *engine)
> >>>>
> >>>> /* WaInPlaceDecompressionHang:skl */
> >>>> if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
> >>>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
> >>>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> >>> Anything using a precalculated RMW value for a ctx register is indeed
> >>> fishy. Whilst you are checking this register, can you check whether the
> >>> other users of WA_SET_BIT/WA_CLR_BIT are indeed context bound?
> >>> -Chris
> >> Sure, I'll try to go through all of them (but I'd like to clarify first
> >> if I should also be moving those I find to xxx_init_clock_gating).
> > The short answer is probably not, init_clock_gating we expect to be
> > targetting display w/a. There's not always a clear divide between GT and
> > display, but we keep on muttering that we should keep them them as
> > cleanly separated as possible so that we know where to look when
> > different IP blocks are updated. (And yes the name is one of those
> > things that we keep on waiting for someone else to fix.)
> > -Chris
>
> It's not only the name, there is even a comment saying non-context,
> non-WABB GT workarounds go here:
>
> /**
> * intel_init_clock_gating_hooks - setup the clock gating hooks
> * @dev_priv: device private
> *
> * Setup the hooks that configure which clocks of a given platform can be
> * gated and also apply various GT and display specific workarounds for
> these
> * platforms. Note that some GT specific workarounds are applied separately
> * when GPU contexts or batchbuffers start their execution.
> */
IIRC one problem at least used to be that we called .init_clock_gating()
after we'd already touched the GT. So it was probabably called too late.
Not sure what the order is these days.
And I have a feeling that some platforms had a few registers that are
clobbered by a GPU reset but aren't saved in the context. So those we'd
need to reconfigure somewhere else becase .init_clock_gating() isn't
called on GPU reset (except on pre-g4x where also the display gets
clobbered). I have a feeling these were some L3 related things on IVB
perhaps?
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
2017-09-07 9:30 ` Mika Kuoppala
@ 2017-09-07 15:50 ` Oscar Mateo
0 siblings, 0 replies; 11+ messages in thread
From: Oscar Mateo @ 2017-09-07 15:50 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: Rodrigo Vivi
On 09/07/2017 02:30 AM, Mika Kuoppala wrote:
> Oscar Mateo <oscar.mateo@intel.com> writes:
>
>> Hey Mika,
>>
>> Regarding this patch: is there a consensus on where is the most
>> appropriate place to apply workarounds? My understanding is that
>> per-context workarounds (WAS_SET_BIT, etc...) go in
>> xxx_init_workarounds, while those that are needed only during
>> initialization (I915_WRITE) go in xxx_init_clock_gating. But it doesn't
>> look like this general rule is being followed (probably because
>> xxx_init_clock_gating is a very misleading name?).
>>
>> This has probably been discussed before, so it would be good if we could
>> document the answer somewhere (maybe it already is?).
>>
> Yep, xxx_init_workarounds for per context. init_clock_gating for
> globals.
>
> If I recall init_clock_gating was a just suitable spot wrt
> to init/reset to inject globals.
>
> Have we reached the pain treshold and move to genx_workarounds.c
> where there are per context and global entries?
>
> -Mika
>
I don't know about several genX_workarounds.c, but at least one
intel_workarounds.c where per-context and global entries live in harmony
(maybe WABB stuff as well?).
-- Oscar
>> Thanks,
>>
>> Oscar
>>
>>
>>
>> On 09/06/2017 02:12 PM, Oscar Mateo wrote:
>>> Afaict, GEN9_GAMT_ECO_REG_RW_IA does not live in the context, so writing
>>> it on every context creation is overkill (and wrong).
>>>
>>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_engine_cs.c | 25 +++++++++++++++----------
>>> 1 file changed, 15 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
>>> index 23812ec..9f01a5c 100644
>>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
>>> @@ -985,8 +985,9 @@ static int skl_init_workarounds(struct intel_engine_cs *engine)
>>>
>>> /* WaInPlaceDecompressionHang:skl */
>>> if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
>>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>>> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>>> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>>
>>> /* WaDisableLSQCROPERFforOCL:skl */
>>> ret = wa_ring_whitelist_reg(engine, GEN8_L3SQCREG4);
>>> @@ -1059,8 +1060,9 @@ static int bxt_init_workarounds(struct intel_engine_cs *engine)
>>>
>>> /* WaInPlaceDecompressionHang:bxt */
>>> if (IS_BXT_REVID(dev_priv, BXT_REVID_C0, REVID_FOREVER))
>>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>>> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>>> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>>
>>> return 0;
>>> }
>>> @@ -1089,8 +1091,9 @@ static int cnl_init_workarounds(struct intel_engine_cs *engine)
>>> GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE);
>>>
>>> /* WaInPlaceDecompressionHang:cnl */
>>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>>> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>>> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>>
>>> /* WaPushConstantDereferenceHoldDisable:cnl */
>>> WA_SET_BIT(GEN7_ROW_CHICKEN2, PUSH_CONSTANT_DEREF_DISABLE);
>>> @@ -1143,8 +1146,9 @@ static int kbl_init_workarounds(struct intel_engine_cs *engine)
>>> GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
>>>
>>> /* WaInPlaceDecompressionHang:kbl */
>>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>>> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>>> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>>
>>> /* WaDisableLSQCROPERFforOCL:kbl */
>>> ret = wa_ring_whitelist_reg(engine, GEN8_L3SQCREG4);
>>> @@ -1196,8 +1200,9 @@ static int cfl_init_workarounds(struct intel_engine_cs *engine)
>>> GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
>>>
>>> /* WaInPlaceDecompressionHang:cfl */
>>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
>>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>> + I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
>>> + (I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
>>> + GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>>>
>>> return 0;
>>> }
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write
2017-09-07 11:54 ` Ville Syrjälä
@ 2017-09-08 7:15 ` Daniel Vetter
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2017-09-08 7:15 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Mika Kuoppala, Rodrigo Vivi
On Thu, Sep 07, 2017 at 02:54:53PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 06, 2017 at 02:51:10PM -0700, Oscar Mateo wrote:
> >
> >
> > On 09/06/2017 02:43 PM, Chris Wilson wrote:
> > > Quoting Oscar Mateo (2017-09-06 22:27:47)
> > >>
> > >> On 09/06/2017 02:19 PM, Chris Wilson wrote:
> > >>> Quoting Oscar Mateo (2017-09-06 22:12:11)
> > >>>> Afaict, GEN9_GAMT_ECO_REG_RW_IA does not live in the context, so writing
> > >>>> it on every context creation is overkill (and wrong).
> > >>>>
> > >>>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > >>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > >>>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> > >>>> ---
> > >>>> drivers/gpu/drm/i915/intel_engine_cs.c | 25 +++++++++++++++----------
> > >>>> 1 file changed, 15 insertions(+), 10 deletions(-)
> > >>>>
> > >>>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > >>>> index 23812ec..9f01a5c 100644
> > >>>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > >>>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > >>>> @@ -985,8 +985,9 @@ static int skl_init_workarounds(struct intel_engine_cs *engine)
> > >>>>
> > >>>> /* WaInPlaceDecompressionHang:skl */
> > >>>> if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
> > >>>> - WA_SET_BIT(GEN9_GAMT_ECO_REG_RW_IA,
> > >>>> - GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> > >>> Anything using a precalculated RMW value for a ctx register is indeed
> > >>> fishy. Whilst you are checking this register, can you check whether the
> > >>> other users of WA_SET_BIT/WA_CLR_BIT are indeed context bound?
> > >>> -Chris
> > >> Sure, I'll try to go through all of them (but I'd like to clarify first
> > >> if I should also be moving those I find to xxx_init_clock_gating).
> > > The short answer is probably not, init_clock_gating we expect to be
> > > targetting display w/a. There's not always a clear divide between GT and
> > > display, but we keep on muttering that we should keep them them as
> > > cleanly separated as possible so that we know where to look when
> > > different IP blocks are updated. (And yes the name is one of those
> > > things that we keep on waiting for someone else to fix.)
> > > -Chris
> >
> > It's not only the name, there is even a comment saying non-context,
> > non-WABB GT workarounds go here:
> >
> > /**
> > * intel_init_clock_gating_hooks - setup the clock gating hooks
> > * @dev_priv: device private
> > *
> > * Setup the hooks that configure which clocks of a given platform can be
> > * gated and also apply various GT and display specific workarounds for
> > these
> > * platforms. Note that some GT specific workarounds are applied separately
> > * when GPU contexts or batchbuffers start their execution.
> > */
>
> IIRC one problem at least used to be that we called .init_clock_gating()
> after we'd already touched the GT. So it was probabably called too late.
> Not sure what the order is these days.
>
> And I have a feeling that some platforms had a few registers that are
> clobbered by a GPU reset but aren't saved in the context. So those we'd
> need to reconfigure somewhere else becase .init_clock_gating() isn't
> called on GPU reset (except on pre-g4x where also the display gets
> clobbered). I have a feeling these were some L3 related things on IVB
> perhaps?
Those should all be put in the engine setup. And yes we got this wrong a
few times, where a gpu reset broke of fixed mesa (depending upon which way
round the w/a vs reset defaults was).
Maybe we need a "how to apply w/a writes and where" doc page somewhere
that we use to whack every wa patch with in review? This comes up for
every platform a few times ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-09-08 7:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-06 21:12 [PATCH] drm/i915: Transform WaInPlaceDecompressionHang to a simple reg write Oscar Mateo
2017-09-06 21:17 ` Oscar Mateo
2017-09-07 9:30 ` Mika Kuoppala
2017-09-07 15:50 ` Oscar Mateo
2017-09-06 21:19 ` Chris Wilson
2017-09-06 21:27 ` Oscar Mateo
2017-09-06 21:43 ` Chris Wilson
2017-09-06 21:51 ` Oscar Mateo
2017-09-07 11:54 ` Ville Syrjälä
2017-09-08 7:15 ` Daniel Vetter
2017-09-06 21:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox