All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomasz Figa <tomasz.figa@gmail.com>
To: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: linux-samsung-soc@vger.kernel.org,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>
Subject: Re: s5p-fimc VIDIOC_STREAMOFF bug
Date: Sat, 21 Apr 2012 17:22:07 +0200	[thread overview]
Message-ID: <1671013.6k1K925o3Y@flatron> (raw)
In-Reply-To: <4F91A9E6.3060402@samsung.com>

Hi Sylwester,

Thanks for your response.

On Friday 20 of April 2012 20:24:38 Sylwester Nawrocki wrote:
> Hi Tomasz,
> 
> On 04/19/2012 11:45 PM, Tomasz Figa wrote:
> > Hi,
> > 
> > I have been working on adapting s5p-fimc driver for s3c6410 and
> > everything seems to be working just fine after some minor changes
> > (except minor loss of functionality - only codec path is supported,
> > but for most use cases it does not matter).
> > 
> > However I think that I have spotted a bug in capture stop / capture
> > suspend handling. In fimc_capture_state_cleanup() the
> > ST_CAPT_SUSPENDED status bit of fimc->state field is being set
> > regardless of suspend parameter, which confuses the driver that FIMC
> > is suspended and might not accept buffers into active queue and so
> > the driver will never start the capture process unless the device
> > gets closed and reopened (because of the condition checking the
> > count of active buffers).
> > 
> > In my fork for s3c6410 I have moved the set_bit call into
> > fimc_capture_suspend(), so the bit gets set only when the device gets
> > suspended. This seems to solve the problem and I do not see any
> > issues that this could introduce, so it might be a good solution.
> > 
> > Let me know if I am wrong in anything I have written.
> 
> Your conclusions are correct, there was indeed a bug like that.
> There is already a patch fixing this [1], but it is going to be available
> just from v3.4. I'm considering sending it to Greg for inclusion in the
> stable releases, after it gets upstream.

OK, I must have missed the patch. Good that it has been already noticed and 
fixed.

> Once I did some preliminary work for the s3c-fimc driver, but dropped
> this due to lack of time. If you ever decide you want to mainline your
> work, just send the patches to linux-media@vger.kernel.org (and perhaps
> also to samsung-soc and ARM Linux) for review.

I am not sure sure if there is any interest for more advanced support of 
S3C6410 going into mainline, but if such shows up I am ready to cooperate.

> 
> > Best regards,
> > Tomasz Figa
> 
> [1] http://patchwork.linuxtv.org/patch/10417
> 
> 
> Thanks,

      reply	other threads:[~2012-04-21 15:22 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-19 21:45 s5p-fimc VIDIOC_STREAMOFF bug Tomasz Figa
2012-04-20 18:24 ` Sylwester Nawrocki
2012-04-21 15:22   ` Tomasz Figa [this message]

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=1671013.6k1K925o3Y@flatron \
    --to=tomasz.figa@gmail.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=s.nawrocki@samsung.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.