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(+)
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 {
Hi Yongqiang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on linus/master v6.3-rc3 next-20230320] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Yongqiang-Niu/drm-mediatek-ad... base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230320030449.5397-1-yongqiang.niu%40mediatek.com patch subject: [v3, PATCH] drm/mediatek: add dma buffer control for drm plane disable config: arm64-randconfig-r001-20230320 (https://download.01.org/0day-ci/archive/20230320/202303201543.ahrAhliY-lkp@i...) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm64 cross compiling tool for clang build # apt-get install binutils-aarch64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/ae19fefd6d548a2766bc6d1902c46d... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Yongqiang-Niu/drm-mediatek-add-dma-buffer-control-for-drm-plane-disable/20230320-110649 git checkout ae19fefd6d548a2766bc6d1902c46d5baa39a202 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com | Link: https://lore.kernel.org/oe-kbuild-all/202303201543.ahrAhliY-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
ERROR: modpost: module mediatek-drm uses symbol dma_buf_put from namespace DMA_BUF, but does not import it.
Il 20/03/23 04:04, Yongqiang Niu ha scritto:
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
The Fixes tag currently has wrong format, and it goes here, not at the beginning; Please fix.
P.S. The right format is: Fixes: commitid ("commit title")
Signed-off-by: Yongqiang Niu yongqiang.niu@mediatek.com
Regards, Angelo
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
linaro-mm-sig@lists.linaro.org
-
AngeloGioacchino Del Regno -
Chun-Kuang Hu -
kernel test robot -
Yongqiang Niu