Add current load setting for qcom camss csiphy

[RFC PATCH 0/3] Add current load setting for qcom camss csiphy

Posted by Wenmeng Liu 3 months, 2 weeks ago

Currently qcom camss csiphy drivers don’t set regulator’s currents
load properly using Linux regulator framework. This causes every 
regulator’s initial mode set as HPM (high current mode), 
which may have higher power consumption.
To address this issue, add current configuration for CSIPHY.

Wenmeng Liu (3):
  media: dt-bindings: Add regulator current load
  media: qcom: camss: csiphy: Add regulator current load setting
  arm64: dts: qcom: qcs6490-rb3gen2: Add csiphy current support

 .../bindings/media/qcom,sc7280-camss.yaml     |  6 ++++
 .../qcs6490-rb3gen2-vision-mezzanine.dtso     |  1 +
 .../media/platform/qcom/camss/camss-csiphy.c  | 29 +++++++++++++++++++
 .../media/platform/qcom/camss/camss-csiphy.h  |  1 +
 4 files changed, 37 insertions(+)

-- 
2.34.1

Re: [RFC PATCH 0/3] Add current load setting for qcom camss csiphy

Posted by Bryan O'Donoghue 3 months, 2 weeks ago

On 20/06/2025 05:07, Wenmeng Liu wrote:
> Currently qcom camss csiphy drivers don’t set regulator’s currents
> load properly using Linux regulator framework. This causes every
> regulator’s initial mode set as HPM (high current mode),
> which may have higher power consumption.
> To address this issue, add current configuration for CSIPHY.
> 
> Wenmeng Liu (3):
>    media: dt-bindings: Add regulator current load
>    media: qcom: camss: csiphy: Add regulator current load setting
>    arm64: dts: qcom: qcs6490-rb3gen2: Add csiphy current support
> 
>   .../bindings/media/qcom,sc7280-camss.yaml     |  6 ++++
>   .../qcs6490-rb3gen2-vision-mezzanine.dtso     |  1 +
>   .../media/platform/qcom/camss/camss-csiphy.c  | 29 +++++++++++++++++++
>   .../media/platform/qcom/camss/camss-csiphy.h  |  1 +
>   4 files changed, 37 insertions(+)
> 

How are these load-currents determined ?

Looking at other instances of setting current for PHYs

  grep -r regulator_set_load * | grep com 
  
            [git:camss-bugfix-6.17] ✖
