Re: [PATCH v2 4/7] drm/mediatek: Use the panel-bridge helper.

On Thu, 11 May 2017 11:31:25 -0700 Eric Anholt eric@anholt.net wrote:

Avoids a bunch of connector boilerplate. Note that this causes panel prepare() to be moved before mtk_dsi_poweron() and unprepare() to be after poweroff(). I think this is the expected usage of the panel API (enable should be when you do things that require the link to be brought up), but there may be issues here.

Signed-off-by: Eric Anholt eric@anholt.net

Reviewed-by: Boris Brezillon boris.brezillon@free-electrons.com

---

Note that I haven't tested this change, and am not committed to this patch. It's just an optional cleanup, if it works for you.

drivers/gpu/drm/mediatek/mtk_dsi.c | 125 ++++--------------------------------- 1 file changed, 13 insertions(+), 112 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 808b995a990f..764bd8b9c256 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -160,7 +160,6 @@ struct mtk_dsi { struct device *dev; struct mipi_dsi_host host; struct drm_encoder encoder;

• struct drm_connector conn; struct drm_panel *panel; struct drm_bridge *bridge; struct phy *phy;

@@ -188,11 +187,6 @@ static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e) return container_of(e, struct mtk_dsi, encoder); }

-static inline struct mtk_dsi *connector_to_dsi(struct drm_connector *c) -{

• return container_of(c, struct mtk_dsi, conn);

-}

static inline struct mtk_dsi *host_to_dsi(struct mipi_dsi_host *h) { return container_of(h, struct mtk_dsi, host); @@ -603,16 +597,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) mtk_dsi_lane0_ulp_mode_leave(dsi); mtk_dsi_clk_hs_mode(dsi, 0);

• if (dsi->panel) {

• if (drm_panel_prepare(dsi->panel)) {
  

• 	DRM_ERROR("failed to prepare the panel\n");
  

• 	goto err_disable_digital_clk;
  

• }
  

• }
• return 0;

-err_disable_digital_clk:

• clk_disable_unprepare(dsi->digital_clk);

err_disable_engine_clk: clk_disable_unprepare(dsi->engine_clk); err_phy_power_off: @@ -630,15 +615,7 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi) if (--dsi->refcount != 0) return;

• if (!mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) {

• if (dsi->panel) {
  

• 	if (drm_panel_unprepare(dsi->panel)) {
  

• 		DRM_ERROR("failed to unprepare the panel\n");
  

• 		return;
  

• 	}
  

• }
  

• }

• mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); mtk_dsi_reset_engine(dsi); mtk_dsi_lane0_ulp_mode_enter(dsi); mtk_dsi_clk_ulp_mode_enter(dsi);

@@ -669,19 +646,9 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)

mtk_dsi_start(dsi);

• if (dsi->panel) {

• if (drm_panel_enable(dsi->panel)) {
  

• 	DRM_ERROR("failed to enable the panel\n");
  

• 	goto err_dsi_power_off;
  

• }
  

• }

• dsi->enabled = true;

  return;

-err_dsi_power_off:

• mtk_dsi_stop(dsi);
• mtk_dsi_poweroff(dsi);

}

static void mtk_output_dsi_disable(struct mtk_dsi *dsi) @@ -689,13 +656,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi) if (!dsi->enabled) return;

• if (dsi->panel) {

• if (drm_panel_disable(dsi->panel)) {
  

• 	DRM_ERROR("failed to disable the panel\n");
  

• 	return;
  

• }
  

• }
• mtk_dsi_stop(dsi); mtk_dsi_poweroff(dsi);

@@ -750,13 +710,6 @@ static void mtk_dsi_encoder_enable(struct drm_encoder *encoder) mtk_output_dsi_enable(dsi); }

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

• struct mtk_dsi *dsi = connector_to_dsi(connector);
• return drm_panel_get_modes(dsi->panel);

-}

