* [PATCH 2/3] drm/amdgpu/display: remove unused REG_OFFSET macro
2017-11-02 11:26 [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally Arnd Bergmann
@ 2017-11-02 11:26 ` Arnd Bergmann
2017-11-02 11:26 ` [PATCH 3/3] drm/amdgpu/display: fix integer arithmetic problem Arnd Bergmann
2017-11-02 14:09 ` [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally Felix Kuehling
2 siblings, 0 replies; 7+ messages in thread
From: Arnd Bergmann @ 2017-11-02 11:26 UTC (permalink / raw)
To: Alex Deucher, Christian König
Cc: Arnd Bergmann, David Airlie, Harry Wentland, Tony Cheng,
Dave Airlie, Dmytro Laktyushkin, amd-gfx, dri-devel,
linux-kernel
The name conflicts with another macro of the same name on the ARM ixp4xx
platform, leading to build errors.
Neither of the users actually should use a name that generic, but the
other one was here first and the dc driver doesn't actually use it.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/gpu/drm/amd/display/dc/dm_services.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dm_services.h b/drivers/gpu/drm/amd/display/dc/dm_services.h
index c8190c38a644..d4917037ac42 100644
--- a/drivers/gpu/drm/amd/display/dc/dm_services.h
+++ b/drivers/gpu/drm/amd/display/dc/dm_services.h
@@ -160,9 +160,6 @@ unsigned int generic_reg_wait(const struct dc_context *ctx,
/* These macros need to be used with soc15 registers in order to retrieve
* the actual offset.
*/
-#define REG_OFFSET(reg) (reg + DCE_BASE.instance[0].segment[reg##_BASE_IDX])
-#define REG_BIF_OFFSET(reg) (reg + NBIF_BASE.instance[0].segment[reg##_BASE_IDX])
-
#define dm_write_reg_soc15(ctx, reg, inst_offset, value) \
dm_write_reg_func(ctx, reg + DCE_BASE.instance[0].segment[reg##_BASE_IDX] + inst_offset, value, __func__)
--
2.9.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] drm/amdgpu/display: fix integer arithmetic problem
2017-11-02 11:26 [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally Arnd Bergmann
2017-11-02 11:26 ` [PATCH 2/3] drm/amdgpu/display: remove unused REG_OFFSET macro Arnd Bergmann
@ 2017-11-02 11:26 ` Arnd Bergmann
2017-11-02 14:02 ` Harry Wentland
2017-11-02 14:09 ` [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally Felix Kuehling
2 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2017-11-02 11:26 UTC (permalink / raw)
To: Alex Deucher, Christian König
Cc: Arnd Bergmann, David Airlie, Harry Wentland, Tony Cheng,
Dmytro Laktyushkin, Joshua Aberback, Leo (Sunpeng) Li, amd-gfx,
dri-devel, linux-kernel
gcc warns about an ambiguous integer calculation:
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c: In function 'calculate_bandwidth':
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:534:5: error: this decimal constant is unsigned only in ISO C90 [-Werror]
data->lb_line_pitch = bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(2401171875, 100000000), bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))), bw_int_to_fixed(48));
^~~~
Marking the constant as explicitly unsigned makes it work fine everywhere
without warnings.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
index 15cbfc400633..4f8a95368ffc 100644
--- a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
+++ b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
@@ -531,7 +531,7 @@ static void calculate_bandwidth(
}
switch (data->lb_bpc[i]) {
case 8:
- data->lb_line_pitch = bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(2401171875, 100000000), bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))), bw_int_to_fixed(48));
+ data->lb_line_pitch = bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(2401171875ul, 100000000), bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))), bw_int_to_fixed(48));
break;
case 10:
data->lb_line_pitch = bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(300234375, 10000000), bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))), bw_int_to_fixed(48));
--
2.9.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] drm/amdgpu/display: fix integer arithmetic problem
2017-11-02 11:26 ` [PATCH 3/3] drm/amdgpu/display: fix integer arithmetic problem Arnd Bergmann
@ 2017-11-02 14:02 ` Harry Wentland
0 siblings, 0 replies; 7+ messages in thread
From: Harry Wentland @ 2017-11-02 14:02 UTC (permalink / raw)
To: Arnd Bergmann, Alex Deucher, Christian König
Cc: David Airlie, Tony Cheng, Dmytro Laktyushkin, Joshua Aberback,
Leo (Sunpeng) Li, amd-gfx, dri-devel, linux-kernel
On 2017-11-02 07:26 AM, Arnd Bergmann wrote:
> gcc warns about an ambiguous integer calculation:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c: In function 'calculate_bandwidth':
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:534:5: error: this decimal constant is unsigned only in ISO C90 [-Werror]
> data->lb_line_pitch = bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(2401171875, 100000000), bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))), bw_int_to_fixed(48));
> ^~~~
>
> Marking the constant as explicitly unsigned makes it work fine everywhere
> without warnings.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks for these fixes.
Series is
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Harry
> ---
> drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
> index 15cbfc400633..4f8a95368ffc 100644
> --- a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
> +++ b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
> @@ -531,7 +531,7 @@ static void calculate_bandwidth(
> }
> switch (data->lb_bpc[i]) {
> case 8:
> - data->lb_line_pitch = bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(2401171875, 100000000), bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))), bw_int_to_fixed(48));
> + data->lb_line_pitch = bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(2401171875ul, 100000000), bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))), bw_int_to_fixed(48));
> break;
> case 10:
> data->lb_line_pitch = bw_ceil2(bw_mul(bw_div(bw_frc_to_fixed(300234375, 10000000), bw_int_to_fixed(3)), bw_ceil2(data->source_width_in_lb, bw_int_to_fixed(8))), bw_int_to_fixed(48));
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally
2017-11-02 11:26 [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally Arnd Bergmann
2017-11-02 11:26 ` [PATCH 2/3] drm/amdgpu/display: remove unused REG_OFFSET macro Arnd Bergmann
2017-11-02 11:26 ` [PATCH 3/3] drm/amdgpu/display: fix integer arithmetic problem Arnd Bergmann
@ 2017-11-02 14:09 ` Felix Kuehling
2017-11-02 14:20 ` Arnd Bergmann
2 siblings, 1 reply; 7+ messages in thread
From: Felix Kuehling @ 2017-11-02 14:09 UTC (permalink / raw)
To: Arnd Bergmann, Alex Deucher, Christian König
Cc: David Airlie, Harry Wentland, linux-kernel, amd-gfx, dri-devel,
Dave Airlie, Edward O'Callaghan, Tony Cheng
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(); \
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally
2017-11-02 14:09 ` [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally Felix Kuehling
@ 2017-11-02 14:20 ` Arnd Bergmann
2017-11-02 14:26 ` Alex Deucher
0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2017-11-02 14:20 UTC (permalink / raw)
To: Felix Kuehling
Cc: Alex Deucher, Christian König, David Airlie, Harry Wentland,
Linux Kernel Mailing List, amd-gfx, dri-devel, Dave Airlie,
Edward O'Callaghan, Tony Cheng
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] drm/amdgpu/display: provide ASSERT macros unconditionally
2017-11-02 14:20 ` Arnd Bergmann
@ 2017-11-02 14:26 ` Alex Deucher
0 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2017-11-02 14:26 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Felix Kuehling, David Airlie, Edward O'Callaghan,
Linux Kernel Mailing List, amd-gfx list, Tony Cheng, dri-devel,
Alex Deucher, Dave Airlie, Christian König
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
^ permalink raw reply [flat|nested] 7+ messages in thread