[Ozone-Drm] Pick pending buffer over current buffer when modesetting

[Ozone-Drm] Pick pending buffer over current buffer when modesetting

Buffers are expected to be scheduled/released in FIFO order. When
enabling a display we want to re-use a buffer to avoid flashes. As
such we need to make sure that the buffer used for modesetting is the
last buffer queued.

BUG=chrome-os-partner:38267
Committed: https://crrev.com/294fce9a52d66387fc9d5ddcea123778dd446f65
Cr-Commit-Position: refs/heads/master@{#322673}

[dnicoara, 2015-03-27 16:41:33]

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

[dnicoara, 2015-03-27 16:41:34]

Could you try with this patch. I tested locally a bit and I think it fixes the
issue.

[dbehr, 2015-03-27 20:50:07]

On 2015/03/27 16:41:34, dnicoara wrote:
> Could you try with this patch. I tested locally a bit and I think it fixes the
> issue.

Looks good to me.
I tried powerd_dbus_supend and suspend_stress_test while running WebGL Aquarium
and there are no GPU "hangs" and no messages about killing GPU process anymore.

I would sleep better, if this patch was first draining pending/request queue and
then reapplying current buffer, but I do not know if it is possible.

[dnicoara, 2015-03-27 21:03:14]

On 2015/03/27 20:50:07, dbehr wrote:
> On 2015/03/27 16:41:34, dnicoara wrote:
> > Could you try with this patch. I tested locally a bit and I think it fixes
the
> > issue.
> 
> Looks good to me.
> I tried powerd_dbus_supend and suspend_stress_test while running WebGL
Aquarium
> and there are no GPU "hangs" and no messages about killing GPU process
anymore.
> 
> I would sleep better, if this patch was first draining pending/request queue
and
> then reapplying current buffer, but I do not know if it is possible.

HDC::Modeset drains the requests queue and updates the pending/current buffers.
So we shouldn't ever get into a situation that queued requests would still be
around after modesetting.

The real issue was choosing the correct buffer to modeset with since Chrome
expects that buffers are processed in fifo order. Previously it was using the
current buffer to modeset with. The issue with that was if a request was pending
since that would break the fifo ordering expected by Chrome.

[dnicoara, 2015-03-27 21:07:44]

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

[dnicoara, 2015-03-27 21:07:45]

+spang@ for owners review

[spang, 2015-03-27 21:26:11]

https://codereview.chromium.org/1041823002/diff/20001/ui/ozone/platform/drm/g...
File ui/ozone/platform/drm/gpu/hardware_display_controller.cc (right):

https://codereview.chromium.org/1041823002/diff/20001/ui/ozone/platform/drm/g...
ui/ozone/platform/drm/gpu/hardware_display_controller.cc:82: primary =
OverlayPlane::GetPrimaryPlane(current_planes_);
I agree with Dominic.. is it really necessary to do anything here while swaps
are already in flight? Can't we just wait a minute for things to settle before
queuing up more work?

Trying to figure out whatever the last thing we did was and re-do it seems super
confusing to me.

[dnicoara, 2015-03-27 21:28:21]

https://codereview.chromium.org/1041823002/diff/20001/ui/ozone/platform/drm/g...
File ui/ozone/platform/drm/gpu/hardware_display_controller.cc (right):

https://codereview.chromium.org/1041823002/diff/20001/ui/ozone/platform/drm/g...
ui/ozone/platform/drm/gpu/hardware_display_controller.cc:82: primary =
OverlayPlane::GetPrimaryPlane(current_planes_);
On 2015/03/27 21:26:11, spang wrote:
> I agree with Dominic.. is it really necessary to do anything here while swaps
> are already in flight? Can't we just wait a minute for things to settle before
> queuing up more work?
> 
> Trying to figure out whatever the last thing we did was and re-do it seems
super
> confusing to me.

The Browser treats display configuration and buffer swaps as orthogonal
operations. If we wait, we can wait for ever since the compositors will continue
pushing new frames.

[spang, 2015-03-27 21:30:54]

On 2015/03/27 21:28:21, dnicoara wrote:
>
https://codereview.chromium.org/1041823002/diff/20001/ui/ozone/platform/drm/g...
> File ui/ozone/platform/drm/gpu/hardware_display_controller.cc (right):
> 
>
https://codereview.chromium.org/1041823002/diff/20001/ui/ozone/platform/drm/g...
> ui/ozone/platform/drm/gpu/hardware_display_controller.cc:82: primary =
> OverlayPlane::GetPrimaryPlane(current_planes_);
> On 2015/03/27 21:26:11, spang wrote:
> > I agree with Dominic.. is it really necessary to do anything here while
swaps
> > are already in flight? Can't we just wait a minute for things to settle
before
> > queuing up more work?
> > 
> > Trying to figure out whatever the last thing we did was and re-do it seems
> super
> > confusing to me.
> 
> The Browser treats display configuration and buffer swaps as orthogonal
> operations. If we wait, we can wait for ever since the compositors will
continue
> pushing new frames.

I'm not super familiar with this can't you just turn the next flip into a set?

I really cannot believe that this is the right thing to do.

[spang, 2015-03-27 22:28:05]

On 2015/03/27 21:30:54, spang wrote:
> On 2015/03/27 21:28:21, dnicoara wrote:
> >
>
https://codereview.chromium.org/1041823002/diff/20001/ui/ozone/platform/drm/g...
> > File ui/ozone/platform/drm/gpu/hardware_display_controller.cc (right):
> > 
> >
>
https://codereview.chromium.org/1041823002/diff/20001/ui/ozone/platform/drm/g...
> > ui/ozone/platform/drm/gpu/hardware_display_controller.cc:82: primary =
> > OverlayPlane::GetPrimaryPlane(current_planes_);
> > On 2015/03/27 21:26:11, spang wrote:
> > > I agree with Dominic.. is it really necessary to do anything here while
> swaps
> > > are already in flight? Can't we just wait a minute for things to settle
> before
> > > queuing up more work?
> > > 
> > > Trying to figure out whatever the last thing we did was and re-do it seems
> > super
> > > confusing to me.
> > 
> > The Browser treats display configuration and buffer swaps as orthogonal
> > operations. If we wait, we can wait for ever since the compositors will
> continue
> > pushing new frames.
> 
> I'm not super familiar with this can't you just turn the next flip into a set?
> 
> I really cannot believe that this is the right thing to do.

alright let's do this for now to get the P0 fixed

lgtm

[dnicoara, 2015-03-27 23:17:16]

The CQ bit was checked by dnicoara@chromium.org

[commit-bot: I haz the power, 2015-03-27 23:17:43]

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

[commit-bot: I haz the power, 2015-03-27 23:21:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-27 23:22:37]

Patchset 3 (id:??) landed as
https://crrev.com/294fce9a52d66387fc9d5ddcea123778dd446f65
Cr-Commit-Position: refs/heads/master@{#322673}