[PATCH 0/3] fix mediatek drm, dis, and disp-* unbind/bind

[PATCH 0/3] fix mediatek drm, dis, and disp-* unbind/bind

[Hsin-Yi Wang]

There are some errors when unbinding and rebinding mediatek drm, dsi,
and disp-* drivers. This series is to fix those errors and warnings.

Hsin-Yi Wang (3):
  drm: mediatek: fix unbind functions
  drm: mediatek: remove clk_unprepare() in mtk_drm_crtc_destroy()
  drm: mediatek: unbind components in mtk_drm_unbind()

 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 ----
 drivers/gpu/drm/mediatek/mtk_drm_drv.c  | 8 +++-----
 drivers/gpu/drm/mediatek/mtk_dsi.c      | 4 +++-
 3 files changed, 6 insertions(+), 10 deletions(-)

-- 
2.20.1

[PATCH 1/3] drm: mediatek: fix unbind functions

[Hsin-Yi Wang]

move mipi_dsi_host_unregister() to .remove since mipi_dsi_host_register()
is called in .probe.

detatch panel in mtk_dsi_destroy_conn_enc(), since .bind will try to
attach it again.

Fixes: 2e54c14e310f ("drm/mediatek: Add DSI sub driver")
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index b00eb2d2e086..c9b6d3a68c8b 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -844,6 +844,8 @@ static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi)
 	/* Skip connector cleanup if creation was delegated to the bridge */
 	if (dsi->conn.dev)
 		drm_connector_cleanup(&dsi->conn);
+	if (dsi->panel)
+		drm_panel_detach(dsi->panel);
 }
 
 static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp)
@@ -1073,7 +1075,6 @@ static void mtk_dsi_unbind(struct device *dev, struct device *master,
 	struct mtk_dsi *dsi = dev_get_drvdata(dev);
 
 	mtk_dsi_destroy_conn_enc(dsi);
-	mipi_dsi_host_unregister(&dsi->host);
 	mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
 }
 
@@ -1179,6 +1180,7 @@ static int mtk_dsi_remove(struct platform_device *pdev)
 
 	mtk_output_dsi_disable(dsi);
 	component_del(&pdev->dev, &mtk_dsi_component_ops);
+	mipi_dsi_host_unregister(&dsi->host);
 
 	return 0;
 }
-- 
2.20.1

Re: [PATCH 1/3] drm: mediatek: fix unbind functions

[CK Hu]

Hi, Hsin-yi:

On Mon, 2019-05-27 at 12:50 +0800, Hsin-Yi Wang wrote:
> move mipi_dsi_host_unregister() to .remove since mipi_dsi_host_register()
> is called in .probe.

In the latest kernel [1], mipi_dsi_host_register() is called in
mtk_dsi_bind(), I think we don't need this part.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mediatek/mtk_dsi.c?h=v5.2-rc2

> 
> detatch panel in mtk_dsi_destroy_conn_enc(), since .bind will try to
> attach it again.
> 
> Fixes: 2e54c14e310f ("drm/mediatek: Add DSI sub driver")
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index b00eb2d2e086..c9b6d3a68c8b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -844,6 +844,8 @@ static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi)
>  	/* Skip connector cleanup if creation was delegated to the bridge */
>  	if (dsi->conn.dev)
>  		drm_connector_cleanup(&dsi->conn);
> +	if (dsi->panel)
> +		drm_panel_detach(dsi->panel);

I think mtk_dsi_destroy_conn_enc() has much thing to do and I would like
you to do more. You could refer to [2] for complete implementation.

[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/exynos/exynos_drm_dsi.c?h=v5.2-rc2#n1575

Regards,
CK

>  }
>  
>  static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp)
> @@ -1073,7 +1075,6 @@ static void mtk_dsi_unbind(struct device *dev, struct device *master,
>  	struct mtk_dsi *dsi = dev_get_drvdata(dev);
>  
>  	mtk_dsi_destroy_conn_enc(dsi);
> -	mipi_dsi_host_unregister(&dsi->host);
>  	mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
>  }
>  
> @@ -1179,6 +1180,7 @@ static int mtk_dsi_remove(struct platform_device *pdev)
>  
>  	mtk_output_dsi_disable(dsi);
>  	component_del(&pdev->dev, &mtk_dsi_component_ops);
> +	mipi_dsi_host_unregister(&dsi->host);
>  
>  	return 0;
>  }

