| Message ID | 20240628-gpucc-no-request-v1-1-b680c2f90817@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | clk: qcom: gpucc-*: stop requesting register resource | expand |
Quoting Dmitry Baryshkov (2024-06-27 22:20:22) > The GPU clock controllers use memory region that is a part of the GMU's > memory region. Add qcom_cc_map_norequest() to be used by GPUCC, so that > GPU driver can use devm_ioremap_resource for GMU resources. Why does GMU map the gpu clk controller? Does it use those registers? We don't want to allow two different drivers to map the same region because then they don't coordinate and write over things.
On Tue, 9 Jul 2024 at 01:30, Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Dmitry Baryshkov (2024-06-27 22:20:22) > > The GPU clock controllers use memory region that is a part of the GMU's > > memory region. Add qcom_cc_map_norequest() to be used by GPUCC, so that > > GPU driver can use devm_ioremap_resource for GMU resources. > > Why does GMU map the gpu clk controller? Does it use those registers? We > don't want to allow two different drivers to map the same region because > then they don't coordinate and write over things. It's not that GMU maps gpu CC separately. It looks more like gpucc is a part of the GMU address space. I think GMU manages some of the clocks or GDSCs directly.
Quoting Dmitry Baryshkov (2024-07-10 16:32:18) > On Tue, 9 Jul 2024 at 01:30, Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Dmitry Baryshkov (2024-06-27 22:20:22) > > > The GPU clock controllers use memory region that is a part of the GMU's > > > memory region. Add qcom_cc_map_norequest() to be used by GPUCC, so that > > > GPU driver can use devm_ioremap_resource for GMU resources. > > > > Why does GMU map the gpu clk controller? Does it use those registers? We > > don't want to allow two different drivers to map the same region because > > then they don't coordinate and write over things. > > It's not that GMU maps gpu CC separately. It looks more like gpucc is > a part of the GMU address space. I think GMU manages some of the > clocks or GDSCs directly. > I imagine GMU is a collection of stuff, so the register range is large because it's basically a subsystem unto itself. Can the range in DT be split up, or changed so that different devices within GMU are split out? Or maybe the gpu clk controller can be made into a child of some GMU node, where the GMU node has a driver that populates devices that match drivers in different subsystems.
On Thu, 11 Jul 2024 at 03:04, Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Dmitry Baryshkov (2024-07-10 16:32:18) > > On Tue, 9 Jul 2024 at 01:30, Stephen Boyd <sboyd@kernel.org> wrote: > > > > > > Quoting Dmitry Baryshkov (2024-06-27 22:20:22) > > > > The GPU clock controllers use memory region that is a part of the GMU's > > > > memory region. Add qcom_cc_map_norequest() to be used by GPUCC, so that > > > > GPU driver can use devm_ioremap_resource for GMU resources. > > > > > > Why does GMU map the gpu clk controller? Does it use those registers? We > > > don't want to allow two different drivers to map the same region because > > > then they don't coordinate and write over things. > > > > It's not that GMU maps gpu CC separately. It looks more like gpucc is > > a part of the GMU address space. I think GMU manages some of the > > clocks or GDSCs directly. > > > > I imagine GMU is a collection of stuff, so the register range is large > because it's basically a subsystem unto itself. Can the range in DT be > split up, or changed so that different devices within GMU are split out? No, we have to remain compatible with existing DT. It's not a problem of a single new platform, the issue has always been present there. > Or maybe the gpu clk controller can be made into a child of some GMU > node, where the GMU node has a driver that populates devices that match > drivers in different subsystems. Well... Technically yes, but this brings another pack of issues. There is no separate GMU driver, so we will likely have a chicken-and-egg problem, as probing of the GPU driver will also create the gpucc device which is further used by the GPU.
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c index c92e10c60322..dcc73bc22606 100644 --- a/drivers/clk/qcom/common.c +++ b/drivers/clk/qcom/common.c @@ -113,6 +113,26 @@ qcom_cc_map(struct platform_device *pdev, const struct qcom_cc_desc *desc) } EXPORT_SYMBOL_GPL(qcom_cc_map); +/* gpucc shares memory region with GMU, don't request the region */ +struct regmap * +qcom_cc_map_norequest(struct platform_device *pdev, const struct qcom_cc_desc *desc) +{ + struct device *dev = &pdev->dev; + struct resource *r; + void __iomem *base; + + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!r) + return ERR_PTR(dev_err_probe(dev, -EINVAL, "resource not found\n")); + + base = devm_ioremap(dev, r->start, resource_size(r)); + if (IS_ERR(base)) + return ERR_CAST(base); + + return devm_regmap_init_mmio(dev, base, desc->config); +} +EXPORT_SYMBOL_GPL(qcom_cc_map_norequest); + void qcom_pll_set_fsm_mode(struct regmap *map, u32 reg, u8 bias_count, u8 lock_count) { diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h index d048bdeeba10..6cab7805a92c 100644 --- a/drivers/clk/qcom/common.h +++ b/drivers/clk/qcom/common.h @@ -60,6 +60,8 @@ extern int qcom_cc_register_sleep_clk(struct device *dev); extern struct regmap *qcom_cc_map(struct platform_device *pdev, const struct qcom_cc_desc *desc); +extern struct regmap *qcom_cc_map_norequest(struct platform_device *pdev, + const struct qcom_cc_desc *desc); extern int qcom_cc_really_probe(struct device *dev, const struct qcom_cc_desc *desc, struct regmap *regmap);
The GPU clock controllers use memory region that is a part of the GMU's memory region. Add qcom_cc_map_norequest() to be used by GPUCC, so that GPU driver can use devm_ioremap_resource for GMU resources. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/clk/qcom/common.c | 20 ++++++++++++++++++++ drivers/clk/qcom/common.h | 2 ++ 2 files changed, 22 insertions(+)