[PATCH v2 0/3] Exynos SYSMMU (IOMMU) updates for Exynos DRM

[PATCH v2 0/3] Exynos SYSMMU (IOMMU) updates for Exynos DRM

[Marek Szyprowski]

Hello,

Main changes for Exynos SYSMMU (IOMMU) driver has been finally scheduled
for merging - see
https://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/commit/?h=next

To let Exynos IOMMU driver work correctly, some fixes are also needed in
Exynos DRM driver.

v2 of this patchset has been rebased onto latest exynos-drm-next branch
with atomic mode setting patches applied.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Marek Szyprowski (3):
  drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()
  drm/exynos: iommu: detach from default dma-mapping domain on init
  drm/exynos: iommu: improve a check for non-iommu dma_ops

 drivers/gpu/drm/exynos/exynos_drm_fimd.c  | 22 ++++++++++++++++++----
 drivers/gpu/drm/exynos/exynos_drm_iommu.c |  7 +++++--
 2 files changed, 23 insertions(+), 6 deletions(-)

-- 
1.9.2

[PATCH v2 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Marek Szyprowski]

One should not do any assumptions on the stare of the fimd hardware
during driver initialization, so to properly reset fimd before enabling
IOMMU, one should ensure that all power domains and clocks are really
enabled. This patch adds pm_runtime and clocks management in the
fimd_clear_channel() function to ensure that any access to fimd
registers will be performed with clocks and power domains enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Tested-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 96618534358e..3ec9d4299a86 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -242,12 +242,21 @@ static void fimd_enable_shadow_channel_path(struct fimd_context *ctx,
 	writel(val, ctx->regs + SHADOWCON);
 }
 
+static int fimd_enable_vblank(struct exynos_drm_crtc *crtc);
+static void fimd_disable_vblank(struct exynos_drm_crtc *crtc);
+
 static void fimd_clear_channel(struct fimd_context *ctx)
 {
 	unsigned int win, ch_enabled = 0;
 
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
+	/* Hardware is in unknown state, so ensure it gets enabled properly */
+	pm_runtime_get_sync(ctx->dev);
+
+	clk_prepare_enable(ctx->bus_clk);
+	clk_prepare_enable(ctx->lcd_clk);
+
 	/* Check if any channel is enabled. */
 	for (win = 0; win < WINDOWS_NR; win++) {
 		u32 val = readl(ctx->regs + WINCON(win));
@@ -265,12 +274,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
 
 	/* Wait for vsync, as disable channel takes effect at next vsync */
 	if (ch_enabled) {
-		unsigned int state = ctx->suspended;
-
-		ctx->suspended = 0;
+		ctx->suspended = false;
+		fimd_enable_vblank(ctx->crtc);
 		fimd_wait_for_vblank(ctx->crtc);
-		ctx->suspended = state;
+		fimd_disable_vblank(ctx->crtc);
+		ctx->suspended = true;
 	}
+
+	clk_disable_unprepare(ctx->lcd_clk);
+	clk_disable_unprepare(ctx->bus_clk);
+
+	pm_runtime_put(ctx->dev);
 }
 
 static int fimd_iommu_attach_devices(struct fimd_context *ctx,
-- 
1.9.2

Re: [PATCH v2 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Inki Dae]

Hi Marek,

On 2015년 06월 03일 17:26, Marek Szyprowski wrote:
> One should not do any assumptions on the stare of the fimd hardware
> during driver initialization, so to properly reset fimd before enabling
> IOMMU, one should ensure that all power domains and clocks are really
> enabled. This patch adds pm_runtime and clocks management in the
> fimd_clear_channel() function to ensure that any access to fimd
> registers will be performed with clocks and power domains enabled.

I have tested this patch series on trats2 board which uses Exynos4412
SoC. However, the booting is halted out. Without iommu, the booting and
display works well.

For this test, I also merged another your patch series in iommu exynos
tree and added device node relevant codes like below,

in exynos4.dtsi file:
fimd: fimd@11c00000 {
                 ...
               iommus = <&sysmmu_fimd0>;
                 ...

sysmmu_fimd0: sysmmu@11E20000 {
        compatible = "samsung,exynos-sysmmu";
        reg = <0x11E20000 0x1000>;
        interrupt-parent = <&combiner>;
        interrupts = <5 2>;
        clock-names = "sysmmu", "master";
        clocks = <&clock CLK_SMMU_FIMD0>, <&clock CLK_FIMD0>;
         power-domains = <&pd_lcd0>;
        #iommu-cells = <0>;
};

in exynos4412-trats2.dts file:
fimd@11c00000 {
                status = "okay";
                iommu-reserved-mapping = <0x40000000 0x40000000 0x40000000>;
};

Can you check it out?

Thanks,
Inki Dae

> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 96618534358e..3ec9d4299a86 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -242,12 +242,21 @@ static void fimd_enable_shadow_channel_path(struct fimd_context *ctx,
>  	writel(val, ctx->regs + SHADOWCON);
>  }
>  
> +static int fimd_enable_vblank(struct exynos_drm_crtc *crtc);
> +static void fimd_disable_vblank(struct exynos_drm_crtc *crtc);
> +
>  static void fimd_clear_channel(struct fimd_context *ctx)
>  {
>  	unsigned int win, ch_enabled = 0;
>  
>  	DRM_DEBUG_KMS("%s\n", __FILE__);
>  
> +	/* Hardware is in unknown state, so ensure it gets enabled properly */
> +	pm_runtime_get_sync(ctx->dev);
> +
> +	clk_prepare_enable(ctx->bus_clk);
> +	clk_prepare_enable(ctx->lcd_clk);
> +
>  	/* Check if any channel is enabled. */
>  	for (win = 0; win < WINDOWS_NR; win++) {
>  		u32 val = readl(ctx->regs + WINCON(win));
> @@ -265,12 +274,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
>  
>  	/* Wait for vsync, as disable channel takes effect at next vsync */
>  	if (ch_enabled) {
> -		unsigned int state = ctx->suspended;
> -
> -		ctx->suspended = 0;
> +		ctx->suspended = false;
> +		fimd_enable_vblank(ctx->crtc);
>  		fimd_wait_for_vblank(ctx->crtc);
> -		ctx->suspended = state;
> +		fimd_disable_vblank(ctx->crtc);
> +		ctx->suspended = true;
>  	}
> +
> +	clk_disable_unprepare(ctx->lcd_clk);
> +	clk_disable_unprepare(ctx->bus_clk);
> +
> +	pm_runtime_put(ctx->dev);
>  }
>  
>  static int fimd_iommu_attach_devices(struct fimd_context *ctx,
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Inki Dae]

On 2015년 06월 04일 22:08, Inki Dae wrote:
> Hi Marek,
> 
> On 2015년 06월 03일 17:26, Marek Szyprowski wrote:
>> One should not do any assumptions on the stare of the fimd hardware
>> during driver initialization, so to properly reset fimd before enabling
>> IOMMU, one should ensure that all power domains and clocks are really
>> enabled. This patch adds pm_runtime and clocks management in the
>> fimd_clear_channel() function to ensure that any access to fimd
>> registers will be performed with clocks and power domains enabled.
> 
> I have tested this patch series on trats2 board which uses Exynos4412
> SoC. However, the booting is halted out. Without iommu, the booting and
> display works well.
> 
> For this test, I also merged another your patch series in iommu exynos
> tree and added device node relevant codes like below,
> 
> in exynos4.dtsi file:
> fimd: fimd@11c00000 {
>                  ...
>                iommus = <&sysmmu_fimd0>;
>                  ...
> 
> sysmmu_fimd0: sysmmu@11E20000 {
>         compatible = "samsung,exynos-sysmmu";
>         reg = <0x11E20000 0x1000>;
>         interrupt-parent = <&combiner>;
>         interrupts = <5 2>;
>         clock-names = "sysmmu", "master";
>         clocks = <&clock CLK_SMMU_FIMD0>, <&clock CLK_FIMD0>;
>          power-domains = <&pd_lcd0>;
>         #iommu-cells = <0>;
> };
> 
> in exynos4412-trats2.dts file:
> fimd@11c00000 {
>                 status = "okay";
>                 iommu-reserved-mapping = <0x40000000 0x40000000 0x40000000>;
> };
> 
> Can you check it out?

In addition, sometimes I see below kernel panic logs which means page
fault to fimd occurs while booting:

[    0.394228] 13800000.serial: ttySAC0 at MMIO 0x13800000 (irq = 56,
base_baud = 0) is a S3C6400/10
[    0.394788] 13810000.serial: ttySAC1 at MMIO 0x13810000 (irq = 57,
base_baud = 0) is a S3C6400/10
[    0.395281] 13820000.serial: ttySAC2 at MMIO 0x13820000 (irq = 58,
base_baud = 0) is a S3C6400/10
[    1.122219] console [ttySAC2] enabled
[    1.126419] 13830000.serial: ttySAC3 at MMIO 0x13830000 (irq = 59,
base_baud = 0) is a S3C6400/10
[    1.136250] [drm] Initialized drm 1.1.0 20060810
[    1.142710] PAGE FAULT occurred at 0x52188000 by 11e20000.sysmmu(Page
table base: 0x6ea80000)
[    1.149754]  Lv1 entry: 0x6e92dc01
[    1.153172] ------------[ cut here ]------------
[    1.157740] kernel BUG at drivers/iommu/exynos-iommu.c:364!
[    1.163296] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
[    1.169110] Modules linked in:
[    1.172154] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.1.0-rc4-00563-gee14f4e-dirty #1384
[    1.180394] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    1.186472] task: c06d2df0 ti: c06ce000 task.ti: c06ce000
[    1.191861] PC is at exynos_sysmmu_irq+0x184/0x208
[    1.196628] LR is at exynos_sysmmu_irq+0xd4/0x208
[    1.201316] pc : [<c02677cc>]    lr : [<c026771c>]    psr: 60000193
[    1.201316] sp : c06cfe90  ip : 00000000  fp : 00000000
[    1.212772] r10: c06ff6a3  r9 : 00000521  r8 : 52188000
[    1.217980] r7 : eea80000  r6 : ee9b3428  r5 : ee9b3410  r4 : 00000000
[    1.224489] r3 : 6e92dc01  r2 : 6e92dc01  r1 : eea55810  r0 : ee9c4e00
[    1.231002] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
Segment kernel
[    1.238378] Control: 10c5387d  Table: 4000404a  DAC: 00000015
[    1.244107] Process swapper/0 (pid: 0, stack limit = 0xc06ce210)
[    1.250096] Stack: (0xc06cfe90 to 0xc06d0000)
[    1.254438] fe80:                                     c06cfe9c
c039caac eef82b80 6ea80000
[    1.262599] fea0: 16b1580f ee9b4240 ee84ad20 00000000 00000000
00000026 ee84acc0 c0060308
[    1.270757] fec0: 00000000 eef81380 ee84acc0 ee84ad20 ee9b4240
00000015 ee804450 c06cff68
[    1.278916] fee0: ee808000 c0060400 ee84acc0 ee84ad20 ee807000
c00630a8 00000026 c06dddb8
[    1.287075] ff00: ee807000 c005f98c 0000000a c0200ccc 00000015
00000000 00000015 00000000
[    1.295235] ff20: 00000001 c005f98c c06caaac c005fc58 f002000c
00000015 c06d07a0 c06cff68
[    1.303394] ff40: f0020000 c06ff6a1 00000001 c0009434 c0010068
60000113 ffffffff c06cff9c
[    1.311553] ff60: c06cffb8 c0012f40 00000000 00000000 00001288
c001c880 c06ce000 c06d04f8
[    1.319712] ff80: c04a1f50 c06c92c4 c06cffb8 c06ff6a1 00000001
00000000 01000000 c06cffb0
[    1.327871] ffa0: c0010064 c0010068 60000113 ffffffff c06ce000
c0053f58 ffffffff c067bc54
[    1.336031] ffc0: ffffffff ffffffff c067b678 00000000 00000000
c06a70b8 c07023d4 c06d0480
[    1.344190] ffe0: c06a70b4 c06d3f04 4000406a 413fc090 00000000
4000807c 00000000 00000000
[    1.352366] [<c02677cc>] (exynos_sysmmu_irq) from [<c0060308>]
(handle_irq_event_percpu+0x78/0x134)
[    1.361382] [<c0060308>] (handle_irq_event_percpu) from [<c0060400>]
(handle_irq_event+0x3c/0x5c)
[    1.370235] [<c0060400>] (handle_irq_event) from [<c00630a8>]
(handle_level_irq+0xc4/0x13c)
[    1.378567] [<c00630a8>] (handle_level_irq) from [<c005f98c>]
(generic_handle_irq+0x2c/0x3c)
[    1.386990] [<c005f98c>] (generic_handle_irq) from [<c0200ccc>]
(combiner_handle_cascade_irq+0x94/0x100)
[    1.396448] [<c0200ccc>] (combiner_handle_cascade_irq) from
[<c005f98c>] (generic_handle_irq+0x2c/0x3c)
[    1.405820] [<c005f98c>] (generic_handle_irq) from [<c005fc58>]
(__handle_domain_irq+0x7c/0xec)
[    1.414502] [<c005fc58>] (__handle_domain_irq) from [<c0009434>]
(gic_handle_irq+0x30/0x68)
[    1.422833] [<c0009434>] (gic_handle_irq) from [<c0012f40>]
(__irq_svc+0x40/0x74)
[    1.430292] Exception stack(0xc06cff68 to 0xc06cffb0)
[    1.435330] ff60:                   00000000 00000000 00001288
c001c880 c06ce000 c06d04f8
[    1.443489] ff80: c04a1f50 c06c92c4 c06cffb8 c06ff6a1 00000001
00000000 01000000 c06cffb0
[    1.451646] ffa0: c0010064 c0010068 60000113 ffffffff
[    1.456689] [<c0012f40>] (__irq_svc) from [<c0010068>]
(arch_cpu_idle+0x38/0x3c)
[    1.464069] [<c0010068>] (arch_cpu_idle) from [<c0053f58>]
(cpu_startup_entry+0x12c/0x1c4)
[    1.472314] [<c0053f58>] (cpu_startup_entry) from [<c067bc54>]
(start_kernel+0x398/0x3a4)
[    1.480468] [<c067bc54>] (start_kernel) from [<4000807c>] (0x4000807c)
[    1.486977] Code: e28dd014 e8bd83f0 e5913008 eaffffc0 (e7f001f2)
[    1.493062] ---[ end trace e8c5db152e219756 ]---
[    1.497650] Kernel panic - not syncing: Fatal exception in interrupt
[    1.503990] CPU2: stopping
[    1.506681] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G      D
4.1.0-rc4-00563-gee14f4e-dirty #1384
[    1.516137] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    1.522229] [<c0015388>] (unwind_backtrace) from [<c0012440>]
(show_stack+0x10/0x14)
[    1.529951] [<c0012440>] (show_stack) from [<c0499840>]
(dump_stack+0x84/0xc4)
[    1.537151] [<c0499840>] (dump_stack) from [<c0014634>]
(handle_IPI+0x17c/0x18c)
[    1.544526] [<c0014634>] (handle_IPI) from [<c0009468>]
(gic_handle_irq+0x64/0x68)
[    1.552078] [<c0009468>] (gic_handle_irq) from [<c0012f40>]
(__irq_svc+0x40/0x74)
[    1.559537] Exception stack(0xee8b1fa0 to 0xee8b1fe8)
[    1.564576] 1fa0: 00000002 00000000 000005de c001c880 ee8b0000
c06d04f8 c04a1f50 c06c92c4
[    1.572735] 1fc0: ee8b1ff0 c06ff6a1 00000001 00000000 01000000
ee8b1fe8 c0010064 c0010068
[    1.580890] 1fe0: 60000113 ffffffff
[    1.584369] [<c0012f40>] (__irq_svc) from [<c0010068>]
(arch_cpu_idle+0x38/0x3c)
[    1.591749] [<c0010068>] (arch_cpu_idle) from [<c0053f58>]
(cpu_startup_entry+0x12c/0x1c4)
[    1.599992] [<c0053f58>] (cpu_startup_entry) from [<4000950c>]
(0x4000950c)
[    1.606932] CPU3: stopping
[    1.609626] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G      D
4.1.0-rc4-00563-gee14f4e-dirty #1384
[    1.619082] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    1.625171] [<c0015388>] (unwind_backtrace) from [<c0012440>]
(show_stack+0x10/0x14)
[    1.632892] [<c0012440>] (show_stack) from [<c0499840>]
(dump_stack+0x84/0xc4)
[    1.640094] [<c0499840>] (dump_stack) from [<c0014634>]
(handle_IPI+0x17c/0x18c)
[    1.647470] [<c0014634>] (handle_IPI) from [<c0009468>]
(gic_handle_irq+0x64/0x68)
[    1.655022] [<c0009468>] (gic_handle_irq) from [<c0012f40>]
(__irq_svc+0x40/0x74)
[    1.662482] Exception stack(0xee8b3fa0 to 0xee8b3fe8)
[    1.667521] 3fa0: 00000003 00000000 000009c4 c001c880 ee8b2000
c06d04f8 c04a1f50 c06c92c4
[    1.675680] 3fc0: ee8b3ff0 c06ff6a1 00000001 00000000 01000000
ee8b3fe8 c0010064 c0010068
[    1.683835] 3fe0: 60000113 ffffffff
[    1.687314] [<c0012f40>] (__irq_svc) from [<c0010068>]
(arch_cpu_idle+0x38/0x3c)
[    1.694694] [<c0010068>] (arch_cpu_idle) from [<c0053f58>]
(cpu_startup_entry+0x12c/0x1c4)
[    1.702936] [<c0053f58>] (cpu_startup_entry) from [<4000950c>]
(0x4000950c)
[    2.847590] SMP: failed to stop secondary CPUs
[    2.850551] ---[ end Kernel panic - not syncing: Fatal exception in
interrupt


> 
> Thanks,
> Inki Dae
> 
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 22 ++++++++++++++++++----
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 96618534358e..3ec9d4299a86 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -242,12 +242,21 @@ static void fimd_enable_shadow_channel_path(struct fimd_context *ctx,
>>  	writel(val, ctx->regs + SHADOWCON);
>>  }
>>  
>> +static int fimd_enable_vblank(struct exynos_drm_crtc *crtc);
>> +static void fimd_disable_vblank(struct exynos_drm_crtc *crtc);
>> +
>>  static void fimd_clear_channel(struct fimd_context *ctx)
>>  {
>>  	unsigned int win, ch_enabled = 0;
>>  
>>  	DRM_DEBUG_KMS("%s\n", __FILE__);
>>  
>> +	/* Hardware is in unknown state, so ensure it gets enabled properly */
>> +	pm_runtime_get_sync(ctx->dev);
>> +
>> +	clk_prepare_enable(ctx->bus_clk);
>> +	clk_prepare_enable(ctx->lcd_clk);
>> +
>>  	/* Check if any channel is enabled. */
>>  	for (win = 0; win < WINDOWS_NR; win++) {
>>  		u32 val = readl(ctx->regs + WINCON(win));
>> @@ -265,12 +274,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
>>  
>>  	/* Wait for vsync, as disable channel takes effect at next vsync */
>>  	if (ch_enabled) {
>> -		unsigned int state = ctx->suspended;
>> -
>> -		ctx->suspended = 0;
>> +		ctx->suspended = false;
>> +		fimd_enable_vblank(ctx->crtc);
>>  		fimd_wait_for_vblank(ctx->crtc);
>> -		ctx->suspended = state;
>> +		fimd_disable_vblank(ctx->crtc);
>> +		ctx->suspended = true;
>>  	}
>> +
>> +	clk_disable_unprepare(ctx->lcd_clk);
>> +	clk_disable_unprepare(ctx->bus_clk);
>> +
>> +	pm_runtime_put(ctx->dev);
>>  }
>>  
>>  static int fimd_iommu_attach_devices(struct fimd_context *ctx,
>>
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Marek Szyprowski]

Hello,

On 2015-06-04 15:13, Inki Dae wrote:
> On 2015년 06월 04일 22:08, Inki Dae wrote:
>> On 2015년 06월 03일 17:26, Marek Szyprowski wrote:
>>> One should not do any assumptions on the stare of the fimd hardware
>>> during driver initialization, so to properly reset fimd before enabling
>>> IOMMU, one should ensure that all power domains and clocks are really
>>> enabled. This patch adds pm_runtime and clocks management in the
>>> fimd_clear_channel() function to ensure that any access to fimd
>>> registers will be performed with clocks and power domains enabled.
>> I have tested this patch series on trats2 board which uses Exynos4412
>> SoC. However, the booting is halted out. Without iommu, the booting and
>> display works well.
>>
>> For this test, I also merged another your patch series in iommu exynos
>> tree and added device node relevant codes like below,
>>
>> in exynos4.dtsi file:
>> fimd: fimd@11c00000 {
>>                   ...
>>                 iommus = <&sysmmu_fimd0>;
>>                   ...
>>
>> sysmmu_fimd0: sysmmu@11E20000 {
>>          compatible = "samsung,exynos-sysmmu";
>>          reg = <0x11E20000 0x1000>;
>>          interrupt-parent = <&combiner>;
>>          interrupts = <5 2>;
>>          clock-names = "sysmmu", "master";
>>          clocks = <&clock CLK_SMMU_FIMD0>, <&clock CLK_FIMD0>;
>>           power-domains = <&pd_lcd0>;
>>          #iommu-cells = <0>;
>> };
>>
>> in exynos4412-trats2.dts file:
>> fimd@11c00000 {
>>                  status = "okay";
>>                  iommu-reserved-mapping = <0x40000000 0x40000000 0x40000000>;
>> };
>>
>> Can you check it out?
> In addition, sometimes I see below kernel panic logs which means page
> fault to fimd occurs while booting:

It looks that you didn't apply patch '[PATCH v7 24/25] ARM: DMA-mapping: add
support for creating reserved mappings in iova space'
(http://thread.gmane.org/gmane.linux.kernel.samsung-soc/45416/focus=45429 ).
There was no consensus on it and it was left unmerged. I will check if it
can be reworked on top of recently introduced iommu default domains feature,
however it would be great if the fixed for FIMD and DRM gets merged earlier,
so the issues in the drivers will no longer be a source of the problem.


> [    0.394228] 13800000.serial: ttySAC0 at MMIO 0x13800000 (irq = 56,
> base_baud = 0) is a S3C6400/10
> [    0.394788] 13810000.serial: ttySAC1 at MMIO 0x13810000 (irq = 57,
> base_baud = 0) is a S3C6400/10
> [    0.395281] 13820000.serial: ttySAC2 at MMIO 0x13820000 (irq = 58,
> base_baud = 0) is a S3C6400/10
> [    1.122219] console [ttySAC2] enabled
> [    1.126419] 13830000.serial: ttySAC3 at MMIO 0x13830000 (irq = 59,
> base_baud = 0) is a S3C6400/10
> [    1.136250] [drm] Initialized drm 1.1.0 20060810
> [    1.142710] PAGE FAULT occurred at 0x52188000 by 11e20000.sysmmu(Page
> table base: 0x6ea80000)
> [    1.149754]  Lv1 entry: 0x6e92dc01
> [    1.153172] ------------[ cut here ]------------
> [    1.157740] kernel BUG at drivers/iommu/exynos-iommu.c:364!
> [    1.163296] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> [    1.169110] Modules linked in:
> [    1.172154] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 4.1.0-rc4-00563-gee14f4e-dirty #1384
> [    1.180394] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [    1.186472] task: c06d2df0 ti: c06ce000 task.ti: c06ce000
> [    1.191861] PC is at exynos_sysmmu_irq+0x184/0x208
> [    1.196628] LR is at exynos_sysmmu_irq+0xd4/0x208
> [    1.201316] pc : [<c02677cc>]    lr : [<c026771c>]    psr: 60000193
> [    1.201316] sp : c06cfe90  ip : 00000000  fp : 00000000
> [    1.212772] r10: c06ff6a3  r9 : 00000521  r8 : 52188000
> [    1.217980] r7 : eea80000  r6 : ee9b3428  r5 : ee9b3410  r4 : 00000000
> [    1.224489] r3 : 6e92dc01  r2 : 6e92dc01  r1 : eea55810  r0 : ee9c4e00
> [    1.231002] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
> Segment kernel
> [    1.238378] Control: 10c5387d  Table: 4000404a  DAC: 00000015
> [    1.244107] Process swapper/0 (pid: 0, stack limit = 0xc06ce210)
> [    1.250096] Stack: (0xc06cfe90 to 0xc06d0000)
> [    1.254438] fe80:                                     c06cfe9c
> c039caac eef82b80 6ea80000
> [    1.262599] fea0: 16b1580f ee9b4240 ee84ad20 00000000 00000000
> 00000026 ee84acc0 c0060308
> [    1.270757] fec0: 00000000 eef81380 ee84acc0 ee84ad20 ee9b4240
> 00000015 ee804450 c06cff68
> [    1.278916] fee0: ee808000 c0060400 ee84acc0 ee84ad20 ee807000
> c00630a8 00000026 c06dddb8
> [    1.287075] ff00: ee807000 c005f98c 0000000a c0200ccc 00000015
> 00000000 00000015 00000000
> [    1.295235] ff20: 00000001 c005f98c c06caaac c005fc58 f002000c
> 00000015 c06d07a0 c06cff68
> [    1.303394] ff40: f0020000 c06ff6a1 00000001 c0009434 c0010068
> 60000113 ffffffff c06cff9c
> [    1.311553] ff60: c06cffb8 c0012f40 00000000 00000000 00001288
> c001c880 c06ce000 c06d04f8
> [    1.319712] ff80: c04a1f50 c06c92c4 c06cffb8 c06ff6a1 00000001
> 00000000 01000000 c06cffb0
> [    1.327871] ffa0: c0010064 c0010068 60000113 ffffffff c06ce000
> c0053f58 ffffffff c067bc54
> [    1.336031] ffc0: ffffffff ffffffff c067b678 00000000 00000000
> c06a70b8 c07023d4 c06d0480
> [    1.344190] ffe0: c06a70b4 c06d3f04 4000406a 413fc090 00000000
> 4000807c 00000000 00000000
> [    1.352366] [<c02677cc>] (exynos_sysmmu_irq) from [<c0060308>]
> (handle_irq_event_percpu+0x78/0x134)
> [    1.361382] [<c0060308>] (handle_irq_event_percpu) from [<c0060400>]
> (handle_irq_event+0x3c/0x5c)
> [    1.370235] [<c0060400>] (handle_irq_event) from [<c00630a8>]
> (handle_level_irq+0xc4/0x13c)
> [    1.378567] [<c00630a8>] (handle_level_irq) from [<c005f98c>]
> (generic_handle_irq+0x2c/0x3c)
> [    1.386990] [<c005f98c>] (generic_handle_irq) from [<c0200ccc>]
> (combiner_handle_cascade_irq+0x94/0x100)
> [    1.396448] [<c0200ccc>] (combiner_handle_cascade_irq) from
> [<c005f98c>] (generic_handle_irq+0x2c/0x3c)
> [    1.405820] [<c005f98c>] (generic_handle_irq) from [<c005fc58>]
> (__handle_domain_irq+0x7c/0xec)
> [    1.414502] [<c005fc58>] (__handle_domain_irq) from [<c0009434>]
> (gic_handle_irq+0x30/0x68)
> [    1.422833] [<c0009434>] (gic_handle_irq) from [<c0012f40>]
> (__irq_svc+0x40/0x74)
> [    1.430292] Exception stack(0xc06cff68 to 0xc06cffb0)
> [    1.435330] ff60:                   00000000 00000000 00001288
> c001c880 c06ce000 c06d04f8
> [    1.443489] ff80: c04a1f50 c06c92c4 c06cffb8 c06ff6a1 00000001
> 00000000 01000000 c06cffb0
> [    1.451646] ffa0: c0010064 c0010068 60000113 ffffffff
> [    1.456689] [<c0012f40>] (__irq_svc) from [<c0010068>]
> (arch_cpu_idle+0x38/0x3c)
> [    1.464069] [<c0010068>] (arch_cpu_idle) from [<c0053f58>]
> (cpu_startup_entry+0x12c/0x1c4)
> [    1.472314] [<c0053f58>] (cpu_startup_entry) from [<c067bc54>]
> (start_kernel+0x398/0x3a4)
> [    1.480468] [<c067bc54>] (start_kernel) from [<4000807c>] (0x4000807c)
> [    1.486977] Code: e28dd014 e8bd83f0 e5913008 eaffffc0 (e7f001f2)
> [    1.493062] ---[ end trace e8c5db152e219756 ]---
> [    1.497650] Kernel panic - not syncing: Fatal exception in interrupt
> [    1.503990] CPU2: stopping
> [    1.506681] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G      D
> 4.1.0-rc4-00563-gee14f4e-dirty #1384
> [    1.516137] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [    1.522229] [<c0015388>] (unwind_backtrace) from [<c0012440>]
> (show_stack+0x10/0x14)
> [    1.529951] [<c0012440>] (show_stack) from [<c0499840>]
> (dump_stack+0x84/0xc4)
> [    1.537151] [<c0499840>] (dump_stack) from [<c0014634>]
> (handle_IPI+0x17c/0x18c)
> [    1.544526] [<c0014634>] (handle_IPI) from [<c0009468>]
> (gic_handle_irq+0x64/0x68)
> [    1.552078] [<c0009468>] (gic_handle_irq) from [<c0012f40>]
> (__irq_svc+0x40/0x74)
> [    1.559537] Exception stack(0xee8b1fa0 to 0xee8b1fe8)
> [    1.564576] 1fa0: 00000002 00000000 000005de c001c880 ee8b0000
> c06d04f8 c04a1f50 c06c92c4
> [    1.572735] 1fc0: ee8b1ff0 c06ff6a1 00000001 00000000 01000000
> ee8b1fe8 c0010064 c0010068
> [    1.580890] 1fe0: 60000113 ffffffff
> [    1.584369] [<c0012f40>] (__irq_svc) from [<c0010068>]
> (arch_cpu_idle+0x38/0x3c)
> [    1.591749] [<c0010068>] (arch_cpu_idle) from [<c0053f58>]
> (cpu_startup_entry+0x12c/0x1c4)
> [    1.599992] [<c0053f58>] (cpu_startup_entry) from [<4000950c>]
> (0x4000950c)
> [    1.606932] CPU3: stopping
> [    1.609626] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G      D
> 4.1.0-rc4-00563-gee14f4e-dirty #1384
> [    1.619082] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [    1.625171] [<c0015388>] (unwind_backtrace) from [<c0012440>]
> (show_stack+0x10/0x14)
> [    1.632892] [<c0012440>] (show_stack) from [<c0499840>]
> (dump_stack+0x84/0xc4)
> [    1.640094] [<c0499840>] (dump_stack) from [<c0014634>]
> (handle_IPI+0x17c/0x18c)
> [    1.647470] [<c0014634>] (handle_IPI) from [<c0009468>]
> (gic_handle_irq+0x64/0x68)
> [    1.655022] [<c0009468>] (gic_handle_irq) from [<c0012f40>]
> (__irq_svc+0x40/0x74)
> [    1.662482] Exception stack(0xee8b3fa0 to 0xee8b3fe8)
> [    1.667521] 3fa0: 00000003 00000000 000009c4 c001c880 ee8b2000
> c06d04f8 c04a1f50 c06c92c4
> [    1.675680] 3fc0: ee8b3ff0 c06ff6a1 00000001 00000000 01000000
> ee8b3fe8 c0010064 c0010068
> [    1.683835] 3fe0: 60000113 ffffffff
> [    1.687314] [<c0012f40>] (__irq_svc) from [<c0010068>]
> (arch_cpu_idle+0x38/0x3c)
> [    1.694694] [<c0010068>] (arch_cpu_idle) from [<c0053f58>]
> (cpu_startup_entry+0x12c/0x1c4)
> [    1.702936] [<c0053f58>] (cpu_startup_entry) from [<4000950c>]
> (0x4000950c)
> [    2.847590] SMP: failed to stop secondary CPUs
> [    2.850551] ---[ end Kernel panic - not syncing: Fatal exception in
> interrupt
>
>
>> Thanks,
>> Inki Dae
>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>>> ---
>>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c | 22 ++++++++++++++++++----
>>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> index 96618534358e..3ec9d4299a86 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> @@ -242,12 +242,21 @@ static void fimd_enable_shadow_channel_path(struct fimd_context *ctx,
>>>   	writel(val, ctx->regs + SHADOWCON);
>>>   }
>>>   
>>> +static int fimd_enable_vblank(struct exynos_drm_crtc *crtc);
>>> +static void fimd_disable_vblank(struct exynos_drm_crtc *crtc);
>>> +
>>>   static void fimd_clear_channel(struct fimd_context *ctx)
>>>   {
>>>   	unsigned int win, ch_enabled = 0;
>>>   
>>>   	DRM_DEBUG_KMS("%s\n", __FILE__);
>>>   
>>> +	/* Hardware is in unknown state, so ensure it gets enabled properly */
>>> +	pm_runtime_get_sync(ctx->dev);
>>> +
>>> +	clk_prepare_enable(ctx->bus_clk);
>>> +	clk_prepare_enable(ctx->lcd_clk);
>>> +
>>>   	/* Check if any channel is enabled. */
>>>   	for (win = 0; win < WINDOWS_NR; win++) {
>>>   		u32 val = readl(ctx->regs + WINCON(win));
>>> @@ -265,12 +274,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
>>>   
>>>   	/* Wait for vsync, as disable channel takes effect at next vsync */
>>>   	if (ch_enabled) {
>>> -		unsigned int state = ctx->suspended;
>>> -
>>> -		ctx->suspended = 0;
>>> +		ctx->suspended = false;
>>> +		fimd_enable_vblank(ctx->crtc);
>>>   		fimd_wait_for_vblank(ctx->crtc);
>>> -		ctx->suspended = state;
>>> +		fimd_disable_vblank(ctx->crtc);
>>> +		ctx->suspended = true;
>>>   	}
>>> +
>>> +	clk_disable_unprepare(ctx->lcd_clk);
>>> +	clk_disable_unprepare(ctx->bus_clk);
>>> +
>>> +	pm_runtime_put(ctx->dev);
>>>   }
>>>   
>>>   static int fimd_iommu_attach_devices(struct fimd_context *ctx,
>>>
>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Inki Dae]

On 2015년 06월 03일 17:26, Marek Szyprowski wrote:
> One should not do any assumptions on the stare of the fimd hardware
> during driver initialization, so to properly reset fimd before enabling
> IOMMU, one should ensure that all power domains and clocks are really
> enabled. This patch adds pm_runtime and clocks management in the
> fimd_clear_channel() function to ensure that any access to fimd
> registers will be performed with clocks and power domains enabled.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 96618534358e..3ec9d4299a86 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -242,12 +242,21 @@ static void fimd_enable_shadow_channel_path(struct fimd_context *ctx,
>  	writel(val, ctx->regs + SHADOWCON);
>  }
>  
> +static int fimd_enable_vblank(struct exynos_drm_crtc *crtc);
> +static void fimd_disable_vblank(struct exynos_drm_crtc *crtc);

You can remove abvoe declarations. See the below comment.

> +
>  static void fimd_clear_channel(struct fimd_context *ctx)
>  {
>  	unsigned int win, ch_enabled = 0;
>  
>  	DRM_DEBUG_KMS("%s\n", __FILE__);
>  
> +	/* Hardware is in unknown state, so ensure it gets enabled properly */
> +	pm_runtime_get_sync(ctx->dev);
> +
> +	clk_prepare_enable(ctx->bus_clk);
> +	clk_prepare_enable(ctx->lcd_clk);
> +
>  	/* Check if any channel is enabled. */
>  	for (win = 0; win < WINDOWS_NR; win++) {
>  		u32 val = readl(ctx->regs + WINCON(win));
> @@ -265,12 +274,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
>  
>  	/* Wait for vsync, as disable channel takes effect at next vsync */
>  	if (ch_enabled) {
> -		unsigned int state = ctx->suspended;
> -
> -		ctx->suspended = 0;
> +		ctx->suspended = false;
> +		fimd_enable_vblank(ctx->crtc);

I think you can call enable_vblank callback instead of
fimd_enable_vblank function because ctx object has exynos_crtc object.

i.e.,
		struct exynos_drm_crtc_ops *ops = ctx->crtc->ops;
		...
		if (ops->enable_vblank)
			ops->enable_vblank(ctx->crtc);
		...

>  		fimd_wait_for_vblank(ctx->crtc);
> -		ctx->suspended = state;
> +		fimd_disable_vblank(ctx->crtc);

Ditto.

Thanks,
Inki Dae

> +		ctx->suspended = true;
>  	}
> +
> +	clk_disable_unprepare(ctx->lcd_clk);
> +	clk_disable_unprepare(ctx->bus_clk);
> +
> +	pm_runtime_put(ctx->dev);
>  }
>  
>  static int fimd_iommu_attach_devices(struct fimd_context *ctx,
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Marek Szyprowski]

Hello,

On 2015-06-11 17:04, Inki Dae wrote:
> On 2015년 06월 03일 17:26, Marek Szyprowski wrote:
>> One should not do any assumptions on the stare of the fimd hardware
>> during driver initialization, so to properly reset fimd before enabling
>> IOMMU, one should ensure that all power domains and clocks are really
>> enabled. This patch adds pm_runtime and clocks management in the
>> fimd_clear_channel() function to ensure that any access to fimd
>> registers will be performed with clocks and power domains enabled.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c | 22 ++++++++++++++++++----
>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 96618534358e..3ec9d4299a86 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -242,12 +242,21 @@ static void fimd_enable_shadow_channel_path(struct fimd_context *ctx,
>>   	writel(val, ctx->regs + SHADOWCON);
>>   }
>>   
>> +static int fimd_enable_vblank(struct exynos_drm_crtc *crtc);
>> +static void fimd_disable_vblank(struct exynos_drm_crtc *crtc);
> You can remove abvoe declarations. See the below comment.
>
>> +
>>   static void fimd_clear_channel(struct fimd_context *ctx)
>>   {
>>   	unsigned int win, ch_enabled = 0;
>>   
>>   	DRM_DEBUG_KMS("%s\n", __FILE__);
>>   
>> +	/* Hardware is in unknown state, so ensure it gets enabled properly */
>> +	pm_runtime_get_sync(ctx->dev);
>> +
>> +	clk_prepare_enable(ctx->bus_clk);
>> +	clk_prepare_enable(ctx->lcd_clk);
>> +
>>   	/* Check if any channel is enabled. */
>>   	for (win = 0; win < WINDOWS_NR; win++) {
>>   		u32 val = readl(ctx->regs + WINCON(win));
>> @@ -265,12 +274,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
>>   
>>   	/* Wait for vsync, as disable channel takes effect at next vsync */
>>   	if (ch_enabled) {
>> -		unsigned int state = ctx->suspended;
>> -
>> -		ctx->suspended = 0;
>> +		ctx->suspended = false;
>> +		fimd_enable_vblank(ctx->crtc);
> I think you can call enable_vblank callback instead of
> fimd_enable_vblank function because ctx object has exynos_crtc object.
>
> i.e.,
> 		struct exynos_drm_crtc_ops *ops = ctx->crtc->ops;
> 		...
> 		if (ops->enable_vblank)
> 			ops->enable_vblank(ctx->crtc);
> 		...

Well, I don't like such indirect calls to known functions, but if you prefer
this approach I will send an updated patch in a minute. There is also 
alternative
way of getting rid of forward declarations. Code of fimd_enable_vblank and
fimd_disable_vblank can be moved closer to fimd_wait_vblank function.
I will also send such alternative patch. Feel free to select the one 
that better
fits your preferences.

>>   		fimd_wait_for_vblank(ctx->crtc);
>> -		ctx->suspended = state;
>> +		fimd_disable_vblank(ctx->crtc);
> Ditto.
>
> Thanks,
> Inki Dae
>
>> +		ctx->suspended = true;
>>   	}
>> +
>> +	clk_disable_unprepare(ctx->lcd_clk);
>> +	clk_disable_unprepare(ctx->bus_clk);
>> +
>> +	pm_runtime_put(ctx->dev);
>>   }
>>   
>>   static int fimd_iommu_attach_devices(struct fimd_context *ctx,
>>
>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Marek Szyprowski]

One should not do any assumptions on the stare of the fimd hardware
during driver initialization, so to properly reset fimd before enabling
IOMMU, one should ensure that all power domains and clocks are really
enabled. This patch adds pm_runtime and clocks management in the
fimd_clear_channel() function to ensure that any access to fimd
registers will be performed with clocks and power domains enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
Changelog:
v3:
- replaced forward declaration and calls to fimd_{enable,disable}_vblank
  functions by calls throught ctrt ops

v2:
- rebased onto latest exynos-drm-next branch with atomic mode setting
  patches applied.
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 96618534358e..c67bb6b97399 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -248,6 +248,12 @@ static void fimd_clear_channel(struct fimd_context *ctx)
 
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
+	/* Hardware is in unknown state, so ensure it gets enabled properly */
+	pm_runtime_get_sync(ctx->dev);
+
+	clk_prepare_enable(ctx->bus_clk);
+	clk_prepare_enable(ctx->lcd_clk);
+
 	/* Check if any channel is enabled. */
 	for (win = 0; win < WINDOWS_NR; win++) {
 		u32 val = readl(ctx->regs + WINCON(win));
@@ -265,12 +271,22 @@ static void fimd_clear_channel(struct fimd_context *ctx)
 
 	/* Wait for vsync, as disable channel takes effect at next vsync */
 	if (ch_enabled) {
-		unsigned int state = ctx->suspended;
-
-		ctx->suspended = 0;
-		fimd_wait_for_vblank(ctx->crtc);
-		ctx->suspended = state;
+		const struct exynos_drm_crtc_ops *ops = ctx->crtc->ops;
+
+		ctx->suspended = false;
+		if (ops->enable_vblank)
+			ops->enable_vblank(ctx->crtc);
+		if (ops->wait_for_vblank)
+			ops->wait_for_vblank(ctx->crtc);
+		if (ops->disable_vblank)
+			ops->disable_vblank(ctx->crtc);
+		ctx->suspended = true;
 	}
+
+	clk_disable_unprepare(ctx->lcd_clk);
+	clk_disable_unprepare(ctx->bus_clk);
+
+	pm_runtime_put(ctx->dev);
 }
 
 static int fimd_iommu_attach_devices(struct fimd_context *ctx,
-- 
1.9.2

[PATCH v3 (alternative) 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Marek Szyprowski]

One should not do any assumptions on the stare of the fimd hardware
during driver initialization, so to properly reset fimd before enabling
IOMMU, one should ensure that all power domains and clocks are really
enabled. This patch adds pm_runtime and clocks management in the
fimd_clear_channel() function to ensure that any access to fimd
registers will be performed with clocks and power domains enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
Changelog:
v3 (alternative):
- araranged code by moving fimd_{enable,disable}_vblank functions before
  fimd_clear_channel to avoid forward declaration usage

v2:
- rebased onto latest exynos-drm-next branch with atomic mode setting
  patches applied.


 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 130 +++++++++++++++++--------------
 1 file changed, 71 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 96618534358e..f490895a8b02 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -196,6 +196,62 @@ static inline struct fimd_driver_data *drm_fimd_get_driver_data(
 	return (struct fimd_driver_data *)of_id->data;
 }
 
+static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
+{
+	struct fimd_context *ctx = crtc->ctx;
+	u32 val;
+
+	if (ctx->suspended)
+		return -EPERM;
+
+	if (!test_and_set_bit(0, &ctx->irq_flags)) {
+		val = readl(ctx->regs + VIDINTCON0);
+
+		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;
+
+			val &= ~VIDINTCON0_FRAMESEL0_MASK;
+			val |= VIDINTCON0_FRAMESEL0_VSYNC;
+			val &= ~VIDINTCON0_FRAMESEL1_MASK;
+			val |= VIDINTCON0_FRAMESEL1_NONE;
+		}
+
+		writel(val, ctx->regs + VIDINTCON0);
+	}
+
+	return 0;
+}
+
+static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
+{
+	struct fimd_context *ctx = crtc->ctx;
+	u32 val;
+
+	if (ctx->suspended)
+		return;
+
+	if (test_and_clear_bit(0, &ctx->irq_flags)) {
+		val = readl(ctx->regs + VIDINTCON0);
+
+		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);
+	}
+}
+
 static void fimd_wait_for_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct fimd_context *ctx = crtc->ctx;
@@ -248,6 +304,12 @@ static void fimd_clear_channel(struct fimd_context *ctx)
 
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
+	/* Hardware is in unknown state, so ensure it gets enabled properly */
+	pm_runtime_get_sync(ctx->dev);
+
+	clk_prepare_enable(ctx->bus_clk);
+	clk_prepare_enable(ctx->lcd_clk);
+
 	/* Check if any channel is enabled. */
 	for (win = 0; win < WINDOWS_NR; win++) {
 		u32 val = readl(ctx->regs + WINCON(win));
@@ -265,12 +327,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
 
 	/* Wait for vsync, as disable channel takes effect at next vsync */
 	if (ch_enabled) {
-		unsigned int state = ctx->suspended;
-
-		ctx->suspended = 0;
+		ctx->suspended = false;
+		fimd_enable_vblank(ctx->crtc);
 		fimd_wait_for_vblank(ctx->crtc);
-		ctx->suspended = state;
+		fimd_disable_vblank(ctx->crtc);
+		ctx->suspended = true;
 	}
+
+	clk_disable_unprepare(ctx->lcd_clk);
+	clk_disable_unprepare(ctx->bus_clk);
+
+	pm_runtime_put(ctx->dev);
 }
 
 static int fimd_iommu_attach_devices(struct fimd_context *ctx,
@@ -434,61 +501,6 @@ static void fimd_commit(struct exynos_drm_crtc *crtc)
 	writel(val, ctx->regs + VIDCON0);
 }
 
-static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
-{
-	struct fimd_context *ctx = crtc->ctx;
-	u32 val;
-
-	if (ctx->suspended)
-		return -EPERM;
-
-	if (!test_and_set_bit(0, &ctx->irq_flags)) {
-		val = readl(ctx->regs + VIDINTCON0);
-
-		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;
-
-			val &= ~VIDINTCON0_FRAMESEL0_MASK;
-			val |= VIDINTCON0_FRAMESEL0_VSYNC;
-			val &= ~VIDINTCON0_FRAMESEL1_MASK;
-			val |= VIDINTCON0_FRAMESEL1_NONE;
-		}
-
-		writel(val, ctx->regs + VIDINTCON0);
-	}
-
-	return 0;
-}
-
-static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
-{
-	struct fimd_context *ctx = crtc->ctx;
-	u32 val;
-
-	if (ctx->suspended)
-		return;
-
-	if (test_and_clear_bit(0, &ctx->irq_flags)) {
-		val = readl(ctx->regs + VIDINTCON0);
-
-		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);
-	}
-}
 
 static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win)
 {
-- 
1.9.2

Re: [PATCH v3 (alternative) 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Inki Dae]

On 2015년 06월 12일 18:07, Marek Szyprowski wrote:
> One should not do any assumptions on the stare of the fimd hardware
> during driver initialization, so to properly reset fimd before enabling
> IOMMU, one should ensure that all power domains and clocks are really
> enabled. This patch adds pm_runtime and clocks management in the
> fimd_clear_channel() function to ensure that any access to fimd
> registers will be performed with clocks and power domains enabled.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
> Changelog:
> v3 (alternative):
> - araranged code by moving fimd_{enable,disable}_vblank functions before
>   fimd_clear_channel to avoid forward declaration usage

Marek,

For the iommu and atomic feature test, I merged below patches in
addition and tested them.

[PATCH v2 1/3] drm/exynos: remove to call mixer_wait_for_vblank
[PATCH v2 2/3] drm/exynos: remove chained calls to enable
[PATCH v2 3/3] drm/exynos: initialize VIDCON0 when fimd is disabled


However, I see below one warning log,

[    1.227115] [drm] Initialized drm 1.1.0 20060810
[    1.235852] exynos-drm exynos-drm: bound exynos-drm-vidi (ops
vidi_component_ops)
[    1.253548] ------------[ cut here ]------------
[    1.256700] WARNING: CPU: 0 PID: 0 at drivers/gpu/drm/drm_irq.c:1718
drm_handle_vblank+0x2a0/0x308()
[    1.265800] Modules linked in:
[    1.268844] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.1.0-rc4-00574-gf8eb363-dirty #1412
[    1.277085] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    1.283184] [<c0015464>] (unwind_backtrace) from [<c00123c0>]
(show_stack+0x10/0x14)
[    1.288540] exynos-drm exynos-drm: bound 11c00000.fimd (ops
fimd_component_ops)
[    1.288883] exynos-drm exynos-drm: bound 11c80000.dsi (ops
exynos_dsi_component_ops)
[    1.288888] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[    1.288891] [drm] No driver support for vblank timestamp query.
[    1.288932] [drm] Initialized exynos 1.0.0 20110530 on minor 0
[    1.324248] [<c00123c0>] (show_stack) from [<c0528f74>]
(dump_stack+0x84/0xc4)
[    1.331437] [<c0528f74>] (dump_stack) from [<c00246ac>]
(warn_slowpath_common+0x80/0xb0)
[    1.339504] [<c00246ac>] (warn_slowpath_common) from [<c0024778>]
(warn_slowpath_null+0x1c/0x24)
[    1.348270] [<c0024778>] (warn_slowpath_null) from [<c029c898>]
(drm_handle_vblank+0x2a0/0x308)
[    1.356965] [<c029c898>] (drm_handle_vblank) from [<c02b8c80>]
(fimd_irq_handler+0x78/0xd0)
[    1.365292] [<c02b8c80>] (fimd_irq_handler) from [<c00608f8>]
(handle_irq_event_percpu+0x78/0x134)
[    1.374231] [<c00608f8>] (handle_irq_event_percpu) from [<c00609f0>]
(handle_irq_event+0x3c/0x5c)
[    1.374244] [<c00609f0>] (handle_irq_event) from [<c0063698>]
(handle_level_irq+0xc4/0x13c)
[    1.374256] [<c0063698>] (handle_level_irq) from [<c005ff7c>]
(generic_handle_irq+0x2c/0x3c)
[    1.374269] [<c005ff7c>] (generic_handle_irq) from [<c02214ec>]
(combiner_handle_cascade_irq+0x94/0x100)
[    1.374281] [<c02214ec>] (combiner_handle_cascade_irq) from
[<c005ff7c>] (generic_handle_irq+0x2c/0x3c)
[    1.374290] [<c005ff7c>] (generic_handle_irq) from [<c0060248>]
(__handle_domain_irq+0x7c/0xec)
[    1.374300] [<c0060248>] (__handle_domain_irq) from [<c0009434>]
(gic_handle_irq+0x30/0x68)


And below one page fault error when modetest is terminated,

# modetest -M exynos -v -s 31@29:720x1280
setting mode 720x1280-60Hz@XR24 on connectors 31, crtc 29
freq: 59.99Hz

[   34.831025] PAGE FAULT occurred at 0x20400000 by 11e20000.sysmmu(Page
table base: 0x6e324000)
[   34.838072]  Lv1 entry: 0x6e92dc01
[   34.841489] ------------[ cut here ]------------
[   34.846058] kernel BUG at drivers/iommu/exynos-iommu.c:364!
[   34.851614] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
[   34.857428] Modules linked in:
[   34.860472] CPU: 0 PID: 6 Comm: kworker/u8:0 Tainted: G        W
  4.1.0-rc4-00574-gf8eb363-dirty #1412
[   34.870188] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   34.876273] Workqueue: events_unbound async_run_entry_fn
[   34.881560] task: ee879540 ti: ee8a2000 task.ti: ee8a2000
[   34.886946] PC is at exynos_sysmmu_irq+0x184/0x208
[   34.891716] LR is at exynos_sysmmu_irq+0xd4/0x208
[   34.896404] pc : [<c02880d0>]    lr : [<c0288020>]    psr: 60000193
[   34.896404] sp : ee8a3c00  ip : 00000000  fp : eeacbc10
[   34.907860] r10: c07c27ef  r9 : 00000204  r8 : 20400000
[   34.913068] r7 : ee324000  r6 : ee9b3428  r5 : ee9b3410  r4 : 00000000
[   34.919577] r3 : 6e92dc01  r2 : 6e92dc01  r1 : eea55810  r0 : eeb0a000
[   34.926089] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
Segment kernel
[   34.933466] Control: 10c5387d  Table: 6dd2c04a  DAC: 00000015
[   34.939195] Process kworker/u8:0 (pid: 6, stack limit = 0xee8a2210)
[   34.945444] Stack: (0xee8a3c00 to 0xee8a4000)
[   34.949788] 3c00: ee8a3c0c c0049df8 00000000 6e324000 0003d4f3
ee9b13c0 ee84ad20 00000000
[   34.957947] 3c20: 00000000 00000026 ee84acc0 c00608f8 60000093
eef84580 ee84acc0 ee84ad20
[   34.966106] 3c40: ee9b13c0 00000015 ee804450 ee8a3cd8 ee808000
c00609f0 ee84acc0 ee84ad20
[   34.974265] 3c60: ee807000 c0063698 00000026 c079d6f8 ee807000
c005ff7c 0000000a c02214ec
[   34.982424] 3c80: 00000015 00000000 00000015 00000000 00000001
c005ff7c c0789aac c0060248
[   34.990583] 3ca0: f002000c 00000015 c078e7ac ee8a3cd8 f0020000
ee8a2000 00000001 c0009434
[   34.998742] 3cc0: c052e8fc 60000013 ffffffff ee8a3d0c 00000004
c0012ec0 eeacbd18 ee879540
[   35.006901] 3ce0: 00000000 00000271 eeacbd18 00000000 eea55010
c078e100 00000004 ee8a2000
[   35.015061] 3d00: 00000001 eeacbc10 eeacbd88 ee8a3d20 c052e8f8
c052e8fc 60000013 ffffffff
[   35.023220] 3d20: eeacbd18 c02dab48 20000113 00000003 60000013
eeacbd18 00000004 eeacbc60
[   35.031379] 3d40: ffff30d5 eeacbca8 eeacbd18 00000004 60000013
ffff30d5 c0802fe8 c02db05c
[   35.039538] 3d60: eeacbc60 00000002 c078e100 eeacbc60 ffff30d5
c03b0f2c c078bb40 ee8a3e28
[   35.047697] 3d80: 00000001 ee8a3db8 eeacbc70 00000002 eeacbc70
ee8a3db8 eeacbca8 c052e880
[   35.055857] 3da0: eeacbc70 c052d818 00000000 00000009 00000000
c002791c ee8a3db8 11111111
[   35.064016] 3dc0: 11111111 ee8a3dc4 11111111 eeacbc60 00000002
c078e100 ee8a3e28 ffff30d5
[   35.072175] 3de0: c0802fe8 00000001 ee80b400 c03ac4cc 00000000
eeacbc60 ee8a3e28 00000002
[   35.080334] 3e00: eea47ac0 00000000 ee849d00 c03ac69c 00000000
00000001 00000001 eea47ac0
[   35.088493] 3e20: eea47ac0 c02e9f10 00000009 60000001 eea47ac0
00010009 00000001 eea47ac0
[   35.096653] 3e40: ee29149c 00000000 ee00c200 c02e5bdc 00000001
c004425c ee00c200 00000047
[   35.104812] 3e60: ee00c200 ee00c200 ee8a3eb4 ee8a3eb4 00000000
c02e5c5c ee00c200 ee00c200
[   35.112971] 3e80: 00000047 c02e564c ee00c200 00000047 ee8a3eb4
ee80da00 00000000 c02e5704
[   35.121130] 3ea0: eeafd800 c07c5020 eeafd84c c025383c ee849d00
c052d110 eeafd800 c0252a48
[   35.129289] 3ec0: ee914724 c07c5020 edcc9c40 c0252f58 edcc9c50
c003ed48 eef84580 eea2f700
[   35.137448] 3ee0: ee849d00 edcc9c50 ee80b400 c0037944 ee8a2000
ee80b400 00000001 ee80b400
[   35.145608] 3f00: ee849d18 ee80b420 ee8a2000 00000088 c07c27bc
ee849d00 ee80b400 c0037ba0
[   35.153767] 3f20: c078e100 ee80b570 ee849d00 00000000 ee84b540
ee849d00 c0037b54 00000000
[   35.161926] 3f40: 00000000 00000000 00000000 c003c8b8 ffffffff
00000000 ffffffff ee849d00
[   35.170085] 3f60: 00000000 00000000 dead4ead ffffffff ffffffff
ee8a3f74 ee8a3f74 00000000
[   35.178245] 3f80: 00000000 dead4ead ffffffff ffffffff ee8a3f90
ee8a3f90 ee8a3fac ee84b540
[   35.186403] 3fa0: c003c7dc 00000000 00000000 c000f5a0 00000000
00000000 00000000 00000000
[   35.194562] 3fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[   35.202722] 3fe0: 00000000 00000000 00000000 00000000 00000013
00000000 ffffffff ffffffff
[   35.210894] [<c02880d0>] (exynos_sysmmu_irq) from [<c00608f8>]
(handle_irq_event_percpu+0x78/0x134)
[   35.219914] [<c00608f8>] (handle_irq_event_percpu) from [<c00609f0>]
(handle_irq_event+0x3c/0x5c)
[   35.228768] [<c00609f0>] (handle_irq_event) from [<c0063698>]
(handle_level_irq+0xc4/0x13c)
[   35.237101] [<c0063698>] (handle_level_irq) from [<c005ff7c>]
(generic_handle_irq+0x2c/0x3c)
[   35.245521] [<c005ff7c>] (generic_handle_irq) from [<c02214ec>]
(combiner_handle_cascade_irq+0x94/0x100)
[   35.254980] [<c02214ec>] (combiner_handle_cascade_irq) from
[<c005ff7c>] (generic_handle_irq+0x2c/0x3c)
[   35.264353] [<c005ff7c>] (generic_handle_irq) from [<c0060248>]
(__handle_domain_irq+0x7c/0xec)
[   35.273034] [<c0060248>] (__handle_domain_irq) from [<c0009434>]
(gic_handle_irq+0x30/0x68)
[   35.281366] [<c0009434>] (gic_handle_irq) from [<c0012ec0>]
(__irq_svc+0x40/0x74)

Could you check the iommu with atomic feature? I tested these features
on trats2 board, and enabled one more crtc drivers - HDMI, VIDI, and FIMD.

To other Exynos guys,
could you guys check the same issues also? If you give some helps to us,
we'd be glad.

Thanks,
Inki Dae

> 
> v2:
> - rebased onto latest exynos-drm-next branch with atomic mode setting
>   patches applied.
> 
> 
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 130 +++++++++++++++++--------------
>  1 file changed, 71 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 96618534358e..f490895a8b02 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -196,6 +196,62 @@ static inline struct fimd_driver_data *drm_fimd_get_driver_data(
>  	return (struct fimd_driver_data *)of_id->data;
>  }
>  
> +static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
> +{
> +	struct fimd_context *ctx = crtc->ctx;
> +	u32 val;
> +
> +	if (ctx->suspended)
> +		return -EPERM;
> +
> +	if (!test_and_set_bit(0, &ctx->irq_flags)) {
> +		val = readl(ctx->regs + VIDINTCON0);
> +
> +		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;
> +
> +			val &= ~VIDINTCON0_FRAMESEL0_MASK;
> +			val |= VIDINTCON0_FRAMESEL0_VSYNC;
> +			val &= ~VIDINTCON0_FRAMESEL1_MASK;
> +			val |= VIDINTCON0_FRAMESEL1_NONE;
> +		}
> +
> +		writel(val, ctx->regs + VIDINTCON0);
> +	}
> +
> +	return 0;
> +}
> +
> +static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
> +{
> +	struct fimd_context *ctx = crtc->ctx;
> +	u32 val;
> +
> +	if (ctx->suspended)
> +		return;
> +
> +	if (test_and_clear_bit(0, &ctx->irq_flags)) {
> +		val = readl(ctx->regs + VIDINTCON0);
> +
> +		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);
> +	}
> +}
> +
>  static void fimd_wait_for_vblank(struct exynos_drm_crtc *crtc)
>  {
>  	struct fimd_context *ctx = crtc->ctx;
> @@ -248,6 +304,12 @@ static void fimd_clear_channel(struct fimd_context *ctx)
>  
>  	DRM_DEBUG_KMS("%s\n", __FILE__);
>  
> +	/* Hardware is in unknown state, so ensure it gets enabled properly */
> +	pm_runtime_get_sync(ctx->dev);
> +
> +	clk_prepare_enable(ctx->bus_clk);
> +	clk_prepare_enable(ctx->lcd_clk);
> +
>  	/* Check if any channel is enabled. */
>  	for (win = 0; win < WINDOWS_NR; win++) {
>  		u32 val = readl(ctx->regs + WINCON(win));
> @@ -265,12 +327,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
>  
>  	/* Wait for vsync, as disable channel takes effect at next vsync */
>  	if (ch_enabled) {
> -		unsigned int state = ctx->suspended;
> -
> -		ctx->suspended = 0;
> +		ctx->suspended = false;
> +		fimd_enable_vblank(ctx->crtc);
>  		fimd_wait_for_vblank(ctx->crtc);
> -		ctx->suspended = state;
> +		fimd_disable_vblank(ctx->crtc);
> +		ctx->suspended = true;
>  	}
> +
> +	clk_disable_unprepare(ctx->lcd_clk);
> +	clk_disable_unprepare(ctx->bus_clk);
> +
> +	pm_runtime_put(ctx->dev);
>  }
>  
>  static int fimd_iommu_attach_devices(struct fimd_context *ctx,
> @@ -434,61 +501,6 @@ static void fimd_commit(struct exynos_drm_crtc *crtc)
>  	writel(val, ctx->regs + VIDCON0);
>  }
>  
> -static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
> -{
> -	struct fimd_context *ctx = crtc->ctx;
> -	u32 val;
> -
> -	if (ctx->suspended)
> -		return -EPERM;
> -
> -	if (!test_and_set_bit(0, &ctx->irq_flags)) {
> -		val = readl(ctx->regs + VIDINTCON0);
> -
> -		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;
> -
> -			val &= ~VIDINTCON0_FRAMESEL0_MASK;
> -			val |= VIDINTCON0_FRAMESEL0_VSYNC;
> -			val &= ~VIDINTCON0_FRAMESEL1_MASK;
> -			val |= VIDINTCON0_FRAMESEL1_NONE;
> -		}
> -
> -		writel(val, ctx->regs + VIDINTCON0);
> -	}
> -
> -	return 0;
> -}
> -
> -static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
> -{
> -	struct fimd_context *ctx = crtc->ctx;
> -	u32 val;
> -
> -	if (ctx->suspended)
> -		return;
> -
> -	if (test_and_clear_bit(0, &ctx->irq_flags)) {
> -		val = readl(ctx->regs + VIDINTCON0);
> -
> -		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);
> -	}
> -}
>  
>  static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win)
>  {
> 

Re: [PATCH v3 (alternative) 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Marek Szyprowski]

Hello,

On 2015-06-12 14:10, Inki Dae wrote:
> On 2015년 06월 12일 18:07, Marek Szyprowski wrote:
>> One should not do any assumptions on the stare of the fimd hardware
>> during driver initialization, so to properly reset fimd before enabling
>> IOMMU, one should ensure that all power domains and clocks are really
>> enabled. This patch adds pm_runtime and clocks management in the
>> fimd_clear_channel() function to ensure that any access to fimd
>> registers will be performed with clocks and power domains enabled.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>> Changelog:
>> v3 (alternative):
>> - araranged code by moving fimd_{enable,disable}_vblank functions before
>>    fimd_clear_channel to avoid forward declaration usage
> Marek,
>
> For the iommu and atomic feature test, I merged below patches in
> addition and tested them.
>
> [PATCH v2 1/3] drm/exynos: remove to call mixer_wait_for_vblank
> [PATCH v2 2/3] drm/exynos: remove chained calls to enable
> [PATCH v2 3/3] drm/exynos: initialize VIDCON0 when fimd is disabled
>
>
> However, I see below one warning log,
>
> [    1.227115] [drm] Initialized drm 1.1.0 20060810
> [    1.235852] exynos-drm exynos-drm: bound exynos-drm-vidi (ops
> vidi_component_ops)
> [    1.253548] ------------[ cut here ]------------
> [    1.256700] WARNING: CPU: 0 PID: 0 at drivers/gpu/drm/drm_irq.c:1718
> drm_handle_vblank+0x2a0/0x308()
> [    1.265800] Modules linked in:
> [    1.268844] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 4.1.0-rc4-00574-gf8eb363-dirty #1412
> [    1.277085] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [    1.283184] [<c0015464>] (unwind_backtrace) from [<c00123c0>]
> (show_stack+0x10/0x14)
> [    1.288540] exynos-drm exynos-drm: bound 11c00000.fimd (ops
> fimd_component_ops)
> [    1.288883] exynos-drm exynos-drm: bound 11c80000.dsi (ops
> exynos_dsi_component_ops)
> [    1.288888] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [    1.288891] [drm] No driver support for vblank timestamp query.
> [    1.288932] [drm] Initialized exynos 1.0.0 20110530 on minor 0
> [    1.324248] [<c00123c0>] (show_stack) from [<c0528f74>]
> (dump_stack+0x84/0xc4)
> [    1.331437] [<c0528f74>] (dump_stack) from [<c00246ac>]
> (warn_slowpath_common+0x80/0xb0)
> [    1.339504] [<c00246ac>] (warn_slowpath_common) from [<c0024778>]
> (warn_slowpath_null+0x1c/0x24)
> [    1.348270] [<c0024778>] (warn_slowpath_null) from [<c029c898>]
> (drm_handle_vblank+0x2a0/0x308)
> [    1.356965] [<c029c898>] (drm_handle_vblank) from [<c02b8c80>]
> (fimd_irq_handler+0x78/0xd0)
> [    1.365292] [<c02b8c80>] (fimd_irq_handler) from [<c00608f8>]
> (handle_irq_event_percpu+0x78/0x134)
> [    1.374231] [<c00608f8>] (handle_irq_event_percpu) from [<c00609f0>]
> (handle_irq_event+0x3c/0x5c)
> [    1.374244] [<c00609f0>] (handle_irq_event) from [<c0063698>]
> (handle_level_irq+0xc4/0x13c)
> [    1.374256] [<c0063698>] (handle_level_irq) from [<c005ff7c>]
> (generic_handle_irq+0x2c/0x3c)
> [    1.374269] [<c005ff7c>] (generic_handle_irq) from [<c02214ec>]
> (combiner_handle_cascade_irq+0x94/0x100)
> [    1.374281] [<c02214ec>] (combiner_handle_cascade_irq) from
> [<c005ff7c>] (generic_handle_irq+0x2c/0x3c)
> [    1.374290] [<c005ff7c>] (generic_handle_irq) from [<c0060248>]
> (__handle_domain_irq+0x7c/0xec)
> [    1.374300] [<c0060248>] (__handle_domain_irq) from [<c0009434>]
> (gic_handle_irq+0x30/0x68)
>
>
> And below one page fault error when modetest is terminated,
>
> # modetest -M exynos -v -s 31@29:720x1280
> setting mode 720x1280-60Hz@XR24 on connectors 31, crtc 29
> freq: 59.99Hz
>
> [   34.831025] PAGE FAULT occurred at 0x20400000 by 11e20000.sysmmu(Page
> table base: 0x6e324000)
> [   34.838072]  Lv1 entry: 0x6e92dc01
> [   34.841489] ------------[ cut here ]------------
> [   34.846058] kernel BUG at drivers/iommu/exynos-iommu.c:364!
> [   34.851614] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> [   34.857428] Modules linked in:
> [   34.860472] CPU: 0 PID: 6 Comm: kworker/u8:0 Tainted: G        W
>    4.1.0-rc4-00574-gf8eb363-dirty #1412
> [   34.870188] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [   34.876273] Workqueue: events_unbound async_run_entry_fn
> [   34.881560] task: ee879540 ti: ee8a2000 task.ti: ee8a2000
> [   34.886946] PC is at exynos_sysmmu_irq+0x184/0x208
> [   34.891716] LR is at exynos_sysmmu_irq+0xd4/0x208
> [   34.896404] pc : [<c02880d0>]    lr : [<c0288020>]    psr: 60000193
> [   34.896404] sp : ee8a3c00  ip : 00000000  fp : eeacbc10
> [   34.907860] r10: c07c27ef  r9 : 00000204  r8 : 20400000
> [   34.913068] r7 : ee324000  r6 : ee9b3428  r5 : ee9b3410  r4 : 00000000
> [   34.919577] r3 : 6e92dc01  r2 : 6e92dc01  r1 : eea55810  r0 : eeb0a000
> [   34.926089] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
> Segment kernel
> [   34.933466] Control: 10c5387d  Table: 6dd2c04a  DAC: 00000015
> [   34.939195] Process kworker/u8:0 (pid: 6, stack limit = 0xee8a2210)
> [   34.945444] Stack: (0xee8a3c00 to 0xee8a4000)
> [   34.949788] 3c00: ee8a3c0c c0049df8 00000000 6e324000 0003d4f3
> ee9b13c0 ee84ad20 00000000
> [   34.957947] 3c20: 00000000 00000026 ee84acc0 c00608f8 60000093
> eef84580 ee84acc0 ee84ad20
> [   34.966106] 3c40: ee9b13c0 00000015 ee804450 ee8a3cd8 ee808000
> c00609f0 ee84acc0 ee84ad20
> [   34.974265] 3c60: ee807000 c0063698 00000026 c079d6f8 ee807000
> c005ff7c 0000000a c02214ec
> [   34.982424] 3c80: 00000015 00000000 00000015 00000000 00000001
> c005ff7c c0789aac c0060248
> [   34.990583] 3ca0: f002000c 00000015 c078e7ac ee8a3cd8 f0020000
> ee8a2000 00000001 c0009434
> [   34.998742] 3cc0: c052e8fc 60000013 ffffffff ee8a3d0c 00000004
> c0012ec0 eeacbd18 ee879540
> [   35.006901] 3ce0: 00000000 00000271 eeacbd18 00000000 eea55010
> c078e100 00000004 ee8a2000
> [   35.015061] 3d00: 00000001 eeacbc10 eeacbd88 ee8a3d20 c052e8f8
> c052e8fc 60000013 ffffffff
> [   35.023220] 3d20: eeacbd18 c02dab48 20000113 00000003 60000013
> eeacbd18 00000004 eeacbc60
> [   35.031379] 3d40: ffff30d5 eeacbca8 eeacbd18 00000004 60000013
> ffff30d5 c0802fe8 c02db05c
> [   35.039538] 3d60: eeacbc60 00000002 c078e100 eeacbc60 ffff30d5
> c03b0f2c c078bb40 ee8a3e28
> [   35.047697] 3d80: 00000001 ee8a3db8 eeacbc70 00000002 eeacbc70
> ee8a3db8 eeacbca8 c052e880
> [   35.055857] 3da0: eeacbc70 c052d818 00000000 00000009 00000000
> c002791c ee8a3db8 11111111
> [   35.064016] 3dc0: 11111111 ee8a3dc4 11111111 eeacbc60 00000002
> c078e100 ee8a3e28 ffff30d5
> [   35.072175] 3de0: c0802fe8 00000001 ee80b400 c03ac4cc 00000000
> eeacbc60 ee8a3e28 00000002
> [   35.080334] 3e00: eea47ac0 00000000 ee849d00 c03ac69c 00000000
> 00000001 00000001 eea47ac0
> [   35.088493] 3e20: eea47ac0 c02e9f10 00000009 60000001 eea47ac0
> 00010009 00000001 eea47ac0
> [   35.096653] 3e40: ee29149c 00000000 ee00c200 c02e5bdc 00000001
> c004425c ee00c200 00000047
> [   35.104812] 3e60: ee00c200 ee00c200 ee8a3eb4 ee8a3eb4 00000000
> c02e5c5c ee00c200 ee00c200
> [   35.112971] 3e80: 00000047 c02e564c ee00c200 00000047 ee8a3eb4
> ee80da00 00000000 c02e5704
> [   35.121130] 3ea0: eeafd800 c07c5020 eeafd84c c025383c ee849d00
> c052d110 eeafd800 c0252a48
> [   35.129289] 3ec0: ee914724 c07c5020 edcc9c40 c0252f58 edcc9c50
> c003ed48 eef84580 eea2f700
> [   35.137448] 3ee0: ee849d00 edcc9c50 ee80b400 c0037944 ee8a2000
> ee80b400 00000001 ee80b400
> [   35.145608] 3f00: ee849d18 ee80b420 ee8a2000 00000088 c07c27bc
> ee849d00 ee80b400 c0037ba0
> [   35.153767] 3f20: c078e100 ee80b570 ee849d00 00000000 ee84b540
> ee849d00 c0037b54 00000000
> [   35.161926] 3f40: 00000000 00000000 00000000 c003c8b8 ffffffff
> 00000000 ffffffff ee849d00
> [   35.170085] 3f60: 00000000 00000000 dead4ead ffffffff ffffffff
> ee8a3f74 ee8a3f74 00000000
> [   35.178245] 3f80: 00000000 dead4ead ffffffff ffffffff ee8a3f90
> ee8a3f90 ee8a3fac ee84b540
> [   35.186403] 3fa0: c003c7dc 00000000 00000000 c000f5a0 00000000
> 00000000 00000000 00000000
> [   35.194562] 3fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [   35.202722] 3fe0: 00000000 00000000 00000000 00000000 00000013
> 00000000 ffffffff ffffffff
> [   35.210894] [<c02880d0>] (exynos_sysmmu_irq) from [<c00608f8>]
> (handle_irq_event_percpu+0x78/0x134)
> [   35.219914] [<c00608f8>] (handle_irq_event_percpu) from [<c00609f0>]
> (handle_irq_event+0x3c/0x5c)
> [   35.228768] [<c00609f0>] (handle_irq_event) from [<c0063698>]
> (handle_level_irq+0xc4/0x13c)
> [   35.237101] [<c0063698>] (handle_level_irq) from [<c005ff7c>]
> (generic_handle_irq+0x2c/0x3c)
> [   35.245521] [<c005ff7c>] (generic_handle_irq) from [<c02214ec>]
> (combiner_handle_cascade_irq+0x94/0x100)
> [   35.254980] [<c02214ec>] (combiner_handle_cascade_irq) from
> [<c005ff7c>] (generic_handle_irq+0x2c/0x3c)
> [   35.264353] [<c005ff7c>] (generic_handle_irq) from [<c0060248>]
> (__handle_domain_irq+0x7c/0xec)
> [   35.273034] [<c0060248>] (__handle_domain_irq) from [<c0009434>]
> (gic_handle_irq+0x30/0x68)
> [   35.281366] [<c0009434>] (gic_handle_irq) from [<c0012ec0>]
> (__irq_svc+0x40/0x74)
>
> Could you check the iommu with atomic feature? I tested these features
> on trats2 board, and enabled one more crtc drivers - HDMI, VIDI, and FIMD.

I've tested it on Trats2 with FIMD and Odroid U3 with HDMI and I didn't 
manage to
trigger page fault issue.

I've pushed the branch I've used for testing here:
https://git.linaro.org/people/marek.szyprowski/linux-srpol.git 
v4.1-exynos-drm-iommu

> To other Exynos guys,
> could you guys check the same issues also? If you give some helps to us,
> we'd be glad.
>
> Thanks,
> Inki Dae

> > (...)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v3 (alternative) 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Inki Dae]

On 2015년 06월 12일 22:00, Marek Szyprowski wrote:
> Hello,
> 
> On 2015-06-12 14:10, Inki Dae wrote:
>> On 2015년 06월 12일 18:07, Marek Szyprowski wrote:
>>> One should not do any assumptions on the stare of the fimd hardware
>>> during driver initialization, so to properly reset fimd before enabling
>>> IOMMU, one should ensure that all power domains and clocks are really
>>> enabled. This patch adds pm_runtime and clocks management in the
>>> fimd_clear_channel() function to ensure that any access to fimd
>>> registers will be performed with clocks and power domains enabled.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>>> ---
>>> Changelog:
>>> v3 (alternative):
>>> - araranged code by moving fimd_{enable,disable}_vblank functions before
>>>    fimd_clear_channel to avoid forward declaration usage
>> Marek,
>>
>> For the iommu and atomic feature test, I merged below patches in
>> addition and tested them.
>>
>> [PATCH v2 1/3] drm/exynos: remove to call mixer_wait_for_vblank
>> [PATCH v2 2/3] drm/exynos: remove chained calls to enable
>> [PATCH v2 3/3] drm/exynos: initialize VIDCON0 when fimd is disabled
>>
>>
>> However, I see below one warning log,
>>
>> [    1.227115] [drm] Initialized drm 1.1.0 20060810
>> [    1.235852] exynos-drm exynos-drm: bound exynos-drm-vidi (ops
>> vidi_component_ops)
>> [    1.253548] ------------[ cut here ]------------
>> [    1.256700] WARNING: CPU: 0 PID: 0 at drivers/gpu/drm/drm_irq.c:1718
>> drm_handle_vblank+0x2a0/0x308()
>> [    1.265800] Modules linked in:
>> [    1.268844] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
>> 4.1.0-rc4-00574-gf8eb363-dirty #1412
>> [    1.277085] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [    1.283184] [<c0015464>] (unwind_backtrace) from [<c00123c0>]
>> (show_stack+0x10/0x14)
>> [    1.288540] exynos-drm exynos-drm: bound 11c00000.fimd (ops
>> fimd_component_ops)
>> [    1.288883] exynos-drm exynos-drm: bound 11c80000.dsi (ops
>> exynos_dsi_component_ops)
>> [    1.288888] [drm] Supports vblank timestamp caching Rev 2
>> (21.10.2013).
>> [    1.288891] [drm] No driver support for vblank timestamp query.
>> [    1.288932] [drm] Initialized exynos 1.0.0 20110530 on minor 0
>> [    1.324248] [<c00123c0>] (show_stack) from [<c0528f74>]
>> (dump_stack+0x84/0xc4)
>> [    1.331437] [<c0528f74>] (dump_stack) from [<c00246ac>]
>> (warn_slowpath_common+0x80/0xb0)
>> [    1.339504] [<c00246ac>] (warn_slowpath_common) from [<c0024778>]
>> (warn_slowpath_null+0x1c/0x24)
>> [    1.348270] [<c0024778>] (warn_slowpath_null) from [<c029c898>]
>> (drm_handle_vblank+0x2a0/0x308)
>> [    1.356965] [<c029c898>] (drm_handle_vblank) from [<c02b8c80>]
>> (fimd_irq_handler+0x78/0xd0)
>> [    1.365292] [<c02b8c80>] (fimd_irq_handler) from [<c00608f8>]
>> (handle_irq_event_percpu+0x78/0x134)
>> [    1.374231] [<c00608f8>] (handle_irq_event_percpu) from [<c00609f0>]
>> (handle_irq_event+0x3c/0x5c)
>> [    1.374244] [<c00609f0>] (handle_irq_event) from [<c0063698>]
>> (handle_level_irq+0xc4/0x13c)
>> [    1.374256] [<c0063698>] (handle_level_irq) from [<c005ff7c>]
>> (generic_handle_irq+0x2c/0x3c)
>> [    1.374269] [<c005ff7c>] (generic_handle_irq) from [<c02214ec>]
>> (combiner_handle_cascade_irq+0x94/0x100)
>> [    1.374281] [<c02214ec>] (combiner_handle_cascade_irq) from
>> [<c005ff7c>] (generic_handle_irq+0x2c/0x3c)
>> [    1.374290] [<c005ff7c>] (generic_handle_irq) from [<c0060248>]
>> (__handle_domain_irq+0x7c/0xec)
>> [    1.374300] [<c0060248>] (__handle_domain_irq) from [<c0009434>]
>> (gic_handle_irq+0x30/0x68)
>>
>>
>> And below one page fault error when modetest is terminated,
>>
>> # modetest -M exynos -v -s 31@29:720x1280
>> setting mode 720x1280-60Hz@XR24 on connectors 31, crtc 29
>> freq: 59.99Hz
>>
>> [   34.831025] PAGE FAULT occurred at 0x20400000 by 11e20000.sysmmu(Page
>> table base: 0x6e324000)
>> [   34.838072]  Lv1 entry: 0x6e92dc01
>> [   34.841489] ------------[ cut here ]------------
>> [   34.846058] kernel BUG at drivers/iommu/exynos-iommu.c:364!
>> [   34.851614] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
>> [   34.857428] Modules linked in:
>> [   34.860472] CPU: 0 PID: 6 Comm: kworker/u8:0 Tainted: G        W
>>    4.1.0-rc4-00574-gf8eb363-dirty #1412
>> [   34.870188] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [   34.876273] Workqueue: events_unbound async_run_entry_fn
>> [   34.881560] task: ee879540 ti: ee8a2000 task.ti: ee8a2000
>> [   34.886946] PC is at exynos_sysmmu_irq+0x184/0x208
>> [   34.891716] LR is at exynos_sysmmu_irq+0xd4/0x208
>> [   34.896404] pc : [<c02880d0>]    lr : [<c0288020>]    psr: 60000193
>> [   34.896404] sp : ee8a3c00  ip : 00000000  fp : eeacbc10
>> [   34.907860] r10: c07c27ef  r9 : 00000204  r8 : 20400000
>> [   34.913068] r7 : ee324000  r6 : ee9b3428  r5 : ee9b3410  r4 : 00000000
>> [   34.919577] r3 : 6e92dc01  r2 : 6e92dc01  r1 : eea55810  r0 : eeb0a000
>> [   34.926089] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
>> Segment kernel
>> [   34.933466] Control: 10c5387d  Table: 6dd2c04a  DAC: 00000015
>> [   34.939195] Process kworker/u8:0 (pid: 6, stack limit = 0xee8a2210)
>> [   34.945444] Stack: (0xee8a3c00 to 0xee8a4000)
>> [   34.949788] 3c00: ee8a3c0c c0049df8 00000000 6e324000 0003d4f3
>> ee9b13c0 ee84ad20 00000000
>> [   34.957947] 3c20: 00000000 00000026 ee84acc0 c00608f8 60000093
>> eef84580 ee84acc0 ee84ad20
>> [   34.966106] 3c40: ee9b13c0 00000015 ee804450 ee8a3cd8 ee808000
>> c00609f0 ee84acc0 ee84ad20
>> [   34.974265] 3c60: ee807000 c0063698 00000026 c079d6f8 ee807000
>> c005ff7c 0000000a c02214ec
>> [   34.982424] 3c80: 00000015 00000000 00000015 00000000 00000001
>> c005ff7c c0789aac c0060248
>> [   34.990583] 3ca0: f002000c 00000015 c078e7ac ee8a3cd8 f0020000
>> ee8a2000 00000001 c0009434
>> [   34.998742] 3cc0: c052e8fc 60000013 ffffffff ee8a3d0c 00000004
>> c0012ec0 eeacbd18 ee879540
>> [   35.006901] 3ce0: 00000000 00000271 eeacbd18 00000000 eea55010
>> c078e100 00000004 ee8a2000
>> [   35.015061] 3d00: 00000001 eeacbc10 eeacbd88 ee8a3d20 c052e8f8
>> c052e8fc 60000013 ffffffff
>> [   35.023220] 3d20: eeacbd18 c02dab48 20000113 00000003 60000013
>> eeacbd18 00000004 eeacbc60
>> [   35.031379] 3d40: ffff30d5 eeacbca8 eeacbd18 00000004 60000013
>> ffff30d5 c0802fe8 c02db05c
>> [   35.039538] 3d60: eeacbc60 00000002 c078e100 eeacbc60 ffff30d5
>> c03b0f2c c078bb40 ee8a3e28
>> [   35.047697] 3d80: 00000001 ee8a3db8 eeacbc70 00000002 eeacbc70
>> ee8a3db8 eeacbca8 c052e880
>> [   35.055857] 3da0: eeacbc70 c052d818 00000000 00000009 00000000
>> c002791c ee8a3db8 11111111
>> [   35.064016] 3dc0: 11111111 ee8a3dc4 11111111 eeacbc60 00000002
>> c078e100 ee8a3e28 ffff30d5
>> [   35.072175] 3de0: c0802fe8 00000001 ee80b400 c03ac4cc 00000000
>> eeacbc60 ee8a3e28 00000002
>> [   35.080334] 3e00: eea47ac0 00000000 ee849d00 c03ac69c 00000000
>> 00000001 00000001 eea47ac0
>> [   35.088493] 3e20: eea47ac0 c02e9f10 00000009 60000001 eea47ac0
>> 00010009 00000001 eea47ac0
>> [   35.096653] 3e40: ee29149c 00000000 ee00c200 c02e5bdc 00000001
>> c004425c ee00c200 00000047
>> [   35.104812] 3e60: ee00c200 ee00c200 ee8a3eb4 ee8a3eb4 00000000
>> c02e5c5c ee00c200 ee00c200
>> [   35.112971] 3e80: 00000047 c02e564c ee00c200 00000047 ee8a3eb4
>> ee80da00 00000000 c02e5704
>> [   35.121130] 3ea0: eeafd800 c07c5020 eeafd84c c025383c ee849d00
>> c052d110 eeafd800 c0252a48
>> [   35.129289] 3ec0: ee914724 c07c5020 edcc9c40 c0252f58 edcc9c50
>> c003ed48 eef84580 eea2f700
>> [   35.137448] 3ee0: ee849d00 edcc9c50 ee80b400 c0037944 ee8a2000
>> ee80b400 00000001 ee80b400
>> [   35.145608] 3f00: ee849d18 ee80b420 ee8a2000 00000088 c07c27bc
>> ee849d00 ee80b400 c0037ba0
>> [   35.153767] 3f20: c078e100 ee80b570 ee849d00 00000000 ee84b540
>> ee849d00 c0037b54 00000000
>> [   35.161926] 3f40: 00000000 00000000 00000000 c003c8b8 ffffffff
>> 00000000 ffffffff ee849d00
>> [   35.170085] 3f60: 00000000 00000000 dead4ead ffffffff ffffffff
>> ee8a3f74 ee8a3f74 00000000
>> [   35.178245] 3f80: 00000000 dead4ead ffffffff ffffffff ee8a3f90
>> ee8a3f90 ee8a3fac ee84b540
>> [   35.186403] 3fa0: c003c7dc 00000000 00000000 c000f5a0 00000000
>> 00000000 00000000 00000000
>> [   35.194562] 3fc0: 00000000 00000000 00000000 00000000 00000000
>> 00000000 00000000 00000000
>> [   35.202722] 3fe0: 00000000 00000000 00000000 00000000 00000013
>> 00000000 ffffffff ffffffff
>> [   35.210894] [<c02880d0>] (exynos_sysmmu_irq) from [<c00608f8>]
>> (handle_irq_event_percpu+0x78/0x134)
>> [   35.219914] [<c00608f8>] (handle_irq_event_percpu) from [<c00609f0>]
>> (handle_irq_event+0x3c/0x5c)
>> [   35.228768] [<c00609f0>] (handle_irq_event) from [<c0063698>]
>> (handle_level_irq+0xc4/0x13c)
>> [   35.237101] [<c0063698>] (handle_level_irq) from [<c005ff7c>]
>> (generic_handle_irq+0x2c/0x3c)
>> [   35.245521] [<c005ff7c>] (generic_handle_irq) from [<c02214ec>]
>> (combiner_handle_cascade_irq+0x94/0x100)
>> [   35.254980] [<c02214ec>] (combiner_handle_cascade_irq) from
>> [<c005ff7c>] (generic_handle_irq+0x2c/0x3c)
>> [   35.264353] [<c005ff7c>] (generic_handle_irq) from [<c0060248>]
>> (__handle_domain_irq+0x7c/0xec)
>> [   35.273034] [<c0060248>] (__handle_domain_irq) from [<c0009434>]
>> (gic_handle_irq+0x30/0x68)
>> [   35.281366] [<c0009434>] (gic_handle_irq) from [<c0012ec0>]
>> (__irq_svc+0x40/0x74)
>>
>> Could you check the iommu with atomic feature? I tested these features
>> on trats2 board, and enabled one more crtc drivers - HDMI, VIDI, and
>> FIMD.
> 
> I've tested it on Trats2 with FIMD and Odroid U3 with HDMI and I didn't
> manage to
> trigger page fault issue.
> 
> I've pushed the branch I've used for testing here:
> https://git.linaro.org/people/marek.szyprowski/linux-srpol.git
> v4.1-exynos-drm-iommu

Hm.. I've also tested it on Trarts2 and above your git tree but I still
see the page fault issue. Please run modetest app and terminate it
repeatedly.

Thanks,
Inki Dae

> 
>> To other Exynos guys,
>> could you guys check the same issues also? If you give some helps to us,
>> we'd be glad.
>>
>> Thanks,
>> Inki Dae
> 
>> > (...)
> 
> Best regards

[PATCH v4] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Marek Szyprowski]

One should not do any assumptions on the stare of the fimd hardware
during driver initialization, so to properly reset fimd before enabling
IOMMU, one should ensure that all power domains and clocks are really
enabled. This patch adds pm_runtime and clocks management in the
fimd_clear_channel() function to ensure that any access to fimd
registers will be performed with clocks and power domains enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
Changelog:
v4:
- added temporary changing of ctx->pipe to -1 to avoid propagating vblank
  to drm core

v3 (alternative):
- araranged code by moving fimd_{enable,disable}_vblank functions before
  fimd_clear_channel to avoid forward declaration usage

v2:
- rebased onto latest exynos-drm-next branch with atomic mode setting
  patches applied.
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 135 ++++++++++++++++++-------------
 1 file changed, 77 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 96618534358e..f35a6f197f58 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -196,6 +196,62 @@ static inline struct fimd_driver_data *drm_fimd_get_driver_data(
 	return (struct fimd_driver_data *)of_id->data;
 }
 
+static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
+{
+	struct fimd_context *ctx = crtc->ctx;
+	u32 val;
+
+	if (ctx->suspended)
+		return -EPERM;
+
+	if (!test_and_set_bit(0, &ctx->irq_flags)) {
+		val = readl(ctx->regs + VIDINTCON0);
+
+		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;
+
+			val &= ~VIDINTCON0_FRAMESEL0_MASK;
+			val |= VIDINTCON0_FRAMESEL0_VSYNC;
+			val &= ~VIDINTCON0_FRAMESEL1_MASK;
+			val |= VIDINTCON0_FRAMESEL1_NONE;
+		}
+
+		writel(val, ctx->regs + VIDINTCON0);
+	}
+
+	return 0;
+}
+
+static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
+{
+	struct fimd_context *ctx = crtc->ctx;
+	u32 val;
+
+	if (ctx->suspended)
+		return;
+
+	if (test_and_clear_bit(0, &ctx->irq_flags)) {
+		val = readl(ctx->regs + VIDINTCON0);
+
+		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);
+	}
+}
+
 static void fimd_wait_for_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct fimd_context *ctx = crtc->ctx;
@@ -248,6 +304,12 @@ static void fimd_clear_channel(struct fimd_context *ctx)
 
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
+	/* Hardware is in unknown state, so ensure it gets enabled properly */
+	pm_runtime_get_sync(ctx->dev);
+
+	clk_prepare_enable(ctx->bus_clk);
+	clk_prepare_enable(ctx->lcd_clk);
+
 	/* Check if any channel is enabled. */
 	for (win = 0; win < WINDOWS_NR; win++) {
 		u32 val = readl(ctx->regs + WINCON(win));
@@ -265,12 +327,24 @@ static void fimd_clear_channel(struct fimd_context *ctx)
 
 	/* Wait for vsync, as disable channel takes effect at next vsync */
 	if (ch_enabled) {
-		unsigned int state = ctx->suspended;
+		int pipe = ctx->pipe;
+
+		/* ensure that vblank interrupt won't be reported to core */
+		ctx->suspended = false;
+		ctx->pipe = -1;
 
-		ctx->suspended = 0;
+		fimd_enable_vblank(ctx->crtc);
 		fimd_wait_for_vblank(ctx->crtc);
-		ctx->suspended = state;
+		fimd_disable_vblank(ctx->crtc);
+
+		ctx->suspended = true;
+		ctx->pipe = pipe;
 	}
+
+	clk_disable_unprepare(ctx->lcd_clk);
+	clk_disable_unprepare(ctx->bus_clk);
+
+	pm_runtime_put(ctx->dev);
 }
 
 static int fimd_iommu_attach_devices(struct fimd_context *ctx,
@@ -434,61 +508,6 @@ static void fimd_commit(struct exynos_drm_crtc *crtc)
 	writel(val, ctx->regs + VIDCON0);
 }
 
-static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
-{
-	struct fimd_context *ctx = crtc->ctx;
-	u32 val;
-
-	if (ctx->suspended)
-		return -EPERM;
-
-	if (!test_and_set_bit(0, &ctx->irq_flags)) {
-		val = readl(ctx->regs + VIDINTCON0);
-
-		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;
-
-			val &= ~VIDINTCON0_FRAMESEL0_MASK;
-			val |= VIDINTCON0_FRAMESEL0_VSYNC;
-			val &= ~VIDINTCON0_FRAMESEL1_MASK;
-			val |= VIDINTCON0_FRAMESEL1_NONE;
-		}
-
-		writel(val, ctx->regs + VIDINTCON0);
-	}
-
-	return 0;
-}
-
-static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
-{
-	struct fimd_context *ctx = crtc->ctx;
-	u32 val;
-
-	if (ctx->suspended)
-		return;
-
-	if (test_and_clear_bit(0, &ctx->irq_flags)) {
-		val = readl(ctx->regs + VIDINTCON0);
-
-		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);
-	}
-}
 
 static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win)
 {
-- 
1.9.2

Re: [PATCH v3 (alternative) 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Inki Dae]

On 2015년 06월 12일 21:10, Inki Dae wrote:
> On 2015년 06월 12일 18:07, Marek Szyprowski wrote:
>> One should not do any assumptions on the stare of the fimd hardware
>> during driver initialization, so to properly reset fimd before enabling
>> IOMMU, one should ensure that all power domains and clocks are really
>> enabled. This patch adds pm_runtime and clocks management in the
>> fimd_clear_channel() function to ensure that any access to fimd
>> registers will be performed with clocks and power domains enabled.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>> Changelog:
>> v3 (alternative):
>> - araranged code by moving fimd_{enable,disable}_vblank functions before
>>   fimd_clear_channel to avoid forward declaration usage
> 
> Marek,
> 
> For the iommu and atomic feature test, I merged below patches in
> addition and tested them.
> 
> [PATCH v2 1/3] drm/exynos: remove to call mixer_wait_for_vblank
> [PATCH v2 2/3] drm/exynos: remove chained calls to enable
> [PATCH v2 3/3] drm/exynos: initialize VIDCON0 when fimd is disabled
> 
> 
> However, I see below one warning log,
> 
> [    1.227115] [drm] Initialized drm 1.1.0 20060810
> [    1.235852] exynos-drm exynos-drm: bound exynos-drm-vidi (ops
> vidi_component_ops)
> [    1.253548] ------------[ cut here ]------------
> [    1.256700] WARNING: CPU: 0 PID: 0 at drivers/gpu/drm/drm_irq.c:1718
> drm_handle_vblank+0x2a0/0x308()
> [    1.265800] Modules linked in:
> [    1.268844] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 4.1.0-rc4-00574-gf8eb363-dirty #1412
> [    1.277085] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [    1.283184] [<c0015464>] (unwind_backtrace) from [<c00123c0>]
> (show_stack+0x10/0x14)
> [    1.288540] exynos-drm exynos-drm: bound 11c00000.fimd (ops
> fimd_component_ops)
> [    1.288883] exynos-drm exynos-drm: bound 11c80000.dsi (ops
> exynos_dsi_component_ops)
> [    1.288888] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [    1.288891] [drm] No driver support for vblank timestamp query.
> [    1.288932] [drm] Initialized exynos 1.0.0 20110530 on minor 0
> [    1.324248] [<c00123c0>] (show_stack) from [<c0528f74>]
> (dump_stack+0x84/0xc4)
> [    1.331437] [<c0528f74>] (dump_stack) from [<c00246ac>]
> (warn_slowpath_common+0x80/0xb0)
> [    1.339504] [<c00246ac>] (warn_slowpath_common) from [<c0024778>]
> (warn_slowpath_null+0x1c/0x24)
> [    1.348270] [<c0024778>] (warn_slowpath_null) from [<c029c898>]
> (drm_handle_vblank+0x2a0/0x308)
> [    1.356965] [<c029c898>] (drm_handle_vblank) from [<c02b8c80>]
> (fimd_irq_handler+0x78/0xd0)
> [    1.365292] [<c02b8c80>] (fimd_irq_handler) from [<c00608f8>]
> (handle_irq_event_percpu+0x78/0x134)
> [    1.374231] [<c00608f8>] (handle_irq_event_percpu) from [<c00609f0>]
> (handle_irq_event+0x3c/0x5c)
> [    1.374244] [<c00609f0>] (handle_irq_event) from [<c0063698>]
> (handle_level_irq+0xc4/0x13c)
> [    1.374256] [<c0063698>] (handle_level_irq) from [<c005ff7c>]
> (generic_handle_irq+0x2c/0x3c)
> [    1.374269] [<c005ff7c>] (generic_handle_irq) from [<c02214ec>]
> (combiner_handle_cascade_irq+0x94/0x100)
> [    1.374281] [<c02214ec>] (combiner_handle_cascade_irq) from
> [<c005ff7c>] (generic_handle_irq+0x2c/0x3c)
> [    1.374290] [<c005ff7c>] (generic_handle_irq) from [<c0060248>]
> (__handle_domain_irq+0x7c/0xec)
> [    1.374300] [<c0060248>] (__handle_domain_irq) from [<c0009434>]
> (gic_handle_irq+0x30/0x68)
> 
> 
> And below one page fault error when modetest is terminated,
> 
> # modetest -M exynos -v -s 31@29:720x1280
> setting mode 720x1280-60Hz@XR24 on connectors 31, crtc 29
> freq: 59.99Hz
> 
> [   34.831025] PAGE FAULT occurred at 0x20400000 by 11e20000.sysmmu(Page
> table base: 0x6e324000)
> [   34.838072]  Lv1 entry: 0x6e92dc01
> [   34.841489] ------------[ cut here ]------------
> [   34.846058] kernel BUG at drivers/iommu/exynos-iommu.c:364!
> [   34.851614] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> [   34.857428] Modules linked in:
> [   34.860472] CPU: 0 PID: 6 Comm: kworker/u8:0 Tainted: G        W
>   4.1.0-rc4-00574-gf8eb363-dirty #1412
> [   34.870188] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [   34.876273] Workqueue: events_unbound async_run_entry_fn
> [   34.881560] task: ee879540 ti: ee8a2000 task.ti: ee8a2000
> [   34.886946] PC is at exynos_sysmmu_irq+0x184/0x208
> [   34.891716] LR is at exynos_sysmmu_irq+0xd4/0x208
> [   34.896404] pc : [<c02880d0>]    lr : [<c0288020>]    psr: 60000193
> [   34.896404] sp : ee8a3c00  ip : 00000000  fp : eeacbc10
> [   34.907860] r10: c07c27ef  r9 : 00000204  r8 : 20400000
> [   34.913068] r7 : ee324000  r6 : ee9b3428  r5 : ee9b3410  r4 : 00000000
> [   34.919577] r3 : 6e92dc01  r2 : 6e92dc01  r1 : eea55810  r0 : eeb0a000
> [   34.926089] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
> Segment kernel
> [   34.933466] Control: 10c5387d  Table: 6dd2c04a  DAC: 00000015
> [   34.939195] Process kworker/u8:0 (pid: 6, stack limit = 0xee8a2210)
> [   34.945444] Stack: (0xee8a3c00 to 0xee8a4000)
> [   34.949788] 3c00: ee8a3c0c c0049df8 00000000 6e324000 0003d4f3
> ee9b13c0 ee84ad20 00000000
> [   34.957947] 3c20: 00000000 00000026 ee84acc0 c00608f8 60000093
> eef84580 ee84acc0 ee84ad20
> [   34.966106] 3c40: ee9b13c0 00000015 ee804450 ee8a3cd8 ee808000
> c00609f0 ee84acc0 ee84ad20
> [   34.974265] 3c60: ee807000 c0063698 00000026 c079d6f8 ee807000
> c005ff7c 0000000a c02214ec
> [   34.982424] 3c80: 00000015 00000000 00000015 00000000 00000001
> c005ff7c c0789aac c0060248
> [   34.990583] 3ca0: f002000c 00000015 c078e7ac ee8a3cd8 f0020000
> ee8a2000 00000001 c0009434
> [   34.998742] 3cc0: c052e8fc 60000013 ffffffff ee8a3d0c 00000004
> c0012ec0 eeacbd18 ee879540
> [   35.006901] 3ce0: 00000000 00000271 eeacbd18 00000000 eea55010
> c078e100 00000004 ee8a2000
> [   35.015061] 3d00: 00000001 eeacbc10 eeacbd88 ee8a3d20 c052e8f8
> c052e8fc 60000013 ffffffff
> [   35.023220] 3d20: eeacbd18 c02dab48 20000113 00000003 60000013
> eeacbd18 00000004 eeacbc60
> [   35.031379] 3d40: ffff30d5 eeacbca8 eeacbd18 00000004 60000013
> ffff30d5 c0802fe8 c02db05c
> [   35.039538] 3d60: eeacbc60 00000002 c078e100 eeacbc60 ffff30d5
> c03b0f2c c078bb40 ee8a3e28
> [   35.047697] 3d80: 00000001 ee8a3db8 eeacbc70 00000002 eeacbc70
> ee8a3db8 eeacbca8 c052e880
> [   35.055857] 3da0: eeacbc70 c052d818 00000000 00000009 00000000
> c002791c ee8a3db8 11111111
> [   35.064016] 3dc0: 11111111 ee8a3dc4 11111111 eeacbc60 00000002
> c078e100 ee8a3e28 ffff30d5
> [   35.072175] 3de0: c0802fe8 00000001 ee80b400 c03ac4cc 00000000
> eeacbc60 ee8a3e28 00000002
> [   35.080334] 3e00: eea47ac0 00000000 ee849d00 c03ac69c 00000000
> 00000001 00000001 eea47ac0
> [   35.088493] 3e20: eea47ac0 c02e9f10 00000009 60000001 eea47ac0
> 00010009 00000001 eea47ac0
> [   35.096653] 3e40: ee29149c 00000000 ee00c200 c02e5bdc 00000001
> c004425c ee00c200 00000047
> [   35.104812] 3e60: ee00c200 ee00c200 ee8a3eb4 ee8a3eb4 00000000
> c02e5c5c ee00c200 ee00c200
> [   35.112971] 3e80: 00000047 c02e564c ee00c200 00000047 ee8a3eb4
> ee80da00 00000000 c02e5704
> [   35.121130] 3ea0: eeafd800 c07c5020 eeafd84c c025383c ee849d00
> c052d110 eeafd800 c0252a48
> [   35.129289] 3ec0: ee914724 c07c5020 edcc9c40 c0252f58 edcc9c50
> c003ed48 eef84580 eea2f700
> [   35.137448] 3ee0: ee849d00 edcc9c50 ee80b400 c0037944 ee8a2000
> ee80b400 00000001 ee80b400
> [   35.145608] 3f00: ee849d18 ee80b420 ee8a2000 00000088 c07c27bc
> ee849d00 ee80b400 c0037ba0
> [   35.153767] 3f20: c078e100 ee80b570 ee849d00 00000000 ee84b540
> ee849d00 c0037b54 00000000
> [   35.161926] 3f40: 00000000 00000000 00000000 c003c8b8 ffffffff
> 00000000 ffffffff ee849d00
> [   35.170085] 3f60: 00000000 00000000 dead4ead ffffffff ffffffff
> ee8a3f74 ee8a3f74 00000000
> [   35.178245] 3f80: 00000000 dead4ead ffffffff ffffffff ee8a3f90
> ee8a3f90 ee8a3fac ee84b540
> [   35.186403] 3fa0: c003c7dc 00000000 00000000 c000f5a0 00000000
> 00000000 00000000 00000000
> [   35.194562] 3fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [   35.202722] 3fe0: 00000000 00000000 00000000 00000000 00000013
> 00000000 ffffffff ffffffff
> [   35.210894] [<c02880d0>] (exynos_sysmmu_irq) from [<c00608f8>]
> (handle_irq_event_percpu+0x78/0x134)
> [   35.219914] [<c00608f8>] (handle_irq_event_percpu) from [<c00609f0>]
> (handle_irq_event+0x3c/0x5c)
> [   35.228768] [<c00609f0>] (handle_irq_event) from [<c0063698>]
> (handle_level_irq+0xc4/0x13c)
> [   35.237101] [<c0063698>] (handle_level_irq) from [<c005ff7c>]
> (generic_handle_irq+0x2c/0x3c)
> [   35.245521] [<c005ff7c>] (generic_handle_irq) from [<c02214ec>]
> (combiner_handle_cascade_irq+0x94/0x100)
> [   35.254980] [<c02214ec>] (combiner_handle_cascade_irq) from
> [<c005ff7c>] (generic_handle_irq+0x2c/0x3c)
> [   35.264353] [<c005ff7c>] (generic_handle_irq) from [<c0060248>]
> (__handle_domain_irq+0x7c/0xec)
> [   35.273034] [<c0060248>] (__handle_domain_irq) from [<c0009434>]
> (gic_handle_irq+0x30/0x68)
> [   35.281366] [<c0009434>] (gic_handle_irq) from [<c0012ec0>]
> (__irq_svc+0x40/0x74)
> 
> Could you check the iommu with atomic feature? I tested these features
> on trats2 board, and enabled one more crtc drivers - HDMI, VIDI, and FIMD.
> 
> To other Exynos guys,
> could you guys check the same issues also? If you give some helps to us,
> we'd be glad.

I resolved above page fault issue with below patch:
	[PATCH] drm/exynos: fimd: fix page fault issue with iommu

However, I still see above warning log. The problem is because
dev->num_crtcs is 0 when drm_handle_vblank function is called by fimd
irq handler.

So I added one line for num_crtcs to be updated at end of
exynos_drm_crtc_create function like below,
	drm_dev->num_crtcs = drm_dev->mode_config.num_crtc;

However, with this, kernel booting is halted. See the below booting message,

[    1.226408] [drm] Initialized drm 1.1.0 20060810
[    1.235144] exynos-drm exynos-drm: bound exynos-drm-vidi (ops
vidi_component_ops)
[    1.250154] BUG: spinlock bad magic on CPU#0, swapper/0/0
[    1.254079]  lock: 0xee188128, .magic: 00000000, .owner: <none>/-1,
.owner_cpu: 0
[    1.261544] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.1.0-rc4-00575-gfedbaf7-dirty #1433
[    1.269783] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    1.275884] [<c0015464>] (unwind_backtrace) from [<c00123c0>]
(show_stack+0x10/0x14)
[    1.283595] [<c00123c0>] (show_stack) from [<c0528f94>]
(dump_stack+0x84/0xc4)
[    1.288136] exynos-sysmmu 11e20000.sysmmu: Disabled
[    1.288161] exynos4-fb 11c00000.fimd: exynos_iommu_detach_device:
Detached IOMMU with pgtable 0x6ea50000
[    1.288180] exynos-sysmmu 11e20000.sysmmu: Enabled
[    1.288187] exynos4-fb 11c00000.fimd: exynos_iommu_attach_device:
Attached IOMMU with pgtable 0x6e3b4000
[    1.288201] exynos-drm exynos-drm: bound 11c00000.fimd (ops
fimd_component_ops)
[    1.288538] exynos-drm exynos-drm: bound 11c80000.dsi (ops
exynos_dsi_component_ops)
[    1.288543] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[    1.288547] [drm] No driver support for vblank timestamp query.
[    1.288588] [drm] Initialized exynos 1.0.0 20110530 on minor 0
[    1.289957] panel_s6e8aa0 11c80000.dsi.0: GPIO lookup for consumer reset
[    1.289963] panel_s6e8aa0 11c80000.dsi.0: using device tree for GPIO
lookup
[    1.290003] of_get_named_gpiod_flags: parsed 'reset-gpios' property
of node '/dsi@11C80000/panel@0[0]' - status (0)

We would need to resolve this issue somehow.

Thanks,
Inki Dae

> 
> Thanks,
> Inki Dae
> 
>>
>> v2:
>> - rebased onto latest exynos-drm-next branch with atomic mode setting
>>   patches applied.
>>
>>
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 130 +++++++++++++++++--------------
>>  1 file changed, 71 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 96618534358e..f490895a8b02 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -196,6 +196,62 @@ static inline struct fimd_driver_data *drm_fimd_get_driver_data(
>>  	return (struct fimd_driver_data *)of_id->data;
>>  }
>>  
>> +static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
>> +{
>> +	struct fimd_context *ctx = crtc->ctx;
>> +	u32 val;
>> +
>> +	if (ctx->suspended)
>> +		return -EPERM;
>> +
>> +	if (!test_and_set_bit(0, &ctx->irq_flags)) {
>> +		val = readl(ctx->regs + VIDINTCON0);
>> +
>> +		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;
>> +
>> +			val &= ~VIDINTCON0_FRAMESEL0_MASK;
>> +			val |= VIDINTCON0_FRAMESEL0_VSYNC;
>> +			val &= ~VIDINTCON0_FRAMESEL1_MASK;
>> +			val |= VIDINTCON0_FRAMESEL1_NONE;
>> +		}
>> +
>> +		writel(val, ctx->regs + VIDINTCON0);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
>> +{
>> +	struct fimd_context *ctx = crtc->ctx;
>> +	u32 val;
>> +
>> +	if (ctx->suspended)
>> +		return;
>> +
>> +	if (test_and_clear_bit(0, &ctx->irq_flags)) {
>> +		val = readl(ctx->regs + VIDINTCON0);
>> +
>> +		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);
>> +	}
>> +}
>> +
>>  static void fimd_wait_for_vblank(struct exynos_drm_crtc *crtc)
>>  {
>>  	struct fimd_context *ctx = crtc->ctx;
>> @@ -248,6 +304,12 @@ static void fimd_clear_channel(struct fimd_context *ctx)
>>  
>>  	DRM_DEBUG_KMS("%s\n", __FILE__);
>>  
>> +	/* Hardware is in unknown state, so ensure it gets enabled properly */
>> +	pm_runtime_get_sync(ctx->dev);
>> +
>> +	clk_prepare_enable(ctx->bus_clk);
>> +	clk_prepare_enable(ctx->lcd_clk);
>> +
>>  	/* Check if any channel is enabled. */
>>  	for (win = 0; win < WINDOWS_NR; win++) {
>>  		u32 val = readl(ctx->regs + WINCON(win));
>> @@ -265,12 +327,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
>>  
>>  	/* Wait for vsync, as disable channel takes effect at next vsync */
>>  	if (ch_enabled) {
>> -		unsigned int state = ctx->suspended;
>> -
>> -		ctx->suspended = 0;
>> +		ctx->suspended = false;
>> +		fimd_enable_vblank(ctx->crtc);
>>  		fimd_wait_for_vblank(ctx->crtc);
>> -		ctx->suspended = state;
>> +		fimd_disable_vblank(ctx->crtc);
>> +		ctx->suspended = true;
>>  	}
>> +
>> +	clk_disable_unprepare(ctx->lcd_clk);
>> +	clk_disable_unprepare(ctx->bus_clk);
>> +
>> +	pm_runtime_put(ctx->dev);
>>  }
>>  
>>  static int fimd_iommu_attach_devices(struct fimd_context *ctx,
>> @@ -434,61 +501,6 @@ static void fimd_commit(struct exynos_drm_crtc *crtc)
>>  	writel(val, ctx->regs + VIDCON0);
>>  }
>>  
>> -static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
>> -{
>> -	struct fimd_context *ctx = crtc->ctx;
>> -	u32 val;
>> -
>> -	if (ctx->suspended)
>> -		return -EPERM;
>> -
>> -	if (!test_and_set_bit(0, &ctx->irq_flags)) {
>> -		val = readl(ctx->regs + VIDINTCON0);
>> -
>> -		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;
>> -
>> -			val &= ~VIDINTCON0_FRAMESEL0_MASK;
>> -			val |= VIDINTCON0_FRAMESEL0_VSYNC;
>> -			val &= ~VIDINTCON0_FRAMESEL1_MASK;
>> -			val |= VIDINTCON0_FRAMESEL1_NONE;
>> -		}
>> -
>> -		writel(val, ctx->regs + VIDINTCON0);
>> -	}
>> -
>> -	return 0;
>> -}
>> -
>> -static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
>> -{
>> -	struct fimd_context *ctx = crtc->ctx;
>> -	u32 val;
>> -
>> -	if (ctx->suspended)
>> -		return;
>> -
>> -	if (test_and_clear_bit(0, &ctx->irq_flags)) {
>> -		val = readl(ctx->regs + VIDINTCON0);
>> -
>> -		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);
>> -	}
>> -}
>>  
>>  static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win)
>>  {
>>
> 

Re: [PATCH v3 (alternative) 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Marek Szyprowski]

Hello,

On 2015-06-12 15:51, Inki Dae wrote:
> On 2015년 06월 12일 21:10, Inki Dae wrote:
>> On 2015년 06월 12일 18:07, Marek Szyprowski wrote:
>>> One should not do any assumptions on the stare of the fimd hardware
>>> during driver initialization, so to properly reset fimd before enabling
>>> IOMMU, one should ensure that all power domains and clocks are really
>>> enabled. This patch adds pm_runtime and clocks management in the
>>> fimd_clear_channel() function to ensure that any access to fimd
>>> registers will be performed with clocks and power domains enabled.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>>> ---
>>> Changelog:
>>> v3 (alternative):
>>> - araranged code by moving fimd_{enable,disable}_vblank functions before
>>>    fimd_clear_channel to avoid forward declaration usage
>> Marek,
>>
>> For the iommu and atomic feature test, I merged below patches in
>> addition and tested them.
>>
>> [PATCH v2 1/3] drm/exynos: remove to call mixer_wait_for_vblank
>> [PATCH v2 2/3] drm/exynos: remove chained calls to enable
>> [PATCH v2 3/3] drm/exynos: initialize VIDCON0 when fimd is disabled
>>
>>
>> However, I see below one warning log,
>>
>> [    1.227115] [drm] Initialized drm 1.1.0 20060810
>> [    1.235852] exynos-drm exynos-drm: bound exynos-drm-vidi (ops
>> vidi_component_ops)
>> [    1.253548] ------------[ cut here ]------------
>> [    1.256700] WARNING: CPU: 0 PID: 0 at drivers/gpu/drm/drm_irq.c:1718
>> drm_handle_vblank+0x2a0/0x308()
>> [    1.265800] Modules linked in:
>> [    1.268844] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
>> 4.1.0-rc4-00574-gf8eb363-dirty #1412
>> [    1.277085] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [    1.283184] [<c0015464>] (unwind_backtrace) from [<c00123c0>]
>> (show_stack+0x10/0x14)
>> [    1.288540] exynos-drm exynos-drm: bound 11c00000.fimd (ops
>> fimd_component_ops)
>> [    1.288883] exynos-drm exynos-drm: bound 11c80000.dsi (ops
>> exynos_dsi_component_ops)
>> [    1.288888] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
>> [    1.288891] [drm] No driver support for vblank timestamp query.
>> [    1.288932] [drm] Initialized exynos 1.0.0 20110530 on minor 0
>> [    1.324248] [<c00123c0>] (show_stack) from [<c0528f74>]
>> (dump_stack+0x84/0xc4)
>> [    1.331437] [<c0528f74>] (dump_stack) from [<c00246ac>]
>> (warn_slowpath_common+0x80/0xb0)
>> [    1.339504] [<c00246ac>] (warn_slowpath_common) from [<c0024778>]
>> (warn_slowpath_null+0x1c/0x24)
>> [    1.348270] [<c0024778>] (warn_slowpath_null) from [<c029c898>]
>> (drm_handle_vblank+0x2a0/0x308)
>> [    1.356965] [<c029c898>] (drm_handle_vblank) from [<c02b8c80>]
>> (fimd_irq_handler+0x78/0xd0)
>> [    1.365292] [<c02b8c80>] (fimd_irq_handler) from [<c00608f8>]
>> (handle_irq_event_percpu+0x78/0x134)
>> [    1.374231] [<c00608f8>] (handle_irq_event_percpu) from [<c00609f0>]
>> (handle_irq_event+0x3c/0x5c)
>> [    1.374244] [<c00609f0>] (handle_irq_event) from [<c0063698>]
>> (handle_level_irq+0xc4/0x13c)
>> [    1.374256] [<c0063698>] (handle_level_irq) from [<c005ff7c>]
>> (generic_handle_irq+0x2c/0x3c)
>> [    1.374269] [<c005ff7c>] (generic_handle_irq) from [<c02214ec>]
>> (combiner_handle_cascade_irq+0x94/0x100)
>> [    1.374281] [<c02214ec>] (combiner_handle_cascade_irq) from
>> [<c005ff7c>] (generic_handle_irq+0x2c/0x3c)
>> [    1.374290] [<c005ff7c>] (generic_handle_irq) from [<c0060248>]
>> (__handle_domain_irq+0x7c/0xec)
>> [    1.374300] [<c0060248>] (__handle_domain_irq) from [<c0009434>]
>> (gic_handle_irq+0x30/0x68)
>>
>>
>> And below one page fault error when modetest is terminated,
>>
>> # modetest -M exynos -v -s 31@29:720x1280
>> setting mode 720x1280-60Hz@XR24 on connectors 31, crtc 29
>> freq: 59.99Hz
>>
>> [   34.831025] PAGE FAULT occurred at 0x20400000 by 11e20000.sysmmu(Page
>> table base: 0x6e324000)
>> [   34.838072]  Lv1 entry: 0x6e92dc01
>> [   34.841489] ------------[ cut here ]------------
>> [   34.846058] kernel BUG at drivers/iommu/exynos-iommu.c:364!
>> [   34.851614] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
>> [   34.857428] Modules linked in:
>> [   34.860472] CPU: 0 PID: 6 Comm: kworker/u8:0 Tainted: G        W
>>    4.1.0-rc4-00574-gf8eb363-dirty #1412
>> [   34.870188] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [   34.876273] Workqueue: events_unbound async_run_entry_fn
>> [   34.881560] task: ee879540 ti: ee8a2000 task.ti: ee8a2000
>> [   34.886946] PC is at exynos_sysmmu_irq+0x184/0x208
>> [   34.891716] LR is at exynos_sysmmu_irq+0xd4/0x208
>> [   34.896404] pc : [<c02880d0>]    lr : [<c0288020>]    psr: 60000193
>> [   34.896404] sp : ee8a3c00  ip : 00000000  fp : eeacbc10
>> [   34.907860] r10: c07c27ef  r9 : 00000204  r8 : 20400000
>> [   34.913068] r7 : ee324000  r6 : ee9b3428  r5 : ee9b3410  r4 : 00000000
>> [   34.919577] r3 : 6e92dc01  r2 : 6e92dc01  r1 : eea55810  r0 : eeb0a000
>> [   34.926089] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
>> Segment kernel
>> [   34.933466] Control: 10c5387d  Table: 6dd2c04a  DAC: 00000015
>> [   34.939195] Process kworker/u8:0 (pid: 6, stack limit = 0xee8a2210)
>> [   34.945444] Stack: (0xee8a3c00 to 0xee8a4000)
>> [   34.949788] 3c00: ee8a3c0c c0049df8 00000000 6e324000 0003d4f3
>> ee9b13c0 ee84ad20 00000000
>> [   34.957947] 3c20: 00000000 00000026 ee84acc0 c00608f8 60000093
>> eef84580 ee84acc0 ee84ad20
>> [   34.966106] 3c40: ee9b13c0 00000015 ee804450 ee8a3cd8 ee808000
>> c00609f0 ee84acc0 ee84ad20
>> [   34.974265] 3c60: ee807000 c0063698 00000026 c079d6f8 ee807000
>> c005ff7c 0000000a c02214ec
>> [   34.982424] 3c80: 00000015 00000000 00000015 00000000 00000001
>> c005ff7c c0789aac c0060248
>> [   34.990583] 3ca0: f002000c 00000015 c078e7ac ee8a3cd8 f0020000
>> ee8a2000 00000001 c0009434
>> [   34.998742] 3cc0: c052e8fc 60000013 ffffffff ee8a3d0c 00000004
>> c0012ec0 eeacbd18 ee879540
>> [   35.006901] 3ce0: 00000000 00000271 eeacbd18 00000000 eea55010
>> c078e100 00000004 ee8a2000
>> [   35.015061] 3d00: 00000001 eeacbc10 eeacbd88 ee8a3d20 c052e8f8
>> c052e8fc 60000013 ffffffff
>> [   35.023220] 3d20: eeacbd18 c02dab48 20000113 00000003 60000013
>> eeacbd18 00000004 eeacbc60
>> [   35.031379] 3d40: ffff30d5 eeacbca8 eeacbd18 00000004 60000013
>> ffff30d5 c0802fe8 c02db05c
>> [   35.039538] 3d60: eeacbc60 00000002 c078e100 eeacbc60 ffff30d5
>> c03b0f2c c078bb40 ee8a3e28
>> [   35.047697] 3d80: 00000001 ee8a3db8 eeacbc70 00000002 eeacbc70
>> ee8a3db8 eeacbca8 c052e880
>> [   35.055857] 3da0: eeacbc70 c052d818 00000000 00000009 00000000
>> c002791c ee8a3db8 11111111
>> [   35.064016] 3dc0: 11111111 ee8a3dc4 11111111 eeacbc60 00000002
>> c078e100 ee8a3e28 ffff30d5
>> [   35.072175] 3de0: c0802fe8 00000001 ee80b400 c03ac4cc 00000000
>> eeacbc60 ee8a3e28 00000002
>> [   35.080334] 3e00: eea47ac0 00000000 ee849d00 c03ac69c 00000000
>> 00000001 00000001 eea47ac0
>> [   35.088493] 3e20: eea47ac0 c02e9f10 00000009 60000001 eea47ac0
>> 00010009 00000001 eea47ac0
>> [   35.096653] 3e40: ee29149c 00000000 ee00c200 c02e5bdc 00000001
>> c004425c ee00c200 00000047
>> [   35.104812] 3e60: ee00c200 ee00c200 ee8a3eb4 ee8a3eb4 00000000
>> c02e5c5c ee00c200 ee00c200
>> [   35.112971] 3e80: 00000047 c02e564c ee00c200 00000047 ee8a3eb4
>> ee80da00 00000000 c02e5704
>> [   35.121130] 3ea0: eeafd800 c07c5020 eeafd84c c025383c ee849d00
>> c052d110 eeafd800 c0252a48
>> [   35.129289] 3ec0: ee914724 c07c5020 edcc9c40 c0252f58 edcc9c50
>> c003ed48 eef84580 eea2f700
>> [   35.137448] 3ee0: ee849d00 edcc9c50 ee80b400 c0037944 ee8a2000
>> ee80b400 00000001 ee80b400
>> [   35.145608] 3f00: ee849d18 ee80b420 ee8a2000 00000088 c07c27bc
>> ee849d00 ee80b400 c0037ba0
>> [   35.153767] 3f20: c078e100 ee80b570 ee849d00 00000000 ee84b540
>> ee849d00 c0037b54 00000000
>> [   35.161926] 3f40: 00000000 00000000 00000000 c003c8b8 ffffffff
>> 00000000 ffffffff ee849d00
>> [   35.170085] 3f60: 00000000 00000000 dead4ead ffffffff ffffffff
>> ee8a3f74 ee8a3f74 00000000
>> [   35.178245] 3f80: 00000000 dead4ead ffffffff ffffffff ee8a3f90
>> ee8a3f90 ee8a3fac ee84b540
>> [   35.186403] 3fa0: c003c7dc 00000000 00000000 c000f5a0 00000000
>> 00000000 00000000 00000000
>> [   35.194562] 3fc0: 00000000 00000000 00000000 00000000 00000000
>> 00000000 00000000 00000000
>> [   35.202722] 3fe0: 00000000 00000000 00000000 00000000 00000013
>> 00000000 ffffffff ffffffff
>> [   35.210894] [<c02880d0>] (exynos_sysmmu_irq) from [<c00608f8>]
>> (handle_irq_event_percpu+0x78/0x134)
>> [   35.219914] [<c00608f8>] (handle_irq_event_percpu) from [<c00609f0>]
>> (handle_irq_event+0x3c/0x5c)
>> [   35.228768] [<c00609f0>] (handle_irq_event) from [<c0063698>]
>> (handle_level_irq+0xc4/0x13c)
>> [   35.237101] [<c0063698>] (handle_level_irq) from [<c005ff7c>]
>> (generic_handle_irq+0x2c/0x3c)
>> [   35.245521] [<c005ff7c>] (generic_handle_irq) from [<c02214ec>]
>> (combiner_handle_cascade_irq+0x94/0x100)
>> [   35.254980] [<c02214ec>] (combiner_handle_cascade_irq) from
>> [<c005ff7c>] (generic_handle_irq+0x2c/0x3c)
>> [   35.264353] [<c005ff7c>] (generic_handle_irq) from [<c0060248>]
>> (__handle_domain_irq+0x7c/0xec)
>> [   35.273034] [<c0060248>] (__handle_domain_irq) from [<c0009434>]
>> (gic_handle_irq+0x30/0x68)
>> [   35.281366] [<c0009434>] (gic_handle_irq) from [<c0012ec0>]
>> (__irq_svc+0x40/0x74)
>>
>> Could you check the iommu with atomic feature? I tested these features
>> on trats2 board, and enabled one more crtc drivers - HDMI, VIDI, and FIMD.
>>
>> To other Exynos guys,
>> could you guys check the same issues also? If you give some helps to us,
>> we'd be glad.
> I resolved above page fault issue with below patch:
> 	[PATCH] drm/exynos: fimd: fix page fault issue with iommu
>
> However, I still see above warning log. The problem is because
> dev->num_crtcs is 0 when drm_handle_vblank function is called by fimd
> irq handler.

Okay, I've managed to reproduce this issue. I will post v4 patch soon. 
Setting
ctx->pipe to -1 temporarily for vblank enable/wait/disable sequence 
solves this
issue.

 > (...)


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v2 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Inki Dae]

On 2015년 06월 12일 18:05, Marek Szyprowski wrote:
> Hello,
> 
> On 2015-06-11 17:04, Inki Dae wrote:
>> On 2015년 06월 03일 17:26, Marek Szyprowski wrote:
>>> One should not do any assumptions on the stare of the fimd hardware
>>> during driver initialization, so to properly reset fimd before enabling
>>> IOMMU, one should ensure that all power domains and clocks are really
>>> enabled. This patch adds pm_runtime and clocks management in the
>>> fimd_clear_channel() function to ensure that any access to fimd
>>> registers will be performed with clocks and power domains enabled.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>>> ---
>>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c | 22 ++++++++++++++++++----
>>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> index 96618534358e..3ec9d4299a86 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> @@ -242,12 +242,21 @@ static void
>>> fimd_enable_shadow_channel_path(struct fimd_context *ctx,
>>>       writel(val, ctx->regs + SHADOWCON);
>>>   }
>>>   +static int fimd_enable_vblank(struct exynos_drm_crtc *crtc);
>>> +static void fimd_disable_vblank(struct exynos_drm_crtc *crtc);
>> You can remove abvoe declarations. See the below comment.
>>
>>> +
>>>   static void fimd_clear_channel(struct fimd_context *ctx)
>>>   {
>>>       unsigned int win, ch_enabled = 0;
>>>         DRM_DEBUG_KMS("%s\n", __FILE__);
>>>   +    /* Hardware is in unknown state, so ensure it gets enabled
>>> properly */
>>> +    pm_runtime_get_sync(ctx->dev);
>>> +
>>> +    clk_prepare_enable(ctx->bus_clk);
>>> +    clk_prepare_enable(ctx->lcd_clk);
>>> +
>>>       /* Check if any channel is enabled. */
>>>       for (win = 0; win < WINDOWS_NR; win++) {
>>>           u32 val = readl(ctx->regs + WINCON(win));
>>> @@ -265,12 +274,17 @@ static void fimd_clear_channel(struct
>>> fimd_context *ctx)
>>>         /* Wait for vsync, as disable channel takes effect at next
>>> vsync */
>>>       if (ch_enabled) {
>>> -        unsigned int state = ctx->suspended;
>>> -
>>> -        ctx->suspended = 0;
>>> +        ctx->suspended = false;
>>> +        fimd_enable_vblank(ctx->crtc);
>> I think you can call enable_vblank callback instead of
>> fimd_enable_vblank function because ctx object has exynos_crtc object.
>>
>> i.e.,
>>         struct exynos_drm_crtc_ops *ops = ctx->crtc->ops;
>>         ...
>>         if (ops->enable_vblank)
>>             ops->enable_vblank(ctx->crtc);
>>         ...
> 
> Well, I don't like such indirect calls to known functions, but if you
> prefer
> this approach I will send an updated patch in a minute. There is also
> alternative
> way of getting rid of forward declarations. Code of fimd_enable_vblank and
> fimd_disable_vblank can be moved closer to fimd_wait_vblank function.
> I will also send such alternative patch. Feel free to select the one
> that better
> fits your preferences.

Latter one it's better.

Thanks,
Inki Dae

> 
>>>           fimd_wait_for_vblank(ctx->crtc);
>>> -        ctx->suspended = state;
>>> +        fimd_disable_vblank(ctx->crtc);
>> Ditto.
>>
>> Thanks,
>> Inki Dae
>>
>>> +        ctx->suspended = true;
>>>       }
>>> +
>>> +    clk_disable_unprepare(ctx->lcd_clk);
>>> +    clk_disable_unprepare(ctx->bus_clk);
>>> +
>>> +    pm_runtime_put(ctx->dev);
>>>   }
>>>     static int fimd_iommu_attach_devices(struct fimd_context *ctx,
>>>
>>
> 
> Best regards

[PATCH v2 2/3] drm/exynos: iommu: detach from default dma-mapping domain on init

[Marek Szyprowski]

This patch adds code, which detach sub-device nodes from default iommu
domain if such has been configured. This lets Exynos DRM driver to properly
attach sub-devices to its own, common for all sub-devices domain.

Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Tested-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
 drivers/gpu/drm/exynos/exynos_drm_iommu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
index b32b291f88ff..323601a52a25 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
@@ -100,6 +100,9 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
 
 	dma_set_max_seg_size(subdrv_dev, 0xffffffffu);
 
+	if (subdrv_dev->archdata.mapping)
+		arm_iommu_detach_device(subdrv_dev);
+
 	ret = arm_iommu_attach_device(subdrv_dev, dev->archdata.mapping);
 	if (ret < 0) {
 		DRM_DEBUG_KMS("failed iommu attach.\n");
-- 
1.9.2

[PATCH v2 3/3] drm/exynos: iommu: improve a check for non-iommu dma_ops

[Marek Szyprowski]

DRM Exynos driver is relying on dma-mapping internal structures when used
with IOMMU enabled. This patch partially hides dma-mapping internal things
by using proper get_dma_ops/set_dma_ops calls.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
index 323601a52a25..34596da7be33 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
@@ -117,8 +117,8 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
 	 * If iommu attach succeeded, the sub driver would have dma_ops
 	 * for iommu and also all sub drivers have same dma_ops.
 	 */
-	if (!dev->archdata.dma_ops)
-		dev->archdata.dma_ops = subdrv_dev->archdata.dma_ops;
+	if (get_dma_ops(dev) == get_dma_ops(NULL))
+		set_dma_ops(dev, get_dma_ops(subdrv_dev));
 
 	return 0;
 }
-- 
1.9.2