Re: [Freedreno] [PATCH v2 5/7] arm64: dts: qcom: sc7280: Update gpu register list
On 7/19/2022 12:49 PM, Stephen Boyd wrote: Quoting Akhil P Oommen (2022-07-18 23:37:16) On 7/19/2022 11:19 AM, Stephen Boyd wrote: Quoting Akhil P Oommen (2022-07-18 21:07:05) On 7/14/2022 11:10 AM, Akhil P Oommen wrote: IIUC, qcom gdsc driver doesn't ensure hardware is collapsed since they are vote-able switches. Ideally, we should ensure that the hw has collapsed for gpu recovery because there could be transient votes from other subsystems like hypervisor using their vote register. I am not sure how complex the plumbing to gpucc driver would be to allow gpu driver to check hw status. OTOH, with this patch, gpu driver does a read operation on a gpucc register which is in always-on domain. That means we don't need to vote any resource to access this register. Reading between the lines here, you're saying that you have to read the gdsc register to make sure that the gdsc is in some state? Can you clarify exactly what you're doing? And how do you know that something else in the kernel can't cause the register to change after it is read? It certainly seems like we can't be certain because there is voting involved. yes, this looks like the best case effort to get the gpu to recover, but the kernel driver really has no control to make sure this condition can always be met (because it depends on other entities like hyp, trustzone etc right?) Why not just put a worst case polling delay? Stephen/Rajendra/Taniya, any suggestion? Why can't you assert a gpu reset signal with the reset APIs? This series seems to jump through a bunch of hoops to get the gdsc and power domain to "reset" when I don't know why any of that is necessary. Can't we simply assert a reset to the hardware after recovery completes so the device is back into a good known POR (power on reset) state? That is because there is no register interface to reset GPU CX domain. The recommended sequence from HW design folks is to collapse both cx and gx gdsc to properly reset gpu/gmu. Ok. One knee jerk reaction is to treat the gdsc as a reset then and possibly mux that request along with any power domain on/off so that if the reset is requested and the power domain is off nothing happens. Otherwise if the power domain is on then it manually sequences and controls the two gdscs so that the GPU is reset and then restores the enable state of the power domain.
Re: [PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate
On 10/4/2020 3:56 AM, Kuogee Hsieh wrote: Set link rate by using OPP set rate api so that CX level will be set accordingly based on the link rate. Changes in v2: -- remove dev from dp_ctrl_put() parameters -- address review comments This needs to go below '---' and should not be part of the change log. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/dp/dp_ctrl.c| 26 + drivers/gpu/drm/msm/dp/dp_display.c | 2 +- drivers/gpu/drm/msm/dp/dp_power.c | 44 ++--- drivers/gpu/drm/msm/dp/dp_power.h | 2 +- 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 2e3e1917351f..6eb9cdad1421 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -76,6 +77,8 @@ struct dp_ctrl_private { struct dp_parser *parser; struct dp_catalog *catalog; + struct opp_table *opp_table; + struct completion idle_comp; struct completion video_comp; }; @@ -1836,6 +1839,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, struct dp_parser *parser) { struct dp_ctrl_private *ctrl; + int ret; if (!dev || !panel || !aux || !link || !catalog) { @@ -1849,6 +1853,19 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, return ERR_PTR(-ENOMEM); } + ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link"); + if (IS_ERR(ctrl->opp_table)) { + dev_err(dev, "invalid DP OPP table in device tree\n"); You do this regardless of an OPP table in DT, so for starters the error message is wrong. Secondly this can return you a -EPROBE_DEFER if the clock driver isn't ready yet. So the ideal thing to do here, is return a PTR_ERR(ctrl->opp_table) + ctrl->opp_table = NULL; + } else { + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (ret && ret != -ENODEV) { + dev_pm_opp_put_clkname(ctrl->opp_table); + ctrl->opp_table = NULL; + } + } + init_completion(>idle_comp); init_completion(>video_comp); @@ -1866,4 +1883,13 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, void dp_ctrl_put(struct dp_ctrl *dp_ctrl) { + struct dp_ctrl_private *ctrl; + + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); + + if (ctrl->opp_table) { + dev_pm_opp_of_remove_table(ctrl->dev); + dev_pm_opp_put_clkname(ctrl->opp_table); + ctrl->opp_table = NULL; + } } diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index e175aa3fd3a9..269f83550b46 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -698,7 +698,7 @@ static int dp_init_sub_modules(struct dp_display_private *dp) goto error; } - dp->power = dp_power_get(dp->parser); + dp->power = dp_power_get(dev, dp->parser); if (IS_ERR(dp->power)) { rc = PTR_ERR(dp->power); DRM_ERROR("failed to initialize power, rc = %d\n", rc); diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c index 17c1fc6a2d44..9c4ea00a5f2a 100644 --- a/drivers/gpu/drm/msm/dp/dp_power.c +++ b/drivers/gpu/drm/msm/dp/dp_power.c @@ -8,12 +8,14 @@ #include #include #include +#include #include "dp_power.h" #include "msm_drv.h" struct dp_power_private { struct dp_parser *parser; struct platform_device *pdev; + struct device *dev; struct clk *link_clk_src; struct clk *pixel_provider; struct clk *link_provider; @@ -148,18 +150,51 @@ static int dp_power_clk_deinit(struct dp_power_private *power) return 0; } +static int dp_power_clk_set_link_rate(struct dp_power_private *power, + struct dss_clk *clk_arry, int num_clk, int enable) +{ + u32 rate; + int i, rc = 0; + + for (i = 0; i < num_clk; i++) { + if (clk_arry[i].clk) { + if (clk_arry[i].type == DSS_CLK_PCLK) { + if (enable) + rate = clk_arry[i].rate; + else + rate = 0; + + rc = dev_pm_opp_set_rate(power->dev, rate); I am not sure how this is expected to work when you have multiple link clocks, since you can only associate one of them with the OPP table which ends up getting scaled when you do a dev_pm_opp_set_rate() Do you really have platforms which will have multiple link clocks? +
Re: [PATCH] drm/msm/dp: add voltage corners voting support base on dp link rate
On 9/30/2020 1:54 PM, Stephen Boyd wrote: Quoting Kuogee Hsieh (2020-09-29 10:10:26) Set link rate by using OPP set rate api so that CX level will be set accordingly base on the link rate. s/base/based/ Signed-off-by: Kuogee Hsieh --- diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 2e3e1917351f..e1595d829e04 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1849,6 +1853,21 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, return ERR_PTR(-ENOMEM); } + ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link"); I see that downstream has multiple DP clocks which end up voting on CX, we don't have a way of associating multiple OPP tables with a device upstream, so whats usually done is (assuming all the clocks get scaled in lock step, which I assume is the case here) we pick the clock with the 'highest' CX requirement and associate that with the OPP table. I haven't looked but I am hoping thats how we have decided to associate "ctrl_link" clock here? + + if (IS_ERR(ctrl->opp_table)) { + dev_err(dev, "invalid DP OPP table in device tree\n"); + ctrl->opp_table = NULL; + } else { + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (ret && ret != -ENODEV) { + dev_err(dev, "add DP OPP table\n"); This is debug noise right? + dev_pm_opp_put_clkname(ctrl->opp_table); + ctrl->opp_table = NULL; + } + } + init_completion(>idle_comp); init_completion(>video_comp); @@ -1864,6 +1883,18 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, return >dp_ctrl; } -void dp_ctrl_put(struct dp_ctrl *dp_ctrl) +void dp_ctrl_put(struct device *dev, struct dp_ctrl *dp_ctrl) { + struct dp_ctrl_private *ctrl; + + if (!dp_ctrl) Can this happen? + return; + + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); + + if (ctrl->opp_table != NULL) { This is usually written as if (ctrl->opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(ctrl->opp_table); + ctrl->opp_table = NULL; + } } diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h index f60ba93c8678..19b412a93e02 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h @@ -31,6 +31,6 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, struct dp_panel *panel, struct drm_dp_aux *aux, struct dp_power *power, struct dp_catalog *catalog, struct dp_parser *parser); -void dp_ctrl_put(struct dp_ctrl *dp_ctrl); +void dp_ctrl_put(struct device *dev, struct dp_ctrl *dp_ctrl); Is 'dev' not inside 'dp_ctrl'? #endif /* _DP_CTRL_H_ */ diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c index 17c1fc6a2d44..3d75bf09e38f 100644 --- a/drivers/gpu/drm/msm/dp/dp_power.c +++ b/drivers/gpu/drm/msm/dp/dp_power.c @@ -8,12 +8,14 @@ #include #include #include +#include #include "dp_power.h" #include "msm_drv.h" struct dp_power_private { struct dp_parser *parser; struct platform_device *pdev; + struct device *dev; struct clk *link_clk_src; struct clk *pixel_provider; struct clk *link_provider; @@ -148,18 +150,49 @@ static int dp_power_clk_deinit(struct dp_power_private *power) return 0; } +static int dp_power_clk_set_link_rate(struct dp_power_private *power, + struct dss_clk *clk_arry, int num_clk, int enable) +{ + u32 rate; + int i, rc = 0; + + for (i = 0; i < num_clk; i++) { + if (clk_arry[i].clk) { + if (clk_arry[i].type == DSS_CLK_PCLK) { + if (enable) + rate = clk_arry[i].rate; + else + rate = 0; + + rc = dev_pm_opp_set_rate(power->dev, rate); Why do we keep going if rc is non-zero? + } + + } + } + return rc; +} + static int dp_power_clk_set_rate(struct dp_power_private *power, enum dp_pm_type module, bool enable) { int rc = 0; struct dss_module_power *mp = >parser->mp[module]; - if (enable) { - rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk); + if (module == DP_CTRL_PM) { + rc = dp_power_clk_set_link_rate(power, mp->clk_config, mp->num_clk, enable); if (rc) { - DRM_ERROR("failed to set
Re: [PATCH V2 3/8] drm/msm: Unconditionally call dev_pm_opp_of_remove_table()
On 9/1/2020 2:08 PM, Viresh Kumar wrote: On 01-09-20, 13:01, Rajendra Nayak wrote: So FWIU, dpu_unbind() gets called even when dpu_bind() fails for some reason. Ahh, I see. I tried to address that earlier [1] which I realized did not land. I don't think that patch was required, as you can call dev_pm_opp_put_clkname() multiple times and it will return without any errors/crash. We did see a crash (Sai had reported it), perhaps with dsi [1] and not this driver. But it was the same scenario that was possible here as well, which is dev_pm_opp_put_clkname() getting called without dev_pm_opp_set_clkname() being done. I think we ended up passing a NULL as opp_table in that case and the function tries de-referencing it. But with these changes it will be even more broken unless we identify if we failed dpu_bind() before adding the OPP table, while adding it, or all went well with opps and handle things accordingly in dpu_unbind. Maybe not as dev_pm_opp_of_remove_table() can be called multiple times as well without any errors or crash. Can it be called without the driver ever doing a dev_pm_opp_of_add_table()? [1] https://lore.kernel.org/patchwork/patch/1275628/ -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2 3/8] drm/msm: Unconditionally call dev_pm_opp_of_remove_table()
On 8/28/2020 11:37 AM, Viresh Kumar wrote: dev_pm_opp_of_remove_table() doesn't report any errors when it fails to find the OPP table with error -ENODEV (i.e. OPP table not present for the device). And we can call dev_pm_opp_of_remove_table() unconditionally here. Its a little tricky to call things unconditionally for this driver, more below. While at it, also create a label to put clkname. Signed-off-by: Viresh Kumar --- V2: - Compare with -ENODEV only for failures. - Create new label to put clkname. --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 - drivers/gpu/drm/msm/dsi/dsi_host.c | 8 ++-- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index c0a4d4e16d82..c8287191951f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1010,12 +1010,9 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) return PTR_ERR(dpu_kms->opp_table); /* OPP table is optional */ ret = dev_pm_opp_of_add_table(dev); - if (!ret) { - dpu_kms->has_opp_table = true; - } else if (ret != -ENODEV) { + if (ret && ret != -ENODEV) { dev_err(dev, "invalid OPP table in device tree\n"); - dev_pm_opp_put_clkname(dpu_kms->opp_table); - return ret; + goto put_clkname; So FWIU, dpu_unbind() gets called even when dpu_bind() fails for some reason. I tried to address that earlier [1] which I realized did not land. But with these changes it will be even more broken unless we identify if we failed dpu_bind() before adding the OPP table, while adding it, or all went well with opps and handle things accordingly in dpu_unbind. [1] https://lore.kernel.org/patchwork/patch/1275632/ } mp = _kms->mp; @@ -1037,8 +1034,8 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = _kms->base; return ret; err: - if (dpu_kms->has_opp_table) - dev_pm_opp_of_remove_table(dev); + dev_pm_opp_of_remove_table(dev); +put_clkname: dev_pm_opp_put_clkname(dpu_kms->opp_table); return ret; } @@ -1056,8 +1053,7 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) if (dpu_kms->rpm_enabled) pm_runtime_disable(>dev); - if (dpu_kms->has_opp_table) - dev_pm_opp_of_remove_table(dev); + dev_pm_opp_of_remove_table(dev); dev_pm_opp_put_clkname(dpu_kms->opp_table); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index e140cd633071..8295979a7165 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -129,7 +129,6 @@ struct dpu_kms { bool rpm_enabled; struct opp_table *opp_table; - bool has_opp_table; struct dss_module_power mp; diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index b17ac6c27554..4335fe33250c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -113,7 +113,6 @@ struct msm_dsi_host { struct clk *byte_intf_clk; struct opp_table *opp_table; - bool has_opp_table; u32 byte_clk_rate; u32 pixel_clk_rate; @@ -1891,9 +1890,7 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) return PTR_ERR(msm_host->opp_table); /* OPP table is optional */ ret = dev_pm_opp_of_add_table(>dev); - if (!ret) { - msm_host->has_opp_table = true; - } else if (ret != -ENODEV) { + if (ret && ret != -ENODEV) { dev_err(>dev, "invalid OPP table in device tree\n"); dev_pm_opp_put_clkname(msm_host->opp_table); return ret; @@ -1934,8 +1931,7 @@ void msm_dsi_host_destroy(struct mipi_dsi_host *host) mutex_destroy(_host->cmd_mutex); mutex_destroy(_host->dev_mutex); - if (msm_host->has_opp_table) - dev_pm_opp_of_remove_table(_host->pdev->dev); + dev_pm_opp_of_remove_table(_host->pdev->dev); dev_pm_opp_put_clkname(msm_host->opp_table); pm_runtime_disable(_host->pdev->dev); } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
On 8/12/2020 1:09 PM, Rajendra Nayak wrote: On 8/12/2020 1:05 PM, Amit Pundir wrote: Hi Rajendra, On Wed, 12 Aug 2020 at 11:18, Rajendra Nayak wrote: On 8/12/2020 7:03 AM, John Stultz wrote: On Tue, Aug 11, 2020 at 4:11 PM John Stultz wrote: On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak wrote: geni serial needs to express a perforamnce state requirement on CX depending on the frequency of the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- drivers/tty/serial/qcom_geni_serial.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) Hey, I just wanted to follow up on this patch, as I've bisected it (a5819b548af0) down as having broken qca bluetooth on the Dragonboard 845c. I haven't yet had time to debug it yet, but wanted to raise the issue in case anyone else has seen similar trouble. So I dug in a bit further, and this chunk seems to be causing the issue: @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, goto out_restart_rx; uport->uartclk = clk_rate; - clk_set_rate(port->se.clk, clk_rate); + dev_pm_opp_set_rate(port->dev, clk_rate); ser_clk_cfg = SER_CLK_EN; ser_clk_cfg |= clk_div << CLK_DIV_SHFT; With that applied, I see the following errors in dmesg and bluetooth fails to function: [ 4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) [ 4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) With just that chunk reverted on linus/HEAD, bluetooth seems to work ok. This seems like the same issue that was also reported on venus [1] because the clock frequency tables apparently don;t exactly match the achievable clock frequencies (which we also used to construct the OPP tables) Can you try updating the OPP table for QUP to have 10240 instead of the current 1 and see if that fixes it? That worked. Thanks. Should this change be common to base sdm845.dtsi or platform specific dts? For what it's worth, we see this BT breakage on PocoF1 phone too. Thanks for confirming, it will have to be part of the SoC dtsi, and I am guessing a similar change is perhaps also needed on sc7180. I will send a patch out to fix the OPP tables for both. I spent some more time looking at this and it does not look like this is the rounding issues with clock FMAX tables. I had these tables picked from downstream clock code and it turns out these tables were reworked at clock init based on the silicon rev, so I need to fix up the OPP tables accordingly which will add a new OPP entry for 102.4Mhz. I'll post a patch shortly. Regards, Amit Pundir [1] https://lkml.org/lkml/2020/7/27/507 thanks -john -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
On 8/12/2020 1:05 PM, Amit Pundir wrote: Hi Rajendra, On Wed, 12 Aug 2020 at 11:18, Rajendra Nayak wrote: On 8/12/2020 7:03 AM, John Stultz wrote: On Tue, Aug 11, 2020 at 4:11 PM John Stultz wrote: On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak wrote: geni serial needs to express a perforamnce state requirement on CX depending on the frequency of the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- drivers/tty/serial/qcom_geni_serial.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) Hey, I just wanted to follow up on this patch, as I've bisected it (a5819b548af0) down as having broken qca bluetooth on the Dragonboard 845c. I haven't yet had time to debug it yet, but wanted to raise the issue in case anyone else has seen similar trouble. So I dug in a bit further, and this chunk seems to be causing the issue: @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, goto out_restart_rx; uport->uartclk = clk_rate; - clk_set_rate(port->se.clk, clk_rate); + dev_pm_opp_set_rate(port->dev, clk_rate); ser_clk_cfg = SER_CLK_EN; ser_clk_cfg |= clk_div << CLK_DIV_SHFT; With that applied, I see the following errors in dmesg and bluetooth fails to function: [4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) [4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) With just that chunk reverted on linus/HEAD, bluetooth seems to work ok. This seems like the same issue that was also reported on venus [1] because the clock frequency tables apparently don;t exactly match the achievable clock frequencies (which we also used to construct the OPP tables) Can you try updating the OPP table for QUP to have 10240 instead of the current 1 and see if that fixes it? That worked. Thanks. Should this change be common to base sdm845.dtsi or platform specific dts? For what it's worth, we see this BT breakage on PocoF1 phone too. Thanks for confirming, it will have to be part of the SoC dtsi, and I am guessing a similar change is perhaps also needed on sc7180. I will send a patch out to fix the OPP tables for both. Regards, Amit Pundir [1] https://lkml.org/lkml/2020/7/27/507 thanks -john -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
On 8/12/2020 7:03 AM, John Stultz wrote: On Tue, Aug 11, 2020 at 4:11 PM John Stultz wrote: On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak wrote: geni serial needs to express a perforamnce state requirement on CX depending on the frequency of the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- drivers/tty/serial/qcom_geni_serial.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) Hey, I just wanted to follow up on this patch, as I've bisected it (a5819b548af0) down as having broken qca bluetooth on the Dragonboard 845c. I haven't yet had time to debug it yet, but wanted to raise the issue in case anyone else has seen similar trouble. So I dug in a bit further, and this chunk seems to be causing the issue: @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, goto out_restart_rx; uport->uartclk = clk_rate; - clk_set_rate(port->se.clk, clk_rate); + dev_pm_opp_set_rate(port->dev, clk_rate); ser_clk_cfg = SER_CLK_EN; ser_clk_cfg |= clk_div << CLK_DIV_SHFT; With that applied, I see the following errors in dmesg and bluetooth fails to function: [4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) [4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) With just that chunk reverted on linus/HEAD, bluetooth seems to work ok. This seems like the same issue that was also reported on venus [1] because the clock frequency tables apparently don;t exactly match the achievable clock frequencies (which we also used to construct the OPP tables) Can you try updating the OPP table for QUP to have 10240 instead of the current 1 and see if that fixes it? [1] https://lkml.org/lkml/2020/7/27/507 thanks -john -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dpu: dev_pm_opp_put_clkname() only when an opp_table exists
Its possible that dpu_bind() fails early enough before dev_pm_opp_set_clkname() is called. In such cases, unconditionally calling dev_pm_opp_put_clkname() in dpu_unbind() can result in a crash. Put an additional check so that dev_pm_opp_put_clkname() is called only when an opp_table exists. Fixes: aa3950767d05 ("drm/msm/dpu: Use OPP API to set clk/perf state") Signed-off-by: Rajendra Nayak --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index f2bbce4..843a1c1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1079,7 +1079,8 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) if (dpu_kms->has_opp_table) dev_pm_opp_of_remove_table(dev); - dev_pm_opp_put_clkname(dpu_kms->opp_table); + if (dpu_kms->opp_table) + dev_pm_opp_put_clkname(dpu_kms->opp_table); } static const struct component_ops dpu_ops = { -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm: dsi: dev_pm_opp_put_clkname() only when an opp_table exists
Its possible for msm_dsi_host_init() to fail early, before dev_pm_opp_set_clkname() is called. In such cases, unconditionally calling dev_pm_opp_put_clkname() in msm_dsi_host_destroy() results in a crash. Put an additional check so that dev_pm_opp_put_clkname() is called only when an opp_table exists. Fixes: f99131fa7a23 ("drm/msm: dsi: Use OPP API to set clk/perf state") Reported-by: Sai Prakash Ranjan Signed-off-by: Rajendra Nayak --- drivers/gpu/drm/msm/dsi/dsi_host.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 0a14c4a..4f580f7 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1936,7 +1936,8 @@ void msm_dsi_host_destroy(struct mipi_dsi_host *host) if (msm_host->has_opp_table) dev_pm_opp_of_remove_table(_host->pdev->dev); - dev_pm_opp_put_clkname(msm_host->opp_table); + if (msm_host->opp_table) + dev_pm_opp_put_clkname(msm_host->opp_table); pm_runtime_disable(_host->pdev->dev); } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/4] drm/msm: dsi: Use OPP API to set clk/perf state
On SDM845 and SC7180 DSI needs to express a performance state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. dev_pm_opp_set_rate() is designed to be equivalent to clk_set_rate() for devices without an OPP table, hence the change works fine on devices/platforms which only need to set a clock rate. Signed-off-by: Rajendra Nayak Reviewed-by: Matthias Kaehlcke --- drivers/gpu/drm/msm/dsi/dsi_host.c | 27 +-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..0a14c4a 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,9 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp_table; + bool has_opp_table; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -512,9 +516,10 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) DBG("Set clk rates: pclk=%d, byteclk=%d", msm_host->mode->clock, msm_host->byte_clk_rate); - ret = clk_set_rate(msm_host->byte_clk, msm_host->byte_clk_rate); + ret = dev_pm_opp_set_rate(_host->pdev->dev, + msm_host->byte_clk_rate); if (ret) { - pr_err("%s: Failed to set rate byte clk, %d\n", __func__, ret); + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); return ret; } @@ -658,6 +663,8 @@ int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host) void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host) { + /* Drop the performance state vote */ + dev_pm_opp_set_rate(_host->pdev->dev, 0); clk_disable_unprepare(msm_host->esc_clk); clk_disable_unprepare(msm_host->pixel_clk); if (msm_host->byte_intf_clk) @@ -1879,6 +1886,19 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } + msm_host->opp_table = dev_pm_opp_set_clkname(>dev, "byte"); + if (IS_ERR(msm_host->opp_table)) + return PTR_ERR(msm_host->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(>dev); + if (!ret) { + msm_host->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(>dev, "invalid OPP table in device tree\n"); + dev_pm_opp_put_clkname(msm_host->opp_table); + return ret; + } + init_completion(_host->dma_comp); init_completion(_host->video_comp); mutex_init(_host->dev_mutex); @@ -1914,6 +1934,9 @@ void msm_dsi_host_destroy(struct mipi_dsi_host *host) mutex_destroy(_host->cmd_mutex); mutex_destroy(_host->dev_mutex); + if (msm_host->has_opp_table) + dev_pm_opp_of_remove_table(_host->pdev->dev); + dev_pm_opp_put_clkname(msm_host->opp_table); pm_runtime_disable(_host->pdev->dev); } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/4] drm/msm/dpu: Use OPP API to set clk/perf state
On some qualcomm platforms DPU needs to express a performance state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Reviewed-by: Rob Clark Reviewed-by: Matthias Kaehlcke --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 27 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 7c230f7..b36919d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -218,7 +219,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 680527e..2d5e8e8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1025,11 +1026,24 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dpu_kms->opp_table = dev_pm_opp_set_clkname(dev, "core"); + if (IS_ERR(dpu_kms->opp_table)) + return PTR_ERR(dpu_kms->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (!ret) { + dpu_kms->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(dev, "invalid OPP table in device tree\n"); + dev_pm_opp_put_clkname(dpu_kms->opp_table); + return ret; + } + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { DPU_ERROR("failed to parse clocks, ret=%d\n", ret); - return ret; + goto err; } platform_set_drvdata(pdev, dpu_kms); @@ -1043,6 +1057,11 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = _kms->base; return ret; +err: + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); + return ret; } static void dpu_unbind(struct device *dev, struct device *master, void *data) @@ -1057,6 +1076,10 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) if (dpu_kms->rpm_enabled) pm_runtime_disable(>dev); + + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); } static const struct component_ops dpu_ops = { @@ -1082,6 +1105,8 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index a3b122b..7400cd7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -128,6 +128,10 @@ struct dpu_kms { struct platform_device *pdev; bool rpm_enabled; + + struct opp_table *opp_table; + bool has_opp_table; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/4] arm64: dts: sc7180: Add DSI and MDP OPP tables and power-domains
Add the OPP tables for DSI and MDP based on the perf state/clk requirements, and add the power-domains property to specify the scalable power domain. Signed-off-by: Rajendra Nayak Reviewed-by: Matthias Kaehlcke --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 49 1 file changed, 49 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index 2be81a2..1ce895f 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -2642,6 +2642,8 @@ <1920>, <1920>, <1920>; + operating-points-v2 = <_opp_table>; + power-domains = < SC7180_CX>; interrupt-parent = <>; interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; @@ -2659,6 +2661,31 @@ }; }; }; + + mdp_opp_table: mdp-opp-table { + compatible = "operating-points-v2"; + + opp-2 { + opp-hz = /bits/ 64 <2>; + required-opps = <_opp_low_svs>; + }; + + opp-3 { + opp-hz = /bits/ 64 <3>; + required-opps = <_opp_svs>; + }; + + opp-34500 { + opp-hz = /bits/ 64 <34500>; + required-opps = <_opp_svs_l1>; + }; + + opp-46000 { + opp-hz = /bits/ 64 <46000>; + required-opps = <_opp_nom>; + }; + }; + }; dsi0: dsi@ae94000 { @@ -2682,6 +2709,9 @@ "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SC7180_CX>; + phys = <_phy>; phy-names = "dsi"; @@ -2707,6 +2737,25 @@ }; }; }; + + dsi_opp_table: dsi-opp-table { + compatible = "operating-points-v2"; + + opp-18750 { + opp-hz = /bits/ 64 <18750>; + required-opps = <_opp_low_svs>; + }; + + opp-3 { + opp-hz = /bits/ 64 <3>; + required-opps = <_opp_svs>; + }; + + opp-35800 { + opp-hz = /bits/ 64 <35800>; + required-opps = <_opp_svs_l1>; + }; + }; }; dsi_phy: dsi-phy@ae94400 { -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/4] arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains
Add the OPP tables for DSI and MDP based on the perf state/clk requirements, and add the power-domains property to specify the scalable power domain. Signed-off-by: Rajendra Nayak Reviewed-by: Matthias Kaehlcke --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 59 1 file changed, 59 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index fee50d9..3efdd70 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -3296,6 +3296,35 @@ #power-domain-cells = <1>; }; + dsi_opp_table: dsi-opp-table { + compatible = "operating-points-v2"; + + opp-1920 { + opp-hz = /bits/ 64 <1920>; + required-opps = <_opp_min_svs>; + }; + + opp-18000 { + opp-hz = /bits/ 64 <18000>; + required-opps = <_opp_low_svs>; + }; + + opp-27500 { + opp-hz = /bits/ 64 <27500>; + required-opps = <_opp_svs>; + }; + + opp-32858 { + opp-hz = /bits/ 64 <32858>; + required-opps = <_opp_svs_l1>; + }; + + opp-35800 { + opp-hz = /bits/ 64 <35800>; + required-opps = <_opp_nom>; + }; + }; + mdss: mdss@ae0 { compatible = "qcom,sdm845-mdss"; reg = <0 0x0ae0 0 0x1000>; @@ -3340,6 +3369,8 @@ < DISP_CC_MDSS_VSYNC_CLK>; assigned-clock-rates = <3>, <1920>; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; interrupt-parent = <>; interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; @@ -3364,6 +3395,30 @@ }; }; }; + + mdp_opp_table: mdp-opp-table { + compatible = "operating-points-v2"; + + opp-1920 { + opp-hz = /bits/ 64 <1920>; + required-opps = <_opp_min_svs>; + }; + + opp-171428571 { + opp-hz = /bits/ 64 <171428571>; + required-opps = <_opp_low_svs>; + }; + + opp-34400 { + opp-hz = /bits/ 64 <34400>; + required-opps = <_opp_svs_l1>; + }; + + opp-43000 { + opp-hz = /bits/ 64 <43000>; + required-opps = <_opp_nom>; + }; + }; }; dsi0: dsi@ae94000 { @@ -3386,6 +3441,8 @@ "core", "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; phys = <_phy>; phy-names = "dsi"; @@ -3450,6 +3507,8 @@ "core", "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; phys = <_phy>; phy-names = "dsi"; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/4] DVFS support for dpu and dsi
Changes in v3 - Added dev_pm_opp_put_clkname() in the error path Changes in v2 - Patch 2: Dropped dsi_link_clk_set_rate_6g_v2 and dsi_link_clk_disable_6g_v2 as suggested by Matthias These patches add DVFS support for DPU and DSI. These patches have no other dependency. Patch 1 and 2 will need to be merged in via the MSM DRM tree. DT patches will need to land via the msm tree. Rajendra Nayak (4): drm/msm/dpu: Use OPP API to set clk/perf state drm/msm: dsi: Use OPP API to set clk/perf state arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains arm64: dts: sc7180: Add DSI and MDP OPP tables and power-domains arch/arm64/boot/dts/qcom/sc7180.dtsi | 49 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 59 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 27 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 ++ drivers/gpu/drm/msm/dsi/dsi_host.c| 27 +++- 6 files changed, 165 insertions(+), 4 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/4] drm/msm: dsi: Use OPP API to set clk/perf state
On 7/6/2020 9:40 PM, Matthias Kaehlcke wrote: On Thu, Jul 02, 2020 at 04:39:09PM +0530, Rajendra Nayak wrote: On SDM845 and SC7180 DSI needs to express a performance state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. dev_pm_opp_set_rate() is designed to be equivalent to clk_set_rate() for devices without an OPP table, hence the change works fine on devices/platforms which only need to set a clock rate. Signed-off-by: Rajendra Nayak --- drivers/gpu/drm/msm/dsi/dsi_host.c | 26 -- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..09e16b8 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,9 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp_table; + bool has_opp_table; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -512,9 +516,10 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) DBG("Set clk rates: pclk=%d, byteclk=%d", msm_host->mode->clock, msm_host->byte_clk_rate); - ret = clk_set_rate(msm_host->byte_clk, msm_host->byte_clk_rate); + ret = dev_pm_opp_set_rate(_host->pdev->dev, + msm_host->byte_clk_rate); if (ret) { - pr_err("%s: Failed to set rate byte clk, %d\n", __func__, ret); + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); return ret; } @@ -658,6 +663,8 @@ int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host) void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host) { + /* Drop the performance state vote */ + dev_pm_opp_set_rate(_host->pdev->dev, 0); clk_disable_unprepare(msm_host->esc_clk); clk_disable_unprepare(msm_host->pixel_clk); if (msm_host->byte_intf_clk) @@ -1879,6 +1886,18 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } + msm_host->opp_table = dev_pm_opp_set_clkname(>dev, "byte"); + if (IS_ERR(msm_host->opp_table)) + return PTR_ERR(msm_host->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(>dev); + if (!ret) { + msm_host->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(>dev, "invalid OPP table in device tree\n"); dev_pm_opp_put_clkname(msm_host->opp_table); + return ret; + } With the missing _put_clkname() fixed: Thanks, I'll fix and resend. Reviewed-by: Matthias Kaehlcke -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/4] arm64: dts: sc7180: Add DSI and MDP OPP tables and power-domains
Add the OPP tables for DSI and MDP based on the perf state/clk requirements, and add the power-domains property to specify the scalable power domain. Signed-off-by: Rajendra Nayak Reviewed-by: Matthias Kaehlcke --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 49 1 file changed, 49 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index ad57df2..3430c33f 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -2482,6 +2482,8 @@ <1920>, <1920>, <1920>; + operating-points-v2 = <_opp_table>; + power-domains = < SC7180_CX>; interrupt-parent = <>; interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; @@ -2499,6 +2501,31 @@ }; }; }; + + mdp_opp_table: mdp-opp-table { + compatible = "operating-points-v2"; + + opp-2 { + opp-hz = /bits/ 64 <2>; + required-opps = <_opp_low_svs>; + }; + + opp-3 { + opp-hz = /bits/ 64 <3>; + required-opps = <_opp_svs>; + }; + + opp-34500 { + opp-hz = /bits/ 64 <34500>; + required-opps = <_opp_svs_l1>; + }; + + opp-46000 { + opp-hz = /bits/ 64 <46000>; + required-opps = <_opp_nom>; + }; + }; + }; dsi0: dsi@ae94000 { @@ -2522,6 +2549,9 @@ "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SC7180_CX>; + phys = <_phy>; phy-names = "dsi"; @@ -2547,6 +2577,25 @@ }; }; }; + + dsi_opp_table: dsi-opp-table { + compatible = "operating-points-v2"; + + opp-18750 { + opp-hz = /bits/ 64 <18750>; + required-opps = <_opp_low_svs>; + }; + + opp-3 { + opp-hz = /bits/ 64 <3>; + required-opps = <_opp_svs>; + }; + + opp-35800 { + opp-hz = /bits/ 64 <35800>; + required-opps = <_opp_svs_l1>; + }; + }; }; dsi_phy: dsi-phy@ae94400 { -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/4] DVFS support for dpu and dsi
Changes in v2 - Patch 2: Dropped dsi_link_clk_set_rate_6g_v2 and dsi_link_clk_disable_6g_v2 as suggested by Matthias These patches add DVFS support for DPU and DSI. These patches have no other dependency. Patch 1 and 2 will need to be merged in via the MSM DRM tree. DT patches will need to land via the msm tree. Rajendra Nayak (4): drm/msm/dpu: Use OPP API to set clk/perf state drm/msm: dsi: Use OPP API to set clk/perf state arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains arm64: dts: sc7180: Add DSI and MDP OPP tables and power-domains arch/arm64/boot/dts/qcom/sc7180.dtsi | 49 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 59 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 26 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 ++ drivers/gpu/drm/msm/dsi/dsi_host.c| 26 +++- 6 files changed, 163 insertions(+), 4 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/4] drm/msm/dpu: Use OPP API to set clk/perf state
On some qualcomm platforms DPU needs to express a performance state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Reviewed-by: Rob Clark Reviewed-by: Matthias Kaehlcke --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 26 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 7c230f7..b36919d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -218,7 +219,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index b8615d4..0bc8ec4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1025,11 +1026,23 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dpu_kms->opp_table = dev_pm_opp_set_clkname(dev, "core"); + if (IS_ERR(dpu_kms->opp_table)) + return PTR_ERR(dpu_kms->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (!ret) { + dpu_kms->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(dev, "invalid OPP table in device tree\n"); + return ret; + } + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { DPU_ERROR("failed to parse clocks, ret=%d\n", ret); - return ret; + goto err; } platform_set_drvdata(pdev, dpu_kms); @@ -1043,6 +1056,11 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = _kms->base; return ret; +err: + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); + return ret; } static void dpu_unbind(struct device *dev, struct device *master, void *data) @@ -1057,6 +1075,10 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) if (dpu_kms->rpm_enabled) pm_runtime_disable(>dev); + + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); } static const struct component_ops dpu_ops = { @@ -1082,6 +1104,8 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index a3b122b..7400cd7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -128,6 +128,10 @@ struct dpu_kms { struct platform_device *pdev; bool rpm_enabled; + + struct opp_table *opp_table; + bool has_opp_table; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/4] drm/msm: dsi: Use OPP API to set clk/perf state
On SDM845 and SC7180 DSI needs to express a performance state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. dev_pm_opp_set_rate() is designed to be equivalent to clk_set_rate() for devices without an OPP table, hence the change works fine on devices/platforms which only need to set a clock rate. Signed-off-by: Rajendra Nayak --- drivers/gpu/drm/msm/dsi/dsi_host.c | 26 -- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..09e16b8 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,9 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp_table; + bool has_opp_table; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -512,9 +516,10 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) DBG("Set clk rates: pclk=%d, byteclk=%d", msm_host->mode->clock, msm_host->byte_clk_rate); - ret = clk_set_rate(msm_host->byte_clk, msm_host->byte_clk_rate); + ret = dev_pm_opp_set_rate(_host->pdev->dev, + msm_host->byte_clk_rate); if (ret) { - pr_err("%s: Failed to set rate byte clk, %d\n", __func__, ret); + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); return ret; } @@ -658,6 +663,8 @@ int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host) void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host) { + /* Drop the performance state vote */ + dev_pm_opp_set_rate(_host->pdev->dev, 0); clk_disable_unprepare(msm_host->esc_clk); clk_disable_unprepare(msm_host->pixel_clk); if (msm_host->byte_intf_clk) @@ -1879,6 +1886,18 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } + msm_host->opp_table = dev_pm_opp_set_clkname(>dev, "byte"); + if (IS_ERR(msm_host->opp_table)) + return PTR_ERR(msm_host->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(>dev); + if (!ret) { + msm_host->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(>dev, "invalid OPP table in device tree\n"); + return ret; + } + init_completion(_host->dma_comp); init_completion(_host->video_comp); mutex_init(_host->dev_mutex); @@ -1914,6 +1933,9 @@ void msm_dsi_host_destroy(struct mipi_dsi_host *host) mutex_destroy(_host->cmd_mutex); mutex_destroy(_host->dev_mutex); + if (msm_host->has_opp_table) + dev_pm_opp_of_remove_table(_host->pdev->dev); + dev_pm_opp_put_clkname(msm_host->opp_table); pm_runtime_disable(_host->pdev->dev); } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/4] arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains
Add the OPP tables for DSI and MDP based on the perf state/clk requirements, and add the power-domains property to specify the scalable power domain. Signed-off-by: Rajendra Nayak Reviewed-by: Matthias Kaehlcke --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 59 1 file changed, 59 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 8eb5a31..b6afeb2 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -3296,6 +3296,35 @@ #power-domain-cells = <1>; }; + dsi_opp_table: dsi-opp-table { + compatible = "operating-points-v2"; + + opp-1920 { + opp-hz = /bits/ 64 <1920>; + required-opps = <_opp_min_svs>; + }; + + opp-18000 { + opp-hz = /bits/ 64 <18000>; + required-opps = <_opp_low_svs>; + }; + + opp-27500 { + opp-hz = /bits/ 64 <27500>; + required-opps = <_opp_svs>; + }; + + opp-32858 { + opp-hz = /bits/ 64 <32858>; + required-opps = <_opp_svs_l1>; + }; + + opp-35800 { + opp-hz = /bits/ 64 <35800>; + required-opps = <_opp_nom>; + }; + }; + mdss: mdss@ae0 { compatible = "qcom,sdm845-mdss"; reg = <0 0x0ae0 0 0x1000>; @@ -3340,6 +3369,8 @@ < DISP_CC_MDSS_VSYNC_CLK>; assigned-clock-rates = <3>, <1920>; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; interrupt-parent = <>; interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; @@ -3364,6 +3395,30 @@ }; }; }; + + mdp_opp_table: mdp-opp-table { + compatible = "operating-points-v2"; + + opp-1920 { + opp-hz = /bits/ 64 <1920>; + required-opps = <_opp_min_svs>; + }; + + opp-171428571 { + opp-hz = /bits/ 64 <171428571>; + required-opps = <_opp_low_svs>; + }; + + opp-34400 { + opp-hz = /bits/ 64 <34400>; + required-opps = <_opp_svs_l1>; + }; + + opp-43000 { + opp-hz = /bits/ 64 <43000>; + required-opps = <_opp_nom>; + }; + }; }; dsi0: dsi@ae94000 { @@ -3386,6 +3441,8 @@ "core", "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; phys = <_phy>; phy-names = "dsi"; @@ -3450,6 +3507,8 @@ "core", "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; phys = <_phy>; phy-names = "dsi"; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/msm: dsi: Use OPP API to set clk/perf state
On 7/1/2020 9:57 PM, Matthias Kaehlcke wrote: On Tue, Jun 30, 2020 at 05:26:14PM +0530, Rajendra Nayak wrote: On SDM845 DSI needs to express a perforamnce state nit: performance requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak --- drivers/gpu/drm/msm/dsi/dsi.h | 2 ++ drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 +-- drivers/gpu/drm/msm/dsi/dsi_host.c | 58 ++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 4de771d..ba7583c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -180,10 +180,12 @@ int msm_dsi_runtime_suspend(struct device *dev); int msm_dsi_runtime_resume(struct device *dev); int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_set_rate_v2(struct msm_dsi_host *msm_host); +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host); +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host); int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size); int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size); void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index 813d69d..773c4fe 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -210,9 +210,9 @@ static const struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = { }; static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = { - .link_clk_set_rate = dsi_link_clk_set_rate_6g, + .link_clk_set_rate = dsi_link_clk_set_rate_6g_v2, .link_clk_enable = dsi_link_clk_enable_6g, - .link_clk_disable = dsi_link_clk_disable_6g, + .link_clk_disable = dsi_link_clk_disable_6g_v2, .clk_init_ver = dsi_clk_init_6g_v2, .tx_buf_alloc = dsi_tx_buf_alloc_6g, .tx_buf_get = dsi_tx_buf_get_6g, diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..890531c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,9 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp_table; + bool has_opp_table; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -537,6 +541,38 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) return 0; } +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host) +{ + int ret; + struct device *dev = _host->pdev->dev; + + DBG("Set clk rates: pclk=%d, byteclk=%d", + msm_host->mode->clock, msm_host->byte_clk_rate); + + ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate); + if (ret) { + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); + return ret; + } + + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); + if (ret) { + pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); + return ret; + } + + if (msm_host->byte_intf_clk) { + ret = clk_set_rate(msm_host->byte_intf_clk, + msm_host->byte_clk_rate / 2); + if (ret) { + pr_err("%s: Failed to set rate byte intf clk, %d\n", + __func__, ret); + return ret; + } + } + + return 0; +} xThis function is essentially the same as dsi_link_clk_set_rate_6g(), except for the use of dev_pm_opp_set_rate() instead of clk_set_rate(). IIUC dev_pm_opp_set_rate() just calls clk_set_rate() if the device has no OPP table. If that's correct you could just call dev_pm_opp_set_rate() in dsi_link_clk_set_rate_6g(). /* * For IO devices which require an OPP on some platforms/SoCs * while just needing to scale the clock on some others * we look for empty OPP tables with just a clock handle and * scale only the clk. This makes dev_pm_opp_set_rate() * equivalent to a clk_set_rate() */ if (!_get_opp_count(opp_table)) { ret = _generic_set_opp_clk_only(dev, clk, freq); goto put_opp_table; } ah
[PATCH 2/4] drm/msm: dsi: Use OPP API to set clk/perf state
On SDM845 DSI needs to express a perforamnce state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak --- drivers/gpu/drm/msm/dsi/dsi.h | 2 ++ drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 +-- drivers/gpu/drm/msm/dsi/dsi_host.c | 58 ++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 4de771d..ba7583c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -180,10 +180,12 @@ int msm_dsi_runtime_suspend(struct device *dev); int msm_dsi_runtime_resume(struct device *dev); int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_set_rate_v2(struct msm_dsi_host *msm_host); +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host); +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host); int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size); int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size); void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index 813d69d..773c4fe 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -210,9 +210,9 @@ static const struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = { }; static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = { - .link_clk_set_rate = dsi_link_clk_set_rate_6g, + .link_clk_set_rate = dsi_link_clk_set_rate_6g_v2, .link_clk_enable = dsi_link_clk_enable_6g, - .link_clk_disable = dsi_link_clk_disable_6g, + .link_clk_disable = dsi_link_clk_disable_6g_v2, .clk_init_ver = dsi_clk_init_6g_v2, .tx_buf_alloc = dsi_tx_buf_alloc_6g, .tx_buf_get = dsi_tx_buf_get_6g, diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..890531c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,9 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp_table; + bool has_opp_table; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -537,6 +541,38 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) return 0; } +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host) +{ + int ret; + struct device *dev = _host->pdev->dev; + + DBG("Set clk rates: pclk=%d, byteclk=%d", + msm_host->mode->clock, msm_host->byte_clk_rate); + + ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate); + if (ret) { + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); + return ret; + } + + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); + if (ret) { + pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); + return ret; + } + + if (msm_host->byte_intf_clk) { + ret = clk_set_rate(msm_host->byte_intf_clk, + msm_host->byte_clk_rate / 2); + if (ret) { + pr_err("%s: Failed to set rate byte intf clk, %d\n", + __func__, ret); + return ret; + } + } + + return 0; +} int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) { @@ -665,6 +701,13 @@ void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host) clk_disable_unprepare(msm_host->byte_clk); } +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host) +{ + /* Drop the performance state vote */ + dev_pm_opp_set_rate(_host->pdev->dev, 0); + dsi_link_clk_disable_6g(msm_host); +} + void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) { clk_disable_unprepare(msm_host->pixel_clk); @@ -1879,6 +1922,18 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } + msm_host->opp_table = dev_pm_opp_set_clkname(>dev, "byte"); + if (IS_ERR(msm_host->opp_table)) + return PTR_ERR(msm_host->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(>dev); + if (!ret) { + msm_host->has_opp_t
[PATCH 0/4] DVFS support for dpu and dsi
These patches add DVFS support for DPU and DSI. Where posted earlier as part of a series with multiple different drivers [1] I have split them into specific driver changes in order to avoid confusion on dependencies. Also added the corresponding device tree changes for sdm845 and sc7180 platforms. These patches have no other dependency and will need to be merged in via the MSM DRM tree. DT patches will need to land via the msm tree. [1] https://lkml.org/lkml/2020/6/15/535 Rajendra Nayak (4): drm/msm/dpu: Use OPP API to set clk/perf state drm/msm: dsi: Use OPP API to set clk/perf state arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains arm64: dts: sc7180: Add DSI and MDP OPP tables and power-domains arch/arm64/boot/dts/qcom/sc7180.dtsi | 49 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 59 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 26 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 ++ drivers/gpu/drm/msm/dsi/dsi.h | 2 + drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 +- drivers/gpu/drm/msm/dsi/dsi_host.c| 58 ++ 8 files changed, 201 insertions(+), 4 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] arm64: dts: sc7180: Add DSI and MDP OPP tables and power-domains
Add the OPP tables for DSI and MDP based on the perf state/clk requirements, and add the power-domains property to specify the scalable power domain. Signed-off-by: Rajendra Nayak --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 49 1 file changed, 49 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index ad57df2..3430c33f 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -2482,6 +2482,8 @@ <1920>, <1920>, <1920>; + operating-points-v2 = <_opp_table>; + power-domains = < SC7180_CX>; interrupt-parent = <>; interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; @@ -2499,6 +2501,31 @@ }; }; }; + + mdp_opp_table: mdp-opp-table { + compatible = "operating-points-v2"; + + opp-2 { + opp-hz = /bits/ 64 <2>; + required-opps = <_opp_low_svs>; + }; + + opp-3 { + opp-hz = /bits/ 64 <3>; + required-opps = <_opp_svs>; + }; + + opp-34500 { + opp-hz = /bits/ 64 <34500>; + required-opps = <_opp_svs_l1>; + }; + + opp-46000 { + opp-hz = /bits/ 64 <46000>; + required-opps = <_opp_nom>; + }; + }; + }; dsi0: dsi@ae94000 { @@ -2522,6 +2549,9 @@ "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SC7180_CX>; + phys = <_phy>; phy-names = "dsi"; @@ -2547,6 +2577,25 @@ }; }; }; + + dsi_opp_table: dsi-opp-table { + compatible = "operating-points-v2"; + + opp-18750 { + opp-hz = /bits/ 64 <18750>; + required-opps = <_opp_low_svs>; + }; + + opp-3 { + opp-hz = /bits/ 64 <3>; + required-opps = <_opp_svs>; + }; + + opp-35800 { + opp-hz = /bits/ 64 <35800>; + required-opps = <_opp_svs_l1>; + }; + }; }; dsi_phy: dsi-phy@ae94400 { -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains
Add the OPP tables for DSI and MDP based on the perf state/clk requirements, and add the power-domains property to specify the scalable power domain. Signed-off-by: Rajendra Nayak --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 59 1 file changed, 59 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 8eb5a31..b6afeb2 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -3296,6 +3296,35 @@ #power-domain-cells = <1>; }; + dsi_opp_table: dsi-opp-table { + compatible = "operating-points-v2"; + + opp-1920 { + opp-hz = /bits/ 64 <1920>; + required-opps = <_opp_min_svs>; + }; + + opp-18000 { + opp-hz = /bits/ 64 <18000>; + required-opps = <_opp_low_svs>; + }; + + opp-27500 { + opp-hz = /bits/ 64 <27500>; + required-opps = <_opp_svs>; + }; + + opp-32858 { + opp-hz = /bits/ 64 <32858>; + required-opps = <_opp_svs_l1>; + }; + + opp-35800 { + opp-hz = /bits/ 64 <35800>; + required-opps = <_opp_nom>; + }; + }; + mdss: mdss@ae0 { compatible = "qcom,sdm845-mdss"; reg = <0 0x0ae0 0 0x1000>; @@ -3340,6 +3369,8 @@ < DISP_CC_MDSS_VSYNC_CLK>; assigned-clock-rates = <3>, <1920>; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; interrupt-parent = <>; interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; @@ -3364,6 +3395,30 @@ }; }; }; + + mdp_opp_table: mdp-opp-table { + compatible = "operating-points-v2"; + + opp-1920 { + opp-hz = /bits/ 64 <1920>; + required-opps = <_opp_min_svs>; + }; + + opp-171428571 { + opp-hz = /bits/ 64 <171428571>; + required-opps = <_opp_low_svs>; + }; + + opp-34400 { + opp-hz = /bits/ 64 <34400>; + required-opps = <_opp_svs_l1>; + }; + + opp-43000 { + opp-hz = /bits/ 64 <43000>; + required-opps = <_opp_nom>; + }; + }; }; dsi0: dsi@ae94000 { @@ -3386,6 +3441,8 @@ "core", "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; phys = <_phy>; phy-names = "dsi"; @@ -3450,6 +3507,8 @@ "core", "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; phys = <_phy>; phy-names = "dsi"; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/msm/dpu: Use OPP API to set clk/perf state
On some qualcomm platforms DPU needs to express a performance state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Reviewed-by: Rob Clark Reviewed-by: Matthias Kaehlcke --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 26 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 7c230f7..b36919d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -218,7 +219,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index b8615d4..0bc8ec4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1025,11 +1026,23 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dpu_kms->opp_table = dev_pm_opp_set_clkname(dev, "core"); + if (IS_ERR(dpu_kms->opp_table)) + return PTR_ERR(dpu_kms->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (!ret) { + dpu_kms->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(dev, "invalid OPP table in device tree\n"); + return ret; + } + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { DPU_ERROR("failed to parse clocks, ret=%d\n", ret); - return ret; + goto err; } platform_set_drvdata(pdev, dpu_kms); @@ -1043,6 +1056,11 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = _kms->base; return ret; +err: + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); + return ret; } static void dpu_unbind(struct device *dev, struct device *master, void *data) @@ -1057,6 +1075,10 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) if (dpu_kms->rpm_enabled) pm_runtime_disable(>dev); + + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); } static const struct component_ops dpu_ops = { @@ -1082,6 +1104,8 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index a3b122b..7400cd7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -128,6 +128,10 @@ struct dpu_kms { struct platform_device *pdev; bool rpm_enabled; + + struct opp_table *opp_table; + bool has_opp_table; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 3/6] drm/msm/dpu: Use OPP API to set clk/perf state
On some qualcomm platforms DPU needs to express a performance state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Reviewed-by: Rob Clark Reviewed-by: Matthias Kaehlcke Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- No functional change in v6, rebased over 5.8-rc1 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 26 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 7c230f7..b36919d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -218,7 +219,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index b8615d4..0bc8ec4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1025,11 +1026,23 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dpu_kms->opp_table = dev_pm_opp_set_clkname(dev, "core"); + if (IS_ERR(dpu_kms->opp_table)) + return PTR_ERR(dpu_kms->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (!ret) { + dpu_kms->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(dev, "invalid OPP table in device tree\n"); + return ret; + } + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { DPU_ERROR("failed to parse clocks, ret=%d\n", ret); - return ret; + goto err; } platform_set_drvdata(pdev, dpu_kms); @@ -1043,6 +1056,11 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = _kms->base; return ret; +err: + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); + return ret; } static void dpu_unbind(struct device *dev, struct device *master, void *data) @@ -1057,6 +1075,10 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) if (dpu_kms->rpm_enabled) pm_runtime_disable(>dev); + + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); } static const struct component_ops dpu_ops = { @@ -1082,6 +1104,8 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index a3b122b..7400cd7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -128,6 +128,10 @@ struct dpu_kms { struct platform_device *pdev; bool rpm_enabled; + + struct opp_table *opp_table; + bool has_opp_table; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 4/6] drm/msm: dsi: Use OPP API to set clk/perf state
On SDM845 DSI needs to express a perforamnce state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- No functional change in v6, rebased over 5.8-rc1 drivers/gpu/drm/msm/dsi/dsi.h | 2 ++ drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 +-- drivers/gpu/drm/msm/dsi/dsi_host.c | 58 ++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 4de771d..ba7583c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -180,10 +180,12 @@ int msm_dsi_runtime_suspend(struct device *dev); int msm_dsi_runtime_resume(struct device *dev); int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_set_rate_v2(struct msm_dsi_host *msm_host); +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host); +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host); int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size); int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size); void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index 813d69d..773c4fe 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -210,9 +210,9 @@ static const struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = { }; static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = { - .link_clk_set_rate = dsi_link_clk_set_rate_6g, + .link_clk_set_rate = dsi_link_clk_set_rate_6g_v2, .link_clk_enable = dsi_link_clk_enable_6g, - .link_clk_disable = dsi_link_clk_disable_6g, + .link_clk_disable = dsi_link_clk_disable_6g_v2, .clk_init_ver = dsi_clk_init_6g_v2, .tx_buf_alloc = dsi_tx_buf_alloc_6g, .tx_buf_get = dsi_tx_buf_get_6g, diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..890531c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,9 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp_table; + bool has_opp_table; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -537,6 +541,38 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) return 0; } +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host) +{ + int ret; + struct device *dev = _host->pdev->dev; + + DBG("Set clk rates: pclk=%d, byteclk=%d", + msm_host->mode->clock, msm_host->byte_clk_rate); + + ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate); + if (ret) { + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); + return ret; + } + + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); + if (ret) { + pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); + return ret; + } + + if (msm_host->byte_intf_clk) { + ret = clk_set_rate(msm_host->byte_intf_clk, + msm_host->byte_clk_rate / 2); + if (ret) { + pr_err("%s: Failed to set rate byte intf clk, %d\n", + __func__, ret); + return ret; + } + } + + return 0; +} int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) { @@ -665,6 +701,13 @@ void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host) clk_disable_unprepare(msm_host->byte_clk); } +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host) +{ + /* Drop the performance state vote */ + dev_pm_opp_set_rate(_host->pdev->dev, 0); + dsi_link_clk_disable_6g(msm_host); +} + void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) { clk_disable_unprepare(msm_host->pixel_clk); @@ -1879,6 +1922,18 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } + msm_host->opp_table = dev_pm_opp_set_clkname(>dev, "byte"); + if (IS_ERR(msm_host->opp_table)) + return PTR_ERR(msm_host->opp_table); + /* OPP tab
[PATCH v5 4/6] drm/msm: dsi: Use OPP API to set clk/perf state
On SDM845 DSI needs to express a perforamnce state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/dsi/dsi.h | 2 ++ drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 +-- drivers/gpu/drm/msm/dsi/dsi_host.c | 58 ++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 4de771d..ba7583c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -180,10 +180,12 @@ int msm_dsi_runtime_suspend(struct device *dev); int msm_dsi_runtime_resume(struct device *dev); int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_set_rate_v2(struct msm_dsi_host *msm_host); +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host); +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host); int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size); int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size); void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index 813d69d..773c4fe 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -210,9 +210,9 @@ static const struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = { }; static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = { - .link_clk_set_rate = dsi_link_clk_set_rate_6g, + .link_clk_set_rate = dsi_link_clk_set_rate_6g_v2, .link_clk_enable = dsi_link_clk_enable_6g, - .link_clk_disable = dsi_link_clk_disable_6g, + .link_clk_disable = dsi_link_clk_disable_6g_v2, .clk_init_ver = dsi_clk_init_6g_v2, .tx_buf_alloc = dsi_tx_buf_alloc_6g, .tx_buf_get = dsi_tx_buf_get_6g, diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..890531c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,9 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp_table; + bool has_opp_table; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -537,6 +541,38 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) return 0; } +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host) +{ + int ret; + struct device *dev = _host->pdev->dev; + + DBG("Set clk rates: pclk=%d, byteclk=%d", + msm_host->mode->clock, msm_host->byte_clk_rate); + + ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate); + if (ret) { + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); + return ret; + } + + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); + if (ret) { + pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); + return ret; + } + + if (msm_host->byte_intf_clk) { + ret = clk_set_rate(msm_host->byte_intf_clk, + msm_host->byte_clk_rate / 2); + if (ret) { + pr_err("%s: Failed to set rate byte intf clk, %d\n", + __func__, ret); + return ret; + } + } + + return 0; +} int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) { @@ -665,6 +701,13 @@ void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host) clk_disable_unprepare(msm_host->byte_clk); } +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host) +{ + /* Drop the performance state vote */ + dev_pm_opp_set_rate(_host->pdev->dev, 0); + dsi_link_clk_disable_6g(msm_host); +} + void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) { clk_disable_unprepare(msm_host->pixel_clk); @@ -1879,6 +1922,18 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } + msm_host->opp_table = dev_pm_opp_set_clkname(>dev, "byte"); + if (IS_ERR(msm_host->opp_table)) + return PTR_ERR(msm_host->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(&
[PATCH v5 3/6] drm/msm/dpu: Use OPP API to set clk/perf state
On some qualcomm platforms DPU needs to express a performance state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Reviewed-by: Rob Clark Reviewed-by: Matthias Kaehlcke Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- Change in v5: left msm_dss_clk_set_rate() as-is since its used by the DP driver under review on the lists. drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 26 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 11f2beb..fe5717df 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -239,7 +240,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index ce19f1d..d3a46b0 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1033,11 +1034,23 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dpu_kms->opp_table = dev_pm_opp_set_clkname(dev, "core"); + if (IS_ERR(dpu_kms->opp_table)) + return PTR_ERR(dpu_kms->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (!ret) { + dpu_kms->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(dev, "invalid OPP table in device tree\n"); + return ret; + } + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { DPU_ERROR("failed to parse clocks, ret=%d\n", ret); - return ret; + goto err; } platform_set_drvdata(pdev, dpu_kms); @@ -1051,6 +1064,11 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = _kms->base; return ret; +err: + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); + return ret; } static void dpu_unbind(struct device *dev, struct device *master, void *data) @@ -1065,6 +1083,10 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) if (dpu_kms->rpm_enabled) pm_runtime_disable(>dev); + + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); } static const struct component_ops dpu_ops = { @@ -1090,6 +1112,8 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 211f5de9..2a52e4e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -128,6 +128,10 @@ struct dpu_kms { struct platform_device *pdev; bool rpm_enabled; + + struct opp_table *opp_table; + bool has_opp_table; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 4/6] drm/msm: dsi: Use OPP API to set clk/perf state
On SDM845 DSI needs to express a perforamnce state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/dsi/dsi.h | 2 ++ drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 +-- drivers/gpu/drm/msm/dsi/dsi_host.c | 58 ++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 4de771d..ba7583c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -180,10 +180,12 @@ int msm_dsi_runtime_suspend(struct device *dev); int msm_dsi_runtime_resume(struct device *dev); int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_set_rate_v2(struct msm_dsi_host *msm_host); +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host); +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host); int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size); int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size); void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index 813d69d..773c4fe 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -210,9 +210,9 @@ static const struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = { }; static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = { - .link_clk_set_rate = dsi_link_clk_set_rate_6g, + .link_clk_set_rate = dsi_link_clk_set_rate_6g_v2, .link_clk_enable = dsi_link_clk_enable_6g, - .link_clk_disable = dsi_link_clk_disable_6g, + .link_clk_disable = dsi_link_clk_disable_6g_v2, .clk_init_ver = dsi_clk_init_6g_v2, .tx_buf_alloc = dsi_tx_buf_alloc_6g, .tx_buf_get = dsi_tx_buf_get_6g, diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..d5f3dcd 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,9 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp_table; + bool has_opp_table; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -537,6 +541,38 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) return 0; } +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host) +{ + int ret; + struct device *dev = _host->pdev->dev; + + DBG("Set clk rates: pclk=%d, byteclk=%d", + msm_host->mode->clock, msm_host->byte_clk_rate); + + ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate); + if (ret) { + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); + return ret; + } + + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); + if (ret) { + pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); + return ret; + } + + if (msm_host->byte_intf_clk) { + ret = clk_set_rate(msm_host->byte_intf_clk, + msm_host->byte_clk_rate / 2); + if (ret) { + pr_err("%s: Failed to set rate byte intf clk, %d\n", + __func__, ret); + return ret; + } + } + + return 0; +} int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) { @@ -665,6 +701,13 @@ void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host) clk_disable_unprepare(msm_host->byte_clk); } +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host) +{ + /* Drop the performance state vote */ + dev_pm_opp_set_rate(_host->pdev->dev, 0); + dsi_link_clk_disable_6g(msm_host); +} + void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) { clk_disable_unprepare(msm_host->pixel_clk); @@ -1879,6 +1922,18 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } + msm_host->opp_table = dev_pm_opp_set_clkname(>dev, "byte"); + if (IS_ERR(msm_host->opp_table)) + return PTR_ERR(msm_host->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(&
[PATCH v4 3/6] drm/msm/dpu: Use OPP API to set clk/perf state
On some qualcomm platforms DPU needs to express a performance state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Reviewed-by: Rob Clark Reviewed-by: Matthias Kaehlcke Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c | 33 --- drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 25 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 5 files changed, 30 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 11f2beb..fe5717df 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -239,7 +240,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c index 078afc5..b9a4bf8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c @@ -51,39 +51,6 @@ int msm_dss_get_clk(struct device *dev, struct dss_clk *clk_arry, int num_clk) return rc; } -int msm_dss_clk_set_rate(struct dss_clk *clk_arry, int num_clk) -{ - int i, rc = 0; - - for (i = 0; i < num_clk; i++) { - if (clk_arry[i].clk) { - if (clk_arry[i].type != DSS_CLK_AHB) { - DEV_DBG("%pS->%s: '%s' rate %ld\n", - __builtin_return_address(0), __func__, - clk_arry[i].clk_name, - clk_arry[i].rate); - rc = clk_set_rate(clk_arry[i].clk, - clk_arry[i].rate); - if (rc) { - DEV_ERR("%pS->%s: %s failed. rc=%d\n", - __builtin_return_address(0), - __func__, - clk_arry[i].clk_name, rc); - break; - } - } - } else { - DEV_ERR("%pS->%s: '%s' is not available\n", - __builtin_return_address(0), __func__, - clk_arry[i].clk_name); - rc = -EPERM; - break; - } - } - - return rc; -} - int msm_dss_enable_clk(struct dss_clk *clk_arry, int num_clk, int enable) { int i, rc = 0; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h index e6b5c77..ca25c90 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h @@ -33,7 +33,6 @@ struct dss_module_power { int msm_dss_get_clk(struct device *dev, struct dss_clk *clk_arry, int num_clk); void msm_dss_put_clk(struct dss_clk *clk_arry, int num_clk); -int msm_dss_clk_set_rate(struct dss_clk *clk_arry, int num_clk); int msm_dss_enable_clk(struct dss_clk *clk_arry, int num_clk, int enable); int msm_dss_parse_clock(struct platform_device *pdev, struct dss_module_power *mp); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index ce19f1d..f16d715 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1033,11 +1034,23 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dpu_kms->opp_table = dev_pm_opp_set_clkname(dev, "core"); + if (IS_ERR(dpu_kms->opp_table)) + return PTR_ERR(dpu_kms->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (!ret) { + dpu_kms->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(dev, "invalid OPP table in device tree\n"); + return ret; +
Re: [PATCH v3 05/17] drm/msm/dpu: Use OPP API to set clk/perf state
On 4/29/2020 5:44 AM, Matthias Kaehlcke wrote: On Tue, Apr 28, 2020 at 07:02:53PM +0530, Rajendra Nayak wrote: On some qualcomm platforms DPU needs to express a perforamnce state requirement on a power domain depennding on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 25 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 11f2beb..fe5717df 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -239,7 +240,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index ce19f1d..2f53bbf 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1033,11 +1034,23 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dpu_kms->opp_table = dev_pm_opp_set_clkname(dev, "core"); + if (IS_ERR(dpu_kms->opp_table)) + return PTR_ERR(dpu_kms->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (!ret) { + dpu_kms->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(dev, "Invalid OPP table in Device tree\n"); nit: s/Device/device/ ? uber-nit: s/Invalid/invalid/ most log messages in this file start with a lower case letter, except for acronyms/register names please also change it in the other drivers unless you disagree. Sure, will do. Thanks. + return ret; + } + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { DPU_ERROR("failed to parse clocks, ret=%d\n", ret); - return ret; + goto err; } platform_set_drvdata(pdev, dpu_kms); @@ -1051,6 +1064,11 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = _kms->base; return ret; +err: + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); + return ret; } static void dpu_unbind(struct device *dev, struct device *master, void *data) @@ -1059,6 +1077,9 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(>dev, mp->clk_config); mp->num_clk = 0; @@ -1090,6 +,8 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 211f5de9..2a52e4e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -128,6 +128,10 @@ struct dpu_kms { struct platform_device *pdev; bool rpm_enabled; + + struct opp_table *opp_table; + bool has_opp_table; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can Reviewed-by: Matthias Kaehlcke -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 05/17] drm/msm/dpu: Use OPP API to set clk/perf state
On 4/28/2020 10:02 PM, Rob Clark wrote: On Tue, Apr 28, 2020 at 6:39 AM Rajendra Nayak wrote: On some qualcomm platforms DPU needs to express a perforamnce state s/perforamnce/performance/ requirement on a power domain depennding on the clock rates. s/depennding/depending/ Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 25 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 11f2beb..fe5717df 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -239,7 +240,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); I think this leaves msm_dss_clk_set_rate() unused now? yup, I didn't realise, I will get rid of it when I respin. Other than that, Reviewed-by: Rob Clark Thanks. } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index ce19f1d..2f53bbf 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1033,11 +1034,23 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dpu_kms->opp_table = dev_pm_opp_set_clkname(dev, "core"); + if (IS_ERR(dpu_kms->opp_table)) + return PTR_ERR(dpu_kms->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (!ret) { + dpu_kms->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(dev, "Invalid OPP table in Device tree\n"); + return ret; + } + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { DPU_ERROR("failed to parse clocks, ret=%d\n", ret); - return ret; + goto err; } platform_set_drvdata(pdev, dpu_kms); @@ -1051,6 +1064,11 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = _kms->base; return ret; +err: + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); + return ret; } static void dpu_unbind(struct device *dev, struct device *master, void *data) @@ -1059,6 +1077,9 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(>dev, mp->clk_config); mp->num_clk = 0; @@ -1090,6 +,8 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 211f5de9..2a52e4e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -128,6 +128,10 @@ struct dpu_kms { struct platform_device *pdev; bool rpm_enabled; + + struct opp_table *opp_table; + bool has_opp_table; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foun
[PATCH v3 06/17] drm/msm: dsi: Use OPP API to set clk/perf state
On SDM845 DSI needs to express a perforamnce state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/dsi/dsi.h | 2 ++ drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 +-- drivers/gpu/drm/msm/dsi/dsi_host.c | 58 ++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 4de771d..ba7583c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -180,10 +180,12 @@ int msm_dsi_runtime_suspend(struct device *dev); int msm_dsi_runtime_resume(struct device *dev); int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_set_rate_v2(struct msm_dsi_host *msm_host); +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host); +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host); int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size); int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size); void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index 813d69d..773c4fe 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -210,9 +210,9 @@ static const struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = { }; static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = { - .link_clk_set_rate = dsi_link_clk_set_rate_6g, + .link_clk_set_rate = dsi_link_clk_set_rate_6g_v2, .link_clk_enable = dsi_link_clk_enable_6g, - .link_clk_disable = dsi_link_clk_disable_6g, + .link_clk_disable = dsi_link_clk_disable_6g_v2, .clk_init_ver = dsi_clk_init_6g_v2, .tx_buf_alloc = dsi_tx_buf_alloc_6g, .tx_buf_get = dsi_tx_buf_get_6g, diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..3844fdc 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,9 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp_table; + bool has_opp_table; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -537,6 +541,38 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) return 0; } +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host) +{ + int ret; + struct device *dev = _host->pdev->dev; + + DBG("Set clk rates: pclk=%d, byteclk=%d", + msm_host->mode->clock, msm_host->byte_clk_rate); + + ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate); + if (ret) { + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); + return ret; + } + + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); + if (ret) { + pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); + return ret; + } + + if (msm_host->byte_intf_clk) { + ret = clk_set_rate(msm_host->byte_intf_clk, + msm_host->byte_clk_rate / 2); + if (ret) { + pr_err("%s: Failed to set rate byte intf clk, %d\n", + __func__, ret); + return ret; + } + } + + return 0; +} int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) { @@ -665,6 +701,13 @@ void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host) clk_disable_unprepare(msm_host->byte_clk); } +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host) +{ + /* Drop the performance state vote */ + dev_pm_opp_set_rate(_host->pdev->dev, 0); + dsi_link_clk_disable_6g(msm_host); +} + void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) { clk_disable_unprepare(msm_host->pixel_clk); @@ -1879,6 +1922,18 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } + msm_host->opp_table = dev_pm_opp_set_clkname(>dev, "byte"); + if (IS_ERR(msm_host->opp_table)) + return PTR_ERR(msm_host->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(&
[PATCH v3 05/17] drm/msm/dpu: Use OPP API to set clk/perf state
On some qualcomm platforms DPU needs to express a perforamnce state requirement on a power domain depennding on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 25 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 11f2beb..fe5717df 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -239,7 +240,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index ce19f1d..2f53bbf 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1033,11 +1034,23 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dpu_kms->opp_table = dev_pm_opp_set_clkname(dev, "core"); + if (IS_ERR(dpu_kms->opp_table)) + return PTR_ERR(dpu_kms->opp_table); + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (!ret) { + dpu_kms->has_opp_table = true; + } else if (ret != -ENODEV) { + dev_err(dev, "Invalid OPP table in Device tree\n"); + return ret; + } + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { DPU_ERROR("failed to parse clocks, ret=%d\n", ret); - return ret; + goto err; } platform_set_drvdata(pdev, dpu_kms); @@ -1051,6 +1064,11 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = _kms->base; return ret; +err: + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); + return ret; } static void dpu_unbind(struct device *dev, struct device *master, void *data) @@ -1059,6 +1077,9 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + if (dpu_kms->has_opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp_table); msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(>dev, mp->clk_config); mp->num_clk = 0; @@ -1090,6 +,8 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 211f5de9..2a52e4e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -128,6 +128,10 @@ struct dpu_kms { struct platform_device *pdev; bool rpm_enabled; + + struct opp_table *opp_table; + bool has_opp_table; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 05/17] drm/msm/dpu: Use OPP API to set clk/perf state
On 4/17/2020 11:47 PM, Matthias Kaehlcke wrote: Hi Rajendra, I have essentially the same comments as for "tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state" (https://patchwork.kernel.org/patch/11495209/). about error handling of 'dev_pm_opp_of_add_table' and misleading struct member names 'opp'/'opp_table'. Please apply the requested changes to the entire series unless you disagree (we can keep the discussion in the patch referenced above). Thanks, yes, I will apply those changes across the series and respin. Will wait a few days to see I get any more feedback. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 05/17] drm/msm/dpu: Use OPP API to set clk/perf state
On some qualcomm platforms DPU needs to express a perforamnce state requirement on a power domain depennding on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 20 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 11f2beb..fe5717df 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -239,7 +240,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index ce19f1d..cfce642 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1033,11 +1034,18 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dpu_kms->opp = dev_pm_opp_set_clkname(dev, "core"); + if (IS_ERR(dpu_kms->opp)) + return PTR_ERR(dpu_kms->opp); + /* OPP table is optional */ + if (!dev_pm_opp_of_add_table(dev)) + dpu_kms->opp_table = true; + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { DPU_ERROR("failed to parse clocks, ret=%d\n", ret); - return ret; + goto err; } platform_set_drvdata(pdev, dpu_kms); @@ -1051,6 +1059,11 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = _kms->base; return ret; +err: + if (dpu_kms->opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp); + return ret; } static void dpu_unbind(struct device *dev, struct device *master, void *data) @@ -1059,6 +1072,9 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + if (dpu_kms->opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(dpu_kms->opp); msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(>dev, mp->clk_config); mp->num_clk = 0; @@ -1090,6 +1106,8 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 211f5de9..0060709 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -128,6 +128,10 @@ struct dpu_kms { struct platform_device *pdev; bool rpm_enabled; + + struct opp_table *opp; + bool opp_table; + struct dss_module_power mp; /* reference count bandwidth requests, so we know when we can -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 06/17] drm/msm: dsi: Use OPP API to set clk/perf state
On SDM845 DSI needs to express a perforamnce state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/dsi/dsi.h | 2 ++ drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 +-- drivers/gpu/drm/msm/dsi/dsi_host.c | 53 ++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 4de771d..ba7583c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -180,10 +180,12 @@ int msm_dsi_runtime_suspend(struct device *dev); int msm_dsi_runtime_resume(struct device *dev); int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_set_rate_v2(struct msm_dsi_host *msm_host); +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host); +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host); int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size); int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size); void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index 813d69d..773c4fe 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -210,9 +210,9 @@ static const struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = { }; static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = { - .link_clk_set_rate = dsi_link_clk_set_rate_6g, + .link_clk_set_rate = dsi_link_clk_set_rate_6g_v2, .link_clk_enable = dsi_link_clk_enable_6g, - .link_clk_disable = dsi_link_clk_disable_6g, + .link_clk_disable = dsi_link_clk_disable_6g_v2, .clk_init_ver = dsi_clk_init_6g_v2, .tx_buf_alloc = dsi_tx_buf_alloc_6g, .tx_buf_get = dsi_tx_buf_get_6g, diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..d532fab 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,9 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp; + bool opp_table; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -537,6 +541,38 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) return 0; } +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host) +{ + int ret; + struct device *dev = _host->pdev->dev; + + DBG("Set clk rates: pclk=%d, byteclk=%d", + msm_host->mode->clock, msm_host->byte_clk_rate); + + ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate); + if (ret) { + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); + return ret; + } + + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); + if (ret) { + pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); + return ret; + } + + if (msm_host->byte_intf_clk) { + ret = clk_set_rate(msm_host->byte_intf_clk, + msm_host->byte_clk_rate / 2); + if (ret) { + pr_err("%s: Failed to set rate byte intf clk, %d\n", + __func__, ret); + return ret; + } + } + + return 0; +} int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) { @@ -665,6 +701,13 @@ void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host) clk_disable_unprepare(msm_host->byte_clk); } +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host) +{ + /* Drop the performance state vote */ + dev_pm_opp_set_rate(_host->pdev->dev, 0); + dsi_link_clk_disable_6g(msm_host); +} + void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) { clk_disable_unprepare(msm_host->pixel_clk); @@ -1879,6 +1922,13 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } + msm_host->opp = dev_pm_opp_set_clkname(>dev, "byte"); + if (IS_ERR(msm_host->opp)) + return PTR_ERR(msm_host->opp); + /* OPP table is optional */ + if (!dev_pm_opp_of_add_table(>dev)) + msm
[PATCH 10/21] drm/msm: dsi: Use OPP API to set clk/perf state
On SDM845 DSI needs to express a perforamnce state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/dsi/dsi.h | 2 ++ drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 ++-- drivers/gpu/drm/msm/dsi/dsi_host.c | 48 ++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 4de771d..ba7583c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -180,10 +180,12 @@ int msm_dsi_runtime_suspend(struct device *dev); int msm_dsi_runtime_resume(struct device *dev); int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_set_rate_v2(struct msm_dsi_host *msm_host); +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host); +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host); int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size); int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size); void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index 813d69d..773c4fe 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -210,9 +210,9 @@ static const struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = { }; static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = { - .link_clk_set_rate = dsi_link_clk_set_rate_6g, + .link_clk_set_rate = dsi_link_clk_set_rate_6g_v2, .link_clk_enable = dsi_link_clk_enable_6g, - .link_clk_disable = dsi_link_clk_disable_6g, + .link_clk_disable = dsi_link_clk_disable_6g_v2, .clk_init_ver = dsi_clk_init_6g_v2, .tx_buf_alloc = dsi_tx_buf_alloc_6g, .tx_buf_get = dsi_tx_buf_get_6g, diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 11ae5b8..c47d9af 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,8 @@ struct msm_dsi_host { struct clk *pixel_clk_src; struct clk *byte_intf_clk; + struct opp_table *opp; + u32 byte_clk_rate; u32 pixel_clk_rate; u32 esc_clk_rate; @@ -537,6 +540,40 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) return 0; } +int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host) +{ + int ret; + struct device *dev = _host->pdev->dev; + + DBG("Set clk rates: pclk=%d, byteclk=%d", + msm_host->mode->clock, msm_host->byte_clk_rate); + + ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate); + if (ret) { + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); + return ret; + } + + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); + if (ret) { + pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); + return ret; + } + + if (msm_host->byte_intf_clk) { + ret = clk_set_rate(msm_host->byte_intf_clk, + msm_host->byte_clk_rate / 2); + if (ret) { + pr_err("%s: Failed to set rate byte intf clk, %d\n", + __func__, ret); + return ret; + } + } + + return 0; +} + + int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) { @@ -665,6 +702,13 @@ void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host) clk_disable_unprepare(msm_host->byte_clk); } +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host) +{ + /* Drop the performance state vote */ + dev_pm_opp_set_rate(_host->pdev->dev, 0); + dsi_link_clk_disable_6g(msm_host); +} + void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host) { clk_disable_unprepare(msm_host->pixel_clk); @@ -1879,6 +1923,9 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) goto fail; } + msm_host->opp = dev_pm_opp_set_clkname(>dev, "byte"); + dev_pm_opp_of_add_table(>dev); + init_completion(_host->dma_comp); init_completion(_host->video_comp); mutex_init(_host->dev_mutex); @@ -1904,6 +1951,7 @@ v
[PATCH 09/21] drm/msm/dpu: Use OPP API to set clk/perf state
On some qualcomm platforms DPU needs to express a perforamnce state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Cc: Rob Clark Cc: Sean Paul Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 11f2beb..fe5717df 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -239,7 +240,7 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index ce19f1d..949157a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -1033,6 +1034,9 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dev_pm_opp_set_clkname(dev, "core"); + dev_pm_opp_of_add_table(dev); + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { @@ -1059,6 +1063,7 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + dev_pm_opp_of_remove_table(dev); msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(>dev, mp->clk_config); mp->num_clk = 0; @@ -1090,6 +1095,7 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
On 6/17/2019 9:47 AM, Viresh Kumar wrote: On 17-06-19, 09:37, Rajendra Nayak wrote: On 6/17/2019 9:20 AM, Viresh Kumar wrote: On 14-06-19, 10:57, Viresh Kumar wrote: Hmm, so this patch won't break anything and I am inclined to apply it again :) Does anyone see any other issues with it, which I might be missing ? I have updated the commit log a bit more to clarify on things, please let me know if it looks okay. opp: Don't overwrite rounded clk rate The OPP table normally contains 'fmax' values corresponding to the voltage or performance levels of each OPP, but we don't necessarily want all the devices to run at fmax all the time. Running at fmax makes sense for devices like CPU/GPU, which have a finite amount of work to do and since a specific amount of energy is consumed at an OPP, its better to run at the highest possible frequency for that voltage value. On the other hand, we have IO devices which need to run at specific frequencies only for their proper functioning, instead of maximum possible frequency. The OPP core currently roundup to the next possible OPP for a frequency and select the fmax value. To support the IO devices by the OPP core, lets do the roundup to fetch the voltage or performance state values, but not use the OPP frequency value. Rather use the value returned by clk_round_rate(). The current user, cpufreq, of dev_pm_opp_set_rate() already does the rounding to the next OPP before calling this routine and it won't have any side affects because of this change. Looks good to me. Should this also be documented someplace that dev_pm_opp_set_rate() would not be able to distinguish between its users trying to scale CPU/GPU's vs IO devices, so its the callers responsibility to round it accordingly before calling the API? diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 0fbc77f05048..bae94bfa1e96 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -751,8 +751,11 @@ static int _set_required_opps(struct device *dev, * @dev:device for which we do this operation * @target_freq: frequency to achieve * - * This configures the power-supplies and clock source to the levels specified - * by the OPP corresponding to the target_freq. + * This configures the power-supplies to the levels specified by the OPP + * corresponding to the target_freq, and programs the clock to a value <= + * target_freq, as rounded by clk_round_rate(). Device wanting to run at fmax + * provided by the opp, should have already rounded to the target OPP's + * frequency. */ Perfect, thanks. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
On 6/17/2019 9:20 AM, Viresh Kumar wrote: On 14-06-19, 10:57, Viresh Kumar wrote: Hmm, so this patch won't break anything and I am inclined to apply it again :) Does anyone see any other issues with it, which I might be missing ? I have updated the commit log a bit more to clarify on things, please let me know if it looks okay. opp: Don't overwrite rounded clk rate The OPP table normally contains 'fmax' values corresponding to the voltage or performance levels of each OPP, but we don't necessarily want all the devices to run at fmax all the time. Running at fmax makes sense for devices like CPU/GPU, which have a finite amount of work to do and since a specific amount of energy is consumed at an OPP, its better to run at the highest possible frequency for that voltage value. On the other hand, we have IO devices which need to run at specific frequencies only for their proper functioning, instead of maximum possible frequency. The OPP core currently roundup to the next possible OPP for a frequency and select the fmax value. To support the IO devices by the OPP core, lets do the roundup to fetch the voltage or performance state values, but not use the OPP frequency value. Rather use the value returned by clk_round_rate(). The current user, cpufreq, of dev_pm_opp_set_rate() already does the rounding to the next OPP before calling this routine and it won't have any side affects because of this change. Looks good to me. Should this also be documented someplace that dev_pm_opp_set_rate() would not be able to distinguish between its users trying to scale CPU/GPU's vs IO devices, so its the callers responsibility to round it accordingly before calling the API? Signed-off-by: Stephen Boyd Signed-off-by: Rajendra Nayak [ Viresh: Massaged changelog and use temp_opp variable instead ] Signed-off-by: Viresh Kumar -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid
On 6/14/2019 12:02 PM, Viresh Kumar wrote: On 20-03-19, 15:19, Rajendra Nayak wrote: For devices with performance state, we use dev_pm_opp_set_rate() to set the appropriate clk rate and the performance state. We do need a way to *remove* the performance state vote when we idle the device and turn the clocks off. Use dev_pm_opp_set_rate() with freq=0 to achieve this. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- drivers/opp/core.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) What about this instead ? yes, this works, thanks for updating the patch. diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 2fe96c2363a3..9accf8bb6afc 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -711,7 +711,7 @@ static int _set_required_opps(struct device *dev, /* Single genpd case */ if (!genpd_virt_devs) { - pstate = opp->required_opps[0]->pstate; + pstate = likely(opp) ? opp->required_opps[0]->pstate : 0; ret = dev_pm_genpd_set_performance_state(dev, pstate); if (ret) { dev_err(dev, "Failed to set performance state of %s: %d (%d)\n", @@ -729,7 +729,7 @@ static int _set_required_opps(struct device *dev, mutex_lock(_table->genpd_virt_dev_lock); for (i = 0; i < opp_table->required_opp_count; i++) { - pstate = opp->required_opps[i]->pstate; + pstate = likely(opp) ? opp->required_opps[i]->pstate : 0; if (!genpd_virt_devs[i]) continue; @@ -770,14 +770,13 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) if (unlikely(!target_freq)) { if (opp_table->required_opp_tables) { - /* drop the performance state vote */ - dev_pm_genpd_set_performance_state(dev, 0); - return 0; + ret = _set_required_opps(dev, opp_table, NULL); } else { - dev_err(dev, "%s: Invalid target frequency %lu\n", __func__, - target_freq); - return -EINVAL; + dev_err(dev, "target frequency can't be 0\n"); + ret = -EINVAL; } + + goto put_opp_table; } clk = opp_table->clk; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
Now, the request to change the frequency starts from cpufreq governors, like schedutil when they calls: __cpufreq_driver_target(policy, 599 MHz, CPUFREQ_RELATION_L); CPUFREQ_RELATION_L means: lowest frequency at or above target. And so I would expect the frequency to get set to 600MHz (if we look at clock driver) or 700MHz (if we look at OPP table). I think we should decide this thing from the OPP table only as that's what the platform guys want us to use. So, we should end up with 700 MHz. Then we land into dev_pm_opp_set_rate(), which does this (which is code copied from earlier version of cpufreq-dt driver): so before we land into dev_pm_opp_set_rate() from a __cpufreq_driver_target() I guess we do have a cpufreq driver callback that gets called in between? which is either .target_index or .target In case of .target_index, the cpufreq core looks for a OPP index and we would land up with 700Mhz i guess, so we are good. In case of .target though the 'relation' CPUFREQ_RELATION_L does get passed over to the cpufreq driver which I am guessing is expected to handle it in some way to make sure the target frequency set is not less than whats requested? instead of simply passing the requested frequency over to dev_pm_opp_set_rate()? Looking at all the existing cpufreq drivers upstream, while most support .target_index the 3 which do support .target seem to completely ignore this 'relation' input that's passed to them. drivers/cpufreq/cppc_cpufreq.c: .target = cppc_cpufreq_set_target, drivers/cpufreq/cpufreq-nforce2.c: .target = nforce2_target, drivers/cpufreq/pcc-cpufreq.c: .target = pcc_cpufreq_target, This kind of behavior (introduced by this patch) is important for other devices which want to run at the nearest frequency to target one, but not for CPUs/GPUs. So, we need to tag these IO devices separately, maybe from DT ? So we select the closest match instead of most optimal one. yes we do need some way to distinguish between CPU/GPU devices and other IO devices. CPU/GPU can always run at fmax for a given voltage, that's not true for IO devices and I don't see how we can satisfy both cases without clearly knowing if we are serving a processor or an IO device, unless the higher layers (cpufreq/devfreq) are able to handle this somehow without expecting the OPP layer to handle the differences. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
On 6/11/2019 4:24 PM, Viresh Kumar wrote: On 20-03-19, 15:19, Rajendra Nayak wrote: From: Stephen Boyd Doing this allows us to call this API with any rate requested and have it not need to match in the OPP table. Instead, we'll round the rate up to the nearest OPP that we see so that we can get the voltage or level that's required for that OPP. This supports users of OPP that want to specify the 'fmax' tables of a device instead of every single frequency that they need. And for devices that required the exact frequency, we can rely on the clk framework to round the rate to the nearest supported frequency instead of the OPP framework to do so. Note that this may affect drivers that don't want the clk framework to do rounding, but instead want the OPP table to do the rounding for them. Do we have that case? Should we add some flag to the OPP table to indicate this and then not have that flag set when there isn't an OPP table for the device and also introduce a property like 'opp-use-clk' to tell the table that it should use the clk APIs to round rates instead of OPP? Signed-off-by: Stephen Boyd Signed-off-by: Rajendra Nayak --- []... I see a logical problem with this patch. Suppose the clock driver supports following frequencies: 500M, 800M, 1G, 1.2G and the OPP table contains following list: 500M, 1G, 1.2G (i.e. missing 800M). Now 800M should never get programmed as it isn't part of the OPP table. But if you pass 600M to opp-set-rate, then it will end up selecting 800M as clock driver will round up to the closest value. correct Even if no one is doing this right now, it is a sensible usecase, specially during testing of patches and I don't think we should avoid it. What exactly is the use case for which we need this patch ? Like the changelog says 'This supports users of OPP that want to specify the 'fmax' tables of a device instead of every single frequency that they need' so the 'fmax' tables basically say what the max frequency the device can operate at for a given performance state/voltage level. so in your example it would be for instance 500M, Perf state = 2 1G, Perf state = 3 1.2G, Perf state = 4 Now when the device wants to operate at say 800Mhz, you need to set the Perf state to 3, so this patch basically avoids you having to put those additional OPPs in the table which would otherwise look something like this 500M, Perf state = 2 800M, Perf state = 3 <-- redundant OPP 1G, Perf state = 3 1.2G, Perf state = 4 Your example had just 1 missing entry in the 'fmax' tables in reality its a lot more, atleast on all qualcomm platforms. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 00/11] DVFS in the OPP core
On 5/21/2019 11:52 AM, Viresh Kumar wrote: On 20-03-19, 15:19, Rajendra Nayak wrote: This is a v2 of the RFC posted earlier by Stephen Boyd [1] As part of v2 I still follow the same approach of dev_pm_opp_set_rate() API using clk framework to round the frequency passed and making it accept 0 as a valid frequency indicating the frequency isn't required anymore. It just has a few more drivers converted to use this approach like dsi/dpu and ufs. ufs demonstrates the case of having to handle multiple power domains, one of which is scalable. The patches are based on 5.1-rc1 and depend on some ufs fixes I posted earlier [2] and a DT patch to include the rpmpd header [3] [1] https://lkml.org/lkml/2019/1/28/2086 [2] https://lkml.org/lkml/2019/3/8/70 [3] https://lkml.org/lkml/2019/3/20/120 Hi Rajendra, I am inclined to apply/push this series for 5.3-rc1, will it be possible for you to spend some time on this at priority ? Hey Viresh, I was on vacation, just got back. I will refresh this series and address your previous feedback, I haven't received much feedback for the driver changes :/ but we can atleast review and get the OPP layer changes finalized. thanks. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 11/11] arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains
Add the OPP tables for DSI and MDP based on the perf state/clk requirements, and add the power-domains property to specify the scalable power domain. Signed-off-by: Rajendra Nayak --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 59 1 file changed, 59 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index a3af4a1757b4..675954fde391 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -1857,6 +1857,59 @@ #reset-cells = <1>; }; + mdp_opp_table: mdp-opp-table { + compatible = "operating-points-v2"; + + opp-1920 { + opp-hz = /bits/ 64 <1920>; + required-opps = <_opp_min_svs>; + }; + + opp-171428571 { + opp-hz = /bits/ 64 <171428571>; + required-opps = <_opp_low_svs>; + }; + + opp-34400 { + opp-hz = /bits/ 64 <34400>; + required-opps = <_opp_svs_l1>; + }; + + opp-43000 { + opp-hz = /bits/ 64 <43000>; + required-opps = <_opp_nom>; + }; + }; + + dsi_opp_table: dsi-opp-table { + compatible = "operating-points-v2"; + + opp-1920 { + opp-hz = /bits/ 64 <1920>; + required-opps = <_opp_min_svs>; + }; + + opp-18000 { + opp-hz = /bits/ 64 <18000>; + required-opps = <_opp_low_svs>; + }; + + opp-27500 { + opp-hz = /bits/ 64 <27500>; + required-opps = <_opp_svs>; + }; + + opp-32858 { + opp-hz = /bits/ 64 <32858>; + required-opps = <_opp_svs_l1>; + }; + + opp-35800 { + opp-hz = /bits/ 64 <35800>; + required-opps = <_opp_nom>; + }; + }; + mdss: mdss@ae0 { compatible = "qcom,sdm845-mdss"; reg = <0 0x0ae0 0 0x1000>; @@ -1901,6 +1954,8 @@ < DISP_CC_MDSS_VSYNC_CLK>; assigned-clock-rates = <3>, <1920>; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; interrupt-parent = <>; interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; @@ -1947,6 +2002,8 @@ "core", "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; phys = <_phy>; phy-names = "dsi"; @@ -2013,6 +2070,8 @@ "core", "iface", "bus"; + operating-points-v2 = <_opp_table>; + power-domains = < SDM845_CX>; phys = <_phy>; phy-names = "dsi"; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 09/11] drm/msm/dpu: Use OPP API to set clk/perf state
On some qualcomm platforms DPU needs to express a perforamnce state requirement on a power domain depennding on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 7 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 9f20f397f77d..db21a86b242b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -298,7 +299,11 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate) rate = core_clk->max_rate; core_clk->rate = rate; - return msm_dss_clk_set_rate(core_clk, 1); + + if (dev_pm_opp_get_opp_table(>pdev->dev)) + return dev_pm_opp_set_rate(>pdev->dev, core_clk->rate); + else + return msm_dss_clk_set_rate(core_clk, 1); } static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 885bf88afa3e..684bd6982aaf 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "msm_drv.h" #include "msm_mmu.h" @@ -1014,6 +1015,12 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) if (!dpu_kms) return -ENOMEM; + dev_pm_opp_set_clkname(dev, "core"); + + ret = dev_pm_opp_of_add_table(dev); + if (ret) + dev_err(dev, "failed to init OPP table: %d\n", ret); + mp = _kms->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { @@ -1040,6 +1047,7 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); struct dss_module_power *mp = _kms->mp; + dev_pm_opp_of_remove_table(dev); msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(>dev, mp->clk_config); mp->num_clk = 0; @@ -1078,6 +1086,7 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev) return rc; } + dev_pm_opp_set_rate(dev, 0); rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (rc) DPU_ERROR("clock disable failed rc:%d\n", rc); -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 10/11] drm/msm: dsi: Use OPP API to set clk/perf state
On SDM845 DSI needs to express a perforamnce state requirement on a power domain depending on the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak --- drivers/gpu/drm/msm/dsi/dsi.h | 2 + drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 88 +++--- 3 files changed, 84 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 9c6b31c2d79f..b4398a798370 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -188,8 +188,10 @@ int msm_dsi_runtime_suspend(struct device *dev); int msm_dsi_runtime_resume(struct device *dev); int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host); int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host); +int dsi_link_clk_enable_6g_v2(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host); void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host); +void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host); int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size); int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size); void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c index dcdfb1bb54f9..c18532f92e4a 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c @@ -159,8 +159,8 @@ const static struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = { }; const static struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = { - .link_clk_enable = dsi_link_clk_enable_6g, - .link_clk_disable = dsi_link_clk_disable_6g, + .link_clk_enable = dsi_link_clk_enable_6g_v2, + .link_clk_disable = dsi_link_clk_disable_6g_v2, .clk_init_ver = dsi_clk_init_6g_v2, .tx_buf_alloc = dsi_tx_buf_alloc_6g, .tx_buf_get = dsi_tx_buf_get_6g, diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 610183db1daf..6ed9e6a0520c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -511,7 +512,7 @@ int msm_dsi_runtime_resume(struct device *dev) return dsi_bus_clk_enable(msm_host); } -int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) +static int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) { int ret; @@ -521,29 +522,65 @@ int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) ret = clk_set_rate(msm_host->byte_clk, msm_host->byte_clk_rate); if (ret) { pr_err("%s: Failed to set rate byte clk, %d\n", __func__, ret); - goto error; + return ret; } ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); if (ret) { pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); - goto error; + return ret; } if (msm_host->byte_intf_clk) { ret = clk_set_rate(msm_host->byte_intf_clk, msm_host->byte_clk_rate / 2); - if (ret) { + if (ret) + pr_err("%s: Failed to set rate byte intf clk, %d\n", + __func__, ret); + } + + return ret; +} + +static int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host) +{ + int ret; + struct device *dev = _host->pdev->dev; + + DBG("Set clk rates: pclk=%d, byteclk=%d", + msm_host->mode->clock, msm_host->byte_clk_rate); + + ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate); + if (ret) { + pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret); + return ret; + } + + ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); + if (ret) { + pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); + return ret; + } + + if (msm_host->byte_intf_clk) { + ret = clk_set_rate(msm_host->byte_intf_clk, + msm_host->byte_clk_rate / 2); + if (ret) pr_err("%s: Failed to set rate byte intf clk, %d\n", __func__, ret); - goto error; - } } + return ret; +} + +static int dsi_link_clk_prepare_enable_6g(struct msm_dsi_host *msm_host) +{ + int ret; + ret = clk_prepare_enable(msm_host->esc_clk); if (ret) { pr_err("%s: Failed to enable dsi esc clk\n&
[RFC v2 07/11] scsi: ufs: Add support for specifying OPP tables in DT
Some platforms like qualcomms sdm845 SoC have a need to set a performance state of a power domain for UFS along with setting the clock rate. Add support for passing this freq/perf state tuple from DT as an OPP table. Modify the driver to read the OPP table and register with OPP layer. Signed-off-by: Rajendra Nayak --- drivers/scsi/ufs/ufshcd.c | 21 +++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index ffa9e58680b4..2b260e83874a 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -913,6 +913,16 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) if (ret) return ret; + if (hba->virt_devs) { + struct dev_pm_opp *opp; + unsigned long freq = scale_up ? INT_MAX: 0; + if (scale_up) + opp = dev_pm_opp_find_freq_floor(hba->dev, ); + else + opp = dev_pm_opp_find_freq_ceil(hba->dev, ); + dev_pm_opp_set_rate(hba->dev, dev_pm_opp_get_freq(opp)); + } + list_for_each_entry(clki, head, list) { if (!IS_ERR_OR_NULL(clki->clk)) { if (scale_up && clki->max_freq) { @@ -1318,6 +1328,7 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba) struct list_head *clk_list = >clk_list_head; struct ufs_clk_info *clki; struct devfreq *devfreq; + struct device *virt_dev; int ret; /* Skip devfreq if we don't have any clocks in the list */ @@ -1325,8 +1336,14 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba) return 0; clki = list_first_entry(clk_list, struct ufs_clk_info, list); - dev_pm_opp_add(hba->dev, clki->min_freq, 0); - dev_pm_opp_add(hba->dev, clki->max_freq, 0); + + if (dev_pm_opp_of_add_table(hba->dev)) { + dev_pm_opp_add(hba->dev, clki->min_freq, 0); + dev_pm_opp_add(hba->dev, clki->max_freq, 0); + } else { + virt_dev = hba->virt_devs[hba->num_virt_devs -1]; + dev_pm_opp_set_genpd_virt_dev(hba->dev, virt_dev, 0); + } devfreq = devfreq_add_device(hba->dev, _devfreq_profile, -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 08/11] arm64: dts: sdm845: Add ufs opps and power-domains
Add the additional power domain and the OPP table for ufs on sdm845 so the driver can set the appropriate performance state of the power domain while setting the clock rate. Signed-off-by: Rajendra Nayak --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 027ffe6e93e8..a3af4a1757b4 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -1140,6 +1140,21 @@ }; }; + ufs_opp_table: ufs-opp-table { + compatible = "operating-points-v2"; + + opp-5000 { + opp-hz = /bits/ 64 <5000>; + required-opps = <_opp_min_svs>; + }; + + opp-2 { + opp-hz = /bits/ 64 <2>; + required-opps = <_opp_nom>; + + }; + }; + ufs_mem_hc: ufshc@1d84000 { compatible = "qcom,sdm845-ufshc", "qcom,ufshc", "jedec,ufs-2.0"; @@ -1148,7 +1163,7 @@ phys = <_mem_phy_lanes>; phy-names = "ufsphy"; lanes-per-direction = <2>; - power-domains = < UFS_PHY_GDSC>; + power-domains = < UFS_PHY_GDSC>, < SDM845_CX>; iommus = <_smmu 0x100 0xf>; @@ -1170,6 +1185,9 @@ < GCC_UFS_PHY_TX_SYMBOL_0_CLK>, < GCC_UFS_PHY_RX_SYMBOL_0_CLK>, < GCC_UFS_PHY_RX_SYMBOL_1_CLK>; + + operating-points-v2 = <_opp_table>; + freq-table-hz = <5000 2>, <0 0>, -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
geni serial needs to express a perforamnce state requirement on CX depending on the frequency of the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- drivers/tty/serial/qcom_geni_serial.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 3bcec1c20219..422852911141 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -115,6 +116,7 @@ struct qcom_geni_serial_port { bool brk; unsigned int tx_remaining; + struct device *dev; }; static const struct uart_ops qcom_geni_console_pops; @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, goto out_restart_rx; uport->uartclk = clk_rate; - clk_set_rate(port->se.clk, clk_rate); + dev_pm_opp_set_rate(port->dev, clk_rate); ser_clk_cfg = SER_CLK_EN; ser_clk_cfg |= clk_div << CLK_DIV_SHFT; @@ -1198,8 +1200,10 @@ static void qcom_geni_serial_pm(struct uart_port *uport, if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF) geni_se_resources_on(>se); else if (new_state == UART_PM_STATE_OFF && - old_state == UART_PM_STATE_ON) + old_state == UART_PM_STATE_ON) { + dev_pm_opp_set_rate(port->dev, 0); geni_se_resources_off(>se); + } } static const struct uart_ops qcom_geni_console_pops = { @@ -1265,6 +1269,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) dev_err(>dev, "Invalid line %d\n", line); return PTR_ERR(port); } + port->dev = >dev; uport = >uport; /* Don't allow 2 drivers to access the same port */ @@ -1286,6 +1291,12 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) return -EINVAL; uport->mapbase = res->start; + ret = dev_pm_opp_of_add_table(>dev); + if (ret < 0) { + dev_err(>dev, "failed to init OPP table: %d\n", ret); + return ret; + } + port->tx_fifo_depth = DEF_FIFO_DEPTH_WORDS; port->rx_fifo_depth = DEF_FIFO_DEPTH_WORDS; port->tx_fifo_width = DEF_FIFO_WIDTH_BITS; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 06/11] scsi: ufs: Add support to manage multiple power domains in ufshcd-pltfrm
Some UFS devices need to manage multiple powerdomains. Add support for it as part of the ufshcd-pltfrm driver. Signed-off-by: Rajendra Nayak --- drivers/scsi/ufs/ufshcd-pltfrm.c | 52 +++- drivers/scsi/ufs/ufshcd.h| 3 ++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c index 238a79f21e74..ce33eb8b7510 100644 --- a/drivers/scsi/ufs/ufshcd-pltfrm.c +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c @@ -34,6 +34,7 @@ */ #include +#include #include #include @@ -289,6 +290,43 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba) } } +static int ufshcd_attach_pds(struct device *dev, struct ufs_hba *hba, int num_pds) +{ + int i, ret; + + hba->virt_devs = devm_kzalloc(dev, sizeof(struct device *) * num_pds, + GFP_KERNEL); + if (!hba->virt_devs) + return -ENOMEM; + + hba->num_virt_devs = num_pds; + for (i = 0; i < num_pds; i++) { + hba->virt_devs[i] = dev_pm_domain_attach_by_id(dev, i); + if (IS_ERR(hba->virt_devs[i])) { + ret = PTR_ERR(hba->virt_devs[i]); + goto unroll_attach; + } + device_link_add(dev,hba->virt_devs[i], DL_FLAG_RPM_ACTIVE | + DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS); + } + + return ret; + +unroll_attach: + for (i--; i >= 0; i--) + dev_pm_domain_detach(hba->virt_devs[i], false); + + return ret; +} + +static void ufshcd_detach_pds(struct ufs_hba *hba) +{ + int i; + + for (i = 0; i < hba->num_virt_devs; i++) + dev_pm_domain_detach(hba->virt_devs[i], false); +} + /** * ufshcd_pltfrm_init - probe routine of the driver * @pdev: pointer to Platform device handle @@ -302,7 +340,7 @@ int ufshcd_pltfrm_init(struct platform_device *pdev, struct ufs_hba *hba; void __iomem *mmio_base; struct resource *mem_res; - int irq, err; + int irq, err, num_pds; struct device *dev = >dev; mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -340,6 +378,16 @@ int ufshcd_pltfrm_init(struct platform_device *pdev, goto dealloc_host; } + num_pds = of_count_phandle_with_args(dev->of_node, "power-domains", +"#power-domain-cells"); + if (num_pds > 1) { + err = ufshcd_attach_pds(>dev, hba, num_pds); + if (err) { + dev_err(>dev, "%s: attach of power domains failed %d\n", + __func__, err); + goto dealloc_host; + } + } pm_runtime_get_noresume(>dev); pm_runtime_set_active(>dev); pm_runtime_enable(>dev); @@ -358,6 +406,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev, return 0; out_disable_rpm: + if (num_pds > 1) + ufshcd_detach_pds(hba); pm_runtime_disable(>dev); pm_runtime_set_suspended(>dev); pm_runtime_put_noidle(>dev); diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index ecfa898b9ccc..bca1e008f506 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -517,6 +517,9 @@ struct ufs_hba { struct Scsi_Host *host; struct device *dev; + struct device **virt_devs; + u8 num_virt_devs; + /* * This field is to keep a reference to "scsi_device" corresponding to * "UFS device" W-LU. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 05/11] arm64: dts: sdm845: Add OPP table for all qup devices
qup has a requirement to vote on the performance state of the CX domain in sdm845 devices. Add OPP tables for these and also add power-domains property for all qup instances. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 115 +++ 1 file changed, 115 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index e4b69c74fe07..027ffe6e93e8 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -409,6 +409,25 @@ clock-names = "core"; }; + qup_opp_table: qup-opp-table { + compatible = "operating-points-v2"; + + opp-1920 { + opp-hz = /bits/ 64 <1920>; + required-opps = <_opp_min_svs>; + }; + + opp-7500 { + opp-hz = /bits/ 64 <7500>; + required-opps = <_opp_low_svs>; + }; + + opp-1 { + opp-hz = /bits/ 64 <1>; + required-opps = <_opp_svs>; + }; + }; + qupv3_id_0: geniqup@8c { compatible = "qcom,geni-se-qup"; reg = <0 0x008c 0 0x6000>; @@ -430,6 +449,8 @@ interrupts = ; #address-cells = <1>; #size-cells = <0>; + power-domains = < SDM845_CX>; + operating-points-v2 = <_opp_table>; status = "disabled"; }; @@ -443,6 +464,8 @@ interrupts = ; #address-cells = <1>; #size-cells = <0>; + power-domains = < SDM845_CX>; + operating-points-v2 = <_opp_table>; status = "disabled"; }; @@ -454,6 +477,8 @@ pinctrl-names = "default"; pinctrl-0 = <_uart0_default>; interrupts = ; + power-domains = < SDM845_CX>; + operating-points-v2 = <_opp_table>; status = "disabled"; }; @@ -467,6 +492,8 @@ interrupts = ; #address-cells = <1>; #size-cells = <0>; + power-domains = < SDM845_CX>; + operating-points-v2 = <_opp_table>; status = "disabled"; }; @@ -480,6 +507,8 @@ interrupts = ; #address-cells = <1>; #size-cells = <0>; + power-domains = < SDM845_CX>; + operating-points-v2 = <_opp_table>; status = "disabled"; }; @@ -491,6 +520,8 @@ pinctrl-names = "default"; pinctrl-0 = <_uart1_default>; interrupts = ; + power-domains = < SDM845_CX>; + operating-points-v2 = <_opp_table>; status = "disabled"; }; @@ -504,6 +535,8 @@ interrupts = ; #address-cells = <1>; #size-cells = <0>; + power-domains = < SDM845_CX>; + operating-points-v2 = <_opp_table>; status = "disabled"; }; @@ -517,6 +550,8 @@ interrupts = ; #address-cells = <1>; #size-cells = <0>; + power-domains = < SDM845_CX>; + operating-points-v2 = <_opp_table>; status = "disabled"; }; @@ -528,6 +563,8
[RFC v2 04/11] spi: spi-geni-qcom: Use OPP API to set clk/perf state
geni spi needs to express a perforamnce state requirement on CX depending on the frequency of the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- drivers/spi/spi-geni-qcom.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 5f0b0d5bfef4..c251e6df1bc0 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -96,7 +97,6 @@ static int get_spi_clk_cfg(unsigned int speed_hz, { unsigned long sclk_freq; unsigned int actual_hz; - struct geni_se *se = >se; int ret; ret = geni_se_clk_freq_match(>se, @@ -113,9 +113,9 @@ static int get_spi_clk_cfg(unsigned int speed_hz, dev_dbg(mas->dev, "req %u=>%u sclk %lu, idx %d, div %d\n", speed_hz, actual_hz, sclk_freq, *clk_idx, *clk_div); - ret = clk_set_rate(se->clk, sclk_freq); + ret = dev_pm_opp_set_rate(mas->dev, sclk_freq); if (ret) - dev_err(mas->dev, "clk_set_rate failed %d\n", ret); + dev_err(mas->dev, "dev_pm_opp_set_rate failed %d\n", ret); return ret; } @@ -560,6 +560,12 @@ static int spi_geni_probe(struct platform_device *pdev) if (!spi) return -ENOMEM; + ret = dev_pm_opp_of_add_table(>dev); + if (ret < 0) { + dev_err(>dev, "failed to init OPP table: %d\n", ret); + return ret; + } + platform_set_drvdata(pdev, spi); mas = spi_master_get_devdata(spi); mas->irq = irq; @@ -625,6 +631,8 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev) struct spi_master *spi = dev_get_drvdata(dev); struct spi_geni_master *mas = spi_master_get_devdata(spi); + /* Drop the performance state vote */ + dev_pm_opp_set_rate(dev, 0); return geni_se_resources_off(>se); } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid
For devices with performance state, we use dev_pm_opp_set_rate() to set the appropriate clk rate and the performance state. We do need a way to *remove* the performance state vote when we idle the device and turn the clocks off. Use dev_pm_opp_set_rate() with freq=0 to achieve this. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- drivers/opp/core.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index bc9a7762dd4c..d6acc880676e 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -708,18 +708,24 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) struct clk *clk; int ret; - if (unlikely(!target_freq)) { - dev_err(dev, "%s: Invalid target frequency %lu\n", __func__, - target_freq); - return -EINVAL; - } - opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { dev_err(dev, "%s: device opp doesn't exist\n", __func__); return PTR_ERR(opp_table); } + if (unlikely(!target_freq)) { + if (opp_table->required_opp_tables) { + /* drop the performance state vote */ + dev_pm_genpd_set_performance_state(dev, 0); + return 0; + } else { + dev_err(dev, "%s: Invalid target frequency %lu\n", __func__, + target_freq); + return -EINVAL; + } + } + clk = opp_table->clk; if (IS_ERR(clk)) { dev_err(dev, "%s: No clock available for the device\n", -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 00/11] DVFS in the OPP core
This is a v2 of the RFC posted earlier by Stephen Boyd [1] As part of v2 I still follow the same approach of dev_pm_opp_set_rate() API using clk framework to round the frequency passed and making it accept 0 as a valid frequency indicating the frequency isn't required anymore. It just has a few more drivers converted to use this approach like dsi/dpu and ufs. ufs demonstrates the case of having to handle multiple power domains, one of which is scalable. The patches are based on 5.1-rc1 and depend on some ufs fixes I posted earlier [2] and a DT patch to include the rpmpd header [3] [1] https://lkml.org/lkml/2019/1/28/2086 [2] https://lkml.org/lkml/2019/3/8/70 [3] https://lkml.org/lkml/2019/3/20/120 Rajendra Nayak (10): OPP: Make dev_pm_opp_set_rate() with freq=0 as valid tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state spi: spi-geni-qcom: Use OPP API to set clk/perf state arm64: dts: sdm845: Add OPP table for all qup devices scsi: ufs: Add support to manage multiple power domains in ufshcd-pltfrm scsi: ufs: Add support for specifying OPP tables in DT arm64: dts: sdm845: Add ufs opps and power-domains drm/msm/dpu: Use OPP API to set clk/perf state drm/msm: dsi: Use OPP API to set clk/perf state arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains Stephen Boyd (1): OPP: Don't overwrite rounded clk rate arch/arm64/boot/dts/qcom/sdm845.dtsi | 194 +- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 7 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 + drivers/gpu/drm/msm/dsi/dsi.h | 2 + drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 +- drivers/gpu/drm/msm/dsi/dsi_host.c| 88 +++- drivers/opp/core.c| 26 ++- drivers/scsi/ufs/ufshcd-pltfrm.c | 52 - drivers/scsi/ufs/ufshcd.c | 21 +- drivers/scsi/ufs/ufshcd.h | 3 + drivers/spi/spi-geni-qcom.c | 14 +- drivers/tty/serial/qcom_geni_serial.c | 15 +- 12 files changed, 406 insertions(+), 29 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 01/11] OPP: Don't overwrite rounded clk rate
From: Stephen Boyd Doing this allows us to call this API with any rate requested and have it not need to match in the OPP table. Instead, we'll round the rate up to the nearest OPP that we see so that we can get the voltage or level that's required for that OPP. This supports users of OPP that want to specify the 'fmax' tables of a device instead of every single frequency that they need. And for devices that required the exact frequency, we can rely on the clk framework to round the rate to the nearest supported frequency instead of the OPP framework to do so. Note that this may affect drivers that don't want the clk framework to do rounding, but instead want the OPP table to do the rounding for them. Do we have that case? Should we add some flag to the OPP table to indicate this and then not have that flag set when there isn't an OPP table for the device and also introduce a property like 'opp-use-clk' to tell the table that it should use the clk APIs to round rates instead of OPP? Signed-off-by: Stephen Boyd Signed-off-by: Rajendra Nayak --- drivers/opp/core.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 0420f7e8ad5b..bc9a7762dd4c 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -703,7 +703,7 @@ static int _set_required_opps(struct device *dev, int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) { struct opp_table *opp_table; - unsigned long freq, old_freq; + unsigned long freq, opp_freq, old_freq, old_opp_freq; struct dev_pm_opp *old_opp, *opp; struct clk *clk; int ret; @@ -742,13 +742,15 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) goto put_opp_table; } - old_opp = _find_freq_ceil(opp_table, _freq); + old_opp_freq = old_freq; + old_opp = _find_freq_ceil(opp_table, _opp_freq); if (IS_ERR(old_opp)) { dev_err(dev, "%s: failed to find current OPP for freq %lu (%ld)\n", __func__, old_freq, PTR_ERR(old_opp)); } - opp = _find_freq_ceil(opp_table, ); + opp_freq = freq; + opp = _find_freq_ceil(opp_table, _freq); if (IS_ERR(opp)) { ret = PTR_ERR(opp); dev_err(dev, "%s: failed to find OPP for freq %lu (%d)\n", -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 6/6] arm64: dts: sdm845: Add gpu and gmu device nodes
On 1/9/2019 10:50 AM, Doug Anderson wrote: ...but in the meantime Rajendra has had to change his bindings, so you still need to spin this to account for Rajendra's v9 bindings [2]. Specifically you need to make changes like: - compatible = "operating-points-v2-qcom-level"; + compatible = "operating-points-v2-level"; so there's now a v10 [1] and the new compatible is completely dropped and opp-level is now an optional property using the default "operating-points-v2" so the change should be - compatible = "operating-points-v2-qcom-level"; + compatible = "operating-points-v2"; [1] https://lkml.org/lkml/2019/1/9/152 -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 2/2] arm64: dts: sdm845: Add gpu and gmu device nodes
On 12/29/2018 6:59 AM, Stephen Boyd wrote: So I am guessing the conclusion is to use a fallback "operating-points-v2" compatible*only* when we do have opp-hz along with qcom,level (as in the case with gpu) and not have a fallback compatible in cases when we don't have opp-hz (as in the case of rpm power domains)? That seems a little inconsistent, and given Rob said either way is fine, just do one way or the other and not both, I am inclined to think we should just have a "operating-points-v2-qcom-level" and no fallback compatible. Does that make sense? Are you going to update the skip table to not create platform devices? Or introduce some generic property to indicate that this is just data and not a device node? Is any of it really needed, given the bindings specify that the OPP table should actually be a child node of the device/power domain supporting it? I don't see who would end up creating platform devices for them. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 2/2] arm64: dts: sdm845: Add gpu and gmu device nodes
On 12/21/2018 2:59 AM, Stephen Boyd wrote: Quoting Rob Herring (2018-12-19 15:47:25) On Wed, Dec 19, 2018 at 4:40 PM Doug Anderson wrote: On Wed, Dec 19, 2018 at 12:40 PM Doug Anderson wrote: On Wed, Dec 19, 2018 at 12:09 PM Rob Herring wrote: ...but it does have a frequency, doesn't it? + compatible = "operating-points-v2-qcom-level"; + + opp-71000 { + opp-hz = /bits/ 64 <71000>; + qcom,level = ; + }; Ah, I perhaps see the confusion. So Rajendra's usage of "operating-points-v2-qcom-level" [1] doesn't have a frequency but Jordan's do. So I guess it makes sense that Jordan's have the fallback compatible but Rajendra's don't? Is having it useful to s/w that doesn't understand "operating-points-v2-qcom-level"? If so, then add "operating-points-v2". If not, then don't. The only benefit I see in having "operating-points-v2" is that we don't need to update the of_skipped_node_table[] in drivers/platform/of.c to have all the variants of operating-points-v2-* when they decide to not use anything from the "base" binding. If that fails to work because opp-hz is required for the "operating-points-v2" binding but sometimes operating-points-v2-qcom-level doesn't require it I guess we need to update the skip table or make some generic property like 'this-is-not-a-device' that these various data tables in DT can be marked with so we don't make platform devices for them. Regardless of the above, we should update the binding for operating-points-v2-qcom-level to say that opp-hz isn't always required when the qcom-level compatible is present. It looks like it just says that it builds on top of the opp binding so that's not obvious. Sure, I can respin with those details added in. So I am guessing the conclusion is to use a fallback "operating-points-v2" compatible *only* when we do have opp-hz along with qcom,level (as in the case with gpu) and not have a fallback compatible in cases when we don't have opp-hz (as in the case of rpm power domains)? That seems a little inconsistent, and given Rob said either way is fine, just do one way or the other and not both, I am inclined to think we should just have a "operating-points-v2-qcom-level" and no fallback compatible. Does that make sense? ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Patch v3 ] drm/msm/dpu: Correct dpu destroy and disable order
On 11/19/2018 11:55 AM, Jayant Shekhar wrote: In case of msm drm bind failure, pm runtime put sync is called from dsi driver which issues an asynchronous put on mdss device. Subsequently when dpu_mdss_destroy is triggered the change will make sure to put the mdss device in suspend and clearing pending work if not scheduled. Changes in v2: - Removed double spacings [Jeykumar] Changes in v3: - Fix clock on issue during bootup [Rajendra] Signed-off-by: Jayant Shekhar So I no longer see the clock usecount mismatch with this patch now, that I had earlier reported with the v2. Tested-by: Rajendra Nayak --- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c index fd9c893..df8127b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c @@ -156,18 +156,16 @@ static void dpu_mdss_destroy(struct drm_device *dev) struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss); struct dss_module_power *mp = _mdss->mp; + pm_runtime_suspend(dev->dev); + pm_runtime_disable(dev->dev); _dpu_mdss_irq_domain_fini(dpu_mdss); - free_irq(platform_get_irq(pdev, 0), dpu_mdss); - msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(>dev, mp->clk_config); if (dpu_mdss->mmio) devm_iounmap(>dev, dpu_mdss->mmio); dpu_mdss->mmio = NULL; - - pm_runtime_disable(dev->dev); priv->mdss = NULL; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/msm/dpu: Correct dpu destroy and disable order
On 11/2/2018 6:19 PM, Jayant Shekhar wrote: In case of msm drm bind failure, dpu_mdss_destroy is triggered. In this function, resources are freed and pm runtime disable is called, which triggers dpu_mdss_disable. Now in dpu_mdss_disable, driver tries to access a memory which is already freed. This results in kernel panic. Fix this by ensuring proper sequence of dpu destroy and disable calls. Changes in v2: - Removed double spacings [Jeykumar] Signed-off-by: Jayant Shekhar I was testing this patch out and in my setup I see that with this change, during a dsi probe defer case, a dpu_mdss_enable() is called followed by a dpu_mdss_destroy() but dpu_mdss_disable() is never called. This results in a mismatch in clock usecount causing the mdp clocks to stay enabled even with display turned off. localhost ~ # cat /sys/kernel/debug/clk/clk_summary | grep mdss disp_cc_mdss_rscc_ahb_clk000 0 0 0 5 disp_cc_mdss_axi_clk 000 0 0 0 5 disp_cc_mdss_ahb_clk 000 0 0 0 5 disp_cc_mdss_mdp_clk_src 1101920 0 0 5 disp_cc_mdss_mdp_lut_clk 0001920 0 0 5 disp_cc_mdss_mdp_clk1101920 0 0 5 disp_cc_mdss_vsync_clk_src 0001920 0 0 5 Also to note, on an older 4.14 kernel, I do not see any issue in the order in which these functions are called in the dsi probe defer case, i.e its an enable followed by a disable and then a destroy (and that's without this patch) --- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c index fd9c893..902bb4c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c @@ -156,18 +156,15 @@ static void dpu_mdss_destroy(struct drm_device *dev) struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss); struct dss_module_power *mp = _mdss->mp; + pm_runtime_disable(dev->dev); _dpu_mdss_irq_domain_fini(dpu_mdss); - free_irq(platform_get_irq(pdev, 0), dpu_mdss); - msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(>dev, mp->clk_config); if (dpu_mdss->mmio) devm_iounmap(>dev, dpu_mdss->mmio); dpu_mdss->mmio = NULL; - - pm_runtime_disable(dev->dev); priv->mdss = NULL; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU
Hey Jordan/Viresh, On 03/09/2018 09:13 AM, Viresh Kumar wrote: > On 08-03-18, 13:14, Jordan Crouse wrote: >> It seems to me that performance_state has a direct relationship with genpd >> which is good for CPU votes but in this case, we're just passing along raw >> data >> to an independent microcontroller. The 'qcom,arc-level' is used to construct >> the actual values that the GMU will program into the RPMh. Since these are > > The "genpd" here is created specially for this RPM. The performance-state > thing > is designed to solve this very specific problem of qualcomm SoCs and there is > no > way we are going to add another property for this now. > >> informational (from the CPU perspective) rather than functional I feel like >> that using performance_state for this would be as hacky as using >> opp-microvolt >> or something else. > > There is some WIP stuff here Rajendra is testing currently. What I am doing is a powerdomain driver to communicate magic values from CPU to rpmh, looks like we need another one to communicate from CPU to GMU now :) A few things, * someone will need to map these larger magic values into something that rpmh would understand (between 0 to 15 on the sdm845), thats done by reading the command db level maps. Is this done by GMU? * are these communicated just once for every OPP at init/boot, or for every OPP switch? * whats the communication mechanism we use between CPU and GMU? regards Rajendra > > ssh://g...@git.linaro.org/people/viresh.kumar/mylinux.git opp/genpd/qcom > > Please have a talk with Rajendra who can help you understand on how this can > be > used for GPUs. > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 1/3] clk: inherit display clocks enabled by bootloader
Hi Rob, On 07/11/2017 11:50 PM, Rob Clark wrote: > The goal here is to support inheriting a display setup by bootloader, > although there may also be some non-display related use-cases. > > Rough idea is to add a flag for clks and power domains that might > already be enabled when kernel starts, and make corresponding fixups > to clk enable/prepare_count and power-domain state so that these are > not automatically disabled late in boot. > > If bootloader is enabling display, and kernel is using efifb before > real display driver is loaded (potentially from kernel module after > userspace starts, in a typical distro kernel), we don't want to kill > the clocks and power domains that are used by the display before > userspace starts. > > Second part is for drm/msm to check if display related clocks are > enabled when it is loaded, and if so read back hw state to sync > existing display state w/ software state, and skip the initial > clk_enable's and otherwise fixing up clk/regulator/etc ref counts > (skipping the normal display-enable codepaths), therefore inheriting > the enable done by bootloader. > > Obviously this should be split up into multiple patches and many > TODOs addressed. But I guess this is enough for now to start > discussing the approach, and in particular how drm and clock/pd > drivers work together to handle handover from bootloader. > > The CLK_INHERIT_BOOTLOADER and related gsdc flag should only be set > on leaf nodes. > --- [].. > diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c > index d523991c945f..90b698c910d0 100644 > --- a/drivers/clk/qcom/common.c > +++ b/drivers/clk/qcom/common.c > @@ -11,6 +11,7 @@ > * GNU General Public License for more details. > */ > > +#include > #include > #include > #include > @@ -258,6 +259,33 @@ int qcom_cc_really_probe(struct platform_device *pdev, > if (ret) > return ret; > > + /* Check which of clocks that we inherit state from bootloader > + * are enabled, and fixup enable/prepare state (as well as that > + * of it's parents). > + * > + * TODO can we assume that parents coming from another clk > + * driver are already registered? > + */ > + for (i = 0; i < num_clks; i++) { > + struct clk_hw *hw; > + > + if (!rclks[i]) > + continue; > + > + hw = [i]->hw; > + > + if (!(hw->init->flags & CLK_INHERIT_BOOTLOADER)) > + continue; > + > + if (!clk_is_enabled_regmap(hw)) > + continue; > + > + dev_dbg(dev, "%s is enabled from bootloader!\n", > + hw->init->name); > + > + clk_inherit_enabled(hw->clk); how about just calling a clk_prepare_enable(hw->clk) instead of adding a new API? The flag could also be something qcom specific and then we avoid having to add anything in generic CCF code and its all handled in the qcom clock drivers. > + } > + > reset = >reset; > reset->rcdev.of_node = dev->of_node; > reset->rcdev.ops = _reset_ops; [] .. > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c > index a4f3580587b7..440d819b2d9d 100644 > --- a/drivers/clk/qcom/gdsc.c > +++ b/drivers/clk/qcom/gdsc.c > @@ -291,6 +291,12 @@ static int gdsc_init(struct gdsc *sc) > if ((sc->flags & VOTABLE) && on) > gdsc_enable(>pd); > > + if ((sc->flags & INHERIT_BL) && on) { > + pr_debug("gdsc: %s is enabled from bootloader!\n", sc->pd.name); > + gdsc_enable(>pd); > + sc->pd.flags |= GENPD_FLAG_ALWAYS_ON; Would this not prevent the powerdomain from ever getting disabled? regards, Rajendra > + } > + > if (on || (sc->pwrsts & PWRSTS_RET)) > gdsc_force_mem_on(sc); > else > diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h > index 39648348e5ec..3b5e64b060c2 100644 > --- a/drivers/clk/qcom/gdsc.h > +++ b/drivers/clk/qcom/gdsc.h > @@ -53,6 +53,7 @@ struct gdsc { > #define VOTABLE BIT(0) > #define CLAMP_IO BIT(1) > #define HW_CTRL BIT(2) > +#define INHERIT_BL BIT(3) > struct reset_controller_dev *rcdev; > unsigned int*resets; > unsigned intreset_count; > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index c59c62571e4f..4d5505f92329 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -35,6 +35,7 @@ > #define CLK_IS_CRITICAL BIT(11) /* do not gate, ever */ > /* parents need enable during gate/ungate, set rate and re-parent */ > #define CLK_OPS_PARENT_ENABLEBIT(12) > +#define CLK_INHERIT_BOOTLOADER BIT(13) /* clk may be enabled from > bootloader */ > > struct clk; > struct clk_hw; > diff --git a/include/linux/clk.h b/include/linux/clk.h > index 91bd464f4c9b..461991fc57e2 100644 > ---
Re: [RFC] clk: inherit display clocks enabled by bootloader
On 07/04/2017 11:21 PM, Rob Clark wrote: > The goal here is to support inheriting a display setup by bootloader, > although there may also be some non-display related use-cases. > > Rough idea is to add a flag for clks and power domains that might > already be enabled when kernel starts, and make corresponding fixups > to clk enable/prepare_count and power-domain state so that these are > not automatically disabled late in boot. > > If bootloader is enabling display, and kernel is using efifb before > real display driver is loaded (potentially from kernel module after > userspace starts, in a typical distro kernel), we don't want to kill > the clocks and power domains that are used by the display before > userspace starts. > > Second part will be (*waves hands*) for drm/msm to check if display > related clocks are enabled when it is loaded, and if so use drm > atomic framework's hooks to read back hw state to sync existing > display state w/ software state, and skip the initial clk_enable. > Therefore inheriting the enable done by bootloader. > > Obviously this should be split up into multiple patches and many > TODOs addressed. But I guess this is enough for now to start > discussing the approach, and in particular how drm and clock/pd > drivers work together to handle handover from bootloader. > > The CLK_INHERIT_BOOTLOADER and related gsdc flag should only be set > on leaf nodes. > --- > A bit hacky right now, but display survives clk_disable_unused() > and genpd_power_off_unused(). It hangs just after that late in > boot, which I'm still debugging (might be unrelated shenanigans). > And haven't started on the drm/msm side of this. But I figured > it was half baked enough to send out for comments/ideas, or to > see if anyone had some different idea about how to solve this. Another RFC proposed around to handle similar situations is https://lkml.org/lkml/2017/6/28/188 That one though, I guess deals with only regulator supplies for now. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel