| Message ID | 20221012191226.1646315-1-greenjustin@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/mediatek: Add AFBC support to Mediatek DRM driver | expand |
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
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?
* , , 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 >
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
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;
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(-)