cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Thu Dec 3 04:00:22 CET 2015 git branch: test git hash: 10897dacea26943dd80bd6629117f4620fc320ef gcc version:i686-linux-gcc (GCC) 5.1.0 sparse version: v0.5.0 smatch version: v0.5.0-3202-g618e15b host hardware: x86_64 host os:4.2.0-164 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.32.27-i686: ERRORS linux-2.6.33.7-i686: ERRORS linux-2.6.34.7-i686: ERRORS linux-2.6.35.9-i686: ERRORS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12.23-i686: OK linux-3.13.11-i686: OK linux-3.14.9-i686: OK linux-3.15.2-i686: OK linux-3.16.7-i686: OK linux-3.17.8-i686: OK linux-3.18.7-i686: OK linux-3.19-i686: OK linux-4.0-i686: OK linux-4.1.1-i686: OK linux-4.2-i686: OK linux-4.3-i686: OK linux-4.4-rc1-i686: OK linux-2.6.32.27-x86_64: ERRORS linux-2.6.33.7-x86_64: ERRORS linux-2.6.34.7-x86_64: ERRORS linux-2.6.35.9-x86_64: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12.23-x86_64: OK linux-3.13.11-x86_64: OK linux-3.14.9-x86_64: OK linux-3.15.2-x86_64: OK linux-3.16.7-x86_64: OK linux-3.17.8-x86_64: OK linux-3.18.7-x86_64: OK linux-3.19-x86_64: OK linux-4.0-x86_64: OK linux-4.1.1-x86_64: OK linux-4.2-x86_64: OK linux-4.3-x86_64: OK linux-4.4-rc1-x86_64: OK apps: WARNINGS spec-git: OK sparse: ERRORS smatch: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Sabrent (stk1160) / Easycap driver problem
On 2 December 2015 at 14:43, Philippe Desrochers wrote: > I'm sending the email again (in plain text) since it seems it was blocked by > the server. > > EASYCAP CHINA CLONE (OK): > [ 8630.596236] usb 2-1: new high-speed USB device number 6 using ehci-pci > [ 8630.729074] usb 2-1: New USB device found, idVendor=05e1, idProduct=0408 > [ 8630.729084] usb 2-1: New USB device strings: Mfr=1, Product=2, > SerialNumber=0 > [ 8630.729091] usb 2-1: Product: USB 2.0 Video Capture Controller > [ 8630.729097] usb 2-1: Manufacturer: Syntek Semiconductor > [ 8630.729648] usb 2-1: New device Syntek Semiconductor USB 2.0 Video > Capture Controller @ 480 Mbps (05e1:0408, interface 0, class 0) > [ 8630.729656] usb 2-1: video interface 0 found > [ 8631.242258] saa7115 7-0025: saa7113 found @ 0x4a (stk1160) Hmm.. seems the bad device doesn't found a decoder chip. Let me refresh my mind and get back to you. -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Sabrent (stk1160) / Easycap driver problem
Hi Philippe, Ccing the linux-media ML. Please don't reply dropping the Cc! 2015-12-02 11:58 GMT-03:00 Philippe Desrochers : > Hello Ezequiel, > > I'm using your stk1160 driver with some EasyCap (china) clone and it is > working fine. > > However, a few days ago, I bought two Sabrent USB-AVCPT and both of them are > not working with my Ubuntu 14.04 computer. > http://www.sabrent.com/category/video-converters/USB-AVCPT/ > > All of these devices have the Syntek 1160 chipset. > > Normally, I am using VLC to use these devices but with the Sabrent grabber I > can't see the video/display window. (it just does not show) > > I also tried mplayer and it is giving me a "select timeout". > > See attached file for the "lsusb -vv" output. > > Can you give me some hint where to look for to fix this issue ? > If needed, I can compile code or driver and do some tests. > > For now, I am a bit lost because I don't know where to start looking... > Can you paste the kernel log (dmesg output) in each case? -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] [media] coda: don't start streaming without queued buffers
We could support start streaming with an empty output queue for the
BIT decoders due to the bitstream buffer which could still contain
data at this point, but there is really no reason for userspace to
expect this to work. Simplify the code by disallowing it.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 007f458..2452f46 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1252,6 +1252,9 @@ static int coda_start_streaming(struct vb2_queue *q,
unsigned int count)
struct vb2_v4l2_buffer *buf;
int ret = 0;
+ if (count < 1)
+ return -EINVAL;
+
q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
if (ctx->inst_type == CODA_INST_DECODER && ctx->use_bit) {
@@ -1264,20 +1267,10 @@ static int coda_start_streaming(struct vb2_queue *q,
unsigned int count)
ret = -EINVAL;
goto err;
}
- } else {
- if (count < 1) {
- ret = -EINVAL;
- goto err;
- }
}
ctx->streamon_out = 1;
} else {
- if (count < 1) {
- ret = -EINVAL;
- goto err;
- }
-
ctx->streamon_cap = 1;
}
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] [media] coda: relax coda_jpeg_check_buffer for trailing bytes
From: Philipp Zabel
coda_jpeg_check_buffer only cares about the buffer length and contents,
so change the parameter type back from v4l2_vb2_buffer to just the
vb2_buffer.
Instead of just checking the first and last bytes for the SOI and EOI
markers, relax the EOI marker check a bit and allow up to 32 trailing
bytes after the EOI marker as hardware generated JPEGs sometimes contain
some alignment overhead.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 2 +-
drivers/media/platform/coda/coda-jpeg.c | 26 --
drivers/media/platform/coda/coda.h | 2 +-
3 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 654e964..36424ae 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -246,7 +246,7 @@ void coda_fill_bitstream(struct coda_ctx *ctx, bool
streaming)
/* Drop frames that do not start/end with a SOI/EOI markers */
if (ctx->codec->src_fourcc == V4L2_PIX_FMT_JPEG &&
- !coda_jpeg_check_buffer(ctx, src_buf)) {
+ !coda_jpeg_check_buffer(ctx, &src_buf->vb2_buf)) {
v4l2_err(&ctx->dev->v4l2_dev,
"dropping invalid JPEG frame %d\n",
ctx->qsequence);
diff --git a/drivers/media/platform/coda/coda-jpeg.c
b/drivers/media/platform/coda/coda-jpeg.c
index 96cd42a..9f899a6 100644
--- a/drivers/media/platform/coda/coda-jpeg.c
+++ b/drivers/media/platform/coda/coda-jpeg.c
@@ -178,14 +178,28 @@ int coda_jpeg_write_tables(struct coda_ctx *ctx)
return 0;
}
-bool coda_jpeg_check_buffer(struct coda_ctx *ctx, struct vb2_v4l2_buffer *vb)
+bool coda_jpeg_check_buffer(struct coda_ctx *ctx, struct vb2_buffer *vb)
{
- void *vaddr = vb2_plane_vaddr(&vb->vb2_buf, 0);
- u16 soi = be16_to_cpup((__be16 *)vaddr);
- u16 eoi = be16_to_cpup((__be16 *)(vaddr +
- vb2_get_plane_payload(&vb->vb2_buf, 0) - 2));
+ void *vaddr = vb2_plane_vaddr(vb, 0);
+ u16 soi, eoi;
+ int len, i;
+
+ soi = be16_to_cpup((__be16 *)vaddr);
+ if (soi != SOI_MARKER)
+ return false;
+
+ len = vb2_get_plane_payload(vb, 0);
+ vaddr += len - 2;
+ for (i = 0; i < 32; i++) {
+ eoi = be16_to_cpup((__be16 *)(vaddr - i));
+ if (eoi == EOI_MARKER) {
+ if (i > 0)
+ vb2_set_plane_payload(vb, 0, len - i);
+ return true;
+ }
+ }
- return soi == SOI_MARKER && eoi == EOI_MARKER;
+ return false;
}
/*
diff --git a/drivers/media/platform/coda/coda.h
b/drivers/media/platform/coda/coda.h
index 96532b0..ce5f5ad 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -289,7 +289,7 @@ void coda_m2m_buf_done(struct coda_ctx *ctx, struct
vb2_v4l2_buffer *buf,
int coda_h264_padding(int size, char *p);
-bool coda_jpeg_check_buffer(struct coda_ctx *ctx, struct vb2_v4l2_buffer *vb);
+bool coda_jpeg_check_buffer(struct coda_ctx *ctx, struct vb2_buffer *vb);
int coda_jpeg_write_tables(struct coda_ctx *ctx);
void coda_set_jpeg_compression_quality(struct coda_ctx *ctx, int quality);
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] [media] coda: hook up vidioc_prepare_buf
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 03b46de..007f458 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -921,6 +921,7 @@ static const struct v4l2_ioctl_ops coda_ioctl_ops = {
.vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
.vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
.vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
+ .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
.vidioc_streamon= v4l2_m2m_ioctl_streamon,
.vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] [media] coda: enable MPEG-2 ES decoding
Hook up the MPEG-2 ES decoder.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 2452f46..2197032 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -131,6 +131,7 @@ static const struct coda_codec coda7_codecs[] = {
CODA_CODEC(CODA7_MODE_ENCODE_MP4, V4L2_PIX_FMT_YUV420,
V4L2_PIX_FMT_MPEG4, 1280, 720),
CODA_CODEC(CODA7_MODE_ENCODE_MJPG, V4L2_PIX_FMT_YUV420,
V4L2_PIX_FMT_JPEG, 8192, 8192),
CODA_CODEC(CODA7_MODE_DECODE_H264, V4L2_PIX_FMT_H264,
V4L2_PIX_FMT_YUV420, 1920, 1088),
+ CODA_CODEC(CODA7_MODE_DECODE_MP2, V4L2_PIX_FMT_MPEG2,
V4L2_PIX_FMT_YUV420, 1920, 1088),
CODA_CODEC(CODA7_MODE_DECODE_MP4, V4L2_PIX_FMT_MPEG4,
V4L2_PIX_FMT_YUV420, 1920, 1088),
CODA_CODEC(CODA7_MODE_DECODE_MJPG, V4L2_PIX_FMT_JPEG,
V4L2_PIX_FMT_YUV420, 8192, 8192),
};
@@ -139,6 +140,7 @@ static const struct coda_codec coda9_codecs[] = {
CODA_CODEC(CODA9_MODE_ENCODE_H264, V4L2_PIX_FMT_YUV420,
V4L2_PIX_FMT_H264, 1920, 1088),
CODA_CODEC(CODA9_MODE_ENCODE_MP4, V4L2_PIX_FMT_YUV420,
V4L2_PIX_FMT_MPEG4, 1920, 1088),
CODA_CODEC(CODA9_MODE_DECODE_H264, V4L2_PIX_FMT_H264,
V4L2_PIX_FMT_YUV420, 1920, 1088),
+ CODA_CODEC(CODA9_MODE_DECODE_MP2, V4L2_PIX_FMT_MPEG2,
V4L2_PIX_FMT_YUV420, 1920, 1088),
CODA_CODEC(CODA9_MODE_DECODE_MP4, V4L2_PIX_FMT_MPEG4,
V4L2_PIX_FMT_YUV420, 1920, 1088),
};
@@ -187,6 +189,7 @@ static const struct coda_video_device coda_bit_decoder = {
.ops = &coda_bit_decode_ops,
.src_formats = {
V4L2_PIX_FMT_H264,
+ V4L2_PIX_FMT_MPEG2,
V4L2_PIX_FMT_MPEG4,
},
.dst_formats = {
@@ -470,6 +473,7 @@ static int coda_try_fmt(struct coda_ctx *ctx, const struct
coda_codec *codec,
/* fallthrough */
case V4L2_PIX_FMT_H264:
case V4L2_PIX_FMT_MPEG4:
+ case V4L2_PIX_FMT_MPEG2:
f->fmt.pix.bytesperline = 0;
f->fmt.pix.sizeimage = coda_estimate_sizeimage(ctx,
f->fmt.pix.sizeimage,
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] [media] coda: make to_coda_video_device static
This function is not used outside coda-common.c.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 15516a6..03b46de 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -293,7 +293,8 @@ static void coda_get_max_dimensions(struct coda_dev *dev,
*max_h = h;
}
-const struct coda_video_device *to_coda_video_device(struct video_device *vdev)
+static const struct coda_video_device *to_coda_video_device(struct video_device
+ *vdev)
{
struct coda_dev *dev = video_get_drvdata(vdev);
unsigned int i = vdev - dev->vfd;
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND RFC/PATCH 6/8] media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver
On 02/12/15 13:08, tiffany lin wrote:
We need MTK_STATE_ABORT to inform encoder thread (mtk_venc_worker) that
stop encodeing job from stopped ctx instance.
When user space qbuf, we need to make sure everything is ready to sent
buf to encode.
Agree that you need a flag here. In fact currently you have two,
MTK_STATE_ABORT and an unused one called aborting.
You need to be very careful with these flags though. They are a magnet
for data race bugs (especially combined with SMP).
For example at present I can't see any locking in the worker code. This
means there is nothing to make all those read-modify-write sequences
that manage the state atomic (thus risking state corruption).
We prevent that one function set the flag and others clear the flag.
So there is no special lock to protect state.
What prevents concurrent access from different calling contexts? It
looks to me like the work on the work queue may run concurrently with
the ioctl calls.
+static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
+{
+ struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(q);
+ struct v4l2_device *v4l2_dev = &ctx->dev->v4l2_dev;
+ struct vb2_buffer *src_buf, *dst_buf;
+ int retry;
+ int ret;
+
+ mtk_v4l2_debug(2, "[%d]-> type=%d", ctx->idx, q->type);
+
+ retry = 0;
+ while ((ctx->state & MTK_STATE_RUNNING) && (retry < 10)) {
+ mtk_vcodec_clean_ctx_int_flags(ctx);
+ ctx->state |= MTK_STATE_ABORT;
As a simple example I think the above line can run concurrently with the
following code near the end of the worker code.
+ ctx->state &= ~MTK_STATE_RUNNING;
+ v4l2_m2m_job_finish(ctx->dev->m2m_dev_enc, ctx->m2m_ctx);
If I'm right then the state of the flags can definitely get clobbered
due to the read-modify-write actions on the state.
Daniel.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 04/11] v4l2-ctrls: add config store support
2015-12-02 13:33 GMT+01:00 Hans Verkuil : > On 12/02/15 13:03, Enric Balletbo Serra wrote: >> Dear Hans, >> >> We've a driver that uses your confstore stuff and we'd like to push >> upstream. I'm wondering if there is any plan to upstream the confstore >> patches or if this was abandoned for some reason. Thanks > > Ouch, that's really old code you're using. > > The latest version is here: > > http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=requests > > But that too won't be the final version. > > There is still work going on in this area (specifically by Laurent Pinchart) > since we really need this functionality. But we need to make sure that the > API is good enough to handle complex hardware before it can be upstreamed. > > I suspect that once Laurent has it working for his non-trivial use-case we > can start looking at upstreaming it. > > I recommend rebasing to at least the version in my git tree as that will > be much closer to the final version. I'll try to rebase that branch to > the latest kernel, but that's a bit difficult and takes more time than I > have at the moment. > Thanks to point me in the right direction. Regards, Enric > Regards, > > Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND RFC/PATCH 6/8] media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver
On Tue, 2015-12-01 at 15:42 +, Daniel Thompson wrote:
> On 01/12/15 10:42, tiffany lin wrote:
> > diff --git a/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
> b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
> > new file mode 100644
> > index 000..9b3f025
> > --- /dev/null
> > [snip]
> > +int venc_if_create(void *ctx, unsigned int fourcc, unsigned long
> *handle)
> > +{
> > + struct venc_handle *h;
> > + char str[10];
> > +
> > + mtk_vcodec_fmt2str(fourcc, str);
> > +
> > + h = kzalloc(sizeof(*h), GFP_KERNEL);
> > + if (!h)
> > + return -ENOMEM;
> > +
> > + h->fourcc = fourcc;
> > + h->ctx = ctx;
> > + mtk_vcodec_debug(h, "fmt = %s handle = %p", str, h);
> > +
> > + switch (fourcc) {
> > + default:
> > + mtk_vcodec_err(h, "invalid format %s", str);
> > + goto err_out;
> > + }
> > +
> > + *handle = (unsigned long)h;
> > + return 0;
> > +
> > +err_out:
> > + kfree(h);
> > + return -EINVAL;
> > +}
> > +
> > +int venc_if_init(unsigned long handle)
> > +{
> > + int ret = 0;
> > + struct venc_handle *h = (struct venc_handle *)handle;
> > +
> > + mtk_vcodec_debug_enter(h);
> > +
> > + mtk_venc_lock(h->ctx);
> > + mtk_vcodec_enc_clock_on();
> > + vpu_enable_clock(vpu_get_plat_device(h->ctx->dev->plat_dev));
> > + ret = h->enc_if->init(h->ctx, (unsigned long *)&h->drv_handle);
> > + vpu_disable_clock(vpu_get_plat_device(h->ctx->dev->plat_dev));
> > + mtk_vcodec_enc_clock_off();
> > + mtk_venc_unlock(h->ctx);
> > +
> > + return ret;
> > +}
>
> To me this looks more like an obfuscation layer rather than a
> abstraction layer. I don't understand why we need to hide things from
> the V4L2 implementation that this code forms part of.
>
> More importantly, if this code was included somewhere where it could be
> properly integrated with the device model you might be able to use the
> pm_runtime system to avoid this sort of "heroics" to manage the clocks
> anyway.
>
> >>> We want to abstract common part from encoder driver.
> >>> Every encoder driver follow same calling flow and only need to take care
> >>> about how to communicate with vpu to encode specific format.
> >>> Encoder driver do not need to take care clock and multiple instance
> >>> issue.
> >>
> >> Looking at each of those stages:
> >>
> >> mtk_venc_lock():
> >> Why isn't one of the existing V4L2 locking strategies ok for you?
> >>
> > We only has one encoder hw.
> > To support multiple encode instances.
> > When one encoder ctx access encoder hw, it need to get lock first.
> >
> >> mtk_vcodec_enc_clock_on():
> >> This does seem like something a sub-driver *should* be doing for itself
> > This is for enabling encoder hw related clock.
> > To support multiple instances, one encode ctx must get hw lock first
> > then clock on/off hw relate clock.
> >
> >> vpu_enable_clock():
> >> Why can't the VPU driver manage this internally using pm_runtime?
> >>
> > Our VPU do not have power domain.
> > We will remove VPU clock on/off and let vpu control it in next version.
> >
> >>
> >> That is why I described this as an obfuscation layer. It is collecting
> >> a bunch of stuff that can be handled using the kernel driver model and
> >> clumping them together in a special middle layer.
> >>
> > We do use kernel driver model, but we put it in
> > mtk_vcodec_enc_clock_on/mtk_vcodec_enc_clock_off.
> > Every sub-driver has no need to write the same code.
> > And once clock configuration change or porting to other chips, we don't
> > need to change sub-driver one-by-one, just change abstract layer.
>
> I'm afraid I remain extremely unconvinced by the value of this API. It
> is possible that once the types are fixed and it is tidied up it won't
> stick out so much but I will be very surprised.
>
> Either way, I can wait until v2 before we discuss it further.
>
>
> If the start streaming operation implemented cleanup-on-error properly
> then there would only be two useful states: Started and stopped. Even
> the "sticky" error behavior looks unnecessary to me (meaning we don't
> need to track its state).
>
> >>> We cannot guaranteed that IOCTLs called from the user space follow
> >>> required sequence.
> >>> We need states to know if our driver could accept IOCTL command.
> >>
> >> I believe that knowing whether the streaming is started or stopped
> >> (e.g. two states) is sufficient for a driver to correctly handle
> >> abitrary ioctls from userspace and even then, the core code tracks
> >> this state for you so there's no need for you do it.
> >>
> >> The queue/dequeue i
Re: [RFC PATCH 04/11] v4l2-ctrls: add config store support
On 12/02/15 13:03, Enric Balletbo Serra wrote: > Dear Hans, > > We've a driver that uses your confstore stuff and we'd like to push > upstream. I'm wondering if there is any plan to upstream the confstore > patches or if this was abandoned for some reason. Thanks Ouch, that's really old code you're using. The latest version is here: http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=requests But that too won't be the final version. There is still work going on in this area (specifically by Laurent Pinchart) since we really need this functionality. But we need to make sure that the API is good enough to handle complex hardware before it can be upstreamed. I suspect that once Laurent has it working for his non-trivial use-case we can start looking at upstreaming it. I recommend rebasing to at least the version in my git tree as that will be much closer to the final version. I'll try to rebase that branch to the latest kernel, but that's a bit difficult and takes more time than I have at the moment. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 04/11] v4l2-ctrls: add config store support
Dear Hans,
2014-11-17 9:46 GMT+01:00 Hans Verkuil :
> On 11/14/2014 04:44 PM, Sakari Ailus wrote:
>> Hi Hans,
>>
>> Some comments below.
>>
>> On Sun, Sep 21, 2014 at 04:48:22PM +0200, Hans Verkuil wrote:
>>> From: Hans Verkuil
>>>
>>> Signed-off-by: Hans Verkuil
>>> ---
>>> drivers/media/v4l2-core/v4l2-ctrls.c | 150
>>> +--
>>> drivers/media/v4l2-core/v4l2-ioctl.c | 4 +-
>>> include/media/v4l2-ctrls.h | 14
>>> 3 files changed, 141 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> index 35d1f3d..df0f3ee 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> @@ -1478,6 +1478,15 @@ static int cur_to_user(struct v4l2_ext_control *c,
>>> return ptr_to_user(c, ctrl, ctrl->p_cur);
>>> }
>>>
>>> +/* Helper function: copy the store's control value back to the caller */
>>> +static int store_to_user(struct v4l2_ext_control *c,
>>> + struct v4l2_ctrl *ctrl, unsigned store)
>>> +{
>>> +if (store == 0)
>>> +return ptr_to_user(c, ctrl, ctrl->p_new);
>>> +return ptr_to_user(c, ctrl, ctrl->p_stores[store - 1]);
>>> +}
>>> +
>>> /* Helper function: copy the new control value back to the caller */
>>> static int new_to_user(struct v4l2_ext_control *c,
>>> struct v4l2_ctrl *ctrl)
>>> @@ -1585,6 +1594,14 @@ static void new_to_cur(struct v4l2_fh *fh, struct
>>> v4l2_ctrl *ctrl, u32 ch_flags)
>>> }
>>> }
>>>
>>> +/* Helper function: copy the new control value to the store */
>>> +static void new_to_store(struct v4l2_ctrl *ctrl)
>>> +{
>>> +/* has_changed is set by cluster_changed */
>>> +if (ctrl && ctrl->has_changed)
>>> +ptr_to_ptr(ctrl, ctrl->p_new, ctrl->p_stores[ctrl->store - 1]);
>>> +}
>>> +
>>> /* Copy the current value to the new value */
>>> static void cur_to_new(struct v4l2_ctrl *ctrl)
>>> {
>>> @@ -1603,13 +1620,18 @@ static int cluster_changed(struct v4l2_ctrl *master)
>>>
>>> for (i = 0; i < master->ncontrols; i++) {
>>> struct v4l2_ctrl *ctrl = master->cluster[i];
>>> +union v4l2_ctrl_ptr ptr;
>>> bool ctrl_changed = false;
>>>
>>> if (ctrl == NULL)
>>> continue;
>>> +if (ctrl->store)
>>> +ptr = ctrl->p_stores[ctrl->store - 1];
>>> +else
>>> +ptr = ctrl->p_cur;
>>> for (idx = 0; !ctrl_changed && idx < ctrl->elems; idx++)
>>> ctrl_changed = !ctrl->type_ops->equal(ctrl, idx,
>>> -ctrl->p_cur, ctrl->p_new);
>>> +ptr, ctrl->p_new);
>>> ctrl->has_changed = ctrl_changed;
>>> changed |= ctrl->has_changed;
>>> }
>>> @@ -1740,6 +1762,13 @@ void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler
>>> *hdl)
>>> list_del(&ctrl->node);
>>> list_for_each_entry_safe(sev, next_sev, &ctrl->ev_subs, node)
>>> list_del(&sev->node);
>>> +if (ctrl->p_stores) {
>>> +unsigned s;
>>> +
>>> +for (s = 0; s < ctrl->nr_of_stores; s++)
>>> +kfree(ctrl->p_stores[s].p);
>>> +}
>>> +kfree(ctrl->p_stores);
>>> kfree(ctrl);
>>> }
>>> kfree(hdl->buckets);
>>> @@ -1970,7 +1999,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
>>> v4l2_ctrl_handler *hdl,
>>> handler_set_err(hdl, -ERANGE);
>>> return NULL;
>>> }
>>> -if (is_array &&
>>> +if ((is_array || (flags & V4L2_CTRL_FLAG_CAN_STORE)) &&
>>> (type == V4L2_CTRL_TYPE_BUTTON ||
>>> type == V4L2_CTRL_TYPE_CTRL_CLASS)) {
>>> handler_set_err(hdl, -EINVAL);
>>> @@ -2084,8 +2113,10 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct
>>> v4l2_ctrl_handler *hdl,
>>> is_menu ? cfg->menu_skip_mask : step, def,
>>> cfg->dims, cfg->elem_size,
>>> flags, qmenu, qmenu_int, priv);
>>> -if (ctrl)
>>> +if (ctrl) {
>>
>> I think it'd be cleaner to return NULL here if ctrl == NULL. Up to you.
>
> Agreed.
>
>>
>>> ctrl->is_private = cfg->is_private;
>>> +ctrl->can_store = cfg->can_store;
>>> +}
>>> return ctrl;
>>> }
>>> EXPORT_SYMBOL(v4l2_ctrl_new_custom);
>>> @@ -2452,6 +2483,7 @@ int v4l2_ctrl_handler_setup(struct v4l2_ctrl_handler
>>> *hdl)
>>> cur_to_new(master->cluster[i]);
>>> master->cluster[i]->is_new = 1;
>>> master->cluster[i]->done = true;
>>> +master->cluster[i]->store = 0;
>>> }
>>> }
>>> ret = call_op(master, s_ctrl);
>>> @@ -2539,6 +2571,8 @@ int v4l2_que
[PATCH 2/6] s5p-mfc: make queue cleanup code common
Code for queue cleanup has nothing specific to hardware version.
Signed-off-by: Andrzej Hajda
---
drivers/media/platform/s5p-mfc/s5p_mfc.c| 26 +
drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 1 +
drivers/media/platform/s5p-mfc/s5p_mfc_dec.c| 6 ++
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 6 ++
drivers/media/platform/s5p-mfc/s5p_mfc_opr.h| 2 --
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 16 ---
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 16 ---
7 files changed, 23 insertions(+), 50 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 8bae7df..79f5c81 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -125,6 +125,20 @@ static void wake_up_dev(struct s5p_mfc_dev *dev, unsigned
int reason,
wake_up(&dev->queue);
}
+void s5p_mfc_cleanup_queue(struct list_head *lh, struct vb2_queue *vq)
+{
+ struct s5p_mfc_buf *b;
+ int i;
+
+ while (!list_empty(lh)) {
+ b = list_entry(lh->next, struct s5p_mfc_buf, list);
+ for (i = 0; i < b->b->vb2_buf.num_planes; i++)
+ vb2_set_plane_payload(&b->b->vb2_buf, i, 0);
+ vb2_buffer_done(&b->b->vb2_buf, VB2_BUF_STATE_ERROR);
+ list_del(&b->list);
+ }
+}
+
static void s5p_mfc_watchdog(unsigned long arg)
{
struct s5p_mfc_dev *dev = (struct s5p_mfc_dev *)arg;
@@ -170,10 +184,8 @@ static void s5p_mfc_watchdog_worker(struct work_struct
*work)
if (!ctx)
continue;
ctx->state = MFCINST_ERROR;
- s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
- &ctx->dst_queue, &ctx->vq_dst);
- s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
- &ctx->src_queue, &ctx->vq_src);
+ s5p_mfc_cleanup_queue(&ctx->dst_queue, &ctx->vq_dst);
+ s5p_mfc_cleanup_queue(&ctx->src_queue, &ctx->vq_src);
clear_work_bit(ctx);
wake_up_ctx(ctx, S5P_MFC_R2H_CMD_ERR_RET, 0);
}
@@ -471,11 +483,9 @@ static void s5p_mfc_handle_error(struct s5p_mfc_dev *dev,
ctx->state = MFCINST_ERROR;
/* Mark all dst buffers as having an error */
spin_lock_irqsave(&dev->irqlock, flags);
- s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
- &ctx->dst_queue, &ctx->vq_dst);
+ s5p_mfc_cleanup_queue(&ctx->dst_queue, &ctx->vq_dst);
/* Mark all src buffers as having an error */
- s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
- &ctx->src_queue, &ctx->vq_src);
+ s5p_mfc_cleanup_queue(&ctx->src_queue, &ctx->vq_src);
spin_unlock_irqrestore(&dev->irqlock, flags);
wake_up_ctx(ctx, reason, err);
break;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 0cdb37e..f560240 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -711,6 +711,7 @@ void set_work_bit(struct s5p_mfc_ctx *ctx);
void clear_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
int s5p_mfc_get_new_ctx(struct s5p_mfc_dev *dev);
+void s5p_mfc_cleanup_queue(struct list_head *lh, struct vb2_queue *vq);
#define HAS_PORTNUM(dev) (dev ? (dev->variant ? \
(dev->variant->port_num ? 1 : 0) : 0) : 0)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 1c4998c..cbb 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -1033,8 +1033,7 @@ static void s5p_mfc_stop_streaming(struct vb2_queue *q)
}
if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
spin_lock_irqsave(&dev->irqlock, flags);
- s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
- &ctx->dst_queue, &ctx->vq_dst);
+ s5p_mfc_cleanup_queue(&ctx->dst_queue, &ctx->vq_dst);
INIT_LIST_HEAD(&ctx->dst_queue);
ctx->dst_queue_cnt = 0;
ctx->dpb_flush_flag = 1;
@@ -1051,8 +1050,7 @@ static void s5p_mfc_stop_streaming(struct vb2_queue *q)
}
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
spin_lock_irqsave(&dev->irqlock, flags);
- s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
-
[PATCH 0/6] MFC locking rework and code cleanup
Hi Kamil, This patchset contains one patch which should fix races during accessing context fields. As the patch significantly changes locking I have tested it in different different scenarios. Other patches just clean up the code. The patchset is based on the latest media-tree branch. Regards Andrzej Andrzej Hajda (6): s5p-mfc: use one implementation of s5p_mfc_get_new_ctx s5p-mfc: make queue cleanup code common s5p-mfc: remove unnecessary callbacks s5p-mfc: use spinlock to protect MFC context s5p-mfc: merge together s5p_mfc_hw_call and s5p_mfc_hw_call_void s5p-mfc: remove volatile attribute from MFC register addresses drivers/media/platform/s5p-mfc/s5p_mfc.c| 99 +++-- drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 12 +- drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 16 +- drivers/media/platform/s5p-mfc/s5p_mfc_dec.c| 31 +- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 40 +- drivers/media/platform/s5p-mfc/s5p_mfc_opr.h| 507 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 94 - drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 121 +- 8 files changed, 351 insertions(+), 569 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] s5p-mfc: use one implementation of s5p_mfc_get_new_ctx
Both version of MFC driver uses functions with the same body and name.
The patch moves them to common location. It also simplifies it.
Signed-off-by: Andrzej Hajda
---
drivers/media/platform/s5p-mfc/s5p_mfc.c| 20
drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 1 +
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 21 -
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 24
4 files changed, 21 insertions(+), 45 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 3ffe2ec..8bae7df 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -85,6 +85,26 @@ void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx)
spin_unlock_irqrestore(&dev->condlock, flags);
}
+int s5p_mfc_get_new_ctx(struct s5p_mfc_dev *dev)
+{
+ unsigned long flags;
+ int ctx;
+
+ spin_lock_irqsave(&dev->condlock, flags);
+ ctx = dev->curr_ctx;
+ do {
+ ctx = (ctx + 1) % MFC_NUM_CONTEXTS;
+ if (ctx == dev->curr_ctx) {
+ if (!test_bit(ctx, &dev->ctx_work_bits))
+ ctx = -EAGAIN;
+ break;
+ }
+ } while (!test_bit(ctx, &dev->ctx_work_bits));
+ spin_unlock_irqrestore(&dev->condlock, flags);
+
+ return ctx;
+}
+
/* Wake up context wait_queue */
static void wake_up_ctx(struct s5p_mfc_ctx *ctx, unsigned int reason,
unsigned int err)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index d1a3f9b..0cdb37e 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -710,6 +710,7 @@ void clear_work_bit(struct s5p_mfc_ctx *ctx);
void set_work_bit(struct s5p_mfc_ctx *ctx);
void clear_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
+int s5p_mfc_get_new_ctx(struct s5p_mfc_dev *dev);
#define HAS_PORTNUM(dev) (dev ? (dev->variant ? \
(dev->variant->port_num ? 1 : 0) : 0) : 0)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
index 873c933..d9e5d68 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
@@ -1153,27 +1153,6 @@ static int s5p_mfc_encode_one_frame_v5(struct
s5p_mfc_ctx *ctx)
return 0;
}
-static int s5p_mfc_get_new_ctx(struct s5p_mfc_dev *dev)
-{
- unsigned long flags;
- int new_ctx;
- int cnt;
-
- spin_lock_irqsave(&dev->condlock, flags);
- new_ctx = (dev->curr_ctx + 1) % MFC_NUM_CONTEXTS;
- cnt = 0;
- while (!test_bit(new_ctx, &dev->ctx_work_bits)) {
- new_ctx = (new_ctx + 1) % MFC_NUM_CONTEXTS;
- if (++cnt > MFC_NUM_CONTEXTS) {
- /* No contexts to run */
- spin_unlock_irqrestore(&dev->condlock, flags);
- return -EAGAIN;
- }
- }
- spin_unlock_irqrestore(&dev->condlock, flags);
- return new_ctx;
-}
-
static void s5p_mfc_run_res_change(struct s5p_mfc_ctx *ctx)
{
struct s5p_mfc_dev *dev = ctx->dev;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index b958453..f68653f 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -1507,30 +1507,6 @@ static int s5p_mfc_encode_one_frame_v6(struct
s5p_mfc_ctx *ctx)
return 0;
}
-static inline int s5p_mfc_get_new_ctx(struct s5p_mfc_dev *dev)
-{
- unsigned long flags;
- int new_ctx;
- int cnt;
-
- spin_lock_irqsave(&dev->condlock, flags);
- mfc_debug(2, "Previous context: %d (bits %08lx)\n", dev->curr_ctx,
- dev->ctx_work_bits);
- new_ctx = (dev->curr_ctx + 1) % MFC_NUM_CONTEXTS;
- cnt = 0;
- while (!test_bit(new_ctx, &dev->ctx_work_bits)) {
- new_ctx = (new_ctx + 1) % MFC_NUM_CONTEXTS;
- cnt++;
- if (cnt > MFC_NUM_CONTEXTS) {
- /* No contexts to run */
- spin_unlock_irqrestore(&dev->condlock, flags);
- return -EAGAIN;
- }
- }
- spin_unlock_irqrestore(&dev->condlock, flags);
- return new_ctx;
-}
-
static inline void s5p_mfc_run_dec_last_frames(struct s5p_mfc_ctx *ctx)
{
struct s5p_mfc_dev *dev = ctx->dev;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] s5p-mfc: merge together s5p_mfc_hw_call and s5p_mfc_hw_call_void
Both macros can be merged into one.
Signed-off-by: Andrzej Hajda
---
drivers/media/platform/s5p-mfc/s5p_mfc.c| 38 -
drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 8 +-
drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 16 +--
drivers/media/platform/s5p-mfc/s5p_mfc_dec.c| 12
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 20 ++---
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 16 +--
6 files changed, 52 insertions(+), 58 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index c4d9e34..81ffb67 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -374,11 +374,11 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx,
if (res_change == S5P_FIMV_RES_INCREASE ||
res_change == S5P_FIMV_RES_DECREASE) {
ctx->state = MFCINST_RES_CHANGE_INIT;
- s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
+ s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
wake_up_ctx(ctx, reason, err);
WARN_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
s5p_mfc_clock_off();
- s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
+ s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
return;
}
if (ctx->dpb_flush_flag)
@@ -446,7 +446,7 @@ leave_handle_frame:
if ((ctx->src_queue_cnt == 0 && ctx->state != MFCINST_FINISHING)
|| ctx->dst_queue_cnt < ctx->pb_count)
clear_work_bit(ctx);
- s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
+ s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
wake_up_ctx(ctx, reason, err);
WARN_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
s5p_mfc_clock_off();
@@ -454,7 +454,7 @@ leave_handle_frame:
if (test_bit(0, &dev->enter_suspend))
wake_up_dev(dev, reason, err);
else
- s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
+ s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
}
/* Error handling for interrupt */
@@ -490,7 +490,7 @@ static void s5p_mfc_handle_error(struct s5p_mfc_dev *dev,
}
}
WARN_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
- s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
+ s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_clock_off();
wake_up_dev(dev, reason, err);
return;
@@ -514,7 +514,7 @@ static void s5p_mfc_handle_seq_done(struct s5p_mfc_ctx *ctx,
ctx->img_height = s5p_mfc_hw_call(dev->mfc_ops, get_img_height,
dev);
- s5p_mfc_hw_call_void(dev->mfc_ops, dec_calc_dpb_size, ctx);
+ s5p_mfc_hw_call(dev->mfc_ops, dec_calc_dpb_size, ctx);
ctx->pb_count = s5p_mfc_hw_call(dev->mfc_ops, get_dpb_count,
dev);
@@ -541,11 +541,11 @@ static void s5p_mfc_handle_seq_done(struct s5p_mfc_ctx
*ctx,
ctx->head_processed = 1;
}
}
- s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
+ s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
clear_work_bit(ctx);
WARN_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
s5p_mfc_clock_off();
- s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
+ s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
wake_up_ctx(ctx, reason, err);
}
@@ -559,7 +559,7 @@ static void s5p_mfc_handle_init_buffers(struct s5p_mfc_ctx
*ctx,
if (ctx == NULL)
return;
dev = ctx->dev;
- s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
+ s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
ctx->int_type = reason;
ctx->int_err = err;
ctx->int_cond = 1;
@@ -583,7 +583,7 @@ static void s5p_mfc_handle_init_buffers(struct s5p_mfc_ctx
*ctx,
s5p_mfc_clock_off();
wake_up(&ctx->queue);
- s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
+ s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
} else {
WARN_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
@@ -617,7 +617,7 @@ static void s5p_mfc_handle_stream_complete(struct
s5p_mfc_ctx *ctx)
s5p_mfc_clock_off();
wake_up(&ctx->queue);
- s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
+ s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
}
/* Interrupt processing */
@@ -658,15 +658,15 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)
if (ctx->state == MFCINST_FINISHING &&
list_empty(&ctx->ref_queue)) {
- s5p_mfc_hw_call_void(dev->mfc
[PATCH 6/6] s5p-mfc: remove volatile attribute from MFC register addresses
MFC register addresses are used only by writel/readl macros which already
takes care of proper register accessing.
Signed-off-by: Andrzej Hajda
---
drivers/media/platform/s5p-mfc/s5p_mfc_opr.h | 488 +--
1 file changed, 244 insertions(+), 244 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
index 33dae96..b6ac417 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
@@ -20,254 +20,254 @@
struct s5p_mfc_regs {
/* codec common registers */
- volatile void __iomem *risc_on;
- volatile void __iomem *risc2host_int;
- volatile void __iomem *host2risc_int;
- volatile void __iomem *risc_base_address;
- volatile void __iomem *mfc_reset;
- volatile void __iomem *host2risc_command;
- volatile void __iomem *risc2host_command;
- volatile void __iomem *mfc_bus_reset_ctrl;
- volatile void __iomem *firmware_version;
- volatile void __iomem *instance_id;
- volatile void __iomem *codec_type;
- volatile void __iomem *context_mem_addr;
- volatile void __iomem *context_mem_size;
- volatile void __iomem *pixel_format;
- volatile void __iomem *metadata_enable;
- volatile void __iomem *mfc_version;
- volatile void __iomem *dbg_info_enable;
- volatile void __iomem *dbg_buffer_addr;
- volatile void __iomem *dbg_buffer_size;
- volatile void __iomem *hed_control;
- volatile void __iomem *mfc_timeout_value;
- volatile void __iomem *hed_shared_mem_addr;
- volatile void __iomem *dis_shared_mem_addr;/* only v7 */
- volatile void __iomem *ret_instance_id;
- volatile void __iomem *error_code;
- volatile void __iomem *dbg_buffer_output_size;
- volatile void __iomem *metadata_status;
- volatile void __iomem *metadata_addr_mb_info;
- volatile void __iomem *metadata_size_mb_info;
- volatile void __iomem *dbg_info_stage_counter;
+ void __iomem *risc_on;
+ void __iomem *risc2host_int;
+ void __iomem *host2risc_int;
+ void __iomem *risc_base_address;
+ void __iomem *mfc_reset;
+ void __iomem *host2risc_command;
+ void __iomem *risc2host_command;
+ void __iomem *mfc_bus_reset_ctrl;
+ void __iomem *firmware_version;
+ void __iomem *instance_id;
+ void __iomem *codec_type;
+ void __iomem *context_mem_addr;
+ void __iomem *context_mem_size;
+ void __iomem *pixel_format;
+ void __iomem *metadata_enable;
+ void __iomem *mfc_version;
+ void __iomem *dbg_info_enable;
+ void __iomem *dbg_buffer_addr;
+ void __iomem *dbg_buffer_size;
+ void __iomem *hed_control;
+ void __iomem *mfc_timeout_value;
+ void __iomem *hed_shared_mem_addr;
+ void __iomem *dis_shared_mem_addr;/* only v7 */
+ void __iomem *ret_instance_id;
+ void __iomem *error_code;
+ void __iomem *dbg_buffer_output_size;
+ void __iomem *metadata_status;
+ void __iomem *metadata_addr_mb_info;
+ void __iomem *metadata_size_mb_info;
+ void __iomem *dbg_info_stage_counter;
/* decoder registers */
- volatile void __iomem *d_crc_ctrl;
- volatile void __iomem *d_dec_options;
- volatile void __iomem *d_display_delay;
- volatile void __iomem *d_set_frame_width;
- volatile void __iomem *d_set_frame_height;
- volatile void __iomem *d_sei_enable;
- volatile void __iomem *d_min_num_dpb;
- volatile void __iomem *d_min_first_plane_dpb_size;
- volatile void __iomem *d_min_second_plane_dpb_size;
- volatile void __iomem *d_min_third_plane_dpb_size;/* only v8 */
- volatile void __iomem *d_min_num_mv;
- volatile void __iomem *d_mvc_num_views;
- volatile void __iomem *d_min_num_dis;/* only v7 */
- volatile void __iomem *d_min_first_dis_size;/* only v7 */
- volatile void __iomem *d_min_second_dis_size;/* only v7 */
- volatile void __iomem *d_min_third_dis_size;/* only v7 */
- volatile void __iomem *d_post_filter_luma_dpb0;/* v7 and v8 */
- volatile void __iomem *d_post_filter_luma_dpb1;/* v7 and v8 */
- volatile void __iomem *d_post_filter_luma_dpb2;/* only v7 */
- volatile void __iomem *d_post_filter_chroma_dpb0;/* v7 and v8 */
- volatile void __iomem *d_post_filter_chroma_dpb1;/* v7 and v8 */
- volatile void __iomem *d_post_filter_chroma_dpb2;/* only v7 */
- volatile void __iomem *d_num_dpb;
- volatile void __iomem *d_num_mv;
- volatile void __iomem *d_init_buffer_options;
- volatile void __iomem *d_first_plane_dpb_stride_size;/* only v8 */
- volatile void __iomem *d_second_plane_dpb_stride_size;/* only v8 */
- volatile void __iomem *d_third_plane_dpb_stride_size;/* only v8 */
- volatile void __iomem *d_first_plane_dpb_s
[PATCH 3/6] s5p-mfc: remove unnecessary callbacks
Many version specific functions are not called by common code, so there
is no need to use callbacks. Additionally some of them are not used at all,
so they can be safely removed.
Signed-off-by: Andrzej Hajda
---
drivers/media/platform/s5p-mfc/s5p_mfc_opr.h| 17 -
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 38
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 47 -
3 files changed, 102 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
index b89df89..33dae96 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
@@ -281,26 +281,14 @@ struct s5p_mfc_hw_ops {
void (*release_dev_context_buffer)(struct s5p_mfc_dev *dev);
void (*dec_calc_dpb_size)(struct s5p_mfc_ctx *ctx);
void (*enc_calc_src_size)(struct s5p_mfc_ctx *ctx);
- int (*set_dec_stream_buffer)(struct s5p_mfc_ctx *ctx,
- int buf_addr, unsigned int start_num_byte,
- unsigned int buf_size);
- int (*set_dec_frame_buffer)(struct s5p_mfc_ctx *ctx);
int (*set_enc_stream_buffer)(struct s5p_mfc_ctx *ctx,
unsigned long addr, unsigned int size);
void (*set_enc_frame_buffer)(struct s5p_mfc_ctx *ctx,
unsigned long y_addr, unsigned long c_addr);
void (*get_enc_frame_buffer)(struct s5p_mfc_ctx *ctx,
unsigned long *y_addr, unsigned long *c_addr);
- int (*set_enc_ref_buffer)(struct s5p_mfc_ctx *ctx);
- int (*init_decode)(struct s5p_mfc_ctx *ctx);
- int (*init_encode)(struct s5p_mfc_ctx *ctx);
- int (*encode_one_frame)(struct s5p_mfc_ctx *ctx);
void (*try_run)(struct s5p_mfc_dev *dev);
void (*clear_int_flags)(struct s5p_mfc_dev *dev);
- void (*write_info)(struct s5p_mfc_ctx *ctx, unsigned int data,
- unsigned int ofs);
- unsigned int (*read_info)(struct s5p_mfc_ctx *ctx,
- unsigned long ofs);
int (*get_dspl_y_adr)(struct s5p_mfc_dev *dev);
int (*get_dec_y_adr)(struct s5p_mfc_dev *dev);
int (*get_dspl_status)(struct s5p_mfc_dev *dev);
@@ -311,7 +299,6 @@ struct s5p_mfc_hw_ops {
int (*get_int_reason)(struct s5p_mfc_dev *dev);
int (*get_int_err)(struct s5p_mfc_dev *dev);
int (*err_dec)(unsigned int err);
- int (*err_dspl)(unsigned int err);
int (*get_img_width)(struct s5p_mfc_dev *dev);
int (*get_img_height)(struct s5p_mfc_dev *dev);
int (*get_dpb_count)(struct s5p_mfc_dev *dev);
@@ -320,10 +307,6 @@ struct s5p_mfc_hw_ops {
int (*get_enc_strm_size)(struct s5p_mfc_dev *dev);
int (*get_enc_slice_type)(struct s5p_mfc_dev *dev);
int (*get_enc_dpb_count)(struct s5p_mfc_dev *dev);
- int (*get_enc_pic_count)(struct s5p_mfc_dev *dev);
- int (*get_sei_avail_status)(struct s5p_mfc_ctx *ctx);
- int (*get_mvc_num_views)(struct s5p_mfc_dev *dev);
- int (*get_mvc_view_id)(struct s5p_mfc_dev *dev);
unsigned int (*get_pic_type_top)(struct s5p_mfc_ctx *ctx);
unsigned int (*get_pic_type_bot)(struct s5p_mfc_ctx *ctx);
unsigned int (*get_crop_info_h)(struct s5p_mfc_ctx *ctx);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
index ae4c950..8754b7e 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
@@ -1554,11 +1554,6 @@ static int s5p_mfc_err_dec_v5(unsigned int err)
return (err & S5P_FIMV_ERR_DEC_MASK) >> S5P_FIMV_ERR_DEC_SHIFT;
}
-static int s5p_mfc_err_dspl_v5(unsigned int err)
-{
- return (err & S5P_FIMV_ERR_DSPL_MASK) >> S5P_FIMV_ERR_DSPL_SHIFT;
-}
-
static int s5p_mfc_get_img_width_v5(struct s5p_mfc_dev *dev)
{
return mfc_read(dev, S5P_FIMV_SI_HRESOL);
@@ -1600,26 +1595,6 @@ static int s5p_mfc_get_enc_dpb_count_v5(struct
s5p_mfc_dev *dev)
return -1;
}
-static int s5p_mfc_get_enc_pic_count_v5(struct s5p_mfc_dev *dev)
-{
- return mfc_read(dev, S5P_FIMV_ENC_SI_PIC_CNT);
-}
-
-static int s5p_mfc_get_sei_avail_status_v5(struct s5p_mfc_ctx *ctx)
-{
- return s5p_mfc_read_info_v5(ctx, FRAME_PACK_SEI_AVAIL);
-}
-
-static int s5p_mfc_get_mvc_num_views_v5(struct s5p_mfc_dev *dev)
-{
- return -1;
-}
-
-static int s5p_mfc_get_mvc_view_id_v5(struct s5p_mfc_dev *dev)
-{
- return -1;
-}
-
static unsigned int s5p_mfc_get_pic_type_top_v5(struct s5p_mfc_ctx *ctx)
{
return s5p_mfc_read_info_v5(ctx, PIC_TIME_TOP);
@@ -1652,19 +1627,11 @@ static struct s5p_mfc_hw_ops s5p_mfc_ops_v5 = {
.release_dev_context_buffer = s5p_mfc_release_dev_context_buffer_v5,
.dec_calc_dpb_size = s5p_mfc_dec_calc_dpb_size_v5,
.enc_calc_src_size = s5p_mfc_enc_calc_src_size_v5,
- .set_dec_stream_buffer = s5
[PATCH 4/6] s5p-mfc: use spinlock to protect MFC context
MFC driver uses dev->irqlock spinlock to protect queues only, but many context
fields requires protection also - they can be accessed concurrently
from IOCTLs and IRQ handler. The patch increases protection range of irqlock
to those fields also.
Signed-off-by: Andrzej Hajda
---
drivers/media/platform/s5p-mfc/s5p_mfc.c| 15 +++
drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 2 +-
drivers/media/platform/s5p-mfc/s5p_mfc_dec.c| 13 +++--
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 14 --
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 19 ---
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 18 --
6 files changed, 11 insertions(+), 70 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 79f5c81..c4d9e34 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -359,7 +359,6 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx,
unsigned int dst_frame_status;
unsigned int dec_frame_status;
struct s5p_mfc_buf *src_buf;
- unsigned long flags;
unsigned int res_change;
dst_frame_status = s5p_mfc_hw_call(dev->mfc_ops, get_dspl_status, dev)
@@ -385,7 +384,6 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx,
if (ctx->dpb_flush_flag)
ctx->dpb_flush_flag = 0;
- spin_lock_irqsave(&dev->irqlock, flags);
/* All frames remaining in the buffer have been extracted */
if (dst_frame_status == S5P_FIMV_DEC_STATUS_DECODING_EMPTY) {
if (ctx->state == MFCINST_RES_CHANGE_FLUSH) {
@@ -445,7 +443,6 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx,
}
}
leave_handle_frame:
- spin_unlock_irqrestore(&dev->irqlock, flags);
if ((ctx->src_queue_cnt == 0 && ctx->state != MFCINST_FINISHING)
|| ctx->dst_queue_cnt < ctx->pb_count)
clear_work_bit(ctx);
@@ -464,8 +461,6 @@ leave_handle_frame:
static void s5p_mfc_handle_error(struct s5p_mfc_dev *dev,
struct s5p_mfc_ctx *ctx, unsigned int reason, unsigned int err)
{
- unsigned long flags;
-
mfc_err("Interrupt Error: %08x\n", err);
if (ctx != NULL) {
@@ -482,11 +477,9 @@ static void s5p_mfc_handle_error(struct s5p_mfc_dev *dev,
clear_work_bit(ctx);
ctx->state = MFCINST_ERROR;
/* Mark all dst buffers as having an error */
- spin_lock_irqsave(&dev->irqlock, flags);
s5p_mfc_cleanup_queue(&ctx->dst_queue, &ctx->vq_dst);
/* Mark all src buffers as having an error */
s5p_mfc_cleanup_queue(&ctx->src_queue, &ctx->vq_src);
- spin_unlock_irqrestore(&dev->irqlock, flags);
wake_up_ctx(ctx, reason, err);
break;
default:
@@ -562,7 +555,6 @@ static void s5p_mfc_handle_init_buffers(struct s5p_mfc_ctx
*ctx,
{
struct s5p_mfc_buf *src_buf;
struct s5p_mfc_dev *dev;
- unsigned long flags;
if (ctx == NULL)
return;
@@ -575,7 +567,6 @@ static void s5p_mfc_handle_init_buffers(struct s5p_mfc_ctx
*ctx,
if (err == 0) {
ctx->state = MFCINST_RUNNING;
if (!ctx->dpb_flush_flag && ctx->head_processed) {
- spin_lock_irqsave(&dev->irqlock, flags);
if (!list_empty(&ctx->src_queue)) {
src_buf = list_entry(ctx->src_queue.next,
struct s5p_mfc_buf, list);
@@ -584,7 +575,6 @@ static void s5p_mfc_handle_init_buffers(struct s5p_mfc_ctx
*ctx,
vb2_buffer_done(&src_buf->b->vb2_buf,
VB2_BUF_STATE_DONE);
}
- spin_unlock_irqrestore(&dev->irqlock, flags);
} else {
ctx->dpb_flush_flag = 0;
}
@@ -612,7 +602,6 @@ static void s5p_mfc_handle_stream_complete(struct
s5p_mfc_ctx *ctx)
ctx->state = MFCINST_FINISHED;
- spin_lock(&dev->irqlock);
if (!list_empty(&ctx->dst_queue)) {
mb_entry = list_entry(ctx->dst_queue.next, struct s5p_mfc_buf,
list);
@@ -621,7 +610,6 @@ static void s5p_mfc_handle_stream_complete(struct
s5p_mfc_ctx *ctx)
vb2_set_plane_payload(&mb_entry->b->vb2_buf, 0, 0);
vb2_buffer_done(&mb_entry->b->vb2_buf, VB2_BUF_STATE_DONE);
}
- spin_unlock(&dev->irqlock);
clear_work_bit(ctx);
@@ -643,6 +631,7 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)
mfc_debug_
