Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753380AbcJMJrX (ORCPT ); Thu, 13 Oct 2016 05:47:23 -0400 Received: from foss.arm.com ([217.140.101.70]:45578 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752777AbcJMJrQ (ORCPT ); Thu, 13 Oct 2016 05:47:16 -0400 From: Brian Starkey To: daniel@ffwll.ch, dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org Subject: [PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs_modeset Date: Thu, 13 Oct 2016 10:47:08 +0100 Message-Id: <1476352028-16701-1-git-send-email-brian.starkey@arm.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <20161012065618.GI20761@phenom.ffwll.local> References: <20161012065618.GI20761@phenom.ffwll.local> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4001 Lines: 92 Add some additional comments to more explicitly describe the meaning and usage of the three CRTC modeset detection booleans: mode_changed, connectors_changed and active_changed. Suggested-by: Daniel Vetter Signed-off-by: Brian Starkey --- Hi Daniel, I guess I asked for this one :-), please just check my understanding is correct. Thanks, Brian drivers/gpu/drm/drm_atomic_helper.c | 9 +++++---- include/drm/drm_atomic.h | 11 ++++++++++- include/drm/drm_crtc.h | 5 +++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 78ea735..fb4071a 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -458,10 +458,11 @@ mode_fixup(struct drm_atomic_state *state) * removed from the crtc. * crtc_state->active_changed is set when crtc_state->active changes, * which is used for dpms. + * See also: drm_atomic_crtc_needs_modeset() * * IMPORTANT: * - * Drivers which update ->mode_changed (e.g. in their ->atomic_check hooks if a + * Drivers which set ->mode_changed (e.g. in their ->atomic_check hooks if a * plane update can't be done without a full modeset) _must_ call this function * afterwards after that change. It is permitted to call this function multiple * times for the same update, e.g. when the ->atomic_check functions depend upon @@ -510,9 +511,9 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, for_each_connector_in_state(state, connector, connector_state, i) { /* - * This only sets crtc->mode_changed for routing changes, - * drivers must set crtc->mode_changed themselves when connector - * properties need to be updated. + * This only sets crtc->connectors_changed for routing changes, + * drivers must set crtc->connectors_changed themselves when + * connector properties need to be updated. */ ret = update_connector_routing(state, connector, connector_state); diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index d9aff06..1ce255f 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -368,8 +368,17 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); * * To give drivers flexibility struct &drm_crtc_state has 3 booleans to track * whether the state CRTC changed enough to need a full modeset cycle: - * connectors_changed, mode_changed and active_change. This helper simply + * connectors_changed, mode_changed and active_changed. This helper simply * combines these three to compute the overall need for a modeset for @state. + * + * The atomic helper code sets these booleans, but drivers can and should + * change them appropriately to accurately represent whether a modeset is + * really needed. In general, drivers should avoid full modesets whenever + * possible. + * + * For example if the CRTC mode has changed, and the hardware is able to enact + * the requested mode change without going through a full modeset, the driver + * should clear mode_changed during its ->atomic_check. */ static inline bool drm_atomic_crtc_needs_modeset(struct drm_crtc_state *state) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c4a3164..1f094d2 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -116,6 +116,11 @@ struct drm_plane_helper_funcs; * never return in a failure from the ->atomic_check callback. Userspace assumes * that a DPMS On will always succeed. In other words: @enable controls resource * assignment, @active controls the actual hardware state. + * + * The three booleans active_changed, connectors_changed and mode_changed are + * intended to indicate whether a full modeset is needed, rather than strictly + * describing what has changed in a commit. + * See also: drm_atomic_crtc_needs_modeset() */ struct drm_crtc_state { struct drm_crtc *crtc; -- 1.7.9.5