On Fri, Aug 03, 2018 at 03:10:57PM -0300, Gustavo Padovan wrote: > On Fri, Aug 03, 2018 at 12:42:15PM -0400, Sean Paul wrote: > > On Thu, Aug 02, 2018 at 08:09:51AM -0300, Gustavo Padovan wrote: > > > Hi Haneen, > > > > > > On Thu, Aug 02, 2018 at 04:10:26AM +0300, Haneen Mohammed wrote: > > > > This patch implement the necessary functions to compute and add CRCs > > > > entries: > > > > > > > > - Implement the set_crc_source() callback. > > > > - Compute CRC using crc32 on the visible part of the framebuffer. > > > > - Use ordered workqueue per output to compute and add CRC at the end > > > > of a vblank. > > > > - Use appropriate synchronization methods since the CRC computation must > > > > be atomic wrt the generated vblank event for a given atomic update, by > > > > using spinlock across atomic_begin/atomic_flush to wrap the event > > > > handling code completely and match the flip event with the CRC. > > > > > > > > Since vkms_crc_work_handle() can sleep, spinlock can't be acquired > > > > while accessing vkms_output->primary_crc to compute CRC. > > > > To make sure the data is updated and released without conflict with > > > > the vkms_crc_work_handle(), the work_struct is flushed @crtc_destroy > > > > and the data is updated before scheduling the work handle again, as > > > > follow: > > > > > > I'm not sure I get why this is a spinlock and not a mutex. With the > > > later you are able to sleep. Also, your spinlock held through the whole > > > atomic commit (from the begin to flush) which seems too much time for > > > busy waiting. Or am I missing something here? > > > > I expressed some of the same concerns here: > > > > https://lists.freedesktop.org/archives/dri-devel/2018-July/183584.html > > > > The tl;dr is that the spinlock is required so we can acquire it in the vblank > > hrtimer. It is held across begin/flush to prevent sending vblank eventss until > > the crc has been fully calculated for the frame. More details in the thread, > > ofc. > > Sounds good to me! Thanks! I've pushed the patches to drm-misc-next Sean > > Gustavo > -- Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
