On Tue, Aug 27, 2019 at 02:33:34PM -0700, Rob Clark wrote:
> From: Rob Clark <[email protected]>
> 
> First step in re-working the atomic related internal API to prepare for
> async updates pending.. ->wait_flush() is intended to block until there
> is no in-progress flush.
> 
> A crtc_mask is used, rather than an atomic state object, as this will
> later be used for async flush after the atomic state is destroyed.
> 
> This replaces ->wait_for_crtc_commit_done()
> 
> Signed-off-by: Rob Clark <[email protected]>

Hi Rob,
A few nits below, but lgtm overall


> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 11 ++++++-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 17 ++++++----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 17 ++++++----
>  drivers/gpu/drm/msm/msm_atomic.c         | 42 ++++++++++--------------
>  drivers/gpu/drm/msm/msm_kms.h            |  9 +++--
>  5 files changed, 54 insertions(+), 42 deletions(-)
> 

/snip

> index e5aae1645933..639cc88c31a1 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -10,28 +10,6 @@
>  #include "msm_gem.h"
>  #include "msm_kms.h"
>  
> -static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
> -             struct drm_atomic_state *old_state)
> -{
> -     struct drm_crtc *crtc;
> -     struct drm_crtc_state *new_crtc_state;
> -     struct msm_drm_private *priv = old_state->dev->dev_private;
> -     struct msm_kms *kms = priv->kms;
> -     int i;
> -
> -     for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
> -             if (!new_crtc_state->active)
> -                     continue;
> -
> -             if (drm_crtc_vblank_get(crtc))
> -                     continue;
> -
> -             kms->funcs->wait_for_crtc_commit_done(kms, crtc);
> -
> -             drm_crtc_vblank_put(crtc);
> -     }
> -}
> -
>  int msm_atomic_prepare_fb(struct drm_plane *plane,
>                         struct drm_plane_state *new_state)
>  {
> @@ -51,11 +29,28 @@ int msm_atomic_prepare_fb(struct drm_plane *plane,
>       return msm_framebuffer_prepare(new_state->fb, kms->aspace);
>  }
>  
> +/* Get bitmask of crtcs that will need to be flushed.  The bitmask
> + * can be used with for_each_crtc_mask() iterator, to iterate
> + * effected crtcs without needing to preserve the atomic state.
> + */
> +static unsigned get_crtc_mask(struct drm_atomic_state *state)
> +{
> +     struct drm_crtc_state *crtc_state;
> +     struct drm_crtc *crtc;
> +     unsigned i, mask = 0;
> +
> +     for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> +             mask |= BIT(crtc->index);

mask |= drm_crtc_mask(crtc);

> +
> +     return mask;
> +}
> +
>  void msm_atomic_commit_tail(struct drm_atomic_state *state)
>  {
>       struct drm_device *dev = state->dev;
>       struct msm_drm_private *priv = dev->dev_private;
>       struct msm_kms *kms = priv->kms;
> +     unsigned crtc_mask = get_crtc_mask(state);
>  
>       kms->funcs->prepare_commit(kms, state);
>  
> @@ -70,8 +65,7 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state)
>               kms->funcs->commit(kms, state);
>       }
>  
> -     msm_atomic_wait_for_commit_done(dev, state);
> -
> +     kms->funcs->wait_flush(kms, crtc_mask);
>       kms->funcs->complete_commit(kms, state);
>  
>       drm_atomic_helper_commit_hw_done(state);
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index c7588a42635e..8562bbfd5dca 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -34,9 +34,8 @@ struct msm_kms_funcs {
>       void (*prepare_commit)(struct msm_kms *kms, struct drm_atomic_state 
> *state);
>       void (*commit)(struct msm_kms *kms, struct drm_atomic_state *state);
>       void (*complete_commit)(struct msm_kms *kms, struct drm_atomic_state 
> *state);
> -     /* functions to wait for atomic commit completed on each CRTC */
> -     void (*wait_for_crtc_commit_done)(struct msm_kms *kms,
> -                                     struct drm_crtc *crtc);
> +     void (*wait_flush)(struct msm_kms *kms, unsigned crtc_mask);
> +
>       /* get msm_format w/ optional format modifiers from drm_mode_fb_cmd2 */
>       const struct msm_format *(*get_format)(struct msm_kms *kms,
>                                       const uint32_t format,
> @@ -98,4 +97,8 @@ struct msm_mdss {
>  int mdp5_mdss_init(struct drm_device *dev);
>  int dpu_mdss_init(struct drm_device *dev);
>  
> +#define for_each_crtc_mask(dev, crtc, crtc_mask) \
> +     list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head) \

drm_for_each_crtc

> +             for_each_if (BIT((crtc)->index) & (crtc_mask))
> +
>  #endif /* __MSM_KMS_H__ */
> -- 
> 2.21.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno

Reply via email to