All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shuah Khan <shuahkh@osg.samsung.com>
To: Devin Heitmueller <dheitmueller@kernellabs.com>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Prabhakar Lad <prabhakar.csengg@gmail.com>,
	"sakari.ailus@linux.intel.com" <sakari.ailus@linux.intel.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Tim Mester <ttmesterr@gmail.com>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 3/3] media: au0828 remove video and vbi buffer timeout work-around
Date: Tue, 13 Jan 2015 07:12:56 -0700	[thread overview]
Message-ID: <54B527E8.3080004@osg.samsung.com> (raw)
In-Reply-To: <CAGoCfixdyOJoyUQfMWzM2KHjMGJE5pRS8C0+rR1nDCir7jTpwQ@mail.gmail.com>

On 01/12/2015 09:44 PM, Devin Heitmueller wrote:
> Hi Shuah,
> 
> On Mon, Jan 12, 2015 at 9:56 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> au0828 does video and vbi buffer timeout handling to prevent
>> applications such as tvtime from hanging by ensuring that the
>> video frames continue to be delivered even when the ITU-656
>> input isn't receiving any data. This work-around is complex
>> as it introduces set and clear timer code paths in start/stop
>> streaming, and close interfaces. However, tvtime will hang
>> without the following tvtime change:
> 
> I'm confused.  When we last debated whether this patch would be
> accepted, the last message from Mauro said the following:
> 
>> That means that we'll need to keep holding such timeout code for
>> years, until all distros update to a new tvtime, of course assuming
>> that this is the only one application with such issue.
> 
> In other words, the timeout code has to stay in there since otherwise
> it will cause ABI breakage.  It's great that Hans has submitted a
> patch to improve tvtime, and over the next couple of years that patch
> might actually start to appear in distributions.  That unfortunately
> doesn't change the fact that everybody who updates their kernel (or
> has it updated for them as part of their distribution) will go from
> "works fine" to "completely broken".
> 
> The driver was working before the VB2 conversion, so if there is now
> instability then it's likely that some bug was introduced during the
> conversion to VB2.  Simply ripping out the timeout code seems like the
> wrong approach to addressing a regression likely caused by your own
> VB2 conversion patch.
> 

I couldn't reproduce what I was seeing when I did patch v2 series
work. What I noticed was that I was seeing a few too many green screens
and I had to re-tune xawtv when the timeout code is in place. My
thinking was that this timeout handling could be introducing blank
green frames when there is no need. However, I can't reproduce the
problem on 3.19-rc4 base which is what I am using to test the changes
to the patch series. Hence, I am not positive if the timeout code
indeed was doing anything bad.

I am seeing tvtime hangs without the timeout. I am fine with this
patch not going. It does make the code cleaner and also reduces
buffer handling during streaming. However, there is a clear regression
to tvtime.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

  reply	other threads:[~2015-01-13 14:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-13  2:56 [PATCH v3 0/3] au0828 vb2 conversion Shuah Khan
2015-01-13  2:56 ` [PATCH v3 1/3] media: fix au0828 compile error from au0828_boards initialization Shuah Khan
2015-01-22 12:44   ` Lad, Prabhakar
2015-01-13  2:56 ` [PATCH v3 2/3] media: au0828 - convert to use videobuf2 Shuah Khan
2015-01-22 12:41   ` Lad, Prabhakar
2015-01-22 15:00     ` Devin Heitmueller
2015-01-22 15:05       ` Shuah Khan
2015-01-22 16:16     ` Shuah Khan
2015-01-22 20:47       ` Lad, Prabhakar
2015-01-22 22:27         ` Shuah Khan
2015-01-13  2:56 ` [PATCH v3 3/3] media: au0828 remove video and vbi buffer timeout work-around Shuah Khan
2015-01-13  4:44   ` Devin Heitmueller
2015-01-13 14:12     ` Shuah Khan [this message]
2015-01-13 14:19       ` Devin Heitmueller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54B527E8.3080004@osg.samsung.com \
    --to=shuahkh@osg.samsung.com \
    --cc=dheitmueller@kernellabs.com \
    --cc=hans.verkuil@cisco.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=m.chehab@samsung.com \
    --cc=prabhakar.csengg@gmail.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=ttmesterr@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.