Re: [PATCH v2 11/15] v4l: vsp1: Add per-display list internal completion notification support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

On 05/04/18 10:18, Laurent Pinchart wrote:
> Display list completion is already reported to the frame end handler,
> but that mechanism is global to all display lists. In order to implement
> BRU and BRS reassignment in DRM pipelines we will need to commit a
> display list and wait for its completion internally, without reporting
> it to the DRM driver. Extend the display list API to support such an
> internal use of the display list.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
> Changes since v1:
> 
> - Use the frame end status flags to report notification
> - Rename the notify flag to internal

This reads much better to me.

Thanks for the respin.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>


> ---
>  drivers/media/platform/vsp1/vsp1_dl.c    | 23 ++++++++++++++++++++++-
>  drivers/media/platform/vsp1/vsp1_dl.h    |  3 ++-
>  drivers/media/platform/vsp1/vsp1_drm.c   |  2 +-
>  drivers/media/platform/vsp1/vsp1_video.c |  2 +-
>  4 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
> index 662fa2a347c9..30ad491605ff 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -72,6 +72,7 @@ struct vsp1_dl_body {
>   * @fragments: list of extra display list bodies
>   * @has_chain: if true, indicates that there's a partition chain
>   * @chain: entry in the display list partition chain
> + * @internal: whether the display list is used for internal purpose
>   */
>  struct vsp1_dl_list {
>  	struct list_head list;
> @@ -85,6 +86,8 @@ struct vsp1_dl_list {
>  
>  	bool has_chain;
>  	struct list_head chain;
> +
> +	bool internal;
>  };
>  
>  enum vsp1_dl_mode {
> @@ -550,8 +553,16 @@ static void vsp1_dl_list_commit_continuous(struct vsp1_dl_list *dl)
>  	 * case we can't replace the queued list by the new one, as we could
>  	 * race with the hardware. We thus mark the update as pending, it will
>  	 * be queued up to the hardware by the frame end interrupt handler.
> +	 *
> +	 * If a display list is already pending we simply drop it as the new
> +	 * display list is assumed to contain a more recent configuration. It is
> +	 * an error if the already pending list has the internal flag set, as
> +	 * there is then a process waiting for that list to complete. This
> +	 * shouldn't happen as the waiting process should perform proper
> +	 * locking, but warn just in case.
>  	 */
>  	if (vsp1_dl_list_hw_update_pending(dlm)) {
> +		WARN_ON(dlm->pending && dlm->pending->internal);
>  		__vsp1_dl_list_put(dlm->pending);
>  		dlm->pending = dl;
>  		return;
> @@ -581,7 +592,7 @@ static void vsp1_dl_list_commit_singleshot(struct vsp1_dl_list *dl)
>  	dlm->active = dl;
>  }
>  
> -void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
> +void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal)
>  {
>  	struct vsp1_dl_manager *dlm = dl->dlm;
>  	struct vsp1_dl_list *dl_child;
> @@ -598,6 +609,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
>  		}
>  	}
>  
> +	dl->internal = internal;
> +
>  	spin_lock_irqsave(&dlm->lock, flags);
>  
>  	if (dlm->singleshot)
> @@ -624,6 +637,10 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
>   * raced with the frame end interrupt. The function always returns with the flag
>   * set in header mode as display list processing is then not continuous and
>   * races never occur.
> + *
> + * The VSP1_DL_FRAME_END_INTERNAL flag indicates that the previous display list
> + * has completed and had been queued with the internal notification flag.
> + * Internal notification is only supported for continuous mode.
>   */
>  unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
>  {
> @@ -656,6 +673,10 @@ unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
>  	 * frame end interrupt. The display list thus becomes active.
>  	 */
>  	if (dlm->queued) {
> +		if (dlm->queued->internal)
> +			flags |= VSP1_DL_FRAME_END_INTERNAL;
> +		dlm->queued->internal = false;
> +
>  		__vsp1_dl_list_put(dlm->active);
>  		dlm->active = dlm->queued;
>  		dlm->queued = NULL;
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.h b/drivers/media/platform/vsp1/vsp1_dl.h
> index cbc2fc53e10b..1a5bbd5ddb7b 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.h
> +++ b/drivers/media/platform/vsp1/vsp1_dl.h
> @@ -21,6 +21,7 @@ struct vsp1_dl_list;
>  struct vsp1_dl_manager;
>  
>  #define VSP1_DL_FRAME_END_COMPLETED		BIT(0)
> +#define VSP1_DL_FRAME_END_INTERNAL		BIT(1)
>  
>  void vsp1_dlm_setup(struct vsp1_device *vsp1);
>  
> @@ -34,7 +35,7 @@ unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm);
>  struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm);
>  void vsp1_dl_list_put(struct vsp1_dl_list *dl);
>  void vsp1_dl_list_write(struct vsp1_dl_list *dl, u32 reg, u32 data);
> -void vsp1_dl_list_commit(struct vsp1_dl_list *dl);
> +void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal);
>  
>  struct vsp1_dl_body *vsp1_dl_fragment_alloc(struct vsp1_device *vsp1,
>  					    unsigned int num_entries);
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
> index 541473b1df67..68b126044ea1 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -370,7 +370,7 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
>  		}
>  	}
>  
> -	vsp1_dl_list_commit(dl);
> +	vsp1_dl_list_commit(dl, false);
>  }
>  
>  /* -----------------------------------------------------------------------------
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
> index 4152704c2ccb..a76a44698aff 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -437,7 +437,7 @@ static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
>  	}
>  
>  	/* Complete, and commit the head display list. */
> -	vsp1_dl_list_commit(pipe->dl);
> +	vsp1_dl_list_commit(pipe->dl, false);
>  	pipe->dl = NULL;
>  
>  	vsp1_pipeline_run(pipe);
> 



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux