On 2014ë
10ì 01ì¼ 15:19, YoungJun Cho wrote:
> For the I80 interface, the video interrupt pending register(VIDINTCON1)
> should be handled in fimd_irq_handler() and the video interrupt control
> register(VIDINTCON0) should be handled in fimd_enable_vblank() and
> fimd_disable_vblank() like RGB interface.
> So this patch moves each set / unset routines into proper positions.
> And adds triggering unset routine in fimd_trigger() to exit from it
> because there is a case like set config which requires triggering
> but vblank is not enabled.
Reasonable but how about separating this patch into two patches. One is
set/unset properly the registers relevant to interrupt, and other?
It seems that your patch includes some codes not relevant to above
description. And below is my comment.
Thanks,
Inki Dae
>
> Signed-off-by: YoungJun Cho <yj44.cho at samsung.com>
> Acked-by: Inki Dae <inki.dae at samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park at samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 60
> ++++++++++++++++++--------------
> 1 file changed, 34 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index f062335..c949099 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -463,12 +463,19 @@ static int fimd_enable_vblank(struct exynos_drm_manager
> *mgr)
> val = readl(ctx->regs + VIDINTCON0);
>
> val |= VIDINTCON0_INT_ENABLE;
> - val |= VIDINTCON0_INT_FRAME;
>
> - val &= ~VIDINTCON0_FRAMESEL0_MASK;
> - val |= VIDINTCON0_FRAMESEL0_VSYNC;
> - val &= ~VIDINTCON0_FRAMESEL1_MASK;
> - val |= VIDINTCON0_FRAMESEL1_NONE;
> + if (ctx->i80_if) {
> + val |= VIDINTCON0_INT_I80IFDONE;
> + val |= VIDINTCON0_INT_SYSMAINCON;
> + val &= ~VIDINTCON0_INT_SYSSUBCON;
> + } else {
> + val |= VIDINTCON0_INT_FRAME;
> +
> + val &= ~VIDINTCON0_FRAMESEL0_MASK;
> + val |= VIDINTCON0_FRAMESEL0_VSYNC;
> + val &= ~VIDINTCON0_FRAMESEL1_MASK;
> + val |= VIDINTCON0_FRAMESEL1_NONE;
> + }
>
> writel(val, ctx->regs + VIDINTCON0);
> }
> @@ -487,9 +494,15 @@ static void fimd_disable_vblank(struct
> exynos_drm_manager *mgr)
> if (test_and_clear_bit(0, &ctx->irq_flags)) {
> val = readl(ctx->regs + VIDINTCON0);
>
> - val &= ~VIDINTCON0_INT_FRAME;
> val &= ~VIDINTCON0_INT_ENABLE;
>
> + if (ctx->i80_if) {
> + val &= ~VIDINTCON0_INT_I80IFDONE;
> + val &= ~VIDINTCON0_INT_SYSMAINCON;
> + val &= ~VIDINTCON0_INT_SYSSUBCON;
> + } else
> + val &= ~VIDINTCON0_INT_FRAME;
> +
> writel(val, ctx->regs + VIDINTCON0);
> }
> }
> @@ -945,16 +958,19 @@ static void fimd_trigger(struct device *dev)
> void *timing_base = ctx->regs + driver_data->timing_base;
> u32 reg;
>
> + /* Enters triggering mode */
> atomic_set(&ctx->triggering, 1);
>
> - reg = readl(ctx->regs + VIDINTCON0);
> - reg |= (VIDINTCON0_INT_ENABLE | VIDINTCON0_INT_I80IFDONE |
> - VIDINTCON0_INT_SYSMAINCON);
> - writel(reg, ctx->regs + VIDINTCON0);
> -
> reg = readl(timing_base + TRIGCON);
> reg |= (TRGMODE_I80_RGB_ENABLE_I80 | SWTRGCMD_I80_RGB_ENABLE);
> writel(reg, timing_base + TRIGCON);
> +
> + /*
> + * Exits triggering mode if vblank is not enabled yet, because when the
> + * VIDINTCON0 register is not set, it can not exit from triggering mode.
> + */
> + if (!test_bit(0, &ctx->irq_flags))
> + atomic_set(&ctx->triggering, 0);
I think this code would make for other trigger requested while triggering.
> }
>
> static void fimd_te_handler(struct exynos_drm_manager *mgr)
> @@ -966,9 +982,9 @@ static void fimd_te_handler(struct exynos_drm_manager
> *mgr)
> return;
>
> /*
> - * Skips to trigger if in triggering state, because multiple triggering
> - * requests can cause panel reset.
> - */
> + * Skips triggering if in triggering mode, because multiple triggering
> + * requests can cause panel reset.
> + */
> if (atomic_read(&ctx->triggering))
> return;
>
> @@ -1023,21 +1039,13 @@ static irqreturn_t fimd_irq_handler(int irq, void
> *dev_id)
> if (ctx->pipe < 0 || !ctx->drm_dev)
> goto out;
>
> - if (ctx->i80_if) {
> - /* unset I80 frame done interrupt */
> - val = readl(ctx->regs + VIDINTCON0);
> - val &= ~(VIDINTCON0_INT_I80IFDONE | VIDINTCON0_INT_SYSMAINCON);
> - writel(val, ctx->regs + VIDINTCON0);
> + drm_handle_vblank(ctx->drm_dev, ctx->pipe);
> + exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>
> - /* exit triggering mode */
> + if (ctx->i80_if) {
> + /* Exits triggering mode */
> atomic_set(&ctx->triggering, 0);
> -
> - drm_handle_vblank(ctx->drm_dev, ctx->pipe);
> - exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
> } else {
> - drm_handle_vblank(ctx->drm_dev, ctx->pipe);
> - exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
> -
> /* set wait vsync event to zero and wake up queue. */
> if (atomic_read(&ctx->wait_vsync_event)) {
> atomic_set(&ctx->wait_vsync_event, 0);
>
