* [PATCH 1/2] drm/i915: Make use of 'engine->uncore'
@ 2019-04-05 18:57 Chris Wilson
2019-04-05 18:57 ` [PATCH 2/2] drm/i915: Convert i915_reset.c over to using uncore mmio Chris Wilson
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Chris Wilson @ 2019-04-05 18:57 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
The engine has a direct link to the intel_uncore mmio handler, so make
use of it rather than going indirectly via &engine->i915->uncore.
v2: Update gen11_lock_sfc() to use engine->uncore as well
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reset.c | 32 ++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
index ddc403ee8855..d44dc8422e8c 100644
--- a/drivers/gpu/drm/i915/i915_reset.c
+++ b/drivers/gpu/drm/i915/i915_reset.c
@@ -331,11 +331,10 @@ static int gen6_reset_engines(struct drm_i915_private *i915,
return gen6_hw_domain_reset(i915, hw_mask);
}
-static u32 gen11_lock_sfc(struct drm_i915_private *dev_priv,
- struct intel_engine_cs *engine)
+static u32 gen11_lock_sfc(struct intel_engine_cs *engine)
{
- struct intel_uncore *uncore = &dev_priv->uncore;
- u8 vdbox_sfc_access = RUNTIME_INFO(dev_priv)->vdbox_sfc_access;
+ struct intel_uncore *uncore = engine->uncore;
+ u8 vdbox_sfc_access = RUNTIME_INFO(engine->i915)->vdbox_sfc_access;
i915_reg_t sfc_forced_lock, sfc_forced_lock_ack;
u32 sfc_forced_lock_bit, sfc_forced_lock_ack_bit;
i915_reg_t sfc_usage;
@@ -399,12 +398,13 @@ static u32 gen11_lock_sfc(struct drm_i915_private *dev_priv,
return 0;
}
-static void gen11_unlock_sfc(struct drm_i915_private *dev_priv,
- struct intel_engine_cs *engine)
+static void gen11_unlock_sfc(struct intel_engine_cs *engine)
{
- u8 vdbox_sfc_access = RUNTIME_INFO(dev_priv)->vdbox_sfc_access;
+ struct intel_uncore *uncore = engine->uncore;
+ u8 vdbox_sfc_access = RUNTIME_INFO(engine->i915)->vdbox_sfc_access;
i915_reg_t sfc_forced_lock;
u32 sfc_forced_lock_bit;
+ u32 val;
switch (engine->class) {
case VIDEO_DECODE_CLASS:
@@ -424,8 +424,9 @@ static void gen11_unlock_sfc(struct drm_i915_private *dev_priv,
return;
}
- I915_WRITE_FW(sfc_forced_lock,
- I915_READ_FW(sfc_forced_lock) & ~sfc_forced_lock_bit);
+ val = intel_uncore_read_fw(uncore, sfc_forced_lock);
+ val &= ~sfc_forced_lock_bit;
+ intel_uncore_write_fw(uncore, sfc_forced_lock, val);
}
static int gen11_reset_engines(struct drm_i915_private *i915,
@@ -454,7 +455,7 @@ static int gen11_reset_engines(struct drm_i915_private *i915,
for_each_engine_masked(engine, i915, engine_mask, tmp) {
GEM_BUG_ON(engine->id >= ARRAY_SIZE(hw_engine_mask));
hw_mask |= hw_engine_mask[engine->id];
- hw_mask |= gen11_lock_sfc(i915, engine);
+ hw_mask |= gen11_lock_sfc(engine);
}
}
@@ -462,17 +463,18 @@ static int gen11_reset_engines(struct drm_i915_private *i915,
if (engine_mask != ALL_ENGINES)
for_each_engine_masked(engine, i915, engine_mask, tmp)
- gen11_unlock_sfc(i915, engine);
+ gen11_unlock_sfc(engine);
return ret;
}
static int gen8_engine_reset_prepare(struct intel_engine_cs *engine)
{
- struct intel_uncore *uncore = &engine->i915->uncore;
+ struct intel_uncore *uncore = engine->uncore;
int ret;
- intel_uncore_write_fw(uncore, RING_RESET_CTL(engine->mmio_base),
+ intel_uncore_write_fw(uncore,
+ RING_RESET_CTL(engine->mmio_base),
_MASKED_BIT_ENABLE(RESET_CTL_REQUEST_RESET));
ret = __intel_wait_for_register_fw(uncore,
@@ -647,7 +649,7 @@ static void reset_prepare_engine(struct intel_engine_cs *engine)
* written to the powercontext is undefined and so we may lose
* GPU state upon resume, i.e. fail to restart after a reset.
*/
- intel_uncore_forcewake_get(&engine->i915->uncore, FORCEWAKE_ALL);
+ intel_uncore_forcewake_get(engine->uncore, FORCEWAKE_ALL);
engine->reset.prepare(engine);
}
@@ -719,7 +721,7 @@ static int gt_reset(struct drm_i915_private *i915,
static void reset_finish_engine(struct intel_engine_cs *engine)
{
engine->reset.finish(engine);
- intel_uncore_forcewake_put(&engine->i915->uncore, FORCEWAKE_ALL);
+ intel_uncore_forcewake_put(engine->uncore, FORCEWAKE_ALL);
}
struct i915_gpu_restart {
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] drm/i915: Convert i915_reset.c over to using uncore mmio
2019-04-05 18:57 [PATCH 1/2] drm/i915: Make use of 'engine->uncore' Chris Wilson
@ 2019-04-05 18:57 ` Chris Wilson
2019-04-05 20:06 ` Paulo Zanoni
2019-04-05 20:24 ` [PATCH v2] " Chris Wilson
2019-04-05 18:59 ` [PATCH 1/2] drm/i915: Make use of 'engine->uncore' Chris Wilson
` (3 subsequent siblings)
4 siblings, 2 replies; 12+ messages in thread
From: Chris Wilson @ 2019-04-05 18:57 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Currently i915_reset.c mixes calls to intel_uncore, pci and our old
style I915_READ mmio interfaces. Cast aside the old implicit macros,
and harmonise on using uncore throughout.
add/remove: 1/1 grow/shrink: 0/4 up/down: 65/-207 (-142)
Function old new delta
rmw_register - 65 +65
gen8_reset_engines 945 942 -3
g4x_do_reset 407 376 -31
intel_gpu_reset 545 509 -36
clear_register 63 - -63
i915_clear_error_registers 461 387 -74
A little bit of pointer dancing elimination works wonders.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reset.c | 125 +++++++++++++++++-------------
1 file changed, 73 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
index d44dc8422e8c..f50534a833ca 100644
--- a/drivers/gpu/drm/i915/i915_reset.c
+++ b/drivers/gpu/drm/i915/i915_reset.c
@@ -18,6 +18,28 @@
/* XXX How to handle concurrent GGTT updates using tiling registers? */
#define RESET_UNDER_STOP_MACHINE 0
+static void rmw_register(struct intel_uncore *uncore,
+ i915_reg_t reg, u32 clr, u32 set)
+{
+ u32 val;
+
+ val = intel_uncore_read(uncore, reg);
+ val &= ~clr;
+ val |= set;
+ intel_uncore_write(uncore, reg, val);
+}
+
+static void rmw_register_fw(struct intel_uncore *uncore,
+ i915_reg_t reg, u32 clr, u32 set)
+{
+ u32 val;
+
+ val = intel_uncore_read_fw(uncore, reg);
+ val &= ~clr;
+ val |= set;
+ intel_uncore_write_fw(uncore, reg, val);
+}
+
static void engine_skip_context(struct i915_request *rq)
{
struct intel_engine_cs *engine = rq->engine;
@@ -119,7 +141,7 @@ void i915_reset_request(struct i915_request *rq, bool guilty)
static void gen3_stop_engine(struct intel_engine_cs *engine)
{
- struct drm_i915_private *dev_priv = engine->i915;
+ struct intel_uncore *uncore = engine->uncore;
const u32 base = engine->mmio_base;
GEM_TRACE("%s\n", engine->name);
@@ -127,20 +149,23 @@ static void gen3_stop_engine(struct intel_engine_cs *engine)
if (intel_engine_stop_cs(engine))
GEM_TRACE("%s: timed out on STOP_RING\n", engine->name);
- I915_WRITE_FW(RING_HEAD(base), I915_READ_FW(RING_TAIL(base)));
- POSTING_READ_FW(RING_HEAD(base)); /* paranoia */
+ intel_uncore_write_fw(uncore,
+ RING_HEAD(base),
+ intel_uncore_read_fw(uncore, RING_TAIL(base)));
+ intel_uncore_posting_read_fw(uncore, RING_HEAD(base)); /* paranoia */
- I915_WRITE_FW(RING_HEAD(base), 0);
- I915_WRITE_FW(RING_TAIL(base), 0);
- POSTING_READ_FW(RING_TAIL(base));
+ intel_uncore_write_fw(uncore, RING_HEAD(base), 0);
+ intel_uncore_write_fw(uncore, RING_TAIL(base), 0);
+ intel_uncore_posting_read_fw(uncore, RING_TAIL(base));
/* The ring must be empty before it is disabled */
- I915_WRITE_FW(RING_CTL(base), 0);
+ intel_uncore_write_fw(uncore, RING_CTL(base), 0);
/* Check acts as a post */
- if (I915_READ_FW(RING_HEAD(base)))
+ if (intel_uncore_read_fw(uncore, RING_HEAD(base)))
GEM_TRACE("%s: ring head [%x] not parked\n",
- engine->name, I915_READ_FW(RING_HEAD(base)));
+ engine->name,
+ intel_uncore_read_fw(uncore, RING_HEAD(base)));
}
static void i915_stop_engines(struct drm_i915_private *i915,
@@ -203,17 +228,17 @@ static int g33_do_reset(struct drm_i915_private *i915,
return wait_for_atomic(g4x_reset_complete(pdev), 50);
}
-static int g4x_do_reset(struct drm_i915_private *dev_priv,
+static int g4x_do_reset(struct drm_i915_private *i915,
intel_engine_mask_t engine_mask,
unsigned int retry)
{
- struct pci_dev *pdev = dev_priv->drm.pdev;
+ struct pci_dev *pdev = i915->drm.pdev;
+ struct intel_uncore *uncore = &i915->uncore;
int ret;
/* WaVcpClkGateDisableForMediaReset:ctg,elk */
- I915_WRITE_FW(VDECCLK_GATE_D,
- I915_READ(VDECCLK_GATE_D) | VCP_UNIT_CLOCK_GATE_DISABLE);
- POSTING_READ_FW(VDECCLK_GATE_D);
+ rmw_register_fw(uncore, VDECCLK_GATE_D, 0, VCP_UNIT_CLOCK_GATE_DISABLE);
+ intel_uncore_posting_read_fw(uncore, VDECCLK_GATE_D);
pci_write_config_byte(pdev, I915_GDRST,
GRDOM_MEDIA | GRDOM_RESET_ENABLE);
@@ -234,18 +259,17 @@ static int g4x_do_reset(struct drm_i915_private *dev_priv,
out:
pci_write_config_byte(pdev, I915_GDRST, 0);
- I915_WRITE_FW(VDECCLK_GATE_D,
- I915_READ(VDECCLK_GATE_D) & ~VCP_UNIT_CLOCK_GATE_DISABLE);
- POSTING_READ_FW(VDECCLK_GATE_D);
+ rmw_register_fw(uncore, VDECCLK_GATE_D, VCP_UNIT_CLOCK_GATE_DISABLE, 0);
+ intel_uncore_posting_read_fw(uncore, VDECCLK_GATE_D);
return ret;
}
-static int ironlake_do_reset(struct drm_i915_private *dev_priv,
+static int ironlake_do_reset(struct drm_i915_private *i915,
intel_engine_mask_t engine_mask,
unsigned int retry)
{
- struct intel_uncore *uncore = &dev_priv->uncore;
+ struct intel_uncore *uncore = &i915->uncore;
int ret;
intel_uncore_write_fw(uncore, ILK_GDSR,
@@ -277,10 +301,10 @@ static int ironlake_do_reset(struct drm_i915_private *dev_priv,
}
/* Reset the hardware domains (GENX_GRDOM_*) specified by mask */
-static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
+static int gen6_hw_domain_reset(struct drm_i915_private *i915,
u32 hw_domain_mask)
{
- struct intel_uncore *uncore = &dev_priv->uncore;
+ struct intel_uncore *uncore = &i915->uncore;
int err;
/*
@@ -381,7 +405,7 @@ static u32 gen11_lock_sfc(struct intel_engine_cs *engine)
* ends up being locked to the engine we want to reset, we have to reset
* it as well (we will unlock it once the reset sequence is completed).
*/
- intel_uncore_rmw_or_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
+ rmw_register_fw(uncore, sfc_forced_lock, 0, sfc_forced_lock_bit);
if (__intel_wait_for_register_fw(uncore,
sfc_forced_lock_ack,
@@ -404,7 +428,6 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine)
u8 vdbox_sfc_access = RUNTIME_INFO(engine->i915)->vdbox_sfc_access;
i915_reg_t sfc_forced_lock;
u32 sfc_forced_lock_bit;
- u32 val;
switch (engine->class) {
case VIDEO_DECODE_CLASS:
@@ -424,9 +447,7 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine)
return;
}
- val = intel_uncore_read_fw(uncore, sfc_forced_lock);
- val &= ~sfc_forced_lock_bit;
- intel_uncore_write_fw(uncore, sfc_forced_lock, val);
+ rmw_register_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit, 0);
}
static int gen11_reset_engines(struct drm_i915_private *i915,
@@ -491,10 +512,9 @@ static int gen8_engine_reset_prepare(struct intel_engine_cs *engine)
static void gen8_engine_reset_cancel(struct intel_engine_cs *engine)
{
- struct drm_i915_private *dev_priv = engine->i915;
-
- I915_WRITE_FW(RING_RESET_CTL(engine->mmio_base),
- _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
+ intel_uncore_write_fw(engine->uncore,
+ RING_RESET_CTL(engine->mmio_base),
+ _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
}
static int gen8_reset_engines(struct drm_i915_private *i915,
@@ -1178,49 +1198,50 @@ static void i915_reset_device(struct drm_i915_private *i915,
kobject_uevent_env(kobj, KOBJ_CHANGE, reset_done_event);
}
-static void clear_register(struct drm_i915_private *dev_priv, i915_reg_t reg)
+static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
{
- I915_WRITE(reg, I915_READ(reg));
+ rmw_register(uncore, reg, 0, 0);
}
-void i915_clear_error_registers(struct drm_i915_private *dev_priv)
+void i915_clear_error_registers(struct drm_i915_private *i915)
{
+ struct intel_uncore *uncore = &i915->uncore;
u32 eir;
- if (!IS_GEN(dev_priv, 2))
- clear_register(dev_priv, PGTBL_ER);
+ if (!IS_GEN(i915, 2))
+ clear_register(uncore, PGTBL_ER);
- if (INTEL_GEN(dev_priv) < 4)
- clear_register(dev_priv, IPEIR(RENDER_RING_BASE));
+ if (INTEL_GEN(i915) < 4)
+ clear_register(uncore, IPEIR(RENDER_RING_BASE));
else
- clear_register(dev_priv, IPEIR_I965);
+ clear_register(uncore, IPEIR_I965);
- clear_register(dev_priv, EIR);
- eir = I915_READ(EIR);
+ clear_register(uncore, EIR);
+ eir = intel_uncore_read(uncore, EIR);
if (eir) {
/*
* some errors might have become stuck,
* mask them.
*/
DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masking\n", eir);
- I915_WRITE(EMR, I915_READ(EMR) | eir);
- I915_WRITE(IIR, I915_MASTER_ERROR_INTERRUPT);
+ rmw_register(uncore, EMR, 0, eir);
+ intel_uncore_write(uncore, IIR, I915_MASTER_ERROR_INTERRUPT);
}
- if (INTEL_GEN(dev_priv) >= 8) {
- I915_WRITE(GEN8_RING_FAULT_REG,
- I915_READ(GEN8_RING_FAULT_REG) & ~RING_FAULT_VALID);
- POSTING_READ(GEN8_RING_FAULT_REG);
- } else if (INTEL_GEN(dev_priv) >= 6) {
+ if (INTEL_GEN(i915) >= 8) {
+ rmw_register(uncore, GEN8_RING_FAULT_REG, RING_FAULT_VALID, 0);
+ intel_uncore_posting_read(uncore, GEN8_RING_FAULT_REG);
+ } else if (INTEL_GEN(i915) >= 6) {
struct intel_engine_cs *engine;
enum intel_engine_id id;
- for_each_engine(engine, dev_priv, id) {
- I915_WRITE(RING_FAULT_REG(engine),
- I915_READ(RING_FAULT_REG(engine)) &
- ~RING_FAULT_VALID);
+ for_each_engine(engine, i915, id) {
+ rmw_register(uncore,
+ RING_FAULT_REG(engine),
+ RING_FAULT_VALID, 0);
+ intel_uncore_posting_read(uncore,
+ RING_FAULT_REG(engine));
}
- POSTING_READ(RING_FAULT_REG(dev_priv->engine[RCS0]));
}
}
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Make use of 'engine->uncore'
2019-04-05 18:57 [PATCH 1/2] drm/i915: Make use of 'engine->uncore' Chris Wilson
2019-04-05 18:57 ` [PATCH 2/2] drm/i915: Convert i915_reset.c over to using uncore mmio Chris Wilson
@ 2019-04-05 18:59 ` Chris Wilson
2019-04-05 20:15 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] " Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-04-05 18:59 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Quoting Chris Wilson (2019-04-05 19:57:32)
> The engine has a direct link to the intel_uncore mmio handler, so make
> use of it rather than going indirectly via &engine->i915->uncore.
>
> v2: Update gen11_lock_sfc() to use engine->uncore as well
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
I'll presume Daniele's r-b carries over,
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Convert i915_reset.c over to using uncore mmio
2019-04-05 18:57 ` [PATCH 2/2] drm/i915: Convert i915_reset.c over to using uncore mmio Chris Wilson
@ 2019-04-05 20:06 ` Paulo Zanoni
2019-04-05 20:24 ` [PATCH v2] " Chris Wilson
1 sibling, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2019-04-05 20:06 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Em sex, 2019-04-05 às 19:57 +0100, Chris Wilson escreveu:
> Currently i915_reset.c mixes calls to intel_uncore, pci and our old
> style I915_READ mmio interfaces. Cast aside the old implicit macros,
> and harmonise on using uncore throughout.
>
> add/remove: 1/1 grow/shrink: 0/4 up/down: 65/-207 (-142)
> Function old new delta
> rmw_register - 65 +65
> gen8_reset_engines 945 942 -3
> g4x_do_reset 407 376 -31
> intel_gpu_reset 545 509 -36
> clear_register 63 - -63
> i915_clear_error_registers 461 387 -74
>
> A little bit of pointer dancing elimination works wonders.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reset.c | 125 +++++++++++++++++-------------
> 1 file changed, 73 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> index d44dc8422e8c..f50534a833ca 100644
> --- a/drivers/gpu/drm/i915/i915_reset.c
> +++ b/drivers/gpu/drm/i915/i915_reset.c
> @@ -18,6 +18,28 @@
> /* XXX How to handle concurrent GGTT updates using tiling registers? */
> #define RESET_UNDER_STOP_MACHINE 0
>
> +static void rmw_register(struct intel_uncore *uncore,
> + i915_reg_t reg, u32 clr, u32 set)
> +{
> + u32 val;
> +
> + val = intel_uncore_read(uncore, reg);
> + val &= ~clr;
> + val |= set;
> + intel_uncore_write(uncore, reg, val);
> +}
> +
> +static void rmw_register_fw(struct intel_uncore *uncore,
> + i915_reg_t reg, u32 clr, u32 set)
> +{
> + u32 val;
> +
> + val = intel_uncore_read_fw(uncore, reg);
> + val &= ~clr;
> + val |= set;
> + intel_uncore_write_fw(uncore, reg, val);
> +}
I like the idea here, maybe we could expose these concepts to the whole
driver.
But one thing I did notice is that in all of the calls there's an
argument that's zero. It's easy to get confused on the set/clear
parameter order and make a mistake. Also, when reading the code we have
to keep remembering which one comes first. Perhaps some small wrappers
for the cases where we only set or clear something (currently, 100% of
the cases) would make the code even more readable:
static inline void rmw_register_set(uncore, reg, bits_to_set)
{
rmw_register(uncore, reg, 0, bits);
}
static inline void rmw_register_clear(uncore, reg, bits_to_clear)
{
rmw_register(uncore, reg, bits, 0);
}
IMHO that would make the code much more comfortable to read. But maybe
that's because my brain is just too small.
> +
> static void engine_skip_context(struct i915_request *rq)
> {
> struct intel_engine_cs *engine = rq->engine;
> @@ -119,7 +141,7 @@ void i915_reset_request(struct i915_request *rq, bool guilty)
>
> static void gen3_stop_engine(struct intel_engine_cs *engine)
> {
> - struct drm_i915_private *dev_priv = engine->i915;
> + struct intel_uncore *uncore = engine->uncore;
> const u32 base = engine->mmio_base;
>
> GEM_TRACE("%s\n", engine->name);
> @@ -127,20 +149,23 @@ static void gen3_stop_engine(struct intel_engine_cs *engine)
> if (intel_engine_stop_cs(engine))
> GEM_TRACE("%s: timed out on STOP_RING\n", engine->name);
>
> - I915_WRITE_FW(RING_HEAD(base), I915_READ_FW(RING_TAIL(base)));
> - POSTING_READ_FW(RING_HEAD(base)); /* paranoia */
> + intel_uncore_write_fw(uncore,
> + RING_HEAD(base),
> + intel_uncore_read_fw(uncore, RING_TAIL(base)));
> + intel_uncore_posting_read_fw(uncore, RING_HEAD(base)); /* paranoia */
>
> - I915_WRITE_FW(RING_HEAD(base), 0);
> - I915_WRITE_FW(RING_TAIL(base), 0);
> - POSTING_READ_FW(RING_TAIL(base));
> + intel_uncore_write_fw(uncore, RING_HEAD(base), 0);
> + intel_uncore_write_fw(uncore, RING_TAIL(base), 0);
> + intel_uncore_posting_read_fw(uncore, RING_TAIL(base));
>
> /* The ring must be empty before it is disabled */
> - I915_WRITE_FW(RING_CTL(base), 0);
> + intel_uncore_write_fw(uncore, RING_CTL(base), 0);
>
> /* Check acts as a post */
> - if (I915_READ_FW(RING_HEAD(base)))
> + if (intel_uncore_read_fw(uncore, RING_HEAD(base)))
> GEM_TRACE("%s: ring head [%x] not parked\n",
> - engine->name, I915_READ_FW(RING_HEAD(base)));
> + engine->name,
> + intel_uncore_read_fw(uncore, RING_HEAD(base)));
> }
>
> static void i915_stop_engines(struct drm_i915_private *i915,
> @@ -203,17 +228,17 @@ static int g33_do_reset(struct drm_i915_private *i915,
> return wait_for_atomic(g4x_reset_complete(pdev), 50);
> }
>
> -static int g4x_do_reset(struct drm_i915_private *dev_priv,
> +static int g4x_do_reset(struct drm_i915_private *i915,
> intel_engine_mask_t engine_mask,
> unsigned int retry)
> {
> - struct pci_dev *pdev = dev_priv->drm.pdev;
> + struct pci_dev *pdev = i915->drm.pdev;
> + struct intel_uncore *uncore = &i915->uncore;
> int ret;
>
> /* WaVcpClkGateDisableForMediaReset:ctg,elk */
> - I915_WRITE_FW(VDECCLK_GATE_D,
> - I915_READ(VDECCLK_GATE_D) | VCP_UNIT_CLOCK_GATE_DISABLE);
> - POSTING_READ_FW(VDECCLK_GATE_D);
> + rmw_register_fw(uncore, VDECCLK_GATE_D, 0, VCP_UNIT_CLOCK_GATE_DISABLE);
> + intel_uncore_posting_read_fw(uncore, VDECCLK_GATE_D);
>
> pci_write_config_byte(pdev, I915_GDRST,
> GRDOM_MEDIA | GRDOM_RESET_ENABLE);
> @@ -234,18 +259,17 @@ static int g4x_do_reset(struct drm_i915_private *dev_priv,
> out:
> pci_write_config_byte(pdev, I915_GDRST, 0);
>
> - I915_WRITE_FW(VDECCLK_GATE_D,
> - I915_READ(VDECCLK_GATE_D) & ~VCP_UNIT_CLOCK_GATE_DISABLE);
> - POSTING_READ_FW(VDECCLK_GATE_D);
> + rmw_register_fw(uncore, VDECCLK_GATE_D, VCP_UNIT_CLOCK_GATE_DISABLE, 0);
> + intel_uncore_posting_read_fw(uncore, VDECCLK_GATE_D);
>
> return ret;
> }
>
> -static int ironlake_do_reset(struct drm_i915_private *dev_priv,
> +static int ironlake_do_reset(struct drm_i915_private *i915,
> intel_engine_mask_t engine_mask,
> unsigned int retry)
> {
> - struct intel_uncore *uncore = &dev_priv->uncore;
> + struct intel_uncore *uncore = &i915->uncore;
> int ret;
>
> intel_uncore_write_fw(uncore, ILK_GDSR,
> @@ -277,10 +301,10 @@ static int ironlake_do_reset(struct drm_i915_private *dev_priv,
> }
>
> /* Reset the hardware domains (GENX_GRDOM_*) specified by mask */
> -static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
> +static int gen6_hw_domain_reset(struct drm_i915_private *i915,
> u32 hw_domain_mask)
> {
> - struct intel_uncore *uncore = &dev_priv->uncore;
> + struct intel_uncore *uncore = &i915->uncore;
> int err;
>
> /*
> @@ -381,7 +405,7 @@ static u32 gen11_lock_sfc(struct intel_engine_cs *engine)
> * ends up being locked to the engine we want to reset, we have to reset
> * it as well (we will unlock it once the reset sequence is completed).
> */
> - intel_uncore_rmw_or_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
We killed the only caller for that, might as well kill the definition.
Btw this name is (was) very confusing to me.
> + rmw_register_fw(uncore, sfc_forced_lock, 0, sfc_forced_lock_bit);
>
> if (__intel_wait_for_register_fw(uncore,
> sfc_forced_lock_ack,
> @@ -404,7 +428,6 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine)
> u8 vdbox_sfc_access = RUNTIME_INFO(engine->i915)->vdbox_sfc_access;
> i915_reg_t sfc_forced_lock;
> u32 sfc_forced_lock_bit;
> - u32 val;
>
> switch (engine->class) {
> case VIDEO_DECODE_CLASS:
> @@ -424,9 +447,7 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine)
> return;
> }
>
> - val = intel_uncore_read_fw(uncore, sfc_forced_lock);
> - val &= ~sfc_forced_lock_bit;
> - intel_uncore_write_fw(uncore, sfc_forced_lock, val);
> + rmw_register_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit, 0);
> }
>
> static int gen11_reset_engines(struct drm_i915_private *i915,
> @@ -491,10 +512,9 @@ static int gen8_engine_reset_prepare(struct intel_engine_cs *engine)
>
> static void gen8_engine_reset_cancel(struct intel_engine_cs *engine)
> {
> - struct drm_i915_private *dev_priv = engine->i915;
> -
> - I915_WRITE_FW(RING_RESET_CTL(engine->mmio_base),
> - _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
> + intel_uncore_write_fw(engine->uncore,
> + RING_RESET_CTL(engine->mmio_base),
> + _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
> }
>
> static int gen8_reset_engines(struct drm_i915_private *i915,
> @@ -1178,49 +1198,50 @@ static void i915_reset_device(struct drm_i915_private *i915,
> kobject_uevent_env(kobj, KOBJ_CHANGE, reset_done_event);
> }
>
> -static void clear_register(struct drm_i915_private *dev_priv, i915_reg_t reg)
> +static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
> {
> - I915_WRITE(reg, I915_READ(reg));
> + rmw_register(uncore, reg, 0, 0);
> }
>
> -void i915_clear_error_registers(struct drm_i915_private *dev_priv)
> +void i915_clear_error_registers(struct drm_i915_private *i915)
> {
> + struct intel_uncore *uncore = &i915->uncore;
> u32 eir;
>
> - if (!IS_GEN(dev_priv, 2))
> - clear_register(dev_priv, PGTBL_ER);
> + if (!IS_GEN(i915, 2))
> + clear_register(uncore, PGTBL_ER);
>
> - if (INTEL_GEN(dev_priv) < 4)
> - clear_register(dev_priv, IPEIR(RENDER_RING_BASE));
> + if (INTEL_GEN(i915) < 4)
> + clear_register(uncore, IPEIR(RENDER_RING_BASE));
> else
> - clear_register(dev_priv, IPEIR_I965);
> + clear_register(uncore, IPEIR_I965);
>
> - clear_register(dev_priv, EIR);
> - eir = I915_READ(EIR);
> + clear_register(uncore, EIR);
> + eir = intel_uncore_read(uncore, EIR);
> if (eir) {
> /*
> * some errors might have become stuck,
> * mask them.
> */
> DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masking\n", eir);
> - I915_WRITE(EMR, I915_READ(EMR) | eir);
> - I915_WRITE(IIR, I915_MASTER_ERROR_INTERRUPT);
> + rmw_register(uncore, EMR, 0, eir);
> + intel_uncore_write(uncore, IIR, I915_MASTER_ERROR_INTERRUPT);
> }
>
> - if (INTEL_GEN(dev_priv) >= 8) {
> - I915_WRITE(GEN8_RING_FAULT_REG,
> - I915_READ(GEN8_RING_FAULT_REG) & ~RING_FAULT_VALID);
> - POSTING_READ(GEN8_RING_FAULT_REG);
> - } else if (INTEL_GEN(dev_priv) >= 6) {
> + if (INTEL_GEN(i915) >= 8) {
> + rmw_register(uncore, GEN8_RING_FAULT_REG, RING_FAULT_VALID, 0);
> + intel_uncore_posting_read(uncore, GEN8_RING_FAULT_REG);
> + } else if (INTEL_GEN(i915) >= 6) {
> struct intel_engine_cs *engine;
> enum intel_engine_id id;
>
> - for_each_engine(engine, dev_priv, id) {
> - I915_WRITE(RING_FAULT_REG(engine),
> - I915_READ(RING_FAULT_REG(engine)) &
> - ~RING_FAULT_VALID);
> + for_each_engine(engine, i915, id) {
> + rmw_register(uncore,
> + RING_FAULT_REG(engine),
> + RING_FAULT_VALID, 0);
> + intel_uncore_posting_read(uncore,
> + RING_FAULT_REG(engine));
> }
> - POSTING_READ(RING_FAULT_REG(dev_priv->engine[RCS0]));
That's a small behavior change, I'll assume it's on purpose.
With or without changes (but I'd really love to see the clr/set
wrappers):
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
(r-b on patch 1 also stands in the newer version)
> }
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Make use of 'engine->uncore'
2019-04-05 18:57 [PATCH 1/2] drm/i915: Make use of 'engine->uncore' Chris Wilson
2019-04-05 18:57 ` [PATCH 2/2] drm/i915: Convert i915_reset.c over to using uncore mmio Chris Wilson
2019-04-05 18:59 ` [PATCH 1/2] drm/i915: Make use of 'engine->uncore' Chris Wilson
@ 2019-04-05 20:15 ` Patchwork
2019-04-05 20:59 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Make use of 'engine->uncore' (rev2) Patchwork
2019-04-06 18:49 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-04-05 20:15 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Make use of 'engine->uncore'
URL : https://patchwork.freedesktop.org/series/59081/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5881 -> Patchwork_12707
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_12707 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_12707, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/59081/revisions/1/mbox/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_12707:
### IGT changes ###
#### Possible regressions ####
* igt@gem_ctx_exec@basic:
- fi-icl-u2: PASS -> INCOMPLETE
Known issues
------------
Here are the changes found in Patchwork_12707 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_basic@basic-blt:
- fi-icl-y: PASS -> INCOMPLETE [fdo#110246]
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]
* igt@kms_frontbuffer_tracking@basic:
- fi-byt-clapper: PASS -> FAIL [fdo#103167]
* igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362] +2
#### Possible fixes ####
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: FAIL [fdo#108511] -> PASS
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
[fdo#110246]: https://bugs.freedesktop.org/show_bug.cgi?id=110246
Participating hosts (49 -> 44)
------------------------------
Missing (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-icl-u3 fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_5881 -> Patchwork_12707
CI_DRM_5881: b070175c76da1440a747fd023ee6253e573055f8 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4931: 019f892e5d1a0a9643cb726c47ce2d99c14b444f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12707: 4d957624fef6474fda91923240a31aa67d51fca8 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
4d957624fef6 drm/i915: Convert i915_reset.c over to using uncore mmio
4ec5cd61abbc drm/i915: Make use of 'engine->uncore'
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12707/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] drm/i915: Convert i915_reset.c over to using uncore mmio
2019-04-05 18:57 ` [PATCH 2/2] drm/i915: Convert i915_reset.c over to using uncore mmio Chris Wilson
2019-04-05 20:06 ` Paulo Zanoni
@ 2019-04-05 20:24 ` Chris Wilson
2019-04-05 20:39 ` Paulo Zanoni
2019-04-05 20:39 ` Lucas De Marchi
1 sibling, 2 replies; 12+ messages in thread
From: Chris Wilson @ 2019-04-05 20:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Currently i915_reset.c mixes calls to intel_uncore, pci and our old
style I915_READ mmio interfaces. Cast aside the old implicit macros,
and harmonise on using uncore throughout.
add/remove: 1/1 grow/shrink: 0/4 up/down: 65/-207 (-142)
Function old new delta
rmw_register - 65 +65
gen8_reset_engines 945 942 -3
g4x_do_reset 407 376 -31
intel_gpu_reset 545 509 -36
clear_register 63 - -63
i915_clear_error_registers 461 387 -74
A little bit of pointer dancing elimination works wonders.
v2: Roll up the helpers into intel_uncore for general use
With the helpers gcc was a little more eager to inline:
add/remove: 0/1 grow/shrink: 1/3 up/down: 99/-133 (-34)
Function old new delta
i915_clear_error_registers 461 560 +99
gen8_reset_engines 945 942 -3
g4x_do_reset 407 376 -31
intel_gpu_reset 545 509 -36
clear_register 63 - -63
Total: Before=1544400, After=1544366, chg -0.00%
Win some, lose some, gcc is gcc.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reset.c | 122 ++++++++++++++++------------
drivers/gpu/drm/i915/intel_uncore.h | 23 +++++-
2 files changed, 89 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
index d44dc8422e8c..ac168de6a66e 100644
--- a/drivers/gpu/drm/i915/i915_reset.c
+++ b/drivers/gpu/drm/i915/i915_reset.c
@@ -18,6 +18,26 @@
/* XXX How to handle concurrent GGTT updates using tiling registers? */
#define RESET_UNDER_STOP_MACHINE 0
+static void rmw_set(struct intel_uncore *uncore, i915_reg_t reg, u32 set)
+{
+ intel_uncore_rmw(uncore, reg, 0, set);
+}
+
+static void rmw_clear(struct intel_uncore *uncore, i915_reg_t reg, u32 clr)
+{
+ intel_uncore_rmw(uncore, reg, clr, 0);
+}
+
+static void rwm_set_fw(struct intel_uncore *uncore, i915_reg_t reg, u32 set)
+{
+ intel_uncore_rmw_fw(uncore, reg, 0, set);
+}
+
+static void rmw_clear_fw(struct intel_uncore *uncore, i915_reg_t reg, u32 clr)
+{
+ intel_uncore_rmw_fw(uncore, reg, clr, 0);
+}
+
static void engine_skip_context(struct i915_request *rq)
{
struct intel_engine_cs *engine = rq->engine;
@@ -119,7 +139,7 @@ void i915_reset_request(struct i915_request *rq, bool guilty)
static void gen3_stop_engine(struct intel_engine_cs *engine)
{
- struct drm_i915_private *dev_priv = engine->i915;
+ struct intel_uncore *uncore = engine->uncore;
const u32 base = engine->mmio_base;
GEM_TRACE("%s\n", engine->name);
@@ -127,20 +147,23 @@ static void gen3_stop_engine(struct intel_engine_cs *engine)
if (intel_engine_stop_cs(engine))
GEM_TRACE("%s: timed out on STOP_RING\n", engine->name);
- I915_WRITE_FW(RING_HEAD(base), I915_READ_FW(RING_TAIL(base)));
- POSTING_READ_FW(RING_HEAD(base)); /* paranoia */
+ intel_uncore_write_fw(uncore,
+ RING_HEAD(base),
+ intel_uncore_read_fw(uncore, RING_TAIL(base)));
+ intel_uncore_posting_read_fw(uncore, RING_HEAD(base)); /* paranoia */
- I915_WRITE_FW(RING_HEAD(base), 0);
- I915_WRITE_FW(RING_TAIL(base), 0);
- POSTING_READ_FW(RING_TAIL(base));
+ intel_uncore_write_fw(uncore, RING_HEAD(base), 0);
+ intel_uncore_write_fw(uncore, RING_TAIL(base), 0);
+ intel_uncore_posting_read_fw(uncore, RING_TAIL(base));
/* The ring must be empty before it is disabled */
- I915_WRITE_FW(RING_CTL(base), 0);
+ intel_uncore_write_fw(uncore, RING_CTL(base), 0);
/* Check acts as a post */
- if (I915_READ_FW(RING_HEAD(base)))
+ if (intel_uncore_read_fw(uncore, RING_HEAD(base)))
GEM_TRACE("%s: ring head [%x] not parked\n",
- engine->name, I915_READ_FW(RING_HEAD(base)));
+ engine->name,
+ intel_uncore_read_fw(uncore, RING_HEAD(base)));
}
static void i915_stop_engines(struct drm_i915_private *i915,
@@ -203,17 +226,17 @@ static int g33_do_reset(struct drm_i915_private *i915,
return wait_for_atomic(g4x_reset_complete(pdev), 50);
}
-static int g4x_do_reset(struct drm_i915_private *dev_priv,
+static int g4x_do_reset(struct drm_i915_private *i915,
intel_engine_mask_t engine_mask,
unsigned int retry)
{
- struct pci_dev *pdev = dev_priv->drm.pdev;
+ struct pci_dev *pdev = i915->drm.pdev;
+ struct intel_uncore *uncore = &i915->uncore;
int ret;
/* WaVcpClkGateDisableForMediaReset:ctg,elk */
- I915_WRITE_FW(VDECCLK_GATE_D,
- I915_READ(VDECCLK_GATE_D) | VCP_UNIT_CLOCK_GATE_DISABLE);
- POSTING_READ_FW(VDECCLK_GATE_D);
+ rwm_set_fw(uncore, VDECCLK_GATE_D, VCP_UNIT_CLOCK_GATE_DISABLE);
+ intel_uncore_posting_read_fw(uncore, VDECCLK_GATE_D);
pci_write_config_byte(pdev, I915_GDRST,
GRDOM_MEDIA | GRDOM_RESET_ENABLE);
@@ -234,18 +257,17 @@ static int g4x_do_reset(struct drm_i915_private *dev_priv,
out:
pci_write_config_byte(pdev, I915_GDRST, 0);
- I915_WRITE_FW(VDECCLK_GATE_D,
- I915_READ(VDECCLK_GATE_D) & ~VCP_UNIT_CLOCK_GATE_DISABLE);
- POSTING_READ_FW(VDECCLK_GATE_D);
+ rmw_clear_fw(uncore, VDECCLK_GATE_D, VCP_UNIT_CLOCK_GATE_DISABLE);
+ intel_uncore_posting_read_fw(uncore, VDECCLK_GATE_D);
return ret;
}
-static int ironlake_do_reset(struct drm_i915_private *dev_priv,
+static int ironlake_do_reset(struct drm_i915_private *i915,
intel_engine_mask_t engine_mask,
unsigned int retry)
{
- struct intel_uncore *uncore = &dev_priv->uncore;
+ struct intel_uncore *uncore = &i915->uncore;
int ret;
intel_uncore_write_fw(uncore, ILK_GDSR,
@@ -277,10 +299,10 @@ static int ironlake_do_reset(struct drm_i915_private *dev_priv,
}
/* Reset the hardware domains (GENX_GRDOM_*) specified by mask */
-static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
+static int gen6_hw_domain_reset(struct drm_i915_private *i915,
u32 hw_domain_mask)
{
- struct intel_uncore *uncore = &dev_priv->uncore;
+ struct intel_uncore *uncore = &i915->uncore;
int err;
/*
@@ -381,7 +403,7 @@ static u32 gen11_lock_sfc(struct intel_engine_cs *engine)
* ends up being locked to the engine we want to reset, we have to reset
* it as well (we will unlock it once the reset sequence is completed).
*/
- intel_uncore_rmw_or_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
+ rwm_set_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
if (__intel_wait_for_register_fw(uncore,
sfc_forced_lock_ack,
@@ -404,7 +426,6 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine)
u8 vdbox_sfc_access = RUNTIME_INFO(engine->i915)->vdbox_sfc_access;
i915_reg_t sfc_forced_lock;
u32 sfc_forced_lock_bit;
- u32 val;
switch (engine->class) {
case VIDEO_DECODE_CLASS:
@@ -424,9 +445,7 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine)
return;
}
- val = intel_uncore_read_fw(uncore, sfc_forced_lock);
- val &= ~sfc_forced_lock_bit;
- intel_uncore_write_fw(uncore, sfc_forced_lock, val);
+ rmw_clear_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
}
static int gen11_reset_engines(struct drm_i915_private *i915,
@@ -491,10 +510,9 @@ static int gen8_engine_reset_prepare(struct intel_engine_cs *engine)
static void gen8_engine_reset_cancel(struct intel_engine_cs *engine)
{
- struct drm_i915_private *dev_priv = engine->i915;
-
- I915_WRITE_FW(RING_RESET_CTL(engine->mmio_base),
- _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
+ intel_uncore_write_fw(engine->uncore,
+ RING_RESET_CTL(engine->mmio_base),
+ _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
}
static int gen8_reset_engines(struct drm_i915_private *i915,
@@ -1178,49 +1196,49 @@ static void i915_reset_device(struct drm_i915_private *i915,
kobject_uevent_env(kobj, KOBJ_CHANGE, reset_done_event);
}
-static void clear_register(struct drm_i915_private *dev_priv, i915_reg_t reg)
+static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
{
- I915_WRITE(reg, I915_READ(reg));
+ intel_uncore_rmw(uncore, reg, 0, 0);
}
-void i915_clear_error_registers(struct drm_i915_private *dev_priv)
+void i915_clear_error_registers(struct drm_i915_private *i915)
{
+ struct intel_uncore *uncore = &i915->uncore;
u32 eir;
- if (!IS_GEN(dev_priv, 2))
- clear_register(dev_priv, PGTBL_ER);
+ if (!IS_GEN(i915, 2))
+ clear_register(uncore, PGTBL_ER);
- if (INTEL_GEN(dev_priv) < 4)
- clear_register(dev_priv, IPEIR(RENDER_RING_BASE));
+ if (INTEL_GEN(i915) < 4)
+ clear_register(uncore, IPEIR(RENDER_RING_BASE));
else
- clear_register(dev_priv, IPEIR_I965);
+ clear_register(uncore, IPEIR_I965);
- clear_register(dev_priv, EIR);
- eir = I915_READ(EIR);
+ clear_register(uncore, EIR);
+ eir = intel_uncore_read(uncore, EIR);
if (eir) {
/*
* some errors might have become stuck,
* mask them.
*/
DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masking\n", eir);
- I915_WRITE(EMR, I915_READ(EMR) | eir);
- I915_WRITE(IIR, I915_MASTER_ERROR_INTERRUPT);
+ rmw_set(uncore, EMR, eir);
+ intel_uncore_write(uncore, IIR, I915_MASTER_ERROR_INTERRUPT);
}
- if (INTEL_GEN(dev_priv) >= 8) {
- I915_WRITE(GEN8_RING_FAULT_REG,
- I915_READ(GEN8_RING_FAULT_REG) & ~RING_FAULT_VALID);
- POSTING_READ(GEN8_RING_FAULT_REG);
- } else if (INTEL_GEN(dev_priv) >= 6) {
+ if (INTEL_GEN(i915) >= 8) {
+ rmw_clear(uncore, GEN8_RING_FAULT_REG, RING_FAULT_VALID);
+ intel_uncore_posting_read(uncore, GEN8_RING_FAULT_REG);
+ } else if (INTEL_GEN(i915) >= 6) {
struct intel_engine_cs *engine;
enum intel_engine_id id;
- for_each_engine(engine, dev_priv, id) {
- I915_WRITE(RING_FAULT_REG(engine),
- I915_READ(RING_FAULT_REG(engine)) &
- ~RING_FAULT_VALID);
+ for_each_engine(engine, i915, id) {
+ rmw_clear(uncore,
+ RING_FAULT_REG(engine), RING_FAULT_VALID);
+ intel_uncore_posting_read(uncore,
+ RING_FAULT_REG(engine));
}
- POSTING_READ(RING_FAULT_REG(dev_priv->engine[RCS0]));
}
}
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index a64d3dc0db4d..d6af3de70121 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -369,11 +369,26 @@ intel_uncore_read64_2x32(struct intel_uncore *uncore,
#define intel_uncore_write64_fw(...) __raw_uncore_write64(__VA_ARGS__)
#define intel_uncore_posting_read_fw(...) ((void)intel_uncore_read_fw(__VA_ARGS__))
-static inline void intel_uncore_rmw_or_fw(struct intel_uncore *uncore,
- i915_reg_t reg, u32 or_val)
+static inline void intel_uncore_rmw(struct intel_uncore *uncore,
+ i915_reg_t reg, u32 clear, u32 set)
{
- intel_uncore_write_fw(uncore, reg,
- intel_uncore_read_fw(uncore, reg) | or_val);
+ u32 val;
+
+ val = intel_uncore_read(uncore, reg);
+ val &= ~clear;
+ val |= set;
+ intel_uncore_write(uncore, reg, val);
+}
+
+static inline void intel_uncore_rmw_fw(struct intel_uncore *uncore,
+ i915_reg_t reg, u32 clear, u32 set)
+{
+ u32 val;
+
+ val = intel_uncore_read_fw(uncore, reg);
+ val &= ~clear;
+ val |= set;
+ intel_uncore_write_fw(uncore, reg, val);
}
#define raw_reg_read(base, reg) \
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915: Convert i915_reset.c over to using uncore mmio
2019-04-05 20:24 ` [PATCH v2] " Chris Wilson
@ 2019-04-05 20:39 ` Paulo Zanoni
2019-04-05 20:39 ` Lucas De Marchi
1 sibling, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2019-04-05 20:39 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Em sex, 2019-04-05 às 21:24 +0100, Chris Wilson escreveu:
> Currently i915_reset.c mixes calls to intel_uncore, pci and our old
> style I915_READ mmio interfaces. Cast aside the old implicit macros,
> and harmonise on using uncore throughout.
>
> add/remove: 1/1 grow/shrink: 0/4 up/down: 65/-207 (-142)
> Function old new delta
> rmw_register - 65 +65
> gen8_reset_engines 945 942 -3
> g4x_do_reset 407 376 -31
> intel_gpu_reset 545 509 -36
> clear_register 63 - -63
> i915_clear_error_registers 461 387 -74
>
> A little bit of pointer dancing elimination works wonders.
>
> v2: Roll up the helpers into intel_uncore for general use
>
> With the helpers gcc was a little more eager to inline:
> add/remove: 0/1 grow/shrink: 1/3 up/down: 99/-133 (-34)
> Function old new delta
> i915_clear_error_registers 461 560 +99
> gen8_reset_engines 945 942 -3
> g4x_do_reset 407 376 -31
> intel_gpu_reset 545 509 -36
> clear_register 63 - -63
> Total: Before=1544400, After=1544366, chg -0.00%
>
> Win some, lose some, gcc is gcc.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reset.c | 122 ++++++++++++++++------------
> drivers/gpu/drm/i915/intel_uncore.h | 23 +++++-
> 2 files changed, 89 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> index d44dc8422e8c..ac168de6a66e 100644
> --- a/drivers/gpu/drm/i915/i915_reset.c
> +++ b/drivers/gpu/drm/i915/i915_reset.c
> @@ -18,6 +18,26 @@
> /* XXX How to handle concurrent GGTT updates using tiling registers? */
> #define RESET_UNDER_STOP_MACHINE 0
>
> +static void rmw_set(struct intel_uncore *uncore, i915_reg_t reg, u32 set)
> +{
> + intel_uncore_rmw(uncore, reg, 0, set);
> +}
> +
> +static void rmw_clear(struct intel_uncore *uncore, i915_reg_t reg, u32 clr)
> +{
> + intel_uncore_rmw(uncore, reg, clr, 0);
> +}
> +
> +static void rwm_set_fw(struct intel_uncore *uncore, i915_reg_t reg, u32 set)
We're not reading, writing and then modifying :).
> +{
> + intel_uncore_rmw_fw(uncore, reg, 0, set);
> +}
> +
> +static void rmw_clear_fw(struct intel_uncore *uncore, i915_reg_t reg, u32 clr)
> +{
> + intel_uncore_rmw_fw(uncore, reg, clr, 0);
> +}
> +
> static void engine_skip_context(struct i915_request *rq)
> {
> struct intel_engine_cs *engine = rq->engine;
> @@ -119,7 +139,7 @@ void i915_reset_request(struct i915_request *rq, bool guilty)
>
> static void gen3_stop_engine(struct intel_engine_cs *engine)
> {
> - struct drm_i915_private *dev_priv = engine->i915;
> + struct intel_uncore *uncore = engine->uncore;
> const u32 base = engine->mmio_base;
>
> GEM_TRACE("%s\n", engine->name);
> @@ -127,20 +147,23 @@ static void gen3_stop_engine(struct intel_engine_cs *engine)
> if (intel_engine_stop_cs(engine))
> GEM_TRACE("%s: timed out on STOP_RING\n", engine->name);
>
> - I915_WRITE_FW(RING_HEAD(base), I915_READ_FW(RING_TAIL(base)));
> - POSTING_READ_FW(RING_HEAD(base)); /* paranoia */
> + intel_uncore_write_fw(uncore,
> + RING_HEAD(base),
> + intel_uncore_read_fw(uncore, RING_TAIL(base)));
> + intel_uncore_posting_read_fw(uncore, RING_HEAD(base)); /* paranoia */
>
> - I915_WRITE_FW(RING_HEAD(base), 0);
> - I915_WRITE_FW(RING_TAIL(base), 0);
> - POSTING_READ_FW(RING_TAIL(base));
> + intel_uncore_write_fw(uncore, RING_HEAD(base), 0);
> + intel_uncore_write_fw(uncore, RING_TAIL(base), 0);
> + intel_uncore_posting_read_fw(uncore, RING_TAIL(base));
>
> /* The ring must be empty before it is disabled */
> - I915_WRITE_FW(RING_CTL(base), 0);
> + intel_uncore_write_fw(uncore, RING_CTL(base), 0);
>
> /* Check acts as a post */
> - if (I915_READ_FW(RING_HEAD(base)))
> + if (intel_uncore_read_fw(uncore, RING_HEAD(base)))
> GEM_TRACE("%s: ring head [%x] not parked\n",
> - engine->name, I915_READ_FW(RING_HEAD(base)));
> + engine->name,
> + intel_uncore_read_fw(uncore, RING_HEAD(base)));
> }
>
> static void i915_stop_engines(struct drm_i915_private *i915,
> @@ -203,17 +226,17 @@ static int g33_do_reset(struct drm_i915_private *i915,
> return wait_for_atomic(g4x_reset_complete(pdev), 50);
> }
>
> -static int g4x_do_reset(struct drm_i915_private *dev_priv,
> +static int g4x_do_reset(struct drm_i915_private *i915,
> intel_engine_mask_t engine_mask,
> unsigned int retry)
> {
> - struct pci_dev *pdev = dev_priv->drm.pdev;
> + struct pci_dev *pdev = i915->drm.pdev;
> + struct intel_uncore *uncore = &i915->uncore;
> int ret;
>
> /* WaVcpClkGateDisableForMediaReset:ctg,elk */
> - I915_WRITE_FW(VDECCLK_GATE_D,
> - I915_READ(VDECCLK_GATE_D) | VCP_UNIT_CLOCK_GATE_DISABLE);
> - POSTING_READ_FW(VDECCLK_GATE_D);
> + rwm_set_fw(uncore, VDECCLK_GATE_D, VCP_UNIT_CLOCK_GATE_DISABLE);
> + intel_uncore_posting_read_fw(uncore, VDECCLK_GATE_D);
>
> pci_write_config_byte(pdev, I915_GDRST,
> GRDOM_MEDIA | GRDOM_RESET_ENABLE);
> @@ -234,18 +257,17 @@ static int g4x_do_reset(struct drm_i915_private *dev_priv,
> out:
> pci_write_config_byte(pdev, I915_GDRST, 0);
>
> - I915_WRITE_FW(VDECCLK_GATE_D,
> - I915_READ(VDECCLK_GATE_D) & ~VCP_UNIT_CLOCK_GATE_DISABLE);
> - POSTING_READ_FW(VDECCLK_GATE_D);
> + rmw_clear_fw(uncore, VDECCLK_GATE_D, VCP_UNIT_CLOCK_GATE_DISABLE);
> + intel_uncore_posting_read_fw(uncore, VDECCLK_GATE_D);
>
> return ret;
> }
>
> -static int ironlake_do_reset(struct drm_i915_private *dev_priv,
> +static int ironlake_do_reset(struct drm_i915_private *i915,
> intel_engine_mask_t engine_mask,
> unsigned int retry)
> {
> - struct intel_uncore *uncore = &dev_priv->uncore;
> + struct intel_uncore *uncore = &i915->uncore;
> int ret;
>
> intel_uncore_write_fw(uncore, ILK_GDSR,
> @@ -277,10 +299,10 @@ static int ironlake_do_reset(struct drm_i915_private *dev_priv,
> }
>
> /* Reset the hardware domains (GENX_GRDOM_*) specified by mask */
> -static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
> +static int gen6_hw_domain_reset(struct drm_i915_private *i915,
> u32 hw_domain_mask)
> {
> - struct intel_uncore *uncore = &dev_priv->uncore;
> + struct intel_uncore *uncore = &i915->uncore;
> int err;
>
> /*
> @@ -381,7 +403,7 @@ static u32 gen11_lock_sfc(struct intel_engine_cs *engine)
> * ends up being locked to the engine we want to reset, we have to reset
> * it as well (we will unlock it once the reset sequence is completed).
> */
> - intel_uncore_rmw_or_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
> + rwm_set_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
>
> if (__intel_wait_for_register_fw(uncore,
> sfc_forced_lock_ack,
> @@ -404,7 +426,6 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine)
> u8 vdbox_sfc_access = RUNTIME_INFO(engine->i915)->vdbox_sfc_access;
> i915_reg_t sfc_forced_lock;
> u32 sfc_forced_lock_bit;
> - u32 val;
>
> switch (engine->class) {
> case VIDEO_DECODE_CLASS:
> @@ -424,9 +445,7 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine)
> return;
> }
>
> - val = intel_uncore_read_fw(uncore, sfc_forced_lock);
> - val &= ~sfc_forced_lock_bit;
> - intel_uncore_write_fw(uncore, sfc_forced_lock, val);
> + rmw_clear_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
> }
>
> static int gen11_reset_engines(struct drm_i915_private *i915,
> @@ -491,10 +510,9 @@ static int gen8_engine_reset_prepare(struct intel_engine_cs *engine)
>
> static void gen8_engine_reset_cancel(struct intel_engine_cs *engine)
> {
> - struct drm_i915_private *dev_priv = engine->i915;
> -
> - I915_WRITE_FW(RING_RESET_CTL(engine->mmio_base),
> - _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
> + intel_uncore_write_fw(engine->uncore,
> + RING_RESET_CTL(engine->mmio_base),
> + _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
> }
>
> static int gen8_reset_engines(struct drm_i915_private *i915,
> @@ -1178,49 +1196,49 @@ static void i915_reset_device(struct drm_i915_private *i915,
> kobject_uevent_env(kobj, KOBJ_CHANGE, reset_done_event);
> }
>
> -static void clear_register(struct drm_i915_private *dev_priv, i915_reg_t reg)
> +static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
> {
> - I915_WRITE(reg, I915_READ(reg));
> + intel_uncore_rmw(uncore, reg, 0, 0);
> }
>
> -void i915_clear_error_registers(struct drm_i915_private *dev_priv)
> +void i915_clear_error_registers(struct drm_i915_private *i915)
> {
> + struct intel_uncore *uncore = &i915->uncore;
> u32 eir;
>
> - if (!IS_GEN(dev_priv, 2))
> - clear_register(dev_priv, PGTBL_ER);
> + if (!IS_GEN(i915, 2))
> + clear_register(uncore, PGTBL_ER);
>
> - if (INTEL_GEN(dev_priv) < 4)
> - clear_register(dev_priv, IPEIR(RENDER_RING_BASE));
> + if (INTEL_GEN(i915) < 4)
> + clear_register(uncore, IPEIR(RENDER_RING_BASE));
> else
> - clear_register(dev_priv, IPEIR_I965);
> + clear_register(uncore, IPEIR_I965);
>
> - clear_register(dev_priv, EIR);
> - eir = I915_READ(EIR);
> + clear_register(uncore, EIR);
> + eir = intel_uncore_read(uncore, EIR);
> if (eir) {
> /*
> * some errors might have become stuck,
> * mask them.
> */
> DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masking\n", eir);
> - I915_WRITE(EMR, I915_READ(EMR) | eir);
> - I915_WRITE(IIR, I915_MASTER_ERROR_INTERRUPT);
> + rmw_set(uncore, EMR, eir);
> + intel_uncore_write(uncore, IIR, I915_MASTER_ERROR_INTERRUPT);
> }
>
> - if (INTEL_GEN(dev_priv) >= 8) {
> - I915_WRITE(GEN8_RING_FAULT_REG,
> - I915_READ(GEN8_RING_FAULT_REG) & ~RING_FAULT_VALID);
> - POSTING_READ(GEN8_RING_FAULT_REG);
> - } else if (INTEL_GEN(dev_priv) >= 6) {
> + if (INTEL_GEN(i915) >= 8) {
> + rmw_clear(uncore, GEN8_RING_FAULT_REG, RING_FAULT_VALID);
> + intel_uncore_posting_read(uncore, GEN8_RING_FAULT_REG);
> + } else if (INTEL_GEN(i915) >= 6) {
> struct intel_engine_cs *engine;
> enum intel_engine_id id;
>
> - for_each_engine(engine, dev_priv, id) {
> - I915_WRITE(RING_FAULT_REG(engine),
> - I915_READ(RING_FAULT_REG(engine)) &
> - ~RING_FAULT_VALID);
> + for_each_engine(engine, i915, id) {
> + rmw_clear(uncore,
> + RING_FAULT_REG(engine), RING_FAULT_VALID);
> + intel_uncore_posting_read(uncore,
> + RING_FAULT_REG(engine));
> }
> - POSTING_READ(RING_FAULT_REG(dev_priv->engine[RCS0]));
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index a64d3dc0db4d..d6af3de70121 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -369,11 +369,26 @@ intel_uncore_read64_2x32(struct intel_uncore *uncore,
> #define intel_uncore_write64_fw(...) __raw_uncore_write64(__VA_ARGS__)
> #define intel_uncore_posting_read_fw(...) ((void)intel_uncore_read_fw(__VA_ARGS__))
>
> -static inline void intel_uncore_rmw_or_fw(struct intel_uncore *uncore,
> - i915_reg_t reg, u32 or_val)
> +static inline void intel_uncore_rmw(struct intel_uncore *uncore,
> + i915_reg_t reg, u32 clear, u32 set)
> {
> - intel_uncore_write_fw(uncore, reg,
> - intel_uncore_read_fw(uncore, reg) | or_val);
> + u32 val;
> +
> + val = intel_uncore_read(uncore, reg);
> + val &= ~clear;
> + val |= set;
> + intel_uncore_write(uncore, reg, val);
> +}
> +
> +static inline void intel_uncore_rmw_fw(struct intel_uncore *uncore,
> + i915_reg_t reg, u32 clear, u32 set)
> +{
> + u32 val;
> +
> + val = intel_uncore_read_fw(uncore, reg);
> + val &= ~clear;
> + val |= set;
> + intel_uncore_write_fw(uncore, reg, val);
> }
>
> #define raw_reg_read(base, reg) \
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915: Convert i915_reset.c over to using uncore mmio
2019-04-05 20:24 ` [PATCH v2] " Chris Wilson
2019-04-05 20:39 ` Paulo Zanoni
@ 2019-04-05 20:39 ` Lucas De Marchi
2019-04-05 20:43 ` Chris Wilson
1 sibling, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2019-04-05 20:39 UTC (permalink / raw)
To: Chris Wilson; +Cc: Intel Graphics, Paulo Zanoni
On Fri, Apr 5, 2019 at 1:24 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Currently i915_reset.c mixes calls to intel_uncore, pci and our old
> style I915_READ mmio interfaces. Cast aside the old implicit macros,
> and harmonise on using uncore throughout.
>
> add/remove: 1/1 grow/shrink: 0/4 up/down: 65/-207 (-142)
> Function old new delta
> rmw_register - 65 +65
> gen8_reset_engines 945 942 -3
> g4x_do_reset 407 376 -31
> intel_gpu_reset 545 509 -36
> clear_register 63 - -63
> i915_clear_error_registers 461 387 -74
>
> A little bit of pointer dancing elimination works wonders.
>
> v2: Roll up the helpers into intel_uncore for general use
>
> With the helpers gcc was a little more eager to inline:
> add/remove: 0/1 grow/shrink: 1/3 up/down: 99/-133 (-34)
> Function old new delta
> i915_clear_error_registers 461 560 +99
> gen8_reset_engines 945 942 -3
> g4x_do_reset 407 376 -31
> intel_gpu_reset 545 509 -36
> clear_register 63 - -63
> Total: Before=1544400, After=1544366, chg -0.00%
>
> Win some, lose some, gcc is gcc.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reset.c | 122 ++++++++++++++++------------
> drivers/gpu/drm/i915/intel_uncore.h | 23 +++++-
> 2 files changed, 89 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> index d44dc8422e8c..ac168de6a66e 100644
> --- a/drivers/gpu/drm/i915/i915_reset.c
> +++ b/drivers/gpu/drm/i915/i915_reset.c
> @@ -18,6 +18,26 @@
> /* XXX How to handle concurrent GGTT updates using tiling registers? */
> #define RESET_UNDER_STOP_MACHINE 0
>
> +static void rmw_set(struct intel_uncore *uncore, i915_reg_t reg, u32 set)
> +{
> + intel_uncore_rmw(uncore, reg, 0, set);
> +}
> +
> +static void rmw_clear(struct intel_uncore *uncore, i915_reg_t reg, u32 clr)
> +{
> + intel_uncore_rmw(uncore, reg, clr, 0);
> +}
> +
> +static void rwm_set_fw(struct intel_uncore *uncore, i915_reg_t reg, u32 set)
rwm?!?
Lucas De Marchi
> +{
> + intel_uncore_rmw_fw(uncore, reg, 0, set);
> +}
> +
> +static void rmw_clear_fw(struct intel_uncore *uncore, i915_reg_t reg, u32 clr)
> +{
> + intel_uncore_rmw_fw(uncore, reg, clr, 0);
> +}
> +
> static void engine_skip_context(struct i915_request *rq)
> {
> struct intel_engine_cs *engine = rq->engine;
> @@ -119,7 +139,7 @@ void i915_reset_request(struct i915_request *rq, bool guilty)
>
> static void gen3_stop_engine(struct intel_engine_cs *engine)
> {
> - struct drm_i915_private *dev_priv = engine->i915;
> + struct intel_uncore *uncore = engine->uncore;
> const u32 base = engine->mmio_base;
>
> GEM_TRACE("%s\n", engine->name);
> @@ -127,20 +147,23 @@ static void gen3_stop_engine(struct intel_engine_cs *engine)
> if (intel_engine_stop_cs(engine))
> GEM_TRACE("%s: timed out on STOP_RING\n", engine->name);
>
> - I915_WRITE_FW(RING_HEAD(base), I915_READ_FW(RING_TAIL(base)));
> - POSTING_READ_FW(RING_HEAD(base)); /* paranoia */
> + intel_uncore_write_fw(uncore,
> + RING_HEAD(base),
> + intel_uncore_read_fw(uncore, RING_TAIL(base)));
> + intel_uncore_posting_read_fw(uncore, RING_HEAD(base)); /* paranoia */
>
> - I915_WRITE_FW(RING_HEAD(base), 0);
> - I915_WRITE_FW(RING_TAIL(base), 0);
> - POSTING_READ_FW(RING_TAIL(base));
> + intel_uncore_write_fw(uncore, RING_HEAD(base), 0);
> + intel_uncore_write_fw(uncore, RING_TAIL(base), 0);
> + intel_uncore_posting_read_fw(uncore, RING_TAIL(base));
>
> /* The ring must be empty before it is disabled */
> - I915_WRITE_FW(RING_CTL(base), 0);
> + intel_uncore_write_fw(uncore, RING_CTL(base), 0);
>
> /* Check acts as a post */
> - if (I915_READ_FW(RING_HEAD(base)))
> + if (intel_uncore_read_fw(uncore, RING_HEAD(base)))
> GEM_TRACE("%s: ring head [%x] not parked\n",
> - engine->name, I915_READ_FW(RING_HEAD(base)));
> + engine->name,
> + intel_uncore_read_fw(uncore, RING_HEAD(base)));
> }
>
> static void i915_stop_engines(struct drm_i915_private *i915,
> @@ -203,17 +226,17 @@ static int g33_do_reset(struct drm_i915_private *i915,
> return wait_for_atomic(g4x_reset_complete(pdev), 50);
> }
>
> -static int g4x_do_reset(struct drm_i915_private *dev_priv,
> +static int g4x_do_reset(struct drm_i915_private *i915,
> intel_engine_mask_t engine_mask,
> unsigned int retry)
> {
> - struct pci_dev *pdev = dev_priv->drm.pdev;
> + struct pci_dev *pdev = i915->drm.pdev;
> + struct intel_uncore *uncore = &i915->uncore;
> int ret;
>
> /* WaVcpClkGateDisableForMediaReset:ctg,elk */
> - I915_WRITE_FW(VDECCLK_GATE_D,
> - I915_READ(VDECCLK_GATE_D) | VCP_UNIT_CLOCK_GATE_DISABLE);
> - POSTING_READ_FW(VDECCLK_GATE_D);
> + rwm_set_fw(uncore, VDECCLK_GATE_D, VCP_UNIT_CLOCK_GATE_DISABLE);
> + intel_uncore_posting_read_fw(uncore, VDECCLK_GATE_D);
>
> pci_write_config_byte(pdev, I915_GDRST,
> GRDOM_MEDIA | GRDOM_RESET_ENABLE);
> @@ -234,18 +257,17 @@ static int g4x_do_reset(struct drm_i915_private *dev_priv,
> out:
> pci_write_config_byte(pdev, I915_GDRST, 0);
>
> - I915_WRITE_FW(VDECCLK_GATE_D,
> - I915_READ(VDECCLK_GATE_D) & ~VCP_UNIT_CLOCK_GATE_DISABLE);
> - POSTING_READ_FW(VDECCLK_GATE_D);
> + rmw_clear_fw(uncore, VDECCLK_GATE_D, VCP_UNIT_CLOCK_GATE_DISABLE);
> + intel_uncore_posting_read_fw(uncore, VDECCLK_GATE_D);
>
> return ret;
> }
>
> -static int ironlake_do_reset(struct drm_i915_private *dev_priv,
> +static int ironlake_do_reset(struct drm_i915_private *i915,
> intel_engine_mask_t engine_mask,
> unsigned int retry)
> {
> - struct intel_uncore *uncore = &dev_priv->uncore;
> + struct intel_uncore *uncore = &i915->uncore;
> int ret;
>
> intel_uncore_write_fw(uncore, ILK_GDSR,
> @@ -277,10 +299,10 @@ static int ironlake_do_reset(struct drm_i915_private *dev_priv,
> }
>
> /* Reset the hardware domains (GENX_GRDOM_*) specified by mask */
> -static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
> +static int gen6_hw_domain_reset(struct drm_i915_private *i915,
> u32 hw_domain_mask)
> {
> - struct intel_uncore *uncore = &dev_priv->uncore;
> + struct intel_uncore *uncore = &i915->uncore;
> int err;
>
> /*
> @@ -381,7 +403,7 @@ static u32 gen11_lock_sfc(struct intel_engine_cs *engine)
> * ends up being locked to the engine we want to reset, we have to reset
> * it as well (we will unlock it once the reset sequence is completed).
> */
> - intel_uncore_rmw_or_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
> + rwm_set_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
>
> if (__intel_wait_for_register_fw(uncore,
> sfc_forced_lock_ack,
> @@ -404,7 +426,6 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine)
> u8 vdbox_sfc_access = RUNTIME_INFO(engine->i915)->vdbox_sfc_access;
> i915_reg_t sfc_forced_lock;
> u32 sfc_forced_lock_bit;
> - u32 val;
>
> switch (engine->class) {
> case VIDEO_DECODE_CLASS:
> @@ -424,9 +445,7 @@ static void gen11_unlock_sfc(struct intel_engine_cs *engine)
> return;
> }
>
> - val = intel_uncore_read_fw(uncore, sfc_forced_lock);
> - val &= ~sfc_forced_lock_bit;
> - intel_uncore_write_fw(uncore, sfc_forced_lock, val);
> + rmw_clear_fw(uncore, sfc_forced_lock, sfc_forced_lock_bit);
> }
>
> static int gen11_reset_engines(struct drm_i915_private *i915,
> @@ -491,10 +510,9 @@ static int gen8_engine_reset_prepare(struct intel_engine_cs *engine)
>
> static void gen8_engine_reset_cancel(struct intel_engine_cs *engine)
> {
> - struct drm_i915_private *dev_priv = engine->i915;
> -
> - I915_WRITE_FW(RING_RESET_CTL(engine->mmio_base),
> - _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
> + intel_uncore_write_fw(engine->uncore,
> + RING_RESET_CTL(engine->mmio_base),
> + _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
> }
>
> static int gen8_reset_engines(struct drm_i915_private *i915,
> @@ -1178,49 +1196,49 @@ static void i915_reset_device(struct drm_i915_private *i915,
> kobject_uevent_env(kobj, KOBJ_CHANGE, reset_done_event);
> }
>
> -static void clear_register(struct drm_i915_private *dev_priv, i915_reg_t reg)
> +static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
> {
> - I915_WRITE(reg, I915_READ(reg));
> + intel_uncore_rmw(uncore, reg, 0, 0);
> }
>
> -void i915_clear_error_registers(struct drm_i915_private *dev_priv)
> +void i915_clear_error_registers(struct drm_i915_private *i915)
> {
> + struct intel_uncore *uncore = &i915->uncore;
> u32 eir;
>
> - if (!IS_GEN(dev_priv, 2))
> - clear_register(dev_priv, PGTBL_ER);
> + if (!IS_GEN(i915, 2))
> + clear_register(uncore, PGTBL_ER);
>
> - if (INTEL_GEN(dev_priv) < 4)
> - clear_register(dev_priv, IPEIR(RENDER_RING_BASE));
> + if (INTEL_GEN(i915) < 4)
> + clear_register(uncore, IPEIR(RENDER_RING_BASE));
> else
> - clear_register(dev_priv, IPEIR_I965);
> + clear_register(uncore, IPEIR_I965);
>
> - clear_register(dev_priv, EIR);
> - eir = I915_READ(EIR);
> + clear_register(uncore, EIR);
> + eir = intel_uncore_read(uncore, EIR);
> if (eir) {
> /*
> * some errors might have become stuck,
> * mask them.
> */
> DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masking\n", eir);
> - I915_WRITE(EMR, I915_READ(EMR) | eir);
> - I915_WRITE(IIR, I915_MASTER_ERROR_INTERRUPT);
> + rmw_set(uncore, EMR, eir);
> + intel_uncore_write(uncore, IIR, I915_MASTER_ERROR_INTERRUPT);
> }
>
> - if (INTEL_GEN(dev_priv) >= 8) {
> - I915_WRITE(GEN8_RING_FAULT_REG,
> - I915_READ(GEN8_RING_FAULT_REG) & ~RING_FAULT_VALID);
> - POSTING_READ(GEN8_RING_FAULT_REG);
> - } else if (INTEL_GEN(dev_priv) >= 6) {
> + if (INTEL_GEN(i915) >= 8) {
> + rmw_clear(uncore, GEN8_RING_FAULT_REG, RING_FAULT_VALID);
> + intel_uncore_posting_read(uncore, GEN8_RING_FAULT_REG);
> + } else if (INTEL_GEN(i915) >= 6) {
> struct intel_engine_cs *engine;
> enum intel_engine_id id;
>
> - for_each_engine(engine, dev_priv, id) {
> - I915_WRITE(RING_FAULT_REG(engine),
> - I915_READ(RING_FAULT_REG(engine)) &
> - ~RING_FAULT_VALID);
> + for_each_engine(engine, i915, id) {
> + rmw_clear(uncore,
> + RING_FAULT_REG(engine), RING_FAULT_VALID);
> + intel_uncore_posting_read(uncore,
> + RING_FAULT_REG(engine));
> }
> - POSTING_READ(RING_FAULT_REG(dev_priv->engine[RCS0]));
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index a64d3dc0db4d..d6af3de70121 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -369,11 +369,26 @@ intel_uncore_read64_2x32(struct intel_uncore *uncore,
> #define intel_uncore_write64_fw(...) __raw_uncore_write64(__VA_ARGS__)
> #define intel_uncore_posting_read_fw(...) ((void)intel_uncore_read_fw(__VA_ARGS__))
>
> -static inline void intel_uncore_rmw_or_fw(struct intel_uncore *uncore,
> - i915_reg_t reg, u32 or_val)
> +static inline void intel_uncore_rmw(struct intel_uncore *uncore,
> + i915_reg_t reg, u32 clear, u32 set)
> {
> - intel_uncore_write_fw(uncore, reg,
> - intel_uncore_read_fw(uncore, reg) | or_val);
> + u32 val;
> +
> + val = intel_uncore_read(uncore, reg);
> + val &= ~clear;
> + val |= set;
> + intel_uncore_write(uncore, reg, val);
> +}
> +
> +static inline void intel_uncore_rmw_fw(struct intel_uncore *uncore,
> + i915_reg_t reg, u32 clear, u32 set)
> +{
> + u32 val;
> +
> + val = intel_uncore_read_fw(uncore, reg);
> + val &= ~clear;
> + val |= set;
> + intel_uncore_write_fw(uncore, reg, val);
> }
>
> #define raw_reg_read(base, reg) \
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915: Convert i915_reset.c over to using uncore mmio
2019-04-05 20:39 ` Lucas De Marchi
@ 2019-04-05 20:43 ` Chris Wilson
2019-04-06 5:57 ` Lucas De Marchi
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2019-04-05 20:43 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: Intel Graphics, Paulo Zanoni
Quoting Lucas De Marchi (2019-04-05 21:39:46)
> On Fri, Apr 5, 2019 at 1:24 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Currently i915_reset.c mixes calls to intel_uncore, pci and our old
> > style I915_READ mmio interfaces. Cast aside the old implicit macros,
> > and harmonise on using uncore throughout.
> >
> > add/remove: 1/1 grow/shrink: 0/4 up/down: 65/-207 (-142)
> > Function old new delta
> > rmw_register - 65 +65
> > gen8_reset_engines 945 942 -3
> > g4x_do_reset 407 376 -31
> > intel_gpu_reset 545 509 -36
> > clear_register 63 - -63
> > i915_clear_error_registers 461 387 -74
> >
> > A little bit of pointer dancing elimination works wonders.
> >
> > v2: Roll up the helpers into intel_uncore for general use
> >
> > With the helpers gcc was a little more eager to inline:
> > add/remove: 0/1 grow/shrink: 1/3 up/down: 99/-133 (-34)
> > Function old new delta
> > i915_clear_error_registers 461 560 +99
> > gen8_reset_engines 945 942 -3
> > g4x_do_reset 407 376 -31
> > intel_gpu_reset 545 509 -36
> > clear_register 63 - -63
> > Total: Before=1544400, After=1544366, chg -0.00%
> >
> > Win some, lose some, gcc is gcc.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reset.c | 122 ++++++++++++++++------------
> > drivers/gpu/drm/i915/intel_uncore.h | 23 +++++-
> > 2 files changed, 89 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> > index d44dc8422e8c..ac168de6a66e 100644
> > --- a/drivers/gpu/drm/i915/i915_reset.c
> > +++ b/drivers/gpu/drm/i915/i915_reset.c
> > @@ -18,6 +18,26 @@
> > /* XXX How to handle concurrent GGTT updates using tiling registers? */
> > #define RESET_UNDER_STOP_MACHINE 0
> >
> > +static void rmw_set(struct intel_uncore *uncore, i915_reg_t reg, u32 set)
> > +{
> > + intel_uncore_rmw(uncore, reg, 0, set);
> > +}
> > +
> > +static void rmw_clear(struct intel_uncore *uncore, i915_reg_t reg, u32 clr)
> > +{
> > + intel_uncore_rmw(uncore, reg, clr, 0);
> > +}
> > +
> > +static void rwm_set_fw(struct intel_uncore *uncore, i915_reg_t reg, u32 set)
>
> rwm?!?
read, write, then modify. It's when you have that senior moment and
enter a room forgetting why.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Make use of 'engine->uncore' (rev2)
2019-04-05 18:57 [PATCH 1/2] drm/i915: Make use of 'engine->uncore' Chris Wilson
` (2 preceding siblings ...)
2019-04-05 20:15 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] " Patchwork
@ 2019-04-05 20:59 ` Patchwork
2019-04-06 18:49 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-04-05 20:59 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Make use of 'engine->uncore' (rev2)
URL : https://patchwork.freedesktop.org/series/59081/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5881 -> Patchwork_12708
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/59081/revisions/2/mbox/
Known issues
------------
Here are the changes found in Patchwork_12708 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_contexts:
- fi-bdw-gvtdvm: PASS -> DMESG-FAIL [fdo#110235 ]
* igt@kms_frontbuffer_tracking@basic:
- fi-byt-clapper: PASS -> FAIL [fdo#103167]
* igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362]
#### Possible fixes ####
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: FAIL [fdo#108511] -> PASS
* igt@i915_selftest@live_uncore:
- fi-ivb-3770: DMESG-FAIL [fdo#110210] -> PASS
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-byt-clapper: FAIL [fdo#103191] / [fdo#107362] -> PASS
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
[fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
[fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
Participating hosts (49 -> 43)
------------------------------
Missing (6): fi-ilk-m540 fi-bdw-5557u fi-byt-squawks fi-bsw-cyan fi-bdw-samus fi-snb-2600
Build changes
-------------
* Linux: CI_DRM_5881 -> Patchwork_12708
CI_DRM_5881: b070175c76da1440a747fd023ee6253e573055f8 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4931: 019f892e5d1a0a9643cb726c47ce2d99c14b444f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12708: 5a816ec8285576a09223b15bbac20d70300cec61 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
5a816ec82855 drm/i915: Convert i915_reset.c over to using uncore mmio
1214c154f5ec drm/i915: Make use of 'engine->uncore'
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12708/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915: Convert i915_reset.c over to using uncore mmio
2019-04-05 20:43 ` Chris Wilson
@ 2019-04-06 5:57 ` Lucas De Marchi
0 siblings, 0 replies; 12+ messages in thread
From: Lucas De Marchi @ 2019-04-06 5:57 UTC (permalink / raw)
To: Chris Wilson; +Cc: Intel Graphics, Paulo Zanoni
On Fri, Apr 5, 2019 at 1:43 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Lucas De Marchi (2019-04-05 21:39:46)
> > On Fri, Apr 5, 2019 at 1:24 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > >
> > > Currently i915_reset.c mixes calls to intel_uncore, pci and our old
> > > style I915_READ mmio interfaces. Cast aside the old implicit macros,
> > > and harmonise on using uncore throughout.
> > >
> > > add/remove: 1/1 grow/shrink: 0/4 up/down: 65/-207 (-142)
> > > Function old new delta
> > > rmw_register - 65 +65
> > > gen8_reset_engines 945 942 -3
> > > g4x_do_reset 407 376 -31
> > > intel_gpu_reset 545 509 -36
> > > clear_register 63 - -63
> > > i915_clear_error_registers 461 387 -74
> > >
> > > A little bit of pointer dancing elimination works wonders.
> > >
> > > v2: Roll up the helpers into intel_uncore for general use
> > >
> > > With the helpers gcc was a little more eager to inline:
> > > add/remove: 0/1 grow/shrink: 1/3 up/down: 99/-133 (-34)
> > > Function old new delta
> > > i915_clear_error_registers 461 560 +99
> > > gen8_reset_engines 945 942 -3
> > > g4x_do_reset 407 376 -31
> > > intel_gpu_reset 545 509 -36
> > > clear_register 63 - -63
> > > Total: Before=1544400, After=1544366, chg -0.00%
> > >
> > > Win some, lose some, gcc is gcc.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_reset.c | 122 ++++++++++++++++------------
> > > drivers/gpu/drm/i915/intel_uncore.h | 23 +++++-
> > > 2 files changed, 89 insertions(+), 56 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> > > index d44dc8422e8c..ac168de6a66e 100644
> > > --- a/drivers/gpu/drm/i915/i915_reset.c
> > > +++ b/drivers/gpu/drm/i915/i915_reset.c
> > > @@ -18,6 +18,26 @@
> > > /* XXX How to handle concurrent GGTT updates using tiling registers? */
> > > #define RESET_UNDER_STOP_MACHINE 0
> > >
> > > +static void rmw_set(struct intel_uncore *uncore, i915_reg_t reg, u32 set)
> > > +{
> > > + intel_uncore_rmw(uncore, reg, 0, set);
> > > +}
> > > +
> > > +static void rmw_clear(struct intel_uncore *uncore, i915_reg_t reg, u32 clr)
> > > +{
> > > + intel_uncore_rmw(uncore, reg, clr, 0);
> > > +}
> > > +
> > > +static void rwm_set_fw(struct intel_uncore *uncore, i915_reg_t reg, u32 set)
> >
> > rwm?!?
>
> read, write, then modify. It's when you have that senior moment and
> enter a room forgetting why.
:)
I think the rwm_* functions are a nice addition. I know people say
"rmw is considered harmful", but is a real necessity in some places.
And the alternative is to roll our own in each part of the code. I
remember converting
icl_mg_pll_write() to use a rmw function, but then that got killed
under this argument.
Any chance to promote this to i915_drv.h (the place those functions
are today, but could be another smaller one, too) so we can use them
in more places?
thanks
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Make use of 'engine->uncore' (rev2)
2019-04-05 18:57 [PATCH 1/2] drm/i915: Make use of 'engine->uncore' Chris Wilson
` (3 preceding siblings ...)
2019-04-05 20:59 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Make use of 'engine->uncore' (rev2) Patchwork
@ 2019-04-06 18:49 ` Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-04-06 18:49 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Make use of 'engine->uncore' (rev2)
URL : https://patchwork.freedesktop.org/series/59081/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5881_full -> Patchwork_12708_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_12708_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_12708_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_12708_full:
### IGT changes ###
#### Possible regressions ####
* igt@i915_pm_rpm@cursor-dpms:
- shard-glk: PASS -> DMESG-WARN +1
#### Warnings ####
* igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-pri-shrfb-draw-pwrite:
- shard-iclb: SKIP [fdo#109280] -> INCOMPLETE
Known issues
------------
Here are the changes found in Patchwork_12708_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_schedule@preempt-other-chain-bsd2:
- shard-iclb: NOTRUN -> SKIP [fdo#109276] +3
* igt@gem_exec_store@cachelines-bsd1:
- shard-snb: NOTRUN -> SKIP [fdo#109271] +80
* igt@gem_tiled_swapping@non-threaded:
- shard-iclb: PASS -> FAIL [fdo#108686]
* igt@i915_pm_rpm@pm-tiling:
- shard-skl: NOTRUN -> INCOMPLETE [fdo#107807]
* igt@kms_atomic_transition@4x-modeset-transitions-nonblocking-fencing:
- shard-skl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +14
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
- shard-iclb: NOTRUN -> DMESG-WARN [fdo#110222]
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
- shard-apl: NOTRUN -> DMESG-WARN [fdo#110222]
- shard-skl: NOTRUN -> DMESG-WARN [fdo#110222] +1
* igt@kms_busy@extended-modeset-hang-oldfb-render-f:
- shard-iclb: NOTRUN -> SKIP [fdo#109278] +1
* igt@kms_busy@extended-pageflip-hang-newfb-render-c:
- shard-snb: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +6
* igt@kms_busy@extended-pageflip-hang-oldfb-render-f:
- shard-kbl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1
* igt@kms_ccs@pipe-c-missing-ccs-buffer:
- shard-apl: NOTRUN -> SKIP [fdo#109271] +36
* igt@kms_chamelium@dp-crc-single:
- shard-iclb: NOTRUN -> SKIP [fdo#109284] +1
* igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
- shard-iclb: NOTRUN -> SKIP [fdo#109274]
* igt@kms_frontbuffer_tracking@basic:
- shard-iclb: PASS -> FAIL [fdo#103167] +2
* igt@kms_frontbuffer_tracking@fbcpsr-2p-shrfb-fliptrack:
- shard-iclb: NOTRUN -> SKIP [fdo#109280] +3
* igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-gtt:
- shard-kbl: NOTRUN -> SKIP [fdo#109271] +22
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-gtt:
- shard-iclb: PASS -> FAIL [fdo#109247] +16
* igt@kms_lease@atomic_implicit_crtc:
- shard-skl: NOTRUN -> FAIL [fdo#110279]
- shard-apl: NOTRUN -> FAIL [fdo#110279]
* igt@kms_lease@setcrtc_implicit_plane:
- shard-skl: NOTRUN -> FAIL [fdo#110281]
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
- shard-skl: PASS -> FAIL [fdo#103191] / [fdo#107362]
* igt@kms_pipe_crc_basic@read-crc-pipe-e:
- shard-apl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2
* igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
- shard-kbl: NOTRUN -> FAIL [fdo#108145] / [fdo#108590]
- shard-skl: NOTRUN -> FAIL [fdo#107815] / [fdo#108145] +2
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
- shard-skl: NOTRUN -> FAIL [fdo#108145]
* igt@kms_psr@cursor_mmap_gtt:
- shard-iclb: PASS -> FAIL [fdo#107383] / [fdo#110215] +1
* igt@kms_psr@psr2_primary_render:
- shard-iclb: PASS -> SKIP [fdo#109441]
* igt@kms_psr@psr2_sprite_plane_onoff:
- shard-iclb: NOTRUN -> SKIP [fdo#109441]
* igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
- shard-kbl: PASS -> DMESG-FAIL [fdo#105763]
* igt@kms_rotation_crc@primary-yf-tiled-reflect-x-0:
- shard-iclb: PASS -> INCOMPLETE [fdo#110026] / [fdo#110040 ]
* igt@kms_setmode@basic:
- shard-skl: NOTRUN -> FAIL [fdo#99912]
* igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
- shard-apl: PASS -> FAIL [fdo#104894] +1
* igt@perf_pmu@semaphore-wait-vcs1:
- shard-skl: NOTRUN -> SKIP [fdo#109271] +148
#### Possible fixes ####
* igt@gem_ppgtt@blt-vs-render-ctx0:
- shard-iclb: INCOMPLETE [fdo#109801] -> PASS
* igt@i915_pm_rpm@reg-read-ioctl:
- shard-skl: INCOMPLETE [fdo#107807] -> PASS
* igt@i915_pm_rpm@universal-planes-dpms:
- shard-iclb: INCOMPLETE [fdo#107713] / [fdo#108840] -> PASS
* igt@i915_selftest@live_workarounds:
- shard-iclb: DMESG-FAIL [fdo#108954] -> PASS
* igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
- shard-glk: FAIL [fdo#106509] / [fdo#107409] -> PASS
* igt@kms_cursor_legacy@cursor-vs-flip-atomic:
- shard-iclb: FAIL [fdo#103355] -> PASS
* igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-xtiled:
- shard-skl: FAIL [fdo#107791] -> PASS
* igt@kms_fbcon_fbt@fbc:
- shard-iclb: DMESG-WARN [fdo#109593] -> PASS
* igt@kms_flip@flip-vs-expired-vblank:
- shard-skl: FAIL [fdo#105363] -> PASS
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-iclb: FAIL [fdo#103375] -> PASS
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
- shard-iclb: FAIL [fdo#103167] -> PASS +3
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
- shard-iclb: FAIL [fdo#109247] -> PASS +21
* igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-pwrite:
- shard-iclb: FAIL [fdo#105682] / [fdo#109247] -> PASS
* igt@kms_frontbuffer_tracking@psr-rgb565-draw-blt:
- shard-skl: FAIL [fdo#103167] -> PASS
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: FAIL [fdo#108145] -> PASS
* igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
- shard-skl: FAIL [fdo#107815] -> PASS
* igt@kms_plane_scaling@pipe-a-scaler-with-pixel-format:
- shard-glk: SKIP [fdo#109271] / [fdo#109278] -> PASS +1
* igt@kms_psr@primary_mmap_cpu:
- shard-iclb: FAIL [fdo#107383] / [fdo#110215] -> PASS +1
* igt@kms_psr@psr2_primary_mmap_cpu:
- shard-iclb: SKIP [fdo#109441] -> PASS +5
* igt@kms_setmode@basic:
- shard-kbl: FAIL [fdo#99912] -> PASS
* igt@kms_vblank@pipe-a-ts-continuation-suspend:
- shard-iclb: FAIL [fdo#104894] -> PASS
* igt@kms_vblank@pipe-b-ts-continuation-modeset-hang:
- shard-apl: FAIL [fdo#104894] -> PASS
* igt@perf@oa-exponents:
- shard-glk: FAIL [fdo#105483] -> PASS
#### Warnings ####
* igt@runner@aborted:
- shard-glk: FAIL [fdo#109373] / [k.org#202321] -> ( 3 FAIL ) [fdo#109373] / [k.org#202321]
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
[fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
[fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105483]: https://bugs.freedesktop.org/show_bug.cgi?id=105483
[fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
[fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
[fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
[fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107791]: https://bugs.freedesktop.org/show_bug.cgi?id=107791
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
[fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
[fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
[fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
[fdo#110026]: https://bugs.freedesktop.org/show_bug.cgi?id=110026
[fdo#110040 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110040
[fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
[fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
[fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
[fdo#110281]: https://bugs.freedesktop.org/show_bug.cgi?id=110281
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
[k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321
Participating hosts (10 -> 9)
------------------------------
Missing (1): shard-hsw
Build changes
-------------
* Linux: CI_DRM_5881 -> Patchwork_12708
CI_DRM_5881: b070175c76da1440a747fd023ee6253e573055f8 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4931: 019f892e5d1a0a9643cb726c47ce2d99c14b444f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12708: 5a816ec8285576a09223b15bbac20d70300cec61 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12708/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-04-06 18:49 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-05 18:57 [PATCH 1/2] drm/i915: Make use of 'engine->uncore' Chris Wilson
2019-04-05 18:57 ` [PATCH 2/2] drm/i915: Convert i915_reset.c over to using uncore mmio Chris Wilson
2019-04-05 20:06 ` Paulo Zanoni
2019-04-05 20:24 ` [PATCH v2] " Chris Wilson
2019-04-05 20:39 ` Paulo Zanoni
2019-04-05 20:39 ` Lucas De Marchi
2019-04-05 20:43 ` Chris Wilson
2019-04-06 5:57 ` Lucas De Marchi
2019-04-05 18:59 ` [PATCH 1/2] drm/i915: Make use of 'engine->uncore' Chris Wilson
2019-04-05 20:15 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] " Patchwork
2019-04-05 20:59 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Make use of 'engine->uncore' (rev2) Patchwork
2019-04-06 18:49 ` ✗ Fi.CI.IGT: failure " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.