Re: [PATCH v3 06/10] drm/msm/dsi: Switch the QCM2290-specific compatible to index autodetection
On 14.03.2023 01:03, Marijn Suijten wrote: > On 2023-03-07 14:01:44, Konrad Dybcio wrote: >> Now that the logic can handle multiple sets of registers, move >> the QCM2290 to the common logic and mark it deprecated. This allows us >> to remove a couple of structs, saving some memory. >> >> Reviewed-by: Dmitry Baryshkov >> Signed-off-by: Konrad Dybcio >> --- >> drivers/gpu/drm/msm/dsi/dsi.c | 4 +++- >> drivers/gpu/drm/msm/dsi/dsi_cfg.c | 28 ++-- >> 2 files changed, 5 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c >> index 31fdee2052be..90d43628b22b 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi.c >> +++ b/drivers/gpu/drm/msm/dsi/dsi.c >> @@ -174,7 +174,9 @@ static int dsi_dev_remove(struct platform_device *pdev) >> >> static const struct of_device_id dt_match[] = { >> { .compatible = "qcom,mdss-dsi-ctrl", .data = NULL /* autodetect cfg */ >> }, >> -{ .compatible = "qcom,dsi-ctrl-6g-qcm2290", .data = >> &qcm2290_dsi_cfg_handler }, >> + >> +/* Deprecated, don't use */ >> +{ .compatible = "qcom,dsi-ctrl-6g-qcm2290", .data = NULL }, >> {} >> }; >> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c >> b/drivers/gpu/drm/msm/dsi/dsi_cfg.c >> index 6d4b2ce4b918..29ccd755cc2e 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c >> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c >> @@ -169,7 +169,8 @@ static const struct msm_dsi_config sdm845_dsi_cfg = { >> .bus_clk_names = dsi_v2_4_clk_names, >> .num_bus_clks = ARRAY_SIZE(dsi_v2_4_clk_names), >> .io_start = { >> -{ 0xae94000, 0xae96000 }, /* SDM845 / SDM670 / SC7180 */ >> +{ 0xae94000, 0xae96000 }, /* SDM845 / SDM670 */ >> +{ 0x5e94000 }, /* QCM2290 / SM6115 / SM6125 / SM6375 */ >> }, >> }; >> >> @@ -203,25 +204,6 @@ static const struct msm_dsi_config sc7280_dsi_cfg = { >> }, >> }; >> >> -static const char * const dsi_qcm2290_bus_clk_names[] = { >> -"iface", "bus", >> -}; >> - >> -static const struct regulator_bulk_data qcm2290_dsi_cfg_regulators[] = { >> -{ .supply = "vdda", .init_load_uA = 21800 },/* 1.2 V */ >> -}; > > These two consts should really have already been deleted as part of > 04/10: drm/msm/dsi: dsi_cfg: Deduplicate identical structs. Right, will fix > >> -static const struct msm_dsi_config qcm2290_dsi_cfg = { >> -.io_offset = DSI_6G_REG_SHIFT, >> -.regulator_data = qcm2290_dsi_cfg_regulators, >> -.num_regulators = ARRAY_SIZE(qcm2290_dsi_cfg_regulators), >> -.bus_clk_names = dsi_qcm2290_bus_clk_names, >> -.num_bus_clks = ARRAY_SIZE(dsi_qcm2290_bus_clk_names), >> -.io_start = { >> -{ 0x5e94000 }, >> -}, >> -}; >> - >> static const struct msm_dsi_host_cfg_ops msm_dsi_v2_host_ops = { >> .link_clk_set_rate = dsi_link_clk_set_rate_v2, >> .link_clk_enable = dsi_link_clk_enable_v2, >> @@ -312,9 +294,3 @@ const struct msm_dsi_cfg_handler *msm_dsi_cfg_get(u32 >> major, u32 minor) >> >> return cfg_hnd; >> } >> - >> -/* Non autodetect configs */ >> -const struct msm_dsi_cfg_handler qcm2290_dsi_cfg_handler = { >> -.cfg = &qcm2290_dsi_cfg, >> -.ops = &msm_dsi_6g_v2_host_ops, >> -}; > > And how do you think dsi.c is able to reference this... don't forget to > remove it from dsi_cfg.h in v4. In fact, if you look at how this was > implemented you should also be able to remove #include "dsi_cfg.h" from > dsi.c. A clean revert of that patch would be nice, or just use it as > reference to find the remnants: > > https://lore.kernel.org/all/1644853060-1-2-git-send-email-loic.poul...@linaro.org/ Ack Konrad > > - Marijn
Re: [PATCH v3 06/10] drm/msm/dsi: Switch the QCM2290-specific compatible to index autodetection
On 2023-03-07 14:01:44, Konrad Dybcio wrote: > Now that the logic can handle multiple sets of registers, move > the QCM2290 to the common logic and mark it deprecated. This allows us > to remove a couple of structs, saving some memory. > > Reviewed-by: Dmitry Baryshkov > Signed-off-by: Konrad Dybcio > --- > drivers/gpu/drm/msm/dsi/dsi.c | 4 +++- > drivers/gpu/drm/msm/dsi/dsi_cfg.c | 28 ++-- > 2 files changed, 5 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c > index 31fdee2052be..90d43628b22b 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi.c > +++ b/drivers/gpu/drm/msm/dsi/dsi.c > @@ -174,7 +174,9 @@ static int dsi_dev_remove(struct platform_device *pdev) > > static const struct of_device_id dt_match[] = { > { .compatible = "qcom,mdss-dsi-ctrl", .data = NULL /* autodetect cfg */ > }, > - { .compatible = "qcom,dsi-ctrl-6g-qcm2290", .data = > &qcm2290_dsi_cfg_handler }, > + > + /* Deprecated, don't use */ > + { .compatible = "qcom,dsi-ctrl-6g-qcm2290", .data = NULL }, > {} > }; > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c > b/drivers/gpu/drm/msm/dsi/dsi_cfg.c > index 6d4b2ce4b918..29ccd755cc2e 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c > @@ -169,7 +169,8 @@ static const struct msm_dsi_config sdm845_dsi_cfg = { > .bus_clk_names = dsi_v2_4_clk_names, > .num_bus_clks = ARRAY_SIZE(dsi_v2_4_clk_names), > .io_start = { > - { 0xae94000, 0xae96000 }, /* SDM845 / SDM670 / SC7180 */ > + { 0xae94000, 0xae96000 }, /* SDM845 / SDM670 */ > + { 0x5e94000 }, /* QCM2290 / SM6115 / SM6125 / SM6375 */ > }, > }; > > @@ -203,25 +204,6 @@ static const struct msm_dsi_config sc7280_dsi_cfg = { > }, > }; > > -static const char * const dsi_qcm2290_bus_clk_names[] = { > - "iface", "bus", > -}; > - > -static const struct regulator_bulk_data qcm2290_dsi_cfg_regulators[] = { > - { .supply = "vdda", .init_load_uA = 21800 },/* 1.2 V */ > -}; These two consts should really have already been deleted as part of 04/10: drm/msm/dsi: dsi_cfg: Deduplicate identical structs. > -static const struct msm_dsi_config qcm2290_dsi_cfg = { > - .io_offset = DSI_6G_REG_SHIFT, > - .regulator_data = qcm2290_dsi_cfg_regulators, > - .num_regulators = ARRAY_SIZE(qcm2290_dsi_cfg_regulators), > - .bus_clk_names = dsi_qcm2290_bus_clk_names, > - .num_bus_clks = ARRAY_SIZE(dsi_qcm2290_bus_clk_names), > - .io_start = { > - { 0x5e94000 }, > - }, > -}; > - > static const struct msm_dsi_host_cfg_ops msm_dsi_v2_host_ops = { > .link_clk_set_rate = dsi_link_clk_set_rate_v2, > .link_clk_enable = dsi_link_clk_enable_v2, > @@ -312,9 +294,3 @@ const struct msm_dsi_cfg_handler *msm_dsi_cfg_get(u32 > major, u32 minor) > > return cfg_hnd; > } > - > -/* Non autodetect configs */ > -const struct msm_dsi_cfg_handler qcm2290_dsi_cfg_handler = { > - .cfg = &qcm2290_dsi_cfg, > - .ops = &msm_dsi_6g_v2_host_ops, > -}; And how do you think dsi.c is able to reference this... don't forget to remove it from dsi_cfg.h in v4. In fact, if you look at how this was implemented you should also be able to remove #include "dsi_cfg.h" from dsi.c. A clean revert of that patch would be nice, or just use it as reference to find the remnants: https://lore.kernel.org/all/1644853060-1-2-git-send-email-loic.poul...@linaro.org/ - Marijn
[PATCH v3 06/10] drm/msm/dsi: Switch the QCM2290-specific compatible to index autodetection
Now that the logic can handle multiple sets of registers, move the QCM2290 to the common logic and mark it deprecated. This allows us to remove a couple of structs, saving some memory. Reviewed-by: Dmitry Baryshkov Signed-off-by: Konrad Dybcio --- drivers/gpu/drm/msm/dsi/dsi.c | 4 +++- drivers/gpu/drm/msm/dsi/dsi_cfg.c | 28 ++-- 2 files changed, 5 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c index 31fdee2052be..90d43628b22b 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.c +++ b/drivers/gpu/drm/msm/dsi/dsi.c @@ -174,7 +174,9 @@ static int dsi_dev_remove(struct platform_device *pdev) static const struct of_device_id dt_match[] = { { .compatible = "qcom,mdss-dsi-ctrl", .data = NULL /* autodetect cfg */ }, - { .compatible = "qcom,dsi-ctrl-6g-qcm2290", .data = &qcm2290_dsi_cfg_handler }, + + /* Deprecated, don't use */ + { .compatible = "qcom,dsi-ctrl-6g-qcm2290", .data = NULL }, {} }; diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index 6d4b2ce4b918..29ccd755cc2e 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -169,7 +169,8 @@ static const struct msm_dsi_config sdm845_dsi_cfg = { .bus_clk_names = dsi_v2_4_clk_names, .num_bus_clks = ARRAY_SIZE(dsi_v2_4_clk_names), .io_start = { - { 0xae94000, 0xae96000 }, /* SDM845 / SDM670 / SC7180 */ + { 0xae94000, 0xae96000 }, /* SDM845 / SDM670 */ + { 0x5e94000 }, /* QCM2290 / SM6115 / SM6125 / SM6375 */ }, }; @@ -203,25 +204,6 @@ static const struct msm_dsi_config sc7280_dsi_cfg = { }, }; -static const char * const dsi_qcm2290_bus_clk_names[] = { - "iface", "bus", -}; - -static const struct regulator_bulk_data qcm2290_dsi_cfg_regulators[] = { - { .supply = "vdda", .init_load_uA = 21800 },/* 1.2 V */ -}; - -static const struct msm_dsi_config qcm2290_dsi_cfg = { - .io_offset = DSI_6G_REG_SHIFT, - .regulator_data = qcm2290_dsi_cfg_regulators, - .num_regulators = ARRAY_SIZE(qcm2290_dsi_cfg_regulators), - .bus_clk_names = dsi_qcm2290_bus_clk_names, - .num_bus_clks = ARRAY_SIZE(dsi_qcm2290_bus_clk_names), - .io_start = { - { 0x5e94000 }, - }, -}; - static const struct msm_dsi_host_cfg_ops msm_dsi_v2_host_ops = { .link_clk_set_rate = dsi_link_clk_set_rate_v2, .link_clk_enable = dsi_link_clk_enable_v2, @@ -312,9 +294,3 @@ const struct msm_dsi_cfg_handler *msm_dsi_cfg_get(u32 major, u32 minor) return cfg_hnd; } - -/* Non autodetect configs */ -const struct msm_dsi_cfg_handler qcm2290_dsi_cfg_handler = { - .cfg = &qcm2290_dsi_cfg, - .ops = &msm_dsi_6g_v2_host_ops, -}; -- 2.39.2