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

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:

quoted

Hi Philipp:

On Wed, 2016-02-03 at 12:01 +0100, Philipp Zabel wrote:

quoted

Hi Daniel,

quoted

quoted

quoted

+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().

quoted

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