Re: [Intel-gfx] [PATCH 2/5] drm/i915: Move vblank evasion to commit (v3)

2014-12-23 Thread Ander Conselvan de Oliveira

On 12/16/2014 02:23 AM, Matt Roper wrote:

Move the vblank evasion up from the low-level, hw-specific
update_plane() handlers to the general plane commit operation.
Everything inside commit should now be non-sleeping, so this brings us
closer to how vblank evasion will behave once we move over to atomic.

v2:
  - Restore lost intel_crtc-active check on vblank evasion

v3:
  - Replace assert_pipe_enabled() in intel_disable_primary_hw_plane()
with an intel_crtc-active test; it turns out assert_pipe_enabled()
grabs some mutexes and can sleep, which we can't do with interrupts
disabled.


Sounds like this should have gone in the previous patch, or a separate one.

Ander



Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
  drivers/gpu/drm/i915/intel_display.c | 13 ++-
  drivers/gpu/drm/i915/intel_drv.h |  4 
  drivers/gpu/drm/i915/intel_sprite.c  | 42 
  3 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5d90114..ce552d1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2165,7 +2165,8 @@ static void intel_disable_primary_hw_plane(struct 
drm_plane *plane,
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

-   assert_pipe_enabled(dev_priv, intel_crtc-pipe);
+   if (WARN_ON(!intel_crtc-active))
+   return;

if (!intel_crtc-primary_enabled)
return;
@@ -11861,6 +11862,12 @@ static void intel_begin_crtc_commit(struct drm_crtc 
*crtc)

if (intel_crtc-atomic.update_wm)
intel_update_watermarks(crtc);
+
+   /* Perform vblank evasion around commit operation */
+   if (intel_crtc-active)
+   intel_crtc-atomic.evade =
+   intel_pipe_update_start(intel_crtc,
+   
intel_crtc-atomic.start_vbl_count);
  }

  static void intel_finish_crtc_commit(struct drm_crtc *crtc)
@@ -11869,6 +11876,10 @@ static void intel_finish_crtc_commit(struct drm_crtc 
*crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_plane *p;

+   if (intel_crtc-atomic.evade)
+   intel_pipe_update_end(intel_crtc,
+ intel_crtc-atomic.start_vbl_count);
+
if (intel_crtc-atomic.wait_vblank)
intel_wait_for_vblank(dev, intel_crtc-pipe);

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a03bd72..1934156 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -428,6 +428,10 @@ struct skl_pipe_wm {
   * and thus can't be run with interrupts disabled.
   */
  struct intel_crtc_atomic_commit {
+   /* vblank evasion */
+   bool evade;
+   unsigned start_vbl_count;
+
/* Sleepable operations to perform before commit */
bool wait_for_flips;
bool disable_fbc;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index ff7d6a1..2520748 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -412,8 +412,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc,
u32 sprctl;
unsigned long sprsurf_offset, linear_offset;
int pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
-   u32 start_vbl_count;
-   bool atomic_update;

sprctl = I915_READ(SPCNTR(pipe, plane));

@@ -502,8 +500,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc,
linear_offset += src_h * fb-pitches[0] + src_w * pixel_size;
}

-   atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count);
-
intel_update_primary_plane(intel_crtc);

if (IS_CHERRYVIEW(dev)  pipe == PIPE_B)
@@ -525,9 +521,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc,
   sprsurf_offset);

intel_flush_primary_plane(dev_priv, intel_crtc-plane);
-
-   if (atomic_update)
-   intel_pipe_update_end(intel_crtc, start_vbl_count);
  }

  static void
@@ -539,10 +532,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct 
drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_plane-pipe;
int plane = intel_plane-plane;
-   u32 start_vbl_count;
-   bool atomic_update;
-
-   atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count);

intel_update_primary_plane(intel_crtc);

@@ -553,9 +542,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc)

intel_flush_primary_plane(dev_priv, intel_crtc-plane);

