July 2024 - Linux-stable-mirror

[PATCH AUTOSEL 4.19 1/5] media: uvcvideo: Ignore empty TS packets

by Sasha Levin

From: Ricardo Ribalda <ribalda(a)chromium.org> [ Upstream commit 5cd7c25f6f0576073b3d03bc4cfb1e8ca63a1195 ] Some SunplusIT cameras took a borderline interpretation of the UVC 1.5 standard, and fill the PTS and SCR fields with invalid data if the package does not contain data. "STC must be captured when the first video data of a video frame is put on the USB bus." Some SunplusIT devices send, e.g., buffer: 0xa7755c00 len 000012 header:0x8c stc 00000000 sof 0000 pts 00000000 buffer: 0xa7755c00 len 000012 header:0x8c stc 00000000 sof 0000 pts 00000000 buffer: 0xa7755c00 len 000668 header:0x8c stc 73779dba sof 070c pts 7376d37a While the UVC specification meant that the first two packets shouldn't have had the SCR bit set in the header. This borderline/buggy interpretation has been implemented in a variety of devices, from directly SunplusIT and from other OEMs that rebrand SunplusIT products. So quirking based on VID:PID will be problematic. All the affected modules have the following extension unit: VideoControl Interface Descriptor: guidExtensionCode {82066163-7050-ab49-b8cc-b3855e8d221d} But the vendor plans to use that GUID in the future and fix the bug, this means that we should use heuristic to figure out the broken packets. This patch takes care of this. lsusb of one of the affected cameras: Bus 001 Device 003: ID 1bcf:2a01 Sunplus Innovation Technology Inc. Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.01 bDeviceClass 239 Miscellaneous Device bDeviceSubClass 2 ? bDeviceProtocol 1 Interface Association bMaxPacketSize0 64 idVendor 0x1bcf Sunplus Innovation Technology Inc. idProduct 0x2a01 bcdDevice 0.02 iManufacturer 1 SunplusIT Inc iProduct 2 HanChen Wise Camera iSerial 3 01.00.00 bNumConfigurations 1 Tested-by: HungNien Chen <hn.chen(a)sunplusit.com> Reviewed-by: Sergey Senozhatsky <senozhatsky(a)chromium.org> Reviewed-by: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com> Signed-off-by: Ricardo Ribalda <ribalda(a)chromium.org> Reviewed-by: Tomasz Figa <tfiga(a)chromium.org> Link: https://lore.kernel.org/r/20240323-resend-hwtimestamp-v10-2-b08e590d97c7@ch… Signed-off-by: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/media/usb/uvc/uvc_video.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index c57bc62251bb8..e2c1b98fb4a25 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -473,6 +473,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf, ktime_t time; u16 host_sof; u16 dev_sof; + u32 dev_stc; switch (data[1] & (UVC_STREAM_PTS | UVC_STREAM_SCR)) { case UVC_STREAM_PTS | UVC_STREAM_SCR: @@ -517,6 +518,34 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf, if (dev_sof == stream->clock.last_sof) return; + dev_stc = get_unaligned_le32(&data[header_size - 6]); + + /* + * STC (Source Time Clock) is the clock used by the camera. The UVC 1.5 + * standard states that it "must be captured when the first video data + * of a video frame is put on the USB bus". This is generally understood + * as requiring devices to clear the payload header's SCR bit before + * the first packet containing video data. + * + * Most vendors follow that interpretation, but some (namely SunplusIT + * on some devices) always set the `UVC_STREAM_SCR` bit, fill the SCR + * field with 0's,and expect that the driver only processes the SCR if + * there is data in the packet. + * + * Ignore all the hardware timestamp information if we haven't received + * any data for this frame yet, the packet contains no data, and both + * STC and SOF are zero. This heuristics should be safe on compliant + * devices. This should be safe with compliant devices, as in the very + * unlikely case where a UVC 1.1 device would send timing information + * only before the first packet containing data, and both STC and SOF + * happen to be zero for a particular frame, we would only miss one + * clock sample from many and the clock recovery algorithm wouldn't + * suffer from this condition. + */ + if (buf && buf->bytesused == 0 && len == header_size && + dev_stc == 0 && dev_sof == 0) + return; + stream->clock.last_sof = dev_sof; host_sof = usb_get_current_frame_number(stream->dev->udev); @@ -554,7 +583,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf, spin_lock_irqsave(&stream->clock.lock, flags); sample = &stream->clock.samples[stream->clock.head]; - sample->dev_stc = get_unaligned_le32(&data[header_size - 6]); + sample->dev_stc = dev_stc; sample->dev_sof = dev_sof; sample->host_sof = host_sof; sample->host_time = time; -- 2.43.0

