[Ozone-DRI] Fix waiting for page flips in mirror mode

[Ozone-DRI] Fix waiting for page flips in mirror mode

Page flips arrive in random order so we want to make sure that we
process the page flips for all affected controllers.

BUG=none
NOTRY=true

Committed: https://crrev.com/1efa022be075636b97031f46a7e0798898576214
Cr-Commit-Position: refs/heads/master@{#299129}

[dnicoara, 2014-10-09 19:27:40]

dnicoara@chromium.org changed reviewers:
+ alexst@chromium.org

[dnicoara, 2014-10-09 19:27:40]

PTAL

[alexst (slow to review), 2014-10-09 20:54:40]

https://codereview.chromium.org/641173003/diff/1/ui/ozone/platform/dri/hardwa...
File ui/ozone/platform/dri/hardware_display_controller.cc (right):

https://codereview.chromium.org/641173003/diff/1/ui/ozone/platform/dri/hardwa...
ui/ozone/platform/dri/hardware_display_controller.cc:122: // Messages do not
come in any particular order so we need to make sure the
This deals with messages from multiple monitors on a crtc in mirrored mode if I
understand it correctly. I think it's a crucial thing to add in a comment here.

[dnicoara, 2014-10-10 13:54:00]

https://codereview.chromium.org/641173003/diff/1/ui/ozone/platform/dri/hardwa...
File ui/ozone/platform/dri/hardware_display_controller.cc (right):

https://codereview.chromium.org/641173003/diff/1/ui/ozone/platform/dri/hardwa...
ui/ozone/platform/dri/hardware_display_controller.cc:122: // Messages do not
come in any particular order so we need to make sure the
On 2014/10/09 20:54:40, alexst wrote:
> This deals with messages from multiple monitors on a crtc in mirrored mode if
I
> understand it correctly. I think it's a crucial thing to add in a comment
here.

Actually, it can be multiple CRTCs in mirror mode. HandleEvent can process an
event for a different CRTC, so without the while-loop it is possible that it'd
process an event for the second CRTC. Moving to the second CRTC this condition
would fail and it'd end up waiting for 1 event rather than 2.

[alexst (slow to review), 2014-10-10 15:50:55]

lgtm

https://codereview.chromium.org/641173003/diff/1/ui/ozone/platform/dri/hardwa...
File ui/ozone/platform/dri/hardware_display_controller.cc (right):

https://codereview.chromium.org/641173003/diff/1/ui/ozone/platform/dri/hardwa...
ui/ozone/platform/dri/hardware_display_controller.cc:122: // Messages do not
come in any particular order so we need to make sure the
On 2014/10/10 13:54:00, dnicoara wrote:
> On 2014/10/09 20:54:40, alexst wrote:
> > This deals with messages from multiple monitors on a crtc in mirrored mode
if
> I
> > understand it correctly. I think it's a crucial thing to add in a comment
> here.
> 
> Actually, it can be multiple CRTCs in mirror mode. HandleEvent can process an
> event for a different CRTC, so without the while-loop it is possible that it'd
> process an event for the second CRTC. Moving to the second CRTC this condition
> would fail and it'd end up waiting for 1 event rather than 2.

Right, I actually like your response better than the comment you left in the
code.

I am just being extra nitty about comments. I know my way around this code, but
it's got subtitles and I still have to open my editor and browse around to
convince myself things do what I understand them to do, which is why I keep
asking for more comments.

[dnicoara, 2014-10-10 16:59:59]

On 2014/10/10 15:50:55, alexst wrote:
> lgtm
> 
>
https://codereview.chromium.org/641173003/diff/1/ui/ozone/platform/dri/hardwa...
> File ui/ozone/platform/dri/hardware_display_controller.cc (right):
> 
>
https://codereview.chromium.org/641173003/diff/1/ui/ozone/platform/dri/hardwa...
> ui/ozone/platform/dri/hardware_display_controller.cc:122: // Messages do not
> come in any particular order so we need to make sure the
> On 2014/10/10 13:54:00, dnicoara wrote:
> > On 2014/10/09 20:54:40, alexst wrote:
> > > This deals with messages from multiple monitors on a crtc in mirrored mode
> if
> > I
> > > understand it correctly. I think it's a crucial thing to add in a comment
> > here.
> > 
> > Actually, it can be multiple CRTCs in mirror mode. HandleEvent can process
an
> > event for a different CRTC, so without the while-loop it is possible that
it'd
> > process an event for the second CRTC. Moving to the second CRTC this
condition
> > would fail and it'd end up waiting for 1 event rather than 2.
> 
> Right, I actually like your response better than the comment you left in the
> code.
> 
> I am just being extra nitty about comments. I know my way around this code,
but
> it's got subtitles and I still have to open my editor and browse around to
> convince myself things do what I understand them to do, which is why I keep
> asking for more comments.

Updated. Let me know if you still think it needs clarifications.

[alexst (slow to review), 2014-10-10 17:01:13]

Perfect, thank you!

[dnicoara, 2014-10-10 17:02:42]

The CQ bit was checked by dnicoara@chromium.org

[commit-bot: I haz the power, 2014-10-10 17:03:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/641173003/130001

[commit-bot: I haz the power, 2014-10-10 17:04:30]

Committed patchset #2 (id:130001)

[commit-bot: I haz the power, 2014-10-10 17:05:39]

Patchset 2 (id:??) landed as
https://crrev.com/1efa022be075636b97031f46a7e0798898576214
Cr-Commit-Position: refs/heads/master@{#299129}