Hi, Philipp: On Thu, 2016-02-04 at 13:48 +0100, Philipp Zabel wrote: > Am Donnerstag, den 04.02.2016, 14:37 +0800 schrieb CK Hu: > > Hi Philipp: > > > > On Wed, 2016-02-03 at 12:01 +0100, Philipp Zabel wrote: > > > Hi Daniel, > > > > > > > > > > +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); > > > > > > > > The order is a bit suspicious here; I would expect to poweroff dsi > > > > before the panel to mirror the turn on order. > > > > > > CK, could you comment on this? > > > > > > > According to the experience of other Mediatek SoC, > > In mtk_output_dsi_enable(), we should do power on dsi first and then > > prepare panel because dsi should be ready to receive panel prepare error > > message. So we should disable panel and then power off dsi in > > mtk_output_dsi_disable(). > > > > > I can reorder this, but I'm not sure about the reasoning (what happens > > > hardware wise if we just cut panel power vs. if the DSI panel first sees > > > the ULP transition). Further, I don't have a panel to test, just the > > > PS8640. > > > > > > thanks > > > Philipp > > I just realized that this code isn't even using drm_panel_enable and > drm_panel_unprepare. I suppose the order generally should be: > > prepare and enable dsi (but don't start stream yet) > drm_panel_prepare() > enable dsi output > drm_panel_enable() > > and to disable: > > drm_panel_disable() > disable dsi output > drm_panel_unprepare() > power off dsi > > ? > I think the flow you suppose is ok and more general. > regards > Philipp > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html