| Message ID | 20210708143736.2018135-1-thierry.reding@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | drm/tegra: gr2d: Explicitly control module reset | expand |
08.07.2021 17:37, Thierry Reding пишет: > From: Thierry Reding <treding@nvidia.com> > > As of commit 4782c0a5dd88 ("clk: tegra: Don't deassert reset on enabling > clocks"), module resets are no longer automatically deasserted when the > module clock is enabled. To make sure that the gr2d module continues to > work, we need to explicitly control the module reset. > > Fixes: 4782c0a5dd88 ("clk: tegra: Don't deassert reset on enabling clocks") > Signed-off-by: Thierry Reding <treding@nvidia.com> On which board do see this problem? TRM says that 2d should be in reset by default, but somehow it's not a problem on devices that use fastboot.. why would it touch the 2d reset? > --- > drivers/gpu/drm/tegra/gr2d.c | 33 +++++++++++++++++++++++++++++++-- > 1 file changed, 31 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c > index de288cba3905..ba3722f1b865 100644 > --- a/drivers/gpu/drm/tegra/gr2d.c > +++ b/drivers/gpu/drm/tegra/gr2d.c > @@ -4,9 +4,11 @@ > */ > > #include <linux/clk.h> > +#include <linux/delay.h> > #include <linux/iommu.h> > #include <linux/module.h> > #include <linux/of_device.h> > +#include <linux/reset.h> > > #include "drm.h" > #include "gem.h" > @@ -19,6 +21,7 @@ struct gr2d_soc { > struct gr2d { > struct tegra_drm_client client; > struct host1x_channel *channel; > + struct reset_control *rst; Unused variable? > struct clk *clk; > > const struct gr2d_soc *soc; > @@ -208,6 +211,12 @@ static int gr2d_probe(struct platform_device *pdev) > if (!syncpts) > return -ENOMEM; > > + gr2d->rst = devm_reset_control_get(dev, NULL); > + if (IS_ERR(gr2d->rst)) { > + dev_err(dev, "cannot get reset\n"); > + return PTR_ERR(gr2d->rst); > + } > + > gr2d->clk = devm_clk_get(dev, NULL); > if (IS_ERR(gr2d->clk)) { > dev_err(dev, "cannot get clock\n"); > @@ -220,6 +229,14 @@ static int gr2d_probe(struct platform_device *pdev) > return err; > } > > + usleep_range(2000, 4000); > + > + err = reset_control_deassert(gr2d->rst); > + if (err < 0) { > + dev_err(dev, "failed to deassert reset: %d\n", err); > + goto disable_clk; > + } > + > INIT_LIST_HEAD(&gr2d->client.base.list); > gr2d->client.base.ops = &gr2d_client_ops; > gr2d->client.base.dev = dev; > @@ -234,8 +251,7 @@ static int gr2d_probe(struct platform_device *pdev) > err = host1x_client_register(&gr2d->client.base); > if (err < 0) { > dev_err(dev, "failed to register host1x client: %d\n", err); > - clk_disable_unprepare(gr2d->clk); > - return err; > + goto assert_rst; > } > > /* initialize address register map */ > @@ -245,6 +261,13 @@ static int gr2d_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, gr2d); > > return 0; > + > +assert_rst: > + (void)reset_control_assert(gr2d->rst); (void)?
08.07.2021 18:13, Dmitry Osipenko пишет: >> #include "drm.h" >> #include "gem.h" >> @@ -19,6 +21,7 @@ struct gr2d_soc { >> struct gr2d { >> struct tegra_drm_client client; >> struct host1x_channel *channel; >> + struct reset_control *rst; > Unused variable? Ah, I haven't noticed that it's struct. Looks okay.
08.07.2021 18:13, Dmitry Osipenko пишет: >> +assert_rst: >> + (void)reset_control_assert(gr2d->rst); > (void)? I forgot that the 2d reset shouldn't be asserted. See comment in gr2d_runtime_suspend() [1]. [1] https://lore.kernel.org/lkml/20210701232728.23591-15-digetx@gmail.com/
diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c index de288cba3905..ba3722f1b865 100644 --- a/drivers/gpu/drm/tegra/gr2d.c +++ b/drivers/gpu/drm/tegra/gr2d.c @@ -4,9 +4,11 @@ */ #include <linux/clk.h> +#include <linux/delay.h> #include <linux/iommu.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/reset.h> #include "drm.h" #include "gem.h" @@ -19,6 +21,7 @@ struct gr2d_soc { struct gr2d { struct tegra_drm_client client; struct host1x_channel *channel; + struct reset_control *rst; struct clk *clk; const struct gr2d_soc *soc; @@ -208,6 +211,12 @@ static int gr2d_probe(struct platform_device *pdev) if (!syncpts) return -ENOMEM; + gr2d->rst = devm_reset_control_get(dev, NULL); + if (IS_ERR(gr2d->rst)) { + dev_err(dev, "cannot get reset\n"); + return PTR_ERR(gr2d->rst); + } + gr2d->clk = devm_clk_get(dev, NULL); if (IS_ERR(gr2d->clk)) { dev_err(dev, "cannot get clock\n"); @@ -220,6 +229,14 @@ static int gr2d_probe(struct platform_device *pdev) return err; } + usleep_range(2000, 4000); + + err = reset_control_deassert(gr2d->rst); + if (err < 0) { + dev_err(dev, "failed to deassert reset: %d\n", err); + goto disable_clk; + } + INIT_LIST_HEAD(&gr2d->client.base.list); gr2d->client.base.ops = &gr2d_client_ops; gr2d->client.base.dev = dev; @@ -234,8 +251,7 @@ static int gr2d_probe(struct platform_device *pdev) err = host1x_client_register(&gr2d->client.base); if (err < 0) { dev_err(dev, "failed to register host1x client: %d\n", err); - clk_disable_unprepare(gr2d->clk); - return err; + goto assert_rst; } /* initialize address register map */ @@ -245,6 +261,13 @@ static int gr2d_probe(struct platform_device *pdev) platform_set_drvdata(pdev, gr2d); return 0; + +assert_rst: + (void)reset_control_assert(gr2d->rst); +disable_clk: + clk_disable_unprepare(gr2d->clk); + + return err; } static int gr2d_remove(struct platform_device *pdev) @@ -259,6 +282,12 @@ static int gr2d_remove(struct platform_device *pdev) return err; } + err = reset_control_assert(gr2d->rst); + if (err < 0) + dev_err(&pdev->dev, "failed to assert reset: %d\n", err); + + usleep_range(2000, 4000); + clk_disable_unprepare(gr2d->clk); return 0;