Ozone: HDC should only keep track of planes owned by it’s crtc controllers

Ozone: HDC should only keep track of planes owned by it’s crtc controllers

Hardware Display Controller keeps track of planes owned by crtc controller,
even after it’s removed. This can result in unbinding the frame buffer of
primary plane for that controller in subsequent page flip call. After this
any page flip requests for that crtc controller will fail with error
EBUSY(Tested 3.18 + kernel), assuming that user space is still handling some
hot plug events.

I encountered this when connected to an external display and changing between
extended/mirrored mode. Frame buffer associated with primary plane of the
extended display was Zero and Kernel started reporting EBUSY.

Now we make sure that Hardware Display Controller only keeps track of
the planes owned by its controllers.

Committed: https://crrev.com/8e36b241b80f875cf72b90798b65a106a76ace75
Cr-Commit-Position: refs/heads/master@{#344261}

[kalyank, 2015-08-07 20:32:51]

kalyan.kondapally@intel.com changed reviewers:
+ dnicoara@chromium.org

[kalyank, 2015-08-07 20:33:20]

kalyan.kondapally@intel.com changed reviewers:
+ achaulk@chromium.org, alexst@chromium.org

[dnicoara, 2015-08-10 20:40:40]

I think you mean fb as in framebuffer rather than fd (just use the long form in
descriptions as it will make it easier for readers). As for the change, I'll let
Albert do a thorough review since he is way more familiar with the logic in
plane manager.

[kalyank, 2015-08-10 20:45:53]

On 2015/08/10 20:40:40, dnicoara wrote:
> I think you mean fb as in framebuffer rather than fd (just use the long form
in
> descriptions as it will make it easier for readers).

updated description

[achaulk, 2015-08-10 20:55:55]

This doesn't seem to have anything to do with the framebuffer though. The only
effect of this change is to change the else branch from

!hw_plane->is_dummy() && !plane_list->legacy_page_flips.empty() &&
      plane_list->legacy_page_flips.back().crtc_id == crtc_id to just
!hw_plane->is_dummy()

However, since the planes list is used only for overlays planes, I can't see how
this would affect regular display, and its only effect could be to add the
primary plane as an overlay to its own page flip, if it happened to show up as a
non-dummy plane

[kalyank, 2015-08-10 21:33:59]

On 2015/08/10 20:55:55, achaulk wrote:
> This doesn't seem to have anything to do with the framebuffer though. The only
> effect of this change is to change the else branch from
> 
> !hw_plane->is_dummy() && !plane_list->legacy_page_flips.empty() &&
>       plane_list->legacy_page_flips.back().crtc_id == crtc_id to just
> !hw_plane->is_dummy()
> 
> However, since the planes list is used only for overlays planes, I can't see
how
> this would affect regular display, and its only effect could be to add the
> primary plane as an overlay to its own page flip, if it happened to show up as
a
> non-dummy plane

We call ScheduleOverlayPlane in GBMPixmap, which will inform the appropriate
DrmWindow, which will add the overlay to its pending list. During page flip
call, window will pass the information as to what planes need to be committed to
Hardware Controller. I don't think we differentiate if a plane is primary or
overlay anywhere (Unless I am missing something here).

[achaulk, 2015-08-10 21:55:03]

On 2015/08/10 21:33:59, kalyank wrote:
> On 2015/08/10 20:55:55, achaulk wrote:
> > This doesn't seem to have anything to do with the framebuffer though. The
only
> > effect of this change is to change the else branch from
> > 
> > !hw_plane->is_dummy() && !plane_list->legacy_page_flips.empty() &&
> >       plane_list->legacy_page_flips.back().crtc_id == crtc_id to just
> > !hw_plane->is_dummy()
> > 
> > However, since the planes list is used only for overlays planes, I can't see
> how
> > this would affect regular display, and its only effect could be to add the
> > primary plane as an overlay to its own page flip, if it happened to show up
as
> a
> > non-dummy plane
> 
> We call ScheduleOverlayPlane in GBMPixmap, which will inform the appropriate
> DrmWindow, which will add the overlay to its pending list. During page flip
> call, window will pass the information as to what planes need to be committed
to
> Hardware Controller. I don't think we differentiate if a plane is primary or
> overlay anywhere (Unless I am missing something here).

Primary planes are set via drmModePageFlip and overlays via drmModeSetPlane. The
plane at 0 z-index gets assigned to the primary plane for a crtc. Sometimes the
primary planes for a crtc aren't present in resource queries, so we create dummy
planes in those cases

[kalyank, 2015-08-10 23:11:10]

On 2015/08/10 21:55:03, achaulk wrote:
> On 2015/08/10 21:33:59, kalyank wrote:
> > On 2015/08/10 20:55:55, achaulk wrote:
> > > This doesn't seem to have anything to do with the framebuffer though. The
> only
> > > effect of this change is to change the else branch from
> > > 
> > > !hw_plane->is_dummy() && !plane_list->legacy_page_flips.empty() &&
> > >       plane_list->legacy_page_flips.back().crtc_id == crtc_id to just
> > > !hw_plane->is_dummy()
> > > 
> > > However, since the planes list is used only for overlays planes, I can't
see
> > how
> > > this would affect regular display, and its only effect could be to add the
> > > primary plane as an overlay to its own page flip, if it happened to show
up
> as
> > a
> > > non-dummy plane
> > 
> > We call ScheduleOverlayPlane in GBMPixmap, which will inform the appropriate
> > DrmWindow, which will add the overlay to its pending list. During page flip
> > call, window will pass the information as to what planes need to be
committed
> to
> > Hardware Controller. I don't think we differentiate if a plane is primary or
> > overlay anywhere (Unless I am missing something here).
> 
> Primary planes are set via drmModePageFlip and overlays via drmModeSetPlane.
The
> plane at 0 z-index gets assigned to the primary plane for a crtc. Sometimes
the
> primary planes for a crtc aren't present in resource queries, so we create
dummy
> planes in those cases


Ya,sorry what I meant is we don't differentiate between a primary or overlay
while setting the frame buffer to zero.

What I am seeing is this:
In HardwareDisplayManagerLegacy commit call: Depending on
PageFlipInfo(legacy_page_flips), we set framebuffer to overlay as needed and
update the primary plane frame buffer with page flip call.
After that, we go through all old_plane_list(Which contains list of all
HardwarePlanes committed last time), check if the plane is not in use and
unbound it's frame buffer.  

With latest kernel, in drm_mode_page_flip_ioctl call, it first checks if a frame
buffer is already bound to the  primary plane of crtc. If a frame buffer is not
bound, then it returns EBUSY thinking that frame buffer is currently unbound due
to some hot plug events not yet discovered by user space.

I will try to add here more info about exact chain of events, so it becomes
clear.

[kalyank, 2015-08-11 16:22:54]

HDC = Hardware Display Controller
CRTC = CrtcController

Here is my observation as to how many HDC we have, add/remove CRTC to them,
Owned hardware planes of HDC and as to how we end up in this situation.

1) Initial State (No external display attached):
    HDC A
       -- CRTC 1 (Internal display)
       ---Owned hardware planes = All planes owned by CRTC 1 

2) When device is connected to  external display:
    HDC A
        -- CRTC 1 (Internal display)
        -- CRTC 2 (External display) 
        -- Owned hardware planes = All planes owned by CRTC 1 + All planes owned
by 
                                                      CRTC2
   HDC B
        -- None
  We create another HDC B and add CRTC 2 to it. However, we end up using HDC A
as mirror controller.
  CRTC 2 is removed from HDC B and added to HDC A. Origin for both HDC  A & B is
0,0. 

3)Stop mirroring from Display Settings(Still connected to external display)
  HDC A
      -- CRTC 1 (Internal display)
      -- Owned hardware planes = All planes owned by CRTC1 + All planes owned 
                                 by CRTC 2 (? For at least 1 frame after
mirroring stops)
  HDC B
      -- None

  HDC C
      -- CRTC 2 (External display)
In this case the origin has changed (0, 1040) for CRTC 2 and we end up creating
another 
HDC C in ActualConfigureDisplayController call. We also remove CRTC 2 from HDC A
and add it to HDC C. 
When RemoveCrtc is called for HDC A, we don’t delete owned_hardware_planes_ as
it’s shared 
with CRTC1 (Same DRMDevice).

The last flip before mirroring is stopped, all planes owned by CRTC 1 & 2 are
tracked by HDC A as part of old_plane_list(Like every commit call). When
mirroring is stopped,  CRTC 2 is removed from HDC A and added to HDC C. HDC A
receives a request to schedule page flip, here we call BeginFrame in
HardwareDisplayManager, which marks all the planes in old_plane_list as unused
(Remember it contains planes owned by CRTC 1 and 2) . After this HDC A requests
CRTC 1 to schedule a page flip. Now, all planes oowned by CRTC 1 are marked as
in use. HDC A then asks Hardware Display Manager to commit all the planes.
During commit, we check if we have any planes which are part of old_plane_list
and unused . If so, we unbound frame buffer for that plane. Thus, all the
framebuffers for planes owned by CRTC 2 are unbound here. Now, when HDC C gets a
request to schedule its page flip, we end up trying to page flip without having
a frame buffer bound to the primary plane of CRTC 2.

