On Thu, Aug 27, 2015 at 03:05:38PM +0200, Maarten Lankhorst wrote: > Op 27-08-15 om 14:52 schreef Ville Syrjälä: > > On Thu, Aug 27, 2015 at 02:50:34PM +0200, Maarten Lankhorst wrote: > >> Op 27-08-15 om 14:48 schreef Ville Syrjälä: > >>> On Thu, Aug 27, 2015 at 02:43:35PM +0200, Maarten Lankhorst wrote: > >>>> Op 27-08-15 om 14:19 schreef Daniel Stone: > >>>>> Hi, > >>>>> > >>>>> On 4 August 2015 at 12:34, Maarten Lankhorst > >>>>> <maarten.lankhorst@xxxxxxxxxxxxxxx> wrote: > >>>>>> Commit ec9f932ed41622d120de52a5b525e4d77b9ef17e > >>>>>> "drm/atomic: Cleanup on error properly in the atomic ioctl." > >>>>>> cleaned up some error paths, but didn't fix the TEST_ONLY path. > >>>>>> In the check only case plane->fb shouldn't be updated, and > >>>>>> the vblank events should be cleared as on failure. > >>>>> Bikeshedding a bit ... > >>>>> > >>>>> An early test precludes TEST_ONLY | PAGE_FLIP_EVENT, so you don't need > >>>>> to mention this in the commit message; in this case, the main change > >>>>> is about plane->{,old_}fb. > >>>> Even testing with PAGE_FLIP_EVENT would be useful because > >>>> event && !crtc_state->active should not be allowed. In that case test > >>>> could succeed but commit could fail. > >>> Why would commit fail when the we're in DPMS off? I would suggest it > >>> should be allowed. The operation would just a be a nop from a HW point > >>> of view, all the calculation/checks would still be performed. > >>> > >> You can commit, just not with PAGE_FLIP_EVENT set when crtc is inactive. > > What's so special about the event here? Just send it out as soon as the > > state has been swapped. > Previously this has been disallowed for legacy page flips. I don't think so. Speaking for i915, I think we've just rejected legacy page flips entirely with the pipe is off on account of drm_vblank_get() failing. > I don't see why this should be relaxed. It just complicates things and you have nothing to stick in for the vblank counter. We could stick the last vbl count/timestamp in there. Not allowing means userspace is forced to consider the dpms state whenever it wants to call the atomic ioctl. -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel