mbox series

[0/6] media: qcom: camss: Add sc8280xp support

Message ID 20231102-b4-camss-sc8280xp-v1-0-9996f4bcb8f4@linaro.org
Headers show
Series media: qcom: camss: Add sc8280xp support | expand

Message

Bryan O'Donoghue Nov. 2, 2023, 11:41 a.m. UTC 
sc8280xp is the SoC found in the Lenovo X13s. This series adds support to
bring up the CSIPHY, CSID, VFE/RDI interfaces.

A number of precursor patches make this series smaller overall than
previous series.

sc8280xp provides

- 4 x VFE, 4 RDI per VFE
- 4 x VFE Lite, 4 RDI per VFE
- 4 x CSID
- 4 x CSID Lite
- 4 x CSI PHY

I've taken the yaml from a dtsi series and included it here since 1) I sent
the yaml to the wrong person and 2) it already has RB from Krzysztof.

Requires CAMCC for sc8280xp which applies to qcom/clk-for-6.7:
https://lore.kernel.org/linux-arm-msm/20231026105345.3376-1-bryan.odonoghue@linaro.org/
b4 shazam 20231026105345.3376-1-bryan.odonoghue@linaro.org

Requires the named power-domain patches which apply to media-tree/* :
https://lore.kernel.org/linux-arm-msm/20231101-b4-camss-named-power-domains-v3-5-bbdf5f22462a@linaro.org/
b4 shazam 20231101-b4-camss-named-power-domains-v3-5-bbdf5f22462a@linaro.org

To use the camera on x13s with say Google Hangouts or Microsoft Teams you
will need to

1. Run Firefox
2. Update about:config to enable pipewire
3. Use this WIP version of libcamera
   https://gitlab.freedesktop.org/camera/libcamera-softisp

A working bootable tree can be found here:
Link: https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/lenovo-x13s-linux-6.5.y

b4 base:
https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/b4/camss-sc8280xp

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
Bryan O'Donoghue (6):
      media: dt-bindings: media: camss: Add qcom,sc8280xp-camss binding
      media: qcom: camss: Add CAMSS_SC8280XP enum
      media: qcom: camss: csiphy-3ph: Add Gen2 v1.1 two-phase MIPI CSI-2 DPHY init
      media: qcom: camss: Add sc8280xp resource details
      media: qcom: camss: Add sc8280xp support
      media: qcom: camss: vfe-17x: Rename camss-vfe-170 to camss-vfe-17x

 .../bindings/media/qcom,sc8280xp-camss.yaml        | 581 +++++++++++++++++++++
 drivers/media/platform/qcom/camss/Makefile         |   2 +-
 .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 108 +++-
 drivers/media/platform/qcom/camss/camss-csiphy.c   |   1 +
 .../camss/{camss-vfe-170.c => camss-vfe-17x.c}     |   0
 drivers/media/platform/qcom/camss/camss-vfe.c      |  25 +-
 drivers/media/platform/qcom/camss/camss-video.c    |   1 +
 drivers/media/platform/qcom/camss/camss.c          | 376 +++++++++++++
 drivers/media/platform/qcom/camss/camss.h          |   1 +
 9 files changed, 1086 insertions(+), 9 deletions(-)
---
base-commit: f5602ca81598f12d3fdb327beb29688871955596
change-id: 20231101-b4-camss-sc8280xp-0e1b91eb21bf

Best regards,

Comments

Konrad Dybcio Nov. 2, 2023, 1:35 p.m. UTC | #1 
On 02/11/2023 12:41, Bryan O'Donoghue wrote:
> Add in functional logic throughout the code to support the sc8280xp.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   .../platform/qcom/camss/camss-csiphy-3ph-1-0.c     | 23 +++++++++++++++++---
>   drivers/media/platform/qcom/camss/camss-csiphy.c   |  1 +
>   drivers/media/platform/qcom/camss/camss-vfe.c      | 25 +++++++++++++++++-----
>   drivers/media/platform/qcom/camss/camss-video.c    |  1 +
>   4 files changed, 42 insertions(+), 8 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 2eb3531ffd00b..2810d0fa06c13 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
> @@ -513,6 +513,10 @@ static void csiphy_gen2_config_lanes(struct csiphy_device *csiphy,
>   		r = &lane_regs_sm8250[0][0];
>   		array_size = ARRAY_SIZE(lane_regs_sm8250[0]);
>   		break;
> +	case CAMSS_8280XP:
> +		r = &lane_regs_sc8280xp[0][0];
> +		array_size = ARRAY_SIZE(lane_regs_sc8280xp[0]);
> +		break;
>   	default:
>   		WARN(1, "unknown cspi version\n");
>   		return;
> @@ -548,13 +552,26 @@ static u8 csiphy_get_lane_mask(struct csiphy_lanes_cfg *lane_cfg)
>   	return lane_mask;
>   }
>   
> +static bool csiphy_is_gen2(u32 version)
> +{
> +	bool ret = false;
delete

> +
> +	switch (version) {
> +	case CAMSS_845:
> +	case CAMSS_8250:
> +	case CAMSS_8280XP:
> +		ret = true; > +		break;
return true

> +	}
> +
> +	return ret;
return false

> +}
> +
>   static void csiphy_lanes_enable(struct csiphy_device *csiphy,
>   				struct csiphy_config *cfg,
>   				s64 link_freq, u8 lane_mask)
>   {
>   	struct csiphy_lanes_cfg *c = &cfg->csi2->lane_cfg;
> -	bool is_gen2 = (csiphy->camss->res->version == CAMSS_845 ||
> -			csiphy->camss->res->version == CAMSS_8250);
>   	u8 settle_cnt;
>   	u8 val;
>   	int i;
> @@ -576,7 +593,7 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
>   	val = 0x00;
>   	writel_relaxed(val, csiphy->base + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(0));
>   
> -	if (is_gen2)
> +	if (csiphy_is_gen2(csiphy->camss->res->version))
>   		csiphy_gen2_config_lanes(csiphy, settle_cnt);
>   	else
>   		csiphy_gen1_config_lanes(csiphy, cfg, settle_cnt);
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index edd573606a6ae..8241acf789865 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,
>   		break;
>   	case CAMSS_845:
>   	case CAMSS_8250:
> +	case CAMSS_8280XP:
>   		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 123e5ead7602d..21812d40716f4 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -225,6 +225,7 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code,
>   	case CAMSS_660:
>   	case CAMSS_845:
>   	case CAMSS_8250:
> +	case CAMSS_8280XP:
>   		switch (sink_code) {
>   		case MEDIA_BUS_FMT_YUYV8_1X16:
>   		{
> @@ -1522,6 +1523,7 @@ int msm_vfe_subdev_init(struct camss *camss, struct vfe_device *vfe,
>   			break;
>   		case CAMSS_845:
>   		case CAMSS_8250:
> +		case CAMSS_8280XP:
>   			l->formats = formats_rdi_845;
>   			l->nformats = ARRAY_SIZE(formats_rdi_845);
>   			break;
> @@ -1600,6 +1602,23 @@ static const struct media_entity_operations vfe_media_ops = {
>   	.link_validate = v4l2_subdev_link_validate,
>   };
>   
> +static int vfe_bpl_align(struct vfe_device *vfe)
> +{
> +	int ret = 8;
return the value directly

> +
> +	switch (vfe->camss->res->version) {
> +	case CAMSS_845:
> +	case CAMSS_8250:
> +	case CAMSS_8280XP:
> +		ret = 16;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}
Konrad
Bryan O'Donoghue Nov. 2, 2023, 1:38 p.m. UTC | #2 
On 02/11/2023 13:35, Konrad Dybcio wrote:
>> +static int vfe_bpl_align(struct vfe_device *vfe)
>> +{
>> +    int ret = 8;
> return the value directly

What's the motive for this comment ?

---
bod
Konrad Dybcio Nov. 2, 2023, 1:40 p.m. UTC | #3 
On 02/11/2023 12:41, Bryan O'Donoghue wrote:
> vfe-170 and vfe-175 can be supported in the same file with some minimal
> indirection to differentiate between the silicon versions.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
But you directly assigned v170 to sc8280xp, without any renaming or
meaningful changes I think?

Konrad
Bryan O'Donoghue Nov. 2, 2023, 1:41 p.m. UTC | #4 
On 02/11/2023 13:40, Konrad Dybcio wrote:
> 
> 
> On 02/11/2023 12:41, Bryan O'Donoghue wrote:
>> vfe-170 and vfe-175 can be supported in the same file with some minimal
>> indirection to differentiate between the silicon versions.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
> But you directly assigned v170 to sc8280xp, without any renaming or
> meaningful changes I think?
> 
> Konrad

The specific version for sc8280xp is 175-200 => the motivation for the 
name change.

I'll amend the log to reflect.

---
bod
Konrad Dybcio Nov. 2, 2023, 1:45 p.m. UTC | #5 
On 02/11/2023 14:38, Bryan O'Donoghue wrote:
> On 02/11/2023 13:35, Konrad Dybcio wrote:
>>> +static int vfe_bpl_align(struct vfe_device *vfe)
>>> +{
>>> +    int ret = 8;
>> return the value directly
> 
> What's the motive for this comment ?
stack size
> 
> ---
> bod
Bryan O'Donoghue Nov. 2, 2023, 1:47 p.m. UTC | #6 
On 02/11/2023 13:45, Konrad Dybcio wrote:
> 
> 
> On 02/11/2023 14:38, Bryan O'Donoghue wrote:
>> On 02/11/2023 13:35, Konrad Dybcio wrote:
>>>> +static int vfe_bpl_align(struct vfe_device *vfe)
>>>> +{
>>>> +    int ret = 8;
>>> return the value directly
>>
>> What's the motive for this comment ?
> stack size

An optimizing compiler - older than me - will not differentiate that at all.

:)

---
bod