[PATCH weston v9 20/62] compositor-drm: Add DRM property cache

Daniel Stone daniels at collabora.com
Fri Mar 3 23:05:31 UTC 2017


Add a cache for DRM property IDs and values, and use it for the two
connector properties we currently update: DPMS and EDID.

As DRM property ID values are not stable, we need to do a name -> ID
lookup each run in order to discover the property IDs and enum values to
use for those properties. Rather than open-coding this, add a property
cache which we can use across multiple different object types.

This patch takes substantial work from the universal planes support
originally authored by Pekka Paalanen, though it has been heavily
reworked. In particular, the enum map has been lifted into the property
item itself, and changed from per-object to per-object-type, which
necessitated removing the value cache.

Signed-off-by: Daniel Stone <daniels at collabora.com>
Co-authored-with: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
---
 libweston/compositor-drm.c | 418 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 363 insertions(+), 55 deletions(-)

v9: Split out from universal planes patch. Reworked to cache properties
    per-object-type rather than per-object, and also to not cache values
    and the drmProperty themselves. Fold enum value map into the property
    item itself. Use for connector properties.

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 7f7d936..d1c8675 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -79,6 +79,44 @@
 #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
 #endif
 
+/**
+ * List of properties attached to a DRM connector
+ */
+enum wdrm_connector_property {
+	WDRM_CONNECTOR_EDID = 0,
+	WDRM_CONNECTOR_DPMS,
+	WDRM_CONNECTOR__COUNT
+};
+
+/**
+ * Represents the values of an enum-type KMS property
+ */
+struct drm_property_enum_info {
+	const char *name; /**< name as string (static, not freed) */
+	bool valid; /**< true if value is supported; ignore if false */
+	uint64_t value; /**< raw value */
+};
+
+/**
+ * Holds information on a DRM property, including its ID and the enum
+ * values it holds.
+ *
+ * DRM properties are allocated dynamically, and maintained as DRM objects
+ * within the normal object ID space; they thus do not have a stable ID
+ * to refer to. This includes enum values, which must be referred to by
+ * integer values, but these are not stable.
+ *
+ * drm_property_info allows a cache to be maintained where Weston can use
+ * enum values internally to refer to properties, with the mapping to DRM
+ * ID values being maintained internally.
+ */
+struct drm_property_info {
+	const char *name; /**< name as string (static, not freed) */
+	uint32_t prop_id; /**< KMS property object ID */
+	unsigned int num_enum_values; /**< number of enum values */
+	struct drm_property_enum_info *enum_values; /**< array of enum values */
+};
+
 struct drm_backend {
 	struct weston_backend base;
 	struct weston_compositor *compositor;
@@ -116,6 +154,9 @@ struct drm_backend {
 
 	struct udev_input input;
 
+	/* Holds the properties for connectors */
+	struct drm_property_info props_conn[WDRM_CONNECTOR__COUNT];
+
 	int32_t cursor_width;
 	int32_t cursor_height;
 
@@ -206,7 +247,6 @@ struct drm_output {
 	uint32_t connector_id;
 	drmModeCrtcPtr original_crtc;
 	struct drm_edid edid;
-	drmModePropertyPtr dpms_prop;
 
 	enum dpms_enum dpms;
 	struct backlight *backlight;
@@ -253,6 +293,262 @@ to_drm_backend(struct weston_compositor *base)
 	return container_of(base->backend, struct drm_backend, base);
 }
 
+/**
+ * Get the current value of a KMS property
+ *
+ * Given a drmModeObjectGetProperties return, as well as the drm_property_info
+ * for the target property, return the current value of that property,
+ * with an optional default. If the property is a KMS enum type, the return
+ * value will be translated into the appropriate internal enum.
+ *
+ * If the property is not present, the default value will be returned.
+ *
+ * @param info Internal structure for property to look up
+ * @param props Raw KMS properties for the target object
+ * @param def Value to return if property is not found
+ */
+static uint64_t
+drm_property_get_value(struct drm_property_info *info,
+		       drmModeObjectPropertiesPtr props,
+		       uint64_t def)
+{
+	unsigned int i;
+
+	if (info->prop_id == 0)
+		return def;
+
+	for (i = 0; i < props->count_props; i++) {
+		unsigned int j;
+
+		if (props->props[i] != info->prop_id)
+			continue;
+
+		/* Simple (non-enum) types can return the value directly */
+		if (info->num_enum_values == 0)
+			return props->prop_values[i];
+
+		/* Map from raw value to enum value */
+		for (j = 0; j < info->num_enum_values; j++) {
+			if (!info->enum_values[j].valid)
+				continue;
+			if (info->enum_values[j].value != props->prop_values[i])
+				continue;
+
+			return j;
+		}
+	}
+
+	return def;
+}
+
+/**
+ * Cache DRM property values
+ *
+ * Update a per-object-type array of drm_property_info structures, given the
+ * current property values for a particular object.
+ *
+ * Call this every time an object newly appears (note that only connectors
+ * can be hotplugged), the first time it is seen, or when its status changes
+ * in a way which invalidates the potential property values (currently, the
+ * only case for this is connector hotplug).
+ *
+ * This updates the property IDs and enum values within the drm_property_info
+ * array.
+ *
+ * Property lookup is not mandatory and may fail; users should carefully
+ * check the return value, which is a bitmask populated with the indices
+ * of properties which were successfully looked up.
+ *
+ * DRM property enum values are dynamic at runtime; the user must query the
+ * property to find out the desired runtime value for a requested string
+ * name. Using  the 'type' field on planes as an example, there is no single
+ * hardcoded constant for primary plane types; instead, the property must be
+ * queried at runtime to find the value associated with the string "Primary".
+ *
+ * This helper queries and caches the enum values, to allow us to use a set
+ * of compile-time-constant enums portably across various implementations.
+ * The values given in enum_names are searched for, and stored in the
+ * same-indexed field of the map array.
+ *
+ * For example, if the DRM driver exposes 'Foo' as value 7 and 'Bar' as value
+ * 19, passing in enum_names containing 'Foo' and 'Bar' in that order, will
+ * populate map with 7 and 19, in that order.
+ *
+ * This should always be used with static C enums to represent each value, e.g.
+ * WDRM_PLANE_MISC_FOO, WDRM_PLANE_MISC_BAR.
+ *
+ * @param b DRM backend object
+ * @param info DRM property info array
+ * @param num_infos Number of entries in DRM property info array
+ * @param props DRM object properties for a given object
+ *
+ * @returns Bitmask of populated entries in map
+ */
+static uint32_t
+drm_property_info_update(struct drm_backend *b,
+			 struct drm_property_info *info,
+			 unsigned int num_infos,
+			 drmModeObjectProperties *props)
+{
+	drmModePropertyRes *prop;
+	uint32_t valid_mask = 0;
+	unsigned i, j;
+
+	assert(num_infos <= 32 && "update return type");
+
+	for (i = 0; i < props->count_props; i++) {
+		bool props_incomplete = false;
+		unsigned int k;
+
+		for (j = 0; j < num_infos; j++) {
+			if (info[j].prop_id == props->props[i])
+				break;
+			if (!info[j].prop_id)
+				props_incomplete = true;
+		}
+
+		/* We've already discovered this property. */
+		if (j != num_infos)
+			continue;
+
+		/* We haven't found this property ID, but as we've already
+		 * found all known properties, we don't need to look any
+		 * further. */
+		if (!props_incomplete)
+			break;
+
+		prop = drmModeGetProperty(b->drm.fd, props->props[i]);
+		if (!prop)
+			continue;
+
+		for (j = 0; j < num_infos; j++) {
+			if (!strcmp(prop->name, info[j].name))
+				break;
+		}
+
+		/* We don't know/care about this property. */
+		if (j == num_infos) {
+#ifdef DEBUG
+			weston_log("DRM debug: unrecognized property %u '%s'\n",
+				   prop->prop_id, prop->name);
+#endif
+			drmModeFreeProperty(prop);
+			continue;
+		}
+
+		info[j].prop_id = props->props[i];
+		valid_mask |= 1U << j;
+
+		if (info[j].num_enum_values == 0) {
+			drmModeFreeProperty(prop);
+			continue;
+		}
+
+		if (!(prop->flags & DRM_MODE_PROP_ENUM)) {
+			weston_log("DRM: expected property %s to be an enum,"
+				   " but it is not; ignoring\n", prop->name);
+			drmModeFreeProperty(prop);
+			continue;
+		}
+
+		for (k = 0; k < info[j].num_enum_values; k++) {
+			int l;
+
+			if (info[j].enum_values[k].valid)
+				continue;
+
+			for (l = 0; l < prop->count_enums; l++) {
+				if (!strcmp(prop->enums[l].name,
+					    info[j].enum_values[k].name))
+					break;
+			}
+
+			if (l == prop->count_enums)
+				continue;
+
+			info[j].enum_values[k].valid = true;
+			info[j].enum_values[k].value = prop->enums[l].value;
+		}
+
+		drmModeFreeProperty(prop);
+	}
+
+#ifdef DEBUG
+	for (i = 0; i < num_infos; i++) {
+		if (info[i].prop_id == 0)
+			weston_log("DRM warning: property '%s' missing\n",
+				   info[i].name);
+	}
+#endif
+
+	return valid_mask;
+}
+
+/**
+ * Copy DRM property information
+ *
+ * Copies an array of drm_property_info entries.
+ *
+ * @param dst_out Pointer to (already-allocated) new array
+ * @param src Property info array to copy from
+ * @param num_props Number of properties to copy from src
+ */
+static bool
+drm_property_info_copy(struct drm_property_info *dst,
+		       const struct drm_property_info *src,
+		       unsigned int num_props)
+{
+	unsigned int i;
+
+	memcpy(dst, src, num_props * sizeof(*dst));
+
+	for (i = 0; i < num_props; i++) {
+		unsigned int j;
+
+		dst[i].prop_id = 0;
+
+		if (src[i].num_enum_values == 0)
+			continue;
+
+		dst[i].enum_values =
+			malloc(src[i].num_enum_values *
+			       sizeof(*dst[i].enum_values));
+		if (!dst[i].enum_values)
+			goto err;
+
+		memcpy(dst[i].enum_values, src[i].enum_values,
+		       src[i].num_enum_values * sizeof(*dst[i].enum_values));
+
+		for (j = 0; j < dst[i].num_enum_values; j++)
+			dst[i].enum_values[j].valid = false;
+	}
+
+	return true;
+
+err:
+	while (i--)
+		free(dst[i].enum_values);
+	free(dst);
+	return false;
+}
+
+/**
+ * Free DRM property information
+ *
+ * Frees a DRM property information array and all associated memory
+ *
+ * @param info DRM property info array
+ * @param num_props Number of entries in array to free
+ */
+static void
+drm_property_info_free(struct drm_property_info *info, int num_props)
+{
+	int i;
+
+	for (i = 0; i < num_props; i++)
+		free(info[i].enum_values);
+}
+
 static void
 drm_output_set_cursor(struct drm_output *output);
 
@@ -1950,39 +2246,20 @@ drm_set_backlight(struct weston_output *output_base, uint32_t value)
 	backlight_set_brightness(output->backlight, new_brightness);
 }
 
-static drmModePropertyPtr
-drm_get_prop(int fd, drmModeConnectorPtr connector, const char *name)
-{
-	drmModePropertyPtr props;
-	int i;
-
-	for (i = 0; i < connector->count_props; i++) {
-		props = drmModeGetProperty(fd, connector->props[i]);
-		if (!props)
-			continue;
-
-		if (!strcmp(props->name, name))
-			return props;
-
-		drmModeFreeProperty(props);
-	}
-
-	return NULL;
-}
-
 static void
 drm_set_dpms(struct weston_output *output_base, enum dpms_enum level)
 {
 	struct drm_output *output = to_drm_output(output_base);
 	struct weston_compositor *ec = output_base->compositor;
 	struct drm_backend *b = to_drm_backend(ec);
+	struct drm_property_info *prop = &b->props_conn[WDRM_CONNECTOR_DPMS];
 	int ret;
 
-	if (!output->dpms_prop)
+	if (!prop->prop_id)
 		return;
 
 	ret = drmModeConnectorSetProperty(b->drm.fd, output->connector_id,
-				 	  output->dpms_prop->prop_id, level);
+					  prop->prop_id, level);
 	if (ret) {
 		weston_log("DRM: DPMS: failed property set for %s\n",
 			   output->base.name);
@@ -2337,26 +2614,19 @@ edid_parse(struct drm_edid *edid, const uint8_t *data, size_t length)
 }
 
 static void
-find_and_parse_output_edid(struct drm_backend *b,
-			   struct drm_output *output,
-			   drmModeConnector *connector)
+find_and_parse_output_edid(struct drm_backend *b, struct drm_output *output,
+			   drmModeObjectPropertiesPtr props)
 {
 	drmModePropertyBlobPtr edid_blob = NULL;
-	drmModePropertyPtr property;
-	int i;
+	uint32_t blob_id;
 	int rc;
 
-	for (i = 0; i < connector->count_props && !edid_blob; i++) {
-		property = drmModeGetProperty(b->drm.fd, connector->props[i]);
-		if (!property)
-			continue;
-		if ((property->flags & DRM_MODE_PROP_BLOB) &&
-		    !strcmp(property->name, "EDID")) {
-			edid_blob = drmModeGetPropertyBlob(b->drm.fd,
-							   connector->prop_values[i]);
-		}
-		drmModeFreeProperty(property);
-	}
+	blob_id = drm_property_get_value(&b->props_conn[WDRM_CONNECTOR_EDID],
+					 props, 0);
+	if (!blob_id)
+		return;
+
+	edid_blob = drmModeGetPropertyBlob(b->drm.fd, blob_id);
 	if (!edid_blob)
 		return;
 
@@ -2647,16 +2917,14 @@ drm_output_enable(struct weston_output *base)
 	struct drm_backend *b = to_drm_backend(base->compositor);
 	struct weston_mode *m;
 
-	output->dpms_prop = drm_get_prop(b->drm.fd, output->connector, "DPMS");
-
 	if (b->use_pixman) {
 		if (drm_output_init_pixman(output, b) < 0) {
 			weston_log("Failed to init output pixman state\n");
-			goto err_free;
+			goto err;
 		}
 	} else if (drm_output_init_egl(output, b) < 0) {
 		weston_log("Failed to init output gl state\n");
-		goto err_free;
+		goto err;
 	}
 
 	if (output->backlight) {
@@ -2679,7 +2947,6 @@ drm_output_enable(struct weston_output *base)
 
 	output->base.subpixel = drm_subpixel_to_wayland(output->connector->subpixel);
 
-	find_and_parse_output_edid(b, output, output->connector);
 	if (output->connector->connector_type == DRM_MODE_CONNECTOR_LVDS ||
 	    output->connector->connector_type == DRM_MODE_CONNECTOR_eDP)
 		output->base.connection_internal = 1;
@@ -2706,9 +2973,7 @@ drm_output_enable(struct weston_output *base)
 
 	return 0;
 
-err_free:
-	drmModeFreeProperty(output->dpms_prop);
-
+err:
 	return -1;
 }
 
@@ -2726,8 +2991,6 @@ drm_output_deinit(struct weston_output *base)
 	weston_plane_release(&output->scanout_plane);
 	weston_plane_release(&output->cursor_plane);
 
-	drmModeFreeProperty(output->dpms_prop);
-
 	/* Turn off hardware cursor */
 	drmModeSetCursor(b->drm.fd, output->crtc_id, 0, 0, 0);
 }
@@ -2812,6 +3075,7 @@ drm_output_disable(struct weston_output *base)
 static int
 create_output_for_connector(struct drm_backend *b,
 			    drmModeRes *resources,
+			    drmModeObjectPropertiesPtr props,
 			    drmModeConnector *connector,
 			    struct udev_device *drm_device)
 {
@@ -2847,6 +3111,8 @@ create_output_for_connector(struct drm_backend *b,
 	output->destroy_pending = 0;
 	output->disable_pending = 0;
 
+	find_and_parse_output_edid(b, output, props);
+
 	weston_output_init(&output->base, b->compositor);
 
 	wl_list_init(&output->base.mode_list);
@@ -2875,6 +3141,16 @@ create_outputs(struct drm_backend *b, struct udev_device *drm_device)
 	drmModeConnector *connector;
 	drmModeRes *resources;
 	int i;
+	static const struct drm_property_info connector_props[] = {
+		[WDRM_CONNECTOR_EDID] = { .name = "EDID" },
+		[WDRM_CONNECTOR_DPMS] = { .name = "DPMS" },
+	};
+
+	if (!drm_property_info_copy(b->props_conn, connector_props,
+				    WDRM_CONNECTOR__COUNT)) {
+		weston_log("failed to copy connector property info\n");
+		return -1;
+	}
 
 	resources = drmModeGetResources(b->drm.fd);
 	if (!resources) {
@@ -2888,20 +3164,38 @@ create_outputs(struct drm_backend *b, struct udev_device *drm_device)
 	b->max_height = resources->max_height;
 
 	for (i = 0; i < resources->count_connectors; i++) {
+		drmModeObjectPropertiesPtr props;
+		int ret;
+
 		connector = drmModeGetConnector(b->drm.fd,
 						resources->connectors[i]);
 		if (connector == NULL)
 			continue;
 
+		props = drmModeObjectGetProperties(b->drm.fd,
+						   resources->connectors[i],
+						   DRM_MODE_OBJECT_CONNECTOR);
+		if (!props) {
+			drmModeFreeConnector(connector);
+			continue;
+		}
+		drm_property_info_update(b, b->props_conn,
+					 WDRM_CONNECTOR__COUNT, props);
+
 		if (connector->connection == DRM_MODE_CONNECTED &&
 		    (b->connector == 0 ||
 		     connector->connector_id == b->connector)) {
-			if (create_output_for_connector(b, resources,
-							connector, drm_device) < 0)
-				continue;
-		} else {
-			drmModeFreeConnector(connector);
+			ret = create_output_for_connector(b, resources, props,
+							  connector,
+							  drm_device);
+			if (ret < 0)
+				weston_log("failed to create new connector\n");
+			else
+				connector = NULL; /* consumed by the output */
 		}
+
+		drmModeFreeObjectProperties(props);
+		drmModeFreeConnector(connector);
 	}
 
 	if (wl_list_empty(&b->compositor->output_list) &&
@@ -2937,6 +3231,7 @@ update_outputs(struct drm_backend *b, struct udev_device *drm_device)
 	/* collect new connects */
 	for (i = 0; i < resources->count_connectors; i++) {
 		uint32_t connector_id = resources->connectors[i];
+		drmModeObjectPropertiesPtr props;
 
 		connector = drmModeGetConnector(b->drm.fd, connector_id);
 		if (connector == NULL)
@@ -2953,14 +3248,25 @@ update_outputs(struct drm_backend *b, struct udev_device *drm_device)
 		}
 
 		connected[i] = connector_id;
+		props = drmModeObjectGetProperties(b->drm.fd, connector_id,
+						   DRM_MODE_OBJECT_CONNECTOR);
+		if (!props) {
+			drmModeFreeConnector(connector);
+			continue;
+		}
+		drm_property_info_update(b, b->props_conn,
+					 WDRM_CONNECTOR__COUNT, props);
 
 		if (drm_output_find_by_connector(b, connector_id)) {
+			drmModeFreeObjectProperties(props);
 			drmModeFreeConnector(connector);
 			continue;
 		}
 
-		create_output_for_connector(b, resources,
+		create_output_for_connector(b, resources, props,
 					    connector, drm_device);
+		drmModeFreeConnector(connector);
+		drmModeFreeObjectProperties(props);
 		weston_log("connector %d connected\n", connector_id);
 	}
 
@@ -3053,6 +3359,8 @@ drm_destroy(struct weston_compositor *ec)
 	wl_event_source_remove(b->udev_drm_source);
 	wl_event_source_remove(b->drm_source);
 
+	drm_property_info_free(b->props_conn, WDRM_CONNECTOR__COUNT);
+
 	destroy_sprites(b);
 
 	weston_compositor_shutdown(ec);
-- 
2.9.3



More information about the wayland-devel mailing list