From patchwork Tue Jan 7 18:58:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 11321595 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 440A2930 for ; Tue, 7 Jan 2020 18:58:23 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2C67024656 for ; Tue, 7 Jan 2020 18:58:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2C67024656 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=collabora.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B38BC6E12C; Tue, 7 Jan 2020 18:58:20 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by gabe.freedesktop.org (Postfix) with ESMTPS id AABE06E125 for ; Tue, 7 Jan 2020 18:58:18 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 2E18A290504; Tue, 7 Jan 2020 18:58:17 +0000 (GMT) From: Boris Brezillon To: Andrzej Hajda , Neil Armstrong , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Daniel Vetter Subject: [PATCH v2 1/5] Revert "drm/bridge: Fix a NULL pointer dereference in drm_atomic_bridge_chain_check()" Date: Tue, 7 Jan 2020 19:58:03 +0100 Message-Id: <20200107185807.606999-2-boris.brezillon@collabora.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20200107185807.606999-1-boris.brezillon@collabora.com> References: <20200107185807.606999-1-boris.brezillon@collabora.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Boris Brezillon , dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" This reverts commit b18398c16e17 ("drm/bridge: Fix a NULL pointer dereference in drm_atomic_bridge_chain_check()"). Commit 6ed7e9625fa6 ("drm/bridge: Add a drm_bridge_state object") introduced a circular dependency between drm.ko and drm_kms_helper.ko which uncovered a misdesign in how the whole thing was implemented. Let's revert all patches depending on the bridge_state infrastructure for now. Signed-off-by: Boris Brezillon --- drivers/gpu/drm/drm_bridge.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 32d43bfeeca1..37400607e9b7 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -938,19 +938,15 @@ int drm_atomic_bridge_chain_check(struct drm_bridge *bridge, struct drm_connector_state *conn_state) { struct drm_connector *conn = conn_state->connector; - struct drm_encoder *encoder; + struct drm_encoder *encoder = bridge->encoder; struct drm_bridge *iter; int ret; - if (!bridge) - return 0; - ret = drm_atomic_bridge_chain_select_bus_fmts(bridge, crtc_state, conn_state); if (ret) return ret; - encoder = bridge->encoder; list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) { int ret; From patchwork Tue Jan 7 18:58:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 11321599 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5E4E9921 for ; Tue, 7 Jan 2020 18:58:27 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4668420848 for ; Tue, 7 Jan 2020 18:58:27 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4668420848 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=collabora.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D54A66E12D; Tue, 7 Jan 2020 18:58:20 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id DC01B6E128 for ; Tue, 7 Jan 2020 18:58:18 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 7D8052904BB; Tue, 7 Jan 2020 18:58:17 +0000 (GMT) From: Boris Brezillon To: Andrzej Hajda , Neil Armstrong , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Daniel Vetter Subject: [PATCH v2 2/5] Revert "drm/bridge: Add the necessary bits to support bus format negotiation" Date: Tue, 7 Jan 2020 19:58:04 +0100 Message-Id: <20200107185807.606999-3-boris.brezillon@collabora.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20200107185807.606999-1-boris.brezillon@collabora.com> References: <20200107185807.606999-1-boris.brezillon@collabora.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Boris Brezillon , dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" This reverts commit e351e4d5eaec ("drm/bridge: Add the necessary bits to support bus format negotiation"). Commit 6ed7e9625fa6 ("drm/bridge: Add a drm_bridge_state object") introduced a circular dependency between drm.ko and drm_kms_helper.ko which uncovered a misdesign in how the whole thing was implemented. Let's revert all patches depending on the bridge_state infrastructure for now. Signed-off-by: Boris Brezillon --- drivers/gpu/drm/drm_bridge.c | 267 +---------------------------------- include/drm/drm_bridge.h | 124 ---------------- 2 files changed, 1 insertion(+), 390 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 37400607e9b7..8e4b799150b0 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -671,261 +671,13 @@ static int drm_atomic_bridge_check(struct drm_bridge *bridge, return 0; } -/** - * drm_atomic_helper_bridge_propagate_bus_fmt() - Propagate output format to - * the input end of a bridge - * @bridge: bridge control structure - * @bridge_state: new bridge state - * @crtc_state: new CRTC state - * @conn_state: new connector state - * @output_fmt: tested output bus format - * @num_input_fmts: will contain the size of the returned array - * - * This helper is a pluggable implementation of the - * &drm_bridge_funcs.atomic_get_input_bus_fmts operation for bridges that don't - * modify the bus configuration between their input and their output. It - * returns an array of input formats with a single element set to @output_fmt. - * - * RETURNS: - * a valid format array of size @num_input_fmts, or NULL if the allocation - * failed - */ -u32 * -drm_atomic_helper_bridge_propagate_bus_fmt(struct drm_bridge *bridge, - struct drm_bridge_state *bridge_state, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state, - u32 output_fmt, - unsigned int *num_input_fmts) -{ - u32 *input_fmts; - - input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL); - if (!input_fmts) { - *num_input_fmts = 0; - return NULL; - } - - *num_input_fmts = 1; - input_fmts[0] = output_fmt; - return input_fmts; -} -EXPORT_SYMBOL(drm_atomic_helper_bridge_propagate_bus_fmt); - -static int select_bus_fmt_recursive(struct drm_bridge *first_bridge, - struct drm_bridge *cur_bridge, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state, - u32 out_bus_fmt) -{ - struct drm_bridge_state *cur_state; - unsigned int num_in_bus_fmts, i; - struct drm_bridge *prev_bridge; - u32 *in_bus_fmts; - int ret; - - prev_bridge = drm_bridge_get_prev_bridge(cur_bridge); - cur_state = drm_atomic_get_new_bridge_state(crtc_state->state, - cur_bridge); - if (WARN_ON(!cur_state)) - return -EINVAL; - - /* - * If bus format negotiation is not supported by this bridge, let's - * pass MEDIA_BUS_FMT_FIXED to the previous bridge in the chain and - * hope that it can handle this situation gracefully (by providing - * appropriate default values). - */ - if (!cur_bridge->funcs->atomic_get_input_bus_fmts) { - if (cur_bridge != first_bridge) { - ret = select_bus_fmt_recursive(first_bridge, - prev_bridge, crtc_state, - conn_state, - MEDIA_BUS_FMT_FIXED); - if (ret) - return ret; - } - - cur_state->input_bus_cfg.format = MEDIA_BUS_FMT_FIXED; - cur_state->output_bus_cfg.format = out_bus_fmt; - return 0; - } - - in_bus_fmts = cur_bridge->funcs->atomic_get_input_bus_fmts(cur_bridge, - cur_state, - crtc_state, - conn_state, - out_bus_fmt, - &num_in_bus_fmts); - if (!num_in_bus_fmts) - return -ENOTSUPP; - else if (!in_bus_fmts) - return -ENOMEM; - - if (first_bridge == cur_bridge) { - cur_state->input_bus_cfg.format = in_bus_fmts[0]; - cur_state->output_bus_cfg.format = out_bus_fmt; - kfree(in_bus_fmts); - return 0; - } - - for (i = 0; i < num_in_bus_fmts; i++) { - ret = select_bus_fmt_recursive(first_bridge, prev_bridge, - crtc_state, conn_state, - in_bus_fmts[i]); - if (ret != -ENOTSUPP) - break; - } - - if (!ret) { - cur_state->input_bus_cfg.format = in_bus_fmts[i]; - cur_state->output_bus_cfg.format = out_bus_fmt; - } - - kfree(in_bus_fmts); - return ret; -} - -/* - * This function is called by &drm_atomic_bridge_chain_check() just before - * calling &drm_bridge_funcs.atomic_check() on all elements of the chain. - * It performs bus format negotiation between bridge elements. The negotiation - * happens in reverse order, starting from the last element in the chain up to - * @bridge. - * - * Negotiation starts by retrieving supported output bus formats on the last - * bridge element and testing them one by one. The test is recursive, meaning - * that for each tested output format, the whole chain will be walked backward, - * and each element will have to choose an input bus format that can be - * transcoded to the requested output format. When a bridge element does not - * support transcoding into a specific output format -ENOTSUPP is returned and - * the next bridge element will have to try a different format. If none of the - * combinations worked, -ENOTSUPP is returned and the atomic modeset will fail. - * - * This implementation is relying on - * &drm_bridge_funcs.atomic_get_output_bus_fmts() and - * &drm_bridge_funcs.atomic_get_input_bus_fmts() to gather supported - * input/output formats. - * - * When &drm_bridge_funcs.atomic_get_output_bus_fmts() is not implemented by - * the last element of the chain, &drm_atomic_bridge_chain_select_bus_fmts() - * tries a single format: &drm_connector.display_info.bus_formats[0] if - * available, MEDIA_BUS_FMT_FIXED otherwise. - * - * When &drm_bridge_funcs.atomic_get_input_bus_fmts() is not implemented, - * &drm_atomic_bridge_chain_select_bus_fmts() skips the negotiation on the - * bridge element that lacks this hook and asks the previous element in the - * chain to try MEDIA_BUS_FMT_FIXED. It's up to bridge drivers to decide what - * to do in that case (fail if they want to enforce bus format negotiation, or - * provide a reasonable default if they need to support pipelines where not - * all elements support bus format negotiation). - */ -static int -drm_atomic_bridge_chain_select_bus_fmts(struct drm_bridge *bridge, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state) -{ - struct drm_connector *conn = conn_state->connector; - struct drm_encoder *encoder = bridge->encoder; - struct drm_bridge_state *last_bridge_state; - unsigned int i, num_out_bus_fmts; - struct drm_bridge *last_bridge; - u32 *out_bus_fmts; - int ret = 0; - - last_bridge = list_last_entry(&encoder->bridge_chain, - struct drm_bridge, chain_node); - last_bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state, - last_bridge); - if (WARN_ON(!last_bridge_state)) - return -EINVAL; - - if (last_bridge->funcs->atomic_get_output_bus_fmts) { - const struct drm_bridge_funcs *funcs = last_bridge->funcs; - - out_bus_fmts = funcs->atomic_get_output_bus_fmts(last_bridge, - last_bridge_state, - crtc_state, - conn_state, - &num_out_bus_fmts); - if (!num_out_bus_fmts) - return -ENOTSUPP; - else if (!out_bus_fmts) - return -ENOMEM; - } else { - num_out_bus_fmts = 1; - out_bus_fmts = kmalloc(sizeof(*out_bus_fmts), GFP_KERNEL); - if (!out_bus_fmts) - return -ENOMEM; - - if (conn->display_info.num_bus_formats && - conn->display_info.bus_formats) - out_bus_fmts[0] = conn->display_info.bus_formats[0]; - else - out_bus_fmts[0] = MEDIA_BUS_FMT_FIXED; - } - - for (i = 0; i < num_out_bus_fmts; i++) { - ret = select_bus_fmt_recursive(bridge, last_bridge, crtc_state, - conn_state, out_bus_fmts[i]); - if (ret != -ENOTSUPP) - break; - } - - kfree(out_bus_fmts); - - return ret; -} - -static void -drm_atomic_bridge_propagate_bus_flags(struct drm_bridge *bridge, - struct drm_connector *conn, - struct drm_atomic_state *state) -{ - struct drm_bridge_state *bridge_state, *next_bridge_state; - struct drm_bridge *next_bridge; - u32 output_flags; - - bridge_state = drm_atomic_get_new_bridge_state(state, bridge); - next_bridge = drm_bridge_get_next_bridge(bridge); - - /* - * Let's try to apply the most common case here, that is, propagate - * display_info flags for the last bridge, and propagate the input - * flags of the next bridge element to the output end of the current - * bridge when the bridge is not the last one. - * There are exceptions to this rule, like when signal inversion is - * happening at the board level, but that's something drivers can deal - * with from their &drm_bridge_funcs.atomic_check() implementation by - * simply overriding the flags value we've set here. - */ - if (!next_bridge) { - output_flags = conn->display_info.bus_flags; - } else { - next_bridge_state = drm_atomic_get_new_bridge_state(state, - next_bridge); - output_flags = next_bridge_state->input_bus_cfg.flags; - } - - bridge_state->output_bus_cfg.flags = output_flags; - - /* - * Propage the output flags to the input end of the bridge. Again, it's - * not necessarily what all bridges want, but that's what most of them - * do, and by doing that by default we avoid forcing drivers to - * duplicate the "dummy propagation" logic. - */ - bridge_state->input_bus_cfg.flags = output_flags; -} - /** * drm_atomic_bridge_chain_check() - Do an atomic check on the bridge chain * @bridge: bridge control structure * @crtc_state: new CRTC state * @conn_state: new connector state * - * First trigger a bus format negotiation before calling - * &drm_bridge_funcs.atomic_check() (falls back on + * Calls &drm_bridge_funcs.atomic_check() (falls back on * &drm_bridge_funcs.mode_fixup()) op for all the bridges in the encoder chain, * starting from the last bridge to the first. These are called before calling * &drm_encoder_helper_funcs.atomic_check() @@ -937,29 +689,12 @@ int drm_atomic_bridge_chain_check(struct drm_bridge *bridge, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { - struct drm_connector *conn = conn_state->connector; struct drm_encoder *encoder = bridge->encoder; struct drm_bridge *iter; - int ret; - - ret = drm_atomic_bridge_chain_select_bus_fmts(bridge, crtc_state, - conn_state); - if (ret) - return ret; list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) { int ret; - /* - * Bus flags are propagated by default. If a bridge needs to - * tweak the input bus flags for any reason, it should happen - * in its &drm_bridge_funcs.atomic_check() implementation such - * that preceding bridges in the chain can propagate the new - * bus flags. - */ - drm_atomic_bridge_propagate_bus_flags(iter, conn, - crtc_state->state); - ret = drm_atomic_bridge_check(iter, crtc_state, conn_state); if (ret) return ret; diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 46e15526b087..ae0595c70132 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -35,38 +35,6 @@ struct drm_bridge; struct drm_bridge_timings; struct drm_panel; -/** - * struct drm_bus_cfg - bus configuration - * - * This structure stores the configuration of a physical bus between two - * components in an output pipeline, usually between two bridges, an encoder - * and a bridge, or a bridge and a connector. - * - * The bus configuration is stored in &drm_bridge_state separately for the - * input and output buses, as seen from the point of view of each bridge. The - * bus configuration of a bridge output is usually identical to the - * configuration of the next bridge's input, but may differ if the signals are - * modified between the two bridges, for instance by an inverter on the board. - * The input and output configurations of a bridge may differ if the bridge - * modifies the signals internally, for instance by performing format - * conversion, or modifying signals polarities. - */ -struct drm_bus_cfg { - /** - * @format: format used on this bus (one of the MEDIA_BUS_FMT_* format) - * - * This field should not be directly modified by drivers - * (&drm_atomic_bridge_chain_select_bus_fmts() takes care of the bus - * format negotiation). - */ - u32 format; - - /** - * @flags: DRM_BUS_* flags used on this bus - */ - u32 flags; -}; - /** * struct drm_bridge_state - Atomic bridge state object * @base: inherit from &drm_private_state @@ -76,16 +44,6 @@ struct drm_bridge_state { struct drm_private_state base; struct drm_bridge *bridge; - - /** - * @input_bus_cfg: input bus configuration - */ - struct drm_bus_cfg input_bus_cfg; - - /** - * @output_bus_cfg: input bus configuration - */ - struct drm_bus_cfg output_bus_cfg; }; static inline struct drm_bridge_state * @@ -429,72 +387,6 @@ struct drm_bridge_funcs { void (*atomic_destroy_state)(struct drm_bridge *bridge, struct drm_bridge_state *state); - /** - * @atomic_get_output_bus_fmts: - * - * Return the supported bus formats on the output end of a bridge. - * The returned array must be allocated with kmalloc() and will be - * freed by the caller. If the allocation fails, NULL should be - * returned. num_output_fmts must be set to the returned array size. - * Formats listed in the returned array should be listed in decreasing - * preference order (the core will try all formats until it finds one - * that works). - * - * This method is only called on the last element of the bridge chain - * as part of the bus format negotiation process that happens in - * &drm_atomic_bridge_chain_select_bus_fmts(). - * This method is optional. When not implemented, the core will - * fall back to &drm_connector.display_info.bus_formats[0] if - * &drm_connector.display_info.num_bus_formats > 0, - * or to MEDIA_BUS_FMT_FIXED otherwise. - */ - u32 *(*atomic_get_output_bus_fmts)(struct drm_bridge *bridge, - struct drm_bridge_state *bridge_state, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state, - unsigned int *num_output_fmts); - - /** - * @atomic_get_input_bus_fmts: - * - * Return the supported bus formats on the input end of a bridge for - * a specific output bus format. - * - * The returned array must be allocated with kmalloc() and will be - * freed by the caller. If the allocation fails, NULL should be - * returned. num_output_fmts must be set to the returned array size. - * Formats listed in the returned array should be listed in decreasing - * preference order (the core will try all formats until it finds one - * that works). When the format is not supported NULL should be - * returned and *num_output_fmts should be set to 0. - * - * This method is called on all elements of the bridge chain as part of - * the bus format negotiation process that happens in - * &drm_atomic_bridge_chain_select_bus_fmts(). - * This method is optional. When not implemented, the core will bypass - * bus format negotiation on this element of the bridge without - * failing, and the previous element in the chain will be passed - * MEDIA_BUS_FMT_FIXED as its output bus format. - * - * Bridge drivers that need to support being linked to bridges that are - * not supporting bus format negotiation should handle the - * output_fmt == MEDIA_BUS_FMT_FIXED case appropriately, by selecting a - * sensible default value or extracting this information from somewhere - * else (FW property, &drm_display_mode, &drm_display_info, ...) - * - * Note: Even if input format selection on the first bridge has no - * impact on the negotiation process (bus format negotiation stops once - * we reach the first element of the chain), drivers are expected to - * return accurate input formats as the input format may be used to - * configure the CRTC output appropriately. - */ - u32 *(*atomic_get_input_bus_fmts)(struct drm_bridge *bridge, - struct drm_bridge_state *bridge_state, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state, - u32 output_fmt, - unsigned int *num_input_fmts); - /** * @atomic_check: * @@ -509,14 +401,6 @@ struct drm_bridge_funcs { * called when &drm_bridge_funcs.atomic_check() is implemented, so only * one of them should be provided. * - * If drivers need to tweak &drm_bridge_state.input_bus_cfg.flags or - * &drm_bridge_state.output_bus_cfg.flags it should should happen in - * this function. By default the &drm_bridge_state.output_bus_cfg.flags - * field is set to the next bridge - * &drm_bridge_state.input_bus_cfg.flags value or - * &drm_connector.display_info.bus_flags if the bridge is the last - * element in the chain. - * * RETURNS: * zero if the check passed, a negative error code otherwise. */ @@ -704,14 +588,6 @@ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge, void drm_atomic_bridge_chain_enable(struct drm_bridge *bridge, struct drm_atomic_state *state); -u32 * -drm_atomic_helper_bridge_propagate_bus_fmt(struct drm_bridge *bridge, - struct drm_bridge_state *bridge_state, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state, - u32 output_fmt, - unsigned int *num_input_fmts); - void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge, struct drm_bridge_state *state); void __drm_atomic_helper_bridge_duplicate_state(struct drm_bridge *bridge, From patchwork Tue Jan 7 18:58:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 11321597 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5AA5B930 for ; Tue, 7 Jan 2020 18:58:25 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3FDE320848 for ; Tue, 7 Jan 2020 18:58:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3FDE320848 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=collabora.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9DCD86E12A; Tue, 7 Jan 2020 18:58:20 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 183C56E125 for ; Tue, 7 Jan 2020 18:58:19 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id C7054290546; Tue, 7 Jan 2020 18:58:17 +0000 (GMT) From: Boris Brezillon To: Andrzej Hajda , Neil Armstrong , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Daniel Vetter Subject: [PATCH v2 3/5] Revert "drm/bridge: Add an ->atomic_check() hook" Date: Tue, 7 Jan 2020 19:58:05 +0100 Message-Id: <20200107185807.606999-4-boris.brezillon@collabora.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20200107185807.606999-1-boris.brezillon@collabora.com> References: <20200107185807.606999-1-boris.brezillon@collabora.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Boris Brezillon , dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" This reverts commit b86d895524ab ("drm/bridge: Add an ->atomic_check() hook"). Commit 6ed7e9625fa6 ("drm/bridge: Add a drm_bridge_state object") introduced a circular dependency between drm.ko and drm_kms_helper.ko which uncovered a misdesign in how the whole thing was implemented. Let's revert all patches depending on the bridge_state infrastructure for now. Signed-off-by: Boris Brezillon --- drivers/gpu/drm/drm_atomic_helper.c | 12 +++--- drivers/gpu/drm/drm_bridge.c | 62 ----------------------------- include/drm/drm_bridge.h | 29 +------------- 3 files changed, 7 insertions(+), 96 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index afe14f72a824..ad8eae98d9e8 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -437,12 +437,12 @@ mode_fixup(struct drm_atomic_state *state) funcs = encoder->helper_private; bridge = drm_bridge_chain_get_first_bridge(encoder); - ret = drm_atomic_bridge_chain_check(bridge, - new_crtc_state, - new_conn_state); - if (ret) { - DRM_DEBUG_ATOMIC("Bridge atomic check failed\n"); - return ret; + ret = drm_bridge_chain_mode_fixup(bridge, + &new_crtc_state->mode, + &new_crtc_state->adjusted_mode); + if (!ret) { + DRM_DEBUG_ATOMIC("Bridge fixup failed\n"); + return -EINVAL; } if (funcs && funcs->atomic_check) { diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 8e4b799150b0..872e159fcb42 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -645,68 +645,6 @@ void drm_atomic_bridge_chain_enable(struct drm_bridge *bridge, } EXPORT_SYMBOL(drm_atomic_bridge_chain_enable); -static int drm_atomic_bridge_check(struct drm_bridge *bridge, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state) -{ - if (bridge->funcs->atomic_check) { - struct drm_bridge_state *bridge_state; - int ret; - - bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state, - bridge); - if (WARN_ON(!bridge_state)) - return -EINVAL; - - ret = bridge->funcs->atomic_check(bridge, bridge_state, - crtc_state, conn_state); - if (ret) - return ret; - } else if (bridge->funcs->mode_fixup) { - if (!bridge->funcs->mode_fixup(bridge, &crtc_state->mode, - &crtc_state->adjusted_mode)) - return -EINVAL; - } - - return 0; -} - -/** - * drm_atomic_bridge_chain_check() - Do an atomic check on the bridge chain - * @bridge: bridge control structure - * @crtc_state: new CRTC state - * @conn_state: new connector state - * - * Calls &drm_bridge_funcs.atomic_check() (falls back on - * &drm_bridge_funcs.mode_fixup()) op for all the bridges in the encoder chain, - * starting from the last bridge to the first. These are called before calling - * &drm_encoder_helper_funcs.atomic_check() - * - * RETURNS: - * 0 on success, a negative error code on failure - */ -int drm_atomic_bridge_chain_check(struct drm_bridge *bridge, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state) -{ - struct drm_encoder *encoder = bridge->encoder; - struct drm_bridge *iter; - - list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) { - int ret; - - ret = drm_atomic_bridge_check(iter, crtc_state, conn_state); - if (ret) - return ret; - - if (iter == bridge) - break; - } - - return 0; -} -EXPORT_SYMBOL(drm_atomic_bridge_chain_check); - /** * __drm_atomic_helper_bridge_reset() - Initialize a bridge state to its * default diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index ae0595c70132..52d3ed150618 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -128,9 +128,7 @@ struct drm_bridge_funcs { * this function passes all other callbacks must succeed for this * configuration. * - * The mode_fixup callback is optional. &drm_bridge_funcs.mode_fixup() - * is not called when &drm_bridge_funcs.atomic_check() is implemented, - * so only one of them should be provided. + * The @mode_fixup callback is optional. * * NOTE: * @@ -387,28 +385,6 @@ struct drm_bridge_funcs { void (*atomic_destroy_state)(struct drm_bridge *bridge, struct drm_bridge_state *state); - /** - * @atomic_check: - * - * This method is responsible for checking bridge state correctness. - * It can also check the state of the surrounding components in chain - * to make sure the whole pipeline can work properly. - * - * &drm_bridge_funcs.atomic_check() hooks are called in reverse - * order (from the last to the first bridge). - * - * This method is optional. &drm_bridge_funcs.mode_fixup() is not - * called when &drm_bridge_funcs.atomic_check() is implemented, so only - * one of them should be provided. - * - * RETURNS: - * zero if the check passed, a negative error code otherwise. - */ - int (*atomic_check)(struct drm_bridge *bridge, - struct drm_bridge_state *bridge_state, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state); - /** * @atomic_reset: * @@ -576,9 +552,6 @@ void drm_bridge_chain_mode_set(struct drm_bridge *bridge, void drm_bridge_chain_pre_enable(struct drm_bridge *bridge); void drm_bridge_chain_enable(struct drm_bridge *bridge); -int drm_atomic_bridge_chain_check(struct drm_bridge *bridge, - struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state); void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge, struct drm_atomic_state *state); void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge, From patchwork Tue Jan 7 18:58:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 11321601 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1C721930 for ; Tue, 7 Jan 2020 18:58:29 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0442C214D8 for ; Tue, 7 Jan 2020 18:58:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0442C214D8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=collabora.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 70CAA6E128; Tue, 7 Jan 2020 18:58:21 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9AB4D6E128 for ; Tue, 7 Jan 2020 18:58:19 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 16CB629054A; Tue, 7 Jan 2020 18:58:18 +0000 (GMT) From: Boris Brezillon To: Andrzej Hajda , Neil Armstrong , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Daniel Vetter Subject: [PATCH v2 4/5] Revert "drm/bridge: Patch atomic hooks to take a drm_bridge_state" Date: Tue, 7 Jan 2020 19:58:06 +0100 Message-Id: <20200107185807.606999-5-boris.brezillon@collabora.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20200107185807.606999-1-boris.brezillon@collabora.com> References: <20200107185807.606999-1-boris.brezillon@collabora.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Boris Brezillon , dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" This reverts commit f7619a58ef92 ("drm/bridge: Patch atomic hooks to take a drm_bridge_state"). Commit 6ed7e9625fa6 ("drm/bridge: Add a drm_bridge_state object") introduced a circular dependency between drm.ko and drm_kms_helper.ko which uncovered a misdesign in how the whole thing was implemented. Let's revert all patches depending on the bridge_state infrastructure for now. Signed-off-by: Boris Brezillon --- .../drm/bridge/analogix/analogix_dp_core.c | 41 ++++++------- drivers/gpu/drm/drm_bridge.c | 61 ++++--------------- drivers/gpu/drm/rcar-du/rcar_lvds.c | 8 +-- include/drm/drm_bridge.h | 8 +-- 4 files changed, 36 insertions(+), 82 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 6fab71985cd4..6effe532f820 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1289,21 +1289,19 @@ struct drm_crtc *analogix_dp_get_new_crtc(struct analogix_dp_device *dp, return conn_state->crtc; } -static void -analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge, - struct drm_bridge_state *old_bridge_state) +static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge, + struct drm_atomic_state *state) { - struct drm_atomic_state *old_state = old_bridge_state->base.state; struct analogix_dp_device *dp = bridge->driver_private; struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state; int ret; - crtc = analogix_dp_get_new_crtc(dp, old_state); + crtc = analogix_dp_get_new_crtc(dp, state); if (!crtc) return; - old_crtc_state = drm_atomic_get_old_crtc_state(old_state, crtc); + old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc); /* Don't touch the panel if we're coming back from PSR */ if (old_crtc_state && old_crtc_state->self_refresh_active) return; @@ -1368,22 +1366,20 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp) return ret; } -static void -analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge, - struct drm_bridge_state *old_bridge_state) +static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge, + struct drm_atomic_state *state) { - struct drm_atomic_state *old_state = old_bridge_state->base.state; struct analogix_dp_device *dp = bridge->driver_private; struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state; int timeout_loop = 0; int ret; - crtc = analogix_dp_get_new_crtc(dp, old_state); + crtc = analogix_dp_get_new_crtc(dp, state); if (!crtc) return; - old_crtc_state = drm_atomic_get_old_crtc_state(old_state, crtc); + old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc); /* Not a full enable, just disable PSR and continue */ if (old_crtc_state && old_crtc_state->self_refresh_active) { ret = analogix_dp_disable_psr(dp); @@ -1444,20 +1440,18 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) dp->dpms_mode = DRM_MODE_DPMS_OFF; } -static void -analogix_dp_bridge_atomic_disable(struct drm_bridge *bridge, - struct drm_bridge_state *old_bridge_state) +static void analogix_dp_bridge_atomic_disable(struct drm_bridge *bridge, + struct drm_atomic_state *state) { - struct drm_atomic_state *old_state = old_bridge_state->base.state; struct analogix_dp_device *dp = bridge->driver_private; struct drm_crtc *crtc; struct drm_crtc_state *new_crtc_state = NULL; - crtc = analogix_dp_get_new_crtc(dp, old_state); + crtc = analogix_dp_get_new_crtc(dp, state); if (!crtc) goto out; - new_crtc_state = drm_atomic_get_new_crtc_state(old_state, crtc); + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc); if (!new_crtc_state) goto out; @@ -1469,21 +1463,20 @@ analogix_dp_bridge_atomic_disable(struct drm_bridge *bridge, analogix_dp_bridge_disable(bridge); } -static void -analogix_dp_bridge_atomic_post_disable(struct drm_bridge *bridge, - struct drm_bridge_state *old_bridge_state) +static +void analogix_dp_bridge_atomic_post_disable(struct drm_bridge *bridge, + struct drm_atomic_state *state) { - struct drm_atomic_state *old_state = old_bridge_state->base.state; struct analogix_dp_device *dp = bridge->driver_private; struct drm_crtc *crtc; struct drm_crtc_state *new_crtc_state; int ret; - crtc = analogix_dp_get_new_crtc(dp, old_state); + crtc = analogix_dp_get_new_crtc(dp, state); if (!crtc) return; - new_crtc_state = drm_atomic_get_new_crtc_state(old_state, crtc); + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc); if (!new_crtc_state || !new_crtc_state->self_refresh_active) return; diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 872e159fcb42..a213c9042f2c 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -501,19 +501,10 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge, encoder = bridge->encoder; list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) { - if (iter->funcs->atomic_disable) { - struct drm_bridge_state *old_bridge_state; - - old_bridge_state = - drm_atomic_get_old_bridge_state(old_state, - iter); - if (WARN_ON(!old_bridge_state)) - return; - - iter->funcs->atomic_disable(iter, old_bridge_state); - } else if (iter->funcs->disable) { + if (iter->funcs->atomic_disable) + iter->funcs->atomic_disable(iter, old_state); + else if (iter->funcs->disable) iter->funcs->disable(iter); - } if (iter == bridge) break; @@ -544,20 +535,10 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge, encoder = bridge->encoder; list_for_each_entry_from(bridge, &encoder->bridge_chain, chain_node) { - if (bridge->funcs->atomic_post_disable) { - struct drm_bridge_state *old_bridge_state; - - old_bridge_state = - drm_atomic_get_old_bridge_state(old_state, - bridge); - if (WARN_ON(!old_bridge_state)) - return; - - bridge->funcs->atomic_post_disable(bridge, - old_bridge_state); - } else if (bridge->funcs->post_disable) { + if (bridge->funcs->atomic_post_disable) + bridge->funcs->atomic_post_disable(bridge, old_state); + else if (bridge->funcs->post_disable) bridge->funcs->post_disable(bridge); - } } } EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable); @@ -586,19 +567,10 @@ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge, encoder = bridge->encoder; list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) { - if (iter->funcs->atomic_pre_enable) { - struct drm_bridge_state *old_bridge_state; - - old_bridge_state = - drm_atomic_get_old_bridge_state(old_state, - iter); - if (WARN_ON(!old_bridge_state)) - return; - - iter->funcs->atomic_pre_enable(iter, old_bridge_state); - } else if (iter->funcs->pre_enable) { + if (iter->funcs->atomic_pre_enable) + iter->funcs->atomic_pre_enable(iter, old_state); + else if (iter->funcs->pre_enable) iter->funcs->pre_enable(iter); - } if (iter == bridge) break; @@ -628,19 +600,10 @@ void drm_atomic_bridge_chain_enable(struct drm_bridge *bridge, encoder = bridge->encoder; list_for_each_entry_from(bridge, &encoder->bridge_chain, chain_node) { - if (bridge->funcs->atomic_enable) { - struct drm_bridge_state *old_bridge_state; - - old_bridge_state = - drm_atomic_get_old_bridge_state(old_state, - bridge); - if (WARN_ON(!old_bridge_state)) - return; - - bridge->funcs->atomic_enable(bridge, old_bridge_state); - } else if (bridge->funcs->enable) { + if (bridge->funcs->atomic_enable) + bridge->funcs->atomic_enable(bridge, old_state); + else if (bridge->funcs->enable) bridge->funcs->enable(bridge); - } } } EXPORT_SYMBOL(drm_atomic_bridge_chain_enable); diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 961519ce6634..8ffa4fbbdeb3 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -590,9 +590,8 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge, } static void rcar_lvds_atomic_enable(struct drm_bridge *bridge, - struct drm_bridge_state *old_bridge_state) + struct drm_atomic_state *state) { - struct drm_atomic_state *state = old_bridge_state->base.state; struct drm_connector *connector; struct drm_crtc *crtc; @@ -604,7 +603,7 @@ static void rcar_lvds_atomic_enable(struct drm_bridge *bridge, } static void rcar_lvds_atomic_disable(struct drm_bridge *bridge, - struct drm_bridge_state *old_bridge_state) + struct drm_atomic_state *state) { struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); @@ -619,8 +618,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge, /* Disable the companion LVDS encoder in dual-link mode. */ if (lvds->link_type != RCAR_LVDS_SINGLE_LINK && lvds->companion) - lvds->companion->funcs->atomic_disable(lvds->companion, - old_bridge_state); + lvds->companion->funcs->atomic_disable(lvds->companion, state); clk_disable_unprepare(lvds->clocks.mod); } diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 52d3ed150618..fc7c71f4de55 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -282,7 +282,7 @@ struct drm_bridge_funcs { * The @atomic_pre_enable callback is optional. */ void (*atomic_pre_enable)(struct drm_bridge *bridge, - struct drm_bridge_state *old_bridge_state); + struct drm_atomic_state *old_state); /** * @atomic_enable: @@ -307,7 +307,7 @@ struct drm_bridge_funcs { * The @atomic_enable callback is optional. */ void (*atomic_enable)(struct drm_bridge *bridge, - struct drm_bridge_state *old_bridge_state); + struct drm_atomic_state *old_state); /** * @atomic_disable: * @@ -330,7 +330,7 @@ struct drm_bridge_funcs { * The @atomic_disable callback is optional. */ void (*atomic_disable)(struct drm_bridge *bridge, - struct drm_bridge_state *old_bridge_state); + struct drm_atomic_state *old_state); /** * @atomic_post_disable: @@ -356,7 +356,7 @@ struct drm_bridge_funcs { * The @atomic_post_disable callback is optional. */ void (*atomic_post_disable)(struct drm_bridge *bridge, - struct drm_bridge_state *old_bridge_state); + struct drm_atomic_state *old_state); /** * @atomic_duplicate_state: From patchwork Tue Jan 7 18:58:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 11321603 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id AE45E930 for ; Tue, 7 Jan 2020 18:58:30 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 96B3020848 for ; Tue, 7 Jan 2020 18:58:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 96B3020848 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=collabora.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8A49D6E835; Tue, 7 Jan 2020 18:58:23 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id BD8036E12A for ; Tue, 7 Jan 2020 18:58:19 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 62AED29054B; Tue, 7 Jan 2020 18:58:18 +0000 (GMT) From: Boris Brezillon To: Andrzej Hajda , Neil Armstrong , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Daniel Vetter Subject: [PATCH v2 5/5] Revert "drm/bridge: Add a drm_bridge_state object" Date: Tue, 7 Jan 2020 19:58:07 +0100 Message-Id: <20200107185807.606999-6-boris.brezillon@collabora.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20200107185807.606999-1-boris.brezillon@collabora.com> References: <20200107185807.606999-1-boris.brezillon@collabora.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Boris Brezillon , dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" This reverts commit 6ed7e9625fa6 ("drm/bridge: Add a drm_bridge_state object") which introduced a circular dependency between drm.ko and drm_kms_helper.ko. Looks like the helper/core split is not appropriate and fixing that is not simple. Signed-off-by: Boris Brezillon --- drivers/gpu/drm/drm_atomic.c | 39 -------- drivers/gpu/drm/drm_atomic_helper.c | 20 ---- drivers/gpu/drm/drm_bridge.c | 139 ++-------------------------- include/drm/drm_atomic.h | 3 - include/drm/drm_bridge.h | 114 ----------------------- 5 files changed, 6 insertions(+), 309 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index bf1b9c37d515..d33691512a8e 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -30,7 +30,6 @@ #include #include -#include #include #include #include @@ -1018,44 +1017,6 @@ static void drm_atomic_connector_print_state(struct drm_printer *p, connector->funcs->atomic_print_state(p, state); } -/** - * drm_atomic_add_encoder_bridges - add bridges attached to an encoder - * @state: atomic state - * @encoder: DRM encoder - * - * This function adds all bridges attached to @encoder. This is needed to add - * bridge states to @state and make them available when - * &bridge_funcs.atomic_{check,pre_enable,enable,disable_post_disable}() are - * called - * - * Returns: - * 0 on success or can fail with -EDEADLK or -ENOMEM. When the error is EDEADLK - * then the w/w mutex code has detected a deadlock and the entire atomic - * sequence must be restarted. All other errors are fatal. - */ -int -drm_atomic_add_encoder_bridges(struct drm_atomic_state *state, - struct drm_encoder *encoder) -{ - struct drm_bridge_state *bridge_state; - struct drm_bridge *bridge; - - if (!encoder) - return 0; - - DRM_DEBUG_ATOMIC("Adding all bridges for [encoder:%d:%s] to %p\n", - encoder->base.id, encoder->name, state); - - drm_for_each_bridge_in_chain(encoder, bridge) { - bridge_state = drm_atomic_get_bridge_state(state, bridge); - if (IS_ERR(bridge_state)) - return PTR_ERR(bridge_state); - } - - return 0; -} -EXPORT_SYMBOL(drm_atomic_add_encoder_bridges); - /** * drm_atomic_add_affected_connectors - add connectors for CRTC * @state: atomic state diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ad8eae98d9e8..4511c2e07bb9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -730,26 +730,6 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, return ret; } - /* - * Iterate over all connectors again, and add all affected bridges to - * the state. - */ - for_each_oldnew_connector_in_state(state, connector, - old_connector_state, - new_connector_state, i) { - struct drm_encoder *encoder; - - encoder = old_connector_state->best_encoder; - ret = drm_atomic_add_encoder_bridges(state, encoder); - if (ret) - return ret; - - encoder = new_connector_state->best_encoder; - ret = drm_atomic_add_encoder_bridges(state, encoder); - if (ret) - return ret; - } - ret = mode_valid(state); if (ret) return ret; diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index a213c9042f2c..c2cf0c90fa26 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -25,7 +25,6 @@ #include #include -#include #include #include @@ -90,74 +89,6 @@ void drm_bridge_remove(struct drm_bridge *bridge) } EXPORT_SYMBOL(drm_bridge_remove); -static struct drm_bridge_state * -drm_atomic_default_bridge_duplicate_state(struct drm_bridge *bridge) -{ - struct drm_bridge_state *new; - - if (WARN_ON(!bridge->base.state)) - return NULL; - - new = kzalloc(sizeof(*new), GFP_KERNEL); - if (new) - __drm_atomic_helper_bridge_duplicate_state(bridge, new); - - return new; -} - -static struct drm_private_state * -drm_bridge_atomic_duplicate_priv_state(struct drm_private_obj *obj) -{ - struct drm_bridge *bridge = drm_priv_to_bridge(obj); - struct drm_bridge_state *state; - - if (bridge->funcs->atomic_duplicate_state) - state = bridge->funcs->atomic_duplicate_state(bridge); - else - state = drm_atomic_default_bridge_duplicate_state(bridge); - - return state ? &state->base : NULL; -} - -static void -drm_atomic_default_bridge_destroy_state(struct drm_bridge *bridge, - struct drm_bridge_state *state) -{ - /* Just a simple kfree() for now */ - kfree(state); -} - -static void -drm_bridge_atomic_destroy_priv_state(struct drm_private_obj *obj, - struct drm_private_state *s) -{ - struct drm_bridge_state *state = drm_priv_to_bridge_state(s); - struct drm_bridge *bridge = drm_priv_to_bridge(obj); - - if (bridge->funcs->atomic_destroy_state) - bridge->funcs->atomic_destroy_state(bridge, state); - else - drm_atomic_default_bridge_destroy_state(bridge, state); -} - -static const struct drm_private_state_funcs drm_bridge_priv_state_funcs = { - .atomic_duplicate_state = drm_bridge_atomic_duplicate_priv_state, - .atomic_destroy_state = drm_bridge_atomic_destroy_priv_state, -}; - -static struct drm_bridge_state * -drm_atomic_default_bridge_reset(struct drm_bridge *bridge) -{ - struct drm_bridge_state *bridge_state; - - bridge_state = kzalloc(sizeof(*bridge_state), GFP_KERNEL); - if (!bridge_state) - return ERR_PTR(-ENOMEM); - - __drm_atomic_helper_bridge_reset(bridge, bridge_state); - return bridge_state; -} - /** * drm_bridge_attach - attach the bridge to an encoder's chain * @@ -183,7 +114,6 @@ drm_atomic_default_bridge_reset(struct drm_bridge *bridge) int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge, struct drm_bridge *previous) { - struct drm_bridge_state *state; int ret; if (!encoder || !bridge) @@ -205,35 +135,15 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge, if (bridge->funcs->attach) { ret = bridge->funcs->attach(bridge); - if (ret < 0) - goto err_reset_bridge; - } - - if (bridge->funcs->atomic_reset) - state = bridge->funcs->atomic_reset(bridge); - else - state = drm_atomic_default_bridge_reset(bridge); - - if (IS_ERR(state)) { - ret = PTR_ERR(state); - goto err_detach_bridge; + if (ret < 0) { + list_del(&bridge->chain_node); + bridge->dev = NULL; + bridge->encoder = NULL; + return ret; + } } - drm_atomic_private_obj_init(bridge->dev, &bridge->base, - &state->base, - &drm_bridge_priv_state_funcs); - return 0; - -err_detach_bridge: - if (bridge->funcs->detach) - bridge->funcs->detach(bridge); - -err_reset_bridge: - bridge->dev = NULL; - bridge->encoder = NULL; - list_del(&bridge->chain_node); - return ret; } EXPORT_SYMBOL(drm_bridge_attach); @@ -245,8 +155,6 @@ void drm_bridge_detach(struct drm_bridge *bridge) if (WARN_ON(!bridge->dev)) return; - drm_atomic_private_obj_fini(&bridge->base); - if (bridge->funcs->detach) bridge->funcs->detach(bridge); @@ -608,41 +516,6 @@ void drm_atomic_bridge_chain_enable(struct drm_bridge *bridge, } EXPORT_SYMBOL(drm_atomic_bridge_chain_enable); -/** - * __drm_atomic_helper_bridge_reset() - Initialize a bridge state to its - * default - * @bridge: the bridge this state is refers to - * @state: bridge state to initialize - * - * Initialize the bridge state to default values. This is meant to be* called - * by the bridge &drm_plane_funcs.reset hook for bridges that subclass the - * bridge state. - */ -void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge, - struct drm_bridge_state *state) -{ - memset(state, 0, sizeof(*state)); - state->bridge = bridge; -} -EXPORT_SYMBOL(__drm_atomic_helper_bridge_reset); - -/** - * __drm_atomic_helper_bridge_duplicate_state() - Copy atomic bridge state - * @bridge: bridge object - * @state: atomic bridge state - * - * Copies atomic state from a bridge's current state and resets inferred values. - * This is useful for drivers that subclass the bridge state. - */ -void __drm_atomic_helper_bridge_duplicate_state(struct drm_bridge *bridge, - struct drm_bridge_state *state) -{ - __drm_atomic_helper_private_obj_duplicate_state(&bridge->base, - &state->base); - state->bridge = bridge; -} -EXPORT_SYMBOL(__drm_atomic_helper_bridge_duplicate_state); - #ifdef CONFIG_OF /** * of_drm_find_bridge - find the bridge corresponding to the device node in diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index ccce65e14917..951dfb15c27b 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -669,9 +669,6 @@ __drm_atomic_get_current_plane_state(struct drm_atomic_state *state, return plane->state; } -int __must_check -drm_atomic_add_encoder_bridges(struct drm_atomic_state *state, - struct drm_encoder *encoder); int __must_check drm_atomic_add_affected_connectors(struct drm_atomic_state *state, struct drm_crtc *crtc); diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index fc7c71f4de55..694e153a7531 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -25,8 +25,6 @@ #include #include - -#include #include #include #include @@ -35,23 +33,6 @@ struct drm_bridge; struct drm_bridge_timings; struct drm_panel; -/** - * struct drm_bridge_state - Atomic bridge state object - * @base: inherit from &drm_private_state - * @bridge: the bridge this state refers to - */ -struct drm_bridge_state { - struct drm_private_state base; - - struct drm_bridge *bridge; -}; - -static inline struct drm_bridge_state * -drm_priv_to_bridge_state(struct drm_private_state *priv) -{ - return container_of(priv, struct drm_bridge_state, base); -} - /** * struct drm_bridge_funcs - drm_bridge control functions */ @@ -357,49 +338,6 @@ struct drm_bridge_funcs { */ void (*atomic_post_disable)(struct drm_bridge *bridge, struct drm_atomic_state *old_state); - - /** - * @atomic_duplicate_state: - * - * Duplicate the current bridge state object (which is guaranteed to be - * non-NULL). - * - * The atomic_duplicate_state() is optional. When not implemented the - * core allocates a drm_bridge_state object and calls - * &__drm_atomic_helper_bridge_duplicate_state() to initialize it. - * - * RETURNS: - * A valid drm_bridge_state object or NULL if the allocation fails. - */ - struct drm_bridge_state *(*atomic_duplicate_state)(struct drm_bridge *bridge); - - /** - * @atomic_destroy_state: - * - * Destroy a bridge state object previously allocated by - * &drm_bridge_funcs.atomic_duplicate_state(). - * - * The atomic_destroy_state hook is optional. When not implemented the - * core calls kfree() on the state. - */ - void (*atomic_destroy_state)(struct drm_bridge *bridge, - struct drm_bridge_state *state); - - /** - * @atomic_reset: - * - * Reset the bridge to a predefined state (or retrieve its current - * state) and return a &drm_bridge_state object matching this state. - * This function is called at attach time. - * - * The atomic_reset hook is optional. When not implemented the core - * allocates a new state and calls &__drm_atomic_helper_bridge_reset(). - * - * RETURNS: - * A valid drm_bridge_state object in case of success, an ERR_PTR() - * giving the reason of the failure otherwise. - */ - struct drm_bridge_state *(*atomic_reset)(struct drm_bridge *bridge); }; /** @@ -442,8 +380,6 @@ struct drm_bridge_timings { * struct drm_bridge - central DRM bridge control structure */ struct drm_bridge { - /** @base: inherit from &drm_private_object */ - struct drm_private_obj base; /** @dev: DRM device this bridge belongs to */ struct drm_device *dev; /** @encoder: encoder to which this bridge is connected */ @@ -468,12 +404,6 @@ struct drm_bridge { void *driver_private; }; -static inline struct drm_bridge * -drm_priv_to_bridge(struct drm_private_obj *priv) -{ - return container_of(priv, struct drm_bridge, base); -} - void drm_bridge_add(struct drm_bridge *bridge); void drm_bridge_remove(struct drm_bridge *bridge); struct drm_bridge *of_drm_find_bridge(struct device_node *np); @@ -561,50 +491,6 @@ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge, void drm_atomic_bridge_chain_enable(struct drm_bridge *bridge, struct drm_atomic_state *state); -void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge, - struct drm_bridge_state *state); -void __drm_atomic_helper_bridge_duplicate_state(struct drm_bridge *bridge, - struct drm_bridge_state *new); - -static inline struct drm_bridge_state * -drm_atomic_get_bridge_state(struct drm_atomic_state *state, - struct drm_bridge *bridge) -{ - struct drm_private_state *obj_state; - - obj_state = drm_atomic_get_private_obj_state(state, &bridge->base); - if (IS_ERR(obj_state)) - return ERR_CAST(obj_state); - - return drm_priv_to_bridge_state(obj_state); -} - -static inline struct drm_bridge_state * -drm_atomic_get_old_bridge_state(struct drm_atomic_state *state, - struct drm_bridge *bridge) -{ - struct drm_private_state *obj_state; - - obj_state = drm_atomic_get_old_private_obj_state(state, &bridge->base); - if (!obj_state) - return NULL; - - return drm_priv_to_bridge_state(obj_state); -} - -static inline struct drm_bridge_state * -drm_atomic_get_new_bridge_state(struct drm_atomic_state *state, - struct drm_bridge *bridge) -{ - struct drm_private_state *obj_state; - - obj_state = drm_atomic_get_new_private_obj_state(state, &bridge->base); - if (!obj_state) - return NULL; - - return drm_priv_to_bridge_state(obj_state); -} - #ifdef CONFIG_DRM_PANEL_BRIDGE struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel); struct drm_bridge *drm_panel_bridge_add_typed(struct drm_panel *panel,