[PATCH] drm/xe: Support 'nomodeset' kernel command-line option

[Lucas De Marchi]

On Wed, Aug 21, 2024 at 04:48:23PM GMT, Thomas Zimmermann wrote:
>Hi
>
>Am 21.08.24 um 16:29 schrieb Jani Nikula:
>>On Wed, 21 Aug 2024, Gustavo Sousa <gustavo.sousa at intel.com> wrote:
>>>Quoting Thomas Zimmermann (2024-08-21 10:56:59-03:00)
>>>>Setting 'nomodeset' on the kernel command line disables all graphics
>>>>drivers with modesetting capabilities; leaving only firmware drivers,
>>>>such as simpledrm or efifb.
>>>>
>>>>Most DRM drivers automatically support 'nomodeset' via DRM's module
>>>>helper macros. In xe, which uses regular module_init(), manually call
>>>>drm_firmware_drivers_only() to test for 'nomodeset'. Do not register
>>>>the driver if set.
>>>>
>>>>Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>>>>---
>>>>drivers/gpu/drm/xe/xe_module.c | 5 +++++
>>>>1 file changed, 5 insertions(+)
>>>>
>>>>diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
>>>>index 923460119cec..60fb7dd26903 100644
>>>>--- a/drivers/gpu/drm/xe/xe_module.c
>>>>+++ b/drivers/gpu/drm/xe/xe_module.c
>>>>@@ -8,6 +8,8 @@
>>>>#include <linux/init.h>
>>>>#include <linux/module.h>
>>>>
>>>>+#include <drm/drm_module.h>
>>>>+
>>>>#include "xe_drv.h"
>>>>#include "xe_hw_fence.h"
>>>>#include "xe_pci.h"
>>>>@@ -92,6 +94,9 @@ static int __init xe_init(void)
>>>>{
>>>>         int err, i;
>>>>
>>>>+        if (drm_firmware_drivers_only())
>>>>+                return -ENODEV;
>>>>+
>>>Hm... But what if xe is to be used only for compute or render? Shouldn't
>>>we handle this somewhere else?
>>The question becomes, what does "nomodeset" really mean here?
>
>That function's name 'firmware drivers only' says it better than the 
>option's name. We used 'nomodeset', because it was there already and 
>had the correct semantics.

agreed this should be on a module-level to maintain the behavior already
used. If we were not maintaining that behavior, then we should probably
not use "nomodeset" and choose something else :).

Also we already have the other 2 as module params:  probe_display and
disable_display, with driver still registering as a drm driver, but
leaving the display part out.

Thomas, are you going to send a v2 to use the init table?

thanks
Lucas De Marchi

>
>>
>>See what i915 does in i915_module.c.
>
>i915 and the other drivers for PCI-based hardware don't load at all. 
>Drivers for external displays (e.g., SPI, USB) ignore nomodeset, as 
>these displays are not initialized by firmware.
>
>Best regards
>Thomas
>
>>
>>Cc: Sima.
>>
>>BR,
>>Jani.
>>
>>
>>
>>>Taking a quick look, xe_display_probe() might be a good candidate?
>>>
>>>--
>>>Gustavo Sousa
>>>
>>>>         for (i = 0; i < ARRAY_SIZE(init_funcs); i++) {
>>>>                 err = init_funcs[i].init();
>>>>                 if (err) {
>>>>-- 
>>>>2.46.0
>>>>
>
>-- 
>--
>Thomas Zimmermann
>Graphics Driver Developer
>SUSE Software Solutions Germany GmbH
>Frankenstrasse 146, 90461 Nuernberg, Germany
>GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>HRB 36809 (AG Nuernberg)
>