[PATCH 0/6] drm/gma500: 1 fix + further cleanups

[Hans de Goede]

Hi Patrik,

On 9/14/22 09:50, Patrik Jakobsson wrote:
> On Fri, Sep 9, 2022 at 1:56 PM Hans de Goede <hdegoede at redhat.com> wrote:
>>
>> Hi Patrik,
>>
>> Here is another gma500 patch-series with one more bugfix and a bunch
>> of other cleanups of stuff which I noticed while doing the previous
>> set of bugfixes.
>>
> 
> Hi Hans, nice cleanups!
> 
> I'm rather busy at the moment so you can commit these yourself to
> drm-misc-next if you like.
> 
> "drm/gma500: Wait longer for the GPU to power-down" can go through
> drm-misc-fixes if you prefer. It fixed the timeout message on two of
> my CDV machines but I never saw an actual problem from the timeouts.
> 
> For the entire series:
> Acked-by: Patrik Jakobsson <patrik.r.jakobsson at gmail.com>

Thanks.

I'm pushing these out to drm-misc-next now, but with some small
changes:

1. I have dropped the "drm/gma500: Wait longer for the GPU to power-down"
patch I'm still seeing timeouts even if I increase the wait time to
a full seconds. I believe that the actual issue is this line:

	dev_priv->apm_base = CDV_MSG_READ32(domain, PSB_PUNIT_PORT, PSB_APMBA);

sometimes failing. When the timeout happens I see apm_base is set to 0 and
reading apm_base + cmd / sts offset returns bogus values.

I have yet to have a successful boot where the timeout does not happen
since I have been poking at this (it seems success/fail wrt the timeout
is random). But I suspect that with a successful boot apm_base will not
be 0 and that the problem is there. To be continued...


2. For the "drm/gma500: Rewrite power management code" I noticed the
following error during further testing (for the actual backlight changes):

[   12.292509] gma500 0000:00:02.0: Unbalanced pm_runtime_enable!

The problem is that pci_pm_init() which the PCI core runs for each
device already does:

        pm_runtime_forbid(&dev->dev);
        pm_runtime_set_active(&dev->dev);
        pm_runtime_enable(&dev->dev);

So the pm_runtime_enable() call in "drm/gma500: Rewrite power management code"
was a second enable call and as such was not necessary. So I'm going to
squash in the following small fix while pushing this out:

diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c
index 62d2cc1923f1..0080b692dc3e 100644
--- a/drivers/gpu/drm/gma500/power.c
+++ b/drivers/gpu/drm/gma500/power.c
@@ -61,10 +61,11 @@ void gma_power_init(struct drm_device *dev)
 	 * To fix this we need to call pm_runtime_get() once for each active
 	 * pipe at boot and then put() / get() for each pipe disable / enable
 	 * so that the device gets runtime suspended when no pipes are active.
+	 * Once this is in place the pm_runtime_get() below should be replaced
+	 * by a pm_runtime_allow() call to undo the pm_runtime_forbid() from
+	 * pci_pm_init().
 	 */
 	pm_runtime_get(dev->dev);
-	pm_runtime_set_active(dev->dev); /* Must be done before pm_runtime_enable()! */
-	pm_runtime_enable(dev->dev);
 
 	dev_priv->pm_initialized = true;
 }
@@ -83,7 +83,6 @@ void gma_power_uninit(struct drm_device *dev)
 	if (!dev_priv->pm_initialized)
 		return;
 
-	pm_runtime_disable(dev->dev);
 	pm_runtime_put_noidle(dev->dev);
 }
 

As you can see all the removed lines are already taken care of by the
PCI core, so this squashed in change really is a no-op (other then
that it silences the "Unbalanced pm_runtime_enable!" message).

Regards,

Hans




> 
> 
>> Regards,
>>
>> Hans
>>
>>
>> Hans de Goede (6):
>>   drm/gma500: Wait longer for the GPU to power-down
>>   drm/gma500: Remove runtime_allowed dead code in psb_unlocked_ioctl()
>>   drm/gma500: Remove never set dev_priv->rpm_enabled flag
>>   drm/gma500: Remove a couple of not useful function wrappers
>>   drm/gma500: Rewrite power management code
>>   drm/gma500: Remove unnecessary suspend/resume wrappers
>>
>>  drivers/gpu/drm/gma500/cdv_device.c    |   2 +-
>>  drivers/gpu/drm/gma500/gma_display.c   |  19 +--
>>  drivers/gpu/drm/gma500/gma_display.h   |   2 -
>>  drivers/gpu/drm/gma500/oaktrail_lvds.c |   1 -
>>  drivers/gpu/drm/gma500/power.c         | 156 +++++--------------------
>>  drivers/gpu/drm/gma500/power.h         |  18 ---
>>  drivers/gpu/drm/gma500/psb_drv.c       |  35 +-----
>>  drivers/gpu/drm/gma500/psb_drv.h       |   7 +-
>>  drivers/gpu/drm/gma500/psb_irq.c       |  15 ++-
>>  9 files changed, 41 insertions(+), 214 deletions(-)
>>
>> --
>> 2.37.2
>>
>