From: Rob Clark <r...@ti.com>

Use atomic properties mechanism to set plane attributes.  This by
itself doesn't accomplish anything, but it avoids having multiple
code paths to do the same thing when nuclear-pageflip and atomic-
modeset are introduced.
---
 drivers/gpu/drm/drm_crtc.c |  270 ++++++++++++++++++++++++++------------------
 include/drm/drm_crtc.h     |   32 ++++--
 2 files changed, 187 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 12829de..e680fbe 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -38,6 +38,10 @@
 #include "drm_edid.h"
 #include "drm_fourcc.h"
 
+static int drm_mode_set_obj_prop(struct drm_device *dev,
+               struct drm_mode_object *obj, void *state,
+               struct drm_property *property, uint64_t value);
+
 /* Avoid boilerplate.  I'm tired of typing. */
 #define DRM_ENUM_NAME_FN(fnname, list)                         \
        char *fnname(int val)                                   \
@@ -380,11 +384,19 @@ EXPORT_SYMBOL(drm_framebuffer_cleanup);
 void drm_framebuffer_remove(struct drm_framebuffer *fb)
 {
        struct drm_device *dev = fb->dev;
+       struct drm_mode_config *config = &dev->mode_config;
        struct drm_crtc *crtc;
        struct drm_plane *plane;
        struct drm_mode_set set;
+       void *state;
        int ret;
 
+       state = dev->driver->atomic_begin(dev);
+       if (IS_ERR(state)) {
+               DRM_ERROR("failed to disable crtc and/or plane when fb was 
deleted\n");
+               return;
+       }
+
        /* remove from any CRTC */
        list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
                if (crtc->fb == fb) {
@@ -401,15 +413,19 @@ void drm_framebuffer_remove(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");
-                       /* disconnect the plane from the fb and crtc: */
-                       plane->fb = NULL;
-                       plane->crtc = NULL;
+                       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);
                }
        }
 
+       /* just disabling stuff shouldn't fail, hopefully: */
+       if(dev->driver->atomic_check(dev, state))
+               DRM_ERROR("failed to disable crtc and/or plane when fb was 
deleted\n");
+       else
+               dev->driver->atomic_commit(dev, state, NULL);
+
+       dev->driver->atomic_end(dev, state);
+
        list_del(&fb->filp_head);
 
        drm_framebuffer_unreference(fb);
@@ -661,6 +677,7 @@ int drm_plane_init(struct drm_device *dev, struct drm_plane 
*plane,
                   const uint32_t *formats, uint32_t format_count,
                   bool priv)
 {
+       struct drm_mode_config *config = &dev->mode_config;
        int ret;
 
        mutex_lock(&dev->mode_config.mutex);
@@ -696,6 +713,17 @@ int drm_plane_init(struct drm_device *dev, struct 
drm_plane *plane,
                INIT_LIST_HEAD(&plane->head);
        }
 
+       drm_object_attach_property(&plane->base, config->prop_fb_id, 0);
+       drm_object_attach_property(&plane->base, config->prop_crtc_id, 0);
+       drm_object_attach_property(&plane->base, config->prop_crtc_x, 0);
+       drm_object_attach_property(&plane->base, config->prop_crtc_y, 0);
+       drm_object_attach_property(&plane->base, config->prop_crtc_w, 0);
+       drm_object_attach_property(&plane->base, config->prop_crtc_h, 0);
+       drm_object_attach_property(&plane->base, config->prop_src_x, 0);
+       drm_object_attach_property(&plane->base, config->prop_src_y, 0);
+       drm_object_attach_property(&plane->base, config->prop_src_w, 0);
+       drm_object_attach_property(&plane->base, config->prop_src_h, 0);
+
  out:
        mutex_unlock(&dev->mode_config.mutex);
 
@@ -769,23 +797,89 @@ void drm_mode_destroy(struct drm_device *dev, struct 
drm_display_mode *mode)
 }
 EXPORT_SYMBOL(drm_mode_destroy);
 
-static int drm_mode_create_standard_connector_properties(struct drm_device 
*dev)
+static int drm_mode_create_standard_properties(struct drm_device *dev)
 {
-       struct drm_property *edid;
-       struct drm_property *dpms;
+       struct drm_property *prop;
 
        /*
         * Standard properties (apply to all connectors)
         */
-       edid = drm_property_create(dev, DRM_MODE_PROP_BLOB |
+       prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
                                   DRM_MODE_PROP_IMMUTABLE,
                                   "EDID", 0);
-       dev->mode_config.edid_property = edid;
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.edid_property = prop;
 
-       dpms = drm_property_create_enum(dev, 0,
+       prop = drm_property_create_enum(dev, 0,
                                   "DPMS", drm_dpms_enum_list,
                                   ARRAY_SIZE(drm_dpms_enum_list));
-       dev->mode_config.dpms_property = dpms;
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.dpms_property = prop;
+
+
+       /* TODO we need the driver to control which of these are dynamic
+        * and which are not..  or maybe we should just set all to zero
+        * and let the individual drivers frob the prop->flags for the
+        * properties they can support dynamic changes on..
+        */
+
+       prop = drm_property_create_range(dev, DRM_MODE_PROP_DYNAMIC,
+                       "src_x", 0, UINT_MAX);
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.prop_src_x = prop;
+
+       prop = drm_property_create_range(dev, DRM_MODE_PROP_DYNAMIC,
+                       "src_y", 0, UINT_MAX);
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.prop_src_y = prop;
+
+       prop = drm_property_create_range(dev, 0, "src_w", 0, UINT_MAX);
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.prop_src_w = prop;
+
+       prop = drm_property_create_range(dev, 0, "src_h", 0, UINT_MAX);
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.prop_src_h = prop;
+
+       prop = drm_property_create_range(dev, DRM_MODE_PROP_DYNAMIC,
+                       "crtc_x", INT_MIN, INT_MAX);
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.prop_crtc_x = prop;
+
+       prop = drm_property_create_range(dev, DRM_MODE_PROP_DYNAMIC,
+                       "crtc_y", INT_MIN, INT_MAX);
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.prop_crtc_y = prop;
+
+       prop = drm_property_create_range(dev, 0, "crtc_w", 0, INT_MAX);
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.prop_crtc_w = prop;
+
+       prop = drm_property_create_range(dev, 0, "crtc_h", 0, INT_MAX);
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.prop_crtc_h = prop;
+
+       prop = drm_property_create_object(dev, DRM_MODE_PROP_DYNAMIC,
+                       "fb", DRM_MODE_OBJECT_FB);
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.prop_fb_id = prop;
+
+       prop = drm_property_create_object(dev, 0,
+                       "crtc", DRM_MODE_OBJECT_CRTC);
+       if (!prop)
+               return -ENOMEM;
+       dev->mode_config.prop_crtc_id = prop;
 
        return 0;
 }
@@ -1000,7 +1094,7 @@ void drm_mode_config_init(struct drm_device *dev)
        idr_init(&dev->mode_config.crtc_idr);
 
        mutex_lock(&dev->mode_config.mutex);
-       drm_mode_create_standard_connector_properties(dev);
+       drm_mode_create_standard_properties(dev);
        mutex_unlock(&dev->mode_config.mutex);
 
        /* Just to be sure */
@@ -1747,23 +1841,22 @@ 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_config *config = &dev->mode_config;
        struct drm_mode_object *obj;
-       struct drm_plane *plane;
-       struct drm_crtc *crtc;
-       struct drm_framebuffer *fb;
+       void *state;
        int ret = 0;
-       unsigned int fb_width, fb_height;
-       int i;
 
        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.
-        */
+       state = dev->driver->atomic_begin(dev);
+       if (IS_ERR(state)) {
+               ret = PTR_ERR(state);
+               goto out_unlock;
+       }
+
        obj = drm_mode_object_find(dev, plane_req->plane_id,
                                   DRM_MODE_OBJECT_PLANE);
        if (!obj) {
@@ -1772,91 +1865,38 @@ int drm_mode_setplane(struct drm_device *dev, void 
*data,
                ret = -ENOENT;
                goto out;
        }
-       plane = obj_to_plane(obj);
-
-       /* No fb means shut it down */
-       if (!plane_req->fb_id) {
-               plane->funcs->disable_plane(plane);
-               plane->crtc = NULL;
-               plane->fb = NULL;
-               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 = -ENOENT;
-               goto out;
-       }
-       crtc = obj_to_crtc(obj);
+       ret =
+               drm_mode_set_obj_prop(dev, obj, state,
+                               config->prop_crtc_id, plane_req->crtc_id) ||
+               drm_mode_set_obj_prop(dev, obj, state,
+                               config->prop_fb_id, plane_req->fb_id) ||
+               drm_mode_set_obj_prop(dev, obj, state,
+                               config->prop_crtc_x, *(uint32_t 
*)&plane_req->crtc_x) ||
+               drm_mode_set_obj_prop(dev, obj, state,
+                               config->prop_crtc_y, *(uint32_t 
*)&plane_req->crtc_y) ||
+               drm_mode_set_obj_prop(dev, obj, state,
+                               config->prop_crtc_w, plane_req->crtc_w) ||
+               drm_mode_set_obj_prop(dev, obj, state,
+                               config->prop_crtc_h, plane_req->crtc_h) ||
+               drm_mode_set_obj_prop(dev, obj, state,
+                               config->prop_src_w, plane_req->src_w) ||
+               drm_mode_set_obj_prop(dev, obj, state,
+                               config->prop_src_h, plane_req->src_h) ||
+               drm_mode_set_obj_prop(dev, obj, state,
+                               config->prop_src_x, plane_req->src_x) ||
+               drm_mode_set_obj_prop(dev, obj, state,
+                               config->prop_src_y, plane_req->src_y) ||
+               dev->driver->atomic_check(dev, state);
 
-       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 = -ENOENT;
-               goto out;
-       }
-       fb = obj_to_fb(obj);
-
-       /* 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);
-               ret = -EINVAL;
-               goto out;
-       }
-
-       fb_width = fb->width << 16;
-       fb_height = fb->height << 16;
-
-       /* Make sure source coordinates are inside the fb. */
-       if (plane_req->src_w > fb_width ||
-           plane_req->src_x > fb_width - plane_req->src_w ||
-           plane_req->src_h > fb_height ||
-           plane_req->src_y > fb_height - plane_req->src_h) {
-               DRM_DEBUG_KMS("Invalid source coordinates "
-                             "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n",
-                             plane_req->src_w >> 16,
-                             ((plane_req->src_w & 0xffff) * 15625) >> 10,
-                             plane_req->src_h >> 16,
-                             ((plane_req->src_h & 0xffff) * 15625) >> 10,
-                             plane_req->src_x >> 16,
-                             ((plane_req->src_x & 0xffff) * 15625) >> 10,
-                             plane_req->src_y >> 16,
-                             ((plane_req->src_y & 0xffff) * 15625) >> 10);
-               ret = -ENOSPC;
-               goto out;
-       }
-
-       /* Give drivers some help against integer overflows */
-       if (plane_req->crtc_w > INT_MAX ||
-           plane_req->crtc_x > INT_MAX - (int32_t) plane_req->crtc_w ||
-           plane_req->crtc_h > INT_MAX ||
-           plane_req->crtc_y > INT_MAX - (int32_t) plane_req->crtc_h) {
-               DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
-                             plane_req->crtc_w, plane_req->crtc_h,
-                             plane_req->crtc_x, plane_req->crtc_y);
-               ret = -ERANGE;
+       if (ret)
                goto out;
-       }
 
-       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_w, plane_req->src_h);
-       if (!ret) {
-               plane->crtc = crtc;
-               plane->fb = fb;
-       }
+       ret = dev->driver->atomic_commit(dev, state, NULL);
 
 out:
+       dev->driver->atomic_end(dev, state);
+out_unlock:
        mutex_unlock(&dev->mode_config.mutex);
 
        return ret;
@@ -3247,7 +3287,7 @@ int drm_mode_connector_property_set_ioctl(struct 
drm_device *dev,
        return drm_mode_obj_set_property_ioctl(dev, &obj_set_prop, file_priv);
 }
 
-static int drm_mode_connector_set_obj_prop(struct drm_connector *connector,
+int drm_mode_connector_set_obj_prop(struct drm_connector *connector,
                                           void *state, struct drm_property 
*property,
                                           uint64_t value)
 {
@@ -3266,8 +3306,9 @@ static int drm_mode_connector_set_obj_prop(struct 
drm_connector *connector,
                drm_connector_property_set_value(connector, property, value);
        return ret;
 }
+EXPORT_SYMBOL(drm_mode_connector_set_obj_prop);
 
-static int drm_mode_crtc_set_obj_prop(struct drm_crtc *crtc,
+int drm_mode_crtc_set_obj_prop(struct drm_crtc *crtc,
                                      void *state, struct drm_property 
*property,
                                      uint64_t value)
 {
@@ -3280,8 +3321,9 @@ static int drm_mode_crtc_set_obj_prop(struct drm_crtc 
*crtc,
 
        return ret;
 }
+EXPORT_SYMBOL(drm_mode_crtc_set_obj_prop);
 
-static int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
+int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
                                      void *state, struct drm_property 
*property,
                                      uint64_t value)
 {
@@ -3294,19 +3336,33 @@ static int drm_mode_plane_set_obj_prop(struct drm_plane 
*plane,
 
        return ret;
 }
+EXPORT_SYMBOL(drm_mode_plane_set_obj_prop);
 
 static int drm_mode_set_obj_prop(struct drm_device *dev,
                struct drm_mode_object *obj, void *state,
                struct drm_property *property, uint64_t value)
 {
-       if (drm_property_change_is_valid(property, value)) {
+if (!drm_property_change_is_valid(property, value)) {
+       DRM_DEBUG("invalid value: %s = %llx\n", property->name, value);
+}
+// TODO signed properties don't really work out so well, thanks
+// to uint64_t stuff everywhere.. INT_MIN for crtc_x/crtc_y end
+// up being a big positive #..
+       if (true || drm_property_change_is_valid(property, value)) {
                /* a zero value for an object property translates to null: */
                if ((property->flags & DRM_MODE_PROP_OBJECT) && value) {
                        struct drm_mode_object *obj =
                                        drm_mode_object_find(dev, value, 
property->values[0]);
                        if (!obj)
                                return -EINVAL;
-                       value = VOID2U64(obj);
+                       switch (obj->type) {
+                       case DRM_MODE_OBJECT_CRTC:
+                               value = VOID2U64(obj_to_crtc(obj));
+                               break;
+                       case DRM_MODE_OBJECT_FB:
+                               value = VOID2U64(obj_to_fb(obj));
+                               break;
+                       }
                }
                switch (obj->type) {
                case DRM_MODE_OBJECT_CONNECTOR:
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 92df2f4..546026c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -621,13 +621,6 @@ struct drm_connector {
  * @set_property: called when a property is changed
  */
 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);
 
        int (*set_property)(struct drm_plane *plane, void *state,
@@ -796,8 +789,20 @@ struct drm_mode_config {
        bool poll_enabled;
        struct delayed_work output_poll_work;
 
-       /* pointers to standard properties */
+       /* just so blob properties can always be in a list: */
        struct list_head property_blob_list;
+
+       /* pointers to standard properties */
+       struct drm_property *prop_src_x;
+       struct drm_property *prop_src_y;
+       struct drm_property *prop_src_w;
+       struct drm_property *prop_src_h;
+       struct drm_property *prop_crtc_x;
+       struct drm_property *prop_crtc_y;
+       struct drm_property *prop_crtc_w;
+       struct drm_property *prop_crtc_h;
+       struct drm_property *prop_fb_id;
+       struct drm_property *prop_crtc_id;
        struct drm_property *edid_property;
        struct drm_property *dpms_property;
 
@@ -932,6 +937,17 @@ extern int drm_object_property_set_value(struct 
drm_mode_object *obj,
 extern int drm_object_property_get_value(struct drm_mode_object *obj,
                                         struct drm_property *property,
                                         uint64_t *value);
+
+int drm_mode_connector_set_obj_prop(struct drm_connector *connector,
+                                          void *state, struct drm_property 
*property,
+                                          uint64_t value);
+int drm_mode_crtc_set_obj_prop(struct drm_crtc *crtc,
+                                     void *state, struct drm_property 
*property,
+                                     uint64_t value);
+int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
+                                     void *state, struct drm_property 
*property,
+                                     uint64_t value);
+
 extern int drm_framebuffer_init(struct drm_device *dev,
                                struct drm_framebuffer *fb,
                                const struct drm_framebuffer_funcs *funcs);
-- 
1.7.9.5

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

Reply via email to