diff mbox

[RFC,14/14] WIP/drm/i915: Nuke posting reads from plane updates

Message ID 20170317211808.14693-15-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä March 17, 2017, 9:18 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We'll maybe want just one posting read at the end? This does seem to
potentially shave a few usec off from the update.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 --------
 drivers/gpu/drm/i915/intel_sprite.c  | 8 --------
 2 files changed, 16 deletions(-)

Comments

Chris Wilson March 17, 2017, 10:12 p.m. UTC | #1 
On Fri, Mar 17, 2017 at 11:18:08PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We'll maybe want just one posting read at the end? This does seem to
> potentially shave a few usec off from the update.

They are all superfluous until proven otherwise. The only cases where I
would (quietly) argue for a post is before a latch (to document the
serialisation of the latch with the controls that go before) and before
completing the transaction and waiting for the hw/irq.
-Chris
Maarten Lankhorst March 22, 2017, 10:42 a.m. UTC | #2 
Op 17-03-17 om 23:12 schreef Chris Wilson:
> On Fri, Mar 17, 2017 at 11:18:08PM +0200, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> We'll maybe want just one posting read at the end? This does seem to
>> potentially shave a few usec off from the update.
> They are all superfluous until proven otherwise. The only cases where I
> would (quietly) argue for a post is before a latch (to document the
> serialisation of the latch with the controls that go before) and before
> completing the transaction and waiting for the hw/irq.
> -Chris
>
Patch series look sane, but didn't review extensively for errors.

Was considering doing patch 13 myself, but didn't have the courage to.

Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c35818b42762..d9ec654db5a3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3125,7 +3125,6 @@  static void i9xx_update_primary_plane(struct drm_plane *primary,
 			      intel_plane_ggtt_offset(plane_state) +
 			      intel_crtc->dspaddr_offset);
 	}
-	POSTING_READ_FW(reg);
 }
 
 static void i9xx_disable_primary_plane(struct drm_plane *primary,
@@ -3141,7 +3140,6 @@  static void i9xx_disable_primary_plane(struct drm_plane *primary,
 		I915_WRITE_FW(DSPSURF(plane), 0);
 	else
 		I915_WRITE_FW(DSPADDR(plane), 0);
-	POSTING_READ_FW(DSPCNTR(plane));
 }
 
 static u32
@@ -3379,7 +3377,6 @@  static void skylake_update_primary_plane(struct drm_plane *plane,
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id),
 		      intel_plane_ggtt_offset(plane_state) + surf_addr);
 
-	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
 }
 
 static void skylake_disable_primary_plane(struct drm_plane *primary,
@@ -3392,7 +3389,6 @@  static void skylake_disable_primary_plane(struct drm_plane *primary,
 
 	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), 0);
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id), 0);
-	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
 }
 
 /* Assume fb object is pinned & idle & fenced and just update base pointers */
@@ -9171,7 +9167,6 @@  static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
 		 * whilst the cursor is disabled.
 		 */
 		I915_WRITE_FW(CURCNTR(PIPE_A), 0);
-		POSTING_READ_FW(CURCNTR(PIPE_A));
 		intel_crtc->cursor_cntl = 0;
 	}
 
@@ -9187,7 +9182,6 @@  static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
 
 	if (intel_crtc->cursor_cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
-		POSTING_READ_FW(CURCNTR(PIPE_A));
 		intel_crtc->cursor_cntl = cntl;
 	}
 }
@@ -9243,13 +9237,11 @@  static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 
 	if (intel_crtc->cursor_cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(pipe), cntl);
-		POSTING_READ_FW(CURCNTR(pipe));
 		intel_crtc->cursor_cntl = cntl;
 	}
 
 	/* and commit changes on next vblank */
 	I915_WRITE_FW(CURBASE(pipe), base);
-	POSTING_READ_FW(CURBASE(pipe));
 
 	intel_crtc->cursor_base = base;
 }
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 06c89deca36b..5cf88f95c23e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -288,7 +288,6 @@  skl_update_plane(struct drm_plane *drm_plane,
 	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl);
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id),
 		      intel_plane_ggtt_offset(plane_state) + surf_addr);
-	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
 }
 
 static void
@@ -303,7 +302,6 @@  skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), 0);
 
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id), 0);
-	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
 }
 
 static void
@@ -459,7 +457,6 @@  vlv_update_plane(struct drm_plane *dplane,
 	I915_WRITE_FW(SPCNTR(pipe, plane_id), sprctl);
 	I915_WRITE_FW(SPSURF(pipe, plane_id),
 		      intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
-	POSTING_READ_FW(SPSURF(pipe, plane_id));
 }
 
 static void
@@ -474,7 +471,6 @@  vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	I915_WRITE_FW(SPCNTR(pipe, plane_id), 0);
 
 	I915_WRITE_FW(SPSURF(pipe, plane_id), 0);
-	POSTING_READ_FW(SPSURF(pipe, plane_id));
 }
 
 static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
@@ -591,7 +587,6 @@  ivb_update_plane(struct drm_plane *plane,
 	I915_WRITE_FW(SPRCTL(pipe), sprctl);
 	I915_WRITE_FW(SPRSURF(pipe),
 		      intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
-	POSTING_READ_FW(SPRSURF(pipe));
 }
 
 static void
@@ -608,7 +603,6 @@  ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 		I915_WRITE_FW(SPRSCALE(pipe), 0);
 
 	I915_WRITE_FW(SPRSURF(pipe), 0);
-	POSTING_READ_FW(SPRSURF(pipe));
 }
 
 static u32 ilk_sprite_ctl(const struct intel_crtc_state *crtc_state,
@@ -717,7 +711,6 @@  ilk_update_plane(struct drm_plane *plane,
 	I915_WRITE_FW(DVSCNTR(pipe), dvscntr);
 	I915_WRITE_FW(DVSSURF(pipe),
 		      intel_plane_ggtt_offset(plane_state) + dvssurf_offset);
-	POSTING_READ_FW(DVSSURF(pipe));
 }
 
 static void
@@ -733,7 +726,6 @@  ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	I915_WRITE_FW(DVSSCALE(pipe), 0);
 
 	I915_WRITE_FW(DVSSURF(pipe), 0);
-	POSTING_READ_FW(DVSSURF(pipe));
 }
 
 static int