On Mon, Nov 04, 2019 at 11:12:25PM +0100, Andrzej Pietrasiewicz wrote:
> These are useful for other users of afbc, e.g. rockchip.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/Kconfig | 8 +++
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/arm/Kconfig | 1 +
> drivers/gpu/drm/drm_afbc.c | 129 ++++++++++++++++++++++++++++++++++++
> include/drm/drm_afbc.h | 36 ++++++++++
> 5 files changed, 175 insertions(+)
> create mode 100644 drivers/gpu/drm/drm_afbc.c
> create mode 100644 include/drm/drm_afbc.h
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 36357a36a281..ae1ca5e02bfe 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -205,6 +205,14 @@ config DRM_SCHED
> tristate
> depends on DRM
>
> +config DRM_AFBC
> + tristate
> + depends on DRM
> + help
> + AFBC is a proprietary lossless image compression protocol and format.
> + It provides fine-grained random access and minimizes the amount of
> + data transferred between IP blocks.
Uh a module option for 2 functions ... seems like massive overkill. Please
just put that into existing format helpers instead.
What's missing otoh is kernel doc for these.
> +
> source "drivers/gpu/drm/i2c/Kconfig"
>
> source "drivers/gpu/drm/arm/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 9f1c7c486f88..3a5d092ea514 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -31,6 +31,7 @@ drm-$(CONFIG_OF) += drm_of.o
> drm-$(CONFIG_AGP) += drm_agpsupport.o
> drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
> drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> +drm-$(CONFIG_DRM_AFBC) += drm_afbc.o
>
> drm_vram_helper-y := drm_gem_vram_helper.o \
> drm_vram_helper_common.o
> diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> index a204103b3efb..25c3dc408cda 100644
> --- a/drivers/gpu/drm/arm/Kconfig
> +++ b/drivers/gpu/drm/arm/Kconfig
> @@ -29,6 +29,7 @@ config DRM_MALI_DISPLAY
> select DRM_KMS_HELPER
> select DRM_KMS_CMA_HELPER
> select DRM_GEM_CMA_HELPER
> + select DRM_AFBC
> select VIDEOMODE_HELPERS
> help
> Choose this option if you want to compile the ARM Mali Display
> diff --git a/drivers/gpu/drm/drm_afbc.c b/drivers/gpu/drm/drm_afbc.c
> new file mode 100644
> index 000000000000..010ca9eb0480
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_afbc.c
> @@ -0,0 +1,129 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * (C) 2019 Collabora Ltd.
> + *
> + * author: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx>
> + *
> + */
> +#include <linux/module.h>
> +
> +#include <drm/drm_afbc.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_gem.h>
> +#include <drm/drm_mode.h>
> +#include <drm/drm_print.h>
> +
> +#define AFBC_HEADER_SIZE 16
> +
> +bool drm_afbc_check_offset(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + if (mode_cmd->offsets[0] != 0) {
> + DRM_DEBUG_KMS("AFBC buffers' plane offset should be 0\n");
> + return false;
> + }
> +
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(drm_afbc_check_offset);
> +
> +bool drm_afbc_check_size_align(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> + if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) {
> + DRM_DEBUG_KMS(
> + "AFBC buffer must be aligned to 16 pixels\n"
> + );
> + return false;
> + }
> + break;
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> + /* fall through */
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
> + /* fall through */
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
> + /* fall through */
> + default:
> + DRM_DEBUG_KMS("Unsupported AFBC block size\n");
> + return false;
> + }
> +
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(drm_afbc_check_size_align);
> +
> +bool drm_afbc_get_superblk_wh(u64 modifier, u32 *w, u32 *h)
> +{
> + switch (modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> + *w = 16;
> + *h = 16;
> + break;
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> + *w = 32;
> + *h = 8;
> + break;
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
> + /* fall through */
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
> + /* fall through */
> + default:
> + DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> + modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> + return false;
> + }
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(drm_afbc_get_superblk_wh);
> +
> +bool drm_afbc_check_fb_size_ret(u32 pitch, int bpp,
> + u32 w, u32 h, u32 superblk_w, u32 superblk_h,
> + size_t size, u32 offset, u32 hdr_align,
> + u32 *payload_off, u32 *total_size)
> +{
> + int n_superblks = 0;
> + u32 superblk_sz = 0;
> + u32 afbc_size = 0;
> +
> + n_superblks = (w / superblk_w) * (h / superblk_h);
> + superblk_sz = (bpp * superblk_w * superblk_h) / BITS_PER_BYTE;
> + afbc_size = ALIGN(n_superblks * AFBC_HEADER_SIZE, hdr_align);
> + *payload_off = afbc_size;
> +
> + afbc_size += n_superblks * ALIGN(superblk_sz, AFBC_SUPERBLK_ALIGNMENT);
> + *total_size = afbc_size + offset;
> +
> + if ((w * bpp) != (pitch * BITS_PER_BYTE)) {
> + DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) should be same as width (=%u) * bpp (=%u)\n",
> + pitch * BITS_PER_BYTE, w, bpp
> + );
> + return false;
> + }
> +
> + if (size < afbc_size) {
> + DRM_DEBUG_KMS("buffer size (%zu) too small for AFBC buffer size = %u\n",
> + size, afbc_size
> + );
> +
> + return false;
> + }
> +
> + return true;
> +}
> +EXPORT_SYMBOL(drm_afbc_check_fb_size_ret);
> +
> +bool drm_afbc_check_fb_size(u32 pitch, int bpp,
> + u32 w, u32 h, u32 superblk_w, u32 superblk_h,
> + size_t size, u32 offset, u32 hdr_align)
> +{
> + u32 payload_offset, total_size;
> +
> + return drm_afbc_check_fb_size_ret(pitch, bpp, w, h,
> + superblk_w, superblk_h,
> + size, offset, hdr_align,
> + &payload_offset, &total_size);
> +}
> +EXPORT_SYMBOL(drm_afbc_check_fb_size);
Why don't we have one overall "check afbc parameters against buffer"
function?
Also would be kinda neat if we could wire up that check code directly into
core addfb2 code, since afbc is a cross driver standard, everyone will
have to use the same code. Doing it that way would also solve your
kerneldoc need (since core code checks it directly). We already have an
example for the tiled nv12 format, which is also checked in core.
Cheers, Daniel
> diff --git a/include/drm/drm_afbc.h b/include/drm/drm_afbc.h
> new file mode 100644
> index 000000000000..b28ae2849f96
> --- /dev/null
> +++ b/include/drm/drm_afbc.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * (C) 2019 Collabora Ltd.
> + *
> + * author: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx>
> + *
> + */
> +#ifndef __DRM_AFBC_H__
> +#define __DRM_AFBC_H__
> +
> +#include <linux/types.h>
> +
> +struct drm_device;
> +struct drm_mode_fb_cmd2;
> +struct drm_gem_object;
> +
> +#define AFBC_SUPERBLK_ALIGNMENT 128
> +
> +bool drm_afbc_check_offset(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd);
> +
> +bool drm_afbc_check_size_align(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd);
> +
> +bool drm_afbc_check_fb_size_ret(u32 pitch, int bpp,
> + u32 w, u32 h, u32 superblk_w, u32 superblk_h,
> + size_t size, u32 offset, u32 hdr_align,
> + u32 *payload_off, u32 *total_size);
> +
> +bool drm_afbc_check_fb_size(u32 pitch, int bpp,
> + u32 w, u32 h, u32 superblk_w, u32 superblk_h,
> + size_t size, u32 offset, u32 hdr_align);
> +
> +bool drm_afbc_get_superblk_wh(u64 modifier, u32 *w, u32 *h);
> +
> +#endif /* __DRM_AFBC_H__ */
> --
> 2.17.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
