Re: [PATCH] drm/radeon: Make CIK support in Radeon conditional

2017-04-08 Thread Christian König 

How about an option DRM_RADEON_FORCE_CIK that enables Radeon CIK support
even if DRM_AMDGPU_CIK is enabled? That way the default depends only on
the AMDGPU configuration and makes sense for end users and
distributions. The DRM_RADEON_FORCE_CIK option would be an override for
developers to make testing and comparison of both drivers simpler.

Seems reasonable.

Completely agree.

Am 07.04.2017 um 21:50 schrieb Alex Deucher:

On Fri, Apr 7, 2017 at 3:47 PM, Felix Kuehling  wrote:

Kconfig still calls CIK and SI support in amdgpu "experimental". Is that
still true? Should we stop calling it experimental if we actually want
distributions to start using it?

I guess they aren't really experimental anymore.  We can drop that
from CIK, but should probably keep it for SI for the moment since SI
on amdgpu still lacks UVD and VCE support.


Yeah, CIK is quite stable now.

Nobody tackled UVD/VCE support on SI yet? Sounds like a good starting 
job if anybody wants to earn some credits.


Anybody want to volunteer?

Christian.



Alex


Regards,
   Felix


On 17-04-07 03:17 PM, Alex Deucher wrote:

On Fri, Apr 7, 2017 at 2:00 PM, Felix Kuehling  wrote:

On 17-04-07 12:10 PM, Christian König wrote:

Am 07.04.2017 um 18:01 schrieb Felix Kuehling:

Advertise CIK PCI IDs only when they are not supported by amdgpu.
Use the CONFIG_DRM_AMDGPU_CIK to check so that a single option in
the kernel config keeps both drivers in sync.

This is the simplest possible change. A more complete solution
may want to conditionally disable more CIK-specific code in the
Radeon driver.

Yeah, thought about that as well.

Just two notes:
1. Add a separate Radeon specific config option for this.

We clearly want to be able to enable both Radeon and Amdgpu to compare
them without recompile.

How about an option DRM_RADEON_FORCE_CIK that enables Radeon CIK support
even if DRM_AMDGPU_CIK is enabled? That way the default depends only on
the AMDGPU configuration and makes sense for end users and
distributions. The DRM_RADEON_FORCE_CIK option would be an override for
developers to make testing and comparison of both drivers simpler.


Seems reasonable.


2. Do the same thing for SI as well.

OK. In the same commit or separate?

Separate please.

Alex


Regards,
   Felix


Regards,
Christian.


Signed-off-by: Felix Kuehling 
---
   drivers/gpu/drm/radeon/radeon_drv.c |   3 +
   include/drm/drm_pciids.h| 114
++--
   2 files changed, 61 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index 2e5d680..551cd5f 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -302,6 +302,9 @@ static inline void
radeon_unregister_atpx_handler(void) {}
   module_param_named(vce, radeon_vce, int, 0444);
 static struct pci_device_id pciidlist[] = {
+#ifndef CONFIG_DRM_AMDGPU_CIK
+radeon_CIK_PCI_IDS,
+#endif
   radeon_PCI_IDS
   };
   diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h
