| Message ID | 20201208210150.20001-1-vasyl.vavrychuk@opensynergy.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RESEND,v2] virtio-input: add multi-touch support | expand |
Hi Vasyl, On Tue, Dec 08, 2020 at 11:01:50PM +0200, Vasyl Vavrychuk wrote: > From: Mathias Crombez <mathias.crombez@faurecia.com> > > Without multi-touch slots allocated, ABS_MT_SLOT events will be lost by > input_handle_abs_event. > > Signed-off-by: Mathias Crombez <mathias.crombez@faurecia.com> > Signed-off-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> > Tested-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> > --- > v2: fix patch corrupted by corporate email server > > drivers/virtio/Kconfig | 11 +++++++++++ > drivers/virtio/virtio_input.c | 8 ++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > index 7b41130d3f35..2cfd5b01d96d 100644 > --- a/drivers/virtio/Kconfig > +++ b/drivers/virtio/Kconfig > @@ -111,6 +111,17 @@ config VIRTIO_INPUT > > If unsure, say M. > > +config VIRTIO_INPUT_MULTITOUCH_SLOTS > + depends on VIRTIO_INPUT > + int "Number of multitouch slots" > + range 0 64 > + default 10 > + help > + Define the number of multitouch slots used. Default to 10. > + This parameter is unused if there is no multitouch capability. I believe the number of slots should be communicated to the guest by the host, similarly to how the rest of input device capabilities is transferred, instead of having static compile-time option. > + > + 0 will disable the feature. > + > config VIRTIO_MMIO > tristate "Platform bus driver for memory mapped virtio devices" > depends on HAS_IOMEM && HAS_DMA > diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c > index f1f6208edcf5..13f3d90e6c30 100644 > --- a/drivers/virtio/virtio_input.c > +++ b/drivers/virtio/virtio_input.c > @@ -7,6 +7,7 @@ > > #include <uapi/linux/virtio_ids.h> > #include <uapi/linux/virtio_input.h> > +#include <linux/input/mt.h> > > struct virtio_input { > struct virtio_device *vdev; > @@ -205,6 +206,7 @@ static int virtinput_probe(struct virtio_device *vdev) > unsigned long flags; > size_t size; > int abs, err; > + bool is_mt = false; > > if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) > return -ENODEV; > @@ -287,9 +289,15 @@ static int virtinput_probe(struct virtio_device *vdev) > for (abs = 0; abs < ABS_CNT; abs++) { > if (!test_bit(abs, vi->idev->absbit)) > continue; > + if (input_is_mt_value(abs)) > + is_mt = true; > virtinput_cfg_abs(vi, abs); > } > } > + if (is_mt) > + input_mt_init_slots(vi->idev, > + CONFIG_VIRTIO_INPUT_MULTITOUCH_SLOTS, > + INPUT_MT_DIRECT); Here errors need to be handled. > > virtio_device_ready(vdev); > vi->ready = true; > -- > 2.23.0 > Thanks.
On Tue, Dec 08, 2020 at 11:01:50PM +0200, Vasyl Vavrychuk wrote: > From: Mathias Crombez <mathias.crombez@faurecia.com> > > Without multi-touch slots allocated, ABS_MT_SLOT events will be lost by > input_handle_abs_event. > > Signed-off-by: Mathias Crombez <mathias.crombez@faurecia.com> > Signed-off-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> > Tested-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> > --- > v2: fix patch corrupted by corporate email server > > drivers/virtio/Kconfig | 11 +++++++++++ > drivers/virtio/virtio_input.c | 8 ++++++++ > 2 files changed, 19 insertions(+) <formletter> This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. </formletter>
On Tue, Dec 08, 2020 at 11:01:50PM +0200, Vasyl Vavrychuk wrote: > From: Mathias Crombez <mathias.crombez@faurecia.com> > Cc: stable@vger.kernel.org I don't believe this is appropriate for stable, looks like a new feature to me. > > Without multi-touch slots allocated, ABS_MT_SLOT events will be lost by > input_handle_abs_event. > > Signed-off-by: Mathias Crombez <mathias.crombez@faurecia.com> > Signed-off-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> > Tested-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> > --- > v2: fix patch corrupted by corporate email server > > drivers/virtio/Kconfig | 11 +++++++++++ > drivers/virtio/virtio_input.c | 8 ++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > index 7b41130d3f35..2cfd5b01d96d 100644 > --- a/drivers/virtio/Kconfig > +++ b/drivers/virtio/Kconfig > @@ -111,6 +111,17 @@ config VIRTIO_INPUT > > If unsure, say M. > > +config VIRTIO_INPUT_MULTITOUCH_SLOTS > + depends on VIRTIO_INPUT > + int "Number of multitouch slots" > + range 0 64 > + default 10 > + help > + Define the number of multitouch slots used. Default to 10. > + This parameter is unused if there is no multitouch capability. > + > + 0 will disable the feature. > + Most people won't be using this config so the defaults matter. So why 10? 10 fingers? And where does 64 come from? > config VIRTIO_MMIO > tristate "Platform bus driver for memory mapped virtio devices" > depends on HAS_IOMEM && HAS_DMA > diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c > index f1f6208edcf5..13f3d90e6c30 100644 > --- a/drivers/virtio/virtio_input.c > +++ b/drivers/virtio/virtio_input.c > @@ -7,6 +7,7 @@ > > #include <uapi/linux/virtio_ids.h> > #include <uapi/linux/virtio_input.h> > +#include <linux/input/mt.h> > > struct virtio_input { > struct virtio_device *vdev; > @@ -205,6 +206,7 @@ static int virtinput_probe(struct virtio_device *vdev) > unsigned long flags; > size_t size; > int abs, err; > + bool is_mt = false; > > if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) > return -ENODEV; > @@ -287,9 +289,15 @@ static int virtinput_probe(struct virtio_device *vdev) > for (abs = 0; abs < ABS_CNT; abs++) { > if (!test_bit(abs, vi->idev->absbit)) > continue; > + if (input_is_mt_value(abs)) > + is_mt = true; > virtinput_cfg_abs(vi, abs); > } > } > + if (is_mt) > + input_mt_init_slots(vi->idev, > + CONFIG_VIRTIO_INPUT_MULTITOUCH_SLOTS, > + INPUT_MT_DIRECT); Do we need the number in config space maybe? And maybe with a feature bit so host can find out whether guest supports MT? > > virtio_device_ready(vdev); > vi->ready = true; > -- > 2.23.0
Hi, Dmitry, Thanks for you suggestion. I have sent v3 version of the patch where I have applied it. Kind regards, Vasyl On 09.12.20 00:05, Dmitry Torokhov wrote: > CAUTION: This email originated from outside of the organization. > Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > Hi Vasyl, > > On Tue, Dec 08, 2020 at 11:01:50PM +0200, Vasyl Vavrychuk wrote: >> From: Mathias Crombez <mathias.crombez@faurecia.com> >> >> Without multi-touch slots allocated, ABS_MT_SLOT events will be lost by >> input_handle_abs_event. >> >> Signed-off-by: Mathias Crombez <mathias.crombez@faurecia.com> >> Signed-off-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> >> Tested-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> >> --- >> v2: fix patch corrupted by corporate email server >> >> drivers/virtio/Kconfig | 11 +++++++++++ >> drivers/virtio/virtio_input.c | 8 ++++++++ >> 2 files changed, 19 insertions(+) >> >> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig >> index 7b41130d3f35..2cfd5b01d96d 100644 >> --- a/drivers/virtio/Kconfig >> +++ b/drivers/virtio/Kconfig >> @@ -111,6 +111,17 @@ config VIRTIO_INPUT >> >> If unsure, say M. >> >> +config VIRTIO_INPUT_MULTITOUCH_SLOTS >> + depends on VIRTIO_INPUT >> + int "Number of multitouch slots" >> + range 0 64 >> + default 10 >> + help >> + Define the number of multitouch slots used. Default to 10. >> + This parameter is unused if there is no multitouch capability. > > I believe the number of slots should be communicated to the guest by > the host, similarly to how the rest of input device capabilities is > transferred, instead of having static compile-time option. > >> + >> + 0 will disable the feature. >> + >> config VIRTIO_MMIO >> tristate "Platform bus driver for memory mapped virtio devices" >> depends on HAS_IOMEM && HAS_DMA >> diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c >> index f1f6208edcf5..13f3d90e6c30 100644 >> --- a/drivers/virtio/virtio_input.c >> +++ b/drivers/virtio/virtio_input.c >> @@ -7,6 +7,7 @@ >> >> #include <uapi/linux/virtio_ids.h> >> #include <uapi/linux/virtio_input.h> >> +#include <linux/input/mt.h> >> >> struct virtio_input { >> struct virtio_device *vdev; >> @@ -205,6 +206,7 @@ static int virtinput_probe(struct virtio_device *vdev) >> unsigned long flags; >> size_t size; >> int abs, err; >> + bool is_mt = false; >> >> if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) >> return -ENODEV; >> @@ -287,9 +289,15 @@ static int virtinput_probe(struct virtio_device *vdev) >> for (abs = 0; abs < ABS_CNT; abs++) { >> if (!test_bit(abs, vi->idev->absbit)) >> continue; >> + if (input_is_mt_value(abs)) >> + is_mt = true; >> virtinput_cfg_abs(vi, abs); >> } >> } >> + if (is_mt) >> + input_mt_init_slots(vi->idev, >> + CONFIG_VIRTIO_INPUT_MULTITOUCH_SLOTS, >> + INPUT_MT_DIRECT); > > Here errors need to be handled. > >> >> virtio_device_ready(vdev); >> vi->ready = true; >> -- >> 2.23.0 >> > > Thanks. > > -- > Dmitry >
On 09.12.20 10:28, Michael S. Tsirkin wrote: > On Tue, Dec 08, 2020 at 11:01:50PM +0200, Vasyl Vavrychuk wrote: >> From: Mathias Crombez <mathias.crombez@faurecia.com> >> Cc: stable@vger.kernel.org > > I don't believe this is appropriate for stable, looks like > a new feature to me. Agree, removed. >> >> +config VIRTIO_INPUT_MULTITOUCH_SLOTS >> + depends on VIRTIO_INPUT >> + int "Number of multitouch slots" >> + range 0 64 >> + default 10 >> + help >> + Define the number of multitouch slots used. Default to 10. >> + This parameter is unused if there is no multitouch capability. >> + >> + 0 will disable the feature. >> + > > Most people won't be using this config so the defaults matter. So why 10? 10 fingers? > > And where does 64 come from? I have sent v3 version where number of slots it obtained from the host. >> + if (is_mt) >> + input_mt_init_slots(vi->idev, >> + CONFIG_VIRTIO_INPUT_MULTITOUCH_SLOTS, >> + INPUT_MT_DIRECT); > > > Do we need the number in config space maybe? And maybe with a feature > bit so host can find out whether guest supports MT? I think it is not applicable in v3 which I sent, because number of slots is commit from the host. So, now host controls whether guest support MT.
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index 7b41130d3f35..2cfd5b01d96d 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -111,6 +111,17 @@ config VIRTIO_INPUT If unsure, say M. +config VIRTIO_INPUT_MULTITOUCH_SLOTS + depends on VIRTIO_INPUT + int "Number of multitouch slots" + range 0 64 + default 10 + help + Define the number of multitouch slots used. Default to 10. + This parameter is unused if there is no multitouch capability. + + 0 will disable the feature. + config VIRTIO_MMIO tristate "Platform bus driver for memory mapped virtio devices" depends on HAS_IOMEM && HAS_DMA diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c index f1f6208edcf5..13f3d90e6c30 100644 --- a/drivers/virtio/virtio_input.c +++ b/drivers/virtio/virtio_input.c @@ -7,6 +7,7 @@ #include <uapi/linux/virtio_ids.h> #include <uapi/linux/virtio_input.h> +#include <linux/input/mt.h> struct virtio_input { struct virtio_device *vdev; @@ -205,6 +206,7 @@ static int virtinput_probe(struct virtio_device *vdev) unsigned long flags; size_t size; int abs, err; + bool is_mt = false; if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) return -ENODEV; @@ -287,9 +289,15 @@ static int virtinput_probe(struct virtio_device *vdev) for (abs = 0; abs < ABS_CNT; abs++) { if (!test_bit(abs, vi->idev->absbit)) continue; + if (input_is_mt_value(abs)) + is_mt = true; virtinput_cfg_abs(vi, abs); } } + if (is_mt) + input_mt_init_slots(vi->idev, + CONFIG_VIRTIO_INPUT_MULTITOUCH_SLOTS, + INPUT_MT_DIRECT); virtio_device_ready(vdev); vi->ready = true;