Re: [PATCH 2/2] drm/msm/dsi: switch to DRM_PANEL_BRIDGE

On 08/12/2021 01:29, Dmitry Baryshkov wrote:

Currently the DSI driver has two separate paths: one if the next device in a chain is a bridge and another one if the panel is connected directly to the DSI host. Simplify the code path by using panel-bridge driver (already selected in Kconfig) and dropping support for handling the panel directly.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

In favour of merging DSC first, I'm deferring this patch till we can pass DSC data w/o using the drm panel.

---

drivers/gpu/drm/msm/dsi/dsi.c | 32 +--- drivers/gpu/drm/msm/dsi/dsi.h | 10 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 20 +- drivers/gpu/drm/msm/dsi/dsi_manager.c | 257 +++----------------------- 4 files changed, 32 insertions(+), 287 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c index 9670e548b3e9..b61f451a282e 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.c +++ b/drivers/gpu/drm/msm/dsi/dsi.c @@ -208,7 +208,7 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev, struct drm_encoder *encoder) { struct msm_drm_private *priv;

• struct drm_bridge *ext_bridge;

• struct drm_connector *connector; int ret;

  if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))

@@ -238,31 +238,17 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev, goto fail; }

• /*

• * check if the dsi encoder output is connected to a panel or an
  

• * external bridge. We create a connector only if we're connected to a
  

• * drm_panel device. When we're connected to an external bridge, we
  

• * assume that the drm_bridge driver will create the connector itself.
  

• */
  

• ext_bridge = msm_dsi_host_get_bridge(msm_dsi->host);
• if (ext_bridge)

• msm_dsi->connector =
  

• 	msm_dsi_manager_ext_bridge_init(msm_dsi->id);
  

• else

• msm_dsi->connector =
  

• 	msm_dsi_manager_connector_init(msm_dsi->id);
  

• if (IS_ERR(msm_dsi->connector)) {

• ret = PTR_ERR(msm_dsi->connector);
  

• connector = msm_dsi_manager_ext_bridge_init(msm_dsi->id);
• if (IS_ERR(connector)) {

• ret = PTR_ERR(connector);
  

  DRM_DEV_ERROR(dev->dev, "failed to create dsi connector: %d\n", ret);

• msm_dsi->connector = NULL;
  

  goto fail; }

  priv->bridges[priv->num_bridges++] = msm_dsi->bridge;

• priv->connectors[priv->num_connectors++] = msm_dsi->connector;

• priv->connectors[priv->num_connectors++] = connector;

  return 0; fail:

@@ -272,12 +258,6 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev, msm_dsi->bridge = NULL; }

• /* don't destroy connector if we didn't make it */
• if (msm_dsi->connector && !msm_dsi->external_bridge)

• msm_dsi->connector->funcs->destroy(msm_dsi->connector);
  

• msm_dsi->connector = NULL;
• return ret; }

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index f46df52c6985..7eb778070a8c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -49,8 +49,6 @@ struct msm_dsi { struct drm_device *dev; struct platform_device *pdev;

• /* connector managed by us when we're connected to a drm_panel */
• struct drm_connector *connector; /* internal dsi bridge attached to MDP interface */ struct drm_bridge *bridge;

@@ -58,10 +56,8 @@ struct msm_dsi { struct msm_dsi_phy *phy;

/*

• * panel/external_bridge connected to dsi bridge output, only one of the
  

• * two can be valid at a time
  

• * external_bridge connected to dsi bridge output
  

  */

• struct drm_panel *panel; struct drm_bridge *external_bridge;

  struct device *phy_dev;

@@ -76,7 +72,6 @@ struct msm_dsi { /* dsi manager */ struct drm_bridge *msm_dsi_manager_bridge_init(u8 id); void msm_dsi_manager_bridge_destroy(struct drm_bridge *bridge); -struct drm_connector *msm_dsi_manager_connector_init(u8 id); struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id); int msm_dsi_manager_cmd_xfer(int id, const struct mipi_dsi_msg *msg); bool msm_dsi_manager_cmd_xfer_trigger(int id, u32 dma_base, u32 len); @@ -88,7 +83,7 @@ void msm_dsi_manager_tpg_enable(void); /* msm dsi */ static inline bool msm_dsi_device_connected(struct msm_dsi *msm_dsi) {

• return msm_dsi->panel || msm_dsi->external_bridge;

• return msm_dsi->external_bridge; }

  struct drm_encoder *msm_dsi_get_encoder(struct msm_dsi *msm_dsi);

@@ -115,7 +110,6 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host, int msm_dsi_host_power_off(struct mipi_dsi_host *host); int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host, const struct drm_display_mode *mode); -struct drm_panel *msm_dsi_host_get_panel(struct mipi_dsi_host *host); unsigned long msm_dsi_host_get_mode_flags(struct mipi_dsi_host *host); struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host); int msm_dsi_host_register(struct mipi_dsi_host *host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 5b4bb722f750..58bd48d4ec03 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -159,7 +159,6 @@ struct msm_dsi_host { struct drm_display_mode *mode;

/* connected device info */

• struct device_node *device_node; unsigned int channel; unsigned int lanes; enum mipi_dsi_pixel_format format;

@@ -1604,8 +1603,6 @@ static int dsi_host_detach(struct mipi_dsi_host *host,

dsi_dev_detach(msm_host->pdev);

• msm_host->device_node = NULL;
• DBG("id=%d", msm_host->id); if (msm_host->dev) queue_work(msm_host->workqueue, &msm_host->hpd_work);

@@ -1745,16 +1742,6 @@ static int dsi_host_parse_dt(struct msm_dsi_host *msm_host) goto err; }

• /* Get panel node from the output port's endpoint data */
• device_node = of_graph_get_remote_node(np, 1, 0);
• if (!device_node) {

• DRM_DEV_DEBUG(dev, "%s: no valid device\n", __func__);
  

• ret = -ENODEV;
  

• goto err;
  

• }
• msm_host->device_node = device_node;
• if (of_property_read_bool(np, "syscon-sfpb")) { msm_host->sfpb = syscon_regmap_lookup_by_phandle(np, "syscon-sfpb");

@@ -2405,11 +2392,6 @@ int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host, return 0; }

-struct drm_panel *msm_dsi_host_get_panel(struct mipi_dsi_host *host) -{

• return of_drm_find_panel(to_msm_dsi_host(host)->device_node);

-}

• unsigned long msm_dsi_host_get_mode_flags(struct mipi_dsi_host *host) { return to_msm_dsi_host(host)->mode_flags;

@@ -2419,7 +2401,7 @@ struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host) { struct msm_dsi_host *msm_host = to_msm_dsi_host(host);

• return of_drm_find_bridge(msm_host->device_node);

• return devm_drm_of_get_bridge(&msm_host->pdev->dev, msm_host->pdev->dev.of_node, 1, 0); }

  void msm_dsi_host_snapshot(struct msm_disp_state *disp_state, struct mipi_dsi_host *host)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index 497719efb9e9..43b8a1268c4b 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -188,39 +188,26 @@ static void dsi_mgr_phy_disable(int id) } }

-struct dsi_connector {

• struct drm_connector base;
• int id;

-};

• struct dsi_bridge { struct drm_bridge base; int id; };

-#define to_dsi_connector(x) container_of(x, struct dsi_connector, base) #define to_dsi_bridge(x) container_of(x, struct dsi_bridge, base)

-static inline int dsi_mgr_connector_get_id(struct drm_connector *connector) -{

• struct dsi_connector *dsi_connector = to_dsi_connector(connector);
• return dsi_connector->id;

-}

• static int dsi_mgr_bridge_get_id(struct drm_bridge *bridge) { struct dsi_bridge *dsi_bridge = to_dsi_bridge(bridge); return dsi_bridge->id; }

