[PATCH v7 0/8] drm/tidss: Use new connector model for tidss

[PATCH v7 0/8] drm/tidss: Use new connector model for tidss

[Aradhya Bhatia]

Hi all,

I have picked up this long standing series from Nikhil Devshatwar[1].

This series moves the tidss to using new connectoe model, where the SoC
driver (tidss) creates the connector and all the bridges are attached
with the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR. It also now creates bridge
to support format negotiation and and 'simple' encoder to expose it to
the userspace.

Since the bridges do not create the connector, the bus_format and
bus_flag is set via atomic hooks.

Support format negotiations in the tfp410, sii902x and mhdp-8546 bridge
drivers as a first step before moving the connector model.

These patches were tested on AM625-SK EVM, AM625 SoC based BeaglePlay,
and J721E-SK. Display support for AM625 SoC has not been added upstream
and is a WIP. To test this series on AM625 based platforms, basic
display support patches, (for driver + devicetree), can be found in
the "next_AttachNoConn-v2" branch on my github fork[2].

Thanks,
Aradhya

[1]: https://patchwork.freedesktop.org/series/82765/#rev5
[2]: https://github.com/aradhya07/linux-ab/tree/next_AttachNoConn-v2

Change Log:
V6 -> V7
  - Rebase and cosmetic changes.
  - Drop the output format check condition for mhdp8546 and hence,
    drop Tomi Valkeinen's R-b tag.
  - Added tags wherever suggested.

V5 -> V6
  - Rebase and cosmetic changes
  - Dropped the output format check condition for tfp410 and hence,
    dropped Tomi Valkeinen's and Laurent Pinchart's R-b tags.
  - Based on Boris Brezillon's comments: dropped patches 5 and 6 from
    the series and instead created a single patch that,
      1. Creates tidss bridge for format negotiation.
      2. Creates 'simple' encoder for userspace exposure.
      3. Creates a tidss connector.
      4. Attaches the next-bridge to encoder with the
         DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
  - Add format negotiation support for sii902x driver.

Previous versions:
V1 to V6: https://patchwork.freedesktop.org/series/82765/

Aradhya Bhatia (3):
  drm/bridge: sii902x: Support format negotiation hooks
  drm/bridge: sii902x: Set input_bus_flags in atomic_check
  drm/tidss: Update encoder/bridge chain connect model

Nikhil Devshatwar (5):
  drm/bridge: tfp410: Support format negotiation hooks
  drm/bridge: tfp410: Set input_bus_flags in atomic_check
  drm/bridge: mhdp8546: Add minimal format negotiation
  drm/bridge: mhdp8546: Set input_bus_flags from atomic_check
  drm/bridge: cdns-mhdp8546: Fix the interrupt enable/disable

 .../drm/bridge/cadence/cdns-mhdp8546-core.c   |  77 ++++++----
 .../drm/bridge/cadence/cdns-mhdp8546-core.h   |   2 +-
 .../drm/bridge/cadence/cdns-mhdp8546-j721e.c  |   9 +-
 .../drm/bridge/cadence/cdns-mhdp8546-j721e.h  |   2 +-
 drivers/gpu/drm/bridge/sii902x.c              |  40 +++++
 drivers/gpu/drm/bridge/ti-tfp410.c            |  43 ++++++
 drivers/gpu/drm/tidss/tidss_encoder.c         | 140 +++++++++++-------
 drivers/gpu/drm/tidss/tidss_encoder.h         |   5 +-
 drivers/gpu/drm/tidss/tidss_kms.c             |  12 +-
 9 files changed, 235 insertions(+), 95 deletions(-)

-- 
2.40.1

[PATCH v7 1/8] drm/bridge: tfp410: Support format negotiation hooks

[Aradhya Bhatia]

From: Nikhil Devshatwar <nikhil.nd@ti.com>

With new connector model, tfp410 will not create the connector and
SoC driver will rely on format negotiation to setup the encoder format.

Support format negotiations hooks in the drm_bridge_funcs.
Use helper functions for state management.

Input format is the one selected by the bridge from DT properties.

Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
[a-bhatia1: Removed output fmt condition check]
Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
---

Notes:
    changes from v1:
    * Use only MEDIA_BUS_FMT_FIXED for output

    changes from V5:
    * Drop the output format check condition because the output
      format for HDMI bridges should be RGB888_1X24 and not FIXED.
      Hence, also drop Tomi Valkeinen's and Laurent Pinchart's R-b
      tags.

 drivers/gpu/drm/bridge/ti-tfp410.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index ab63225cd635..7dacc7e03eee 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -206,12 +206,38 @@ static enum drm_mode_status tfp410_mode_valid(struct drm_bridge *bridge,
 	return MODE_OK;
 }
 
+static u32 *tfp410_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)
+{
+	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
+	u32 *input_fmts;
+
+	*num_input_fmts = 0;
+
+	input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+
+	*num_input_fmts = 1;
+	input_fmts[0] = dvi->bus_format;
+
+	return input_fmts;
+}
+
 static const struct drm_bridge_funcs tfp410_bridge_funcs = {
 	.attach		= tfp410_attach,
 	.detach		= tfp410_detach,
 	.enable		= tfp410_enable,
 	.disable	= tfp410_disable,
 	.mode_valid	= tfp410_mode_valid,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts,
 };
 
 static const struct drm_bridge_timings tfp410_default_timings = {
-- 
2.40.1

[PATCH v7 2/8] drm/bridge: tfp410: Set input_bus_flags in atomic_check

[Aradhya Bhatia]

From: Nikhil Devshatwar <nikhil.nd@ti.com>

input_bus_flags are specified in drm_bridge_timings (legacy) as well
as drm_bridge_state->input_bus_cfg.flags

The flags from the timings will be deprecated. Bridges are supposed
to validate and set the bridge state flags from atomic_check.

Implement atomic_check hook for the same.

Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
---

Notes:
    changes from v4:
    * fix a warning Reported-by: kernel test robot <lkp@intel.com>

    changes from v5:
    * Move the return statement here from patch 4 (where it was added
      by mistake).

    changes from v6:
    * Add new line before return statement.
    * Add Neil Armstrong's R-b tag.

 drivers/gpu/drm/bridge/ti-tfp410.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index 7dacc7e03eee..edf4d30f4f2a 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -228,6 +228,22 @@ static u32 *tfp410_get_input_bus_fmts(struct drm_bridge *bridge,
 	return input_fmts;
 }
 
+static int tfp410_atomic_check(struct drm_bridge *bridge,
+			       struct drm_bridge_state *bridge_state,
+			       struct drm_crtc_state *crtc_state,
+			       struct drm_connector_state *conn_state)
+{
+	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
+
+	/*
+	 * There might be flags negotiation supported in future.
+	 * Set the bus flags in atomic_check statically for now.
+	 */
+	bridge_state->input_bus_cfg.flags = dvi->timings.input_bus_flags;
+
+	return 0;
+}
+
 static const struct drm_bridge_funcs tfp410_bridge_funcs = {
 	.attach		= tfp410_attach,
 	.detach		= tfp410_detach,
@@ -238,6 +254,7 @@ static const struct drm_bridge_funcs tfp410_bridge_funcs = {
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
 	.atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts,
+	.atomic_check = tfp410_atomic_check,
 };
 
 static const struct drm_bridge_timings tfp410_default_timings = {
-- 
2.40.1

[PATCH v7 3/8] drm/bridge: mhdp8546: Add minimal format negotiation

[Aradhya Bhatia]

From: Nikhil Devshatwar <nikhil.nd@ti.com>

With new connector model, mhdp bridge will not create the connector and
SoC driver will rely on format negotiation to setup the encoder format.

Support minimal format negotiations hooks in the drm_bridge_funcs.
Complete format negotiation can be added based on EDID data.
This patch adds the minimal required support to avoid failure
after moving to new connector model.

Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
[a-bhatia1: Drop the output_fmt check condition]
Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
---

Notes:

    changes from v1:
    * cosmetic fixes, commit message update.

    changes from v5:
    * drop the default_bus_format variable and directly assigned
      MEDIA_BUS_FMT_RGB121212_1X36 to input_fmts.

    changes from v6:
    * Drop the output_fmt check condition and hence drop Tomi
      Valkeinen's R-b tag.

 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index f6822dfa3805..afd4e353f37a 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2146,6 +2146,27 @@ cdns_mhdp_bridge_atomic_reset(struct drm_bridge *bridge)
 	return &cdns_mhdp_state->base;
 }
 
+static u32 *cdns_mhdp_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)
+{
+	u32 *input_fmts;
+
+	*num_input_fmts = 0;
+
+	input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+
+	*num_input_fmts = 1;
+	input_fmts[0] = MEDIA_BUS_FMT_RGB121212_1X36;
+
+	return input_fmts;
+}
+
 static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
 				  struct drm_bridge_state *bridge_state,
 				  struct drm_crtc_state *crtc_state,
@@ -2210,6 +2231,7 @@ static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = {
 	.atomic_duplicate_state = cdns_mhdp_bridge_atomic_duplicate_state,
 	.atomic_destroy_state = cdns_mhdp_bridge_atomic_destroy_state,
 	.atomic_reset = cdns_mhdp_bridge_atomic_reset,
+	.atomic_get_input_bus_fmts = cdns_mhdp_get_input_bus_fmts,
 	.detect = cdns_mhdp_bridge_detect,
 	.get_edid = cdns_mhdp_bridge_get_edid,
 	.hpd_enable = cdns_mhdp_bridge_hpd_enable,
-- 
2.40.1

[PATCH v7 4/8] drm/bridge: mhdp8546: Set input_bus_flags from atomic_check

[Aradhya Bhatia]

From: Nikhil Devshatwar <nikhil.nd@ti.com>

input_bus_flags are specified in drm_bridge_timings (legacy) as well
as drm_bridge_state->input_bus_cfg.flags

The flags from the timings will be deprecated. Bridges are supposed
to validate and set the bridge state flags from atomic_check.

Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
[a-bhatia1: replace timings in cdns_mhdp_platform_info by input_bus_flags]
Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
---

Notes:

    changes from v5:
    * remove the wrongly addded return statement in tfp410 driver.
    * replace the timings field in cdns_mhdp_platform_info by
      input_bus_flags field, in order to get rid of bridge->timings
      altogether.

    changes from v6:
    * Add Neil Armstrong's R-b tag.

 drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c  | 11 ++++++++---
 drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h  |  2 +-
 drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.c |  9 ++++-----
 drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.h |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index afd4e353f37a..f12fb62821f7 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2186,6 +2186,13 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
 		return -EINVAL;
 	}
 
+	/*
+	 * There might be flags negotiation supported in future.
+	 * Set the bus flags in atomic_check statically for now.
+	 */
+	if (mhdp->info)
+		bridge_state->input_bus_cfg.flags = *mhdp->info->input_bus_flags;
+
 	mutex_unlock(&mhdp->link_mutex);
 	return 0;
 }
@@ -2551,8 +2558,6 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
 	mhdp->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID |
 			   DRM_BRIDGE_OP_HPD;
 	mhdp->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
-	if (mhdp->info)
-		mhdp->bridge.timings = mhdp->info->timings;
 
 	ret = phy_init(mhdp->phy);
 	if (ret) {
@@ -2639,7 +2644,7 @@ static const struct of_device_id mhdp_ids[] = {
 #ifdef CONFIG_DRM_CDNS_MHDP8546_J721E
 	{ .compatible = "ti,j721e-mhdp8546",
 	  .data = &(const struct cdns_mhdp_platform_info) {
-		  .timings = &mhdp_ti_j721e_bridge_timings,
+		  .input_bus_flags = &mhdp_ti_j721e_bridge_input_bus_flags,
 		  .ops = &mhdp_ti_j721e_ops,
 	  },
 	},
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
index bedddd510d17..bad2fc0c7306 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
@@ -336,7 +336,7 @@ struct cdns_mhdp_bridge_state {
 };
 
 struct cdns_mhdp_platform_info {
-	const struct drm_bridge_timings *timings;
+	const u32 *input_bus_flags;
 	const struct mhdp_platform_ops *ops;
 };
 
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.c
index dfe1b59514f7..12d04be4e242 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.c
@@ -71,8 +71,7 @@ const struct mhdp_platform_ops mhdp_ti_j721e_ops = {
 	.disable = cdns_mhdp_j721e_disable,
 };
 
-const struct drm_bridge_timings mhdp_ti_j721e_bridge_timings = {
-	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
-			   DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE |
-			   DRM_BUS_FLAG_DE_HIGH,
-};
+const u32
+mhdp_ti_j721e_bridge_input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
+				       DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE |
+				       DRM_BUS_FLAG_DE_HIGH;
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.h b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.h
index 97d20d115a24..5ddca07a4255 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.h
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-j721e.h
@@ -14,6 +14,6 @@
 struct mhdp_platform_ops;
 
 extern const struct mhdp_platform_ops mhdp_ti_j721e_ops;
-extern const struct drm_bridge_timings mhdp_ti_j721e_bridge_timings;
+extern const u32 mhdp_ti_j721e_bridge_input_bus_flags;
 
 #endif /* !CDNS_MHDP8546_J721E_H */
-- 
2.40.1

[PATCH v7 5/8] drm/bridge: sii902x: Support format negotiation hooks

[Aradhya Bhatia]

With new connector model, sii902x will not create the connector, when
DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and SoC driver will rely on format
negotiation to setup the encoder format.

Support format negotiations hooks in the drm_bridge_funcs.
Use helper functions for state management.

Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is
the case with older model.

Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
---

Notes:

    changes from v6:
    * Add Neil Armstrong's R-b tag.

 drivers/gpu/drm/bridge/sii902x.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index ef66461e7f7c..70aeb04b7f77 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -473,6 +473,27 @@ static struct edid *sii902x_bridge_get_edid(struct drm_bridge *bridge,
 	return sii902x_get_edid(sii902x, connector);
 }
 
+static u32 *sii902x_bridge_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)
+{
+	u32 *input_fmts;
+
+	*num_input_fmts = 0;
+
+	input_fmts = kcalloc(1, sizeof(*input_fmts), GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+
+	input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+	*num_input_fmts = 1;
+
+	return input_fmts;
+}
+
 static const struct drm_bridge_funcs sii902x_bridge_funcs = {
 	.attach = sii902x_bridge_attach,
 	.mode_set = sii902x_bridge_mode_set,
@@ -480,6 +501,10 @@ static const struct drm_bridge_funcs sii902x_bridge_funcs = {
 	.enable = sii902x_bridge_enable,
 	.detect = sii902x_bridge_detect,
 	.get_edid = sii902x_bridge_get_edid,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_get_input_bus_fmts = sii902x_bridge_atomic_get_input_bus_fmts,
 };
 
 static int sii902x_mute(struct sii902x *sii902x, bool mute)
-- 
2.40.1

[PATCH v7 6/8] drm/bridge: sii902x: Set input_bus_flags in atomic_check

[Aradhya Bhatia]

input_bus_flags are specified in drm_bridge_timings (legacy) as well
as drm_bridge_state->input_bus_cfg.flags

The flags from the timings will be deprecated. Bridges are supposed
to validate and set the bridge state flags from atomic_check.

Implement atomic_check hook for the same.

Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
---

Notes:

    changes from v6:
    * Add Neil Armstrong's R-b tag.

 drivers/gpu/drm/bridge/sii902x.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 70aeb04b7f77..8f1ec526863a 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -494,6 +494,20 @@ static u32 *sii902x_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
 	return input_fmts;
 }
 
+static int sii902x_bridge_atomic_check(struct drm_bridge *bridge,
+				       struct drm_bridge_state *bridge_state,
+				       struct drm_crtc_state *crtc_state,
+				       struct drm_connector_state *conn_state)
+{
+	/*
+	 * There might be flags negotiation supported in future but
+	 * set the bus flags in atomic_check statically for now.
+	 */
+	bridge_state->input_bus_cfg.flags = bridge->timings->input_bus_flags;
+
+	return 0;
+}
+
 static const struct drm_bridge_funcs sii902x_bridge_funcs = {
 	.attach = sii902x_bridge_attach,
 	.mode_set = sii902x_bridge_mode_set,
@@ -505,6 +519,7 @@ static const struct drm_bridge_funcs sii902x_bridge_funcs = {
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
 	.atomic_get_input_bus_fmts = sii902x_bridge_atomic_get_input_bus_fmts,
+	.atomic_check = sii902x_bridge_atomic_check,
 };
 
 static int sii902x_mute(struct sii902x *sii902x, bool mute)
-- 
2.40.1

[PATCH v7 7/8] drm/tidss: Update encoder/bridge chain connect model

[Aradhya Bhatia]

With the new encoder/bridge chain model, the display controller driver
is required to create a drm_connector entity instead of asking the
bridge to do so during drm_bridge_attach. Moreover, the controller
driver should create a drm_bridge entity to negotiate bus formats and a
'simple' drm_encoder entity to expose it to userspace.

Update the encoder/bridge initialization sequence in tidss as per the
new model.

Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
---

Notes:

    changes from v5:
    * Drop patches 5 and 6 from the original series.
    * Instead add this patch that addresses Boris Brezillon's comments
      of creating bridge, simple encoder and connector.

 drivers/gpu/drm/tidss/tidss_encoder.c | 140 ++++++++++++++++----------
 drivers/gpu/drm/tidss/tidss_encoder.h |   5 +-
 drivers/gpu/drm/tidss/tidss_kms.c     |  12 +--
 3 files changed, 94 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c
index 0d4865e9c03d..17a86bed8054 100644
--- a/drivers/gpu/drm/tidss/tidss_encoder.c
+++ b/drivers/gpu/drm/tidss/tidss_encoder.c
@@ -6,91 +6,125 @@
 
 #include <linux/export.h>
 
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_of.h>
+#include <drm/drm_simple_kms_helper.h>
 
 #include "tidss_crtc.h"
 #include "tidss_drv.h"
 #include "tidss_encoder.h"
 
-static int tidss_encoder_atomic_check(struct drm_encoder *encoder,
-				      struct drm_crtc_state *crtc_state,
-				      struct drm_connector_state *conn_state)
+struct tidss_encoder {
+	struct drm_bridge bridge;
+	struct drm_encoder encoder;
+	struct drm_connector *connector;
+	struct drm_bridge *next_bridge;
+	struct tidss_device *tidss;
+};
+
+static inline struct tidss_encoder
+*bridge_to_tidss_encoder(struct drm_bridge *b)
+{
+	return container_of(b, struct tidss_encoder, bridge);
+}
+
+static int tidss_bridge_attach(struct drm_bridge *bridge,
+			       enum drm_bridge_attach_flags flags)
+{
+	struct tidss_encoder *t_enc = bridge_to_tidss_encoder(bridge);
+
+	return drm_bridge_attach(bridge->encoder, t_enc->next_bridge,
+				 bridge, flags);
+}
+
+static int tidss_bridge_atomic_check(struct drm_bridge *bridge,
+				     struct drm_bridge_state *bridge_state,
+				     struct drm_crtc_state *crtc_state,
+				     struct drm_connector_state *conn_state)
 {
-	struct drm_device *ddev = encoder->dev;
+	struct tidss_encoder *t_enc = bridge_to_tidss_encoder(bridge);
+	struct tidss_device *tidss = t_enc->tidss;
 	struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc_state);
 	struct drm_display_info *di = &conn_state->connector->display_info;
-	struct drm_bridge *bridge;
-	bool bus_flags_set = false;
-
-	dev_dbg(ddev->dev, "%s\n", __func__);
-
-	/*
-	 * Take the bus_flags from the first bridge that defines
-	 * bridge timings, or from the connector's display_info if no
-	 * bridge defines the timings.
-	 */
-	drm_for_each_bridge_in_chain(encoder, bridge) {
-		if (!bridge->timings)
-			continue;
-
-		tcrtc_state->bus_flags = bridge->timings->input_bus_flags;
-		bus_flags_set = true;
-		break;
-	}
+	struct drm_bridge_state *next_bridge_state = NULL;
+
+	if (t_enc->next_bridge)
+		next_bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state,
+								    t_enc->next_bridge);
 
-	if (!di->bus_formats || di->num_bus_formats == 0)  {
-		dev_err(ddev->dev, "%s: No bus_formats in connected display\n",
+	if (next_bridge_state) {
+		tcrtc_state->bus_flags = next_bridge_state->input_bus_cfg.flags;
+		tcrtc_state->bus_format = next_bridge_state->input_bus_cfg.format;
+	} else if (di->num_bus_formats) {
+		tcrtc_state->bus_format = di->bus_formats[0];
+		tcrtc_state->bus_flags = di->bus_flags;
+	} else {
+		dev_err(tidss->dev, "%s: No bus_formats in connected display\n",
 			__func__);
 		return -EINVAL;
 	}
 
-	// XXX any cleaner way to set bus format and flags?
-	tcrtc_state->bus_format = di->bus_formats[0];
-	if (!bus_flags_set)
-		tcrtc_state->bus_flags = di->bus_flags;
-
 	return 0;
 }
 
-static void tidss_encoder_destroy(struct drm_encoder *encoder)
-{
-	drm_encoder_cleanup(encoder);
-	kfree(encoder);
-}
-
-static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
-	.atomic_check = tidss_encoder_atomic_check,
-};
-
-static const struct drm_encoder_funcs encoder_funcs = {
-	.destroy = tidss_encoder_destroy,
+static const struct drm_bridge_funcs tidss_bridge_funcs = {
+	.attach				= tidss_bridge_attach,
+	.atomic_check			= tidss_bridge_atomic_check,
+	.atomic_reset			= drm_atomic_helper_bridge_reset,
+	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
 };
 
-struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss,
-					 u32 encoder_type, u32 possible_crtcs)
+int tidss_encoder_create(struct tidss_device *tidss,
+			 struct drm_bridge *next_bridge,
+			 u32 encoder_type, u32 possible_crtcs)
 {
+	struct tidss_encoder *t_enc;
 	struct drm_encoder *enc;
+	struct drm_connector *connector;
 	int ret;
 
-	enc = kzalloc(sizeof(*enc), GFP_KERNEL);
-	if (!enc)
-		return ERR_PTR(-ENOMEM);
+	t_enc = drmm_simple_encoder_alloc(&tidss->ddev, struct tidss_encoder,
+					  encoder, encoder_type);
+	if (IS_ERR(t_enc))
+		return PTR_ERR(t_enc);
+
+	t_enc->tidss = tidss;
+	t_enc->next_bridge = next_bridge;
+	t_enc->bridge.funcs = &tidss_bridge_funcs;
 
+	enc = &t_enc->encoder;
 	enc->possible_crtcs = possible_crtcs;
 
-	ret = drm_encoder_init(&tidss->ddev, enc, &encoder_funcs,
-			       encoder_type, NULL);
-	if (ret < 0) {
-		kfree(enc);
-		return ERR_PTR(ret);
+	/* Attaching first bridge to the encoder */
+	ret = drm_bridge_attach(enc, &t_enc->bridge, NULL,
+				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+	if (ret) {
+		dev_err(tidss->dev, "bridge attach failed: %d\n", ret);
+		return ret;
+	}
+
+	/* Initializing the connector at the end of bridge-chain */
+	connector = drm_bridge_connector_init(&tidss->ddev, enc);
+	if (IS_ERR(connector)) {
+		dev_err(tidss->dev, "bridge_connector create failed\n");
+		return PTR_ERR(connector);
+	}
+
+	ret = drm_connector_attach_encoder(connector, enc);
+	if (ret) {
+		dev_err(tidss->dev, "attaching encoder to connector failed\n");
+		return ret;
 	}
 
-	drm_encoder_helper_add(enc, &encoder_helper_funcs);
+	t_enc->connector = connector;
 
 	dev_dbg(tidss->dev, "Encoder create done\n");
 
-	return enc;
+	return ret;
 }
diff --git a/drivers/gpu/drm/tidss/tidss_encoder.h b/drivers/gpu/drm/tidss/tidss_encoder.h
index ace877c0e0fd..3e561d6b1e83 100644
--- a/drivers/gpu/drm/tidss/tidss_encoder.h
+++ b/drivers/gpu/drm/tidss/tidss_encoder.h
@@ -11,7 +11,8 @@
 
 struct tidss_device;
 
-struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss,
-					 u32 encoder_type, u32 possible_crtcs);
+int tidss_encoder_create(struct tidss_device *tidss,
+			 struct drm_bridge *next_bridge,
+			 u32 encoder_type, u32 possible_crtcs);
 
 #endif
diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
index ad2fa3c3d4a7..c979ad1af236 100644
--- a/drivers/gpu/drm/tidss/tidss_kms.c
+++ b/drivers/gpu/drm/tidss/tidss_kms.c
@@ -193,7 +193,6 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
 	for (i = 0; i < num_pipes; ++i) {
 		struct tidss_plane *tplane;
 		struct tidss_crtc *tcrtc;
-		struct drm_encoder *enc;
 		u32 hw_plane_id = feat->vid_order[tidss->num_planes];
 		int ret;
 
@@ -216,16 +215,13 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
 
 		tidss->crtcs[tidss->num_crtcs++] = &tcrtc->crtc;
 
-		enc = tidss_encoder_create(tidss, pipes[i].enc_type,
+		ret = tidss_encoder_create(tidss, pipes[i].bridge,
+					   pipes[i].enc_type,
 					   1 << tcrtc->crtc.index);
-		if (IS_ERR(enc)) {
+		if (ret) {
 			dev_err(tidss->dev, "encoder create failed\n");
-			return PTR_ERR(enc);
-		}
-
-		ret = drm_bridge_attach(enc, pipes[i].bridge, NULL, 0);
-		if (ret)
 			return ret;
+		}
 	}
 
 	/* create overlay planes of the leftover planes */
-- 
2.40.1

[PATCH v7 8/8] drm/bridge: cdns-mhdp8546: Fix the interrupt enable/disable

[Aradhya Bhatia]

From: Nikhil Devshatwar <nikhil.nd@ti.com>

When removing the tidss driver, there is a warning reported by
kernel about an unhandled interrupt for mhdp driver.

[   43.238895] irq 31: nobody cared (try booting with the "irqpoll" option)
... [snipped backtrace]
[   43.330735] handlers:
[   43.333020] [<000000005367c4f9>] irq_default_primary_handler threaded [<000000007e02b601>]
cdns_mhdp_irq_handler [cdns_mhdp8546]
[   43.344607] Disabling IRQ #31

This happens because as part of cdns_mhdp_bridge_hpd_disable, driver tries
to disable the interrupts. While disabling the SW_EVENT interrupts,
it accidentally enables the MBOX interrupts, which are not handled by
the driver.

Fix this with a read-modify-write to update only required bits.
Use the enable / disable function as required in other places.

Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
Reviewed-by: Swapnil Jakhade <sjakhade@cadence.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
---

Notes:
    changes from v2:
    * Fix the interrupt enable logic at other places in code
    * Reorder functions so that they can be used earlier in the program

    changes from v5:
    * Manual rebase to linux-next.

 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 44 +++++++++----------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index f12fb62821f7..ecb935e46b62 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -54,6 +54,26 @@
 #include "cdns-mhdp8546-hdcp.h"
 #include "cdns-mhdp8546-j721e.h"
 
+static void cdns_mhdp_bridge_hpd_enable(struct drm_bridge *bridge)
+{
+	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
+
+	/* Enable SW event interrupts */
+	if (mhdp->bridge_attached)
+		writel(readl(mhdp->regs + CDNS_APB_INT_MASK) &
+		       ~CDNS_APB_INT_MASK_SW_EVENT_INT,
+		       mhdp->regs + CDNS_APB_INT_MASK);
+}
+
+static void cdns_mhdp_bridge_hpd_disable(struct drm_bridge *bridge)
+{
+	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
+
+	writel(readl(mhdp->regs + CDNS_APB_INT_MASK) |
+	       CDNS_APB_INT_MASK_SW_EVENT_INT,
+	       mhdp->regs + CDNS_APB_INT_MASK);
+}
+
 static int cdns_mhdp_mailbox_read(struct cdns_mhdp_device *mhdp)
 {
 	int ret, empty;
@@ -749,9 +769,7 @@ static int cdns_mhdp_fw_activate(const struct firmware *fw,
 	 * MHDP_HW_STOPPED happens only due to driver removal when
 	 * bridge should already be detached.
 	 */
-	if (mhdp->bridge_attached)
-		writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT,
-		       mhdp->regs + CDNS_APB_INT_MASK);
+	cdns_mhdp_bridge_hpd_enable(&mhdp->bridge);
 
 	spin_unlock(&mhdp->start_lock);
 
@@ -1740,8 +1758,7 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
 
 	/* Enable SW event interrupts */
 	if (hw_ready)
-		writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT,
-		       mhdp->regs + CDNS_APB_INT_MASK);
+		cdns_mhdp_bridge_hpd_enable(bridge);
 
 	return 0;
 aux_unregister:
@@ -2212,23 +2229,6 @@ static struct edid *cdns_mhdp_bridge_get_edid(struct drm_bridge *bridge,
 	return cdns_mhdp_get_edid(mhdp, connector);
 }
 
-static void cdns_mhdp_bridge_hpd_enable(struct drm_bridge *bridge)
-{
-	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
-
-	/* Enable SW event interrupts */
-	if (mhdp->bridge_attached)
-		writel(~(u32)CDNS_APB_INT_MASK_SW_EVENT_INT,
-		       mhdp->regs + CDNS_APB_INT_MASK);
-}
-
-static void cdns_mhdp_bridge_hpd_disable(struct drm_bridge *bridge)
-{
-	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
-
-	writel(CDNS_APB_INT_MASK_SW_EVENT_INT, mhdp->regs + CDNS_APB_INT_MASK);
-}
-
 static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = {
 	.atomic_enable = cdns_mhdp_atomic_enable,
 	.atomic_disable = cdns_mhdp_atomic_disable,
-- 
2.40.1

Re: [PATCH v7 3/8] drm/bridge: mhdp8546: Add minimal format negotiation

[Neil Armstrong]

On 06/06/2023 10:21, Aradhya Bhatia wrote:
> From: Nikhil Devshatwar <nikhil.nd@ti.com>
> 
> With new connector model, mhdp bridge will not create the connector and
> SoC driver will rely on format negotiation to setup the encoder format.
> 
> Support minimal format negotiations hooks in the drm_bridge_funcs.
> Complete format negotiation can be added based on EDID data.
> This patch adds the minimal required support to avoid failure
> after moving to new connector model.
> 
> Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
> [a-bhatia1: Drop the output_fmt check condition]
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> ---
> 
> Notes:
> 
>      changes from v1:
>      * cosmetic fixes, commit message update.
> 
>      changes from v5:
>      * drop the default_bus_format variable and directly assigned
>        MEDIA_BUS_FMT_RGB121212_1X36 to input_fmts.
> 
>      changes from v6:
>      * Drop the output_fmt check condition and hence drop Tomi
>        Valkeinen's R-b tag.
> 
>   .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 22 +++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index f6822dfa3805..afd4e353f37a 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -2146,6 +2146,27 @@ cdns_mhdp_bridge_atomic_reset(struct drm_bridge *bridge)
>   	return &cdns_mhdp_state->base;
>   }
>   
> +static u32 *cdns_mhdp_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)
> +{
> +	u32 *input_fmts;
> +
> +	*num_input_fmts = 0;
> +
> +	input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
> +	if (!input_fmts)
> +		return NULL;
> +
> +	*num_input_fmts = 1;
> +	input_fmts[0] = MEDIA_BUS_FMT_RGB121212_1X36;
> +
> +	return input_fmts;
> +}
> +
>   static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
>   				  struct drm_bridge_state *bridge_state,
>   				  struct drm_crtc_state *crtc_state,
> @@ -2210,6 +2231,7 @@ static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = {
>   	.atomic_duplicate_state = cdns_mhdp_bridge_atomic_duplicate_state,
>   	.atomic_destroy_state = cdns_mhdp_bridge_atomic_destroy_state,
>   	.atomic_reset = cdns_mhdp_bridge_atomic_reset,
> +	.atomic_get_input_bus_fmts = cdns_mhdp_get_input_bus_fmts,
>   	.detect = cdns_mhdp_bridge_detect,
>   	.get_edid = cdns_mhdp_bridge_get_edid,
>   	.hpd_enable = cdns_mhdp_bridge_hpd_enable,

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH v7 1/8] drm/bridge: tfp410: Support format negotiation hooks

[Neil Armstrong]

On 06/06/2023 10:21, Aradhya Bhatia wrote:
> From: Nikhil Devshatwar <nikhil.nd@ti.com>
> 
> With new connector model, tfp410 will not create the connector and
> SoC driver will rely on format negotiation to setup the encoder format.
> 
> Support format negotiations hooks in the drm_bridge_funcs.
> Use helper functions for state management.
> 
> Input format is the one selected by the bridge from DT properties.
> 
> Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
> [a-bhatia1: Removed output fmt condition check]
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> ---
> 
> Notes:
>      changes from v1:
>      * Use only MEDIA_BUS_FMT_FIXED for output
> 
>      changes from V5:
>      * Drop the output format check condition because the output
>        format for HDMI bridges should be RGB888_1X24 and not FIXED.
>        Hence, also drop Tomi Valkeinen's and Laurent Pinchart's R-b
>        tags.
> 
>   drivers/gpu/drm/bridge/ti-tfp410.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> index ab63225cd635..7dacc7e03eee 100644
> --- a/drivers/gpu/drm/bridge/ti-tfp410.c
> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> @@ -206,12 +206,38 @@ static enum drm_mode_status tfp410_mode_valid(struct drm_bridge *bridge,
>   	return MODE_OK;
>   }
>   
> +static u32 *tfp410_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)
> +{
> +	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
> +	u32 *input_fmts;
> +
> +	*num_input_fmts = 0;
> +
> +	input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL);
> +	if (!input_fmts)
> +		return NULL;
> +
> +	*num_input_fmts = 1;
> +	input_fmts[0] = dvi->bus_format;
> +
> +	return input_fmts;
> +}
> +
>   static const struct drm_bridge_funcs tfp410_bridge_funcs = {
>   	.attach		= tfp410_attach,
>   	.detach		= tfp410_detach,
>   	.enable		= tfp410_enable,
>   	.disable	= tfp410_disable,
>   	.mode_valid	= tfp410_mode_valid,
> +	.atomic_reset = drm_atomic_helper_bridge_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts,
>   };
>   
>   static const struct drm_bridge_timings tfp410_default_timings = {

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH v7 0/8] drm/tidss: Use new connector model for tidss

[Neil Armstrong]

Hi,

On 06/06/2023 10:21, Aradhya Bhatia wrote:
> Hi all,
> 
> I have picked up this long standing series from Nikhil Devshatwar[1].
> 
> This series moves the tidss to using new connectoe model, where the SoC
> driver (tidss) creates the connector and all the bridges are attached
> with the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR. It also now creates bridge
> to support format negotiation and and 'simple' encoder to expose it to
> the userspace.
> 
> Since the bridges do not create the connector, the bus_format and
> bus_flag is set via atomic hooks.
> 
> Support format negotiations in the tfp410, sii902x and mhdp-8546 bridge
> drivers as a first step before moving the connector model.
> 
> These patches were tested on AM625-SK EVM, AM625 SoC based BeaglePlay,
> and J721E-SK. Display support for AM625 SoC has not been added upstream
> and is a WIP. To test this series on AM625 based platforms, basic
> display support patches, (for driver + devicetree), can be found in
> the "next_AttachNoConn-v2" branch on my github fork[2].

I can apply all bridge patches right now so only the tidss change remain,
is that ok for you ?


> 
> Thanks,
> Aradhya
> 
> [1]: https://patchwork.freedesktop.org/series/82765/#rev5
> [2]: https://github.com/aradhya07/linux-ab/tree/next_AttachNoConn-v2
> 
> Change Log:
> V6 -> V7
>    - Rebase and cosmetic changes.
>    - Drop the output format check condition for mhdp8546 and hence,
>      drop Tomi Valkeinen's R-b tag.
>    - Added tags wherever suggested.
> 
> V5 -> V6
>    - Rebase and cosmetic changes
>    - Dropped the output format check condition for tfp410 and hence,
>      dropped Tomi Valkeinen's and Laurent Pinchart's R-b tags.
>    - Based on Boris Brezillon's comments: dropped patches 5 and 6 from
>      the series and instead created a single patch that,
>        1. Creates tidss bridge for format negotiation.
>        2. Creates 'simple' encoder for userspace exposure.
>        3. Creates a tidss connector.
>        4. Attaches the next-bridge to encoder with the
>           DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
>    - Add format negotiation support for sii902x driver.
> 
> Previous versions:
> V1 to V6: https://patchwork.freedesktop.org/series/82765/
> 
> Aradhya Bhatia (3):
>    drm/bridge: sii902x: Support format negotiation hooks
>    drm/bridge: sii902x: Set input_bus_flags in atomic_check
>    drm/tidss: Update encoder/bridge chain connect model
> 
> Nikhil Devshatwar (5):
>    drm/bridge: tfp410: Support format negotiation hooks
>    drm/bridge: tfp410: Set input_bus_flags in atomic_check
>    drm/bridge: mhdp8546: Add minimal format negotiation
>    drm/bridge: mhdp8546: Set input_bus_flags from atomic_check
>    drm/bridge: cdns-mhdp8546: Fix the interrupt enable/disable
> 
>   .../drm/bridge/cadence/cdns-mhdp8546-core.c   |  77 ++++++----
>   .../drm/bridge/cadence/cdns-mhdp8546-core.h   |   2 +-
>   .../drm/bridge/cadence/cdns-mhdp8546-j721e.c  |   9 +-
>   .../drm/bridge/cadence/cdns-mhdp8546-j721e.h  |   2 +-
>   drivers/gpu/drm/bridge/sii902x.c              |  40 +++++
>   drivers/gpu/drm/bridge/ti-tfp410.c            |  43 ++++++
>   drivers/gpu/drm/tidss/tidss_encoder.c         | 140 +++++++++++-------
>   drivers/gpu/drm/tidss/tidss_encoder.h         |   5 +-
>   drivers/gpu/drm/tidss/tidss_kms.c             |  12 +-
>   9 files changed, 235 insertions(+), 95 deletions(-)
> 

Re: [PATCH v7 0/8] drm/tidss: Use new connector model for tidss

[Aradhya Bhatia]

Hi Neil,

Thank you for reviewing the previous patches!

On 06-Jun-23 14:37, Neil Armstrong wrote:
> Hi,
> 
> On 06/06/2023 10:21, Aradhya Bhatia wrote:
>> Hi all,
>>
>> I have picked up this long standing series from Nikhil Devshatwar[1].
>>
>> This series moves the tidss to using new connectoe model, where the SoC
>> driver (tidss) creates the connector and all the bridges are attached
>> with the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR. It also now creates bridge
>> to support format negotiation and and 'simple' encoder to expose it to
>> the userspace.
>>
>> Since the bridges do not create the connector, the bus_format and
>> bus_flag is set via atomic hooks.
>>
>> Support format negotiations in the tfp410, sii902x and mhdp-8546 bridge
>> drivers as a first step before moving the connector model.
>>
>> These patches were tested on AM625-SK EVM, AM625 SoC based BeaglePlay,
>> and J721E-SK. Display support for AM625 SoC has not been added upstream
>> and is a WIP. To test this series on AM625 based platforms, basic
>> display support patches, (for driver + devicetree), can be found in
>> the "next_AttachNoConn-v2" branch on my github fork[2].
> 
> I can apply all bridge patches right now so only the tidss change remain,
> is that ok for you ?
> 

While the bridge patches and the tidss patch can be separately built
without any issue, the tidss functionality will break if only the bridge
patches get picked up, and not the tidss.

Would it be possible for you to pick all the patches together once Tomi
acks the tidss patch?


Regards
Aradhya

> 
>>
>> Thanks,
>> Aradhya
>>
>> [1]: https://patchwork.freedesktop.org/series/82765/#rev5
>> [2]: https://github.com/aradhya07/linux-ab/tree/next_AttachNoConn-v2
>>
>> Change Log:
>> V6 -> V7
>>    - Rebase and cosmetic changes.
>>    - Drop the output format check condition for mhdp8546 and hence,
>>      drop Tomi Valkeinen's R-b tag.
>>    - Added tags wherever suggested.
>>
>> V5 -> V6
>>    - Rebase and cosmetic changes
>>    - Dropped the output format check condition for tfp410 and hence,
>>      dropped Tomi Valkeinen's and Laurent Pinchart's R-b tags.
>>    - Based on Boris Brezillon's comments: dropped patches 5 and 6 from
>>      the series and instead created a single patch that,
>>        1. Creates tidss bridge for format negotiation.
>>        2. Creates 'simple' encoder for userspace exposure.
>>        3. Creates a tidss connector.
>>        4. Attaches the next-bridge to encoder with the
>>           DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
>>    - Add format negotiation support for sii902x driver.
>>
>> Previous versions:
>> V1 to V6: https://patchwork.freedesktop.org/series/82765/
>>
>> Aradhya Bhatia (3):
>>    drm/bridge: sii902x: Support format negotiation hooks
>>    drm/bridge: sii902x: Set input_bus_flags in atomic_check
>>    drm/tidss: Update encoder/bridge chain connect model
>>
>> Nikhil Devshatwar (5):
>>    drm/bridge: tfp410: Support format negotiation hooks
>>    drm/bridge: tfp410: Set input_bus_flags in atomic_check
>>    drm/bridge: mhdp8546: Add minimal format negotiation
>>    drm/bridge: mhdp8546: Set input_bus_flags from atomic_check
>>    drm/bridge: cdns-mhdp8546: Fix the interrupt enable/disable
>>
>>   .../drm/bridge/cadence/cdns-mhdp8546-core.c   |  77 ++++++----
>>   .../drm/bridge/cadence/cdns-mhdp8546-core.h   |   2 +-
>>   .../drm/bridge/cadence/cdns-mhdp8546-j721e.c  |   9 +-
>>   .../drm/bridge/cadence/cdns-mhdp8546-j721e.h  |   2 +-
>>   drivers/gpu/drm/bridge/sii902x.c              |  40 +++++
>>   drivers/gpu/drm/bridge/ti-tfp410.c            |  43 ++++++
>>   drivers/gpu/drm/tidss/tidss_encoder.c         | 140 +++++++++++-------
>>   drivers/gpu/drm/tidss/tidss_encoder.h         |   5 +-
>>   drivers/gpu/drm/tidss/tidss_kms.c             |  12 +-
>>   9 files changed, 235 insertions(+), 95 deletions(-)
>>
> 

Re: [PATCH v7 0/8] drm/tidss: Use new connector model for tidss

[neil.armstrong]

On 06/06/2023 11:46, Aradhya Bhatia wrote:
> Hi Neil,
> 
> Thank you for reviewing the previous patches!
> 
> On 06-Jun-23 14:37, Neil Armstrong wrote:
>> Hi,
>>
>> On 06/06/2023 10:21, Aradhya Bhatia wrote:
>>> Hi all,
>>>
>>> I have picked up this long standing series from Nikhil Devshatwar[1].
>>>
>>> This series moves the tidss to using new connectoe model, where the SoC
>>> driver (tidss) creates the connector and all the bridges are attached
>>> with the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR. It also now creates bridge
>>> to support format negotiation and and 'simple' encoder to expose it to
>>> the userspace.
>>>
>>> Since the bridges do not create the connector, the bus_format and
>>> bus_flag is set via atomic hooks.
>>>
>>> Support format negotiations in the tfp410, sii902x and mhdp-8546 bridge
>>> drivers as a first step before moving the connector model.
>>>
>>> These patches were tested on AM625-SK EVM, AM625 SoC based BeaglePlay,
>>> and J721E-SK. Display support for AM625 SoC has not been added upstream
>>> and is a WIP. To test this series on AM625 based platforms, basic
>>> display support patches, (for driver + devicetree), can be found in
>>> the "next_AttachNoConn-v2" branch on my github fork[2].
>>
>> I can apply all bridge patches right now so only the tidss change remain,
>> is that ok for you ?
>>
> 
> While the bridge patches and the tidss patch can be separately built
> without any issue, the tidss functionality will break if only the bridge
> patches get picked up, and not the tidss.
> 
> Would it be possible for you to pick all the patches together once Tomi
> acks the tidss patch?

Sure

Neil
> 
> 
> Regards
> Aradhya
> 
>>
>>>
>>> Thanks,
>>> Aradhya
>>>
>>> [1]: https://patchwork.freedesktop.org/series/82765/#rev5
>>> [2]: https://github.com/aradhya07/linux-ab/tree/next_AttachNoConn-v2
>>>
>>> Change Log:
>>> V6 -> V7
>>>     - Rebase and cosmetic changes.
>>>     - Drop the output format check condition for mhdp8546 and hence,
>>>       drop Tomi Valkeinen's R-b tag.
>>>     - Added tags wherever suggested.
>>>
>>> V5 -> V6
>>>     - Rebase and cosmetic changes
>>>     - Dropped the output format check condition for tfp410 and hence,
>>>       dropped Tomi Valkeinen's and Laurent Pinchart's R-b tags.
>>>     - Based on Boris Brezillon's comments: dropped patches 5 and 6 from
>>>       the series and instead created a single patch that,
>>>         1. Creates tidss bridge for format negotiation.
>>>         2. Creates 'simple' encoder for userspace exposure.
>>>         3. Creates a tidss connector.
>>>         4. Attaches the next-bridge to encoder with the
>>>            DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
>>>     - Add format negotiation support for sii902x driver.
>>>
>>> Previous versions:
>>> V1 to V6: https://patchwork.freedesktop.org/series/82765/
>>>
>>> Aradhya Bhatia (3):
>>>     drm/bridge: sii902x: Support format negotiation hooks
>>>     drm/bridge: sii902x: Set input_bus_flags in atomic_check
>>>     drm/tidss: Update encoder/bridge chain connect model
>>>
>>> Nikhil Devshatwar (5):
>>>     drm/bridge: tfp410: Support format negotiation hooks
>>>     drm/bridge: tfp410: Set input_bus_flags in atomic_check
>>>     drm/bridge: mhdp8546: Add minimal format negotiation
>>>     drm/bridge: mhdp8546: Set input_bus_flags from atomic_check
>>>     drm/bridge: cdns-mhdp8546: Fix the interrupt enable/disable
>>>
>>>    .../drm/bridge/cadence/cdns-mhdp8546-core.c   |  77 ++++++----
>>>    .../drm/bridge/cadence/cdns-mhdp8546-core.h   |   2 +-
>>>    .../drm/bridge/cadence/cdns-mhdp8546-j721e.c  |   9 +-
>>>    .../drm/bridge/cadence/cdns-mhdp8546-j721e.h  |   2 +-
>>>    drivers/gpu/drm/bridge/sii902x.c              |  40 +++++
>>>    drivers/gpu/drm/bridge/ti-tfp410.c            |  43 ++++++
>>>    drivers/gpu/drm/tidss/tidss_encoder.c         | 140 +++++++++++-------
>>>    drivers/gpu/drm/tidss/tidss_encoder.h         |   5 +-
>>>    drivers/gpu/drm/tidss/tidss_kms.c             |  12 +-
>>>    9 files changed, 235 insertions(+), 95 deletions(-)
>>>
>>

Re: [PATCH v7 0/8] drm/tidss: Use new connector model for tidss

[Tomi Valkeinen]

On 06/06/2023 12:48, neil.armstrong@linaro.org wrote:
> On 06/06/2023 11:46, Aradhya Bhatia wrote:
>> Hi Neil,
>>
>> Thank you for reviewing the previous patches!
>>
>> On 06-Jun-23 14:37, Neil Armstrong wrote:
>>> Hi,
>>>
>>> On 06/06/2023 10:21, Aradhya Bhatia wrote:
>>>> Hi all,
>>>>
>>>> I have picked up this long standing series from Nikhil Devshatwar[1].
>>>>
>>>> This series moves the tidss to using new connectoe model, where the SoC
>>>> driver (tidss) creates the connector and all the bridges are attached
>>>> with the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR. It also now creates 
>>>> bridge
>>>> to support format negotiation and and 'simple' encoder to expose it to
>>>> the userspace.
>>>>
>>>> Since the bridges do not create the connector, the bus_format and
>>>> bus_flag is set via atomic hooks.
>>>>
>>>> Support format negotiations in the tfp410, sii902x and mhdp-8546 bridge
>>>> drivers as a first step before moving the connector model.
>>>>
>>>> These patches were tested on AM625-SK EVM, AM625 SoC based BeaglePlay,
>>>> and J721E-SK. Display support for AM625 SoC has not been added upstream
>>>> and is a WIP. To test this series on AM625 based platforms, basic
>>>> display support patches, (for driver + devicetree), can be found in
>>>> the "next_AttachNoConn-v2" branch on my github fork[2].
>>>
>>> I can apply all bridge patches right now so only the tidss change 
>>> remain,
>>> is that ok for you ?
>>>
>>
>> While the bridge patches and the tidss patch can be separately built
>> without any issue, the tidss functionality will break if only the bridge
>> patches get picked up, and not the tidss.
>>
>> Would it be possible for you to pick all the patches together once Tomi
>> acks the tidss patch?
> 
> Sure

I think this looks fine. For the series:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi

Re: [PATCH v7 0/8] drm/tidss: Use new connector model for tidss

[Javier Martinez Canillas]

Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> writes:

Hello Tomi and Neil,

> On 06/06/2023 12:48, neil.armstrong@linaro.org wrote:
>> On 06/06/2023 11:46, Aradhya Bhatia wrote:
>>> Hi Neil,
>>>
>>> Thank you for reviewing the previous patches!
>>>
>>> On 06-Jun-23 14:37, Neil Armstrong wrote:
>>>> Hi,
>>>>
>>>> On 06/06/2023 10:21, Aradhya Bhatia wrote:
>>>>> Hi all,
>>>>>
>>>>> I have picked up this long standing series from Nikhil Devshatwar[1].
>>>>>
>>>>> This series moves the tidss to using new connectoe model, where the SoC
>>>>> driver (tidss) creates the connector and all the bridges are attached
>>>>> with the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR. It also now creates 
>>>>> bridge
>>>>> to support format negotiation and and 'simple' encoder to expose it to
>>>>> the userspace.
>>>>>
>>>>> Since the bridges do not create the connector, the bus_format and
>>>>> bus_flag is set via atomic hooks.
>>>>>
>>>>> Support format negotiations in the tfp410, sii902x and mhdp-8546 bridge
>>>>> drivers as a first step before moving the connector model.
>>>>>
>>>>> These patches were tested on AM625-SK EVM, AM625 SoC based BeaglePlay,
>>>>> and J721E-SK. Display support for AM625 SoC has not been added upstream
>>>>> and is a WIP. To test this series on AM625 based platforms, basic
>>>>> display support patches, (for driver + devicetree), can be found in
>>>>> the "next_AttachNoConn-v2" branch on my github fork[2].
>>>>
>>>> I can apply all bridge patches right now so only the tidss change 
>>>> remain,
>>>> is that ok for you ?
>>>>
>>>
>>> While the bridge patches and the tidss patch can be separately built
>>> without any issue, the tidss functionality will break if only the bridge
>>> patches get picked up, and not the tidss.
>>>
>>> Would it be possible for you to pick all the patches together once Tomi
>>> acks the tidss patch?
>> 
>> Sure
>
> I think this looks fine. For the series:
>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>

It seems this series fell through the cracks? Since you both already
reviewed the patches, I've just pushed all the set to drm-misc-next.

Thanks all!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v7 5/8] drm/bridge: sii902x: Support format negotiation hooks

[Sam Ravnborg]

Hi Aradhya,

On Tue, Jun 06, 2023 at 01:51:39PM +0530, Aradhya Bhatia wrote:
> With new connector model, sii902x will not create the connector, when
> DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and SoC driver will rely on format
> negotiation to setup the encoder format.
> 
> Support format negotiations hooks in the drm_bridge_funcs.
> Use helper functions for state management.
> 
> Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is
> the case with older model.
> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

As noted by Javier, this patch-set was forgotten, so sorry for not
providing timely feedback.


> ---
> 
> Notes:
> 
>     changes from v6:
>     * Add Neil Armstrong's R-b tag.
> 
>  drivers/gpu/drm/bridge/sii902x.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index ef66461e7f7c..70aeb04b7f77 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -473,6 +473,27 @@ static struct edid *sii902x_bridge_get_edid(struct drm_bridge *bridge,
>  	return sii902x_get_edid(sii902x, connector);
>  }
>  
> +static u32 *sii902x_bridge_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)
> +{
> +	u32 *input_fmts;
> +
> +	*num_input_fmts = 0;
> +
> +	input_fmts = kcalloc(1, sizeof(*input_fmts), GFP_KERNEL);
> +	if (!input_fmts)
> +		return NULL;
> +
> +	input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> +	*num_input_fmts = 1;
> +
> +	return input_fmts;
> +}

An alternative implementation of the above is:
{
        switch (output_fmt) {
        case MEDIA_BUS_FMT_RGB888_1X24:
                break;

        default:
        /* Fail for any other formats */
               *num_input_fmts = 0;
                return NULL;
        }

       return drm_atomic_helper_bridge_propagate_bus_fmt(bridge, bridge_state,
                                                         crtc_state, conn_state,
                                                         output_fmt,
                                                         num_input_fmts);
}

If you agree and have the time to do it it would be nice to use this
simpler variant.
Mostly so we avoid more open coded variants like you already did, and
which we have plenty of already.

It would be even better to walk through other implementations of
get_input_bus_fmts and update them accordingly.

Again, sorry for being late here. Feel free to ignore if you already
moved on with something else.

	Sam

Re: [PATCH v7 5/8] drm/bridge: sii902x: Support format negotiation hooks

[Aradhya Bhatia]

Hi Sam,

On 10-Jul-23 20:38, Sam Ravnborg wrote:
> Hi Aradhya,
> 
> On Tue, Jun 06, 2023 at 01:51:39PM +0530, Aradhya Bhatia wrote:
>> With new connector model, sii902x will not create the connector, when
>> DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and SoC driver will rely on format
>> negotiation to setup the encoder format.
>>
>> Support format negotiations hooks in the drm_bridge_funcs.
>> Use helper functions for state management.
>>
>> Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is
>> the case with older model.
>>
>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> As noted by Javier, this patch-set was forgotten, so sorry for not
> providing timely feedback.

Thank you for reviewing my patch nevertheless! =)

> 
> 
>> ---
>>
>> Notes:
>>
>>     changes from v6:
>>     * Add Neil Armstrong's R-b tag.
>>
>>  drivers/gpu/drm/bridge/sii902x.c | 25 +++++++++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
>> index ef66461e7f7c..70aeb04b7f77 100644
>> --- a/drivers/gpu/drm/bridge/sii902x.c
>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>> @@ -473,6 +473,27 @@ static struct edid *sii902x_bridge_get_edid(struct drm_bridge *bridge,
>>  	return sii902x_get_edid(sii902x, connector);
>>  }
>>  
>> +static u32 *sii902x_bridge_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)
>> +{
>> +	u32 *input_fmts;
>> +
>> +	*num_input_fmts = 0;
>> +
>> +	input_fmts = kcalloc(1, sizeof(*input_fmts), GFP_KERNEL);
>> +	if (!input_fmts)
>> +		return NULL;
>> +
>> +	input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>> +	*num_input_fmts = 1;
>> +
>> +	return input_fmts;
>> +}
> 
> An alternative implementation of the above is:
> {
>         switch (output_fmt) {
>         case MEDIA_BUS_FMT_RGB888_1X24:
>                 break;
> 
>         default:
>         /* Fail for any other formats */
>                *num_input_fmts = 0;
>                 return NULL;
>         }
> 
>        return drm_atomic_helper_bridge_propagate_bus_fmt(bridge, bridge_state,
>                                                          crtc_state, conn_state,
>                                                          output_fmt,
>                                                          num_input_fmts);
> }
> 
> If you agree and have the time to do it it would be nice to use this
> simpler variant.
> Mostly so we avoid more open coded variants like you already did, and
> which we have plenty of already.

I agree with the idea that these hooks should get streamlined.

However, this particular approach will break things when the output_fmt
is defaulted to MEDIA_BUS_FMT_FIXED. Even if we add this format as a
fall-through case along with MEDIA_BUS_FMT_RGB888_1X24, tidss driver
will too then receive MEDIA_BUS_FMT_FIXED as an expected output format
and will throw an error.

The possibility of an equivalent if-check was discussed in the previous
version[1].

> 
> It would be even better to walk through other implementations of
> get_input_bus_fmts and update them accordingly.
> 
> Again, sorry for being late here. Feel free to ignore if you already
> moved on with something else.
> 

I am working on adding OLDI support for tidss, but if we can resolve the
above concern, and Javier agrees, I will be happy to add an incremental
fix for this! =)


Regards
Aradhya

[1]: https://patchwork.freedesktop.org/patch/536008/?series=82765&rev=6

Re: [PATCH v7 5/8] drm/bridge: sii902x: Support format negotiation hooks

[Javier Martinez Canillas]

Aradhya Bhatia <a-bhatia1@ti.com> writes:

Hello Aradhya,

> Hi Sam,
>

[...]

>> 
>> It would be even better to walk through other implementations of
>> get_input_bus_fmts and update them accordingly.
>> 
>> Again, sorry for being late here. Feel free to ignore if you already
>> moved on with something else.
>> 
>
> I am working on adding OLDI support for tidss, but if we can resolve the
> above concern, and Javier agrees, I will be happy to add an incremental
> fix for this! =)
>
>

Yes, an incremental patch on top of what has already been merged in
drm-misc-next works. Thanks!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v7 7/8] drm/tidss: Update encoder/bridge chain connect model

[Jan Kiszka]

