[Intel-gfx] [PATCH 28/28] drm/atomic-helper: Reject legacy flips on a disabled pipe

[Daniel Stone]

Hi,

On 4 December 2015 at 08:46, Daniel Vetter <daniel.vetter at ffwll.ch> 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.

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. :)

Cheers,
Daniel