* [PATCH 1/2] drm: Refactor framebuffer creation to allow internal use @ 2014-05-16 22:36 Matt Roper 2014-05-16 22:36 ` [PATCH 2/2] drm: Refactor setplane " Matt Roper 2014-05-19 7:17 ` [PATCH 1/2] drm: Refactor framebuffer creation " Daniel Vetter 0 siblings, 2 replies; 4+ messages in thread From: Matt Roper @ 2014-05-16 22:36 UTC (permalink / raw) To: intel-gfx, dri-devel Refactor DRM framebuffer creation into a new function that returns a struct drm_framebuffer directly. The upcoming universal cursor support will want to create framebuffers internally to wrap cursor buffers, so we want to be able to share that framebuffer creation with the drm_mode_addfb2 ioctl handler. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/drm_crtc.c | 64 +++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index b6d6c04..1a1a5f4 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2796,56 +2796,39 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) return 0; } -/** - * drm_mode_addfb2 - add an FB to the graphics configuration - * @dev: drm device for the ioctl - * @data: data pointer for the ioctl - * @file_priv: drm file for the ioctl call - * - * Add a new FB to the specified CRTC, given a user request with format. This is - * the 2nd version of the addfb ioctl, which supports multi-planar framebuffers - * and uses fourcc codes as pixel format specifiers. - * - * 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) +static struct drm_framebuffer *add_framebuffer_internal(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; - if (!drm_core_check_feature(dev, DRIVER_MODESET)) - return -EINVAL; - if (r->flags & ~DRM_MODE_FB_INTERLACED) { DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags); - return -EINVAL; + return ERR_PTR(-EINVAL); } if ((config->min_width > r->width) || (r->width > config->max_width)) { DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n", r->width, config->min_width, config->max_width); - return -EINVAL; + return ERR_PTR(-EINVAL); } if ((config->min_height > r->height) || (r->height > config->max_height)) { DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n", r->height, config->min_height, config->max_height); - return -EINVAL; + return ERR_PTR(-EINVAL); } ret = framebuffer_check(r); if (ret) - return ret; + return ERR_PTR(ret); fb = dev->mode_config.funcs->fb_create(dev, file_priv, r); if (IS_ERR(fb)) { DRM_DEBUG_KMS("could not create framebuffer\n"); - return PTR_ERR(fb); + return fb; } mutex_lock(&file_priv->fbs_lock); @@ -2854,8 +2837,37 @@ int drm_mode_addfb2(struct drm_device *dev, DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id); mutex_unlock(&file_priv->fbs_lock); + return fb; +} + +/** + * drm_mode_addfb2 - add an FB to the graphics configuration + * @dev: drm device for the ioctl + * @data: data pointer for the ioctl + * @file_priv: drm file for the ioctl call + * + * Add a new FB to the specified CRTC, given a user request with format. This is + * the 2nd version of the addfb ioctl, which supports multi-planar framebuffers + * and uses fourcc codes as pixel format specifiers. + * + * 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_framebuffer *fb; - return ret; + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + fb = add_framebuffer_internal(dev, data, file_priv); + if (IS_ERR(fb)) + return PTR_ERR(fb); + + return 0; } /** -- 1.8.5.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] drm: Refactor setplane to allow internal use 2014-05-16 22:36 [PATCH 1/2] drm: Refactor framebuffer creation to allow internal use Matt Roper @ 2014-05-16 22:36 ` Matt Roper 2014-05-19 7:24 ` Daniel Vetter 2014-05-19 7:17 ` [PATCH 1/2] drm: Refactor framebuffer creation " Daniel Vetter 1 sibling, 1 reply; 4+ messages in thread From: Matt Roper @ 2014-05-16 22:36 UTC (permalink / raw) To: intel-gfx, dri-devel Refactor DRM setplane code into a new setplane_internal() function that takes DRM objects directly as parameters rather than looking them up by ID. We'll use this in a future patch when we implement legacy cursor ioctls on top of the universal plane interface. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/drm_crtc.c | 178 +++++++++++++++++++++++++-------------------- 1 file changed, 100 insertions(+), 78 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 1a1a5f4..201c317 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2075,48 +2075,39 @@ out: return ret; } -/** - * drm_mode_setplane - configure a plane's configuration - * @dev: DRM device - * @data: ioctl data* - * @file_priv: DRM file info +/* + * setplane_internal - setplane handler for internal callers * - * Set plane configuration, including placement, fb, scaling, and other factors. - * Or pass a NULL fb to disable. + * Note that we assume an extra reference has already been taken on fb. If the + * update fails, this reference will be dropped before return; if it succeeds, + * the previous framebuffer (if any) will be unreferenced instead. * - * Returns: - * Zero on success, errno on failure. + * src_{x,y,w,h} are provided in 16.16 fixed point format */ -int drm_mode_setplane(struct drm_device *dev, void *data, - struct drm_file *file_priv) +static int setplane_internal(struct drm_crtc *crtc, + struct drm_plane *plane, + struct drm_framebuffer *fb, + int32_t crtc_x, int32_t crtc_y, + uint32_t crtc_w, uint32_t crtc_h, + /* src_{x,y,w,h} values are 16.16 fixed point */ + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h) { - 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 = NULL, *old_fb = NULL; + struct drm_device *dev = crtc->dev; + struct drm_framebuffer *old_fb = NULL; int ret = 0; unsigned int fb_width, fb_height; int i; - if (!drm_core_check_feature(dev, DRIVER_MODESET)) - return -EINVAL; - - /* - * 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); - return -ENOENT; + /* Check whether this plane is usable on this CRTC */ + if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) { + DRM_DEBUG_KMS("Invalid crtc for plane\n"); + ret = -EINVAL; + goto out; } - plane = obj_to_plane(obj); /* No fb means shut it down */ - if (!plane_req->fb_id) { + if (!fb) { drm_modeset_lock_all(dev); old_fb = plane->fb; ret = plane->funcs->disable_plane(plane); @@ -2130,31 +2121,6 @@ int drm_mode_setplane(struct drm_device *dev, void *data, 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); - - /* Check whether this plane is usable on this CRTC */ - if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) { - DRM_DEBUG_KMS("Invalid crtc for plane\n"); - ret = -EINVAL; - goto out; - } - - fb = drm_framebuffer_lookup(dev, plane_req->fb_id); - if (!fb) { - DRM_DEBUG_KMS("Unknown framebuffer ID %d\n", - plane_req->fb_id); - ret = -ENOENT; - goto out; - } - /* 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]) @@ -2170,32 +2136,27 @@ int drm_mode_setplane(struct drm_device *dev, void *data, 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) { + if (src_w > fb_width || + src_x > fb_width - src_w || + src_h > fb_height || + src_y > fb_height - 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); + src_w >> 16, ((src_w & 0xffff) * 15625) >> 10, + src_h >> 16, ((src_h & 0xffff) * 15625) >> 10, + src_x >> 16, ((src_x & 0xffff) * 15625) >> 10, + src_y >> 16, ((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) { + if (crtc_w > INT_MAX || + crtc_x > INT_MAX - (int32_t) crtc_w || + crtc_h > INT_MAX || + crtc_y > INT_MAX - (int32_t) 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); + crtc_w, crtc_h, crtc_x, crtc_y); ret = -ERANGE; goto out; } @@ -2203,10 +2164,8 @@ int drm_mode_setplane(struct drm_device *dev, void *data, drm_modeset_lock_all(dev); old_fb = plane->fb; 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); + crtc_x, crtc_y, crtc_w, crtc_h, + src_x, src_y, src_w, src_h); if (!ret) { plane->crtc = crtc; plane->fb = fb; @@ -2223,6 +2182,69 @@ out: drm_framebuffer_unreference(old_fb); return ret; + +} + +/** + * drm_mode_setplane - configure a plane's configuration + * @dev: DRM device + * @data: ioctl data* + * @file_priv: DRM file info + * + * Set plane configuration, including placement, fb, scaling, and other factors. + * Or pass a NULL fb to disable. + * + * Returns: + * Zero on success, errno on failure. + */ +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 = NULL; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + /* + * 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); + return -ENOENT; + } + plane = obj_to_plane(obj); + + 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); + return -ENOENT; + } + crtc = obj_to_crtc(obj); + + if (plane_req->fb_id) { + fb = drm_framebuffer_lookup(dev, plane_req->fb_id); + if (!fb) { + DRM_DEBUG_KMS("Unknown framebuffer ID %d\n", + plane_req->fb_id); + return -ENOENT; + } + } + + return setplane_internal(crtc, plane, 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); } /** -- 1.8.5.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] drm: Refactor setplane to allow internal use 2014-05-16 22:36 ` [PATCH 2/2] drm: Refactor setplane " Matt Roper @ 2014-05-19 7:24 ` Daniel Vetter 0 siblings, 0 replies; 4+ messages in thread From: Daniel Vetter @ 2014-05-19 7:24 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Fri, May 16, 2014 at 03:36:48PM -0700, Matt Roper wrote: > Refactor DRM setplane code into a new setplane_internal() function that > takes DRM objects directly as parameters rather than looking them up by > ID. We'll use this in a future patch when we implement legacy cursor > ioctls on top of the universal plane interface. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> A few small things below, with those addressed this is Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Aside: Disabling a plane doesn't require a valid crtc, I think we should have an igt testcase for this. > --- > drivers/gpu/drm/drm_crtc.c | 178 +++++++++++++++++++++++++-------------------- > 1 file changed, 100 insertions(+), 78 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 1a1a5f4..201c317 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -2075,48 +2075,39 @@ out: > return ret; > } > > -/** > - * drm_mode_setplane - configure a plane's configuration > - * @dev: DRM device > - * @data: ioctl data* > - * @file_priv: DRM file info > +/* > + * setplane_internal - setplane handler for internal callers > * > - * Set plane configuration, including placement, fb, scaling, and other factors. > - * Or pass a NULL fb to disable. > + * Note that we assume an extra reference has already been taken on fb. If the > + * update fails, this reference will be dropped before return; if it succeeds, > + * the previous framebuffer (if any) will be unreferenced instead. > * > - * Returns: > - * Zero on success, errno on failure. > + * src_{x,y,w,h} are provided in 16.16 fixed point format > */ > -int drm_mode_setplane(struct drm_device *dev, void *data, > - struct drm_file *file_priv) > +static int setplane_internal(struct drm_crtc *crtc, > + struct drm_plane *plane, > + struct drm_framebuffer *fb, > + int32_t crtc_x, int32_t crtc_y, > + uint32_t crtc_w, uint32_t crtc_h, > + /* src_{x,y,w,h} values are 16.16 fixed point */ > + uint32_t src_x, uint32_t src_y, > + uint32_t src_w, uint32_t src_h) > { > - 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 = NULL, *old_fb = NULL; > + struct drm_device *dev = crtc->dev; > + struct drm_framebuffer *old_fb = NULL; > int ret = 0; > unsigned int fb_width, fb_height; > int i; > > - if (!drm_core_check_feature(dev, DRIVER_MODESET)) > - return -EINVAL; > - > - /* > - * 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); > - return -ENOENT; > + /* Check whether this plane is usable on this CRTC */ > + if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) { > + DRM_DEBUG_KMS("Invalid crtc for plane\n"); > + ret = -EINVAL; > + goto out; > } Should we keep this check below the no fb case? For shutting off a plane you don't need to spec a valid crtc afaik. > - plane = obj_to_plane(obj); > > /* No fb means shut it down */ > - if (!plane_req->fb_id) { > + if (!fb) { > drm_modeset_lock_all(dev); > old_fb = plane->fb; > ret = plane->funcs->disable_plane(plane); > @@ -2130,31 +2121,6 @@ int drm_mode_setplane(struct drm_device *dev, void *data, > 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); > - > - /* Check whether this plane is usable on this CRTC */ > - if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) { > - DRM_DEBUG_KMS("Invalid crtc for plane\n"); > - ret = -EINVAL; > - goto out; > - } > - > - fb = drm_framebuffer_lookup(dev, plane_req->fb_id); > - if (!fb) { > - DRM_DEBUG_KMS("Unknown framebuffer ID %d\n", > - plane_req->fb_id); > - ret = -ENOENT; > - goto out; > - } > - > /* 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]) > @@ -2170,32 +2136,27 @@ int drm_mode_setplane(struct drm_device *dev, void *data, > 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) { > + if (src_w > fb_width || > + src_x > fb_width - src_w || > + src_h > fb_height || > + src_y > fb_height - 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); > + src_w >> 16, ((src_w & 0xffff) * 15625) >> 10, > + src_h >> 16, ((src_h & 0xffff) * 15625) >> 10, > + src_x >> 16, ((src_x & 0xffff) * 15625) >> 10, > + src_y >> 16, ((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) { > + if (crtc_w > INT_MAX || > + crtc_x > INT_MAX - (int32_t) crtc_w || > + crtc_h > INT_MAX || > + crtc_y > INT_MAX - (int32_t) 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); > + crtc_w, crtc_h, crtc_x, crtc_y); > ret = -ERANGE; > goto out; > } > @@ -2203,10 +2164,8 @@ int drm_mode_setplane(struct drm_device *dev, void *data, > drm_modeset_lock_all(dev); > old_fb = plane->fb; > 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); > + crtc_x, crtc_y, crtc_w, crtc_h, > + src_x, src_y, src_w, src_h); > if (!ret) { > plane->crtc = crtc; > plane->fb = fb; > @@ -2223,6 +2182,69 @@ out: > drm_framebuffer_unreference(old_fb); > > return ret; > + > +} > + > +/** > + * drm_mode_setplane - configure a plane's configuration > + * @dev: DRM device > + * @data: ioctl data* > + * @file_priv: DRM file info > + * > + * Set plane configuration, including placement, fb, scaling, and other factors. > + * Or pass a NULL fb to disable. > + * > + * Returns: > + * Zero on success, errno on failure. > + */ > +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 = NULL; > + > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > + return -EINVAL; > + > + /* > + * 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); > + return -ENOENT; > + } > + plane = obj_to_plane(obj); > + > + 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); > + return -ENOENT; > + } > + crtc = obj_to_crtc(obj); The crtc lookup should be moved into the "have fb" since for disabling a plane we don't need a valid crtc. > + > + if (plane_req->fb_id) { > + fb = drm_framebuffer_lookup(dev, plane_req->fb_id); > + if (!fb) { > + DRM_DEBUG_KMS("Unknown framebuffer ID %d\n", > + plane_req->fb_id); > + return -ENOENT; > + } > + } > + > + return setplane_internal(crtc, plane, fb, plane_req->crtc_x, Bikeshed: I tend to split lines such that x/y and w/h pairs stay on the same line. Bonus points if you match the line-breaking of the function definition ;-) > + 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); > } > > /** > -- > 1.8.5.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] drm: Refactor framebuffer creation to allow internal use 2014-05-16 22:36 [PATCH 1/2] drm: Refactor framebuffer creation to allow internal use Matt Roper 2014-05-16 22:36 ` [PATCH 2/2] drm: Refactor setplane " Matt Roper @ 2014-05-19 7:17 ` Daniel Vetter 1 sibling, 0 replies; 4+ messages in thread From: Daniel Vetter @ 2014-05-19 7:17 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Fri, May 16, 2014 at 03:36:47PM -0700, Matt Roper wrote: > Refactor DRM framebuffer creation into a new function that returns a > struct drm_framebuffer directly. The upcoming universal cursor support > will want to create framebuffers internally to wrap cursor buffers, so > we want to be able to share that framebuffer creation with the > drm_mode_addfb2 ioctl handler. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/drm_crtc.c | 64 +++++++++++++++++++++++++++------------------- > 1 file changed, 38 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index b6d6c04..1a1a5f4 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -2796,56 +2796,39 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) > return 0; > } > > -/** > - * drm_mode_addfb2 - add an FB to the graphics configuration > - * @dev: drm device for the ioctl > - * @data: data pointer for the ioctl > - * @file_priv: drm file for the ioctl call > - * > - * Add a new FB to the specified CRTC, given a user request with format. This is > - * the 2nd version of the addfb ioctl, which supports multi-planar framebuffers > - * and uses fourcc codes as pixel format specifiers. > - * > - * 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) > +static struct drm_framebuffer *add_framebuffer_internal(struct drm_device *dev, > + void *data, s/void/struct drm_mode_fb_cmd2/ With that fixed this is Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + 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; > > - if (!drm_core_check_feature(dev, DRIVER_MODESET)) > - return -EINVAL; > - > if (r->flags & ~DRM_MODE_FB_INTERLACED) { > DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags); > - return -EINVAL; > + return ERR_PTR(-EINVAL); > } > > if ((config->min_width > r->width) || (r->width > config->max_width)) { > DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n", > r->width, config->min_width, config->max_width); > - return -EINVAL; > + return ERR_PTR(-EINVAL); > } > if ((config->min_height > r->height) || (r->height > config->max_height)) { > DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n", > r->height, config->min_height, config->max_height); > - return -EINVAL; > + return ERR_PTR(-EINVAL); > } > > ret = framebuffer_check(r); > if (ret) > - return ret; > + return ERR_PTR(ret); > > fb = dev->mode_config.funcs->fb_create(dev, file_priv, r); > if (IS_ERR(fb)) { > DRM_DEBUG_KMS("could not create framebuffer\n"); > - return PTR_ERR(fb); > + return fb; > } > > mutex_lock(&file_priv->fbs_lock); > @@ -2854,8 +2837,37 @@ int drm_mode_addfb2(struct drm_device *dev, > DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id); > mutex_unlock(&file_priv->fbs_lock); > > + return fb; > +} > + > +/** > + * drm_mode_addfb2 - add an FB to the graphics configuration > + * @dev: drm device for the ioctl > + * @data: data pointer for the ioctl > + * @file_priv: drm file for the ioctl call > + * > + * Add a new FB to the specified CRTC, given a user request with format. This is > + * the 2nd version of the addfb ioctl, which supports multi-planar framebuffers > + * and uses fourcc codes as pixel format specifiers. > + * > + * 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_framebuffer *fb; > > - return ret; > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > + return -EINVAL; > + > + fb = add_framebuffer_internal(dev, data, file_priv); > + if (IS_ERR(fb)) > + return PTR_ERR(fb); > + > + return 0; > } > > /** > -- > 1.8.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-05-19 7:24 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-16 22:36 [PATCH 1/2] drm: Refactor framebuffer creation to allow internal use Matt Roper 2014-05-16 22:36 ` [PATCH 2/2] drm: Refactor setplane " Matt Roper 2014-05-19 7:24 ` Daniel Vetter 2014-05-19 7:17 ` [PATCH 1/2] drm: Refactor framebuffer creation " Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox