From mboxrd@z Thu Jan 1 00:00:00 1970 From: CK Hu Subject: Re: [PATCH v9 03/14] drm/mediatek: Add DSI sub driver Date: Fri, 5 Feb 2016 17:06:26 +0800 Message-ID: <1454663186.16005.2.camel@mtksdaap41> References: <1452611750-16283-1-git-send-email-p.zabel@pengutronix.de> <1452611750-16283-4-git-send-email-p.zabel@pengutronix.de> <1454497304.3867.1.camel@pengutronix.de> <1454567865.12573.7.camel@mtksdaap41> <1454590099.3356.22.camel@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1454590099.3356.22.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Philipp Zabel Cc: Daniel Kurtz , Mark Rutland , Michael Turquette , dri-devel , Dave Airlie , Jie Qiu , Cawa Cheng , Daniel Stone , YT Shen , Yingjoe Chen , "open list:OPEN FIRMWARE AND..." , Jitao Shi , Sasha Hauer , Pawel Moll , Ian Campbell , Rob Herring , "moderated list:ARM/Mediatek SoC support" , Matthias Brugger , Paul Bolle , Stephen Boyd , Tomasz Figa Daniel Vetter List-Id: devicetree@vger.kernel.org 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html