dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [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).