public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things
@ 2015-10-22 12:34 ville.syrjala
  2015-10-22 12:34 ` [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions ville.syrjala
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: ville.syrjala @ 2015-10-22 12:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

These patches fell off from my type safe register access series. I
figured I'd post them separately since they're a fairly separate
piece of work.

Ville Syrjälä (5):
  drm/i915: Turn __raw_i915_read8() & co. in to inline functions
  drm/i915: Read FORCEWAKE registers with I915_READ_FW()
  drm/i915: Minor style nits in intel_uncore.c
  drm/i915: Respin vlv/chv reagister access to look more like SKL
  drm/i915: Add NEEDS_FORCEWAKE() checks for vlv/chv

 drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
 drivers/gpu/drm/i915/i915_drv.h       | 30 ++++++++++++++-
 drivers/gpu/drm/i915/i915_gpu_error.c |  6 +--
 drivers/gpu/drm/i915/i915_irq.c       | 10 ++---
 drivers/gpu/drm/i915/i915_vgpu.c      |  6 +--
 drivers/gpu/drm/i915/intel_uncore.c   | 69 +++++++++++++++++------------------
 6 files changed, 73 insertions(+), 50 deletions(-)

-- 
2.4.9

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

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

* [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions
  2015-10-22 12:34 [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things ville.syrjala
@ 2015-10-22 12:34 ` ville.syrjala
  2015-10-22 13:32   ` Daniel Vetter
  2015-10-22 12:34 ` [PATCH 2/5] drm/i915: Read FORCEWAKE registers with I915_READ_FW() ville.syrjala
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: ville.syrjala @ 2015-10-22 12:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

There's no need for __raw_i915_read8() & co. bot be macros, so make them
inline funcitons. To avoid typo mistakes generate the inline functions
using preprocessor templates.

We have a few users of the raw register acces functions outside
intel_uncore.c, so let's also move the functions into intel_drv.h.

While doing that switch I915_READ_FW() & co. to use the
__raw_i915_read() functions, and use the _FW macros everywhere
outside intel_uncore.c where we want to read registers without
grabbing forcewake and whatnot. The only exception is
i915_check_vgpu() which itself gets called from intel_uncore.c,
so using the __raw_i915_read stuff there seems appropriate.

v2: Squash in the intel_uncore.c->i915_drv.h move
    Convert I915_READ_FW() to use __raw_i915_read(), and use
    I915_READ_FW() outside of intel_uncore.c (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
 drivers/gpu/drm/i915/i915_drv.h     | 30 ++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_irq.c     | 10 ++++------
 drivers/gpu/drm/i915/i915_vgpu.c    |  6 +++---
 drivers/gpu/drm/i915/intel_uncore.c | 14 +-------------
 5 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index eca94d0..89ba549 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1523,7 +1523,7 @@ static int gen6_drpc_info(struct seq_file *m)
 		seq_printf(m, "RC information accurate: %s\n", yesno(count < 51));
 	}
 
-	gt_core_status = readl(dev_priv->regs + GEN6_GT_CORE_STATUS);
+	gt_core_status = I915_READ_FW(GEN6_GT_CORE_STATUS);
 	trace_i915_reg_rw(false, GEN6_GT_CORE_STATUS, gt_core_status, 4, true);
 
 	rpmodectl1 = I915_READ(GEN6_RP_CONTROL);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9a15ebe..01fdc70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3452,6 +3452,32 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
 #define POSTING_READ(reg)	(void)I915_READ_NOTRACE(reg)
 #define POSTING_READ16(reg)	(void)I915_READ16_NOTRACE(reg)
 
+#define __raw_read(x, s) \
+static inline uint##x##_t __raw_i915_read##x(struct drm_i915_private *dev_priv, \
+					     uint32_t reg) \
+{ \
+	return read##s(dev_priv->regs + reg); \
+}
+
+#define __raw_write(x, s) \
+static inline void __raw_i915_write##x(struct drm_i915_private *dev_priv, \
+				       uint32_t reg, uint##x##_t val) \
+{ \
+	write##s(val, dev_priv->regs + reg); \
+}
+__raw_read(8, b)
+__raw_read(16, w)
+__raw_read(32, l)
+__raw_read(64, q)
+
+__raw_write(8, b)
+__raw_write(16, w)
+__raw_write(32, l)
+__raw_write(64, q)
+
+#undef __raw_read
+#undef __raw_write
+
 /* These are untraced mmio-accessors that are only valid to be used inside
  * criticial sections inside IRQ handlers where forcewake is explicitly
  * controlled.
@@ -3459,8 +3485,8 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
  * Note: Should only be used between intel_uncore_forcewake_irqlock() and
  * intel_uncore_forcewake_irqunlock().
  */
-#define I915_READ_FW(reg__) readl(dev_priv->regs + (reg__))
-#define I915_WRITE_FW(reg__, val__) writel(val__, dev_priv->regs + (reg__))
+#define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__))
+#define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), (val__))
 #define POSTING_READ_FW(reg__) (void)I915_READ_FW(reg__)
 
 /* "Broadcast RGB" property */
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9caf22c..a0ed58d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -717,9 +717,7 @@ static u32 g4x_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 	return I915_READ(PIPE_FRMCOUNT_G4X(pipe));
 }
 
-/* raw reads, only for fast reads of display block, no need for forcewake etc. */
-#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + (reg__))
-
+/* I915_READ_FW, only for fast reads of display block, no need for forcewake etc. */
 static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
@@ -733,9 +731,9 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
 		vtotal /= 2;
 
 	if (IS_GEN2(dev))
-		position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN2;
+		position = I915_READ_FW(PIPEDSL(pipe)) & DSL_LINEMASK_GEN2;
 	else
-		position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
+		position = I915_READ_FW(PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
 
 	/*
 	 * On HSW, the DSL reg (0x70000) appears to return 0 if we
@@ -827,7 +825,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
 		 * We can split this into vertical and horizontal
 		 * scanout position.
 		 */
-		position = (__raw_i915_read32(dev_priv, PIPEFRAMEPIXEL(pipe)) & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT;
+		position = (I915_READ_FW(PIPEFRAMEPIXEL(pipe)) & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT;
 
 		/* convert to pixel counts */
 		vbl_start *= htotal;
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 5eee75b..dea7429 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -69,13 +69,13 @@ void i915_check_vgpu(struct drm_device *dev)
 	if (!IS_HASWELL(dev))
 		return;
 
-	magic = readq(dev_priv->regs + vgtif_reg(magic));
+	magic = __raw_i915_read64(dev_priv, vgtif_reg(magic));
 	if (magic != VGT_MAGIC)
 		return;
 
 	version = INTEL_VGT_IF_VERSION_ENCODE(
-		readw(dev_priv->regs + vgtif_reg(version_major)),
-		readw(dev_priv->regs + vgtif_reg(version_minor)));
+		__raw_i915_read16(dev_priv, vgtif_reg(version_major)),
+		__raw_i915_read16(dev_priv, vgtif_reg(version_minor)));
 	if (version != INTEL_VGT_IF_VERSION) {
 		DRM_INFO("VGT interface version mismatch!\n");
 		return;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 1663ea5..8dfeac9 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -29,19 +29,7 @@
 
 #define FORCEWAKE_ACK_TIMEOUT_MS 50
 
-#define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + (reg__))
-#define __raw_i915_write8(dev_priv__, reg__, val__) writeb(val__, (dev_priv__)->regs + (reg__))
-
-#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)->regs + (reg__))
-#define __raw_i915_write16(dev_priv__, reg__, val__) writew(val__, (dev_priv__)->regs + (reg__))
-
-#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + (reg__))
-#define __raw_i915_write32(dev_priv__, reg__, val__) writel(val__, (dev_priv__)->regs + (reg__))
-
-#define __raw_i915_read64(dev_priv__, reg__) readq((dev_priv__)->regs + (reg__))
-#define __raw_i915_write64(dev_priv__, reg__, val__) writeq(val__, (dev_priv__)->regs + (reg__))
-
-#define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32(dev_priv__, reg__)
+#define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__))
 
 static const char * const forcewake_domain_names[] = {
 	"render",
-- 
2.4.9

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

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

* [PATCH 2/5] drm/i915: Read FORCEWAKE registers with I915_READ_FW()
  2015-10-22 12:34 [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things ville.syrjala
  2015-10-22 12:34 ` [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions ville.syrjala
@ 2015-10-22 12:34 ` ville.syrjala
  2015-10-22 13:37   ` Daniel Vetter
  2015-10-22 12:34 ` [PATCH 3/5] drm/i915: Minor style nits in intel_uncore.c ville.syrjala
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: ville.syrjala @ 2015-10-22 12:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Change FORCEWAKE & co. reads for the error state to use I915_READ_FW().
Reading a FORCEWAKE register using a function that can frob forcewake
just seems wrong.

There is a check to skip grabbing the forcewake for accessing FORCEWAKE
in intel_uncore.c, but there's no such check for FORCEWAKE_MT. So no
idea what is currently happening with FORCEWAKE_MT reads. FORCEWAKE_VLV
is fortunately outside the forcewake range anyway, so no actual issue
with that one.

So let's just make the rule that you can access FORCEWAKE registers with
the normal I915_READ() stuff, and we can drop the extra FORCEWAKE check
from NEEDS_FORCEWAKE(). While at it use NEEDS_FORCEWAKE() on BDW, where
it was skipped for whatever bikeshed reason that I've already forgotten.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 6 +++---
 drivers/gpu/drm/i915/intel_uncore.c   | 5 ++---
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 2f04e4f..de86f26 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1181,7 +1181,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 	if (IS_VALLEYVIEW(dev)) {
 		error->gtier[0] = I915_READ(GTIER);
 		error->ier = I915_READ(VLV_IER);
-		error->forcewake = I915_READ(FORCEWAKE_VLV);
+		error->forcewake = I915_READ_FW(FORCEWAKE_VLV);
 	}
 
 	if (IS_GEN7(dev))
@@ -1193,14 +1193,14 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 	}
 
 	if (IS_GEN6(dev)) {
-		error->forcewake = I915_READ(FORCEWAKE);
+		error->forcewake = I915_READ_FW(FORCEWAKE);
 		error->gab_ctl = I915_READ(GAB_CTL);
 		error->gfx_mode = I915_READ(GFX_MODE);
 	}
 
 	/* 2: Registers which belong to multiple generations */
 	if (INTEL_INFO(dev)->gen >= 7)
-		error->forcewake = I915_READ(FORCEWAKE_MT);
+		error->forcewake = I915_READ_FW(FORCEWAKE_MT);
 
 	if (INTEL_INFO(dev)->gen >= 6) {
 		error->derrmr = I915_READ(DERRMR);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 8dfeac9..dca0979 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -513,8 +513,7 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
 }
 
 /* We give fast paths for the really cool registers */
-#define NEEDS_FORCE_WAKE(reg) \
-	 ((reg) < 0x40000 && (reg) != FORCEWAKE)
+#define NEEDS_FORCE_WAKE(reg) ((reg) < 0x40000)
 
 #define REG_RANGE(reg, start, end) ((reg) >= (start) && (reg) < (end))
 
@@ -918,7 +917,7 @@ static void \
 gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
 	GEN6_WRITE_HEADER; \
 	hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \
-	if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) \
+	if (NEEDS_FORCE_WAKE(reg) && !is_gen8_shadowed(dev_priv, reg)) \
 		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
 	__raw_i915_write##x(dev_priv, reg, val); \
 	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
-- 
2.4.9

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

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

* [PATCH 3/5] drm/i915: Minor style nits in intel_uncore.c
  2015-10-22 12:34 [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things ville.syrjala
  2015-10-22 12:34 ` [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions ville.syrjala
  2015-10-22 12:34 ` [PATCH 2/5] drm/i915: Read FORCEWAKE registers with I915_READ_FW() ville.syrjala
@ 2015-10-22 12:34 ` ville.syrjala
  2015-10-22 13:38   ` Daniel Vetter
  2015-10-22 12:34 ` [PATCH 4/5] drm/i915: Respin vlv/chv reagister access to look more like SKL ville.syrjala
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: ville.syrjala @ 2015-10-22 12:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index dca0979..57af2c4 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -576,7 +576,7 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
 	REG_RANGE((reg), 0x9400, 0x9800)
 
 #define FORCEWAKE_GEN9_BLITTER_RANGE_OFFSET(reg) \
-	((reg) < 0x40000 &&\
+	((reg) < 0x40000 && \
 	 !FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg) && \
 	 !FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg) && \
 	 !FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg) && \
@@ -749,7 +749,7 @@ chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 }
 
 #define SKL_NEEDS_FORCE_WAKE(reg) \
-	 ((reg) < 0x40000 && !FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg))
+	((reg) < 0x40000 && !FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg))
 
 #define __gen9_read(x) \
 static u##x \
-- 
2.4.9

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

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

* [PATCH 4/5] drm/i915: Respin vlv/chv reagister access to look more like SKL
  2015-10-22 12:34 [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things ville.syrjala
                   ` (2 preceding siblings ...)
  2015-10-22 12:34 ` [PATCH 3/5] drm/i915: Minor style nits in intel_uncore.c ville.syrjala
@ 2015-10-22 12:34 ` ville.syrjala
  2015-10-22 13:42   ` Daniel Vetter
  2015-10-22 12:35 ` [PATCH 5/5] drm/i915: Add NEEDS_FORCEWAKE() checks for vlv/chv ville.syrjala
  2015-10-22 13:41 ` [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things Chris Wilson
  5 siblings, 1 reply; 15+ messages in thread
From: ville.syrjala @ 2015-10-22 12:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Change the fw domain handling in the vlv/chv register read/write
functions to look more like the SKL code, ie. have a single
__force_wake_get() get call instead of multiple ones.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 57af2c4..f38e88b 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -724,11 +724,14 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 #define __vlv_read(x) \
 static u##x \
 vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
+	enum forcewake_domains fw_engine = 0; \
 	GEN6_READ_HEADER(x); \
 	if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) \
-		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
+		fw_engine = FORCEWAKE_RENDER; \
 	else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) \
-		__force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
+		fw_engine = FORCEWAKE_MEDIA; \
+	if (fw_engine) \
+		__force_wake_get(dev_priv, fw_engine); \
 	val = __raw_i915_read##x(dev_priv, reg); \
 	GEN6_READ_FOOTER; \
 }
@@ -736,14 +739,16 @@ vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 #define __chv_read(x) \
 static u##x \
 chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
+	enum forcewake_domains fw_engine = 0; \
 	GEN6_READ_HEADER(x); \
 	if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
-		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
+		fw_engine = FORCEWAKE_RENDER; \
 	else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
-		__force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
+		fw_engine = FORCEWAKE_MEDIA; \
 	else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) \
-		__force_wake_get(dev_priv, \
-				 FORCEWAKE_RENDER | FORCEWAKE_MEDIA); \
+		fw_engine = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
+	if (fw_engine) \
+		__force_wake_get(dev_priv, fw_engine); \
 	val = __raw_i915_read##x(dev_priv, reg); \
 	GEN6_READ_FOOTER; \
 }
@@ -928,16 +933,18 @@ gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
 #define __chv_write(x) \
 static void \
 chv_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
-	bool shadowed = is_gen8_shadowed(dev_priv, reg); \
+	enum forcewake_domains fw_engine = 0; \
 	GEN6_WRITE_HEADER; \
-	if (!shadowed) { \
-		if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
-			__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
-		else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
-			__force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
-		else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) \
-			__force_wake_get(dev_priv, FORCEWAKE_RENDER | FORCEWAKE_MEDIA); \
-	} \
+	if (is_gen8_shadowed(dev_priv, reg)) \
+		fw_engine = 0; \
+	else if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
+		fw_engine = FORCEWAKE_RENDER; \
+	else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
+		fw_engine = FORCEWAKE_MEDIA; \
+	else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) \
+		fw_engine = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
+	if (fw_engine) \
+		__force_wake_get(dev_priv, fw_engine); \
 	__raw_i915_write##x(dev_priv, reg, val); \
 	GEN6_WRITE_FOOTER; \
 }
-- 
2.4.9

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

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

* [PATCH 5/5] drm/i915: Add NEEDS_FORCEWAKE() checks for vlv/chv
  2015-10-22 12:34 [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things ville.syrjala
                   ` (3 preceding siblings ...)
  2015-10-22 12:34 ` [PATCH 4/5] drm/i915: Respin vlv/chv reagister access to look more like SKL ville.syrjala
@ 2015-10-22 12:35 ` ville.syrjala
  2015-10-22 13:44   ` Daniel Vetter
  2015-10-22 13:41 ` [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things Chris Wilson
  5 siblings, 1 reply; 15+ messages in thread
From: ville.syrjala @ 2015-10-22 12:35 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Include an early NEEDS_FORCEWAKE() check for vlv and chv.
Hopefully that will avoid doing so many range checks in for many
register accesses (at least for all display registers).

Note that vlv already had the check in the write path since it shares
the gen6+ code for that.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f38e88b..f0f97b2 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -726,7 +726,9 @@ static u##x \
 vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 	enum forcewake_domains fw_engine = 0; \
 	GEN6_READ_HEADER(x); \
-	if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) \
+	if (!NEEDS_FORCE_WAKE(reg)) \
+		fw_engine = 0; \
+	else if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) \
 		fw_engine = FORCEWAKE_RENDER; \
 	else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) \
 		fw_engine = FORCEWAKE_MEDIA; \
@@ -741,7 +743,9 @@ static u##x \
 chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
 	enum forcewake_domains fw_engine = 0; \
 	GEN6_READ_HEADER(x); \
-	if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
+	if (!NEEDS_FORCE_WAKE(reg)) \
+		fw_engine = 0; \
+	else if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
 		fw_engine = FORCEWAKE_RENDER; \
 	else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
 		fw_engine = FORCEWAKE_MEDIA; \
@@ -935,7 +939,8 @@ static void \
 chv_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
 	enum forcewake_domains fw_engine = 0; \
 	GEN6_WRITE_HEADER; \
-	if (is_gen8_shadowed(dev_priv, reg)) \
+	if (!NEEDS_FORCE_WAKE(reg) || \
+	    is_gen8_shadowed(dev_priv, reg)) \
 		fw_engine = 0; \
 	else if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
 		fw_engine = FORCEWAKE_RENDER; \
-- 
2.4.9

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

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

* Re: [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions
  2015-10-22 12:34 ` [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions ville.syrjala
@ 2015-10-22 13:32   ` Daniel Vetter
  2015-10-22 13:49     ` Ville Syrjälä
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2015-10-22 13:32 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Oct 22, 2015 at 03:34:56PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There's no need for __raw_i915_read8() & co. bot be macros, so make them

s/bot/be

> inline funcitons. To avoid typo mistakes generate the inline functions

s/funcitons/functions/

> using preprocessor templates.
> 
> We have a few users of the raw register acces functions outside
> intel_uncore.c, so let's also move the functions into intel_drv.h.
> 
> While doing that switch I915_READ_FW() & co. to use the
> __raw_i915_read() functions, and use the _FW macros everywhere
> outside intel_uncore.c where we want to read registers without
> grabbing forcewake and whatnot. The only exception is
> i915_check_vgpu() which itself gets called from intel_uncore.c,
> so using the __raw_i915_read stuff there seems appropriate.
> 
> v2: Squash in the intel_uncore.c->i915_drv.h move
>     Convert I915_READ_FW() to use __raw_i915_read(), and use
>     I915_READ_FW() outside of intel_uncore.c (Chris)
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h     | 30 ++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/i915_irq.c     | 10 ++++------
>  drivers/gpu/drm/i915/i915_vgpu.c    |  6 +++---
>  drivers/gpu/drm/i915/intel_uncore.c | 14 +-------------
>  5 files changed, 37 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index eca94d0..89ba549 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1523,7 +1523,7 @@ static int gen6_drpc_info(struct seq_file *m)
>  		seq_printf(m, "RC information accurate: %s\n", yesno(count < 51));
>  	}
>  
> -	gt_core_status = readl(dev_priv->regs + GEN6_GT_CORE_STATUS);
> +	gt_core_status = I915_READ_FW(GEN6_GT_CORE_STATUS);
>  	trace_i915_reg_rw(false, GEN6_GT_CORE_STATUS, gt_core_status, 4, true);
>  
>  	rpmodectl1 = I915_READ(GEN6_RP_CONTROL);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9a15ebe..01fdc70 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3452,6 +3452,32 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
>  #define POSTING_READ(reg)	(void)I915_READ_NOTRACE(reg)
>  #define POSTING_READ16(reg)	(void)I915_READ16_NOTRACE(reg)
>  
> +#define __raw_read(x, s) \
> +static inline uint##x##_t __raw_i915_read##x(struct drm_i915_private *dev_priv, \
> +					     uint32_t reg) \
> +{ \
> +	return read##s(dev_priv->regs + reg); \
> +}
> +
> +#define __raw_write(x, s) \
> +static inline void __raw_i915_write##x(struct drm_i915_private *dev_priv, \
> +				       uint32_t reg, uint##x##_t val) \
> +{ \
> +	write##s(val, dev_priv->regs + reg); \
> +}
> +__raw_read(8, b)
> +__raw_read(16, w)
> +__raw_read(32, l)
> +__raw_read(64, q)
> +
> +__raw_write(8, b)
> +__raw_write(16, w)
> +__raw_write(32, l)
> +__raw_write(64, q)
> +
> +#undef __raw_read
> +#undef __raw_write
> +
>  /* These are untraced mmio-accessors that are only valid to be used inside
>   * criticial sections inside IRQ handlers where forcewake is explicitly
>   * controlled.
> @@ -3459,8 +3485,8 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
>   * Note: Should only be used between intel_uncore_forcewake_irqlock() and
>   * intel_uncore_forcewake_irqunlock().
>   */
> -#define I915_READ_FW(reg__) readl(dev_priv->regs + (reg__))
> -#define I915_WRITE_FW(reg__, val__) writel(val__, dev_priv->regs + (reg__))
> +#define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__))
> +#define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), (val__))
>  #define POSTING_READ_FW(reg__) (void)I915_READ_FW(reg__)
>  
>  /* "Broadcast RGB" property */
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9caf22c..a0ed58d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -717,9 +717,7 @@ static u32 g4x_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
>  	return I915_READ(PIPE_FRMCOUNT_G4X(pipe));
>  }
>  
> -/* raw reads, only for fast reads of display block, no need for forcewake etc. */
> -#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + (reg__))
> -
> +/* I915_READ_FW, only for fast reads of display block, no need for forcewake etc. */
>  static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->base.dev;
> @@ -733,9 +731,9 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
>  		vtotal /= 2;
>  
>  	if (IS_GEN2(dev))
> -		position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN2;
> +		position = I915_READ_FW(PIPEDSL(pipe)) & DSL_LINEMASK_GEN2;
>  	else
> -		position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
> +		position = I915_READ_FW(PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
>  
>  	/*
>  	 * On HSW, the DSL reg (0x70000) appears to return 0 if we
> @@ -827,7 +825,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
>  		 * We can split this into vertical and horizontal
>  		 * scanout position.
>  		 */
> -		position = (__raw_i915_read32(dev_priv, PIPEFRAMEPIXEL(pipe)) & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT;
> +		position = (I915_READ_FW(PIPEFRAMEPIXEL(pipe)) & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT;
>  
>  		/* convert to pixel counts */
>  		vbl_start *= htotal;
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index 5eee75b..dea7429 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -69,13 +69,13 @@ void i915_check_vgpu(struct drm_device *dev)
>  	if (!IS_HASWELL(dev))
>  		return;
>  
> -	magic = readq(dev_priv->regs + vgtif_reg(magic));
> +	magic = __raw_i915_read64(dev_priv, vgtif_reg(magic));
>  	if (magic != VGT_MAGIC)
>  		return;
>  
>  	version = INTEL_VGT_IF_VERSION_ENCODE(
> -		readw(dev_priv->regs + vgtif_reg(version_major)),
> -		readw(dev_priv->regs + vgtif_reg(version_minor)));
> +		__raw_i915_read16(dev_priv, vgtif_reg(version_major)),
> +		__raw_i915_read16(dev_priv, vgtif_reg(version_minor)));
>  	if (version != INTEL_VGT_IF_VERSION) {
>  		DRM_INFO("VGT interface version mismatch!\n");
>  		return;
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 1663ea5..8dfeac9 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -29,19 +29,7 @@
>  
>  #define FORCEWAKE_ACK_TIMEOUT_MS 50
>  
> -#define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + (reg__))
> -#define __raw_i915_write8(dev_priv__, reg__, val__) writeb(val__, (dev_priv__)->regs + (reg__))
> -
> -#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)->regs + (reg__))
> -#define __raw_i915_write16(dev_priv__, reg__, val__) writew(val__, (dev_priv__)->regs + (reg__))
> -
> -#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + (reg__))
> -#define __raw_i915_write32(dev_priv__, reg__, val__) writel(val__, (dev_priv__)->regs + (reg__))
> -
> -#define __raw_i915_read64(dev_priv__, reg__) readq((dev_priv__)->regs + (reg__))
> -#define __raw_i915_write64(dev_priv__, reg__, val__) writeq(val__, (dev_priv__)->regs + (reg__))
> -
> -#define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32(dev_priv__, reg__)
> +#define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__))
>  
>  static const char * const forcewake_domain_names[] = {
>  	"render",
> -- 
> 2.4.9
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/5] drm/i915: Read FORCEWAKE registers with I915_READ_FW()
  2015-10-22 12:34 ` [PATCH 2/5] drm/i915: Read FORCEWAKE registers with I915_READ_FW() ville.syrjala
@ 2015-10-22 13:37   ` Daniel Vetter
  2015-10-22 13:50     ` Ville Syrjälä
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2015-10-22 13:37 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Oct 22, 2015 at 03:34:57PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Change FORCEWAKE & co. reads for the error state to use I915_READ_FW().
> Reading a FORCEWAKE register using a function that can frob forcewake
> just seems wrong.
> 
> There is a check to skip grabbing the forcewake for accessing FORCEWAKE
> in intel_uncore.c, but there's no such check for FORCEWAKE_MT. So no
> idea what is currently happening with FORCEWAKE_MT reads. FORCEWAKE_VLV
> is fortunately outside the forcewake range anyway, so no actual issue
> with that one.
> 
> So let's just make the rule that you can access FORCEWAKE registers with

s/can/cannot/ I presume? With that changed

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> the normal I915_READ() stuff, and we can drop the extra FORCEWAKE check
> from NEEDS_FORCEWAKE(). While at it use NEEDS_FORCEWAKE() on BDW, where
> it was skipped for whatever bikeshed reason that I've already forgotten.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gpu_error.c | 6 +++---
>  drivers/gpu/drm/i915/intel_uncore.c   | 5 ++---
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 2f04e4f..de86f26 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1181,7 +1181,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>  	if (IS_VALLEYVIEW(dev)) {
>  		error->gtier[0] = I915_READ(GTIER);
>  		error->ier = I915_READ(VLV_IER);
> -		error->forcewake = I915_READ(FORCEWAKE_VLV);
> +		error->forcewake = I915_READ_FW(FORCEWAKE_VLV);
>  	}
>  
>  	if (IS_GEN7(dev))
> @@ -1193,14 +1193,14 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>  	}
>  
>  	if (IS_GEN6(dev)) {
> -		error->forcewake = I915_READ(FORCEWAKE);
> +		error->forcewake = I915_READ_FW(FORCEWAKE);
>  		error->gab_ctl = I915_READ(GAB_CTL);
>  		error->gfx_mode = I915_READ(GFX_MODE);
>  	}
>  
>  	/* 2: Registers which belong to multiple generations */
>  	if (INTEL_INFO(dev)->gen >= 7)
> -		error->forcewake = I915_READ(FORCEWAKE_MT);
> +		error->forcewake = I915_READ_FW(FORCEWAKE_MT);
>  
>  	if (INTEL_INFO(dev)->gen >= 6) {
>  		error->derrmr = I915_READ(DERRMR);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 8dfeac9..dca0979 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -513,8 +513,7 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
>  }
>  
>  /* We give fast paths for the really cool registers */
> -#define NEEDS_FORCE_WAKE(reg) \
> -	 ((reg) < 0x40000 && (reg) != FORCEWAKE)
> +#define NEEDS_FORCE_WAKE(reg) ((reg) < 0x40000)
>  
>  #define REG_RANGE(reg, start, end) ((reg) >= (start) && (reg) < (end))
>  
> @@ -918,7 +917,7 @@ static void \
>  gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
>  	GEN6_WRITE_HEADER; \
>  	hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \
> -	if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) \
> +	if (NEEDS_FORCE_WAKE(reg) && !is_gen8_shadowed(dev_priv, reg)) \
>  		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
>  	__raw_i915_write##x(dev_priv, reg, val); \
>  	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> -- 
> 2.4.9
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/5] drm/i915: Minor style nits in intel_uncore.c
  2015-10-22 12:34 ` [PATCH 3/5] drm/i915: Minor style nits in intel_uncore.c ville.syrjala
@ 2015-10-22 13:38   ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2015-10-22 13:38 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Oct 22, 2015 at 03:34:58PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I even bothered to check whether you still have spaces in here ;-)

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index dca0979..57af2c4 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -576,7 +576,7 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
>  	REG_RANGE((reg), 0x9400, 0x9800)
>  
>  #define FORCEWAKE_GEN9_BLITTER_RANGE_OFFSET(reg) \
> -	((reg) < 0x40000 &&\
> +	((reg) < 0x40000 && \
>  	 !FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg) && \
>  	 !FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg) && \
>  	 !FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg) && \
> @@ -749,7 +749,7 @@ chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
>  }
>  
>  #define SKL_NEEDS_FORCE_WAKE(reg) \
> -	 ((reg) < 0x40000 && !FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg))
> +	((reg) < 0x40000 && !FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg))
>  
>  #define __gen9_read(x) \
>  static u##x \
> -- 
> 2.4.9
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things
  2015-10-22 12:34 [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things ville.syrjala
                   ` (4 preceding siblings ...)
  2015-10-22 12:35 ` [PATCH 5/5] drm/i915: Add NEEDS_FORCEWAKE() checks for vlv/chv ville.syrjala
@ 2015-10-22 13:41 ` Chris Wilson
  2015-10-26 14:46   ` Ville Syrjälä
  5 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2015-10-22 13:41 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Oct 22, 2015 at 03:34:55PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> These patches fell off from my type safe register access series. I
> figured I'd post them separately since they're a fairly separate
> piece of work.
> 
> Ville Syrjälä (5):
>   drm/i915: Turn __raw_i915_read8() & co. in to inline functions
>   drm/i915: Read FORCEWAKE registers with I915_READ_FW()
>   drm/i915: Minor style nits in intel_uncore.c
>   drm/i915: Respin vlv/chv reagister access to look more like SKL
>   drm/i915: Add NEEDS_FORCEWAKE() checks for vlv/chv

All Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

We can still replace the vGPU block with just a volatile struct * which
would remove a few more warts.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 4/5] drm/i915: Respin vlv/chv reagister access to look more like SKL
  2015-10-22 12:34 ` [PATCH 4/5] drm/i915: Respin vlv/chv reagister access to look more like SKL ville.syrjala
@ 2015-10-22 13:42   ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2015-10-22 13:42 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Oct 22, 2015 at 03:34:59PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Change the fw domain handling in the vlv/chv register read/write
> functions to look more like the SKL code, ie. have a single
> __force_wake_get() get call instead of multiple ones.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Seems a bit ocd, but looks correct.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 37 ++++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 57af2c4..f38e88b 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -724,11 +724,14 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
>  #define __vlv_read(x) \
>  static u##x \
>  vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> +	enum forcewake_domains fw_engine = 0; \
>  	GEN6_READ_HEADER(x); \
>  	if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) \
> -		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> +		fw_engine = FORCEWAKE_RENDER; \
>  	else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) \
> -		__force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
> +		fw_engine = FORCEWAKE_MEDIA; \
> +	if (fw_engine) \
> +		__force_wake_get(dev_priv, fw_engine); \
>  	val = __raw_i915_read##x(dev_priv, reg); \
>  	GEN6_READ_FOOTER; \
>  }
> @@ -736,14 +739,16 @@ vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
>  #define __chv_read(x) \
>  static u##x \
>  chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> +	enum forcewake_domains fw_engine = 0; \
>  	GEN6_READ_HEADER(x); \
>  	if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
> -		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> +		fw_engine = FORCEWAKE_RENDER; \
>  	else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
> -		__force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
> +		fw_engine = FORCEWAKE_MEDIA; \
>  	else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) \
> -		__force_wake_get(dev_priv, \
> -				 FORCEWAKE_RENDER | FORCEWAKE_MEDIA); \
> +		fw_engine = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
> +	if (fw_engine) \
> +		__force_wake_get(dev_priv, fw_engine); \
>  	val = __raw_i915_read##x(dev_priv, reg); \
>  	GEN6_READ_FOOTER; \
>  }
> @@ -928,16 +933,18 @@ gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
>  #define __chv_write(x) \
>  static void \
>  chv_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> -	bool shadowed = is_gen8_shadowed(dev_priv, reg); \
> +	enum forcewake_domains fw_engine = 0; \
>  	GEN6_WRITE_HEADER; \
> -	if (!shadowed) { \
> -		if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
> -			__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> -		else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
> -			__force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
> -		else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) \
> -			__force_wake_get(dev_priv, FORCEWAKE_RENDER | FORCEWAKE_MEDIA); \
> -	} \
> +	if (is_gen8_shadowed(dev_priv, reg)) \
> +		fw_engine = 0; \
> +	else if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
> +		fw_engine = FORCEWAKE_RENDER; \
> +	else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
> +		fw_engine = FORCEWAKE_MEDIA; \
> +	else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) \
> +		fw_engine = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
> +	if (fw_engine) \
> +		__force_wake_get(dev_priv, fw_engine); \
>  	__raw_i915_write##x(dev_priv, reg, val); \
>  	GEN6_WRITE_FOOTER; \
>  }
> -- 
> 2.4.9
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 5/5] drm/i915: Add NEEDS_FORCEWAKE() checks for vlv/chv
  2015-10-22 12:35 ` [PATCH 5/5] drm/i915: Add NEEDS_FORCEWAKE() checks for vlv/chv ville.syrjala
@ 2015-10-22 13:44   ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2015-10-22 13:44 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Oct 22, 2015 at 03:35:00PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Include an early NEEDS_FORCEWAKE() check for vlv and chv.
> Hopefully that will avoid doing so many range checks in for many
> register accesses (at least for all display registers).
> 
> Note that vlv already had the check in the write path since it shares
> the gen6+ code for that.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I double-checked that all ranges are below the NEEDS_FORCEWAKE limit.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index f38e88b..f0f97b2 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -726,7 +726,9 @@ static u##x \
>  vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
>  	enum forcewake_domains fw_engine = 0; \
>  	GEN6_READ_HEADER(x); \
> -	if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) \
> +	if (!NEEDS_FORCE_WAKE(reg)) \
> +		fw_engine = 0; \
> +	else if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) \
>  		fw_engine = FORCEWAKE_RENDER; \
>  	else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) \
>  		fw_engine = FORCEWAKE_MEDIA; \
> @@ -741,7 +743,9 @@ static u##x \
>  chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
>  	enum forcewake_domains fw_engine = 0; \
>  	GEN6_READ_HEADER(x); \
> -	if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
> +	if (!NEEDS_FORCE_WAKE(reg)) \
> +		fw_engine = 0; \
> +	else if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
>  		fw_engine = FORCEWAKE_RENDER; \
>  	else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
>  		fw_engine = FORCEWAKE_MEDIA; \
> @@ -935,7 +939,8 @@ static void \
>  chv_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
>  	enum forcewake_domains fw_engine = 0; \
>  	GEN6_WRITE_HEADER; \
> -	if (is_gen8_shadowed(dev_priv, reg)) \
> +	if (!NEEDS_FORCE_WAKE(reg) || \
> +	    is_gen8_shadowed(dev_priv, reg)) \
>  		fw_engine = 0; \
>  	else if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
>  		fw_engine = FORCEWAKE_RENDER; \
> -- 
> 2.4.9
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions
  2015-10-22 13:32   ` Daniel Vetter
@ 2015-10-22 13:49     ` Ville Syrjälä
  0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2015-10-22 13:49 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Thu, Oct 22, 2015 at 03:32:11PM +0200, Daniel Vetter wrote:
> On Thu, Oct 22, 2015 at 03:34:56PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > There's no need for __raw_i915_read8() & co. bot be macros, so make them
> 
> s/bot/be

Actually s/bot/to/ :)

> 
> > inline funcitons. To avoid typo mistakes generate the inline functions
> 
> s/funcitons/functions/
> 
> > using preprocessor templates.
> > 
> > We have a few users of the raw register acces functions outside
> > intel_uncore.c, so let's also move the functions into intel_drv.h.
> > 
> > While doing that switch I915_READ_FW() & co. to use the
> > __raw_i915_read() functions, and use the _FW macros everywhere
> > outside intel_uncore.c where we want to read registers without
> > grabbing forcewake and whatnot. The only exception is
> > i915_check_vgpu() which itself gets called from intel_uncore.c,
> > so using the __raw_i915_read stuff there seems appropriate.
> > 
> > v2: Squash in the intel_uncore.c->i915_drv.h move
> >     Convert I915_READ_FW() to use __raw_i915_read(), and use
> >     I915_READ_FW() outside of intel_uncore.c (Chris)
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
> >  drivers/gpu/drm/i915/i915_drv.h     | 30 ++++++++++++++++++++++++++++--
> >  drivers/gpu/drm/i915/i915_irq.c     | 10 ++++------
> >  drivers/gpu/drm/i915/i915_vgpu.c    |  6 +++---
> >  drivers/gpu/drm/i915/intel_uncore.c | 14 +-------------
> >  5 files changed, 37 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index eca94d0..89ba549 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1523,7 +1523,7 @@ static int gen6_drpc_info(struct seq_file *m)
> >  		seq_printf(m, "RC information accurate: %s\n", yesno(count < 51));
> >  	}
> >  
> > -	gt_core_status = readl(dev_priv->regs + GEN6_GT_CORE_STATUS);
> > +	gt_core_status = I915_READ_FW(GEN6_GT_CORE_STATUS);
> >  	trace_i915_reg_rw(false, GEN6_GT_CORE_STATUS, gt_core_status, 4, true);
> >  
> >  	rpmodectl1 = I915_READ(GEN6_RP_CONTROL);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 9a15ebe..01fdc70 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3452,6 +3452,32 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
> >  #define POSTING_READ(reg)	(void)I915_READ_NOTRACE(reg)
> >  #define POSTING_READ16(reg)	(void)I915_READ16_NOTRACE(reg)
> >  
> > +#define __raw_read(x, s) \
> > +static inline uint##x##_t __raw_i915_read##x(struct drm_i915_private *dev_priv, \
> > +					     uint32_t reg) \
> > +{ \
> > +	return read##s(dev_priv->regs + reg); \
> > +}
> > +
> > +#define __raw_write(x, s) \
> > +static inline void __raw_i915_write##x(struct drm_i915_private *dev_priv, \
> > +				       uint32_t reg, uint##x##_t val) \
> > +{ \
> > +	write##s(val, dev_priv->regs + reg); \
> > +}
> > +__raw_read(8, b)
> > +__raw_read(16, w)
> > +__raw_read(32, l)
> > +__raw_read(64, q)
> > +
> > +__raw_write(8, b)
> > +__raw_write(16, w)
> > +__raw_write(32, l)
> > +__raw_write(64, q)
> > +
> > +#undef __raw_read
> > +#undef __raw_write
> > +
> >  /* These are untraced mmio-accessors that are only valid to be used inside
> >   * criticial sections inside IRQ handlers where forcewake is explicitly
> >   * controlled.
> > @@ -3459,8 +3485,8 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
> >   * Note: Should only be used between intel_uncore_forcewake_irqlock() and
> >   * intel_uncore_forcewake_irqunlock().
> >   */
> > -#define I915_READ_FW(reg__) readl(dev_priv->regs + (reg__))
> > -#define I915_WRITE_FW(reg__, val__) writel(val__, dev_priv->regs + (reg__))
> > +#define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__))
> > +#define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), (val__))
> >  #define POSTING_READ_FW(reg__) (void)I915_READ_FW(reg__)
> >  
> >  /* "Broadcast RGB" property */
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 9caf22c..a0ed58d 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -717,9 +717,7 @@ static u32 g4x_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
> >  	return I915_READ(PIPE_FRMCOUNT_G4X(pipe));
> >  }
> >  
> > -/* raw reads, only for fast reads of display block, no need for forcewake etc. */
> > -#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + (reg__))
> > -
> > +/* I915_READ_FW, only for fast reads of display block, no need for forcewake etc. */
> >  static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
> >  {
> >  	struct drm_device *dev = crtc->base.dev;
> > @@ -733,9 +731,9 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
> >  		vtotal /= 2;
> >  
> >  	if (IS_GEN2(dev))
> > -		position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN2;
> > +		position = I915_READ_FW(PIPEDSL(pipe)) & DSL_LINEMASK_GEN2;
> >  	else
> > -		position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
> > +		position = I915_READ_FW(PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
> >  
> >  	/*
> >  	 * On HSW, the DSL reg (0x70000) appears to return 0 if we
> > @@ -827,7 +825,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
> >  		 * We can split this into vertical and horizontal
> >  		 * scanout position.
> >  		 */
> > -		position = (__raw_i915_read32(dev_priv, PIPEFRAMEPIXEL(pipe)) & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT;
> > +		position = (I915_READ_FW(PIPEFRAMEPIXEL(pipe)) & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT;
> >  
> >  		/* convert to pixel counts */
> >  		vbl_start *= htotal;
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> > index 5eee75b..dea7429 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.c
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> > @@ -69,13 +69,13 @@ void i915_check_vgpu(struct drm_device *dev)
> >  	if (!IS_HASWELL(dev))
> >  		return;
> >  
> > -	magic = readq(dev_priv->regs + vgtif_reg(magic));
> > +	magic = __raw_i915_read64(dev_priv, vgtif_reg(magic));
> >  	if (magic != VGT_MAGIC)
> >  		return;
> >  
> >  	version = INTEL_VGT_IF_VERSION_ENCODE(
> > -		readw(dev_priv->regs + vgtif_reg(version_major)),
> > -		readw(dev_priv->regs + vgtif_reg(version_minor)));
> > +		__raw_i915_read16(dev_priv, vgtif_reg(version_major)),
> > +		__raw_i915_read16(dev_priv, vgtif_reg(version_minor)));
> >  	if (version != INTEL_VGT_IF_VERSION) {
> >  		DRM_INFO("VGT interface version mismatch!\n");
> >  		return;
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 1663ea5..8dfeac9 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -29,19 +29,7 @@
> >  
> >  #define FORCEWAKE_ACK_TIMEOUT_MS 50
> >  
> > -#define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + (reg__))
> > -#define __raw_i915_write8(dev_priv__, reg__, val__) writeb(val__, (dev_priv__)->regs + (reg__))
> > -
> > -#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)->regs + (reg__))
> > -#define __raw_i915_write16(dev_priv__, reg__, val__) writew(val__, (dev_priv__)->regs + (reg__))
> > -
> > -#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + (reg__))
> > -#define __raw_i915_write32(dev_priv__, reg__, val__) writel(val__, (dev_priv__)->regs + (reg__))
> > -
> > -#define __raw_i915_read64(dev_priv__, reg__) readq((dev_priv__)->regs + (reg__))
> > -#define __raw_i915_write64(dev_priv__, reg__, val__) writeq(val__, (dev_priv__)->regs + (reg__))
> > -
> > -#define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32(dev_priv__, reg__)
> > +#define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32((dev_priv__), (reg__))
> >  
> >  static const char * const forcewake_domain_names[] = {
> >  	"render",
> > -- 
> > 2.4.9
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

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

* Re: [PATCH 2/5] drm/i915: Read FORCEWAKE registers with I915_READ_FW()
  2015-10-22 13:37   ` Daniel Vetter
@ 2015-10-22 13:50     ` Ville Syrjälä
  0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2015-10-22 13:50 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Thu, Oct 22, 2015 at 03:37:33PM +0200, Daniel Vetter wrote:
> On Thu, Oct 22, 2015 at 03:34:57PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Change FORCEWAKE & co. reads for the error state to use I915_READ_FW().
> > Reading a FORCEWAKE register using a function that can frob forcewake
> > just seems wrong.
> > 
> > There is a check to skip grabbing the forcewake for accessing FORCEWAKE
> > in intel_uncore.c, but there's no such check for FORCEWAKE_MT. So no
> > idea what is currently happening with FORCEWAKE_MT reads. FORCEWAKE_VLV
> > is fortunately outside the forcewake range anyway, so no actual issue
> > with that one.
> > 
> > So let's just make the rule that you can access FORCEWAKE registers with
> 
> s/can/cannot/ I presume?

Yes.

> With that changed
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > the normal I915_READ() stuff, and we can drop the extra FORCEWAKE check
> > from NEEDS_FORCEWAKE(). While at it use NEEDS_FORCEWAKE() on BDW, where
> > it was skipped for whatever bikeshed reason that I've already forgotten.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gpu_error.c | 6 +++---
> >  drivers/gpu/drm/i915/intel_uncore.c   | 5 ++---
> >  2 files changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 2f04e4f..de86f26 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -1181,7 +1181,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> >  	if (IS_VALLEYVIEW(dev)) {
> >  		error->gtier[0] = I915_READ(GTIER);
> >  		error->ier = I915_READ(VLV_IER);
> > -		error->forcewake = I915_READ(FORCEWAKE_VLV);
> > +		error->forcewake = I915_READ_FW(FORCEWAKE_VLV);
> >  	}
> >  
> >  	if (IS_GEN7(dev))
> > @@ -1193,14 +1193,14 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> >  	}
> >  
> >  	if (IS_GEN6(dev)) {
> > -		error->forcewake = I915_READ(FORCEWAKE);
> > +		error->forcewake = I915_READ_FW(FORCEWAKE);
> >  		error->gab_ctl = I915_READ(GAB_CTL);
> >  		error->gfx_mode = I915_READ(GFX_MODE);
> >  	}
> >  
> >  	/* 2: Registers which belong to multiple generations */
> >  	if (INTEL_INFO(dev)->gen >= 7)
> > -		error->forcewake = I915_READ(FORCEWAKE_MT);
> > +		error->forcewake = I915_READ_FW(FORCEWAKE_MT);
> >  
> >  	if (INTEL_INFO(dev)->gen >= 6) {
> >  		error->derrmr = I915_READ(DERRMR);
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 8dfeac9..dca0979 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -513,8 +513,7 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
> >  }
> >  
> >  /* We give fast paths for the really cool registers */
> > -#define NEEDS_FORCE_WAKE(reg) \
> > -	 ((reg) < 0x40000 && (reg) != FORCEWAKE)
> > +#define NEEDS_FORCE_WAKE(reg) ((reg) < 0x40000)
> >  
> >  #define REG_RANGE(reg, start, end) ((reg) >= (start) && (reg) < (end))
> >  
> > @@ -918,7 +917,7 @@ static void \
> >  gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> >  	GEN6_WRITE_HEADER; \
> >  	hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \
> > -	if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) \
> > +	if (NEEDS_FORCE_WAKE(reg) && !is_gen8_shadowed(dev_priv, reg)) \
> >  		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> >  	__raw_i915_write##x(dev_priv, reg, val); \
> >  	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> > -- 
> > 2.4.9
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

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

* Re: [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things
  2015-10-22 13:41 ` [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things Chris Wilson
@ 2015-10-26 14:46   ` Ville Syrjälä
  0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2015-10-26 14:46 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Thu, Oct 22, 2015 at 02:41:29PM +0100, Chris Wilson wrote:
> On Thu, Oct 22, 2015 at 03:34:55PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > These patches fell off from my type safe register access series. I
> > figured I'd post them separately since they're a fairly separate
> > piece of work.
> > 
> > Ville Syrjälä (5):
> >   drm/i915: Turn __raw_i915_read8() & co. in to inline functions
> >   drm/i915: Read FORCEWAKE registers with I915_READ_FW()
> >   drm/i915: Minor style nits in intel_uncore.c
> >   drm/i915: Respin vlv/chv reagister access to look more like SKL
> >   drm/i915: Add NEEDS_FORCEWAKE() checks for vlv/chv
> 
> All Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Entire series merged, with typos corrected. Thanks for the reviews.

> 
> We can still replace the vGPU block with just a volatile struct * which
> would remove a few more warts.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

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

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

end of thread, other threads:[~2015-10-26 14:46 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-22 12:34 [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things ville.syrjala
2015-10-22 12:34 ` [PATCH 1/5] drm/i915: Turn __raw_i915_read8() & co. in to inline functions ville.syrjala
2015-10-22 13:32   ` Daniel Vetter
2015-10-22 13:49     ` Ville Syrjälä
2015-10-22 12:34 ` [PATCH 2/5] drm/i915: Read FORCEWAKE registers with I915_READ_FW() ville.syrjala
2015-10-22 13:37   ` Daniel Vetter
2015-10-22 13:50     ` Ville Syrjälä
2015-10-22 12:34 ` [PATCH 3/5] drm/i915: Minor style nits in intel_uncore.c ville.syrjala
2015-10-22 13:38   ` Daniel Vetter
2015-10-22 12:34 ` [PATCH 4/5] drm/i915: Respin vlv/chv reagister access to look more like SKL ville.syrjala
2015-10-22 13:42   ` Daniel Vetter
2015-10-22 12:35 ` [PATCH 5/5] drm/i915: Add NEEDS_FORCEWAKE() checks for vlv/chv ville.syrjala
2015-10-22 13:44   ` Daniel Vetter
2015-10-22 13:41 ` [PATCH 0/5] drm/i915: Expose __raw_i915_read() & co. to the outside, and a few forcewake things Chris Wilson
2015-10-26 14:46   ` Ville Syrjälä

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