Should we remove planes owned by CRTC2 from the list in HDC A (In RemoveCrtc
call) ?

[kalyank, 2015-08-12 16:50:08]

@dnicoara can you please take another look as issue seems to be in controller.

[achaulk, 2015-08-12 17:15:06]

Hmm I think I see the problem now. One HDC will clear the state for all HDCs. We
may need to track how many HDCs are connected and only commit when all of them
have run

[kalyank, 2015-08-12 17:51:01]

On 2015/08/12 17:15:06, achaulk wrote:
> Hmm I think I see the problem now. One HDC will clear the state for all HDCs.
We
> may need to track how many HDCs are connected and only commit when all of them
> have run

Isn't it enough that when a CRTC is removed, we make sure that HDC doesn't keep
track of
planes owned by that CRTC ? I dont fully understand as to why we want to keep
track of all
HDC and do the initial commit only after they have run.

[achaulk, 2015-08-12 20:42:02]

On 2015/08/12 17:51:01, kalyank wrote:
> On 2015/08/12 17:15:06, achaulk wrote:
> > Hmm I think I see the problem now. One HDC will clear the state for all
HDCs.
> We
> > may need to track how many HDCs are connected and only commit when all of
them
> > have run
> 
> Isn't it enough that when a CRTC is removed, we make sure that HDC doesn't
keep
> track of
> planes owned by that CRTC ? I dont fully understand as to why we want to keep
> track of all
> HDC and do the initial commit only after they have run.

Possibly, but in the case of a CRTC being removed and not added somewhere (if
this happens) it wouldn't blank the screen

[kalyank, 2015-08-14 08:20:19]

On 2015/08/12 20:42:02, achaulk wrote:
> On 2015/08/12 17:51:01, kalyank wrote:
> > On 2015/08/12 17:15:06, achaulk wrote:
> Possibly, but in the case of a CRTC being removed and not added somewhere (if
> this happens) it wouldn't blank the screen

Now, we make sure that the planes are removed from the list only if controller 
moves to another HDC and is enabled successfully. Also, we track the planes
owned by that CRTC in new HDC. Can you PTAL.

[dnicoara, 2015-08-14 14:09:45]

I'm disliking the fact that ScreenManager now needs to explicitly take care of
plane management (it is easy to miss a corner case, makes ScreenManager more
complex, and adds extra dependencies in ScreenManager).

Also, Albert, I think the case you've mentioned is a non issue as there are 3
possible scenarios when a CRTC is removed from a HDC:
 1) moving to extended/mirror mode in which case the new HDC will modeset the
CRTC appropriately,
 2) CRTC is disabled in which case the new HDC will simply disable the CRTC,
 3) CRTC is disconnected and the destructor for the CRTC will call