Re: [PATCH 1/3] drm: mediatek: fix unbind functions

[Hsin-Yi Wang]

On Wed, May 29, 2019 at 9:35 AM CK Hu <ck.hu@mediatek.com> wrote:
>
> Hi, Hsin-yi:
>
> On Mon, 2019-05-27 at 12:50 +0800, Hsin-Yi Wang wrote:
> > move mipi_dsi_host_unregister() to .remove since mipi_dsi_host_register()
> > is called in .probe.
>
> In the latest kernel [1], mipi_dsi_host_register() is called in
> mtk_dsi_bind(), I think we don't need this part.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mediatek/mtk_dsi.c?h=v5.2-rc2

This patch https://patchwork.kernel.org/patch/10949305/ moves
mipi_dsi_host_register() from .bind to .probe. I'll reply there to ask
the owner to do this.

>
> >
> > detatch panel in mtk_dsi_destroy_conn_enc(), since .bind will try to
> > attach it again.
> >
> > Fixes: 2e54c14e310f ("drm/mediatek: Add DSI sub driver")
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dsi.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index b00eb2d2e086..c9b6d3a68c8b 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -844,6 +844,8 @@ static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi)
> >       /* Skip connector cleanup if creation was delegated to the bridge */
> >       if (dsi->conn.dev)
> >               drm_connector_cleanup(&dsi->conn);
> > +     if (dsi->panel)
> > +             drm_panel_detach(dsi->panel);
>
> I think mtk_dsi_destroy_conn_enc() has much thing to do and I would like
> you to do more. You could refer to [2] for complete implementation.
>
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/exynos/exynos_drm_dsi.c?h=v5.2-rc2#n1575

Will update in next version.

Thanks

Re: [PATCH 1/3] drm: mediatek: fix unbind functions

[Hsin-Yi Wang]

On Wed, May 29, 2019 at 9:35 AM CK Hu <ck.hu@mediatek.com> wrote:

>
> I think mtk_dsi_destroy_conn_enc() has much thing to do and I would like
> you to do more. You could refer to [2] for complete implementation.
>
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/exynos/exynos_drm_dsi.c?h=v5.2-rc2#n1575
>
Hi CK,

Since drm_encoder_cleanup() would already call drm_bridge_detach() to
detach bridge, I think we only need to handle panel case here.
We don't need to call mtk_dsi_encoder_disable() since
mtk_output_dsi_disable() is called in mtk_dsi_remove() and
dsi->enabled will be set to false. Calling second time will just
returns immediately.
So, besides setting

dsi->panel = NULL;
dsi->conn.status = connector_status_disconnected;

are there other things we need to do here?

Original code doesn't have drm_kms_helper_hotplug_event(), and I'm not
sure if mtk dsi would need this.
Also, mtk_dsi_stop() would also stop irq.

Thanks

Re: [PATCH 1/3] drm: mediatek: fix unbind functions

[CK Hu]

Hi, Hsin-Yi:

On Wed, 2019-05-29 at 15:06 +0800, Hsin-Yi Wang wrote:
> On Wed, May 29, 2019 at 9:35 AM CK Hu <ck.hu@mediatek.com> wrote:
> 
> >
> > I think mtk_dsi_destroy_conn_enc() has much thing to do and I would like
> > you to do more. You could refer to [2] for complete implementation.
> >
> > [2]
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/exynos/exynos_drm_dsi.c?h=v5.2-rc2#n1575
> >
> Hi CK,
> 
> Since drm_encoder_cleanup() would already call drm_bridge_detach() to
> detach bridge, I think we only need to handle panel case here.
> We don't need to call mtk_dsi_encoder_disable() since
> mtk_output_dsi_disable() is called in mtk_dsi_remove() and
> dsi->enabled will be set to false. Calling second time will just
> returns immediately.
> So, besides setting
> 
> dsi->panel = NULL;
> dsi->conn.status = connector_status_disconnected;

Sorry, I think your original patch is good enough, and you need not to
do the besides setting.

Regards,
CK

> 
> are there other things we need to do here?
> 
> Original code doesn't have drm_kms_helper_hotplug_event(), and I'm not
> sure if mtk dsi would need this.
> Also, mtk_dsi_stop() would also stop irq.
> 
> Thanks

[PATCH 2/3] drm: mediatek: remove clk_unprepare() in mtk_drm_crtc_destroy()

[Hsin-Yi Wang]

There is no clk_prepare() called in mtk_drm_crtc_reset(), when unbinding
drm device, mtk_drm_crtc_destroy() will be triggered, and the clocks will
be disabled and unprepared in mtk_crtc_ddp_clk_disable. If clk_unprepare()
is called here, we'll get warnings[1], so remove clk_unprepare() here.

[1]
[   19.416020] mm_disp_ovl0 already unprepared
....
[   19.487536] pstate: 60000005 (nZCv daif -PAN -UAO)
[   19.492325] pc : clk_core_unprepare+0x1d8/0x220
[   19.496851] lr : clk_core_unprepare+0x1d8/0x220
[   19.501373] sp : ffffff8017bbba30
[   19.504681] x29: ffffff8017bbba50 x28: fffffff3f7978000
[   19.509989] x27: 0000000000000000 x26: 0000000000000000
[   19.515298] x25: 0000000044000000 x24: fffffff3f7978000
[   19.520605] x23: 0000000000000060 x22: ffffff9688a89f48
[   19.525912] x21: fffffff3f8755540 x20: 0000000000000000
[   19.531219] x19: fffffff3f9d5ca00 x18: 00000000fffebd18
[   19.536526] x17: 000000000000003c x16: ffffff96881458e4
[   19.541833] x15: 0000000000000005 x14: 706572706e752079
[   19.547140] x13: ffffff80085cc950 x12: 0000000000000000
[   19.552446] x11: 0000000000000000 x10: 0000000000000000
[   19.557754] x9 : 1b0fa21f0ec0d800 x8 : 1b0fa21f0ec0d800
[   19.563060] x7 : 0000000000000000 x6 : ffffff9688b5dd07
[   19.568366] x5 : 0000000000000000 x4 : 0000000000000000
[   19.573673] x3 : 0000000000000000 x2 : fffffff3fffa0248
[   19.578979] x1 : fffffff3fff97a00 x0 : 000000000000001f
[   19.584288] Call trace:
[   19.586734]  clk_core_unprepare+0x1d8/0x220
[   19.590914]  clk_unprepare+0x30/0x40
[   19.594491]  mtk_drm_crtc_destroy+0x30/0x5c
[   19.598672]  drm_mode_config_cleanup+0x124/0x290
[   19.603286]  mtk_drm_unbind+0x44/0x5c
[   19.606946]  take_down_master+0x40/0x54
[   19.610775]  component_master_del+0x70/0x94
[   19.614952]  mtk_drm_remove+0x28/0x44
[   19.618612]  platform_drv_remove+0x28/0x50
[   19.622702]  device_release_driver_internal+0x138/0x1ec
[   19.627921]  device_release_driver+0x24/0x30
[   19.632185]  unbind_store+0x90/0xdc
[   19.635667]  drv_attr_store+0x3c/0x54
[   19.639327]  sysfs_kf_write+0x50/0x68
[   19.642986]  kernfs_fop_write+0x12c/0x1c8
[   19.646997]  __vfs_write+0x54/0x15c
[   19.650482]  vfs_write+0xcc/0x188
[   19.653792]  ksys_write+0x78/0xd8
[   19.657104]  __arm64_sys_write+0x20/0x2c
[   19.661027]  el0_svc_common+0x9c/0xfc
[   19.664686]  el0_svc_compat_handler+0x2c/0x38
[   19.669039]  el0_svc_compat+0x8/0x18
[   19.672609] ---[ end trace 41ce954855cda6f0 ]---

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index acad088173da..c2b38997ac8b 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -98,10 +98,6 @@ static void mtk_drm_finish_page_flip(struct mtk_drm_crtc *mtk_crtc)
 static void mtk_drm_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
-	int i;
-
-	for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
-		clk_unprepare(mtk_crtc->ddp_comp[i]->clk);
 
 	mtk_disp_mutex_put(mtk_crtc->mutex);
 
-- 
2.20.1

Re: [PATCH 2/3] drm: mediatek: remove clk_unprepare() in mtk_drm_crtc_destroy()

[CK Hu]

Hi, Hsin-Yi:

On Mon, 2019-05-27 at 12:50 +0800, Hsin-Yi Wang wrote:
> There is no clk_prepare() called in mtk_drm_crtc_reset(), when unbinding
> drm device, mtk_drm_crtc_destroy() will be triggered, and the clocks will
> be disabled and unprepared in mtk_crtc_ddp_clk_disable. If clk_unprepare()
> is called here, we'll get warnings[1], so remove clk_unprepare() here.

In original code, clk_prepare() is called in mtk_drm_crtc_create() and
clk_unprepare() is called in mtk_drm_crtc_destroy(). This looks correct.
I don't know why we should do any thing about clock in
mtk_drm_crtc_reset(). To debug this, the first step is to print message
when mediatek drm call clk_prepare() and clk_unprepare(). If these two
interface is called in pair, I think we should not modify mediatek drm
driver, the bug maybe in clock driver.

Regards,
CK

