We forgot one call site directly using drm->dev_private. This leads to a crash like this:

8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 00000918 pgd = (ptrval) [00000918] *pgd=00000000 Internal error: Oops: 5 [#1] SMP ARM Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-15001-gfa384b50b96b-dirty #514 Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support) PC is at mcde_display_enable+0x78/0x7c0 LR is at mcde_display_enable+0x78/0x7c0

Fix this by using to_mcde() as in other functions.

Cc: Daniel Vetter daniel.vetter@ffwll.ch Fixes: fd7ee85cfe7b ("drm/mcde: Don't use drm_device->dev_private") Signed-off-by: Linus Walleij linus.walleij@linaro.org --- drivers/gpu/drm/mcde/mcde_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c index b319a474f801..212aee60cf61 100644 --- a/drivers/gpu/drm/mcde/mcde_display.c +++ b/drivers/gpu/drm/mcde/mcde_display.c @@ -838,7 +838,7 @@ static void mcde_display_enable(struct drm_simple_display_pipe *pipe, struct drm_crtc *crtc = &pipe->crtc; struct drm_plane *plane = &pipe->plane; struct drm_device *drm = crtc->dev; - struct mcde *mcde = drm->dev_private; + struct mcde *mcde = to_mcde(drm); const struct drm_display_mode *mode = &cstate->mode; struct drm_framebuffer *fb = plane->state->fb; u32 format = fb->format->format;

Hi Linus.

On Sun, Jun 14, 2020 at 12:30:26AM +0200, Linus Walleij wrote:

The following bug appeared in the MCDE driver/display initialization during the recent merge window.

First the place we call drm_fbdev_generic_setup() in the wrong place: this needs to be called AFTER calling drm_dev_register() else we get this splat:

------------[ cut here ]------------ WARNING: CPU: 0 PID: 1 at ../drivers/gpu/drm/drm_fb_helper.c:2198 drm_fbdev_generic_setup+0x164/0x1a8 mcde a0350000.mcde: Device has not been registered. Modules linked in: Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support) [<c010e704>] (unwind_backtrace) from [<c010a86c>] (show_stack+0x10/0x14) [<c010a86c>] (show_stack) from [<c0414f38>] (dump_stack+0x9c/0xb0) [<c0414f38>] (dump_stack) from [<c0121c8c>] (__warn+0xb8/0xd0) [<c0121c8c>] (__warn) from [<c0121d18>] (warn_slowpath_fmt+0x74/0xb8) [<c0121d18>] (warn_slowpath_fmt) from [<c04b154c>] (drm_fbdev_generic_setup+0x164/0x1a8) [<c04b154c>] (drm_fbdev_generic_setup) from [<c04ed278>] (mcde_drm_bind+0xc4/0x160) [<c04ed278>] (mcde_drm_bind) from [<c04f06b8>] (try_to_bring_up_master+0x15c/0x1a4) (...)

Signed-off-by: Linus Walleij linus.walleij@linaro.org

Yup, this is the right way to do it.

Reviewed-by: Sam Ravnborg sam@ravnborg.org

One small thing to consider, see below. But for another patch.

---

drivers/gpu/drm/mcde/mcde_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c index 84f3e2dbd77b..80082d6dce3a 100644 --- a/drivers/gpu/drm/mcde/mcde_drv.c +++ b/drivers/gpu/drm/mcde/mcde_drv.c @@ -209,7 +209,6 @@ static int mcde_modeset_init(struct drm_device *drm)

drm_mode_config_reset(drm); drm_kms_helper_poll_init(drm);

• drm_fbdev_generic_setup(drm, 32);

  return 0;

} @@ -264,6 +263,8 @@ static int mcde_drm_bind(struct device *dev) if (ret < 0) goto unbind;

• drm_fbdev_generic_setup(drm, 32);

Consider using mode_config.preferred_depth, then you can pass 0 here.

• return 0;

unbind:

2.26.2

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel