lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 10 Aug 2021 12:46:05 +0200 From: Paul Cercueil <paul@...pouillou.net> To: Daniel Vetter <daniel@...ll.ch> Cc: David Airlie <airlied@...ux.ie>, "H . Nikolaus Schaller" <hns@...delico.com>, Paul Boddie <paul@...die.org.uk>, list@...ndingux.net, Sam Ravnborg <sam@...nborg.org>, linux-mips@...r.kernel.org, dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH 3/8] drm/ingenic: Use standard drm_atomic_helper_commit_tail Hi Daniel, Le mar., août 10 2021 at 12:20:38 +0200, Daniel Vetter <daniel@...ll.ch> a écrit : > On Sun, Aug 08, 2021 at 03:45:21PM +0200, Paul Cercueil wrote: >> By making the CRTC's .vblank_enable() function return an error when >> it >> is known that the hardware won't deliver a VBLANK, we can drop the >> ingenic_drm_atomic_helper_commit_tail() function and use the >> standard >> drm_atomic_helper_commit_tail() function instead. >> >> Signed-off-by: Paul Cercueil <paul@...pouillou.net> >> --- >> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 28 >> ++++------------------- >> 1 file changed, 4 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c >> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c >> index bc71ba44ccf4..3ed7c27a8dde 100644 >> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c >> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c >> @@ -706,29 +706,6 @@ static int >> ingenic_drm_encoder_atomic_check(struct drm_encoder *encoder, >> } >> } >> >> -static void ingenic_drm_atomic_helper_commit_tail(struct >> drm_atomic_state *old_state) >> -{ >> - /* >> - * Just your regular drm_atomic_helper_commit_tail(), but only >> calls >> - * drm_atomic_helper_wait_for_vblanks() if priv->no_vblank. >> - */ >> - struct drm_device *dev = old_state->dev; >> - struct ingenic_drm *priv = drm_device_get_priv(dev); >> - >> - drm_atomic_helper_commit_modeset_disables(dev, old_state); >> - >> - drm_atomic_helper_commit_planes(dev, old_state, 0); >> - >> - drm_atomic_helper_commit_modeset_enables(dev, old_state); >> - >> - drm_atomic_helper_commit_hw_done(old_state); >> - >> - if (!priv->no_vblank) >> - drm_atomic_helper_wait_for_vblanks(dev, old_state); >> - >> - drm_atomic_helper_cleanup_planes(dev, old_state); >> -} >> - >> static irqreturn_t ingenic_drm_irq_handler(int irq, void *arg) >> { >> struct ingenic_drm *priv = drm_device_get_priv(arg); >> @@ -749,6 +726,9 @@ static int ingenic_drm_enable_vblank(struct >> drm_crtc *crtc) >> { >> struct ingenic_drm *priv = drm_crtc_get_priv(crtc); >> >> + if (priv->no_vblank) >> + return -EWOULDBLOCK; > > I think other drivers return EINVAL here. I'm not sure exactly this is > specified, but the errno here is visible to userspace. > > Maybe would be good to update the kerneldoc for this hook? > > Anyway this is great, with the errno fixed: > > Reviewed-by: Daniel Vetter <daniel.vetter@...ll.ch> I thought about it afterwards, that my error code wasn't great. In my mind it was "the driver will hang while waiting for vblank" hence -EWOULDBLOCK ;) I'll need to v2 anyway so I'll change it to -EINVAL then. Cheers, -Paul > >> + >> regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, >> JZ_LCD_CTRL_EOF_IRQ, JZ_LCD_CTRL_EOF_IRQ); >> >> @@ -856,7 +836,7 @@ static const struct drm_mode_config_funcs >> ingenic_drm_mode_config_funcs = { >> }; >> >> static struct drm_mode_config_helper_funcs >> ingenic_drm_mode_config_helpers = { >> - .atomic_commit_tail = ingenic_drm_atomic_helper_commit_tail, >> + .atomic_commit_tail = drm_atomic_helper_commit_tail, >> }; >> >> static void ingenic_drm_unbind_all(void *d) >> -- >> 2.30.2 >> > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Powered by blists - more mailing lists