[PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver

[Justin Green]

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