input/touchscreen/sur40: use COLORSPACE_RAW
diff mbox series

Message ID 25bfb7ad-0c12-3d47-81b1-6feb1906cd42@xs4all.nl
State Mainlined
Commit 43266ad2b47db87b91f3198afa5ad61a4206c253
Headers show
Series
  • input/touchscreen/sur40: use COLORSPACE_RAW
Related show

Commit Message

Hans Verkuil June 26, 2019, 9:52 a.m. UTC
This driver set the colorspace to SRGB, but that makes no sense for
a touchscreen. Use RAW instead. This also ensures consistency with the
v4l_pix_format_touch() call that's used in v4l2-ioctl.c.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
Dmitry, do you want to take this, or shall I? I have no preference.
---

Comments

Florian Echtler June 27, 2019, 8:12 a.m. UTC | #1
On 26.06.19 11:52, Hans Verkuil wrote:
> This driver set the colorspace to SRGB, but that makes no sense for
> a touchscreen. Use RAW instead. This also ensures consistency with the
> v4l_pix_format_touch() call that's used in v4l2-ioctl.c.

One question for clarification: this will only affect userspace applications
which explicitly request a certain colorspace, correct?

Best regards, Florian
Hans Verkuil June 27, 2019, 8:20 a.m. UTC | #2
On 6/27/19 10:12 AM, Florian Echtler wrote:
> On 26.06.19 11:52, Hans Verkuil wrote:
>> This driver set the colorspace to SRGB, but that makes no sense for
>> a touchscreen. Use RAW instead. This also ensures consistency with the
>> v4l_pix_format_touch() call that's used in v4l2-ioctl.c.
> 
> One question for clarification: this will only affect userspace applications
> which explicitly request a certain colorspace, correct?

You can't request a colorspace from userspace. The driver sets it.

In this case is it inconsistent anyway since VIDIOC_S_FMT will return RAW
(due to the v4l_pix_format_touch() call), but G/TRY_FMT will return SRGB
from the driver. TRY_FMT should return RAW as well, but it didn't call
v4l_pix_format_touch(), for which I posted a separate patch fixing that.

Regards,

	Hans

> 
> Best regards, Florian
>
Florian Echtler June 28, 2019, 7:17 a.m. UTC | #3
On 27.06.19 10:20, Hans Verkuil wrote:
> On 6/27/19 10:12 AM, Florian Echtler wrote:
>> On 26.06.19 11:52, Hans Verkuil wrote:
>>> This driver set the colorspace to SRGB, but that makes no sense for
>>> a touchscreen. Use RAW instead. This also ensures consistency with the
>>> v4l_pix_format_touch() call that's used in v4l2-ioctl.c.
>>
>> One question for clarification: this will only affect userspace applications
>> which explicitly request a certain colorspace, correct?
> 
> You can't request a colorspace from userspace. The driver sets it.

What I meant is: ... will only affect applications which explicitly search for
formats with a specific colorspace value.

> In this case is it inconsistent anyway since VIDIOC_S_FMT will return RAW
> (due to the v4l_pix_format_touch() call), but G/TRY_FMT will return SRGB
> from the driver. TRY_FMT should return RAW as well, but it didn't call
> v4l_pix_format_touch(), for which I posted a separate patch fixing that.

OK, understood.

Acked-by: Florian Echtler <floe@butterbrot.org>

Best regards, Florian
Dmitry Torokhov June 30, 2019, 7:23 a.m. UTC | #4
On Wed, Jun 26, 2019 at 11:52:16AM +0200, Hans Verkuil wrote:
> This driver set the colorspace to SRGB, but that makes no sense for
> a touchscreen. Use RAW instead. This also ensures consistency with the
> v4l_pix_format_touch() call that's used in v4l2-ioctl.c.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> Dmitry, do you want to take this, or shall I? I have no preference.

Please take it.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 00cb1ba2d364..3fd3e862269b 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -186,7 +186,7 @@ static const struct v4l2_pix_format sur40_pix_format[] = {
>  		.width  = SENSOR_RES_X / 2,
>  		.height = SENSOR_RES_Y / 2,
>  		.field = V4L2_FIELD_NONE,
> -		.colorspace = V4L2_COLORSPACE_SRGB,
> +		.colorspace = V4L2_COLORSPACE_RAW,
>  		.bytesperline = SENSOR_RES_X / 2,
>  		.sizeimage = (SENSOR_RES_X/2) * (SENSOR_RES_Y/2),
>  	},
> @@ -195,7 +195,7 @@ static const struct v4l2_pix_format sur40_pix_format[] = {
>  		.width  = SENSOR_RES_X / 2,
>  		.height = SENSOR_RES_Y / 2,
>  		.field = V4L2_FIELD_NONE,
> -		.colorspace = V4L2_COLORSPACE_SRGB,
> +		.colorspace = V4L2_COLORSPACE_RAW,
>  		.bytesperline = SENSOR_RES_X / 2,
>  		.sizeimage = (SENSOR_RES_X/2) * (SENSOR_RES_Y/2),
>  	}

Patch
diff mbox series

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 00cb1ba2d364..3fd3e862269b 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -186,7 +186,7 @@  static const struct v4l2_pix_format sur40_pix_format[] = {
 		.width  = SENSOR_RES_X / 2,
 		.height = SENSOR_RES_Y / 2,
 		.field = V4L2_FIELD_NONE,
-		.colorspace = V4L2_COLORSPACE_SRGB,
+		.colorspace = V4L2_COLORSPACE_RAW,
 		.bytesperline = SENSOR_RES_X / 2,
 		.sizeimage = (SENSOR_RES_X/2) * (SENSOR_RES_Y/2),
 	},
@@ -195,7 +195,7 @@  static const struct v4l2_pix_format sur40_pix_format[] = {
 		.width  = SENSOR_RES_X / 2,
 		.height = SENSOR_RES_Y / 2,
 		.field = V4L2_FIELD_NONE,
-		.colorspace = V4L2_COLORSPACE_SRGB,
+		.colorspace = V4L2_COLORSPACE_RAW,
 		.bytesperline = SENSOR_RES_X / 2,
 		.sizeimage = (SENSOR_RES_X/2) * (SENSOR_RES_Y/2),
 	}