mbox series

[v6,00/14] add drm support for MT8183

Message ID 1577937624-14313-1-git-send-email-yongqiang.niu@mediatek.com (mailing list archive)
Headers show
Series add drm support for MT8183 | expand

Message

Yongqiang Niu Jan. 2, 2020, 4 a.m. UTC
This series are based on 5.5-rc1 and provid 14 patch
to support mediatek SOC MT8183

Change since v5
- fix reviewed issue in v5

Change since v4
- fix reviewed issue in v4

Change since v3
- fix reviewed issue in v3
- fix type error in v3
- fix conflict with iommu patch

Change since v2
- fix reviewed issue in v2
- add mutex node into dts file

Changes since v1:
- fix reviewed issue in v1
- add dts for mt8183 display nodes
- adjust display clock control flow in patch 22
- add vmap support for mediatek drm in patch 23
- fix page offset issue for mmap function in patch 24
- enable allow_fb_modifiers for mediatek drm in patch 25

Yongqiang Niu (14):
  arm64: dts: add display nodes for mt8183
  drm/mediatek: move dsi/dpi select input into mtk_ddp_sel_in
  drm/mediatek: make sout select function  format same with select input
  drm/mediatek: add mmsys private data for ddp path config
  drm/mediatek: move rdma sout from mtk_ddp_mout_en into
    mtk_ddp_sout_sel
  drm/mediatek: add connection from OVL0 to OVL_2L0
  drm/mediatek: add connection from RDMA0 to COLOR0
  drm/mediatek: add connection from RDMA1 to DSI0
  drm/mediatek: add connection from OVL_2L0 to RDMA0
  drm/mediatek: add connection from OVL_2L1 to RDMA1
  drm/mediatek: add connection from DITHER0 to DSI0
  drm/mediatek: add connection from RDMA0 to DSI0
  drm/mediatek: add fifo_size into rdma private data
  drm/mediatek: add support for mediatek SOC MT8183

 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 103 +++++++++++
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c  |  18 ++
 drivers/gpu/drm/mediatek/mtk_disp_rdma.c |  27 ++-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c  |   4 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c   | 291 +++++++++++++++++++++++--------
 drivers/gpu/drm/mediatek/mtk_drm_ddp.h   |   7 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c   |  51 ++++++
 drivers/gpu/drm/mediatek/mtk_drm_drv.h   |   3 +
 8 files changed, 434 insertions(+), 70 deletions(-)

Comments

Yongqiang Niu Jan. 2, 2020, 6:21 a.m. UTC | #1
On Thu, 2020-01-02 at 14:02 +0800, CK Hu wrote:
> Hi, Yongqiang:
> 
> On Thu, 2020-01-02 at 13:39 +0800, Yongqiang Niu wrote:
> > On Thu, 2020-01-02 at 13:03 +0800, CK Hu wrote:
> > > Hi, Yongqiang:
> > > 
> > > On Thu, 2020-01-02 at 12:00 +0800, Yongqiang Niu wrote:
> > > > move dsi/dpi select input into mtk_ddp_sel_in
> > > > 
> > > > Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> > > > ---
> > > >  drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 10 ++++++----
> > > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > > > index 39700b9..91c9b19 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > > > @@ -376,6 +376,12 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
> > > >  	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
> > > >  		*addr = DISP_REG_CONFIG_DSI_SEL;
> > > >  		value = DSI_SEL_IN_BLS;
> > > > +	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI0) {
> > > > +		*addr = DISP_REG_CONFIG_DSI_SEL;
> > > > +		value = DSI_SEL_IN_RDMA;
> > > 
> > > In original code, this is set when cur == DDP_COMPONENT_BLS and next ==
> > > DDP_COMPONENT_DPI0. Why do you change the condition?
> > > 
> > > Regards,
> > > CK
> > 
> > if bls connect with dpi0, rdma1 should connect with dsi0, the condition
> > is same with before.
> 
> You suggest that two crtcs are both enabled. If only one crtc is
> enabled, just one of these two would be set.
> 
> Regards,
> CK

OK, i will modify like this
else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
		*addr = DISP_REG_CONFIG_DSI_SEL;
		value = DPI_SEL_IN_RDMA;
	}
in mtk_ddp_sel_in.

don't set DISP_REG_CONFIG_DPI_SEL to DPI_SEL_IN_BLS anymore, because
DPI_SEL_IN_BLS is zero, it is same with hardware default setting.
> 
> > > 
> > > > +	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
> > > > +		*addr = DISP_REG_CONFIG_DPI_SEL;
> > > > +		value = DPI_SEL_IN_BLS;
> > > >  	} else {
> > > >  		value = 0;
> > > >  	}
> > > > @@ -393,10 +399,6 @@ static void mtk_ddp_sout_sel(struct regmap *config_regs,
> > > >  	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
> > > >  		regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
> > > >  				BLS_TO_DPI_RDMA1_TO_DSI);
> > > > -		regmap_write(config_regs, DISP_REG_CONFIG_DSI_SEL,
> > > > -				DSI_SEL_IN_RDMA);
> > > > -		regmap_write(config_regs, DISP_REG_CONFIG_DPI_SEL,
> > > > -				DPI_SEL_IN_BLS);
> > > >  	}
> > > >  }
> > > >  
> > > 
> > > 
> > 
> > 
> 
>
Yongqiang Niu Jan. 2, 2020, 7 a.m. UTC | #2
On Thu, 2020-01-02 at 14:40 +0800, CK Hu wrote:
> Hi, Yongqiang:
> 
> On Thu, 2020-01-02 at 14:21 +0800, Yongqiang Niu wrote:
> > On Thu, 2020-01-02 at 14:02 +0800, CK Hu wrote:
> > > Hi, Yongqiang:
> > > 
> > > On Thu, 2020-01-02 at 13:39 +0800, Yongqiang Niu wrote:
> > > > On Thu, 2020-01-02 at 13:03 +0800, CK Hu wrote:
> > > > > Hi, Yongqiang:
> > > > > 
> > > > > On Thu, 2020-01-02 at 12:00 +0800, Yongqiang Niu wrote:
> > > > > > move dsi/dpi select input into mtk_ddp_sel_in
> > > > > > 
> > > > > > Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 10 ++++++----
> > > > > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > > > > > index 39700b9..91c9b19 100644
> > > > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > > > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > > > > > @@ -376,6 +376,12 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
> > > > > >  	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
> > > > > >  		*addr = DISP_REG_CONFIG_DSI_SEL;
> > > > > >  		value = DSI_SEL_IN_BLS;
> > > > > > +	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI0) {
> > > > > > +		*addr = DISP_REG_CONFIG_DSI_SEL;
> > > > > > +		value = DSI_SEL_IN_RDMA;
> > > > > 
> > > > > In original code, this is set when cur == DDP_COMPONENT_BLS and next ==
> > > > > DDP_COMPONENT_DPI0. Why do you change the condition?
> > > > > 
> > > > > Regards,
> > > > > CK
> > > > 
> > > > if bls connect with dpi0, rdma1 should connect with dsi0, the condition
> > > > is same with before.
> > > 
> > > You suggest that two crtcs are both enabled. If only one crtc is
> > > enabled, just one of these two would be set.
> > > 
> > > Regards,
> > > CK
> > 
> > OK, i will modify like this
> > else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
> > 		*addr = DISP_REG_CONFIG_DSI_SEL;
> > 		value = DPI_SEL_IN_RDMA;
> > 	}
> > in mtk_ddp_sel_in.
> > 
> > don't set DISP_REG_CONFIG_DPI_SEL to DPI_SEL_IN_BLS anymore, because
> > DPI_SEL_IN_BLS is zero, it is same with hardware default setting.
> 
> In Bibby's case, there is only the path from BLS to DPI0 and has no path
> from RDMA1 to DSI0, but it need to set these two register. Maybe its
> setting is just for some SoC, so you may use the compatible name to
> judge how to set this two register.
> 
> Regards,
> CK
> 

it the original use case, if bls->dpi0, it set 3 register,
DISP_REG_CONFIG_DPI_SEL set to DPI_SEL_IN_BLS ,and this is 
0, so this is useless setting.

then are only 2 useful setting.
in this patch i have upload, i keep DISP_REG_CONFIG_OUT_SEL
still in mtk_ddp_sout_sel.
and only move DISP_REG_CONFIG_DSI_SEL into mtk_ddp_sel_in.
i suppose this is enough for this use case.
and no need compatible name to control this.
please double confirm.

and there will more and more SOC upstream in the future.
these function will be more complex.
there should be coding one more suitable function to 
handle these connection

> > > 
> > > > > 
> > > > > > +	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
> > > > > > +		*addr = DISP_REG_CONFIG_DPI_SEL;
> > > > > > +		value = DPI_SEL_IN_BLS;
> > > > > >  	} else {
> > > > > >  		value = 0;
> > > > > >  	}
> > > > > > @@ -393,10 +399,6 @@ static void mtk_ddp_sout_sel(struct regmap *config_regs,
> > > > > >  	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
> > > > > >  		regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
> > > > > >  				BLS_TO_DPI_RDMA1_TO_DSI);
> > > > > > -		regmap_write(config_regs, DISP_REG_CONFIG_DSI_SEL,
> > > > > > -				DSI_SEL_IN_RDMA);
> > > > > > -		regmap_write(config_regs, DISP_REG_CONFIG_DPI_SEL,
> > > > > > -				DPI_SEL_IN_BLS);
> > > > > >  	}
> > > > > >  }
> > > > > >  
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> > 
> 
>