Am 05.04.20 um 12:18 schrieb Noralf Trønnes: > > > Den 03.04.2020 15.57, skrev Daniel Vetter: >> Also init the fbdev emulation before we register the device, that way >> we can rely on the auto-cleanup and simplify the probe error code even >> more. >> >> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> >> Cc: Dave Airlie <airlied@xxxxxxxxxx> >> Cc: Sean Paul <sean@xxxxxxxxxx> >> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> >> Cc: Emil Velikov <emil.l.velikov@xxxxxxxxx> >> Cc: Sam Ravnborg <sam@xxxxxxxxxxxx> >> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/udl/udl_drv.c | 36 +++++++++++------------------------ >> 1 file changed, 11 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c >> index 1ce2d865c36d..4ba5149fdd57 100644 >> --- a/drivers/gpu/drm/udl/udl_drv.c >> +++ b/drivers/gpu/drm/udl/udl_drv.c >> @@ -57,27 +57,20 @@ static struct udl_device *udl_driver_create(struct usb_interface *interface) >> struct udl_device *udl; >> int r; >> >> - udl = kzalloc(sizeof(*udl), GFP_KERNEL); >> - if (!udl) >> - return ERR_PTR(-ENOMEM); >> - >> - r = drm_dev_init(&udl->drm, &driver, &interface->dev); >> - if (r) { >> - kfree(udl); >> - return ERR_PTR(r); >> - } >> + udl = devm_drm_dev_alloc(&interface->dev, &driver, >> + struct udl_device, drm); >> + if (IS_ERR(udl)) >> + return udl; >> >> udl->udev = udev; >> udl->drm.dev_private = udl; >> - drmm_add_final_kfree(&udl->drm, udl); >> >> r = udl_init(udl); >> - if (r) { >> - drm_dev_put(&udl->drm); >> + if (r) >> return ERR_PTR(r); >> - } >> >> usb_set_intfdata(interface, udl); >> + >> return udl; >> } >> >> @@ -91,23 +84,17 @@ static int udl_usb_probe(struct usb_interface *interface, >> if (IS_ERR(udl)) >> return PTR_ERR(udl); >> >> + r = drm_fbdev_generic_setup(&udl->drm, 0); > > It doesn't feel right to have a client open the device before the DRM > device itself is registered. I would prefer to keep it where it is but Agreed. IMHO we should also go through drivers and make the fbdev setup the final step everywhere. Best regards Thomas > ignore any errors. A failing client shouldn't prevent the driver from > probing. drm_fbdev_generic_setup() do print errors if it fails. So yeah, > in hindsight I should have made drm_fbdev_generic_setup() return void. > > Noralf. > >> + if (r) >> + return r; >> + >> r = drm_dev_register(&udl->drm, 0); >> if (r) >> - goto err_free; >> + return r; >> >> DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index); >> >> - r = drm_fbdev_generic_setup(&udl->drm, 0); >> - if (r) >> - goto err_drm_dev_unregister; >> - >> return 0; >> - >> -err_drm_dev_unregister: >> - drm_dev_unregister(&udl->drm); >> -err_free: >> - drm_dev_put(&udl->drm); >> - return r; >> } >> >> static void udl_usb_disconnect(struct usb_interface *interface) >> @@ -117,7 +104,6 @@ static void udl_usb_disconnect(struct usb_interface *interface) >> drm_kms_helper_poll_fini(dev); >> udl_drop_usb(dev); >> drm_dev_unplug(dev); >> - drm_dev_put(dev); >> } >> >> /* >> > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx