Hi Hans,
On Fri, May 11, 2018 at 12:28:55PM +0200, Hans Verkuil wrote:
> On 05/11/18 11:59, Jacopo Mondi wrote:
> > Since commit (015060cb "media: rcar-vin: enable field toggle after a set
> > number of lines for Gen3) the VIN generates an internal field signal
> > toggle after a fixed number of received lines, and uses the internal
> > field signal to drive capture operations. When capturing from digital
> > input, using FTEH driven field signal toggling messes up the received
> > image sizes. Fall back to use FTEV driven signal toggling when capturing
> > from digital input.
> >
> > As explained in the comment, this disables buffer overflow protection
> > for digital input capture, for which the FOE overflow might be used in
> > future.
>
> I don't know the details of the hardware, but this sounds dangerous.
>
> You should know that with HDMI input it is perfectly possible that you get
> more data than you should. I.e. instead of 1080 lines (assuming full HD)
> you might get more lines. This happens if the vertical sync is missed due
> to pin bounce when connecting a source.
>
> Other reasons for this are flaky signals, bad clocks, etc.
>
> It's rare, but it really happens.
>
> A good DMA engine will refuse to write more than fits in the buffer.
>
> If you disable that, then you will get overflows eventually.
>
> The reality with HDMI input is that you should never assume clean valid
> data. You do not control the source and it can send anything it likes.
Thanks for the informations. I agree HDMI input is lot of fun (-.-)
and we've seen weird things happening too.
With the patch Niklas has just sent that fixes the crop/compose
rectangle, also the previously in-place protection agains overflow has
been reverted, so this patch is not required anymore.
I re-send re-basing this on top of Niklas' latest fix.
Thanks
j
>
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> > ---
> > drivers/media/platform/rcar-vin/rcar-dma.c | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index ea7a120..8dc3455 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -685,11 +685,27 @@ static int rvin_setup(struct rvin_dev *vin)
> > break;
> > }
> >
> > - if (vin->info->model == RCAR_GEN3) {
> > + if (vin->info->model == RCAR_GEN3 &&
> > + vin->mbus_cfg.type == V4L2_MBUS_CSI2) {
> > /* Enable HSYNC Field Toggle mode after height HSYNC inputs. */
> > lines = vin->format.height / (halfsize ? 2 : 1);
> > dmr2 = VNDMR2_FTEH | VNDMR2_HLV(lines);
> > vin_dbg(vin, "Field Toogle after %u lines\n", lines);
>
> Typo: Toogle -> Toggle
>
> > + } else if (vin->info->model == RCAR_GEN3 &&
> > + vin->mbus_cfg.type == V4L2_MBUS_PARALLEL) {
> > + /*
> > + * FIXME
> > + * Section 26.3.17 specifies that for digital input there's no
> > + * need to program FTEH or FTEV to generate internal
> > + * field toggle signal to driver capture. Although when
> > + * running on GEN3 with digital input no EFE interrupt is ever
> > + * generated, and we need to rely on FTEV driven field signal
> > + * toggling, as using FTEH as in the CSI-2 case, messes up
> > + * the output image size. This implies no protection
> > + * against buffer overflow is in place for Gen3 digital input
> > + * capture.
> > + */
> > + dmr2 = VNDMR2_FTEV;
> > } else {
> > /* Enable VSYNC Field Toogle mode after one VSYNC input. */
>
> Ditto. Search and replace?
>
> > dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
> >
>
> Regards,
>
> Hans
Attachment:
signature.asc
Description: PGP signature