index 8bc073d..cf17901 100644
--- a/include/drm/drm_pciids.h
+++ b/include/drm/drm_pciids.h
@@ -1,4 +1,4 @@
-#define radeon_PCI_IDS \
+#define radeon_CIK_PCI_IDS \
   {0x1002, 0x1304, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
   {0x1002, 0x1305, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
   {0x1002, 0x1306, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
@@ -21,6 +21,63 @@
   {0x1002, 0x131B, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
   {0x1002, 0x131C, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
   {0x1002, 0x131D, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
+{0x1002, 0x6640, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
+{0x1002, 0x6641, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
+{0x1002, 0x6646, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
+{0x1002, 0x6647, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
+{0x1002, 0x6649, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+{0x1002, 0x6650, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+{0x1002, 0x6651, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+{0x1002, 0x6658, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+{0x1002, 0x665c, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+{0x1002, 0x665d, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+{0x1002, 0x665f, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+

Re: [PATCH] drm/radeon: Make CIK support in Radeon conditional

2017-04-07 Thread Felix Kuehling 
Kconfig still calls CIK and SI support in amdgpu "experimental". Is that
still true? Should we stop calling it experimental if we actually want
distributions to start using it?

Regards,
  Felix


On 17-04-07 03:17 PM, Alex Deucher wrote:
> On Fri, Apr 7, 2017 at 2:00 PM, Felix Kuehling  wrote:
>> On 17-04-07 12:10 PM, Christian König wrote:
>>> Am 07.04.2017 um 18:01 schrieb Felix Kuehling:
 Advertise CIK PCI IDs only when they are not supported by amdgpu.
 Use the CONFIG_DRM_AMDGPU_CIK to check so that a single option in
 the kernel config keeps both drivers in sync.

 This is the simplest possible change. A more complete solution
 may want to conditionally disable more CIK-specific code in the
 Radeon driver.
>>> Yeah, thought about that as well.
>>>
>>> Just two notes:
>>> 1. Add a separate Radeon specific config option for this.
>>>
>>> We clearly want to be able to enable both Radeon and Amdgpu to compare
>>> them without recompile.
>> How about an option DRM_RADEON_FORCE_CIK that enables Radeon CIK support
>> even if DRM_AMDGPU_CIK is enabled? That way the default depends only on
>> the AMDGPU configuration and makes sense for end users and
>> distributions. The DRM_RADEON_FORCE_CIK option would be an override for
>> developers to make testing and comparison of both drivers simpler.
>>
> Seems reasonable.
>
>>> 2. Do the same thing for SI as well.
>> OK. In the same commit or separate?
> Separate please.
>
> Alex
>
>> Regards,
>>   Felix
>>
>>> Regards,
>>> Christian.
>>>
 Signed-off-by: Felix Kuehling 
 ---
   drivers/gpu/drm/radeon/radeon_drv.c |   3 +
   include/drm/drm_pciids.h| 114
 ++--
   2 files changed, 61 insertions(+), 56 deletions(-)

 diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
 b/drivers/gpu/drm/radeon/radeon_drv.c
 index 2e5d680..551cd5f 100644
 --- a/drivers/gpu/drm/radeon/radeon_drv.c
 +++ b/drivers/gpu/drm/radeon/radeon_drv.c
 @@ -302,6 +302,9 @@ static inline void
 radeon_unregister_atpx_handler(void) {}
   module_param_named(vce, radeon_vce, int, 0444);
 static struct pci_device_id pciidlist[] = {
 +#ifndef CONFIG_DRM_AMDGPU_CIK
 +radeon_CIK_PCI_IDS,
 +#endif
   radeon_PCI_IDS
   };
   diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h
 index 8bc073d..cf17901 100644
 --- a/include/drm/drm_pciids.h
 +++ b/include/drm/drm_pciids.h
 @@ -1,4 +1,4 @@
 -#define radeon_PCI_IDS \
 +#define radeon_CIK_PCI_IDS \
   {0x1002, 0x1304, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
   {0x1002, 0x1305, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
   {0x1002, 0x1306, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
 @@ -21,6 +21,63 @@
   {0x1002, 0x131B, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
   {0x1002, 0x131C, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
   {0x1002, 0x131D, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
 +{0x1002, 0x6640, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x6641, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x6646, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x6647, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x6649, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x6650, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x6651, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x6658, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x665c, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x665d, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x665f, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x67A0, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x67A1, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x67A2, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x67A8, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
 +{0x1002, 0x67A9, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
 +

Re: [PATCH] drm/radeon: Make CIK support in Radeon conditional

2017-04-07 Thread Alex Deucher 
On Fri, Apr 7, 2017 at 3:47 PM, Felix Kuehling  wrote:
> Kconfig still calls CIK and SI support in amdgpu "experimental". Is that
> still true? Should we stop calling it experimental if we actually want
> distributions to start using it?

I guess they aren't really experimental anymore.  We can drop that
from CIK, but should probably keep it for SI for the moment since SI
on amdgpu still lacks UVD and VCE support.

Alex

>
> Regards,
>   Felix
>
>
> On 17-04-07 03:17 PM, Alex Deucher wrote:
>> On Fri, Apr 7, 2017 at 2:00 PM, Felix Kuehling  
>> wrote:
>>> On 17-04-07 12:10 PM, Christian König wrote:
 Am 07.04.2017 um 18:01 schrieb Felix Kuehling:
> Advertise CIK PCI IDs only when they are not supported by amdgpu.
> Use the CONFIG_DRM_AMDGPU_CIK to check so that a single option in
> the kernel config keeps both drivers in sync.
>
> This is the simplest possible change. A more complete solution
> may want to conditionally disable more CIK-specific code in the
> Radeon driver.
 Yeah, thought about that as well.

 Just two notes:
 1. Add a separate Radeon specific config option for this.

 We clearly want to be able to enable both Radeon and Amdgpu to compare
 them without recompile.
>>> How about an option DRM_RADEON_FORCE_CIK that enables Radeon CIK support
>>> even if DRM_AMDGPU_CIK is enabled? That way the default depends only on
>>> the AMDGPU configuration and makes sense for end users and
>>> distributions. The DRM_RADEON_FORCE_CIK option would be an override for
>>> developers to make testing and comparison of both drivers simpler.
>>>
>> Seems reasonable.
>>
 2. Do the same thing for SI as well.
>>> OK. In the same commit or separate?
>> Separate please.
>>
>> Alex
>>
>>> Regards,
>>>   Felix
>>>
 Regards,
 Christian.

> Signed-off-by: Felix Kuehling 
> ---
>   drivers/gpu/drm/radeon/radeon_drv.c |   3 +
>   include/drm/drm_pciids.h| 114
> ++--
>   2 files changed, 61 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 2e5d680..551cd5f 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -302,6 +302,9 @@ static inline void
> radeon_unregister_atpx_handler(void) {}
>   module_param_named(vce, radeon_vce, int, 0444);
> static struct pci_device_id pciidlist[] = {
> +#ifndef CONFIG_DRM_AMDGPU_CIK
> +radeon_CIK_PCI_IDS,
> +#endif
>   radeon_PCI_IDS
>   };
>   diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h
> index 8bc073d..cf17901 100644
> --- a/include/drm/drm_pciids.h
> +++ b/include/drm/drm_pciids.h
> @@ -1,4 +1,4 @@
> -#define radeon_PCI_IDS \
> +#define radeon_CIK_PCI_IDS \
>   {0x1002, 0x1304, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>   {0x1002, 0x1305, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>   {0x1002, 0x1306, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
> @@ -21,6 +21,63 @@
>   {0x1002, 0x131B, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>   {0x1002, 0x131C, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>   {0x1002, 0x131D, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
> +{0x1002, 0x6640, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x6641, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x6646, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x6647, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x6649, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x6650, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x6651, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x6658, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x665c, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x665d, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x665f, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
> +{0x1002, 0x67A0, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
> +

Re: [PATCH] drm/radeon: Make CIK support in Radeon conditional

2017-04-07 Thread Alex Deucher 
On Fri, Apr 7, 2017 at 2:00 PM, Felix Kuehling  wrote:
>
> On 17-04-07 12:10 PM, Christian König wrote:
>> Am 07.04.2017 um 18:01 schrieb Felix Kuehling:
>>> Advertise CIK PCI IDs only when they are not supported by amdgpu.
>>> Use the CONFIG_DRM_AMDGPU_CIK to check so that a single option in
>>> the kernel config keeps both drivers in sync.
>>>
>>> This is the simplest possible change. A more complete solution
>>> may want to conditionally disable more CIK-specific code in the
>>> Radeon driver.
>>
>> Yeah, thought about that as well.
>>
>> Just two notes:
>> 1. Add a separate Radeon specific config option for this.
>>
>> We clearly want to be able to enable both Radeon and Amdgpu to compare
>> them without recompile.
>
> How about an option DRM_RADEON_FORCE_CIK that enables Radeon CIK support
> even if DRM_AMDGPU_CIK is enabled? That way the default depends only on
> the AMDGPU configuration and makes sense for end users and
> distributions. The DRM_RADEON_FORCE_CIK option would be an override for
> developers to make testing and comparison of both drivers simpler.
>

Seems reasonable.

>>
>> 2. Do the same thing for SI as well.
>
> OK. In the same commit or separate?

Separate please.

Alex

>
> Regards,
>   Felix
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Felix Kuehling 
>>> ---
>>>   drivers/gpu/drm/radeon/radeon_drv.c |   3 +
>>>   include/drm/drm_pciids.h| 114
>>> ++--
>>>   2 files changed, 61 insertions(+), 56 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
>>> b/drivers/gpu/drm/radeon/radeon_drv.c
>>> index 2e5d680..551cd5f 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_drv.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
>>> @@ -302,6 +302,9 @@ static inline void
>>> radeon_unregister_atpx_handler(void) {}
>>>   module_param_named(vce, radeon_vce, int, 0444);
>>> static struct pci_device_id pciidlist[] = {
>>> +#ifndef CONFIG_DRM_AMDGPU_CIK
>>> +radeon_CIK_PCI_IDS,
>>> +#endif
>>>   radeon_PCI_IDS
>>>   };
>>>   diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h
>>> index 8bc073d..cf17901 100644
>>> --- a/include/drm/drm_pciids.h
>>> +++ b/include/drm/drm_pciids.h
>>> @@ -1,4 +1,4 @@
>>> -#define radeon_PCI_IDS \
>>> +#define radeon_CIK_PCI_IDS \
>>>   {0x1002, 0x1304, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>>>   {0x1002, 0x1305, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>>>   {0x1002, 0x1306, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>>> @@ -21,6 +21,63 @@
>>>   {0x1002, 0x131B, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>>>   {0x1002, 0x131C, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>>>   {0x1002, 0x131D, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>>> +{0x1002, 0x6640, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x6641, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x6646, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x6647, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x6649, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x6650, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x6651, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x6658, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x665c, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x665d, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x665f, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x67A0, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x67A1, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x67A2, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x67A8, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x67A9, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x67AA, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x67B0, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x67B1, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>>> +{0x1002, 0x67B8, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>>>

Re: [PATCH] drm/radeon: Make CIK support in Radeon conditional

2017-04-07 Thread Felix Kuehling 

On 17-04-07 12:10 PM, Christian König wrote:
> Am 07.04.2017 um 18:01 schrieb Felix Kuehling:
>> Advertise CIK PCI IDs only when they are not supported by amdgpu.
>> Use the CONFIG_DRM_AMDGPU_CIK to check so that a single option in
>> the kernel config keeps both drivers in sync.
>>
>> This is the simplest possible change. A more complete solution
>> may want to conditionally disable more CIK-specific code in the
>> Radeon driver.
>
> Yeah, thought about that as well.
>
> Just two notes:
> 1. Add a separate Radeon specific config option for this.
>
> We clearly want to be able to enable both Radeon and Amdgpu to compare
> them without recompile.

How about an option DRM_RADEON_FORCE_CIK that enables Radeon CIK support
even if DRM_AMDGPU_CIK is enabled? That way the default depends only on
the AMDGPU configuration and makes sense for end users and
distributions. The DRM_RADEON_FORCE_CIK option would be an override for
developers to make testing and comparison of both drivers simpler.

>
> 2. Do the same thing for SI as well.

OK. In the same commit or separate?

Regards,
  Felix

>
> Regards,
> Christian.
>
>>
>> Signed-off-by: Felix Kuehling 
>> ---
>>   drivers/gpu/drm/radeon/radeon_drv.c |   3 +
>>   include/drm/drm_pciids.h| 114
>> ++--
>>   2 files changed, 61 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
>> b/drivers/gpu/drm/radeon/radeon_drv.c
>> index 2e5d680..551cd5f 100644
>> --- a/drivers/gpu/drm/radeon/radeon_drv.c
>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
>> @@ -302,6 +302,9 @@ static inline void
>> radeon_unregister_atpx_handler(void) {}
>>   module_param_named(vce, radeon_vce, int, 0444);
>> static struct pci_device_id pciidlist[] = {
>> +#ifndef CONFIG_DRM_AMDGPU_CIK
>> +radeon_CIK_PCI_IDS,
>> +#endif
>>   radeon_PCI_IDS
>>   };
>>   diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h
>> index 8bc073d..cf17901 100644
>> --- a/include/drm/drm_pciids.h
>> +++ b/include/drm/drm_pciids.h
>> @@ -1,4 +1,4 @@
>> -#define radeon_PCI_IDS \
>> +#define radeon_CIK_PCI_IDS \
>>   {0x1002, 0x1304, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>>   {0x1002, 0x1305, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>>   {0x1002, 0x1306, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>> @@ -21,6 +21,63 @@
>>   {0x1002, 0x131B, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>>   {0x1002, 0x131C, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>>   {0x1002, 0x131D, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
>> +{0x1002, 0x6640, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x6641, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x6646, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x6647, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x6649, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x6650, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x6651, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x6658, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x665c, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x665d, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x665f, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67A0, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67A1, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67A2, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67A8, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67A9, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67AA, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67B0, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67B1, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67B8, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67B9, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67BA, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
>> CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
>> +{0x1002, 0x67BE,

Re: [PATCH] drm/radeon: Make CIK support in Radeon conditional

2017-04-07 Thread Tom St Denis 

On 07/04/17 12:01 PM, Felix Kuehling wrote:

Advertise CIK PCI IDs only when they are not supported by amdgpu.
Use the CONFIG_DRM_AMDGPU_CIK to check so that a single option in
the kernel config keeps both drivers in sync.

This is the simplest possible change. A more complete solution
may want to conditionally disable more CIK-specific code in the
Radeon driver.

Signed-off-by: Felix Kuehling 


Good idea.  We could do something similar for SI down the road too.

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

Re: [PATCH] drm/radeon: Make CIK support in Radeon conditional

2017-04-07 Thread Christian König 

Am 07.04.2017 um 18:01 schrieb Felix Kuehling:

Advertise CIK PCI IDs only when they are not supported by amdgpu.
Use the CONFIG_DRM_AMDGPU_CIK to check so that a single option in
the kernel config keeps both drivers in sync.

This is the simplest possible change. A more complete solution
may want to conditionally disable more CIK-specific code in the
Radeon driver.


Yeah, thought about that as well.

Just two notes:
1. Add a separate Radeon specific config option for this.

We clearly want to be able to enable both Radeon and Amdgpu to compare 
them without recompile.


2. Do the same thing for SI as well.

Regards,
Christian.



Signed-off-by: Felix Kuehling 
---
  drivers/gpu/drm/radeon/radeon_drv.c |   3 +
  include/drm/drm_pciids.h| 114 ++--
  2 files changed, 61 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index 2e5d680..551cd5f 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -302,6 +302,9 @@ static inline void radeon_unregister_atpx_handler(void) {}
  module_param_named(vce, radeon_vce, int, 0444);
  
  static struct pci_device_id pciidlist[] = {

+#ifndef CONFIG_DRM_AMDGPU_CIK
+   radeon_CIK_PCI_IDS,
+#endif
radeon_PCI_IDS
  };
  
diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h

index 8bc073d..cf17901 100644
--- a/include/drm/drm_pciids.h
+++ b/include/drm/drm_pciids.h
@@ -1,4 +1,4 @@
-#define radeon_PCI_IDS \
+#define radeon_CIK_PCI_IDS \
{0x1002, 0x1304, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
{0x1002, 0x1305, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
{0x1002, 0x1306, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_KAVERI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
@@ -21,6 +21,63 @@
{0x1002, 0x131B, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
{0x1002, 0x131C, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
{0x1002, 0x131D, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_KAVERI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
+   {0x1002, 0x6640, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x6641, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x6646, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x6647, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x6649, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x6650, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x6651, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x6658, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x665c, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x665d, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x665f, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_BONAIRE|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67A0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67A1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67A2, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67A8, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67A9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67AA, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67B0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67B1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67B8, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67B9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67BA, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x67BE, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_HAWAII|RADEON_NEW_MEMMAP}, \
+   {0x1002, 0x9830, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_KABINI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
+   {0x1002, 0x9831, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_KABINI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
+   {0x1002, 0x9832, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_KABINI|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
+   {0x1002, 0x9833, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_KABINI|RADEON_NEW_MEMMAP|RADEON_IS_IGP}, \
+   {0x1002, 0x9834, PCI_ANY_ID, PCI_ANY_ID, 0, 0,