Convert planes and crtc state updates to atomic.
Subsystems (2)
cover Convert planes and crtc state updates to atomic.
From: Maarten Lankhorst <[email protected]>
This applies on top of the patch series:
"[PATCH 00/35] Make legacy modeset a lot more atomic-like"
after the last patch crtc will no longer be enabled/disabled except through atomic updates, which should make it a lot easier to rely on the atomic state in the future.
Maarten Lankhorst (5):
drm/i915: Get rid of intel_crtc_disable and related code. drm/i915: Only update required power domains. drm/i915: use intel_crtc_control everywhere drm/i915: make plane helpers fully atomic drm/i915: Implement intel_crtc_toggle using atomic state
drivers/gpu/drm/i915/i915_debugfs.c | 18 +- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_atomic_plane.c | 49 +- drivers/gpu/drm/i915/intel_display.c | 796 ++++++++++++++++-------------- drivers/gpu/drm/i915/intel_drv.h | 2 - drivers/gpu/drm/i915/intel_sprite.c | 33 +- 6 files changed, 467 insertions(+), 432 deletions(-)
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
RFC PATCH 1/5 drm/i915: Get rid of intel_crtc_disable and related code.
From: Maarten Lankhorst <[email protected]>
Now that the dpll updates are (mostly) atomic, the .off() code is no longer used, and there are no more callers for intel_put_shared_dpll. Move all the updates done in intel_crtc_disable to intel_modeset_update_state, one less special case to worry about.
--- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_display.c | 100 +++++------------------------------ drivers/gpu/drm/i915/intel_drv.h | 1 - 3 files changed, 14 insertions(+), 88 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +0 -1
@@ -572,7 +572,6 @@ struct drm_i915_display_funcs { struct intel_crtc_state *crtc_state);void (*crtc_enable)(struct drm_crtc *crtc);void (*crtc_disable)(struct drm_crtc *crtc); -void (*off)(struct drm_crtc *crtc);void (*audio_codec_enable)(struct drm_connector *connector,struct intel_encoder *encoder,struct drm_display_mode *mode);
drivers/gpu/drm/i915/intel_display.c +14 -86
188 lines not on this page · raw message
drivers/gpu/drm/i915/intel_drv.h +0 -1
@@ -1078,7 +1078,6 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,#define assert_shared_dpll_disabled (d, p) assert_shared_dpll(d, p, false)struct intel_shared_dpll *intel_get_shared_dpll (struct intel_crtc *crtc,struct intel_crtc_state *state); -void intel_put_shared_dpll (struct intel_crtc *crtc);void vlv_force_pll_on (struct drm_device *dev,enum pipe pipe,const struct dpll *dpll);
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH RFC 1/5] drm/i915: Get rid of intel_crtc_disable and related code.
Quoted 4 lines · On Wed, 2015-04-22 at 13:24 +0200, [email protected] wrote:
From: Maarten Lankhorst <[email protected]>
Now that the dpll updates are (mostly) atomic, the .off() code is no longer used, and there are no more callers for
intel_put_shared_dpll.
This is a bit confusing since until this patch the .off() code is actually called from intel_crtc_disable, which calls intel_put_shared_dpll(). I think what you intended to say here is that intel_shared_dpll_commit() has the same effect as put_shared_dpll(), so it is safe to remove that call, at which point the .off() hook is not necessary anymore.
Quoted 3 lines
Move all the updates
done inintel_crtc_disabletointel_modeset_update_state, one less special case to worry about.
Maybe split this to a separate patch?
Ander
Quoted 230 lines
Signed-off-by Maarten Lankhorst <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_display.c | 100 +++++------------------------------ drivers/gpu/drm/i915/intel_drv.h | 1 - 3 files changed, 14 insertions(+), 88 deletions(-)drivers/gpu/drm/i915/i915_drv.h +0
-1@@ -572,7 +572,6 @@ struct drm_i915_display_funcs { struct intel_crtc_state *crtc_state);void (*crtc_enable)(struct drm_crtc *crtc);void (*crtc_disable)(struct drm_crtc *crtc); -void (*off)(struct drm_crtc *crtc);void (*audio_codec_enable)(struct drm_connector *connector,struct intel_encoder *encoder,struct drm_display_mode *mode);drivers/gpu/drm/i915/intel_display.c +14
-86@@ -4123,27 +4123,6 @@ static void lpt_pch_enable(struct drm_crtc *crtc)lpt_enable_pch_transcoder (dev_priv, cpu_transcoder); } -void intel_put_shared_dpll (struct intel_crtc *crtc) -{ -struct intel_shared_dpll *pll =intel_crtc_to_shared_dpll (crtc); - -if (pll ==NULL ) -return ; - -if (!(pll->config.crtc_mask & (1 << crtc->pipe))) { -WARN (1 ,"bad %s crtc mask\n" , pll->name); -return ; - } - - pll->config.crtc_mask &= ~(1 << crtc->pipe); -if (pll->config.crtc_mask ==0 ) { -WARN_ON (pll->on); -WARN_ON (pll->active); - } - - crtc->config->shared_dpll =DPLL_ID_PRIVATE ; -} -struct intel_shared_dpll *intel_get_shared_dpll (struct intel_crtc *crtc,struct intel_crtc_state *crtc_state) {@@ -5089,13 +5068,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)intel_disable_shared_dpll (intel_crtc); } -static void ironlake_crtc_off (struct drm_crtc *crtc) -{ -struct intel_crtc *intel_crtc =to_intel_crtc (crtc); -intel_put_shared_dpll (intel_crtc); -} - -static void i9xx_pfit_enable (struct intel_crtc *crtc) {struct drm_device *dev = crtc->base.dev;@@ -5722,10 +5694,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)mutex_unlock (&dev->struct_mutex); } -static void i9xx_crtc_off (struct drm_crtc *crtc) -{ -} -/* Master function to enable/disable CRTC and corresponding power wells */ void intel_crtc_control (struct drm_crtc *crtc,bool enable) {@@ -5775,34 +5743,6 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc) crtc->state->active = enable; } -static void intel_crtc_disable (struct drm_crtc *crtc) -{ -struct drm_device *dev = crtc->dev; -struct drm_connector *connector; -struct drm_i915_private *dev_priv = dev->dev_private; - -/* crtc should still be enabled when we disable it. */ -WARN_ON (!crtc->state->enable); - -intel_crtc_disable_planes (crtc); - dev_priv->display.crtc_disable (crtc); - dev_priv->display.off (crtc); - -drm_plane_helper_disable (crtc->primary); - -/* Update computed state. */ -list_for_each_entry (connector, &dev->mode_config.connector_list, head) { -if (!connector->encoder || !connector->encoder->crtc) -continue ; - -if (connector->encoder->crtc != crtc) -continue ; - - connector->dpms =DRM_MODE_DPMS_OFF ; -to_intel_encoder (connector->encoder)->connectors_active = false; - } -} -void intel_encoder_destroy (struct drm_encoder *encoder) {struct intel_encoder *intel_encoder =to_intel_encoder (encoder);@@ -11219,7 +11159,6 @@ intel_modeset_update_state(struct drm_atomic_state *state)struct drm_crtc *crtc;struct drm_crtc_state *crtc_state;struct drm_connector *connector; -int i;intel_shared_dpll_commit (dev_priv);@@ -11227,15 +11166,12 @@ intel_modeset_update_state(struct drm_atomic_state *state)if (!intel_encoder->base.crtc)continue ; -for_each_crtc_in_state (state, crtc, crtc_state, i) -if (crtc == intel_encoder->base.crtc) -break ; - -if (crtc != intel_encoder->base.crtc) + crtc = intel_encoder->base.crtc; + crtc_state = state->crtc_states[drm_crtc_index (crtc)]; +if (!crtc_state || !needs_modeset (crtc->state))continue ; -if (crtc_state->enable &&needs_modeset (crtc_state)) - intel_encoder->connectors_active = false; + intel_encoder->connectors_active = false; }drm_atomic_helper_swap_state (state->dev, state);@@ -11250,14 +11186,12 @@ intel_modeset_update_state(struct drm_atomic_state *state)if (!connector->encoder || !connector->encoder->crtc)continue ; -for_each_crtc_in_state (state, crtc, crtc_state, i) -if (crtc == connector->encoder->crtc) -break ; - -if (crtc != connector->encoder->crtc) + crtc = connector->encoder->crtc; + crtc_state = state->crtc_states[drm_crtc_index (crtc)]; +if (!crtc_state || !needs_modeset (crtc->state))continue ; -if (crtc->state->enable &&needs_modeset (crtc->state)) { +if (crtc->state->enable) {struct drm_property *dpms_property = dev->mode_config.dpms_property;@@ -11268,7 +11202,8 @@ intel_modeset_update_state(struct drm_atomic_state *state) intel_encoder =to_intel_encoder (connector->encoder); intel_encoder->connectors_active = true; - } + }else + connector->dpms =DRM_MODE_DPMS_OFF ; } }@@ -11946,12 +11881,10 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,if (!needs_modeset (crtc_state))continue ; -if (!crtc_state->enable) { -intel_crtc_disable (crtc); - }else if (crtc->state->enable) { -intel_crtc_disable_planes (crtc); - dev_priv->display.crtc_disable (crtc); - } +intel_crtc_disable_planes (crtc); + dev_priv->display.crtc_disable (crtc); +if (!crtc_state->enable) +drm_plane_helper_disable (crtc->primary); }/* crtc->mode is already used by the ->mode_set callbacks, hence we need @@ -13615,7 +13548,6 @@ static void intel_init_display(struct drm_device *dev) haswell_crtc_compute_clock; dev_priv->display.crtc_enable = haswell_crtc_enable; dev_priv->display.crtc_disable = haswell_crtc_disable; - dev_priv->display.off = ironlake_crtc_off; dev_priv->display.update_primary_plane = skylake_update_primary_plane; }else if (HAS_DDI (dev)) {@@ -13626,7 +13558,6 @@ static void intel_init_display(struct drm_device *dev) haswell_crtc_compute_clock; dev_priv->display.crtc_enable = haswell_crtc_enable; dev_priv->display.crtc_disable = haswell_crtc_disable; - dev_priv->display.off = ironlake_crtc_off; dev_priv->display.update_primary_plane = ironlake_update_primary_plane; }else if (HAS_PCH_SPLIT (dev)) {@@ -13637,7 +13568,6 @@ static void intel_init_display(struct drm_device *dev) ironlake_crtc_compute_clock; dev_priv->display.crtc_enable = ironlake_crtc_enable; dev_priv->display.crtc_disable = ironlake_crtc_disable; - dev_priv->display.off = ironlake_crtc_off; dev_priv->display.update_primary_plane = ironlake_update_primary_plane; }else if (IS_VALLEYVIEW (dev)) {@@ -13647,7 +13577,6 @@ static void intel_init_display(struct drm_device *dev) dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock; dev_priv->display.crtc_enable = valleyview_crtc_enable; dev_priv->display.crtc_disable = i9xx_crtc_disable; - dev_priv->display.off = i9xx_crtc_off; dev_priv->display.update_primary_plane = i9xx_update_primary_plane; }else {@@ -13657,7 +13586,6 @@ static void intel_init_display(struct drm_device *dev) dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock; dev_priv->display.crtc_enable = i9xx_crtc_enable; dev_priv->display.crtc_disable = i9xx_crtc_disable; - dev_priv->display.off = i9xx_crtc_off; dev_priv->display.update_primary_plane = i9xx_update_primary_plane; }drivers/gpu/drm/i915/intel_drv.h +0
-0
drivers/gpu/drm/i915/intel_drv.h void assert_shared_dpll(struct drm_i915_private *dev_priv, · line 1083
Quoted 7 lines
drivers/gpu/drm/i915/intel_drv.h +0
-1@@ -1078,7 +1078,6 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,#define assert_shared_dpll_disabled (d, p) assert_shared_dpll(d, p, false)struct intel_shared_dpll *intel_get_shared_dpll (struct intel_crtc *crtc,struct intel_crtc_state *state); -void intel_put_shared_dpll (struct intel_crtc *crtc);void vlv_force_pll_on (struct drm_device *dev,enum pipe pipe,const struct dpll *dpll);
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
RFC PATCH 2/5 drm/i915: Only update required power domains.
In reply to [email protected]From: Maarten Lankhorst <[email protected]>
This prevents unnecessarily updating power domains, while still enabling all power domains on initial setup.
--- drivers/gpu/drm/i915/intel_display.c | 52 ++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 11 deletions(-)
drivers/gpu/drm/i915/intel_display.c +41 -11
@@ -5163,36 +5163,72 @@ static unsigned long get_crtc_power_domains(struct drm_crtc *crtc)return mask; } +static bool +needs_modeset (struct drm_crtc_state *state) +{ +return state->mode_changed || state->active_changed; +} +static void modeset_update_crtc_power_domains (struct drm_atomic_state *state) {struct drm_device *dev = state->dev;struct drm_i915_private *dev_priv = dev->dev_private;unsigned long pipe_domains[I915_MAX_PIPES ] = {0 , };struct intel_crtc *crtc; +bool init_power = dev_priv->power_domains.init_power_on; +bool any_power = init_power, any_modeset = false; +unsigned long domains;/* * First get all needed power domains, then put all unneeded, to avoid * any unnecessary toggling of the power wells. */ for_each_intel_crtc (dev, crtc) { +int idx =drm_crtc_index (&crtc->base); +struct drm_crtc_state *crtc_state = state->crtc_states[idx];enum intel_display_power_domain domain; -if (!crtc->base.state->enable) +if (!init_power && !crtc_state) +continue ; + +if (needs_modeset (crtc->base.state)) + any_modeset = true; + +if (crtc->base.state->enable) + pipe_domains[crtc->pipe] = +get_crtc_power_domains (&crtc->base); + +if (pipe_domains[crtc->pipe] == crtc->enabled_power_domains)continue ; - pipe_domains[crtc->pipe] =get_crtc_power_domains (&crtc->base); +WARN_ON (!init_power && !needs_modeset (crtc->base.state)); + + any_power = true; + domains = pipe_domains[crtc->pipe] & + ~crtc->enabled_power_domains; -for_each_power_domain (domain, pipe_domains[crtc->pipe]) +for_each_power_domain (domain, domains)intel_display_power_get (dev_priv, domain); } -if (dev_priv->display.modeset_global_resources) +if (any_modeset && dev_priv->display.modeset_global_resources) dev_priv->display.modeset_global_resources (state); +if (!any_power) +return ; +for_each_intel_crtc (dev, crtc) { +int idx =drm_crtc_index (&crtc->base); +struct drm_crtc_state *crtc_state = state->crtc_states[idx];enum intel_display_power_domain domain; -for_each_power_domain (domain, crtc->enabled_power_domains) +if (!init_power && !crtc_state) +continue ; + + domains = crtc->enabled_power_domains & + ~pipe_domains[crtc->pipe]; + +for_each_power_domain (domain, domains)intel_display_power_put (dev_priv, domain); crtc->enabled_power_domains = pipe_domains[crtc->pipe];
@@ -11144,12 +11180,6 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc)return false; } -static bool -needs_modeset (struct drm_crtc_state *state) -{ -return state->mode_changed || state->active_changed; -} -static void intel_modeset_update_state (struct drm_atomic_state *state) {
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH RFC 2/5] drm/i915: Only update required power domains.
Quoted 57 lines · On Wed, 2015-04-22 at 13:24 +0200, [email protected] wrote:
From: Maarten Lankhorst <[email protected]>
This prevents unnecessarily updating power domains, while still enabling all power domains on initial setup.
Signed-off-by Maarten Lankhorst <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 52 ++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 11 deletions(-)drivers/gpu/drm/i915/intel_display.c +22
-1@@ -5163,36 +5163,72 @@ static unsigned long get_crtc_power_domains(struct drm_crtc *crtc)return mask; } +static bool +needs_modeset (struct drm_crtc_state *state) +{ +return state->mode_changed || state->active_changed; +} +static void modeset_update_crtc_power_domains (struct drm_atomic_state *state) {struct drm_device *dev = state->dev;struct drm_i915_private *dev_priv = dev->dev_private;unsigned long pipe_domains[I915_MAX_PIPES ] = {0 , };struct intel_crtc *crtc; +bool init_power = dev_priv->power_domains.init_power_on; +bool any_power = init_power, any_modeset = false; +unsigned long domains;/* * First get all needed power domains, then put all unneeded, to avoid * any unnecessary toggling of the power wells. */ for_each_intel_crtc (dev, crtc) { +int idx =drm_crtc_index (&crtc->base); +struct drm_crtc_state *crtc_state = state->crtc_states[idx];enum intel_display_power_domain domain; -if (!crtc->base.state->enable) +if (!init_power && !crtc_state) +continue ; + +if (needs_modeset (crtc->base.state)) + any_modeset = true; + +if (crtc->base.state->enable) + pipe_domains[crtc->pipe] = +get_crtc_power_domains (&crtc->base); + +if (pipe_domains[crtc->pipe] == crtc->enabled_power_domains)continue ;
drivers/gpu/drm/i915/intel_display.c static unsigned long get_crtc_power_domains(struct drm_crtc *crtc) · line 5211
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +6
-2- pipe_domains[crtc->pipe] =get_crtc_power_domains (&crtc->base); +WARN_ON (!init_power && !needs_modeset (crtc->base.state)); + + any_power = true; + domains = pipe_domains[crtc->pipe] & + ~crtc->enabled_power_domains; -for_each_power_domain (domain, pipe_domains[crtc->pipe]) +for_each_power_domain (domain, domains)intel_display_power_get (dev_priv, domain);
Isn't intel_display_power_get() already a no-op if the power domain is already active, except for the reference counting? Or did I miss something?
Ander
Quoted 28 lines
drivers/gpu/drm/i915/intel_display.c +13
-2} -if (dev_priv->display.modeset_global_resources) +if (any_modeset && dev_priv->display.modeset_global_resources) dev_priv->display.modeset_global_resources (state); +if (!any_power) +return ; +for_each_intel_crtc (dev, crtc) { +int idx =drm_crtc_index (&crtc->base); +struct drm_crtc_state *crtc_state = state->crtc_states[idx];enum intel_display_power_domain domain; -for_each_power_domain (domain, crtc->enabled_power_domains) +if (!init_power && !crtc_state) +continue ; + + domains = crtc->enabled_power_domains & + ~pipe_domains[crtc->pipe]; + +for_each_power_domain (domain, domains)intel_display_power_put (dev_priv, domain); crtc->enabled_power_domains = pipe_domains[crtc->pipe];@@ -11144,12 +11180,6 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc)return false; }
drivers/gpu/drm/i915/intel_display.c static bool intel_crtc_in_use(struct drm_crtc *crtc) · line 11185
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +0
-6-static bool -needs_modeset (struct drm_crtc_state *state) -{ -return state->mode_changed || state->active_changed; -} -static void intel_modeset_update_state (struct drm_atomic_state *state) {
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
RFC PATCH 3/5 drm/i915: use intel_crtc_control everywhere
In reply to [email protected]From: Maarten Lankhorst <[email protected]>
--- drivers/gpu/drm/i915/i915_debugfs.c | 18 ++++++++-- drivers/gpu/drm/i915/intel_display.c | 68 +++++++++++++----------------------- drivers/gpu/drm/i915/intel_drv.h | 1 - 3 files changed, 39 insertions(+), 48 deletions(-)
drivers/gpu/drm/i915/i915_debugfs.c +15 -3
@@ -3589,12 +3589,18 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev)*/ if (crtc->config->cpu_transcoder ==TRANSCODER_EDP && !crtc->config->pch_pfit.enabled) { +bool active = crtc->active; + +if (active) +intel_crtc_control (&crtc->base, false); + crtc->config->pch_pfit.force_thru = true;intel_display_power_get (dev_priv,POWER_DOMAIN_PIPE_PANEL_FITTER (PIPE_A )); -intel_crtc_reset (&crtc->base); +if (active) +intel_crtc_control (&crtc->base, true); }drm_modeset_unlock_all (dev); }
@@ -3613,12 +3619,18 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev)* routing. */ if (crtc->config->pch_pfit.force_thru) { - crtc->config->pch_pfit.force_thru = false; +bool active = crtc->active; -intel_crtc_reset (&crtc->base); +if (active) +intel_crtc_control (&crtc->base, false); + + crtc->config->pch_pfit.force_thru = false;intel_display_power_put (dev_priv,POWER_DOMAIN_PIPE_PANEL_FITTER (PIPE_A )); + +if (active) +intel_crtc_control (&crtc->base, true); }drm_modeset_unlock_all (dev); }
drivers/gpu/drm/i915/intel_display.c +24 -44
106 lines not on this page · raw message
drivers/gpu/drm/i915/intel_drv.h +0 -1
@@ -987,7 +987,6 @@ void intel_mark_busy(struct drm_device *dev);void intel_mark_idle (struct drm_device *dev);void intel_crtc_restore_mode (struct drm_crtc *crtc);void intel_crtc_control (struct drm_crtc *crtc,bool enable); -void intel_crtc_reset (struct intel_crtc *crtc);void intel_crtc_update_dpms (struct drm_crtc *crtc);void intel_encoder_destroy (struct drm_encoder *encoder);int intel_connector_init (struct intel_connector *);
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH RFC 3/5] drm/i915: use intel_crtc_control everywhere
Quoted 1 line · On Wed, Apr 22, 2015 at 01:24:20PM +0200, [email protected]…
From: Maarten Lankhorst <[email protected]>
A bit more explanation in the commit message here would be useful, i.e. what changed that we need this now.
-Daniel
Quoted 187 lines
Signed-off-by Maarten Lankhorst <[email protected]>--- drivers/gpu/drm/i915/i915_debugfs.c | 18 ++++++++-- drivers/gpu/drm/i915/intel_display.c | 68 +++++++++++++----------------------- drivers/gpu/drm/i915/intel_drv.h | 1 - 3 files changed, 39 insertions(+), 48 deletions(-)drivers/gpu/drm/i915/i915_debugfs.c +15
-3@@ -3589,12 +3589,18 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev)*/ if (crtc->config->cpu_transcoder ==TRANSCODER_EDP && !crtc->config->pch_pfit.enabled) { +bool active = crtc->active; + +if (active) +intel_crtc_control (&crtc->base, false); + crtc->config->pch_pfit.force_thru = true;intel_display_power_get (dev_priv,POWER_DOMAIN_PIPE_PANEL_FITTER (PIPE_A )); -intel_crtc_reset (&crtc->base); +if (active) +intel_crtc_control (&crtc->base, true); }drm_modeset_unlock_all (dev); }@@ -3613,12 +3619,18 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev)* routing. */ if (crtc->config->pch_pfit.force_thru) { - crtc->config->pch_pfit.force_thru = false; +bool active = crtc->active; -intel_crtc_reset (&crtc->base); +if (active) +intel_crtc_control (&crtc->base, false); + + crtc->config->pch_pfit.force_thru = false;intel_display_power_put (dev_priv,POWER_DOMAIN_PIPE_PANEL_FITTER (PIPE_A )); + +if (active) +intel_crtc_control (&crtc->base, true); }drm_modeset_unlock_all (dev); }drivers/gpu/drm/i915/intel_display.c +24
-44@@ -3101,22 +3101,8 @@ static void intel_update_primary_planes(struct drm_device *dev) } } -void intel_crtc_reset (struct intel_crtc *crtc) -{ -struct drm_i915_private *dev_priv =to_i915 (crtc->base.dev); - -if (!crtc->active) -return ; - -intel_crtc_disable_planes (&crtc->base); - dev_priv->display.crtc_disable (&crtc->base); - dev_priv->display.crtc_enable (&crtc->base); -intel_crtc_enable_planes (&crtc->base); -} -void intel_prepare_reset (struct drm_device *dev) { -struct drm_i915_private *dev_priv =to_i915 (dev);struct intel_crtc *crtc;/* no reset support for gen2 */ @@ -3128,18 +3114,12 @@ void intel_prepare_reset(struct drm_device *dev)return ;drm_modeset_lock_all (dev); -/* * Disabling the crtcs gracefully seems nicer. Also the * g33 docs say we should at least disable all the planes. */ -for_each_intel_crtc (dev, crtc) { -if (!crtc->active) -continue ; - -intel_crtc_disable_planes (&crtc->base); - dev_priv->display.crtc_disable (&crtc->base); - } +for_each_intel_crtc (dev, crtc) +intel_crtc_control (&crtc->base, false); }void intel_finish_reset (struct drm_device *dev)@@ -5739,26 +5719,29 @@ void intel_crtc_control(struct drm_crtc *crtc, bool enable)enum intel_display_power_domain domain;unsigned long domains; +if (enable == intel_crtc->active) +return ; + +if (enable && !crtc->state->enable) +return ; + + crtc->state->active = enable;if (enable) { -if (!intel_crtc->active) { - domains =get_crtc_power_domains (crtc); -for_each_power_domain (domain, domains) -intel_display_power_get (dev_priv, domain); - intel_crtc->enabled_power_domains = domains; - - dev_priv->display.crtc_enable (crtc); -intel_crtc_enable_planes (crtc); - } + domains =get_crtc_power_domains (crtc); +for_each_power_domain (domain, domains) +intel_display_power_get (dev_priv, domain); + intel_crtc->enabled_power_domains = domains; + + dev_priv->display.crtc_enable (crtc); +intel_crtc_enable_planes (crtc); }else { -if (intel_crtc->active) { -intel_crtc_disable_planes (crtc); - dev_priv->display.crtc_disable (crtc); - - domains = intel_crtc->enabled_power_domains; -for_each_power_domain (domain, domains) -intel_display_power_put (dev_priv, domain); - intel_crtc->enabled_power_domains =0 ; - } +intel_crtc_disable_planes (crtc); + dev_priv->display.crtc_disable (crtc); + + domains = intel_crtc->enabled_power_domains; +for_each_power_domain (domain, domains) +intel_display_power_put (dev_priv, domain); + intel_crtc->enabled_power_domains =0 ; } }@@ -5775,8 +5758,6 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc) enable |= intel_encoder->connectors_active;intel_crtc_control (crtc, enable); - - crtc->state->active = enable; }void intel_encoder_destroy (struct drm_encoder *encoder)@@ -14087,8 +14068,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) plane = crtc->plane;to_intel_plane_state (crtc->base.primary->state)->visible = true; crtc->plane = !plane; -intel_crtc_disable_planes (&crtc->base); - dev_priv->display.crtc_disable (&crtc->base); +intel_crtc_control (&crtc->base, false); crtc->plane = plane;/* ... and break all links. */ drivers/gpu/drm/i915/intel_drv.h +0
-1@@ -987,7 +987,6 @@ void intel_mark_busy(struct drm_device *dev);void intel_mark_idle (struct drm_device *dev);void intel_crtc_restore_mode (struct drm_crtc *crtc);void intel_crtc_control (struct drm_crtc *crtc,bool enable); -void intel_crtc_reset (struct intel_crtc *crtc);void intel_crtc_update_dpms (struct drm_crtc *crtc);void intel_encoder_destroy (struct drm_encoder *encoder);int intel_connector_init (struct intel_connector *);Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
RFC PATCH 4/5 drm/i915: make plane helpers fully atomic
In reply to [email protected]From: Maarten Lankhorst <[email protected]>
--- drivers/gpu/drm/i915/intel_atomic_plane.c | 49 +-- drivers/gpu/drm/i915/intel_display.c | 481 +++++++++++++++++++++--------- drivers/gpu/drm/i915/intel_sprite.c | 33 +- 3 files changed, 353 insertions(+), 210 deletions(-)
drivers/gpu/drm/i915/intel_atomic_plane.c +16 -33
@@ -110,32 +110,31 @@ static int intel_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) {struct drm_crtc *crtc = state->crtc; -struct intel_crtc *intel_crtc; -struct intel_crtc_state *crtc_state; +struct drm_crtc_state *crtc_state;struct intel_plane *intel_plane =to_intel_plane (plane);struct intel_plane_state *intel_state =to_intel_plane_state (state); - crtc = crtc ? crtc : plane->crtc; - intel_crtc =to_intel_crtc (crtc); - + intel_state->visible = false;/* * Both crtc and plane->crtc could be NULL if we're updating a * property while the plane is disabled. We don't actually have * anything driver-specific we need to test in that case, so * just return success. */ -if (!crtc) +if (!crtc) { +DRM_DEBUG_ATOMIC ("Invisible: no crtc\n" ); +return 0 ; + } + + crtc_state = state->state->crtc_states[drm_crtc_index (crtc)]; +if (WARN_ON (!crtc_state) || +WARN_ON (!crtc_state->enable)) {return 0 ; + } -/* FIXME: temporary hack necessary while we still use the plane update -* helper. */ -if (state->state) { - crtc_state = -intel_atomic_get_crtc_state (state->state, intel_crtc); -if (IS_ERR (crtc_state)) -return PTR_ERR (crtc_state); - }else { - crtc_state = intel_crtc->config; +if (!crtc_state->active) { +DRM_DEBUG_ATOMIC ("Invisible: dpms off\n" ); +return 0 ; }/*
@@ -155,24 +154,8 @@ static int intel_plane_atomic_check(struct drm_plane *plane,/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */ intel_state->clip.x1 =0 ; intel_state->clip.y1 =0 ; - intel_state->clip.x2 = - crtc_state->base.active ? crtc_state->pipe_src_w :0 ; - intel_state->clip.y2 = - crtc_state->base.active ? crtc_state->pipe_src_h :0 ; - -/* -* Disabling a plane is always okay; we just need to update -* fb tracking in a special way since cleanup_fb() won't -* get called by the plane helpers. -*/ -if (state->fb ==NULL && plane->state->fb !=NULL ) { -/* -* 'prepare' is never called when plane is being disabled, so -* we need to handle frontbuffer tracking as a special case -*/ - intel_crtc->atomic.disabled_planes |= - (1 <<drm_plane_index (plane)); - } + intel_state->clip.x2 =to_intel_crtc_state (crtc_state)->pipe_src_w; + intel_state->clip.y2 =to_intel_crtc_state (crtc_state)->pipe_src_h;if (state->fb &&intel_rotation_90_or_270 (state->rotation)) { if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
drivers/gpu/drm/i915/intel_display.c +334 -147
711 lines not on this page · raw message
drivers/gpu/drm/i915/intel_sprite.c +3 -30
@@ -811,12 +811,8 @@ intel_check_sprite_plane(struct drm_plane *plane,int max_scale, min_scale;int pixel_size; - intel_crtc = intel_crtc ? intel_crtc :to_intel_crtc (plane->crtc); - -if (!fb) { - state->visible = false; - goto finish; - } +if (!fb) +return 0 ;/* Don't modify another pipe's plane */ if (intel_plane->pipe != intel_crtc->pipe) {
@@ -847,7 +843,7 @@ intel_check_sprite_plane(struct drm_plane *plane, vscale =drm_rect_calc_vscale_relaxed (src, dst, min_scale, max_scale);BUG_ON (vscale <0 ); - state->visible =drm_rect_clip_scaled (src, dst, clip, hscale, vscale); + state->visible =drm_rect_clip_scaled (src, dst, clip, hscale, vscale); crtc_x = dst->x1; crtc_y = dst->y1;
@@ -952,29 +948,6 @@ intel_check_sprite_plane(struct drm_plane *plane, dst->y1 = crtc_y; dst->y2 = crtc_y + crtc_h; -finish: -/* -* If the sprite is completely covering the primary plane, -* we can disable the primary and save power. -*/ -if (intel_crtc->active) { - intel_crtc->atomic.fb_bits |= -INTEL_FRONTBUFFER_SPRITE (intel_crtc->pipe); - -if (intel_wm_need_update (plane, &state->base)) - intel_crtc->atomic.update_wm = true; - -if (!state->visible) { -/* -* Avoid underruns when disabling the sprite. -* FIXME remove once watermark updates are done properly. -*/ - intel_crtc->atomic.wait_vblank = true; - intel_crtc->atomic.update_sprite_watermarks |= - (1 <<drm_plane_index (plane)); - } - } -return 0 ; }
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
RFC PATCH v2 4/5 drm/i915: make plane helpers fully atomic
This kills off most of the transitional sers and uses atomic plane updates in the modeset path to update everything.
---
Changes since v1:
- Add atomic and sprite planes during a modeset too so they will be restored.
- Drop a WARN_ON(!crtc_state->enabled) in atomic_plane_check for cursor and sprite planes. Keep it for primary as this probably indicates we messed up somewhere.
drivers/gpu/drm/i915/intel_atomic_plane.c | 58 ++-- drivers/gpu/drm/i915/intel_display.c | 485 +++++++++++++++++++++--------- drivers/gpu/drm/i915/intel_sprite.c | 33 +- 3 files changed, 366 insertions(+), 210 deletions(-)
drivers/gpu/drm/i915/intel_atomic_plane.c +25 -33
@@ -110,32 +110,40 @@ static int intel_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) {struct drm_crtc *crtc = state->crtc; -struct intel_crtc *intel_crtc; -struct intel_crtc_state *crtc_state; +struct drm_crtc_state *crtc_state;struct intel_plane *intel_plane =to_intel_plane (plane);struct intel_plane_state *intel_state =to_intel_plane_state (state); - crtc = crtc ? crtc : plane->crtc; - intel_crtc =to_intel_crtc (crtc); - + intel_state->visible = false;/* * Both crtc and plane->crtc could be NULL if we're updating a * property while the plane is disabled. We don't actually have * anything driver-specific we need to test in that case, so * just return success. */ -if (!crtc) +if (!crtc) { +DRM_DEBUG_ATOMIC ("Invisible: no crtc\n" );return 0 ; + } + + crtc_state = state->state->crtc_states[drm_crtc_index (crtc)]; +if (WARN_ON (!crtc_state)) +return 0 ; + +if (!crtc_state->enable) { +DRM_DEBUG_ATOMIC ("Invisible: crtc off\n" ); -/* FIXME: temporary hack necessary while we still use the plane update -* helper. */ -if (state->state) { - crtc_state = -intel_atomic_get_crtc_state (state->state, intel_crtc); -if (IS_ERR (crtc_state)) -return PTR_ERR (crtc_state); - }else { - crtc_state = intel_crtc->config; +/* +* Probably allowed after converting to atomic. Right +* now it probably means we have the state confused. +*/ +I915_STATE_WARN_ON (plane->type ==DRM_PLANE_TYPE_PRIMARY ); +return 0 ; + } + +if (!crtc_state->active) { +DRM_DEBUG_ATOMIC ("Invisible: dpms off\n" ); +return 0 ; }/*
@@ -155,24 +163,8 @@ static int intel_plane_atomic_check(struct drm_plane *plane,/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */ intel_state->clip.x1 =0 ; intel_state->clip.y1 =0 ; - intel_state->clip.x2 = - crtc_state->base.active ? crtc_state->pipe_src_w :0 ; - intel_state->clip.y2 = - crtc_state->base.active ? crtc_state->pipe_src_h :0 ; - -/* -* Disabling a plane is always okay; we just need to update -* fb tracking in a special way since cleanup_fb() won't -* get called by the plane helpers. -*/ -if (state->fb ==NULL && plane->state->fb !=NULL ) { -/* -* 'prepare' is never called when plane is being disabled, so -* we need to handle frontbuffer tracking as a special case -*/ - intel_crtc->atomic.disabled_planes |= - (1 <<drm_plane_index (plane)); - } + intel_state->clip.x2 =to_intel_crtc_state (crtc_state)->pipe_src_w; + intel_state->clip.y2 =to_intel_crtc_state (crtc_state)->pipe_src_h;if (state->fb &&intel_rotation_90_or_270 (state->rotation)) { if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
drivers/gpu/drm/i915/intel_display.c +338 -147
715 lines not on this page · raw message
drivers/gpu/drm/i915/intel_sprite.c +3 -30
@@ -811,12 +811,8 @@ intel_check_sprite_plane(struct drm_plane *plane,int max_scale, min_scale;int pixel_size; - intel_crtc = intel_crtc ? intel_crtc :to_intel_crtc (plane->crtc); - -if (!fb) { - state->visible = false; - goto finish; - } +if (!fb) +return 0 ;/* Don't modify another pipe's plane */ if (intel_plane->pipe != intel_crtc->pipe) {
@@ -847,7 +843,7 @@ intel_check_sprite_plane(struct drm_plane *plane, vscale =drm_rect_calc_vscale_relaxed (src, dst, min_scale, max_scale);BUG_ON (vscale <0 ); - state->visible =drm_rect_clip_scaled (src, dst, clip, hscale, vscale); + state->visible =drm_rect_clip_scaled (src, dst, clip, hscale, vscale); crtc_x = dst->x1; crtc_y = dst->y1;
@@ -952,29 +948,6 @@ intel_check_sprite_plane(struct drm_plane *plane, dst->y1 = crtc_y; dst->y2 = crtc_y + crtc_h; -finish: -/* -* If the sprite is completely covering the primary plane, -* we can disable the primary and save power. -*/ -if (intel_crtc->active) { - intel_crtc->atomic.fb_bits |= -INTEL_FRONTBUFFER_SPRITE (intel_crtc->pipe); - -if (intel_wm_need_update (plane, &state->base)) - intel_crtc->atomic.update_wm = true; - -if (!state->visible) { -/* -* Avoid underruns when disabling the sprite. -* FIXME remove once watermark updates are done properly. -*/ - intel_crtc->atomic.wait_vblank = true; - intel_crtc->atomic.update_sprite_watermarks |= - (1 <<drm_plane_index (plane)); - } - } -return 0 ; }
Signature · 7 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v2 RFC 4/5] drm/i915: make plane helpers fully atomic
Quoted 212 lines · Maarten Lankhorst
This kills off most of the transitional sers and uses atomic plane updates in the modeset path to update everything.
Signed-off-by Maarten Lankhorst <[email protected]>---
Changes since v1:
- Add atomic and sprite planes during a modeset too so they will be restored.
- Drop aWARN_ON(!crtc_state->enabled)inatomic_plane_checkfor cursor and sprite planes. Keep it for primary as this probably indicates we messed up somewhere.drivers/gpu/drm/i915/intel_atomic_plane.c | 58 ++-- drivers/gpu/drm/i915/intel_display.c | 485 +++++++++++++++++++++--------- drivers/gpu/drm/i915/intel_sprite.c | 33 +- 3 files changed, 366 insertions(+), 210 deletions(-)drivers/gpu/drm/i915/intel_atomic_plane.c +25
-33@@ -110,32 +110,40 @@ static int intel_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) {struct drm_crtc *crtc = state->crtc; -struct intel_crtc *intel_crtc; -struct intel_crtc_state *crtc_state; +struct drm_crtc_state *crtc_state;struct intel_plane *intel_plane =to_intel_plane (plane);struct intel_plane_state *intel_state =to_intel_plane_state (state); - crtc = crtc ? crtc : plane->crtc; - intel_crtc =to_intel_crtc (crtc); - + intel_state->visible = false;/* * Both crtc and plane->crtc could be NULL if we're updating a * property while the plane is disabled. We don't actually have * anything driver-specific we need to test in that case, so * just return success. */ -if (!crtc) +if (!crtc) { +DRM_DEBUG_ATOMIC ("Invisible: no crtc\n" );return 0 ; + } + + crtc_state = state->state->crtc_states[drm_crtc_index (crtc)]; +if (WARN_ON (!crtc_state)) +return 0 ; + +if (!crtc_state->enable) { +DRM_DEBUG_ATOMIC ("Invisible: crtc off\n" ); -/* FIXME: temporary hack necessary while we still use the plane update -* helper. */ -if (state->state) { - crtc_state = -intel_atomic_get_crtc_state (state->state, intel_crtc); -if (IS_ERR (crtc_state)) -return PTR_ERR (crtc_state); - }else { - crtc_state = intel_crtc->config; +/* +* Probably allowed after converting to atomic. Right +* now it probably means we have the state confused. +*/ +I915_STATE_WARN_ON (plane->type ==DRM_PLANE_TYPE_PRIMARY ); +return 0 ; + } + +if (!crtc_state->active) { +DRM_DEBUG_ATOMIC ("Invisible: dpms off\n" ); +return 0 ; }/* @@ -155,24 +163,8 @@ static int intel_plane_atomic_check(struct drm_plane *plane,/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */ intel_state->clip.x1 =0 ; intel_state->clip.y1 =0 ; - intel_state->clip.x2 = - crtc_state->base.active ? crtc_state->pipe_src_w :0 ; - intel_state->clip.y2 = - crtc_state->base.active ? crtc_state->pipe_src_h :0 ; - -/* -* Disabling a plane is always okay; we just need to update -* fb tracking in a special way since cleanup_fb() won't -* get called by the plane helpers. -*/ -if (state->fb ==NULL && plane->state->fb !=NULL ) { -/* -* 'prepare' is never called when plane is being disabled, so -* we need to handle frontbuffer tracking as a special case -*/ - intel_crtc->atomic.disabled_planes |= - (1 <<drm_plane_index (plane)); - } + intel_state->clip.x2 =to_intel_crtc_state (crtc_state)->pipe_src_w; + intel_state->clip.y2 =to_intel_crtc_state (crtc_state)->pipe_src_h;if (state->fb &&intel_rotation_90_or_270 (state->rotation)) { if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||drivers/gpu/drm/i915/intel_display.c +27
-16@@ -100,12 +100,16 @@ static void vlv_prepare_pll(struct intel_crtc *crtc, const struct intel_crtc_state *pipe_config);static void chv_prepare_pll (struct intel_crtc *crtc,const struct intel_crtc_state *pipe_config); +static int intel_atomic_check_crtc (struct drm_crtc *crtc, +struct drm_crtc_state *crtc_state);static void intel_begin_crtc_commit (struct drm_crtc *crtc);static void intel_finish_crtc_commit (struct drm_crtc *crtc);static void skl_init_scalers (struct drm_device *dev,struct intel_crtc *intel_crtc,struct intel_crtc_state *crtc_state);static void intel_crtc_enable_planes (struct drm_crtc *crtc);static void intel_crtc_disable_planes (struct drm_crtc *crtc); +static void intel_pre_disable_primary (struct drm_crtc *crtc); +static void intel_post_enable_primary (struct drm_crtc *crtc);static struct intel_encoder *intel_find_encoder (struct intel_connector *connector,int pipe) {@@ -3056,11 +3060,20 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb, {struct drm_device *dev = crtc->dev;struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_plane_state *plane_state = +to_intel_plane_state (crtc->primary->state); +bool was_visible = plane_state->visible; -if (dev_priv->display.disable_fbc) +/* Not supported right now by the helper, but lets be thorough. */ +if (was_visible && !fb) +intel_pre_disable_primary (crtc); +else if (was_visible && dev_priv->display.disable_fbc) dev_priv->display.disable_fbc (dev); + plane_state->visible = !!fb; dev_priv->display.update_primary_plane (crtc, fb, x, y); +if (!was_visible && fb) +intel_post_enable_primary (crtc);return 0 ; }@@ -3087,16 +3100,17 @@ static void intel_update_primary_planes(struct drm_device *dev)struct intel_crtc *intel_crtc =to_intel_crtc (crtc);drm_modeset_lock (&crtc->mutex,NULL ); -/* -* FIXME: Once we have proper support for primary planes (and -* disabling them without disabling the entire crtc) allow again -* a NULL crtc->primary->fb. -*/ -if (intel_crtc->active && crtc->primary->fb) + +if (intel_crtc->active) { +const struct intel_plane_state *state = +to_intel_plane_state (crtc->primary->state); + dev_priv->display.update_primary_plane (crtc, - crtc->primary->fb, - crtc->x, - crtc->y); + state->base.fb, + state->src.x1 >>16 , + state->src.y1 >>16 ); + } +drm_modeset_unlock (&crtc->mutex); } }@@ -10659,6 +10673,7 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = { .load_lut = intel_crtc_load_lut, .atomic_begin = intel_begin_crtc_commit, .atomic_flush = intel_finish_crtc_commit, + .atomic_check = intel_atomic_check_crtc, };/** @@ -10713,7 +10728,6 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)*/ static void intel_modeset_fixup_state (struct drm_atomic_state *state) { -struct intel_crtc *crtc;struct intel_encoder *encoder;struct intel_connector *connector;@@ -10736,11 +10750,6 @@ static void intel_modeset_fixup_state(struct drm_atomic_state *state) encoder->base.crtc =NULL ; } -for_each_intel_crtc (state->dev, crtc) { - crtc->base.enabled = crtc->base.state->enable; - crtc->config =to_intel_crtc_state (crtc->base.state); - } -/* Copy the new configuration to the staged state, to keep the few * pieces of code that haven't been converted yet happy */ intel_modeset_update_staged_output_state (state->dev);@@ -11170,6 +11179,8 @@ intel_modeset_update_state(struct drm_atomic_state *state)struct drm_crtc *crtc;struct drm_crtc_state *crtc_state;struct drm_connector *connector; +struct drm_connector_state *connector_state; +int i;intel_shared_dpll_commit (dev_priv);
drivers/gpu/drm/i915/intel_display.c intel_modeset_update_state(struct drm_atomic_state *state) · line 11203
Quoted 9 lines
drivers/gpu/drm/i915/intel_display.c +5
-1@@ -11185,7 +11196,26 @@ intel_modeset_update_state(struct drm_atomic_state *state) intel_encoder->connectors_active = false; } -drm_atomic_helper_swap_state (state->dev, state); +/* +* swap crtc and connector state, plane state is already swapped in +* __intel_set_mode_update_planes. Once .crtc_disable is fixed +* all state should be swapped before disabling crtc's. +*/
Can't we just fix .crtc_disable() first?
Quoted 125 lines
drivers/gpu/drm/i915/intel_display.c +93
-3+for_each_crtc_in_state (state, crtc, crtc_state, i) { + crtc->enabled = crtc_state->enable; +to_intel_crtc (crtc)->config =to_intel_crtc_state (crtc_state); + + crtc->state->state = state; +swap (state->crtc_states[i], crtc->state); + crtc->state->state =NULL ; + } + +for_each_connector_in_state (state, connector, connector_state, i) { + connector->state->state = state; +swap (state->connector_states[i], connector->state); + connector->state->state =NULL ; + } +intel_modeset_fixup_state (state);/* Double check state. */ @@ -11740,6 +11770,30 @@ static void update_scanline_offset(struct intel_crtc *crtc) crtc->scanline_offset =1 ; } +static int intel_modeset_compute_planes (struct drm_atomic_state *state, +struct drm_crtc *crtc) +{ +struct intel_crtc *intel_crtc =to_intel_crtc (crtc); +int pipe = intel_crtc->pipe; +struct drm_plane_state *plane_state; +struct drm_plane *plane; + + plane_state =drm_atomic_get_plane_state (state, crtc->cursor); +if (IS_ERR (plane_state)) +return PTR_ERR (plane_state); + +/* Add all overlay planes */ +drm_for_each_legacy_plane (plane, &crtc->dev->mode_config.plane_list) { +if (to_intel_plane (plane)->pipe == pipe) { + plane_state =drm_atomic_get_plane_state (state, plane); +if (IS_ERR (plane_state)) +return PTR_ERR (plane_state); + } + } + +return 0 ; +} +static struct intel_crtc_state *intel_modeset_compute_config (struct drm_crtc *crtc,struct drm_atomic_state *state)@@ -11765,8 +11819,14 @@ intel_modeset_compute_config(struct drm_crtc *crtc,if (IS_ERR (pipe_config))return pipe_config; +if (needs_modeset (&pipe_config->base)) { + ret =intel_modeset_compute_planes (state, crtc); +if (ret) +return ERR_PTR (ret); + } +if (!pipe_config->base.enable) -return pipe_config; + goto done; ret =intel_modeset_pipe_config (crtc, state, pipe_config);if (ret)@@ -11784,8 +11844,8 @@ intel_modeset_compute_config(struct drm_crtc *crtc,* required changes and forcing a mode set. */ -intel_dump_pipe_config (to_intel_crtc (crtc), pipe_config,"[modeset]" ); - +intel_dump_pipe_config (to_intel_crtc (crtc), pipe_config,"[modeset]" ); +done: ret =drm_atomic_helper_check_planes (state->dev, state);if (ret)return ERR_PTR (ret);@@ -11869,6 +11929,113 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)return 0 ; } +static void __intel_set_mode_update_planes (struct drm_device *dev, +struct drm_atomic_state *state) +{ +int i; +struct drm_plane_state *old_plane_state; +struct drm_crtc_state *crtc_state; +struct drm_plane *plane; +struct drm_crtc *crtc; + +/* +* For now only swap plane state, should be replaced with a +* call to drm_atomic_helper_swap_state +*/ +for_each_plane_in_state (state, plane, old_plane_state, i) { +struct drm_plane *plane = state->planes[i]; + +if (!plane) +continue ; + + plane->state->state = state; +swap (state->plane_states[i], plane->state); + plane->state->state =NULL ; + } + +for_each_crtc_in_state (state, crtc, crtc_state, i) { +const struct drm_crtc_helper_funcs *funcs; + + funcs = crtc->helper_private; + +if (!funcs || !funcs->atomic_begin) +continue ; + +/* XXX: Hack because crtc state is not swapped */ + crtc->state->mode_changed = crtc_state->mode_changed; + crtc->state->active_changed = crtc_state->active_changed; + +DRM_DEBUG_ATOMIC ("Calling atomic_begin on crtc %i\n" , i); + funcs->atomic_begin (crtc); + } + +for_each_plane_in_state (state, plane, old_plane_state, i) { +bool visible =to_intel_plane_state (plane->state)->visible; +struct intel_plane *intel_plane =to_intel_plane (plane); +const struct drm_plane_helper_funcs *funcs; +
drivers/gpu/drm/i915/intel_display.c static int __intel_set_mode_checks(struct drm_atomic_state *state) · line 11986
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +10
-0+ crtc = plane->state->crtc; + funcs = plane->helper_private; + +if (!funcs) +continue ; + +DRM_DEBUG_ATOMIC ("Plane %i is visible: %i\n" , i, visible); + +if (!visible) + funcs->atomic_update (plane, old_plane_state);
I'm getting a NULL pointer dereference in intel_commit_primary_plane() because of a NULL crtc when an already disabled plane is disabled again. Should we just skip the update here?
Quoted 39 lines
drivers/gpu/drm/i915/intel_display.c +39
-0+else if (crtc->state->mode_changed) + intel_plane->disable_plane (plane, crtc, true); + } +} + +static void __intel_set_mode_cleanup_planes (struct drm_device *dev, +struct drm_atomic_state *old_state) +{ +int nplanes = dev->mode_config.num_total_plane; +int ncrtcs = dev->mode_config.num_crtc; +int i; + +for (i =0 ; i < nplanes; i++) { +const struct drm_plane_helper_funcs *funcs; +struct drm_plane *plane = old_state->planes[i]; +struct drm_plane_state *old_plane_state; + +if (!plane) +continue ; + + funcs = plane->helper_private; + +if (!funcs) +continue ; + + old_plane_state = old_state->plane_states[i]; + +if (to_intel_plane_state (plane->state)->visible) { +DRM_DEBUG_ATOMIC ("Plane %i is updated\n" , i); + funcs->atomic_update (plane, old_plane_state); + }else +DRM_DEBUG_ATOMIC ("Plane %i is left alone\n" , i); + } + +for (i =0 ; i < ncrtcs; i++) { +const struct drm_crtc_helper_funcs *funcs; +struct drm_crtc *crtc = old_state->crtcs[i]; + +if (!crtc)
drivers/gpu/drm/i915/intel_display.c static int __intel_set_mode_checks(struct drm_atomic_state *state) · line 12035
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +10
-0+continue ; + + funcs = crtc->helper_private; + +if (!funcs || !funcs->atomic_flush) +continue ; + +DRM_DEBUG_ATOMIC ("Calling atomic_flush on crtc %i\n" , i); + funcs->atomic_flush (crtc); + }
These loops could use for_each_{crtc,plane}_in_state macros.
Quoted 43 lines
drivers/gpu/drm/i915/intel_display.c +7
-6+drm_atomic_helper_cleanup_planes (dev, old_state); +} +static int __intel_set_mode (struct drm_crtc *modeset_crtc,struct intel_crtc_state *pipe_config) {@@ -11877,7 +12044,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,struct drm_atomic_state *state = pipe_config->base.state;struct drm_crtc *crtc;struct drm_crtc_state *crtc_state; -int ret =0 ; +int ret;int i; ret =__intel_set_mode_checks (state);@@ -11888,14 +12055,14 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,if (ret)return ret; +__intel_set_mode_update_planes (dev, state); +for_each_crtc_in_state (state, crtc, crtc_state, i) {if (!needs_modeset (crtc_state))continue ; -intel_crtc_disable_planes (crtc); +intel_crtc_dpms_overlay_disable (to_intel_crtc (crtc)); dev_priv->display.crtc_disable (crtc); -if (!crtc_state->enable) -drm_plane_helper_disable (crtc->primary); }/* crtc->mode is already used by the ->mode_set callbacks, hence we need @@ -11926,8 +12093,6 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,modeset_update_crtc_power_domains (state); -drm_atomic_helper_commit_planes (dev, state); -/* Now enable the clocks, plane, pipe, and connectors that we set up. */ for_each_crtc_in_state (state, crtc, crtc_state, i) {if (!crtc->state->enable)
drivers/gpu/drm/i915/intel_display.c static int __intel_set_mode(struct drm_crtc *modeset_crtc, · line 12104
Quoted 7 lines
drivers/gpu/drm/i915/intel_display.c +2
-2@@ -11935,13 +12100,13 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,update_scanline_offset (to_intel_crtc (crtc)); - dev_priv->display.crtc_enable (crtc); -intel_crtc_enable_planes (crtc); +if (needs_modeset (crtc->state)) + dev_priv->display.crtc_enable (crtc);
In v2 of the patch that removes modeset_pipes and friends, I added a check for needs_modeset() before update_scanline_offset().
Quoted 11 lines
drivers/gpu/drm/i915/intel_display.c +2
-2} -/* FIXME: add subpixel order */ +__intel_set_mode_cleanup_planes (dev, state); -drm_atomic_helper_cleanup_planes (dev, state); +/* FIXME: add subpixel order */ drm_atomic_state_free (state);
drivers/gpu/drm/i915/intel_display.c intel_modeset_stage_output_state(struct drm_device *dev, · line 12338
Quoted 4 lines
drivers/gpu/drm/i915/intel_display.c +1
-0@@ -12170,6 +12335,7 @@ intel_modeset_stage_output_state(struct drm_device *dev,return ret; crtc_state->enable =drm_atomic_connectors_for_crtc (state, crtc); + crtc_state->active = crtc_state->enable;
Should this be a separate fix?
Quoted 50 lines
drivers/gpu/drm/i915/intel_display.c +0
-30} ret = intel_modeset_setup_plane_state(state, set->crtc, set->mode,@@ -12190,20 +12356,11 @@ intel_modeset_stage_output_state(struct drm_device *dev,return 0 ; } -static bool primary_plane_visible (struct drm_crtc *crtc) -{ -struct intel_plane_state *plane_state = -to_intel_plane_state (crtc->primary->state); - -return plane_state->visible; -} -static int intel_crtc_set_config (struct drm_mode_set *set) {struct drm_device *dev;struct drm_atomic_state *state =NULL ;struct intel_crtc_state *pipe_config; -bool primary_plane_was_visible;int ret;BUG_ON (!set);@@ -12242,38 +12399,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)intel_update_pipe_size (to_intel_crtc (set->crtc)); - primary_plane_was_visible =primary_plane_visible (set->crtc); - ret =intel_set_mode_with_config (set->crtc, pipe_config); - -if (ret ==0 && - pipe_config->base.enable && - pipe_config->base.planes_changed && - !needs_modeset (&pipe_config->base)) { -struct intel_crtc *intel_crtc =to_intel_crtc (set->crtc); - -/* -* We need to make sure the primary plane is re-enabled if it -* has previously been turned off. -*/ -if (ret ==0 && !primary_plane_was_visible && -primary_plane_visible (set->crtc)) { -WARN_ON (!intel_crtc->active); -intel_post_enable_primary (set->crtc); - } - -/* -* In the fastboot case this may be our only check of the
drivers/gpu/drm/i915/intel_display.c static int intel_crtc_set_config(struct drm_mode_set *set) · line 12276
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +0
-10-* state after boot. It would be better to only do it on -* the first update, but we don't have a nice way of doing that -* (and really, set_config isn't used much for high freq page -* flipping, so increasing its cost here shouldn't be a big -* deal). -*/ -if (i915.fastboot && ret ==0 ) -intel_modeset_check_state (set->crtc->dev); - } -
Maybe this could be a separate patch?
Quoted 218 lines
drivers/gpu/drm/i915/intel_display.c +120
-60if (ret) {DRM_DEBUG_KMS ("failed to set mode on [CRTC:%d], err = %d\n" , set->crtc->base.id, ret);@@ -12413,6 +12539,9 @@ bool intel_wm_need_update(struct drm_plane *plane, plane->state->rotation != state->rotation)return true; +if (plane->state->crtc_w != state->crtc_w) +return true; +return false; }@@ -12507,74 +12636,22 @@ intel_check_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) {struct drm_device *dev = plane->dev; -struct drm_i915_private *dev_priv = dev->dev_private;struct drm_crtc *crtc = state->base.crtc; -struct intel_crtc *intel_crtc;struct drm_framebuffer *fb = state->base.fb;struct drm_rect *dest = &state->dst;struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip;bool can_position = false; -int ret; - - crtc = crtc ? crtc : plane->crtc; - intel_crtc =to_intel_crtc (crtc);if (INTEL_INFO (dev)->gen >=9 ) can_position = true; - ret =drm_plane_helper_check_update (plane, crtc, fb, - src, dest, clip, -DRM_PLANE_HELPER_NO_SCALING , -DRM_PLANE_HELPER_NO_SCALING , - can_position, true, - &state->visible); -if (ret) -return ret; - -if (intel_crtc->active) { -struct intel_plane_state *old_state = -to_intel_plane_state (plane->state); - - intel_crtc->atomic.wait_for_flips = true; - -/* -* FBC does not work on some platforms for rotated -* planes, so disable it when rotation is not 0 and -* update it when rotation is set back to 0. -* -* FIXME: This is redundant with the fbc update done in -* the primary plane enable function except that that -* one is done too late. We eventually need to unify -* this. -*/ -if (state->visible && -INTEL_INFO (dev)->gen <=4 && !IS_G4X (dev) && - dev_priv->fbc.crtc == intel_crtc && - state->base.rotation !=BIT (DRM_ROTATE_0 )) { - intel_crtc->atomic.disable_fbc = true; - } - -if (state->visible && !old_state->visible) { -/* -* BDW signals flip done immediately if the plane -* is disabled, even if the plane enable is already -* armed to occur at the next vblank :( -*/ -if (IS_BROADWELL (dev)) - intel_crtc->atomic.wait_vblank = true; - } - - intel_crtc->atomic.fb_bits |= -INTEL_FRONTBUFFER_PRIMARY (intel_crtc->pipe); - - intel_crtc->atomic.update_fbc = true; - -if (intel_wm_need_update (plane, &state->base)) - intel_crtc->atomic.update_wm = true; - } - -return 0 ; +return drm_plane_helper_check_update (plane, crtc, fb, + src, dest, clip, +DRM_PLANE_HELPER_NO_SCALING , +DRM_PLANE_HELPER_NO_SCALING , + can_position, true, + &state->visible); } static void@@ -12600,8 +12677,10 @@ intel_commit_primary_plane(struct drm_plane *plane,/* FIXME: kill this fastboot hack */ intel_update_pipe_size (intel_crtc); - dev_priv->display.update_primary_plane (crtc, plane->fb, - crtc->x, crtc->y); + dev_priv->display.update_primary_plane (crtc, + state->base.fb, + state->src.x1 >>16 , + state->src.y1 >>16 ); } }@@ -12616,6 +12695,123 @@ intel_disable_primary_plane(struct drm_plane *plane, dev_priv->display.update_primary_plane (crtc,NULL ,0 ,0 ); } +/* Transitional checking here, mostly for plane updates */ +static int intel_atomic_check_crtc (struct drm_crtc *crtc, +struct drm_crtc_state *crtc_state) +{ +struct intel_crtc_state *pipe_config =to_intel_crtc_state (crtc_state); +struct intel_crtc *intel_crtc =to_intel_crtc (crtc); +struct drm_atomic_state *state = crtc_state->state; +struct drm_device *dev = crtc->dev; +struct drm_i915_private *dev_priv = dev->dev_private; +unsigned plane_mask; +int i, nplanes = dev->mode_config.num_total_plane, idx; +bool mode_changed =needs_modeset (crtc_state); +bool is_crtc_enabled = crtc_state->active; +bool was_crtc_enabled = crtc->state->active && intel_crtc->active; + +memset (&intel_crtc->atomic,0 ,sizeof (intel_crtc->atomic)); + intel_crtc->atomic.update_wm = mode_changed; + + idx =drm_crtc_index (crtc); +DRM_DEBUG_ATOMIC ("Crtc %i was enabled %i now enabled: %i\n" , + idx, was_crtc_enabled, is_crtc_enabled); + + plane_mask = crtc_state->plane_mask | crtc->state->plane_mask; +for (i =0 ; i < nplanes; i++) { +struct intel_plane_state *plane_state, *old_plane_state; +struct intel_plane *plane =to_intel_plane (state->planes[i]); +bool turn_off, turn_on, visible, was_visible; +struct drm_framebuffer *fb; + +if (!plane) +continue ; + + plane_state =to_intel_plane_state (state->plane_states[i]); + old_plane_state =to_intel_plane_state (plane->base.state); + + was_visible = old_plane_state->visible && was_crtc_enabled; + visible = plane_state->visible && is_crtc_enabled; + + turn_off = was_visible && (!visible || mode_changed); + turn_on = visible && (!was_visible || mode_changed); + fb = plane_state->base.fb; + +DRM_DEBUG_ATOMIC ("Crtc %i has plane %i with fb %i\n" , idx, +drm_plane_index (&plane->base), fb ? fb->base.id :-1 ); +DRM_DEBUG_ATOMIC ("\tvisible %i -> %i, off %i, on %i, ms %i\n" , + was_visible, visible, turn_off, turn_on, mode_changed); + +/* plane being turned off as part of modeset or changes? */ +if (intel_wm_need_update (&plane->base, &plane_state->base)) + intel_crtc->atomic.update_wm = true; + +/* +* 'prepare' is never called when plane is being disabled, so +* we need to handle frontbuffer tracking as a special case +*/ +if (old_plane_state->base.fb && !plane_state->base.fb) + intel_crtc->atomic.disabled_planes |= + (1 <<drm_plane_index (&plane->base)); + +switch (plane->base.type) { +case DRM_PLANE_TYPE_PRIMARY : + intel_crtc->atomic.fb_bits |= +INTEL_FRONTBUFFER_PRIMARY (intel_crtc->pipe); + + intel_crtc->atomic.wait_for_flips = true; + intel_crtc->atomic.pre_disable_primary = turn_off; + intel_crtc->atomic.post_enable_primary = turn_on; + +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->base.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 = true; +break ; +case DRM_PLANE_TYPE_CURSOR : + intel_crtc->atomic.fb_bits |= +INTEL_FRONTBUFFER_CURSOR (intel_crtc->pipe); +break ; +case DRM_PLANE_TYPE_OVERLAY : + intel_crtc->atomic.fb_bits |= +INTEL_FRONTBUFFER_SPRITE (intel_crtc->pipe); + +if (turn_off) {
drivers/gpu/drm/i915/intel_display.c intel_disable_primary_plane(struct drm_plane *plane, · line 12814
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +10
-0+ intel_crtc->atomic.wait_vblank = true; + intel_crtc->atomic.update_sprite_watermarks |= + (1 <<drm_plane_index (&plane->base)); + } +break ; + } + } +return 0 ; +} +
Is there a specific reason why this needs to be in check_crtc vs check_plane? IMO, it would make review easier if this move was a separate patch.
Quoted 89 lines
drivers/gpu/drm/i915/intel_display.c +11
-17static void intel_begin_crtc_commit (struct drm_crtc *crtc) {struct drm_device *dev = crtc->dev;@@ -12664,10 +12860,13 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)intel_runtime_pm_get (dev_priv);/* Perform vblank evasion around commit operation */ -if (intel_crtc->active) +if (intel_crtc->active && !needs_modeset (crtc->state) && + !dev_priv->power_domains.init_power_on) intel_crtc->atomic.evade =intel_pipe_update_start (intel_crtc, &intel_crtc->atomic.start_vbl_count); +else + intel_crtc->atomic.evade = false; }static void intel_finish_crtc_commit (struct drm_crtc *crtc)@@ -12703,6 +12902,8 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc) false, false);memset (&intel_crtc->atomic,0 ,sizeof (intel_crtc->atomic)); + crtc->state->mode_changed = false; + crtc->state->active_changed = false; }/** @@ -12812,13 +13013,9 @@ intel_check_cursor_plane(struct drm_plane *plane,struct drm_rect *src = &state->src;const struct drm_rect *clip = &state->clip;struct drm_i915_gem_object *obj =intel_fb_obj (fb); -struct intel_crtc *intel_crtc;unsigned stride;int ret; - crtc = crtc ? crtc : plane->crtc; - intel_crtc =to_intel_crtc (crtc); - ret = drm_plane_helper_check_update(plane, crtc, fb, src, dest, clip, DRM_PLANE_HELPER_NO_SCALING,@@ -12830,7 +13027,7 @@ intel_check_cursor_plane(struct drm_plane *plane,/* if we want to turn off the cursor ignore width and height */ if (!obj) - goto finish; +return 0 ;/* Check for which cursor types we support */ if (!cursor_size_ok (dev, state->base.crtc_w, state->base.crtc_h)) {@@ -12847,19 +13044,10 @@ intel_check_cursor_plane(struct drm_plane *plane,if (fb->modifier[0 ] !=DRM_FORMAT_MOD_NONE ) {DRM_DEBUG_KMS ("cursor cannot be tiled\n" ); - ret = -EINVAL ; - } - -finish: -if (intel_crtc->active) { -if (plane->state->crtc_w != state->base.crtc_w) - intel_crtc->atomic.update_wm = true; - - intel_crtc->atomic.fb_bits |= -INTEL_FRONTBUFFER_CURSOR (intel_crtc->pipe); +return -EINVAL ; } -return ret; +return 0 ; } static void@@ -14089,6 +14277,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)WARN_ON (crtc->active); crtc->base.state->enable = false; + crtc->base.state->active = false; crtc->base.enabled = false; }@@ -14117,6 +14306,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) crtc->active ? "enabled" : "disabled"); crtc->base.state->enable = crtc->active; + crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active;/* Because we only establish the connector -> encoder ->
drivers/gpu/drm/i915/intel_display.c static void intel_modeset_readout_hw_state(struct drm_device *dev) · line 14450
Quoted 6 lines
drivers/gpu/drm/i915/intel_display.c +1
-0@@ -14255,6 +14445,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) crtc->config); crtc->base.state->enable = crtc->active; + crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active;
Should this be a separate bug fix?
Ander
Quoted 49 lines
drivers/gpu/drm/i915/intel_display.c +0
-0plane_state =to_intel_plane_state (primary->state);drivers/gpu/drm/i915/intel_sprite.c +3
-23@@ -811,12 +811,8 @@ intel_check_sprite_plane(struct drm_plane *plane,int max_scale, min_scale;int pixel_size; - intel_crtc = intel_crtc ? intel_crtc :to_intel_crtc (plane->crtc); - -if (!fb) { - state->visible = false; - goto finish; - } +if (!fb) +return 0 ;/* Don't modify another pipe's plane */ if (intel_plane->pipe != intel_crtc->pipe) {@@ -847,7 +843,7 @@ intel_check_sprite_plane(struct drm_plane *plane, vscale =drm_rect_calc_vscale_relaxed (src, dst, min_scale, max_scale);BUG_ON (vscale <0 ); - state->visible =drm_rect_clip_scaled (src, dst, clip, hscale, vscale); + state->visible =drm_rect_clip_scaled (src, dst, clip, hscale, vscale); crtc_x = dst->x1; crtc_y = dst->y1;@@ -952,29 +948,6 @@ intel_check_sprite_plane(struct drm_plane *plane, dst->y1 = crtc_y; dst->y2 = crtc_y + crtc_h; -finish: -/* -* If the sprite is completely covering the primary plane, -* we can disable the primary and save power. -*/ -if (intel_crtc->active) { - intel_crtc->atomic.fb_bits |= -INTEL_FRONTBUFFER_SPRITE (intel_crtc->pipe); - -if (intel_wm_need_update (plane, &state->base)) - intel_crtc->atomic.update_wm = true; - -if (!state->visible) { -/* -* Avoid underruns when disabling the sprite. -* FIXME remove once watermark updates are done properly.
drivers/gpu/drm/i915/intel_sprite.c intel_check_sprite_plane(struct drm_plane *plane, · line 953
Quoted 10 lines
drivers/gpu/drm/i915/intel_sprite.c +0
-7-*/ - intel_crtc->atomic.wait_vblank = true; - intel_crtc->atomic.update_sprite_watermarks |= - (1 <<drm_plane_index (plane)); - } - } -return 0 ; }
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
RFC PATCH 5/5 drm/i915: Implement intel_crtc_toggle using atomic state
In reply to [email protected]From: Maarten Lankhorst <[email protected]>
Assume the function is locked with drm_modeset_lock_all for now.
--- drivers/gpu/drm/i915/intel_display.c | 131 ++++++++++------------------------- 1 file changed, 38 insertions(+), 93 deletions(-)
drivers/gpu/drm/i915/intel_display.c +38 -93
@@ -106,8 +106,6 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc);static void intel_finish_crtc_commit (struct drm_crtc *crtc);static void skl_init_scalers (struct drm_device *dev,struct intel_crtc *intel_crtc,struct intel_crtc_state *crtc_state); -static void intel_crtc_enable_planes (struct drm_crtc *crtc); -static void intel_crtc_disable_planes (struct drm_crtc *crtc);static void intel_pre_disable_primary (struct drm_crtc *crtc);static void intel_post_enable_primary (struct drm_crtc *crtc);
@@ -2197,28 +2195,6 @@ void intel_flush_primary_plane(struct drm_i915_private *dev_priv,POSTING_READ (reg); } -/** -* intel_enable_primary_hw_plane - enable the primary plane on a given pipe -* @plane: plane to be enabled -* @crtc: crtc for the plane -* -* Enable @plane on @crtc, making sure that the pipe is running first. -*/ -static void intel_enable_primary_hw_plane (struct drm_plane *plane, -struct drm_crtc *crtc) -{ -struct drm_device *dev = plane->dev; -struct drm_i915_private *dev_priv = dev->dev_private; -struct intel_crtc *intel_crtc =to_intel_crtc (crtc); - -/* If the pipe isn't enabled, we can't pump pixels and may hang */ -assert_pipe_enabled (dev_priv, intel_crtc->pipe); -to_intel_plane_state (plane->state)->visible = true; - - dev_priv->display.update_primary_plane (crtc, plane->fb, - crtc->x, crtc->y); -} -static bool need_vtd_wa (struct drm_device *dev) {#ifdef CONFIG_INTEL_IOMMU
@@ -4455,20 +4431,6 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc) } } -static void intel_enable_sprite_planes (struct drm_crtc *crtc) -{ -struct drm_device *dev = crtc->dev; -enum pipe pipe =to_intel_crtc (crtc)->pipe; -struct drm_plane *plane; -struct intel_plane *intel_plane; - -drm_for_each_legacy_plane (plane, &dev->mode_config.plane_list) { - intel_plane =to_intel_plane (plane); -if (intel_plane->pipe == pipe) -intel_plane_restore (&intel_plane->base); - } -} -void hsw_enable_ips (struct intel_crtc *crtc) {struct drm_device *dev = crtc->base.dev;
@@ -4698,44 +4660,6 @@ intel_pre_disable_primary(struct drm_crtc *crtc)hsw_disable_ips (intel_crtc); } -static void intel_crtc_enable_planes (struct drm_crtc *crtc) -{ -intel_enable_primary_hw_plane (crtc->primary, crtc); -intel_enable_sprite_planes (crtc); -intel_crtc_update_cursor (crtc, true); - -intel_post_enable_primary (crtc); -} - -static void intel_crtc_disable_planes (struct drm_crtc *crtc) -{ -struct drm_device *dev = crtc->dev; -struct intel_crtc *intel_crtc =to_intel_crtc (crtc); -struct intel_plane *intel_plane; -int pipe = intel_crtc->pipe; - -intel_crtc_wait_for_pending_flips (crtc); - -intel_pre_disable_primary (crtc); - -intel_crtc_dpms_overlay_disable (intel_crtc); -for_each_intel_plane (dev, intel_plane) { -if (intel_plane->pipe == pipe) { -struct drm_crtc *from = intel_plane->base.crtc; - - intel_plane->disable_plane (&intel_plane->base, - from ?: crtc, true); - } - } - -/* -* FIXME: Once we grow proper nuclear flip support out of this we need -* to compute the mask of flip planes precisely. For the time being -* consider this a flip to a NULL plane. -*/ -intel_frontbuffer_flip (dev,INTEL_FRONTBUFFER_ALL_MASK (pipe)); -} -static void ironlake_crtc_enable (struct drm_crtc *crtc) {struct drm_device *dev = crtc->dev;
@@ -5728,10 +5652,13 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)void intel_crtc_control (struct drm_crtc *crtc,bool enable) {struct drm_device *dev = crtc->dev; -struct drm_i915_private *dev_priv = dev->dev_private; +struct drm_mode_config *config = &dev->mode_config; +struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;struct intel_crtc *intel_crtc =to_intel_crtc (crtc); -enum intel_display_power_domain domain; -unsigned long domains; +struct intel_crtc_state *pipe_config; +struct drm_plane_state *plane_state; +struct drm_atomic_state *state; +int ret;if (enable == intel_crtc->active)return ;
@@ -5740,23 +5667,41 @@ void intel_crtc_control(struct drm_crtc *crtc, bool enable)return ; crtc->state->active = enable; -if (enable) { - domains =get_crtc_power_domains (crtc); -for_each_power_domain (domain, domains) -intel_display_power_get (dev_priv, domain); - intel_crtc->enabled_power_domains = domains; - dev_priv->display.crtc_enable (crtc); -intel_crtc_enable_planes (crtc); - }else { -intel_crtc_disable_planes (crtc); - dev_priv->display.crtc_disable (crtc); +/* this function should be called with drm_modeset_lock_all for now */ +if (WARN_ON (!ctx)) +return ; +lockdep_assert_held (&ctx->ww_ctx); - domains = intel_crtc->enabled_power_domains; -for_each_power_domain (domain, domains) -intel_display_power_put (dev_priv, domain); - intel_crtc->enabled_power_domains =0 ; + state =drm_atomic_state_alloc (dev); +if (WARN_ON (!state)) +return ; + + state->acquire_ctx = ctx; + state->allow_modeset = true; + + pipe_config =intel_atomic_get_crtc_state (state, intel_crtc); +if (IS_ERR (pipe_config)) { + ret =PTR_ERR (pipe_config); + goto err; + } + pipe_config->base.active = enable; + + plane_state =drm_atomic_get_plane_state (state, crtc->primary); +if (IS_ERR (plane_state)) { + ret =PTR_ERR (plane_state); + goto err; } + + ret =intel_set_mode (crtc, state); +if (!ret) +return ; + +DRM_ERROR ("Failed to toggle crtc!\n" ); + +err: +DRM_ERROR ("Updating crtc active failed with %i\n" , ret); +drm_atomic_state_free (state); }/**
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH RFC 5/5] drm/i915: Implement intel_crtc_toggle using atomic state
Op 22-04-15 om 13:24 schreef [email protected]:
Quoted 30 lines
From: Maarten Lankhorst <[email protected]>
Assume the function is locked with
drm_modeset_lock_allfor now.Signed-off-by Maarten Lankhorst <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 131 ++++++++++------------------------- 1 file changed, 38 insertions(+), 93 deletions(-)<snip>
static voidironlake_crtc_enable(struct drm_crtc *crtc){drivers/gpu/drm/i915/intel_display.c +6
-3struct drm_device *dev = crtc->dev;@@ -5728,10 +5652,13 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)void intel_crtc_control (struct drm_crtc *crtc,bool enable) {struct drm_device *dev = crtc->dev; -struct drm_i915_private *dev_priv = dev->dev_private; +struct drm_mode_config *config = &dev->mode_config; +struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;struct intel_crtc *intel_crtc =to_intel_crtc (crtc); -enum intel_display_power_domain domain; -unsigned long domains; +struct intel_crtc_state *pipe_config; +struct drm_plane_state *plane_state; +struct drm_atomic_state *state; +int ret;if (enable == intel_crtc->active)return ;
drivers/gpu/drm/i915/intel_display.c void intel_crtc_control(struct drm_crtc *crtc, bool enable) · line 5669
Quoted 3 lines
drivers/gpu/drm/i915/intel_display.c +0
-0@@ -5740,23 +5667,41 @@ void intel_crtc_control(struct drm_crtc *crtc, bool enable)return ; crtc->state->active = enable;
^Oops, this line should be removed now or this patch will glitch badly.
And a lot of places that use state->enable should use state->active instead.
Still leaving this here as it's a RFC. :-)
Quoted 42 lines
drivers/gpu/drm/i915/intel_display.c +25
-14-if (enable) { - domains =get_crtc_power_domains (crtc); -for_each_power_domain (domain, domains) -intel_display_power_get (dev_priv, domain); - intel_crtc->enabled_power_domains = domains; - dev_priv->display.crtc_enable (crtc); -intel_crtc_enable_planes (crtc); - }else { -intel_crtc_disable_planes (crtc); - dev_priv->display.crtc_disable (crtc); +/* this function should be called with drm_modeset_lock_all for now */ +if (WARN_ON (!ctx)) +return ; +lockdep_assert_held (&ctx->ww_ctx); - domains = intel_crtc->enabled_power_domains; -for_each_power_domain (domain, domains) -intel_display_power_put (dev_priv, domain); - intel_crtc->enabled_power_domains =0 ; + state =drm_atomic_state_alloc (dev); +if (WARN_ON (!state)) +return ; + + state->acquire_ctx = ctx; + state->allow_modeset = true; + + pipe_config =intel_atomic_get_crtc_state (state, intel_crtc); +if (IS_ERR (pipe_config)) { + ret =PTR_ERR (pipe_config); + goto err; + } + pipe_config->base.active = enable; + + plane_state =drm_atomic_get_plane_state (state, crtc->primary); +if (IS_ERR (plane_state)) { + ret =PTR_ERR (plane_state); + goto err; } + + ret =intel_set_mode (crtc, state); +if (!ret)
drivers/gpu/drm/i915/intel_display.c void intel_crtc_control(struct drm_crtc *crtc, bool enable) · line 5707
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +7
-0+return ; + +DRM_ERROR ("Failed to toggle crtc!\n" ); + +err: +DRM_ERROR ("Updating crtc active failed with %i\n" , ret); +drm_atomic_state_free (state); }/**
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
RFC PATCH v2 5/5 drm/i915: Implement intel_crtc_toggle using atomic state
In reply to [email protected]Assume the function is locked with drm_modeset_lock_all for now.
---
Changes since RFC v1:
- Get rid of the crtc->state->active assignment in intel_crtc_control, it caused the whole state to be confused.
- Convert some places that use state->enable to state->active.
- crtc->state->active should mirror crtc->active now, latter should be removed at some point.
drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 174 +++++++++++++---------------------- 2 files changed, 65 insertions(+), 111 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +1 -1
@@ -796,7 +796,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,return -EINVAL ; } -if (!crtc->state->enable) { +if (!crtc->state->active) {DRM_DEBUG_KMS ("crtc %d is disabled\n" , pipe);return -EBUSY ; }
drivers/gpu/drm/i915/intel_display.c +64 -110
309 lines not on this page · raw message
Signature · 7 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[PATCH v2 RFC 6/5] drm/i915: Update less state during modeset.
In reply to [email protected]No need to repeatedly call update_watermarks, or update_fbc. For update_watermarks once should be enough after disabling crtc's and swapping the state.
Down to a single call to update_watermarks in .crtc_enable
---
There is no v1, this patch was added later to the series. :-)
The order in ironlake_crtc_disable is slightly changed with intel_disable_shared_dpll and ironlake_fdi_pll_disable order changed. I have no idea if this can cause problems, but it works on my system.
drivers/gpu/drm/i915/intel_display.c | 79 +++++++++++++----------------------- 1 file changed, 28 insertions(+), 51 deletions(-)
drivers/gpu/drm/i915/intel_display.c +28 -51
@@ -4588,10 +4588,6 @@ intel_post_enable_primary(struct drm_crtc *crtc)*/ hsw_enable_ips (intel_crtc); -mutex_lock (&dev->struct_mutex); -intel_fbc_update (dev); -mutex_unlock (&dev->struct_mutex); -/* * Gen2 reports pipe underruns whenever all planes are disabled. * So don't enable underrun reporting before at least some planes
@@ -4646,11 +4642,6 @@ intel_pre_disable_primary(struct drm_crtc *crtc)if (HAS_GMCH_DISPLAY (dev))intel_set_memory_cxsr (dev_priv, false); -mutex_lock (&dev->struct_mutex); -if (dev_priv->fbc.crtc == intel_crtc) -intel_fbc_disable (dev); -mutex_unlock (&dev->struct_mutex); -/* * FIXME IPS should be fine as long as one plane is * enabled, but in practice it seems to have problems
@@ -4876,9 +4867,6 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)int pipe = intel_crtc->pipe;u32 reg, temp; -if (!intel_crtc->active) -return ; -for_each_encoder_on_crtc (dev, crtc, encoder) encoder->disable (encoder);
@@ -4916,18 +4904,8 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)I915_WRITE (PCH_DPLL_SEL , temp); } -/* disable PCH DPLL */ -intel_disable_shared_dpll (intel_crtc); -ironlake_fdi_pll_disable (intel_crtc); } - - intel_crtc->active = false; -intel_update_watermarks (crtc); - -mutex_lock (&dev->struct_mutex); -intel_fbc_update (dev); -mutex_unlock (&dev->struct_mutex); }static void haswell_crtc_disable (struct drm_crtc *crtc)
@@ -4938,9 +4916,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)struct intel_encoder *encoder;enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder; -if (!intel_crtc->active) -return ; -for_each_encoder_on_crtc (dev, crtc, encoder) {intel_opregion_notify_encoder (encoder, false); encoder->disable (encoder);
@@ -4974,16 +4949,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)for_each_encoder_on_crtc (dev, crtc, encoder)if (encoder->post_disable) encoder->post_disable (encoder); - - intel_crtc->active = false; -intel_update_watermarks (crtc); - -mutex_lock (&dev->struct_mutex); -intel_fbc_update (dev); -mutex_unlock (&dev->struct_mutex); - -if (intel_crtc_to_shared_dpll (intel_crtc)) -intel_disable_shared_dpll (intel_crtc); }static void i9xx_pfit_enable (struct intel_crtc *crtc)
@@ -5603,9 +5568,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)struct intel_encoder *encoder;int pipe = intel_crtc->pipe; -if (!intel_crtc->active) -return ; -/* * On gen2 planes are double buffered but the pipe isn't, so we must * wait for planes to fully turn off before disabling the pipe.
@@ -5639,13 +5601,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)if (!IS_GEN2 (dev))intel_set_cpu_fifo_underrun_reporting (dev_priv, pipe, false); - - intel_crtc->active = false; -intel_update_watermarks (crtc); - -mutex_lock (&dev->struct_mutex); -intel_fbc_update (dev); -mutex_unlock (&dev->struct_mutex); }/* Master function to enable/disable CRTC and corresponding power wells */
@@ -12008,11 +11963,21 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,__intel_set_mode_update_planes (dev, state);for_each_crtc_in_state (state, crtc, crtc_state, i) { +struct intel_crtc *intel_crtc =to_intel_crtc (crtc); +if (!needs_modeset (crtc_state))continue ; +if (!crtc->state->active) +continue ; +intel_crtc_dpms_overlay_disable (to_intel_crtc (crtc)); dev_priv->display.crtc_disable (crtc); + + intel_crtc->active = false; + +if (intel_crtc_to_shared_dpll (intel_crtc)) +intel_disable_shared_dpll (intel_crtc); }/* crtc->mode is already used by the ->mode_set callbacks, hence we need
@@ -12045,8 +12010,11 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,/* Now enable the clocks, plane, pipe, and connectors that we set up. */ for_each_crtc_in_state (state, crtc, crtc_state, i) { -if (!crtc->state->active) +if (!crtc->state->active) { +if (needs_modeset (crtc->state)) +intel_update_watermarks (crtc);continue ; + }update_scanline_offset (to_intel_crtc (crtc));
@@ -12662,6 +12630,7 @@ static int intel_atomic_check_crtc(struct drm_crtc *crtc,memset (&intel_crtc->atomic,0 ,sizeof (intel_crtc->atomic)); intel_crtc->atomic.update_wm = mode_changed; + intel_crtc->atomic.disable_fbc = mode_changed; idx = crtc->base.id; I915_STATE_WARN(crtc->state->active != intel_crtc->active,
@@ -12763,6 +12732,9 @@ static int intel_atomic_check_crtc(struct drm_crtc *crtc,break ; } } + +if (mode_changed) + intel_crtc->atomic.update_wm = false;return 0 ; }
@@ -12802,8 +12774,13 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)if (intel_crtc->atomic.wait_for_flips)intel_crtc_wait_for_pending_flips (crtc); -if (intel_crtc->atomic.disable_fbc) -intel_fbc_disable (dev); +if (intel_crtc->atomic.disable_fbc && + dev_priv->fbc.crtc == intel_crtc) { +mutex_lock (&dev->struct_mutex); +if (dev_priv->fbc.crtc == intel_crtc) +intel_fbc_disable (dev); +mutex_unlock (&dev->struct_mutex); + }if (intel_crtc->atomic.pre_disable_primary)intel_pre_disable_primary (crtc);
@@ -12841,15 +12818,15 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc)intel_frontbuffer_flip (dev, intel_crtc->atomic.fb_bits); +if (intel_crtc->atomic.post_enable_primary) +intel_post_enable_primary (crtc); +if (intel_crtc->atomic.update_fbc) {mutex_lock (&dev->struct_mutex);intel_fbc_update (dev);mutex_unlock (&dev->struct_mutex); } -if (intel_crtc->atomic.post_enable_primary) -intel_post_enable_primary (crtc); -drm_for_each_legacy_plane (p, &dev->mode_config.plane_list)if (intel_crtc->atomic.update_sprite_watermarks &drm_plane_index (p)) intel_update_sprite_watermarks(p, crtc, 0, 0, 0,
Signature · 7 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx