Merge into development-branch : mesa-hybrid-cursor : Code : Mir

Status:	Merged
Approved by:	Alan Griffiths on 2017-06-09
Approved revision:	no longer in the source branch.
Merged at revision:	4193
Proposed branch:	lp:~alan-griffiths/mir/mesa-hybrid-cursor
Merge into:	lp:mir
Diff against target:	547 lines (+259/-63) 7 files modified src/common/CMakeLists.txt (+2/-1) src/platforms/mesa/server/kms/CMakeLists.txt (+1/-0) src/platforms/mesa/server/kms/cursor.cpp (+117/-31) src/platforms/mesa/server/kms/cursor.h (+21/-7) src/platforms/mesa/server/kms/display.cpp (+3/-14) src/platforms/mesa/server/kms/mutex.h (+104/-0) tests/unit-tests/platforms/mesa/kms/test_cursor.cpp (+11/-10)
To merge this branch:	bzr merge lp:~alan-griffiths/mir/mesa-hybrid-cursor
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Mir CI Bot	continuous-integration	2017-06-07	Approve on 2017-06-09
Chris Halse Rogers			Approve on 2017-06-08
Alberto Aguirre (community)			Approve on 2017-06-08
Alan Griffiths			Pending
Review via email: mp+325215@code.launchpad.net

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-03-22: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3208/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-03-23: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3218/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-03-26: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3247/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-03-29: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3273/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-04-07: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3350/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2017-06-06: Posted in a previous version of this proposal

#

It isn't clear why this failed CI (and too much time has passed to see any logs).

It still merges and doesn't obviously break things - so is it still worth landing?

review: Needs Information

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-06-06: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3443/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2017-06-07: Posted in a previous version of this proposal

#

It's still worth merging. Once it builds, it seems!

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-06-07:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3446/rebuild

PASSED: Continuous integration, rev:4191
https://mir-jenkins.ubuntu.com/job/mir-ci/3446/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4695
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4830
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/4820
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4820
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4820
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4732
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4732/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4732
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4732/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4732
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4732/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4732
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4732/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4732
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4732/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4732
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4732/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4732
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4732/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4732
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4732/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3446/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2017-06-07:

#

+{
87 + const_cast<gbm_bo*&>(from.buffer) = nullptr;
88 + const_cast<gbm_device*&>(from.device) = nullptr;

Seems like undefined behavior.

Use a unique_ptr with custom deleter instead and move the unique_ptrs on move constructor.

165 + memset(dest, 0, buffer_stride * (gbm_bo_get_height(buffer) - image_height));

"gbm_bo_get_height(buffer) - image_height" could be negative and implicitly converted to size_t (so a huge number - Needs a check.

review: Needs Fixing

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2017-06-08:

#

> +{
> 87 + const_cast<gbm_bo*&>(from.buffer) = nullptr;
> 88 + const_cast<gbm_device*&>(from.device) = nullptr;
>
> Seems like undefined behavior.

Fixed

> 165 + memset(dest, 0, buffer_stride * (gbm_bo_get_height(buffer) -
> image_height));
>
> "gbm_bo_get_height(buffer) - image_height" could be negative and implicitly
> converted to size_t (so a huge number - Needs a check.

Unless I'm missing something (always possible) there is a check Vis:

    if (image_width > min_buffer_width || image_height > min_buffer_height)
    {
        BOOST_THROW_EXCEPTION(std::logic_error("Image is too big for GBM cursor buffer"));
    }

AIUI min_buffer_height ought to be <= gbm_bo_get_height(buffer)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-06-08:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3448/rebuild

PASSED: Continuous integration, rev:4192
https://mir-jenkins.ubuntu.com/job/mir-ci/3448/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4699
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4836
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/4826
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4826
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4826
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4736
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4736/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4736
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4736/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4736
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4736/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4736
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4736/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4736
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4736/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4736
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4736/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4736
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4736/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4736
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4736/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3448/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2017-06-08:

#

Oh right... it was just a convoluted way of checking yeah.

review: Approve

Revision history for this message

Mir CI Bot (mir-ci-bot) on 2017-06-08:

#

review: Approve (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-06-08:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2017-06-08:

#

LGTM.

FWIW, this seems to be against our style guide:
191 + {
192 + pad_and_write_image_data_locked(lg, buffer, cursor_image);
193 + } else
194 + {

review: Approve

Revision history for this message

Mir CI Bot (mir-ci-bot) on 2017-06-09:

#

review: Approve (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-06-09:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-06-09:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-06-09:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3451/rebuild

PASSED: Continuous integration, rev:4193
https://mir-jenkins.ubuntu.com/job/mir-ci/3451/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4707
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4846
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/4836
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4836
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4836
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4744
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4744/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4744
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4744/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4744
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4744/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4744
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4744/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4744
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4744/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4744
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4744/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4744
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4744/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4744
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4744/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3451/rebuild

review: Approve (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) on 2017-06-09:

#

review: Approve (continuous-integration)

Mir

Merge lp:~alan-griffiths/mir/mesa-hybrid-cursor into lp:mir

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'src/common/CMakeLists.txt'
 --- src/common/CMakeLists.txt	2017-05-08 03:04:26 +0000
 +++ src/common/CMakeLists.txt	2017-06-09 10:22:51 +0000
@@ -20,7 +20,8 @@
    $<TARGET_OBJECTS:mirtime>
    ${CMAKE_CURRENT_SOURCE_DIR}/output_type_names.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/log.cpp
--  ${CMAKE_CURRENT_SOURCE_DIR}/libname.cpp ${PROJECT_SOURCE_DIR}/include/common/mir/libname.h
++  ${CMAKE_CURRENT_SOURCE_DIR}/libname.cpp
++  ${PROJECT_SOURCE_DIR}/include/common/mir/libname.h
    ${PROJECT_SOURCE_DIR}/include/common/mir/posix_rw_mutex.h
    posix_rw_mutex.cpp
    edid.cpp
 === modified file 'src/platforms/mesa/server/kms/CMakeLists.txt'
 --- src/platforms/mesa/server/kms/CMakeLists.txt	2017-05-17 04:48:46 +0000
 +++ src/platforms/mesa/server/kms/CMakeLists.txt	2017-06-09 10:22:51 +0000
@@ -43,6 +43,7 @@
    real_kms_output_container.cpp
    egl_helper.h
    egl_helper.cpp
++  mutex.h
+ )
  configure_file(${CMAKE_CURRENT_SOURCE_DIR}/symbols.map.in
 === modified file 'src/platforms/mesa/server/kms/cursor.cpp'
 --- src/platforms/mesa/server/kms/cursor.cpp	2017-05-08 03:04:26 +0000
 +++ src/platforms/mesa/server/kms/cursor.cpp	2017-06-09 10:22:51 +0000
@@ -88,34 +88,74 @@
         drmGetCap(fd, DRM_CAP_CURSOR_WIDTH, &width);
      return int(width);
+ }
--}
--
--mgm::Cursor::GBMBOWrapper::GBMBOWrapper(gbm_device* gbm) :
--    buffer(gbm_bo_create(
--                gbm,
--                get_drm_cursor_width(gbm_device_get_fd(gbm)),
--                get_drm_cursor_height(gbm_device_get_fd(gbm)),
--                GBM_FORMAT_ARGB8888,
--                GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE))
++
++gbm_device* gbm_create_device_checked(int fd)
++{
++    auto device = gbm_create_device(fd);
++    if (!device)
++    {
++        BOOST_THROW_EXCEPTION(std::runtime_error("Failed to create gbm device"));
++    }
++    return device;
++}
++}
++
++mgm::Cursor::GBMBOWrapper::GBMBOWrapper(int fd) :
++    device{gbm_create_device_checked(fd)},
++    buffer{
++        gbm_bo_create(
++            device,
++            get_drm_cursor_width(fd),
++            get_drm_cursor_height(fd),
++            GBM_FORMAT_ARGB8888,
++            GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE)}
+ {
      if (!buffer) BOOST_THROW_EXCEPTION(std::runtime_error("failed to create gbm buffer"));
+ }
--inline mgm::Cursor::GBMBOWrapper::operator gbm_bo*() { return buffer; }
--inline mgm::Cursor::GBMBOWrapper::~GBMBOWrapper()    { gbm_bo_destroy(buffer); }
++inline mgm::Cursor::GBMBOWrapper::operator gbm_bo*()
++{
++    return buffer;
++}
++
++inline mgm::Cursor::GBMBOWrapper::~GBMBOWrapper()
++{
++    if (device)
++        gbm_device_destroy(device);
++    if (buffer)
++        gbm_bo_destroy(buffer);
++}
++
++mgm::Cursor::GBMBOWrapper::GBMBOWrapper(GBMBOWrapper&& from)
++    : device{from.device},
++      buffer{from.buffer}
++{
++    from.buffer = nullptr;
++    from.device = nullptr;
++}
  mgm::Cursor::Cursor(
--    gbm_device* gbm,
      KMSOutputContainer& output_container,
      std::shared_ptr<CurrentConfiguration> const& current_configuration) :
          output_container(output_container),
          current_position(),
          last_set_failed(false),
--        buffer(gbm),
--        buffer_width(gbm_bo_get_width(buffer)),
--        buffer_height(gbm_bo_get_height(buffer)),
++        min_buffer_width{std::numeric_limits<uint32_t>::max()},
++        min_buffer_height{std::numeric_limits<uint32_t>::max()},
          current_configuration(current_configuration)
+ {
++    // Generate the buffers for the initial configuration.
++    current_configuration->with_current_configuration_do(
++        [this](KMSDisplayConfiguration const& kms_conf)
++        {
++            kms_conf.for_each_output(
++                [this, &kms_conf](auto const& output)
++                {
++                    // I'm not sure why g++ needs the explicit "this->" but it does - alan_g
++                    this->buffer_for_output(*kms_conf.get_output_for(output.id));
++                });
++        });
++
      hide();
      if (last_set_failed)
          throw std::runtime_error("Initial KMS cursor set failed");
@@ -126,7 +166,11 @@
      hide();
+ }
--void mgm::Cursor::write_buffer_data_locked(std::lock_guard<std::mutex> const&, void const* data, size_t count)
++void mgm::Cursor::write_buffer_data_locked(
++    std::lock_guard<std::mutex> const&,
++    gbm_bo* buffer,
++    void const* data,
++    size_t count)
+ {
      if (auto result = gbm_bo_write(buffer, data, count))
+     {
@@ -136,20 +180,23 @@
+     }
+ }
--void mgm::Cursor::pad_and_write_image_data_locked(std::lock_guard<std::mutex> const& lg, CursorImage const& image)
++void mgm::Cursor::pad_and_write_image_data_locked(
++    std::lock_guard<std::mutex> const& lg,
++    gbm_bo* buffer,
++    CursorImage const& image)
+ {
      auto image_argb = static_cast<uint8_t const*>(image.as_argb_8888());
      auto image_width = image.size().width.as_uint32_t();
      auto image_height = image.size().height.as_uint32_t();
      auto image_stride = image_width * 4;
--    if (image_width > buffer_width || image_height > buffer_height)
++    if (image_width > min_buffer_width || image_height > min_buffer_height)
+     {
          BOOST_THROW_EXCEPTION(std::logic_error("Image is too big for GBM cursor buffer"));
+     }
--
++
      size_t buffer_stride = gbm_bo_get_stride(buffer);  // in bytes
--    size_t padded_size = buffer_stride * buffer_height;
++    size_t padded_size = buffer_stride * gbm_bo_get_height(buffer);
      auto padded = std::unique_ptr<uint8_t[]>(new uint8_t[padded_size]);
      size_t rhs_padding = buffer_stride - image_stride;
@@ -164,9 +211,9 @@
          src += image_stride;
+     }
--    memset(dest, 0, buffer_stride * (buffer_height - image_height));
++    memset(dest, 0, buffer_stride * (gbm_bo_get_height(buffer) - image_height));
--    write_buffer_data_locked(lg, &padded[0], padded_size);
++    write_buffer_data_locked(lg, buffer, &padded[0], padded_size);
+ }
  void mgm::Cursor::show()
@@ -186,14 +233,21 @@
      auto const& size = cursor_image.size();
--    if (size != geometry::Size{buffer_width, buffer_height})
--    {
--        pad_and_write_image_data_locked(lg, cursor_image);
--    }
--    else
--    {
--        auto const count = size.width.as_uint32_t() * size.height.as_uint32_t() * sizeof(uint32_t);
--        write_buffer_data_locked(lg, cursor_image.as_argb_8888(), count);
++    {
++        auto locked_buffers = buffers.lock();
++        for (auto& pair : *locked_buffers)
++        {
++            auto& buffer = pair.second;
++            if (size != geometry::Size{gbm_bo_get_width(buffer), gbm_bo_get_height(buffer)})
++            {
++                pad_and_write_image_data_locked(lg, buffer, cursor_image);
++            }
++            else
++            {
++                auto const count = size.width.as_uint32_t() * size.height.as_uint32_t() * sizeof(uint32_t);
++                write_buffer_data_locked(lg, buffer, cursor_image.as_argb_8888(), count);
++            }
++        }
+     }
      hotspot = cursor_image.hotspot();
@@ -288,7 +342,7 @@
              output.move_cursor(geom::Point{} + dp - hotspot);
              if (force_state || !output.has_cursor()) // TODO - or if orientation had changed - then set buffer..
+             {
--                if (!output.set_cursor(buffer) || !output.has_cursor())
++                if (!output.set_cursor(buffer_for_output(output)) || !output.has_cursor())
                      set_on_all_outputs = false;
+             }
+         }
@@ -303,3 +357,35 @@
      last_set_failed = !set_on_all_outputs;
+ }
++
++gbm_bo* mgm::Cursor::buffer_for_output(KMSOutput const& output)
++{
++    auto locked_buffers = buffers.lock();
++
++    auto buffer_it = std::find_if(
++        locked_buffers->begin(),
++        locked_buffers->end(),
++        [&output](auto const& candidate)
++        {
++            return candidate.first == output.drm_fd();
++        });
++
++    if (buffer_it != locked_buffers->end())
++    {
++        return buffer_it->second;
++    }
++
++    locked_buffers->push_back(std::make_pair(output.drm_fd(), GBMBOWrapper(output.drm_fd())));
++
++    gbm_bo* bo = locked_buffers->back().second;
++    if (gbm_bo_get_width(bo) < min_buffer_width)
++    {
++        min_buffer_width = gbm_bo_get_width(bo);
++    }
++    if (gbm_bo_get_height(bo) < min_buffer_height)
++    {
++        min_buffer_height = gbm_bo_get_height(bo);
++    }
++
++    return bo;
++}
 === modified file 'src/platforms/mesa/server/kms/cursor.h'
 --- src/platforms/mesa/server/kms/cursor.h	2017-05-25 02:47:48 +0000
 +++ src/platforms/mesa/server/kms/cursor.h	2017-06-09 10:22:51 +0000
@@ -25,12 +25,14 @@
  #include "mir/geometry/displacement.h"
  #include "mir_toolkit/common.h"
++#include "mutex.h"
  #include <gbm.h>
  #include <functional>
  #include <memory>
  #include <mutex>
++#include <vector>
  namespace mir
+ {
@@ -67,7 +69,6 @@
+ {
  public:
      Cursor(
--        gbm_device* device,
          KMSOutputContainer& output_container,
          std::shared_ptr<CurrentConfiguration> const& current_configuration);
@@ -87,9 +88,18 @@
      void for_each_used_output(std::function<void(KMSOutput&, geometry::Rectangle const&, MirOrientation orientation)> const& f);
      void place_cursor_at(geometry::Point position, ForceCursorState force_state);
      void place_cursor_at_locked(std::lock_guard<std::mutex> const&, geometry::Point position, ForceCursorState force_state);
--    void write_buffer_data_locked(std::lock_guard<std::mutex> const&, void const* data, size_t count);
--    void pad_and_write_image_data_locked(std::lock_guard<std::mutex> const&, CursorImage const& image);
++    void write_buffer_data_locked(
++        std::lock_guard<std::mutex> const&,
++        gbm_bo* buffer,
++        void const* data,
++        size_t count);
++    void pad_and_write_image_data_locked(
++        std::lock_guard<std::mutex> const&,
++        gbm_bo* buffer,
++        CursorImage const& image);
      void clear(std::lock_guard<std::mutex> const&);
++
++    gbm_bo* buffer_for_output(KMSOutput const& output);
      std::mutex guard;
@@ -102,17 +112,21 @@
      struct GBMBOWrapper
+     {
--        GBMBOWrapper(gbm_device* gbm);
++        GBMBOWrapper(int fd);
          operator gbm_bo*();
          ~GBMBOWrapper();
++
++        GBMBOWrapper(GBMBOWrapper&& from);
      private:
++        gbm_device* device;
          gbm_bo* buffer;
          GBMBOWrapper(GBMBOWrapper const&) = delete;
          GBMBOWrapper& operator=(GBMBOWrapper const&) = delete;
--    } buffer;
++    };
++    Mutex<std::vector<std::pair<int, GBMBOWrapper>>> buffers;
--    uint32_t buffer_width;
--    uint32_t buffer_height;
++    uint32_t min_buffer_width;
++    uint32_t min_buffer_height;
      std::shared_ptr<CurrentConfiguration> const current_configuration;
  };
 === modified file 'src/platforms/mesa/server/kms/display.cpp'
 --- src/platforms/mesa/server/kms/display.cpp	2017-05-08 03:04:26 +0000
 +++ src/platforms/mesa/server/kms/display.cpp	2017-06-09 10:22:51 +0000
@@ -339,17 +339,6 @@
  auto mgm::Display::create_hardware_cursor() -> std::shared_ptr<graphics::Cursor>
+ {
--    /*
--     * TODO: Using the hardware cursor in a hybrid-output situation requires making
--     * mgm::Cursor hybrid-aware so it can create a cursor bo on each GPU.
--     *
--     * For a first cut, just disable the hardware cursor on hybrid systems.
--     */
--    if (drm.size() > 1)
--    {
--        return nullptr;
--    }
--
      // There is only one hardware cursor. We do not keep a strong reference to it in the display though,
      // if no other component of Mir is interested (i.e. the input stack does not keep a reference to send
      // position updates) we must be configured not to use a cursor and thusly let it deallocate.
@@ -377,9 +366,9 @@
          try
+         {
--            locked_cursor = std::make_shared<Cursor>(gbm->device,
--                              *output_container,
--                              std::make_shared<KMSCurrentConfiguration>(*this));
++            locked_cursor = std::make_shared<Cursor>(
++                *output_container,
++                std::make_shared<KMSCurrentConfiguration>(*this));
+         }
          catch (std::runtime_error const&)
+         {
 === added file 'src/platforms/mesa/server/kms/mutex.h'
 --- src/platforms/mesa/server/kms/mutex.h	1970-01-01 00:00:00 +0000
 +++ src/platforms/mesa/server/kms/mutex.h	2017-06-09 10:22:51 +0000
@@ -0,0 +1,104 @@
++/*
++ * Copyright © 2017 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU Lesser General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_MUTEX_H_
++#define MIR_MUTEX_H_
++
++#include <mutex>
++
++namespace mir
++{
++/**
++ * Smart-pointer-esque accessor for Mutex<> protected data.
++ *
++ * Ensures exclusive access to the referenced data.
++ *
++ * \tparam Guarded Type of data guarded by the mutex.
++ */
++template<typename Guarded>
++class MutexGuard
++{
++public:
++    MutexGuard(std::unique_lock<std::mutex>&& lock, Guarded& value)
++        : value{value},
++          lock{std::move(lock)}
++    {
++    }
++    MutexGuard(MutexGuard&& from) = default;
++    ~MutexGuard() noexcept(false)
++    {
++        if (lock.owns_lock())
++        {
++            lock.unlock();
++        }
++    }
++
++    Guarded& operator*()
++    {
++        return value;
++    }
++    Guarded* operator->()
++    {
++        return &value;
++    }
++private:
++    Guarded& value;
++    std::unique_lock<std::mutex> lock;
++};
++
++/**
++ * A data-locking mutex
++ *
++ * This is a mutex which owns the data it guards, and can give out a
++ * smart-pointer-esque lock to lock and access it.
++ *
++ * \tparam Guarded  The type of data guarded by the mutex
++ */
++template<typename Guarded>
++class Mutex
++{
++public:
++    Mutex() = default;
++    Mutex(Guarded&& initial_value)
++        : value{std::move(initial_value)}
++    {
++    }
++
++    Mutex(Mutex const&) = delete;
++    Mutex& operator=(Mutex const&) = delete;
++
++    /**
++     * Lock the mutex and return an accessor for the protected data.
++     *
++     * \return A smart-pointer-esque accessor for the contained data.
++     *          While code has access to the MutexGuard it is guaranteed to have exclusive
++     *          access to the contained data.
++     */
++    MutexGuard<Guarded> lock()
++    {
++        return MutexGuard<Guarded>{std::unique_lock<std::mutex>{mutex}, value};
++    }
++
++private:
++    std::mutex mutex;
++    Guarded value;
++};
++
++}
++
++#endif //MIR_MUTEX_H_
 === modified file 'tests/unit-tests/platforms/mesa/kms/test_cursor.cpp'
 --- tests/unit-tests/platforms/mesa/kms/test_cursor.cpp	2017-05-08 03:04:26 +0000
 +++ tests/unit-tests/platforms/mesa/kms/test_cursor.cpp	2017-06-09 10:22:51 +0000
@@ -300,7 +300,7 @@
      size_t const cursor_side{64};
      MesaCursorTest()
--        : cursor{mock_gbm.fake_gbm.device, output_container,
++        : cursor{output_container,
              mt::fake_shared(current_configuration)}
+     {
          using namespace ::testing;
@@ -351,7 +351,7 @@
                                          GBM_FORMAT_ARGB8888,
                                          GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE));
--    mgm::Cursor cursor_tmp{mock_gbm.fake_gbm.device, output_container,
++    mgm::Cursor cursor_tmp{output_container,
          std::make_shared<StubCurrentConfiguration>(output_container)};
+ }
@@ -359,10 +359,11 @@
+ {
      using namespace ::testing;
--    EXPECT_CALL(mock_gbm, gbm_bo_get_width(_));
--    EXPECT_CALL(mock_gbm, gbm_bo_get_height(_));
++    // Our standard mock DRM has 2 GPU devices, and we should construct a cursor on both.
++    EXPECT_CALL(mock_gbm, gbm_bo_get_width(_)).Times(2);
++    EXPECT_CALL(mock_gbm, gbm_bo_get_height(_)).Times(2);
--    mgm::Cursor cursor_tmp{mock_gbm.fake_gbm.device, output_container,
++    mgm::Cursor cursor_tmp{output_container,
          std::make_shared<StubCurrentConfiguration>(output_container)};
+ }
@@ -377,7 +378,7 @@
      EXPECT_CALL(mock_gbm, gbm_bo_create(_, drm_buffer_size, drm_buffer_size, _, _));
--    mgm::Cursor cursor_tmp{mock_gbm.fake_gbm.device, output_container,
++    mgm::Cursor cursor_tmp{output_container,
                             std::make_shared<StubCurrentConfiguration>(output_container)};
+ }
@@ -394,7 +395,7 @@
      EXPECT_CALL(mock_gbm, gbm_bo_create(_, 64, 64, _, _));
--    mgm::Cursor cursor_tmp{mock_gbm.fake_gbm.device, output_container,
++    mgm::Cursor cursor_tmp{output_container,
                             std::make_shared<StubCurrentConfiguration>(output_container)};
+ }
@@ -458,7 +459,7 @@
      EXPECT_CALL(mock_gbm, gbm_bo_write(mock_gbm.fake_gbm.bo, ContainsASingleWhitePixel(width*height), buffer_size_bytes));
--    mgm::Cursor cursor_tmp{mock_gbm.fake_gbm.device, output_container,
++    mgm::Cursor cursor_tmp{output_container,
          std::make_shared<StubCurrentConfiguration>(output_container)};
      cursor_tmp.show(SinglePixelCursorImage());
+ }
@@ -495,7 +496,7 @@
      EXPECT_CALL(*output_container.outputs[1], has_cursor()).Times(0);
      EXPECT_CALL(*output_container.outputs[2], has_cursor()).Times(0);
--    mgm::Cursor cursor_tmp{mock_gbm.fake_gbm.device, output_container,
++    mgm::Cursor cursor_tmp{output_container,
         std::make_shared<StubCurrentConfiguration>(output_container)};
      output_container.verify_and_clear_expectations();
@@ -513,7 +514,7 @@
          .WillByDefault(Return(false));
      EXPECT_THROW(
--        mgm::Cursor cursor_tmp(mock_gbm.fake_gbm.device, output_container,
++        mgm::Cursor cursor_tmp(output_container,
             std::make_shared<StubCurrentConfiguration>(output_container));
      , std::runtime_error);
+ }