Hi Jani Am 07.04.20 um 09:24 schrieb Jani Nikula: > On Tue, 07 Apr 2020, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote: >> Hi Sam >> >> Am 06.04.20 um 22:00 schrieb Sam Ravnborg: >>> Hi Thomas. >>> >>> On Mon, Apr 06, 2020 at 03:43:55PM +0200, Thomas Zimmermann wrote: >>>> Generic fbdev emulation is a DRM client. If possible, it should behave >>>> like userspace clients. Therefore it should not run before the driver >>>> registered the new DRM device. If the setup function fails, the driver >>>> should not report an error. >>> >>> Thanks for taking the time to refactor all the relevant drivers. >>> >>> I have received some push-back in the past when suggesting this, >>> but cannot remember from who. >>> Let's see what review comments you get. >>> >>> As the rule is that the fbdev setup shall be setup after registering >>> the DRM device - it would be nice to have this included in the >>> documentation of drm_fbdev_generic_setup >>> >>> Could you try to to update the documentation to cover this? >> >> Good idea. I'll add this to patchset's next iteration. > > How about something like: > > drm_WARN_ON(dev, !dev->registered); > > (Not sure if that needs to be !dev->driver->load && !dev->registered). I added such a warning to the patch. Only radeon uses load(), but it has its own fbdev code. So the original test should be fine. Best regards Thomas > > This can be a follow-up patch later too. > > BR, > Jani. > > >> >> Best regards >> Thomas >> >>> >>> I will get back to the patches later this week. >>> >>> Sam >>> >>>> >>>> This is a follow-up patchset to the discussion at [1]. I went >>>> through all calls to drm_fbdev_generic_setup(), moved them to the >>>> final operation of their driver's probe function, and removed the >>>> return value. >>>> >>>> Built-tested on x86-64, aarch64 and arm. >>>> >>>> [1] https://lore.kernel.org/dri-devel/20200403135828.2542770-1-daniel.vetter@xxxxxxxx/T/#m216b5b37aeeb7b28d55ad73b7a702b3d1d7bf867 >>>> >>>> Thomas Zimmermann (10): >>>> drm/ast: Set up fbdev after registering device; remove error checks >>>> drm/hibmc: Remove error check from fbdev setup >>>> drm/kirin: Set up fbdev after fully registering device >>>> drm/ingenic: Remove error check from fbdev setup >>>> drm/mediathek: Remove error check from fbdev setup >>>> drm/mgag200: Set up fbdev after registering device; remove error >>>> checks >>>> drm/tilcdc: Set up fbdev after fully registering device >>>> drm/udl: Remove error check from fbdev setup >>>> drm/vboxvideo: Set up fbdev after registering device; remove error >>>> checks >>>> drm/fb-helper: Remove return value from drm_fbdev_generic_setup() >>>> >>>> drivers/gpu/drm/ast/ast_drv.c | 3 +++ >>>> drivers/gpu/drm/ast/ast_main.c | 5 ----- >>>> drivers/gpu/drm/drm_fb_helper.c | 18 ++++++++---------- >>>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 +----- >>>> .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 ++-- >>>> drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +--- >>>> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +--- >>>> drivers/gpu/drm/mgag200/mgag200_drv.c | 2 ++ >>>> drivers/gpu/drm/mgag200/mgag200_main.c | 4 ---- >>>> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +-- >>>> drivers/gpu/drm/udl/udl_drv.c | 6 +----- >>>> drivers/gpu/drm/vboxvideo/vbox_drv.c | 6 ++---- >>>> include/drm/drm_fb_helper.h | 5 +++-- >>>> 13 files changed, 25 insertions(+), 45 deletions(-) >>>> >>>> -- >>>> 2.26.0 > -- 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
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
