On Mon, Dec 07, 2015 at 03:33:00PM +0000, Daniel Stone wrote:
> Hi,
>
> On 4 December 2015 at 08:46, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
> > + /*
> > + * Reject event generation for when a CRTC is off and stays off. It
> > + * wouldn't be hard to implement this, but userspace has a track record
> > + * of happily burning through 100% cpu (or worse, crash) when the
> > + * display pipe is suspended. To avoid all that fun just reject updates
> > + * that ask for events since likely that indicates a bug in the
> > + * compositors drawing loop. This is consistent with the vblank ioctl
> > + * which also rejects service on a disabled pipe.
> > + */
>
> Sorry to keep whingeing, but this isn't actually related to event
> generation. To the best of my knowledge, this change does a few
> things. Firstly, comments the check above (enforcing that (flags &
> ALLOW_MODESET) == {crtcs}->allow_modeset), which is good. But the
> comment is incorrect, because whether or not an event is requested is
> wholly unrelated. Secondly, it disables allow_modeset on pageflip,
> which shouldn't be changing DPMS stage (good!). Thirdly, it enforces
> something like the above statement on pageflips, except again it has
> no regard to events: it just enforces the no-DPMS-on-flip rule, for
> which event generation is a subset.
Well the comment is completely misplace, I wanted to put it next to the
check for event generation, not here.
> If you fix the above comment to more accurately note that this just
> enforces that you need the ALLOW_MODESET flag to change active, mode
> or connector routing, and (as Thierry asked), add a comment below to
> note that we enforce existing no-DPMS-on-flip semantics in the helper,
> then you're welcome to my R-b. But please don't mention events in the
> new comment. :)
Hm, I didn't really want to type a comment for ALLOW_MODESET - imo it's
pretty clear what it does and why it's useful. I'll try again at making
something coheren here ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
