Re: [PATCH] drm/mediatek: add mipi_tx driver for mt8183

[Jitao Shi]

On Tue, 2019-02-12 at 10:28 +0100, Matthias Brugger wrote:
> 
> On 12/02/2019 07:19, Jitao Shi wrote:
> > This patch adds mipi tx driver support for mt8183.
> > 
> > Mipi_tx of mt8183 is very different to mt8173.
> > 1.Separate mipi tx setting to mtk_mt8173_mipi_tx.c for mt8173
> > 2.Separate mipi tx setting to mtk_mt8183_mipi_tx.c for mt8183
> > 3.To reuse the common code, make the common functions in mtk_mipi_tx.c
> > 
> 
> I hadn't a look on the code, but this commit message already indicates, that 
> you
> should split this up in several patches.
> Something like this:
> 1. patch: carve out common functions to mtk_mipi_tx.c at this point only used 
> by
> mt8173
> 2. patch: add support for mt8183
> 
> Regards,
> Matthias
> 

Thanks for your advice. I'll split it to several patches.

Best Regards
Jitao

> > Signed-off-by: Jitao Shi 
> > ---
> >  drivers/gpu/drm/mediatek/Makefile |   2 +
> >  drivers/gpu/drm/mediatek/mtk_mipi_tx.c| 352 ++
> >  drivers/gpu/drm/mediatek/mtk_mipi_tx.h|  52 +++
> >  drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c | 290 +++
> >  drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c | 168 +
> >  5 files changed, 549 insertions(+), 315 deletions(-)
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_mipi_tx.h
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c
> > 
> > diff --git a/drivers/gpu/drm/mediatek/Makefile 
> > b/drivers/gpu/drm/mediatek/Makefile
> > index 82ae49c64221..8067a4be8311 100644
> > --- a/drivers/gpu/drm/mediatek/Makefile
> > +++ b/drivers/gpu/drm/mediatek/Makefile
> > @@ -12,6 +12,8 @@ mediatek-drm-y := mtk_disp_color.o \
> >   mtk_drm_plane.o \
> >   mtk_dsi.o \
> >   mtk_mipi_tx.o \
> > + mtk_mt8173_mipi_tx.o \
> > + mtk_mt8183_mipi_tx.o \
> >   mtk_dpi.o
> >  
> >  obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
> > diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c 
> > b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > index 90e913108950..7591a38ca565 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > @@ -11,292 +11,45 @@
> >   * GNU General Public License for more details.
> >   */
> >  
> > -#include 
> > -#include 
> > -#include 
> > -#include 
> > -#include 
> > -#include 
> > -#include 
> > -#include 
> > -
> > -#define MIPITX_DSI_CON 0x00
> > -#define RG_DSI_LDOCORE_EN  BIT(0)
> > -#define RG_DSI_CKG_LDOOUT_EN   BIT(1)
> > -#define RG_DSI_BCLK_SEL(3 << 2)
> > -#define RG_DSI_LD_IDX_SEL  (7 << 4)
> > -#define RG_DSI_PHYCLK_SEL  (2 << 8)
> > -#define RG_DSI_DSICLK_FREQ_SEL BIT(10)
> > -#define RG_DSI_LPTX_CLMP_ENBIT(11)
> > -
> > -#define MIPITX_DSI_CLOCK_LANE  0x04
> > -#define MIPITX_DSI_DATA_LANE0  0x08
> > -#define MIPITX_DSI_DATA_LANE1  0x0c
> > -#define MIPITX_DSI_DATA_LANE2  0x10
> > -#define MIPITX_DSI_DATA_LANE3  0x14
> > -#define RG_DSI_LNTx_LDOOUT_EN  BIT(0)
> > -#define RG_DSI_LNTx_CKLANE_EN  BIT(1)
> > -#define RG_DSI_LNTx_LPTX_IPLUS1BIT(2)
> > -#define RG_DSI_LNTx_LPTX_IPLUS2BIT(3)
> > -#define RG_DSI_LNTx_LPTX_IMINUSBIT(4)
> > -#define RG_DSI_LNTx_LPCD_IPLUS BIT(5)
> > -#define RG_DSI_LNTx_LPCD_IMINUSBIT(6)
> > -#define RG_DSI_LNTx_RT_CODE(0xf << 8)
> > -
> > -#define MIPITX_DSI_TOP_CON 0x40
> > -#define RG_DSI_LNT_INTR_EN BIT(0)
> > -#define RG_DSI_LNT_HS_BIAS_EN  BIT(1)
> > -#define RG_DSI_LNT_IMP_CAL_EN  BIT(2)
> > -#define RG_DSI_LNT_TESTMODE_EN BIT(3)
> > -#define RG_DSI_LNT_IMP_CAL_CODE(0xf << 4)
> > -#define RG_DSI_LNT_AIO_SEL (7 << 8)
> > -#define RG_DSI_PAD_TIE_LOW_EN  BIT(11)
> > -#define RG_DSI_DEBUG_INPUT_EN  BIT(12)
> > -#define RG_DSI_PRESERVE(7 << 13)
> > -
> > -#define MIPITX_DSI_BG_CON  0x44
> > -#define RG_DSI_BG_CORE_EN  BIT(0)
> > -#define RG_DSI_BG_CKEN BIT(1)
> > -#define RG_DSI_BG_DIV  (0x3 << 2)
> > -#define RG_DSI_BG_FAST_CHARGE  BIT(4)
> > -#define RG_DSI_VOUT_MSK(0x3 << 5)
> > -#define RG_DSI_V12_SEL (7 << 5)
> > -#define RG_DSI_V10_SEL (7 << 8)
> > -#define RG_DSI_V072_SEL(7 << 11)
> > -#define RG_DSI_V04_SEL (7 << 14)
> > -#define RG_DSI_V032_SEL(7 << 17)
> > -#define RG_DSI_V02_SEL (7 << 20)
> > -#define RG_DSI_BG_R1_TRIM  (0xf << 24)
> > -#define RG_DSI_BG_R2_TRIM  (0xf << 28)
> > -
> > -#define MIPITX_DSI_PLL_CON00x50
> > -#define RG_DSI_MPPLL_PLL_ENBIT(0)
> > -#define 

Re: [PATCH] drm/mediatek: add mipi_tx driver for mt8183

[CK Hu]

Hi, Jitao:

On Tue, 2019-02-12 at 14:19 +0800, Jitao Shi wrote:
> This patch adds mipi tx driver support for mt8183.
> 
> Mipi_tx of mt8183 is very different to mt8173.
> 1.Separate mipi tx setting to mtk_mt8173_mipi_tx.c for mt8173
> 2.Separate mipi tx setting to mtk_mt8183_mipi_tx.c for mt8183
> 3.To reuse the common code, make the common functions in mtk_mipi_tx.c
> 
> Signed-off-by: Jitao Shi 
> ---
>  drivers/gpu/drm/mediatek/Makefile |   2 +
>  drivers/gpu/drm/mediatek/mtk_mipi_tx.c| 352 ++
>  drivers/gpu/drm/mediatek/mtk_mipi_tx.h|  52 +++
>  drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c | 290 +++
>  drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c | 168 +
>  5 files changed, 549 insertions(+), 315 deletions(-)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mipi_tx.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c
> 
> diff --git a/drivers/gpu/drm/mediatek/Makefile 
> b/drivers/gpu/drm/mediatek/Makefile
> index 82ae49c64221..8067a4be8311 100644
> --- a/drivers/gpu/drm/mediatek/Makefile
> +++ b/drivers/gpu/drm/mediatek/Makefile
> @@ -12,6 +12,8 @@ mediatek-drm-y := mtk_disp_color.o \
> mtk_drm_plane.o \
> mtk_dsi.o \
> mtk_mipi_tx.o \
> +   mtk_mt8173_mipi_tx.o \
> +   mtk_mt8183_mipi_tx.o \
> mtk_dpi.o
>  
>  obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
> diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c 
> b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> index 90e913108950..7591a38ca565 100644
> --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> @@ -11,292 +11,45 @@
>   * GNU General Public License for more details.
>   */
>  
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#define MIPITX_DSI_CON   0x00
> -#define RG_DSI_LDOCORE_ENBIT(0)
> -#define RG_DSI_CKG_LDOOUT_EN BIT(1)
> -#define RG_DSI_BCLK_SEL  (3 << 2)
> -#define RG_DSI_LD_IDX_SEL(7 << 4)
> -#define RG_DSI_PHYCLK_SEL(2 << 8)
> -#define RG_DSI_DSICLK_FREQ_SEL   BIT(10)
> -#define RG_DSI_LPTX_CLMP_EN  BIT(11)
> -
> -#define MIPITX_DSI_CLOCK_LANE0x04
> -#define MIPITX_DSI_DATA_LANE00x08
> -#define MIPITX_DSI_DATA_LANE10x0c
> -#define MIPITX_DSI_DATA_LANE20x10
> -#define MIPITX_DSI_DATA_LANE30x14
> -#define RG_DSI_LNTx_LDOOUT_ENBIT(0)
> -#define RG_DSI_LNTx_CKLANE_ENBIT(1)
> -#define RG_DSI_LNTx_LPTX_IPLUS1  BIT(2)
> -#define RG_DSI_LNTx_LPTX_IPLUS2  BIT(3)
> -#define RG_DSI_LNTx_LPTX_IMINUS  BIT(4)
> -#define RG_DSI_LNTx_LPCD_IPLUS   BIT(5)
> -#define RG_DSI_LNTx_LPCD_IMINUS  BIT(6)
> -#define RG_DSI_LNTx_RT_CODE  (0xf << 8)
> -
> -#define MIPITX_DSI_TOP_CON   0x40
> -#define RG_DSI_LNT_INTR_EN   BIT(0)
> -#define RG_DSI_LNT_HS_BIAS_ENBIT(1)
> -#define RG_DSI_LNT_IMP_CAL_ENBIT(2)
> -#define RG_DSI_LNT_TESTMODE_EN   BIT(3)
> -#define RG_DSI_LNT_IMP_CAL_CODE  (0xf << 4)
> -#define RG_DSI_LNT_AIO_SEL   (7 << 8)
> -#define RG_DSI_PAD_TIE_LOW_ENBIT(11)
> -#define RG_DSI_DEBUG_INPUT_ENBIT(12)
> -#define RG_DSI_PRESERVE  (7 << 13)
> -
> -#define MIPITX_DSI_BG_CON0x44
> -#define RG_DSI_BG_CORE_ENBIT(0)
> -#define RG_DSI_BG_CKEN   BIT(1)
> -#define RG_DSI_BG_DIV(0x3 << 2)
> -#define RG_DSI_BG_FAST_CHARGEBIT(4)
> -#define RG_DSI_VOUT_MSK  (0x3 << 5)
> -#define RG_DSI_V12_SEL   (7 << 5)
> -#define RG_DSI_V10_SEL   (7 << 8)
> -#define RG_DSI_V072_SEL  (7 << 11)
> -#define RG_DSI_V04_SEL   (7 << 14)
> -#define RG_DSI_V032_SEL  (7 << 17)
> -#define RG_DSI_V02_SEL   (7 << 20)
> -#define RG_DSI_BG_R1_TRIM(0xf << 24)
> -#define RG_DSI_BG_R2_TRIM(0xf << 28)
> -
> -#define MIPITX_DSI_PLL_CON0  0x50
> -#define RG_DSI_MPPLL_PLL_EN  BIT(0)
> -#define RG_DSI_MPPLL_DIV_MSK (0x1ff << 1)
> -#define RG_DSI_MPPLL_PREDIV  (3 << 1)
> -#define RG_DSI_MPPLL_TXDIV0  (3 << 3)
> -#define RG_DSI_MPPLL_TXDIV1  (3 << 5)
> -#define RG_DSI_MPPLL_POSDIV  (7 << 7)
> -#define RG_DSI_MPPLL_MONVC_ENBIT(10)
> -#define RG_DSI_MPPLL_MONREF_EN   BIT(11)
> -#define RG_DSI_MPPLL_VOD_EN  BIT(12)
> -
> -#define MIPITX_DSI_PLL_CON1  0x54
> -#define RG_DSI_MPPLL_SDM_FRA_EN  BIT(0)
> -#define RG_DSI_MPPLL_SDM_SSC_PH_INIT BIT(1)
> -#define RG_DSI_MPPLL_SDM_SSC_EN

Re: [PATCH] drm/mediatek: add mipi_tx driver for mt8183

[Matthias Brugger]

On 12/02/2019 07:19, Jitao Shi wrote:
> This patch adds mipi tx driver support for mt8183.
> 
> Mipi_tx of mt8183 is very different to mt8173.
> 1.Separate mipi tx setting to mtk_mt8173_mipi_tx.c for mt8173
> 2.Separate mipi tx setting to mtk_mt8183_mipi_tx.c for mt8183
> 3.To reuse the common code, make the common functions in mtk_mipi_tx.c
> 

I hadn't a look on the code, but this commit message already indicates, that you
should split this up in several patches.
Something like this:
1. patch: carve out common functions to mtk_mipi_tx.c at this point only used by
mt8173
2. patch: add support for mt8183

Regards,
Matthias

> Signed-off-by: Jitao Shi 
> ---
>  drivers/gpu/drm/mediatek/Makefile |   2 +
>  drivers/gpu/drm/mediatek/mtk_mipi_tx.c| 352 ++
>  drivers/gpu/drm/mediatek/mtk_mipi_tx.h|  52 +++
>  drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c | 290 +++
>  drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c | 168 +
>  5 files changed, 549 insertions(+), 315 deletions(-)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mipi_tx.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c
> 
> diff --git a/drivers/gpu/drm/mediatek/Makefile 
> b/drivers/gpu/drm/mediatek/Makefile
> index 82ae49c64221..8067a4be8311 100644
> --- a/drivers/gpu/drm/mediatek/Makefile
> +++ b/drivers/gpu/drm/mediatek/Makefile
> @@ -12,6 +12,8 @@ mediatek-drm-y := mtk_disp_color.o \
> mtk_drm_plane.o \
> mtk_dsi.o \
> mtk_mipi_tx.o \
> +   mtk_mt8173_mipi_tx.o \
> +   mtk_mt8183_mipi_tx.o \
> mtk_dpi.o
>  
>  obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
> diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c 
> b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> index 90e913108950..7591a38ca565 100644
> --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> @@ -11,292 +11,45 @@
>   * GNU General Public License for more details.
>   */
>  
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#define MIPITX_DSI_CON   0x00
> -#define RG_DSI_LDOCORE_ENBIT(0)
> -#define RG_DSI_CKG_LDOOUT_EN BIT(1)
> -#define RG_DSI_BCLK_SEL  (3 << 2)
> -#define RG_DSI_LD_IDX_SEL(7 << 4)
> -#define RG_DSI_PHYCLK_SEL(2 << 8)
> -#define RG_DSI_DSICLK_FREQ_SEL   BIT(10)
> -#define RG_DSI_LPTX_CLMP_EN  BIT(11)
> -
> -#define MIPITX_DSI_CLOCK_LANE0x04
> -#define MIPITX_DSI_DATA_LANE00x08
> -#define MIPITX_DSI_DATA_LANE10x0c
> -#define MIPITX_DSI_DATA_LANE20x10
> -#define MIPITX_DSI_DATA_LANE30x14
> -#define RG_DSI_LNTx_LDOOUT_ENBIT(0)
> -#define RG_DSI_LNTx_CKLANE_ENBIT(1)
> -#define RG_DSI_LNTx_LPTX_IPLUS1  BIT(2)
> -#define RG_DSI_LNTx_LPTX_IPLUS2  BIT(3)
> -#define RG_DSI_LNTx_LPTX_IMINUS  BIT(4)
> -#define RG_DSI_LNTx_LPCD_IPLUS   BIT(5)
> -#define RG_DSI_LNTx_LPCD_IMINUS  BIT(6)
> -#define RG_DSI_LNTx_RT_CODE  (0xf << 8)
> -
> -#define MIPITX_DSI_TOP_CON   0x40
> -#define RG_DSI_LNT_INTR_EN   BIT(0)
> -#define RG_DSI_LNT_HS_BIAS_ENBIT(1)
> -#define RG_DSI_LNT_IMP_CAL_ENBIT(2)
> -#define RG_DSI_LNT_TESTMODE_EN   BIT(3)
> -#define RG_DSI_LNT_IMP_CAL_CODE  (0xf << 4)
> -#define RG_DSI_LNT_AIO_SEL   (7 << 8)
> -#define RG_DSI_PAD_TIE_LOW_ENBIT(11)
> -#define RG_DSI_DEBUG_INPUT_ENBIT(12)
> -#define RG_DSI_PRESERVE  (7 << 13)
> -
> -#define MIPITX_DSI_BG_CON0x44
> -#define RG_DSI_BG_CORE_ENBIT(0)
> -#define RG_DSI_BG_CKEN   BIT(1)
> -#define RG_DSI_BG_DIV(0x3 << 2)
> -#define RG_DSI_BG_FAST_CHARGEBIT(4)
> -#define RG_DSI_VOUT_MSK  (0x3 << 5)
> -#define RG_DSI_V12_SEL   (7 << 5)
> -#define RG_DSI_V10_SEL   (7 << 8)
> -#define RG_DSI_V072_SEL  (7 << 11)
> -#define RG_DSI_V04_SEL   (7 << 14)
> -#define RG_DSI_V032_SEL  (7 << 17)
> -#define RG_DSI_V02_SEL   (7 << 20)
> -#define RG_DSI_BG_R1_TRIM(0xf << 24)
> -#define RG_DSI_BG_R2_TRIM(0xf << 28)
> -
> -#define MIPITX_DSI_PLL_CON0  0x50
> -#define RG_DSI_MPPLL_PLL_EN  BIT(0)
> -#define RG_DSI_MPPLL_DIV_MSK (0x1ff << 1)
> -#define RG_DSI_MPPLL_PREDIV  (3 << 1)
> -#define RG_DSI_MPPLL_TXDIV0  (3 << 3)
> -#define RG_DSI_MPPLL_TXDIV1  (3 << 5)
> -#define RG_DSI_MPPLL_POSDIV  (7 << 7)
> -#define RG_DSI_MPPLL_MONVC_ENBIT(10)
> -#define