On Tue, Jan 05, 2016 at 07:55:52PM +0900, Inki Dae wrote:
> Hi Daniel,
>
> 2016년 01월 05일 05:24에 Daniel Stone 이(가) 쓴 글:
> > Hi Inki,
> >
> > On 4 January 2016 at 12:57, Inki Dae <inki.dae@xxxxxxxxxxx> wrote:
> >> 2015년 12월 24일 22:32에 Daniel Stone 이(가) 쓴 글:
> >>> On 24 December 2015 at 09:10, Inki Dae <inki.dae@xxxxxxxxxxx> wrote:
> >>>> +void exynos_drm_crtc_cancel_page_flip(struct drm_crtc *crtc)
> >>>> +{
> >>>> + struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> >>>> + unsigned long flags;
> >>>> +
> >>>> + spin_lock_irqsave(&crtc->dev->event_lock, flags);
> >>>> + exynos_crtc->event = NULL;
> >>>> + spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> >>>> +}
> >>>
> >>> This will leak the event and event space; you should call
> >>> event->base.destroy() here. With that fixed:
> >>
> >> Right. we don't use exynos specific page flip function anymore which managed the event as a list so that the event objects can be freed by postclose callback.
> >> Anyway, would it be better for event->base.destory() to be called between spin lock/unlock?
> >
> > You must increment event->base.file_priv->event_space (see
> > drm_atomic.c:destroy_vblank_event), as well as calling
>
> Reasonable to me. Seems other DRM drivers don't increment event_space.
>
> > event->base.destroy (see drm_fops.c:drm_read) underneath event_lock,
> > yes.
>
> In addition, only event objects belonging to the request process should be destroyed.
Just random comment out of the far left field, but robclark had a bunch of
patches to clean up all that event alloc/cleanup code a bit and extract it
into core functions. Might be good to ping him on irc to figure out where
that series is and whether you could take it over.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
