On 29.11.2018 19:17, Thierry Reding wrote:
> 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.
Seems there is no need to use clk_bulk_* for getting the clocks, you could build up the bulk-array manually.. somewhat like this:
struct tegra_display_hub {
struct clk *clk_hub;
struct reset_control *rst;
unsigned int num_heads;
struct clk_bulk_data *clk_heads;
...
};
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;
}
hub->clk_heads[i] = 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);
return PTR_ERR(clk);
}
}
...
err = clk_bulk_prepare_enable(hub->num_heads, hub->clk_heads);
if (err < 0)
goto disable_dsc;
...
