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 <tred...@nvidia.com>
> > 
> > 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 <tred...@nvidia.com>
> > ---
> >  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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to