Re: [PATCH] drm/msm/gen_header: allow skipping the validation
Hi Dmitry, On Tue, Apr 09, 2024 at 05:22:54PM +0300, Dmitry Baryshkov wrote: > We don't need to run the validation of the XML files if we are just > compiling the kernel. Skip the validation unless the user enables > corresponding Kconfig option. This removes a warning from gen_header.py > about lxml being not installed. > > Reported-by: Stephen Rothwell > Closes: https://lore.kernel.org/all/20240409120108.2303d...@canb.auug.org.au/ > Signed-off-by: Dmitry Baryshkov > --- > drivers/gpu/drm/msm/Kconfig | 8 > drivers/gpu/drm/msm/Makefile| 9 - > drivers/gpu/drm/msm/registers/gen_header.py | 14 +++--- > 3 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > index f202f26adab2..4c9bf237d4a2 100644 > --- a/drivers/gpu/drm/msm/Kconfig > +++ b/drivers/gpu/drm/msm/Kconfig > @@ -54,6 +54,14 @@ config DRM_MSM_GPU_SUDO > Only use this if you are a driver developer. This should *not* > be enabled for production kernels. If unsure, say N. > > +config DRM_MSM_VALIDATE_XML > + bool "Validate XML register files against schema" > + depends on DRM_MSM && EXPERT > + depends on $(success,$(PYTHON3) -c "import lxml") > + help > + Validate XML files with register definitions against rules-fd schema. > + This option is mostly targeting DRM MSM developers. If unsure, say N. Is this change going to be applied? I have gotten a little tired of seeing "lxml not found, skipping validation" in all of my builds :) Cheers, Nathan
Re: [PATCH v3 0/2] Fix Kernel CI issues
Hi Tomi, On Sat, Apr 27, 2024 at 10:48:16AM +0300, Tomi Valkeinen wrote: > On 26/04/2024 22:27, Anatoliy Klymenko wrote: > > Fix number of CI reported W=1 build issues. > > > > Patch 1/2: Fix function arguments description. > > Closes: > > https://lore.kernel.org/oe-kbuild-all/202404260616.kfgdpcdn-...@intel.com/ > > > > Patch 2/2: Fix clang compilation error. > > Closes: > > https://lore.kernel.org/oe-kbuild-all/202404260946.4ozxvhd2-...@intel.com/ > > > > Signed-off-by: Anatoliy Klymenko > > --- > > Changes in v3: > > - Add Signed-off-by tag. > > > > - Link to v2: > > https://lore.kernel.org/r/20240425-dp-live-fmt-fix-v2-0-6048e8121...@amd.com > > > > Changes in v2: > > - Compilation error fix added. > > > > - Link to v1: > > https://lore.kernel.org/r/20240425-dp-live-fmt-fix-v1-1-405f352d3...@amd.com > > > > --- > > Anatoliy Klymenko (2): > >drm: xlnx: zynqmp_dpsub: Fix few function comments > >drm: xlnx: zynqmp_dpsub: Fix compilation error > > > > drivers/gpu/drm/xlnx/zynqmp_disp.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > --- > > base-commit: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f > > change-id: 20240425-dp-live-fmt-fix-a10bf7973596 > > > > Best regards, > > Thanks, pushed to drm-misc-next. I think the second patch also needs to go to drm-misc-next-fixes? The clang warning fixed by it has returned in next-20240503 because it appears that for-linux-next was switch from drm-misc-next to drm-misc-next-fixes, as I see for-linux-next was pointing to commit 235e60653f8d ("drm/debugfs: Drop conditionals around of_node pointers") on drm-misc-next in next-20240502 but it is now pointing to commit be3f3042391d ("drm: zynqmp_dpsub: Always register bridge") on drm-misc-next-fixes in next-20240503. Cheers, Nathan
[PATCH 2/2] drm/amd/display: Fix CFLAGS for dml2_core_dcn4_calcs.o
-Wframe-larger-than=2048 is a part of both CFLAGS and CFLAGS_REMOVE for dml2_core_dcn4_calcs.o, which means that it ultimately gets removed altogether for 64-bit targets, as 2048 is the default FRAME_WARN value for 64-bit platforms, resulting in no -Wframe-larger-than coverage for this file. Remove -Wframe-larger-than from CFLAGS_REMOVE_dml2_core_dcn4_calcs.o and move to $(frame_warn_flag) for CFLAGS_dml2_core_dcn4_calcs.o, as that accounts for the fact that -Wframe-larger-than may need to be larger than 2048 in certain situations, such as when the sanitizers are enabled. Fixes: d546a39c6b10 ("drm/amd/display: Add misc DC changes for DCN401") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/display/dc/dml2/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile index c35212a4a968..904a2d419638 100644 --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile @@ -111,7 +111,7 @@ CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml_top.o := $(dml2_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml_top_mcache.o := $(dml2_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml2_top_optimization := $(dml2_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4.o := $(dml2_ccflags) -CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.o := $(dml2_ccflags) -Wframe-larger-than=2048 +CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.o := $(dml2_ccflags) $(frame_warn_flag) CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_factory.o := $(dml2_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_shared.o := $(dml2_ccflags) $(frame_warn_flag) CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.o := $(dml2_ccflags) @@ -134,7 +134,7 @@ CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml_top.o := $(dml2_rcfla CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml_top_mcache.o := $(dml2_rcflags) CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml2_top_optimization.o := $(dml2_rcflags) CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4.o := $(dml2_rcflags) -CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.o := $(dml2_rcflags) -Wframe-larger-than=2048 +CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.o := $(dml2_rcflags) CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_factory.o := $(dml2_rcflags) CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_shared.o := $(dml2_rcflags) CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.o := $(dml2_rcflags) -- 2.44.0
[PATCH 1/2] drm/amd/display: Add frame_warn_flag to dml2_core_shared.o
When building with tip of tree Clang, there are some new instances of -Wframe-larger-than from the new display code (which become fatal with CONFIG_WERROR=y): drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml21/src/dml2_core/dml2_core_shared.c:754:6: error: stack frame size (2488) exceeds limit (2048) in 'dml2_core_shared_mode_support' [-Werror,-Wframe-larger-than] 754 | bool dml2_core_shared_mode_support(struct dml2_core_calcs_mode_support_ex *in_out_params) | ^ drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml21/src/dml2_core/dml2_core_shared.c:9834:6: error: stack frame size (2152) exceeds limit (2048) in 'dml2_core_shared_mode_programming' [-Werror,-Wframe-larger-than] 9834 | bool dml2_core_shared_mode_programming(struct dml2_core_calcs_mode_programming_ex *in_out_params) | ^ 2 errors generated. These warnings do not occur when CONFIG_K{A,C,M}SAN are disabled, so add $(frame_warn_flag) to dml2_core_shared.o's CFLAGS, which was added in commit 6740ec97bcdb ("drm/amd/display: Increase frame warning limit with KASAN or KCSAN in dml2") to account for this situation. Fixes: d546a39c6b10 ("drm/amd/display: Add misc DC changes for DCN401") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/display/dc/dml2/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile index 6c76f346b237..c35212a4a968 100644 --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile @@ -113,7 +113,7 @@ CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_top/dml2_top_optimization := $(dml2_ CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4.o := $(dml2_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.o := $(dml2_ccflags) -Wframe-larger-than=2048 CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_factory.o := $(dml2_ccflags) -CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_shared.o := $(dml2_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_core/dml2_core_shared.o := $(dml2_ccflags) $(frame_warn_flag) CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.o := $(dml2_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_factory.o := $(dml2_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml2/dml21/src/dml2_mcg/dml2_mcg_dcn4.o := $(dml2_ccflags) -- 2.44.0
[PATCH 0/2] drm/amd/display: Use frame_warn_flag consistently in dml2 Makefile
Hi all, This series resolves a couple instances of -Wframe-larger-than from the new display code that appear with newer versions of clang along without another inconsistency I noticed while fixing this, which have been accounted for with the $(frame_warn_flag) variable. --- Nathan Chancellor (2): drm/amd/display: Add frame_warn_flag to dml2_core_shared.o drm/amd/display: Fix CFLAGS for dml2_core_dcn4_calcs.o drivers/gpu/drm/amd/display/dc/dml2/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) --- base-commit: d60dc4dd72412d5d9566fdf391e4202b05f88912 change-id: 20240424-amdgpu-dml2-fix-frame-larger-than-dcn401-48ff7e1f51ea Best regards, -- Nathan Chancellor
[PATCH] drm/amd/display: Avoid -Wenum-float-conversion in add_margin_and_round_to_dfs_grainularity()
When building with clang 19 or newer (which strengthened some of the enum conversion warnings for C), there is a warning (or error with CONFIG_WERROR=y) around doing arithmetic with an enumerated type and a floating point expression. drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c:181:58: error: arithmetic between enumeration type 'enum dentist_divider_range' and floating-point type 'double' [-Werror,-Wenum-float-conversion] 181 | divider = (unsigned int)(DFS_DIVIDER_RANGE_SCALE_FACTOR * (vco_freq_khz / clock_khz)); | ~~ ^ ~~ 1 error generated. This conversion is expected due to the nature of the enumerated value and definition, so silence the warning by casting the enumeration to an integer explicitly to make it clear to the compiler. Fixes: 3df48ddedee4 ("drm/amd/display: Add new DCN401 sources") Signed-off-by: Nathan Chancellor --- Alternatively, perhaps the potential truncation could happen before the multiplication? divider = DFS_DIVIDER_RANGE_SCALE_FACTOR * (unsigned int)(vco_freq_khz / clock_khz); I suspect the result of the division is probably not very large (certainly not within UINT_MAX / 4), so I would not expect the multiplication to overflow, but I was not sure so I decided to take the safer, NFC change. I am happy to respin as necessary. --- .../gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c| 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c b/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c index e6698ee65843..65eb0187e965 100644 --- a/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c +++ b/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_dpmm/dml2_dpmm_dcn4.c @@ -178,7 +178,7 @@ static bool add_margin_and_round_to_dfs_grainularity(double clock_khz, double ma clock_khz *= 1.0 + margin; - divider = (unsigned int)(DFS_DIVIDER_RANGE_SCALE_FACTOR * (vco_freq_khz / clock_khz)); + divider = (unsigned int)((int)DFS_DIVIDER_RANGE_SCALE_FACTOR * (vco_freq_khz / clock_khz)); /* we want to floor here to get higher clock than required rather than lower */ if (divider < DFS_DIVIDER_RANGE_2_START) { --- base-commit: d60dc4dd72412d5d9566fdf391e4202b05f88912 change-id: 20240424-amdgpu-display-dcn401-enum-float-conversion-c09cc1826ea2 Best regards, -- Nathan Chancellor
[PATCH] drm/xe: Add xe_guc_ads.c to uses_generated_oob
A recent change added a use of xe_wa_oob.h without adding the file that uses it to uses_generated_oob, which means xe_wa_oob.h does not get properly generated before attempting to build the object file: LINK resolve_btfids CC [M] drivers/gpu/drm/xe/xe_guc_ads.o drivers/gpu/drm/xe/xe_guc_ads.c:10:10: fatal error: generated/xe_wa_oob.h: No such file or directory 10 | #include | ^~~ After adding '$(obj)/xe_guc_ads.o' to uses_generated_oob, xe_wa_oob.h is always generated before building the file, resulting in no errors: LINK resolve_btfids HOSTCC drivers/gpu/drm/xe/xe_gen_wa_oob GEN xe_wa_oob.c xe_wa_oob.h CC [M] drivers/gpu/drm/xe/xe_guc_ads.o Fixes: c151ff5c9053 ("drm/xe/lnl: Enable GuC Wa_14019882105") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/xe/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile index e106767c9a6e..60c90dc918b2 100644 --- a/drivers/gpu/drm/xe/Makefile +++ b/drivers/gpu/drm/xe/Makefile @@ -49,6 +49,7 @@ $(obj)/generated/%_wa_oob.c $(obj)/generated/%_wa_oob.h: $(obj)/xe_gen_wa_oob \ uses_generated_oob := \ $(obj)/xe_gsc.o \ $(obj)/xe_guc.o \ + $(obj)/xe_guc_ads.o \ $(obj)/xe_migrate.o \ $(obj)/xe_ring_ops.o \ $(obj)/xe_vm.o \ --- base-commit: 9c1857d587e91dfc10875a8c1083360db047404f change-id: 20240410-drm-xe-fix-xe_guc_ads-using-xe_wa_oob-9cb394101ad8 Best regards, -- Nathan Chancellor
[PATCH] drm/panthor: Fix clang -Wunused-but-set-variable in tick_ctx_apply()
Clang warns (or errors with CONFIG_WERROR): drivers/gpu/drm/panthor/panthor_sched.c:2048:6: error: variable 'csg_mod_mask' set but not used [-Werror,-Wunused-but-set-variable] 2048 | u32 csg_mod_mask = 0, free_csg_slots = 0; | ^ 1 error generated. The variable is an artifact left over from refactoring that occurred during the development of the initial series for this driver. Remove it to resolve the warning. Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/panthor/panthor_sched.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c index 5f7803b6fc48..e5a710f190d2 100644 --- a/drivers/gpu/drm/panthor/panthor_sched.c +++ b/drivers/gpu/drm/panthor/panthor_sched.c @@ -2045,7 +2045,7 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c struct panthor_device *ptdev = sched->ptdev; struct panthor_csg_slot *csg_slot; int prio, new_csg_prio = MAX_CSG_PRIO, i; - u32 csg_mod_mask = 0, free_csg_slots = 0; + u32 free_csg_slots = 0; struct panthor_csg_slots_upd_ctx upd_ctx; int ret; @@ -2139,7 +2139,6 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id); csg_slot = >csg_slots[csg_id]; - csg_mod_mask |= BIT(csg_id); group_bind_locked(group, csg_id); csg_slot_prog_locked(ptdev, csg_id, new_csg_prio--); csgs_upd_ctx_queue_reqs(ptdev, _ctx, csg_id, --- base-commit: d180649238f04183950d9c8a7d8a2c2f1788a89c change-id: 20240328-panthor-drop-csg_mod_mask-b4bbe317d690 Best regards, -- Nathan Chancellor
Re: [PATCH v6 10/14] drm/panthor: Add the scheduler logical block
Hi Boris, On Thu, Feb 29, 2024 at 05:22:24PM +0100, Boris Brezillon wrote: > --- > drivers/gpu/drm/panthor/panthor_sched.c | 3502 +++ > drivers/gpu/drm/panthor/panthor_sched.h | 50 + > 2 files changed, 3552 insertions(+) > create mode 100644 drivers/gpu/drm/panthor/panthor_sched.c > create mode 100644 drivers/gpu/drm/panthor/panthor_sched.h > > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c > b/drivers/gpu/drm/panthor/panthor_sched.c > new file mode 100644 > index ..5f7803b6fc48 > --- /dev/null > +++ b/drivers/gpu/drm/panthor/panthor_sched.c > +static void > +tick_ctx_apply(struct panthor_scheduler *sched, struct > panthor_sched_tick_ctx *ctx) > +{ > + struct panthor_group *group, *tmp; > + struct panthor_device *ptdev = sched->ptdev; > + struct panthor_csg_slot *csg_slot; > + int prio, new_csg_prio = MAX_CSG_PRIO, i; > + u32 csg_mod_mask = 0, free_csg_slots = 0; > + struct panthor_csg_slots_upd_ctx upd_ctx; > + int ret; > + > + csgs_upd_ctx_init(_ctx); > + > + for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) { > + /* Suspend or terminate evicted groups. */ > + list_for_each_entry(group, >old_groups[prio], run_node) { > + bool term = !group_can_run(group); > + int csg_id = group->csg_id; > + > + if (drm_WARN_ON(>base, csg_id < 0)) > + continue; > + > + csg_slot = >csg_slots[csg_id]; > + csgs_upd_ctx_queue_reqs(ptdev, _ctx, csg_id, > + term ? CSG_STATE_TERMINATE : > CSG_STATE_SUSPEND, > + CSG_STATE_MASK); > + } > + > + /* Update priorities on already running groups. */ > + list_for_each_entry(group, >groups[prio], run_node) { > + struct panthor_fw_csg_iface *csg_iface; > + int csg_id = group->csg_id; > + > + if (csg_id < 0) { > + new_csg_prio--; > + continue; > + } > + > + csg_slot = >csg_slots[csg_id]; > + csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id); > + if (csg_slot->priority == new_csg_prio) { > + new_csg_prio--; > + continue; > + } > + > + panthor_fw_update_reqs(csg_iface, endpoint_req, > + > CSG_EP_REQ_PRIORITY(new_csg_prio), > +CSG_EP_REQ_PRIORITY_MASK); > + csgs_upd_ctx_queue_reqs(ptdev, _ctx, csg_id, > + csg_iface->output->ack ^ > CSG_ENDPOINT_CONFIG, > + CSG_ENDPOINT_CONFIG); > + new_csg_prio--; > + } > + } > + > + ret = csgs_upd_ctx_apply_locked(ptdev, _ctx); > + if (ret) { > + panthor_device_schedule_reset(ptdev); > + ctx->csg_upd_failed_mask |= upd_ctx.timedout_mask; > + return; > + } > + > + /* Unbind evicted groups. */ > + for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) { > + list_for_each_entry(group, >old_groups[prio], run_node) { > + /* This group is gone. Process interrupts to clear > + * any pending interrupts before we start the new > + * group. > + */ > + if (group->csg_id >= 0) > + sched_process_csg_irq_locked(ptdev, > group->csg_id); > + > + group_unbind_locked(group); > + } > + } > + > + for (i = 0; i < sched->csg_slot_count; i++) { > + if (!sched->csg_slots[i].group) > + free_csg_slots |= BIT(i); > + } > + > + csgs_upd_ctx_init(_ctx); > + new_csg_prio = MAX_CSG_PRIO; > + > + /* Start new groups. */ > + for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) { > + list_for_each_entry(group, >groups[prio], run_node) { > + int csg_id = group->csg_id; > + struct panthor_fw_csg_iface *csg_iface; > + > + if (csg_id >= 0) { > + new_csg_prio--; > + continue; > + } > + > + csg_id = ffs(free_csg_slots) - 1; > + if (drm_WARN_ON(>base, csg_id < 0)) > + break; > + > + csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id); > + csg_slot = >csg_slots[csg_id]; > + csg_mod_mask |= BIT(csg_id); > +
Re: [RESEND v3 2/2] drm: Add CONFIG_DRM_WERROR
On Wed, Mar 27, 2024 at 09:59:01AM +0200, Jani Nikula wrote: > On Wed, 27 Mar 2024, Maxime Ripard wrote: > > On Tue, Mar 26, 2024 at 03:56:50PM -0700, Nathan Chancellor wrote: > >> On Tue, Mar 05, 2024 at 11:07:36AM +0200, 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. > >> > > >> > v2: Don't depend on COMPILE_TEST > >> > > >> > Reviewed-by: Hamza Mahfooz # v1 > >> > Signed-off-by: Jani Nikula > >> > --- > >> > drivers/gpu/drm/Kconfig | 13 + > >> > drivers/gpu/drm/Makefile | 3 +++ > >> > 2 files changed, 16 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > >> > index 6e853acf15da..c08e18108c2a 100644 > >> > --- a/drivers/gpu/drm/Kconfig > >> > +++ b/drivers/gpu/drm/Kconfig > >> > @@ -416,3 +416,16 @@ config DRM_LIB_RANDOM > >> > config DRM_PRIVACY_SCREEN > >> > bool > >> > default n > >> > + > >> > +config DRM_WERROR > >> > +bool "Compile the drm subsystem with warnings as errors" > >> > +depends on EXPERT > >> > +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. > >> > >> While I understand the desire for an easy switch that maintainers and > >> developers can use to ensure that their changes are warning free for the > >> drm subsystem specifically, I think subsystem specific configuration > >> options like this are actively detrimental to developers and continuous > >> integration systems that build test the entire kernel. For example, we > >> turned off CONFIG_WERROR for our Hexagon builds because of warnings that > >> appear with -Wextra that are legitimate but require treewide changes to > >> resolve in a manner sufficient for Linus: > >> > >> https://github.com/ClangBuiltLinux/linux/issues/1285 > >> https://lore.kernel.org/all/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/ > >> https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/ > >> > >> But now, due to CONFIG_DRM_WERROR getting enabled by all{mod,yes}config > >> and -Wextra being unconditionally enabled for DRM, those warnings hard > >> break the build despite CONFIG_WERROR=n... > > > > Would making DRM_WERROR depends on WERROR address your concerns? > > But then what would be the point of having DRM_WERROR at all? For me the > point is, "werror in drm, ignore the rest, they're someone else's > problem". Right, I do think this is a valid view point and one I am sympathetic to, especially since it is in the pursuit of increased code quality. I do not want to disrupt that. > An alternative would be to "depends on !COMPILE_TEST" that we have in > i915, but then some folks want to have COMPILE_TEST in drm, because some > drivers are otherwise hard for people to build. Right. I think it is unfortunate how (at least in my opinion) CONFIG_COMPILE_TEST has two meanings: genuinely just compile testing or "allmodconfig". For the first case, we would want CONFIG_DRM_WERROR=y but for the second case, it would be nice for CONFIG_DRM_WERROR to default to off (because CONFIG_WERROR is enabled) but allow developers to turn it on explicitly. Another lofty/wistful idea to solve this would be to implement something similar to compiler diagnostic groups for Kconfig, where there would be a hierarchy like CONFIG_WERROR CONFIG_DRM_WERROR CONFIG_SUBSYSTEM_A_WERROR CONFIG_SUBSYSTEM_B_WERROR where the value of CONFIG_WERROR is the same value for all subconfigurations under it but they could still be enabled individually without any additional dependencies (ala something like '-Wno-unused -Wunused-variable'), which would allow my use case of CONFIG_WERROR=n removing all instances of -Werror to continue to work but allow other
Re: [RESEND v3 2/2] drm: Add CONFIG_DRM_WERROR
On Tue, Mar 05, 2024 at 11:07:36AM +0200, 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. > > v2: Don't depend on COMPILE_TEST > > Reviewed-by: Hamza Mahfooz # v1 > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/Kconfig | 13 + > drivers/gpu/drm/Makefile | 3 +++ > 2 files changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 6e853acf15da..c08e18108c2a 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -416,3 +416,16 @@ config DRM_LIB_RANDOM > config DRM_PRIVACY_SCREEN > bool > default n > + > +config DRM_WERROR > + bool "Compile the drm subsystem with warnings as errors" > + depends on EXPERT > + 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. While I understand the desire for an easy switch that maintainers and developers can use to ensure that their changes are warning free for the drm subsystem specifically, I think subsystem specific configuration options like this are actively detrimental to developers and continuous integration systems that build test the entire kernel. For example, we turned off CONFIG_WERROR for our Hexagon builds because of warnings that appear with -Wextra that are legitimate but require treewide changes to resolve in a manner sufficient for Linus: https://github.com/ClangBuiltLinux/linux/issues/1285 https://lore.kernel.org/all/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/ https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/ But now, due to CONFIG_DRM_WERROR getting enabled by all{mod,yes}config and -Wextra being unconditionally enabled for DRM, those warnings hard break the build despite CONFIG_WERROR=n... https://storage.tuxsuite.com/public/clangbuiltlinux/continuous-integration2/builds/2eEBDGEqfmMZjGg3ZvDx2af2pde/build.log Same thing with PowerPC allmodconfig because we see -Wframe-larger-than that appears because allmodconfig enables CONFIG_KASAN or CONFIG_KCSAN usually: https://storage.tuxsuite.com/public/clangbuiltlinux/continuous-integration2/builds/2eE2HDsODudQGqkMKAPQnId7pRd/build.log I don't know what the solution for this conflict is through. I guess it is just the nature of the kernel being a federation of independent subsystems that want to have their own policies. I suppose we can just set CONFIG_DRM_WERROR=n and be done with it but I would like to avoid this issue from spreading to other subsystems because it does not scale for folks like us who do many builds across many trees. It would be nice if there was something like CONFIG_WERROR_DIRS or something that could take a set of directories that should have -Werror enabled so that you could do something like CONFIG_WERROR_DIRS="drivers/gpu/drm" and have -Werror automatically added to all commands within that directory like subdir-ccflags-y but it is explicitly opt in on the part of the developer/tester, rather than just happening to get enabled due to all{mod,yes}config. No idea if that is feasible or not though. > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index ea456f057e8a..a73c04d2d7a3 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -30,6 +30,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 >
Re: [PATCH V8 08/12] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface
On Tue, Feb 06, 2024 at 12:50:16PM -0600, Adam Ford wrote: > On Tue, Feb 6, 2024 at 11:06 AM Nathan Chancellor wrote: > > > > Hi all, > > > > On Sat, Feb 03, 2024 at 10:52:48AM -0600, Adam Ford wrote: > > > From: Lucas Stach > > > > > > This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a > > > full timing generator and can switch between different video sources. On > > > the i.MX8MP however the only supported source is the LCDIF. The block > > > just needs to be powered up and told about the polarity of the video > > > sync signals to act in bypass mode. > > > > > > Signed-off-by: Lucas Stach > > > Reviewed-by: Luca Ceresoli (v7) > > > Tested-by: Marek Vasut (v1) > > > Tested-by: Luca Ceresoli (v7) > > > Tested-by: Richard Leitner (v2) > > > Tested-by: Frieder Schrempf (v2) > > > Reviewed-by: Laurent Pinchart (v3) > > > Reviewed-by: Luca Ceresoli > > > Tested-by: Luca Ceresoli > > > Tested-by: Fabio Estevam > > > Signed-off-by: Adam Ford > > > > > > > > > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > > > b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > > > new file mode 100644 > > > index ..a76b7669fe8a > > > --- /dev/null > > > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > > ... > > > +static void imx8mp_hdmi_pvi_bridge_enable(struct drm_bridge *bridge, > > > + struct drm_bridge_state > > > *bridge_state) > > > +{ > > > + struct drm_atomic_state *state = bridge_state->base.state; > > > + struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge); > > > + struct drm_connector_state *conn_state; > > > + const struct drm_display_mode *mode; > > > + struct drm_crtc_state *crtc_state; > > > + struct drm_connector *connector; > > > + u32 bus_flags, val; > > > + > > > + connector = drm_atomic_get_new_connector_for_encoder(state, > > > bridge->encoder); > > > + conn_state = drm_atomic_get_new_connector_state(state, connector); > > > + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); > > > + > > > + if (WARN_ON(pm_runtime_resume_and_get(pvi->dev))) > > > + return; > > > + > > > + mode = _state->adjusted_mode; > > > + > > > + val = FIELD_PREP(PVI_CTRL_MODE_MASK, PVI_CTRL_MODE_LCDIF) | > > > PVI_CTRL_EN; > > > + > > > + if (mode->flags & DRM_MODE_FLAG_PVSYNC) > > > + val |= PVI_CTRL_OP_VSYNC_POL | PVI_CTRL_INP_VSYNC_POL; > > > + > > > + if (mode->flags & DRM_MODE_FLAG_PHSYNC) > > > + val |= PVI_CTRL_OP_HSYNC_POL | PVI_CTRL_INP_HSYNC_POL; > > > + > > > + if (pvi->next_bridge->timings) > > > + bus_flags = pvi->next_bridge->timings->input_bus_flags; > > > + else if (bridge_state) > > > + bus_flags = bridge_state->input_bus_cfg.flags; > > > + > > > + if (bus_flags & DRM_BUS_FLAG_DE_HIGH) > > > + val |= PVI_CTRL_OP_DE_POL | PVI_CTRL_INP_DE_POL; > > > + > > > + writel(val, pvi->regs + HTX_PVI_CTRL); > > > +} > > > > Apologies if this has already been reported or fixed, I searched lore > > and did not find anything. Clang warns (or errors with CONFIG_WERROR=y) > > for this function: > > > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:81:11: error: variable > > 'bus_flags' is used uninitialized whenever 'if' condition is false > > [-Werror,-Wsometimes-uninitialized] > > 81 | else if (bridge_state) > > | ^~~~ > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:84:6: note: uninitialized > > use occurs here > > 84 | if (bus_flags & DRM_BUS_FLAG_DE_HIGH) > > | ^ > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:81:7: note: remove the 'if' > > if its condition is always true > > 81 | else if (bridge_state) > > | ^ > > 82 | bus_flags = bridge_state->input_bus_cfg.flags; > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:60:15: note: initialize the > > variable 'bus_flags' to silence this warning > > 60 | u32 bus_flags, val; > > |
Re: [PATCH V8 08/12] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface
Hi all, On Sat, Feb 03, 2024 at 10:52:48AM -0600, Adam Ford wrote: > From: Lucas Stach > > This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a > full timing generator and can switch between different video sources. On > the i.MX8MP however the only supported source is the LCDIF. The block > just needs to be powered up and told about the polarity of the video > sync signals to act in bypass mode. > > Signed-off-by: Lucas Stach > Reviewed-by: Luca Ceresoli (v7) > Tested-by: Marek Vasut (v1) > Tested-by: Luca Ceresoli (v7) > Tested-by: Richard Leitner (v2) > Tested-by: Frieder Schrempf (v2) > Reviewed-by: Laurent Pinchart (v3) > Reviewed-by: Luca Ceresoli > Tested-by: Luca Ceresoli > Tested-by: Fabio Estevam > Signed-off-by: Adam Ford > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > new file mode 100644 > index ..a76b7669fe8a > --- /dev/null > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c ... > +static void imx8mp_hdmi_pvi_bridge_enable(struct drm_bridge *bridge, > + struct drm_bridge_state *bridge_state) > +{ > + struct drm_atomic_state *state = bridge_state->base.state; > + struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge); > + struct drm_connector_state *conn_state; > + const struct drm_display_mode *mode; > + struct drm_crtc_state *crtc_state; > + struct drm_connector *connector; > + u32 bus_flags, val; > + > + connector = drm_atomic_get_new_connector_for_encoder(state, > bridge->encoder); > + conn_state = drm_atomic_get_new_connector_state(state, connector); > + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); > + > + if (WARN_ON(pm_runtime_resume_and_get(pvi->dev))) > + return; > + > + mode = _state->adjusted_mode; > + > + val = FIELD_PREP(PVI_CTRL_MODE_MASK, PVI_CTRL_MODE_LCDIF) | PVI_CTRL_EN; > + > + if (mode->flags & DRM_MODE_FLAG_PVSYNC) > + val |= PVI_CTRL_OP_VSYNC_POL | PVI_CTRL_INP_VSYNC_POL; > + > + if (mode->flags & DRM_MODE_FLAG_PHSYNC) > + val |= PVI_CTRL_OP_HSYNC_POL | PVI_CTRL_INP_HSYNC_POL; > + > + if (pvi->next_bridge->timings) > + bus_flags = pvi->next_bridge->timings->input_bus_flags; > + else if (bridge_state) > + bus_flags = bridge_state->input_bus_cfg.flags; > + > + if (bus_flags & DRM_BUS_FLAG_DE_HIGH) > + val |= PVI_CTRL_OP_DE_POL | PVI_CTRL_INP_DE_POL; > + > + writel(val, pvi->regs + HTX_PVI_CTRL); > +} Apologies if this has already been reported or fixed, I searched lore and did not find anything. Clang warns (or errors with CONFIG_WERROR=y) for this function: drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:81:11: error: variable 'bus_flags' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] 81 | else if (bridge_state) | ^~~~ drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:84:6: note: uninitialized use occurs here 84 | if (bus_flags & DRM_BUS_FLAG_DE_HIGH) | ^ drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:81:7: note: remove the 'if' if its condition is always true 81 | else if (bridge_state) | ^ 82 | bus_flags = bridge_state->input_bus_cfg.flags; drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:60:15: note: initialize the variable 'bus_flags' to silence this warning 60 | u32 bus_flags, val; | ^ | = 0 1 error generated. This seems legitimate. If bridge_state can be NULL, should bus_flags be initialized to zero like it suggests or should that 'else if' be turned into a plain 'else'? I am happy to send a patch with that guidance. Cheers, Nathan
[PATCH] drm/amd/display: Increase frame-larger-than for all display_mode_vba files
After a recent change in LLVM, allmodconfig (which has CONFIG_KCSAN=y and CONFIG_WERROR=y enabled) has a few new instances of -Wframe-larger-than for the mode support and system configuration functions: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20v2.c:3393:6: error: stack frame size (2144) exceeds limit (2048) in 'dml20v2_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] 3393 | void dml20v2_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) | ^ 1 error generated. drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:3520:6: error: stack frame size (2192) exceeds limit (2048) in 'dml21_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] 3520 | void dml21_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) | ^ 1 error generated. drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3286:6: error: stack frame size (2128) exceeds limit (2048) in 'dml20_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] 3286 | void dml20_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) | ^ 1 error generated. Without the sanitizers enabled, there are no warnings. This was the catalyst for commit 6740ec97bcdb ("drm/amd/display: Increase frame warning limit with KASAN or KCSAN in dml2") and that same change was made to dml in commit 5b750b22530f ("drm/amd/display: Increase frame warning limit with KASAN or KCSAN in dml") but the frame_warn_flag variable was not applied to all files. Do so now to clear up the warnings and make all these files consistent. Cc: sta...@vger.kernel.org Closes: https://github.com/ClangBuiltLinux/linux/issue/1990 Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/display/dc/dml/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile index 6042a5a6a44f..59ade76ffb18 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile @@ -72,11 +72,11 @@ CFLAGS_$(AMDDALPATH)/dc/dml/display_mode_lib.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/display_mode_vba.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn10/dcn10_fpu.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/dcn20_fpu.o := $(dml_ccflags) -CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_mode_vba_20.o := $(dml_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_mode_vba_20.o := $(dml_ccflags) $(frame_warn_flag) CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_rq_dlg_calc_20.o := $(dml_ccflags) -CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_mode_vba_20v2.o := $(dml_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_mode_vba_20v2.o := $(dml_ccflags) $(frame_warn_flag) CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_rq_dlg_calc_20v2.o := $(dml_ccflags) -CFLAGS_$(AMDDALPATH)/dc/dml/dcn21/display_mode_vba_21.o := $(dml_ccflags) +CFLAGS_$(AMDDALPATH)/dc/dml/dcn21/display_mode_vba_21.o := $(dml_ccflags) $(frame_warn_flag) CFLAGS_$(AMDDALPATH)/dc/dml/dcn21/display_rq_dlg_calc_21.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn30/display_mode_vba_30.o := $(dml_ccflags) $(frame_warn_flag) CFLAGS_$(AMDDALPATH)/dc/dml/dcn30/display_rq_dlg_calc_30.o := $(dml_ccflags) --- base-commit: 6813cdca4ab94a238f8eb0cef3d3f3fcbdfb0ee0 change-id: 20240205-amdgpu-raise-flt-for-dml-vba-files-ee5b5a9c5e43 Best regards, -- Nathan Chancellor
Re: [PATCH 1/3] selftests/bpf: Update LLVM Phabricator links
Hi Alexei, On Thu, Jan 11, 2024 at 12:00:50PM -0800, Alexei Starovoitov wrote: > On Thu, Jan 11, 2024 at 11:40 AM Nathan Chancellor wrote: > > > > Hi Yonghong, > > > > On Wed, Jan 10, 2024 at 08:05:36PM -0800, Yonghong Song wrote: > > > > > > On 1/9/24 2:16 PM, Nathan Chancellor wrote: > > > > reviews.llvm.org was LLVM's Phabricator instances for code review. It > > > > has been abandoned in favor of GitHub pull requests. While the majority > > > > of links in the kernel sources still work because of the work Fangrui > > > > has done turning the dynamic Phabricator instance into a static archive, > > > > there are some issues with that work, so preemptively convert all the > > > > links in the kernel sources to point to the commit on GitHub. > > > > > > > > Most of the commits have the corresponding differential review link in > > > > the commit message itself so there should not be any loss of fidelity in > > > > the relevant information. > > > > > > > > Additionally, fix a typo in the xdpwall.c print ("LLMV" -> "LLVM") while > > > > in the area. > > > > > > > > Link: > > > > https://discourse.llvm.org/t/update-on-github-pull-requests/71540/172 > > > > Signed-off-by: Nathan Chancellor > > > > > > Ack with one nit below. > > > > > > Acked-by: Yonghong Song > > > > > > > > > > @@ -304,6 +304,6 @@ from running test_progs will look like: > > > > .. code-block:: console > > > > - test_xdpwall:FAIL:Does LLVM have https://reviews.llvm.org/D109073? > > > > unexpected error: -4007 > > > > + test_xdpwall:FAIL:Does LLVM have > > > > https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d5? > > > > unexpected error: -4007 > > > > -__ https://reviews.llvm.org/D109073 > > > > +__ > > > > https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d > > > > > > To be consistent with other links, could you add the missing last alnum > > > '5' to the above link? > > > > Thanks a lot for catching this and providing an ack. Andrew, could you > > squash this update into selftests-bpf-update-llvm-phabricator-links.patch? > > Please send a new patch. > We'd like to take all bpf patches through the bpf tree to avoid conflicts. Very well, I've sent a standalone v2 on top of bpf-next: https://lore.kernel.org/20240111-bpf-update-llvm-phabricator-links-v2-1-9a7ae976b...@kernel.org/ Andrew, just drop selftests-bpf-update-llvm-phabricator-links.patch altogether in that case, the other two patches are fine to go via -mm I think. Cheers, Nathan
Re: [PATCH 1/3] selftests/bpf: Update LLVM Phabricator links
Hi Yonghong, On Wed, Jan 10, 2024 at 08:05:36PM -0800, Yonghong Song wrote: > > On 1/9/24 2:16 PM, Nathan Chancellor wrote: > > reviews.llvm.org was LLVM's Phabricator instances for code review. It > > has been abandoned in favor of GitHub pull requests. While the majority > > of links in the kernel sources still work because of the work Fangrui > > has done turning the dynamic Phabricator instance into a static archive, > > there are some issues with that work, so preemptively convert all the > > links in the kernel sources to point to the commit on GitHub. > > > > Most of the commits have the corresponding differential review link in > > the commit message itself so there should not be any loss of fidelity in > > the relevant information. > > > > Additionally, fix a typo in the xdpwall.c print ("LLMV" -> "LLVM") while > > in the area. > > > > Link: https://discourse.llvm.org/t/update-on-github-pull-requests/71540/172 > > Signed-off-by: Nathan Chancellor > > Ack with one nit below. > > Acked-by: Yonghong Song > > @@ -304,6 +304,6 @@ from running test_progs will look like: > > .. code-block:: console > > - test_xdpwall:FAIL:Does LLVM have https://reviews.llvm.org/D109073? > > unexpected error: -4007 > > + test_xdpwall:FAIL:Does LLVM have > > https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d5? > > unexpected error: -4007 > > -__ https://reviews.llvm.org/D109073 > > +__ > > https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d > > To be consistent with other links, could you add the missing last alnum '5' > to the above link? Thanks a lot for catching this and providing an ack. Andrew, could you squash this update into selftests-bpf-update-llvm-phabricator-links.patch? diff --git a/tools/testing/selftests/bpf/README.rst b/tools/testing/selftests/bpf/README.rst index b9a493f66557..e56034abb3c2 100644 --- a/tools/testing/selftests/bpf/README.rst +++ b/tools/testing/selftests/bpf/README.rst @@ -306,4 +306,4 @@ from running test_progs will look like: test_xdpwall:FAIL:Does LLVM have https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d5? unexpected error: -4007 -__ https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d +__ https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d5
[PATCH] drm/amd/display: Avoid enum conversion warning
Clang warns (or errors with CONFIG_WERROR=y) when performing arithmetic with different enumerated types, which is usually a bug: drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_dpia_bw.c:548:24: error: arithmetic between different enumeration types ('const enum dc_link_rate' and 'const enum dc_lane_count') [-Werror,-Wenum-enum-conversion] 548 | link_cap->link_rate * link_cap->lane_count * LINK_RATE_REF_FREQ_IN_KHZ * 8; | ~~~ ^ 1 error generated. In this case, there is not a problem because the enumerated types are basically treated as '#define' values. Add an explicit cast to an integral type to silence the warning. Closes: https://github.com/ClangBuiltLinux/linux/issues/1976 Fixes: 5f3bce13266e ("drm/amd/display: Request usb4 bw for mst streams") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c index 4ef1a6a1d129..dd0d2b206462 100644 --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_dpia_bw.c @@ -544,8 +544,9 @@ int link_dp_dpia_get_dp_overhead_in_dp_tunneling(struct dc_link *link) */ const struct dc_link_settings *link_cap = dc_link_get_link_cap(link); - uint32_t link_bw_in_kbps = - link_cap->link_rate * link_cap->lane_count * LINK_RATE_REF_FREQ_IN_KHZ * 8; + uint32_t link_bw_in_kbps = (uint32_t)link_cap->link_rate * + (uint32_t)link_cap->lane_count * + LINK_RATE_REF_FREQ_IN_KHZ * 8; link_mst_overhead = (link_bw_in_kbps / 64) + ((link_bw_in_kbps % 64) ? 1 : 0); } --- base-commit: 6e7a29f011ac03a765f53844f7c3f04cdd421715 change-id: 20240110-amdgpu-display-enum-enum-conversion-e2451adbf4a7 Best regards, -- Nathan Chancellor
[PATCH 1/3] selftests/bpf: Update LLVM Phabricator links
reviews.llvm.org was LLVM's Phabricator instances for code review. It has been abandoned in favor of GitHub pull requests. While the majority of links in the kernel sources still work because of the work Fangrui has done turning the dynamic Phabricator instance into a static archive, there are some issues with that work, so preemptively convert all the links in the kernel sources to point to the commit on GitHub. Most of the commits have the corresponding differential review link in the commit message itself so there should not be any loss of fidelity in the relevant information. Additionally, fix a typo in the xdpwall.c print ("LLMV" -> "LLVM") while in the area. Link: https://discourse.llvm.org/t/update-on-github-pull-requests/71540/172 Signed-off-by: Nathan Chancellor --- Cc: a...@kernel.org Cc: dan...@iogearbox.net Cc: and...@kernel.org Cc: myko...@fb.com Cc: b...@vger.kernel.org Cc: linux-kselft...@vger.kernel.org --- tools/testing/selftests/bpf/README.rst | 32 +++--- tools/testing/selftests/bpf/prog_tests/xdpwall.c | 2 +- .../selftests/bpf/progs/test_core_reloc_type_id.c | 2 +- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tools/testing/selftests/bpf/README.rst b/tools/testing/selftests/bpf/README.rst index cb9b95702ac6..b9a493f66557 100644 --- a/tools/testing/selftests/bpf/README.rst +++ b/tools/testing/selftests/bpf/README.rst @@ -115,7 +115,7 @@ the insn 20 undoes map_value addition. It is currently impossible for the verifier to understand such speculative pointer arithmetic. Hence `this patch`__ addresses it on the compiler side. It was committed on llvm 12. -__ https://reviews.llvm.org/D85570 +__ https://github.com/llvm/llvm-project/commit/ddf1864ace484035e3cde5e83b3a31ac81e059c6 The corresponding C code @@ -165,7 +165,7 @@ This is due to a llvm BPF backend bug. `The fix`__ has been pushed to llvm 10.x release branch and will be available in 10.0.1. The patch is available in llvm 11.0.0 trunk. -__ https://reviews.llvm.org/D78466 +__ https://github.com/llvm/llvm-project/commit/3cb7e7bf959dcd3b8080986c62e10a75c7af43f0 bpf_verif_scale/loop6.bpf.o test failure with Clang 12 == @@ -204,7 +204,7 @@ r5(w5) is eventually saved on stack at insn #24 for later use. This cause later verifier failure. The bug has been `fixed`__ in Clang 13. -__ https://reviews.llvm.org/D97479 +__ https://github.com/llvm/llvm-project/commit/1959ead525b8830cc8a345f45e1c3ef9902d3229 BPF CO-RE-based tests and Clang version === @@ -221,11 +221,11 @@ failures: - __builtin_btf_type_id() [0_, 1_, 2_]; - __builtin_preserve_type_info(), __builtin_preserve_enum_value() [3_, 4_]. -.. _0: https://reviews.llvm.org/D74572 -.. _1: https://reviews.llvm.org/D74668 -.. _2: https://reviews.llvm.org/D85174 -.. _3: https://reviews.llvm.org/D83878 -.. _4: https://reviews.llvm.org/D83242 +.. _0: https://github.com/llvm/llvm-project/commit/6b01b465388b204d543da3cf49efd6080db094a9 +.. _1: https://github.com/llvm/llvm-project/commit/072cde03aaa13a2c57acf62d79876bf79aa1919f +.. _2: https://github.com/llvm/llvm-project/commit/00602ee7ef0bf6c68d690a2bd729c12b95c95c99 +.. _3: https://github.com/llvm/llvm-project/commit/6d218b4adb093ff2e9764febbbc89f429412006c +.. _4: https://github.com/llvm/llvm-project/commit/6d6750696400e7ce988d66a1a00e1d0cb32815f8 Floating-point tests and Clang version == @@ -234,7 +234,7 @@ Certain selftests, e.g. core_reloc, require support for the floating-point types, which was introduced in `Clang 13`__. The older Clang versions will either crash when compiling these tests, or generate an incorrect BTF. -__ https://reviews.llvm.org/D83289 +__ https://github.com/llvm/llvm-project/commit/a7137b238a07d9399d3ae96c0b461571bd5aa8b2 Kernel function call test and Clang version === @@ -248,7 +248,7 @@ Without it, the error from compiling bpf selftests looks like: libbpf: failed to find BTF for extern 'tcp_slow_start' [25] section: -2 -__ https://reviews.llvm.org/D93563 +__ https://github.com/llvm/llvm-project/commit/886f9ff53155075bd5f1e994f17b85d1e1b7470c btf_tag test and Clang version == @@ -264,8 +264,8 @@ Without them, the btf_tag selftest will be skipped and you will observe: # btf_tag:SKIP -.. _0: https://reviews.llvm.org/D111588 -.. _1: https://reviews.llvm.org/D99 +.. _0: https://github.com/llvm/llvm-project/commit/a162b67c98066218d0d00aa13b99afb95d9bb5e6 +.. _1: https://github.com/llvm/llvm-project/commit/3466e00716e12e32fdb100e3fcfca5c2b3e8d784 Clang dependencies for static linking tests === @@ -274,7 +274,7 @@ linked_vars, linked_maps, and linked_funcs tests depend on `Clang fix`__ to generate valid BTF information for weak
[PATCH 2/3] arch and include: Update LLVM Phabricator links
reviews.llvm.org was LLVM's Phabricator instances for code review. It has been abandoned in favor of GitHub pull requests. While the majority of links in the kernel sources still work because of the work Fangrui has done turning the dynamic Phabricator instance into a static archive, there are some issues with that work, so preemptively convert all the links in the kernel sources to point to the commit on GitHub. Most of the commits have the corresponding differential review link in the commit message itself so there should not be any loss of fidelity in the relevant information. Link: https://discourse.llvm.org/t/update-on-github-pull-requests/71540/172 Signed-off-by: Nathan Chancellor --- arch/arm64/Kconfig | 4 ++-- arch/riscv/Kconfig | 2 +- arch/riscv/include/asm/ftrace.h | 2 +- include/linux/compiler-clang.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 7b071a00425d..3304ba7c6c2a 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -380,7 +380,7 @@ config BROKEN_GAS_INST config BUILTIN_RETURN_ADDRESS_STRIPS_PAC bool # Clang's __builtin_return_adddress() strips the PAC since 12.0.0 - # https://reviews.llvm.org/D75044 + # https://github.com/llvm/llvm-project/commit/2a96f47c5ffca84cd774ad402cacd137f4bf45e2 default y if CC_IS_CLANG && (CLANG_VERSION >= 12) # GCC's __builtin_return_address() strips the PAC since 11.1.0, # and this was backported to 10.2.0, 9.4.0, 8.5.0, but not earlier @@ -2202,7 +2202,7 @@ config STACKPROTECTOR_PER_TASK config UNWIND_PATCH_PAC_INTO_SCS bool "Enable shadow call stack dynamically using code patching" - # needs Clang with https://reviews.llvm.org/D111780 incorporated + # needs Clang with https://github.com/llvm/llvm-project/commit/de07cde67b5d205d58690be012106022aea6d2b3 incorporated depends on CC_IS_CLANG && CLANG_VERSION >= 15 depends on ARM64_PTR_AUTH_KERNEL && CC_HAS_BRANCH_PROT_PAC_RET depends on SHADOW_CALL_STACK diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index cd4c9a204d08..f7453eba0b62 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -291,7 +291,7 @@ config AS_HAS_INSN def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero) config AS_HAS_OPTION_ARCH - # https://reviews.llvm.org/D123515 + # https://github.com/llvm/llvm-project/commit/9e8ed3403c191ab9c4903e8eeb8f732ff8a43cb4 def_bool y depends on $(as-instr, .option arch$(comma) +m) depends on !$(as-instr, .option arch$(comma) -i) diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index 2b2f5df7ef2c..3f526404a718 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -15,7 +15,7 @@ /* * Clang prior to 13 had "mcount" instead of "_mcount": - * https://reviews.llvm.org/D98881 + * https://github.com/llvm/llvm-project/commit/ef58ae86ba778ed7d01cd3f6bd6d08f943abab44 */ #if defined(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 13 #define MCOUNT_NAME _mcount diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h index ddab1ef22bee..f0a47afef125 100644 --- a/include/linux/compiler-clang.h +++ b/include/linux/compiler-clang.h @@ -9,7 +9,7 @@ * Clang prior to 17 is being silly and considers many __cleanup() variables * as unused (because they are, their sole purpose is to go out of scope). * - * https://reviews.llvm.org/D152180 + * https://github.com/llvm/llvm-project/commit/877210faa447f4cc7db87812f8ed80e398fedd61 */ #undef __cleanup #define __cleanup(func) __maybe_unused __attribute__((__cleanup__(func))) -- 2.43.0
[PATCH 3/3] treewide: Update LLVM Bugzilla links
LLVM moved their issue tracker from their own Bugzilla instance to GitHub issues. While all of the links are still valid, they may not necessarily show the most up to date information around the issues, as all updates will occur on GitHub, not Bugzilla. Another complication is that the Bugzilla issue number is not always the same as the GitHub issue number. Thankfully, LLVM maintains this mapping through two shortlinks: https://llvm.org/bz -> https://bugs.llvm.org/show_bug.cgi?id= https://llvm.org/pr -> https://github.com/llvm/llvm-project/issues/ Switch all "https://bugs.llvm.org/show_bug.cgi?id=" links to the "https://llvm.org/pr" shortlink so that the links show the most up to date information. Each migrated issue links back to the Bugzilla entry, so there should be no loss of fidelity of information here. Signed-off-by: Nathan Chancellor --- arch/powerpc/Makefile | 4 ++-- arch/powerpc/kvm/book3s_hv_nested.c | 2 +- arch/s390/include/asm/ftrace.h | 2 +- arch/x86/power/Makefile | 2 +- crypto/blake2b_generic.c| 2 +- drivers/firmware/efi/libstub/Makefile | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c| 2 +- drivers/media/test-drivers/vicodec/codec-fwht.c | 2 +- drivers/regulator/Kconfig | 2 +- include/asm-generic/vmlinux.lds.h | 2 +- lib/Kconfig.kasan | 2 +- lib/raid6/Makefile | 2 +- lib/stackinit_kunit.c | 2 +- mm/slab_common.c| 2 +- net/bridge/br_multicast.c | 2 +- security/Kconfig| 2 +- 16 files changed, 17 insertions(+), 17 deletions(-) diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index f19dbaa1d541..cd6aaa45f355 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -133,11 +133,11 @@ CFLAGS-$(CONFIG_PPC64)+= $(call cc-option,-mno-pointers-to-nested-functions) CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mlong-double-128) # Clang unconditionally reserves r2 on ppc32 and does not support the flag -# https://bugs.llvm.org/show_bug.cgi?id=39555 +# https://llvm.org/pr39555 CFLAGS-$(CONFIG_PPC32) := $(call cc-option, -ffixed-r2) # Clang doesn't support -mmultiple / -mno-multiple -# https://bugs.llvm.org/show_bug.cgi?id=39556 +# https://llvm.org/pr39556 CFLAGS-$(CONFIG_PPC32) += $(call cc-option, $(MULTIPLEWORD)) CFLAGS-$(CONFIG_PPC32) += $(call cc-option,-mno-readonly-in-sdata) diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c index 3b658b8696bc..3f5970f74c6b 100644 --- a/arch/powerpc/kvm/book3s_hv_nested.c +++ b/arch/powerpc/kvm/book3s_hv_nested.c @@ -55,7 +55,7 @@ void kvmhv_save_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr) hr->dawrx1 = vcpu->arch.dawrx1; } -/* Use noinline_for_stack due to https://bugs.llvm.org/show_bug.cgi?id=49610 */ +/* Use noinline_for_stack due to https://llvm.org/pr49610 */ static noinline_for_stack void byteswap_pt_regs(struct pt_regs *regs) { unsigned long *addr = (unsigned long *) regs; diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h index 5a82b08f03cd..621f23d5ae30 100644 --- a/arch/s390/include/asm/ftrace.h +++ b/arch/s390/include/asm/ftrace.h @@ -9,7 +9,7 @@ #ifndef __ASSEMBLY__ #ifdef CONFIG_CC_IS_CLANG -/* https://bugs.llvm.org/show_bug.cgi?id=41424 */ +/* https://llvm.org/pr41424 */ #define ftrace_return_address(n) 0UL #else #define ftrace_return_address(n) __builtin_return_address(n) diff --git a/arch/x86/power/Makefile b/arch/x86/power/Makefile index 379777572bc9..e0cd7afd5302 100644 --- a/arch/x86/power/Makefile +++ b/arch/x86/power/Makefile @@ -5,7 +5,7 @@ CFLAGS_cpu.o := -fno-stack-protector # Clang may incorrectly inline functions with stack protector enabled into -# __restore_processor_state(): https://bugs.llvm.org/show_bug.cgi?id=47479 +# __restore_processor_state(): https://llvm.org/pr47479 CFLAGS_REMOVE_cpu.o := $(CC_FLAGS_LTO) obj-$(CONFIG_PM_SLEEP) += cpu.o diff --git a/crypto/blake2b_generic.c b/crypto/blake2b_generic.c index 6704c0355889..32e380b714b6 100644 --- a/crypto/blake2b_generic.c +++ b/crypto/blake2b_generic.c @@ -102,7 +102,7 @@ static void blake2b_compress_one_generic(struct blake2b_state *S, ROUND(10); ROUND(11); #ifdef CONFIG_CC_IS_CLANG -#pragma nounroll /* https://bugs.llvm.org/show_bug.cgi?id=45803 */ +#pragma nounroll /* https://llvm.org/pr45803 */ #endif for (i = 0; i < 8; ++i) S->h[i] = S->h[i] ^ v[i] ^ v[i + 8]; diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index 06964a3c130f..a223bd10564b 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/l
[PATCH 0/3] Update LLVM Phabricator and Bugzilla links
This series updates all instances of LLVM Phabricator and Bugzilla links to point to GitHub commits directly and LLVM's Bugzilla to GitHub issue shortlinks respectively. I split up the Phabricator patch into BPF selftests and the rest of the kernel in case the BPF folks want to take it separately from the rest of the series, there are obviously no dependency issues in that case. The Bugzilla change was mechanical enough and should have no conflicts. I am aiming this at Andrew and CC'ing other lists, in case maintainers want to chime in, but I think this is pretty uncontroversial (famous last words...). --- Nathan Chancellor (3): selftests/bpf: Update LLVM Phabricator links arch and include: Update LLVM Phabricator links treewide: Update LLVM Bugzilla links arch/arm64/Kconfig | 4 +-- arch/powerpc/Makefile | 4 +-- arch/powerpc/kvm/book3s_hv_nested.c| 2 +- arch/riscv/Kconfig | 2 +- arch/riscv/include/asm/ftrace.h| 2 +- arch/s390/include/asm/ftrace.h | 2 +- arch/x86/power/Makefile| 2 +- crypto/blake2b_generic.c | 2 +- drivers/firmware/efi/libstub/Makefile | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 2 +- drivers/media/test-drivers/vicodec/codec-fwht.c| 2 +- drivers/regulator/Kconfig | 2 +- include/asm-generic/vmlinux.lds.h | 2 +- include/linux/compiler-clang.h | 2 +- lib/Kconfig.kasan | 2 +- lib/raid6/Makefile | 2 +- lib/stackinit_kunit.c | 2 +- mm/slab_common.c | 2 +- net/bridge/br_multicast.c | 2 +- security/Kconfig | 2 +- tools/testing/selftests/bpf/README.rst | 32 +++--- tools/testing/selftests/bpf/prog_tests/xdpwall.c | 2 +- .../selftests/bpf/progs/test_core_reloc_type_id.c | 2 +- 23 files changed, 40 insertions(+), 40 deletions(-) --- base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a change-id: 20240109-update-llvm-links-d03f9d649e1e Best regards, -- Nathan Chancellor
[PATCH 3/3] drm/bridge: Return NULL instead of plain 0 in drm_dp_hpd_bridge_register() stub
sparse complains: drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c: note: in included file: include/drm/bridge/aux-bridge.h:29:16: sparse: sparse: Using plain integer as NULL pointer Return NULL to clear up the warning. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202312060025.bdeqzrwx-...@intel.com/ Fixes: e560518a6c2e ("drm/bridge: implement generic DP HPD bridge") Signed-off-by: Nathan Chancellor --- include/drm/bridge/aux-bridge.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/bridge/aux-bridge.h b/include/drm/bridge/aux-bridge.h index 66249ff0858e..c4c423e97f06 100644 --- a/include/drm/bridge/aux-bridge.h +++ b/include/drm/bridge/aux-bridge.h @@ -26,7 +26,7 @@ void drm_aux_hpd_bridge_notify(struct device *dev, enum drm_connector_status sta static inline struct device *drm_dp_hpd_bridge_register(struct device *parent, struct device_node *np) { - return 0; + return NULL; } static inline void drm_aux_hpd_bridge_notify(struct device *dev, enum drm_connector_status status) -- 2.43.0
[PATCH 2/3] usb: typec: qcom-pmic-typec: Only select DRM_AUX_HPD_BRIDGE with OF
CONFIG_DRM_AUX_HPD_BRIDGE depends on CONFIG_OF but that dependency is not included when CONFIG_TYPEC_QCOM_PMIC selects it, resulting in a Kconfig warning when CONFIG_OF is disabled: WARNING: unmet direct dependencies detected for DRM_AUX_HPD_BRIDGE Depends on [n]: HAS_IOMEM [=y] && DRM_BRIDGE [=y] && OF [=n] Selected by [m]: - TYPEC_QCOM_PMIC [=m] && USB_SUPPORT [=y] && TYPEC [=m] && TYPEC_TCPM [=m] && (ARCH_QCOM || COMPILE_TEST [=y]) && (DRM [=m] || DRM [=m]=n) && DRM_BRIDGE [=y] Only select CONFIG_DRM_AUX_HPD_BRIDGE with both CONFIG_DRM_BRIDGE and CONFIG_OF to clear up the warning. Fixes: 7d9f1b72b296 ("usb: typec: qcom-pmic-typec: switch to DRM_AUX_HPD_BRIDGE") Signed-off-by: Nathan Chancellor --- drivers/usb/typec/tcpm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig index 64d5421c69e6..8cdd84ca5d6f 100644 --- a/drivers/usb/typec/tcpm/Kconfig +++ b/drivers/usb/typec/tcpm/Kconfig @@ -80,7 +80,7 @@ config TYPEC_QCOM_PMIC tristate "Qualcomm PMIC USB Type-C Port Controller Manager driver" depends on ARCH_QCOM || COMPILE_TEST depends on DRM || DRM=n - select DRM_AUX_HPD_BRIDGE if DRM_BRIDGE + select DRM_AUX_HPD_BRIDGE if DRM_BRIDGE && OF help A Type-C port and Power Delivery driver which aggregates two discrete pieces of silicon in the PM8150b PMIC block: the -- 2.43.0
[PATCH 0/3] A few fixes for transparent bridge support
Hi all, This series fixes two Kconfig issues that I noticed with the selection of CONFIG_DRM_AUX{,_HPD}_BRIDGE along with a fix for a sparse report that I noticed while seeing if these had already been resolved. --- Nathan Chancellor (3): usb: typec: nb7vpq904m: Only select DRM_AUX_BRIDGE with OF usb: typec: qcom-pmic-typec: Only select DRM_AUX_HPD_BRIDGE with OF drm/bridge: Return NULL instead of plain 0 in drm_dp_hpd_bridge_register() stub drivers/usb/typec/mux/Kconfig | 2 +- drivers/usb/typec/tcpm/Kconfig | 2 +- include/drm/bridge/aux-bridge.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) --- base-commit: 4900e0396e59be233cfa636369d4eec6b40dbeca change-id: 20231205-drm_aux_bridge-fixes-162780ed704a Best regards, -- Nathan Chancellor
[PATCH 1/3] usb: typec: nb7vpq904m: Only select DRM_AUX_BRIDGE with OF
CONFIG_DRM_AUX_BRIDGE depends on CONFIG_OF but that dependency is not included when CONFIG_TYPEC_MUX_NB7VPQ904M selects it, resulting in a Kconfig warning when CONFIG_OF is disabled: WARNING: unmet direct dependencies detected for DRM_AUX_BRIDGE Depends on [n]: HAS_IOMEM [=y] && DRM_BRIDGE [=y] && OF [=n] Selected by [y]: - TYPEC_MUX_NB7VPQ904M [=y] && USB_SUPPORT [=y] && TYPEC [=y] && I2C [=y] && (DRM [=y] || DRM [=y]=n) && DRM_BRIDGE [=y] Only select CONFIG_DRM_AUX_BRIDGE with both CONFIG_DRM_BRIDGE and CONFIG_OF to clear up the warning. Fixes: c5d296bad640 ("usb: typec: nb7vpq904m: switch to DRM_AUX_BRIDGE") Signed-off-by: Nathan Chancellor --- drivers/usb/typec/mux/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig index 5120942f309d..38416fb0cc3c 100644 --- a/drivers/usb/typec/mux/Kconfig +++ b/drivers/usb/typec/mux/Kconfig @@ -40,7 +40,7 @@ config TYPEC_MUX_NB7VPQ904M tristate "On Semiconductor NB7VPQ904M Type-C redriver driver" depends on I2C depends on DRM || DRM=n - select DRM_AUX_BRIDGE if DRM_BRIDGE + select DRM_AUX_BRIDGE if DRM_BRIDGE && OF select REGMAP_I2C help Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C -- 2.43.0
Re: [RFC] drm: enable W=1 warnings by default across the subsystem
On Thu, Nov 30, 2023 at 10:52:17AM +0200, Jani Nikula wrote: > On Wed, 29 Nov 2023, Hamza Mahfooz wrote: > > Cc: Nathan Chancellor > > > > On 11/29/23 13:12, Jani Nikula wrote: > >> At least the i915 and amd drivers enable a bunch more compiler warnings > >> than the kernel defaults. > >> > >> Extend 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. > > > > I think this should go in after drm-misc-next has a clean build (for > > COMPILE_TEST builds) with this patch applied. Otherwise, it will break a > > lot of build configs. > > Oh, I'm absolutely not suggesting this should be merged before known > warnings have been addressed one way or another. Either by fixing them > or by disabling said warning in driver local Makefiles, depending on the > case. > > While I'm suggesting this, I obviously (I hope) can't take on fixing all > the warnings this exposes. One way to scale would be for folks to apply > this locally, see what it uncovers in their drivers, and fix some. > > As an intermediate step we might also apply this to a topic branch in > drm-tip, so you'd see the warnings when building drm-tip, but not in > indidual branches or in anything that's merged upstream. For what it's worth, I ran this through my personal build matrix with LLVM/clang and it only found a few instances of -Wunused-but-set-variable: drivers/gpu/drm/mediatek/mtk_disp_gamma.c:121:6: warning: variable 'cfg_val' set but not used [-Wunused-but-set-variable] drivers/gpu/drm/nouveau/nouveau_svm.c:115:24: warning: variable 'priority' set but not used [-Wunused-but-set-variable] drivers/gpu/drm/nouveau/nouveau_svm.c:929:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable] drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c:221:7: warning: variable 'loc' set but not used [-Wunused-but-set-variable] drivers/gpu/drm/qxl/qxl_cmd.c:424:6: warning: variable 'count' set but not used [-Wunused-but-set-variable] drivers/gpu/drm/qxl/qxl_ioctl.c:148:14: warning: variable 'num_relocs' set but not used [-Wunused-but-set-variable] I know that clang does not support all the warnings that are being enabled here but I suspect there won't be too many instances of the other warnings. -Wformat-overflow and -Wformat-truncation might be the big ones. I know -Wstringop-overflow is being turned on globally in -next so that is one that you shouldn't have to worry much about. Cheers, Nathan
Re: [PATCH 3/3] drm/amd/display: Support DRM_AMD_DC_FP on RISC-V
On Thu, Nov 23, 2023 at 02:23:01PM +, Conor Dooley wrote: > On Tue, Nov 21, 2023 at 07:05:15PM -0800, Samuel Holland wrote: > > RISC-V uses kernel_fpu_begin()/kernel_fpu_end() like several other > > architectures. Enabling hardware FP requires overriding the ISA string > > for the relevant compilation units. > > Ah yes, bringing the joy of frame-larger-than warnings to RISC-V: > ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:58:13: > warning: stack frame size (2416) exceeds limit (2048) in > 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' > [-Wframe-larger-than] :( > Nathan, have you given up on these being sorted out? Does your configuration have KASAN (I don't think RISC-V supports KCSAN)? It is possible that dml/dcn32 needs something similar to commit 6740ec97bcdb ("drm/amd/display: Increase frame warning limit with KASAN or KCSAN in dml2")? I am not really interested in playing whack-a-mole with these warnings like I have done in the past for the reasons I outlined here: https://lore.kernel.org/20231019205117.GA839902@dev-arch.thelio-3990X/ > Also, what on earth is that function name, it exceeds 80 characters > before even considering anything else? Actually, I don't think I want > to know. Welcome to "gcc-parsable HW gospel, coming straight from HW engineers" :) Cheers, Nathan
Re: [PATCH 1/3] kunit: Add a macro to wrap a deferred action function
nd trace ]--- [08:06:03] RIP: 0010:__kunit_action_free+0x18/0x20 ... With this series applied with https://lore.kernel.org/20231106172557.2963-1...@opensource.cirrus.com/, all the tests pass for arm64 and x86_64 on my machine. I see no remaining casts in the tree in this state. It seems like the documentation in Documentation/dev-tools/kunit/usage.rst may want to be updated to remove mention of casting to kunit_action_t as well? Regardless: Reviewed-by: Nathan Chancellor Tested-by: Nathan Chancellor > include/kunit/resource.h | 9 + > lib/kunit/kunit-test.c | 5 + > lib/kunit/test.c | 6 -- > 3 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/include/kunit/resource.h b/include/kunit/resource.h > index c7383e90f5c9..4110e13970dc 100644 > --- a/include/kunit/resource.h > +++ b/include/kunit/resource.h > @@ -390,6 +390,15 @@ void kunit_remove_resource(struct kunit *test, struct > kunit_resource *res); > /* A 'deferred action' function to be used with kunit_add_action. */ > typedef void (kunit_action_t)(void *); > > +/* We can't cast function pointers to kunit_action_t if CFI is enabled. */ > +#define KUNIT_DEFINE_ACTION_WRAPPER(wrapper, orig, arg_type) \ > + static void wrapper(void *in) \ > + { \ > + arg_type arg = (arg_type)in; \ > + orig(arg); \ > + } > + > + > /** > * kunit_add_action() - Call a function when the test ends. > * @test: Test case to associate the action with. > diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c > index de2113a58fa0..ee6927c60979 100644 > --- a/lib/kunit/kunit-test.c > +++ b/lib/kunit/kunit-test.c > @@ -538,10 +538,7 @@ static struct kunit_suite kunit_resource_test_suite = { > #if IS_BUILTIN(CONFIG_KUNIT_TEST) > > /* This avoids a cast warning if kfree() is passed direct to > kunit_add_action(). */ > -static void kfree_wrapper(void *p) > -{ > - kfree(p); > -} > +KUNIT_DEFINE_ACTION_WRAPPER(kfree_wrapper, kfree, const void *); > > static void kunit_log_test(struct kunit *test) > { > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index f2eb71f1a66c..0308865194bb 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -772,6 +772,8 @@ static struct notifier_block kunit_mod_nb = { > }; > #endif > > +KUNIT_DEFINE_ACTION_WRAPPER(kfree_action_wrapper, kfree, const void *) > + > void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t > gfp) > { > void *data; > @@ -781,7 +783,7 @@ void *kunit_kmalloc_array(struct kunit *test, size_t n, > size_t size, gfp_t gfp) > if (!data) > return NULL; > > - if (kunit_add_action_or_reset(test, (kunit_action_t *)kfree, data) != 0) > + if (kunit_add_action_or_reset(test, kfree_action_wrapper, data) != 0) > return NULL; > > return data; > @@ -793,7 +795,7 @@ void kunit_kfree(struct kunit *test, const void *ptr) > if (!ptr) > return; > > - kunit_release_action(test, (kunit_action_t *)kfree, (void *)ptr); > + kunit_release_action(test, kfree_action_wrapper, (void *)ptr); > } > EXPORT_SYMBOL_GPL(kunit_kfree); > > -- > 2.42.0.869.gea05f2083d-goog >
Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()
On Tue, Nov 07, 2023 at 10:17:43AM +0100, Uwe Kleine-König wrote: > On today's platforms the benefit of platform_driver_probe() isn't that > relevant any more. It allows to drop some code after booting (or module > loading) for .probe() and discard the .remove() function completely if > the driver is built-in. This typically saves a few 100k. > > The downside of platform_driver_probe() is that the driver cannot be > bound and unbound at runtime which is ancient and also slightly > complicates testing. There are also thoughts to deprecate > platform_driver_probe() because it adds some complexity in the driver > core for little gain. Also many drivers don't use it correctly. This > driver for example misses to mark the driver struct with __refdata which > is needed to suppress a (W=1) modpost warning: > > WARNING: modpost: drivers/video/fbdev/atmel_lcdfb: section mismatch in > reference: atmel_lcdfb_driver+0x4 (section: .data) -> atmel_lcdfb_remove > (section: .exit.text) > > Signed-off-by: Uwe Kleine-König > --- > drivers/video/fbdev/atmel_lcdfb.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/video/fbdev/atmel_lcdfb.c > b/drivers/video/fbdev/atmel_lcdfb.c > index a908db233409..b218731ef732 100644 > --- a/drivers/video/fbdev/atmel_lcdfb.c > +++ b/drivers/video/fbdev/atmel_lcdfb.c > @@ -1017,7 +1017,7 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info > *sinfo) > return ret; > } > > -static int __init atmel_lcdfb_probe(struct platform_device *pdev) > +static int atmel_lcdfb_probe(struct platform_device *pdev) > { > struct device *dev = >dev; > struct fb_info *info; > @@ -1223,7 +1223,7 @@ static int __init atmel_lcdfb_probe(struct > platform_device *pdev) > return ret; > } > > -static int __exit atmel_lcdfb_remove(struct platform_device *pdev) > +static int atmel_lcdfb_remove(struct platform_device *pdev) > { > struct device *dev = >dev; > struct fb_info *info = dev_get_drvdata(dev); > @@ -1301,7 +1301,8 @@ static int atmel_lcdfb_resume(struct platform_device > *pdev) > #endif > > static struct platform_driver atmel_lcdfb_driver = { > - .remove = __exit_p(atmel_lcdfb_remove), > + .probe = atmel_lcdfb_probe, > + .remove = atmel_lcdfb_remove, > .suspend= atmel_lcdfb_suspend, > .resume = atmel_lcdfb_resume, > .driver = { > @@ -1310,7 +1311,7 @@ static struct platform_driver atmel_lcdfb_driver = { > }, > }; > > -module_platform_driver_probe(atmel_lcdfb_driver, atmel_lcdfb_probe); > +module_platform_driver(atmel_lcdfb_driver, ); > > MODULE_DESCRIPTION("AT91 LCD Controller framebuffer driver"); > MODULE_AUTHOR("Nicolas Ferre "); > -- > 2.42.0 > For what it's worth, this introduces a warning when building certain configurations (such as ARCH=arm multi_v5_defconfig) with clang: WARNING: modpost: vmlinux: section mismatch in reference: atmel_lcdfb_probe+0x6c4 (section: .text) -> atmel_lcdfb_init_fbinfo (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: atmel_lcdfb_probe+0x858 (section: .text) -> atmel_lcdfb_fix (section: .init.rodata) This appears to be legitimate to me? GCC did not warn but I assume that is due to differences in inlining. The following clears it up for me, should I send a standalone patch or should this be squashed in? Cheers, Nathan diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c index 88c75ae7d315..9e391e5eaf9d 100644 --- a/drivers/video/fbdev/atmel_lcdfb.c +++ b/drivers/video/fbdev/atmel_lcdfb.c @@ -220,7 +220,7 @@ static inline void atmel_lcdfb_power_control(struct atmel_lcdfb_info *sinfo, int } } -static const struct fb_fix_screeninfo atmel_lcdfb_fix __initconst = { +static const struct fb_fix_screeninfo atmel_lcdfb_fix = { .type = FB_TYPE_PACKED_PIXELS, .visual = FB_VISUAL_TRUECOLOR, .xpanstep = 0, @@ -841,7 +841,7 @@ static void atmel_lcdfb_task(struct work_struct *work) atmel_lcdfb_reset(sinfo); } -static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo) +static int atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo) { struct fb_info *info = sinfo->info; int ret = 0;
Re: [PATCH] drm/amd/display: Increase frame warning limit for clang in dml2
On Thu, Nov 02, 2023 at 12:59:00PM -0400, Hamza Mahfooz wrote: > On 11/2/23 12:24, Nathan Chancellor wrote: > > When building ARCH=x86_64 allmodconfig with clang, which have sanitizers > > enabled, there is a warning about a large stack frame. > > > > > > drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6265:13: > > error: stack frame size (2520) exceeds limit (2048) in 'dml_prefetch_check' > > [-Werror,-Wframe-larger-than] > > 6265 | static void dml_prefetch_check(struct display_mode_lib_st > > *mode_lib) > > | ^ > >1 error generated. > > > > Notably, GCC 13.2.0 does not do too much of a better job, as it is right > > at the current limit of 2048: > > > >drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c: In > > function 'dml_prefetch_check': > > > > drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6705:1: > > error: the frame size of 2048 bytes is larger than 1800 bytes > > [-Werror=frame-larger-than=] > > 6705 | } > > | ^ > > > > In the past, these warnings have been avoided by reducing the number of > > parameters to various functions so that not as many arguments need to be > > passed on the stack. However, these patches take a good amount of effort > > to write despite being mechanical due to code structure and complexity > > and they are never carried forward to new generations of the code so > > that effort has to be expended every new hardware generation, which > > becomes harder to justify as time goes on. > > > > There is some effort to improve clang's code generation but that may > > take some time between code review, shifting priorities, and release > > cycles. To avoid having a noticeable or lengthy breakage in > > all{mod,yes}config, which are easy testing targets that have -Werror > > enabled, increase the limit for clang by 50% so that cases of extremely > > poor code generation can still be caught while not breaking the majority > > of builds. When clang's code generation improves, the limit increase can > > be restricted to older clang versions. > > > > Signed-off-by: Nathan Chancellor > > --- > > If there is another DRM pull before 6.7-rc1, it would be much > > appreciated if this could make that so that other trees are not > > potentially broken by this. If not, no worries, as it was my fault for > > not sending this sooner. > > --- > > drivers/gpu/drm/amd/display/dc/dml2/Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile > > b/drivers/gpu/drm/amd/display/dc/dml2/Makefile > > index 70ae5eba624e..dff8237c0999 100644 > > --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile > > +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile > > @@ -60,7 +60,7 @@ endif > > endif > > ifneq ($(CONFIG_FRAME_WARN),0) > > -frame_warn_flag := -Wframe-larger-than=2048 > > +frame_warn_flag := -Wframe-larger-than=$(if > > $(CONFIG_CC_IS_CLANG),3072,2048) > > I would prefer checking for `CONFIG_KASAN || CONFIG_KCSAN` instead > since the stack usage shouldn't change much if both of those are disabled. So something like this? Or were you talking about replacing the clang check entirely with the KASAN/KCSAN check? diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile index 70ae5eba624e..0fc1b13295eb 100644 --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile @@ -60,8 +60,12 @@ endif endif ifneq ($(CONFIG_FRAME_WARN),0) +ifeq ($(CONFIG_CC_IS_CLANG)$(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),yy) +frame_warn_flag := -Wframe-larger-than=3072 +else frame_warn_flag := -Wframe-larger-than=2048 endif +endif CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) $(frame_warn_flag) CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_util.o := $(dml2_ccflags) > > endif > > CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) > > $(frame_warn_flag) > > > > --- > > base-commit: 21e80f3841c01aeaf32d7aee7bbc87b3db1aa0c6 > > change-id: > > 20231102-amdgpu-dml2-increase-frame-size-warning-for-clang-c93bd2d6a871 > > > > Best regards, > -- > Hamza >
[PATCH v2] drm/amd/display: Increase frame warning limit with KASAN or KCSAN in dml2
When building ARCH=x86_64 allmodconfig with clang, which will typically have sanitizers enabled, there is a warning about a large stack frame. drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6265:13: error: stack frame size (2520) exceeds limit (2048) in 'dml_prefetch_check' [-Werror,-Wframe-larger-than] 6265 | static void dml_prefetch_check(struct display_mode_lib_st *mode_lib) | ^ 1 error generated. Notably, GCC 13.2.0 does not do too much of a better job, as it is right at the current limit of 2048 (and others have reported being over with older GCC versions): drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c: In function 'dml_prefetch_check': drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6705:1: error: the frame size of 2048 bytes is larger than 1800 bytes [-Werror=frame-larger-than=] 6705 | } | ^ In the past, these warnings have been avoided by reducing the number of parameters to various functions so that not as many arguments need to be passed on the stack. However, these patches take a good amount of effort to write despite being mechanical due to code structure and complexity and they are never carried forward to new generations of the code so that effort has to be expended every new hardware generation, which becomes harder to justify as time goes on. To avoid having a noticeable or lengthy breakage in all{mod,yes}config, which are easy testing targets that have -Werror enabled, increase the limit for configurations that have KASAN or KCSAN enabled by 50% so that cases of extremely poor code generation can still be caught while not breaking the majority of builds. CONFIG_KMSAN also causes high stack usage but the frame limit is already set to zero when it is enabled, which is accounted for by the check for CONFIG_FRAME_WARN=0 in the dml2 Makefile. Signed-off-by: Nathan Chancellor --- If there is another DRM pull before 6.7-rc1, it would be much appreciated if this could make that so that other trees are not potentially broken by this. If not, no worries, as it was my fault for not sending this sooner. Changes in v2: - Adjust workaround to check for either CONFIG_KASAN=y or CONFIG_KCSAN=y, as the same problem has been reported with older versions of GCC (Hamza, Alex) - Link to v1: https://lore.kernel.org/r/20231102-amdgpu-dml2-increase-frame-size-warning-for-clang-v1-1-6eb157352...@kernel.org --- drivers/gpu/drm/amd/display/dc/dml2/Makefile | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile index 70ae5eba624e..acff3449b8d7 100644 --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile @@ -60,8 +60,12 @@ endif endif ifneq ($(CONFIG_FRAME_WARN),0) +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) +frame_warn_flag := -Wframe-larger-than=3072 +else frame_warn_flag := -Wframe-larger-than=2048 endif +endif CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) $(frame_warn_flag) CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_util.o := $(dml2_ccflags) --- base-commit: 21e80f3841c01aeaf32d7aee7bbc87b3db1aa0c6 change-id: 20231102-amdgpu-dml2-increase-frame-size-warning-for-clang-c93bd2d6a871 Best regards, -- Nathan Chancellor
[PATCH] drm/amd/display: Increase frame warning limit for clang in dml2
When building ARCH=x86_64 allmodconfig with clang, which have sanitizers enabled, there is a warning about a large stack frame. drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6265:13: error: stack frame size (2520) exceeds limit (2048) in 'dml_prefetch_check' [-Werror,-Wframe-larger-than] 6265 | static void dml_prefetch_check(struct display_mode_lib_st *mode_lib) | ^ 1 error generated. Notably, GCC 13.2.0 does not do too much of a better job, as it is right at the current limit of 2048: drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c: In function 'dml_prefetch_check': drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6705:1: error: the frame size of 2048 bytes is larger than 1800 bytes [-Werror=frame-larger-than=] 6705 | } | ^ In the past, these warnings have been avoided by reducing the number of parameters to various functions so that not as many arguments need to be passed on the stack. However, these patches take a good amount of effort to write despite being mechanical due to code structure and complexity and they are never carried forward to new generations of the code so that effort has to be expended every new hardware generation, which becomes harder to justify as time goes on. There is some effort to improve clang's code generation but that may take some time between code review, shifting priorities, and release cycles. To avoid having a noticeable or lengthy breakage in all{mod,yes}config, which are easy testing targets that have -Werror enabled, increase the limit for clang by 50% so that cases of extremely poor code generation can still be caught while not breaking the majority of builds. When clang's code generation improves, the limit increase can be restricted to older clang versions. Signed-off-by: Nathan Chancellor --- If there is another DRM pull before 6.7-rc1, it would be much appreciated if this could make that so that other trees are not potentially broken by this. If not, no worries, as it was my fault for not sending this sooner. --- drivers/gpu/drm/amd/display/dc/dml2/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile index 70ae5eba624e..dff8237c0999 100644 --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile @@ -60,7 +60,7 @@ endif endif ifneq ($(CONFIG_FRAME_WARN),0) -frame_warn_flag := -Wframe-larger-than=2048 +frame_warn_flag := -Wframe-larger-than=$(if $(CONFIG_CC_IS_CLANG),3072,2048) endif CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) $(frame_warn_flag) --- base-commit: 21e80f3841c01aeaf32d7aee7bbc87b3db1aa0c6 change-id: 20231102-amdgpu-dml2-increase-frame-size-warning-for-clang-c93bd2d6a871 Best regards, -- Nathan Chancellor
[PATCH] drm/amd/display: Respect CONFIG_FRAME_WARN=0 in DML2
display_mode_code.c is unconditionally built with -Wframe-larger-than=2048, which causes warnings even when CONFIG_FRAME_WARN has been set to 0, which should show no warnings. Use the existing $(frame_warn_flag) variable, which handles this situation. This is basically commit 25f178bbd078 ("drm/amd/display: Respect CONFIG_FRAME_WARN=0 in dml Makefile") but for DML2. Fixes: 7966f319c66d ("drm/amd/display: Introduce DML2") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/display/dc/dml2/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile index f35ed8de260d..66431525f2a0 100644 --- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile @@ -61,7 +61,7 @@ ifneq ($(CONFIG_FRAME_WARN),0) frame_warn_flag := -Wframe-larger-than=2048 endif -CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) -Wframe-larger-than=2048 +CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) $(frame_warn_flag) CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_util.o := $(dml2_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml2/dml2_wrapper.o := $(dml2_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml2/dml2_utils.o := $(dml2_ccflags) --- base-commit: cd90511557fdfb394bb4ac4c3b539b007383914c change-id: 20231018-amdgpu-dml2-respect-frame_warn-536e19b69ce0 Best regards, -- Nathan Chancellor
[PATCH] drm/debugfs: Fix drm_debugfs_remove_files() stub
When building without CONFIG_DEBUG_FS: drivers/gpu/drm/tegra/dc.c:1757:59: error: too many arguments to function call, expected 3, have 4 1757 | drm_debugfs_remove_files(dc->debugfs_files, count, root, minor); | ^ include/drm/drm_debugfs.h:162:19: note: 'drm_debugfs_remove_files' declared here 162 | static inline int drm_debugfs_remove_files(const struct drm_info_list *files, | ^ ~~ 163 |int count, struct drm_minor *minor) | ~~ 1 error generated. Update the stub to include the root parameter. Fixes: 8e455145d8f1 ("drm/debugfs: rework drm_debugfs_create_files implementation v2") Signed-off-by: Nathan Chancellor --- include/drm/drm_debugfs.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h index 7213ce15e4ff..3bba169f9bae 100644 --- a/include/drm/drm_debugfs.h +++ b/include/drm/drm_debugfs.h @@ -160,7 +160,8 @@ static inline void drm_debugfs_create_files(const struct drm_info_list *files, {} static inline int drm_debugfs_remove_files(const struct drm_info_list *files, - int count, struct drm_minor *minor) + int count, struct dentry *root, + struct drm_minor *minor) { return 0; } --- base-commit: fc71f615fd08a530d24c7af0a1efa72ec6ea8e34 change-id: 20230913-fix-drm_debugfs_remove_files-stub-bd864127c162 Best regards, -- Nathan Chancellor
[PATCH] drm/amd/display: Fix -Wuninitialized in dm_helpers_dp_mst_send_payload_allocation()
When building with clang, there is a warning (or error when CONFIG_WERROR is set): drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:368:21: error: variable 'old_payload' is uninitialized when used here [-Werror,-Wuninitialized] 368 | new_payload, old_payload); | ^~~ drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:344:61: note: initialize the variable 'old_payload' to silence this warning 344 | struct drm_dp_mst_atomic_payload *new_payload, *old_payload; |^ | = NULL 1 error generated. This variable is not required outside of this function so allocate old_payload on the stack and pass it by reference to dm_helpers_construct_old_payload(), resolving the warning. Closes: https://github.com/ClangBuiltLinux/linux/issues/1931 Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 9ad509279b0a..c4c35f6844f4 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -341,7 +341,7 @@ bool dm_helpers_dp_mst_send_payload_allocation( struct amdgpu_dm_connector *aconnector; struct drm_dp_mst_topology_state *mst_state; struct drm_dp_mst_topology_mgr *mst_mgr; - struct drm_dp_mst_atomic_payload *new_payload, *old_payload; + struct drm_dp_mst_atomic_payload *new_payload, old_payload; enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD; enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD; int ret = 0; @@ -365,8 +365,8 @@ bool dm_helpers_dp_mst_send_payload_allocation( ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, new_payload); } else { dm_helpers_construct_old_payload(stream->link, mst_state->pbn_div, -new_payload, old_payload); - drm_dp_remove_payload_part2(mst_mgr, mst_state, old_payload, new_payload); +new_payload, _payload); + drm_dp_remove_payload_part2(mst_mgr, mst_state, _payload, new_payload); } if (ret) { --- base-commit: 8569c31545385195bdb0c021124e68336e91c693 change-id: 20230913-fix-wuninitialized-dm_helpers_dp_mst_send_payload_allocation-c37b33aaad18 Best regards, -- Nathan Chancellor
Re: [PATCH] backlight: lp855x: Drop ret variable in brightness change function
On Wed, Aug 09, 2023 at 01:42:16PM +0200, Artur Weber wrote: > Fixes the following warning: > > drivers/video/backlight/lp855x_bl.c:252:7: warning: variable 'ret' is used > uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] > > Signed-off-by: Artur Weber > Fixes: 5145531be5fb ("backlight: lp855x: Catch errors when changing > brightness") > Reported-by: kernel test robot > Closes: > https://lore.kernel.org/oe-kbuild-all/202308091728.nejhgupp-...@intel.com/ Reviewed-by: Nathan Chancellor > --- > drivers/video/backlight/lp855x_bl.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/video/backlight/lp855x_bl.c > b/drivers/video/backlight/lp855x_bl.c > index 61a7f45bfad8..da1f124db69c 100644 > --- a/drivers/video/backlight/lp855x_bl.c > +++ b/drivers/video/backlight/lp855x_bl.c > @@ -241,19 +241,17 @@ static int lp855x_bl_update_status(struct > backlight_device *bl) > { > struct lp855x *lp = bl_get_data(bl); > int brightness = bl->props.brightness; > - int ret; > > if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) > brightness = 0; > > if (lp->mode == PWM_BASED) > - ret = lp855x_pwm_ctrl(lp, brightness, > + return lp855x_pwm_ctrl(lp, brightness, > bl->props.max_brightness); > else if (lp->mode == REGISTER_BASED) > - ret = lp855x_write_byte(lp, lp->cfg->reg_brightness, > + return lp855x_write_byte(lp, lp->cfg->reg_brightness, > (u8)brightness); > - > - return ret; > + return -EINVAL; > } > > static const struct backlight_ops lp855x_bl_ops = { > > base-commit: 21ef7b1e17d039053edaeaf41142423810572741 > -- > 2.41.0 >
Re: [PATCH 1/2] drm/exec: use unique instead of local label
On Thu, Aug 10, 2023 at 08:48:05AM +0200, Christian König wrote: > Am 10.08.23 um 08:40 schrieb Boris Brezillon: > > On Wed, 9 Aug 2023 08:37:55 -0700 > > Nathan Chancellor wrote: > > > > > Hi Christian, > > > > > > Can this be applied to drm-misc? Other drivers are starting to make use > > > of this API and our builds with clang-17 and clang-18 have been broken > > > for some time due to this. > > Queued to drm-misc-next. > > Sorry for the delay I have been on vacation last week and haven't yet > catched up to this point in my mails. No worries, 'tis the season :) hope it was a good time and thank you both for getting this fixed! Cheers, Nathan
Re: [PATCH 1/2] drm/exec: use unique instead of local label
Hi Christian, Can this be applied to drm-misc? Other drivers are starting to make use of this API and our builds with clang-17 and clang-18 have been broken for some time due to this. Cheers, Nathan On Mon, Jul 31, 2023 at 02:36:24PM +0200, Christian König wrote: > GCC forbids to jump to labels in loop conditions and a new clang > check stumbled over this. > > So instead using a local label inside the loop condition use an > unique label outside of it. > > Fixes: commit 09593216bff1 ("drm: execution context for GEM buffers v7") > Link: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html > Link: https://github.com/ClangBuiltLinux/linux/issues/1890 > Link: > https://github.com/llvm/llvm-project/commit/20219106060208f0c2f5d096eb3aed7b712f5067 > Reported-by: Nathan Chancellor > Reported-by: Naresh Kamboju > CC: Boris Brezillon > Signed-off-by: Christian König > --- > include/drm/drm_exec.h | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h > index 73205afec162..e0462361adf9 100644 > --- a/include/drm/drm_exec.h > +++ b/include/drm/drm_exec.h > @@ -3,6 +3,7 @@ > #ifndef __DRM_EXEC_H__ > #define __DRM_EXEC_H__ > > +#include > #include > > #define DRM_EXEC_INTERRUPTIBLE_WAIT BIT(0) > @@ -74,13 +75,12 @@ struct drm_exec { > * Since labels can't be defined local to the loops body we use a jump > pointer > * to make sure that the retry is only used from within the loops body. > */ > -#define drm_exec_until_all_locked(exec) \ > - for (void *__drm_exec_retry_ptr; ({ \ > - __label__ __drm_exec_retry; \ > -__drm_exec_retry:\ > - __drm_exec_retry_ptr = &&__drm_exec_retry; \ > - (void)__drm_exec_retry_ptr; \ > - drm_exec_cleanup(exec); \ > +#define drm_exec_until_all_locked(exec) > \ > +__PASTE(__drm_exec_, __LINE__): > \ > + for (void *__drm_exec_retry_ptr; ({ \ > + __drm_exec_retry_ptr = &&__PASTE(__drm_exec_, __LINE__);\ > + (void)__drm_exec_retry_ptr; \ > + drm_exec_cleanup(exec); \ > });) > > /** > -- > 2.34.1 >
Re: [PATCH 1/2] drm/exec: use unique instead of local label
On Mon, Jul 31, 2023 at 02:36:24PM +0200, Christian König wrote: > GCC forbids to jump to labels in loop conditions and a new clang > check stumbled over this. > > So instead using a local label inside the loop condition use an > unique label outside of it. > > Fixes: commit 09593216bff1 ("drm: execution context for GEM buffers v7") > Link: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html > Link: https://github.com/ClangBuiltLinux/linux/issues/1890 > Link: > https://github.com/llvm/llvm-project/commit/20219106060208f0c2f5d096eb3aed7b712f5067 > Reported-by: Nathan Chancellor > Reported-by: Naresh Kamboju > CC: Boris Brezillon > Signed-off-by: Christian König Passes my build tests and I inspected the preprocessed output to make sure it should work. I ran the KUnit tests, which all pass (although [1] is needed to fix a tangential issue): Tested-by: Nathan Chancellor Thanks for fixing this! [1]: https://lore.kernel.org/20230728183400.306193-1-arthurgri...@riseup.net/ > --- > include/drm/drm_exec.h | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h > index 73205afec162..e0462361adf9 100644 > --- a/include/drm/drm_exec.h > +++ b/include/drm/drm_exec.h > @@ -3,6 +3,7 @@ > #ifndef __DRM_EXEC_H__ > #define __DRM_EXEC_H__ > > +#include > #include > > #define DRM_EXEC_INTERRUPTIBLE_WAIT BIT(0) > @@ -74,13 +75,12 @@ struct drm_exec { > * Since labels can't be defined local to the loops body we use a jump > pointer > * to make sure that the retry is only used from within the loops body. > */ > -#define drm_exec_until_all_locked(exec) \ > - for (void *__drm_exec_retry_ptr; ({ \ > - __label__ __drm_exec_retry; \ > -__drm_exec_retry:\ > - __drm_exec_retry_ptr = &&__drm_exec_retry; \ > - (void)__drm_exec_retry_ptr; \ > - drm_exec_cleanup(exec); \ > +#define drm_exec_until_all_locked(exec) > \ > +__PASTE(__drm_exec_, __LINE__): > \ > + for (void *__drm_exec_retry_ptr; ({ \ > + __drm_exec_retry_ptr = &&__PASTE(__drm_exec_, __LINE__);\ > + (void)__drm_exec_retry_ptr; \ > + drm_exec_cleanup(exec); \ > });) > > /** > -- > 2.34.1 > >
Re: [PATCH] drm/radeon: Prefer 'unsigned int' to bare use of 'unsigned'
On Sat, Jul 29, 2023 at 09:12:05PM +0700, Bagas Sanjaya wrote: > On Fri, Jul 28, 2023 at 10:35:19PM +0800, 孙冉 wrote: > > WARNING: Prefer 'unsigned int' to bare use of 'unsigned' > > > > Signed-off-by: Ran Sun > > Your From: address != SoB identity While the comment below is a completely valid complaint, I think this comment is being rather pedantic. Google Translate will tell you that 孙冉 is Sun Ran, so while the name does not strictly match, it is clearly the same... > > --- > > drivers/gpu/drm/radeon/radeon_object.h | 8 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/radeon/radeon_object.h > > b/drivers/gpu/drm/radeon/radeon_object.h > > index 39cc87a59a9a..9b55a7103cfd 100644 > > --- a/drivers/gpu/drm/radeon/radeon_object.h > > +++ b/drivers/gpu/drm/radeon/radeon_object.h > > @@ -37,7 +37,7 @@ > > * > > * Returns corresponding domain of the ttm mem_type > > */ > > -static inline unsigned radeon_mem_type_to_domain(u32 mem_type) > > +static inline unsigned int radeon_mem_type_to_domain(u32 mem_type) > > { > > switch (mem_type) { > > case TTM_PL_VRAM: > > @@ -112,12 +112,12 @@ static inline unsigned long radeon_bo_size(struct > > radeon_bo *bo) > > return bo->tbo.base.size; > > } > > > > -static inline unsigned radeon_bo_ngpu_pages(struct radeon_bo *bo) > > +static inline unsigned int radeon_bo_ngpu_pages(struct radeon_bo *bo) > > { > > return bo->tbo.base.size / RADEON_GPU_PAGE_SIZE; > > } > > > > -static inline unsigned radeon_bo_gpu_page_alignment(struct radeon_bo *bo) > > +static inline unsigned int radeon_bo_gpu_page_alignment(struct radeon_bo > > *bo) > > { > > return (bo->tbo.page_alignment << PAGE_SHIFT) / RADEON_GPU_PAGE_SIZE; > > } > > @@ -189,7 +189,7 @@ static inline void *radeon_sa_bo_cpu_addr(struct > > drm_suballoc *sa_bo) > > > > extern int radeon_sa_bo_manager_init(struct radeon_device *rdev, > > struct radeon_sa_manager *sa_manager, > > - unsigned size, u32 align, u32 domain, > > + unsigned int size, u32 align, u32 domain, > > u32 flags); > > extern void radeon_sa_bo_manager_fini(struct radeon_device *rdev, > >struct radeon_sa_manager *sa_manager); > > The patch is whitespace-corrupted. Use git-send-email(1) to submit patches. > Also, your patch is also MIME-encoded, hence the corruption. > > To Alex: Please don't apply this patch due to reasons above. > > Thanks. > > -- > An old man doll... just what I always wanted! - Clara
Re: [PATCH v2] drm: fix indirect goto into statement expression UB
+ people from trailers of 09593216bff1 On Thu, Jul 27, 2023 at 03:50:58PM -0700, ndesaulni...@google.com wrote: > A new diagnostic in clang-17 now produces the following build error: > > drivers/gpu/drm/tests/drm_exec_test.c:41:3: error: cannot jump from this > indirect goto statement to one of its possible targets >41 | drm_exec_retry_on_contention(); > | ^ > include/drm/drm_exec.h:96:4: note: expanded from macro > 'drm_exec_retry_on_contention' >96 | goto *__drm_exec_retry_ptr; > | ^ > drivers/gpu/drm/tests/drm_exec_test.c:39:2: note: possible target of > indirect goto statement >39 | drm_exec_until_all_locked() { > | ^ > include/drm/drm_exec.h:79:33: note: expanded from macro > 'drm_exec_until_all_locked' >79 | __label__ __drm_exec_retry; > drivers/gpu/drm/tests/drm_exec_test.c:39:2: note: jump enters a > statement expression > > The GCC manually currently states that: ^ manual > >> Jumping into a statement expression with a computed goto (see Labels > >> as Values) has undefined behavior. > > So the diagnostic appears correct, even if codegen happened to produce > working code. > > Looking closer at this code, while the original combination of statement > expression, local label, and computed/indirect goto GNU C expressions > were clever, a simple while loop and continue block might have sufficed. > > This approach might not work as expected if drm_exec_until_all_locked > "loops" can be nested, but that doesn't appear to be an existing use > case in the codebase. > > Fixes: commit 09593216bff1 ("drm: execution context for GEM buffers v7") > Link: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html > Link: https://github.com/ClangBuiltLinux/linux/issues/1890 > Link: > https://github.com/llvm/llvm-project/commit/20219106060208f0c2f5d096eb3aed7b712f5067 > Reported-by: Nathan Chancellor > Reported-by: Naresh Kamboju > Signed-off-by: Nick Desaulniers Thanks for the patch! Tested-by: Nathan Chancellor # build > --- > Changes in v2: > Fix the continue to be outside of the do while > - Link to v1: > https://lore.kernel.org/r/20230727-amdgpu-v1-1-a95690e75...@google.com > --- > include/drm/drm_exec.h | 21 + > 1 file changed, 5 insertions(+), 16 deletions(-) > > diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h > index 73205afec162..fa1cc5c3d021 100644 > --- a/include/drm/drm_exec.h > +++ b/include/drm/drm_exec.h > @@ -70,18 +70,8 @@ struct drm_exec { > * Core functionality of the drm_exec object. Loops until all GEM objects are > * locked and no more contention exists. At the beginning of the loop it is > * guaranteed that no GEM object is locked. > - * > - * Since labels can't be defined local to the loops body we use a jump > pointer > - * to make sure that the retry is only used from within the loops body. > */ > -#define drm_exec_until_all_locked(exec) \ > - for (void *__drm_exec_retry_ptr; ({ \ > - __label__ __drm_exec_retry; \ > -__drm_exec_retry:\ > - __drm_exec_retry_ptr = &&__drm_exec_retry; \ > - (void)__drm_exec_retry_ptr; \ > - drm_exec_cleanup(exec); \ > - });) > +#define drm_exec_until_all_locked(exec) while(drm_exec_cleanup(exec)) > > /** > * drm_exec_retry_on_contention - restart the loop to grap all locks > @@ -90,11 +80,10 @@ __drm_exec_retry: > \ > * Control flow helper to continue when a contention was detected and we > need to > * clean up and re-start the loop to prepare all GEM objects. > */ > -#define drm_exec_retry_on_contention(exec) \ > - do {\ > - if (unlikely(drm_exec_is_contended(exec))) \ > - goto *__drm_exec_retry_ptr; \ > - } while (0) > +#define drm_exec_retry_on_contention(exec) \ > + if (unlikely(drm_exec_is_contended(exec))) \ > + continue; \ > + do {} while (0) > > /** > * drm_exec_is_contended - check for contention > > --- > base-commit: 451cc82bd11eb6a374f4dbcfc1cf007eafea91ab > change-id: 20230727-amdgpu-93c0e5302951 > > Best regards, > -- > Nick Desaulniers >
Re: [PATCH 1/2] drm/v3d: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()
Hi Maira, On Thu, Jul 27, 2023 at 11:01:27AM -0300, Maira Canal wrote: > Hi Nathan, > > On 7/18/23 18:44, Nathan Chancellor wrote: > > A proposed update to clang's -Wconstant-logical-operand to warn when the > > left hand side is a constant shows the following instance in > > nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ, > > such as CONFIG_HZ=300: > > > >In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12: > >drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with > > constant operand [-Wconstant-logical-operand] > > 343 | if (NSEC_PER_SEC % HZ && > > | ~ ^ > >drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise > > operation > > 343 | if (NSEC_PER_SEC % HZ && > > | ^~ > > | & > >drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence > > this warning > >1 warning generated. > > > > Turn this into an explicit comparison against zero to make the > > expression a boolean to make it clear this should be a logical check, > > not a bitwise one. > > > > Link: https://reviews.llvm.org/D142609 > > Signed-off-by: Nathan Chancellor > > Reviewed-by: Maíra Canal > > Thanks for all the hard work with clang! Let me know if you need someone > to push it to drm-misc-next. Yes I will, I do not have drm commit rights. I think the i915 patch can go to drm-misc as well with Tvrtko's ack. Thanks a lot for taking a look! Cheers, Nathan > Best Regards, > - Maíra > > > --- > > drivers/gpu/drm/v3d/v3d_drv.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h > > index b74b1351bfc8..7f664a4b2a75 100644 > > --- a/drivers/gpu/drm/v3d/v3d_drv.h > > +++ b/drivers/gpu/drm/v3d/v3d_drv.h > > @@ -340,7 +340,7 @@ struct v3d_submit_ext { > > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n) > > { > > /* nsecs_to_jiffies64() does not guard against overflow */ > > - if (NSEC_PER_SEC % HZ && > > + if ((NSEC_PER_SEC % HZ) != 0 && > > div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ) > > return MAX_JIFFY_OFFSET; > >
Re: [PATCH 1/6] drm: execution context for GEM buffers v7
Hi Christian, On Tue, Jul 11, 2023 at 03:31:17PM +0200, Christian König wrote: > This adds the infrastructure for an execution context for GEM buffers > which is similar to the existing TTMs execbuf util and intended to replace > it in the long term. > > The basic functionality is that we abstracts the necessary loop to lock > many different GEM buffers with automated deadlock and duplicate handling. > > v2: drop xarray and use dynamic resized array instead, the locking > overhead is unnecessary and measurable. > v3: drop duplicate tracking, radeon is really the only one needing that. > v4: fixes issues pointed out by Danilo, some typos in comments and a > helper for lock arrays of GEM objects. > v5: some suggestions by Boris Brezillon, especially just use one retry > macro, drop loop in prepare_array, use flags instead of bool > v6: minor changes suggested by Thomas, Boris and Danilo > v7: minor typos pointed out by checkpatch.pl fixed > > Signed-off-by: Christian König > Reviewed-by: Boris Brezillon > Reviewed-by: Danilo Krummrich > Tested-by: Danilo Krummrich > diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h > new file mode 100644 > index ..73205afec162 > --- /dev/null > +++ b/include/drm/drm_exec.h > + * Since labels can't be defined local to the loops body we use a jump > pointer > + * to make sure that the retry is only used from within the loops body. > + */ > +#define drm_exec_until_all_locked(exec) \ > + for (void *__drm_exec_retry_ptr; ({ \ > + __label__ __drm_exec_retry; \ > +__drm_exec_retry:\ > + __drm_exec_retry_ptr = &&__drm_exec_retry; \ > + (void)__drm_exec_retry_ptr; \ > + drm_exec_cleanup(exec); \ > + });) > + > +/** > + * drm_exec_retry_on_contention - restart the loop to grap all locks > + * @exec: drm_exec object > + * > + * Control flow helper to continue when a contention was detected and we > need to > + * clean up and re-start the loop to prepare all GEM objects. > + */ > +#define drm_exec_retry_on_contention(exec) \ > + do {\ > + if (unlikely(drm_exec_is_contended(exec))) \ > + goto *__drm_exec_retry_ptr; \ > + } while (0) This construct of using a label as a value and goto to jump into a GNU C statement expression is explicitly documented in GCC's manual [1] as undefined behavior: "Jumping into a statement expression with a computed goto (see Labels as Values [2]) has undefined behavior. " A recent change in clang [3] to prohibit this altogether points this out, so builds using clang-17 and newer will break with this change applied: drivers/gpu/drm/tests/drm_exec_test.c:41:3: error: cannot jump from this indirect goto statement to one of its possible targets 41 | drm_exec_retry_on_contention(); | ^ include/drm/drm_exec.h:96:4: note: expanded from macro 'drm_exec_retry_on_contention' 96 | goto *__drm_exec_retry_ptr; \ | ^ drivers/gpu/drm/tests/drm_exec_test.c:39:2: note: possible target of indirect goto statement 39 | drm_exec_until_all_locked() { | ^ include/drm/drm_exec.h:79:33: note: expanded from macro 'drm_exec_until_all_locked' 79 | __label__ __drm_exec_retry; \ | ^ drivers/gpu/drm/tests/drm_exec_test.c:39:2: note: jump enters a statement expression It seems like if this construct works, it is by chance, although I am not sure if there is another solution. [1]: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html [2]: https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html [3]: https://github.com/llvm/llvm-project/commit/20219106060208f0c2f5d096eb3aed7b712f5067 Cheers, Nathan
Re: [PATCH 2/2] drm/i915: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()
On Thu, Jul 20, 2023 at 09:43:05AM +0100, Tvrtko Ursulin wrote: > > On 18/07/2023 22:44, Nathan Chancellor wrote: > > A proposed update to clang's -Wconstant-logical-operand to warn when the > > left hand side is a constant shows the following instance in > > nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ, > > such as CONFIG_HZ=300: > > > >drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: warning: use of logical > > '&&' with constant operand [-Wconstant-logical-operand] > > 189 | if (NSEC_PER_SEC % HZ && > > | ~ ^ > >drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: note: use '&' for a > > bitwise operation > > 189 | if (NSEC_PER_SEC % HZ && > > | ^~ > > | & > >drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: note: remove constant > > to silence this warning > >1 warning generated. > > > > Turn this into an explicit comparison against zero to make the > > expression a boolean to make it clear this should be a logical check, > > not a bitwise one. > > So -Wconstant-logical-operand only triggers when it is a > constant but not zero constant? Why does that make sense is not > a kludge to avoid too much noise? Yes, the warning purposefully does not trigger when the constant is a 1 or 0 (as those are usually indicative of an intentional logical operation): https://github.com/llvm/llvm-project/blob/dfdfd306cfaf54fbc43e2d5eb36489dac3eb9976/clang/lib/Sema/SemaExpr.cpp#L13917-L13919 In this case, it is 100, so I kind of understand why this might be ambiguous to the compiler. > Personally, it all feels a bit over the top as a warning, > since code in both cases should optimise away. And we may end I do not necessarily disagree, as you can see from the differential review that I linked in the message, but I also understand it is a fine line to tread when writing compiler warnings between wanting to catch as many potential problems as possible and having too much noise for developers to sift through. I think this is erring on the side of caution. > up papering over it if it becomes a default. diagtool tree tells me this warning is already on by default. > Then again this patch IMO does make the code more readable, so I think so too. > I am happy to take this one via our tree. Or either give ack to > bring it in via drm-misc-next: > > Acked-by: Tvrtko Ursulin > > Let me know which route works best. Thanks for the feedback! Either route is fine with me but if the v3d patch is going to go in via drm-misc-next, it seems like it would not be too much trouble to push this one with it. Cheers, Nathan
[PATCH 0/2] Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()
Hi all, A proposed update to clang's -Wconstant-logical-operand [1] to warn when the left hand side is a constant as well now triggers with the modulo expression in nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ, such as CONFIG_HZ=300: drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand] 189 | if (NSEC_PER_SEC % HZ && | ~ ^ drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: note: use '&' for a bitwise operation 189 | if (NSEC_PER_SEC % HZ && | ^~ | & drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: note: remove constant to silence this warning 1 warning generated. In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12: drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand] 343 | if (NSEC_PER_SEC % HZ && | ~ ^ drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation 343 | if (NSEC_PER_SEC % HZ && | ^~ | & drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this warning 1 warning generated. These patches add an explicit comparison to zero to make the expression a boolean, which clears up the warning. The patches have no real dependency on each other but I felt like they made send together since it is the same code. If these could go into mainline sooner rather than later to avoid breaking builds that can hit this with CONFIG_WERROR, that would be nice, but I won't insist since I don't think our own CI has builds that has those conditions, but others might. --- Nathan Chancellor (2): drm/v3d: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout() drm/i915: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout() drivers/gpu/drm/i915/gem/i915_gem_wait.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.h| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) --- base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c change-id: 20230718-nsecs_to_jiffies_timeout-constant-logical-operand-4a944690f3e9 Best regards, -- Nathan Chancellor
[PATCH 2/2] drm/i915: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()
A proposed update to clang's -Wconstant-logical-operand to warn when the left hand side is a constant shows the following instance in nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ, such as CONFIG_HZ=300: drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand] 189 | if (NSEC_PER_SEC % HZ && | ~ ^ drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: note: use '&' for a bitwise operation 189 | if (NSEC_PER_SEC % HZ && | ^~ | & drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: note: remove constant to silence this warning 1 warning generated. Turn this into an explicit comparison against zero to make the expression a boolean to make it clear this should be a logical check, not a bitwise one. Link: https://reviews.llvm.org/D142609 Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/i915/gem/i915_gem_wait.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c b/drivers/gpu/drm/i915/gem/i915_gem_wait.c index 4a33ad2d122b..d4b918fb11ce 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c @@ -186,7 +186,7 @@ i915_gem_object_wait(struct drm_i915_gem_object *obj, static inline unsigned long nsecs_to_jiffies_timeout(const u64 n) { /* nsecs_to_jiffies64() does not guard against overflow */ - if (NSEC_PER_SEC % HZ && + if ((NSEC_PER_SEC % HZ) != 0 && div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ) return MAX_JIFFY_OFFSET; -- 2.41.0
[PATCH 1/2] drm/v3d: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()
A proposed update to clang's -Wconstant-logical-operand to warn when the left hand side is a constant shows the following instance in nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ, such as CONFIG_HZ=300: In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12: drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand] 343 | if (NSEC_PER_SEC % HZ && | ~ ^ drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation 343 | if (NSEC_PER_SEC % HZ && | ^~ | & drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this warning 1 warning generated. Turn this into an explicit comparison against zero to make the expression a boolean to make it clear this should be a logical check, not a bitwise one. Link: https://reviews.llvm.org/D142609 Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/v3d/v3d_drv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h index b74b1351bfc8..7f664a4b2a75 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.h +++ b/drivers/gpu/drm/v3d/v3d_drv.h @@ -340,7 +340,7 @@ struct v3d_submit_ext { static inline unsigned long nsecs_to_jiffies_timeout(const u64 n) { /* nsecs_to_jiffies64() does not guard against overflow */ - if (NSEC_PER_SEC % HZ && + if ((NSEC_PER_SEC % HZ) != 0 && div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ) return MAX_JIFFY_OFFSET; -- 2.41.0
Re: [PATCH] drm/amd/display: Allow building DC with clang on RISC-V
On Mon, Jul 17, 2023 at 03:29:23PM -0700, Samuel Holland wrote: > clang on RISC-V appears to be unaffected by the bug causing excessive > stack usage in calculate_bandwidth(). clang 16 with -fstack-usage > reports a 304 byte stack frame size with CONFIG_ARCH_RV32I, and 512 > bytes with CONFIG_ARCH_RV64I. > > Signed-off-by: Samuel Holland I built ARCH=riscv allmodconfig drivers/gpu/drm/amd/amdgpu/ (confirming that CONFIG_DRM_AMD_DC gets enabled) with LLVM 11 through 17 with and without CONFIG_KASAN=y and I never saw the -Wframe-larger-than instance that this was disabled for, so I agree. Reviewed-by: Nathan Chancellor Tested-by: Nathan Chancellor > > drivers/gpu/drm/amd/display/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/Kconfig > b/drivers/gpu/drm/amd/display/Kconfig > index bf0a655d009e..901d1961b739 100644 > --- a/drivers/gpu/drm/amd/display/Kconfig > +++ b/drivers/gpu/drm/amd/display/Kconfig > @@ -5,7 +5,7 @@ menu "Display Engine Configuration" > config DRM_AMD_DC > bool "AMD DC - Enable new display engine" > default y > - depends on BROKEN || !CC_IS_CLANG || X86_64 || SPARC64 || ARM64 > + depends on BROKEN || !CC_IS_CLANG || ARM64 || RISCV || SPARC64 || X86_64 > select SND_HDA_COMPONENT if SND_HDA_CORE > # !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752 > select DRM_AMD_DC_FP if (X86 || LOONGARCH || (PPC64 && ALTIVEC) || > (ARM64 && KERNEL_MODE_NEON && !CC_IS_CLANG)) > -- > 2.40.1 >
Re: [PATCH v4 18/21] compiler.h: RFC - s/__LINE__/__COUNTER__/ in __UNIQUE_ID fallback
Hi Jim On Thu, Jul 13, 2023 at 10:36:23AM -0600, Jim Cromie wrote: > We currently have 3 defns for __UNIQUE_ID(); gcc and clang are using > __COUNTER__ for real uniqueness, 3rd just uses __LINE__, which should > fail on this (and harder to avoid situations): > > DECLARE_FOO(); DECLARE_FOO(); > > Its 2023, can we haz a no-fallback __UNIQUE_ID ? Yeah, I fail to see how this fallback definition can actually be used after commit 95207db8166a ("Remove Intel compiler support"); even before that, it would be pretty unlikely since icc usage has not been visible for a long time. The kernel only officially supports clang or GCC now, so the definitions of __UNIQUE_ID() in include/linux/compiler-clang.h and include/linux/compiler-gcc.h should always be used because of the include in include/linux/compiler_types.h, right? I think the correct clean up is to just hoist the definition of __UNIQUE_ID() out of the individual compiler headers into the common one here but... > NOTE: > > This also changes __UNIQUE_ID_ to _kaUID_. Ive been getting > lkp-reports of collisions on names which should be unique; this > shouldnt happen on gcc & clang, but does on some older ones, on some > platforms, on some allyes & rand-configs. Like this: > > mips64-linux-ld: > drivers/gpu/drm/display/drm_dp_helper.o:(__dyndbg_class_users+0x0): > multiple definition of `__UNIQUE_ID_ddebug_class_user405'; > drivers/gpu/drm/drm_gem_shmem_helper.o:(__dyndbg_class_users+0x0): > first defined here This problem cannot be addressed with this patch given the above information, no? Seems like that might mean that __COUNTER__ has issues in earlier compilers? Cheers, Nathan > Like above, the collision reports appear to always be 3-digit > counters, which look like line-numbers. Changing to _kaUID_ in this > defn should make it more obvious (in *.i file) when a fallback has > happened. To be clear, I havent seen it yet. Nor have I seen the > multiple-defn problem above since adding this patch. > > Lets see what lkp-robot says about this. > > CC: Luc Van Oostenryck (maintainer:SPARSE > CHECKER) > CC: Nathan Chancellor (supporter:CLANG/LLVM BUILD SUPPORT) > CC: Nick Desaulniers (supporter:CLANG/LLVM BUILD > SUPPORT) > CC: Tom Rix (reviewer:CLANG/LLVM BUILD SUPPORT) > CC: linux-spa...@vger.kernel.org (open list:SPARSE CHECKER) > CC: linux-ker...@vger.kernel.org (open list) > CC: l...@lists.linux.dev (open list:CLANG/LLVM BUILD SUPPORT) > Signed-off-by: Jim Cromie > --- > include/linux/compiler.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index d7779a18b24f..677d6c47cd9e 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -177,9 +177,9 @@ void ftrace_likely_update(struct ftrace_likely_data *f, > int val, > __asm__ ("" : "=r" (var) : "0" (var)) > #endif > > -/* Not-quite-unique ID. */ > +/* JFTI: to fix Not-quite-unique ID */ > #ifndef __UNIQUE_ID > -# define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __LINE__) > +# define __UNIQUE_ID(prefix) __PASTE(__PASTE(_kaUID_, prefix), __COUNTER__) > #endif > > /** > -- > 2.41.0 >
[PATCH 0/2] drm/amdgpu: Fix instances of -Wunused-const-variable with CONFIG_DEBUG_FS=n
Hi all, After commit a25a9dae2067 ("drm/amd/amdgpu: enable W=1 for amdgpu"), I see a few instances of -Wunused-const-variable with configurations that do not enable CONFIG_DEBUG_FS, such as Alpine Linux's. This series includes two patches to resolve each warning I see. --- Nathan Chancellor (2): drm/amdgpu: Remove CONFIG_DEBUG_FS guard around body of amdgpu_rap_debugfs_init() drm/amdgpu: Move clocks closer to its only usage in amdgpu_parse_cg_state() drivers/gpu/drm/amd/amdgpu/amdgpu_rap.c | 2 - drivers/gpu/drm/amd/pm/amdgpu_pm.c | 76 - 2 files changed, 38 insertions(+), 40 deletions(-) --- base-commit: d297eedf83f5af96751c0da1e4355c19244a55a2 change-id: 20230615-amdgpu-wunused-const-variable-wo-debugfs-308ce8e17329 Best regards, -- Nathan Chancellor
[PATCH 2/2] drm/amdgpu: Move clocks closer to its only usage in amdgpu_parse_cg_state()
After commit a25a9dae2067 ("drm/amd/amdgpu: enable W=1 for amdgpu"), there is an instance of -Wunused-const-variable when CONFIG_DEBUG_FS is disabled: drivers/gpu/drm/amd/amdgpu/../pm/amdgpu_pm.c:38:34: error: unused variable 'clocks' [-Werror,-Wunused-const-variable] 38 | static const struct cg_flag_name clocks[] = { | ^ 1 error generated. clocks is only used when CONFIG_DEBUG_FS is set, so move the definition into the CONFIG_DEBUG_FS block right above its only usage to clear up the warning. Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/pm/amdgpu_pm.c | 76 +++--- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c index a57952b93e73..386ccf11e657 100644 --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c @@ -35,44 +35,6 @@ #include #include -static const struct cg_flag_name clocks[] = { - {AMD_CG_SUPPORT_GFX_FGCG, "Graphics Fine Grain Clock Gating"}, - {AMD_CG_SUPPORT_GFX_MGCG, "Graphics Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_GFX_MGLS, "Graphics Medium Grain memory Light Sleep"}, - {AMD_CG_SUPPORT_GFX_CGCG, "Graphics Coarse Grain Clock Gating"}, - {AMD_CG_SUPPORT_GFX_CGLS, "Graphics Coarse Grain memory Light Sleep"}, - {AMD_CG_SUPPORT_GFX_CGTS, "Graphics Coarse Grain Tree Shader Clock Gating"}, - {AMD_CG_SUPPORT_GFX_CGTS_LS, "Graphics Coarse Grain Tree Shader Light Sleep"}, - {AMD_CG_SUPPORT_GFX_CP_LS, "Graphics Command Processor Light Sleep"}, - {AMD_CG_SUPPORT_GFX_RLC_LS, "Graphics Run List Controller Light Sleep"}, - {AMD_CG_SUPPORT_GFX_3D_CGCG, "Graphics 3D Coarse Grain Clock Gating"}, - {AMD_CG_SUPPORT_GFX_3D_CGLS, "Graphics 3D Coarse Grain memory Light Sleep"}, - {AMD_CG_SUPPORT_MC_LS, "Memory Controller Light Sleep"}, - {AMD_CG_SUPPORT_MC_MGCG, "Memory Controller Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_SDMA_LS, "System Direct Memory Access Light Sleep"}, - {AMD_CG_SUPPORT_SDMA_MGCG, "System Direct Memory Access Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_BIF_MGCG, "Bus Interface Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_BIF_LS, "Bus Interface Light Sleep"}, - {AMD_CG_SUPPORT_UVD_MGCG, "Unified Video Decoder Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_VCE_MGCG, "Video Compression Engine Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_HDP_LS, "Host Data Path Light Sleep"}, - {AMD_CG_SUPPORT_HDP_MGCG, "Host Data Path Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_DRM_MGCG, "Digital Right Management Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_DRM_LS, "Digital Right Management Light Sleep"}, - {AMD_CG_SUPPORT_ROM_MGCG, "Rom Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_DF_MGCG, "Data Fabric Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_VCN_MGCG, "VCN Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_HDP_DS, "Host Data Path Deep Sleep"}, - {AMD_CG_SUPPORT_HDP_SD, "Host Data Path Shutdown"}, - {AMD_CG_SUPPORT_IH_CG, "Interrupt Handler Clock Gating"}, - {AMD_CG_SUPPORT_JPEG_MGCG, "JPEG Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_REPEATER_FGCG, "Repeater Fine Grain Clock Gating"}, - {AMD_CG_SUPPORT_GFX_PERF_CLK, "Perfmon Clock Gating"}, - {AMD_CG_SUPPORT_ATHUB_MGCG, "Address Translation Hub Medium Grain Clock Gating"}, - {AMD_CG_SUPPORT_ATHUB_LS, "Address Translation Hub Light Sleep"}, - {0, NULL}, -}; - static const struct hwmon_temp_label { enum PP_HWMON_TEMP channel; const char *label; @@ -3684,6 +3646,44 @@ static int amdgpu_debugfs_pm_info_pp(struct seq_file *m, struct amdgpu_device *a return 0; } +static const struct cg_flag_name clocks[] = { + {AMD_CG_SUPPORT_GFX_FGCG, "Graphics Fine Grain Clock Gating"}, + {AMD_CG_SUPPORT_GFX_MGCG, "Graphics Medium Grain Clock Gating"}, + {AMD_CG_SUPPORT_GFX_MGLS, "Graphics Medium Grain memory Light Sleep"}, + {AMD_CG_SUPPORT_GFX_CGCG, "Graphics Coarse Grain Clock Gating"}, + {AMD_CG_SUPPORT_GFX_CGLS, "Graphics Coarse Grain memory Light Sleep"}, + {AMD_CG_SUPPORT_GFX_CGTS, "Graphics Coarse Grain Tree Shader Clock Gating"}, + {AMD_CG_SUPPORT_GFX_CGTS_LS, "Graphics Coarse Grain Tree Shader Light Sleep"}, + {AMD_CG_SUPPORT_GFX_CP_LS, "Graphics Command Processor Light Sleep"}, + {AMD_CG_SUPPORT_GFX_R
[PATCH 1/2] drm/amdgpu: Remove CONFIG_DEBUG_FS guard around body of amdgpu_rap_debugfs_init()
After commit a25a9dae2067 ("drm/amd/amdgpu: enable W=1 for amdgpu"), there is an instance of -Wunused-const-variable when CONFIG_DEBUG_FS is disabled: drivers/gpu/drm/amd/amdgpu/amdgpu_rap.c:110:37: error: unused variable 'amdgpu_rap_debugfs_ops' [-Werror,-Wunused-const-variable] 110 | static const struct file_operations amdgpu_rap_debugfs_ops = { | ^ 1 error generated. There is no reason for the body of this function to be guarded when CONFIG_DEBUG_FS is disabled, as debugfs_create_file() is a stub that just returns an error pointer in that situation. Remove the preprocessor guards so that the variable never appears unused, while not changing anything at run time. Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/amdgpu/amdgpu_rap.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_rap.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_rap.c index 12010c988c8b..123bcf5c2bb1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_rap.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_rap.c @@ -116,7 +116,6 @@ static const struct file_operations amdgpu_rap_debugfs_ops = { void amdgpu_rap_debugfs_init(struct amdgpu_device *adev) { -#if defined(CONFIG_DEBUG_FS) struct drm_minor *minor = adev_to_drm(adev)->primary; if (!adev->psp.rap_context.context.initialized) @@ -124,5 +123,4 @@ void amdgpu_rap_debugfs_init(struct amdgpu_device *adev) debugfs_create_file("rap_test", S_IWUSR, minor->debugfs_root, adev, _rap_debugfs_ops); -#endif } -- 2.41.0
Re: [PATCH] drm/amdgpu: Wrap -Wunused-but-set-variable in cc-option
On Sat, Jun 10, 2023 at 10:14:05AM +0300, Jani Nikula wrote: > On Thu, 08 Jun 2023, Nathan Chancellor wrote: > > -Wunused-but-set-variable was only supported in clang starting with > > 13.0.0, so earlier versions will emit a warning, which is turned into a > > hard error for the kernel to mirror GCC: > > > > error: unknown warning option '-Wunused-but-set-variable'; did you mean > > '-Wunused-const-variable'? [-Werror,-Wunknown-warning-option] > > > > The minimum supported version of clang for building the kernel is > > 11.0.0, so match the rest of the kernel and wrap > > -Wunused-but-set-variable in a cc-option call, so that it is only used > > when supported by the compiler. > > I wonder if there's a table somewhere listing all the warning options, > which GCC and Clang versions support them, and which versions have them > in -Wall and -Wextra. Would be really useful. I don't think there is anything other than the official documentations for each listing all the warning options. I know each version has its own documentation for comparing warnings between releases but that is obviously tedious. The clang -Wall question is easy enough to answer based on the test case: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0/clang/test/Misc/warning-wall.c https://github.com/llvm/llvm-project/blob/llvmorg-15.0.0/clang/test/Misc/warning-wall.c https://github.com/llvm/llvm-project/blob/llvmorg-14.0.0/clang/test/Misc/warning-wall.c https://github.com/llvm/llvm-project/blob/llvmorg-13.0.0/clang/test/Misc/warning-wall.c https://github.com/llvm/llvm-project/blob/llvmorg-12.0.0/clang/test/Misc/warning-wall.c https://github.com/llvm/llvm-project/blob/llvmorg-11.0.0/clang/test/Misc/warning-wall.c Clang has a tool, diagtool, that can print information about -Wextra, but I do not ship it with the kernel.org LLVM releases, nor does Debian it seems. On a recent clang-17 (the colors don't matter for this exercise): $ diagtool tree -Wextra GREEN = enabled by default YELLOW = disabled by default RED = unimplemented (accepted for GCC compatibility) -Wextra -Wdeprecated-copy -Wdeprecated-copy-with-user-provided-copy -Wmissing-field-initializers -Wignored-qualifiers -Wignored-reference-qualifiers -Winitializer-overrides -Wsemicolon-before-method-body -Wmissing-method-return-type -Wsign-compare -Wunused-parameter -Wunused-but-set-parameter -Wnull-pointer-arithmetic -Wgnu-null-pointer-arithmetic -Wnull-pointer-subtraction -Wempty-init-stmt -Wstring-concatenation -Wfuse-ld-path Maybe some of that can be useful for future travelers. > If there isn't one, it would be really helpful. *wink*. Heh, that does sound like an interesting project but I am not sure I have the bandwidth at the moment to do something like that, especially since the number of warnings that are different between GCC and clang are continuing to dwindle :) Cheers, Nathan > > Closes: https://github.com/ClangBuiltLinux/linux/issues/1869 > > Fixes: a0fd5a5f676c ("drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR") > > Signed-off-by: Nathan Chancellor > > --- > > drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile > > b/drivers/gpu/drm/amd/amdgpu/Makefile > > index 7ee68b1bbfed..86b833085f19 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/Makefile > > +++ b/drivers/gpu/drm/amd/amdgpu/Makefile > > @@ -40,7 +40,7 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \ > > -I$(FULL_AMD_PATH)/amdkfd > > > > subdir-ccflags-y := -Wextra > > -subdir-ccflags-y += -Wunused-but-set-variable > > +subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable) > > subdir-ccflags-y += -Wno-unused-parameter > > subdir-ccflags-y += -Wno-type-limits > > subdir-ccflags-y += -Wno-sign-compare > > > > --- > > base-commit: 6bd4b01e8938779b0d959bdf33949a9aa258a363 > > change-id: > > 20230608-amdgpu-wrap-wunused-but-set-variable-in-cc-option-0be9528ac5c8 > > > > Best regards, > > -- > Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm/amd/amdgpu: enable W=1 for amdgpu
+ Masahiro and linux-kbuild On Fri, Jun 09, 2023 at 12:42:06PM -0400, Hamza Mahfooz wrote: > We have a clean build with W=1 as of > commit 12a15dd589ac ("drm/amd/display/amdgpu_dm/amdgpu_dm_helpers: Move > SYNAPTICS_DEVICE_ID into CONFIG_DRM_AMD_DC_DCN ifdef"). So, let's enable > these checks unconditionally for the entire module to catch these errors > during development. > > Cc: Alex Deucher > Cc: Nathan Chancellor > Signed-off-by: Hamza Mahfooz I think this is fine, especially since it will help catch issues in amdgpu quickly and hopefully encourage developers to fix their problems before they make it to a tree with wider impact lika -next. However, this is now the third place that W=1 has been effectively enabled (i915 and btrfs are the other two I know of) and it would be nice if this was a little more unified, especially since it is not uncommon for the warnings under W=1 to shift around and keeping them unified will make maintainence over the longer term a little easier. I am not sure if this has been brought up in the past and I don't want to hold up this change but I suspect this sentiment of wanting to enable W=1 on a per-subsystem basis is going to continue to grow. Regardless, for clang 11.1.0 to 16.0.5, I see no warnings when building drivers/gpu/drm/amd/amdgpu/ with Arch Linux's configuration or allmodconfig. Reviewed-by: Nathan Chancellor Tested-by: Nathan Chancellor > --- > drivers/gpu/drm/amd/amdgpu/Makefile | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile > b/drivers/gpu/drm/amd/amdgpu/Makefile > index 86b833085f19..8d16f280b695 100644 > --- a/drivers/gpu/drm/amd/amdgpu/Makefile > +++ b/drivers/gpu/drm/amd/amdgpu/Makefile > @@ -40,7 +40,18 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \ > -I$(FULL_AMD_PATH)/amdkfd > > subdir-ccflags-y := -Wextra > -subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable) > +subdir-ccflags-y += -Wunused > +subdir-ccflags-y += -Wmissing-prototypes > +subdir-ccflags-y += -Wmissing-declarations > +subdir-ccflags-y += -Wmissing-include-dirs > +subdir-ccflags-y += -Wold-style-definition > +subdir-ccflags-y += -Wmissing-format-attribute > +# Need this to avoid recursive variable evaluation issues > +cond-flags := $(call cc-option, -Wunused-but-set-variable) \ > + $(call cc-option, -Wunused-const-variable) \ > + $(call cc-option, -Wstringop-truncation) \ > + $(call cc-option, -Wpacked-not-aligned) > +subdir-ccflags-y += $(cond-flags) > subdir-ccflags-y += -Wno-unused-parameter > subdir-ccflags-y += -Wno-type-limits > subdir-ccflags-y += -Wno-sign-compare > -- > 2.40.1 >
[PATCH] drm/amdgpu: Wrap -Wunused-but-set-variable in cc-option
-Wunused-but-set-variable was only supported in clang starting with 13.0.0, so earlier versions will emit a warning, which is turned into a hard error for the kernel to mirror GCC: error: unknown warning option '-Wunused-but-set-variable'; did you mean '-Wunused-const-variable'? [-Werror,-Wunknown-warning-option] The minimum supported version of clang for building the kernel is 11.0.0, so match the rest of the kernel and wrap -Wunused-but-set-variable in a cc-option call, so that it is only used when supported by the compiler. Closes: https://github.com/ClangBuiltLinux/linux/issues/1869 Fixes: a0fd5a5f676c ("drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index 7ee68b1bbfed..86b833085f19 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -40,7 +40,7 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \ -I$(FULL_AMD_PATH)/amdkfd subdir-ccflags-y := -Wextra -subdir-ccflags-y += -Wunused-but-set-variable +subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable) subdir-ccflags-y += -Wno-unused-parameter subdir-ccflags-y += -Wno-type-limits subdir-ccflags-y += -Wno-sign-compare --- base-commit: 6bd4b01e8938779b0d959bdf33949a9aa258a363 change-id: 20230608-amdgpu-wrap-wunused-but-set-variable-in-cc-option-0be9528ac5c8 Best regards, -- Nathan Chancellor
[PATCH] drm/i915/pxp: Fix size_t format specifier in gsccs_send_message()
When building ARCH=i386 allmodconfig, the following warning occurs: In file included from include/linux/device.h:15, from include/linux/node.h:18, from include/linux/cpu.h:17, from include/linux/static_call.h:135, from arch/x86/include/asm/perf_event.h:5, from include/linux/perf_event.h:25, from drivers/gpu/drm/i915/i915_pmu.h:11, from drivers/gpu/drm/i915/gt/intel_engine_types.h:21, from drivers/gpu/drm/i915/gt/intel_context_types.h:18, from drivers/gpu/drm/i915/gem/i915_gem_context_types.h:20, from drivers/gpu/drm/i915/i915_request.h:34, from drivers/gpu/drm/i915/i915_active.h:13, from drivers/gpu/drm/i915/gt/intel_context.h:13, from drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c:8: drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c: In function 'gsccs_send_message': include/drm/drm_print.h:456:39: error: format '%ld' expects argument of type 'long int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Werror=format=] 456 | dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__) | ^~~~ include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap' 110 | _p_func(dev, fmt, ##__VA_ARGS__); \ | ^~~ include/linux/dev_printk.h:146:61: note: in expansion of macro 'dev_fmt' 146 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__) | ^~~ include/drm/drm_print.h:456:9: note: in expansion of macro 'dev_warn' 456 | dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__) | ^~~~ include/drm/drm_print.h:466:9: note: in expansion of macro '__drm_printk' 466 | __drm_printk((drm), warn,, fmt, ##__VA_ARGS__) | ^~~~ drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c:146:17: note: in expansion of macro 'drm_warn' 146 | drm_warn(>drm, "caller with insufficient PXP reply size %u (%ld)\n", | ^~~~ cc1: all warnings being treated as errors Use the '%zu' format specifier, as the variable is a 'size_t'. Fixes: dc9ac125d81f ("drm/i915/pxp: Add GSC-CS backend to send GSC fw messages") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c index 8dc41de3f6f7..a217821eb0fb 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c @@ -143,7 +143,7 @@ gsccs_send_message(struct intel_pxp *pxp, reply_size = header->message_size - sizeof(*header); if (reply_size > msg_out_size_max) { - drm_warn(>drm, "caller with insufficient PXP reply size %u (%ld)\n", + drm_warn(>drm, "caller with insufficient PXP reply size %u (%zu)\n", reply_size, msg_out_size_max); reply_size = msg_out_size_max; } --- base-commit: 08264f85c5c05ecc38d409c84d48cfb00ccd3bc4 change-id: 20230530-i915-pxp-size_t-wformat-1d73ed1f8d23 Best regards, -- Nathan Chancellor
[PATCH 2/2] drm/i915/gt: Fix parameter in gmch_ggtt_insert_{entries,page}()
When building with clang's -Wincompatible-function-pointer-types-strict, the following warnings occur: drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c:102:23: error: incompatible function pointer types assigning to 'void (*)(struct i915_address_space *, dma_addr_t, u64, unsigned int, u32)' (aka 'void (*)(struct i915_address_space *, unsigned int, unsigned long long, unsigned int, unsigned int)') from 'void (struct i915_address_space *, dma_addr_t, u64, enum i915_cache_level, u32)' (aka 'void (struct i915_address_space *, unsigned int, unsigned long long, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict] ggtt->vm.insert_page = gmch_ggtt_insert_page; ^ ~ drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c:103:26: error: incompatible function pointer types assigning to 'void (*)(struct i915_address_space *, struct i915_vma_resource *, unsigned int, u32)' (aka 'void (*)(struct i915_address_space *, struct i915_vma_resource *, unsigned int, unsigned int)') from 'void (struct i915_address_space *, struct i915_vma_resource *, enum i915_cache_level, u32)' (aka 'void (struct i915_address_space *, struct i915_vma_resource *, enum i915_cache_level, unsigned int)') [-Werror, -Wincompatible-function-pointer-types-strict] ggtt->vm.insert_entries = gmch_ggtt_insert_entries; ^ 2 errors generated. The warning is pointing out that while 'enum i915_cache_level' and 'unsigned int' are ABI compatible, these indirect calls will fail clang's kernel Control Flow Integrity (kCFI) checks, as the callback's signature does not exactly match the prototype's signature. To fix this, replace the cache_level parameter with pat_index, as was done in other places within i915 where there is no difference between cache_level and pat_index on certain generations. Fixes: 9275277d5324 ("drm/i915: use pat_index instead of cache_level") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c b/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c index d6a74ae2527b..866c416afb73 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c @@ -18,10 +18,10 @@ static void gmch_ggtt_insert_page(struct i915_address_space *vm, dma_addr_t addr, u64 offset, - enum i915_cache_level cache_level, + unsigned int pat_index, u32 unused) { - unsigned int flags = (cache_level == I915_CACHE_NONE) ? + unsigned int flags = (pat_index == I915_CACHE_NONE) ? AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY; intel_gmch_gtt_insert_page(addr, offset >> PAGE_SHIFT, flags); @@ -29,10 +29,10 @@ static void gmch_ggtt_insert_page(struct i915_address_space *vm, static void gmch_ggtt_insert_entries(struct i915_address_space *vm, struct i915_vma_resource *vma_res, -enum i915_cache_level cache_level, +unsigned int pat_index, u32 unused) { - unsigned int flags = (cache_level == I915_CACHE_NONE) ? + unsigned int flags = (pat_index == I915_CACHE_NONE) ? AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY; intel_gmch_gtt_insert_sg_entries(vma_res->bi.pages, vma_res->start >> PAGE_SHIFT, -- 2.40.1
[PATCH 1/2] drm/i915/gt: Fix second parameter type of pre-gen8 pte_encode callbacks
When booting a kernel compiled with CONFIG_CFI_CLANG (kCFI), there is a CFI failure in ggtt_probe_common() when trying to call hsw_pte_encode() via an indirect call: [5.030027] CFI failure at ggtt_probe_common+0xd1/0x130 [i915] (target: hsw_pte_encode+0x0/0x30 [i915]; expected type: 0xf5c1d0fc) With kCFI, indirect calls are validated against their expected type versus actual type and failures occur when the two types do not match. clang's -Wincompatible-function-pointer-types-strict can catch this at compile time but it is not enabled for the kernel yet: drivers/gpu/drm/i915/gt/intel_ggtt.c:1155:23: error: incompatible function pointer types assigning to 'u64 (*)(dma_addr_t, unsigned int, u32)' (aka 'unsigned long long (*)(unsigned int, unsigned int, unsigned int)') from 'u64 (dma_addr_t, enum i915_cache_level, u32)' (aka 'unsigned long long (unsigned int, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict] ggtt->vm.pte_encode = iris_pte_encode; ^ ~~~ drivers/gpu/drm/i915/gt/intel_ggtt.c:1157:23: error: incompatible function pointer types assigning to 'u64 (*)(dma_addr_t, unsigned int, u32)' (aka 'unsigned long long (*)(unsigned int, unsigned int, unsigned int)') from 'u64 (dma_addr_t, enum i915_cache_level, u32)' (aka 'unsigned long long (unsigned int, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict] ggtt->vm.pte_encode = hsw_pte_encode; ^ ~~ drivers/gpu/drm/i915/gt/intel_ggtt.c:1159:23: error: incompatible function pointer types assigning to 'u64 (*)(dma_addr_t, unsigned int, u32)' (aka 'unsigned long long (*)(unsigned int, unsigned int, unsigned int)') from 'u64 (dma_addr_t, enum i915_cache_level, u32)' (aka 'unsigned long long (unsigned int, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict] ggtt->vm.pte_encode = byt_pte_encode; ^ ~~ drivers/gpu/drm/i915/gt/intel_ggtt.c:1161:23: error: incompatible function pointer types assigning to 'u64 (*)(dma_addr_t, unsigned int, u32)' (aka 'unsigned long long (*)(unsigned int, unsigned int, unsigned int)') from 'u64 (dma_addr_t, enum i915_cache_level, u32)' (aka 'unsigned long long (unsigned int, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict] ggtt->vm.pte_encode = ivb_pte_encode; ^ ~~ drivers/gpu/drm/i915/gt/intel_ggtt.c:1163:23: error: incompatible function pointer types assigning to 'u64 (*)(dma_addr_t, unsigned int, u32)' (aka 'unsigned long long (*)(unsigned int, unsigned int, unsigned int)') from 'u64 (dma_addr_t, enum i915_cache_level, u32)' (aka 'unsigned long long (unsigned int, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict] ggtt->vm.pte_encode = snb_pte_encode; ^ ~~ 5 errors generated. In this case, the pre-gen8 pte_encode functions have a second parameter type of 'enum i915_cache_level' whereas the function pointer prototype in 'struct i915_address_space' expects a second parameter type of 'unsigned int'. Update the second parameter of the callbacks and the comment above them noting that these statements are still valid, which matches other functions and files, to clear up the kCFI failures at run time. Fixes: 9275277d5324 ("drm/i915: use pat_index instead of cache_level") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 2a7942fac798..122197737ef2 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -1015,16 +1015,16 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) /* * For pre-gen8 platforms pat_index is the same as enum i915_cache_level, - * so these PTE encode functions are left with using cache_level. + * so the switch-case statements in these PTE encode functions are still valid. * See translation table LEGACY_CACHELEVEL. */ static u64 snb_pte_encode(dma_addr_t addr, - enum i915_cache_level level, + unsigned int pat_index, u32 flags) { gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID; - switch (level) { + switch (pat_index) { case I915_CACHE_L3_LLC: case I915_CACHE_LLC: pte |= GEN6_PTE_CACHE_LLC; @@ -1033,19 +1033,19 @@ static u64 snb_pte_encode(dma_addr_t addr, pte |= GEN6_PTE_UNCACH
[PATCH 0/2] drm/i915/gt: Fix recent kCFI violations
Hi all, This series fixes a few clang kernel Control Flow Integrity (kCFI) violations that appear after commit 9275277d5324 ("drm/i915: use pat_index instead of cache_level"). They were found between run time testing on real hardware and compile time testing with -Wincompatible-function-pointer-types-strict (which is not yet enabled for the kernel but I build with it locally to catch new instances). If there are any problems or questions, please let me know. --- Nathan Chancellor (2): drm/i915/gt: Fix second parameter type of pre-gen8 pte_encode callbacks drm/i915/gt: Fix parameter in gmch_ggtt_insert_{entries,page}() drivers/gpu/drm/i915/gt/intel_ggtt.c | 26 +- drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 8 2 files changed, 17 insertions(+), 17 deletions(-) --- base-commit: 08264f85c5c05ecc38d409c84d48cfb00ccd3bc4 change-id: 20230530-i915-gt-cache_level-wincompatible-function-pointer-types-strict-32a5c65249a5 Best regards, -- Nathan Chancellor
Re: [PATCH] drm/amdkfd: remove unused function get_reserved_sdma_queues_bitmap
On Thu, May 25, 2023 at 04:07:59PM -0400, Tom Rix wrote: > clang with W=1 reports > drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:122:24: error: > unused function 'get_reserved_sdma_queues_bitmap' > [-Werror,-Wunused-function] > static inline uint64_t get_reserved_sdma_queues_bitmap(struct > device_queue_manager *dqm) >^ > This function is not used so remove it. > > Signed-off-by: Tom Rix Caused by commit 09a95a85cf3e ("drm/amdkfd: Update SDMA queue management for GFX9.4.3") it seems. You can actually go a step farther and remove the reserved_sdma_queues_bitmap member from 'struct kfd_device_info' because it is now only assigned, never read. $ git grep reserved_sdma_queues_bitmap next-20230525 next:20230525:drivers/gpu/drm/amd/amdkfd/kfd_device.c: kfd->device_info.reserved_sdma_queues_bitmap = 0xFULL; next:20230525:drivers/gpu/drm/amd/amdkfd/kfd_device.c: kfd->device_info.reserved_sdma_queues_bitmap = 0x3ULL; next:20230525:drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c:static inline uint64_t get_reserved_sdma_queues_bitmap(struct device_queue_manager *dqm) next:20230525:drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c:return dqm->dev->kfd->device_info.reserved_sdma_queues_bitmap; next:20230525:drivers/gpu/drm/amd/amdkfd/kfd_priv.h:uint64_t reserved_sdma_queues_bitmap; > --- > drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c > b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c > index 493b4b66f180..2fbd0a96424f 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c > @@ -119,11 +119,6 @@ unsigned int get_num_xgmi_sdma_queues(struct > device_queue_manager *dqm) > dqm->dev->kfd->device_info.num_sdma_queues_per_engine; > } > > -static inline uint64_t get_reserved_sdma_queues_bitmap(struct > device_queue_manager *dqm) > -{ > - return dqm->dev->kfd->device_info.reserved_sdma_queues_bitmap; > -} > - > static void init_sdma_bitmaps(struct device_queue_manager *dqm) > { > bitmap_zero(dqm->sdma_bitmap, KFD_MAX_SDMA_QUEUES); > -- > 2.27.0 >
Re: [PATCH v2] drm/amd/display: enable more strict compile checks
On Thu, May 25, 2023 at 08:37:07AM -0700, Kees Cook wrote: > Hi! > > On Wed, May 24, 2023 at 04:27:31PM -0400, Hamza Mahfooz wrote: > > + Kees > > > > On 5/24/23 15:50, Alex Deucher wrote: > > > On Wed, May 24, 2023 at 3:46 PM Felix Kuehling > > > wrote: > > > > > > > > Sure, I think we tried enabling warnings as errors before and had to > > > > revert it because of weird compiler quirks or the variety of compiler > > > > versions that need to be supported. > > > > > > > > Alex, are you planning to upstream this, or is this only to enforce more > > > > internal discipline about not ignoring warnings? > > > > > > I'd like to upstream it. Upstream already has CONFIG_WERROR as a > > > config option, but it's been problematic to enable in CI because of > > > various breakages outside of the driver and in different compilers. > > > That said, I don't know how much trouble enabling it will cause with > > > various compilers in the wild. > > -Wmisleading-indentation is already part of -Wall, so this is globally > enabled already. > > -Wunused is enabled under W=1, and it's pretty noisy still. If you can > get builds clean in drm, that'll be a good step towards getting it > enabled globally. (A middle ground with less to clean up might be > -Wunused-but-set-variable) > > I agree about -Werror: just stick with CONFIG_WERROR instead. There is also W=e, added by commit c77d06e70d59 ("kbuild: support W=e to make build abort in case of warning") in 5.19, which works well for building with configurations that do not have CONFIG_WERROR enabled and avoiding dipping into menuconfig. Unconditionally enabling -Werror with no way to turn it off is just asking for problems over time with new compiler versions, either due to new warnings in -Wall or warnings that have been improved or changed. Should that still be desired, consider doing what i915 and PowerPC have done and add a Kconfig option that can be disabled. Cheers, Nathan
Re: [PATCH] drm/i915: Fix clang -Wimplicit-fallthrough in intel_async_flip_check_hw()
On Wed, May 24, 2023 at 11:32:32AM -0700, Nick Desaulniers wrote: > On Wed, May 24, 2023 at 8:38 AM Nathan Chancellor wrote: > > > > Clang warns: > > > > drivers/gpu/drm/i915/display/intel_display.c:6012:3: error: unannotated > > fall-through between switch labels [-Werror,-Wimplicit-fallthrough] > > case I915_FORMAT_MOD_X_TILED: > > ^ > > drivers/gpu/drm/i915/display/intel_display.c:6012:3: note: insert > > 'break;' to avoid fall-through > > case I915_FORMAT_MOD_X_TILED: > > ^ > > break; > > 1 error generated. > > > > Clang is a little more pedantic than GCC, which does not warn when > > falling through to a case that is just break or return. Clang's version > > is more in line with the kernel's own stance in deprecated.rst, which > > states that all switch/case blocks must end in either break, > > fallthrough, continue, goto, or return. Add the missing break to silence > > the warning. > > > > Fixes: 937859485aef ("drm/i915: Support Async Flip on Linear buffers") > > Reported-by: kernel test robot > > Closes: https://lore.kernel.org/202305241902.uvhtmoxa-...@intel.com/ > > Reported-by: Naresh Kamboju > > Closes: > > https://lore.kernel.org/CA+G9fYv68V3ewK0Qj-syQj7qX-hQr0H1MFL=qfnudoe_j2z...@mail.gmail.com/ > > Signed-off-by: Nathan Chancellor > > Thanks for the patch! I've never seen the closes tag before, that's > new to me. Can you tell me more about it? It is new to me (at least in the context of the kernel) as well. I only used it over Link: because checkpatch.pl told me to: WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #26: Reported-by: kernel test robot Reported-by: Naresh Kamboju WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #27: Reported-by: Naresh Kamboju Signed-off-by: Nathan Chancellor It was Link: for a bit but commit 44c31888098a ("checkpatch: allow Closes tags with links") changed it to Closes:. Looks odd to me but whatever the linter says I suppose. Thanks for the review! Cheers, Nathan > A few more tags > > Reported-by: Tom Rix > Link: https://lore.kernel.org/all/20230523125116.1669057-1-t...@redhat.com/ > Reviewed-by: Nick Desaulniers > > > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 0490c6412ab5..6d49e0ab3e85 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -6008,6 +6008,7 @@ static int intel_async_flip_check_hw(struct > > intel_atomic_state *state, struct in > > plane->base.base.id, > > plane->base.name); > > return -EINVAL; > > } > > + break; > > > > case I915_FORMAT_MOD_X_TILED: > > case I915_FORMAT_MOD_Y_TILED: > > > > --- > > base-commit: 9a2cb1b31c040e2f1b313e2f7921f0f5e6b66d82 > > change-id: > > 20230524-intel_async_flip_check_hw-implicit-fallthrough-c4c40b03802f > > > > Best regards, > > -- > > Nathan Chancellor > > > > > -- > Thanks, > ~Nick Desaulniers
[PATCH] drm/amdgpu: Fix return types of certain NBIOv7.9 callbacks
When building with clang's -Wincompatible-function-pointer-types-strict, which ensures that function pointer signatures match exactly to avoid tripping clang's Control Flow Integrity (kCFI) checks at run time and will eventually be turned on for the kernel, the following instances appear in the NBIOv7.9 code: drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c:465:32: error: incompatible function pointer types initializing 'int (*)(struct amdgpu_device *)' with an expression of type 'enum amdgpu_gfx_partition (struct amdgpu_device *)' [-Werror,-Wincompatible-function-pointer-types-strict] .get_compute_partition_mode = nbio_v7_9_get_compute_partition_mode, ^~~~ drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c:467:31: error: incompatible function pointer types initializing 'u32 (*)(struct amdgpu_device *, u32 *)' (aka 'unsigned int (*)(struct amdgpu_device *, unsigned int *)') with an expression of type 'enum amdgpu_memory_partition (struct amdgpu_device *, u32 *)' (aka 'enum amdgpu_memory_partition (struct amdgpu_device *, unsigned int *)') [-Werror,-Wincompatible-function-pointer-types-strict] .get_memory_partition_mode = nbio_v7_9_get_memory_partition_mode, ^~~ 2 errors generated. Change the return types of these callbacks to match the prototypes to clear up the warning and avoid tripping kCFI at run time. Both functions return a value from ffs(), which is an integer that can fit into either int or unsigned int. Fixes: 11f64eb1472f ("drm/amdgpu: add sysfs node for compute partition mode") Fixes: 41a717ea8afc ("drm/amdgpu: detect current GPU memory partition mode") Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c index e082f6343d20..d19325476752 100644 --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c @@ -382,7 +382,7 @@ static void nbio_v7_9_enable_doorbell_interrupt(struct amdgpu_device *adev, DOORBELL_INTERRUPT_DISABLE, enable ? 0 : 1); } -static enum amdgpu_gfx_partition nbio_v7_9_get_compute_partition_mode(struct amdgpu_device *adev) +static int nbio_v7_9_get_compute_partition_mode(struct amdgpu_device *adev) { u32 tmp, px; @@ -408,8 +408,8 @@ static void nbio_v7_9_set_compute_partition_mode(struct amdgpu_device *adev, WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_PARTITION_COMPUTE_STATUS, tmp); } -static enum amdgpu_memory_partition -nbio_v7_9_get_memory_partition_mode(struct amdgpu_device *adev, u32 *supp_modes) +static u32 nbio_v7_9_get_memory_partition_mode(struct amdgpu_device *adev, + u32 *supp_modes) { u32 tmp; --- base-commit: fd8f7bb391fa9c1979232cb5ab5bedb08abc855d change-id: 20230524-nbio_v7_9-wincompatible-function-pointer-types-strict-c894636ce146 Best regards, -- Nathan Chancellor
Re: next: clang: x86_64: /intel_display.c:6012:3: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
Hi Naresh, On Wed, May 24, 2023 at 12:32:24PM +0530, Naresh Kamboju wrote: > Linux next-20230523 and next-20230524 the x86_64 and i386 builds failed > with clang. > > Reported-by: Linux Kernel Functional Testing > > make --silent --keep-going \ > --jobs=8 O=/home/tuxbuild/.cache/tuxmake/builds/1/build ARCH=x86_64 \ > SRCARCH=x86 CROSS_COMPILE=x86_64-linux-gnu- \ > 'HOSTCC=sccache clang' 'CC=sccache clang' \ >LLVM=1 LLVM_IAS=1 > > drivers/gpu/drm/i915/display/intel_display.c:6012:3: error: > unannotated fall-through between switch labels > [-Werror,-Wimplicit-fallthrough] > case I915_FORMAT_MOD_X_TILED: > ^ > drivers/gpu/drm/i915/display/intel_display.c:6012:3: note: insert > 'break;' to avoid fall-through > case I915_FORMAT_MOD_X_TILED: > ^ > break; > 1 error generated. Thanks for the report, I have sent https://lore.kernel.org/20230524-intel_async_flip_check_hw-implicit-fallthrough-v1-1-83de89e37...@kernel.org/ for this. Cheers, Nathan
[PATCH] drm/i915: Fix clang -Wimplicit-fallthrough in intel_async_flip_check_hw()
Clang warns: drivers/gpu/drm/i915/display/intel_display.c:6012:3: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough] case I915_FORMAT_MOD_X_TILED: ^ drivers/gpu/drm/i915/display/intel_display.c:6012:3: note: insert 'break;' to avoid fall-through case I915_FORMAT_MOD_X_TILED: ^ break; 1 error generated. Clang is a little more pedantic than GCC, which does not warn when falling through to a case that is just break or return. Clang's version is more in line with the kernel's own stance in deprecated.rst, which states that all switch/case blocks must end in either break, fallthrough, continue, goto, or return. Add the missing break to silence the warning. Fixes: 937859485aef ("drm/i915: Support Async Flip on Linear buffers") Reported-by: kernel test robot Closes: https://lore.kernel.org/202305241902.uvhtmoxa-...@intel.com/ Reported-by: Naresh Kamboju Closes: https://lore.kernel.org/CA+G9fYv68V3ewK0Qj-syQj7qX-hQr0H1MFL=qfnudoe_j2z...@mail.gmail.com/ Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/i915/display/intel_display.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 0490c6412ab5..6d49e0ab3e85 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -6008,6 +6008,7 @@ static int intel_async_flip_check_hw(struct intel_atomic_state *state, struct in plane->base.base.id, plane->base.name); return -EINVAL; } + break; case I915_FORMAT_MOD_X_TILED: case I915_FORMAT_MOD_Y_TILED: --- base-commit: 9a2cb1b31c040e2f1b313e2f7921f0f5e6b66d82 change-id: 20230524-intel_async_flip_check_hw-implicit-fallthrough-c4c40b03802f Best regards, -- Nathan Chancellor
Re: [PATCH v5 2/3] drm/panel: Add Samsung S6D7AA0 panel controller driver
Hi Artur, On Fri, May 19, 2023 at 07:03:53PM +0200, Artur Weber wrote: > Initial driver for S6D7AA0-controlled panels. Currently, the following > panels are supported: > > - S6D7AA0-LSL080AL02 (Samsung Galaxy Tab 3 8.0) > - S6D7AA0-LSL080AL03 (Samsung Galaxy Tab A 8.0 2015) > - S6D7AA0-LTL101AT01 (Samsung Galaxy Tab A 9.7 2015) > > It should be possible to extend this driver to work with other panels > using this IC. > > Tested-by: Nikita Travkin #ltl101at01 > Signed-off-by: Artur Weber This change as commit 6810bb390282 ("drm/panel: Add Samsung S6D7AA0 panel controller driver") in -next causes the following build errors with clang and GCC older than 8.x (the kernel supports back to GCC 5.1). With clang: drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c:312:14: error: initializer element is not a compile-time constant .drm_mode = s6d7aa0_lsl080al02_mode, ^~~ drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c:415:14: error: initializer element is not a compile-time constant .drm_mode = s6d7aa0_lsl080al03_mode, ^~~ drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c:443:14: error: initializer element is not a compile-time constant .drm_mode = s6d7aa0_ltl101at01_mode, ^~~ 3 errors generated. With GCC: drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c:312:14: error: initializer element is not constant .drm_mode = s6d7aa0_lsl080al02_mode, ^~~ drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c:312:14: note: (near initialization for 's6d7aa0_lsl080al02_desc.drm_mode') drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c:415:14: error: initializer element is not constant .drm_mode = s6d7aa0_lsl080al03_mode, ^~~ drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c:415:14: note: (near initialization for 's6d7aa0_lsl080al03_desc.drm_mode') drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c:443:14: error: initializer element is not constant .drm_mode = s6d7aa0_ltl101at01_mode, ^~~ drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c:443:14: note: (near initialization for 's6d7aa0_ltl101at01_desc.drm_mode') You can find these toolchains at https://mirrors.edge.kernel.org/pub/tools/crosstool/ and https://mirrors.edge.kernel.org/pub/tools/llvm/ if you need help testing. clang may eventually match GCC's newer behavior but there appears to be some unresolved concerns around the proposed implementation and we have not been able to double back to it: https://reviews.llvm.org/D76096 > +static const struct drm_display_mode s6d7aa0_lsl080al03_mode = { > + .clock = (768 + 18 + 16 + 126) * (1024 + 8 + 2 + 6) * 60 / 1000, > + .hdisplay = 768, > + .hsync_start = 768 + 18, > + .hsync_end = 768 + 18 + 16, > + .htotal = 768 + 18 + 16 + 126, > + .vdisplay = 1024, > + .vsync_start = 1024 + 8, > + .vsync_end = 1024 + 8 + 2, > + .vtotal = 1024 + 8 + 2 + 6, > + .width_mm = 122, > + .height_mm = 163, > +}; > + > +static const struct s6d7aa0_panel_desc s6d7aa0_lsl080al03_desc = { > + .panel_type = S6D7AA0_PANEL_LSL080AL03, > + .init_func = s6d7aa0_lsl080al03_init, > + .off_func = s6d7aa0_lsl080al03_off, > + .drm_mode = s6d7aa0_lsl080al03_mode, > + .mode_flags = MIPI_DSI_MODE_NO_EOT_PACKET, > + .bus_flags = 0, > + > + .has_backlight = true, > + .use_passwd3 = true, > +}; > + > +/* Initialization structures for LTL101AT01 panel */ > + > +static const struct drm_display_mode s6d7aa0_ltl101at01_mode = { > + .clock = (768 + 96 + 16 + 184) * (1024 + 8 + 2 + 6) * 60 / 1000, > + .hdisplay = 768, > + .hsync_start = 768 + 96, > + .hsync_end = 768 + 96 + 16, > + .htotal = 768 + 96 + 16 + 184, > + .vdisplay = 1024, > + .vsync_start = 1024 + 8, > + .vsync_end = 1024 + 8 + 2, > + .vtotal = 1024 + 8 + 2 + 6, > + .width_mm = 148, > + .height_mm = 197, > +}; > + > +static const struct s6d7aa0_panel_desc s6d7aa0_ltl101at01_desc = { > + .panel_type = S6D7AA0_PANEL_LTL101AT01, > + .init_func = s6d7aa0_lsl080al03_init, /* Similar init to LSL080AL03 */ > + .off_func = s6d7aa0_lsl080al03_off, > + .drm_mode = s6d7aa0_ltl101at01_mode, > + .mode_flags = MIPI_DSI_MODE_NO_EOT_PACKET, > + .bus_flags = 0, > + > + .has_backlight = true, > + .use_passwd3 = true, > +}; Cheers, Nathan
Re: [PATCH v2 1/2] phy: mediatek: hdmi: mt8195: fix uninitialized variable usage in pll_calc
On Fri, Apr 14, 2023 at 06:07:46PM +0200, Guillaume Ranquet wrote: > The ret variable in mtk_hdmi_pll_calc() was used unitialized as reported > by the kernel test robot. > > Fix the issue by removing the variable altogether and testing out the > return value of mtk_hdmi_pll_set_hw() > > Fixes: 45810d486bb44 ("phy: mediatek: add support for phy-mtk-hdmi-mt8195") > Reported-by: kernel test robot > Signed-off-by: Guillaume Ranquet Reviewed-by: Nathan Chancellor Can somebody pick this up? It fixes a rather obvious warning, which is breaking clang builds (as evidenced by three versions of the same fix). > --- > drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c | 8 ++-- > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > index abfc077fb0a8..054b73cb31ee 100644 > --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > @@ -213,7 +213,7 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy > *hdmi_phy, struct clk_hw *hw, > u64 tmds_clk, pixel_clk, da_hdmitx21_ref_ck, ns_hdmipll_ck, pcw; > u8 txpredivs[4] = { 2, 4, 6, 12 }; > u32 fbkdiv_low; > - int i, ret; > + int i; > > pixel_clk = rate; > tmds_clk = pixel_clk; > @@ -292,13 +292,9 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy > *hdmi_phy, struct clk_hw *hw, > if (!(digital_div <= 32 && digital_div >= 1)) > return -EINVAL; > > - mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, > + return mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, > PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, > txposdiv, digital_div); > - if (ret) > - return -EINVAL; > - > - return 0; > } > > static int mtk_hdmi_pll_drv_setting(struct clk_hw *hw) > > -- > 2.40.0 >
Re: linux-next: manual merge of the drm-misc tree with the mm-stable tree
On Wed, Apr 19, 2023 at 06:24:37PM +0200, Daniel Vetter wrote: > On Tue, Apr 18, 2023 at 07:34:44PM +0100, Mark Brown wrote: > > On Sun, Apr 16, 2023 at 09:58:50AM +0200, Daniel Vetter wrote: > > > > > Note there was a ppc compile fail, which is why we pushed the ttm revert. > > > That /should/ be fixed now, but would be good if you can confirm? > > > > According to Nathan (CCed) there's still issues with the interaction > > with the PowerPC tree. > > So this revert was supposed to fix this: 56e51681246e ("drm/ttm: revert > "Reduce the number of used allocation orders for TTM pages"") > > If there's anything left then I need to chase that asap since the merge > window will open soon. I think we are talking about two different issues here. My issue is not a compilation failure, it is an incorrect merge resolution that is happening in -next because of two independent changes in the drm and powerpc tree, the thread below should have more information. https://lore.kernel.org/20230413184725.GA3183133@dev-arch.thelio-3990X/ I do not think this is something that either tree can solve independently of each other, -next has to resolve the conflict correctly (which is what I point out in the message above) and a note of it should be passed along to Linus so it can be resolved correctly in mainline when the time comes. Cheers, Nathan
Re: linux-next: manual merge of the drm tree with the powerpc tree
On Tue, Apr 18, 2023 at 07:25:00PM +0100, Mark Brown wrote: > On Tue, Apr 18, 2023 at 11:21:45AM -0700, Nathan Chancellor wrote: > > On Fri, Apr 14, 2023 at 05:55:10PM +0100, Mark Brown wrote: > > > > Done. > > > Thanks a lot, sorry for not saying it sooner! It looks like this > > regressed in next-20230417 and next-20230418 though. > > Someone sent a mail saying they thought they'd fixed the DRM tree - is > that not the case? Does not seem like it: $ git show -s --format='%h ("%s")' 67d5d9f013d6 ("Add linux-next specific files for 20230418") $ git grep DRM_AMD_DC_DCN drivers/gpu/drm/amd/display/Kconfig:select DRM_AMD_DC_DCN if (X86 || (PPC64 && ALTIVEC) || (ARM64 && KERNEL_MODE_NEON && !CC_IS_CLANG)) Cheers, Nathan
Re: linux-next: manual merge of the drm tree with the powerpc tree
On Fri, Apr 14, 2023 at 05:55:10PM +0100, Mark Brown wrote: > On Thu, Apr 13, 2023 at 11:47:25AM -0700, Nathan Chancellor wrote: > > On Wed, Apr 12, 2023 at 11:22:13AM +1000, Stephen Rothwell wrote: > > > select SND_HDA_COMPONENT if SND_HDA_CORE > > # !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752 > > - select DRM_AMD_DC_DCN if (X86 || (PPC64 && ALTIVEC) || (ARM64 && > > KERNEL_MODE_NEON && !CC_IS_CLANG)) > > + select DRM_AMD_DC_FP if (X86 || (PPC64 && ALTIVEC) || (ARM64 && > > KERNEL_MODE_NEON && !CC_IS_CLANG)) > > help > > Choose this option if you want to use the new display engine > > support for AMDGPU. This adds required support for Vega and > > > Please consider resolving this in a future -next update, I was rather > > surprised that my AMD test machine's graphical output was not working > > until I noticed the configuration difference :) > > Done. Thanks a lot, sorry for not saying it sooner! It looks like this regressed in next-20230417 and next-20230418 though. Cheers, Nathan
Re: [PATCH] phy: mediatek: fix returning garbage
On Fri, Apr 14, 2023 at 08:22:53AM -0400, Tom Rix wrote: > clang reports > drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c:298:6: error: variable > 'ret' is uninitialized when used here [-Werror,-Wuninitialized] > if (ret) > ^~~ > ret should have been set by the preceding call to mtk_hdmi_pll_set_hw. > > Fixes: 45810d486bb4 ("phy: mediatek: add support for phy-mtk-hdmi-mt8195") > Signed-off-by: Tom Rix Reviewed-by: Nathan Chancellor Angelo brought up a good point on another patch to fix this issue that we should probably be returning ret instead of unconditionally returning -EINVAL but it sounds like it does not really matter at the moment since mtk_hdmi_pll_set_hw() can only return -EINVAL or 0. https://lore.kernel.org/ada769e0-0141-634f-3753-eb3a50f0e...@collabora.com/ > --- > drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > index abfc077fb0a8..c63294e451d6 100644 > --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > @@ -292,9 +292,9 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy > *hdmi_phy, struct clk_hw *hw, > if (!(digital_div <= 32 && digital_div >= 1)) > return -EINVAL; > > - mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, > - PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, > - txposdiv, digital_div); > + ret = mtk_hdmi_pll_set_hw(hw, PLL_PREDIV, fbkdiv_high, fbkdiv_low, > + PLL_FBKDIV_HS3, posdiv1, posdiv2, txprediv, > + txposdiv, digital_div); > if (ret) > return -EINVAL; > > -- > 2.27.0 >
Re: linux-next: manual merge of the drm tree with the powerpc tree
Hi Mark, On Wed, Apr 12, 2023 at 11:22:13AM +1000, Stephen Rothwell wrote: > Hi all, > > Today's linux-next merge of the drm tree got a conflict in: > > drivers/gpu/drm/amd/display/Kconfig > > between commit: > > 78f0929884d4 ("powerpc/64: Always build with 128-bit long double") > > from the powerpc tree and commit: > > 4652ae7a51b7 ("drm/amd/display: Rename DCN config to FP") > > from the drm tree. > > I fixed it up (I used the powerpc version - with "(PPC64 && ALTIVEC)") > and can carry the fix as necessary. This is now fixed as far as > linux-next is concerned, but any non trivial conflicts should be > mentioned to your upstream maintainer when your tree is submitted for > merging. You may also want to consider cooperating with the maintainer > of the conflicting tree to minimise any particularly complex conflicts. This resolution is not quite right on next-20230412 and next-20230413, as the drm tree's rename was not taken into account on the conflicting line. In other words, I need this diff for everything to work properly. diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig index b990ef80d686..2d8e55e29637 100644 --- a/drivers/gpu/drm/amd/display/Kconfig +++ b/drivers/gpu/drm/amd/display/Kconfig @@ -8,7 +8,7 @@ config DRM_AMD_DC depends on BROKEN || !CC_IS_CLANG || X86_64 || SPARC64 || ARM64 select SND_HDA_COMPONENT if SND_HDA_CORE # !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752 - select DRM_AMD_DC_DCN if (X86 || (PPC64 && ALTIVEC) || (ARM64 && KERNEL_MODE_NEON && !CC_IS_CLANG)) + select DRM_AMD_DC_FP if (X86 || (PPC64 && ALTIVEC) || (ARM64 && KERNEL_MODE_NEON && !CC_IS_CLANG)) help Choose this option if you want to use the new display engine support for AMDGPU. This adds required support for Vega and Please consider resolving this in a future -next update, I was rather surprised that my AMD test machine's graphical output was not working until I noticed the configuration difference :) Cheers, Nathan
Re: [PATCH v2] drm/vblank: Fix for drivers that do not drm_vblank_init()
On Mon, Apr 03, 2023 at 09:03:14AM -0700, Rob Clark wrote: > From: Rob Clark > > This should fix a crash that was reported on ast (and possibly other > drivers which do not initialize vblank). > >fbcon: Taking over console >Unable to handle kernel NULL pointer dereference at virtual address > 0074 >Mem abort info: > ESR = 0x9604 > EC = 0x25: DABT (current EL), IL = 32 bits > SET = 0, FnV = 0 > EA = 0, S1PTW = 0 > FSC = 0x04: level 0 translation fault >Data abort info: > ISV = 0, ISS = 0x0004 > CM = 0, WnR = 0 >user pgtable: 4k pages, 48-bit VAs, pgdp=080009d16000 >[0074] pgd=, p4d= >Internal error: Oops: 9604 [#1] SMP >Modules linked in: ip6table_nat tun nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 > nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct > nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill ip_set > nf_tables nfnetlink qrtr sunrpc binfmt_misc vfat fat xfs snd_usb_audio > snd_hwdep snd_usbmidi_lib snd_seq snd_pcm snd_rawmidi snd_timer > snd_seq_device snd soundcore joydev mc ipmi_ssif ipmi_devintf ipmi_msghandler > arm_spe_pmu arm_cmn arm_dsu_pmu arm_dmc620_pmu cppc_cpufreq loop zram > crct10dif_ce polyval_ce nvme polyval_generic ghash_ce sbsa_gwdt igb nvme_core > ast nvme_common i2c_algo_bit xgene_hwmon gpio_dwapb scsi_dh_rdac scsi_dh_emc > scsi_dh_alua ip6_tables ip_tables dm_multipath fuse >CPU: 12 PID: 469 Comm: kworker/12:1 Not tainted > 6.3.0-rc2-8-gd39e48ca80c0 #1 >Hardware name: ADLINK AVA Developer Platform/AVA Developer Platform, BIOS > TianoCore 2.04.100.07 (SYS: 2.06.20220308) 09/08/2022 >Workqueue: events fbcon_register_existing_fbs >pstate: 2049 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) >pc : drm_crtc_next_vblank_start+0x2c/0x98 >lr : drm_atomic_helper_wait_for_fences+0x90/0x240 >sp : 8d583960 >x29: 8d583960 x28: 07ff8fc187b0 x27: >x26: 07ff99c08c00 x25: 0038 x24: 07ff99c0c000 >x23: 0001 x22: 0038 x21: >x20: 07ff9640a280 x19: x18: >x17: x16: b24d2eece1c0 x15: 003038303178 >x14: 303239310048 x13: x12: >x11: x10: x9 : b24d2eeeaca0 >x8 : 8d583628 x7 : 080077783000 x6 : >x5 : 8d584000 x4 : 07ff99c0c000 x3 : 0130 >x2 : x1 : 8d5839c0 x0 : 07ff99c0cc08 >Call trace: > drm_crtc_next_vblank_start+0x2c/0x98 > drm_atomic_helper_wait_for_fences+0x90/0x240 > drm_atomic_helper_commit+0xb0/0x188 > drm_atomic_commit+0xb0/0xf0 > drm_client_modeset_commit_atomic+0x218/0x280 > drm_client_modeset_commit_locked+0x64/0x1a0 > drm_client_modeset_commit+0x38/0x68 > __drm_fb_helper_restore_fbdev_mode_unlocked+0xb0/0xf8 > drm_fb_helper_set_par+0x44/0x88 > fbcon_init+0x1e0/0x4a8 > visual_init+0xbc/0x118 > do_bind_con_driver.isra.0+0x194/0x3a0 > do_take_over_console+0x50/0x70 > do_fbcon_takeover+0x74/0xf8 > do_fb_registered+0x13c/0x158 > fbcon_register_existing_fbs+0x78/0xc0 > process_one_work+0x1ec/0x478 > worker_thread+0x74/0x418 > kthread+0xec/0x100 > ret_from_fork+0x10/0x20 >Code: f944 b9409013 f940a082 9ba30a73 (b9407662) >---[ end trace ]--- > > v2: Use drm_dev_has_vblank() > > Reported-by: Nathan Chancellor > Fixes: d39e48ca80c0 ("drm/atomic-helper: Set fence deadline for vblank") > Signed-off-by: Rob Clark > Reviewed-by: Thomas Zimmermann Still appears to work for me: Tested-by: Nathan Chancellor > --- > drivers/gpu/drm/drm_vblank.c | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 299fa2a19a90..877e2067534f 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -996,10 +996,16 @@ EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); > int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime) > { > unsigned int pipe = drm_crtc_index(crtc); > - struct drm_vblank_crtc *vblank = >dev->vblank[pipe]; > - struct drm_display_mode *mode = >hwmode; > + struct drm_vblank_crtc *vblank; > + struct drm_display_mode *mode; > u64 vblank_start; > > + if (!drm_dev_has_vblank(crtc->dev)) > + return -EINVAL; > + > + vblank = >dev->vblank[pipe]; > + mode = >hwmode; > + > if (!vblank->framedur_ns || !vblank->linedur_ns) > return -EINVAL; > > -- > 2.39.2 >
Re: [PATCH] drm/vblank: Fix for drivers that do not drm_vblank_init()
On Sat, Apr 01, 2023 at 08:38:02AM -0700, Rob Clark wrote: > From: Rob Clark > > This should fix a crash that was reported on ast (and possibly other > drivers which do not initialize vblank). > >fbcon: Taking over console >Unable to handle kernel NULL pointer dereference at virtual address > 0074 >Mem abort info: > ESR = 0x9604 > EC = 0x25: DABT (current EL), IL = 32 bits > SET = 0, FnV = 0 > EA = 0, S1PTW = 0 > FSC = 0x04: level 0 translation fault >Data abort info: > ISV = 0, ISS = 0x0004 > CM = 0, WnR = 0 >user pgtable: 4k pages, 48-bit VAs, pgdp=080009d16000 >[0074] pgd=, p4d= >Internal error: Oops: 9604 [#1] SMP >Modules linked in: ip6table_nat tun nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 > nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct > nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill ip_set > nf_tables nfnetlink qrtr sunrpc binfmt_misc vfat fat xfs snd_usb_audio > snd_hwdep snd_usbmidi_lib snd_seq snd_pcm snd_rawmidi snd_timer > snd_seq_device snd soundcore joydev mc ipmi_ssif ipmi_devintf ipmi_msghandler > arm_spe_pmu arm_cmn arm_dsu_pmu arm_dmc620_pmu cppc_cpufreq loop zram > crct10dif_ce polyval_ce nvme polyval_generic ghash_ce sbsa_gwdt igb nvme_core > ast nvme_common i2c_algo_bit xgene_hwmon gpio_dwapb scsi_dh_rdac scsi_dh_emc > scsi_dh_alua ip6_tables ip_tables dm_multipath fuse >CPU: 12 PID: 469 Comm: kworker/12:1 Not tainted > 6.3.0-rc2-8-gd39e48ca80c0 #1 >Hardware name: ADLINK AVA Developer Platform/AVA Developer Platform, BIOS > TianoCore 2.04.100.07 (SYS: 2.06.20220308) 09/08/2022 >Workqueue: events fbcon_register_existing_fbs >pstate: 2049 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) >pc : drm_crtc_next_vblank_start+0x2c/0x98 >lr : drm_atomic_helper_wait_for_fences+0x90/0x240 >sp : 8d583960 >x29: 8d583960 x28: 07ff8fc187b0 x27: >x26: 07ff99c08c00 x25: 0038 x24: 07ff99c0c000 >x23: 0001 x22: 0038 x21: >x20: 07ff9640a280 x19: x18: >x17: x16: b24d2eece1c0 x15: 003038303178 >x14: 303239310048 x13: x12: >x11: x10: x9 : b24d2eeeaca0 >x8 : 8d583628 x7 : 080077783000 x6 : >x5 : 8d584000 x4 : 07ff99c0c000 x3 : 0130 >x2 : x1 : 8d5839c0 x0 : 07ff99c0cc08 >Call trace: > drm_crtc_next_vblank_start+0x2c/0x98 > drm_atomic_helper_wait_for_fences+0x90/0x240 > drm_atomic_helper_commit+0xb0/0x188 > drm_atomic_commit+0xb0/0xf0 > drm_client_modeset_commit_atomic+0x218/0x280 > drm_client_modeset_commit_locked+0x64/0x1a0 > drm_client_modeset_commit+0x38/0x68 > __drm_fb_helper_restore_fbdev_mode_unlocked+0xb0/0xf8 > drm_fb_helper_set_par+0x44/0x88 > fbcon_init+0x1e0/0x4a8 > visual_init+0xbc/0x118 > do_bind_con_driver.isra.0+0x194/0x3a0 > do_take_over_console+0x50/0x70 > do_fbcon_takeover+0x74/0xf8 > do_fb_registered+0x13c/0x158 > fbcon_register_existing_fbs+0x78/0xc0 > process_one_work+0x1ec/0x478 > worker_thread+0x74/0x418 > kthread+0xec/0x100 > ret_from_fork+0x10/0x20 >Code: f944 b9409013 f940a082 9ba30a73 (b9407662) >---[ end trace 0000 ]--- > > Reported-by: Nathan Chancellor > Fixes: d39e48ca80c0 ("drm/atomic-helper: Set fence deadline for vblank") > Signed-off-by: Rob Clark Seems to work for me, I no longer see the above crash. Tested-by: Nathan Chancellor > --- > drivers/gpu/drm/drm_vblank.c | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 299fa2a19a90..e98e3cefba3a 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -996,10 +996,16 @@ EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); > int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime) > { > unsigned int pipe = drm_crtc_index(crtc); > - struct drm_vblank_crtc *vblank = >dev->vblank[pipe]; > - struct drm_display_mode *mode = >hwmode; > + struct drm_vblank_crtc *vblank; > + struct drm_display_mode *mode; > u64 vblank_start; > > + if (!crtc->dev->vblank) > + return -EINVAL; > + > + vblank = >dev->vblank[pipe]; > + mode = >hwmode; > + > if (!vblank->framedur_ns || !vblank->linedur_ns) > return -EINVAL; > > -- > 2.39.2 >
Re: [PATCH v10 11/15] drm/atomic-helper: Set fence deadline for vblank
On Fri, Mar 31, 2023 at 03:14:30PM -0700, Rob Clark wrote: > On Fri, Mar 31, 2023 at 1:44 PM Nathan Chancellor wrote: > > > > Hi Rob, > > > > On Wed, Mar 08, 2023 at 07:53:02AM -0800, Rob Clark wrote: > > > From: Rob Clark > > > > > > For an atomic commit updating a single CRTC (ie. a pageflip) calculate > > > the next vblank time, and inform the fence(s) of that deadline. > > > > > > v2: Comment typo fix (danvet) > > > v3: If there are multiple CRTCs, consider the time of the soonest vblank > > > > > > Signed-off-by: Rob Clark > > > Reviewed-by: Daniel Vetter > > > Signed-off-by: Rob Clark > > > > I apologize if this has already been reported or fixed, I searched lore > > but did not find anything. > > > > This change as commit d39e48ca80c0 ("drm/atomic-helper: Set fence > > deadline for vblank") in -next causes a hang while running LTP's > > read_all test on /proc on my Ampere Altra system (it seems it is hanging > > on a pagemap file?). Additionally, I have this splat in dmesg, which > > seems related based on the call stack. > > Hi, I'm not familiar with this hardware.. do you know which drm driver > is used? I can't tell from the call-stack. I think it is drivers/gpu/drm/ast, as I see ast in lsmod? > > [ 20.542591] fbcon: Taking over console > > [ 20.550772] Unable to handle kernel NULL pointer dereference at virtual > > address 0074 > > [ 20.550776] Mem abort info: > > [ 20.550777] ESR = 0x9604 > > [ 20.550779] EC = 0x25: DABT (current EL), IL = 32 bits > > [ 20.550781] SET = 0, FnV = 0 > > [ 20.550782] EA = 0, S1PTW = 0 > > [ 20.550784] FSC = 0x04: level 0 translation fault > > [ 20.550785] Data abort info: > > [ 20.550786] ISV = 0, ISS = 0x0004 > > [ 20.550788] CM = 0, WnR = 0 > > [ 20.550789] user pgtable: 4k pages, 48-bit VAs, pgdp=080009d16000 > > [ 20.550791] [0074] pgd=, p4d= > > [ 20.550796] Internal error: Oops: 9604 [#1] SMP > > [ 20.550800] Modules linked in: ip6table_nat tun nft_fib_inet > > nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 > > nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack > > nf_defrag_ipv6 nf_defrag_ipv4 rfkill ip_set nf_tables nfnetlink qrtr sunrpc > > binfmt_misc vfat fat xfs snd_usb_audio snd_hwdep snd_usbmidi_lib snd_seq > > snd_pcm snd_rawmidi snd_timer snd_seq_device snd soundcore joydev mc > > ipmi_ssif ipmi_devintf ipmi_msghandler arm_spe_pmu arm_cmn arm_dsu_pmu > > arm_dmc620_pmu cppc_cpufreq loop zram crct10dif_ce polyval_ce nvme > > polyval_generic ghash_ce sbsa_gwdt igb nvme_core ast nvme_common > > i2c_algo_bit xgene_hwmon gpio_dwapb scsi_dh_rdac scsi_dh_emc scsi_dh_alua > > ip6_tables ip_tables dm_multipath fuse > > [ 20.550869] CPU: 12 PID: 469 Comm: kworker/12:1 Not tainted > > 6.3.0-rc2-8-gd39e48ca80c0 #1 > > [ 20.550872] Hardware name: ADLINK AVA Developer Platform/AVA Developer > > Platform, BIOS TianoCore 2.04.100.07 (SYS: 2.06.20220308) 09/08/2022 > > [ 20.550875] Workqueue: events fbcon_register_existing_fbs > > [ 20.550884] pstate: 2049 (nzCv daif +PAN -UAO -TCO -DIT -SSBS > > BTYPE=--) > > [ 20.550888] pc : drm_crtc_next_vblank_start+0x2c/0x98 > > [ 20.550894] lr : drm_atomic_helper_wait_for_fences+0x90/0x240 > > [ 20.550898] sp : 8d583960 > > [ 20.550900] x29: 8d583960 x28: 07ff8fc187b0 x27: > > > > [ 20.550904] x26: 07ff99c08c00 x25: 0038 x24: > > 07ff99c0c000 > > [ 20.550908] x23: 0001 x22: 0038 x21: > > > > [ 20.550912] x20: 07ff9640a280 x19: x18: > > > > [ 20.550915] x17: x16: b24d2eece1c0 x15: > > 003038303178 > > [ 20.550919] x14: 303239310048 x13: x12: > > > > [ 20.550923] x11: x10: x9 : > > b24d2eeeaca0 > > [ 20.550926] x8 : 8d583628 x7 : 080077783000 x6 : > > > > [ 20.550930] x5 : 8d584000 x4 : 07ff99c0c000 x3 : > > 0130 > > [ 20.550934] x2 : x1 : 8d5839c0 x0 : > > 07ff99c0cc08 > > [ 20.550937] Call trace: > > [ 20.550939] drm_crtc_next_vblank_start+0x2c/0x98 > > [ 20.550942] drm_atomic_helper_wait_for_fences+0x90/0x240 >
Re: [PATCH v10 11/15] drm/atomic-helper: Set fence deadline for vblank
Hi Rob, On Wed, Mar 08, 2023 at 07:53:02AM -0800, Rob Clark wrote: > From: Rob Clark > > For an atomic commit updating a single CRTC (ie. a pageflip) calculate > the next vblank time, and inform the fence(s) of that deadline. > > v2: Comment typo fix (danvet) > v3: If there are multiple CRTCs, consider the time of the soonest vblank > > Signed-off-by: Rob Clark > Reviewed-by: Daniel Vetter > Signed-off-by: Rob Clark I apologize if this has already been reported or fixed, I searched lore but did not find anything. This change as commit d39e48ca80c0 ("drm/atomic-helper: Set fence deadline for vblank") in -next causes a hang while running LTP's read_all test on /proc on my Ampere Altra system (it seems it is hanging on a pagemap file?). Additionally, I have this splat in dmesg, which seems related based on the call stack. [ 20.542591] fbcon: Taking over console [ 20.550772] Unable to handle kernel NULL pointer dereference at virtual address 0074 [ 20.550776] Mem abort info: [ 20.550777] ESR = 0x9604 [ 20.550779] EC = 0x25: DABT (current EL), IL = 32 bits [ 20.550781] SET = 0, FnV = 0 [ 20.550782] EA = 0, S1PTW = 0 [ 20.550784] FSC = 0x04: level 0 translation fault [ 20.550785] Data abort info: [ 20.550786] ISV = 0, ISS = 0x0004 [ 20.550788] CM = 0, WnR = 0 [ 20.550789] user pgtable: 4k pages, 48-bit VAs, pgdp=080009d16000 [ 20.550791] [0074] pgd=, p4d= [ 20.550796] Internal error: Oops: 9604 [#1] SMP [ 20.550800] Modules linked in: ip6table_nat tun nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill ip_set nf_tables nfnetlink qrtr sunrpc binfmt_misc vfat fat xfs snd_usb_audio snd_hwdep snd_usbmidi_lib snd_seq snd_pcm snd_rawmidi snd_timer snd_seq_device snd soundcore joydev mc ipmi_ssif ipmi_devintf ipmi_msghandler arm_spe_pmu arm_cmn arm_dsu_pmu arm_dmc620_pmu cppc_cpufreq loop zram crct10dif_ce polyval_ce nvme polyval_generic ghash_ce sbsa_gwdt igb nvme_core ast nvme_common i2c_algo_bit xgene_hwmon gpio_dwapb scsi_dh_rdac scsi_dh_emc scsi_dh_alua ip6_tables ip_tables dm_multipath fuse [ 20.550869] CPU: 12 PID: 469 Comm: kworker/12:1 Not tainted 6.3.0-rc2-8-gd39e48ca80c0 #1 [ 20.550872] Hardware name: ADLINK AVA Developer Platform/AVA Developer Platform, BIOS TianoCore 2.04.100.07 (SYS: 2.06.20220308) 09/08/2022 [ 20.550875] Workqueue: events fbcon_register_existing_fbs [ 20.550884] pstate: 2049 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 20.550888] pc : drm_crtc_next_vblank_start+0x2c/0x98 [ 20.550894] lr : drm_atomic_helper_wait_for_fences+0x90/0x240 [ 20.550898] sp : 8d583960 [ 20.550900] x29: 8d583960 x28: 07ff8fc187b0 x27: [ 20.550904] x26: 07ff99c08c00 x25: 0038 x24: 07ff99c0c000 [ 20.550908] x23: 0001 x22: 0038 x21: [ 20.550912] x20: 07ff9640a280 x19: x18: [ 20.550915] x17: x16: b24d2eece1c0 x15: 003038303178 [ 20.550919] x14: 303239310048 x13: x12: [ 20.550923] x11: x10: x9 : b24d2eeeaca0 [ 20.550926] x8 : 8d583628 x7 : 080077783000 x6 : [ 20.550930] x5 : 8d584000 x4 : 07ff99c0c000 x3 : 0130 [ 20.550934] x2 : x1 : 8d5839c0 x0 : 07ff99c0cc08 [ 20.550937] Call trace: [ 20.550939] drm_crtc_next_vblank_start+0x2c/0x98 [ 20.550942] drm_atomic_helper_wait_for_fences+0x90/0x240 [ 20.550946] drm_atomic_helper_commit+0xb0/0x188 [ 20.550949] drm_atomic_commit+0xb0/0xf0 [ 20.550953] drm_client_modeset_commit_atomic+0x218/0x280 [ 20.550957] drm_client_modeset_commit_locked+0x64/0x1a0 [ 20.550961] drm_client_modeset_commit+0x38/0x68 [ 20.550965] __drm_fb_helper_restore_fbdev_mode_unlocked+0xb0/0xf8 [ 20.550970] drm_fb_helper_set_par+0x44/0x88 [ 20.550973] fbcon_init+0x1e0/0x4a8 [ 20.550976] visual_init+0xbc/0x118 [ 20.550981] do_bind_con_driver.isra.0+0x194/0x3a0 [ 20.550984] do_take_over_console+0x50/0x70 [ 20.550987] do_fbcon_takeover+0x74/0xf8 [ 20.550989] do_fb_registered+0x13c/0x158 [ 20.550992] fbcon_register_existing_fbs+0x78/0xc0 [ 20.550995] process_one_work+0x1ec/0x478 [ 20.551000] worker_thread+0x74/0x418 [ 20.551002] kthread+0xec/0x100 [ 20.551005] ret_from_fork+0x10/0x20 [ 20.551011] Code: f944 b9409013 f940a082 9ba30a73 (b9407662) [ 20.551013] ---[ end trace ]--- If there is any additional information that I can provide or patches I can test, I am more than happy to do so. Cheers, Nathan # bad: [4b0f4525dc4fe8af17b3daefe585f0c2eb0fe0a5] Add linux-next specific files for
Re: Linux 6.3-rc3
On Fri, Mar 24, 2023 at 05:23:12PM +0200, Kalle Valo wrote: > Nathan Chancellor writes: > > >> This is nitpicking but it would be nice if the tarball contents wouldn't > >> conflict with each other. Now both llvm-16.0.0-aarch64.tar.gz and > >> llvm-16.0.0-x86_64.tar extract to the same directory llvm-16.0.0 with > >> same binary names. It would be much better if they would extract to > >> llvm-16.0.0-aarch64 and llvm-16.0.0-x86_64, respectively. > >> > >> For example, Arnd's crosstool packages don't conflict with each other: > >> > >> https://mirrors.edge.kernel.org/pub/tools/crosstool/ > > > > I could certainly do that but what is the use case for extracting both? > > You cannot run the aarch64 version on an x86_64 host and vice versa, so > > why bother extracting them? > > Ah, I didn't realise that. I assumed llvm-16.0.0-aarch64.tar.gz was a > cross compiler. I'm sure you documented that in the page but hey who > reads the documentation ;) :) I have adjusted the README to hopefully make that clearer. > > I had figured the architecture would be irrelevant once installed on > > the host, so I opted only to include it in the tarball name. Perhaps I > > should make it clearer that these are the host architectures, not the > > target architectures (because clang is multi-targeted, unlike GCC)? > > Makes sense now. But I still think it's good style that a tarball named > llvm-16.0.0-aarch64.tar.gz extracts to llvm-16.0.0-aarch64. Indeed, I have adjusted it for future builds: https://github.com/nathanchance/env/commit/314837e6706889138121a32140d2acdc7895d390 > >> And maybe request a similar llvm directory under pub/tools to make it > >> more official? :) We now have https://kernel.org/pub/tools/llvm/, which is about as official as we can get I suppose :) https://kernel.org/pub/linux/kernel/people/nathan/llvm/ now points people there. Cheers, Nathan
Re: [PATCH v8 2/2] drm: add kms driver for loongson display controller
On Tue, Mar 28, 2023 at 11:22:50PM +0800, Sui Jingfeng wrote: > HI, > > On 2023/3/28 17:27, kernel test robot wrote: > > Hi Sui, > > > > Thank you for the patch! Perhaps something to improve: > > > > [auto build test WARNING on drm-misc/drm-misc-next] > > [also build test WARNING on linus/master v6.3-rc4 next-20230328] > > [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/Sui-Jingfeng/MAINTAINERS-add-maintainers-for-DRM-LOONGSON-driver/20230320-180408 > > base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next > > patch link: > > https://lore.kernel.org/r/20230320100131.1277034-3-15330273260%40189.cn > > patch subject: [PATCH v8 2/2] drm: add kms driver for loongson display > > controller > > config: i386-allyesconfig > > (https://download.01.org/0day-ci/archive/20230328/202303281754.jwi20j2c-...@intel.com/config) > > compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project > > f28c006a5895fc0e329fe15fead81e37457cb1d1) > > reproduce (this is a W=1 build): > > wget > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > > ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # > > https://github.com/intel-lab-lkp/linux/commit/80b4115f44993f4ebf47b1cb9e8f02953575b977 > > git remote add linux-review https://github.com/intel-lab-lkp/linux > > git fetch --no-tags linux-review > > Sui-Jingfeng/MAINTAINERS-add-maintainers-for-DRM-LOONGSON-driver/20230320-180408 > > git checkout 80b4115f44993f4ebf47b1cb9e8f02953575b977 > > # save the config file > > mkdir build_dir && cp config build_dir/.config > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 > > O=build_dir ARCH=i386 olddefconfig > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 > > O=build_dir ARCH=i386 SHELL=/bin/bash drivers/accel/ > > drivers/gpu/drm/loongson/ drivers/iio/light/ drivers/media/pci/intel/ > > > > If you fix the issue, kindly add following tag where applicable > > | Reported-by: kernel test robot > > | Link: > > https://lore.kernel.org/oe-kbuild-all/202303281754.jwi20j2c-...@intel.com/ > > > > All warnings (new ones prefixed by >>): > > > > > > drivers/gpu/drm/loongson/lsdc_drv.c:232:11: warning: variable 'gpu' is > > > > used uninitialized whenever 'if' condition is false > > > > [-Wsometimes-uninitialized] > > else if (descp->chip == CHIP_LS7A2000) > > ^~~~ > > drivers/gpu/drm/loongson/lsdc_drv.c:235:7: note: uninitialized use > > occurs here > > if (!gpu) { > > ^~~ > > drivers/gpu/drm/loongson/lsdc_drv.c:232:7: note: remove the 'if' if its > > condition is always true > > else if (descp->chip == CHIP_LS7A2000) > > ^ > > drivers/gpu/drm/loongson/lsdc_drv.c:217:21: note: initialize the > > variable 'gpu' to silence this warning > > struct pci_dev *gpu; > >^ > > = NULL > > 1 warning generated. > > -- > > In practice, either descp->chip == CHIP_LS7A2000 or descp->chip == > CHIP_LS7A1000 will be happened at runtime. > > the variable 'gpu' is guaranteed to be initialized when code run at > drivers/gpu/drm/loongson/lsdc_drv.c:235 > > This warnning is almost wrong here. Clang's semantic analysis happens before optimizations, meaning it does not perform interprocedural analysis, so it does not have enough information at this point to tell that. Either just initialize gpu to NULL and let the existing 'if (!gpu)' handle it or add a separate else branch that warns about an unhandled chip value so that it is obvious what needs to be done if someone forgets to update this statement when a new chip is supported by this driver. > > > > drivers/gpu/drm/loongson/lsdc_pll.c:188:14: warning: variable 'diff' is > > > > used uninitialized whenever 'if' condition is false > > > > [-Wsometimes-uninitialized] > > else if (clock_khz < computed) > > ^~~~ > > drivers/gpu/drm/loongson/lsdc_pll.c:191:9: note: uninitialized use > > occurs here > > if (diff < min) { > > ^~~~ > > drivers/gpu/drm/loongson/lsdc_pll.c:188:10: note: remove the 'if' if > > its condition is always true > > else if (clock_khz < computed) > > ^ > > drivers/gpu/drm/loongson/lsdc_pll.c:177:22: note: initialize the > > variable 'diff' to silence
Re: Linux 6.3-rc3
On Fri, Mar 24, 2023 at 12:54:01PM +0200, Kalle Valo wrote: > Nathan Chancellor writes: > > > On Wed, Mar 22, 2023 at 02:44:47PM +0200, Kalle Valo wrote: > >> Nathan Chancellor writes: > >> > >> > Perhaps these would make doing allmodconfig builds with clang more > >> > frequently less painful for you? > >> > > >> > https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/ > >> > >> Thank you, at least for me this is really helpful. > > > > Really glad to hear! I hope this helps make testing and verifying > > changes with clang and LLVM easier for developers and maintainers. > > It really does. And I hope you are able to update these packages in > future as well so that it would be easy to get the latest clang. That is the current plan (I will push 16.0.1, 16.0.2, etc. as they are released), I have a relatively automated process for this going forward. > >> I tried now clang for the first time but seeing a strange problem. > >> > >> I prefer to define the compiler in GNUmakefile so it's easy to change > >> compilers and I don't need to remember the exact command line. So I have > >> this in the top level GNUmakefile (all the rest commented out): > >> > >> LLVM=/opt/clang/llvm-16.0.0/bin/ > >> > >> If I run 'make oldconfig' it seems to use clang but after I run just > >> 'make' it seems to switch back to the host GCC compiler and ask for GCC > >> specific config questions again. Workaround for this seems to be adding > >> 'export LLVM' to GNUmakefile, after that also 'make' uses clang as > >> expected. > > > > Interesting... I just tested with a basic GNUmakefile and everything > > seems to work fine without an export. At the same time, the export > > should not hurt anything, so as long as it works, that is what matters. > > Sure, once I figured out the quirks I can workaround them. I was just > hoping that other users would not have to go through the same hassle as > I did :) > > > If you have any further issues, please do not hesitate to reach out! > > This is nitpicking but it would be nice if the tarball contents wouldn't > conflict with each other. Now both llvm-16.0.0-aarch64.tar.gz and > llvm-16.0.0-x86_64.tar extract to the same directory llvm-16.0.0 with > same binary names. It would be much better if they would extract to > llvm-16.0.0-aarch64 and llvm-16.0.0-x86_64, respectively. > > For example, Arnd's crosstool packages don't conflict with each other: > > https://mirrors.edge.kernel.org/pub/tools/crosstool/ I could certainly do that but what is the use case for extracting both? You cannot run the aarch64 version on an x86_64 host and vice versa, so why bother extracting them? I had figured the architecture would be irrelevant once installed on the host, so I opted only to include it in the tarball name. Perhaps I should make it clearer that these are the host architectures, not the target architectures (because clang is multi-targeted, unlike GCC)? > And maybe request a similar llvm directory under pub/tools to make it > more official? :) Yes, I was talking that over with Nick recently, as having it under a group on kernel.org would make taking over maintainership easier should something happen to me :) Thanks for all the feedback so far, it is much appreciated! Cheers, Nathan
Re: Linux 6.3-rc3
On Wed, Mar 22, 2023 at 09:36:37AM -0700, Nathan Chancellor wrote: > On Wed, Mar 22, 2023 at 02:44:47PM +0200, Kalle Valo wrote: > > Nathan Chancellor writes: > > > > > On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote: > > >> On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor > > >> wrote: > > >> > > > >> > On the clang front, I am still seeing the following warning turned > > >> > error > > >> > for arm64 allmodconfig at least: > > >> > > > >> > drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is > > >> > uninitialized when used here [-Werror,-Wuninitialized] > > >> > if (syncpt_irq < 0) > > >> > ^~ > > >> > > >> Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised > > >> that gcc doesn't warn about this. > > > > > > Perhaps these would make doing allmodconfig builds with clang more > > > frequently less painful for you? > > > > > > https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/ > > > > Thank you, at least for me this is really helpful. > > Really glad to hear! I hope this helps make testing and verifying > changes with clang and LLVM easier for developers and maintainers. > > > I tried now clang for the first time but seeing a strange problem. > > > > I prefer to define the compiler in GNUmakefile so it's easy to change > > compilers and I don't need to remember the exact command line. So I have > > this in the top level GNUmakefile (all the rest commented out): > > > > LLVM=/opt/clang/llvm-16.0.0/bin/ > > > > If I run 'make oldconfig' it seems to use clang but after I run just > > 'make' it seems to switch back to the host GCC compiler and ask for GCC > > specific config questions again. Workaround for this seems to be adding > > 'export LLVM' to GNUmakefile, after that also 'make' uses clang as > > expected. > > Interesting... I just tested with a basic GNUmakefile and everything > seems to work fine without an export. At the same time, the export > should not hurt anything, so as long as it works, that is what matters. Ah, the export is needed so that mixed-build works properly (see lines 324 to 361 in Makefile), as 'make' will be called to process each target individually; without the export, LLVM is not set for the subsequent 'make' calls, so gcc is called. I just saw the same behavior as you did while testing with $ make -j(nproc) clean defconfig all without the export (GCC was used instead of LLVM). > $ gcc --version > fish: Unknown command: gcc > > > $ fd -t x . $CBL_TC_LLVM_STORE/16.0.0/bin -x basename > clang-16 > llvm-nm > llvm-objdump > llvm-objcopy > llvm-symbolizer > llvm-strings > llvm-readobj > llvm-dwarfdump > lld > llvm-ar > > > $ cat GNUmakefile > LLVM := $(CBL_TC_LLVM_STORE)/16.0.0/bin/ > > include Makefile > > > $ make -sj(nproc) defconfig > > > $ head -13 .config > # > # Automatically generated file; DO NOT EDIT. > # Linux/x86 6.3.0-rc3 Kernel Configuration > # > CONFIG_CC_VERSION_TEXT="ClangBuiltLinux clang version 16.0.0" > CONFIG_GCC_VERSION=0 > CONFIG_CC_IS_CLANG=y > CONFIG_CLANG_VERSION=16 > CONFIG_AS_IS_LLVM=y > CONFIG_AS_VERSION=16 > CONFIG_LD_VERSION=0 > CONFIG_LD_IS_LLD=y > CONFIG_LLD_VERSION=16 > > > $ make -sj(nproc) init/main.o > > > $ $CBL_TC_LLVM_STORE/16.0.0/bin/llvm-readelf -p .comment init/main.o > String dump of section '.comment': > [ 1] ClangBuiltLinux clang version 16.0.0 > > > I added an informational print and I always saw the correct value: > > diff --git a/Makefile b/Makefile > index a2c310df2145..070394c4cb8c 100644 > --- a/Makefile > +++ b/Makefile > @@ -431,6 +431,7 @@ HOST_LFS_LDFLAGS := $(shell getconf LFS_LDFLAGS > 2>/dev/null) > HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null) > > ifneq ($(LLVM),) > +$(info LLVM: $(LLVM)) > ifneq ($(filter %/,$(LLVM)),) > LLVM_PREFIX := $(LLVM) > else ifneq ($(filter -%,$(LLVM)),) > > If you have any further issues, please do not hesitate to reach out! > > Cheers, > Nathan >
Re: Linux 6.3-rc3
On Wed, Mar 22, 2023 at 02:44:47PM +0200, Kalle Valo wrote: > Nathan Chancellor writes: > > > On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote: > >> On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor > >> wrote: > >> > > >> > On the clang front, I am still seeing the following warning turned error > >> > for arm64 allmodconfig at least: > >> > > >> > drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is > >> > uninitialized when used here [-Werror,-Wuninitialized] > >> > if (syncpt_irq < 0) > >> > ^~ > >> > >> Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised > >> that gcc doesn't warn about this. > > > > Perhaps these would make doing allmodconfig builds with clang more > > frequently less painful for you? > > > > https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/ > > Thank you, at least for me this is really helpful. Really glad to hear! I hope this helps make testing and verifying changes with clang and LLVM easier for developers and maintainers. > I tried now clang for the first time but seeing a strange problem. > > I prefer to define the compiler in GNUmakefile so it's easy to change > compilers and I don't need to remember the exact command line. So I have > this in the top level GNUmakefile (all the rest commented out): > > LLVM=/opt/clang/llvm-16.0.0/bin/ > > If I run 'make oldconfig' it seems to use clang but after I run just > 'make' it seems to switch back to the host GCC compiler and ask for GCC > specific config questions again. Workaround for this seems to be adding > 'export LLVM' to GNUmakefile, after that also 'make' uses clang as > expected. Interesting... I just tested with a basic GNUmakefile and everything seems to work fine without an export. At the same time, the export should not hurt anything, so as long as it works, that is what matters. $ gcc --version fish: Unknown command: gcc $ fd -t x . $CBL_TC_LLVM_STORE/16.0.0/bin -x basename clang-16 llvm-nm llvm-objdump llvm-objcopy llvm-symbolizer llvm-strings llvm-readobj llvm-dwarfdump lld llvm-ar $ cat GNUmakefile LLVM := $(CBL_TC_LLVM_STORE)/16.0.0/bin/ include Makefile $ make -sj(nproc) defconfig $ head -13 .config # # Automatically generated file; DO NOT EDIT. # Linux/x86 6.3.0-rc3 Kernel Configuration # CONFIG_CC_VERSION_TEXT="ClangBuiltLinux clang version 16.0.0" CONFIG_GCC_VERSION=0 CONFIG_CC_IS_CLANG=y CONFIG_CLANG_VERSION=16 CONFIG_AS_IS_LLVM=y CONFIG_AS_VERSION=16 CONFIG_LD_VERSION=0 CONFIG_LD_IS_LLD=y CONFIG_LLD_VERSION=16 $ make -sj(nproc) init/main.o $ $CBL_TC_LLVM_STORE/16.0.0/bin/llvm-readelf -p .comment init/main.o String dump of section '.comment': [ 1] ClangBuiltLinux clang version 16.0.0 I added an informational print and I always saw the correct value: diff --git a/Makefile b/Makefile index a2c310df2145..070394c4cb8c 100644 --- a/Makefile +++ b/Makefile @@ -431,6 +431,7 @@ HOST_LFS_LDFLAGS := $(shell getconf LFS_LDFLAGS 2>/dev/null) HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null) ifneq ($(LLVM),) +$(info LLVM: $(LLVM)) ifneq ($(filter %/,$(LLVM)),) LLVM_PREFIX := $(LLVM) else ifneq ($(filter -%,$(LLVM)),) If you have any further issues, please do not hesitate to reach out! Cheers, Nathan
Re: Linux 6.3-rc3
On Mon, Mar 20, 2023 at 01:30:17PM -0700, Linus Torvalds wrote: > On Mon, Mar 20, 2023 at 1:05 PM Guenter Roeck wrote: > > > > I have noticed that gcc doesn't always warn about uninitialized variables > > in most architectures. > > Yeah, I'm getting the feeling that when the gcc people were trying to > make -Wmaybe-uninitialized work better (when moving it into "-Wall"), > they ended up moving a lot of "clearly uninitialized" cases into it. > > So then because we disable the "maybe" case (with > -Wno-maybe-uninitialized) because it had too many random false > positives, we end up not seeing the obvious cases either. Right, this seems like a subtle difference in semantics between -Wuninitialized between clang and GCC. My naive attempt to reduce the problem with cvise spits out: $ cat dev.i void *host1x_probe___trans_tmp_1; void host1x_unregister(); int host1x_probe_syncpt_irqhost1x_probe() { int err; if (host1x_probe___trans_tmp_1) return 2; if (err) host1x_unregister(); return err; } $ gcc -O2 -Wall -c -o /dev/null dev.i dev.i: In function ‘host1x_probe_syncpt_irqhost1x_probe’: dev.i:7:6: warning: ‘err’ may be used uninitialized [-Wmaybe-uninitialized] 7 | if (err) | ^ dev.i:4:7: note: ‘err’ was declared here 4 | int err; | ^~~ $ clang -Wall -fsyntax-only dev.i dev.i:7:7: warning: variable 'err' is uninitialized when used here [-Wuninitialized] if (err) ^~~ dev.i:4:10: note: initialize the variable 'err' to silence this warning int err; ^ = 0 1 warning generated. If I remove the first branch, both compilers show -Wuninitialized. $ cat dev.i void *host1x_probe___trans_tmp_1; void host1x_unregister(); int host1x_probe_syncpt_irqhost1x_probe() { int err; if (err) host1x_unregister(); return err; } $ gcc -O2 -Wall -c -o /dev/null dev.i dev.i: In function ‘host1x_probe_syncpt_irqhost1x_probe’: dev.i:5:6: warning: ‘err’ is used uninitialized [-Wuninitialized] 5 | if (err) | ^ dev.i:4:7: note: ‘err’ was declared here 4 | int err; | ^~~ $ clang -Wall -fsyntax-only dev.i dev.i:5:7: warning: variable 'err' is uninitialized when used here [-Wuninitialized] if (err) ^~~ dev.i:4:10: note: initialize the variable 'err' to silence this warning int err; ^ = 0 1 warning generated. It seems like clang takes into account that the branch has no effect on how uninitialized err is, although it does acknowledge there may be control flow where err is not used uninitialized because it is not used at all by stating "when used here". I guess GCC does not make this distinction and places it under -Wmaybe-uninitialized. I could be totally wrong though :) Cheers, Nathan
Re: Linux 6.3-rc3
On Mon, Mar 20, 2023 at 11:53:37AM -0700, Nathan Chancellor wrote: > On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote: > > On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor > > wrote: > > > > > > On the clang front, I am still seeing the following warning turned error > > > for arm64 allmodconfig at least: > > > > > > drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is > > > uninitialized when used here [-Werror,-Wuninitialized] > > > if (syncpt_irq < 0) > > > ^~ > > > > Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised > > that gcc doesn't warn about this. > > Perhaps these would make doing allmodconfig builds with clang more > frequently less painful for you? > > https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/ > > > That syncpt_irq thing isn't written to anywhere, so that's pretty egregious. > > > > We use -Wno-maybe-uninitialized because gcc gets it so wrong, but > > that's different from the "-Wuninitialized" thing (without the > > "maybe"). > > > > I've seen gcc mess this up when there is one single assignment, > > because then the SSA format makes it *so* easy to just use that > > assignment out-of-order (or unconditionally), but this case looks > > unusually clear-cut. > > > > So the fact that gcc doesn't warn about it is outright odd. > > > > > If that does not come to you through other means before -rc4, could you > > > just apply it directly so that I can stop applying it to our CI? :) > > > > Bah. I took it now, there's no excuse for that thing. > > Thanks! > > > Do we have any gcc people around that could explain why gcc failed so > > miserably at this trivial case? > > Cc'ing linux-toolchains. The start of the thread is here: > > https://lore.kernel.org/CAHk-=wgsqpdkejbb92m37jntdrqjrnruaprahke8ughtqqu...@mail.gmail.com/ > > The problematic function before the fix is here: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/host1x/dev.c?id=3d3699bde4b043eea17993e4e76804a8128f0fdb#n487 > > I will see if I have some cycles to try and reduce something out for the > GCC folks. While setting up the reduction, I noticed that there is an instance of -Wmaybe-uninitialized at this site. Seems odd that it is not sure, I will reduce on that. ../drivers/gpu/host1x/dev.c: In function 'host1x_probe': ../drivers/gpu/host1x/dev.c:520:12: error: 'syncpt_irq' may be used uninitialized [-Werror=maybe-uninitialized] 520 | if (syncpt_irq < 0) |^ ../drivers/gpu/host1x/dev.c:490:13: note: 'syncpt_irq' was declared here 490 | int syncpt_irq; | ^~ cc1: all warnings being treated as errors Cheers, Nathan
Re: Linux 6.3-rc3
On Mon, Mar 20, 2023 at 11:49:55AM -0700, Linus Torvalds wrote: > On Mon, Mar 20, 2023 at 11:26 AM Linus Torvalds > wrote: > > > > Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised > > that gcc doesn't warn about this. > > Side note: I'm also wondering why that TEGRA_HOST1X config has that > ARM dependency in > > depends on ARCH_TEGRA || (ARM && COMPILE_TEST) > > because it seems to build just fine at least on x86-64 if I change it to be > just > > depends on ARCH_TEGRA || COMPILE_TEST > > ie there seems to be nothing ARM-specific in there. > > Limiting it to just the tegra platform by default makes 100% sense, > but that "only do compile-testing on ARM" seems a bit bogus. > > That limit goes back to forever (commit 6f44c2b5280f: "gpu: host1x: > Increase compile test coverage" back in Nov 2013), so maybe things > didn't use to work as well back in the dark ages? > > None of this explains why gcc didn't catch it, but at least allowing > the build on x86-64 would likely have made it easier for people to see > clang catching this. I did see a patch fly by to fix that: https://lore.kernel.org/20230316082035.567520-3-christian.koe...@amd.com/ It seems like the DRM_TEGRA half of it is broken though: https://lore.kernel.org/202303170635.a2rsq1wu-...@intel.com/ Cheers, Nathan
Re: Linux 6.3-rc3
On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote: > On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor wrote: > > > > On the clang front, I am still seeing the following warning turned error > > for arm64 allmodconfig at least: > > > > drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is > > uninitialized when used here [-Werror,-Wuninitialized] > > if (syncpt_irq < 0) > > ^~ > > Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised > that gcc doesn't warn about this. Perhaps these would make doing allmodconfig builds with clang more frequently less painful for you? https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/ > That syncpt_irq thing isn't written to anywhere, so that's pretty egregious. > > We use -Wno-maybe-uninitialized because gcc gets it so wrong, but > that's different from the "-Wuninitialized" thing (without the > "maybe"). > > I've seen gcc mess this up when there is one single assignment, > because then the SSA format makes it *so* easy to just use that > assignment out-of-order (or unconditionally), but this case looks > unusually clear-cut. > > So the fact that gcc doesn't warn about it is outright odd. > > > If that does not come to you through other means before -rc4, could you > > just apply it directly so that I can stop applying it to our CI? :) > > Bah. I took it now, there's no excuse for that thing. Thanks! > Do we have any gcc people around that could explain why gcc failed so > miserably at this trivial case? Cc'ing linux-toolchains. The start of the thread is here: https://lore.kernel.org/CAHk-=wgsqpdkejbb92m37jntdrqjrnruaprahke8ughtqqu...@mail.gmail.com/ The problematic function before the fix is here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/host1x/dev.c?id=3d3699bde4b043eea17993e4e76804a8128f0fdb#n487 I will see if I have some cycles to try and reduce something out for the GCC folks. Cheers, Nathan
Re: Linux 6.3-rc3
On Sun, Mar 19, 2023 at 01:50:21PM -0700, Linus Torvalds wrote: > So rc3 is fairly big, but that's not hugely usual: it's when a lot of > the fixes tick up as it takes a while before people find and start > reporting issues. ... > Please test and report any issues you find, On the clang front, I am still seeing the following warning turned error for arm64 allmodconfig at least: drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is uninitialized when used here [-Werror,-Wuninitialized] if (syncpt_irq < 0) ^~ drivers/gpu/host1x/dev.c:490:16: note: initialize the variable 'syncpt_irq' to silence this warning int syncpt_irq; ^ = 0 1 error generated. There is an obvious fix that has been available on the mailing list for some time: https://lore.kernel.org/20230127221418.2522612-1-a...@kernel.org/ It appears there was some sort of process snafu, since the fix never got applied to the drm tree before the main pull for 6.3 and I have not been able to get anyone to apply it to a tree targeting -rc releases. https://lore.kernel.org/Y%2FeULFO4jbivQ679@dev-arch.thelio-3990X/ https://lore.kernel.org/67f9fe7f-392a-9acd-1a4d-0a43da634...@nvidia.com/ If that does not come to you through other means before -rc4, could you just apply it directly so that I can stop applying it to our CI? :) Cheers, Nathan
Re: [PATCH] drm/rockchip: vop2: fix uninitialized variable possible_crtcs
On Thu, Mar 16, 2023 at 09:23:02AM -0400, Tom Rix wrote: > clang reportes this error > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2322:8: error: > variable 'possible_crtcs' is used uninitialized whenever 'if' > condition is false [-Werror,-Wsometimes-uninitialized] > if (vp) { > ^~ > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2336:36: note: > uninitialized use occurs here > ret = vop2_plane_init(vop2, win, possible_crtcs); > ^~ > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2322:4: > note: remove the 'if' if its condition is always true > if (vp) { > ^~~~ > > The else-statement changes the win->type to OVERLAY without setting the > possible_crtcs variable. Rework the block, initialize possible_crtcs to > 0 to remove the else-statement. Split the else-if-statement out to its > own if-statement so the OVERLAY check will catch when the win-type has > been changed. > > Fixes: 368419a2d429 ("drm/rockchip: vop2: initialize possible_crtcs properly") > Signed-off-by: Tom Rix Reviewed-by: Nathan Chancellor > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 9 - > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > index 03ca32cd2050..fce992c3506f 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > @@ -2301,7 +2301,7 @@ static int vop2_create_crtcs(struct vop2 *vop2) > nvp = 0; > for (i = 0; i < vop2->registered_num_wins; i++) { > struct vop2_win *win = >win[i]; > - u32 possible_crtcs; > + u32 possible_crtcs = 0; > > if (vop2->data->soc_id == 3566) { > /* > @@ -2327,12 +2327,11 @@ static int vop2_create_crtcs(struct vop2 *vop2) > /* change the unused primary window to overlay > window */ > win->type = DRM_PLANE_TYPE_OVERLAY; > } > - } else if (win->type == DRM_PLANE_TYPE_OVERLAY) { > - possible_crtcs = (1 << nvps) - 1; > - } else { > - possible_crtcs = 0; > } > > + if (win->type == DRM_PLANE_TYPE_OVERLAY) > + possible_crtcs = (1 << nvps) - 1; > + > ret = vop2_plane_init(vop2, win, possible_crtcs); > if (ret) { > drm_err(vop2->drm, "failed to init plane %s: %d\n", > -- > 2.27.0 >
Re: [PATCH] drm/vmwgfx: Fix src/dst_pitch confusion
On Tue, Mar 14, 2023 at 05:14:45PM -0400, Zack Rusin wrote: > From: Zack Rusin > > The src/dst_pitch got mixed up during the rework of the function, make > sure the offset's refer to the correct one. > > Spotted by clang: > Clang warns (or errors with CONFIG_WERROR): > > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:509:29: error: variable 'dst_pitch' is > uninitialized when used here [-Werror,-Wuninitialized] > src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp; > ^ > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:492:26: note: initialize the variable > 'dst_pitch' to silence this warning > s32 src_pitch, dst_pitch; > ^ >= 0 > 1 error generated. > > Signed-off-by: Zack Rusin > Link: https://github.com/ClangBuiltLinux/linux/issues/1811 > Reported-by: Nathan Chancellor > Reported-by: Dave Airlie > Fixes: 39985eea5a6d ("drm/vmwgfx: Abstract placement selection") Reviewed-by: Nathan Chancellor Thanks for the quick response and patch! > --- > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > index d79a6eccfaa4..ba0c0e12cfe9 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > @@ -506,11 +506,11 @@ static void vmw_stdu_bo_cpu_commit(struct vmw_kms_dirty > *dirty) > /* Assume we are blitting from Guest (bo) to Host (display_srf) */ > src_pitch = stdu->display_srf->metadata.base_size.width * stdu->cpp; > src_bo = >display_srf->res.guest_memory_bo->tbo; > - src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp; > + src_offset = ddirty->top * src_pitch + ddirty->left * stdu->cpp; > > dst_pitch = ddirty->pitch; > dst_bo = >buf->tbo; > - dst_offset = ddirty->fb_top * src_pitch + ddirty->fb_left * stdu->cpp; > + dst_offset = ddirty->fb_top * dst_pitch + ddirty->fb_left * stdu->cpp; > > (void) vmw_bo_cpu_blit(dst_bo, dst_offset, dst_pitch, > src_bo, src_offset, src_pitch, > -- > 2.38.1 >
Re: [PATCH v3 1/6] drm/rockchip: vop2: initialize possible_crtcs properly
Hi Michael, On Tue, Jan 24, 2023 at 06:47:01AM +0100, Michael Riesch wrote: > The variable possible_crtcs is only initialized for primary and > overlay planes. Since the VOP2 driver only supports these plane > types at the moment, the current code is safe. However, in order > to provide a future-proof solution, fix the initialization of > the variable. > > Reported-by: kernel test robot > Reported-by: Dan Carpenter > Signed-off-by: Michael Riesch > --- > v3: > - no changes > v2: > - new patch > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > index 8cecf81a5ae0..374ef821b453 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > @@ -2322,10 +2322,11 @@ static int vop2_create_crtc(struct vop2 *vop2) > /* change the unused primary window to overlay > window */ > win->type = DRM_PLANE_TYPE_OVERLAY; > } > - } > - > - if (win->type == DRM_PLANE_TYPE_OVERLAY) > + } else if (win->type == DRM_PLANE_TYPE_OVERLAY) { > possible_crtcs = (1 << nvps) - 1; > + } else { > + possible_crtcs = 0; > + } > > ret = vop2_plane_init(vop2, win, possible_crtcs); > if (ret) { > -- > 2.30.2 > This patch is now in -next as commit 368419a2d429 ("drm/rockchip: vop2: initialize possible_crtcs properly") and it actually appears to introduce a path where possible_crtcs could be used uninitialized. drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2316:8: error: variable 'possible_crtcs' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] if (vp) { ^~ drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2330:36: note: uninitialized use occurs here ret = vop2_plane_init(vop2, win, possible_crtcs); ^~ drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2316:4: note: remove the 'if' if its condition is always true if (vp) { ^~~~ drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2298:21: note: initialize the variable 'possible_crtcs' to silence this warning u32 possible_crtcs; ^ = 0 1 error generated. Prior to this change, if that else path was hit, clang recognized based on the assignment that the next if statement would always be true. Now, if the else path is taken, the possible_crtcs assignment will be missed. Is that intentional? Cheers, Nathan
[PATCH] drm/vmwgfx: Fix uninitialized use of dst_pitch in vmw_stdu_bo_cpu_commit()
Clang warns (or errors with CONFIG_WERROR): drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:509:29: error: variable 'dst_pitch' is uninitialized when used here [-Werror,-Wuninitialized] src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp; ^ drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:492:26: note: initialize the variable 'dst_pitch' to silence this warning s32 src_pitch, dst_pitch; ^ = 0 1 error generated. The assignments were switched around in such a way that dst_pitch was used before it was assigned. Swap the pitch assignments to fix the issue and make it clear which section they are used in. Fixes: 39985eea5a6d ("drm/vmwgfx: Abstract placement selection") Link: https://github.com/ClangBuiltLinux/linux/issues/1811 Signed-off-by: Nathan Chancellor --- I am not sure if this is the right fix, as it was not entirely clear to me that src_pitch and dst_pitch were being used in the right assignments but this is the obvious fix otherwise. Consider this a bug report if not :) --- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c index d79a6eccfaa4..030e977c68e2 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c @@ -504,11 +504,11 @@ static void vmw_stdu_bo_cpu_commit(struct vmw_kms_dirty *dirty) return; /* Assume we are blitting from Guest (bo) to Host (display_srf) */ - src_pitch = stdu->display_srf->metadata.base_size.width * stdu->cpp; + dst_pitch = ddirty->pitch; src_bo = >display_srf->res.guest_memory_bo->tbo; src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp; - dst_pitch = ddirty->pitch; + src_pitch = stdu->display_srf->metadata.base_size.width * stdu->cpp; dst_bo = >buf->tbo; dst_offset = ddirty->fb_top * src_pitch + ddirty->fb_left * stdu->cpp; --- base-commit: c87e859cdeb5d106cb861326e3135c606d61f88d change-id: 20230314-vmware-wuninitialized-28666fb5a62b Best regards, -- Nathan Chancellor
Re: [PATCH] gpu: host1x: fix uninitialized variable use
Ping? This warning is now in 6.3-rc1. On Thu, Feb 23, 2023 at 09:28:28AM -0700, Nathan Chancellor wrote: > Hi Thierry, Daniel, and David, > > On Fri, Jan 27, 2023 at 11:14:00PM +0100, Arnd Bergmann wrote: > > From: Arnd Bergmann > > > > The error handling for platform_get_irq() failing no longer > > works after a recent change, clang now points this out with > > a warning: > > > > drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is > > uninitialized when used here [-Werror,-Wuninitialized] > > if (syncpt_irq < 0) > > ^~ > > > > Fix this by removing the variable and checking the correct > > error status. > > > > Fixes: 625d4ffb438c ("gpu: host1x: Rewrite syncpoint interrupt handling") > > Signed-off-by: Arnd Bergmann > > --- > > drivers/gpu/host1x/dev.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c > > index 4872d183d860..aae2efeef503 100644 > > --- a/drivers/gpu/host1x/dev.c > > +++ b/drivers/gpu/host1x/dev.c > > @@ -487,7 +487,6 @@ static int host1x_get_resets(struct host1x *host) > > static int host1x_probe(struct platform_device *pdev) > > { > > struct host1x *host; > > - int syncpt_irq; > > int err; > > > > host = devm_kzalloc(>dev, sizeof(*host), GFP_KERNEL); > > @@ -517,8 +516,8 @@ static int host1x_probe(struct platform_device *pdev) > > } > > > > host->syncpt_irq = platform_get_irq(pdev, 0); > > - if (syncpt_irq < 0) > > - return syncpt_irq; > > + if (host->syncpt_irq < 0) > > + return host->syncpt_irq; > > > > mutex_init(>devices_lock); > > INIT_LIST_HEAD(>devices); > > -- > > 2.39.0 > > > > Apologies if this has been reported already or has a solution in > progress but mainline is now broken because this change got separated > from the change it is fixing: > > https://github.com/ClangBuiltLinux/continuous-integration2/actions/runs/4249931209/jobs/7391912774 > https://storage.tuxsuite.com/public/clangbuiltlinux/continuous-integration2/builds/2M7y9HpiXB13qiC2mkHMyeZOcLW/build.log > > I see this change sitting in the drm-tegra tree [1], which is getting > merged into -next, so it is fixed there, which is why we did not notice > any issues until the drm-next tree was merged into mainline. Can this be > fast tracked to Linus to unbreak clang builds with -Werror? > > [1]: > https://gitlab.freedesktop.org/drm/tegra/-/commit/b9930311641cf2ed905a84aabe27e8f3868aee4a
[PATCH] drm: omapdrm: Do not use helper unininitialized in omap_fbdev_init()
Clang warns (or errors with CONFIG_WERROR): ../drivers/gpu/drm/omapdrm/omap_fbdev.c:235:6: error: variable 'helper' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized] if (!fbdev) ^~ ../drivers/gpu/drm/omapdrm/omap_fbdev.c:259:26: note: uninitialized use occurs here drm_fb_helper_unprepare(helper); ^~ ../drivers/gpu/drm/omapdrm/omap_fbdev.c:235:2: note: remove the 'if' if its condition is always false if (!fbdev) ^~~ ../drivers/gpu/drm/omapdrm/omap_fbdev.c:228:30: note: initialize the variable 'helper' to silence this warning struct drm_fb_helper *helper; ^ = NULL 1 error generated. Return early, as there is nothing for the function to do if memory cannot be allocated. There is no point in adding another label to just emit the warning at the end of the function in this case, as memory allocation failures are already logged. Fixes: 3fb1f62f80a1 ("drm/fb-helper: Remove drm_fb_helper_unprepare() from drm_fb_helper_fini()") Link: https://github.com/ClangBuiltLinux/linux/issues/1809 Link: https://lore.kernel.org/oe-kbuild-all/202302250058.fyte9atp-...@intel.com/ Reported-by: kernel test robot Signed-off-by: Nathan Chancellor --- This is currently showing in mainline so I believe this should go to drm-misc-next-fixes. --- drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c index 84429728347f..a6c8542087ec 100644 --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c @@ -233,7 +233,7 @@ void omap_fbdev_init(struct drm_device *dev) fbdev = kzalloc(sizeof(*fbdev), GFP_KERNEL); if (!fbdev) - goto fail; + return; INIT_WORK(>work, pan_worker); --- base-commit: e034b8a18d4badceecb672c58b488bad1e901d95 change-id: 20230224-omapdrm-wsometimes-uninitialized-0125f7692fbb Best regards, -- Nathan Chancellor
Re: [PATCH] gpu: host1x: fix uninitialized variable use
Hi Thierry, Daniel, and David, On Fri, Jan 27, 2023 at 11:14:00PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > The error handling for platform_get_irq() failing no longer > works after a recent change, clang now points this out with > a warning: > > drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is uninitialized > when used here [-Werror,-Wuninitialized] > if (syncpt_irq < 0) > ^~ > > Fix this by removing the variable and checking the correct > error status. > > Fixes: 625d4ffb438c ("gpu: host1x: Rewrite syncpoint interrupt handling") > Signed-off-by: Arnd Bergmann > --- > drivers/gpu/host1x/dev.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c > index 4872d183d860..aae2efeef503 100644 > --- a/drivers/gpu/host1x/dev.c > +++ b/drivers/gpu/host1x/dev.c > @@ -487,7 +487,6 @@ static int host1x_get_resets(struct host1x *host) > static int host1x_probe(struct platform_device *pdev) > { > struct host1x *host; > - int syncpt_irq; > int err; > > host = devm_kzalloc(>dev, sizeof(*host), GFP_KERNEL); > @@ -517,8 +516,8 @@ static int host1x_probe(struct platform_device *pdev) > } > > host->syncpt_irq = platform_get_irq(pdev, 0); > - if (syncpt_irq < 0) > - return syncpt_irq; > + if (host->syncpt_irq < 0) > + return host->syncpt_irq; > > mutex_init(>devices_lock); > INIT_LIST_HEAD(>devices); > -- > 2.39.0 > Apologies if this has been reported already or has a solution in progress but mainline is now broken because this change got separated from the change it is fixing: https://github.com/ClangBuiltLinux/continuous-integration2/actions/runs/4249931209/jobs/7391912774 https://storage.tuxsuite.com/public/clangbuiltlinux/continuous-integration2/builds/2M7y9HpiXB13qiC2mkHMyeZOcLW/build.log I see this change sitting in the drm-tegra tree [1], which is getting merged into -next, so it is fixed there, which is why we did not notice any issues until the drm-next tree was merged into mainline. Can this be fast tracked to Linus to unbreak clang builds with -Werror? [1]: https://gitlab.freedesktop.org/drm/tegra/-/commit/b9930311641cf2ed905a84aabe27e8f3868aee4a Cheers, Nathan
Re: [PATCH] drm/msm: return early when allocating fbdev fails
On Wed, Feb 22, 2023 at 05:09:40PM +0100, Thomas Zimmermann wrote: > Hi > > Am 22.02.23 um 16:56 schrieb Tom Rix: > > building with clang and W=1 reports > > drivers/gpu/drm/msm/msm_fbdev.c:144:6: error: variable 'helper' is used > >uninitialized whenever 'if' condition is true > > [-Werror,-Wsometimes-uninitialized] > >if (!fbdev) > >^~ > > > > helper is only initialized after fbdev succeeds, so is in a garbage state at > > the fail: label. There is nothing to unwinded if fbdev alloaction fails, > > return NULL. > > > > Fixes: 3fb1f62f80a1 ("drm/fb-helper: Remove drm_fb_helper_unprepare() from > > drm_fb_helper_fini()") > > Signed-off-by: Tom Rix > > Already fixed here: > https://lore.kernel.org/dri-devel/08e3340e-b459-0e60-4bba-30716b675...@suse.de/T/#t There is also: ../drivers/gpu/drm/omapdrm/omap_fbdev.c:235:6: error: variable 'helper' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized] if (!fbdev) ^~ ../drivers/gpu/drm/omapdrm/omap_fbdev.c:259:26: note: uninitialized use occurs here drm_fb_helper_unprepare(helper); ^~ ../drivers/gpu/drm/omapdrm/omap_fbdev.c:235:2: note: remove the 'if' if its condition is always false if (!fbdev) ^~~ ../drivers/gpu/drm/omapdrm/omap_fbdev.c:228:30: note: initialize the variable 'helper' to silence this warning struct drm_fb_helper *helper; ^ = NULL 1 error generated. Is the fix the same as the one you have linked above? Cheers, Nathan
Re: [PATCH] gpu: host1x: fix uninitialized variable use
On Fri, Jan 27, 2023 at 11:14:00PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > The error handling for platform_get_irq() failing no longer > works after a recent change, clang now points this out with > a warning: > > drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is uninitialized > when used here [-Werror,-Wuninitialized] > if (syncpt_irq < 0) > ^~ > > Fix this by removing the variable and checking the correct > error status. > > Fixes: 625d4ffb438c ("gpu: host1x: Rewrite syncpoint interrupt handling") > Signed-off-by: Arnd Bergmann I had the same diff pending but civic duty called today :) Reviewed-by: Nathan Chancellor > --- > drivers/gpu/host1x/dev.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c > index 4872d183d860..aae2efeef503 100644 > --- a/drivers/gpu/host1x/dev.c > +++ b/drivers/gpu/host1x/dev.c > @@ -487,7 +487,6 @@ static int host1x_get_resets(struct host1x *host) > static int host1x_probe(struct platform_device *pdev) > { > struct host1x *host; > - int syncpt_irq; > int err; > > host = devm_kzalloc(>dev, sizeof(*host), GFP_KERNEL); > @@ -517,8 +516,8 @@ static int host1x_probe(struct platform_device *pdev) > } > > host->syncpt_irq = platform_get_irq(pdev, 0); > - if (syncpt_irq < 0) > - return syncpt_irq; > + if (host->syncpt_irq < 0) > + return host->syncpt_irq; > > mutex_init(>devices_lock); > INIT_LIST_HEAD(>devices); > -- > 2.39.0 >
Re: [PATCH v2 00/14] Remove clang's -Qunused-arguments from KBUILD_CPPFLAGS
Hi Naresh, On Mon, Jan 23, 2023 at 07:28:10PM +0530, Naresh Kamboju wrote: > FYI, > [ please provide comments, feedback and improvements on build/ ltp smoke > tests ] > > LKFT test farm have fetched your patch series [1] > [PATCH v2 00/14] Remove clang's -Qunused-arguments from KBUILD_CPPFLAGS > [1] > https://lore.kernel.org/llvm/20221228-drop-qunused-arguments-v2-0-9adbddd20...@kernel.org/ Thank you a lot for testing this series, it is much appreciated! It looks like this was applied on top of 6.2-rc3 if I am reading your logs right but your mainline testing is recent, 6.2-rc5. I think the errors you are seeing here are just existing mainline regressions that were later fixed. > Following build warnings and errors reported. > > sh: > gcc-11-defconfig — FAIL > gcc-11-shx3_defconfig — FAIL > https://qa-reports.linaro.org/~anders.roxell/linux-mainline-patches/build/https___lore_kernel_org_llvm_20221228-drop-qunused-arguments-v2-1-9adbddd20d86_kernel_org/testrun/14221835/suite/build/tests/ > > mainline getting passed. > https://qa-reports.linaro.org/lkft/linux-mainline-master/build/v6.2-rc5/testrun/14298156/suite/build/test/gcc-11-defconfig/history/ > https://qa-reports.linaro.org/lkft/linux-mainline-master/build/v6.2-rc5/testrun/14298156/suite/build/test/gcc-11-shx3_defconfig/history/ > > Build error: > In function 'follow_pmd_mask', > inlined from 'follow_pud_mask' at /builds/linux/mm/gup.c:735:9, > inlined from 'follow_p4d_mask' at /builds/linux/mm/gup.c:752:9, > inlined from 'follow_page_mask' at /builds/linux/mm/gup.c:809:9: > /builds/linux/include/linux/compiler_types.h:358:45: error: call to > '__compiletime_assert_263' declared with attribute error: Unsupported > access size for {READ,WRITE}_ONCE(). > 358 | _compiletime_assert(condition, msg, > __compiletime_assert_, __COUNTER__) I think this was fixed with mainline commit 526970be53d5 ("sh/mm: Fix pmd_t for real"), released in 6.2-rc4. You can see a previous build failing in the same manner: https://qa-reports.linaro.org/lkft/linux-mainline-master/build/v6.2-rc3-9-g5a41237ad1d4/testrun/14056384/suite/build/tests/ > s390: > clang-15-defconfig — FAIL > https://qa-reports.linaro.org/~anders.roxell/linux-mainline-patches/build/https___lore_kernel_org_llvm_20221228-drop-qunused-arguments-v2-1-9adbddd20d86_kernel_org/testrun/14221913/suite/build/tests/ > > mainline getting passed. > https://qa-reports.linaro.org/lkft/linux-mainline-master/build/v6.2-rc5/testrun/14300495/suite/build/test/clang-15-defconfig/history/ > > Build error: > make --silent --keep-going --jobs=8 > O=/home/tuxbuild/.cache/tuxmake/builds/1/build LLVM_IAS=0 ARCH=s390 > CROSS_COMPILE=s390x-linux-gnu- 'HOSTCC=sccache clang' 'CC=sccache > clang' > `.exit.text' referenced in section `__jump_table' of fs/fuse/inode.o: > defined in discarded section `.exit.text' of fs/fuse/inode.o > `.exit.text' referenced in section `__jump_table' of fs/fuse/inode.o: > defined in discarded section `.exit.text' of fs/fuse/inode.o > `.exit.text' referenced in section `__bug_table' of crypto/algboss.o: > defined in discarded section `.exit.text' of crypto/algboss.o > `.exit.text' referenced in section `__bug_table' of drivers/scsi/sd.o: > defined in discarded section `.exit.text' of drivers/scsi/sd.o > `.exit.text' referenced in section `__jump_table' of drivers/md/md.o: > defined in discarded section `.exit.text' of drivers/md/md.o > `.exit.text' referenced in section `__jump_table' of drivers/md/md.o: > defined in discarded section `.exit.text' of drivers/md/md.o > `.exit.text' referenced in section `.altinstructions' of > drivers/md/md.o: defined in discarded section `.exit.text' of > drivers/md/md.o > `.exit.text' referenced in section `.altinstructions' of > drivers/md/md.o: defined in discarded section `.exit.text' of > drivers/md/md.o > `.exit.text' referenced in section `.altinstructions' of > net/iucv/iucv.o: defined in discarded section `.exit.text' of > net/iucv/iucv.o > `.exit.text' referenced in section `__bug_table' of > drivers/s390/cio/qdio_thinint.o: defined in discarded section > `.exit.text' of drivers/s390/cio/qdio_thinint.o > `.exit.text' referenced in section `__bug_table' of > drivers/s390/net/qeth_l3_main.o: defined in discarded section > `.exit.text' of drivers/s390/net/qeth_l3_main.o > `.exit.text' referenced in section `__bug_table' of > drivers/s390/net/qeth_l3_main.o: defined in discarded section > `.exit.text' of drivers/s390/net/qeth_l3_main.o > s390x-linux-gnu-ld: BFD (GNU Binutils for Debian) 2.35.2 assertion > fail ../../bfd/elf64-s390.c:3349 > make[2]: *** [/builds/linux/scripts/Makefile.vmlinux:34: vmlinux] Error 1 This should be fixed with mainline commit a494398bde27 ("s390: define RUNTIME_DISCARD_EXIT to fix link error with GNU ld < 2.36"), released in 6.2-rc4 as well. Same as before, visible in mainline at one point without this series:
Re: [PATCH v2] drm/i915: Fix CFI violations in gt_sysfs
Hi Jocelyn, On Thu, Jan 12, 2023 at 11:08:17AM +0100, Jocelyn Falempe wrote: > This patch does also solve a kernel crash when reading > /sys/class/drm/card1/gt/gt0/* on a skylake machine: > https://bugzilla.redhat.com/show_bug.cgi?id=2154880 Interesting, I wonder what aspect of this patch fixes this because I am not sure that is an intended consequence of this change but that is still good to hear! For the record, this is commit a8a4f0467d70 ("drm/i915: Fix CFI violations in gt_sysfs") in mainline. > Do you think it can be backported to stable releases ? > Conflicts are trivial on top of v6.0 at least. I had a report from another user of this crash affecting them with kCFI so it is on my TODO to backport it to 6.1 (6.0 just went EOL) but I am currently out of the office until next Wednesday so I won't be able to get to it until then (as I would like to test the backport on affected hardware). If someone wants to beat me to it, I won't complain ;) Cheers, Nathan
[PATCH v2 12/14] drm/amd/display: Do not add '-mhard-float' to dml_ccflags for clang
When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it warns: clang-16: error: argument unused during compilation: '-mhard-float' [-Werror,-Wunused-command-line-argument] Similar to commit 84edc2eff827 ("selftest/fpu: avoid clang warning"), just add this flag to GCC builds. Commit 0f0727d971f6 ("drm/amd/display: readd -msse2 to prevent Clang from emitting libcalls to undefined SW FP routines") added '-msse2' to prevent clang from emitting software floating point routines. Signed-off-by: Nathan Chancellor Acked-by: Alex Deucher --- Cc: amd-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/amd/display/dc/dml/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile index 0ecea87cf48f..9d0f79dff2e3 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile @@ -26,7 +26,8 @@ # subcomponents. ifdef CONFIG_X86 -dml_ccflags := -mhard-float -msse +dml_ccflags-$(CONFIG_CC_IS_GCC) := -mhard-float +dml_ccflags := $(dml_ccflags-y) -msse endif ifdef CONFIG_PPC64 -- 2.39.0
[PATCH v2 00/14] Remove clang's -Qunused-arguments from KBUILD_CPPFLAGS
Hi all, Clang can emit a few different warnings when it encounters a flag that it recognizes but does not support internally. These warnings are elevated to errors within {as,cc}-option via -Werror to catch unsupported flags that should not be added to KBUILD_{A,C}FLAGS; see commit c3f0d0bc5b01 ("kbuild, LLVMLinux: Add -Werror to cc-option to support clang"). If an unsupported flag is unconditionally to KBUILD_{A,C}FLAGS, all subsequent {as,cc}-option will always fail, preventing supported and even potentially necessary flags from getting adding to the tool flags. One would expect these warnings to be visible in the kernel build logs since they are added to KBUILD_{A,C}FLAGS but unfortunately, these warnings are hidden with clang's -Qunused-arguments flag, which is added to KBUILD_CPPFLAGS and used for both compiling and assembling files. Patches 1-4 address the internal inconsistencies of invoking the assembler within kbuild by using KBUILD_AFLAGS consistently and using '-x assembler-with-cpp' over '-x assembler'. This matches how assembly files are built across the kernel and helps avoid problems in situations where macro definitions or warning flags are present in KBUILD_AFLAGS, which cause instances of -Wunused-command-line-argument when the preprocessor is not called to consume them. There were a couple of places in architecture code where this change would break things so those are fixed first. Patches 5-12 clean up warnings that will show up when -Qunused-argument is dropped. I hope none of these are controversial. Patch 13 turns two warnings into errors so that the presence of unused flags cannot be easily ignored. Patch 14 drops -Qunused-argument. This is done last so that it can be easily reverted if need be. This series has seen my personal test framework, which tests several different configurations and architectures, with LLVM tip of tree (16.0.0). I have done defconfig, allmodconfig, and allnoconfig builds for arm, arm64, i386, mips, powerpc, riscv, s390, and x86_64 with GCC 12.2.0 as well but I am hoping the rest of the test infrastructure will catch any lurking problems. I would like this series to stay together so that there is no opportunity for breakage so please consider giving acks so that this can be carried via the kbuild tree (and many thanks to the people who have already provided such tags). --- Changes in v2: - Pick up tags where provided (thank you everyone!) - Patch 6 and 9: Clarify that '-s' is a compiler flag that is only relevant to the linking phase and remove all mention of the assembler's '-s' flag, as the assembler is never directly invoked (Nick, Segher) - Patch 7: Move '-z noexecstack' into new ldflags-y variable (Nick) - Patch 8: Reword commit message to explain the problem in a clearer manner (Nick) - Link to v1: https://lore.kernel.org/r/20221228-drop-qunused-arguments-v1-0-658cbc8fc...@kernel.org --- Nathan Chancellor (12): MIPS: Always use -Wa,-msoft-float and eliminate GAS_HAS_SET_HARDFLOAT MIPS: Prefer cc-option for additions to cflags powerpc: Remove linker flag from KBUILD_AFLAGS powerpc/vdso: Remove unused '-s' flag from ASFLAGS powerpc/vdso: Improve linker flags powerpc/vdso: Remove an unsupported flag from vgettimeofday-32.o with clang s390/vdso: Drop unused '-s' flag from KBUILD_AFLAGS_64 s390/vdso: Drop '-shared' from KBUILD_CFLAGS_64 s390/purgatory: Remove unused '-MD' and unnecessary '-c' flags drm/amd/display: Do not add '-mhard-float' to dml_ccflags for clang kbuild: Turn a couple more of clang's unused option warnings into errors kbuild: Stop using '-Qunused-arguments' with clang Nick Desaulniers (2): x86/boot/compressed: prefer cc-option for CFLAGS additions kbuild: Update assembler calls to use proper flags and language target Makefile| 1 - arch/mips/Makefile | 13 ++--- arch/mips/include/asm/asmmacro-32.h | 4 +-- arch/mips/include/asm/asmmacro.h| 42 ++--- arch/mips/include/asm/fpregdef.h| 14 -- arch/mips/include/asm/mipsregs.h| 20 +++--- arch/mips/kernel/genex.S| 2 +- arch/mips/kernel/r2300_fpu.S| 4 +-- arch/mips/kernel/r4k_fpu.S | 12 - arch/mips/kvm/fpu.S | 6 ++--- arch/mips/loongson2ef/Platform | 2 +- arch/powerpc/Makefile | 2 +- arch/powerpc/kernel/vdso/Makefile | 25 +++-- arch/s390/kernel/vdso64/Makefile| 4 +-- arch/s390/purgatory/Makefile| 2 +- arch/x86/boot/compressed/Makefile | 2 +- drivers/gpu/drm/amd/display/dc/dml/Makefile | 3 ++- scripts/Kconfig.include | 2 +- scripts/Makefile.clang | 2 ++ scripts/Makefile.compiler
[PATCH 12/14] drm/amd/display: Do not add '-mhard-float' to dml_ccflags for clang
When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it warns: clang-16: error: argument unused during compilation: '-mhard-float' [-Werror,-Wunused-command-line-argument] Similar to commit 84edc2eff827 ("selftest/fpu: avoid clang warning"), just add this flag to GCC builds. Commit 0f0727d971f6 ("drm/amd/display: readd -msse2 to prevent Clang from emitting libcalls to undefined SW FP routines") added '-msse2' to prevent clang from emitting software floating point routines. Signed-off-by: Nathan Chancellor --- Cc: harry.wentl...@amd.com Cc: sunpeng...@amd.com Cc: rodrigo.sique...@amd.com Cc: alexander.deuc...@amd.com Cc: christian.koe...@amd.com Cc: xinhui@amd.com Cc: amd-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/amd/display/dc/dml/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile index 0ecea87cf48f..9d0f79dff2e3 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile @@ -26,7 +26,8 @@ # subcomponents. ifdef CONFIG_X86 -dml_ccflags := -mhard-float -msse +dml_ccflags-$(CONFIG_CC_IS_GCC) := -mhard-float +dml_ccflags := $(dml_ccflags-y) -msse endif ifdef CONFIG_PPC64 -- 2.39.0