diff mbox

drm/vc4: Remove broken attempt at GPU reset using genpd.

Message ID 1450750124-17020-1-git-send-email-eric@anholt.net (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Anholt Dec. 22, 2015, 2:08 a.m. UTC
I've tested and confirmed that it doesn't actually work.  We'll need
to sort out how to do this properly later, but for now just remove it
since it also caused build breakage due to using CONFIG_PM_SLEEP
functions without our Kconfig depending on PM_SLEEP.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_v3d.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

Comments

Sudip Mukherjee Dec. 22, 2015, 4:43 a.m. UTC | #1
On Mon, Dec 21, 2015 at 06:08:44PM -0800, Eric Anholt wrote:
> I've tested and confirmed that it doesn't actually work.  We'll need
> to sort out how to do this properly later, but for now just remove it
> since it also caused build breakage due to using CONFIG_PM_SLEEP
> functions without our Kconfig depending on PM_SLEEP.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>

Acked-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>

regards
sudip
Sudip Mukherjee Jan. 1, 2016, 4:56 a.m. UTC | #2
On Mon, Dec 21, 2015 at 06:08:44PM -0800, Eric Anholt wrote:
> I've tested and confirmed that it doesn't actually work.  We'll need
> to sort out how to do this properly later, but for now just remove it
> since it also caused build breakage due to using CONFIG_PM_SLEEP
> functions without our Kconfig depending on PM_SLEEP.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---

It has still not landed in linux-next. next-20151231 still fails with
the same error.

regards
sudip
Guenter Roeck Jan. 22, 2016, 2:31 p.m. UTC | #3
On Mon, Dec 21, 2015 at 06:08:44PM -0800, Eric Anholt wrote:
> I've tested and confirmed that it doesn't actually work.  We'll need
> to sort out how to do this properly later, but for now just remove it
> since it also caused build breakage due to using CONFIG_PM_SLEEP
> functions without our Kconfig depending on PM_SLEEP.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>

Still broken upstream.

FWIW:

Acked-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/gpu/drm/vc4/vc4_v3d.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
> index 424d515..314ff71 100644
> --- a/drivers/gpu/drm/vc4/vc4_v3d.c
> +++ b/drivers/gpu/drm/vc4/vc4_v3d.c
> @@ -144,19 +144,16 @@ int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
>  }
>  #endif /* CONFIG_DEBUG_FS */
>  
> -/*
> - * Asks the firmware to turn on power to the V3D engine.
> - *
> - * This may be doable with just the clocks interface, though this
> - * packet does some other register setup from the firmware, too.
> - */
>  int
>  vc4_v3d_set_power(struct vc4_dev *vc4, bool on)
>  {
> -	if (on)
> -		return pm_generic_poweroff(&vc4->v3d->pdev->dev);
> -	else
> -		return pm_generic_resume(&vc4->v3d->pdev->dev);
> +	/* XXX: This interface is needed for GPU reset, and the way to
> +	 * do it is to turn our power domain off and back on.  We
> +	 * can't just reset from within the driver, because the reset
> +	 * bits are in the power domain's register area, and get set
> +	 * during the poweron process.
> +	 */
> +	return 0;
>  }
>  
>  static void vc4_v3d_init_hw(struct drm_device *dev)
> -- 
> 2.6.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
>
Sudip Mukherjee Jan. 22, 2016, 4:54 p.m. UTC | #4
On Friday 22 January 2016 08:01 PM, Guenter Roeck wrote:
> On Mon, Dec 21, 2015 at 06:08:44PM -0800, Eric Anholt wrote:
>> I've tested and confirmed that it doesn't actually work.  We'll need
>> to sort out how to do this properly later, but for now just remove it
>> since it also caused build breakage due to using CONFIG_PM_SLEEP
>> functions without our Kconfig depending on PM_SLEEP.
>>
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>
> Still broken upstream.

I saw Eric sending a pull request for it. But it was never pulled, maybe 
for the subject.
Looks like rc1 will be broken for many arch.

regards
sudip
Guenter Roeck Jan. 22, 2016, 5:06 p.m. UTC | #5
On 01/22/2016 08:54 AM, Sudip Mukherjee wrote:
> On Friday 22 January 2016 08:01 PM, Guenter Roeck wrote:
>> On Mon, Dec 21, 2015 at 06:08:44PM -0800, Eric Anholt wrote:
>>> I've tested and confirmed that it doesn't actually work.  We'll need
>>> to sort out how to do this properly later, but for now just remove it
>>> since it also caused build breakage due to using CONFIG_PM_SLEEP
>>> functions without our Kconfig depending on PM_SLEEP.
>>>
>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>
>> Still broken upstream.
>
> I saw Eric sending a pull request for it. But it was never pulled, maybe for the subject.
> Looks like rc1 will be broken for many arch.
>

Could be worse. Essentially every allmodconfig/allyesconfig which doesn't configure pm
is affected.

With my tests (v4.4-10282-geae2177):

Build results:
	total: 146 pass: 142 fail: 4
Failed builds:
	alpha:allmodconfig
	m68k:allmodconfig
	parisc:allmodconfig
	xtensa:allmodconfig

Qemu test results:
	total: 96 pass: 94 fail: 2
Failed tests:
	arm64:smp:defconfig
	arm64:nosmp:defconfig

arm64 qemu tests are broken for a different reason. There is a patch to fix it,
but for some reason it didn't make it into mainline (yet). It is in -next, though.

Guenter
diff mbox

Patch

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 424d515..314ff71 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -144,19 +144,16 @@  int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
 }
 #endif /* CONFIG_DEBUG_FS */
 
-/*
- * Asks the firmware to turn on power to the V3D engine.
- *
- * This may be doable with just the clocks interface, though this
- * packet does some other register setup from the firmware, too.
- */
 int
 vc4_v3d_set_power(struct vc4_dev *vc4, bool on)
 {
-	if (on)
-		return pm_generic_poweroff(&vc4->v3d->pdev->dev);
-	else
-		return pm_generic_resume(&vc4->v3d->pdev->dev);
+	/* XXX: This interface is needed for GPU reset, and the way to
+	 * do it is to turn our power domain off and back on.  We
+	 * can't just reset from within the driver, because the reset
+	 * bits are in the power domain's register area, and get set
+	 * during the poweron process.
+	 */
+	return 0;
 }
 
 static void vc4_v3d_init_hw(struct drm_device *dev)