linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: "Heiko Stübner" <heiko@sntech.de>, "Sean Paul" <seanpaul@chromium.org>
Cc: "Sandy Huang" <hjc@rock-chips.com>,
	"Michel Dänzer" <michel.daenzer@mailbox.org>,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	dri-devel@lists.freedesktop.org,
	"Brian Norris" <briannorris@chromium.org>,
	stable@vger.kernel.org, "kernelci.org bot" <bot@kernelci.org>
Subject: [PATCH 2/2] drm/rockchip: vop: Leave vblank enabled in self-refresh
Date: Thu,  5 Jan 2023 17:40:18 -0800	[thread overview]
Message-ID: <20230105174001.2.Ic07cba4ab9a7bd3618a9e4258b8f92ea7d10ae5a@changeid> (raw)
In-Reply-To: <20230105174001.1.I3904f697863649eb1be540ecca147a66e42bfad7@changeid>

If we disable vblank when entering self-refresh, vblank APIs (like
DRM_IOCTL_WAIT_VBLANK) no longer work. But user space is not aware when
we enter self-refresh, so this appears to be an API violation -- that
DRM_IOCTL_WAIT_VBLANK fails with EINVAL whenever the display is idle and
enters self-refresh.

The downstream driver used by many of these systems never used to
disable vblank for PSR, and in fact, even upstream, we didn't do that
until radically redesigning the state machine in commit 6c836d965bad
("drm/rockchip: Use the helpers for PSR").

Thus, it seems like a reasonable API fix to simply restore that
behavior, and leave vblank enabled.

Note that this appears to potentially unbalance the
drm_crtc_vblank_{off,on}() calls in some cases, but:
(a) drm_crtc_vblank_on() documents this as OK and
(b) if I do the naive balancing, I find state machine issues such that
    we're not in sync properly; so it's easier to take advantage of (a).

Backporting notes:
Marking as 'Fixes' commit 6c836d965bad ("drm/rockchip: Use the helpers
for PSR"), but it probably depends on commit bed030a49f3e
("drm/rockchip: Don't fully disable vop on self refresh") as well.

We also need the previous patch ("drm/atomic: Allow vblank-enabled +
self-refresh "disable""), of course.

Fixes: 6c836d965bad ("drm/rockchip: Use the helpers for PSR")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/dri-devel/Y5itf0+yNIQa6fU4@sirena.org.uk/
Reported-by: "kernelci.org bot" <bot@kernelci.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index fa1f4ee6d195..c541967114b4 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -719,11 +719,11 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
 
 	mutex_lock(&vop->vop_lock);
 
-	drm_crtc_vblank_off(crtc);
-
 	if (crtc->state->self_refresh_active)
 		goto out;
 
+	drm_crtc_vblank_off(crtc);
+
 	/*
 	 * Vop standby will take effect at end of current frame,
 	 * if dsp hold valid irq happen, it means standby complete.
-- 
2.39.0.314.g84b9a713c41-goog


  reply	other threads:[~2023-01-06  1:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-06  1:40 [PATCH 1/2] drm/atomic: Allow vblank-enabled + self-refresh "disable" Brian Norris
2023-01-06  1:40 ` Brian Norris [this message]
2023-01-06 11:42   ` [PATCH 2/2] drm/rockchip: vop: Leave vblank enabled in self-refresh Michel Dänzer
2023-01-07  1:21     ` Brian Norris
2023-01-06  7:04 ` [PATCH 1/2] drm/atomic: Allow vblank-enabled + self-refresh "disable" Greg KH
2023-01-06 17:49   ` Daniel Vetter
2023-01-06 17:53 ` Daniel Vetter
2023-01-06 18:08   ` Brian Norris
2023-01-06 18:20     ` Daniel Vetter
2023-01-06 19:25       ` Brian Norris
2023-01-06 18:17   ` Daniel Vetter
2023-01-06 19:33     ` Brian Norris
2023-01-06 20:30       ` Daniel Vetter
2023-01-06 21:30         ` Brian Norris
2023-01-06 22:44           ` Daniel Vetter
2023-01-11 15:03         ` Ville Syrjälä

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=20230105174001.2.Ic07cba4ab9a7bd3618a9e4258b8f92ea7d10ae5a@changeid \
    --to=briannorris@chromium.org \
    --cc=bot@kernelci.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=heiko@sntech.de \
    --cc=hjc@rock-chips.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=michel.daenzer@mailbox.org \
    --cc=seanpaul@chromium.org \
    --cc=stable@vger.kernel.org \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).