Hi Inki, On 9 February 2015 at 14:53, Inki Dae <inki.dae@xxxxxxxxxxx> wrote: > I'm merging this patch series but I found two issues. One is already > pointed out. Fantastic - thanks a lot for doing this! > On 2015년 02월 07일 04:37, Gustavo Padovan wrote: >> @@ -628,20 +638,6 @@ static void fimd_win_commit(struct exynos_drm_crtc *crtc, unsigned int win) >> return; >> } >> >> - /* >> - * SHADOWCON/PRTCON register is used for enabling timing. >> - * >> - * for example, once only width value of a register is set, >> - * if the dma is started then fimd hardware could malfunction so >> - * with protect window setting, the register fields with prefix '_F' >> - * wouldn't be updated at vsync also but updated once unprotect window >> - * is set. >> - */ >> - >> - /* protect windows */ >> - fimd_shadow_protect_win(ctx, win, true); > > You removed to protect shadow register updating at vsync so fimd > hardware could malfunction if setcrtc/page flip/setplane are requested > by userspace. Actually, when I run modetest repeatedly, I could see > overlay isn't rarely turned on. > > So how about calling win_protect/unprotect callbacks like below? If you > are ok, I can modify it and merge it. I think you are missing some details about atomic and the helpers. The helpers wrap _all_ legacy codepaths, e.g. SetCrtc, SetPlane, etc. All of these operations are intercepted by the code in drm_atomic_helper.c / drm_plane_helper.c code, and the legacy operations are turned into atomic operations. For the driver - there are no legacy operations. The atomic helpers guarantee that atomic_begin() will be called before the state is committed, and atomic_flush() will be called after the state is committed. Thus this change is completely safe. > This warning would mean a fb object leak and while I have a review, I > couldn't find why it causes the leak. I'm not really surprised; the fb refcounting in Exynos has long had quite a few issues. The refcounting is incredibly difficult to get right, and whilst atomic changes the order of operations a little, it also makes the semantics of fb/vblank refcounting much more consistent, so it should be much easier to fix than usual. > However, I'd like to merge this patch series this time and fix this > issue at RC for phase 3. Great, thankyou! Really very happy about this. :) Cheers, Daniel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
