From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
We need to call the plane .atomic_check() hook when enabling the crtc
to make sure all the derived state (eg. visible, clipped src/dst
coordinates) are up to date when we enable the plane. This allows us to
trust this derived state everywhere.
We also take the opportunity to rewrite the plane enable sequence to
perform it as a single atomic update, which is a bit closer to where we
want to end up. Obviously this is a bit of a hack as we can't deal with
errors from .atomic_check(), so we just paper over that by making sure
visible=false so that we don't try to enable the plane with potentially
garbage coordinates and whatnot.
We also abuse the atomic code a bit by not making another copy of the
plane state and just frobbing directly with the plane->state.
Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
Cc: Sonika Jindal <sonika.jindal@xxxxxxxxx>
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/intel_display.c | 222 ++++++++++++++---------------------
2 files changed, 88 insertions(+), 137 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5462cbf..b16c0a7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -238,6 +238,9 @@ enum hpd_pin {
#define for_each_intel_crtc(dev, intel_crtc) \
list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head)
+#define for_each_intel_plane(dev, intel_plane) \
+ list_for_each_entry(intel_plane, &dev->mode_config.plane_list, base.head)
+
#define for_each_intel_encoder(dev, intel_encoder) \
list_for_each_entry(intel_encoder, \
&(dev)->mode_config.encoder_list, \
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0da3abf..fdc83f1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2124,66 +2124,6 @@ void intel_flush_primary_plane(struct drm_i915_private *dev_priv,
POSTING_READ(reg);
}
-/**
- * intel_enable_primary_hw_plane - enable the primary plane on a given pipe
- * @plane: plane to be enabled
- * @crtc: crtc for the plane
- *
- * Enable @plane on @crtc, making sure that the pipe is running first.
- */
-static void intel_enable_primary_hw_plane(struct drm_plane *plane,
- struct drm_crtc *crtc)
-{
- struct drm_device *dev = plane->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- /* If the pipe isn't enabled, we can't pump pixels and may hang */
- assert_pipe_enabled(dev_priv, intel_crtc->pipe);
-
- if (intel_crtc->primary_enabled)
- return;
-
- intel_crtc->primary_enabled = true;
-
- 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))
- intel_wait_for_vblank(dev, intel_crtc->pipe);
-}
-
-/**
- * intel_disable_primary_hw_plane - disable the primary hardware plane
- * @plane: plane to be disabled
- * @crtc: crtc for the plane
- *
- * Disable @plane on @crtc, making sure that the pipe is running first.
- */
-static void intel_disable_primary_hw_plane(struct drm_plane *plane,
- struct drm_crtc *crtc)
-{
- struct drm_device *dev = plane->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- if (WARN_ON(!intel_crtc->active))
- return;
-
- if (!intel_crtc->primary_enabled)
- return;
-
- intel_crtc->primary_enabled = false;
-
- dev_priv->display.update_primary_plane(crtc, plane->fb,
- crtc->x, crtc->y);
-}
-
static bool need_vtd_wa(struct drm_device *dev)
{
#ifdef CONFIG_INTEL_IOMMU
@@ -2895,7 +2835,10 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
POSTING_READ(PLANE_SURF(pipe, 0));
}
-/* Assume fb object is pinned & idle & fenced and just update base pointers */
+/*
+ * Assume fb object is big enough & pinned & idle & fenced,
+ * and just update base pointers
+ */
static int
intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
int x, int y, enum mode_set_atomic state)
@@ -2906,6 +2849,7 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
if (dev_priv->display.disable_fbc)
dev_priv->display.disable_fbc(dev);
+ to_intel_crtc(crtc)->primary_enabled = true;
dev_priv->display.update_primary_plane(crtc, fb, x, y);
return 0;
@@ -2933,16 +2877,17 @@ static void intel_update_primary_planes(struct drm_device *dev)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
drm_modeset_lock(&crtc->mutex, NULL);
- /*
- * FIXME: Once we have proper support for primary planes (and
- * disabling them without disabling the entire crtc) allow again
- * a NULL crtc->primary->fb.
- */
- if (intel_crtc->active && crtc->primary->fb)
+
+ if (intel_crtc->active) {
+ const struct intel_plane_state *state =
+ to_intel_plane_state(crtc->primary->state);
+
dev_priv->display.update_primary_plane(crtc,
- crtc->primary->fb,
- crtc->x,
- crtc->y);
+ state->base.fb,
+ state->src.x1 >> 16,
+ state->src.y1 >> 16);
+ }
+
drm_modeset_unlock(&crtc->mutex);
}
}
@@ -4183,50 +4128,76 @@ static void ironlake_pfit_enable(struct intel_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;
- struct drm_plane *plane;
- struct intel_plane *intel_plane;
-
- drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
- intel_plane = to_intel_plane(plane);
- if (intel_plane->pipe == pipe)
- intel_plane_restore(&intel_plane->base);
- }
-}
-
/*
- * Disable a plane internally without actually modifying the plane's state.
- * This will allow us to easily restore the plane later by just reprogramming
- * its state.
+ * Ugly hack to make sure we update the planes correctly
*/
-static void disable_plane_internal(struct drm_plane *plane)
+static void _intel_crtc_enable_planes(struct intel_crtc *crtc)
{
- struct intel_plane *intel_plane = to_intel_plane(plane);
- struct drm_plane_state *state =
- plane->funcs->atomic_duplicate_state(plane);
- struct intel_plane_state *intel_state = to_intel_plane_state(state);
+ struct drm_device *dev = crtc->base.dev;
+ enum pipe pipe = crtc->pipe;
+ struct intel_plane *plane;
+ const struct drm_crtc_helper_funcs *crtc_funcs =
+ crtc->base.helper_private;
- intel_state->visible = false;
- intel_plane->commit_plane(plane, intel_state);
+ for_each_intel_plane(dev, plane) {
+ const struct drm_plane_helper_funcs *funcs =
+ plane->base.helper_private;
+ struct intel_plane_state *state =
+ to_intel_plane_state(plane->base.state);
- intel_plane_destroy_state(plane, state);
+ if (plane->pipe != pipe)
+ continue;
+
+ if (funcs->atomic_check(&plane->base, &state->base))
+ state->visible = false;
+ }
+
+ crtc_funcs->atomic_begin(&crtc->base);
+
+ for_each_intel_plane(dev, plane) {
+ const struct drm_plane_helper_funcs *funcs =
+ plane->base.helper_private;
+
+ if (plane->pipe != pipe)
+ continue;
+
+ funcs->atomic_update(&plane->base, plane->base.state);
+ }
+
+ crtc_funcs->atomic_flush(&crtc->base);
}
-static void intel_disable_sprite_planes(struct drm_crtc *crtc)
+static void _intel_crtc_disable_planes(struct intel_crtc *crtc)
{
- struct drm_device *dev = crtc->dev;
- enum pipe pipe = to_intel_crtc(crtc)->pipe;
- struct drm_plane *plane;
- struct intel_plane *intel_plane;
-
- drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
- intel_plane = to_intel_plane(plane);
- if (plane->fb && intel_plane->pipe == pipe)
- disable_plane_internal(plane);
+ struct drm_device *dev = crtc->base.dev;
+ enum pipe pipe = crtc->pipe;
+ struct intel_plane *plane;
+ const struct drm_crtc_helper_funcs *crtc_funcs =
+ crtc->base.helper_private;
+
+ for_each_intel_plane(dev, plane) {
+ struct intel_plane_state *state =
+ to_intel_plane_state(plane->base.state);
+
+ if (plane->pipe != pipe)
+ continue;
+
+ state->visible = false;
}
+
+ crtc_funcs->atomic_begin(&crtc->base);
+
+ for_each_intel_plane(dev, plane) {
+ const struct drm_plane_helper_funcs *funcs =
+ plane->base.helper_private;
+
+ if (plane->pipe != pipe)
+ continue;
+
+ funcs->atomic_update(&plane->base, plane->base.state);
+ }
+
+ crtc_funcs->atomic_flush(&crtc->base);
}
void hsw_enable_ips(struct intel_crtc *crtc)
@@ -4358,9 +4329,7 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_crtc->pipe;
- intel_enable_primary_hw_plane(crtc->primary, crtc);
- intel_enable_sprite_planes(crtc);
- intel_crtc_update_cursor(crtc, true);
+ _intel_crtc_enable_planes(intel_crtc);
intel_crtc_dpms_overlay(intel_crtc, true);
hsw_enable_ips(intel_crtc);
@@ -4392,9 +4361,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
hsw_disable_ips(intel_crtc);
intel_crtc_dpms_overlay(intel_crtc, false);
- intel_crtc_update_cursor(crtc, false);
- intel_disable_sprite_planes(crtc);
- intel_disable_primary_hw_plane(crtc->primary, crtc);
+ _intel_crtc_disable_planes(intel_crtc);
/*
* FIXME: Once we grow proper nuclear flip support out of this we need
@@ -11708,7 +11675,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
modeset_pipes, prepare_pipes,
disable_pipes);
} else if (config->fb_changed) {
- struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
struct drm_plane *primary = set->crtc->primary;
int vdisplay, hdisplay;
@@ -11719,15 +11685,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
hdisplay << 16, vdisplay << 16);
/*
- * We need to make sure the primary plane is re-enabled if it
- * has previously been turned off.
- */
- if (!intel_crtc->primary_enabled && ret == 0) {
- WARN_ON(!intel_crtc->active);
- intel_enable_primary_hw_plane(set->crtc->primary, set->crtc);
- }
-
- /*
* In the fastboot case this may be our only check of the
* state after boot. It would be better to only do it on
* the first update, but we don't have a nice way of doing that
@@ -12054,24 +12011,15 @@ intel_commit_primary_plane(struct drm_plane *plane,
intel_plane->obj = obj;
if (intel_crtc->active) {
- if (state->visible) {
- /* FIXME: kill this fastboot hack */
+ /* FIXME: kill this fastboot hack */
+ if (state->visible)
intel_update_pipe_size(intel_crtc);
- intel_crtc->primary_enabled = true;
-
- dev_priv->display.update_primary_plane(crtc, plane->fb,
- crtc->x, crtc->y);
- } else {
- /*
- * If clipping results in a non-visible primary plane,
- * we'll disable the primary plane. Note that this is
- * a bit different than what happens if userspace
- * explicitly disables the plane by passing fb=0
- * because plane->fb still gets set and pinned.
- */
- intel_disable_primary_hw_plane(plane, crtc);
- }
+ intel_crtc->primary_enabled = state->visible;
+ dev_priv->display.update_primary_plane(crtc,
+ state->base.fb,
+ state->src.x1 >> 16,
+ state->src.y1 >> 16);
}
}
--
2.0.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
