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

Hi, Xinlei:

On Mon, 2022-04-11 at 10:31 +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 | 51 +++++++++++++++++++---------

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

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 262c027d8c2f..cf76c53a1af6 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
  

• * 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;

} @@ -762,13 +753,35 @@ static void mtk_dsi_bridge_enable(struct drm_bridge *bridge) { struct mtk_dsi *dsi = bridge_to_dsi(bridge);

• if (dsi->refcount == 0)

• return;
  

• mtk_output_dsi_enable(dsi);

}

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

• struct mtk_dsi *dsi = bridge_to_dsi(bridge);
• int ret;
• ret = mtk_dsi_poweron(dsi);
• if (ret < 0)

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

+}

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

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

+}

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,

The flow looks good to me, but according to [1], pre_enable is deprecated. Use atomic_pre_enable instead.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/incl...

• .post_disable = mtk_dsi_bridge_post_disable,

Ditto.

Regards, CK

.mode_set = mtk_dsi_bridge_mode_set, };