Hi Philipp, CK,

Some small comments.
Nothing that couldn't be addressed after merging, if you prefer.

On Tue, Mar 8, 2016 at 9:27 PM, Philipp Zabel [off-list ref] wrote:

What is the point of this refcount?
I believe dsi->enabled already ensures poweron/poweroff calls are paired.

+
+       /**
+        * data_rate = (pixel_clock / 1000) * pixel_dipth * mipi_ratio;
+        * pixel_clock unit is Khz, data_rata unit is MHz, so need divide 1000.
+        * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
+        * we set mipi_ratio is 1.05.
+        */
+       dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
+
+       ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
+       if (ret < 0) {
+               dev_err(dev, "Failed to set data rate: %d\n", ret);
+               goto err_refcount;
+       }
+
+       phy_power_on(dsi->phy);
+
+       ret = clk_prepare_enable(dsi->engine_clk);
+       if (ret < 0) {
+               dev_err(dev, "Failed to enable engine clock: %d\n", ret);
+               goto err_phy_power_off;
+       }
+
+       ret = clk_prepare_enable(dsi->digital_clk);
+       if (ret < 0) {
+               dev_err(dev, "Failed to enable digital clock: %d\n", ret);
+               goto err_disable_engine_clk;
+       }
+
+       mtk_dsi_enable(dsi);
+       mtk_dsi_reset(dsi);
+       dsi_phy_timconfig(dsi);
+
+       return 0;
+
+err_disable_engine_clk:
+       clk_disable_unprepare(dsi->engine_clk);
+err_phy_power_off:
+       phy_power_off(dsi->phy);
+err_refcount:
+       dsi->refcount--;
+       return ret;
+}
+
+static void dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
+{
+       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
+       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
+}
+
+static void dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
+{
+       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
+       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_WAKEUP_EN, LC_WAKEUP_EN);
+       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_WAKEUP_EN, 0);
+}
+
+static void dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
+{
+       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_HS_TX_EN, 0);
+       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
+}
+
+static void dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
+{
+       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
+       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_WAKEUP_EN, LD0_WAKEUP_EN);
+       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_WAKEUP_EN, 0);
+}
+
+static bool dsi_clk_hs_state(struct mtk_dsi *dsi)
+{
+       u32 tmp_reg1;
+
+       tmp_reg1 = readl(dsi->regs + DSI_PHY_LCCON);
+       return ((tmp_reg1 & LC_HS_TX_EN) == 1) ? true : false;
+}
+
+static void dsi_clk_hs_mode(struct mtk_dsi *dsi, bool enter)
+{
+       if (enter && !dsi_clk_hs_state(dsi))
+               mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, LC_HS_TX_EN);
+       else if (!enter && dsi_clk_hs_state(dsi))
+               mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
+}
+
+static void dsi_set_mode(struct mtk_dsi *dsi)
+{
+       u32 vid_mode = CMD_MODE;
+
+       if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
+               vid_mode = SYNC_PULSE_MODE;
+
+               if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
+                   !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
+                       vid_mode = BURST_MODE;
+       }
+
+       writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
+}
+
+static void dsi_ps_control_vact(struct mtk_dsi *dsi)
+{
+       struct videomode *vm = &dsi->vm;
+       u32 dsi_buf_bpp, ps_wc;
+       u32 ps_bpp_mode;
+
+       if (dsi->format == MIPI_DSI_FMT_RGB565)
+               dsi_buf_bpp = 2;
+       else
+               dsi_buf_bpp = 3;
+
+       ps_wc = vm->hactive * dsi_buf_bpp;
+       ps_bpp_mode = ps_wc;
+
+       switch (dsi->format) {
+       case MIPI_DSI_FMT_RGB888:
+               ps_bpp_mode |= PACKED_PS_24BIT_RGB888;
+               break;
+       case MIPI_DSI_FMT_RGB666:
+               ps_bpp_mode |= PACKED_PS_18BIT_RGB666;
+               break;
+       case MIPI_DSI_FMT_RGB666_PACKED:
+               ps_bpp_mode |= LOOSELY_PS_18BIT_RGB666;
+               break;
+       case MIPI_DSI_FMT_RGB565:
+               ps_bpp_mode |= PACKED_PS_16BIT_RGB565;
+               break;
+       }
+
+       writel(vm->vactive, dsi->regs + DSI_VACT_NL);
+       writel(ps_bpp_mode, dsi->regs + DSI_PSCTRL);
+       writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
+}
+
+static void dsi_rxtx_control(struct mtk_dsi *dsi)
+{
+       u32 tmp_reg;
+
+       switch (dsi->lanes) {
+       case 1:
+               tmp_reg = 1 << 2;
+               break;
+       case 2:
+               tmp_reg = 3 << 2;
+               break;
+       case 3:
+               tmp_reg = 7 << 2;
+               break;
+       case 4:
+               tmp_reg = 0xf << 2;
+               break;
+       default:
+               tmp_reg = 0xf << 2;
+               break;
+       }
+
+       writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
+}
+
+static void dsi_ps_control(struct mtk_dsi *dsi)
+{
+       unsigned int dsi_tmp_buf_bpp;
+       u32 tmp_reg;
+
+       switch (dsi->format) {
+       case MIPI_DSI_FMT_RGB888:
+               tmp_reg = PACKED_PS_24BIT_RGB888;
+               dsi_tmp_buf_bpp = 3;
+               break;
+       case MIPI_DSI_FMT_RGB666:
+               tmp_reg = LOOSELY_PS_18BIT_RGB666;
+               dsi_tmp_buf_bpp = 3;
+               break;
+       case MIPI_DSI_FMT_RGB666_PACKED:
+               tmp_reg = PACKED_PS_18BIT_RGB666;
+               dsi_tmp_buf_bpp = 3;
+               break;
+       case MIPI_DSI_FMT_RGB565:
+               tmp_reg = PACKED_PS_16BIT_RGB565;
+               dsi_tmp_buf_bpp = 2;
+               break;
+       default:
+               tmp_reg = PACKED_PS_24BIT_RGB888;
+               dsi_tmp_buf_bpp = 3;
+               break;
+       }
+
+       tmp_reg += dsi->vm.hactive * dsi_tmp_buf_bpp & DSI_PS_WC;
+       writel(tmp_reg, dsi->regs + DSI_PSCTRL);
+}
+
+static void dsi_config_vdo_timing(struct mtk_dsi *dsi)
+{
+       unsigned int horizontal_sync_active_byte;
+       unsigned int horizontal_backporch_byte;
+       unsigned int horizontal_frontporch_byte;
+       unsigned int dsi_tmp_buf_bpp;
+
+       struct videomode *vm = &dsi->vm;
+
+       if (dsi->format == MIPI_DSI_FMT_RGB565)
+               dsi_tmp_buf_bpp = 2;
+       else
+               dsi_tmp_buf_bpp = 3;
+
+       writel(vm->vsync_len, dsi->regs + DSI_VSA_NL);
+       writel(vm->vback_porch, dsi->regs + DSI_VBP_NL);
+       writel(vm->vfront_porch, dsi->regs + DSI_VFP_NL);
+       writel(vm->vactive, dsi->regs + DSI_VACT_NL);
+
+       horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10);
+
+       if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
+               horizontal_backporch_byte =
+                       (vm->hback_porch * dsi_tmp_buf_bpp - 10);
+       else
+               horizontal_backporch_byte = ((vm->hback_porch + vm->hsync_len) *
+                       dsi_tmp_buf_bpp - 10);
+
+       horizontal_frontporch_byte = (vm->hfront_porch * dsi_tmp_buf_bpp - 12);
+
+       writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC);
+       writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
+       writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
+
+       dsi_ps_control(dsi);
+}
+
+static void mtk_dsi_start(struct mtk_dsi *dsi)
+{
+       writel(0, dsi->regs + DSI_START);
+       writel(1, dsi->regs + DSI_START);
+}
+
+static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
+{
+       if (WARN_ON(dsi->refcount == 0))
+               return;
+
+       if (--dsi->refcount != 0)
+               return;
+
+       dsi_lane0_ulp_mode_enter(dsi);
+       dsi_clk_ulp_mode_enter(dsi);
+
+       mtk_dsi_disable(dsi);
+
+       clk_disable_unprepare(dsi->engine_clk);
+       clk_disable_unprepare(dsi->digital_clk);
+
+       phy_power_off(dsi->phy);
+}
+
+static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
+{
+       int ret;
+
+       if (dsi->enabled)
+               return;
+
+       if (dsi->panel) {
+               if (drm_panel_prepare(dsi->panel)) {
+                       DRM_ERROR("failed to setup the panel\n");
+                       return;
+               }
+       }
+
+       ret = mtk_dsi_poweron(dsi);
+       if (ret < 0) {
+               DRM_ERROR("failed to power on dsi\n");
+               return;
+       }
+
+       dsi_rxtx_control(dsi);
+
+       dsi_clk_ulp_mode_leave(dsi);
+       dsi_lane0_ulp_mode_leave(dsi);
+       dsi_clk_hs_mode(dsi, 0);
+       dsi_set_mode(dsi);
+
+       dsi_ps_control_vact(dsi);
+       dsi_config_vdo_timing(dsi);
+
+       dsi_set_mode(dsi);
+       dsi_clk_hs_mode(dsi, 1);
+
+       mtk_dsi_start(dsi);
+
+       dsi->enabled = true;
+}
+
+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_poweroff(dsi);
+
+       dsi->enabled = false;
+}
+
+static void mtk_dsi_encoder_destroy(struct drm_encoder *encoder)
+{
+       drm_encoder_cleanup(encoder);
+}
+
+static const struct drm_encoder_funcs mtk_dsi_encoder_funcs = {
+       .destroy = mtk_dsi_encoder_destroy,
+};
+
+static bool mtk_dsi_encoder_mode_fixup(struct drm_encoder *encoder,
+                                      const struct drm_display_mode *mode,
+                                      struct drm_display_mode *adjusted_mode)
+{
+       return true;
+}
+
+static void mtk_dsi_encoder_mode_set(struct drm_encoder *encoder,
+                                    struct drm_display_mode *mode,
+                                    struct drm_display_mode *adjusted)
+{
+       struct mtk_dsi *dsi = encoder_to_dsi(encoder);
+
+       dsi->vm.pixelclock = adjusted->clock;
+       dsi->vm.hactive = adjusted->hdisplay;
+       dsi->vm.hback_porch = adjusted->htotal - adjusted->hsync_end;
+       dsi->vm.hfront_porch = adjusted->hsync_start - adjusted->hdisplay;
+       dsi->vm.hsync_len = adjusted->hsync_end - adjusted->hsync_start;
+
+       dsi->vm.vactive = adjusted->vdisplay;
+       dsi->vm.vback_porch = adjusted->vtotal - adjusted->vsync_end;
+       dsi->vm.vfront_porch = adjusted->vsync_start - adjusted->vdisplay;
+       dsi->vm.vsync_len = adjusted->vsync_end - adjusted->vsync_start;
+}
+
+static void mtk_dsi_encoder_disable(struct drm_encoder *encoder)
+{
+       struct mtk_dsi *dsi = encoder_to_dsi(encoder);
+
+       mtk_output_dsi_disable(dsi);
+}
+
+static void mtk_dsi_encoder_enable(struct drm_encoder *encoder)
+{
+       struct mtk_dsi *dsi = encoder_to_dsi(encoder);
+
+       mtk_output_dsi_enable(dsi);
+}
+
+static enum drm_connector_status mtk_dsi_connector_detect(
+       struct drm_connector *connector, bool force)
+{
+       return connector_status_connected;
+}
+
+static void mtk_dsi_connector_destroy(struct drm_connector *connector)
+{
+       drm_connector_unregister(connector);
+       drm_connector_cleanup(connector);
+}
+
+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 struct drm_encoder *mtk_dsi_connector_best_encoder(
+               struct drm_connector *connector)
+{
+       struct mtk_dsi *dsi = connector_to_dsi(connector);
+
+       return &dsi->encoder;
+}
+
+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,
+       .disable = mtk_dsi_encoder_disable,
+       .enable = mtk_dsi_encoder_enable,
+};
+
+static const struct drm_connector_funcs mtk_dsi_connector_funcs = {
+       .dpms = drm_atomic_helper_connector_dpms,
+       .detect = mtk_dsi_connector_detect,
+       .fill_modes = drm_helper_probe_single_connector_modes,
+       .destroy = mtk_dsi_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
+       mtk_dsi_connector_helper_funcs = {
+       .get_modes = mtk_dsi_connector_get_modes,
+       .best_encoder = mtk_dsi_connector_best_encoder,
+};
+
+static int mtk_drm_attach_bridge(struct drm_bridge *bridge,
+                                struct drm_encoder *encoder)
+{
+       int ret;
+
+       if (!bridge)
+               return -ENOENT;
+
+       encoder->bridge = bridge;
+       bridge->encoder = encoder;
+       ret = drm_bridge_attach(encoder->dev, bridge);
+       if (ret) {
+               DRM_ERROR("Failed to attach bridge to drm\n");
+               encoder->bridge = NULL;
+               bridge->encoder = NULL;
+       }
+
+       return ret;
+}
+
+static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi)
+{
+       int ret;
+
+       ret = drm_encoder_init(drm, &dsi->encoder, &mtk_dsi_encoder_funcs,
+                              DRM_MODE_ENCODER_DSI, NULL);
+       if (ret) {
+               DRM_ERROR("Failed to encoder init to drm\n");
+               return ret;
+       }
+       drm_encoder_helper_add(&dsi->encoder, &mtk_dsi_encoder_helper_funcs);
+
+       /*
+        * Currently display data paths are statically assigned to a crtc each.
+        * crtc 0 is OVL0 -> COLOR0 -> AAL -> OD -> RDMA0 -> UFOE -> DSI0
+        */
+       dsi->encoder.possible_crtcs = 1;
+
+       /* Pre-empt DP connector creation if there's a bridge */
+       ret = mtk_drm_attach_bridge(dsi->bridge, &dsi->encoder);
+       if (!ret)
+               return 0;

nit: the above valid early termination of this function here is a bit unusual.
It might be more clear if the "connector init" part below was split out into its
own helper function.

+
+       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");
+               goto err_encoder_cleanup;
+       }
+
+       drm_connector_helper_add(&dsi->conn, &mtk_dsi_connector_helper_funcs);
+
+       ret = drm_connector_register(&dsi->conn);
+       if (ret) {
+               DRM_ERROR("Failed to connector register to drm\n");
+               goto err_connector_cleanup;
+       }
+
+       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_unregister;
+               }
+       }
+       return 0;
+
+err_connector_unregister:
+       drm_connector_unregister(&dsi->conn);
+err_connector_cleanup:
+       drm_connector_cleanup(&dsi->conn);
+err_encoder_cleanup:
+       drm_encoder_cleanup(&dsi->encoder);
+
+       return ret;
+}
+
+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_unregister(&dsi->conn);
+               drm_connector_cleanup(&dsi->conn);
+       }
+}
+
+static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp)
+{
+       struct mtk_dsi *dsi = container_of(comp, struct mtk_dsi, ddp_comp);
+
+       mtk_dsi_poweron(dsi);
+}
+
+static void mtk_dsi_ddp_stop(struct mtk_ddp_comp *comp)
+{
+       struct mtk_dsi *dsi = container_of(comp, struct mtk_dsi, ddp_comp);
+
+       mtk_dsi_poweroff(dsi);
+}
+
+static const struct mtk_ddp_comp_funcs mtk_dsi_funcs = {
+       .start = mtk_dsi_ddp_start,
+       .stop = mtk_dsi_ddp_stop,
+};
+
+static int mtk_dsi_host_attach(struct mipi_dsi_host *host,
+                              struct mipi_dsi_device *device)
+{
+       struct mtk_dsi *dsi = host_to_dsi(host);
+
+       dsi->lanes = device->lanes;
+       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;
+}
+
+static const struct mipi_dsi_host_ops mtk_dsi_ops = {
+       .attach = mtk_dsi_host_attach,
+       .detach = mtk_dsi_host_detach,
+};
+
+static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
+{
+       int ret;
+       struct drm_device *drm = data;
+       struct mtk_dsi *dsi = dev_get_drvdata(dev);
+
+       ret = mtk_ddp_comp_register(drm, &dsi->ddp_comp);
+       if (ret < 0) {
+               dev_err(dev, "Failed to register component %s: %d\n",
+                       dev->of_node->full_name, ret);
+               return ret;
+       }
+
+       ret = mipi_dsi_host_register(&dsi->host);
+       if (ret < 0) {
+               dev_err(dev, "failed to register DSI host: %d\n", ret);
+               goto err_ddp_comp_unregister;
+       }
+
+       ret = mtk_dsi_create_conn_enc(drm, dsi);
+       if (ret) {
+               DRM_ERROR("Encoder create failed with %d\n", ret);
+               goto err_unregister;
+       }
+
+       return 0;
+
+err_unregister:
+       mipi_dsi_host_unregister(&dsi->host);
+err_ddp_comp_unregister:
+       mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
+       return ret;
+}
+
+static void mtk_dsi_unbind(struct device *dev, struct device *master,
+                          void *data)
+{
+       struct drm_device *drm = data;
+       struct mtk_dsi *dsi = dev_get_drvdata(dev);
+
+       mtk_dsi_destroy_conn_enc(dsi);
+       mipi_dsi_host_unregister(&dsi->host);
+       mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
+}
+
+static const struct component_ops mtk_dsi_component_ops = {
+       .bind = mtk_dsi_bind,
+       .unbind = mtk_dsi_unbind,
+};
+
+static int mtk_dsi_probe(struct platform_device *pdev)
+{
+       struct mtk_dsi *dsi;
+       struct device *dev = &pdev->dev;
+       struct device_node *remote_node, *endpoint;
+       struct resource *regs;
+       int comp_id;
+       int ret;
+
+       dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
+       if (!dsi)
+               return -ENOMEM;
+
+       dsi->host.ops = &mtk_dsi_ops;
+       dsi->host.dev = dev;
+
+       endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
+       if (endpoint) {
+               remote_node = of_graph_get_remote_port_parent(endpoint);
+               if (!remote_node) {
+                       dev_err(dev, "No panel connected\n");
+                       return -ENODEV;
+               }
+
+               dsi->bridge = of_drm_find_bridge(remote_node);
+               dsi->panel = of_drm_find_panel(remote_node);
+               of_node_put(remote_node);
+               if (!dsi->bridge && !dsi->panel) {
+                       dev_info(dev, "Waiting for bridge or panel driver\n");
+                       return -EPROBE_DEFER;
+               }
+       }
+
+       dsi->engine_clk = devm_clk_get(dev, "engine");
+       if (IS_ERR(dsi->engine_clk)) {
+               ret = PTR_ERR(dsi->engine_clk);
+               dev_err(dev, "Failed to get engine clock: %d\n", ret);
+               return ret;
+       }
+
+       dsi->digital_clk = devm_clk_get(dev, "digital");
+       if (IS_ERR(dsi->digital_clk)) {
+               ret = PTR_ERR(dsi->digital_clk);
+               dev_err(dev, "Failed to get digital clock: %d\n", ret);
+               return ret;
+       }
+
+       dsi->hs_clk = devm_clk_get(dev, "hs");
+       if (IS_ERR(dsi->hs_clk)) {
+               ret = PTR_ERR(dsi->hs_clk);
+               dev_err(dev, "Failed to get hs clock: %d\n", ret);
+               return ret;
+       }
+
+       regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       dsi->regs = devm_ioremap_resource(dev, regs);
+       if (IS_ERR(dsi->regs)) {
+               ret = PTR_ERR(dsi->regs);
+               dev_err(dev, "Failed to ioremap memory: %d\n", ret);
+               return ret;
+       }
+
+       dsi->phy = devm_phy_get(dev, "dphy");
+       if (IS_ERR(dsi->phy)) {
+               ret = PTR_ERR(dsi->phy);
+               dev_err(dev, "Failed to get MIPI-DPHY: %d\n", ret);
+               return ret;
+       }
+
+       comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DSI);
+       if (comp_id < 0) {
+               dev_err(dev, "Failed to identify by alias: %d\n", comp_id);
+               return comp_id;
+       }
+
+       ret = mtk_ddp_comp_init(dev, dev->of_node, &dsi->ddp_comp, comp_id,
+                               &mtk_dsi_funcs);
+       if (ret) {
+               dev_err(dev, "Failed to initialize component: %d\n", ret);
+               return ret;
+       }
+
+       platform_set_drvdata(pdev, dsi);
+
+       return component_add(&pdev->dev, &mtk_dsi_component_ops);
+}
+
+static int mtk_dsi_remove(struct platform_device *pdev)
+{
+       struct mtk_dsi *dsi = platform_get_drvdata(pdev);
+
+       mtk_output_dsi_disable(dsi);

This one looks unmatched.

+       component_del(&pdev->dev, &mtk_dsi_component_ops);
+
+       return 0;
+}
+
+#ifdef CONFIG_PM
+static int mtk_dsi_suspend(struct device *dev)
+{
+       struct mtk_dsi *dsi;
+
+       dsi = dev_get_drvdata(dev);
+
+       mtk_output_dsi_disable(dsi);
+       DRM_DEBUG_DRIVER("dsi suspend success!\n");
+
+       return 0;
+}
+
+static int mtk_dsi_resume(struct device *dev)
+{
+       struct mtk_dsi *dsi;
+
+       dsi = dev_get_drvdata(dev);
+
+       mtk_output_dsi_enable(dsi);
+       DRM_DEBUG_DRIVER("dsi resume success!\n");
+
+       return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(mtk_dsi_pm_ops, mtk_dsi_suspend, mtk_dsi_resume);

I don't think we want PM ops.
The MTK DRM driver disables/enables encoders during suspend/resume.

An inline function / macro would help here.

+       return mipi_tx->data_rate;
+}
+
+static const struct clk_ops mtk_mipi_tx_pll_ops = {
+       .prepare = mtk_mipi_tx_pll_prepare,
+       .unprepare = mtk_mipi_tx_pll_unprepare,
+       .round_rate = mtk_mipi_tx_pll_round_rate,
+       .set_rate = mtk_mipi_tx_pll_set_rate,
+       .recalc_rate = mtk_mipi_tx_pll_recalc_rate,
+};
+
+static int mtk_mipi_tx_power_on_signal(struct phy *phy)
+{
+       struct mtk_mipi_tx *mipi_tx = phy_get_drvdata(phy);
+
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_CLOCK_LANE,
+                        RG_DSI_LNTC_LDOOUT_EN, RG_DSI_LNTC_LDOOUT_EN);
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_DATA_LANE0,
+                        RG_DSI_LNT0_LDOOUT_EN, RG_DSI_LNT0_LDOOUT_EN);
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_DATA_LANE1,
+                        RG_DSI_LNT1_LDOOUT_EN, RG_DSI_LNT1_LDOOUT_EN);
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_DATA_LANE2,
+                        RG_DSI_LNT2_LDOOUT_EN, RG_DSI_LNT2_LDOOUT_EN);
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_DATA_LANE3,
+                        RG_DSI_LNT3_LDOOUT_EN, RG_DSI_LNT3_LDOOUT_EN);
+
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_TOP_CON, RG_DSI_PAD_TIE_LOW_EN, 0);
+
+       return 0;
+}
+
+static int mtk_mipi_tx_power_on(struct phy *phy)
+{
+       struct mtk_mipi_tx *mipi_tx = phy_get_drvdata(phy);
+       int ret;
+
+       /* Power up core and enable PLL */
+       ret = clk_prepare_enable(mipi_tx->pll);
+       if (ret < 0)
+               return ret;
+
+       /* Enable DSI Lane LDO outputs, disable pad tie low */
+       mtk_mipi_tx_power_on_signal(phy);
+
+       return 0;
+}
+
+static void mtk_mipi_tx_power_off_signal(struct phy *phy)
+{
+       struct mtk_mipi_tx *mipi_tx = phy_get_drvdata(phy);
+
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_TOP_CON, RG_DSI_PAD_TIE_LOW_EN,
+                        RG_DSI_PAD_TIE_LOW_EN);

