| Message ID | 20170626181226.29575-3-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
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@ideasonboard.com> 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@ideasonboard.com> > --- > 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; > } > >
Em Thu, 13 Jul 2017 18:02:20 +0100 Kieran Bingham <kieran.bingham@ideasonboard.com> escreveu: > 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@ideasonboard.com> > > 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@ideasonboard.com> Acked-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> Thanks, Mauro
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; }
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@ideasonboard.com> --- 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(-)