diff mbox series

[v3] drm/mediatek: add dma buffer control for drm plane disable

Message ID 20230320030449.5397-1-yongqiang.niu@mediatek.com
State Superseded
Headers show
Series [v3] drm/mediatek: add dma buffer control for drm plane disable | expand

Commit Message

Yongqiang Niu March 20, 2023, 3:04 a.m. UTC
Fixes: 41016fe1028e4 (drm: Rename plane->state variables in atomic update and disable)
dma buffer release before overlay disable, that will cause
m4u translation fault warning.

add dma buffer control flow in mediatek driver:
get dma buffer when drm plane disable
put dma buffer when overlay really disable

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c  | 25 ++++++++++++++++++++++++
 drivers/gpu/drm/mediatek/mtk_drm_plane.c | 17 ++++++++++++++++
 drivers/gpu/drm/mediatek/mtk_drm_plane.h |  1 +
 3 files changed, 43 insertions(+)

Comments

Chun-Kuang Hu March 27, 2023, 3:32 p.m. UTC | #1
Hi, Yongqiang:

Yongqiang Niu <yongqiang.niu@mediatek.com> 於 2023年3月20日 週一 上午11:05寫道:
>
> Fixes: 41016fe1028e4 (drm: Rename plane->state variables in atomic update and disable)

[1] has introduction how to add Fixes tag, one information is:

please use the ‘Fixes:’ tag with the first 12 characters of the SHA-1
ID, and the one line summary. Do not split the tag across multiple
lines, tags are exempt from the “wrap at 75 columns” rule in order to
simplify parsing scripts.

And move this tag to the line before your sign-off tag.

[1] https://www.kernel.org/doc/html/v6.2/process/submitting-patches.html


> dma buffer release before overlay disable, that will cause
> m4u translation fault warning.
>
> add dma buffer control flow in mediatek driver:
> get dma buffer when drm plane disable
> put dma buffer when overlay really disable
>
> Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c  | 25 ++++++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c | 17 ++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_drm_plane.h |  1 +
>  3 files changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 5071f1263216..9cf1c1778868 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -4,6 +4,7 @@
>   */
>
>  #include <linux/clk.h>
> +#include <linux/dma-buf.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/mailbox_controller.h>
>  #include <linux/pm_runtime.h>
> @@ -282,6 +283,23 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct drm_crtc *crtc,
>         return NULL;
>  }
>
> +static void mtk_drm_dma_buf_put(struct mtk_drm_crtc *mtk_crtc)
> +{
> +       unsigned int i;
> +
> +       for (i = 0; i < mtk_crtc->layer_nr; i++) {
> +               struct drm_plane *plane = &mtk_crtc->planes[i];
> +               struct mtk_plane_state *plane_state;
> +
> +               plane_state = to_mtk_plane_state(plane->state);
> +
> +               if (plane_state && plane_state->pending.dma_buf) {
> +                       dma_buf_put(plane_state->pending.dma_buf);
> +                       plane_state->pending.dma_buf = NULL;
> +               }
> +       }
> +}
> +
>  #if IS_REACHABLE(CONFIG_MTK_CMDQ)
>  static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg)
>  {
> @@ -322,6 +340,8 @@ static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg)
>                 mtk_crtc->pending_async_planes = false;
>         }
>
> +       mtk_drm_dma_buf_put(mtk_crtc);
> +
>         mtk_crtc->cmdq_vblank_cnt = 0;
>         wake_up(&mtk_crtc->cb_blocking_queue);
>  }
> @@ -614,9 +634,14 @@ static void mtk_crtc_ddp_irq(void *data)
>         else if (mtk_crtc->cmdq_vblank_cnt > 0 && --mtk_crtc->cmdq_vblank_cnt == 0)
>                 DRM_ERROR("mtk_crtc %d CMDQ execute command timeout!\n",
>                           drm_crtc_index(&mtk_crtc->base));
> +
> +       if (!mtk_crtc->cmdq_client.chan)
> +               mtk_drm_dma_buf_put(mtk_crtc);
>  #else
>         if (!priv->data->shadow_register)
>                 mtk_crtc_ddp_config(crtc, NULL);
> +
> +       mtk_drm_dma_buf_put(mtk_crtc);
>  #endif
>         mtk_drm_finish_page_flip(mtk_crtc);
>  }
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index d54fbf34b000..c169ca49129c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -12,6 +12,7 @@
>  #include <drm/drm_framebuffer.h>
>  #include <drm/drm_gem_atomic_helper.h>
>  #include <linux/align.h>
> +#include <linux/dma-buf.h>
>
>  #include "mtk_drm_crtc.h"
>  #include "mtk_drm_ddp_comp.h"
> @@ -280,6 +281,22 @@ static void mtk_plane_atomic_disable(struct drm_plane *plane,
>         struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state,
>                                                                            plane);
>         struct mtk_plane_state *mtk_plane_state = to_mtk_plane_state(new_state);
> +       struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state,
> +                                                                          plane);
> +
> +       if (old_state && old_state->fb) {
> +               struct drm_gem_object *gem = old_state->fb->obj[0];
> +
> +               if (mtk_plane_state->pending.dma_buf) {

When this happen, the pending.dma_buf is still accessed by OVL, right?
So you free this buffer and m4u translation fault occur.

Regards,
Chun-Kuang.

> +                       dma_buf_put(mtk_plane_state->pending.dma_buf);
> +                       mtk_plane_state->pending.dma_buf = NULL;
> +               }
> +
> +               if (gem && gem->dma_buf) {
> +                       get_dma_buf(gem->dma_buf);
> +                       mtk_plane_state->pending.dma_buf = gem->dma_buf;
> +               }
> +       }
>         mtk_plane_state->pending.enable = false;
>         wmb(); /* Make sure the above parameter is set before update */
>         mtk_plane_state->pending.dirty = true;
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> index 8f39011cdbfc..b724e56b7283 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> @@ -33,6 +33,7 @@ struct mtk_plane_pending_state {
>         bool                            async_dirty;
>         bool                            async_config;
>         enum drm_color_encoding         color_encoding;
> +       struct dma_buf                  *dma_buf;
>  };
>
>  struct mtk_plane_state {
> --
> 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 5071f1263216..9cf1c1778868 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -4,6 +4,7 @@ 
  */
 
 #include <linux/clk.h>
+#include <linux/dma-buf.h>
 #include <linux/dma-mapping.h>
 #include <linux/mailbox_controller.h>
 #include <linux/pm_runtime.h>
@@ -282,6 +283,23 @@  struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct drm_crtc *crtc,
 	return NULL;
 }
 
+static void mtk_drm_dma_buf_put(struct mtk_drm_crtc *mtk_crtc)
+{
+	unsigned int i;
+
+	for (i = 0; i < mtk_crtc->layer_nr; i++) {
+		struct drm_plane *plane = &mtk_crtc->planes[i];
+		struct mtk_plane_state *plane_state;
+
+		plane_state = to_mtk_plane_state(plane->state);
+
+		if (plane_state && plane_state->pending.dma_buf) {
+			dma_buf_put(plane_state->pending.dma_buf);
+			plane_state->pending.dma_buf = NULL;
+		}
+	}
+}
+
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
 static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg)
 {
@@ -322,6 +340,8 @@  static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg)
 		mtk_crtc->pending_async_planes = false;
 	}
 
+	mtk_drm_dma_buf_put(mtk_crtc);
+
 	mtk_crtc->cmdq_vblank_cnt = 0;
 	wake_up(&mtk_crtc->cb_blocking_queue);
 }
@@ -614,9 +634,14 @@  static void mtk_crtc_ddp_irq(void *data)
 	else if (mtk_crtc->cmdq_vblank_cnt > 0 && --mtk_crtc->cmdq_vblank_cnt == 0)
 		DRM_ERROR("mtk_crtc %d CMDQ execute command timeout!\n",
 			  drm_crtc_index(&mtk_crtc->base));
+
+	if (!mtk_crtc->cmdq_client.chan)
+		mtk_drm_dma_buf_put(mtk_crtc);
 #else
 	if (!priv->data->shadow_register)
 		mtk_crtc_ddp_config(crtc, NULL);
+
+	mtk_drm_dma_buf_put(mtk_crtc);
 #endif
 	mtk_drm_finish_page_flip(mtk_crtc);
 }
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index d54fbf34b000..c169ca49129c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -12,6 +12,7 @@ 
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_atomic_helper.h>
 #include <linux/align.h>
+#include <linux/dma-buf.h>
 
 #include "mtk_drm_crtc.h"
 #include "mtk_drm_ddp_comp.h"
@@ -280,6 +281,22 @@  static void mtk_plane_atomic_disable(struct drm_plane *plane,
 	struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state,
 									   plane);
 	struct mtk_plane_state *mtk_plane_state = to_mtk_plane_state(new_state);
+	struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state,
+									   plane);
+
+	if (old_state && old_state->fb) {
+		struct drm_gem_object *gem = old_state->fb->obj[0];
+
+		if (mtk_plane_state->pending.dma_buf) {
+			dma_buf_put(mtk_plane_state->pending.dma_buf);
+			mtk_plane_state->pending.dma_buf = NULL;
+		}
+
+		if (gem && gem->dma_buf) {
+			get_dma_buf(gem->dma_buf);
+			mtk_plane_state->pending.dma_buf = gem->dma_buf;
+		}
+	}
 	mtk_plane_state->pending.enable = false;
 	wmb(); /* Make sure the above parameter is set before update */
 	mtk_plane_state->pending.dirty = true;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
index 8f39011cdbfc..b724e56b7283 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
@@ -33,6 +33,7 @@  struct mtk_plane_pending_state {
 	bool				async_dirty;
 	bool				async_config;
 	enum drm_color_encoding		color_encoding;
+	struct dma_buf			*dma_buf;
 };
 
 struct mtk_plane_state {