| Message ID | 20200726111215.22361-5-konradybcio@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SDM630/36/60 driver enablement | expand |
On 26-07-20, 13:12, Konrad Dybcio wrote: > These SoCs make use of the 14nm phy, but at different > addresses than other 14nm units. > > Signed-off-by: Konrad Dybcio <konradybcio@gmail.com> > --- > .../devicetree/bindings/display/msm/dsi.txt | 1 + > drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 2 ++ > drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 1 + > drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 18 ++++++++++++++++++ Is there a reason why dsi phy needs to be here and not in phy subsystem drivers/phy/ ? > 4 files changed, 22 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt > index af95586c898f..7884fd7a85c1 100644 > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt > @@ -87,6 +87,7 @@ Required properties: > * "qcom,dsi-phy-20nm" > * "qcom,dsi-phy-28nm-8960" > * "qcom,dsi-phy-14nm" > + * "qcom,dsi-phy-14nm-660" > * "qcom,dsi-phy-10nm" > * "qcom,dsi-phy-10nm-8998" > - reg: Physical base address and length of the registers of PLL, PHY. Some > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c > index f509ebd77500..009f5b843dd1 100644 > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c > @@ -499,6 +499,8 @@ static const struct of_device_id dsi_phy_dt_match[] = { > #ifdef CONFIG_DRM_MSM_DSI_14NM_PHY > { .compatible = "qcom,dsi-phy-14nm", > .data = &dsi_phy_14nm_cfgs }, > + { .compatible = "qcom,dsi-phy-14nm-660", > + .data = &dsi_phy_14nm_660_cfgs }, > #endif > #ifdef CONFIG_DRM_MSM_DSI_10NM_PHY > { .compatible = "qcom,dsi-phy-10nm", > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h > index 24b294ed3059..ef8672d7b123 100644 > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h > @@ -45,6 +45,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_28nm_lp_cfgs; > extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs; > extern const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs; > extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs; > +extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs; > extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs; > extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs; > > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c > index 1594f1422372..519400501bcd 100644 > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c > @@ -161,3 +161,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs = { > .io_start = { 0x994400, 0x996400 }, > .num_dsi_phy = 2, > }; > + > +const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = { > + .type = MSM_DSI_PHY_14NM, > + .src_pll_truthtable = { {false, false}, {true, false} }, > + .reg_cfg = { > + .num = 1, > + .regs = { > + {"vcca", 17000, 32}, > + }, > + }, > + .ops = { > + .enable = dsi_14nm_phy_enable, > + .disable = dsi_14nm_phy_disable, > + .init = dsi_14nm_phy_init, > + }, > + .io_start = { 0xc994400, 0xc996000 }, > + .num_dsi_phy = 2, > +}; > -- > 2.27.0
On Mon, Aug 3, 2020 at 4:00 AM Vinod Koul <vkoul@kernel.org> wrote: > > On 26-07-20, 13:12, Konrad Dybcio wrote: > > These SoCs make use of the 14nm phy, but at different > > addresses than other 14nm units. > > > > Signed-off-by: Konrad Dybcio <konradybcio@gmail.com> > > --- > > .../devicetree/bindings/display/msm/dsi.txt | 1 + > > drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 2 ++ > > drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 1 + > > drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 18 ++++++++++++++++++ > > Is there a reason why dsi phy needs to be here and not in phy subsystem > drivers/phy/ ? *maybe* it would be possible to split out all of the dsi (and hdmi) phy to drivers/phy. But splitting out just the new ones wouldn't be practical (it would duplicate a lot of code, and make the rest of the dsi code have to deal with both cases). And unlike dp/usb-c I'm not really sure I see an advantage to justify the churn. BR, -R > > > 4 files changed, 22 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt > > index af95586c898f..7884fd7a85c1 100644 > > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt > > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt > > @@ -87,6 +87,7 @@ Required properties: > > * "qcom,dsi-phy-20nm" > > * "qcom,dsi-phy-28nm-8960" > > * "qcom,dsi-phy-14nm" > > + * "qcom,dsi-phy-14nm-660" > > * "qcom,dsi-phy-10nm" > > * "qcom,dsi-phy-10nm-8998" > > - reg: Physical base address and length of the registers of PLL, PHY. Some > > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c > > index f509ebd77500..009f5b843dd1 100644 > > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c > > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c > > @@ -499,6 +499,8 @@ static const struct of_device_id dsi_phy_dt_match[] = { > > #ifdef CONFIG_DRM_MSM_DSI_14NM_PHY > > { .compatible = "qcom,dsi-phy-14nm", > > .data = &dsi_phy_14nm_cfgs }, > > + { .compatible = "qcom,dsi-phy-14nm-660", > > + .data = &dsi_phy_14nm_660_cfgs }, > > #endif > > #ifdef CONFIG_DRM_MSM_DSI_10NM_PHY > > { .compatible = "qcom,dsi-phy-10nm", > > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h > > index 24b294ed3059..ef8672d7b123 100644 > > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h > > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h > > @@ -45,6 +45,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_28nm_lp_cfgs; > > extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs; > > extern const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs; > > extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs; > > +extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs; > > extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs; > > extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs; > > > > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c > > index 1594f1422372..519400501bcd 100644 > > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c > > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c > > @@ -161,3 +161,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs = { > > .io_start = { 0x994400, 0x996400 }, > > .num_dsi_phy = 2, > > }; > > + > > +const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = { > > + .type = MSM_DSI_PHY_14NM, > > + .src_pll_truthtable = { {false, false}, {true, false} }, > > + .reg_cfg = { > > + .num = 1, > > + .regs = { > > + {"vcca", 17000, 32}, > > + }, > > + }, > > + .ops = { > > + .enable = dsi_14nm_phy_enable, > > + .disable = dsi_14nm_phy_disable, > > + .init = dsi_14nm_phy_init, > > + }, > > + .io_start = { 0xc994400, 0xc996000 }, > > + .num_dsi_phy = 2, > > +}; > > -- > > 2.27.0 > > -- > ~Vinod
On 03-08-20, 09:06, Rob Clark wrote: > On Mon, Aug 3, 2020 at 4:00 AM Vinod Koul <vkoul@kernel.org> wrote: > > > > On 26-07-20, 13:12, Konrad Dybcio wrote: > > > These SoCs make use of the 14nm phy, but at different > > > addresses than other 14nm units. > > > > > > Signed-off-by: Konrad Dybcio <konradybcio@gmail.com> > > > --- > > > .../devicetree/bindings/display/msm/dsi.txt | 1 + > > > drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 2 ++ > > > drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 1 + > > > drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 18 ++++++++++++++++++ > > > > Is there a reason why dsi phy needs to be here and not in phy subsystem > > drivers/phy/ ? > > *maybe* it would be possible to split out all of the dsi (and hdmi) > phy to drivers/phy. But splitting out just the new ones wouldn't be > practical (it would duplicate a lot of code, and make the rest of the > dsi code have to deal with both cases). And unlike dp/usb-c I'm not > really sure I see an advantage to justify the churn. So the question would be if it helps in reuse if we do that and does it result in a better solution than dsi code managing the phy. The advantage of framework (like phy) is that different subsystems can use a (phy) driver and common framework helps reduce duplicates. Yes sure the question was not for a new phy but about the whole msm/dsi/phy code and future for it.
On Tue, Aug 4, 2020 at 5:09 AM Vinod Koul <vkoul@kernel.org> wrote: > > On 03-08-20, 09:06, Rob Clark wrote: > > On Mon, Aug 3, 2020 at 4:00 AM Vinod Koul <vkoul@kernel.org> wrote: > > > > > > On 26-07-20, 13:12, Konrad Dybcio wrote: > > > > These SoCs make use of the 14nm phy, but at different > > > > addresses than other 14nm units. > > > > > > > > Signed-off-by: Konrad Dybcio <konradybcio@gmail.com> > > > > --- > > > > .../devicetree/bindings/display/msm/dsi.txt | 1 + > > > > drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 2 ++ > > > > drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 1 + > > > > drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 18 ++++++++++++++++++ > > > > > > Is there a reason why dsi phy needs to be here and not in phy subsystem > > > drivers/phy/ ? > > > > *maybe* it would be possible to split out all of the dsi (and hdmi) > > phy to drivers/phy. But splitting out just the new ones wouldn't be > > practical (it would duplicate a lot of code, and make the rest of the > > dsi code have to deal with both cases). And unlike dp/usb-c I'm not > > really sure I see an advantage to justify the churn. > > So the question would be if it helps in reuse if we do that and does it > result in a better solution than dsi code managing the phy. The > advantage of framework (like phy) is that different subsystems can use > a (phy) driver and common framework helps reduce duplicates. I'm not aware of any re-use that would be possible by splitting it out.. if there were, it would be a more compelling argument. It does increase the complexity and possibilities for getting kernel config wrong. There are devices like the aarch64 laptops which do not have a debug serial port, where debugging issues like that can be a pain when you get no display. OTOH that might be balanced out a bit by using a common framework/api that others are familiar with. Overall, nowhere near high enough on my priority list to spend time on.. there are bigger fires. If someone was really motivated about this and wanted to send (tested) patches, then I'd take a look and see how it turned out. BR, -R > Yes sure the question was not for a new phy but about the whole > msm/dsi/phy code and future for it. > > -- > ~Vinod
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index af95586c898f..7884fd7a85c1 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -87,6 +87,7 @@ Required properties: * "qcom,dsi-phy-20nm" * "qcom,dsi-phy-28nm-8960" * "qcom,dsi-phy-14nm" + * "qcom,dsi-phy-14nm-660" * "qcom,dsi-phy-10nm" * "qcom,dsi-phy-10nm-8998" - reg: Physical base address and length of the registers of PLL, PHY. Some diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c index f509ebd77500..009f5b843dd1 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c @@ -499,6 +499,8 @@ static const struct of_device_id dsi_phy_dt_match[] = { #ifdef CONFIG_DRM_MSM_DSI_14NM_PHY { .compatible = "qcom,dsi-phy-14nm", .data = &dsi_phy_14nm_cfgs }, + { .compatible = "qcom,dsi-phy-14nm-660", + .data = &dsi_phy_14nm_660_cfgs }, #endif #ifdef CONFIG_DRM_MSM_DSI_10NM_PHY { .compatible = "qcom,dsi-phy-10nm", diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h index 24b294ed3059..ef8672d7b123 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h @@ -45,6 +45,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_28nm_lp_cfgs; extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs; extern const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs; extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs; +extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs; extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs; extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs; diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c index 1594f1422372..519400501bcd 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c @@ -161,3 +161,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs = { .io_start = { 0x994400, 0x996400 }, .num_dsi_phy = 2, }; + +const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = { + .type = MSM_DSI_PHY_14NM, + .src_pll_truthtable = { {false, false}, {true, false} }, + .reg_cfg = { + .num = 1, + .regs = { + {"vcca", 17000, 32}, + }, + }, + .ops = { + .enable = dsi_14nm_phy_enable, + .disable = dsi_14nm_phy_disable, + .init = dsi_14nm_phy_init, + }, + .io_start = { 0xc994400, 0xc996000 }, + .num_dsi_phy = 2, +};
These SoCs make use of the 14nm phy, but at different addresses than other 14nm units. Signed-off-by: Konrad Dybcio <konradybcio@gmail.com> --- .../devicetree/bindings/display/msm/dsi.txt | 1 + drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 2 ++ drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 1 + drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 18 ++++++++++++++++++ 4 files changed, 22 insertions(+)