| Message ID | 20181008092730.1199-3-skolluku@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use interconnect API in MDSS on SDM845 | expand |
On Mon, Oct 08, 2018 at 02:57:29PM +0530, Sravanthi Kollukuduru wrote: > The interconnect framework is designed to provide a > standard kernel interface to control the settings of > the interconnects on a SoC. > > The interconnect API uses a consumer/provider-based model, > where the providers are the interconnect buses and the > consumers could be various drivers. > > MDSS is one of the interconnect consumers which uses the > interconnect APIs to get the path between endpoints and > set its bandwidth/latency/QoS requirements for the given > interconnected path. > > Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 56 +++++++++++++++++++++++++++++--- > 1 file changed, 52 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 2235ef8129f4..8391e5c1e559 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > @@ -4,10 +4,12 @@ > */ > > #include "dpu_kms.h" > +#include <linux/interconnect.h> > > #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base) > > #define HW_INTR_STATUS 0x0010 > +#define MAX_AXI_PORT_COUNT 3 > > struct dpu_mdss { > struct msm_mdss base; > @@ -16,8 +18,36 @@ struct dpu_mdss { > u32 hwversion; > struct dss_module_power mp; > struct dpu_irq_controller irq_controller; > + struct icc_path *path[MAX_AXI_PORT_COUNT]; > + u32 num_paths; > }; > > +static int dpu_mdss_parse_data_bus_icc_path( > + struct drm_device *dev, struct dpu_mdss *dpu_mdss) > +{ > + struct icc_path *path0 = of_icc_get(dev->dev, "port0"); > + struct icc_path *path1 = of_icc_get(dev->dev, "port1"); > + int total_num_paths = 0; > + > + if (IS_ERR(path0)) > + return PTR_ERR(path0); > + > + dpu_mdss->path[0] = path0; > + total_num_paths = 1; > + > + if (!IS_ERR(path1)) { > + dpu_mdss->path[1] = path1; > + total_num_paths++; > + } > + > + if (total_num_paths > MAX_AXI_PORT_COUNT) > + return -EINVAL; Since you are using fixed names you know that by design this isn't possible. > + dpu_mdss->num_paths = total_num_paths; > + > + return 0; > +} > + > static irqreturn_t dpu_mdss_irq(int irq, void *arg) > { > struct dpu_mdss *dpu_mdss = arg; > @@ -127,7 +157,12 @@ static int dpu_mdss_enable(struct msm_mdss *mdss) > { > struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss); > struct dss_module_power *mp = &dpu_mdss->mp; > - int ret; > + int ret, i; > + u64 ab = (dpu_mdss->num_paths) ? 6800000000/dpu_mdss->num_paths : 0; > + u64 ib = 6800000000; > + > + for (i = 0; i < dpu_mdss->num_paths; i++) > + icc_set(dpu_mdss->path[i], ab, ib); > > ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true); > if (ret) > @@ -140,12 +175,15 @@ static int dpu_mdss_disable(struct msm_mdss *mdss) > { > struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss); > struct dss_module_power *mp = &dpu_mdss->mp; > - int ret; > + int ret, i; > > ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); > if (ret) > DPU_ERROR("clock disable failed, ret:%d\n", ret); > > + for (i = 0; i < dpu_mdss->num_paths; i++) > + icc_set(dpu_mdss->path[i], 0, 0); > + > return ret; > } > > @@ -155,6 +193,7 @@ static void dpu_mdss_destroy(struct drm_device *dev) > struct msm_drm_private *priv = dev->dev_private; > struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss); > struct dss_module_power *mp = &dpu_mdss->mp; > + int i; > > _dpu_mdss_irq_domain_fini(dpu_mdss); > > @@ -163,6 +202,9 @@ static void dpu_mdss_destroy(struct drm_device *dev) > msm_dss_put_clk(mp->clk_config, mp->num_clk); > devm_kfree(&pdev->dev, mp->clk_config); > > + for (i = 0; i < dpu_mdss->num_paths; i++) > + icc_put(dpu_mdss->path[i]); > + > if (dpu_mdss->mmio) > devm_iounmap(&pdev->dev, dpu_mdss->mmio); > dpu_mdss->mmio = NULL; > @@ -203,6 +245,12 @@ int dpu_mdss_init(struct drm_device *dev) > } > dpu_mdss->mmio_len = resource_size(res); > > + ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss); > + if (ret) { > + DPU_ERROR("failed to parse icc path, ret=%d\n", ret); Depending on the ordering between DPU and interconnect, this might return -EPROBE_DEFER for a few times before working. You should skip printing an error message on -EPROBE_DEFER so it doesn't spam the log. > + return ret; > + } > + > mp = &dpu_mdss->mp; > ret = msm_dss_parse_clock(pdev, mp); > if (ret) { > @@ -224,14 +272,14 @@ int dpu_mdss_init(struct drm_device *dev) > goto irq_error; > } > > + priv->mdss = &dpu_mdss->base; > + This seems like it might have snuck in from a different patch? > pm_runtime_enable(dev->dev); > > pm_runtime_get_sync(dev->dev); > dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio); > pm_runtime_put_sync(dev->dev); > > - priv->mdss = &dpu_mdss->base; > - > return ret; > > irq_error:
On 2018-10-08 20:09, Jordan Crouse wrote: > On Mon, Oct 08, 2018 at 02:57:29PM +0530, Sravanthi Kollukuduru wrote: >> The interconnect framework is designed to provide a >> standard kernel interface to control the settings of >> the interconnects on a SoC. >> >> The interconnect API uses a consumer/provider-based model, >> where the providers are the interconnect buses and the >> consumers could be various drivers. >> >> MDSS is one of the interconnect consumers which uses the >> interconnect APIs to get the path between endpoints and >> set its bandwidth/latency/QoS requirements for the given >> interconnected path. >> >> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 56 >> +++++++++++++++++++++++++++++--- >> 1 file changed, 52 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 2235ef8129f4..8391e5c1e559 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c >> @@ -4,10 +4,12 @@ >> */ >> >> #include "dpu_kms.h" >> +#include <linux/interconnect.h> >> >> #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base) >> >> #define HW_INTR_STATUS 0x0010 >> +#define MAX_AXI_PORT_COUNT 3 >> >> struct dpu_mdss { >> struct msm_mdss base; >> @@ -16,8 +18,36 @@ struct dpu_mdss { >> u32 hwversion; >> struct dss_module_power mp; >> struct dpu_irq_controller irq_controller; >> + struct icc_path *path[MAX_AXI_PORT_COUNT]; >> + u32 num_paths; >> }; >> >> +static int dpu_mdss_parse_data_bus_icc_path( >> + struct drm_device *dev, struct dpu_mdss *dpu_mdss) >> +{ >> + struct icc_path *path0 = of_icc_get(dev->dev, "port0"); >> + struct icc_path *path1 = of_icc_get(dev->dev, "port1"); >> + int total_num_paths = 0; >> + >> + if (IS_ERR(path0)) >> + return PTR_ERR(path0); >> + >> + dpu_mdss->path[0] = path0; >> + total_num_paths = 1; >> + >> + if (!IS_ERR(path1)) { >> + dpu_mdss->path[1] = path1; >> + total_num_paths++; >> + } >> + >> + if (total_num_paths > MAX_AXI_PORT_COUNT) >> + return -EINVAL; > > Since you are using fixed names you know that by design this isn't > possible. > Ok, will remove the check and the macro. >> + dpu_mdss->num_paths = total_num_paths; >> + >> + return 0; >> +} >> + >> static irqreturn_t dpu_mdss_irq(int irq, void *arg) >> { >> struct dpu_mdss *dpu_mdss = arg; >> @@ -127,7 +157,12 @@ static int dpu_mdss_enable(struct msm_mdss *mdss) >> { >> struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss); >> struct dss_module_power *mp = &dpu_mdss->mp; >> - int ret; >> + int ret, i; >> + u64 ab = (dpu_mdss->num_paths) ? 6800000000/dpu_mdss->num_paths : 0; >> + u64 ib = 6800000000; >> + >> + for (i = 0; i < dpu_mdss->num_paths; i++) >> + icc_set(dpu_mdss->path[i], ab, ib); >> >> ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true); >> if (ret) >> @@ -140,12 +175,15 @@ static int dpu_mdss_disable(struct msm_mdss >> *mdss) >> { >> struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss); >> struct dss_module_power *mp = &dpu_mdss->mp; >> - int ret; >> + int ret, i; >> >> ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); >> if (ret) >> DPU_ERROR("clock disable failed, ret:%d\n", ret); >> >> + for (i = 0; i < dpu_mdss->num_paths; i++) >> + icc_set(dpu_mdss->path[i], 0, 0); >> + >> return ret; >> } >> >> @@ -155,6 +193,7 @@ static void dpu_mdss_destroy(struct drm_device >> *dev) >> struct msm_drm_private *priv = dev->dev_private; >> struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss); >> struct dss_module_power *mp = &dpu_mdss->mp; >> + int i; >> >> _dpu_mdss_irq_domain_fini(dpu_mdss); >> >> @@ -163,6 +202,9 @@ static void dpu_mdss_destroy(struct drm_device >> *dev) >> msm_dss_put_clk(mp->clk_config, mp->num_clk); >> devm_kfree(&pdev->dev, mp->clk_config); >> >> + for (i = 0; i < dpu_mdss->num_paths; i++) >> + icc_put(dpu_mdss->path[i]); >> + >> if (dpu_mdss->mmio) >> devm_iounmap(&pdev->dev, dpu_mdss->mmio); >> dpu_mdss->mmio = NULL; >> @@ -203,6 +245,12 @@ int dpu_mdss_init(struct drm_device *dev) >> } >> dpu_mdss->mmio_len = resource_size(res); >> >> + ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss); >> + if (ret) { >> + DPU_ERROR("failed to parse icc path, ret=%d\n", ret); > > Depending on the ordering between DPU and interconnect, this might > return > -EPROBE_DEFER for a few times before working. You should skip printing > an error message on -EPROBE_DEFER so it doesn't spam the log. > Ok, will remove it. >> + return ret; >> + } >> + >> mp = &dpu_mdss->mp; >> ret = msm_dss_parse_clock(pdev, mp); >> if (ret) { >> @@ -224,14 +272,14 @@ int dpu_mdss_init(struct drm_device *dev) >> goto irq_error; >> } >> >> + priv->mdss = &dpu_mdss->base; >> + > > This seems like it might have snuck in from a different patch? No, this was included as part of this patch to ensure the mdss structure is intialized before the first pm runtime resume call. If not, the dpu_mdss_enable will not be called which has the logic to vote for bw and subsequently, enable the clocks. > >> pm_runtime_enable(dev->dev); >> >> pm_runtime_get_sync(dev->dev); >> dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio); >> pm_runtime_put_sync(dev->dev); >> >> - priv->mdss = &dpu_mdss->base; >> - >> return ret; >> >> irq_error:
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c index 2235ef8129f4..8391e5c1e559 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c @@ -4,10 +4,12 @@ */ #include "dpu_kms.h" +#include <linux/interconnect.h> #define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base) #define HW_INTR_STATUS 0x0010 +#define MAX_AXI_PORT_COUNT 3 struct dpu_mdss { struct msm_mdss base; @@ -16,8 +18,36 @@ struct dpu_mdss { u32 hwversion; struct dss_module_power mp; struct dpu_irq_controller irq_controller; + struct icc_path *path[MAX_AXI_PORT_COUNT]; + u32 num_paths; }; +static int dpu_mdss_parse_data_bus_icc_path( + struct drm_device *dev, struct dpu_mdss *dpu_mdss) +{ + struct icc_path *path0 = of_icc_get(dev->dev, "port0"); + struct icc_path *path1 = of_icc_get(dev->dev, "port1"); + int total_num_paths = 0; + + if (IS_ERR(path0)) + return PTR_ERR(path0); + + dpu_mdss->path[0] = path0; + total_num_paths = 1; + + if (!IS_ERR(path1)) { + dpu_mdss->path[1] = path1; + total_num_paths++; + } + + if (total_num_paths > MAX_AXI_PORT_COUNT) + return -EINVAL; + + dpu_mdss->num_paths = total_num_paths; + + return 0; +} + static irqreturn_t dpu_mdss_irq(int irq, void *arg) { struct dpu_mdss *dpu_mdss = arg; @@ -127,7 +157,12 @@ static int dpu_mdss_enable(struct msm_mdss *mdss) { struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss); struct dss_module_power *mp = &dpu_mdss->mp; - int ret; + int ret, i; + u64 ab = (dpu_mdss->num_paths) ? 6800000000/dpu_mdss->num_paths : 0; + u64 ib = 6800000000; + + for (i = 0; i < dpu_mdss->num_paths; i++) + icc_set(dpu_mdss->path[i], ab, ib); ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true); if (ret) @@ -140,12 +175,15 @@ static int dpu_mdss_disable(struct msm_mdss *mdss) { struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss); struct dss_module_power *mp = &dpu_mdss->mp; - int ret; + int ret, i; ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false); if (ret) DPU_ERROR("clock disable failed, ret:%d\n", ret); + for (i = 0; i < dpu_mdss->num_paths; i++) + icc_set(dpu_mdss->path[i], 0, 0); + return ret; } @@ -155,6 +193,7 @@ static void dpu_mdss_destroy(struct drm_device *dev) struct msm_drm_private *priv = dev->dev_private; struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss); struct dss_module_power *mp = &dpu_mdss->mp; + int i; _dpu_mdss_irq_domain_fini(dpu_mdss); @@ -163,6 +202,9 @@ static void dpu_mdss_destroy(struct drm_device *dev) msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(&pdev->dev, mp->clk_config); + for (i = 0; i < dpu_mdss->num_paths; i++) + icc_put(dpu_mdss->path[i]); + if (dpu_mdss->mmio) devm_iounmap(&pdev->dev, dpu_mdss->mmio); dpu_mdss->mmio = NULL; @@ -203,6 +245,12 @@ int dpu_mdss_init(struct drm_device *dev) } dpu_mdss->mmio_len = resource_size(res); + ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss); + if (ret) { + DPU_ERROR("failed to parse icc path, ret=%d\n", ret); + return ret; + } + mp = &dpu_mdss->mp; ret = msm_dss_parse_clock(pdev, mp); if (ret) { @@ -224,14 +272,14 @@ int dpu_mdss_init(struct drm_device *dev) goto irq_error; } + priv->mdss = &dpu_mdss->base; + pm_runtime_enable(dev->dev); pm_runtime_get_sync(dev->dev); dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio); pm_runtime_put_sync(dev->dev); - priv->mdss = &dpu_mdss->base; - return ret; irq_error:
The interconnect framework is designed to provide a standard kernel interface to control the settings of the interconnects on a SoC. The interconnect API uses a consumer/provider-based model, where the providers are the interconnect buses and the consumers could be various drivers. MDSS is one of the interconnect consumers which uses the interconnect APIs to get the path between endpoints and set its bandwidth/latency/QoS requirements for the given interconnected path. Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 56 +++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 4 deletions(-)