Re: [PATCH v4 5/8] drm/mediatek: add dsi interrupt control

[YT Shen]

Hi CK,

On Tue, 2016-07-19 at 13:39 +0800, CK Hu wrote:
> Hi, YT:
> 
> Some comments inline.
> 
> On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote:
> > From: shaoming chen 
> > 
> > add dsi interrupt control
> > 
> > Signed-off-by: shaoming chen 
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dsi.c |  130 
> > 
> >  1 file changed, 130 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
> > b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index 2d808e5..de5ad7f 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -18,6 +18,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -32,6 +33,13 @@
> >  
> >  #define DSI_START  0x00
> >  
> > +#define DSI_INTEN  0x08
> > +
> > +#define DSI_INTSTA 0x0c
> > +#define LPRX_RD_RDY_INT_FLAG   BIT(0)
> > +#define CMD_DONE_INT_FLAG  BIT(1)
> > +#define DSI_BUSY   BIT(31)
> 
> Why need LPRX_RD_RDY_INT_FLAG, CMD_DONE_INT_FLAG, and DSI_BUSY? Maybe
> these three should be moved to other patch.
OK.

> 
> > +
> >  #define DSI_CON_CTRL   0x10
> >  #define DSI_RESET  BIT(0)
> >  #define DSI_EN BIT(1)
> > @@ -74,6 +82,9 @@
> >  
> >  #define DSI_HSTX_CKL_WC0x64
> >  
> > +#define DSI_RACK   0x84
> > +#define RACK   BIT(0)
> > +
> >  #define DSI_PHY_LCCON  0x104
> >  #define LC_HS_TX_ENBIT(0)
> >  #define LC_ULPM_EN BIT(1)
> > @@ -134,6 +145,18 @@ struct mtk_dsi {
> > struct videomode vm;
> > int refcount;
> > bool enabled;
> > +   int irq_num, irq_data;
> > +};
> > +
> > +enum {
> > +   DSI_INT_SLEEPOUT_DONE_FLAG  = BIT(6),
> > +   DSI_INT_VM_CMD_DONE_FLAG= BIT(5),
> > +   DSI_INT_EXT_TE_RDY_FLAG = BIT(4),
> > +   DSI_INT_VM_DONE_FLAG= BIT(3),
> > +   DSI_INT_TE_RDY_FLAG = BIT(2),
> > +   DSI_INT_CMD_DONE_FLAG   = BIT(1),
> > +   DSI_INT_LPRX_RD_RDY_FLAG= BIT(0),
> > +   DSI_INT_ALL_BITS= (0x7f)
> >  };
> 
> I think you should use '#define' instead of 'enum'. The code would be
> like below, and these definition should be moved to after DSI_INTEN or
> DSI_INTSTA.
> 
> #define DSI_INT_LPRX_RD_RDY_FLAG  BIT(0)
> #define DSI_INT_CMD_DONE_FLAG BIT(1)
> #define DSI_INT_TE_RDY_FLAG   BIT(2)
> #define DSI_INT_VM_DONE_FLAG  BIT(3)
> #define DSI_INT_EXT_TE_RDY_FLAG   BIT(4)
> #define DSI_INT_VM_CMD_DONE_FLAG  BIT(5)
> #define DSI_INT_SLEEPOUT_DONE_FLAGBIT(6)
> #define DSI_INT_ALL_BITS  (DSI_INT_LPRX_RD_RDY_FLAG | \
>   DSI_INT_CMD_DONE_FLAG | \
>   DSI_INT_TE_RDY_FLAG | \
>   DSI_INT_VM_DONE_FLAG | \
>   DSI_INT_EXT_TE_RDY_FLAG | \
>   DSI_INT_VM_CMD_DONE_FLAG | \
>   DSI_INT_SLEEPOUT_DONE_FLAG)
> 
I will update this part in the next version.

> >  
> >  static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> > @@ -440,6 +463,94 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
> > writel(1, dsi->regs + DSI_START);
> >  }
> >  
> > +static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> > +{
> > +   u32 inten = DSI_INT_ALL_BITS;
> > +
> > +   if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO)
> > +   inten &= ~(DSI_INT_TE_RDY_FLAG | DSI_INT_EXT_TE_RDY_FLAG);
> > +
> > +   writel(inten, dsi->regs + DSI_INTEN);
> > +}
> > +
> > +static void mtk_dsi_irq_wakeup(struct mtk_dsi *dsi, u32 irq_bit)
> > +{
> > +   dsi->irq_data |= irq_bit;
> > +}
> > +
> > +static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> > +{
> > +   struct mtk_dsi *dsi = dev_id;
> > +
> > +   u32 status, tmp;
> > +
> > +   status = readl(dsi->regs + DSI_INTSTA);
> > +
> > +   if (status & DSI_INT_LPRX_RD_RDY_FLAG) {
> > +   /* write clear RD_RDY interrupt */
> > +   /* write clear RD_RDY interrupt must be before DSI_RACK */
> > +   /* because CMD_DONE will raise after DSI_RACK, */
> > +   /* so write clear RD_RDY after that will clear CMD_DONE too */
> > +   do {
> > +   /* send read ACK */
> > +   mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
> > +   tmp = readl(dsi->regs + DSI_INTSTA);
> > +   } while (tmp & DSI_BUSY);
> > +
> > +   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_LPRX_RD_RDY_FLAG, 0);
> > +   mtk_dsi_irq_wakeup(dsi, DSI_INT_LPRX_RD_RDY_FLAG);
> > +   }
> > +
> > +   if (status & DSI_INT_CMD_DONE_FLAG) {
> > +   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_CMD_DONE_FLAG, 0);
> > +   mtk_dsi_irq_wakeup(dsi, 

Re: [PATCH v4 5/8] drm/mediatek: add dsi interrupt control

[YT Shen]

Hi CK,

On Tue, 2016-07-19 at 13:39 +0800, CK Hu wrote:
> Hi, YT:
> 
> Some comments inline.
> 
> On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote:
> > From: shaoming chen 
> > 
> > add dsi interrupt control
> > 
> > Signed-off-by: shaoming chen 
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dsi.c |  130 
> > 
> >  1 file changed, 130 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
> > b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index 2d808e5..de5ad7f 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -18,6 +18,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -32,6 +33,13 @@
> >  
> >  #define DSI_START  0x00
> >  
> > +#define DSI_INTEN  0x08
> > +
> > +#define DSI_INTSTA 0x0c
> > +#define LPRX_RD_RDY_INT_FLAG   BIT(0)
> > +#define CMD_DONE_INT_FLAG  BIT(1)
> > +#define DSI_BUSY   BIT(31)
> 
> Why need LPRX_RD_RDY_INT_FLAG, CMD_DONE_INT_FLAG, and DSI_BUSY? Maybe
> these three should be moved to other patch.
OK.

> 
> > +
> >  #define DSI_CON_CTRL   0x10
> >  #define DSI_RESET  BIT(0)
> >  #define DSI_EN BIT(1)
> > @@ -74,6 +82,9 @@
> >  
> >  #define DSI_HSTX_CKL_WC0x64
> >  
> > +#define DSI_RACK   0x84
> > +#define RACK   BIT(0)
> > +
> >  #define DSI_PHY_LCCON  0x104
> >  #define LC_HS_TX_ENBIT(0)
> >  #define LC_ULPM_EN BIT(1)
> > @@ -134,6 +145,18 @@ struct mtk_dsi {
> > struct videomode vm;
> > int refcount;
> > bool enabled;
> > +   int irq_num, irq_data;
> > +};
> > +
> > +enum {
> > +   DSI_INT_SLEEPOUT_DONE_FLAG  = BIT(6),
> > +   DSI_INT_VM_CMD_DONE_FLAG= BIT(5),
> > +   DSI_INT_EXT_TE_RDY_FLAG = BIT(4),
> > +   DSI_INT_VM_DONE_FLAG= BIT(3),
> > +   DSI_INT_TE_RDY_FLAG = BIT(2),
> > +   DSI_INT_CMD_DONE_FLAG   = BIT(1),
> > +   DSI_INT_LPRX_RD_RDY_FLAG= BIT(0),
> > +   DSI_INT_ALL_BITS= (0x7f)
> >  };
> 
> I think you should use '#define' instead of 'enum'. The code would be
> like below, and these definition should be moved to after DSI_INTEN or
> DSI_INTSTA.
> 
> #define DSI_INT_LPRX_RD_RDY_FLAG  BIT(0)
> #define DSI_INT_CMD_DONE_FLAG BIT(1)
> #define DSI_INT_TE_RDY_FLAG   BIT(2)
> #define DSI_INT_VM_DONE_FLAG  BIT(3)
> #define DSI_INT_EXT_TE_RDY_FLAG   BIT(4)
> #define DSI_INT_VM_CMD_DONE_FLAG  BIT(5)
> #define DSI_INT_SLEEPOUT_DONE_FLAGBIT(6)
> #define DSI_INT_ALL_BITS  (DSI_INT_LPRX_RD_RDY_FLAG | \
>   DSI_INT_CMD_DONE_FLAG | \
>   DSI_INT_TE_RDY_FLAG | \
>   DSI_INT_VM_DONE_FLAG | \
>   DSI_INT_EXT_TE_RDY_FLAG | \
>   DSI_INT_VM_CMD_DONE_FLAG | \
>   DSI_INT_SLEEPOUT_DONE_FLAG)
> 
I will update this part in the next version.

> >  
> >  static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> > @@ -440,6 +463,94 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
> > writel(1, dsi->regs + DSI_START);
> >  }
> >  
> > +static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> > +{
> > +   u32 inten = DSI_INT_ALL_BITS;
> > +
> > +   if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO)
> > +   inten &= ~(DSI_INT_TE_RDY_FLAG | DSI_INT_EXT_TE_RDY_FLAG);
> > +
> > +   writel(inten, dsi->regs + DSI_INTEN);
> > +}
> > +
> > +static void mtk_dsi_irq_wakeup(struct mtk_dsi *dsi, u32 irq_bit)
> > +{
> > +   dsi->irq_data |= irq_bit;
> > +}
> > +
> > +static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> > +{
> > +   struct mtk_dsi *dsi = dev_id;
> > +
> > +   u32 status, tmp;
> > +
> > +   status = readl(dsi->regs + DSI_INTSTA);
> > +
> > +   if (status & DSI_INT_LPRX_RD_RDY_FLAG) {
> > +   /* write clear RD_RDY interrupt */
> > +   /* write clear RD_RDY interrupt must be before DSI_RACK */
> > +   /* because CMD_DONE will raise after DSI_RACK, */
> > +   /* so write clear RD_RDY after that will clear CMD_DONE too */
> > +   do {
> > +   /* send read ACK */
> > +   mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
> > +   tmp = readl(dsi->regs + DSI_INTSTA);
> > +   } while (tmp & DSI_BUSY);
> > +
> > +   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_LPRX_RD_RDY_FLAG, 0);
> > +   mtk_dsi_irq_wakeup(dsi, DSI_INT_LPRX_RD_RDY_FLAG);
> > +   }
> > +
> > +   if (status & DSI_INT_CMD_DONE_FLAG) {
> > +   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_CMD_DONE_FLAG, 0);
> > +   mtk_dsi_irq_wakeup(dsi, DSI_INT_CMD_DONE_FLAG);
> > +   }
> > +
> > +   if (status & 

Re: [PATCH v4 5/8] drm/mediatek: add dsi interrupt control

[CK Hu]

Hi, YT:

Some comments inline.

On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote:
> From: shaoming chen 
> 
> add dsi interrupt control
> 
> Signed-off-by: shaoming chen 
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c |  130 
> 
>  1 file changed, 130 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 2d808e5..de5ad7f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -32,6 +33,13 @@
>  
>  #define DSI_START0x00
>  
> +#define DSI_INTEN0x08
> +
> +#define DSI_INTSTA   0x0c
> +#define LPRX_RD_RDY_INT_FLAG BIT(0)
> +#define CMD_DONE_INT_FLAGBIT(1)
> +#define DSI_BUSY BIT(31)

Why need LPRX_RD_RDY_INT_FLAG, CMD_DONE_INT_FLAG, and DSI_BUSY? Maybe
these three should be moved to other patch.

> +
>  #define DSI_CON_CTRL 0x10
>  #define DSI_RESETBIT(0)
>  #define DSI_EN   BIT(1)
> @@ -74,6 +82,9 @@
>  
>  #define DSI_HSTX_CKL_WC  0x64
>  
> +#define DSI_RACK 0x84
> +#define RACK BIT(0)
> +
>  #define DSI_PHY_LCCON0x104
>  #define LC_HS_TX_EN  BIT(0)
>  #define LC_ULPM_EN   BIT(1)
> @@ -134,6 +145,18 @@ struct mtk_dsi {
>   struct videomode vm;
>   int refcount;
>   bool enabled;
> + int irq_num, irq_data;
> +};
> +
> +enum {
> + DSI_INT_SLEEPOUT_DONE_FLAG  = BIT(6),
> + DSI_INT_VM_CMD_DONE_FLAG= BIT(5),
> + DSI_INT_EXT_TE_RDY_FLAG = BIT(4),
> + DSI_INT_VM_DONE_FLAG= BIT(3),
> + DSI_INT_TE_RDY_FLAG = BIT(2),
> + DSI_INT_CMD_DONE_FLAG   = BIT(1),
> + DSI_INT_LPRX_RD_RDY_FLAG= BIT(0),
> + DSI_INT_ALL_BITS= (0x7f)
>  };

I think you should use '#define' instead of 'enum'. The code would be
like below, and these definition should be moved to after DSI_INTEN or
DSI_INTSTA.

#define DSI_INT_LPRX_RD_RDY_FLAGBIT(0)
#define DSI_INT_CMD_DONE_FLAG   BIT(1)
#define DSI_INT_TE_RDY_FLAG BIT(2)
#define DSI_INT_VM_DONE_FLAGBIT(3)
#define DSI_INT_EXT_TE_RDY_FLAG BIT(4)
#define DSI_INT_VM_CMD_DONE_FLAGBIT(5)
#define DSI_INT_SLEEPOUT_DONE_FLAG  BIT(6)
#define DSI_INT_ALL_BITS(DSI_INT_LPRX_RD_RDY_FLAG | \
DSI_INT_CMD_DONE_FLAG | \
DSI_INT_TE_RDY_FLAG | \
DSI_INT_VM_DONE_FLAG | \
DSI_INT_EXT_TE_RDY_FLAG | \
DSI_INT_VM_CMD_DONE_FLAG | \
DSI_INT_SLEEPOUT_DONE_FLAG)

>  
>  static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> @@ -440,6 +463,94 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
>   writel(1, dsi->regs + DSI_START);
>  }
>  
> +static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> +{
> + u32 inten = DSI_INT_ALL_BITS;
> +
> + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO)
> + inten &= ~(DSI_INT_TE_RDY_FLAG | DSI_INT_EXT_TE_RDY_FLAG);
> +
> + writel(inten, dsi->regs + DSI_INTEN);
> +}
> +
> +static void mtk_dsi_irq_wakeup(struct mtk_dsi *dsi, u32 irq_bit)
> +{
> + dsi->irq_data |= irq_bit;
> +}
> +
> +static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> +{
> + struct mtk_dsi *dsi = dev_id;
> +
> + u32 status, tmp;
> +
> + status = readl(dsi->regs + DSI_INTSTA);
> +
> + if (status & DSI_INT_LPRX_RD_RDY_FLAG) {
> + /* write clear RD_RDY interrupt */
> + /* write clear RD_RDY interrupt must be before DSI_RACK */
> + /* because CMD_DONE will raise after DSI_RACK, */
> + /* so write clear RD_RDY after that will clear CMD_DONE too */
> + do {
> + /* send read ACK */
> + mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
> + tmp = readl(dsi->regs + DSI_INTSTA);
> + } while (tmp & DSI_BUSY);
> +
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_LPRX_RD_RDY_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_LPRX_RD_RDY_FLAG);
> + }
> +
> + if (status & DSI_INT_CMD_DONE_FLAG) {
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_CMD_DONE_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_CMD_DONE_FLAG);
> + }
> +
> + if (status & DSI_INT_TE_RDY_FLAG) {
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_TE_RDY_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_TE_RDY_FLAG);
> + }
> +
> + if (status & DSI_INT_VM_DONE_FLAG) {
> + mtk_dsi_mask(dsi, 

Re: [PATCH v4 5/8] drm/mediatek: add dsi interrupt control

[CK Hu]

Hi, YT:

Some comments inline.

On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote:
> From: shaoming chen 
> 
> add dsi interrupt control
> 
> Signed-off-by: shaoming chen 
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c |  130 
> 
>  1 file changed, 130 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 2d808e5..de5ad7f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -32,6 +33,13 @@
>  
>  #define DSI_START0x00
>  
> +#define DSI_INTEN0x08
> +
> +#define DSI_INTSTA   0x0c
> +#define LPRX_RD_RDY_INT_FLAG BIT(0)
> +#define CMD_DONE_INT_FLAGBIT(1)
> +#define DSI_BUSY BIT(31)

Why need LPRX_RD_RDY_INT_FLAG, CMD_DONE_INT_FLAG, and DSI_BUSY? Maybe
these three should be moved to other patch.

> +
>  #define DSI_CON_CTRL 0x10
>  #define DSI_RESETBIT(0)
>  #define DSI_EN   BIT(1)
> @@ -74,6 +82,9 @@
>  
>  #define DSI_HSTX_CKL_WC  0x64
>  
> +#define DSI_RACK 0x84
> +#define RACK BIT(0)
> +
>  #define DSI_PHY_LCCON0x104
>  #define LC_HS_TX_EN  BIT(0)
>  #define LC_ULPM_EN   BIT(1)
> @@ -134,6 +145,18 @@ struct mtk_dsi {
>   struct videomode vm;
>   int refcount;
>   bool enabled;
> + int irq_num, irq_data;
> +};
> +
> +enum {
> + DSI_INT_SLEEPOUT_DONE_FLAG  = BIT(6),
> + DSI_INT_VM_CMD_DONE_FLAG= BIT(5),
> + DSI_INT_EXT_TE_RDY_FLAG = BIT(4),
> + DSI_INT_VM_DONE_FLAG= BIT(3),
> + DSI_INT_TE_RDY_FLAG = BIT(2),
> + DSI_INT_CMD_DONE_FLAG   = BIT(1),
> + DSI_INT_LPRX_RD_RDY_FLAG= BIT(0),
> + DSI_INT_ALL_BITS= (0x7f)
>  };

I think you should use '#define' instead of 'enum'. The code would be
like below, and these definition should be moved to after DSI_INTEN or
DSI_INTSTA.

#define DSI_INT_LPRX_RD_RDY_FLAGBIT(0)
#define DSI_INT_CMD_DONE_FLAG   BIT(1)
#define DSI_INT_TE_RDY_FLAG BIT(2)
#define DSI_INT_VM_DONE_FLAGBIT(3)
#define DSI_INT_EXT_TE_RDY_FLAG BIT(4)
#define DSI_INT_VM_CMD_DONE_FLAGBIT(5)
#define DSI_INT_SLEEPOUT_DONE_FLAG  BIT(6)
#define DSI_INT_ALL_BITS(DSI_INT_LPRX_RD_RDY_FLAG | \
DSI_INT_CMD_DONE_FLAG | \
DSI_INT_TE_RDY_FLAG | \
DSI_INT_VM_DONE_FLAG | \
DSI_INT_EXT_TE_RDY_FLAG | \
DSI_INT_VM_CMD_DONE_FLAG | \
DSI_INT_SLEEPOUT_DONE_FLAG)

>  
>  static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> @@ -440,6 +463,94 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
>   writel(1, dsi->regs + DSI_START);
>  }
>  
> +static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> +{
> + u32 inten = DSI_INT_ALL_BITS;
> +
> + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO)
> + inten &= ~(DSI_INT_TE_RDY_FLAG | DSI_INT_EXT_TE_RDY_FLAG);
> +
> + writel(inten, dsi->regs + DSI_INTEN);
> +}
> +
> +static void mtk_dsi_irq_wakeup(struct mtk_dsi *dsi, u32 irq_bit)
> +{
> + dsi->irq_data |= irq_bit;
> +}
> +
> +static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> +{
> + struct mtk_dsi *dsi = dev_id;
> +
> + u32 status, tmp;
> +
> + status = readl(dsi->regs + DSI_INTSTA);
> +
> + if (status & DSI_INT_LPRX_RD_RDY_FLAG) {
> + /* write clear RD_RDY interrupt */
> + /* write clear RD_RDY interrupt must be before DSI_RACK */
> + /* because CMD_DONE will raise after DSI_RACK, */
> + /* so write clear RD_RDY after that will clear CMD_DONE too */
> + do {
> + /* send read ACK */
> + mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
> + tmp = readl(dsi->regs + DSI_INTSTA);
> + } while (tmp & DSI_BUSY);
> +
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_LPRX_RD_RDY_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_LPRX_RD_RDY_FLAG);
> + }
> +
> + if (status & DSI_INT_CMD_DONE_FLAG) {
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_CMD_DONE_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_CMD_DONE_FLAG);
> + }
> +
> + if (status & DSI_INT_TE_RDY_FLAG) {
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_TE_RDY_FLAG, 0);
> + mtk_dsi_irq_wakeup(dsi, DSI_INT_TE_RDY_FLAG);
> + }
> +
> + if (status & DSI_INT_VM_DONE_FLAG) {
> + mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_VM_DONE_FLAG, 0);
> + 

[PATCH v4 5/8] drm/mediatek: add dsi interrupt control

[YT Shen]

From: shaoming chen 

add dsi interrupt control

Signed-off-by: shaoming chen 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c |  130 
 1 file changed, 130 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 2d808e5..de5ad7f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -32,6 +33,13 @@
 
 #define DSI_START  0x00
 
+#define DSI_INTEN  0x08
+
+#define DSI_INTSTA 0x0c
+#define LPRX_RD_RDY_INT_FLAG   BIT(0)
+#define CMD_DONE_INT_FLAG  BIT(1)
+#define DSI_BUSY   BIT(31)
+
 #define DSI_CON_CTRL   0x10
 #define DSI_RESET  BIT(0)
 #define DSI_EN BIT(1)
@@ -74,6 +82,9 @@
 
 #define DSI_HSTX_CKL_WC0x64
 
+#define DSI_RACK   0x84
+#define RACK   BIT(0)
+
 #define DSI_PHY_LCCON  0x104
 #define LC_HS_TX_ENBIT(0)
 #define LC_ULPM_EN BIT(1)
@@ -134,6 +145,18 @@ struct mtk_dsi {
struct videomode vm;
int refcount;
bool enabled;
+   int irq_num, irq_data;
+};
+
+enum {
+   DSI_INT_SLEEPOUT_DONE_FLAG  = BIT(6),
+   DSI_INT_VM_CMD_DONE_FLAG= BIT(5),
+   DSI_INT_EXT_TE_RDY_FLAG = BIT(4),
+   DSI_INT_VM_DONE_FLAG= BIT(3),
+   DSI_INT_TE_RDY_FLAG = BIT(2),
+   DSI_INT_CMD_DONE_FLAG   = BIT(1),
+   DSI_INT_LPRX_RD_RDY_FLAG= BIT(0),
+   DSI_INT_ALL_BITS= (0x7f)
 };
 
 static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
@@ -440,6 +463,94 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
writel(1, dsi->regs + DSI_START);
 }
 
+static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
+{
+   u32 inten = DSI_INT_ALL_BITS;
+
+   if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO)
+   inten &= ~(DSI_INT_TE_RDY_FLAG | DSI_INT_EXT_TE_RDY_FLAG);
+
+   writel(inten, dsi->regs + DSI_INTEN);
+}
+
+static void mtk_dsi_irq_wakeup(struct mtk_dsi *dsi, u32 irq_bit)
+{
+   dsi->irq_data |= irq_bit;
+}
+
+static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
+{
+   struct mtk_dsi *dsi = dev_id;
+
+   u32 status, tmp;
+
+   status = readl(dsi->regs + DSI_INTSTA);
+
+   if (status & DSI_INT_LPRX_RD_RDY_FLAG) {
+   /* write clear RD_RDY interrupt */
+   /* write clear RD_RDY interrupt must be before DSI_RACK */
+   /* because CMD_DONE will raise after DSI_RACK, */
+   /* so write clear RD_RDY after that will clear CMD_DONE too */
+   do {
+   /* send read ACK */
+   mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
+   tmp = readl(dsi->regs + DSI_INTSTA);
+   } while (tmp & DSI_BUSY);
+
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_LPRX_RD_RDY_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_LPRX_RD_RDY_FLAG);
+   }
+
+   if (status & DSI_INT_CMD_DONE_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_CMD_DONE_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_CMD_DONE_FLAG);
+   }
+
+   if (status & DSI_INT_TE_RDY_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_TE_RDY_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_TE_RDY_FLAG);
+   }
+
+   if (status & DSI_INT_VM_DONE_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_VM_DONE_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_VM_DONE_FLAG);
+   }
+
+   if (status & DSI_INT_EXT_TE_RDY_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_EXT_TE_RDY_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_EXT_TE_RDY_FLAG);
+   }
+
+   if (status & DSI_INT_VM_CMD_DONE_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_VM_CMD_DONE_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_VM_CMD_DONE_FLAG);
+   }
+
+   if (status & DSI_INT_SLEEPOUT_DONE_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_SLEEPOUT_DONE_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_SLEEPOUT_DONE_FLAG);
+   }
+
+   return IRQ_HANDLED;
+}
+
+static s32 mtk_dsi_wait_for_irq_timeout(struct mtk_dsi *dsi, u32 irq_bit,
+   u32 timeout_ms)
+{
+   while (timeout_ms--) {
+   if (dsi->irq_data & irq_bit) {
+   dsi->irq_data &= ~irq_bit;
+   return 0;
+   }
+
+   usleep_range(1000, 1100);
+   }
+
+   dsi->irq_data = 0;
+
+   return -1;
+}
+
 static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
 {
if (WARN_ON(dsi->refcount == 0))
@@ -488,6 +599,7 @@ 

[PATCH v4 5/8] drm/mediatek: add dsi interrupt control

[YT Shen]

From: shaoming chen 

add dsi interrupt control

Signed-off-by: shaoming chen 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c |  130 
 1 file changed, 130 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 2d808e5..de5ad7f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -32,6 +33,13 @@
 
 #define DSI_START  0x00
 
+#define DSI_INTEN  0x08
+
+#define DSI_INTSTA 0x0c
+#define LPRX_RD_RDY_INT_FLAG   BIT(0)
+#define CMD_DONE_INT_FLAG  BIT(1)
+#define DSI_BUSY   BIT(31)
+
 #define DSI_CON_CTRL   0x10
 #define DSI_RESET  BIT(0)
 #define DSI_EN BIT(1)
@@ -74,6 +82,9 @@
 
 #define DSI_HSTX_CKL_WC0x64
 
+#define DSI_RACK   0x84
+#define RACK   BIT(0)
+
 #define DSI_PHY_LCCON  0x104
 #define LC_HS_TX_ENBIT(0)
 #define LC_ULPM_EN BIT(1)
@@ -134,6 +145,18 @@ struct mtk_dsi {
struct videomode vm;
int refcount;
bool enabled;
+   int irq_num, irq_data;
+};
+
+enum {
+   DSI_INT_SLEEPOUT_DONE_FLAG  = BIT(6),
+   DSI_INT_VM_CMD_DONE_FLAG= BIT(5),
+   DSI_INT_EXT_TE_RDY_FLAG = BIT(4),
+   DSI_INT_VM_DONE_FLAG= BIT(3),
+   DSI_INT_TE_RDY_FLAG = BIT(2),
+   DSI_INT_CMD_DONE_FLAG   = BIT(1),
+   DSI_INT_LPRX_RD_RDY_FLAG= BIT(0),
+   DSI_INT_ALL_BITS= (0x7f)
 };
 
 static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
@@ -440,6 +463,94 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
writel(1, dsi->regs + DSI_START);
 }
 
+static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
+{
+   u32 inten = DSI_INT_ALL_BITS;
+
+   if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO)
+   inten &= ~(DSI_INT_TE_RDY_FLAG | DSI_INT_EXT_TE_RDY_FLAG);
+
+   writel(inten, dsi->regs + DSI_INTEN);
+}
+
+static void mtk_dsi_irq_wakeup(struct mtk_dsi *dsi, u32 irq_bit)
+{
+   dsi->irq_data |= irq_bit;
+}
+
+static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
+{
+   struct mtk_dsi *dsi = dev_id;
+
+   u32 status, tmp;
+
+   status = readl(dsi->regs + DSI_INTSTA);
+
+   if (status & DSI_INT_LPRX_RD_RDY_FLAG) {
+   /* write clear RD_RDY interrupt */
+   /* write clear RD_RDY interrupt must be before DSI_RACK */
+   /* because CMD_DONE will raise after DSI_RACK, */
+   /* so write clear RD_RDY after that will clear CMD_DONE too */
+   do {
+   /* send read ACK */
+   mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
+   tmp = readl(dsi->regs + DSI_INTSTA);
+   } while (tmp & DSI_BUSY);
+
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_LPRX_RD_RDY_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_LPRX_RD_RDY_FLAG);
+   }
+
+   if (status & DSI_INT_CMD_DONE_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_CMD_DONE_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_CMD_DONE_FLAG);
+   }
+
+   if (status & DSI_INT_TE_RDY_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_TE_RDY_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_TE_RDY_FLAG);
+   }
+
+   if (status & DSI_INT_VM_DONE_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_VM_DONE_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_VM_DONE_FLAG);
+   }
+
+   if (status & DSI_INT_EXT_TE_RDY_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_EXT_TE_RDY_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_EXT_TE_RDY_FLAG);
+   }
+
+   if (status & DSI_INT_VM_CMD_DONE_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_VM_CMD_DONE_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_VM_CMD_DONE_FLAG);
+   }
+
+   if (status & DSI_INT_SLEEPOUT_DONE_FLAG) {
+   mtk_dsi_mask(dsi, DSI_INTSTA, DSI_INT_SLEEPOUT_DONE_FLAG, 0);
+   mtk_dsi_irq_wakeup(dsi, DSI_INT_SLEEPOUT_DONE_FLAG);
+   }
+
+   return IRQ_HANDLED;
+}
+
+static s32 mtk_dsi_wait_for_irq_timeout(struct mtk_dsi *dsi, u32 irq_bit,
+   u32 timeout_ms)
+{
+   while (timeout_ms--) {
+   if (dsi->irq_data & irq_bit) {
+   dsi->irq_data &= ~irq_bit;
+   return 0;
+   }
+
+   usleep_range(1000, 1100);
+   }
+
+   dsi->irq_data = 0;
+
+   return -1;
+}
+
 static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
 {
if (WARN_ON(dsi->refcount == 0))
@@ -488,6 +599,7 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)