From patchwork Fri Dec 4 08:45:47 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 7766451 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id E49DBBEEE1 for ; Fri, 4 Dec 2015 08:46:33 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8797F20498 for ; Fri, 4 Dec 2015 08:46:32 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id C156B2054C for ; Fri, 4 Dec 2015 08:46:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B23B58A7B8; Fri, 4 Dec 2015 00:46:27 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wm0-f52.google.com (mail-wm0-f52.google.com [74.125.82.52]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5DC3E8A7B8 for ; Fri, 4 Dec 2015 00:46:25 -0800 (PST) Received: by wmuu63 with SMTP id u63so52716090wmu.0 for ; Fri, 04 Dec 2015 00:46:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=YW3Gik0ukyz7oBehUbL9xNhT3TY1XSqh6j2ulInBNCs=; b=Wqo+2bwTNhYAb3wB6lT240ba0MRWyfUtvnspgbyVRLI9Zv1U2tJCj6NhuE3Bsac2Wo bf0qD2pTbuQFwEc91g0dTsNCHNWGe6Q/BCSjVZAqVbiH0AJhkyRFAu5HczIAnByUpKBA 1iz2GkdgPV3oYh6IHMRsmXHxbUrsODXnhluDg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=YW3Gik0ukyz7oBehUbL9xNhT3TY1XSqh6j2ulInBNCs=; b=EsY5OdqhfZO/az4xILocrPMCnmA1FS//LcOyDGqEIhKzfKDDWtuH7e4KMJw8xHHCkM L9B8bwDxt2cq5I0sFndUGOxExnzMZKNvdTVL6fnrVboZTwoZ8dYizAlCZwIwNtGGRQei hNNjjtyuiYbfjWosoiKCj07MW5Z+wWoo2fsCEL3UqvbOuK1eRABUipoJi5NTxgh8TzFI LbdA0+pa/FicWHp+ZFRTxoLDwe/ufUXcu18t/E1rrz1cj/p1NiRhvsTfTlA6buLRJncl tyjnuDH4bcFA8OIUNfUaAIiVM06LAm5SxlvyZPBttqpSJftB6rU79YacFy4YxoGP17Rj mfYg== X-Gm-Message-State: ALoCoQmjs7exvZnPi2Z+9b7FWE4OK0ankplGMTzNndwDvbRJmdz8wakmjclbwcduGmM1ij8vg/f5 X-Received: by 10.194.117.163 with SMTP id kf3mr16015823wjb.139.1449218784159; Fri, 04 Dec 2015 00:46:24 -0800 (PST) Received: from phenom.ffwll.local (212-51-149-109.fiber7.init7.net. [212.51.149.109]) by smtp.gmail.com with ESMTPSA id pc2sm6926311wjb.11.2015.12.04.00.46.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 04 Dec 2015 00:46:23 -0800 (PST) From: Daniel Vetter To: DRI Development Subject: [PATCH 06/28] drm/bridge: Improve kerneldoc Date: Fri, 4 Dec 2015 09:45:47 +0100 Message-Id: <1449218769-16577-7-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.5.1 In-Reply-To: <1449218769-16577-1-git-send-email-daniel.vetter@ffwll.ch> References: <1449218769-16577-1-git-send-email-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Intel Graphics Development X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Especially document the assumptions and semantics of the callbacks carefully. Just a warm-up excercise really. v2: Spelling fixes (Eric). v3: Consolidate more with existing docs: - Remove the overview section explaining the bridge funcs, that's now all in the drm_bridge_funcs kerneldoc in much more detail. - Use & to reference structs so that kerneldoc automatically inserts hyperlinks. Cc: Eric Anholt Cc: Archit Taneja Signed-off-by: Daniel Vetter Reviewed-by: Archit Taneja --- drivers/gpu/drm/drm_bridge.c | 69 +++++++++++------------------ include/drm/drm_crtc.h | 102 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 122 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 6b8f7211e543..26dd1cfb6078 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -31,14 +31,14 @@ /** * DOC: overview * - * drm_bridge represents a device that hangs on to an encoder. These are handy - * when a regular drm_encoder entity isn't enough to represent the entire + * struct &drm_bridge represents a device that hangs on to an encoder. These are + * handy when a regular &drm_encoder entity isn't enough to represent the entire * encoder chain. * - * A bridge is always associated to a single drm_encoder at a time, but can be + * A bridge is always associated to a single &drm_encoder at a time, but can be * either connected to it directly, or through an intermediate bridge: * - * encoder ---> bridge B ---> bridge A + * encoder ---> bridge B ---> bridge A * * Here, the output of the encoder feeds to bridge B, and that furthers feeds to * bridge A. @@ -46,11 +46,16 @@ * The driver using the bridge is responsible to make the associations between * the encoder and bridges. Once these links are made, the bridges will * participate along with encoder functions to perform mode_set/enable/disable - * through the ops provided in drm_bridge_funcs. + * through the ops provided in &drm_bridge_funcs. * * drm_bridge, like drm_panel, aren't drm_mode_object entities like planes, - * crtcs, encoders or connectors. They just provide additional hooks to get the - * desired output at the end of the encoder chain. + * crtcs, encoders or connectors and hence are not visible to userspace. They + * just provide additional hooks to get the desired output at the end of the + * encoder chain. + * + * Bridges can also be chained up using the next pointer in struct &drm_bridge. + * + * Both legacy CRTC helpers and the new atomic modeset helpers support bridges. */ static DEFINE_MUTEX(bridge_lock); @@ -122,34 +127,12 @@ EXPORT_SYMBOL(drm_bridge_attach); /** * DOC: bridge callbacks * - * The drm_bridge_funcs ops are populated by the bridge driver. The drm - * internals(atomic and crtc helpers) use the helpers defined in drm_bridge.c - * These helpers call a specific drm_bridge_funcs op for all the bridges + * The &drm_bridge_funcs ops are populated by the bridge driver. The drm + * internals (atomic and crtc helpers) use the helpers defined in drm_bridge.c + * These helpers call a specific &drm_bridge_funcs op for all the bridges * during encoder configuration. * - * When creating a bridge driver, one can implement drm_bridge_funcs op with - * the help of these rough rules: - * - * pre_enable: this contains things needed to be done for the bridge before - * its clock and timings are enabled by its source. For a bridge, its source - * is generally the encoder or bridge just before it in the encoder chain. - * - * enable: this contains things needed to be done for the bridge once its - * source is enabled. In other words, enable is called once the source is - * ready with clock and timing needed by the bridge. - * - * disable: this contains things needed to be done for the bridge assuming - * that its source is still enabled, i.e. clock and timings are still on. - * - * post_disable: this contains things needed to be done for the bridge once - * its source is disabled, i.e. once clocks and timings are off. - * - * mode_fixup: this should fixup the given mode for the bridge. It is called - * after the encoder's mode fixup. mode_fixup can also reject a mode completely - * if it's unsuitable for the hardware. - * - * mode_set: this sets up the mode for the bridge. It assumes that its source - * (an encoder or a bridge) has set the mode too. + * For detailed specification of the bridge callbacks see &drm_bridge_funcs. */ /** @@ -159,7 +142,7 @@ EXPORT_SYMBOL(drm_bridge_attach); * @mode: desired mode to be set for the bridge * @adjusted_mode: updated mode that works for this bridge * - * Calls 'mode_fixup' drm_bridge_funcs op for all the bridges in the + * Calls 'mode_fixup' &drm_bridge_funcs op for all the bridges in the * encoder chain, starting from the first bridge to the last. * * Note: the bridge passed should be the one closest to the encoder @@ -186,11 +169,11 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge, EXPORT_SYMBOL(drm_bridge_mode_fixup); /** - * drm_bridge_disable - calls 'disable' drm_bridge_funcs op for all + * drm_bridge_disable - calls 'disable' &drm_bridge_funcs op for all * bridges in the encoder chain. * @bridge: bridge control structure * - * Calls 'disable' drm_bridge_funcs op for all the bridges in the encoder + * Calls 'disable' &drm_bridge_funcs op for all the bridges in the encoder * chain, starting from the last bridge to the first. These are called before * calling the encoder's prepare op. * @@ -208,11 +191,11 @@ void drm_bridge_disable(struct drm_bridge *bridge) EXPORT_SYMBOL(drm_bridge_disable); /** - * drm_bridge_post_disable - calls 'post_disable' drm_bridge_funcs op for + * drm_bridge_post_disable - calls 'post_disable' &drm_bridge_funcs op for * all bridges in the encoder chain. * @bridge: bridge control structure * - * Calls 'post_disable' drm_bridge_funcs op for all the bridges in the + * Calls 'post_disable' &drm_bridge_funcs op for all the bridges in the * encoder chain, starting from the first bridge to the last. These are called * after completing the encoder's prepare op. * @@ -236,7 +219,7 @@ EXPORT_SYMBOL(drm_bridge_post_disable); * @mode: desired mode to be set for the bridge * @adjusted_mode: updated mode that works for this bridge * - * Calls 'mode_set' drm_bridge_funcs op for all the bridges in the + * Calls 'mode_set' &drm_bridge_funcs op for all the bridges in the * encoder chain, starting from the first bridge to the last. * * Note: the bridge passed should be the one closest to the encoder @@ -256,11 +239,11 @@ void drm_bridge_mode_set(struct drm_bridge *bridge, EXPORT_SYMBOL(drm_bridge_mode_set); /** - * drm_bridge_pre_enable - calls 'pre_enable' drm_bridge_funcs op for all + * drm_bridge_pre_enable - calls 'pre_enable' &drm_bridge_funcs op for all * bridges in the encoder chain. * @bridge: bridge control structure * - * Calls 'pre_enable' drm_bridge_funcs op for all the bridges in the encoder + * Calls 'pre_enable' &drm_bridge_funcs op for all the bridges in the encoder * chain, starting from the last bridge to the first. These are called * before calling the encoder's commit op. * @@ -278,11 +261,11 @@ void drm_bridge_pre_enable(struct drm_bridge *bridge) EXPORT_SYMBOL(drm_bridge_pre_enable); /** - * drm_bridge_enable - calls 'enable' drm_bridge_funcs op for all bridges + * drm_bridge_enable - calls 'enable' &drm_bridge_funcs op for all bridges * in the encoder chain. * @bridge: bridge control structure * - * Calls 'enable' drm_bridge_funcs op for all the bridges in the encoder + * Calls 'enable' &drm_bridge_funcs op for all the bridges in the encoder * chain, starting from the first bridge to the last. These are called * after completing the encoder's commit op. * diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index e8f8e4e9a6a1..ec3632e62b3f 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -883,24 +883,114 @@ struct drm_plane { /** * struct drm_bridge_funcs - drm_bridge control functions * @attach: Called during drm_bridge_attach - * @mode_fixup: Try to fixup (or reject entirely) proposed mode for this bridge - * @disable: Called right before encoder prepare, disables the bridge - * @post_disable: Called right after encoder prepare, for lockstepped disable - * @mode_set: Set this mode to the bridge - * @pre_enable: Called right before encoder commit, for lockstepped commit - * @enable: Called right after encoder commit, enables the bridge */ struct drm_bridge_funcs { int (*attach)(struct drm_bridge *bridge); + + /** + * @mode_fixup: + * + * This callback is used to validate and adjust a mode. The paramater + * mode is the display mode that should be fed to the next element in + * the display chain, either the final &drm_connector or the next + * &drm_bridge. The parameter adjusted_mode is the input mode the bridge + * requires. It can be modified by this callback and does not need to + * match mode. + * + * This is the only hook that allows a bridge to reject a modeset. If + * this function passes all other callbacks must succeed for this + * configuration. + * + * NOTE: + * + * This function is called in the check phase of atomic modesets, which + * can be aborted for any reason (including on userspace's request to + * just check whether a configuration would be possible). Drivers MUST + * NOT touch any persistent state (hardware or software) or data + * structures except the passed in adjusted_mode parameter. + * + * RETURNS: + * + * True if an acceptable configuration is possible, false if the modeset + * operation should be rejected. + */ bool (*mode_fixup)(struct drm_bridge *bridge, const struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode); + /** + * @disable: + * + * This callback should disable the bridge. It is called right before + * the preceding element in the display pipe is disabled. If the + * preceding element is a bridge this means it's called before that + * bridge's ->disaable function. If the preceding element is a + * &drm_encoder it's called right before the encoder's ->disable, + * ->prepare or ->dpms hook from struct &drm_encoder_helper_funcs. + * + * The bridge can assume that the display pipe (i.e. clocks and timing + * signals) feeding it is still running when this callback is called. + */ void (*disable)(struct drm_bridge *bridge); + + /** + * @post_disable: + * + * This callback should disable the bridge. It is called right after + * the preceding element in the display pipe is disabled. If the + * preceding element is a bridge this means it's called after that + * bridge's ->post_disaable function. If the preceding element is a + * &drm_encoder it's called right after the encoder's ->disable, + * ->prepare or ->dpms hook from struct &drm_encoder_helper_funcs. + * + * The bridge must assume that the display pipe (i.e. clocks and timing + * singals) feeding it is no longer running when this callback is + * called. + */ void (*post_disable)(struct drm_bridge *bridge); + + /** + * @mode_set: + * + * This callback should set the given mode on the bridge. It is called + * after the ->mode_set callback for the preceding element in the + * display pipeline has been called already. The display pipe (i.e. + * clocks and timing signals) is off when this function is called. + */ void (*mode_set)(struct drm_bridge *bridge, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode); + /** + * @pre_enable: + * + * This callback should enable the bridge. It is called right before + * the preceding element in the display pipe is enabled. If the + * preceding element is a bridge this means it's called before that + * bridge's ->pre_enable function. If the preceding element is a + * &drm_encoder it's called right before the encoder's ->enable, + * ->commit or ->dpms hook from struct &drm_encoder_helper_funcs. + * + * The display pipe (i.e. clocks and timing signals) feeding this bridge + * will not yet be running when this callback is called. The bridge must + * not enable the display link feeding the next bridge in the chain (if + * there is one) when this callback is called. + */ void (*pre_enable)(struct drm_bridge *bridge); + + /** + * @enable: + * + * This callback should enable the bridge. It is called right after + * the preceding element in the display pipe is enabled. If the + * preceding element is a bridge this means it's called after that + * bridge's ->enable function. If the preceding element is a + * &drm_encoder it's called right after the encoder's ->enable, + * ->commit or ->dpms hook from struct &drm_encoder_helper_funcs. + * + * The bridge can assume that the display pipe (i.e. clocks and timing + * signals) feeding it is running when this callback is called. This + * callback must enable the display link feeding the next bridge in the + * chain if there is one. + */ void (*enable)(struct drm_bridge *bridge); };