[PATCH 2/2] drm/exynos: fix the initialization order in FIMD

Hyungwon Hwang human.hwang at samsung.com
Tue Mar 31 20:35:28 PDT 2015


Dear Joonyoung Shim,

On Tue, 31 Mar 2015 17:05:47 +0900
Joonyoung Shim <jy0922.shim at samsung.com> wrote:

> Hi,
> 
> Sorry for late comments.
> 
> On 03/13/2015 07:32 PM, Inki Dae wrote:
> > On 2015년 03월 12일 13:36, Hyungwon Hwang wrote:
> >> Since commit 0f04cf8df0b20a97369cb634663fef0578cbf273 ("drm/exynos:
> >> fix wrong pipe calculation for crtc"), fimd_clear_channel() can be
> >> called when is_drm_iommu_supported() returns true. In this case,
> >> the kernel is going to be panicked because crtc is not set yet.
> > 
> > Nice catch!!!
> > 
> >>
> >> [    1.211156] [drm] Initialized drm 1.1.0 20060810
> >> [    1.216785] Unable to handle kernel NULL pointer dereference at
> >> virtual address 00000350 [    1.223415] pgd = c0004000
> >> [    1.226086] [00000350] *pgd=00000000
> >> [    1.229649] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> >> [    1.234940] Modules linked in:
> >> [    1.237982] CPU: 2 PID: 1 Comm: swapper/0 Not tainted
> >> 4.0.0-rc1-00062-g7a7cc79-dirty #123 [    1.246136] Hardware name:
> >> SAMSUNG EXYNOS (Flattened Device Tree) [    1.252214] task:
> >> ee8c8000 ti: ee8d0000 task.ti: ee8d0000 [    1.257606] PC is at
> >> fimd_wait_for_vblank+0x8/0xc8 [    1.262370] LR is at
> >> fimd_bind+0x138/0x1a8 [    1.266450] pc : [<c02fb63c>]    lr :
> >> [<c02fb834>]    psr: 20000113 [    1.266450] sp : ee8d1d28  ip :
> >> 00000000  fp : 00000000 [    1.277906] r10: 00000001  r9 :
> >> c09d693c  r8 : c0a2d6a8 [    1.283114] r7 : 00000034  r6 :
> >> 00000001  r5 : ee0bb400  r4 : ee244c10 [    1.289624] r3 :
> >> 00000000  r2 : 00000000  r1 : 00000001  r0 : 00000000
> >> [    1.296135] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA
> >> ARM  Segment kernel [    1.303426] Control: 10c5387d  Table:
> >> 4000404a  DAC: 00000015 [    1.309154] Process swapper/0 (pid: 1,
> >> stack limit = 0xee8d0210) [    1.315143] Stack: (0xee8d1d28 to
> >> 0xee8d2000) [    1.319486] 1d20:                   00000000
> >> c0113d18 ee0bb400 ee0bb400 ee245c30 eebbe210 [    1.327645] 1d40:
> >> ee008a40 ee244c10 ee0bb400 00000001 00000034 c02fb834 00000000
> >> c030a858 [    1.335804] 1d60: ee244a10 eeb60780 ee008a40 eeb60740
> >> ee0bb400 c03030d0 00000000 00000000 [    1.343963] 1d80: ee244a10
> >> ee0bb400 00000000 eeb60740 eeb60810 00000000 00000000 c02f6ba4
> >> [    1.352123] 1da0: ee0bb400 00000000 00000000 c02e0500 ee244a00
> >> c0a04a14 ee0bb400 c02e1de4 [    1.360282] 1dc0: 00000000 c030a858
> >> 00000002 eeb60820 eeb60820 00000002 eeb60780 c03033d4
> >> [    1.368441] 1de0: c06e9cec 00000000 ee244a10 eeb60780 c0a056f8
> >> c03035fc c0a04b24 c0a04b24 [    1.376600] 1e00: ee244a10 00000001
> >> c0a049d0 c02f6d34 c0ad462c eeba0790 00000000 ee244a10
> >> [    1.384759] 1e20: ffffffed c0a049d0 00000000 c03090b0 ee244a10
> >> c0ad462c c0a2d840 c03077a0 [    1.392919] 1e40: eeb5e880 c024b738
> >> 000008db ee244a10 c0a049d0 ee244a44 00000000 c09e71d8
> >> [    1.401078] 1e60: 000000c6 c0307a6c c0a049d0 00000000 c03079e0
> >> c0305ea8 ee826e5c ee1dc7b4 [    1.409237] 1e80: c0a049d0 eeb5e880
> >> c0a058a8 c0306e2c c0896204 c0a049d0 c06e9d10 c0a049d0
> >> [    1.417396] 1ea0: c06e9d10 c0ad4600 00000000 c0308360 00000000
> >> 00000003 c06e9d10 c02f6e14 [    1.425555] 1ec0: 00000000 c0896204
> >> ffffffff 00000000 00000000 00000000 00000000 00000000
> >> [    1.433714] 1ee0: 00000000 00000000 c02f6d5c c02f6d5c 00000000
> >> eeb5d740 c09e71d8 c0008a30 [    1.441874] 1f00: ef7fca5e 00000000
> >> 00000000 00000066 00000000 ee8d1f28 c003ff1c c02514e8
> >> [    1.450033] 1f20: 60000113 ffffffff c093906c ef7fca5e 000000c6
> >> c004018c 00000000 c093906c [    1.458192] 1f40: c08a9690 c093840c
> >> 00000006 00000006 c09eb2ac c09c0d74 00000006 c09c0d54
> >> [    1.466351] 1f60: c0a3d680 c09745a0 c09d693c 000000c6 00000000
> >> c0974db4 00000006 00000006 [    1.474510] 1f80: c09745a0 ffffffff
> >> 00000000 c0692e00 00000000 00000000 00000000 00000000
> >> [    1.482669] 1fa0: 00000000 c0692e08 00000000 c000f040 00000000
> >> 00000000 00000000 00000000 [    1.490828] 1fc0: 00000000 00000000
> >> 00000000 00000000 00000000 00000000 00000000 00000000
> >> [    1.498988] 1fe0: 00000000 00000000 00000000 00000000 00000013
> >> 00000000 ffffffff ffffffff [    1.507159] [<c02fb63c>]
> >> (fimd_wait_for_vblank) from [<c02fb834>] (fimd_bind+0x138/0x1a8)
> >> [    1.515313] [<c02fb834>] (fimd_bind) from [<c03030d0>]
> >> (component_bind_all+0xc4/0x20c) [    1.523209] [<c03030d0>]
> >> (component_bind_all) from [<c02f6ba4>]
> >> (exynos_drm_load+0xa0/0x140) [    1.531632] [<c02f6ba4>]
> >> (exynos_drm_load) from [<c02e0500>] (drm_dev_register+0xa0/0xf4)
> >> [    1.539788] [<c02e0500>] (drm_dev_register) from [<c02e1de4>]
> >> (drm_platform_init+0x44/0xcc) [    1.548121] [<c02e1de4>]
> >> (drm_platform_init) from [<c03033d4>]
> >> (try_to_bring_up_master.part.1+0xc8/0x104) [    1.557668]
> >> [<c03033d4>] (try_to_bring_up_master.part.1) from [<c03035fc>]
> >> (component_master_add_with_match+0xd0/0x118) [    1.568431]
> >> [<c03035fc>] (component_master_add_with_match) from [<c02f6d34>]
> >> (exynos_drm_platform_probe+0xf0/0x118) [    1.578847] [<c02f6d34>]
> >> (exynos_drm_platform_probe) from [<c03090b0>]
> >> (platform_drv_probe+0x48/0x98) [    1.588052] [<c03090b0>]
> >> (platform_drv_probe) from [<c03077a0>]
> >> (driver_probe_device+0x140/0x380) [    1.596902] [<c03077a0>]
> >> (driver_probe_device) from [<c0307a6c>]
> >> (__driver_attach+0x8c/0x90) [    1.605321] [<c0307a6c>]
> >> (__driver_attach) from [<c0305ea8>] (bus_for_each_dev+0x54/0x88)
> >> [    1.613480] [<c0305ea8>] (bus_for_each_dev) from [<c0306e2c>]
> >> (bus_add_driver+0xec/0x200) [    1.621640] [<c0306e2c>]
> >> (bus_add_driver) from [<c0308360>] (driver_register+0x78/0xf4)
> >> [    1.629625] [<c0308360>] (driver_register) from [<c02f6e14>]
> >> (exynos_drm_init+0xb8/0x11c) [    1.637785] [<c02f6e14>]
> >> (exynos_drm_init) from [<c0008a30>] (do_one_initcall+0xac/0x1ec)
> >> [    1.645950] [<c0008a30>] (do_one_initcall) from [<c0974db4>]
> >> (kernel_init_freeable+0x194/0x268) [    1.654626] [<c0974db4>]
> >> (kernel_init_freeable) from [<c0692e08>] (kernel_init+0x8/0xe4)
> >> [    1.662699] [<c0692e08>] (kernel_init) from [<c000f040>]
> >> (ret_from_fork+0x14/0x34) [    1.670246] Code: eaffffd5 c09df884
> >> e92d40f0 e24dd01c (e5905350) [    1.676408] ---[ end trace
> >> 804468492f306a6f ]--- [    1.680948] Kernel panic - not syncing:
> >> Attempted to kill init! exitcode=0x0000000b [    1.680948]
> >> [    1.690035] CPU1: stopping [    1.692727] CPU: 1 PID: 0 Comm:
> >> swapper/1 Tainted: G      D         4.0.0-rc1-00062-g7a7cc79-dirty
> >> #123 [    1.702097] Hardware name: SAMSUNG EXYNOS (Flattened
> >> Device Tree) [    1.708192] [<c0016c84>] (unwind_backtrace) from
> >> [<c00129bc>] (show_stack+0x10/0x14) [    1.715908] [<c00129bc>]
> >> (show_stack) from [<c0696f58>] (dump_stack+0x78/0xc8)
> >> [    1.723108] [<c0696f58>] (dump_stack) from [<c0015020>]
> >> (handle_IPI+0x16c/0x2b4) [    1.730485] [<c0015020>] (handle_IPI)
> >> from [<c00086bc>] (gic_handle_irq+0x64/0x6c) [    1.738036]
> >> [<c00086bc>] (gic_handle_irq) from [<c00134c0>]
> >> (__irq_svc+0x40/0x74) [    1.745498] Exception stack(0xee8fdf98 to
> >> 0xee8fdfe0) [    1.750533]
> >> df80:
> >> 00000000 00000000 [    1.758695] dfa0: ee8fdfe8 c0021780 c09df938
> >> 00000015 10c0387d c0a3d988 4000406a c09df8d4 [    1.766853] dfc0:
> >> c0a27a74 c09df940 01000000 ee8fdfe0 c00101c0 c00101c4 60000113
> >> ffffffff [    1.775015] [<c00134c0>] (__irq_svc) from [<c00101c4>]
> >> (arch_cpu_idle+0x30/0x3c) [    1.782397] [<c00101c4>]
> >> (arch_cpu_idle) from [<c005e804>] (cpu_startup_entry+0x180/0x324)
> >> [    1.790639] [<c005e804>] (cpu_startup_entry) from [<40008764>]
> >> (0x40008764) [    1.797579] CPU0: stopping [    1.800272] CPU: 0
> >> PID: 0 Comm: swapper/0 Tainted: G      D
> >> 4.0.0-rc1-00062-g7a7cc79-dirty #123 [    1.809642] Hardware name:
> >> SAMSUNG EXYNOS (Flattened Device Tree) [    1.815730] [<c0016c84>]
> >> (unwind_backtrace) from [<c00129bc>] (show_stack+0x10/0x14)
> >> [    1.823450] [<c00129bc>] (show_stack) from [<c0696f58>]
> >> (dump_stack+0x78/0xc8) [    1.830653] [<c0696f58>] (dump_stack)
> >> from [<c0015020>] (handle_IPI+0x16c/0x2b4) [    1.838030]
> >> [<c0015020>] (handle_IPI) from [<c00086bc>]
> >> (gic_handle_irq+0x64/0x6c) [    1.845581] [<c00086bc>]
> >> (gic_handle_irq) from [<c00134c0>] (__irq_svc+0x40/0x74)
> >> [    1.853043] Exception stack(0xc09ddf60 to 0xc09ddfa8)
> >> [    1.858081] df60: 00000000 00000000 c09ddfb0 c0021780 c09df938
> >> 00000001 ffffffff c0a3d680 [    1.866239] df80: c09c0dec c09df8d4
> >> c0a27a74 c09df940 01000000 c09ddfa8 c00101c0 c00101c4
> >> [    1.874396] dfa0: 60000113 ffffffff [    1.877872] [<c00134c0>]
> >> (__irq_svc) from [<c00101c4>] (arch_cpu_idle+0x30/0x3c)
> >> [    1.885251] [<c00101c4>] (arch_cpu_idle) from [<c005e804>]
> >> (cpu_startup_entry+0x180/0x324) [    1.893499] [<c005e804>]
> >> (cpu_startup_entry) from [<c0974bc8>] (start_kernel+0x324/0x37c)
> >> [    1.901655] [<c0974bc8>] (start_kernel) from [<40008074>]
> >> (0x40008074) [    1.908161] CPU3: stopping [    1.910855] CPU: 3
> >> PID: 0 Comm: swapper/3 Tainted: G      D
> >> 4.0.0-rc1-00062-g7a7cc79-dirty #123 [    1.920225] Hardware name:
> >> SAMSUNG EXYNOS (Flattened Device Tree) [    1.926313] [<c0016c84>]
> >> (unwind_backtrace) from [<c00129bc>] (show_stack+0x10/0x14)
> >> [    1.934034] [<c00129bc>] (show_stack) from [<c0696f58>]
> >> (dump_stack+0x78/0xc8) [    1.941237] [<c0696f58>] (dump_stack)
> >> from [<c0015020>] (handle_IPI+0x16c/0x2b4) [    1.948613]
> >> [<c0015020>] (handle_IPI) from [<c00086bc>]
> >> (gic_handle_irq+0x64/0x6c) [    1.956165] [<c00086bc>]
> >> (gic_handle_irq) from [<c00134c0>] (__irq_svc+0x40/0x74)
> >> [    1.963626] Exception stack(0xee901f98 to 0xee901fe0)
> >> [    1.968661]
> >> 1f80:
> >> 00000000 00000000 [    1.976823] 1fa0: ee901fe8 c0021780 c09df938
> >> 00000015 10c0387d c0a3d988 4000406a c09df8d4 [    1.984982] 1fc0:
> >> c0a27a74 c09df940 01000000 ee901fe0 c00101c0 c00101c4 60000113
> >> ffffffff [    1.993143] [<c00134c0>] (__irq_svc) from [<c00101c4>]
> >> (arch_cpu_idle+0x30/0x3c) [    2.000522] [<c00101c4>]
> >> (arch_cpu_idle) from [<c005e804>] (cpu_startup_entry+0x180/0x324)
> >> [    2.008765] [<c005e804>] (cpu_startup_entry) from [<40008764>]
> >> (0x40008764) [    2.015710] ---[ end Kernel panic - not syncing:
> >> Attempted to kill init! exitcode=0x0000000b
> >>
> >> Signed-off-by: Hyungwon Hwang <human.hwang at samsung.com>
> >> Cc: Inki Dae <inki.dae at samsung.com>
> >> ---
> >>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 31
> >> +++++++++++++------------------ 1 file changed, 13 insertions(+),
> >> 18 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> >> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 925fc69..31dfa80
> >> 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> >> @@ -284,14 +284,9 @@ static void fimd_clear_channel(struct
> >> fimd_context *ctx) }
> >>  }
> >>
> >> -static int fimd_ctx_initialize(struct fimd_context *ctx,
> >> +static int fimd_iommu_attach_devices(struct fimd_context *ctx,
> >>  			struct drm_device *drm_dev)
> >>  {
> >> -	struct exynos_drm_private *priv;
> >> -	priv = drm_dev->dev_private;
> >> -
> >> -	ctx->drm_dev = drm_dev;
> >> -	ctx->pipe = priv->pipe++;
> >>
> >>  	/* attach this sub driver to iommu mapping if supported.
> >> */ if (is_drm_iommu_supported(ctx->drm_dev)) {
> >> @@ -313,7 +308,7 @@ static int fimd_ctx_initialize(struct
> >> fimd_context *ctx, return 0;
> >>  }
> >>
> >> -static void fimd_ctx_remove(struct fimd_context *ctx)
> >> +static void fimd_iommu_detach_devices(struct fimd_context *ctx)
> >>  {
> >>  	/* detach this sub driver from iommu mapping if
> >> supported. */ if (is_drm_iommu_supported(ctx->drm_dev))
> >> @@ -1056,25 +1051,25 @@ static int fimd_bind(struct device *dev,
> >> struct device *master, void *data) {
> >>  	struct fimd_context *ctx = dev_get_drvdata(dev);
> >>  	struct drm_device *drm_dev = data;
> >> +	struct exynos_drm_private *priv = drm_dev->dev_private;
> >>  	int ret;
> >>
> >> -	ret = fimd_ctx_initialize(ctx, drm_dev);
> >> -	if (ret) {
> >> -		DRM_ERROR("fimd_ctx_initialize failed.\n");
> >> -		return ret;
> >> -	}
> >> +	ctx->drm_dev = drm_dev;
> >> +	ctx->pipe = priv->pipe++;
> >>
> >>  	ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe,
> >>  					   EXYNOS_DISPLAY_TYPE_LCD,
> >>  					   &fimd_crtc_ops, ctx);
> >> -	if (IS_ERR(ctx->crtc)) {
> 
> Why doesn't this error checking need?
> 
> Thanks.

The error checking code is needed. This patch is already merged. So I
will make another patch to add the checking code.

Thanks for your review.

Best regards,
Hyungwon Hwang

> 
> >> -		fimd_ctx_remove(ctx);
> >> -		return PTR_ERR(ctx->crtc);
> >> -	}
> >>
> >>  	if (ctx->display)
> >>  		exynos_drm_create_enc_conn(drm_dev, ctx->display);
> >>
> >> +	ret = fimd_iommu_attach_devices(ctx, drm_dev);
> >> +	if (ret) {
> >> +		DRM_ERROR("fimd_ctx_initialize failed.\n");
> >> +		return ret;
> >> +	}
> > 
> > Above error message wouldn't be unnecessary because the error
> > message is already printed out at fimd_iommu_attach_devices
> > function. But this is a trivial issue so I fixed and merged it.
> > 
> > Thanks,
> > Inki Dae
> > 
> >> +
> >>  	return 0;
> >>
> >>  }
> >> @@ -1086,10 +1081,10 @@ static void fimd_unbind(struct device
> >> *dev, struct device *master,
> >>
> >>  	fimd_dpms(ctx->crtc, DRM_MODE_DPMS_OFF);
> >>
> >> +	fimd_iommu_detach_devices(ctx);
> >> +
> >>  	if (ctx->display)
> >>  		exynos_dpi_remove(ctx->display);
> >> -
> >> -	fimd_ctx_remove(ctx);
> >>  }
> >>
> >>  static const struct component_ops fimd_component_ops = {
> >> --
> >> 1.9.1
> >>
> >>
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 



More information about the dri-devel mailing list