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