Re: [PATCH v9 03/14] drm/mediatek: Add DSI sub driver

CK Hu
2016-02-05T09:06:26Z

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