On Fri, Jul 11, 2014 at 11:47 AM, Boris BREZILLON <boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote: > On Fri, 11 Jul 2014 11:41:12 -0400 > Rob Clark <robdclark@xxxxxxxxx> wrote: > >> On Fri, Jul 11, 2014 at 11:17 AM, Boris BREZILLON >> <boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote: >> > Make use of lists instead of kfifo in order to dynamically allocate >> > task entry when someone require some delayed work, and thus preventing >> > drm_flip_work_queue from directly calling func instead of queuing this >> > call. >> > This allow drm_flip_work_queue to be safely called even within irq >> > handlers. >> > >> > Add new helper functions to allocate a flip work task and queue it when >> > needed. This prevents allocating data within irq context (which might >> > impact the time spent in the irq handler). >> > >> > Signed-off-by: Boris BREZILLON <boris.brezillon@xxxxxxxxxxxxxxxxxx> >> > --- >> > Hi Rob, >> > >> > This is a proposal for what you suggested (dynamically growing the drm >> > flip work queue in order to avoid direct call of work->func when calling >> > drm_flip_work_queue). >> > >> > I'm not sure this is exactly what you expected, because I'm now using >> > lists instead of kfifo (and thus lose the lockless part), but at least >> > we can now safely call drm_flip_work_queue or drm_flip_work_queue_task >> > from irq handlers :-). >> > >> > You were also worried about queueing the same framebuffer multiple times >> > and with this implementation you shouldn't have any problem (at least with >> > drm_flip_work_queue, what people do with drm_flip_work_queue_task is their >> > own responsability, but they should allocate one task for each operation >> > even if they are manipulating the same framebuffer). >> >> yeah, if we are dynamically allocating the list nodes, that solves the >> queuing-up-multiple-times issue.. >> >> I wonder if drm_flip_work_allocate_task() should use GPF_ATOMIC when >> allocating? > > That's funny, I was actually modifying the API to pass gfp_t flags to > this function ;-) yeah, I think passing gfp flags is the better idea >> I guess maybe it is possible to pre-allocate the task >> from non-irq context, and then queue it from irq context.. it makes >> the API a bit more complex, but there are only a couple users >> currently, so I suppose this should be doable. > > I tried to keep the existing API so that existing users won't see the > difference (I guess none of them are calling drm_flip_work_queue). we do have existing users that call drm_flip_work_queue() from irq.. but I suppose adding gfp flags arg to drm_flip_work_queue() seems like a reasonable solution. BR, -R > I just added the drm_flip_work_allocate_task and > drm_flip_work_queue_task for those who want more control on the > queuing process. > > Best Regards, > > Boris > > > > > > -- > Boris Brezillon, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
