On Thu, Nov 29, 2018 at 07:01:52PM +0300, Dmitry Osipenko wrote:
> On 28.11.2018 19:44, Thierry Reding wrote:
> > From: Thierry Reding <treding@xxxxxxxxxx>
> >
> > The display architecture on Tegra186 and Tegra194 requires that there be
> > some valid clock on all domains before accessing any display register. A
> > further requirement is that in addition to the host1x, hub, disp and dsc
> > clocks, all the head clocks (pclk0-2 on Tegra186 or pclk0-3 on Tegra194)
> > must also be enabled.
> >
> > Implement this logic within the display hub driver to ensure the clocks
> > are always enabled at the right time.
> >
> > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > ---
> > drivers/gpu/drm/tegra/hub.c | 47 +++++++++++++++++++++++++++++++++++--
> > drivers/gpu/drm/tegra/hub.h | 3 +++
> > 2 files changed, 48 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
> > index 6112d9042979..7e7742877b90 100644
> > --- a/drivers/gpu/drm/tegra/hub.c
> > +++ b/drivers/gpu/drm/tegra/hub.c
> > @@ -742,7 +742,9 @@ static const struct host1x_client_ops tegra_display_hub_ops = {
> >
> > static int tegra_display_hub_probe(struct platform_device *pdev)
> > {
> > + struct device_node *child = NULL;
> > struct tegra_display_hub *hub;
> > + struct clk *clk;
> > unsigned int i;
> > int err;
> >
> > @@ -801,6 +803,33 @@ static int tegra_display_hub_probe(struct platform_device *pdev)
> > return err;
> > }
> >
> > + hub->num_heads = of_get_child_count(pdev->dev.of_node);
> > +
> > + hub->clk_heads = devm_kcalloc(&pdev->dev, hub->num_heads, sizeof(clk),
> > + GFP_KERNEL);
> > + if (!hub->clk_heads)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < hub->num_heads; i++) {
> > + child = of_get_next_child(pdev->dev.of_node, child);
> > + if (!child) {
> > + dev_err(&pdev->dev, "failed to find node for head %u\n",
> > + i);
> > + return -ENODEV;
> > + }
> > +
> > + clk = devm_get_clk_from_child(&pdev->dev, child, "dc");
> > + if (IS_ERR(clk)) {
> > + dev_err(&pdev->dev, "failed to get clock for head %u\n",
> > + i);
>
> Looks like "of_node_put(child);" missed here.
Indeed, good catch!
> > + return PTR_ERR(clk);
> > + }
> > +
> > + hub->clk_heads[i] = clk;
> > + }
> > +
> > + of_node_put(child);
> > +
> > /* XXX: enable clock across reset? */
> > err = reset_control_assert(hub->rst);
> > if (err < 0)
> > @@ -840,12 +869,16 @@ static int tegra_display_hub_remove(struct platform_device *pdev)
> > static int __maybe_unused tegra_display_hub_suspend(struct device *dev)
> > {
> > struct tegra_display_hub *hub = dev_get_drvdata(dev);
> > + unsigned int i;
> > int err;
> >
> > err = reset_control_assert(hub->rst);
> > if (err < 0)
> > return err;
> >
> > + for (i = 0; i < hub->num_heads; i++)
> > + clk_disable_unprepare(hub->clk_heads[i]);
> > +
> > clk_disable_unprepare(hub->clk_hub);
> > clk_disable_unprepare(hub->clk_dsc);
> > clk_disable_unprepare(hub->clk_disp);
> > @@ -856,6 +889,7 @@ static int __maybe_unused tegra_display_hub_suspend(struct device *dev)
> > static int __maybe_unused tegra_display_hub_resume(struct device *dev)
> > {
> > struct tegra_display_hub *hub = dev_get_drvdata(dev);
> > + unsigned int i;
> > int err;
> >
> > err = clk_prepare_enable(hub->clk_disp);
> > @@ -870,13 +904,22 @@ static int __maybe_unused tegra_display_hub_resume(struct device *dev)
> > if (err < 0)
> > goto disable_dsc;
> >
> > + for (i = 0; i < hub->num_heads; i++) {
> > + err = clk_prepare_enable(hub->clk_heads[i]);
> > + if (err < 0)
> > + goto disable_heads;
> > + }
>
> What about to use clk_bulk_* API?
Can't do that because we get these clocks from the child nodes, which is
something that we can't do with clk_bulk_*.
What I could do, though, is to call clk_disable_unprepare() in reverse
order to more closely mirror what clk_bulk_* does, which is obviously
the right thing in terms of symmetry.
Thierry
Attachment:
signature.asc
Description: PGP signature
