<html><body><p>
<pre>
Hi CK,
Thanks for the reviews.
On Tue, 2025-06-24 at 09:39 +0000, CK Hu (胡俊光) wrote:
> 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.
>
Yes, this API is only used for GCE flow.
I'll add the condition in the beginning of this API to avoid breaking
the non-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.
>
I think the cursor won't be blocked until timeout here.
mtk_crtc->cmdq_vblank_cnt will be reset to 0 in ddp_cmdq_cb(), so that
we can make sure GCE has configured all the HW settings.
Regards,
Jason-JH Lin
> Regards,
> CK
>
>
</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><!--}-->