> 
> [1]
> [   19.416020] mm_disp_ovl0 already unprepared
> ....
> [   19.487536] pstate: 60000005 (nZCv daif -PAN -UAO)
> [   19.492325] pc : clk_core_unprepare+0x1d8/0x220
> [   19.496851] lr : clk_core_unprepare+0x1d8/0x220
> [   19.501373] sp : ffffff8017bbba30
> [   19.504681] x29: ffffff8017bbba50 x28: fffffff3f7978000
> [   19.509989] x27: 0000000000000000 x26: 0000000000000000
> [   19.515298] x25: 0000000044000000 x24: fffffff3f7978000
> [   19.520605] x23: 0000000000000060 x22: ffffff9688a89f48
> [   19.525912] x21: fffffff3f8755540 x20: 0000000000000000
> [   19.531219] x19: fffffff3f9d5ca00 x18: 00000000fffebd18
> [   19.536526] x17: 000000000000003c x16: ffffff96881458e4
> [   19.541833] x15: 0000000000000005 x14: 706572706e752079
> [   19.547140] x13: ffffff80085cc950 x12: 0000000000000000
> [   19.552446] x11: 0000000000000000 x10: 0000000000000000
> [   19.557754] x9 : 1b0fa21f0ec0d800 x8 : 1b0fa21f0ec0d800
> [   19.563060] x7 : 0000000000000000 x6 : ffffff9688b5dd07
> [   19.568366] x5 : 0000000000000000 x4 : 0000000000000000
> [   19.573673] x3 : 0000000000000000 x2 : fffffff3fffa0248
> [   19.578979] x1 : fffffff3fff97a00 x0 : 000000000000001f
> [   19.584288] Call trace:
> [   19.586734]  clk_core_unprepare+0x1d8/0x220
> [   19.590914]  clk_unprepare+0x30/0x40
> [   19.594491]  mtk_drm_crtc_destroy+0x30/0x5c
> [   19.598672]  drm_mode_config_cleanup+0x124/0x290
> [   19.603286]  mtk_drm_unbind+0x44/0x5c
> [   19.606946]  take_down_master+0x40/0x54
> [   19.610775]  component_master_del+0x70/0x94
> [   19.614952]  mtk_drm_remove+0x28/0x44
> [   19.618612]  platform_drv_remove+0x28/0x50
> [   19.622702]  device_release_driver_internal+0x138/0x1ec
> [   19.627921]  device_release_driver+0x24/0x30
> [   19.632185]  unbind_store+0x90/0xdc
> [   19.635667]  drv_attr_store+0x3c/0x54
> [   19.639327]  sysfs_kf_write+0x50/0x68
> [   19.642986]  kernfs_fop_write+0x12c/0x1c8
> [   19.646997]  __vfs_write+0x54/0x15c
> [   19.650482]  vfs_write+0xcc/0x188
> [   19.653792]  ksys_write+0x78/0xd8
> [   19.657104]  __arm64_sys_write+0x20/0x2c
> [   19.661027]  el0_svc_common+0x9c/0xfc
> [   19.664686]  el0_svc_compat_handler+0x2c/0x38
> [   19.669039]  el0_svc_compat+0x8/0x18
> [   19.672609] ---[ end trace 41ce954855cda6f0 ]---
> 
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index acad088173da..c2b38997ac8b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -98,10 +98,6 @@ static void mtk_drm_finish_page_flip(struct mtk_drm_crtc *mtk_crtc)
>  static void mtk_drm_crtc_destroy(struct drm_crtc *crtc)
>  {
>  	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> -	int i;
> -
> -	for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
> -		clk_unprepare(mtk_crtc->ddp_comp[i]->clk);
>  
>  	mtk_disp_mutex_put(mtk_crtc->mutex);
>  

Re: [PATCH 2/3] drm: mediatek: remove clk_unprepare() in mtk_drm_crtc_destroy()

[Hsin-Yi Wang]

On Wed, May 29, 2019 at 1:58 PM CK Hu <ck.hu@mediatek.com> wrote:
>
> Hi, Hsin-Yi:
>
> On Mon, 2019-05-27 at 12:50 +0800, Hsin-Yi Wang wrote:
> > There is no clk_prepare() called in mtk_drm_crtc_reset(), when unbinding
> > drm device, mtk_drm_crtc_destroy() will be triggered, and the clocks will
> > be disabled and unprepared in mtk_crtc_ddp_clk_disable. If clk_unprepare()
> > is called here, we'll get warnings[1], so remove clk_unprepare() here.
>
> In original code, clk_prepare() is called in mtk_drm_crtc_create() and
> clk_unprepare() is called in mtk_drm_crtc_destroy(). This looks correct.

clk_prepare() is removed in https://patchwork.kernel.org/patch/10872777/.

> I don't know why we should do any thing about clock in
> mtk_drm_crtc_reset(). To debug this, the first step is to print message
> when mediatek drm call clk_prepare() and clk_unprepare(). If these two
> interface is called in pair, I think we should not modify mediatek drm
> driver, the bug maybe in clock driver.
>

Re: [PATCH 2/3] drm: mediatek: remove clk_unprepare() in mtk_drm_crtc_destroy()

[CK Hu]

Hi, Hsin-Yi:

On Wed, 2019-05-29 at 14:08 +0800, Hsin-Yi Wang wrote:
> On Wed, May 29, 2019 at 1:58 PM CK Hu <ck.hu@mediatek.com> wrote:
> >
> > Hi, Hsin-Yi:
> >
> > On Mon, 2019-05-27 at 12:50 +0800, Hsin-Yi Wang wrote:
> > > There is no clk_prepare() called in mtk_drm_crtc_reset(), when unbinding
> > > drm device, mtk_drm_crtc_destroy() will be triggered, and the clocks will
> > > be disabled and unprepared in mtk_crtc_ddp_clk_disable. If clk_unprepare()
> > > is called here, we'll get warnings[1], so remove clk_unprepare() here.
> >
> > In original code, clk_prepare() is called in mtk_drm_crtc_create() and
> > clk_unprepare() is called in mtk_drm_crtc_destroy(). This looks correct.
> 
> clk_prepare() is removed in https://patchwork.kernel.org/patch/10872777/.
> 

I think this patch is a fix of that patch, and I've already applied that
patch, so I merge this patch with that patch in my tree [1], thanks.

[1]
https://github.com/ckhu-mediatek/linux.git-tags/commit/937f861def1a1d49abb92e041efaa5c259281fbf

Regards,
CK

> > I don't know why we should do any thing about clock in
> > mtk_drm_crtc_reset(). To debug this, the first step is to print message
> > when mediatek drm call clk_prepare() and clk_unprepare(). If these two
> > interface is called in pair, I think we should not modify mediatek drm
> > driver, the bug maybe in clock driver.
> >

[PATCH 3/3] drm: mediatek: unbind components in mtk_drm_unbind()

[Hsin-Yi Wang]

Unbinding components (i.e. mtk_dsi and mtk_disp_ovl/rdma/color) will
trigger master(mtk_drm)'s .unbind(), and currently mtk_drm's unbind
won't actually unbind components. During the next bind,
mtk_drm_kms_init() is called, and the components are added back.

.unbind() should call mtk_drm_kms_deinit() to unbind components.

And since component_master_del() in .remove() will trigger .unbind(),
which will also unregister device, it's fine to remove original functions
called here.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 57ce4708ef1b..bbfe3a464aea 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -311,6 +311,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 static void mtk_drm_kms_deinit(struct drm_device *drm)
 {
 	drm_kms_helper_poll_fini(drm);
+	drm_atomic_helper_shutdown(drm);
 
 	component_unbind_all(drm->dev, drm);
 	drm_mode_config_cleanup(drm);
@@ -397,7 +398,9 @@ static void mtk_drm_unbind(struct device *dev)
 	struct mtk_drm_private *private = dev_get_drvdata(dev);
 
 	drm_dev_unregister(private->drm);
+	mtk_drm_kms_deinit(private->drm);
 	drm_dev_put(private->drm);
+	private->num_pipes = 0;
 	private->drm = NULL;
 }
 
@@ -568,13 +571,8 @@ static int mtk_drm_probe(struct platform_device *pdev)
 static int mtk_drm_remove(struct platform_device *pdev)
 {
 	struct mtk_drm_private *private = platform_get_drvdata(pdev);
-	struct drm_device *drm = private->drm;
 	int i;
 
-	drm_dev_unregister(drm);
-	mtk_drm_kms_deinit(drm);
-	drm_dev_put(drm);
-
 	component_master_del(&pdev->dev, &mtk_drm_ops);
 	pm_runtime_disable(&pdev->dev);
 	of_node_put(private->mutex_node);
-- 
2.20.1

Re: [PATCH 3/3] drm: mediatek: unbind components in mtk_drm_unbind()

[CK Hu]

Hi, Hsin-Yi:

On Mon, 2019-05-27 at 12:50 +0800, Hsin-Yi Wang wrote:
> Unbinding components (i.e. mtk_dsi and mtk_disp_ovl/rdma/color) will
> trigger master(mtk_drm)'s .unbind(), and currently mtk_drm's unbind
> won't actually unbind components. During the next bind,
> mtk_drm_kms_init() is called, and the components are added back.
> 
> .unbind() should call mtk_drm_kms_deinit() to unbind components.
> 
> And since component_master_del() in .remove() will trigger .unbind(),
> which will also unregister device, it's fine to remove original functions
> called here.
> 
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 57ce4708ef1b..bbfe3a464aea 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -311,6 +311,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>  static void mtk_drm_kms_deinit(struct drm_device *drm)
>  {
>  	drm_kms_helper_poll_fini(drm);
> +	drm_atomic_helper_shutdown(drm);

This looks not related to this patch. This patch is related to the
unbind timing. You could separate this to an independent patch.

>  
>  	component_unbind_all(drm->dev, drm);
>  	drm_mode_config_cleanup(drm);
> @@ -397,7 +398,9 @@ static void mtk_drm_unbind(struct device *dev)
>  	struct mtk_drm_private *private = dev_get_drvdata(dev);
>  
>  	drm_dev_unregister(private->drm);
> +	mtk_drm_kms_deinit(private->drm);
>  	drm_dev_put(private->drm);
> +	private->num_pipes = 0;

This looks not related to this patch. This patch is related to the
unbind timing. You could separate this to an independent patch.

Regards,
CK

>  	private->drm = NULL;
>  }
>  
> @@ -568,13 +571,8 @@ static int mtk_drm_probe(struct platform_device *pdev)
>  static int mtk_drm_remove(struct platform_device *pdev)
>  {
>  	struct mtk_drm_private *private = platform_get_drvdata(pdev);
> -	struct drm_device *drm = private->drm;
>  	int i;
>  
> -	drm_dev_unregister(drm);
> -	mtk_drm_kms_deinit(drm);
> -	drm_dev_put(drm);
> -
>  	component_master_del(&pdev->dev, &mtk_drm_ops);
>  	pm_runtime_disable(&pdev->dev);
>  	of_node_put(private->mutex_node);