Messages in this thread | | | Date | Tue, 10 Aug 2021 12:46:05 +0200 | From | Paul Cercueil <> | 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@ffwll.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@crapouillou.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@ffwll.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
| |