Re: [RFC PATCH] drm/aperture: Add param to disable conflicting framebuffers removal

[Javier Martinez Canillas <javierm@redhat.com>]

Hello Michel,

On 10/25/21 12:45, Michel Dänzer wrote:
> On 2021-10-24 22:32, Javier Martinez Canillas wrote:
>> Hello Ville,
>>
>> On 10/22/21 21:12, Ville Syrjälä wrote:
>>> On Fri, Oct 22, 2021 at 04:40:40PM +0200, Javier Martinez Canillas wrote:
>>>> The simpledrm driver allows to use the frame buffer that was set-up by the
>>>> firmware. This gives early video output before the platform DRM driver is
>>>> probed and takes over.
>>>>
>>>> But it would be useful to have a way to disable this take over by the real
>>>> DRM drivers. For example, there may be bugs in the DRM drivers that could
>>>> cause the display output to not work correctly.
>>>>
>>>> For those cases, it would be good to keep the simpledrm driver instead and
>>>> at least get a working display as set-up by the firmware.
>>>>
>>>> Let's add a drm.remove_fb boolean kernel command line parameter, that when
>>>> set to false will prevent the conflicting framebuffers to being removed.
>>>>
>>>> Since the drivers call drm_aperture_remove_conflicting_framebuffers() very
>>>> early in their probe callback, this will cause the drivers' probe to fail.
>>>
>>> Why is that better than just modprobe.blacklisting those drivers?
>>
>> Because would allow to deny list all native (as Thomas called it) DRM drivers
>> and only allow the simpledrm driver to be probed. This is useful for distros,
>> since could add a "Basic graphics mode" to the boot menu entries, that could
>> boot the kernel passing a "drm.disable_native_drivers=1" cmdline option.
>>
>> That way, if there's any problem with a given DRM driver, the distro may be
>> installed and booted using the simpledrm driver and troubleshoot why a native
>> DRM driver is not working. Or try updating the kernel package, etc.
> 
> For troubleshooting, it'll be helpful if this new parameter can be enabled for the boot via the kernel command line, then disabled again after boot-up. One simple possibility for this would be allowing the parameter to be changed via /sys/module

That's already the case with the current patch, i.e:

$ grep -o drm.* /proc/cmdline 
drm.disable_native_drivers=1

$ cat /proc/fb 
0 simpledrm

$ modprobe virtio_gpu

$ dmesg
[  125.731549] [drm] pci: virtio-vga detected at 0000:00:01.0
[  125.732410] virtio_gpu: probe of virtio0 failed with error -16

$ echo 0 > /sys/module/drm/parameters/disable_native_drivers

$ modprobe virtio_gpu

$ dmesg 
[  187.889136] [drm] pci: virtio-vga detected at 0000:00:01.0
[  187.894578] Console: switching to colour dummy device 80x25
[  187.897090] virtio-pci 0000:00:01.0: vgaarb: deactivate vga console
[  187.899983] [drm] features: -virgl +edid -resource_blob -host_visible
[  187.907176] [drm] number of scanouts: 1
[  187.907714] [drm] number of cap sets: 0
[  187.914108] [drm] Initialized virtio_gpu 0.1.0 0 for virtio0 on minor 1
[  187.930807] Console: switching to colour frame buffer device 128x48
[  187.938737] virtio_gpu virtio0: [drm] fb0: virtio_gpu frame buffer device

$ cat /proc/fb 
0 virtio_gpu

/drm/parameters/<name>, which I suspect doesn't work with the patch as is (due to the 0600 permissions).
> 
> 

I followed the convention used by other drm module parameters, hence the
0600. Do you mean that for this parameter we should be less restrictive ?

Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat