Re: [PATCH] Fix failure of simpledrm probe when trying to grab FB from the EFI-based Framebuffer

[Javier Martinez Canillas]

Andrew Worsley <amworsley@xxxxxxxxx> writes:

> It's inline - part of the email - not an attachment?
>
> I can see it in the copy that went to me...
>
> Andrew
>
> On Sat, 11 Nov 2023 at 15:30, Andrew Worsley <amworsley@xxxxxxxxx> wrote:
>>
>>    The simpledrm.c does not call aperture_remove_conflicting_devices() in it's probe
>>    function as the drivers/video/aperture.c documentation says it should. Consequently
>>    it's request for the FB memory fails.
>>

The current behaviour is correct since aperture_remove_conflicting_devices()
is for native drivers to remove simple framebuffer devices such as simpledrm,
simplefb, efifb, etc.

>> ...
>> [    3.085302] simple-framebuffer bd58dc000.framebuffer: [drm] *ERROR* could not acquire memory range [??? 0xffff6e1d8629d580-0x2a5000001a7 flags 0x0]: -16
>> [    3.086433] simple-framebuffer: probe of bd58dc000.framebuffer failed with error -16
>> ...
>>

This is -EBUSY. What is your kernel configuration? Can you share it please.

>>    In my case no driver provided /dev/dri/card0 device is available on boot up and X
>>    fails to start as per this from X start up log.
>>
>> ...
>> [     5.616] (WW) Falling back to old probe method for modesetting
>> [     5.616] (EE) open /dev/dri/card0: No such file or directory
>> ...
>>
>>    Fault confirmed and fixed on Asahi 6.5.0 kernel with both CONFIG_FB_EFI and
>>    CONFIG_DRM_SIMPLEDRM config options set.
>>
>> Signed-off-by: Andrew Worsley <amworsley@xxxxxxxxx>
>> ---

I wonder if this is anothe side effect of commit 60aebc955949
("drivers/firmware: Move sysfb_init() from device_initcall to
subsys_initcall_sync").

Can you try reverting that one and see if it helps?

>>  drivers/gpu/drm/tiny/simpledrm.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
>> index 5fefc895bca2..e55a536b04cf 100644
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -8,6 +8,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_domain.h>
>>  #include <linux/regulator/consumer.h>
>> +#include <linux/aperture.h>
>>
>>  #include <drm/drm_aperture.h>
>>  #include <drm/drm_atomic.h>
>> @@ -828,6 +829,13 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>         if (mem) {
>>                 void *screen_base;
>>
>> +               ret = aperture_remove_conflicting_devices(mem->start, resource_size(mem),
>> +                       DRIVER_NAME);
>> +               if (ret) {

DRM drivers should use drm_aperture_remove_framebuffers() instead. But
this shouldn't be needed for the simpledrm driver as mentioned, since
there shouldn't be another device grabbing this aperture at this point.

I would rather try to understand what is going on in your setup and why 
the acquire is returning -EBUSY.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat