Re: [PATCH v2 31/37] drm: rcar-du: Add support for the nomodeset kernel parameter

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3639 bytes --]

Hi

Am 28.01.22 um 11:34 schrieb Laurent Pinchart:
> Hi Thomas,
> 
> On Fri, Jan 28, 2022 at 10:33:21AM +0100, Thomas Zimmermann wrote:
>> Am 28.01.22 um 10:13 schrieb Kieran Bingham:
>>> Quoting Javier Martinez Canillas (2021-12-17 00:37:46)
>>>> According to disable Documentation/admin-guide/kernel-parameters.txt, this
>>>> parameter can be used to disable kernel modesetting.
>>>>
>>>> DRM drivers will not perform display-mode changes or accelerated rendering
>>>> and only the system framebuffer will be available if it was set-up.
>>>
>>> What is the 'system framebuffer' in this instance? Reading
>>> https://www.kernel.org/doc/Documentation/admin-guide/kernel-parameters.txt
>>> it sounds like that means anything already set up by the bootloader.
>>
>> Exactly this.
>>
>>>> But only a few DRM drivers currently check for nomodeset, make this driver
>>>> to also support the command line parameter.
>>>>
>>>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>>>> ---
>>>>
>>>> (no changes since v1)
>>>>
>>>>    drivers/gpu/drm/rcar-du/rcar_du_drv.c | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>>> index 5a8131ef81d5..982e450233ed 100644
>>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>>> @@ -701,6 +701,9 @@ static struct platform_driver rcar_du_platform_driver = {
>>>>    
>>>>    static int __init rcar_du_init(void)
>>>>    {
>>>> +       if (drm_firmware_drivers_only())
>>>> +               return -ENODEV;
>>>> +
>>>
>>> This will completely disable all control of the display device when
>>> nomodeset is enabled.
>>>
>>> Is there any requirement for us to support outputting to the display if
>>> it was previously set up? presumably without setting or changing any
>>> modes, but simply allowing the existing frame to be updated?
>>
>> There's no requirement for your driver. We just want a parameter where
>> we can conveniently disable most of DRM's drivers and reduce it to a
>> minimum. Helps distributions to provide a simple fallback mode.  Most
>> PCI-based drivers already support that. Now we're added it to the other
>> drivers as well.
>>
>>>
>>> I think the implication is that 'firmware drivers' would mean a display
>>> could be updated through some firmware interface, which we won't have
>>> ... so it seems reasonable to accept that this whole driver can be
>>> disabled in that instance.
>>
>> It cannot be 'mode-setted'. We get a pre-configured framebuffer from the
>> firmware or bootloader. Whatever we draw there shows up on the screen.
> 
> I doubt that's going to work as you expect, clocks and regulators will
> get disabled at boot if not used by any driver.

Simpledrm and simplefb attach to these firmware framebuffers. Both 
drivers look at the device tree nodes to acquire the relevant clocks and 
regulators.

Best regards
Thomas

> 
>>> Reading your mail that brought this thread up in my inbox, I think
>>> you've already hit merge on this, so don't worry about adding a tag in
>>> that instance, but I think this is ok.
>>>
>>> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>>
>>>>           rcar_du_of_init(rcar_du_of_table);
>>>>    
>>>>           return platform_driver_register(&rcar_du_platform_driver);
> 

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]