drmModeSetCrtc and blank the CRTC.

I think the cleanest way is to have a clear separation on which planes the HDC
operates on. As HDC is meant as a composite, it should only operate on the CRTC
objects it owns (and planes associated with said CRTCs). If we need to perform
extra operations for clean-up purposes, we should rely on the calls to
Modeset/Disable the CRTC/HDC to perform these operations.

[kalyank, 2015-08-14 20:19:44]

On 2015/08/14 14:09:45, dnicoara wrote:
> I'm disliking the fact that ScreenManager now needs to explicitly take care of
> plane management (it is easy to miss a corner case, makes ScreenManager more
> complex, and adds extra dependencies in ScreenManager).
> 
> Also, Albert, I think the case you've mentioned is a non issue as there are 3
> possible scenarios when a CRTC is removed from a HDC:
>  1) moving to extended/mirror mode in which case the new HDC will modeset the
> CRTC appropriately,
>  2) CRTC is disabled in which case the new HDC will simply disable the CRTC,
>  3) CRTC is disconnected and the destructor for the CRTC will call
> drmModeSetCrtc and blank the CRTC.
> 
> I think the cleanest way is to have a clear separation on which planes the HDC
> operates on. As HDC is meant as a composite, it should only operate on the
CRTC
> objects it owns (and planes associated with said CRTCs). If we need to perform
> extra operations for clean-up purposes, we should rely on the calls to
> Modeset/Disable the CRTC/HDC to perform these operations.

This should be fine in case we are doing a full modeset i.e. CRTC disabled,
pixel formats
changed or fb set to null during modeset. 

If we have a case where two planes are enabled on CRTC1 i.e. Video and Primary.
Now it's moved 
to another HDC(We dont do a full modeset here), in the next frame if only
Primary is in use for 
CRTC1, Video plane will not get disabled.

Are you thinking that when CRTC is removed from HDC, it should stop tracking
planes owned by that
CRTC and disable that CRTC. When it's moved to another HDC, it gets enabled by
ScreenManager(We do
this part any way now).

[dnicoara, 2015-08-14 21:53:34]

On 2015/08/14 20:19:44, kalyank wrote:
> On 2015/08/14 14:09:45, dnicoara wrote:
> > I'm disliking the fact that ScreenManager now needs to explicitly take care
of
> > plane management (it is easy to miss a corner case, makes ScreenManager more
> > complex, and adds extra dependencies in ScreenManager).
> > 
> > Also, Albert, I think the case you've mentioned is a non issue as there are
3
> > possible scenarios when a CRTC is removed from a HDC:
> >  1) moving to extended/mirror mode in which case the new HDC will modeset
the
> > CRTC appropriately,
> >  2) CRTC is disabled in which case the new HDC will simply disable the CRTC,
> >  3) CRTC is disconnected and the destructor for the CRTC will call
> > drmModeSetCrtc and blank the CRTC.
> > 
> > I think the cleanest way is to have a clear separation on which planes the
HDC
> > operates on. As HDC is meant as a composite, it should only operate on the
> CRTC
> > objects it owns (and planes associated with said CRTCs). If we need to
perform
> > extra operations for clean-up purposes, we should rely on the calls to
> > Modeset/Disable the CRTC/HDC to perform these operations.
> 
> This should be fine in case we are doing a full modeset i.e. CRTC disabled,
> pixel formats
> changed or fb set to null during modeset. 
> 
> If we have a case where two planes are enabled on CRTC1 i.e. Video and
Primary.
> Now it's moved 
> to another HDC(We dont do a full modeset here), in the next frame if only
> Primary is in use for 
> CRTC1, Video plane will not get disabled.
> 
> Are you thinking that when CRTC is removed from HDC, it should stop tracking
> planes owned by that
> CRTC and disable that CRTC. When it's moved to another HDC, it gets enabled by
> ScreenManager(We do
> this part any way now).

