[PATCH] drm/msm/dsi: support CPHY mode for 7nm pll/phy
Jonathan Marek
jonathan at marek.ca
Fri Mar 5 23:06:43 UTC 2021
On 3/5/21 5:45 PM, Dmitry Baryshkov wrote:
> On 15/02/2021 19:27, Jonathan Marek wrote:
>> Add the required changes to support 7nm pll/phy in CPHY mode.
>>
>> This adds a "qcom,dsi-phy-cphy-mode" property for the PHY node to enable
>> the CPHY mode.
>>
>> Signed-off-by: Jonathan Marek <jonathan at marek.ca>
>
> Other that few comments bellow:
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>
>> ---
>> .../devicetree/bindings/display/msm/dsi.txt | 1 +
>> drivers/gpu/drm/msm/dsi/dsi.c | 12 +--
>> drivers/gpu/drm/msm/dsi/dsi.h | 6 +-
>> drivers/gpu/drm/msm/dsi/dsi.xml.h | 2 +
>> drivers/gpu/drm/msm/dsi/dsi_host.c | 34 +++++--
>> drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 49 +++++++++-
>> drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 3 +
>> drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 89 ++++++++++++++-----
>> drivers/gpu/drm/msm/dsi/pll/dsi_pll.c | 4 +-
>> drivers/gpu/drm/msm/dsi/pll/dsi_pll.h | 5 +-
>> drivers/gpu/drm/msm/dsi/pll/dsi_pll_7nm.c | 71 +++++++++------
>> 11 files changed, 210 insertions(+), 66 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt
>> b/Documentation/devicetree/bindings/display/msm/dsi.txt
>> index b9a64d3ff184..7ffc86a9816b 100644
>> --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
>> +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
>> @@ -124,6 +124,7 @@ Required properties:
>> Optional properties:
>> - qcom,dsi-phy-regulator-ldo-mode: Boolean value indicating if the
>> LDO mode PHY
>> regulator is wanted.
>> +- qcom,dsi-phy-cphy-mode: Boolean value indicating if CPHY mode is
>> wanted.
>> - qcom,mdss-mdp-transfer-time-us: Specifies the dsi transfer time
>> for command mode
>> panels in microseconds. Driver uses this number
>> to adjust
>> the clock rate according to the expected
>> transfer time.
>
> This should go in a separate patch, shan't it?
>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c
>> b/drivers/gpu/drm/msm/dsi/dsi.c
>> index 627048851d99..68d8547f7264 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
>> @@ -13,7 +13,7 @@ struct drm_encoder *msm_dsi_get_encoder(struct
>> msm_dsi *msm_dsi)
>> return msm_dsi->encoder;
>> }
>> -static int dsi_get_phy(struct msm_dsi *msm_dsi)
>> +static int dsi_get_phy(struct msm_dsi *msm_dsi, bool *cphy_mode)
>
> I see no need to pass the 'cphy_mode' through the bool pointer and back
> to msm_dsi_host_init. What about just putting it into struct msm_dsi?
>
Because it doesn't need to be stored in msm_dsi (need it in msm_dsi_host
which doesn't have access to msm_dsi). But I suppose it doesn't hurt to
also have it in msm_dsi and make things a bit cleaner.
>> {
>> struct platform_device *pdev = msm_dsi->pdev;
>> struct platform_device *phy_pdev;
>> @@ -29,6 +29,7 @@ static int dsi_get_phy(struct msm_dsi *msm_dsi)
>> if (phy_pdev)
>> msm_dsi->phy = platform_get_drvdata(phy_pdev);
>> + *cphy_mode = of_property_read_bool(phy_node,
>> "qcom,dsi-phy-cphy-mode");
>> of_node_put(phy_node);
>> if (!phy_pdev || !msm_dsi->phy) {
>> @@ -65,6 +66,7 @@ static void dsi_destroy(struct msm_dsi *msm_dsi)
>> static struct msm_dsi *dsi_init(struct platform_device *pdev)
>> {
>> struct msm_dsi *msm_dsi;
>> + bool cphy_mode;
>> int ret;
>> if (!pdev)
>> @@ -79,13 +81,13 @@ static struct msm_dsi *dsi_init(struct
>> platform_device *pdev)
>> msm_dsi->pdev = pdev;
>> platform_set_drvdata(pdev, msm_dsi);
>> - /* Init dsi host */
>> - ret = msm_dsi_host_init(msm_dsi);
>> + /* GET dsi PHY */
>> + ret = dsi_get_phy(msm_dsi, &cphy_mode);
>> if (ret)
>> goto destroy_dsi;
>> - /* GET dsi PHY */
>> - ret = dsi_get_phy(msm_dsi);
>> + /* Init dsi host */
>> + ret = msm_dsi_host_init(msm_dsi, cphy_mode);
>> if (ret)
>> goto destroy_dsi;
>
>
More information about the dri-devel
mailing list