<html><body><p>
<pre>
On Thu, 2025-05-22 at 16:34 +0800, Jason-JH Lin wrote:
> Our hardware registers are set through GCE, not by the CPU.
> DRM might assume the hardware is disabled immediately after calling
> atomic_disable() of drm_plane, but it is only truly disabled after the
> GCE IRQ is triggered.
>
> Additionally, the cursor plane in DRM uses async_commit, so DRM will
> not wait for vblank and will free the buffer immediately after calling
> atomic_disable().
>
> To prevent the framebuffer from being freed before the layer disable
> settings are configured into the hardware, which can cause an IOMMU
> fault error, a wait_event_timeout has been added to wait for the
> ddp_cmdq_cb() callback,indicating that the GCE IRQ has been triggered.
>
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_crtc.c | 30 ++++++++++++++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_crtc.h | 1 +
> drivers/gpu/drm/mediatek/mtk_plane.c | 5 +++++
> 3 files changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
> index 8f6fba4217ec..944a3d1e5ec9 100644
> --- a/drivers/gpu/drm/mediatek/mtk_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
> @@ -719,6 +719,36 @@ int mtk_crtc_plane_check(struct drm_crtc *crtc, struct drm_plane *plane,
> return 0;
> }
>
> +void mtk_crtc_plane_disable(struct drm_crtc *crtc, struct drm_plane *plane)
> +{
> +struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc);
> +struct mtk_plane_state *plane_state = to_mtk_plane_state(plane->state);
> +int i;
> +
> +if (!mtk_crtc->enabled)
> +return;
> +
> +/* set pending plane state to disabled */
> +for (i = 0; i < mtk_crtc->layer_nr; i++) {
> +struct drm_plane *mtk_plane = &mtk_crtc->planes[i];
> +struct mtk_plane_state *mtk_plane_state = to_mtk_plane_state(mtk_plane->state);
> +
> +if (mtk_plane->index == plane->index) {
> +memcpy(mtk_plane_state, plane_state, sizeof(*plane_state));
In commit message, you mention GCE flow has problem.
This also modify non-GCE flow.
If non-GCE flow does not need this, move this to GCE flow.
> +break;
> +}
> +}
> +mtk_crtc_update_config(mtk_crtc, false);
> +
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +/* wait for planes to be disabled by cmdq */
> +if (mtk_crtc->cmdq_client.chan)
> +wait_event_timeout(mtk_crtc->cb_blocking_queue,
> + mtk_crtc->cmdq_vblank_cnt == 0,
Check 'mtk_crtc->cmdq_vblank_cnt == 0' may be not good.
If a video is playing and mtk_crtc_update_config() would be call every frame,
mtk_crtc->cmdq_vblank_cnt may not be zero and cursor would be block until timeout.
Regards,
CK
> + msecs_to_jiffies(500));
> +#endif
> +}
> +
> void mtk_crtc_async_update(struct drm_crtc *crtc, struct drm_plane *plane,
> struct drm_atomic_state *state)
> {
> diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.h b/drivers/gpu/drm/mediatek/mtk_crtc.h
> index 388e900b6f4d..828f109b83e7 100644
> --- a/drivers/gpu/drm/mediatek/mtk_crtc.h
> +++ b/drivers/gpu/drm/mediatek/mtk_crtc.h
> @@ -21,6 +21,7 @@ int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,
> unsigned int num_conn_routes);
> int mtk_crtc_plane_check(struct drm_crtc *crtc, struct drm_plane *plane,
> struct mtk_plane_state *state);
> +void mtk_crtc_plane_disable(struct drm_crtc *crtc, struct drm_plane *plane);
> void mtk_crtc_async_update(struct drm_crtc *crtc, struct drm_plane *plane,
> struct drm_atomic_state *plane_state);
> struct device *mtk_crtc_dma_dev_get(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c
> index 655106bbb76d..59edbe26f01e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_plane.c
> @@ -285,9 +285,14 @@ 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);
> +
> mtk_plane_state->pending.enable = false;
> wmb(); /* Make sure the above parameter is set before update */
> mtk_plane_state->pending.dirty = true;
> +
> +mtk_crtc_plane_disable(old_state->crtc, plane);
> }
>
> static void mtk_plane_atomic_update(struct drm_plane *plane,
</pre>
</p></body></html><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice
********************
The information contained in this e-mail message (including any
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be
conveyed only to the designated recipient(s). Any use, dissemination,
distribution, printing, retaining or copying of this e-mail (including its
attachments) by unintended recipient(s) is strictly prohibited and may
be unlawful. If you are not an intended recipient of this e-mail, or believe
that you have received this e-mail in error, please notify the sender
immediately (by replying to this e-mail), delete any and all copies of
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->