From: Brian Norris <briannorris@chromium.org>
To: "Michel Dänzer" <michel.daenzer@mailbox.org>
Cc: "Heiko Stübner" <heiko@sntech.de>,
"Sean Paul" <seanpaul@chromium.org>,
"kernelci.org bot" <bot@kernelci.org>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
"Sandy Huang" <hjc@rock-chips.com>,
linux-rockchip@lists.infradead.org, stable@vger.kernel.org
Subject: Re: [PATCH 2/2] drm/rockchip: vop: Leave vblank enabled in self-refresh
Date: Fri, 6 Jan 2023 17:21:19 -0800 [thread overview]
Message-ID: <Y7jJDw4RcNceyLbR@google.com> (raw)
In-Reply-To: <9455bc5b-2074-4f48-71a7-5c816ee19a78@mailbox.org>
On Fri, Jan 06, 2023 at 12:42:54PM +0100, Michel Dänzer wrote:
> On 1/6/23 02:40, Brian Norris wrote:
> > --- 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.
>
> The out label immediately unlocks vop->vop_lock again, seems a bit pointless. :)
Oops, of course. Will change that in v2.
> AFAICT the self_refresh_active case should just return immediately,
> the out label would also send any pending atomic commit completion
> event, which seems wrong now that the vblank interrupt is left
> enabled.
If I return immediately and drop that completion, I hit a warning:
[ 4.423876] WARNING: CPU: 5 PID: 58 at drivers/gpu/drm/drm_atomic_helper.c:2460 drm_atomic_helper_commit_hw_done+0xe0/0xe8
...
[ 4.424036] Call trace:
[ 4.424039] drm_atomic_helper_commit_hw_done+0xe0/0xe8
[ 4.424045] drm_atomic_helper_commit_tail_rpm+0x58/0x7c
...
I plan to leave it as-is for v2.
> (I also wonder if drm_crtc_vblank_off doesn't take care of
> that already, at least amdgpu doesn't do this explicitly AFAICT)
I'm not sure.
Brian
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <briannorris@chromium.org>
To: "Michel Dänzer" <michel.daenzer@mailbox.org>
Cc: "Heiko Stübner" <heiko@sntech.de>,
"Sean Paul" <seanpaul@chromium.org>,
"kernelci.org bot" <bot@kernelci.org>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
"Sandy Huang" <hjc@rock-chips.com>,
linux-rockchip@lists.infradead.org, stable@vger.kernel.org
Subject: Re: [PATCH 2/2] drm/rockchip: vop: Leave vblank enabled in self-refresh
Date: Fri, 6 Jan 2023 17:21:19 -0800 [thread overview]
Message-ID: <Y7jJDw4RcNceyLbR@google.com> (raw)
In-Reply-To: <9455bc5b-2074-4f48-71a7-5c816ee19a78@mailbox.org>
On Fri, Jan 06, 2023 at 12:42:54PM +0100, Michel Dänzer wrote:
> On 1/6/23 02:40, Brian Norris wrote:
> > --- 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.
>
> The out label immediately unlocks vop->vop_lock again, seems a bit pointless. :)
Oops, of course. Will change that in v2.
> AFAICT the self_refresh_active case should just return immediately,
> the out label would also send any pending atomic commit completion
> event, which seems wrong now that the vblank interrupt is left
> enabled.
If I return immediately and drop that completion, I hit a warning:
[ 4.423876] WARNING: CPU: 5 PID: 58 at drivers/gpu/drm/drm_atomic_helper.c:2460 drm_atomic_helper_commit_hw_done+0xe0/0xe8
...
[ 4.424036] Call trace:
[ 4.424039] drm_atomic_helper_commit_hw_done+0xe0/0xe8
[ 4.424045] drm_atomic_helper_commit_tail_rpm+0x58/0x7c
...
I plan to leave it as-is for v2.
> (I also wonder if drm_crtc_vblank_off doesn't take care of
> that already, at least amdgpu doesn't do this explicitly AFAICT)
I'm not sure.
Brian
WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <briannorris@chromium.org>
To: "Michel Dänzer" <michel.daenzer@mailbox.org>
Cc: "kernelci.org bot" <bot@kernelci.org>,
Sandy Huang <hjc@rock-chips.com>,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
linux-rockchip@lists.infradead.org,
Sean Paul <seanpaul@chromium.org>,
stable@vger.kernel.org
Subject: Re: [PATCH 2/2] drm/rockchip: vop: Leave vblank enabled in self-refresh
Date: Fri, 6 Jan 2023 17:21:19 -0800 [thread overview]
Message-ID: <Y7jJDw4RcNceyLbR@google.com> (raw)
In-Reply-To: <9455bc5b-2074-4f48-71a7-5c816ee19a78@mailbox.org>
On Fri, Jan 06, 2023 at 12:42:54PM +0100, Michel Dänzer wrote:
> On 1/6/23 02:40, Brian Norris wrote:
> > --- 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.
>
> The out label immediately unlocks vop->vop_lock again, seems a bit pointless. :)
Oops, of course. Will change that in v2.
> AFAICT the self_refresh_active case should just return immediately,
> the out label would also send any pending atomic commit completion
> event, which seems wrong now that the vblank interrupt is left
> enabled.
If I return immediately and drop that completion, I hit a warning:
[ 4.423876] WARNING: CPU: 5 PID: 58 at drivers/gpu/drm/drm_atomic_helper.c:2460 drm_atomic_helper_commit_hw_done+0xe0/0xe8
...
[ 4.424036] Call trace:
[ 4.424039] drm_atomic_helper_commit_hw_done+0xe0/0xe8
[ 4.424045] drm_atomic_helper_commit_tail_rpm+0x58/0x7c
...
I plan to leave it as-is for v2.
> (I also wonder if drm_crtc_vblank_off doesn't take care of
> that already, at least amdgpu doesn't do this explicitly AFAICT)
I'm not sure.
Brian
next prev parent reply other threads:[~2023-01-07 1:21 UTC|newest]
Thread overview: 41+ 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
2023-01-06 1:40 ` Brian Norris
2023-01-06 1:40 ` [PATCH 2/2] drm/rockchip: vop: Leave vblank enabled in self-refresh Brian Norris
2023-01-06 1:40 ` Brian Norris
2023-01-06 1:40 ` Brian Norris
2023-01-06 11:42 ` Michel Dänzer
2023-01-06 11:42 ` Michel Dänzer
2023-01-07 1:21 ` Brian Norris [this message]
2023-01-07 1:21 ` Brian Norris
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 7:04 ` Greg KH
2023-01-06 7:04 ` Greg KH
2023-01-06 17:49 ` Daniel Vetter
2023-01-06 17:49 ` Daniel Vetter
2023-01-06 17:49 ` Daniel Vetter
2023-01-06 17:53 ` Daniel Vetter
2023-01-06 17:53 ` Daniel Vetter
2023-01-06 17:53 ` Daniel Vetter
2023-01-06 18:08 ` Brian Norris
2023-01-06 18:08 ` Brian Norris
2023-01-06 18:20 ` Daniel Vetter
2023-01-06 18:20 ` Daniel Vetter
2023-01-06 18:20 ` Daniel Vetter
2023-01-06 19:25 ` Brian Norris
2023-01-06 19:25 ` Brian Norris
2023-01-06 18:17 ` Daniel Vetter
2023-01-06 18:17 ` Daniel Vetter
2023-01-06 19:33 ` Brian Norris
2023-01-06 19:33 ` Brian Norris
2023-01-06 20:30 ` Daniel Vetter
2023-01-06 20:30 ` Daniel Vetter
2023-01-06 20:30 ` Daniel Vetter
2023-01-06 21:30 ` Brian Norris
2023-01-06 21:30 ` Brian Norris
2023-01-06 22:44 ` Daniel Vetter
2023-01-06 22:44 ` Daniel Vetter
2023-01-06 22:44 ` Daniel Vetter
2023-01-11 15:03 ` Ville Syrjälä
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=Y7jJDw4RcNceyLbR@google.com \
--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 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.