Hi Patrik,
On 9/14/22 09:50, Patrik Jakobsson wrote:
> On Fri, Sep 9, 2022 at 1:56 PM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxx>
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
>>
>
