Re: [PATCH v1.1 2/2] drm: rcar-du: Repair vblank for DRM page flips using the VSP1

2017-07-02 Thread Laurent Pinchart
Hi Morimoto-san,

On Friday 30 Jun 2017 08:32:04 Kuninori Morimoto wrote:
> Hi Kieran
> 
> > -static void rcar_du_vsp_complete(void *private)
> > +static void rcar_du_vsp_complete(void *private, bool completed)
> >  {
> > struct rcar_du_crtc *crtc = private;
> > 
> > -   rcar_du_crtc_finish_page_flip(crtc);
> > +   if (crtc->vblank_enable)
> > +   drm_crtc_handle_vblank(>crtc);
> > +
> > +   if (completed)
> > +   rcar_du_crtc_finish_page_flip(crtc);
> >  }
> 
> Here, this "vblank_enable" flag, timestamp will be update on
> drm_crtc_handle_vblank().
> 
> For example modetest Flip test, if we stop it by Ctrl+C, then, vblank_enable
> will be false, Then, vblank timestamp isn't updated on waiting method on
> drm_atomic_helper_wait_for_vblanks(). Thus we will have timeout error.

I've noticed this issue as well when testing Kieran's patch, and I will fix 
it.

> And, print complete is now indicated on VSP Frame End,
> in interlace input case, print complete will be indicated to user
> on each ODD, EVEN timing.
> 
> Before this patch, for example 1080i@60Hz, print complete indication
> happen in 30Hz.
> After this patch, in interlace case, indication coming 60Hz

Isn't this to be expected ? In 1080i@60Hz the frame rate is 60 frames per 
second, so shouldn't vertical blanking be reported at 60Hz ?

-- 
Regards,

Laurent Pinchart



Re: [PATCH v1.1 2/2] drm: rcar-du: Repair vblank for DRM page flips using the VSP1

2017-06-30 Thread Kuninori Morimoto

Hi Kieran

> -static void rcar_du_vsp_complete(void *private)
> +static void rcar_du_vsp_complete(void *private, bool completed)
>  {
>   struct rcar_du_crtc *crtc = private;
>  
> - rcar_du_crtc_finish_page_flip(crtc);
> + if (crtc->vblank_enable)
> + drm_crtc_handle_vblank(>crtc);
> +
> + if (completed)
> + rcar_du_crtc_finish_page_flip(crtc);
>  }

Here, this "vblank_enable" flag, timestamp will be update on 
drm_crtc_handle_vblank().

For example modetest Flip test, if we stop it by Ctrl+C, then, vblank_enable 
will be false,
Then, vblank timestamp isn't updated on waiting method on 
drm_atomic_helper_wait_for_vblanks().
Thus we will have timeout error.

And, print complete is now indicated on VSP Frame End,
in interlace input case, print complete will be indicated to user
on each ODD, EVEN timing.

Before this patch, for example 1080i@60Hz, print complete indication
happen in 30Hz.
After this patch, in interlace case, indication coming 60Hz

Best regards
---
Kuninori Morimoto


[PATCH v1.1 2/2] drm: rcar-du: Repair vblank for DRM page flips using the VSP1

2017-06-29 Thread Kieran Bingham
A recent change to the frame completion handling has changed the order
in which the vblank timestamps are updated.

To fix this requires handling the vblank events on the frame end event
which comes from the VSP1 driver on Gen3 instead.

Prevent the CRTC IRQ from being enabled on Gen3 hardware and handle
vblank events through the existing rcar_du_vsp_complete() callback.

The addition of this callback was to provide notification of frame
completions in the event of a race. Further extend the DU callback to
provide notification as to whether the frame was completed or not,
allowing the callback to act as a full vblank interrupt notifier.

The VSP frame end interrupt occurs approximately 50 µs earlier than the
DU frame end interrupt, but this should not cause any undue harm.

Fixes: d503a43ac06a ("drm: rcar-du: Register a completion callback with
VSP1")

Signed-off-by: Kieran Bingham 
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c   | 19 ---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h   |  2 ++
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c|  8 ++--
 drivers/media/platform/vsp1/vsp1_drm.c   |  5 +++--
 drivers/media/platform/vsp1/vsp1_drm.h   |  2 +-
 drivers/media/platform/vsp1/vsp1_pipe.c  | 20 ++--
 drivers/media/platform/vsp1/vsp1_pipe.h  |  2 +-
 drivers/media/platform/vsp1/vsp1_video.c |  6 +-
 include/media/vsp1.h |  2 +-
 9 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 9f53a8243941..e6b8eaa13419 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -621,6 +621,7 @@ static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
 
rcar_du_crtc_write(rcrtc, DSRCR, DSRCR_VBCL);
rcar_du_crtc_set(rcrtc, DIER, DIER_FRE);
+   rcrtc->vblank_enable = true;
 
return 0;
 }
@@ -629,6 +630,7 @@ static void rcar_du_crtc_disable_vblank(struct drm_crtc 
*crtc)
 {
struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
 
+   rcrtc->vblank_enable = false;
rcar_du_crtc_clr(rcrtc, DIER, DIER_FRE);
 }
 
@@ -658,10 +660,12 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
rcar_du_crtc_write(rcrtc, DSRCR, status & DSRCR_MASK);
 
if (status & DSSR_FRM) {
+   /*
+* Gen 3 vblank and page flips are handled through the VSP
+* completion handler
+*/
drm_crtc_handle_vblank(>crtc);
-
-   if (rcdu->info->gen < 3)
-   rcar_du_crtc_finish_page_flip(rcrtc);
+   rcar_du_crtc_finish_page_flip(rcrtc);
 
ret = IRQ_HANDLED;
}
@@ -735,6 +739,15 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, 
unsigned int index)
/* Start with vertical blanking interrupt reporting disabled. */
drm_crtc_vblank_off(crtc);
 
+   /*
+* DU with a VSP1 source uses the VSP1 frame completion event to handle
+* vblanking and page flipping events.
+*
+* Do not register the IRQ handler in this instance.
+*/
+   if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
+   return 0;
+
/* Register the interrupt handler. */
if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
irq = platform_get_irq(pdev, index);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index b199ed5adf36..cf5fcc3a3418 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -31,6 +31,7 @@ struct rcar_du_vsp;
  * @mmio_offset: offset of the CRTC registers in the DU MMIO block
  * @index: CRTC software and hardware index
  * @started: whether the CRTC has been started and is running
+ * @vblank_enable: whether vblank events are enabled on this CRTC
  * @event: event to post when the pending page flip completes
  * @flip_wait: wait queue used to signal page flip completion
  * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
@@ -45,6 +46,7 @@ struct rcar_du_crtc {
unsigned int index;
bool started;
 
+   bool vblank_enable;
struct drm_pending_vblank_event *event;
wait_queue_head_t flip_wait;
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index f870445ebc8d..9144d3d50067 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -30,11 +30,15 @@
 #include "rcar_du_kms.h"
 #include "rcar_du_vsp.h"
 
-static void rcar_du_vsp_complete(void *private)
+static void rcar_du_vsp_complete(void *private, bool completed)
 {
struct rcar_du_crtc *crtc = private;
 
-   rcar_du_crtc_finish_page_flip(crtc);
+   if (crtc->vblank_enable)
+   drm_crtc_handle_vblank(>crtc);
+
+   if (completed)
+