Re: [PATCH v5 5/7] fbdev: Disable sysfb device registration when removing conflicting FBs

[Javier Martinez Canillas]

Hello Daniel,

On 6/7/22 17:01, Daniel Vetter wrote:

[snip]

>>
>>  drivers/video/fbdev/core/fbmem.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
>> index 9b035ef4d552..265efa189bcc 100644
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -1789,6 +1789,17 @@ int remove_conflicting_framebuffers(struct apertures_struct *a,
>>  	if (do_free)
>>  		kfree(a);
>>  
>> +	/*
>> +	 * If a driver asked to unregister a platform device registered by
>> +	 * sysfb, then can be assumed that this is a driver for a display
>> +	 * that is set up by the system firmware and has a generic driver.
>> +	 *
>> +	 * Drivers for devices that don't have a generic driver will never
>> +	 * ask for this, so let's assume that a real driver for the display
>> +	 * was already probed and prevent sysfb to register devices later.
>> +	 */
>> +	sysfb_disable();
> 
> So the og version had (or should have had at least) the sysfb_disable()
> call before we go through the loop and try to unregister stuff. I think
> this needs to be done before we call do_remove_conflicting_framebuffer()
> instead. With that:
>

Yes, the original version did that but also the original version didn't
attempt to remove the devices registered by sysfb on sysfb_disable().

I was going to answer that this has to be done after the loop because
that way fbmem could first ask sysfb to remove the devices, but then I
realized that you are correct That this wouldn't be needed if sysfb does
the disable (and unregistration) before the loop.

So by doing it before the loop, we should be able to drop [PATCH v5 4/7]
fbdev: Make sysfb to unregister its own registered devices:

https://lists.freedesktop.org/archives/dri-devel/2022-May/355201.html

Since by the time the loop is executed, no registered_fb associated with
a device that was registered by sysfb should be present in that array.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat