Re: [v7, PATCH 7/7] drm/mediatek: add support for mediatek SOC MT8183

Chun-Kuang Hu
2020-07-25T03:45:45Z

Hi, Yongqiang:

Yongqiang Niu <yongqiang.niu@mediatek.com> 於 2020年7月25日 週六 上午11:32寫道：
>
> On Sat, 2020-07-25 at 07:24 +0800, Chun-Kuang Hu wrote:
> > Hi Yongqiang:
> >
> > Yongqiang Niu <yongqiang.niu@mediatek.com> 於 2020年7月23日 週四 上午10:15寫道：
> > >
> > > This patch add support for mediatek SOC MT8183
> > > 1.ovl_2l share driver with ovl
> >
> > I think this is done in [1], [2], [3], this patch just add the support
> > of mt8183-ovl and mt8183-ovl-2l.
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek?h=v5.8-rc6&id=132c6e250ed745443973cada8db17cdbaebdf551
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek?h=v5.8-rc6&id=318462d1a568634ba09263cc730cb0fb1d56c2b3
> > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek?h=v5.8-rc6&id=57148baac8b78461e394953cfd5317bde8f795ab
> >
> > > 2.rdma1 share drive with rdma0, but fifo size is different
> >
> > I think this is done in [4], this patch just add the support of mt8183-rdma.
> >
> > [4] https://patchwork.kernel.org/patch/11679549/
> >
> > > 3.add mt8183 mutex private data, and mmsys private data
> > > 4.add mt8183 main and external path module for crtc create
> >
> > The fourth item is the mmsys private data in third item, so you need
> > not to repeat it.
> >
>
> i will remove some useless description in next version.
> > >
> > > Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> > > ---
> > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 18 ++++++++++++
> > > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 ++++
> > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 47 ++++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 43 +++++++++++++++++++++++++++++
> > > 4 files changed, 114 insertions(+)
> > >
> >
> > [snip]
> >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > > index 014c1bb..60788c1 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > > @@ -15,6 +15,8 @@
> > >
> > > #define MT2701_DISP_MUTEX0_MOD0 0x2c
> > > #define MT2701_DISP_MUTEX0_SOF0 0x30
> > > +#define MT8183_DISP_MUTEX0_MOD0 0x30
> > > +#define MT8183_DISP_MUTEX0_SOF0 0x2c
> > >
> > > #define DISP_REG_MUTEX_EN(n) (0x20 + 0x20 * (n))
> > > #define DISP_REG_MUTEX(n) (0x24 + 0x20 * (n))
> > > @@ -25,6 +27,18 @@
> > >
> > > #define INT_MUTEX BIT(1)
> > >
> > > +#define MT8183_MUTEX_MOD_DISP_RDMA0 0
> > > +#define MT8183_MUTEX_MOD_DISP_RDMA1 1
> > > +#define MT8183_MUTEX_MOD_DISP_OVL0 9
> > > +#define MT8183_MUTEX_MOD_DISP_OVL0_2L 10
> > > +#define MT8183_MUTEX_MOD_DISP_OVL1_2L 11
> > > +#define MT8183_MUTEX_MOD_DISP_WDMA0 12
> > > +#define MT8183_MUTEX_MOD_DISP_COLOR0 13
> > > +#define MT8183_MUTEX_MOD_DISP_CCORR0 14
> > > +#define MT8183_MUTEX_MOD_DISP_AAL0 15
> > > +#define MT8183_MUTEX_MOD_DISP_GAMMA0 16
> > > +#define MT8183_MUTEX_MOD_DISP_DITHER0 17
> > > +
> > > #define MT8173_MUTEX_MOD_DISP_OVL0 11
> > > #define MT8173_MUTEX_MOD_DISP_OVL1 12
> > > #define MT8173_MUTEX_MOD_DISP_RDMA0 13
> > > @@ -74,6 +88,10 @@
> > > #define MUTEX_SOF_DSI2 5
> > > #define MUTEX_SOF_DSI3 6
> > >
> > > +#define MT8183_MUTEX_SOF_DPI0 2
> > > +#define MT8183_MUTEX_EOF_DSI0 (MUTEX_SOF_DSI0 << 6)
> > > +#define MT8183_MUTEX_EOF_DPI0 (MT8183_MUTEX_SOF_DPI0 << 6)
> > > +
> > >
> > > struct mtk_disp_mutex {
> > > int id;
> > > @@ -153,6 +171,20 @@ struct mtk_ddp {
> > > [DDP_COMPONENT_WDMA1] = MT8173_MUTEX_MOD_DISP_WDMA1,
> > > };
> > >
> > > +static const unsigned int mt8183_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> > > + [DDP_COMPONENT_AAL0] = MT8183_MUTEX_MOD_DISP_AAL0,
> > > + [DDP_COMPONENT_CCORR] = MT8183_MUTEX_MOD_DISP_CCORR0,
> > > + [DDP_COMPONENT_COLOR0] = MT8183_MUTEX_MOD_DISP_COLOR0,
> > > + [DDP_COMPONENT_DITHER] = MT8183_MUTEX_MOD_DISP_DITHER0,
> > > + [DDP_COMPONENT_GAMMA] = MT8183_MUTEX_MOD_DISP_GAMMA0,
> > > + [DDP_COMPONENT_OVL0] = MT8183_MUTEX_MOD_DISP_OVL0,
> > > + [DDP_COMPONENT_OVL_2L0] = MT8183_MUTEX_MOD_DISP_OVL0_2L,
> > > + [DDP_COMPONENT_OVL_2L1] = MT8183_MUTEX_MOD_DISP_OVL1_2L,
> > > + [DDP_COMPONENT_RDMA0] = MT8183_MUTEX_MOD_DISP_RDMA0,
> > > + [DDP_COMPONENT_RDMA1] = MT8183_MUTEX_MOD_DISP_RDMA1,
> > > + [DDP_COMPONENT_WDMA0] = MT8183_MUTEX_MOD_DISP_WDMA0,
> > > +};
> > > +
> > > static const unsigned int mt2712_mutex_sof[DDP_MUTEX_SOF_DSI3 + 1] = {
> > > [DDP_MUTEX_SOF_SINGLE_MODE] = MUTEX_SOF_SINGLE_MODE,
> > > [DDP_MUTEX_SOF_DSI0] = MUTEX_SOF_DSI0,
> > > @@ -163,6 +195,12 @@ struct mtk_ddp {
> > > [DDP_MUTEX_SOF_DSI3] = MUTEX_SOF_DSI3,
> > > };
> > >
> > > +static const unsigned int mt8183_mutex_sof[DDP_MUTEX_SOF_DSI3 + 1] = {
> > > + [DDP_MUTEX_SOF_SINGLE_MODE] = MUTEX_SOF_SINGLE_MODE,
> > > + [DDP_MUTEX_SOF_DSI0] = MUTEX_SOF_DSI0 | MT8183_MUTEX_EOF_DSI0,
> >
> > I think this array is for 'sof', so you should drop MT8183_MUTEX_EOF_DSI0.
> >
> > > + [DDP_MUTEX_SOF_DPI0] = MT8183_MUTEX_SOF_DPI0 | MT8183_MUTEX_EOF_DPI0,
> >
> > Ditto.
>
> we need set EOF at the same on MT8183, that is different with before SoC
> EOF and SOF are location in the same hardware register.

Even though EOF and SOF are located in the same hardware register, why
should we set EOF?
We just need mutex hardware to send SOF to MODs, we don't need mutex
to send EOF.

> >
> > Regards,
> > Chun-Kuang.
> >
> > > +};
> > > +
> > > static const struct mtk_ddp_data mt2701_ddp_driver_data = {
> > > .mutex_mod = mt2701_mutex_mod,
> > > .mutex_sof = mt2712_mutex_sof,
> > > @@ -184,6 +222,13 @@ struct mtk_ddp {
> > > .mutex_sof_reg = MT2701_DISP_MUTEX0_SOF0,
> > > };
> > >
>