[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