<html><body><p> <pre> Hi Daniel, Thanks for the review. On Sat, 2025-05-24 at 13:47 +0100, Daniel Stone wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > Hi Jason, > > On Thu, 22 May 2025 at 09:52, Jason-JH Lin > <jason-jh.lin@mediatek.com> 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. > > Waiting up to 500ms for each plane to be disabled is ... not ideal. It a timeout it won't really wait up to 500ms. I can shrink the timeout to 1 VSYNC because it must be less than 16.666ms (depend on encoder's timing). But it's enough to free the framebuffer at this moment. > Especially as multiple planes can be disabled at once. This may > happen > dynamically during runtime, e.g. when a video is playing and a user > moves their cursor over the plane to make the UI controls visible. The video layer will be disabled or video layer and UI layer will be swapped by the atomic_comit() with drm_pending_vblank_event in this case. > > I think this should be handled through the atomic_commit() handler, > with asynchronous tracking of the state, instead of the hard block > here. In my case, I encounter the UI process is killed and all the framebuffer are set to NULL to each plane. The log is shown as below: // After the process is killed // framebuffer(0xf9c00000) of UI layer0 will be freed after ddp_cmdq_cb [ 55.996903] mtk_plane_atomic_disable 296: idx:0, addr=0xf9c00000 [ 55.996923] mtk_crtc_update_config 980: needs_vblank=1 [ 56.001300] mtk_gem_free_object 110: dma_va = 00000000b49ad5c8, dma_iova = 0x00000000fb980000, size = 262144 [ 56.001538] ddp_cmdq_cb 533: need_vblank:1, sta:0 [ 56.017297] mtk_gem_free_object 110: dma_va = 000000003f29e25b, dma_iova = 0x00000000fb000000, size = 9216000 [ 56.017553] mtk_gem_free_object 110: dma_va = 000000000fb130cb, dma_iova = 0x00000000fa600000, size = 9216000 [ 56.017750] mtk_gem_free_object 110: dma_va = 00000000f786f524, dma_iova = 0x00000000f9c00000, size = 9216000 ... // framebuffer(0xfb9c0000) of cursor layer3 will be freed before ddp_cmdq_cb [ 62.851250] mtk_plane_atomic_disable 296: idx:3, addr=0xfb9c0000 [ 62.851263] mtk_crtc_update_config 980: needs_vblank=0 [ 62.851273] mtk_gem_free_object 110: dma_va = 00000000716d147d, dma_iova = 0x00000000fb9c0000, size = 262144 [ 62.851442] arm-smmu-v3 30800000.iommu: TBU_id-0- fault_id:0x40c(0x40c), TF read in NORMAL world, iova:0xfb9c5000, sid:144, ssid:0, ssidv:0, secsidv:0 [ 62.851453] arm-smmu-v3 30800000.iommu: event 0x10 received: [ 62.851457] arm-smmu-v3 30800000.iommu: 0x0000009000000010 [ 62.851461] arm-smmu-v3 30800000.iommu: 0x0000020800000000 [ 62.851464] arm-smmu-v3 30800000.iommu: 0x00000000fb9c5000 [ 62.851467] arm-smmu-v3 30800000.iommu: 0x0000000000000000 ... [ 62.854666] ddp_cmdq_cb 533: need_vblank:1, sta:-103 [ 62.854691] mtk_crtc_update_config 980: needs_vblank=1 [ 62.867886] ddp_cmdq_cb 533: need_vblank:1, sta:0 --- After applying this path: // framebuffer(0xfb9c0000) of cursor layer3 will be freed after ddp_cmdq_cb [ 1106.422478] mtk_plane_atomic_disable 296: idx:3, addr=0xfb9c0000 [ 1106.422487] ddp_cmdq_cb 533: need_vblank:0, sta:-103 [ 1106.422508] mtk_crtc_update_config 980: needs_vblank=0 [ 1106.434976] ddp_cmdq_cb 533: need_vblank:0, sta:0 [ 1106.435027] mtk_plane_atomic_disable 308: idx:3 done! [ 1106.435052] mtk_crtc_update_config 980: needs_vblank=0 [ 1106.435077] mtk_gem_free_object 110: dma_va = 0000000075285c43, dma_iova = 0x00000000fb9c0000, size = 262144 --- We have already tracking the async state for cursor layer3 in atomic_commit(), but it seems its framebuffer will still be freed before ddp_cmdq_cb. Do you have any ideal rather than adding a hard block in mtk_plane_atomic_disable()? Regards, Jason-JH Lin > > Cheers, > Daniel </pre> </p></body></html><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>