Hi Kieran, On Monday, 16 July 2018 20:20:30 EEST Kieran Bingham wrote: > On 24/05/18 12:50, Laurent Pinchart wrote: > > On Thursday, 3 May 2018 16:36:22 EEST Kieran Bingham wrote: > >> Use the newly exposed VSP1 interface to enable interlaced frame support > >> through the VSP1 lif pipelines. > > > > s/lif/LIF/ > > Fixed. > > >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > >> --- > >> > >> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 1 + > >> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +++ > >> 2 files changed, 4 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > >> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index d71d709fe3d9..206532959ec9 > >> 100644 > >> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > >> @@ -289,6 +289,7 @@ static void rcar_du_crtc_set_display_timing(struct > >> rcar_du_crtc *rcrtc) > >> /* Signal polarities */ > >> value = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0) > >> | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DSMR_HSL : 0) > >> + | ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? DSMR_ODEV : 0) > > > > How will this affect Gen2 ?. > > The bit is documented identically for Gen2. Potentially / Probably it > 'might' reverse the fields... I'm not certain yet. I don't have access > to a Gen2 platform to test. > > I'll see if this change can be dropped, but I think it is playing a role > in ensuring that the field detection occurs in VSP1 through the > VI6_STATUS_FLD_STD() field. (see vsp1_dlm_irq_frame_end()) > > > Could you document what this change does in the > > commit message ? > > This sets the position in the buffer of the ODDF. With this set, the ODD > field is located in the second half (BOTTOM) of the same frame of the > interlace display. > > Otherwise, it's in the first half (TOP) > > I faced some issues as to the ordering when testing, so I suspect this > might actually be related to that. (re VI6_STATUS_FLD_STD in > vsp1_dlm_irq_frame_end()). > > As you mention - this may have a negative effect on the Gen2 > implementation - so it needs considering with that. > > > /me to investigate further. Thank you. I don't object to this change, but I'd like to know what its implications are on Gen2. It might even fix a bug :-) Let me know if you'd like me to run tests on a Lager board. > >> | DSMR_DIPM_DISP | DSMR_CSPM; > >> > >> rcar_du_crtc_write(rcrtc, DSMR, value); > >> > >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > >> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index af7822a66dee..c7b37232ee91 > >> 100644 > >> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > >> @@ -186,6 +186,9 @@ static void rcar_du_vsp_plane_setup(struct > >> rcar_du_vsp_plane *plane) > >> }; > >> unsigned int i; > >> > >> + cfg.interlaced = !!(plane->plane.state->crtc->mode.flags > >> + & DRM_MODE_FLAG_INTERLACE); > >> + > >> cfg.src.left = state->state.src.x1 >> 16; > >> cfg.src.top = state->state.src.y1 >> 16; > >> cfg.src.width = drm_rect_width(&state->state.src) >> 16; -- Regards, Laurent Pinchart