Re: [v5, PATCH] drm/mediatek: add dma buffer control for drm plane disable

[CK Hu (胡俊光)]

Hi, Maxime:

On Tue, 2023-08-01 at 16:02 +0800, CK Hu wrote:
> Hi, Maxime:
> 
> On Wed, 2023-07-05 at 09:48 +0200, Maxime Ripard wrote:
> > Hi,
> > 
> > On Wed, Jul 05, 2023 at 02:07:18PM +0800, Yongqiang Niu wrote:
> > > 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
> > > 
> > > Fixes: 41016fe1028e ("drm: Rename plane->state variables in
> > > atomic
> > > update and disable")
> > > Signed-off-by: Yongqiang Niu <yongqiang.niu@xxxxxxxxxxxx>
> > 
> > I think we need more details in the commit message about what the
> > issue
> > is exactly and how it's fixed.
> > 
> > This definitely feels like it's not something drivers should have
> > to
> > do.
> 
> Because MediaTek display hardware has no internal SRAM which could
> store the on screen buffer, plane should be updated in vblank period
> to
> prevent tearing effect. In MediaTek plane disable callback function,
> if
> we wait vblank period to disable plane, the software would be blocked
> for a long time. If there are four planes to be disabled, the total
> blocking time would be almost 4 vsync time. So this patch is just a
> work around to use get_dma_buf() to prevent buffer to be freed before
> vblank period and not block the software. For the behavior DRM core
> free plane buffer just after plane disable callback function return,
> I'm not sure it's DRM core's bug or it's DRM core's correct behavior.
> If this is correct behavior, it means that DRM core assume that
> hardware should have internal SRAM but MediaTek display hardware
> violate this assumption. If so, I think I could just accept this work
> around patch.

Sorry. I've found that drm_atomic_helper_commit_tail() has wait for
vblank after commit. So I think that's MediaTek driver's problem.
MediaTek driver should promise that configuration should take effect
before send vblank event.

Regards,
CK

> 
> Regards,
> CK
> 
> 
> > 
> > Maxime