[PATCH 1/3] drm/tegra: vic: Implement explicit reset support

[PATCH 1/3] drm/tegra: vic: Implement explicit reset support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Tegra supports generic PM domains on 64-bit ARM, and if that is enabled,
the power domain code will make sure that resets are asserted and
deasserted at appropriate points in time.

If generic PM domains are not implemented, such as on 32-bit Tegra, the
resets need to be asserted and deasserted explicitly by the driver.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/vic.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
index 9fa77405db01..23f530db45ad 100644
--- a/drivers/gpu/drm/tegra/vic.c
+++ b/drivers/gpu/drm/tegra/vic.c
@@ -38,6 +38,7 @@ struct vic {
 	struct iommu_domain *domain;
 	struct device *dev;
 	struct clk *clk;
+	struct reset_control *rst;
 
 	/* Platform configuration */
 	const struct vic_config *config;
@@ -56,13 +57,37 @@ static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
 static int vic_runtime_resume(struct device *dev)
 {
 	struct vic *vic = dev_get_drvdata(dev);
+	int err;
+
+	err = clk_prepare_enable(vic->clk);
+	if (err < 0)
+		return err;
+
+	usleep_range(2000, 4000);
+
+	err = reset_control_deassert(vic->rst);
+	if (err < 0)
+		goto disable;
+
+	usleep_range(2000, 4000);
+
+	return 0;
 
-	return clk_prepare_enable(vic->clk);
+disable:
+	clk_disable_unprepare(vic->clk);
+	return err;
 }
 
 static int vic_runtime_suspend(struct device *dev)
 {
 	struct vic *vic = dev_get_drvdata(dev);
+	int err;
+
+	err = reset_control_assert(vic->rst);
+	if (err < 0)
+		return err;
+
+	usleep_range(2000, 4000);
 
 	clk_disable_unprepare(vic->clk);
 
@@ -324,6 +349,14 @@ static int vic_probe(struct platform_device *pdev)
 		return PTR_ERR(vic->clk);
 	}
 
+	if (!dev->pm_domain) {
+		vic->rst = devm_reset_control_get(dev, "vic");
+		if (IS_ERR(vic->rst)) {
+			dev_err(&pdev->dev, "failed to get reset\n");
+			return PTR_ERR(vic->rst);
+		}
+	}
+
 	vic->falcon.dev = dev;
 	vic->falcon.regs = vic->regs;
 	vic->falcon.ops = &vic_falcon_ops;
-- 
2.19.1

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

[PATCH 2/3] drm/tegra: falcon: Fix error handling

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The ->alloc() callback in struct falcon_ops returns an ERR_PTR()-encoded
error code on failure, so it needs to be properly checked for, otherwise
subsequent code may dereference an invalid pointer.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/falcon.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/falcon.c b/drivers/gpu/drm/tegra/falcon.c
index f685e72949d1..78c7a0156601 100644
--- a/drivers/gpu/drm/tegra/falcon.c
+++ b/drivers/gpu/drm/tegra/falcon.c
@@ -141,9 +141,9 @@ int falcon_load_firmware(struct falcon *falcon)
 	/* allocate iova space for the firmware */
 	falcon->firmware.vaddr = falcon->ops->alloc(falcon, firmware->size,
 						    &falcon->firmware.paddr);
-	if (!falcon->firmware.vaddr) {
-		dev_err(falcon->dev, "dma memory mapping failed\n");
-		return -ENOMEM;
+	if (IS_ERR(falcon->firmware.vaddr)) {
+		dev_err(falcon->dev, "DMA memory mapping failed\n");
+		return PTR_ERR(falcon->firmware.vaddr);
 	}
 
 	/* copy firmware image into local area. this also ensures endianness */
-- 
2.19.1

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

[PATCH 3/3] drm/tegra: falcon: Wait for memory scrubbing to complete

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Before booting the Falcon processor, make sure to wait for memory
scrubbing to complete.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/falcon.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/tegra/falcon.c b/drivers/gpu/drm/tegra/falcon.c
index 78c7a0156601..352d05feabb0 100644
--- a/drivers/gpu/drm/tegra/falcon.c
+++ b/drivers/gpu/drm/tegra/falcon.c
@@ -197,11 +197,19 @@ void falcon_exit(struct falcon *falcon)
 int falcon_boot(struct falcon *falcon)
 {
 	unsigned long offset;
+	u32 value;
 	int err;
 
 	if (!falcon->firmware.vaddr)
 		return -EINVAL;
 
+	err = readl_poll_timeout(falcon->regs + FALCON_DMACTL, value,
+				 (value & (FALCON_DMACTL_IMEM_SCRUBBING |
+					   FALCON_DMACTL_DMEM_SCRUBBING)) == 0,
+				 10, 10000);
+	if (err < 0)
+		return err;
+
 	falcon_writel(falcon, 0, FALCON_DMACTL);
 
 	/* setup the address of the binary data so Falcon can access it later */
-- 
2.19.1

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

Re: [PATCH 1/3] drm/tegra: vic: Implement explicit reset support

[Jon Hunter]

On 23/11/2018 12:06, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Tegra supports generic PM domains on 64-bit ARM, and if that is enabled,
> the power domain code will make sure that resets are asserted and
> deasserted at appropriate points in time.
> 
> If generic PM domains are not implemented, such as on 32-bit Tegra, the
> resets need to be asserted and deasserted explicitly by the driver.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/vic.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> index 9fa77405db01..23f530db45ad 100644
> --- a/drivers/gpu/drm/tegra/vic.c
> +++ b/drivers/gpu/drm/tegra/vic.c
> @@ -38,6 +38,7 @@ struct vic {
>  	struct iommu_domain *domain;
>  	struct device *dev;
>  	struct clk *clk;
> +	struct reset_control *rst;
>  
>  	/* Platform configuration */
>  	const struct vic_config *config;
> @@ -56,13 +57,37 @@ static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
>  static int vic_runtime_resume(struct device *dev)
>  {
>  	struct vic *vic = dev_get_drvdata(dev);
> +	int err;
> +
> +	err = clk_prepare_enable(vic->clk);
> +	if (err < 0)
> +		return err;
> +
> +	usleep_range(2000, 4000);

The Tegra genpd code has a usleep_range(10, 20), is that not sufficient
here? If it is, it would be good to be consistent.

> +
> +	err = reset_control_deassert(vic->rst);
> +	if (err < 0)
> +		goto disable;
> +
> +	usleep_range(2000, 4000);
> +
> +	return 0;
>  
> -	return clk_prepare_enable(vic->clk);
> +disable:
> +	clk_disable_unprepare(vic->clk);
> +	return err;
>  }
>  
>  static int vic_runtime_suspend(struct device *dev)
>  {
>  	struct vic *vic = dev_get_drvdata(dev);
> +	int err;
> +
> +	err = reset_control_assert(vic->rst);
> +	if (err < 0)
> +		return err;
> +
> +	usleep_range(2000, 4000);
>  
>  	clk_disable_unprepare(vic->clk);
>  
> @@ -324,6 +349,14 @@ static int vic_probe(struct platform_device *pdev)
>  		return PTR_ERR(vic->clk);
>  	}
>  
> +	if (!dev->pm_domain) {
> +		vic->rst = devm_reset_control_get(dev, "vic");
> +		if (IS_ERR(vic->rst)) {
> +			dev_err(&pdev->dev, "failed to get reset\n");
> +			return PTR_ERR(vic->rst);
> +		}
> +	}
> +
>  	vic->falcon.dev = dev;
>  	vic->falcon.regs = vic->regs;
>  	vic->falcon.ops = &vic_falcon_ops;
> 

Cheers
Jon

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

Re: [PATCH 1/3] drm/tegra: vic: Implement explicit reset support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1956 bytes --]

On Thu, Nov 29, 2018 at 01:40:32PM +0000, Jon Hunter wrote:
> 
> On 23/11/2018 12:06, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Tegra supports generic PM domains on 64-bit ARM, and if that is enabled,
> > the power domain code will make sure that resets are asserted and
> > deasserted at appropriate points in time.
> > 
> > If generic PM domains are not implemented, such as on 32-bit Tegra, the
> > resets need to be asserted and deasserted explicitly by the driver.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/tegra/vic.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> > index 9fa77405db01..23f530db45ad 100644
> > --- a/drivers/gpu/drm/tegra/vic.c
> > +++ b/drivers/gpu/drm/tegra/vic.c
> > @@ -38,6 +38,7 @@ struct vic {
> >  	struct iommu_domain *domain;
> >  	struct device *dev;
> >  	struct clk *clk;
> > +	struct reset_control *rst;
> >  
> >  	/* Platform configuration */
> >  	const struct vic_config *config;
> > @@ -56,13 +57,37 @@ static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
> >  static int vic_runtime_resume(struct device *dev)
> >  {
> >  	struct vic *vic = dev_get_drvdata(dev);
> > +	int err;
> > +
> > +	err = clk_prepare_enable(vic->clk);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	usleep_range(2000, 4000);
> 
> The Tegra genpd code has a usleep_range(10, 20), is that not sufficient
> here? If it is, it would be good to be consistent.

Yeah, I think that's enough. The Tegra DRM driver uses these ranges in
many places, so that's where I copied them from. None of these are part
of a hot path or anything, so whether this sleeps for 10 ns or 4 ms is
not going to matter much.

With that changed, can I consider this R-b you?

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 1/3] drm/tegra: vic: Implement explicit reset support

[Jon Hunter]

On 29/11/2018 14:51, Thierry Reding wrote:
> On Thu, Nov 29, 2018 at 01:40:32PM +0000, Jon Hunter wrote:
>>
>> On 23/11/2018 12:06, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> Tegra supports generic PM domains on 64-bit ARM, and if that is enabled,
>>> the power domain code will make sure that resets are asserted and
>>> deasserted at appropriate points in time.
>>>
>>> If generic PM domains are not implemented, such as on 32-bit Tegra, the
>>> resets need to be asserted and deasserted explicitly by the driver.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  drivers/gpu/drm/tegra/vic.c | 35 ++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 34 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
>>> index 9fa77405db01..23f530db45ad 100644
>>> --- a/drivers/gpu/drm/tegra/vic.c
>>> +++ b/drivers/gpu/drm/tegra/vic.c
>>> @@ -38,6 +38,7 @@ struct vic {
>>>  	struct iommu_domain *domain;
>>>  	struct device *dev;
>>>  	struct clk *clk;
>>> +	struct reset_control *rst;
>>>  
>>>  	/* Platform configuration */
>>>  	const struct vic_config *config;
>>> @@ -56,13 +57,37 @@ static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
>>>  static int vic_runtime_resume(struct device *dev)
>>>  {
>>>  	struct vic *vic = dev_get_drvdata(dev);
>>> +	int err;
>>> +
>>> +	err = clk_prepare_enable(vic->clk);
>>> +	if (err < 0)
>>> +		return err;
>>> +
>>> +	usleep_range(2000, 4000);
>>
>> The Tegra genpd code has a usleep_range(10, 20), is that not sufficient
>> here? If it is, it would be good to be consistent.
> 
> Yeah, I think that's enough. The Tegra DRM driver uses these ranges in
> many places, so that's where I copied them from. None of these are part
> of a hot path or anything, so whether this sleeps for 10 ns or 4 ms is
> not going to matter much.
> 
> With that changed, can I consider this R-b you?

Yes please add my r-b.

Thanks
Jon

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