[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <67d014f1-9424-4b88-b031-096a5596c5c8@linaro.org>
Date: Wed, 13 Nov 2024 15:28:24 +0200
From: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>
To: barnabas.czeman@...nlining.org,
Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Cc: Robert Foss <rfoss@...nel.org>, Todor Tomov <todor.too@...il.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, linux-media@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, Vladimir Lypak <vladimir.lypak@...il.com>
Subject: Re: [PATCH v4 3/3] media: qcom: camss: Add MSM8953 resources
Hello Barnabás, Bryan.
On 11/13/24 10:01, barnabas.czeman@...nlining.org wrote:
> On 2024-11-13 05:58, Vladimir Zapolskiy wrote:
>> On 11/3/24 11:45, Barnabás Czémán wrote:
>>> From: Vladimir Lypak <vladimir.lypak@...il.com>
>>>
>>> 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@...il.com>
>>> Acked-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
>>> Signed-off-by: Barnabás Czémán <barnabas.czeman@...nlining.org>
>>> ---
>>> drivers/media/platform/qcom/camss/camss-csiphy.c | 1 +
>>> drivers/media/platform/qcom/camss/camss-ispif.c | 5 +
>>> drivers/media/platform/qcom/camss/camss-vfe.c | 1 +
>>> drivers/media/platform/qcom/camss/camss.c | 170
>>> +++++++++++++++++++++++
>>> drivers/media/platform/qcom/camss/camss.h | 1 +
>>> 5 files changed, 178 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c
>>> b/drivers/media/platform/qcom/camss/camss-csiphy.c
>>> index
>>> 68a3ea1ba2a5299cf28289dfdb958cfdff3c91e0..5af2b382a843c2b8857339ba28930fe1682c9412
>>> 100644
>>> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
>>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
>>> @@ -596,6 +596,7 @@ int msm_csiphy_subdev_init(struct camss *camss,
>>> return PTR_ERR(csiphy->base);
>>> if (camss->res->version == CAMSS_8x16 ||
>>> + camss->res->version == CAMSS_8x53 ||
>>> camss->res->version == CAMSS_8x96) {
>>> csiphy->base_clk_mux =
>>> devm_platform_ioremap_resource_byname(pdev, res->reg[1]);
>>> diff --git a/drivers/media/platform/qcom/camss/camss-ispif.c
>>> b/drivers/media/platform/qcom/camss/camss-ispif.c
>>> index
>>> a12dcc7ff438c55167bc2981fd399dbf178181df..2dc585c6123dd248a5bacd9c7a88cb5375644311
>>> 100644
>>> --- a/drivers/media/platform/qcom/camss/camss-ispif.c
>>> +++ b/drivers/media/platform/qcom/camss/camss-ispif.c
>>> @@ -830,6 +830,7 @@ static int ispif_set_stream(struct v4l2_subdev
>>> *sd, int enable)
>>> ispif_select_cid(ispif, intf, cid, vfe, 1);
>>> ispif_config_irq(ispif, intf, vfe, 1);
>>> if (camss->res->version == CAMSS_8x96 ||
>>> + camss->res->version == CAMSS_8x53 ||
>>> camss->res->version == CAMSS_660)
>>> ispif_config_pack(ispif,
>>> line->fmt[MSM_ISPIF_PAD_SINK].code,
>>> @@ -848,6 +849,7 @@ static int ispif_set_stream(struct v4l2_subdev
>>> *sd, int enable)
>>> mutex_lock(&ispif->config_lock);
>>> if (camss->res->version == CAMSS_8x96 ||
>>> + camss->res->version == CAMSS_8x53 ||
>>> camss->res->version == CAMSS_660)
>>> ispif_config_pack(ispif,
>>> line->fmt[MSM_ISPIF_PAD_SINK].code,
>>> @@ -1111,6 +1113,7 @@ int msm_ispif_subdev_init(struct camss *camss,
>>> if (camss->res->version == CAMSS_8x16)
>>> ispif->line_num = 2;
>>> else if (camss->res->version == CAMSS_8x96 ||
>>> + camss->res->version == CAMSS_8x53 ||
>>> camss->res->version == CAMSS_660)
>>> ispif->line_num = 4;
>>> else
>>> @@ -1130,6 +1133,7 @@ int msm_ispif_subdev_init(struct camss *camss,
>>> ispif->line[i].nformats =
>>> ARRAY_SIZE(ispif_formats_8x16);
>>> } else if (camss->res->version == CAMSS_8x96 ||
>>> + camss->res->version == CAMSS_8x53 ||
>>> camss->res->version == CAMSS_660) {
>>> ispif->line[i].formats = ispif_formats_8x96;
>>> ispif->line[i].nformats =
>>> @@ -1162,6 +1166,7 @@ int msm_ispif_subdev_init(struct camss *camss,
>>> ret = devm_request_irq(dev, ispif->irq, ispif_isr_8x16,
>>> IRQF_TRIGGER_RISING, ispif->irq_name, ispif);
>>> else if (camss->res->version == CAMSS_8x96 ||
>>> + camss->res->version == CAMSS_8x53 ||
>>> camss->res->version == CAMSS_660)
>>> ret = devm_request_irq(dev, ispif->irq, ispif_isr_8x96,
>>> IRQF_TRIGGER_RISING, ispif->irq_name, ispif);
>>> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c
>>> b/drivers/media/platform/qcom/camss/camss-vfe.c
>>> index
>>> 83c5a36d071fcc32c4b8a89e4e429dc1820df139..80a62ba11295042802cbaec617fb87c492ea6a55
>>> 100644
>>> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
>>> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
>>> @@ -285,6 +285,7 @@ static u32 vfe_src_pad_code(struct vfe_line *line,
>>> u32 sink_code,
>>> switch (vfe->camss->res->version) {
>>> case CAMSS_8x16:
>>> + case CAMSS_8x53:
>>> switch (sink_code) {
>>> case MEDIA_BUS_FMT_YUYV8_1X16:
>>> {
>>> diff --git a/drivers/media/platform/qcom/camss/camss.c
>>> b/drivers/media/platform/qcom/camss/camss.c
>>> index
>>> fabe034081ed0a7c0e0fcd8bc76c4eb396cb0067..9fb31f4c18adee886cd0bcf84438a8f27635e07f
>>> 100644
>>> --- a/drivers/media/platform/qcom/camss/camss.c
>>> +++ b/drivers/media/platform/qcom/camss/camss.c
>>> @@ -152,6 +152,160 @@ static const struct camss_subdev_resources
>>> vfe_res_8x16[] = {
>>> }
>>> };
>>> +static const struct camss_subdev_resources csid_res_8x53[] = {
>>> + /* CSID0 */
>>> + {
>>> + .regulators = { "vdda" },
>>
>> I see that you do reuse csiphy_res_8x16 for this platform support, in
>> this case let me ask you to double check/test that the "vdda" regulator
>> is actually a CSIPHY regulator, and if so, please move the registration
>> of the regulators to csiphy_res_8x16 as a preceding change.
> It is placed in CSID at downstream and this is the documentation of
> the downstream property:
> - qcom,mipi-csi-vdd-supply : should contain regulator to be used for
> this csid core
> so it should be a csid regulator as i understand.
> It is also placed at CSIDs in msm8953-camera.dtsi
I've opened DB410C board schematics, and CAMSS IP is supplied over
VREG_L2_1P2 power lane entering VDDA_MIPI_CSI pad on the APQ8016 SoC.
The same voltage regulator also supplies display and SDRAM memory, most
probably the latter implies that CAMSS IP and driver cannot be tested
at least on this reference board, when the regulator is disabled.
So, we have to rely on the documentation here. Bryan, can you please
check, if VDDA_MIPI_CSI pad on MSM8916 and/or MSM8953 is related to
CSIPHY or CSID power supply? Thank you in advance.
--
Best wishes,
Vladimir
Powered by blists - more mailing lists