As mentioned in the HDMI review, _set_bits() / _clr_bits() is more readable than
_mask() if we are just setting/clearing groups of bits.

+
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_CLOCK_LANE,
+                        RG_DSI_LNTC_LDOOUT_EN, 0);
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_DATA_LANE0,
+                        RG_DSI_LNT0_LDOOUT_EN, 0);
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_DATA_LANE1,
+                        RG_DSI_LNT1_LDOOUT_EN, 0);
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_DATA_LANE2,
+                        RG_DSI_LNT2_LDOOUT_EN, 0);
+       mtk_mipi_tx_mask(mipi_tx, MIPITX_DSI_DATA_LANE3,
+                        RG_DSI_LNT3_LDOOUT_EN, 0);
+}
+
+static int mtk_mipi_tx_power_off(struct phy *phy)
+{
+       struct mtk_mipi_tx *mipi_tx = phy_get_drvdata(phy);
+
+       /* Enable pad tie low, disable DSI Lane LDO outputs */
+       mtk_mipi_tx_power_off_signal(phy);
+
+       /* Disable PLL and power down core */
+       clk_disable_unprepare(mipi_tx->pll);
+
+       return 0;
+}
+
+static const struct phy_ops mtk_mipi_tx_ops = {
+       .power_on = mtk_mipi_tx_power_on,
+       .power_off = mtk_mipi_tx_power_off,
+       .owner = THIS_MODULE,
+};
+
+static int mtk_mipi_tx_probe(struct platform_device *pdev)
+{
+       struct device *dev = &pdev->dev;
+       struct mtk_mipi_tx *mipi_tx;
+       struct resource *mem;
+       struct clk *ref_clk;
+       const char *ref_clk_name;
+       struct clk_init_data clk_init = {
+               .ops = &mtk_mipi_tx_pll_ops,
+               .num_parents = 1,
+               .parent_names = (const char * const *)&ref_clk_name,
+               .flags = CLK_SET_RATE_GATE,
+       };
+       struct phy *phy;
+       struct phy_provider *phy_provider;
+       int ret;
+
+       mipi_tx = devm_kzalloc(dev, sizeof(*mipi_tx), GFP_KERNEL);
+       if (!mipi_tx)
+               return -ENOMEM;
+
+       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       mipi_tx->regs = devm_ioremap_resource(dev, mem);
+       if (IS_ERR(mipi_tx->regs)) {
+               ret = PTR_ERR(mipi_tx->regs);
+               dev_err(dev, "Failed to get memory resource: %d\n", ret);
+               return ret;
+       }
+
+       ref_clk = devm_clk_get(dev, NULL);
+       if (IS_ERR(ref_clk)) {
+               ret = PTR_ERR(ref_clk);
+               dev_err(dev, "Failed to get reference clock: %d\n", ret);
+               return ret;
+       }
+       ref_clk_name = __clk_get_name(ref_clk);
+
+       ret = of_property_read_string(dev->of_node, "clock-output-names",
+                                     &clk_init.name);
+       if (ret < 0) {
+               dev_err(dev, "Failed to read clock-output-names: %d\n", ret);
+               return ret;
+       }
+
+       mipi_tx->pll_hw.init = &clk_init;
+       mipi_tx->pll = devm_clk_register(dev, &mipi_tx->pll_hw);
+       if (IS_ERR(mipi_tx->pll)) {
+               ret = PTR_ERR(mipi_tx->pll);
+               dev_err(dev, "Failed to register PLL: %d\n", ret);
+               return ret;
+       }
+
+       phy = devm_phy_create(dev, NULL, &mtk_mipi_tx_ops);
+       if (IS_ERR(phy)) {
+               ret = PTR_ERR(phy);
+               dev_err(dev, "Failed to create MIPI D-PHY: %d\n", ret);
+               return ret;
+       }
+       phy_set_drvdata(phy, mipi_tx);
+
+       phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+       if (IS_ERR(phy)) {
+               ret = PTR_ERR(phy_provider);
+               return ret;
+       }
+
+       mipi_tx->dev = dev;
+
+       return of_clk_add_provider(dev->of_node, of_clk_src_simple_get,
+                                  mipi_tx->pll);
+}
+
+static int mtk_mipi_tx_remove(struct platform_device *pdev)
+{
+       of_clk_del_provider(pdev->dev.of_node);
+       return 0;
+}
+
+static const struct of_device_id mtk_mipi_tx_match[] = {
+       { .compatible = "mediatek,mt8173-mipi-tx", },
+       {},
+};
+
+struct platform_driver mtk_mipi_tx_driver = {
+       .probe = mtk_mipi_tx_probe,
+       .remove = mtk_mipi_tx_remove,
+       .driver = {
+               .name = "mediatek-mipi-tx",
+               .of_match_table = mtk_mipi_tx_match,
+       },
+};
--
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html