-
sent/20240713-linux-next-24-07-13-camss-fixes-fa98c0965a5d-v346a4722f · ·
media: qcom: camss: Fix two CAMSS bugs found by dogfooding with SoftISP v3: - Amends the commit log for patch #1 per Johan's suggestion. - Link to v2: https://lore.kernel.org/r/20240716-linux-next-24-07-13-camss-fixes-v2-0-e60c9f6742f2@linaro.org v2: - Updates commits with Johan's Review/Reported tags - Adds Closes: https://lore.kernel.org/lkml/ZoVNHOTI0PKMNt4_@hovoldconsulting.com - Cc's stable - Adds in suggested kernel log to allow others to more easily match kernel log to fixes - Link to v1: https://lore.kernel.org/r/20240714-linux-next-24-07-13-camss-fixes-v1-0-8f8954bc8c85@linaro.org V1: Dogfooding with SoftISP has uncovered two bugs in this series which I'm posting fixes for. - The first error: A simple race condition which to be honest I'm surprised I haven't found earlier nor has anybody else. Simply stated the order we typically end up loading CAMSS on boot has masked out the pm_runtime_enable() race condition that has been present in CAMSS for a long time. If you blacklist qcom-camss in modules.d and then modprobe after boot, the race condition shows up easily. Moving the pm_runtime_enable prior to subdevice registration fixes the problem. The second error: Nomenclature: - CSIPHY: CSI Physical layer analogue to digital domain serialiser - CSID: CSI Decoder - VFE: Video Front End - RDI: Raw Data Interface - VC: Virtual Channel In order to support streaming multiple virtual-channels on the same RDI a V4L2 provided use_count variable is used to decide whether or not to actually terminate streaming and release buffers for 'msm_vfe_rdiX'. Unfortunately use_count indicates the number of times msm_vfe_rdiX has been opened by user-space not the number of concurrent streams on msm_vfe_rdiX. Simply stated use_count and stream_count are two different things. The silicon enabling code to select between VCs is valid but, a different solution needs to be found to support _concurrent_ VC streams. Right now the upstream use_count as-is is breaking the non concurrent VC case and I don't believe there are upstream users of concurrent VCs on CAMSS. This series implements a revert for the invalid use_count check, retaining the ability to select which VC is active on the RDI. Dogfooding with libcamera's SoftISP in Hangouts, Zoom and multiple runs of libcamera's "qcam" application is a very different test-case to the simple capture of frames we previously did when validating the 'use_count' change. A partial revert in expectation of a renewed push to fixup that concurrent VC issue is included. To: Robert Foss <rfoss@kernel.org> To: Todor Tomov <todor.too@gmail.com> To: Mauro Carvalho Chehab <mchehab@kernel.org> To: Hans Verkuil <hansverk@cisco.com> To: Hans Verkuil <hverkuil-cisco@xs4all.nl> To: Milen Mitkov <quic_mmitkov@quicinc.com> Cc: linux-media@vger.kernel.org Cc: linux-arm-msm@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> Cc: Johan Hovold <johan+linaro@kernel.org> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- Bryan O'Donoghue (2): media: qcom: camss: Remove use_count guard in stop_streaming media: qcom: camss: Fix ordering of pm_runtime_enable drivers/media/platform/qcom/camss/camss-video.c | 6 ------ drivers/media/platform/qcom/camss/camss.c | 5 +++-- 2 files changed, 3 insertions(+), 8 deletions(-) --- base-commit: c6ce8f9ab92edc9726996a0130bfc1c408132d47 change-id: 20240713-linux-next-24-07-13-camss-fixes-fa98c0965a5d Best regards,