7 months, 2 weeks

1
4
0 0

[PATCH AUTOSEL 5.4 1/6] drm/amdgpu: Fix the null pointer dereference to ras_manager

by Sasha Levin

From: Ma Jun <Jun.Ma2(a)amd.com> [ Upstream commit 4c11d30c95576937c6c35e6f29884761f2dddb43 ] Check ras_manager before using it Signed-off-by: Ma Jun <Jun.Ma2(a)amd.com> Reviewed-by: Lijo Lazar <lijo.lazar(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index b6fc191c353a7..96aad6cc83b1e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1053,12 +1053,15 @@ static void amdgpu_ras_interrupt_process_handler(struct work_struct *work) int amdgpu_ras_interrupt_dispatch(struct amdgpu_device *adev, struct ras_dispatch_if *info) { - struct ras_manager *obj = amdgpu_ras_find_obj(adev, &info->head); - struct ras_ih_data *data = &obj->ih_data; + struct ras_manager *obj; + struct ras_ih_data *data; + obj = amdgpu_ras_find_obj(adev, &info->head); if (!obj) return -EINVAL; + data = &obj->ih_data; + if (data->inuse == 0) return 0; -- 2.43.0

7 months, 2 weeks

1
5
0 0

[PATCH AUTOSEL 5.10 1/7] drm/amdgpu: Fix the null pointer dereference to ras_manager

by Sasha Levin

From: Ma Jun <Jun.Ma2(a)amd.com> [ Upstream commit 4c11d30c95576937c6c35e6f29884761f2dddb43 ] Check ras_manager before using it Signed-off-by: Ma Jun <Jun.Ma2(a)amd.com> Reviewed-by: Lijo Lazar <lijo.lazar(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index e971d2b9e3c00..56f10679a26d1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1351,12 +1351,15 @@ static void amdgpu_ras_interrupt_process_handler(struct work_struct *work) int amdgpu_ras_interrupt_dispatch(struct amdgpu_device *adev, struct ras_dispatch_if *info) { - struct ras_manager *obj = amdgpu_ras_find_obj(adev, &info->head); - struct ras_ih_data *data = &obj->ih_data; + struct ras_manager *obj; + struct ras_ih_data *data; + obj = amdgpu_ras_find_obj(adev, &info->head); if (!obj) return -EINVAL; + data = &obj->ih_data; + if (data->inuse == 0) return 0; -- 2.43.0

7 months, 2 weeks

1
6
0 0

[PATCH AUTOSEL 5.15 01/10] drm/amdgpu/pm: Fix the null pointer dereference for smu7

by Sasha Levin

From: Ma Jun <Jun.Ma2(a)amd.com> [ Upstream commit c02c1960c93eede587576625a1221205a68a904f ] optimize the code to avoid pass a null pointer (hwmgr->backend) to function smu7_update_edc_leakage_table. Signed-off-by: Ma Jun <Jun.Ma2(a)amd.com> Reviewed-by: Yang Wang <kevinyang.wang(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- .../drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 50 +++++++++---------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c index 9bfc465d08fb0..2451cec031dda 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c @@ -2907,6 +2907,7 @@ static int smu7_update_edc_leakage_table(struct pp_hwmgr *hwmgr) static int smu7_hwmgr_backend_init(struct pp_hwmgr *hwmgr) { + struct amdgpu_device *adev = hwmgr->adev; struct smu7_hwmgr *data; int result = 0; @@ -2943,40 +2944,37 @@ static int smu7_hwmgr_backend_init(struct pp_hwmgr *hwmgr) /* Initalize Dynamic State Adjustment Rule Settings */ result = phm_initializa_dynamic_state_adjustment_rule_settings(hwmgr); - if (0 == result) { - struct amdgpu_device *adev = hwmgr->adev; + if (result) + goto fail; - data->is_tlu_enabled = false; + data->is_tlu_enabled = false; - hwmgr->platform_descriptor.hardwareActivityPerformanceLevels = + hwmgr->platform_descriptor.hardwareActivityPerformanceLevels = SMU7_MAX_HARDWARE_POWERLEVELS; - hwmgr->platform_descriptor.hardwarePerformanceLevels = 2; - hwmgr->platform_descriptor.minimumClocksReductionPercentage = 50; + hwmgr->platform_descriptor.hardwarePerformanceLevels = 2; + hwmgr->platform_descriptor.minimumClocksReductionPercentage = 50; - data->pcie_gen_cap = adev->pm.pcie_gen_mask; - if (data->pcie_gen_cap & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) - data->pcie_spc_cap = 20; - else - data->pcie_spc_cap = 16; - data->pcie_lane_cap = adev->pm.pcie_mlw_mask; - - hwmgr->platform_descriptor.vbiosInterruptId = 0x20000400; /* IRQ_SOURCE1_SW_INT */ -/* The true clock step depends on the frequency, typically 4.5 or 9 MHz. Here we use 5. */ - hwmgr->platform_descriptor.clockStep.engineClock = 500; - hwmgr->platform_descriptor.clockStep.memoryClock = 500; - smu7_thermal_parameter_init(hwmgr); - } else { - /* Ignore return value in here, we are cleaning up a mess. */ - smu7_hwmgr_backend_fini(hwmgr); - } + data->pcie_gen_cap = adev->pm.pcie_gen_mask; + if (data->pcie_gen_cap & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) + data->pcie_spc_cap = 20; + else + data->pcie_spc_cap = 16; + data->pcie_lane_cap = adev->pm.pcie_mlw_mask; + + hwmgr->platform_descriptor.vbiosInterruptId = 0x20000400; /* IRQ_SOURCE1_SW_INT */ + /* The true clock step depends on the frequency, typically 4.5 or 9 MHz. Here we use 5. */ + hwmgr->platform_descriptor.clockStep.engineClock = 500; + hwmgr->platform_descriptor.clockStep.memoryClock = 500; + smu7_thermal_parameter_init(hwmgr); result = smu7_update_edc_leakage_table(hwmgr); - if (result) { - smu7_hwmgr_backend_fini(hwmgr); - return result; - } + if (result) + goto fail; return 0; +fail: + smu7_hwmgr_backend_fini(hwmgr); + return result; } static int smu7_force_dpm_highest(struct pp_hwmgr *hwmgr) -- 2.43.0

7 months, 2 weeks

1
9
0 0

[PATCH AUTOSEL 6.1 01/17] drm/amdgpu/pm: Fix the param type of set_power_profile_mode

by Sasha Levin

From: Ma Jun <Jun.Ma2(a)amd.com> [ Upstream commit f683f24093dd94a831085fe0ea8e9dc4c6c1a2d1 ] Function .set_power_profile_mode need an array as input parameter. So define variable workload as an array to fix the below coverity warning. "Passing &workload to function hwmgr->hwmgr_func->set_power_profile_mode which uses it as an array. This might corrupt or misinterpret adjacent memory locations" Signed-off-by: Ma Jun <Jun.Ma2(a)amd.com> Acked-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 8 ++++---- drivers/gpu/drm/amd/pm/powerplay/hwmgr/pp_psm.c | 8 ++++---- drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 16 ++++++++-------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c index 179e1c593a53f..f3668911a88fd 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c +++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c @@ -928,7 +928,7 @@ static int pp_dpm_switch_power_profile(void *handle, enum PP_SMC_POWER_PROFILE type, bool en) { struct pp_hwmgr *hwmgr = handle; - long workload; + long workload[1]; uint32_t index; if (!hwmgr || !hwmgr->pm_en) @@ -946,12 +946,12 @@ static int pp_dpm_switch_power_profile(void *handle, hwmgr->workload_mask &= ~(1 << hwmgr->workload_prority[type]); index = fls(hwmgr->workload_mask); index = index > 0 && index <= Workload_Policy_Max ? index - 1 : 0; - workload = hwmgr->workload_setting[index]; + workload[0] = hwmgr->workload_setting[index]; } else { hwmgr->workload_mask |= (1 << hwmgr->workload_prority[type]); index = fls(hwmgr->workload_mask); index = index <= Workload_Policy_Max ? index - 1 : 0; - workload = hwmgr->workload_setting[index]; + workload[0] = hwmgr->workload_setting[index]; } if (type == PP_SMC_POWER_PROFILE_COMPUTE && @@ -961,7 +961,7 @@ static int pp_dpm_switch_power_profile(void *handle, } if (hwmgr->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL) - hwmgr->hwmgr_func->set_power_profile_mode(hwmgr, &workload, 0); + hwmgr->hwmgr_func->set_power_profile_mode(hwmgr, workload, 0); return 0; } diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/pp_psm.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/pp_psm.c index 1d829402cd2e2..f4bd8e9357e22 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/pp_psm.c +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/pp_psm.c @@ -269,7 +269,7 @@ int psm_adjust_power_state_dynamic(struct pp_hwmgr *hwmgr, bool skip_display_set struct pp_power_state *new_ps) { uint32_t index; - long workload; + long workload[1]; if (hwmgr->not_vf) { if (!skip_display_settings) @@ -294,10 +294,10 @@ int psm_adjust_power_state_dynamic(struct pp_hwmgr *hwmgr, bool skip_display_set if (hwmgr->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL) { index = fls(hwmgr->workload_mask); index = index > 0 && index <= Workload_Policy_Max ? index - 1 : 0; - workload = hwmgr->workload_setting[index]; + workload[0] = hwmgr->workload_setting[index]; - if (hwmgr->power_profile_mode != workload && hwmgr->hwmgr_func->set_power_profile_mode) - hwmgr->hwmgr_func->set_power_profile_mode(hwmgr, &workload, 0); + if (hwmgr->power_profile_mode != workload[0] && hwmgr->hwmgr_func->set_power_profile_mode) + hwmgr->hwmgr_func->set_power_profile_mode(hwmgr, workload, 0); } return 0; diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c index 1d0693dad8185..91f0646eb3ee0 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c @@ -1834,7 +1834,7 @@ static int smu_adjust_power_state_dynamic(struct smu_context *smu, { int ret = 0; int index = 0; - long workload; + long workload[1]; struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm); if (!skip_display_settings) { @@ -1874,10 +1874,10 @@ static int smu_adjust_power_state_dynamic(struct smu_context *smu, smu_dpm_ctx->dpm_level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM) { index = fls(smu->workload_mask); index = index > 0 && index <= WORKLOAD_POLICY_MAX ? index - 1 : 0; - workload = smu->workload_setting[index]; + workload[0] = smu->workload_setting[index]; - if (smu->power_profile_mode != workload) - smu_bump_power_profile_mode(smu, &workload, 0); + if (smu->power_profile_mode != workload[0]) + smu_bump_power_profile_mode(smu, workload, 0); } return ret; @@ -1927,7 +1927,7 @@ static int smu_switch_power_profile(void *handle, { struct smu_context *smu = handle; struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm); - long workload; + long workload[1]; uint32_t index; if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) @@ -1940,17 +1940,17 @@ static int smu_switch_power_profile(void *handle, smu->workload_mask &= ~(1 << smu->workload_prority[type]); index = fls(smu->workload_mask); index = index > 0 && index <= WORKLOAD_POLICY_MAX ? index - 1 : 0; - workload = smu->workload_setting[index]; + workload[0] = smu->workload_setting[index]; } else { smu->workload_mask |= (1 << smu->workload_prority[type]); index = fls(smu->workload_mask); index = index <= WORKLOAD_POLICY_MAX ? index - 1 : 0; - workload = smu->workload_setting[index]; + workload[0] = smu->workload_setting[index]; } if (smu_dpm_ctx->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL && smu_dpm_ctx->dpm_level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM) - smu_bump_power_profile_mode(smu, &workload, 0); + smu_bump_power_profile_mode(smu, workload, 0); return 0; } -- 2.43.0

7 months, 2 weeks

1
16
0 0

[PATCH AUTOSEL 6.6 01/20] drm/amd/display: Add delay to improve LTTPR UHBR interop

by Sasha Levin

From: Michael Strauss <michael.strauss(a)amd.com> [ Upstream commit 10839ee6a977ed1f7d0f4deb29f2d7e5d1f2a9dd ] [WHY] Avoid race condition which puts LTTPR into bad state during UHBR LT. [HOW] Delay 30ms between starting UHBR TPS1 PHY output and sending TPS1 via DPCD. Reviewed-by: Wenjing Liu <wenjing.liu(a)amd.com> Acked-by: Aurabindo Pillai <aurabindo.pillai(a)amd.com> Signed-off-by: Michael Strauss <michael.strauss(a)amd.com> Tested-by: Daniel Wheeler <daniel.wheeler(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- .../dc/link/hwss/link_hwss_hpo_fixed_vs_pe_retimer_dp.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_hpo_fixed_vs_pe_retimer_dp.c b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_hpo_fixed_vs_pe_retimer_dp.c index b621b97711b61..a7f5b0f6272ce 100644 --- a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_hpo_fixed_vs_pe_retimer_dp.c +++ b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_hpo_fixed_vs_pe_retimer_dp.c @@ -162,7 +162,12 @@ static void set_hpo_fixed_vs_pe_retimer_dp_link_test_pattern(struct dc_link *lin link_res->hpo_dp_link_enc->funcs->set_link_test_pattern( link_res->hpo_dp_link_enc, tp_params); } + link->dc->link_srv->dp_trace_source_sequence(link, DPCD_SOURCE_SEQ_AFTER_SET_SOURCE_PATTERN); + + // Give retimer extra time to lock before updating DP_TRAINING_PATTERN_SET to TPS1 + if (tp_params->dp_phy_pattern == DP_TEST_PATTERN_128b_132b_TPS1_TRAINING_MODE) + msleep(30); } static void set_hpo_fixed_vs_pe_retimer_dp_lane_settings(struct dc_link *link, -- 2.43.0

7 months, 2 weeks

1
19
0 0

[PATCH AUTOSEL 6.10 01/34] drm/xe/preempt_fence: enlarge the fence critical section

by Sasha Levin

From: Matthew Auld <matthew.auld(a)intel.com> [ Upstream commit 3cd1585e57908b6efcd967465ef7685f40b2a294 ] It is really easy to introduce subtle deadlocks in preempt_fence_work_func() since we operate on single global ordered-wq for signalling our preempt fences behind the scenes, so even though we signal a particular fence, everything in the callback should be in the fence critical section, since blocking in the callback will prevent other published fences from signalling. If we enlarge the fence critical section to cover the entire callback, then lockdep should be able to understand this better, and complain if we grab a sensitive lock like vm->lock, which is also held when waiting on preempt fences. Signed-off-by: Matthew Auld <matthew.auld(a)intel.com> Cc: Matthew Brost <matthew.brost(a)intel.com> Reviewed-by: Matthew Brost <matthew.brost(a)intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240418144630.299531-2-matth… Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/xe/xe_preempt_fence.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_preempt_fence.c b/drivers/gpu/drm/xe/xe_preempt_fence.c index 7d50c6e89d8e7..5b243b7feb59d 100644 --- a/drivers/gpu/drm/xe/xe_preempt_fence.c +++ b/drivers/gpu/drm/xe/xe_preempt_fence.c @@ -23,11 +23,19 @@ static void preempt_fence_work_func(struct work_struct *w) q->ops->suspend_wait(q); dma_fence_signal(&pfence->base); - dma_fence_end_signalling(cookie); - + /* + * Opt for keep everything in the fence critical section. This looks really strange since we + * have just signalled the fence, however the preempt fences are all signalled via single + * global ordered-wq, therefore anything that happens in this callback can easily block + * progress on the entire wq, which itself may prevent other published preempt fences from + * ever signalling. Therefore try to keep everything here in the callback in the fence + * critical section. For example if something below grabs a scary lock like vm->lock, + * lockdep should complain since we also hold that lock whilst waiting on preempt fences to + * complete. + */ xe_vm_queue_rebind_worker(q->vm); - xe_exec_queue_put(q); + dma_fence_end_signalling(cookie); } static const char * -- 2.43.0

7 months, 2 weeks

1
33
0 0

c1839501fe3e tested

by Edmund Raile

c1839501fe3e ("ALSA: firewire-lib: fix wrong value as length of header for CIP_NO_HEADER case") indeed fixes the issue. Thank you very much! I just hope non-CIP_NO_HEADER cases work, but then again, looking at the original struct ``` struct { struct fw_iso_packet params; __be32 header[CIP_HEADER_QUADLETS]; } template = { {0}, {0} }; ``` the fw_iso_packet wasn't at the end of the array, thus ``` struct fw_iso_packet { u16 payload_length; /* Length of indirect payload */ u32 interrupt:1; /* Generate interrupt on this packet */ u32 skip:1; /* tx: Set to not send packet at all */ /* rx: Sync bit, wait for matching sy */ u32 tag:2; /* tx: Tag in packet header */ u32 sy:4; /* tx: Sy in packet header */ u32 header_length:8; /* Size of immediate header */ u32 header[]; /* tx: Top of 1394 isoch. data_block */ }; ``` its header array wouldn't ever end up being initialized here with > 0 elements. Maybe that wasn't ever required. Tested-by: Edmund Raile <edmund.raile(a)protonmail.com> Link: https://lore.kernel.org/r/20240725155640.128442-1-o-takashi@sakamocchi.jp

7 months, 2 weeks

1
0
0 0

[PATCH v2 0/2] ALSA: firewire-lib: restore process context workqueue to prevent deadlock

by Edmund Raile

This patchset serves to prevent a deadlock between process context and softIRQ context: A. In the process context * (lock A) Acquiring spin_lock by snd_pcm_stream_lock_irq() in snd_pcm_status64() * (lock B) Then attempt to enter tasklet B. In the softIRQ context * (lock B) Enter tasklet * (lock A) Attempt to acquire spin_lock by snd_pcm_stream_lock_irqsave() in snd_pcm_period_elapsed() ? tasklet_unlock_spin_wait </NMI> <TASK> ohci_flush_iso_completions firewire_ohci amdtp_domain_stream_pcm_pointer snd_firewire_lib snd_pcm_update_hw_ptr0 snd_pcm snd_pcm_status64 snd_pcm ? native_queued_spin_lock_slowpath </NMI> <IRQ> _raw_spin_lock_irqsave snd_pcm_period_elapsed snd_pcm process_rx_packets snd_firewire_lib irq_target_callback snd_firewire_lib handle_it_packet firewire_ohci context_tasklet firewire_ohci The issue has been reported as a regression of kernel 5.14: Link: https://lore.kernel.org/regressions/kwryofzdmjvzkuw6j3clftsxmoolynljztxqwg7… ("[REGRESSION] ALSA: firewire-lib: snd_pcm_period_elapsed deadlock with Fireface 800") Commit 7ba5ca32fe6e ("ALSA: firewire-lib: operate for period elapse event in process context") removed the process context workqueue from amdtp_domain_stream_pcm_pointer() and update_pcm_pointers() to remove its overhead. Commit b5b519965c4c ("ALSA: firewire-lib: obsolete workqueue for period update") belongs to the same patch series and removed the now-unused workqueue entirely. Though being observed on RME Fireface 800, this issue would affect all Firewire audio interfaces using ohci amdtp + pcm streaming. ALSA streaming, especially under intensive CPU load will reveal this issue the soonest due to issuing more hardIRQs, with time to occurrence ranging from 2 secons to 30 minutes after starting playback. to reproduce the issue: direct ALSA playback to the device: mpv --audio-device=alsa/sysdefault:CARD=Fireface800 Spor-Ignition.flac Time to occurrence: 2s to 30m Likelihood increased by: - high CPU load stress --cpu $(nproc) - switching between applications via workspaces tested with i915 in Xfce PulsaAudio / PipeWire conceal the issue as they run PCM substream without period wakeup mode, issuing less hardIRQs. Backport note: Also applies to and fixes on (tested): 6.10.2, 6.9.12, 6.6.43, 6.1.102, 5.15.164 Edmund Raile (2): ALSA: firewire-lib: restore workqueue for process context ALSA: firewire-lib: prevent deadlock between process and softIRQ context sound/firewire/amdtp-stream.c | 36 ++++++++++++++++++++++------------- sound/firewire/amdtp-stream.h | 1 + 2 files changed, 24 insertions(+), 13 deletions(-) -- 2.45.2

7 months, 2 weeks

1
2
0 0

[PATCH v2 2/2] ALSA: firewire-lib: prevent deadlock between process and softIRQ context

by Edmund Raile

Commit 7ba5ca32fe6e ("ALSA: firewire-lib: operate for period elapse event in process context") removed the process context workqueue from amdtp_domain_stream_pcm_pointer() and update_pcm_pointers() to remove its overhead. With RME Fireface 800, this lead to a regression since Kernels 5.14.0, causing a deadlock with eventual system freeze under ALSA operation: A. In the process context * (lock A) Acquiring spin_lock by snd_pcm_stream_lock_irq() in snd_pcm_status64() * (lock B) Then attempt to enter tasklet B. In the softIRQ context * (lock B) Enter tasklet * (lock A) Attempt to acquire spin_lock by snd_pcm_stream_lock_irqsave() in snd_pcm_period_elapsed() ? tasklet_unlock_spin_wait </NMI> <TASK> ohci_flush_iso_completions firewire_ohci amdtp_domain_stream_pcm_pointer snd_firewire_lib snd_pcm_update_hw_ptr0 snd_pcm snd_pcm_status64 snd_pcm ? native_queued_spin_lock_slowpath </NMI> <IRQ> _raw_spin_lock_irqsave snd_pcm_period_elapsed snd_pcm process_rx_packets snd_firewire_lib irq_target_callback snd_firewire_lib handle_it_packet firewire_ohci context_tasklet firewire_ohci Restore the process context work queue to prevent deadlock between ALSA substream process context spin_lock of snd_pcm_stream_lock_irq() in snd_pcm_status64() and OHCI 1394 IT softIRQ context spin_lock of snd_pcm_stream_lock_irqsave() in snd_pcm_period_elapsed(). Cc: stable(a)vger.kernel.org Fixes: 7ba5ca32fe6e ("ALSA: firewire-lib: operate for period elapse event in process context") Link: https://lore.kernel.org/r/kwryofzdmjvzkuw6j3clftsxmoolynljztxqwg76hzeo4simn… Reported-by: edmund.raile <edmund.raile(a)proton.me> Signed-off-by: Edmund Raile <edmund.raile(a)protonmail.com> --- sound/firewire/amdtp-stream.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 31201d506a21..c037d7de1fdc 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -615,16 +615,9 @@ static void update_pcm_pointers(struct amdtp_stream *s, // The program in user process should periodically check the status of intermediate // buffer associated to PCM substream to process PCM frames in the buffer, instead // of receiving notification of period elapsed by poll wait. - if (!pcm->runtime->no_period_wakeup) { - if (in_softirq()) { - // In software IRQ context for 1394 OHCI. - snd_pcm_period_elapsed(pcm); - } else { - // In process context of ALSA PCM application under acquired lock of - // PCM substream. - snd_pcm_period_elapsed_under_stream_lock(pcm); - } - } + if (!pcm->runtime->no_period_wakeup) + // wq used with amdtp_domain_stream_pcm_pointer() to prevent deadlock + queue_work(system_highpri_wq, &s->period_work); } } @@ -1866,9 +1859,11 @@ unsigned long amdtp_domain_stream_pcm_pointer(struct amdtp_domain *d, // Process isochronous packets queued till recent isochronous cycle to handle PCM frames. if (irq_target && amdtp_stream_running(irq_target)) { - // In software IRQ context, the call causes dead-lock to disable the tasklet - // synchronously. - if (!in_softirq()) + // use wq to prevent deadlock between process context spin_lock + // of snd_pcm_stream_lock_irq() in snd_pcm_status64() and + // softIRQ context spin_lock of snd_pcm_stream_lock_irqsave() + // in snd_pcm_period_elapsed() + if ((!in_softirq()) && (current_work() != &s->period_work)) fw_iso_context_flush_completions(irq_target->context); } -- 2.45.2

7 months, 2 weeks

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror July 2024