| Message ID | 20240320141136.26827-9-quic_depengs@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: qcom: camss: Add sm8550 support | expand |
On 20/03/2024 14:11, Depeng Shao wrote: > Add in functional logic throughout the code to support the SM8550. > > Signed-off-by: Depeng Shao <quic_depengs@quicinc.com> > --- > .../media/platform/qcom/camss/camss-csid.c | 19 +++++++++++++++++++ > .../media/platform/qcom/camss/camss-csiphy.c | 1 + > drivers/media/platform/qcom/camss/camss-vfe.c | 7 +++++++ > .../media/platform/qcom/camss/camss-video.c | 1 + > 4 files changed, 28 insertions(+) > > diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c > index eb27d69e89a1..e9203dc15798 100644 > --- a/drivers/media/platform/qcom/camss/camss-csid.c > +++ b/drivers/media/platform/qcom/camss/camss-csid.c > @@ -590,6 +590,25 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid, > csid->base = camss->vfe[id].base + VFE_480_LITE_CSID_OFFSET; > else > csid->base = camss->vfe[id].base + VFE_480_CSID_OFFSET; > + } else if (camss->res->version == CAMSS_8550) { > + /* for titan 780, CSID lite registers are inside the VFE lite region, > + * between the VFE "top" and "bus" registers. this requires > + * VFE to be initialized before CSID > + */ > + if (id >= 2) > + csid->base = camss->vfe[id].base; Hard-coded magic numbers are definitely out. If you need to differentiate - please include something in the struct resources so that the flag is always available and we don't have to start doing funky magic index/magic number gymnastics. > + else { > + csid->base = > + devm_platform_ioremap_resource_byname(pdev, res->reg[0]); > + if (id != 0) > + csid->top_base = camss->csid[0].top_base; > + else > + csid->top_base = > + devm_platform_ioremap_resource_byname(pdev, res->reg[1]); > + } What is the point of hooking the TOP base just to clear our the status registers ? We take no meaningful action in the ISR that I can see. > + > + if (IS_ERR(csid->base)) > + return PTR_ERR(csid->base); > } else { > csid->base = devm_platform_ioremap_resource_byname(pdev, res->reg[0]); > if (IS_ERR(csid->base)) > diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c > index 45b3a8e5dea4..f35af0dd2147 100644 > --- a/drivers/media/platform/qcom/camss/camss-csiphy.c > +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c > @@ -579,6 +579,7 @@ int msm_csiphy_subdev_init(struct camss *camss, > case CAMSS_845: > case CAMSS_8250: > case CAMSS_8280XP: > + case CAMSS_8550: > csiphy->formats = csiphy_formats_sdm845; > csiphy->nformats = ARRAY_SIZE(csiphy_formats_sdm845); > break; > diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c > index d875237cf244..ff115c5521c6 100644 > --- a/drivers/media/platform/qcom/camss/camss-vfe.c > +++ b/drivers/media/platform/qcom/camss/camss-vfe.c > @@ -226,6 +226,7 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code, > case CAMSS_845: > case CAMSS_8250: > case CAMSS_8280XP: > + case CAMSS_8550: > switch (sink_code) { > case MEDIA_BUS_FMT_YUYV8_1X16: > { > @@ -296,6 +297,10 @@ int vfe_reset(struct vfe_device *vfe) > > reinit_completion(&vfe->reset_complete); > > + // The reset has been moved to csid in 8550 Please run checkpatch.pl on your code before submission C++ are not allowed. --- bod
Hi Bryan, On 3/21/2024 12:01 AM, Bryan O'Donoghue wrote: > On 20/03/2024 14:11, Depeng Shao wrote: >> Add in functional logic throughout the code to support the SM8550. >> >> Signed-off-by: Depeng Shao <quic_depengs@quicinc.com> >> --- >> .../media/platform/qcom/camss/camss-csid.c | 19 +++++++++++++++++++ >> .../media/platform/qcom/camss/camss-csiphy.c | 1 + >> drivers/media/platform/qcom/camss/camss-vfe.c | 7 +++++++ >> .../media/platform/qcom/camss/camss-video.c | 1 + >> 4 files changed, 28 insertions(+) >> >> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c >> b/drivers/media/platform/qcom/camss/camss-csid.c >> index eb27d69e89a1..e9203dc15798 100644 >> --- a/drivers/media/platform/qcom/camss/camss-csid.c >> +++ b/drivers/media/platform/qcom/camss/camss-csid.c >> @@ -590,6 +590,25 @@ int msm_csid_subdev_init(struct camss *camss, >> struct csid_device *csid, >> csid->base = camss->vfe[id].base + >> VFE_480_LITE_CSID_OFFSET; >> else >> csid->base = camss->vfe[id].base + VFE_480_CSID_OFFSET; >> + } else if (camss->res->version == CAMSS_8550) { >> + /* for titan 780, CSID lite registers are inside the VFE lite >> region, >> + * between the VFE "top" and "bus" registers. this requires >> + * VFE to be initialized before CSID >> + */ >> + if (id >= 2) >> + csid->base = camss->vfe[id].base; > > Hard-coded magic numbers are definitely out. > > If you need to differentiate - please include something in the struct > resources so that the flag is always available and we don't have to > start doing funky magic index/magic number gymnastics. > Sure, will update this part. >> + else { >> + csid->base = >> + devm_platform_ioremap_resource_byname(pdev, >> res->reg[0]); >> + if (id != 0) >> + csid->top_base = camss->csid[0].top_base; >> + else >> + csid->top_base = >> + devm_platform_ioremap_resource_byname(pdev, >> res->reg[1]); >> + } > > What is the point of hooking the TOP base just to clear our the status > registers ? > > We take no meaningful action in the ISR that I can see. > The csid top is a new small hardware block, it controls the output of CSID, since the CSID can connect to Sensor Front End(SFE) or original VFE in CSID gen3. > >> + >> + if (IS_ERR(csid->base)) >> + return PTR_ERR(csid->base); >> } else { >> csid->base = devm_platform_ioremap_resource_byname(pdev, >> res->reg[0]); >> if (IS_ERR(csid->base)) >> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c >> b/drivers/media/platform/qcom/camss/camss-csiphy.c >> index 45b3a8e5dea4..f35af0dd2147 100644 >> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c >> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c >> @@ -579,6 +579,7 @@ int msm_csiphy_subdev_init(struct camss *camss, >> case CAMSS_845: >> case CAMSS_8250: >> case CAMSS_8280XP: >> + case CAMSS_8550: >> csiphy->formats = csiphy_formats_sdm845; >> csiphy->nformats = ARRAY_SIZE(csiphy_formats_sdm845); >> break; >> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c >> b/drivers/media/platform/qcom/camss/camss-vfe.c >> index d875237cf244..ff115c5521c6 100644 >> --- a/drivers/media/platform/qcom/camss/camss-vfe.c >> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c >> @@ -226,6 +226,7 @@ static u32 vfe_src_pad_code(struct vfe_line *line, >> u32 sink_code, >> case CAMSS_845: >> case CAMSS_8250: >> case CAMSS_8280XP: >> + case CAMSS_8550: >> switch (sink_code) { >> case MEDIA_BUS_FMT_YUYV8_1X16: >> { >> @@ -296,6 +297,10 @@ int vfe_reset(struct vfe_device *vfe) >> reinit_completion(&vfe->reset_complete); >> + // The reset has been moved to csid in 8550 > > Please run checkpatch.pl on your code before submission C++ are not > allowed. > > --- > bod Thanks, Depeng
diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c index eb27d69e89a1..e9203dc15798 100644 --- a/drivers/media/platform/qcom/camss/camss-csid.c +++ b/drivers/media/platform/qcom/camss/camss-csid.c @@ -590,6 +590,25 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid, csid->base = camss->vfe[id].base + VFE_480_LITE_CSID_OFFSET; else csid->base = camss->vfe[id].base + VFE_480_CSID_OFFSET; + } else if (camss->res->version == CAMSS_8550) { + /* for titan 780, CSID lite registers are inside the VFE lite region, + * between the VFE "top" and "bus" registers. this requires + * VFE to be initialized before CSID + */ + if (id >= 2) + csid->base = camss->vfe[id].base; + else { + csid->base = + devm_platform_ioremap_resource_byname(pdev, res->reg[0]); + if (id != 0) + csid->top_base = camss->csid[0].top_base; + else + csid->top_base = + devm_platform_ioremap_resource_byname(pdev, res->reg[1]); + } + + if (IS_ERR(csid->base)) + return PTR_ERR(csid->base); } else { csid->base = devm_platform_ioremap_resource_byname(pdev, res->reg[0]); if (IS_ERR(csid->base)) diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c index 45b3a8e5dea4..f35af0dd2147 100644 --- a/drivers/media/platform/qcom/camss/camss-csiphy.c +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c @@ -579,6 +579,7 @@ int msm_csiphy_subdev_init(struct camss *camss, case CAMSS_845: case CAMSS_8250: case CAMSS_8280XP: + case CAMSS_8550: csiphy->formats = csiphy_formats_sdm845; csiphy->nformats = ARRAY_SIZE(csiphy_formats_sdm845); break; diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c index d875237cf244..ff115c5521c6 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe.c +++ b/drivers/media/platform/qcom/camss/camss-vfe.c @@ -226,6 +226,7 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code, case CAMSS_845: case CAMSS_8250: case CAMSS_8280XP: + case CAMSS_8550: switch (sink_code) { case MEDIA_BUS_FMT_YUYV8_1X16: { @@ -296,6 +297,10 @@ int vfe_reset(struct vfe_device *vfe) reinit_completion(&vfe->reset_complete); + // The reset has been moved to csid in 8550 + if (vfe->camss->res->version == CAMSS_8550) + return 0; + vfe->ops->global_reset(vfe); time = wait_for_completion_timeout(&vfe->reset_complete, @@ -1520,6 +1525,7 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe, case CAMSS_845: case CAMSS_8250: case CAMSS_8280XP: + case CAMSS_8550: l->formats = formats_rdi_845; l->nformats = ARRAY_SIZE(formats_rdi_845); break; @@ -1605,6 +1611,7 @@ static int vfe_bpl_align(struct vfe_device *vfe) case CAMSS_845: case CAMSS_8250: case CAMSS_8280XP: + case CAMSS_8550: ret = 16; break; default: diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c index 54cd82f74115..78a746be952c 100644 --- a/drivers/media/platform/qcom/camss/camss-video.c +++ b/drivers/media/platform/qcom/camss/camss-video.c @@ -1029,6 +1029,7 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev, case CAMSS_845: case CAMSS_8250: case CAMSS_8280XP: + case CAMSS_8550: video->formats = formats_rdi_845; video->nformats = ARRAY_SIZE(formats_rdi_845); break;
Add in functional logic throughout the code to support the SM8550. Signed-off-by: Depeng Shao <quic_depengs@quicinc.com> --- .../media/platform/qcom/camss/camss-csid.c | 19 +++++++++++++++++++ .../media/platform/qcom/camss/camss-csiphy.c | 1 + drivers/media/platform/qcom/camss/camss-vfe.c | 7 +++++++ .../media/platform/qcom/camss/camss-video.c | 1 + 4 files changed, 28 insertions(+)