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
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.orcar-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
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.orcar-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
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
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.orcar-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
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.orcar-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
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
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
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
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
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
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
Content-type: Text/plain
No virus found Checked by Hillstone Network AntiVirus
在 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.
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-d... 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/d8277431962a65912c00e968b5df7bf103cd... 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
dri-devel@lists.freedesktop.org
-
Jackie Liu -
Jackie Liu -
kernel test robot -
Kieran Bingham -
Laurent Pinchart