[RFC] drm: add overlays as first class KMS objects

[RFC] drm: add overlays as first class KMS objects

[Jesse Barnes]

Looking for comments on this.  Obviously if we're going to add a new type
of KMS object, we'd better get the ioctl more or less right to begin with,
which means having all the attributes we'd like to track, plus some
padding, available from the outset.

So I'd like comments on this; the whole approach may be broken for things
like OMAP; if so I'd like to hear about it now.  Overall, overlays are
treated a little like CRTCs, but without associated modes our encoder
trees hanging off of them.  That is, they can be enabled with a specific
fb attached at a specific location, but they don't have to worry about
mode setting, per se (though they do need to have an associated CRTC to
actually pump their pixels out, post-blend).

Flipping could be done either with the existing ioctl by updating the fb
base pointer, or through a new flip ioctl similar to what we have already,
but taking an overlay object instead of a CRTC.

Overlays are a bit like half-CRTCs.  They have a location and fb, but
don't drive outputs directly.  Add support for handling them to the core
KMS code.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/drm_crtc.c |  219 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm.h          |    3 +
 include/drm/drm_crtc.h     |   61 ++++++++++++
 include/drm/drm_mode.h     |   39 ++++++++
 4 files changed, 322 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 799e149..77ff9e0 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -533,6 +533,34 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
 }
 EXPORT_SYMBOL(drm_encoder_cleanup);
 
+void drm_overlay_init(struct drm_device *dev, struct drm_overlay *overlay,
+		      const struct drm_overlay_funcs *funcs)
+{
+	mutex_lock(&dev->mode_config.mutex);
+
+	overlay->dev = dev;
+	drm_mode_object_get(dev, &overlay->base, DRM_MODE_OBJECT_OVERLAY);
+	overlay->funcs = funcs;
+
+	list_add_tail(&overlay->head, &dev->mode_config.overlay_list);
+	dev->mode_config.num_overlay++;
+
+	mutex_unlock(&dev->mode_config.mutex);
+}
+EXPORT_SYMBOL(drm_overlay_init);
+
+void drm_overlay_cleanup(struct drm_overlay *overlay)
+{
+	struct drm_device *dev = overlay->dev;
+
+	mutex_lock(&dev->mode_config.mutex);
+	drm_mode_object_put(dev, &overlay->base);
+	list_del(&overlay->head);
+	dev->mode_config.num_overlay--;
+	mutex_unlock(&dev->mode_config.mutex);
+}
+EXPORT_SYMBOL(drm_overlay_cleanup);
+
 /**
  * drm_mode_create - create a new display mode
  * @dev: DRM device
@@ -864,6 +892,7 @@ void drm_mode_config_init(struct drm_device *dev)
 	INIT_LIST_HEAD(&dev->mode_config.encoder_list);
 	INIT_LIST_HEAD(&dev->mode_config.property_list);
 	INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
+	INIT_LIST_HEAD(&dev->mode_config.overlay_list);
 	idr_init(&dev->mode_config.crtc_idr);
 
 	mutex_lock(&dev->mode_config.mutex);
@@ -1467,6 +1496,196 @@ out:
 }
 
 /**
+ * drm_mode_getoverlay_res - get overlay info
+ * @dev: DRM device
+ * @data: ioctl data
+ * @file_priv: DRM file info
+ *
+ * Return an overlay count and set of IDs.
+ */
+int drm_mode_getoverlay_res(struct drm_device *dev, void *data,
+			    struct drm_file *file_priv)
+{
+	struct drm_mode_get_overlay_res *overlay_resp = data;
+	struct drm_mode_config *config;
+	struct drm_overlay *overlay;
+	uint32_t __user *overlay_ptr;
+	int copied = 0, ret = 0;
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -EINVAL;
+
+	mutex_lock(&dev->mode_config.mutex);
+	config = &dev->mode_config;
+
+	/*
+	 * This ioctl is called twice, once to determine how much space is
+	 * needed, and the 2nd time to fill it.
+	 */
+	if (config->num_overlay &&
+	    (overlay_resp->count_overlays >= config->num_overlay)) {
+		overlay_ptr = (uint32_t *)(unsigned long)overlay_resp->overlay_id_ptr;
+
+		list_for_each_entry(overlay, &config->overlay_list, head) {
+			if (put_user(overlay->base.id, overlay_ptr + copied)) {
+				ret = -EFAULT;
+				goto out;
+			}
+			copied++;
+		}
+	}
+	overlay_resp->count_overlays = config->num_overlay;
+
+out:
+	mutex_unlock(&dev->mode_config.mutex);
+	return ret;
+}
+
+/**
+ * drm_mode_getoverlay - get overlay info
+ * @dev: DRM device
+ * @data: ioctl data
+ * @file_priv: DRM file info
+ *
+ * Return overlay info, including formats supported, gamma size, any
+ * current fb, etc.
+ */
+int drm_mode_getoverlay(struct drm_device *dev, void *data,
+			struct drm_file *file_priv)
+{
+	struct drm_mode_get_overlay *overlay_resp = data;
+	struct drm_mode_object *obj;
+	struct drm_overlay *overlay;
+	uint32_t __user *format_ptr;
+	int ret = 0;
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -EINVAL;
+
+	mutex_lock(&dev->mode_config.mutex);
+	obj = drm_mode_object_find(dev, overlay_resp->overlay_id,
+				   DRM_MODE_OBJECT_OVERLAY);
+	if (!obj) {
+		ret = -EINVAL;
+		goto out;
+	}
+	overlay = obj_to_overlay(obj);
+
+	if (overlay->crtc)
+		overlay_resp->crtc_id = overlay->crtc->base.id;
+	else
+		overlay_resp->crtc_id = 0;
+
+	if (overlay->fb)
+		overlay_resp->fb_id = overlay->fb->base.id;
+	else
+		overlay_resp->fb_id = 0;
+
+	overlay_resp->overlay_id = overlay->base.id;
+	overlay_resp->possible_crtcs = overlay->possible_crtcs;
+	overlay_resp->gamma_size = overlay->gamma_size;
+	overlay_resp->crtc_x = overlay->crtc_x;
+	overlay_resp->crtc_y = overlay->crtc_y;
+	overlay_resp->x = overlay->x;
+	overlay_resp->y = overlay->y;
+
+	/*
+	 * This ioctl is called twice, once to determine how much space is
+	 * needed, and the 2nd time to fill it.
+	 */
+	if (overlay->format_count &&
+	    (overlay_resp->count_format_types >= overlay->format_count)) {
+		format_ptr = (uint32_t *)(unsigned long)overlay_resp->format_type_ptr;
+		if (copy_to_user(format_ptr,
+				 overlay->format_types,
+				 sizeof(uint32_t) * overlay->format_count)) {
+			ret = -EFAULT;
+			goto out;
+		}
+	}
+	overlay_resp->count_format_types = overlay->format_count;
+
+out:
+	mutex_unlock(&dev->mode_config.mutex);
+	return ret;
+}
+
+/**
+ * drm_mode_setoverlay - set up or tear down an overlay
+ * @dev: DRM device
+ * @data: ioctl data*
+ * @file_prive: DRM file info
+ *
+ * Set overlay info, including placement, fb, scaling, and other factors.
+ * Or pass a NULL fb to disable.
+ */
+int drm_mode_setoverlay(struct drm_device *dev, void *data,
+			struct drm_file *file_priv)
+{
+	struct drm_mode_set_overlay *overlay_req = data;
+	struct drm_mode_object *obj;
+	struct drm_overlay *overlay;
+	struct drm_crtc *crtc;
+	struct drm_framebuffer *fb;
+	int ret = 0;
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -EINVAL;
+
+	mutex_lock(&dev->mode_config.mutex);
+
+	/*
+	 * First, find the overlay, crtc, and fb objects.  If not available,
+	 * we don't bother to call the driver.
+	 */
+	obj = drm_mode_object_find(dev, overlay_req->overlay_id,
+				   DRM_MODE_OBJECT_OVERLAY);
+	if (!obj) {
+		DRM_DEBUG_KMS("Unknown overlay ID %d\n",
+			      overlay_req->overlay_id);
+		ret = -EINVAL;
+		goto out;
+	}
+	overlay = obj_to_overlay(obj);
+
+	/* No fb means shut it down */
+	if (!overlay_req->fb_id) {
+		overlay->funcs->disable_overlay(overlay);
+		goto out;
+	}
+
+	obj = drm_mode_object_find(dev, overlay_req->crtc_id,
+				   DRM_MODE_OBJECT_CRTC);
+	if (!obj) {
+		DRM_DEBUG_KMS("Unknown crtc ID %d\n",
+			      overlay_req->crtc_id);
+		ret = -EINVAL;
+		goto out;
+	}
+	crtc = obj_to_crtc(obj);
+
+	obj = drm_mode_object_find(dev, overlay_req->fb_id,
+				   DRM_MODE_OBJECT_FB);
+	if (!obj) {
+		DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
+			      overlay_req->fb_id);
+		ret = -EINVAL;
+		goto out;
+	}
+	fb = obj_to_fb(obj);
+
+	ret = overlay->funcs->update_overlay(overlay, crtc, fb,
+					     overlay_req->crtc_x,
+					     overlay_req->crtc_y,
+					     overlay_req->x, overlay_req->y);
+
+out:
+	mutex_unlock(&dev->mode_config.mutex);
+
+	return ret;
+}
+
+/**
  * drm_mode_setcrtc - set CRTC configuration
  * @inode: inode from the ioctl
  * @filp: file * from the ioctl
diff --git a/include/drm/drm.h b/include/drm/drm.h
index 4be33b4..47909af 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -714,6 +714,9 @@ struct drm_get_cap {
 #define DRM_IOCTL_MODE_CREATE_DUMB DRM_IOWR(0xB2, struct drm_mode_create_dumb)
 #define DRM_IOCTL_MODE_MAP_DUMB    DRM_IOWR(0xB3, struct drm_mode_map_dumb)
 #define DRM_IOCTL_MODE_DESTROY_DUMB    DRM_IOWR(0xB4, struct drm_mode_destroy_dumb)
+#define DRM_IOCTL_MODE_GETOVERLAYRESOURCES DRM_IOWR(0xB5, struct drm_mode_get_overlay_res)
+#define DRM_IOCTL_MODE_GETOVERLAY	DRM_IOWR(0xB6, struct drm_mode_get_overlay)
+#define DRM_IOCTL_MODE_SETOVERLAY	DRM_IOWR(0xB7, struct drm_mode_set_overlay)
 
 /**
  * Device specific ioctls should only be in their respective headers
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index b786a24..07a9025 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -44,6 +44,7 @@ struct drm_framebuffer;
 #define DRM_MODE_OBJECT_PROPERTY 0xb0b0b0b0
 #define DRM_MODE_OBJECT_FB 0xfbfbfbfb
 #define DRM_MODE_OBJECT_BLOB 0xbbbbbbbb
+#define DRM_MODE_OBJECT_OVERLAY 0xeeeeeeee
 
 struct drm_mode_object {
 	uint32_t id;
@@ -276,6 +277,7 @@ struct drm_crtc;
 struct drm_connector;
 struct drm_encoder;
 struct drm_pending_vblank_event;
+struct drm_overlay;
 
 /**
  * drm_crtc_funcs - control CRTCs for a given device
@@ -523,6 +525,57 @@ struct drm_connector {
 };
 
 /**
+ * drm_overlay_funcs - driver overlay control functions
+ * @update_overlay: update the overlay configuration
+ */
+struct drm_overlay_funcs {
+	int (*update_overlay)(struct drm_overlay *overlay,
+			      struct drm_crtc *crtc, struct drm_framebuffer *fb,
+			      int crtc_x, int crtc_y, int x, int y);
+	void (*disable_overlay)(struct drm_overlay *overlay);
+};
+
+/**
+ * drm_overlay - central DRM overlay control structure
+ * @dev: DRM device this overlay belongs to
+ * @kdev: kernel device
+ * @attr: kdev attributes
+ * @head: for list management
+ * @base: base mode object
+ * @crtc_x: x position of overlay (relative to pipe base)
+ * @crtc_y: y position of overlay
+ * @x: x offset into fb
+ * @y: y offset into fb
+ * @crtc: CRTC this overlay is feeding
+ */
+struct drm_overlay {
+	struct drm_device *dev;
+	struct device kdev;
+	struct device_attribute *attr;
+	struct list_head head;
+
+	struct drm_mode_object base;
+
+	int crtc_x, crtc_y;
+	int x, y;
+	uint32_t possible_crtcs;
+	uint32_t *format_types;
+	uint32_t format_count;
+
+	struct drm_crtc *crtc;
+	struct drm_framebuffer *fb;
+
+	/* CRTC gamma size for reporting to userspace */
+	uint32_t gamma_size;
+	uint16_t *gamma_store;
+
+	bool enabled;
+
+	const struct drm_overlay_funcs *funcs;
+	void *helper_private;
+};
+
+/**
  * struct drm_mode_set
  *
  * Represents a single crtc the connectors that it drives with what mode
@@ -576,6 +629,8 @@ struct drm_mode_config {
 	struct list_head connector_list;
 	int num_encoder;
 	struct list_head encoder_list;
+	int num_overlay;
+	struct list_head overlay_list;
 
 	int num_crtc;
 	struct list_head crtc_list;
@@ -628,6 +683,7 @@ struct drm_mode_config {
 #define obj_to_fb(x) container_of(x, struct drm_framebuffer, base)
 #define obj_to_property(x) container_of(x, struct drm_property, base)
 #define obj_to_blob(x) container_of(x, struct drm_property_blob, base)
+#define obj_to_overlay(x) container_of(x, struct drm_overlay, base)
 
 
 extern void drm_crtc_init(struct drm_device *dev,
@@ -647,6 +703,11 @@ extern void drm_encoder_init(struct drm_device *dev,
 			     const struct drm_encoder_funcs *funcs,
 			     int encoder_type);
 
+extern void drm_overlay_init(struct drm_device *dev,
+			     struct drm_overlay *overlay,
+			     const struct drm_overlay_funcs *funcs);
+extern void drm_overlay_cleanup(struct drm_overlay *overlay);
+
 extern void drm_encoder_cleanup(struct drm_encoder *encoder);
 
 extern char *drm_get_connector_name(struct drm_connector *connector);
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index ae6b7a3..349052a 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -120,6 +120,45 @@ struct drm_mode_crtc {
 	struct drm_mode_modeinfo mode;
 };
 
+#define DRM_MODE_OVERLAY_FORMAT_YUV422		1 /* YUV 4:2:2 packed */
+#define DRM_MODE_OVERLAY_FORMAT_RGBX101010	2 /* RGB 10bpc, ign. alpha */
+#define DRM_MODE_OVERLAY_FORMAT_RGBX888		3 /* Standard x:8:8:8 RGB */
+#define DRM_MODE_OVERLAY_FORMAT_RGBX161616	4 /* x:16:16:16 float RGB */
+
+/* Overlays blend with or override other bits on the CRTC */
+struct drm_mode_set_overlay {
+	__u32 overlay_id;
+	__u32 crtc_id;
+	__u32 fb_id; /* contains surface format type */
+
+	__u32 crtc_x, crtc_y;
+	__u32 x, y;
+
+	/* FIXME: color key/mask, scaling, z-order, other? */
+};
+
+struct drm_mode_get_overlay {
+	__u64 format_type_ptr;
+	__u32 overlay_id;
+
+	__u32 crtc_id;
+	__u32 fb_id;
+
+	__u32 crtc_x, crtc_y;
+	__u32 x, y;
+
+	__u32 possible_crtcs;
+	__u32 gamma_size;
+
+	__u32 count_format_types;
+};
+
+
+struct drm_mode_get_overlay_res {
+	__u64 overlay_id_ptr;
+	__u32 count_overlays;
+};
+
 #define DRM_MODE_ENCODER_NONE	0
 #define DRM_MODE_ENCODER_DAC	1
 #define DRM_MODE_ENCODER_TMDS	2
-- 
1.7.4.1

Re: [RFC] drm: add overlays as first class KMS objects

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 570 bytes --]

On Mon, 25 Apr 2011 15:12:20 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Overlays are a bit like half-CRTCs.  They have a location and fb, but
> don't drive outputs directly.  Add support for handling them to the core
> KMS code.

Are overlays/underlays not associated with a specific CRTC? To my mind,
overlays are another scanout buffer associated with a specific CRTC, so
you'd create a scanout buffer and attach that to a specific scanout slot
in a crtc, with the 'default' slot being the usual graphics plane.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] drm: add overlays as first class KMS objects

[Jesse Barnes]

On Mon, 25 Apr 2011 16:16:18 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Mon, 25 Apr 2011 15:12:20 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > Overlays are a bit like half-CRTCs.  They have a location and fb, but
> > don't drive outputs directly.  Add support for handling them to the core
> > KMS code.
> 
> Are overlays/underlays not associated with a specific CRTC? To my mind,
> overlays are another scanout buffer associated with a specific CRTC, so
> you'd create a scanout buffer and attach that to a specific scanout slot
> in a crtc, with the 'default' slot being the usual graphics plane.

Yes, that matches my understanding as well.  I've deliberately made the
implementation flexible there though, under the assumption that some
hardware allows a plane to be directed at more than one CRTC (though
probably not simultaneously).

Arguably, this is something we should have done when the
connector/encoder split was done (making planes in general first class
objects).  But with today's code, treating a CRTC as a pixel pump and a
primary plane seems fine, with overlays tacked onto the side as
secondary pixel sources but tied to a specific CRTC.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFC] drm: add overlays as first class KMS objects

[Stéphane Marchesin]

On Mon, Apr 25, 2011 at 16:22, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 25 Apr 2011 16:16:18 -0700
> Keith Packard <keithp@keithp.com> wrote:
>
>> On Mon, 25 Apr 2011 15:12:20 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>>
>> > Overlays are a bit like half-CRTCs.  They have a location and fb, but
>> > don't drive outputs directly.  Add support for handling them to the core
>> > KMS code.
>>
>> Are overlays/underlays not associated with a specific CRTC? To my mind,
>> overlays are another scanout buffer associated with a specific CRTC, so
>> you'd create a scanout buffer and attach that to a specific scanout slot
>> in a crtc, with the 'default' slot being the usual graphics plane.
>
> Yes, that matches my understanding as well.  I've deliberately made the
> implementation flexible there though, under the assumption that some
> hardware allows a plane to be directed at more than one CRTC (though
> probably not simultaneously).
>
> Arguably, this is something we should have done when the
> connector/encoder split was done (making planes in general first class
> objects).  But with today's code, treating a CRTC as a pixel pump and a
> primary plane seems fine, with overlays tacked onto the side as
> secondary pixel sources but tied to a specific CRTC.
>

What is the plan for supporting multiple formats? When I looked at
this for nouveau it ended up growing out of control when adding
support for all the YUV (planar, packed, 12 or 16 bpp formats) and RGB
format combinations.

Stéphane

Re: [RFC] drm: add overlays as first class KMS objects

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 1062 bytes --]

On Mon, 25 Apr 2011 16:22:16 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> Yes, that matches my understanding as well.  I've deliberately made the
> implementation flexible there though, under the assumption that some
> hardware allows a plane to be directed at more than one CRTC (though
> probably not simultaneously).

So you create a scanout buffer and assign it to the appropriate slot in
the CRTC. Describing *how* the CRTC mixes pixels would be a good idea,
so the ordering of the slots might be relevant, and they might have a
blend mode (per-pixel alpha values, color key, separate alpha plane, etc).

> Arguably, this is something we should have done when the
> connector/encoder split was done (making planes in general first class
> objects).  But with today's code, treating a CRTC as a pixel pump and a
> primary plane seems fine, with overlays tacked onto the side as
> secondary pixel sources but tied to a specific CRTC.

I know of hardware that needs a lot more than one overlay...

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] drm: add overlays as first class KMS objects

[Jesse Barnes]

On Mon, 25 Apr 2011 16:35:20 -0700
Stéphane Marchesin <stephane.marchesin@gmail.com> wrote:

> On Mon, Apr 25, 2011 at 16:22, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > On Mon, 25 Apr 2011 16:16:18 -0700
> > Keith Packard <keithp@keithp.com> wrote:
> >
> >> On Mon, 25 Apr 2011 15:12:20 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >>
> >> > Overlays are a bit like half-CRTCs.  They have a location and fb, but
> >> > don't drive outputs directly.  Add support for handling them to the core
> >> > KMS code.
> >>
> >> Are overlays/underlays not associated with a specific CRTC? To my mind,
> >> overlays are another scanout buffer associated with a specific CRTC, so
> >> you'd create a scanout buffer and attach that to a specific scanout slot
> >> in a crtc, with the 'default' slot being the usual graphics plane.
> >
> > Yes, that matches my understanding as well.  I've deliberately made the
> > implementation flexible there though, under the assumption that some
> > hardware allows a plane to be directed at more than one CRTC (though
> > probably not simultaneously).
> >
> > Arguably, this is something we should have done when the
> > connector/encoder split was done (making planes in general first class
> > objects).  But with today's code, treating a CRTC as a pixel pump and a
> > primary plane seems fine, with overlays tacked onto the side as
> > secondary pixel sources but tied to a specific CRTC.
> >
> 
> What is the plan for supporting multiple formats? When I looked at
> this for nouveau it ended up growing out of control when adding
> support for all the YUV (planar, packed, 12 or 16 bpp formats) and RGB
> format combinations.

I know there are a ton of surface formats, but I don't want to restrict
what drivers can export as supported.

I was planning on adding a new fb ioctl to allow us to create fbs with
specific surface format types.  We could either enumerate all of the
ones we support (a list which will grow as drivers and devices are
added) or try to factor out commit bits into a separate surface struct:

struct drm_mode_surface {
	enum components; /* YUV, VUY, RGB, BGR, ARGB, ... */
	int depth;
	enum packing; /* some list of packing types? */
	...
};

Even if we did that we may end up needing special surface formats not
covered by the general description.  I think pixman just ends up
enumerating them all; it's a little messy, but we know it would be
extensible at least. :)

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFC] drm: add overlays as first class KMS objects

[Jesse Barnes]

On Mon, 25 Apr 2011 16:37:46 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Mon, 25 Apr 2011 16:22:16 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > Yes, that matches my understanding as well.  I've deliberately made the
> > implementation flexible there though, under the assumption that some
> > hardware allows a plane to be directed at more than one CRTC (though
> > probably not simultaneously).
> 
> So you create a scanout buffer and assign it to the appropriate slot in
> the CRTC. Describing *how* the CRTC mixes pixels would be a good idea,
> so the ordering of the slots might be relevant, and they might have a
> blend mode (per-pixel alpha values, color key, separate alpha plane, etc).

Yeah, pixel blending and z order should be exposed.  I think that means
returning more info in the get_overlay_res ioctl.  Hm.. not sure of the
best way of representing that...

> > Arguably, this is something we should have done when the
> > connector/encoder split was done (making planes in general first class
> > objects).  But with today's code, treating a CRTC as a pixel pump and a
> > primary plane seems fine, with overlays tacked onto the side as
> > secondary pixel sources but tied to a specific CRTC.
> 
> I know of hardware that needs a lot more than one overlay...

Yeah I don't want to preclude that (and I don't think this
implementation does; it allows a many to one relationship between
overlays and CRTCs). TVs in particular can get messy since the z order
may be semi-fixed between certain planes, and blending may be limited to
different types at different layers.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFC] drm: add overlays as first class KMS objects

[Alex Deucher]

On Mon, Apr 25, 2011 at 7:22 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 25 Apr 2011 16:16:18 -0700
> Keith Packard <keithp@keithp.com> wrote:
>
>> On Mon, 25 Apr 2011 15:12:20 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>>
>> > Overlays are a bit like half-CRTCs.  They have a location and fb, but
>> > don't drive outputs directly.  Add support for handling them to the core
>> > KMS code.
>>
>> Are overlays/underlays not associated with a specific CRTC? To my mind,
>> overlays are another scanout buffer associated with a specific CRTC, so
>> you'd create a scanout buffer and attach that to a specific scanout slot
>> in a crtc, with the 'default' slot being the usual graphics plane.
>
> Yes, that matches my understanding as well.  I've deliberately made the
> implementation flexible there though, under the assumption that some
> hardware allows a plane to be directed at more than one CRTC (though
> probably not simultaneously).

A lot of older hardware had one overlay that could be sourced to any
crtc, just not simultaneously.  The tricky part is the formats and
capabilities: alpha blending, color/chromakey, gamma correction, etc.
Even the current crtc gamma stuff is somewhat lacking in in terms of
what hardware is capable of (PWL vs. LUT, user defined conversion
matrices, gamut remapping, etc.).

Alex

Re: [RFC] drm: add overlays as first class KMS objects

[Jesse Barnes]

On Mon, 25 Apr 2011 20:28:20 -0400
Alex Deucher <alexdeucher@gmail.com> wrote:

> On Mon, Apr 25, 2011 at 7:22 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > On Mon, 25 Apr 2011 16:16:18 -0700
> > Keith Packard <keithp@keithp.com> wrote:
> >
> >> On Mon, 25 Apr 2011 15:12:20 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >>
> >> > Overlays are a bit like half-CRTCs.  They have a location and fb, but
> >> > don't drive outputs directly.  Add support for handling them to the core
> >> > KMS code.
> >>
> >> Are overlays/underlays not associated with a specific CRTC? To my mind,
> >> overlays are another scanout buffer associated with a specific CRTC, so
> >> you'd create a scanout buffer and attach that to a specific scanout slot
> >> in a crtc, with the 'default' slot being the usual graphics plane.
> >
> > Yes, that matches my understanding as well.  I've deliberately made the
> > implementation flexible there though, under the assumption that some
> > hardware allows a plane to be directed at more than one CRTC (though
> > probably not simultaneously).
> 
> A lot of older hardware had one overlay that could be sourced to any
> crtc, just not simultaneously.  The tricky part is the formats and
> capabilities: alpha blending, color/chromakey, gamma correction, etc.
> Even the current crtc gamma stuff is somewhat lacking in in terms of
> what hardware is capable of (PWL vs. LUT, user defined conversion
> matrices, gamut remapping, etc.).

Right, this implementation allows an overlay to be tied to any crtc
listed in the possible_crtcs mask (matching the other possible_*
fields), but only one at a time.  I think that's fairly common.

Agree about formats and capabilities.  I think enumerating available
formats is best, perhaps making that a driver specific array if we
can't agree on a common set.

Dealing with color correction could also be driver specific; once
the client has an overlay id it can use driver specific ioctls to
get/set specifics.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFC] drm: add overlays as first class KMS objects

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 591 bytes --]

On Mon, 25 Apr 2011 16:52:58 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> struct drm_mode_surface {
> 	enum components; /* YUV, VUY, RGB, BGR, ARGB, ... */
> 	int depth;
> 	enum packing; /* some list of packing types? */
> 	...
> };

Might just make a uint32_t 'type', predefine some obvious types and
allow drivers to provide more, perhaps with a magic 'vendor' bit so it's
easy to add more standard types. And, these are scanout buffers, I think
it would be nice to use that in the name rather than the
all-too-overused 'surface'.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] drm: add overlays as first class KMS objects

[Alan Cox]

> I was planning on adding a new fb ioctl to allow us to create fbs with
> specific surface format types.  We could either enumerate all of the
> ones we support (a list which will grow as drivers and devices are
> added) or try to factor out commit bits into a separate surface struct:
> 
> struct drm_mode_surface {
> 	enum components; /* YUV, VUY, RGB, BGR, ARGB, ... */
> 	int depth;
> 	enum packing; /* some list of packing types? */
> 	...
> };


Any reason for not just using the Video4Linux 2 formats here, they've
been enumerating video formats for some time already.

Re: [RFC] drm: add overlays as first class KMS objects

[Alan Cox]

> A lot of older hardware had one overlay that could be sourced to any
> crtc, just not simultaneously.  The tricky part is the formats and
> capabilities: alpha blending, color/chromakey, gamma correction, etc.
> Even the current crtc gamma stuff is somewhat lacking in in terms of
> what hardware is capable of (PWL vs. LUT, user defined conversion
> matrices, gamut remapping, etc.).

Rather than re-inventing enough wheels to run a large truck would it not
make sense to make hardware sourced overlays Video4Linux objects in their
entirity so you can just say "attach V4L object A as overlay B"

That would provide format definitions, provide control interfaces for
the surface (eg for overlays of cameras such as on some of the Intel
embedded and non PC devices), give you an existing well understood API.

For some hardware you are going to need this integration anyway so that
you can do things like move a GEM object which is currently a DMA target
of a capture device (as well as to fence it).

For a software surface you could either expose it as a V4L object that
is GEM or fb memory backed or at least use the same descriptions so that
the kernel has a consistent set of descriptions for formats and we don't
have user libraries doing adhoc format translation crap.

A lot of capture hardware would map very nicely onto GEM objects I
suspect and if you want to merge live video into Wayland it seems a
logical path ?

Alan

Re: [RFC] drm: add overlays as first class KMS objects

[Jerome Glisse]

On Mon, Apr 25, 2011 at 8:33 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 25 Apr 2011 20:28:20 -0400
> Alex Deucher <alexdeucher@gmail.com> wrote:
>
>> On Mon, Apr 25, 2011 at 7:22 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> > On Mon, 25 Apr 2011 16:16:18 -0700
>> > Keith Packard <keithp@keithp.com> wrote:
>> >
>> >> On Mon, 25 Apr 2011 15:12:20 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> >>
>> >> > Overlays are a bit like half-CRTCs.  They have a location and fb, but
>> >> > don't drive outputs directly.  Add support for handling them to the core
>> >> > KMS code.
>> >>
>> >> Are overlays/underlays not associated with a specific CRTC? To my mind,
>> >> overlays are another scanout buffer associated with a specific CRTC, so
>> >> you'd create a scanout buffer and attach that to a specific scanout slot
>> >> in a crtc, with the 'default' slot being the usual graphics plane.
>> >
>> > Yes, that matches my understanding as well.  I've deliberately made the
>> > implementation flexible there though, under the assumption that some
>> > hardware allows a plane to be directed at more than one CRTC (though
>> > probably not simultaneously).
>>
>> A lot of older hardware had one overlay that could be sourced to any
>> crtc, just not simultaneously.  The tricky part is the formats and
>> capabilities: alpha blending, color/chromakey, gamma correction, etc.
>> Even the current crtc gamma stuff is somewhat lacking in in terms of
>> what hardware is capable of (PWL vs. LUT, user defined conversion
>> matrices, gamut remapping, etc.).
>
> Right, this implementation allows an overlay to be tied to any crtc
> listed in the possible_crtcs mask (matching the other possible_*
> fields), but only one at a time.  I think that's fairly common.
>
> Agree about formats and capabilities.  I think enumerating available
> formats is best, perhaps making that a driver specific array if we
> can't agree on a common set.

I would rather have format be common to all hardware, rather than
having a list per hardware. uint32_t would give enough room to add all
formats even if one format is only supported by one hardware only (at
one point in time). Also this would allow to have second dumb way to
allocate scanout buffer in non driver specific way. Maybe we need
another ioctl to query support format somethings like :

struct drm_format_supported {
uint32_t               nformats;
uint32_t __user  *formats;
};

Userspace supply an array of format and driver overwritte entry that
are not supported with 0, 0 would be a special INVALID format.

> Dealing with color correction could also be driver specific; once
> the client has an overlay id it can use driver specific ioctls to
> get/set specifics.
>
> --
> Jesse Barnes, Intel Open Source Technology Center

Is there that many different way to do color corrections ?

Cheers,
Jerome

Re: [RFC] drm: add overlays as first class KMS objects

[Alan Cox]

> having a list per hardware. uint32_t would give enough room to add all
> formats even if one format is only supported by one hardware only (at

It would indeed. A point realised by the Amiga designers in 1985 and
turned into a standard of sorts with a registry and process and adopted
by folks like Apple and Microsoft.

See www.fourcc.org. 4CC is already used by the kernel for Video4Linux.

Re: [RFC] drm: add overlays as first class KMS objects

[Jerome Glisse]

On Tue, Apr 26, 2011 at 10:16 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> having a list per hardware. uint32_t would give enough room to add all
>> formats even if one format is only supported by one hardware only (at
>
> It would indeed. A point realised by the Amiga designers in 1985 and
> turned into a standard of sorts with a registry and process and adopted
> by folks like Apple and Microsoft.
>
> See www.fourcc.org. 4CC is already used by the kernel for Video4Linux.
>

I think 4cc it bit useless with RGB stuff (or maybe i just don't
understand 4cc). For instance i think we want uniq different id for
RGB555, RGB565,RGBX8888, RGBA8888, BGRA8888 ... it seems 4cc just says
rgb and than rely on additional informations for color order or
components size.

Cheers,
Jerome

Re: [RFC] drm: add overlays as first class KMS objects

[Jesse Barnes]

On Tue, 26 Apr 2011 11:01:30 +0100
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > A lot of older hardware had one overlay that could be sourced to any
> > crtc, just not simultaneously.  The tricky part is the formats and
> > capabilities: alpha blending, color/chromakey, gamma correction, etc.
> > Even the current crtc gamma stuff is somewhat lacking in in terms of
> > what hardware is capable of (PWL vs. LUT, user defined conversion
> > matrices, gamut remapping, etc.).
> 
> Rather than re-inventing enough wheels to run a large truck would it not
> make sense to make hardware sourced overlays Video4Linux objects in their
> entirity so you can just say "attach V4L object A as overlay B"
> 
> That would provide format definitions, provide control interfaces for
> the surface (eg for overlays of cameras such as on some of the Intel
> embedded and non PC devices), give you an existing well understood API.
> 
> For some hardware you are going to need this integration anyway so that
> you can do things like move a GEM object which is currently a DMA target
> of a capture device (as well as to fence it).
> 
> For a software surface you could either expose it as a V4L object that
> is GEM or fb memory backed or at least use the same descriptions so that
> the kernel has a consistent set of descriptions for formats and we don't
> have user libraries doing adhoc format translation crap.
> 
> A lot of capture hardware would map very nicely onto GEM objects I
> suspect and if you want to merge live video into Wayland it seems a
> logical path ?

Thanks Alan, of course that's a good idea, I'll see about integrating
the two.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFC] drm: add overlays as first class KMS objects

[Ville Syrjälä]

On Mon, Apr 25, 2011 at 04:16:18PM -0700, Keith Packard wrote:
> On Mon, 25 Apr 2011 15:12:20 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > Overlays are a bit like half-CRTCs.  They have a location and fb, but
> > don't drive outputs directly.  Add support for handling them to the core
> > KMS code.
> 
> Are overlays/underlays not associated with a specific CRTC? To my mind,
> overlays are another scanout buffer associated with a specific CRTC, so
> you'd create a scanout buffer and attach that to a specific scanout slot
> in a crtc, with the 'default' slot being the usual graphics plane.

And what if you don't have a "default" plane as such. For example, OMAP3 
has one graphics plane and two video planes, and two output paths. Each
of the planes can be assigned to zero or one outputs. To accomodate this,
the design should allow for CRTCs without any scanout buffers.

Also a glance at DirectFB and OpenWF Display APIs might be helpful.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

Re: [RFC] drm: add overlays as first class KMS objects

[Alan Cox]

> I think 4cc it bit useless with RGB stuff (or maybe i just don't
> understand 4cc). For instance i think we want uniq different id for
> RGB555, RGB565,RGBX8888, RGBA8888, BGRA8888 ... it seems 4cc just says
> rgb and than rely on additional informations for color order or
> components size.

Yes - grep "fourcc" include/linux/videodev2.h

plus see the docs - I think its sufficient for pretty much all of it we
would end up needing except maybe a few texture formats like S3TC (which
are of course 4CC codes too)

Alan

Re: [RFC] drm: add overlays as first class KMS objects

[Jesse Barnes]

On Tue, 26 Apr 2011 18:20:03 +0300
Ville Syrjälä <syrjala@sci.fi> wrote:

> On Mon, Apr 25, 2011 at 04:16:18PM -0700, Keith Packard wrote:
> > On Mon, 25 Apr 2011 15:12:20 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > 
> > > Overlays are a bit like half-CRTCs.  They have a location and fb, but
> > > don't drive outputs directly.  Add support for handling them to the core
> > > KMS code.
> > 
> > Are overlays/underlays not associated with a specific CRTC? To my mind,
> > overlays are another scanout buffer associated with a specific CRTC, so
> > you'd create a scanout buffer and attach that to a specific scanout slot
> > in a crtc, with the 'default' slot being the usual graphics plane.
> 
> And what if you don't have a "default" plane as such. For example, OMAP3 
> has one graphics plane and two video planes, and two output paths. Each
> of the planes can be assigned to zero or one outputs. To accomodate this,
> the design should allow for CRTCs without any scanout buffers.
> 
> Also a glance at DirectFB and OpenWF Display APIs might be helpful.

The current drm crtc code ties together a crtc and fb, but it wouldn't
be too hard to split it out a little (such that passing a null fb on a
mode set wouldn't disable the crtc, or attaching a new scanout surface
to a crtc would enable it) to support something like that.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFC] drm: add overlays as first class KMS objects

[Alan Cox]

> And what if you don't have a "default" plane as such. For example, OMAP3 
> has one graphics plane and two video planes, and two output paths. Each
> of the planes can be assigned to zero or one outputs. To accomodate this,
> the design should allow for CRTCs without any scanout buffers.

Some TV type stuff is a bit like that - well there may be a scanout buffer
but its on a protected hardware path and no part of the system except
certain bits of hardware can touch it from decrypt to output connector.

Clearly a scanout buffer isn't the only way to describe what a crtc is
outputting and you need a somewhat more flexible handle including one you
can acquire somehow to represent other objects like capture buffers,
protected planes and live video merges.

Alan

Re: [RFC] drm: add overlays as first class KMS objects

[Daniel Vetter]

Hi Jesse,

I like it. It's a bit of a chicken-egg api design problem, but if a
proof-of-concept
implementation exists for an embedded chip plus something to check whether
it's good enough to implement Xv on ancient hw (intel overlay or for the qemu
kms driver, if that could do this), that should be good enough.

A few comments on the ioctl interface.

> +/* Overlays blend with or override other bits on the CRTC */
> +struct drm_mode_set_overlay {
> +       __u32 overlay_id;
> +       __u32 crtc_id;
> +       __u32 fb_id; /* contains surface format type */
> +
> +       __u32 crtc_x, crtc_y;
> +       __u32 x, y;
> +
> +       /* FIXME: color key/mask, scaling, z-order, other? */
> +};

I think scaling is required, you can't implement Xv without that. The
problem is some arbitraray
hw range restrictions, but returning -EINVAL if they're violated looks okay. So

float scale_x, scale_y;

should be good enough. For the remaining things (color conversion,
blending, ...) I don't think
there's any generic way to map hw capabilities (without going insane).
I think a bunch of
properties (with standard stuff for gamma, color key, ...) would be
perfect for that. If we
set the defaults such that it Just Works for Xv (after setting the
color key, if present), that
would be great!

> +struct drm_mode_get_overlay {
> +       __u64 format_type_ptr;
> +       __u32 overlay_id;
> +
> +       __u32 crtc_id;
> +       __u32 fb_id;
> +
> +       __u32 crtc_x, crtc_y;
> +       __u32 x, y;
> +
> +       __u32 possible_crtcs;
> +       __u32 gamma_size;
> +
> +       __u32 count_format_types;
> +};

Imo the real problem with formats is stride restrictions and other hw
restrictions (tiling, ...).
ARM/v4l people seem to want that to be in the kernel so that they can
e.g. dma decoded
video streams directly to the gpu (also for other stuff). Perhaps we
want to extend the
create_dumb_gem_object ioctl for that use case, but I'm not too
convinced that this belongs
into the kernel.

Cheers, Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

Re: [RFC] drm: add overlays as first class KMS objects

[Jerome Glisse]

On Wed, Apr 27, 2011 at 8:19 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> Hi Jesse,
>
> I like it. It's a bit of a chicken-egg api design problem, but if a
> proof-of-concept
> implementation exists for an embedded chip plus something to check whether
> it's good enough to implement Xv on ancient hw (intel overlay or for the qemu
> kms driver, if that could do this), that should be good enough.
>
> A few comments on the ioctl interface.
>
>> +/* Overlays blend with or override other bits on the CRTC */
>> +struct drm_mode_set_overlay {
>> +       __u32 overlay_id;
>> +       __u32 crtc_id;
>> +       __u32 fb_id; /* contains surface format type */
>> +
>> +       __u32 crtc_x, crtc_y;
>> +       __u32 x, y;
>> +
>> +       /* FIXME: color key/mask, scaling, z-order, other? */
>> +};
>
> I think scaling is required, you can't implement Xv without that. The
> problem is some arbitraray
> hw range restrictions, but returning -EINVAL if they're violated looks okay. So
>
> float scale_x, scale_y;

No float in kernel. Would have to use fixed point.

> should be good enough. For the remaining things (color conversion,
> blending, ...) I don't think
> there's any generic way to map hw capabilities (without going insane).
> I think a bunch of
> properties (with standard stuff for gamma, color key, ...) would be
> perfect for that. If we
> set the defaults such that it Just Works for Xv (after setting the
> color key, if present), that
> would be great!
>
>> +struct drm_mode_get_overlay {
>> +       __u64 format_type_ptr;
>> +       __u32 overlay_id;
>> +
>> +       __u32 crtc_id;
>> +       __u32 fb_id;
>> +
>> +       __u32 crtc_x, crtc_y;
>> +       __u32 x, y;
>> +
>> +       __u32 possible_crtcs;
>> +       __u32 gamma_size;
>> +
>> +       __u32 count_format_types;
>> +};
>
> Imo the real problem with formats is stride restrictions and other hw
> restrictions (tiling, ...).
> ARM/v4l people seem to want that to be in the kernel so that they can
> e.g. dma decoded
> video streams directly to the gpu (also for other stuff). Perhaps we
> want to extend the
> create_dumb_gem_object ioctl for that use case, but I'm not too
> convinced that this belongs
> into the kernel.
>
> Cheers, Daniel
> --
> Daniel Vetter
> daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

One thing i wonder is if there is hw that doesn't support non tiled
surface at all.

Cheers,
Jerome

Re: [RFC] drm: add overlays as first class KMS objects

[Daniel Vetter]

On Wed, Apr 27, 2011 at 3:32 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 8:19 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> float scale_x, scale_y;
>
> No float in kernel. Would have to use fixed point.

Bleh, of course ;-) 32bit with a 16 bit shift should be good enough
for all scaling needs
-Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

Re: [RFC] drm: add overlays as first class KMS objects

[Chris Wilson]

On Wed, 27 Apr 2011 16:27:55 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Apr 27, 2011 at 3:32 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
> > On Wed, Apr 27, 2011 at 8:19 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> float scale_x, scale_y;
> >
> > No float in kernel. Would have to use fixed point.
> 
> Bleh, of course ;-) 32bit with a 16 bit shift should be good enough
> for all scaling needs

Or just specify the source size. You already know the output size...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [RFC] drm: add overlays as first class KMS objects

[Daniel Vetter]

On Wed, Apr 27, 2011 at 4:34 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Or just specify the source size. You already know the output size...

Hm, I've thought that x, y are fb offsets, not width/height of the
target ... Maybe we need that,
too?
-Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

Re: [RFC] drm: add overlays as first class KMS objects

[Ville Syrjälä]

On Wed, Apr 27, 2011 at 03:34:42PM +0100, Chris Wilson wrote:
> On Wed, 27 Apr 2011 16:27:55 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Apr 27, 2011 at 3:32 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
> > > On Wed, Apr 27, 2011 at 8:19 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > >> float scale_x, scale_y;
> > >
> > > No float in kernel. Would have to use fixed point.
> > 
> > Bleh, of course ;-) 32bit with a 16 bit shift should be good enough
> > for all scaling needs
> 
> Or just specify the source size. You already know the output size...

The source size needs to be specified with sub-pixel precision.
Otherwise you can't do accurate clipping.

Another thing to consider is whether you expect user space to do the
clipping against the CRTC dimensions, or if the kernel will do it.

Personally I like the OpenWF Display approach with it's attributes,
and atomic commits. That kind of thing is a lot easier to extend than
a fixed structure which is supposed to have all the bells and whistles
from day 1.

Also some overlay related properties are in fact more CRTC properties,
eg. on OMAP3 colorkeying and alpha blending are configured for the whole
output path, not for individual overlays. Perhaps connector properties
can be abused for those, even though they have nothing to do with
connectors as such. But some way to atomically commit the whole state
should be available.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

Re: [RFC] drm: add overlays as first class KMS objects

[Jesse Barnes]

On Wed, 27 Apr 2011 14:19:05 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> Hi Jesse,
> 
> I like it. It's a bit of a chicken-egg api design problem, but if a
> proof-of-concept
> implementation exists for an embedded chip plus something to check whether
> it's good enough to implement Xv on ancient hw (intel overlay or for the qemu
> kms driver, if that could do this), that should be good enough.
> 
> A few comments on the ioctl interface.

Thanks for checking it out.

> > +/* Overlays blend with or override other bits on the CRTC */
> > +struct drm_mode_set_overlay {
> > +       __u32 overlay_id;
> > +       __u32 crtc_id;
> > +       __u32 fb_id; /* contains surface format type */
> > +
> > +       __u32 crtc_x, crtc_y;
> > +       __u32 x, y;
> > +
> > +       /* FIXME: color key/mask, scaling, z-order, other? */
> > +};
> 
> I think scaling is required, you can't implement Xv without that. The
> problem is some arbitraray
> hw range restrictions, but returning -EINVAL if they're violated looks okay. So
> 
> float scale_x, scale_y;

Ok, I'll collect more fields based on this thread and make this
structure bigger.  Still not sure how to handle arbitrary blend and
z-order restrictions without passing a tree around though...

> 
> should be good enough. For the remaining things (color conversion,
> blending, ...) I don't think
> there's any generic way to map hw capabilities (without going insane).
> I think a bunch of
> properties (with standard stuff for gamma, color key, ...) would be
> perfect for that. If we
> set the defaults such that it Just Works for Xv (after setting the
> color key, if present), that
> would be great!

Yeah, properties for those is probably the way to go.

> 
> > +struct drm_mode_get_overlay {
> > +       __u64 format_type_ptr;
> > +       __u32 overlay_id;
> > +
> > +       __u32 crtc_id;
> > +       __u32 fb_id;
> > +
> > +       __u32 crtc_x, crtc_y;
> > +       __u32 x, y;
> > +
> > +       __u32 possible_crtcs;
> > +       __u32 gamma_size;
> > +
> > +       __u32 count_format_types;
> > +};
> 
> Imo the real problem with formats is stride restrictions and other hw
> restrictions (tiling, ...).
> ARM/v4l people seem to want that to be in the kernel so that they can
> e.g. dma decoded
> video streams directly to the gpu (also for other stuff). Perhaps we
> want to extend the
> create_dumb_gem_object ioctl for that use case, but I'm not too
> convinced that this belongs
> into the kernel.

I think it's a bit like handling tiling formats; for the most part the
kernel doesn't care because it's not reading or writing the data, but
it does need to know the format when programming certain regs.  So I
don't think we can avoid having surface format information passed in
when creating an fb for an overlay.  And if we do that we may as well
enumerate the types we support when overlay info is fetched to make
portability for app code a little easier.

Jesse
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] drm: add overlays as first class KMS objects

[Daniel Vetter]

On Wed, Apr 27, 2011 at 11:12 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Wed, 27 Apr 2011 14:19:05 +0200 Daniel Vetter <daniel@ffwll.ch> wrote:
>> Imo the real problem with formats is stride restrictions and other hw
>> restrictions (tiling, ...).
>> ARM/v4l people seem to want that to be in the kernel so that they can
>> e.g. dma decoded
>> video streams directly to the gpu (also for other stuff). Perhaps we
>> want to extend the
>> create_dumb_gem_object ioctl for that use case, but I'm not too
>> convinced that this belongs
>> into the kernel.
>
> I think it's a bit like handling tiling formats; for the most part the
> kernel doesn't care because it's not reading or writing the data, but
> it does need to know the format when programming certain regs.  So I
> don't think we can avoid having surface format information passed in
> when creating an fb for an overlay.  And if we do that we may as well
> enumerate the types we support when overlay info is fetched to make
> portability for app code a little easier.

I agree with your design ;-) My above comment was just in the light of
the ARM unified memory management discussion where some people
seem to want to move a complete buffer format description beast into
the kernel (like v4l already has). I think for gpus that'd get out of hand
quickly and format/layout arbitrage code should life in userspace.
-Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

Re: [RFC] drm: add overlays as first class KMS objects

[Rob Clark]

2011/4/25 Stéphane Marchesin <stephane.marchesin@gmail.com>:
> On Mon, Apr 25, 2011 at 16:22, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> On Mon, 25 Apr 2011 16:16:18 -0700
>> Keith Packard <keithp@keithp.com> wrote:
>>
>>> On Mon, 25 Apr 2011 15:12:20 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>>>
>>> > Overlays are a bit like half-CRTCs.  They have a location and fb, but
>>> > don't drive outputs directly.  Add support for handling them to the core
>>> > KMS code.
>>>
>>> Are overlays/underlays not associated with a specific CRTC? To my mind,
>>> overlays are another scanout buffer associated with a specific CRTC, so
>>> you'd create a scanout buffer and attach that to a specific scanout slot
>>> in a crtc, with the 'default' slot being the usual graphics plane.
>>
>> Yes, that matches my understanding as well.  I've deliberately made the
>> implementation flexible there though, under the assumption that some
>> hardware allows a plane to be directed at more than one CRTC (though
>> probably not simultaneously).
>>
>> Arguably, this is something we should have done when the
>> connector/encoder split was done (making planes in general first class
>> objects).  But with today's code, treating a CRTC as a pixel pump and a
>> primary plane seems fine, with overlays tacked onto the side as
>> secondary pixel sources but tied to a specific CRTC.
>>
>
> What is the plan for supporting multiple formats? When I looked at
> this for nouveau it ended up growing out of control when adding
> support for all the YUV (planar, packed, 12 or 16 bpp formats) and RGB
> format combinations.

maybe a dumb idea, but since all the GEM buffer allocation is already
done thru driver specific ioctl, couldn't the color format (and the
one or more plane pointers) be something that the DRM overlay
infrastructure doesn't have to care about.  I mean, I guess it is
somehow analogous to various tiled formats that you might have.

If the layout of the bytes is a property of the actual buffer object,
then wouldn't it be ok for DRM overlay infrastructure to ignore it and
the individual driver implementations just do the right thing based on
some private driver properties of the bo?

Maybe I'm over-simplifying or overlooking something, though..

BR,
-R

> Stéphane
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

Re: [RFC] drm: add overlays as first class KMS objects

[Rob Clark]

On Tue, Apr 26, 2011 at 5:01 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> A lot of older hardware had one overlay that could be sourced to any
>> crtc, just not simultaneously.  The tricky part is the formats and
>> capabilities: alpha blending, color/chromakey, gamma correction, etc.
>> Even the current crtc gamma stuff is somewhat lacking in in terms of
>> what hardware is capable of (PWL vs. LUT, user defined conversion
>> matrices, gamut remapping, etc.).
>
> Rather than re-inventing enough wheels to run a large truck would it not
> make sense to make hardware sourced overlays Video4Linux objects in their
> entirity so you can just say "attach V4L object A as overlay B"


One thing I'd like to be able to do is use a CRTC as either an overlay
or an primary framebuffer..

For example on omap4, if we have one display plugged in, then we have
3 pipes that can be used for overlay.  If a second display is plugged
in, one of those overlay pipes gets used as the primary gfx layer on
the 2nd display.

The ideal situation would let us share buffers w/ x11 with dri2
(although we need more than just front/back buffer.. you might have >
16 buffers for video playback).  And then in the xorg driver decide
whether to use an overlay or GPU blit depending on what hw resources
are not already in use.

So I'm not completely sure about the v4l2/drm coexistance for
overlays.  (Although for capture devices, it makes 100% sense to have
a way of sharing GEM buffers w/ v4l2.)  On the other hand, flipping
video buffers is kind of (if you squint slightly) just like flipping
buffers in a 3d gl app.

BR,
-R

> That would provide format definitions, provide control interfaces for
> the surface (eg for overlays of cameras such as on some of the Intel
> embedded and non PC devices), give you an existing well understood API.
>
> For some hardware you are going to need this integration anyway so that
> you can do things like move a GEM object which is currently a DMA target
> of a capture device (as well as to fence it).
>
> For a software surface you could either expose it as a V4L object that
> is GEM or fb memory backed or at least use the same descriptions so that
> the kernel has a consistent set of descriptions for formats and we don't
> have user libraries doing adhoc format translation crap.
>
> A lot of capture hardware would map very nicely onto GEM objects I
> suspect and if you want to merge live video into Wayland it seems a
> logical path ?
>
> Alan
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

Re: [RFC] drm: add overlays as first class KMS objects

[Rob Clark]

On Mon, Apr 25, 2011 at 5:12 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Looking for comments on this.  Obviously if we're going to add a new type
> of KMS object, we'd better get the ioctl more or less right to begin with,
> which means having all the attributes we'd like to track, plus some
> padding, available from the outset.
>
> So I'd like comments on this; the whole approach may be broken for things
> like OMAP; if so I'd like to hear about it now.  Overall, overlays are
> treated a little like CRTCs, but without associated modes our encoder
> trees hanging off of them.  That is, they can be enabled with a specific
> fb attached at a specific location, but they don't have to worry about
> mode setting, per se (though they do need to have an associated CRTC to
> actually pump their pixels out, post-blend).
>
> Flipping could be done either with the existing ioctl by updating the fb
> base pointer, or through a new flip ioctl similar to what we have already,
> but taking an overlay object instead of a CRTC.

One thing I am wondering about is how to synchronize overlay position
w/ flipping in the primary gfx layer?  Assuming you actually are
flipping in primary layer you'd want a new set of overlay
position/size to take effect on the same vblank that the flip in the
gfx layer happens, because you are probably relying on some
transparent pixels (or colorkey, if anyone still uses that) to be
drawn in the UI layer.

BR,
-R


> Overlays are a bit like half-CRTCs.  They have a location and fb, but
> don't drive outputs directly.  Add support for handling them to the core
> KMS code.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/drm_crtc.c |  219 ++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm.h          |    3 +
>  include/drm/drm_crtc.h     |   61 ++++++++++++
>  include/drm/drm_mode.h     |   39 ++++++++
>  4 files changed, 322 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 799e149..77ff9e0 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -533,6 +533,34 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
>  }
>  EXPORT_SYMBOL(drm_encoder_cleanup);
>
> +void drm_overlay_init(struct drm_device *dev, struct drm_overlay *overlay,
> +                     const struct drm_overlay_funcs *funcs)
> +{
> +       mutex_lock(&dev->mode_config.mutex);
> +
> +       overlay->dev = dev;
> +       drm_mode_object_get(dev, &overlay->base, DRM_MODE_OBJECT_OVERLAY);
> +       overlay->funcs = funcs;
> +
> +       list_add_tail(&overlay->head, &dev->mode_config.overlay_list);
> +       dev->mode_config.num_overlay++;
> +
> +       mutex_unlock(&dev->mode_config.mutex);
> +}
> +EXPORT_SYMBOL(drm_overlay_init);
> +
> +void drm_overlay_cleanup(struct drm_overlay *overlay)
> +{
> +       struct drm_device *dev = overlay->dev;
> +
> +       mutex_lock(&dev->mode_config.mutex);
> +       drm_mode_object_put(dev, &overlay->base);
> +       list_del(&overlay->head);
> +       dev->mode_config.num_overlay--;
> +       mutex_unlock(&dev->mode_config.mutex);
> +}
> +EXPORT_SYMBOL(drm_overlay_cleanup);
> +
>  /**
>  * drm_mode_create - create a new display mode
>  * @dev: DRM device
> @@ -864,6 +892,7 @@ void drm_mode_config_init(struct drm_device *dev)
>        INIT_LIST_HEAD(&dev->mode_config.encoder_list);
>        INIT_LIST_HEAD(&dev->mode_config.property_list);
>        INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
> +       INIT_LIST_HEAD(&dev->mode_config.overlay_list);
>        idr_init(&dev->mode_config.crtc_idr);
>
>        mutex_lock(&dev->mode_config.mutex);
> @@ -1467,6 +1496,196 @@ out:
>  }
>
>  /**
> + * drm_mode_getoverlay_res - get overlay info
> + * @dev: DRM device
> + * @data: ioctl data
> + * @file_priv: DRM file info
> + *
> + * Return an overlay count and set of IDs.
> + */
> +int drm_mode_getoverlay_res(struct drm_device *dev, void *data,
> +                           struct drm_file *file_priv)
> +{
> +       struct drm_mode_get_overlay_res *overlay_resp = data;
> +       struct drm_mode_config *config;
> +       struct drm_overlay *overlay;
> +       uint32_t __user *overlay_ptr;
> +       int copied = 0, ret = 0;
> +
> +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +               return -EINVAL;
> +
> +       mutex_lock(&dev->mode_config.mutex);
> +       config = &dev->mode_config;
> +
> +       /*
> +        * This ioctl is called twice, once to determine how much space is
> +        * needed, and the 2nd time to fill it.
> +        */
> +       if (config->num_overlay &&
> +           (overlay_resp->count_overlays >= config->num_overlay)) {
> +               overlay_ptr = (uint32_t *)(unsigned long)overlay_resp->overlay_id_ptr;
> +
> +               list_for_each_entry(overlay, &config->overlay_list, head) {
> +                       if (put_user(overlay->base.id, overlay_ptr + copied)) {
> +                               ret = -EFAULT;
> +                               goto out;
> +                       }
> +                       copied++;
> +               }
> +       }
> +       overlay_resp->count_overlays = config->num_overlay;
> +
> +out:
> +       mutex_unlock(&dev->mode_config.mutex);
> +       return ret;
> +}
> +
> +/**
> + * drm_mode_getoverlay - get overlay info
> + * @dev: DRM device
> + * @data: ioctl data
> + * @file_priv: DRM file info
> + *
> + * Return overlay info, including formats supported, gamma size, any
> + * current fb, etc.
> + */
> +int drm_mode_getoverlay(struct drm_device *dev, void *data,
> +                       struct drm_file *file_priv)
> +{
> +       struct drm_mode_get_overlay *overlay_resp = data;
> +       struct drm_mode_object *obj;
> +       struct drm_overlay *overlay;
> +       uint32_t __user *format_ptr;
> +       int ret = 0;
> +
> +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +               return -EINVAL;
> +
> +       mutex_lock(&dev->mode_config.mutex);
> +       obj = drm_mode_object_find(dev, overlay_resp->overlay_id,
> +                                  DRM_MODE_OBJECT_OVERLAY);
> +       if (!obj) {
> +               ret = -EINVAL;
> +               goto out;
> +       }
> +       overlay = obj_to_overlay(obj);
> +
> +       if (overlay->crtc)
> +               overlay_resp->crtc_id = overlay->crtc->base.id;
> +       else
> +               overlay_resp->crtc_id = 0;
> +
> +       if (overlay->fb)
> +               overlay_resp->fb_id = overlay->fb->base.id;
> +       else
> +               overlay_resp->fb_id = 0;
> +
> +       overlay_resp->overlay_id = overlay->base.id;
> +       overlay_resp->possible_crtcs = overlay->possible_crtcs;
> +       overlay_resp->gamma_size = overlay->gamma_size;
> +       overlay_resp->crtc_x = overlay->crtc_x;
> +       overlay_resp->crtc_y = overlay->crtc_y;
> +       overlay_resp->x = overlay->x;
> +       overlay_resp->y = overlay->y;
> +
> +       /*
> +        * This ioctl is called twice, once to determine how much space is
> +        * needed, and the 2nd time to fill it.
> +        */
> +       if (overlay->format_count &&
> +           (overlay_resp->count_format_types >= overlay->format_count)) {
> +               format_ptr = (uint32_t *)(unsigned long)overlay_resp->format_type_ptr;
> +               if (copy_to_user(format_ptr,
> +                                overlay->format_types,
> +                                sizeof(uint32_t) * overlay->format_count)) {
> +                       ret = -EFAULT;
> +                       goto out;
> +               }
> +       }
> +       overlay_resp->count_format_types = overlay->format_count;
> +
> +out:
> +       mutex_unlock(&dev->mode_config.mutex);
> +       return ret;
> +}
> +
> +/**
> + * drm_mode_setoverlay - set up or tear down an overlay
> + * @dev: DRM device
> + * @data: ioctl data*
> + * @file_prive: DRM file info
> + *
> + * Set overlay info, including placement, fb, scaling, and other factors.
> + * Or pass a NULL fb to disable.
> + */
> +int drm_mode_setoverlay(struct drm_device *dev, void *data,
> +                       struct drm_file *file_priv)
> +{
> +       struct drm_mode_set_overlay *overlay_req = data;
> +       struct drm_mode_object *obj;
> +       struct drm_overlay *overlay;
> +       struct drm_crtc *crtc;
> +       struct drm_framebuffer *fb;
> +       int ret = 0;
> +
> +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +               return -EINVAL;
> +
> +       mutex_lock(&dev->mode_config.mutex);
> +
> +       /*
> +        * First, find the overlay, crtc, and fb objects.  If not available,
> +        * we don't bother to call the driver.
> +        */
> +       obj = drm_mode_object_find(dev, overlay_req->overlay_id,
> +                                  DRM_MODE_OBJECT_OVERLAY);
> +       if (!obj) {
> +               DRM_DEBUG_KMS("Unknown overlay ID %d\n",
> +                             overlay_req->overlay_id);
> +               ret = -EINVAL;
> +               goto out;
> +       }
> +       overlay = obj_to_overlay(obj);
> +
> +       /* No fb means shut it down */
> +       if (!overlay_req->fb_id) {
> +               overlay->funcs->disable_overlay(overlay);
> +               goto out;
> +       }
> +
> +       obj = drm_mode_object_find(dev, overlay_req->crtc_id,
> +                                  DRM_MODE_OBJECT_CRTC);
> +       if (!obj) {
> +               DRM_DEBUG_KMS("Unknown crtc ID %d\n",
> +                             overlay_req->crtc_id);
> +               ret = -EINVAL;
> +               goto out;
> +       }
> +       crtc = obj_to_crtc(obj);
> +
> +       obj = drm_mode_object_find(dev, overlay_req->fb_id,
> +                                  DRM_MODE_OBJECT_FB);
> +       if (!obj) {
> +               DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
> +                             overlay_req->fb_id);
> +               ret = -EINVAL;
> +               goto out;
> +       }
> +       fb = obj_to_fb(obj);
> +
> +       ret = overlay->funcs->update_overlay(overlay, crtc, fb,
> +                                            overlay_req->crtc_x,
> +                                            overlay_req->crtc_y,
> +                                            overlay_req->x, overlay_req->y);
> +
> +out:
> +       mutex_unlock(&dev->mode_config.mutex);
> +
> +       return ret;
> +}
> +
> +/**
>  * drm_mode_setcrtc - set CRTC configuration
>  * @inode: inode from the ioctl
>  * @filp: file * from the ioctl
> diff --git a/include/drm/drm.h b/include/drm/drm.h
> index 4be33b4..47909af 100644
> --- a/include/drm/drm.h
> +++ b/include/drm/drm.h
> @@ -714,6 +714,9 @@ struct drm_get_cap {
>  #define DRM_IOCTL_MODE_CREATE_DUMB DRM_IOWR(0xB2, struct drm_mode_create_dumb)
>  #define DRM_IOCTL_MODE_MAP_DUMB    DRM_IOWR(0xB3, struct drm_mode_map_dumb)
>  #define DRM_IOCTL_MODE_DESTROY_DUMB    DRM_IOWR(0xB4, struct drm_mode_destroy_dumb)
> +#define DRM_IOCTL_MODE_GETOVERLAYRESOURCES DRM_IOWR(0xB5, struct drm_mode_get_overlay_res)
> +#define DRM_IOCTL_MODE_GETOVERLAY      DRM_IOWR(0xB6, struct drm_mode_get_overlay)
> +#define DRM_IOCTL_MODE_SETOVERLAY      DRM_IOWR(0xB7, struct drm_mode_set_overlay)
>
>  /**
>  * Device specific ioctls should only be in their respective headers
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index b786a24..07a9025 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -44,6 +44,7 @@ struct drm_framebuffer;
>  #define DRM_MODE_OBJECT_PROPERTY 0xb0b0b0b0
>  #define DRM_MODE_OBJECT_FB 0xfbfbfbfb
>  #define DRM_MODE_OBJECT_BLOB 0xbbbbbbbb
> +#define DRM_MODE_OBJECT_OVERLAY 0xeeeeeeee
>
>  struct drm_mode_object {
>        uint32_t id;
> @@ -276,6 +277,7 @@ struct drm_crtc;
>  struct drm_connector;
>  struct drm_encoder;
>  struct drm_pending_vblank_event;
> +struct drm_overlay;
>
>  /**
>  * drm_crtc_funcs - control CRTCs for a given device
> @@ -523,6 +525,57 @@ struct drm_connector {
>  };
>
>  /**
> + * drm_overlay_funcs - driver overlay control functions
> + * @update_overlay: update the overlay configuration
> + */
> +struct drm_overlay_funcs {
> +       int (*update_overlay)(struct drm_overlay *overlay,
> +                             struct drm_crtc *crtc, struct drm_framebuffer *fb,
> +                             int crtc_x, int crtc_y, int x, int y);
> +       void (*disable_overlay)(struct drm_overlay *overlay);
> +};
> +
> +/**
> + * drm_overlay - central DRM overlay control structure
> + * @dev: DRM device this overlay belongs to
> + * @kdev: kernel device
> + * @attr: kdev attributes
> + * @head: for list management
> + * @base: base mode object
> + * @crtc_x: x position of overlay (relative to pipe base)
> + * @crtc_y: y position of overlay
> + * @x: x offset into fb
> + * @y: y offset into fb
> + * @crtc: CRTC this overlay is feeding
> + */
> +struct drm_overlay {
> +       struct drm_device *dev;
> +       struct device kdev;
> +       struct device_attribute *attr;
> +       struct list_head head;
> +
> +       struct drm_mode_object base;
> +
> +       int crtc_x, crtc_y;
> +       int x, y;
> +       uint32_t possible_crtcs;
> +       uint32_t *format_types;
> +       uint32_t format_count;
> +
> +       struct drm_crtc *crtc;
> +       struct drm_framebuffer *fb;
> +
> +       /* CRTC gamma size for reporting to userspace */
> +       uint32_t gamma_size;
> +       uint16_t *gamma_store;
> +
> +       bool enabled;
> +
> +       const struct drm_overlay_funcs *funcs;
> +       void *helper_private;
> +};
> +
> +/**
>  * struct drm_mode_set
>  *
>  * Represents a single crtc the connectors that it drives with what mode
> @@ -576,6 +629,8 @@ struct drm_mode_config {
>        struct list_head connector_list;
>        int num_encoder;
>        struct list_head encoder_list;
> +       int num_overlay;
> +       struct list_head overlay_list;
>
>        int num_crtc;
>        struct list_head crtc_list;
> @@ -628,6 +683,7 @@ struct drm_mode_config {
>  #define obj_to_fb(x) container_of(x, struct drm_framebuffer, base)
>  #define obj_to_property(x) container_of(x, struct drm_property, base)
>  #define obj_to_blob(x) container_of(x, struct drm_property_blob, base)
> +#define obj_to_overlay(x) container_of(x, struct drm_overlay, base)
>
>
>  extern void drm_crtc_init(struct drm_device *dev,
> @@ -647,6 +703,11 @@ extern void drm_encoder_init(struct drm_device *dev,
>                             const struct drm_encoder_funcs *funcs,
>                             int encoder_type);
>
> +extern void drm_overlay_init(struct drm_device *dev,
> +                            struct drm_overlay *overlay,
> +                            const struct drm_overlay_funcs *funcs);
> +extern void drm_overlay_cleanup(struct drm_overlay *overlay);
> +
>  extern void drm_encoder_cleanup(struct drm_encoder *encoder);
>
>  extern char *drm_get_connector_name(struct drm_connector *connector);
> diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
> index ae6b7a3..349052a 100644
> --- a/include/drm/drm_mode.h
> +++ b/include/drm/drm_mode.h
> @@ -120,6 +120,45 @@ struct drm_mode_crtc {
>        struct drm_mode_modeinfo mode;
>  };
>
> +#define DRM_MODE_OVERLAY_FORMAT_YUV422         1 /* YUV 4:2:2 packed */
> +#define DRM_MODE_OVERLAY_FORMAT_RGBX101010     2 /* RGB 10bpc, ign. alpha */
> +#define DRM_MODE_OVERLAY_FORMAT_RGBX888                3 /* Standard x:8:8:8 RGB */
> +#define DRM_MODE_OVERLAY_FORMAT_RGBX161616     4 /* x:16:16:16 float RGB */
> +
> +/* Overlays blend with or override other bits on the CRTC */
> +struct drm_mode_set_overlay {
> +       __u32 overlay_id;
> +       __u32 crtc_id;
> +       __u32 fb_id; /* contains surface format type */
> +
> +       __u32 crtc_x, crtc_y;
> +       __u32 x, y;
> +
> +       /* FIXME: color key/mask, scaling, z-order, other? */
> +};
> +
> +struct drm_mode_get_overlay {
> +       __u64 format_type_ptr;
> +       __u32 overlay_id;
> +
> +       __u32 crtc_id;
> +       __u32 fb_id;
> +
> +       __u32 crtc_x, crtc_y;
> +       __u32 x, y;
> +
> +       __u32 possible_crtcs;
> +       __u32 gamma_size;
> +
> +       __u32 count_format_types;
> +};
> +
> +
> +struct drm_mode_get_overlay_res {
> +       __u64 overlay_id_ptr;
> +       __u32 count_overlays;
> +};
> +
>  #define DRM_MODE_ENCODER_NONE  0
>  #define DRM_MODE_ENCODER_DAC   1
>  #define DRM_MODE_ENCODER_TMDS  2
> --
> 1.7.4.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

Re: [RFC] drm: add overlays as first class KMS objects

[Jakob Bornecrantz]

On Wed, Apr 27, 2011 at 11:12 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Wed, 27 Apr 2011 14:19:05 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
>> Hi Jesse,
>>
>> I like it. It's a bit of a chicken-egg api design problem, but if a
>> proof-of-concept
>> implementation exists for an embedded chip plus something to check whether
>> it's good enough to implement Xv on ancient hw (intel overlay or for the qemu
>> kms driver, if that could do this), that should be good enough.
>>
>> A few comments on the ioctl interface.
>
> Thanks for checking it out.
>
>> > +/* Overlays blend with or override other bits on the CRTC */
>> > +struct drm_mode_set_overlay {
>> > +       __u32 overlay_id;
>> > +       __u32 crtc_id;
>> > +       __u32 fb_id; /* contains surface format type */
>> > +
>> > +       __u32 crtc_x, crtc_y;
>> > +       __u32 x, y;
>> > +
>> > +       /* FIXME: color key/mask, scaling, z-order, other? */
>> > +};
>>
>> I think scaling is required, you can't implement Xv without that. The
>> problem is some arbitraray
>> hw range restrictions, but returning -EINVAL if they're violated looks okay. So
>>
>> float scale_x, scale_y;
>
> Ok, I'll collect more fields based on this thread and make this
> structure bigger.  Still not sure how to handle arbitrary blend and
> z-order restrictions without passing a tree around though...

What we send over the ioctl interface (in vmwgfx) to control the
overlays is this:

/**
 * struct drm_vmw_control_stream_arg
 *
 * @stream_id: Stearm to control
 * @enabled: If false all following arguments are ignored.
 * @handle: Handle to buffer for getting data from.
 * @format: Format of the overlay as understood by the host.
 * @width: Width of the overlay.
 * @height: Height of the overlay.
 * @size: Size of the overlay in bytes.
 * @pitch: Array of pitches, the two last are only used for YUV12 formats.
 * @offset: Offset from start of dma buffer to overlay.
 * @src: Source rect, must be within the defined area above.
 * @dst: Destination rect, x and y may be negative.
 *
 * Argument to the DRM_VMW_CONTROL_STREAM Ioctl.
 */

struct drm_vmw_control_stream_arg {
	uint32_t stream_id;
	uint32_t enabled;

	uint32_t flags;
	uint32_t color_key;

	uint32_t handle;
	uint32_t offset;
	int32_t format;
	uint32_t size;
	uint32_t width;
	uint32_t height;
	uint32_t pitch[3];

	uint32_t pad64;
	struct drm_vmw_rect src;
	struct drm_vmw_rect dst;
};

The command contains information describing the source "framebuffer"
as well as the data for controlling the overlay. The args are by a
amazing coincidence is almost 1:1 what we also send down to the host.
The biggest change here from what you proposed is that we send two
rects describing from where within the buffer we get the data and to
where it should go. My vote is to do that as well (makes my life
easier at least).

Tho I guess that color_key and flags could be made into a property.

Cheers Jakob.

>
>>
>> should be good enough. For the remaining things (color conversion,
>> blending, ...) I don't think
>> there's any generic way to map hw capabilities (without going insane).
>> I think a bunch of
>> properties (with standard stuff for gamma, color key, ...) would be
>> perfect for that. If we
>> set the defaults such that it Just Works for Xv (after setting the
>> color key, if present), that
>> would be great!
>
> Yeah, properties for those is probably the way to go.
>
>>
>> > +struct drm_mode_get_overlay {
>> > +       __u64 format_type_ptr;
>> > +       __u32 overlay_id;
>> > +
>> > +       __u32 crtc_id;
>> > +       __u32 fb_id;
>> > +
>> > +       __u32 crtc_x, crtc_y;
>> > +       __u32 x, y;
>> > +
>> > +       __u32 possible_crtcs;
>> > +       __u32 gamma_size;
>> > +
>> > +       __u32 count_format_types;
>> > +};
>>
>> Imo the real problem with formats is stride restrictions and other hw
>> restrictions (tiling, ...).
>> ARM/v4l people seem to want that to be in the kernel so that they can
>> e.g. dma decoded
>> video streams directly to the gpu (also for other stuff). Perhaps we
>> want to extend the
>> create_dumb_gem_object ioctl for that use case, but I'm not too
>> convinced that this belongs
>> into the kernel.
>
> I think it's a bit like handling tiling formats; for the most part the
> kernel doesn't care because it's not reading or writing the data, but
> it does need to know the format when programming certain regs.  So I
> don't think we can avoid having surface format information passed in
> when creating an fb for an overlay.  And if we do that we may as well
> enumerate the types we support when overlay info is fetched to make
> portability for app code a little easier.
>
> Jesse

Re: [RFC] drm: add overlays as first class KMS objects

[Ville Syrjälä]

On Thu, Apr 28, 2011 at 12:03:32PM -0500, Rob Clark wrote:
> On Mon, Apr 25, 2011 at 5:12 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > Looking for comments on this.  Obviously if we're going to add a new type
> > of KMS object, we'd better get the ioctl more or less right to begin with,
> > which means having all the attributes we'd like to track, plus some
> > padding, available from the outset.
> >
> > So I'd like comments on this; the whole approach may be broken for things
> > like OMAP; if so I'd like to hear about it now.  Overall, overlays are
> > treated a little like CRTCs, but without associated modes our encoder
> > trees hanging off of them.  That is, they can be enabled with a specific
> > fb attached at a specific location, but they don't have to worry about
> > mode setting, per se (though they do need to have an associated CRTC to
> > actually pump their pixels out, post-blend).
> >
> > Flipping could be done either with the existing ioctl by updating the fb
> > base pointer, or through a new flip ioctl similar to what we have already,
> > but taking an overlay object instead of a CRTC.
> 
> One thing I am wondering about is how to synchronize overlay position
> w/ flipping in the primary gfx layer?  Assuming you actually are
> flipping in primary layer you'd want a new set of overlay
> position/size to take effect on the same vblank that the flip in the
> gfx layer happens, because you are probably relying on some
> transparent pixels (or colorkey, if anyone still uses that) to be
> drawn in the UI layer.

That's a good reason to aim for an OpenWF Display type of API where you
can commit the whole device state atomically.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

Re: [RFC] drm: add overlays as first class KMS objects

[Daniel Vetter]

Hi Jesse,

Discussion here in Budapest with v4l and embedded graphics folks was
extremely fruitful. A few quick things to take away - I'll try to dig
through all
the stuff I've learned more in-depth later (probably in a blog post or two):

- embedded graphics is insane. The output routing/blending/whatever
  currently shipping hw can do is crazy and kms as-is is nowhere near up
  to snuff to support this. We've discussed omap4 and a ti chip targeted at
  video surveillance as use cases. I'll post block diagrams and explanations
  some when later.
- we should immediately stop to call anything an overlay. It's a confusing
  concept that has a different meaning in every subsystem and for every hw
  manufacturer. More sensible names are dma fifo engines for things that slurp
  in planes and make them available to the display subsystem. Blend engines
  for blocks that take multiple input pipes and overlay/underlay/blend them
  together. Display subsytem/controller for the aggregate thing including
  encoders/resizers/outputs and especially the crazy routing network that
  connects everything.

Most of the discussion centered around clearing up the confusion and
reaching a mutual understanding between desktop graphics, embedded
graphics and v4l people. Two rough ideas emerged though:

1) Splitting the crtc object into two objects: crtc with associated output mode
(pixel clock, encoders/connectors) and dma engines (possibly multiple) that
feed it. omap 4 has essentially just 4 dma engines that can be freely assigned
to the available outputs, so a distinction between normal crtcs and overlay
engines just does not make sense. There's the major open question of where
to put the various attributes to set up the output pipeline. Also some of these
attributes might need to be changed atomicly together with pageflips on
a bunch of dma engines all associated with the same crtc on the next vsync,
e.g. output position of an overlaid video buffer.

2) The above should be good enough to support halfway sane chips like
omap4. But hw with more insane routing capabilities that can also use v4l
devices as sources (even video input connectors) media controller might be
a good fit. Media controller is designed to expose multimedia pipe routing
across different subsystem. But the first version, still marked experimental,
only got merged in .39. We discussed a few ideas as how to splice
media controller into kms but nothing clear emerged. So a possible kms
integration with media
controller is rather far away.

Yours, Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

Re: [RFC] drm: add overlays as first class KMS objects

[Jesse Barnes]

On Fri, 13 May 2011 18:16:30 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Hi Jesse,
> 
> Discussion here in Budapest with v4l and embedded graphics folks was
> extremely fruitful. A few quick things to take away - I'll try to dig
> through all
> the stuff I've learned more in-depth later (probably in a blog post or two):
> 
> - embedded graphics is insane. The output routing/blending/whatever
>   currently shipping hw can do is crazy and kms as-is is nowhere near up
>   to snuff to support this. We've discussed omap4 and a ti chip targeted at
>   video surveillance as use cases. I'll post block diagrams and explanations
>   some when later.

Yeah I expected that; even just TVs can have really funky restrictions
about z order and blend capability.

> - we should immediately stop to call anything an overlay. It's a confusing
>   concept that has a different meaning in every subsystem and for every hw
>   manufacturer. More sensible names are dma fifo engines for things that slurp
>   in planes and make them available to the display subsystem. Blend engines
>   for blocks that take multiple input pipes and overlay/underlay/blend them
>   together. Display subsytem/controller for the aggregate thing including
>   encoders/resizers/outputs and especially the crazy routing network that
>   connects everything.

How about just "display plane" then?  Specifically in the context of
display output hardware...

> 1) Splitting the crtc object into two objects: crtc with associated output mode
> (pixel clock, encoders/connectors) and dma engines (possibly multiple) that
> feed it. omap 4 has essentially just 4 dma engines that can be freely assigned
> to the available outputs, so a distinction between normal crtcs and overlay
> engines just does not make sense. There's the major open question of where
> to put the various attributes to set up the output pipeline. Also some of these
> attributes might need to be changed atomicly together with pageflips on
> a bunch of dma engines all associated with the same crtc on the next vsync,
> e.g. output position of an overlaid video buffer.

Yeah, that's a good goal, and pretty much what I had in mind here.
However, breaking the existing interface is a non-starter, so either we
need a new CRTC object altogether, or we preserve the idea of a
"primary" plane (whatever that means for a given platform) that's tied
to each CRTC, which each additional plane described in a separate
structure.  Z order and blend restrictions will have to be communicated
separately I think...

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFC] drm: add overlays as first class KMS objects

[Clark, Rob]

On Fri, May 13, 2011 at 8:02 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Fri, 13 May 2011 18:16:30 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
>> Hi Jesse,
>>
>> Discussion here in Budapest with v4l and embedded graphics folks was
>> extremely fruitful. A few quick things to take away - I'll try to dig
>> through all
>> the stuff I've learned more in-depth later (probably in a blog post or two):

Hi Daniel, thanks for writing this up

>> - embedded graphics is insane. The output routing/blending/whatever
>>   currently shipping hw can do is crazy and kms as-is is nowhere near up
>>   to snuff to support this. We've discussed omap4 and a ti chip targeted at
>>   video surveillance as use cases. I'll post block diagrams and explanations
>>   some when later.
>
> Yeah I expected that; even just TVs can have really funky restrictions
> about z order and blend capability.
>
>> - we should immediately stop to call anything an overlay. It's a confusing
>>   concept that has a different meaning in every subsystem and for every hw
>>   manufacturer. More sensible names are dma fifo engines for things that slurp
>>   in planes and make them available to the display subsystem. Blend engines
>>   for blocks that take multiple input pipes and overlay/underlay/blend them
>>   together. Display subsytem/controller for the aggregate thing including
>>   encoders/resizers/outputs and especially the crazy routing network that
>>   connects everything.
>
> How about just "display plane" then?  Specifically in the context of
> display output hardware...

"display plane" could be a good name.. actually in omap4 case it is a
single dma engine that is multiplexing fetches for however many
attached video pipes.. that is perhaps an implementation detail, but
it makes "display plane" sound nicer as a name


>
>> 1) Splitting the crtc object into two objects: crtc with associated output mode
>> (pixel clock, encoders/connectors) and dma engines (possibly multiple) that
>> feed it. omap 4 has essentially just 4 dma engines that can be freely assigned
>> to the available outputs, so a distinction between normal crtcs and overlay
>> engines just does not make sense. There's the major open question of where
>> to put the various attributes to set up the output pipeline. Also some of these
>> attributes might need to be changed atomicly together with pageflips on
>> a bunch of dma engines all associated with the same crtc on the next vsync,
>> e.g. output position of an overlaid video buffer.
>
> Yeah, that's a good goal, and pretty much what I had in mind here.
> However, breaking the existing interface is a non-starter, so either we
> need a new CRTC object altogether, or we preserve the idea of a
> "primary" plane (whatever that means for a given platform) that's tied
> to each CRTC, which each additional plane described in a separate
> structure.  Z order and blend restrictions will have to be communicated
> separately I think...

In the cases I can think of, you'll always have a primary plane, so
userspace need not explicitly specify it.  But I think you want the
driver to pick which display plane to be automatically hooked between
the primary fb and crtc, or at least this should be the case if some
new bit is set in driver_features to indicate the driver supports
multiple display planes per crtc.

BR,
-R

> Thanks,
> --
> Jesse Barnes, Intel Open Source Technology Center
>

Re: [RFC] drm: add overlays as first class KMS objects

[Laurent Pinchart]

Hi Daniel,

On Friday 13 May 2011 18:16:30 Daniel Vetter wrote:
> Hi Jesse,
> 
> Discussion here in Budapest with v4l and embedded graphics folks was
> extremely fruitful. A few quick things to take away - I'll try to dig
> through all
> the stuff I've learned more in-depth later (probably in a blog post or
> two):
> 
> - embedded graphics is insane. The output routing/blending/whatever
>   currently shipping hw can do is crazy and kms as-is is nowhere near up
>   to snuff to support this. We've discussed omap4 and a ti chip targeted at
>   video surveillance as use cases. I'll post block diagrams and
> explanations some when later.
> - we should immediately stop to call anything an overlay. It's a confusing
>   concept that has a different meaning in every subsystem and for every hw
>   manufacturer. More sensible names are dma fifo engines for things that
> slurp in planes and make them available to the display subsystem. Blend
> engines for blocks that take multiple input pipes and overlay/underlay/blend
> them together. Display subsytem/controller for the aggregate thing including
> encoders/resizers/outputs and especially the crazy routing network that
> connects everything.
> 
> Most of the discussion centered around clearing up the confusion and
> reaching a mutual understanding between desktop graphics, embedded
> graphics and v4l people. Two rough ideas emerged though:
> 
> 1) Splitting the crtc object into two objects: crtc with associated output
> mode (pixel clock, encoders/connectors) and dma engines (possibly
> multiple) that feed it. omap 4 has essentially just 4 dma engines that can
> be freely assigned to the available outputs, so a distinction between
> normal crtcs and overlay engines just does not make sense. There's the
> major open question of where to put the various attributes to set up the
> output pipeline. Also some of these attributes might need to be changed
> atomicly together with pageflips on a bunch of dma engines all associated
> with the same crtc on the next vsync, e.g. output position of an overlaid
> video buffer.

I like that idea. Setting attributes atomically will likely be one of the 
biggest challenge. V4L2 shares the same need, but we haven't had time to 
address it yet.

> 2) The above should be good enough to support halfway sane chips like
> omap4. But hw with more insane routing capabilities that can also use v4l
> devices as sources (even video input connectors) media controller might be
> a good fit. Media controller is designed to expose multimedia pipe routing
> across different subsystem. But the first version, still marked
> experimental, only got merged in .39. We discussed a few ideas as how to
> splice media controller into kms but nothing clear emerged. So a possible
> kms integration with media controller is rather far away.

You're probably right, but far away doesn't mean never. Especially when one of 
the media controller developers is interested in the project and could spend 
time on it :-)

I've started working on a prototype implementation that would use the media 
controller API to report the CRTCs, encoders and connectors topology to 
userspace. The learning curve is pretty steep as I'm not familiar with the DRM 
and KMS code, but the code base turned out to be much easier to dive in than 
it seemed a couple of years ago.

-- 
Regards,

Laurent Pinchart