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.gheor...@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
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
