Add macros to enable request relative mode.
Change the REL_Y value passed by input_report_rel function,
to match the direction of mouse movement.
Signed-off-by: Zongmin Zhou<[email protected]>
---
drivers/input/mouse/vmmouse.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index ea9eff7c8099..f39ce21f7af9 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -21,6 +21,12 @@
#include "psmouse.h"
#include "vmmouse.h"
+/*
+ * Enable this to request relative mode.
+ * Defaut to absolute mode.
+ */
+//#define VMMOUSE_RELATIVE_MODE
+
#define VMMOUSE_PROTO_MAGIC 0x564D5868U
/*
@@ -184,7 +190,7 @@ static psmouse_ret_t vmmouse_report_events(struct psmouse *psmouse)
if (status & VMMOUSE_RELATIVE_PACKET) {
pref_dev = rel_dev;
input_report_rel(rel_dev, REL_X, (s32)x);
- input_report_rel(rel_dev, REL_Y, -(s32)y);
+ input_report_rel(rel_dev, REL_Y, (s32)y);
} else {
pref_dev = abs_dev;
input_report_abs(abs_dev, ABS_X, x);
@@ -304,8 +310,13 @@ static int vmmouse_enable(struct psmouse *psmouse)
VMMOUSE_CMD(ABSPOINTER_RESTRICT, VMMOUSE_RESTRICT_CPL0,
dummy1, dummy2, dummy3, dummy4);
+#ifdef VMMOUSE_RELATIVE_MODE
+ VMMOUSE_CMD(ABSPOINTER_COMMAND, VMMOUSE_CMD_REQUEST_RELATIVE,
+ dummy1, dummy2, dummy3, dummy4);
+#else
VMMOUSE_CMD(ABSPOINTER_COMMAND, VMMOUSE_CMD_REQUEST_ABSOLUTE,
dummy1, dummy2, dummy3, dummy4);
+#endif
return 0;
}
--
2.34.1
No virus found
Checked by Hillstone Network AntiVirus
On Thu, 2023-04-13 at 16:56 +0800, Zongmin Zhou wrote:
> Add macros to enable request relative mode.
>
> Change the REL_Y value passed by input_report_rel function,
> to match the direction of mouse movement.
Thanks for the patch, but in its current form it's a nack. First of all we don't
want any defines in the driver code that affect compilation, it's never going to be
tested or compiled in. Either a kconfig or a module parameter would be acceptable
but that's only if you can actually explain what it is that you're fixing. The
current single line description just mentions what the effect it has (not completely
correctly either because for merged packets absolute will still be x=x, y=y, but
relative will be x += dx, y -= dy) but not why it's done, what it's fixing and how
to reproduce.
z
On Fri, 2023-04-14 at 01:22 +0000, Zack Rusin wrote:
> On Thu, 2023-04-13 at 16:56 +0800, Zongmin Zhou wrote:
> > Add macros to enable request relative mode.
> >
> > Change the REL_Y value passed by input_report_rel function,
> > to match the direction of mouse movement.
>
> Thanks for the patch, but in its current form it's a nack. First of
> all we don't
> want any defines in the driver code that affect compilation, it's
> never going to be
> tested or compiled in. Either a kconfig or a module parameter would
> be acceptable
> but that's only if you can actually explain what it is that you're
> fixing. The
> current single line description just mentions what the effect it has
> (not completely
> correctly either because for merged packets absolute will still be
> x=x, y=y, but
> relative will be x += dx, y -= dy) but not why it's done, what it's
> fixing and how
> to reproduce.
>
> z
>
Dear zack:
Firstly,thanks for your reply.
The reason I want to add macros to request different vmmouse
modes(relative or absolute) is that the vmmouse drivers currently only
supports request absolute mode.But in some case we want request
relative mode so that Pointer acceleration feature can be used.(as I
know,libinput module only support Pointer acceleration feature in
relative mode.)
So I think we can provide two vmmouse modes to facilitate the use of
different needs.
If need,I can change it to a kconfig or a module parameter.
The reasons of fix for REL_Y value，are as follows:
When I request relative vmmouse mode,and let mouse move up,the mouse
pointer moved down instead.
Similarly, when I move the mouse down, the mouse pointer moved up.
it obviously with a wrong motion direction in y.
Actually，I understand that the value of y here is the end calculation
result of relative coordinate movement，the real calculation is in
motion_event() of spice-gtk and legacy_mouse_event() of qemu.
Test scenario:
1) start virtual machine with qemu command "vmport=on"，also with spice
protocal.
2) modify guest vmmouse drivers to request relative mode.
3) move the mouse,will observe the pointer freezed,it's because driver
not match the condition 'status & VMMOUSE_RELATIVE_PACKET',can't find
correct input device.need merge this patch in qemu:
https://lore.kernel.org/all/[email protected]/
4) after merge the patch in qemu,we can observe the issue of wrong
motion direction in y.
Looking forward to your reply.
On Fri, 2023-04-14 at 16:58 +0800, zongmin zhou wrote:
> On Fri, 2023-04-14 at 01:22 +0000, Zack Rusin wrote:
> > On Thu, 2023-04-13 at 16:56 +0800, Zongmin Zhou wrote:
> > > Add macros to enable request relative mode.
> > >
> > > Change the REL_Y value passed by input_report_rel function,
> > > to match the direction of mouse movement.
> >
> > Thanks for the patch, but in its current form it's a nack. First of
> > all we don't
> > want any defines in the driver code that affect compilation, it's
> > never going to be
> > tested or compiled in. Either a kconfig or a module parameter would
> > be acceptable
> > but that's only if you can actually explain what it is that you're
> > fixing. The
> > current single line description just mentions what the effect it has
> > (not completely
> > correctly either because for merged packets absolute will still be
> > x=x, y=y, but
> > relative will be x += dx, y -= dy) but not why it's done, what it's
> > fixing and how
> > to reproduce.
> >
> > z
> >
> Dear zack:
>
> Firstly,thanks for your reply.
>
> The reason I want to add macros to request different vmmouse
> modes(relative or absolute) is that the vmmouse drivers currently only
> supports request absolute mode.But in some case we want request
> relative mode so that Pointer acceleration feature can be used.(as I
> know,libinput module only support Pointer acceleration feature in
> relative mode.)
> So I think we can provide two vmmouse modes to facilitate the use of
> different needs.
> If need,I can change it to a kconfig or a module parameter.
>
> The reasons of fix for REL_Y value，are as follows:
> When I request relative vmmouse mode,and let mouse move up,the mouse
> pointer moved down instead.
> Similarly, when I move the mouse down, the mouse pointer moved up.
> it obviously with a wrong motion direction in y.
>
> Actually，I understand that the value of y here is the end calculation
> result of relative coordinate movement，the real calculation is in
> motion_event() of spice-gtk and legacy_mouse_event() of qemu.
>
> Test scenario:
> 1) start virtual machine with qemu command "vmport=on"，also with spice
> protocal.
> 2) modify guest vmmouse drivers to request relative mode.
> 3) move the mouse,will observe the pointer freezed,it's because driver
> not match the condition 'status & VMMOUSE_RELATIVE_PACKET',can't find
> correct input device.need merge this patch in qemu:
> https://lore.kernel.org/all/[email protected]/
> 4) after merge the patch in qemu,we can observe the issue of wrong
> motion direction in y.
Sounds like you have a bug in the monitor code to me. The mouse might request
relative mode, but that doesn't mean that it's going to be switched, it's a hint. On
enable we definitely want to request the default absolute mode.
Ultimately it's the delivery mechanism (i.e. whether the delivered event is
VMMOUSE_RELATIVE_PACKET or not) that defines what an event is. This sounds to me
like the monitor delivers VMMOUSE_RELATIVE_PACKET events but doesn't mark them as
such. You can confirm by putting some debugging output in the vmmouse_report_events.
z
On Thu, 2023-04-20 at 09:49 +0800, 周宗敏 wrote:
> Dear zack:
>
> As far as I know, I think in the current design for vmmouse device,
> the mouse mode can only choose one,can't request both two mode.
>
> The flowchart for vmmouse device between host and guest roughly as follows：
> QEMU VMMouse emulation code defined variables of 's->absolute'
> to identify the mouse mode requested by the guest driver.
> Based on the value of 's->absolute',qemu will add different spice-input VD-
> Interface,
> spice-server will notify client to use the correct mouse mode,
> and then spice client will update mouse motion/position based on the mouse mode.
>
> About the VMMOUSE_RELATIVE_PACKET events,I guess that designer may want use it
> to distinguish relative from absolute of the process events.But doesn't mark them
> as such
> on QEMU's vmmouse device code.In fact, the status corresponds to the following
> buttons value on QEMU：
>
> From the screenshot we can know it didn't mark the mouse mode status on original
> design,
> only set the actual button state. So I think have to mark it here according to the
> value of 's->absolute'.
Hi,
we had a quick chat about this internally and I don't think we can make this work as
is. The core issue is that this driver is for VMware's mouse, if qemu emulation of
that device diverges from that then we can't make it work, i.e. if the device that
qemu emulates doesn't match the behaviour of vmmouse then it's not a vmmouse.
I think there are three possible solutions here:
1) Fix qemu so that it properly flags relative vs absolute events
2) Implement some capabilities mechanism so that we can shoehorn what effectively is
a different device that happens to have the same pci id into this driver (not ideal)
3) Fork the driver, make any changes you wish and just remember to change the device
detection for the qemu mouse so it differs from vmmouse. The driver is fairly simple
and is not too big, if fixing qemu is not an option then you effectively have a
different device and then just properly treat it as such. From your initial patch
where you wanted to have the relative flag as a define, it seems that your kernel is
custom anyway (because otherwise you wouldn't be able to set that define) so this
should be trivially doable on your config. And long term you can submit the new
driver to the kernel (as long as you can make sure the mouse detection code doesn't
detect the actual vmmouse of course to conflict with this driver).
z
On Thu, 2023-04-20 at 09:49 +0800, 周宗敏 wrote:
> > Dear zack:
> >
> > As far as I know, I think in the current design for vmmouse device,
> > the mouse mode can only choose one,can't request both two mode.
> >
> > The flowchart for vmmouse device between host and guest roughly as
> > > follows：
picture link: https://imgs.wiki/imgs/2023/04/28/1898b0428ecb366a.png
> > QEMU VMMouse emulation code defined variables of 's->absolute'
> > to identify the mouse mode requested by the guest driver.
> > Based on the value of 's->absolute',qemu will add different >
> > spice-input VD-Interface,
> > spice-server will notify client to use the correct mouse mode,
> > and then spice client will update mouse motion/position based on
> > the > mouse mode.
> >
> > About the VMMOUSE_RELATIVE_PACKET events,I guess that designer may
> > > want use it
> > to distinguish relative from absolute of the process events.But >
> > doesn't mark them as such
> > on QEMU's vmmouse device code.In fact, the status corresponds to
> > the > following buttons value on QEMU：
link: https://imgs.wiki/imgs/2023/04/28/2c6a92c33e2537dd.png
> >
> > From the screenshot we can know it didn't mark the mouse mode
> > status > on original design,
> > only set the actual button state. So I think have to mark it here >
> > according to the value of 's->absolute'.
> >
Dear zack:
I guess the last mail sent to you may has been lost
as I don't see it in lore.kernel.org.Just resending it.
If you have any questions, please let me know.
Thanks.
> > Best Regards.
> >
> > ----
> >
> >
> >
> >
> > 主 题：Re: [PATCH] Input: vmmouse - add macros to enable vmmouse >
> > relative mode
> > 日 期：2023-04-14 23:32
> > 发件人：[email protected]
> > 收件人：[email protected];Pv-
> > drivers;[email protected];Linux> -graphics-maintainer;
> >
> > On Fri, 2023-04-14 at 16:58 +0800, zongmin zhou wrote:
> > > > On Fri, 2023-04-14 at 01:22 +0000, Zack Rusin wrote:
> > > > > > On Thu, 2023-04-13 at 16:56 +0800, Zongmin Zhou wrote:
> > > > > > > > Add macros to enable request relative mode.
> > > > > > > >
> > > > > > > > Change the REL_Y value passed by input_report_rel
> > > > > > > > function,
> > > > > > > > to match the direction of mouse movement.
> > > > > >
> > > > > > Thanks for the patch, but in its current form it's a nack.
> > > > > > First > > > of
> > > > > > all we don't
> > > > > > want any defines in the driver code that affect
> > > > > > compilation, it's
> > > > > > never going to be
> > > > > > tested or compiled in. Either a kconfig or a module
> > > > > > parameter > > > would
> > > > > > be acceptable
> > > > > > but that's only if you can actually explain what it is that
> > > > > > > > > you're
> > > > > > fixing. The
> > > > > > current single line description just mentions what the
> > > > > > effect it > > > has
> > > > > > (not completely
> > > > > > correctly either because for merged packets absolute will
> > > > > > still > > > be
> > > > > > x=x, y=y, but
> > > > > > relative will be x += dx, y -= dy) but not why it's done,
> > > > > > what > > > it's
> > > > > > fixing and how
> > > > > > to reproduce.
> > > > > >
> > > > > > z
> > > > > >
> > > > Dear zack:
> > > >
> > > > Firstly,thanks for your reply.
> > > >
> > > > The reason I want to add macros to request different vmmouse
> > > > modes(relative or absolute) is that the vmmouse drivers
> > > > currently > > only
> > > > supports request absolute mode.But in some case we want request
> > > > relative mode so that Pointer acceleration feature can be
> > > > used.(as > > I
> > > > know,libinput module only support Pointer acceleration feature
> > > > in
> > > > relative mode.)
> > > > So I think we can provide two vmmouse modes to facilitate the
> > > > use > > of
> > > > different needs.
> > > > If need,I can change it to a kconfig or a module parameter.
> > > >
> > > > The reasons of fix for REL_Y value，are as follows:
> > > > When I request relative vmmouse mode,and let mouse move up,the
> > > > > > mouse
> > > > pointer moved down instead.
> > > > Similarly, when I move the mouse down, the mouse pointer moved
> > > > up.
> > > > it obviously with a wrong motion direction in y.
> > > >
> > > > Actually，I understand that the value of y here is the end > >
> > > > calculation
> > > > result of relative coordinate movement，the real calculation is
> > > > in
> > > > motion_event() of spice-gtk and legacy_mouse_event() of qemu.
> > > >
> > > > Test scenario:
> > > > 1) start virtual machine with qemu command "vmport=on"，also
> > > > with > > spice
> > > > protocal.
> > > > 2) modify guest vmmouse drivers to request relative mode.
> > > > 3) move the mouse,will observe the pointer freezed,it's because
> > > > > > driver
> > > > not match the condition 'status &
> > > > VMMOUSE_RELATIVE_PACKET',can't > > find
> > > > correct input device.need merge this patch in qemu:
> > > > https://lore.kernel.org/all/[email protected]/
> > > > 4) after merge the patch in qemu,we can observe the issue of
> > > > wrong
> > > > motion direction in y.
> >
> > Sounds like you have a bug in the monitor code to me. The mouse
> > might > request
> > relative mode, but that doesn't mean that it's going to be
> > switched, > it's a hint. On
> > enable we definitely want to request the default absolute mode.
> >
> > Ultimately it's the delivery mechanism (i.e. whether the delivered
> > > event is
> > VMMOUSE_RELATIVE_PACKET or not) that defines what an event is. This
> > > sounds to me
> > like the monitor delivers VMMOUSE_RELATIVE_PACKET events but
> > doesn't > mark them as
> > such. You can confirm by putting some debugging output in the >
> > vmmouse_report_events.
> >
> > z
> >
On Fri, 2023-04-28 at 10:34 +0800, zongmin zhou wrote:
> On Thu, 2023-04-20 at 09:49 +0800, 周宗敏 wrote:
> > > Dear zack:
> > >
> > > As far as I know, I think in the current design for vmmouse device,
> > > the mouse mode can only choose one,can't request both two mode.
> > >
> > > The flowchart for vmmouse device between host and guest roughly as
> > > > follows：
> picture link:
> https://imgs.wiki/imgs/2023/04/28/1898b0428ecb366a.png
>
> > > QEMU VMMouse emulation code defined variables of 's->absolute'
> > > to identify the mouse mode requested by the guest driver.
> > > Based on the value of 's->absolute',qemu will add different >
> > > spice-input VD-Interface,
> > > spice-server will notify client to use the correct mouse mode,
> > > and then spice client will update mouse motion/position based on
> > > the > mouse mode.
> > >
> > > About the VMMOUSE_RELATIVE_PACKET events,I guess that designer may
> > > > want use it
> > > to distinguish relative from absolute of the process events.But >
> > > doesn't mark them as such
> > > on QEMU's vmmouse device code.In fact, the status corresponds to
> > > the > following buttons value on QEMU：
> link:
> https://imgs.wiki/imgs/2023/04/28/2c6a92c33e2537dd.png
> > >
> > > From the screenshot we can know it didn't mark the mouse mode
> > > status > on original design,
> > > only set the actual button state. So I think have to mark it here >
> > > according to the value of 's->absolute'.
> > >
> Dear zack:
>
> I guess the last mail sent to you may has been lost
> as I don't see it in lore.kernel.org.Just resending it.
Do you mean the one I replied to yesterday? My reply to it is here
https://lore.kernel.org/lkml/[email protected]/
(and surely somewhere in your inbox as well)
z
