Fix black screen when switching primary display without bounds change

Fix black screen when switching primary display without bounds change

In ScreenManager::GetModesetBuffer() we need to check controller's
DrmDevice and only re-use primary buffer if it matches the DrmDevice
on which the buffer is allocated.

Also in DrmWindow::SchedulePageFlip(), we need to force buffer
reallocation when the buffer's DrmDevice doesn't match that of the
hardware display controller.

BUG=591368
TEST=verify on auron_yuna with UDL resolution at 1920x1080 and switch primary

Committed: https://crrev.com/28c0dd4fc82d8834c0508492fc693204da39e21b
Cr-Commit-Position: refs/heads/master@{#392165}

[hshi, 2016-05-06 18:42:27]

Description was changed from

==========
DrmWindow: force buffer reallocation when DrmDevice doesn't match

BUG=
==========

to

==========
Fix black screen when switching primary display without bounds change

In ScreenManager::GetModesetBuffer() we need to check controller's
DrmDevice and only re-use primary buffer if it matches the DrmDevice
on which the buffer is allocated.

Also in DrmWindow::SchedulePageFlip(), we need to force buffer
reallocation when the buffer's DrmDevice doesn't match that of the
hardware display controller.

BUG=591368
TEST=verify on auron_yuna with UDL resolution at 1920x1080 and switch primary
==========

[hshi, 2016-05-06 18:49:05]

hshi@google.com changed reviewers:
+ dnicoara@chromium.org, hshi@google.com, ihf@chromium.org, marcheu@chromium.org

[hshi, 2016-05-06 18:49:06]

PTAL. I have verified that this is finally working. The trick is to also add a
DrmDevice check in addition to the size check in
ScreenManager::GetModesetBuffer().

dnicoara@ (OWNER)
marcheu@ and ihf@ (FYI)

[dnicoara, 2016-05-06 19:11:04]

You'll need to update the MockScanoutBuffer implementation (see the failing
bot).

Also, would you mind adding some unittests (ScreenManager nad DrmWindow).

[dnicoara, 2016-05-06 19:17:21]

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

https://codereview.chromium.org/1780013002/diff/20001/ui/ozone/platform/drm/g...
ui/ozone/platform/drm/gpu/drm_window.cc:129: if (plane.buffer &&
plane.buffer->GetDrmDevice() != drm) {
If I understood this correctly this is only required because there may be
in-flight buffers. Could you please add a comment mentioning this. At first this
isn't obvious since |force_buffer_reallocation_| is set to true during the
window bounds update.

[dnicoara, 2016-05-06 19:19:06]

On 2016/05/06 19:17:21, dnicoara wrote:
>
https://codereview.chromium.org/1780013002/diff/20001/ui/ozone/platform/drm/g...
> File ui/ozone/platform/drm/gpu/drm_window.cc (right):
> 
>
https://codereview.chromium.org/1780013002/diff/20001/ui/ozone/platform/drm/g...
> ui/ozone/platform/drm/gpu/drm_window.cc:129: if (plane.buffer &&
> plane.buffer->GetDrmDevice() != drm) {
> If I understood this correctly this is only required because there may be
> in-flight buffers. Could you please add a comment mentioning this. At first
this
> isn't obvious since |force_buffer_reallocation_| is set to true during the
> window bounds update.

Actually, perhaps we could even remove |force_buffer_reallocation_| since we're
always checking in SchedulePageFlip.

[hshi, 2016-05-06 19:19:14]

On 2016/05/06 19:17:21, dnicoara wrote:
>
https://codereview.chromium.org/1780013002/diff/20001/ui/ozone/platform/drm/g...
> File ui/ozone/platform/drm/gpu/drm_window.cc (right):
> 
>
https://codereview.chromium.org/1780013002/diff/20001/ui/ozone/platform/drm/g...
> ui/ozone/platform/drm/gpu/drm_window.cc:129: if (plane.buffer &&
> plane.buffer->GetDrmDevice() != drm) {
> If I understood this correctly this is only required because there may be
> in-flight buffers. Could you please add a comment mentioning this. At first
this
> isn't obvious since |force_buffer_reallocation_| is set to true during the
> window bounds update.

Ok will do. I have debugged this and most of the time there are 1 or 2 in-flight
buffers so this will be necessary

[hshi, 2016-05-06 19:22:31]

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

https://codereview.chromium.org/1780013002/diff/20001/ui/ozone/platform/drm/g...
ui/ozone/platform/drm/gpu/drm_window.cc:129: if (plane.buffer &&
plane.buffer->GetDrmDevice() != drm) {
On 2016/05/06 19:17:21, dnicoara wrote:
> If I understood this correctly this is only required because there may be
> in-flight buffers. Could you please add a comment mentioning this. At first
this
> isn't obvious since |force_buffer_reallocation_| is set to true during the
> window bounds update.

Done.

[dnicoara, 2016-05-06 19:37:15]

lgtm

[hshi1, 2016-05-06 21:36:09]

The CQ bit was checked by hshi@chromium.org

[hshi1, 2016-05-06 21:36:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
dnicoara@chromium.org
Link to the patchset: https://codereview.chromium.org/1780013002/#ps80001
(title: "Fix build failure in MockScanoutBuffer (again! sorry)")

[commit-bot: I haz the power, 2016-05-06 21:36:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1780013002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1780013002/80001

[commit-bot: I haz the power, 2016-05-06 21:40:17]

Description was changed from

==========
Fix black screen when switching primary display without bounds change

In ScreenManager::GetModesetBuffer() we need to check controller's
DrmDevice and only re-use primary buffer if it matches the DrmDevice
on which the buffer is allocated.

Also in DrmWindow::SchedulePageFlip(), we need to force buffer
reallocation when the buffer's DrmDevice doesn't match that of the
hardware display controller.

BUG=591368
TEST=verify on auron_yuna with UDL resolution at 1920x1080 and switch primary
==========

to

==========
Fix black screen when switching primary display without bounds change

In ScreenManager::GetModesetBuffer() we need to check controller's
DrmDevice and only re-use primary buffer if it matches the DrmDevice
on which the buffer is allocated.

Also in DrmWindow::SchedulePageFlip(), we need to force buffer
reallocation when the buffer's DrmDevice doesn't match that of the
hardware display controller.

BUG=591368
TEST=verify on auron_yuna with UDL resolution at 1920x1080 and switch primary
==========

[commit-bot: I haz the power, 2016-05-06 21:40:18]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-06 21:43:03]

Description was changed from

==========
Fix black screen when switching primary display without bounds change

In ScreenManager::GetModesetBuffer() we need to check controller's
DrmDevice and only re-use primary buffer if it matches the DrmDevice
on which the buffer is allocated.

Also in DrmWindow::SchedulePageFlip(), we need to force buffer
reallocation when the buffer's DrmDevice doesn't match that of the
hardware display controller.

BUG=591368
TEST=verify on auron_yuna with UDL resolution at 1920x1080 and switch primary
==========

to

==========
Fix black screen when switching primary display without bounds change

In ScreenManager::GetModesetBuffer() we need to check controller's
DrmDevice and only re-use primary buffer if it matches the DrmDevice
on which the buffer is allocated.

Also in DrmWindow::SchedulePageFlip(), we need to force buffer
reallocation when the buffer's DrmDevice doesn't match that of the
hardware display controller.

BUG=591368
TEST=verify on auron_yuna with UDL resolution at 1920x1080 and switch primary

Committed: https://crrev.com/28c0dd4fc82d8834c0508492fc693204da39e21b
Cr-Commit-Position: refs/heads/master@{#392165}
==========

[commit-bot: I haz the power, 2016-05-06 21:43:04]

Patchset 5 (id:??) landed as
https://crrev.com/28c0dd4fc82d8834c0508492fc693204da39e21b
Cr-Commit-Position: refs/heads/master@{#392165}