devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@collabora.com>
To: dri-devel@lists.freedesktop.org
Cc: Lucas Stach <l.stach@pengutronix.de>,
	Chris Healy <cphealy@gmail.com>,
	Andrey Smirnov <andrew.smirnov@gmail.com>,
	Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
	kernel@collabora.com, Daniel Vetter <daniel@ffwll.ch>,
	Inki Dae <inki.dae@samsung.com>,
	Joonyoung Shim <jy0922.shim@samsung.com>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Rob Clark <robdclark@gmail.com>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, Eric Anholt <eric@anholt.net>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: [PATCH v4 03/11] drm: Stop accessing encoder->bridge directly
Date: Tue,  3 Dec 2019 15:15:07 +0100	[thread overview]
Message-ID: <20191203141515.3597631-4-boris.brezillon@collabora.com> (raw)
In-Reply-To: <20191203141515.3597631-1-boris.brezillon@collabora.com>

We are about to replace the single-linked bridge list by a double-linked
one based on list.h, leading to the suppression of the encoder->bridge
field. But before we can do that we must provide a
drm_bridge_chain_get_first_bridge() bridge helper and patch all drivers
and core helpers to use it instead of directly accessing encoder->bridge.

Note that we still have 2 drivers (VC4 and Exynos) manipulating the
encoder->bridge field directly because they need to cut the bridge chain
in order to control the enable/disable sequence. This is definitely
not something we want to encourage, so let's keep those 2 oddities
around until we find a better solution.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
---
Changes in v4:
* Coding style issues
* Add Rbs
* Adjust the commit message to explain why VC4 and exynos still
  access encoder->bridge directly

Changes in v3:
* None

Changes in v2:
* None
---
 drivers/gpu/drm/drm_atomic_helper.c    | 26 +++++++++++++++++---------
 drivers/gpu/drm/drm_encoder.c          |  3 ++-
 drivers/gpu/drm/drm_probe_helper.c     |  4 +++-
 drivers/gpu/drm/msm/edp/edp_bridge.c   | 10 ++++++++--
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 10 +++++++---
 include/drm/drm_bridge.h               | 15 +++++++++++++++
 6 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index cf678be58fa4..29c3115bf9e2 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -419,6 +419,7 @@ mode_fixup(struct drm_atomic_state *state)
 	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
+		struct drm_bridge *bridge;
 
 		WARN_ON(!!new_conn_state->best_encoder != !!new_conn_state->crtc);
 
@@ -435,7 +436,8 @@ mode_fixup(struct drm_atomic_state *state)
 		encoder = new_conn_state->best_encoder;
 		funcs = encoder->helper_private;
 
-		ret = drm_bridge_chain_mode_fixup(encoder->bridge,
+		bridge = drm_bridge_chain_get_first_bridge(encoder);
+		ret = drm_bridge_chain_mode_fixup(bridge,
 					&new_crtc_state->mode,
 					&new_crtc_state->adjusted_mode);
 		if (!ret) {
@@ -493,6 +495,7 @@ static enum drm_mode_status mode_valid_path(struct drm_connector *connector,
 					    struct drm_crtc *crtc,
 					    const struct drm_display_mode *mode)
 {
+	struct drm_bridge *bridge;
 	enum drm_mode_status ret;
 
 	ret = drm_encoder_mode_valid(encoder, mode);
@@ -502,7 +505,8 @@ static enum drm_mode_status mode_valid_path(struct drm_connector *connector,
 		return ret;
 	}
 
-	ret = drm_bridge_chain_mode_valid(encoder->bridge, mode);
+	bridge = drm_bridge_chain_get_first_bridge(encoder);
+	ret = drm_bridge_chain_mode_valid(bridge, mode);
 	if (ret != MODE_OK) {
 		DRM_DEBUG_ATOMIC("[BRIDGE] mode_valid() failed\n");
 		return ret;
@@ -985,6 +989,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 	for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
+		struct drm_bridge *bridge;
 
 		/* Shut down everything that's in the changeset and currently
 		 * still on. So need to check the old, saved state. */
@@ -1021,7 +1026,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 		 * Each encoder has at most one connector (since we always steal
 		 * it away), so we won't call disable hooks twice.
 		 */
-		drm_atomic_bridge_chain_disable(encoder->bridge, old_state);
+		bridge = drm_bridge_chain_get_first_bridge(encoder);
+		drm_atomic_bridge_chain_disable(bridge, old_state);
 
 		/* Right function depends upon target state. */
 		if (funcs) {
@@ -1035,8 +1041,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 				funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
 		}
 
-		drm_atomic_bridge_chain_post_disable(encoder->bridge,
-						     old_state);
+		drm_atomic_bridge_chain_post_disable(bridge, old_state);
 	}
 
 	for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) {
@@ -1190,6 +1195,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
 		struct drm_display_mode *mode, *adjusted_mode;
+		struct drm_bridge *bridge;
 
 		if (!new_conn_state->best_encoder)
 			continue;
@@ -1217,8 +1223,8 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
 			funcs->mode_set(encoder, mode, adjusted_mode);
 		}
 
-		drm_bridge_chain_mode_set(encoder->bridge, mode,
-					  adjusted_mode);
+		bridge = drm_bridge_chain_get_first_bridge(encoder);
+		drm_bridge_chain_mode_set(bridge, mode, adjusted_mode);
 	}
 }
 
@@ -1317,6 +1323,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 	for_each_new_connector_in_state(old_state, connector, new_conn_state, i) {
 		const struct drm_encoder_helper_funcs *funcs;
 		struct drm_encoder *encoder;
+		struct drm_bridge *bridge;
 
 		if (!new_conn_state->best_encoder)
 			continue;
@@ -1335,7 +1342,8 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 		 * Each encoder has at most one connector (since we always steal
 		 * it away), so we won't call enable hooks twice.
 		 */
-		drm_atomic_bridge_chain_pre_enable(encoder->bridge, old_state);
+		bridge = drm_bridge_chain_get_first_bridge(encoder);
+		drm_atomic_bridge_chain_pre_enable(bridge, old_state);
 
 		if (funcs) {
 			if (funcs->atomic_enable)
@@ -1346,7 +1354,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 				funcs->commit(encoder);
 		}
 
-		drm_atomic_bridge_chain_enable(encoder->bridge, old_state);
+		drm_atomic_bridge_chain_enable(bridge, old_state);
 	}
 
 	drm_atomic_helper_commit_writebacks(dev, old_state);
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index fcd3af1ac432..a2cc7e7241a9 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -167,9 +167,10 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
 	 */
 
 	if (encoder->bridge) {
-		struct drm_bridge *bridge = encoder->bridge;
+		struct drm_bridge *bridge;
 		struct drm_bridge *next;
 
+		bridge = drm_bridge_chain_get_first_bridge(encoder);
 		while (bridge) {
 			next = drm_bridge_get_next_bridge(bridge);
 			drm_bridge_detach(bridge);
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index c3ea722065c4..576b4b7dcd89 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -101,6 +101,7 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
 
 	/* Step 2: Validate against encoders and crtcs */
 	drm_connector_for_each_possible_encoder(connector, encoder) {
+		struct drm_bridge *bridge;
 		struct drm_crtc *crtc;
 
 		ret = drm_encoder_mode_valid(encoder, mode);
@@ -112,7 +113,8 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
 			continue;
 		}
 
-		ret = drm_bridge_chain_mode_valid(encoder->bridge, mode);
+		bridge = drm_bridge_chain_get_first_bridge(encoder);
+		ret = drm_bridge_chain_mode_valid(bridge, mode);
 		if (ret != MODE_OK) {
 			/* There is also no point in continuing for crtc check
 			 * here. */
diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c b/drivers/gpu/drm/msm/edp/edp_bridge.c
index 2950bba4aca9..b65b5cc2dba2 100644
--- a/drivers/gpu/drm/msm/edp/edp_bridge.c
+++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
@@ -55,8 +55,14 @@ static void edp_bridge_mode_set(struct drm_bridge *bridge,
 	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		if ((connector->encoder != NULL) &&
-			(connector->encoder->bridge == bridge)) {
+		struct drm_encoder *encoder = connector->encoder;
+		struct drm_bridge *first_bridge;
+
+		if (!connector->encoder)
+			continue;
+
+		first_bridge = drm_bridge_chain_get_first_bridge(encoder);
+		if (bridge == first_bridge) {
 			msm_edp_ctrl_timing_cfg(edp->ctrl,
 				adjusted_mode, &connector->display_info);
 			break;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 2da46e3dc4ae..f2ae4c410244 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -14,6 +14,7 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_device.h>
 #include <drm/drm_fb_cma_helper.h>
@@ -680,9 +681,10 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 			rcdu->encoders[RCAR_DU_OUTPUT_LVDS0 + rcrtc->index];
 		const struct drm_display_mode *mode =
 			&crtc->state->adjusted_mode;
+		struct drm_bridge *bridge;
 
-		rcar_lvds_clk_enable(encoder->base.bridge,
-				     mode->clock * 1000);
+		bridge = drm_bridge_chain_get_first_bridge(&encoder->base);
+		rcar_lvds_clk_enable(bridge, mode->clock * 1000);
 	}
 
 	rcar_du_crtc_start(rcrtc);
@@ -702,12 +704,14 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
 	    rstate->outputs == BIT(RCAR_DU_OUTPUT_DPAD0)) {
 		struct rcar_du_encoder *encoder =
 			rcdu->encoders[RCAR_DU_OUTPUT_LVDS0 + rcrtc->index];
+		struct drm_bridge *bridge;
 
 		/*
 		 * Disable the LVDS clock output, see
 		 * rcar_du_crtc_atomic_enable().
 		 */
-		rcar_lvds_clk_disable(encoder->base.bridge);
+		bridge = drm_bridge_chain_get_first_bridge(&encoder->base);
+		rcar_lvds_clk_disable(bridge);
 	}
 
 	spin_lock_irq(&crtc->dev->event_lock);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 46fc17c63a44..bd78c256b1ed 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -25,6 +25,7 @@
 
 #include <linux/list.h>
 #include <linux/ctype.h>
+#include <drm/drm_encoder.h>
 #include <drm/drm_mode_object.h>
 #include <drm/drm_modes.h>
 
@@ -422,6 +423,20 @@ drm_bridge_get_next_bridge(struct drm_bridge *bridge)
 	return bridge->next;
 }
 
+/**
+ * drm_bridge_chain_get_first_bridge() - Get the first bridge in the chain
+ * @encoder: encoder object
+ *
+ * RETURNS:
+ * the first bridge in the chain, or NULL if @encoder has no bridge attached
+ * to it.
+ */
+static inline struct drm_bridge *
+drm_bridge_chain_get_first_bridge(struct drm_encoder *encoder)
+{
+	return encoder->bridge;
+}
+
 bool drm_bridge_chain_mode_fixup(struct drm_bridge *bridge,
 				 const struct drm_display_mode *mode,
 				 struct drm_display_mode *adjusted_mode);
-- 
2.23.0


  parent reply	other threads:[~2019-12-03 14:15 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-03 14:15 [PATCH v4 00/11] drm: Add support for bus-format negotiation Boris Brezillon
2019-12-03 14:15 ` [PATCH v4 01/11] drm/bridge: Rename bridge helpers targeting a bridge chain Boris Brezillon
2019-12-03 14:15 ` [PATCH v4 02/11] drm/bridge: Introduce drm_bridge_get_next_bridge() Boris Brezillon
2019-12-03 14:15 ` Boris Brezillon [this message]
2019-12-03 14:15 ` [PATCH v4 04/11] drm/bridge: Make the bridge chain a double-linked list Boris Brezillon
2019-12-16 13:54   ` Marek Szyprowski
2019-12-16 14:55     ` Boris Brezillon
2019-12-16 15:02       ` Marek Szyprowski
2019-12-16 15:25         ` Boris Brezillon
2019-12-23  9:55           ` Marek Szyprowski
2019-12-24  9:16             ` Andrzej Hajda
2019-12-24  9:44               ` Boris Brezillon
2019-12-24  9:49                 ` Boris Brezillon
2019-12-24 10:03                   ` Boris Brezillon
2019-12-27 10:25                     ` Marek Szyprowski
2019-12-27 11:03                       ` Marek Szyprowski
2019-12-24 11:31                 ` Sam Ravnborg
2019-12-25  1:36                   ` Laurent Pinchart
2019-12-27 12:39                   ` Boris Brezillon
2019-12-27  9:42                 ` Andrzej Hajda
2019-12-27 10:51                   ` Laurent Pinchart
2019-12-27 12:21                     ` Boris Brezillon
2020-01-01 17:13                       ` Laurent Pinchart
2019-12-03 14:15 ` [PATCH v4 05/11] drm/bridge: Add the drm_for_each_bridge_in_chain() helper Boris Brezillon
2019-12-03 14:15 ` [PATCH v4 06/11] drm/bridge: Add the drm_bridge_get_prev_bridge() helper Boris Brezillon
2019-12-03 14:15 ` [PATCH v4 07/11] drm/bridge: Clarify the atomic enable/disable hooks semantics Boris Brezillon
2019-12-03 18:02   ` Laurent Pinchart
2019-12-04  9:00     ` Boris Brezillon
2019-12-03 14:15 ` [PATCH v4 08/11] drm/bridge: Add a drm_bridge_state object Boris Brezillon
2019-12-03 18:17   ` Laurent Pinchart
2019-12-04  9:03     ` Boris Brezillon
2019-12-04  9:12       ` Laurent Pinchart
2019-12-04  9:42         ` Boris Brezillon
2019-12-04 10:38           ` Laurent Pinchart
2019-12-03 14:15 ` [PATCH v4 09/11] drm/bridge: Patch atomic hooks to take a drm_bridge_state Boris Brezillon
2019-12-03 14:15 ` [PATCH v4 10/11] drm/bridge: Add an ->atomic_check() hook Boris Brezillon
2019-12-03 14:15 ` [PATCH v4 11/11] drm/bridge: Add the necessary bits to support bus format negotiation Boris Brezillon
2019-12-03 18:19 ` [PATCH v4 00/11] drm: Add support for bus-format negotiation Laurent Pinchart
2019-12-04  9:09   ` Boris Brezillon
2019-12-04  9:15     ` Laurent Pinchart
2019-12-04 13:43   ` Neil Armstrong
2019-12-09  9:43   ` Boris Brezillon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191203141515.3597631-4-boris.brezillon@collabora.com \
    --to=boris.brezillon@collabora.com \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=a.hajda@samsung.com \
    --cc=andrew.smirnov@gmail.com \
    --cc=cphealy@gmail.com \
    --cc=daniel@ffwll.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eric@anholt.net \
    --cc=inki.dae@samsung.com \
    --cc=jernej.skrabec@siol.net \
    --cc=jonas@kwiboo.se \
    --cc=jy0922.shim@samsung.com \
    --cc=kernel@collabora.com \
    --cc=kyungmin.park@samsung.com \
    --cc=l.stach@pengutronix.de \
    --cc=mark.rutland@arm.com \
    --cc=narmstrong@baylibre.com \
    --cc=nikita.yoush@cogentembedded.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robdclark@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=sam@ravnborg.org \
    --cc=sw0312.kim@samsung.com \
    --cc=thierry.reding@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).