Re: [PATCH] drm/amdgpu: Add 'modeset' module parameter

2022-05-12 Thread Thomas Zimmermann

Hi

Am 11.05.22 um 20:19 schrieb Lyude Paul:

Many DRM drivers feature a 'modeset' argument, which can be used to
enable/disable the entire driver (as opposed to passing nomodeset to the
kernel, which would disable modesetting globally and make it difficult to
load amdgpu afterwards). Apparently amdgpu is actually missing this
however, so let's add it!


We have recently consolidated the handling of the modeset parameter in 
the macro drm_module_pci_driver_if_modeset(), [1]  which has a 
deprecation warning in the docs. Only a few older drivers use modeset 
and we don't want to use it further.


Better alternatives are nomodeset or initcall_blacklist=amdgpu_init.

Best regards
Thomas

[1] 
https://cgit.freedesktop.org/drm/drm-tip/tree/include/drm/drm_module.h#n79




Keep in mind that this currently just lets one enable or disable amdgpu, I
haven't bothered adding a headless mode like nouveau has - however I'm sure
someone else can add this if needed.

Signed-off-by: Lyude Paul 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index ebd37fb19cdb..24e6fb4517cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -872,6 +872,15 @@ MODULE_PARM_DESC(smu_pptable_id,
"specify pptable id to be used (-1 = auto(default) value, 0 = use pptable from 
vbios, > 0 = soft pptable id)");
  module_param_named(smu_pptable_id, amdgpu_smu_pptable_id, int, 0444);
  
+/**

+ * DOC: modeset (int)
+ * Used to enable/disable modesetting for amdgpu exclusively.
+ */
+bool amdgpu_enable_modeset = true;
+MODULE_PARM_DESC(modeset,
+"Enable or disable display driver (1 = on (default), 0 = off");
+module_param_named(modeset, amdgpu_enable_modeset, bool, 0444);
+
  /* These devices are not supported by amdgpu.
   * They are supported by the mach64, r128, radeon drivers
   */
@@ -2003,6 +2012,11 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
bool is_fw_fb;
resource_size_t base, size;
  
+	if (!amdgpu_enable_modeset) {

+   DRM_INFO("modeset=0 passed to amdgpu, driver will not be 
enabled\n");
+   return -ENODEV;
+   }
+
/* skip devices which are owned by radeon */
for (i = 0; i < ARRAY_SIZE(amdgpu_unsupported_pciidlist); i++) {
if (amdgpu_unsupported_pciidlist[i] == pdev->device)


--
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] drm/amdgpu: Add 'modeset' module parameter

2022-05-11 Thread Javier Martinez Canillas
On 5/12/22 08:17, Christian König wrote:
> Am 11.05.22 um 20:36 schrieb Alex Deucher:
>> On Wed, May 11, 2022 at 2:20 PM Lyude Paul  wrote:
>>> Many DRM drivers feature a 'modeset' argument, which can be used to
>>> enable/disable the entire driver (as opposed to passing nomodeset to the
>>> kernel, which would disable modesetting globally and make it difficult to
>>> load amdgpu afterwards). Apparently amdgpu is actually missing this
>>> however, so let's add it!
>> You can already do that by passing modprobe.blacklist=amdgpu on the
>> kernel command line.  I don't think we need another option to do that.
> 
> Yeah, this already came up multiple times and so far we have always 
> rejected it.
> 
> Stuffing that into drivers is not a good approach I think. If we want to 
> prevent some device from exposing it's display functionalities we should 
> rather push that into the drm layer.
>

Absolutely agree on this. I think what we want is a drm.modeset parameter
that would have more precedence than "nomodeset". Because the latter is a
built-in parameter and so it can't be disabled at runtime using sysfs.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat



Re: [PATCH] drm/amdgpu: Add 'modeset' module parameter

2022-05-11 Thread Christian König

Am 11.05.22 um 20:36 schrieb Alex Deucher:

On Wed, May 11, 2022 at 2:20 PM Lyude Paul  wrote:

Many DRM drivers feature a 'modeset' argument, which can be used to
enable/disable the entire driver (as opposed to passing nomodeset to the
kernel, which would disable modesetting globally and make it difficult to
load amdgpu afterwards). Apparently amdgpu is actually missing this
however, so let's add it!

You can already do that by passing modprobe.blacklist=amdgpu on the
kernel command line.  I don't think we need another option to do that.


Yeah, this already came up multiple times and so far we have always 
rejected it.


Stuffing that into drivers is not a good approach I think. If we want to 
prevent some device from exposing it's display functionalities we should 
rather push that into the drm layer.


Regards,
Christian.





Keep in mind that this currently just lets one enable or disable amdgpu, I
haven't bothered adding a headless mode like nouveau has - however I'm sure
someone else can add this if needed.

You can do this as well by passing an IP block mask which disables the
display IP, or media IP, etc.

Alex


Signed-off-by: Lyude Paul 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index ebd37fb19cdb..24e6fb4517cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -872,6 +872,15 @@ MODULE_PARM_DESC(smu_pptable_id,
 "specify pptable id to be used (-1 = auto(default) value, 0 = use pptable from 
vbios, > 0 = soft pptable id)");
  module_param_named(smu_pptable_id, amdgpu_smu_pptable_id, int, 0444);

+/**
+ * DOC: modeset (int)
+ * Used to enable/disable modesetting for amdgpu exclusively.
+ */
+bool amdgpu_enable_modeset = true;
+MODULE_PARM_DESC(modeset,
+"Enable or disable display driver (1 = on (default), 0 = off");
+module_param_named(modeset, amdgpu_enable_modeset, bool, 0444);
+
  /* These devices are not supported by amdgpu.
   * They are supported by the mach64, r128, radeon drivers
   */
@@ -2003,6 +2012,11 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
 bool is_fw_fb;
 resource_size_t base, size;

+   if (!amdgpu_enable_modeset) {
+   DRM_INFO("modeset=0 passed to amdgpu, driver will not be 
enabled\n");
+   return -ENODEV;
+   }
+
 /* skip devices which are owned by radeon */
 for (i = 0; i < ARRAY_SIZE(amdgpu_unsupported_pciidlist); i++) {
 if (amdgpu_unsupported_pciidlist[i] == pdev->device)
--
2.35.1





Re: [PATCH] drm/amdgpu: Add 'modeset' module parameter

2022-05-11 Thread Alex Deucher
On Wed, May 11, 2022 at 2:20 PM Lyude Paul  wrote:
>
> Many DRM drivers feature a 'modeset' argument, which can be used to
> enable/disable the entire driver (as opposed to passing nomodeset to the
> kernel, which would disable modesetting globally and make it difficult to
> load amdgpu afterwards). Apparently amdgpu is actually missing this
> however, so let's add it!

You can already do that by passing modprobe.blacklist=amdgpu on the
kernel command line.  I don't think we need another option to do that.


>
> Keep in mind that this currently just lets one enable or disable amdgpu, I
> haven't bothered adding a headless mode like nouveau has - however I'm sure
> someone else can add this if needed.

You can do this as well by passing an IP block mask which disables the
display IP, or media IP, etc.

Alex

>
> Signed-off-by: Lyude Paul 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index ebd37fb19cdb..24e6fb4517cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -872,6 +872,15 @@ MODULE_PARM_DESC(smu_pptable_id,
> "specify pptable id to be used (-1 = auto(default) value, 0 = use 
> pptable from vbios, > 0 = soft pptable id)");
>  module_param_named(smu_pptable_id, amdgpu_smu_pptable_id, int, 0444);
>
> +/**
> + * DOC: modeset (int)
> + * Used to enable/disable modesetting for amdgpu exclusively.
> + */
> +bool amdgpu_enable_modeset = true;
> +MODULE_PARM_DESC(modeset,
> +"Enable or disable display driver (1 = on (default), 0 = 
> off");
> +module_param_named(modeset, amdgpu_enable_modeset, bool, 0444);
> +
>  /* These devices are not supported by amdgpu.
>   * They are supported by the mach64, r128, radeon drivers
>   */
> @@ -2003,6 +2012,11 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> bool is_fw_fb;
> resource_size_t base, size;
>
> +   if (!amdgpu_enable_modeset) {
> +   DRM_INFO("modeset=0 passed to amdgpu, driver will not be 
> enabled\n");
> +   return -ENODEV;
> +   }
> +
> /* skip devices which are owned by radeon */
> for (i = 0; i < ARRAY_SIZE(amdgpu_unsupported_pciidlist); i++) {
> if (amdgpu_unsupported_pciidlist[i] == pdev->device)
> --
> 2.35.1
>


[PATCH] drm/amdgpu: Add 'modeset' module parameter

2022-05-11 Thread Lyude Paul
Many DRM drivers feature a 'modeset' argument, which can be used to
enable/disable the entire driver (as opposed to passing nomodeset to the
kernel, which would disable modesetting globally and make it difficult to
load amdgpu afterwards). Apparently amdgpu is actually missing this
however, so let's add it!

Keep in mind that this currently just lets one enable or disable amdgpu, I
haven't bothered adding a headless mode like nouveau has - however I'm sure
someone else can add this if needed.

Signed-off-by: Lyude Paul 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index ebd37fb19cdb..24e6fb4517cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -872,6 +872,15 @@ MODULE_PARM_DESC(smu_pptable_id,
"specify pptable id to be used (-1 = auto(default) value, 0 = use 
pptable from vbios, > 0 = soft pptable id)");
 module_param_named(smu_pptable_id, amdgpu_smu_pptable_id, int, 0444);
 
+/**
+ * DOC: modeset (int)
+ * Used to enable/disable modesetting for amdgpu exclusively.
+ */
+bool amdgpu_enable_modeset = true;
+MODULE_PARM_DESC(modeset,
+"Enable or disable display driver (1 = on (default), 0 = off");
+module_param_named(modeset, amdgpu_enable_modeset, bool, 0444);
+
 /* These devices are not supported by amdgpu.
  * They are supported by the mach64, r128, radeon drivers
  */
@@ -2003,6 +2012,11 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
bool is_fw_fb;
resource_size_t base, size;
 
+   if (!amdgpu_enable_modeset) {
+   DRM_INFO("modeset=0 passed to amdgpu, driver will not be 
enabled\n");
+   return -ENODEV;
+   }
+
/* skip devices which are owned by radeon */
for (i = 0; i < ARRAY_SIZE(amdgpu_unsupported_pciidlist); i++) {
if (amdgpu_unsupported_pciidlist[i] == pdev->device)
-- 
2.35.1