-static int msm_dsi_manager_panel_init(struct drm_connector *conn, u8 id) +static void msm_dsi_manager_set_split_display(u8 id) {

• struct msm_drm_private *priv = conn->dev->dev_private;
• struct msm_kms *kms = priv->kms; struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); struct msm_dsi *other_dsi = dsi_mgr_get_other_dsi(id);

• struct msm_drm_private *priv = msm_dsi->dev->dev_private;
• struct msm_kms *kms = priv->kms; struct msm_dsi *master_dsi, *slave_dsi;

• struct drm_panel *panel;

  if (IS_BONDED_DSI() && !IS_MASTER_DSI_LINK(id)) { master_dsi = other_dsi;

@@ -230,88 +217,29 @@ static int msm_dsi_manager_panel_init(struct drm_connector *conn, u8 id) slave_dsi = other_dsi; }

• /*

• * There is only 1 panel in the global panel list for bonded DSI mode.
  

• * Therefore slave dsi should get the drm_panel instance from master
  

• * dsi.
  

• */
  

• panel = msm_dsi_host_get_panel(master_dsi->host);
• if (IS_ERR(panel)) {

• DRM_ERROR("Could not find panel for %u (%ld)\n", msm_dsi->id,
  

• 	  PTR_ERR(panel));
  

• return PTR_ERR(panel);
  

• }
• if (!panel || !IS_BONDED_DSI())

• goto out;
  

• drm_object_attach_property(&conn->base,

• 		   conn->dev->mode_config.tile_property, 0);
  

• if (!msm_dsi->external_bridge || !IS_BONDED_DSI())

• return;
  
  

  /*

  • Set split display info to kms once bonded DSI panel is connected to
  • both hosts.

  */

• if (other_dsi && other_dsi->panel && kms->funcs->set_split_display) {

• if (other_dsi && other_dsi->external_bridge && kms->funcs->set_split_display) { kms->funcs->set_split_display(kms, master_dsi->encoder, slave_dsi->encoder, msm_dsi_is_cmd_mode(msm_dsi)); }

-out:

• msm_dsi->panel = panel;
• return 0;

-}

-static enum drm_connector_status dsi_mgr_connector_detect(

• struct drm_connector *connector, bool force)
  

-{

• int id = dsi_mgr_connector_get_id(connector);
• struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
• return msm_dsi->panel ? connector_status_connected :

• connector_status_disconnected;
  

-}

-static void dsi_mgr_connector_destroy(struct drm_connector *connector) -{

• struct dsi_connector *dsi_connector = to_dsi_connector(connector);
• DBG("");
• drm_connector_cleanup(connector);
• kfree(dsi_connector);

-}

-static int dsi_mgr_connector_get_modes(struct drm_connector *connector) -{

• int id = dsi_mgr_connector_get_id(connector);
• struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
• struct drm_panel *panel = msm_dsi->panel;
• int num;
• if (!panel)

• return 0;
  

• /*

• * In bonded DSI mode, we have one connector that can be
  

• * attached to the drm_panel.
  

• */
  

• num = drm_panel_get_modes(panel, connector);
• if (!num)

• return 0;
  

• return num; }

-static enum drm_mode_status dsi_mgr_connector_mode_valid(struct drm_connector *connector,

• 		struct drm_display_mode *mode)
  

+static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,

• 		const struct drm_display_info *info,
  

• 		const struct drm_display_mode *mode)
  

  {

• int id = dsi_mgr_connector_get_id(connector);

• int id = dsi_mgr_bridge_get_id(bridge); struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); struct drm_encoder *encoder = msm_dsi_get_encoder(msm_dsi);

• struct msm_drm_private *priv = connector->dev->dev_private;

• struct drm_device *dev = msm_dsi->dev;
• struct msm_drm_private *priv = dev->dev_private; struct msm_kms *kms = priv->kms; long actual, requested;

@@ -326,16 +254,6 @@ static enum drm_mode_status dsi_mgr_connector_mode_valid(struct drm_connector *c return MODE_OK; }

-static struct drm_encoder * -dsi_mgr_connector_best_encoder(struct drm_connector *connector) -{

• int id = dsi_mgr_connector_get_id(connector);
• struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
• DBG("");
• return msm_dsi_get_encoder(msm_dsi);

-}

• static void dsi_mgr_bridge_power_on(struct drm_bridge *bridge) { int id = dsi_mgr_bridge_get_id(bridge);

@@ -398,7 +316,6 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); struct mipi_dsi_host *host = msm_dsi->host;

• struct drm_panel *panel = msm_dsi->panel; bool is_bonded_dsi = IS_BONDED_DSI(); int ret;

@@ -410,18 +327,6 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id)) return;

• /* Always call panel functions once, because even for dual panels,

• * there is only one drm_panel instance.
  

• */
  

• if (panel) {

• ret = drm_panel_prepare(panel);
  

• if (ret) {
  

• 	pr_err("%s: prepare panel %d failed, %d\n", __func__,
  

• 						id, ret);
  

• 	goto panel_prep_fail;
  

• }
  

• }
• ret = msm_dsi_host_enable(host); if (ret) { pr_err("%s: enable host %d failed, %d\n", __func__, id, ret);

@@ -441,9 +346,6 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) host1_en_fail: msm_dsi_host_disable(host); host_en_fail:

• if (panel)

• drm_panel_unprepare(panel);
  

-panel_prep_fail:

return; } @@ -461,62 +363,12 @@ void msm_dsi_manager_tpg_enable(void) } }

-static void dsi_mgr_bridge_enable(struct drm_bridge *bridge) -{

• int id = dsi_mgr_bridge_get_id(bridge);
• struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
• struct drm_panel *panel = msm_dsi->panel;
• bool is_bonded_dsi = IS_BONDED_DSI();
• int ret;
• DBG("id=%d", id);
• if (!msm_dsi_device_connected(msm_dsi))

• return;
  

• /* Do nothing with the host if it is slave-DSI in case of bonded DSI */
• if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id))

• return;
  

• if (panel) {

• ret = drm_panel_enable(panel);
  

• if (ret) {
  

• 	pr_err("%s: enable panel %d failed, %d\n", __func__, id,
  

• 							ret);
  

• }
  

• }

-}

-static void dsi_mgr_bridge_disable(struct drm_bridge *bridge) -{

• int id = dsi_mgr_bridge_get_id(bridge);
• struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
• struct drm_panel *panel = msm_dsi->panel;
• bool is_bonded_dsi = IS_BONDED_DSI();
• int ret;
• DBG("id=%d", id);
• if (!msm_dsi_device_connected(msm_dsi))

• return;
  

• /* Do nothing with the host if it is slave-DSI in case of bonded DSI */
• if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id))

• return;
  

• if (panel) {

• ret = drm_panel_disable(panel);
  

• if (ret)
  

• 	pr_err("%s: Panel %d OFF failed, %d\n", __func__, id,
  

• 							ret);
  

• }

-}

• static void dsi_mgr_bridge_post_disable(struct drm_bridge *bridge) { int id = dsi_mgr_bridge_get_id(bridge); struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); struct mipi_dsi_host *host = msm_dsi->host;
• struct drm_panel *panel = msm_dsi->panel; bool is_bonded_dsi = IS_BONDED_DSI(); int ret;

@@ -543,13 +395,6 @@ static void dsi_mgr_bridge_post_disable(struct drm_bridge *bridge) pr_err("%s: host1 disable failed, %d\n", __func__, ret); }

• if (panel) {

• ret = drm_panel_unprepare(panel);
  

• if (ret)
  

• 	pr_err("%s: Panel %d unprepare failed,%d\n", __func__,
  

• 						id, ret);
  

• }
• msm_dsi_host_disable_irq(host); if (is_bonded_dsi && msm_dsi1) msm_dsi_host_disable_irq(msm_dsi1->host);

@@ -594,76 +439,13 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge, dsi_mgr_bridge_power_on(bridge); }

-static const struct drm_connector_funcs dsi_mgr_connector_funcs = {

• .detect = dsi_mgr_connector_detect,
• .fill_modes = drm_helper_probe_single_connector_modes,
• .destroy = dsi_mgr_connector_destroy,
• .reset = drm_atomic_helper_connector_reset,
• .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
• .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,

-};

-static const struct drm_connector_helper_funcs dsi_mgr_conn_helper_funcs = {

• .get_modes = dsi_mgr_connector_get_modes,
• .mode_valid = dsi_mgr_connector_mode_valid,
• .best_encoder = dsi_mgr_connector_best_encoder,

-};

• static const struct drm_bridge_funcs dsi_mgr_bridge_funcs = { .pre_enable = dsi_mgr_bridge_pre_enable,
• .enable = dsi_mgr_bridge_enable,
• .disable = dsi_mgr_bridge_disable, .post_disable = dsi_mgr_bridge_post_disable, .mode_set = dsi_mgr_bridge_mode_set,

• .mode_valid = dsi_mgr_bridge_mode_valid, };

-/* initialize connector when we're connected to a drm_panel */ -struct drm_connector *msm_dsi_manager_connector_init(u8 id) -{

• struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
• struct drm_connector *connector = NULL;
• struct dsi_connector *dsi_connector;
• int ret;
• dsi_connector = kzalloc(sizeof(*dsi_connector), GFP_KERNEL);
• if (!dsi_connector)

• return ERR_PTR(-ENOMEM);
  

• dsi_connector->id = id;
• connector = &dsi_connector->base;
• ret = drm_connector_init(msm_dsi->dev, connector,

• 	&dsi_mgr_connector_funcs, DRM_MODE_CONNECTOR_DSI);
  

• if (ret)

• return ERR_PTR(ret);
  

• drm_connector_helper_add(connector, &dsi_mgr_conn_helper_funcs);
• /* Enable HPD to let hpd event is handled

• * when panel is attached to the host.
  

• */
  

• connector->polled = DRM_CONNECTOR_POLL_HPD;
• /* Display driver doesn't support interlace now. */
• connector->interlace_allowed = 0;
• connector->doublescan_allowed = 0;
• drm_connector_attach_encoder(connector, msm_dsi->encoder);
• ret = msm_dsi_manager_panel_init(connector, id);
• if (ret) {

• DRM_DEV_ERROR(msm_dsi->dev->dev, "init panel failed %d\n", ret);
  

• goto fail;
  

• }
• return connector;

-fail:

• connector->funcs->destroy(msm_dsi->connector);
• return ERR_PTR(ret);

-}

• bool msm_dsi_manager_validate_current_config(u8 id) { bool is_bonded_dsi = IS_BONDED_DSI();

@@ -727,8 +509,11 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id) int ret;

int_bridge = msm_dsi->bridge;

• ext_bridge = msm_dsi->external_bridge =

• 	msm_dsi_host_get_bridge(msm_dsi->host);
  

• ext_bridge = msm_dsi_host_get_bridge(msm_dsi->host);

• if (IS_ERR(ext_bridge))

• return ERR_PTR(PTR_ERR(ext_bridge));
  

• msm_dsi->external_bridge = ext_bridge;

  encoder = msm_dsi->encoder;

@@ -755,7 +540,7 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id)

list_for_each_entry(connector, connector_list, head) {
	if (drm_connector_has_possible_encoder(connector, encoder))

• 		return connector;
  

• 		goto out;
  

  }

  return ERR_PTR(-ENODEV);

@@ -769,6 +554,10 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id)

drm_connector_attach_encoder(connector, encoder);

+out:

• /* The pipeline is ready, ping encoders if necessary */
• msm_dsi_manager_set_split_display(id);
• return connector; }