Merge lp:~peat-new/mir/load-fb-hal-before-hwc into lp:mir

[Ratchanan Srirattanamet]

android: open FB HAL before HWComposer.

Some devices (for example Samsung Galaxy Tab 2 7.0) may insist that the
FB HAL be opened before HWC. This revision make HalComponentFactory to
comply to this requirement.

I'm porting Ubuntu Touch to Samsung Galaxy Tab 2 7.0. While porting, I found that this device's HWComposer (OMAP's, and has version 1.0 of API) have to be opened after FB HAL. Looking at Android's SurfaceFlinger code, I found that it does open FB HAL before HWComposer, but then close FB HAL if HWComposer is successfully opened and has version 1.1 of API or later.

This branch changes HalComponentFactory to do essentially the same thing as Android. With this fix, I can confirm that HWComposer is opened successfully on Samsung Galaxy Tab 2 7.0. Although graphic doesn't work yet. (Actually, using FB HAL alone, graphic does work but isn't complete.)

[Daniel van Vugt (vanvugt)]

Just running automated tests first. Unfortunately the only way I know to make that happen is to resubmit the code from a mir-team member:
https://code.launchpad.net/~mir-team/mir/load-fb-hal-before-hwc/+merge/302909

[Kevin DuBois (kdub)]

This has the problem that we have to load the fb hal, even when its not needed. It also seems to be a problem on older devices only, so it causes a bit more work for the bulk of supported devices. So, if we want to support the older devices (like the omap hwc 1.0), this is the only way to do it... so alright by me.

review: Approve

[Kevin DuBois (kdub)]

Obviously there are other problems with drivers that require a particular build order.. but since we can't move that source code (due to old age, and closed-source nature), we probably have to just work around it.

[Ratchanan Srirattanamet (peat-new)]

Actually, we have HWC's code here (highlighting the line that requires FB HAL to be opened first): https://github.com/CyanogenMod/android_hardware_ti_omap4/blob/cm-13.0/hwc/hwc.c#L2529

I have another problem with this HWC, which I'll file a bug because I can't figure out how to fix that properly (I have a hack but it's obviously not a proper fix).

[Daniel van Vugt (vanvugt)]

Still waiting (sorry) for CI ...
https://code.launchpad.net/~mir-team/mir/load-fb-hal-before-hwc/+merge/302909

review: Needs Information

[Daniel van Vugt (vanvugt)]

CI is mostly happy with the branch (only failing for unrelated reasons today).

So I'm happy with it.

review: Approve

[Daniel van Vugt (vanvugt)]

This change got reverted yesterday due to a regression: bug 1615703

Please fix and resubmit under a new branch name (Launchpad won't accept the old name after it already landed once).

review: Needs Resubmitting

[Kevin DuBois (kdub)]

yeah, a quirk that detects on the device name is probably the easiest way to go, although adding hwc-vendor detection would probably be a bit more robust. The device that had problems was the MX4, we can test the resubmission against the device.

review: Abstain

[Cemil Azizoglu (cemil-azizoglu)]

I'll set this to WiP as it's not ready for review. Thanks.

=== modified file 'src/platforms/android/server/hal_component_factory.cpp'
--- src/platforms/android/server/hal_component_factory.cpp	2016-06-02 05:33:50 +0000
+++ src/platforms/android/server/hal_component_factory.cpp	2016-08-14 10:11:51 +0000
@@ -50,6 +50,16 @@
       working_egl_sync(quirks->working_egl_sync()),
       hwc_version{mga::HwcVersion::unknown}
 {
+    bool fb_native_failed_to_load = false;
+
+    try
+    {
+        fb_native = res_factory->create_fb_native_device();
+    } catch (...)
+    {
+        fb_native_failed_to_load = true;
+    }
+
     try
     {
         std::tie(hwc_wrapper, hwc_version) = res_factory->create_hwc_wrapper(hwc_report);
@@ -61,9 +71,24 @@
 
     if (force_backup_display || hwc_version == mga::HwcVersion::hwc10)
     {
-        fb_native = res_factory->create_fb_native_device();
-        //guarantee always 2 fb's allocated
-        num_framebuffers = std::max(2u, static_cast<unsigned int>(fb_native->numFramebuffers));
+        //We're using FB HAL
+        if (fb_native_failed_to_load)
+        {
+            BOOST_THROW_EXCEPTION(std::runtime_error("fb display is mandory but failed to create one"));
+        }
+        else
+        {
+            //get framebuffers amount from FB HAL but guarantee always 2 fb's allocated
+            num_framebuffers = std::max(2u, static_cast<unsigned int>(fb_native->numFramebuffers));
+        }
+    }
+    else
+    {
+        if (!fb_native_failed_to_load)
+        {
+            //Close FB HAL as we don't need it.
+            fb_native.reset(); //This will close the FB HAL via destructor
+        }
     }
 
     command_stream_sync_factory = create_command_stream_sync_factory();

=== modified file 'tests/unit-tests/graphics/android/test_output_builder.cpp'
--- tests/unit-tests/graphics/android/test_output_builder.cpp	2016-06-02 05:33:50 +0000
+++ tests/unit-tests/graphics/android/test_output_builder.cpp	2016-08-14 10:11:51 +0000
@@ -94,9 +94,12 @@
 TEST_F(HalComponentFactory, builds_hwc_version_10)
 {
     using namespace testing;
+    Sequence seq;
+    EXPECT_CALL(*mock_resource_factory, create_fb_native_device())
+        .InSequence(seq);
     EXPECT_CALL(*mock_resource_factory, create_hwc_wrapper(_))
+        .InSequence(seq)
         .WillOnce(Return(std::make_tuple(mock_wrapper, mga::HwcVersion::hwc10)));
-    EXPECT_CALL(*mock_resource_factory, create_fb_native_device());
     EXPECT_CALL(*mock_hwc_report, report_hwc_version(mga::HwcVersion::hwc10));
 
     mga::HalComponentFactory factory(
@@ -109,7 +112,11 @@
 TEST_F(HalComponentFactory, builds_hwc_version_11_and_later)
 {
     using namespace testing;
+    Sequence seq;
+    EXPECT_CALL(*mock_resource_factory, create_fb_native_device())
+        .InSequence(seq);
     EXPECT_CALL(*mock_resource_factory, create_hwc_wrapper(_))
+        .InSequence(seq)
         .WillOnce(Return(std::make_tuple(mock_wrapper, mga::HwcVersion::hwc11)));
     EXPECT_CALL(*mock_hwc_report, report_hwc_version(mga::HwcVersion::hwc11));
 
@@ -137,9 +144,12 @@
 TEST_F(HalComponentFactory, hwc_failure_falls_back_to_fb)
 {
     using namespace testing;
+    Sequence seq;
+    EXPECT_CALL(*mock_resource_factory, create_fb_native_device())
+        .InSequence(seq);
     EXPECT_CALL(*mock_resource_factory, create_hwc_wrapper(_))
+        .InSequence(seq)
         .WillOnce(Throw(std::runtime_error("")));
-    EXPECT_CALL(*mock_resource_factory, create_fb_native_device());
     EXPECT_CALL(*mock_hwc_report, report_legacy_fb_module());
 
     mga::HalComponentFactory factory(
@@ -152,9 +162,12 @@
 TEST_F(HalComponentFactory, hwc_and_fb_failure_fatal)
 {
     using namespace testing;
+    Sequence seq;
+    EXPECT_CALL(*mock_resource_factory, create_fb_native_device())
+        .InSequence(seq)
+        .WillOnce(Throw(std::runtime_error("")));
     EXPECT_CALL(*mock_resource_factory, create_hwc_wrapper(_))
-        .WillOnce(Throw(std::runtime_error("")));
-    EXPECT_CALL(*mock_resource_factory, create_fb_native_device())
+        .InSequence(seq)
         .WillOnce(Throw(std::runtime_error("")));
 
     EXPECT_THROW({
@@ -169,11 +182,14 @@
 TEST_F(HalComponentFactory, determine_fbnum_always_reports_2_minimum)
 {
     using namespace testing;
-    EXPECT_CALL(*mock_resource_factory, create_hwc_wrapper(_))
-        .WillOnce(Throw(std::runtime_error("")));
+    Sequence seq;
     EXPECT_CALL(*mock_resource_factory, create_fb_native_device())
+        .InSequence(seq)
         .WillOnce(Return(std::make_shared<mtd::MockFBHalDevice>(
             0, 0, mir_pixel_format_abgr_8888, 0, 0.0, 0.0)));
+    EXPECT_CALL(*mock_resource_factory, create_hwc_wrapper(_))
+        .InSequence(seq)
+        .WillOnce(Throw(std::runtime_error("")));
 
     mga::HalComponentFactory factory(
         mock_resource_factory,
@@ -192,11 +208,11 @@
 {
     using namespace testing;
     auto supported_versions = {
-        mga::HwcVersion::hwc10, 
-        mga::HwcVersion::hwc11, 
-        mga::HwcVersion::hwc12, 
-        mga::HwcVersion::hwc13, 
-        mga::HwcVersion::hwc14, 
+        mga::HwcVersion::hwc10,
+        mga::HwcVersion::hwc11,
+        mga::HwcVersion::hwc12,
+        mga::HwcVersion::hwc13,
+        mga::HwcVersion::hwc14,
         mga::HwcVersion::hwc15 };
     for (auto supported_version : supported_versions)
     {