[PATCH 1/2] drm/amd/display: change variables type

[PATCH 1/2] drm/amd/display: change variables type

[Magali Lemes]

As "dcn3_15_soc" and "dcn3_16_soc" are of type "struct
_vcs_dpi_soc_bounding_box_st", change their types accordingly.

Signed-off-by: Magali Lemes <magalilemes00@gmail.com>
---
 drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h | 2 +-
 drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
index 39929fa67a51..45276317c057 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
@@ -32,7 +32,7 @@
 	container_of(pool, struct dcn315_resource_pool, base)
 
 extern struct _vcs_dpi_ip_params_st dcn3_15_ip;
-extern struct _vcs_dpi_ip_params_st dcn3_15_soc;
+extern struct _vcs_dpi_soc_bounding_box_st dcn3_15_soc;
 
 struct dcn315_resource_pool {
 	struct resource_pool base;
diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
index 0dc5a6c13ae7..d2234aac5449 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
@@ -32,7 +32,7 @@
 	container_of(pool, struct dcn316_resource_pool, base)
 
 extern struct _vcs_dpi_ip_params_st dcn3_16_ip;
-extern struct _vcs_dpi_ip_params_st dcn3_16_soc;
+extern struct _vcs_dpi_soc_bounding_box_st dcn3_16_soc;
 
 struct dcn316_resource_pool {
 	struct resource_pool base;
-- 
2.37.1

[PATCH 2/2] drm/amd/display: include missing headers

[Magali Lemes]

Add missing headers to solve the following warnings from sparse:

drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:656:17: warning: symbol 'ddr4_wm_table_gs' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:693:17: warning: symbol 'lpddr4_wm_table_gs' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:730:17: warning: symbol 'lpddr4_wm_table_with_disabled_ppt' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:767:17: warning: symbol 'ddr4_wm_table_rn' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:804:17: warning: symbol 'ddr4_1R_wm_table_rn' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:841:17: warning: symbol 'lpddr4_wm_table_rn' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn301/dcn301_fpu.c:217:17: warning: symbol 'ddr4_wm_table' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn301/dcn301_fpu.c:254:17: warning: symbol 'lpddr5_wm_table' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:53:30: warning: symbol 'dcn3_1_ip' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:117:37: warning: symbol 'dcn3_1_soc' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:197:30: warning: symbol 'dcn3_15_ip' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:262:37: warning: symbol 'dcn3_15_soc' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:293:30: warning: symbol 'dcn3_16_ip' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:358:37: warning: symbol 'dcn3_16_soc' was not declared. Should it be static?

Signed-off-by: Magali Lemes <magalilemes00@gmail.com>
---
 drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h | 1 +
 drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c      | 1 +
 drivers/gpu/drm/amd/display/dc/dml/dcn301/dcn301_fpu.c    | 1 +
 drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c      | 3 +++
 4 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h
index 2e088c5171b2..f1319957e400 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h
@@ -28,6 +28,7 @@
 
 #include "clk_mgr.h"
 #include "dm_pp_smu.h"
+#include "clk_mgr_internal.h"
 
 extern struct wm_table ddr4_wm_table_gs;
 extern struct wm_table lpddr4_wm_table_gs;
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
index eeeae52fe6fc..45e9f4663abe 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
@@ -30,6 +30,7 @@
 #include "dchubbub.h"
 #include "dcn20/dcn20_resource.h"
 #include "dcn21/dcn21_resource.h"
+#include "clk_mgr/dcn21/rn_clk_mgr.h"
 
 #include "dcn20_fpu.h"
 
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn301/dcn301_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn301/dcn301_fpu.c
index 7ef66e511ec8..d211cf6d234c 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn301/dcn301_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn301/dcn301_fpu.c
@@ -26,6 +26,7 @@
 #include "clk_mgr.h"
 #include "dcn20/dcn20_resource.h"
 #include "dcn301/dcn301_resource.h"
+#include "clk_mgr/dcn301/vg_clk_mgr.h"
 
 #include "dml/dcn20/dcn20_fpu.h"
 #include "dcn301_fpu.h"
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c
index e36cfa5985ea..2d11a2c13345 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c
@@ -25,6 +25,9 @@
 
 #include "resource.h"
 #include "clk_mgr.h"
+#include "dcn31/dcn31_resource.h"
+#include "dcn315/dcn315_resource.h"
+#include "dcn316/dcn316_resource.h"
 
 #include "dml/dcn20/dcn20_fpu.h"
 #include "dcn31_fpu.h"
-- 
2.37.1

Re: [PATCH 2/2] drm/amd/display: include missing headers

[Melissa Wen]

[-- Attachment #1: Type: text/plain, Size: 5149 bytes --]

On 07/25, Magali Lemes wrote:
> Add missing headers to solve the following warnings from sparse:
> 
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:656:17: warning: symbol 'ddr4_wm_table_gs' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:693:17: warning: symbol 'lpddr4_wm_table_gs' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:730:17: warning: symbol 'lpddr4_wm_table_with_disabled_ppt' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:767:17: warning: symbol 'ddr4_wm_table_rn' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:804:17: warning: symbol 'ddr4_1R_wm_table_rn' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:841:17: warning: symbol 'lpddr4_wm_table_rn' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn301/dcn301_fpu.c:217:17: warning: symbol 'ddr4_wm_table' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn301/dcn301_fpu.c:254:17: warning: symbol 'lpddr5_wm_table' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:53:30: warning: symbol 'dcn3_1_ip' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:117:37: warning: symbol 'dcn3_1_soc' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:197:30: warning: symbol 'dcn3_15_ip' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:262:37: warning: symbol 'dcn3_15_soc' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:293:30: warning: symbol 'dcn3_16_ip' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/dcn31_fpu.c:358:37: warning: symbol 'dcn3_16_soc' was not declared. Should it be static?
> 
Hi Magali,

Nice catch! See some comments below:

> Signed-off-by: Magali Lemes <magalilemes00@gmail.com>
> ---
>  drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h | 1 +
>  drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c      | 1 +
>  drivers/gpu/drm/amd/display/dc/dml/dcn301/dcn301_fpu.c    | 1 +
>  drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c      | 3 +++
>  4 files changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h
> index 2e088c5171b2..f1319957e400 100644
> --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h
> +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h
> @@ -28,6 +28,7 @@
>  
>  #include "clk_mgr.h"
>  #include "dm_pp_smu.h"
> +#include "clk_mgr_internal.h"

I see that this fix (right above) comes from another warning not listed
in the commit message. Can you explicit it there or split it in
another commit if possible?

Also, clk_mgr/dcn21/rn_clk_mgr.c includes clk_mgr_internal.h and
rn_clk_mgr.h; I wonder if a cleaner solution is to remove the
`#include clk_mgr_internal.h` from rn_clk_mgr.c too.

The remaining changes seems correct to me.

Thanks,

Melissa

>  
>  extern struct wm_table ddr4_wm_table_gs;
>  extern struct wm_table lpddr4_wm_table_gs;
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
> index eeeae52fe6fc..45e9f4663abe 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
> @@ -30,6 +30,7 @@
>  #include "dchubbub.h"
>  #include "dcn20/dcn20_resource.h"
>  #include "dcn21/dcn21_resource.h"
> +#include "clk_mgr/dcn21/rn_clk_mgr.h"
>  
>  #include "dcn20_fpu.h"
>  
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn301/dcn301_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn301/dcn301_fpu.c
> index 7ef66e511ec8..d211cf6d234c 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn301/dcn301_fpu.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn301/dcn301_fpu.c
> @@ -26,6 +26,7 @@
>  #include "clk_mgr.h"
>  #include "dcn20/dcn20_resource.h"
>  #include "dcn301/dcn301_resource.h"
> +#include "clk_mgr/dcn301/vg_clk_mgr.h"
>  
>  #include "dml/dcn20/dcn20_fpu.h"
>  #include "dcn301_fpu.h"
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c
> index e36cfa5985ea..2d11a2c13345 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c
> @@ -25,6 +25,9 @@
>  
>  #include "resource.h"
>  #include "clk_mgr.h"
> +#include "dcn31/dcn31_resource.h"
> +#include "dcn315/dcn315_resource.h"
> +#include "dcn316/dcn316_resource.h"
>  
>  #include "dml/dcn20/dcn20_fpu.h"
>  #include "dcn31_fpu.h"
> -- 
> 2.37.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] drm/amd/display: change variables type

[André Almeida]

Hi Magali,

Às 15:15 de 25/07/22, Magali Lemes escreveu:
> As "dcn3_15_soc" and "dcn3_16_soc" are of type "struct
> _vcs_dpi_soc_bounding_box_st", change their types accordingly.
> 

I can see that indeed this type change sense for those variables, but
isn't a bit strange that the type was wrong in the first place? I wonder
if this variable is even used, given that it would very likely throw a
compiler error when using the wrong type and trying to access struct
members that aren't defined.

> Signed-off-by: Magali Lemes <magalilemes00@gmail.com>
> ---
>  drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h | 2 +-
>  drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> index 39929fa67a51..45276317c057 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> @@ -32,7 +32,7 @@
>  	container_of(pool, struct dcn315_resource_pool, base)
>  
>  extern struct _vcs_dpi_ip_params_st dcn3_15_ip;
> -extern struct _vcs_dpi_ip_params_st dcn3_15_soc;
> +extern struct _vcs_dpi_soc_bounding_box_st dcn3_15_soc;
>  
>  struct dcn315_resource_pool {
>  	struct resource_pool base;
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> index 0dc5a6c13ae7..d2234aac5449 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> @@ -32,7 +32,7 @@
>  	container_of(pool, struct dcn316_resource_pool, base)
>  
>  extern struct _vcs_dpi_ip_params_st dcn3_16_ip;
> -extern struct _vcs_dpi_ip_params_st dcn3_16_soc;
> +extern struct _vcs_dpi_soc_bounding_box_st dcn3_16_soc;
>  
>  struct dcn316_resource_pool {
>  	struct resource_pool base;

Re: [PATCH 1/2] drm/amd/display: change variables type

[Magali Lemes]

On 7/25/22 16:42, André Almeida wrote:
> Hi Magali,
>
> Às 15:15 de 25/07/22, Magali Lemes escreveu:
>> As "dcn3_15_soc" and "dcn3_16_soc" are of type "struct
>> _vcs_dpi_soc_bounding_box_st", change their types accordingly.
>>
> I can see that indeed this type change sense for those variables, but
> isn't a bit strange that the type was wrong in the first place? I wonder
> if this variable is even used, given that it would very likely throw a
> compiler error when using the wrong type and trying to access struct
> members that aren't defined.


A compilation error would be thrown if either 
"dc/dcn315/dcn315_resource.h" or "dc/dcn316/dcn316_resource.h" were 
included in the files where "dcn3_15_soc" and "dcn3_16_soc" are 
initialized. Since they are not included, the wrong variable type error 
is not shown.
To solve the sparse warning in the second patch of this series, those 
variables need to be declared first, but they are already declared, 
we're only missing the headers. If I only add the headers, then those 
variables will be seen, and I get the expected incompatible variables 
types error. So, fixing the types here is a preliminary work for the 
next patch.


Magali


>
>> Signed-off-by: Magali Lemes <magalilemes00@gmail.com>
>> ---
>>   drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h | 2 +-
>>   drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
>> index 39929fa67a51..45276317c057 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
>> @@ -32,7 +32,7 @@
>>   	container_of(pool, struct dcn315_resource_pool, base)
>>   
>>   extern struct _vcs_dpi_ip_params_st dcn3_15_ip;
>> -extern struct _vcs_dpi_ip_params_st dcn3_15_soc;
>> +extern struct _vcs_dpi_soc_bounding_box_st dcn3_15_soc;
>>   
>>   struct dcn315_resource_pool {
>>   	struct resource_pool base;
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
>> index 0dc5a6c13ae7..d2234aac5449 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
>> @@ -32,7 +32,7 @@
>>   	container_of(pool, struct dcn316_resource_pool, base)
>>   
>>   extern struct _vcs_dpi_ip_params_st dcn3_16_ip;
>> -extern struct _vcs_dpi_ip_params_st dcn3_16_soc;
>> +extern struct _vcs_dpi_soc_bounding_box_st dcn3_16_soc;
>>   
>>   struct dcn316_resource_pool {
>>   	struct resource_pool base;

Re: [PATCH 1/2] drm/amd/display: change variables type

[Melissa Wen]

[-- Attachment #1: Type: text/plain, Size: 3294 bytes --]

On 07/25, Magali Lemes wrote:
> 
> On 7/25/22 16:42, André Almeida wrote:
> > Hi Magali,
> > 
> > Às 15:15 de 25/07/22, Magali Lemes escreveu:
> > > As "dcn3_15_soc" and "dcn3_16_soc" are of type "struct
> > > _vcs_dpi_soc_bounding_box_st", change their types accordingly.
> > > 
> > I can see that indeed this type change sense for those variables, but
> > isn't a bit strange that the type was wrong in the first place? I wonder
> > if this variable is even used, given that it would very likely throw a
> > compiler error when using the wrong type and trying to access struct
> > members that aren't defined.
> 
> 
> A compilation error would be thrown if either "dc/dcn315/dcn315_resource.h"
> or "dc/dcn316/dcn316_resource.h" were included in the files where
> "dcn3_15_soc" and "dcn3_16_soc" are initialized. Since they are not
> included, the wrong variable type error is not shown.
> To solve the sparse warning in the second patch of this series, those
> variables need to be declared first, but they are already declared, we're
> only missing the headers. If I only add the headers, then those variables
> will be seen, and I get the expected incompatible variables types error. So,
> fixing the types here is a preliminary work for the next patch.
> 

Hi Magali,

Thanks for inspecting it. What you say makes sense, but André pointed
out something that makes sense to me too.

As fas as I checked, dcn3_15_soc and dcn16_soc is not used outside their
respective FPU files. Maybe the proper solution is removing those
declarations (and make the struct static). Can you take a look at it?

Best Regards,

Melissa
> 
> Magali
> 
> 
> > 
> > > Signed-off-by: Magali Lemes <magalilemes00@gmail.com>
> > > ---
> > >   drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h | 2 +-
> > >   drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h | 2 +-
> > >   2 files changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> > > index 39929fa67a51..45276317c057 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> > > +++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> > > @@ -32,7 +32,7 @@
> > >   	container_of(pool, struct dcn315_resource_pool, base)
> > >   extern struct _vcs_dpi_ip_params_st dcn3_15_ip;
> > > -extern struct _vcs_dpi_ip_params_st dcn3_15_soc;
> > > +extern struct _vcs_dpi_soc_bounding_box_st dcn3_15_soc;
> > >   struct dcn315_resource_pool {
> > >   	struct resource_pool base;
> > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> > > index 0dc5a6c13ae7..d2234aac5449 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> > > +++ b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> > > @@ -32,7 +32,7 @@
> > >   	container_of(pool, struct dcn316_resource_pool, base)
> > >   extern struct _vcs_dpi_ip_params_st dcn3_16_ip;
> > > -extern struct _vcs_dpi_ip_params_st dcn3_16_soc;
> > > +extern struct _vcs_dpi_soc_bounding_box_st dcn3_16_soc;
> > >   struct dcn316_resource_pool {
> > >   	struct resource_pool base;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] drm/amd/display: change variables type

[Magali Lemes]

On 7/25/22 20:38, Melissa Wen wrote:

> On 07/25, Magali Lemes wrote:
>> On 7/25/22 16:42, André Almeida wrote:
>>> Hi Magali,
>>>
>>> Às 15:15 de 25/07/22, Magali Lemes escreveu:
>>>> As "dcn3_15_soc" and "dcn3_16_soc" are of type "struct
>>>> _vcs_dpi_soc_bounding_box_st", change their types accordingly.
>>>>
>>> I can see that indeed this type change sense for those variables, but
>>> isn't a bit strange that the type was wrong in the first place? I wonder
>>> if this variable is even used, given that it would very likely throw a
>>> compiler error when using the wrong type and trying to access struct
>>> members that aren't defined.
>>
>> A compilation error would be thrown if either "dc/dcn315/dcn315_resource.h"
>> or "dc/dcn316/dcn316_resource.h" were included in the files where
>> "dcn3_15_soc" and "dcn3_16_soc" are initialized. Since they are not
>> included, the wrong variable type error is not shown.
>> To solve the sparse warning in the second patch of this series, those
>> variables need to be declared first, but they are already declared, we're
>> only missing the headers. If I only add the headers, then those variables
>> will be seen, and I get the expected incompatible variables types error. So,
>> fixing the types here is a preliminary work for the next patch.
>>
> Hi Magali,
>
> Thanks for inspecting it. What you say makes sense, but André pointed
> out something that makes sense to me too.
>
> As fas as I checked, dcn3_15_soc and dcn16_soc is not used outside their
> respective FPU files. Maybe the proper solution is removing those
> declarations (and make the struct static). Can you take a look at it?
>
> Best Regards,
>
> Melissa

Hi, Melissa. Thank you for the suggestion!
My sole reason not to make those structs static was to keep some sort of 
consistency with the rest of the dcn*_resource.h files, since that is 
where all the other structs are first declared. I'm not sure, though, if 
that's a good enough reason not to turn these variables into static. Let 
me know what you think.

Magali



> Magali
>
>
>>>> Signed-off-by: Magali Lemes <magalilemes00@gmail.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h | 2 +-
>>>>    drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h | 2 +-
>>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
>>>> index 39929fa67a51..45276317c057 100644
>>>> --- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
>>>> +++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
>>>> @@ -32,7 +32,7 @@
>>>>    	container_of(pool, struct dcn315_resource_pool, base)
>>>>    extern struct _vcs_dpi_ip_params_st dcn3_15_ip;
>>>> -extern struct _vcs_dpi_ip_params_st dcn3_15_soc;
>>>> +extern struct _vcs_dpi_soc_bounding_box_st dcn3_15_soc;
>>>>    struct dcn315_resource_pool {
>>>>    	struct resource_pool base;
>>>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
>>>> index 0dc5a6c13ae7..d2234aac5449 100644
>>>> --- a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
>>>> +++ b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
>>>> @@ -32,7 +32,7 @@
>>>>    	container_of(pool, struct dcn316_resource_pool, base)
>>>>    extern struct _vcs_dpi_ip_params_st dcn3_16_ip;
>>>> -extern struct _vcs_dpi_ip_params_st dcn3_16_soc;
>>>> +extern struct _vcs_dpi_soc_bounding_box_st dcn3_16_soc;
>>>>    struct dcn316_resource_pool {
>>>>    	struct resource_pool base;

Re: [PATCH 1/2] drm/amd/display: change variables type

[Melissa Wen]

[-- Attachment #1.1: Type: text/plain, Size: 4376 bytes --]

On 07/26, Magali Lemes wrote:
> On 7/25/22 20:38, Melissa Wen wrote:
> 
> > On 07/25, Magali Lemes wrote:
> > > On 7/25/22 16:42, André Almeida wrote:
> > > > Hi Magali,
> > > > 
> > > > Às 15:15 de 25/07/22, Magali Lemes escreveu:
> > > > > As "dcn3_15_soc" and "dcn3_16_soc" are of type "struct
> > > > > _vcs_dpi_soc_bounding_box_st", change their types accordingly.
> > > > > 
> > > > I can see that indeed this type change sense for those variables, but
> > > > isn't a bit strange that the type was wrong in the first place? I wonder
> > > > if this variable is even used, given that it would very likely throw a
> > > > compiler error when using the wrong type and trying to access struct
> > > > members that aren't defined.
> > > 
> > > A compilation error would be thrown if either "dc/dcn315/dcn315_resource.h"
> > > or "dc/dcn316/dcn316_resource.h" were included in the files where
> > > "dcn3_15_soc" and "dcn3_16_soc" are initialized. Since they are not
> > > included, the wrong variable type error is not shown.
> > > To solve the sparse warning in the second patch of this series, those
> > > variables need to be declared first, but they are already declared, we're
> > > only missing the headers. If I only add the headers, then those variables
> > > will be seen, and I get the expected incompatible variables types error. So,
> > > fixing the types here is a preliminary work for the next patch.
> > > 
> > Hi Magali,
> > 
> > Thanks for inspecting it. What you say makes sense, but André pointed
> > out something that makes sense to me too.
> > 
> > As fas as I checked, dcn3_15_soc and dcn16_soc is not used outside their
> > respective FPU files. Maybe the proper solution is removing those
> > declarations (and make the struct static). Can you take a look at it?
> > 
> > Best Regards,
> > 
> > Melissa
> 
> Hi, Melissa. Thank you for the suggestion!
> My sole reason not to make those structs static was to keep some sort of
> consistency with the rest of the dcn*_resource.h files, since that is where
> all the other structs are first declared. I'm not sure, though, if that's a
> good enough reason not to turn these variables into static. Let me know what
> you think.

I don't see any other file using dcn3_15_soc, it's only in dcn30_fpu, so
better make it static. Also, I see that doing this will ring a bell
for some misuse of the struct outside FPU protection, in the future.

With those points addressed, you can add in the next version:

Reviewed-by: Melissa Wen <mwen@igalia.com>

Thanks,

Melissa

> 
> Magali
> 
> 
> 
> > Magali
> > 
> > 
> > > > > Signed-off-by: Magali Lemes <magalilemes00@gmail.com>
> > > > > ---
> > > > >    drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h | 2 +-
> > > > >    drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h | 2 +-
> > > > >    2 files changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> > > > > index 39929fa67a51..45276317c057 100644
> > > > > --- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> > > > > +++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> > > > > @@ -32,7 +32,7 @@
> > > > >    	container_of(pool, struct dcn315_resource_pool, base)
> > > > >    extern struct _vcs_dpi_ip_params_st dcn3_15_ip;
> > > > > -extern struct _vcs_dpi_ip_params_st dcn3_15_soc;
> > > > > +extern struct _vcs_dpi_soc_bounding_box_st dcn3_15_soc;
> > > > >    struct dcn315_resource_pool {
> > > > >    	struct resource_pool base;
> > > > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> > > > > index 0dc5a6c13ae7..d2234aac5449 100644
> > > > > --- a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> > > > > +++ b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> > > > > @@ -32,7 +32,7 @@
> > > > >    	container_of(pool, struct dcn316_resource_pool, base)
> > > > >    extern struct _vcs_dpi_ip_params_st dcn3_16_ip;
> > > > > -extern struct _vcs_dpi_ip_params_st dcn3_16_soc;
> > > > > +extern struct _vcs_dpi_soc_bounding_box_st dcn3_16_soc;
> > > > >    struct dcn316_resource_pool {
> > > > >    	struct resource_pool base;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] drm/amd/display: change variables type

[André Almeida]

Às 17:47 de 25/07/22, Magali Lemes escreveu:
> 
> On 7/25/22 16:42, André Almeida wrote:
>> Hi Magali,
>>
>> Às 15:15 de 25/07/22, Magali Lemes escreveu:
>>> As "dcn3_15_soc" and "dcn3_16_soc" are of type "struct
>>> _vcs_dpi_soc_bounding_box_st", change their types accordingly.
>>>
>> I can see that indeed this type change sense for those variables, but
>> isn't a bit strange that the type was wrong in the first place? I wonder
>> if this variable is even used, given that it would very likely throw a
>> compiler error when using the wrong type and trying to access struct
>> members that aren't defined.
> 
> 
> A compilation error would be thrown if either
> "dc/dcn315/dcn315_resource.h" or "dc/dcn316/dcn316_resource.h" were
> included in the files where "dcn3_15_soc" and "dcn3_16_soc" are
> initialized. Since they are not included, the wrong variable type error
> is not shown.
> To solve the sparse warning in the second patch of this series, those
> variables need to be declared first, but they are already declared,
> we're only missing the headers. If I only add the headers, then those
> variables will be seen, and I get the expected incompatible variables
> types error. So, fixing the types here is a preliminary work for the
> next patch.
> 

And what happens if you just drop this declaration? It still not clear
to me why this belongs to this header, giving that AFAIK this var is
only used at drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c

Note that this kind of information you explained to me would be very
useful if you had included it in a cover letter to add more context for
your patches :)

> 
> Magali
> 
> 
>>
>>> Signed-off-by: Magali Lemes <magalilemes00@gmail.com>
>>> ---
>>>   drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h | 2 +-
>>>   drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
>>> b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
>>> index 39929fa67a51..45276317c057 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
>>> @@ -32,7 +32,7 @@
>>>       container_of(pool, struct dcn315_resource_pool, base)
>>>     extern struct _vcs_dpi_ip_params_st dcn3_15_ip;
>>> -extern struct _vcs_dpi_ip_params_st dcn3_15_soc;
>>> +extern struct _vcs_dpi_soc_bounding_box_st dcn3_15_soc;
>>>     struct dcn315_resource_pool {
>>>       struct resource_pool base;
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
>>> b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
>>> index 0dc5a6c13ae7..d2234aac5449 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
>>> @@ -32,7 +32,7 @@
>>>       container_of(pool, struct dcn316_resource_pool, base)
>>>     extern struct _vcs_dpi_ip_params_st dcn3_16_ip;
>>> -extern struct _vcs_dpi_ip_params_st dcn3_16_soc;
>>> +extern struct _vcs_dpi_soc_bounding_box_st dcn3_16_soc;
>>>     struct dcn316_resource_pool {
>>>       struct resource_pool base;

Re: [PATCH 1/2] drm/amd/display: change variables type

[Maíra Canal]

Hi Magali

On 7/25/22 15:15, Magali Lemes wrote:
> As "dcn3_15_soc" and "dcn3_16_soc" are of type "struct
> _vcs_dpi_soc_bounding_box_st", change their types accordingly.
> 
> Signed-off-by: Magali Lemes <magalilemes00@gmail.com>
> ---

Great catch! To the whole series:

Reviewed-by: Maíra Canal <mairacanal@riseup.net>

Best Regards,
- Maíra Canal

>  drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h | 2 +-
>  drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> index 39929fa67a51..45276317c057 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.h
> @@ -32,7 +32,7 @@
>  	container_of(pool, struct dcn315_resource_pool, base)
>  
>  extern struct _vcs_dpi_ip_params_st dcn3_15_ip;
> -extern struct _vcs_dpi_ip_params_st dcn3_15_soc;
> +extern struct _vcs_dpi_soc_bounding_box_st dcn3_15_soc;
>  
>  struct dcn315_resource_pool {
>  	struct resource_pool base;
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> index 0dc5a6c13ae7..d2234aac5449 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.h
> @@ -32,7 +32,7 @@
>  	container_of(pool, struct dcn316_resource_pool, base)
>  
>  extern struct _vcs_dpi_ip_params_st dcn3_16_ip;
> -extern struct _vcs_dpi_ip_params_st dcn3_16_soc;
> +extern struct _vcs_dpi_soc_bounding_box_st dcn3_16_soc;
>  
>  struct dcn316_resource_pool {
>  	struct resource_pool base;