Hi Laurent,
On 26/06/17 19:12, Laurent Pinchart wrote:
> When the display start interrupt occurs, we know that the hardware has
> finished loading the active display list. The driver then proceeds to
> recycle the list, assuming it won't be needed anymore.
>
> This assumption holds true for headerless display lists, as the VSP
> doesn't reload the list for the next frame if it hasn't changed.
> However, this isn't true anymore for header display lists, as they are
> loaded at every frame start regardless of whether they have been
> updated.
>
> To prepare for header display lists usage in display pipelines, we need
> to postpone recycling the list until it gets replaced by a new one
> through a page flip. The driver already does so in the frame end
> interrupt handler, so all we need is to skip list recycling in the
> display start interrupt handler.
>
> While the active list can be recycled at display start for headerless
> display lists, there's no real harm in postponing that to the frame end
> interrupt handler in all cases. This simplifies interrupt handling as we
> don't need to process the display start interrupt anymore.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
Ok, I had skipped this one as I was concerned about its effects in relation to
11/14 but I see how that's working now.
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/platform/vsp1/vsp1_dl.c | 16 ----------------
> drivers/media/platform/vsp1/vsp1_dl.h | 1 -
> drivers/media/platform/vsp1/vsp1_drm.c | 12 ++++--------
> drivers/media/platform/vsp1/vsp1_drm.h | 2 --
> drivers/media/platform/vsp1/vsp1_drv.c | 8 --------
> 5 files changed, 4 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
> index dc47e236c780..bb92be4fe0f0 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -547,22 +547,6 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
> * Display List Manager
> */
>
> -/* Interrupt Handling */
> -void vsp1_dlm_irq_display_start(struct vsp1_dl_manager *dlm)
> -{
> - spin_lock(&dlm->lock);
> -
> - /*
> - * The display start interrupt signals the end of the display list
> - * processing by the device. The active display list, if any, won't be
> - * accessed anymore and can be reused.
> - */
> - __vsp1_dl_list_put(dlm->active);
> - dlm->active = NULL;
> -
> - spin_unlock(&dlm->lock);
> -}
> -
> /**
> * vsp1_dlm_irq_frame_end - Display list handler for the frame end interrupt
> * @dlm: the display list manager
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.h b/drivers/media/platform/vsp1/vsp1_dl.h
> index 6ec1380a10af..ee3508172f0a 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.h
> +++ b/drivers/media/platform/vsp1/vsp1_dl.h
> @@ -27,7 +27,6 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
> unsigned int prealloc);
> void vsp1_dlm_destroy(struct vsp1_dl_manager *dlm);
> void vsp1_dlm_reset(struct vsp1_dl_manager *dlm);
> -void vsp1_dlm_irq_display_start(struct vsp1_dl_manager *dlm);
> bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm);
>
> struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm);
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
> index 9377aafa8996..bc3fd9bc7126 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -32,11 +32,6 @@
> * Interrupt Handling
> */
>
> -void vsp1_drm_display_start(struct vsp1_device *vsp1)
> -{
> - vsp1_dlm_irq_display_start(vsp1->drm->pipe.output->dlm);
> -}
> -
> static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe)
> {
> struct vsp1_drm *drm = to_vsp1_drm(pipe);
> @@ -224,6 +219,10 @@ int vsp1_du_setup_lif(struct device *dev, const struct vsp1_du_lif_config *cfg)
> return ret;
> }
>
> + /* Disable the display interrupts. */
> + vsp1_write(vsp1, VI6_DISP_IRQ_STA, 0);
> + vsp1_write(vsp1, VI6_DISP_IRQ_ENB, 0);
> +
> dev_dbg(vsp1->dev, "%s: pipeline enabled\n", __func__);
>
> return 0;
> @@ -529,13 +528,10 @@ void vsp1_du_atomic_flush(struct device *dev)
>
> /* Start or stop the pipeline if needed. */
> if (!vsp1->drm->num_inputs && pipe->num_inputs) {
> - vsp1_write(vsp1, VI6_DISP_IRQ_STA, 0);
> - vsp1_write(vsp1, VI6_DISP_IRQ_ENB, VI6_DISP_IRQ_ENB_DSTE);
> spin_lock_irqsave(&pipe->irqlock, flags);
> vsp1_pipeline_run(pipe);
> spin_unlock_irqrestore(&pipe->irqlock, flags);
> } else if (vsp1->drm->num_inputs && !pipe->num_inputs) {
> - vsp1_write(vsp1, VI6_DISP_IRQ_ENB, 0);
> vsp1_pipeline_stop(pipe);
> }
> }
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.h b/drivers/media/platform/vsp1/vsp1_drm.h
> index e9f80727ff92..cbdbb8a39883 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.h
> +++ b/drivers/media/platform/vsp1/vsp1_drm.h
> @@ -50,6 +50,4 @@ int vsp1_drm_init(struct vsp1_device *vsp1);
> void vsp1_drm_cleanup(struct vsp1_device *vsp1);
> int vsp1_drm_create_links(struct vsp1_device *vsp1);
>
> -void vsp1_drm_display_start(struct vsp1_device *vsp1);
> -
> #endif /* __VSP1_DRM_H__ */
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
> index 95c26edead85..6b35e043b554 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -68,14 +68,6 @@ static irqreturn_t vsp1_irq_handler(int irq, void *data)
> }
> }
>
> - status = vsp1_read(vsp1, VI6_DISP_IRQ_STA);
> - vsp1_write(vsp1, VI6_DISP_IRQ_STA, ~status & VI6_DISP_IRQ_STA_DST);
> -
> - if (status & VI6_DISP_IRQ_STA_DST) {
> - vsp1_drm_display_start(vsp1);
> - ret = IRQ_HANDLED;
> - }
> -
> return ret;
> }
>
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
