mbox series

[v8,00/16] media: qcom: camss: Add sm8550 support

Message ID 20250108143733.2761200-1-quic_depengs@quicinc.com
Headers show
Series media: qcom: camss: Add sm8550 support | expand

Message

Depeng Shao Jan. 8, 2025, 2:37 p.m. UTC
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/

v7:
- Due to the patches in https://lore.kernel.org/all/20241217140656.965235-1-quic_vikramsa@quicinc.com/
  are merged, so rebased below patches to fix the conflict.
  media: qcom: camss: csiphy-3ph: Remove redundant PHY init sequence control loop
  media: qcom: camss: csiphy-3ph: Move CSIPHY variables to data field inside csiphy struct
  media: qcom: camss: Add sm8550 compatible
  media: qcom: camss: csiphy-3ph: Move CSIPHY variables to data field inside csiphy struct
  media: qcom: camss: csiphy-3ph: Add Gen2 v2.1.2 two-phase MIPI CSI-2 DPHY support
  media: qcom: camss: Add support for VFE 780
- Add RB for "media: qcom: camss: Add CSID 780 support" - Bryan
- Use TAG name for ICC and remove offline HW ICC - Bryan
- Remove the logic that moving enable_irq();/disable_irq(); to wm_start() 
  and wm_stop() to make sure no logical change in VFE refactor change.
- Update the commit message and title for the TPG change. - Bryan
- Link to v6: https://lore.kernel.org/all/20241211140738.3835588-1-quic_depengs@quicinc.com/

v6:
- Add bus type property in dt-binding which will be limited
  by a latest change 
  https://lore.kernel.org/all/20241209-camss-dphy-v1-0-5f1b6f25ed92@fairphone.com/
- Add RB for "media: qcom: camss: Add sm8550 compatible" and
  "media: qcom: camss: Add support for VFE 780"
- Uppercase the hex in csiphy register list - Bryan
- Add empty function for csid tpg - Vladimir
- Set testgen mode to CSID_PAYLOAD_MODE_DISABLED in subdev init interface
- encapsulate the guard __thus__ for new header - Bryan
- Add a standalone patch for the platform which doesn't support CSID TPG
  to make sure new platform driver can set CSID_PAYLOAD_MODE_DISABLED
  to disable TPG
- Update the csid for csid and vfe driver - Bryan
- Link to v5: https://lore.kernel.org/all/20241205155538.250743-1-quic_depengs@quicinc.com/

v5:
- Update dt-bindings required items order - Krzysztof
- Sort the reg order based on the comments in sc7280 dt-binding - Vladimir
- Change the irq type to IRQ_TYPE_EDGE_RISING - Vladimir
- Remove the Krzysztof's RB tag from dt-binding patch due to above
  updates in dt-binding patch
- Move regulator from csid resource to csiphy resource - Bryan, Vladimir
- Move the change to add default case in vfe_src_pad_code to a
  standalone patch. - Bryan
- Rename csid-gen3 as csid-780 - Bryan
- use macros() to bury bit shifts - Bryan
- Sort the macros by register offset order  -  Vladimir
- Redefine a macro for rup_aup in csid driver - Vladimir
- Remove the unused macros in vfe 780 driver - Vladimir
- Add dummy function for unsupported hw_ops in vfe 780
  driver - Vladimir, Bryan
- Use a standalone patch for the callback API of RUP and buf done update
- Use a standalone patch to make CSID TPG optional - Vladimir
- Link to v4: https://lore.kernel.org/all/20240812144131.369378-1-quic_depengs@quicinc.com/

v4:
- Update dt-bindings based on comments - Krzysztof, bod, Vladimir
- Move common code into csid core and vfe core driver - bod
- Remove *_relaxed in the csid and vfe drivers - Krzysztof
- Reorganize patches in logical junks, make sure that new added
structures have users in current patch - Krzysztof
- Remove notify function  and add new functions in camss for buf done
and reg update - bod
- Remove custom code to get csid base - bod
- Remove ISR function in vfe780 driver since it is never fired - bod
- Move csid_top_base to camss structure since we only have one csid
top block, and just need to get base once for csid top
- Add Vladimir's RB
- Remove prerequisite-patch-id in the cover letter since the changes
have been merged
- Add dtsi patch link for reference - Krzysztof
https://lore.kernel.org/all/20240807123333.2056518-1-quic_depengs@quicinc.com/
- Link to v3: https://lore.kernel.org/all/20240709160656.31146-1-quic_depengs@quicinc.com/

v3:
- Rebased the change based on below change which will be merged firstly.
"Move camss version related defs in to resources"
Link: https://lore.kernel.org/all/20240522154659.510-1-quic_grosikop@quicinc.com/
- Rebased the change based on Bryan's csiphy optimization change and add
these changes into this series, so that the new csiphy-3ph driver don't
need to add duplicate code. This has got Bryan's permission to add his
patches into this series.
- Refactor some changes based on the comments to move the random code to
patches where they are used.
- Remove the vfe780 irq function since it isn't doing the actual work.
- Add dt-binding for sm8550 camss driver.
Link to V2: https://lore.kernel.org/all/20240320141136.26827-1-quic_depengs@quicinc.com/

v2:
- Update some commit messages
Link to V1: https://lore.kernel.org/all/20240320134227.16587-1-quic_depengs@quicinc.com/

v1:
SM8550 is a Qualcomm flagship SoC. This series adds support to
bring up the CSIPHY, CSID, VFE/RDI interfaces in SM8550.

SM8550 provides

- 3 x VFE, 3 RDI per VFE
- 2 x VFE Lite, 4 RDI per VFE
- 3 x CSID
- 2 x CSID Lite
- 8 x CSI PHY

Bryan O'Donoghue (6):
  media: qcom: camss: csiphy-3ph: Fix trivial indentation fault in
    defines
  media: qcom: camss: csiphy-3ph: Remove redundant PHY init sequence
    control loop
  media: qcom: camss: csiphy-3ph: Rename struct
  media: qcom: camss: csiphy: Add an init callback to CSI PHY devices
  media: qcom: camss: csiphy-3ph: Move CSIPHY variables to data field
    inside csiphy struct
  media: qcom: camss: csiphy-3ph: Use an offset variable to find common
    control regs

Depeng Shao (10):
  media: qcom: camss: csid: Move common code into csid core
  media: qcom: camss: vfe: Move common code into vfe core
  media: qcom: camss: Add callback API for RUP update and buf done
  media: qcom: camss: Add default case in vfe_src_pad_code
  media: qcom: camss: csid: Only add TPG v4l2 ctrl if TPG hardware is
    available
  dt-bindings: media: camss: Add qcom,sm8550-camss binding
  media: qcom: camss: Add sm8550 compatible
  media: qcom: camss: csiphy-3ph: Add Gen2 v2.1.2 two-phase MIPI CSI-2
    DPHY support
  media: qcom: camss: Add CSID 780 support
  media: qcom: camss: Add support for VFE 780

 .../bindings/media/qcom,sm8550-camss.yaml     | 597 +++++++++++++
 drivers/media/platform/qcom/camss/Makefile    |   2 +
 .../platform/qcom/camss/camss-csid-4-1.c      |  19 -
 .../platform/qcom/camss/camss-csid-4-7.c      |  42 -
 .../platform/qcom/camss/camss-csid-780.c      | 337 ++++++++
 .../platform/qcom/camss/camss-csid-780.h      |  25 +
 .../platform/qcom/camss/camss-csid-gen2.c     |  60 --
 .../media/platform/qcom/camss/camss-csid.c    | 137 ++-
 .../media/platform/qcom/camss/camss-csid.h    |  31 +
 .../qcom/camss/camss-csiphy-2ph-1-0.c         |   6 +
 .../qcom/camss/camss-csiphy-3ph-1-0.c         | 794 ++++++++++--------
 .../media/platform/qcom/camss/camss-csiphy.c  |   4 +
 .../media/platform/qcom/camss/camss-csiphy.h  |   8 +
 .../media/platform/qcom/camss/camss-vfe-17x.c | 112 +--
 .../media/platform/qcom/camss/camss-vfe-4-1.c |   9 -
 .../media/platform/qcom/camss/camss-vfe-4-7.c |  11 -
 .../media/platform/qcom/camss/camss-vfe-4-8.c |  11 -
 .../media/platform/qcom/camss/camss-vfe-480.c | 274 +-----
 .../media/platform/qcom/camss/camss-vfe-780.c | 159 ++++
 drivers/media/platform/qcom/camss/camss-vfe.c | 274 ++++++
 drivers/media/platform/qcom/camss/camss-vfe.h |  59 +-
 drivers/media/platform/qcom/camss/camss.c     | 359 ++++++++
 drivers/media/platform/qcom/camss/camss.h     |   4 +
 23 files changed, 2464 insertions(+), 870 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/qcom,sm8550-camss.yaml
 create mode 100644 drivers/media/platform/qcom/camss/camss-csid-780.c
 create mode 100644 drivers/media/platform/qcom/camss/camss-csid-780.h
 create mode 100644 drivers/media/platform/qcom/camss/camss-vfe-780.c


base-commit: 8155b4ef3466f0e289e8fcc9e6e62f3f4dceeac2

Comments

Bryan O'Donoghue Jan. 10, 2025, 1:50 p.m. UTC | #1
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
Depeng Shao Jan. 10, 2025, 3:11 p.m. UTC | #2
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
Krzysztof Kozlowski Jan. 11, 2025, 8:54 a.m. UTC | #3
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
Bryan O'Donoghue Jan. 15, 2025, 3:51 p.m. UTC | #4
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
Laurentiu Tudor Jan. 15, 2025, 6:01 p.m. UTC | #5
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;
Bryan O'Donoghue Jan. 15, 2025, 9:01 p.m. UTC | #6
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
Vladimir Zapolskiy Jan. 15, 2025, 9:15 p.m. UTC | #7
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
Bryan O'Donoghue Jan. 15, 2025, 9:39 p.m. UTC | #8
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