Message ID | 20191217145020.14645-5-andrzej.p@collabora.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Add AFBC support for Rockchip | expand |
Hi Andrzej: On Tue, Dec 17, 2019 at 03:49:50PM +0100, Andrzej Pietrasiewicz wrote: > Extend the size-checking special function to handle afbc. > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > --- > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 49 +++++++++++++++++-- > include/drm/drm_framebuffer.h | 50 ++++++++++++++++++++ > include/drm/drm_gem_framebuffer_helper.h | 1 + > 3 files changed, 96 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c > index d2fce1ec8f37..5fe9032a5ee8 100644 > --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c > +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c > @@ -21,6 +21,11 @@ > #include <drm/drm_modeset_helper.h> > #include <drm/drm_simple_kms_helper.h> > > +#define AFBC_HEADER_SIZE 16 > +#define AFBC_TH_LAYOUT_ALIGNMENT 8 > +#define AFBC_SUPERBLOCK_PIXELS 256 > +#define AFBC_SUPERBLOCK_ALIGNMENT 128 > + > /** > * DOC: overview > * > @@ -299,6 +304,34 @@ int drm_gem_fb_lookup(struct drm_device *dev, > } > EXPORT_SYMBOL_GPL(drm_gem_fb_lookup); > > +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) > +{ > + u32 n_blocks; > + > + if (!drm_afbc_get_superblock_wh(mode_cmd->modifier[0], &afbc_fb->block_width, &afbc_fb->block_height)) > + return -EINVAL; > + > + /* tiled header afbc */ > + if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) { > + afbc_fb->block_width *= AFBC_TH_LAYOUT_ALIGNMENT; > + afbc_fb->block_height *= AFBC_TH_LAYOUT_ALIGNMENT; > + } TBH, here caculated afbc_fb->block_with/height are not block_width/height, but fb w/h alignment. Per my understanding, afbc only has block size: 16x16, 32x8, 64x4 ... generally the afbc w/h alignment according the the block_size, but once the tiled header enabled, since one tiled header describes 8x8 superblocks, so the alignment of w/h need to mutiple 8. So I think we'd better name the variable to width/height_alignment. BTW: no matter block_w/h or w/h_alignmtent are only for size calculation, seems no need to store them to afbc_fb. > + > + afbc_fb->aligned_width = ALIGN(mode_cmd->width, afbc_fb->block_width); > + afbc_fb->aligned_height = ALIGN(mode_cmd->height, afbc_fb->block_height); > + afbc_fb->offset = mode_cmd->offsets[0]; > + > + n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height) / AFBC_SUPERBLOCK_PIXELS; > + afbc_fb->offset_payload = ALIGN(n_blocks * AFBC_HEADER_SIZE, afbc_fb->alignment_header); > + After check the references in malidp, rockchip and komeda, seems this afbc->alignment_header is dedicated for komeda only and a pass in argument. This is not true. Per afbc HW spec alignment is essential for all afbc usage. according to the spec the requiremnt are: AFBC1.0/1.1: 64 byte alignment both for header and body buffer. AFBC1.2 (tiled header enabled): 4096 alignment. So this alignement is not a vendor specific value, but afbc feature requirement, can be determined by afbc modifier. (malidp and komeda obeys this spec, not sure about Rockchip, but I think it should be) But you may see, komeda uses 1024 (not 64) for none-tiled-header afbc, that's because GPU(MALI) changed this value to 1024 for bus performance (sorry I don't know the detail), and komeda changed to 1024 to follow. Back to alignment_header here, I think we can just follow the spec, use 64 for none-tiled-header, 4096 for tiled-header, and no need to let the caller to specify it > + afbc_fb->afbc_size = afbc_fb->offset_payload > + + n_blocks * ALIGN(afbc_fb->bpp * AFBC_SUPERBLOCK_PIXELS / 8, AFBC_SUPERBLOCK_ALIGNMENT); > + > + return 0; > +} > + > /** > * drm_gem_fb_size_check2() - Helper function for use in > * &drm_mode_config_funcs.fb_create implementations > @@ -334,19 +367,27 @@ int drm_gem_fb_size_check2(struct drm_device *dev, > check->pitch_modulo) > return -EINVAL; > > - if (check && check->use_min_size) > + if (check && check->use_min_size) { > min_size = check->min_size[i]; > - else > + } else if (check && check->data && drm_is_afbc(mode_cmd->modifier[0])) { > + struct drm_afbc_framebuffer *afbc_fb; > + int ret; > + > + afbc_fb = check->data; > + ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb); > + if (ret < 0) > + return ret; > + min_size = ret; > + } else { > min_size = (height - 1) * pitch > + drm_format_info_min_pitch(info, i, width) > + mode_cmd->offsets[i]; > - > + } > if (objs[i]->size < min_size) > return -EINVAL; > } > > return 0; > - > } > EXPORT_SYMBOL_GPL(drm_gem_fb_size_check2); > > diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h > index c0e0256e3e98..c8a06e37585a 100644 > --- a/include/drm/drm_framebuffer.h > +++ b/include/drm/drm_framebuffer.h > @@ -297,4 +297,54 @@ 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; Since malidp and komeda have no requirement for none-zero offset, so I think we can reject none zero offset as error like did in rockchip in afbc_size_check(). > + /** > + * @alignment_header: required alignment for afbc headers > + */ > + u32 alignment_header; > + /** > + * @afbc_size: minimum size of afbc buffer > + */ > + u32 afbc_size; > + /** > + * @offset_payload: start of afbc body buffer > + */ > + u32 offset_payload; > + /** > + * @bpp: bpp value for this afbc buffer > + */ > + u32 bpp; Seems we can remove this bpp or no need to define it as a pass in argument for size check, maybe the komeda/malidp get_afbc_bpp() function mislead you that afbc formats may have vendor specific bpp. But the story is: for afbc only formats like DRM_FORMAT_YUV420_8BIT/10BIT, we have set nothing in drm_format_info, neither cpp nor block_size, so both malidp or komeda introduce a get_bpp(), but actually the two funcs basically are same. So my suggestion is we can temporary use the get_afbc_bpp() in malidp or komeda. and eventually I think we'd better set the block size for these formats, then we can defines a common get_bpp() like pitch Thanks James > +}; > + > +#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 4955af96d6c3..17e3f849a0fb 100644 > --- a/include/drm/drm_gem_framebuffer_helper.h > +++ b/include/drm/drm_gem_framebuffer_helper.h > @@ -22,6 +22,7 @@ struct drm_size_check { > u32 pitch_multiplier[4]; > u32 pitch_modulo; > bool use_pitch_multiplier; > + void *data; > }; > > struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb, > -- > 2.17.1
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index d2fce1ec8f37..5fe9032a5ee8 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -21,6 +21,11 @@ #include <drm/drm_modeset_helper.h> #include <drm/drm_simple_kms_helper.h> +#define AFBC_HEADER_SIZE 16 +#define AFBC_TH_LAYOUT_ALIGNMENT 8 +#define AFBC_SUPERBLOCK_PIXELS 256 +#define AFBC_SUPERBLOCK_ALIGNMENT 128 + /** * DOC: overview * @@ -299,6 +304,34 @@ int drm_gem_fb_lookup(struct drm_device *dev, } EXPORT_SYMBOL_GPL(drm_gem_fb_lookup); +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) +{ + u32 n_blocks; + + if (!drm_afbc_get_superblock_wh(mode_cmd->modifier[0], &afbc_fb->block_width, &afbc_fb->block_height)) + return -EINVAL; + + /* tiled header afbc */ + if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) { + afbc_fb->block_width *= AFBC_TH_LAYOUT_ALIGNMENT; + afbc_fb->block_height *= AFBC_TH_LAYOUT_ALIGNMENT; + } + + afbc_fb->aligned_width = ALIGN(mode_cmd->width, afbc_fb->block_width); + afbc_fb->aligned_height = ALIGN(mode_cmd->height, afbc_fb->block_height); + afbc_fb->offset = mode_cmd->offsets[0]; + + n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height) / AFBC_SUPERBLOCK_PIXELS; + afbc_fb->offset_payload = ALIGN(n_blocks * AFBC_HEADER_SIZE, afbc_fb->alignment_header); + + afbc_fb->afbc_size = afbc_fb->offset_payload + + n_blocks * ALIGN(afbc_fb->bpp * AFBC_SUPERBLOCK_PIXELS / 8, AFBC_SUPERBLOCK_ALIGNMENT); + + return 0; +} + /** * drm_gem_fb_size_check2() - Helper function for use in * &drm_mode_config_funcs.fb_create implementations @@ -334,19 +367,27 @@ int drm_gem_fb_size_check2(struct drm_device *dev, check->pitch_modulo) return -EINVAL; - if (check && check->use_min_size) + if (check && check->use_min_size) { min_size = check->min_size[i]; - else + } else if (check && check->data && drm_is_afbc(mode_cmd->modifier[0])) { + struct drm_afbc_framebuffer *afbc_fb; + int ret; + + afbc_fb = check->data; + ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb); + if (ret < 0) + return ret; + min_size = ret; + } else { min_size = (height - 1) * pitch + drm_format_info_min_pitch(info, i, width) + mode_cmd->offsets[i]; - + } if (objs[i]->size < min_size) return -EINVAL; } return 0; - } EXPORT_SYMBOL_GPL(drm_gem_fb_size_check2); diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h index c0e0256e3e98..c8a06e37585a 100644 --- a/include/drm/drm_framebuffer.h +++ b/include/drm/drm_framebuffer.h @@ -297,4 +297,54 @@ 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; + /** + * @alignment_header: required alignment for afbc headers + */ + u32 alignment_header; + /** + * @afbc_size: minimum size of afbc buffer + */ + u32 afbc_size; + /** + * @offset_payload: start of afbc body buffer + */ + u32 offset_payload; + /** + * @bpp: bpp value for this afbc buffer + */ + 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 4955af96d6c3..17e3f849a0fb 100644 --- a/include/drm/drm_gem_framebuffer_helper.h +++ b/include/drm/drm_gem_framebuffer_helper.h @@ -22,6 +22,7 @@ struct drm_size_check { u32 pitch_multiplier[4]; u32 pitch_modulo; bool use_pitch_multiplier; + void *data; }; struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
Extend the size-checking special function to handle afbc. Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> --- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 49 +++++++++++++++++-- include/drm/drm_framebuffer.h | 50 ++++++++++++++++++++ include/drm/drm_gem_framebuffer_helper.h | 1 + 3 files changed, 96 insertions(+), 4 deletions(-)