This reverts commit cc1ecabe67.
This commit has a basic flaw in that it relies on camss->res->csid_num as a
control to index the array camss->vfe[i].
Testing on a platform where csid_num > vfe_num showed this bug up.
camss->vfe should only be indexed by camss->res->vfe_num. Since this commit
is meant to make the code be more readable reverting will simply restore
the previous correct bounds checking.
We can make another pass at making camss_link_entities look prettier but,
for now we should zap the bug introduced.
Fixes: cc1ecabe67 ("media: qcom: camss: Restructure camss_link_entities")
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
hfi_core_ping() was added by 2017's
commit 09c2845e8f ("[media] media: venus: hfi: add Host Firmware
Interface (HFI)")
but has remained unused.
Remove it.
It was the only caller of the ->core_ping member of hfi_ops,
so remove it, and the venus_core_ping that it pointed to.
Note I've left pky_sys_ping which seems to be the lowest level
definition of the command.
Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Populate encoder and decoder node-name entries for the upstream parts. Once
done the compat="video-encoder" and compat="video-decoder" in the dtsi can
be dropped though the venus driver will continue to favour DT declared
video-encoder/video-decoder declarations over static declarations for
compatibility.
Tested-by: Renjiang Han <quic_renjiang@quicinc.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Add resource structure data and probe() logic to support static
declarations of encoder and decoder.
Right now we rely on video encoder/decoder selection happening in the dtb
but, this goes against the remit of device tree which is supposed to
describe hardware, not select functional logic in Linux drivers.
Provide two strings in the venus resource structure enc_nodename and
dec_nodename.
When set the venus driver will create an OF entry in-memory consistent
with:
dec_nodename {
compat = "video-decoder";
};
and/or
enc_nodename {
compat = "video-encoder";
};
This will allow us to reuse the existing driver scheme of relying on compat
names maintaining compatibility with old dtb files.
dec_nodename can be "video-decoder" or "video0"
enc_nodename can be "video-encoder" or "video1"
This change relies on of_changeset() API as a result select OF_DYNAMIC will
be added to venus/Kconfig
Tested-by: Renjiang Han <quic_renjiang@quicinc.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Match order of instances' initialization in venc_open()/vdec_close()
and order of destruction in venus_common_close().
Suggested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
This partially reverts commit that made hfi_session_destroy()
the first step of vdec/venc close(). The reason being is a
regression report when, supposedly, encode/decoder is closed
with still active streaming (no ->stop_streaming() call before
close()) and pending pkts, so isr_thread cannot find instance
and fails to process those pending pkts. This was the idea
behind the original patch - make it impossible to use instance
under destruction, because this is racy, but apparently there
are uses cases that depend on that unsafe pattern. Return to
the old (unsafe) behaviour for the time being (until a better
fix is found).
Fixes: 45b1a1b348 ("media: venus: sync with threaded IRQ during inst destruction")
Cc: stable@vger.kernel.org
Reported-by: Nathan Hebert <nhebert@google.com>
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Refactor the camss_link_entities function by breaking it down into
three distinct functions. Each function will handle the linking of
a specific entity separately.
SC7280 and later targets mandates for 1:1 linking for csid -> vfe.
i.e. csid0 can be mapped to vfe0 only.
Signed-off-by: Suresh Vankadara <quic_svankada@quicinc.com>
Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@quicinc.com>
Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Introducing a new function camss_link_err to avoid repition of
same error message, improving code maintainability.
Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE+QmuaPwR3wnBdVwACF8+vY7k4RUFAmc8Q80ACgkQCF8+vY7k
4RX+3g//dMBSmu3uC9OiXyfw3aB8w62RMeieRxSVPMdkiacUm1J8HyzHnXPXIUn3
tfBT9E/YbeFZ+PlrOXRDUi1i8jmN47VuwRe01rxxF/FdlYknC2eGH3Ug9DW90VBh
wmZ1kSjyjizwDkKAm+Jc2xynTaX+iInJ4Kzp9RStDZPuaqj2Qzd1qVRk2FJwAYRh
5dTpi0W1PexjxQXDIcnHi/tPapGLSP5PnrunrAJR0tYfp60wrKMaxTO36yJzbnDP
MxkF8A+9dWtePRqoPWxPIvnOVu/+Twc730xkQp62qPvwEM2HZRtU7cgQFlWos6p/
ijK2i6sAQslMhQ9oIyKlO7HpXX60rjE3XtdzEtGxBq6DyIqx1riN+OqJB2C4Cdsr
2qUET8aTIisPURw1ecNAbthvLt8tljBe08/eX0GYaWFjALJx3Pds23ahH8hw295N
o3SY5NaGmO9Tg6HzYLSwfBmxgGpWDuRic6PDCVKok5mS5D1+uV/tu8fQFiNiFNVe
Okufjvo7HtZ3+rWR90b/Udpz/lBB/dceppnUX2iKevrG190VHxEwJ2pQKkfdH9ha
LUZQajikiv5rbGxKIGrjrCnjrJ24TC2vCSPhkgOb1r91LUY4RUV61c3hZbH73rzQ
2Ykwvmy+gpE4GEYiJRfSR6KlXdBruMa5FToLUHEK0uBMSlG5k7Q=
=25BS
-----END PGP SIGNATURE-----
Merge tag 'media/v6.13-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
Pull media updates from Mauro Carvalho Chehab:
- removal of the old omap4iss media driver
- mantis: remove orphan mantis_core.h
- add support for Raspberypi CFE
- uvc driver got a co-maintainer
- main media tree moved to git://linuxtv.org/media.git
- lots of driver cleanups, updates and fixes
* tag 'media/v6.13-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media: (233 commits)
docs: media: update location of the media patches
MAINTAINERS: update location of media main tree
media: MAINTAINERS: Add Hans de Goede as USB VIDEO CLASS co-maintainer
media: platform: samsung: s5p-jpeg: Remove deadcode
media: qcom: camss: Add MSM8953 resources
media: dt-bindings: Add qcom,msm8953-camss
media: qcom: camss: implement pm domain ops for VFE v4.1
media: platform: exynos4-is: Fix an OF node reference leak in fimc_md_is_isp_available
media: adv7180: Also check for "adi,force-bt656-4"
media: dt-bindings: adv7180: Document 'adi,force-bt656-4'
media: mgb4: Fix inconsistent input/output alignment in loopback mode
media: replace obsolete hans.verkuil@cisco.com alias
Documentation: media: improve V4L2_CID_MIN_BUFFERS_FOR_*, doc
media: vicodec: add V4L2_CID_MIN_BUFFERS_FOR_* controls
media: atomisp: Add check for rgby_data memory allocation failure
media: atomisp: remove redundant re-checking of err
media: atomisp: Fix spelling errors reported by codespell
media: atomisp: Remove License information boilerplate
media: atomisp: Fix typos in comment
media: atomisp: hmm_bo: Fix spelling errors in hmm_bo.h
...
This commit describes the hardware layout for the MSM8953
for the following hardware blocks:
- 2 x VFE, 3 RDI per VFE
- 3 x CSID
- 3 x CSI PHY
Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
Acked-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
MSM8917 and MSM8953 have multiple VFE power-domains they
have to be powered on/off explicitly.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Factor out common instance destruction code into
a common function.
Suggested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
When destroying an inst we should make sure that we don't race
against threaded IRQ (or pending IRQ), otherwise we can concurrently
kfree() inst context and inst itself.
BUG: KASAN: slab-use-after-free in vb2_queue_error+0x80/0x90
Call trace:
dump_backtrace+0x1c4/0x1f8
show_stack+0x38/0x60
dump_stack_lvl+0x168/0x1f0
print_report+0x170/0x4c8
kasan_report+0x94/0xd0
__asan_report_load2_noabort+0x20/0x30
vb2_queue_error+0x80/0x90
venus_helper_vb2_queue_error+0x54/0x78
venc_event_notify+0xec/0x158
hfi_event_notify+0x878/0xd20
hfi_process_msg_packet+0x27c/0x4e0
venus_isr_thread+0x258/0x6e8
hfi_isr_thread+0x70/0x90
venus_isr_thread+0x34/0x50
irq_thread_fn+0x88/0x130
irq_thread+0x160/0x2c0
kthread+0x294/0x328
ret_from_fork+0x10/0x20
Allocated by task 20291:
kasan_set_track+0x4c/0x80
kasan_save_alloc_info+0x28/0x38
__kasan_kmalloc+0x84/0xa0
kmalloc_trace+0x7c/0x98
v4l2_m2m_ctx_init+0x74/0x280
venc_open+0x444/0x6d0
v4l2_open+0x19c/0x2a0
chrdev_open+0x374/0x3f0
do_dentry_open+0x710/0x10a8
vfs_open+0x88/0xa8
path_openat+0x1e6c/0x2700
do_filp_open+0x1a4/0x2e0
do_sys_openat2+0xe8/0x508
do_sys_open+0x15c/0x1a0
__arm64_sys_openat+0xa8/0xc8
invoke_syscall+0xdc/0x270
el0_svc_common+0x1ec/0x250
do_el0_svc+0x54/0x70
el0_svc+0x50/0xe8
el0t_64_sync_handler+0x48/0x120
el0t_64_sync+0x1a8/0x1b0
Freed by task 20291:
kasan_set_track+0x4c/0x80
kasan_save_free_info+0x3c/0x60
____kasan_slab_free+0x124/0x1a0
__kasan_slab_free+0x18/0x28
__kmem_cache_free+0x134/0x300
kfree+0xc8/0x1a8
v4l2_m2m_ctx_release+0x44/0x60
venc_close+0x78/0x130 [venus_enc]
v4l2_release+0x20c/0x2f8
__fput+0x328/0x7f0
____fput+0x2c/0x48
task_work_run+0x1e0/0x280
get_signal+0xfb8/0x1190
do_notify_resume+0x34c/0x16a8
el0_svc+0x9c/0xe8
el0t_64_sync_handler+0x48/0x120
el0t_64_sync+0x1a8/0x1b0
Rearrange inst destruction. First remove the inst from the
core->instances list, second synchronize IRQ/IRQ-thread to
make sure that nothing else would see the inst while we take
it down.
Fixes: 7472c1c691 ("[media] media: venus: vdec: add video decoder files")
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
We destroy mutex-es too early as they are still taken in
v4l2_fh_exit()->v4l2_event_unsubscribe()->v4l2_ctrl_find().
We should destroy mutex-es right before kfree(). Also
do not vdec_ctrl_deinit() before v4l2_fh_exit().
Fixes: 7472c1c691 ("[media] media: venus: vdec: add video decoder files")
Suggested-by: Tomasz Figa <tfiga@google.com>
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
HFI supports hierarchical P encoding and the ability to specify the
bitrate for the different layers.
Connect the controls that V4L2 provides and HFI supports.
Signed-off-by: Fritz Koenig <frkoenig@chromium.org>
Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Move the dynamic bitrate updating functionality to a separate function
so that it can be shared.
No functionality changes.
Signed-off-by: Fritz Koenig <frkoenig@chromium.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
It is not valid to call pm_runtime_set_suspended() for devices
with runtime PM enabled because it returns -EAGAIN if it is enabled
already and working. So, call pm_runtime_disable() before to fix it.
Cc: stable@vger.kernel.org
Fixes: af2c3834c8 ("[media] media: venus: adding core part and helper functions")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Since commit 88785982a1 ("media: vb2: use lock if wait_prepare/finish
are NULL") it is no longer needed to set the wait_prepare/finish
vb2_ops callbacks as long as the lock field in vb2_queue is set.
Since the vb2_ops_wait_prepare/finish callbacks already rely on that field,
we can safely drop these callbacks.
This simplifies the code and this is a step towards the goal of deleting
these callbacks.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> # for meson-ge2d
Acked-by: Andrzej Pietrasiewicz <andrzejtp2010@gmail.com>
On Qualcomm SM8250 SoC there are two sets of regulators, and each of
both sets is specific to six CSIPHY IPs. At the moment there is no
proper split of two "combined" regulators with quite arbitrary selected
names in the driver or platform CAMSS device tree node, however for sake
of clarity and better hardware description it makes sense to move the
currently existing regulator resources from all CSID subdevices to all
CSIPHY subdevices.
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
This change allows to properly assign and manage supply regulator
resources by CSIPHY subdevices of CAMSS, this is needed to fine tune
description of supply regulators on newer platforms, conversion of
old platforms to the new scheme is also anticipated.
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Tested-by: Depeng Shao <quic_depengs@quicinc.com> # SM8550
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
New SoCs have CSID devices inside of a shared "wrapper" i.e. a set of regs
which is responsible for manging the muxes of the CSID to various other
blocks throughout CAMSS.
Not every SoC has this top-level muxing layer so make it optional depending
on whether its declared as a resource or not.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
There is a chance to meet runtime issues during configuration of CAMSS
power domains, because on the error path dev_pm_domain_detach() is
unexpectedly called with NULL or error pointer.
One of the simplest ways to reproduce the problem is to probe CAMSS
driver before registration of CAMSS power domains, for instance if
a platform CAMCC driver is simply not built.
Warning backtrace example:
Unable to handle kernel NULL pointer dereference at virtual address 00000000000001a2
<snip>
pc : dev_pm_domain_detach+0x8/0x48
lr : camss_probe+0x374/0x9c0
<snip>
Call trace:
dev_pm_domain_detach+0x8/0x48
platform_probe+0x70/0xf0
really_probe+0xc4/0x2a8
__driver_probe_device+0x80/0x140
driver_probe_device+0x48/0x170
__device_attach_driver+0xc0/0x148
bus_for_each_drv+0x88/0xf0
__device_attach+0xb0/0x1c0
device_initial_probe+0x1c/0x30
bus_probe_device+0xb4/0xc0
deferred_probe_work_func+0x90/0xd0
process_one_work+0x164/0x3e0
worker_thread+0x310/0x420
kthread+0x120/0x130
ret_from_fork+0x10/0x20
Fixes: 23aa4f0cd3 ("media: qcom: camss: Move VFE power-domain specifics into vfe.c")
Cc: <stable@vger.kernel.org>
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
After commit 0edb555a65 ("platform: Make platform_driver::remove()
return void") .remove() is (again) the right callback to implement for
platform drivers.
Convert all platform drivers below drivers/media to use .remove(), with
the eventual goal to drop struct platform_driver::remove_new(). As
.remove() and .remove_new() have the same prototypes, conversion is done
by just changing the structure member name in the driver initializer.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Without these ops the v4l2-compliance blocking wait test will fail.
These ops are required to ensure that when VIDIOC_DQBUF has to
wait for buffers to arrive, the queue lock is correctly released
and retaken. Otherwise the wait for a buffer would block all other
queue ioctls.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Rather than hooking up the PM domain through devm_pm_opp_attach_genpd() and
manage the device-link, let's avoid the boilerplate-code by converting into
devm_pm_domain_attach_list().
Acked-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Link: https://lore.kernel.org/r/20241002122232.194245-10-ulf.hansson@linaro.org
An iommu domain is allocated in venus_firmware_init() and is attached to
core->fw.dev in the same function. Use iommu_paging_domain_alloc() to
make it explicit.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/20240610085555.88197-10-baolu.lu@linux.intel.com
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Replace the old style single element array at the end of the struct with
a flex array.
The code does not allocate this structure, so the size change should not
be a problem.
This fixes the following cocci warning:
drivers/media/platform/qcom/venus/hfi_helper.h:1233:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
The single data array data[1] is only used to save the extradata_size.
Replace it with a single element field.
This fixes the following cocci warning:
drivers/media/platform/qcom/venus/hfi_cmds.h:175:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Replace a single length element array with an element.
This fixes the following cocci warning:
drivers/media/platform/qcom/venus/hfi_cmds.h:77:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
The single element array data[1] is never used. Replace it with a
padding field of the same size.
This fixes the following cocci warning:
drivers/media/platform/qcom/venus/hfi_cmds.h:146:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
The single element array data[1] is never used. Replace it with a
padding field of the same size.
This fixes the following cocci error:
drivers/media/platform/qcom/venus/hfi_cmds.h:163:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
plane_info is not a typical array, the data is not contiguous:
pinfo = (void *)pinfo + sizeof(*constr) * num_planes +
2 * sizeof(u32);
Replace the single element array with a single element field.
This fixes the following cocci warning:
drivers/media/platform/qcom/venus/hfi_helper.h:1009:36-46: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
The struct hfi_session_get_property_pkt is always used to fetch a
single property. Make that explicit in the code and avoid a single
element array at the end of the struct.
This change fixes the following cocci warning:
drivers/media/platform/qcom/venus/hfi_cmds.h:194:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
This field is never used, but if we remove it we would change the size
of the struct and can lead to behavior change. Stay on the safe side by
replacing the single element array with a single element field.
This fixes the following cocci warning:
drivers/media/platform/qcom/venus/hfi_helper.h:1003:43-60: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
"struct dec_bufsize_ops and "struct enc_bufsize_ops" are not modified in
this driver.
Constifying these structures moves some data to a read-only section, so
increase overall security.
On a x86_64, with allmodconfig:
Before:
text data bss dec hex filename
12494 822 0 13316 3404 drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.o
After:
text data bss dec hex filename
12766 566 0 13332 3414 drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.o
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
in venus_probe, core->work is bound with venus_sys_error_handler, which is
used to handle error. The code use core->sys_err_done to make sync work.
The core->work is started in venus_event_notify.
If we call venus_remove, there might be an unfished work. The possible
sequence is as follows:
CPU0 CPU1
|venus_sys_error_handler
venus_remove |
hfi_destroy |
venus_hfi_destroy |
kfree(hdev); |
|hfi_reinit
|venus_hfi_queues_reinit
|//use hdev
Fix it by canceling the work in venus_remove.
Cc: stable@vger.kernel.org
Fixes: af2c3834c8 ("[media] media: venus: adding core part and helper functions")
Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
The only direct user of struct hfi_session_release_buffer_pkt is
pkt_session_unset_buffers() which sets "num_buffers" before using it
as a loop counter for accessing "buffer_info". Add the __counted_by
annotation to reflect the relationship.
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Link: https://lore.kernel.org/r/20240710230914.3156277-2-kees@kernel.org
Signed-off-by: Kees Cook <kees@kernel.org>
Replace the deprecated[1] use of a 1-element array in
struct hfi_session_release_buffer_pkt with a modern flexible array.
No binary differences are present after this conversion.
Link: https://github.com/KSPP/linux/issues/79 [1]
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://lore.kernel.org/r/20240710230914.3156277-1-kees@kernel.org
Signed-off-by: Kees Cook <kees@kernel.org>
pm_runtime_enable() should happen prior to vfe_get() since vfe_get() calls
pm_runtime_resume_and_get().
This is a basic race condition that doesn't show up for most users so is
not widely reported. If you blacklist qcom-camss in modules.d and then
subsequently modprobe the module post-boot it is possible to reliably show
this error up.
The kernel log for this error looks like this:
qcom-camss ac5a000.camss: Failed to power up pipeline: -13
Fixes: 02afa816db ("media: camss: Add basic runtime PM support")
Reported-by: Johan Hovold <johan+linaro@kernel.org>
Closes: https://lore.kernel.org/lkml/ZoVNHOTI0PKMNt4_@hovoldconsulting.com/
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Konrad Dybcio <konradybcio@kernel.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
The use_count check was introduced so that multiple concurrent Raw Data
Interfaces RDIs could be driven by different virtual channels VCs on the
CSIPHY input driving the video pipeline.
This is an invalid use of use_count though as use_count pertains to the
number of times a video entity has been opened by user-space not the number
of active streams.
If use_count and stream-on count don't agree then stop_streaming() will
break as is currently the case and has become apparent when using CAMSS
with libcamera's released softisp 0.3.
The use of use_count like this is a bit hacky and right now breaks regular
usage of CAMSS for a single stream case. Stopping qcam results in the splat
below, and then it cannot be started again and any attempts to do so fails
with -EBUSY.
[ 1265.509831] WARNING: CPU: 5 PID: 919 at drivers/media/common/videobuf2/videobuf2-core.c:2183 __vb2_queue_cancel+0x230/0x2c8 [videobuf2_common]
...
[ 1265.510630] Call trace:
[ 1265.510636] __vb2_queue_cancel+0x230/0x2c8 [videobuf2_common]
[ 1265.510648] vb2_core_streamoff+0x24/0xcc [videobuf2_common]
[ 1265.510660] vb2_ioctl_streamoff+0x5c/0xa8 [videobuf2_v4l2]
[ 1265.510673] v4l_streamoff+0x24/0x30 [videodev]
[ 1265.510707] __video_do_ioctl+0x190/0x3f4 [videodev]
[ 1265.510732] video_usercopy+0x304/0x8c4 [videodev]
[ 1265.510757] video_ioctl2+0x18/0x34 [videodev]
[ 1265.510782] v4l2_ioctl+0x40/0x60 [videodev]
...
[ 1265.510944] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 0 in active state
[ 1265.511175] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 1 in active state
[ 1265.511398] videobuf2_common: driver bug: stop_streaming operation is leaving buffer 2 in active st
One CAMSS specific way to handle multiple VCs on the same RDI might be:
- Reference count each pipeline enable for CSIPHY, CSID, VFE and RDIx.
- The video buffers are already associated with msm_vfeN_rdiX so
release video buffers when told to do so by stop_streaming.
- Only release the power-domains for the CSIPHY, CSID and VFE when
their internal refcounts drop.
Either way refusing to release video buffers based on use_count is
erroneous and should be reverted. The silicon enabling code for selecting
VCs is perfectly fine. Its a "known missing feature" that concurrent VCs
won't work with CAMSS right now.
Initial testing with this code didn't show an error but, SoftISP and "real"
usage with Google Hangouts breaks the upstream code pretty quickly, we need
to do a partial revert and take another pass at VCs.
This commit partially reverts commit 89013969e2 ("media: camss: sm8250:
Pipeline starting and stopping for multiple virtual channels")
Fixes: 89013969e2 ("media: camss: sm8250: Pipeline starting and stopping for multiple virtual channels")
Reported-by: Johan Hovold <johan+linaro@kernel.org>
Closes: https://lore.kernel.org/lkml/ZoVNHOTI0PKMNt4_@hovoldconsulting.com/
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE+QmuaPwR3wnBdVwACF8+vY7k4RUFAmaXfCQACgkQCF8+vY7k
4RWuBg/+NRAVuzYW3AQPIaggajTGHfkk6WCTCVgQQZFCDqphS6YtgfXUJ8qO5YXk
ZieGu+g2081BFehzcZxcaSo9pFWyqX1fjUU2sjFwRDSl9NRctsjvKE9J1DVKCsMW
QU5yOYyBJmoVugj4YCH7Yga8OElZAWperxJidV4AmFkX93OwZDZl+wNKuSTmG/lX
ju+Z6yzv0DN0WvgL8+LlZ2k5tpx+kAld07FFwQM54MPI9CBWyQjogGyro/1S6ymh
WAbwbEMCvGSvGhi4issMMOK2mpmh2EAKCXBMWF5bXNOLuFWrU9TtCBr6AITKDvn7
btQNpa8GApO+GehEQtWOX5WgZp2ypwCrMUtiwftPOtF4Z8Tl7MJfn4u6wWCxj4cy
67HbOgWRZQRIzyUSF8vay6PeMrh8jYi+unWuOxGpnzilno1nV2hTzh4n1we15qIn
8pnNSbtgrJCvrIgtATYjP1FWgjBxwuNIpFGxo2ly+hgbu6COLZFfg0Oju3FBdOF1
ZxGkp1SaxcKeuFa6kbATj7y2dAjtre8drB9RfJY1C97Ta+C9ws4jBytVHbceA7u+
GJfAis2CEStLPpe3ND9n0ekeB/qSPcgGC2HLQR7L1u30Kx75T4I49HF0lcKev9gK
oTRUPvZu/bI6NmSRwYYY7jo1rox5ffftJ2ZICeQaluV2dbOMUc8=
=7nBb
-----END PGP SIGNATURE-----
Merge tag 'media/v6.11-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
Pull media updates from Mauro Carvalho Chehab:
- New sensor drivers: gc05a2, gc08a3 and imx283
- New serializer/deserializer drivers: max96714 and max96717
- New JPEG encoder driver: e5010
- Support for Raspberry Pi PiSP Backend (BE) ISP driver
- Old documentation for av7110 driver removed, as a new version was
added as Documentation/userspace-api/media/dvb/legacy*.rst
- atompisp: Linux firmwares are now available, so drop firmware-related
task from TODO and update firmware logic
- The imx258 driver has gained several improvements
- wave5 driver has gained support for HEVC decoding
- em28xx gained support for MyGica UTV3
- av7110 budget-patch driver removed
- Lots of other cleanups, improvements and fixes
* tag 'media/v6.11-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media: (301 commits)
media: raspberrypi: Switch to remove_new
media: uapi: pisp_be_config: Add extra config fields
media: uapi: pisp_be_config: Re-sort pisp_be_tiles_config
media: uapi: pisp_common: Capitalize all macros
media: uapi: pisp_common: Add 32 bpp format test
media: uapi: pisp_be_config: Drop BIT() from uAPI
media: stm32: dcmipp: correct error handling in dcmipp_create_subdevs
media: atomisp: Fix spelling mistakes in sh_css_sp.c
media: atomisp: Fix spelling mistake in ia_css_debug.c
media: atomisp: Fix spelling mistake in hmm_bo.c
media: atomisp: Fix spelling mistake in ia_css_eed1_8.host.c
media: atomisp: Fix spelling mistake in sh_css_internal.h
media: atomisp: Fix spelling mistake "pipline" -> "pipeline"
media: atomisp: Remove unused GPIO related defines and APIs
media: atomisp: Replace COMPILATION_ERROR_IF() by static_assert()
media: atomisp: Clean up unused macros from math_support.h
media: atomisp: csi2-bridge: Add DMI quirk for OV5693 on Xiaomi Mipad2
media: atomisp: Update TODO
media: atomisp: Prefix firmware paths with "intel/ipu/"
media: atomisp: Remove firmware_name module parameter
...
The Venus driver requires vcodec GDSC to be ON in SW mode for clock
operations and move it back to HW mode to gain power benefits. Earlier,
as there is no interface to switch the GDSC mode from GenPD framework,
the GDSC is moved to HW control mode as part of GDSC enable callback and
venus driver is writing to its POWER_CONTROL register to keep the GDSC ON
from SW whereever required. But the POWER_CONTROL register addresses
are not constant and can vary across the variants.
Also as per the HW recommendation, the GDSC mode switching needs to be
controlled from respective GDSC register and this is a uniform approach
across all the targets. Hence use dev_pm_genpd_set_hwmode() API which
controls GDSC mode switching using its respective GDSC register.
In venus V6 variants, the vcodec gdsc gets enabled in SW mode by default
with new HW_CTRL_TRIGGER flag and there is no need to switch it to SW
mode again after enable, hence add check to avoid switching gdsc to SW mode
again after gdsc enable. Similarly add check to avoid switching GDSC to HW
mode before disabling the GDSC, so GDSC gets enabled in SW mode in the next
enable.
Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Taniya Das <quic_tdas@quicinc.com>
Link: https://lore.kernel.org/r/20240624044809.17751-6-quic_jkona@quicinc.com
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
There appears to be a possible use after free with vdec_close().
The firmware will add buffer release work to the work queue through
HFI callbacks as a normal part of decoding. Randomly closing the
decoder device from userspace during normal decoding can incur
a read after free for inst.
Fix it by cancelling the work in vdec_close.
Cc: stable@vger.kernel.org
Fixes: af2c3834c8 ("[media] media: venus: adding core part and helper functions")
Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
