* [PATCH 1/5] drm: add plane support
2011-11-07 18:02 [PATCH] DRM planes Jesse Barnes
@ 2011-11-07 18:02 ` Jesse Barnes
2011-11-08 14:20 ` Daniel Vetter
2011-11-07 18:02 ` [PATCH 2/5] drm: add an fb creation ioctl that takes a pixel format Jesse Barnes
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Jesse Barnes @ 2011-11-07 18:02 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, rob.clark
Planes 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 | 251 +++++++++++++++++++++++++++++++++++++++++++-
drivers/gpu/drm/drm_drv.c | 3 +
include/drm/drm.h | 3 +
include/drm/drm_crtc.h | 79 ++++++++++++++-
include/drm/drm_mode.h | 33 ++++++
5 files changed, 366 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index fe738f0..fac8043 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -321,6 +321,7 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb)
{
struct drm_device *dev = fb->dev;
struct drm_crtc *crtc;
+ struct drm_plane *plane;
struct drm_mode_set set;
int ret;
@@ -337,6 +338,15 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb)
}
}
+ list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
+ if (plane->fb == fb) {
+ /* should turn off the crtc */
+ ret = plane->funcs->disable_plane(plane);
+ if (ret)
+ DRM_ERROR("failed to disable plane with busy fb\n");
+ }
+ }
+
drm_mode_object_put(dev, &fb->base);
list_del(&fb->head);
dev->mode_config.num_fb--;
@@ -535,6 +545,48 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
}
EXPORT_SYMBOL(drm_encoder_cleanup);
+void drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
+ unsigned long possible_crtcs,
+ const struct drm_plane_funcs *funcs,
+ uint32_t *formats, uint32_t format_count)
+{
+ mutex_lock(&dev->mode_config.mutex);
+
+ plane->dev = dev;
+ drm_mode_object_get(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
+ plane->funcs = funcs;
+ plane->format_types = kmalloc(sizeof(uint32_t) * format_count,
+ GFP_KERNEL);
+ if (!plane->format_types) {
+ DRM_DEBUG_KMS("out of memory when allocating plane\n");
+ drm_mode_object_put(dev, &plane->base);
+ return;
+ }
+
+ memcpy(plane->format_types, formats, format_count);
+ plane->format_count = format_count;
+ plane->possible_crtcs = possible_crtcs;
+
+ list_add_tail(&plane->head, &dev->mode_config.plane_list);
+ dev->mode_config.num_plane++;
+
+ mutex_unlock(&dev->mode_config.mutex);
+}
+EXPORT_SYMBOL(drm_plane_init);
+
+void drm_plane_cleanup(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+
+ mutex_lock(&dev->mode_config.mutex);
+ kfree(plane->format_types);
+ drm_mode_object_put(dev, &plane->base);
+ list_del(&plane->head);
+ dev->mode_config.num_plane--;
+ mutex_unlock(&dev->mode_config.mutex);
+}
+EXPORT_SYMBOL(drm_plane_cleanup);
+
/**
* drm_mode_create - create a new display mode
* @dev: DRM device
@@ -866,6 +918,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.plane_list);
idr_init(&dev->mode_config.crtc_idr);
mutex_lock(&dev->mode_config.mutex);
@@ -942,6 +995,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
struct drm_encoder *encoder, *enct;
struct drm_framebuffer *fb, *fbt;
struct drm_property *property, *pt;
+ struct drm_plane *plane, *plt;
list_for_each_entry_safe(encoder, enct, &dev->mode_config.encoder_list,
head) {
@@ -966,6 +1020,10 @@ void drm_mode_config_cleanup(struct drm_device *dev)
crtc->funcs->destroy(crtc);
}
+ list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
+ head) {
+ plane->funcs->destroy(plane);
+ }
}
EXPORT_SYMBOL(drm_mode_config_cleanup);
@@ -1466,6 +1524,193 @@ out:
}
/**
+ * drm_mode_getplane_res - get plane info
+ * @dev: DRM device
+ * @data: ioctl data
+ * @file_priv: DRM file info
+ *
+ * Return an plane count and set of IDs.
+ */
+int drm_mode_getplane_res(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_get_plane_res *plane_resp = data;
+ struct drm_mode_config *config;
+ struct drm_plane *plane;
+ uint32_t __user *plane_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_plane &&
+ (plane_resp->count_planes >= config->num_plane)) {
+ plane_ptr = (uint32_t *)(unsigned long)plane_resp->plane_id_ptr;
+
+ list_for_each_entry(plane, &config->plane_list, head) {
+ if (put_user(plane->base.id, plane_ptr + copied)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ copied++;
+ }
+ }
+ plane_resp->count_planes = config->num_plane;
+
+out:
+ mutex_unlock(&dev->mode_config.mutex);
+ return ret;
+}
+
+/**
+ * drm_mode_getplane - get plane info
+ * @dev: DRM device
+ * @data: ioctl data
+ * @file_priv: DRM file info
+ *
+ * Return plane info, including formats supported, gamma size, any
+ * current fb, etc.
+ */
+int drm_mode_getplane(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_get_plane *plane_resp = data;
+ struct drm_mode_object *obj;
+ struct drm_plane *plane;
+ 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, plane_resp->plane_id,
+ DRM_MODE_OBJECT_PLANE);
+ if (!obj) {
+ ret = -EINVAL;
+ goto out;
+ }
+ plane = obj_to_plane(obj);
+
+ if (plane->crtc)
+ plane_resp->crtc_id = plane->crtc->base.id;
+ else
+ plane_resp->crtc_id = 0;
+
+ if (plane->fb)
+ plane_resp->fb_id = plane->fb->base.id;
+ else
+ plane_resp->fb_id = 0;
+
+ plane_resp->plane_id = plane->base.id;
+ plane_resp->possible_crtcs = plane->possible_crtcs;
+ plane_resp->gamma_size = plane->gamma_size;
+
+ /*
+ * This ioctl is called twice, once to determine how much space is
+ * needed, and the 2nd time to fill it.
+ */
+ if (plane->format_count &&
+ (plane_resp->count_format_types >= plane->format_count)) {
+ format_ptr = (uint32_t *)(unsigned long)plane_resp->format_type_ptr;
+ if (copy_to_user(format_ptr,
+ plane->format_types,
+ sizeof(uint32_t) * plane->format_count)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ }
+ plane_resp->count_format_types = plane->format_count;
+
+out:
+ mutex_unlock(&dev->mode_config.mutex);
+ return ret;
+}
+
+/**
+ * drm_mode_setplane - set up or tear down an plane
+ * @dev: DRM device
+ * @data: ioctl data*
+ * @file_prive: DRM file info
+ *
+ * Set plane info, including placement, fb, scaling, and other factors.
+ * Or pass a NULL fb to disable.
+ */
+int drm_mode_setplane(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_set_plane *plane_req = data;
+ struct drm_mode_object *obj;
+ struct drm_plane *plane;
+ 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 plane, crtc, and fb objects. If not available,
+ * we don't bother to call the driver.
+ */
+ obj = drm_mode_object_find(dev, plane_req->plane_id,
+ DRM_MODE_OBJECT_PLANE);
+ if (!obj) {
+ DRM_DEBUG_KMS("Unknown plane ID %d\n",
+ plane_req->plane_id);
+ ret = -EINVAL;
+ goto out;
+ }
+ plane = obj_to_plane(obj);
+
+ /* No fb means shut it down */
+ if (!plane_req->fb_id) {
+ plane->funcs->disable_plane(plane);
+ goto out;
+ }
+
+ obj = drm_mode_object_find(dev, plane_req->crtc_id,
+ DRM_MODE_OBJECT_CRTC);
+ if (!obj) {
+ DRM_DEBUG_KMS("Unknown crtc ID %d\n",
+ plane_req->crtc_id);
+ ret = -EINVAL;
+ goto out;
+ }
+ crtc = obj_to_crtc(obj);
+
+ obj = drm_mode_object_find(dev, plane_req->fb_id,
+ DRM_MODE_OBJECT_FB);
+ if (!obj) {
+ DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
+ plane_req->fb_id);
+ ret = -EINVAL;
+ goto out;
+ }
+ fb = obj_to_fb(obj);
+
+ ret = plane->funcs->update_plane(plane, crtc, fb,
+ plane_req->crtc_x, plane_req->crtc_y,
+ plane_req->crtc_w, plane_req->crtc_h,
+ plane_req->src_x, plane_req->src_y,
+ plane_req->src_h, plane_req->src_w);
+
+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
@@ -1688,11 +1933,13 @@ int drm_mode_addfb(struct drm_device *dev,
return -EINVAL;
if ((config->min_width > r->width) || (r->width > config->max_width)) {
- DRM_ERROR("mode new framebuffer width not within limits\n");
+ DRM_ERROR("bad framebuffer width %d, should be >= %d && <= %d\n",
+ r->width, config->min_width, config->max_width);
return -EINVAL;
}
if ((config->min_height > r->height) || (r->height > config->max_height)) {
- DRM_ERROR("mode new framebuffer height not within limits\n");
+ DRM_ERROR("bad framebuffer height %d, should be >= %d && <= %d\n",
+ r->height, config->min_height, config->max_height);
return -EINVAL;
}
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7a87e08..d782bd1 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -135,8 +135,11 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPLANE, drm_mode_setplane, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR, drm_mode_cursor_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETGAMMA, drm_mode_gamma_get_ioctl, DRM_MASTER|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETGAMMA, drm_mode_gamma_set_ioctl, DRM_MASTER|DRM_UNLOCKED),
diff --git a/include/drm/drm.h b/include/drm/drm.h
index 4be33b4..2897967 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_GETPLANERESOURCES DRM_IOWR(0xB5, struct drm_mode_get_plane_res)
+#define DRM_IOCTL_MODE_GETPLANE DRM_IOWR(0xB6, struct drm_mode_get_plane)
+#define DRM_IOCTL_MODE_SETPLANE DRM_IOWR(0xB7, struct drm_mode_set_plane)
/**
* 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 8020798..8331dc6 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_PLANE 0xeeeeeeee
struct drm_mode_object {
uint32_t id;
@@ -278,6 +279,7 @@ struct drm_crtc;
struct drm_connector;
struct drm_encoder;
struct drm_pending_vblank_event;
+struct drm_plane;
/**
* drm_crtc_funcs - control CRTCs for a given device
@@ -536,6 +538,66 @@ struct drm_connector {
};
/**
+ * drm_plane_funcs - driver plane control functions
+ * @update_plane: update the plane configuration
+ * @disable_plane: shut down the plane
+ * @destroy: clean up plane resources
+ */
+struct drm_plane_funcs {
+ int (*update_plane)(struct drm_plane *plane,
+ struct drm_crtc *crtc, struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h);
+ int (*disable_plane)(struct drm_plane *plane);
+ void (*destroy)(struct drm_plane *plane);
+};
+
+/**
+ * drm_plane - central DRM plane control structure
+ * @dev: DRM device this plane belongs to
+ * @kdev: kernel device
+ * @attr: kdev attributes
+ * @head: for list management
+ * @base: base mode object
+ * @possible_crtcs: pipes this plane can be bound to
+ * @format_types: array of formats supported by this plane
+ * @format_count: number of formats supported
+ * @crtc: currently bound CRTC
+ * @fb: currently bound fb
+ * @gamma_size: size of gamma table
+ * @gamma_store: gamma correction table
+ * @enabled: enabled flag
+ * @funcs: helper functions
+ * @helper_private: storage for drver layer
+ */
+struct drm_plane {
+ struct drm_device *dev;
+ struct device kdev;
+ struct device_attribute *attr;
+ struct list_head head;
+
+ struct drm_mode_object base;
+
+ 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_plane_funcs *funcs;
+ void *helper_private;
+};
+
+/**
* struct drm_mode_set
*
* Represents a single crtc the connectors that it drives with what mode
@@ -589,6 +651,8 @@ struct drm_mode_config {
struct list_head connector_list;
int num_encoder;
struct list_head encoder_list;
+ int num_plane;
+ struct list_head plane_list;
int num_crtc;
struct list_head crtc_list;
@@ -641,6 +705,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_plane(x) container_of(x, struct drm_plane, base)
extern void drm_crtc_init(struct drm_device *dev,
@@ -660,6 +725,13 @@ extern void drm_encoder_init(struct drm_device *dev,
const struct drm_encoder_funcs *funcs,
int encoder_type);
+extern void drm_plane_init(struct drm_device *dev,
+ struct drm_plane *plane,
+ unsigned long possible_crtcs,
+ const struct drm_plane_funcs *funcs,
+ uint32_t *formats, uint32_t format_count);
+extern void drm_plane_cleanup(struct drm_plane *plane);
+
extern void drm_encoder_cleanup(struct drm_encoder *encoder);
extern char *drm_get_connector_name(struct drm_connector *connector);
@@ -753,13 +825,18 @@ extern struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
/* IOCTLs */
extern int drm_mode_getresources(struct drm_device *dev,
void *data, struct drm_file *file_priv);
-
+extern int drm_mode_getplane_res(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
extern int drm_mode_getcrtc(struct drm_device *dev,
void *data, struct drm_file *file_priv);
extern int drm_mode_getconnector(struct drm_device *dev,
void *data, struct drm_file *file_priv);
extern int drm_mode_setcrtc(struct drm_device *dev,
void *data, struct drm_file *file_priv);
+extern int drm_mode_getplane(struct drm_device *dev,
+ void *data, struct drm_file *file_priv);
+extern int drm_mode_setplane(struct drm_device *dev,
+ void *data, struct drm_file *file_priv);
extern int drm_mode_cursor_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv);
extern int drm_mode_addfb(struct drm_device *dev,
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index c4961ea..07711b0 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -120,6 +120,39 @@ struct drm_mode_crtc {
struct drm_mode_modeinfo mode;
};
+/* Planes blend with or override other bits on the CRTC */
+struct drm_mode_set_plane {
+ __u32 plane_id;
+ __u32 crtc_id;
+ __u32 fb_id; /* fb object contains surface format type */
+
+ /* Signed dest location allows it to be partially off screen */
+ __s32 crtc_x, crtc_y;
+ __u32 crtc_w, crtc_h;
+
+ /* Source values are 16.16 fixed point */
+ __u32 src_x, src_y;
+ __u32 src_h, src_w;
+};
+
+struct drm_mode_get_plane {
+ __u64 format_type_ptr;
+ __u32 plane_id;
+
+ __u32 crtc_id;
+ __u32 fb_id;
+
+ __u32 possible_crtcs;
+ __u32 gamma_size;
+
+ __u32 count_format_types;
+};
+
+struct drm_mode_get_plane_res {
+ __u64 plane_id_ptr;
+ __u32 count_planes;
+};
+
#define DRM_MODE_ENCODER_NONE 0
#define DRM_MODE_ENCODER_DAC 1
#define DRM_MODE_ENCODER_TMDS 2
--
1.7.4.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/5] drm: add plane support
2011-11-07 18:02 ` [PATCH 1/5] drm: add plane support Jesse Barnes
@ 2011-11-08 14:20 ` Daniel Vetter
2011-11-08 16:34 ` Jesse Barnes
0 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2011-11-08 14:20 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx, dri-devel, rob.clark
On Mon, Nov 07, 2011 at 10:02:52AM -0800, Jesse Barnes wrote:
> Planes 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>
One question belowk, but still:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> /**
> + * drm_plane_funcs - driver plane control functions
> + * @update_plane: update the plane configuration
> + * @disable_plane: shut down the plane
> + * @destroy: clean up plane resources
> + */
> +struct drm_plane_funcs {
> + int (*update_plane)(struct drm_plane *plane,
> + struct drm_crtc *crtc, struct drm_framebuffer *fb,
> + int crtc_x, int crtc_y,
> + unsigned int crtc_w, unsigned int crtc_h,
> + uint32_t src_x, uint32_t src_y,
> + uint32_t src_w, uint32_t src_h);
> + int (*disable_plane)(struct drm_plane *plane);
> + void (*destroy)(struct drm_plane *plane);
> +};
> +
> +/**
> + * drm_plane - central DRM plane control structure
> + * @dev: DRM device this plane belongs to
> + * @kdev: kernel device
> + * @attr: kdev attributes
> + * @head: for list management
> + * @base: base mode object
> + * @possible_crtcs: pipes this plane can be bound to
> + * @format_types: array of formats supported by this plane
> + * @format_count: number of formats supported
> + * @crtc: currently bound CRTC
> + * @fb: currently bound fb
> + * @gamma_size: size of gamma table
> + * @gamma_store: gamma correction table
> + * @enabled: enabled flag
> + * @funcs: helper functions
> + * @helper_private: storage for drver layer
> + */
> +struct drm_plane {
> + struct drm_device *dev;
> + struct device kdev;
> + struct device_attribute *attr;
What are these two for?
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/5] drm: add plane support
2011-11-08 14:20 ` Daniel Vetter
@ 2011-11-08 16:34 ` Jesse Barnes
0 siblings, 0 replies; 17+ messages in thread
From: Jesse Barnes @ 2011-11-08 16:34 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel, rob.clark
[-- Attachment #1.1: Type: text/plain, Size: 2165 bytes --]
On Tue, 8 Nov 2011 15:20:37 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Nov 07, 2011 at 10:02:52AM -0800, Jesse Barnes wrote:
> > Planes 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>
>
> One question belowk, but still:
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> > /**
> > + * drm_plane_funcs - driver plane control functions
> > + * @update_plane: update the plane configuration
> > + * @disable_plane: shut down the plane
> > + * @destroy: clean up plane resources
> > + */
> > +struct drm_plane_funcs {
> > + int (*update_plane)(struct drm_plane *plane,
> > + struct drm_crtc *crtc, struct drm_framebuffer *fb,
> > + int crtc_x, int crtc_y,
> > + unsigned int crtc_w, unsigned int crtc_h,
> > + uint32_t src_x, uint32_t src_y,
> > + uint32_t src_w, uint32_t src_h);
> > + int (*disable_plane)(struct drm_plane *plane);
> > + void (*destroy)(struct drm_plane *plane);
> > +};
> > +
> > +/**
> > + * drm_plane - central DRM plane control structure
> > + * @dev: DRM device this plane belongs to
> > + * @kdev: kernel device
> > + * @attr: kdev attributes
> > + * @head: for list management
> > + * @base: base mode object
> > + * @possible_crtcs: pipes this plane can be bound to
> > + * @format_types: array of formats supported by this plane
> > + * @format_count: number of formats supported
> > + * @crtc: currently bound CRTC
> > + * @fb: currently bound fb
> > + * @gamma_size: size of gamma table
> > + * @gamma_store: gamma correction table
> > + * @enabled: enabled flag
> > + * @funcs: helper functions
> > + * @helper_private: storage for drver layer
> > + */
> > +struct drm_plane {
> > + struct drm_device *dev;
> > + struct device kdev;
> > + struct device_attribute *attr;
>
> What are these two for?
Oh I can drop those. I had plans to expose some sysfs stuff, but I
don't think I'll bother. Thanks.
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/5] drm: add an fb creation ioctl that takes a pixel format
2011-11-07 18:02 [PATCH] DRM planes Jesse Barnes
2011-11-07 18:02 ` [PATCH 1/5] drm: add plane support Jesse Barnes
@ 2011-11-07 18:02 ` Jesse Barnes
2011-11-07 18:02 ` [PATCH 3/5] drm/i915: add SNB and IVB video sprite support Jesse Barnes
` (2 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Jesse Barnes @ 2011-11-07 18:02 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, rob.clark
To properly support the various plane formats supported by different
hardware, the kernel must know the pixel format of a framebuffer object.
So add a new ioctl taking a format argument corresponding to a fourcc
name from videodev2.h.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/drm_crtc.c | 109 ++++++++++++++++++++++++++++-
drivers/gpu/drm/drm_crtc_helper.c | 50 ++++++++++++-
drivers/gpu/drm/drm_drv.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 34 +++++----
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_fb.c | 11 ++--
drivers/gpu/drm/nouveau/nouveau_display.c | 4 +-
drivers/gpu/drm/radeon/radeon_display.c | 4 +-
drivers/gpu/drm/radeon/radeon_fb.c | 18 +++--
drivers/gpu/drm/radeon/radeon_mode.h | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
drivers/staging/gma500/framebuffer.c | 2 +-
include/drm/drm.h | 1 +
include/drm/drm_crtc.h | 7 ++-
include/drm/drm_crtc_helper.h | 4 +-
include/drm/drm_mode.h | 28 +++++++-
include/linux/videodev2.h | 1 +
17 files changed, 236 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index fac8043..1158fa6 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1703,6 +1703,10 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
plane_req->crtc_w, plane_req->crtc_h,
plane_req->src_x, plane_req->src_y,
plane_req->src_h, plane_req->src_w);
+ if (!ret) {
+ plane->crtc = crtc;
+ plane->fb = fb;
+ }
out:
mutex_unlock(&dev->mode_config.mutex);
@@ -1904,6 +1908,42 @@ out:
return ret;
}
+/* Original addfb only supported RGB formats, so figure out which one */
+uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
+{
+ uint32_t fmt;
+
+ switch (bpp) {
+ case 8:
+ fmt = V4L2_PIX_FMT_RGB332;
+ break;
+ case 16:
+ if (depth == 15)
+ fmt = V4L2_PIX_FMT_RGB555;
+ else
+ fmt = V4L2_PIX_FMT_RGB565;
+ break;
+ case 24:
+ fmt = V4L2_PIX_FMT_RGB24;
+ break;
+ case 32:
+ if (depth == 24)
+ fmt = V4L2_PIX_FMT_RGB24;
+ else if (depth == 30)
+ fmt = V4L2_PIX_FMT_INTC_RGB30;
+ else
+ fmt = V4L2_PIX_FMT_RGB32;
+ break;
+ default:
+ DRM_ERROR("bad bpp, assuming RGB24 pixel format\n");
+ fmt = V4L2_PIX_FMT_RGB24;
+ break;
+ }
+
+ return fmt;
+}
+EXPORT_SYMBOL(drm_mode_legacy_fb_format);
+
/**
* drm_mode_addfb - add an FB to the graphics configuration
* @inode: inode from the ioctl
@@ -1924,7 +1964,74 @@ out:
int drm_mode_addfb(struct drm_device *dev,
void *data, struct drm_file *file_priv)
{
- struct drm_mode_fb_cmd *r = data;
+ struct drm_mode_fb_cmd *or = data;
+ struct drm_mode_fb_cmd2 r;
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_framebuffer *fb;
+ int ret = 0;
+
+ /* Use new struct with format internally */
+ r.fb_id = or->fb_id;
+ r.width = or->width;
+ r.height = or->height;
+ r.pitches[0] = or->pitch;
+ r.pixel_format = drm_mode_legacy_fb_format(or->bpp, or->depth);
+ r.handle = or->handle;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return -EINVAL;
+
+ if ((config->min_width > r.width) || (r.width > config->max_width)) {
+ DRM_ERROR("mode new framebuffer width not within limits\n");
+ return -EINVAL;
+ }
+ if ((config->min_height > r.height) || (r.height > config->max_height)) {
+ DRM_ERROR("mode new framebuffer height not within limits\n");
+ return -EINVAL;
+ }
+
+ mutex_lock(&dev->mode_config.mutex);
+
+ /* TODO check buffer is sufficiently large */
+ /* TODO setup destructor callback */
+
+ fb = dev->mode_config.funcs->fb_create(dev, file_priv, &r);
+ if (IS_ERR(fb)) {
+ DRM_ERROR("could not create framebuffer\n");
+ ret = PTR_ERR(fb);
+ goto out;
+ }
+
+ or->fb_id = fb->base.id;
+ list_add(&fb->filp_head, &file_priv->fbs);
+ DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id);
+
+out:
+ mutex_unlock(&dev->mode_config.mutex);
+ return ret;
+}
+
+/**
+ * drm_mode_addfb2 - add an FB to the graphics configuration
+ * @inode: inode from the ioctl
+ * @filp: file * from the ioctl
+ * @cmd: cmd from ioctl
+ * @arg: arg from ioctl
+ *
+ * LOCKING:
+ * Takes mode config lock.
+ *
+ * Add a new FB to the specified CRTC, given a user request with format.
+ *
+ * Called by the user via ioctl.
+ *
+ * RETURNS:
+ * Zero on success, errno on failure.
+ */
+int drm_mode_addfb2(struct drm_device *dev,
+ void *data, struct drm_file *file_priv)
+{
+ struct drm_mode_fb_cmd2 *r = data;
struct drm_mode_config *config = &dev->mode_config;
struct drm_framebuffer *fb;
int ret = 0;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index f236644..68011bb 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -807,14 +807,56 @@ void drm_helper_connector_dpms(struct drm_connector *connector, int mode)
}
EXPORT_SYMBOL(drm_helper_connector_dpms);
+/*
+ * Just need to support RGB formats here for compat with code that doesn't
+ * use pixel formats directly yet.
+ */
+void drm_helper_get_fb_bpp_depth(uint32_t format, unsigned int *depth,
+ int *bpp)
+{
+ switch (format) {
+ case V4L2_PIX_FMT_RGB332:
+ *depth = 8;
+ *bpp = 8;
+ break;
+ case V4L2_PIX_FMT_RGB555:
+ *depth = 15;
+ *bpp = 16;
+ break;
+ case V4L2_PIX_FMT_RGB565:
+ *depth = 16;
+ *bpp = 16;
+ break;
+ case V4L2_PIX_FMT_RGB24:
+ *depth = 24;
+ *bpp = 32;
+ break;
+ case V4L2_PIX_FMT_INTC_RGB30:
+ *depth = 30;
+ *bpp = 32;
+ break;
+ case V4L2_PIX_FMT_RGB32:
+ *depth = 32;
+ *bpp = 32;
+ break;
+ default:
+ DRM_DEBUG_KMS("unsupported pixel format\n");
+ *depth = 0;
+ *bpp = 0;
+ break;
+ }
+}
+EXPORT_SYMBOL(drm_helper_get_fb_bpp_depth);
+
int drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
- struct drm_mode_fb_cmd *mode_cmd)
+ struct drm_mode_fb_cmd2 *mode_cmd)
{
fb->width = mode_cmd->width;
fb->height = mode_cmd->height;
- fb->pitch = mode_cmd->pitch;
- fb->bits_per_pixel = mode_cmd->bpp;
- fb->depth = mode_cmd->depth;
+ fb->pitch = mode_cmd->pitches[0];
+ drm_helper_get_fb_bpp_depth(mode_cmd->pixel_format, &fb->depth,
+ &fb->bits_per_pixel);
+ fb->pixel_format = mode_cmd->pixel_format;
return 0;
}
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index d782bd1..6d87a59 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -152,6 +152,7 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB, drm_mode_addfb, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB2, drm_mode_addfb2, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_PAGE_FLIP, drm_mode_page_flip_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_DIRTYFB, drm_mode_dirtyfb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9fa342e..b2061b0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6279,7 +6279,7 @@ static struct drm_display_mode load_detect_mode = {
static struct drm_framebuffer *
intel_framebuffer_create(struct drm_device *dev,
- struct drm_mode_fb_cmd *mode_cmd,
+ struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_i915_gem_object *obj)
{
struct intel_framebuffer *intel_fb;
@@ -6321,7 +6321,7 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
int depth, int bpp)
{
struct drm_i915_gem_object *obj;
- struct drm_mode_fb_cmd mode_cmd;
+ struct drm_mode_fb_cmd2 mode_cmd;
obj = i915_gem_alloc_object(dev,
intel_framebuffer_size_for_mode(mode, bpp));
@@ -6330,9 +6330,9 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
mode_cmd.width = mode->hdisplay;
mode_cmd.height = mode->vdisplay;
- mode_cmd.depth = depth;
- mode_cmd.bpp = bpp;
- mode_cmd.pitch = intel_framebuffer_pitch_for_width(mode_cmd.width, bpp);
+ mode_cmd.pitches[0] = intel_framebuffer_pitch_for_width(mode_cmd.width,
+ bpp);
+ mode_cmd.pixel_format = 0;
return intel_framebuffer_create(dev, &mode_cmd, obj);
}
@@ -7573,7 +7573,7 @@ static const struct drm_framebuffer_funcs intel_fb_funcs = {
int intel_framebuffer_init(struct drm_device *dev,
struct intel_framebuffer *intel_fb,
- struct drm_mode_fb_cmd *mode_cmd,
+ struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_i915_gem_object *obj)
{
int ret;
@@ -7584,18 +7584,20 @@ int intel_framebuffer_init(struct drm_device *dev,
if (mode_cmd->pitch & 63)
return -EINVAL;
- switch (mode_cmd->bpp) {
- case 8:
- case 16:
- /* Only pre-ILK can handle 5:5:5 */
- if (mode_cmd->depth == 15 && !HAS_PCH_SPLIT(dev))
- return -EINVAL;
+ switch (mode_cmd->pixel_format) {
+ case V4L2_PIX_FMT_RGB332:
+ case V4L2_PIX_FMT_RGB565:
+ case V4L2_PIX_FMT_RGB24:
+ case V4L2_PIX_FMT_INTC_RGB30:
+ /* RGB formats are common across chipsets */
break;
-
- case 24:
- case 32:
+ case V4L2_PIX_FMT_YUYV:
+ case V4L2_PIX_FMT_UYVY:
+ case V4L2_PIX_FMT_YVYU:
+ case V4L2_PIX_FMT_VYUY:
break;
default:
+ DRM_ERROR("unsupported pixel format\n");
return -EINVAL;
}
@@ -7613,7 +7615,7 @@ int intel_framebuffer_init(struct drm_device *dev,
static struct drm_framebuffer *
intel_user_framebuffer_create(struct drm_device *dev,
struct drm_file *filp,
- struct drm_mode_fb_cmd *mode_cmd)
+ struct drm_mode_fb_cmd2 *mode_cmd)
{
struct drm_i915_gem_object *obj;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bd9a604..23c5622 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -359,7 +359,7 @@ extern int intel_pin_and_fence_fb_obj(struct drm_device *dev,
extern int intel_framebuffer_init(struct drm_device *dev,
struct intel_framebuffer *ifb,
- struct drm_mode_fb_cmd *mode_cmd,
+ struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_i915_gem_object *obj);
extern int intel_fbdev_init(struct drm_device *dev);
extern void intel_fbdev_fini(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index ec49bae..dc1db4f 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -65,7 +65,7 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
struct drm_i915_private *dev_priv = dev->dev_private;
struct fb_info *info;
struct drm_framebuffer *fb;
- struct drm_mode_fb_cmd mode_cmd;
+ struct drm_mode_fb_cmd2 mode_cmd;
struct drm_i915_gem_object *obj;
struct device *device = &dev->pdev->dev;
int size, ret;
@@ -77,11 +77,12 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
mode_cmd.width = sizes->surface_width;
mode_cmd.height = sizes->surface_height;
- mode_cmd.bpp = sizes->surface_bpp;
- mode_cmd.pitch = ALIGN(mode_cmd.width * ((mode_cmd.bpp + 7) / 8), 64);
- mode_cmd.depth = sizes->surface_depth;
+ mode_cmd.pitches[0] = ALIGN(mode_cmd.width * ((sizes->surface_bpp + 7) /
+ 8), 64);
+ mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
+ sizes->surface_depth);
- size = mode_cmd.pitch * mode_cmd.height;
+ size = mode_cmd.pitches[0] * mode_cmd.height;
size = ALIGN(size, PAGE_SIZE);
obj = i915_gem_alloc_object(dev, size);
if (!obj) {
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index ddbabef..7a428a9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -64,7 +64,7 @@ static const struct drm_framebuffer_funcs nouveau_framebuffer_funcs = {
int
nouveau_framebuffer_init(struct drm_device *dev,
struct nouveau_framebuffer *nv_fb,
- struct drm_mode_fb_cmd *mode_cmd,
+ struct drm_mode_fb_cmd2 *mode_cmd,
struct nouveau_bo *nvbo)
{
struct drm_nouveau_private *dev_priv = dev->dev_private;
@@ -124,7 +124,7 @@ nouveau_framebuffer_init(struct drm_device *dev,
static struct drm_framebuffer *
nouveau_user_framebuffer_create(struct drm_device *dev,
struct drm_file *file_priv,
- struct drm_mode_fb_cmd *mode_cmd)
+ struct drm_mode_fb_cmd2 *mode_cmd)
{
struct nouveau_framebuffer *nouveau_fb;
struct drm_gem_object *gem;
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 6adb3e5..6fde9c3 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -1113,7 +1113,7 @@ static const struct drm_framebuffer_funcs radeon_fb_funcs = {
void
radeon_framebuffer_init(struct drm_device *dev,
struct radeon_framebuffer *rfb,
- struct drm_mode_fb_cmd *mode_cmd,
+ struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj)
{
rfb->obj = obj;
@@ -1124,7 +1124,7 @@ radeon_framebuffer_init(struct drm_device *dev,
static struct drm_framebuffer *
radeon_user_framebuffer_create(struct drm_device *dev,
struct drm_file *file_priv,
- struct drm_mode_fb_cmd *mode_cmd)
+ struct drm_mode_fb_cmd2 *mode_cmd)
{
struct drm_gem_object *obj;
struct radeon_framebuffer *radeon_fb;
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 0b7b486..ea110ad 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -103,7 +103,7 @@ static void radeonfb_destroy_pinned_object(struct drm_gem_object *gobj)
}
static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev,
- struct drm_mode_fb_cmd *mode_cmd,
+ struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object **gobj_p)
{
struct radeon_device *rdev = rfbdev->rdev;
@@ -114,13 +114,17 @@ static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev,
int ret;
int aligned_size, size;
int height = mode_cmd->height;
+ u32 bpp, depth;
+
+ drm_helper_get_fb_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
/* need to align pitch with crtc limits */
- mode_cmd->pitch = radeon_align_pitch(rdev, mode_cmd->width, mode_cmd->bpp, fb_tiled) * ((mode_cmd->bpp + 1) / 8);
+ mode_cmd->pitches[0] = radeon_align_pitch(rdev, mode_cmd->width, bpp,
+ fb_tiled) * ((bpp + 1) / 8);
if (rdev->family >= CHIP_R600)
height = ALIGN(mode_cmd->height, 8);
- size = mode_cmd->pitch * height;
+ size = mode_cmd->pitches[0] * height;
aligned_size = ALIGN(size, PAGE_SIZE);
ret = radeon_gem_object_create(rdev, aligned_size, 0,
RADEON_GEM_DOMAIN_VRAM,
@@ -151,7 +155,7 @@ static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev,
if (tiling_flags) {
ret = radeon_bo_set_tiling_flags(rbo,
tiling_flags | RADEON_TILING_SURFACE,
- mode_cmd->pitch);
+ mode_cmd->pitches[0]);
if (ret)
dev_err(rdev->dev, "FB failed to set tiling flags\n");
}
@@ -187,7 +191,7 @@ static int radeonfb_create(struct radeon_fbdev *rfbdev,
struct radeon_device *rdev = rfbdev->rdev;
struct fb_info *info;
struct drm_framebuffer *fb = NULL;
- struct drm_mode_fb_cmd mode_cmd;
+ struct drm_mode_fb_cmd2 mode_cmd;
struct drm_gem_object *gobj = NULL;
struct radeon_bo *rbo = NULL;
struct device *device = &rdev->pdev->dev;
@@ -201,8 +205,8 @@ static int radeonfb_create(struct radeon_fbdev *rfbdev,
if ((sizes->surface_bpp == 24) && ASIC_IS_AVIVO(rdev))
sizes->surface_bpp = 32;
- mode_cmd.bpp = sizes->surface_bpp;
- mode_cmd.depth = sizes->surface_depth;
+ mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
+ sizes->surface_depth);
ret = radeonfb_create_pinned_object(rfbdev, &mode_cmd, &gobj);
rbo = gem_to_radeon_bo(gobj);
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index ed0178f..dd429a1 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -645,7 +645,7 @@ extern void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green
u16 *blue, int regno);
void radeon_framebuffer_init(struct drm_device *dev,
struct radeon_framebuffer *rfb,
- struct drm_mode_fb_cmd *mode_cmd,
+ struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj);
int radeonfb_remove(struct drm_device *dev, struct drm_framebuffer *fb);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 8b14dfd..6719170 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -974,7 +974,7 @@ out_err1:
static struct drm_framebuffer *vmw_kms_fb_create(struct drm_device *dev,
struct drm_file *file_priv,
- struct drm_mode_fb_cmd *mode_cmd)
+ struct drm_mode_fb_cmd2 *mode_cmd)
{
struct vmw_private *dev_priv = vmw_priv(dev);
struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
diff --git a/drivers/staging/gma500/framebuffer.c b/drivers/staging/gma500/framebuffer.c
index 3f39a37..1e77229 100644
--- a/drivers/staging/gma500/framebuffer.c
+++ b/drivers/staging/gma500/framebuffer.c
@@ -546,7 +546,7 @@ out_err1:
*/
static struct drm_framebuffer *psb_user_framebuffer_create
(struct drm_device *dev, struct drm_file *filp,
- struct drm_mode_fb_cmd *cmd)
+ struct drm_mode_fb_cmd2 *cmd)
{
struct gtt_range *r;
struct drm_gem_object *obj;
diff --git a/include/drm/drm.h b/include/drm/drm.h
index 2897967..49d94ed 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -717,6 +717,7 @@ struct drm_get_cap {
#define DRM_IOCTL_MODE_GETPLANERESOURCES DRM_IOWR(0xB5, struct drm_mode_get_plane_res)
#define DRM_IOCTL_MODE_GETPLANE DRM_IOWR(0xB6, struct drm_mode_get_plane)
#define DRM_IOCTL_MODE_SETPLANE DRM_IOWR(0xB7, struct drm_mode_set_plane)
+#define DRM_IOCTL_MODE_ADDFB2 DRM_IOWR(0xB8, struct drm_mode_fb_cmd2)
/**
* 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 8331dc6..1d09af1 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -29,6 +29,7 @@
#include <linux/spinlock.h>
#include <linux/types.h>
#include <linux/idr.h>
+#include <linux/videodev2.h> /* for plane formats */
#include <linux/fb.h>
@@ -246,6 +247,7 @@ struct drm_framebuffer {
unsigned int depth;
int bits_per_pixel;
int flags;
+ uint32_t pixel_format; /* fourcc format */
struct list_head filp_head;
/* if you are using the helper */
void *helper_private;
@@ -623,7 +625,7 @@ struct drm_mode_set {
* struct drm_mode_config_funcs - configure CRTCs for a given screen layout
*/
struct drm_mode_config_funcs {
- struct drm_framebuffer *(*fb_create)(struct drm_device *dev, struct drm_file *file_priv, struct drm_mode_fb_cmd *mode_cmd);
+ struct drm_framebuffer *(*fb_create)(struct drm_device *dev, struct drm_file *file_priv, struct drm_mode_fb_cmd2 *mode_cmd);
void (*output_poll_changed)(struct drm_device *dev);
};
@@ -841,6 +843,9 @@ extern int drm_mode_cursor_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv);
extern int drm_mode_addfb(struct drm_device *dev,
void *data, struct drm_file *file_priv);
+extern int drm_mode_addfb2(struct drm_device *dev,
+ void *data, struct drm_file *file_priv);
+extern uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
extern int drm_mode_rmfb(struct drm_device *dev,
void *data, struct drm_file *file_priv);
extern int drm_mode_getfb(struct drm_device *dev,
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 73b0712..b4abb33 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -116,8 +116,10 @@ extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
extern void drm_helper_connector_dpms(struct drm_connector *connector, int mode);
+extern void drm_helper_get_fb_bpp_depth(uint32_t format, unsigned int *depth,
+ int *bpp);
extern int drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
- struct drm_mode_fb_cmd *mode_cmd);
+ struct drm_mode_fb_cmd2 *mode_cmd);
static inline void drm_crtc_helper_add(struct drm_crtc *crtc,
const struct drm_crtc_helper_funcs *funcs)
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 07711b0..3cfa160 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -27,6 +27,8 @@
#ifndef _DRM_MODE_H
#define _DRM_MODE_H
+#include <linux/videodev2.h>
+
#define DRM_DISPLAY_INFO_LEN 32
#define DRM_CONNECTOR_NAME_LEN 32
#define DRM_DISPLAY_MODE_LEN 32
@@ -136,7 +138,6 @@ struct drm_mode_set_plane {
};
struct drm_mode_get_plane {
- __u64 format_type_ptr;
__u32 plane_id;
__u32 crtc_id;
@@ -146,6 +147,7 @@ struct drm_mode_get_plane {
__u32 gamma_size;
__u32 count_format_types;
+ __u64 format_type_ptr;
};
struct drm_mode_get_plane_res {
@@ -262,6 +264,30 @@ struct drm_mode_fb_cmd {
__u32 handle;
};
+struct drm_mode_fb_cmd2 {
+ __u32 fb_id;
+ __u32 width, height;
+ __u32 pixel_format; /* fourcc code from videodev2.h */
+
+ /*
+ * In case of planar formats, this ioctl allows one
+ * buffer object with offets and pitches per plane.
+ * The pitch and offset order is dictated by the fourcc,
+ * e.g. NV12 (http://fourcc.org/yuv.php#NV12) is described as:
+ *
+ * YUV 4:2:0 image with a plane of 8 bit Y samples
+ * followed by an interleaved U/V plane containing
+ * 8 bit 2x2 subsampled colour difference samples.
+ *
+ * So it would consist of Y as offset[0] and UV as
+ * offeset[1]. Note that offset[0] will generally
+ * be 0.
+ */
+ __u32 handle;
+ __u32 pitches[4]; /* pitch for each plane */
+ __u32 offsets[4]; /* offset of each plane */
+};
+
#define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01
#define DRM_MODE_FB_DIRTY_ANNOTATE_FILL 0x02
#define DRM_MODE_FB_DIRTY_FLAGS 0x03
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index fca24cc..6da4ab4 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -412,6 +412,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_KONICA420 v4l2_fourcc('K', 'O', 'N', 'I') /* YUV420 planar in blocks of 256 pixels */
#define V4L2_PIX_FMT_JPGL v4l2_fourcc('J', 'P', 'G', 'L') /* JPEG-Lite */
#define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
+#define V4L2_PIX_FMT_INTC_RGB30 v4l2_fourcc('R', 'G', 'B', '0') /* RGB x:10:10:10 */
/*
* F O R M A T E N U M E R A T I O N
--
1.7.4.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 3/5] drm/i915: add SNB and IVB video sprite support
2011-11-07 18:02 [PATCH] DRM planes Jesse Barnes
2011-11-07 18:02 ` [PATCH 1/5] drm: add plane support Jesse Barnes
2011-11-07 18:02 ` [PATCH 2/5] drm: add an fb creation ioctl that takes a pixel format Jesse Barnes
@ 2011-11-07 18:02 ` Jesse Barnes
2011-11-08 21:57 ` Daniel Vetter
2011-11-07 18:02 ` [PATCH 4/5] drm/i915: add destination color key support Jesse Barnes
2011-11-07 18:02 ` [PATCH 5/5] drm/i915: track sprite coverage and disable primary plane if possible Jesse Barnes
4 siblings, 1 reply; 17+ messages in thread
From: Jesse Barnes @ 2011-11-07 18:02 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, rob.clark
The video sprites support various video surface formats natively and can
handle scaling as well. So add support for them using the new DRM core
sprite support functions.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_reg.h | 123 ++++++++++
drivers/gpu/drm/i915/intel_display.c | 31 ++-
drivers/gpu/drm/i915/intel_drv.h | 22 ++
drivers/gpu/drm/i915/intel_fb.c | 6 +
drivers/gpu/drm/i915/intel_sprite.c | 428 ++++++++++++++++++++++++++++++++++
6 files changed, 600 insertions(+), 11 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_sprite.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0ae6a7c..808b255 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -28,6 +28,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \
intel_dvo.o \
intel_ringbuffer.o \
intel_overlay.o \
+ intel_sprite.o \
intel_opregion.o \
dvo_ch7xxx.o \
dvo_ch7017.o \
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5a09416..b2270fa 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2450,6 +2450,8 @@
#define WM3_LP_ILK 0x45110
#define WM3_LP_EN (1<<31)
#define WM1S_LP_ILK 0x45120
+#define WM2S_LP_IVB 0x45124
+#define WM3S_LP_IVB 0x45128
#define WM1S_LP_EN (1<<31)
/* Memory latency timer register */
@@ -2666,6 +2668,127 @@
#define _DSPBSURF 0x7119C
#define _DSPBTILEOFF 0x711A4
+/* Sprite A control */
+#define _DVSACNTR 0x72180
+#define DVS_ENABLE (1<<31)
+#define DVS_GAMMA_ENABLE (1<<30)
+#define DVS_PIXFORMAT_MASK (3<<25)
+#define DVS_FORMAT_YUV422 (0<<25)
+#define DVS_FORMAT_RGBX101010 (1<<25)
+#define DVS_FORMAT_RGBX888 (2<<25)
+#define DVS_FORMAT_RGBX161616 (3<<25)
+#define DVS_SOURCE_KEY (1<<22)
+#define DVS_RGB_ORDER_RGBX (1<<20)
+#define DVS_YUV_BYTE_ORDER_MASK (3<<16)
+#define DVS_YUV_ORDER_YUYV (0<<16)
+#define DVS_YUV_ORDER_UYVY (1<<16)
+#define DVS_YUV_ORDER_YVYU (2<<16)
+#define DVS_YUV_ORDER_VYUY (3<<16)
+#define DVS_DEST_KEY (1<<2)
+#define DVS_TRICKLE_FEED_DISABLE (1<<14)
+#define DVS_TILED (1<<10)
+#define _DVSASTRIDE 0x72188
+#define _DVSAPOS 0x7218c
+#define _DVSASIZE 0x72190
+#define _DVSAKEYVAL 0x72194
+#define _DVSAKEYMSK 0x72198
+#define _DVSASURF 0x7219c
+#define _DVSAKEYMAXVAL 0x721a0
+#define _DVSATILEOFF 0x721a4
+#define _DVSASURFLIVE 0x721ac
+#define _DVSASCALE 0x72204
+#define DVS_SCALE_ENABLE (1<<31)
+#define DVS_FILTER_MASK (3<<29)
+#define DVS_FILTER_MEDIUM (0<<29)
+#define DVS_FILTER_ENHANCING (1<<29)
+#define DVS_FILTER_SOFTENING (2<<29)
+#define _DVSAGAMC 0x72300
+
+#define _DVSBCNTR 0x73180
+#define _DVSBSTRIDE 0x73188
+#define _DVSBPOS 0x7318c
+#define _DVSBSIZE 0x73190
+#define _DVSBKEYVAL 0x73194
+#define _DVSBKEYMSK 0x73198
+#define _DVSBSURF 0x7319c
+#define _DVSBKEYMAXVAL 0x731a0
+#define _DVSBTILEOFF 0x731a4
+#define _DVSBSURFLIVE 0x731ac
+#define _DVSBSCALE 0x73204
+#define _DVSBGAMC 0x73300
+
+#define DVSCNTR(pipe) _PIPE(pipe, _DVSACNTR, _DVSBCNTR)
+#define DVSSTRIDE(pipe) _PIPE(pipe, _DVSASTRIDE, _DVSBSTRIDE)
+#define DVSPOS(pipe) _PIPE(pipe, _DVSAPOS, _DVSBPOS)
+#define DVSSURF(pipe) _PIPE(pipe, _DVSASURF, _DVSBSURF)
+#define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE)
+#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE)
+#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
+
+#define _SPRA_CTL 0x70280
+#define SPRITE_ENABLE (1<<31)
+#define SPRITE_GAMMA_ENABLE (1<<30)
+#define SPRITE_PIXFORMAT_MASK (7<<25)
+#define SPRITE_FORMAT_YUV422 (0<<25)
+#define SPRITE_FORMAT_RGBX101010 (1<<25)
+#define SPRITE_FORMAT_RGBX888 (2<<25)
+#define SPRITE_FORMAT_RGBX161616 (3<<25)
+#define SPRITE_FORMAT_YUV444 (4<<25)
+#define SPRITE_FORMAT_XBGR101010 (5<<25)
+#define SPRITE_CSC_ENABLE (1<<24)
+#define SPRITE_SOURCE_KEY (1<<22)
+#define SPRITE_RGB_ORDER_RGBX (1<<20) /* only for 888 and 161616 */
+#define SPRITE_YUV_TO_RGB_CSC_DISABLE (1<<19)
+#define SPRITE_YUV_CSC_FORMAT_BT709 (1<<18) /* 0 is BT601 */
+#define SPRITE_YUV_BYTE_ORDER_MASK (3<<16)
+#define SPRITE_YUV_ORDER_YUYV (0<<16)
+#define SPRITE_YUV_ORDER_UYVY (1<<16)
+#define SPRITE_YUV_ORDER_YVYU (2<<16)
+#define SPRITE_YUV_ORDER_VYUY (3<<16)
+#define SPRITE_TRICKLE_FEED_DISABLE (1<<14)
+#define SPRITE_INT_GAMMA_ENABLE (1<<13)
+#define SPRITE_TILED (1<<10)
+#define SPRITE_DEST_KEY (1<<2)
+#define _SPRA_STRIDE 0x70288
+#define _SPRA_POS 0x7028c
+#define _SPRA_SIZE 0x70290
+#define _SPRA_KEYVAL 0x70294
+#define _SPRA_KEYMSK 0x70298
+#define _SPRA_SURF 0x7029c
+#define _SPRA_KEYMAX 0x702a0
+#define _SPRA_TILEOFF 0x702a4
+#define _SPRA_SCALE 0x70304
+#define SPRITE_SCALE_ENABLE (1<<31)
+#define SPRITE_FILTER_MASK (3<<29)
+#define SPRITE_FILTER_MEDIUM (0<<29)
+#define SPRITE_FILTER_ENHANCING (1<<29)
+#define SPRITE_FILTER_SOFTENING (2<<29)
+#define _SPRA_GAMC 0x70400
+
+#define _SPRB_CTL 0x71280
+#define _SPRB_STRIDE 0x71288
+#define _SPRB_POS 0x7128c
+#define _SPRB_SIZE 0x71290
+#define _SPRB_KEYVAL 0x71294
+#define _SPRB_KEYMSK 0x71298
+#define _SPRB_SURF 0x7129c
+#define _SPRB_KEYMAX 0x712a0
+#define _SPRB_TILEOFF 0x712a4
+#define _SPRB_SCALE 0x71304
+#define _SPRB_GAMC 0x71400
+
+#define SPRCTL(pipe) _PIPE(pipe, _SPRA_CTL, _SPRB_CTL)
+#define SPRSTRIDE(pipe) _PIPE(pipe, _SPRA_STRIDE, _SPRB_STRIDE)
+#define SPRPOS(pipe) _PIPE(pipe, _SPRA_POS, _SPRB_POS)
+#define SPRSIZE(pipe) _PIPE(pipe, _SPRA_SIZE, _SPRB_SIZE)
+#define SPRKEYVAL(pipe) _PIPE(pipe, _SPRA_KEYVAL, _SPRB_KEYVAL)
+#define SPRKEYMSK(pipe) _PIPE(pipe, _SPRA_KEYMSK, _SPRB_KEYMSK)
+#define SPRSURF(pipe) _PIPE(pipe, _SPRA_SURF, _SPRB_SURF)
+#define SPRKEYMAX(pipe) _PIPE(pipe, _SPRA_KEYMAX, _SPRB_KEYMAX)
+#define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
+#define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
+#define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC)
+
/* VBIOS regs */
#define VGACNTRL 0x71400
# define VGA_DISP_DISABLE (1 << 31)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b2061b0..09228dd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -915,8 +915,8 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
pipe_name(pipe));
}
-static void assert_pipe(struct drm_i915_private *dev_priv,
- enum pipe pipe, bool state)
+void assert_pipe(struct drm_i915_private *dev_priv,
+ enum pipe pipe, bool state)
{
int reg;
u32 val;
@@ -929,8 +929,6 @@ static void assert_pipe(struct drm_i915_private *dev_priv,
"pipe %c assertion failure (expected %s, current %s)\n",
pipe_name(pipe), state_string(state), state_string(cur_state));
}
-#define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
-#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
static void assert_plane_enabled(struct drm_i915_private *dev_priv,
enum plane plane)
@@ -4439,7 +4437,8 @@ static void ironlake_update_wm(struct drm_device *dev)
ILK_LP0_CURSOR_LATENCY,
&plane_wm, &cursor_wm)) {
I915_WRITE(WM0_PIPEA_ILK,
- (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
+ (plane_wm << WM0_PIPE_PLANE_SHIFT) |
+ (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
" plane %d, " "cursor: %d\n",
plane_wm, cursor_wm);
@@ -4453,7 +4452,8 @@ static void ironlake_update_wm(struct drm_device *dev)
ILK_LP0_CURSOR_LATENCY,
&plane_wm, &cursor_wm)) {
I915_WRITE(WM0_PIPEB_ILK,
- (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
+ (plane_wm << WM0_PIPE_PLANE_SHIFT) |
+ (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
" plane %d, cursor: %d\n",
plane_wm, cursor_wm);
@@ -4521,7 +4521,8 @@ static void sandybridge_update_wm(struct drm_device *dev)
&sandybridge_cursor_wm_info, latency,
&plane_wm, &cursor_wm)) {
I915_WRITE(WM0_PIPEA_ILK,
- (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
+ (plane_wm << WM0_PIPE_PLANE_SHIFT) |
+ (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
" plane %d, " "cursor: %d\n",
plane_wm, cursor_wm);
@@ -4533,7 +4534,8 @@ static void sandybridge_update_wm(struct drm_device *dev)
&sandybridge_cursor_wm_info, latency,
&plane_wm, &cursor_wm)) {
I915_WRITE(WM0_PIPEB_ILK,
- (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
+ (plane_wm << WM0_PIPE_PLANE_SHIFT) |
+ (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
" plane %d, cursor: %d\n",
plane_wm, cursor_wm);
@@ -4547,7 +4549,8 @@ static void sandybridge_update_wm(struct drm_device *dev)
&sandybridge_cursor_wm_info, latency,
&plane_wm, &cursor_wm)) {
I915_WRITE(WM0_PIPEC_IVB,
- (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
+ (plane_wm << WM0_PIPE_PLANE_SHIFT) |
+ (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
DRM_DEBUG_KMS("FIFO watermarks For pipe C -"
" plane %d, cursor: %d\n",
plane_wm, cursor_wm);
@@ -7581,7 +7584,7 @@ int intel_framebuffer_init(struct drm_device *dev,
if (obj->tiling_mode == I915_TILING_Y)
return -EINVAL;
- if (mode_cmd->pitch & 63)
+ if (mode_cmd->pitches[0] & 63)
return -EINVAL;
switch (mode_cmd->pixel_format) {
@@ -8693,7 +8696,7 @@ static void i915_disable_vga(struct drm_device *dev)
void intel_modeset_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- int i;
+ int i, ret;
drm_mode_config_init(dev);
@@ -8723,6 +8726,12 @@ void intel_modeset_init(struct drm_device *dev)
for (i = 0; i < dev_priv->num_pipe; i++) {
intel_crtc_init(dev, i);
+ if (HAS_PCH_SPLIT(dev)) {
+ ret = intel_plane_init(dev, i);
+ if (ret)
+ DRM_ERROR("plane %d init failed: %d\n",
+ i, ret);
+ }
}
/* Just disable it once at startup */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 23c5622..6284562 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -176,10 +176,26 @@ struct intel_crtc {
bool use_pll_a;
};
+struct intel_plane {
+ struct drm_plane base;
+ enum pipe pipe;
+ struct drm_i915_gem_object *obj;
+ int max_downscale;
+ u32 lut_r[1024], lut_g[1024], lut_b[1024];
+ void (*update_plane)(struct drm_plane *plane,
+ struct drm_framebuffer *fb, unsigned long start,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t x, uint32_t y,
+ uint32_t src_w, uint32_t src_h);
+ void (*disable_plane)(struct drm_plane *plane);
+};
+
#define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
#define to_intel_connector(x) container_of(x, struct intel_connector, base)
#define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
#define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
+#define to_intel_plane(x) container_of(x, struct intel_plane, base)
#define DIP_HEADER_SIZE 5
@@ -289,6 +305,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
extern bool intel_dpd_is_edp(struct drm_device *dev);
extern void intel_edp_link_config(struct intel_encoder *, int *, int *);
extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
+extern int intel_plane_init(struct drm_device *dev, enum pipe pipe);
/* intel_panel.c */
extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
@@ -379,6 +396,11 @@ extern int intel_overlay_attrs(struct drm_device *dev, void *data,
extern void intel_fb_output_poll_changed(struct drm_device *dev);
extern void intel_fb_restore_mode(struct drm_device *dev);
+extern void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
+ bool state);
+#define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
+#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
+
extern void intel_init_clock_gating(struct drm_device *dev);
extern void intel_write_eld(struct drm_encoder *encoder,
struct drm_display_mode *mode);
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index dc1db4f..068b086 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -270,8 +270,14 @@ void intel_fb_restore_mode(struct drm_device *dev)
{
int ret;
drm_i915_private_t *dev_priv = dev->dev_private;
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_plane *plane;
ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
if (ret)
DRM_DEBUG("failed to restore crtc mode\n");
+
+ /* Be sure to shut off any planes that may be active */
+ list_for_each_entry(plane, &config->plane_list, head)
+ plane->funcs->disable_plane(plane);
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
new file mode 100644
index 0000000..f458921
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -0,0 +1,428 @@
+/*
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ * Jesse Barnes <jbarnes@virtuousgeek.org>
+ *
+ * New plane/sprite handling.
+ *
+ * The older chips had a separate interface for programming plane related
+ * registers; newer ones are much simpler and we can use the new DRM plane
+ * support.
+ */
+#include "drmP.h"
+#include "drm_crtc.h"
+#include "intel_drv.h"
+#include "i915_drm.h"
+#include "i915_drv.h"
+
+/*
+ * Note on refcounting:
+ * When the user creates an fb for the GEM object to be used for the plane,
+ * a ref is taken on the object. However, if the application exits before
+ * disabling the plane, the DRM close handling will free all the fbs and
+ * unless we take a ref on the object, it will be destroyed before the
+ * plane disable hook is called, causing obvious trouble with our efforts
+ * to look up and unpin the object. So we take a ref after we move the
+ * object to the display plane so it won't be destroyed until our disable
+ * hook is called and we drop our private reference.
+ */
+
+static void
+ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
+ unsigned long start, int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t x, uint32_t y,
+ uint32_t src_w, uint32_t src_h)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ int pipe = intel_plane->pipe;
+ u32 sprctl, sprscale = 0;
+ u32 reg = SPRCTL(pipe);
+
+ sprctl = I915_READ(reg);
+
+ /* Mask out pixel format bits in case we change it */
+ sprctl &= ~(SPRITE_DEST_KEY | SPRITE_SOURCE_KEY);
+ sprctl &= ~SPRITE_PIXFORMAT_MASK;
+ sprctl &= ~SPRITE_RGB_ORDER_RGBX;
+ sprctl &= ~SPRITE_YUV_BYTE_ORDER_MASK;
+
+ switch (fb->pixel_format) {
+ case V4L2_PIX_FMT_BGR32:
+ sprctl |= SPRITE_FORMAT_RGBX888;
+ break;
+ case V4L2_PIX_FMT_RGB32:
+ sprctl |= SPRITE_FORMAT_RGBX888 | SPRITE_RGB_ORDER_RGBX;
+ break;
+ case V4L2_PIX_FMT_YUYV:
+ sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YUYV;
+ break;
+ case V4L2_PIX_FMT_YVYU:
+ sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YVYU;
+ break;
+ case V4L2_PIX_FMT_UYVY:
+ sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_UYVY;
+ break;
+ case V4L2_PIX_FMT_VYUY:
+ sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_VYUY;
+ break;
+ default:
+ DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
+ sprctl |= DVS_FORMAT_RGBX888;
+ break;
+ }
+
+ sprctl |= SPRITE_TILED;
+
+ /* must disable */
+ sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
+ sprctl |= SPRITE_ENABLE;
+
+ /* Sizes are 0 based */
+ src_w--;
+ src_h--;
+ crtc_w--;
+ crtc_h--;
+
+ /* Adjust watermarks as needed */
+ I915_WRITE(WM1S_LP_ILK, 0x100);
+ I915_WRITE(WM2S_LP_IVB, 0x100);
+ I915_WRITE(WM3S_LP_IVB, 0x100);
+
+ if (crtc_w != src_w || crtc_h != src_h)
+ sprscale = SPRITE_SCALE_ENABLE | (src_h << 16) | src_w;
+
+ I915_WRITE(SPRSTRIDE(pipe), fb->pitch);
+ I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
+ I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
+ I915_WRITE(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
+ I915_WRITE(SPRSCALE(pipe), sprscale);
+ I915_WRITE(reg, sprctl);
+ I915_WRITE(SPRSURF(pipe), start);
+ POSTING_READ(SPRSURF(pipe));
+}
+
+static void
+ivb_disable_plane(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ int pipe = intel_plane->pipe;
+
+ I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);
+ I915_WRITE(SPRSCALE(pipe), 0);
+ I915_WRITE(SPRSURF(pipe), 0);
+ POSTING_READ(SPRSURF(pipe));
+ intel_wait_for_vblank(dev, pipe);
+}
+
+static void
+snb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
+ unsigned long start, int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t x, uint32_t y,
+ uint32_t src_w, uint32_t src_h)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ int pipe = intel_plane->pipe;
+ u32 dvscntr, dvsscale = 0;
+ u32 reg = DVSCNTR(pipe);
+
+ dvscntr = I915_READ(reg);
+
+ /* Mask out pixel format bits in case we change it */
+ dvscntr &= ~(DVS_DEST_KEY | DVS_SOURCE_KEY);
+ dvscntr &= ~DVS_PIXFORMAT_MASK;
+ dvscntr &= ~DVS_RGB_ORDER_RGBX;
+ dvscntr &= ~DVS_YUV_BYTE_ORDER_MASK;
+
+ switch (fb->pixel_format) {
+ case V4L2_PIX_FMT_BGR32:
+ dvscntr |= DVS_FORMAT_RGBX888;
+ break;
+ case V4L2_PIX_FMT_RGB32:
+ dvscntr |= DVS_FORMAT_RGBX888 | DVS_RGB_ORDER_RGBX;
+ break;
+ case V4L2_PIX_FMT_YUYV:
+ dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YUYV;
+ break;
+ case V4L2_PIX_FMT_YVYU:
+ dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YVYU;
+ break;
+ case V4L2_PIX_FMT_UYVY:
+ dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_UYVY;
+ break;
+ case V4L2_PIX_FMT_VYUY:
+ dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_VYUY;
+ break;
+ default:
+ DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
+ dvscntr |= DVS_FORMAT_RGBX888;
+ break;
+ }
+
+ dvscntr |= DVS_TILED;
+
+ /* must disable */
+ dvscntr |= DVS_TRICKLE_FEED_DISABLE;
+ dvscntr |= DVS_ENABLE;
+
+ /* Sizes are 0 based */
+ src_w--;
+ src_h--;
+ crtc_w--;
+ crtc_h--;
+
+ if (crtc_w != src_w || crtc_h != src_h)
+ dvsscale = DVS_SCALE_ENABLE | (src_h << 16) | src_w;
+
+ I915_WRITE(DVSSTRIDE(pipe), fb->pitch);
+ I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
+ I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x);
+ I915_WRITE(DVSSIZE(pipe), (crtc_h << 16) | crtc_w);
+ I915_WRITE(DVSSCALE(pipe), dvsscale);
+ I915_WRITE(reg, dvscntr);
+ I915_WRITE(DVSSURF(pipe), start);
+ POSTING_READ(DVSSURF(pipe));
+}
+
+static void
+snb_disable_plane(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ int pipe = intel_plane->pipe;
+
+ I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE);
+ I915_WRITE(DVSSCALE(pipe), 0);
+ I915_WRITE(DVSSURF(pipe), 0);
+ POSTING_READ(DVSSURF(pipe));
+ intel_wait_for_vblank(dev, pipe);
+}
+
+static int
+intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+ struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct intel_framebuffer *intel_fb;
+ struct drm_i915_gem_object *obj, *old_obj;
+ int pipe = intel_plane->pipe;
+ unsigned long start;
+ int ret = 0;
+ int x = src_x >> 16, y = src_y >> 16;
+ int primary_w = crtc->mode.hdisplay, primary_h = crtc->mode.vdisplay;
+ bool disable_primary = false;
+
+ intel_fb = to_intel_framebuffer(fb);
+ obj = intel_fb->obj;
+
+ old_obj = intel_plane->obj;
+
+ /* Pipe must be running... */
+ if (!(I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE))
+ return -EINVAL;
+
+ if (crtc_x >= primary_w || crtc_y >= primary_h)
+ return -EINVAL;
+
+ /* Don't modify another pipe's plane */
+ if (intel_plane->pipe != intel_crtc->pipe)
+ return -EINVAL;
+
+ /*
+ * Clamp the width & height into the visible area. Note we don't
+ * try to scale the source if part of the visible region is offscreen.
+ * The caller must handle that by adjusting source offset and size.
+ */
+ if (crtc_x < 0) {
+ crtc_w += crtc_x;
+ crtc_x = 0;
+ }
+ if (crtc_x + crtc_w > primary_w)
+ crtc_w = primary_w - crtc_x;
+
+ if (crtc_y < 0) {
+ crtc_h += crtc_y;
+ crtc_y = 0;
+ }
+ if (crtc_y + crtc_h > primary_h)
+ crtc_h = primary_h - crtc_y;
+
+ /*
+ * We can take a larger source and scale it down, but
+ * only so much... 16x is the max on SNB.
+ */
+ if (((src_w * src_h) / (crtc_w * crtc_h)) > intel_plane->max_downscale)
+ return -EINVAL;
+
+ /*
+ * If the sprite is completely covering the primary plane,
+ * we can disable the primary and save power.
+ */
+ if ((crtc_x == 0) && (crtc_y == 0) &&
+ (crtc_w == primary_w) && (crtc_h == primary_h))
+ disable_primary = true;
+
+ mutex_lock(&dev->struct_mutex);
+
+ if (obj->tiling_mode != I915_TILING_X) {
+ DRM_ERROR("plane surfaces must be X tiled\n");
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
+ if (ret) {
+ DRM_ERROR("failed to pin object\n");
+ goto out_unlock;
+ }
+
+ drm_gem_object_reference(&obj->base);
+
+ intel_plane->obj = obj;
+
+ start = obj->gtt_offset;
+
+ intel_plane->update_plane(plane, fb, start, crtc_x, crtc_y,
+ crtc_w, crtc_h, x, y, src_w, src_h);
+
+ /* Unpin old obj after new one is active to avoid ugliness */
+ if (old_obj) {
+ /*
+ * It's fairly common to simply update the position of
+ * an existing object. In that case, we don't need to
+ * wait for vblank to avoid ugliness, we only need to
+ * do the pin & ref bookkeeping.
+ */
+ if (old_obj != obj)
+ intel_wait_for_vblank(dev, to_intel_crtc(crtc)->pipe);
+ i915_gem_object_unpin(old_obj);
+ drm_gem_object_unreference(&old_obj->base);
+ }
+
+out_unlock:
+ mutex_unlock(&dev->struct_mutex);
+
+ return ret;
+}
+
+static int
+intel_disable_plane(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ int ret = 0;
+
+ mutex_lock(&dev->struct_mutex);
+
+ intel_plane->disable_plane(plane);
+
+ if (!intel_plane->obj)
+ goto out_unlock;
+
+ ret = i915_gem_object_finish_gpu(intel_plane->obj);
+ if (ret)
+ goto out_unlock;
+
+ i915_gem_object_unpin(intel_plane->obj);
+
+ drm_gem_object_unreference(&intel_plane->obj->base);
+
+out_unlock:
+ intel_plane->obj = NULL;
+
+ mutex_unlock(&dev->struct_mutex);
+
+ return ret;
+}
+
+static void intel_destroy_plane(struct drm_plane *plane)
+{
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ intel_disable_plane(plane);
+ drm_plane_cleanup(plane);
+ kfree(intel_plane);
+}
+
+static const struct drm_plane_funcs intel_plane_funcs = {
+ .update_plane = intel_update_plane,
+ .disable_plane = intel_disable_plane,
+ .destroy = intel_destroy_plane,
+};
+
+static uint32_t snb_plane_formats[] = {
+ V4L2_PIX_FMT_BGR32,
+ V4L2_PIX_FMT_RGB32,
+ V4L2_PIX_FMT_YUYV,
+ V4L2_PIX_FMT_YVYU,
+ V4L2_PIX_FMT_UYVY,
+ V4L2_PIX_FMT_VYUY,
+};
+
+int
+intel_plane_init(struct drm_device *dev, enum pipe pipe)
+{
+ struct intel_plane *intel_plane;
+ unsigned long possible_crtcs;
+
+ if (!(IS_GEN6(dev) || IS_GEN7(dev))) {
+ DRM_ERROR("new plane code only for SNB+\n");
+ return -ENODEV;
+ }
+
+ intel_plane = kzalloc(sizeof(struct intel_plane), GFP_KERNEL);
+ if (!intel_plane)
+ return -ENOMEM;
+
+ if (IS_GEN6(dev)) {
+ intel_plane->max_downscale = 16;
+ intel_plane->update_plane = snb_update_plane;
+ intel_plane->disable_plane = snb_disable_plane;
+ } else if (IS_GEN7(dev)) {
+ intel_plane->max_downscale = 2;
+ intel_plane->update_plane = ivb_update_plane;
+ intel_plane->disable_plane = ivb_disable_plane;
+ }
+
+ intel_plane->pipe = pipe;
+ possible_crtcs = (1 << pipe);
+ drm_plane_init(dev, &intel_plane->base, possible_crtcs,
+ &intel_plane_funcs, snb_plane_formats,
+ ARRAY_SIZE(snb_plane_formats));
+
+ return 0;
+}
+
--
1.7.4.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/5] drm/i915: add SNB and IVB video sprite support
2011-11-07 18:02 ` [PATCH 3/5] drm/i915: add SNB and IVB video sprite support Jesse Barnes
@ 2011-11-08 21:57 ` Daniel Vetter
2011-11-08 22:16 ` [Intel-gfx] " Daniel Vetter
2011-11-08 22:31 ` Jesse Barnes
0 siblings, 2 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-08 21:57 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx, dri-devel, rob.clark
On Mon, Nov 07, 2011 at 10:02:54AM -0800, Jesse Barnes wrote:
> The video sprites support various video surface formats natively and can
> handle scaling as well. So add support for them using the new DRM core
> sprite support functions.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
I've not checked the watermarks, that kind of magic eludes me ;-) A few
other comments below.
Cheers, Daniel
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_reg.h | 123 ++++++++++
> drivers/gpu/drm/i915/intel_display.c | 31 ++-
> drivers/gpu/drm/i915/intel_drv.h | 22 ++
> drivers/gpu/drm/i915/intel_fb.c | 6 +
> drivers/gpu/drm/i915/intel_sprite.c | 428 ++++++++++++++++++++++++++++++++++
> 6 files changed, 600 insertions(+), 11 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_sprite.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 0ae6a7c..808b255 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -28,6 +28,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \
> intel_dvo.o \
> intel_ringbuffer.o \
> intel_overlay.o \
> + intel_sprite.o \
> intel_opregion.o \
> dvo_ch7xxx.o \
> dvo_ch7017.o \
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 5a09416..b2270fa 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2450,6 +2450,8 @@
> #define WM3_LP_ILK 0x45110
> #define WM3_LP_EN (1<<31)
> #define WM1S_LP_ILK 0x45120
> +#define WM2S_LP_IVB 0x45124
> +#define WM3S_LP_IVB 0x45128
> #define WM1S_LP_EN (1<<31)
>
> /* Memory latency timer register */
> @@ -2666,6 +2668,127 @@
> #define _DSPBSURF 0x7119C
> #define _DSPBTILEOFF 0x711A4
>
> +/* Sprite A control */
> +#define _DVSACNTR 0x72180
> +#define DVS_ENABLE (1<<31)
> +#define DVS_GAMMA_ENABLE (1<<30)
> +#define DVS_PIXFORMAT_MASK (3<<25)
> +#define DVS_FORMAT_YUV422 (0<<25)
> +#define DVS_FORMAT_RGBX101010 (1<<25)
> +#define DVS_FORMAT_RGBX888 (2<<25)
> +#define DVS_FORMAT_RGBX161616 (3<<25)
> +#define DVS_SOURCE_KEY (1<<22)
> +#define DVS_RGB_ORDER_RGBX (1<<20)
> +#define DVS_YUV_BYTE_ORDER_MASK (3<<16)
> +#define DVS_YUV_ORDER_YUYV (0<<16)
> +#define DVS_YUV_ORDER_UYVY (1<<16)
> +#define DVS_YUV_ORDER_YVYU (2<<16)
> +#define DVS_YUV_ORDER_VYUY (3<<16)
> +#define DVS_DEST_KEY (1<<2)
> +#define DVS_TRICKLE_FEED_DISABLE (1<<14)
> +#define DVS_TILED (1<<10)
> +#define _DVSASTRIDE 0x72188
> +#define _DVSAPOS 0x7218c
> +#define _DVSASIZE 0x72190
> +#define _DVSAKEYVAL 0x72194
> +#define _DVSAKEYMSK 0x72198
> +#define _DVSASURF 0x7219c
> +#define _DVSAKEYMAXVAL 0x721a0
> +#define _DVSATILEOFF 0x721a4
> +#define _DVSASURFLIVE 0x721ac
> +#define _DVSASCALE 0x72204
> +#define DVS_SCALE_ENABLE (1<<31)
> +#define DVS_FILTER_MASK (3<<29)
> +#define DVS_FILTER_MEDIUM (0<<29)
> +#define DVS_FILTER_ENHANCING (1<<29)
> +#define DVS_FILTER_SOFTENING (2<<29)
BSpec has some bits for deinterlace support here, maybe add them just for
documentation.
> +#define _DVSAGAMC 0x72300
> +
> +#define _DVSBCNTR 0x73180
> +#define _DVSBSTRIDE 0x73188
> +#define _DVSBPOS 0x7318c
> +#define _DVSBSIZE 0x73190
> +#define _DVSBKEYVAL 0x73194
> +#define _DVSBKEYMSK 0x73198
> +#define _DVSBSURF 0x7319c
> +#define _DVSBKEYMAXVAL 0x731a0
> +#define _DVSBTILEOFF 0x731a4
> +#define _DVSBSURFLIVE 0x731ac
> +#define _DVSBSCALE 0x73204
> +#define _DVSBGAMC 0x73300
> +
> +#define DVSCNTR(pipe) _PIPE(pipe, _DVSACNTR, _DVSBCNTR)
> +#define DVSSTRIDE(pipe) _PIPE(pipe, _DVSASTRIDE, _DVSBSTRIDE)
> +#define DVSPOS(pipe) _PIPE(pipe, _DVSAPOS, _DVSBPOS)
> +#define DVSSURF(pipe) _PIPE(pipe, _DVSASURF, _DVSBSURF)
> +#define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE)
> +#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE)
> +#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
SNB register block and it's usage looks good.
> +
> +#define _SPRA_CTL 0x70280
> +#define SPRITE_ENABLE (1<<31)
> +#define SPRITE_GAMMA_ENABLE (1<<30)
> +#define SPRITE_PIXFORMAT_MASK (7<<25)
> +#define SPRITE_FORMAT_YUV422 (0<<25)
> +#define SPRITE_FORMAT_RGBX101010 (1<<25)
> +#define SPRITE_FORMAT_RGBX888 (2<<25)
> +#define SPRITE_FORMAT_RGBX161616 (3<<25)
> +#define SPRITE_FORMAT_YUV444 (4<<25)
> +#define SPRITE_FORMAT_XBGR101010 (5<<25)
That XBRG is a bit misleading, because the X here means extended range as
opposed to the usual gl meaning of unused. It sounds like a format with
shared exponent, gl seems to use E for that, i.e E2BGR101010.
> +#define SPRITE_CSC_ENABLE (1<<24)
> +#define SPRITE_SOURCE_KEY (1<<22)
> +#define SPRITE_RGB_ORDER_RGBX (1<<20) /* only for 888 and 161616 */
> +#define SPRITE_YUV_TO_RGB_CSC_DISABLE (1<<19)
> +#define SPRITE_YUV_CSC_FORMAT_BT709 (1<<18) /* 0 is BT601 */
> +#define SPRITE_YUV_BYTE_ORDER_MASK (3<<16)
> +#define SPRITE_YUV_ORDER_YUYV (0<<16)
> +#define SPRITE_YUV_ORDER_UYVY (1<<16)
> +#define SPRITE_YUV_ORDER_YVYU (2<<16)
> +#define SPRITE_YUV_ORDER_VYUY (3<<16)
> +#define SPRITE_TRICKLE_FEED_DISABLE (1<<14)
> +#define SPRITE_INT_GAMMA_ENABLE (1<<13)
> +#define SPRITE_TILED (1<<10)
> +#define SPRITE_DEST_KEY (1<<2)
> +#define _SPRA_STRIDE 0x70288
> +#define _SPRA_POS 0x7028c
> +#define _SPRA_SIZE 0x70290
> +#define _SPRA_KEYVAL 0x70294
> +#define _SPRA_KEYMSK 0x70298
> +#define _SPRA_SURF 0x7029c
> +#define _SPRA_KEYMAX 0x702a0
> +#define _SPRA_TILEOFF 0x702a4
> +#define _SPRA_SCALE 0x70304
> +#define SPRITE_SCALE_ENABLE (1<<31)
> +#define SPRITE_FILTER_MASK (3<<29)
> +#define SPRITE_FILTER_MEDIUM (0<<29)
> +#define SPRITE_FILTER_ENHANCING (1<<29)
> +#define SPRITE_FILTER_SOFTENING (2<<29)
Again I'd find the interlace bit definitions useful.
> +#define _SPRA_GAMC 0x70400
Gamma regs are a bit funky, especially on SNB. I assume you'll add all the
necessary defines when set_gamma support shows up.
> +#define _SPRB_CTL 0x71280
> +#define _SPRB_STRIDE 0x71288
> +#define _SPRB_POS 0x7128c
> +#define _SPRB_SIZE 0x71290
> +#define _SPRB_KEYVAL 0x71294
> +#define _SPRB_KEYMSK 0x71298
> +#define _SPRB_SURF 0x7129c
> +#define _SPRB_KEYMAX 0x712a0
> +#define _SPRB_TILEOFF 0x712a4
> +#define _SPRB_SCALE 0x71304
> +#define _SPRB_GAMC 0x71400
> +
> +#define SPRCTL(pipe) _PIPE(pipe, _SPRA_CTL, _SPRB_CTL)
> +#define SPRSTRIDE(pipe) _PIPE(pipe, _SPRA_STRIDE, _SPRB_STRIDE)
> +#define SPRPOS(pipe) _PIPE(pipe, _SPRA_POS, _SPRB_POS)
> +#define SPRSIZE(pipe) _PIPE(pipe, _SPRA_SIZE, _SPRB_SIZE)
> +#define SPRKEYVAL(pipe) _PIPE(pipe, _SPRA_KEYVAL, _SPRB_KEYVAL)
> +#define SPRKEYMSK(pipe) _PIPE(pipe, _SPRA_KEYMSK, _SPRB_KEYMSK)
> +#define SPRSURF(pipe) _PIPE(pipe, _SPRA_SURF, _SPRB_SURF)
> +#define SPRKEYMAX(pipe) _PIPE(pipe, _SPRA_KEYMAX, _SPRB_KEYMAX)
> +#define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
> +#define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
> +#define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC)
IVB register block looks good, too.
> +
> /* VBIOS regs */
> #define VGACNTRL 0x71400
> # define VGA_DISP_DISABLE (1 << 31)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b2061b0..09228dd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -915,8 +915,8 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
> pipe_name(pipe));
> }
>
> -static void assert_pipe(struct drm_i915_private *dev_priv,
> - enum pipe pipe, bool state)
> +void assert_pipe(struct drm_i915_private *dev_priv,
> + enum pipe pipe, bool state)
> {
> int reg;
> u32 val;
> @@ -929,8 +929,6 @@ static void assert_pipe(struct drm_i915_private *dev_priv,
> "pipe %c assertion failure (expected %s, current %s)\n",
> pipe_name(pipe), state_string(state), state_string(cur_state));
> }
> -#define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
> -#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
>
> static void assert_plane_enabled(struct drm_i915_private *dev_priv,
> enum plane plane)
> @@ -4439,7 +4437,8 @@ static void ironlake_update_wm(struct drm_device *dev)
> ILK_LP0_CURSOR_LATENCY,
> &plane_wm, &cursor_wm)) {
> I915_WRITE(WM0_PIPEA_ILK,
> - (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> + (plane_wm << WM0_PIPE_PLANE_SHIFT) |
> + (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
> DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
> " plane %d, " "cursor: %d\n",
> plane_wm, cursor_wm);
> @@ -4453,7 +4452,8 @@ static void ironlake_update_wm(struct drm_device *dev)
> ILK_LP0_CURSOR_LATENCY,
> &plane_wm, &cursor_wm)) {
> I915_WRITE(WM0_PIPEB_ILK,
> - (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> + (plane_wm << WM0_PIPE_PLANE_SHIFT) |
> + (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
> DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
> " plane %d, cursor: %d\n",
> plane_wm, cursor_wm);
> @@ -4521,7 +4521,8 @@ static void sandybridge_update_wm(struct drm_device *dev)
> &sandybridge_cursor_wm_info, latency,
> &plane_wm, &cursor_wm)) {
> I915_WRITE(WM0_PIPEA_ILK,
> - (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> + (plane_wm << WM0_PIPE_PLANE_SHIFT) |
> + (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
> DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
> " plane %d, " "cursor: %d\n",
> plane_wm, cursor_wm);
> @@ -4533,7 +4534,8 @@ static void sandybridge_update_wm(struct drm_device *dev)
> &sandybridge_cursor_wm_info, latency,
> &plane_wm, &cursor_wm)) {
> I915_WRITE(WM0_PIPEB_ILK,
> - (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> + (plane_wm << WM0_PIPE_PLANE_SHIFT) |
> + (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
> DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
> " plane %d, cursor: %d\n",
> plane_wm, cursor_wm);
> @@ -4547,7 +4549,8 @@ static void sandybridge_update_wm(struct drm_device *dev)
> &sandybridge_cursor_wm_info, latency,
> &plane_wm, &cursor_wm)) {
> I915_WRITE(WM0_PIPEC_IVB,
> - (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> + (plane_wm << WM0_PIPE_PLANE_SHIFT) |
> + (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
> DRM_DEBUG_KMS("FIFO watermarks For pipe C -"
> " plane %d, cursor: %d\n",
> plane_wm, cursor_wm);
> @@ -7581,7 +7584,7 @@ int intel_framebuffer_init(struct drm_device *dev,
> if (obj->tiling_mode == I915_TILING_Y)
> return -EINVAL;
>
> - if (mode_cmd->pitch & 63)
> + if (mode_cmd->pitches[0] & 63)
> return -EINVAL;
>
> switch (mode_cmd->pixel_format) {
> @@ -8693,7 +8696,7 @@ static void i915_disable_vga(struct drm_device *dev)
> void intel_modeset_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - int i;
> + int i, ret;
>
> drm_mode_config_init(dev);
>
> @@ -8723,6 +8726,12 @@ void intel_modeset_init(struct drm_device *dev)
>
> for (i = 0; i < dev_priv->num_pipe; i++) {
> intel_crtc_init(dev, i);
> + if (HAS_PCH_SPLIT(dev)) {
> + ret = intel_plane_init(dev, i);
> + if (ret)
> + DRM_ERROR("plane %d init failed: %d\n",
> + i, ret);
> + }
> }
>
> /* Just disable it once at startup */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 23c5622..6284562 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -176,10 +176,26 @@ struct intel_crtc {
> bool use_pll_a;
> };
>
> +struct intel_plane {
> + struct drm_plane base;
> + enum pipe pipe;
> + struct drm_i915_gem_object *obj;
> + int max_downscale;
> + u32 lut_r[1024], lut_g[1024], lut_b[1024];
> + void (*update_plane)(struct drm_plane *plane,
> + struct drm_framebuffer *fb, unsigned long start,
start is a strange name for gtt_offset/fb_base/... I'd just pass the
i915_gem_object.
> + int crtc_x, int crtc_y,
> + unsigned int crtc_w, unsigned int crtc_h,
> + uint32_t x, uint32_t y,
> + uint32_t src_w, uint32_t src_h);
> + void (*disable_plane)(struct drm_plane *plane);
> +};
> +
> #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
> #define to_intel_connector(x) container_of(x, struct intel_connector, base)
> #define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
> #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
> +#define to_intel_plane(x) container_of(x, struct intel_plane, base)
>
> #define DIP_HEADER_SIZE 5
>
> @@ -289,6 +305,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
> extern bool intel_dpd_is_edp(struct drm_device *dev);
> extern void intel_edp_link_config(struct intel_encoder *, int *, int *);
> extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
> +extern int intel_plane_init(struct drm_device *dev, enum pipe pipe);
>
> /* intel_panel.c */
> extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
> @@ -379,6 +396,11 @@ extern int intel_overlay_attrs(struct drm_device *dev, void *data,
> extern void intel_fb_output_poll_changed(struct drm_device *dev);
> extern void intel_fb_restore_mode(struct drm_device *dev);
>
> +extern void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
> + bool state);
> +#define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
> +#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
> +
> extern void intel_init_clock_gating(struct drm_device *dev);
> extern void intel_write_eld(struct drm_encoder *encoder,
> struct drm_display_mode *mode);
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index dc1db4f..068b086 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -270,8 +270,14 @@ void intel_fb_restore_mode(struct drm_device *dev)
> {
> int ret;
> drm_i915_private_t *dev_priv = dev->dev_private;
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_plane *plane;
>
> ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
> if (ret)
> DRM_DEBUG("failed to restore crtc mode\n");
> +
> + /* Be sure to shut off any planes that may be active */
> + list_for_each_entry(plane, &config->plane_list, head)
> + plane->funcs->disable_plane(plane);
This should be part of the fb_helper above.
> }
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> new file mode 100644
> index 0000000..f458921
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -0,0 +1,428 @@
> +/*
> + * Copyright © 2011 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + * Authors:
> + * Jesse Barnes <jbarnes@virtuousgeek.org>
> + *
> + * New plane/sprite handling.
> + *
> + * The older chips had a separate interface for programming plane related
> + * registers; newer ones are much simpler and we can use the new DRM plane
> + * support.
> + */
> +#include "drmP.h"
> +#include "drm_crtc.h"
> +#include "intel_drv.h"
> +#include "i915_drm.h"
> +#include "i915_drv.h"
> +
> +/*
> + * Note on refcounting:
> + * When the user creates an fb for the GEM object to be used for the plane,
> + * a ref is taken on the object. However, if the application exits before
> + * disabling the plane, the DRM close handling will free all the fbs and
> + * unless we take a ref on the object, it will be destroyed before the
> + * plane disable hook is called, causing obvious trouble with our efforts
> + * to look up and unpin the object. So we take a ref after we move the
> + * object to the display plane so it won't be destroyed until our disable
> + * hook is called and we drop our private reference.
> + */
Actually, this is wrong. Before the fb gets destroyed we call
drm_framebuffer_cleanup which takes care of this problem. The fact that
- currently drivers call this instead of the drm core
- it's a ducttape solution instead of refcounting
doesn't really make it nice, but it works.
Aside: The insane refcoungting dance in page_flip because the drm core
doesn't know that we still need the to-be-flipped-out fb, it forgets about
it right away. Maybe that's the reason for the above confusion.
> +
> +static void
> +ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
> + unsigned long start, int crtc_x, int crtc_y,
> + unsigned int crtc_w, unsigned int crtc_h,
> + uint32_t x, uint32_t y,
> + uint32_t src_w, uint32_t src_h)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + int pipe = intel_plane->pipe;
> + u32 sprctl, sprscale = 0;
> + u32 reg = SPRCTL(pipe);
> +
> + sprctl = I915_READ(reg);
reg isn't really a great var name. Imo just drop it, only used twice and
reduces readability.
> +
> + /* Mask out pixel format bits in case we change it */
> + sprctl &= ~(SPRITE_DEST_KEY | SPRITE_SOURCE_KEY);
> + sprctl &= ~SPRITE_PIXFORMAT_MASK;
> + sprctl &= ~SPRITE_RGB_ORDER_RGBX;
> + sprctl &= ~SPRITE_YUV_BYTE_ORDER_MASK;
> +
> + switch (fb->pixel_format) {
> + case V4L2_PIX_FMT_BGR32:
> + sprctl |= SPRITE_FORMAT_RGBX888;
> + break;
> + case V4L2_PIX_FMT_RGB32:
> + sprctl |= SPRITE_FORMAT_RGBX888 | SPRITE_RGB_ORDER_RGBX;
> + break;
> + case V4L2_PIX_FMT_YUYV:
> + sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YUYV;
> + break;
> + case V4L2_PIX_FMT_YVYU:
> + sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YVYU;
> + break;
> + case V4L2_PIX_FMT_UYVY:
> + sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_UYVY;
> + break;
> + case V4L2_PIX_FMT_VYUY:
> + sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_VYUY;
> + break;
> + default:
> + DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
> + sprctl |= DVS_FORMAT_RGBX888;
> + break;
> + }
> +
> + sprctl |= SPRITE_TILED;
> +
> + /* must disable */
> + sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
> + sprctl |= SPRITE_ENABLE;
> +
> + /* Sizes are 0 based */
> + src_w--;
> + src_h--;
> + crtc_w--;
> + crtc_h--;
> +
> + /* Adjust watermarks as needed */
> + I915_WRITE(WM1S_LP_ILK, 0x100);
> + I915_WRITE(WM2S_LP_IVB, 0x100);
> + I915_WRITE(WM3S_LP_IVB, 0x100);
> +
> + if (crtc_w != src_w || crtc_h != src_h)
> + sprscale = SPRITE_SCALE_ENABLE | (src_h << 16) | src_w;
> +
> + I915_WRITE(SPRSTRIDE(pipe), fb->pitch);
> + I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
> + I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
> + I915_WRITE(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
> + I915_WRITE(SPRSCALE(pipe), sprscale);
> + I915_WRITE(reg, sprctl);
> + I915_WRITE(SPRSURF(pipe), start);
> + POSTING_READ(SPRSURF(pipe));
> +}
> +
> +static void
> +ivb_disable_plane(struct drm_plane *plane)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + int pipe = intel_plane->pipe;
> +
> + I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);
> + I915_WRITE(SPRSCALE(pipe), 0);
> + I915_WRITE(SPRSURF(pipe), 0);
Is that required or just paranoia? I haven't found anything in bspec
suggesting it's required.
> + POSTING_READ(SPRSURF(pipe));
> + intel_wait_for_vblank(dev, pipe);
> +}
> +
> +static void
> +snb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
> + unsigned long start, int crtc_x, int crtc_y,
> + unsigned int crtc_w, unsigned int crtc_h,
> + uint32_t x, uint32_t y,
> + uint32_t src_w, uint32_t src_h)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + int pipe = intel_plane->pipe;
> + u32 dvscntr, dvsscale = 0;
> + u32 reg = DVSCNTR(pipe);
> +
> + dvscntr = I915_READ(reg);
dito about reg.
> +
> + /* Mask out pixel format bits in case we change it */
> + dvscntr &= ~(DVS_DEST_KEY | DVS_SOURCE_KEY);
> + dvscntr &= ~DVS_PIXFORMAT_MASK;
> + dvscntr &= ~DVS_RGB_ORDER_RGBX;
> + dvscntr &= ~DVS_YUV_BYTE_ORDER_MASK;
> +
> + switch (fb->pixel_format) {
> + case V4L2_PIX_FMT_BGR32:
> + dvscntr |= DVS_FORMAT_RGBX888;
> + break;
> + case V4L2_PIX_FMT_RGB32:
> + dvscntr |= DVS_FORMAT_RGBX888 | DVS_RGB_ORDER_RGBX;
> + break;
> + case V4L2_PIX_FMT_YUYV:
> + dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YUYV;
> + break;
> + case V4L2_PIX_FMT_YVYU:
> + dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YVYU;
> + break;
> + case V4L2_PIX_FMT_UYVY:
> + dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_UYVY;
> + break;
> + case V4L2_PIX_FMT_VYUY:
> + dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_VYUY;
> + break;
> + default:
> + DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
> + dvscntr |= DVS_FORMAT_RGBX888;
> + break;
> + }
> +
> + dvscntr |= DVS_TILED;
> +
> + /* must disable */
> + dvscntr |= DVS_TRICKLE_FEED_DISABLE;
> + dvscntr |= DVS_ENABLE;
> +
> + /* Sizes are 0 based */
> + src_w--;
> + src_h--;
> + crtc_w--;
> + crtc_h--;
> +
> + if (crtc_w != src_w || crtc_h != src_h)
> + dvsscale = DVS_SCALE_ENABLE | (src_h << 16) | src_w;
> +
> + I915_WRITE(DVSSTRIDE(pipe), fb->pitch);
> + I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
> + I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x);
> + I915_WRITE(DVSSIZE(pipe), (crtc_h << 16) | crtc_w);
> + I915_WRITE(DVSSCALE(pipe), dvsscale);
> + I915_WRITE(reg, dvscntr);
> + I915_WRITE(DVSSURF(pipe), start);
> + POSTING_READ(DVSSURF(pipe));
> +}
> +
> +static void
> +snb_disable_plane(struct drm_plane *plane)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + int pipe = intel_plane->pipe;
> +
> + I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE);
> + I915_WRITE(DVSSCALE(pipe), 0);
> + I915_WRITE(DVSSURF(pipe), 0);
dito about paranoia
> + POSTING_READ(DVSSURF(pipe));
> + intel_wait_for_vblank(dev, pipe);
> +}
> +
> +static int
> +intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> + struct drm_framebuffer *fb, int crtc_x, int crtc_y,
> + unsigned int crtc_w, unsigned int crtc_h,
> + uint32_t src_x, uint32_t src_y,
> + uint32_t src_w, uint32_t src_h)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + struct intel_framebuffer *intel_fb;
> + struct drm_i915_gem_object *obj, *old_obj;
> + int pipe = intel_plane->pipe;
> + unsigned long start;
> + int ret = 0;
> + int x = src_x >> 16, y = src_y >> 16;
> + int primary_w = crtc->mode.hdisplay, primary_h = crtc->mode.vdisplay;
> + bool disable_primary = false;
> +
> + intel_fb = to_intel_framebuffer(fb);
> + obj = intel_fb->obj;
> +
> + old_obj = intel_plane->obj;
> +
> + /* Pipe must be running... */
> + if (!(I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE))
> + return -EINVAL;
Ok, how does this tie in with dpms handling? This here feels like a major
cludge ... I suspect we want a dpms function on the plane that gets called
by the common dpms helper before switching of the crtc and after switching
it on. Or at least some driver-hack like I've done for the overlay.
> +
> + if (crtc_x >= primary_w || crtc_y >= primary_h)
> + return -EINVAL;
> +
> + /* Don't modify another pipe's plane */
> + if (intel_plane->pipe != intel_crtc->pipe)
> + return -EINVAL;
> +
> + /*
> + * Clamp the width & height into the visible area. Note we don't
> + * try to scale the source if part of the visible region is offscreen.
> + * The caller must handle that by adjusting source offset and size.
> + */
Allowing the crtc dest rect to extend beyond that of the crtc and then refusing to
properly adjust tiling is a bit inconsistent. I call design-by-committee
on this one ;-)
If the goal is that this plane stuff is somewhat generically useable, I
think this needs to be fixed. If the goal is simply to keep the bubble
intact, I don't mind this as-is, because I don't care ;-)
> + if (crtc_x < 0) {
> + crtc_w += crtc_x;
> + crtc_x = 0;
> + }
> + if (crtc_x + crtc_w > primary_w)
> + crtc_w = primary_w - crtc_x;
> +
> + if (crtc_y < 0) {
> + crtc_h += crtc_y;
> + crtc_y = 0;
> + }
> + if (crtc_y + crtc_h > primary_h)
> + crtc_h = primary_h - crtc_y;
> +
> + /*
> + * We can take a larger source and scale it down, but
> + * only so much... 16x is the max on SNB.
> + */
> + if (((src_w * src_h) / (crtc_w * crtc_h)) > intel_plane->max_downscale)
> + return -EINVAL;
> +
> + /*
> + * If the sprite is completely covering the primary plane,
> + * we can disable the primary and save power.
> + */
> + if ((crtc_x == 0) && (crtc_y == 0) &&
> + (crtc_w == primary_w) && (crtc_h == primary_h))
> + disable_primary = true;
> +
> + mutex_lock(&dev->struct_mutex);
> +
> + if (obj->tiling_mode != I915_TILING_X) {
> + DRM_ERROR("plane surfaces must be X tiled\n");
> + ret = -EINVAL;
> + goto out_unlock;
> + }
Afaics the hw supports untiled buffers (you need to frob the linear offset
register instead of the x/y register for tiled surfaces). Does that not
work?
> +
> + ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
> + if (ret) {
> + DRM_ERROR("failed to pin object\n");
> + goto out_unlock;
> + }
> +
> + drm_gem_object_reference(&obj->base);
> +
> + intel_plane->obj = obj;
> +
> + start = obj->gtt_offset;
Passing obj instead of start would also make untiled sprite easier ...
> +
> + intel_plane->update_plane(plane, fb, start, crtc_x, crtc_y,
> + crtc_w, crtc_h, x, y, src_w, src_h);
> +
> + /* Unpin old obj after new one is active to avoid ugliness */
> + if (old_obj) {
> + /*
> + * It's fairly common to simply update the position of
> + * an existing object. In that case, we don't need to
> + * wait for vblank to avoid ugliness, we only need to
> + * do the pin & ref bookkeeping.
> + */
> + if (old_obj != obj)
> + intel_wait_for_vblank(dev, to_intel_crtc(crtc)->pipe);
> + i915_gem_object_unpin(old_obj);
> + drm_gem_object_unreference(&old_obj->base);
> + }
> +
> +out_unlock:
> + mutex_unlock(&dev->struct_mutex);
Minor locking nitpick: You only need to hold around pin and unpin, not
over the vblank. Stopping gem for 20 msec just to show a sprite is not so
great. I know, our locking isn't really great in general and there are
many other places where we stall in similarly bad ways, still ...
> +
> + return ret;
> +}
> +
> +static int
> +intel_disable_plane(struct drm_plane *plane)
> +{
> + struct drm_device *dev = plane->dev;
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + int ret = 0;
> +
> + mutex_lock(&dev->struct_mutex);
> +
> + intel_plane->disable_plane(plane);
Again, move the vblank_wait which is inside the disable_plane outside of
the struct_mutex lock. We really need some generic infrastructure to run
something after the next vblank in a workqueu. Would clean up pageflip,
the legacy overlay and kill the vblank here ...
> +
> + if (!intel_plane->obj)
> + goto out_unlock;
> +
> + ret = i915_gem_object_finish_gpu(intel_plane->obj);
> + if (ret)
> + goto out_unlock;
What's the reason for that finish_gpu here?
> +
> + i915_gem_object_unpin(intel_plane->obj);
> +
> + drm_gem_object_unreference(&intel_plane->obj->base);
> +
> +out_unlock:
> + intel_plane->obj = NULL;
> +
> + mutex_unlock(&dev->struct_mutex);
> +
> + return ret;
> +}
> +
> +static void intel_destroy_plane(struct drm_plane *plane)
> +{
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + intel_disable_plane(plane);
> + drm_plane_cleanup(plane);
> + kfree(intel_plane);
> +}
> +
> +static const struct drm_plane_funcs intel_plane_funcs = {
> + .update_plane = intel_update_plane,
> + .disable_plane = intel_disable_plane,
> + .destroy = intel_destroy_plane,
> +};
> +
> +static uint32_t snb_plane_formats[] = {
> + V4L2_PIX_FMT_BGR32,
> + V4L2_PIX_FMT_RGB32,
> + V4L2_PIX_FMT_YUYV,
> + V4L2_PIX_FMT_YVYU,
> + V4L2_PIX_FMT_UYVY,
> + V4L2_PIX_FMT_VYUY,
> +};
> +
> +int
> +intel_plane_init(struct drm_device *dev, enum pipe pipe)
> +{
> + struct intel_plane *intel_plane;
> + unsigned long possible_crtcs;
> +
> + if (!(IS_GEN6(dev) || IS_GEN7(dev))) {
> + DRM_ERROR("new plane code only for SNB+\n");
> + return -ENODEV;
> + }
> +
> + intel_plane = kzalloc(sizeof(struct intel_plane), GFP_KERNEL);
> + if (!intel_plane)
> + return -ENOMEM;
> +
> + if (IS_GEN6(dev)) {
> + intel_plane->max_downscale = 16;
> + intel_plane->update_plane = snb_update_plane;
> + intel_plane->disable_plane = snb_disable_plane;
> + } else if (IS_GEN7(dev)) {
> + intel_plane->max_downscale = 2;
> + intel_plane->update_plane = ivb_update_plane;
> + intel_plane->disable_plane = ivb_disable_plane;
> + }
> +
> + intel_plane->pipe = pipe;
> + possible_crtcs = (1 << pipe);
> + drm_plane_init(dev, &intel_plane->base, possible_crtcs,
> + &intel_plane_funcs, snb_plane_formats,
> + ARRAY_SIZE(snb_plane_formats));
> +
> + return 0;
> +}
> +
> --
> 1.7.4.1
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH 3/5] drm/i915: add SNB and IVB video sprite support
2011-11-08 21:57 ` Daniel Vetter
@ 2011-11-08 22:16 ` Daniel Vetter
2011-11-08 22:31 ` Jesse Barnes
1 sibling, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-08 22:16 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx, dri-devel, rob.clark
On Tue, Nov 08, 2011 at 10:57:03PM +0100, Daniel Vetter wrote:
> > + /*
> > + * Clamp the width & height into the visible area. Note we don't
> > + * try to scale the source if part of the visible region is offscreen.
> > + * The caller must handle that by adjusting source offset and size.
> > + */
>
> Allowing the crtc dest rect to extend beyond that of the crtc and then refusing to
> properly adjust tiling is a bit inconsistent. I call design-by-committee
> on this one ;-)
Oops, that comment went fubar while transferring from brainwaves to bits.
Instead of "adjust tiling" it should be "adjust the source rectangle to
the restricted crtc dest".
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] drm/i915: add SNB and IVB video sprite support
2011-11-08 21:57 ` Daniel Vetter
2011-11-08 22:16 ` [Intel-gfx] " Daniel Vetter
@ 2011-11-08 22:31 ` Jesse Barnes
2011-11-08 22:57 ` [Intel-gfx] " Daniel Vetter
1 sibling, 1 reply; 17+ messages in thread
From: Jesse Barnes @ 2011-11-08 22:31 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel, rob.clark
[-- Attachment #1.1: Type: text/plain, Size: 8380 bytes --]
On Tue, 8 Nov 2011 22:57:03 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:
> I've not checked the watermarks, that kind of magic eludes me ;-) A few
> other comments below.
I need to get docs on that; in particular WM handling with downscaling
is an unknown.
> > +#define DVS_SCALE_ENABLE (1<<31)
> > +#define DVS_FILTER_MASK (3<<29)
> > +#define DVS_FILTER_MEDIUM (0<<29)
> > +#define DVS_FILTER_ENHANCING (1<<29)
> > +#define DVS_FILTER_SOFTENING (2<<29)
>
> BSpec has some bits for deinterlace support here, maybe add them just for
> documentation.
Sure, I can toss those in.
> > +#define _SPRA_CTL 0x70280
> > +#define SPRITE_ENABLE (1<<31)
> > +#define SPRITE_GAMMA_ENABLE (1<<30)
> > +#define SPRITE_PIXFORMAT_MASK (7<<25)
> > +#define SPRITE_FORMAT_YUV422 (0<<25)
> > +#define SPRITE_FORMAT_RGBX101010 (1<<25)
> > +#define SPRITE_FORMAT_RGBX888 (2<<25)
> > +#define SPRITE_FORMAT_RGBX161616 (3<<25)
> > +#define SPRITE_FORMAT_YUV444 (4<<25)
> > +#define SPRITE_FORMAT_XBGR101010 (5<<25)
>
> That XBRG is a bit misleading, because the X here means extended range as
> opposed to the usual gl meaning of unused. It sounds like a format with
> shared exponent, gl seems to use E for that, i.e E2BGR101010.
Ah I see, yeah there are 2 30 bit formats, one with XR bias... I
should probably drop that since we don't know the actual bit layout.
> > +#define _SPRA_GAMC 0x70400
>
> Gamma regs are a bit funky, especially on SNB. I assume you'll add all the
> necessary defines when set_gamma support shows up.
Yeah we may need some new ioctls to control the different colorspace
correction and gamma stuff the latest hw supports. I think we can
correct at various places in the pipeline as well...
> > +struct intel_plane {
> > + struct drm_plane base;
> > + enum pipe pipe;
> > + struct drm_i915_gem_object *obj;
> > + int max_downscale;
> > + u32 lut_r[1024], lut_g[1024], lut_b[1024];
> > + void (*update_plane)(struct drm_plane *plane,
> > + struct drm_framebuffer *fb, unsigned long start,
>
> start is a strange name for gtt_offset/fb_base/... I'd just pass the
> i915_gem_object.
Yeah, good point.
> > @@ -270,8 +270,14 @@ void intel_fb_restore_mode(struct drm_device *dev)
> > {
> > int ret;
> > drm_i915_private_t *dev_priv = dev->dev_private;
> > + struct drm_mode_config *config = &dev->mode_config;
> > + struct drm_plane *plane;
> >
> > ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
> > if (ret)
> > DRM_DEBUG("failed to restore crtc mode\n");
> > +
> > + /* Be sure to shut off any planes that may be active */
> > + list_for_each_entry(plane, &config->plane_list, head)
> > + plane->funcs->disable_plane(plane);
>
> This should be part of the fb_helper above.
That would be a bit invasive... and may not be what every driver
wants. Does it belong in set_config? Or do we just forcibly disable
the planes everywhere?
> > + * Note on refcounting:
> > + * When the user creates an fb for the GEM object to be used for the plane,
> > + * a ref is taken on the object. However, if the application exits before
> > + * disabling the plane, the DRM close handling will free all the fbs and
> > + * unless we take a ref on the object, it will be destroyed before the
> > + * plane disable hook is called, causing obvious trouble with our efforts
> > + * to look up and unpin the object. So we take a ref after we move the
> > + * object to the display plane so it won't be destroyed until our disable
> > + * hook is called and we drop our private reference.
> > + */
>
> Actually, this is wrong. Before the fb gets destroyed we call
> drm_framebuffer_cleanup which takes care of this problem. The fact that
> - currently drivers call this instead of the drm core
> - it's a ducttape solution instead of refcounting
> doesn't really make it nice, but it works.
Not sure I understand what you mean about drm_framebuffer_cleanup
taking care of the problem. The refcounting and fb handling may not be
ideal, but taking refs on the underlying objects is what we need to do
today.
I don't want to change that as part of this series, but did want to
explain why we take a private ref for the plane object here.
> > + sprctl = I915_READ(reg);
>
> reg isn't really a great var name. Imo just drop it, only used twice and
> reduces readability.
Sure, easy enough.
> > + I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);
> > + I915_WRITE(SPRSCALE(pipe), 0);
> > + I915_WRITE(SPRSURF(pipe), 0);
>
> Is that required or just paranoia? I haven't found anything in bspec
> suggesting it's required.
The scaling disable was just to avoid surprises (in fact now that I look
I need to mask it off in the update function).
The surf write is definitely needed, since it triggers the double
buffered reg update.
> > +
> > + /* Pipe must be running... */
> > + if (!(I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE))
> > + return -EINVAL;
>
> Ok, how does this tie in with dpms handling? This here feels like a major
> cludge ... I suspect we want a dpms function on the plane that gets called
> by the common dpms helper before switching of the crtc and after switching
> it on. Or at least some driver-hack like I've done for the overlay.
It doesn't atm, but it should.
> > + /*
> > + * Clamp the width & height into the visible area. Note we don't
> > + * try to scale the source if part of the visible region is offscreen.
> > + * The caller must handle that by adjusting source offset and size.
> > + */
>
> Allowing the crtc dest rect to extend beyond that of the crtc and then refusing to
> properly adjust tiling is a bit inconsistent. I call design-by-committee
> on this one ;-)
>
> If the goal is that this plane stuff is somewhat generically useable, I
> think this needs to be fixed. If the goal is simply to keep the bubble
> intact, I don't mind this as-is, because I don't care ;-)
The primary goal here is to keep the plane from breaking if a crtc rect
that includes an offscreen portion is passed in.
Moving the source rect would involve potential scaling. I can do that,
but was trying to avoid it. We can also change it when we see what the
embedded folks had in mind (I think Rob was the one who wanted signed
crtc position to make some things easy).
> > + if (obj->tiling_mode != I915_TILING_X) {
> > + DRM_ERROR("plane surfaces must be X tiled\n");
> > + ret = -EINVAL;
> > + goto out_unlock;
> > + }
>
> Afaics the hw supports untiled buffers (you need to frob the linear offset
> register instead of the x/y register for tiled surfaces). Does that not
> work?
Yes it does, and I see the regs on IVB now. I'll implement support for
them to make it easier to display surfaces from misc. sources without
copying.
> > +out_unlock:
> > + mutex_unlock(&dev->struct_mutex);
>
> Minor locking nitpick: You only need to hold around pin and unpin, not
> over the vblank. Stopping gem for 20 msec just to show a sprite is not so
> great. I know, our locking isn't really great in general and there are
> many other places where we stall in similarly bad ways, still ...
Yes, good point. Will fix.
>
> > +
> > + return ret;
> > +}
> > +
> > +static int
> > +intel_disable_plane(struct drm_plane *plane)
> > +{
> > + struct drm_device *dev = plane->dev;
> > + struct intel_plane *intel_plane = to_intel_plane(plane);
> > + int ret = 0;
> > +
> > + mutex_lock(&dev->struct_mutex);
> > +
> > + intel_plane->disable_plane(plane);
>
> Again, move the vblank_wait which is inside the disable_plane outside of
> the struct_mutex lock. We really need some generic infrastructure to run
> something after the next vblank in a workqueu. Would clean up pageflip,
> the legacy overlay and kill the vblank here ...
Yeah, that would be nice. Blocking userspace on the vblank is also not
nice.
> > + ret = i915_gem_object_finish_gpu(intel_plane->obj);
> > + if (ret)
> > + goto out_unlock;
>
> What's the reason for that finish_gpu here?
Slavish emulation of some other teardown code paths. If we ever do
flipping on the sprites, I think we'll want it.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Intel-gfx] [PATCH 3/5] drm/i915: add SNB and IVB video sprite support
2011-11-08 22:31 ` Jesse Barnes
@ 2011-11-08 22:57 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-08 22:57 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx, dri-devel, rob.clark
On Tue, Nov 08, 2011 at 02:31:00PM -0800, Jesse Barnes wrote:
> On Tue, 8 Nov 2011 22:57:03 +0100 Daniel Vetter <daniel@ffwll.ch> wrote:
> > > @@ -270,8 +270,14 @@ void intel_fb_restore_mode(struct drm_device *dev)
> > > {
> > > int ret;
> > > drm_i915_private_t *dev_priv = dev->dev_private;
> > > + struct drm_mode_config *config = &dev->mode_config;
> > > + struct drm_plane *plane;
> > >
> > > ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper);
> > > if (ret)
> > > DRM_DEBUG("failed to restore crtc mode\n");
> > > +
> > > + /* Be sure to shut off any planes that may be active */
> > > + list_for_each_entry(plane, &config->plane_list, head)
> > > + plane->funcs->disable_plane(plane);
> >
> > This should be part of the fb_helper above.
>
> That would be a bit invasive... and may not be what every driver
> wants. Does it belong in set_config? Or do we just forcibly disable
> the planes everywhere?
Core drm code currently does not call fb_helper_restore, that's the
drivers job atm (yeah, somewhere on my todo). And i915 is currently the
only one with sprite support. And I don't think we want the last video
frame to occlude the OOPS when X died. So yes, this imo belongs into the
fb_helper.
> > > + * Note on refcounting:
> > > + * When the user creates an fb for the GEM object to be used for the plane,
> > > + * a ref is taken on the object. However, if the application exits before
> > > + * disabling the plane, the DRM close handling will free all the fbs and
> > > + * unless we take a ref on the object, it will be destroyed before the
> > > + * plane disable hook is called, causing obvious trouble with our efforts
> > > + * to look up and unpin the object. So we take a ref after we move the
> > > + * object to the display plane so it won't be destroyed until our disable
> > > + * hook is called and we drop our private reference.
> > > + */
> >
> > Actually, this is wrong. Before the fb gets destroyed we call
> > drm_framebuffer_cleanup which takes care of this problem. The fact that
> > - currently drivers call this instead of the drm core
> > - it's a ducttape solution instead of refcounting
> > doesn't really make it nice, but it works.
>
> Not sure I understand what you mean about drm_framebuffer_cleanup
> taking care of the problem. The refcounting and fb handling may not be
> ideal, but taking refs on the underlying objects is what we need to do
> today.
Well, I think we don't need to take refs. set_base gets away with it, too:
- the pin/unpin ensures that the buffer doesn't move.
- the drm core holds onto both the old and the new fb for the duration of
the update_plane, so we also have a reference on that. And that
reference won't disappear without a disable_plane thanks to the addition to
drm_framebuffer_cleanup I've prodded you into writing.
> I don't want to change that as part of this series, but did want to
> explain why we take a private ref for the plane object here.
Yeah, cleanup up our fb handling will be a bit messy.
>
> > > + sprctl = I915_READ(reg);
> >
> > reg isn't really a great var name. Imo just drop it, only used twice and
> > reduces readability.
>
> Sure, easy enough.
>
> > > + I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);
> > > + I915_WRITE(SPRSCALE(pipe), 0);
> > > + I915_WRITE(SPRSURF(pipe), 0);
> >
> > Is that required or just paranoia? I haven't found anything in bspec
> > suggesting it's required.
>
> The scaling disable was just to avoid surprises (in fact now that I look
> I need to mask it off in the update function).
>
> The surf write is definitely needed, since it triggers the double
> buffered reg update.
Ok, missed that in bspec review. Maybe add a comment for dummies
somewhere?
[snip]
> > > + ret = i915_gem_object_finish_gpu(intel_plane->obj);
> > > + if (ret)
> > > + goto out_unlock;
> >
> > What's the reason for that finish_gpu here?
>
> Slavish emulation of some other teardown code paths. If we ever do
> flipping on the sprites, I think we'll want it.
I think when we do flipping on sprites, we should try MI_LOAD_REG to latch
the double-buffered regs in the command stream. That way it'd work like
the pageflip paths.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/5] drm/i915: add destination color key support
2011-11-07 18:02 [PATCH] DRM planes Jesse Barnes
` (2 preceding siblings ...)
2011-11-07 18:02 ` [PATCH 3/5] drm/i915: add SNB and IVB video sprite support Jesse Barnes
@ 2011-11-07 18:02 ` Jesse Barnes
2011-11-08 22:06 ` Daniel Vetter
2011-11-07 18:02 ` [PATCH 5/5] drm/i915: track sprite coverage and disable primary plane if possible Jesse Barnes
4 siblings, 1 reply; 17+ messages in thread
From: Jesse Barnes @ 2011-11-07 18:02 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, rob.clark
Add new ioctls for getting and setting the current destination color
key. This allows for simple overlay display control by matching a color
key value in the primary plane before blending the overlay on top.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/i915_dma.c | 2 +
drivers/gpu/drm/i915/i915_reg.h | 2 +
drivers/gpu/drm/i915/intel_drv.h | 8 ++
drivers/gpu/drm/i915/intel_sprite.c | 146 +++++++++++++++++++++++++++++++++++
include/drm/i915_drm.h | 16 ++++
5 files changed, 174 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2eac955..0385a27 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2294,6 +2294,8 @@ struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_DESTKEY, intel_sprite_set_destkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_DESTKEY, intel_sprite_get_destkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
};
int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b2270fa..8bed5d8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2724,6 +2724,8 @@
#define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE)
#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE)
#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
+#define DVSKEYVAL(pipe) _PIPE(pipe, _DVSAKEYVAL, _DVSBKEYVAL)
+#define DVSKEYMSK(pipe) _PIPE(pipe, _DVSAKEYMSK, _DVSBKEYMSK)
#define _SPRA_CTL 0x70280
#define SPRITE_ENABLE (1<<31)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6284562..b0081d2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -189,6 +189,8 @@ struct intel_plane {
uint32_t x, uint32_t y,
uint32_t src_w, uint32_t src_h);
void (*disable_plane)(struct drm_plane *plane);
+ int (*update_destkey)(struct drm_plane *plane, u32 value);
+ u32 (*get_destkey)(struct drm_plane *plane);
};
#define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
@@ -406,4 +408,10 @@ extern void intel_write_eld(struct drm_encoder *encoder,
struct drm_display_mode *mode);
extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
+extern int intel_sprite_set_destkey(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
+extern int intel_sprite_get_destkey(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
+
+
#endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index f458921..1adeda8 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -99,6 +99,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
/* must disable */
sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
sprctl |= SPRITE_ENABLE;
+ sprctl |= SPRITE_DEST_KEY;
/* Sizes are 0 based */
src_w--;
@@ -139,6 +140,45 @@ ivb_disable_plane(struct drm_plane *plane)
intel_wait_for_vblank(dev, pipe);
}
+static int
+ivb_update_destkey(struct drm_plane *plane, u32 value)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane;
+ int ret = 0;
+
+ if (value > 0xffffff)
+ return -EINVAL;
+
+ intel_plane = to_intel_plane(plane);
+
+ mutex_lock(&dev->struct_mutex);
+ I915_WRITE(SPRKEYVAL(intel_plane->pipe), value);
+ I915_WRITE(SPRKEYMSK(intel_plane->pipe), 0xffffff);
+ POSTING_READ(SPRKEYMSK(intel_plane->pipe));
+ mutex_unlock(&dev->struct_mutex);
+
+ return ret;
+}
+
+static u32
+ivb_get_destkey(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane;
+ u32 value;
+
+ intel_plane = to_intel_plane(plane);
+
+ mutex_lock(&dev->struct_mutex);
+ value = I915_READ(SPRKEYVAL(intel_plane->pipe));
+ mutex_unlock(&dev->struct_mutex);
+
+ return value;
+}
+
static void
snb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
unsigned long start, int crtc_x, int crtc_y,
@@ -227,6 +267,45 @@ snb_disable_plane(struct drm_plane *plane)
}
static int
+snb_update_destkey(struct drm_plane *plane, u32 value)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane;
+ int ret = 0;
+
+ if (value > 0xffffff)
+ return -EINVAL;
+
+ intel_plane = to_intel_plane(plane);
+
+ mutex_lock(&dev->struct_mutex);
+ I915_WRITE(DVSKEYVAL(intel_plane->pipe), value);
+ I915_WRITE(DVSKEYMSK(intel_plane->pipe), 0xffffff);
+ POSTING_READ(DVSKEYMSK(intel_plane->pipe));
+ mutex_unlock(&dev->struct_mutex);
+
+ return ret;
+}
+
+static u32
+snb_get_destkey(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane;
+ u32 value;
+
+ intel_plane = to_intel_plane(plane);
+
+ mutex_lock(&dev->struct_mutex);
+ value = I915_READ(DVSKEYVAL(intel_plane->pipe));
+ mutex_unlock(&dev->struct_mutex);
+
+ return value;
+}
+
+static int
intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
struct drm_framebuffer *fb, int crtc_x, int crtc_y,
unsigned int crtc_w, unsigned int crtc_h,
@@ -377,6 +456,69 @@ static void intel_destroy_plane(struct drm_plane *plane)
kfree(intel_plane);
}
+int intel_sprite_set_destkey(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_intel_set_sprite_destkey *set = data;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_mode_object *obj;
+ struct drm_plane *plane;
+ struct intel_plane *intel_plane;
+ int ret = 0;
+
+ if (!dev_priv)
+ return -EINVAL;
+
+ if (set->value > 0xffffff)
+ return -EINVAL;
+
+ mutex_lock(&dev->mode_config.mutex);
+
+ obj = drm_mode_object_find(dev, set->plane_id, DRM_MODE_OBJECT_PLANE);
+ if (!obj) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ plane = obj_to_plane(obj);
+ intel_plane = to_intel_plane(plane);
+ ret = intel_plane->update_destkey(plane, set->value);
+
+out_unlock:
+ mutex_unlock(&dev->mode_config.mutex);
+ return ret;
+}
+
+int intel_sprite_get_destkey(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_intel_get_sprite_destkey *get = data;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_mode_object *obj;
+ struct drm_plane *plane;
+ struct intel_plane *intel_plane;
+ int ret = 0;
+
+ if (!dev_priv)
+ return -EINVAL;
+
+ mutex_lock(&dev->mode_config.mutex);
+
+ obj = drm_mode_object_find(dev, get->plane_id, DRM_MODE_OBJECT_PLANE);
+ if (!obj) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ plane = obj_to_plane(obj);
+ intel_plane = to_intel_plane(plane);
+ get->value = intel_plane->get_destkey(plane);
+
+out_unlock:
+ mutex_unlock(&dev->mode_config.mutex);
+ return ret;
+}
+
static const struct drm_plane_funcs intel_plane_funcs = {
.update_plane = intel_update_plane,
.disable_plane = intel_disable_plane,
@@ -411,10 +553,14 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe)
intel_plane->max_downscale = 16;
intel_plane->update_plane = snb_update_plane;
intel_plane->disable_plane = snb_disable_plane;
+ intel_plane->update_destkey = snb_update_destkey;
+ intel_plane->get_destkey = snb_get_destkey;
} else if (IS_GEN7(dev)) {
intel_plane->max_downscale = 2;
intel_plane->update_plane = ivb_update_plane;
intel_plane->disable_plane = ivb_disable_plane;
+ intel_plane->update_destkey = ivb_update_destkey;
+ intel_plane->get_destkey = ivb_get_destkey;
}
intel_plane->pipe = pipe;
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 28c0d11..71674b3 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -198,6 +198,8 @@ typedef struct _drm_i915_sarea {
#define DRM_I915_OVERLAY_PUT_IMAGE 0x27
#define DRM_I915_OVERLAY_ATTRS 0x28
#define DRM_I915_GEM_EXECBUFFER2 0x29
+#define DRM_I915_GET_SPRITE_DESTKEY 0x2a
+#define DRM_I915_SET_SPRITE_DESTKEY 0x2b
#define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
#define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
@@ -239,6 +241,8 @@ typedef struct _drm_i915_sarea {
#define DRM_IOCTL_I915_GEM_MADVISE DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MADVISE, struct drm_i915_gem_madvise)
#define DRM_IOCTL_I915_OVERLAY_PUT_IMAGE DRM_IOW(DRM_COMMAND_BASE + DRM_I915_OVERLAY_PUT_IMAGE, struct drm_intel_overlay_put_image)
#define DRM_IOCTL_I915_OVERLAY_ATTRS DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_OVERLAY_ATTRS, struct drm_intel_overlay_attrs)
+#define DRM_IOCTL_I915_SET_SPRITE_DESTKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_DESTKEY, struct drm_intel_set_sprite_destkey)
+#define DRM_IOCTL_I915_GET_SPRITE_DESTKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_DESTKEY, struct drm_intel_get_sprite_destkey)
/* Allow drivers to submit batchbuffers directly to hardware, relying
* on the security mechanisms provided by hardware.
@@ -844,4 +848,16 @@ struct drm_intel_overlay_attrs {
__u32 gamma5;
};
+/* Set the destination color key on a given sprite */
+struct drm_intel_set_sprite_destkey {
+ __u32 plane_id;
+ __u32 value;
+};
+
+/* Get the current destination color key on a given sprite */
+struct drm_intel_get_sprite_destkey {
+ __u32 plane_id;
+ __u32 value;
+};
+
#endif /* _I915_DRM_H_ */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/5] drm/i915: add destination color key support
2011-11-07 18:02 ` [PATCH 4/5] drm/i915: add destination color key support Jesse Barnes
@ 2011-11-08 22:06 ` Daniel Vetter
2011-11-08 22:36 ` Alex Deucher
2011-11-08 22:40 ` Jesse Barnes
0 siblings, 2 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-08 22:06 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx, dri-devel, rob.clark
On Mon, Nov 07, 2011 at 10:02:55AM -0800, Jesse Barnes wrote:
> Add new ioctls for getting and setting the current destination color
> key. This allows for simple overlay display control by matching a color
> key value in the primary plane before blending the overlay on top.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
A few comments below and a snide-y one here: If the goal is simply to keep
the bubble intact, this is fine by me. If we want something that has the
chance to be useable by a generic driver I think we want a drm interface
for plane/crtc properties (you could wire up set_gamma on planes in the
same series ... ;-) Again, I don't really care ...
Cheers, Daniel
> ---
> drivers/gpu/drm/i915/i915_dma.c | 2 +
> drivers/gpu/drm/i915/i915_reg.h | 2 +
> drivers/gpu/drm/i915/intel_drv.h | 8 ++
> drivers/gpu/drm/i915/intel_sprite.c | 146 +++++++++++++++++++++++++++++++++++
> include/drm/i915_drm.h | 16 ++++
> 5 files changed, 174 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 2eac955..0385a27 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -2294,6 +2294,8 @@ struct drm_ioctl_desc i915_ioctls[] = {
> DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
> DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_DESTKEY, intel_sprite_set_destkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_DESTKEY, intel_sprite_get_destkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> };
>
> int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b2270fa..8bed5d8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2724,6 +2724,8 @@
> #define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE)
> #define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE)
> #define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
> +#define DVSKEYVAL(pipe) _PIPE(pipe, _DVSAKEYVAL, _DVSBKEYVAL)
> +#define DVSKEYMSK(pipe) _PIPE(pipe, _DVSAKEYMSK, _DVSBKEYMSK)
>
> #define _SPRA_CTL 0x70280
> #define SPRITE_ENABLE (1<<31)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6284562..b0081d2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -189,6 +189,8 @@ struct intel_plane {
> uint32_t x, uint32_t y,
> uint32_t src_w, uint32_t src_h);
> void (*disable_plane)(struct drm_plane *plane);
> + int (*update_destkey)(struct drm_plane *plane, u32 value);
> + u32 (*get_destkey)(struct drm_plane *plane);
> };
>
> #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
> @@ -406,4 +408,10 @@ extern void intel_write_eld(struct drm_encoder *encoder,
> struct drm_display_mode *mode);
> extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
>
> +extern int intel_sprite_set_destkey(struct drm_device *dev, void *data,
> + struct drm_file *file_priv);
> +extern int intel_sprite_get_destkey(struct drm_device *dev, void *data,
> + struct drm_file *file_priv);
> +
> +
> #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index f458921..1adeda8 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -99,6 +99,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
> /* must disable */
> sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
> sprctl |= SPRITE_ENABLE;
> + sprctl |= SPRITE_DEST_KEY;
>
> /* Sizes are 0 based */
> src_w--;
> @@ -139,6 +140,45 @@ ivb_disable_plane(struct drm_plane *plane)
> intel_wait_for_vblank(dev, pipe);
> }
>
> +static int
> +ivb_update_destkey(struct drm_plane *plane, u32 value)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_plane *intel_plane;
> + int ret = 0;
> +
> + if (value > 0xffffff)
> + return -EINVAL;
> +
> + intel_plane = to_intel_plane(plane);
> +
> + mutex_lock(&dev->struct_mutex);
> + I915_WRITE(SPRKEYVAL(intel_plane->pipe), value);
> + I915_WRITE(SPRKEYMSK(intel_plane->pipe), 0xffffff);
> + POSTING_READ(SPRKEYMSK(intel_plane->pipe));
> + mutex_unlock(&dev->struct_mutex);
"Just grab the lock in case" is nice and all, but totally unnecessary
here. You already grab the mode_config.mutex in the intel ioctl interface
function.
> +
> + return ret;
> +}
> +
> +static u32
> +ivb_get_destkey(struct drm_plane *plane)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_plane *intel_plane;
> + u32 value;
> +
> + intel_plane = to_intel_plane(plane);
> +
> + mutex_lock(&dev->struct_mutex);
> + value = I915_READ(SPRKEYVAL(intel_plane->pipe));
> + mutex_unlock(&dev->struct_mutex);
dito
> +
> + return value;
> +}
> +
> static void
> snb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
> unsigned long start, int crtc_x, int crtc_y,
> @@ -227,6 +267,45 @@ snb_disable_plane(struct drm_plane *plane)
> }
>
> static int
> +snb_update_destkey(struct drm_plane *plane, u32 value)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_plane *intel_plane;
> + int ret = 0;
> +
> + if (value > 0xffffff)
> + return -EINVAL;
> +
> + intel_plane = to_intel_plane(plane);
> +
> + mutex_lock(&dev->struct_mutex);
> + I915_WRITE(DVSKEYVAL(intel_plane->pipe), value);
> + I915_WRITE(DVSKEYMSK(intel_plane->pipe), 0xffffff);
> + POSTING_READ(DVSKEYMSK(intel_plane->pipe));
> + mutex_unlock(&dev->struct_mutex);
same
> +
> + return ret;
> +}
> +
> +static u32
> +snb_get_destkey(struct drm_plane *plane)
> +{
> + struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_plane *intel_plane;
> + u32 value;
> +
> + intel_plane = to_intel_plane(plane);
> +
> + mutex_lock(&dev->struct_mutex);
> + value = I915_READ(DVSKEYVAL(intel_plane->pipe));
> + mutex_unlock(&dev->struct_mutex);
again
> +
> + return value;
> +}
> +
> +static int
> intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> struct drm_framebuffer *fb, int crtc_x, int crtc_y,
> unsigned int crtc_w, unsigned int crtc_h,
> @@ -377,6 +456,69 @@ static void intel_destroy_plane(struct drm_plane *plane)
> kfree(intel_plane);
> }
>
> +int intel_sprite_set_destkey(struct drm_device *dev, void *data,
> + struct drm_file *file_priv)
> +{
> + struct drm_intel_set_sprite_destkey *set = data;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_mode_object *obj;
> + struct drm_plane *plane;
> + struct intel_plane *intel_plane;
> + int ret = 0;
> +
> + if (!dev_priv)
> + return -EINVAL;
> +
> + if (set->value > 0xffffff)
> + return -EINVAL;
> +
> + mutex_lock(&dev->mode_config.mutex);
> +
> + obj = drm_mode_object_find(dev, set->plane_id, DRM_MODE_OBJECT_PLANE);
> + if (!obj) {
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + plane = obj_to_plane(obj);
> + intel_plane = to_intel_plane(plane);
> + ret = intel_plane->update_destkey(plane, set->value);
> +
> +out_unlock:
> + mutex_unlock(&dev->mode_config.mutex);
> + return ret;
> +}
> +
> +int intel_sprite_get_destkey(struct drm_device *dev, void *data,
> + struct drm_file *file_priv)
> +{
> + struct drm_intel_get_sprite_destkey *get = data;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_mode_object *obj;
> + struct drm_plane *plane;
> + struct intel_plane *intel_plane;
> + int ret = 0;
> +
> + if (!dev_priv)
> + return -EINVAL;
> +
> + mutex_lock(&dev->mode_config.mutex);
> +
> + obj = drm_mode_object_find(dev, get->plane_id, DRM_MODE_OBJECT_PLANE);
> + if (!obj) {
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + plane = obj_to_plane(obj);
> + intel_plane = to_intel_plane(plane);
> + get->value = intel_plane->get_destkey(plane);
> +
> +out_unlock:
> + mutex_unlock(&dev->mode_config.mutex);
> + return ret;
> +}
Frankly I'd prefer the driver code to simply keep track of the destkey
instead of interogating the hw with the ->get_destkey call.
> +
> static const struct drm_plane_funcs intel_plane_funcs = {
> .update_plane = intel_update_plane,
> .disable_plane = intel_disable_plane,
> @@ -411,10 +553,14 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe)
> intel_plane->max_downscale = 16;
> intel_plane->update_plane = snb_update_plane;
> intel_plane->disable_plane = snb_disable_plane;
> + intel_plane->update_destkey = snb_update_destkey;
> + intel_plane->get_destkey = snb_get_destkey;
> } else if (IS_GEN7(dev)) {
> intel_plane->max_downscale = 2;
> intel_plane->update_plane = ivb_update_plane;
> intel_plane->disable_plane = ivb_disable_plane;
> + intel_plane->update_destkey = ivb_update_destkey;
> + intel_plane->get_destkey = ivb_get_destkey;
> }
>
> intel_plane->pipe = pipe;
> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> index 28c0d11..71674b3 100644
> --- a/include/drm/i915_drm.h
> +++ b/include/drm/i915_drm.h
> @@ -198,6 +198,8 @@ typedef struct _drm_i915_sarea {
> #define DRM_I915_OVERLAY_PUT_IMAGE 0x27
> #define DRM_I915_OVERLAY_ATTRS 0x28
> #define DRM_I915_GEM_EXECBUFFER2 0x29
> +#define DRM_I915_GET_SPRITE_DESTKEY 0x2a
> +#define DRM_I915_SET_SPRITE_DESTKEY 0x2b
>
> #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
> #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
> @@ -239,6 +241,8 @@ typedef struct _drm_i915_sarea {
> #define DRM_IOCTL_I915_GEM_MADVISE DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MADVISE, struct drm_i915_gem_madvise)
> #define DRM_IOCTL_I915_OVERLAY_PUT_IMAGE DRM_IOW(DRM_COMMAND_BASE + DRM_I915_OVERLAY_PUT_IMAGE, struct drm_intel_overlay_put_image)
> #define DRM_IOCTL_I915_OVERLAY_ATTRS DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_OVERLAY_ATTRS, struct drm_intel_overlay_attrs)
> +#define DRM_IOCTL_I915_SET_SPRITE_DESTKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_DESTKEY, struct drm_intel_set_sprite_destkey)
> +#define DRM_IOCTL_I915_GET_SPRITE_DESTKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_DESTKEY, struct drm_intel_get_sprite_destkey)
>
> /* Allow drivers to submit batchbuffers directly to hardware, relying
> * on the security mechanisms provided by hardware.
> @@ -844,4 +848,16 @@ struct drm_intel_overlay_attrs {
> __u32 gamma5;
> };
>
> +/* Set the destination color key on a given sprite */
> +struct drm_intel_set_sprite_destkey {
> + __u32 plane_id;
> + __u32 value;
> +};
> +
> +/* Get the current destination color key on a given sprite */
> +struct drm_intel_get_sprite_destkey {
> + __u32 plane_id;
> + __u32 value;
> +};
> +
> #endif /* _I915_DRM_H_ */
> --
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 4/5] drm/i915: add destination color key support
2011-11-08 22:06 ` Daniel Vetter
@ 2011-11-08 22:36 ` Alex Deucher
2011-11-08 23:08 ` Daniel Vetter
2011-11-08 22:40 ` Jesse Barnes
1 sibling, 1 reply; 17+ messages in thread
From: Alex Deucher @ 2011-11-08 22:36 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel, rob.clark
On Tue, Nov 8, 2011 at 5:06 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Nov 07, 2011 at 10:02:55AM -0800, Jesse Barnes wrote:
>> Add new ioctls for getting and setting the current destination color
>> key. This allows for simple overlay display control by matching a color
>> key value in the primary plane before blending the overlay on top.
>>
>> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>
> A few comments below and a snide-y one here: If the goal is simply to keep
> the bubble intact, this is fine by me. If we want something that has the
> chance to be useable by a generic driver I think we want a drm interface
> for plane/crtc properties (you could wire up set_gamma on planes in the
> same series ... ;-) Again, I don't really care ...
Even further off topic, the panel scaler support should ideally be a
property of crtcs/planes rather than a connector property. However,
that would be a bit of pain to implement and keep backwards
compatibility.
Alex
>
> Cheers, Daniel
>
>> ---
>> drivers/gpu/drm/i915/i915_dma.c | 2 +
>> drivers/gpu/drm/i915/i915_reg.h | 2 +
>> drivers/gpu/drm/i915/intel_drv.h | 8 ++
>> drivers/gpu/drm/i915/intel_sprite.c | 146 +++++++++++++++++++++++++++++++++++
>> include/drm/i915_drm.h | 16 ++++
>> 5 files changed, 174 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 2eac955..0385a27 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -2294,6 +2294,8 @@ struct drm_ioctl_desc i915_ioctls[] = {
>> DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
>> DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>> DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>> + DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_DESTKEY, intel_sprite_set_destkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>> + DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_DESTKEY, intel_sprite_get_destkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>> };
>>
>> int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index b2270fa..8bed5d8 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2724,6 +2724,8 @@
>> #define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE)
>> #define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE)
>> #define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
>> +#define DVSKEYVAL(pipe) _PIPE(pipe, _DVSAKEYVAL, _DVSBKEYVAL)
>> +#define DVSKEYMSK(pipe) _PIPE(pipe, _DVSAKEYMSK, _DVSBKEYMSK)
>>
>> #define _SPRA_CTL 0x70280
>> #define SPRITE_ENABLE (1<<31)
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 6284562..b0081d2 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -189,6 +189,8 @@ struct intel_plane {
>> uint32_t x, uint32_t y,
>> uint32_t src_w, uint32_t src_h);
>> void (*disable_plane)(struct drm_plane *plane);
>> + int (*update_destkey)(struct drm_plane *plane, u32 value);
>> + u32 (*get_destkey)(struct drm_plane *plane);
>> };
>>
>> #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
>> @@ -406,4 +408,10 @@ extern void intel_write_eld(struct drm_encoder *encoder,
>> struct drm_display_mode *mode);
>> extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
>>
>> +extern int intel_sprite_set_destkey(struct drm_device *dev, void *data,
>> + struct drm_file *file_priv);
>> +extern int intel_sprite_get_destkey(struct drm_device *dev, void *data,
>> + struct drm_file *file_priv);
>> +
>> +
>> #endif /* __INTEL_DRV_H__ */
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>> index f458921..1adeda8 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -99,6 +99,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>> /* must disable */
>> sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
>> sprctl |= SPRITE_ENABLE;
>> + sprctl |= SPRITE_DEST_KEY;
>>
>> /* Sizes are 0 based */
>> src_w--;
>> @@ -139,6 +140,45 @@ ivb_disable_plane(struct drm_plane *plane)
>> intel_wait_for_vblank(dev, pipe);
>> }
>>
>> +static int
>> +ivb_update_destkey(struct drm_plane *plane, u32 value)
>> +{
>> + struct drm_device *dev = plane->dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + struct intel_plane *intel_plane;
>> + int ret = 0;
>> +
>> + if (value > 0xffffff)
>> + return -EINVAL;
>> +
>> + intel_plane = to_intel_plane(plane);
>> +
>> + mutex_lock(&dev->struct_mutex);
>> + I915_WRITE(SPRKEYVAL(intel_plane->pipe), value);
>> + I915_WRITE(SPRKEYMSK(intel_plane->pipe), 0xffffff);
>> + POSTING_READ(SPRKEYMSK(intel_plane->pipe));
>> + mutex_unlock(&dev->struct_mutex);
>
> "Just grab the lock in case" is nice and all, but totally unnecessary
> here. You already grab the mode_config.mutex in the intel ioctl interface
> function.
>
>> +
>> + return ret;
>> +}
>> +
>> +static u32
>> +ivb_get_destkey(struct drm_plane *plane)
>> +{
>> + struct drm_device *dev = plane->dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + struct intel_plane *intel_plane;
>> + u32 value;
>> +
>> + intel_plane = to_intel_plane(plane);
>> +
>> + mutex_lock(&dev->struct_mutex);
>> + value = I915_READ(SPRKEYVAL(intel_plane->pipe));
>> + mutex_unlock(&dev->struct_mutex);
>
> dito
>
>> +
>> + return value;
>> +}
>> +
>> static void
>> snb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>> unsigned long start, int crtc_x, int crtc_y,
>> @@ -227,6 +267,45 @@ snb_disable_plane(struct drm_plane *plane)
>> }
>>
>> static int
>> +snb_update_destkey(struct drm_plane *plane, u32 value)
>> +{
>> + struct drm_device *dev = plane->dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + struct intel_plane *intel_plane;
>> + int ret = 0;
>> +
>> + if (value > 0xffffff)
>> + return -EINVAL;
>> +
>> + intel_plane = to_intel_plane(plane);
>> +
>> + mutex_lock(&dev->struct_mutex);
>> + I915_WRITE(DVSKEYVAL(intel_plane->pipe), value);
>> + I915_WRITE(DVSKEYMSK(intel_plane->pipe), 0xffffff);
>> + POSTING_READ(DVSKEYMSK(intel_plane->pipe));
>> + mutex_unlock(&dev->struct_mutex);
>
> same
>
>> +
>> + return ret;
>> +}
>> +
>> +static u32
>> +snb_get_destkey(struct drm_plane *plane)
>> +{
>> + struct drm_device *dev = plane->dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + struct intel_plane *intel_plane;
>> + u32 value;
>> +
>> + intel_plane = to_intel_plane(plane);
>> +
>> + mutex_lock(&dev->struct_mutex);
>> + value = I915_READ(DVSKEYVAL(intel_plane->pipe));
>> + mutex_unlock(&dev->struct_mutex);
>
> again
>
>> +
>> + return value;
>> +}
>> +
>> +static int
>> intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>> struct drm_framebuffer *fb, int crtc_x, int crtc_y,
>> unsigned int crtc_w, unsigned int crtc_h,
>> @@ -377,6 +456,69 @@ static void intel_destroy_plane(struct drm_plane *plane)
>> kfree(intel_plane);
>> }
>>
>> +int intel_sprite_set_destkey(struct drm_device *dev, void *data,
>> + struct drm_file *file_priv)
>> +{
>> + struct drm_intel_set_sprite_destkey *set = data;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + struct drm_mode_object *obj;
>> + struct drm_plane *plane;
>> + struct intel_plane *intel_plane;
>> + int ret = 0;
>> +
>> + if (!dev_priv)
>> + return -EINVAL;
>> +
>> + if (set->value > 0xffffff)
>> + return -EINVAL;
>> +
>> + mutex_lock(&dev->mode_config.mutex);
>> +
>> + obj = drm_mode_object_find(dev, set->plane_id, DRM_MODE_OBJECT_PLANE);
>> + if (!obj) {
>> + ret = -EINVAL;
>> + goto out_unlock;
>> + }
>> +
>> + plane = obj_to_plane(obj);
>> + intel_plane = to_intel_plane(plane);
>> + ret = intel_plane->update_destkey(plane, set->value);
>> +
>> +out_unlock:
>> + mutex_unlock(&dev->mode_config.mutex);
>> + return ret;
>> +}
>> +
>> +int intel_sprite_get_destkey(struct drm_device *dev, void *data,
>> + struct drm_file *file_priv)
>> +{
>> + struct drm_intel_get_sprite_destkey *get = data;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + struct drm_mode_object *obj;
>> + struct drm_plane *plane;
>> + struct intel_plane *intel_plane;
>> + int ret = 0;
>> +
>> + if (!dev_priv)
>> + return -EINVAL;
>> +
>> + mutex_lock(&dev->mode_config.mutex);
>> +
>> + obj = drm_mode_object_find(dev, get->plane_id, DRM_MODE_OBJECT_PLANE);
>> + if (!obj) {
>> + ret = -EINVAL;
>> + goto out_unlock;
>> + }
>> +
>> + plane = obj_to_plane(obj);
>> + intel_plane = to_intel_plane(plane);
>> + get->value = intel_plane->get_destkey(plane);
>> +
>> +out_unlock:
>> + mutex_unlock(&dev->mode_config.mutex);
>> + return ret;
>> +}
>
> Frankly I'd prefer the driver code to simply keep track of the destkey
> instead of interogating the hw with the ->get_destkey call.
>
>> +
>> static const struct drm_plane_funcs intel_plane_funcs = {
>> .update_plane = intel_update_plane,
>> .disable_plane = intel_disable_plane,
>> @@ -411,10 +553,14 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe)
>> intel_plane->max_downscale = 16;
>> intel_plane->update_plane = snb_update_plane;
>> intel_plane->disable_plane = snb_disable_plane;
>> + intel_plane->update_destkey = snb_update_destkey;
>> + intel_plane->get_destkey = snb_get_destkey;
>> } else if (IS_GEN7(dev)) {
>> intel_plane->max_downscale = 2;
>> intel_plane->update_plane = ivb_update_plane;
>> intel_plane->disable_plane = ivb_disable_plane;
>> + intel_plane->update_destkey = ivb_update_destkey;
>> + intel_plane->get_destkey = ivb_get_destkey;
>> }
>>
>> intel_plane->pipe = pipe;
>> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
>> index 28c0d11..71674b3 100644
>> --- a/include/drm/i915_drm.h
>> +++ b/include/drm/i915_drm.h
>> @@ -198,6 +198,8 @@ typedef struct _drm_i915_sarea {
>> #define DRM_I915_OVERLAY_PUT_IMAGE 0x27
>> #define DRM_I915_OVERLAY_ATTRS 0x28
>> #define DRM_I915_GEM_EXECBUFFER2 0x29
>> +#define DRM_I915_GET_SPRITE_DESTKEY 0x2a
>> +#define DRM_I915_SET_SPRITE_DESTKEY 0x2b
>>
>> #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
>> #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
>> @@ -239,6 +241,8 @@ typedef struct _drm_i915_sarea {
>> #define DRM_IOCTL_I915_GEM_MADVISE DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MADVISE, struct drm_i915_gem_madvise)
>> #define DRM_IOCTL_I915_OVERLAY_PUT_IMAGE DRM_IOW(DRM_COMMAND_BASE + DRM_I915_OVERLAY_PUT_IMAGE, struct drm_intel_overlay_put_image)
>> #define DRM_IOCTL_I915_OVERLAY_ATTRS DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_OVERLAY_ATTRS, struct drm_intel_overlay_attrs)
>> +#define DRM_IOCTL_I915_SET_SPRITE_DESTKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_DESTKEY, struct drm_intel_set_sprite_destkey)
>> +#define DRM_IOCTL_I915_GET_SPRITE_DESTKEY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_SET_SPRITE_DESTKEY, struct drm_intel_get_sprite_destkey)
>>
>> /* Allow drivers to submit batchbuffers directly to hardware, relying
>> * on the security mechanisms provided by hardware.
>> @@ -844,4 +848,16 @@ struct drm_intel_overlay_attrs {
>> __u32 gamma5;
>> };
>>
>> +/* Set the destination color key on a given sprite */
>> +struct drm_intel_set_sprite_destkey {
>> + __u32 plane_id;
>> + __u32 value;
>> +};
>> +
>> +/* Get the current destination color key on a given sprite */
>> +struct drm_intel_get_sprite_destkey {
>> + __u32 plane_id;
>> + __u32 value;
>> +};
>> +
>> #endif /* _I915_DRM_H_ */
>> --
> --
> Daniel Vetter
> Mail: daniel@ffwll.ch
> Mobile: +41 (0)79 365 57 48
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 4/5] drm/i915: add destination color key support
2011-11-08 22:36 ` Alex Deucher
@ 2011-11-08 23:08 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-08 23:08 UTC (permalink / raw)
To: Alex Deucher; +Cc: dri-devel, intel-gfx, rob.clark
On Tue, Nov 08, 2011 at 05:36:11PM -0500, Alex Deucher wrote:
> On Tue, Nov 8, 2011 at 5:06 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, Nov 07, 2011 at 10:02:55AM -0800, Jesse Barnes wrote:
> >> Add new ioctls for getting and setting the current destination color
> >> key. This allows for simple overlay display control by matching a color
> >> key value in the primary plane before blending the overlay on top.
> >>
> >> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> >
> > A few comments below and a snide-y one here: If the goal is simply to keep
> > the bubble intact, this is fine by me. If we want something that has the
> > chance to be useable by a generic driver I think we want a drm interface
> > for plane/crtc properties (you could wire up set_gamma on planes in the
> > same series ... ;-) Again, I don't really care ...
>
> Even further off topic, the panel scaler support should ideally be a
> property of crtcs/planes rather than a connector property. However,
> that would be a bit of pain to implement and keep backwards
> compatibility.
I think we could just internally route the output pannel-fitter prop to
the crtc one. After all if the hw has the fitter really on the crtc,
moving it also logically to the crtc won't make anything previously
possibel impossible ...
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] drm/i915: add destination color key support
2011-11-08 22:06 ` Daniel Vetter
2011-11-08 22:36 ` Alex Deucher
@ 2011-11-08 22:40 ` Jesse Barnes
1 sibling, 0 replies; 17+ messages in thread
From: Jesse Barnes @ 2011-11-08 22:40 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel, rob.clark
[-- Attachment #1.1: Type: text/plain, Size: 1651 bytes --]
On Tue, 8 Nov 2011 23:06:57 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Nov 07, 2011 at 10:02:55AM -0800, Jesse Barnes wrote:
> > Add new ioctls for getting and setting the current destination color
> > key. This allows for simple overlay display control by matching a color
> > key value in the primary plane before blending the overlay on top.
> >
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>
> A few comments below and a snide-y one here: If the goal is simply to keep
> the bubble intact, this is fine by me. If we want something that has the
> chance to be useable by a generic driver I think we want a drm interface
> for plane/crtc properties (you could wire up set_gamma on planes in the
> same series ... ;-) Again, I don't really care ...
I'm ok with having generic properties for these. But I don't want to
go through the same 8 month process of getting buy-in from other
chipsets and people before the code is pushed as I've done with the
core DRM bits.
And since I know nothing of the color key support details for other
chipsets, I've simply made this one device specific. We'll have more
in the future too; current hardware can do source keying, and future
hardware will be able to blend 3 planes per pipe together using alpha
blending.
If some other hardware has similar features and someone steps up to do
the code, I'd be happy to work with them to get some generic interfaces
in. In the meantime though, I'd like to see some i915 interfaces so we
can actually use this stuff before the world ends in 2012.
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/5] drm/i915: track sprite coverage and disable primary plane if possible
2011-11-07 18:02 [PATCH] DRM planes Jesse Barnes
` (3 preceding siblings ...)
2011-11-07 18:02 ` [PATCH 4/5] drm/i915: add destination color key support Jesse Barnes
@ 2011-11-07 18:02 ` Jesse Barnes
2011-11-08 22:08 ` Daniel Vetter
4 siblings, 1 reply; 17+ messages in thread
From: Jesse Barnes @ 2011-11-07 18:02 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, rob.clark
To save power when the sprite is full screen, we can disable the primary
plane on the same pipe. Track the sprite status and enable/disable the
primary opportunistically.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_sprite.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b0081d2..ff553a6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -180,6 +180,7 @@ struct intel_plane {
struct drm_plane base;
enum pipe pipe;
struct drm_i915_gem_object *obj;
+ bool primary_disabled;
int max_downscale;
u32 lut_r[1024], lut_g[1024], lut_b[1024];
void (*update_plane)(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 1adeda8..e65498d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -395,9 +395,23 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
start = obj->gtt_offset;
+ /*
+ * Be sure to re-enable the primary before the sprite is no longer
+ * covering it fully.
+ */
+ if (!disable_primary && intel_plane->primary_disabled) {
+ //dev_priv->display.enable_primary(dev, crtc);
+ intel_plane->primary_disabled = false;
+ }
+
intel_plane->update_plane(plane, fb, start, crtc_x, crtc_y,
crtc_w, crtc_h, x, y, src_w, src_h);
+ if (disable_primary) {
+ //dev_priv->display.disable_primary(dev, crtc);
+ intel_plane->primary_disabled = true;
+ }
+
/* Unpin old obj after new one is active to avoid ugliness */
if (old_obj) {
/*
@@ -427,6 +441,11 @@ intel_disable_plane(struct drm_plane *plane)
mutex_lock(&dev->struct_mutex);
+ if (intel_plane->primary_disabled) {
+ //dev_priv->display.enable_primary(dev, crtc);
+ intel_plane->primary_disabled = false;
+ }
+
intel_plane->disable_plane(plane);
if (!intel_plane->obj)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 17+ messages in thread