From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v3 3/4] drm/vblank: estimate vblank while disabling vblank if interrupt disabled Date: Wed, 26 Jun 2019 16:30:25 +0200 Message-ID: <20190626143025.GN12905@phenom.ffwll.local> References: <20190626132732.GP5942@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20190626132732.GP5942@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: Robert Beckett , Maxime Ripard , Shawn Guo , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, David Airlie , NXP Linux Team , Pengutronix Kernel Team , Sean Paul , linux-arm-kernel@lists.infradead.org List-Id: dri-devel@lists.freedesktop.org On Wed, Jun 26, 2019 at 04:27:32PM +0300, Ville Syrj=E4l=E4 wrote: > On Tue, Jun 25, 2019 at 06:59:14PM +0100, Robert Beckett wrote: > > If interrupts are disabled (e.g. via vblank_disable_fn) and we come to > > disable vblank, update the vblank count to best guess as to what it > > would be had the interrupts remained enabled, and update the timesamp to > > now. > > = > > This avoids a stale vblank event being sent while disabling crtcs during > > atomic modeset. > > = > > Fixes: 68036b08b91bc ("drm/vblank: Do not update vblank count if interr= upts > > are already disabled.") > = > I don't understand that commit. drm_vblank_off() should be called > when the power is still present, so it looks to me like that > commit is actually wrong. So I think we may want to just revert > it and figure out what the actual bug was. Hm yeah we might need a power domain get/put around our drm_crtc_vblank_off() call to make sure it dtrt. Revert sounds like a good idea instead of adding more kludges ... a-b: me on the revert, even though I did ack the original patch too. -Daniel > = > > = > > Signed-off-by: Robert Beckett > > --- > > drivers/gpu/drm/drm_vblank.c | 16 +++++++++++++++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > = > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > > index 7dabb2bdb733..db68b8cbf797 100644 > > --- a/drivers/gpu/drm/drm_vblank.c > > +++ b/drivers/gpu/drm/drm_vblank.c > > @@ -375,9 +375,23 @@ void drm_vblank_disable_and_save(struct drm_device= *dev, unsigned int pipe) > > * interrupts were enabled. This avoids calling the ->disable_vblank() > > * operation in atomic context with the hardware potentially runtime > > * suspended. > > + * If interrupts are disabled (e.g. via blank_disable_fn) then make > > + * best guess as to what it would be now and make sure we have an up > > + * to date timestamp. > > */ > > - if (!vblank->enabled) > > + if (!vblank->enabled) { > > + ktime_t now =3D ktime_get(); > > + u32 diff =3D 0; > > + if (vblank->framedur_ns) { > > + u64 diff_ns =3D ktime_to_ns(ktime_sub(now, vblank->time)); > > + diff =3D DIV_ROUND_CLOSEST_ULL(diff_ns, > > + vblank->framedur_ns); > > + } > > + > > + store_vblank(dev, pipe, diff, now, vblank->count); > > + > > goto out; > > + } > > = > > /* > > * Update the count and timestamp to maintain the > > -- = > > 2.18.0 > > = > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > = > -- = > Ville Syrj=E4l=E4 > Intel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- = Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch