drm/rockchip: include rockchip_drm_drv.h

[PATCH] drm/rockchip: include rockchip_drm_drv.h

Posted by Min-Hua Chen 2 weeks ago

Include rockchip_drm_drv.h to fix the follow sparse warning:

drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse:
warning: symbol 'vop2_platform_driver' was not declared.
Should it be static?

No functional change intended.

Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com>
---
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index 18efb3fe1c00..c678d1b0fd7c 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -14,6 +14,7 @@
 #include <drm/drm_print.h>
 
 #include "rockchip_drm_vop2.h"
+#include "rockchip_drm_drv.h"
 
 static const uint32_t formats_cluster[] = {
 	DRM_FORMAT_XRGB2101010,
-- 
2.43.0

Re:[PATCH] drm/rockchip: include rockchip_drm_drv.h

Posted by Andy Yan 2 weeks ago

Hi  Min Hua，
   Thanks for your work

At 2024-09-06 06:38:50, "Min-Hua Chen" <minhuadotchen@gmail.com> wrote:
>Include rockchip_drm_drv.h to fix the follow sparse warning:
>
>drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse:
>warning: symbol 'vop2_platform_driver' was not declared.
>Should it be static?
>
>No functional change intended.
>
>Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com>
>---
> drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
>index 18efb3fe1c00..c678d1b0fd7c 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
>@@ -14,6 +14,7 @@
> #include <drm/drm_print.h>
> 
> #include "rockchip_drm_vop2.h"
>+#include "rockchip_drm_drv.h"
> 

We already have a patch[0] include rockchip_drm_drv.h in rockchip_drm_vop2.h

[0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240904120238.3856782-3-andyshrk@163.com/ 

> static const uint32_t formats_cluster[] = {
> 	DRM_FORMAT_XRGB2101010,
>-- 
>2.43.0
>
>
>_______________________________________________
>Linux-rockchip mailing list
>Linux-rockchip@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] drm/rockchip: include rockchip_drm_drv.h

Posted by Diederik de Haas 1 week, 6 days ago

On Fri Sep 6, 2024 at 2:42 AM CEST, Andy Yan wrote:
> At 2024-09-06 06:38:50, "Min-Hua Chen" <minhuadotchen@gmail.com> wrote:
> >Include rockchip_drm_drv.h to fix the follow sparse warning:
> >
> >drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse:
> >warning: symbol 'vop2_platform_driver' was not declared.
> >Should it be static?
> >
> >No functional change intended.
> >
> >Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com>
> >---
> > drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> >index 18efb3fe1c00..c678d1b0fd7c 100644
> >--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> >+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> >@@ -14,6 +14,7 @@
> > #include <drm/drm_print.h>
> > 
> > #include "rockchip_drm_vop2.h"
> >+#include "rockchip_drm_drv.h"
> > 
>
> We already have a patch[0] include rockchip_drm_drv.h in rockchip_drm_vop2.h
>
> [0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240904120238.3856782-3-andyshrk@163.com/ 

Maybe I'm missing something, but this patch seems to fix an already
existing bug (which should have a Fixes tag?), which Andy also fixed
while implementing a different (and unrelated) feature?

Cheers,
  Diederik

Re:Re: [PATCH] drm/rockchip: include rockchip_drm_drv.h

Posted by Andy Yan 1 week, 6 days ago


Hi，
At 2024-09-06 17:28:33, "Diederik de Haas" <didi.debian@cknow.org> wrote:
>On Fri Sep 6, 2024 at 2:42 AM CEST, Andy Yan wrote:
>> At 2024-09-06 06:38:50, "Min-Hua Chen" <minhuadotchen@gmail.com> wrote:
>> >Include rockchip_drm_drv.h to fix the follow sparse warning:
>> >
>> >drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse:
>> >warning: symbol 'vop2_platform_driver' was not declared.
>> >Should it be static?
>> >
>> >No functional change intended.
>> >
>> >Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com>
>> >---
>> > drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 +
>> > 1 file changed, 1 insertion(+)
>> >
>> >diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
>> >index 18efb3fe1c00..c678d1b0fd7c 100644
>> >--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
>> >+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
>> >@@ -14,6 +14,7 @@
>> > #include <drm/drm_print.h>
>> > 
>> > #include "rockchip_drm_vop2.h"
>> >+#include "rockchip_drm_drv.h"
>> > 
>>
>> We already have a patch[0] include rockchip_drm_drv.h in rockchip_drm_vop2.h
>>
>> [0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240904120238.3856782-3-andyshrk@163.com/ 
>
>Maybe I'm missing something, but this patch seems to fix an already
>existing bug (which should have a Fixes tag?), which Andy also fixed

>while implementing a different (and unrelated) feature?


In fact, I don't know how to reproduce this compilation issue.
While implementing my feature, I happened to find that I need to include rockchip_drm_drv.h in rockchip_drm_vop2.h



>
>Cheers,
>  Diederik

Re: [PATCH] drm/rockchip: include rockchip_drm_drv.h

Posted by Diederik de Haas 1 week, 6 days ago

Hi,

On Fri Sep 6, 2024 at 11:50 AM CEST, Andy Yan wrote:
> At 2024-09-06 17:28:33, "Diederik de Haas" <didi.debian@cknow.org> wrote:
> >On Fri Sep 6, 2024 at 2:42 AM CEST, Andy Yan wrote:
> >> At 2024-09-06 06:38:50, "Min-Hua Chen" <minhuadotchen@gmail.com> wrote:
> >> >Include rockchip_drm_drv.h to fix the follow sparse warning:
> >> >
> >> >drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse:
> >> >warning: symbol 'vop2_platform_driver' was not declared.
> >> >Should it be static?
> >> >
> >> >No functional change intended.
> >> >
> >> >Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com>
> >> >---
> >> > drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 +
> >> > 1 file changed, 1 insertion(+)
> >> >
> >> >diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> >> >index 18efb3fe1c00..c678d1b0fd7c 100644
> >> >--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> >> >+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> >> >@@ -14,6 +14,7 @@
> >> > #include <drm/drm_print.h>
> >> > 
> >> > #include "rockchip_drm_vop2.h"
> >> >+#include "rockchip_drm_drv.h"
> >> > 
> >>
> >> We already have a patch[0] include rockchip_drm_drv.h in rockchip_drm_vop2.h
> >>
> >> [0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240904120238.3856782-3-andyshrk@163.com/ 
> >
> >Maybe I'm missing something, but this patch seems to fix an already
> >existing bug (which should have a Fixes tag?), which Andy also fixed
> >while implementing a different (and unrelated) feature?
>
> In fact, I don't know how to reproduce this compilation issue.

FWIW: I didn't see it either, but I assumed I was missing the right
context (i.e. patches) needed to trigger that warning.

> While implementing my feature, I happened to find that I need to
> include rockchip_drm_drv.h in rockchip_drm_vop2.h

Makes perfect sense :)
But if the warning is indeed valid, it should be fixed on its own (IMO).

Cheers,
  Diederik

Re: [PATCH] drm/rockchip: include rockchip_drm_drv.h

Posted by Min-Hua Chen 1 week, 6 days ago

>FWIW: I didn't see it either, but I assumed I was missing the right
>context (i.e. patches) needed to trigger that warning.
>
FYI

I triggered the warning by the following step:

install 'sparse' first

git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
cd linux
ARCH=arm64 LLVM=1 make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' mrproper defconfig all -j8


cheers,
Min-Hua

>
>> While implementing my feature, I happened to find that I need to
>> include rockchip_drm_drv.h in rockchip_drm_vop2.h
>
>Makes perfect sense :)
>But if the warning is indeed valid, it should be fixed on its own (IMO).
>
>Cheers,
>  Diederik

Re: [PATCH] drm/rockchip: include rockchip_drm_drv.h

Posted by Diederik de Haas 1 week, 5 days ago

On Sat Sep 7, 2024 at 5:02 AM CEST, Min-Hua Chen wrote:
> >FWIW: I didn't see it either, but I assumed I was missing the right
> >context (i.e. patches) needed to trigger that warning.
>
> I triggered the warning by the following step:
>
> install 'sparse' first
>
> ARCH=arm64 LLVM=1 make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' mrproper defconfig all -j8

This, especially the 'LLVM' part,  is important context information
and should be part of the commit message.

I had only just started when I saw a number of sparse warnings:

  DTC     arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-io.dtb
  OVL     arch/arm64/boot/dts/ti/k3-j721e-evm.dtb
  OVL     arch/arm64/boot/dts/ti/k3-j721s2-evm.dtb
  OVL     arch/arm64/boot/dts/ti/k3-am654-idk.dtb
  DTC     arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dtb
  DTC     arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-wifi.dtbo
  DTC     arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dtb
  DTC     arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6b-io.dtb
  DTC     arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dtb
  DTC     arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dtb
../init/main.c:192:12: sparse: warning: symbol 'envp_init' was not declared. Should it be static?
../init/main.c:290:16: sparse: warning: cast to restricted __le32
../init/main.c:291:16: sparse: warning: cast to restricted __le32
  CHECK   ../init/do_mounts.c

And several followed, including in c-code files. So I stopped the build
and assume you've identified a or several actual issues.

I've seen several commits where changes were made because LLVM flagged
potentially problematic code, where GCC did not, so it's quite possible
you're on to something here.

But it would be helpful if the commit message said what code was
potentially problematic and why. And then the proper fix for that could
indeed be to include `rockchip_drm_drv.h`.

Cheers,
  Diederik

Re:Re: [PATCH] drm/rockchip: include rockchip_drm_drv.h

Posted by Andy Yan 1 week, 5 days ago


Hi ，
At 2024-09-07 16:47:18, "Diederik de Haas" <didi.debian@cknow.org> wrote:
>On Sat Sep 7, 2024 at 5:02 AM CEST, Min-Hua Chen wrote:
>> >FWIW: I didn't see it either, but I assumed I was missing the right
>> >context (i.e. patches) needed to trigger that warning.
>>
>> I triggered the warning by the following step:
>>
>> install 'sparse' first
>>
>> ARCH=arm64 LLVM=1 make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' mrproper defconfig all -j8
>
>This, especially the 'LLVM' part,  is important context information
>and should be part of the commit message.
>
>I had only just started when I saw a number of sparse warnings:
>
>  DTC     arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-io.dtb
>  OVL     arch/arm64/boot/dts/ti/k3-j721e-evm.dtb
>  OVL     arch/arm64/boot/dts/ti/k3-j721s2-evm.dtb
>  OVL     arch/arm64/boot/dts/ti/k3-am654-idk.dtb
>  DTC     arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dtb
>  DTC     arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-wifi.dtbo
>  DTC     arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dtb
>  DTC     arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6b-io.dtb
>  DTC     arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dtb
>  DTC     arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dtb
>../init/main.c:192:12: sparse: warning: symbol 'envp_init' was not declared. Should it be static?
>../init/main.c:290:16: sparse: warning: cast to restricted __le32
>../init/main.c:291:16: sparse: warning: cast to restricted __le32
>  CHECK   ../init/do_mounts.c

>


I can see same warnings， a lots of。
And also see the warning in vop2：
drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse: warning: symbol 'vop2_platform_driver' was not declared. Should it be static?


Min Hua，If you are agree，I will split it from my patch， and add a Fix tag ，and also add a SoB of you， Then resend in My V3 series，this
will make my patch series easier。
 >And several followed, including in c-code files. So I stopped the build
>and assume you've identified a or several actual issues.
>
>I've seen several commits where changes were made because LLVM flagged
>potentially problematic code, where GCC did not, so it's quite possible
>you're on to something here.
>
>But it would be helpful if the commit message said what code was
>potentially problematic and why. And then the proper fix for that could
>indeed be to include `rockchip_drm_drv.h`.
>
>Cheers,
>  Diederik

Re:Re: [PATCH] drm/rockchip: include rockchip_drm_drv.h

Posted by Min-Hua Chen 1 week, 4 days ago

>>  DTC     arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dtb
>>  DTC     arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-wifi.dtbo
>>  DTC     arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dtb
>>  DTC     arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6b-io.dtb
>>  DTC     arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dtb
>>  DTC     arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dtb
>>../init/main.c:192:12: sparse: warning: symbol 'envp_init' was not declared. Should it be static?
>>../init/main.c:290:16: sparse: warning: cast to restricted __le32
>>../init/main.c:291:16: sparse: warning: cast to restricted __le32
>>  CHECK   ../init/do_mounts.c
>
>>
>
>
>I can see same warnings， a lots of。
>And also see the warning in vop2：
>drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse: warning: symbol 'vop2_platform_driver' was not declared. Should it be static?
>
>
>Min Hua，If you are agree，I will split it from my patch， and add a Fix tag ，and also add a SoB of you， Then resend in My V3 series，this
>will make my patch series easier。

It looks good to me. Thank you and Diederik for commenting.

cheers,
Min-Hua

> >And several followed, including in c-code files. So I stopped the build
>>and assume you've identified a or several actual issues.
>>
>>I've seen several commits where changes were made because LLVM flagged
>>potentially problematic code, where GCC did not, so it's quite possible
>>you're on to something here.
>>
>>But it would be helpful if the commit message said what code was
>>potentially problematic and why. And then the proper fix for that could
>>indeed be to include `rockchip_drm_drv.h`.
>>
>>Cheers,
>>  Diederik

Re:[PATCH] drm/rockchip: include rockchip_drm_drv.h

Posted by Min-Hua Chen 1 week, 6 days ago

Hi,

>Hi  Min Hua，
>   Thanks for your work
>
>At 2024-09-06 06:38:50, "Min-Hua Chen" <minhuadotchen@gmail.com> wrote:
>>Include rockchip_drm_drv.h to fix the follow sparse warning:
>>
>>drivers/gpu/drm/rockchip/rockchip_vop2_reg.c:502:24: sparse:
>>warning: symbol 'vop2_platform_driver' was not declared.
>>Should it be static?
>>
>>No functional change intended.
>>
>>Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com>
>>---
>> drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>>diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
>>index 18efb3fe1c00..c678d1b0fd7c 100644
>>--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
>>+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
>>@@ -14,6 +14,7 @@
>> #include <drm/drm_print.h>
>> 
>> #include "rockchip_drm_vop2.h"
>>+#include "rockchip_drm_drv.h"
>> 
>
>We already have a patch[0] include rockchip_drm_drv.h in rockchip_drm_vop2.h
>
>[0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240904120238.3856782-3-andyshrk@163.com/ 

that's great, thanks for letting me know.

Min-Hua