Re: [BTTV] [FSL P50x0] [PASEMI] TV Time doesn't work anymore after dropping the overlay support

[Deborah Brouwer <deborahbrouwer3563@gmail.com>]

On Thu, Sep 7, 2023 at 8:23 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> Hi Deb,
>
> On 04/09/2023 13:51, Christian Zigotzky wrote:
> > On 05 May 2023 at 09:20 am, Christian Zigotzky wrote:
> >> On 05 May 2023 at 08:45 am, Hans Verkuil wrote:
> >>> On 05/05/2023 08:25, Christian Zigotzky wrote:
> >>>> On 02 May 2023 at 08:57 am, Hans Verkuil wrote:
> >>>>> If v4l2-ctl fails, then try again
> >>>>> after applying this series:
> >>>>>
> >>>>> https://patchwork.linuxtv.org/project/linux-media/cover/cover.1682995256.git.deborah.brouwer@collabora.com/
> >>>> Your patch series solved the issue. Thanks a lot!
> >>> Nice, but somewhat unexpected :-)
> >>>
> >>> I'm a little bit unsure how to proceed: the 6.4 kernel will remove destructive overlay
> >>> support, but it won't have this series yet, that's for 6.5. But that would make 6.4
> >>> unusable for you.
> >>>
> >>> I might have to revert the overlay removal, at least for bttv.
> >>>
> >>> Regards,
> >>>
> >>>     Hans
> >> Hans,
> >>
> >> You don't need to revert the overlay removal because your patch series work with the latest git kernel (6.4).
> >>
> >> Thanks for your help,
> >>
> >> Christian
> >
> > Hello Hans,
> >
> > I successfully used your patches for the kernel 6.5. Everything works without any problems with your patch series from May.
> >
> > Your patches have been added with the latest Media updates [1] for the kernel 6.6.
> >
> > The patches works but I have a green bar in the bottum of the window if I use the composite input. [2]
> >
> > I don't have this green bar with your May patch series.
> >
> > Could you please check your latest patches?
> >
> > Thanks,
> >
> > Christian
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=307d59039fb26212a84a9aa6a134a7d2bdea34ca
> > [2] https://i.ibb.co/D4K6j2c/Kernel-6-6-alpha2-Power-PC.png
> >
> >
>
> After some testing and debugging I found this change in the bttv vb2 conversion
> patch:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/d785cd8b0d8c19c05fcaa1536622e2fdd9f8ede3.1689379982.git.deborah.brouwer@collabora.com/
>
> > diff --git a/drivers/media/pci/bt8xx/bttv-risc.c b/drivers/media/pci/bt8xx/bttv-risc.c
> > index 3e0dac56de54..436baf6c8b08 100644
> > --- a/drivers/media/pci/bt8xx/bttv-risc.c
> > +++ b/drivers/media/pci/bt8xx/bttv-risc.c
> > @@ -67,8 +67,10 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
> >       /* scan lines */
> >       sg = sglist;
> >       for (line = 0; line < store_lines; line++) {
> > -             if ((btv->opt_vcr_hack) &&
> > -                 (line >= (store_lines - VCR_HACK_LINES)))
> > +             if ((line >= (store_lines - VCR_HACK_LINES)) &&
> > +                 (btv->opt_vcr_hack ||
> > +                 (V4L2_FIELD_HAS_BOTH(btv->field) ||
> > +                  btv->field == V4L2_FIELD_ALTERNATE)))
> >                       continue;
> >               while (offset && offset >= sg_dma_len(sg)) {
> >                       offset -= sg_dma_len(sg);
>
> It is this change that causes the issue: basically the condition
> (V4L2_FIELD_HAS_BOTH(btv->field) || btv->field == V4L2_FIELD_ALTERNATE)
> is almost always true (it is only false for FIELD_TOP/BOTTOM), so it is
> as if vcr_hack is always turned on.
>
> It looks to me like some debug code accidentally ended up in this patch.
> Reverting this change makes everything look good again (Christian, it would
> be great if you can confirm that this also fixes the issue for you!).
>
> Deb, can you remember anything about this change?

This is not debug code.  I made the change to fix the latency and
frame drop issues
that were otherwise occurring with vb2; as I said in the cover letter to v3:

"- remove the last four lines in interlaced,
sequential top/bottom, and alternating buffers
to prevent long latency and frame drops
(this is instead of just enabling the existing
VCR hack by default);"
https://lore.kernel.org/linux-media/cover.1689379982.git.deborah.brouwer@collabora.com/

However, if your testing shows that it isn't needed, then it would be
fine to remove this
code and just let the user enable the "vcr hack" as needed.  This was
my original approach
in v2, but I thought you had said at the time that you were seeing
massive framedrops in v2?

I didn't notice this green line before because I was testing in qv4l2
with the default
Pixel Format  : 'BGR3' (24-bit BGR 8-8-8) whereas tvtime is using
YUYV' (YUYV 4:2:2)

One fix that worked for me was to adjust the "OverScan" configuration in tvtime
so that it is at least 3.5.  The /etc/tvtime/tvtime.xml configuration
file recommends
even higher at 8.0.  Christian, please try adjusting the overscan
value to see if
that is a possible solution as well.

Thanks,
Deb

>
> Regards,
>
>         Hans
>
>
>