Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6e8f706c authored by Steven Thomas's avatar Steven Thomas
Browse files

Fix usage of HWC_DISPLAY_PRIMARY in vr flinger 

The hardware composer functions require display ids obtained from the
onHotplug() composer callback. Our vr flinger code was supplying
HWC_DISPLAY_PRIMARY as the primary display id, which is incorrect. The
HWC_DISPLAY_* values are used for representing the display type, not the
display id. The code worked anyway because most hardware composers
always use 0 as the primary display id, which happens to be the same
value as HWC_DISPLAY_PRIMARY. The emulator uses 1 as the primary display
id though, which exposed the bug.

This CL changes the vr flinger code to get the display id from the
onHotplug() composer callback, and uses that value when talking to
hardware composer, instead of HWC_DISPLAY_PRIMARY. This matches the
behavior of surface flinger.

Bug: 69631196

Test: Verified the vr flinger code path works as expected on phones and
standalones.

Change-Id: Ia691941d0eafaa1f89e0ee81a4ae27fdef5a57cf
parent 123704e1

libs/vr/libvrflinger/display_service.cpp

+3 −1
Original line number Diff line number Diff line
@@ -38,10 +38,12 @@ namespace android { 
namespace dvr {



DisplayService::DisplayService(Hwc2::Composer* hidl,

                               hwc2_display_t primary_display_id,

                               RequestDisplayCallback request_display_callback)

    : BASE("DisplayService",

           Endpoint::Create(display::DisplayProtocol::kClientPath)) {

  hardware_composer_.Initialize(hidl, request_display_callback);

    hardware_composer_.Initialize(

        hidl, primary_display_id, request_display_callback);

}



bool DisplayService::IsInitialized() const {

libs/vr/libvrflinger/display_service.h

+1 −0
Original line number Diff line number Diff line
@@ -81,6 +81,7 @@ class DisplayService : public pdx::ServiceBase<DisplayService> { 
  using RequestDisplayCallback = std::function<void(bool)>;



  DisplayService(android::Hwc2::Composer* hidl,

                 hwc2_display_t primary_display_id,

                 RequestDisplayCallback request_display_callback);



  pdx::Status<BorrowedNativeBufferHandle> OnGetGlobalBuffer(

libs/vr/libvrflinger/hardware_composer.cpp

+54 −64
Original line number Diff line number Diff line
@@ -121,7 +121,8 @@ HardwareComposer::~HardwareComposer(void) { 
}



bool HardwareComposer::Initialize(

    Hwc2::Composer* composer, RequestDisplayCallback request_display_callback) {

    Hwc2::Composer* composer, hwc2_display_t primary_display_id,

    RequestDisplayCallback request_display_callback) {

  if (initialized_) {

    ALOGE("HardwareComposer::Initialize: already initialized.");

    return false;
@@ -134,7 +135,7 @@ bool HardwareComposer::Initialize( 
  HWC::Error error = HWC::Error::None;



  Hwc2::Config config;

  error = composer->getActiveConfig(HWC_DISPLAY_PRIMARY, &config);

  error = composer->getActiveConfig(primary_display_id, &config);



  if (error != HWC::Error::None) {

    ALOGE("HardwareComposer: Failed to get current display config : %d",
@@ -142,7 +143,7 @@ bool HardwareComposer::Initialize( 
    return false;

  }



  error = GetDisplayMetrics(composer, HWC_DISPLAY_PRIMARY, config,

  error = GetDisplayMetrics(composer, primary_display_id, config,

                            &native_display_metrics_);



  if (error != HWC::Error::None) {
@@ -233,7 +234,10 @@ void HardwareComposer::OnPostThreadResumed() { 
    composer_.reset(new Hwc2::Composer("default"));

    composer_callback_ = new ComposerCallback;

    composer_->registerCallback(composer_callback_);

    LOG_ALWAYS_FATAL_IF(!composer_callback_->HasDisplayId(),

        "Registered composer callback but didn't get primary display");

    Layer::SetComposer(composer_.get());

    Layer::SetDisplayId(composer_callback_->GetDisplayId());

  } else {

    SetPowerMode(true);

  }
@@ -263,6 +267,7 @@ void HardwareComposer::OnPostThreadPaused() { 
    composer_callback_ = nullptr;

    composer_.reset(nullptr);

    Layer::SetComposer(nullptr);

    Layer::SetDisplayId(0);

  } else {

    SetPowerMode(false);

  }
@@ -290,7 +295,7 @@ HWC::Error HardwareComposer::Validate(hwc2_display_t display) { 


HWC::Error HardwareComposer::EnableVsync(bool enabled) {

  return composer_->setVsyncEnabled(

      HWC_DISPLAY_PRIMARY,

      composer_callback_->GetDisplayId(),

      (Hwc2::IComposerClient::Vsync)(enabled ? HWC2_VSYNC_ENABLE

                                             : HWC2_VSYNC_DISABLE));

}
@@ -298,7 +303,8 @@ HWC::Error HardwareComposer::EnableVsync(bool enabled) { 
HWC::Error HardwareComposer::SetPowerMode(bool active) {

  HWC::PowerMode power_mode = active ? HWC::PowerMode::On : HWC::PowerMode::Off;

  return composer_->setPowerMode(

      HWC_DISPLAY_PRIMARY, power_mode.cast<Hwc2::IComposerClient::PowerMode>());

      composer_callback_->GetDisplayId(),

      power_mode.cast<Hwc2::IComposerClient::PowerMode>());

}



HWC::Error HardwareComposer::Present(hwc2_display_t display) {
@@ -419,7 +425,7 @@ void HardwareComposer::PostLayers() { 
    layer.Prepare();

  }



  HWC::Error error = Validate(HWC_DISPLAY_PRIMARY);

  HWC::Error error = Validate(composer_callback_->GetDisplayId());

  if (error != HWC::Error::None) {

    ALOGE("HardwareComposer::PostLayers: Validate failed: %s",

          error.to_string().c_str());
@@ -466,7 +472,7 @@ void HardwareComposer::PostLayers() { 
  }

#endif



  error = Present(HWC_DISPLAY_PRIMARY);

  error = Present(composer_callback_->GetDisplayId());

  if (error != HWC::Error::None) {

    ALOGE("HardwareComposer::PostLayers: Present failed: %s",

          error.to_string().c_str());
@@ -475,8 +481,8 @@ void HardwareComposer::PostLayers() { 


  std::vector<Hwc2::Layer> out_layers;

  std::vector<int> out_fences;

  error = composer_->getReleaseFences(HWC_DISPLAY_PRIMARY, &out_layers,

                                      &out_fences);

  error = composer_->getReleaseFences(composer_callback_->GetDisplayId(),

                                      &out_layers, &out_fences);

  ALOGE_IF(error != HWC::Error::None,

           "HardwareComposer::PostLayers: Failed to get release fences: %s",

           error.to_string().c_str());
@@ -615,14 +621,6 @@ int HardwareComposer::PostThreadPollInterruptible( 
  }

}



Status<int64_t> HardwareComposer::GetVSyncTime() {

  auto status = composer_callback_->GetVsyncTime(HWC_DISPLAY_PRIMARY);

  ALOGE_IF(!status,

           "HardwareComposer::GetVSyncTime: Failed to get vsync timestamp: %s",

           status.GetErrorMessage().c_str());

  return status;

}



// Waits for the next vsync and returns the timestamp of the vsync event. If

// vsync already passed since the last call, returns the latest vsync timestamp

// instead of blocking.
@@ -795,8 +793,7 @@ void HardwareComposer::PostThread() { 
    // Signal all of the vsync clients. Because absolute time is used for the

    // wakeup time below, this can take a little time if necessary.

    if (vsync_callback_)

      vsync_callback_(HWC_DISPLAY_PRIMARY, vsync_timestamp,

                      /*frame_time_estimate*/ 0, vsync_count_);

      vsync_callback_(vsync_timestamp, /*frame_time_estimate*/ 0, vsync_count_);



    {

      // Sleep until shortly before vsync.
@@ -819,7 +816,7 @@ void HardwareComposer::PostThread() { 
            // If the layer config changed we need to validateDisplay() even if

            // we're going to drop the frame, to flush the Composer object's

            // internal command buffer and apply our layer changes.

            Validate(HWC_DISPLAY_PRIMARY);

            Validate(composer_callback_->GetDisplayId());

          }

          continue;

        }
@@ -827,7 +824,7 @@ void HardwareComposer::PostThread() { 
    }



    {

      auto status = GetVSyncTime();

      auto status = composer_callback_->GetVsyncTime();

      if (!status) {

        ALOGE("HardwareComposer::PostThread: Failed to get VSYNC time: %s",

              status.GetErrorMessage().c_str());
@@ -946,10 +943,17 @@ void HardwareComposer::SetBacklightBrightness(int brightness) { 
}



Return<void> HardwareComposer::ComposerCallback::onHotplug(

    Hwc2::Display display, IComposerCallback::Connection /*conn*/) {

  // See if the driver supports the vsync_event node in sysfs.

  if (display < HWC_NUM_PHYSICAL_DISPLAY_TYPES &&

      !displays_[display].driver_vsync_event_fd) {

    Hwc2::Display display, IComposerCallback::Connection conn) {

  // Our first onHotplug callback is always for the primary display.

  //

  // Ignore any other hotplug callbacks since the primary display is never

  // disconnected and we don't care about other displays.

  if (!has_display_id_) {

    LOG_ALWAYS_FATAL_IF(conn != IComposerCallback::Connection::CONNECTED,

        "Initial onHotplug callback should be primary display connected");

    has_display_id_ = true;

    display_id_ = display;



    std::array<char, 1024> buffer;

    snprintf(buffer.data(), buffer.size(),

             "/sys/class/graphics/fb%" PRIu64 "/vsync_event", display);
@@ -958,7 +962,7 @@ Return<void> HardwareComposer::ComposerCallback::onHotplug( 
          "HardwareComposer::ComposerCallback::onHotplug: Driver supports "

          "vsync_event node for display %" PRIu64,

          display);

      displays_[display].driver_vsync_event_fd = std::move(handle);

      driver_vsync_event_fd_ = std::move(handle);

    } else {

      ALOGI(

          "HardwareComposer::ComposerCallback::onHotplug: Driver does not "
@@ -977,36 +981,23 @@ Return<void> HardwareComposer::ComposerCallback::onRefresh( 


Return<void> HardwareComposer::ComposerCallback::onVsync(Hwc2::Display display,

                                                         int64_t timestamp) {

  // Ignore any onVsync callbacks for the non-primary display.

  if (has_display_id_ && display == display_id_) {

    TRACE_FORMAT("vsync_callback|display=%" PRIu64 ";timestamp=%" PRId64 "|",

                 display, timestamp);

  if (display < HWC_NUM_PHYSICAL_DISPLAY_TYPES) {

    displays_[display].callback_vsync_timestamp = timestamp;

  } else {

    ALOGW(

        "HardwareComposer::ComposerCallback::onVsync: Received vsync on "

        "non-physical display: display=%" PRId64,

        display);

    callback_vsync_timestamp_ = timestamp;

  }

  return Void();

}



Status<int64_t> HardwareComposer::ComposerCallback::GetVsyncTime(

    Hwc2::Display display) {

  if (display >= HWC_NUM_PHYSICAL_DISPLAY_TYPES) {

    ALOGE(

        "HardwareComposer::ComposerCallback::GetVsyncTime: Invalid physical "

        "display requested: display=%" PRIu64,

        display);

    return ErrorStatus(EINVAL);

  }



Status<int64_t> HardwareComposer::ComposerCallback::GetVsyncTime() {

  // See if the driver supports direct vsync events.

  LocalHandle& event_fd = displays_[display].driver_vsync_event_fd;

  LocalHandle& event_fd = driver_vsync_event_fd_;

  if (!event_fd) {

    // Fall back to returning the last timestamp returned by the vsync

    // callback.

    std::lock_guard<std::mutex> autolock(vsync_mutex_);

    return displays_[display].callback_vsync_timestamp;

    return callback_vsync_timestamp_;

  }



  // When the driver supports the vsync_event sysfs node we can use it to
@@ -1056,10 +1047,11 @@ Status<int64_t> HardwareComposer::ComposerCallback::GetVsyncTime( 


Hwc2::Composer* Layer::composer_{nullptr};

HWCDisplayMetrics Layer::display_metrics_{0, 0, {0, 0}, 0};

hwc2_display_t Layer::display_id_{0};



void Layer::Reset() {

  if (hardware_composer_layer_) {

    composer_->destroyLayer(HWC_DISPLAY_PRIMARY, hardware_composer_layer_);

    composer_->destroyLayer(display_id_, hardware_composer_layer_);

    hardware_composer_layer_ = 0;

  }
@@ -1154,17 +1146,16 @@ void Layer::UpdateVisibilitySettings() { 
    pending_visibility_settings_ = false;



    HWC::Error error;

    hwc2_display_t display = HWC_DISPLAY_PRIMARY;



    error = composer_->setLayerBlendMode(

        display, hardware_composer_layer_,

        display_id_, hardware_composer_layer_,

        blending_.cast<Hwc2::IComposerClient::BlendMode>());

    ALOGE_IF(error != HWC::Error::None,

             "Layer::UpdateLayerSettings: Error setting layer blend mode: %s",

             error.to_string().c_str());



    error =

        composer_->setLayerZOrder(display, hardware_composer_layer_, z_order_);

    error = composer_->setLayerZOrder(display_id_, hardware_composer_layer_,

                                      z_order_);

    ALOGE_IF(error != HWC::Error::None,

             "Layer::UpdateLayerSettings: Error setting z_ order: %s",

             error.to_string().c_str());
@@ -1173,36 +1164,35 @@ void Layer::UpdateVisibilitySettings() { 


void Layer::UpdateLayerSettings() {

  HWC::Error error;

  hwc2_display_t display = HWC_DISPLAY_PRIMARY;



  UpdateVisibilitySettings();



  // TODO(eieio): Use surface attributes or some other mechanism to control

  // the layer display frame.

  error = composer_->setLayerDisplayFrame(

      display, hardware_composer_layer_,

      display_id_, hardware_composer_layer_,

      {0, 0, display_metrics_.width, display_metrics_.height});

  ALOGE_IF(error != HWC::Error::None,

           "Layer::UpdateLayerSettings: Error setting layer display frame: %s",

           error.to_string().c_str());



  error = composer_->setLayerVisibleRegion(

      display, hardware_composer_layer_,

      display_id_, hardware_composer_layer_,

      {{0, 0, display_metrics_.width, display_metrics_.height}});

  ALOGE_IF(error != HWC::Error::None,

           "Layer::UpdateLayerSettings: Error setting layer visible region: %s",

           error.to_string().c_str());



  error =

      composer_->setLayerPlaneAlpha(display, hardware_composer_layer_, 1.0f);

  error = composer_->setLayerPlaneAlpha(display_id_, hardware_composer_layer_,

                                        1.0f);

  ALOGE_IF(error != HWC::Error::None,

           "Layer::UpdateLayerSettings: Error setting layer plane alpha: %s",

           error.to_string().c_str());

}



void Layer::CommonLayerSetup() {

  HWC::Error error =

      composer_->createLayer(HWC_DISPLAY_PRIMARY, &hardware_composer_layer_);

  HWC::Error error = composer_->createLayer(display_id_,

                                            &hardware_composer_layer_);

  ALOGE_IF(error != HWC::Error::None,

           "Layer::CommonLayerSetup: Failed to create layer on primary "

           "display: %s",
@@ -1246,10 +1236,10 @@ void Layer::Prepare() { 
    if (composition_type_ == HWC::Composition::Invalid) {

      composition_type_ = HWC::Composition::SolidColor;

      composer_->setLayerCompositionType(

          HWC_DISPLAY_PRIMARY, hardware_composer_layer_,

          display_id_, hardware_composer_layer_,

          composition_type_.cast<Hwc2::IComposerClient::Composition>());

      Hwc2::IComposerClient::Color layer_color = {0, 0, 0, 0};

      composer_->setLayerColor(HWC_DISPLAY_PRIMARY, hardware_composer_layer_,

      composer_->setLayerColor(display_id_, hardware_composer_layer_,

                               layer_color);

    } else {

      // The composition type is already set. Nothing else to do until a
@@ -1259,7 +1249,7 @@ void Layer::Prepare() { 
    if (composition_type_ != target_composition_type_) {

      composition_type_ = target_composition_type_;

      composer_->setLayerCompositionType(

          HWC_DISPLAY_PRIMARY, hardware_composer_layer_,

          display_id_, hardware_composer_layer_,

          composition_type_.cast<Hwc2::IComposerClient::Composition>());

    }
@@ -1270,7 +1260,7 @@ void Layer::Prepare() { 


    HWC::Error error{HWC::Error::None};

    error =

        composer_->setLayerBuffer(HWC_DISPLAY_PRIMARY, hardware_composer_layer_,

        composer_->setLayerBuffer(display_id_, hardware_composer_layer_,

                                  slot, handle, acquire_fence_.Get());



    ALOGE_IF(error != HWC::Error::None,
@@ -1280,7 +1270,7 @@ void Layer::Prepare() { 
    if (!surface_rect_functions_applied_) {

      const float float_right = right;

      const float float_bottom = bottom;

      error = composer_->setLayerSourceCrop(HWC_DISPLAY_PRIMARY,

      error = composer_->setLayerSourceCrop(display_id_,

                                            hardware_composer_layer_,

                                            {0, 0, float_right, float_bottom});

libs/vr/libvrflinger/hardware_composer.h

+19 −9
Original line number Diff line number Diff line
@@ -152,6 +152,11 @@ class Layer { 
    display_metrics_ = display_metrics;

  }



  // Sets the display id used by all Layer instances.

  static void SetDisplayId(hwc2_display_t display_id) {

    display_id_ = display_id;

  }



 private:

  void CommonLayerSetup();
@@ -180,6 +185,11 @@ class Layer { 
  // thereafter.

  static HWCDisplayMetrics display_metrics_;



  // Id of the primary display. Shared by all instances of Layer. This must be

  // set whenever the primary display id changes. This can be left unset as long

  // as there are no instances of Layer that might need to use it.

  static hwc2_display_t display_id_;



  // The hardware composer layer and metrics to use during the prepare cycle.

  hwc2_layer_t hardware_composer_layer_ = 0;
@@ -298,13 +308,14 @@ class Layer { 
class HardwareComposer {

 public:

  // Type for vsync callback.

  using VSyncCallback = std::function<void(int, int64_t, int64_t, uint32_t)>;

  using VSyncCallback = std::function<void(int64_t, int64_t, uint32_t)>;

  using RequestDisplayCallback = std::function<void(bool)>;



  HardwareComposer();

  ~HardwareComposer();



  bool Initialize(Hwc2::Composer* composer,

                  hwc2_display_t primary_display_id,

                  RequestDisplayCallback request_display_callback);



  bool IsInitialized() const { return initialized_; }
@@ -362,16 +373,16 @@ class HardwareComposer { 
    hardware::Return<void> onVsync(Hwc2::Display display,

                                   int64_t timestamp) override;



    pdx::Status<int64_t> GetVsyncTime(Hwc2::Display display);

    bool HasDisplayId() { return has_display_id_; }

    hwc2_display_t GetDisplayId() { return display_id_; }

    pdx::Status<int64_t> GetVsyncTime();



   private:

    std::mutex vsync_mutex_;



    struct Display {

      pdx::LocalHandle driver_vsync_event_fd;

      int64_t callback_vsync_timestamp{0};

    };

    std::array<Display, HWC_NUM_PHYSICAL_DISPLAY_TYPES> displays_;

    bool has_display_id_ = false;

    hwc2_display_t display_id_;

    pdx::LocalHandle driver_vsync_event_fd_;

    int64_t callback_vsync_timestamp_{0};

  };



  HWC::Error Validate(hwc2_display_t display);
@@ -412,7 +423,6 @@ class HardwareComposer { 
  // kPostThreadInterrupted.

  int ReadWaitPPState();

  pdx::Status<int64_t> WaitForVSync();

  pdx::Status<int64_t> GetVSyncTime();

  int SleepUntil(int64_t wakeup_timestamp);



  // Reconfigures the layer stack if the display surfaces changed since the last

libs/vr/libvrflinger/include/dvr/vr_flinger.h

+10 −1
Original line number Diff line number Diff line
@@ -4,6 +4,12 @@ 
#include <thread>

#include <memory>



#define HWC2_INCLUDE_STRINGIFICATION

#define HWC2_USE_CPP11

#include <hardware/hwcomposer2.h>

#undef HWC2_INCLUDE_STRINGIFICATION

#undef HWC2_USE_CPP11



#include <pdx/service_dispatcher.h>

#include <vr/vr_manager/vr_manager.h>
@@ -21,7 +27,9 @@ class VrFlinger { 
 public:

  using RequestDisplayCallback = std::function<void(bool)>;

  static std::unique_ptr<VrFlinger> Create(

      Hwc2::Composer* hidl, RequestDisplayCallback request_display_callback);

      Hwc2::Composer* hidl,

      hwc2_display_t primary_display_id,

      RequestDisplayCallback request_display_callback);

  ~VrFlinger();



  // These functions are all called on surface flinger's main thread.
@@ -35,6 +43,7 @@ class VrFlinger { 
 private:

  VrFlinger();

  bool Init(Hwc2::Composer* hidl,

            hwc2_display_t primary_display_id,

            RequestDisplayCallback request_display_callback);



  // Needs to be a separate class for binder's ref counting