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
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.
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.
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
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. :)
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'.
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.
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
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...
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.
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
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.
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
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.
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
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
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
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.
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
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.
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.
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
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
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
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
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
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
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.
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
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
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
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
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.
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
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...
- 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,
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
- 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
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:
- 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.
- 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.
dri-devel@lists.freedesktop.org
-
Alan Cox -
Alex Deucher -
Chris Wilson -
Clark, Rob -
Daniel Vetter -
Daniel Vetter -
Jakob Bornecrantz -
Jerome Glisse -
Jesse Barnes -
Keith Packard -
Laurent Pinchart -
Rob Clark -
Stéphane Marchesin -
Ville Syrjälä