-   if (atomic_update)
-   intel_pipe_update_end(intel_crtc, start_vbl_count);
-
intel_update_sprite_watermarks(dplane, crtc, 0, 

[Intel-gfx] [PATCH 2/5] drm/i915: Move vblank evasion to commit (v3)

2014-12-15 Thread Matt Roper
Move the vblank evasion up from the low-level, hw-specific
update_plane() handlers to the general plane commit operation.
Everything inside commit should now be non-sleeping, so this brings us
closer to how vblank evasion will behave once we move over to atomic.

v2:
 - Restore lost intel_crtc-active check on vblank evasion

v3:
 - Replace assert_pipe_enabled() in intel_disable_primary_hw_plane()
   with an intel_crtc-active test; it turns out assert_pipe_enabled()
   grabs some mutexes and can sleep, which we can't do with interrupts
   disabled.

Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 13 ++-
 drivers/gpu/drm/i915/intel_drv.h |  4 
 drivers/gpu/drm/i915/intel_sprite.c  | 42 
 3 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5d90114..ce552d1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2165,7 +2165,8 @@ static void intel_disable_primary_hw_plane(struct 
drm_plane *plane,
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-   assert_pipe_enabled(dev_priv, intel_crtc-pipe);
+   if (WARN_ON(!intel_crtc-active))
+   return;
 
if (!intel_crtc-primary_enabled)
return;
@@ -11861,6 +11862,12 @@ static void intel_begin_crtc_commit(struct drm_crtc 
*crtc)
 
if (intel_crtc-atomic.update_wm)
intel_update_watermarks(crtc);
+
+   /* Perform vblank evasion around commit operation */
+   if (intel_crtc-active)
+   intel_crtc-atomic.evade =
+   intel_pipe_update_start(intel_crtc,
+   
intel_crtc-atomic.start_vbl_count);
 }
 
 static void intel_finish_crtc_commit(struct drm_crtc *crtc)
@@ -11869,6 +11876,10 @@ static void intel_finish_crtc_commit(struct drm_crtc 
*crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_plane *p;
 
+   if (intel_crtc-atomic.evade)
+   intel_pipe_update_end(intel_crtc,
+ intel_crtc-atomic.start_vbl_count);
+
if (intel_crtc-atomic.wait_vblank)
intel_wait_for_vblank(dev, intel_crtc-pipe);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a03bd72..1934156 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -428,6 +428,10 @@ struct skl_pipe_wm {
  * and thus can't be run with interrupts disabled.
  */
 struct intel_crtc_atomic_commit {
+   /* vblank evasion */
+   bool evade;
+   unsigned start_vbl_count;
+
/* Sleepable operations to perform before commit */
bool wait_for_flips;
bool disable_fbc;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index ff7d6a1..2520748 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -412,8 +412,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc,
u32 sprctl;
unsigned long sprsurf_offset, linear_offset;
int pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
-   u32 start_vbl_count;
-   bool atomic_update;
 
sprctl = I915_READ(SPCNTR(pipe, plane));
 
@@ -502,8 +500,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc,
linear_offset += src_h * fb-pitches[0] + src_w * pixel_size;
}
 
-   atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count);
-
intel_update_primary_plane(intel_crtc);
 
if (IS_CHERRYVIEW(dev)  pipe == PIPE_B)
@@ -525,9 +521,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc,
   sprsurf_offset);
 
intel_flush_primary_plane(dev_priv, intel_crtc-plane);
-
-   if (atomic_update)
-   intel_pipe_update_end(intel_crtc, start_vbl_count);
 }
 
 static void
@@ -539,10 +532,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct 
drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_plane-pipe;
int plane = intel_plane-plane;
-   u32 start_vbl_count;
-   bool atomic_update;
-
-   atomic_update = intel_pipe_update_start(intel_crtc, start_vbl_count);
 
intel_update_primary_plane(intel_crtc);
 
@@ -553,9 +542,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc 
*crtc)
 
intel_flush_primary_plane(dev_priv, intel_crtc-plane);
 
-   if (atomic_update)
-   intel_pipe_update_end(intel_crtc, start_vbl_count);
-
intel_update_sprite_watermarks(dplane, crtc, 0, 0, 0, false, false);
 }
 
@@ -626,8 +612,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
u32 sprctl,