| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <bbd19ffd-038c-435c-a63b-260a0b933660@linaro.org>
Date: Wed, 15 Jan 2025 15:51:55 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Depeng Shao <quic_depengs@...cinc.com>, rfoss@...nel.org,
todor.too@...il.com, mchehab@...nel.org, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, vladimir.zapolskiy@...aro.org,
hverkuil@...all.nl
Cc: quic_eberman@...cinc.com, linux-media@...r.kernel.org,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, kernel@...cinc.com
Subject: Re: [PATCH v8 11/16] media: qcom: camss: csid: Only add TPG v4l2 ctrl
if TPG hardware is available
On 08/01/2025 14:37, Depeng Shao wrote:
> There is no CSID TPG on some SoCs, so the v4l2 ctrl in CSID driver
> shouldn't be registered. Checking the supported TPG modes to indicate
> if the TPG hardware exists or not and only registering v4l2 ctrl for
> CSID only when the TPG hardware is present.
>
> Signed-off-by: Depeng Shao <quic_depengs@...cinc.com>
> ---
> .../media/platform/qcom/camss/camss-csid.c | 60 +++++++++++--------
> 1 file changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
> index 6cf8e434dc05..e26a69a454a7 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
> @@ -760,11 +760,13 @@ static int csid_set_stream(struct v4l2_subdev *sd, int enable)
> int ret;
>
> if (enable) {
> - ret = v4l2_ctrl_handler_setup(&csid->ctrls);
> - if (ret < 0) {
> - dev_err(csid->camss->dev,
> - "could not sync v4l2 controls: %d\n", ret);
> - return ret;
> + if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED) {
> + ret = v4l2_ctrl_handler_setup(&csid->ctrls);
> + if (ret < 0) {
> + dev_err(csid->camss->dev,
> + "could not sync v4l2 controls: %d\n", ret);
> + return ret;
> + }
> }
>
> if (!csid->testgen.enabled &&
> @@ -838,7 +840,8 @@ static void csid_try_format(struct csid_device *csid,
> break;
>
> case MSM_CSID_PAD_SRC:
> - if (csid->testgen_mode->cur.val == 0) {
> + if (csid->testgen.nmodes == CSID_PAYLOAD_MODE_DISABLED ||
> + csid->testgen_mode->cur.val == 0) {
> /* Test generator is disabled, */
> /* keep pad formats in sync */
> u32 code = fmt->code;
> @@ -888,7 +891,8 @@ static int csid_enum_mbus_code(struct v4l2_subdev *sd,
>
> code->code = csid->res->formats->formats[code->index].code;
> } else {
> - if (csid->testgen_mode->cur.val == 0) {
> + if (csid->testgen.nmodes == CSID_PAYLOAD_MODE_DISABLED ||
> + csid->testgen_mode->cur.val == 0) {
> struct v4l2_mbus_framefmt *sink_fmt;
>
> sink_fmt = __csid_get_format(csid, sd_state,
> @@ -1267,7 +1271,8 @@ static int csid_link_setup(struct media_entity *entity,
>
> /* If test generator is enabled */
> /* do not allow a link from CSIPHY to CSID */
> - if (csid->testgen_mode->cur.val != 0)
> + if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED &&
> + csid->testgen_mode->cur.val != 0)
> return -EBUSY;
>
> sd = media_entity_to_v4l2_subdev(remote->entity);
> @@ -1360,24 +1365,27 @@ int msm_csid_register_entity(struct csid_device *csid,
> MSM_CSID_NAME, csid->id);
> v4l2_set_subdevdata(sd, csid);
>
> - ret = v4l2_ctrl_handler_init(&csid->ctrls, 1);
> - if (ret < 0) {
> - dev_err(dev, "Failed to init ctrl handler: %d\n", ret);
> - return ret;
> - }
> + if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED) {
> + ret = v4l2_ctrl_handler_init(&csid->ctrls, 1);
> + if (ret < 0) {
> + dev_err(dev, "Failed to init ctrl handler: %d\n", ret);
> + return ret;
> + }
>
> - csid->testgen_mode = v4l2_ctrl_new_std_menu_items(&csid->ctrls,
> - &csid_ctrl_ops, V4L2_CID_TEST_PATTERN,
> - csid->testgen.nmodes, 0, 0,
> - csid->testgen.modes);
> + csid->testgen_mode =
> + v4l2_ctrl_new_std_menu_items(&csid->ctrls,
> + &csid_ctrl_ops, V4L2_CID_TEST_PATTERN,
> + csid->testgen.nmodes, 0, 0,
> + csid->testgen.modes);
>
> - if (csid->ctrls.error) {
> - dev_err(dev, "Failed to init ctrl: %d\n", csid->ctrls.error);
> - ret = csid->ctrls.error;
> - goto free_ctrl;
> - }
> + if (csid->ctrls.error) {
> + dev_err(dev, "Failed to init ctrl: %d\n", csid->ctrls.error);
> + ret = csid->ctrls.error;
> + goto free_ctrl;
> + }
>
> - csid->subdev.ctrl_handler = &csid->ctrls;
> + csid->subdev.ctrl_handler = &csid->ctrls;
> + }
>
> ret = csid_init_formats(sd, NULL);
> if (ret < 0) {
> @@ -1408,7 +1416,8 @@ int msm_csid_register_entity(struct csid_device *csid,
> media_cleanup:
> media_entity_cleanup(&sd->entity);
> free_ctrl:
> - v4l2_ctrl_handler_free(&csid->ctrls);
> + if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
> + v4l2_ctrl_handler_free(&csid->ctrls);
>
> return ret;
> }
> @@ -1421,7 +1430,8 @@ void msm_csid_unregister_entity(struct csid_device *csid)
> {
> v4l2_device_unregister_subdev(&csid->subdev);
> media_entity_cleanup(&csid->subdev.entity);
> - v4l2_ctrl_handler_free(&csid->ctrls);
> + if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
> + v4l2_ctrl_handler_free(&csid->ctrls);
> }
>
> inline bool csid_is_lite(struct csid_device *csid)
The TPG on the RB5 has a known bug that not all test patterns work. I
verified that the coloured box TPG still works after this change.
Like so:
# colour bars test pattern 9
media-ctl --reset
yavta --no-query -w '0x009f0903 9' /dev/v4l-subdev6
yavta --list /dev/v4l-subdev6
media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:SGRBG10_1X10/3280x2464]'
media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -d /dev/media0 -p
yavta -B capture-mplane --capture=5 -n 5 -I -f SGRBG10P -s 3280x2464
--file=TPG-SGRBG10-3280x2464-000-#.bin /dev/video0
I think we had some confusion about the TPG regressing on v6/v7 of this
patch but, I suspect the wrong test pattern was tested.
This works as expected for me.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org> # qrb5165 rb5
Powered by blists - more mailing lists