Re: [PATCH 3/8] drm/ingenic: Use standard drm_atomic_helper_commit_tail

[Christophe JAILLET]

Le 08/08/2021 à 22:09, Paul Cercueil a écrit :
> Hi Christophe,
>
> Le dim., août 8 2021 at 21:50:04 +0200, Christophe JAILLET 
> <christophe.jaillet@xxxxxxxxxx> a écrit :
> > Le 08/08/2021 à 15:45, Paul Cercueil a écrit :
> > > 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@xxxxxxxxxxxxxxx>
> > > ---
> > >   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);
> > > -}
> >
> > Hi,
> > if this function is removed, shouldn't:
> >   static struct drm_mode_config_helper_funcs 
> > ingenic_drm_mode_config_helpers = {
> >       .atomic_commit_tail = ingenic_drm_atomic_helper_commit_tail,
> >   };
> > be updated as well?
> >
> > I've not seen it in the serie.
>
> It is there though :) At the bottom of this very patch.

My email client played me some tricks, apparently!
Sorry for the noise.

CJ

> > Just my 2v.
> > CJ
>
> Cheers,
> -Paul