| Message ID | 20241205155538.250743-9-quic_depengs@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: qcom: camss: Add sm8550 support | expand |
Hi Depeng, thank you for the changes and updates. On 12/5/24 17:55, Depeng Shao wrote: > From: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > The Test Pattern Generator TPG has been moved out of the CSID and into a > standalone silicon block at the same level as a regular CSIPHY. > > Make the TPG calls optional to reflect the fact some CSID blocks will now > not implement this feature. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Please don't forget to add your Signed-off-by tag, if you pull someone's changes. > --- > .../media/platform/qcom/camss/camss-csid.c | 33 ++++++++++++------- > 1 file changed, 21 insertions(+), 12 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c > index 6cf8e434dc05..2cb8c37982f8 100644 > --- a/drivers/media/platform/qcom/camss/camss-csid.c > +++ b/drivers/media/platform/qcom/camss/camss-csid.c > @@ -838,7 +838,7 @@ 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_mode || csid->testgen_mode->cur.val == 0) { > /* Test generator is disabled, */ > /* keep pad formats in sync */ > u32 code = fmt->code; > @@ -1042,6 +1042,7 @@ static int csid_init_formats(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > static int csid_set_test_pattern(struct csid_device *csid, s32 value) > { > struct csid_testgen_config *tg = &csid->testgen; > + const struct csid_hw_ops *hw_ops = csid->res->hw_ops; > > /* If CSID is linked to CSIPHY, do not allow to enable test generator */ > if (value && media_pad_remote_pad_first(&csid->pads[MSM_CSID_PAD_SINK])) > @@ -1049,7 +1050,10 @@ static int csid_set_test_pattern(struct csid_device *csid, s32 value) > > tg->enabled = !!value; > > - return csid->res->hw_ops->configure_testgen_pattern(csid, value); > + if (hw_ops->configure_testgen_pattern) > + return -EOPNOTSUPP; > + else > + return hw_ops->configure_testgen_pattern(csid, value); Last time I reported about the regression here, it is announced as fixed in the changelog, but I see it is not, unfortunately. > } > > /* > @@ -1267,7 +1271,7 @@ 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_mode && csid->testgen_mode->cur.val != 0) > return -EBUSY; > > sd = media_entity_to_v4l2_subdev(remote->entity); > @@ -1366,15 +1370,20 @@ int msm_csid_register_entity(struct csid_device *csid, > 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); > - > - 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->res->hw_ops->configure_testgen_pattern) { > + 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; > + } > } > > csid->subdev.ctrl_handler = &csid->ctrls; -- Best wishes, Vladimir
Hi Vladimir, >> --- >> .../media/platform/qcom/camss/camss-csid.c | 33 ++++++++++++------- >> 1 file changed, 21 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/ >> media/platform/qcom/camss/camss-csid.c >> index 6cf8e434dc05..2cb8c37982f8 100644 >> --- a/drivers/media/platform/qcom/camss/camss-csid.c >> +++ b/drivers/media/platform/qcom/camss/camss-csid.c >> @@ -838,7 +838,7 @@ 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_mode || csid->testgen_mode->cur.val == 0) { >> /* Test generator is disabled, */ >> /* keep pad formats in sync */ >> u32 code = fmt->code; >> @@ -1042,6 +1042,7 @@ static int csid_init_formats(struct v4l2_subdev >> *sd, struct v4l2_subdev_fh *fh) >> static int csid_set_test_pattern(struct csid_device *csid, s32 value) >> { >> struct csid_testgen_config *tg = &csid->testgen; >> + const struct csid_hw_ops *hw_ops = csid->res->hw_ops; >> /* If CSID is linked to CSIPHY, do not allow to enable test >> generator */ >> if (value && media_pad_remote_pad_first(&csid- >> >pads[MSM_CSID_PAD_SINK])) >> @@ -1049,7 +1050,10 @@ static int csid_set_test_pattern(struct >> csid_device *csid, s32 value) >> tg->enabled = !!value; >> - return csid->res->hw_ops->configure_testgen_pattern(csid, value); >> + if (hw_ops->configure_testgen_pattern) >> + return -EOPNOTSUPP; >> + else >> + return hw_ops->configure_testgen_pattern(csid, value); > > Last time I reported about the regression here, it is announced as fixed > in the > changelog, but I see it is not, unfortunately. > Sorry, I just picked Bryan's standalone change, then forget your comments. Will update it in new version. Will set the tpg mode as CSID_PAYLOAD_MODE_DISABLED in configure_testgen_pattern interface. Thanks, Depeng
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c index 6cf8e434dc05..2cb8c37982f8 100644 --- a/drivers/media/platform/qcom/camss/camss-csid.c +++ b/drivers/media/platform/qcom/camss/camss-csid.c @@ -838,7 +838,7 @@ 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_mode || csid->testgen_mode->cur.val == 0) { /* Test generator is disabled, */ /* keep pad formats in sync */ u32 code = fmt->code; @@ -1042,6 +1042,7 @@ static int csid_init_formats(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) static int csid_set_test_pattern(struct csid_device *csid, s32 value) { struct csid_testgen_config *tg = &csid->testgen; + const struct csid_hw_ops *hw_ops = csid->res->hw_ops; /* If CSID is linked to CSIPHY, do not allow to enable test generator */ if (value && media_pad_remote_pad_first(&csid->pads[MSM_CSID_PAD_SINK])) @@ -1049,7 +1050,10 @@ static int csid_set_test_pattern(struct csid_device *csid, s32 value) tg->enabled = !!value; - return csid->res->hw_ops->configure_testgen_pattern(csid, value); + if (hw_ops->configure_testgen_pattern) + return -EOPNOTSUPP; + else + return hw_ops->configure_testgen_pattern(csid, value); } /* @@ -1267,7 +1271,7 @@ 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_mode && csid->testgen_mode->cur.val != 0) return -EBUSY; sd = media_entity_to_v4l2_subdev(remote->entity); @@ -1366,15 +1370,20 @@ int msm_csid_register_entity(struct csid_device *csid, 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); - - 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->res->hw_ops->configure_testgen_pattern) { + 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; + } } csid->subdev.ctrl_handler = &csid->ctrls;