Re: [PATCH 00/10] Set up generic fbdev after registering device

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 07 Apr 2020, Thomas Zimmermann <tzimmermann@suse.de> 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).

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@ffwll.ch/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

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel