RE: [PATCH 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-30 Thread Deucher, Alexander
> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Michel Dänzer
> Sent: Monday, May 29, 2017 5:20 AM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH 3/3] drm/amdgpu/radeon: Use radeon by default for CIK
> GPUs
> 
> From: Michel Dänzer <michel.daen...@amd.com>
> 
> Even if CONFIG_DRM_AMDGPU_CIK is enabled.
> 
> There is no feature parity yet for CIK, in particular amdgpu doesn't
> support HDMI/DisplayPort without DC.

"HDMI/DisplayPort audio"

With that fixed, the series is:
Reviewed-by: Alex Deucher <alexander.deuc...@amd.com>

> 
> Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/Kconfig  | 8 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 7 +--
>  drivers/gpu/drm/radeon/radeon_drv.c | 4 ++--
>  drivers/gpu/drm/radeon/radeon_kms.c | 5 +
>  5 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig
> b/drivers/gpu/drm/amd/amdgpu/Kconfig
> index 8d36087fc186..e0121f8b436e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
> @@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
>   help
> Choose this option if you want to enable support for CIK asics.
> 
> -   CIK is already supported in radeon. If you enable this option,
> -   support for CIK will be provided by amdgpu and disabled in
> -   radeon by default. Use module options to override this:
> +   CIK is already supported in radeon. Support for SI in amdgpu
> +   will be disabled by default and is still provided by radeon.
> +   Use module options to override this:
> 
> -   radeon.cik_support=1 amdgpu.cik_support=0
> +   radeon.cik_support=0 amdgpu.cik_support=1
> 
>  config DRM_AMDGPU_USERPTR
>   bool "Always enable userptr write support"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 76dea5fe620b..27599db7d630 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -240,8 +240,8 @@ module_param_named(si_support,
> amdgpu_si_support, int, 0444);
>  #endif
> 
>  #ifdef CONFIG_DRM_AMDGPU_CIK
> -int amdgpu_cik_support = 1;
> -MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled (default), 0 =
> disabled)");
> +int amdgpu_cik_support = 0;
> +MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled
> (default))");
>  module_param_named(cik_support, amdgpu_cik_support, int, 0444);
>  #endif
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index f4f77b99afeb..31901d2886d9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -98,7 +98,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev,
> unsigned long flags)
>   dev_info(dev->dev,
>"SI support provided by radeon.\n");
>   dev_info(dev->dev,
> - "Use radeon.si_support=0 amdgpu.si_support=1 to
> override.\n"
> +  "Use radeon.si_support=0
> amdgpu.si_support=1 to override.\n"
>   );
>   return -ENODEV;
>   }
> @@ -113,7 +113,10 @@ int amdgpu_driver_load_kms(struct drm_device
> *dev, unsigned long flags)
>   case CHIP_KABINI:
>   case CHIP_MULLINS:
>   dev_info(dev->dev,
> -  "CIK support disabled by module param\n");
> +  "CIK support provided by radeon.\n");
> + dev_info(dev->dev,
> +  "Use radeon.cik_support=0
> amdgpu.cik_support=1 to override.\n"
> + );
>   return -ENODEV;
>   }
>   }
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 1cefadfe4a41..853ef118a735 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -307,8 +307,8 @@ int radeon_si_support = 1;
>  MODULE_PARM_DESC(si_support, "SI support (1 = enabled (default), 0 =
> disabled)");
>  module_param_named(si_support, radeon_si_support, int, 0444);
> 
> -int radeon_cik_support = 0;
> -MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled
> (default))");
> +int 

Re: [PATCH 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-30 Thread Michel Dänzer
On 30/05/17 06:17 PM, Grazvydas Ignotas wrote:
> On Tue, May 30, 2017 at 6:30 AM, Michel Dänzer  wrote:
>> On 29/05/17 06:20 PM, Michel Dänzer wrote:
>>> From: Michel Dänzer 
>>>
>>> Even if CONFIG_DRM_AMDGPU_CIK is enabled.
>>>
>>> There is no feature parity yet for CIK, in particular amdgpu doesn't
>>> support HDMI/DisplayPort without DC.
>>>
>>> Signed-off-by: Michel Dänzer 
>>
>> [...]
>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
>>> b/drivers/gpu/drm/amd/amdgpu/Kconfig
>>> index 8d36087fc186..e0121f8b436e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
>>> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
>>> @@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
>>>   help
>>> Choose this option if you want to enable support for CIK asics.
>>>
>>> -   CIK is already supported in radeon. If you enable this option,
>>> -   support for CIK will be provided by amdgpu and disabled in
>>> -   radeon by default. Use module options to override this:
>>> +   CIK is already supported in radeon. Support for SI in amdgpu
>>
>> Consider this "SI" typo fixed in v2.
> 
> While you are here, what about adding the full codenames here? You
> can't expect every user configuring the kernel to know what SI/CIK
> means, it's not even documented in
> https://www.x.org/wiki/RadeonFeature/ or wikipedia, while the long
> codenames are.

That's out of scope for this series, and I definitely won't get around
to doing that before next week. Anybody feel free to beat me to it.


-- 
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 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-30 Thread Grazvydas Ignotas
On Tue, May 30, 2017 at 6:30 AM, Michel Dänzer  wrote:
> On 29/05/17 06:20 PM, Michel Dänzer wrote:
>> From: Michel Dänzer 
>>
>> Even if CONFIG_DRM_AMDGPU_CIK is enabled.
>>
>> There is no feature parity yet for CIK, in particular amdgpu doesn't
>> support HDMI/DisplayPort without DC.
>>
>> Signed-off-by: Michel Dänzer 
>
> [...]
>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
>> b/drivers/gpu/drm/amd/amdgpu/Kconfig
>> index 8d36087fc186..e0121f8b436e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
>> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
>> @@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
>>   help
>> Choose this option if you want to enable support for CIK asics.
>>
>> -   CIK is already supported in radeon. If you enable this option,
>> -   support for CIK will be provided by amdgpu and disabled in
>> -   radeon by default. Use module options to override this:
>> +   CIK is already supported in radeon. Support for SI in amdgpu
>
> Consider this "SI" typo fixed in v2.

While you are here, what about adding the full codenames here? You
can't expect every user configuring the kernel to know what SI/CIK
means, it's not even documented in
https://www.x.org/wiki/RadeonFeature/ or wikipedia, while the long
codenames are.

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


Re: [PATCH 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-29 Thread Michel Dänzer
On 29/05/17 06:20 PM, Michel Dänzer wrote:
> From: Michel Dänzer 
> 
> Even if CONFIG_DRM_AMDGPU_CIK is enabled.
> 
> There is no feature parity yet for CIK, in particular amdgpu doesn't
> support HDMI/DisplayPort without DC.
> 
> Signed-off-by: Michel Dänzer 

[...]

> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
> b/drivers/gpu/drm/amd/amdgpu/Kconfig
> index 8d36087fc186..e0121f8b436e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
> @@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
>   help
> Choose this option if you want to enable support for CIK asics.
>  
> -   CIK is already supported in radeon. If you enable this option,
> -   support for CIK will be provided by amdgpu and disabled in
> -   radeon by default. Use module options to override this:
> +   CIK is already supported in radeon. Support for SI in amdgpu

Consider this "SI" typo fixed in v2.


-- 
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 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-29 Thread Michel Dänzer
On 29/05/17 11:24 PM, Kai Wasserbäch wrote:
> Hey Michel,
> Michel Dänzer wrote on 29.05.2017 11:20:
>> From: Michel Dänzer 
>>
>> Even if CONFIG_DRM_AMDGPU_CIK is enabled.
>>
>> There is no feature parity yet for CIK, in particular amdgpu doesn't
>> support HDMI/DisplayPort without DC.
> 
> that can't be correct.

Indeed, I meant "HDMI/DisplayPort audio", sorry for the confusion. Will
be fixed in v2.


>> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
>> b/drivers/gpu/drm/amd/amdgpu/Kconfig
>> index 8d36087fc186..e0121f8b436e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
>> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
>> @@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
>>  help
>>Choose this option if you want to enable support for CIK asics.
>>  
>> -  CIK is already supported in radeon. If you enable this option,
>> -  support for CIK will be provided by amdgpu and disabled in
>> -  radeon by default. Use module options to override this:
>> +  CIK is already supported in radeon. Support for SI in amdgpu
>> +  will be disabled by default and is still provided by radeon.
>> +  Use module options to override this:
>>  
>> -  radeon.cik_support=1 amdgpu.cik_support=0
>> +  radeon.cik_support=0 amdgpu.cik_support=1
> 
> What happens if I have radeon blacklisted? Do I still need to set this
> additional flag?

Yes, you still need to set amdgpu.cik_support=1.

> If so I'd kindly request to at least fall back to amdgpu if somebody
> blacklisted radeon (or didn't compile it).

I'm afraid that's not possible, at least I don't know how it could be done.

The intention is that these options provide a cleaner way than
blacklisting for choosing between the drivers, so the options should be
used instead of blacklisting.


> Maybe issue a warning that not all features are supported yet.

The drivers print a message when they're ignoring a GPU due to these
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 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-29 Thread Kai Wasserbäch
Hey Michel,
Michel Dänzer wrote on 29.05.2017 11:20:
> From: Michel Dänzer 
> 
> Even if CONFIG_DRM_AMDGPU_CIK is enabled.
> 
> There is no feature parity yet for CIK, in particular amdgpu doesn't
> support HDMI/DisplayPort without DC.

that can't be correct. I'm using amdgpu on a CIK ASIC (Hawaii Pro) and my
monitor is connected by DisplayPort. From my Xorg.0.log:
> [37.811] (II) AMDGPU(0): EDID for output DisplayPort-0
> [37.811] (II) AMDGPU(0): Manufacturer: SAM  Model: 83d  Serial#: 0
> [37.811] (II) AMDGPU(0): Year: 2011  Week: 7
> [37.811] (II) AMDGPU(0): EDID Version: 1.4
> [37.811] (II) AMDGPU(0): Digital Display Input
> [37.811] (II) AMDGPU(0): 8 bits per channel
> [37.811] (II) AMDGPU(0): Digital interface is DisplayPort
> [37.811] (II) AMDGPU(0): Max Image Size [cm]: horiz.: 52  vert.: 32
> [37.811] (II) AMDGPU(0): Gamma: 2.20
> [37.811] (II) AMDGPU(0): DPMS capabilities: Off
> [37.811] (II) AMDGPU(0): Supported color encodings: RGB 4:4:4 
> [37.811] (II) AMDGPU(0): First detailed timing is preferred mode
> [37.811] (II) AMDGPU(0): Preferred mode is native pixel format and 
> refresh rate
> [37.811] (II) AMDGPU(0): redX: 0.650 redY: 0.337   greenX: 0.296 greenY: 
> 0.604
> [37.811] (II) AMDGPU(0): blueX: 0.147 blueY: 0.074   whiteX: 0.312 
> whiteY: 0.329
> [37.811] (II) AMDGPU(0): Supported established timings: [...]

Or do you mean some features like audio over HDMI/DP?

> Signed-off-by: Michel Dänzer 
> ---
>  drivers/gpu/drm/amd/amdgpu/Kconfig  | 8 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 7 +--
>  drivers/gpu/drm/radeon/radeon_drv.c | 4 ++--
>  drivers/gpu/drm/radeon/radeon_kms.c | 5 +
>  5 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
> b/drivers/gpu/drm/amd/amdgpu/Kconfig
> index 8d36087fc186..e0121f8b436e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
> @@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
>   help
> Choose this option if you want to enable support for CIK asics.
>  
> -   CIK is already supported in radeon. If you enable this option,
> -   support for CIK will be provided by amdgpu and disabled in
> -   radeon by default. Use module options to override this:
> +   CIK is already supported in radeon. Support for SI in amdgpu
> +   will be disabled by default and is still provided by radeon.
> +   Use module options to override this:
>  
> -   radeon.cik_support=1 amdgpu.cik_support=0
> +   radeon.cik_support=0 amdgpu.cik_support=1

What happens if I have radeon blacklisted? Do I still need to set this
additional flag? If so I'd kindly request to at least fall back to amdgpu if
somebody blacklisted radeon (or didn't compile it). Maybe issue a warning that
not all features are supported yet.

Thank you for your consideration!

Cheers,
Kai


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


[PATCH 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-29 Thread Michel Dänzer
From: Michel Dänzer 

Even if CONFIG_DRM_AMDGPU_CIK is enabled.

There is no feature parity yet for CIK, in particular amdgpu doesn't
support HDMI/DisplayPort without DC.

Signed-off-by: Michel Dänzer 
---
 drivers/gpu/drm/amd/amdgpu/Kconfig  | 8 
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 7 +--
 drivers/gpu/drm/radeon/radeon_drv.c | 4 ++--
 drivers/gpu/drm/radeon/radeon_kms.c | 5 +
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
b/drivers/gpu/drm/amd/amdgpu/Kconfig
index 8d36087fc186..e0121f8b436e 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
help
  Choose this option if you want to enable support for CIK asics.
 
- CIK is already supported in radeon. If you enable this option,
- support for CIK will be provided by amdgpu and disabled in
- radeon by default. Use module options to override this:
+ CIK is already supported in radeon. Support for SI in amdgpu
+ will be disabled by default and is still provided by radeon.
+ Use module options to override this:
 
- radeon.cik_support=1 amdgpu.cik_support=0
+ radeon.cik_support=0 amdgpu.cik_support=1
 
 config DRM_AMDGPU_USERPTR
bool "Always enable userptr write support"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 76dea5fe620b..27599db7d630 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -240,8 +240,8 @@ module_param_named(si_support, amdgpu_si_support, int, 
0444);
 #endif
 
 #ifdef CONFIG_DRM_AMDGPU_CIK
-int amdgpu_cik_support = 1;
-MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled (default), 0 = 
disabled)");
+int amdgpu_cik_support = 0;
+MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled 
(default))");
 module_param_named(cik_support, amdgpu_cik_support, int, 0444);
 #endif
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index f4f77b99afeb..31901d2886d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -98,7 +98,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned 
long flags)
dev_info(dev->dev,
 "SI support provided by radeon.\n");
dev_info(dev->dev,
-   "Use radeon.si_support=0 amdgpu.si_support=1 to override.\n"
+"Use radeon.si_support=0 amdgpu.si_support=1 
to override.\n"
);
return -ENODEV;
}
@@ -113,7 +113,10 @@ int amdgpu_driver_load_kms(struct drm_device *dev, 
unsigned long flags)
case CHIP_KABINI:
case CHIP_MULLINS:
dev_info(dev->dev,
-"CIK support disabled by module param\n");
+"CIK support provided by radeon.\n");
+   dev_info(dev->dev,
+"Use radeon.cik_support=0 amdgpu.cik_support=1 
to override.\n"
+   );
return -ENODEV;
}
}
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index 1cefadfe4a41..853ef118a735 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -307,8 +307,8 @@ int radeon_si_support = 1;
 MODULE_PARM_DESC(si_support, "SI support (1 = enabled (default), 0 = 
disabled)");
 module_param_named(si_support, radeon_si_support, int, 0444);
 
-int radeon_cik_support = 0;
-MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled 
(default))");
+int radeon_cik_support = 1;
+MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled (default), 0 = 
disabled)");
 module_param_named(cik_support, radeon_cik_support, int, 0444);
 
 static struct pci_device_id pciidlist[] = {
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c 
b/drivers/gpu/drm/radeon/radeon_kms.c
index baaddf989f6d..8f2579772c34 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -118,10 +118,7 @@ int radeon_driver_load_kms(struct drm_device *dev, 
unsigned long flags)
case CHIP_KABINI:
case CHIP_MULLINS:
dev_info(dev->dev,
-"CIK support provided by amdgpu.\n");
-   dev_info(dev->dev,
-   "Use radeon.cik_support=1 amdgpu.cik_support=0 to override.\n"
-   );
+"CIK support disabled by module param\n");
return -ENODEV;