[PATCH] drm: rcar-du: crtc: force depends on cmm

[PATCH] drm: rcar-du: crtc: force depends on cmm

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM operations"),
the cmm module must be included in the crtc. We cannot remove this
configuration option separately. Therefore, simply linking them together
is the best solution, otherwise some errors will be reported.

 rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
 rcar_du_crtc.c:(.text+0x11bc): undefined reference to `rcar_cmm_enable'
 rcar_du_crtc.c:(.text+0x1604): undefined reference to `rcar_cmm_disable'
 rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'

Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
Reported-by: kernel-bot <k2ci@kylinos.cn>
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
 drivers/gpu/drm/rcar-du/Makefile | 2 +-
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index b47e74421e34..bc71ad2a472f 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -4,7 +4,6 @@ config DRM_RCAR_DU
 	depends on DRM && OF
 	depends on ARM || ARM64
 	depends on ARCH_RENESAS || COMPILE_TEST
-	imply DRM_RCAR_CMM
 	imply DRM_RCAR_LVDS
 	select DRM_KMS_HELPER
 	select DRM_KMS_CMA_HELPER
@@ -14,13 +13,6 @@ config DRM_RCAR_DU
 	  Choose this option if you have an R-Car chipset.
 	  If M is selected the module will be called rcar-du-drm.
 
-config DRM_RCAR_CMM
-	tristate "R-Car DU Color Management Module (CMM) Support"
-	depends on DRM && OF
-	depends on DRM_RCAR_DU
-	help
-	  Enable support for R-Car Color Management Module (CMM).
-
 config DRM_RCAR_DW_HDMI
 	tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
 	depends on DRM && OF
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index 4d1187ccc3e5..76ff2e15bc2d 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
 		 rcar_du_group.o \
 		 rcar_du_kms.o \
 		 rcar_du_plane.o \
+		 rcar_cmm.o
 
 rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
 					   rcar_du_of_lvds_r8a7790.dtb.o \
@@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
 rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
 rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
 
-obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
 obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
 obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
 obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
-- 
2.25.1

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Kieran Bingham]

Hi Jackie,

On 28/07/2021 09:42, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
> 
> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM operations"),
> the cmm module must be included in the crtc. We cannot remove this
> configuration option separately. Therefore, simply linking them together
> is the best solution, otherwise some errors will be reported.
> 

Yikes, I'm sure we've had plenty of problems with the config options on
this module. The churn alone makes me wonder if it should just be part
of the overall module to simplify things... but...

>  rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
>  rcar_du_crtc.c:(.text+0x11bc): undefined reference to `rcar_cmm_enable'
>  rcar_du_crtc.c:(.text+0x1604): undefined reference to `rcar_cmm_disable'
>  rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'

Those are guarded by #if IS_ENABLED in the header.

So from that - perhaps we can assume that the config attempted here was
a built-in DU - but a module CMM which wouldn't be supported?



> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")

That fixes tag is missing a 'd', but that's trivial.


> Reported-by: kernel-bot <k2ci@kylinos.cn>
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
>  drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
>  drivers/gpu/drm/rcar-du/Makefile | 2 +-
>  2 files changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index b47e74421e34..bc71ad2a472f 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
>  	depends on DRM && OF
>  	depends on ARM || ARM64
>  	depends on ARCH_RENESAS || COMPILE_TEST
> -	imply DRM_RCAR_CMM
>  	imply DRM_RCAR_LVDS
>  	select DRM_KMS_HELPER
>  	select DRM_KMS_CMA_HELPER
> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
>  	  Choose this option if you have an R-Car chipset.
>  	  If M is selected the module will be called rcar-du-drm.
>  
> -config DRM_RCAR_CMM
> -	tristate "R-Car DU Color Management Module (CMM) Support"
> -	depends on DRM && OF
> -	depends on DRM_RCAR_DU
> -	help
> -	  Enable support for R-Car Color Management Module (CMM).
> -

I suspect the issue lies somewhere in this config that the CMM is not
being enforced to be a built in when the DU is built in ?


>  config DRM_RCAR_DW_HDMI
>  	tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
>  	depends on DRM && OF
> diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
> index 4d1187ccc3e5..76ff2e15bc2d 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>  		 rcar_du_group.o \
>  		 rcar_du_kms.o \
>  		 rcar_du_plane.o \
> +		 rcar_cmm.o
>  
>  rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
>  					   rcar_du_of_lvds_r8a7790.dtb.o \
> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
>  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
>  rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>  
> -obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
>  obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
>  obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
>  obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
> 

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Jackie Liu]

Hi Kieran.

Thanks for replying to the email so quickly.

在 2021/7/28 下午4:58, Kieran Bingham 写道:
> Hi Jackie,
> 
> On 28/07/2021 09:42, Jackie Liu wrote:
>> From: Jackie Liu <liuyun01@kylinos.cn>
>>
>> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM operations"),
>> the cmm module must be included in the crtc. We cannot remove this
>> configuration option separately. Therefore, simply linking them together
>> is the best solution, otherwise some errors will be reported.
>>
> 
> Yikes, I'm sure we've had plenty of problems with the config options on
> this module. The churn alone makes me wonder if it should just be part
> of the overall module to simplify things... but...
> 
>>   rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
>>   rcar_du_crtc.c:(.text+0x11bc): undefined reference to `rcar_cmm_enable'
>>   rcar_du_crtc.c:(.text+0x1604): undefined reference to `rcar_cmm_disable'
>>   rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'
> 
> Those are guarded by #if IS_ENABLED in the header.
> 
> So from that - perhaps we can assume that the config attempted here was
> a built-in DU - but a module CMM which wouldn't be supported?

I know you want to keep CMM as a module, I might think it’s too simple.
will rewrite the patch.

> 
> 
> 
>> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
> 
> That fixes tag is missing a 'd', but that's trivial.

12 characters of sha is enough.

---
BR, Thanks.
Jackie Liu

> 
> 
>> Reported-by: kernel-bot <k2ci@kylinos.cn>
>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>> ---
>>   drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
>>   drivers/gpu/drm/rcar-du/Makefile | 2 +-
>>   2 files changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
>> index b47e74421e34..bc71ad2a472f 100644
>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
>>   	depends on DRM && OF
>>   	depends on ARM || ARM64
>>   	depends on ARCH_RENESAS || COMPILE_TEST
>> -	imply DRM_RCAR_CMM
>>   	imply DRM_RCAR_LVDS
>>   	select DRM_KMS_HELPER
>>   	select DRM_KMS_CMA_HELPER
>> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
>>   	  Choose this option if you have an R-Car chipset.
>>   	  If M is selected the module will be called rcar-du-drm.
>>   
>> -config DRM_RCAR_CMM
>> -	tristate "R-Car DU Color Management Module (CMM) Support"
>> -	depends on DRM && OF
>> -	depends on DRM_RCAR_DU
>> -	help
>> -	  Enable support for R-Car Color Management Module (CMM).
>> -
> 
> I suspect the issue lies somewhere in this config that the CMM is not
> being enforced to be a built in when the DU is built in ?
> 
> 
>>   config DRM_RCAR_DW_HDMI
>>   	tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
>>   	depends on DRM && OF
>> diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
>> index 4d1187ccc3e5..76ff2e15bc2d 100644
>> --- a/drivers/gpu/drm/rcar-du/Makefile
>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>   		 rcar_du_group.o \
>>   		 rcar_du_kms.o \
>>   		 rcar_du_plane.o \
>> +		 rcar_cmm.o
>>   
>>   rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
>>   					   rcar_du_of_lvds_r8a7790.dtb.o \
>> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
>>   rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
>>   rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>>   
>> -obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
>>   obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
>>   obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
>>   obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
>>

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Kieran Bingham]

On 28/07/2021 10:34, Jackie Liu wrote:
> Hi Kieran.
> 
> Thanks for replying to the email so quickly.
> 
> 在 2021/7/28 下午4:58, Kieran Bingham 写道:
>> Hi Jackie,
>>
>> On 28/07/2021 09:42, Jackie Liu wrote:
>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>
>>> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM
>>> operations"),
>>> the cmm module must be included in the crtc. We cannot remove this
>>> configuration option separately. Therefore, simply linking them together
>>> is the best solution, otherwise some errors will be reported.
>>>
>>
>> Yikes, I'm sure we've had plenty of problems with the config options on
>> this module. The churn alone makes me wonder if it should just be part
>> of the overall module to simplify things... but...
>>
>>>   rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
>>>   rcar_du_crtc.c:(.text+0x11bc): undefined reference to
>>> `rcar_cmm_enable'
>>>   rcar_du_crtc.c:(.text+0x1604): undefined reference to
>>> `rcar_cmm_disable'
>>>   rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'
>>
>> Those are guarded by #if IS_ENABLED in the header.
>>
>> So from that - perhaps we can assume that the config attempted here was
>> a built-in DU - but a module CMM which wouldn't be supported?
> 
> I know you want to keep CMM as a module, I might think it’s too simple.
> will rewrite the patch.

There are DU's which do not have a CMM, so I think that is why it is an
optional feature/module.


>>> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
>>
>> That fixes tag is missing a 'd', but that's trivial.
> 
> 12 characters of sha is enough.

I meant the 'd' of 'drm' (it's 'rm' in your text)

> ---
> BR, Thanks.
> Jackie Liu
> 
>>
>>
>>> Reported-by: kernel-bot <k2ci@kylinos.cn>
>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>> ---
>>>   drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
>>>   drivers/gpu/drm/rcar-du/Makefile | 2 +-
>>>   2 files changed, 1 insertion(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>> index b47e74421e34..bc71ad2a472f 100644
>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
>>>       depends on DRM && OF
>>>       depends on ARM || ARM64
>>>       depends on ARCH_RENESAS || COMPILE_TEST
>>> -    imply DRM_RCAR_CMM
>>>       imply DRM_RCAR_LVDS
>>>       select DRM_KMS_HELPER
>>>       select DRM_KMS_CMA_HELPER
>>> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
>>>         Choose this option if you have an R-Car chipset.
>>>         If M is selected the module will be called rcar-du-drm.
>>>   -config DRM_RCAR_CMM
>>> -    tristate "R-Car DU Color Management Module (CMM) Support"
>>> -    depends on DRM && OF
>>> -    depends on DRM_RCAR_DU
>>> -    help
>>> -      Enable support for R-Car Color Management Module (CMM).
>>> -
>>
>> I suspect the issue lies somewhere in this config that the CMM is not
>> being enforced to be a built in when the DU is built in ?


Checked locally, and indeed KConfig lets us enable the DU as a built in
- but the CMM as a module - that can't be allowed. :-(

--
Kieran


>>
>>
>>>   config DRM_RCAR_DW_HDMI
>>>       tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
>>>       depends on DRM && OF
>>> diff --git a/drivers/gpu/drm/rcar-du/Makefile
>>> b/drivers/gpu/drm/rcar-du/Makefile
>>> index 4d1187ccc3e5..76ff2e15bc2d 100644
>>> --- a/drivers/gpu/drm/rcar-du/Makefile
>>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>>> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>>            rcar_du_group.o \
>>>            rcar_du_kms.o \
>>>            rcar_du_plane.o \
>>> +         rcar_cmm.o
>>>     rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    += rcar_du_of.o \
>>>                          rcar_du_of_lvds_r8a7790.dtb.o \
>>> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    +=
>>> rcar_du_of.o \
>>>   rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)    += rcar_du_vsp.o
>>>   rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>>>   -obj-$(CONFIG_DRM_RCAR_CMM)        += rcar_cmm.o
>>>   obj-$(CONFIG_DRM_RCAR_DU)        += rcar-du-drm.o
>>>   obj-$(CONFIG_DRM_RCAR_DW_HDMI)        += rcar_dw_hdmi.o
>>>   obj-$(CONFIG_DRM_RCAR_LVDS)        += rcar_lvds.o
>>>

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Laurent Pinchart]

Hi Jackie,

On Wed, Jul 28, 2021 at 05:34:38PM +0800, Jackie Liu wrote:
> 在 2021/7/28 下午4:58, Kieran Bingham 写道:
> > On 28/07/2021 09:42, Jackie Liu wrote:
> >> From: Jackie Liu <liuyun01@kylinos.cn>
> >>
> >> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM operations"),
> >> the cmm module must be included in the crtc. We cannot remove this
> >> configuration option separately. Therefore, simply linking them together
> >> is the best solution, otherwise some errors will be reported.
> >>
> > 
> > Yikes, I'm sure we've had plenty of problems with the config options on
> > this module. The churn alone makes me wonder if it should just be part
> > of the overall module to simplify things... but...
> > 
> >>   rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
> >>   rcar_du_crtc.c:(.text+0x11bc): undefined reference to `rcar_cmm_enable'
> >>   rcar_du_crtc.c:(.text+0x1604): undefined reference to `rcar_cmm_disable'
> >>   rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'
> > 
> > Those are guarded by #if IS_ENABLED in the header.
> > 
> > So from that - perhaps we can assume that the config attempted here was
> > a built-in DU - but a module CMM which wouldn't be supported?
> 
> I know you want to keep CMM as a module, I might think it’s too simple.
> will rewrite the patch.
> 
> >> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
> > 
> > That fixes tag is missing a 'd', but that's trivial.
> 
> 12 characters of sha is enough.

I think Kieran meant s/"rm:/"drm:/ :-)

> >> Reported-by: kernel-bot <k2ci@kylinos.cn>
> >> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> >> ---
> >>   drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
> >>   drivers/gpu/drm/rcar-du/Makefile | 2 +-
> >>   2 files changed, 1 insertion(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> >> index b47e74421e34..bc71ad2a472f 100644
> >> --- a/drivers/gpu/drm/rcar-du/Kconfig
> >> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> >> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
> >>   	depends on DRM && OF
> >>   	depends on ARM || ARM64
> >>   	depends on ARCH_RENESAS || COMPILE_TEST
> >> -	imply DRM_RCAR_CMM
> >>   	imply DRM_RCAR_LVDS
> >>   	select DRM_KMS_HELPER
> >>   	select DRM_KMS_CMA_HELPER
> >> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
> >>   	  Choose this option if you have an R-Car chipset.
> >>   	  If M is selected the module will be called rcar-du-drm.
> >>   
> >> -config DRM_RCAR_CMM
> >> -	tristate "R-Car DU Color Management Module (CMM) Support"
> >> -	depends on DRM && OF
> >> -	depends on DRM_RCAR_DU
> >> -	help
> >> -	  Enable support for R-Car Color Management Module (CMM).
> >> -
> > 
> > I suspect the issue lies somewhere in this config that the CMM is not
> > being enforced to be a built in when the DU is built in ?
> > 
> > 
> >>   config DRM_RCAR_DW_HDMI
> >>   	tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
> >>   	depends on DRM && OF
> >> diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
> >> index 4d1187ccc3e5..76ff2e15bc2d 100644
> >> --- a/drivers/gpu/drm/rcar-du/Makefile
> >> +++ b/drivers/gpu/drm/rcar-du/Makefile
> >> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
> >>   		 rcar_du_group.o \
> >>   		 rcar_du_kms.o \
> >>   		 rcar_du_plane.o \
> >> +		 rcar_cmm.o
> >>   
> >>   rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
> >>   					   rcar_du_of_lvds_r8a7790.dtb.o \
> >> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
> >>   rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
> >>   rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
> >>   
> >> -obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
> >>   obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
> >>   obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
> >>   obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Jackie Liu]

Hi Kieran.

How about this.

diff --git a/drivers/gpu/drm/rcar-du/Kconfig 
b/drivers/gpu/drm/rcar-du/Kconfig
index b47e74421e34..14cf3e6415d7 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -1,7 +1,7 @@
  # SPDX-License-Identifier: GPL-2.0
  config DRM_RCAR_DU
         tristate "DRM Support for R-Car Display Unit"
-       depends on DRM && OF
+       depends on DRM && OF && m
         depends on ARM || ARM64
         depends on ARCH_RENESAS || COMPILE_TEST
         imply DRM_RCAR_CMM


Of course, this is not a good way, in fact, as long as rcar-du built-in, 
cmm must also be built-in, otherwise an error will be reported.

Do you have a good way?

Thanks, Jackie.

在 2021/7/28 下午4:58, Kieran Bingham 写道:
> Hi Jackie,
> 
> On 28/07/2021 09:42, Jackie Liu wrote:
>> From: Jackie Liu <liuyun01@kylinos.cn>
>>
>> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM operations"),
>> the cmm module must be included in the crtc. We cannot remove this
>> configuration option separately. Therefore, simply linking them together
>> is the best solution, otherwise some errors will be reported.
>>
> 
> Yikes, I'm sure we've had plenty of problems with the config options on
> this module. The churn alone makes me wonder if it should just be part
> of the overall module to simplify things... but...
> 
>>   rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
>>   rcar_du_crtc.c:(.text+0x11bc): undefined reference to `rcar_cmm_enable'
>>   rcar_du_crtc.c:(.text+0x1604): undefined reference to `rcar_cmm_disable'
>>   rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'
> 
> Those are guarded by #if IS_ENABLED in the header.
> 
> So from that - perhaps we can assume that the config attempted here was
> a built-in DU - but a module CMM which wouldn't be supported?
> 
> 
> 
>> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
> 
> That fixes tag is missing a 'd', but that's trivial.
> 
> 
>> Reported-by: kernel-bot <k2ci@kylinos.cn>
>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>> ---
>>   drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
>>   drivers/gpu/drm/rcar-du/Makefile | 2 +-
>>   2 files changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
>> index b47e74421e34..bc71ad2a472f 100644
>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
>>   	depends on DRM && OF
>>   	depends on ARM || ARM64
>>   	depends on ARCH_RENESAS || COMPILE_TEST
>> -	imply DRM_RCAR_CMM
>>   	imply DRM_RCAR_LVDS
>>   	select DRM_KMS_HELPER
>>   	select DRM_KMS_CMA_HELPER
>> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
>>   	  Choose this option if you have an R-Car chipset.
>>   	  If M is selected the module will be called rcar-du-drm.
>>   
>> -config DRM_RCAR_CMM
>> -	tristate "R-Car DU Color Management Module (CMM) Support"
>> -	depends on DRM && OF
>> -	depends on DRM_RCAR_DU
>> -	help
>> -	  Enable support for R-Car Color Management Module (CMM).
>> -
> 
> I suspect the issue lies somewhere in this config that the CMM is not
> being enforced to be a built in when the DU is built in ?
> 
> 
>>   config DRM_RCAR_DW_HDMI
>>   	tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
>>   	depends on DRM && OF
>> diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
>> index 4d1187ccc3e5..76ff2e15bc2d 100644
>> --- a/drivers/gpu/drm/rcar-du/Makefile
>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>   		 rcar_du_group.o \
>>   		 rcar_du_kms.o \
>>   		 rcar_du_plane.o \
>> +		 rcar_cmm.o
>>   
>>   rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
>>   					   rcar_du_of_lvds_r8a7790.dtb.o \
>> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_of.o \
>>   rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
>>   rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>>   
>> -obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
>>   obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
>>   obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
>>   obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
>>

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Kieran Bingham]

Hi Jackie,

On 28/07/2021 10:57, Jackie Liu wrote:
> Hi Kieran.
> 
> How about this.
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
> b/drivers/gpu/drm/rcar-du/Kconfig
> index b47e74421e34..14cf3e6415d7 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  config DRM_RCAR_DU
>         tristate "DRM Support for R-Car Display Unit"
> -       depends on DRM && OF
> +       depends on DRM && OF && m
>         depends on ARM || ARM64
>         depends on ARCH_RENESAS || COMPILE_TEST
>         imply DRM_RCAR_CMM
> 
> 
> Of course, this is not a good way, in fact, as long as rcar-du built-in,
> cmm must also be built-in, otherwise an error will be reported.

Correct, ideally we should enforce that if the RCAR_DU is built in (y),
then the CMM can only be y/n and not m.

I thought that the depends on DRM_RCAR_DU should do that, but it appears
it doesn't enforce the built-in rule to match...



> 
> Do you have a good way?
> 

Kconfig-language.rst says:

  Note: If the combination of FOO=y and BAR=m causes a link error,
  you can guard the function call with IS_REACHABLE()::

        foo_init()
        {
                if (IS_REACHABLE(CONFIG_BAZ))
                        baz_register(&foo);
                ...
        }


But that seems redundant, so I suspect we could/should change the
drivers/gpu/drm/rcar-du/rcar_cmm.h from:


#if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
to
#if IS_REACHABLE(CONFIG_DRM_RCAR_CMM)

...


Seems odd that we might allow the module to be compiled at all if it
won't be reachable and that we can't enforce that at the KConfig level -
but at least IS_REACHABLE would prevent the linker error..

--
Kieran



> Thanks, Jackie.
> 
> 在 2021/7/28 下午4:58, Kieran Bingham 写道:
>> Hi Jackie,
>>
>> On 28/07/2021 09:42, Jackie Liu wrote:
>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>
>>> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM
>>> operations"),
>>> the cmm module must be included in the crtc. We cannot remove this
>>> configuration option separately. Therefore, simply linking them together
>>> is the best solution, otherwise some errors will be reported.
>>>
>>
>> Yikes, I'm sure we've had plenty of problems with the config options on
>> this module. The churn alone makes me wonder if it should just be part
>> of the overall module to simplify things... but...
>>
>>>   rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
>>>   rcar_du_crtc.c:(.text+0x11bc): undefined reference to
>>> `rcar_cmm_enable'
>>>   rcar_du_crtc.c:(.text+0x1604): undefined reference to
>>> `rcar_cmm_disable'
>>>   rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'
>>
>> Those are guarded by #if IS_ENABLED in the header.
>>
>> So from that - perhaps we can assume that the config attempted here was
>> a built-in DU - but a module CMM which wouldn't be supported?
>>
>>
>>
>>> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
>>
>> That fixes tag is missing a 'd', but that's trivial.
>>
>>
>>> Reported-by: kernel-bot <k2ci@kylinos.cn>
>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>> ---
>>>   drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
>>>   drivers/gpu/drm/rcar-du/Makefile | 2 +-
>>>   2 files changed, 1 insertion(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>> index b47e74421e34..bc71ad2a472f 100644
>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
>>>       depends on DRM && OF
>>>       depends on ARM || ARM64
>>>       depends on ARCH_RENESAS || COMPILE_TEST
>>> -    imply DRM_RCAR_CMM
>>>       imply DRM_RCAR_LVDS
>>>       select DRM_KMS_HELPER
>>>       select DRM_KMS_CMA_HELPER
>>> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
>>>         Choose this option if you have an R-Car chipset.
>>>         If M is selected the module will be called rcar-du-drm.
>>>   -config DRM_RCAR_CMM
>>> -    tristate "R-Car DU Color Management Module (CMM) Support"
>>> -    depends on DRM && OF
>>> -    depends on DRM_RCAR_DU
>>> -    help
>>> -      Enable support for R-Car Color Management Module (CMM).
>>> -
>>
>> I suspect the issue lies somewhere in this config that the CMM is not
>> being enforced to be a built in when the DU is built in ?
>>
>>
>>>   config DRM_RCAR_DW_HDMI
>>>       tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
>>>       depends on DRM && OF
>>> diff --git a/drivers/gpu/drm/rcar-du/Makefile
>>> b/drivers/gpu/drm/rcar-du/Makefile
>>> index 4d1187ccc3e5..76ff2e15bc2d 100644
>>> --- a/drivers/gpu/drm/rcar-du/Makefile
>>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>>> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>>            rcar_du_group.o \
>>>            rcar_du_kms.o \
>>>            rcar_du_plane.o \
>>> +         rcar_cmm.o
>>>     rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    += rcar_du_of.o \
>>>                          rcar_du_of_lvds_r8a7790.dtb.o \
>>> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    +=
>>> rcar_du_of.o \
>>>   rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)    += rcar_du_vsp.o
>>>   rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>>>   -obj-$(CONFIG_DRM_RCAR_CMM)        += rcar_cmm.o
>>>   obj-$(CONFIG_DRM_RCAR_DU)        += rcar-du-drm.o
>>>   obj-$(CONFIG_DRM_RCAR_DW_HDMI)        += rcar_dw_hdmi.o
>>>   obj-$(CONFIG_DRM_RCAR_LVDS)        += rcar_lvds.o
>>>

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Laurent Pinchart]

On Wed, Jul 28, 2021 at 12:09:34PM +0100, Kieran Bingham wrote:
> Hi Jackie,
> 
> On 28/07/2021 10:57, Jackie Liu wrote:
> > Hi Kieran.
> > 
> > How about this.
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/Kconfig
> > b/drivers/gpu/drm/rcar-du/Kconfig
> > index b47e74421e34..14cf3e6415d7 100644
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -1,7 +1,7 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  config DRM_RCAR_DU
> >         tristate "DRM Support for R-Car Display Unit"
> > -       depends on DRM && OF
> > +       depends on DRM && OF && m
> >         depends on ARM || ARM64
> >         depends on ARCH_RENESAS || COMPILE_TEST
> >         imply DRM_RCAR_CMM
> > 
> > 
> > Of course, this is not a good way, in fact, as long as rcar-du built-in,
> > cmm must also be built-in, otherwise an error will be reported.
> 
> Correct, ideally we should enforce that if the RCAR_DU is built in (y),
> then the CMM can only be y/n and not m.
> 
> I thought that the depends on DRM_RCAR_DU should do that, but it appears
> it doesn't enforce the built-in rule to match...
>
> > Do you have a good way?
> 
> Kconfig-language.rst says:
> 
>   Note: If the combination of FOO=y and BAR=m causes a link error,
>   you can guard the function call with IS_REACHABLE()::
> 
>         foo_init()
>         {
>                 if (IS_REACHABLE(CONFIG_BAZ))
>                         baz_register(&foo);
>                 ...
>         }
> 
> 
> But that seems redundant, so I suspect we could/should change the
> drivers/gpu/drm/rcar-du/rcar_cmm.h from:
> 
> 
> #if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
> to
> #if IS_REACHABLE(CONFIG_DRM_RCAR_CMM)
> 
> ...
> 
> 
> Seems odd that we might allow the module to be compiled at all if it
> won't be reachable and that we can't enforce that at the KConfig level -
> but at least IS_REACHABLE would prevent the linker error..

This has been discussed before:

https://lore.kernel.org/dri-devel/20200408202711.1198966-6-arnd@arndb.de/

> > 在 2021/7/28 下午4:58, Kieran Bingham 写道:
> >> On 28/07/2021 09:42, Jackie Liu wrote:
> >>> From: Jackie Liu <liuyun01@kylinos.cn>
> >>>
> >>> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM
> >>> operations"),
> >>> the cmm module must be included in the crtc. We cannot remove this
> >>> configuration option separately. Therefore, simply linking them together
> >>> is the best solution, otherwise some errors will be reported.
> >>>
> >>
> >> Yikes, I'm sure we've had plenty of problems with the config options on
> >> this module. The churn alone makes me wonder if it should just be part
> >> of the overall module to simplify things... but...
> >>
> >>>   rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
> >>>   rcar_du_crtc.c:(.text+0x11bc): undefined reference to
> >>> `rcar_cmm_enable'
> >>>   rcar_du_crtc.c:(.text+0x1604): undefined reference to
> >>> `rcar_cmm_disable'
> >>>   rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'
> >>
> >> Those are guarded by #if IS_ENABLED in the header.
> >>
> >> So from that - perhaps we can assume that the config attempted here was
> >> a built-in DU - but a module CMM which wouldn't be supported?
> >>
> >>
> >>
> >>> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
> >>
> >> That fixes tag is missing a 'd', but that's trivial.
> >>
> >>
> >>> Reported-by: kernel-bot <k2ci@kylinos.cn>
> >>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> >>> ---
> >>>   drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
> >>>   drivers/gpu/drm/rcar-du/Makefile | 2 +-
> >>>   2 files changed, 1 insertion(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
> >>> b/drivers/gpu/drm/rcar-du/Kconfig
> >>> index b47e74421e34..bc71ad2a472f 100644
> >>> --- a/drivers/gpu/drm/rcar-du/Kconfig
> >>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> >>> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
> >>>       depends on DRM && OF
> >>>       depends on ARM || ARM64
> >>>       depends on ARCH_RENESAS || COMPILE_TEST
> >>> -    imply DRM_RCAR_CMM
> >>>       imply DRM_RCAR_LVDS
> >>>       select DRM_KMS_HELPER
> >>>       select DRM_KMS_CMA_HELPER
> >>> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
> >>>         Choose this option if you have an R-Car chipset.
> >>>         If M is selected the module will be called rcar-du-drm.
> >>>   -config DRM_RCAR_CMM
> >>> -    tristate "R-Car DU Color Management Module (CMM) Support"
> >>> -    depends on DRM && OF
> >>> -    depends on DRM_RCAR_DU
> >>> -    help
> >>> -      Enable support for R-Car Color Management Module (CMM).
> >>> -
> >>
> >> I suspect the issue lies somewhere in this config that the CMM is not
> >> being enforced to be a built in when the DU is built in ?
> >>
> >>
> >>>   config DRM_RCAR_DW_HDMI
> >>>       tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
> >>>       depends on DRM && OF
> >>> diff --git a/drivers/gpu/drm/rcar-du/Makefile
> >>> b/drivers/gpu/drm/rcar-du/Makefile
> >>> index 4d1187ccc3e5..76ff2e15bc2d 100644
> >>> --- a/drivers/gpu/drm/rcar-du/Makefile
> >>> +++ b/drivers/gpu/drm/rcar-du/Makefile
> >>> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
> >>>            rcar_du_group.o \
> >>>            rcar_du_kms.o \
> >>>            rcar_du_plane.o \
> >>> +         rcar_cmm.o
> >>>     rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    += rcar_du_of.o \
> >>>                          rcar_du_of_lvds_r8a7790.dtb.o \
> >>> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    +=
> >>> rcar_du_of.o \
> >>>   rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)    += rcar_du_vsp.o
> >>>   rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
> >>>   -obj-$(CONFIG_DRM_RCAR_CMM)        += rcar_cmm.o
> >>>   obj-$(CONFIG_DRM_RCAR_DU)        += rcar-du-drm.o
> >>>   obj-$(CONFIG_DRM_RCAR_DW_HDMI)        += rcar_dw_hdmi.o
> >>>   obj-$(CONFIG_DRM_RCAR_LVDS)        += rcar_lvds.o

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Kieran Bingham]

Hi Jackie / Laurent,

On 28/07/2021 12:30, Laurent Pinchart wrote:
> On Wed, Jul 28, 2021 at 12:09:34PM +0100, Kieran Bingham wrote:
>> Hi Jackie,
>>
>> On 28/07/2021 10:57, Jackie Liu wrote:
>>> Hi Kieran.
>>>
>>> How about this.
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>> index b47e74421e34..14cf3e6415d7 100644
>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>> @@ -1,7 +1,7 @@
>>>  # SPDX-License-Identifier: GPL-2.0
>>>  config DRM_RCAR_DU
>>>         tristate "DRM Support for R-Car Display Unit"
>>> -       depends on DRM && OF
>>> +       depends on DRM && OF && m
>>>         depends on ARM || ARM64
>>>         depends on ARCH_RENESAS || COMPILE_TEST
>>>         imply DRM_RCAR_CMM
>>>
>>>
>>> Of course, this is not a good way, in fact, as long as rcar-du built-in,
>>> cmm must also be built-in, otherwise an error will be reported.
>>
>> Correct, ideally we should enforce that if the RCAR_DU is built in (y),
>> then the CMM can only be y/n and not m.
>>
>> I thought that the depends on DRM_RCAR_DU should do that, but it appears
>> it doesn't enforce the built-in rule to match...
>>
>>> Do you have a good way?
>>
>> Kconfig-language.rst says:
>>
>>   Note: If the combination of FOO=y and BAR=m causes a link error,
>>   you can guard the function call with IS_REACHABLE()::
>>
>>         foo_init()
>>         {
>>                 if (IS_REACHABLE(CONFIG_BAZ))
>>                         baz_register(&foo);
>>                 ...
>>         }
>>
>>
>> But that seems redundant, so I suspect we could/should change the
>> drivers/gpu/drm/rcar-du/rcar_cmm.h from:
>>
>>
>> #if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
>> to
>> #if IS_REACHABLE(CONFIG_DRM_RCAR_CMM)
>>
>> ...
>>
>>
>> Seems odd that we might allow the module to be compiled at all if it
>> won't be reachable and that we can't enforce that at the KConfig level -
>> but at least IS_REACHABLE would prevent the linker error..
> 
> This has been discussed before:
> 
> https://lore.kernel.org/dri-devel/20200408202711.1198966-6-arnd@arndb.de/


Arnd suggested this as a solution which looks like it solves the overall
issue (locally to cmm) with the current restrictions we have...


> In that case, a Makefile trick could also work, doing
> 
> ifdef CONFIG_DRM_RCAR_CMM
> obj-$(CONFIG_DRM_RCAR_DU) += rcar-cmm.o
> endif
> 
> Thereby making the cmm module have the same state (y or m) as
> the du module whenever the option is enabled.

That seems like a reasonable solution to me until someone comes up with
a solution in KConfig.

--
Kieran


>>> 在 2021/7/28 下午4:58, Kieran Bingham 写道:
>>>> On 28/07/2021 09:42, Jackie Liu wrote:
>>>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>>>
>>>>> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM
>>>>> operations"),
>>>>> the cmm module must be included in the crtc. We cannot remove this
>>>>> configuration option separately. Therefore, simply linking them together
>>>>> is the best solution, otherwise some errors will be reported.
>>>>>
>>>>
>>>> Yikes, I'm sure we've had plenty of problems with the config options on
>>>> this module. The churn alone makes me wonder if it should just be part
>>>> of the overall module to simplify things... but...
>>>>
>>>>>   rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
>>>>>   rcar_du_crtc.c:(.text+0x11bc): undefined reference to
>>>>> `rcar_cmm_enable'
>>>>>   rcar_du_crtc.c:(.text+0x1604): undefined reference to
>>>>> `rcar_cmm_disable'
>>>>>   rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'
>>>>
>>>> Those are guarded by #if IS_ENABLED in the header.
>>>>
>>>> So from that - perhaps we can assume that the config attempted here was
>>>> a built-in DU - but a module CMM which wouldn't be supported?
>>>>
>>>>
>>>>
>>>>> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
>>>>
>>>> That fixes tag is missing a 'd', but that's trivial.
>>>>
>>>>
>>>>> Reported-by: kernel-bot <k2ci@kylinos.cn>
>>>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>>>> ---
>>>>>   drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
>>>>>   drivers/gpu/drm/rcar-du/Makefile | 2 +-
>>>>>   2 files changed, 1 insertion(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>>>> index b47e74421e34..bc71ad2a472f 100644
>>>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>>>> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
>>>>>       depends on DRM && OF
>>>>>       depends on ARM || ARM64
>>>>>       depends on ARCH_RENESAS || COMPILE_TEST
>>>>> -    imply DRM_RCAR_CMM
>>>>>       imply DRM_RCAR_LVDS
>>>>>       select DRM_KMS_HELPER
>>>>>       select DRM_KMS_CMA_HELPER
>>>>> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
>>>>>         Choose this option if you have an R-Car chipset.
>>>>>         If M is selected the module will be called rcar-du-drm.
>>>>>   -config DRM_RCAR_CMM
>>>>> -    tristate "R-Car DU Color Management Module (CMM) Support"
>>>>> -    depends on DRM && OF
>>>>> -    depends on DRM_RCAR_DU
>>>>> -    help
>>>>> -      Enable support for R-Car Color Management Module (CMM).
>>>>> -
>>>>
>>>> I suspect the issue lies somewhere in this config that the CMM is not
>>>> being enforced to be a built in when the DU is built in ?
>>>>
>>>>
>>>>>   config DRM_RCAR_DW_HDMI
>>>>>       tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
>>>>>       depends on DRM && OF
>>>>> diff --git a/drivers/gpu/drm/rcar-du/Makefile
>>>>> b/drivers/gpu/drm/rcar-du/Makefile
>>>>> index 4d1187ccc3e5..76ff2e15bc2d 100644
>>>>> --- a/drivers/gpu/drm/rcar-du/Makefile
>>>>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>>>>> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>>>>            rcar_du_group.o \
>>>>>            rcar_du_kms.o \
>>>>>            rcar_du_plane.o \
>>>>> +         rcar_cmm.o
>>>>>     rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    += rcar_du_of.o \
>>>>>                          rcar_du_of_lvds_r8a7790.dtb.o \
>>>>> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    +=
>>>>> rcar_du_of.o \
>>>>>   rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)    += rcar_du_vsp.o
>>>>>   rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>>>>>   -obj-$(CONFIG_DRM_RCAR_CMM)        += rcar_cmm.o
>>>>>   obj-$(CONFIG_DRM_RCAR_DU)        += rcar-du-drm.o
>>>>>   obj-$(CONFIG_DRM_RCAR_DW_HDMI)        += rcar_dw_hdmi.o
>>>>>   obj-$(CONFIG_DRM_RCAR_LVDS)        += rcar_lvds.o
> 

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Jackie Liu]

Hi Kieran.

在 2021/7/28 下午8:13, Kieran Bingham 写道:
> Hi Jackie / Laurent,
> 
> On 28/07/2021 12:30, Laurent Pinchart wrote:
>> On Wed, Jul 28, 2021 at 12:09:34PM +0100, Kieran Bingham wrote:
>>> Hi Jackie,
>>>
>>> On 28/07/2021 10:57, Jackie Liu wrote:
>>>> Hi Kieran.
>>>>
>>>> How about this.
>>>>
>>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>>> index b47e74421e34..14cf3e6415d7 100644
>>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>>> @@ -1,7 +1,7 @@
>>>>   # SPDX-License-Identifier: GPL-2.0
>>>>   config DRM_RCAR_DU
>>>>          tristate "DRM Support for R-Car Display Unit"
>>>> -       depends on DRM && OF
>>>> +       depends on DRM && OF && m
>>>>          depends on ARM || ARM64
>>>>          depends on ARCH_RENESAS || COMPILE_TEST
>>>>          imply DRM_RCAR_CMM
>>>>
>>>>
>>>> Of course, this is not a good way, in fact, as long as rcar-du built-in,
>>>> cmm must also be built-in, otherwise an error will be reported.
>>>
>>> Correct, ideally we should enforce that if the RCAR_DU is built in (y),
>>> then the CMM can only be y/n and not m.
>>>
>>> I thought that the depends on DRM_RCAR_DU should do that, but it appears
>>> it doesn't enforce the built-in rule to match...
>>>
>>>> Do you have a good way?
>>>
>>> Kconfig-language.rst says:
>>>
>>>    Note: If the combination of FOO=y and BAR=m causes a link error,
>>>    you can guard the function call with IS_REACHABLE()::
>>>
>>>          foo_init()
>>>          {
>>>                  if (IS_REACHABLE(CONFIG_BAZ))
>>>                          baz_register(&foo);
>>>                  ...
>>>          }
>>>
>>>
>>> But that seems redundant, so I suspect we could/should change the
>>> drivers/gpu/drm/rcar-du/rcar_cmm.h from:
>>>
>>>
>>> #if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
>>> to
>>> #if IS_REACHABLE(CONFIG_DRM_RCAR_CMM)
>>>
>>> ...
>>>
>>>
>>> Seems odd that we might allow the module to be compiled at all if it
>>> won't be reachable and that we can't enforce that at the KConfig level -
>>> but at least IS_REACHABLE would prevent the linker error..
>>
>> This has been discussed before:
>>
>> https://lore.kernel.org/dri-devel/20200408202711.1198966-6-arnd@arndb.de/
> 
> 
> Arnd suggested this as a solution which looks like it solves the overall
> issue (locally to cmm) with the current restrictions we have...
> 
> 
>> In that case, a Makefile trick could also work, doing
>>
>> ifdef CONFIG_DRM_RCAR_CMM
>> obj-$(CONFIG_DRM_RCAR_DU) += rcar-cmm.o
>> endif
>>
>> Thereby making the cmm module have the same state (y or m) as
>> the du module whenever the option is enabled.
> 
> That seems like a reasonable solution to me until someone comes up with
> a solution in KConfig.
> 

