| Message ID | 20200303120136.18294-2-andrzej.p@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add AFBC support for Rockchip | expand |
Hi Andrzej, On Tue, 3 Mar 2020 at 12:01, Andrzej Pietrasiewicz <andrzej.p@collabora.com> wrote: > * Returns: > * Pointer to a &drm_framebuffer on success or an error pointer on failure. > */ > struct drm_framebuffer * > -drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, > - const struct drm_mode_fb_cmd2 *mode_cmd, > - const struct drm_framebuffer_funcs *funcs) > +drm_gem_fb_init_with_funcs(struct drm_device *dev, struct drm_framebuffer *fb, > +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, > + const struct drm_mode_fb_cmd2 *mode_cmd, > + const struct drm_framebuffer_funcs *funcs) > +{ > + struct drm_framebuffer *fb, *ret; > + > + fb = kzalloc(sizeof(*fb), GFP_KERNEL); > + if (!fb) > + return ERR_PTR(-ENOMEM); > + > + ret = drm_gem_fb_init_with_funcs(dev, fb, file, mode_cmd, funcs); > + if (IS_ERR_OR_NULL(ret)) We can make this "IS_ERR", since the function never returns NULL. The documentation explicitly states that error pointer is returned. -Emil
On Tue, Mar 03, 2020 at 01:01:31PM +0100, Andrzej Pietrasiewicz wrote: > Allow allocating a specialized version of struct drm_framebuffer > by moving the actual fb allocation out of drm_gem_fb_create_with_funcs(); > the respective functions names are adjusted to reflect that fact. > Please note, though, that standard size checks are performed on buffers, > so the drm_gem_fb_init_with_funcs() is useful for cases where those > standard size checks are appropriate or at least don't conflict the > checks to be performed in the specialized case. > > Thanks to this change the drivers can call drm_gem_fb_init_with_funcs() > having allocated their special version of struct drm_framebuffer, exactly > the way the new version of drm_gem_fb_create_with_funcs() does. > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > --- > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 65 +++++++++++++++----- > include/drm/drm_gem_framebuffer_helper.h | 5 ++ > 2 files changed, 56 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c > index 3a7ace19a902..388a080cd2df 100644 > --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c > +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c > @@ -55,18 +55,14 @@ struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb, > EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj); > > static struct drm_framebuffer * > -drm_gem_fb_alloc(struct drm_device *dev, > +drm_gem_fb_init(struct drm_device *dev, > + struct drm_framebuffer *fb, > const struct drm_mode_fb_cmd2 *mode_cmd, > struct drm_gem_object **obj, unsigned int num_planes, > const struct drm_framebuffer_funcs *funcs) > { > - struct drm_framebuffer *fb; > int ret, i; > > - fb = kzalloc(sizeof(*fb), GFP_KERNEL); > - if (!fb) > - return ERR_PTR(-ENOMEM); > - > drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd); > > for (i = 0; i < num_planes; i++) > @@ -123,10 +119,13 @@ int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file, > EXPORT_SYMBOL(drm_gem_fb_create_handle); > > /** > - * drm_gem_fb_create_with_funcs() - Helper function for the > - * &drm_mode_config_funcs.fb_create > - * callback > + * drm_gem_fb_init_with_funcs() - Helper function for implementing > + * &drm_mode_config_funcs.fb_create > + * callback in cases when the driver > + * allocates a subclass of > + * struct drm_framebuffer > * @dev: DRM device > + * @fb: framebuffer object > * @file: DRM file that holds the GEM handle(s) backing the framebuffer > * @mode_cmd: Metadata from the userspace framebuffer creation request > * @funcs: vtable to be used for the new framebuffer object > @@ -134,18 +133,21 @@ EXPORT_SYMBOL(drm_gem_fb_create_handle); > * This function can be used to set &drm_framebuffer_funcs for drivers that need > * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to > * change &drm_framebuffer_funcs. The function does buffer size validation. > + * The buffer size validation is for a general case, though, so users should > + * pay attention to the checks being appropriate for them or, at least, > + * non-conflicting. > * > * Returns: > * Pointer to a &drm_framebuffer on success or an error pointer on failure. > */ > struct drm_framebuffer * > -drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, > - const struct drm_mode_fb_cmd2 *mode_cmd, > - const struct drm_framebuffer_funcs *funcs) > +drm_gem_fb_init_with_funcs(struct drm_device *dev, struct drm_framebuffer *fb, > + struct drm_file *file, > + const struct drm_mode_fb_cmd2 *mode_cmd, > + const struct drm_framebuffer_funcs *funcs) For these two new added funcs: fb_init()/fb_init_with_funcs(), can we change the return type "struct drm_framebuffer *" to "int". > { > const struct drm_format_info *info; > struct drm_gem_object *objs[4]; > - struct drm_framebuffer *fb; > int ret, i; > > info = drm_get_format_info(dev, mode_cmd); > @@ -175,7 +177,7 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, > } > } > > - fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs); > + fb = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs); > if (IS_ERR(fb)) { > ret = PTR_ERR(fb); > goto err_gem_object_put; > @@ -189,6 +191,41 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, > > return ERR_PTR(ret); > } > +EXPORT_SYMBOL_GPL(drm_gem_fb_init_with_funcs); > + > +/** > + * drm_gem_fb_create_with_funcs() - Helper function for the > + * &drm_mode_config_funcs.fb_create > + * callback > + * @dev: DRM device > + * @file: DRM file that holds the GEM handle(s) backing the framebuffer > + * @mode_cmd: Metadata from the userspace framebuffer creation request > + * @funcs: vtable to be used for the new framebuffer object > + * > + * This function can be used to set &drm_framebuffer_funcs for drivers that need > + * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to > + * change &drm_framebuffer_funcs. The function does buffer size validation. > + * > + * Returns: > + * Pointer to a &drm_framebuffer on success or an error pointer on failure. > + */ > +struct drm_framebuffer * > +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, > + const struct drm_mode_fb_cmd2 *mode_cmd, > + const struct drm_framebuffer_funcs *funcs) > +{ > + struct drm_framebuffer *fb, *ret; > + > + fb = kzalloc(sizeof(*fb), GFP_KERNEL); > + if (!fb) > + return ERR_PTR(-ENOMEM); > + > + ret = drm_gem_fb_init_with_funcs(dev, fb, file, mode_cmd, funcs); > + if (IS_ERR_OR_NULL(ret)) > + kfree(fb); > + > + return ret; > +} > EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs); > > static const struct drm_framebuffer_funcs drm_gem_fb_funcs = { > diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h > index d9f13fd25b0a..3f61d9f832ee 100644 > --- a/include/drm/drm_gem_framebuffer_helper.h > +++ b/include/drm/drm_gem_framebuffer_helper.h > @@ -19,6 +19,11 @@ int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file, > unsigned int *handle); > > struct drm_framebuffer * > +drm_gem_fb_init_with_funcs(struct drm_device *dev, struct drm_framebuffer *fb, > + struct drm_file *file, > + const struct drm_mode_fb_cmd2 *mode_cmd, > + const struct drm_framebuffer_funcs *funcs); > +struct drm_framebuffer * > drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, > const struct drm_mode_fb_cmd2 *mode_cmd, > const struct drm_framebuffer_funcs *funcs);
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index 3a7ace19a902..388a080cd2df 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -55,18 +55,14 @@ struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb, EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj); static struct drm_framebuffer * -drm_gem_fb_alloc(struct drm_device *dev, +drm_gem_fb_init(struct drm_device *dev, + struct drm_framebuffer *fb, const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **obj, unsigned int num_planes, const struct drm_framebuffer_funcs *funcs) { - struct drm_framebuffer *fb; int ret, i; - fb = kzalloc(sizeof(*fb), GFP_KERNEL); - if (!fb) - return ERR_PTR(-ENOMEM); - drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd); for (i = 0; i < num_planes; i++) @@ -123,10 +119,13 @@ int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file, EXPORT_SYMBOL(drm_gem_fb_create_handle); /** - * drm_gem_fb_create_with_funcs() - Helper function for the - * &drm_mode_config_funcs.fb_create - * callback + * drm_gem_fb_init_with_funcs() - Helper function for implementing + * &drm_mode_config_funcs.fb_create + * callback in cases when the driver + * allocates a subclass of + * struct drm_framebuffer * @dev: DRM device + * @fb: framebuffer object * @file: DRM file that holds the GEM handle(s) backing the framebuffer * @mode_cmd: Metadata from the userspace framebuffer creation request * @funcs: vtable to be used for the new framebuffer object @@ -134,18 +133,21 @@ EXPORT_SYMBOL(drm_gem_fb_create_handle); * This function can be used to set &drm_framebuffer_funcs for drivers that need * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to * change &drm_framebuffer_funcs. The function does buffer size validation. + * The buffer size validation is for a general case, though, so users should + * pay attention to the checks being appropriate for them or, at least, + * non-conflicting. * * Returns: * Pointer to a &drm_framebuffer on success or an error pointer on failure. */ struct drm_framebuffer * -drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, - const struct drm_mode_fb_cmd2 *mode_cmd, - const struct drm_framebuffer_funcs *funcs) +drm_gem_fb_init_with_funcs(struct drm_device *dev, struct drm_framebuffer *fb, + struct drm_file *file, + const struct drm_mode_fb_cmd2 *mode_cmd, + const struct drm_framebuffer_funcs *funcs) { const struct drm_format_info *info; struct drm_gem_object *objs[4]; - struct drm_framebuffer *fb; int ret, i; info = drm_get_format_info(dev, mode_cmd); @@ -175,7 +177,7 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, } } - fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs); + fb = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs); if (IS_ERR(fb)) { ret = PTR_ERR(fb); goto err_gem_object_put; @@ -189,6 +191,41 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, return ERR_PTR(ret); } +EXPORT_SYMBOL_GPL(drm_gem_fb_init_with_funcs); + +/** + * drm_gem_fb_create_with_funcs() - Helper function for the + * &drm_mode_config_funcs.fb_create + * callback + * @dev: DRM device + * @file: DRM file that holds the GEM handle(s) backing the framebuffer + * @mode_cmd: Metadata from the userspace framebuffer creation request + * @funcs: vtable to be used for the new framebuffer object + * + * This function can be used to set &drm_framebuffer_funcs for drivers that need + * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to + * change &drm_framebuffer_funcs. The function does buffer size validation. + * + * Returns: + * Pointer to a &drm_framebuffer on success or an error pointer on failure. + */ +struct drm_framebuffer * +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, + const struct drm_mode_fb_cmd2 *mode_cmd, + const struct drm_framebuffer_funcs *funcs) +{ + struct drm_framebuffer *fb, *ret; + + fb = kzalloc(sizeof(*fb), GFP_KERNEL); + if (!fb) + return ERR_PTR(-ENOMEM); + + ret = drm_gem_fb_init_with_funcs(dev, fb, file, mode_cmd, funcs); + if (IS_ERR_OR_NULL(ret)) + kfree(fb); + + return ret; +} EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs); static const struct drm_framebuffer_funcs drm_gem_fb_funcs = { diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h index d9f13fd25b0a..3f61d9f832ee 100644 --- a/include/drm/drm_gem_framebuffer_helper.h +++ b/include/drm/drm_gem_framebuffer_helper.h @@ -19,6 +19,11 @@ int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file, unsigned int *handle); struct drm_framebuffer * +drm_gem_fb_init_with_funcs(struct drm_device *dev, struct drm_framebuffer *fb, + struct drm_file *file, + const struct drm_mode_fb_cmd2 *mode_cmd, + const struct drm_framebuffer_funcs *funcs); +struct drm_framebuffer * drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file, const struct drm_mode_fb_cmd2 *mode_cmd, const struct drm_framebuffer_funcs *funcs);
Allow allocating a specialized version of struct drm_framebuffer by moving the actual fb allocation out of drm_gem_fb_create_with_funcs(); the respective functions names are adjusted to reflect that fact. Please note, though, that standard size checks are performed on buffers, so the drm_gem_fb_init_with_funcs() is useful for cases where those standard size checks are appropriate or at least don't conflict the checks to be performed in the specialized case. Thanks to this change the drivers can call drm_gem_fb_init_with_funcs() having allocated their special version of struct drm_framebuffer, exactly the way the new version of drm_gem_fb_create_with_funcs() does. Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> --- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 65 +++++++++++++++----- include/drm/drm_gem_framebuffer_helper.h | 5 ++ 2 files changed, 56 insertions(+), 14 deletions(-)