[PATCH v2 1/2] solo6x10: Set FRAME_BUF_SIZE to 200KB

[PATCH v2 1/2] solo6x10: Set FRAME_BUF_SIZE to 200KB

[Ismael Luceno]

From: Andrey Utkin <andrey.utkin@corp.bluecherry.net>

Such frame size is met in practice. Also report oversized frames.

[ismael: Reworked warning and commit message]

Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
---
 drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
index 67a14c4..f98017b 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
@@ -33,7 +33,7 @@
 #include "solo6x10-jpeg.h"
 
 #define MIN_VID_BUFFERS		2
-#define FRAME_BUF_SIZE		(196 * 1024)
+#define FRAME_BUF_SIZE		(200 * 1024)
 #define MP4_QS			16
 #define DMA_ALIGN		4096
 
@@ -323,8 +323,11 @@ static int solo_send_desc(struct solo_enc_dev *solo_enc, int skip,
 	int i;
 	int ret;
 
-	if (WARN_ON_ONCE(size > FRAME_BUF_SIZE))
+	if (WARN_ON_ONCE(size > FRAME_BUF_SIZE)) {
+		dev_warn(&solo_dev->pdev->dev,
+			 "oversized frame (%d bytes)\n", size);
 		return -EINVAL;
+	}
 
 	solo_enc->desc_count = 1;
 
-- 
2.8.0

[PATCH v2 2/2] solo6x10: Simplify solo_enum_ext_input

[Ismael Luceno]

Additionally, now it specifies which channels it's showing.

Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
---
 drivers/media/pci/solo6x10/solo6x10-v4l2.c | 34 ++++++++++++++----------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2.c b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
index 721ff53..953d6bf 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
@@ -386,26 +386,24 @@ static int solo_querycap(struct file *file, void  *priv,
 static int solo_enum_ext_input(struct solo_dev *solo_dev,
 			       struct v4l2_input *input)
 {
-	static const char * const dispnames_1[] = { "4UP" };
-	static const char * const dispnames_2[] = { "4UP-1", "4UP-2" };
-	static const char * const dispnames_5[] = {
-		"4UP-1", "4UP-2", "4UP-3", "4UP-4", "16UP"
-	};
-	const char * const *dispnames;
-
-	if (input->index >= (solo_dev->nr_chans + solo_dev->nr_ext))
-		return -EINVAL;
-
-	if (solo_dev->nr_ext == 5)
-		dispnames = dispnames_5;
-	else if (solo_dev->nr_ext == 2)
-		dispnames = dispnames_2;
-	else
-		dispnames = dispnames_1;
+	int ext = input->index - solo_dev->nr_chans;
+	unsigned int nup, first;
 
-	snprintf(input->name, sizeof(input->name), "Multi %s",
-		 dispnames[input->index - solo_dev->nr_chans]);
+	if (ext >= solo_dev->nr_ext)
+		return -EINVAL;
 
+	nup   = (ext == 4) ? 16 : 4;
+	first = (ext & 3) << 2; /* first channel in the n-up */
+	snprintf(input->name, sizeof(input->name),
+		 "Multi %d-up (cameras %d-%d)",
+		 nup, first + 1, first + nup);
+	/* Possible outputs:
+	 *  Multi 4-up (cameras 1-4)
+	 *  Multi 4-up (cameras 5-8)
+	 *  Multi 4-up (cameras 9-12)
+	 *  Multi 4-up (cameras 13-16)
+	 *  Multi 16-up (cameras 1-16)
+	 */
 	return 0;
 }
 
-- 
2.8.0

Re: [PATCH v2 1/2] solo6x10: Set FRAME_BUF_SIZE to 200KB

[Andrey Utkin]

On Wed, May 04, 2016 at 01:21:20PM -0300, Ismael Luceno wrote:
> From: Andrey Utkin <andrey.utkin@corp.bluecherry.net>
> 
> Such frame size is met in practice. Also report oversized frames.
> 
> [ismael: Reworked warning and commit message]
> 
> Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>

I object against merging the first part.

> ---
>  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> index 67a14c4..f98017b 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> @@ -33,7 +33,7 @@
>  #include "solo6x10-jpeg.h"
>  
>  #define MIN_VID_BUFFERS		2
> -#define FRAME_BUF_SIZE		(196 * 1024)
> +#define FRAME_BUF_SIZE		(200 * 1024)

Please don't push this.
It doesn't matter whether there are 196 or 200 KiB because there happen
bigger frames.
I don't remember details so I cannot point to all time max frame size.
AFAIK this issue appeared on one particular customer installation. I
don't monitor it closely right now. I think I have compiled custom
package for that setup with FRAME_BUF_SIZE increased much more (maybe
10x?).

Re: [PATCH v2 1/2] solo6x10: Set FRAME_BUF_SIZE to 200KB

[Ismael Luceno]

On 05/Mai/2016 00:14, Andrey Utkin wrote:
> On Wed, May 04, 2016 at 01:21:20PM -0300, Ismael Luceno wrote:
> > From: Andrey Utkin <andrey.utkin@corp.bluecherry.net>
> > 
> > Such frame size is met in practice. Also report oversized frames.
> > 
> > [ismael: Reworked warning and commit message]
> > 
> > Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
> 
> I object against merging the first part.
> 
> > ---
> >  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> > index 67a14c4..f98017b 100644
> > --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> > +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> > @@ -33,7 +33,7 @@
> >  #include "solo6x10-jpeg.h"
> >  
> >  #define MIN_VID_BUFFERS		2
> > -#define FRAME_BUF_SIZE		(196 * 1024)
> > +#define FRAME_BUF_SIZE		(200 * 1024)
> 
> Please don't push this.
> It doesn't matter whether there are 196 or 200 KiB because there happen
> bigger frames.
> I don't remember details so I cannot point to all time max frame size.
> AFAIK this issue appeared on one particular customer installation. I
> don't monitor it closely right now. I think I have compiled custom
> package for that setup with FRAME_BUF_SIZE increased much more (maybe
> 10x?).

I don't quite remember the overscan, but the maximum should be around
1.2MB, so yes. If the QM hasn't been tweaked, then the image must
be terrible.

Re: [PATCH v2 1/2] solo6x10: Set FRAME_BUF_SIZE to 200KB

[Hans Verkuil]

Andrey,

Since you are the original author, can you give me your Signed-off-by line?

My apologies for the very late reply, it's been very busy lately and I am
finally catching up...

Thanks!

	Hans

On 05/04/2016 06:21 PM, Ismael Luceno wrote:
> From: Andrey Utkin <andrey.utkin@corp.bluecherry.net>
> 
> Such frame size is met in practice. Also report oversized frames.
> 
> [ismael: Reworked warning and commit message]
> 
> Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
> ---
>  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> index 67a14c4..f98017b 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> @@ -33,7 +33,7 @@
>  #include "solo6x10-jpeg.h"
>  
>  #define MIN_VID_BUFFERS		2
> -#define FRAME_BUF_SIZE		(196 * 1024)
> +#define FRAME_BUF_SIZE		(200 * 1024)
>  #define MP4_QS			16
>  #define DMA_ALIGN		4096
>  
> @@ -323,8 +323,11 @@ static int solo_send_desc(struct solo_enc_dev *solo_enc, int skip,
>  	int i;
>  	int ret;
>  
> -	if (WARN_ON_ONCE(size > FRAME_BUF_SIZE))
> +	if (WARN_ON_ONCE(size > FRAME_BUF_SIZE)) {
> +		dev_warn(&solo_dev->pdev->dev,
> +			 "oversized frame (%d bytes)\n", size);
>  		return -EINVAL;
> +	}
>  
>  	solo_enc->desc_count = 1;
>  
> 

Re: [PATCH v2 1/2] solo6x10: Set FRAME_BUF_SIZE to 200KB

[Andrey Utkin]

On Mon, Jun 27, 2016 at 11:12:42AM +0200, Hans Verkuil wrote:
> Andrey,
> 
> Since you are the original author, can you give me your Signed-off-by line?

No, as increasing buffer size by few kilobytes doesn't change anything. I've
increased it from 200 to 204, then found new occurances of the issue,
then increased it again and again by few kilobytes. Then I got that this
is not a (nice) solution, and have never came back to this. Maybe
doubling current buffer size would make users forget about this, but I'm
not sure maintainers would be glad with such patch.

Re: [PATCH v2 1/2] solo6x10: Set FRAME_BUF_SIZE to 200KB

[Hans Verkuil]

On 06/28/16 13:48, Andrey Utkin wrote:
> On Mon, Jun 27, 2016 at 11:12:42AM +0200, Hans Verkuil wrote:
>> Andrey,
>>
>> Since you are the original author, can you give me your Signed-off-by line?
> 
> No, as increasing buffer size by few kilobytes doesn't change anything. I've
> increased it from 200 to 204, then found new occurances of the issue,
> then increased it again and again by few kilobytes. Then I got that this
> is not a (nice) solution, and have never came back to this. Maybe
> doubling current buffer size would make users forget about this, but I'm
> not sure maintainers would be glad with such patch.

I don't care. Right now it doesn't work. The cause is that the buffers are
too small to handle the worst-case situation. So if doubling the size makes
it work, then that's perfectly OK. Memory is cheap these days. If it will
fail, then that's much worse than consuming a few meg more.

Ideally you can calculate what the worst-case size is, but I expect that to
be quite difficult if not impossible.

Regards,

	Hans