Hi Laurent, On 17/07/18 14:51, Laurent Pinchart wrote: > 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. I've done some testing with this (removing the DSMR change, and inverting the VI6_STATUS_FLD_STD handling) and had some odd results. Perhaps my testing needs refinement. So, yes please - I think I'd really like to know what the effects are on a Lager platform. Would you (or anyone with a Gen2 and interest in vsp1/du) be able to test my latest vsp1/du/interlaced branch/tag on your local Gen2 platform please? I'm testing interlaced with: kmstest # sanity test. kmstest -c 1 -r 1920x1080i --flip Any (easy) other methods for testing interlaced pipelines are welcome. Is it possible to set the mode for kmscube? (--help doesn't look promising) I have various test streams of interlaced media content in my media library, but not an easy way of decoding and presenting these on the screen on the Gen3. I believe GStreamer now has a drm/kms sink ... Perhaps I should get that recompiled. (That would help me with other tasks too actually) > >>>> | 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; >
