Re: [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally

2017-11-02 Thread Alex Deucher 
On Thu, Nov 2, 2017 at 10:20 AM, Arnd Bergmann  wrote:
> On Thu, Nov 2, 2017 at 3:09 PM, Felix Kuehling  wrote:
>> On 2017-11-02 07:26 AM, Arnd Bergmann wrote:
>>> It seems impossible to build this driver without setting either
>>> CONFIG_DEBUG_KERNEL or CONFIG_DEBUG_DRIVER:
>>>
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h: In function 
>>> 'set_reg_field_value_ex':
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:132:2: error: 
>>> implicit declaration of function 'ASSERT'; did you mean 'IS_ERR'? 
>>> [-Werror=implicit-function-declaration]
>>>
>>> This moves the ASSERT() macro and related helpers outside of
>>> the #ifdef to get it to build again.
>>>
>>> Signed-off-by: Arnd Bergmann 
>>> ---
>>>  drivers/gpu/drm/amd/display/dc/os_types.h | 6 +-
>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h 
>>> b/drivers/gpu/drm/amd/display/dc/os_types.h
>>> index 86170b40b5c5..499bd52004cf 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/os_types.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/os_types.h
>>> @@ -61,8 +61,6 @@
>>>   * general debug capabilities
>>>   *
>>>   */
>>> -#if defined(CONFIG_DEBUG_KERNEL) || defined(CONFIG_DEBUG_DRIVER)
>>> -
>>>  #if defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB)
>>>  #define ASSERT_CRITICAL(expr) do {   \
>>>   if (WARN_ON(!(expr))) { \
>>> @@ -75,7 +73,7 @@
>>>   ; \
>>>   } \
>>>  } while (0)
>>> -#endif
>>> +#endif /* CONFIG_DEBUG_KERNEL || CONFIG_DEBUG_DRIVER */
>>
>> Is this the right comment here? Looks like it should be /*
>> defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB) */.
>
> Right, my mistake. I had tried a couple of different versions before I found
> one that built in all configurations. This slipped through. I'll send a fixed
> version of this one patch but not the other two in the series if that's ok.

Sounds good.  Thanks!

Alex

Re: [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally

2017-11-02 Thread Alex Deucher 
On Thu, Nov 2, 2017 at 10:20 AM, Arnd Bergmann  wrote:
> On Thu, Nov 2, 2017 at 3:09 PM, Felix Kuehling  wrote:
>> On 2017-11-02 07:26 AM, Arnd Bergmann wrote:
>>> It seems impossible to build this driver without setting either
>>> CONFIG_DEBUG_KERNEL or CONFIG_DEBUG_DRIVER:
>>>
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h: In function 
>>> 'set_reg_field_value_ex':
>>> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:132:2: error: 
>>> implicit declaration of function 'ASSERT'; did you mean 'IS_ERR'? 
>>> [-Werror=implicit-function-declaration]
>>>
>>> This moves the ASSERT() macro and related helpers outside of
>>> the #ifdef to get it to build again.
>>>
>>> Signed-off-by: Arnd Bergmann 
>>> ---
>>>  drivers/gpu/drm/amd/display/dc/os_types.h | 6 +-
>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h 
>>> b/drivers/gpu/drm/amd/display/dc/os_types.h
>>> index 86170b40b5c5..499bd52004cf 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/os_types.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/os_types.h
>>> @@ -61,8 +61,6 @@
>>>   * general debug capabilities
>>>   *
>>>   */
>>> -#if defined(CONFIG_DEBUG_KERNEL) || defined(CONFIG_DEBUG_DRIVER)
>>> -
>>>  #if defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB)
>>>  #define ASSERT_CRITICAL(expr) do {   \
>>>   if (WARN_ON(!(expr))) { \
>>> @@ -75,7 +73,7 @@
>>>   ; \
>>>   } \
>>>  } while (0)
>>> -#endif
>>> +#endif /* CONFIG_DEBUG_KERNEL || CONFIG_DEBUG_DRIVER */
>>
>> Is this the right comment here? Looks like it should be /*
>> defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB) */.
>
> Right, my mistake. I had tried a couple of different versions before I found
> one that built in all configurations. This slipped through. I'll send a fixed
> version of this one patch but not the other two in the series if that's ok.

Sounds good.  Thanks!

Alex

Re: [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally

2017-11-02 Thread Arnd Bergmann 
On Thu, Nov 2, 2017 at 3:09 PM, Felix Kuehling  wrote:
> On 2017-11-02 07:26 AM, Arnd Bergmann wrote:
>> It seems impossible to build this driver without setting either
>> CONFIG_DEBUG_KERNEL or CONFIG_DEBUG_DRIVER:
>>
>> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h: In function 
>> 'set_reg_field_value_ex':
>> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:132:2: error: 
>> implicit declaration of function 'ASSERT'; did you mean 'IS_ERR'? 
>> [-Werror=implicit-function-declaration]
>>
>> This moves the ASSERT() macro and related helpers outside of
>> the #ifdef to get it to build again.
>>
>> Signed-off-by: Arnd Bergmann 
>> ---
>>  drivers/gpu/drm/amd/display/dc/os_types.h | 6 +-
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h 
>> b/drivers/gpu/drm/amd/display/dc/os_types.h
>> index 86170b40b5c5..499bd52004cf 100644
>> --- a/drivers/gpu/drm/amd/display/dc/os_types.h
>> +++ b/drivers/gpu/drm/amd/display/dc/os_types.h
>> @@ -61,8 +61,6 @@
>>   * general debug capabilities
>>   *
>>   */
>> -#if defined(CONFIG_DEBUG_KERNEL) || defined(CONFIG_DEBUG_DRIVER)
>> -
>>  #if defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB)
>>  #define ASSERT_CRITICAL(expr) do {   \
>>   if (WARN_ON(!(expr))) { \
>> @@ -75,7 +73,7 @@
>>   ; \
>>   } \
>>  } while (0)
>> -#endif
>> +#endif /* CONFIG_DEBUG_KERNEL || CONFIG_DEBUG_DRIVER */
>
> Is this the right comment here? Looks like it should be /*
> defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB) */.

Right, my mistake. I had tried a couple of different versions before I found
one that built in all configurations. This slipped through. I'll send a fixed
version of this one patch but not the other two in the series if that's ok.

  Arnd

Re: [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally

2017-11-02 Thread Arnd Bergmann 
On Thu, Nov 2, 2017 at 3:09 PM, Felix Kuehling  wrote:
> On 2017-11-02 07:26 AM, Arnd Bergmann wrote:
>> It seems impossible to build this driver without setting either
>> CONFIG_DEBUG_KERNEL or CONFIG_DEBUG_DRIVER:
>>
>> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h: In function 
>> 'set_reg_field_value_ex':
>> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:132:2: error: 
>> implicit declaration of function 'ASSERT'; did you mean 'IS_ERR'? 
>> [-Werror=implicit-function-declaration]
>>
>> This moves the ASSERT() macro and related helpers outside of
>> the #ifdef to get it to build again.
>>
>> Signed-off-by: Arnd Bergmann 
>> ---
>>  drivers/gpu/drm/amd/display/dc/os_types.h | 6 +-
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h 
>> b/drivers/gpu/drm/amd/display/dc/os_types.h
>> index 86170b40b5c5..499bd52004cf 100644
>> --- a/drivers/gpu/drm/amd/display/dc/os_types.h
>> +++ b/drivers/gpu/drm/amd/display/dc/os_types.h
>> @@ -61,8 +61,6 @@
>>   * general debug capabilities
>>   *
>>   */
>> -#if defined(CONFIG_DEBUG_KERNEL) || defined(CONFIG_DEBUG_DRIVER)
>> -
>>  #if defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB)
>>  #define ASSERT_CRITICAL(expr) do {   \
>>   if (WARN_ON(!(expr))) { \
>> @@ -75,7 +73,7 @@
>>   ; \
>>   } \
>>  } while (0)
>> -#endif
>> +#endif /* CONFIG_DEBUG_KERNEL || CONFIG_DEBUG_DRIVER */
>
> Is this the right comment here? Looks like it should be /*
> defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB) */.

Right, my mistake. I had tried a couple of different versions before I found
one that built in all configurations. This slipped through. I'll send a fixed
version of this one patch but not the other two in the series if that's ok.

  Arnd

Re: [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally

2017-11-02 Thread Felix Kuehling 
On 2017-11-02 07:26 AM, Arnd Bergmann wrote:
> It seems impossible to build this driver without setting either
> CONFIG_DEBUG_KERNEL or CONFIG_DEBUG_DRIVER:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h: In function 
> 'set_reg_field_value_ex':
> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:132:2: error: implicit 
> declaration of function 'ASSERT'; did you mean 'IS_ERR'? 
> [-Werror=implicit-function-declaration]
>
> This moves the ASSERT() macro and related helpers outside of
> the #ifdef to get it to build again.
>
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/gpu/drm/amd/display/dc/os_types.h | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h 
> b/drivers/gpu/drm/amd/display/dc/os_types.h
> index 86170b40b5c5..499bd52004cf 100644
> --- a/drivers/gpu/drm/amd/display/dc/os_types.h
> +++ b/drivers/gpu/drm/amd/display/dc/os_types.h
> @@ -61,8 +61,6 @@
>   * general debug capabilities
>   *
>   */
> -#if defined(CONFIG_DEBUG_KERNEL) || defined(CONFIG_DEBUG_DRIVER)
> -
>  #if defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB)
>  #define ASSERT_CRITICAL(expr) do {   \
>   if (WARN_ON(!(expr))) { \
> @@ -75,7 +73,7 @@
>   ; \
>   } \
>  } while (0)
> -#endif
> +#endif /* CONFIG_DEBUG_KERNEL || CONFIG_DEBUG_DRIVER */

Is this the right comment here? Looks like it should be /*
defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB) */.

Regards,
  Felix

>  
>  #if defined(CONFIG_DEBUG_KERNEL_DC)
>  #define ASSERT(expr) ASSERT_CRITICAL(expr)
> @@ -86,8 +84,6 @@
>  
>  #define BREAK_TO_DEBUGGER() ASSERT(0)
>  
> -#endif /* CONFIG_DEBUG_KERNEL || CONFIG_DEBUG_DRIVER */
> -
>  #define DC_ERR(...)  do { \
>   dm_error(__VA_ARGS__); \
>   BREAK_TO_DEBUGGER(); \

Re: [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally

2017-11-02 Thread Felix Kuehling 
On 2017-11-02 07:26 AM, Arnd Bergmann wrote:
> It seems impossible to build this driver without setting either
> CONFIG_DEBUG_KERNEL or CONFIG_DEBUG_DRIVER:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h: In function 
> 'set_reg_field_value_ex':
> drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:132:2: error: implicit 
> declaration of function 'ASSERT'; did you mean 'IS_ERR'? 
> [-Werror=implicit-function-declaration]
>
> This moves the ASSERT() macro and related helpers outside of
> the #ifdef to get it to build again.
>
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/gpu/drm/amd/display/dc/os_types.h | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h 
> b/drivers/gpu/drm/amd/display/dc/os_types.h
> index 86170b40b5c5..499bd52004cf 100644
> --- a/drivers/gpu/drm/amd/display/dc/os_types.h
> +++ b/drivers/gpu/drm/amd/display/dc/os_types.h
> @@ -61,8 +61,6 @@
>   * general debug capabilities
>   *
>   */
> -#if defined(CONFIG_DEBUG_KERNEL) || defined(CONFIG_DEBUG_DRIVER)
> -
>  #if defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB)
>  #define ASSERT_CRITICAL(expr) do {   \
>   if (WARN_ON(!(expr))) { \
> @@ -75,7 +73,7 @@
>   ; \
>   } \
>  } while (0)
> -#endif
> +#endif /* CONFIG_DEBUG_KERNEL || CONFIG_DEBUG_DRIVER */

Is this the right comment here? Looks like it should be /*
defined(CONFIG_HAVE_KGDB) || defined(CONFIG_KGDB) */.

Regards,
  Felix

>  
>  #if defined(CONFIG_DEBUG_KERNEL_DC)
>  #define ASSERT(expr) ASSERT_CRITICAL(expr)
> @@ -86,8 +84,6 @@
>  
>  #define BREAK_TO_DEBUGGER() ASSERT(0)
>  
> -#endif /* CONFIG_DEBUG_KERNEL || CONFIG_DEBUG_DRIVER */
> -
>  #define DC_ERR(...)  do { \
>   dm_error(__VA_ARGS__); \
>   BREAK_TO_DEBUGGER(); \