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

[Christophe JAILLET <christophe.jaillet@wanadoo.fr>]

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@wanadoo.fr> 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@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,
>>>       }
>>>   }
>>>   \x7f-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
> 
> 
>