Yes, when the CRTC is removed from HDC1, HDC1 should stop tracking the planes
owned by the CRTC. Though at this point HDC1 shouldn't assume anything about the
CRTC (as in it shouldn't disable the CRTC or any of its associated resources).
At this point the CRTC could be added to HDC2 or deleted. When HDC2 modesets the
CRTC it should perform proper cleanup and disable any unused planes. If the CRTC
is deleted, the disable operation should perform the appropriate cleanup.

I think that HardwareDisplayPlaneManager has enough information to know which
plane is associated with which CRTC, so it may be that the modeset operations
should flow through it so that unused planes are correctly disabled. Albert
please correct me if I've missed something.

[kalyank, 2015-08-15 18:26:16]

On 2015/08/14 21:53:34, dnicoara wrote:
> On 2015/08/14 20:19:44, kalyank wrote:
> > On 2015/08/14 14:09:45, dnicoara wrote:
> Yes, when the CRTC is removed from HDC1, HDC1 should stop tracking the planes
> owned by the CRTC. Though at this point HDC1 shouldn't assume anything about
the
> CRTC (as in it shouldn't disable the CRTC or any of its associated resources).
> At this point the CRTC could be added to HDC2 or deleted. When HDC2 modesets
the
> CRTC it should perform proper cleanup and disable any unused planes. If the
CRTC
> is deleted, the disable operation should perform the appropriate cleanup.
> 
> I think that HardwareDisplayPlaneManager has enough information to know which
> plane is associated with which CRTC, so it may be that the modeset operations
> should flow through it so that unused planes are correctly disabled. Albert
> please correct me if I've missed something.

Now, we make sure HDC internally keeps it's list up to date. When CRTC is
removed from HDC, 
we remove list of planes owned by that CRTC from it's list. During Modeset call,
 we refresh 
the list of HDC w.r.t all Controllers it has at that point. This avoids
disabling planes
only to re-enable them in next page flip call(As needed).

[dnicoara, 2015-08-18 17:07:52]

Also, could you please add some unittests to make sure this doesn't regress in
the future?

https://codereview.chromium.org/1279703004/diff/100001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/gpu/hardware_display_controller.cc (right):

https://codereview.chromium.org/1279703004/diff/100001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/gpu/hardware_display_controller.cc:48: // CRTC controllers
owned by this HDC might have changed, refresh the list.
Since the RemoveCrtc() all updates the list for the old HDC, couldn't you add
the planes for new controllers in AddCrtc()?

[kalyank, 2015-08-18 23:15:40]

https://codereview.chromium.org/1279703004/diff/100001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/gpu/hardware_display_controller.cc (right):

https://codereview.chromium.org/1279703004/diff/100001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/gpu/hardware_display_controller.cc:48: // CRTC controllers
owned by this HDC might have changed, refresh the list.
On 2015/08/18 17:07:52, dnicoara wrote:
> Since the RemoveCrtc() all updates the list for the old HDC, couldn't you add
> the planes for new controllers in AddCrtc()?

Done.

[kalyank, 2015-08-18 23:16:13]

On 2015/08/18 17:07:52, dnicoara wrote:
> Also, could you please add some unittests to make sure this doesn't regress in
> the future?

Done. 

>
https://codereview.chromium.org/1279703004/diff/100001/ui/ozone/platform/drm/...
> File ui/ozone/platform/drm/gpu/hardware_display_controller.cc (right):
> 
>
https://codereview.chromium.org/1279703004/diff/100001/ui/ozone/platform/drm/...
> ui/ozone/platform/drm/gpu/hardware_display_controller.cc:48: // CRTC
controllers
> owned by this HDC might have changed, refresh the list.
> Since the RemoveCrtc() all updates the list for the old HDC, couldn't you add
> the planes for new controllers in AddCrtc()?

[dnicoara, 2015-08-19 14:09:44]

https://codereview.chromium.org/1279703004/diff/160001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/gpu/hardware_display_controller.cc (right):

https://codereview.chromium.org/1279703004/diff/160001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/gpu/hardware_display_controller.cc:155:
owned_hardware_planes_.add(drm.get(), crtc_plane_list.Pass());
Move this at the begining and use the result to get the iterator to the
crtc_plane_list.

There's a subtle behavior here as add() will not replace an already existing
element. You'll want to keep that behavior otherwise you'd be losing the planes
for the CRTCs already present.

https://codereview.chromium.org/1279703004/diff/160001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc (right):

https://codereview.chromium.org/1279703004/diff/160001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc:331:
EXPECT_FALSE(controller_->HasPlane(drm_.get(), primary_crtc_plane->plane_id(),
I'm not a fan of test only functions in non-test objects. Could you just test
for behavior? (ie: make sure that when the HDC schedules another page flip, the
planes for the removed CRTC aren't affected)

[kalyank, 2015-08-19 15:38:55]

https://codereview.chromium.org/1279703004/diff/160001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/gpu/hardware_display_controller.cc (right):

https://codereview.chromium.org/1279703004/diff/160001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/gpu/hardware_display_controller.cc:155:
owned_hardware_planes_.add(drm.get(), crtc_plane_list.Pass());
On 2015/08/19 14:09:44, dnicoara wrote:
> Move this at the begining and use the result to get the iterator to the
> crtc_plane_list.
> 
> There's a subtle behavior here as add() will not replace an already existing
> element. You'll want to keep that behavior otherwise you'd be losing the
planes
> for the CRTCs already present.

Acknowledged.

https://codereview.chromium.org/1279703004/diff/160001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/gpu/hardware_display_controller.cc:155:
owned_hardware_planes_.add(drm.get(), crtc_plane_list.Pass());
On 2015/08/19 14:09:44, dnicoara wrote:
> Move this at the begining and use the result to get the iterator to the
> crtc_plane_list.
> 
> There's a subtle behavior here as add() will not replace an already existing
> element. You'll want to keep that behavior otherwise you'd be losing the
planes
> for the CRTCs already present.

Done.

https://codereview.chromium.org/1279703004/diff/160001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc (right):

https://codereview.chromium.org/1279703004/diff/160001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc:331:
EXPECT_FALSE(controller_->HasPlane(drm_.get(), primary_crtc_plane->plane_id(),
On 2015/08/19 14:09:44, dnicoara wrote:
> I'm not a fan of test only functions in non-test objects. Could you just test
> for behavior? (ie: make sure that when the HDC schedules another page flip,
the
> planes for the removed CRTC aren't affected)

Done.

[dnicoara, 2015-08-19 16:08:46]

lgtm with nit

https://codereview.chromium.org/1279703004/diff/180001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc (right):

https://codereview.chromium.org/1279703004/diff/180001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc:351:
primary_crtc_plane->set_owning_crtc(0);
Do you need these 2 calls? set_in_use() is called in
HDPlaneManager::BeginFrame() and set_owning_crtc(0) shouldn't matter.

[kalyank, 2015-08-19 16:15:29]

https://codereview.chromium.org/1279703004/diff/180001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc (right):

https://codereview.chromium.org/1279703004/diff/180001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/gpu/hardware_display_controller_unittest.cc:351:
primary_crtc_plane->set_owning_crtc(0);
On 2015/08/19 16:08:46, dnicoara wrote:
> Do you need these 2 calls? set_in_use() is called in
> HDPlaneManager::BeginFrame() and set_owning_crtc(0) shouldn't matter.
This was to test that the plane was actually added to hdc_controller. If addcrtc
call fails to add the planes for some reason, the checks after page flip call
here should fail.

[kalyank, 2015-08-19 17:23:16]

The CQ bit was checked by kalyan.kondapally@intel.com

[kalyank, 2015-08-19 17:23:17]

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/1279703004/#ps200001
(title: "Add comments to clarify what test is doing")

[commit-bot: I haz the power, 2015-08-19 17:23:45]

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

[commit-bot: I haz the power, 2015-08-19 17:29:05]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2015-08-19 17:29:39]

Patchset 11 (id:??) landed as
https://crrev.com/8e36b241b80f875cf72b90798b65a106a76ace75
Cr-Commit-Position: refs/heads/master@{#344261}