Arnd's suggestion it's much better than me.

But I still propose another solution, but it doesn’t seem very clever.

======
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index ea7e39d03545..23d4f0e1823b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -512,7 +512,9 @@ static void rcar_du_cmm_setup(struct drm_crtc *crtc)
         if (drm_lut)
                 cmm_config.lut.table = (struct drm_color_lut 
*)drm_lut->data;

+#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
         rcar_cmm_setup(rcrtc->cmm, &cmm_config);
+#endif
  }

  /* 
-----------------------------------------------------------------------------
@@ -660,8 +662,10 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc 
*rcrtc)
         if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
                 rcar_du_vsp_disable(rcrtc);

+#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
         if (rcrtc->cmm)
                 rcar_cmm_disable(rcrtc->cmm);
+#endif

         /*
          * Select switch sync mode. This stops display operation and 
configures
@@ -719,8 +723,10 @@ static void rcar_du_crtc_atomic_enable(struct 
drm_crtc *crtc,
         struct rcar_du_crtc_state *rstate = 
to_rcar_crtc_state(crtc->state);
         struct rcar_du_device *rcdu = rcrtc->dev;

+#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
         if (rcrtc->cmm)
                 rcar_cmm_enable(rcrtc->cmm);
+#endif
         rcar_du_crtc_get(rcrtc);

         /*
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c 
b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index fdb8a0d127ad..b6a10fa7aaa4 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -726,7 +726,12 @@ static int rcar_du_cmm_init(struct rcar_du_device 
*rcdu)
                  * -ENODEV is used to report that the CMM config option is
                  * disabled: return 0 and let the DU continue probing.
                  */
-               ret = rcar_cmm_init(pdev);
+               ret =
+#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
+                       rcar_cmm_init(pdev);
+#else
+                       -ENODEV;
                 if (ret) {
                         platform_device_put(pdev);
                         return ret == -ENODEV ? 0 : ret;

It doesn’t look good, but it seems to solve the problem.

--
Jackie Liu


> --
> Kieran
> 
> 
>>>> 在 2021/7/28 下午4:58, Kieran Bingham 写道:
>>>>> On 28/07/2021 09:42, Jackie Liu wrote:
>>>>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>>>>
>>>>>> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM
>>>>>> operations"),
>>>>>> the cmm module must be included in the crtc. We cannot remove this
>>>>>> configuration option separately. Therefore, simply linking them together
>>>>>> is the best solution, otherwise some errors will be reported.
>>>>>>
>>>>>
>>>>> Yikes, I'm sure we've had plenty of problems with the config options on
>>>>> this module. The churn alone makes me wonder if it should just be part
>>>>> of the overall module to simplify things... but...
>>>>>
>>>>>>    rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
>>>>>>    rcar_du_crtc.c:(.text+0x11bc): undefined reference to
>>>>>> `rcar_cmm_enable'
>>>>>>    rcar_du_crtc.c:(.text+0x1604): undefined reference to
>>>>>> `rcar_cmm_disable'
>>>>>>    rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'
>>>>>
>>>>> Those are guarded by #if IS_ENABLED in the header.
>>>>>
>>>>> So from that - perhaps we can assume that the config attempted here was
>>>>> a built-in DU - but a module CMM which wouldn't be supported?
>>>>>
>>>>>
>>>>>
>>>>>> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
>>>>>
>>>>> That fixes tag is missing a 'd', but that's trivial.
>>>>>
>>>>>
>>>>>> Reported-by: kernel-bot <k2ci@kylinos.cn>
>>>>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>>>>> ---
>>>>>>    drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
>>>>>>    drivers/gpu/drm/rcar-du/Makefile | 2 +-
>>>>>>    2 files changed, 1 insertion(+), 9 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>>>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>>>>> index b47e74421e34..bc71ad2a472f 100644
>>>>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>>>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>>>>> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
>>>>>>        depends on DRM && OF
>>>>>>        depends on ARM || ARM64
>>>>>>        depends on ARCH_RENESAS || COMPILE_TEST
>>>>>> -    imply DRM_RCAR_CMM
>>>>>>        imply DRM_RCAR_LVDS
>>>>>>        select DRM_KMS_HELPER
>>>>>>        select DRM_KMS_CMA_HELPER
>>>>>> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
>>>>>>          Choose this option if you have an R-Car chipset.
>>>>>>          If M is selected the module will be called rcar-du-drm.
>>>>>>    -config DRM_RCAR_CMM
>>>>>> -    tristate "R-Car DU Color Management Module (CMM) Support"
>>>>>> -    depends on DRM && OF
>>>>>> -    depends on DRM_RCAR_DU
>>>>>> -    help
>>>>>> -      Enable support for R-Car Color Management Module (CMM).
>>>>>> -
>>>>>
>>>>> I suspect the issue lies somewhere in this config that the CMM is not
>>>>> being enforced to be a built in when the DU is built in ?
>>>>>
>>>>>
>>>>>>    config DRM_RCAR_DW_HDMI
>>>>>>        tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
>>>>>>        depends on DRM && OF
>>>>>> diff --git a/drivers/gpu/drm/rcar-du/Makefile
>>>>>> b/drivers/gpu/drm/rcar-du/Makefile
>>>>>> index 4d1187ccc3e5..76ff2e15bc2d 100644
>>>>>> --- a/drivers/gpu/drm/rcar-du/Makefile
>>>>>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>>>>>> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>>>>>             rcar_du_group.o \
>>>>>>             rcar_du_kms.o \
>>>>>>             rcar_du_plane.o \
>>>>>> +         rcar_cmm.o
>>>>>>      rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    += rcar_du_of.o \
>>>>>>                           rcar_du_of_lvds_r8a7790.dtb.o \
>>>>>> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    +=
>>>>>> rcar_du_of.o \
>>>>>>    rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)    += rcar_du_vsp.o
>>>>>>    rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>>>>>>    -obj-$(CONFIG_DRM_RCAR_CMM)        += rcar_cmm.o
>>>>>>    obj-$(CONFIG_DRM_RCAR_DU)        += rcar-du-drm.o
>>>>>>    obj-$(CONFIG_DRM_RCAR_DW_HDMI)        += rcar_dw_hdmi.o
>>>>>>    obj-$(CONFIG_DRM_RCAR_LVDS)        += rcar_lvds.o
>>

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Kieran Bingham]

Hi Jackie

On 28/07/2021 13:21, Jackie Liu wrote:
> Hi Kieran.
> 
> 在 2021/7/28 下午8:13, Kieran Bingham 写道:
>> Hi Jackie / Laurent,
>>
>> On 28/07/2021 12:30, Laurent Pinchart wrote:
>>> On Wed, Jul 28, 2021 at 12:09:34PM +0100, Kieran Bingham wrote:
>>>> Hi Jackie,
>>>>
>>>> On 28/07/2021 10:57, Jackie Liu wrote:
>>>>> Hi Kieran.
>>>>>
>>>>> How about this.
>>>>>
>>>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>>>> index b47e74421e34..14cf3e6415d7 100644
>>>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>>>> @@ -1,7 +1,7 @@
>>>>>   # SPDX-License-Identifier: GPL-2.0
>>>>>   config DRM_RCAR_DU
>>>>>          tristate "DRM Support for R-Car Display Unit"
>>>>> -       depends on DRM && OF
>>>>> +       depends on DRM && OF && m
>>>>>          depends on ARM || ARM64
>>>>>          depends on ARCH_RENESAS || COMPILE_TEST
>>>>>          imply DRM_RCAR_CMM
>>>>>
>>>>>
>>>>> Of course, this is not a good way, in fact, as long as rcar-du
>>>>> built-in,
>>>>> cmm must also be built-in, otherwise an error will be reported.
>>>>
>>>> Correct, ideally we should enforce that if the RCAR_DU is built in (y),
>>>> then the CMM can only be y/n and not m.
>>>>
>>>> I thought that the depends on DRM_RCAR_DU should do that, but it
>>>> appears
>>>> it doesn't enforce the built-in rule to match...
>>>>
>>>>> Do you have a good way?
>>>>
>>>> Kconfig-language.rst says:
>>>>
>>>>    Note: If the combination of FOO=y and BAR=m causes a link error,
>>>>    you can guard the function call with IS_REACHABLE()::
>>>>
>>>>          foo_init()
>>>>          {
>>>>                  if (IS_REACHABLE(CONFIG_BAZ))
>>>>                          baz_register(&foo);
>>>>                  ...
>>>>          }
>>>>
>>>>
>>>> But that seems redundant, so I suspect we could/should change the
>>>> drivers/gpu/drm/rcar-du/rcar_cmm.h from:
>>>>
>>>>
>>>> #if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
>>>> to
>>>> #if IS_REACHABLE(CONFIG_DRM_RCAR_CMM)
>>>>
>>>> ...
>>>>
>>>>
>>>> Seems odd that we might allow the module to be compiled at all if it
>>>> won't be reachable and that we can't enforce that at the KConfig
>>>> level -
>>>> but at least IS_REACHABLE would prevent the linker error..
>>>
>>> This has been discussed before:
>>>
>>> https://lore.kernel.org/dri-devel/20200408202711.1198966-6-arnd@arndb.de/
>>>
>>
>>
>> Arnd suggested this as a solution which looks like it solves the overall
>> issue (locally to cmm) with the current restrictions we have...
>>
>>
>>> In that case, a Makefile trick could also work, doing
>>>
>>> ifdef CONFIG_DRM_RCAR_CMM
>>> obj-$(CONFIG_DRM_RCAR_DU) += rcar-cmm.o
>>> endif
>>>
>>> Thereby making the cmm module have the same state (y or m) as
>>> the du module whenever the option is enabled.
>>
>> That seems like a reasonable solution to me until someone comes up with
>> a solution in KConfig.
>>
> 
> Arnd's suggestion it's much better than me.
> 
> But I still propose another solution, but it doesn’t seem very clever.
> 
> ======
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index ea7e39d03545..23d4f0e1823b 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -512,7 +512,9 @@ static void rcar_du_cmm_setup(struct drm_crtc *crtc)
>         if (drm_lut)
>                 cmm_config.lut.table = (struct drm_color_lut
> *)drm_lut->data;
> 
> +#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))

I think you could replace those lines with

	if (IS_REACHABLE(CONFIG_DRM_RCAR_CMM))


But I think Arnd's solution is cleaner overall and doesn't require
modifying the driver code.


>         rcar_cmm_setup(rcrtc->cmm, &cmm_config);
> +#endif
>  }
> 
>  /*
> -----------------------------------------------------------------------------
> 
> @@ -660,8 +662,10 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc
> *rcrtc)
>         if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>                 rcar_du_vsp_disable(rcrtc);
> 
> +#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
>         if (rcrtc->cmm)
>                 rcar_cmm_disable(rcrtc->cmm);
> +#endif
> 
>         /*
>          * Select switch sync mode. This stops display operation and
> configures
> @@ -719,8 +723,10 @@ static void rcar_du_crtc_atomic_enable(struct
> drm_crtc *crtc,
>         struct rcar_du_crtc_state *rstate =
> to_rcar_crtc_state(crtc->state);
>         struct rcar_du_device *rcdu = rcrtc->dev;
> 
> +#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
>         if (rcrtc->cmm)
>                 rcar_cmm_enable(rcrtc->cmm);
> +#endif
>         rcar_du_crtc_get(rcrtc);
> 
>         /*
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index fdb8a0d127ad..b6a10fa7aaa4 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -726,7 +726,12 @@ static int rcar_du_cmm_init(struct rcar_du_device
> *rcdu)
>                  * -ENODEV is used to report that the CMM config option is
>                  * disabled: return 0 and let the DU continue probing.
>                  */
> -               ret = rcar_cmm_init(pdev);
> +               ret =
> +#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
> +                       rcar_cmm_init(pdev);
> +#else
> +                       -ENODEV;
>                 if (ret) {
>                         platform_device_put(pdev);
>                         return ret == -ENODEV ? 0 : ret;
> 
> It doesn’t look good, but it seems to solve the problem.
> 
> -- 
> Jackie Liu
> 
> 
>> -- 
>> Kieran
>>
>>
>>>>> 在 2021/7/28 下午4:58, Kieran Bingham 写道:
>>>>>> On 28/07/2021 09:42, Jackie Liu wrote:
>>>>>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>>>>>
>>>>>>> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM
>>>>>>> operations"),
>>>>>>> the cmm module must be included in the crtc. We cannot remove this
>>>>>>> configuration option separately. Therefore, simply linking them
>>>>>>> together
>>>>>>> is the best solution, otherwise some errors will be reported.
>>>>>>>
>>>>>>
>>>>>> Yikes, I'm sure we've had plenty of problems with the config
>>>>>> options on
>>>>>> this module. The churn alone makes me wonder if it should just be
>>>>>> part
>>>>>> of the overall module to simplify things... but...
>>>>>>
>>>>>>>    rcar_du_crtc.c:(.text+0x194): undefined reference to
>>>>>>> `rcar_cmm_setup'
>>>>>>>    rcar_du_crtc.c:(.text+0x11bc): undefined reference to
>>>>>>> `rcar_cmm_enable'
>>>>>>>    rcar_du_crtc.c:(.text+0x1604): undefined reference to
>>>>>>> `rcar_cmm_disable'
>>>>>>>    rcar_du_kms.c:(.text+0x768): undefined reference to
>>>>>>> `rcar_cmm_init'
>>>>>>
>>>>>> Those are guarded by #if IS_ENABLED in the header.
>>>>>>
>>>>>> So from that - perhaps we can assume that the config attempted
>>>>>> here was
>>>>>> a built-in DU - but a module CMM which wouldn't be supported?
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
>>>>>>
>>>>>> That fixes tag is missing a 'd', but that's trivial.
>>>>>>
>>>>>>
>>>>>>> Reported-by: kernel-bot <k2ci@kylinos.cn>
>>>>>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>>>>>> ---
>>>>>>>    drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
>>>>>>>    drivers/gpu/drm/rcar-du/Makefile | 2 +-
>>>>>>>    2 files changed, 1 insertion(+), 9 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>>>>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>>>>>> index b47e74421e34..bc71ad2a472f 100644
>>>>>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>>>>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>>>>>> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
>>>>>>>        depends on DRM && OF
>>>>>>>        depends on ARM || ARM64
>>>>>>>        depends on ARCH_RENESAS || COMPILE_TEST
>>>>>>> -    imply DRM_RCAR_CMM
>>>>>>>        imply DRM_RCAR_LVDS
>>>>>>>        select DRM_KMS_HELPER
>>>>>>>        select DRM_KMS_CMA_HELPER
>>>>>>> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
>>>>>>>          Choose this option if you have an R-Car chipset.
>>>>>>>          If M is selected the module will be called rcar-du-drm.
>>>>>>>    -config DRM_RCAR_CMM
>>>>>>> -    tristate "R-Car DU Color Management Module (CMM) Support"
>>>>>>> -    depends on DRM && OF
>>>>>>> -    depends on DRM_RCAR_DU
>>>>>>> -    help
>>>>>>> -      Enable support for R-Car Color Management Module (CMM).
>>>>>>> -
>>>>>>
>>>>>> I suspect the issue lies somewhere in this config that the CMM is not
>>>>>> being enforced to be a built in when the DU is built in ?
>>>>>>
>>>>>>
>>>>>>>    config DRM_RCAR_DW_HDMI
>>>>>>>        tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
>>>>>>>        depends on DRM && OF
>>>>>>> diff --git a/drivers/gpu/drm/rcar-du/Makefile
>>>>>>> b/drivers/gpu/drm/rcar-du/Makefile
>>>>>>> index 4d1187ccc3e5..76ff2e15bc2d 100644
>>>>>>> --- a/drivers/gpu/drm/rcar-du/Makefile
>>>>>>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>>>>>>> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>>>>>>             rcar_du_group.o \
>>>>>>>             rcar_du_kms.o \
>>>>>>>             rcar_du_plane.o \
>>>>>>> +         rcar_cmm.o
>>>>>>>      rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    += rcar_du_of.o \
>>>>>>>                           rcar_du_of_lvds_r8a7790.dtb.o \
>>>>>>> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    +=
>>>>>>> rcar_du_of.o \
>>>>>>>    rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)    += rcar_du_vsp.o
>>>>>>>    rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>>>>>>>    -obj-$(CONFIG_DRM_RCAR_CMM)        += rcar_cmm.o
>>>>>>>    obj-$(CONFIG_DRM_RCAR_DU)        += rcar-du-drm.o
>>>>>>>    obj-$(CONFIG_DRM_RCAR_DW_HDMI)        += rcar_dw_hdmi.o
>>>>>>>    obj-$(CONFIG_DRM_RCAR_LVDS)        += rcar_lvds.o
>>>

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Jackie Liu]

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


Hi Kieran.

在 2021/7/28 下午8:24, Kieran Bingham 写道:
> Hi Jackie
> 
> On 28/07/2021 13:21, Jackie Liu wrote:
>> Hi Kieran.
>>
>> 在 2021/7/28 下午8:13, Kieran Bingham 写道:
>>> Hi Jackie / Laurent,
>>>
>>> On 28/07/2021 12:30, Laurent Pinchart wrote:
>>>> On Wed, Jul 28, 2021 at 12:09:34PM +0100, Kieran Bingham wrote:
>>>>> Hi Jackie,
>>>>>
>>>>> On 28/07/2021 10:57, Jackie Liu wrote:
>>>>>> Hi Kieran.
>>>>>>
>>>>>> How about this.
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>>>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>>>>> index b47e74421e34..14cf3e6415d7 100644
>>>>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>>>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>>>>> @@ -1,7 +1,7 @@
>>>>>>    # SPDX-License-Identifier: GPL-2.0
>>>>>>    config DRM_RCAR_DU
>>>>>>           tristate "DRM Support for R-Car Display Unit"
>>>>>> -       depends on DRM && OF
>>>>>> +       depends on DRM && OF && m
>>>>>>           depends on ARM || ARM64
>>>>>>           depends on ARCH_RENESAS || COMPILE_TEST
>>>>>>           imply DRM_RCAR_CMM
>>>>>>
>>>>>>
>>>>>> Of course, this is not a good way, in fact, as long as rcar-du
>>>>>> built-in,
>>>>>> cmm must also be built-in, otherwise an error will be reported.
>>>>>
>>>>> Correct, ideally we should enforce that if the RCAR_DU is built in (y),
>>>>> then the CMM can only be y/n and not m.
>>>>>
>>>>> I thought that the depends on DRM_RCAR_DU should do that, but it
>>>>> appears
>>>>> it doesn't enforce the built-in rule to match...
>>>>>
>>>>>> Do you have a good way?
>>>>>
>>>>> Kconfig-language.rst says:
>>>>>
>>>>>     Note: If the combination of FOO=y and BAR=m causes a link error,
>>>>>     you can guard the function call with IS_REACHABLE()::
>>>>>
>>>>>           foo_init()
>>>>>           {
>>>>>                   if (IS_REACHABLE(CONFIG_BAZ))
>>>>>                           baz_register(&foo);
>>>>>                   ...
>>>>>           }
>>>>>
>>>>>
>>>>> But that seems redundant, so I suspect we could/should change the
>>>>> drivers/gpu/drm/rcar-du/rcar_cmm.h from:
>>>>>
>>>>>
>>>>> #if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
>>>>> to
>>>>> #if IS_REACHABLE(CONFIG_DRM_RCAR_CMM)
>>>>>
>>>>> ...
>>>>>
>>>>>
>>>>> Seems odd that we might allow the module to be compiled at all if it
>>>>> won't be reachable and that we can't enforce that at the KConfig
>>>>> level -
>>>>> but at least IS_REACHABLE would prevent the linker error..
>>>>
>>>> This has been discussed before:
>>>>
>>>> https://lore.kernel.org/dri-devel/20200408202711.1198966-6-arnd@arndb.de/
>>>>
>>>
>>>
>>> Arnd suggested this as a solution which looks like it solves the overall
>>> issue (locally to cmm) with the current restrictions we have...
>>>
>>>
>>>> In that case, a Makefile trick could also work, doing
>>>>
>>>> ifdef CONFIG_DRM_RCAR_CMM
>>>> obj-$(CONFIG_DRM_RCAR_DU) += rcar-cmm.o
>>>> endif
>>>>
>>>> Thereby making the cmm module have the same state (y or m) as
>>>> the du module whenever the option is enabled.
>>>
>>> That seems like a reasonable solution to me until someone comes up with
>>> a solution in KConfig.
>>>
>>
>> Arnd's suggestion it's much better than me.
>>
>> But I still propose another solution, but it doesn’t seem very clever.
>>
>> ======
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> index ea7e39d03545..23d4f0e1823b 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> @@ -512,7 +512,9 @@ static void rcar_du_cmm_setup(struct drm_crtc *crtc)
>>          if (drm_lut)
>>                  cmm_config.lut.table = (struct drm_color_lut
>> *)drm_lut->data;
>>
>> +#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
> 
> I think you could replace those lines with
> 
> 	if (IS_REACHABLE(CONFIG_DRM_RCAR_CMM))
> 
> 
> But I think Arnd's solution is cleaner overall and doesn't require
> modifying the driver code.

Cool!!!

If you decide to apply this patch, you can add my tag:

Reviewed-by: Jackie Liu <liuyun01@kylinos.cn>

Thanks.
--
Jackie Liu

> 
> 
>>          rcar_cmm_setup(rcrtc->cmm, &cmm_config);
>> +#endif
>>   }
>>
>>   /*
>> -----------------------------------------------------------------------------
>>
>> @@ -660,8 +662,10 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc
>> *rcrtc)
>>          if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
>>                  rcar_du_vsp_disable(rcrtc);
>>
>> +#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
>>          if (rcrtc->cmm)
>>                  rcar_cmm_disable(rcrtc->cmm);
>> +#endif
>>
>>          /*
>>           * Select switch sync mode. This stops display operation and
>> configures
>> @@ -719,8 +723,10 @@ static void rcar_du_crtc_atomic_enable(struct
>> drm_crtc *crtc,
>>          struct rcar_du_crtc_state *rstate =
>> to_rcar_crtc_state(crtc->state);
>>          struct rcar_du_device *rcdu = rcrtc->dev;
>>
>> +#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
>>          if (rcrtc->cmm)
>>                  rcar_cmm_enable(rcrtc->cmm);
>> +#endif
>>          rcar_du_crtc_get(rcrtc);
>>
>>          /*
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> index fdb8a0d127ad..b6a10fa7aaa4 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> @@ -726,7 +726,12 @@ static int rcar_du_cmm_init(struct rcar_du_device
>> *rcdu)
>>                   * -ENODEV is used to report that the CMM config option is
>>                   * disabled: return 0 and let the DU continue probing.
>>                   */
>> -               ret = rcar_cmm_init(pdev);
>> +               ret =
>> +#if !(IS_MODULE(CONFIG_DRM_RCAR_CMM) && IS_BUILTIN(CONFIG_DRM_RCAR_DU))
>> +                       rcar_cmm_init(pdev);
>> +#else
>> +                       -ENODEV;
>>                  if (ret) {
>>                          platform_device_put(pdev);
>>                          return ret == -ENODEV ? 0 : ret;
>>
>> It doesn’t look good, but it seems to solve the problem.
>>
>> -- 
>> Jackie Liu
>>
>>
>>> -- 
>>> Kieran
>>>
>>>
>>>>>> 在 2021/7/28 下午4:58, Kieran Bingham 写道:
>>>>>>> On 28/07/2021 09:42, Jackie Liu wrote:
>>>>>>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>>>>>>
>>>>>>>> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM
>>>>>>>> operations"),
>>>>>>>> the cmm module must be included in the crtc. We cannot remove this
>>>>>>>> configuration option separately. Therefore, simply linking them
>>>>>>>> together
>>>>>>>> is the best solution, otherwise some errors will be reported.
>>>>>>>>
>>>>>>>
>>>>>>> Yikes, I'm sure we've had plenty of problems with the config
>>>>>>> options on
>>>>>>> this module. The churn alone makes me wonder if it should just be
>>>>>>> part
>>>>>>> of the overall module to simplify things... but...
>>>>>>>
>>>>>>>>     rcar_du_crtc.c:(.text+0x194): undefined reference to
>>>>>>>> `rcar_cmm_setup'
>>>>>>>>     rcar_du_crtc.c:(.text+0x11bc): undefined reference to
>>>>>>>> `rcar_cmm_enable'
>>>>>>>>     rcar_du_crtc.c:(.text+0x1604): undefined reference to
>>>>>>>> `rcar_cmm_disable'
>>>>>>>>     rcar_du_kms.c:(.text+0x768): undefined reference to
>>>>>>>> `rcar_cmm_init'
>>>>>>>
>>>>>>> Those are guarded by #if IS_ENABLED in the header.
>>>>>>>
>>>>>>> So from that - perhaps we can assume that the config attempted
>>>>>>> here was
>>>>>>> a built-in DU - but a module CMM which wouldn't be supported?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
>>>>>>>
>>>>>>> That fixes tag is missing a 'd', but that's trivial.
>>>>>>>
>>>>>>>
>>>>>>>> Reported-by: kernel-bot <k2ci@kylinos.cn>
>>>>>>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>>>>>>> ---
>>>>>>>>     drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
>>>>>>>>     drivers/gpu/drm/rcar-du/Makefile | 2 +-
>>>>>>>>     2 files changed, 1 insertion(+), 9 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>>>>>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>>>>>>> index b47e74421e34..bc71ad2a472f 100644
>>>>>>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>>>>>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>>>>>>> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
>>>>>>>>         depends on DRM && OF
>>>>>>>>         depends on ARM || ARM64
>>>>>>>>         depends on ARCH_RENESAS || COMPILE_TEST
>>>>>>>> -    imply DRM_RCAR_CMM
>>>>>>>>         imply DRM_RCAR_LVDS
>>>>>>>>         select DRM_KMS_HELPER
>>>>>>>>         select DRM_KMS_CMA_HELPER
>>>>>>>> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
>>>>>>>>           Choose this option if you have an R-Car chipset.
>>>>>>>>           If M is selected the module will be called rcar-du-drm.
>>>>>>>>     -config DRM_RCAR_CMM
>>>>>>>> -    tristate "R-Car DU Color Management Module (CMM) Support"
>>>>>>>> -    depends on DRM && OF
>>>>>>>> -    depends on DRM_RCAR_DU
>>>>>>>> -    help
>>>>>>>> -      Enable support for R-Car Color Management Module (CMM).
>>>>>>>> -
>>>>>>>
>>>>>>> I suspect the issue lies somewhere in this config that the CMM is not
>>>>>>> being enforced to be a built in when the DU is built in ?
>>>>>>>
>>>>>>>
>>>>>>>>     config DRM_RCAR_DW_HDMI
>>>>>>>>         tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
>>>>>>>>         depends on DRM && OF
>>>>>>>> diff --git a/drivers/gpu/drm/rcar-du/Makefile
>>>>>>>> b/drivers/gpu/drm/rcar-du/Makefile
>>>>>>>> index 4d1187ccc3e5..76ff2e15bc2d 100644
>>>>>>>> --- a/drivers/gpu/drm/rcar-du/Makefile
>>>>>>>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>>>>>>>> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>>>>>>>              rcar_du_group.o \
>>>>>>>>              rcar_du_kms.o \
>>>>>>>>              rcar_du_plane.o \
>>>>>>>> +         rcar_cmm.o
>>>>>>>>       rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    += rcar_du_of.o \
>>>>>>>>                            rcar_du_of_lvds_r8a7790.dtb.o \
>>>>>>>> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    +=
>>>>>>>> rcar_du_of.o \
>>>>>>>>     rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)    += rcar_du_vsp.o
>>>>>>>>     rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>>>>>>>>     -obj-$(CONFIG_DRM_RCAR_CMM)        += rcar_cmm.o
>>>>>>>>     obj-$(CONFIG_DRM_RCAR_DU)        += rcar-du-drm.o
>>>>>>>>     obj-$(CONFIG_DRM_RCAR_DW_HDMI)        += rcar_dw_hdmi.o
>>>>>>>>     obj-$(CONFIG_DRM_RCAR_LVDS)        += rcar_lvds.o
>>>>

[-- Attachment #2: Type: text/plain, Size: 82 bytes --]

Content-type: Text/plain

No virus found
		Checked by Hillstone Network AntiVirus

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[Jackie Liu]

在 2021/7/28 下午7:30, Laurent Pinchart 写道:
> On Wed, Jul 28, 2021 at 12:09:34PM +0100, Kieran Bingham wrote:
>> Hi Jackie,
>>
>> On 28/07/2021 10:57, Jackie Liu wrote:
>>> Hi Kieran.
>>>
>>> How about this.
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>> index b47e74421e34..14cf3e6415d7 100644
>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>> @@ -1,7 +1,7 @@
>>>   # SPDX-License-Identifier: GPL-2.0
>>>   config DRM_RCAR_DU
>>>          tristate "DRM Support for R-Car Display Unit"
>>> -       depends on DRM && OF
>>> +       depends on DRM && OF && m
>>>          depends on ARM || ARM64
>>>          depends on ARCH_RENESAS || COMPILE_TEST
>>>          imply DRM_RCAR_CMM
>>>
>>>
>>> Of course, this is not a good way, in fact, as long as rcar-du built-in,
>>> cmm must also be built-in, otherwise an error will be reported.
>>
>> Correct, ideally we should enforce that if the RCAR_DU is built in (y),
>> then the CMM can only be y/n and not m.
>>
>> I thought that the depends on DRM_RCAR_DU should do that, but it appears
>> it doesn't enforce the built-in rule to match...
>>
>>> Do you have a good way?
>>
>> Kconfig-language.rst says:
>>
>>    Note: If the combination of FOO=y and BAR=m causes a link error,
>>    you can guard the function call with IS_REACHABLE()::
>>
>>          foo_init()
>>          {
>>                  if (IS_REACHABLE(CONFIG_BAZ))
>>                          baz_register(&foo);
>>                  ...
>>          }
>>
>>
>> But that seems redundant, so I suspect we could/should change the
>> drivers/gpu/drm/rcar-du/rcar_cmm.h from:
>>
>>
>> #if IS_ENABLED(CONFIG_DRM_RCAR_CMM)
>> to
>> #if IS_REACHABLE(CONFIG_DRM_RCAR_CMM)
>>
>> ...
>>
>>
>> Seems odd that we might allow the module to be compiled at all if it
>> won't be reachable and that we can't enforce that at the KConfig level -
>> but at least IS_REACHABLE would prevent the linker error..
> 
> This has been discussed before:
> 
> https://lore.kernel.org/dri-devel/20200408202711.1198966-6-arnd@arndb.de/

Hi Laurent.

Thanks for tell me this.

-- 
Jackie Liu

> 
>>> 在 2021/7/28 下午4:58, Kieran Bingham 写道:
>>>> On 28/07/2021 09:42, Jackie Liu wrote:
>>>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>>>
>>>>> After the patch 78b6bb1d24db ("drm: rcar-du: crtc: Control CMM
>>>>> operations"),
>>>>> the cmm module must be included in the crtc. We cannot remove this
>>>>> configuration option separately. Therefore, simply linking them together
>>>>> is the best solution, otherwise some errors will be reported.
>>>>>
>>>>
>>>> Yikes, I'm sure we've had plenty of problems with the config options on
>>>> this module. The churn alone makes me wonder if it should just be part
>>>> of the overall module to simplify things... but...
>>>>
>>>>>    rcar_du_crtc.c:(.text+0x194): undefined reference to `rcar_cmm_setup'
>>>>>    rcar_du_crtc.c:(.text+0x11bc): undefined reference to
>>>>> `rcar_cmm_enable'
>>>>>    rcar_du_crtc.c:(.text+0x1604): undefined reference to
>>>>> `rcar_cmm_disable'
>>>>>    rcar_du_kms.c:(.text+0x768): undefined reference to `rcar_cmm_init'
>>>>
>>>> Those are guarded by #if IS_ENABLED in the header.
>>>>
>>>> So from that - perhaps we can assume that the config attempted here was
>>>> a built-in DU - but a module CMM which wouldn't be supported?
>>>>
>>>>
>>>>
>>>>> Fixes: 78b6bb1d24db ("rm: rcar-du: crtc: Control CMM operations")
>>>>
>>>> That fixes tag is missing a 'd', but that's trivial.
>>>>
>>>>
>>>>> Reported-by: kernel-bot <k2ci@kylinos.cn>
>>>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>>>> ---
>>>>>    drivers/gpu/drm/rcar-du/Kconfig  | 8 --------
>>>>>    drivers/gpu/drm/rcar-du/Makefile | 2 +-
>>>>>    2 files changed, 1 insertion(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig
>>>>> b/drivers/gpu/drm/rcar-du/Kconfig
>>>>> index b47e74421e34..bc71ad2a472f 100644
>>>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>>>> @@ -4,7 +4,6 @@ config DRM_RCAR_DU
>>>>>        depends on DRM && OF
>>>>>        depends on ARM || ARM64
>>>>>        depends on ARCH_RENESAS || COMPILE_TEST
>>>>> -    imply DRM_RCAR_CMM
>>>>>        imply DRM_RCAR_LVDS
>>>>>        select DRM_KMS_HELPER
>>>>>        select DRM_KMS_CMA_HELPER
>>>>> @@ -14,13 +13,6 @@ config DRM_RCAR_DU
>>>>>          Choose this option if you have an R-Car chipset.
>>>>>          If M is selected the module will be called rcar-du-drm.
>>>>>    -config DRM_RCAR_CMM
>>>>> -    tristate "R-Car DU Color Management Module (CMM) Support"
>>>>> -    depends on DRM && OF
>>>>> -    depends on DRM_RCAR_DU
>>>>> -    help
>>>>> -      Enable support for R-Car Color Management Module (CMM).
>>>>> -
>>>>
>>>> I suspect the issue lies somewhere in this config that the CMM is not
>>>> being enforced to be a built in when the DU is built in ?
>>>>
>>>>
>>>>>    config DRM_RCAR_DW_HDMI
>>>>>        tristate "R-Car Gen3 and RZ/G2 DU HDMI Encoder Support"
>>>>>        depends on DRM && OF
>>>>> diff --git a/drivers/gpu/drm/rcar-du/Makefile
>>>>> b/drivers/gpu/drm/rcar-du/Makefile
>>>>> index 4d1187ccc3e5..76ff2e15bc2d 100644
>>>>> --- a/drivers/gpu/drm/rcar-du/Makefile
>>>>> +++ b/drivers/gpu/drm/rcar-du/Makefile
>>>>> @@ -5,6 +5,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>>>>>             rcar_du_group.o \
>>>>>             rcar_du_kms.o \
>>>>>             rcar_du_plane.o \
>>>>> +         rcar_cmm.o
>>>>>      rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    += rcar_du_of.o \
>>>>>                           rcar_du_of_lvds_r8a7790.dtb.o \
>>>>> @@ -15,7 +16,6 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)    +=
>>>>> rcar_du_of.o \
>>>>>    rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)    += rcar_du_vsp.o
>>>>>    rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>>>>>    -obj-$(CONFIG_DRM_RCAR_CMM)        += rcar_cmm.o
>>>>>    obj-$(CONFIG_DRM_RCAR_DU)        += rcar-du-drm.o
>>>>>    obj-$(CONFIG_DRM_RCAR_DW_HDMI)        += rcar_dw_hdmi.o
>>>>>    obj-$(CONFIG_DRM_RCAR_LVDS)        += rcar_lvds.o
> 

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[kernel test robot]

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

Hi Jackie,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pinchartl-media/drm/du/next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.14-rc3 next-20210727]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jackie-Liu/drm-rcar-du-crtc-force-depends-on-cmm/20210728-222353
base:   git://linuxtv.org/pinchartl/media.git drm/du/next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/d8277431962a65912c00e968b5df7bf103cda67a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jackie-Liu/drm-rcar-du-crtc-force-depends-on-cmm/20210728-222353
        git checkout d8277431962a65912c00e968b5df7bf103cda67a
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash drivers/gpu/drm/rcar-du/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/rcar-du/rcar_cmm.c:81:5: error: redefinition of 'rcar_cmm_setup'
      81 | int rcar_cmm_setup(struct platform_device *pdev,
         |     ^~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/rcar-du/rcar_cmm.c:16:
   drivers/gpu/drm/rcar-du/rcar_cmm.h:51:19: note: previous definition of 'rcar_cmm_setup' was here
      51 | static inline int rcar_cmm_setup(struct platform_device *pdev,
         |                   ^~~~~~~~~~~~~~
>> drivers/gpu/drm/rcar-du/rcar_cmm.c:121:5: error: redefinition of 'rcar_cmm_enable'
     121 | int rcar_cmm_enable(struct platform_device *pdev)
         |     ^~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/rcar-du/rcar_cmm.c:16:
   drivers/gpu/drm/rcar-du/rcar_cmm.h:42:19: note: previous definition of 'rcar_cmm_enable' was here
      42 | static inline int rcar_cmm_enable(struct platform_device *pdev)
         |                   ^~~~~~~~~~~~~~~
>> drivers/gpu/drm/rcar-du/rcar_cmm.c:143:6: error: redefinition of 'rcar_cmm_disable'
     143 | void rcar_cmm_disable(struct platform_device *pdev)
         |      ^~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/rcar-du/rcar_cmm.c:16:
   drivers/gpu/drm/rcar-du/rcar_cmm.h:47:20: note: previous definition of 'rcar_cmm_disable' was here
      47 | static inline void rcar_cmm_disable(struct platform_device *pdev)
         |                    ^~~~~~~~~~~~~~~~
>> drivers/gpu/drm/rcar-du/rcar_cmm.c:161:5: error: redefinition of 'rcar_cmm_init'
     161 | int rcar_cmm_init(struct platform_device *pdev)
         |     ^~~~~~~~~~~~~
   In file included from drivers/gpu/drm/rcar-du/rcar_cmm.c:16:
   drivers/gpu/drm/rcar-du/rcar_cmm.h:37:19: note: previous definition of 'rcar_cmm_init' was here
      37 | static inline int rcar_cmm_init(struct platform_device *pdev)
         |                   ^~~~~~~~~~~~~


vim +/rcar_cmm_setup +81 drivers/gpu/drm/rcar-du/rcar_cmm.c

e08e934d6c289ed Jacopo Mondi  2019-10-17   64  
e08e934d6c289ed Jacopo Mondi  2019-10-17   65  /*
e08e934d6c289ed Jacopo Mondi  2019-10-17   66   * rcar_cmm_setup() - Configure the CMM unit
e08e934d6c289ed Jacopo Mondi  2019-10-17   67   * @pdev: The platform device associated with the CMM instance
e08e934d6c289ed Jacopo Mondi  2019-10-17   68   * @config: The CMM unit configuration
e08e934d6c289ed Jacopo Mondi  2019-10-17   69   *
e08e934d6c289ed Jacopo Mondi  2019-10-17   70   * Configure the CMM unit with the given configuration. Currently enabling,
e08e934d6c289ed Jacopo Mondi  2019-10-17   71   * disabling and programming of the 1-D LUT unit is supported.
e08e934d6c289ed Jacopo Mondi  2019-10-17   72   *
e08e934d6c289ed Jacopo Mondi  2019-10-17   73   * As rcar_cmm_setup() accesses the CMM registers the unit should be powered
e08e934d6c289ed Jacopo Mondi  2019-10-17   74   * and its functional clock enabled. To guarantee this, before any call to
e08e934d6c289ed Jacopo Mondi  2019-10-17   75   * this function is made, the CMM unit has to be enabled by calling
e08e934d6c289ed Jacopo Mondi  2019-10-17   76   * rcar_cmm_enable() first.
e08e934d6c289ed Jacopo Mondi  2019-10-17   77   *
e08e934d6c289ed Jacopo Mondi  2019-10-17   78   * TODO: Add support for LUT double buffer operations to avoid updating the
e08e934d6c289ed Jacopo Mondi  2019-10-17   79   * LUT table entries while a frame is being displayed.
e08e934d6c289ed Jacopo Mondi  2019-10-17   80   */
e08e934d6c289ed Jacopo Mondi  2019-10-17  @81  int rcar_cmm_setup(struct platform_device *pdev,
e08e934d6c289ed Jacopo Mondi  2019-10-17   82  		   const struct rcar_cmm_config *config)
e08e934d6c289ed Jacopo Mondi  2019-10-17   83  {
e08e934d6c289ed Jacopo Mondi  2019-10-17   84  	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
e08e934d6c289ed Jacopo Mondi  2019-10-17   85  
e08e934d6c289ed Jacopo Mondi  2019-10-17   86  	/* Disable LUT if no table is provided. */
e08e934d6c289ed Jacopo Mondi  2019-10-17   87  	if (!config->lut.table) {
e08e934d6c289ed Jacopo Mondi  2019-10-17   88  		if (rcmm->lut.enabled) {
e08e934d6c289ed Jacopo Mondi  2019-10-17   89  			rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
e08e934d6c289ed Jacopo Mondi  2019-10-17   90  			rcmm->lut.enabled = false;
e08e934d6c289ed Jacopo Mondi  2019-10-17   91  		}
e08e934d6c289ed Jacopo Mondi  2019-10-17   92  
e08e934d6c289ed Jacopo Mondi  2019-10-17   93  		return 0;
e08e934d6c289ed Jacopo Mondi  2019-10-17   94  	}
e08e934d6c289ed Jacopo Mondi  2019-10-17   95  
e08e934d6c289ed Jacopo Mondi  2019-10-17   96  	/* Enable LUT and program the new gamma table values. */
e08e934d6c289ed Jacopo Mondi  2019-10-17   97  	if (!rcmm->lut.enabled) {
e08e934d6c289ed Jacopo Mondi  2019-10-17   98  		rcar_cmm_write(rcmm, CM2_LUT_CTRL, CM2_LUT_CTRL_LUT_EN);
e08e934d6c289ed Jacopo Mondi  2019-10-17   99  		rcmm->lut.enabled = true;
e08e934d6c289ed Jacopo Mondi  2019-10-17  100  	}
e08e934d6c289ed Jacopo Mondi  2019-10-17  101  
e08e934d6c289ed Jacopo Mondi  2019-10-17  102  	rcar_cmm_lut_write(rcmm, config->lut.table);
e08e934d6c289ed Jacopo Mondi  2019-10-17  103  
e08e934d6c289ed Jacopo Mondi  2019-10-17  104  	return 0;
e08e934d6c289ed Jacopo Mondi  2019-10-17  105  }
e08e934d6c289ed Jacopo Mondi  2019-10-17  106  EXPORT_SYMBOL_GPL(rcar_cmm_setup);
e08e934d6c289ed Jacopo Mondi  2019-10-17  107  
e08e934d6c289ed Jacopo Mondi  2019-10-17  108  /*
e08e934d6c289ed Jacopo Mondi  2019-10-17  109   * rcar_cmm_enable() - Enable the CMM unit
e08e934d6c289ed Jacopo Mondi  2019-10-17  110   * @pdev: The platform device associated with the CMM instance
e08e934d6c289ed Jacopo Mondi  2019-10-17  111   *
e08e934d6c289ed Jacopo Mondi  2019-10-17  112   * When the output of the corresponding DU channel is routed to the CMM unit,
e08e934d6c289ed Jacopo Mondi  2019-10-17  113   * the unit shall be enabled before the DU channel is started, and remain
e08e934d6c289ed Jacopo Mondi  2019-10-17  114   * enabled until the channel is stopped. The CMM unit shall be disabled with
e08e934d6c289ed Jacopo Mondi  2019-10-17  115   * rcar_cmm_disable().
e08e934d6c289ed Jacopo Mondi  2019-10-17  116   *
e08e934d6c289ed Jacopo Mondi  2019-10-17  117   * Calls to rcar_cmm_enable() and rcar_cmm_disable() are not reference-counted.
e08e934d6c289ed Jacopo Mondi  2019-10-17  118   * It is an error to attempt to enable an already enabled CMM unit, or to
e08e934d6c289ed Jacopo Mondi  2019-10-17  119   * attempt to disable a disabled unit.
e08e934d6c289ed Jacopo Mondi  2019-10-17  120   */
e08e934d6c289ed Jacopo Mondi  2019-10-17 @121  int rcar_cmm_enable(struct platform_device *pdev)
e08e934d6c289ed Jacopo Mondi  2019-10-17  122  {
e08e934d6c289ed Jacopo Mondi  2019-10-17  123  	int ret;
e08e934d6c289ed Jacopo Mondi  2019-10-17  124  
136ce7684bc1ff4 Qinglang Miao 2020-11-27  125  	ret = pm_runtime_resume_and_get(&pdev->dev);
e08e934d6c289ed Jacopo Mondi  2019-10-17  126  	if (ret < 0)
e08e934d6c289ed Jacopo Mondi  2019-10-17  127  		return ret;
e08e934d6c289ed Jacopo Mondi  2019-10-17  128  
e08e934d6c289ed Jacopo Mondi  2019-10-17  129  	return 0;
e08e934d6c289ed Jacopo Mondi  2019-10-17  130  }
e08e934d6c289ed Jacopo Mondi  2019-10-17  131  EXPORT_SYMBOL_GPL(rcar_cmm_enable);
e08e934d6c289ed Jacopo Mondi  2019-10-17  132  
e08e934d6c289ed Jacopo Mondi  2019-10-17  133  /*
e08e934d6c289ed Jacopo Mondi  2019-10-17  134   * rcar_cmm_disable() - Disable the CMM unit
e08e934d6c289ed Jacopo Mondi  2019-10-17  135   * @pdev: The platform device associated with the CMM instance
e08e934d6c289ed Jacopo Mondi  2019-10-17  136   *
e08e934d6c289ed Jacopo Mondi  2019-10-17  137   * See rcar_cmm_enable() for usage information.
e08e934d6c289ed Jacopo Mondi  2019-10-17  138   *
e08e934d6c289ed Jacopo Mondi  2019-10-17  139   * Disabling the CMM unit disable all the internal processing blocks. The CMM
e08e934d6c289ed Jacopo Mondi  2019-10-17  140   * state shall thus be restored with rcar_cmm_setup() when re-enabling the CMM
e08e934d6c289ed Jacopo Mondi  2019-10-17  141   * unit after the next rcar_cmm_enable() call.
e08e934d6c289ed Jacopo Mondi  2019-10-17  142   */
e08e934d6c289ed Jacopo Mondi  2019-10-17 @143  void rcar_cmm_disable(struct platform_device *pdev)
e08e934d6c289ed Jacopo Mondi  2019-10-17  144  {
e08e934d6c289ed Jacopo Mondi  2019-10-17  145  	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
e08e934d6c289ed Jacopo Mondi  2019-10-17  146  
e08e934d6c289ed Jacopo Mondi  2019-10-17  147  	rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
e08e934d6c289ed Jacopo Mondi  2019-10-17  148  	rcmm->lut.enabled = false;
e08e934d6c289ed Jacopo Mondi  2019-10-17  149  
e08e934d6c289ed Jacopo Mondi  2019-10-17  150  	pm_runtime_put(&pdev->dev);
e08e934d6c289ed Jacopo Mondi  2019-10-17  151  }
e08e934d6c289ed Jacopo Mondi  2019-10-17  152  EXPORT_SYMBOL_GPL(rcar_cmm_disable);
e08e934d6c289ed Jacopo Mondi  2019-10-17  153  
e08e934d6c289ed Jacopo Mondi  2019-10-17  154  /*
e08e934d6c289ed Jacopo Mondi  2019-10-17  155   * rcar_cmm_init() - Initialize the CMM unit
e08e934d6c289ed Jacopo Mondi  2019-10-17  156   * @pdev: The platform device associated with the CMM instance
e08e934d6c289ed Jacopo Mondi  2019-10-17  157   *
e08e934d6c289ed Jacopo Mondi  2019-10-17  158   * Return: 0 on success, -EPROBE_DEFER if the CMM is not available yet,
e08e934d6c289ed Jacopo Mondi  2019-10-17  159   *         -ENODEV if the DRM_RCAR_CMM config option is disabled
e08e934d6c289ed Jacopo Mondi  2019-10-17  160   */
e08e934d6c289ed Jacopo Mondi  2019-10-17 @161  int rcar_cmm_init(struct platform_device *pdev)
e08e934d6c289ed Jacopo Mondi  2019-10-17  162  {
e08e934d6c289ed Jacopo Mondi  2019-10-17  163  	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
e08e934d6c289ed Jacopo Mondi  2019-10-17  164  
e08e934d6c289ed Jacopo Mondi  2019-10-17  165  	if (!rcmm)
e08e934d6c289ed Jacopo Mondi  2019-10-17  166  		return -EPROBE_DEFER;
e08e934d6c289ed Jacopo Mondi  2019-10-17  167  
e08e934d6c289ed Jacopo Mondi  2019-10-17  168  	return 0;
e08e934d6c289ed Jacopo Mondi  2019-10-17  169  }
e08e934d6c289ed Jacopo Mondi  2019-10-17  170  EXPORT_SYMBOL_GPL(rcar_cmm_init);
e08e934d6c289ed Jacopo Mondi  2019-10-17  171  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54032 bytes --]

Re: [PATCH] drm: rcar-du: crtc: force depends on cmm

[kernel test robot]

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

Hi Jackie,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pinchartl-media/drm/du/next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.14-rc3 next-20210727]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jackie-Liu/drm-rcar-du-crtc-force-depends-on-cmm/20210728-222353
base:   git://linuxtv.org/pinchartl/media.git drm/du/next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/d8277431962a65912c00e968b5df7bf103cda67a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jackie-Liu/drm-rcar-du-crtc-force-depends-on-cmm/20210728-222353
        git checkout d8277431962a65912c00e968b5df7bf103cda67a
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash drivers/gpu/drm/rcar-du/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/rcar-du/rcar_cmm.c:81:5: error: redefinition of 'rcar_cmm_setup'
      81 | int rcar_cmm_setup(struct platform_device *pdev,
         |     ^~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/rcar-du/rcar_cmm.c:16:
   drivers/gpu/drm/rcar-du/rcar_cmm.h:51:19: note: previous definition of 'rcar_cmm_setup' was here
      51 | static inline int rcar_cmm_setup(struct platform_device *pdev,
         |                   ^~~~~~~~~~~~~~
>> drivers/gpu/drm/rcar-du/rcar_cmm.c:121:5: error: redefinition of 'rcar_cmm_enable'
     121 | int rcar_cmm_enable(struct platform_device *pdev)
         |     ^~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/rcar-du/rcar_cmm.c:16:
   drivers/gpu/drm/rcar-du/rcar_cmm.h:42:19: note: previous definition of 'rcar_cmm_enable' was here
      42 | static inline int rcar_cmm_enable(struct platform_device *pdev)
         |                   ^~~~~~~~~~~~~~~
>> drivers/gpu/drm/rcar-du/rcar_cmm.c:143:6: error: redefinition of 'rcar_cmm_disable'
     143 | void rcar_cmm_disable(struct platform_device *pdev)
         |      ^~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/rcar-du/rcar_cmm.c:16:
   drivers/gpu/drm/rcar-du/rcar_cmm.h:47:20: note: previous definition of 'rcar_cmm_disable' was here
      47 | static inline void rcar_cmm_disable(struct platform_device *pdev)
         |                    ^~~~~~~~~~~~~~~~
>> drivers/gpu/drm/rcar-du/rcar_cmm.c:161:5: error: redefinition of 'rcar_cmm_init'
     161 | int rcar_cmm_init(struct platform_device *pdev)
         |     ^~~~~~~~~~~~~
   In file included from drivers/gpu/drm/rcar-du/rcar_cmm.c:16:
   drivers/gpu/drm/rcar-du/rcar_cmm.h:37:19: note: previous definition of 'rcar_cmm_init' was here
      37 | static inline int rcar_cmm_init(struct platform_device *pdev)
         |                   ^~~~~~~~~~~~~


vim +/rcar_cmm_setup +81 drivers/gpu/drm/rcar-du/rcar_cmm.c

e08e934d6c289ed Jacopo Mondi  2019-10-17   64  
e08e934d6c289ed Jacopo Mondi  2019-10-17   65  /*
e08e934d6c289ed Jacopo Mondi  2019-10-17   66   * rcar_cmm_setup() - Configure the CMM unit
e08e934d6c289ed Jacopo Mondi  2019-10-17   67   * @pdev: The platform device associated with the CMM instance
e08e934d6c289ed Jacopo Mondi  2019-10-17   68   * @config: The CMM unit configuration
e08e934d6c289ed Jacopo Mondi  2019-10-17   69   *
e08e934d6c289ed Jacopo Mondi  2019-10-17   70   * Configure the CMM unit with the given configuration. Currently enabling,
e08e934d6c289ed Jacopo Mondi  2019-10-17   71   * disabling and programming of the 1-D LUT unit is supported.
e08e934d6c289ed Jacopo Mondi  2019-10-17   72   *
e08e934d6c289ed Jacopo Mondi  2019-10-17   73   * As rcar_cmm_setup() accesses the CMM registers the unit should be powered
e08e934d6c289ed Jacopo Mondi  2019-10-17   74   * and its functional clock enabled. To guarantee this, before any call to
e08e934d6c289ed Jacopo Mondi  2019-10-17   75   * this function is made, the CMM unit has to be enabled by calling
e08e934d6c289ed Jacopo Mondi  2019-10-17   76   * rcar_cmm_enable() first.
e08e934d6c289ed Jacopo Mondi  2019-10-17   77   *
e08e934d6c289ed Jacopo Mondi  2019-10-17   78   * TODO: Add support for LUT double buffer operations to avoid updating the
e08e934d6c289ed Jacopo Mondi  2019-10-17   79   * LUT table entries while a frame is being displayed.
e08e934d6c289ed Jacopo Mondi  2019-10-17   80   */
e08e934d6c289ed Jacopo Mondi  2019-10-17  @81  int rcar_cmm_setup(struct platform_device *pdev,
e08e934d6c289ed Jacopo Mondi  2019-10-17   82  		   const struct rcar_cmm_config *config)
e08e934d6c289ed Jacopo Mondi  2019-10-17   83  {
e08e934d6c289ed Jacopo Mondi  2019-10-17   84  	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
e08e934d6c289ed Jacopo Mondi  2019-10-17   85  
e08e934d6c289ed Jacopo Mondi  2019-10-17   86  	/* Disable LUT if no table is provided. */
e08e934d6c289ed Jacopo Mondi  2019-10-17   87  	if (!config->lut.table) {
e08e934d6c289ed Jacopo Mondi  2019-10-17   88  		if (rcmm->lut.enabled) {
e08e934d6c289ed Jacopo Mondi  2019-10-17   89  			rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
e08e934d6c289ed Jacopo Mondi  2019-10-17   90  			rcmm->lut.enabled = false;
e08e934d6c289ed Jacopo Mondi  2019-10-17   91  		}
e08e934d6c289ed Jacopo Mondi  2019-10-17   92  
e08e934d6c289ed Jacopo Mondi  2019-10-17   93  		return 0;
e08e934d6c289ed Jacopo Mondi  2019-10-17   94  	}
e08e934d6c289ed Jacopo Mondi  2019-10-17   95  
e08e934d6c289ed Jacopo Mondi  2019-10-17   96  	/* Enable LUT and program the new gamma table values. */
e08e934d6c289ed Jacopo Mondi  2019-10-17   97  	if (!rcmm->lut.enabled) {
e08e934d6c289ed Jacopo Mondi  2019-10-17   98  		rcar_cmm_write(rcmm, CM2_LUT_CTRL, CM2_LUT_CTRL_LUT_EN);
e08e934d6c289ed Jacopo Mondi  2019-10-17   99  		rcmm->lut.enabled = true;
e08e934d6c289ed Jacopo Mondi  2019-10-17  100  	}
e08e934d6c289ed Jacopo Mondi  2019-10-17  101  
e08e934d6c289ed Jacopo Mondi  2019-10-17  102  	rcar_cmm_lut_write(rcmm, config->lut.table);
e08e934d6c289ed Jacopo Mondi  2019-10-17  103  
e08e934d6c289ed Jacopo Mondi  2019-10-17  104  	return 0;
e08e934d6c289ed Jacopo Mondi  2019-10-17  105  }
e08e934d6c289ed Jacopo Mondi  2019-10-17  106  EXPORT_SYMBOL_GPL(rcar_cmm_setup);
e08e934d6c289ed Jacopo Mondi  2019-10-17  107  
e08e934d6c289ed Jacopo Mondi  2019-10-17  108  /*
e08e934d6c289ed Jacopo Mondi  2019-10-17  109   * rcar_cmm_enable() - Enable the CMM unit
e08e934d6c289ed Jacopo Mondi  2019-10-17  110   * @pdev: The platform device associated with the CMM instance
e08e934d6c289ed Jacopo Mondi  2019-10-17  111   *
e08e934d6c289ed Jacopo Mondi  2019-10-17  112   * When the output of the corresponding DU channel is routed to the CMM unit,
e08e934d6c289ed Jacopo Mondi  2019-10-17  113   * the unit shall be enabled before the DU channel is started, and remain
e08e934d6c289ed Jacopo Mondi  2019-10-17  114   * enabled until the channel is stopped. The CMM unit shall be disabled with
e08e934d6c289ed Jacopo Mondi  2019-10-17  115   * rcar_cmm_disable().
e08e934d6c289ed Jacopo Mondi  2019-10-17  116   *
e08e934d6c289ed Jacopo Mondi  2019-10-17  117   * Calls to rcar_cmm_enable() and rcar_cmm_disable() are not reference-counted.
e08e934d6c289ed Jacopo Mondi  2019-10-17  118   * It is an error to attempt to enable an already enabled CMM unit, or to
e08e934d6c289ed Jacopo Mondi  2019-10-17  119   * attempt to disable a disabled unit.
e08e934d6c289ed Jacopo Mondi  2019-10-17  120   */
e08e934d6c289ed Jacopo Mondi  2019-10-17 @121  int rcar_cmm_enable(struct platform_device *pdev)
e08e934d6c289ed Jacopo Mondi  2019-10-17  122  {
e08e934d6c289ed Jacopo Mondi  2019-10-17  123  	int ret;
e08e934d6c289ed Jacopo Mondi  2019-10-17  124  
136ce7684bc1ff4 Qinglang Miao 2020-11-27  125  	ret = pm_runtime_resume_and_get(&pdev->dev);
e08e934d6c289ed Jacopo Mondi  2019-10-17  126  	if (ret < 0)
e08e934d6c289ed Jacopo Mondi  2019-10-17  127  		return ret;
e08e934d6c289ed Jacopo Mondi  2019-10-17  128  
e08e934d6c289ed Jacopo Mondi  2019-10-17  129  	return 0;
e08e934d6c289ed Jacopo Mondi  2019-10-17  130  }
e08e934d6c289ed Jacopo Mondi  2019-10-17  131  EXPORT_SYMBOL_GPL(rcar_cmm_enable);
e08e934d6c289ed Jacopo Mondi  2019-10-17  132  
e08e934d6c289ed Jacopo Mondi  2019-10-17  133  /*
e08e934d6c289ed Jacopo Mondi  2019-10-17  134   * rcar_cmm_disable() - Disable the CMM unit
e08e934d6c289ed Jacopo Mondi  2019-10-17  135   * @pdev: The platform device associated with the CMM instance
e08e934d6c289ed Jacopo Mondi  2019-10-17  136   *
e08e934d6c289ed Jacopo Mondi  2019-10-17  137   * See rcar_cmm_enable() for usage information.
e08e934d6c289ed Jacopo Mondi  2019-10-17  138   *
e08e934d6c289ed Jacopo Mondi  2019-10-17  139   * Disabling the CMM unit disable all the internal processing blocks. The CMM
e08e934d6c289ed Jacopo Mondi  2019-10-17  140   * state shall thus be restored with rcar_cmm_setup() when re-enabling the CMM
e08e934d6c289ed Jacopo Mondi  2019-10-17  141   * unit after the next rcar_cmm_enable() call.
e08e934d6c289ed Jacopo Mondi  2019-10-17  142   */
e08e934d6c289ed Jacopo Mondi  2019-10-17 @143  void rcar_cmm_disable(struct platform_device *pdev)
e08e934d6c289ed Jacopo Mondi  2019-10-17  144  {
e08e934d6c289ed Jacopo Mondi  2019-10-17  145  	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
e08e934d6c289ed Jacopo Mondi  2019-10-17  146  
e08e934d6c289ed Jacopo Mondi  2019-10-17  147  	rcar_cmm_write(rcmm, CM2_LUT_CTRL, 0);
e08e934d6c289ed Jacopo Mondi  2019-10-17  148  	rcmm->lut.enabled = false;
e08e934d6c289ed Jacopo Mondi  2019-10-17  149  
e08e934d6c289ed Jacopo Mondi  2019-10-17  150  	pm_runtime_put(&pdev->dev);
e08e934d6c289ed Jacopo Mondi  2019-10-17  151  }
e08e934d6c289ed Jacopo Mondi  2019-10-17  152  EXPORT_SYMBOL_GPL(rcar_cmm_disable);
e08e934d6c289ed Jacopo Mondi  2019-10-17  153  
e08e934d6c289ed Jacopo Mondi  2019-10-17  154  /*
e08e934d6c289ed Jacopo Mondi  2019-10-17  155   * rcar_cmm_init() - Initialize the CMM unit
e08e934d6c289ed Jacopo Mondi  2019-10-17  156   * @pdev: The platform device associated with the CMM instance
e08e934d6c289ed Jacopo Mondi  2019-10-17  157   *
e08e934d6c289ed Jacopo Mondi  2019-10-17  158   * Return: 0 on success, -EPROBE_DEFER if the CMM is not available yet,
e08e934d6c289ed Jacopo Mondi  2019-10-17  159   *         -ENODEV if the DRM_RCAR_CMM config option is disabled
e08e934d6c289ed Jacopo Mondi  2019-10-17  160   */
e08e934d6c289ed Jacopo Mondi  2019-10-17 @161  int rcar_cmm_init(struct platform_device *pdev)
e08e934d6c289ed Jacopo Mondi  2019-10-17  162  {
e08e934d6c289ed Jacopo Mondi  2019-10-17  163  	struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
e08e934d6c289ed Jacopo Mondi  2019-10-17  164  
e08e934d6c289ed Jacopo Mondi  2019-10-17  165  	if (!rcmm)
e08e934d6c289ed Jacopo Mondi  2019-10-17  166  		return -EPROBE_DEFER;
e08e934d6c289ed Jacopo Mondi  2019-10-17  167  
e08e934d6c289ed Jacopo Mondi  2019-10-17  168  	return 0;
e08e934d6c289ed Jacopo Mondi  2019-10-17  169  }
e08e934d6c289ed Jacopo Mondi  2019-10-17  170  EXPORT_SYMBOL_GPL(rcar_cmm_init);
e08e934d6c289ed Jacopo Mondi  2019-10-17  171  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 54032 bytes --]