Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 22/05/17 10:54 AM, Michel Dänzer wrote:
> On 20/05/17 03:55 AM, Felix Kuehling wrote:
>> On 17-05-18 09:17 PM, Michel Dänzer wrote:
> 
> The default at this point should possibly still be for CIK GPUs to be
> driven by radeon, even if CONFIG_DRM_AMDGPU_CIK is enabled;
 Alex and Christian seem to think otherwise.
>>> FWIW, on my AMD notebook (HP EliteBook 725 G2, Kaveri), suspend/resume
>>> with amdgpu results in a black screen (can reboot blindly); works fine
>>> with radeon.
>>
>> Has this problem been reported anywhere? Is anyone working on fixing it?
>> If it's not reported, it will never be fixed.
> 
> You're right, I'll file a bug report.

I get the same error lines as described in
https://bugs.freedesktop.org/100949, I'm trying to get clarification if
it's the same bug.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 20/05/17 03:55 AM, Felix Kuehling wrote:
> On 17-05-18 09:17 PM, Michel Dänzer wrote:
>> Obviously, out-of-tree module builds will have to continue doing what
>> they've been doing so far with kernels without your patch. I'm thinking
>> about making it easier for them with kernels which do have your patch.
>> At some point in the future, maybe the support for kernels without your
>> patch can be dropped entirely.
> 
> I think out-of-tree builds using the CONFIG_AMDGPU_CIK option of the
> target kernel is not safe. Hybrid should use its own option that is
> determined by what the hybrid stack supports (kernel and user mode).

Sure, that's not what I mean. I mean out-of-tree builds can use

 options radeon ci_support=0 si_support=0

e.g. in /etc/modprobe.d/amdgpu-dkms.conf to prefer amdgpu in a cleaner
way than by blacklisting radeon.


 The default at this point should possibly still be for CIK GPUs to be
 driven by radeon, even if CONFIG_DRM_AMDGPU_CIK is enabled;
>>> Alex and Christian seem to think otherwise.
>> FWIW, on my AMD notebook (HP EliteBook 725 G2, Kaveri), suspend/resume
>> with amdgpu results in a black screen (can reboot blindly); works fine
>> with radeon.
> 
> Has this problem been reported anywhere? Is anyone working on fixing it?
> If it's not reported, it will never be fixed.

You're right, I'll file a bug report.


Another issue is the lack of HDMI / DP audio support without DC. It
seems weird to change the default to amdgpu before there is feature parity.


Anyway, go ahead and land your latest series, you can have my

Acked-by: Michel Dänzer 

for that. I'll propose some changes on top of that.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 19/05/17 04:35 AM, Felix Kuehling wrote:
> On 17-04-27 05:22 AM, Michel Dänzer wrote:
>> On 27/04/17 05:15 AM, Felix Kuehling wrote:
>>> Hi Michel,
>>>
>>> You said in an earlier email that it doesn't have to be convenient. With
>>> that in mind, I went back to a minimalistic solution that doesn't need
>>> any additional Kconfig options and only uses two module parameters (one
>>> in radeon, one in amdgpu).
>>>
>>> I'm attaching my WIP patches for reference (currently based on
>>> amd-kfd-staging). For an official review I'd rebase them on
>>> amd-staging-4.9, remove the #if-0-ed hack that didn't work out, and add
>>> the same for SI.
>>>
>>> Can everyone can agree that this is good enough?
>> Yes, something like this could be a good first step in the right
>> direction. Some comments:
>>
>> The radeon options should be available regardless of
>> CONFIG_DRM_AMDGPU_CIK/SI, or they won't be useful for amdgpu-pro / other
>> out-of-tree amdgpu builds.
> 
> Hmm, when an amdgpu-pro build is installed on a system with an older
> kernel, radeon won't have the module option at all. Unless the pro-build
> blacklists radeon, or replaces radeon with its own version, it will
> always have to be compiled without CIK support.
> 
> Therefore, I think my patch, meant for upstream, should not consider
> this case.

Obviously, out-of-tree module builds will have to continue doing what
they've been doing so far with kernels without your patch. I'm thinking
about making it easier for them with kernels which do have your patch.
At some point in the future, maybe the support for kernels without your
patch can be dropped entirely.


>> The default at this point should possibly still be for CIK GPUs to be
>> driven by radeon, even if CONFIG_DRM_AMDGPU_CIK is enabled;
> 
> Alex and Christian seem to think otherwise.

FWIW, on my AMD notebook (HP EliteBook 725 G2, Kaveri), suspend/resume
with amdgpu results in a black screen (can reboot blindly); works fine
with radeon.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Felix Kuehling]

On 17-04-27 05:22 AM, Michel Dänzer wrote:
> On 27/04/17 05:15 AM, Felix Kuehling wrote:
>> Hi Michel,
>>
>> You said in an earlier email that it doesn't have to be convenient. With
>> that in mind, I went back to a minimalistic solution that doesn't need
>> any additional Kconfig options and only uses two module parameters (one
>> in radeon, one in amdgpu).
>>
>> I'm attaching my WIP patches for reference (currently based on
>> amd-kfd-staging). For an official review I'd rebase them on
>> amd-staging-4.9, remove the #if-0-ed hack that didn't work out, and add
>> the same for SI.
>>
>> Can everyone can agree that this is good enough?
> Yes, something like this could be a good first step in the right
> direction. Some comments:
>
> The radeon options should be available regardless of
> CONFIG_DRM_AMDGPU_CIK/SI, or they won't be useful for amdgpu-pro / other
> out-of-tree amdgpu builds.

Hmm, when an amdgpu-pro build is installed on a system with an older
kernel, radeon won't have the module option at all. Unless the pro-build
blacklists radeon, or replaces radeon with its own version, it will
always have to be compiled without CIK support.

Therefore, I think my patch, meant for upstream, should not consider
this case.

>
> The default at this point should possibly still be for CIK GPUs to be
> driven by radeon, even if CONFIG_DRM_AMDGPU_CIK is enabled;

Alex and Christian seem to think otherwise.

Regards,
  Felix

>  more
> definitely so for SI. Then we can flip the default and remove
> CONFIG_DRM_AMDGPU_CIK/SI at some point in the future, and (much) later
> maybe remove the CIK/SI code from radeon.
>
>

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 27/04/17 05:15 AM, Felix Kuehling wrote:
> 
> Hi Michel,
> 
> You said in an earlier email that it doesn't have to be convenient. With
> that in mind, I went back to a minimalistic solution that doesn't need
> any additional Kconfig options and only uses two module parameters (one
> in radeon, one in amdgpu).
> 
> I'm attaching my WIP patches for reference (currently based on
> amd-kfd-staging). For an official review I'd rebase them on
> amd-staging-4.9, remove the #if-0-ed hack that didn't work out, and add
> the same for SI.
> 
> Can everyone can agree that this is good enough?

Yes, something like this could be a good first step in the right
direction. Some comments:

The radeon options should be available regardless of
CONFIG_DRM_AMDGPU_CIK/SI, or they won't be useful for amdgpu-pro / other
out-of-tree amdgpu builds.

The default at this point should possibly still be for CIK GPUs to be
driven by radeon, even if CONFIG_DRM_AMDGPU_CIK is enabled; more
definitely so for SI. Then we can flip the default and remove
CONFIG_DRM_AMDGPU_CIK/SI at some point in the future, and (much) later
maybe remove the CIK/SI code from radeon.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Christian König]

Am 26.04.2017 um 22:15 schrieb Felix Kuehling:

On 17-04-25 02:28 AM, Michel Dänzer wrote:

On 22/04/17 02:05 AM, Felix Kuehling wrote:

__setup doesn't work in modules.

Right. We could build something like
drivers/video/fbdev/core/fb_cmdline.c:video_setup() into the kernel to
handle this, but it's a bit ugly, which is one reason why I was leaning
towards:



s8250_options is only compiled if the driver is not a module.

That doesn't prevent us from using __module_param_call directly, does it?

Although, that still doesn't work as I envision if only one driver's
option is set e.g. in /etc/modprobe.d/*.conf .


So, I'm starting to think we need a shared module for this, which
provides one or multiple module parameters to choose which driver to use
for CIK/SI[0], and provides the result to the amdgpu/radeon drivers.
That might make things easier for amdgpu-pro / other standalone amdgpu
versions in the long run as well, as they could add files to
/etc/modprobe.d/ choosing themselves by default, without having to
blacklist radeon.

What do you guys think?

Hi Michel,

You said in an earlier email that it doesn't have to be convenient. With
that in mind, I went back to a minimalistic solution that doesn't need
any additional Kconfig options and only uses two module parameters (one
in radeon, one in amdgpu).

I'm attaching my WIP patches for reference (currently based on
amd-kfd-staging). For an official review I'd rebase them on
amd-staging-4.9, remove the #if-0-ed hack that didn't work out, and add
the same for SI.

Can everyone can agree that this is good enough?


It's certainly good enough for me.

With the disabled dummy code removed feel free to stick a Reviewed-by: 
Christian König  at those patches.


Regards,
Christian.



Regards,
   Felix



[0] or possibly even more fine-grained in the future




___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Felix Kuehling]

On 17-04-25 02:28 AM, Michel Dänzer wrote:
> On 22/04/17 02:05 AM, Felix Kuehling wrote:
>> __setup doesn't work in modules.
> Right. We could build something like
> drivers/video/fbdev/core/fb_cmdline.c:video_setup() into the kernel to
> handle this, but it's a bit ugly, which is one reason why I was leaning
> towards:
>
>
>> s8250_options is only compiled if the driver is not a module.
> That doesn't prevent us from using __module_param_call directly, does it?
>
> Although, that still doesn't work as I envision if only one driver's
> option is set e.g. in /etc/modprobe.d/*.conf .
>
>
> So, I'm starting to think we need a shared module for this, which
> provides one or multiple module parameters to choose which driver to use
> for CIK/SI[0], and provides the result to the amdgpu/radeon drivers.
> That might make things easier for amdgpu-pro / other standalone amdgpu
> versions in the long run as well, as they could add files to
> /etc/modprobe.d/ choosing themselves by default, without having to
> blacklist radeon.
>
> What do you guys think?

Hi Michel,

You said in an earlier email that it doesn't have to be convenient. With
that in mind, I went back to a minimalistic solution that doesn't need
any additional Kconfig options and only uses two module parameters (one
in radeon, one in amdgpu).

I'm attaching my WIP patches for reference (currently based on
amd-kfd-staging). For an official review I'd rebase them on
amd-staging-4.9, remove the #if-0-ed hack that didn't work out, and add
the same for SI.

Can everyone can agree that this is good enough?

Regards,
  Felix

>
>
> [0] or possibly even more fine-grained in the future
>

>From e1d5fc320dbe2f60d922c466986863489f4bfe77 Mon Sep 17 00:00:00 2001
From: Felix Kuehling 
Date: Thu, 20 Apr 2017 14:41:34 -0400
Subject: [PATCH 1/2] drm/radeon: Add module param to control CIK support

If AMDGPU supports CIK, add a module parameter to control CIK
support in radeon. It's off by default in radeon, while it will be
on by default in AMDGPU.

Change-Id: Ib594f7359b9b3143c859b3be5c0244a9a2a773cd
Signed-off-by: Felix Kuehling 
---
 drivers/gpu/drm/radeon/radeon.h |  4 
 drivers/gpu/drm/radeon/radeon_drv.c |  6 ++
 drivers/gpu/drm/radeon/radeon_kms.c | 18 ++
 3 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 5f16bf3..a05aaea 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -116,6 +116,10 @@
 extern int radeon_uvd;
 extern int radeon_vce;
 
+#ifdef CONFIG_DRM_AMDGPU_CIK
+extern int radeon_cik_support;
+#endif
+
 /*
  * Copy from radeon_drv.h so we don't have to include both and have conflicting
  * symbol;
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 2e5d680..2cdb01b 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -301,6 +301,12 @@ static inline void radeon_unregister_atpx_handler(void) {}
 MODULE_PARM_DESC(vce, "vce enable/disable vce support (1 = enable, 0 = disable)");
 module_param_named(vce, radeon_vce, int, 0444);
 
+#ifdef CONFIG_DRM_AMDGPU_CIK
+int radeon_cik_support = 0;
+MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled (default))");
+module_param_named(cik_support, radeon_cik_support, int, 0444);
+#endif
+
 static struct pci_device_id pciidlist[] = {
 	radeon_PCI_IDS
 };
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index d6d58bc..28ab255 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -99,6 +99,24 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	struct radeon_device *rdev;
 	int r, acpi_status;
 
+#ifdef CONFIG_DRM_AMDGPU_CIK
+	if (!radeon_cik_support) {
+		switch (flags & RADEON_FAMILY_MASK) {
+		case CHIP_KAVERI:
+		case CHIP_BONAIRE:
+		case CHIP_HAWAII:
+		case CHIP_KABINI:
+		case CHIP_MULLINS:
+			dev_warn(dev->dev,
+ "CIK support provided by amdgpu.\n");
+			dev_warn(dev->dev,
+		"Use radeon.cik_support=1 amdgpu.cik_support=0 to override.\n"
+);
+			return -ENODEV;
+		}
+	}
+#endif
+
 	rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
 	if (rdev == NULL) {
 		return -ENOMEM;
-- 
1.9.1

>From 18d4b8b6282215073c3c6762f36c9009b1218f3c Mon Sep 17 00:00:00 2001
From: Felix Kuehling 
Date: Thu, 20 Apr 2017 14:42:04 -0400
Subject: [PATCH 2/2] drm/amdgpu: Add module param to control CIK support

If AMDGPU supports CIK, add a module parameter to control CIK
support. It's on by default in AMDGPU, while it is off by default
in radeon.

Change-Id: Ica1296beae96b1c5f48698c06cfd2453fe1ff080
Signed-off-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 41 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 26/04/17 03:06 AM, Nicolai Hähnle wrote:
> On 25.04.2017 08:28, Michel Dänzer wrote:
>> On 22/04/17 02:05 AM, Felix Kuehling wrote:
>>
>> So, I'm starting to think we need a shared module for this, which
>> provides one or multiple module parameters to choose which driver to use
>> for CIK/SI[0], and provides the result to the amdgpu/radeon drivers.
>> That might make things easier for amdgpu-pro / other standalone amdgpu
>> versions in the long run as well, as they could add files to
>> /etc/modprobe.d/ choosing themselves by default, without having to
>> blacklist radeon.
>>
>> What do you guys think?
> 
> I suspect that adding an entire module

That sounds quite dramatic. :) Which particular aspect(s) of that are
you concerned about?


> just to select between two other modules

FWIW, I suspect that could actually end up smaller overall than the
alternatives we've discussed at least by some metrics.

And maybe this module could be used for sharing / coordinating more
stuff between the drivers in the future.


> is the kind of thing that should be discussed in a wider audience first.

I'd like to have at least a proof of concept before that.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Nicolai Hähnle]

On 25.04.2017 08:28, Michel Dänzer wrote:

On 22/04/17 02:05 AM, Felix Kuehling wrote:

__setup doesn't work in modules.


Right. We could build something like
drivers/video/fbdev/core/fb_cmdline.c:video_setup() into the kernel to
handle this, but it's a bit ugly, which is one reason why I was leaning
towards:



s8250_options is only compiled if the driver is not a module.


That doesn't prevent us from using __module_param_call directly, does it?

Although, that still doesn't work as I envision if only one driver's
option is set e.g. in /etc/modprobe.d/*.conf .


So, I'm starting to think we need a shared module for this, which
provides one or multiple module parameters to choose which driver to use
for CIK/SI[0], and provides the result to the amdgpu/radeon drivers.
That might make things easier for amdgpu-pro / other standalone amdgpu
versions in the long run as well, as they could add files to
/etc/modprobe.d/ choosing themselves by default, without having to
blacklist radeon.

What do you guys think?


I suspect that adding an entire module just to select between two other 
modules is the kind of thing that should be discussed in a wider 
audience first.


It is probably the cleanest solution that doesn't require teaching the 
general modprobe architecture about having multiple modules for the same 
PCI ID...


Cheers,
Nicolai





[0] or possibly even more fine-grained in the future




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 22/04/17 02:05 AM, Felix Kuehling wrote:
> __setup doesn't work in modules.

Right. We could build something like
drivers/video/fbdev/core/fb_cmdline.c:video_setup() into the kernel to
handle this, but it's a bit ugly, which is one reason why I was leaning
towards:


> s8250_options is only compiled if the driver is not a module.

That doesn't prevent us from using __module_param_call directly, does it?

Although, that still doesn't work as I envision if only one driver's
option is set e.g. in /etc/modprobe.d/*.conf .


So, I'm starting to think we need a shared module for this, which
provides one or multiple module parameters to choose which driver to use
for CIK/SI[0], and provides the result to the amdgpu/radeon drivers.
That might make things easier for amdgpu-pro / other standalone amdgpu
versions in the long run as well, as they could add files to
/etc/modprobe.d/ choosing themselves by default, without having to
blacklist radeon.

What do you guys think?


[0] or possibly even more fine-grained in the future

-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Felix Kuehling]

__setup doesn't work in modules.

s8250_options is only compiled if the driver is not a module.

I came up with a different way to parse another module's options. But I
can't get the commandline that I would parse in that way:

@@ -787,12 +793,45 @@ long amdgpu_drm_ioctl(struct file *filp,
.driver.pm = _pm_ops,
 };
 
-
+#if 0
+static int radeon_cik_support = 0;
+static int param_set(const char *val, const struct kernel_param *kp)
+{
+   return kstrtoint(val, 0, kp.arg);
+}
+static struct kernel_param_ops radeon_force_cik_param_ops = {
+   .flags = 0,
+   .set = param_set,
+   .get = NULL,
+   .free = NULL
+};
+static struct kernel_param radeon_cik_support_param = {
+   .name = "radeon.cik_support",
+   .mod = THIS_MODULE,
+   .ops = radeon_cik_support_param_ops,
+   .perm = 0444,
+   .level = -1,
+   .flags = 0,
+   .arg = _cik_support
+};
+#endif
 
 static int __init amdgpu_init(void)
 {
int r;
 
+#if 0
+   /* Doesn't work because saved_command_line isn't exported to
+* modules */
+   r = parse_args("amdgpu", saved_command_line,
+  _cik_support_param, 1,
+  -32768, 32767, NULL, NULL);
+   if (r)
+   return r;
+
+   amdgpu_cik_support = !radeon_cik_support;
+#endif
+
r = amdgpu_sync_init();
if (r)
goto error_sync;

Regards,
  Felix

On 17-04-20 05:25 AM, Michel Dänzer wrote:
> On 20/04/17 03:59 AM, Felix Kuehling wrote:
>> On 17-04-11 10:23 PM, Michel Dänzer wrote:
>>> One possibility would be making each driver also parse the other
>>> driver's module parameter on the kernel command line. I.e. radeon would
>>> parse
>>>
>>>  amdgpu.enable_cik=0
>> I looked for a way to do this. I think I figured out the parsing part.
>> But I can't see where to get the kernel command line from. The command
>> line that is parsed by modules in load_module comes from user mode, and
>> I think it's pre-processed by modprobe to strip the "." prefix
>> and only include the module-specific options.
>>
>> There is a global variable saved_command_line, which is what's used for
>> /proc/cmdline. But that variable is not exported to modules.
> I see two possibilities:
>
> Either use __module_param_call directly, like
> drivers/tty/serial/8250/8250_core.c:s8250_options().
>
> Or use __setup, like e.g.
> drivers/video/fbdev/core/fb_cmdline.c:video_setup().
>
> I'm leaning towards the former.
>
>
> Anyway, my offer to look into this as a follow-up change stands; the
> only issue that needs to be resolved for your patch to land is the
> Kconfig options.
>
>

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 20/04/17 03:59 AM, Felix Kuehling wrote:
> On 17-04-11 10:23 PM, Michel Dänzer wrote:
>> One possibility would be making each driver also parse the other
>> driver's module parameter on the kernel command line. I.e. radeon would
>> parse
>>
>>  amdgpu.enable_cik=0
> I looked for a way to do this. I think I figured out the parsing part.
> But I can't see where to get the kernel command line from. The command
> line that is parsed by modules in load_module comes from user mode, and
> I think it's pre-processed by modprobe to strip the "." prefix
> and only include the module-specific options.
> 
> There is a global variable saved_command_line, which is what's used for
> /proc/cmdline. But that variable is not exported to modules.

I see two possibilities:

Either use __module_param_call directly, like
drivers/tty/serial/8250/8250_core.c:s8250_options().

Or use __setup, like e.g.
drivers/video/fbdev/core/fb_cmdline.c:video_setup().

I'm leaning towards the former.


Anyway, my offer to look into this as a follow-up change stands; the
only issue that needs to be resolved for your patch to land is the
Kconfig options.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Felix Kuehling]

On 17-04-11 10:23 PM, Michel Dänzer wrote:
> One possibility would be making each driver also parse the other
> driver's module parameter on the kernel command line. I.e. radeon would
> parse
>
>  amdgpu.enable_cik=0
I looked for a way to do this. I think I figured out the parsing part.
But I can't see where to get the kernel command line from. The command
line that is parsed by modules in load_module comes from user mode, and
I think it's pre-processed by modprobe to strip the "." prefix
and only include the module-specific options.

There is a global variable saved_command_line, which is what's used for
/proc/cmdline. But that variable is not exported to modules.

Regards,
  Felix
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 11/04/17 10:38 AM, Michel Dänzer wrote:
> On 11/04/17 08:21 AM, Felix Kuehling wrote:
>> Provide convenient compile time and boot time options for selecting
>> CIK ASIC support in either or both drivers.
>>
>> v2: git add missing files
> 
> [...]
> 
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 483059a..f85f81c 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -148,6 +148,57 @@ config DRM_AMDGPU
>>  
>>  source "drivers/gpu/drm/amd/amdgpu/Kconfig"
>>  
>> +choice DRM_CIK_SUPPORT
>> +prompt "Support for AMD CIK ASICs"
>> +default DRM_CIK_RADEON
>> +depends on DRM_AMDGPU || DRM_RADEON
>> +help
>> +  Choose the driver used to support AMD CIK ASICs.
>> +
>> +config DRM_CIK_RADEON
>> +bool "Radeon"
>> +depends on DRM_RADEON
>> +select DRM_RADEON_CIK
>> +select DRM_RADEON_CIK_ENABLED
>> +help
>> +  The Radeon driver was traditionally used to support CIK ASICs.
>> +
>> +config DRM_CIK_AMDGPU
>> +bool "AMD GPU"
>> +depends on DRM_AMDGPU
>> +select DRM_AMDGPU_CIK
>> +select DRM_AMDGPU_CIK_ENABLED
>> +help
>> +  The AMD GPU driver has more active development for features and
>> +  performance. If you choose this driver, you also need the amdgpu
>> +  DDX driver for X.org.
>> +
>> +config DRM_CIK_BOTH_DEFAULT_RADEON
>> +bool "Both, use Radeon by default"
>> +depends on DRM_AMDGPU && DRM_RADEON
>> +select DRM_RADEON_CIK
>> +select DRM_RADEON_CIK_ENABLED
>> +select DRM_AMDGPU_CIK
>> +help
>> +  This option is useful for driver developers who want to test
>> +  both drivers while running the same kernel. The active driver
>> +  can be selected using the module parameters radeon.enable_cik
>> +  and amdgpu.enable_cik.
>> +
>> +config DRM_CIK_BOTH_DEFAULT_AMDGPU
>> +bool "Both, use AMD GPU by default"
>> +depends on DRM_AMDGPU && DRM_RADEON
>> +select DRM_RADEON_CIK
>> +select DRM_AMDGPU_CIK
>> +select DRM_AMDGPU_CIK_ENABLED
>> +help
>> +  This option is useful for driver developers who want to test
>> +  both drivers while running the same kernel. The active driver
>> +  can be selected using the module parameters radeon.enable_cik
>> +  and amdgpu.enable_cik.
>> +
>> +endchoice
> 
> I wonder if we really need all these Kconfig options. I was thinking we
> could simply always compile CIK support in amdgpu, and make
> DRM_AMDGPU_CIK choose between using amdgpu or radeon by default for CIK.

BTW, if we decide we do need the new options, let's call them something
like DRM_AMD_CIK_* to make it clearer where they belong to.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 12/04/17 01:05 AM, Felix Kuehling wrote:
> On 17-04-11 12:01 AM, Michel Dänzer wrote:
>> One issue with this per-driver enable_cik option is that if the user
>> only enables it in the driver where it's disabled by default, without
>> also disabling it in the driver where it's enabled by default, it's back
>> to the current situation where both drivers try to use the same GPUs,
>> and it's up to chance which one actually gets to.
>>
>> That's why I was thinking about also having a shared command line option
>> respected by both drivers, e.g. amd_cik_driver=amdgpu/radeon . That
>> would be the preferred way to choose the driver at runtime.
> 
> The shared option could not be in either amdgpu or radeon. Otherwise
> we'd create a cross-dependency between the drivers. It would have to be
> in drm, I guess?
> 
>> Note that the per-driver enable_cik option will still be needed to be
>> able to override the kernel command line when manually loading the drivers.
> 
> Why? I could make the shared option writable in
> /sys/module/drm/parameters/amd_cik_driver so you can change it before
> loading the module.

I was thinking of a pure kernel command line option which is parsed by
both drivers, not a module parameter.

One possibility would be making each driver also parse the other
driver's module parameter on the kernel command line. I.e. radeon would
parse

 amdgpu.enable_cik=0

on the kernel command line and set its own enable_cik parameter to 1,
and vice versa. This would probably require making the default value of
the enable_cik options e.g. -1, and only parsing the other driver's
option in that case.

If you don't want to tackle that, I can give it a go as a follow-up patch.

We do need to settle on whether all those Kconfig options are needed
though; I think they're overkill.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Shawn Starr]

On 04/11/2017 12:16 PM, Christian König wrote:

Am 11.04.2017 um 18:05 schrieb Felix Kuehling:

On 17-04-11 12:01 AM, Michel Dänzer wrote:

One issue with this per-driver enable_cik option is that if the user
only enables it in the driver where it's disabled by default, without
also disabling it in the driver where it's enabled by default, it's back
to the current situation where both drivers try to use the same GPUs,
and it's up to chance which one actually gets to.

That's why I was thinking about also having a shared command line option
respected by both drivers, e.g. amd_cik_driver=amdgpu/radeon . That
would be the preferred way to choose the driver at runtime.

The shared option could not be in either amdgpu or radeon. Otherwise
we'd create a cross-dependency between the drivers. It would have to be
in drm, I guess?


Yes, but to be honest that sounds more and more like shooting with
cannonballs on birds to me.

At least I was fine with the per driver config option.

Regards,
Christian.





Any reason we can't just rip out the PCI IDs for CIK in radeon? If 
distros then can enable both drivers the kernel will load the correct 
one based on where the PCI IDs are being probed?


Or are there drm ioctl issues that make that incompatible?

Thanks,
Shawn


Note that the per-driver enable_cik option will still be needed to be
able to override the kernel command line when manually loading the
drivers.

Why? I could make the shared option writable in
/sys/module/drm/parameters/amd_cik_driver so you can change it before
loading the module.

Regards,
   Felix


Maybe there's a better way to handle both cases that I haven't
thought of.



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Christian König]

Am 11.04.2017 um 18:05 schrieb Felix Kuehling:

On 17-04-11 12:01 AM, Michel Dänzer wrote:

One issue with this per-driver enable_cik option is that if the user
only enables it in the driver where it's disabled by default, without
also disabling it in the driver where it's enabled by default, it's back
to the current situation where both drivers try to use the same GPUs,
and it's up to chance which one actually gets to.

That's why I was thinking about also having a shared command line option
respected by both drivers, e.g. amd_cik_driver=amdgpu/radeon . That
would be the preferred way to choose the driver at runtime.

The shared option could not be in either amdgpu or radeon. Otherwise
we'd create a cross-dependency between the drivers. It would have to be
in drm, I guess?


Yes, but to be honest that sounds more and more like shooting with 
cannonballs on birds to me.


At least I was fine with the per driver config option.

Regards,
Christian.




Note that the per-driver enable_cik option will still be needed to be
able to override the kernel command line when manually loading the drivers.

Why? I could make the shared option writable in
/sys/module/drm/parameters/amd_cik_driver so you can change it before
loading the module.

Regards,
   Felix


Maybe there's a better way to handle both cases that I haven't thought of.



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Felix Kuehling]

On 17-04-11 12:01 AM, Michel Dänzer wrote:
> One issue with this per-driver enable_cik option is that if the user
> only enables it in the driver where it's disabled by default, without
> also disabling it in the driver where it's enabled by default, it's back
> to the current situation where both drivers try to use the same GPUs,
> and it's up to chance which one actually gets to.
>
> That's why I was thinking about also having a shared command line option
> respected by both drivers, e.g. amd_cik_driver=amdgpu/radeon . That
> would be the preferred way to choose the driver at runtime.

The shared option could not be in either amdgpu or radeon. Otherwise
we'd create a cross-dependency between the drivers. It would have to be
in drm, I guess?

> Note that the per-driver enable_cik option will still be needed to be
> able to override the kernel command line when manually loading the drivers.

Why? I could make the shared option writable in
/sys/module/drm/parameters/amd_cik_driver so you can change it before
loading the module.

Regards,
  Felix

>
> Maybe there's a better way to handle both cases that I haven't thought of.
>
>

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Felix Kuehling]

On 17-04-10 09:42 PM, Michel Dänzer wrote:
> On 11/04/17 08:29 AM, Felix Kuehling wrote:
>> I tested this with Hawaii on the KFD branch and
>> DRM_CIK_BOTH_DEFAULT_AMDGPU. Both modules get loaded, but radeon doesn't
>> initialize the device. Amdgpu works with kfdtest.
> Did radeon print the "CIK support disabled by module param" message?

Yes.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 11/04/17 08:21 AM, Felix Kuehling wrote:
> Provide convenient compile time and boot time options for selecting
> CIK ASIC support in either or both drivers.
> 
> v2: git add missing files
> 
> Signed-off-by: Felix Kuehling 

[...]

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6238e2e..84d393d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -109,6 +109,11 @@
>  int amdgpu_pos_buf_per_se = 0;
>  int amdgpu_cntl_sb_buf_per_se = 0;
>  int amdgpu_param_buf_per_se = 0;
> +#ifdef CONFIG_DRM_AMDGPU_CIK_ENABLED
> +int amdgpu_enable_cik = 1;
> +#else
> +int amdgpu_enable_cik = 0;
> +#endif
>  
>  MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
>  module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
> @@ -234,6 +239,10 @@
>  MODULE_PARM_DESC(param_buf_per_se, "the size of Off-Chip Pramater Cache per 
> Shader Engine (default depending on gfx)");
>  module_param_named(param_buf_per_se, amdgpu_param_buf_per_se, int, 0444);
>  
> +#ifdef DRM_AMDGPU_CIK
> +MODULE_PARAM_DESC(enable_cik, "Enable CIK support");
> +module_param_named(enable_cik, amdgpu_enable_cik, int, 0444);
> +#endif

One issue with this per-driver enable_cik option is that if the user
only enables it in the driver where it's disabled by default, without
also disabling it in the driver where it's enabled by default, it's back
to the current situation where both drivers try to use the same GPUs,
and it's up to chance which one actually gets to.

That's why I was thinking about also having a shared command line option
respected by both drivers, e.g. amd_cik_driver=amdgpu/radeon . That
would be the preferred way to choose the driver at runtime.

Note that the per-driver enable_cik option will still be needed to be
able to override the kernel command line when manually loading the drivers.

Maybe there's a better way to handle both cases that I haven't thought of.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Michel Dänzer]

On 11/04/17 08:29 AM, Felix Kuehling wrote:
> I tested this with Hawaii on the KFD branch and
> DRM_CIK_BOTH_DEFAULT_AMDGPU. Both modules get loaded, but radeon doesn't
> initialize the device. Amdgpu works with kfdtest.

Did radeon print the "CIK support disabled by module param" message?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Felix Kuehling]

I tested this with Hawaii on the KFD branch and
DRM_CIK_BOTH_DEFAULT_AMDGPU. Both modules get loaded, but radeon doesn't
initialize the device. Amdgpu works with kfdtest.

To keep the driver code clean I only use straight-forward per-driver
options in #ifdefs. The CONFIG_DRM_CIK_... options that affect both
drivers select the right set of driver-specific options but aren't
referenced in the code directly.

Sorry about messed up v1. I missed some files when moving from
amd-kfd-staging to amd-staging-4.9.

If you like this approach, I can do the same for SI.

Regards,
  Felix


On 17-04-10 07:21 PM, Felix Kuehling wrote:
> Provide convenient compile time and boot time options for selecting
> CIK ASIC support in either or both drivers.
>
> v2: git add missing files
>
> Signed-off-by: Felix Kuehling 
> ---
>  drivers/gpu/drm/Kconfig |  51 ++
>  drivers/gpu/drm/amd/amdgpu/Kconfig  |  10 ++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h |   1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |   9 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  26 
>  drivers/gpu/drm/radeon/Kconfig  |   8 +++
>  drivers/gpu/drm/radeon/radeon.h |   1 +
>  drivers/gpu/drm/radeon/radeon_drv.c |  13 
>  drivers/gpu/drm/radeon/radeon_kms.c |  13 
>  include/drm/drm_pciids.h| 114 
> 
>  10 files changed, 184 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 483059a..f85f81c 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -148,6 +148,57 @@ config DRM_AMDGPU
>  
>  source "drivers/gpu/drm/amd/amdgpu/Kconfig"
>  
> +choice DRM_CIK_SUPPORT
> + prompt "Support for AMD CIK ASICs"
> + default DRM_CIK_RADEON
> + depends on DRM_AMDGPU || DRM_RADEON
> + help
> +   Choose the driver used to support AMD CIK ASICs.
> +
> +config DRM_CIK_RADEON
> + bool "Radeon"
> + depends on DRM_RADEON
> + select DRM_RADEON_CIK
> + select DRM_RADEON_CIK_ENABLED
> + help
> +   The Radeon driver was traditionally used to support CIK ASICs.
> +
> +config DRM_CIK_AMDGPU
> + bool "AMD GPU"
> + depends on DRM_AMDGPU
> + select DRM_AMDGPU_CIK
> + select DRM_AMDGPU_CIK_ENABLED
> + help
> +   The AMD GPU driver has more active development for features and
> +   performance. If you choose this driver, you also need the amdgpu
> +   DDX driver for X.org.
> +
> +config DRM_CIK_BOTH_DEFAULT_RADEON
> + bool "Both, use Radeon by default"
> + depends on DRM_AMDGPU && DRM_RADEON
> + select DRM_RADEON_CIK
> + select DRM_RADEON_CIK_ENABLED
> + select DRM_AMDGPU_CIK
> + help
> +   This option is useful for driver developers who want to test
> +   both drivers while running the same kernel. The active driver
> +   can be selected using the module parameters radeon.enable_cik
> +   and amdgpu.enable_cik.
> +
> +config DRM_CIK_BOTH_DEFAULT_AMDGPU
> + bool "Both, use AMD GPU by default"
> + depends on DRM_AMDGPU && DRM_RADEON
> + select DRM_RADEON_CIK
> + select DRM_AMDGPU_CIK
> + select DRM_AMDGPU_CIK_ENABLED
> + help
> +   This option is useful for driver developers who want to test
> +   both drivers while running the same kernel. The active driver
> +   can be selected using the module parameters radeon.enable_cik
> +   and amdgpu.enable_cik.
> +
> +endchoice
> +
>  source "drivers/gpu/drm/nouveau/Kconfig"
>  
>  source "drivers/gpu/drm/i915/Kconfig"
> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
> b/drivers/gpu/drm/amd/amdgpu/Kconfig
> index f3b6df8..7bc171d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
> @@ -6,14 +6,12 @@ config DRM_AMDGPU_SI
> for SI asics.
>  
>  config DRM_AMDGPU_CIK
> - bool "Enable amdgpu support for CIK parts"
> + bool
>   depends on DRM_AMDGPU
> - help
> -   Choose this option if you want to enable experimental support
> -   for CIK asics.
>  
> -   CIK is already supported in radeon.  CIK support in amdgpu
> -   is for experimentation and testing.
> +config DRM_AMDGPU_CIK_ENABLED
> + bool
> + depends on DRM_AMDGPU
>  
>  config DRM_AMDGPU_USERPTR
>   bool "Always enable userptr write support"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 5487580..e028169 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -111,6 +111,7 @@
>  extern int amdgpu_pos_buf_per_se;
>  extern int amdgpu_cntl_sb_buf_per_se;
>  extern int amdgpu_param_buf_per_se;
> +extern int amdgpu_enable_cik;
>  
>  #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS   3000
>  #define AMDGPU_MAX_USEC_TIMEOUT  10  /* 100 ms */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> 

[PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

[Felix Kuehling]

Provide convenient compile time and boot time options for selecting
CIK ASIC support in either or both drivers.

v2: git add missing files

Signed-off-by: Felix Kuehling 
---
 drivers/gpu/drm/Kconfig |  51 ++
 drivers/gpu/drm/amd/amdgpu/Kconfig  |  10 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |   9 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  26 
 drivers/gpu/drm/radeon/Kconfig  |   8 +++
 drivers/gpu/drm/radeon/radeon.h |   1 +
 drivers/gpu/drm/radeon/radeon_drv.c |  13 
 drivers/gpu/drm/radeon/radeon_kms.c |  13 
 include/drm/drm_pciids.h| 114 
 10 files changed, 184 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 483059a..f85f81c 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -148,6 +148,57 @@ config DRM_AMDGPU
 
 source "drivers/gpu/drm/amd/amdgpu/Kconfig"
 
+choice DRM_CIK_SUPPORT
+   prompt "Support for AMD CIK ASICs"
+   default DRM_CIK_RADEON
+   depends on DRM_AMDGPU || DRM_RADEON
+   help
+ Choose the driver used to support AMD CIK ASICs.
+
+config DRM_CIK_RADEON
+   bool "Radeon"
+   depends on DRM_RADEON
+   select DRM_RADEON_CIK
+   select DRM_RADEON_CIK_ENABLED
+   help
+ The Radeon driver was traditionally used to support CIK ASICs.
+
+config DRM_CIK_AMDGPU
+   bool "AMD GPU"
+   depends on DRM_AMDGPU
+   select DRM_AMDGPU_CIK
+   select DRM_AMDGPU_CIK_ENABLED
+   help
+ The AMD GPU driver has more active development for features and
+ performance. If you choose this driver, you also need the amdgpu
+ DDX driver for X.org.
+
+config DRM_CIK_BOTH_DEFAULT_RADEON
+   bool "Both, use Radeon by default"
+   depends on DRM_AMDGPU && DRM_RADEON
+   select DRM_RADEON_CIK
+   select DRM_RADEON_CIK_ENABLED
+   select DRM_AMDGPU_CIK
+   help
+ This option is useful for driver developers who want to test
+ both drivers while running the same kernel. The active driver
+ can be selected using the module parameters radeon.enable_cik
+ and amdgpu.enable_cik.
+
+config DRM_CIK_BOTH_DEFAULT_AMDGPU
+   bool "Both, use AMD GPU by default"
+   depends on DRM_AMDGPU && DRM_RADEON
+   select DRM_RADEON_CIK
+   select DRM_AMDGPU_CIK
+   select DRM_AMDGPU_CIK_ENABLED
+   help
+ This option is useful for driver developers who want to test
+ both drivers while running the same kernel. The active driver
+ can be selected using the module parameters radeon.enable_cik
+ and amdgpu.enable_cik.
+
+endchoice
+
 source "drivers/gpu/drm/nouveau/Kconfig"
 
 source "drivers/gpu/drm/i915/Kconfig"
diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
b/drivers/gpu/drm/amd/amdgpu/Kconfig
index f3b6df8..7bc171d 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -6,14 +6,12 @@ config DRM_AMDGPU_SI
  for SI asics.
 
 config DRM_AMDGPU_CIK
-   bool "Enable amdgpu support for CIK parts"
+   bool
depends on DRM_AMDGPU
-   help
- Choose this option if you want to enable experimental support
- for CIK asics.
 
- CIK is already supported in radeon.  CIK support in amdgpu
- is for experimentation and testing.
+config DRM_AMDGPU_CIK_ENABLED
+   bool
+   depends on DRM_AMDGPU
 
 config DRM_AMDGPU_USERPTR
bool "Always enable userptr write support"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 5487580..e028169 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -111,6 +111,7 @@
 extern int amdgpu_pos_buf_per_se;
 extern int amdgpu_cntl_sb_buf_per_se;
 extern int amdgpu_param_buf_per_se;
+extern int amdgpu_enable_cik;
 
 #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
 #define AMDGPU_MAX_USEC_TIMEOUT10  /* 100 ms */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6238e2e..84d393d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -109,6 +109,11 @@
 int amdgpu_pos_buf_per_se = 0;
 int amdgpu_cntl_sb_buf_per_se = 0;
 int amdgpu_param_buf_per_se = 0;
+#ifdef CONFIG_DRM_AMDGPU_CIK_ENABLED
+int amdgpu_enable_cik = 1;
+#else
+int amdgpu_enable_cik = 0;
+#endif
 
 MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
 module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
@@ -234,6 +239,10 @@
 MODULE_PARM_DESC(param_buf_per_se, "the size of Off-Chip Pramater Cache per 
Shader Engine (default depending on gfx)");
 module_param_named(param_buf_per_se, amdgpu_param_buf_per_se, int, 0444);
 
+#ifdef DRM_AMDGPU_CIK