On Tue, Jul 07, 2015 at 09:08:12AM +0200, Maarten Lankhorst wrote:
> This can be a separate case from mode_changed, when connectors stay the
> same but only the mode is different. Drivers may choose to implement specific
> optimizations to prevent a full modeset for this case.
>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 25 +++++++++++++++++++------
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> include/drm/drm_atomic.h | 3 ++-
> include/drm/drm_crtc.h | 8 +++++---
> 4 files changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 66e76f4f43be..b818e3111380 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -124,7 +124,7 @@ steal_encoder(struct drm_atomic_state *state,
> if (IS_ERR(crtc_state))
> return PTR_ERR(crtc_state);
>
> - crtc_state->mode_changed = true;
> + crtc_state->connectors_changed = true;
>
> list_for_each_entry(connector, &config->connector_list, head) {
> if (connector->state->best_encoder != encoder)
> @@ -174,14 +174,14 @@ update_connector_routing(struct drm_atomic_state *state, int conn_idx)
> idx = drm_crtc_index(connector->state->crtc);
>
> crtc_state = state->crtc_states[idx];
> - crtc_state->mode_changed = true;
> + crtc_state->connectors_changed = true;
> }
>
> if (connector_state->crtc) {
> idx = drm_crtc_index(connector_state->crtc);
>
> crtc_state = state->crtc_states[idx];
> - crtc_state->mode_changed = true;
> + crtc_state->connectors_changed = true;
> }
> }
>
> @@ -233,7 +233,7 @@ update_connector_routing(struct drm_atomic_state *state, int conn_idx)
> idx = drm_crtc_index(connector_state->crtc);
>
> crtc_state = state->crtc_states[idx];
> - crtc_state->mode_changed = true;
> + crtc_state->connectors_changed = true;
>
> DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d]\n",
> connector->base.id,
> @@ -256,7 +256,8 @@ mode_fixup(struct drm_atomic_state *state)
> bool ret;
>
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> - if (!crtc_state->mode_changed)
> + if (!crtc_state->mode_changed &&
> + !crtc_state->connectors_changed)
> continue;
>
> drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode);
> @@ -312,7 +313,8 @@ mode_fixup(struct drm_atomic_state *state)
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> const struct drm_crtc_helper_funcs *funcs;
>
> - if (!crtc_state->mode_changed)
> + if (!crtc_state->mode_changed &&
> + !crtc_state->connectors_changed)
> continue;
>
> funcs = crtc->helper_private;
> @@ -373,7 +375,17 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> if (crtc->state->enable != crtc_state->enable) {
> DRM_DEBUG_ATOMIC("[CRTC:%d] enable changed\n",
> crtc->base.id);
> +
> + /*
> + * For clarity this assignment is done here, but
> + * enable == 0 is only true when there are no
> + * connectors and a NULL mode.
> + *
> + * The other way around is true as well. enable != 0
> + * iff connectors are attached and a mode is set.
> + */
> crtc_state->mode_changed = true;
I'd drop this one so that connectors_changed and mode_changed are truly
orthogonal. Also ->enable implies connectors changed since we do check
that there's only connected connectors if the crtc is on. Needs kerneldoc
update too ofc.
> + crtc_state->connectors_changed = true;
> }
> }
>
> @@ -2072,6 +2084,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
> state->mode_changed = false;
> state->active_changed = false;
> state->planes_changed = false;
> + state->connectors_changed = false;
> state->event = NULL;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e91613b1c871..6ddb462b4124 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -421,7 +421,7 @@ static const intel_limit_t intel_limits_bxt = {
> static bool
> needs_modeset(struct drm_crtc_state *state)
> {
> - return state->mode_changed || state->active_changed;
> + return drm_atomic_crtc_needs_modeset(state);
> }
>
> /**
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 8a3a913320eb..e67aeac2aee0 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -166,7 +166,8 @@ int __must_check drm_atomic_async_commit(struct drm_atomic_state *state);
> static inline bool
> drm_atomic_crtc_needs_modeset(struct drm_crtc_state *state)
> {
> - return state->mode_changed || state->active_changed;
> + return state->mode_changed || state->active_changed ||
> + state->connectors_changed;
> }
>
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 57ca8cc383a6..e39d6f1de5f7 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -255,12 +255,13 @@ struct drm_atomic_state;
> * @crtc: backpointer to the CRTC
> * @enable: whether the CRTC should be enabled, gates all other state
> * @active: whether the CRTC is actively displaying (used for DPMS)
> - * @mode_changed: for use by helpers and drivers when computing state updates
> - * @active_changed: for use by helpers and drivers when computing state updates
> + * @planes_changed: planes on this crtc are updated
> + * @mode_changed: crtc_state->mode or crtc_state->enable has been changed
> + * @active_changed: crtc_state->active
missing a "has changed".
> + * @connectors_changed: connectors to this crtc have been updated
> * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
> * @last_vblank_count: for helpers and drivers to capture the vblank of the
> * update to ensure framebuffer cleanup isn't done too early
> - * @planes_changed: for use by helpers and drivers when computing state updates
> * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
> * @mode: current mode timings
> * @event: optional pointer to a DRM event to signal upon completion of the
> @@ -283,6 +284,7 @@ struct drm_crtc_state {
> bool planes_changed : 1;
> bool mode_changed : 1;
> bool active_changed : 1;
> + bool connectors_changed : 1;
>
> /* attached planes bitmask:
> * WARNING: transitional helpers do not maintain plane_mask so
> --
> 2.1.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
