[PATCH 0/5] ti-sn65dsi83: Finalize transition to atomic operations

[PATCH 0/5] ti-sn65dsi83: Finalize transition to atomic operations

[Laurent Pinchart]

Hello,

This patch series is based on top of "[PATCH] drm/bridge: ti-sn65dsi83:
Replace connector format patching with atomic_get_input_bus_fmts". It
completes the transition to atomic operations in the ti-sn65dsi83
driver. The main reason for this change is patch 4/5 that fixes a few
issues in the driver (see the patch's commit message for details), but
overall it also brings the driver to the most recent API which is nice
in itself.

Laurent Pinchart (5):
  drm: bridge: ti-sn65dsi83: Move LVDS format selection to .mode_set()
  drm: bridge: ti-sn65dsi83: Pass mode explicitly to helper functions
  drm: bridge: ti-sn65dsi83: Switch to atomic operations
  drm: bridge: ti-sn65dsi83: Retrieve output format from bridge state
  drm: bridge: ti-sn65dsi83: Retrieve the display mode from the state

 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 166 +++++++++++++-------------
 1 file changed, 82 insertions(+), 84 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 1/5] drm: bridge: ti-sn65dsi83: Move LVDS format selection to .mode_set()

[Laurent Pinchart]

The LVDS format is selected based on the bus format reported by the
connector. This is currently done in .mode_fixup(), but that's not the
right place, as the format should be selected when setting the mode.
Move it to .mode_set().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index b4e8e61da76e..db2e7aa90667 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -509,19 +509,12 @@ static void sn65dsi83_mode_set(struct drm_bridge *bridge,
 			       const struct drm_display_mode *adj)
 {
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
-
-	ctx->mode = *adj;
-}
-
-static bool sn65dsi83_mode_fixup(struct drm_bridge *bridge,
-				 const struct drm_display_mode *mode,
-				 struct drm_display_mode *adj)
-{
-	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
 	struct drm_encoder *encoder = bridge->encoder;
 	struct drm_device *ddev = encoder->dev;
 	struct drm_connector *connector;
 
+	ctx->mode = *adj;
+
 	/* The DSI format is always RGB888_1X24 */
 	list_for_each_entry(connector, &ddev->mode_config.connector_list, head) {
 		switch (connector->display_info.bus_formats[0]) {
@@ -551,8 +544,6 @@ static bool sn65dsi83_mode_fixup(struct drm_bridge *bridge,
 			break;
 		}
 	}
-
-	return true;
 }
 
 #define MAX_INPUT_SEL_FORMATS	1
@@ -589,7 +580,6 @@ static const struct drm_bridge_funcs sn65dsi83_funcs = {
 	.post_disable	= sn65dsi83_post_disable,
 	.mode_valid	= sn65dsi83_mode_valid,
 	.mode_set	= sn65dsi83_mode_set,
-	.mode_fixup	= sn65dsi83_mode_fixup,
 
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
-- 
Regards,

Laurent Pinchart

[PATCH 2/5] drm: bridge: ti-sn65dsi83: Pass mode explicitly to helper functions

[Laurent Pinchart]

Pass the display mode explicitly to the sn65dsi83_get_lvds_range() and
sn65dsi83_get_dsi_range() functions to prepare for its removal from the
sn65dsi83 structure. This is not meant to bring any functional change.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index db2e7aa90667..2fb2bd4e3625 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -306,7 +306,8 @@ static void sn65dsi83_pre_enable(struct drm_bridge *bridge)
 	usleep_range(1000, 1100);
 }
 
-static u8 sn65dsi83_get_lvds_range(struct sn65dsi83 *ctx)
+static u8 sn65dsi83_get_lvds_range(struct sn65dsi83 *ctx,
+				   const struct drm_display_mode *mode)
 {
 	/*
 	 * The encoding of the LVDS_CLK_RANGE is as follows:
@@ -322,7 +323,7 @@ static u8 sn65dsi83_get_lvds_range(struct sn65dsi83 *ctx)
 	 * the clock to 25..154 MHz, the range calculation can be simplified
 	 * as follows:
 	 */
-	int mode_clock = ctx->mode.clock;
+	int mode_clock = mode->clock;
 
 	if (ctx->lvds_dual_link)
 		mode_clock /= 2;
@@ -330,7 +331,8 @@ static u8 sn65dsi83_get_lvds_range(struct sn65dsi83 *ctx)
 	return (mode_clock - 12500) / 25000;
 }
 
-static u8 sn65dsi83_get_dsi_range(struct sn65dsi83 *ctx)
+static u8 sn65dsi83_get_dsi_range(struct sn65dsi83 *ctx,
+				  const struct drm_display_mode *mode)
 {
 	/*
 	 * The encoding of the CHA_DSI_CLK_RANGE is as follows:
@@ -346,7 +348,7 @@ static u8 sn65dsi83_get_dsi_range(struct sn65dsi83 *ctx)
 	 *  DSI_CLK = mode clock * bpp / dsi_data_lanes / 2
 	 * the 2 is there because the bus is DDR.
 	 */
-	return DIV_ROUND_UP(clamp((unsigned int)ctx->mode.clock *
+	return DIV_ROUND_UP(clamp((unsigned int)mode->clock *
 			    mipi_dsi_pixel_format_to_bpp(ctx->dsi->format) /
 			    ctx->dsi_lanes / 2, 40000U, 500000U), 5000U);
 }
@@ -378,10 +380,10 @@ static void sn65dsi83_enable(struct drm_bridge *bridge)
 
 	/* Reference clock derived from DSI link clock. */
 	regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
-		     REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx)) |
+		     REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx, &ctx->mode)) |
 		     REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
 	regmap_write(ctx->regmap, REG_DSI_CLK,
-		     REG_DSI_CLK_CHA_DSI_CLK_RANGE(sn65dsi83_get_dsi_range(ctx)));
+		     REG_DSI_CLK_CHA_DSI_CLK_RANGE(sn65dsi83_get_dsi_range(ctx, &ctx->mode)));
 	regmap_write(ctx->regmap, REG_RC_DSI_CLK,
 		     REG_RC_DSI_CLK_DSI_CLK_DIVIDER(sn65dsi83_get_dsi_div(ctx)));
 
-- 
Regards,

Laurent Pinchart

[PATCH 3/5] drm: bridge: ti-sn65dsi83: Switch to atomic operations

[Laurent Pinchart]

Use the atomic version of the enable/disable operations to continue the
transition to the atomic API, started with the introduction of
.atomic_get_input_bus_fmts(). This will be needed to access the mode
from the atomic state.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 2fb2bd4e3625..8c1189a8bb4d 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -291,7 +291,8 @@ static int sn65dsi83_attach(struct drm_bridge *bridge,
 	return ret;
 }
 
-static void sn65dsi83_pre_enable(struct drm_bridge *bridge)
+static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
+					struct drm_bridge_state *old_bridge_state)
 {
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
 
@@ -366,7 +367,8 @@ static u8 sn65dsi83_get_dsi_div(struct sn65dsi83 *ctx)
 	return dsi_div - 1;
 }
 
-static void sn65dsi83_enable(struct drm_bridge *bridge)
+static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
+				    struct drm_bridge_state *old_bridge_state)
 {
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
 	unsigned int pval;
@@ -475,7 +477,8 @@ static void sn65dsi83_enable(struct drm_bridge *bridge)
 	regmap_write(ctx->regmap, REG_IRQ_STAT, pval);
 }
 
-static void sn65dsi83_disable(struct drm_bridge *bridge)
+static void sn65dsi83_atomic_disable(struct drm_bridge *bridge,
+				     struct drm_bridge_state *old_bridge_state)
 {
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
 
@@ -484,7 +487,8 @@ static void sn65dsi83_disable(struct drm_bridge *bridge)
 	regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
 }
 
-static void sn65dsi83_post_disable(struct drm_bridge *bridge)
+static void sn65dsi83_atomic_post_disable(struct drm_bridge *bridge,
+					  struct drm_bridge_state *old_bridge_state)
 {
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
 
@@ -575,13 +579,13 @@ sn65dsi83_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
 }
 
 static const struct drm_bridge_funcs sn65dsi83_funcs = {
-	.attach		= sn65dsi83_attach,
-	.pre_enable	= sn65dsi83_pre_enable,
-	.enable		= sn65dsi83_enable,
-	.disable	= sn65dsi83_disable,
-	.post_disable	= sn65dsi83_post_disable,
-	.mode_valid	= sn65dsi83_mode_valid,
-	.mode_set	= sn65dsi83_mode_set,
+	.attach			= sn65dsi83_attach,
+	.atomic_pre_enable	= sn65dsi83_atomic_pre_enable,
+	.atomic_enable		= sn65dsi83_atomic_enable,
+	.atomic_disable		= sn65dsi83_atomic_disable,
+	.atomic_post_disable	= sn65dsi83_atomic_post_disable,
+	.mode_valid		= sn65dsi83_mode_valid,
+	.mode_set		= sn65dsi83_mode_set,
 
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
-- 
Regards,

Laurent Pinchart

[PATCH 4/5] drm: bridge: ti-sn65dsi83: Retrieve output format from bridge state

[Laurent Pinchart]

The driver currently iterates over all connectors to get the bus format,
used to configure the LVDS output format. This causes several issues:

- If other connectors than the LVDS output are present, the format used
  by the driver may end up belonging to an entirely different output.

- The code can crash if some connectors are not connected, as bus_format
  may then be NULL.

- There's no guarantee that the bus format on the connector at the
  output of the pipeline matches the output of the sn65dsi83, as there
  may be other bridges in the pipeline.

Solve this by retrieving the format from the bridge state instead, which
provides the format corresponding to the output of the bridge.

The struct sn65dsi83 lvds_format_24bpp and lvds_format_jeida fields are
moved to local variables in sn65dsi83_atomic_enable() as they're now
used in that function only.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 73 +++++++++++++--------------
 1 file changed, 36 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 8c1189a8bb4d..8cfa96977832 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -147,8 +147,6 @@ struct sn65dsi83 {
 	int				dsi_lanes;
 	bool				lvds_dual_link;
 	bool				lvds_dual_link_even_odd_swap;
-	bool				lvds_format_24bpp;
-	bool				lvds_format_jeida;
 };
 
 static const struct regmap_range sn65dsi83_readable_ranges[] = {
@@ -371,11 +369,45 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 				    struct drm_bridge_state *old_bridge_state)
 {
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+	struct drm_atomic_state *state = old_bridge_state->base.state;
+	const struct drm_bridge_state *bridge_state;
+	bool lvds_format_24bpp;
+	bool lvds_format_jeida;
 	unsigned int pval;
 	__le16 le16val;
 	u16 val;
 	int ret;
 
+	/* Get the LVDS format from the bridge state. */
+	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
+
+	switch (bridge_state->output_bus_cfg.format) {
+	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+		lvds_format_24bpp = false;
+		lvds_format_jeida = true;
+		break;
+	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+		lvds_format_24bpp = true;
+		lvds_format_jeida = true;
+		break;
+	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+		lvds_format_24bpp = true;
+		lvds_format_jeida = false;
+		break;
+	default:
+		/*
+		 * Some bridges still don't set the correct
+		 * LVDS bus pixel format, use SPWG24 default
+		 * format until those are fixed.
+		 */
+		lvds_format_24bpp = true;
+		lvds_format_jeida = false;
+		dev_warn(ctx->dev,
+			 "Unsupported LVDS bus format 0x%04x, please check output bridge driver. Falling back to SPWG24.\n",
+			 bridge_state->output_bus_cfg.format);
+		break;
+	}
+
 	/* Clear reset, disable PLL */
 	regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
 	regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
@@ -405,14 +437,14 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 	       REG_LVDS_FMT_VS_NEG_POLARITY : 0);
 
 	/* Set up bits-per-pixel, 18bpp or 24bpp. */
-	if (ctx->lvds_format_24bpp) {
+	if (lvds_format_24bpp) {
 		val |= REG_LVDS_FMT_CHA_24BPP_MODE;
 		if (ctx->lvds_dual_link)
 			val |= REG_LVDS_FMT_CHB_24BPP_MODE;
 	}
 
 	/* Set up LVDS format, JEIDA/Format 1 or SPWG/Format 2 */
-	if (ctx->lvds_format_jeida) {
+	if (lvds_format_jeida) {
 		val |= REG_LVDS_FMT_CHA_24BPP_FORMAT1;
 		if (ctx->lvds_dual_link)
 			val |= REG_LVDS_FMT_CHB_24BPP_FORMAT1;
@@ -515,41 +547,8 @@ static void sn65dsi83_mode_set(struct drm_bridge *bridge,
 			       const struct drm_display_mode *adj)
 {
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
-	struct drm_encoder *encoder = bridge->encoder;
-	struct drm_device *ddev = encoder->dev;
-	struct drm_connector *connector;
 
 	ctx->mode = *adj;
-
-	/* The DSI format is always RGB888_1X24 */
-	list_for_each_entry(connector, &ddev->mode_config.connector_list, head) {
-		switch (connector->display_info.bus_formats[0]) {
-		case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
-			ctx->lvds_format_24bpp = false;
-			ctx->lvds_format_jeida = true;
-			break;
-		case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
-			ctx->lvds_format_24bpp = true;
-			ctx->lvds_format_jeida = true;
-			break;
-		case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
-			ctx->lvds_format_24bpp = true;
-			ctx->lvds_format_jeida = false;
-			break;
-		default:
-			/*
-			 * Some bridges still don't set the correct
-			 * LVDS bus pixel format, use SPWG24 default
-			 * format until those are fixed.
-			 */
-			ctx->lvds_format_24bpp = true;
-			ctx->lvds_format_jeida = false;
-			dev_warn(ctx->dev,
-				 "Unsupported LVDS bus format 0x%04x, please check output bridge driver. Falling back to SPWG24.\n",
-				 connector->display_info.bus_formats[0]);
-			break;
-		}
-	}
 }
 
 #define MAX_INPUT_SEL_FORMATS	1
-- 
Regards,

Laurent Pinchart

[PATCH 5/5] drm: bridge: ti-sn65dsi83: Retrieve the display mode from the state

[Laurent Pinchart]

Instead of storing a copy of the display mode in the sn65dsi83
structure, retrieve it from the atomic state in
sn65dsi83_atomic_enable(). This allows the removal of the .mode_set()
operation, and completes the transition to the atomic API.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 49 ++++++++++++++-------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 8cfa96977832..9072342566f3 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -137,7 +137,6 @@ enum sn65dsi83_model {
 
 struct sn65dsi83 {
 	struct drm_bridge		bridge;
-	struct drm_display_mode		mode;
 	struct device			*dev;
 	struct regmap			*regmap;
 	struct device_node		*host_node;
@@ -371,6 +370,10 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
 	struct drm_atomic_state *state = old_bridge_state->base.state;
 	const struct drm_bridge_state *bridge_state;
+	const struct drm_crtc_state *crtc_state;
+	const struct drm_display_mode *mode;
+	struct drm_connector *connector;
+	struct drm_crtc *crtc;
 	bool lvds_format_24bpp;
 	bool lvds_format_jeida;
 	unsigned int pval;
@@ -408,16 +411,26 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 		break;
 	}
 
+	/*
+	 * Retrieve the CRTC adjusted mode. This requires a little dance to go
+	 * from the bridge to the encoder, to the connector and to the CRTC.
+	 */
+	connector = drm_atomic_get_new_connector_for_encoder(state,
+							     bridge->encoder);
+	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	mode = &crtc_state->adjusted_mode;
+
 	/* Clear reset, disable PLL */
 	regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
 	regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
 
 	/* Reference clock derived from DSI link clock. */
 	regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
-		     REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx, &ctx->mode)) |
+		     REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx, mode)) |
 		     REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
 	regmap_write(ctx->regmap, REG_DSI_CLK,
-		     REG_DSI_CLK_CHA_DSI_CLK_RANGE(sn65dsi83_get_dsi_range(ctx, &ctx->mode)));
+		     REG_DSI_CLK_CHA_DSI_CLK_RANGE(sn65dsi83_get_dsi_range(ctx, mode)));
 	regmap_write(ctx->regmap, REG_RC_DSI_CLK,
 		     REG_RC_DSI_CLK_DSI_CLK_DIVIDER(sn65dsi83_get_dsi_div(ctx)));
 
@@ -431,9 +444,9 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 	regmap_write(ctx->regmap, REG_DSI_EQ, 0x00);
 
 	/* Set up sync signal polarity. */
-	val = (ctx->mode.flags & DRM_MODE_FLAG_NHSYNC ?
+	val = (mode->flags & DRM_MODE_FLAG_NHSYNC ?
 	       REG_LVDS_FMT_HS_NEG_POLARITY : 0) |
-	      (ctx->mode.flags & DRM_MODE_FLAG_NVSYNC ?
+	      (mode->flags & DRM_MODE_FLAG_NVSYNC ?
 	       REG_LVDS_FMT_VS_NEG_POLARITY : 0);
 
 	/* Set up bits-per-pixel, 18bpp or 24bpp. */
@@ -463,29 +476,29 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
 		     REG_LVDS_LANE_CHB_LVDS_TERM);
 	regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
 
-	le16val = cpu_to_le16(ctx->mode.hdisplay);
+	le16val = cpu_to_le16(mode->hdisplay);
 	regmap_bulk_write(ctx->regmap, REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
 			  &le16val, 2);
-	le16val = cpu_to_le16(ctx->mode.vdisplay);
+	le16val = cpu_to_le16(mode->vdisplay);
 	regmap_bulk_write(ctx->regmap, REG_VID_CHA_VERTICAL_DISPLAY_SIZE_LOW,
 			  &le16val, 2);
 	/* 32 + 1 pixel clock to ensure proper operation */
 	le16val = cpu_to_le16(32 + 1);
 	regmap_bulk_write(ctx->regmap, REG_VID_CHA_SYNC_DELAY_LOW, &le16val, 2);
-	le16val = cpu_to_le16(ctx->mode.hsync_end - ctx->mode.hsync_start);
+	le16val = cpu_to_le16(mode->hsync_end - mode->hsync_start);
 	regmap_bulk_write(ctx->regmap, REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW,
 			  &le16val, 2);
-	le16val = cpu_to_le16(ctx->mode.vsync_end - ctx->mode.vsync_start);
+	le16val = cpu_to_le16(mode->vsync_end - mode->vsync_start);
 	regmap_bulk_write(ctx->regmap, REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW,
 			  &le16val, 2);
 	regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_BACK_PORCH,
-		     ctx->mode.htotal - ctx->mode.hsync_end);
+		     mode->htotal - mode->hsync_end);
 	regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_BACK_PORCH,
-		     ctx->mode.vtotal - ctx->mode.vsync_end);
+		     mode->vtotal - mode->vsync_end);
 	regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_FRONT_PORCH,
-		     ctx->mode.hsync_start - ctx->mode.hdisplay);
+		     mode->hsync_start - mode->hdisplay);
 	regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_FRONT_PORCH,
-		     ctx->mode.vsync_start - ctx->mode.vdisplay);
+		     mode->vsync_start - mode->vdisplay);
 	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN, 0x00);
 
 	/* Enable PLL */
@@ -542,15 +555,6 @@ sn65dsi83_mode_valid(struct drm_bridge *bridge,
 	return MODE_OK;
 }
 
-static void sn65dsi83_mode_set(struct drm_bridge *bridge,
-			       const struct drm_display_mode *mode,
-			       const struct drm_display_mode *adj)
-{
-	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
-
-	ctx->mode = *adj;
-}
-
 #define MAX_INPUT_SEL_FORMATS	1
 
 static u32 *
@@ -584,7 +588,6 @@ static const struct drm_bridge_funcs sn65dsi83_funcs = {
 	.atomic_disable		= sn65dsi83_atomic_disable,
 	.atomic_post_disable	= sn65dsi83_atomic_post_disable,
 	.mode_valid		= sn65dsi83_mode_valid,
-	.mode_set		= sn65dsi83_mode_set,
 
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/5] ti-sn65dsi83: Finalize transition to atomic operations

[Sam Ravnborg]

Hi Laurent,

On Mon, Jun 21, 2021 at 03:55:13PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> This patch series is based on top of "[PATCH] drm/bridge: ti-sn65dsi83:
> Replace connector format patching with atomic_get_input_bus_fmts". It
> completes the transition to atomic operations in the ti-sn65dsi83
> driver. The main reason for this change is patch 4/5 that fixes a few
> issues in the driver (see the patch's commit message for details), but
> overall it also brings the driver to the most recent API which is nice
> in itself.
> 
> Laurent Pinchart (5):
>   drm: bridge: ti-sn65dsi83: Move LVDS format selection to .mode_set()
>   drm: bridge: ti-sn65dsi83: Pass mode explicitly to helper functions
>   drm: bridge: ti-sn65dsi83: Switch to atomic operations
>   drm: bridge: ti-sn65dsi83: Retrieve output format from bridge state
>   drm: bridge: ti-sn65dsi83: Retrieve the display mode from the state
> 
>  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 166 +++++++++++++-------------
>  1 file changed, 82 insertions(+), 84 deletions(-)

I have browsed the series and it all looked good.
Acked-by: Sam Ravnborg <sam@ravnborg.org>

on them all.

It is news to me that the atomic ops are the way to go - but then I have
been off-line for a while so no suprise or maybe I just missed it
before.

It would be good if the comments in drm_bridge.h could point out what is
deprecated, so we know what to avoid in new and updated bridge drivers.
But this is all un-related to this series.

	Sam

Re: [PATCH 0/5] ti-sn65dsi83: Finalize transition to atomic operations

[Laurent Pinchart]

Hi Sam,

On Mon, Jun 21, 2021 at 08:49:53PM +0200, Sam Ravnborg wrote:
> On Mon, Jun 21, 2021 at 03:55:13PM +0300, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series is based on top of "[PATCH] drm/bridge: ti-sn65dsi83:
> > Replace connector format patching with atomic_get_input_bus_fmts". It
> > completes the transition to atomic operations in the ti-sn65dsi83
> > driver. The main reason for this change is patch 4/5 that fixes a few
> > issues in the driver (see the patch's commit message for details), but
> > overall it also brings the driver to the most recent API which is nice
> > in itself.
> > 
> > Laurent Pinchart (5):
> >   drm: bridge: ti-sn65dsi83: Move LVDS format selection to .mode_set()
> >   drm: bridge: ti-sn65dsi83: Pass mode explicitly to helper functions
> >   drm: bridge: ti-sn65dsi83: Switch to atomic operations
> >   drm: bridge: ti-sn65dsi83: Retrieve output format from bridge state
> >   drm: bridge: ti-sn65dsi83: Retrieve the display mode from the state
> > 
> >  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 166 +++++++++++++-------------
> >  1 file changed, 82 insertions(+), 84 deletions(-)
> 
> I have browsed the series and it all looked good.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> on them all.
> 
> It is news to me that the atomic ops are the way to go - but then I have
> been off-line for a while so no suprise or maybe I just missed it
> before.

They're not mandatory as such, but they give us access to the atomic
state, which is sometimes required. Overall I think it would be nice to
move to the atomic operations and drop the legacy ones, to avoid
maintaining two sets of operations. It will take time :-)

> It would be good if the comments in drm_bridge.h could point out what is
> deprecated, so we know what to avoid in new and updated bridge drivers.
> But this is all un-related to this series.

It's a good point. Would you like to submit a patch, or should I do so ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/5] ti-sn65dsi83: Finalize transition to atomic operations

[Sam Ravnborg]

Hi Laurent,

> > 
> > It is news to me that the atomic ops are the way to go - but then I have
> > been off-line for a while so no suprise or maybe I just missed it
> > before.
> 
> They're not mandatory as such, but they give us access to the atomic
> state, which is sometimes required. Overall I think it would be nice to
> move to the atomic operations and drop the legacy ones, to avoid
> maintaining two sets of operations. It will take time :-)
Yeah, but if we can get more people working on the job..
> 
> > It would be good if the comments in drm_bridge.h could point out what is
> > deprecated, so we know what to avoid in new and updated bridge drivers.
> > But this is all un-related to this series.
> 
> It's a good point. Would you like to submit a patch, or should I do so ?
Please do as I would have to dig around to do it right as I have
fogotten most of the drm internals the last couple of months.

Just something simple like: "This is deprecated, do not use!" would do
the trick for me. Then I would know what to look for if I was reviewing
a new bridge driver or patching an existing one or just trying to gentle
push someone in the right direction.

For drm_drv.h this really helped me to understand what should not be
used.

	Sam

Re: [PATCH 0/5] ti-sn65dsi83: Finalize transition to atomic operations

[Robert Foss]

Applied to drm-misc-next


On Mon, 21 Jun 2021 at 20:49, Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Laurent,
>
> On Mon, Jun 21, 2021 at 03:55:13PM +0300, Laurent Pinchart wrote:
> > Hello,
> >
> > This patch series is based on top of "[PATCH] drm/bridge: ti-sn65dsi83:
> > Replace connector format patching with atomic_get_input_bus_fmts". It
> > completes the transition to atomic operations in the ti-sn65dsi83
> > driver. The main reason for this change is patch 4/5 that fixes a few
> > issues in the driver (see the patch's commit message for details), but
> > overall it also brings the driver to the most recent API which is nice
> > in itself.
> >
> > Laurent Pinchart (5):
> >   drm: bridge: ti-sn65dsi83: Move LVDS format selection to .mode_set()
> >   drm: bridge: ti-sn65dsi83: Pass mode explicitly to helper functions
> >   drm: bridge: ti-sn65dsi83: Switch to atomic operations
> >   drm: bridge: ti-sn65dsi83: Retrieve output format from bridge state
> >   drm: bridge: ti-sn65dsi83: Retrieve the display mode from the state
> >
> >  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 166 +++++++++++++-------------
> >  1 file changed, 82 insertions(+), 84 deletions(-)
>
> I have browsed the series and it all looked good.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
> on them all.
>
> It is news to me that the atomic ops are the way to go - but then I have
> been off-line for a while so no suprise or maybe I just missed it
> before.
>
> It would be good if the comments in drm_bridge.h could point out what is
> deprecated, so we know what to avoid in new and updated bridge drivers.
> But this is all un-related to this series.
>
>         Sam