* [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem
@ 2024-01-10 17:39 Jani Nikula
2024-01-10 17:39 ` [PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable Jani Nikula
` (6 more replies)
0 siblings, 7 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-10 17:39 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
This is v2 of [1] to enable most W=1 warnings across the drm
subsystem. Some fixes first, and a CONFIG_DRM_WERROR on top.
I build tested this for x86 (both gcc and clang), arm and arm64.
BR,
Jani.
[1] https://lore.kernel.org/r/20231129181219.1237887-1-jani.nikula@intel.com
Jani Nikula (6):
drm/nouveau/acr/ga102: remove unused but set variable
drm/nouveau/svm: remove unused but set variables
drm/amdgpu: prefer snprintf over sprintf
drm/imx: prefer snprintf over sprintf
drm: enable (most) W=1 warnings by default across the subsystem
drm: Add CONFIG_DRM_WERROR
drivers/gpu/drm/Kconfig | 18 +++++++++++
drivers/gpu/drm/Makefile | 30 +++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 3 +-
drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_svm.c | 10 ++-----
.../gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c | 3 +-
6 files changed, 55 insertions(+), 11 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable
2024-01-10 17:39 [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Jani Nikula
@ 2024-01-10 17:39 ` Jani Nikula
2024-01-31 12:50 ` Jani Nikula
2024-01-31 19:01 ` Danilo Krummrich
2024-01-10 17:39 ` [PATCH 2/6] drm/nouveau/svm: remove unused but set variables Jani Nikula
` (5 subsequent siblings)
6 siblings, 2 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-10 17:39 UTC (permalink / raw)
To: dri-devel; +Cc: Karol Herbst, jani.nikula, nouveau, intel-gfx, Danilo Krummrich
Fix the W=1 warning -Wunused-but-set-variable.
Cc: Karol Herbst <kherbst@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Danilo Krummrich <dakr@redhat.com>
Cc: nouveau@lists.freedesktop.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
index f36a359d4531..bd104a030243 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
@@ -218,7 +218,7 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev *subdev,
const struct firmware *hsbl;
const struct nvfw_ls_hsbl_bin_hdr *hdr;
const struct nvfw_ls_hsbl_hdr *hshdr;
- u32 loc, sig, cnt, *meta;
+ u32 sig, cnt, *meta;
ret = nvkm_firmware_load_name(subdev, path, "hs_bl_sig", ver, &hsbl);
if (ret)
@@ -227,7 +227,6 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev *subdev,
hdr = nvfw_ls_hsbl_bin_hdr(subdev, hsbl->data);
hshdr = nvfw_ls_hsbl_hdr(subdev, hsbl->data + hdr->header_offset);
meta = (u32 *)(hsbl->data + hshdr->meta_data_offset);
- loc = *(u32 *)(hsbl->data + hshdr->patch_loc);
sig = *(u32 *)(hsbl->data + hshdr->patch_sig);
cnt = *(u32 *)(hsbl->data + hshdr->num_sig);
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/6] drm/nouveau/svm: remove unused but set variables
2024-01-10 17:39 [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Jani Nikula
2024-01-10 17:39 ` [PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable Jani Nikula
@ 2024-01-10 17:39 ` Jani Nikula
2024-01-31 12:51 ` Jani Nikula
2024-01-31 19:10 ` Danilo Krummrich
2024-01-10 17:39 ` [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf Jani Nikula
` (4 subsequent siblings)
6 siblings, 2 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-10 17:39 UTC (permalink / raw)
To: dri-devel; +Cc: Karol Herbst, jani.nikula, nouveau, intel-gfx, Danilo Krummrich
Fix the W=1 warning -Wunused-but-set-variable.
Cc: Karol Herbst <kherbst@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Danilo Krummrich <dakr@redhat.com>
Cc: nouveau@lists.freedesktop.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/nouveau/nouveau_svm.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
index cc03e0c22ff3..4d1008915499 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -112,7 +112,7 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
{
struct nouveau_cli *cli = nouveau_cli(file_priv);
struct drm_nouveau_svm_bind *args = data;
- unsigned target, cmd, priority;
+ unsigned target, cmd;
unsigned long addr, end;
struct mm_struct *mm;
@@ -136,9 +136,6 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
return -EINVAL;
}
- priority = args->header >> NOUVEAU_SVM_BIND_PRIORITY_SHIFT;
- priority &= NOUVEAU_SVM_BIND_PRIORITY_MASK;
-
/* FIXME support CPU target ie all target value < GPU_VRAM */
target = args->header >> NOUVEAU_SVM_BIND_TARGET_SHIFT;
target &= NOUVEAU_SVM_BIND_TARGET_MASK;
@@ -926,15 +923,14 @@ nouveau_pfns_map(struct nouveau_svmm *svmm, struct mm_struct *mm,
unsigned long addr, u64 *pfns, unsigned long npages)
{
struct nouveau_pfnmap_args *args = nouveau_pfns_to_args(pfns);
- int ret;
args->p.addr = addr;
args->p.size = npages << PAGE_SHIFT;
mutex_lock(&svmm->mutex);
- ret = nvif_object_ioctl(&svmm->vmm->vmm.object, args,
- struct_size(args, p.phys, npages), NULL);
+ nvif_object_ioctl(&svmm->vmm->vmm.object, args,
+ struct_size(args, p.phys, npages), NULL);
mutex_unlock(&svmm->mutex);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf
2024-01-10 17:39 [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Jani Nikula
2024-01-10 17:39 ` [PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable Jani Nikula
2024-01-10 17:39 ` [PATCH 2/6] drm/nouveau/svm: remove unused but set variables Jani Nikula
@ 2024-01-10 17:39 ` Jani Nikula
2024-01-12 4:09 ` kernel test robot
2024-01-12 14:57 ` Alex Deucher
2024-01-10 17:39 ` [PATCH 4/6] drm/imx: " Jani Nikula
` (3 subsequent siblings)
6 siblings, 2 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-10 17:39 UTC (permalink / raw)
To: dri-devel
Cc: jani.nikula, intel-gfx, Xinhui, amd-gfx, Alex Deucher,
Christian König, Pan
This will trade the W=1 warning -Wformat-overflow to
-Wformat-truncation. This lets us enable -Wformat-overflow subsystem
wide.
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Pan, Xinhui <Xinhui.Pan@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index b9674c57c436..82b4b2019fca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -329,7 +329,8 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
ring->eop_gpu_addr = kiq->eop_gpu_addr;
ring->no_scheduler = true;
- sprintf(ring->name, "kiq_%d.%d.%d.%d", xcc_id, ring->me, ring->pipe, ring->queue);
+ snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
+ xcc_id, ring->me, ring->pipe, ring->queue);
r = amdgpu_ring_init(adev, ring, 1024, irq, AMDGPU_CP_KIQ_IRQ_DRIVER0,
AMDGPU_RING_PRIO_DEFAULT, NULL);
if (r)
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 4/6] drm/imx: prefer snprintf over sprintf
2024-01-10 17:39 [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Jani Nikula
` (2 preceding siblings ...)
2024-01-10 17:39 ` [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf Jani Nikula
@ 2024-01-10 17:39 ` Jani Nikula
2024-01-12 10:49 ` kernel test robot
2024-01-12 14:20 ` Philipp Zabel
2024-01-10 17:39 ` [PATCH 5/6] drm: enable (most) W=1 warnings by default across the subsystem Jani Nikula
` (2 subsequent siblings)
6 siblings, 2 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-10 17:39 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
This will trade the W=1 warning -Wformat-overflow to
-Wformat-truncation. This lets us enable -Wformat-overflow subsystem
wide.
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/imx/ipuv3/imx-ldb.c b/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
index 53840ab054c7..71d70194fcbd 100644
--- a/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
+++ b/drivers/gpu/drm/imx/ipuv3/imx-ldb.c
@@ -655,7 +655,7 @@ static int imx_ldb_probe(struct platform_device *pdev)
for (i = 0; i < 4; i++) {
char clkname[16];
- sprintf(clkname, "di%d_sel", i);
+ snprintf(clkname, sizeof(clkname), "di%d_sel", i);
imx_ldb->clk_sel[i] = devm_clk_get(imx_ldb->dev, clkname);
if (IS_ERR(imx_ldb->clk_sel[i])) {
ret = PTR_ERR(imx_ldb->clk_sel[i]);
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 5/6] drm: enable (most) W=1 warnings by default across the subsystem
2024-01-10 17:39 [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Jani Nikula
` (3 preceding siblings ...)
2024-01-10 17:39 ` [PATCH 4/6] drm/imx: " Jani Nikula
@ 2024-01-10 17:39 ` Jani Nikula
2024-01-10 20:15 ` Hamza Mahfooz
2024-01-10 17:39 ` [PATCH 6/6] drm: Add CONFIG_DRM_WERROR Jani Nikula
2024-04-04 15:16 ` [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Aishwarya TCV
6 siblings, 1 reply; 26+ messages in thread
From: Jani Nikula @ 2024-01-10 17:39 UTC (permalink / raw)
To: dri-devel
Cc: Sui Jingfeng, Karol Herbst, Hamza Mahfooz, Marijn Suijten,
Javier Martinez Canillas, Danilo Krummrich, Pan, jani.nikula,
intel-gfx, Abhinav Kumar, Maxime Ripard, Alex Deucher, Sean Paul,
Xinhui, Thomas Zimmermann, Dmitry Baryshkov,
Christian König
At least the i915 and amd drivers enable a bunch more compiler warnings
than the kernel defaults.
Extend most of the W=1 warnings to the entire drm subsystem by
default. Use the copy-pasted warnings from scripts/Makefile.extrawarn
with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and
keep up with them in the future.
This is similar to the approach currently used in i915.
Some of the -Wextra warnings do need to be disabled, just like in
Makefile.extrawarn, but take care to not disable them for W=2 or W=3
builds, depending on the warning.
There are too many -Wformat-truncation warnings to cleanly fix up front;
leave that warning disabled for now.
v2:
- Drop -Wformat-truncation (too many warnings)
- Drop -Wstringop-overflow (enabled by default upstream)
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Pan, Xinhui <Xinhui.Pan@amd.com>
Cc: Karol Herbst <kherbst@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Danilo Krummrich <dakr@redhat.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Sean Paul <sean@poorly.run>
Cc: Marijn Suijten <marijn.suijten@somainline.org>
Cc: Hamza Mahfooz <hamza.mahfooz@amd.com>
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/Makefile | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 104b42df2e95..8b6be830f7c3 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -5,6 +5,33 @@
CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
+# Unconditionally enable W=1 warnings locally
+# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
+subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter
+subdir-ccflags-y += -Wmissing-declarations
+subdir-ccflags-y += $(call cc-option, -Wrestrict)
+subdir-ccflags-y += -Wmissing-format-attribute
+subdir-ccflags-y += -Wmissing-prototypes
+subdir-ccflags-y += -Wold-style-definition
+subdir-ccflags-y += -Wmissing-include-dirs
+subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
+subdir-ccflags-y += $(call cc-option, -Wunused-const-variable)
+subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned)
+subdir-ccflags-y += $(call cc-option, -Wformat-overflow)
+# FIXME: fix -Wformat-truncation warnings and uncomment
+#subdir-ccflags-y += $(call cc-option, -Wformat-truncation)
+subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
+# The following turn off the warnings enabled by -Wextra
+ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),)
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-shift-negative-value
+endif
+ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),)
+subdir-ccflags-y += -Wno-sign-compare
+endif
+# --- end copy-paste
+
drm-y := \
drm_aperture.o \
drm_atomic.o \
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 6/6] drm: Add CONFIG_DRM_WERROR
2024-01-10 17:39 [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Jani Nikula
` (4 preceding siblings ...)
2024-01-10 17:39 ` [PATCH 5/6] drm: enable (most) W=1 warnings by default across the subsystem Jani Nikula
@ 2024-01-10 17:39 ` Jani Nikula
2024-01-10 20:17 ` Hamza Mahfooz
2024-04-04 15:16 ` [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Aishwarya TCV
6 siblings, 1 reply; 26+ messages in thread
From: Jani Nikula @ 2024-01-10 17:39 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
Add kconfig to enable -Werror subsystem wide. This is useful for
development and CI to keep the subsystem warning free, while avoiding
issues outside of the subsystem that kernel wide CONFIG_WERROR=y might
hit.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/Kconfig | 18 ++++++++++++++++++
drivers/gpu/drm/Makefile | 3 +++
2 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 6ec33d36f3a4..36a00cba2540 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -414,3 +414,21 @@ config DRM_LIB_RANDOM
config DRM_PRIVACY_SCREEN
bool
default n
+
+config DRM_WERROR
+ bool "Compile the drm subsystem with warnings as errors"
+ # As this may inadvertently break the build, only allow the user
+ # to shoot oneself in the foot iff they aim really hard
+ depends on EXPERT
+ # We use the dependency on !COMPILE_TEST to not be enabled in
+ # allmodconfig or allyesconfig configurations
+ depends on !COMPILE_TEST
+ default n
+ help
+ A kernel build should not cause any compiler warnings, and this
+ enables the '-Werror' flag to enforce that rule in the drm subsystem.
+
+ The drm subsystem enables more warnings than the kernel default, so
+ this config option is disabled by default.
+
+ If in doubt, say N.
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 8b6be830f7c3..b7fd3e58b7af 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -32,6 +32,9 @@ subdir-ccflags-y += -Wno-sign-compare
endif
# --- end copy-paste
+# Enable -Werror in CI and development
+subdir-ccflags-$(CONFIG_DRM_WERROR) += -Werror
+
drm-y := \
drm_aperture.o \
drm_atomic.o \
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 5/6] drm: enable (most) W=1 warnings by default across the subsystem
2024-01-10 17:39 ` [PATCH 5/6] drm: enable (most) W=1 warnings by default across the subsystem Jani Nikula
@ 2024-01-10 20:15 ` Hamza Mahfooz
2024-01-11 9:43 ` Jani Nikula
0 siblings, 1 reply; 26+ messages in thread
From: Hamza Mahfooz @ 2024-01-10 20:15 UTC (permalink / raw)
To: Jani Nikula, dri-devel
Cc: Sui Jingfeng, Thomas Zimmermann, Karol Herbst, Sean Paul,
intel-gfx, Xinhui, Abhinav Kumar, Maxime Ripard,
Javier Martinez Canillas, Dmitry Baryshkov, Danilo Krummrich,
Alex Deucher, Marijn Suijten, Christian König
On 1/10/24 12:39, Jani Nikula wrote:
> At least the i915 and amd drivers enable a bunch more compiler warnings
> than the kernel defaults.
>
> Extend most of the W=1 warnings to the entire drm subsystem by
> default. Use the copy-pasted warnings from scripts/Makefile.extrawarn
> with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and
> keep up with them in the future.
>
> This is similar to the approach currently used in i915.
>
> Some of the -Wextra warnings do need to be disabled, just like in
> Makefile.extrawarn, but take care to not disable them for W=2 or W=3
> builds, depending on the warning.
>
> There are too many -Wformat-truncation warnings to cleanly fix up front;
> leave that warning disabled for now.
>
> v2:
> - Drop -Wformat-truncation (too many warnings)
> - Drop -Wstringop-overflow (enabled by default upstream)
>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Pan, Xinhui <Xinhui.Pan@amd.com>
> Cc: Karol Herbst <kherbst@redhat.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Danilo Krummrich <dakr@redhat.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Cc: Hamza Mahfooz <hamza.mahfooz@amd.com>
> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/Makefile | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 104b42df2e95..8b6be830f7c3 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -5,6 +5,33 @@
>
> CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
>
> +# Unconditionally enable W=1 warnings locally
> +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
> +subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter
> +subdir-ccflags-y += -Wmissing-declarations
> +subdir-ccflags-y += $(call cc-option, -Wrestrict)
It would be safer to do something along the lines of:
cond-flags := $(call cc-option, -Wrestrict) \
$(call cc-option, -Wunused-but-set-variable) \
$(call cc-option, -Wunused-const-variable) \
$(call cc-option, -Wunused-const-variable) \
$(call cc-option, -Wformat-overflow) \
$(call cc-option, -Wstringop-truncation)
subdir-ccflags-y += $(cond-flags)
Otherwise, you will end up breaking `$ make M=drivers/gpu/drm`
for a bunch of people.
> +subdir-ccflags-y += -Wmissing-format-attribute
> +subdir-ccflags-y += -Wmissing-prototypes
> +subdir-ccflags-y += -Wold-style-definition
> +subdir-ccflags-y += -Wmissing-include-dirs
> +subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
> +subdir-ccflags-y += $(call cc-option, -Wunused-const-variable)
> +subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned)
> +subdir-ccflags-y += $(call cc-option, -Wformat-overflow)
> +# FIXME: fix -Wformat-truncation warnings and uncomment
> +#subdir-ccflags-y += $(call cc-option, -Wformat-truncation)
> +subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
> +# The following turn off the warnings enabled by -Wextra
> +ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),)
> +subdir-ccflags-y += -Wno-missing-field-initializers
> +subdir-ccflags-y += -Wno-type-limits
> +subdir-ccflags-y += -Wno-shift-negative-value
> +endif
> +ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),)
> +subdir-ccflags-y += -Wno-sign-compare
> +endif
> +# --- end copy-paste
> +
> drm-y := \
> drm_aperture.o \
> drm_atomic.o \
--
Hamza
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 6/6] drm: Add CONFIG_DRM_WERROR
2024-01-10 17:39 ` [PATCH 6/6] drm: Add CONFIG_DRM_WERROR Jani Nikula
@ 2024-01-10 20:17 ` Hamza Mahfooz
0 siblings, 0 replies; 26+ messages in thread
From: Hamza Mahfooz @ 2024-01-10 20:17 UTC (permalink / raw)
To: Jani Nikula, dri-devel; +Cc: intel-gfx
On 1/10/24 12:39, Jani Nikula wrote:
> Add kconfig to enable -Werror subsystem wide. This is useful for
> development and CI to keep the subsystem warning free, while avoiding
> issues outside of the subsystem that kernel wide CONFIG_WERROR=y might
> hit.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
> ---
> drivers/gpu/drm/Kconfig | 18 ++++++++++++++++++
> drivers/gpu/drm/Makefile | 3 +++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 6ec33d36f3a4..36a00cba2540 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -414,3 +414,21 @@ config DRM_LIB_RANDOM
> config DRM_PRIVACY_SCREEN
> bool
> default n
> +
> +config DRM_WERROR
> + bool "Compile the drm subsystem with warnings as errors"
> + # As this may inadvertently break the build, only allow the user
> + # to shoot oneself in the foot iff they aim really hard
> + depends on EXPERT
> + # We use the dependency on !COMPILE_TEST to not be enabled in
> + # allmodconfig or allyesconfig configurations
> + depends on !COMPILE_TEST
> + default n
> + help
> + A kernel build should not cause any compiler warnings, and this
> + enables the '-Werror' flag to enforce that rule in the drm subsystem.
> +
> + The drm subsystem enables more warnings than the kernel default, so
> + this config option is disabled by default.
> +
> + If in doubt, say N.
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 8b6be830f7c3..b7fd3e58b7af 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -32,6 +32,9 @@ subdir-ccflags-y += -Wno-sign-compare
> endif
> # --- end copy-paste
>
> +# Enable -Werror in CI and development
> +subdir-ccflags-$(CONFIG_DRM_WERROR) += -Werror
> +
> drm-y := \
> drm_aperture.o \
> drm_atomic.o \
--
Hamza
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 5/6] drm: enable (most) W=1 warnings by default across the subsystem
2024-01-10 20:15 ` Hamza Mahfooz
@ 2024-01-11 9:43 ` Jani Nikula
0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-11 9:43 UTC (permalink / raw)
To: Hamza Mahfooz, dri-devel
Cc: Sui Jingfeng, Thomas Zimmermann, Karol Herbst, Sean Paul,
intel-gfx, Xinhui, Abhinav Kumar, Maxime Ripard,
Javier Martinez Canillas, Dmitry Baryshkov, Danilo Krummrich,
Alex Deucher, Marijn Suijten, Masahiro Yamada,
Christian König
On Wed, 10 Jan 2024, Hamza Mahfooz <hamza.mahfooz@amd.com> wrote:
> On 1/10/24 12:39, Jani Nikula wrote:
>> At least the i915 and amd drivers enable a bunch more compiler warnings
>> than the kernel defaults.
>>
>> Extend most of the W=1 warnings to the entire drm subsystem by
>> default. Use the copy-pasted warnings from scripts/Makefile.extrawarn
>> with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and
>> keep up with them in the future.
>>
>> This is similar to the approach currently used in i915.
>>
>> Some of the -Wextra warnings do need to be disabled, just like in
>> Makefile.extrawarn, but take care to not disable them for W=2 or W=3
>> builds, depending on the warning.
>>
>> There are too many -Wformat-truncation warnings to cleanly fix up front;
>> leave that warning disabled for now.
>>
>> v2:
>> - Drop -Wformat-truncation (too many warnings)
>> - Drop -Wstringop-overflow (enabled by default upstream)
>>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Pan, Xinhui <Xinhui.Pan@amd.com>
>> Cc: Karol Herbst <kherbst@redhat.com>
>> Cc: Lyude Paul <lyude@redhat.com>
>> Cc: Danilo Krummrich <dakr@redhat.com>
>> Cc: Rob Clark <robdclark@gmail.com>
>> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Cc: Sean Paul <sean@poorly.run>
>> Cc: Marijn Suijten <marijn.suijten@somainline.org>
>> Cc: Hamza Mahfooz <hamza.mahfooz@amd.com>
>> Acked-by: Javier Martinez Canillas <javierm@redhat.com>
>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/Makefile | 27 +++++++++++++++++++++++++++
>> 1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 104b42df2e95..8b6be830f7c3 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -5,6 +5,33 @@
>>
>> CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
>>
>> +# Unconditionally enable W=1 warnings locally
>> +# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
>> +subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter
>> +subdir-ccflags-y += -Wmissing-declarations
>> +subdir-ccflags-y += $(call cc-option, -Wrestrict)
>
> It would be safer to do something along the lines of:
>
> cond-flags := $(call cc-option, -Wrestrict) \
> $(call cc-option, -Wunused-but-set-variable) \
> $(call cc-option, -Wunused-const-variable) \
> $(call cc-option, -Wunused-const-variable) \
> $(call cc-option, -Wformat-overflow) \
> $(call cc-option, -Wstringop-truncation)
> subdir-ccflags-y += $(cond-flags)
>
> Otherwise, you will end up breaking `$ make M=drivers/gpu/drm`
> for a bunch of people.
I discussed this with Alex on IRC yesterday. The above seems obviously
correct in that it just changes the evaluation time of $(call cc-option,
...). Apparently not having it may lead to:
scripts/Makefile.lib:10: *** Recursive variable 'KBUILD_CFLAGS' references itself (eventually). Stop.
Of course, I could just throw that in and be happy, but me being me, I'd
really like to know what is going on here first. :)
For one thing, I always thought M=dir was only for out-of-tree modules,
though the IRC discussion seems to indicate a lot of people also use it
for in-tree modules. But I can't even make it to work for a lot of cases
on top of current drm-tip, without the changes here.
M=drivers/gpu/drm/i915 fails immediately. So does
M=drivers/gpu/drm/amd. And M=drivers/gpu/drm/nouveau. And
M=drivers/gpu/drm/radeon.
M=drivers/gpu/drm fails with the same cases as above. I always use
KBUILD_OUTPUT=dir but I don't know if it makes a difference, I didn't
try.
However M=drivers/gpu/drm/amd/amdgpu works.
The only way I could reproduce the "recursive variable" issue in that
was using:
subdir-ccflags-y = -Wextra
instead of:
subdir-ccflags-y := -Wextra
or:
subdir-ccflags-y += -Wextra
in amdgpu/Makefile
Since I use the latter form in this pach, I think it should be fine for
M=dir users even if M=dir doesn't really seem to generally work for
in-tree modules (at least not for me).
Cc: Masahiro
BR,
Jani.
>
>> +subdir-ccflags-y += -Wmissing-format-attribute
>> +subdir-ccflags-y += -Wmissing-prototypes
>> +subdir-ccflags-y += -Wold-style-definition
>> +subdir-ccflags-y += -Wmissing-include-dirs
>> +subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
>> +subdir-ccflags-y += $(call cc-option, -Wunused-const-variable)
>> +subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned)
>> +subdir-ccflags-y += $(call cc-option, -Wformat-overflow)
>> +# FIXME: fix -Wformat-truncation warnings and uncomment
>> +#subdir-ccflags-y += $(call cc-option, -Wformat-truncation)
>> +subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
>> +# The following turn off the warnings enabled by -Wextra
>> +ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),)
>> +subdir-ccflags-y += -Wno-missing-field-initializers
>> +subdir-ccflags-y += -Wno-type-limits
>> +subdir-ccflags-y += -Wno-shift-negative-value
>> +endif
>> +ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),)
>> +subdir-ccflags-y += -Wno-sign-compare
>> +endif
>> +# --- end copy-paste
>> +
>> drm-y := \
>> drm_aperture.o \
>> drm_atomic.o \
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf
2024-01-10 17:39 ` [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf Jani Nikula
@ 2024-01-12 4:09 ` kernel test robot
2024-01-12 9:07 ` Jani Nikula
2024-01-12 14:57 ` Alex Deucher
1 sibling, 1 reply; 26+ messages in thread
From: kernel test robot @ 2024-01-12 4:09 UTC (permalink / raw)
To: Jani Nikula, dri-devel
Cc: Pan, jani.nikula, intel-gfx, Xinhui, amd-gfx, oe-kbuild-all,
Alex Deucher, Christian König
Hi Jani,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7 next-20240111]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-nouveau-acr-ga102-remove-unused-but-set-variable/20240111-014206
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/fea7a52924f98b1ac24f4a7e6ba21d7754422430.1704908087.git.jani.nikula%40intel.com
patch subject: [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf
config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20240112/202401121126.i9VGrvMb-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240112/202401121126.i9VGrvMb-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401121126.i9VGrvMb-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c: In function 'amdgpu_gfx_kiq_init_ring':
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c:332:61: warning: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size between 0 and 8 [-Wformat-truncation=]
332 | snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
| ^~
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c:332:50: note: directive argument in the range [0, 2147483647]
332 | snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
| ^~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c:332:9: note: 'snprintf' output between 12 and 41 bytes into a destination of size 16
332 | snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
333 | xcc_id, ring->me, ring->pipe, ring->queue);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +332 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
306
307 int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
308 struct amdgpu_ring *ring,
309 struct amdgpu_irq_src *irq, int xcc_id)
310 {
311 struct amdgpu_kiq *kiq = &adev->gfx.kiq[xcc_id];
312 int r = 0;
313
314 spin_lock_init(&kiq->ring_lock);
315
316 ring->adev = NULL;
317 ring->ring_obj = NULL;
318 ring->use_doorbell = true;
319 ring->xcc_id = xcc_id;
320 ring->vm_hub = AMDGPU_GFXHUB(xcc_id);
321 ring->doorbell_index =
322 (adev->doorbell_index.kiq +
323 xcc_id * adev->doorbell_index.xcc_doorbell_range)
324 << 1;
325
326 r = amdgpu_gfx_kiq_acquire(adev, ring, xcc_id);
327 if (r)
328 return r;
329
330 ring->eop_gpu_addr = kiq->eop_gpu_addr;
331 ring->no_scheduler = true;
> 332 snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
333 xcc_id, ring->me, ring->pipe, ring->queue);
334 r = amdgpu_ring_init(adev, ring, 1024, irq, AMDGPU_CP_KIQ_IRQ_DRIVER0,
335 AMDGPU_RING_PRIO_DEFAULT, NULL);
336 if (r)
337 dev_warn(adev->dev, "(%d) failed to init kiq ring\n", r);
338
339 return r;
340 }
341
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf
2024-01-12 4:09 ` kernel test robot
@ 2024-01-12 9:07 ` Jani Nikula
0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-12 9:07 UTC (permalink / raw)
To: kernel test robot, dri-devel
Cc: Pan, intel-gfx, Xinhui, amd-gfx, oe-kbuild-all, Alex Deucher,
Christian König
On Fri, 12 Jan 2024, kernel test robot <lkp@intel.com> wrote:
> Hi Jani,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on drm-misc/drm-misc-next]
> [also build test WARNING on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7 next-20240111]
> [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#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-nouveau-acr-ga102-remove-unused-but-set-variable/20240111-014206
> base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> patch link: https://lore.kernel.org/r/fea7a52924f98b1ac24f4a7e6ba21d7754422430.1704908087.git.jani.nikula%40intel.com
> patch subject: [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf
> config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20240112/202401121126.i9VGrvMb-lkp@intel.com/config)
> compiler: sparc64-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240112/202401121126.i9VGrvMb-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202401121126.i9VGrvMb-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c: In function 'amdgpu_gfx_kiq_init_ring':
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c:332:61: warning: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size between 0 and 8 [-Wformat-truncation=]
> 332 | snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
> | ^~
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c:332:50: note: directive argument in the range [0, 2147483647]
> 332 | snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
> | ^~~~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c:332:9: note: 'snprintf' output between 12 and 41 bytes into a destination of size 16
> 332 | snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 333 | xcc_id, ring->me, ring->pipe, ring->queue);
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
As the commit message says,
This will trade the W=1 warning -Wformat-overflow to
-Wformat-truncation. This lets us enable -Wformat-overflow subsystem
wide.
BR,
Jani.
>
>
> vim +332 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>
> 306
> 307 int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
> 308 struct amdgpu_ring *ring,
> 309 struct amdgpu_irq_src *irq, int xcc_id)
> 310 {
> 311 struct amdgpu_kiq *kiq = &adev->gfx.kiq[xcc_id];
> 312 int r = 0;
> 313
> 314 spin_lock_init(&kiq->ring_lock);
> 315
> 316 ring->adev = NULL;
> 317 ring->ring_obj = NULL;
> 318 ring->use_doorbell = true;
> 319 ring->xcc_id = xcc_id;
> 320 ring->vm_hub = AMDGPU_GFXHUB(xcc_id);
> 321 ring->doorbell_index =
> 322 (adev->doorbell_index.kiq +
> 323 xcc_id * adev->doorbell_index.xcc_doorbell_range)
> 324 << 1;
> 325
> 326 r = amdgpu_gfx_kiq_acquire(adev, ring, xcc_id);
> 327 if (r)
> 328 return r;
> 329
> 330 ring->eop_gpu_addr = kiq->eop_gpu_addr;
> 331 ring->no_scheduler = true;
> > 332 snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
> 333 xcc_id, ring->me, ring->pipe, ring->queue);
> 334 r = amdgpu_ring_init(adev, ring, 1024, irq, AMDGPU_CP_KIQ_IRQ_DRIVER0,
> 335 AMDGPU_RING_PRIO_DEFAULT, NULL);
> 336 if (r)
> 337 dev_warn(adev->dev, "(%d) failed to init kiq ring\n", r);
> 338
> 339 return r;
> 340 }
> 341
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/6] drm/imx: prefer snprintf over sprintf
2024-01-10 17:39 ` [PATCH 4/6] drm/imx: " Jani Nikula
@ 2024-01-12 10:49 ` kernel test robot
2024-01-12 14:20 ` Philipp Zabel
1 sibling, 0 replies; 26+ messages in thread
From: kernel test robot @ 2024-01-12 10:49 UTC (permalink / raw)
To: Jani Nikula, dri-devel; +Cc: jani.nikula, intel-gfx, oe-kbuild-all
Hi Jani,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7 next-20240112]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-nouveau-acr-ga102-remove-unused-but-set-variable/20240111-014206
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/14c0108a54007a8360d84162a1d63cba9613b945.1704908087.git.jani.nikula%40intel.com
patch subject: [PATCH 4/6] drm/imx: prefer snprintf over sprintf
config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20240112/202401121801.3j6GnsGm-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240112/202401121801.3j6GnsGm-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401121801.3j6GnsGm-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/imx/ipuv3/imx-ldb.c: In function 'imx_ldb_probe':
>> drivers/gpu/drm/imx/ipuv3/imx-ldb.c:658:57: warning: '_sel' directive output may be truncated writing 4 bytes into a region of size between 3 and 13 [-Wformat-truncation=]
658 | snprintf(clkname, sizeof(clkname), "di%d_sel", i);
| ^~~~
drivers/gpu/drm/imx/ipuv3/imx-ldb.c:658:17: note: 'snprintf' output between 8 and 18 bytes into a destination of size 16
658 | snprintf(clkname, sizeof(clkname), "di%d_sel", i);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/_sel +658 drivers/gpu/drm/imx/ipuv3/imx-ldb.c
617
618 static int imx_ldb_probe(struct platform_device *pdev)
619 {
620 struct device *dev = &pdev->dev;
621 struct device_node *np = dev->of_node;
622 struct device_node *child;
623 struct imx_ldb *imx_ldb;
624 int dual;
625 int ret;
626 int i;
627
628 imx_ldb = devm_kzalloc(dev, sizeof(*imx_ldb), GFP_KERNEL);
629 if (!imx_ldb)
630 return -ENOMEM;
631
632 imx_ldb->regmap = syscon_regmap_lookup_by_phandle(np, "gpr");
633 if (IS_ERR(imx_ldb->regmap)) {
634 dev_err(dev, "failed to get parent regmap\n");
635 return PTR_ERR(imx_ldb->regmap);
636 }
637
638 /* disable LDB by resetting the control register to POR default */
639 regmap_write(imx_ldb->regmap, IOMUXC_GPR2, 0);
640
641 imx_ldb->dev = dev;
642 imx_ldb->lvds_mux = device_get_match_data(dev);
643
644 dual = of_property_read_bool(np, "fsl,dual-channel");
645 if (dual)
646 imx_ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN;
647
648 /*
649 * There are three different possible clock mux configurations:
650 * i.MX53: ipu1_di0_sel, ipu1_di1_sel
651 * i.MX6q: ipu1_di0_sel, ipu1_di1_sel, ipu2_di0_sel, ipu2_di1_sel
652 * i.MX6dl: ipu1_di0_sel, ipu1_di1_sel, lcdif_sel
653 * Map them all to di0_sel...di3_sel.
654 */
655 for (i = 0; i < 4; i++) {
656 char clkname[16];
657
> 658 snprintf(clkname, sizeof(clkname), "di%d_sel", i);
659 imx_ldb->clk_sel[i] = devm_clk_get(imx_ldb->dev, clkname);
660 if (IS_ERR(imx_ldb->clk_sel[i])) {
661 ret = PTR_ERR(imx_ldb->clk_sel[i]);
662 imx_ldb->clk_sel[i] = NULL;
663 break;
664 }
665
666 imx_ldb->clk_parent[i] = clk_get_parent(imx_ldb->clk_sel[i]);
667 }
668 if (i == 0)
669 return ret;
670
671 for_each_child_of_node(np, child) {
672 struct imx_ldb_channel *channel;
673 int bus_format;
674
675 ret = of_property_read_u32(child, "reg", &i);
676 if (ret || i < 0 || i > 1) {
677 ret = -EINVAL;
678 goto free_child;
679 }
680
681 if (!of_device_is_available(child))
682 continue;
683
684 if (dual && i > 0) {
685 dev_warn(dev, "dual-channel mode, ignoring second output\n");
686 continue;
687 }
688
689 channel = &imx_ldb->channel[i];
690 channel->ldb = imx_ldb;
691 channel->chno = i;
692
693 /*
694 * The output port is port@4 with an external 4-port mux or
695 * port@2 with the internal 2-port mux.
696 */
697 ret = drm_of_find_panel_or_bridge(child,
698 imx_ldb->lvds_mux ? 4 : 2, 0,
699 &channel->panel, &channel->bridge);
700 if (ret && ret != -ENODEV)
701 goto free_child;
702
703 /* panel ddc only if there is no bridge */
704 if (!channel->bridge) {
705 ret = imx_ldb_panel_ddc(dev, channel, child);
706 if (ret)
707 goto free_child;
708 }
709
710 bus_format = of_get_bus_format(dev, child);
711 if (bus_format == -EINVAL) {
712 /*
713 * If no bus format was specified in the device tree,
714 * we can still get it from the connected panel later.
715 */
716 if (channel->panel && channel->panel->funcs &&
717 channel->panel->funcs->get_modes)
718 bus_format = 0;
719 }
720 if (bus_format < 0) {
721 dev_err(dev, "could not determine data mapping: %d\n",
722 bus_format);
723 ret = bus_format;
724 goto free_child;
725 }
726 channel->bus_format = bus_format;
727 channel->child = child;
728 }
729
730 platform_set_drvdata(pdev, imx_ldb);
731
732 return component_add(&pdev->dev, &imx_ldb_ops);
733
734 free_child:
735 of_node_put(child);
736 return ret;
737 }
738
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/6] drm/imx: prefer snprintf over sprintf
2024-01-10 17:39 ` [PATCH 4/6] drm/imx: " Jani Nikula
2024-01-12 10:49 ` kernel test robot
@ 2024-01-12 14:20 ` Philipp Zabel
2024-01-31 12:49 ` Jani Nikula
1 sibling, 1 reply; 26+ messages in thread
From: Philipp Zabel @ 2024-01-12 14:20 UTC (permalink / raw)
To: Jani Nikula, dri-devel; +Cc: intel-gfx
Hi Jani,
On Mi, 2024-01-10 at 19:39 +0200, Jani Nikula wrote:
> This will trade the W=1 warning -Wformat-overflow to
> -Wformat-truncation. This lets us enable -Wformat-overflow subsystem
> wide.
>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf
2024-01-10 17:39 ` [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf Jani Nikula
2024-01-12 4:09 ` kernel test robot
@ 2024-01-12 14:57 ` Alex Deucher
2024-01-31 12:48 ` Jani Nikula
1 sibling, 1 reply; 26+ messages in thread
From: Alex Deucher @ 2024-01-12 14:57 UTC (permalink / raw)
To: Jani Nikula
Cc: Pan, intel-gfx, Xinhui, amd-gfx, dri-devel, Alex Deucher,
Christian König
On Wed, Jan 10, 2024 at 12:39 PM Jani Nikula <jani.nikula@intel.com> wrote:
>
> This will trade the W=1 warning -Wformat-overflow to
> -Wformat-truncation. This lets us enable -Wformat-overflow subsystem
> wide.
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Pan, Xinhui <Xinhui.Pan@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
Feel free to take this via whichever tree makes sense.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index b9674c57c436..82b4b2019fca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -329,7 +329,8 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>
> ring->eop_gpu_addr = kiq->eop_gpu_addr;
> ring->no_scheduler = true;
> - sprintf(ring->name, "kiq_%d.%d.%d.%d", xcc_id, ring->me, ring->pipe, ring->queue);
> + snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
> + xcc_id, ring->me, ring->pipe, ring->queue);
> r = amdgpu_ring_init(adev, ring, 1024, irq, AMDGPU_CP_KIQ_IRQ_DRIVER0,
> AMDGPU_RING_PRIO_DEFAULT, NULL);
> if (r)
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf
2024-01-12 14:57 ` Alex Deucher
@ 2024-01-31 12:48 ` Jani Nikula
0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-31 12:48 UTC (permalink / raw)
To: Alex Deucher
Cc: Pan, intel-gfx, Xinhui, amd-gfx, dri-devel, Alex Deucher,
Christian König
On Fri, 12 Jan 2024, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Wed, Jan 10, 2024 at 12:39 PM Jani Nikula <jani.nikula@intel.com> wrote:
>>
>> This will trade the W=1 warning -Wformat-overflow to
>> -Wformat-truncation. This lets us enable -Wformat-overflow subsystem
>> wide.
>>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Pan, Xinhui <Xinhui.Pan@amd.com>
>> Cc: amd-gfx@lists.freedesktop.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>
> Feel free to take this via whichever tree makes sense.
Thanks, pushed this one patch to drm-misc-next as prep work.
BR,
Jani.
>
> Alex
>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> index b9674c57c436..82b4b2019fca 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> @@ -329,7 +329,8 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>>
>> ring->eop_gpu_addr = kiq->eop_gpu_addr;
>> ring->no_scheduler = true;
>> - sprintf(ring->name, "kiq_%d.%d.%d.%d", xcc_id, ring->me, ring->pipe, ring->queue);
>> + snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
>> + xcc_id, ring->me, ring->pipe, ring->queue);
>> r = amdgpu_ring_init(adev, ring, 1024, irq, AMDGPU_CP_KIQ_IRQ_DRIVER0,
>> AMDGPU_RING_PRIO_DEFAULT, NULL);
>> if (r)
>> --
>> 2.39.2
>>
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/6] drm/imx: prefer snprintf over sprintf
2024-01-12 14:20 ` Philipp Zabel
@ 2024-01-31 12:49 ` Jani Nikula
0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-31 12:49 UTC (permalink / raw)
To: Philipp Zabel, dri-devel; +Cc: intel-gfx
On Fri, 12 Jan 2024, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Hi Jani,
>
> On Mi, 2024-01-10 at 19:39 +0200, Jani Nikula wrote:
>> This will trade the W=1 warning -Wformat-overflow to
>> -Wformat-truncation. This lets us enable -Wformat-overflow subsystem
>> wide.
>>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Thanks, pushed this one patch to drm-misc-next as prep work.
BR,
Jani.
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable
2024-01-10 17:39 ` [PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable Jani Nikula
@ 2024-01-31 12:50 ` Jani Nikula
2024-01-31 19:01 ` Danilo Krummrich
1 sibling, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-31 12:50 UTC (permalink / raw)
To: dri-devel; +Cc: Danilo Krummrich, intel-gfx, Karol Herbst, nouveau
On Wed, 10 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> Fix the W=1 warning -Wunused-but-set-variable.
>
> Cc: Karol Herbst <kherbst@redhat.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Danilo Krummrich <dakr@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Ping?
> ---
> drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
> index f36a359d4531..bd104a030243 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
> @@ -218,7 +218,7 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev *subdev,
> const struct firmware *hsbl;
> const struct nvfw_ls_hsbl_bin_hdr *hdr;
> const struct nvfw_ls_hsbl_hdr *hshdr;
> - u32 loc, sig, cnt, *meta;
> + u32 sig, cnt, *meta;
>
> ret = nvkm_firmware_load_name(subdev, path, "hs_bl_sig", ver, &hsbl);
> if (ret)
> @@ -227,7 +227,6 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev *subdev,
> hdr = nvfw_ls_hsbl_bin_hdr(subdev, hsbl->data);
> hshdr = nvfw_ls_hsbl_hdr(subdev, hsbl->data + hdr->header_offset);
> meta = (u32 *)(hsbl->data + hshdr->meta_data_offset);
> - loc = *(u32 *)(hsbl->data + hshdr->patch_loc);
> sig = *(u32 *)(hsbl->data + hshdr->patch_sig);
> cnt = *(u32 *)(hsbl->data + hshdr->num_sig);
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/6] drm/nouveau/svm: remove unused but set variables
2024-01-10 17:39 ` [PATCH 2/6] drm/nouveau/svm: remove unused but set variables Jani Nikula
@ 2024-01-31 12:51 ` Jani Nikula
2024-01-31 19:10 ` Danilo Krummrich
1 sibling, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2024-01-31 12:51 UTC (permalink / raw)
To: dri-devel; +Cc: Danilo Krummrich, intel-gfx, Karol Herbst, nouveau
On Wed, 10 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> Fix the W=1 warning -Wunused-but-set-variable.
>
> Cc: Karol Herbst <kherbst@redhat.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Danilo Krummrich <dakr@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Ping?
> ---
> drivers/gpu/drm/nouveau/nouveau_svm.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index cc03e0c22ff3..4d1008915499 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -112,7 +112,7 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
> {
> struct nouveau_cli *cli = nouveau_cli(file_priv);
> struct drm_nouveau_svm_bind *args = data;
> - unsigned target, cmd, priority;
> + unsigned target, cmd;
> unsigned long addr, end;
> struct mm_struct *mm;
>
> @@ -136,9 +136,6 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
> return -EINVAL;
> }
>
> - priority = args->header >> NOUVEAU_SVM_BIND_PRIORITY_SHIFT;
> - priority &= NOUVEAU_SVM_BIND_PRIORITY_MASK;
> -
> /* FIXME support CPU target ie all target value < GPU_VRAM */
> target = args->header >> NOUVEAU_SVM_BIND_TARGET_SHIFT;
> target &= NOUVEAU_SVM_BIND_TARGET_MASK;
> @@ -926,15 +923,14 @@ nouveau_pfns_map(struct nouveau_svmm *svmm, struct mm_struct *mm,
> unsigned long addr, u64 *pfns, unsigned long npages)
> {
> struct nouveau_pfnmap_args *args = nouveau_pfns_to_args(pfns);
> - int ret;
>
> args->p.addr = addr;
> args->p.size = npages << PAGE_SHIFT;
>
> mutex_lock(&svmm->mutex);
>
> - ret = nvif_object_ioctl(&svmm->vmm->vmm.object, args,
> - struct_size(args, p.phys, npages), NULL);
> + nvif_object_ioctl(&svmm->vmm->vmm.object, args,
> + struct_size(args, p.phys, npages), NULL);
>
> mutex_unlock(&svmm->mutex);
> }
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable
2024-01-10 17:39 ` [PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable Jani Nikula
2024-01-31 12:50 ` Jani Nikula
@ 2024-01-31 19:01 ` Danilo Krummrich
2024-02-01 9:41 ` Jani Nikula
1 sibling, 1 reply; 26+ messages in thread
From: Danilo Krummrich @ 2024-01-31 19:01 UTC (permalink / raw)
To: Jani Nikula, dri-devel; +Cc: nouveau, intel-gfx, Karol Herbst
On 1/10/24 18:39, Jani Nikula wrote:
> Fix the W=1 warning -Wunused-but-set-variable.
>
> Cc: Karol Herbst <kherbst@redhat.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Danilo Krummrich <dakr@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Danilo Krummrich <dakr@redhat.com>
> ---
> drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
> index f36a359d4531..bd104a030243 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
> @@ -218,7 +218,7 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev *subdev,
> const struct firmware *hsbl;
> const struct nvfw_ls_hsbl_bin_hdr *hdr;
> const struct nvfw_ls_hsbl_hdr *hshdr;
> - u32 loc, sig, cnt, *meta;
> + u32 sig, cnt, *meta;
>
> ret = nvkm_firmware_load_name(subdev, path, "hs_bl_sig", ver, &hsbl);
> if (ret)
> @@ -227,7 +227,6 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev *subdev,
> hdr = nvfw_ls_hsbl_bin_hdr(subdev, hsbl->data);
> hshdr = nvfw_ls_hsbl_hdr(subdev, hsbl->data + hdr->header_offset);
> meta = (u32 *)(hsbl->data + hshdr->meta_data_offset);
> - loc = *(u32 *)(hsbl->data + hshdr->patch_loc);
> sig = *(u32 *)(hsbl->data + hshdr->patch_sig);
> cnt = *(u32 *)(hsbl->data + hshdr->num_sig);
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/6] drm/nouveau/svm: remove unused but set variables
2024-01-10 17:39 ` [PATCH 2/6] drm/nouveau/svm: remove unused but set variables Jani Nikula
2024-01-31 12:51 ` Jani Nikula
@ 2024-01-31 19:10 ` Danilo Krummrich
2024-02-01 9:42 ` Jani Nikula
1 sibling, 1 reply; 26+ messages in thread
From: Danilo Krummrich @ 2024-01-31 19:10 UTC (permalink / raw)
To: Jani Nikula, dri-devel; +Cc: nouveau, intel-gfx, Karol Herbst
On 1/10/24 18:39, Jani Nikula wrote:
> Fix the W=1 warning -Wunused-but-set-variable.
>
> Cc: Karol Herbst <kherbst@redhat.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Danilo Krummrich <dakr@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Danilo Krummrich <dakr@redhat.com>
> ---
> drivers/gpu/drm/nouveau/nouveau_svm.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index cc03e0c22ff3..4d1008915499 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -112,7 +112,7 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
> {
> struct nouveau_cli *cli = nouveau_cli(file_priv);
> struct drm_nouveau_svm_bind *args = data;
> - unsigned target, cmd, priority;
> + unsigned target, cmd;
> unsigned long addr, end;
> struct mm_struct *mm;
>
> @@ -136,9 +136,6 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
> return -EINVAL;
> }
>
> - priority = args->header >> NOUVEAU_SVM_BIND_PRIORITY_SHIFT;
> - priority &= NOUVEAU_SVM_BIND_PRIORITY_MASK;
> -
> /* FIXME support CPU target ie all target value < GPU_VRAM */
> target = args->header >> NOUVEAU_SVM_BIND_TARGET_SHIFT;
> target &= NOUVEAU_SVM_BIND_TARGET_MASK;
> @@ -926,15 +923,14 @@ nouveau_pfns_map(struct nouveau_svmm *svmm, struct mm_struct *mm,
> unsigned long addr, u64 *pfns, unsigned long npages)
> {
> struct nouveau_pfnmap_args *args = nouveau_pfns_to_args(pfns);
> - int ret;
>
> args->p.addr = addr;
> args->p.size = npages << PAGE_SHIFT;
>
> mutex_lock(&svmm->mutex);
>
> - ret = nvif_object_ioctl(&svmm->vmm->vmm.object, args,
> - struct_size(args, p.phys, npages), NULL);
> + nvif_object_ioctl(&svmm->vmm->vmm.object, args,
> + struct_size(args, p.phys, npages), NULL);
>
> mutex_unlock(&svmm->mutex);
> }
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable
2024-01-31 19:01 ` Danilo Krummrich
@ 2024-02-01 9:41 ` Jani Nikula
0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2024-02-01 9:41 UTC (permalink / raw)
To: Danilo Krummrich, dri-devel; +Cc: nouveau, intel-gfx, Karol Herbst
On Wed, 31 Jan 2024, Danilo Krummrich <dakr@redhat.com> wrote:
> On 1/10/24 18:39, Jani Nikula wrote:
>> Fix the W=1 warning -Wunused-but-set-variable.
>>
>> Cc: Karol Herbst <kherbst@redhat.com>
>> Cc: Lyude Paul <lyude@redhat.com>
>> Cc: Danilo Krummrich <dakr@redhat.com>
>> Cc: nouveau@lists.freedesktop.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Reviewed-by: Danilo Krummrich <dakr@redhat.com>
Thanks, pushed to drm-misc-next.
>
>> ---
>> drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
>> index f36a359d4531..bd104a030243 100644
>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
>> @@ -218,7 +218,7 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev *subdev,
>> const struct firmware *hsbl;
>> const struct nvfw_ls_hsbl_bin_hdr *hdr;
>> const struct nvfw_ls_hsbl_hdr *hshdr;
>> - u32 loc, sig, cnt, *meta;
>> + u32 sig, cnt, *meta;
>>
>> ret = nvkm_firmware_load_name(subdev, path, "hs_bl_sig", ver, &hsbl);
>> if (ret)
>> @@ -227,7 +227,6 @@ nvkm_acr_lsfw_load_sig_image_desc_v2(struct nvkm_subdev *subdev,
>> hdr = nvfw_ls_hsbl_bin_hdr(subdev, hsbl->data);
>> hshdr = nvfw_ls_hsbl_hdr(subdev, hsbl->data + hdr->header_offset);
>> meta = (u32 *)(hsbl->data + hshdr->meta_data_offset);
>> - loc = *(u32 *)(hsbl->data + hshdr->patch_loc);
>> sig = *(u32 *)(hsbl->data + hshdr->patch_sig);
>> cnt = *(u32 *)(hsbl->data + hshdr->num_sig);
>>
>
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/6] drm/nouveau/svm: remove unused but set variables
2024-01-31 19:10 ` Danilo Krummrich
@ 2024-02-01 9:42 ` Jani Nikula
0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2024-02-01 9:42 UTC (permalink / raw)
To: Danilo Krummrich, dri-devel; +Cc: nouveau, intel-gfx, Karol Herbst
On Wed, 31 Jan 2024, Danilo Krummrich <dakr@redhat.com> wrote:
> On 1/10/24 18:39, Jani Nikula wrote:
>> Fix the W=1 warning -Wunused-but-set-variable.
>>
>> Cc: Karol Herbst <kherbst@redhat.com>
>> Cc: Lyude Paul <lyude@redhat.com>
>> Cc: Danilo Krummrich <dakr@redhat.com>
>> Cc: nouveau@lists.freedesktop.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Reviewed-by: Danilo Krummrich <dakr@redhat.com>
Thanks, pushed to drm-misc-next.
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem
2024-01-10 17:39 [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Jani Nikula
` (5 preceding siblings ...)
2024-01-10 17:39 ` [PATCH 6/6] drm: Add CONFIG_DRM_WERROR Jani Nikula
@ 2024-04-04 15:16 ` Aishwarya TCV
2024-04-05 9:57 ` Jani Nikula
6 siblings, 1 reply; 26+ messages in thread
From: Aishwarya TCV @ 2024-04-04 15:16 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Mark Brown, dri-devel
On 10/01/2024 17:39, Jani Nikula wrote:
> This is v2 of [1] to enable most W=1 warnings across the drm
> subsystem. Some fixes first, and a CONFIG_DRM_WERROR on top.
>
> I build tested this for x86 (both gcc and clang), arm and arm64.
>
> BR,
> Jani.
>
>
> [1] https://lore.kernel.org/r/20231129181219.1237887-1-jani.nikula@intel.com
>
>
>
>
> Jani Nikula (6):
> drm/nouveau/acr/ga102: remove unused but set variable
> drm/nouveau/svm: remove unused but set variables
> drm/amdgpu: prefer snprintf over sprintf
> drm/imx: prefer snprintf over sprintf
> drm: enable (most) W=1 warnings by default across the subsystem
> drm: Add CONFIG_DRM_WERROR
>
> drivers/gpu/drm/Kconfig | 18 +++++++++++
> drivers/gpu/drm/Makefile | 30 +++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 3 +-
> drivers/gpu/drm/imx/ipuv3/imx-ldb.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_svm.c | 10 ++-----
> .../gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c | 3 +-
> 6 files changed, 55 insertions(+), 11 deletions(-)
>
Hi Jani,
Observed warning "include/drm/drm_print.h:536:35: warning: '%4.4s'
directive argument is null [-Wformat-overflow=]" when building the
modules with "defconfig+kselftest-ftrace"(
https://github.com/torvalds/linux/blob/master/tools/testing/selftests/ftrace/config
) against next-master(next-20240404) kernel with Arm64 in our CI.
A bisect identified a61ddb4393ad1be61d2ffd92576d42707b05be17 as the
first bad commit. Bisected it on the tag "next-20240326" at repo
"https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git".
I understand that you are turning on the warning here, thought worth
mentioning about the observation.
Build log:
---------------
In file included from ../include/drm/drm_mm.h:51,
from ../include/drm/drm_vma_manager.h:26,
from ../include/drm/drm_gem.h:42,
from ../drivers/gpu/drm/msm/msm_drv.h:34,
from ../drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:20:
In function '_dpu_plane_set_qos_lut',
inlined from 'dpu_plane_sspp_update_pipe' at
../drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:1078:2:
../include/drm/drm_print.h:536:35: warning: '%4.4s' directive argument
is null [-Wformat-overflow=]
536 | #define __drm_dbg(cat, fmt, ...) ___drm_dbg(NULL, cat, fmt,
##__VA_ARGS__)
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/drm/drm_print.h:594:2: note: in expansion of macro '__drm_dbg'
594 | __drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
| ^~~~~~~~~
../drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:30:39: note: in expansion
of macro 'DRM_DEBUG_ATOMIC'
30 | #define DPU_DEBUG_PLANE(pl, fmt, ...) DRM_DEBUG_ATOMIC("plane%d
" fmt,\
| ^~~~~~~~~~~~~~~~
../drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:290:2: note: in expansion
of macro 'DPU_DEBUG_PLANE'
290 | DPU_DEBUG_PLANE(pdpu, "pnum:%d fmt: %4.4s rt:%d fl:%u
lut:0x%llx\n",
| ^~~~~~~~~~~~~~~
CC [M] drivers/net/can/spi/mcp251xfd/mcp251xfd-ethtool.o
Bisect log:
------------
git bisect start
# good: [4cece764965020c22cff7665b18a012006359095] Linux 6.9-rc1
git bisect good 4cece764965020c22cff7665b18a012006359095
# bad: [084c8e315db34b59d38d06e684b1a0dd07d30287] Add linux-next
specific files for 20240326
git bisect bad 084c8e315db34b59d38d06e684b1a0dd07d30287
# good: [1cc931629f2b3de04b7608b8d26692c6f6a52aeb] Merge branch
'nand/next' of git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
git bisect good 1cc931629f2b3de04b7608b8d26692c6f6a52aeb
# bad: [4f5a3415aaf8fdf945e4cb67b847254ddda2f583] Merge branch
'drm-xe-next' of https://gitlab.freedesktop.org/drm/xe/kernel
git bisect bad 4f5a3415aaf8fdf945e4cb67b847254ddda2f583
# bad: [a13305486485d0657fbf09cee72fca9553d7d6cd] Merge branch
'drm-next' of https://gitlab.freedesktop.org/agd5f/linux
git bisect bad a13305486485d0657fbf09cee72fca9553d7d6cd
# good: [417f78a2a1c8c2d517db8b2e04785c1c94a563b4] drm/amdkfd: Cleanup
workqueue during module unload
git bisect good 417f78a2a1c8c2d517db8b2e04785c1c94a563b4
# bad: [57a4e3a94caee6cfda41700da877bee77eab939c] Revert "drm/panthor:
Fix undefined panthor_device_suspend/resume symbol issue"
git bisect bad 57a4e3a94caee6cfda41700da877bee77eab939c
# bad: [2cddf770be0cebb663af3d72c049b9e24928f335] drm/kunit: fix
drm_kunit_helpers.h kernel-doc
git bisect bad 2cddf770be0cebb663af3d72c049b9e24928f335
# good: [d72f049087d4f973f6332b599c92177e718107de] drm/panthor: Allow
driver compilation
git bisect good d72f049087d4f973f6332b599c92177e718107de
# good: [e18aeeda0b6905c333df5a0566b99f5c84426098] drm/bridge: Fix
improper bridge init order with pre_enable_prev_first
git bisect good e18aeeda0b6905c333df5a0566b99f5c84426098
# bad: [f89632a9e5fa6c4787c14458cd42a9ef42025434] drm: Add CONFIG_DRM_WERROR
git bisect bad f89632a9e5fa6c4787c14458cd42a9ef42025434
# good: [460be1d527a8e296d85301e8b14923299508d4fc] drm/nouveau: move
more missing UAPI bits
git bisect good 460be1d527a8e296d85301e8b14923299508d4fc
# bad: [a61ddb4393ad1be61d2ffd92576d42707b05be17] drm: enable (most) W=1
warnings by default across the subsystem
git bisect bad a61ddb4393ad1be61d2ffd92576d42707b05be17
# first bad commit: [a61ddb4393ad1be61d2ffd92576d42707b05be17] drm:
enable (most) W=1 warnings by default across the subsystem
Thanks,
Aishwarya
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem
2024-04-04 15:16 ` [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Aishwarya TCV
@ 2024-04-05 9:57 ` Jani Nikula
2024-04-05 10:28 ` Aishwarya TCV
0 siblings, 1 reply; 26+ messages in thread
From: Jani Nikula @ 2024-04-05 9:57 UTC (permalink / raw)
To: Aishwarya TCV; +Cc: intel-gfx, Mark Brown, dri-devel
On Thu, 04 Apr 2024, Aishwarya TCV <aishwarya.tcv@arm.com> wrote:
> Observed warning "include/drm/drm_print.h:536:35: warning: '%4.4s'
> directive argument is null [-Wformat-overflow=]" when building the
> modules with "defconfig+kselftest-ftrace"(
> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/ftrace/config
> ) against next-master(next-20240404) kernel with Arm64 in our CI.
>
> A bisect identified a61ddb4393ad1be61d2ffd92576d42707b05be17 as the
> first bad commit. Bisected it on the tag "next-20240326" at repo
> "https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git".
>
> I understand that you are turning on the warning here, thought worth
> mentioning about the observation.
Thanks for the report. I can't reproduce this myself, but please see if
[1] fixes the issue.
BR,
Jani.
[1] https://lore.kernel.org/dri-devel/20240405092907.2334007-1-jani.nikula@intel.com
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem
2024-04-05 9:57 ` Jani Nikula
@ 2024-04-05 10:28 ` Aishwarya TCV
0 siblings, 0 replies; 26+ messages in thread
From: Aishwarya TCV @ 2024-04-05 10:28 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Mark Brown, dri-devel
On 05/04/2024 10:57, Jani Nikula wrote:
> On Thu, 04 Apr 2024, Aishwarya TCV <aishwarya.tcv@arm.com> wrote:
>> Observed warning "include/drm/drm_print.h:536:35: warning: '%4.4s'
>> directive argument is null [-Wformat-overflow=]" when building the
>> modules with "defconfig+kselftest-ftrace"(
>> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/ftrace/config
>> ) against next-master(next-20240404) kernel with Arm64 in our CI.
>>
>> A bisect identified a61ddb4393ad1be61d2ffd92576d42707b05be17 as the
>> first bad commit. Bisected it on the tag "next-20240326" at repo
>> "https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git".
>>
>> I understand that you are turning on the warning here, thought worth
>> mentioning about the observation.
>
> Thanks for the report. I can't reproduce this myself, but please see if
> [1] fixes the issue.
>
>
> BR,
> Jani.
>
> [1] https://lore.kernel.org/dri-devel/20240405092907.2334007-1-jani.nikula@intel.com
>
>
Thanks, as expected this fixes the issue. Tested the attached patch by
building the modules with "defconfig+kselftest-ftrace" against
next-20240405 kernel with Arm64.
Tested-by: Aishwarya TCV <aishwarya.tcv@arm.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2024-04-05 10:28 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-10 17:39 [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Jani Nikula
2024-01-10 17:39 ` [PATCH 1/6] drm/nouveau/acr/ga102: remove unused but set variable Jani Nikula
2024-01-31 12:50 ` Jani Nikula
2024-01-31 19:01 ` Danilo Krummrich
2024-02-01 9:41 ` Jani Nikula
2024-01-10 17:39 ` [PATCH 2/6] drm/nouveau/svm: remove unused but set variables Jani Nikula
2024-01-31 12:51 ` Jani Nikula
2024-01-31 19:10 ` Danilo Krummrich
2024-02-01 9:42 ` Jani Nikula
2024-01-10 17:39 ` [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf Jani Nikula
2024-01-12 4:09 ` kernel test robot
2024-01-12 9:07 ` Jani Nikula
2024-01-12 14:57 ` Alex Deucher
2024-01-31 12:48 ` Jani Nikula
2024-01-10 17:39 ` [PATCH 4/6] drm/imx: " Jani Nikula
2024-01-12 10:49 ` kernel test robot
2024-01-12 14:20 ` Philipp Zabel
2024-01-31 12:49 ` Jani Nikula
2024-01-10 17:39 ` [PATCH 5/6] drm: enable (most) W=1 warnings by default across the subsystem Jani Nikula
2024-01-10 20:15 ` Hamza Mahfooz
2024-01-11 9:43 ` Jani Nikula
2024-01-10 17:39 ` [PATCH 6/6] drm: Add CONFIG_DRM_WERROR Jani Nikula
2024-01-10 20:17 ` Hamza Mahfooz
2024-04-04 15:16 ` [PATCH 0/6] drm: enable W=1 warnings by default across the subsystem Aishwarya TCV
2024-04-05 9:57 ` Jani Nikula
2024-04-05 10:28 ` Aishwarya TCV
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).