* [PATCH 1/5] drm/i915: Refactor work that can sleep out of commit (v6)
2014-12-23 18:41 [PATCH 0/5] i915 atomic plane helper conversion (v5) Matt Roper
@ 2014-12-23 18:41 ` Matt Roper
2014-12-24 15:59 ` [PATCH 1/5] drm/i915: Refactor work that can sleep out of commit (v7) Matt Roper
2014-12-23 18:41 ` [PATCH 2/5] drm/i915: Move vblank evasion to commit (v4) Matt Roper
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Matt Roper @ 2014-12-23 18:41 UTC (permalink / raw)
To: intel-gfx
Once we integrate our work into the atomic pipeline, plane commit
operations will need to happen with interrupts disabled, due to vblank
evasion. Our commit functions today include sleepable work, so those
operations need to be split out and run either before or after the
atomic register programming.
The solution here calculates which of those operations will need to be
performed during the 'check' phase and sets flags in an intel_crtc
sub-struct. New intel_begin_crtc_commit() and
intel_finish_crtc_commit() functions are added before and after the
actual register programming; these will eventually be called from the
atomic plane helper's .atomic_begin() and .atomic_end() entrypoints.
v2: Fix broken sprite code split
v3: Make the pre/post commit work crtc-based to match how we eventually
want this to be called from the atomic plane helpers.
v4: Some platforms that haven't had their watermark code reworked were
waiting for vblank, then calling update_sprite_watermarks in their
platform-specific disable code. These also need to be flagged out
of the critical section.
v5: Sprite plane test for primary show/hide should just set the flag to
wait for pending flips, not actually perform the wait. (Ander)
v6:
- Rebase onto latest di-nightly; picks up an important runtime PM fix.
- Handle 'wait_for_flips' flag in intel_begin_crtc_commit(). (Ander)
- Use wait_for_flips flag for primary plane update rather than
performing the wait in the check routine.
- Added kerneldoc to pre_disable/post_enable functions that are no
longer static. (Ander)
- 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.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 199 ++++++++++++++++++++++-------------
drivers/gpu/drm/i915/intel_drv.h | 31 ++++++
drivers/gpu/drm/i915/intel_sprite.c | 98 ++++++++---------
3 files changed, 209 insertions(+), 119 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a03955d..6bd44f3 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;
@@ -11737,7 +11738,11 @@ static int
intel_check_primary_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc = state->base.crtc;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_framebuffer *fb = state->base.fb;
struct drm_rect *dest = &state->dst;
struct drm_rect *src = &state->src;
@@ -11752,10 +11757,40 @@ intel_check_primary_plane(struct drm_plane *plane,
if (ret)
return ret;
- intel_crtc_wait_for_pending_flips(crtc);
- if (intel_crtc_has_pending_flip(crtc)) {
- DRM_ERROR("pipe is still busy with an old pageflip\n");
- return -EBUSY;
+ if (intel_crtc->active) {
+ intel_crtc->atomic.wait_for_flips = true;
+
+ /*
+ * FBC does not work on some platforms for rotated
+ * planes, so disable it when rotation is not 0 and
+ * update it when rotation is set back to 0.
+ *
+ * FIXME: This is redundant with the fbc update done in
+ * the primary plane enable function except that that
+ * one is done too late. We eventually need to unify
+ * this.
+ */
+ if (intel_crtc->primary_enabled &&
+ INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+ dev_priv->fbc.plane == intel_crtc->plane &&
+ intel_plane->rotation != BIT(DRM_ROTATE_0)) {
+ intel_crtc->atomic.disable_fbc = true;
+ }
+
+ if (state->visible) {
+ /*
+ * BDW signals flip done immediately if the plane
+ * is disabled, even if the plane enable is already
+ * armed to occur at the next vblank :(
+ */
+ if (IS_BROADWELL(dev) && !intel_crtc->primary_enabled)
+ intel_crtc->atomic.wait_vblank = true;
+ }
+
+ intel_crtc->atomic.fb_bits |=
+ INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
+
+ intel_crtc->atomic.update_fbc = true;
}
return 0;
@@ -11773,18 +11808,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_rect *src = &state->src;
- enum pipe pipe = intel_plane->pipe;
-
- if (!fb) {
- /*
- * 'prepare' is never called when plane is being disabled, so
- * we need to handle frontbuffer tracking here
- */
- mutex_lock(&dev->struct_mutex);
- i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
- INTEL_FRONTBUFFER_PRIMARY(pipe));
- mutex_unlock(&dev->struct_mutex);
- }
plane->fb = fb;
crtc->x = src->x1 >> 16;
@@ -11801,26 +11824,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
intel_plane->obj = obj;
if (intel_crtc->active) {
- /*
- * FBC does not work on some platforms for rotated
- * planes, so disable it when rotation is not 0 and
- * update it when rotation is set back to 0.
- *
- * FIXME: This is redundant with the fbc update done in
- * the primary plane enable function except that that
- * one is done too late. We eventually need to unify
- * this.
- */
- if (intel_crtc->primary_enabled &&
- INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
- dev_priv->fbc.plane == intel_crtc->plane &&
- intel_plane->rotation != BIT(DRM_ROTATE_0)) {
- intel_fbc_disable(dev);
- }
-
if (state->visible) {
- bool was_enabled = intel_crtc->primary_enabled;
-
/* FIXME: kill this fastboot hack */
intel_update_pipe_size(intel_crtc);
@@ -11828,14 +11832,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
dev_priv->display.update_primary_plane(crtc, plane->fb,
crtc->x, crtc->y);
-
- /*
- * BDW signals flip done immediately if the plane
- * is disabled, even if the plane enable is already
- * armed to occur at the next vblank :(
- */
- if (IS_BROADWELL(dev) && !was_enabled)
- intel_wait_for_vblank(dev, intel_crtc->pipe);
} else {
/*
* If clipping results in a non-visible primary plane,
@@ -11846,13 +11842,59 @@ intel_commit_primary_plane(struct drm_plane *plane,
*/
intel_disable_primary_hw_plane(plane, crtc);
}
+ }
+}
+
+static void intel_begin_crtc_commit(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
+ if (intel_crtc->atomic.wait_for_flips)
+ intel_crtc_wait_for_pending_flips(crtc);
+ if (intel_crtc->atomic.disable_fbc)
+ intel_fbc_disable(dev);
+
+ if (intel_crtc->atomic.pre_disable_primary)
+ intel_pre_disable_primary(crtc);
+
+ if (intel_crtc->atomic.update_wm)
+ intel_update_watermarks(crtc);
+
+ intel_runtime_pm_get(dev_priv);
+}
+
+static void intel_finish_crtc_commit(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_plane *p;
+
+ intel_runtime_pm_put(dev_priv);
+
+ if (intel_crtc->atomic.wait_vblank)
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+ intel_frontbuffer_flip(dev, intel_crtc->atomic.fb_bits);
+
+ if (intel_crtc->atomic.update_fbc) {
mutex_lock(&dev->struct_mutex);
intel_fbc_update(dev);
mutex_unlock(&dev->struct_mutex);
}
+
+ if (intel_crtc->atomic.post_enable_primary)
+ intel_post_enable_primary(crtc);
+
+ drm_for_each_legacy_plane(p, &dev->mode_config.plane_list)
+ if (intel_crtc->atomic.update_sprite_watermarks & drm_plane_index(p))
+ intel_update_sprite_watermarks(p, crtc, 0, 0, 0,
+ false, false);
+
+ memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic));
}
int
@@ -11863,9 +11905,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_w, uint32_t src_h)
{
struct drm_device *dev = plane->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_framebuffer *old_fb = plane->fb;
- struct intel_plane_state state;
+ struct intel_plane_state state = {{ 0 }};
struct intel_plane *intel_plane = to_intel_plane(plane);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int ret;
@@ -11903,9 +11944,33 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
return ret;
}
- intel_runtime_pm_get(dev_priv);
+ if (!state.base.fb) {
+ unsigned fb_bits = 0;
+
+ switch (plane->type) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ fb_bits = INTEL_FRONTBUFFER_PRIMARY(intel_plane->pipe);
+ break;
+ case DRM_PLANE_TYPE_CURSOR:
+ fb_bits = INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe);
+ break;
+ case DRM_PLANE_TYPE_OVERLAY:
+ fb_bits = INTEL_FRONTBUFFER_SPRITE(intel_plane->pipe);
+ break;
+ }
+
+ /*
+ * 'prepare' is never called when plane is being disabled, so
+ * we need to handle frontbuffer tracking here
+ */
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, fb_bits);
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ intel_begin_crtc_commit(crtc);
intel_plane->commit_plane(plane, &state);
- intel_runtime_pm_put(dev_priv);
+ intel_finish_crtc_commit(crtc);
if (fb != old_fb && old_fb) {
if (intel_crtc->active)
@@ -12012,6 +12077,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
struct drm_rect *src = &state->src;
const struct drm_rect *clip = &state->clip;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int crtc_w, crtc_h;
unsigned stride;
int ret;
@@ -12027,7 +12093,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
/* if we want to turn off the cursor ignore width and height */
if (!obj)
- return 0;
+ goto finish;
/* Check for which cursor types we support */
crtc_w = drm_rect_width(&state->orig_dst);
@@ -12054,6 +12120,16 @@ intel_check_cursor_plane(struct drm_plane *plane,
}
mutex_unlock(&dev->struct_mutex);
+finish:
+ if (intel_crtc->active) {
+ if (intel_crtc->cursor_width !=
+ drm_rect_width(&state->orig_dst))
+ intel_crtc->atomic.update_wm = true;
+
+ intel_crtc->atomic.fb_bits |=
+ INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+ }
+
return ret;
}
@@ -12066,9 +12142,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
- struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
- enum pipe pipe = intel_crtc->pipe;
- unsigned old_width;
uint32_t addr;
plane->fb = state->base.fb;
@@ -12088,17 +12161,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
if (intel_crtc->cursor_bo == obj)
goto update;
- /*
- * 'prepare' is only called when fb != NULL; we still need to update
- * frontbuffer tracking for the 'disable' case here.
- */
- if (!obj) {
- mutex_lock(&dev->struct_mutex);
- i915_gem_track_fb(old_obj, NULL,
- INTEL_FRONTBUFFER_CURSOR(pipe));
- mutex_unlock(&dev->struct_mutex);
- }
-
if (!obj)
addr = 0;
else if (!INTEL_INFO(dev)->cursor_needs_physical)
@@ -12109,18 +12171,11 @@ intel_commit_cursor_plane(struct drm_plane *plane,
intel_crtc->cursor_addr = addr;
intel_crtc->cursor_bo = obj;
update:
- old_width = intel_crtc->cursor_width;
-
intel_crtc->cursor_width = drm_rect_width(&state->orig_dst);
intel_crtc->cursor_height = drm_rect_height(&state->orig_dst);
- if (intel_crtc->active) {
- if (old_width != intel_crtc->cursor_width)
- intel_update_watermarks(crtc);
+ if (intel_crtc->active)
intel_crtc_update_cursor(crtc, state->visible);
-
- intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe));
- }
}
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 588b618..a03bd72 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -251,6 +251,12 @@ struct intel_plane_state {
struct drm_rect orig_src;
struct drm_rect orig_dst;
bool visible;
+
+ /*
+ * used only for sprite planes to determine when to implicitly
+ * enable/disable the primary plane
+ */
+ bool hides_primary;
};
struct intel_plane_config {
@@ -415,6 +421,27 @@ struct skl_pipe_wm {
uint32_t linetime;
};
+/*
+ * Tracking of operations that need to be performed at the beginning/end of an
+ * atomic commit, outside the atomic section where interrupts are disabled.
+ * These are generally operations that grab mutexes or might otherwise sleep
+ * and thus can't be run with interrupts disabled.
+ */
+struct intel_crtc_atomic_commit {
+ /* Sleepable operations to perform before commit */
+ bool wait_for_flips;
+ bool disable_fbc;
+ bool pre_disable_primary;
+ bool update_wm;
+
+ /* Sleepable operations to perform after commit */
+ unsigned fb_bits;
+ bool wait_vblank;
+ bool update_fbc;
+ bool post_enable_primary;
+ unsigned update_sprite_watermarks;
+};
+
struct intel_crtc {
struct drm_crtc base;
enum pipe pipe;
@@ -468,6 +495,8 @@ struct intel_crtc {
int scanline_offset;
struct intel_mmio_flip mmio_flip;
+
+ struct intel_crtc_atomic_commit atomic;
};
struct intel_plane_wm_parameters {
@@ -1213,6 +1242,8 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data,
bool intel_pipe_update_start(struct intel_crtc *crtc,
uint32_t *start_vbl_count);
void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count);
+void intel_post_enable_primary(struct drm_crtc *crtc);
+void intel_pre_disable_primary(struct drm_crtc *crtc);
/* intel_tv.c */
void intel_tv_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c18e57d..6e649de 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -771,9 +771,8 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
* Avoid underruns when disabling the sprite.
* FIXME remove once watermark updates are done properly.
*/
- intel_wait_for_vblank(dev, pipe);
-
- intel_update_sprite_watermarks(plane, crtc, 0, 0, 0, false, false);
+ intel_crtc->atomic.wait_vblank = true;
+ intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane));
}
static int
@@ -976,12 +975,21 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
* Avoid underruns when disabling the sprite.
* FIXME remove once watermark updates are done properly.
*/
- intel_wait_for_vblank(dev, pipe);
-
- intel_update_sprite_watermarks(plane, crtc, 0, 0, 0, false, false);
+ intel_crtc->atomic.wait_vblank = true;
+ intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane));
}
-static void
+/**
+ * intel_post_enable_primary - Perform operations after enabling primary plane
+ * @crtc: the CRTC whose primary plane was just enabled
+ *
+ * Performs potentially sleeping operations that must be done after the primary
+ * plane is enabled, such as updating FBC and IPS. Note that this may be
+ * called due to an explicit primary plane update, or due to an implicit
+ * re-enable that is caused when a sprite plane is updated to no longer
+ * completely hide the primary plane.
+ */
+void
intel_post_enable_primary(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
@@ -1008,7 +1016,17 @@ intel_post_enable_primary(struct drm_crtc *crtc)
mutex_unlock(&dev->struct_mutex);
}
-static void
+/**
+ * intel_pre_disable_primary - Perform operations before disabling primary plane
+ * @crtc: the CRTC whose primary plane is to be disabled
+ *
+ * Performs potentially sleeping operations that must be done before the
+ * primary plane is enabled, such as updating FBC and IPS. Note that this may
+ * be called due to an explicit primary plane update, or due to an implicit
+ * disable that is caused when a sprite plane completely hides the primary
+ * plane.
+ */
+void
intel_pre_disable_primary(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
@@ -1113,7 +1131,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (!fb) {
state->visible = false;
- return 0;
+ goto finish;
}
/* Don't modify another pipe's plane */
@@ -1260,6 +1278,29 @@ intel_check_sprite_plane(struct drm_plane *plane,
dst->y1 = crtc_y;
dst->y2 = crtc_y + crtc_h;
+finish:
+ /*
+ * If the sprite is completely covering the primary plane,
+ * we can disable the primary and save power.
+ */
+ state->hides_primary = drm_rect_equals(dst, clip) &&
+ !colorkey_enabled(intel_plane);
+ WARN_ON(state->hides_primary && !state->visible && intel_crtc->active);
+
+ if (intel_crtc->active) {
+ if (intel_crtc->primary_enabled == state->hides_primary)
+ intel_crtc->atomic.wait_for_flips = true;
+
+ if (intel_crtc->primary_enabled && state->hides_primary)
+ intel_crtc->atomic.pre_disable_primary = true;
+
+ intel_crtc->atomic.fb_bits |=
+ INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
+
+ if (!intel_crtc->primary_enabled && !state->hides_primary)
+ intel_crtc->atomic.post_enable_primary = true;
+ }
+
return 0;
}
@@ -1267,37 +1308,14 @@ static void
intel_commit_sprite_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
- struct drm_device *dev = plane->dev;
struct drm_crtc *crtc = state->base.crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
- enum pipe pipe = intel_crtc->pipe;
struct drm_framebuffer *fb = state->base.fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
int crtc_x, crtc_y;
unsigned int crtc_w, crtc_h;
uint32_t src_x, src_y, src_w, src_h;
- struct drm_rect *dst = &state->dst;
- const struct drm_rect *clip = &state->clip;
- bool primary_enabled;
-
- /*
- * 'prepare' is never called when plane is being disabled, so we need
- * to handle frontbuffer tracking here
- */
- if (!fb) {
- mutex_lock(&dev->struct_mutex);
- i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
- INTEL_FRONTBUFFER_SPRITE(pipe));
- mutex_unlock(&dev->struct_mutex);
- }
-
- /*
- * If the sprite is completely covering the primary plane,
- * we can disable the primary and save power.
- */
- primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);
- WARN_ON(!primary_enabled && !state->visible && intel_crtc->active);
intel_plane->crtc_x = state->orig_dst.x1;
intel_plane->crtc_y = state->orig_dst.y1;
@@ -1310,15 +1328,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
intel_plane->obj = obj;
if (intel_crtc->active) {
- bool primary_was_enabled = intel_crtc->primary_enabled;
-
- intel_crtc->primary_enabled = primary_enabled;
-
- if (primary_was_enabled != primary_enabled)
- intel_crtc_wait_for_pending_flips(crtc);
-
- if (primary_was_enabled && !primary_enabled)
- intel_pre_disable_primary(crtc);
+ intel_crtc->primary_enabled = !state->hides_primary;
if (state->visible) {
crtc_x = state->dst.x1;
@@ -1335,12 +1345,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
} else {
intel_plane->disable_plane(plane, crtc);
}
-
-
- intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_SPRITE(pipe));
-
- if (!primary_was_enabled && primary_enabled)
- intel_post_enable_primary(crtc);
}
}
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 1/5] drm/i915: Refactor work that can sleep out of commit (v7)
2014-12-23 18:41 ` [PATCH 1/5] drm/i915: Refactor work that can sleep out of commit (v6) Matt Roper
@ 2014-12-24 15:59 ` Matt Roper
2015-01-09 12:09 ` Ander Conselvan de Oliveira
0 siblings, 1 reply; 12+ messages in thread
From: Matt Roper @ 2014-12-24 15:59 UTC (permalink / raw)
To: intel-gfx
Once we integrate our work into the atomic pipeline, plane commit
operations will need to happen with interrupts disabled, due to vblank
evasion. Our commit functions today include sleepable work, so those
operations need to be split out and run either before or after the
atomic register programming.
The solution here calculates which of those operations will need to be
performed during the 'check' phase and sets flags in an intel_crtc
sub-struct. New intel_begin_crtc_commit() and
intel_finish_crtc_commit() functions are added before and after the
actual register programming; these will eventually be called from the
atomic plane helper's .atomic_begin() and .atomic_end() entrypoints.
v2: Fix broken sprite code split
v3: Make the pre/post commit work crtc-based to match how we eventually
want this to be called from the atomic plane helpers.
v4: Some platforms that haven't had their watermark code reworked were
waiting for vblank, then calling update_sprite_watermarks in their
platform-specific disable code. These also need to be flagged out
of the critical section.
v5: Sprite plane test for primary show/hide should just set the flag to
wait for pending flips, not actually perform the wait. (Ander)
v6:
- Rebase onto latest di-nightly; picks up an important runtime PM fix.
- Handle 'wait_for_flips' flag in intel_begin_crtc_commit(). (Ander)
- Use wait_for_flips flag for primary plane update rather than
performing the wait in the check routine.
- Added kerneldoc to pre_disable/post_enable functions that are no
longer static. (Ander)
- 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.
v7:
- Check for fb != NULL when deciding whether the sprite plane hides the
primary plane during a sprite update. (PRTS)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 199 ++++++++++++++++++++++-------------
drivers/gpu/drm/i915/intel_drv.h | 31 ++++++
drivers/gpu/drm/i915/intel_sprite.c | 98 ++++++++---------
3 files changed, 209 insertions(+), 119 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a03955d..6bd44f3 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;
@@ -11737,7 +11738,11 @@ static int
intel_check_primary_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc = state->base.crtc;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_framebuffer *fb = state->base.fb;
struct drm_rect *dest = &state->dst;
struct drm_rect *src = &state->src;
@@ -11752,10 +11757,40 @@ intel_check_primary_plane(struct drm_plane *plane,
if (ret)
return ret;
- intel_crtc_wait_for_pending_flips(crtc);
- if (intel_crtc_has_pending_flip(crtc)) {
- DRM_ERROR("pipe is still busy with an old pageflip\n");
- return -EBUSY;
+ if (intel_crtc->active) {
+ intel_crtc->atomic.wait_for_flips = true;
+
+ /*
+ * FBC does not work on some platforms for rotated
+ * planes, so disable it when rotation is not 0 and
+ * update it when rotation is set back to 0.
+ *
+ * FIXME: This is redundant with the fbc update done in
+ * the primary plane enable function except that that
+ * one is done too late. We eventually need to unify
+ * this.
+ */
+ if (intel_crtc->primary_enabled &&
+ INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+ dev_priv->fbc.plane == intel_crtc->plane &&
+ intel_plane->rotation != BIT(DRM_ROTATE_0)) {
+ intel_crtc->atomic.disable_fbc = true;
+ }
+
+ if (state->visible) {
+ /*
+ * BDW signals flip done immediately if the plane
+ * is disabled, even if the plane enable is already
+ * armed to occur at the next vblank :(
+ */
+ if (IS_BROADWELL(dev) && !intel_crtc->primary_enabled)
+ intel_crtc->atomic.wait_vblank = true;
+ }
+
+ intel_crtc->atomic.fb_bits |=
+ INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
+
+ intel_crtc->atomic.update_fbc = true;
}
return 0;
@@ -11773,18 +11808,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_rect *src = &state->src;
- enum pipe pipe = intel_plane->pipe;
-
- if (!fb) {
- /*
- * 'prepare' is never called when plane is being disabled, so
- * we need to handle frontbuffer tracking here
- */
- mutex_lock(&dev->struct_mutex);
- i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
- INTEL_FRONTBUFFER_PRIMARY(pipe));
- mutex_unlock(&dev->struct_mutex);
- }
plane->fb = fb;
crtc->x = src->x1 >> 16;
@@ -11801,26 +11824,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
intel_plane->obj = obj;
if (intel_crtc->active) {
- /*
- * FBC does not work on some platforms for rotated
- * planes, so disable it when rotation is not 0 and
- * update it when rotation is set back to 0.
- *
- * FIXME: This is redundant with the fbc update done in
- * the primary plane enable function except that that
- * one is done too late. We eventually need to unify
- * this.
- */
- if (intel_crtc->primary_enabled &&
- INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
- dev_priv->fbc.plane == intel_crtc->plane &&
- intel_plane->rotation != BIT(DRM_ROTATE_0)) {
- intel_fbc_disable(dev);
- }
-
if (state->visible) {
- bool was_enabled = intel_crtc->primary_enabled;
-
/* FIXME: kill this fastboot hack */
intel_update_pipe_size(intel_crtc);
@@ -11828,14 +11832,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
dev_priv->display.update_primary_plane(crtc, plane->fb,
crtc->x, crtc->y);
-
- /*
- * BDW signals flip done immediately if the plane
- * is disabled, even if the plane enable is already
- * armed to occur at the next vblank :(
- */
- if (IS_BROADWELL(dev) && !was_enabled)
- intel_wait_for_vblank(dev, intel_crtc->pipe);
} else {
/*
* If clipping results in a non-visible primary plane,
@@ -11846,13 +11842,59 @@ intel_commit_primary_plane(struct drm_plane *plane,
*/
intel_disable_primary_hw_plane(plane, crtc);
}
+ }
+}
+
+static void intel_begin_crtc_commit(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
+ if (intel_crtc->atomic.wait_for_flips)
+ intel_crtc_wait_for_pending_flips(crtc);
+ if (intel_crtc->atomic.disable_fbc)
+ intel_fbc_disable(dev);
+
+ if (intel_crtc->atomic.pre_disable_primary)
+ intel_pre_disable_primary(crtc);
+
+ if (intel_crtc->atomic.update_wm)
+ intel_update_watermarks(crtc);
+
+ intel_runtime_pm_get(dev_priv);
+}
+
+static void intel_finish_crtc_commit(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_plane *p;
+
+ intel_runtime_pm_put(dev_priv);
+
+ if (intel_crtc->atomic.wait_vblank)
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+ intel_frontbuffer_flip(dev, intel_crtc->atomic.fb_bits);
+
+ if (intel_crtc->atomic.update_fbc) {
mutex_lock(&dev->struct_mutex);
intel_fbc_update(dev);
mutex_unlock(&dev->struct_mutex);
}
+
+ if (intel_crtc->atomic.post_enable_primary)
+ intel_post_enable_primary(crtc);
+
+ drm_for_each_legacy_plane(p, &dev->mode_config.plane_list)
+ if (intel_crtc->atomic.update_sprite_watermarks & drm_plane_index(p))
+ intel_update_sprite_watermarks(p, crtc, 0, 0, 0,
+ false, false);
+
+ memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic));
}
int
@@ -11863,9 +11905,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_w, uint32_t src_h)
{
struct drm_device *dev = plane->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_framebuffer *old_fb = plane->fb;
- struct intel_plane_state state;
+ struct intel_plane_state state = {{ 0 }};
struct intel_plane *intel_plane = to_intel_plane(plane);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int ret;
@@ -11903,9 +11944,33 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
return ret;
}
- intel_runtime_pm_get(dev_priv);
+ if (!state.base.fb) {
+ unsigned fb_bits = 0;
+
+ switch (plane->type) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ fb_bits = INTEL_FRONTBUFFER_PRIMARY(intel_plane->pipe);
+ break;
+ case DRM_PLANE_TYPE_CURSOR:
+ fb_bits = INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe);
+ break;
+ case DRM_PLANE_TYPE_OVERLAY:
+ fb_bits = INTEL_FRONTBUFFER_SPRITE(intel_plane->pipe);
+ break;
+ }
+
+ /*
+ * 'prepare' is never called when plane is being disabled, so
+ * we need to handle frontbuffer tracking here
+ */
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, fb_bits);
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ intel_begin_crtc_commit(crtc);
intel_plane->commit_plane(plane, &state);
- intel_runtime_pm_put(dev_priv);
+ intel_finish_crtc_commit(crtc);
if (fb != old_fb && old_fb) {
if (intel_crtc->active)
@@ -12012,6 +12077,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
struct drm_rect *src = &state->src;
const struct drm_rect *clip = &state->clip;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int crtc_w, crtc_h;
unsigned stride;
int ret;
@@ -12027,7 +12093,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
/* if we want to turn off the cursor ignore width and height */
if (!obj)
- return 0;
+ goto finish;
/* Check for which cursor types we support */
crtc_w = drm_rect_width(&state->orig_dst);
@@ -12054,6 +12120,16 @@ intel_check_cursor_plane(struct drm_plane *plane,
}
mutex_unlock(&dev->struct_mutex);
+finish:
+ if (intel_crtc->active) {
+ if (intel_crtc->cursor_width !=
+ drm_rect_width(&state->orig_dst))
+ intel_crtc->atomic.update_wm = true;
+
+ intel_crtc->atomic.fb_bits |=
+ INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+ }
+
return ret;
}
@@ -12066,9 +12142,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
- struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
- enum pipe pipe = intel_crtc->pipe;
- unsigned old_width;
uint32_t addr;
plane->fb = state->base.fb;
@@ -12088,17 +12161,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
if (intel_crtc->cursor_bo == obj)
goto update;
- /*
- * 'prepare' is only called when fb != NULL; we still need to update
- * frontbuffer tracking for the 'disable' case here.
- */
- if (!obj) {
- mutex_lock(&dev->struct_mutex);
- i915_gem_track_fb(old_obj, NULL,
- INTEL_FRONTBUFFER_CURSOR(pipe));
- mutex_unlock(&dev->struct_mutex);
- }
-
if (!obj)
addr = 0;
else if (!INTEL_INFO(dev)->cursor_needs_physical)
@@ -12109,18 +12171,11 @@ intel_commit_cursor_plane(struct drm_plane *plane,
intel_crtc->cursor_addr = addr;
intel_crtc->cursor_bo = obj;
update:
- old_width = intel_crtc->cursor_width;
-
intel_crtc->cursor_width = drm_rect_width(&state->orig_dst);
intel_crtc->cursor_height = drm_rect_height(&state->orig_dst);
- if (intel_crtc->active) {
- if (old_width != intel_crtc->cursor_width)
- intel_update_watermarks(crtc);
+ if (intel_crtc->active)
intel_crtc_update_cursor(crtc, state->visible);
-
- intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe));
- }
}
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 588b618..a03bd72 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -251,6 +251,12 @@ struct intel_plane_state {
struct drm_rect orig_src;
struct drm_rect orig_dst;
bool visible;
+
+ /*
+ * used only for sprite planes to determine when to implicitly
+ * enable/disable the primary plane
+ */
+ bool hides_primary;
};
struct intel_plane_config {
@@ -415,6 +421,27 @@ struct skl_pipe_wm {
uint32_t linetime;
};
+/*
+ * Tracking of operations that need to be performed at the beginning/end of an
+ * atomic commit, outside the atomic section where interrupts are disabled.
+ * These are generally operations that grab mutexes or might otherwise sleep
+ * and thus can't be run with interrupts disabled.
+ */
+struct intel_crtc_atomic_commit {
+ /* Sleepable operations to perform before commit */
+ bool wait_for_flips;
+ bool disable_fbc;
+ bool pre_disable_primary;
+ bool update_wm;
+
+ /* Sleepable operations to perform after commit */
+ unsigned fb_bits;
+ bool wait_vblank;
+ bool update_fbc;
+ bool post_enable_primary;
+ unsigned update_sprite_watermarks;
+};
+
struct intel_crtc {
struct drm_crtc base;
enum pipe pipe;
@@ -468,6 +495,8 @@ struct intel_crtc {
int scanline_offset;
struct intel_mmio_flip mmio_flip;
+
+ struct intel_crtc_atomic_commit atomic;
};
struct intel_plane_wm_parameters {
@@ -1213,6 +1242,8 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data,
bool intel_pipe_update_start(struct intel_crtc *crtc,
uint32_t *start_vbl_count);
void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count);
+void intel_post_enable_primary(struct drm_crtc *crtc);
+void intel_pre_disable_primary(struct drm_crtc *crtc);
/* intel_tv.c */
void intel_tv_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c18e57d..3e6e95f 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -771,9 +771,8 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
* Avoid underruns when disabling the sprite.
* FIXME remove once watermark updates are done properly.
*/
- intel_wait_for_vblank(dev, pipe);
-
- intel_update_sprite_watermarks(plane, crtc, 0, 0, 0, false, false);
+ intel_crtc->atomic.wait_vblank = true;
+ intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane));
}
static int
@@ -976,12 +975,21 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
* Avoid underruns when disabling the sprite.
* FIXME remove once watermark updates are done properly.
*/
- intel_wait_for_vblank(dev, pipe);
-
- intel_update_sprite_watermarks(plane, crtc, 0, 0, 0, false, false);
+ intel_crtc->atomic.wait_vblank = true;
+ intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane));
}
-static void
+/**
+ * intel_post_enable_primary - Perform operations after enabling primary plane
+ * @crtc: the CRTC whose primary plane was just enabled
+ *
+ * Performs potentially sleeping operations that must be done after the primary
+ * plane is enabled, such as updating FBC and IPS. Note that this may be
+ * called due to an explicit primary plane update, or due to an implicit
+ * re-enable that is caused when a sprite plane is updated to no longer
+ * completely hide the primary plane.
+ */
+void
intel_post_enable_primary(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
@@ -1008,7 +1016,17 @@ intel_post_enable_primary(struct drm_crtc *crtc)
mutex_unlock(&dev->struct_mutex);
}
-static void
+/**
+ * intel_pre_disable_primary - Perform operations before disabling primary plane
+ * @crtc: the CRTC whose primary plane is to be disabled
+ *
+ * Performs potentially sleeping operations that must be done before the
+ * primary plane is enabled, such as updating FBC and IPS. Note that this may
+ * be called due to an explicit primary plane update, or due to an implicit
+ * disable that is caused when a sprite plane completely hides the primary
+ * plane.
+ */
+void
intel_pre_disable_primary(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
@@ -1113,7 +1131,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (!fb) {
state->visible = false;
- return 0;
+ goto finish;
}
/* Don't modify another pipe's plane */
@@ -1260,6 +1278,29 @@ intel_check_sprite_plane(struct drm_plane *plane,
dst->y1 = crtc_y;
dst->y2 = crtc_y + crtc_h;
+finish:
+ /*
+ * If the sprite is completely covering the primary plane,
+ * we can disable the primary and save power.
+ */
+ state->hides_primary = fb != NULL && drm_rect_equals(dst, clip) &&
+ !colorkey_enabled(intel_plane);
+ WARN_ON(state->hides_primary && !state->visible && intel_crtc->active);
+
+ if (intel_crtc->active) {
+ if (intel_crtc->primary_enabled == state->hides_primary)
+ intel_crtc->atomic.wait_for_flips = true;
+
+ if (intel_crtc->primary_enabled && state->hides_primary)
+ intel_crtc->atomic.pre_disable_primary = true;
+
+ intel_crtc->atomic.fb_bits |=
+ INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
+
+ if (!intel_crtc->primary_enabled && !state->hides_primary)
+ intel_crtc->atomic.post_enable_primary = true;
+ }
+
return 0;
}
@@ -1267,37 +1308,14 @@ static void
intel_commit_sprite_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
- struct drm_device *dev = plane->dev;
struct drm_crtc *crtc = state->base.crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
- enum pipe pipe = intel_crtc->pipe;
struct drm_framebuffer *fb = state->base.fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
int crtc_x, crtc_y;
unsigned int crtc_w, crtc_h;
uint32_t src_x, src_y, src_w, src_h;
- struct drm_rect *dst = &state->dst;
- const struct drm_rect *clip = &state->clip;
- bool primary_enabled;
-
- /*
- * 'prepare' is never called when plane is being disabled, so we need
- * to handle frontbuffer tracking here
- */
- if (!fb) {
- mutex_lock(&dev->struct_mutex);
- i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
- INTEL_FRONTBUFFER_SPRITE(pipe));
- mutex_unlock(&dev->struct_mutex);
- }
-
- /*
- * If the sprite is completely covering the primary plane,
- * we can disable the primary and save power.
- */
- primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);
- WARN_ON(!primary_enabled && !state->visible && intel_crtc->active);
intel_plane->crtc_x = state->orig_dst.x1;
intel_plane->crtc_y = state->orig_dst.y1;
@@ -1310,15 +1328,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
intel_plane->obj = obj;
if (intel_crtc->active) {
- bool primary_was_enabled = intel_crtc->primary_enabled;
-
- intel_crtc->primary_enabled = primary_enabled;
-
- if (primary_was_enabled != primary_enabled)
- intel_crtc_wait_for_pending_flips(crtc);
-
- if (primary_was_enabled && !primary_enabled)
- intel_pre_disable_primary(crtc);
+ intel_crtc->primary_enabled = !state->hides_primary;
if (state->visible) {
crtc_x = state->dst.x1;
@@ -1335,12 +1345,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
} else {
intel_plane->disable_plane(plane, crtc);
}
-
-
- intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_SPRITE(pipe));
-
- if (!primary_was_enabled && primary_enabled)
- intel_post_enable_primary(crtc);
}
}
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/5] drm/i915: Refactor work that can sleep out of commit (v7)
2014-12-24 15:59 ` [PATCH 1/5] drm/i915: Refactor work that can sleep out of commit (v7) Matt Roper
@ 2015-01-09 12:09 ` Ander Conselvan de Oliveira
0 siblings, 0 replies; 12+ messages in thread
From: Ander Conselvan de Oliveira @ 2015-01-09 12:09 UTC (permalink / raw)
To: Matt Roper, intel-gfx
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
On 12/24/2014 05:59 PM, Matt Roper wrote:
> Once we integrate our work into the atomic pipeline, plane commit
> operations will need to happen with interrupts disabled, due to vblank
> evasion. Our commit functions today include sleepable work, so those
> operations need to be split out and run either before or after the
> atomic register programming.
>
> The solution here calculates which of those operations will need to be
> performed during the 'check' phase and sets flags in an intel_crtc
> sub-struct. New intel_begin_crtc_commit() and
> intel_finish_crtc_commit() functions are added before and after the
> actual register programming; these will eventually be called from the
> atomic plane helper's .atomic_begin() and .atomic_end() entrypoints.
>
> v2: Fix broken sprite code split
>
> v3: Make the pre/post commit work crtc-based to match how we eventually
> want this to be called from the atomic plane helpers.
>
> v4: Some platforms that haven't had their watermark code reworked were
> waiting for vblank, then calling update_sprite_watermarks in their
> platform-specific disable code. These also need to be flagged out
> of the critical section.
>
> v5: Sprite plane test for primary show/hide should just set the flag to
> wait for pending flips, not actually perform the wait. (Ander)
>
> v6:
> - Rebase onto latest di-nightly; picks up an important runtime PM fix.
> - Handle 'wait_for_flips' flag in intel_begin_crtc_commit(). (Ander)
> - Use wait_for_flips flag for primary plane update rather than
> performing the wait in the check routine.
> - Added kerneldoc to pre_disable/post_enable functions that are no
> longer static. (Ander)
> - 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.
>
> v7:
> - Check for fb != NULL when deciding whether the sprite plane hides the
> primary plane during a sprite update. (PRTS)
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 199 ++++++++++++++++++++++-------------
> drivers/gpu/drm/i915/intel_drv.h | 31 ++++++
> drivers/gpu/drm/i915/intel_sprite.c | 98 ++++++++---------
> 3 files changed, 209 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a03955d..6bd44f3 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;
> @@ -11737,7 +11738,11 @@ static int
> intel_check_primary_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> {
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *crtc = state->base.crtc;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_framebuffer *fb = state->base.fb;
> struct drm_rect *dest = &state->dst;
> struct drm_rect *src = &state->src;
> @@ -11752,10 +11757,40 @@ intel_check_primary_plane(struct drm_plane *plane,
> if (ret)
> return ret;
>
> - intel_crtc_wait_for_pending_flips(crtc);
> - if (intel_crtc_has_pending_flip(crtc)) {
> - DRM_ERROR("pipe is still busy with an old pageflip\n");
> - return -EBUSY;
> + if (intel_crtc->active) {
> + intel_crtc->atomic.wait_for_flips = true;
> +
> + /*
> + * FBC does not work on some platforms for rotated
> + * planes, so disable it when rotation is not 0 and
> + * update it when rotation is set back to 0.
> + *
> + * FIXME: This is redundant with the fbc update done in
> + * the primary plane enable function except that that
> + * one is done too late. We eventually need to unify
> + * this.
> + */
> + if (intel_crtc->primary_enabled &&
> + INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> + dev_priv->fbc.plane == intel_crtc->plane &&
> + intel_plane->rotation != BIT(DRM_ROTATE_0)) {
> + intel_crtc->atomic.disable_fbc = true;
> + }
> +
> + if (state->visible) {
> + /*
> + * BDW signals flip done immediately if the plane
> + * is disabled, even if the plane enable is already
> + * armed to occur at the next vblank :(
> + */
> + if (IS_BROADWELL(dev) && !intel_crtc->primary_enabled)
> + intel_crtc->atomic.wait_vblank = true;
> + }
> +
> + intel_crtc->atomic.fb_bits |=
> + INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
> +
> + intel_crtc->atomic.update_fbc = true;
> }
>
> return 0;
> @@ -11773,18 +11808,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_rect *src = &state->src;
> - enum pipe pipe = intel_plane->pipe;
> -
> - if (!fb) {
> - /*
> - * 'prepare' is never called when plane is being disabled, so
> - * we need to handle frontbuffer tracking here
> - */
> - mutex_lock(&dev->struct_mutex);
> - i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
> - INTEL_FRONTBUFFER_PRIMARY(pipe));
> - mutex_unlock(&dev->struct_mutex);
> - }
>
> plane->fb = fb;
> crtc->x = src->x1 >> 16;
> @@ -11801,26 +11824,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
> intel_plane->obj = obj;
>
> if (intel_crtc->active) {
> - /*
> - * FBC does not work on some platforms for rotated
> - * planes, so disable it when rotation is not 0 and
> - * update it when rotation is set back to 0.
> - *
> - * FIXME: This is redundant with the fbc update done in
> - * the primary plane enable function except that that
> - * one is done too late. We eventually need to unify
> - * this.
> - */
> - if (intel_crtc->primary_enabled &&
> - INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> - dev_priv->fbc.plane == intel_crtc->plane &&
> - intel_plane->rotation != BIT(DRM_ROTATE_0)) {
> - intel_fbc_disable(dev);
> - }
> -
> if (state->visible) {
> - bool was_enabled = intel_crtc->primary_enabled;
> -
> /* FIXME: kill this fastboot hack */
> intel_update_pipe_size(intel_crtc);
>
> @@ -11828,14 +11832,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
>
> dev_priv->display.update_primary_plane(crtc, plane->fb,
> crtc->x, crtc->y);
> -
> - /*
> - * BDW signals flip done immediately if the plane
> - * is disabled, even if the plane enable is already
> - * armed to occur at the next vblank :(
> - */
> - if (IS_BROADWELL(dev) && !was_enabled)
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> } else {
> /*
> * If clipping results in a non-visible primary plane,
> @@ -11846,13 +11842,59 @@ intel_commit_primary_plane(struct drm_plane *plane,
> */
> intel_disable_primary_hw_plane(plane, crtc);
> }
> + }
> +}
> +
> +static void intel_begin_crtc_commit(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
> + if (intel_crtc->atomic.wait_for_flips)
> + intel_crtc_wait_for_pending_flips(crtc);
>
> + if (intel_crtc->atomic.disable_fbc)
> + intel_fbc_disable(dev);
> +
> + if (intel_crtc->atomic.pre_disable_primary)
> + intel_pre_disable_primary(crtc);
> +
> + if (intel_crtc->atomic.update_wm)
> + intel_update_watermarks(crtc);
> +
> + intel_runtime_pm_get(dev_priv);
> +}
> +
> +static void intel_finish_crtc_commit(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_plane *p;
> +
> + intel_runtime_pm_put(dev_priv);
> +
> + if (intel_crtc->atomic.wait_vblank)
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> + intel_frontbuffer_flip(dev, intel_crtc->atomic.fb_bits);
> +
> + if (intel_crtc->atomic.update_fbc) {
> mutex_lock(&dev->struct_mutex);
> intel_fbc_update(dev);
> mutex_unlock(&dev->struct_mutex);
> }
> +
> + if (intel_crtc->atomic.post_enable_primary)
> + intel_post_enable_primary(crtc);
> +
> + drm_for_each_legacy_plane(p, &dev->mode_config.plane_list)
> + if (intel_crtc->atomic.update_sprite_watermarks & drm_plane_index(p))
> + intel_update_sprite_watermarks(p, crtc, 0, 0, 0,
> + false, false);
> +
> + memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic));
> }
>
> int
> @@ -11863,9 +11905,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> uint32_t src_w, uint32_t src_h)
> {
> struct drm_device *dev = plane->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_framebuffer *old_fb = plane->fb;
> - struct intel_plane_state state;
> + struct intel_plane_state state = {{ 0 }};
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int ret;
> @@ -11903,9 +11944,33 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> return ret;
> }
>
> - intel_runtime_pm_get(dev_priv);
> + if (!state.base.fb) {
> + unsigned fb_bits = 0;
> +
> + switch (plane->type) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + fb_bits = INTEL_FRONTBUFFER_PRIMARY(intel_plane->pipe);
> + break;
> + case DRM_PLANE_TYPE_CURSOR:
> + fb_bits = INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe);
> + break;
> + case DRM_PLANE_TYPE_OVERLAY:
> + fb_bits = INTEL_FRONTBUFFER_SPRITE(intel_plane->pipe);
> + break;
> + }
> +
> + /*
> + * 'prepare' is never called when plane is being disabled, so
> + * we need to handle frontbuffer tracking here
> + */
> + mutex_lock(&dev->struct_mutex);
> + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, fb_bits);
> + mutex_unlock(&dev->struct_mutex);
> + }
> +
> + intel_begin_crtc_commit(crtc);
> intel_plane->commit_plane(plane, &state);
> - intel_runtime_pm_put(dev_priv);
> + intel_finish_crtc_commit(crtc);
>
> if (fb != old_fb && old_fb) {
> if (intel_crtc->active)
> @@ -12012,6 +12077,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
> struct drm_rect *src = &state->src;
> const struct drm_rect *clip = &state->clip;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int crtc_w, crtc_h;
> unsigned stride;
> int ret;
> @@ -12027,7 +12093,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
>
> /* if we want to turn off the cursor ignore width and height */
> if (!obj)
> - return 0;
> + goto finish;
>
> /* Check for which cursor types we support */
> crtc_w = drm_rect_width(&state->orig_dst);
> @@ -12054,6 +12120,16 @@ intel_check_cursor_plane(struct drm_plane *plane,
> }
> mutex_unlock(&dev->struct_mutex);
>
> +finish:
> + if (intel_crtc->active) {
> + if (intel_crtc->cursor_width !=
> + drm_rect_width(&state->orig_dst))
> + intel_crtc->atomic.update_wm = true;
> +
> + intel_crtc->atomic.fb_bits |=
> + INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
> + }
> +
> return ret;
> }
>
> @@ -12066,9 +12142,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
> - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
> - enum pipe pipe = intel_crtc->pipe;
> - unsigned old_width;
> uint32_t addr;
>
> plane->fb = state->base.fb;
> @@ -12088,17 +12161,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
> if (intel_crtc->cursor_bo == obj)
> goto update;
>
> - /*
> - * 'prepare' is only called when fb != NULL; we still need to update
> - * frontbuffer tracking for the 'disable' case here.
> - */
> - if (!obj) {
> - mutex_lock(&dev->struct_mutex);
> - i915_gem_track_fb(old_obj, NULL,
> - INTEL_FRONTBUFFER_CURSOR(pipe));
> - mutex_unlock(&dev->struct_mutex);
> - }
> -
> if (!obj)
> addr = 0;
> else if (!INTEL_INFO(dev)->cursor_needs_physical)
> @@ -12109,18 +12171,11 @@ intel_commit_cursor_plane(struct drm_plane *plane,
> intel_crtc->cursor_addr = addr;
> intel_crtc->cursor_bo = obj;
> update:
> - old_width = intel_crtc->cursor_width;
> -
> intel_crtc->cursor_width = drm_rect_width(&state->orig_dst);
> intel_crtc->cursor_height = drm_rect_height(&state->orig_dst);
>
> - if (intel_crtc->active) {
> - if (old_width != intel_crtc->cursor_width)
> - intel_update_watermarks(crtc);
> + if (intel_crtc->active)
> intel_crtc_update_cursor(crtc, state->visible);
> -
> - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe));
> - }
> }
>
> static const struct drm_plane_funcs intel_cursor_plane_funcs = {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 588b618..a03bd72 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -251,6 +251,12 @@ struct intel_plane_state {
> struct drm_rect orig_src;
> struct drm_rect orig_dst;
> bool visible;
> +
> + /*
> + * used only for sprite planes to determine when to implicitly
> + * enable/disable the primary plane
> + */
> + bool hides_primary;
> };
>
> struct intel_plane_config {
> @@ -415,6 +421,27 @@ struct skl_pipe_wm {
> uint32_t linetime;
> };
>
> +/*
> + * Tracking of operations that need to be performed at the beginning/end of an
> + * atomic commit, outside the atomic section where interrupts are disabled.
> + * These are generally operations that grab mutexes or might otherwise sleep
> + * and thus can't be run with interrupts disabled.
> + */
> +struct intel_crtc_atomic_commit {
> + /* Sleepable operations to perform before commit */
> + bool wait_for_flips;
> + bool disable_fbc;
> + bool pre_disable_primary;
> + bool update_wm;
> +
> + /* Sleepable operations to perform after commit */
> + unsigned fb_bits;
> + bool wait_vblank;
> + bool update_fbc;
> + bool post_enable_primary;
> + unsigned update_sprite_watermarks;
> +};
> +
> struct intel_crtc {
> struct drm_crtc base;
> enum pipe pipe;
> @@ -468,6 +495,8 @@ struct intel_crtc {
>
> int scanline_offset;
> struct intel_mmio_flip mmio_flip;
> +
> + struct intel_crtc_atomic_commit atomic;
> };
>
> struct intel_plane_wm_parameters {
> @@ -1213,6 +1242,8 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data,
> bool intel_pipe_update_start(struct intel_crtc *crtc,
> uint32_t *start_vbl_count);
> void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count);
> +void intel_post_enable_primary(struct drm_crtc *crtc);
> +void intel_pre_disable_primary(struct drm_crtc *crtc);
>
> /* intel_tv.c */
> void intel_tv_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index c18e57d..3e6e95f 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -771,9 +771,8 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
> * Avoid underruns when disabling the sprite.
> * FIXME remove once watermark updates are done properly.
> */
> - intel_wait_for_vblank(dev, pipe);
> -
> - intel_update_sprite_watermarks(plane, crtc, 0, 0, 0, false, false);
> + intel_crtc->atomic.wait_vblank = true;
> + intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane));
> }
>
> static int
> @@ -976,12 +975,21 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
> * Avoid underruns when disabling the sprite.
> * FIXME remove once watermark updates are done properly.
> */
> - intel_wait_for_vblank(dev, pipe);
> -
> - intel_update_sprite_watermarks(plane, crtc, 0, 0, 0, false, false);
> + intel_crtc->atomic.wait_vblank = true;
> + intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane));
> }
>
> -static void
> +/**
> + * intel_post_enable_primary - Perform operations after enabling primary plane
> + * @crtc: the CRTC whose primary plane was just enabled
> + *
> + * Performs potentially sleeping operations that must be done after the primary
> + * plane is enabled, such as updating FBC and IPS. Note that this may be
> + * called due to an explicit primary plane update, or due to an implicit
> + * re-enable that is caused when a sprite plane is updated to no longer
> + * completely hide the primary plane.
> + */
> +void
> intel_post_enable_primary(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> @@ -1008,7 +1016,17 @@ intel_post_enable_primary(struct drm_crtc *crtc)
> mutex_unlock(&dev->struct_mutex);
> }
>
> -static void
> +/**
> + * intel_pre_disable_primary - Perform operations before disabling primary plane
> + * @crtc: the CRTC whose primary plane is to be disabled
> + *
> + * Performs potentially sleeping operations that must be done before the
> + * primary plane is enabled, such as updating FBC and IPS. Note that this may
> + * be called due to an explicit primary plane update, or due to an implicit
> + * disable that is caused when a sprite plane completely hides the primary
> + * plane.
> + */
> +void
> intel_pre_disable_primary(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> @@ -1113,7 +1131,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
>
> if (!fb) {
> state->visible = false;
> - return 0;
> + goto finish;
> }
>
> /* Don't modify another pipe's plane */
> @@ -1260,6 +1278,29 @@ intel_check_sprite_plane(struct drm_plane *plane,
> dst->y1 = crtc_y;
> dst->y2 = crtc_y + crtc_h;
>
> +finish:
> + /*
> + * If the sprite is completely covering the primary plane,
> + * we can disable the primary and save power.
> + */
> + state->hides_primary = fb != NULL && drm_rect_equals(dst, clip) &&
> + !colorkey_enabled(intel_plane);
> + WARN_ON(state->hides_primary && !state->visible && intel_crtc->active);
> +
> + if (intel_crtc->active) {
> + if (intel_crtc->primary_enabled == state->hides_primary)
> + intel_crtc->atomic.wait_for_flips = true;
> +
> + if (intel_crtc->primary_enabled && state->hides_primary)
> + intel_crtc->atomic.pre_disable_primary = true;
> +
> + intel_crtc->atomic.fb_bits |=
> + INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
> +
> + if (!intel_crtc->primary_enabled && !state->hides_primary)
> + intel_crtc->atomic.post_enable_primary = true;
> + }
> +
> return 0;
> }
>
> @@ -1267,37 +1308,14 @@ static void
> intel_commit_sprite_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> {
> - struct drm_device *dev = plane->dev;
> struct drm_crtc *crtc = state->base.crtc;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_plane *intel_plane = to_intel_plane(plane);
> - enum pipe pipe = intel_crtc->pipe;
> struct drm_framebuffer *fb = state->base.fb;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> int crtc_x, crtc_y;
> unsigned int crtc_w, crtc_h;
> uint32_t src_x, src_y, src_w, src_h;
> - struct drm_rect *dst = &state->dst;
> - const struct drm_rect *clip = &state->clip;
> - bool primary_enabled;
> -
> - /*
> - * 'prepare' is never called when plane is being disabled, so we need
> - * to handle frontbuffer tracking here
> - */
> - if (!fb) {
> - mutex_lock(&dev->struct_mutex);
> - i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
> - INTEL_FRONTBUFFER_SPRITE(pipe));
> - mutex_unlock(&dev->struct_mutex);
> - }
> -
> - /*
> - * If the sprite is completely covering the primary plane,
> - * we can disable the primary and save power.
> - */
> - primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);
> - WARN_ON(!primary_enabled && !state->visible && intel_crtc->active);
>
> intel_plane->crtc_x = state->orig_dst.x1;
> intel_plane->crtc_y = state->orig_dst.y1;
> @@ -1310,15 +1328,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> intel_plane->obj = obj;
>
> if (intel_crtc->active) {
> - bool primary_was_enabled = intel_crtc->primary_enabled;
> -
> - intel_crtc->primary_enabled = primary_enabled;
> -
> - if (primary_was_enabled != primary_enabled)
> - intel_crtc_wait_for_pending_flips(crtc);
> -
> - if (primary_was_enabled && !primary_enabled)
> - intel_pre_disable_primary(crtc);
> + intel_crtc->primary_enabled = !state->hides_primary;
>
> if (state->visible) {
> crtc_x = state->dst.x1;
> @@ -1335,12 +1345,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> } else {
> intel_plane->disable_plane(plane, crtc);
> }
> -
> -
> - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_SPRITE(pipe));
> -
> - if (!primary_was_enabled && primary_enabled)
> - intel_post_enable_primary(crtc);
> }
> }
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/5] drm/i915: Move vblank evasion to commit (v4)
2014-12-23 18:41 [PATCH 0/5] i915 atomic plane helper conversion (v5) Matt Roper
2014-12-23 18:41 ` [PATCH 1/5] drm/i915: Refactor work that can sleep out of commit (v6) Matt Roper
@ 2014-12-23 18:41 ` Matt Roper
2015-01-09 12:20 ` Ander Conselvan de Oliveira
2014-12-23 18:41 ` [PATCH 3/5] drm/i915: Clarify sprite plane function names (v4) Matt Roper
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Matt Roper @ 2014-12-23 18:41 UTC (permalink / raw)
To: intel-gfx
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.
v4:
- Equivalent to v2; v3 change is now squashed into an earlier patch
of the series. (Ander).
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 10 +++++++++
drivers/gpu/drm/i915/intel_drv.h | 4 ++++
drivers/gpu/drm/i915/intel_sprite.c | 42 ------------------------------------
3 files changed, 14 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6bd44f3..8d6e710 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11864,6 +11864,12 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
intel_update_watermarks(crtc);
intel_runtime_pm_get(dev_priv);
+
+ /* 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)
@@ -11873,6 +11879,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);
+
intel_runtime_pm_put(dev_priv);
if (intel_crtc->atomic.wait_vblank)
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 6e649de..18be827 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, sprscale = 0;
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(SPRCTL(pipe));
@@ -711,8 +695,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
}
}
- atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
-
intel_update_primary_plane(intel_crtc);
I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
@@ -735,9 +717,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
i915_gem_obj_ggtt_offset(obj) + 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
@@ -748,10 +727,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
struct intel_plane *intel_plane = to_intel_plane(plane);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_plane->pipe;
- u32 start_vbl_count;
- bool atomic_update;
-
- atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
intel_update_primary_plane(intel_crtc);
@@ -764,9 +739,6 @@ ivb_disable_plane(struct drm_plane *plane, 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);
-
/*
* Avoid underruns when disabling the sprite.
* FIXME remove once watermark updates are done properly.
@@ -845,8 +817,6 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
unsigned long dvssurf_offset, linear_offset;
u32 dvscntr, dvsscale;
int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
- u32 start_vbl_count;
- bool atomic_update;
dvscntr = I915_READ(DVSCNTR(pipe));
@@ -921,8 +891,6 @@ ilk_update_plane(struct drm_plane *plane, 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);
I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
@@ -940,9 +908,6 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
i915_gem_obj_ggtt_offset(obj) + dvssurf_offset);
intel_flush_primary_plane(dev_priv, intel_crtc->plane);
-
- if (atomic_update)
- intel_pipe_update_end(intel_crtc, start_vbl_count);
}
static void
@@ -953,10 +918,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
struct intel_plane *intel_plane = to_intel_plane(plane);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_plane->pipe;
- u32 start_vbl_count;
- bool atomic_update;
-
- atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
intel_update_primary_plane(intel_crtc);
@@ -968,9 +929,6 @@ ilk_disable_plane(struct drm_plane *plane, 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);
-
/*
* Avoid underruns when disabling the sprite.
* FIXME remove once watermark updates are done properly.
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/5] drm/i915: Move vblank evasion to commit (v4)
2014-12-23 18:41 ` [PATCH 2/5] drm/i915: Move vblank evasion to commit (v4) Matt Roper
@ 2015-01-09 12:20 ` Ander Conselvan de Oliveira
0 siblings, 0 replies; 12+ messages in thread
From: Ander Conselvan de Oliveira @ 2015-01-09 12:20 UTC (permalink / raw)
To: Matt Roper, intel-gfx
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
On 12/23/2014 08:41 PM, 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.
>
> v4:
> - Equivalent to v2; v3 change is now squashed into an earlier patch
> of the series. (Ander).
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 10 +++++++++
> drivers/gpu/drm/i915/intel_drv.h | 4 ++++
> drivers/gpu/drm/i915/intel_sprite.c | 42 ------------------------------------
> 3 files changed, 14 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6bd44f3..8d6e710 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11864,6 +11864,12 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
> intel_update_watermarks(crtc);
>
> intel_runtime_pm_get(dev_priv);
> +
> + /* 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)
> @@ -11873,6 +11879,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);
> +
> intel_runtime_pm_put(dev_priv);
>
> if (intel_crtc->atomic.wait_vblank)
> 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 6e649de..18be827 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, sprscale = 0;
> 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(SPRCTL(pipe));
>
> @@ -711,8 +695,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> }
> }
>
> - atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
> -
> intel_update_primary_plane(intel_crtc);
>
> I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
> @@ -735,9 +717,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> i915_gem_obj_ggtt_offset(obj) + 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
> @@ -748,10 +727,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int pipe = intel_plane->pipe;
> - u32 start_vbl_count;
> - bool atomic_update;
> -
> - atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
>
> intel_update_primary_plane(intel_crtc);
>
> @@ -764,9 +739,6 @@ ivb_disable_plane(struct drm_plane *plane, 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);
> -
> /*
> * Avoid underruns when disabling the sprite.
> * FIXME remove once watermark updates are done properly.
> @@ -845,8 +817,6 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> unsigned long dvssurf_offset, linear_offset;
> u32 dvscntr, dvsscale;
> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> - u32 start_vbl_count;
> - bool atomic_update;
>
> dvscntr = I915_READ(DVSCNTR(pipe));
>
> @@ -921,8 +891,6 @@ ilk_update_plane(struct drm_plane *plane, 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);
>
> I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
> @@ -940,9 +908,6 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> i915_gem_obj_ggtt_offset(obj) + dvssurf_offset);
>
> intel_flush_primary_plane(dev_priv, intel_crtc->plane);
> -
> - if (atomic_update)
> - intel_pipe_update_end(intel_crtc, start_vbl_count);
> }
>
> static void
> @@ -953,10 +918,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int pipe = intel_plane->pipe;
> - u32 start_vbl_count;
> - bool atomic_update;
> -
> - atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
>
> intel_update_primary_plane(intel_crtc);
>
> @@ -968,9 +929,6 @@ ilk_disable_plane(struct drm_plane *plane, 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);
> -
> /*
> * Avoid underruns when disabling the sprite.
> * FIXME remove once watermark updates are done properly.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/5] drm/i915: Clarify sprite plane function names (v4)
2014-12-23 18:41 [PATCH 0/5] i915 atomic plane helper conversion (v5) Matt Roper
2014-12-23 18:41 ` [PATCH 1/5] drm/i915: Refactor work that can sleep out of commit (v6) Matt Roper
2014-12-23 18:41 ` [PATCH 2/5] drm/i915: Move vblank evasion to commit (v4) Matt Roper
@ 2014-12-23 18:41 ` Matt Roper
2015-01-09 12:24 ` Ander Conselvan de Oliveira
2014-12-23 18:41 ` [PATCH 4/5] drm/i915: Move to atomic plane helpers (v9) Matt Roper
2014-12-23 18:41 ` [PATCH 5/5] drm/i915: Drop unused position fields (v2) Matt Roper
4 siblings, 1 reply; 12+ messages in thread
From: Matt Roper @ 2014-12-23 18:41 UTC (permalink / raw)
To: intel-gfx
A few of the sprite-related function names in i915 are very similar
(e.g., intel_enable_planes() vs intel_crtc_enable_planes()) and don't
make it clear whether they only operate on sprite planes, or whether
they also apply to all universal plane types. Rename a few functions to
be more consistent with our function naming for primary/cursor planes or
to clarify that they apply specifically to sprite planes:
- s/intel_disable_planes/intel_disable_sprite_planes/
- s/intel_enable_planes/intel_enable_sprite_planes/
Also, drop the sprite-specific intel_destroy_plane() and just use
the type-agnostic intel_plane_destroy() function. The extra 'disable'
call that intel_destroy_plane() did is unnecessary since the plane will
already be disabled due to framebuffer destruction by the point it gets
called.
v2: Earlier consolidation patches have reduced the number of functions
we need to rename here.
v3: Also rename intel_plane_funcs vtable to intel_sprite_plane_funcs
for consistency with primary/cursor. (Ander)
v4: Convert comment for intel_plane_destroy() to kerneldoc now that it
is no longer a static function. (Ander)
Reviewed-by(v1): Bob Paauwe <bob.j.paauwe@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++------
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_sprite.c | 14 +++-----------
3 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8d6e710..4bb1a37 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4037,7 +4037,7 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
}
}
-static void intel_enable_planes(struct drm_crtc *crtc)
+static void intel_enable_sprite_planes(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
enum pipe pipe = to_intel_crtc(crtc)->pipe;
@@ -4051,7 +4051,7 @@ static void intel_enable_planes(struct drm_crtc *crtc)
}
}
-static void intel_disable_planes(struct drm_crtc *crtc)
+static void intel_disable_sprite_planes(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
enum pipe pipe = to_intel_crtc(crtc)->pipe;
@@ -4195,7 +4195,7 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc)
int pipe = intel_crtc->pipe;
intel_enable_primary_hw_plane(crtc->primary, crtc);
- intel_enable_planes(crtc);
+ intel_enable_sprite_planes(crtc);
intel_crtc_update_cursor(crtc, true);
intel_crtc_dpms_overlay(intel_crtc, true);
@@ -4230,7 +4230,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
intel_crtc_dpms_overlay(intel_crtc, false);
intel_crtc_update_cursor(crtc, false);
- intel_disable_planes(crtc);
+ intel_disable_sprite_planes(crtc);
intel_disable_primary_hw_plane(crtc->primary, crtc);
/*
@@ -12012,8 +12012,14 @@ intel_disable_plane(struct drm_plane *plane)
0, 0, 0, 0, 0, 0, 0, 0);
}
-/* Common destruction function for both primary and cursor planes */
-static void intel_plane_destroy(struct drm_plane *plane)
+/**
+ * intel_plane_destroy - destroy a plane
+ * @plane: plane to destroy
+ *
+ * Common destruction function for all types of planes (primary, cursor,
+ * sprite).
+ */
+void intel_plane_destroy(struct drm_plane *plane)
{
struct intel_plane *intel_plane = to_intel_plane(plane);
drm_plane_cleanup(plane);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1934156..2523315 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1053,6 +1053,7 @@ int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h);
int intel_disable_plane(struct drm_plane *plane);
+void intel_plane_destroy(struct drm_plane *plane);
/* intel_dp_mst.c */
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 18be827..537fff25 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1306,14 +1306,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
}
}
-static void intel_destroy_plane(struct drm_plane *plane)
-{
- struct intel_plane *intel_plane = to_intel_plane(plane);
- intel_disable_plane(plane);
- drm_plane_cleanup(plane);
- kfree(intel_plane);
-}
-
int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
@@ -1413,10 +1405,10 @@ int intel_plane_restore(struct drm_plane *plane)
intel_plane->src_w, intel_plane->src_h);
}
-static const struct drm_plane_funcs intel_plane_funcs = {
+static const struct drm_plane_funcs intel_sprite_plane_funcs = {
.update_plane = intel_update_plane,
.disable_plane = intel_disable_plane,
- .destroy = intel_destroy_plane,
+ .destroy = intel_plane_destroy,
.set_property = intel_plane_set_property,
};
@@ -1553,7 +1545,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
intel_plane->commit_plane = intel_commit_sprite_plane;
possible_crtcs = (1 << pipe);
ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
- &intel_plane_funcs,
+ &intel_sprite_plane_funcs,
plane_formats, num_plane_formats,
DRM_PLANE_TYPE_OVERLAY);
if (ret) {
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] drm/i915: Clarify sprite plane function names (v4)
2014-12-23 18:41 ` [PATCH 3/5] drm/i915: Clarify sprite plane function names (v4) Matt Roper
@ 2015-01-09 12:24 ` Ander Conselvan de Oliveira
0 siblings, 0 replies; 12+ messages in thread
From: Ander Conselvan de Oliveira @ 2015-01-09 12:24 UTC (permalink / raw)
To: intel-gfx
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
On 12/23/2014 08:41 PM, Matt Roper wrote:
> A few of the sprite-related function names in i915 are very similar
> (e.g., intel_enable_planes() vs intel_crtc_enable_planes()) and don't
> make it clear whether they only operate on sprite planes, or whether
> they also apply to all universal plane types. Rename a few functions to
> be more consistent with our function naming for primary/cursor planes or
> to clarify that they apply specifically to sprite planes:
>
> - s/intel_disable_planes/intel_disable_sprite_planes/
> - s/intel_enable_planes/intel_enable_sprite_planes/
>
> Also, drop the sprite-specific intel_destroy_plane() and just use
> the type-agnostic intel_plane_destroy() function. The extra 'disable'
> call that intel_destroy_plane() did is unnecessary since the plane will
> already be disabled due to framebuffer destruction by the point it gets
> called.
>
> v2: Earlier consolidation patches have reduced the number of functions
> we need to rename here.
>
> v3: Also rename intel_plane_funcs vtable to intel_sprite_plane_funcs
> for consistency with primary/cursor. (Ander)
>
> v4: Convert comment for intel_plane_destroy() to kerneldoc now that it
> is no longer a static function. (Ander)
>
> Reviewed-by(v1): Bob Paauwe <bob.j.paauwe@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++------
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_sprite.c | 14 +++-----------
> 3 files changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8d6e710..4bb1a37 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4037,7 +4037,7 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
> }
> }
>
> -static void intel_enable_planes(struct drm_crtc *crtc)
> +static void intel_enable_sprite_planes(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> enum pipe pipe = to_intel_crtc(crtc)->pipe;
> @@ -4051,7 +4051,7 @@ static void intel_enable_planes(struct drm_crtc *crtc)
> }
> }
>
> -static void intel_disable_planes(struct drm_crtc *crtc)
> +static void intel_disable_sprite_planes(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> enum pipe pipe = to_intel_crtc(crtc)->pipe;
> @@ -4195,7 +4195,7 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc)
> int pipe = intel_crtc->pipe;
>
> intel_enable_primary_hw_plane(crtc->primary, crtc);
> - intel_enable_planes(crtc);
> + intel_enable_sprite_planes(crtc);
> intel_crtc_update_cursor(crtc, true);
> intel_crtc_dpms_overlay(intel_crtc, true);
>
> @@ -4230,7 +4230,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
>
> intel_crtc_dpms_overlay(intel_crtc, false);
> intel_crtc_update_cursor(crtc, false);
> - intel_disable_planes(crtc);
> + intel_disable_sprite_planes(crtc);
> intel_disable_primary_hw_plane(crtc->primary, crtc);
>
> /*
> @@ -12012,8 +12012,14 @@ intel_disable_plane(struct drm_plane *plane)
> 0, 0, 0, 0, 0, 0, 0, 0);
> }
>
> -/* Common destruction function for both primary and cursor planes */
> -static void intel_plane_destroy(struct drm_plane *plane)
> +/**
> + * intel_plane_destroy - destroy a plane
> + * @plane: plane to destroy
> + *
> + * Common destruction function for all types of planes (primary, cursor,
> + * sprite).
> + */
> +void intel_plane_destroy(struct drm_plane *plane)
> {
> struct intel_plane *intel_plane = to_intel_plane(plane);
> drm_plane_cleanup(plane);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1934156..2523315 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1053,6 +1053,7 @@ int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h);
> int intel_disable_plane(struct drm_plane *plane);
> +void intel_plane_destroy(struct drm_plane *plane);
>
> /* intel_dp_mst.c */
> int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 18be827..537fff25 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1306,14 +1306,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> }
> }
>
> -static void intel_destroy_plane(struct drm_plane *plane)
> -{
> - struct intel_plane *intel_plane = to_intel_plane(plane);
> - intel_disable_plane(plane);
> - drm_plane_cleanup(plane);
> - kfree(intel_plane);
> -}
> -
> int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
> struct drm_file *file_priv)
> {
> @@ -1413,10 +1405,10 @@ int intel_plane_restore(struct drm_plane *plane)
> intel_plane->src_w, intel_plane->src_h);
> }
>
> -static const struct drm_plane_funcs intel_plane_funcs = {
> +static const struct drm_plane_funcs intel_sprite_plane_funcs = {
> .update_plane = intel_update_plane,
> .disable_plane = intel_disable_plane,
> - .destroy = intel_destroy_plane,
> + .destroy = intel_plane_destroy,
> .set_property = intel_plane_set_property,
> };
>
> @@ -1553,7 +1545,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> intel_plane->commit_plane = intel_commit_sprite_plane;
> possible_crtcs = (1 << pipe);
> ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
> - &intel_plane_funcs,
> + &intel_sprite_plane_funcs,
> plane_formats, num_plane_formats,
> DRM_PLANE_TYPE_OVERLAY);
> if (ret) {
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] drm/i915: Move to atomic plane helpers (v9)
2014-12-23 18:41 [PATCH 0/5] i915 atomic plane helper conversion (v5) Matt Roper
` (2 preceding siblings ...)
2014-12-23 18:41 ` [PATCH 3/5] drm/i915: Clarify sprite plane function names (v4) Matt Roper
@ 2014-12-23 18:41 ` Matt Roper
2015-01-09 13:02 ` Ander Conselvan de Oliveira
2014-12-23 18:41 ` [PATCH 5/5] drm/i915: Drop unused position fields (v2) Matt Roper
4 siblings, 1 reply; 12+ messages in thread
From: Matt Roper @ 2014-12-23 18:41 UTC (permalink / raw)
To: intel-gfx
Switch plane handling to use the atomic plane helpers. This means that
rather than provide our own implementations of .update_plane() and
.disable_plane(), we expose the lower-level check/prepare/commit/cleanup
entrypoints and let the DRM core implement update/disable for us using
those entrypoints.
The other main change that falls out of this patch is that our
drm_plane's will now always have a valid plane->state that contains the
relevant plane state (initial state is allocated at plane creation).
The base drm_plane_state pointed to holds the requested source/dest
coordinates, and the subclassed intel_plane_state holds the adjusted
values that our driver actually uses.
v2:
- Renamed file from intel_atomic.c to intel_atomic_plane.c (Daniel)
- Fix a copy/paste comment mistake (Bob)
v3:
- Use prepare/cleanup functions that we've already factored out
- Use newly refactored pre_commit/commit/post_commit to avoid sleeping
during vblank evasion
v4:
- Rebase to latest di-nightly requires adding an 'old_state' parameter
to atomic_update;
v5:
- Must have botched a rebase somewhere and lost some work. Restore
state 'dirty' flag to let begin/end code know which planes to
run the pre_commit/post_commit hooks for. This would have actually
shown up as broken in the next commit rather than this one.
v6:
- Squash kerneldoc patch into this one.
- Previous patches have now already taken care of most of the
infrastructure that used to be in this patch. All we're adding here
now is some thin wrappers.
v7:
- Check return of intel_plane_duplicate_state() for allocation
failures.
v8:
- Drop unused drm_plane_state -> intel_plane_state cast. (Ander)
- Squash in actual transition to plane helpers. Significant
refactoring earlier in the patchset has made the combined
prep+transition much easier to swallow than it was in earlier
iterations. (Ander)
v9:
- s/track_fbs/disabled_planes/ in the atomic crtc flags. The only fb's
we need to update frontbuffer tracking for are those on a plane about
to be disabled (since the atomic helpers never call prepare_fb() when
disabling a plane), so the new name more accurately describes what
we're actually tracking.
Testcase: igt/kms_plane
Testcase: igt/kms_universal_plane
Testcase: igt/kms_cursor_crc
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
Documentation/DocBook/drm.tmpl | 5 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/intel_atomic_plane.c | 151 ++++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 251 +++++++++++++-----------------
drivers/gpu/drm/i915/intel_drv.h | 10 +-
drivers/gpu/drm/i915/intel_sprite.c | 50 ++++--
6 files changed, 303 insertions(+), 165 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_atomic_plane.c
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 3b2571e..169d205 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -4018,6 +4018,11 @@ int num_ioctls;</synopsis>
</para>
</sect2>
<sect2>
+ <title>Atomic Plane Helpers</title>
+!Pdrivers/gpu/drm/i915/intel_atomic_plane.c atomic plane helpers
+!Idrivers/gpu/drm/i915/intel_atomic_plane.c
+ </sect2>
+ <sect2>
<title>Output Probing</title>
<para>
This section covers output probing and related infrastructure like the
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 1849ffa..16e3dc3 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -66,6 +66,7 @@ i915-y += dvo_ch7017.o \
dvo_ns2501.o \
dvo_sil164.o \
dvo_tfp410.o \
+ intel_atomic_plane.o \
intel_crt.o \
intel_ddi.o \
intel_dp.o \
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
new file mode 100644
index 0000000..e7425d6c
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -0,0 +1,151 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+/**
+ * DOC: atomic plane helper support
+ *
+ * The functions here are used by the atomic plane helper functions to
+ * implement legacy plane updates (i.e., drm_plane->update_plane() and
+ * drm_plane->disable_plane()). This allows plane updates to use the
+ * atomic state infrastructure and perform plane updates as separate
+ * prepare/check/commit/cleanup steps.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_plane_helper.h>
+#include "intel_drv.h"
+
+/**
+ * intel_plane_duplicate_state - duplicate plane state
+ * @plane: drm plane
+ *
+ * Allocates and returns a copy of the plane state (both common and
+ * Intel-specific) for the specified plane.
+ *
+ * Returns: The newly allocated plane state, or NULL or failure.
+ */
+struct drm_plane_state *
+intel_plane_duplicate_state(struct drm_plane *plane)
+{
+ struct intel_plane_state *state;
+
+ if (plane->state)
+ state = kmemdup(plane->state, sizeof(*state), GFP_KERNEL);
+ else
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
+
+ if (!state)
+ return NULL;
+
+ if (state->base.fb)
+ drm_framebuffer_reference(state->base.fb);
+
+ return &state->base;
+}
+
+/**
+ * intel_plane_destroy_state - destroy plane state
+ * @plane: drm plane
+ *
+ * Destroys the plane state (both common and Intel-specific) for the
+ * specified plane.
+ */
+void
+intel_plane_destroy_state(struct drm_plane *plane,
+ struct drm_plane_state *state)
+{
+ drm_atomic_helper_plane_destroy_state(plane, state);
+}
+
+static int intel_plane_atomic_check(struct drm_plane *plane,
+ struct drm_plane_state *state)
+{
+ struct drm_crtc *crtc = state->crtc;
+ struct intel_crtc *intel_crtc;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct intel_plane_state *intel_state = to_intel_plane_state(state);
+
+ crtc = crtc ? crtc : plane->crtc;
+ intel_crtc = to_intel_crtc(crtc);
+
+ /*
+ * The original src/dest coordinates are stored in state->base, but
+ * we want to keep another copy internal to our driver that we can
+ * clip/modify ourselves.
+ */
+ intel_state->src.x1 = state->src_x;
+ intel_state->src.y1 = state->src_y;
+ intel_state->src.x2 = state->src_x + state->src_w;
+ intel_state->src.y2 = state->src_y + state->src_h;
+ intel_state->dst.x1 = state->crtc_x;
+ intel_state->dst.y1 = state->crtc_y;
+ intel_state->dst.x2 = state->crtc_x + state->crtc_w;
+ intel_state->dst.y2 = state->crtc_y + state->crtc_h;
+
+ /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
+ intel_state->clip.x1 = 0;
+ intel_state->clip.y1 = 0;
+ intel_state->clip.x2 =
+ intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
+ intel_state->clip.y2 =
+ intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;
+
+ /*
+ * Disabling a plane is always okay; we just need to update
+ * fb tracking in a special way since cleanup_fb() won't
+ * get called by the plane helpers.
+ */
+ if (state->fb == NULL && plane->state->fb != NULL) {
+ /*
+ * 'prepare' is never called when plane is being disabled, so
+ * we need to handle frontbuffer tracking as a special case
+ */
+ intel_crtc->atomic.disabled_planes |=
+ (1 << drm_plane_index(plane));
+ }
+
+ return intel_plane->check_plane(plane, intel_state);
+}
+
+static void intel_plane_atomic_update(struct drm_plane *plane,
+ struct drm_plane_state *old_state)
+{
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct intel_plane_state *intel_state =
+ to_intel_plane_state(plane->state);
+
+ /* Don't disable an already disabled plane */
+ if (!plane->state->fb && !old_state->fb)
+ return;
+
+ intel_plane->commit_plane(plane, intel_state);
+}
+
+const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
+ .prepare_fb = intel_prepare_plane_fb,
+ .cleanup_fb = intel_cleanup_plane_fb,
+ .atomic_check = intel_plane_atomic_check,
+ .atomic_update = intel_plane_atomic_update,
+};
+
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4bb1a37..e7b51db 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -98,6 +98,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
const struct intel_crtc_config *pipe_config);
static void chv_prepare_pll(struct intel_crtc *crtc,
const struct intel_crtc_config *pipe_config);
+static void intel_begin_crtc_commit(struct drm_crtc *crtc);
+static void intel_finish_crtc_commit(struct drm_crtc *crtc);
static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe)
{
@@ -9794,6 +9796,8 @@ out_hang:
static struct drm_crtc_helper_funcs intel_helper_funcs = {
.mode_set_base_atomic = intel_pipe_set_base_atomic,
.load_lut = intel_crtc_load_lut,
+ .atomic_begin = intel_begin_crtc_commit,
+ .atomic_flush = intel_finish_crtc_commit,
};
/**
@@ -11674,7 +11678,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
unsigned frontbuffer_bits = 0;
int ret = 0;
- if (WARN_ON(fb == plane->fb || !obj))
+ if (!obj)
return 0;
switch (plane->type) {
@@ -11741,7 +11745,7 @@ intel_check_primary_plane(struct drm_plane *plane,
struct drm_device *dev = plane->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc = state->base.crtc;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc *intel_crtc;
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_framebuffer *fb = state->base.fb;
struct drm_rect *dest = &state->dst;
@@ -11749,6 +11753,9 @@ intel_check_primary_plane(struct drm_plane *plane,
const struct drm_rect *clip = &state->clip;
int ret;
+ crtc = crtc ? crtc : plane->crtc;
+ intel_crtc = to_intel_crtc(crtc);
+
ret = drm_plane_helper_check_update(plane, crtc, fb,
src, dest, clip,
DRM_PLANE_HELPER_NO_SCALING,
@@ -11804,23 +11811,26 @@ intel_commit_primary_plane(struct drm_plane *plane,
struct drm_framebuffer *fb = state->base.fb;
struct drm_device *dev = plane->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc *intel_crtc;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_rect *src = &state->src;
+ crtc = crtc ? crtc : plane->crtc;
+ intel_crtc = to_intel_crtc(crtc);
+
plane->fb = fb;
crtc->x = src->x1 >> 16;
crtc->y = src->y1 >> 16;
- intel_plane->crtc_x = state->orig_dst.x1;
- intel_plane->crtc_y = state->orig_dst.y1;
- intel_plane->crtc_w = drm_rect_width(&state->orig_dst);
- intel_plane->crtc_h = drm_rect_height(&state->orig_dst);
- intel_plane->src_x = state->orig_src.x1;
- intel_plane->src_y = state->orig_src.y1;
- intel_plane->src_w = drm_rect_width(&state->orig_src);
- intel_plane->src_h = drm_rect_height(&state->orig_src);
+ intel_plane->crtc_x = state->base.crtc_x;
+ intel_plane->crtc_y = state->base.crtc_y;
+ intel_plane->crtc_w = state->base.crtc_w;
+ intel_plane->crtc_h = state->base.crtc_h;
+ intel_plane->src_x = state->base.src_x;
+ intel_plane->src_y = state->base.src_y;
+ intel_plane->src_w = state->base.src_w;
+ intel_plane->src_h = state->base.src_h;
intel_plane->obj = obj;
if (intel_crtc->active) {
@@ -11850,6 +11860,33 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_plane *intel_plane;
+ struct drm_plane *p;
+ unsigned fb_bits = 0;
+
+ /* Track fb's for any planes being disabled */
+ list_for_each_entry(p, &dev->mode_config.plane_list, head) {
+ intel_plane = to_intel_plane(p);
+
+ if (intel_crtc->atomic.disabled_planes &
+ (1 << drm_plane_index(p))) {
+ switch (p->type) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ fb_bits = INTEL_FRONTBUFFER_PRIMARY(intel_plane->pipe);
+ break;
+ case DRM_PLANE_TYPE_CURSOR:
+ fb_bits = INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe);
+ break;
+ case DRM_PLANE_TYPE_OVERLAY:
+ fb_bits = INTEL_FRONTBUFFER_SPRITE(intel_plane->pipe);
+ break;
+ }
+
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(p->fb), NULL, fb_bits);
+ mutex_unlock(&dev->struct_mutex);
+ }
+ }
if (intel_crtc->atomic.wait_for_flips)
intel_crtc_wait_for_pending_flips(crtc);
@@ -11907,111 +11944,6 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc)
memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic));
}
-int
-intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
- struct drm_framebuffer *fb, int crtc_x, int crtc_y,
- unsigned int crtc_w, unsigned int crtc_h,
- uint32_t src_x, uint32_t src_y,
- uint32_t src_w, uint32_t src_h)
-{
- struct drm_device *dev = plane->dev;
- struct drm_framebuffer *old_fb = plane->fb;
- struct intel_plane_state state = {{ 0 }};
- struct intel_plane *intel_plane = to_intel_plane(plane);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int ret;
-
- state.base.crtc = crtc ? crtc : plane->crtc;
- state.base.fb = fb;
-
- /* sample coordinates in 16.16 fixed point */
- state.src.x1 = src_x;
- state.src.x2 = src_x + src_w;
- state.src.y1 = src_y;
- state.src.y2 = src_y + src_h;
-
- /* integer pixels */
- state.dst.x1 = crtc_x;
- state.dst.x2 = crtc_x + crtc_w;
- state.dst.y1 = crtc_y;
- state.dst.y2 = crtc_y + crtc_h;
-
- state.clip.x1 = 0;
- state.clip.y1 = 0;
- state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
- state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;
-
- state.orig_src = state.src;
- state.orig_dst = state.dst;
-
- ret = intel_plane->check_plane(plane, &state);
- if (ret)
- return ret;
-
- if (fb != old_fb && fb) {
- ret = intel_prepare_plane_fb(plane, fb);
- if (ret)
- return ret;
- }
-
- if (!state.base.fb) {
- unsigned fb_bits = 0;
-
- switch (plane->type) {
- case DRM_PLANE_TYPE_PRIMARY:
- fb_bits = INTEL_FRONTBUFFER_PRIMARY(intel_plane->pipe);
- break;
- case DRM_PLANE_TYPE_CURSOR:
- fb_bits = INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe);
- break;
- case DRM_PLANE_TYPE_OVERLAY:
- fb_bits = INTEL_FRONTBUFFER_SPRITE(intel_plane->pipe);
- break;
- }
-
- /*
- * 'prepare' is never called when plane is being disabled, so
- * we need to handle frontbuffer tracking here
- */
- mutex_lock(&dev->struct_mutex);
- i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, fb_bits);
- mutex_unlock(&dev->struct_mutex);
- }
-
- intel_begin_crtc_commit(crtc);
- intel_plane->commit_plane(plane, &state);
- intel_finish_crtc_commit(crtc);
-
- if (fb != old_fb && old_fb) {
- if (intel_crtc->active)
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- intel_cleanup_plane_fb(plane, old_fb);
- }
-
- plane->fb = fb;
-
- return 0;
-}
-
-/**
- * intel_disable_plane - disable a plane
- * @plane: plane to disable
- *
- * General disable handler for all plane types.
- */
-int
-intel_disable_plane(struct drm_plane *plane)
-{
- if (!plane->fb)
- return 0;
-
- if (WARN_ON(!plane->crtc))
- return -EINVAL;
-
- return plane->funcs->update_plane(plane, plane->crtc, NULL,
- 0, 0, 0, 0, 0, 0, 0, 0);
-}
-
/**
* intel_plane_destroy - destroy a plane
* @plane: plane to destroy
@@ -12022,15 +11954,19 @@ intel_disable_plane(struct drm_plane *plane)
void intel_plane_destroy(struct drm_plane *plane)
{
struct intel_plane *intel_plane = to_intel_plane(plane);
+ intel_plane_destroy_state(plane, plane->state);
drm_plane_cleanup(plane);
kfree(intel_plane);
}
static const struct drm_plane_funcs intel_primary_plane_funcs = {
- .update_plane = intel_update_plane,
- .disable_plane = intel_disable_plane,
+ .update_plane = drm_plane_helper_update,
+ .disable_plane = drm_plane_helper_disable,
.destroy = intel_plane_destroy,
- .set_property = intel_plane_set_property
+ .set_property = intel_plane_set_property,
+ .atomic_duplicate_state = intel_plane_duplicate_state,
+ .atomic_destroy_state = intel_plane_destroy_state,
+
};
static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
@@ -12044,6 +11980,12 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
if (primary == NULL)
return NULL;
+ primary->base.state = intel_plane_duplicate_state(&primary->base);
+ if (primary->base.state == NULL) {
+ kfree(primary);
+ return NULL;
+ }
+
primary->can_scale = false;
primary->max_downscale = 1;
primary->pipe = pipe;
@@ -12079,6 +12021,8 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
primary->rotation);
}
+ drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
+
return &primary->base;
}
@@ -12087,17 +12031,19 @@ intel_check_cursor_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
struct drm_crtc *crtc = state->base.crtc;
- struct drm_device *dev = crtc->dev;
+ struct drm_device *dev = plane->dev;
struct drm_framebuffer *fb = state->base.fb;
struct drm_rect *dest = &state->dst;
struct drm_rect *src = &state->src;
const struct drm_rect *clip = &state->clip;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int crtc_w, crtc_h;
+ struct intel_crtc *intel_crtc;
unsigned stride;
int ret;
+ crtc = crtc ? crtc : plane->crtc;
+ intel_crtc = to_intel_crtc(crtc);
+
ret = drm_plane_helper_check_update(plane, crtc, fb,
src, dest, clip,
DRM_PLANE_HELPER_NO_SCALING,
@@ -12112,15 +12058,14 @@ intel_check_cursor_plane(struct drm_plane *plane,
goto finish;
/* Check for which cursor types we support */
- crtc_w = drm_rect_width(&state->orig_dst);
- crtc_h = drm_rect_height(&state->orig_dst);
- if (!cursor_size_ok(dev, crtc_w, crtc_h)) {
- DRM_DEBUG("Cursor dimension not supported\n");
+ if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) {
+ DRM_DEBUG("Cursor dimension %dx%d not supported\n",
+ state->base.crtc_w, state->base.crtc_h);
return -EINVAL;
}
- stride = roundup_pow_of_two(crtc_w) * 4;
- if (obj->base.size < stride * crtc_h) {
+ stride = roundup_pow_of_two(state->base.crtc_w) * 4;
+ if (obj->base.size < stride * state->base.crtc_h) {
DRM_DEBUG_KMS("buffer is too small\n");
return -ENOMEM;
}
@@ -12138,8 +12083,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
finish:
if (intel_crtc->active) {
- if (intel_crtc->cursor_width !=
- drm_rect_width(&state->orig_dst))
+ if (intel_crtc->cursor_width != state->base.crtc_w)
intel_crtc->atomic.update_wm = true;
intel_crtc->atomic.fb_bits |=
@@ -12154,24 +12098,27 @@ intel_commit_cursor_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
struct drm_crtc *crtc = state->base.crtc;
- struct drm_device *dev = crtc->dev;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_device *dev = plane->dev;
+ struct intel_crtc *intel_crtc;
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
uint32_t addr;
+ crtc = crtc ? crtc : plane->crtc;
+ intel_crtc = to_intel_crtc(crtc);
+
plane->fb = state->base.fb;
- crtc->cursor_x = state->orig_dst.x1;
- crtc->cursor_y = state->orig_dst.y1;
-
- intel_plane->crtc_x = state->orig_dst.x1;
- intel_plane->crtc_y = state->orig_dst.y1;
- intel_plane->crtc_w = drm_rect_width(&state->orig_dst);
- intel_plane->crtc_h = drm_rect_height(&state->orig_dst);
- intel_plane->src_x = state->orig_src.x1;
- intel_plane->src_y = state->orig_src.y1;
- intel_plane->src_w = drm_rect_width(&state->orig_src);
- intel_plane->src_h = drm_rect_height(&state->orig_src);
+ crtc->cursor_x = state->base.crtc_x;
+ crtc->cursor_y = state->base.crtc_y;
+
+ intel_plane->crtc_x = state->base.crtc_x;
+ intel_plane->crtc_y = state->base.crtc_y;
+ intel_plane->crtc_w = state->base.crtc_w;
+ intel_plane->crtc_h = state->base.crtc_h;
+ intel_plane->src_x = state->base.src_x;
+ intel_plane->src_y = state->base.src_y;
+ intel_plane->src_w = state->base.src_w;
+ intel_plane->src_h = state->base.src_h;
intel_plane->obj = obj;
if (intel_crtc->cursor_bo == obj)
@@ -12187,18 +12134,20 @@ intel_commit_cursor_plane(struct drm_plane *plane,
intel_crtc->cursor_addr = addr;
intel_crtc->cursor_bo = obj;
update:
- intel_crtc->cursor_width = drm_rect_width(&state->orig_dst);
- intel_crtc->cursor_height = drm_rect_height(&state->orig_dst);
+ intel_crtc->cursor_width = state->base.crtc_w;
+ intel_crtc->cursor_height = state->base.crtc_h;
if (intel_crtc->active)
intel_crtc_update_cursor(crtc, state->visible);
}
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
- .update_plane = intel_update_plane,
- .disable_plane = intel_disable_plane,
+ .update_plane = drm_plane_helper_update,
+ .disable_plane = drm_plane_helper_disable,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property,
+ .atomic_duplicate_state = intel_plane_duplicate_state,
+ .atomic_destroy_state = intel_plane_destroy_state,
};
static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
@@ -12210,6 +12159,12 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
if (cursor == NULL)
return NULL;
+ cursor->base.state = intel_plane_duplicate_state(&cursor->base);
+ if (cursor->base.state == NULL) {
+ kfree(cursor);
+ return NULL;
+ }
+
cursor->can_scale = false;
cursor->max_downscale = 1;
cursor->pipe = pipe;
@@ -12236,6 +12191,8 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
cursor->rotation);
}
+ drm_plane_helper_add(&cursor->base, &intel_plane_helper_funcs);
+
return &cursor->base;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2523315..604bd22 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -248,8 +248,6 @@ struct intel_plane_state {
struct drm_rect src;
struct drm_rect dst;
struct drm_rect clip;
- struct drm_rect orig_src;
- struct drm_rect orig_dst;
bool visible;
/*
@@ -437,6 +435,7 @@ struct intel_crtc_atomic_commit {
bool disable_fbc;
bool pre_disable_primary;
bool update_wm;
+ unsigned disabled_planes;
/* Sleepable operations to perform after commit */
unsigned fb_bits;
@@ -575,6 +574,7 @@ struct cxsr_latency {
#define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
#define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
#define to_intel_plane(x) container_of(x, struct intel_plane, base)
+#define to_intel_plane_state(x) container_of(x, struct intel_plane_state, base)
#define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL)
struct intel_hdmi {
@@ -1253,4 +1253,10 @@ void intel_pre_disable_primary(struct drm_crtc *crtc);
/* intel_tv.c */
void intel_tv_init(struct drm_device *dev);
+/* intel_atomic.c */
+struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane);
+void intel_plane_destroy_state(struct drm_plane *plane,
+ struct drm_plane_state *state);
+extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
+
#endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 537fff25..16e7c96 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -33,6 +33,7 @@
#include <drm/drm_crtc.h>
#include <drm/drm_fourcc.h>
#include <drm/drm_rect.h>
+#include <drm/drm_plane_helper.h>
#include "intel_drv.h"
#include <drm/i915_drm.h>
#include "i915_drv.h"
@@ -1081,12 +1082,13 @@ intel_check_sprite_plane(struct drm_plane *plane,
uint32_t src_x, src_y, src_w, src_h;
struct drm_rect *src = &state->src;
struct drm_rect *dst = &state->dst;
- struct drm_rect *orig_src = &state->orig_src;
const struct drm_rect *clip = &state->clip;
int hscale, vscale;
int max_scale, min_scale;
int pixel_size;
+ intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
+
if (!fb) {
state->visible = false;
goto finish;
@@ -1167,10 +1169,10 @@ intel_check_sprite_plane(struct drm_plane *plane,
intel_plane->rotation);
/* sanity check to make sure the src viewport wasn't enlarged */
- WARN_ON(src->x1 < (int) orig_src->x1 ||
- src->y1 < (int) orig_src->y1 ||
- src->x2 > (int) orig_src->x2 ||
- src->y2 > (int) orig_src->y2);
+ WARN_ON(src->x1 < (int) state->base.src_x ||
+ src->y1 < (int) state->base.src_y ||
+ src->x2 > (int) state->base.src_x + state->base.src_w ||
+ src->y2 > (int) state->base.src_y + state->base.src_h);
/*
* Hardware doesn't handle subpixel coordinates.
@@ -1267,7 +1269,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
struct drm_crtc *crtc = state->base.crtc;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc *intel_crtc;
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_framebuffer *fb = state->base.fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
@@ -1275,14 +1277,19 @@ intel_commit_sprite_plane(struct drm_plane *plane,
unsigned int crtc_w, crtc_h;
uint32_t src_x, src_y, src_w, src_h;
- intel_plane->crtc_x = state->orig_dst.x1;
- intel_plane->crtc_y = state->orig_dst.y1;
- intel_plane->crtc_w = drm_rect_width(&state->orig_dst);
- intel_plane->crtc_h = drm_rect_height(&state->orig_dst);
- intel_plane->src_x = state->orig_src.x1;
- intel_plane->src_y = state->orig_src.y1;
- intel_plane->src_w = drm_rect_width(&state->orig_src);
- intel_plane->src_h = drm_rect_height(&state->orig_src);
+ intel_plane->crtc_x = state->base.crtc_x;
+ intel_plane->crtc_y = state->base.crtc_y;
+ intel_plane->crtc_w = state->base.crtc_w;
+ intel_plane->crtc_h = state->base.crtc_h;
+ intel_plane->src_x = state->base.src_x;
+ intel_plane->src_y = state->base.src_y;
+ intel_plane->src_w = state->base.src_w;
+ intel_plane->src_h = state->base.src_h;
+
+ crtc = crtc ? crtc : plane->crtc;
+ intel_crtc = to_intel_crtc(crtc);
+
+ plane->fb = state->base.fb;
intel_plane->obj = obj;
if (intel_crtc->active) {
@@ -1406,10 +1413,12 @@ int intel_plane_restore(struct drm_plane *plane)
}
static const struct drm_plane_funcs intel_sprite_plane_funcs = {
- .update_plane = intel_update_plane,
- .disable_plane = intel_disable_plane,
+ .update_plane = drm_plane_helper_update,
+ .disable_plane = drm_plane_helper_disable,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property,
+ .atomic_duplicate_state = intel_plane_duplicate_state,
+ .atomic_destroy_state = intel_plane_destroy_state,
};
static uint32_t ilk_plane_formats[] = {
@@ -1471,6 +1480,13 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
if (!intel_plane)
return -ENOMEM;
+ intel_plane->base.state =
+ intel_plane_duplicate_state(&intel_plane->base);
+ if (intel_plane->base.state == NULL) {
+ kfree(intel_plane);
+ return -ENOMEM;
+ }
+
switch (INTEL_INFO(dev)->gen) {
case 5:
case 6:
@@ -1564,6 +1580,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
dev->mode_config.rotation_property,
intel_plane->rotation);
+ drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
+
out:
return ret;
}
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/5] drm/i915: Move to atomic plane helpers (v9)
2014-12-23 18:41 ` [PATCH 4/5] drm/i915: Move to atomic plane helpers (v9) Matt Roper
@ 2015-01-09 13:02 ` Ander Conselvan de Oliveira
0 siblings, 0 replies; 12+ messages in thread
From: Ander Conselvan de Oliveira @ 2015-01-09 13:02 UTC (permalink / raw)
To: Matt Roper, intel-gfx
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
On 12/23/2014 08:41 PM, Matt Roper wrote:
> Switch plane handling to use the atomic plane helpers. This means that
> rather than provide our own implementations of .update_plane() and
> .disable_plane(), we expose the lower-level check/prepare/commit/cleanup
> entrypoints and let the DRM core implement update/disable for us using
> those entrypoints.
>
> The other main change that falls out of this patch is that our
> drm_plane's will now always have a valid plane->state that contains the
> relevant plane state (initial state is allocated at plane creation).
> The base drm_plane_state pointed to holds the requested source/dest
> coordinates, and the subclassed intel_plane_state holds the adjusted
> values that our driver actually uses.
>
> v2:
> - Renamed file from intel_atomic.c to intel_atomic_plane.c (Daniel)
> - Fix a copy/paste comment mistake (Bob)
>
> v3:
> - Use prepare/cleanup functions that we've already factored out
> - Use newly refactored pre_commit/commit/post_commit to avoid sleeping
> during vblank evasion
>
> v4:
> - Rebase to latest di-nightly requires adding an 'old_state' parameter
> to atomic_update;
>
> v5:
> - Must have botched a rebase somewhere and lost some work. Restore
> state 'dirty' flag to let begin/end code know which planes to
> run the pre_commit/post_commit hooks for. This would have actually
> shown up as broken in the next commit rather than this one.
>
> v6:
> - Squash kerneldoc patch into this one.
> - Previous patches have now already taken care of most of the
> infrastructure that used to be in this patch. All we're adding here
> now is some thin wrappers.
>
> v7:
> - Check return of intel_plane_duplicate_state() for allocation
> failures.
>
> v8:
> - Drop unused drm_plane_state -> intel_plane_state cast. (Ander)
> - Squash in actual transition to plane helpers. Significant
> refactoring earlier in the patchset has made the combined
> prep+transition much easier to swallow than it was in earlier
> iterations. (Ander)
>
> v9:
> - s/track_fbs/disabled_planes/ in the atomic crtc flags. The only fb's
> we need to update frontbuffer tracking for are those on a plane about
> to be disabled (since the atomic helpers never call prepare_fb() when
> disabling a plane), so the new name more accurately describes what
> we're actually tracking.
>
> Testcase: igt/kms_plane
> Testcase: igt/kms_universal_plane
> Testcase: igt/kms_cursor_crc
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> Documentation/DocBook/drm.tmpl | 5 +
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/intel_atomic_plane.c | 151 ++++++++++++++++++
> drivers/gpu/drm/i915/intel_display.c | 251 +++++++++++++-----------------
> drivers/gpu/drm/i915/intel_drv.h | 10 +-
> drivers/gpu/drm/i915/intel_sprite.c | 50 ++++--
> 6 files changed, 303 insertions(+), 165 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_atomic_plane.c
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 3b2571e..169d205 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -4018,6 +4018,11 @@ int num_ioctls;</synopsis>
> </para>
> </sect2>
> <sect2>
> + <title>Atomic Plane Helpers</title>
> +!Pdrivers/gpu/drm/i915/intel_atomic_plane.c atomic plane helpers
> +!Idrivers/gpu/drm/i915/intel_atomic_plane.c
> + </sect2>
> + <sect2>
> <title>Output Probing</title>
> <para>
> This section covers output probing and related infrastructure like the
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 1849ffa..16e3dc3 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -66,6 +66,7 @@ i915-y += dvo_ch7017.o \
> dvo_ns2501.o \
> dvo_sil164.o \
> dvo_tfp410.o \
> + intel_atomic_plane.o \
> intel_crt.o \
> intel_ddi.o \
> intel_dp.o \
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> new file mode 100644
> index 0000000..e7425d6c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -0,0 +1,151 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +/**
> + * DOC: atomic plane helper support
> + *
> + * The functions here are used by the atomic plane helper functions to
> + * implement legacy plane updates (i.e., drm_plane->update_plane() and
> + * drm_plane->disable_plane()). This allows plane updates to use the
> + * atomic state infrastructure and perform plane updates as separate
> + * prepare/check/commit/cleanup steps.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_plane_helper.h>
> +#include "intel_drv.h"
> +
> +/**
> + * intel_plane_duplicate_state - duplicate plane state
> + * @plane: drm plane
> + *
> + * Allocates and returns a copy of the plane state (both common and
> + * Intel-specific) for the specified plane.
> + *
> + * Returns: The newly allocated plane state, or NULL or failure.
> + */
> +struct drm_plane_state *
> +intel_plane_duplicate_state(struct drm_plane *plane)
> +{
> + struct intel_plane_state *state;
> +
> + if (plane->state)
> + state = kmemdup(plane->state, sizeof(*state), GFP_KERNEL);
> + else
> + state = kzalloc(sizeof(*state), GFP_KERNEL);
> +
> + if (!state)
> + return NULL;
> +
> + if (state->base.fb)
> + drm_framebuffer_reference(state->base.fb);
> +
> + return &state->base;
> +}
> +
> +/**
> + * intel_plane_destroy_state - destroy plane state
> + * @plane: drm plane
> + *
> + * Destroys the plane state (both common and Intel-specific) for the
> + * specified plane.
> + */
> +void
> +intel_plane_destroy_state(struct drm_plane *plane,
> + struct drm_plane_state *state)
> +{
> + drm_atomic_helper_plane_destroy_state(plane, state);
> +}
> +
> +static int intel_plane_atomic_check(struct drm_plane *plane,
> + struct drm_plane_state *state)
> +{
> + struct drm_crtc *crtc = state->crtc;
> + struct intel_crtc *intel_crtc;
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + struct intel_plane_state *intel_state = to_intel_plane_state(state);
> +
> + crtc = crtc ? crtc : plane->crtc;
> + intel_crtc = to_intel_crtc(crtc);
> +
> + /*
> + * The original src/dest coordinates are stored in state->base, but
> + * we want to keep another copy internal to our driver that we can
> + * clip/modify ourselves.
> + */
> + intel_state->src.x1 = state->src_x;
> + intel_state->src.y1 = state->src_y;
> + intel_state->src.x2 = state->src_x + state->src_w;
> + intel_state->src.y2 = state->src_y + state->src_h;
> + intel_state->dst.x1 = state->crtc_x;
> + intel_state->dst.y1 = state->crtc_y;
> + intel_state->dst.x2 = state->crtc_x + state->crtc_w;
> + intel_state->dst.y2 = state->crtc_y + state->crtc_h;
> +
> + /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
> + intel_state->clip.x1 = 0;
> + intel_state->clip.y1 = 0;
> + intel_state->clip.x2 =
> + intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
> + intel_state->clip.y2 =
> + intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;
> +
> + /*
> + * Disabling a plane is always okay; we just need to update
> + * fb tracking in a special way since cleanup_fb() won't
> + * get called by the plane helpers.
> + */
> + if (state->fb == NULL && plane->state->fb != NULL) {
> + /*
> + * 'prepare' is never called when plane is being disabled, so
> + * we need to handle frontbuffer tracking as a special case
> + */
> + intel_crtc->atomic.disabled_planes |=
> + (1 << drm_plane_index(plane));
> + }
> +
> + return intel_plane->check_plane(plane, intel_state);
> +}
> +
> +static void intel_plane_atomic_update(struct drm_plane *plane,
> + struct drm_plane_state *old_state)
> +{
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + struct intel_plane_state *intel_state =
> + to_intel_plane_state(plane->state);
> +
> + /* Don't disable an already disabled plane */
> + if (!plane->state->fb && !old_state->fb)
> + return;
> +
> + intel_plane->commit_plane(plane, intel_state);
> +}
> +
> +const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
> + .prepare_fb = intel_prepare_plane_fb,
> + .cleanup_fb = intel_cleanup_plane_fb,
> + .atomic_check = intel_plane_atomic_check,
> + .atomic_update = intel_plane_atomic_update,
> +};
> +
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4bb1a37..e7b51db 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -98,6 +98,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
> const struct intel_crtc_config *pipe_config);
> static void chv_prepare_pll(struct intel_crtc *crtc,
> const struct intel_crtc_config *pipe_config);
> +static void intel_begin_crtc_commit(struct drm_crtc *crtc);
> +static void intel_finish_crtc_commit(struct drm_crtc *crtc);
>
> static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe)
> {
> @@ -9794,6 +9796,8 @@ out_hang:
> static struct drm_crtc_helper_funcs intel_helper_funcs = {
> .mode_set_base_atomic = intel_pipe_set_base_atomic,
> .load_lut = intel_crtc_load_lut,
> + .atomic_begin = intel_begin_crtc_commit,
> + .atomic_flush = intel_finish_crtc_commit,
> };
>
> /**
> @@ -11674,7 +11678,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> unsigned frontbuffer_bits = 0;
> int ret = 0;
>
> - if (WARN_ON(fb == plane->fb || !obj))
> + if (!obj)
> return 0;
>
> switch (plane->type) {
> @@ -11741,7 +11745,7 @@ intel_check_primary_plane(struct drm_plane *plane,
> struct drm_device *dev = plane->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *crtc = state->base.crtc;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc *intel_crtc;
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_framebuffer *fb = state->base.fb;
> struct drm_rect *dest = &state->dst;
> @@ -11749,6 +11753,9 @@ intel_check_primary_plane(struct drm_plane *plane,
> const struct drm_rect *clip = &state->clip;
> int ret;
>
> + crtc = crtc ? crtc : plane->crtc;
> + intel_crtc = to_intel_crtc(crtc);
> +
> ret = drm_plane_helper_check_update(plane, crtc, fb,
> src, dest, clip,
> DRM_PLANE_HELPER_NO_SCALING,
> @@ -11804,23 +11811,26 @@ intel_commit_primary_plane(struct drm_plane *plane,
> struct drm_framebuffer *fb = state->base.fb;
> struct drm_device *dev = plane->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc *intel_crtc;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_rect *src = &state->src;
>
> + crtc = crtc ? crtc : plane->crtc;
> + intel_crtc = to_intel_crtc(crtc);
> +
> plane->fb = fb;
> crtc->x = src->x1 >> 16;
> crtc->y = src->y1 >> 16;
>
> - intel_plane->crtc_x = state->orig_dst.x1;
> - intel_plane->crtc_y = state->orig_dst.y1;
> - intel_plane->crtc_w = drm_rect_width(&state->orig_dst);
> - intel_plane->crtc_h = drm_rect_height(&state->orig_dst);
> - intel_plane->src_x = state->orig_src.x1;
> - intel_plane->src_y = state->orig_src.y1;
> - intel_plane->src_w = drm_rect_width(&state->orig_src);
> - intel_plane->src_h = drm_rect_height(&state->orig_src);
> + intel_plane->crtc_x = state->base.crtc_x;
> + intel_plane->crtc_y = state->base.crtc_y;
> + intel_plane->crtc_w = state->base.crtc_w;
> + intel_plane->crtc_h = state->base.crtc_h;
> + intel_plane->src_x = state->base.src_x;
> + intel_plane->src_y = state->base.src_y;
> + intel_plane->src_w = state->base.src_w;
> + intel_plane->src_h = state->base.src_h;
> intel_plane->obj = obj;
>
> if (intel_crtc->active) {
> @@ -11850,6 +11860,33 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_plane *intel_plane;
> + struct drm_plane *p;
> + unsigned fb_bits = 0;
> +
> + /* Track fb's for any planes being disabled */
> + list_for_each_entry(p, &dev->mode_config.plane_list, head) {
> + intel_plane = to_intel_plane(p);
> +
> + if (intel_crtc->atomic.disabled_planes &
> + (1 << drm_plane_index(p))) {
> + switch (p->type) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + fb_bits = INTEL_FRONTBUFFER_PRIMARY(intel_plane->pipe);
> + break;
> + case DRM_PLANE_TYPE_CURSOR:
> + fb_bits = INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe);
> + break;
> + case DRM_PLANE_TYPE_OVERLAY:
> + fb_bits = INTEL_FRONTBUFFER_SPRITE(intel_plane->pipe);
> + break;
> + }
> +
> + mutex_lock(&dev->struct_mutex);
> + i915_gem_track_fb(intel_fb_obj(p->fb), NULL, fb_bits);
> + mutex_unlock(&dev->struct_mutex);
> + }
> + }
>
> if (intel_crtc->atomic.wait_for_flips)
> intel_crtc_wait_for_pending_flips(crtc);
> @@ -11907,111 +11944,6 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc)
> memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic));
> }
>
> -int
> -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> - struct drm_framebuffer *fb, int crtc_x, int crtc_y,
> - unsigned int crtc_w, unsigned int crtc_h,
> - uint32_t src_x, uint32_t src_y,
> - uint32_t src_w, uint32_t src_h)
> -{
> - struct drm_device *dev = plane->dev;
> - struct drm_framebuffer *old_fb = plane->fb;
> - struct intel_plane_state state = {{ 0 }};
> - struct intel_plane *intel_plane = to_intel_plane(plane);
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int ret;
> -
> - state.base.crtc = crtc ? crtc : plane->crtc;
> - state.base.fb = fb;
> -
> - /* sample coordinates in 16.16 fixed point */
> - state.src.x1 = src_x;
> - state.src.x2 = src_x + src_w;
> - state.src.y1 = src_y;
> - state.src.y2 = src_y + src_h;
> -
> - /* integer pixels */
> - state.dst.x1 = crtc_x;
> - state.dst.x2 = crtc_x + crtc_w;
> - state.dst.y1 = crtc_y;
> - state.dst.y2 = crtc_y + crtc_h;
> -
> - state.clip.x1 = 0;
> - state.clip.y1 = 0;
> - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
> - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;
> -
> - state.orig_src = state.src;
> - state.orig_dst = state.dst;
> -
> - ret = intel_plane->check_plane(plane, &state);
> - if (ret)
> - return ret;
> -
> - if (fb != old_fb && fb) {
> - ret = intel_prepare_plane_fb(plane, fb);
> - if (ret)
> - return ret;
> - }
> -
> - if (!state.base.fb) {
> - unsigned fb_bits = 0;
> -
> - switch (plane->type) {
> - case DRM_PLANE_TYPE_PRIMARY:
> - fb_bits = INTEL_FRONTBUFFER_PRIMARY(intel_plane->pipe);
> - break;
> - case DRM_PLANE_TYPE_CURSOR:
> - fb_bits = INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe);
> - break;
> - case DRM_PLANE_TYPE_OVERLAY:
> - fb_bits = INTEL_FRONTBUFFER_SPRITE(intel_plane->pipe);
> - break;
> - }
> -
> - /*
> - * 'prepare' is never called when plane is being disabled, so
> - * we need to handle frontbuffer tracking here
> - */
> - mutex_lock(&dev->struct_mutex);
> - i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, fb_bits);
> - mutex_unlock(&dev->struct_mutex);
> - }
> -
> - intel_begin_crtc_commit(crtc);
> - intel_plane->commit_plane(plane, &state);
> - intel_finish_crtc_commit(crtc);
> -
> - if (fb != old_fb && old_fb) {
> - if (intel_crtc->active)
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> - intel_cleanup_plane_fb(plane, old_fb);
> - }
> -
> - plane->fb = fb;
> -
> - return 0;
> -}
> -
> -/**
> - * intel_disable_plane - disable a plane
> - * @plane: plane to disable
> - *
> - * General disable handler for all plane types.
> - */
> -int
> -intel_disable_plane(struct drm_plane *plane)
> -{
> - if (!plane->fb)
> - return 0;
> -
> - if (WARN_ON(!plane->crtc))
> - return -EINVAL;
> -
> - return plane->funcs->update_plane(plane, plane->crtc, NULL,
> - 0, 0, 0, 0, 0, 0, 0, 0);
> -}
> -
> /**
> * intel_plane_destroy - destroy a plane
> * @plane: plane to destroy
> @@ -12022,15 +11954,19 @@ intel_disable_plane(struct drm_plane *plane)
> void intel_plane_destroy(struct drm_plane *plane)
> {
> struct intel_plane *intel_plane = to_intel_plane(plane);
> + intel_plane_destroy_state(plane, plane->state);
> drm_plane_cleanup(plane);
> kfree(intel_plane);
> }
>
> static const struct drm_plane_funcs intel_primary_plane_funcs = {
> - .update_plane = intel_update_plane,
> - .disable_plane = intel_disable_plane,
> + .update_plane = drm_plane_helper_update,
> + .disable_plane = drm_plane_helper_disable,
> .destroy = intel_plane_destroy,
> - .set_property = intel_plane_set_property
> + .set_property = intel_plane_set_property,
> + .atomic_duplicate_state = intel_plane_duplicate_state,
> + .atomic_destroy_state = intel_plane_destroy_state,
> +
> };
>
> static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> @@ -12044,6 +11980,12 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> if (primary == NULL)
> return NULL;
>
> + primary->base.state = intel_plane_duplicate_state(&primary->base);
> + if (primary->base.state == NULL) {
> + kfree(primary);
> + return NULL;
> + }
> +
> primary->can_scale = false;
> primary->max_downscale = 1;
> primary->pipe = pipe;
> @@ -12079,6 +12021,8 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> primary->rotation);
> }
>
> + drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
> +
> return &primary->base;
> }
>
> @@ -12087,17 +12031,19 @@ intel_check_cursor_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> {
> struct drm_crtc *crtc = state->base.crtc;
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = plane->dev;
> struct drm_framebuffer *fb = state->base.fb;
> struct drm_rect *dest = &state->dst;
> struct drm_rect *src = &state->src;
> const struct drm_rect *clip = &state->clip;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int crtc_w, crtc_h;
> + struct intel_crtc *intel_crtc;
> unsigned stride;
> int ret;
>
> + crtc = crtc ? crtc : plane->crtc;
> + intel_crtc = to_intel_crtc(crtc);
> +
> ret = drm_plane_helper_check_update(plane, crtc, fb,
> src, dest, clip,
> DRM_PLANE_HELPER_NO_SCALING,
> @@ -12112,15 +12058,14 @@ intel_check_cursor_plane(struct drm_plane *plane,
> goto finish;
>
> /* Check for which cursor types we support */
> - crtc_w = drm_rect_width(&state->orig_dst);
> - crtc_h = drm_rect_height(&state->orig_dst);
> - if (!cursor_size_ok(dev, crtc_w, crtc_h)) {
> - DRM_DEBUG("Cursor dimension not supported\n");
> + if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) {
> + DRM_DEBUG("Cursor dimension %dx%d not supported\n",
> + state->base.crtc_w, state->base.crtc_h);
> return -EINVAL;
> }
>
> - stride = roundup_pow_of_two(crtc_w) * 4;
> - if (obj->base.size < stride * crtc_h) {
> + stride = roundup_pow_of_two(state->base.crtc_w) * 4;
> + if (obj->base.size < stride * state->base.crtc_h) {
> DRM_DEBUG_KMS("buffer is too small\n");
> return -ENOMEM;
> }
> @@ -12138,8 +12083,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
>
> finish:
> if (intel_crtc->active) {
> - if (intel_crtc->cursor_width !=
> - drm_rect_width(&state->orig_dst))
> + if (intel_crtc->cursor_width != state->base.crtc_w)
> intel_crtc->atomic.update_wm = true;
>
> intel_crtc->atomic.fb_bits |=
> @@ -12154,24 +12098,27 @@ intel_commit_cursor_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> {
> struct drm_crtc *crtc = state->base.crtc;
> - struct drm_device *dev = crtc->dev;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_device *dev = plane->dev;
> + struct intel_crtc *intel_crtc;
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
> uint32_t addr;
>
> + crtc = crtc ? crtc : plane->crtc;
> + intel_crtc = to_intel_crtc(crtc);
> +
> plane->fb = state->base.fb;
> - crtc->cursor_x = state->orig_dst.x1;
> - crtc->cursor_y = state->orig_dst.y1;
> -
> - intel_plane->crtc_x = state->orig_dst.x1;
> - intel_plane->crtc_y = state->orig_dst.y1;
> - intel_plane->crtc_w = drm_rect_width(&state->orig_dst);
> - intel_plane->crtc_h = drm_rect_height(&state->orig_dst);
> - intel_plane->src_x = state->orig_src.x1;
> - intel_plane->src_y = state->orig_src.y1;
> - intel_plane->src_w = drm_rect_width(&state->orig_src);
> - intel_plane->src_h = drm_rect_height(&state->orig_src);
> + crtc->cursor_x = state->base.crtc_x;
> + crtc->cursor_y = state->base.crtc_y;
> +
> + intel_plane->crtc_x = state->base.crtc_x;
> + intel_plane->crtc_y = state->base.crtc_y;
> + intel_plane->crtc_w = state->base.crtc_w;
> + intel_plane->crtc_h = state->base.crtc_h;
> + intel_plane->src_x = state->base.src_x;
> + intel_plane->src_y = state->base.src_y;
> + intel_plane->src_w = state->base.src_w;
> + intel_plane->src_h = state->base.src_h;
> intel_plane->obj = obj;
>
> if (intel_crtc->cursor_bo == obj)
> @@ -12187,18 +12134,20 @@ intel_commit_cursor_plane(struct drm_plane *plane,
> intel_crtc->cursor_addr = addr;
> intel_crtc->cursor_bo = obj;
> update:
> - intel_crtc->cursor_width = drm_rect_width(&state->orig_dst);
> - intel_crtc->cursor_height = drm_rect_height(&state->orig_dst);
> + intel_crtc->cursor_width = state->base.crtc_w;
> + intel_crtc->cursor_height = state->base.crtc_h;
>
> if (intel_crtc->active)
> intel_crtc_update_cursor(crtc, state->visible);
> }
>
> static const struct drm_plane_funcs intel_cursor_plane_funcs = {
> - .update_plane = intel_update_plane,
> - .disable_plane = intel_disable_plane,
> + .update_plane = drm_plane_helper_update,
> + .disable_plane = drm_plane_helper_disable,
> .destroy = intel_plane_destroy,
> .set_property = intel_plane_set_property,
> + .atomic_duplicate_state = intel_plane_duplicate_state,
> + .atomic_destroy_state = intel_plane_destroy_state,
> };
>
> static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> @@ -12210,6 +12159,12 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> if (cursor == NULL)
> return NULL;
>
> + cursor->base.state = intel_plane_duplicate_state(&cursor->base);
> + if (cursor->base.state == NULL) {
> + kfree(cursor);
> + return NULL;
> + }
> +
> cursor->can_scale = false;
> cursor->max_downscale = 1;
> cursor->pipe = pipe;
> @@ -12236,6 +12191,8 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> cursor->rotation);
> }
>
> + drm_plane_helper_add(&cursor->base, &intel_plane_helper_funcs);
> +
> return &cursor->base;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2523315..604bd22 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -248,8 +248,6 @@ struct intel_plane_state {
> struct drm_rect src;
> struct drm_rect dst;
> struct drm_rect clip;
> - struct drm_rect orig_src;
> - struct drm_rect orig_dst;
> bool visible;
>
> /*
> @@ -437,6 +435,7 @@ struct intel_crtc_atomic_commit {
> bool disable_fbc;
> bool pre_disable_primary;
> bool update_wm;
> + unsigned disabled_planes;
>
> /* Sleepable operations to perform after commit */
> unsigned fb_bits;
> @@ -575,6 +574,7 @@ struct cxsr_latency {
> #define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
> #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
> #define to_intel_plane(x) container_of(x, struct intel_plane, base)
> +#define to_intel_plane_state(x) container_of(x, struct intel_plane_state, base)
> #define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL)
>
> struct intel_hdmi {
> @@ -1253,4 +1253,10 @@ void intel_pre_disable_primary(struct drm_crtc *crtc);
> /* intel_tv.c */
> void intel_tv_init(struct drm_device *dev);
>
> +/* intel_atomic.c */
> +struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane);
> +void intel_plane_destroy_state(struct drm_plane *plane,
> + struct drm_plane_state *state);
> +extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
> +
> #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 537fff25..16e7c96 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -33,6 +33,7 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_fourcc.h>
> #include <drm/drm_rect.h>
> +#include <drm/drm_plane_helper.h>
> #include "intel_drv.h"
> #include <drm/i915_drm.h>
> #include "i915_drv.h"
> @@ -1081,12 +1082,13 @@ intel_check_sprite_plane(struct drm_plane *plane,
> uint32_t src_x, src_y, src_w, src_h;
> struct drm_rect *src = &state->src;
> struct drm_rect *dst = &state->dst;
> - struct drm_rect *orig_src = &state->orig_src;
> const struct drm_rect *clip = &state->clip;
> int hscale, vscale;
> int max_scale, min_scale;
> int pixel_size;
>
> + intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
> +
> if (!fb) {
> state->visible = false;
> goto finish;
> @@ -1167,10 +1169,10 @@ intel_check_sprite_plane(struct drm_plane *plane,
> intel_plane->rotation);
>
> /* sanity check to make sure the src viewport wasn't enlarged */
> - WARN_ON(src->x1 < (int) orig_src->x1 ||
> - src->y1 < (int) orig_src->y1 ||
> - src->x2 > (int) orig_src->x2 ||
> - src->y2 > (int) orig_src->y2);
> + WARN_ON(src->x1 < (int) state->base.src_x ||
> + src->y1 < (int) state->base.src_y ||
> + src->x2 > (int) state->base.src_x + state->base.src_w ||
> + src->y2 > (int) state->base.src_y + state->base.src_h);
>
> /*
> * Hardware doesn't handle subpixel coordinates.
> @@ -1267,7 +1269,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> {
> struct drm_crtc *crtc = state->base.crtc;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc *intel_crtc;
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_framebuffer *fb = state->base.fb;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> @@ -1275,14 +1277,19 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> unsigned int crtc_w, crtc_h;
> uint32_t src_x, src_y, src_w, src_h;
>
> - intel_plane->crtc_x = state->orig_dst.x1;
> - intel_plane->crtc_y = state->orig_dst.y1;
> - intel_plane->crtc_w = drm_rect_width(&state->orig_dst);
> - intel_plane->crtc_h = drm_rect_height(&state->orig_dst);
> - intel_plane->src_x = state->orig_src.x1;
> - intel_plane->src_y = state->orig_src.y1;
> - intel_plane->src_w = drm_rect_width(&state->orig_src);
> - intel_plane->src_h = drm_rect_height(&state->orig_src);
> + intel_plane->crtc_x = state->base.crtc_x;
> + intel_plane->crtc_y = state->base.crtc_y;
> + intel_plane->crtc_w = state->base.crtc_w;
> + intel_plane->crtc_h = state->base.crtc_h;
> + intel_plane->src_x = state->base.src_x;
> + intel_plane->src_y = state->base.src_y;
> + intel_plane->src_w = state->base.src_w;
> + intel_plane->src_h = state->base.src_h;
> +
> + crtc = crtc ? crtc : plane->crtc;
> + intel_crtc = to_intel_crtc(crtc);
> +
> + plane->fb = state->base.fb;
> intel_plane->obj = obj;
>
> if (intel_crtc->active) {
> @@ -1406,10 +1413,12 @@ int intel_plane_restore(struct drm_plane *plane)
> }
>
> static const struct drm_plane_funcs intel_sprite_plane_funcs = {
> - .update_plane = intel_update_plane,
> - .disable_plane = intel_disable_plane,
> + .update_plane = drm_plane_helper_update,
> + .disable_plane = drm_plane_helper_disable,
> .destroy = intel_plane_destroy,
> .set_property = intel_plane_set_property,
> + .atomic_duplicate_state = intel_plane_duplicate_state,
> + .atomic_destroy_state = intel_plane_destroy_state,
> };
>
> static uint32_t ilk_plane_formats[] = {
> @@ -1471,6 +1480,13 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> if (!intel_plane)
> return -ENOMEM;
>
> + intel_plane->base.state =
> + intel_plane_duplicate_state(&intel_plane->base);
> + if (intel_plane->base.state == NULL) {
> + kfree(intel_plane);
> + return -ENOMEM;
> + }
> +
> switch (INTEL_INFO(dev)->gen) {
> case 5:
> case 6:
> @@ -1564,6 +1580,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> dev->mode_config.rotation_property,
> intel_plane->rotation);
>
> + drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
> +
> out:
> return ret;
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/5] drm/i915: Drop unused position fields (v2)
2014-12-23 18:41 [PATCH 0/5] i915 atomic plane helper conversion (v5) Matt Roper
` (3 preceding siblings ...)
2014-12-23 18:41 ` [PATCH 4/5] drm/i915: Move to atomic plane helpers (v9) Matt Roper
@ 2014-12-23 18:41 ` Matt Roper
2014-12-23 22:59 ` shuang.he
4 siblings, 1 reply; 12+ messages in thread
From: Matt Roper @ 2014-12-23 18:41 UTC (permalink / raw)
To: intel-gfx
The userspace-requested plane coordinates are now always available via
plane->state.base (and the i915-adjusted values are stored in
plane->state), so we no longer use the coordinate fields in intel_plane
and can drop them.
Also, note that the error case for pageflip calls update_plane() to
program the values from plane->state; it's simpler to just call
intel_plane_restore() which does the same thing.
v2: Replace manual update_plane() with intel_plane_restore() in pageflip
error handler.
Reviewed-by(v1): Bob Paauwe <bob.j.paauwe@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 27 +--------------------------
drivers/gpu/drm/i915/intel_drv.h | 4 ----
drivers/gpu/drm/i915/intel_sprite.c | 19 ++++---------------
3 files changed, 5 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e7b51db..75a514d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9616,7 +9616,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_plane *primary = crtc->primary;
- struct intel_plane *intel_plane = to_intel_plane(primary);
enum pipe pipe = intel_crtc->pipe;
struct intel_unpin_work *work;
struct intel_engine_cs *ring;
@@ -9775,15 +9774,7 @@ free_work:
if (ret == -EIO) {
out_hang:
- ret = primary->funcs->update_plane(primary, crtc, fb,
- intel_plane->crtc_x,
- intel_plane->crtc_y,
- intel_plane->crtc_h,
- intel_plane->crtc_w,
- intel_plane->src_x,
- intel_plane->src_y,
- intel_plane->src_h,
- intel_plane->src_w);
+ ret = intel_plane_restore(primary);
if (ret == 0 && event) {
spin_lock_irq(&dev->event_lock);
drm_send_vblank_event(dev, pipe, event);
@@ -11823,14 +11814,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
crtc->x = src->x1 >> 16;
crtc->y = src->y1 >> 16;
- intel_plane->crtc_x = state->base.crtc_x;
- intel_plane->crtc_y = state->base.crtc_y;
- intel_plane->crtc_w = state->base.crtc_w;
- intel_plane->crtc_h = state->base.crtc_h;
- intel_plane->src_x = state->base.src_x;
- intel_plane->src_y = state->base.src_y;
- intel_plane->src_w = state->base.src_w;
- intel_plane->src_h = state->base.src_h;
intel_plane->obj = obj;
if (intel_crtc->active) {
@@ -12111,14 +12094,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
crtc->cursor_x = state->base.crtc_x;
crtc->cursor_y = state->base.crtc_y;
- intel_plane->crtc_x = state->base.crtc_x;
- intel_plane->crtc_y = state->base.crtc_y;
- intel_plane->crtc_w = state->base.crtc_w;
- intel_plane->crtc_h = state->base.crtc_h;
- intel_plane->src_x = state->base.src_x;
- intel_plane->src_y = state->base.src_y;
- intel_plane->src_w = state->base.src_w;
- intel_plane->src_h = state->base.src_h;
intel_plane->obj = obj;
if (intel_crtc->cursor_bo == obj)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 604bd22..db3fde3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -517,10 +517,6 @@ struct intel_plane {
struct drm_i915_gem_object *obj;
bool can_scale;
int max_downscale;
- int crtc_x, crtc_y;
- unsigned int crtc_w, crtc_h;
- uint32_t src_x, src_y;
- uint32_t src_w, src_h;
unsigned int rotation;
/* Since we need to change the watermarks before/after
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 16e7c96..37079f1 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1277,15 +1277,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
unsigned int crtc_w, crtc_h;
uint32_t src_x, src_y, src_w, src_h;
- intel_plane->crtc_x = state->base.crtc_x;
- intel_plane->crtc_y = state->base.crtc_y;
- intel_plane->crtc_w = state->base.crtc_w;
- intel_plane->crtc_h = state->base.crtc_h;
- intel_plane->src_x = state->base.src_x;
- intel_plane->src_y = state->base.src_y;
- intel_plane->src_w = state->base.src_w;
- intel_plane->src_h = state->base.src_h;
-
crtc = crtc ? crtc : plane->crtc;
intel_crtc = to_intel_crtc(crtc);
@@ -1400,16 +1391,14 @@ int intel_plane_set_property(struct drm_plane *plane,
int intel_plane_restore(struct drm_plane *plane)
{
- struct intel_plane *intel_plane = to_intel_plane(plane);
-
if (!plane->crtc || !plane->fb)
return 0;
return plane->funcs->update_plane(plane, plane->crtc, plane->fb,
- intel_plane->crtc_x, intel_plane->crtc_y,
- intel_plane->crtc_w, intel_plane->crtc_h,
- intel_plane->src_x, intel_plane->src_y,
- intel_plane->src_w, intel_plane->src_h);
+ plane->state->crtc_x, plane->state->crtc_y,
+ plane->state->crtc_w, plane->state->crtc_h,
+ plane->state->src_x, plane->state->src_y,
+ plane->state->src_w, plane->state->src_h);
}
static const struct drm_plane_funcs intel_sprite_plane_funcs = {
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 5/5] drm/i915: Drop unused position fields (v2)
2014-12-23 18:41 ` [PATCH 5/5] drm/i915: Drop unused position fields (v2) Matt Roper
@ 2014-12-23 22:59 ` shuang.he
0 siblings, 0 replies; 12+ messages in thread
From: shuang.he @ 2014-12-23 22:59 UTC (permalink / raw)
To: shuang.he, intel-gfx, matthew.d.roper
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 363/364 363/364
ILK -1 364/366 363/366
SNB -2 447/450 445/450
IVB -2 496/498 494/498
BYT 288/289 288/289
HSW -1 562/564 561/564
BDW 415/417 415/417
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt_gem_reloc_vs_gpu_forked-faulting-reloc-thrash-inactive PASS(2, M37) DMESG_WARN(1, M37)
*SNB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip PASS(2, M35) DMESG_WARN(1, M35)
*SNB igt_kms_rotation_crc_sprite-rotation PASS(2, M35) DMESG_WARN(1, M35)
*IVB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip PASS(2, M34) DMESG_WARN(1, M34)
*IVB igt_kms_rotation_crc_sprite-rotation PASS(2, M34) DMESG_WARN(1, M34)
*HSW igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip PASS(2, M40) DMESG_WARN(1, M40)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread