On Tue, Aug 27, 2019 at 02:33:34PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@xxxxxxxxxxxx>
>
> 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 <robdclark@xxxxxxxxxxxx>
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
