On 2016-10-18 00:44, Daniel Vetter wrote:
On Mon, Oct 17, 2016 at 02:33:21PM -0700, Stefan Agner wrote:
There is no need to request a CMA backed framebuffer if fbdev emulation is not enabled.
Signed-off-by: Stefan Agner stefan@agner.ch
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index e04efbe..3a5880c 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c @@ -87,7 +87,8 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags) goto done; dev->irq_enabled = true;
- fsl_dcu_fbdev_init(dev);
- if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION))
fsl_dcu_fbdev_init(dev);Totally not required, since this will all no-op out. Also, please nuke that fsl_dcu_fbdv_init wrapper seems like pointless indirection.
Regarding fsl_dcu_fbdev_init wrapper: Fully agreed. That thing was there since inception of that driver, and I always was on the fence of doing it. Will do it for the next merge window!
I somehow remembered there was something more to it than just "no need", but I somehow failed to document it in the patch description... So I went back and tested some things without the patch, here is when it blows:
Unable to handle kernel NULL pointer dereference at virtual address 000002f0 pgd = cc24c000 [000002f0] *pgd=8c0df831, *pte=00000000, *ppte=00000000 Internal error: Oops: 17 [#1] SMP ARM Modules linked in: CPU: 0 PID: 536 Comm: sh Not tainted 4.9.0-rc1-00001-g4b1532a-dirty #568 Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree) task: cc213000 task.stack: cc23e000 PC is at drm_fbdev_cma_fini+0x14/0x5c LR is at fsl_dcu_unload+0x28/0x4c pc : [<c04cfb20>] lr : [<c04fad74>] psr: a0000013 sp : cc23fd80 ip : cc23fd98 fp : cc23fd94 r10: cc1d1e4c r9 : cc23e000 r8 : 00000000 r7 : c0e34888 r6 : 0000000d r5 : 00000000 r4 : ce6ff100 r3 : c0e13b18 r2 : c0e13b18 r1 : 00000110 r0 : ce6ff100 Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 8c24c04a DAC: 00000051 Process sh (pid: 536, stack limit = 0xcc23e210) Stack: (0xcc23fd80 to 0xcc240000) ... Backtrace: [<c04cfb0c>] (drm_fbdev_cma_fini) from [<c04fad74>] (fsl_dcu_unload+0x28/0x4c) r5:ce61e810[ 372.213609] r4:ce61f000
[<c04fad4c>] (fsl_dcu_unload) from [<c04d9550>] (drm_dev_unregister+0x38/0xbc) r5:ce61f000[ 372.228535] r4:ce61f000 ...
And if there really is an issue with the cma helpers allocating an fb when they should, then the correct fix is to fix that in the helpers, not in the drivers.
Hm, to safe memory, it would probably be best to do something like:
--- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -492,6 +492,9 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, struct drm_fb_helper *helper; int ret;
+ if (!drm_fbdev_emulation) + return NULL; + fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL); if (!fbdev_cma) { dev_err(dev->dev, "Failed to allocate drm fbdev.\n");
But we don't have drm_fbdev_emulation emulation there.
Maybe just add some NULL check in drm_fbdev_cma_fini?
-- Stefan
Nack. -Daniel
return 0; done: -- 2.10.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel