2018-07-10 13:21:19

by Ayan Halder

[permalink] [raw]
Subject: [PATCH v2 0/4] Add support for Arm Framebuffer Compression(AFBC)

In the current series of patches, we are trying to add support for AFBC
modifiers in malidp. AFBC modifiers adds some constraints to framebuffer
size, alignment, pitch, formats, etc. Here we are trying to add support
for one combination of AFBC modifier ie AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
AFBC_FORMAT_MOD_SPARSE | AFBC_FORMAT_MOD_YTR.
In future, we intend to add support for more combination of AFBC modifiers.
Currently, we are trying to enable a basic support of AFBC in malidp.

Changes from v2:
- Added ack by Maarten Lankhorst <[email protected]>
for patch 1. However, this has been kept in this series in order to help
reviewers review the other patches (which are related to patch 1)
- For patches 2 and 4, replaced DRM_ERROR() with DRM_DEBUG_KMS()
- For patch 3, reworked malidp_de_set_plane_afbc() so as to consolidate
all afbc specific register configuration in this.

Ayan Kumar Halder (4):
drm/arm/malidp: Add modifier definitions for describing Arm
Framebuffer Compression (AFBC).
drm/arm/malidp: Implemented the size validation for AFBC framebuffers
drm/arm/malidp: Set the AFBC register bits if the framebuffer has AFBC
modifier
drm/arm/malidp: Added support for AFBC modifiers for all layers except
DE_SMART

drivers/gpu/drm/arm/malidp_drv.c | 129 +++++++++++++++++++++++++++++++++++-
drivers/gpu/drm/arm/malidp_hw.c | 27 +++++---
drivers/gpu/drm/arm/malidp_hw.h | 7 ++
drivers/gpu/drm/arm/malidp_planes.c | 129 +++++++++++++++++++++++++++++++++---
drivers/gpu/drm/arm/malidp_regs.h | 20 ++++++
include/uapi/drm/drm_fourcc.h | 83 +++++++++++++++++++++++
6 files changed, 373 insertions(+), 22 deletions(-)

--
2.7.4



2018-07-10 13:20:40

by Ayan Halder

[permalink] [raw]
Subject: [PATCH v2 2/4] drm/arm/malidp: Implemented the size validation for AFBC framebuffers

AFBC buffers include additional metadata which increases the required
allocation size. Implement the appropriate size validation and sanity
checking for AFBC buffers.
Added malidp specific function for framebuffer creation. This checks
if the framebuffer has AFBC modifiers and if so, it verifies the
necessary constraints on the size, alignment, offsets and pitch.

Changes from v2:
- Replaced DRM_ERROR() with DRM_DEBUG_KMS() in
malidp_verify_afbc_framebuffer_caps() and malidp_verify_afbc_framebuffer_size()

Signed-off-by: Ayan Kumar halder <[email protected]>
Reviewed-by: Brian Starkey <[email protected]>
Reviewed-by: Liviu Dudau <[email protected]>
---
drivers/gpu/drm/arm/malidp_drv.c | 128 ++++++++++++++++++++++++++++++++++++++-
drivers/gpu/drm/arm/malidp_hw.h | 5 ++
2 files changed, 132 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 8d20faa..262a830 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -35,6 +35,7 @@
#include "malidp_hw.h"

#define MALIDP_CONF_VALID_TIMEOUT 250
+#define AFBC_HEADER_SIZE 16

static void malidp_write_gamma_table(struct malidp_hw_device *hwdev,
u32 data[MALIDP_COEFFTAB_NUM_COEFFS])
@@ -245,8 +246,133 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
.atomic_commit_tail = malidp_atomic_commit_tail,
};

+static bool
+malidp_verify_afbc_framebuffer_caps(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+ const struct drm_format_info *info;
+
+ if ((mode_cmd->modifier[0] >> 56) != DRM_FORMAT_MOD_VENDOR_ARM) {
+ DRM_DEBUG_KMS("Unknown modifier (not Arm)\n");
+ return false;
+ }
+
+ if (mode_cmd->modifier[0] &
+ ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_MOD_VALID_BITS)) {
+ DRM_DEBUG_KMS("Unsupported modifiers\n");
+ return false;
+ }
+
+ info = drm_get_format_info(dev, mode_cmd);
+ if (!info) {
+ DRM_DEBUG_KMS("Unable to get the format information\n");
+ return false;
+ }
+
+ if (info->num_planes != 1) {
+ DRM_DEBUG_KMS("AFBC buffers expect one plane\n");
+ return false;
+ }
+
+ if (mode_cmd->offsets[0] != 0) {
+ DRM_DEBUG_KMS("AFBC buffers' plane offset should be 0\n");
+ return false;
+ }
+
+ 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 buffers must be aligned to 16 pixels\n");
+ return false;
+ }
+ break;
+ default:
+ DRM_DEBUG_KMS("Unsupported AFBC block size\n");
+ return false;
+ }
+
+ return true;
+}
+
+static bool
+malidp_verify_afbc_framebuffer_size(struct drm_device *dev,
+ struct drm_file *file,
+ const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+ int n_superblocks = 0;
+ const struct drm_format_info *info;
+ struct drm_gem_object *objs = NULL;
+ u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
+ u32 afbc_superblock_width = 0, afbc_size = 0;
+
+ switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
+ case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
+ afbc_superblock_height = 16;
+ afbc_superblock_width = 16;
+ break;
+ default:
+ DRM_DEBUG_KMS("AFBC superblock size is not supported\n");
+ return false;
+ }
+
+ info = drm_get_format_info(dev, mode_cmd);
+
+ n_superblocks = (mode_cmd->width / afbc_superblock_width) *
+ (mode_cmd->height / afbc_superblock_height);
+
+ afbc_superblock_size = info->cpp[0] * afbc_superblock_width *
+ afbc_superblock_height;
+
+ afbc_size = ALIGN(n_superblocks * AFBC_HEADER_SIZE, 128);
+
+ if (mode_cmd->width * info->cpp[0] != mode_cmd->pitches[0]) {
+ DRM_DEBUG_KMS("Invalid value of pitch (=%u) should be same as width (=%u) * cpp (=%u)\n",
+ mode_cmd->pitches[0], mode_cmd->width, info->cpp[0]);
+ return false;
+ }
+
+ objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
+ if (!objs) {
+ DRM_DEBUG_KMS("Failed to lookup GEM object\n");
+ return false;
+ }
+
+ if (objs->size < afbc_size) {
+ DRM_DEBUG_KMS("buffer size (%zu) too small for AFBC buffer size = %u\n",
+ objs->size, afbc_size);
+ drm_gem_object_put_unlocked(objs);
+ return false;
+ }
+
+ drm_gem_object_put_unlocked(objs);
+
+ return true;
+}
+
+static bool
+malidp_verify_afbc_framebuffer(struct drm_device *dev, struct drm_file *file,
+ const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+ if (malidp_verify_afbc_framebuffer_caps(dev, mode_cmd))
+ return malidp_verify_afbc_framebuffer_size(dev, file, mode_cmd);
+
+ return false;
+}
+
+struct drm_framebuffer *
+malidp_fb_create(struct drm_device *dev, struct drm_file *file,
+ const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+ if (mode_cmd->modifier[0]) {
+ if (!malidp_verify_afbc_framebuffer(dev, file, mode_cmd))
+ return ERR_PTR(-EINVAL);
+ }
+
+ return drm_gem_fb_create(dev, file, mode_cmd);
+}
+
static const struct drm_mode_config_funcs malidp_mode_config_funcs = {
- .fb_create = drm_gem_fb_create,
+ .fb_create = malidp_fb_create,
.output_poll_changed = drm_fb_helper_output_poll_changed,
.atomic_check = drm_atomic_helper_check,
.atomic_commit = drm_atomic_helper_commit,
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 41f4521..4390243 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -361,4 +361,9 @@ static inline void malidp_se_set_enh_coeffs(struct malidp_hw_device *hwdev)

#define MALIDP_GAMMA_LUT_SIZE 4096

+#define AFBC_MOD_VALID_BITS (AFBC_FORMAT_MOD_BLOCK_SIZE_MASK | \
+ AFBC_FORMAT_MOD_YTR | AFBC_FORMAT_MOD_SPLIT | \
+ AFBC_FORMAT_MOD_SPARSE | AFBC_FORMAT_MOD_CBR | \
+ AFBC_FORMAT_MOD_TILED | AFBC_FORMAT_MOD_SC)
+
#endif /* __MALIDP_HW_H__ */
--
2.7.4


2018-07-10 13:20:42

by Ayan Halder

[permalink] [raw]
Subject: [PATCH v2 4/4] drm/arm/malidp: Added support for AFBC modifiers for all layers except DE_SMART

On planes which support AFBC, expose an AFBC modifier for use with BGR888.

Signed-off-by: Ayan Kumar halder <[email protected]>
Reviewed-by: Brian Starkey <[email protected]>
Reviewed-by: Liviu Dudau <[email protected]>

Changes from v2:
- Removed the gerrit change-id
- Replaced DRM_ERROR() with DRM_DEBUG_KMS() in malidp_format_mod_supported()
to report unsupported modifiers.
---
drivers/gpu/drm/arm/malidp_drv.c | 1 +
drivers/gpu/drm/arm/malidp_planes.c | 46 +++++++++++++++++++++++++++++++++++--
2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 262a830..4f6e52e 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -392,6 +392,7 @@ static int malidp_init(struct drm_device *drm)
drm->mode_config.max_height = hwdev->max_line_size;
drm->mode_config.funcs = &malidp_mode_config_funcs;
drm->mode_config.helper_private = &malidp_mode_config_helpers;
+ drm->mode_config.allow_fb_modifiers = true;

ret = malidp_crtc_init(drm);
if (ret) {
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 0122091..605c5ae 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -124,6 +124,35 @@ static void malidp_plane_atomic_print_state(struct drm_printer *p,
drm_printf(p, "\tn_planes=%u\n", ms->n_planes);
}

+static bool malidp_format_mod_supported(struct drm_plane *plane,
+ u32 format, u64 modifier)
+{
+ if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
+ return false;
+
+ /* All the pixel formats are supported without any modifier */
+ if (modifier == DRM_FORMAT_MOD_LINEAR)
+ return true;
+
+ if ((modifier >> 56) != DRM_FORMAT_MOD_VENDOR_ARM)
+ return false;
+
+ if (modifier &
+ ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_MOD_VALID_BITS)) {
+ DRM_DEBUG_KMS("Unsupported modifiers\n");
+ return false;
+ }
+
+ switch (modifier) {
+ case DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
+ AFBC_FORMAT_MOD_YTR |
+ AFBC_FORMAT_MOD_SPARSE):
+ if (format == DRM_FORMAT_BGR888)
+ return true;
+ }
+ return false;
+}
+
static const struct drm_plane_funcs malidp_de_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
@@ -132,6 +161,7 @@ static const struct drm_plane_funcs malidp_de_plane_funcs = {
.atomic_duplicate_state = malidp_duplicate_plane_state,
.atomic_destroy_state = malidp_destroy_plane_state,
.atomic_print_state = malidp_plane_atomic_print_state,
+ .format_mod_supported = malidp_format_mod_supported,
};

static int malidp_se_check_scaling(struct malidp_plane *mp,
@@ -526,6 +556,13 @@ int malidp_de_planes_init(struct drm_device *drm)
u32 *formats;
int ret, i, j, n;

+ static const u64 modifiers[] = {
+ DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
+ AFBC_FORMAT_MOD_YTR | AFBC_FORMAT_MOD_SPARSE),
+ DRM_FORMAT_MOD_LINEAR,
+ DRM_FORMAT_MOD_INVALID
+ };
+
formats = kcalloc(map->n_pixel_formats, sizeof(*formats), GFP_KERNEL);
if (!formats) {
ret = -ENOMEM;
@@ -549,9 +586,14 @@ int malidp_de_planes_init(struct drm_device *drm)

plane_type = (i == 0) ? DRM_PLANE_TYPE_PRIMARY :
DRM_PLANE_TYPE_OVERLAY;
+
+ /*
+ * All the layers except smart layer supports AFBC modifiers.
+ */
ret = drm_universal_plane_init(drm, &plane->base, crtcs,
- &malidp_de_plane_funcs, formats,
- n, NULL, plane_type, NULL);
+ &malidp_de_plane_funcs, formats, n,
+ (id == DE_SMART) ? NULL : modifiers, plane_type, NULL);
+
if (ret < 0)
goto cleanup;

--
2.7.4


2018-07-10 13:22:35

by Ayan Halder

[permalink] [raw]
Subject: [PATCH v2 1/4] drm/arm/malidp: Add modifier definitions for describing Arm Framebuffer Compression (AFBC).

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.
AFBC has several features which may be supported and/or used, which are
represented using bits in the modifier. Not all combinations are valid,
and different devices or use-cases may support different combinations.

Changes from v2:-
- Added ack by Maarten Lankhorst

Signed-off-by: Rosen Zhelev <[email protected]>
Signed-off-by: Ayan Kumar halder <[email protected]>
Reviewed-by: Brian Starkey <[email protected]>
Reviewed-by: Liviu Dudau <[email protected]>
Reviewed-by: James (Qian) Wang <[email protected]>
Acked-by: Maarten Lankhorst <[email protected]>
---
include/uapi/drm/drm_fourcc.h | 83 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index d5e5235..d43949b 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -183,6 +183,7 @@ extern "C" {
#define DRM_FORMAT_MOD_VENDOR_QCOM 0x05
#define DRM_FORMAT_MOD_VENDOR_VIVANTE 0x06
#define DRM_FORMAT_MOD_VENDOR_BROADCOM 0x07
+#define DRM_FORMAT_MOD_VENDOR_ARM 0x08
/* add more to the end as needed */

#define DRM_FORMAT_RESERVED ((1ULL << 56) - 1)
@@ -485,6 +486,88 @@ extern "C" {
*/
#define DRM_FORMAT_MOD_BROADCOM_UIF fourcc_mod_code(BROADCOM, 6)

+/*
+ * Arm Framebuffer Compression (AFBC) modifiers
+ *
+ * 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.
+ *
+ * AFBC has several features which may be supported and/or used, which are
+ * represented using bits in the modifier. Not all combinations are valid,
+ * and different devices or use-cases may support different combinations.
+ */
+#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) fourcc_mod_code(ARM, __afbc_mode)
+
+/*
+ * AFBC superblock size
+ *
+ * Indicates the superblock size(s) used for the AFBC buffer. The buffer
+ * size (in pixels) must be aligned to a multiple of the superblock size.
+ * Four lowest significant bits(LSBs) are reserved for block size.
+ */
+#define AFBC_FORMAT_MOD_BLOCK_SIZE_MASK 0xf
+#define AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 (1ULL)
+#define AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 (2ULL)
+
+/*
+ * AFBC lossless colorspace transform
+ *
+ * Indicates that the buffer makes use of the AFBC lossless colorspace
+ * transform.
+ */
+#define AFBC_FORMAT_MOD_YTR (1ULL << 4)
+
+/*
+ * AFBC block-split
+ *
+ * Indicates that the payload of each superblock is split. The second
+ * half of the payload is positioned at a predefined offset from the start
+ * of the superblock payload.
+ */
+#define AFBC_FORMAT_MOD_SPLIT (1ULL << 5)
+
+/*
+ * AFBC sparse layout
+ *
+ * This flag indicates that the payload of each superblock must be stored at a
+ * predefined position relative to the other superblocks in the same AFBC
+ * buffer. This order is the same order used by the header buffer. In this mode
+ * each superblock is given the same amount of space as an uncompressed
+ * superblock of the particular format would require, rounding up to the next
+ * multiple of 128 bytes in size.
+ */
+#define AFBC_FORMAT_MOD_SPARSE (1ULL << 6)
+
+/*
+ * AFBC copy-block restrict
+ *
+ * Buffers with this flag must obey the copy-block restriction. The restriction
+ * is such that there are no copy-blocks referring across the border of 8x8
+ * blocks. For the subsampled data the 8x8 limitation is also subsampled.
+ */
+#define AFBC_FORMAT_MOD_CBR (1ULL << 7)
+
+/*
+ * AFBC tiled layout
+ *
+ * The tiled layout groups superblocks in 8x8 or 4x4 tiles, where all
+ * superblocks inside a tile are stored together in memory. 8x8 tiles are used
+ * for pixel formats up to and including 32 bpp while 4x4 tiles are used for
+ * larger bpp formats. The order between the tiles is scan line.
+ * When the tiled layout is used, the buffer size (in pixels) must be aligned
+ * to the tile size.
+ */
+#define AFBC_FORMAT_MOD_TILED (1ULL << 8)
+
+/*
+ * AFBC solid color blocks
+ *
+ * Indicates that the buffer makes use of solid-color blocks, whereby bandwidth
+ * can be reduced if a whole superblock is a single color.
+ */
+#define AFBC_FORMAT_MOD_SC (1ULL << 9)
+
#if defined(__cplusplus)
}
#endif
--
2.7.4


2018-07-10 13:22:54

by Ayan Halder

[permalink] [raw]
Subject: [PATCH v2 3/4] drm/arm/malidp: Set the AFBC register bits if the framebuffer has AFBC modifier

Added the AFBC decoder registers for DP500 , DP550 and DP650.
These registers control the processing of AFBC buffers. It controls various
features like AFBC decoder enable, lossless transformation and block split
as well as setting of the left, right, top and bottom cropping of AFBC buffers
(in number of pixels).
All the layers (except DE_SMART) support framebuffers with AFBC modifiers.
One needs to set the pixel values of the top, left, bottom and right cropping
for the AFBC framebuffer.
Added the functionality in malidp_de_plane_update() to set the various
registers for AFBC decoder, depending on the modifiers.

Changes from v2:-
- Removed the "if (fb->modifier)" check from malidp_de_plane_update()
and added it in malidp_de_set_plane_afbc(). This will consolidate all the
AFBC specific register configurations in a single function ie
malidp_de_set_plane_afbc().

Signed-off-by: Ayan Kumar halder <[email protected]>
Reviewed-by: Brian Starkey <[email protected]>
---
drivers/gpu/drm/arm/malidp_hw.c | 27 +++++++-----
drivers/gpu/drm/arm/malidp_hw.h | 2 +
drivers/gpu/drm/arm/malidp_planes.c | 83 +++++++++++++++++++++++++++++++++----
drivers/gpu/drm/arm/malidp_regs.h | 20 +++++++++
4 files changed, 113 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 4dbf39f..fd6b510 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -76,33 +76,38 @@ static const struct malidp_format_id malidp550_de_formats[] = {

static const struct malidp_layer malidp500_layers[] = {
{ DE_VIDEO1, MALIDP500_DE_LV_BASE, MALIDP500_DE_LV_PTR_BASE,
- MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, ROTATE_ANY },
+ MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, ROTATE_ANY,
+ MALIDP500_DE_LV_AD_CTRL },
{ DE_GRAPHICS1, MALIDP500_DE_LG1_BASE, MALIDP500_DE_LG1_PTR_BASE,
- MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
+ MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, MALIDP500_DE_LG1_AD_CTRL },
{ DE_GRAPHICS2, MALIDP500_DE_LG2_BASE, MALIDP500_DE_LG2_PTR_BASE,
- MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
+ MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, MALIDP500_DE_LG2_AD_CTRL },
};

static const struct malidp_layer malidp550_layers[] = {
{ DE_VIDEO1, MALIDP550_DE_LV1_BASE, MALIDP550_DE_LV1_PTR_BASE,
- MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
+ MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
+ MALIDP550_DE_LV1_AD_CTRL },
{ DE_GRAPHICS1, MALIDP550_DE_LG_BASE, MALIDP550_DE_LG_PTR_BASE,
- MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
+ MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, MALIDP550_DE_LG_AD_CTRL },
{ DE_VIDEO2, MALIDP550_DE_LV2_BASE, MALIDP550_DE_LV2_PTR_BASE,
- MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
+ MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
+ MALIDP550_DE_LV2_AD_CTRL },
{ DE_SMART, MALIDP550_DE_LS_BASE, MALIDP550_DE_LS_PTR_BASE,
- MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE },
+ MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE, 0 },
};

static const struct malidp_layer malidp650_layers[] = {
{ DE_VIDEO1, MALIDP550_DE_LV1_BASE, MALIDP550_DE_LV1_PTR_BASE,
- MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
+ MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
+ MALIDP550_DE_LV1_AD_CTRL },
{ DE_GRAPHICS1, MALIDP550_DE_LG_BASE, MALIDP550_DE_LG_PTR_BASE,
- MALIDP_DE_LG_STRIDE, 0, ROTATE_COMPRESSED },
+ MALIDP_DE_LG_STRIDE, 0, ROTATE_COMPRESSED, MALIDP550_DE_LG_AD_CTRL },
{ DE_VIDEO2, MALIDP550_DE_LV2_BASE, MALIDP550_DE_LV2_PTR_BASE,
- MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
+ MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
+ MALIDP550_DE_LV2_AD_CTRL },
{ DE_SMART, MALIDP550_DE_LS_BASE, MALIDP550_DE_LS_PTR_BASE,
- MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE },
+ MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE, 0 },
};

#define SE_N_SCALING_COEFFS 96
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 4390243..bbe6883 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -67,6 +67,8 @@ struct malidp_layer {
u16 stride_offset; /* offset to the first stride register. */
s16 yuv2rgb_offset; /* offset to the YUV->RGB matrix entries */
enum rotation_features rot; /* type of rotation supported */
+ /* address offset for the AFBC decoder registers */
+ u16 afbc_decoder_offset;
};

enum malidp_scaling_coeff_set {
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 533cdde..0122091 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -330,6 +330,76 @@ static void malidp_de_set_color_encoding(struct malidp_plane *plane,
}
}

+static void malidp_set_plane_base_addr(struct drm_framebuffer *fb,
+ struct malidp_plane *mp,
+ int plane_index)
+{
+ dma_addr_t paddr;
+ u16 ptr;
+ struct drm_plane *plane = &mp->base;
+ bool afbc = fb->modifier ? true : false;
+
+ ptr = mp->layer->ptr + (plane_index << 4);
+
+ /*
+ * For AFBC buffers, cropping is handled by AFBC decoder rather than
+ * pointer manipulation.
+ */
+ if (!afbc) {
+ paddr = drm_fb_cma_get_gem_addr(fb, plane->state,
+ plane_index);
+ } else {
+ struct drm_gem_cma_object *obj;
+
+ obj = drm_fb_cma_get_gem_obj(fb, plane_index);
+
+ if (WARN_ON(!obj))
+ return;
+ paddr = obj->paddr;
+ }
+
+ malidp_hw_write(mp->hwdev, lower_32_bits(paddr), ptr);
+ malidp_hw_write(mp->hwdev, upper_32_bits(paddr), ptr + 4);
+}
+
+static void malidp_de_set_plane_afbc(struct drm_plane *plane)
+{
+ struct malidp_plane *mp;
+ u32 src_w, src_h, val = 0, src_x, src_y;
+ struct drm_framebuffer *fb = plane->state->fb;
+
+ mp = to_malidp_plane(plane);
+
+ if (!fb->modifier) {
+ malidp_hw_write(mp->hwdev, 0, mp->layer->afbc_decoder_offset);
+ return;
+ }
+
+ /* convert src values from Q16 fixed point to integer */
+ src_w = plane->state->src_w >> 16;
+ src_h = plane->state->src_h >> 16;
+ src_x = plane->state->src_x >> 16;
+ src_y = plane->state->src_y >> 16;
+
+ val = ((fb->width - (src_x + src_w)) << MALIDP_AD_CROP_RIGHT_OFFSET) |
+ src_x;
+ malidp_hw_write(mp->hwdev, val,
+ mp->layer->afbc_decoder_offset + MALIDP_AD_CROP_H);
+
+ val = ((fb->height - (src_y + src_h)) << MALIDP_AD_CROP_BOTTOM_OFFSET) |
+ src_y;
+ malidp_hw_write(mp->hwdev, val,
+ mp->layer->afbc_decoder_offset + MALIDP_AD_CROP_V);
+
+ val = MALIDP_AD_EN;
+ if (fb->modifier & AFBC_FORMAT_MOD_SPLIT)
+ val |= MALIDP_AD_BS;
+ if (fb->modifier & AFBC_FORMAT_MOD_YTR)
+ val |= MALIDP_AD_YTR;
+
+ malidp_hw_write(mp->hwdev, val, mp->layer->afbc_decoder_offset);
+}
+
static void malidp_de_plane_update(struct drm_plane *plane,
struct drm_plane_state *old_state)
{
@@ -338,6 +408,7 @@ static void malidp_de_plane_update(struct drm_plane *plane,
u32 src_w, src_h, dest_w, dest_h, val;
int i;
bool format_has_alpha = plane->state->fb->format->has_alpha;
+ struct drm_framebuffer *fb = plane->state->fb;

mp = to_malidp_plane(plane);

@@ -349,15 +420,9 @@ static void malidp_de_plane_update(struct drm_plane *plane,

malidp_hw_write(mp->hwdev, ms->format, mp->layer->base);

- for (i = 0; i < ms->n_planes; i++) {
- /* calculate the offset for the layer's plane registers */
- u16 ptr = mp->layer->ptr + (i << 4);
- dma_addr_t fb_addr = drm_fb_cma_get_gem_addr(plane->state->fb,
- plane->state, i);
+ for (i = 0; i < ms->n_planes; i++)
+ malidp_set_plane_base_addr(fb, mp, i);

- malidp_hw_write(mp->hwdev, lower_32_bits(fb_addr), ptr);
- malidp_hw_write(mp->hwdev, upper_32_bits(fb_addr), ptr + 4);
- }
malidp_de_set_plane_pitches(mp, ms->n_planes,
plane->state->fb->pitches);

@@ -381,6 +446,8 @@ static void malidp_de_plane_update(struct drm_plane *plane,
LAYER_H_VAL(src_w) | LAYER_V_VAL(src_h),
mp->layer->base + MALIDP550_LS_R1_IN_SIZE);

+ malidp_de_set_plane_afbc(plane);
+
/* first clear the rotation bits */
val = malidp_hw_read(mp->hwdev, mp->layer->base + MALIDP_LAYER_CONTROL);
val &= ~LAYER_ROT_MASK;
diff --git a/drivers/gpu/drm/arm/malidp_regs.h b/drivers/gpu/drm/arm/malidp_regs.h
index 149024f..54f4ec5 100644
--- a/drivers/gpu/drm/arm/malidp_regs.h
+++ b/drivers/gpu/drm/arm/malidp_regs.h
@@ -180,10 +180,13 @@
#define MALIDP500_LV_YUV2RGB ((s16)(-0xB8))
#define MALIDP500_DE_LV_BASE 0x00100
#define MALIDP500_DE_LV_PTR_BASE 0x00124
+#define MALIDP500_DE_LV_AD_CTRL 0x00400
#define MALIDP500_DE_LG1_BASE 0x00200
#define MALIDP500_DE_LG1_PTR_BASE 0x0021c
+#define MALIDP500_DE_LG1_AD_CTRL 0x0040c
#define MALIDP500_DE_LG2_BASE 0x00300
#define MALIDP500_DE_LG2_PTR_BASE 0x0031c
+#define MALIDP500_DE_LG2_AD_CTRL 0x00418
#define MALIDP500_SE_BASE 0x00c00
#define MALIDP500_SE_CONTROL 0x00c0c
#define MALIDP500_SE_PTR_BASE 0x00e0c
@@ -208,10 +211,13 @@
#define MALIDP550_LV_YUV2RGB 0x00084
#define MALIDP550_DE_LV1_BASE 0x00100
#define MALIDP550_DE_LV1_PTR_BASE 0x00124
+#define MALIDP550_DE_LV1_AD_CTRL 0x001B8
#define MALIDP550_DE_LV2_BASE 0x00200
#define MALIDP550_DE_LV2_PTR_BASE 0x00224
+#define MALIDP550_DE_LV2_AD_CTRL 0x002B8
#define MALIDP550_DE_LG_BASE 0x00300
#define MALIDP550_DE_LG_PTR_BASE 0x0031c
+#define MALIDP550_DE_LG_AD_CTRL 0x00330
#define MALIDP550_DE_LS_BASE 0x00400
#define MALIDP550_DE_LS_PTR_BASE 0x0042c
#define MALIDP550_DE_PERF_BASE 0x00500
@@ -223,6 +229,20 @@
#define MALIDP550_CONFIG_VALID 0x0c014
#define MALIDP550_CONFIG_ID 0x0ffd4

+/* AFBC register offsets relative to MALIDPXXX_DE_LX_AD_CTRL */
+/* The following register offsets are common for DP500, DP550 and DP650 */
+#define MALIDP_AD_CROP_H 0x4
+#define MALIDP_AD_CROP_V 0x8
+#define MALIDP_AD_END_PTR_LOW 0xc
+#define MALIDP_AD_END_PTR_HIGH 0x10
+
+/* AFBC decoder Registers */
+#define MALIDP_AD_EN BIT(0)
+#define MALIDP_AD_YTR BIT(4)
+#define MALIDP_AD_BS BIT(8)
+#define MALIDP_AD_CROP_RIGHT_OFFSET 16
+#define MALIDP_AD_CROP_BOTTOM_OFFSET 16
+
/*
* Starting with DP550 the register map blocks has been standardised to the
* following layout:
--
2.7.4


2018-07-11 09:33:49

by Liviu Dudau

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] drm/arm/malidp: Set the AFBC register bits if the framebuffer has AFBC modifier

On Tue, Jul 10, 2018 at 02:18:56PM +0100, Ayan Kumar Halder wrote:
> Added the AFBC decoder registers for DP500 , DP550 and DP650.
> These registers control the processing of AFBC buffers. It controls various
> features like AFBC decoder enable, lossless transformation and block split
> as well as setting of the left, right, top and bottom cropping of AFBC buffers
> (in number of pixels).
> All the layers (except DE_SMART) support framebuffers with AFBC modifiers.
> One needs to set the pixel values of the top, left, bottom and right cropping
> for the AFBC framebuffer.
> Added the functionality in malidp_de_plane_update() to set the various
> registers for AFBC decoder, depending on the modifiers.
>
> Changes from v2:-
> - Removed the "if (fb->modifier)" check from malidp_de_plane_update()
> and added it in malidp_de_set_plane_afbc(). This will consolidate all the
> AFBC specific register configurations in a single function ie
> malidp_de_set_plane_afbc().
>
> Signed-off-by: Ayan Kumar halder <[email protected]>
> Reviewed-by: Brian Starkey <[email protected]>

Acked-by: Liviu Dudau <[email protected]>

Thanks,
Liviu

> ---
> drivers/gpu/drm/arm/malidp_hw.c | 27 +++++++-----
> drivers/gpu/drm/arm/malidp_hw.h | 2 +
> drivers/gpu/drm/arm/malidp_planes.c | 83 +++++++++++++++++++++++++++++++++----
> drivers/gpu/drm/arm/malidp_regs.h | 20 +++++++++
> 4 files changed, 113 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> index 4dbf39f..fd6b510 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.c
> +++ b/drivers/gpu/drm/arm/malidp_hw.c
> @@ -76,33 +76,38 @@ static const struct malidp_format_id malidp550_de_formats[] = {
>
> static const struct malidp_layer malidp500_layers[] = {
> { DE_VIDEO1, MALIDP500_DE_LV_BASE, MALIDP500_DE_LV_PTR_BASE,
> - MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, ROTATE_ANY },
> + MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, ROTATE_ANY,
> + MALIDP500_DE_LV_AD_CTRL },
> { DE_GRAPHICS1, MALIDP500_DE_LG1_BASE, MALIDP500_DE_LG1_PTR_BASE,
> - MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
> + MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, MALIDP500_DE_LG1_AD_CTRL },
> { DE_GRAPHICS2, MALIDP500_DE_LG2_BASE, MALIDP500_DE_LG2_PTR_BASE,
> - MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
> + MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, MALIDP500_DE_LG2_AD_CTRL },
> };
>
> static const struct malidp_layer malidp550_layers[] = {
> { DE_VIDEO1, MALIDP550_DE_LV1_BASE, MALIDP550_DE_LV1_PTR_BASE,
> - MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
> + MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
> + MALIDP550_DE_LV1_AD_CTRL },
> { DE_GRAPHICS1, MALIDP550_DE_LG_BASE, MALIDP550_DE_LG_PTR_BASE,
> - MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
> + MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, MALIDP550_DE_LG_AD_CTRL },
> { DE_VIDEO2, MALIDP550_DE_LV2_BASE, MALIDP550_DE_LV2_PTR_BASE,
> - MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
> + MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
> + MALIDP550_DE_LV2_AD_CTRL },
> { DE_SMART, MALIDP550_DE_LS_BASE, MALIDP550_DE_LS_PTR_BASE,
> - MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE },
> + MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE, 0 },
> };
>
> static const struct malidp_layer malidp650_layers[] = {
> { DE_VIDEO1, MALIDP550_DE_LV1_BASE, MALIDP550_DE_LV1_PTR_BASE,
> - MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
> + MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
> + MALIDP550_DE_LV1_AD_CTRL },
> { DE_GRAPHICS1, MALIDP550_DE_LG_BASE, MALIDP550_DE_LG_PTR_BASE,
> - MALIDP_DE_LG_STRIDE, 0, ROTATE_COMPRESSED },
> + MALIDP_DE_LG_STRIDE, 0, ROTATE_COMPRESSED, MALIDP550_DE_LG_AD_CTRL },
> { DE_VIDEO2, MALIDP550_DE_LV2_BASE, MALIDP550_DE_LV2_PTR_BASE,
> - MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
> + MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
> + MALIDP550_DE_LV2_AD_CTRL },
> { DE_SMART, MALIDP550_DE_LS_BASE, MALIDP550_DE_LS_PTR_BASE,
> - MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE },
> + MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE, 0 },
> };
>
> #define SE_N_SCALING_COEFFS 96
> diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
> index 4390243..bbe6883 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.h
> +++ b/drivers/gpu/drm/arm/malidp_hw.h
> @@ -67,6 +67,8 @@ struct malidp_layer {
> u16 stride_offset; /* offset to the first stride register. */
> s16 yuv2rgb_offset; /* offset to the YUV->RGB matrix entries */
> enum rotation_features rot; /* type of rotation supported */
> + /* address offset for the AFBC decoder registers */
> + u16 afbc_decoder_offset;
> };
>
> enum malidp_scaling_coeff_set {
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> index 533cdde..0122091 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -330,6 +330,76 @@ static void malidp_de_set_color_encoding(struct malidp_plane *plane,
> }
> }
>
> +static void malidp_set_plane_base_addr(struct drm_framebuffer *fb,
> + struct malidp_plane *mp,
> + int plane_index)
> +{
> + dma_addr_t paddr;
> + u16 ptr;
> + struct drm_plane *plane = &mp->base;
> + bool afbc = fb->modifier ? true : false;
> +
> + ptr = mp->layer->ptr + (plane_index << 4);
> +
> + /*
> + * For AFBC buffers, cropping is handled by AFBC decoder rather than
> + * pointer manipulation.
> + */
> + if (!afbc) {
> + paddr = drm_fb_cma_get_gem_addr(fb, plane->state,
> + plane_index);
> + } else {
> + struct drm_gem_cma_object *obj;
> +
> + obj = drm_fb_cma_get_gem_obj(fb, plane_index);
> +
> + if (WARN_ON(!obj))
> + return;
> + paddr = obj->paddr;
> + }
> +
> + malidp_hw_write(mp->hwdev, lower_32_bits(paddr), ptr);
> + malidp_hw_write(mp->hwdev, upper_32_bits(paddr), ptr + 4);
> +}
> +
> +static void malidp_de_set_plane_afbc(struct drm_plane *plane)
> +{
> + struct malidp_plane *mp;
> + u32 src_w, src_h, val = 0, src_x, src_y;
> + struct drm_framebuffer *fb = plane->state->fb;
> +
> + mp = to_malidp_plane(plane);
> +
> + if (!fb->modifier) {
> + malidp_hw_write(mp->hwdev, 0, mp->layer->afbc_decoder_offset);
> + return;
> + }
> +
> + /* convert src values from Q16 fixed point to integer */
> + src_w = plane->state->src_w >> 16;
> + src_h = plane->state->src_h >> 16;
> + src_x = plane->state->src_x >> 16;
> + src_y = plane->state->src_y >> 16;
> +
> + val = ((fb->width - (src_x + src_w)) << MALIDP_AD_CROP_RIGHT_OFFSET) |
> + src_x;
> + malidp_hw_write(mp->hwdev, val,
> + mp->layer->afbc_decoder_offset + MALIDP_AD_CROP_H);
> +
> + val = ((fb->height - (src_y + src_h)) << MALIDP_AD_CROP_BOTTOM_OFFSET) |
> + src_y;
> + malidp_hw_write(mp->hwdev, val,
> + mp->layer->afbc_decoder_offset + MALIDP_AD_CROP_V);
> +
> + val = MALIDP_AD_EN;
> + if (fb->modifier & AFBC_FORMAT_MOD_SPLIT)
> + val |= MALIDP_AD_BS;
> + if (fb->modifier & AFBC_FORMAT_MOD_YTR)
> + val |= MALIDP_AD_YTR;
> +
> + malidp_hw_write(mp->hwdev, val, mp->layer->afbc_decoder_offset);
> +}
> +
> static void malidp_de_plane_update(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> @@ -338,6 +408,7 @@ static void malidp_de_plane_update(struct drm_plane *plane,
> u32 src_w, src_h, dest_w, dest_h, val;
> int i;
> bool format_has_alpha = plane->state->fb->format->has_alpha;
> + struct drm_framebuffer *fb = plane->state->fb;
>
> mp = to_malidp_plane(plane);
>
> @@ -349,15 +420,9 @@ static void malidp_de_plane_update(struct drm_plane *plane,
>
> malidp_hw_write(mp->hwdev, ms->format, mp->layer->base);
>
> - for (i = 0; i < ms->n_planes; i++) {
> - /* calculate the offset for the layer's plane registers */
> - u16 ptr = mp->layer->ptr + (i << 4);
> - dma_addr_t fb_addr = drm_fb_cma_get_gem_addr(plane->state->fb,
> - plane->state, i);
> + for (i = 0; i < ms->n_planes; i++)
> + malidp_set_plane_base_addr(fb, mp, i);
>
> - malidp_hw_write(mp->hwdev, lower_32_bits(fb_addr), ptr);
> - malidp_hw_write(mp->hwdev, upper_32_bits(fb_addr), ptr + 4);
> - }
> malidp_de_set_plane_pitches(mp, ms->n_planes,
> plane->state->fb->pitches);
>
> @@ -381,6 +446,8 @@ static void malidp_de_plane_update(struct drm_plane *plane,
> LAYER_H_VAL(src_w) | LAYER_V_VAL(src_h),
> mp->layer->base + MALIDP550_LS_R1_IN_SIZE);
>
> + malidp_de_set_plane_afbc(plane);
> +
> /* first clear the rotation bits */
> val = malidp_hw_read(mp->hwdev, mp->layer->base + MALIDP_LAYER_CONTROL);
> val &= ~LAYER_ROT_MASK;
> diff --git a/drivers/gpu/drm/arm/malidp_regs.h b/drivers/gpu/drm/arm/malidp_regs.h
> index 149024f..54f4ec5 100644
> --- a/drivers/gpu/drm/arm/malidp_regs.h
> +++ b/drivers/gpu/drm/arm/malidp_regs.h
> @@ -180,10 +180,13 @@
> #define MALIDP500_LV_YUV2RGB ((s16)(-0xB8))
> #define MALIDP500_DE_LV_BASE 0x00100
> #define MALIDP500_DE_LV_PTR_BASE 0x00124
> +#define MALIDP500_DE_LV_AD_CTRL 0x00400
> #define MALIDP500_DE_LG1_BASE 0x00200
> #define MALIDP500_DE_LG1_PTR_BASE 0x0021c
> +#define MALIDP500_DE_LG1_AD_CTRL 0x0040c
> #define MALIDP500_DE_LG2_BASE 0x00300
> #define MALIDP500_DE_LG2_PTR_BASE 0x0031c
> +#define MALIDP500_DE_LG2_AD_CTRL 0x00418
> #define MALIDP500_SE_BASE 0x00c00
> #define MALIDP500_SE_CONTROL 0x00c0c
> #define MALIDP500_SE_PTR_BASE 0x00e0c
> @@ -208,10 +211,13 @@
> #define MALIDP550_LV_YUV2RGB 0x00084
> #define MALIDP550_DE_LV1_BASE 0x00100
> #define MALIDP550_DE_LV1_PTR_BASE 0x00124
> +#define MALIDP550_DE_LV1_AD_CTRL 0x001B8
> #define MALIDP550_DE_LV2_BASE 0x00200
> #define MALIDP550_DE_LV2_PTR_BASE 0x00224
> +#define MALIDP550_DE_LV2_AD_CTRL 0x002B8
> #define MALIDP550_DE_LG_BASE 0x00300
> #define MALIDP550_DE_LG_PTR_BASE 0x0031c
> +#define MALIDP550_DE_LG_AD_CTRL 0x00330
> #define MALIDP550_DE_LS_BASE 0x00400
> #define MALIDP550_DE_LS_PTR_BASE 0x0042c
> #define MALIDP550_DE_PERF_BASE 0x00500
> @@ -223,6 +229,20 @@
> #define MALIDP550_CONFIG_VALID 0x0c014
> #define MALIDP550_CONFIG_ID 0x0ffd4
>
> +/* AFBC register offsets relative to MALIDPXXX_DE_LX_AD_CTRL */
> +/* The following register offsets are common for DP500, DP550 and DP650 */
> +#define MALIDP_AD_CROP_H 0x4
> +#define MALIDP_AD_CROP_V 0x8
> +#define MALIDP_AD_END_PTR_LOW 0xc
> +#define MALIDP_AD_END_PTR_HIGH 0x10
> +
> +/* AFBC decoder Registers */
> +#define MALIDP_AD_EN BIT(0)
> +#define MALIDP_AD_YTR BIT(4)
> +#define MALIDP_AD_BS BIT(8)
> +#define MALIDP_AD_CROP_RIGHT_OFFSET 16
> +#define MALIDP_AD_CROP_BOTTOM_OFFSET 16
> +
> /*
> * Starting with DP550 the register map blocks has been standardised to the
> * following layout:
> --
> 2.7.4
>

--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯

2018-07-11 09:35:26

by Liviu Dudau

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] drm/arm/malidp: Added support for AFBC modifiers for all layers except DE_SMART

On Tue, Jul 10, 2018 at 02:18:57PM +0100, Ayan Kumar Halder wrote:
> On planes which support AFBC, expose an AFBC modifier for use with BGR888.
>
> Signed-off-by: Ayan Kumar halder <[email protected]>
> Reviewed-by: Brian Starkey <[email protected]>
> Reviewed-by: Liviu Dudau <[email protected]>

Acked-by: Liviu Dudau <[email protected]>

>
> Changes from v2:
> - Removed the gerrit change-id
> - Replaced DRM_ERROR() with DRM_DEBUG_KMS() in malidp_format_mod_supported()
> to report unsupported modifiers.
> ---
> drivers/gpu/drm/arm/malidp_drv.c | 1 +
> drivers/gpu/drm/arm/malidp_planes.c | 46 +++++++++++++++++++++++++++++++++++--
> 2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 262a830..4f6e52e 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -392,6 +392,7 @@ static int malidp_init(struct drm_device *drm)
> drm->mode_config.max_height = hwdev->max_line_size;
> drm->mode_config.funcs = &malidp_mode_config_funcs;
> drm->mode_config.helper_private = &malidp_mode_config_helpers;
> + drm->mode_config.allow_fb_modifiers = true;
>
> ret = malidp_crtc_init(drm);
> if (ret) {
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> index 0122091..605c5ae 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -124,6 +124,35 @@ static void malidp_plane_atomic_print_state(struct drm_printer *p,
> drm_printf(p, "\tn_planes=%u\n", ms->n_planes);
> }
>
> +static bool malidp_format_mod_supported(struct drm_plane *plane,
> + u32 format, u64 modifier)
> +{
> + if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
> + return false;
> +
> + /* All the pixel formats are supported without any modifier */
> + if (modifier == DRM_FORMAT_MOD_LINEAR)
> + return true;
> +
> + if ((modifier >> 56) != DRM_FORMAT_MOD_VENDOR_ARM)
> + return false;
> +
> + if (modifier &
> + ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_MOD_VALID_BITS)) {
> + DRM_DEBUG_KMS("Unsupported modifiers\n");
> + return false;
> + }
> +
> + switch (modifier) {
> + case DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
> + AFBC_FORMAT_MOD_YTR |
> + AFBC_FORMAT_MOD_SPARSE):
> + if (format == DRM_FORMAT_BGR888)
> + return true;
> + }
> + return false;
> +}
> +
> static const struct drm_plane_funcs malidp_de_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> @@ -132,6 +161,7 @@ static const struct drm_plane_funcs malidp_de_plane_funcs = {
> .atomic_duplicate_state = malidp_duplicate_plane_state,
> .atomic_destroy_state = malidp_destroy_plane_state,
> .atomic_print_state = malidp_plane_atomic_print_state,
> + .format_mod_supported = malidp_format_mod_supported,
> };
>
> static int malidp_se_check_scaling(struct malidp_plane *mp,
> @@ -526,6 +556,13 @@ int malidp_de_planes_init(struct drm_device *drm)
> u32 *formats;
> int ret, i, j, n;
>
> + static const u64 modifiers[] = {
> + DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
> + AFBC_FORMAT_MOD_YTR | AFBC_FORMAT_MOD_SPARSE),
> + DRM_FORMAT_MOD_LINEAR,
> + DRM_FORMAT_MOD_INVALID
> + };
> +
> formats = kcalloc(map->n_pixel_formats, sizeof(*formats), GFP_KERNEL);
> if (!formats) {
> ret = -ENOMEM;
> @@ -549,9 +586,14 @@ int malidp_de_planes_init(struct drm_device *drm)
>
> plane_type = (i == 0) ? DRM_PLANE_TYPE_PRIMARY :
> DRM_PLANE_TYPE_OVERLAY;
> +
> + /*
> + * All the layers except smart layer supports AFBC modifiers.
> + */
> ret = drm_universal_plane_init(drm, &plane->base, crtcs,
> - &malidp_de_plane_funcs, formats,
> - n, NULL, plane_type, NULL);
> + &malidp_de_plane_funcs, formats, n,
> + (id == DE_SMART) ? NULL : modifiers, plane_type, NULL);
> +
> if (ret < 0)
> goto cleanup;
>
> --
> 2.7.4
>

--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯

2018-07-11 10:06:20

by Liviu Dudau

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] drm/arm/malidp: Implemented the size validation for AFBC framebuffers

On Tue, Jul 10, 2018 at 02:18:55PM +0100, Ayan Kumar Halder wrote:
> AFBC buffers include additional metadata which increases the required
> allocation size. Implement the appropriate size validation and sanity
> checking for AFBC buffers.
> Added malidp specific function for framebuffer creation. This checks
> if the framebuffer has AFBC modifiers and if so, it verifies the
> necessary constraints on the size, alignment, offsets and pitch.
>
> Changes from v2:
> - Replaced DRM_ERROR() with DRM_DEBUG_KMS() in
> malidp_verify_afbc_framebuffer_caps() and malidp_verify_afbc_framebuffer_size()
>
> Signed-off-by: Ayan Kumar halder <[email protected]>
> Reviewed-by: Brian Starkey <[email protected]>
> Reviewed-by: Liviu Dudau <[email protected]>

Acked-by: Liviu Dudau <[email protected]>

Thanks,
Liviu

> ---
> drivers/gpu/drm/arm/malidp_drv.c | 128 ++++++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/arm/malidp_hw.h | 5 ++
> 2 files changed, 132 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 8d20faa..262a830 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -35,6 +35,7 @@
> #include "malidp_hw.h"
>
> #define MALIDP_CONF_VALID_TIMEOUT 250
> +#define AFBC_HEADER_SIZE 16
>
> static void malidp_write_gamma_table(struct malidp_hw_device *hwdev,
> u32 data[MALIDP_COEFFTAB_NUM_COEFFS])
> @@ -245,8 +246,133 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
> .atomic_commit_tail = malidp_atomic_commit_tail,
> };
>
> +static bool
> +malidp_verify_afbc_framebuffer_caps(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + const struct drm_format_info *info;
> +
> + if ((mode_cmd->modifier[0] >> 56) != DRM_FORMAT_MOD_VENDOR_ARM) {
> + DRM_DEBUG_KMS("Unknown modifier (not Arm)\n");
> + return false;
> + }
> +
> + if (mode_cmd->modifier[0] &
> + ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_MOD_VALID_BITS)) {
> + DRM_DEBUG_KMS("Unsupported modifiers\n");
> + return false;
> + }
> +
> + info = drm_get_format_info(dev, mode_cmd);
> + if (!info) {
> + DRM_DEBUG_KMS("Unable to get the format information\n");
> + return false;
> + }
> +
> + if (info->num_planes != 1) {
> + DRM_DEBUG_KMS("AFBC buffers expect one plane\n");
> + return false;
> + }
> +
> + if (mode_cmd->offsets[0] != 0) {
> + DRM_DEBUG_KMS("AFBC buffers' plane offset should be 0\n");
> + return false;
> + }
> +
> + 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 buffers must be aligned to 16 pixels\n");
> + return false;
> + }
> + break;
> + default:
> + DRM_DEBUG_KMS("Unsupported AFBC block size\n");
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static bool
> +malidp_verify_afbc_framebuffer_size(struct drm_device *dev,
> + struct drm_file *file,
> + const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + int n_superblocks = 0;
> + const struct drm_format_info *info;
> + struct drm_gem_object *objs = NULL;
> + u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
> + u32 afbc_superblock_width = 0, afbc_size = 0;
> +
> + switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> + afbc_superblock_height = 16;
> + afbc_superblock_width = 16;
> + break;
> + default:
> + DRM_DEBUG_KMS("AFBC superblock size is not supported\n");
> + return false;
> + }
> +
> + info = drm_get_format_info(dev, mode_cmd);
> +
> + n_superblocks = (mode_cmd->width / afbc_superblock_width) *
> + (mode_cmd->height / afbc_superblock_height);
> +
> + afbc_superblock_size = info->cpp[0] * afbc_superblock_width *
> + afbc_superblock_height;
> +
> + afbc_size = ALIGN(n_superblocks * AFBC_HEADER_SIZE, 128);
> +
> + if (mode_cmd->width * info->cpp[0] != mode_cmd->pitches[0]) {
> + DRM_DEBUG_KMS("Invalid value of pitch (=%u) should be same as width (=%u) * cpp (=%u)\n",
> + mode_cmd->pitches[0], mode_cmd->width, info->cpp[0]);
> + return false;
> + }
> +
> + objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
> + if (!objs) {
> + DRM_DEBUG_KMS("Failed to lookup GEM object\n");
> + return false;
> + }
> +
> + if (objs->size < afbc_size) {
> + DRM_DEBUG_KMS("buffer size (%zu) too small for AFBC buffer size = %u\n",
> + objs->size, afbc_size);
> + drm_gem_object_put_unlocked(objs);
> + return false;
> + }
> +
> + drm_gem_object_put_unlocked(objs);
> +
> + return true;
> +}
> +
> +static bool
> +malidp_verify_afbc_framebuffer(struct drm_device *dev, struct drm_file *file,
> + const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + if (malidp_verify_afbc_framebuffer_caps(dev, mode_cmd))
> + return malidp_verify_afbc_framebuffer_size(dev, file, mode_cmd);
> +
> + return false;
> +}
> +
> +struct drm_framebuffer *
> +malidp_fb_create(struct drm_device *dev, struct drm_file *file,
> + const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + if (mode_cmd->modifier[0]) {
> + if (!malidp_verify_afbc_framebuffer(dev, file, mode_cmd))
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return drm_gem_fb_create(dev, file, mode_cmd);
> +}
> +
> static const struct drm_mode_config_funcs malidp_mode_config_funcs = {
> - .fb_create = drm_gem_fb_create,
> + .fb_create = malidp_fb_create,
> .output_poll_changed = drm_fb_helper_output_poll_changed,
> .atomic_check = drm_atomic_helper_check,
> .atomic_commit = drm_atomic_helper_commit,
> diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
> index 41f4521..4390243 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.h
> +++ b/drivers/gpu/drm/arm/malidp_hw.h
> @@ -361,4 +361,9 @@ static inline void malidp_se_set_enh_coeffs(struct malidp_hw_device *hwdev)
>
> #define MALIDP_GAMMA_LUT_SIZE 4096
>
> +#define AFBC_MOD_VALID_BITS (AFBC_FORMAT_MOD_BLOCK_SIZE_MASK | \
> + AFBC_FORMAT_MOD_YTR | AFBC_FORMAT_MOD_SPLIT | \
> + AFBC_FORMAT_MOD_SPARSE | AFBC_FORMAT_MOD_CBR | \
> + AFBC_FORMAT_MOD_TILED | AFBC_FORMAT_MOD_SC)
> +
> #endif /* __MALIDP_HW_H__ */
> --
> 2.7.4
>

--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯

2018-09-08 14:02:00

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Add support for Arm Framebuffer Compression(AFBC)

Hi Ayan,

On 10/07/2018 15:18, Ayan Kumar Halder wrote:
> In the current series of patches, we are trying to add support for AFBC
> modifiers in malidp. AFBC modifiers adds some constraints to framebuffer
> size, alignment, pitch, formats, etc. Here we are trying to add support
> for one combination of AFBC modifier ie AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
> AFBC_FORMAT_MOD_SPARSE | AFBC_FORMAT_MOD_YTR.
> In future, we intend to add support for more combination of AFBC modifiers.
> Currently, we are trying to enable a basic support of AFBC in malidp.

Thanks for pushing AFBC support, this will help supporting it on other SoCs implementing support
like Amlogic, Rockchip or Samsung.

I have one question, is there a way to generate such AFBC buffers without the Mali GPU ?
I mean, is there a way to generate some sample buffers with some of the modifier features
to validate it without having the complete Mali GPU -> DRM chain ?

Thanks in advance,
Neil

>
> Changes from v2:
> - Added ack by Maarten Lankhorst <[email protected]>
> for patch 1. However, this has been kept in this series in order to help
> reviewers review the other patches (which are related to patch 1)
> - For patches 2 and 4, replaced DRM_ERROR() with DRM_DEBUG_KMS()
> - For patch 3, reworked malidp_de_set_plane_afbc() so as to consolidate
> all afbc specific register configuration in this.
>
> Ayan Kumar Halder (4):
> drm/arm/malidp: Add modifier definitions for describing Arm
> Framebuffer Compression (AFBC).
> drm/arm/malidp: Implemented the size validation for AFBC framebuffers
> drm/arm/malidp: Set the AFBC register bits if the framebuffer has AFBC
> modifier
> drm/arm/malidp: Added support for AFBC modifiers for all layers except
> DE_SMART
>
> drivers/gpu/drm/arm/malidp_drv.c | 129 +++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/arm/malidp_hw.c | 27 +++++---
> drivers/gpu/drm/arm/malidp_hw.h | 7 ++
> drivers/gpu/drm/arm/malidp_planes.c | 129 +++++++++++++++++++++++++++++++++---
> drivers/gpu/drm/arm/malidp_regs.h | 20 ++++++
> include/uapi/drm/drm_fourcc.h | 83 +++++++++++++++++++++++
> 6 files changed, 373 insertions(+), 22 deletions(-)
>


2018-09-09 12:06:30

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Add support for Arm Framebuffer Compression(AFBC)

On Sat, Sep 08, 2018 at 03:58:53PM +0200, Neil Armstrong wrote:
> Hi Ayan,
>
> On 10/07/2018 15:18, Ayan Kumar Halder wrote:
> > In the current series of patches, we are trying to add support for AFBC
> > modifiers in malidp. AFBC modifiers adds some constraints to framebuffer
> > size, alignment, pitch, formats, etc. Here we are trying to add support
> > for one combination of AFBC modifier ie AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
> > AFBC_FORMAT_MOD_SPARSE | AFBC_FORMAT_MOD_YTR.
> > In future, we intend to add support for more combination of AFBC modifiers.
> > Currently, we are trying to enable a basic support of AFBC in malidp.
>
> Thanks for pushing AFBC support, this will help supporting it on other SoCs implementing support
> like Amlogic, Rockchip or Samsung.
>
> I have one question, is there a way to generate such AFBC buffers without the Mali GPU ?
> I mean, is there a way to generate some sample buffers with some of the modifier features
> to validate it without having the complete Mali GPU -> DRM chain ?

An igt would be perfect. We've done that for i915 compressed buffers. Note
that it just needs to be an afbc buffer, not actually compressed. Setting
all the bits to indicate "uncompressed" for each block is what we did for
the i915 test. As long as the igt uses DRIVER_GENERIC and kms driver could
then use it to validate the basics of afbc support.
-Daniel

>
> Thanks in advance,
> Neil
>
> >
> > Changes from v2:
> > - Added ack by Maarten Lankhorst <[email protected]>
> > for patch 1. However, this has been kept in this series in order to help
> > reviewers review the other patches (which are related to patch 1)
> > - For patches 2 and 4, replaced DRM_ERROR() with DRM_DEBUG_KMS()
> > - For patch 3, reworked malidp_de_set_plane_afbc() so as to consolidate
> > all afbc specific register configuration in this.
> >
> > Ayan Kumar Halder (4):
> > drm/arm/malidp: Add modifier definitions for describing Arm
> > Framebuffer Compression (AFBC).
> > drm/arm/malidp: Implemented the size validation for AFBC framebuffers
> > drm/arm/malidp: Set the AFBC register bits if the framebuffer has AFBC
> > modifier
> > drm/arm/malidp: Added support for AFBC modifiers for all layers except
> > DE_SMART
> >
> > drivers/gpu/drm/arm/malidp_drv.c | 129 +++++++++++++++++++++++++++++++++++-
> > drivers/gpu/drm/arm/malidp_hw.c | 27 +++++---
> > drivers/gpu/drm/arm/malidp_hw.h | 7 ++
> > drivers/gpu/drm/arm/malidp_planes.c | 129 +++++++++++++++++++++++++++++++++---
> > drivers/gpu/drm/arm/malidp_regs.h | 20 ++++++
> > include/uapi/drm/drm_fourcc.h | 83 +++++++++++++++++++++++
> > 6 files changed, 373 insertions(+), 22 deletions(-)
> >
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2018-09-10 12:29:20

by Ville Syrjälä

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Add support for Arm Framebuffer Compression(AFBC)

On Sun, Sep 09, 2018 at 02:03:57PM +0200, Daniel Vetter wrote:
> On Sat, Sep 08, 2018 at 03:58:53PM +0200, Neil Armstrong wrote:
> > Hi Ayan,
> >
> > On 10/07/2018 15:18, Ayan Kumar Halder wrote:
> > > In the current series of patches, we are trying to add support for AFBC
> > > modifiers in malidp. AFBC modifiers adds some constraints to framebuffer
> > > size, alignment, pitch, formats, etc. Here we are trying to add support
> > > for one combination of AFBC modifier ie AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
> > > AFBC_FORMAT_MOD_SPARSE | AFBC_FORMAT_MOD_YTR.
> > > In future, we intend to add support for more combination of AFBC modifiers.
> > > Currently, we are trying to enable a basic support of AFBC in malidp.
> >
> > Thanks for pushing AFBC support, this will help supporting it on other SoCs implementing support
> > like Amlogic, Rockchip or Samsung.
> >
> > I have one question, is there a way to generate such AFBC buffers without the Mali GPU ?
> > I mean, is there a way to generate some sample buffers with some of the modifier features
> > to validate it without having the complete Mali GPU -> DRM chain ?
>
> An igt would be perfect. We've done that for i915 compressed buffers. Note
> that it just needs to be an afbc buffer, not actually compressed. Setting
> all the bits to indicate "uncompressed" for each block is what we did for
> the i915 test.

Actually no. The i915 test does try to put some compressed data into the
buffer.

I have also a pending patch series [1] that allows us to render to
compressed buffers with cairo by compressing the results using the
GPU.

[1] https://patchwork.freedesktop.org/series/46876/

--
Ville Syrj?l?
Intel