[PATCH 0/4] [sur40] minor fixes & performance improvements

[PATCH 0/4] [sur40] minor fixes & performance improvements

[Florian Echtler]

This patch series adds several small fixes, features & performance
improvements. Many thanks to Martin Kaltenbrunner for testing the
original driver & submitting the patches. 

Martin Kaltenbrunner (4):
  reduce poll interval to allow full 60 FPS framerate
  add frame size/frame rate query functions
  add extra debug output, remove noisy warning
  return BUF_STATE_ERROR if streaming stopped during acquisition

 drivers/input/touchscreen/sur40.c | 46 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH 1/4] reduce poll interval to allow full 60 FPS framerate

[Florian Echtler]

Signed-off-by: Martin Kaltenbrunner <modin@yuri.at>
Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
 drivers/input/touchscreen/sur40.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index a24eba5..e707b8d 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -125,7 +125,7 @@ struct sur40_image_header {
 #define VIDEO_PACKET_SIZE  16384
 
 /* polling interval (ms) */
-#define POLL_INTERVAL 10
+#define POLL_INTERVAL 4
 
 /* maximum number of contacts FIXME: this is a guess? */
 #define MAX_CONTACTS 64
-- 
1.9.1

[PATCH 2/4] add frame size/frame rate query functions

[Florian Echtler]

Signed-off-by: Martin Kaltenbrunner <modin@yuri.at>
Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
 drivers/input/touchscreen/sur40.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index e707b8d..d860d05 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -778,6 +778,33 @@ static int sur40_vidioc_enum_fmt(struct file *file, void *priv,
 	return 0;
 }
 
+static int sur40_vidioc_enum_framesizes(struct file *file, void *priv,
+					struct v4l2_frmsizeenum *f)
+{
+	if ((f->index != 0) || (f->pixel_format != V4L2_PIX_FMT_GREY))
+		return -EINVAL;
+
+	f->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+	f->discrete.width  = sur40_video_format.width;
+	f->discrete.height = sur40_video_format.height;
+	return 0;
+}
+
+static int sur40_vidioc_enum_frameintervals(struct file *file, void *priv,
+					    struct v4l2_frmivalenum *f)
+{
+	if ((f->index > 1) || (f->pixel_format != V4L2_PIX_FMT_GREY)
+		|| (f->width  != sur40_video_format.width)
+		|| (f->height != sur40_video_format.height))
+			return -EINVAL;
+
+	f->type = V4L2_FRMIVAL_TYPE_DISCRETE;
+	f->discrete.denominator  = 60/(f->index+1);
+	f->discrete.numerator = 1;
+	return 0;
+}
+
+
 static const struct usb_device_id sur40_table[] = {
 	{ USB_DEVICE(ID_MICROSOFT, ID_SUR40) },  /* Samsung SUR40 */
 	{ }                                      /* terminating null entry */
@@ -829,6 +856,9 @@ static const struct v4l2_ioctl_ops sur40_video_ioctl_ops = {
 	.vidioc_s_fmt_vid_cap	= sur40_vidioc_fmt,
 	.vidioc_g_fmt_vid_cap	= sur40_vidioc_fmt,
 
+	.vidioc_enum_framesizes = sur40_vidioc_enum_framesizes,
+	.vidioc_enum_frameintervals = sur40_vidioc_enum_frameintervals,
+
 	.vidioc_enum_input	= sur40_vidioc_enum_input,
 	.vidioc_g_input		= sur40_vidioc_g_input,
 	.vidioc_s_input		= sur40_vidioc_s_input,
-- 
1.9.1

[PATCH 3/4] add extra debug output, remove noisy warning

[Florian Echtler]

Signed-off-by: Martin Kaltenbrunner <modin@yuri.at>
Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
 drivers/input/touchscreen/sur40.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index d860d05..8add986 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -342,7 +342,7 @@ static void sur40_poll(struct input_polled_dev *polldev)
 		 * instead of at the end.
 		 */
 		if (packet_id != header->packet_id)
-			dev_warn(sur40->dev, "packet ID mismatch\n");
+			dev_dbg(sur40->dev, "packet ID mismatch\n");
 
 		packet_blobs = result / sizeof(struct sur40_blob);
 		dev_dbg(sur40->dev, "received %d blobs\n", packet_blobs);
@@ -389,6 +389,8 @@ static void sur40_process_video(struct sur40_state *sur40)
 	list_del(&new_buf->list);
 	spin_unlock(&sur40->qlock);
 
+	dev_dbg(sur40->dev, "buffer acquired\n");
+
 	/* retrieve data via bulk read */
 	result = usb_bulk_msg(sur40->usbdev,
 			usb_rcvbulkpipe(sur40->usbdev, VIDEO_ENDPOINT),
@@ -416,6 +418,8 @@ static void sur40_process_video(struct sur40_state *sur40)
 		goto err_poll;
 	}
 
+	dev_dbg(sur40->dev, "header acquired\n");
+
 	sgt = vb2_dma_sg_plane_desc(&new_buf->vb, 0);
 
 	result = usb_sg_init(&sgr, sur40->usbdev,
@@ -432,11 +436,14 @@ static void sur40_process_video(struct sur40_state *sur40)
 		goto err_poll;
 	}
 
+	dev_dbg(sur40->dev, "image acquired\n");
+
 	/* mark as finished */
 	v4l2_get_timestamp(&new_buf->vb.v4l2_buf.timestamp);
 	new_buf->vb.v4l2_buf.sequence = sur40->sequence++;
 	new_buf->vb.v4l2_buf.field = V4L2_FIELD_NONE;
 	vb2_buffer_done(&new_buf->vb, VB2_BUF_STATE_DONE);
+	dev_dbg(sur40->dev, "buffer marked done\n");
 	return;
 
 err_poll:
-- 
1.9.1

[PATCH 4/4] return BUF_STATE_ERROR if streaming stopped during acquisition

[Florian Echtler]

Signed-off-by: Martin Kaltenbrunner <modin@yuri.at>
Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
 drivers/input/touchscreen/sur40.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 8add986..8be7b9b 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -438,6 +438,10 @@ static void sur40_process_video(struct sur40_state *sur40)
 
 	dev_dbg(sur40->dev, "image acquired\n");
 
+	/* return error if streaming was stopped in the meantime */
+	if (sur40->sequence == -1)
+		goto err_poll;
+
 	/* mark as finished */
 	v4l2_get_timestamp(&new_buf->vb.v4l2_buf.timestamp);
 	new_buf->vb.v4l2_buf.sequence = sur40->sequence++;
@@ -723,6 +727,7 @@ static int sur40_start_streaming(struct vb2_queue *vq, unsigned int count)
 static void sur40_stop_streaming(struct vb2_queue *vq)
 {
 	struct sur40_state *sur40 = vb2_get_drv_priv(vq);
+	sur40->sequence = -1;
 
 	/* Release all active buffers */
 	return_all_buffers(sur40, VB2_BUF_STATE_ERROR);
-- 
1.9.1

Re: [PATCH 0/4] [sur40] minor fixes & performance improvements

[Hans Verkuil]

Hi Florian,

On 05/21/2015 02:29 PM, Florian Echtler wrote:
> This patch series adds several small fixes, features & performance
> improvements. Many thanks to Martin Kaltenbrunner for testing the
> original driver & submitting the patches. 
> 
> Martin Kaltenbrunner (4):
>   reduce poll interval to allow full 60 FPS framerate
>   add frame size/frame rate query functions
>   add extra debug output, remove noisy warning
>   return BUF_STATE_ERROR if streaming stopped during acquisition
> 
>  drivers/input/touchscreen/sur40.c | 46 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 

The patches look good, but can you repost with better commit logs (i.e. not
just a subject line). Maintainers have become picky about that and without logs
Mauro most likely will not accept it. Actually, I'm not even going to try :-)

Regards,

	Hans

Re: [PATCH 0/4] [sur40] minor fixes & performance improvements

[Florian Echtler]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

On 25.05.2015 13:22, Hans Verkuil wrote:
> On 05/21/2015 02:29 PM, Florian Echtler wrote:
>> This patch series adds several small fixes, features & performance
>> improvements. Many thanks to Martin Kaltenbrunner for testing the
>> original driver & submitting the patches. 
> 
> The patches look good, but can you repost with better commit logs (i.e. not
> just a subject line). Maintainers have become picky about that and without logs
> Mauro most likely will not accept it. Actually, I'm not even going to try :-)

OK, will do that later today. Should I just send it as a new patch
series, or in reply to the first one? What's the "best practice" here?

Best, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 0/4] [sur40] minor fixes & performance improvements

[Hans Verkuil]

On 05/25/2015 01:38 PM, Florian Echtler wrote:
> On 25.05.2015 13:22, Hans Verkuil wrote:
>> On 05/21/2015 02:29 PM, Florian Echtler wrote:
>>> This patch series adds several small fixes, features & performance
>>> improvements. Many thanks to Martin Kaltenbrunner for testing the
>>> original driver & submitting the patches. 
>>
>> The patches look good, but can you repost with better commit logs (i.e. not
>> just a subject line). Maintainers have become picky about that and without logs
>> Mauro most likely will not accept it. Actually, I'm not even going to try :-)
> 
> OK, will do that later today. Should I just send it as a new patch
> series, or in reply to the first one? What's the "best practice" here?

New patch series, just prefixed with '[PATCHv2 x/4]'.

Thanks!

	Hans