Hello Thomas,
Thanks for the feedback. It was very useful.
On 4/25/22 11:15, Thomas Zimmermann wrote:
> Hi
>
> Am 25.04.22 um 10:54 schrieb Thomas Zimmermann:
>> Hi
>>
>> Am 20.04.22 um 10:52 schrieb Javier Martinez Canillas:
>>> Hello,
>>>
>>> The patches in this series are mostly changes suggested by Daniel Vetter
>>> to fix some race conditions that exists between the fbdev core (fbmem)
>>> and sysfb with regard to device registration and removal.
>>>
>>> For example, it is currently possible for sysfb to register a platform
>>> device after a real DRM driver was registered and requested to remove the
>>> conflicting framebuffers.
>>>
>>> A symptom of this issue, was worked around with by commit fb561bf9abde
>>> ("fbdev: Prevent probing generic drivers if a FB is already registered")
>>> but that's really a hack and should be reverted.
>>
>> As I mentioned on IRC, I think this series should be merged for the
>> reasons I give in the other comments.
>>
You meant that should *not* get merged, as we discussed over IRC.
>>>
>>> This series attempt to fix it more properly and revert the mentioned
>>> hack.
>>> That will also unblock a pending patch to not make the num_registered_fb
>>> variable visible to drivers anymore, since that's internal to fbdev core.
>>
>> Here's as far as I understand the problem:
>>
>> 1) build DRM/fbdev and sysfb code into the kernel
>> 2) during boot, load the DRM/fbdev modules and have them acquire I/O
>> ranges
>> 3) afterwards load sysfb and have it register platform devices for the
>> generic framebuffers
>> 4) these devices now conflict with the already-registered DRM/fbdev
>> devices
>>
That's correct, yes.
>> If that is the problem here, let's simply set a sysfb_disable flag in
>> sysfb code when the first DRM/fbdev driver first loads. With the flag
>> set, sysfb won't create any platform devices. We assume that there are
>> now DRM/fbdev drivers for the framebuffers and sysfb won't be needed.
>>
>> We can set the flag internally from drm_aperture_detach_drivers() [1]
>> and do_remove_conflicting_framebuffers() [2].
>
> And further thinking about it, it would be better to set such a flag
> after successfully registering a DRM/fbdev device. So we know that
> there's at least one working display in the system. We don't have to
> rely on generic framebuffers after that.
>
Exactly, should be done when the device is registered rather than when
the driver is registered or a call is made to remove the conflicting FB.
I'll rework this series with only the bits for sysfb_disable() and drop
the rest. We can go back to the discussion of the remaining parts later
if that makes sense (I still think that patch 3/5 is a better approach,
but let's defer that for a different series).
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
