Message ID | 20250108143733.2761200-1-quic_depengs@quicinc.com |
---|---|
Headers | show |
Series | media: qcom: camss: Add sm8550 support | expand |
On 08/01/2025 14:37, Depeng Shao wrote: > v8: > - Add correct version number for each patch - Krzysztof, Hans, Bryan > - Correct the copyright in patches [15/16] and [16/16] > - Link to v7: https://lore.kernel.org/all/20241225133523.4034820-1-quic_depengs@quicinc.com/ Patch #9 doesn't apply to media.git/next git remote add media git://linuxtv.org/media.git git fetch media git checkout -b media-next-25-10-01-camss-8550 media/next b4 shazam 20250108143733.2761200-1-quic_depengs@quicinc.com Grabbing thread from lore.kernel.org/all/20250108143733.2761200-1-quic_depengs@quicinc.com/t.mbox.gz Checking for newer revisions Grabbing search results from lore.kernel.org Analyzing 17 messages in the thread Analyzing 260 code-review messages Checking attestation on all messages, may take a moment... --- ✓ [PATCH v8 1/16] media: qcom: camss: csiphy-3ph: Fix trivial indentation fault in defines ✓ [PATCH v8 2/16] media: qcom: camss: csiphy-3ph: Remove redundant PHY init sequence control loop ✓ [PATCH v8 3/16] media: qcom: camss: csiphy-3ph: Rename struct ✓ [PATCH v8 4/16] media: qcom: camss: csiphy: Add an init callback to CSI PHY devices ✓ [PATCH v8 5/16] media: qcom: camss: csiphy-3ph: Move CSIPHY variables to data field inside csiphy struct ✓ [PATCH v8 6/16] media: qcom: camss: csiphy-3ph: Use an offset variable to find common control regs ✓ [PATCH v8 7/16] media: qcom: camss: csid: Move common code into csid core ✓ [PATCH v8 8/16] media: qcom: camss: vfe: Move common code into vfe core ✓ [PATCH v8 9/16] media: qcom: camss: Add callback API for RUP update and buf done ✓ [PATCH v8 10/16] media: qcom: camss: Add default case in vfe_src_pad_code ✓ [PATCH v8 11/16] media: qcom: camss: csid: Only add TPG v4l2 ctrl if TPG hardware is available ✓ [PATCH v8 12/16] dt-bindings: media: camss: Add qcom,sm8550-camss binding ✓ [PATCH v8 13/16] media: qcom: camss: Add sm8550 compatible ✓ [PATCH v8 14/16] media: qcom: camss: csiphy-3ph: Add Gen2 v2.1.2 two-phase MIPI CSI-2 DPHY support ✓ [PATCH v8 15/16] media: qcom: camss: Add CSID 780 support ✓ [PATCH v8 16/16] media: qcom: camss: Add support for VFE 780 --- ✓ Signed: DKIM/quicinc.com --- Total patches: 16 --- Base: using specified base-commit 8155b4ef3466f0e289e8fcc9e6e62f3f4dceeac2 Applying: media: qcom: camss: csiphy-3ph: Fix trivial indentation fault in defines Applying: media: qcom: camss: csiphy-3ph: Remove redundant PHY init sequence control loop Applying: media: qcom: camss: csiphy-3ph: Rename struct Applying: media: qcom: camss: csiphy: Add an init callback to CSI PHY devices Applying: media: qcom: camss: csiphy-3ph: Move CSIPHY variables to data field inside csiphy struct Applying: media: qcom: camss: csiphy-3ph: Use an offset variable to find common control regs Applying: media: qcom: camss: csid: Move common code into csid core Applying: media: qcom: camss: vfe: Move common code into vfe core Applying: media: qcom: camss: Add callback API for RUP update and buf done Patch failed at 0009 media: qcom: camss: Add callback API for RUP update and buf done error: patch failed: drivers/media/platform/qcom/camss/camss.c:2454
Hi Bryan, On 1/10/2025 9:50 PM, Bryan O'Donoghue wrote: > On 08/01/2025 14:37, Depeng Shao wrote: >> v8: >> - Add correct version number for each patch - Krzysztof, Hans, Bryan >> - Correct the copyright in patches [15/16] and [16/16] >> - Link to v7: https://lore.kernel.org/all/20241225133523.4034820-1- >> quic_depengs@quicinc.com/ > Patch #9 doesn't apply to media.git/next > Yes, below patch[1] is merged these days, so my series get conflict. I will rebase my series and verify it next Monday. [1] Revert "media: qcom: camss: Restructure camss_link_entities" > git remote add media git://linuxtv.org/media.git > git fetch media > > git checkout -b media-next-25-10-01-camss-8550 media/next > > b4 shazam 20250108143733.2761200-1-quic_depengs@quicinc.com > Grabbing thread from lore.kernel.org/all/20250108143733.2761200-1- > quic_depengs@quicinc.com/t.mbox.gz > Checking for newer revisions > Grabbing search results from lore.kernel.org > Analyzing 17 messages in the thread > Analyzing 260 code-review messages > Checking attestation on all messages, may take a moment... > --- > ✓ [PATCH v8 1/16] media: qcom: camss: csiphy-3ph: Fix trivial > indentation fault in defines > ✓ [PATCH v8 2/16] media: qcom: camss: csiphy-3ph: Remove redundant > PHY init sequence control loop > ✓ [PATCH v8 3/16] media: qcom: camss: csiphy-3ph: Rename struct > ✓ [PATCH v8 4/16] media: qcom: camss: csiphy: Add an init callback to > CSI PHY devices > ✓ [PATCH v8 5/16] media: qcom: camss: csiphy-3ph: Move CSIPHY > variables to data field inside csiphy struct > ✓ [PATCH v8 6/16] media: qcom: camss: csiphy-3ph: Use an offset > variable to find common control regs > ✓ [PATCH v8 7/16] media: qcom: camss: csid: Move common code into > csid core > ✓ [PATCH v8 8/16] media: qcom: camss: vfe: Move common code into vfe > core > ✓ [PATCH v8 9/16] media: qcom: camss: Add callback API for RUP update > and buf done > ✓ [PATCH v8 10/16] media: qcom: camss: Add default case in > vfe_src_pad_code > ✓ [PATCH v8 11/16] media: qcom: camss: csid: Only add TPG v4l2 ctrl > if TPG hardware is available > ✓ [PATCH v8 12/16] dt-bindings: media: camss: Add qcom,sm8550-camss > binding > ✓ [PATCH v8 13/16] media: qcom: camss: Add sm8550 compatible > ✓ [PATCH v8 14/16] media: qcom: camss: csiphy-3ph: Add Gen2 v2.1.2 > two-phase MIPI CSI-2 DPHY support > ✓ [PATCH v8 15/16] media: qcom: camss: Add CSID 780 support > ✓ [PATCH v8 16/16] media: qcom: camss: Add support for VFE 780 > --- > ✓ Signed: DKIM/quicinc.com > --- > Total patches: 16 > --- > Base: using specified base-commit > 8155b4ef3466f0e289e8fcc9e6e62f3f4dceeac2 > Applying: media: qcom: camss: csiphy-3ph: Fix trivial indentation fault > in defines > Applying: media: qcom: camss: csiphy-3ph: Remove redundant PHY init > sequence control loop > Applying: media: qcom: camss: csiphy-3ph: Rename struct > Applying: media: qcom: camss: csiphy: Add an init callback to CSI PHY > devices > Applying: media: qcom: camss: csiphy-3ph: Move CSIPHY variables to data > field inside csiphy struct > Applying: media: qcom: camss: csiphy-3ph: Use an offset variable to find > common control regs > Applying: media: qcom: camss: csid: Move common code into csid core > Applying: media: qcom: camss: vfe: Move common code into vfe core > Applying: media: qcom: camss: Add callback API for RUP update and buf done > Patch failed at 0009 media: qcom: camss: Add callback API for RUP update > and buf done > error: patch failed: drivers/media/platform/qcom/camss/camss.c:2454 > Thanks, Depeng
On Wed, Jan 08, 2025 at 08:07:29PM +0530, Depeng Shao wrote: > Add bindings for qcom,sm8550-camss in order to support the camera > subsystem for sm8550. > > Co-developed-by: Yongsheng Li <quic_yon@quicinc.com> > Signed-off-by: Yongsheng Li <quic_yon@quicinc.com> > Signed-off-by: Depeng Shao <quic_depengs@quicinc.com> > --- > .../bindings/media/qcom,sm8550-camss.yaml | 597 ++++++++++++++++++ > 1 file changed, 597 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/qcom,sm8550-camss.yaml Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
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@quicinc.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@linaro.org> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # qrb5165 rb5
On 1/8/25 16:37, Depeng Shao wrote: > From: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > A .data field in the csiphy device structure allows us to extend out the > register layout of the three phase capable CSIPHY layer. > > Move the existing lane configuration structure to an encapsulating > structure -> struct csiphy_device_regs which is derived from the .data > field populated at PHY init time, as opposed to calculated at lane > configuration. > > Reviewed-by: default avatarVladimir Zapolskiy <vladimir.zapolskiy@linaro.org> Nit: Something's not right with this tag. --- Best Regards, Laurentiu > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Signed-off-by: Depeng Shao <quic_depengs@quicinc.com> > --- > .../qcom/camss/camss-csiphy-3ph-1-0.c | 54 ++++++++++--------- > .../media/platform/qcom/camss/camss-csiphy.h | 6 +++ > 2 files changed, 36 insertions(+), 24 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c > index b283df7634bb..39cc7109ccf0 100644 > --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c > +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c > @@ -470,32 +470,10 @@ static void csiphy_gen1_config_lanes(struct csiphy_device *csiphy, > static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy, > u8 settle_cnt) > { > - const struct csiphy_lane_regs *r; > - int i, array_size; > + const struct csiphy_lane_regs *r = csiphy->regs->lane_regs; > + int i, array_size = csiphy->regs->lane_array_size; > u32 val; > > - switch (csiphy->camss->res->version) { > - case CAMSS_7280: > - r = &lane_regs_sm8250[0]; > - array_size = ARRAY_SIZE(lane_regs_sm8250); > - break; > - case CAMSS_8250: > - r = &lane_regs_sm8250[0]; > - array_size = ARRAY_SIZE(lane_regs_sm8250); > - break; > - case CAMSS_8280XP: > - r = &lane_regs_sc8280xp[0]; > - array_size = ARRAY_SIZE(lane_regs_sc8280xp); > - break; > - case CAMSS_845: > - r = &lane_regs_sdm845[0]; > - array_size = ARRAY_SIZE(lane_regs_sdm845); > - break; > - default: > - WARN(1, "unknown cspi version\n"); > - return; > - } > - > for (i = 0; i < array_size; i++, r++) { > switch (r->csiphy_param_type) { > case CSIPHY_SETTLE_CNT_LOWER_BYTE: > @@ -588,6 +566,34 @@ static void csiphy_lanes_disable(struct csiphy_device *csiphy, > > static int csiphy_init(struct csiphy_device *csiphy) > { > + struct device *dev = csiphy->camss->dev; > + struct csiphy_device_regs *regs; > + > + regs = devm_kmalloc(dev, sizeof(*regs), GFP_KERNEL); > + if (!regs) > + return -ENOMEM; > + > + csiphy->regs = regs; > + > + switch (csiphy->camss->res->version) { > + case CAMSS_845: > + regs->lane_regs = &lane_regs_sdm845[0]; > + regs->lane_array_size = ARRAY_SIZE(lane_regs_sdm845); > + break; > + case CAMSS_7280: > + case CAMSS_8250: > + regs->lane_regs = &lane_regs_sm8250[0]; > + regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8250); > + break; > + case CAMSS_8280XP: > + regs->lane_regs = &lane_regs_sc8280xp[0]; > + regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp); > + break; > + default: > + WARN(1, "unknown csiphy version\n"); > + return -ENODEV; > + } > + > return 0; > } > > diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h > index 49393dfd5215..4d731597fed7 100644 > --- a/drivers/media/platform/qcom/camss/camss-csiphy.h > +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h > @@ -85,6 +85,11 @@ struct csiphy_subdev_resources { > const struct csiphy_formats *formats; > }; > > +struct csiphy_device_regs { > + const struct csiphy_lane_regs *lane_regs; > + int lane_array_size; > +}; > + > struct csiphy_device { > struct camss *camss; > u8 id; > @@ -103,6 +108,7 @@ struct csiphy_device { > struct csiphy_config cfg; > struct v4l2_mbus_framefmt fmt[MSM_CSIPHY_PADS_NUM]; > const struct csiphy_subdev_resources *res; > + struct csiphy_device_regs *regs; > }; > > struct camss_subdev_resources;
On 15/01/2025 18:01, Laurentiu Tudor wrote: >> Reviewed-by: default avatarVladimir Zapolskiy >> <vladimir.zapolskiy@linaro.org> > > Nit: Something's not right with this tag. > > --- > Best Regards, Laurentiu Doh. @Hans could you possibly drop the "default avatar" when applying ? --- bod
On 1/15/25 23:01, Bryan O'Donoghue wrote: > On 15/01/2025 18:01, Laurentiu Tudor wrote: >>> Reviewed-by: default avatarVladimir Zapolskiy >>> <vladimir.zapolskiy@linaro.org> >> >> Nit: Something's not right with this tag. Thank you for reporting. The original uncorrupted tag comes from this message: https://lore.kernel.org/all/4fdf4f1c-fac0-4c85-8154-45f797c6acfd@linaro.org/ >> --- >> Best Regards, Laurentiu > > Doh. > > @Hans could you possibly drop the "default avatar" when applying ? > FWIW this particular problem has been fixed in v9, while the v9 series itself should gain some time to be reviewed/tested. -- Best wishes, Vladimir
On 15/01/2025 21:15, Vladimir Zapolskiy wrote: >> >> @Hans could you possibly drop the "default avatar" when applying ? >> > > FWIW this particular problem has been fixed in v9, while the v9 series > itself should gain some time to be reviewed/tested. Ah, this is V8. I didn't see. --- bod