[PATCH] drm/komeda: Fix handling of atomic commits in the atomic_commit_tail hook
Carsten Haitzler
carsten.haitzler at foss.arm.com
Mon Aug 1 10:26:36 UTC 2022
Looks good to me - as per log.
On 7/22/22 13:21, Liviu Dudau wrote:
> Komeda driver relies on the generic DRM atomic helper functions to handle
> commits. It only implements an atomic_commit_tail hook for the
> mode_config_helper_funcs and even that one is pretty close to the generic
> implementation with the exception of additional dma_fence signalling.
>
> What the generic helper framework doesn't do is waiting for the actual
> hardware to signal that the commit parameters have been written into the
> appropriate registers. As we signal CRTC events only on the irq handlers,
> we need to flush the configuration and wait for the hardware to respond.
>
> Add the Komeda specific implementation for atomic_commit_hw_done() that
> flushes and waits for flip done before calling drm_atomic_helper_commit_hw_done().
>
> The fix was prompted by a patch from Carsten Haitzler where he was trying to
> solve the same issue but in a different way that I think can lead to wrong
> event signaling to userspace.
>
> Reported-by: Carsten Haitzler <carsten.haitzler at arm.com>
> Tested-by: Carsten Haitzler <carsten.haitzler at arm.com>
> Signed-off-by: Liviu Dudau <liviu.dudau at arm.com>
> ---
> .../gpu/drm/arm/display/komeda/komeda_crtc.c | 4 ++--
> .../gpu/drm/arm/display/komeda/komeda_kms.c | 21 ++++++++++++++++++-
> .../gpu/drm/arm/display/komeda/komeda_kms.h | 2 ++
> 3 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 59172acb9738..292f533d8cf0 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -235,7 +235,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
> crtc->state->event = NULL;
> drm_crtc_send_vblank_event(crtc, event);
> } else {
> - DRM_WARN("CRTC[%d]: FLIP happen but no pending commit.\n",
> + DRM_WARN("CRTC[%d]: FLIP happened but no pending commit.\n",
> drm_crtc_index(&kcrtc->base));
> }
> spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> @@ -286,7 +286,7 @@ komeda_crtc_atomic_enable(struct drm_crtc *crtc,
> komeda_crtc_do_flush(crtc, old);
> }
>
> -static void
> +void
> komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc,
> struct completion *input_flip_done)
> {
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index 93b7f09b96ca..327051bba5b6 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -69,6 +69,25 @@ static const struct drm_driver komeda_kms_driver = {
> .minor = 1,
> };
>
> +static void komeda_kms_atomic_commit_hw_done(struct drm_atomic_state *state)
> +{
> + struct drm_device *dev = state->dev;
> + struct komeda_kms_dev *kms = to_kdev(dev);
> + int i;
> +
> + for (i = 0; i < kms->n_crtcs; i++) {
> + struct komeda_crtc *kcrtc = &kms->crtcs[i];
> +
> + if (kcrtc->base.state->active) {
> + struct completion *flip_done = NULL;
> + if (kcrtc->base.state->event)
> + flip_done = kcrtc->base.state->event->base.completion;
> + komeda_crtc_flush_and_wait_for_flip_done(kcrtc, flip_done);
> + }
> + }
> + drm_atomic_helper_commit_hw_done(state);
> +}
> +
> static void komeda_kms_commit_tail(struct drm_atomic_state *old_state)
> {
> struct drm_device *dev = old_state->dev;
> @@ -81,7 +100,7 @@ static void komeda_kms_commit_tail(struct drm_atomic_state *old_state)
>
> drm_atomic_helper_commit_modeset_enables(dev, old_state);
>
> - drm_atomic_helper_commit_hw_done(old_state);
> + komeda_kms_atomic_commit_hw_done(old_state);
>
> drm_atomic_helper_wait_for_flip_done(dev, old_state);
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> index 7889e380ab23..7339339ef6b8 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> @@ -183,6 +183,8 @@ void komeda_kms_cleanup_private_objs(struct komeda_kms_dev *kms);
>
> void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
> struct komeda_events *evts);
> +void komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc,
> + struct completion *input_flip_done);
>
> struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev);
> void komeda_kms_detach(struct komeda_kms_dev *kms);
More information about the dri-devel
mailing list