diff mbox

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

Message ID 20171102112657.111405-1-arnd@arndb.de (mailing list archive)
State New, archived
Headers show

Commit Message

Arnd Bergmann Nov. 2, 2017, 11:26 a.m. UTC
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 <arnd@arndb.de>
---
 drivers/gpu/drm/amd/display/dc/os_types.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Felix Kuehling Nov. 2, 2017, 2:09 p.m. UTC | #1
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 <arnd@arndb.de>
> ---
>  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(); \
Arnd Bergmann Nov. 2, 2017, 2:20 p.m. UTC | #2
On Thu, Nov 2, 2017 at 3:09 PM, Felix Kuehling <felix.kuehling@amd.com> 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 <arnd@arndb.de>
>> ---
>>  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
Alex Deucher Nov. 2, 2017, 2:26 p.m. UTC | #3
On Thu, Nov 2, 2017 at 10:20 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Nov 2, 2017 at 3:09 PM, Felix Kuehling <felix.kuehling@amd.com> 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 <arnd@arndb.de>
>>> ---
>>>  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
diff mbox

Patch

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 */
 
 #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(); \