diff mbox

[hwc,v2,09/18] drm_hwcomposer: Handle writeback connectors

Message ID 1523460149-1740-10-git-send-email-alexandru-cosmin.gheorghe@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alexandru-Cosmin Gheorghe April 11, 2018, 3:22 p.m. UTC 
When writeback connectors are available assign them to displays, in
order to be able to use them for flattening of the current displayed
scene. The pipeline for each display will look like this:

CRTC ---- encoder ------------ display connector.
 |------- writeback enc ------ writeback connector.

However, the writeback connector will be later used/enabled only if
one of the following conditions are met:
 - Could be a clone of the display connector, as pointed by the
   possible_clones property.
 - The display_connector is disconnected, so we are safe to use it for
   flattening the scene that's already presented on another display.

Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
---
 drmresources.cpp | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 drmresources.h   |  3 +++
 2 files changed, 63 insertions(+), 2 deletions(-)

Comments

Sean Paul April 17, 2018, 3:45 p.m. UTC | #1 
On Wed, Apr 11, 2018 at 04:22:20PM +0100, Alexandru Gheorghe wrote:
> When writeback connectors are available assign them to displays, in
> order to be able to use them for flattening of the current displayed
> scene. The pipeline for each display will look like this:
> 
> CRTC ---- encoder ------------ display connector.
>  |------- writeback enc ------ writeback connector.
> 
> However, the writeback connector will be later used/enabled only if
> one of the following conditions are met:
>  - Could be a clone of the display connector, as pointed by the
>    possible_clones property.
>  - The display_connector is disconnected, so we are safe to use it for
>    flattening the scene that's already presented on another display.
> 
> Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> ---
>  drmresources.cpp | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  drmresources.h   |  3 +++
>  2 files changed, 63 insertions(+), 2 deletions(-)
> 
> diff --git a/drmresources.cpp b/drmresources.cpp
> index 39f50be..fef6835 100644
> --- a/drmresources.cpp
> +++ b/drmresources.cpp
> @@ -64,6 +64,14 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path,
>      return ret;
>    }
>  
> +#ifdef DRM_CLIENT_CAP_WRITEBACK_CONNECTORS
> +  ret = drmSetClientCap(fd(), DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> +  if (ret) {
> +    ALOGI("Failed to set writeback cap %d", ret);
> +    ret = 0;
> +  }
> +#endif
> +
>    drmModeResPtr res = drmModeGetResources(fd());
>    if (!res) {
>      ALOGE("Failed to get DrmResources resources");
> @@ -169,7 +177,7 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path,
>        conn->set_display(0);
>        displays_[0] = 0;
>        found_primary = true;
> -    } else {
> +    } else if (conn->external()) {
>        conn->set_display(display_num);
>        displays_[display_num] = display_num;
>        ++display_num;
> @@ -230,6 +238,8 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path,
>    }
>  
>    for (auto &conn : connectors_) {
> +    if (conn->writeback())
> +      continue;
>      ret = CreateDisplayPipe(conn.get());
>      if (ret) {
>        ALOGE("Failed CreateDisplayPipe %d with %d", conn->id(), ret);
> @@ -245,7 +255,15 @@ bool DrmResources::HandlesDisplay(int display) const {
>  
>  DrmConnector *DrmResources::GetConnectorForDisplay(int display) const {
>    for (auto &conn : connectors_) {
> -    if (conn->display() == display)
> +    if (conn->display() == display && !conn->writeback())
> +      return conn.get();
> +  }
> +  return NULL;
> +}
> +
> +DrmConnector *DrmResources::GetWritebackConnectorForDisplay(int display) const {
> +  for (auto &conn : connectors_) {
> +    if (conn->display() == display && conn->writeback())
>        return conn.get();
>    }
>    return NULL;
> @@ -280,6 +298,7 @@ int DrmResources::TryEncoderForDisplay(int display, DrmEncoder *enc) {
>    DrmCrtc *crtc = enc->crtc();
>    if (crtc && crtc->can_bind(display)) {
>      crtc->set_display(display);
> +    enc->set_display(display);
>      return 0;
>    }
>  
> @@ -306,6 +325,7 @@ int DrmResources::CreateDisplayPipe(DrmConnector *connector) {
>    if (connector->encoder()) {
>      int ret = TryEncoderForDisplay(display, connector->encoder());
>      if (!ret) {
> +      AttachWriteback(connector);

AttachWriteback returns int, but you throw it away here. Additionally,
AttachWriteback always follows a successful TryEncoderForDisplay, so it makes
sense to just call it from there.

>        return 0;
>      } else if (ret != -EAGAIN) {
>        ALOGE("Could not set mode %d/%d", display, ret);
> @@ -317,6 +337,7 @@ int DrmResources::CreateDisplayPipe(DrmConnector *connector) {
>      int ret = TryEncoderForDisplay(display, enc);
>      if (!ret) {
>        connector->set_encoder(enc);
> +      AttachWriteback(connector);
>        return 0;
>      } else if (ret != -EAGAIN) {
>        ALOGE("Could not set mode %d/%d", display, ret);
> @@ -328,6 +349,43 @@ int DrmResources::CreateDisplayPipe(DrmConnector *connector) {
>    return -ENODEV;
>  }
>  
> +/*
> + * Attach writeback connector to the CRTC linked to the display_conn
> + *
> + */
> +int DrmResources::AttachWriteback(DrmConnector *display_conn) {
> +  int ret = -EINVAL;

This isn't really used, just return the error code directly at the bottom.

> +  if (display_conn->writeback())
> +    return -EINVAL;

This condition would benefit from a log

> +  DrmEncoder *display_enc = display_conn->encoder();
> +  if (!display_enc)
> +    return -EINVAL;
> +  DrmCrtc *display_crtc = display_enc->crtc();
> +  if (!display_crtc)
> +    return -EINVAL;

Are these possible given this is only called after a successful
TryEncoderForDisplay()?

> +  if (GetWritebackConnectorForDisplay(display_crtc->display()) != NULL)
> +    return -EINVAL;

Again, logging would be useful.

> +  for (auto &writeback_conn : connectors_) {
> +    if (writeback_conn->display() >= 0 || !writeback_conn->writeback())

There doesn't seem to be any situation where you iterate through connectors_ and
you don't have some type of writeback() check. So it seems like it'd make sense
to track these in different vectors.

> +      continue;
> +    for (DrmEncoder *writeback_enc : writeback_conn->possible_encoders()) {
> +      for (DrmCrtc *possible_crtc : writeback_enc->possible_crtcs()) {
> +        if (possible_crtc != display_crtc)
> +          continue;
> +        // Use just encoders which had not been bound already
> +        if (writeback_enc->can_bind(display_crtc->display())) {
> +          writeback_enc->set_crtc(display_crtc);
> +          writeback_conn->set_encoder(writeback_enc);
> +          writeback_conn->set_display(display_crtc->display());
> +          writeback_conn->UpdateModes();
> +          return 0;
> +        }
> +      }
> +    }
> +  }
> +  return ret;
> +}
> +
>  int DrmResources::CreatePropertyBlob(void *data, size_t length,
>                                       uint32_t *blob_id) {
>    struct drm_mode_create_blob create_blob;
> diff --git a/drmresources.h b/drmresources.h
> index 4cdcd87..4fb17fc 100644
> --- a/drmresources.h
> +++ b/drmresources.h
> @@ -59,6 +59,8 @@ class DrmResources {
>    }
>  
>    DrmConnector *GetConnectorForDisplay(int display) const;
> +  DrmConnector *GetWritebackConnectorForDisplay(int display) const;
> +  DrmConnector *FindWritebackConnector(int display) const;
>    DrmCrtc *GetCrtcForDisplay(int display) const;
>    DrmPlane *GetPlane(uint32_t id) const;
>    DrmEventListener *event_listener();
> @@ -84,6 +86,7 @@ class DrmResources {
>                    DrmProperty *property);
>  
>    int CreateDisplayPipe(DrmConnector *connector);
> +  int AttachWriteback(DrmConnector *display_conn);
>  
>    UniqueFd fd_;
>    uint32_t mode_id_ = 0;
> -- 
> 2.7.4
>
diff mbox

Patch

diff --git a/drmresources.cpp b/drmresources.cpp
index 39f50be..fef6835 100644
--- a/drmresources.cpp
+++ b/drmresources.cpp
@@ -64,6 +64,14 @@  int DrmResources::Init(ResourceManager *resource_manager, char *path,
     return ret;
   }
 
+#ifdef DRM_CLIENT_CAP_WRITEBACK_CONNECTORS
+  ret = drmSetClientCap(fd(), DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
+  if (ret) {
+    ALOGI("Failed to set writeback cap %d", ret);
+    ret = 0;
+  }
+#endif
+
   drmModeResPtr res = drmModeGetResources(fd());
   if (!res) {
     ALOGE("Failed to get DrmResources resources");
@@ -169,7 +177,7 @@  int DrmResources::Init(ResourceManager *resource_manager, char *path,
       conn->set_display(0);
       displays_[0] = 0;
       found_primary = true;
-    } else {
+    } else if (conn->external()) {
       conn->set_display(display_num);
       displays_[display_num] = display_num;
       ++display_num;
@@ -230,6 +238,8 @@  int DrmResources::Init(ResourceManager *resource_manager, char *path,
   }
 
   for (auto &conn : connectors_) {
+    if (conn->writeback())
+      continue;
     ret = CreateDisplayPipe(conn.get());
     if (ret) {
       ALOGE("Failed CreateDisplayPipe %d with %d", conn->id(), ret);
@@ -245,7 +255,15 @@  bool DrmResources::HandlesDisplay(int display) const {
 
 DrmConnector *DrmResources::GetConnectorForDisplay(int display) const {
   for (auto &conn : connectors_) {
-    if (conn->display() == display)
+    if (conn->display() == display && !conn->writeback())
+      return conn.get();
+  }
+  return NULL;
+}
+
+DrmConnector *DrmResources::GetWritebackConnectorForDisplay(int display) const {
+  for (auto &conn : connectors_) {
+    if (conn->display() == display && conn->writeback())
       return conn.get();
   }
   return NULL;
@@ -280,6 +298,7 @@  int DrmResources::TryEncoderForDisplay(int display, DrmEncoder *enc) {
   DrmCrtc *crtc = enc->crtc();
   if (crtc && crtc->can_bind(display)) {
     crtc->set_display(display);
+    enc->set_display(display);
     return 0;
   }
 
@@ -306,6 +325,7 @@  int DrmResources::CreateDisplayPipe(DrmConnector *connector) {
   if (connector->encoder()) {
     int ret = TryEncoderForDisplay(display, connector->encoder());
     if (!ret) {
+      AttachWriteback(connector);
       return 0;
     } else if (ret != -EAGAIN) {
       ALOGE("Could not set mode %d/%d", display, ret);
@@ -317,6 +337,7 @@  int DrmResources::CreateDisplayPipe(DrmConnector *connector) {
     int ret = TryEncoderForDisplay(display, enc);
     if (!ret) {
       connector->set_encoder(enc);
+      AttachWriteback(connector);
       return 0;
     } else if (ret != -EAGAIN) {
       ALOGE("Could not set mode %d/%d", display, ret);
@@ -328,6 +349,43 @@  int DrmResources::CreateDisplayPipe(DrmConnector *connector) {
   return -ENODEV;
 }
 
+/*
+ * Attach writeback connector to the CRTC linked to the display_conn
+ *
+ */
+int DrmResources::AttachWriteback(DrmConnector *display_conn) {
+  int ret = -EINVAL;
+  if (display_conn->writeback())
+    return -EINVAL;
+  DrmEncoder *display_enc = display_conn->encoder();
+  if (!display_enc)
+    return -EINVAL;
+  DrmCrtc *display_crtc = display_enc->crtc();
+  if (!display_crtc)
+    return -EINVAL;
+  if (GetWritebackConnectorForDisplay(display_crtc->display()) != NULL)
+    return -EINVAL;
+  for (auto &writeback_conn : connectors_) {
+    if (writeback_conn->display() >= 0 || !writeback_conn->writeback())
+      continue;
+    for (DrmEncoder *writeback_enc : writeback_conn->possible_encoders()) {
+      for (DrmCrtc *possible_crtc : writeback_enc->possible_crtcs()) {
+        if (possible_crtc != display_crtc)
+          continue;
+        // Use just encoders which had not been bound already
+        if (writeback_enc->can_bind(display_crtc->display())) {
+          writeback_enc->set_crtc(display_crtc);
+          writeback_conn->set_encoder(writeback_enc);
+          writeback_conn->set_display(display_crtc->display());
+          writeback_conn->UpdateModes();
+          return 0;
+        }
+      }
+    }
+  }
+  return ret;
+}
+
 int DrmResources::CreatePropertyBlob(void *data, size_t length,
                                      uint32_t *blob_id) {
   struct drm_mode_create_blob create_blob;
diff --git a/drmresources.h b/drmresources.h
index 4cdcd87..4fb17fc 100644
--- a/drmresources.h
+++ b/drmresources.h
@@ -59,6 +59,8 @@  class DrmResources {
   }
 
   DrmConnector *GetConnectorForDisplay(int display) const;
+  DrmConnector *GetWritebackConnectorForDisplay(int display) const;
+  DrmConnector *FindWritebackConnector(int display) const;
   DrmCrtc *GetCrtcForDisplay(int display) const;
   DrmPlane *GetPlane(uint32_t id) const;
   DrmEventListener *event_listener();
@@ -84,6 +86,7 @@  class DrmResources {
                   DrmProperty *property);
 
   int CreateDisplayPipe(DrmConnector *connector);
+  int AttachWriteback(DrmConnector *display_conn);
 
   UniqueFd fd_;
   uint32_t mode_id_ = 0;