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

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1.1: Type: text/plain, Size: 3906 bytes --]

Hi Jani

Am 07.04.20 um 09:24 schrieb Jani Nikula:
> 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).

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@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
> 

-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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