diff mbox

[RFC,07/11] drm: add drm_plane_state

Message ID 1350089352-18162-8-git-send-email-rob.clark@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Rob Clark Oct. 13, 2012, 12:49 a.m. UTC
From: Rob Clark <rob@ti.com>

Break the mutable state of a plane out into a separate structure.
This makes it easier to have some helpers for plane->set_property()
and for checking for invalid params.  The idea is that individual
drivers can wrap the state struct in their own struct which adds
driver specific parameters, for easy build-up of state across
multiple set_property() calls and for easy atomic commit or roll-
back.

The same should be done for CRTC, encoder, and connector, but this
patch only includes the first part (plane).
---
 drivers/gpu/drm/drm_crtc.c           |  142 ++++++++++++++++++++++++++++++----
 drivers/staging/omapdrm/omap_crtc.c  |    4 +-
 drivers/staging/omapdrm/omap_drv.c   |    2 +-
 drivers/staging/omapdrm/omap_plane.c |   16 ++--
 include/drm/drm_crtc.h               |   52 +++++++++++--
 5 files changed, 181 insertions(+), 35 deletions(-)
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index da29732..63365f0 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -409,14 +409,14 @@  static void drm_framebuffer_remove_legacy(struct drm_framebuffer *fb)
 	}
 
 	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
-		if (plane->fb == fb) {
+		if (plane->state->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");
 			/* disconnect the plane from the fb and crtc: */
-			plane->fb = NULL;
-			plane->crtc = NULL;
+			plane->state->fb = NULL;
+			plane->state->crtc = NULL;
 		}
 	}
 
@@ -470,7 +470,7 @@  void drm_framebuffer_remove(struct drm_framebuffer *fb)
 	}
 
 	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
-		if (plane->fb == fb) {
+		if (plane->state->fb == fb) {
 			/* should turn off the crtc */
 			drm_mode_plane_set_obj_prop(plane, state, config->prop_crtc_id, 0);
 			drm_mode_plane_set_obj_prop(plane, state, config->prop_fb_id, 0);
@@ -743,12 +743,21 @@  int drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
 
 	mutex_lock(&dev->mode_config.mutex);
 
+	if (!dev_supports_atomic(dev)) {
+		plane->state = kzalloc(sizeof(*plane->state), GFP_KERNEL);
+		if (!plane->state) {
+			DRM_DEBUG_KMS("out of memory when allocating state\n");
+			ret = -ENOMEM;
+			goto out;
+		}
+	}
+
 	ret = drm_mode_object_get(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
 	if (ret)
 		goto out;
 
 	plane->base.properties = &plane->properties;
-	plane->base.propvals = &plane->propvals;
+	plane->base.propvals = &plane->state->propvals;
 	plane->dev = dev;
 	plane->funcs = funcs;
 	plane->format_types = kmalloc(sizeof(uint32_t) * format_count,
@@ -812,6 +821,110 @@  void drm_plane_cleanup(struct drm_plane *plane)
 }
 EXPORT_SYMBOL(drm_plane_cleanup);
 
+int drm_plane_check_state(struct drm_plane *plane,
+		struct drm_plane_state *state)
+{
+	unsigned int fb_width, fb_height;
+	struct drm_framebuffer *fb = state->fb;
+	int i;
+
+	/* disabling the plane is allowed: */
+	if (!fb)
+		return 0;
+
+	fb_width = fb->width << 16;
+	fb_height = fb->height << 16;
+
+	/* Check whether this plane supports the fb pixel format. */
+	for (i = 0; i < plane->format_count; i++)
+		if (fb->pixel_format == plane->format_types[i])
+			break;
+	if (i == plane->format_count) {
+		DRM_DEBUG_KMS("Invalid pixel format 0x%08x\n", fb->pixel_format);
+		return -EINVAL;
+	}
+
+	/* Make sure source coordinates are inside the fb. */
+	if (state->src_w > fb_width ||
+			state->src_x > fb_width - state->src_w ||
+			state->src_h > fb_height ||
+			state->src_y > fb_height - state->src_h) {
+		DRM_DEBUG_KMS("Invalid source coordinates "
+			      "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n",
+			      state->src_w >> 16,
+			      ((state->src_w & 0xffff) * 15625) >> 10,
+			      state->src_h >> 16,
+			      ((state->src_h & 0xffff) * 15625) >> 10,
+			      state->src_x >> 16,
+			      ((state->src_x & 0xffff) * 15625) >> 10,
+			      state->src_y >> 16,
+			      ((state->src_y & 0xffff) * 15625) >> 10);
+		return -ENOSPC;
+	}
+
+	/* Give drivers some help against integer overflows */
+	if (state->crtc_w > INT_MAX ||
+			state->crtc_x > INT_MAX - (int32_t) state->crtc_w ||
+			state->crtc_h > INT_MAX ||
+			state->crtc_y > INT_MAX - (int32_t) state->crtc_h) {
+		DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
+			      state->crtc_w, state->crtc_h,
+			      state->crtc_x, state->crtc_y);
+		return -ERANGE;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_check_state);
+
+void drm_plane_commit_state(struct drm_plane *plane,
+		struct drm_plane_state *state)
+{
+	plane->state = state;
+	plane->base.propvals = &state->propvals;
+}
+EXPORT_SYMBOL(drm_plane_commit_state);
+
+int drm_plane_set_property(struct drm_plane *plane,
+		struct drm_plane_state *state,
+		struct drm_property *property, uint64_t value)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	drm_object_property_set_value(&plane->base,
+			&state->propvals, property, value);
+
+	if (property == config->prop_fb_id) {
+		struct drm_mode_object *obj = drm_property_get_obj(property, value);
+		state->fb = obj ? obj_to_fb(obj) : NULL;
+	} else if (property == config->prop_crtc_id) {
+		struct drm_mode_object *obj = drm_property_get_obj(property, value);
+		state->crtc = obj ? obj_to_crtc(obj) : NULL;
+	} else if (property == config->prop_crtc_x) {
+		state->crtc_x = U642I64(value);
+	} else if (property == config->prop_crtc_y) {
+		state->crtc_y = U642I64(value);
+	} else if (property == config->prop_crtc_w) {
+		state->crtc_w = value;
+	} else if (property == config->prop_crtc_h) {
+		state->crtc_h = value;
+	} else if (property == config->prop_src_x) {
+		state->src_x = value;
+	} else if (property == config->prop_src_y) {
+		state->src_y = value;
+	} else if (property == config->prop_src_w) {
+		state->src_w = value;
+	} else if (property == config->prop_src_h) {
+		state->src_h = value;
+	} else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_set_property);
+
 /**
  * drm_mode_create - create a new display mode
  * @dev: DRM device
@@ -1857,13 +1970,13 @@  int drm_mode_getplane(struct drm_device *dev, void *data,
 	}
 	plane = obj_to_plane(obj);
 
-	if (plane->crtc)
-		plane_resp->crtc_id = plane->crtc->base.id;
+	if (plane->state->crtc)
+		plane_resp->crtc_id = plane->state->crtc->base.id;
 	else
 		plane_resp->crtc_id = 0;
 
-	if (plane->fb)
-		plane_resp->fb_id = plane->fb->base.id;
+	if (plane->state->fb)
+		plane_resp->fb_id = plane->state->fb->base.id;
 	else
 		plane_resp->fb_id = 0;
 
@@ -1940,8 +2053,8 @@  static int drm_mode_setplane_legacy(struct drm_device *dev, void *data,
 	/* No fb means shut it down */
 	if (!plane_req->fb_id) {
 		plane->funcs->disable_plane(plane);
-		plane->crtc = NULL;
-		plane->fb = NULL;
+		plane->state->crtc = NULL;
+		plane->state->fb = NULL;
 		goto out;
 	}
 
@@ -2015,8 +2128,8 @@  static int drm_mode_setplane_legacy(struct drm_device *dev, void *data,
 					 plane_req->src_x, plane_req->src_y,
 					 plane_req->src_w, plane_req->src_h);
 	if (!ret) {
-		plane->crtc = crtc;
-		plane->fb = fb;
+		plane->state->crtc = crtc;
+		plane->state->fb = fb;
 	}
 
 out:
@@ -3541,9 +3654,6 @@  int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
 
 	if (plane->funcs->set_property)
 		ret = plane->funcs->set_property(plane, state, property, value);
-	if (!ret)
-		drm_object_property_set_value(&plane->base, &plane->propvals,
-				property, value);
 
 	return ret;
 }
diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c
index eddbb2f..1236d21 100644
--- a/drivers/staging/omapdrm/omap_crtc.c
+++ b/drivers/staging/omapdrm/omap_crtc.c
@@ -168,7 +168,7 @@  static void omap_crtc_dpms(struct drm_crtc *crtc, int mode)
 		/* and any attached overlay planes: */
 		for (i = 0; i < priv->num_planes; i++) {
 			struct drm_plane *plane = priv->planes[i];
-			if (plane->crtc == crtc)
+			if (plane->state->crtc == crtc)
 				WARN_ON(omap_plane_dpms(plane, mode));
 		}
 	}
