* [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
@ 2022-10-12 19:12 ` Justin Green
0 siblings, 0 replies; 11+ messages in thread
From: Justin Green @ 2022-10-12 19:12 UTC (permalink / raw)
To: dri-devel, linux-mediatek
Cc: angelogioacchino.delregno, jason-jh.lin, justin.yeh, wenst,
chunkuang.hu, p.zabel, airlied, matthias.bgg, daniel,
Justin Green
Tested on MT8195 and confirmed both correct video output and improved DRAM
bandwidth performance.
v3:
* Replaced pitch bitshift math with union based approach.
* Refactored overlay register writes to shared code between non-AFBC and
AFBC.
* Minor code cleanups.
v2:
* Marked mtk_ovl_set_afbc as static.
* Reflowed some lines to fit column limit.
Signed-off-by: Justin Green <greenjustin@chromium.org>
---
drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 90 +++++++++++++++++++++++-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 37 +++++++++-
drivers/gpu/drm/mediatek/mtk_drm_plane.h | 8 +++
3 files changed, 131 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 002b0f6cae1a..3f89b5f5064f 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -29,17 +29,24 @@
#define DISP_REG_OVL_DATAPATH_CON 0x0024
#define OVL_LAYER_SMI_ID_EN BIT(0)
#define OVL_BGCLR_SEL_IN BIT(2)
+#define OVL_LAYER_AFBC_EN(n) BIT(4+n)
#define DISP_REG_OVL_ROI_BGCLR 0x0028
#define DISP_REG_OVL_SRC_CON 0x002c
#define DISP_REG_OVL_CON(n) (0x0030 + 0x20 * (n))
#define DISP_REG_OVL_SRC_SIZE(n) (0x0038 + 0x20 * (n))
#define DISP_REG_OVL_OFFSET(n) (0x003c + 0x20 * (n))
+#define DISP_REG_OVL_PITCH_MSB(n) (0x0040 + 0x20 * (n))
+#define OVL_PITCH_MSB_2ND_SUBBUF BIT(16)
+#define OVL_PITCH_MSB_YUV_TRANS BIT(20)
#define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n))
+#define DISP_REG_OVL_CLIP(n) (0x004c + 0x20 * (n))
#define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n))
#define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n))
#define DISP_REG_OVL_ADDR_MT2701 0x0040
#define DISP_REG_OVL_ADDR_MT8173 0x0f40
#define DISP_REG_OVL_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n))
+#define DISP_REG_OVL_HDR_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n) + 0x04)
+#define DISP_REG_OVL_HDR_PITCH(ovl, n) ((ovl)->data->addr + 0x20 * (n) + 0x08)
#define GMC_THRESHOLD_BITS 16
#define GMC_THRESHOLD_HIGH ((1 << GMC_THRESHOLD_BITS) / 4)
@@ -67,6 +74,7 @@ struct mtk_disp_ovl_data {
unsigned int layer_nr;
bool fmt_rgb565_is_0;
bool smi_id_en;
+ bool supports_afbc;
};
/*
@@ -172,7 +180,22 @@ void mtk_ovl_stop(struct device *dev)
reg = reg & ~OVL_LAYER_SMI_ID_EN;
writel_relaxed(reg, ovl->regs + DISP_REG_OVL_DATAPATH_CON);
}
+}
+
+static void mtk_ovl_set_afbc(struct device *dev, struct cmdq_pkt *cmdq_pkt,
+ int idx, bool enabled)
+{
+ struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
+ unsigned int reg;
+ reg = readl(ovl->regs + DISP_REG_OVL_DATAPATH_CON);
+ if (enabled)
+ reg = reg | OVL_LAYER_AFBC_EN(idx);
+ else
+ reg = reg & ~OVL_LAYER_AFBC_EN(idx);
+
+ mtk_ddp_write_relaxed(cmdq_pkt, reg, &ovl->cmdq_reg,
+ ovl->regs, DISP_REG_OVL_DATAPATH_CON);
}
void mtk_ovl_config(struct device *dev, unsigned int w,
@@ -226,6 +249,32 @@ int mtk_ovl_layer_check(struct device *dev, unsigned int idx,
if (state->fb->format->is_yuv && rotation != 0)
return -EINVAL;
+ if (state->fb->modifier) {
+ unsigned long long modifier;
+ unsigned int fourcc;
+ struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
+
+ if (!ovl->data->supports_afbc)
+ return -EINVAL;
+
+ modifier = state->fb->modifier;
+
+ if (modifier != DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
+ AFBC_FORMAT_MOD_SPLIT |
+ AFBC_FORMAT_MOD_SPARSE))
+ return -EINVAL;
+
+ fourcc = state->fb->format->format;
+ if (fourcc != DRM_FORMAT_BGRA8888 &&
+ fourcc != DRM_FORMAT_ABGR8888 &&
+ fourcc != DRM_FORMAT_ARGB8888 &&
+ fourcc != DRM_FORMAT_XRGB8888 &&
+ fourcc != DRM_FORMAT_XBGR8888 &&
+ fourcc != DRM_FORMAT_RGB888 &&
+ fourcc != DRM_FORMAT_BGR888)
+ return -EINVAL;
+ }
+
state->rotation = rotation;
return 0;
@@ -310,11 +359,23 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
struct mtk_plane_pending_state *pending = &state->pending;
unsigned int addr = pending->addr;
- unsigned int pitch = pending->pitch & 0xffff;
+ unsigned int hdr_addr = pending->hdr_addr;
+ unsigned int pitch = pending->pitch;
+ unsigned int hdr_pitch = pending->hdr_pitch;
unsigned int fmt = pending->format;
unsigned int offset = (pending->y << 16) | pending->x;
unsigned int src_size = (pending->height << 16) | pending->width;
unsigned int con;
+ bool is_afbc = pending->modifier;
+ union overlay_pitch {
+ struct split_pitch {
+ u16 lsb;
+ u16 msb;
+ } split_pitch;
+ u32 pitch;
+ } overlay_pitch;
+
+ overlay_pitch.pitch = pitch;
if (!pending->enable) {
mtk_ovl_layer_off(dev, idx, cmdq_pkt);
@@ -335,9 +396,10 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
addr += pending->pitch - 1;
}
+ mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc);
mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs,
DISP_REG_OVL_CON(idx));
- mtk_ddp_write_relaxed(cmdq_pkt, pitch, &ovl->cmdq_reg, ovl->regs,
+ mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb, &ovl->cmdq_reg, ovl->regs,
DISP_REG_OVL_PITCH(idx));
mtk_ddp_write_relaxed(cmdq_pkt, src_size, &ovl->cmdq_reg, ovl->regs,
DISP_REG_OVL_SRC_SIZE(idx));
@@ -345,6 +407,19 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
DISP_REG_OVL_OFFSET(idx));
mtk_ddp_write_relaxed(cmdq_pkt, addr, &ovl->cmdq_reg, ovl->regs,
DISP_REG_OVL_ADDR(ovl, idx));
+ if (is_afbc) {
+ mtk_ddp_write_relaxed(cmdq_pkt, hdr_addr, &ovl->cmdq_reg, ovl->regs,
+ DISP_REG_OVL_HDR_ADDR(ovl, idx));
+ mtk_ddp_write_relaxed(cmdq_pkt,
+ OVL_PITCH_MSB_2ND_SUBBUF | overlay_pitch.split_pitch.msb,
+ &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH_MSB(idx));
+ mtk_ddp_write_relaxed(cmdq_pkt, hdr_pitch, &ovl->cmdq_reg, ovl->regs,
+ DISP_REG_OVL_HDR_PITCH(ovl, idx));
+ } else {
+ mtk_ddp_write_relaxed(cmdq_pkt,
+ overlay_pitch.split_pitch.msb,
+ &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH_MSB(idx));
+ }
mtk_ovl_layer_on(dev, idx, cmdq_pkt);
}
@@ -492,6 +567,15 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = {
.smi_id_en = true,
};
+static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
+ .addr = DISP_REG_OVL_ADDR_MT8173,
+ .gmc_bits = 10,
+ .layer_nr = 4,
+ .fmt_rgb565_is_0 = true,
+ .smi_id_en = true,
+ .supports_afbc = true,
+};
+
static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
{ .compatible = "mediatek,mt2701-disp-ovl",
.data = &mt2701_ovl_driver_data},
@@ -505,6 +589,8 @@ static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
.data = &mt8192_ovl_driver_data},
{ .compatible = "mediatek,mt8192-disp-ovl-2l",
.data = &mt8192_ovl_2l_driver_data},
+ { .compatible = "mediatek,mt8195-disp-ovl",
+ .data = &mt8195_ovl_driver_data},
{},
};
MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 5c0d9ce69931..a285b9ff5081 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -12,6 +12,7 @@
#include <drm/drm_framebuffer.h>
#include <drm/drm_gem_atomic_helper.h>
#include <drm/drm_plane_helper.h>
+#include <linux/align.h>
#include "mtk_drm_crtc.h"
#include "mtk_drm_ddp_comp.h"
@@ -52,6 +53,7 @@ static void mtk_plane_reset(struct drm_plane *plane)
state->base.plane = plane;
state->pending.format = DRM_FORMAT_RGB565;
+ state->pending.modifier = 0;
}
static struct drm_plane_state *mtk_plane_duplicate_state(struct drm_plane *plane)
@@ -120,21 +122,52 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
struct drm_gem_object *gem;
struct mtk_drm_gem_obj *mtk_gem;
unsigned int pitch, format;
+ unsigned long long modifier;
dma_addr_t addr;
+ dma_addr_t hdr_addr = 0;
+ unsigned int hdr_pitch = 0;
gem = fb->obj[0];
mtk_gem = to_mtk_gem_obj(gem);
addr = mtk_gem->dma_addr;
pitch = fb->pitches[0];
format = fb->format->format;
+ modifier = fb->modifier;
- addr += (new_state->src.x1 >> 16) * fb->format->cpp[0];
- addr += (new_state->src.y1 >> 16) * pitch;
+ if (!modifier) {
+ addr += (new_state->src.x1 >> 16) * fb->format->cpp[0];
+ addr += (new_state->src.y1 >> 16) * pitch;
+ } else {
+ int width_in_blocks = ALIGN(fb->width, AFBC_DATA_BLOCK_WIDTH)
+ / AFBC_DATA_BLOCK_WIDTH;
+ int height_in_blocks = ALIGN(fb->height, AFBC_DATA_BLOCK_HEIGHT)
+ / AFBC_DATA_BLOCK_HEIGHT;
+ int x_offset_in_blocks = (new_state->src.x1 >> 16) / AFBC_DATA_BLOCK_WIDTH;
+ int y_offset_in_blocks = (new_state->src.y1 >> 16) / AFBC_DATA_BLOCK_HEIGHT;
+ int hdr_size;
+
+ hdr_pitch = width_in_blocks * AFBC_HEADER_BLOCK_SIZE;
+ pitch = width_in_blocks * AFBC_DATA_BLOCK_WIDTH *
+ AFBC_DATA_BLOCK_HEIGHT * fb->format->cpp[0];
+
+ hdr_size = ALIGN(hdr_pitch * height_in_blocks, AFBC_HEADER_ALIGNMENT);
+
+ hdr_addr = addr + hdr_pitch * y_offset_in_blocks +
+ AFBC_HEADER_BLOCK_SIZE * x_offset_in_blocks;
+ /* The data plane is offset by 1 additional block. */
+ addr = addr + hdr_size +
+ pitch * y_offset_in_blocks +
+ AFBC_DATA_BLOCK_WIDTH * AFBC_DATA_BLOCK_HEIGHT *
+ fb->format->cpp[0] * (x_offset_in_blocks + 1);
+ }
mtk_plane_state->pending.enable = true;
mtk_plane_state->pending.pitch = pitch;
+ mtk_plane_state->pending.hdr_pitch = hdr_pitch;
mtk_plane_state->pending.format = format;
+ mtk_plane_state->pending.modifier = modifier;
mtk_plane_state->pending.addr = addr;
+ mtk_plane_state->pending.hdr_addr = hdr_addr;
mtk_plane_state->pending.x = new_state->dst.x1;
mtk_plane_state->pending.y = new_state->dst.y1;
mtk_plane_state->pending.width = drm_rect_width(&new_state->dst);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
index 2d5ec66e3df1..8f39011cdbfc 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
@@ -10,12 +10,20 @@
#include <drm/drm_crtc.h>
#include <linux/types.h>
+#define AFBC_DATA_BLOCK_WIDTH 32
+#define AFBC_DATA_BLOCK_HEIGHT 8
+#define AFBC_HEADER_BLOCK_SIZE 16
+#define AFBC_HEADER_ALIGNMENT 1024
+
struct mtk_plane_pending_state {
bool config;
bool enable;
dma_addr_t addr;
+ dma_addr_t hdr_addr;
unsigned int pitch;
+ unsigned int hdr_pitch;
unsigned int format;
+ unsigned long long modifier;
unsigned int x;
unsigned int y;
unsigned int width;
--
2.38.0.rc1.362.ged0d419d3c-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
@ 2022-10-12 19:12 ` Justin Green
0 siblings, 0 replies; 11+ messages in thread
From: Justin Green @ 2022-10-12 19:12 UTC (permalink / raw)
To: dri-devel, linux-mediatek
Cc: chunkuang.hu, Justin Green, airlied, jason-jh.lin, justin.yeh,
wenst, matthias.bgg, angelogioacchino.delregno
Tested on MT8195 and confirmed both correct video output and improved DRAM
bandwidth performance.
v3:
* Replaced pitch bitshift math with union based approach.
* Refactored overlay register writes to shared code between non-AFBC and
AFBC.
* Minor code cleanups.
v2:
* Marked mtk_ovl_set_afbc as static.
* Reflowed some lines to fit column limit.
Signed-off-by: Justin Green <greenjustin@chromium.org>
---
drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 90 +++++++++++++++++++++++-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 37 +++++++++-
drivers/gpu/drm/mediatek/mtk_drm_plane.h | 8 +++
3 files changed, 131 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 002b0f6cae1a..3f89b5f5064f 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -29,17 +29,24 @@
#define DISP_REG_OVL_DATAPATH_CON 0x0024
#define OVL_LAYER_SMI_ID_EN BIT(0)
#define OVL_BGCLR_SEL_IN BIT(2)
+#define OVL_LAYER_AFBC_EN(n) BIT(4+n)
#define DISP_REG_OVL_ROI_BGCLR 0x0028
#define DISP_REG_OVL_SRC_CON 0x002c
#define DISP_REG_OVL_CON(n) (0x0030 + 0x20 * (n))
#define DISP_REG_OVL_SRC_SIZE(n) (0x0038 + 0x20 * (n))
#define DISP_REG_OVL_OFFSET(n) (0x003c + 0x20 * (n))
+#define DISP_REG_OVL_PITCH_MSB(n) (0x0040 + 0x20 * (n))
+#define OVL_PITCH_MSB_2ND_SUBBUF BIT(16)
+#define OVL_PITCH_MSB_YUV_TRANS BIT(20)
#define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n))
+#define DISP_REG_OVL_CLIP(n) (0x004c + 0x20 * (n))
#define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n))
#define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n))
#define DISP_REG_OVL_ADDR_MT2701 0x0040
#define DISP_REG_OVL_ADDR_MT8173 0x0f40
#define DISP_REG_OVL_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n))
+#define DISP_REG_OVL_HDR_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n) + 0x04)
+#define DISP_REG_OVL_HDR_PITCH(ovl, n) ((ovl)->data->addr + 0x20 * (n) + 0x08)
#define GMC_THRESHOLD_BITS 16
#define GMC_THRESHOLD_HIGH ((1 << GMC_THRESHOLD_BITS) / 4)
@@ -67,6 +74,7 @@ struct mtk_disp_ovl_data {
unsigned int layer_nr;
bool fmt_rgb565_is_0;
bool smi_id_en;
+ bool supports_afbc;
};
/*
@@ -172,7 +180,22 @@ void mtk_ovl_stop(struct device *dev)
reg = reg & ~OVL_LAYER_SMI_ID_EN;
writel_relaxed(reg, ovl->regs + DISP_REG_OVL_DATAPATH_CON);
}
+}
+
+static void mtk_ovl_set_afbc(struct device *dev, struct cmdq_pkt *cmdq_pkt,
+ int idx, bool enabled)
+{
+ struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
+ unsigned int reg;
+ reg = readl(ovl->regs + DISP_REG_OVL_DATAPATH_CON);
+ if (enabled)
+ reg = reg | OVL_LAYER_AFBC_EN(idx);
+ else
+ reg = reg & ~OVL_LAYER_AFBC_EN(idx);
+
+ mtk_ddp_write_relaxed(cmdq_pkt, reg, &ovl->cmdq_reg,
+ ovl->regs, DISP_REG_OVL_DATAPATH_CON);
}
void mtk_ovl_config(struct device *dev, unsigned int w,
@@ -226,6 +249,32 @@ int mtk_ovl_layer_check(struct device *dev, unsigned int idx,
if (state->fb->format->is_yuv && rotation != 0)
return -EINVAL;
+ if (state->fb->modifier) {
+ unsigned long long modifier;
+ unsigned int fourcc;
+ struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
+
+ if (!ovl->data->supports_afbc)
+ return -EINVAL;
+
+ modifier = state->fb->modifier;
+
+ if (modifier != DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
+ AFBC_FORMAT_MOD_SPLIT |
+ AFBC_FORMAT_MOD_SPARSE))
+ return -EINVAL;
+
+ fourcc = state->fb->format->format;
+ if (fourcc != DRM_FORMAT_BGRA8888 &&
+ fourcc != DRM_FORMAT_ABGR8888 &&
+ fourcc != DRM_FORMAT_ARGB8888 &&
+ fourcc != DRM_FORMAT_XRGB8888 &&
+ fourcc != DRM_FORMAT_XBGR8888 &&
+ fourcc != DRM_FORMAT_RGB888 &&
+ fourcc != DRM_FORMAT_BGR888)
+ return -EINVAL;
+ }
+
state->rotation = rotation;
return 0;
@@ -310,11 +359,23 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
struct mtk_plane_pending_state *pending = &state->pending;
unsigned int addr = pending->addr;
- unsigned int pitch = pending->pitch & 0xffff;
+ unsigned int hdr_addr = pending->hdr_addr;
+ unsigned int pitch = pending->pitch;
+ unsigned int hdr_pitch = pending->hdr_pitch;
unsigned int fmt = pending->format;
unsigned int offset = (pending->y << 16) | pending->x;
unsigned int src_size = (pending->height << 16) | pending->width;
unsigned int con;
+ bool is_afbc = pending->modifier;
+ union overlay_pitch {
+ struct split_pitch {
+ u16 lsb;
+ u16 msb;
+ } split_pitch;
+ u32 pitch;
+ } overlay_pitch;
+
+ overlay_pitch.pitch = pitch;
if (!pending->enable) {
mtk_ovl_layer_off(dev, idx, cmdq_pkt);
@@ -335,9 +396,10 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
addr += pending->pitch - 1;
}
+ mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc);
mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs,
DISP_REG_OVL_CON(idx));
- mtk_ddp_write_relaxed(cmdq_pkt, pitch, &ovl->cmdq_reg, ovl->regs,
+ mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb, &ovl->cmdq_reg, ovl->regs,
DISP_REG_OVL_PITCH(idx));
mtk_ddp_write_relaxed(cmdq_pkt, src_size, &ovl->cmdq_reg, ovl->regs,
DISP_REG_OVL_SRC_SIZE(idx));
@@ -345,6 +407,19 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
DISP_REG_OVL_OFFSET(idx));
mtk_ddp_write_relaxed(cmdq_pkt, addr, &ovl->cmdq_reg, ovl->regs,
DISP_REG_OVL_ADDR(ovl, idx));
+ if (is_afbc) {
+ mtk_ddp_write_relaxed(cmdq_pkt, hdr_addr, &ovl->cmdq_reg, ovl->regs,
+ DISP_REG_OVL_HDR_ADDR(ovl, idx));
+ mtk_ddp_write_relaxed(cmdq_pkt,
+ OVL_PITCH_MSB_2ND_SUBBUF | overlay_pitch.split_pitch.msb,
+ &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH_MSB(idx));
+ mtk_ddp_write_relaxed(cmdq_pkt, hdr_pitch, &ovl->cmdq_reg, ovl->regs,
+ DISP_REG_OVL_HDR_PITCH(ovl, idx));
+ } else {
+ mtk_ddp_write_relaxed(cmdq_pkt,
+ overlay_pitch.split_pitch.msb,
+ &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH_MSB(idx));
+ }
mtk_ovl_layer_on(dev, idx, cmdq_pkt);
}
@@ -492,6 +567,15 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = {
.smi_id_en = true,
};
+static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
+ .addr = DISP_REG_OVL_ADDR_MT8173,
+ .gmc_bits = 10,
+ .layer_nr = 4,
+ .fmt_rgb565_is_0 = true,
+ .smi_id_en = true,
+ .supports_afbc = true,
+};
+
static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
{ .compatible = "mediatek,mt2701-disp-ovl",
.data = &mt2701_ovl_driver_data},
@@ -505,6 +589,8 @@ static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
.data = &mt8192_ovl_driver_data},
{ .compatible = "mediatek,mt8192-disp-ovl-2l",
.data = &mt8192_ovl_2l_driver_data},
+ { .compatible = "mediatek,mt8195-disp-ovl",
+ .data = &mt8195_ovl_driver_data},
{},
};
MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 5c0d9ce69931..a285b9ff5081 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -12,6 +12,7 @@
#include <drm/drm_framebuffer.h>
#include <drm/drm_gem_atomic_helper.h>
#include <drm/drm_plane_helper.h>
+#include <linux/align.h>
#include "mtk_drm_crtc.h"
#include "mtk_drm_ddp_comp.h"
@@ -52,6 +53,7 @@ static void mtk_plane_reset(struct drm_plane *plane)
state->base.plane = plane;
state->pending.format = DRM_FORMAT_RGB565;
+ state->pending.modifier = 0;
}
static struct drm_plane_state *mtk_plane_duplicate_state(struct drm_plane *plane)
@@ -120,21 +122,52 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
struct drm_gem_object *gem;
struct mtk_drm_gem_obj *mtk_gem;
unsigned int pitch, format;
+ unsigned long long modifier;
dma_addr_t addr;
+ dma_addr_t hdr_addr = 0;
+ unsigned int hdr_pitch = 0;
gem = fb->obj[0];
mtk_gem = to_mtk_gem_obj(gem);
addr = mtk_gem->dma_addr;
pitch = fb->pitches[0];
format = fb->format->format;
+ modifier = fb->modifier;
- addr += (new_state->src.x1 >> 16) * fb->format->cpp[0];
- addr += (new_state->src.y1 >> 16) * pitch;
+ if (!modifier) {
+ addr += (new_state->src.x1 >> 16) * fb->format->cpp[0];
+ addr += (new_state->src.y1 >> 16) * pitch;
+ } else {
+ int width_in_blocks = ALIGN(fb->width, AFBC_DATA_BLOCK_WIDTH)
+ / AFBC_DATA_BLOCK_WIDTH;
+ int height_in_blocks = ALIGN(fb->height, AFBC_DATA_BLOCK_HEIGHT)
+ / AFBC_DATA_BLOCK_HEIGHT;
+ int x_offset_in_blocks = (new_state->src.x1 >> 16) / AFBC_DATA_BLOCK_WIDTH;
+ int y_offset_in_blocks = (new_state->src.y1 >> 16) / AFBC_DATA_BLOCK_HEIGHT;
+ int hdr_size;
+
+ hdr_pitch = width_in_blocks * AFBC_HEADER_BLOCK_SIZE;
+ pitch = width_in_blocks * AFBC_DATA_BLOCK_WIDTH *
+ AFBC_DATA_BLOCK_HEIGHT * fb->format->cpp[0];
+
+ hdr_size = ALIGN(hdr_pitch * height_in_blocks, AFBC_HEADER_ALIGNMENT);
+
+ hdr_addr = addr + hdr_pitch * y_offset_in_blocks +
+ AFBC_HEADER_BLOCK_SIZE * x_offset_in_blocks;
+ /* The data plane is offset by 1 additional block. */
+ addr = addr + hdr_size +
+ pitch * y_offset_in_blocks +
+ AFBC_DATA_BLOCK_WIDTH * AFBC_DATA_BLOCK_HEIGHT *
+ fb->format->cpp[0] * (x_offset_in_blocks + 1);
+ }
mtk_plane_state->pending.enable = true;
mtk_plane_state->pending.pitch = pitch;
+ mtk_plane_state->pending.hdr_pitch = hdr_pitch;
mtk_plane_state->pending.format = format;
+ mtk_plane_state->pending.modifier = modifier;
mtk_plane_state->pending.addr = addr;
+ mtk_plane_state->pending.hdr_addr = hdr_addr;
mtk_plane_state->pending.x = new_state->dst.x1;
mtk_plane_state->pending.y = new_state->dst.y1;
mtk_plane_state->pending.width = drm_rect_width(&new_state->dst);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
index 2d5ec66e3df1..8f39011cdbfc 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
@@ -10,12 +10,20 @@
#include <drm/drm_crtc.h>
#include <linux/types.h>
+#define AFBC_DATA_BLOCK_WIDTH 32
+#define AFBC_DATA_BLOCK_HEIGHT 8
+#define AFBC_HEADER_BLOCK_SIZE 16
+#define AFBC_HEADER_ALIGNMENT 1024
+
struct mtk_plane_pending_state {
bool config;
bool enable;
dma_addr_t addr;
+ dma_addr_t hdr_addr;
unsigned int pitch;
+ unsigned int hdr_pitch;
unsigned int format;
+ unsigned long long modifier;
unsigned int x;
unsigned int y;
unsigned int width;
--
2.38.0.rc1.362.ged0d419d3c-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
2022-10-12 19:12 ` Justin Green
@ 2022-10-13 8:04 ` AngeloGioacchino Del Regno
-1 siblings, 0 replies; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-10-13 8:04 UTC (permalink / raw)
To: Justin Green, dri-devel, linux-mediatek
Cc: jason-jh.lin, justin.yeh, wenst, chunkuang.hu, p.zabel, airlied,
matthias.bgg, daniel
Il 12/10/22 21:12, Justin Green ha scritto:
> Tested on MT8195 and confirmed both correct video output and improved DRAM
> bandwidth performance.
>
> v3:
> * Replaced pitch bitshift math with union based approach.
> * Refactored overlay register writes to shared code between non-AFBC and
> AFBC.
> * Minor code cleanups.
>
> v2:
> * Marked mtk_ovl_set_afbc as static.
> * Reflowed some lines to fit column limit.
>
> Signed-off-by: Justin Green <greenjustin@chromium.org>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 90 +++++++++++++++++++++++-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 37 +++++++++-
> drivers/gpu/drm/mediatek/mtk_drm_plane.h | 8 +++
> 3 files changed, 131 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 002b0f6cae1a..3f89b5f5064f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
..snip..
> @@ -335,9 +396,10 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
> addr += pending->pitch - 1;
> }
>
> + mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc);
I'm sorry for not noticing that in the previous review - there's only one
more issue here: I'm not sure that *all* of the MediaTek chips have the
AFBC bits in OVL_DATAPATH_CON... this may be clashing with something else
in the layout of (very?) old chips.
The solution is simple here. Please, guard this call with:
if (ovl->data->supports_afbc)
mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc);
...after which
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cheers!
Angelo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
@ 2022-10-13 8:04 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-10-13 8:04 UTC (permalink / raw)
To: Justin Green, dri-devel, linux-mediatek
Cc: chunkuang.hu, airlied, jason-jh.lin, justin.yeh, wenst, matthias.bgg
Il 12/10/22 21:12, Justin Green ha scritto:
> Tested on MT8195 and confirmed both correct video output and improved DRAM
> bandwidth performance.
>
> v3:
> * Replaced pitch bitshift math with union based approach.
> * Refactored overlay register writes to shared code between non-AFBC and
> AFBC.
> * Minor code cleanups.
>
> v2:
> * Marked mtk_ovl_set_afbc as static.
> * Reflowed some lines to fit column limit.
>
> Signed-off-by: Justin Green <greenjustin@chromium.org>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 90 +++++++++++++++++++++++-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 37 +++++++++-
> drivers/gpu/drm/mediatek/mtk_drm_plane.h | 8 +++
> 3 files changed, 131 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 002b0f6cae1a..3f89b5f5064f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
..snip..
> @@ -335,9 +396,10 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
> addr += pending->pitch - 1;
> }
>
> + mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc);
I'm sorry for not noticing that in the previous review - there's only one
more issue here: I'm not sure that *all* of the MediaTek chips have the
AFBC bits in OVL_DATAPATH_CON... this may be clashing with something else
in the layout of (very?) old chips.
The solution is simple here. Please, guard this call with:
if (ovl->data->supports_afbc)
mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc);
...after which
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cheers!
Angelo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
2022-10-12 19:12 ` Justin Green
(?)
(?)
@ 2022-10-13 8:39 ` Daniel Stone
2022-10-13 14:55 ` Justin Green
-1 siblings, 1 reply; 11+ messages in thread
From: Daniel Stone @ 2022-10-13 8:39 UTC (permalink / raw)
To: Justin Green
Cc: chunkuang.hu, airlied, jason-jh.lin, justin.yeh, dri-devel,
linux-mediatek, wenst, matthias.bgg, angelogioacchino.delregno
[-- Attachment #1: Type: text/plain, Size: 2986 bytes --]
Hi Justin,
On Wed, 12 Oct 2022 at 20:12, Justin Green <greenjustin@chromium.org> wrote:
> @@ -226,6 +249,32 @@ int mtk_ovl_layer_check(struct device *dev, unsigned
> int idx,
> if (state->fb->format->is_yuv && rotation != 0)
> return -EINVAL;
>
> + if (state->fb->modifier) {
>
Please spell this out explicitly as DRM_FORMAT_MOD_LINEAR. For some reason,
we specify that 0 is explicitly block-linear, whilst INVALID ('unknown
layout, figure it out by magic') is a non-zero value. So != 0 isn't a check
for 'was a modifier explicitly specified', even if != 0 is almost always
'is this buffer non-linear'.
+ unsigned long long modifier;
> + unsigned int fourcc;
>
u64, u32, but ...
> + if (!ovl->data->supports_afbc)
> + return -EINVAL;
> +
> + modifier = state->fb->modifier;
> +
> + if (modifier !=
> DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
> +
> AFBC_FORMAT_MOD_SPLIT |
> +
> AFBC_FORMAT_MOD_SPARSE))
> + return -EINVAL;
> +
> + fourcc = state->fb->format->format;
> + if (fourcc != DRM_FORMAT_BGRA8888 &&
> + fourcc != DRM_FORMAT_ABGR8888 &&
> + fourcc != DRM_FORMAT_ARGB8888 &&
> + fourcc != DRM_FORMAT_XRGB8888 &&
> + fourcc != DRM_FORMAT_XBGR8888 &&
> + fourcc != DRM_FORMAT_RGB888 &&
> + fourcc != DRM_FORMAT_BGR888)
> + return -EINVAL;
The core shouldn't allow a framebuffer to be created with a format/modifier
pair which wasn't advertised, so these checks can be dropped. Except that
it's never advertised.
mtk_plane_init() passes a set of formats and modifiers to
drm_universal_plane_init(); the AFBC modifier needs to be added here, as
well as LINEAR and INVALID. Then you'll need to set the
format_mod_supported() hook on the plane to filter out format/modifier
pairs. After that, you should see (e.g. with drm_info) that you get an
IN_FORMATS blob which advertises DRM_FORMAT_MOD_ARM_AFBC(...) as well as
linear for DRM_FORMAT_XRGB8888, but only linear for DRM_FORMAT_RGB565.
After doing that, you should see framebuffer creation fail for unsupported
pairings, e.g. RGB565 + AFBC.
> + bool is_afbc = pending->modifier;
... != DRM_FORMAT_MOD_LINEAR
> @@ -52,6 +53,7 @@ static void mtk_plane_reset(struct drm_plane *plane)
>
> state->base.plane = plane;
> state->pending.format = DRM_FORMAT_RGB565;
> + state->pending.modifier = 0;
>
= DRM_FORMAT_MOD_LINEAR
> @@ -120,21 +122,52 @@ static void mtk_plane_update_new_state(struct
> drm_plane_state *new_state,
> struct drm_gem_object *gem;
> struct mtk_drm_gem_obj *mtk_gem;
> unsigned int pitch, format;
> + unsigned long long modifier;
>
u64
> + if (!modifier) {
>
modifier == DRM_FORMAT_MOD_LINEAR
Cheers,
Daniel
[-- Attachment #2: Type: text/html, Size: 5015 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
2022-10-13 8:39 ` Daniel Stone
@ 2022-10-13 14:55 ` Justin Green
0 siblings, 0 replies; 11+ messages in thread
From: Justin Green @ 2022-10-13 14:55 UTC (permalink / raw)
To: Daniel Stone
Cc: dri-devel, linux-mediatek, chunkuang.hu, airlied, jason-jh.lin,
justin.yeh, wenst, matthias.bgg, angelogioacchino.delregno
Thanks for the comments everyone! I'll upload a new CL sometime today.
I did want to ask though, I realize I should be using u32/u64 for
kernel code in general, but the rest of this file seems to be written
using unsigned int/unsigned long long. In this circumstance, does
keeping with the style of the original source take precedence over
general style guidelines, or vice versa?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
@ 2022-10-13 14:55 ` Justin Green
0 siblings, 0 replies; 11+ messages in thread
From: Justin Green @ 2022-10-13 14:55 UTC (permalink / raw)
To: Daniel Stone
Cc: chunkuang.hu, airlied, jason-jh.lin, justin.yeh, dri-devel,
linux-mediatek, wenst, matthias.bgg, angelogioacchino.delregno
Thanks for the comments everyone! I'll upload a new CL sometime today.
I did want to ask though, I realize I should be using u32/u64 for
kernel code in general, but the rest of this file seems to be written
using unsigned int/unsigned long long. In this circumstance, does
keeping with the style of the original source take precedence over
general style guidelines, or vice versa?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
2022-10-12 19:12 ` Justin Green
@ 2022-11-14 15:07 ` Chun-Kuang Hu
-1 siblings, 0 replies; 11+ messages in thread
From: Chun-Kuang Hu @ 2022-11-14 15:07 UTC (permalink / raw)
To: Justin Green
Cc: dri-devel, linux-mediatek, angelogioacchino.delregno,
jason-jh.lin, justin.yeh, wenst, chunkuang.hu, p.zabel, airlied,
matthias.bgg, daniel
* , ,
Hi, Justin:
Justin Green <greenjustin@chromium.org> 於 2022年10月13日 週四 凌晨3:12寫道：
>
> Tested on MT8195 and confirmed both correct video output and improved DRAM
> bandwidth performance.
>
> v3:
> * Replaced pitch bitshift math with union based approach.
> * Refactored overlay register writes to shared code between non-AFBC and
> AFBC.
> * Minor code cleanups.
>
> v2:
> * Marked mtk_ovl_set_afbc as static.
> * Reflowed some lines to fit column limit.
>
> Signed-off-by: Justin Green <greenjustin@chromium.org>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 90 +++++++++++++++++++++++-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 37 +++++++++-
> drivers/gpu/drm/mediatek/mtk_drm_plane.h | 8 +++
> 3 files changed, 131 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 002b0f6cae1a..3f89b5f5064f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -29,17 +29,24 @@
> #define DISP_REG_OVL_DATAPATH_CON 0x0024
> #define OVL_LAYER_SMI_ID_EN BIT(0)
> #define OVL_BGCLR_SEL_IN BIT(2)
> +#define OVL_LAYER_AFBC_EN(n) BIT(4+n)
> #define DISP_REG_OVL_ROI_BGCLR 0x0028
> #define DISP_REG_OVL_SRC_CON 0x002c
> #define DISP_REG_OVL_CON(n) (0x0030 + 0x20 * (n))
> #define DISP_REG_OVL_SRC_SIZE(n) (0x0038 + 0x20 * (n))
> #define DISP_REG_OVL_OFFSET(n) (0x003c + 0x20 * (n))
> +#define DISP_REG_OVL_PITCH_MSB(n) (0x0040 + 0x20 * (n))
> +#define OVL_PITCH_MSB_2ND_SUBBUF BIT(16)
> +#define OVL_PITCH_MSB_YUV_TRANS BIT(20)
Useless, so drop it.
> #define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n))
> +#define DISP_REG_OVL_CLIP(n) (0x004c + 0x20 * (n))
Useless, so drop it.
> #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n))
> #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n))
> #define DISP_REG_OVL_ADDR_MT2701 0x0040
> #define DISP_REG_OVL_ADDR_MT8173 0x0f40
> #define DISP_REG_OVL_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n))
> +#define DISP_REG_OVL_HDR_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n) + 0x04)
> +#define DISP_REG_OVL_HDR_PITCH(ovl, n) ((ovl)->data->addr + 0x20 * (n) + 0x08)
>
> #define GMC_THRESHOLD_BITS 16
> #define GMC_THRESHOLD_HIGH ((1 << GMC_THRESHOLD_BITS) / 4)
> @@ -67,6 +74,7 @@ struct mtk_disp_ovl_data {
> unsigned int layer_nr;
> bool fmt_rgb565_is_0;
> bool smi_id_en;
> + bool supports_afbc;
> };
>
> /*
> @@ -172,7 +180,22 @@ void mtk_ovl_stop(struct device *dev)
> reg = reg & ~OVL_LAYER_SMI_ID_EN;
> writel_relaxed(reg, ovl->regs + DISP_REG_OVL_DATAPATH_CON);
> }
> +}
> +
> +static void mtk_ovl_set_afbc(struct device *dev, struct cmdq_pkt *cmdq_pkt,
> + int idx, bool enabled)
> +{
> + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
This is a ovl internal function, so you could pass ovl directly into
this function.
> + unsigned int reg;
>
> + reg = readl(ovl->regs + DISP_REG_OVL_DATAPATH_CON);
> + if (enabled)
> + reg = reg | OVL_LAYER_AFBC_EN(idx);
> + else
> + reg = reg & ~OVL_LAYER_AFBC_EN(idx);
> +
> + mtk_ddp_write_relaxed(cmdq_pkt, reg, &ovl->cmdq_reg,
> + ovl->regs, DISP_REG_OVL_DATAPATH_CON);
In normal case, read/write register by cmdq, so
mtk_ddp_write_mask(cmdq_pkt, enable ? OVL_LAYER_AFBC_EN(idx) : 0,
&ovl->cmdq_reg, ovl->regs,
DISP_REG_OVL_DATAPATH_CON,
OVL_LAYER_AFBC_EN(idx));
> }
>
> void mtk_ovl_config(struct device *dev, unsigned int w,
> @@ -226,6 +249,32 @@ int mtk_ovl_layer_check(struct device *dev, unsigned int idx,
> if (state->fb->format->is_yuv && rotation != 0)
> return -EINVAL;
>
> + if (state->fb->modifier) {
> + unsigned long long modifier;
> + unsigned int fourcc;
> + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
> +
> + if (!ovl->data->supports_afbc)
> + return -EINVAL;
> +
> + modifier = state->fb->modifier;
> +
> + if (modifier != DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
> + AFBC_FORMAT_MOD_SPLIT |
> + AFBC_FORMAT_MOD_SPARSE))
> + return -EINVAL;
> +
> + fourcc = state->fb->format->format;
> + if (fourcc != DRM_FORMAT_BGRA8888 &&
> + fourcc != DRM_FORMAT_ABGR8888 &&
> + fourcc != DRM_FORMAT_ARGB8888 &&
> + fourcc != DRM_FORMAT_XRGB8888 &&
> + fourcc != DRM_FORMAT_XBGR8888 &&
> + fourcc != DRM_FORMAT_RGB888 &&
> + fourcc != DRM_FORMAT_BGR888)
> + return -EINVAL;
> + }
> +
> state->rotation = rotation;
>
> return 0;
> @@ -310,11 +359,23 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
> struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
> struct mtk_plane_pending_state *pending = &state->pending;
> unsigned int addr = pending->addr;
> - unsigned int pitch = pending->pitch & 0xffff;
> + unsigned int hdr_addr = pending->hdr_addr;
> + unsigned int pitch = pending->pitch;
> + unsigned int hdr_pitch = pending->hdr_pitch;
> unsigned int fmt = pending->format;
> unsigned int offset = (pending->y << 16) | pending->x;
> unsigned int src_size = (pending->height << 16) | pending->width;
> unsigned int con;
> + bool is_afbc = pending->modifier;
> + union overlay_pitch {
> + struct split_pitch {
> + u16 lsb;
> + u16 msb;
> + } split_pitch;
> + u32 pitch;
> + } overlay_pitch;
> +
> + overlay_pitch.pitch = pitch;
>
> if (!pending->enable) {
> mtk_ovl_layer_off(dev, idx, cmdq_pkt);
> @@ -335,9 +396,10 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
> addr += pending->pitch - 1;
> }
>
> + mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc);
> mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_CON(idx));
> - mtk_ddp_write_relaxed(cmdq_pkt, pitch, &ovl->cmdq_reg, ovl->regs,
> + mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb, &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_PITCH(idx));
Is this general for all MediaTek SoC? If so, separate this to an
independent patch. Otherwise, use a device variable to separate this
setting.
> mtk_ddp_write_relaxed(cmdq_pkt, src_size, &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_SRC_SIZE(idx));
> @@ -345,6 +407,19 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
> DISP_REG_OVL_OFFSET(idx));
> mtk_ddp_write_relaxed(cmdq_pkt, addr, &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_ADDR(ovl, idx));
> + if (is_afbc) {
> + mtk_ddp_write_relaxed(cmdq_pkt, hdr_addr, &ovl->cmdq_reg, ovl->regs,
> + DISP_REG_OVL_HDR_ADDR(ovl, idx));
> + mtk_ddp_write_relaxed(cmdq_pkt,
> + OVL_PITCH_MSB_2ND_SUBBUF | overlay_pitch.split_pitch.msb,
> + &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH_MSB(idx));
> + mtk_ddp_write_relaxed(cmdq_pkt, hdr_pitch, &ovl->cmdq_reg, ovl->regs,
> + DISP_REG_OVL_HDR_PITCH(ovl, idx));
> + } else {
> + mtk_ddp_write_relaxed(cmdq_pkt,
> + overlay_pitch.split_pitch.msb,
> + &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH_MSB(idx));
> + }
>
> mtk_ovl_layer_on(dev, idx, cmdq_pkt);
> }
> @@ -492,6 +567,15 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = {
> .smi_id_en = true,
> };
>
> +static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
In this binding document, mt8195 ovl is compatible to mt8133 ovl.
Please confirm that mt8195 is not identical with mt8133.
> + .addr = DISP_REG_OVL_ADDR_MT8173,
> + .gmc_bits = 10,
> + .layer_nr = 4,
> + .fmt_rgb565_is_0 = true,
> + .smi_id_en = true,
> + .supports_afbc = true,
> +};
> +
> static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
> { .compatible = "mediatek,mt2701-disp-ovl",
> .data = &mt2701_ovl_driver_data},
> @@ -505,6 +589,8 @@ static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
> .data = &mt8192_ovl_driver_data},
> { .compatible = "mediatek,mt8192-disp-ovl-2l",
> .data = &mt8192_ovl_2l_driver_data},
> + { .compatible = "mediatek,mt8195-disp-ovl",
> + .data = &mt8195_ovl_driver_data},
> {},
> };
> MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 5c0d9ce69931..a285b9ff5081 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -12,6 +12,7 @@
> #include <drm/drm_framebuffer.h>
> #include <drm/drm_gem_atomic_helper.h>
> #include <drm/drm_plane_helper.h>
> +#include <linux/align.h>
>
> #include "mtk_drm_crtc.h"
> #include "mtk_drm_ddp_comp.h"
> @@ -52,6 +53,7 @@ static void mtk_plane_reset(struct drm_plane *plane)
>
> state->base.plane = plane;
> state->pending.format = DRM_FORMAT_RGB565;
> + state->pending.modifier = 0;
> }
>
> static struct drm_plane_state *mtk_plane_duplicate_state(struct drm_plane *plane)
> @@ -120,21 +122,52 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
> struct drm_gem_object *gem;
> struct mtk_drm_gem_obj *mtk_gem;
> unsigned int pitch, format;
> + unsigned long long modifier;
> dma_addr_t addr;
> + dma_addr_t hdr_addr = 0;
> + unsigned int hdr_pitch = 0;
>
> gem = fb->obj[0];
> mtk_gem = to_mtk_gem_obj(gem);
> addr = mtk_gem->dma_addr;
> pitch = fb->pitches[0];
> format = fb->format->format;
> + modifier = fb->modifier;
>
> - addr += (new_state->src.x1 >> 16) * fb->format->cpp[0];
> - addr += (new_state->src.y1 >> 16) * pitch;
> + if (!modifier) {
> + addr += (new_state->src.x1 >> 16) * fb->format->cpp[0];
> + addr += (new_state->src.y1 >> 16) * pitch;
> + } else {
> + int width_in_blocks = ALIGN(fb->width, AFBC_DATA_BLOCK_WIDTH)
> + / AFBC_DATA_BLOCK_WIDTH;
> + int height_in_blocks = ALIGN(fb->height, AFBC_DATA_BLOCK_HEIGHT)
> + / AFBC_DATA_BLOCK_HEIGHT;
> + int x_offset_in_blocks = (new_state->src.x1 >> 16) / AFBC_DATA_BLOCK_WIDTH;
> + int y_offset_in_blocks = (new_state->src.y1 >> 16) / AFBC_DATA_BLOCK_HEIGHT;
> + int hdr_size;
> +
> + hdr_pitch = width_in_blocks * AFBC_HEADER_BLOCK_SIZE;
> + pitch = width_in_blocks * AFBC_DATA_BLOCK_WIDTH *
> + AFBC_DATA_BLOCK_HEIGHT * fb->format->cpp[0];
> +
> + hdr_size = ALIGN(hdr_pitch * height_in_blocks, AFBC_HEADER_ALIGNMENT);
Usually the pitch needs alignment. So I guess the formula is
hdr_pitch = ALIGN(width_in_blocks * AFBC_HEADER_BLOCK_SIZE,
AFBC_HEADER_ALIGNMENT);
hdr_size = hdr_pitch * height_in_blocks;
Could you explain the meaning of hdr_pitch?
Regards,
Chun-Kuang.
> +
> + hdr_addr = addr + hdr_pitch * y_offset_in_blocks +
> + AFBC_HEADER_BLOCK_SIZE * x_offset_in_blocks;
> + /* The data plane is offset by 1 additional block. */
> + addr = addr + hdr_size +
> + pitch * y_offset_in_blocks +
> + AFBC_DATA_BLOCK_WIDTH * AFBC_DATA_BLOCK_HEIGHT *
> + fb->format->cpp[0] * (x_offset_in_blocks + 1);
> + }
>
> mtk_plane_state->pending.enable = true;
> mtk_plane_state->pending.pitch = pitch;
> + mtk_plane_state->pending.hdr_pitch = hdr_pitch;
> mtk_plane_state->pending.format = format;
> + mtk_plane_state->pending.modifier = modifier;
> mtk_plane_state->pending.addr = addr;
> + mtk_plane_state->pending.hdr_addr = hdr_addr;
> mtk_plane_state->pending.x = new_state->dst.x1;
> mtk_plane_state->pending.y = new_state->dst.y1;
> mtk_plane_state->pending.width = drm_rect_width(&new_state->dst);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> index 2d5ec66e3df1..8f39011cdbfc 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> @@ -10,12 +10,20 @@
> #include <drm/drm_crtc.h>
> #include <linux/types.h>
>
> +#define AFBC_DATA_BLOCK_WIDTH 32
> +#define AFBC_DATA_BLOCK_HEIGHT 8
> +#define AFBC_HEADER_BLOCK_SIZE 16
> +#define AFBC_HEADER_ALIGNMENT 1024
> +
> struct mtk_plane_pending_state {
> bool config;
> bool enable;
> dma_addr_t addr;
> + dma_addr_t hdr_addr;
> unsigned int pitch;
> + unsigned int hdr_pitch;
> unsigned int format;
> + unsigned long long modifier;
> unsigned int x;
> unsigned int y;
> unsigned int width;
> --
> 2.38.0.rc1.362.ged0d419d3c-goog
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
@ 2022-11-14 15:07 ` Chun-Kuang Hu
0 siblings, 0 replies; 11+ messages in thread
From: Chun-Kuang Hu @ 2022-11-14 15:07 UTC (permalink / raw)
To: Justin Green
Cc: chunkuang.hu, airlied, jason-jh.lin, justin.yeh, dri-devel,
linux-mediatek, wenst, matthias.bgg, angelogioacchino.delregno
* , ,
Hi, Justin:
Justin Green <greenjustin@chromium.org> 於 2022年10月13日 週四 凌晨3:12寫道：
>
> Tested on MT8195 and confirmed both correct video output and improved DRAM
> bandwidth performance.
>
> v3:
> * Replaced pitch bitshift math with union based approach.
> * Refactored overlay register writes to shared code between non-AFBC and
> AFBC.
> * Minor code cleanups.
>
> v2:
> * Marked mtk_ovl_set_afbc as static.
> * Reflowed some lines to fit column limit.
>
> Signed-off-by: Justin Green <greenjustin@chromium.org>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 90 +++++++++++++++++++++++-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 37 +++++++++-
> drivers/gpu/drm/mediatek/mtk_drm_plane.h | 8 +++
> 3 files changed, 131 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 002b0f6cae1a..3f89b5f5064f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -29,17 +29,24 @@
> #define DISP_REG_OVL_DATAPATH_CON 0x0024
> #define OVL_LAYER_SMI_ID_EN BIT(0)
> #define OVL_BGCLR_SEL_IN BIT(2)
> +#define OVL_LAYER_AFBC_EN(n) BIT(4+n)
> #define DISP_REG_OVL_ROI_BGCLR 0x0028
> #define DISP_REG_OVL_SRC_CON 0x002c
> #define DISP_REG_OVL_CON(n) (0x0030 + 0x20 * (n))
> #define DISP_REG_OVL_SRC_SIZE(n) (0x0038 + 0x20 * (n))
> #define DISP_REG_OVL_OFFSET(n) (0x003c + 0x20 * (n))
> +#define DISP_REG_OVL_PITCH_MSB(n) (0x0040 + 0x20 * (n))
> +#define OVL_PITCH_MSB_2ND_SUBBUF BIT(16)
> +#define OVL_PITCH_MSB_YUV_TRANS BIT(20)
Useless, so drop it.
> #define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n))
> +#define DISP_REG_OVL_CLIP(n) (0x004c + 0x20 * (n))
Useless, so drop it.
> #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n))
> #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n))
> #define DISP_REG_OVL_ADDR_MT2701 0x0040
> #define DISP_REG_OVL_ADDR_MT8173 0x0f40
> #define DISP_REG_OVL_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n))
> +#define DISP_REG_OVL_HDR_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n) + 0x04)
> +#define DISP_REG_OVL_HDR_PITCH(ovl, n) ((ovl)->data->addr + 0x20 * (n) + 0x08)
>
> #define GMC_THRESHOLD_BITS 16
> #define GMC_THRESHOLD_HIGH ((1 << GMC_THRESHOLD_BITS) / 4)
> @@ -67,6 +74,7 @@ struct mtk_disp_ovl_data {
> unsigned int layer_nr;
> bool fmt_rgb565_is_0;
> bool smi_id_en;
> + bool supports_afbc;
> };
>
> /*
> @@ -172,7 +180,22 @@ void mtk_ovl_stop(struct device *dev)
> reg = reg & ~OVL_LAYER_SMI_ID_EN;
> writel_relaxed(reg, ovl->regs + DISP_REG_OVL_DATAPATH_CON);
> }
> +}
> +
> +static void mtk_ovl_set_afbc(struct device *dev, struct cmdq_pkt *cmdq_pkt,
> + int idx, bool enabled)
> +{
> + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
This is a ovl internal function, so you could pass ovl directly into
this function.
> + unsigned int reg;
>
> + reg = readl(ovl->regs + DISP_REG_OVL_DATAPATH_CON);
> + if (enabled)
> + reg = reg | OVL_LAYER_AFBC_EN(idx);
> + else
> + reg = reg & ~OVL_LAYER_AFBC_EN(idx);
> +
> + mtk_ddp_write_relaxed(cmdq_pkt, reg, &ovl->cmdq_reg,
> + ovl->regs, DISP_REG_OVL_DATAPATH_CON);
In normal case, read/write register by cmdq, so
mtk_ddp_write_mask(cmdq_pkt, enable ? OVL_LAYER_AFBC_EN(idx) : 0,
&ovl->cmdq_reg, ovl->regs,
DISP_REG_OVL_DATAPATH_CON,
OVL_LAYER_AFBC_EN(idx));
> }
>
> void mtk_ovl_config(struct device *dev, unsigned int w,
> @@ -226,6 +249,32 @@ int mtk_ovl_layer_check(struct device *dev, unsigned int idx,
> if (state->fb->format->is_yuv && rotation != 0)
> return -EINVAL;
>
> + if (state->fb->modifier) {
> + unsigned long long modifier;
> + unsigned int fourcc;
> + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
> +
> + if (!ovl->data->supports_afbc)
> + return -EINVAL;
> +
> + modifier = state->fb->modifier;
> +
> + if (modifier != DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
> + AFBC_FORMAT_MOD_SPLIT |
> + AFBC_FORMAT_MOD_SPARSE))
> + return -EINVAL;
> +
> + fourcc = state->fb->format->format;
> + if (fourcc != DRM_FORMAT_BGRA8888 &&
> + fourcc != DRM_FORMAT_ABGR8888 &&
> + fourcc != DRM_FORMAT_ARGB8888 &&
> + fourcc != DRM_FORMAT_XRGB8888 &&
> + fourcc != DRM_FORMAT_XBGR8888 &&
> + fourcc != DRM_FORMAT_RGB888 &&
> + fourcc != DRM_FORMAT_BGR888)
> + return -EINVAL;
> + }
> +
> state->rotation = rotation;
>
> return 0;
> @@ -310,11 +359,23 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
> struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
> struct mtk_plane_pending_state *pending = &state->pending;
> unsigned int addr = pending->addr;
> - unsigned int pitch = pending->pitch & 0xffff;
> + unsigned int hdr_addr = pending->hdr_addr;
> + unsigned int pitch = pending->pitch;
> + unsigned int hdr_pitch = pending->hdr_pitch;
> unsigned int fmt = pending->format;
> unsigned int offset = (pending->y << 16) | pending->x;
> unsigned int src_size = (pending->height << 16) | pending->width;
> unsigned int con;
> + bool is_afbc = pending->modifier;
> + union overlay_pitch {
> + struct split_pitch {
> + u16 lsb;
> + u16 msb;
> + } split_pitch;
> + u32 pitch;
> + } overlay_pitch;
> +
> + overlay_pitch.pitch = pitch;
>
> if (!pending->enable) {
> mtk_ovl_layer_off(dev, idx, cmdq_pkt);
> @@ -335,9 +396,10 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
> addr += pending->pitch - 1;
> }
>
> + mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc);
> mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_CON(idx));
> - mtk_ddp_write_relaxed(cmdq_pkt, pitch, &ovl->cmdq_reg, ovl->regs,
> + mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb, &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_PITCH(idx));
Is this general for all MediaTek SoC? If so, separate this to an
independent patch. Otherwise, use a device variable to separate this
setting.
> mtk_ddp_write_relaxed(cmdq_pkt, src_size, &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_SRC_SIZE(idx));
> @@ -345,6 +407,19 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
> DISP_REG_OVL_OFFSET(idx));
> mtk_ddp_write_relaxed(cmdq_pkt, addr, &ovl->cmdq_reg, ovl->regs,
> DISP_REG_OVL_ADDR(ovl, idx));
> + if (is_afbc) {
> + mtk_ddp_write_relaxed(cmdq_pkt, hdr_addr, &ovl->cmdq_reg, ovl->regs,
> + DISP_REG_OVL_HDR_ADDR(ovl, idx));
> + mtk_ddp_write_relaxed(cmdq_pkt,
> + OVL_PITCH_MSB_2ND_SUBBUF | overlay_pitch.split_pitch.msb,
> + &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH_MSB(idx));
> + mtk_ddp_write_relaxed(cmdq_pkt, hdr_pitch, &ovl->cmdq_reg, ovl->regs,
> + DISP_REG_OVL_HDR_PITCH(ovl, idx));
> + } else {
> + mtk_ddp_write_relaxed(cmdq_pkt,
> + overlay_pitch.split_pitch.msb,
> + &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH_MSB(idx));
> + }
>
> mtk_ovl_layer_on(dev, idx, cmdq_pkt);
> }
> @@ -492,6 +567,15 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = {
> .smi_id_en = true,
> };
>
> +static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
In this binding document, mt8195 ovl is compatible to mt8133 ovl.
Please confirm that mt8195 is not identical with mt8133.
> + .addr = DISP_REG_OVL_ADDR_MT8173,
> + .gmc_bits = 10,
> + .layer_nr = 4,
> + .fmt_rgb565_is_0 = true,
> + .smi_id_en = true,
> + .supports_afbc = true,
> +};
> +
> static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
> { .compatible = "mediatek,mt2701-disp-ovl",
> .data = &mt2701_ovl_driver_data},
> @@ -505,6 +589,8 @@ static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
> .data = &mt8192_ovl_driver_data},
> { .compatible = "mediatek,mt8192-disp-ovl-2l",
> .data = &mt8192_ovl_2l_driver_data},
> + { .compatible = "mediatek,mt8195-disp-ovl",
> + .data = &mt8195_ovl_driver_data},
> {},
> };
> MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 5c0d9ce69931..a285b9ff5081 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -12,6 +12,7 @@
> #include <drm/drm_framebuffer.h>
> #include <drm/drm_gem_atomic_helper.h>
> #include <drm/drm_plane_helper.h>
> +#include <linux/align.h>
>
> #include "mtk_drm_crtc.h"
> #include "mtk_drm_ddp_comp.h"
> @@ -52,6 +53,7 @@ static void mtk_plane_reset(struct drm_plane *plane)
>
> state->base.plane = plane;
> state->pending.format = DRM_FORMAT_RGB565;
> + state->pending.modifier = 0;
> }
>
> static struct drm_plane_state *mtk_plane_duplicate_state(struct drm_plane *plane)
> @@ -120,21 +122,52 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
> struct drm_gem_object *gem;
> struct mtk_drm_gem_obj *mtk_gem;
> unsigned int pitch, format;
> + unsigned long long modifier;
> dma_addr_t addr;
> + dma_addr_t hdr_addr = 0;
> + unsigned int hdr_pitch = 0;
>
> gem = fb->obj[0];
> mtk_gem = to_mtk_gem_obj(gem);
> addr = mtk_gem->dma_addr;
> pitch = fb->pitches[0];
> format = fb->format->format;
> + modifier = fb->modifier;
>
> - addr += (new_state->src.x1 >> 16) * fb->format->cpp[0];
> - addr += (new_state->src.y1 >> 16) * pitch;
> + if (!modifier) {
> + addr += (new_state->src.x1 >> 16) * fb->format->cpp[0];
> + addr += (new_state->src.y1 >> 16) * pitch;
> + } else {
> + int width_in_blocks = ALIGN(fb->width, AFBC_DATA_BLOCK_WIDTH)
> + / AFBC_DATA_BLOCK_WIDTH;
> + int height_in_blocks = ALIGN(fb->height, AFBC_DATA_BLOCK_HEIGHT)
> + / AFBC_DATA_BLOCK_HEIGHT;
> + int x_offset_in_blocks = (new_state->src.x1 >> 16) / AFBC_DATA_BLOCK_WIDTH;
> + int y_offset_in_blocks = (new_state->src.y1 >> 16) / AFBC_DATA_BLOCK_HEIGHT;
> + int hdr_size;
> +
> + hdr_pitch = width_in_blocks * AFBC_HEADER_BLOCK_SIZE;
> + pitch = width_in_blocks * AFBC_DATA_BLOCK_WIDTH *
> + AFBC_DATA_BLOCK_HEIGHT * fb->format->cpp[0];
> +
> + hdr_size = ALIGN(hdr_pitch * height_in_blocks, AFBC_HEADER_ALIGNMENT);
Usually the pitch needs alignment. So I guess the formula is
hdr_pitch = ALIGN(width_in_blocks * AFBC_HEADER_BLOCK_SIZE,
AFBC_HEADER_ALIGNMENT);
hdr_size = hdr_pitch * height_in_blocks;
Could you explain the meaning of hdr_pitch?
Regards,
Chun-Kuang.
> +
> + hdr_addr = addr + hdr_pitch * y_offset_in_blocks +
> + AFBC_HEADER_BLOCK_SIZE * x_offset_in_blocks;
> + /* The data plane is offset by 1 additional block. */
> + addr = addr + hdr_size +
> + pitch * y_offset_in_blocks +
> + AFBC_DATA_BLOCK_WIDTH * AFBC_DATA_BLOCK_HEIGHT *
> + fb->format->cpp[0] * (x_offset_in_blocks + 1);
> + }
>
> mtk_plane_state->pending.enable = true;
> mtk_plane_state->pending.pitch = pitch;
> + mtk_plane_state->pending.hdr_pitch = hdr_pitch;
> mtk_plane_state->pending.format = format;
> + mtk_plane_state->pending.modifier = modifier;
> mtk_plane_state->pending.addr = addr;
> + mtk_plane_state->pending.hdr_addr = hdr_addr;
> mtk_plane_state->pending.x = new_state->dst.x1;
> mtk_plane_state->pending.y = new_state->dst.y1;
> mtk_plane_state->pending.width = drm_rect_width(&new_state->dst);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> index 2d5ec66e3df1..8f39011cdbfc 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
> @@ -10,12 +10,20 @@
> #include <drm/drm_crtc.h>
> #include <linux/types.h>
>
> +#define AFBC_DATA_BLOCK_WIDTH 32
> +#define AFBC_DATA_BLOCK_HEIGHT 8
> +#define AFBC_HEADER_BLOCK_SIZE 16
> +#define AFBC_HEADER_ALIGNMENT 1024
> +
> struct mtk_plane_pending_state {
> bool config;
> bool enable;
> dma_addr_t addr;
> + dma_addr_t hdr_addr;
> unsigned int pitch;
> + unsigned int hdr_pitch;
> unsigned int format;
> + unsigned long long modifier;
> unsigned int x;
> unsigned int y;
> unsigned int width;
> --
> 2.38.0.rc1.362.ged0d419d3c-goog
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
2022-11-14 15:07 ` Chun-Kuang Hu
@ 2022-11-16 15:45 ` Justin Green
-1 siblings, 0 replies; 11+ messages in thread
From: Justin Green @ 2022-11-16 15:45 UTC (permalink / raw)
To: Chun-Kuang Hu
Cc: dri-devel, linux-mediatek, angelogioacchino.delregno,
jason-jh.lin, justin.yeh, wenst, p.zabel, airlied, matthias.bgg,
daniel
Hi Chun-Kuang,
> > + mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc);
> > mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs,
> > DISP_REG_OVL_CON(idx));
> > - mtk_ddp_write_relaxed(cmdq_pkt, pitch, &ovl->cmdq_reg, ovl->regs,
> > + mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb, &ovl->cmdq_reg, ovl->regs,
> > DISP_REG_OVL_PITCH(idx));
>
> Is this general for all MediaTek SoC? If so, separate this to an
> independent patch. Otherwise, use a device variable to separate this
> setting.
Yes and no. Technically all MediaTek SoCs have two separate registers
for the pitch, each are 16 bits, so this code is technically always
needed. However, because the lsb register is 16 bit, this issue has
never come up, because nobody has tried to display a plane that was
16384 ARGB pixels across. In fact, I think most MediaTek SoCs have a
resolution limit of 4K. The reason this issue comes up now is because
"pitch" is calculated differently for AFBC frames, and actually refers
to the size in bytes of one row of AFBC blocks. Should I still
separate this into an independent patch?
> > }
> > @@ -492,6 +567,15 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = {
> > .smi_id_en = true,
> > };
> >
> > +static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
>
> In this binding document, mt8195 ovl is compatible to mt8133 ovl.
> Please confirm that mt8195 is not identical with mt8133.
Do you mean MT8183? If so, we do not have any documentation indicating
that the MT8183 supports AFBC. Do you have some reason to believe
otherwise?
> Usually the pitch needs alignment. So I guess the formula is
>
> hdr_pitch = ALIGN(width_in_blocks * AFBC_HEADER_BLOCK_SIZE,
> AFBC_HEADER_ALIGNMENT);
> hdr_size = hdr_pitch * height_in_blocks;
The documentation does not indicate that the pitch needs alignment
beyond the AFBC header block size.
> Could you explain the meaning of hdr_pitch?
hdr_pitch refers to the size in bytes of one row of AFBC header
blocks. AFBC is a proprietary compressed frame buffer format, but from
what public information we have, it appears to be block compressed
data stored in 2 contiguous planes. The first is called the "header"
plane, and the second is called the "body" plane. The header plane
contains metadata for each block of pixel data, and the body plane
contains sparse compressed block data.
I'll send another patch with the other changes you mentioned.
Thanks!
Justin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver
@ 2022-11-16 15:45 ` Justin Green
0 siblings, 0 replies; 11+ messages in thread
From: Justin Green @ 2022-11-16 15:45 UTC (permalink / raw)
To: Chun-Kuang Hu
Cc: airlied, jason-jh.lin, justin.yeh, dri-devel, linux-mediatek,
wenst, matthias.bgg, angelogioacchino.delregno
Hi Chun-Kuang,
> > + mtk_ovl_set_afbc(dev, cmdq_pkt, idx, is_afbc);
> > mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs,
> > DISP_REG_OVL_CON(idx));
> > - mtk_ddp_write_relaxed(cmdq_pkt, pitch, &ovl->cmdq_reg, ovl->regs,
> > + mtk_ddp_write_relaxed(cmdq_pkt, overlay_pitch.split_pitch.lsb, &ovl->cmdq_reg, ovl->regs,
> > DISP_REG_OVL_PITCH(idx));
>
> Is this general for all MediaTek SoC? If so, separate this to an
> independent patch. Otherwise, use a device variable to separate this
> setting.
Yes and no. Technically all MediaTek SoCs have two separate registers
for the pitch, each are 16 bits, so this code is technically always
needed. However, because the lsb register is 16 bit, this issue has
never come up, because nobody has tried to display a plane that was
16384 ARGB pixels across. In fact, I think most MediaTek SoCs have a
resolution limit of 4K. The reason this issue comes up now is because
"pitch" is calculated differently for AFBC frames, and actually refers
to the size in bytes of one row of AFBC blocks. Should I still
separate this into an independent patch?
> > }
> > @@ -492,6 +567,15 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = {
> > .smi_id_en = true,
> > };
> >
> > +static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
>
> In this binding document, mt8195 ovl is compatible to mt8133 ovl.
> Please confirm that mt8195 is not identical with mt8133.
Do you mean MT8183? If so, we do not have any documentation indicating
that the MT8183 supports AFBC. Do you have some reason to believe
otherwise?
> Usually the pitch needs alignment. So I guess the formula is
>
> hdr_pitch = ALIGN(width_in_blocks * AFBC_HEADER_BLOCK_SIZE,
> AFBC_HEADER_ALIGNMENT);
> hdr_size = hdr_pitch * height_in_blocks;
The documentation does not indicate that the pitch needs alignment
beyond the AFBC header block size.
> Could you explain the meaning of hdr_pitch?
hdr_pitch refers to the size in bytes of one row of AFBC header
blocks. AFBC is a proprietary compressed frame buffer format, but from
what public information we have, it appears to be block compressed
data stored in 2 contiguous planes. The first is called the "header"
plane, and the second is called the "body" plane. The header plane
contains metadata for each block of pixel data, and the body plane
contains sparse compressed block data.
I'll send another patch with the other changes you mentioned.
Thanks!
Justin
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-11-16 15:45 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-12 19:12 [PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver Justin Green
2022-10-12 19:12 ` Justin Green
2022-10-13 8:04 ` AngeloGioacchino Del Regno
2022-10-13 8:04 ` AngeloGioacchino Del Regno
2022-10-13 8:39 ` Daniel Stone
2022-10-13 14:55 ` Justin Green
2022-10-13 14:55 ` Justin Green
2022-11-14 15:07 ` Chun-Kuang Hu
2022-11-14 15:07 ` Chun-Kuang Hu
2022-11-16 15:45 ` Justin Green
2022-11-16 15:45 ` Justin Green
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.