On 06.06.23 10:21, Aradhya Bhatia wrote:
> With the new encoder/bridge chain model, the display controller driver
> is required to create a drm_connector entity instead of asking the
> bridge to do so during drm_bridge_attach. Moreover, the controller
> driver should create a drm_bridge entity to negotiate bus formats and a
> 'simple' drm_encoder entity to expose it to userspace.
> 
> Update the encoder/bridge initialization sequence in tidss as per the
> new model.
> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> ---
> 
> Notes:
> 
>     changes from v5:
>     * Drop patches 5 and 6 from the original series.
>     * Instead add this patch that addresses Boris Brezillon's comments
>       of creating bridge, simple encoder and connector.
> 
>  drivers/gpu/drm/tidss/tidss_encoder.c | 140 ++++++++++++++++----------
>  drivers/gpu/drm/tidss/tidss_encoder.h |   5 +-
>  drivers/gpu/drm/tidss/tidss_kms.c     |  12 +--
>  3 files changed, 94 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c
> index 0d4865e9c03d..17a86bed8054 100644
> --- a/drivers/gpu/drm/tidss/tidss_encoder.c
> +++ b/drivers/gpu/drm/tidss/tidss_encoder.c
> @@ -6,91 +6,125 @@
>  
>  #include <linux/export.h>
>  
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_modeset_helper_vtables.h>
>  #include <drm/drm_panel.h>
>  #include <drm/drm_of.h>
> +#include <drm/drm_simple_kms_helper.h>
>  
>  #include "tidss_crtc.h"
>  #include "tidss_drv.h"
>  #include "tidss_encoder.h"
>  
> -static int tidss_encoder_atomic_check(struct drm_encoder *encoder,
> -				      struct drm_crtc_state *crtc_state,
> -				      struct drm_connector_state *conn_state)
> +struct tidss_encoder {
> +	struct drm_bridge bridge;
> +	struct drm_encoder encoder;
> +	struct drm_connector *connector;
> +	struct drm_bridge *next_bridge;
> +	struct tidss_device *tidss;
> +};
> +
> +static inline struct tidss_encoder
> +*bridge_to_tidss_encoder(struct drm_bridge *b)
> +{
> +	return container_of(b, struct tidss_encoder, bridge);
> +}
> +
> +static int tidss_bridge_attach(struct drm_bridge *bridge,
> +			       enum drm_bridge_attach_flags flags)
> +{
> +	struct tidss_encoder *t_enc = bridge_to_tidss_encoder(bridge);
> +
> +	return drm_bridge_attach(bridge->encoder, t_enc->next_bridge,
> +				 bridge, flags);
> +}
> +
> +static int tidss_bridge_atomic_check(struct drm_bridge *bridge,
> +				     struct drm_bridge_state *bridge_state,
> +				     struct drm_crtc_state *crtc_state,
> +				     struct drm_connector_state *conn_state)
>  {
> -	struct drm_device *ddev = encoder->dev;
> +	struct tidss_encoder *t_enc = bridge_to_tidss_encoder(bridge);
> +	struct tidss_device *tidss = t_enc->tidss;
>  	struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc_state);
>  	struct drm_display_info *di = &conn_state->connector->display_info;
> -	struct drm_bridge *bridge;
> -	bool bus_flags_set = false;
> -
> -	dev_dbg(ddev->dev, "%s\n", __func__);
> -
> -	/*
> -	 * Take the bus_flags from the first bridge that defines
> -	 * bridge timings, or from the connector's display_info if no
> -	 * bridge defines the timings.
> -	 */
> -	drm_for_each_bridge_in_chain(encoder, bridge) {
> -		if (!bridge->timings)
> -			continue;
> -
> -		tcrtc_state->bus_flags = bridge->timings->input_bus_flags;
> -		bus_flags_set = true;
> -		break;
> -	}
> +	struct drm_bridge_state *next_bridge_state = NULL;
> +
> +	if (t_enc->next_bridge)
> +		next_bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state,
> +								    t_enc->next_bridge);
>  
> -	if (!di->bus_formats || di->num_bus_formats == 0)  {
> -		dev_err(ddev->dev, "%s: No bus_formats in connected display\n",
> +	if (next_bridge_state) {
> +		tcrtc_state->bus_flags = next_bridge_state->input_bus_cfg.flags;
> +		tcrtc_state->bus_format = next_bridge_state->input_bus_cfg.format;
> +	} else if (di->num_bus_formats) {
> +		tcrtc_state->bus_format = di->bus_formats[0];
> +		tcrtc_state->bus_flags = di->bus_flags;
> +	} else {
> +		dev_err(tidss->dev, "%s: No bus_formats in connected display\n",
>  			__func__);
>  		return -EINVAL;
>  	}
>  
> -	// XXX any cleaner way to set bus format and flags?
> -	tcrtc_state->bus_format = di->bus_formats[0];
> -	if (!bus_flags_set)
> -		tcrtc_state->bus_flags = di->bus_flags;
> -
>  	return 0;
>  }
>  
> -static void tidss_encoder_destroy(struct drm_encoder *encoder)
> -{
> -	drm_encoder_cleanup(encoder);
> -	kfree(encoder);
> -}
> -
> -static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
> -	.atomic_check = tidss_encoder_atomic_check,
> -};
> -
> -static const struct drm_encoder_funcs encoder_funcs = {
> -	.destroy = tidss_encoder_destroy,
> +static const struct drm_bridge_funcs tidss_bridge_funcs = {
> +	.attach				= tidss_bridge_attach,
> +	.atomic_check			= tidss_bridge_atomic_check,
> +	.atomic_reset			= drm_atomic_helper_bridge_reset,
> +	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
>  };
>  
> -struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss,
> -					 u32 encoder_type, u32 possible_crtcs)
> +int tidss_encoder_create(struct tidss_device *tidss,
> +			 struct drm_bridge *next_bridge,
> +			 u32 encoder_type, u32 possible_crtcs)
>  {
> +	struct tidss_encoder *t_enc;
>  	struct drm_encoder *enc;
> +	struct drm_connector *connector;
>  	int ret;
>  
> -	enc = kzalloc(sizeof(*enc), GFP_KERNEL);
> -	if (!enc)
> -		return ERR_PTR(-ENOMEM);
> +	t_enc = drmm_simple_encoder_alloc(&tidss->ddev, struct tidss_encoder,
> +					  encoder, encoder_type);
> +	if (IS_ERR(t_enc))
> +		return PTR_ERR(t_enc);
> +
> +	t_enc->tidss = tidss;
> +	t_enc->next_bridge = next_bridge;
> +	t_enc->bridge.funcs = &tidss_bridge_funcs;
>  
> +	enc = &t_enc->encoder;
>  	enc->possible_crtcs = possible_crtcs;
>  
> -	ret = drm_encoder_init(&tidss->ddev, enc, &encoder_funcs,
> -			       encoder_type, NULL);
> -	if (ret < 0) {
> -		kfree(enc);
> -		return ERR_PTR(ret);
> +	/* Attaching first bridge to the encoder */
> +	ret = drm_bridge_attach(enc, &t_enc->bridge, NULL,
> +				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> +	if (ret) {
> +		dev_err(tidss->dev, "bridge attach failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Initializing the connector at the end of bridge-chain */
> +	connector = drm_bridge_connector_init(&tidss->ddev, enc);
> +	if (IS_ERR(connector)) {
> +		dev_err(tidss->dev, "bridge_connector create failed\n");
> +		return PTR_ERR(connector);
> +	}
> +
> +	ret = drm_connector_attach_encoder(connector, enc);
> +	if (ret) {
> +		dev_err(tidss->dev, "attaching encoder to connector failed\n");
> +		return ret;
>  	}
>  
> -	drm_encoder_helper_add(enc, &encoder_helper_funcs);
> +	t_enc->connector = connector;
>  
>  	dev_dbg(tidss->dev, "Encoder create done\n");
>  
> -	return enc;
> +	return ret;
>  }
> diff --git a/drivers/gpu/drm/tidss/tidss_encoder.h b/drivers/gpu/drm/tidss/tidss_encoder.h
> index ace877c0e0fd..3e561d6b1e83 100644
> --- a/drivers/gpu/drm/tidss/tidss_encoder.h
> +++ b/drivers/gpu/drm/tidss/tidss_encoder.h
> @@ -11,7 +11,8 @@
>  
>  struct tidss_device;
>  
> -struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss,
> -					 u32 encoder_type, u32 possible_crtcs);
> +int tidss_encoder_create(struct tidss_device *tidss,
> +			 struct drm_bridge *next_bridge,
> +			 u32 encoder_type, u32 possible_crtcs);
>  
>  #endif
> diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
> index ad2fa3c3d4a7..c979ad1af236 100644
> --- a/drivers/gpu/drm/tidss/tidss_kms.c
> +++ b/drivers/gpu/drm/tidss/tidss_kms.c
> @@ -193,7 +193,6 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>  	for (i = 0; i < num_pipes; ++i) {
>  		struct tidss_plane *tplane;
>  		struct tidss_crtc *tcrtc;
> -		struct drm_encoder *enc;
>  		u32 hw_plane_id = feat->vid_order[tidss->num_planes];
>  		int ret;
>  
> @@ -216,16 +215,13 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>  
>  		tidss->crtcs[tidss->num_crtcs++] = &tcrtc->crtc;
>  
> -		enc = tidss_encoder_create(tidss, pipes[i].enc_type,
> +		ret = tidss_encoder_create(tidss, pipes[i].bridge,
> +					   pipes[i].enc_type,
>  					   1 << tcrtc->crtc.index);
> -		if (IS_ERR(enc)) {
> +		if (ret) {
>  			dev_err(tidss->dev, "encoder create failed\n");
> -			return PTR_ERR(enc);
> -		}
> -
> -		ret = drm_bridge_attach(enc, pipes[i].bridge, NULL, 0);
> -		if (ret)
>  			return ret;
> +		}
>  	}
>  
>  	/* create overlay planes of the leftover planes */

This breaks the IOT2050 devices over 6.6, just bisected to it:

[    3.435153] [drm] Initialized tidss 1.0.0 20180215 for 4a00000.dss on minor 0
[    3.491246] tidss 4a00000.dss: [drm] Cannot find any crtc or sizes

vs.

[    3.436116] [drm] Initialized tidss 1.0.0 20180215 for 4a00000.dss on minor 0
[    3.910574] Console: switching to colour frame buffer device 80x30
[    3.937614] tidss 4a00000.dss: [drm] fb0: tidssdrmfb frame buffer device

Do we need to adjust its device tree to anything, or what may be the 
reason?

Jan

-- 
Siemens AG, Technology
Linux Expert Center

Re: [PATCH v7 7/8] drm/tidss: Update encoder/bridge chain connect model

[Aradhya Bhatia]

On 30-Oct-23 14:55, Jan Kiszka wrote:
> On 06.06.23 10:21, Aradhya Bhatia wrote:
>> With the new encoder/bridge chain model, the display controller driver
>> is required to create a drm_connector entity instead of asking the
>> bridge to do so during drm_bridge_attach. Moreover, the controller
>> driver should create a drm_bridge entity to negotiate bus formats and a
>> 'simple' drm_encoder entity to expose it to userspace.
>>
>> Update the encoder/bridge initialization sequence in tidss as per the
>> new model.
>>
>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>> ---
>>
>> Notes:
>>
>>     changes from v5:
>>     * Drop patches 5 and 6 from the original series.
>>     * Instead add this patch that addresses Boris Brezillon's comments
>>       of creating bridge, simple encoder and connector.
>>
>>  drivers/gpu/drm/tidss/tidss_encoder.c | 140 ++++++++++++++++----------
>>  drivers/gpu/drm/tidss/tidss_encoder.h |   5 +-
>>  drivers/gpu/drm/tidss/tidss_kms.c     |  12 +--
>>  3 files changed, 94 insertions(+), 63 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c
>> index 0d4865e9c03d..17a86bed8054 100644
>> --- a/drivers/gpu/drm/tidss/tidss_encoder.c
>> +++ b/drivers/gpu/drm/tidss/tidss_encoder.c
>> @@ -6,91 +6,125 @@
>>  
>>  #include <linux/export.h>
>>  
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_bridge.h>
>> +#include <drm/drm_bridge_connector.h>
>>  #include <drm/drm_crtc.h>
>>  #include <drm/drm_modeset_helper_vtables.h>
>>  #include <drm/drm_panel.h>
>>  #include <drm/drm_of.h>
>> +#include <drm/drm_simple_kms_helper.h>
>>  
>>  #include "tidss_crtc.h"
>>  #include "tidss_drv.h"
>>  #include "tidss_encoder.h"
>>  
>> -static int tidss_encoder_atomic_check(struct drm_encoder *encoder,
>> -				      struct drm_crtc_state *crtc_state,
>> -				      struct drm_connector_state *conn_state)
>> +struct tidss_encoder {
>> +	struct drm_bridge bridge;
>> +	struct drm_encoder encoder;
>> +	struct drm_connector *connector;
>> +	struct drm_bridge *next_bridge;
>> +	struct tidss_device *tidss;
>> +};
>> +
>> +static inline struct tidss_encoder
>> +*bridge_to_tidss_encoder(struct drm_bridge *b)
>> +{
>> +	return container_of(b, struct tidss_encoder, bridge);
>> +}
>> +
>> +static int tidss_bridge_attach(struct drm_bridge *bridge,
>> +			       enum drm_bridge_attach_flags flags)
>> +{
>> +	struct tidss_encoder *t_enc = bridge_to_tidss_encoder(bridge);
>> +
>> +	return drm_bridge_attach(bridge->encoder, t_enc->next_bridge,
>> +				 bridge, flags);
>> +}
>> +
>> +static int tidss_bridge_atomic_check(struct drm_bridge *bridge,
>> +				     struct drm_bridge_state *bridge_state,
>> +				     struct drm_crtc_state *crtc_state,
>> +				     struct drm_connector_state *conn_state)
>>  {
>> -	struct drm_device *ddev = encoder->dev;
>> +	struct tidss_encoder *t_enc = bridge_to_tidss_encoder(bridge);
>> +	struct tidss_device *tidss = t_enc->tidss;
>>  	struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc_state);
>>  	struct drm_display_info *di = &conn_state->connector->display_info;
>> -	struct drm_bridge *bridge;
>> -	bool bus_flags_set = false;
>> -
>> -	dev_dbg(ddev->dev, "%s\n", __func__);
>> -
>> -	/*
>> -	 * Take the bus_flags from the first bridge that defines
>> -	 * bridge timings, or from the connector's display_info if no
>> -	 * bridge defines the timings.
>> -	 */
>> -	drm_for_each_bridge_in_chain(encoder, bridge) {
>> -		if (!bridge->timings)
>> -			continue;
>> -
>> -		tcrtc_state->bus_flags = bridge->timings->input_bus_flags;
>> -		bus_flags_set = true;
>> -		break;
>> -	}
>> +	struct drm_bridge_state *next_bridge_state = NULL;
>> +
>> +	if (t_enc->next_bridge)
>> +		next_bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state,
>> +								    t_enc->next_bridge);
>>  
>> -	if (!di->bus_formats || di->num_bus_formats == 0)  {
>> -		dev_err(ddev->dev, "%s: No bus_formats in connected display\n",
>> +	if (next_bridge_state) {
>> +		tcrtc_state->bus_flags = next_bridge_state->input_bus_cfg.flags;
>> +		tcrtc_state->bus_format = next_bridge_state->input_bus_cfg.format;
>> +	} else if (di->num_bus_formats) {
>> +		tcrtc_state->bus_format = di->bus_formats[0];
>> +		tcrtc_state->bus_flags = di->bus_flags;
>> +	} else {
>> +		dev_err(tidss->dev, "%s: No bus_formats in connected display\n",
>>  			__func__);
>>  		return -EINVAL;
>>  	}
>>  
>> -	// XXX any cleaner way to set bus format and flags?
>> -	tcrtc_state->bus_format = di->bus_formats[0];
>> -	if (!bus_flags_set)
>> -		tcrtc_state->bus_flags = di->bus_flags;
>> -
>>  	return 0;
>>  }
>>  
>> -static void tidss_encoder_destroy(struct drm_encoder *encoder)
>> -{
>> -	drm_encoder_cleanup(encoder);
>> -	kfree(encoder);
>> -}
>> -
>> -static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
>> -	.atomic_check = tidss_encoder_atomic_check,
>> -};
>> -
>> -static const struct drm_encoder_funcs encoder_funcs = {
>> -	.destroy = tidss_encoder_destroy,
>> +static const struct drm_bridge_funcs tidss_bridge_funcs = {
>> +	.attach				= tidss_bridge_attach,
>> +	.atomic_check			= tidss_bridge_atomic_check,
>> +	.atomic_reset			= drm_atomic_helper_bridge_reset,
>> +	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
>> +	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
>>  };
>>  
>> -struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss,
>> -					 u32 encoder_type, u32 possible_crtcs)
>> +int tidss_encoder_create(struct tidss_device *tidss,
>> +			 struct drm_bridge *next_bridge,
>> +			 u32 encoder_type, u32 possible_crtcs)
>>  {
>> +	struct tidss_encoder *t_enc;
>>  	struct drm_encoder *enc;
>> +	struct drm_connector *connector;
>>  	int ret;
>>  
>> -	enc = kzalloc(sizeof(*enc), GFP_KERNEL);
>> -	if (!enc)
>> -		return ERR_PTR(-ENOMEM);
>> +	t_enc = drmm_simple_encoder_alloc(&tidss->ddev, struct tidss_encoder,
>> +					  encoder, encoder_type);
>> +	if (IS_ERR(t_enc))
>> +		return PTR_ERR(t_enc);
>> +
>> +	t_enc->tidss = tidss;
>> +	t_enc->next_bridge = next_bridge;
>> +	t_enc->bridge.funcs = &tidss_bridge_funcs;
>>  
>> +	enc = &t_enc->encoder;
>>  	enc->possible_crtcs = possible_crtcs;
>>  
>> -	ret = drm_encoder_init(&tidss->ddev, enc, &encoder_funcs,
>> -			       encoder_type, NULL);
>> -	if (ret < 0) {
>> -		kfree(enc);
>> -		return ERR_PTR(ret);
>> +	/* Attaching first bridge to the encoder */
>> +	ret = drm_bridge_attach(enc, &t_enc->bridge, NULL,
>> +				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>> +	if (ret) {
>> +		dev_err(tidss->dev, "bridge attach failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Initializing the connector at the end of bridge-chain */
>> +	connector = drm_bridge_connector_init(&tidss->ddev, enc);
>> +	if (IS_ERR(connector)) {
>> +		dev_err(tidss->dev, "bridge_connector create failed\n");
>> +		return PTR_ERR(connector);
>> +	}
>> +
>> +	ret = drm_connector_attach_encoder(connector, enc);
>> +	if (ret) {
>> +		dev_err(tidss->dev, "attaching encoder to connector failed\n");
>> +		return ret;
>>  	}
>>  
>> -	drm_encoder_helper_add(enc, &encoder_helper_funcs);
>> +	t_enc->connector = connector;
>>  
>>  	dev_dbg(tidss->dev, "Encoder create done\n");
>>  
>> -	return enc;
>> +	return ret;
>>  }
>> diff --git a/drivers/gpu/drm/tidss/tidss_encoder.h b/drivers/gpu/drm/tidss/tidss_encoder.h
>> index ace877c0e0fd..3e561d6b1e83 100644
>> --- a/drivers/gpu/drm/tidss/tidss_encoder.h
>> +++ b/drivers/gpu/drm/tidss/tidss_encoder.h
>> @@ -11,7 +11,8 @@
>>  
>>  struct tidss_device;
>>  
>> -struct drm_encoder *tidss_encoder_create(struct tidss_device *tidss,
>> -					 u32 encoder_type, u32 possible_crtcs);
>> +int tidss_encoder_create(struct tidss_device *tidss,
>> +			 struct drm_bridge *next_bridge,
>> +			 u32 encoder_type, u32 possible_crtcs);
>>  
>>  #endif
>> diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
>> index ad2fa3c3d4a7..c979ad1af236 100644
>> --- a/drivers/gpu/drm/tidss/tidss_kms.c
>> +++ b/drivers/gpu/drm/tidss/tidss_kms.c
>> @@ -193,7 +193,6 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>>  	for (i = 0; i < num_pipes; ++i) {
>>  		struct tidss_plane *tplane;
>>  		struct tidss_crtc *tcrtc;
>> -		struct drm_encoder *enc;
>>  		u32 hw_plane_id = feat->vid_order[tidss->num_planes];
>>  		int ret;
>>  
>> @@ -216,16 +215,13 @@ static int tidss_dispc_modeset_init(struct tidss_device *tidss)
>>  
>>  		tidss->crtcs[tidss->num_crtcs++] = &tcrtc->crtc;
>>  
>> -		enc = tidss_encoder_create(tidss, pipes[i].enc_type,
>> +		ret = tidss_encoder_create(tidss, pipes[i].bridge,
>> +					   pipes[i].enc_type,
>>  					   1 << tcrtc->crtc.index);
>> -		if (IS_ERR(enc)) {
>> +		if (ret) {
>>  			dev_err(tidss->dev, "encoder create failed\n");
>> -			return PTR_ERR(enc);
>> -		}
>> -
>> -		ret = drm_bridge_attach(enc, pipes[i].bridge, NULL, 0);
>> -		if (ret)
>>  			return ret;
>> +		}
>>  	}
>>  
>>  	/* create overlay planes of the leftover planes */
> 
> This breaks the IOT2050 devices over 6.6, just bisected to it:
> 
> [    3.435153] [drm] Initialized tidss 1.0.0 20180215 for 4a00000.dss on minor 0
> [    3.491246] tidss 4a00000.dss: [drm] Cannot find any crtc or sizes
> 
> vs.
> 
> [    3.436116] [drm] Initialized tidss 1.0.0 20180215 for 4a00000.dss on minor 0
> [    3.910574] Console: switching to colour frame buffer device 80x30
> [    3.937614] tidss 4a00000.dss: [drm] fb0: tidssdrmfb frame buffer device
> 
> Do we need to adjust its device tree to anything, or what may be the 
> reason?
> 

Hey Jan,

This patch series added support for the DRM_BRIDGE_ATTACH_NO_CONNECTOR
flag, for tidss, and for a few of the bridges.

Upon a quick look at the devicetree files, I see that the IOT-2050
platform is using Toshiba's TC358767 DPI to DP bridge, and it appears
that the TC358767 driver does not support the get_input_bus_fmt hook.

I have sent a patch for it[0].

Since I do not have hardware with me, I would appreciate it if you could
test and review it. The patch has only been build tested.


Regards
Aradhya


[0]: Patch Link:
https://lore.kernel.org/all/20231030192846.27934-1-a-bhatia1@ti.com/