Re: [PATCH v3,2/4] drm/mediatek: Separate poweron/poweroff from enable/disable and define new funcs

Hello Xinlei,

On Thu, 2022-03-17 at 15:53 +0800, xinlei.lee@mediatek.com wrote:

From: Jitao Shi jitao.shi@mediatek.com

In order to match the changes of "Use the drm_panel_bridge API", the poweron/poweroff of dsi is extracted from enable/disable and defined as new funcs (pre_enable/post_disable).

Fixes: 2dd8075d2185 ("drm/mediatek: mtk_dsi: Use the drm_panel_bridge API")

Signed-off-by: Jitao Shi jitao.shi@mediatek.com Signed-off-by: Xinlei Lee xinlei.lee@mediatek.com

---

drivers/gpu/drm/mediatek/mtk_dsi.c | 45 +++++++++++++++++-----------

1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 262c027d8c2f..e33caaca11a7 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -679,16 +679,6 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi) if (--dsi->refcount != 0) return;

• /*

• * mtk_dsi_stop() and mtk_dsi_start() is asymmetric, since
  

• * mtk_dsi_stop() should be called after
  

mtk_drm_crtc_atomic_disable(),

• * which needs irq for vblank, and mtk_dsi_stop() will disable
  

irq.

• * mtk_dsi_start() needs to be called in
  

mtk_output_dsi_enable(),

• * after dsi is fully set.
  

• */
  

• mtk_dsi_stop(dsi);
• mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); mtk_dsi_reset_engine(dsi); mtk_dsi_lane0_ulp_mode_enter(dsi); mtk_dsi_clk_ulp_mode_enter(dsi);

@@ -703,17 +693,9 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)

static void mtk_output_dsi_enable(struct mtk_dsi *dsi) {

• int ret;

• if (dsi->enabled) return;

• ret = mtk_dsi_poweron(dsi);

• if (ret < 0) {

• DRM_ERROR("failed to power on dsi\n");
  

• return;
  

• }

• mtk_dsi_set_mode(dsi); mtk_dsi_clk_hs_mode(dsi, 1);

@@ -727,7 +709,16 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi) if (!dsi->enabled) return;

• mtk_dsi_poweroff(dsi);

• /*

• * mtk_dsi_stop() and mtk_dsi_start() is asymmetric, since
  

Why they are asymmetric?

• * mtk_dsi_stop() should be called after
  

mtk_drm_crtc_atomic_disable(),

• * which needs irq for vblank, and mtk_dsi_stop() will disable
  

irq.

• * mtk_dsi_start() needs to be called in
  

mtk_output_dsi_enable(),

• * after dsi is fully set.
  

• */
  

• mtk_dsi_stop(dsi);

• mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500);

  dsi->enabled = false;

} @@ -765,10 +756,26 @@ static void mtk_dsi_bridge_enable(struct drm_bridge *bridge) mtk_output_dsi_enable(dsi); }

+static void mtk_dsi_bridge_pre_enable(struct drm_bridge *bridge) +{

• struct mtk_dsi *dsi = bridge_to_dsi(bridge);
• mtk_dsi_poweron(dsi);

Should you handle the error of mtk_dsi_poweron? If you failed to mtk_dsi_bridge_pre_enable and do mtk_dsi_bridge_enable, what will happend?

+}

+static void mtk_dsi_bridge_post_disable(struct drm_bridge *bridge) +{

• struct mtk_dsi *dsi = bridge_to_dsi(bridge);
• mtk_dsi_poweroff(dsi);

If you failed to mtk_dsi_bridge_disable and you do mtk_dsi_bridge_post_disable, what will happend? Do you need to handle this?

BRs, Rex

+}

static const struct drm_bridge_funcs mtk_dsi_bridge_funcs = { .attach = mtk_dsi_bridge_attach, .disable = mtk_dsi_bridge_disable, .enable = mtk_dsi_bridge_enable,

• .pre_enable = mtk_dsi_bridge_pre_enable,
• .post_disable = mtk_dsi_bridge_post_disable, .mode_set = mtk_dsi_bridge_mode_set,

};