| Message ID | 25bfb7ad-0c12-3d47-81b1-6feb1906cd42@xs4all.nl (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 43266ad2b47db87b91f3198afa5ad61a4206c253 |
| Headers | show |
| Series | input/touchscreen/sur40: use COLORSPACE_RAW | expand |
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
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 >
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
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), > }
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), }
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. ---