Re: [PATCH] staging: vboxvideo: Fix reporting invalid suggested-offset-properties
Hello Hans,
12.10.2017 20:10, Hans de Goede wrote:
> The x and y hints receives from the host are unsigned 32 bit integers and
> they get set to -1 (0x) when invalid. Before this commit the
> vboxvideo driver was storing them in an u16 causing the -1 to be truncated
> to 65535 which, once reported to userspace, was breaking gnome 3.26+
> in Wayland mode.
>
> This commit stores the host values in 32 bit variables, removing the
> truncation and checks for -1, replacing it with 0 as -1 is not a valid
> suggested-offset-property value. Likewise the properties are now
> initialized to 0 instead of -1, since -1 is not a valid value.
> This fixes gnome 3.26+ in Wayland mode not working with the vboxvideo
> driver.
Thank you, looks sensible. Minor quibble: wouldn't it be nicer to
compare the unsigned values vbox_connector->vbox_crtc->x_hint and y_hint
against ~0 below, rather than -1 (see inline below)? Other than that,
looks good to me. I will send Gianfranco a patch to test against
Ubuntu's version of the driver.
Regards
Michael
> Reported-by: Gianfranco Costamagna <locutusofb...@debian.org>
> Cc: sta...@vger.kernel.org
> Cc: Michael Thayer <michael.tha...@oracle.com>
> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> ---пишет
> drivers/staging/vboxvideo/vbox_drv.h | 8
> drivers/staging/vboxvideo/vbox_irq.c | 4 ++--
> drivers/staging/vboxvideo/vbox_mode.c | 26 ++
> 3 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/vboxvideo/vbox_drv.h
> b/drivers/staging/vboxvideo/vbox_drv.h
> index 4b9302703b36..eeac4f0cb2c6 100644
> --- a/drivers/staging/vboxvideo/vbox_drv.h
> +++ b/drivers/staging/vboxvideo/vbox_drv.h
> @@ -137,8 +137,8 @@ struct vbox_connector {
> char name[32];
> struct vbox_crtc *vbox_crtc;
> struct {
> - u16 width;
> - u16 height;
> + u32 width;
> + u32 height;
> bool disconnected;
> } mode_hint;
> };
> @@ -150,8 +150,8 @@ struct vbox_crtc {
> unsigned int crtc_id;
> u32 fb_offset;
> bool cursor_enabled;
> - u16 x_hint;
> - u16 y_hint;
> + u32 x_hint;
> + u32 y_hint;
> };
>
> struct vbox_encoder {
> diff --git a/drivers/staging/vboxvideo/vbox_irq.c
> b/drivers/staging/vboxvideo/vbox_irq.c
> index 3ca8bec62ac4..74abdf02d9fd 100644
> --- a/drivers/staging/vboxvideo/vbox_irq.c
> +++ b/drivers/staging/vboxvideo/vbox_irq.c
> @@ -150,8 +150,8 @@ static void vbox_update_mode_hints(struct vbox_private
> *vbox)
>
> disconnected = !(hints->enabled);
> crtc_id = vbox_conn->vbox_crtc->crtc_id;
> - vbox_conn->mode_hint.width = hints->cx & 0x8fff;
> - vbox_conn->mode_hint.height = hints->cy & 0x8fff;
> + vbox_conn->mode_hint.width = hints->cx;
> + vbox_conn->mode_hint.height = hints->cy;
> vbox_conn->vbox_crtc->x_hint = hints->dx;
> vbox_conn->vbox_crtc->y_hint = hints->dy;
> vbox_conn->mode_hint.disconnected = disconnected;
> diff --git a/drivers/staging/vboxvideo/vbox_mode.c
> b/drivers/staging/vboxvideo/vbox_mode.c
> index 257a77830410..6f08dc966719 100644
> --- a/drivers/staging/vboxvideo/vbox_mode.c
> +++ b/drivers/staging/vboxvideo/vbox_mode.c
> @@ -553,12 +553,22 @@ static int vbox_get_modes(struct drm_connector
> *connector)
> ++num_modes;
> }
> vbox_set_edid(connector, preferred_width, preferred_height);
> - drm_object_property_set_value(
> - >base, vbox->dev->mode_config.suggested_x_property,
> - vbox_connector->vbox_crtc->x_hint);
> - drm_object_property_set_value(
> - >base, vbox->dev->mode_config.suggested_y_property,
> - vbox_connector->vbox_crtc->y_hint);
> +
> + if (vbox_connector->vbox_crtc->x_hint != -1)
Unsigned with signed comparison above.
> + drm_object_property_set_value(>base,
> + vbox->dev->mode_config.suggested_x_property,
> + vbox_connector->vbox_crtc->x_hint);
> + else
> + drm_object_property_set_value(>base,
> + vbox->dev->mode_config.suggested_x_property, 0);
> +
> + if (vbox_connector->vbox_crtc->y_hint != -1)
And here.
> + drm_object_property_set_value(>base,
> + vbox->dev->mode_config.suggested_y_property,
> + vbox_connector->vbox_crtc->y_hint);
> + else
>
Re: [PATCH v3] staging: vboxvideo: Add vboxvideo to drivers/staging
Hello Hans,
26.06.2017 18:06, Hans de Goede wrote:
> Hi,
>
> On 23-06-17 11:31, Daniel Vetter wrote:
>> On Thu, Jun 22, 2017 at 11:11:37AM +0200, Hans de Goede wrote:
[vboxvideo driver submission information to staging.]
>>
>> In the end it's up to you, but our experience in drm with -staging has
>> been that's both a pain (separate tree, which makes coordination harder
>> for drm drivers) and a ghetto (no one from drm will look at your
>> patches).
>>
>> Especially for small drivers (and this is one, and I expect if you use
>> all
>> the latest and greates helpers atomic will provide it'll shrink
>> considerably) it's just not worth the pain to polish stuff twice.
>>
>> 0toh I see the benefit of putting stuff into staging so in the end
>> it's up
>> to you, just beware pls.
>
> Thanks for the heads up Daniel, for now I would like to move forward with
> getting this in staging, but I do agree that it would be good to get it
> into drivers/gpu soon. Michael do you think you will have some time soon
> to look at moving this to the atomic helpers ?
Taking a look at that now. There is rather a lot of deprecated API use
there that I will have to clean up first. The ast driver which I was
tracking does not seem to have had much clean-up recently - qxl might
have been a better choice. I will also want to get our local code at
the level of your v4 submission before I start to avoid creating
unneeded difficulties.
By the way, see a small, as yet uncommitted fix below.
Regards
Michael
Index: src/VBox/Additions/linux/drm/vbox_fb.c
===
--- src/VBox/Additions/linux/drm/vbox_fb.c (revision 116355)
+++ src/VBox/Additions/linux/drm/vbox_fb.c (working copy)
@@ -364,6 +364,9 @@
if (fbdev->helper.fbdev) {
info = fbdev->helper.fbdev;
+#ifdef CONFIG_FB_DEFERRED_IO
+ fb_deferred_io_cleanup(info);
+#endif
unregister_framebuffer(info);
if (info->cmap.len)
fb_dealloc_cmap(>cmap);
--
Michael Thayer | VirtualBox engineer
ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt
ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstraße 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603
Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister
der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging
13.06.2017 17:41, Greg Kroah-Hartman wrote: [...] > And why is the closed vbox-devel list still on the cc: here, the bounces > from it are getting annoying. [...] Not sure why you are still getting them, but we added you to the white-list as well. I would prefer the list to stay on as the subject is relevant to it. Regards, and sorry for the annoyance Michael -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging
Hello Hans, 14.06.2017 15:30, Hans de Goede wrote: [Discussion of vboxvideo and vboxguest driver clean-up.] > As I already mentioned in previous mails on this, for the vboxguest driver > my plan is to simply do a fork and remove anything related to the > portability. It currently weighs in at 10+ lines of code which is a bit > much for what it does I believe I will be able to get a Linux only version > of it down to a small fraction of that and the result will be a much > cleaner > and better driver. > > FWIW I've already stared looking into cleaning up the vboxguest driver and > my first target is to remove all dependencies on the r0drv code. Moving off-topic, but please let me know if you set up a git repository for it as you did for vboxvideo. Do you have plans (applies to vboxvideo too) for how to merge changes to our code into the cleaned-up code? Of course, I am open to patches or suggestions as to how to simplify the code in our repository as long as they do not affect other platforms (vboxguest builds and runs for five different operating system kernels). Regards Michael > Regards, > > Hans -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging
Hello Sean, 13.06.2017 20:03, Sean Paul wrote: [Discussion of vboxvideo driver clean-up.] > First, thank you for your submission. Thank you for your feedback. [Discussion of the OS-independent code in the driver submission.] > I took a quick skim through your driver, and there doesn't seem to be much > secret sauce there that will be tough to keep up-to-date across platforms. I have two particular concerns there: first if we add new functionality (which we would do out of tree first) it will need porting over. Acceleration support is the most likely candidate. And if someone does make fixes to that part of the code in the kernel tree they will also need porting over. I agree, that concern is probably overblown, and best addressed by keeping that part of the code as close to our tree as possible while still meeting kernel standards (hence my question as to what that would be). The second concern is not relevant to DRI, but it concerns our other guest drivers (not the host one with the C++ in it Greg!) which Hans also expressed interest in putting upstream after seeing how vboxvideo fares. The OS-independent part is quite a bit larger and more volatile, though it has thankfully stablised a lot. That concern is probably also overblown, though I do wonder whether upstreaming those driver is the best solution (that would be Hans's call though). > One other concern I have is that I noticed there are a few functions > declared/defined in the osindependent code that are never used outside of it, > so > we have dead code off the hop. If the OS-independent part gets converted then they would be removed in the process. Thank you for the reminder. [...] > IMO, in order to accept the driver in drm, the osindependent code needs to be > stripped out and it needs to be converted to atomic. Whether you want to do > this out of tree, or in staging is up to you. As Dave mentioned, people often > overlook staging when making drm core changes, so you'll likely face the same > moving target issues either way. Conversion to Atomic would probably have to happen at some time or another anyway. I have put that off (out-of-tree) so far because I was tracking the AST driver as closely as possible as the simplest way of picking up fixes, and because Dave, who wrote that, knows much more about drm drivers than me. [...] Regards Michael -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging
14.06.2017 16:42, Sean Paul wrote: [Discussion of vboxvideo driver kernel integration and clean-up.] > Once the code is upstream, it's going to be difficult to motivate developers > to > keep the driver close to your downstream version. If I'm working on feature X > which touches your driver, I'm not going to figure out how your internal > version > works so that I might ease your pain. I don't mean that to be rude, just > realistic. I would not expect many changes to happen to the vboxvideo-specific parts of the driver - what is currently in osdependent - at least not without someone talking to me about it first. If changes happen to the other parts I would expect to monitor that myself and integrate them into our out-of-tree driver at regular intervals. Up until now I have been tracking the AST driver in this way - not as diligently as I should have been. I would expect this to be made easier, not harder if I switch to tracking in-kernel vboxvideo. Perhaps I will be proven wrong, but I do not expect much to be made in the way of changes that will not make sense for me to integrate, if only because keeping the difference as small as possible will make sense for me. Regards Michael [...] -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging
13.06.2017 15:59, Greg Kroah-Hartman wrote: > On Tue, Jun 13, 2017 at 03:45:14PM +0200, Michael Thayer wrote: >> 13.06.2017 14:48, Greg Kroah-Hartman wrote: >> [Discussion of vboxvideo coding style.] >>> Once your code is accepted into the main kernel tree, why would you >>> continue to work in an out-of-tree repo anyway? That's ripe for >>> disaster, what's keeping you from just working with the in-tree version? >> >> One of our use cases is customers running older operating systems, >> including legacy Linux distributions. However those customers would >> still like the most up-to-date guest drivers possible without updating >> the kernel. Updating drivers without updating the kernel is not a >> scenario of interest to upstream kernel developers, which is why we will >> continue to maintain the out-of-tree repository (which is actually the >> VirtualBox repository, where the OS-independent code is shared with >> drivers for other platforms). The end result is not unlike what Red Hat >> does when it does back-ports to its stable kernels. > > When Red Hat backports upstream drivers to older kernels, they do not do > so in a way that is a different coding style or anything like that. > They take the existing code, apply some rules and modifications to it, > and complete the backport. Some drivers can be done "automagically" > using some good transformation tools that people have developed. > > In fact, its even easier to do this if the code is upstream. Just keep > a local copy of the latest upstream version, with a "linux_backport.h" > type file to handle the api changes. Lots of people do that, and I > myself did it for many years while working on different driver > subsystems that had to ship in older kernels. > > Here's one example of this type of a file that I helped work on: > https://github.com/gregkh/greybus/blob/master/kernel_ver.h > that covered 5-6 different driver subsystems having to track all kernel > versions from 3.10 to the latest 4.9 release (at that point in time, the > code got merged upstream.) > > Feel free to copy the same idea for your code, if it's applicable, other > drivers do this for their "customers" as well. > > Note, none of this requires "os abstractions" on the upstream code at > all, nor does it require a different coding style at all, that would > just hinder development and cause massive problems when trying to > determine what changed upstream. The reason we have OS abstraction is that we share code between drivers for different platforms. There is always going to be code in a driver - mainly the actual programming of the hardware - which needs to work the same way on different platforms, but which is not likely to be shared with other drivers on the same platform. I personally think that this is a sensible thing to do, and a discussion that is worth having, in this case how to reap the benefits (for Linux too) of sharing among platforms without imposing significant costs on the Linux kernel; but I do agree that this driver is too small to be worth having a long argument about. Regards Michael > This isn't the first time we've done this, some of us have a bit of > experience in operating system development :) > > thanks, > > greg k-h > -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging
13.06.2017 14:48, Greg Kroah-Hartman wrote: [Discussion of vboxvideo coding style.] > Once your code is accepted into the main kernel tree, why would you > continue to work in an out-of-tree repo anyway? That's ripe for > disaster, what's keeping you from just working with the in-tree version? One of our use cases is customers running older operating systems, including legacy Linux distributions. However those customers would still like the most up-to-date guest drivers possible without updating the kernel. Updating drivers without updating the kernel is not a scenario of interest to upstream kernel developers, which is why we will continue to maintain the out-of-tree repository (which is actually the VirtualBox repository, where the OS-independent code is shared with drivers for other platforms). The end result is not unlike what Red Hat does when it does back-ports to its stable kernels. Regards Michael [...] -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging
12.06.2017 18:03, Greg Kroah-Hartman wrote: > On Mon, Jun 12, 2017 at 05:40:21PM +0200, Hans de Goede wrote: >> Hi, >> >> On 12-06-17 13:44, Greg Kroah-Hartman wrote: >>> On Mon, Jun 12, 2017 at 12:07:41PM +0200, Hans de Goede wrote: >>>>> The most important thing is for the driver to be atomic if it's KMS >>>>> only, and it would be good to have someone review that properly. >>>> >>>> I believe it does not use the atomic APIs atm, so that would be one >>>> of the first things to fix then. Another question is if people >>>> (you and Daniel at least) can live with the non kernel-coding >>>> style shared files under the osindependent dir ? >>> >>> Why not just spend a few days and fix up all of the kernel-style issues >>> so it can be a "real" driver? It shouldn't take all that long, >>> especially for someone with Linux kernel experience (hint, hint...) >> >> The intention of the stuff below the osindepedent dir is for it to >> be shared 1:1 between vboxvideo driver implementations for different >> operating-systems. IIRC during the AMD DAL discussion Daniel indicated >> that some OS independent code was fine (and would be exempt from coding >> style rules) as long as it had a reasonable clean interface and was not >> re-implementing anything we already have in the kernel. > > In a quick glance at the code in there, there's lots of reimplementing > happening :( > > Maybe keep the data structures around, but really, you write those once, > and then that's it, they should never change, so it shouldn't matter > what format they are in. > >> If Daniel's verdict is that this needs to be cleaned up, then sure I >> can easily do that. As mentioned I already did a lot of cleanup, >> including moving all the other files to the kernel coding-style and >> removing about 43000 lines of portability cruft / abstraction layers, >> what is left under the osindependent directory is just C-structure >> definitions and a few small plain C helper functions, which VirtualBox >> upstream would like to keep as is... > > wrappers for simple things should not be needed at all, come on, you > know that. We don't like driver-specific malloc/free and in/out > functions, or asserts, or other crap like that. This implies that this > driver is an island in itself and somehow more "important" than the 12+ > million other lines of code that it lives within. Which isn't true. > > Just clean it up, that will make it even smaller, which in the end, is > what really matters, as that will make it easier to maintain, fix, port > to new apis, and everything else. > > There's a good reason why we don't have "os abstraction" layers in > drivers in Linux, please don't ignore our history and knowledge here for > no good reason. I would appreciate getting Dave and/or Daniel's feedback here, if they have time. In particular, they might have ideas about how to further reduce the abstraction surface. In the end, the greater the difference between the code in the kernel, the harder it is for people to pull changes from our code to the kernel, and most of the changes in that part are likely to come from our side. I don't think that converting non-Linux-specific code in our tree to kernel style would go down well in our team, so if the code as is is not acceptable, the next question is, what would be? If we can come up with something which is alright for both modulo a sed script that would still make people's lives somewhat easier (though just the different brace style throws a bit of a spanner in that). Please be clear, I am not trying to dictate to anyone. The code is GPL, it is fine to include it with whatever modifications you deem appropriate. Since individual distributions are already doing that, it will still simplify our life somewhat if it is in the upstream kernel, in whatever form, rather than in multiple forks. > Remember, this code needs us, we don't need this code at all :) I assume that that was not meant that way, but that came over as slightly unfriendly to me. I am assuming here that we are looking for the best way to do something which will be useful to a lot of people. Regards Michael > good luck! > > greg k-h > -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging
Hello Greg, 12.06.2017 13:47, Greg Kroah-Hartman wrote: On Mon, Jun 12, 2017 at 01:44:09PM +0200, Greg Kroah-Hartman wrote: On Mon, Jun 12, 2017 at 12:07:41PM +0200, Hans de Goede wrote: The most important thing is for the driver to be atomic if it's KMS only, and it would be good to have someone review that properly. I believe it does not use the atomic APIs atm, so that would be one of the first things to fix then. Another question is if people (you and Daniel at least) can live with the non kernel-coding style shared files under the osindependent dir ? Why not just spend a few days and fix up all of the kernel-style issues so it can be a "real" driver? It shouldn't take all that long, especially for someone with Linux kernel experience (hint, hint...) The idea was that bits which are device-specific and which there would be no interest in sharing with other drivers in the kernel could stay in the VirtualBox coding style. Since most updates to this code are likely to come from us and it is shared with our drivers for other platforms, that would make it easier for the in-kernel driver maintainer to pull fixes from the VirtualBox tree. We tried to remove code from osindependent which would not fit that bill - notably, Hans reworked the driver to use a kernel gen_pool structure where it was previously using a VirtualBox-specific heap class. My understanding is that something similar was considered to be acceptable for the new AMD driver code. Of course, our code is much simpler than AMD's, so it would not be the end of the world if it were converted to kernel style, but I see more disadvantages than advantages to doing it. Only put stuff in staging for a good reason, and that reason can be "I don't know how to clean this stuff up", but I don't think that is the case here... And why are you cc:ing a mailing list that does not accept non-members to post? that's just annoying... Isn't dri-devel members only too? My messages seem to take a while to show up there, which would be explained by waiting for a moderator, though I don't get a warning that the message is waiting for approval. For now we have disabled that on vbox-dev too (I assume that was the list you meant). Regards Michael -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging
Hello Dave, 12.06.2017 09:27, Dave Airlie wrote: On 12 June 2017 at 16:56, Hans de Goede <hdego...@redhat.com> wrote: This commit adds the vboxvideo drm/kms driver for the virtual graphics card used in Virtual Box virtual machines to drivers/staging. Why drivers/staging? This driver is already being patched into the kernel by several distros, thus it is good to get this driver upstream soon, so that work on the driver can be easily shared. At the same time we want to take our time to get this driver properly cleaned up before submitting it as a normal driver under drivers/gpu/drm, putting this driver in staging for now allows both. Note this driver has already been significantly cleaned up, when I started working on this the files under /usr/src/vboxguest/vboxvideo as installed by Virtual Box 5.1.18 Guest Additions had a total linecount of 52681 lines. The version in this commit has 7275 lines. Of those 7275 lines 3980 lines are in an osindependent directory which contains portable code which is shared with other guest platforms such as C-structure definitions for the virtual graphics card protocol and helper functions for using these structures to communicate with the card. Since these files are shared they are in the standard Virtual Box upstream code style and not in the kernel coding style. All files outside of the osindependent directory fully adhere to the kernel coding style. Just some questions, Does the hw have acceleration support of some kind? this driver seems be a modesetting only one, which is fine, just wondering if there are plans to do more. The device does support acceleration - what you looked at before starting on Virgl. Currently it is only implemented for X11 and goes through the guest device without touching the graphics device at all, but it is also possible to pass the stream through the graphics device, which is what our Windows drivers do. However, at this point we are not sure whether to implement support for this in the drm driver, or to do something new. We haven't used staging for drm drivers for a short while for a few reasons, the speed of iterating the kms APIs esp in the atomic area means nobody remembers to build staging, then you end up with broken staging, I'd like to wait for Daniel to get back from holidays to consult on whether we should just put this in non-staging so we don't lose it down the cracks. The most important thing is for the driver to be atomic if it's KMS only, and it would be good to have someone review that properly. How big a change would atomic support be? The reason I am asking is that our out-of-kernel version of the driver builds on all kernels from 3.11 to 4.11 using ifdefs, and for obvious reasons - reducing the risk of breaking a particular kernel version - we try to minimise the differences. We would continue maintaining our version even with the driver upstream, to be able to update the driver without updating the whole kernel, which is often desirable in virtual machine use cases. But we would also want our version to be as close as possible to the most recent kernel version so that we can pull fixes as easily as possible: generally the people making those fixes will know the drm subsystem much better than we do. To be clear, I am not trying to argue here, just to get an idea for my own planning. Regards Michael Dave. -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] staging: vboxvideo: Add vboxvideo to drivers/staging
Hello, 09.06.2017 12:21, Hans de Goede wrote: Hi, On 09-06-17 12:07, Greg Kroah-Hartman wrote: On Fri, Jun 09, 2017 at 11:58:31AM +0200, Hans de Goede wrote: This commit adds the vboxvideo drm/kms driver for the virtual graphics card used in Virtual Box virtual machines to drivers/staging. Why drivers/staging? This driver is already being patched into the kernel by several distros, thus it is good to get this driver upstream soon, so that work on the driver can be easily shared. Last time I looked, the user/kernel api was a horrid C++ structure mess, and I couldn't tease apart the needed pieces to get everything to even build properly. I'm glad to see this go in, but is that userspace api going to stay solid? What happens when we find bugs (and odds are, there are lots of them) with that api? Note this submission is not the vboxguest driver, which has its own API (that is next on my todo list) this is just the drm/kms driver which only uses the standard drm/kms interfaces to userspace and nothing virtual box specific AFAIK. I think that Greg is referring to the host kernel modules which implement our hypervisor. They do indeed require an exact match between driver and user-space. It does not apply to vboxguest. Sorry if I caused confusion with my: "Up until now this has never been done because of userspace ABI stability concerns surrounding the guest drivers. I've been talking to VirtualBox upstream about mainlining the guest drivers and VirtualBox upstream has agreed to consider the userspace ABI stable and only extend it in a backwards compatible manner." paragraph in the cover letter, that mostly applies to the vboxguest driver, which still needs a lot of work before I consider it even ready for staging. I guess that means I will get to answer your questions from above when I submit the vboxguest driver. All I can say for now is that we will need to deal with any userspace ABI bugs in case by case manner, while trying to maintain backward compatibility wherever possible. Can I get some signed-off-by from teh virtual box developers here that they are ok with this work happening? Michael, can you reply with your S-o-b please ? I haven't gone through your patch yet, but on the assumption that it matches what is in our repository plus the white-space changes you sent last night you have my signed-off-by. Is that enough, or would you like me to add it to the patch? A small comment regarding the Kconfig text - I would appreciate it if it mentioned why it is recommended to build it as a module - namely, so that the driver can be upgraded without upgrading the kernel (which makes sense in a virtual machine, where you often explicitly want to be running older software). And on the other hand, someone who doesn't need that might also appreciate knowing that it doesn't apply to them. Regards and thanks Michael Regards, Hans-- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: remove immutable flag from suggested X/Y connector properties
03.01.2017 11:40, Gerd Hoffmann wrote: >>> Makes sense I think, but for merging we need: >>> - some driver to implement >> >> This is where it starts getting tricky. vboxvideo is out of tree. In >> theory I could look at getting it merged, but that needs time I am >> rather short of (I am the only person maintaining that driver and it is >> just one of my responsibilities; and there are some bits there that are >> probably too ugly to merge as is). I don't think I am really the person >> to be doing this for qxl/virtio-gpu as that required adding the support >> to qemu too. > > Not only kernel driver and qemu. Right now neither qxl nor virtio-gpu > can communicate the actual layout back to the host. So this also > requires changes to the guest/host protocol (i.e. the virtual hardware). > >>> - some userspace to take advantage of it >> >> As I wrote, mutter would be the first candidate. > > Do you have any feedback from mutter developers on the two approaches > (touchscreen-style autoconfig or x+y offsets)? The first time I wrote to them no one responded to my e-mail[1]. I could try again, asking just that question, though my plan at this time was just to submit a patch. (For Daniel, I am looking at our source code again to see if I can get it in a merge-able state with a reasonable time investment. Then there would be the "some driver to implement".) [1] https://mail.gnome.org/archives/gnome-shell-list/2016-December/msg1.html Regards Michael > > cheers, > Gerd > -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: RiesstraÃe 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
[PATCH] drm: remove immutable flag from suggested X/Y connector properties
22.12.2016 08:07, Daniel Vetter пиÑеÑ: > On Wed, Dec 21, 2016 at 03:30:04PM +0100, Michael Thayer wrote: >> 21.12.2016 10:05, Daniel Vetter wrote: >>> On Tue, Dec 20, 2016 at 11:38:52AM +0100, Michael Thayer wrote: >>>> The suggested X and Y connector properties are intended as a way for >>>> drivers >>>> for virtual machine GPUs to provide information about the layout of the >>>> host system windows (or whatever) corresponding to given guest connectors. >>>> The intention is for the guest system to lay out screens in the virtual >>>> desktop in a way which reflects the host layout. Sometimes though the >>>> guest >>>> system chooses not to follow those hints, usually due to user requests. In >>>> this case it is useful to be able to pass information back about the actual >>>> layout chosen. >>>> >>>> The immediate use case for this is host-to-guest pointer input mapping. >>>> Qemu, VirtualBox and VMWare currently handle this by providing an emulated >>>> graphics tablet device to the guest. libinput defaults, as did X.Org >>>> before >>>> it used libinput, to mapping the position information reported by the >>>> device >>>> to the smallest rectangle enclosing the screen layout. Knowing that layout >>>> lets the hypervisor send the right position information through the input >>>> device. >>>> >>>> Signed-off-by: Michael Thayer >>>> --- >>>> Follow-up to thread "Passing multi-screen layout to KMS driver". In that >>>> thread, Gerd suggested an alternative way of solving the use case, namely >>>> emulating one input device per virtual screen, touchscreen-style. My >>>> reasons >>>> for prefering this approach is that it is relatively uninvasive, and closer >>>> to the way things are done now without (in my opinion) being ugly; and that >>>> automatic touchscreen input to screen mapping is still not a solved >>>> problem. >>>> I think that both are valid though. >>>> >>>> Both approaches require changes to the hypervisor and virtual hardware, and >>>> to user-space consumers which would use the interface. I have checked the >>>> mutter source and believe that the change required to support the interface >>>> as implemented here would be minimal and intend to submit a patch if this >>>> change is accepted. I think that the virtual hardware changes are likely >>>> to >>>> be less invasive with this approach than with the other. This change will >>>> though also require small drm driver changes once the virtual hardware has >>>> been adjusted; currently to the qxl driver and to the out-of-tree vboxvideo >>>> driver. It would certainly be nice to have in virtio-gpu. >>> >>> Makes sense I think, but for merging we need: >>> - some driver to implement >> >> This is where it starts getting tricky. vboxvideo is out of tree. In >> theory I could look at getting it merged, but that needs time I am rather >> short of (I am the only person maintaining that driver and it is just one of >> my responsibilities; and there are some bits there that are probably too >> ugly to merge as is). I don't think I am really the person to be doing this >> for qxl/virtio-gpu as that required adding the support to qemu too. I think >> that they really should have it, but I would rather not be the one adding >> it. So would our out-of-tree driver be good enough? > > I don't see the point in merging core code for out-of-tree drivers. If > it's out-of-tree you can just add this locally (by adding the property). > Has ofc the risk of uapi breakage or not upstream opting for a slightly > different flavour, but that's the price for not being upstream. Evil question: I just can't see myself getting it upstream in the near future for lack of time. How much success am I likely to have asking for volunteers? (I do not see a reason why other people should have more time than I do, but you never know.) Obviously I would continue committing to it and would probably switch to keeping our out-of-tree driver in sync with the in-kernel one. For interest, the files are mainly in these places, and the other referenced include files are mainly compatibility layer things which would probably want eliminating: https://www.virtualbox.org/browser/vbox/trunk/src/VBox/Additions/linux/drm https://www.virtualbox.org/browser/vbox/trunk/src/VBox/Additions/common/VBoxVideo https://www.virtualbox.
[PATCH] drm: remove immutable flag from suggested X/Y connector properties
21.12.2016 10:05, Daniel Vetter wrote: > On Tue, Dec 20, 2016 at 11:38:52AM +0100, Michael Thayer wrote: >> The suggested X and Y connector properties are intended as a way for drivers >> for virtual machine GPUs to provide information about the layout of the >> host system windows (or whatever) corresponding to given guest connectors. >> The intention is for the guest system to lay out screens in the virtual >> desktop in a way which reflects the host layout. Sometimes though the guest >> system chooses not to follow those hints, usually due to user requests. In >> this case it is useful to be able to pass information back about the actual >> layout chosen. >> >> The immediate use case for this is host-to-guest pointer input mapping. >> Qemu, VirtualBox and VMWare currently handle this by providing an emulated >> graphics tablet device to the guest. libinput defaults, as did X.Org before >> it used libinput, to mapping the position information reported by the device >> to the smallest rectangle enclosing the screen layout. Knowing that layout >> lets the hypervisor send the right position information through the input >> device. >> >> Signed-off-by: Michael Thayer >> --- >> Follow-up to thread "Passing multi-screen layout to KMS driver". In that >> thread, Gerd suggested an alternative way of solving the use case, namely >> emulating one input device per virtual screen, touchscreen-style. My reasons >> for prefering this approach is that it is relatively uninvasive, and closer >> to the way things are done now without (in my opinion) being ugly; and that >> automatic touchscreen input to screen mapping is still not a solved problem. >> I think that both are valid though. >> >> Both approaches require changes to the hypervisor and virtual hardware, and >> to user-space consumers which would use the interface. I have checked the >> mutter source and believe that the change required to support the interface >> as implemented here would be minimal and intend to submit a patch if this >> change is accepted. I think that the virtual hardware changes are likely to >> be less invasive with this approach than with the other. This change will >> though also require small drm driver changes once the virtual hardware has >> been adjusted; currently to the qxl driver and to the out-of-tree vboxvideo >> driver. It would certainly be nice to have in virtio-gpu. > > Makes sense I think, but for merging we need: > - some driver to implement This is where it starts getting tricky. vboxvideo is out of tree. In theory I could look at getting it merged, but that needs time I am rather short of (I am the only person maintaining that driver and it is just one of my responsibilities; and there are some bits there that are probably too ugly to merge as is). I don't think I am really the person to be doing this for qxl/virtio-gpu as that required adding the support to qemu too. I think that they really should have it, but I would rather not be the one adding it. So would our out-of-tree driver be good enough? > - some userspace to take advantage of it As I wrote, mutter would be the first candidate. > - and some proper kernel-doc, we're deprecating that horrible property > table that no one human can maintain. Latest upstream has lots of > examples of what I have in mind. I can take a look at that. > Also if we make this mutable would be good to switch the entire > scaffolding over to the atomic way of doing things, i.e. put the property > value as a decoded thing into drm_connector_state and wire up all the > handling. Means you need an atomic driver as demonstration vehicle, but > I'd say you want that anyway ;-) Currently vboxvideo is keeping close to ast as I have time to follow it. Switching to atomic as long as ast is not switched likely means more new bugs introduced than I would like to have time to fix, plus maintaining two versions of vboxvideo, one for pre-atomic kernels and one for post. At the moment we support 3.11 to 4.10 with a manageable minimum of conditional compilation. So does "would be good" mean what it says, or is it meant slightly more strongly? Thanks and regards, Michael > -Daniel > >> >> Regards >> Michael >> >> Documentation/gpu/kms-properties.csv | 4 ++-- >> drivers/gpu/drm/drm_connector.c | 4 ++-- >> 2 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/Documentation/gpu/kms-properties.csv >> b/Documentation/gpu/kms-properties.csv >> index 981873a..825238e 100644 >> --- a/Documentation/gpu/kms-properties.csv >> +++ b/Documentation/gpu/kms-properties.csv >> @@ -20,8 +20
[PATCH] drm: remove immutable flag from suggested X/Y connector properties
The suggested X and Y connector properties are intended as a way for drivers
for virtual machine GPUs to provide information about the layout of the
host system windows (or whatever) corresponding to given guest connectors.
The intention is for the guest system to lay out screens in the virtual
desktop in a way which reflects the host layout. Sometimes though the guest
system chooses not to follow those hints, usually due to user requests. In
this case it is useful to be able to pass information back about the actual
layout chosen.
The immediate use case for this is host-to-guest pointer input mapping.
Qemu, VirtualBox and VMWare currently handle this by providing an emulated
graphics tablet device to the guest. libinput defaults, as did X.Org before
it used libinput, to mapping the position information reported by the device
to the smallest rectangle enclosing the screen layout. Knowing that layout
lets the hypervisor send the right position information through the input
device.
Signed-off-by: Michael Thayer
---
Follow-up to thread "Passing multi-screen layout to KMS driver". In that
thread, Gerd suggested an alternative way of solving the use case, namely
emulating one input device per virtual screen, touchscreen-style. My reasons
for prefering this approach is that it is relatively uninvasive, and closer
to the way things are done now without (in my opinion) being ugly; and that
automatic touchscreen input to screen mapping is still not a solved problem.
I think that both are valid though.
Both approaches require changes to the hypervisor and virtual hardware, and
to user-space consumers which would use the interface. I have checked the
mutter source and believe that the change required to support the interface
as implemented here would be minimal and intend to submit a patch if this
change is accepted. I think that the virtual hardware changes are likely to
be less invasive with this approach than with the other. This change will
though also require small drm driver changes once the virtual hardware has
been adjusted; currently to the qxl driver and to the out-of-tree vboxvideo
driver. It would certainly be nice to have in virtio-gpu.
Regards
Michael
Documentation/gpu/kms-properties.csv | 4 ++--
drivers/gpu/drm/drm_connector.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/gpu/kms-properties.csv
b/Documentation/gpu/kms-properties.csv
index 981873a..825238e 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -20,8 +20,8 @@ Owner Module/Drivers,Group,Property Name,Type,Property
Values,Object attached,De
,,âoverscanâ,RANGE,"Min=0, Max=100",Connector,TBD
,,âsaturationâ,RANGE,"Min=0, Max=100",Connector,TBD
,,âhueâ,RANGE,"Min=0, Max=100",Connector,TBD
-,Virtual GPU,âsuggested Xâ,RANGE,"Min=0,
Max=0x",Connector,property to suggest an X offset for a connector
-,,âsuggested Yâ,RANGE,"Min=0, Max=0x",Connector,property to
suggest an Y offset for a connector
+,Virtual GPU,âsuggested Xâ,RANGE,"Min=0,
Max=0x",Connector,"property to suggest an X offset for a connector to
help match positions of host windows and guest screens; can be set by the
driver for the host or user-space for the guest"
+,,âsuggested Yâ,RANGE,"Min=0, Max=0x",Connector,"property to
suggest an Y offset for a connector to help match positions of host windows and
guest screens; can be set by the driver for the host or user--space for the
guest"
,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9""
}",Connector,TDB
i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited
16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is
set, the hardware will be programmed with the result of the multiplication of
CTM by the limited range matrix to ensure the pixels normaly in the range
0..1.0 are remapped to the range 16/255..235/255."
,,âaudioâ,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 5a45262..ebb3cee 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -876,10 +876,10 @@ int drm_mode_create_suggested_offset_properties(struct
drm_device *dev)
return 0;
dev->mode_config.suggested_x_property =
- drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE,
"suggested X", 0, 0x);
+ drm_property_create_range(dev, 0, "suggested X", 0, 0x);
dev->mode_config.suggested_y_property =
- drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE,
"suggested Y", 0, 0x);
+ drm_property_create_range(dev, 0, "suggested Y", 0, 0x);
if (dev->mode_config.suggested_x_property == NULL ||
dev->mode_config.suggested_y_property == NULL)
--
2.9.3
Passing multi-screen layout to KMS driver
Adding Dave on CC here as the original creator of the suggested X and Y properties. 15.12.2016 15:03, Michael Thayer wrote: > 14.12.2016 11:11, Gerd Hoffmann wrote: [Summary of cut text: VirtualBox and Qemu use emulated graphics tablets to pass the host pointer through to the guest. X.Org and GNOME Shell/Wayland map the tablet range to cover the whole virtual desktop, but GNOME Shell/Wayland does not provide any way for the host to discover the layout of that desktop in order to map positions in host windows to positions on guest screens.] >>> So I would be interested to know whether anyone else has thought about >>> this problem, and possibly even about an interface to let the compositor >>> pass the information. If not, would people be open to the idea? I >>> would much rather have something generally agreed on than hack >>> something up. [...] > [...] It would be > nice though as I said if the compositor (or whoever is controlling the > display) could just provide the layout information to the driver itself. > We already have "suggested X" and "suggested Y" for the other > direction, and for now I have solved it by always providing "suggested > X" and "suggested Y" hints in the driver. My idea is to submit a patch which removes the DRM_MODE_PROP_IMMUTABLE flag from the suggested X and Y connector properties (with appropriate documentation), so that the compositor can use them to tell the driver about the layout. It would be greatly appreciated if someone could tell me if that is likely to be acceptable. Regards, Michael -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: RiesstraÃe 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
Passing multi-screen layout to KMS driver
Hello Gerd, 14.12.2016 11:11, Gerd Hoffmann wrote: >> So I would be interested to know whether anyone else has thought about >> this problem, and possibly even about an interface to let the compositor >> pass the information. If not, would people be open to the idea? I >> would much rather have something generally agreed on than hack something up. > > I think the best way to tackle this is to have multiple tablets, one per > display device (touchscreen-style setup). > > qemu can do that, with input routing (must configure on the host which > display belongs to which tablet). Here is some info on that: > > http://git.qemu-project.org/?p=qemu.git;a=blob;f=docs/multiseat.txt;hb=HEAD > > The setup on the guest side is completely manual. You have to use > "xinput --map-to-output" to tell Xorg which tablet belongs to which > display. Maybe it is also possible to stick that into xorg.conf. > Should be improved, and it surely makes sense that qemu and virtualbox > use the same approach here. > > Not sure if and how this works automatically with physical touchscreens. > Any clues are welcome. Thanks for the answer. That was the direction I was initially expecting to go too. In theory libinput lets you map input devices to heads[1] using a udev property[2], though I have yet to test whether anyone supports that yet (couldn't find it in the Mutter/GNOME Shell source, but I'm not familiar with it). As the API description says, the default without that property is still to map the input device to all screens like X.Org on Linux does. [1] https://cgit.freedesktop.org/wayland/libinput/tree/src/udev-seat.c?id=1.5.3#n97 [2] https://wayland.freedesktop.org/libinput/doc/0.99.1/group__device.html#gaf48626f6190e9c9bc14abb704e66cc22 > Another option is to use a guest agent, spice does that since years to > handle multihead. The guest agent queries the display layout using > xrandr, gets x + y + displayid from the spice client and generates > pointer events from that. But I expect that scheme breaks with wayland > because wayland is by design alot more restrictive, so spice-agent > probably isn't allowed to send pointer events. So not really an option > these days ... We actually do something similar in Windows guests: older versions provided the layout information to the driver directly, but at least Windows 10 does not, so we query it with a user-space agent which passes it to the driver. (We send our pointer events from a driver, in all supported guest types, which works fine with Wayland too.) It would be nice though as I said if the compositor (or whoever is controlling the display) could just provide the layout information to the driver itself. We already have "suggested X" and "suggested Y" for the other direction, and for now I have solved it by always providing "suggested X" and "suggested Y" hints in the driver. That which works well enough in a first approximation - if the user changes the layout inside the virtual machine the mapping breaks, and as soon as they change it outside it mends again. So my idea was to try to have people agree on on interface for that. Regards Michael > cheers, > Gerd -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: RiesstraÃe 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
Passing multi-screen layout to KMS driver
Hello, I'm afraid I'm not subscribed to the list, so I would greatly appreciate a CC on replies (if any). I have been fiddling with our (not-upstreamed) VirtualBox guest drm driver to make it work better with the Wayland version of GNOME Shell in Fedora 25. One problem that I have run into (from my testing it seems to affect Qemu too) is that we use an emulated graphics tablet device to make the guest pointer position match the host one. GNOME Shell nicely maps the range of the device to the full virtual desktop, as does X.Org. Unlike X.Org though, GNOME Shell/Wayland does not create a single framebuffer in video RAM for the whole desktop, where we could deduce the relative layout of the screens by looking at the offset of their crtcs in that framebuffer. So I would be interested to know whether anyone else has thought about this problem, and possibly even about an interface to let the compositor pass the information. If not, would people be open to the idea? I would much rather have something generally agreed on than hack something up. Thanks. Regards Michael -- Michael Thayer | VirtualBox engineer ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt ORACLE Deutschland B.V. & Co. KG Hauptverwaltung: RiesstraÃe 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