@@ -605,7 +605,7 @@  struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 
 	omap_crtc->channel = channel;
 	omap_crtc->plane = plane;
-	omap_crtc->plane->crtc = crtc;
+	omap_crtc->plane->state->crtc = crtc;
 	omap_crtc->name = channel_names[channel];
 	omap_crtc->pipe = id;
 
diff --git a/drivers/staging/omapdrm/omap_drv.c b/drivers/staging/omapdrm/omap_drv.c
index 5f5ee84..7321510 100644
--- a/drivers/staging/omapdrm/omap_drv.c
+++ b/drivers/staging/omapdrm/omap_drv.c
@@ -528,7 +528,7 @@  static void dev_lastclose(struct drm_device *dev)
 	for (i = 0; i < priv->num_planes; i++) {
 		struct drm_plane *plane = priv->planes[i];
 		drm_object_property_set_value(&plane->base,
-				&plane->propvals, priv->rotation_prop, 0);
+				&plane->state->propvals, priv->rotation_prop, 0);
 	}
 
 	mutex_lock(&dev->mode_config.mutex);
diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c
index 854fdce..cb5d427 100644
--- a/drivers/staging/omapdrm/omap_plane.c
+++ b/drivers/staging/omapdrm/omap_plane.c
@@ -122,7 +122,7 @@  static void omap_plane_pre_apply(struct omap_drm_apply *apply)
 	struct drm_plane *plane = &omap_plane->base;
 	struct drm_device *dev = plane->dev;
 	struct omap_overlay_info *info = &omap_plane->info;
-	struct drm_crtc *crtc = plane->crtc;
+	struct drm_crtc *crtc = plane->state->crtc;
 	enum omap_channel channel;
 	bool enabled = omap_plane->enabled && crtc;
 	bool ilace, replication;
@@ -131,7 +131,7 @@  static void omap_plane_pre_apply(struct omap_drm_apply *apply)
 	DBG("%s, enabled=%d", omap_plane->name, enabled);
 
 	/* if fb has changed, pin new fb: */
-	update_pin(plane, enabled ? plane->fb : NULL);
+	update_pin(plane, enabled ? plane->state->fb : NULL);
 
 	if (!enabled) {
 		dispc_ovl_enable(omap_plane->id, false);
@@ -141,7 +141,7 @@  static void omap_plane_pre_apply(struct omap_drm_apply *apply)
 	channel = omap_crtc_channel(crtc);
 
 	/* update scanout: */
-	omap_framebuffer_update_scanout(plane->fb, win, info);
+	omap_framebuffer_update_scanout(plane->state->fb, win, info);
 
 	DBG("%dx%d -> %dx%d (%d)", info->width, info->height,
 			info->out_width, info->out_height,
@@ -186,16 +186,16 @@  static void omap_plane_post_apply(struct omap_drm_apply *apply)
 		cb.fxn(cb.arg);
 
 	if (omap_plane->enabled) {
-		omap_framebuffer_flush(plane->fb, info->pos_x, info->pos_y,
+		omap_framebuffer_flush(plane->state->fb, info->pos_x, info->pos_y,
 				info->out_width, info->out_height);
 	}
 }
 
 static int apply(struct drm_plane *plane)
 {
-	if (plane->crtc) {
+	if (plane->state->crtc) {
 		struct omap_plane *omap_plane = to_omap_plane(plane);
-		return omap_crtc_apply(plane->crtc, &omap_plane->apply);
+		return omap_crtc_apply(plane->state->crtc, &omap_plane->apply);
 	}
 	return 0;
 }
@@ -232,8 +232,8 @@  int omap_plane_mode_set(struct drm_plane *plane,
 		omap_plane->apply_done_cb.arg = arg;
 	}
 
-	plane->fb = fb;
-	plane->crtc = crtc;
+	plane->state->fb = fb;
+	plane->state->crtc = crtc;
 
 	return apply(plane);
 }
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 514b3f4..ef558dd 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -648,6 +648,37 @@  struct drm_plane_funcs {
 };
 
 /**
+ * drm_plane_state - mutable plane state
+ * @crtc: currently bound CRTC
+ * @fb: currently bound fb
+ * @crtc_x: left position of visible portion of plane on crtc
+ * @crtc_y: upper position of visible portion of plane on crtc
+ * @crtc_w: width of visible portion of plane on crtc
+ * @crtc_h: height of visible portion of plane on crtc
+ * @src_x: left position of visible portion of plane within
+ *   plane (in 16.16)
+ * @src_y: upper position of visible portion of plane within
+ *   plane (in 16.16)
+ * @src_w: width of visible portion of plane (in 16.16)
+ * @src_h: height of visible portion of plane (in 16.16)
+ * @propvals: property values
+ */
+struct drm_plane_state {
+	struct drm_crtc *crtc;
+	struct drm_framebuffer *fb;
+
+	/* Signed dest location allows it to be partially off screen */
+	int32_t crtc_x, crtc_y;
+	uint32_t crtc_w, crtc_h;
+
+	/* Source values are 16.16 fixed point */
+	uint32_t src_x, src_y;
+	uint32_t src_h, src_w;
+
+	struct drm_object_property_values propvals;
+};
+
+/**
  * drm_plane - central DRM plane control structure
  * @dev: DRM device this plane belongs to
  * @head: for list management
@@ -655,11 +686,9 @@  struct drm_plane_funcs {
  * @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
+ * @state: the mutable state
  * @gamma_size: size of gamma table
  * @gamma_store: gamma correction table
- * @enabled: enabled flag
  * @funcs: helper functions
  * @helper_private: storage for drver layer
  * @properties: property tracking for this plane
@@ -674,20 +703,20 @@  struct drm_plane {
 	uint32_t *format_types;
 	uint32_t format_count;
 
-	struct drm_crtc *crtc;
-	struct drm_framebuffer *fb;
+	/*
+	 * State that can be updated from userspace, and atomically
+	 * commited or rolled back:
+	 */
+	struct drm_plane_state *state;
 
 	/* 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_object_properties properties;
-	struct drm_object_property_values propvals;
 };
 
 /**
@@ -895,6 +924,13 @@  extern int drm_plane_init(struct drm_device *dev,
 			  const uint32_t *formats, uint32_t format_count,
 			  bool priv);
 extern void drm_plane_cleanup(struct drm_plane *plane);
+extern int drm_plane_check_state(struct drm_plane *plane,
+		struct drm_plane_state *state);
+extern void drm_plane_commit_state(struct drm_plane *plane,
+		struct drm_plane_state *state);
+extern int drm_plane_set_property(struct drm_plane *plane,
+		struct drm_plane_state *state,
+		struct drm_property *property, uint64_t value);
 
 extern void drm_encoder_cleanup(struct drm_encoder *encoder);