From: Tony Lindgren <tony@atomide.com> To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Sebastian Reichel <sre@kernel.org>, dri-devel@lists.freedesktop.org, linux-omap@vger.kernel.org Subject: Re: [PATCHv2] drm/omap: Fix issue with clocks left on after resume Date: Mon, 3 May 2021 14:16:04 +0300 [thread overview] Message-ID: <YI/bdLkwtUNFKHyW@atomide.com> (raw) In-Reply-To: <YI/UXqQbvdtC2HqI@atomide.com> Hi, * Tony Lindgren <tony@atomide.com> [210503 10:45]: > * Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> [210503 08:04]: > > On 29/04/2021 07:46, Tony Lindgren wrote: > > > Decoupling the system suspend and resume from PM runtime calls for > > > all the other dss components should still also be done IMO. But that > > > can be done as a separate clean-up patches after we have fixed the > > > $subject issue. > > > > I don't think I still really understand why all this is needed. I mean, > > obviously things don't work correctly at the moment, so maybe this patch can > > be applied to fix the system suspend. But it just feels like a big hack (the > > current pm_runtime_force_suspend/resume work-around feels like a big hack > > too). > > Well omapdrm is not handling the -EBUSY error during system resume. Or rather something on the resume path is not handling and cannot handle -EBUSY. And sounds like the reason is.. > > Slightly off topic, but I just noticed that we're using runtime_put_sync for > > some reason. Found 0eaf9f52e94f756147dbfe1faf1f77a02378dbf9. I've been > > fighting with system suspend for a long time =). > > > > I wonder if using non-sync version would remove the EBUSY problem... > > Worth trying, but it will only help if the -EBUSY error from > pm_runtime_put() is handled somewhere for a retry.. ..the use of pm_runtime_put_sync() like you suggested. I did a quick test with the minimal change below and that works :) Seems like that's probably the best minimal fix for the -rc cycle. Regards, Tony 8< ---------------- diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c --- a/drivers/gpu/drm/omapdrm/dss/dispc.c +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c @@ -664,7 +664,7 @@ void dispc_runtime_put(struct dispc_device *dispc) DSSDBG("dispc_runtime_put\n"); - r = pm_runtime_put_sync(&dispc->pdev->dev); + r = pm_runtime_put(&dispc->pdev->dev); WARN_ON(r < 0 && r != -ENOSYS); }
WARNING: multiple messages have this Message-ID (diff)
From: Tony Lindgren <tony@atomide.com> To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Cc: linux-omap@vger.kernel.org, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, dri-devel@lists.freedesktop.org, Sebastian Reichel <sre@kernel.org> Subject: Re: [PATCHv2] drm/omap: Fix issue with clocks left on after resume Date: Mon, 3 May 2021 14:16:04 +0300 [thread overview] Message-ID: <YI/bdLkwtUNFKHyW@atomide.com> (raw) In-Reply-To: <YI/UXqQbvdtC2HqI@atomide.com> Hi, * Tony Lindgren <tony@atomide.com> [210503 10:45]: > * Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> [210503 08:04]: > > On 29/04/2021 07:46, Tony Lindgren wrote: > > > Decoupling the system suspend and resume from PM runtime calls for > > > all the other dss components should still also be done IMO. But that > > > can be done as a separate clean-up patches after we have fixed the > > > $subject issue. > > > > I don't think I still really understand why all this is needed. I mean, > > obviously things don't work correctly at the moment, so maybe this patch can > > be applied to fix the system suspend. But it just feels like a big hack (the > > current pm_runtime_force_suspend/resume work-around feels like a big hack > > too). > > Well omapdrm is not handling the -EBUSY error during system resume. Or rather something on the resume path is not handling and cannot handle -EBUSY. And sounds like the reason is.. > > Slightly off topic, but I just noticed that we're using runtime_put_sync for > > some reason. Found 0eaf9f52e94f756147dbfe1faf1f77a02378dbf9. I've been > > fighting with system suspend for a long time =). > > > > I wonder if using non-sync version would remove the EBUSY problem... > > Worth trying, but it will only help if the -EBUSY error from > pm_runtime_put() is handled somewhere for a retry.. ..the use of pm_runtime_put_sync() like you suggested. I did a quick test with the minimal change below and that works :) Seems like that's probably the best minimal fix for the -rc cycle. Regards, Tony 8< ---------------- diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c --- a/drivers/gpu/drm/omapdrm/dss/dispc.c +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c @@ -664,7 +664,7 @@ void dispc_runtime_put(struct dispc_device *dispc) DSSDBG("dispc_runtime_put\n"); - r = pm_runtime_put_sync(&dispc->pdev->dev); + r = pm_runtime_put(&dispc->pdev->dev); WARN_ON(r < 0 && r != -ENOSYS); } _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-05-03 11:16 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-28 9:25 [PATCHv2] drm/omap: Fix issue with clocks left on after resume Tony Lindgren 2021-04-28 9:25 ` Tony Lindgren 2021-04-28 14:10 ` Laurent Pinchart 2021-04-28 14:10 ` Laurent Pinchart 2021-04-29 4:46 ` Tony Lindgren 2021-04-29 4:46 ` Tony Lindgren 2021-05-03 8:04 ` Tomi Valkeinen 2021-05-03 8:04 ` Tomi Valkeinen 2021-05-03 10:45 ` Tony Lindgren 2021-05-03 10:45 ` Tony Lindgren 2021-05-03 11:16 ` Tony Lindgren [this message] 2021-05-03 11:16 ` Tony Lindgren 2021-05-03 12:17 ` Tony Lindgren 2021-05-03 12:17 ` Tony Lindgren 2021-05-05 11:12 ` Tony Lindgren 2021-05-05 11:12 ` Tony Lindgren 2021-05-07 13:26 ` Tomi Valkeinen 2021-05-07 13:26 ` Tomi Valkeinen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YI/bdLkwtUNFKHyW@atomide.com \ --to=tony@atomide.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-omap@vger.kernel.org \ --cc=sre@kernel.org \ --cc=tomi.valkeinen@ideasonboard.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.