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

2022-01-28 Thread Thomas Zimmermann

Hi

Am 28.01.22 um 12:36 schrieb Laurent Pinchart:

Hi Thomas,

On Fri, Jan 28, 2022 at 12:26:03PM +0100, Thomas Zimmermann wrote:

Am 28.01.22 um 12:04 schrieb Laurent Pinchart:

On Fri, Jan 28, 2022 at 11:46:49AM +0100, Thomas Zimmermann wrote:

Am 28.01.22 um 11:34 schrieb Laurent Pinchart:

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 
---

(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.


How about clocks and regulators for the ancillary devices, such as
encoders, or in the R-Car case, the external composer handled by the
vsp1 driver (in drivers/media/platform/vsp1) ?

This approach may work fine on x86 desktop systems, but for ARM-based
devices, the situation is usually more complex.


Well, in that case the problem has always been there. We don't make it
worse.


What's the point of adding nomodeset support in those drivers then, if
it's known not to work ?


But nomodeset is know to work: it disables the driver. The behavior is 
consistent among all hardware-specific drivers.


Javier mentioned that there are kernel parameters to keep unused 
clocks/regulators working.


From the distribution POV, if we have users with broken graphics 
drivers we want to give them an easy workaround until we find a 
solution. Passing 'nomodeset ...' on the command line gives us that.


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 


rcar_du_of_init(rcar_du_of_table);
 
return platform_driver_register(_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


OpenPGP_signature
Description: OpenPGP digital signature


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

2022-01-28 Thread Laurent Pinchart
Hi Thomas,

On Fri, Jan 28, 2022 at 12:26:03PM +0100, Thomas Zimmermann wrote:
> Am 28.01.22 um 12:04 schrieb Laurent Pinchart:
> > On Fri, Jan 28, 2022 at 11:46:49AM +0100, Thomas Zimmermann wrote:
> >> Am 28.01.22 um 11:34 schrieb Laurent Pinchart:
> >>> 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 
> >> ---
> >>
> >> (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.
> > 
> > How about clocks and regulators for the ancillary devices, such as
> > encoders, or in the R-Car case, the external composer handled by the
> > vsp1 driver (in drivers/media/platform/vsp1) ?
> > 
> > This approach may work fine on x86 desktop systems, but for ARM-based
> > devices, the situation is usually more complex.
> 
> Well, in that case the problem has always been there. We don't make it 
> worse.

What's the point of adding nomodeset support in those drivers then, if
it's known not to work ?

> > 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 
> >
> >>rcar_du_of_init(rcar_du_of_table);
> >> 
> >>return platform_driver_register(_du_platform_driver);

-- 
Regards,

Laurent Pinchart


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

2022-01-28 Thread Thomas Zimmermann

Hi

Am 28.01.22 um 12:04 schrieb Laurent Pinchart:

Hi Thomas,

On Fri, Jan 28, 2022 at 11:46:49AM +0100, Thomas Zimmermann wrote:

Am 28.01.22 um 11:34 schrieb Laurent Pinchart:

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 
---

(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.


How about clocks and regulators for the ancillary devices, such as
encoders, or in the R-Car case, the external composer handled by the
vsp1 driver (in drivers/media/platform/vsp1) ?

This approach may work fine on x86 desktop systems, but for ARM-based
devices, the situation is usually more complex.


Well, in that case the problem has always been there. We don't make it 
worse.


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 


   rcar_du_of_init(rcar_du_of_table);

   return platform_driver_register(_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


OpenPGP_signature
Description: OpenPGP digital signature


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

2022-01-28 Thread Javier Martinez Canillas
On 1/28/22 11:46, Thomas Zimmermann wrote:
> Am 28.01.22 um 11:34 schrieb Laurent Pinchart:

[snip]

>>>
>>> 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.
>

Laurent is correct that in some cases it might be an issue. For example, if
a SystemReady ES image is booted using the U-boot EFI stub instead of bootz.

In this case the system will be presented as EFI and the firmware framebuffer
just taken from the EFI GOP. There won't be any Device Tree node describing
the clocks, regulators, etc that are required for the framebuffer to work.

The user then will have to pass clk_ignore_unused, pd_ignore_unused and maybe
other cmdline params (or even hack a DT to set regulator-always-on for needed
regulators).

But that is also true if the user wants to disable the real DRM driver with
modprobe.blacklist=rcar-du-drm.

What this patch does is just to make all DRM drivers consistent and honour the
nomodeset param (whose name is really misleading but we can't change that).

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



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

2022-01-28 Thread Laurent Pinchart
Hi Thomas,

On Fri, Jan 28, 2022 at 11:46:49AM +0100, Thomas Zimmermann wrote:
> Am 28.01.22 um 11:34 schrieb Laurent Pinchart:
> > 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 
>  ---
> 
>  (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.

How about clocks and regulators for the ancillary devices, such as
encoders, or in the R-Car case, the external composer handled by the
vsp1 driver (in drivers/media/platform/vsp1) ?

This approach may work fine on x86 desktop systems, but for ARM-based
devices, the situation is usually more complex.

> >>> 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 
> >>>
>    rcar_du_of_init(rcar_du_of_table);
> 
>    return platform_driver_register(_du_platform_driver);

-- 
Regards,

Laurent Pinchart


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

2022-01-28 Thread Thomas Zimmermann

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 
---

(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 


  rcar_du_of_init(rcar_du_of_table);
   
  return platform_driver_register(_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


OpenPGP_signature
Description: OpenPGP digital signature


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

2022-01-28 Thread 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 
> >> ---
> >>
> >> (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.

> > 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 
> > 
> >>  rcar_du_of_init(rcar_du_of_table);
> >>   
> >>  return platform_driver_register(_du_platform_driver);

-- 
Regards,

Laurent Pinchart


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

2022-01-28 Thread Thomas Zimmermann

Hi

Am 28.01.22 um 10:13 schrieb Kieran Bingham:

Hi Javier,

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 
---

(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.


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 


 rcar_du_of_init(rcar_du_of_table);
  
 return platform_driver_register(_du_platform_driver);

--
2.33.1



--
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


OpenPGP_signature
Description: OpenPGP digital signature


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

2022-01-28 Thread Kieran Bingham
Hi Javier,

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.

> 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 
> ---
> 
> (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?

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.

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 

> rcar_du_of_init(rcar_du_of_table);
>  
> return platform_driver_register(_du_platform_driver);
> -- 
> 2.33.1
>


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

2021-12-16 Thread Javier Martinez Canillas
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.

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 
---

(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;
+
rcar_du_of_init(rcar_du_of_table);
 
return platform_driver_register(_du_platform_driver);
-- 
2.33.1