Re: [RFC PATCH v2 3/4] drm/msm/dpu: Remove empty prepare_commit() function

[Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>]

On 2/9/2023 10:51 AM, Dmitry Baryshkov wrote:
> On 09/02/2023 20:44, Jessica Zhang wrote:
> > Now that the TE setup has been moved to prepare_for_kickoff(),
> > move empty prepare_commit() from DPU driver.
>
> s/move/remove/
>
> Also the DPU's prepare_commit() is not empty. Please mention 
> dpu_encoder_prepare_commit() becoming (or being) empty.

Hi Dmitry,

Acked.

> > Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 19 -----------------
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h   |  7 -------
> >   .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  |  6 ------
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       | 21 -------------------
> >   4 files changed, 53 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index dcceed91aed8..35e120b5ef53 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -2090,25 +2090,6 @@ void dpu_encoder_helper_phys_cleanup(struct 
> > dpu_encoder_phys *phys_enc)
> >       ctl->ops.clear_pending_flush(ctl);
> >   }
> > -void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc)
> > -{
> > -    struct dpu_encoder_virt *dpu_enc;
> > -    struct dpu_encoder_phys *phys;
> > -    int i;
> > -
> > -    if (!drm_enc) {
> > -        DPU_ERROR("invalid encoder\n");
> > -        return;
> > -    }
> > -    dpu_enc = to_dpu_encoder_virt(drm_enc);
> > -
> > -    for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> > -        phys = dpu_enc->phys_encs[i];
> > -        if (phys->ops.prepare_commit)
> > -            phys->ops.prepare_commit(phys);
> > -    }
> > -}
> > -
> >   #ifdef CONFIG_DEBUG_FS
> >   static int _dpu_encoder_status_show(struct seq_file *s, void *data)
> >   {
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > index 9e7236ef34e6..2c9ef8d1b877 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > @@ -146,13 +146,6 @@ struct drm_encoder *dpu_encoder_init(
> >   int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
> >           struct msm_display_info *disp_info);
> > -/**
> > - * dpu_encoder_prepare_commit - prepare encoder at the very beginning 
> > of an
> > - *    atomic commit, before any registers are written
> > - * @drm_enc:    Pointer to previously created drm encoder structure
> > - */
> > -void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc);
> > -
> >   /**
> >    * dpu_encoder_set_idle_timeout - set the idle timeout for video
> >    *                    and command mode encoders.
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> > index 561406d92a1a..4c189b9fa9b8 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> > @@ -703,11 +703,6 @@ static void 
> > dpu_encoder_phys_cmd_prepare_for_kickoff(
> >               atomic_read(&phys_enc->pending_kickoff_cnt));
> >   }
> > -static void dpu_encoder_phys_cmd_prepare_commit(
> > -        struct dpu_encoder_phys *phys_enc)
> > -{
> > -}
> > -
> >   static int _dpu_encoder_phys_cmd_wait_for_ctl_start(
> >           struct dpu_encoder_phys *phys_enc)
> >   {
> > @@ -807,7 +802,6 @@ static void dpu_encoder_phys_cmd_trigger_start(
> >   static void dpu_encoder_phys_cmd_init_ops(
> >           struct dpu_encoder_phys_ops *ops)
> >   {
> > -    ops->prepare_commit = dpu_encoder_phys_cmd_prepare_commit;
>
> Also please don't forget to remove prepare_commit from 
> dpu_encoder_phys_ops.

Acked.

Thanks,

Jessica Zhang

> >       ops->is_master = dpu_encoder_phys_cmd_is_master;
> >       ops->atomic_mode_set = dpu_encoder_phys_cmd_atomic_mode_set;
> >       ops->enable = dpu_encoder_phys_cmd_enable;
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index 165958d47ec6..6f7ddbf0d9b7 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -425,26 +425,6 @@ static ktime_t dpu_kms_vsync_time(struct msm_kms 
> > *kms, struct drm_crtc *crtc)
> >       return ktime_get();
> >   }
> > -static void dpu_kms_prepare_commit(struct msm_kms *kms,
> > -        struct drm_atomic_state *state)
> > -{
> > -    struct drm_crtc *crtc;
> > -    struct drm_crtc_state *crtc_state;
> > -    struct drm_encoder *encoder;
> > -    int i;
> > -
> > -    if (!kms)
> > -        return;
> > -
> > -    /* Call prepare_commit for all affected encoders */
> > -    for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > -        drm_for_each_encoder_mask(encoder, crtc->dev,
> > -                      crtc_state->encoder_mask) {
> > -            dpu_encoder_prepare_commit(encoder);
> > -        }
> > -    }
> > -}
> > -
> >   static void dpu_kms_flush_commit(struct msm_kms *kms, unsigned 
> > crtc_mask)
> >   {
> >       struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> > @@ -949,7 +929,6 @@ static const struct msm_kms_funcs kms_funcs = {
> >       .enable_commit   = dpu_kms_enable_commit,
> >       .disable_commit  = dpu_kms_disable_commit,
> >       .vsync_time      = dpu_kms_vsync_time,
> > -    .prepare_commit  = dpu_kms_prepare_commit,
> >       .flush_commit    = dpu_kms_flush_commit,
> >       .wait_flush      = dpu_kms_wait_flush,
> >       .complete_commit = dpu_kms_complete_commit,
>
> --
> With best wishes
> Dmitry