On Fri, Nov 9, 2012 at 10:50 AM, Inki Dae <inki.dae@xxxxxxxxxxx> wrote: > I don't think so. let's see it again. below is my idea AND the reason I > posted this patch. > Original codes, > gem alloc(gem0); > -> gem0 refcount = 1 > gem0 mmap > -> gem0 refcount = 2 > gem alloc(gem1); > -> gem1 refcount =1 > gem1 mmap > -> gem1 refcount =2 > addfb(fb0, gem0); > -> gem0 refcount=3 > addfb(fb1,gem1); > -> gem1 refcount = 3 > setcrtc(fb0, crtc0) > -> crtc0.fb = fb0 > pageflip(crtc0, fb1); > -> crtc0.fb = fb1. > and pageflip is repeated > > close(gem0) > -> gem0 refcount = 2 > close(gem1) > ->gem1 refcount = 2 > munmap(gem0) > ->gem0 refcount = 1 > munmap(gem1) > ->gem1 refcount = 1 > > close(drm) > 1. fb release > -> check if crtc->fb is same as fb0 but current crtc is pointing to fb1 > 2. so free fb0 without disabling current crtc. > -> gem0 refcount = 0 so released. At this time, dma access invalid memory > region unfortunately if the dma is accessing gem0. > I think you should either hold an extra ref to the fb until dma stops, or block when the fb is destroyed until dma stops. See the 'drm: refcnt drm_framebuffer' patch. This lets me solve a similar issue in omapdrm. (Well, you could solve the same issue by holding a ref to the GEM objects somewhere else until the scanout stops, but it is easier for the crtc just to hold a ref to the fb.) BR, -R > > > With my patch, > ... > setcrtc(fb0, crtc0) > -> crtc0.fb = fb0, fb0.crtc = crtc0 > pageflip(crtc0, fb1); > -> crtc0.fb = fb1, fb1.crtc = crtc0. > and pageflip is repeated > > close(gem0) > -> gem0 refcount = 2 > close(gem1) > ->gem1 refcount = 2 > munmap(gem0) > ->gem0 refcount = 1 > munmap(gem1) > ->gem1 refcount = 1 > close(drm) > 1. fb release > -> fb0->crtc is same as crtc so disable crtc (dma stop) > 2. and free fb0. > -> gem0 refcount = 0 so released. We can avoid invalid memory access because > the dma was stopped. > In addition, one thing you pointed should be considered like below, > 1. When rmfb is called, if fb is last one then it sets fb->crtc to NULL. > - the fb is cleaned up with disabling crtc. > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel