Re: [PATCH weston 3/9] compositor-drm: factor out drm_output_init_crtc()

[Daniel Stone]

Hi Pekka,

On 9 February 2018 at 13:07, Pekka Paalanen  wrote:
> +   i = find_crtc_for_connector(b, resources, connector);
> +   if (i < 0) {
> +   weston_log("No usable crtc/encoder pair for connector.\n");
> +   return -1;
> +   }
> +
> +   crtc_id = resources->crtcs[i];
> +
> +   props = drmModeObjectGetProperties(b->drm.fd, crtc_id,
> +  DRM_MODE_OBJECT_CRTC);
> +   if (!props) {
> +   weston_log("failed to get CRTC properties\n");
> +   return -1;
> +   }
> +   drm_property_info_populate(b, crtc_props, output->props_crtc,
> +  WDRM_CRTC__COUNT, props);
> +   drmModeFreeObjectProperties(props);
> +
> +   output->crtc_id = crtc_id;
> +   output->pipe = i;
> +
> +   output->scanout_plane =
> +   drm_output_find_special_plane(b, output,
> + WDRM_PLANE_TYPE_PRIMARY);
> +   if (!output->scanout_plane) {
> +   weston_log("Failed to find primary plane for output %s\n",
> +  output->base.name);
> +   output->crtc_id = 0;
> +   return -1;
> +   }

First nitpick: this leaves output->pipe set. Probably best to just
init crtc_id and pipe at the very top, then undo both in a goto err.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston 3/9] compositor-drm: factor out drm_output_init_crtc()

[Pekka Paalanen]

From: Pekka Paalanen 

Factor out drm_output_init_crtc() and drm_output_fini_crtc(), so that
the call sites can later be moved easily.

On fini, reset scanout_plane and cursor_plane to NULL, so that in the
future when the drm_output is not longer destroyed immediately after, we
free the planes for other use.

Signed-off-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 156 ++---
 1 file changed, 104 insertions(+), 52 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index e1004e0f..c4188d07 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -4593,6 +4593,107 @@ drm_output_init_gamma_size(struct drm_output *output)
return 0;
 }
 
+/** Allocate a CRTC for the output
+ *
+ * @param output The output with no allocated CRTC.
+ * @param resources DRM KMS resources.
+ * @param connector The DRM KMS connector data.
+ * @return 0 on success, -1 on failure.
+ *
+ * Finds a free CRTC that can drive the given connector, reserves the CRTC
+ * for the output, and loads the CRTC properties.
+ *
+ * Populates the cursor and scanout planes.
+ *
+ * On failure, the output remains without a CRTC.
+ */
+static int
+drm_output_init_crtc(struct drm_output *output,
+drmModeRes *resources, drmModeConnector *connector)
+{
+   struct drm_backend *b = to_drm_backend(output->base.compositor);
+   drmModeObjectPropertiesPtr props;
+   uint32_t crtc_id;
+   int i;
+
+   assert(output->crtc_id == 0);
+
+   i = find_crtc_for_connector(b, resources, connector);
+   if (i < 0) {
+   weston_log("No usable crtc/encoder pair for connector.\n");
+   return -1;
+   }
+
+   crtc_id = resources->crtcs[i];
+
+   props = drmModeObjectGetProperties(b->drm.fd, crtc_id,
+  DRM_MODE_OBJECT_CRTC);
+   if (!props) {
+   weston_log("failed to get CRTC properties\n");
+   return -1;
+   }
+   drm_property_info_populate(b, crtc_props, output->props_crtc,
+  WDRM_CRTC__COUNT, props);
+   drmModeFreeObjectProperties(props);
+
+   output->crtc_id = crtc_id;
+   output->pipe = i;
+
+   output->scanout_plane =
+   drm_output_find_special_plane(b, output,
+ WDRM_PLANE_TYPE_PRIMARY);
+   if (!output->scanout_plane) {
+   weston_log("Failed to find primary plane for output %s\n",
+  output->base.name);
+   output->crtc_id = 0;
+   return -1;
+   }
+
+   /* Failing to find a cursor plane is not fatal, as we'll fall back
+* to software cursor. */
+   output->cursor_plane =
+   drm_output_find_special_plane(b, output,
+ WDRM_PLANE_TYPE_CURSOR);
+
+   return 0;
+}
+
+/** Free the CRTC from the output
+ *
+ * @param output The output whose CRTC to deallocate.
+ *
+ * The CRTC reserved for the given output becomes free to use again.
+ */
+static void
+drm_output_fini_crtc(struct drm_output *output)
+{
+   struct drm_backend *b = to_drm_backend(output->base.compositor);
+
+   if (!b->universal_planes && !b->shutting_down) {
+   /* With universal planes, the 'special' planes are allocated at
+* startup, freed at shutdown, and live on the plane list in
+* between. We want the planes to continue to exist and be freed
+* up for other outputs.
+*
+* Without universal planes, our special planes are
+* pseudo-planes allocated at output creation, freed at output
+* destruction, and not usable by other outputs.
+*
+* On the other hand, if the compositor is already shutting 
down,
+* the plane has already been destroyed.
+*/
+   if (output->cursor_plane)
+   drm_plane_destroy(output->cursor_plane);
+   if (output->scanout_plane)
+   drm_plane_destroy(output->scanout_plane);
+   }
+
+   drm_property_info_free(output->props_crtc, WDRM_CRTC__COUNT);
+   output->crtc_id = 0;
+   output->cursor_plane = NULL;
+   output->scanout_plane = NULL;
+}
+
 static int
 drm_output_enable(struct weston_output *base)
 {
@@ -4715,25 +4816,6 @@ drm_output_destroy(struct weston_output *base)
if (output->base.enabled)
drm_output_deinit(>base);
 
-   if (!b->universal_planes && !b->shutting_down) {
-   /* With universal planes, the 'special' planes are allocated at
-* startup, freed at shutdown, and live on the plane list in
-* between. We want the planes to continue to exist and be