From patchwork Mon Sep 10 03:03:17 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Clark X-Patchwork-Id: 11261 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 3124823E00 for ; Mon, 10 Sep 2012 03:05:05 +0000 (UTC) Received: from mail-ie0-f180.google.com (mail-ie0-f180.google.com [209.85.223.180]) by fiordland.canonical.com (Postfix) with ESMTP id AF076A18030 for ; Mon, 10 Sep 2012 03:05:04 +0000 (UTC) Received: by mail-ie0-f180.google.com with SMTP id k11so2235201iea.11 for ; Sun, 09 Sep 2012 20:05:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf :dkim-signature:sender:from:to:cc:subject:date:message-id:x-mailer :in-reply-to:references:x-gm-message-state; bh=3V9OIbTSPFnvR9Zt7K39YibcvpBWqewy1BiGAuxnomE=; b=n1+MFC76yTfDIh6y14J4i0slFQmEbEQeiTEBhlKL+rhvG5AN3QMV79B3qyd1Af7bBI 7W+YzFTrw+RpwfrEpVj0nY1XSm2mJR4p3jB1rH1yMidvYv1+lyrWDTdJDm3ilPVONOKu neri+ZkbaQHgk6ke3hOfpvNfrtMcsgU0ELbuH9XkR/1CHmEqrpVuHwEHeqK5GtCYZWvu L8jjpTA/ZUGEDeln1SQuBWeK70StHRLSbDvptwVUtgo7K3sGjNz2ty/cie6CISp1/XJ4 bjDVQ8MaJJcZtc3tdx40YxJJH9TJNjbEiURJ2CmXrzpq69fNlgdlvrJu7dd06+hizRyk LggQ== Received: by 10.50.242.3 with SMTP id wm3mr8274625igc.0.1347246304447; Sun, 09 Sep 2012 20:05:04 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.50.184.232 with SMTP id ex8csp70790igc; Sun, 9 Sep 2012 20:05:04 -0700 (PDT) Received: by 10.60.29.165 with SMTP id l5mr12921017oeh.105.1347246303924; Sun, 09 Sep 2012 20:05:03 -0700 (PDT) Received: from mail-ob0-f178.google.com (mail-ob0-f178.google.com [209.85.214.178]) by mx.google.com with ESMTPS id is1si7284901obc.87.2012.09.09.20.05.02 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 09 Sep 2012 20:05:02 -0700 (PDT) Received-SPF: pass (google.com: domain of robdclark@gmail.com designates 209.85.214.178 as permitted sender) client-ip=209.85.214.178; Authentication-Results: mx.google.com; spf=pass (google.com: domain of robdclark@gmail.com designates 209.85.214.178 as permitted sender) smtp.mail=robdclark@gmail.com; dkim=pass header.i=@gmail.com Received: by mail-ob0-f178.google.com with SMTP id wd20so2865791obb.37 for ; Sun, 09 Sep 2012 20:05:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=3V9OIbTSPFnvR9Zt7K39YibcvpBWqewy1BiGAuxnomE=; b=JV/AvF2Z5OcB3VvXnfcCcYPaRNSm/H/lKb649XoWGHfXqM/250k315hJh0wQvNn/Is ZKuUv3s+U34+NhBhteyZQLQO97WTNG/QwBepGvatBQMNsSc+htCv7WoCz1dzfO9BSZdg 3T8/LbYyV3HJx/3LLAdq8GuTlKQwwyYoizDHvS5Wzefd6RLzFq8FUYab+To2RHgrOvDS 3tz3gRG8WjygqZlxwiecUaBBRIkfLbhzeEp53eAXTnSi7w+8sSTwPV43HBuY5gTyb6jm ewFzq9gNvNhdLHQUmeMtlIa2dC9EtSXNntYFyeynerk/lcXw80zVyyrM0udsJDIDByJQ vsEg== Received: by 10.182.1.41 with SMTP id 9mr12944989obj.35.1347246302793; Sun, 09 Sep 2012 20:05:02 -0700 (PDT) Received: from localhost (ppp-70-129-131-42.dsl.rcsntx.swbell.net. [70.129.131.42]) by mx.google.com with ESMTPS id ql3sm12210181obc.17.2012.09.09.20.05.01 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 09 Sep 2012 20:05:02 -0700 (PDT) Sender: Rob Clark From: Rob Clark To: dri-devel@lists.freedesktop.org Cc: patches@linaro.org, ville.syrjala@linux.intel.com, jbarnes@virtuousgeek.org, daniel.vetter@ffwll.ch, Rob Clark Subject: [RFC 4/9] drm: convert plane to properties Date: Sun, 9 Sep 2012 22:03:17 -0500 Message-Id: <1347246202-24249-5-git-send-email-rob.clark@linaro.org> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1347246202-24249-1-git-send-email-rob.clark@linaro.org> References: <1347246202-24249-1-git-send-email-rob.clark@linaro.org> X-Gm-Message-State: ALoCoQk8wBCTqcz6zt/779VXRHvx3e1f+0cuwP+YQBptzxEantKsyPQTilv0f0QW62VhaABHIqS8 From: Rob Clark 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);