Disabling CONFIG_PM produces a compile time warning when these functions are not referenced:
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c:1072:12: error: 'sun6i_dsi_runtime_suspend' defined but not used [-Werror=unused-function] static int sun6i_dsi_runtime_suspend(struct device *dev) ^~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c:1043:12: error: 'sun6i_dsi_runtime_resume' defined but not used [-Werror=unused-function] static int sun6i_dsi_runtime_resume(struct device *dev) ^~~~~~~~~~~~~~~~~~~~~~~~
Fixes: 133add5b5ad4 ("drm/sun4i: Add Allwinner A31 MIPI-DSI controller support") Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index bfbf761f0c1d..d4e7d16a2514 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -1040,7 +1040,7 @@ static int sun6i_dsi_remove(struct platform_device *pdev) return 0; }
-static int sun6i_dsi_runtime_resume(struct device *dev) +static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev) { struct sun6i_dsi *dsi = dev_get_drvdata(dev);
@@ -1069,7 +1069,7 @@ static int sun6i_dsi_runtime_resume(struct device *dev) return 0; }
-static int sun6i_dsi_runtime_suspend(struct device *dev) +static int __maybe_unused sun6i_dsi_runtime_suspend(struct device *dev) { struct sun6i_dsi *dsi = dev_get_drvdata(dev);
Casting a pointer to a 64-bit type causes a warning on 32-bit targets:
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c:473:24: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] lower_32_bits((uint64_t)wptr)); ^ drivers/gpu/drm/amd/amdgpu/amdgpu.h:1701:53: note: in definition of macro 'WREG32' #define WREG32(reg, v) amdgpu_mm_wreg(adev, (reg), (v), 0) ^ drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c:473:10: note: in expansion of macro 'lower_32_bits' lower_32_bits((uint64_t)wptr)); ^~~~~~~~~~~~~
The correct method is to cast to 'uintptr_t'.
Fixes: d5a114a6c5f7 ("drm/amdgpu: Add GFXv9 kfd2kgd interface functions") Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c index 8f37991df61b..f0c0d3953f69 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c @@ -470,9 +470,9 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id, WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_HI), upper_32_bits(guessed_wptr)); WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_POLL_ADDR), - lower_32_bits((uint64_t)wptr)); + lower_32_bits((uintptr_t)wptr)); WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_POLL_ADDR_HI), - upper_32_bits((uint64_t)wptr)); + upper_32_bits((uintptr_t)wptr)); WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_PQ_WPTR_POLL_CNTL1), get_queue_mask(adev, pipe_id, queue_id)); }
On Fri, May 25, 2018 at 6:50 PM, Arnd Bergmann arnd@arndb.de wrote:
Casting a pointer to a 64-bit type causes a warning on 32-bit targets:
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c:473:24: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] lower_32_bits((uint64_t)wptr)); ^ drivers/gpu/drm/amd/amdgpu/amdgpu.h:1701:53: note: in definition of macro 'WREG32' #define WREG32(reg, v) amdgpu_mm_wreg(adev, (reg), (v), 0) ^ drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c:473:10: note: in expansion of macro 'lower_32_bits' lower_32_bits((uint64_t)wptr)); ^~~~~~~~~~~~~
The correct method is to cast to 'uintptr_t'.
Fixes: d5a114a6c5f7 ("drm/amdgpu: Add GFXv9 kfd2kgd interface functions") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c index 8f37991df61b..f0c0d3953f69 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c @@ -470,9 +470,9 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id, WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_HI), upper_32_bits(guessed_wptr)); WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_POLL_ADDR),
lower_32_bits((uint64_t)wptr));
lower_32_bits((uintptr_t)wptr)); WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_POLL_ADDR_HI),
upper_32_bits((uint64_t)wptr));
upper_32_bits((uintptr_t)wptr)); WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_PQ_WPTR_POLL_CNTL1), get_queue_mask(adev, pipe_id, queue_id)); }-- 2.9.0
There is a change scheduled for the next merge window that will cause this file to not build anymore on 32-bit targets (because the amdkfd driver is not supported on 32 bit targets).
Never the less I'm taking the patch for good measure.
This patch is: Reviewed-by: Oded Gabbay oded.gabbay@gmail.com
In 32-bit kernel builds, we cannot cast between a pointer and a 64-bit type:
In file included from drivers/gpu/drm/xen/xen_drm_front_cfg.c:18: drivers/gpu/drm/xen/xen_drm_front.h: In function 'xen_drm_front_fb_to_cookie': drivers/gpu/drm/xen/xen_drm_front.h:129:9: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] return (u64)fb;
drivers/gpu/drm/xen/xen_drm_front.h: In function 'xen_drm_front_dbuf_to_cookie': drivers/gpu/drm/xen/xen_drm_front.h:134:9: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] return (u64)gem_obj;
drivers/gpu/drm/xen/xen_drm_front_shbuf.c: In function 'backend_unmap': drivers/gpu/drm/xen/xen_drm_front_shbuf.c:125:4: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] ((phys_addr_t)pfn_to_kaddr(page_to_xen_pfn(page)))
Using uintptr_t instead probably does what we want here, although it's not clear to me why we assign a virtual address pointer to a phys_addr_t in backend_unmap().
Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display frontend") Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/gpu/drm/xen/xen_drm_front.h | 4 ++-- drivers/gpu/drm/xen/xen_drm_front_shbuf.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front.h b/drivers/gpu/drm/xen/xen_drm_front.h index 2c2479b571ae..5693b4a4b02b 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.h +++ b/drivers/gpu/drm/xen/xen_drm_front.h @@ -126,12 +126,12 @@ struct xen_drm_front_drm_info {
static inline u64 xen_drm_front_fb_to_cookie(struct drm_framebuffer *fb) { - return (u64)fb; + return (uintptr_t)fb; }
static inline u64 xen_drm_front_dbuf_to_cookie(struct drm_gem_object *gem_obj) { - return (u64)gem_obj; + return (uintptr_t)gem_obj; }
int xen_drm_front_mode_set(struct xen_drm_front_drm_pipeline *pipeline, diff --git a/drivers/gpu/drm/xen/xen_drm_front_shbuf.c b/drivers/gpu/drm/xen/xen_drm_front_shbuf.c index 8099cb343ae3..d333b67cc1a0 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_shbuf.c +++ b/drivers/gpu/drm/xen/xen_drm_front_shbuf.c @@ -122,7 +122,7 @@ static void guest_calc_num_grefs(struct xen_drm_front_shbuf *buf) }
#define xen_page_to_vaddr(page) \ - ((phys_addr_t)pfn_to_kaddr(page_to_xen_pfn(page))) + ((uintptr_t)pfn_to_kaddr(page_to_xen_pfn(page)))
static int backend_unmap(struct xen_drm_front_shbuf *buf) {
Hi,
On 05/25/2018 06:50 PM, Arnd Bergmann wrote:
In 32-bit kernel builds, we cannot cast between a pointer and a 64-bit type:
In file included from drivers/gpu/drm/xen/xen_drm_front_cfg.c:18: drivers/gpu/drm/xen/xen_drm_front.h: In function 'xen_drm_front_fb_to_cookie': drivers/gpu/drm/xen/xen_drm_front.h:129:9: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] return (u64)fb;
drivers/gpu/drm/xen/xen_drm_front.h: In function 'xen_drm_front_dbuf_to_cookie': drivers/gpu/drm/xen/xen_drm_front.h:134:9: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] return (u64)gem_obj;
drivers/gpu/drm/xen/xen_drm_front_shbuf.c: In function 'backend_unmap': drivers/gpu/drm/xen/xen_drm_front_shbuf.c:125:4: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] ((phys_addr_t)pfn_to_kaddr(page_to_xen_pfn(page)))
Using uintptr_t instead probably does what we want here, although it's not clear to me why we assign a virtual address pointer to a phys_addr_t in backend_unmap().
Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display frontend") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/gpu/drm/xen/xen_drm_front.h | 4 ++-- drivers/gpu/drm/xen/xen_drm_front_shbuf.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front.h b/drivers/gpu/drm/xen/xen_drm_front.h index 2c2479b571ae..5693b4a4b02b 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.h +++ b/drivers/gpu/drm/xen/xen_drm_front.h @@ -126,12 +126,12 @@ struct xen_drm_front_drm_info {
static inline u64 xen_drm_front_fb_to_cookie(struct drm_framebuffer *fb) {
- return (u64)fb;
return (uintptr_t)fb; }
static inline u64 xen_drm_front_dbuf_to_cookie(struct drm_gem_object *gem_obj) {
- return (u64)gem_obj;
return (uintptr_t)gem_obj; }
int xen_drm_front_mode_set(struct xen_drm_front_drm_pipeline *pipeline,
diff --git a/drivers/gpu/drm/xen/xen_drm_front_shbuf.c b/drivers/gpu/drm/xen/xen_drm_front_shbuf.c index 8099cb343ae3..d333b67cc1a0 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_shbuf.c +++ b/drivers/gpu/drm/xen/xen_drm_front_shbuf.c @@ -122,7 +122,7 @@ static void guest_calc_num_grefs(struct xen_drm_front_shbuf *buf) }
#define xen_page_to_vaddr(page) \
((phys_addr_t)pfn_to_kaddr(page_to_xen_pfn(page)))
((uintptr_t)pfn_to_kaddr(page_to_xen_pfn(page)))static int backend_unmap(struct xen_drm_front_shbuf *buf) {
Thank you for your patch: this issue was already discussed [1] and applied [2] to drm-misc-next.
Thank you, Oleksandr
[1] https://patchwork.freedesktop.org/patch/224359/ [2] https://patchwork.freedesktop.org/patch/224920/
On Tue, May 29, 2018 at 7:58 AM, Oleksandr Andrushchenko andr2000@gmail.com wrote:
On 05/25/2018 06:50 PM, Arnd Bergmann wrote:
b/drivers/gpu/drm/xen/xen_drm_front_shbuf.c index 8099cb343ae3..d333b67cc1a0 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_shbuf.c +++ b/drivers/gpu/drm/xen/xen_drm_front_shbuf.c @@ -122,7 +122,7 @@ static void guest_calc_num_grefs(struct xen_drm_front_shbuf *buf) } #define xen_page_to_vaddr(page) \
((phys_addr_t)pfn_to_kaddr(page_to_xen_pfn(page)))
((uintptr_t)pfn_to_kaddr(page_to_xen_pfn(page)))Thank you for your patch: this issue was already discussed [1] and applied [2] to drm-misc-next.
Ok, thanks, and sorry for the duplicate. Linux-next releases have been a bit sporadic recently so I didn't run the latest tree for all cases.
Arnd
The "alpha" struct member was removed in one commit but another user added in another, leading to a build failure:
drivers/gpu/drm/rcar-du/rcar_du_vsp.c: In function 'rcar_du_vsp_plane_atomic_duplicate_state': drivers/gpu/drm/rcar-du/rcar_du_vsp.c:325:6: error: 'struct rcar_du_vsp_plane_state' has no member named 'alpha' copy->alpha = to_rcar_vsp_plane_state(plane->state)->alpha;
This fixes up the merge by using the correct member.
Fixes: 75a07f399cd4 ("drm: rcar-du: Zero-out sg_tables when duplicating plane state") Fixes: 301a9b8d5456 ("drm/rcar-du: Convert to the new generic alpha property") Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 4a98470626d5..d9232e749b6d 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -322,7 +322,7 @@ rcar_du_vsp_plane_atomic_duplicate_state(struct drm_plane *plane) return NULL;
__drm_atomic_helper_plane_duplicate_state(plane, ©->state); - copy->alpha = to_rcar_vsp_plane_state(plane->state)->alpha; + copy->state.alpha = plane->state->alpha;
return ©->state; }
Hi Arnd,
Thank you for the patch.
On Friday, 25 May 2018 18:50:11 EEST Arnd Bergmann wrote:
The "alpha" struct member was removed in one commit but another user added in another, leading to a build failure:
drivers/gpu/drm/rcar-du/rcar_du_vsp.c: In function 'rcar_du_vsp_plane_atomic_duplicate_state': drivers/gpu/drm/rcar-du/rcar_du_vsp.c:325:6: error: 'struct rcar_du_vsp_plane_state' has no member named 'alpha' copy->alpha = to_rcar_vsp_plane_state(plane->state)->alpha;
This fixes up the merge by using the correct member.
Fixes: 75a07f399cd4 ("drm: rcar-du: Zero-out sg_tables when duplicating plane state") Fixes: 301a9b8d5456 ("drm/rcar-du: Convert to the new generic alpha property") Signed-off-by: Arnd Bergmann arnd@arndb.de
I have posted the same fix a week ago, see https://patchwork.freedesktop.org/ patch/msgid/20180515174752.28954-1-laurent.pinchart+renesas@ideasonboard.com. The patch is in Dave's master branch in commit 315852b422972e6ebb1dfddaadada09e46a2681a.
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 4a98470626d5..d9232e749b6d 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -322,7 +322,7 @@ rcar_du_vsp_plane_atomic_duplicate_state(struct drm_plane *plane) return NULL;
__drm_atomic_helper_plane_duplicate_state(plane, ©->state);
- copy->alpha = to_rcar_vsp_plane_state(plane->state)->alpha;
copy->state.alpha = plane->state->alpha;
return ©->state;
}
Without CONFIG_MMU, we get a link error:
drivers/gpu/drm/v3d/v3d_bo.o: In function `v3d_gem_fault': v3d_bo.c:(.text+0x3ca): undefined reference to `vm_insert_mixed'
The other drivers with this problem already depend on CONFIG_MMU, so let's do the same thing here.
Fixes: 57692c94dcbe ("drm/v3d: Introduce a new DRM driver for Broadcom V3D V3.x+") Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/gpu/drm/v3d/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/v3d/Kconfig b/drivers/gpu/drm/v3d/Kconfig index a0c0259355bd..1552bf552c94 100644 --- a/drivers/gpu/drm/v3d/Kconfig +++ b/drivers/gpu/drm/v3d/Kconfig @@ -3,6 +3,7 @@ config DRM_V3D depends on ARCH_BCM || ARCH_BCMSTB || COMPILE_TEST depends on DRM depends on COMMON_CLK + depends on MMU select DRM_SCHED help Choose this option if you have a system that has a Broadcom
Arnd Bergmann arnd@arndb.de writes:
Without CONFIG_MMU, we get a link error:
drivers/gpu/drm/v3d/v3d_bo.o: In function `v3d_gem_fault': v3d_bo.c:(.text+0x3ca): undefined reference to `vm_insert_mixed'
The other drivers with this problem already depend on CONFIG_MMU, so let's do the same thing here.
Fixes: 57692c94dcbe ("drm/v3d: Introduce a new DRM driver for Broadcom V3D V3.x+") Signed-off-by: Arnd Bergmann arnd@arndb.de
Applied to drm-misc-next-fixes. Thanks!
Modern gcc versions warn about returning a ternary operator with an 'int' type in a function returning type 'bool':
drivers/gpu/drm/exynos/exynos_drm_scaler.c: In function 'scaler_task_done': drivers/gpu/drm/exynos/exynos_drm_scaler.c:402:47: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context] return val & SCALER_INT_STATUS_FRAME_END ? 0 : -EINVAL;
From context, it becomes clear that this should have been 'int',
so I'm fixing it, along with parenthesizing the expression to make it clearer what is meant here (I got confused at first, after seeing the warning).
Fixes: 01fb9185dc18 ("drm/exynos: Add driver for Exynos Scaler module") Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/gpu/drm/exynos/exynos_drm_scaler.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_scaler.c b/drivers/gpu/drm/exynos/exynos_drm_scaler.c index 63b05b7c846a..4ad49d7782cd 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_scaler.c +++ b/drivers/gpu/drm/exynos/exynos_drm_scaler.c @@ -397,9 +397,9 @@ static inline u32 scaler_get_int_status(struct scaler_context *scaler) return scaler_read(SCALER_INT_STATUS); }
-static inline bool scaler_task_done(u32 val) +static inline int scaler_task_done(u32 val) { - return val & SCALER_INT_STATUS_FRAME_END ? 0 : -EINVAL; + return (val & SCALER_INT_STATUS_FRAME_END) ? 0 : -EINVAL; }
static irqreturn_t scaler_irq_handler(int irq, void *arg)
The DRM panel bridge code is built into the kms helpers module, so we get a link error when trying to use it from a built-in driver while the kms helper is a loadable module:
drivers/gpu/drm/bridge/lvds-encoder.o: In function `lvds_encoder_probe': lvds-encoder.c:(.text+0x124): undefined reference to `devm_drm_panel_bridge_add'
This adds a the same dependency in the lvds-encoder that we use for all the other users of the panel bridge. I did not bisect the problem, but from inspection it seems to date back to the patch that separated out the panel bridge from lvds encoder.
Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bridge.") Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/gpu/drm/bridge/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 6caa47834194..cf47bfa7a050 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -46,6 +46,7 @@ config DRM_DUMB_VGA_DAC config DRM_LVDS_ENCODER tristate "Transparent parallel to LVDS encoder support" depends on OF + select DRM_KMS_HELPER select DRM_PANEL_BRIDGE help Support for transparent parallel to LVDS encoders that don't require
On Fri, May 25, 2018 at 5:50 PM, Arnd Bergmann arnd@arndb.de wrote:
The DRM panel bridge code is built into the kms helpers module, so we get a link error when trying to use it from a built-in driver while the kms helper is a loadable module:
drivers/gpu/drm/bridge/lvds-encoder.o: In function `lvds_encoder_probe': lvds-encoder.c:(.text+0x124): undefined reference to `devm_drm_panel_bridge_add'
This adds a the same dependency in the lvds-encoder that we use for all the other users of the panel bridge. I did not bisect the problem, but from inspection it seems to date back to the patch that separated out the panel bridge from lvds encoder.
Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bridge.") Signed-off-by: Arnd Bergmann arnd@arndb.de
Reviewed-by: Linus Walleij linus.walleij@linaro.org
Yours, Linus Walleij
Hi Arnd,
Thank you for the patch.
On Friday, 25 May 2018 18:50:14 EEST Arnd Bergmann wrote:
The DRM panel bridge code is built into the kms helpers module, so we get a link error when trying to use it from a built-in driver while the kms helper is a loadable module:
drivers/gpu/drm/bridge/lvds-encoder.o: In function `lvds_encoder_probe': lvds-encoder.c:(.text+0x124): undefined reference to `devm_drm_panel_bridge_add'
This adds a the same dependency in the lvds-encoder that we use for all the other users of the panel bridge. I did not bisect the problem, but from inspection it seems to date back to the patch that separated out the panel bridge from lvds encoder.
Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bridge.") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/gpu/drm/bridge/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 6caa47834194..cf47bfa7a050 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -46,6 +46,7 @@ config DRM_DUMB_VGA_DAC config DRM_LVDS_ENCODER tristate "Transparent parallel to LVDS encoder support" depends on OF
- select DRM_KMS_HELPER select DRM_PANEL_BRIDGE help Support for transparent parallel to LVDS encoders that don't require
Wouldn't it be better to apply the following ?
config DRM_PANEL_BRIDGE def_bool y depends on DRM_BRIDGE - depends on DRM_KMS_HELPER + select DRM_KMS_HELPER select DRM_PANEL help DRM bridge wrapper of DRM panels
Otherwise you'll potentially have to patch every user of DRM_PANEL_BRIDGE as done in this patch.
On Mon, May 28, 2018 at 10:02 AM, Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
Hi Arnd,
Thank you for the patch.
On Friday, 25 May 2018 18:50:14 EEST Arnd Bergmann wrote:
The DRM panel bridge code is built into the kms helpers module, so we get a link error when trying to use it from a built-in driver while the kms helper is a loadable module:
drivers/gpu/drm/bridge/lvds-encoder.o: In function `lvds_encoder_probe': lvds-encoder.c:(.text+0x124): undefined reference to `devm_drm_panel_bridge_add'
This adds a the same dependency in the lvds-encoder that we use for all the other users of the panel bridge. I did not bisect the problem, but from inspection it seems to date back to the patch that separated out the panel bridge from lvds encoder.
Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bridge.") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/gpu/drm/bridge/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 6caa47834194..cf47bfa7a050 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -46,6 +46,7 @@ config DRM_DUMB_VGA_DAC config DRM_LVDS_ENCODER tristate "Transparent parallel to LVDS encoder support" depends on OF
select DRM_KMS_HELPER select DRM_PANEL_BRIDGE help Support for transparent parallel to LVDS encoders that don't requireWouldn't it be better to apply the following ?
config DRM_PANEL_BRIDGE def_bool y depends on DRM_BRIDGE
depends on DRM_KMS_HELPER
select DRM_KMS_HELPER select DRM_PANEL help DRM bridge wrapper of DRM panelsOtherwise you'll potentially have to patch every user of DRM_PANEL_BRIDGE as done in this patch.
Select isn't recursive, so this won't work unfortunately :-/ -Daniel
-- Regards,
Laurent Pinchart
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, May 28, 2018 at 10:06 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Mon, May 28, 2018 at 10:02 AM, Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
Hi Arnd,
Thank you for the patch.
On Friday, 25 May 2018 18:50:14 EEST Arnd Bergmann wrote:
The DRM panel bridge code is built into the kms helpers module, so we get a link error when trying to use it from a built-in driver while the kms helper is a loadable module:
drivers/gpu/drm/bridge/lvds-encoder.o: In function `lvds_encoder_probe': lvds-encoder.c:(.text+0x124): undefined reference to `devm_drm_panel_bridge_add'
This adds a the same dependency in the lvds-encoder that we use for all the other users of the panel bridge. I did not bisect the problem, but from inspection it seems to date back to the patch that separated out the panel bridge from lvds encoder.
Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bridge.") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/gpu/drm/bridge/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 6caa47834194..cf47bfa7a050 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -46,6 +46,7 @@ config DRM_DUMB_VGA_DAC config DRM_LVDS_ENCODER tristate "Transparent parallel to LVDS encoder support" depends on OF
select DRM_KMS_HELPER select DRM_PANEL_BRIDGE help Support for transparent parallel to LVDS encoders that don't requireWouldn't it be better to apply the following ?
config DRM_PANEL_BRIDGE def_bool y depends on DRM_BRIDGE
depends on DRM_KMS_HELPER
select DRM_KMS_HELPER select DRM_PANEL help DRM bridge wrapper of DRM panelsOtherwise you'll potentially have to patch every user of DRM_PANEL_BRIDGE as done in this patch.
Select isn't recursive, so this won't work unfortunately :-/
The problem is a bit different: select *is* recursive, which is part of the reason we normally try to avoid it (it gets hard to disable certain symbols or turn them into modules when there are lots of things selecting them).
However, DRM_PANEL_BRIDGE is a silent 'bool' symbol that is always enabled when DRM_BRIDGE is enabled. Making it 'select DRM_KMS_HELPER' would lead to DRM_KMS_HELPER always being built-in even if all other DRM drivers are configured as loadable modules! Note these Makefile line in drivers/gpu/drm:
drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
The intention is definitely that drm_kms_helper can be a loadable module, (it cannot be built-in when CONFIG_DRM=m) and the panel bridge is simply a component that gets linked into it, as of commit 123387d5efa6 ("drm/bridge: Build the panel wrapper in drm_kms_helper").
Arnd
These two functions are unused in some configurations, and using __maybe_unused is the easiest way to shut up the harmless warnings:
drivers/gpu/drm/bridge/cdns-dsi.c:1353:12: error: 'cdns_dsi_suspend' defined but not used [-Werror=unused-function] static int cdns_dsi_suspend(struct device *dev) ^~~~~~~~~~~~~~~~ drivers/gpu/drm/bridge/cdns-dsi.c:1340:12: error: 'cdns_dsi_resume' defined but not used [-Werror=unused-function] static int cdns_dsi_resume(struct device *dev)
Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver") Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/gpu/drm/bridge/cdns-dsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c index c255fc3e1be5..f2d43f24acfb 100644 --- a/drivers/gpu/drm/bridge/cdns-dsi.c +++ b/drivers/gpu/drm/bridge/cdns-dsi.c @@ -1337,7 +1337,7 @@ static const struct mipi_dsi_host_ops cdns_dsi_ops = { .transfer = cdns_dsi_transfer, };
-static int cdns_dsi_resume(struct device *dev) +static int __maybe_unused cdns_dsi_resume(struct device *dev) { struct cdns_dsi *dsi = dev_get_drvdata(dev);
@@ -1350,7 +1350,7 @@ static int cdns_dsi_resume(struct device *dev) return 0; }
-static int cdns_dsi_suspend(struct device *dev) +static int __maybe_unused cdns_dsi_suspend(struct device *dev) { struct cdns_dsi *dsi = dev_get_drvdata(dev);
+Thierry
Hi Arnd,
On Fri, 25 May 2018 17:50:15 +0200 Arnd Bergmann arnd@arndb.de wrote:
These two functions are unused in some configurations, and using __maybe_unused is the easiest way to shut up the harmless warnings:
drivers/gpu/drm/bridge/cdns-dsi.c:1353:12: error: 'cdns_dsi_suspend' defined but not used [-Werror=unused-function] static int cdns_dsi_suspend(struct device *dev) ^~~~~~~~~~~~~~~~ drivers/gpu/drm/bridge/cdns-dsi.c:1340:12: error: 'cdns_dsi_resume' defined but not used [-Werror=unused-function] static int cdns_dsi_resume(struct device *dev)
Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver") Signed-off-by: Arnd Bergmann arnd@arndb.de
Hm, I thought such a patch had already been applied by Thierry [1].
[1]https://www.spinics.net/lists/dri-devel/msg174363.html
drivers/gpu/drm/bridge/cdns-dsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c index c255fc3e1be5..f2d43f24acfb 100644 --- a/drivers/gpu/drm/bridge/cdns-dsi.c +++ b/drivers/gpu/drm/bridge/cdns-dsi.c @@ -1337,7 +1337,7 @@ static const struct mipi_dsi_host_ops cdns_dsi_ops = { .transfer = cdns_dsi_transfer, };
-static int cdns_dsi_resume(struct device *dev) +static int __maybe_unused cdns_dsi_resume(struct device *dev) { struct cdns_dsi *dsi = dev_get_drvdata(dev);
@@ -1350,7 +1350,7 @@ static int cdns_dsi_resume(struct device *dev) return 0; }
-static int cdns_dsi_suspend(struct device *dev) +static int __maybe_unused cdns_dsi_suspend(struct device *dev) { struct cdns_dsi *dsi = dev_get_drvdata(dev);
On Mon, May 28, 2018 at 04:32:43PM +0200, Boris Brezillon wrote:
+Thierry
Hi Arnd,
On Fri, 25 May 2018 17:50:15 +0200 Arnd Bergmann arnd@arndb.de wrote:
These two functions are unused in some configurations, and using __maybe_unused is the easiest way to shut up the harmless warnings:
drivers/gpu/drm/bridge/cdns-dsi.c:1353:12: error: 'cdns_dsi_suspend' defined but not used [-Werror=unused-function] static int cdns_dsi_suspend(struct device *dev) ^~~~~~~~~~~~~~~~ drivers/gpu/drm/bridge/cdns-dsi.c:1340:12: error: 'cdns_dsi_resume' defined but not used [-Werror=unused-function] static int cdns_dsi_resume(struct device *dev)
Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver") Signed-off-by: Arnd Bergmann arnd@arndb.de
Hm, I thought such a patch had already been applied by Thierry [1].
Yeah, that's in drm-misc-next, but didn't seem to have made it into linux-next until today.
Thierry
On Fri, May 25, 2018 at 05:50:08PM +0200, Arnd Bergmann wrote:
Disabling CONFIG_PM produces a compile time warning when these functions are not referenced:
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c:1072:12: error: 'sun6i_dsi_runtime_suspend' defined but not used [-Werror=unused-function] static int sun6i_dsi_runtime_suspend(struct device *dev) ^~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c:1043:12: error: 'sun6i_dsi_runtime_resume' defined but not used [-Werror=unused-function] static int sun6i_dsi_runtime_resume(struct device *dev) ^~~~~~~~~~~~~~~~~~~~~~~~
Fixes: 133add5b5ad4 ("drm/sun4i: Add Allwinner A31 MIPI-DSI controller support") Signed-off-by: Arnd Bergmann arnd@arndb.de
Applied, thanks! Maxime
dri-devel@lists.freedesktop.org
-
Arnd Bergmann -
Boris Brezillon -
Daniel Vetter -
Eric Anholt -
Laurent Pinchart -
Linus Walleij -
Maxime Ripard -
Oded Gabbay -
Oleksandr Andrushchenko -
Thierry Reding