From f2b816d78a94319b7230430c3d1f99d9f1cb5bd0 Mon Sep 17 00:00:00 2001 From: Andrzej Pietrasiewicz Date: Wed, 11 Mar 2020 15:55:36 +0100 Subject: drm/core: Allow drivers allocate a subclass of struct drm_framebuffer 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 Reviewed-by: Emil Velikov Reviewed-by: James Qian Wang Acked-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20200311145541.29186-2-andrzej.p@collabora.com --- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 87 ++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 25 deletions(-) (limited to 'drivers/gpu/drm/drm_gem_framebuffer_helper.c') diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index 3a7ace19a902..86c1907c579a 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -54,19 +54,15 @@ 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, +static int +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++) @@ -76,10 +72,9 @@ drm_gem_fb_alloc(struct drm_device *dev, if (ret) { drm_err(dev, "Failed to init framebuffer: %d\n", ret); kfree(fb); - return ERR_PTR(ret); } - return fb; + return ret; } /** @@ -123,10 +118,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,23 +132,26 @@ 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. + * Zero or a negative error code. */ -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) +int 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); if (!info) - return ERR_PTR(-EINVAL); + return -EINVAL; for (i = 0; i < info->num_planes; i++) { unsigned int width = mode_cmd->width / (i ? info->hsub : 1); @@ -175,19 +176,55 @@ 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); - if (IS_ERR(fb)) { - ret = PTR_ERR(fb); + ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs); + if (ret) goto err_gem_object_put; - } - return fb; + return 0; err_gem_object_put: for (i--; i >= 0; i--) drm_gem_object_put_unlocked(objs[i]); - return ERR_PTR(ret); + return 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; + int 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 (ret) { + kfree(fb); + return ERR_PTR(ret); + } + + return fb; } EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs); -- cgit From 55f7f72753abdd46f35d027a25b43969dba56fac Mon Sep 17 00:00:00 2001 From: Andrzej Pietrasiewicz Date: Wed, 11 Mar 2020 15:55:37 +0100 Subject: drm/core: Add drm_afbc_framebuffer and a corresponding helper The new struct contains afbc-specific data. The new function can be used by drivers which support afbc to complete the preparation of struct drm_afbc_framebuffer. It must be called after allocating the said struct and calling drm_gem_fb_init_with_funcs(). Signed-off-by: Andrzej Pietrasiewicz Reviewed-by: Emil Velikov Reviewed-by: James Qian Wang Acked-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20200311145541.29186-3-andrzej.p@collabora.com --- Documentation/gpu/todo.rst | 15 ++++ drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++++++++++++++++++++++++++ include/drm/drm_framebuffer.h | 45 +++++++++++ include/drm/drm_gem_framebuffer_helper.h | 10 +++ 4 files changed, 178 insertions(+) (limited to 'drivers/gpu/drm/drm_gem_framebuffer_helper.c') diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 439656f55c5d..37a3a023c114 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers Level: Intermediate +Encode cpp properly in malidp +----------------------------- + +cpp (chars per pixel) is not encoded properly in malidp, zero is +used instead. afbc implementation needs bpp or cpp, but if it is +zero it needs to be provided elsewhere, and so the bpp field exists +in struct drm_afbc_framebuffer. + +Properly encode cpp in malidp and remove the bpp field in struct +drm_afbc_framebuffer. + +Contact: malidp maintainers + +Level: Intermediate + Core refactorings ================= diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index 86c1907c579a..7e3982c36baa 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -21,6 +21,13 @@ #include #include +#define AFBC_HEADER_SIZE 16 +#define AFBC_TH_LAYOUT_ALIGNMENT 8 +#define AFBC_HDR_ALIGN 64 +#define AFBC_SUPERBLOCK_PIXELS 256 +#define AFBC_SUPERBLOCK_ALIGNMENT 128 +#define AFBC_TH_BODY_START_ALIGNMENT 4096 + /** * DOC: overview * @@ -302,6 +309,107 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file, } EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty); +static int drm_gem_afbc_min_size(struct drm_device *dev, + const struct drm_mode_fb_cmd2 *mode_cmd, + struct drm_afbc_framebuffer *afbc_fb) +{ + const struct drm_format_info *info; + __u32 n_blocks, w_alignment, h_alignment, hdr_alignment; + /* remove bpp when all users properly encode cpp in drm_format_info */ + __u32 bpp; + + switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) { + case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16: + afbc_fb->block_width = 16; + afbc_fb->block_height = 16; + break; + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8: + afbc_fb->block_width = 32; + afbc_fb->block_height = 8; + break; + /* no user exists yet - fall through */ + case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4: + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4: + default: + DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n", + mode_cmd->modifier[0] + & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK); + return -EINVAL; + } + + /* tiled header afbc */ + w_alignment = afbc_fb->block_width; + h_alignment = afbc_fb->block_height; + hdr_alignment = AFBC_HDR_ALIGN; + if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) { + w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT; + h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT; + hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT; + } + + afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment); + afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment); + afbc_fb->offset = mode_cmd->offsets[0]; + + info = drm_get_format_info(dev, mode_cmd); + /* + * Change to always using info->cpp[0] + * when all users properly encode it + */ + bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp; + + n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height) + / AFBC_SUPERBLOCK_PIXELS; + afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment); + afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8, + AFBC_SUPERBLOCK_ALIGNMENT); + + return 0; +} + +/** + * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to + * fill and validate all the afbc-specific + * struct drm_afbc_framebuffer members + * + * @dev: DRM device + * @afbc_fb: afbc-specific framebuffer + * @mode_cmd: Metadata from the userspace framebuffer creation request + * @afbc_fb: afbc framebuffer + * + * This function can be used by drivers which support afbc to complete + * the preparation of struct drm_afbc_framebuffer. It must be called after + * allocating the said struct and calling drm_gem_fb_init_with_funcs(). + * It is caller's responsibility to put afbc_fb->base.obj objects in case + * the call is unsuccessful. + * + * Returns: + * Zero on success or a negative error value on failure. + */ +int drm_gem_fb_afbc_init(struct drm_device *dev, + const struct drm_mode_fb_cmd2 *mode_cmd, + struct drm_afbc_framebuffer *afbc_fb) +{ + const struct drm_format_info *info; + struct drm_gem_object **objs; + int ret; + + objs = afbc_fb->base.obj; + info = drm_get_format_info(dev, mode_cmd); + if (!info) + return -EINVAL; + + ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb); + if (ret < 0) + return ret; + + if (objs[0]->size < afbc_fb->afbc_size) + return -EINVAL; + + return 0; +} +EXPORT_SYMBOL_GPL(drm_gem_fb_afbc_init); + /** * drm_gem_fb_prepare_fb() - Prepare a GEM backed framebuffer * @plane: Plane diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h index c0e0256e3e98..e9f1b0e2968d 100644 --- a/include/drm/drm_framebuffer.h +++ b/include/drm/drm_framebuffer.h @@ -297,4 +297,49 @@ int drm_framebuffer_plane_width(int width, int drm_framebuffer_plane_height(int height, const struct drm_framebuffer *fb, int plane); +/** + * struct drm_afbc_framebuffer - a special afbc frame buffer object + * + * A derived class of struct drm_framebuffer, dedicated for afbc use cases. + */ +struct drm_afbc_framebuffer { + /** + * @base: base framebuffer structure. + */ + struct drm_framebuffer base; + /** + * @block_widht: width of a single afbc block + */ + u32 block_width; + /** + * @block_widht: height of a single afbc block + */ + u32 block_height; + /** + * @aligned_width: aligned frame buffer width + */ + u32 aligned_width; + /** + * @aligned_height: aligned frame buffer height + */ + u32 aligned_height; + /** + * @offset: offset of the first afbc header + */ + u32 offset; + /** + * @afbc_size: minimum size of afbc buffer + */ + u32 afbc_size; + /** + * @bpp: bpp value for this afbc buffer + * To be removed when users such as malidp + * properly store the cpp in drm_format_info. + * New users should not start using this field. + */ + u32 bpp; +}; + +#define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base) + #endif diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h index c029c1618661..6b013154911d 100644 --- a/include/drm/drm_gem_framebuffer_helper.h +++ b/include/drm/drm_gem_framebuffer_helper.h @@ -1,6 +1,7 @@ #ifndef __DRM_GEM_FB_HELPER_H__ #define __DRM_GEM_FB_HELPER_H__ +struct drm_afbc_framebuffer; struct drm_device; struct drm_fb_helper_surface_size; struct drm_file; @@ -12,6 +13,8 @@ struct drm_plane; struct drm_plane_state; struct drm_simple_display_pipe; +#define AFBC_VENDOR_AND_TYPE_MASK GENMASK_ULL(63, 52) + struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb, unsigned int plane); void drm_gem_fb_destroy(struct drm_framebuffer *fb); @@ -34,6 +37,13 @@ struct drm_framebuffer * drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file, const struct drm_mode_fb_cmd2 *mode_cmd); +#define drm_is_afbc(modifier) \ + (((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0)) + +int drm_gem_fb_afbc_init(struct drm_device *dev, + const struct drm_mode_fb_cmd2 *mode_cmd, + struct drm_afbc_framebuffer *afbc_fb); + int drm_gem_fb_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state); int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe, -- cgit From 88f1b29a44b197852828d3aba5fefa0082d377ff Mon Sep 17 00:00:00 2001 From: Andrzej Pietrasiewicz Date: Tue, 31 Mar 2020 17:53:07 +0200 Subject: drm/core: Use proper debugging macro Use drm_dbg_kms() instead of DRM_DEBUG_KMS. Signed-off-by: Andrzej Pietrasiewicz Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20200331155308.6345-2-andrzej.p@collabora.com --- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/drm_gem_framebuffer_helper.c') diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index 7e3982c36baa..6fb1841fa71c 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -331,9 +331,9 @@ static int drm_gem_afbc_min_size(struct drm_device *dev, case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4: case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4: default: - DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n", - mode_cmd->modifier[0] - & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK); + drm_dbg_kms(dev, "Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n", + mode_cmd->modifier[0] + & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK); return -EINVAL; } -- cgit From bcf6293d7ae931159fac4fbd9924b0276f1edabd Mon Sep 17 00:00:00 2001 From: Andrzej Pietrasiewicz Date: Tue, 31 Mar 2020 17:53:08 +0200 Subject: drm/core: Calculate bpp in afbc helper Some drivers (komeda, malidp) don't set anything in cpp. If that is the case the right value can be inferred from the format. Then the "bpp" member can be eliminated from struct drm_afbc_framebuffer. Signed-off-by: Andrzej Pietrasiewicz Acked-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20200331155308.6345-3-andrzej.p@collabora.com --- Documentation/gpu/todo.rst | 15 ----------- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 39 +++++++++++++++++++++++----- include/drm/drm_framebuffer.h | 7 ----- 3 files changed, 32 insertions(+), 29 deletions(-) (limited to 'drivers/gpu/drm/drm_gem_framebuffer_helper.c') diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 37a3a023c114..439656f55c5d 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -404,21 +404,6 @@ Contact: Laurent Pinchart, respective driver maintainers Level: Intermediate -Encode cpp properly in malidp ------------------------------ - -cpp (chars per pixel) is not encoded properly in malidp, zero is -used instead. afbc implementation needs bpp or cpp, but if it is -zero it needs to be provided elsewhere, and so the bpp field exists -in struct drm_afbc_framebuffer. - -Properly encode cpp in malidp and remove the bpp field in struct -drm_afbc_framebuffer. - -Contact: malidp maintainers - -Level: Intermediate - Core refactorings ================= diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index 6fb1841fa71c..cac15294aef6 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -309,11 +309,37 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file, } EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty); +static __u32 drm_gem_afbc_get_bpp(struct drm_device *dev, + const struct drm_mode_fb_cmd2 *mode_cmd) +{ + const struct drm_format_info *info; + + info = drm_get_format_info(dev, mode_cmd); + + /* use whatever a driver has set */ + if (info->cpp[0]) + return info->cpp[0] * 8; + + /* guess otherwise */ + switch (info->format) { + case DRM_FORMAT_YUV420_8BIT: + return 12; + case DRM_FORMAT_YUV420_10BIT: + return 15; + case DRM_FORMAT_VUY101010: + return 30; + default: + break; + } + + /* all attempts failed */ + return 0; +} + static int drm_gem_afbc_min_size(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_afbc_framebuffer *afbc_fb) { - const struct drm_format_info *info; __u32 n_blocks, w_alignment, h_alignment, hdr_alignment; /* remove bpp when all users properly encode cpp in drm_format_info */ __u32 bpp; @@ -351,12 +377,11 @@ static int drm_gem_afbc_min_size(struct drm_device *dev, afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment); afbc_fb->offset = mode_cmd->offsets[0]; - info = drm_get_format_info(dev, mode_cmd); - /* - * Change to always using info->cpp[0] - * when all users properly encode it - */ - bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp; + bpp = drm_gem_afbc_get_bpp(dev, mode_cmd); + if (!bpp) { + drm_dbg_kms(dev, "Invalid AFBC bpp value: %d\n", bpp); + return -EINVAL; + } n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height) / AFBC_SUPERBLOCK_PIXELS; diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h index b53c0332f040..be658ebbec72 100644 --- a/include/drm/drm_framebuffer.h +++ b/include/drm/drm_framebuffer.h @@ -331,13 +331,6 @@ struct drm_afbc_framebuffer { * @afbc_size: minimum size of afbc buffer */ u32 afbc_size; - /** - * @bpp: bpp value for this afbc buffer - * To be removed when users such as malidp - * properly store the cpp in drm_format_info. - * New users should not start using this field. - */ - u32 bpp; }; #define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base) -- cgit