static const struct drm_encoder_helper_funcs mtk_dsi_encoder_helper_funcs = { .mode_fixup = mtk_dsi_encoder_mode_fixup, .mode_set = mtk_dsi_encoder_mode_set, @@ -764,52 +717,7 @@ static const struct drm_encoder_helper_funcs mtk_dsi_encoder_helper_funcs = { .enable = mtk_dsi_encoder_enable, };

-static const struct drm_connector_funcs mtk_dsi_connector_funcs = {

• .dpms = drm_atomic_helper_connector_dpms,
• .fill_modes = drm_helper_probe_single_connector_modes,
• .destroy = drm_connector_cleanup,
• .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

• mtk_dsi_connector_helper_funcs = {
• .get_modes = mtk_dsi_connector_get_modes,

-};

-static int mtk_dsi_create_connector(struct drm_device *drm, struct mtk_dsi *dsi) -{

• int ret;
• ret = drm_connector_init(drm, &dsi->conn, &mtk_dsi_connector_funcs,

• 		 DRM_MODE_CONNECTOR_DSI);
  

• if (ret) {

• DRM_ERROR("Failed to connector init to drm\n");
  

• return ret;
  

• }
• drm_connector_helper_add(&dsi->conn, &mtk_dsi_connector_helper_funcs);
• dsi->conn.dpms = DRM_MODE_DPMS_OFF;
• drm_mode_connector_attach_encoder(&dsi->conn, &dsi->encoder);
• if (dsi->panel) {

• ret = drm_panel_attach(dsi->panel, &dsi->conn);
  

• if (ret) {
  

• 	DRM_ERROR("Failed to attach panel to drm\n");
  

• 	goto err_connector_cleanup;
  

• }
  

• }
• return 0;

-err_connector_cleanup:

• drm_connector_cleanup(&dsi->conn);
• return ret;

-}

-static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi) +static int mtk_dsi_create_enc(struct drm_device *drm, struct mtk_dsi *dsi) { int ret;

@@ -827,15 +735,10 @@ static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi) */ dsi->encoder.possible_crtcs = 1;

• /* If there's a bridge, attach to it and let it create the connector */ ret = drm_bridge_attach(&dsi->encoder, dsi->bridge, NULL); if (ret) { DRM_ERROR("Failed to attach bridge to drm\n");

• /* Otherwise create our own connector and attach to a panel */
  

• ret = mtk_dsi_create_connector(drm, dsi);
  

• if (ret)
  

• 	goto err_encoder_cleanup;
  

• goto err_encoder_cleanup;
  

  }

  return 0;

@@ -848,9 +751,8 @@ static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi) static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi) { drm_encoder_cleanup(&dsi->encoder);

• /* Skip connector cleanup if creation was delegated to the bridge */
• if (dsi->conn.dev)

• drm_connector_cleanup(&dsi->conn);
  

• if (dsi->panel)

• drm_panel_bridge_remove(dsi->bridge);
  

}

static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp) @@ -881,20 +783,12 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host, dsi->format = device->format; dsi->mode_flags = device->mode_flags;

• if (dsi->conn.dev)

• drm_helper_hpd_irq_event(dsi->conn.dev);
  

• return 0;

}

static int mtk_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *device) {

• struct mtk_dsi *dsi = host_to_dsi(host);
• if (dsi->conn.dev)

• drm_helper_hpd_irq_event(dsi->conn.dev);
  

• return 0;

}

@@ -1062,7 +956,7 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) goto err_ddp_comp_unregister; }

• ret = mtk_dsi_create_conn_enc(drm, dsi);

• ret = mtk_dsi_create_enc(drm, dsi); if (ret) { DRM_ERROR("Encoder create failed with %d\n", ret); goto err_unregister;

@@ -1114,6 +1008,13 @@ static int mtk_dsi_probe(struct platform_device *pdev) if (ret) return ret;

• if (dsi->panel) {

• dsi->bridge = drm_panel_bridge_add(dsi->panel,
  

• 				   DRM_MODE_CONNECTOR_DSI);
  

• if (IS_ERR(dsi->bridge))
  

• 	return PTR_ERR(dsi->bridge);
  

• }
• dsi->engine_clk = devm_clk_get(dev, "engine"); if (IS_ERR(dsi->engine_clk)) { ret = PTR_ERR(dsi->engine_clk);