drivers/phy/qualcomm/phy-qcom-edp.c:	ret = 
regulator_set_load(edp->supplies[0].consumer, 21800); /* 1.2 V vdda-phy */
drivers/phy/qualcomm/phy-qcom-edp.c:	ret = 
regulator_set_load(edp->supplies[1].consumer, 36000); /* 0.9 V vdda-pll */
drivers/phy/qualcomm/phy-qcom-usb-hs.c:	ret = 
regulator_set_load(uphy->v1p8, 50000);
drivers/phy/qualcomm/phy-qcom-usb-hs.c:	ret = 
regulator_set_load(uphy->v3p3, 50000);
drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c:	ret = 
regulator_set_load(priv->vregs[VDDA_1P8].consumer, 19000);
drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c:	ret = 
regulator_set_load(priv->vregs[VDDA_3P3].consumer, 16000);
drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c: 
regulator_set_load(priv->vregs[VDDA_1P8].consumer, 0);
drivers/phy/qualcomm/phy-qcom-qmp-combo.c:		ret = 
regulator_set_load(qmp->vregs[i].consumer,
drivers/remoteproc/qcom_q6v5_pas.c:		regulator_set_load(adsp->cx_supply, 
100000);
drivers/remoteproc/qcom_wcnss.c:			regulator_set_load(bulk[i].consumer, 
info[i].load_uA);
drivers/remoteproc/qcom_wcnss_iris.c:		 
regulator_set_load(iris->vregs[i].consumer,
drivers/remoteproc/qcom_q6v5_mss.c:			ret = regulator_set_load(regs[i].reg,
drivers/remoteproc/qcom_q6v5_mss.c:			regulator_set_load(regs[i].reg, 0);
drivers/remoteproc/qcom_q6v5_mss.c:			regulator_set_load(regs[i].reg, 0);
drivers/remoteproc/qcom_q6v5_wcss.c:	regulator_set_load(wcss->cx_supply, 
100000);

I think this is the type of thing we should bury in SoC resources not in DT.

I can think of how we might want to change the current depending on the 
pixel rate.. but then I think that is something we could calculate based 
on pixel rate with descriptions as a base in

driver/media/platfrom/qcom/camss/camss.c::static const struct 
camss_subdev_resources csiphy_res_SoCNumber[];

---
bod

Re: [RFC PATCH 0/3] Add current load setting for qcom camss csiphy

Posted by Wenmeng Liu 3 months, 1 week ago


On 2025/6/20 16:23, Bryan O'Donoghue wrote:
> On 20/06/2025 05:07, Wenmeng Liu wrote:
>> Currently qcom camss csiphy drivers don’t set regulator’s currents
>> load properly using Linux regulator framework. This causes every
>> regulator’s initial mode set as HPM (high current mode),
>> which may have higher power consumption.
>> To address this issue, add current configuration for CSIPHY.
>>
>> Wenmeng Liu (3):
>>    media: dt-bindings: Add regulator current load
>>    media: qcom: camss: csiphy: Add regulator current load setting
>>    arm64: dts: qcom: qcs6490-rb3gen2: Add csiphy current support
>>
>>   .../bindings/media/qcom,sc7280-camss.yaml     |  6 ++++
>>   .../qcs6490-rb3gen2-vision-mezzanine.dtso     |  1 +
>>   .../media/platform/qcom/camss/camss-csiphy.c  | 29 +++++++++++++++++++
>>   .../media/platform/qcom/camss/camss-csiphy.h  |  1 +
>>   4 files changed, 37 insertions(+)
>>
> 
> How are these load-currents determined ?

According to my discussion with hw colleague,current value is decided 
based on post silicon calibration, this value is fixed for the 
corresponding chip.

> Looking at other instances of setting current for PHYs
> 
>   grep -r regulator_set_load * | grep com
>             [git:camss-bugfix-6.17] ✖
> drivers/phy/qualcomm/phy-qcom-edp.c:    ret = regulator_set_load(edp- 
>  >supplies[0].consumer, 21800); /* 1.2 V vdda-phy */
> drivers/phy/qualcomm/phy-qcom-edp.c:    ret = regulator_set_load(edp- 
>  >supplies[1].consumer, 36000); /* 0.9 V vdda-pll */
> drivers/phy/qualcomm/phy-qcom-usb-hs.c:    ret = 
> regulator_set_load(uphy->v1p8, 50000);
> drivers/phy/qualcomm/phy-qcom-usb-hs.c:    ret = 
> regulator_set_load(uphy->v3p3, 50000);
> drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c:    ret = 
> regulator_set_load(priv->vregs[VDDA_1P8].consumer, 19000);
> drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c:    ret = 
> regulator_set_load(priv->vregs[VDDA_3P3].consumer, 16000);
> drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c: regulator_set_load(priv- 
>  >vregs[VDDA_1P8].consumer, 0);
> drivers/phy/qualcomm/phy-qcom-qmp-combo.c:        ret = 
> regulator_set_load(qmp->vregs[i].consumer,
> drivers/remoteproc/qcom_q6v5_pas.c:        regulator_set_load(adsp- 
>  >cx_supply, 100000);
> drivers/remoteproc/qcom_wcnss.c:            
> regulator_set_load(bulk[i].consumer, info[i].load_uA);
> drivers/remoteproc/qcom_wcnss_iris.c: regulator_set_load(iris- 
>  >vregs[i].consumer,
> drivers/remoteproc/qcom_q6v5_mss.c:            ret = 
> regulator_set_load(regs[i].reg,
> drivers/remoteproc/qcom_q6v5_mss.c:            
> regulator_set_load(regs[i].reg, 0);
> drivers/remoteproc/qcom_q6v5_mss.c:            
> regulator_set_load(regs[i].reg, 0);
> drivers/remoteproc/qcom_q6v5_wcss.c:    regulator_set_load(wcss- 
>  >cx_supply, 100000);
> 
> I think this is the type of thing we should bury in SoC resources not in 
> DT.
> 
> I can think of how we might want to change the current depending on the 
> pixel rate.. but then I think that is something we could calculate based 
> on pixel rate with descriptions as a base in
> 
> driver/media/platfrom/qcom/camss/camss.c::static const struct 
> camss_subdev_resources csiphy_res_SoCNumber[];
> 
> ---
> bod
> 

Yes, it's more common to put the current value in the code.Will be 
updated in v2.

Thanks,
Wenmeng