[Intel-gfx] [PATCH 2/3] drm: Create a format/modifier blob
Brian Starkey
brian.starkey at arm.com
Wed May 3 11:00:33 UTC 2017
Hi,
Having just caught up on IRC I'm sure I'm far too late for this party,
but...
Wouldn't this whole thing be a whole lot simpler if "IN_FORMATS"
stored "pointers" to separate blobs for the format and modifier lists?
struct drm_format_modifier_blob {
#define FORMAT_BLOB_CURRENT 1
/* Version of this blob format */
u32 version;
/* Flags */
u32 flags;
/* ID of a blob storing an array of FourCCs */
u32 formats_blob_id;
/* ID of a blob storing an array of drm_format_modifiers */
u32 modifiers_blob_id;
} __packed;
I've used/written a few interfaces with offsets to
variable-length-arrays and IMO the code to handle them is always
hideous.
Added bonus: With smart enough helper code the FourCC and modifiers
blobs can be shared across planes.
-Brian
On Tue, May 02, 2017 at 10:14:27PM -0700, Ben Widawsky wrote:
>Updated blob layout (Rob, Daniel, Kristian, xerpi)
>
>Cc: Rob Clark <robdclark at gmail.com>
>Cc: Daniel Stone <daniels at collabora.com>
>Cc: Kristian H. Kristensen <hoegsberg at gmail.com>
>Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>---
> drivers/gpu/drm/drm_mode_config.c | 7 +++
> drivers/gpu/drm/drm_plane.c | 119 ++++++++++++++++++++++++++++++++++++++
> include/drm/drm_mode_config.h | 6 ++
> include/uapi/drm/drm_mode.h | 26 +++++++++
> 4 files changed, 158 insertions(+)
>
>diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>index d9862259a2a7..6bfbc3839df5 100644
>--- a/drivers/gpu/drm/drm_mode_config.c
>+++ b/drivers/gpu/drm/drm_mode_config.c
>@@ -337,6 +337,13 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> return -ENOMEM;
> dev->mode_config.gamma_lut_size_property = prop;
>
>+ prop = drm_property_create(dev,
>+ DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB,
>+ "IN_FORMATS", 0);
>+ if (!prop)
>+ return -ENOMEM;
>+ dev->mode_config.modifiers = prop;
>+
> return 0;
> }
>
>diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>index 286e183891e5..2e89e0e73435 100644
>--- a/drivers/gpu/drm/drm_plane.c
>+++ b/drivers/gpu/drm/drm_plane.c
>@@ -62,6 +62,117 @@ static unsigned int drm_num_planes(struct drm_device *dev)
> return num;
> }
>
>+struct drm_format_modifier_blob {
>+#define FORMAT_BLOB_CURRENT 1
>+ /* Version of this blob format */
>+ u32 version;
>+
>+ /* Flags */
>+ u32 flags;
>+
>+ /* Number of fourcc formats supported */
>+ u32 count_formats;
>+
>+ /* Where in this blob the formats exist (in bytes) */
>+ u32 formats_offset;
>+
>+ /* Number of drm_format_modifiers */
>+ u32 count_modifiers;
>+
>+ /* Where in this blob the modifiers exist (in bytes) */
>+ u32 modifiers_offset;
>+
>+ /* u32 formats[] */
>+ /* struct drm_format_modifier modifiers[] */
>+} __packed;
>+
>+static inline u32 *
>+formats_ptr(struct drm_format_modifier_blob *blob)
>+{
>+ return (u32 *)(((char *)blob) + blob->formats_offset);
>+}
>+
>+static inline struct drm_format_modifier *
>+modifiers_ptr(struct drm_format_modifier_blob *blob)
>+{
>+ return (struct drm_format_modifier *)(((char *)blob) + blob->modifiers_offset);
>+}
>+
>+static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane,
>+ const struct drm_plane_funcs *funcs,
>+ const uint32_t *formats, unsigned int format_count,
>+ const uint64_t *format_modifiers)
>+{
>+ const struct drm_mode_config *config = &dev->mode_config;
>+ const uint64_t *temp_modifiers = format_modifiers;
>+ unsigned int format_modifier_count = 0;
>+ struct drm_property_blob *blob = NULL;
>+ struct drm_format_modifier *mod;
>+ size_t blob_size = 0, formats_size, modifiers_size;
>+ struct drm_format_modifier_blob *blob_data;
>+ int i, j, ret = 0;
>+
>+ if (format_modifiers)
>+ while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID)
>+ format_modifier_count++;
>+
>+ formats_size = sizeof(__u32) * format_count;
>+ if (WARN_ON(!formats_size)) {
>+ /* 0 formats are never expected */
>+ return 0;
>+ }
>+
>+ modifiers_size =
>+ sizeof(struct drm_format_modifier) * format_modifier_count;
>+
>+ blob_size = ALIGN(sizeof(struct drm_format_modifier_blob), 8);
>+ blob_size += ALIGN(formats_size, 8);
>+ blob_size += modifiers_size;
>+
>+ blob = drm_property_create_blob(dev, blob_size, NULL);
>+ if (IS_ERR(blob))
>+ return -1;
>+
>+ blob_data = (struct drm_format_modifier_blob *)blob->data;
>+ blob_data->version = FORMAT_BLOB_CURRENT;
>+ blob_data->count_formats = format_count;
>+ blob_data->formats_offset = sizeof(struct drm_format_modifier_blob);
>+ blob_data->count_modifiers = format_modifier_count;
>+
>+ /* Modifiers offset is a pointer to a struct with a 64 bit field so it
>+ * should be naturally aligned to 8B.
>+ */
>+ blob_data->modifiers_offset =
>+ ALIGN(blob_data->formats_offset + formats_size, 8);
>+
>+ memcpy(formats_ptr(blob_data), formats, formats_size);
>+
>+ /* If we can't determine support, just bail */
>+ if (!funcs->format_mod_supported)
>+ goto done;
>+
>+ mod = modifiers_ptr(blob_data);
>+ for (i = 0; i < format_modifier_count; i++) {
>+ for (j = 0; j < format_count; j++) {
>+ if (funcs->format_mod_supported(plane, formats[j],
>+ format_modifiers[i])) {
>+ mod->formats |= 1 << j;
>+ }
>+ }
>+
>+ mod->modifier = format_modifiers[i];
>+ mod->offset = 0;
>+ mod->pad = 0;
>+ mod++;
>+ }
>+
>+done:
>+ drm_object_attach_property(&plane->base, config->modifiers,
>+ blob->base.id);
>+
>+ return ret;
>+}
>+
> /**
> * drm_universal_plane_init - Initialize a new universal plane object
> * @dev: DRM device
>@@ -130,6 +241,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> return -ENOMEM;
> }
>
>+ /* First driver to need more than 64 formats needs to fix this */
>+ if (WARN_ON(format_count > 64))
>+ return -EINVAL;
>+
> if (name) {
> va_list ap;
>
>@@ -177,6 +292,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> drm_object_attach_property(&plane->base, config->prop_src_h, 0);
> }
>
>+ if (config->allow_fb_modifiers)
>+ create_in_format_blob(dev, plane, funcs, formats, format_count,
>+ format_modifiers);
>+
> return 0;
> }
> EXPORT_SYMBOL(drm_universal_plane_init);
>diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>index 42981711189b..03776e659811 100644
>--- a/include/drm/drm_mode_config.h
>+++ b/include/drm/drm_mode_config.h
>@@ -757,6 +757,12 @@ struct drm_mode_config {
> */
> bool allow_fb_modifiers;
>
>+ /**
>+ * @modifiers: Plane property to list support modifier/format
>+ * combination.
>+ */
>+ struct drm_property *modifiers;
>+
> /* cursor size */
> uint32_t cursor_width, cursor_height;
>
>diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>index 8c67fc03d53d..dcdd04c55792 100644
>--- a/include/uapi/drm/drm_mode.h
>+++ b/include/uapi/drm/drm_mode.h
>@@ -665,6 +665,32 @@ struct drm_mode_atomic {
> __u64 user_data;
> };
>
>+struct drm_format_modifier {
>+ /* Bitmask of formats in get_plane format list this info applies to. The
>+ * offset allows a sliding window of which 64 formats (bits).
>+ *
>+ * Some examples:
>+ * In today's world with < 65 formats, and formats 0, and 2 are
>+ * supported
>+ * 0x0000000000000005
>+ * ^-offset = 0, formats = 5
>+ *
>+ * If the number formats grew to 128, and formats 98-102 are
>+ * supported with the modifier:
>+ *
>+ * 0x0000003c00000000 0000000000000000
>+ * ^
>+ * |__offset = 64, formats = 0x3c00000000
>+ *
>+ */
>+ uint64_t formats;
>+ uint32_t offset;
>+ uint32_t pad;
>+
>+ /* This modifier can be used with the format for this plane. */
>+ uint64_t modifier;
>+} __packed;
>+
> /**
> * Create a new 'blob' data property, copying length bytes from data pointer,
> * and returning new blob ID.
>--
>2.12.2
>
>_______________________________________________
>dri-devel mailing list
>dri-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the Intel-gfx
mailing list