Re: [PATCH RESEND v4 v4 2/4] drm/vc4: Support nomodeset

From: Thomas Zimmermann <tzimmermann@suse.de>
To: Javier Martinez Canillas <javier@dowhile0.org>,
	Maxime Ripard <maxime@cerno.tech>
Cc: devicetree@vger.kernel.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	Dom Cobley <dom@raspberrypi.com>,
	Scott Branden <sbranden@broadcom.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	David Airlie <airlied@linux.ie>, Ray Jui <rjui@broadcom.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Rob Herring <robh+dt@kernel.org>,
	Nicolas Saenz Julienne <nsaenz@kernel.org>,
	bcm-kernel-feedback-list@broadcom.com,
	Tim Gover <tim.gover@raspberrypi.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Frank Rowand <frowand.list@gmail.com>,
	Phil Elwell <phil@raspberrypi.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-rpi-kernel@lists.infradead.org
Subject: Re: [PATCH RESEND v4 v4 2/4] drm/vc4: Support nomodeset
Date: Tue, 14 Dec 2021 09:57:46 +0100	[thread overview]
Message-ID: <2f01c8e7-8091-1cdc-6aa3-ce22974575f3@suse.de> (raw)
In-Reply-To: <CABxcv=ka4uc0Mt7Yv2RKnSYG74WRP--U1n4PhOjCentGEGkaMg@mail.gmail.com>

[-- Attachment #1.1.1: Type: text/plain, Size: 2246 bytes --]

Hi

Am 14.12.21 um 09:30 schrieb Javier Martinez Canillas:
> Hello Maxime
> 
> On Mon, Dec 13, 2021 at 5:26 PM Maxime Ripard <maxime@cerno.tech> wrote:
>>
>> If we have nomodeset on the kernel command line we should have the
>> firmware framebuffer driver kept as is and not try to load the
>> full-blown KMS driver.
>>
> 
> Patch looks good to me. I just have a question, but I'm OK with either way.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 
>> In this case, let's just register the v3d driver.
>>
> 
> I wonder if the v3d driver should be registered if nomodeset is
> present. Most (if not all?) drivers that currently check for this
> parameter disable both KMS and DRM. So even when it seems to imply
> that's about kernel mode settings, it is also used to disable DRM.
> 
> This semantic was never documented and I attempted to do that in
> commit b22a15a5aca3 ("Documentation/admin-guide: Document nomodeset
> kernel parameter"). After feedback from folks in the list, the text
> ended as follows:
> 
> ```
> Disable kernel modesetting. DRM drivers will not perform
> display-mode changes or accelerated rendering. Only the
> system framebuffer will be available for use if this was
> set-up by the firmware or boot loader.
> 
> Useful as fallback, or for testing and debugging.
> ```
> 
> So maybe vc4_drm_register() should just return -EINVAL if
> (drm_firmware_drivers_only()) like the other drivers do?

I second this comment. The intention of this test is to disable 
HW-native drivers if something goes wrong with the display.

The function's name drm_firmware_drivers_only() reflects that. The 
parameter is called nomodeset for historical reasons and it's probably a 
terrible name.

So I think the code should call drm_firmware_drivers_only() at the top 
and return an error is it's true. That's what we will do for other 
drivers as well. Maybe rather return -ENODEV; EINVAL mean 'invalid 
argument'.

Best regards
Thomas

> 
> Best regards,
> Javier
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel