From patchwork Sat Oct 13 00:49:08 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Clark X-Patchwork-Id: 12202 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 911D823EFB for ; Sat, 13 Oct 2012 00:49:37 +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 2A37EA180EC for ; Sat, 13 Oct 2012 00:49:37 +0000 (UTC) Received: by mail-ie0-f180.google.com with SMTP id e10so5509064iej.11 for ; Fri, 12 Oct 2012 17:49:37 -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=FYEh3z2M9A7IzQe/AZNArq+Z1bprcfQP3ApcvlkrJyM=; b=WpNQB0biNcNjrKxZpMG8HgGtkibHxbM3GhdzOrtAv0pqnY7u00EHiMGjD8J9oFCvuK TAdoRlaAcw4dkPofpllZwU9nQDmqP8aWERvOZTKLLsSd0pciuh3eYvuC/3VkdsPFm0Y+ uWdGhriFFCXx/kMxrcDxfkJSNXw5eeo2Ulj972ycXiJ8QG1vxJ5f9H/wL3cSwDtRIj50 9TFjpp4Ab0CqzEzU61rpbv0BosDqb6gjO/qtbo2ksfasBJUzWund6dfn6wjcYyL48qZg FT84kJIfmzc7V8g8IflntZjxFYjcqiUAnafd1rA4trzizycboA+KU6VhN+uuf0RkXkGp hCsg== Received: by 10.42.145.66 with SMTP id e2mr4651048icv.18.1350089376923; Fri, 12 Oct 2012 17:49:36 -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.67.148 with SMTP id n20csp274646igt; Fri, 12 Oct 2012 17:49:36 -0700 (PDT) Received: by 10.60.27.71 with SMTP id r7mr4844196oeg.96.1350089376409; Fri, 12 Oct 2012 17:49:36 -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 d7si101858oea.37.2012.10.12.17.49.36 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 12 Oct 2012 17:49:36 -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 tb18so4158660obb.37 for ; Fri, 12 Oct 2012 17:49:36 -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=FYEh3z2M9A7IzQe/AZNArq+Z1bprcfQP3ApcvlkrJyM=; b=ubj6Fydv5ppXHBRvv95x/07zjOU0khxn7PaljY1ZgPTBpaTHdb2ennZOfTIv+qEDCK C4bmU7R4ncjgyedGu5COx8qgcZOdPHY1PMFR3rwU6irszAw9I2PESztAFvYaZ6qrn+hL EwHzmNP/pHPAKyJlbFYaMYdB8Qq89P28KrNGKstf/EIxB3gQGr/gjsc/YyC20aCghdn8 tZA1ZIsiUwxr0EkWPeS34BfkfNr353YzLRwNo+olWuXp2iMBluyICGcmPNLFwLlHnBJP D3L9L4flkhumics0LziSnz29s4D3/au8BK/NM+IKSe/djH11d8vjs4pruD83BrDr+xwp v8xg== Received: by 10.182.69.36 with SMTP id b4mr4724152obu.96.1350089376278; Fri, 12 Oct 2012 17:49:36 -0700 (PDT) Received: from localhost (ppp-70-129-143-201.dsl.rcsntx.swbell.net. [70.129.143.201]) by mx.google.com with ESMTPS id q1sm6443595oeg.3.2012.10.12.17.49.34 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 12 Oct 2012 17:49:35 -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, krh@bitplanet.net, Rob Clark Subject: [RFC 07/11] drm: add drm_plane_state Date: Fri, 12 Oct 2012 19:49:08 -0500 Message-Id: <1350089352-18162-8-git-send-email-rob.clark@linaro.org> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1350089352-18162-1-git-send-email-rob.clark@linaro.org> References: <1350089352-18162-1-git-send-email-rob.clark@linaro.org> X-Gm-Message-State: ALoCoQlIXhOawYO3h2bahZUSqkyeuCQWt2QPpVVYvuof8pGxMFOGP19NtH80T9w01YomRL3dzoBW From: Rob Clark 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 --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);