* [PATCH v2 0/9] Kill off intel_crtc->atomic!
@ 2016-01-11 12:27 Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 1/9] drm/i915: Unify power domain handling Maarten Lankhorst
` (10 more replies)
0 siblings, 11 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:27 UTC (permalink / raw)
To: intel-gfx
I've fixed some patches with feedback from review.
It's about time that intel_crtc->atomic dies. It was useful
with the transitional helpers, but can be removed safely now.
Maarten Lankhorst (9):
drm/i915: Unify power domain handling.
drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3.
drm/i915: Remove intel_crtc->atomic.disable_ips.
drm/i915: Remove atomic.pre_disable_primary.
drm/i915: Remove update_sprite_watermarks.
drm/i915: Remove some post-commit members from intel_crtc->atomic, v2.
drm/i915: Nuke fbc members from intel_crtc->atomic, v2.
drm/i915: Do not compute watermarks on a noop.
drm/i915: Remove vblank wait from hsw_enable_ips.
drivers/gpu/drm/i915/intel_atomic.c | 2 +
drivers/gpu/drm/i915/intel_display.c | 291 +++++++++++++++++------------------
drivers/gpu/drm/i915/intel_drv.h | 39 ++---
drivers/gpu/drm/i915/intel_fbc.c | 20 ++-
drivers/gpu/drm/i915/intel_pm.c | 59 ++++---
5 files changed, 214 insertions(+), 197 deletions(-)
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/9] drm/i915: Unify power domain handling.
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
@ 2016-01-11 12:27 ` Maarten Lankhorst
2016-01-11 12:43 ` Maarten Lankhorst
2016-01-11 13:31 ` [PATCH v2 0.999/9] drm/i915: Pass crtc state to modeset_get_crtc_power_domains Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3 Maarten Lankhorst
` (9 subsequent siblings)
10 siblings, 2 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:27 UTC (permalink / raw)
To: intel-gfx
Right now there's separate power domain handling for update_pipe and
modesets. Unify this and only grab POWER_DOMAIN_MODESET once.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 64 ++++++++++++------------------------
1 file changed, 21 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3fd6f1c157b8..2aa1c5367625 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5393,31 +5393,6 @@ static void modeset_put_power_domains(struct drm_i915_private *dev_priv,
intel_display_power_put(dev_priv, domain);
}
-static void modeset_update_crtc_power_domains(struct drm_atomic_state *state)
-{
- struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
- struct drm_device *dev = state->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- unsigned long put_domains[I915_MAX_PIPES] = {};
- struct drm_crtc_state *crtc_state;
- struct drm_crtc *crtc;
- int i;
-
- for_each_crtc_in_state(state, crtc, crtc_state, i) {
- if (needs_modeset(crtc->state))
- put_domains[to_intel_crtc(crtc)->pipe] =
- modeset_get_crtc_power_domains(crtc);
- }
-
- if (dev_priv->display.modeset_commit_cdclk &&
- intel_state->dev_cdclk != dev_priv->cdclk_freq)
- dev_priv->display.modeset_commit_cdclk(state);
-
- for (i = 0; i < I915_MAX_PIPES; i++)
- if (put_domains[i])
- modeset_put_power_domains(dev_priv, put_domains[i]);
-}
-
static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
{
int max_cdclk_freq = dev_priv->max_cdclk_freq;
@@ -13561,6 +13536,7 @@ static int intel_atomic_commit(struct drm_device *dev,
struct intel_crtc_state *intel_cstate;
int ret = 0, i;
bool hw_check = intel_state->modeset;
+ unsigned long put_domains[I915_MAX_PIPES] = {};
ret = intel_atomic_prepare_commit(dev, state, async);
if (ret) {
@@ -13576,11 +13552,21 @@ static int intel_atomic_commit(struct drm_device *dev,
sizeof(intel_state->min_pixclk));
dev_priv->active_crtcs = intel_state->active_crtcs;
dev_priv->atomic_cdclk_freq = intel_state->cdclk;
+
+ intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
}
for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ if (needs_modeset(crtc->state) ||
+ to_intel_crtc_state(crtc->state)->update_pipe) {
+ hw_check = true;
+
+ put_domains[to_intel_crtc(crtc)->pipe] =
+ modeset_get_crtc_power_domains(crtc);
+ }
+
if (!needs_modeset(crtc->state))
continue;
@@ -13612,7 +13598,10 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_shared_dpll_commit(state);
drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
- modeset_update_crtc_power_domains(state);
+
+ if (dev_priv->display.modeset_commit_cdclk &&
+ intel_state->dev_cdclk != dev_priv->cdclk_freq)
+ dev_priv->display.modeset_commit_cdclk(state);
}
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
@@ -13621,23 +13610,12 @@ static int intel_atomic_commit(struct drm_device *dev,
bool modeset = needs_modeset(crtc->state);
bool update_pipe = !modeset &&
to_intel_crtc_state(crtc->state)->update_pipe;
- unsigned long put_domains = 0;
-
- if (modeset)
- intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
if (modeset && crtc->state->active) {
update_scanline_offset(to_intel_crtc(crtc));
dev_priv->display.crtc_enable(crtc);
}
- if (update_pipe) {
- put_domains = modeset_get_crtc_power_domains(crtc);
-
- /* make sure intel_modeset_check_state runs */
- hw_check = true;
- }
-
if (!modeset)
intel_pre_plane_update(intel_crtc);
@@ -13645,13 +13623,7 @@ static int intel_atomic_commit(struct drm_device *dev,
(crtc->state->planes_changed || update_pipe))
drm_atomic_helper_commit_planes_on_crtc(crtc_state);
- if (put_domains)
- modeset_put_power_domains(dev_priv, put_domains);
-
intel_post_plane_update(intel_crtc);
-
- if (modeset)
- intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
}
/* FIXME: add subpixel order */
@@ -13670,8 +13642,14 @@ static int intel_atomic_commit(struct drm_device *dev,
if (dev_priv->display.optimize_watermarks)
dev_priv->display.optimize_watermarks(intel_cstate);
+
+ if (put_domains[i])
+ modeset_put_power_domains(dev_priv, put_domains[i]);
}
+ if (intel_state->modeset)
+ intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
+
mutex_lock(&dev->struct_mutex);
drm_atomic_helper_cleanup_planes(dev, state);
mutex_unlock(&dev->struct_mutex);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3.
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 1/9] drm/i915: Unify power domain handling Maarten Lankhorst
@ 2016-01-11 12:27 ` Maarten Lankhorst
2016-01-13 12:58 ` Ville Syrjälä
2016-01-11 12:27 ` [PATCH v2 3/9] drm/i915: Remove intel_crtc->atomic.disable_ips Maarten Lankhorst
` (8 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:27 UTC (permalink / raw)
To: intel-gfx
Currently we perform our own wait in post_plane_update,
but the atomic core performs another one in wait_for_vblanks.
This means that 2 vblanks are done when a fb is changed,
which is a bit overkill.
Merge them by creating a helper function that takes a crtc mask
for the planes to wait on.
The broadwell vblank workaround may look gone entirely but this is
not the case. pipe_config->wm_changed is set to true
when any plane is turned on, which forces a vblank wait.
Changes since v1:
- Removing the double vblank wait on broadwell moved to its own commit.
Changes since v2:
- Move out POWER_DOMAIN_MODESET handling to its own commit.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_atomic.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 80 ++++++++++++++++++++++++++----------
drivers/gpu/drm/i915/intel_drv.h | 2 +-
3 files changed, 61 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 9682d94af710..ba9a57f33c43 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -98,6 +98,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
crtc_state->disable_cxsr = false;
crtc_state->wm_changed = false;
crtc_state->wm.need_postvbl_update = false;
+ crtc_state->fb_changed = false;
return &crtc_state->base;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2aa1c5367625..eac73f005a70 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4796,9 +4796,6 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
to_intel_crtc_state(crtc->base.state);
struct drm_device *dev = crtc->base.dev;
- if (atomic->wait_vblank)
- intel_wait_for_vblank(dev, crtc->pipe);
-
intel_frontbuffer_flip(dev, atomic->fb_bits);
crtc->wm.cxsr_allowed = true;
@@ -11872,6 +11869,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
if (!was_visible && !visible)
return 0;
+ if (fb != old_plane_state->base.fb)
+ pipe_config->fb_changed = true;
+
turn_off = was_visible && (!visible || mode_changed);
turn_on = visible && (!was_visible || mode_changed);
@@ -11887,8 +11887,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
/* must disable cxsr around plane enable/disable */
if (plane->type != DRM_PLANE_TYPE_CURSOR) {
- if (is_crtc_enabled)
- intel_crtc->atomic.wait_vblank = true;
pipe_config->disable_cxsr = true;
}
} else if (intel_wm_need_update(plane, plane_state)) {
@@ -11940,14 +11938,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
plane_state->rotation != BIT(DRM_ROTATE_0))
intel_crtc->atomic.disable_fbc = true;
- /*
- * 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 (turn_on && IS_BROADWELL(dev))
- intel_crtc->atomic.wait_vblank = true;
-
intel_crtc->atomic.update_fbc |= visible || mode_changed;
break;
case DRM_PLANE_TYPE_CURSOR:
@@ -11962,12 +11952,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
if (IS_IVYBRIDGE(dev) &&
needs_scaling(to_intel_plane_state(plane_state)) &&
!needs_scaling(old_plane_state)) {
- to_intel_crtc_state(crtc_state)->disable_lp_wm = true;
- } else if (turn_off && !mode_changed) {
- intel_crtc->atomic.wait_vblank = true;
+ pipe_config->disable_lp_wm = true;
+ } else if (turn_off && !mode_changed)
intel_crtc->atomic.update_sprite_watermarks |=
1 << i;
- }
break;
}
@@ -13509,6 +13497,48 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
return ret;
}
+static void intel_atomic_wait_for_vblanks(struct drm_device *dev,
+ struct drm_i915_private *dev_priv,
+ unsigned crtc_mask)
+{
+ unsigned last_vblank_count[I915_MAX_PIPES];
+ enum pipe pipe;
+ int ret;
+
+ if (!crtc_mask)
+ return;
+
+ for_each_pipe(dev_priv, pipe) {
+ struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+
+ if (!((1 << pipe) & crtc_mask))
+ continue;
+
+ ret = drm_crtc_vblank_get(crtc);
+ if (ret != 0) {
+ crtc_mask &= ~(1 << pipe);
+ continue;
+ }
+
+ last_vblank_count[pipe] = drm_crtc_vblank_count(crtc);
+ }
+
+ for_each_pipe(dev_priv, pipe) {
+ struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+
+ if (!((1 << pipe) & crtc_mask))
+ continue;
+
+ wait_event_timeout(dev->vblank[pipe].queue,
+ last_vblank_count[pipe] !=
+ drm_crtc_vblank_count(crtc),
+ msecs_to_jiffies(50));
+
+ drm_crtc_vblank_put(crtc);
+ }
+}
+
+
/**
* intel_atomic_commit - commit validated state object
* @dev: DRM device
@@ -13537,6 +13567,7 @@ static int intel_atomic_commit(struct drm_device *dev,
int ret = 0, i;
bool hw_check = intel_state->modeset;
unsigned long put_domains[I915_MAX_PIPES] = {};
+ unsigned crtc_vblank_mask = 0;
ret = intel_atomic_prepare_commit(dev, state, async);
if (ret) {
@@ -13608,8 +13639,9 @@ static int intel_atomic_commit(struct drm_device *dev,
for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
bool modeset = needs_modeset(crtc->state);
- bool update_pipe = !modeset &&
- to_intel_crtc_state(crtc->state)->update_pipe;
+ struct intel_crtc_state *pipe_config =
+ to_intel_crtc_state(crtc->state);
+ bool update_pipe = !modeset && pipe_config->update_pipe;
if (modeset && crtc->state->active) {
update_scanline_offset(to_intel_crtc(crtc));
@@ -13623,12 +13655,18 @@ static int intel_atomic_commit(struct drm_device *dev,
(crtc->state->planes_changed || update_pipe))
drm_atomic_helper_commit_planes_on_crtc(crtc_state);
- intel_post_plane_update(intel_crtc);
+ if (pipe_config->base.active &&
+ (pipe_config->wm_changed || pipe_config->disable_cxsr ||
+ pipe_config->fb_changed))
+ crtc_vblank_mask |= 1 << i;
}
/* FIXME: add subpixel order */
- drm_atomic_helper_wait_for_vblanks(dev, state);
+ intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
+
+ for_each_crtc_in_state(state, crtc, crtc_state, i)
+ intel_post_plane_update(to_intel_crtc(crtc));
/*
* Now that the vblank has passed, we can go ahead and program the
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bdfe4035e074..8940aa4cf50c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -385,6 +385,7 @@ struct intel_crtc_state {
bool update_pipe; /* can a fast modeset be performed? */
bool disable_cxsr;
bool wm_changed; /* watermarks are updated */
+ bool fb_changed; /* fb on any of the planes is changed */
/* Pipe source size (ie. panel fitter input size)
* All planes will be positioned inside this space,
@@ -570,7 +571,6 @@ struct intel_crtc_atomic_commit {
/* Sleepable operations to perform after commit */
unsigned fb_bits;
- bool wait_vblank;
bool update_fbc;
bool post_enable_primary;
unsigned update_sprite_watermarks;
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 3/9] drm/i915: Remove intel_crtc->atomic.disable_ips.
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 1/9] drm/i915: Unify power domain handling Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3 Maarten Lankhorst
@ 2016-01-11 12:27 ` Maarten Lankhorst
2016-01-13 13:02 ` Ville Syrjälä
2016-01-11 12:27 ` [PATCH v2 4/9] drm/i915: Remove atomic.pre_disable_primary Maarten Lankhorst
` (7 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:27 UTC (permalink / raw)
To: intel-gfx
This is a revert of commit 066cf55b9ce3 "drm/i915: Fix IPS related flicker".
intel_pre_disable_primary already handles this, and now everything
goes through the atomic path there's no need to try to disable ips twice.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
---
drivers/gpu/drm/i915/intel_display.c | 16 +---------------
drivers/gpu/drm/i915/intel_drv.h | 1 -
2 files changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index eac73f005a70..62044ad5c6ec 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4823,9 +4823,6 @@ static void intel_pre_plane_update(struct intel_crtc *crtc)
if (atomic->disable_fbc)
intel_fbc_deactivate(crtc);
- if (crtc->atomic.disable_ips)
- hsw_disable_ips(crtc);
-
if (atomic->pre_disable_primary)
intel_pre_disable_primary(&crtc->base);
@@ -11907,19 +11904,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
intel_crtc->atomic.pre_disable_primary = turn_off;
intel_crtc->atomic.post_enable_primary = turn_on;
- if (turn_off) {
- /*
- * FIXME: Actually if we will still have any other
- * plane enabled on the pipe we could let IPS enabled
- * still, but for now lets consider that when we make
- * primary invisible by setting DSPCNTR to 0 on
- * update_primary_plane function IPS needs to be
- * disable.
- */
- intel_crtc->atomic.disable_ips = true;
-
+ if (turn_off)
intel_crtc->atomic.disable_fbc = true;
- }
/*
* FBC does not work on some platforms for rotated
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8940aa4cf50c..39adf7cd0b36 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -566,7 +566,6 @@ struct intel_mmio_flip {
struct intel_crtc_atomic_commit {
/* Sleepable operations to perform before commit */
bool disable_fbc;
- bool disable_ips;
bool pre_disable_primary;
/* Sleepable operations to perform after commit */
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 4/9] drm/i915: Remove atomic.pre_disable_primary.
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
` (2 preceding siblings ...)
2016-01-11 12:27 ` [PATCH v2 3/9] drm/i915: Remove intel_crtc->atomic.disable_ips Maarten Lankhorst
@ 2016-01-11 12:27 ` Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 5/9] drm/i915: Remove update_sprite_watermarks Maarten Lankhorst
` (6 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:27 UTC (permalink / raw)
To: intel-gfx
This can be derived from the atomic state in pre_plane_update,
which makes it more clear when it's supposed to be called.
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++-------
drivers/gpu/drm/i915/intel_drv.h | 1 -
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 62044ad5c6ec..8340b43aacd6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4812,19 +4812,33 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
memset(atomic, 0, sizeof(*atomic));
}
-static void intel_pre_plane_update(struct intel_crtc *crtc)
+static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
{
+ struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc->base.state);
+ struct drm_atomic_state *old_state = old_crtc_state->base.state;
+ struct drm_plane *primary = crtc->base.primary;
+ struct drm_plane_state *old_pri_state =
+ drm_atomic_get_existing_plane_state(old_state, primary);
+ bool modeset = needs_modeset(&pipe_config->base);
if (atomic->disable_fbc)
intel_fbc_deactivate(crtc);
- if (atomic->pre_disable_primary)
- intel_pre_disable_primary(&crtc->base);
+ if (old_pri_state) {
+ struct intel_plane_state *primary_state =
+ to_intel_plane_state(primary->state);
+ struct intel_plane_state *old_primary_state =
+ to_intel_plane_state(old_pri_state);
+
+ if (old_primary_state->visible &&
+ (modeset || !primary_state->visible))
+ intel_pre_disable_primary(&crtc->base);
+ }
if (pipe_config->disable_cxsr) {
crtc->wm.cxsr_allowed = false;
@@ -11901,7 +11915,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
switch (plane->type) {
case DRM_PLANE_TYPE_PRIMARY:
- intel_crtc->atomic.pre_disable_primary = turn_off;
intel_crtc->atomic.post_enable_primary = turn_on;
if (turn_off)
@@ -13587,7 +13600,7 @@ static int intel_atomic_commit(struct drm_device *dev,
if (!needs_modeset(crtc->state))
continue;
- intel_pre_plane_update(intel_crtc);
+ intel_pre_plane_update(to_intel_crtc_state(crtc_state));
if (crtc_state->active) {
intel_crtc_disable_planes(crtc, crtc_state->plane_mask);
@@ -13623,7 +13636,6 @@ static int intel_atomic_commit(struct drm_device *dev,
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
for_each_crtc_in_state(state, crtc, crtc_state, i) {
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
bool modeset = needs_modeset(crtc->state);
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc->state);
@@ -13635,7 +13647,7 @@ static int intel_atomic_commit(struct drm_device *dev,
}
if (!modeset)
- intel_pre_plane_update(intel_crtc);
+ intel_pre_plane_update(to_intel_crtc_state(crtc_state));
if (crtc->state->active &&
(crtc->state->planes_changed || update_pipe))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 39adf7cd0b36..f284c61bbae9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -566,7 +566,6 @@ struct intel_mmio_flip {
struct intel_crtc_atomic_commit {
/* Sleepable operations to perform before commit */
bool disable_fbc;
- bool pre_disable_primary;
/* Sleepable operations to perform after commit */
unsigned fb_bits;
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 5/9] drm/i915: Remove update_sprite_watermarks.
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
` (3 preceding siblings ...)
2016-01-11 12:27 ` [PATCH v2 4/9] drm/i915: Remove atomic.pre_disable_primary Maarten Lankhorst
@ 2016-01-11 12:27 ` Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 6/9] drm/i915: Remove some post-commit members from intel_crtc->atomic, v2 Maarten Lankhorst
` (5 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:27 UTC (permalink / raw)
To: intel-gfx
Commit 791a32be6eb2 ("drm/i915: Drop intel_update_sprite_watermarks")
removes the use of this variable, but forgot to remove it.
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 6 +-----
drivers/gpu/drm/i915/intel_drv.h | 1 -
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8340b43aacd6..2d3bfbc72fae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11847,7 +11847,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
struct intel_plane_state *old_plane_state =
to_intel_plane_state(plane->state);
int idx = intel_crtc->base.base.id, ret;
- int i = drm_plane_index(plane);
bool mode_changed = needs_modeset(crtc_state);
bool was_crtc_enabled = crtc->state->active;
bool is_crtc_enabled = crtc_state->active;
@@ -11950,11 +11949,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
*/
if (IS_IVYBRIDGE(dev) &&
needs_scaling(to_intel_plane_state(plane_state)) &&
- !needs_scaling(old_plane_state)) {
+ !needs_scaling(old_plane_state))
pipe_config->disable_lp_wm = true;
- } else if (turn_off && !mode_changed)
- intel_crtc->atomic.update_sprite_watermarks |=
- 1 << i;
break;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f284c61bbae9..a032d95e3d5f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -571,7 +571,6 @@ struct intel_crtc_atomic_commit {
unsigned fb_bits;
bool update_fbc;
bool post_enable_primary;
- unsigned update_sprite_watermarks;
};
struct intel_crtc {
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 6/9] drm/i915: Remove some post-commit members from intel_crtc->atomic, v2.
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
` (4 preceding siblings ...)
2016-01-11 12:27 ` [PATCH v2 5/9] drm/i915: Remove update_sprite_watermarks Maarten Lankhorst
@ 2016-01-11 12:27 ` Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 7/9] drm/i915: Nuke fbc " Maarten Lankhorst
` (4 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:27 UTC (permalink / raw)
To: intel-gfx
fb_bits is useful to have in the crtc_state for cs flips when
the code is updated to use intel_frontbuffer_flip_prepare/complete.
So calculate it in advance and move it to crtc_state. The other stuff
can be calculated in post_plane_update, and aren't useful elsewhere.
Changes since v1:
- Changing wording, remove comment about loop.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
---
drivers/gpu/drm/i915/intel_atomic.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++++---------
drivers/gpu/drm/i915/intel_drv.h | 3 +--
3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index ba9a57f33c43..d6c362a42bd1 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -99,6 +99,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
crtc_state->wm_changed = false;
crtc_state->wm.need_postvbl_update = false;
crtc_state->fb_changed = false;
+ crtc_state->fb_bits = 0;
return &crtc_state->base;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2d3bfbc72fae..18c3e8d8f954 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4789,14 +4789,20 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
hsw_disable_ips(intel_crtc);
}
-static void intel_post_plane_update(struct intel_crtc *crtc)
+static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
{
+ struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
+ struct drm_atomic_state *old_state = old_crtc_state->base.state;
struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc->base.state);
struct drm_device *dev = crtc->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_plane *primary = crtc->base.primary;
+ struct drm_plane_state *old_pri_state =
+ drm_atomic_get_existing_plane_state(old_state, primary);
- intel_frontbuffer_flip(dev, atomic->fb_bits);
+ intel_frontbuffer_flip(dev, pipe_config->fb_bits);
crtc->wm.cxsr_allowed = true;
@@ -4806,8 +4812,17 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
if (atomic->update_fbc)
intel_fbc_update(crtc);
- if (atomic->post_enable_primary)
- intel_post_enable_primary(&crtc->base);
+ if (old_pri_state) {
+ struct intel_plane_state *primary_state =
+ to_intel_plane_state(primary->state);
+ struct intel_plane_state *old_primary_state =
+ to_intel_plane_state(old_pri_state);
+
+ if (primary_state->visible &&
+ (needs_modeset(&pipe_config->base) ||
+ !old_primary_state->visible))
+ intel_post_enable_primary(&crtc->base);
+ }
memset(atomic, 0, sizeof(*atomic));
}
@@ -11909,13 +11924,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
if (visible || was_visible)
- intel_crtc->atomic.fb_bits |=
- to_intel_plane(plane)->frontbuffer_bit;
+ pipe_config->fb_bits |= to_intel_plane(plane)->frontbuffer_bit;
switch (plane->type) {
case DRM_PLANE_TYPE_PRIMARY:
- intel_crtc->atomic.post_enable_primary = turn_on;
-
if (turn_off)
intel_crtc->atomic.disable_fbc = true;
@@ -13660,7 +13672,7 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
for_each_crtc_in_state(state, crtc, crtc_state, i)
- intel_post_plane_update(to_intel_crtc(crtc));
+ intel_post_plane_update(to_intel_crtc_state(crtc_state));
/*
* Now that the vblank has passed, we can go ahead and program the
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a032d95e3d5f..bcab327425b4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -382,6 +382,7 @@ struct intel_crtc_state {
#define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (1<<0) /* unreliable sync mode.flags */
unsigned long quirks;
+ unsigned fb_bits; /* framebuffers to flip */
bool update_pipe; /* can a fast modeset be performed? */
bool disable_cxsr;
bool wm_changed; /* watermarks are updated */
@@ -568,9 +569,7 @@ struct intel_crtc_atomic_commit {
bool disable_fbc;
/* Sleepable operations to perform after commit */
- unsigned fb_bits;
bool update_fbc;
- bool post_enable_primary;
};
struct intel_crtc {
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 7/9] drm/i915: Nuke fbc members from intel_crtc->atomic, v2.
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
` (5 preceding siblings ...)
2016-01-11 12:27 ` [PATCH v2 6/9] drm/i915: Remove some post-commit members from intel_crtc->atomic, v2 Maarten Lankhorst
@ 2016-01-11 12:27 ` Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 8/9] drm/i915: Do not compute watermarks on a noop Maarten Lankhorst
` (3 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:27 UTC (permalink / raw)
To: intel-gfx
Factor out intel_fbc_supports_rotation and use it in
pre_plane_update as well. This leaves intel_crtc->atomic
empty, so remove it too.
Changes since v1:
- Add a intel_fbc_supports_rotation helper.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 87 +++++++++++++-----------------------
drivers/gpu/drm/i915/intel_drv.h | 18 +-------
drivers/gpu/drm/i915/intel_fbc.c | 20 +++++++--
3 files changed, 51 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 18c3e8d8f954..c3aa1d5bd23f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4793,11 +4793,9 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
struct drm_atomic_state *old_state = old_crtc_state->base.state;
- struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc->base.state);
struct drm_device *dev = crtc->base.dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_plane *primary = crtc->base.primary;
struct drm_plane_state *old_pri_state =
drm_atomic_get_existing_plane_state(old_state, primary);
@@ -4809,22 +4807,20 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
if (pipe_config->wm_changed && pipe_config->base.active)
intel_update_watermarks(&crtc->base);
- if (atomic->update_fbc)
- intel_fbc_update(crtc);
-
if (old_pri_state) {
struct intel_plane_state *primary_state =
to_intel_plane_state(primary->state);
struct intel_plane_state *old_primary_state =
to_intel_plane_state(old_pri_state);
+ if (primary_state->visible)
+ intel_fbc_update(crtc);
+
if (primary_state->visible &&
(needs_modeset(&pipe_config->base) ||
!old_primary_state->visible))
intel_post_enable_primary(&crtc->base);
}
-
- memset(atomic, 0, sizeof(*atomic));
}
static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
@@ -4832,7 +4828,6 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc->base.state);
struct drm_atomic_state *old_state = old_crtc_state->base.state;
@@ -4841,18 +4836,32 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
drm_atomic_get_existing_plane_state(old_state, primary);
bool modeset = needs_modeset(&pipe_config->base);
- if (atomic->disable_fbc)
- intel_fbc_deactivate(crtc);
-
if (old_pri_state) {
struct intel_plane_state *primary_state =
to_intel_plane_state(primary->state);
struct intel_plane_state *old_primary_state =
to_intel_plane_state(old_pri_state);
+ bool turn_off = old_primary_state->visible &&
+ (modeset || !primary_state->visible);
+
+ if (turn_off) {
+ intel_fbc_deactivate(crtc);
- if (old_primary_state->visible &&
- (modeset || !primary_state->visible))
intel_pre_disable_primary(&crtc->base);
+ } else if (primary_state->visible &&
+ !intel_fbc_supports_rotation(dev_priv, primary_state)) {
+ /*
+ * 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 post plane update function except that that
+ * one is done too late.
+ */
+
+ intel_fbc_deactivate(crtc);
+ }
}
if (pipe_config->disable_cxsr) {
@@ -11926,46 +11935,17 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
if (visible || was_visible)
pipe_config->fb_bits |= to_intel_plane(plane)->frontbuffer_bit;
- switch (plane->type) {
- case DRM_PLANE_TYPE_PRIMARY:
- if (turn_off)
- intel_crtc->atomic.disable_fbc = 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 (visible &&
- INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
- dev_priv->fbc.crtc == intel_crtc &&
- plane_state->rotation != BIT(DRM_ROTATE_0))
- intel_crtc->atomic.disable_fbc = true;
-
- intel_crtc->atomic.update_fbc |= visible || mode_changed;
- break;
- case DRM_PLANE_TYPE_CURSOR:
- break;
- case DRM_PLANE_TYPE_OVERLAY:
- /*
- * WaCxSRDisabledForSpriteScaling:ivb
- *
- * cstate->update_wm was already set above, so this flag will
- * take effect when we commit and program watermarks.
- */
- if (IS_IVYBRIDGE(dev) &&
- needs_scaling(to_intel_plane_state(plane_state)) &&
- !needs_scaling(old_plane_state))
- pipe_config->disable_lp_wm = true;
+ /*
+ * WaCxSRDisabledForSpriteScaling:ivb
+ *
+ * cstate->update_wm was already set above, so this flag will
+ * take effect when we commit and program watermarks.
+ */
+ if (plane->type == DRM_PLANE_TYPE_OVERLAY && IS_IVYBRIDGE(dev) &&
+ needs_scaling(to_intel_plane_state(plane_state)) &&
+ !needs_scaling(old_plane_state))
+ pipe_config->disable_lp_wm = true;
- break;
- }
return 0;
}
@@ -13373,9 +13353,6 @@ static int intel_atomic_check(struct drm_device *dev,
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc_state);
- memset(&to_intel_crtc(crtc)->atomic, 0,
- sizeof(struct intel_crtc_atomic_commit));
-
/* Catch I915_MODE_FLAG_INHERITED */
if (crtc_state->mode.private_flags != crtc->state->mode.private_flags)
crtc_state->mode_changed = true;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bcab327425b4..ec89659e5d0d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -558,20 +558,6 @@ struct intel_mmio_flip {
unsigned int rotation;
};
-/*
- * 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 disable_fbc;
-
- /* Sleepable operations to perform after commit */
- bool update_fbc;
-};
-
struct intel_crtc {
struct drm_crtc base;
enum pipe pipe;
@@ -632,8 +618,6 @@ struct intel_crtc {
int scanline_start;
} debug;
- struct intel_crtc_atomic_commit atomic;
-
/* scalers available on this crtc */
int num_scalers;
@@ -1348,6 +1332,8 @@ static inline void intel_fbdev_restore_mode(struct drm_device *dev)
/* intel_fbc.c */
bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
+bool intel_fbc_supports_rotation(struct drm_i915_private *dev_priv,
+ const struct intel_plane_state *plane_state);
void intel_fbc_deactivate(struct intel_crtc *crtc);
void intel_fbc_update(struct intel_crtc *crtc);
void intel_fbc_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index a1988a486b92..7d123203ead6 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -733,6 +733,18 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
return effective_w <= max_w && effective_h <= max_h;
}
+bool intel_fbc_supports_rotation(struct drm_i915_private *dev_priv,
+ const struct intel_plane_state *plane_state)
+{
+ if (plane_state->base.rotation == BIT(DRM_ROTATE_0))
+ return true;
+
+ if (INTEL_INFO(dev_priv)->gen <= 4 && !IS_G4X(dev_priv))
+ return false;
+
+ return true;
+}
+
/**
* __intel_fbc_update - activate/deactivate FBC as needed, unlocked
* @crtc: the CRTC that triggered the update
@@ -746,6 +758,7 @@ static void __intel_fbc_update(struct intel_crtc *crtc)
struct drm_framebuffer *fb;
struct drm_i915_gem_object *obj;
const struct drm_display_mode *adjusted_mode;
+ const struct drm_plane_state *primary_state = crtc->base.primary->state;
WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock));
@@ -762,7 +775,7 @@ static void __intel_fbc_update(struct intel_crtc *crtc)
goto out_disable;
}
- fb = crtc->base.primary->fb;
+ fb = primary_state->fb;
obj = intel_fb_obj(fb);
adjusted_mode = &crtc->config->base.adjusted_mode;
@@ -785,8 +798,9 @@ static void __intel_fbc_update(struct intel_crtc *crtc)
set_no_fbc_reason(dev_priv, "framebuffer not tiled or fenced");
goto out_disable;
}
- if (INTEL_INFO(dev_priv)->gen <= 4 && !IS_G4X(dev_priv) &&
- crtc->base.primary->state->rotation != BIT(DRM_ROTATE_0)) {
+
+ if (!intel_fbc_supports_rotation(dev_priv,
+ to_intel_plane_state(primary_state))) {
set_no_fbc_reason(dev_priv, "rotation unsupported");
goto out_disable;
}
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 8/9] drm/i915: Do not compute watermarks on a noop.
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
` (6 preceding siblings ...)
2016-01-11 12:27 ` [PATCH v2 7/9] drm/i915: Nuke fbc " Maarten Lankhorst
@ 2016-01-11 12:27 ` Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 9/9] drm/i915: Remove vblank wait from hsw_enable_ips Maarten Lankhorst
` (2 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:27 UTC (permalink / raw)
To: intel-gfx
This should not be done this late when nothing changed.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 3 +-
drivers/gpu/drm/i915/intel_drv.h | 13 ++++++++
drivers/gpu/drm/i915/intel_pm.c | 59 +++++++++++++++++++++---------------
3 files changed, 49 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c3aa1d5bd23f..a8285fe429e1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12029,7 +12029,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
}
ret = 0;
- if (dev_priv->display.compute_pipe_wm) {
+ if (dev_priv->display.compute_pipe_wm &&
+ (mode_changed || pipe_config->update_pipe || crtc_state->planes_changed)) {
ret = dev_priv->display.compute_pipe_wm(intel_crtc, state);
if (ret) {
DRM_DEBUG_KMS("Target pipe watermarks are invalid\n");
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ec89659e5d0d..1fea7a1a3ed1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1606,6 +1606,19 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state,
return to_intel_crtc_state(crtc_state);
}
+
+static inline struct intel_plane_state *
+intel_atomic_get_existing_plane_state(struct drm_atomic_state *state,
+ struct intel_plane *plane)
+{
+ struct drm_plane_state *plane_state;
+
+ plane_state = drm_atomic_get_existing_plane_state(state, &plane->base);
+
+ return to_intel_plane_state(plane_state);
+}
+
+
int intel_atomic_setup_scalers(struct drm_device *dev,
struct intel_crtc *intel_crtc,
struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9df9e9a22f3c..482c684a116b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2005,11 +2005,18 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
cur_latency *= 5;
}
- result->pri_val = ilk_compute_pri_wm(cstate, pristate,
- pri_latency, level);
- result->spr_val = ilk_compute_spr_wm(cstate, sprstate, spr_latency);
- result->cur_val = ilk_compute_cur_wm(cstate, curstate, cur_latency);
- result->fbc_val = ilk_compute_fbc_wm(cstate, pristate, result->pri_val);
+ if (pristate) {
+ result->pri_val = ilk_compute_pri_wm(cstate, pristate,
+ pri_latency, level);
+ result->fbc_val = ilk_compute_fbc_wm(cstate, pristate, result->pri_val);
+ }
+
+ if (sprstate)
+ result->spr_val = ilk_compute_spr_wm(cstate, sprstate, spr_latency);
+
+ if (curstate)
+ result->cur_val = ilk_compute_cur_wm(cstate, curstate, cur_latency);
+
result->enable = true;
}
@@ -2305,7 +2312,6 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
const struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc_state *cstate = NULL;
struct intel_plane *intel_plane;
- struct drm_plane_state *ps;
struct intel_plane_state *pristate = NULL;
struct intel_plane_state *sprstate = NULL;
struct intel_plane_state *curstate = NULL;
@@ -2319,27 +2325,32 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
pipe_wm = &cstate->wm.optimal.ilk;
for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
- ps = drm_atomic_get_plane_state(state,
- &intel_plane->base);
- if (IS_ERR(ps))
- return PTR_ERR(ps);
+ struct intel_plane_state *ps;
+
+ ps = intel_atomic_get_existing_plane_state(state,
+ intel_plane);
+ if (!ps)
+ continue;
if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
- pristate = to_intel_plane_state(ps);
- else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
- sprstate = to_intel_plane_state(ps);
- else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
- curstate = to_intel_plane_state(ps);
+ pristate = ps;
+ else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) {
+ sprstate = ps;
+
+ if (ps) {
+ pipe_wm->sprites_enabled = sprstate->visible;
+ pipe_wm->sprites_scaled = sprstate->visible &&
+ (drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 ||
+ drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16);
+ }
+ } else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
+ curstate = ps;
}
pipe_wm->pipe_enabled = cstate->base.active;
- pipe_wm->sprites_enabled = sprstate->visible;
- pipe_wm->sprites_scaled = sprstate->visible &&
- (drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 ||
- drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16);
/* ILK/SNB: LP2+ watermarks only w/o sprites */
- if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible)
+ if (INTEL_INFO(dev)->gen <= 6 && pipe_wm->sprites_enabled)
max_level = 1;
/* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */
@@ -2358,20 +2369,18 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
ilk_compute_wm_reg_maximums(dev, 1, &max);
for (level = 1; level <= max_level; level++) {
- struct intel_wm_level wm = {};
+ struct intel_wm_level *wm = &pipe_wm->wm[level];
ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate,
- pristate, sprstate, curstate, &wm);
+ pristate, sprstate, curstate, wm);
/*
* Disable any watermark level that exceeds the
* register maximums since such watermarks are
* always invalid.
*/
- if (!ilk_validate_wm_level(level, &max, &wm))
+ if (!ilk_validate_wm_level(level, &max, wm))
break;
-
- pipe_wm->wm[level] = wm;
}
return 0;
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 9/9] drm/i915: Remove vblank wait from hsw_enable_ips.
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
` (7 preceding siblings ...)
2016-01-11 12:27 ` [PATCH v2 8/9] drm/i915: Do not compute watermarks on a noop Maarten Lankhorst
@ 2016-01-11 12:27 ` Maarten Lankhorst
2016-01-11 12:49 ` ✗ failure: Fi.CI.BAT Patchwork
2016-01-11 14:30 ` Patchwork
10 siblings, 0 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:27 UTC (permalink / raw)
To: intel-gfx
intel_post_plane_update did an extra vblank wait that's no longer needed when enabling ips.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a8285fe429e1..2cce347426cd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4580,9 +4580,6 @@ void hsw_enable_ips(struct intel_crtc *crtc)
if (!crtc->config->ips_enabled)
return;
- /* We can only enable IPS after we enable a plane and wait for a vblank */
- intel_wait_for_vblank(dev, crtc->pipe);
-
assert_plane_enabled(dev_priv, crtc->plane);
if (IS_BROADWELL(dev)) {
mutex_lock(&dev_priv->rps.hw_lock);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/9] drm/i915: Unify power domain handling.
2016-01-11 12:27 ` [PATCH v2 1/9] drm/i915: Unify power domain handling Maarten Lankhorst
@ 2016-01-11 12:43 ` Maarten Lankhorst
2016-01-11 13:31 ` [PATCH v2 0.999/9] drm/i915: Pass crtc state to modeset_get_crtc_power_domains Maarten Lankhorst
1 sibling, 0 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 12:43 UTC (permalink / raw)
To: intel-gfx
Op 11-01-16 om 13:27 schreef Maarten Lankhorst:
> Right now there's separate power domain handling for update_pipe and
> modesets. Unify this and only grab POWER_DOMAIN_MODESET once.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 64 ++++++++++++------------------------
> 1 file changed, 21 insertions(+), 43 deletions(-)
>
Uh oh, this patch can't work right now without atomic encoder_mask patches
and without passing crtc_state to modeset_get_crtc_power_domains.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* ✗ failure: Fi.CI.BAT
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
` (8 preceding siblings ...)
2016-01-11 12:27 ` [PATCH v2 9/9] drm/i915: Remove vblank wait from hsw_enable_ips Maarten Lankhorst
@ 2016-01-11 12:49 ` Patchwork
2016-01-11 14:30 ` Patchwork
10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2016-01-11 12:49 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Summary ==
Built on ff88655b3a5467bbc3be8c67d3e05ebf182557d3 drm-intel-nightly: 2016y-01m-11d-07h-30m-16s UTC integration manifest
Test core_auth:
Subgroup basic-auth:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test core_prop_blob:
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test drv_getparams_basic:
Subgroup basic-eu-total:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-subslice-total:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_basic:
Subgroup bad-close:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup create-close:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_cpu_reloc:
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_ctx_create:
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_ctx_param_basic:
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-default:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup invalid-ctx-get:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup invalid-ctx-set:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup invalid-param-set:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup non-root-set:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup non-root-set-no-zeromap:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup root-set-no-zeromap-disabled:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup root-set-no-zeromap-enabled:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_flink_basic:
Subgroup bad-open:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_linear_blits:
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_mmap:
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_mmap_gtt:
Subgroup basic-copy:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-read:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-read-write-distinct:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-small-bo:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-small-bo-tiledx:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-small-bo-tiledy:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-small-copy:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-small-copy-xy:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-write:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-write-gtt:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-write-no-prefault:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-write-read-distinct:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_pwrite:
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_render_linear_blits:
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_render_tiled_blits:
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test gem_storedw_loop:
Subgroup basic-blt:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-bsd:
pass -> SKIP (bdw-ultra)
Subgroup basic-render:
dmesg-warn -> PASS (bdw-ultra)
Subgroup basic-vebox:
pass -> SKIP (bdw-ultra)
Test kms_addfb_basic:
Subgroup addfb25-modifier-no-flag:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup addfb25-x-tiled:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup addfb25-x-tiled-mismatch:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup addfb25-y-tiled:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup addfb25-yf-tiled:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bad-pitch-0:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bad-pitch-1024:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bad-pitch-128:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bad-pitch-256:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bad-pitch-32:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bad-pitch-63:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bad-pitch-65536:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bad-pitch-999:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-y-tiled:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bo-too-small-due-to-tiling:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup clobberred-modifier:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup framebuffer-vs-set-tiling:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup no-handle:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup size-max:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup small-bo:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup tile-pitch-mismatch:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup too-high:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup too-wide:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup unused-handle:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup unused-modifier:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup unused-offsets:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup unused-pitches:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> DMESG-FAIL (bsw-nuc-2)
pass -> FAIL (snb-dellxps)
pass -> FAIL (bdw-ultra)
dmesg-warn -> PASS (ilk-hp8440p)
pass -> DMESG-FAIL (byt-nuc)
Subgroup basic-flip-vs-modeset:
dmesg-warn -> DMESG-FAIL (bsw-nuc-2)
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
pass -> DMESG-WARN (ilk-hp8440p)
dmesg-warn -> DMESG-FAIL (byt-nuc)
Subgroup basic-flip-vs-wf_vblank:
pass -> DMESG-FAIL (bsw-nuc-2)
dmesg-warn -> PASS (ivb-t430s)
pass -> FAIL (bdw-ultra)
pass -> DMESG-FAIL (byt-nuc)
dmesg-warn -> PASS (snb-x220t)
dmesg-warn -> FAIL (snb-dellxps)
dmesg-warn -> PASS (ilk-hp8440p)
Subgroup basic-plain-flip:
dmesg-warn -> DMESG-FAIL (byt-nuc)
dmesg-warn -> DMESG-FAIL (bsw-nuc-2)
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
dmesg-warn -> PASS (ivb-t430s)
Test kms_force_connector_basic:
Subgroup force-edid:
pass -> SKIP (snb-dellxps)
Subgroup prune-stale-modes:
pass -> SKIP (snb-dellxps)
Test kms_pipe_crc_basic:
Subgroup bad-nb-words-1:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bad-nb-words-3:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup bad-pipe:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup hang-read-crc-pipe-a:
pass -> FAIL (snb-dellxps)
pass -> FAIL (bdw-ultra)
pass -> DMESG-FAIL (byt-nuc)
Subgroup hang-read-crc-pipe-b:
pass -> FAIL (snb-dellxps)
pass -> FAIL (bdw-ultra)
pass -> DMESG-FAIL (byt-nuc)
Subgroup hang-read-crc-pipe-c:
pass -> FAIL (bsw-nuc-2)
pass -> FAIL (bdw-ultra)
Subgroup nonblocking-crc-pipe-a:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
pass -> FAIL (byt-nuc)
Subgroup nonblocking-crc-pipe-a-frame-sequence:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
pass -> FAIL (byt-nuc)
Subgroup nonblocking-crc-pipe-b:
dmesg-warn -> PASS (snb-x220t)
dmesg-warn -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
pass -> DMESG-FAIL (byt-nuc)
Subgroup nonblocking-crc-pipe-b-frame-sequence:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
pass -> DMESG-FAIL (byt-nuc)
Subgroup nonblocking-crc-pipe-c:
pass -> FAIL (bsw-nuc-2)
pass -> SKIP (bdw-ultra)
Subgroup nonblocking-crc-pipe-c-frame-sequence:
pass -> FAIL (bsw-nuc-2)
pass -> FAIL (bdw-ultra)
Subgroup read-crc-pipe-a:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
dmesg-warn -> FAIL (byt-nuc)
Subgroup read-crc-pipe-a-frame-sequence:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
dmesg-warn -> FAIL (byt-nuc)
Subgroup read-crc-pipe-b:
dmesg-warn -> DMESG-FAIL (byt-nuc)
dmesg-warn -> PASS (snb-x220t)
dmesg-warn -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
dmesg-fail -> PASS (ivb-t430s)
Subgroup read-crc-pipe-b-frame-sequence:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
dmesg-warn -> DMESG-FAIL (byt-nuc)
Subgroup read-crc-pipe-c:
dmesg-warn -> FAIL (bsw-nuc-2)
pass -> SKIP (bdw-ultra)
dmesg-fail -> PASS (ivb-t430s)
Subgroup read-crc-pipe-c-frame-sequence:
pass -> FAIL (bsw-nuc-2)
pass -> FAIL (bdw-ultra)
Subgroup suspend-read-crc-pipe-a:
pass -> SKIP (snb-dellxps)
pass -> DMESG-FAIL (byt-nuc)
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-FAIL (byt-nuc)
dmesg-warn -> PASS (snb-x220t)
dmesg-warn -> SKIP (snb-dellxps)
dmesg-warn -> PASS (ilk-hp8440p)
dmesg-fail -> PASS (ivb-t430s)
Subgroup suspend-read-crc-pipe-c:
pass -> DMESG-FAIL (bsw-nuc-2)
dmesg-fail -> PASS (ivb-t430s)
Test kms_psr_sink_crc:
Subgroup psr_basic:
pass -> SKIP (bdw-ultra)
Test kms_setmode:
Subgroup basic-clone-single-crtc:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-rte:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test pm_rps:
Subgroup basic-api:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Test prime_self_import:
Subgroup basic-llseek-bad:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-with_fd_dup:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-with_one_bo_two_files:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
Subgroup basic-with_two_bos:
pass -> SKIP (snb-dellxps)
pass -> SKIP (bdw-ultra)
bdw-nuci7 total:138 pass:129 dwarn:0 dfail:0 fail:0 skip:9
bdw-ultra total:138 pass:31 dwarn:0 dfail:1 fail:7 skip:99
bsw-nuc-2 total:141 pass:106 dwarn:1 dfail:5 fail:5 skip:24
byt-nuc total:141 pass:107 dwarn:3 dfail:12 fail:4 skip:15
hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7
hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4
ilk-hp8440p total:141 pass:103 dwarn:1 dfail:0 fail:0 skip:37
ivb-t430s total:135 pass:129 dwarn:0 dfail:0 fail:0 skip:6
snb-dellxps total:141 pass:31 dwarn:0 dfail:1 fail:4 skip:105
snb-x220t total:141 pass:127 dwarn:0 dfail:0 fail:1 skip:13
HANGED hsw-xps12 in igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a
HANGED skl-i5k-2 in igt@kms_pipe_crc_basic@hang-read-crc-pipe-a
HANGED skl-i7k-2 in igt@kms_pipe_crc_basic@hang-read-crc-pipe-a
Results at /archive/results/CI_IGT_test/Patchwork_1127/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0.999/9] drm/i915: Pass crtc state to modeset_get_crtc_power_domains.
2016-01-11 12:27 ` [PATCH v2 1/9] drm/i915: Unify power domain handling Maarten Lankhorst
2016-01-11 12:43 ` Maarten Lankhorst
@ 2016-01-11 13:31 ` Maarten Lankhorst
1 sibling, 0 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-11 13:31 UTC (permalink / raw)
To: intel-gfx
Use our newly created encoder_mask to iterate over the encoders.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
This depends on the atomic encoder_mask patches and is required for unifying power domain handling.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3fd6f1c157b8..1ac868d03d2c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5342,31 +5342,37 @@ intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder)
}
}
-static unsigned long get_crtc_power_domains(struct drm_crtc *crtc)
+static unsigned long get_crtc_power_domains(struct drm_crtc *crtc,
+ struct intel_crtc_state *crtc_state)
{
struct drm_device *dev = crtc->dev;
- struct intel_encoder *intel_encoder;
+ struct drm_encoder *encoder;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum pipe pipe = intel_crtc->pipe;
unsigned long mask;
- enum transcoder transcoder = intel_crtc->config->cpu_transcoder;
+ enum transcoder transcoder = crtc_state->cpu_transcoder;
- if (!crtc->state->active)
+ if (!crtc_state->base.active)
return 0;
mask = BIT(POWER_DOMAIN_PIPE(pipe));
mask |= BIT(POWER_DOMAIN_TRANSCODER(transcoder));
- if (intel_crtc->config->pch_pfit.enabled ||
- intel_crtc->config->pch_pfit.force_thru)
+ if (crtc_state->pch_pfit.enabled ||
+ crtc_state->pch_pfit.force_thru)
mask |= BIT(POWER_DOMAIN_PIPE_PANEL_FITTER(pipe));
- for_each_encoder_on_crtc(dev, crtc, intel_encoder)
+ drm_for_each_encoder_mask(encoder, dev, crtc_state->base.encoder_mask) {
+ struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
+
mask |= BIT(intel_display_port_power_domain(intel_encoder));
+ }
return mask;
}
-static unsigned long modeset_get_crtc_power_domains(struct drm_crtc *crtc)
+static unsigned long
+modeset_get_crtc_power_domains(struct drm_crtc *crtc,
+ struct intel_crtc_state *crtc_state)
{
struct drm_i915_private *dev_priv = crtc->dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -5374,7 +5380,8 @@ static unsigned long modeset_get_crtc_power_domains(struct drm_crtc *crtc)
unsigned long domains, new_domains, old_domains;
old_domains = intel_crtc->enabled_power_domains;
- intel_crtc->enabled_power_domains = new_domains = get_crtc_power_domains(crtc);
+ intel_crtc->enabled_power_domains = new_domains =
+ get_crtc_power_domains(crtc, crtc_state);
domains = new_domains & ~old_domains;
@@ -5406,7 +5413,8 @@ static void modeset_update_crtc_power_domains(struct drm_atomic_state *state)
for_each_crtc_in_state(state, crtc, crtc_state, i) {
if (needs_modeset(crtc->state))
put_domains[to_intel_crtc(crtc)->pipe] =
- modeset_get_crtc_power_domains(crtc);
+ modeset_get_crtc_power_domains(crtc,
+ to_intel_crtc_state(crtc->state));
}
if (dev_priv->display.modeset_commit_cdclk &&
@@ -13632,7 +13640,8 @@ static int intel_atomic_commit(struct drm_device *dev,
}
if (update_pipe) {
- put_domains = modeset_get_crtc_power_domains(crtc);
+ put_domains = modeset_get_crtc_power_domains(crtc,
+ to_intel_crtc_state(crtc->state));
/* make sure intel_modeset_check_state runs */
hw_check = true;
@@ -15989,7 +15998,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
for_each_intel_crtc(dev, crtc) {
unsigned long put_domains;
- put_domains = modeset_get_crtc_power_domains(&crtc->base);
+ put_domains = modeset_get_crtc_power_domains(&crtc->base, crtc->config);
if (WARN_ON(put_domains))
modeset_put_power_domains(dev_priv, put_domains);
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* ✗ failure: Fi.CI.BAT
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
` (9 preceding siblings ...)
2016-01-11 12:49 ` ✗ failure: Fi.CI.BAT Patchwork
@ 2016-01-11 14:30 ` Patchwork
10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2016-01-11 14:30 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Summary ==
HEAD is now at ff88655 drm-intel-nightly: 2016y-01m-11d-07h-30m-16s UTC integration manifest
Applying: drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3.
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3.
2016-01-11 12:27 ` [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3 Maarten Lankhorst
@ 2016-01-13 12:58 ` Ville Syrjälä
2016-02-09 10:27 ` Maarten Lankhorst
0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2016-01-13 12:58 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Mon, Jan 11, 2016 at 01:27:42PM +0100, Maarten Lankhorst wrote:
> Currently we perform our own wait in post_plane_update,
> but the atomic core performs another one in wait_for_vblanks.
> This means that 2 vblanks are done when a fb is changed,
> which is a bit overkill.
>
> Merge them by creating a helper function that takes a crtc mask
> for the planes to wait on.
>
> The broadwell vblank workaround may look gone entirely but this is
> not the case. pipe_config->wm_changed is set to true
> when any plane is turned on, which forces a vblank wait.
Still NAK, because you just removed the comment entirely.
>
> Changes since v1:
> - Removing the double vblank wait on broadwell moved to its own commit.
> Changes since v2:
> - Move out POWER_DOMAIN_MODESET handling to its own commit.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 1 +
> drivers/gpu/drm/i915/intel_display.c | 80 ++++++++++++++++++++++++++----------
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> 3 files changed, 61 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 9682d94af710..ba9a57f33c43 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -98,6 +98,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
> crtc_state->disable_cxsr = false;
> crtc_state->wm_changed = false;
> crtc_state->wm.need_postvbl_update = false;
> + crtc_state->fb_changed = false;
>
> return &crtc_state->base;
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2aa1c5367625..eac73f005a70 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4796,9 +4796,6 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> to_intel_crtc_state(crtc->base.state);
> struct drm_device *dev = crtc->base.dev;
>
> - if (atomic->wait_vblank)
> - intel_wait_for_vblank(dev, crtc->pipe);
> -
> intel_frontbuffer_flip(dev, atomic->fb_bits);
>
> crtc->wm.cxsr_allowed = true;
> @@ -11872,6 +11869,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> if (!was_visible && !visible)
> return 0;
>
> + if (fb != old_plane_state->base.fb)
> + pipe_config->fb_changed = true;
Isn't that going to slow down cursor updates once again?
> +
> turn_off = was_visible && (!visible || mode_changed);
> turn_on = visible && (!was_visible || mode_changed);
>
> @@ -11887,8 +11887,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>
> /* must disable cxsr around plane enable/disable */
> if (plane->type != DRM_PLANE_TYPE_CURSOR) {
> - if (is_crtc_enabled)
> - intel_crtc->atomic.wait_vblank = true;
> pipe_config->disable_cxsr = true;
> }
> } else if (intel_wm_need_update(plane, plane_state)) {
> @@ -11940,14 +11938,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> plane_state->rotation != BIT(DRM_ROTATE_0))
> intel_crtc->atomic.disable_fbc = true;
>
> - /*
> - * 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 (turn_on && IS_BROADWELL(dev))
> - intel_crtc->atomic.wait_vblank = true;
> -
> intel_crtc->atomic.update_fbc |= visible || mode_changed;
> break;
> case DRM_PLANE_TYPE_CURSOR:
> @@ -11962,12 +11952,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> if (IS_IVYBRIDGE(dev) &&
> needs_scaling(to_intel_plane_state(plane_state)) &&
> !needs_scaling(old_plane_state)) {
> - to_intel_crtc_state(crtc_state)->disable_lp_wm = true;
> - } else if (turn_off && !mode_changed) {
> - intel_crtc->atomic.wait_vblank = true;
> + pipe_config->disable_lp_wm = true;
> + } else if (turn_off && !mode_changed)
> intel_crtc->atomic.update_sprite_watermarks |=
> 1 << i;
> - }
>
> break;
> }
> @@ -13509,6 +13497,48 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
> return ret;
> }
>
> +static void intel_atomic_wait_for_vblanks(struct drm_device *dev,
> + struct drm_i915_private *dev_priv,
> + unsigned crtc_mask)
> +{
> + unsigned last_vblank_count[I915_MAX_PIPES];
> + enum pipe pipe;
> + int ret;
> +
> + if (!crtc_mask)
> + return;
> +
> + for_each_pipe(dev_priv, pipe) {
> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +
> + if (!((1 << pipe) & crtc_mask))
> + continue;
> +
> + ret = drm_crtc_vblank_get(crtc);
> + if (ret != 0) {
> + crtc_mask &= ~(1 << pipe);
> + continue;
> + }
> +
> + last_vblank_count[pipe] = drm_crtc_vblank_count(crtc);
> + }
> +
> + for_each_pipe(dev_priv, pipe) {
> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +
> + if (!((1 << pipe) & crtc_mask))
> + continue;
> +
> + wait_event_timeout(dev->vblank[pipe].queue,
> + last_vblank_count[pipe] !=
> + drm_crtc_vblank_count(crtc),
> + msecs_to_jiffies(50));
> +
> + drm_crtc_vblank_put(crtc);
> + }
> +}
> +
> +
> /**
> * intel_atomic_commit - commit validated state object
> * @dev: DRM device
> @@ -13537,6 +13567,7 @@ static int intel_atomic_commit(struct drm_device *dev,
> int ret = 0, i;
> bool hw_check = intel_state->modeset;
> unsigned long put_domains[I915_MAX_PIPES] = {};
> + unsigned crtc_vblank_mask = 0;
>
> ret = intel_atomic_prepare_commit(dev, state, async);
> if (ret) {
> @@ -13608,8 +13639,9 @@ static int intel_atomic_commit(struct drm_device *dev,
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> bool modeset = needs_modeset(crtc->state);
> - bool update_pipe = !modeset &&
> - to_intel_crtc_state(crtc->state)->update_pipe;
> + struct intel_crtc_state *pipe_config =
> + to_intel_crtc_state(crtc->state);
> + bool update_pipe = !modeset && pipe_config->update_pipe;
>
> if (modeset && crtc->state->active) {
> update_scanline_offset(to_intel_crtc(crtc));
> @@ -13623,12 +13655,18 @@ static int intel_atomic_commit(struct drm_device *dev,
> (crtc->state->planes_changed || update_pipe))
> drm_atomic_helper_commit_planes_on_crtc(crtc_state);
>
> - intel_post_plane_update(intel_crtc);
> + if (pipe_config->base.active &&
> + (pipe_config->wm_changed || pipe_config->disable_cxsr ||
> + pipe_config->fb_changed))
> + crtc_vblank_mask |= 1 << i;
> }
>
> /* FIXME: add subpixel order */
>
> - drm_atomic_helper_wait_for_vblanks(dev, state);
> + intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
> +
> + for_each_crtc_in_state(state, crtc, crtc_state, i)
> + intel_post_plane_update(to_intel_crtc(crtc));
>
> /*
> * Now that the vblank has passed, we can go ahead and program the
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bdfe4035e074..8940aa4cf50c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -385,6 +385,7 @@ struct intel_crtc_state {
> bool update_pipe; /* can a fast modeset be performed? */
> bool disable_cxsr;
> bool wm_changed; /* watermarks are updated */
> + bool fb_changed; /* fb on any of the planes is changed */
>
> /* Pipe source size (ie. panel fitter input size)
> * All planes will be positioned inside this space,
> @@ -570,7 +571,6 @@ struct intel_crtc_atomic_commit {
>
> /* Sleepable operations to perform after commit */
> unsigned fb_bits;
> - bool wait_vblank;
> bool update_fbc;
> bool post_enable_primary;
> unsigned update_sprite_watermarks;
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/9] drm/i915: Remove intel_crtc->atomic.disable_ips.
2016-01-11 12:27 ` [PATCH v2 3/9] drm/i915: Remove intel_crtc->atomic.disable_ips Maarten Lankhorst
@ 2016-01-13 13:02 ` Ville Syrjälä
2016-01-18 12:10 ` Maarten Lankhorst
0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2016-01-13 13:02 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Mon, Jan 11, 2016 at 01:27:43PM +0100, Maarten Lankhorst wrote:
> This is a revert of commit 066cf55b9ce3 "drm/i915: Fix IPS related flicker".
> intel_pre_disable_primary already handles this, and now everything
> goes through the atomic path there's no need to try to disable ips twice.
If it's a revert why wasn't it done with 'git revert'?
Also I'm not sure it isn't a step backwards. Based on the spec we should
be able to keep IPS enabled as long as one plane (possibly referring to
either primary or sprite) is enabled on the pipe. So in theory we should
keep the IPS handling out of the primary plane code, and instead we
should compute the IPS state based on the set of active planes on the
pipe.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 16 +---------------
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> 2 files changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index eac73f005a70..62044ad5c6ec 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4823,9 +4823,6 @@ static void intel_pre_plane_update(struct intel_crtc *crtc)
> if (atomic->disable_fbc)
> intel_fbc_deactivate(crtc);
>
> - if (crtc->atomic.disable_ips)
> - hsw_disable_ips(crtc);
> -
> if (atomic->pre_disable_primary)
> intel_pre_disable_primary(&crtc->base);
>
> @@ -11907,19 +11904,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> intel_crtc->atomic.pre_disable_primary = turn_off;
> intel_crtc->atomic.post_enable_primary = turn_on;
>
> - if (turn_off) {
> - /*
> - * FIXME: Actually if we will still have any other
> - * plane enabled on the pipe we could let IPS enabled
> - * still, but for now lets consider that when we make
> - * primary invisible by setting DSPCNTR to 0 on
> - * update_primary_plane function IPS needs to be
> - * disable.
> - */
> - intel_crtc->atomic.disable_ips = true;
> -
> + if (turn_off)
> intel_crtc->atomic.disable_fbc = true;
> - }
>
> /*
> * FBC does not work on some platforms for rotated
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8940aa4cf50c..39adf7cd0b36 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -566,7 +566,6 @@ struct intel_mmio_flip {
> struct intel_crtc_atomic_commit {
> /* Sleepable operations to perform before commit */
> bool disable_fbc;
> - bool disable_ips;
> bool pre_disable_primary;
>
> /* Sleepable operations to perform after commit */
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/9] drm/i915: Remove intel_crtc->atomic.disable_ips.
2016-01-13 13:02 ` Ville Syrjälä
@ 2016-01-18 12:10 ` Maarten Lankhorst
2016-01-18 13:27 ` Daniel Stone
0 siblings, 1 reply; 22+ messages in thread
From: Maarten Lankhorst @ 2016-01-18 12:10 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 13-01-16 om 14:02 schreef Ville Syrjälä:
> On Mon, Jan 11, 2016 at 01:27:43PM +0100, Maarten Lankhorst wrote:
>> This is a revert of commit 066cf55b9ce3 "drm/i915: Fix IPS related flicker".
>> intel_pre_disable_primary already handles this, and now everything
>> goes through the atomic path there's no need to try to disable ips twice.
> If it's a revert why wasn't it done with 'git revert'?
A straight revert would fail to apply.
> Also I'm not sure it isn't a step backwards. Based on the spec we should
> be able to keep IPS enabled as long as one plane (possibly referring to
> either primary or sprite) is enabled on the pipe. So in theory we should
> keep the IPS handling out of the primary plane code, and instead we
> should compute the IPS state based on the set of active planes on the
> pipe.
You're probably right. But pre_disable_primary has the following warning:
* FIXME IPS should be fine as long as one plane is
* enabled, but in practice it seems to have problems
* when going from primary only to sprite only and vice
* versa.
Hence I'm not doing anything to support that. Even if I did it should be a separate commit.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/9] drm/i915: Remove intel_crtc->atomic.disable_ips.
2016-01-18 12:10 ` Maarten Lankhorst
@ 2016-01-18 13:27 ` Daniel Stone
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Stone @ 2016-01-18 13:27 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
Hi,
On 18 January 2016 at 12:10, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
> Op 13-01-16 om 14:02 schreef Ville Syrjälä:
>> Also I'm not sure it isn't a step backwards. Based on the spec we should
>> be able to keep IPS enabled as long as one plane (possibly referring to
>> either primary or sprite) is enabled on the pipe. So in theory we should
>> keep the IPS handling out of the primary plane code, and instead we
>> should compute the IPS state based on the set of active planes on the
>> pipe.
> You're probably right. But pre_disable_primary has the following warning:
> * FIXME IPS should be fine as long as one plane is
> * enabled, but in practice it seems to have problems
> * when going from primary only to sprite only and vice
> * versa.
>
> Hence I'm not doing anything to support that. Even if I did it should be a separate commit.
Right. I don't think it's worth trying to bandaid the current IPS
setup to deal with that, but instead - as per discussion on the last
round of patches - moving from 'if we're doing thing X then bash Y
into registers' to calculating desired targets based on the state
given.
Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3.
2016-01-13 12:58 ` Ville Syrjälä
@ 2016-02-09 10:27 ` Maarten Lankhorst
2016-02-09 10:42 ` Ville Syrjälä
0 siblings, 1 reply; 22+ messages in thread
From: Maarten Lankhorst @ 2016-02-09 10:27 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Hey,
Op 13-01-16 om 13:58 schreef Ville Syrjälä:
> On Mon, Jan 11, 2016 at 01:27:42PM +0100, Maarten Lankhorst wrote:
>> Currently we perform our own wait in post_plane_update,
>> but the atomic core performs another one in wait_for_vblanks.
>> This means that 2 vblanks are done when a fb is changed,
>> which is a bit overkill.
>>
>> Merge them by creating a helper function that takes a crtc mask
>> for the planes to wait on.
>>
>> The broadwell vblank workaround may look gone entirely but this is
>> not the case. pipe_config->wm_changed is set to true
>> when any plane is turned on, which forces a vblank wait.
> Still NAK, because you just removed the comment entirely.
I may have removed the comment but there will always be an unconditional wait because of the wm changes.
In some future commit I will rework do_intel_finish_page_flip to deal with atomic, and in that function the comment
will be useful again.
>> Changes since v1:
>> - Removing the double vblank wait on broadwell moved to its own commit.
>> Changes since v2:
>> - Move out POWER_DOMAIN_MODESET handling to its own commit.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_atomic.c | 1 +
>> drivers/gpu/drm/i915/intel_display.c | 80 ++++++++++++++++++++++++++----------
>> drivers/gpu/drm/i915/intel_drv.h | 2 +-
>> 3 files changed, 61 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>> index 9682d94af710..ba9a57f33c43 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -98,6 +98,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>> crtc_state->disable_cxsr = false;
>> crtc_state->wm_changed = false;
>> crtc_state->wm.need_postvbl_update = false;
>> + crtc_state->fb_changed = false;
>>
>> return &crtc_state->base;
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 2aa1c5367625..eac73f005a70 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4796,9 +4796,6 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
>> to_intel_crtc_state(crtc->base.state);
>> struct drm_device *dev = crtc->base.dev;
>>
>> - if (atomic->wait_vblank)
>> - intel_wait_for_vblank(dev, crtc->pipe);
>> -
>> intel_frontbuffer_flip(dev, atomic->fb_bits);
>>
>> crtc->wm.cxsr_allowed = true;
>> @@ -11872,6 +11869,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>> if (!was_visible && !visible)
>> return 0;
>>
>> + if (fb != old_plane_state->base.fb)
>> + pipe_config->fb_changed = true;
> Isn't that going to slow down cursor updates once again?
Very likely.. Shall I add a if (!state->legacy_cursor_update) to intel_atomic_wait_for_vblanks ?
>> +
>> turn_off = was_visible && (!visible || mode_changed);
>> turn_on = visible && (!was_visible || mode_changed);
>>
>> @@ -11887,8 +11887,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>
>> /* must disable cxsr around plane enable/disable */
>> if (plane->type != DRM_PLANE_TYPE_CURSOR) {
>> - if (is_crtc_enabled)
>> - intel_crtc->atomic.wait_vblank = true;
>> pipe_config->disable_cxsr = true;
>> }
>> } else if (intel_wm_need_update(plane, plane_state)) {
>> @@ -11940,14 +11938,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>> plane_state->rotation != BIT(DRM_ROTATE_0))
>> intel_crtc->atomic.disable_fbc = true;
>>
>> - /*
>> - * 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 (turn_on && IS_BROADWELL(dev))
>> - intel_crtc->atomic.wait_vblank = true;
>> -
>> intel_crtc->atomic.update_fbc |= visible || mode_changed;
>> break;
>> case DRM_PLANE_TYPE_CURSOR:
>> @@ -11962,12 +11952,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>> if (IS_IVYBRIDGE(dev) &&
>> needs_scaling(to_intel_plane_state(plane_state)) &&
>> !needs_scaling(old_plane_state)) {
>> - to_intel_crtc_state(crtc_state)->disable_lp_wm = true;
>> - } else if (turn_off && !mode_changed) {
>> - intel_crtc->atomic.wait_vblank = true;
>> + pipe_config->disable_lp_wm = true;
>> + } else if (turn_off && !mode_changed)
>> intel_crtc->atomic.update_sprite_watermarks |=
>> 1 << i;
>> - }
>>
>> break;
>> }
>> @@ -13509,6 +13497,48 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
>> return ret;
>> }
>>
>> +static void intel_atomic_wait_for_vblanks(struct drm_device *dev,
>> + struct drm_i915_private *dev_priv,
>> + unsigned crtc_mask)
>> +{
>> + unsigned last_vblank_count[I915_MAX_PIPES];
>> + enum pipe pipe;
>> + int ret;
>> +
>> + if (!crtc_mask)
>> + return;
>> +
>> + for_each_pipe(dev_priv, pipe) {
>> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>> +
>> + if (!((1 << pipe) & crtc_mask))
>> + continue;
>> +
>> + ret = drm_crtc_vblank_get(crtc);
>> + if (ret != 0) {
>> + crtc_mask &= ~(1 << pipe);
>> + continue;
>> + }
>> +
>> + last_vblank_count[pipe] = drm_crtc_vblank_count(crtc);
>> + }
>> +
>> + for_each_pipe(dev_priv, pipe) {
>> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>> +
>> + if (!((1 << pipe) & crtc_mask))
>> + continue;
>> +
>> + wait_event_timeout(dev->vblank[pipe].queue,
>> + last_vblank_count[pipe] !=
>> + drm_crtc_vblank_count(crtc),
>> + msecs_to_jiffies(50));
>> +
>> + drm_crtc_vblank_put(crtc);
>> + }
>> +}
>> +
>> +
>> /**
>> * intel_atomic_commit - commit validated state object
>> * @dev: DRM device
>> @@ -13537,6 +13567,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>> int ret = 0, i;
>> bool hw_check = intel_state->modeset;
>> unsigned long put_domains[I915_MAX_PIPES] = {};
>> + unsigned crtc_vblank_mask = 0;
>>
>> ret = intel_atomic_prepare_commit(dev, state, async);
>> if (ret) {
>> @@ -13608,8 +13639,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>> for_each_crtc_in_state(state, crtc, crtc_state, i) {
>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> bool modeset = needs_modeset(crtc->state);
>> - bool update_pipe = !modeset &&
>> - to_intel_crtc_state(crtc->state)->update_pipe;
>> + struct intel_crtc_state *pipe_config =
>> + to_intel_crtc_state(crtc->state);
>> + bool update_pipe = !modeset && pipe_config->update_pipe;
>>
>> if (modeset && crtc->state->active) {
>> update_scanline_offset(to_intel_crtc(crtc));
>> @@ -13623,12 +13655,18 @@ static int intel_atomic_commit(struct drm_device *dev,
>> (crtc->state->planes_changed || update_pipe))
>> drm_atomic_helper_commit_planes_on_crtc(crtc_state);
>>
>> - intel_post_plane_update(intel_crtc);
>> + if (pipe_config->base.active &&
>> + (pipe_config->wm_changed || pipe_config->disable_cxsr ||
>> + pipe_config->fb_changed))
>> + crtc_vblank_mask |= 1 << i;
>> }
>>
>> /* FIXME: add subpixel order */
>>
>> - drm_atomic_helper_wait_for_vblanks(dev, state);
>> + intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
>> +
>> + for_each_crtc_in_state(state, crtc, crtc_state, i)
>> + intel_post_plane_update(to_intel_crtc(crtc));
>>
>> /*
>> * Now that the vblank has passed, we can go ahead and program the
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index bdfe4035e074..8940aa4cf50c 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -385,6 +385,7 @@ struct intel_crtc_state {
>> bool update_pipe; /* can a fast modeset be performed? */
>> bool disable_cxsr;
>> bool wm_changed; /* watermarks are updated */
>> + bool fb_changed; /* fb on any of the planes is changed */
>>
>> /* Pipe source size (ie. panel fitter input size)
>> * All planes will be positioned inside this space,
>> @@ -570,7 +571,6 @@ struct intel_crtc_atomic_commit {
>>
>> /* Sleepable operations to perform after commit */
>> unsigned fb_bits;
>> - bool wait_vblank;
>> bool update_fbc;
>> bool post_enable_primary;
>> unsigned update_sprite_watermarks;
>> --
>> 2.1.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3.
2016-02-09 10:27 ` Maarten Lankhorst
@ 2016-02-09 10:42 ` Ville Syrjälä
2016-02-09 11:07 ` Maarten Lankhorst
0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2016-02-09 10:42 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Tue, Feb 09, 2016 at 11:27:44AM +0100, Maarten Lankhorst wrote:
> Hey,
>
> Op 13-01-16 om 13:58 schreef Ville Syrjälä:
> > On Mon, Jan 11, 2016 at 01:27:42PM +0100, Maarten Lankhorst wrote:
> >> Currently we perform our own wait in post_plane_update,
> >> but the atomic core performs another one in wait_for_vblanks.
> >> This means that 2 vblanks are done when a fb is changed,
> >> which is a bit overkill.
> >>
> >> Merge them by creating a helper function that takes a crtc mask
> >> for the planes to wait on.
> >>
> >> The broadwell vblank workaround may look gone entirely but this is
> >> not the case. pipe_config->wm_changed is set to true
> >> when any plane is turned on, which forces a vblank wait.
> > Still NAK, because you just removed the comment entirely.
> I may have removed the comment but there will always be an unconditional wait because of the wm changes.
> In some future commit I will rework do_intel_finish_page_flip to deal with atomic, and in that function the comment
> will be useful again.
The comment is the spec here, so it should be kept. Actually what I
really want is to stop using the flip done interrupt entirely since
it's arguably broken, at which point the comment should problably be
moved to somewhere else (interrupt setup code, flip completion code,
etc.). But removing the comment would be bad since then people might
not understand why we do it the way we do.
> >> Changes since v1:
> >> - Removing the double vblank wait on broadwell moved to its own commit.
> >> Changes since v2:
> >> - Move out POWER_DOMAIN_MODESET handling to its own commit.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_atomic.c | 1 +
> >> drivers/gpu/drm/i915/intel_display.c | 80 ++++++++++++++++++++++++++----------
> >> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> >> 3 files changed, 61 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> >> index 9682d94af710..ba9a57f33c43 100644
> >> --- a/drivers/gpu/drm/i915/intel_atomic.c
> >> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> >> @@ -98,6 +98,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
> >> crtc_state->disable_cxsr = false;
> >> crtc_state->wm_changed = false;
> >> crtc_state->wm.need_postvbl_update = false;
> >> + crtc_state->fb_changed = false;
> >>
> >> return &crtc_state->base;
> >> }
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 2aa1c5367625..eac73f005a70 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -4796,9 +4796,6 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> >> to_intel_crtc_state(crtc->base.state);
> >> struct drm_device *dev = crtc->base.dev;
> >>
> >> - if (atomic->wait_vblank)
> >> - intel_wait_for_vblank(dev, crtc->pipe);
> >> -
> >> intel_frontbuffer_flip(dev, atomic->fb_bits);
> >>
> >> crtc->wm.cxsr_allowed = true;
> >> @@ -11872,6 +11869,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >> if (!was_visible && !visible)
> >> return 0;
> >>
> >> + if (fb != old_plane_state->base.fb)
> >> + pipe_config->fb_changed = true;
> > Isn't that going to slow down cursor updates once again?
> Very likely.. Shall I add a if (!state->legacy_cursor_update) to intel_atomic_wait_for_vblanks ?
Not sure. Still wishing we could have just had the proper fps>=vrefresh
support fron the start.
> >> +
> >> turn_off = was_visible && (!visible || mode_changed);
> >> turn_on = visible && (!was_visible || mode_changed);
> >>
> >> @@ -11887,8 +11887,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >>
> >> /* must disable cxsr around plane enable/disable */
> >> if (plane->type != DRM_PLANE_TYPE_CURSOR) {
> >> - if (is_crtc_enabled)
> >> - intel_crtc->atomic.wait_vblank = true;
> >> pipe_config->disable_cxsr = true;
> >> }
> >> } else if (intel_wm_need_update(plane, plane_state)) {
> >> @@ -11940,14 +11938,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >> plane_state->rotation != BIT(DRM_ROTATE_0))
> >> intel_crtc->atomic.disable_fbc = true;
> >>
> >> - /*
> >> - * 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 (turn_on && IS_BROADWELL(dev))
> >> - intel_crtc->atomic.wait_vblank = true;
> >> -
> >> intel_crtc->atomic.update_fbc |= visible || mode_changed;
> >> break;
> >> case DRM_PLANE_TYPE_CURSOR:
> >> @@ -11962,12 +11952,10 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >> if (IS_IVYBRIDGE(dev) &&
> >> needs_scaling(to_intel_plane_state(plane_state)) &&
> >> !needs_scaling(old_plane_state)) {
> >> - to_intel_crtc_state(crtc_state)->disable_lp_wm = true;
> >> - } else if (turn_off && !mode_changed) {
> >> - intel_crtc->atomic.wait_vblank = true;
> >> + pipe_config->disable_lp_wm = true;
> >> + } else if (turn_off && !mode_changed)
> >> intel_crtc->atomic.update_sprite_watermarks |=
> >> 1 << i;
> >> - }
> >>
> >> break;
> >> }
> >> @@ -13509,6 +13497,48 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
> >> return ret;
> >> }
> >>
> >> +static void intel_atomic_wait_for_vblanks(struct drm_device *dev,
> >> + struct drm_i915_private *dev_priv,
> >> + unsigned crtc_mask)
> >> +{
> >> + unsigned last_vblank_count[I915_MAX_PIPES];
> >> + enum pipe pipe;
> >> + int ret;
> >> +
> >> + if (!crtc_mask)
> >> + return;
> >> +
> >> + for_each_pipe(dev_priv, pipe) {
> >> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> >> +
> >> + if (!((1 << pipe) & crtc_mask))
> >> + continue;
> >> +
> >> + ret = drm_crtc_vblank_get(crtc);
> >> + if (ret != 0) {
> >> + crtc_mask &= ~(1 << pipe);
> >> + continue;
> >> + }
> >> +
> >> + last_vblank_count[pipe] = drm_crtc_vblank_count(crtc);
> >> + }
> >> +
> >> + for_each_pipe(dev_priv, pipe) {
> >> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> >> +
> >> + if (!((1 << pipe) & crtc_mask))
> >> + continue;
> >> +
> >> + wait_event_timeout(dev->vblank[pipe].queue,
> >> + last_vblank_count[pipe] !=
> >> + drm_crtc_vblank_count(crtc),
> >> + msecs_to_jiffies(50));
> >> +
> >> + drm_crtc_vblank_put(crtc);
> >> + }
> >> +}
> >> +
> >> +
> >> /**
> >> * intel_atomic_commit - commit validated state object
> >> * @dev: DRM device
> >> @@ -13537,6 +13567,7 @@ static int intel_atomic_commit(struct drm_device *dev,
> >> int ret = 0, i;
> >> bool hw_check = intel_state->modeset;
> >> unsigned long put_domains[I915_MAX_PIPES] = {};
> >> + unsigned crtc_vblank_mask = 0;
> >>
> >> ret = intel_atomic_prepare_commit(dev, state, async);
> >> if (ret) {
> >> @@ -13608,8 +13639,9 @@ static int intel_atomic_commit(struct drm_device *dev,
> >> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> >> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >> bool modeset = needs_modeset(crtc->state);
> >> - bool update_pipe = !modeset &&
> >> - to_intel_crtc_state(crtc->state)->update_pipe;
> >> + struct intel_crtc_state *pipe_config =
> >> + to_intel_crtc_state(crtc->state);
> >> + bool update_pipe = !modeset && pipe_config->update_pipe;
> >>
> >> if (modeset && crtc->state->active) {
> >> update_scanline_offset(to_intel_crtc(crtc));
> >> @@ -13623,12 +13655,18 @@ static int intel_atomic_commit(struct drm_device *dev,
> >> (crtc->state->planes_changed || update_pipe))
> >> drm_atomic_helper_commit_planes_on_crtc(crtc_state);
> >>
> >> - intel_post_plane_update(intel_crtc);
> >> + if (pipe_config->base.active &&
> >> + (pipe_config->wm_changed || pipe_config->disable_cxsr ||
> >> + pipe_config->fb_changed))
> >> + crtc_vblank_mask |= 1 << i;
> >> }
> >>
> >> /* FIXME: add subpixel order */
> >>
> >> - drm_atomic_helper_wait_for_vblanks(dev, state);
> >> + intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
> >> +
> >> + for_each_crtc_in_state(state, crtc, crtc_state, i)
> >> + intel_post_plane_update(to_intel_crtc(crtc));
> >>
> >> /*
> >> * Now that the vblank has passed, we can go ahead and program the
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> index bdfe4035e074..8940aa4cf50c 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -385,6 +385,7 @@ struct intel_crtc_state {
> >> bool update_pipe; /* can a fast modeset be performed? */
> >> bool disable_cxsr;
> >> bool wm_changed; /* watermarks are updated */
> >> + bool fb_changed; /* fb on any of the planes is changed */
> >>
> >> /* Pipe source size (ie. panel fitter input size)
> >> * All planes will be positioned inside this space,
> >> @@ -570,7 +571,6 @@ struct intel_crtc_atomic_commit {
> >>
> >> /* Sleepable operations to perform after commit */
> >> unsigned fb_bits;
> >> - bool wait_vblank;
> >> bool update_fbc;
> >> bool post_enable_primary;
> >> unsigned update_sprite_watermarks;
> >> --
> >> 2.1.0
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3.
2016-02-09 10:42 ` Ville Syrjälä
@ 2016-02-09 11:07 ` Maarten Lankhorst
2016-02-09 11:22 ` Ville Syrjälä
0 siblings, 1 reply; 22+ messages in thread
From: Maarten Lankhorst @ 2016-02-09 11:07 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 09-02-16 om 11:42 schreef Ville Syrjälä:
> On Tue, Feb 09, 2016 at 11:27:44AM +0100, Maarten Lankhorst wrote:
>> Hey,
>>
>> Op 13-01-16 om 13:58 schreef Ville Syrjälä:
>>> On Mon, Jan 11, 2016 at 01:27:42PM +0100, Maarten Lankhorst wrote:
>>>> Currently we perform our own wait in post_plane_update,
>>>> but the atomic core performs another one in wait_for_vblanks.
>>>> This means that 2 vblanks are done when a fb is changed,
>>>> which is a bit overkill.
>>>>
>>>> Merge them by creating a helper function that takes a crtc mask
>>>> for the planes to wait on.
>>>>
>>>> The broadwell vblank workaround may look gone entirely but this is
>>>> not the case. pipe_config->wm_changed is set to true
>>>> when any plane is turned on, which forces a vblank wait.
>>> Still NAK, because you just removed the comment entirely.
>> I may have removed the comment but there will always be an unconditional wait because of the wm changes.
>> In some future commit I will rework do_intel_finish_page_flip to deal with atomic, and in that function the comment
>> will be useful again.
> The comment is the spec here, so it should be kept. Actually what I
> really want is to stop using the flip done interrupt entirely since
> it's arguably broken, at which point the comment should problably be
> moved to somewhere else (interrupt setup code, flip completion code,
> etc.). But removing the comment would be bad since then people might
> not understand why we do it the way we do.
I think the flip done interrupt will still be needed for cs flips, but Chris was against removing it iirc. I'll move the comment to page_flip_finished for now.
>>>> Changes since v1:
>>>> - Removing the double vblank wait on broadwell moved to its own commit.
>>>> Changes since v2:
>>>> - Move out POWER_DOMAIN_MODESET handling to its own commit.
>>>>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/intel_atomic.c | 1 +
>>>> drivers/gpu/drm/i915/intel_display.c | 80 ++++++++++++++++++++++++++----------
>>>> drivers/gpu/drm/i915/intel_drv.h | 2 +-
>>>> 3 files changed, 61 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>>>> index 9682d94af710..ba9a57f33c43 100644
>>>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>>>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>>>> @@ -98,6 +98,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>>>> crtc_state->disable_cxsr = false;
>>>> crtc_state->wm_changed = false;
>>>> crtc_state->wm.need_postvbl_update = false;
>>>> + crtc_state->fb_changed = false;
>>>>
>>>> return &crtc_state->base;
>>>> }
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>> index 2aa1c5367625..eac73f005a70 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -4796,9 +4796,6 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
>>>> to_intel_crtc_state(crtc->base.state);
>>>> struct drm_device *dev = crtc->base.dev;
>>>>
>>>> - if (atomic->wait_vblank)
>>>> - intel_wait_for_vblank(dev, crtc->pipe);
>>>> -
>>>> intel_frontbuffer_flip(dev, atomic->fb_bits);
>>>>
>>>> crtc->wm.cxsr_allowed = true;
>>>> @@ -11872,6 +11869,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>>> if (!was_visible && !visible)
>>>> return 0;
>>>>
>>>> + if (fb != old_plane_state->base.fb)
>>>> + pipe_config->fb_changed = true;
>>> Isn't that going to slow down cursor updates once again?
>> Very likely.. Shall I add a if (!state->legacy_cursor_update) to intel_atomic_wait_for_vblanks ?
> Not sure. Still wishing we could have just had the proper fps>=vrefresh
> support fron the start.
>
Working on it!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3.
2016-02-09 11:07 ` Maarten Lankhorst
@ 2016-02-09 11:22 ` Ville Syrjälä
0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2016-02-09 11:22 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Tue, Feb 09, 2016 at 12:07:01PM +0100, Maarten Lankhorst wrote:
> Op 09-02-16 om 11:42 schreef Ville Syrjälä:
> > On Tue, Feb 09, 2016 at 11:27:44AM +0100, Maarten Lankhorst wrote:
> >> Hey,
> >>
> >> Op 13-01-16 om 13:58 schreef Ville Syrjälä:
> >>> On Mon, Jan 11, 2016 at 01:27:42PM +0100, Maarten Lankhorst wrote:
> >>>> Currently we perform our own wait in post_plane_update,
> >>>> but the atomic core performs another one in wait_for_vblanks.
> >>>> This means that 2 vblanks are done when a fb is changed,
> >>>> which is a bit overkill.
> >>>>
> >>>> Merge them by creating a helper function that takes a crtc mask
> >>>> for the planes to wait on.
> >>>>
> >>>> The broadwell vblank workaround may look gone entirely but this is
> >>>> not the case. pipe_config->wm_changed is set to true
> >>>> when any plane is turned on, which forces a vblank wait.
> >>> Still NAK, because you just removed the comment entirely.
> >> I may have removed the comment but there will always be an unconditional wait because of the wm changes.
> >> In some future commit I will rework do_intel_finish_page_flip to deal with atomic, and in that function the comment
> >> will be useful again.
> > The comment is the spec here, so it should be kept. Actually what I
> > really want is to stop using the flip done interrupt entirely since
> > it's arguably broken, at which point the comment should problably be
> > moved to somewhere else (interrupt setup code, flip completion code,
> > etc.). But removing the comment would be bad since then people might
> > not understand why we do it the way we do.
> I think the flip done interrupt will still be needed for cs flips, but Chris was against removing it iirc. I'll move the comment to page_flip_finished for now.
We don't really need it. Using the vblank interrupt should work just fine.
> >>>> Changes since v1:
> >>>> - Removing the double vblank wait on broadwell moved to its own commit.
> >>>> Changes since v2:
> >>>> - Move out POWER_DOMAIN_MODESET handling to its own commit.
> >>>>
> >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>> ---
> >>>> drivers/gpu/drm/i915/intel_atomic.c | 1 +
> >>>> drivers/gpu/drm/i915/intel_display.c | 80 ++++++++++++++++++++++++++----------
> >>>> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> >>>> 3 files changed, 61 insertions(+), 22 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> >>>> index 9682d94af710..ba9a57f33c43 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_atomic.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> >>>> @@ -98,6 +98,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
> >>>> crtc_state->disable_cxsr = false;
> >>>> crtc_state->wm_changed = false;
> >>>> crtc_state->wm.need_postvbl_update = false;
> >>>> + crtc_state->fb_changed = false;
> >>>>
> >>>> return &crtc_state->base;
> >>>> }
> >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>>> index 2aa1c5367625..eac73f005a70 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>>> @@ -4796,9 +4796,6 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> >>>> to_intel_crtc_state(crtc->base.state);
> >>>> struct drm_device *dev = crtc->base.dev;
> >>>>
> >>>> - if (atomic->wait_vblank)
> >>>> - intel_wait_for_vblank(dev, crtc->pipe);
> >>>> -
> >>>> intel_frontbuffer_flip(dev, atomic->fb_bits);
> >>>>
> >>>> crtc->wm.cxsr_allowed = true;
> >>>> @@ -11872,6 +11869,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >>>> if (!was_visible && !visible)
> >>>> return 0;
> >>>>
> >>>> + if (fb != old_plane_state->base.fb)
> >>>> + pipe_config->fb_changed = true;
> >>> Isn't that going to slow down cursor updates once again?
> >> Very likely.. Shall I add a if (!state->legacy_cursor_update) to intel_atomic_wait_for_vblanks ?
> > Not sure. Still wishing we could have just had the proper fps>=vrefresh
> > support fron the start.
> >
> Working on it!
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-02-09 11:22 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-11 12:27 [PATCH v2 0/9] Kill off intel_crtc->atomic! Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 1/9] drm/i915: Unify power domain handling Maarten Lankhorst
2016-01-11 12:43 ` Maarten Lankhorst
2016-01-11 13:31 ` [PATCH v2 0.999/9] drm/i915: Pass crtc state to modeset_get_crtc_power_domains Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3 Maarten Lankhorst
2016-01-13 12:58 ` Ville Syrjälä
2016-02-09 10:27 ` Maarten Lankhorst
2016-02-09 10:42 ` Ville Syrjälä
2016-02-09 11:07 ` Maarten Lankhorst
2016-02-09 11:22 ` Ville Syrjälä
2016-01-11 12:27 ` [PATCH v2 3/9] drm/i915: Remove intel_crtc->atomic.disable_ips Maarten Lankhorst
2016-01-13 13:02 ` Ville Syrjälä
2016-01-18 12:10 ` Maarten Lankhorst
2016-01-18 13:27 ` Daniel Stone
2016-01-11 12:27 ` [PATCH v2 4/9] drm/i915: Remove atomic.pre_disable_primary Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 5/9] drm/i915: Remove update_sprite_watermarks Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 6/9] drm/i915: Remove some post-commit members from intel_crtc->atomic, v2 Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 7/9] drm/i915: Nuke fbc " Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 8/9] drm/i915: Do not compute watermarks on a noop Maarten Lankhorst
2016-01-11 12:27 ` [PATCH v2 9/9] drm/i915: Remove vblank wait from hsw_enable_ips Maarten Lankhorst
2016-01-11 12:49 ` ✗ failure: Fi.CI.BAT Patchwork
2016-01-11 14:30 ` Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.