Re: [PATCH v5 0/7] Fix some races between sysfb device registration and drivers probe

From: Thomas Zimmermann <tzimmermann@suse.de>
To: Javier Martinez Canillas <javierm@redhat.com>,
	linux-kernel@vger.kernel.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	dri-devel@lists.freedesktop.org, Daniel Vetter <daniel@ffwll.ch>,
	Hans de Goede <hdegoede@redhat.com>, Helge Deller <deller@gmx.de>,
	Jonathan Corbet <corbet@lwn.net>, Peter Jones <pjones@redhat.com>,
	linux-doc@vger.kernel.org, linux-fbdev@vger.kernel.org
Subject: Re: [PATCH v5 0/7] Fix some races between sysfb device registration and drivers probe
Date: Fri, 13 May 2022 13:32:56 +0200	[thread overview]
Message-ID: <026b1c6d-c258-fa88-ed08-d1b5784c95b0@suse.de> (raw)
In-Reply-To: <cc0d6635-c739-490d-9c8d-7f53da48e61a@redhat.com>

[-- Attachment #1.1: Type: text/plain, Size: 2235 bytes --]

Hi Javier

Am 13.05.22 um 13:10 schrieb Javier Martinez Canillas:
...
>> We already track the memory ranges in drm aperture helpers. We'd need to
>> move the code to a more prominent location (e.g., <linux/aperture.h>)
>> and change fbdev to use it. Sysfb and DT code needs to insert platform
>> devices upon creation. We can then implement the more fancy stuff, such
>> as tracking native-device memory.  (And if we ever get to fix all usage
>> of Linux' request-mem-region, we can even move all the functionality there).
>>
> 
> Agreed. And as mentioned, the race that these patches attempt to fix are for
> the less common case when a native driver probes but either no generic driver
> registered a framebuffer yet or the platform device wasn't registered yet.
> 
> But this can only happen if for example a native driver is built-in but the
> generic driver is build as a module, which is not the common configuration.
> 
> What most distros do is the opposite, to have {simple,of,efi,vesa}fb or
> simpledrm built-in and the native drivers built as modules.
> 
> So there's no rush to fix this by piling more hacks on top of the ones we
> already have and instead try to fix it more properly as you suggested.
>   

A first step would be to use DRM's aperture helpers in fbdev. That would 
be a good idea anyway, as it would simplify both. You already mentioned 
some API changes to make aperture helpers DRM-independent.

The affected fbdev drivers use platform devices, so this should be easy.

Aperture helpers have something like the registration_lock. [1] I don't 
know if we need to recreate patch 3 for this as well.

If we absolutely need some special detachment handling for fbdev, we can 
make devm_aperture_aquire() a public interface. The detach helper is 
provided by the caller.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v5.17.6/source/drivers/gpu/drm/drm_aperture.c#L254
[2] 
https://elixir.bootlin.com/linux/v5.17.6/source/drivers/gpu/drm/drm_aperture.c#L159

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]