Re: [PATCH] mi: sprite: SaveUnderCursor2

[Hans de Goede]

Hi,

On 3/9/21 5:34 PM, Madhurkiran Harikrishnan wrote:
> From: Hyun Kwon 
> 
> There is cursor bleeding for the devices which try to acclerate the
> Desktop environment using low powered ARM Mali GPUs. The area under
> the cusrsor is not updated for every swap call within the armsoc,
> resulting in the block artifact.
> 
> Expose miDCSaveUnderCursor2 API, so that other graphics layer like
> armsoc can inoke and save the area under the cursor before actually
> perfroming a swap between the backbuffer and the frontbuffer.

Thank you for the updated version with this nice commit message.

So it seems that this is another approach to fixing the same issue
which I fixed in the modesetting driver with this commit:

https://cgit.freedesktop.org/xorg/xserver/commit/?id=0aaac8d783e78c040a70a55ba8d67809abd7e625

Interesting...

If this works reliable for you, so that you don't need to disable
page-flipping then this seems like a good idea to me.

Regards,

Hans



> 
> Signed-off-by: Hyun Kwon 
> Signed-off-by: Madhurkiran Harikrishnan 
> ---
>  mi/mipointer.h |  2 ++
>  mi/misprite.c  | 17 +
>  2 files changed, 19 insertions(+)
> 
> diff --git a/mi/mipointer.h b/mi/mipointer.h
> index 7ce6409..107b24f 100644
> --- a/mi/mipointer.h
> +++ b/mi/mipointer.h
> @@ -127,4 +127,6 @@ extern _X_EXPORT DevPrivateKeyRec miPointerScreenKeyRec;
>  
>  #define miPointerScreenKey ()
>  
> +extern _X_EXPORT void miDCSaveUnderCursor2(ScreenPtr pScreen);
> +
>  #endif  /* MIPOINTER_H */
> diff --git a/mi/misprite.c b/mi/misprite.c
> index add2c55..46ce9f0 100644
> --- a/mi/misprite.c
> +++ b/mi/misprite.c
> @@ -955,3 +955,20 @@ miSpriteComputeSaved(DeviceIntPtr pDev, ScreenPtr 
> pScreen)
>  pCursorInfo->saved.x2 = pCursorInfo->saved.x1 + w + wpad * 2;
>  pCursorInfo->saved.y2 = pCursorInfo->saved.y1 + h + hpad * 2;
>  }
> +
> +void
> +miDCSaveUnderCursor2(ScreenPtr pScreen)
> +{
> + DeviceIntPtr pDev;
> + miCursorInfoPtr pCursorInfo;
> +
> + for (pDev = inputInfo.devices; pDev; pDev = pDev->next) {
> + if (DevHasCursor(pDev)) {
> + pCursorInfo = GetSprite(pDev);
> + if (pCursorInfo && pCursorInfo->isUp
> + && pCursorInfo->pScreen == pScreen) {
> + miSpriteSaveUnderCursor(pDev, pScreen);
> + }
> + }
> + }
> +}
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] mi: sprite: SaveUnderCursor2

[Hans de Goede]

Hi,

On 3/8/21 11:09 PM, Madhurkiran Harikrishnan wrote:
> From: Hyun Kwon 

There should be a block of text here explaining what this new function
does, why it is needed, etc.

Please submit a new version with an actual commit message describing
the what and why of this patch.

Regards,

Hans



> 
> Signed-off-by: Hyun Kwon 
> Signed-off-by: Madhurkiran Harikrishnan 
> ---
>  mi/mipointer.h |  2 ++
>  mi/misprite.c  | 17 +
>  2 files changed, 19 insertions(+)
> 
> diff --git a/mi/mipointer.h b/mi/mipointer.h
> index 7ce6409..107b24f 100644
> --- a/mi/mipointer.h
> +++ b/mi/mipointer.h
> @@ -127,4 +127,6 @@ extern _X_EXPORT DevPrivateKeyRec miPointerScreenKeyRec;
>  
>  #define miPointerScreenKey ()
>  
> +extern _X_EXPORT void miDCSaveUnderCursor2(ScreenPtr pScreen);
> +
>  #endif  /* MIPOINTER_H */
> diff --git a/mi/misprite.c b/mi/misprite.c
> index add2c55..46ce9f0 100644
> --- a/mi/misprite.c
> +++ b/mi/misprite.c
> @@ -955,3 +955,20 @@ miSpriteComputeSaved(DeviceIntPtr pDev, ScreenPtr 
> pScreen)
>  pCursorInfo->saved.x2 = pCursorInfo->saved.x1 + w + wpad * 2;
>  pCursorInfo->saved.y2 = pCursorInfo->saved.y1 + h + hpad * 2;
>  }
> +
> +void
> +miDCSaveUnderCursor2(ScreenPtr pScreen)
> +{
> + DeviceIntPtr pDev;
> + miCursorInfoPtr pCursorInfo;
> +
> + for (pDev = inputInfo.devices; pDev; pDev = pDev->next) {
> + if (DevHasCursor(pDev)) {
> + pCursorInfo = GetSprite(pDev);
> + if (pCursorInfo && pCursorInfo->isUp
> + && pCursorInfo->pScreen == pScreen) {
> + miSpriteSaveUnderCursor(pDev, pScreen);
> + }
> + }
> + }
> +}
> 

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: USB GPU bugs

[Hans de Goede]

Hi,

On 11-10-2019 12:13, Pekka Paalanen wrote:

On Thu, 10 Oct 2019 09:40:30 +0200
Hans de Goede  wrote:


Hi,

On 09-10-2019 12:23, Pekka Paalanen wrote:

On Wed, 9 Oct 2019 09:59:29 +0200
Hans de Goede  wrote:





I think I might also be seeing some variant of your "not enough hotplug
events" bug. With the gm12u320 projector when hotplugged it is listed
as "unknown" in gnome's display-settings until I change the settings
once and then it becomes "Acer" as it should be. I believe that this
issue is actually also present in the 1.20 branch.


In my case, I see an "add" and three HOTPLUG events via udev when I
hotplug the DisplayLink dock and it creates the DRM device node on the
spot. Mutter receives just one RandR event though, which it deems is
not a hotplug, and looking at the config timestamps I think the event
indeed is reflecting too old state. 'xrandr' will show everything about
the new connector just fine though, IIRC.


I just realized I should probably respond to this bit. You see that
after "xrandr" everything looks ok, but the "xrandr" command causes
a round trip to the kernel and even causes the kernel to re-poll all
connectors. Have you tried "xrandr --current" ? That will return the
current state stored inside the xserver without re-querying the kernel.

Not sure of this helps, but I thought I should mention it.


Hi Hans,

that's a good hint. 'xrandr --current' reports the connector as
connected and with a good looking mode list, when Mutter does not
autoenable it.

Does 'xrandr' call into the kernel with drmModeGetConnectorCurrent(),
or does it really return only what Xorg had internally cached? If it
calls into kernel, then I'm not surprised it returns the good info,
since I believe that Xorg is somehow ignoring a udev hotplug event, or
not realizing that something did still change since the last time it
sent out RandR events.


AFAIK xrandr without --current calls into the kernel. With --current it
just returns only what Xorg had internally cached.

Regards,

Hans

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: USB GPU bugs

[Hans de Goede]

Hi,

On 09-10-2019 12:23, Pekka Paalanen wrote:

On Wed, 9 Oct 2019 09:59:29 +0200
Hans de Goede  wrote:





I think I might also be seeing some variant of your "not enough hotplug
events" bug. With the gm12u320 projector when hotplugged it is listed
as "unknown" in gnome's display-settings until I change the settings
once and then it becomes "Acer" as it should be. I believe that this
issue is actually also present in the 1.20 branch.


In my case, I see an "add" and three HOTPLUG events via udev when I
hotplug the DisplayLink dock and it creates the DRM device node on the
spot. Mutter receives just one RandR event though, which it deems is
not a hotplug, and looking at the config timestamps I think the event
indeed is reflecting too old state. 'xrandr' will show everything about
the new connector just fine though, IIRC.


I just realized I should probably respond to this bit. You see that
after "xrandr" everything looks ok, but the "xrandr" command causes
a round trip to the kernel and even causes the kernel to re-poll all
connectors. Have you tried "xrandr --current" ? That will return the
current state stored inside the xserver without re-querying the kernel.

Not sure of this helps, but I thought I should mention it.

Regards,

Hans

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: USB GPU bugs

[Hans de Goede]

Hi,

On 09-10-2019 12:23, Pekka Paalanen wrote:

On Wed, 9 Oct 2019 09:59:29 +0200
Hans de Goede  wrote:





Let's keep each other updated, so we don't do duplicate effort. :-)


Ack I will drop you a mail when I find / make time to look into this.

Regards,

Hans

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: USB GPU bugs

[Hans de Goede]

Hi Pekka,

On 09-10-2019 09:47, Pekka Paalanen wrote:

On Tue, 8 Oct 2019 22:19:45 +0200
Hans de Goede  wrote:


My main reason for suggesting either one is that I personally am aware
of at least 2 issues (both related to secondary USB GPUs handling) which
are only present in master and not in the 1.20 branch and which I really
would like to see fixed before a new release. I have taking a look at
these on my to do list, but not at the top of it (yet).


Hi Hans,

it so happens that I am, too, looking at some secondary DRM device
hotplug issues. I think I've seen three different things: not enough
RandR events to make Mutter take a hotplugged device & output into use,
Xorg crash on DRM device hotplug, and failure to start with Xorg USB
DRM device plugged in.

Do you have any public records of the issues you have on your plate?


No I've not filed issues yet, since I was planning on debugging them
myself, but if are both seeing these issues then having gitlab
issues to track them might be a good idea.

I am using a gm12u320 based mini projector (Acer C120) for most of
my tests, but I believe that this will reproduce with an udl (USB 2 version)
device too. If necessary I can try to reproduce with an udl device too.

Thinking more about it I'm seeing 3 things:

1. USB GPU is not seen (not shown in xrandr --list-providers, not usable)
   when present before Xorg is started.

2. Hot plugging the USB GPU sometimes causes things to crash shortly
   afterwards.

3. Hot unplugging the USB GPU always causes a crash shortly
   afterwards.

My test environment is xserver master + mutter/gnome-shell 3.34 on
top of Xorg. Last time I tried downgrading the xserver to 1.20 made all
3 issues go away (*)

I think I might also be seeing some variant of your "not enough hotplug
events" bug. With the gm12u320 projector when hotplugged it is listed
as "unknown" in gnome's display-settings until I change the settings
once and then it becomes "Acer" as it should be. I believe that this
issue is actually also present in the 1.20 branch.

Regards,

Hans


*) Which is why I've not given these bugs a high priority so far

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: xserver release process

[Hans de Goede]

Hi,

On 08-10-2019 18:28, Adam Jackson wrote:




In short, releases need to happen, and we have CI, so let's just pop a
release out on scheduled dates assuming CI passes.


Given that the Xorg xserver has a lot of hw interaction, we are
never going to catch everything with CI, so this seems a bit
over-simplified.

I think it would be good to have 2 things:

1. A way to track potential blocker bugs. Note I'm not advocating some
process heavy approach here. The blocker bugs can just be gitlab issues
with a special tag I guess. The idea is that the release-coordinator
at least can get a list of known issues and then decide if those are bad
enough to delay a release or not.

2. Send out a notice that a new release will happen in say 4 weeks
from date of sending with a request for testing + getting any pending
*fixes* upstream asao, maybe together with some "beta" or "rc" tarbals,
but that is optional.

My main reason for suggesting either one is that I personally am aware
of at least 2 issues (both related to secondary USB GPUs handling) which
are only present in master and not in the 1.20 branch and which I really
would like to see fixed before a new release. I have taking a look at
these on my to do list, but not at the top of it (yet).

Having 1. would help in tracking such known issues, I doubt I'm the only
one who has a couple of "I need to look into this and fix it" items on
their TODO, so being able to track these would be good.

Having 2. would help me bump up these TODOs in priority to try and get
them fixed before the release :)

Regards,

Hans

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Xorg's software-cursor support + mutter results in a mess

[Hans de Goede]

Hi All,

Note I've also filed an issue for tracking this:
https://gitlab.freedesktop.org/xorg/xserver/issues/828
but this seems like something to discuss on the list.

When Xorg fallsback to software cursor support, for example because
of using a DisplayLink USB2 device (udl kernel driver) or another
"dumb" kms framebuffer device and then running mutter as compositor on
top of Xorg moving the cursor around leads to 2 type of artifacts:

1) There is a pointer trail of the last couple of cursor positions
2) Sometimes the background for one of the older cursor echo's is a
   rectangle (aprox 32x32 pixels) from a Window which is no longer
   shown.

First of all let me sat that although this can be reproduced with
mutter, I do not believe that this is a mutter bug.

Looking at the visual artifacts, so observing the system as a
blackbox, I believe that the problem is that the software-cursor
code in Xorg and the code responsible for presenting framebuffers
are not aware of each other. This combined with mutter now a days
only re-rendering frames when something has changed leads to the
following:

1) Mutter uses multiple framebuffers, at least 3 of them.
2) When ever the cursor (position) changes the software-cursor code
draws the cursor over the current framebuffer.

If one then slowly moves the cursor over an otherwise unchanging
desktop, then each frame its get drawn in a different location
resulting in the 3 framebuffers having the cursor in 3 different
places. Since nothing is changing mutter keeps recylcing these
framebuffers without re-rendering them. This results in the cursor
jumping from location a, to b, to c and back to a again as
framebuffer a. b and c are presented by mutter.

Another problem happens with the code to restore the image which was
below the cursor under its previous position.

Lets say we start with frame "a" with an ok / cancel dialog box on it
with the mouse cursor over its ok button. Now the user clicks the button
and after release moves the cursor a bit. The release of the button
results in the dialog closing and the rendering of frame "b", without the
dialog, when then the cursor moves, Xorg restores the image which was below
the cursor (from frame "a" with the dialog box) on the place where part
of the ok button used to be, putting a rectangle with part of the ok
button over frame-b (which no longer has the dialog) and then also rendering
the cursor over frame-b in its new posiiton.

This results in a rectangle with the part of the ok button below the cursor
flickering in and out of the display as mutter recycles the last few frames
since nothing is changing on the desktop.

I don't know which API mutter (3.32 or newer) is using to display its
frames. but it seems a reasonable assumption to me that a frame handed over
to Xorg for displaying is treated as read-only by Xorg and can be re-used
later without needing to re-render it.

Note that even if mutter was rerendering every frame it presents, we would
still get artifacts from the software-cursor code assuming that there is
only 1 framebuffer (or so it seems).

One way to fix this would be to tie the code presenting the frames
(maybe at the ddx driver level?) and the software-cursor code together
so that on a frame-flip we can have the cursor-code undo its rendering
of the cursor on the frame before "giving it back" to the client and
likewise calling the software code to render the cursor over the
framebuffer before handing the framebuffer to the kms-driver / hardware
for scanout.

Another option which I'm seriously considering is to fake hw-cursor support
at the kms driver level.

Regards,

Hans

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: xorg-server: logind support requires --enable-udev, however works without udev running, is udev really needed?

[Hans de Goede]

Hi,

On 3/20/19 4:26 PM, Piotr Karbowski wrote:

Hi,

On 20/03/2019 12.32, Hans de Goede wrote:


I'm surprised that this works at all, but regardless using the logind
integration without the udev integration is definitely not something
which we want to support, sorry.


What do you mean by that?


What I mean by that is that the Xserver probe code is already suffering from
a combinatorial explosion 
(https://en.wikipedia.org/wiki/Combinatorial_explosion)
problem wrt all the different options, hardware configs and paths
through the code.

So I do not want to add yet another variable.

You are free to use the logind integration without udev support
and with a small *local* downstream patch you may even be able
to completely build without udev support while using the logind
integration.

But this is not something which I believe we should support in
the upstream xserver, so if it breaks you get to keep both pieces.

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: xorg-server: logind support requires --enable-udev, however works without udev running, is udev really needed?

[Hans de Goede]

Hi,

On 19-03-19 22:01, Piotr Karbowski wrote:

Hi,

Today I've been integrating a elogind, a drop-in replacement for
systemd-logind and I came across a hard dependency of
--enable-systemd-logind on --enable-config-udev.

configure: error: systemd-logind is only supported in combination with
udev configuration.

However I was able to get Xorg working without suid with elogind while
running mdev from busybox, I've been keeping user in `video` and `input`
groups meaning that Xorg running as my user does have access to input
devices.

My system does not run udev at all, however I kept it around as a stub
to make xorg-server build with --enable-config-udev.

Can you please confirm that udev is really needed, or, if not, would it
be possible to make --enable-systemd-logind buildable with
--disable-config-udev?


I'm surprised that this works at all, but regardless using the logind
integration without the udev integration is definitely not something
which we want to support, sorry.

Regards,

Hans

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] XF86keysym: Add XF86XK_RotationLockToggle

[Hans de Goede]

Hi,

On 22-01-19 09:22, Walter Harms wrote:




Hans de Goede  hat am 21. Januar 2019 um 20:23
geschrieben:


Add XF86XK_RotationLockToggle keysym, to be used as mapping for evdev's
KEY_ROTATE_LOCK_TOGGLE.

I've a Point of View P1006W-232 Windows tablet which actually has a
rotate-lock toggle-button. The latest kernel correctly generates
KEY_ROTATE_LOCK_TOGGLE events for this. So now I'm hooking up support for
it through all the higher layers.

Signed-off-by: Hans de Goede 
---
  include/X11/XF86keysym.h | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/include/X11/XF86keysym.h b/include/X11/XF86keysym.h
index 9ad8948..dd287e2 100644
--- a/include/X11/XF86keysym.h
+++ b/include/X11/XF86keysym.h
@@ -205,6 +205,8 @@
  
  #define XF86XK_AudioPreset	0x1008FFB6   /* Select equalizer preset, e.g.

theatre-mode */
  
+#define XF86XK_RotationLockToggle 0x1008FFB7 /* Toggle screen rotation lock

on/off */
+
  /* Keys for special action keys (hot keys) */
  /* Virtual terminals on some operating systems */
  #define XF86XK_Switch_VT_10x1008FE01
--
2.20.1



This is ok with me but i  have a general question:
Is there a policy what to add ? ( i coud not find one).
E.g. i have seems a keyboard with a key to change the backlight
of the keyboard (do ot ask, not mine) Do we simply add everything
found in the wild ? or are there limitations ?


In generally yes we add anything found in the wild (and not just in specs).

The limitation is applying common sense, e.g. the toggle screen rotation lock
button already has an evdev button code, KEY_ROTATE_LOCK_TOGGLE and on
devices with an accelerometer GNOME-shell has a toggle in its system
menu (the top right menu with wifi options, etc.) to lock / unlock
auto-rotating the screen. So my intention is to hook the button on the
Point of View P1006W-232 Windows tablet up all the way to the gnome-shell
level, so that it actually toggles that setting.

IOW I would like to see adding new keysyms limited to adding keysyms which
we plan to actually use. Either for a specific use-case, or as a button
which will be useful as a generic(ish) button for a user to bind an action
to (e.g. launch a new terminal, launch  settings pane of the
control panel, ...).

Note this is just my 2 cents.

As for your specific example, the kernel already has:

#define KEY_KBDILLUMTOGGLE  228
#define KEY_KBDILLUMDOWN229
#define KEY_KBDILLUMUP  230

And xkeyboard-config/rules/inet has:

key{  [ XF86KbdLightOnOff ]   }; // KEY_KBDILLUMTOGGLE
key{  [ XF86KbdBrightnessDown ]   }; // KEY_KBDILLUMDOWN
key{  [ XF86KbdBrightnessUp   ]   }; // KEY_KBDILLUMUP

(note the X "scancodes" are eight higher then the kernel ones)

And GNOME actually responds to these and does the right thing,
assuming there is a *kbd_backlight interface under /sys/class/leds for it
to control :)

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] XF86keysym: Add XF86XK_RotationLockToggle

[Hans de Goede]

Hi,

On 22-01-19 03:47, Jian-Hong Pan wrote:

Peter Hutterer  於 2019年1月22日 週二 上午9:03寫道：


On Mon, Jan 21, 2019 at 08:23:08PM +0100, Hans de Goede wrote:

Add XF86XK_RotationLockToggle keysym, to be used as mapping for evdev's
KEY_ROTATE_LOCK_TOGGLE.

I've a Point of View P1006W-232 Windows tablet which actually has a
rotate-lock toggle-button. The latest kernel correctly generates
KEY_ROTATE_LOCK_TOGGLE events for this. So now I'm hooking up support for
it through all the higher layers.

Signed-off-by: Hans de Goede 


Reviewed-by: Peter Hutterer 



Reviewed-by: Jian-Hong Pan 


Peter, Jian-Hong,

Thank you for the reviews.

I've pushed this patch now.

Regards,

Hans




---
  include/X11/XF86keysym.h | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/include/X11/XF86keysym.h b/include/X11/XF86keysym.h
index 9ad8948..dd287e2 100644
--- a/include/X11/XF86keysym.h
+++ b/include/X11/XF86keysym.h
@@ -205,6 +205,8 @@

  #define XF86XK_AudioPreset   0x1008FFB6   /* Select equalizer preset, e.g. 
theatre-mode */

+#define XF86XK_RotationLockToggle 0x1008FFB7 /* Toggle screen rotation lock 
on/off */
+
  /* Keys for special action keys (hot keys) */
  /* Virtual terminals on some operating systems */
  #define XF86XK_Switch_VT_1   0x1008FE01
--
2.20.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] XF86keysym: Add XF86XK_RotationLockToggle

[Hans de Goede]

Add XF86XK_RotationLockToggle keysym, to be used as mapping for evdev's
KEY_ROTATE_LOCK_TOGGLE.

I've a Point of View P1006W-232 Windows tablet which actually has a
rotate-lock toggle-button. The latest kernel correctly generates
KEY_ROTATE_LOCK_TOGGLE events for this. So now I'm hooking up support for
it through all the higher layers.

Signed-off-by: Hans de Goede 
---
 include/X11/XF86keysym.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/X11/XF86keysym.h b/include/X11/XF86keysym.h
index 9ad8948..dd287e2 100644
--- a/include/X11/XF86keysym.h
+++ b/include/X11/XF86keysym.h
@@ -205,6 +205,8 @@
 
 #define XF86XK_AudioPreset 0x1008FFB6   /* Select equalizer preset, e.g. 
theatre-mode */
 
+#define XF86XK_RotationLockToggle 0x1008FFB7 /* Toggle screen rotation lock 
on/off */
+
 /* Keys for special action keys (hot keys) */
 /* Virtual terminals on some operating systems */
 #define XF86XK_Switch_VT_1 0x1008FE01
-- 
2.20.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xorgproto] XF86keysym: Add XF86XK_MonBrightnessCycle

[Hans de Goede]

Hi,

Thank you for your patch.

On 04-01-19 04:36, Jian-Hong Pan wrote:

Jian-Hong Pan  於 2018年11月28日 週三 下午5:07寫道：


Add XF86XK_MonBrightnessCycle keysym, to be used as mapping for evdev's
KEY_BRIGHTNESS_CYCLE keycode which is generated from ACPI video module's
ACPI_VIDEO_NOTIFY_CYCLE_BRIGHTNESS on some Acer AIO desktop buttons.
The button changes the screen's brightness on Windows.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=108861
Signed-off-by: Jian-Hong Pan 
---
  include/X11/XF86keysym.h | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/X11/XF86keysym.h b/include/X11/XF86keysym.h
index 771fbb0..9ad8948 100644
--- a/include/X11/XF86keysym.h
+++ b/include/X11/XF86keysym.h
@@ -23,11 +23,12 @@
  #define XF86XK_ModeLock0x1008FF01  /* Mode Switch Lock */

  /* Backlight controls. */
-#define XF86XK_MonBrightnessUp   0x1008FF02  /* Monitor/panel brightness */
-#define XF86XK_MonBrightnessDown 0x1008FF03  /* Monitor/panel brightness */
-#define XF86XK_KbdLightOnOff 0x1008FF04  /* Keyboards may be lit */
-#define XF86XK_KbdBrightnessUp   0x1008FF05  /* Keyboards may be lit */
-#define XF86XK_KbdBrightnessDown 0x1008FF06  /* Keyboards may be lit */
+#define XF86XK_MonBrightnessUp0x1008FF02  /* Monitor/panel brightness */
+#define XF86XK_MonBrightnessDown  0x1008FF03  /* Monitor/panel brightness */
+#define XF86XK_KbdLightOnOff  0x1008FF04  /* Keyboards may be lit */
+#define XF86XK_KbdBrightnessUp0x1008FF05  /* Keyboards may be lit */
+#define XF86XK_KbdBrightnessDown  0x1008FF06  /* Keyboards may be lit */
+#define XF86XK_MonBrightnessCycle 0x1008FF07  /* Monitor/panel brightness */

  /*
   * Keys found on some "Internet" keyboards.
--
2.11.0



Gentle ping.  Any additional information for this patch is required?


No extra info required. Just waiting for someone to get around to it.

I needed to do some XF86keysym.h myself anyways, so I've taken a look and this
looks good to me:

Reviewed-by: Hans de Goede 

I've commit rights, so I've added and pushed this right away :)

I've one favor to add, I'm about to submit an XF86keysym.h patch myself,
can you review that please (and reply with a Reviewed-by if the patch
looks good to you) ?

I will Cc you on the patch.

Thanks & Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xkeyboard-config 0/2] 2 evdev/inet mapping fixes

[Hans de Goede]

Hi All,

I'm not sure what the prefered method of patch-review for
xkeyboard-config is. I've submitted a merge-req for this here:

https://gitlab.freedesktop.org/jwrdegoede/xkeyboard-config/merge_requests/1

I'm also sending these out by email in case no-one is looking at
the merge-req.

If I can get a Reviewed-by, either here or in the merge-req I can
push these myself.

Regards,

Hans

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xkeyboard-config 1/2] Add evdev mappings for KEY_SOUND

[Hans de Goede]

KEY_SOUND is used on actual devices and recent xproto versions
defines the AudioPreset keysym for this key.

Signed-off-by: Hans de Goede 
---
 symbols/inet | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/symbols/inet b/symbols/inet
index bc4aaa3..5a8d180 100644
--- a/symbols/inet
+++ b/symbols/inet
@@ -184,7 +184,7 @@ xkb_symbols "evdev" {
 key{  [ Print ]   };
 //  key{  [ ]   }; // KEY_HP
 key{  [ XF86WebCam]   };
-//  key{  [ ]   }; // KEY_SOUND
+key{  [ XF86AudioPreset   ]   };
 //  key{  [ ]   }; // KEY_QUESTION
 key{  [ XF86Mail  ]   };
 key{  [ XF86Messenger ]   }; // KEY_CHAT
-- 
2.20.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xkeyboard-config 2/2] evdev/int: Fix KEY_KEYBOARD mapping

[Hans de Goede]

KEY_KEYBOARD has an evdev-keycode value of 374 not 366. It seems that
with the original addition of the mapping the evdev-keycode value was
mistaken for the X scancode which is 8 higher then the evdev-keycode
since the X scancodes start at 8. What seems to have happened is that
the evdev-keycode was used instead of the X scancode and then 8 was
subtracted for the wrong "// #define KEY_KEYBOARD  366" comment.

This commit corrects the comment to say 374 and corrects the X
scancode being mapped to 382.

Cc: Christian Kellner 
Signed-off-by: Hans de Goede 
---
 keycodes/evdev | 2 +-
 symbols/inet   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/keycodes/evdev b/keycodes/evdev
index 90b8278..dd2b865 100644
--- a/keycodes/evdev
+++ b/keycodes/evdev
@@ -285,7 +285,7 @@ default xkb_keycodes "evdev" {
 = 255;   // #define KEY_RFKILL  247
 
 = 372;   // #define KEY_FAVORITES   364
-= 374;   // #define KEY_KEYBOARD366
+= 382;   // #define KEY_KEYBOARD374
 
// Fake keycodes for virtual keys
 =   92;
diff --git a/symbols/inet b/symbols/inet
index 5a8d180..7ff7c24 100644
--- a/symbols/inet
+++ b/symbols/inet
@@ -219,7 +219,7 @@ xkb_symbols "evdev" {
 key{  [ XF86RFKill]   }; // KEY_RFKILL
 
 key   {   [ XF86Favorites  ]   };
-key   {   [ XF86Keyboard   ]   };
+key   {   [ XF86Keyboard   ]   };
 
 key{  [ XF86Tools ]   };
 key{  [ XF86Launch5   ]   };
-- 
2.20.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Smooth gdm -> GNOME3 session transition broken with 1.20 modesetting driver

[Hans de Goede]

Hi,

On 05-09-18 20:35, Daniel Stone wrote:

Hi Hans,

On Wed, 5 Sep 2018 at 19:23, Hans de Goede  wrote:

Under Fedora 29 (xserver-1.20) the transition from GDM to
the GNOME3 session is no longer smooth, it seems that the
screen is cleared to black when the Xserver starts instead
of inheriting the framebuffer contents from GDM as before.

Changing the DDX driver from modesetting to intel fixes this,
I think this may be caused by the new atomic support in the
modesetting driver.


It's caused by support for modifiers: this allows Mesa to use a
multi-planar framebuffer (auxiliary compression plane), which the new
X server can't then read back from because drmModeGetFB only supports
a single plane.


I do not think that that is the problem in my case. I'm seeing this
when transitioning from a GDM greeter session which is GNOME shell
as Wayland compositor to a GNOME3 on Xorg user session.

And replacing the DDX driver in the *user* session with the intel
ddx fixes this. So I think this is another issue.

Regards,

Hans


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Smooth gdm -> GNOME3 session transition broken with 1.20 modesetting driver

[Hans de Goede]

Hi All,

Under Fedora 29 (xserver-1.20) the transition from GDM to
the GNOME3 session is no longer smooth, it seems that the
screen is cleared to black when the Xserver starts instead
of inheriting the framebuffer contents from GDM as before.

Changing the DDX driver from modesetting to intel fixes this,
I think this may be caused by the new atomic support in the
modesetting driver.

Note if you try to reproduce this on Fedora 29 recent
gdm versions also have some issues (*) which cause the
transition to be non smooth it is best to downgrade
gdm to 3.28.x (from koji) and then try with the intel
ddx vs the modesetting ddx to clearly see the difference.

Regards,

Hans



*) These are mostly fixed, but not entirely yet, see:
https://gitlab.gnome.org/GNOME/gdm/merge_requests/45
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Xorg-1.20.1 crashes when using glamor on top of llvmpipe

[Hans de Goede]

Hi,

On 30-08-18 23:44, Ray Strode wrote:

hi,

what version of mesa?

might be

https://cgit.freedesktop.org/mesa/mesa/commit/?id=9baff597ce021f7691187b0d1d1bbc16d07b13e1


Ah yes, we were still at 18.2.0-rc3, I'm preparing an update to 18.2.0-rc5 now, 
thanks
for pointing me to this.

Regards,

Hans




Ray

On Thu, Aug 30, 2018, 5:00 PM Hans de Goede mailto:hdego...@redhat.com>> wrote:

Hi,

On 30-08-18 22:48, Hans de Goede wrote:
 > HI all,
 >
 > I've been debugging some strange crashes with Xorg-1.20.1 inside
 > a virtualbox guest and I can use some help with this.
 >
 > At first Xorg completely failed to start, running it under
 > gdbserver showed a backtrace pointing to a lazy symbol lookup
 > failure triggered by:
 >
 > drmmode_display.c:905:
 >      return gbm_bo_get_stride(bo->gbm);
 >
 > Which is part of:
 >
 >
 > uint32_t
 > drmmode_bo_get_pitch(drmmode_bo *bo)
 > {
 > #ifdef GLAMOR_HAS_GBM
 >      if (bo->gbm)
 >      return gbm_bo_get_stride(bo->gbm);
 > #endif
 >
 >      return bo->dumb->pitch;
 > }
 >
 > Strange enough a LD_PRELOAD of libgbm does not
 > fix this and libgbm already gets dragged in by
 > libglamor_egl.so so this should not be a problem.
 >
 > Still I tried this change:
 >
 > --- a/hw/xfree86/drivers/modesetting/Makefile.am
 > +++ b/hw/xfree86/drivers/modesetting/Makefile.am
 > @@ -39,7 +39,7 @@ AM_CPPFLAGS = \
 >
 >   modesetting_drv_la_LTLIBRARIES = modesetting_drv.la 
<http://modesetting_drv.la>
 >   modesetting_drv_la_LDFLAGS = -module -avoid-version
 > -modesetting_drv_la_LIBADD = $(UDEV_LIBS) $(DRM_LIBS)
 > +modesetting_drv_la_LIBADD = $(UDEV_LIBS) $(DRM_LIBS) $(GBM_LIBS)
 >   modesetting_drv_ladir = @moduledir@/drivers
 >
 >   modesetting_drv_la_SOURCES = \
 >
 > And now Xorg will start, still very weird since the
 > exact same Xorg binaries work fine on Intel integrated
 > gfx where the gbm_bo_get_stride() call also happens ...
 >
 >
 > So with this "fix" it starts, but it crashes as soon as I resize the
 > vm-window and thus the screen gets resized:
 >
 > bt
 > #0  OsSigHandler (signo=11, sip=0x7ffc160ad2f0, unused=0x7ffc160ad1c0)
 >      at osinit.c:114
 > #1  
 > #2  miModifyPixmapHeader (pPixmap=0x28c4460, width=1920, height=992, 
depth=-1,
 >      bitsPerPixel=-1, devKind=7680, pPixData=0x0) at miscrinit.c:64
 > #3  0x7fdc13d64471 in drmmode_xf86crtc_resize (scrn=0x21751f0, 
width=1920,
 >      height=992) at drmmode_display.c:3166
 > #4  0x004bb9d8 in xf86RandR12ScreenSetSize (pScreen=0x23dc9b0,
 >      width=1920, height=992, mmWidth=508, mmHeight=262) at 
xf86RandR12.c:698
 > #5  0x005092f0 in ProcRRSetScreenSize (client=0x29c6af0)
 >      at rrscreen.c:289
 > #6  0x0043fcee in Dispatch () at dispatch.c:478
 >
 > And miscrinit.c:64 is hte "{" of:
 >
 > Bool
 > miModifyPixmapHeader(PixmapPtr pPixmap, int width, int height, int depth,
 >   int bitsPerPixel, int devKind, void *pPixData)
 > {
 >      if (!pPixmap)
 >      return FALSE;
 >
 > Which is a strange place to crash. Even more weird after adding a
 > breakpoint a bit before drmmode_display.c:3166, I get a segfault
 > while stepping through earlier lines, pointing at SmartScheduleTimer()
 > and specifically again at the opening "{" as if something is wrong with
 > the stack and the stack cannot handle function calls being stacked one
 > level deeper.
 >
 > This makes me wonder if this is a stack depth/overflow issue, does the
 > xserver have code somewhere to limit its stacksize and could we be
 > hitting that ?
 >
 > Or maybe a bad interaction with gcc-s stack protection?
 >
 > This feels as if we are hitting a guard page at the end of the stack
 > here?

One important thing which I only put in the subject, this happens
when using glmamor with llvmpipe, something which is new in 1.20,
older xservers never used glamor on llvmpipe. Things work fine
if I disable glamor in a xorg.conf snippet.

Arguably we should disable glamor when running on llvmpipe because
of performance reasons, still these crashes should not happen.

Regards,

Hans

___
xorg-devel@lists.x.org <mailto:xorg-devel@lists.x.org>: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Xorg-1.20.1 crashes when using glamor on top of llvmpipe

[Hans de Goede]

Hi,

On 30-08-18 22:48, Hans de Goede wrote:

HI all,

I've been debugging some strange crashes with Xorg-1.20.1 inside
a virtualbox guest and I can use some help with this.

At first Xorg completely failed to start, running it under
gdbserver showed a backtrace pointing to a lazy symbol lookup
failure triggered by:

drmmode_display.c:905:
     return gbm_bo_get_stride(bo->gbm);

Which is part of:


uint32_t
drmmode_bo_get_pitch(drmmode_bo *bo)
{
#ifdef GLAMOR_HAS_GBM
     if (bo->gbm)
     return gbm_bo_get_stride(bo->gbm);
#endif

     return bo->dumb->pitch;
}

Strange enough a LD_PRELOAD of libgbm does not
fix this and libgbm already gets dragged in by
libglamor_egl.so so this should not be a problem.

Still I tried this change:

--- a/hw/xfree86/drivers/modesetting/Makefile.am
+++ b/hw/xfree86/drivers/modesetting/Makefile.am
@@ -39,7 +39,7 @@ AM_CPPFLAGS = \

  modesetting_drv_la_LTLIBRARIES = modesetting_drv.la
  modesetting_drv_la_LDFLAGS = -module -avoid-version
-modesetting_drv_la_LIBADD = $(UDEV_LIBS) $(DRM_LIBS)
+modesetting_drv_la_LIBADD = $(UDEV_LIBS) $(DRM_LIBS) $(GBM_LIBS)
  modesetting_drv_ladir = @moduledir@/drivers

  modesetting_drv_la_SOURCES = \

And now Xorg will start, still very weird since the
exact same Xorg binaries work fine on Intel integrated
gfx where the gbm_bo_get_stride() call also happens ...


So with this "fix" it starts, but it crashes as soon as I resize the
vm-window and thus the screen gets resized:

bt
#0  OsSigHandler (signo=11, sip=0x7ffc160ad2f0, unused=0x7ffc160ad1c0)
     at osinit.c:114
#1  
#2  miModifyPixmapHeader (pPixmap=0x28c4460, width=1920, height=992, depth=-1,
     bitsPerPixel=-1, devKind=7680, pPixData=0x0) at miscrinit.c:64
#3  0x7fdc13d64471 in drmmode_xf86crtc_resize (scrn=0x21751f0, width=1920,
     height=992) at drmmode_display.c:3166
#4  0x004bb9d8 in xf86RandR12ScreenSetSize (pScreen=0x23dc9b0,
     width=1920, height=992, mmWidth=508, mmHeight=262) at xf86RandR12.c:698
#5  0x005092f0 in ProcRRSetScreenSize (client=0x29c6af0)
     at rrscreen.c:289
#6  0x0043fcee in Dispatch () at dispatch.c:478

And miscrinit.c:64 is hte "{" of:

Bool
miModifyPixmapHeader(PixmapPtr pPixmap, int width, int height, int depth,
  int bitsPerPixel, int devKind, void *pPixData)
{
     if (!pPixmap)
     return FALSE;

Which is a strange place to crash. Even more weird after adding a
breakpoint a bit before drmmode_display.c:3166, I get a segfault
while stepping through earlier lines, pointing at SmartScheduleTimer()
and specifically again at the opening "{" as if something is wrong with
the stack and the stack cannot handle function calls being stacked one
level deeper.

This makes me wonder if this is a stack depth/overflow issue, does the
xserver have code somewhere to limit its stacksize and could we be
hitting that ?

Or maybe a bad interaction with gcc-s stack protection?

This feels as if we are hitting a guard page at the end of the stack
here?


One important thing which I only put in the subject, this happens
when using glmamor with llvmpipe, something which is new in 1.20,
older xservers never used glamor on llvmpipe. Things work fine
if I disable glamor in a xorg.conf snippet.

Arguably we should disable glamor when running on llvmpipe because
of performance reasons, still these crashes should not happen.

Regards,

Hans

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Xorg-1.20.1 crashes when using glamor on top of llvmpipe

[Hans de Goede]

HI all,

I've been debugging some strange crashes with Xorg-1.20.1 inside
a virtualbox guest and I can use some help with this.

At first Xorg completely failed to start, running it under
gdbserver showed a backtrace pointing to a lazy symbol lookup
failure triggered by:

drmmode_display.c:905:
return gbm_bo_get_stride(bo->gbm);

Which is part of:


uint32_t
drmmode_bo_get_pitch(drmmode_bo *bo)
{
#ifdef GLAMOR_HAS_GBM
if (bo->gbm)
return gbm_bo_get_stride(bo->gbm);
#endif

return bo->dumb->pitch;
}

Strange enough a LD_PRELOAD of libgbm does not
fix this and libgbm already gets dragged in by
libglamor_egl.so so this should not be a problem.

Still I tried this change:

--- a/hw/xfree86/drivers/modesetting/Makefile.am
+++ b/hw/xfree86/drivers/modesetting/Makefile.am
@@ -39,7 +39,7 @@ AM_CPPFLAGS = \

 modesetting_drv_la_LTLIBRARIES = modesetting_drv.la
 modesetting_drv_la_LDFLAGS = -module -avoid-version
-modesetting_drv_la_LIBADD = $(UDEV_LIBS) $(DRM_LIBS)
+modesetting_drv_la_LIBADD = $(UDEV_LIBS) $(DRM_LIBS) $(GBM_LIBS)
 modesetting_drv_ladir = @moduledir@/drivers

 modesetting_drv_la_SOURCES = \

And now Xorg will start, still very weird since the
exact same Xorg binaries work fine on Intel integrated
gfx where the gbm_bo_get_stride() call also happens ...


So with this "fix" it starts, but it crashes as soon as I resize the
vm-window and thus the screen gets resized:

bt
#0  OsSigHandler (signo=11, sip=0x7ffc160ad2f0, unused=0x7ffc160ad1c0)
at osinit.c:114
#1  
#2  miModifyPixmapHeader (pPixmap=0x28c4460, width=1920, height=992, depth=-1,
bitsPerPixel=-1, devKind=7680, pPixData=0x0) at miscrinit.c:64
#3  0x7fdc13d64471 in drmmode_xf86crtc_resize (scrn=0x21751f0, width=1920,
height=992) at drmmode_display.c:3166
#4  0x004bb9d8 in xf86RandR12ScreenSetSize (pScreen=0x23dc9b0,
width=1920, height=992, mmWidth=508, mmHeight=262) at xf86RandR12.c:698
#5  0x005092f0 in ProcRRSetScreenSize (client=0x29c6af0)
at rrscreen.c:289
#6  0x0043fcee in Dispatch () at dispatch.c:478

And miscrinit.c:64 is hte "{" of:

Bool
miModifyPixmapHeader(PixmapPtr pPixmap, int width, int height, int depth,
 int bitsPerPixel, int devKind, void *pPixData)
{
if (!pPixmap)
return FALSE;

Which is a strange place to crash. Even more weird after adding a
breakpoint a bit before drmmode_display.c:3166, I get a segfault
while stepping through earlier lines, pointing at SmartScheduleTimer()
and specifically again at the opening "{" as if something is wrong with
the stack and the stack cannot handle function calls being stacked one
level deeper.

This makes me wonder if this is a stack depth/overflow issue, does the
xserver have code somewhere to limit its stacksize and could we be
hitting that ?

Or maybe a bad interaction with gcc-s stack protection?

This feels as if we are hitting a guard page at the end of the stack
here?

Regards,

Hans

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] Xorg.wrap: Ensure correct ordering of post-install hook

[Hans de Goede]

Hi,

On 30-12-17 23:46, Lukáš Krejčí wrote:

The install rule of Xorg.wrap is currently a dependency of the
install-data target instead of the install-exec target. The build also
uses install-exec-hook to change the ownership and set the SUID bit on
the Xorg.wrap binary. The problem is that install-exec-hook is only
ordered respective to the install-exec target, the rules of install-data
may or may not have been executed.

If install-exec-hook runs before the Xorg.wrap binary is in place,
a message similar to the following will be present in the build log:

chown: cannot access '/pkgdir/usr/lib/xorg-server/Xorg.wrap': No such file or 
directory
make[6]: [Makefile:1151: install-exec-hook] Error 1 (ignored)

All that needs to be done is to change the name of the program variable
to contain 'exec' for the install rule to depend on the install-exec
target.

Excerpt from the Automake manual, chapter 12.2 The Two Parts of Install:
"Any variable using a user-defined directory prefix with ‘exec’ in the
name (e.g., myexecbin_PROGRAMS) is installed by install-exec. All other
user-defined prefixes are installed by install-data."

Signed-off-by: Lukáš Krejčí <lskre...@gmail.com>


Looks good to me:

Acked-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
  hw/xfree86/Makefile.am | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am
index b876b79ab..458720052 100644
--- a/hw/xfree86/Makefile.am
+++ b/hw/xfree86/Makefile.am
@@ -84,8 +84,8 @@ Xorg_DEPENDENCIES = $(LOCAL_LIBS)
  Xorg_LDFLAGS = $(LD_EXPORT_SYMBOLS_FLAG)
  
  if SUID_WRAPPER

-wrapdir = $(SUID_WRAPPER_DIR)
-wrap_PROGRAMS = Xorg.wrap
+wrapexecdir = $(SUID_WRAPPER_DIR)
+wrapexec_PROGRAMS = Xorg.wrap
  Xorg_wrap_SOURCES = xorg-wrapper.c
  endif
  


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 3/3] ramdac: Handle master and slave cursors independently

[Hans de Goede]

Hi,

On 09-11-17 02:34, Alex Goins wrote:

Thanks both for the quick feedback. Replies inline.

On Wed, 8 Nov 2017, Hans de Goede wrote:


https://lists.x.org/archives/xorg-devel/2016-September/050973.html implies
that
xf86CursorScreenPtr is part of the ABI. Mark xf86CursorPriv.h as such.


I'm not sure if exporting this is a good idea. I assume you are just
after xf86CursorScreenPtr->SWCursor ?  If so it is probably a better
idea to add a helper function taking pScreen which gets that and add
that function to the ABI.


That works for me, this is just based on the past advice for drivers to lookup
xf86CursorScreenPtr to tell if we are using a SW cursor, which sets a precedent
of it being considered ABI.


I understand, but I now realize that was partially bad advice, IMHO
we really don't want to grow the ABI, if at all possible we want to
shrink it. In my mind exporting a function is less likely to lead to
future ABI breaks then exporting the contents of a struct, hence
my preference for the helper function.


+Bool
+xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr
infoPtr)
+{
+Bool use_hw_cursor = FALSE;
+
+input_lock();
+
+if (xf86CheckSlavesHWCursor_locked(pScreen, cursor)) {
+use_hw_cursor = TRUE;
+goto unlock;


This goto seems wrong, what if the secondary GPU has outputs which
can do a hw-cursor, but the primary GPU also has active video outputs
and the primary GPU does not support a hw-cursor ?

I think you always need to check the master supports a hw-cursor.


+/* if there are no active slaves that support HW cursor, or using the
HW
+ * cursor on them failed, use the master */
+if (ScreenPriv->SlaveHWCursor) {
+xf86SetSlavesCursor_locked(pScreen, NullCursor, x, y);
+ScreenPriv->SlaveHWCursor = FALSE;
+}
+ret = xf86ScreenSetCursor(pScreen, pCurs, x, y);
  out:
   input_unlock();



Again what if both the primary and secondary GPU have active video outputs,
then you need to update the cursor on both. You seem te be thinking about
something like the XPS15 here, used with the nvidia driver forcing the
nvidia GPU to become the primary GPU, so the primary will not have any
video-outputs. But when using for example a Latitude E6430 in optimus
mode with nouveau, the intel GPU will be the primary, driving the LCD
panel and the nvidia GPU will be the secondary driving the HDMI output,
so you need to set the cursor on both.


You're right, there is a possibility of combined native and PRIME outputs. In
fact, I typically test on a desktop system with iGPU Multi-Monitor, where this
possibility is especially clear.

I overlooked the case of native + PRIME where the PRIME slave supports HW
cursor. Native + PRIME with non-HW cursor slave works, but the former doesn't.


Ok.



On Wed, 8 Nov 2017, Michel Dänzer wrote:


Change 7b634067 added HW cursor support for PRIME by removing the
pixmap_dirty_list check from xf86CursorSetCursor() and making the requisite
cursor functions set/check the cursor both on master and slave.

However, before this change, drivers that did not make use of pixmap_dirty_list
to implement PRIME master were able to pass this check. That may have been a
bug, but in effect it allowed hardware cursor to be enabled with PRIME, where
the master composites the cursor into the shared pixmap.

Naturally, the slave driving an actual hardware cursor is preferable to the
master compositing a cursor into the shared pixmap, but there are certain
situations where the slave cannot drive a hardware cursor (certain DRM drivers,
or when used with a transform). In these cases the master may still be capable
of compositing a cursor,


How and where exactly would this "compositing the cursor into the shared
pixmap" happen? Looks like this is just left for the master screen
driver to handle? If so, there probably needs to be a mechanism for the
master screen driver to opt into this.


Yes, it's just left for the master screen the handle as if it were a hardware
cursor.

Prior to change 7b634067 this was the existing behavior of the NVIDIA driver +
PRIME outputs, due to the fact that the check to exclude HW cursor support on
PRIME outputs relied on pScreen->pixmap_dirty_list not being empty. With NVIDIA
as the master, X would successfully set a cursor on the master despite PRIME
being in use, and the master would use that to composite its own cursor into the
shared pixmap.

Change 7b634067 had the side effect of preventing this configuration, and the
intent here is to re-enable it as an alternative fallback to the server's SW
cursor, while maintaining the possibility of slave-driven HW cursor in
configurations that support it.


and that would be preferable to using the server's software cursor (due
to the fact that it's unsynchronzied by OpenGL rendering to the root
window, it can cause corruption with certain compositors).


Frankly, that sounds like an issue with your direct rendering
i

Re: [PATCH] modesetting: fail PreInit() if the device has zero connectors

[Hans de Goede]

Hi,

On 03-11-17 17:56, Giuseppe Bilotta wrote:

On Fri, Nov 3, 2017 at 10:09 AM, Hans de Goede <hdego...@redhat.com> wrote:

Weird, on my XPS15 9550 where the nvidia GPU does not have/drives any
outputs
I do get 2 devices in xrandr --listproviders as expected. You may want to
start
with figuring out why the normal setup where you load nouveau normally is
not
working. Perhaps once that works, it will also powerdown the GPU properly.


I have an XPS15 9350 with a similar situation. You get two devices if
nouveau is loaded before Xorg starts.

If I understand Tobias' mail, his situation is with nouveau modprobed
_while X is running already_, not if it's loaded before X starts. I've
tried it in the past and I got the same segfaults that he is getting.
I have AutoAddGPU set to false specifically to avoid this segfault,
since I use a similar setup (no driver loaded, modprobe as needed),
although in my case it's more out of a need to be able to switch to
nvidia's proprietary as-needed.

In my case I have verified that without loading nouveau or nvidia the
GPU is powered up, which is supoptimal battery-wise, and I'm not sure
nvidia powers down the GPU while not in use (nouveau does, byt to me
is mostly useless, because the performance it achieves on the dGPU is
less than the one I get on Intel's iGP, and I still need the nvidia
one for CUDA), but as Tobias mentioned, that's a separate issue from
the segfault that comes from runtime nouveau modprobing.


Right, so the thing to fix here is pin down where the segfault from
runtime nouveau probing comes from and fix that, so that once
modprobed things work the same as they do when nouveau is loaded
before X starts.

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 3/3] ramdac: Handle master and slave cursors independently

[Hans de Goede]

Hi,

On 08-11-17 05:15, Alex Goins wrote:

Change 7b634067 added HW cursor support for PRIME by removing the
pixmap_dirty_list check from xf86CursorSetCursor() and making the requisite
cursor functions set/check the cursor both on master and slave.

However, before this change, drivers that did not make use of pixmap_dirty_list
to implement PRIME master were able to pass this check. That may have been a
bug, but in effect it allowed hardware cursor to be enabled with PRIME, where
the master composites the cursor into the shared pixmap.

Naturally, the slave driving an actual hardware cursor is preferable to the
master compositing a cursor into the shared pixmap, but there are certain
situations where the slave cannot drive a hardware cursor (certain DRM drivers,
or when used with a transform). In these cases the master may still be capable
of compositing a cursor, and that would be preferable to using the server's
software cursor (due to the fact that it's unsynchronzied by OpenGL rendering to
the root window, it can cause corruption with certain compositors).

The PRIME HW cursor change works by checking both master and slave HW cursor
capabilities, and setting the cursor on both. For masters such as modesetting,
the master cursor capabilities check will pass but the cursor set will be a
NOOP, effectively driving a hardware cursor only on the slave while still
passing the check. However, if two different drivers with different sets of
capabilities are used, HW cursor ends up only being used if the intersection of
capabilities are supported. For example, if the master can drive a cursor with a
transform, and the slave can't, checking capabilities on both means we
unnecessarily fall back to SW cursor.

To alleviate this issue while still prioritizing HW cursor on the slave, this
change restructures the HW cursor code such that the slave aspects of these
functions are tried first, falling back to the master if they fail.

SlaveHWCursor, a flag for querying if the hardware cursor is being driven by
slaves, is added to xf86CursorScreenRec, resulting in an ABI break.

Signed-off-by: Alex Goins 
---
  hw/xfree86/ramdac/xf86CursorPriv.h |   3 +
  hw/xfree86/ramdac/xf86HWCurs.c | 114 +++--
  2 files changed, 88 insertions(+), 29 deletions(-)

diff --git a/hw/xfree86/ramdac/xf86CursorPriv.h 
b/hw/xfree86/ramdac/xf86CursorPriv.h
index 397d2a1..83ea379 100644
--- a/hw/xfree86/ramdac/xf86CursorPriv.h
+++ b/hw/xfree86/ramdac/xf86CursorPriv.h
@@ -35,6 +35,9 @@ typedef struct {
  Bool HWCursorForced;
  
  void *transparentData;

+
+/* If hardware cursor is being driven by slaves */
+Bool SlaveHWCursor;
  } xf86CursorScreenRec, *xf86CursorScreenPtr;
  
  Bool xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y);

diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
index 15b1cd7..786ed54 100644
--- a/hw/xfree86/ramdac/xf86HWCurs.c
+++ b/hw/xfree86/ramdac/xf86HWCurs.c
@@ -136,18 +136,13 @@ xf86ScreenCheckHWCursor(ScreenPtr pScreen, CursorPtr 
cursor, xf86CursorInfoPtr i
   (!infoPtr->UseHWCursor || infoPtr->UseHWCursor(pScreen, cursor)));
  }
  
-Bool

-xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr 
infoPtr)
+static Bool
+xf86CheckSlavesHWCursor_locked(ScreenPtr pScreen, CursorPtr cursor)
  {
  ScreenPtr pSlave;
-Bool use_hw_cursor = TRUE;
-
-input_lock();
  
-if (!xf86ScreenCheckHWCursor(pScreen, cursor, infoPtr)) {

-use_hw_cursor = FALSE;
-goto unlock;
-}
+Bool use_hw_cursor = FALSE;
+Bool slave_consuming_pixmap = FALSE;
  
  /* ask each driver consuming a pixmap if it can support HW cursor */

  xorg_list_for_each_entry(pSlave, >slave_list, slave_head) {
@@ -156,6 +151,11 @@ xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, 
xf86CursorInfoPtr infoPtr
  if (!RRHasScanoutPixmap(pSlave))
  continue;
  
+if (!slave_consuming_pixmap) {

+slave_consuming_pixmap = TRUE;
+use_hw_cursor = TRUE;
+}
+
  sPriv = dixLookupPrivate(>devPrivates, xf86CursorScreenKey);
  if (!sPriv) { /* NULL if Option "SWCursor", possibly other conditions 
*/
  use_hw_cursor = FALSE;
@@ -169,6 +169,26 @@ xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, 
xf86CursorInfoPtr infoPtr
  }
  }
  
+/* there are active slaves and they all support hw cursor */

+return (slave_consuming_pixmap && use_hw_cursor);
+}
+
+Bool
+xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr 
infoPtr)
+{
+Bool use_hw_cursor = FALSE;
+
+input_lock();
+
+if (xf86CheckSlavesHWCursor_locked(pScreen, cursor)) {
+use_hw_cursor = TRUE;
+goto unlock;


This goto seems wrong, what if the secondary GPU has outputs which
can do a hw-cursor, but the primary GPU also has active video outputs
and the primary GPU does not support a 

Re: [PATCH xserver 2/3] ramdac: Add xf86CursorPriv.h to ABI

[Hans de Goede]

Hi,

On 08-11-17 05:15, Alex Goins wrote:

https://lists.x.org/archives/xorg-devel/2016-September/050973.html implies that
xf86CursorScreenPtr is part of the ABI. Mark xf86CursorPriv.h as such.


I'm not sure if exporting this is a good idea. I assume you are just
after xf86CursorScreenPtr->SWCursor ?  If so it is probably a better
idea to add a helper function taking pScreen which gets that and add
that function to the ABI.

Regards,

Hans





Signed-off-by: Alex Goins 
---
  hw/xfree86/ramdac/Makefile.am | 4 ++--
  hw/xfree86/sdksyms.sh | 1 +
  2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/ramdac/Makefile.am b/hw/xfree86/ramdac/Makefile.am
index 59e0996..89aab61 100644
--- a/hw/xfree86/ramdac/Makefile.am
+++ b/hw/xfree86/ramdac/Makefile.am
@@ -3,9 +3,9 @@ noinst_LTLIBRARIES = libramdac.la
  libramdac_la_SOURCES = xf86RamDac.c xf86RamDacCmap.c \
 xf86CursorRD.c xf86HWCurs.c IBM.c BT.c TI.c
  
-sdk_HEADERS = BT.h IBM.h TI.h xf86Cursor.h xf86RamDac.h

+sdk_HEADERS = BT.h IBM.h TI.h xf86Cursor.h xf86CursorPriv.h xf86RamDac.h
  
-EXTRA_DIST = BTPriv.h IBMPriv.h TIPriv.h xf86CursorPriv.h xf86RamDacPriv.h \

+EXTRA_DIST = BTPriv.h IBMPriv.h TIPriv.h xf86RamDacPriv.h \
CURSOR.NOTES
  
  AM_CFLAGS = $(DIX_CFLAGS) $(XORG_CFLAGS)

diff --git a/hw/xfree86/sdksyms.sh b/hw/xfree86/sdksyms.sh
index 9aa1eec..56e3fab 100755
--- a/hw/xfree86/sdksyms.sh
+++ b/hw/xfree86/sdksyms.sh
@@ -147,6 +147,7 @@ cat > sdksyms.c << EOF
  #include "IBM.h"
  #include "TI.h"
  #include "xf86Cursor.h"
+#include "xf86CursorPriv.h"
  #include "xf86RamDac.h"
  
  


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/3] ramdac: Fix formatting in xf86CheckHWCursor()

[Hans de Goede]

Hi,

On 08-11-17 05:15, Alex Goins wrote:

xf86CheckHWCursor() has spacing that is inconsistent with the rest of the file.
Correct this in preparation for subsequent changes.

Signed-off-by: Alex Goins <ago...@nvidia.com>


LGTM:

Acked-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
  hw/xfree86/ramdac/xf86HWCurs.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
index 366837c..15b1cd7 100644
--- a/hw/xfree86/ramdac/xf86HWCurs.c
+++ b/hw/xfree86/ramdac/xf86HWCurs.c
@@ -146,7 +146,7 @@ xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, 
xf86CursorInfoPtr infoPtr
  
  if (!xf86ScreenCheckHWCursor(pScreen, cursor, infoPtr)) {

  use_hw_cursor = FALSE;
-   goto unlock;
+goto unlock;
  }
  
  /* ask each driver consuming a pixmap if it can support HW cursor */

@@ -159,14 +159,14 @@ xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, 
xf86CursorInfoPtr infoPtr
  sPriv = dixLookupPrivate(>devPrivates, xf86CursorScreenKey);
  if (!sPriv) { /* NULL if Option "SWCursor", possibly other conditions 
*/
  use_hw_cursor = FALSE;
-   break;
-   }
+break;
+}
  
  /* FALSE if HWCursor not supported by slave */

  if (!xf86ScreenCheckHWCursor(pSlave, cursor, sPriv->CursorInfoPtr)) {
  use_hw_cursor = FALSE;
-   break;
-   }
+break;
+}
  }
  
  unlock:



___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] modesetting: fail PreInit() if the device has zero connectors

[Hans de Goede]

HI,

On 30-10-17 10:42, Tobias Jakobi wrote:

Hi again,

I took a closer look at the problem, but I can't really figure out what is going
wrong.

So, the assert that is triggered is always the following one:

Xorg: 
/var/tmp/portage/x11-base/xorg-server-1.19.5/work/xorg-server-1.19.5/dix/privates.c:385:
 dixRegisterPrivateKey: Assertion `!global_keys[type].created' failed.


This happens when glamor_init() calls dixRegisterPrivateKey() with
type=PRIVATE_PIXMAP.

I think the only place where .created is incremented is dixInitScreenPrivates(),
which is called from AllocatePixmap(). But apparantly this is never called
before glamor_init().

I then had a look at modeset, which uses dixRegisterScreenSpecificPrivateKey()
instead of dixRegisterPrivateKey(). I then proceeded to replace all calls with
type=PRIVATE_PIXMAP and type=PRIVATE_GC in glamor with this. This helps
somewhat, but now it crashes later.

Same assertion failure, but now in Xv:

Xorg: 
/var/tmp/portage/x11-base/xorg-server-1.19.5/work/xorg-server-1.19.5/dix/privates.c:385:
 dixRegisterPrivateKey: Assertion `!global_keys[type].created' failed.

Thread 1 "Xorg" received signal SIGABRT, Aborted.
0x7fada58d1178 in raise () from /lib64/libc.so.6
#0  0x7fada58d1178 in raise () from /lib64/libc.so.6
No symbol table info available.
#1  0x7fada58d25fa in abort () from /lib64/libc.so.6
No symbol table info available.
#2  0x7fada58ca0b7 in ?? () from /lib64/libc.so.6
No symbol table info available.
#3  0x7fada58ca162 in __assert_fail () from /lib64/libc.so.6
No symbol table info available.
#4  0x00461871 in dixRegisterPrivateKey (key=0x8bf7a0 
, type=PRIVATE_WINDOW, size=0)
 at 
/var/tmp/portage/x11-base/xorg-server-1.19.5/work/xorg-server-1.19.5/dix/privates.c:385
 t = PRIVATE_XSELINUX
 offset = 21715344
 bytes = 8
 __PRETTY_FUNCTION__ = "dixRegisterPrivateKey"
#5  0x004abbdd in xf86XVScreenInit (pScreen=0xf54de0, 
adaptors=0x7fffebda4eb8, num=1)
 at 
/var/tmp/portage/x11-base/xorg-server-1.19.5/work/xorg-server-1.19.5/hw/xfree86/common/xf86xv.c:239
 pScrn = 0x8
 ScreenPriv = 0xecb850
#6  0x7fad9e5a3a67 in ScreenInit (pScreen=0xf54de0, argc=0, argv=0x0)
 at 
/var/tmp/portage/x11-base/xorg-server-1.19.5/work/xorg-server-1.19.5/hw/xfree86/drivers/modesetting/driver.c:1683
 glamor_adaptor = 0x14fd760
 pScrn = 0xed3680
 ms = 0xefdd80
 visual = 0x99f4e8


I could of course continue to also replace these calls, but honestly it feels
like opening a can of worms.

My impression is that modeset does some initialization in the wrong order, which
by chance works in the non-hotplug scenario. Which led me to investigate what
happens with non-hotplug.

The funny thing is, nothing happens. modeset isn't even loaded for the device.
So something prematurely stops all the probing for /dev/dri/card1.

I'd like to investigate this further, but at the moment I have no idea what to
look for.


Weird, on my XPS15 9550 where the nvidia GPU does not have/drives any outputs
I do get 2 devices in xrandr --listproviders as expected. You may want to start
with figuring out why the normal setup where you load nouveau normally is not
working. Perhaps once that works, it will also powerdown the GPU properly.

Regards,

Hans






With best wishes,
Tobias



Hans de Goede wrote:

Hi,

On 20-10-17 19:08, tobias.jako...@uni-bielefeld.de wrote:

On laptop systems with a dedicated (powerful) GPU A, you usually
have all connectors routed to another (less-powerful) GPU B.

With my setup (GPU A = Nvidia, GPU B = Intel) I keep GPU A switched
off by not loading the nouveau kernel driver during boot.

Loading nouveau while X is running then crashes the server:
[   540.775] (EE) 0: /usr/bin/X (xorg_backtrace+0x41) [0x57fa31]
[   540.775] (EE) 1: /usr/bin/X (0x40+0x183429) [0x583429]
[   540.775] (EE) 2: /lib64/libpthread.so.0 (0x7ff02d508000+0x10ec0)
[0x7ff02d518ec0]
[   540.775] (EE) 3: /lib64/libc.so.6 (gsignal+0x38) [0x7ff02d1a2178]
[   540.775] (EE) 4: /lib64/libc.so.6 (abort+0x16a) [0x7ff02d1a35fa]
[   540.775] (EE) 5: /lib64/libc.so.6 (0x7ff02d16f000+0x2c0b7) [0x7ff02d19b0b7]
[   540.775] (EE) 6: /lib64/libc.so.6 (0x7ff02d16f000+0x2c162) [0x7ff02d19b162]
[   540.775] (EE) 7: /usr/bin/X (dixRegisterPrivateKey+0x247) [0x452197]
[   540.775] (EE) 8: /usr/lib64/xorg/modules/libglamoregl.so
(glamor_init+0x160) [0x7ff00ee564d0]
[   540.775] (EE) 9: /usr/lib64/xorg/modules/drivers/modesetting_drv.so
(0x7ff01c19a000+0x83e1) [0x7ff01c1a23e1]
[   540.775] (EE) 10: /usr/bin/X (AddGPUScreen+0xf3) [0x4348b3]
[   540.775] (EE) 11: /usr/bin/X (0x40+0x90271) [0x490271]
[   540.775] (EE) 12: /usr/bin/X (0x40+0x9547b) [0x49547b]
[   540.775] (EE) 13: /usr/bin/X (0x40+0x905d7) [0x4905d7]
[   540.775] (EE) 14: /usr/bin/X (xf86VTEnter+0x1bb) [0x47399b]
[   540.775] (EE) 15: /usr/bin/X (WakeupHandler+0xda) [0x438e3a]
[   540.775] (EE) 16: /usr/bin

Re: [PATCH] modesetting: fail PreInit() if the device has zero connectors

[Hans de Goede]

Hi,

On 20-10-17 19:08, tobias.jako...@uni-bielefeld.de wrote:

On laptop systems with a dedicated (powerful) GPU A, you usually
have all connectors routed to another (less-powerful) GPU B.

With my setup (GPU A = Nvidia, GPU B = Intel) I keep GPU A switched
off by not loading the nouveau kernel driver during boot.

Loading nouveau while X is running then crashes the server:
[   540.775] (EE) 0: /usr/bin/X (xorg_backtrace+0x41) [0x57fa31]
[   540.775] (EE) 1: /usr/bin/X (0x40+0x183429) [0x583429]
[   540.775] (EE) 2: /lib64/libpthread.so.0 (0x7ff02d508000+0x10ec0) 
[0x7ff02d518ec0]
[   540.775] (EE) 3: /lib64/libc.so.6 (gsignal+0x38) [0x7ff02d1a2178]
[   540.775] (EE) 4: /lib64/libc.so.6 (abort+0x16a) [0x7ff02d1a35fa]
[   540.775] (EE) 5: /lib64/libc.so.6 (0x7ff02d16f000+0x2c0b7) [0x7ff02d19b0b7]
[   540.775] (EE) 6: /lib64/libc.so.6 (0x7ff02d16f000+0x2c162) [0x7ff02d19b162]
[   540.775] (EE) 7: /usr/bin/X (dixRegisterPrivateKey+0x247) [0x452197]
[   540.775] (EE) 8: /usr/lib64/xorg/modules/libglamoregl.so 
(glamor_init+0x160) [0x7ff00ee564d0]
[   540.775] (EE) 9: /usr/lib64/xorg/modules/drivers/modesetting_drv.so 
(0x7ff01c19a000+0x83e1) [0x7ff01c1a23e1]
[   540.775] (EE) 10: /usr/bin/X (AddGPUScreen+0xf3) [0x4348b3]
[   540.775] (EE) 11: /usr/bin/X (0x40+0x90271) [0x490271]
[   540.775] (EE) 12: /usr/bin/X (0x40+0x9547b) [0x49547b]
[   540.775] (EE) 13: /usr/bin/X (0x40+0x905d7) [0x4905d7]
[   540.775] (EE) 14: /usr/bin/X (xf86VTEnter+0x1bb) [0x47399b]
[   540.775] (EE) 15: /usr/bin/X (WakeupHandler+0xda) [0x438e3a]
[   540.775] (EE) 16: /usr/bin/X (WaitForSomething+0x1ce) [0x57d6fe]
[   540.775] (EE) 17: /usr/bin/X (0x40+0x34221) [0x434221]
[   540.775] (EE) 18: /usr/bin/X (0x40+0x382f8) [0x4382f8]
[   540.775] (EE) 19: /lib64/libc.so.6 (__libc_start_main+0xf0) [0x7ff02d18f670]
[   540.776] (EE) 20: /usr/bin/X (_start+0x29) [0x4235b9]

In particular note that GLAMOR is initialized for GPU A, which
makes no sense since it has no connectors.

Fix this by bailing out early in the modesetting DDX when a setup
with zero connectors is detected.

Signed-off-by: Tobias Jakobi 


Sorry, but NACK. The modesetting driver actually had such a check before
and I removed it because not having a driver breaks rendering on
the dedicated GPU with "DRI_PRIME=1" for dri2 clients.

I guess that there is some sort of assumption in the code for dealing
with glamor failure that it only happens on coldplug, if you really
want to be able to modprobe nouveau later you should figure out what
is exactly going wrong and fix that.

Note BTW that nouveau should runtime suspend the GPU, even with Xorg
running and there really is no need to not load nouveau.

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xkeyboard-config] Add evdev mappings for KEY_SOUND, KEY_UWB, KEY_WWAN and KEY_RFKILL

[Hans de Goede]

Hi,

On 15-05-17 16:59, Sergey Udaltsov wrote:

Hi Hans

I can push, of course, if the mutual consensus is that now is the right time 
for it.


I consider the patch ready for merging / pushing, which is why I tried to push 
it
myself :)

Regards,

Hans


On Mon, May 15, 2017 at 8:26 AM, Hans de Goede <hdego...@redhat.com 
<mailto:hdego...@redhat.com>> wrote:

Hi,

On 15-05-17 03:06, Peter Hutterer wrote:

adding svu to CC:

On Fri, May 12, 2017 at 04:18:42PM +0200, Hans de Goede wrote:

KEY_SOUND, KEY_WWAN and KEY_RFKILL are used on actual devices and
current xproto master defines keysyms for these. Add mappings for
these keys.

The XF86UWB keysm exists for quite a while now, as does the 
KEY_SOUND
evdev code for it, so lets adds a mapping for that too.

Note that this replaces the I255 to XF86Hibernate mapping in
xkb_symbols "evdev", I don't know where that came from, but it is
wrong, I255 was until now not defined in keycodes/evdev and
KEY_HIBERNATE is not defined in linux/input-event-codes.h .


see xk-c commit 7b3bfb3ec4b, has a link to the thread. After a bit of
digging, this got merged into dmitry's next tree (see
https://patchwork.kernel.org/patch/9742/ 
<https://patchwork.kernel.org/patch/9742/>) but never ended up in master.
My email archives trail out from there and I'm not sure what exactly
happens, google isn't forthcoming either.

Looks like we simply pushed the xk-c patch too early, assumign that 
-next
will end up in master. But anyway, we've survived 8 years without it, 
so I
think we're good :)


Thank you for the digging I've updated the commit message to reflect
this info and correct the grammar errors Bastien pointed out. I've
tried to push this, but it just hangs ? Maybe because I don't have
write access to xkeyboard-config (it is sitting outside of the
xorg tree after all) ?

Anyways I'll send a v2 with the improved commit message, if you can push
that, that would be great.

Regards,

Hans



___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH v2 xkeyboard-config] Add evdev mappings for KEY_SOUND, KEY_UWB, KEY_WWAN and KEY_RFKILL

[Hans de Goede]

KEY_SOUND, KEY_WWAN and KEY_RFKILL are used on actual devices and
current xproto master defines keysyms for these. Add mappings for
these keys.

The XF86UWB keysm has existed for quite a while now, as has the KEY_UWB
evdev code for it, so let's adds a mapping for that too.

Note that this replaces the I255 to XF86Hibernate mapping in
xkb_symbols "evdev", At one point in time the kernel input next tree
had #define KEY_HIBERNATE 247 on which this mapping was based:
https://patchwork.kernel.org/patch/9742/
but that never got merged in to the mainline kernel.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100970
Cc: Bastien Nocera <had...@hadess.net>
Cc: Benjamin Berg <bb...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net>
---
Changes in v2:
-Improve commit message
---
 keycodes/evdev | 2 ++
 symbols/inet   | 7 ---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/keycodes/evdev b/keycodes/evdev
index 624ac68..51a6e26 100644
--- a/keycodes/evdev
+++ b/keycodes/evdev
@@ -281,6 +281,8 @@ default xkb_keycodes "evdev" {
 = 251;   // #define KEY_BRIGHTNESS_CYCLE243
 = 252;   // #define KEY_BRIGHTNESS_ZERO 244
 = 253;   // #define KEY_DISPLAY_OFF 245
+= 254;   // #define KEY_WWAN246
+= 255;   // #define KEY_RFKILL  247
 
// Fake keycodes for virtual keys
 =   92;
diff --git a/symbols/inet b/symbols/inet
index 5403e77..1e06aa9 100644
--- a/symbols/inet
+++ b/symbols/inet
@@ -184,7 +184,7 @@ xkb_symbols "evdev" {
 key{  [ Print ]   };
 //  key{  [ ]   }; // KEY_HP
 key{  [ XF86WebCam]   };
-//  key{  [ ]   }; // KEY_SOUND
+key{  [ XF86AudioPreset   ]   };
 //  key{  [ ]   }; // KEY_QUESTION
 key{  [ XF86Mail  ]   };
 key{  [ XF86Messenger ]   }; // KEY_CHAT
@@ -210,13 +210,14 @@ xkb_symbols "evdev" {
 key{  [ XF86Battery   ]   };
 key{  [ XF86Bluetooth ]   };
 key{  [ XF86WLAN  ]   };
+key{  [ XF86UWB   ]   };
 //  key{  [ ]   }; // KEY_VIDEO_NEXT  --  drive next video 
source
 //  key{  [ ]   }; // KEY_VIDEO_PREV  --  drive previous 
video source
 //  key{  [ ]   }; // KEY_BRIGHTNESS_CYCLE  --  bright up, 
max++ == min
 //  key{  [ ]   }; // KEY_BRIGHTNESS_ZERO  --  brightness off
 //  key{  [ ]   }; // KEY_DISPLAY_OFF  --  turn off display
-//  key{  [ ]   }; // KEY_WIMAX
-key{  [ XF86Hibernate ]   }; // KEY_HIBERNATE
+key{  [ XF86WWAN  ]   }; // KEY_WWAN
+key{  [ XF86RFKill]   }; // KEY_RFKILL
 
 key{  [ XF86Tools ]   };
 key{  [ XF86Launch5   ]   };
-- 
2.12.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xkeyboard-config] Add evdev mappings for KEY_SOUND, KEY_UWB, KEY_WWAN and KEY_RFKILL

[Hans de Goede]

Hi,

On 15-05-17 03:06, Peter Hutterer wrote:

adding svu to CC:

On Fri, May 12, 2017 at 04:18:42PM +0200, Hans de Goede wrote:

KEY_SOUND, KEY_WWAN and KEY_RFKILL are used on actual devices and
current xproto master defines keysyms for these. Add mappings for
these keys.

The XF86UWB keysm exists for quite a while now, as does the KEY_SOUND
evdev code for it, so lets adds a mapping for that too.

Note that this replaces the I255 to XF86Hibernate mapping in
xkb_symbols "evdev", I don't know where that came from, but it is
wrong, I255 was until now not defined in keycodes/evdev and
KEY_HIBERNATE is not defined in linux/input-event-codes.h .


see xk-c commit 7b3bfb3ec4b, has a link to the thread. After a bit of
digging, this got merged into dmitry's next tree (see
https://patchwork.kernel.org/patch/9742/) but never ended up in master.
My email archives trail out from there and I'm not sure what exactly
happens, google isn't forthcoming either.

Looks like we simply pushed the xk-c patch too early, assumign that -next
will end up in master. But anyway, we've survived 8 years without it, so I
think we're good :)


Thank you for the digging I've updated the commit message to reflect
this info and correct the grammar errors Bastien pointed out. I've
tried to push this, but it just hangs ? Maybe because I don't have
write access to xkeyboard-config (it is sitting outside of the
xorg tree after all) ?

Anyways I'll send a v2 with the improved commit message, if you can push
that, that would be great.

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xkeyboard-config] Add evdev mappings for KEY_SOUND, KEY_UWB, KEY_WWAN and KEY_RFKILL

[Hans de Goede]

Hi,

On 15-05-17 08:50, Bastien Nocera wrote:

Hey,

Comments inline.

On Mon, 2017-05-15 at 08:36 +0200, Hans de Goede wrote:

KEY_SOUND, KEY_WWAN and KEY_RFKILL are used on actual devices and
current xproto master defines keysyms for these. Add mappings for
these keys.

The XF86UWB keysm exists for quite a while now, as does the KEY_SOUND


"keysym has existed"


evdev code for it, so lets adds a mapping for that too.


"let's add"


Thank you both fixed.




Note that this replaces the I255 to XF86Hibernate mapping in
xkb_symbols "evdev", I don't know where that came from, but it is
wrong, I255 was until now not defined in keycodes/evdev and
KEY_HIBERNATE is not defined in linux/input-event-codes.h .

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100970
Cc: Bastien Nocera <had...@hadess.net>
Cc: Benjamin Berg <bb...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net>
---
  keycodes/evdev | 2 ++
  symbols/inet   | 7 ---
  2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/keycodes/evdev b/keycodes/evdev
index 624ac68..51a6e26 100644
--- a/keycodes/evdev
+++ b/keycodes/evdev
@@ -281,6 +281,8 @@ default xkb_keycodes "evdev" {
 = 251; // #define KEY_BRIGHTNESS_CYCLE243
 = 252; // #define KEY_BRIGHTNESS_ZERO 244
 = 253; // #define KEY_DISPLAY_OFF 245
+= 254; // #define KEY_WWAN246
+= 255; // #define KEY_RFKILL  247
  
  	// Fake keycodes for virtual keys

 =   92;
diff --git a/symbols/inet b/symbols/inet
index 5403e77..1e06aa9 100644
--- a/symbols/inet
+++ b/symbols/inet
@@ -184,7 +184,7 @@ xkb_symbols "evdev" {
  key{  [ Print ]   };
  //  key{  [ ]   }; // KEY_HP
  key{  [ XF86WebCam]   };
-//  key{  [ ]   }; // KEY_SOUND
+key{  [ XF86AudioPreset   ]   };


This is an existing keysym, but what does it correspond to?


From XF86keysym.h :

#define XF86XK_AudioPreset 0x1008FFB6 /* Select equalizer preset, e.g. 
theatre-mode */

I221 corresponds to KEY_SOUND which gets used in the kernel in
the following ways:

drivers/platform/x86/peaq-wmi.c:
Used for the Dolby button which under windows toggles through 
theater/game/speach/music mode.

drivers/media/rc/keymaps/rc-kworld-pc150u.c
73: { 0x48, KEY_SOUND}, /* switch theater mode */

drivers/media/rc/keymaps/rc-cec.c
67: { 0x33, KEY_SOUND }, /* CEC Spec: Sound Select */

drivers/media/rc/keymaps/rc-encore-enltv.c
74: { 0x4d, KEY_SOUND },/* DVD sound */





  //  key{  [ ]   }; // KEY_QUESTION
  key{  [ XF86Mail  ]   };
  key{  [ XF86Messenger ]   }; //
KEY_CHAT
@@ -210,13 +210,14 @@ xkb_symbols "evdev" {
  key{  [ XF86Battery   ]   };
  key{  [ XF86Bluetooth ]   };
  key{  [ XF86WLAN  ]   };
+key{  [ XF86UWB   ]   };


Feels weird using up a slot for a key that toggles an obsolete wireless
protocol...


The keycodes such as I247 are hardcoded to EV_FOO keycodes (with an
offset of 8) and the kernel has had:

#define KEY_UWB 239

For ages, so this is not taking a slot, it is merely adding a
mapping since both the keycode and the keysym have existed for
quite a while now.




  //  key{  [ ]   }; // KEY_VIDEO_NEXT  --  drive
next video source
  //  key{  [ ]   }; // KEY_VIDEO_PREV  --  drive
previous video source
  //  key{  [ ]   }; // KEY_BRIGHTNESS_CYCLE
--  bright up, max++ == min
  //  key{  [ ]   }; // KEY_BRIGHTNESS_ZERO
--  brightness off
  //  key{  [ ]   }; // KEY_DISPLAY_OFF  --  turn
off display
-//  key{  [ ]   }; // KEY_WIMAX
-key{  [ XF86Hibernate ]   }; //
KEY_HIBERNATE
+key{  [ XF86WWAN  ]   }; //
KEY_WWAN
+key{  [ XF86RFKill]   }; //
KEY_RFKILL
  
  key{  [ XF86Tools ]   };

  key{  [ XF86Launch5   ]   };


Cheers



Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xkeyboard-config] Add evdev mappings for KEY_SOUND, KEY_UWB, KEY_WWAN and KEY_RFKILL

[Hans de Goede]

KEY_SOUND, KEY_WWAN and KEY_RFKILL are used on actual devices and
current xproto master defines keysyms for these. Add mappings for
these keys.

The XF86UWB keysm exists for quite a while now, as does the KEY_SOUND
evdev code for it, so lets adds a mapping for that too.

Note that this replaces the I255 to XF86Hibernate mapping in
xkb_symbols "evdev", I don't know where that came from, but it is
wrong, I255 was until now not defined in keycodes/evdev and
KEY_HIBERNATE is not defined in linux/input-event-codes.h .

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100970
Cc: Bastien Nocera <had...@hadess.net>
Cc: Benjamin Berg <bb...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net>
---
 keycodes/evdev | 2 ++
 symbols/inet   | 7 ---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/keycodes/evdev b/keycodes/evdev
index 624ac68..51a6e26 100644
--- a/keycodes/evdev
+++ b/keycodes/evdev
@@ -281,6 +281,8 @@ default xkb_keycodes "evdev" {
 = 251;   // #define KEY_BRIGHTNESS_CYCLE243
 = 252;   // #define KEY_BRIGHTNESS_ZERO 244
 = 253;   // #define KEY_DISPLAY_OFF 245
+= 254;   // #define KEY_WWAN246
+= 255;   // #define KEY_RFKILL  247
 
// Fake keycodes for virtual keys
 =   92;
diff --git a/symbols/inet b/symbols/inet
index 5403e77..1e06aa9 100644
--- a/symbols/inet
+++ b/symbols/inet
@@ -184,7 +184,7 @@ xkb_symbols "evdev" {
 key{  [ Print ]   };
 //  key{  [ ]   }; // KEY_HP
 key{  [ XF86WebCam]   };
-//  key{  [ ]   }; // KEY_SOUND
+key{  [ XF86AudioPreset   ]   };
 //  key{  [ ]   }; // KEY_QUESTION
 key{  [ XF86Mail  ]   };
 key{  [ XF86Messenger ]   }; // KEY_CHAT
@@ -210,13 +210,14 @@ xkb_symbols "evdev" {
 key{  [ XF86Battery   ]   };
 key{  [ XF86Bluetooth ]   };
 key{  [ XF86WLAN  ]   };
+key{  [ XF86UWB   ]   };
 //  key{  [ ]   }; // KEY_VIDEO_NEXT  --  drive next video 
source
 //  key{  [ ]   }; // KEY_VIDEO_PREV  --  drive previous 
video source
 //  key{  [ ]   }; // KEY_BRIGHTNESS_CYCLE  --  bright up, 
max++ == min
 //  key{  [ ]   }; // KEY_BRIGHTNESS_ZERO  --  brightness off
 //  key{  [ ]   }; // KEY_DISPLAY_OFF  --  turn off display
-//  key{  [ ]   }; // KEY_WIMAX
-key{  [ XF86Hibernate ]   }; // KEY_HIBERNATE
+key{  [ XF86WWAN  ]   }; // KEY_WWAN
+key{  [ XF86RFKill]   }; // KEY_RFKILL
 
 key{  [ XF86Tools ]   };
 key{  [ XF86Launch5   ]   };
-- 
2.12.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH resend xkeyboard-config] Add evdev mappings for KEY_SOUND, KEY_UWB, KEY_WWAN and KEY_RFKILL

[Hans de Goede]

Hi,

This is a resend with the Cc to Bastien fixed up at his request as he
has some remarks.

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xkeyboard-config] Add evdev mappings for KEY_SOUND, KEY_UWB, KEY_WWAN and KEY_RFKILL

[Hans de Goede]

KEY_SOUND, KEY_WWAN and KEY_RFKILL are used on actual devices and
current xproto master defines keysyms for these. Add mappings for
these keys.

The XF86UWB keysm exists for quite a while now, as does the KEY_SOUND
evdev code for it, so lets adds a mapping for that too.

Note that this replaces the I255 to XF86Hibernate mapping in
xkb_symbols "evdev", I don't know where that came from, but it is
wrong, I255 was until now not defined in keycodes/evdev and
KEY_HIBERNATE is not defined in linux/input-event-codes.h .

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100970
Cc: Bastien Nocera <bnoc...@redhat.com>
Cc: Benjamin Berg <bb...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 keycodes/evdev | 2 ++
 symbols/inet   | 7 ---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/keycodes/evdev b/keycodes/evdev
index 624ac68..51a6e26 100644
--- a/keycodes/evdev
+++ b/keycodes/evdev
@@ -281,6 +281,8 @@ default xkb_keycodes "evdev" {
 = 251;   // #define KEY_BRIGHTNESS_CYCLE243
 = 252;   // #define KEY_BRIGHTNESS_ZERO 244
 = 253;   // #define KEY_DISPLAY_OFF 245
+= 254;   // #define KEY_WWAN246
+= 255;   // #define KEY_RFKILL  247
 
// Fake keycodes for virtual keys
 =   92;
diff --git a/symbols/inet b/symbols/inet
index 5403e77..1e06aa9 100644
--- a/symbols/inet
+++ b/symbols/inet
@@ -184,7 +184,7 @@ xkb_symbols "evdev" {
 key{  [ Print ]   };
 //  key{  [ ]   }; // KEY_HP
 key{  [ XF86WebCam]   };
-//  key{  [ ]   }; // KEY_SOUND
+key{  [ XF86AudioPreset   ]   };
 //  key{  [ ]   }; // KEY_QUESTION
 key{  [ XF86Mail  ]   };
 key{  [ XF86Messenger ]   }; // KEY_CHAT
@@ -210,13 +210,14 @@ xkb_symbols "evdev" {
 key{  [ XF86Battery   ]   };
 key{  [ XF86Bluetooth ]   };
 key{  [ XF86WLAN  ]   };
+key{  [ XF86UWB   ]   };
 //  key{  [ ]   }; // KEY_VIDEO_NEXT  --  drive next video 
source
 //  key{  [ ]   }; // KEY_VIDEO_PREV  --  drive previous 
video source
 //  key{  [ ]   }; // KEY_BRIGHTNESS_CYCLE  --  bright up, 
max++ == min
 //  key{  [ ]   }; // KEY_BRIGHTNESS_ZERO  --  brightness off
 //  key{  [ ]   }; // KEY_DISPLAY_OFF  --  turn off display
-//  key{  [ ]   }; // KEY_WIMAX
-key{  [ XF86Hibernate ]   }; // KEY_HIBERNATE
+key{  [ XF86WWAN  ]   }; // KEY_WWAN
+key{  [ XF86RFKill]   }; // KEY_RFKILL
 
 key{  [ XF86Tools ]   };
 key{  [ XF86Launch5   ]   };
-- 
2.12.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xproto v2 1/2] Add XF86XK_WWAN and XF86XK_Rfkill

[Hans de Goede]

Hi,

On 12-05-17 01:02, Peter Hutterer wrote:

On Wed, May 10, 2017 at 05:53:53PM +0200, Hans de Goede wrote:

Add Keysyms corresponding to the evdev WWAN and RFKILL keys, we already
have Keysyms for WLAN and UWB from linux/input-event-codes.h:

  #define KEY_WLAN238
  #define KEY_UWB 239

But not for the WWAN and generic RFKILL keys:

  #define KEY_WWAN246 /* Wireless WAN (LTE, UMTS, GSM, 
etc.) */
  #define KEY_WIMAX   KEY_WWAN
  #define KEY_RFKILL  247 /* Key that controls all radios */

This commits add Keysyms for these so that we can add proper mappings for
them to xkb.

Cc: Bastien Nocera <bnoc...@redhat.com>
Cc: Benjamin Berg <bb...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2:
-Define XF86XK_WWAN instead of XF86XK_WiMAX, as KEY_WIMAX is an alias
  for KEY_WWAN, I intended to do this for v1 before submitting, but I forgot



  XF86keysym.h | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/XF86keysym.h b/XF86keysym.h
index 89d40b8..b04d45e 100644
--- a/XF86keysym.h
+++ b/XF86keysym.h
@@ -199,6 +199,9 @@
  
  #define XF86XK_Keyboard		0x1008FFB3   /* User defined keyboard related action */
  
+#define XF86XK_WWAN		0x1008FFB4   /* Toggle WWAN (LTE, UMTS, etc.) radio */

+#define XF86XK_Rfkill  0x1008FFB5   /* Toggle radios on/off */


Only comment I have: Rfkill seems odd because RF is an abbreviation, so I'd
go with RFKill. See also the capitalisation on Switch_VT, rather than Switch_Vt.

With that change, Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net>
or without it too, if you think Rfkill is better :)


Thank you, both patches pushed with the suggested RFKill rename.

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xproto v2 1/2] Add XF86XK_WWAN and XF86XK_Rfkill

[Hans de Goede]

Add Keysyms corresponding to the evdev WWAN and RFKILL keys, we already
have Keysyms for WLAN and UWB from linux/input-event-codes.h:

 #define KEY_WLAN238
 #define KEY_UWB 239

But not for the WWAN and generic RFKILL keys:

 #define KEY_WWAN246 /* Wireless WAN (LTE, UMTS, GSM, etc.) 
*/
 #define KEY_WIMAX   KEY_WWAN
 #define KEY_RFKILL  247 /* Key that controls all radios */

This commits add Keysyms for these so that we can add proper mappings for
them to xkb.

Cc: Bastien Nocera <bnoc...@redhat.com>
Cc: Benjamin Berg <bb...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2:
-Define XF86XK_WWAN instead of XF86XK_WiMAX, as KEY_WIMAX is an alias
 for KEY_WWAN, I intended to do this for v1 before submitting, but I forgot
---
 XF86keysym.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/XF86keysym.h b/XF86keysym.h
index 89d40b8..b04d45e 100644
--- a/XF86keysym.h
+++ b/XF86keysym.h
@@ -199,6 +199,9 @@
 
 #define XF86XK_Keyboard0x1008FFB3   /* User defined keyboard 
related action */
 
+#define XF86XK_WWAN0x1008FFB4   /* Toggle WWAN (LTE, UMTS, etc.) 
radio */
+#define XF86XK_Rfkill  0x1008FFB5   /* Toggle radios on/off */
+
 /* Keys for special action keys (hot keys) */
 /* Virtual terminals on some operating systems */
 #define XF86XK_Switch_VT_1 0x1008FE01
-- 
2.12.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xproto v2 2/2] Add XF86XK_AudioPreset

[Hans de Goede]

Add XF86XK_AudioPreset keysym, to be used as mapping for evdev's
KEY_SOUND keycode which is generated on some devices by a button which
on windows selects equalizer presets switching between settings such as
e.g. theatre-mode / game-mode / voice-mode.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 XF86keysym.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/XF86keysym.h b/XF86keysym.h
index b04d45e..b76ea55 100644
--- a/XF86keysym.h
+++ b/XF86keysym.h
@@ -202,6 +202,8 @@
 #define XF86XK_WWAN0x1008FFB4   /* Toggle WWAN (LTE, UMTS, etc.) 
radio */
 #define XF86XK_Rfkill  0x1008FFB5   /* Toggle radios on/off */
 
+#define XF86XK_AudioPreset 0x1008FFB6   /* Select equalizer preset, e.g. 
theatre-mode */
+
 /* Keys for special action keys (hot keys) */
 /* Virtual terminals on some operating systems */
 #define XF86XK_Switch_VT_1 0x1008FE01
-- 
2.12.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xproto 2/2] Add XF86XK_AudioPreset

[Hans de Goede]

Add XF86XK_AudioPreset keysym, to be used as mapping for evdev's
KEY_SOUND keycode which is generated on some devices by a button which
on windows selects equalizer presets switching between settings such as
e.g. theatre-mode / game-mode / voice-mode.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 XF86keysym.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/XF86keysym.h b/XF86keysym.h
index 809f386..c70f45c 100644
--- a/XF86keysym.h
+++ b/XF86keysym.h
@@ -202,6 +202,8 @@
 #define XF86XK_WiMAX   0x1008FFB4   /* Toggle WiMAX radio */
 #define XF86XK_Rfkill  0x1008FFB5   /* Toggle radios on/off */
 
+#define XF86XK_AudioPreset 0x1008FFB6   /* Select equalizer preset, e.g. 
theatre-mode */
+
 /* Keys for special action keys (hot keys) */
 /* Virtual terminals on some operating systems */
 #define XF86XK_Switch_VT_1 0x1008FE01
-- 
2.12.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xproto 1/2] Add XF86XK_WiMAX and XF86XK_Rfkill

[Hans de Goede]

Add Keysyms corresponding to the evdev WIMAX and RFKILL keys, we already
have Keysyms for WLAN and UWB from linux/input-event-codes.h:

 #define KEY_WLAN238
 #define KEY_UWB 239

But not for the WWAN and generic RFKILL keys:

 #define KEY_WWAN246 /* Wireless WAN (LTE, UMTS, GSM, etc.) 
*/
 #define KEY_WIMAX   KEY_WWAN
 #define KEY_RFKILL  247 /* Key that controls all radios */

This commits add Keysyms for these so that we can add proper mappings for
them to xkb.

Cc: Bastien Nocera <bnoc...@redhat.com>
Cc: Benjamin Berg <bb...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 XF86keysym.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/XF86keysym.h b/XF86keysym.h
index 89d40b8..809f386 100644
--- a/XF86keysym.h
+++ b/XF86keysym.h
@@ -199,6 +199,9 @@
 
 #define XF86XK_Keyboard0x1008FFB3   /* User defined keyboard 
related action */
 
+#define XF86XK_WiMAX   0x1008FFB4   /* Toggle WiMAX radio */
+#define XF86XK_Rfkill  0x1008FFB5   /* Toggle radios on/off */
+
 /* Keys for special action keys (hot keys) */
 /* Virtual terminals on some operating systems */
 #define XF86XK_Switch_VT_1 0x1008FE01
-- 
2.12.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [xproto] Add XF86XK_Keyboard

[Hans de Goede]

Hi,

On 19-04-17 15:26, Christian Kellner wrote:

From: Christian Kellner 

The 2017 Thinkpad models have a new hotkey with a keyboard symbols
on it, which is mapped to KEY_KEYBOARD in the kernel.

Signed-off-by: Christian Kellner 


Since Peter said he was going to push this patch and I'm preparing
a patch adding some more symbols which would conflict I've gone ahead
and pushed this patch.

Regards,

Hans



---
  XF86keysym.h | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/XF86keysym.h b/XF86keysym.h
index 8b5646e..89d40b8 100644
--- a/XF86keysym.h
+++ b/XF86keysym.h
@@ -197,6 +197,8 @@
  
  #define XF86XK_AudioMicMute	0x1008FFB2   /* Mute the Mic from the system */
  
+#define XF86XK_Keyboard		0x1008FFB3   /* User defined keyboard related action */

+
  /* Keys for special action keys (hot keys) */
  /* Virtual terminals on some operating systems */
  #define XF86XK_Switch_VT_10x1008FE01


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver resend] xf86: dri2: Use va_gl as vdpau_driver for Intel i965 GPUs

[Hans de Goede]

The modesetting driver (which now often is used with Intel GPUs),
relies on dri2_probe_driver_name() to get the dri and vdpau driver
names, before this commit it would always assign the same name to
the 2 names. But the vdpau driver for i965 GPUs should be va_gl
(i915 does not support vdpau at all).

This commit modifies the used lookup table and dri2_probe_driver_name()
to set the vdpau_driver to va_gl for i965 GPUs, it leaves the 2
names the same for all other GPUs.

Note this commit adds a FIXME comment for a memory leak in
dri2_probe_driver_name(), that leak was already present and fixing
it falls outside of the scope of this commit.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1413733
Cc: kwiz...@gmail.com
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 hw/xfree86/dri2/dri2.c  | 31 +++--
 hw/xfree86/dri2/pci_ids/pci_id_driver_map.h | 21 +--
 2 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index f9f9859e1..53153383c 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -1437,14 +1437,18 @@ get_prime_id(void)
 
 #include "pci_ids/pci_id_driver_map.h"
 
-static char *
-dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr info)
+static void
+dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr info,
+   const char **dri_driver_ret,
+   const char **vdpau_driver_ret)
 {
 #ifdef WITH_LIBDRM
 int i, j;
-char *driver = NULL;
 drmDevicePtr dev;
 
+*dri_driver_ret = NULL;
+*vdpau_driver_ret = NULL;
+
 /* For non-PCI devices and drmGetDevice fail, just assume that
  * the 3D driver is named the same as the kernel driver. This is
  * currently true for vc4 and msm (freedreno).
@@ -1456,12 +1460,14 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr 
info)
 xf86DrvMsg(pScreen->myNum, X_ERROR,
"[DRI2] Couldn't drmGetVersion() on non-PCI device, "
"no driver name found.\n");
-return NULL;
+return;
 }
 
-driver = strndup(version->name, version->name_len);
+/* FIXME this gets leaked */
+*dri_driver_ret = strndup(version->name, version->name_len);
+*vdpau_driver_ret = *dri_driver_ret;
 drmFreeVersion(version);
-return driver;
+return;
 }
 
 for (i = 0; driver_map[i].driver; i++) {
@@ -1469,13 +1475,15 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr 
info)
 continue;
 
 if (driver_map[i].num_chips_ids == -1) {
- driver = strdup(driver_map[i].driver);
+ *dri_driver_ret = driver_map[i].driver;
+ *vdpau_driver_ret = driver_map[i].vdpau_driver;
  goto out;
 }
 
 for (j = 0; j < driver_map[i].num_chips_ids; j++) {
 if (driver_map[i].chip_ids[j] == dev->deviceinfo.pci->device_id) {
-driver = strdup(driver_map[i].driver);
+*dri_driver_ret = driver_map[i].driver;
+*vdpau_driver_ret = driver_map[i].vdpau_driver;
 goto out;
 }
 }
@@ -1487,9 +1495,9 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr 
info)
dev->deviceinfo.pci->vendor_id, dev->deviceinfo.pci->device_id);
 out:
 drmFreeDevice();
-return driver;
 #else
-return NULL;
+*dri_driver_ret = NULL;
+*vdpau_driver_ret = NULL;
 #endif
 }
 
@@ -1611,7 +1619,8 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
 if (info->driverName) {
 ds->driverNames[0] = info->driverName;
 } else {
-ds->driverNames[0] = ds->driverNames[1] = 
dri2_probe_driver_name(pScreen, info);
+dri2_probe_driver_name(pScreen, info,
+   >driverNames[0], >driverNames[1]);
 if (!ds->driverNames[0])
 return FALSE;
 }
diff --git a/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h 
b/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h
index da7ea1c1e..7036d1003 100644
--- a/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h
+++ b/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h
@@ -66,21 +66,22 @@ static const int vmwgfx_chip_ids[] = {
 static const struct {
int vendor_id;
const char *driver;
+   const char *vdpau_driver;
const int *chip_ids;
int num_chips_ids;
 } driver_map[] = {
-   { 0x8086, "i915", i915_chip_ids, ARRAY_SIZE(i915_chip_ids) },
-   { 0x8086, "i965", i965_chip_ids, ARRAY_SIZE(i965_chip_ids) },
+   { 0x8086, "i915", "i915", i915_chip_ids, ARRAY_SIZE(i915_chip_ids) },
+   { 0x8086, "i965", "va_gl", i965_chip_ids, ARRAY_SIZE(i965_chip_ids) },
 #ifndef DRIVER_MAP_GALLIUM_ONLY
-   { 0x1002, "radeon", r10

[PATCH xserver] xf86: dri2: Use va_gl as vdpau_driver for Intel i965 GPUs

[Hans de Goede]

The modesetting driver (which now often is used with Intel GPUs),
relies on dri2_probe_driver_name() to get the dri and vdpau driver
names, before this commit it would always assign the same name to
the 2 names. But the vdpau driver for i965 GPUs should be va_gl
(i915 does not support vdpau at all).

This commit modifies the used lookup table and dri2_probe_driver_name()
to set the vdpau_driver to va_gl for i965 GPUs, it leaves the 2
names the same for all other GPUs.

Note this commit adds a FIXME comment for a memory leak in
dri2_probe_driver_name(), that leak was already present and fixing
it falls outside of the scope of this commit.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1413733
Cc: kwiz...@gmail.com
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 hw/xfree86/dri2/dri2.c  | 31 +++--
 hw/xfree86/dri2/pci_ids/pci_id_driver_map.h | 21 +--
 2 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index f9f9859..5315338 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -1437,14 +1437,18 @@ get_prime_id(void)
 
 #include "pci_ids/pci_id_driver_map.h"
 
-static char *
-dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr info)
+static void
+dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr info,
+   const char **dri_driver_ret,
+   const char **vdpau_driver_ret)
 {
 #ifdef WITH_LIBDRM
 int i, j;
-char *driver = NULL;
 drmDevicePtr dev;
 
+*dri_driver_ret = NULL;
+*vdpau_driver_ret = NULL;
+
 /* For non-PCI devices and drmGetDevice fail, just assume that
  * the 3D driver is named the same as the kernel driver. This is
  * currently true for vc4 and msm (freedreno).
@@ -1456,12 +1460,14 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr 
info)
 xf86DrvMsg(pScreen->myNum, X_ERROR,
"[DRI2] Couldn't drmGetVersion() on non-PCI device, "
"no driver name found.\n");
-return NULL;
+return;
 }
 
-driver = strndup(version->name, version->name_len);
+/* FIXME this gets leaked */
+*dri_driver_ret = strndup(version->name, version->name_len);
+*vdpau_driver_ret = *dri_driver_ret;
 drmFreeVersion(version);
-return driver;
+return;
 }
 
 for (i = 0; driver_map[i].driver; i++) {
@@ -1469,13 +1475,15 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr 
info)
 continue;
 
 if (driver_map[i].num_chips_ids == -1) {
- driver = strdup(driver_map[i].driver);
+ *dri_driver_ret = driver_map[i].driver;
+ *vdpau_driver_ret = driver_map[i].vdpau_driver;
  goto out;
 }
 
 for (j = 0; j < driver_map[i].num_chips_ids; j++) {
 if (driver_map[i].chip_ids[j] == dev->deviceinfo.pci->device_id) {
-driver = strdup(driver_map[i].driver);
+*dri_driver_ret = driver_map[i].driver;
+*vdpau_driver_ret = driver_map[i].vdpau_driver;
 goto out;
 }
 }
@@ -1487,9 +1495,9 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr 
info)
dev->deviceinfo.pci->vendor_id, dev->deviceinfo.pci->device_id);
 out:
 drmFreeDevice();
-return driver;
 #else
-return NULL;
+*dri_driver_ret = NULL;
+*vdpau_driver_ret = NULL;
 #endif
 }
 
@@ -1611,7 +1619,8 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
 if (info->driverName) {
 ds->driverNames[0] = info->driverName;
 } else {
-ds->driverNames[0] = ds->driverNames[1] = 
dri2_probe_driver_name(pScreen, info);
+dri2_probe_driver_name(pScreen, info,
+   >driverNames[0], >driverNames[1]);
 if (!ds->driverNames[0])
 return FALSE;
 }
diff --git a/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h 
b/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h
index da7ea1c..7036d10 100644
--- a/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h
+++ b/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h
@@ -66,21 +66,22 @@ static const int vmwgfx_chip_ids[] = {
 static const struct {
int vendor_id;
const char *driver;
+   const char *vdpau_driver;
const int *chip_ids;
int num_chips_ids;
 } driver_map[] = {
-   { 0x8086, "i915", i915_chip_ids, ARRAY_SIZE(i915_chip_ids) },
-   { 0x8086, "i965", i965_chip_ids, ARRAY_SIZE(i965_chip_ids) },
+   { 0x8086, "i915", "i915", i915_chip_ids, ARRAY_SIZE(i915_chip_ids) },
+   { 0x8086, "i965", "va_gl", i965_chip_ids, ARRAY_SIZE(i965_chip_ids) },
 #ifndef DRIVER_MAP_GALLIUM_ONLY
-   { 0x1002, "radeon", r100_chip_ids, AR

Re: [xserver, 2/2] xf86RandR12: Fix XF86VidModeSetGamma triggering a BadImplementation error

[Hans de Goede]

Hi,

On 20-09-16 03:58, Michel Dänzer wrote:

On 17/09/16 07:00 PM, Hans De Goede wrote:

Commit b4e46c0444bb ("xfree86: Hook up colormaps and RandR 1.2 gamma code")
dropped the providing of a pScrn->ChangeGamma callback from the xf86RandR12
code. Leaving pScrn->ChangeGamma NULL in most cases.

This triggers the BadImplementation error in xf86ChangeGamma() :

if (pScrn->ChangeGamma)
return (*pScrn->ChangeGamma) (pScrn, gamma);

return BadImplementation;

Which causes X-apps using XF86VidModeSetGamma to crash with a
X protocol error.

This commit fixes this by re-introducing the xf86RandR12ChangeGamma
helper removed by the commit and adjusting it to work with the new
combined palette / gamma code.

Fixes: b4e46c0444bb ("xfree86: Hook up colormaps and RandR 1.2 gamma code")


I suspect you really want to fix the modesetting driver to call
xf86HandleColormaps, so it can actually benefit from that change and
e.g. gamma sliders in games start working.


Ok, so going through my TODO list I started looking at this,
amdgpu does:

if (xf86_config->num_crtc) {
xf86DrvMsgVerb(pScrn->scrnIndex, X_INFO, AMDGPU_LOGLEVEL_DEBUG,
   "Initializing kms color map\n");
if (!miCreateDefColormap(pScreen))
return FALSE;
/* all amdgpus support 10 bit CLUTs */
if (!xf86HandleColormaps(pScreen, 256, 10,
 NULL, NULL,
 CMAP_PALETTED_TRUECOLOR
 | CMAP_RELOAD_ON_MODE_SWITCH))
return FALSE;
}

Which seems like it should mostly work for modesetting to,
except for the all "amdgpus support 10 bit CLUTs" part,
any ideas what to put there for modesetting, or how to
query this from the kernel driver ?

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] modesetting: Honor xorg.conf for 16bpp

[Hans de Goede]

Hi,

On 21-03-17 20:57, Adam Jackson wrote:

The 32->24 support patch messed this up.

Bugzilla: https://bugs.freedesktop.org/100246
Bugzilla: https://bugs.freedesktop.org/100295
Signed-off-by: Adam Jackson <a...@redhat.com>


LGTM:

Acked-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 hw/xfree86/drivers/modesetting/driver.c  | 4 
 hw/xfree86/drivers/modesetting/drmmode_display.c | 6 ++
 2 files changed, 10 insertions(+)

diff --git a/hw/xfree86/drivers/modesetting/driver.c 
b/hw/xfree86/drivers/modesetting/driver.c
index d7030e5..762b398 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -922,6 +922,8 @@ PreInit(ScrnInfoPtr pScrn, int flags)
 if (!check_outputs(ms->fd, _count))
 return FALSE;

+defaultdepth = pScrn->confScreen->defaultdepth;
+defaultbpp = pScrn->confScreen->defaultbpp;
 drmmode_get_default_bpp(pScrn, >drmmode, , );
 if (defaultdepth == 24 && defaultbpp == 24) {
 ms->drmmode.force_24_32 = TRUE;
@@ -949,6 +951,8 @@ PreInit(ScrnInfoPtr pScrn, int flags)
pScrn->depth);
 return FALSE;
 }
+if (ms->drmmode.kbpp == 0)
+ms->drmmode.kbpp = pScrn->bitsPerPixel;
 xf86PrintDepthBpp(pScrn);

 /* Process the options */
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index c1e489e..e6158ab 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -2057,6 +2057,8 @@ drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, 
int cpp)
 xf86CrtcConfigInit(pScrn, _xf86crtc_config_funcs);

 drmmode->scrn = pScrn;
+if (drmmode->force_24_32 && cpp == 4)
+cpp = 3;
 drmmode->cpp = cpp;
 mode_res = drmModeGetResources(drmmode->fd);
 if (!mode_res)
@@ -2488,6 +2490,10 @@ drmmode_get_default_bpp(ScrnInfoPtr pScrn, drmmode_ptr 
drmmode, int *depth,
 uint32_t fb_id;
 int ret;

+/* if we've been configured in xorg.conf, trust it */
+if (*depth || *bpp)
+return;
+
 /* 16 is fine */
 ret = drmGetCap(drmmode->fd, DRM_CAP_DUMB_PREFERRED_DEPTH, );
 if (!ret && (value == 16 || value == 8)) {


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xdriinfo] Fix xdriinfo not working with glvnd

[Hans de Goede]

For glx calls to work on libglvnd as glx provider we must first call
glXGetClientString. This also means that we can no longer take the
shortcut to not open the Display when a driver name is past to options.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 xdriinfo.c | 29 -
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/xdriinfo.c b/xdriinfo.c
index c23cfa1..c7e7482 100644
--- a/xdriinfo.c
+++ b/xdriinfo.c
@@ -112,23 +112,9 @@ int main (int argc, char *argv[]) {
return 1;
}
 }
-  /* if the argument to the options command is a driver name, we can handle
-   * it without opening an X connection */
-if (func == OPTIONS && screenNum == -1) {
-   const char *options = (*GetDriverConfig) (funcArg);
-   if (!options) {
-   fprintf (stderr,
-"Driver \"%s\" is not installed or does not support 
configuration.\n",
-funcArg);
-   return 1;
-   }
-   printf ("%s", options);
-   if (isatty (STDOUT_FILENO))
-   printf ("\n");
-   return 0;
-} 
+
   /* driver command needs a valid screen number */
-else if (func == DRIVER && screenNum == -1) {
+if (func == DRIVER && screenNum == -1) {
fprintf (stderr, "Invalid screen number \"%s\".\n", funcArg);
return 1;
 }
@@ -146,6 +132,9 @@ int main (int argc, char *argv[]) {
return 1;
 }
 
+   /* Call glXGetClientString to load vendor libs on glvnd enabled systems */
+glXGetClientString (dpy, GLX_EXTENSIONS);
+
 switch (func) {
   case NSCREENS:
printf ("%d", nScreens);
@@ -165,7 +154,13 @@ int main (int argc, char *argv[]) {
  break;
   }
   case OPTIONS: {
- const char *name = (*GetScreenDriver) (dpy, screenNum), *options;
+ const char *name, *options;
+ 
+ if (screenNum == -1) {
+ name = funcArg;
+ } else {
+ name = (*GetScreenDriver) (dpy, screenNum);
+ }
  if (!name) {
  fprintf (stderr, "Screen \"%d\" is not direct rendering 
capable.\n",
   screenNum);
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 1/3] Use timingsafe_memcmp() to compare MIT-MAGIC-COOKIES CVE-2017-2624

[Hans de Goede]

Hi,

On 28-02-17 23:52, Matthieu Herrb wrote:


On Tue, Feb 28, 2017 at 10:41:29PM +, Emil Velikov wrote:

Hi Matthieu,

On 28 February 2017 at 18:18, Matthieu Herrb  wrote:

Provide the function definition for systems that don't have it.

Signed-off-by: Matthieu Herrb 
Reviewed-by: Alan Coopersmith 
---
 configure.ac|  3 ++-
 include/dix-config.h.in |  3 +++
 include/os.h|  5 +
 os/mitauth.c|  2 +-
 os/timingsafe_memcmp.c  | 45 +
 5 files changed, 56 insertions(+), 2 deletions(-)



--- /dev/null
+++ b/os/timingsafe_memcmp.c

Shouldn't we add this new file to Makefile.am somewhere ?


Hi,

No; AC_REPLACE_FUNCS() takes completely care of it.

In os/Makefile.am you have :

libos_la_LIBADD = @SHA1_LIBS@ $(DLOPEN_LIBS) $(LTLIBOBJS)

and LTLIBOBJS is expanded to the list of filenames corresponding to
functions that need to be provided in the AC_REPLACE_FUNC() macro.


What about make dist for making the source tarbals ?

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 4/4] modesetting: cleanup pci device open

[Hans de Goede]

Hi,

On 10-01-17 11:51, Qiang Yu wrote:

Signed-off-by: Qiang Yu <qiang...@amd.com>



Patch looks good to me:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans





---
 hw/xfree86/drivers/modesetting/driver.c | 24 +++-
 hw/xfree86/drivers/modesetting/driver.h |  6 --
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/driver.c 
b/hw/xfree86/drivers/modesetting/driver.c
index a27b327..d7030e5 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -812,7 +812,6 @@ ms_get_drm_master_fd(ScrnInfoPtr pScrn)
 EntityInfoPtr pEnt;
 modesettingPtr ms;
 modesettingEntPtr ms_ent;
-char *BusID = NULL;

 ms = modesettingPTR(pScrn);
 ms_ent = ms_ent_priv(pScrn);
@@ -845,25 +844,24 @@ ms_get_drm_master_fd(ScrnInfoPtr pScrn)
 }
 else
 #endif
+#if XSERVER_LIBPCIACCESS
 if (pEnt->location.type == BUS_PCI) {
-ms->PciInfo = xf86GetPciInfoForEntity(ms->pEnt->index);
-if (ms->PciInfo) {
+char *BusID = NULL;
+struct pci_device *PciInfo;
+
+PciInfo = xf86GetPciInfoForEntity(ms->pEnt->index);
+if (PciInfo) {
 BusID = XNFalloc(64);
 sprintf(BusID, "PCI:%d:%d:%d",
-#if XSERVER_LIBPCIACCESS
-((ms->PciInfo->domain << 8) | ms->PciInfo->bus),
-ms->PciInfo->dev, ms->PciInfo->func
-#else
-((pciConfigPtr) ms->PciInfo->thisCard)->busnum,
-((pciConfigPtr) ms->PciInfo->thisCard)->devnum,
-((pciConfigPtr) ms->PciInfo->thisCard)->funcnum
-#endif
-);
+((PciInfo->domain << 8) | PciInfo->bus),
+PciInfo->dev, PciInfo->func);
 }
 ms->fd = drmOpen(NULL, BusID);
 free(BusID);
 }
-else {
+else
+#endif
+{
 const char *devicename;
 devicename = xf86FindOptionValue(ms->pEnt->device->options, "kmsdev");
 ms->fd = open_hw(devicename);
diff --git a/hw/xfree86/drivers/modesetting/driver.h 
b/hw/xfree86/drivers/modesetting/driver.h
index eee96e5..25e3a54 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -89,12 +89,6 @@ typedef struct _modesettingRec {

 int Chipset;
 EntityInfoPtr pEnt;
-#if XSERVER_LIBPCIACCESS
-struct pci_device *PciInfo;
-#else
-pciVideoPtr PciInfo;
-PCITAG PciTag;
-#endif

 Bool noAccel;
 CloseScreenProcPtr CloseScreen;


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/4] randr: fix xserver crash when xrandr setprovideroutputsource

[Hans de Goede]

Hi,

On 10-01-17 11:51, Qiang Yu wrote:

xrandr --setprovideroutputsource  
Xorg: ../../../xserver/dix/dispatch.c:4018: AttachOutputGPU:
Assertion `new->isGPU' failed.

GPUScreen is not allowed to be sink output.

Signed-off-by: Qiang Yu <qiang...@amd.com>



Patch looks good to me:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans





---
 randr/rrprovider.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/randr/rrprovider.c b/randr/rrprovider.c
index f9df67e..e4bc2bf 100644
--- a/randr/rrprovider.c
+++ b/randr/rrprovider.c
@@ -338,6 +338,9 @@ ProcRRSetProviderOutputSource(ClientPtr client)
 pScreen = provider->pScreen;
 pScrPriv = rrGetScrPriv(pScreen);

+if (!pScreen->isGPU)
+return BadValue;
+
 pScrPriv->rrProviderSetOutputSource(pScreen, provider, source_provider);

 RRInitPrimeSyncProps(pScreen);


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/4] xfree86: fix wrong usage of xf86optionListMerge

[Hans de Goede]

Hi,

On 10-01-17 11:51, Qiang Yu wrote:

Signed-off-by: Qiang Yu <qiang...@amd.com>


Patch looks good to me:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 hw/xfree86/common/xf86Option.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/xfree86/common/xf86Option.c b/hw/xfree86/common/xf86Option.c
index 929724d..06973bc 100644
--- a/hw/xfree86/common/xf86Option.c
+++ b/hw/xfree86/common/xf86Option.c
@@ -87,7 +87,7 @@ xf86CollectOptions(ScrnInfoPtr pScrn, XF86OptionPtr extraOpts)
 if (device && device->options) {
 tmp = xf86optionListDup(device->options);
 if (pScrn->options)
-xf86optionListMerge(pScrn->options, tmp);
+pScrn->options = xf86optionListMerge(pScrn->options, tmp);
 else
 pScrn->options = tmp;
 }


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [tigervnc-devel] PATCH: Add xorg-xserver 1.19 support to tigervnc

[Hans de Goede]

Hi,

On 09-01-17 18:12, Alan Coopersmith wrote:

On 01/ 9/17 07:57 AM, Hans de Goede wrote:

 close(2);
+/* Avoid xserver >= 1.19's epoll-fd becoming fd 2 / stderr only to be
+   replaced by /dev/null by OsInit() because the pollfd is not
+   writable, breaking ospoll_wait(). */
+open("/dev/null", O_WRONLY);


Hopefully no other threads in the X server are opening files, but if any ever
do, it would be more reliable to do:

nullfd = open("/dev/null", O_WRONLY);
dup2(nullfd, 2);
close(nullfd);

and let dup2 atomically close the old stderr and clone nullfd to it.


Yes I agree that would be better, Pierre, can you take care
of merging Alan's improved version ?

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [tigervnc-devel] PATCH: Add xorg-xserver 1.19 support to tigervnc

[Hans de Goede]

Hi,

On 09-01-17 10:00, Pierre Ossman wrote:

On 05/01/17 17:44, ipilc...@redhat.com wrote:

On Wednesday, October 5, 2016 at 4:13:48 AM UTC-5, Pierre Ossman wrote:

Alright. I'll have a look at doing the finishing touches. Thanks for the
patch. :)


FYI, there seems to be an issue with either the patch or v1.19 itself.

  https://bugzilla.redhat.com/show_bug.cgi?id=1408724



Urgh. I have no 1.19 system yet, so hopefully someone else can have a look.


Attached is a patch fixing this :)

Regards,

Hans
>From 372ff9d6754cd1b375836e5d4559061fb7be3496 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdego...@redhat.com>
Date: Mon, 9 Jan 2017 16:03:30 +0100
Subject: [PATCH] Fix -inetd not working with xserver >= 1.19

xserver 1.19's OsInit will create a pollfd, followed by checking if fd 2 /
stderr is writable and if it is not, replacing fd 2 with /dev/null.

Since we close stderr in inetd mode to avoid xserver messages being send
to the client as vnc data, the pollfd becomes fd 2, only to be replaced
by /dev/null since a pollfd is not writable.

This commit fixes this by opening /dev/null directly after the close(2),
avoiding that the pollfd becomes fd 2.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 unix/xserver/hw/vnc/xvnc.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/unix/xserver/hw/vnc/xvnc.c b/unix/xserver/hw/vnc/xvnc.c
index 2f3cd4a..a747654 100644
--- a/unix/xserver/hw/vnc/xvnc.c
+++ b/unix/xserver/hw/vnc/xvnc.c
@@ -575,6 +575,10 @@ ddxProcessArgument(int argc, char *argv[], int i)
 	dup2(0,3);
 	vncInetdSock = 3;
 	close(2);
+	/* Avoid xserver >= 1.19's epoll-fd becoming fd 2 / stderr only to be
+	   replaced by /dev/null by OsInit() because the pollfd is not
+	   writable, breaking ospoll_wait(). */
+	open("/dev/null", O_WRONLY);
 	
 	if (!displaySpecified) {
 	int port = vncGetSocketPort(vncInetdSock);
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [tigervnc-devel] PATCH: Add xorg-xserver 1.19 support to tigervnc

[Hans de Goede]

Hi,

On 09-01-17 10:00, Pierre Ossman wrote:

On 05/01/17 17:44, ipilc...@redhat.com wrote:

On Wednesday, October 5, 2016 at 4:13:48 AM UTC-5, Pierre Ossman wrote:

Alright. I'll have a look at doing the finishing touches. Thanks for the
patch. :)


FYI, there seems to be an issue with either the patch or v1.19 itself.

  https://bugzilla.redhat.com/show_bug.cgi?id=1408724



Urgh. I have no 1.19 system yet, so hopefully someone else can have a look.


I've been debugging this this afternoon, I believe I've identifief
the problem and I'm testing a fix now.

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH v2 xserver 00/11] modesetting: MS_ALL_IN_ONE

[Hans de Goede]

Hi,

On 08-01-17 15:04, Yu, Qiang wrote:


Hi Hans,

Thanks for your info, I give it a quick try:
Section "OutputClass"
Identifier "amd"
MatchDriver "amdgpu"
Driver "modesetting"
Option "PrimaryGPU" "yes"
EndSection

Section "OutputClass"
Identifier "intel"
MatchDriver "i915_bpo"
Driver "modesetting"
EndSection

Seems indeed some bugs in it, xserver start as expected with amdgpu as master
and i915 as slave (Xorg.0.log attached), but the screen is black, and run 
xrandr:
1. "xrandr --setprovideroutputsource 1 0" won't add any iGPU's output (0 is 
amdgpu, 1 is intel)
2. "xrandr --setprovideroutputsource 0 1" crash the xserver:
Xorg: ../../../xserver/dix/dispatch.c:4018: AttachOutputGPU: Assertion 
`new->isGPU' failed.
But I believe it can be fixed.

MS_ALL_IN_ONE method is my previous work when xserver has no proper PRIME sync.
And I find it's also useful for embedded platforms like iMX6/etnaviv.


I agree that it would be great to have a solution to have things just work
on iMX6/etnaviv, but we should be able to autodetect things on such a setup
without needing a special environment variable. Just an xorg.conf Outputclass 
snippet
matching the drm driver and marking that with Option "PrimaryGPU" "yes" should 
suffice.

> So published here.

As the MS_ALL_IN_ONE environment variable, I can change it to be an option of 
modesetting.


I think we don't really need another option, the Option "PrimaryGPU" "yes" 
should be
sufficient, combined with bug-fixes to the modesetting driver for cases where 
that
does not work.


Now as the PRIME sync is upstreamed, I think they can achieve nearly same 
effect in theory,


Right, so we really should use that and not add a second code path implementing 
in essence
the same thing.

If you could look into this and fix the modesetting driver to support this that 
would be
great.


but still minor difference when implementation in each DDX like modesetting 
need a proper
implementation of PresentSharedPixmap() for “OpenGL Syncing To VBlank”.


I think we should just fix things for the modesetting driver now, AFAICT most 
other
drivers are becoming more or less obsolete (or at least the should). Besides 
since your
original solution modifies the modesetting driver, it too has the drawback of 
only
working with the modesetting driver.

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH v2 xserver 00/11] modesetting: MS_ALL_IN_ONE

[Hans de Goede]

Hi,

On 07-01-17 09:01, Qiang Yu wrote:

V2: add PATCH 11 to support GPUScreen capable of display

This is for hybrid drm device use case that one drm device
is only capable of display and the other is only capable of
rendering.

Usage: start xserver with MS_ALL_IN_ONE=1, and configure both
the display (Screen) and render (GPUScreen) drm devices using
modesetting DDX, it will use Screen as display, GPUScreen as
render and create one screen for them. If the render device
is also capable of display, create a GPUScreen for it in
addition.

Client see only the render device and load render device's DRI
driver. Server side render is also accelerated by the render
device. Display device only display what render device draws.

There is still one problem: DRI3 can't support page flip because
client doesn't know when to create a linear pixmap for flip.


I've recently more or less made the same thing work for the
nvidia binary driver, but in a much simpler way. If you
write a xorg.conf which sets the render-only GPU as primary
then no changes to the modesetting driver are necessary
at all. Specifically some patches of mine were recently
merged to master allowing to do this without needing to
specify bus-ids, a xorg.conf snippet like this one is
sufficient:

https://fedorapeople.org/~jwrdegoede/10-nvidia-driver.conf

The trick here is these 2 lines:

MatchDriver "nvidia-drm"
Option "PrimaryGPU" "yes"

Which make any GPU which has the nvidia-drm kernel driver
be seen as primary, overriding the default primary detection
which looks at which GPU has its vga registers mmio mapped.

At least this works with the nvidia binary driver, it may be
that some small changes are needed when using the modesetting
driver on the render GPU.

I wonder if you can do something similar for your use case
without needing all these changes.

Eitherway depending on an environment variable for this
behavior is unacceptable IMHO, this really should be configured
through a xorg.conf snippet.

> Tested on a laptop with Intel iGPU and AMD dGPU.

On such a setup you should be able to get things to work using
master + a config snippet similar to:

https://fedorapeople.org/~jwrdegoede/10-nvidia-driver.conf

Without needing a patches modesetting driver at all.

Regards,

Hans










Qiang Yu (11):
  modesetting: add MS_ALL_IN_ONE handling
  modesetting: add is_primary to mark entity type
  modesetting: remove unused PciInfo in modesettingRec
  modesetting: add render entity init and free
  Revert "modesetting: Delete dead drmmode_bo_for_pixmap function."
  dri2: refine dri2_probe_driver_name
  modesetting: separate render and display
  modesetting: use drmmode_bo_for_pixmap in ms_do_pageflip
  modesetting: dri2 allocate linear backbuffer
  modesetting: allow display node has no gbm support
  modesetting: still create GPUScreen when it's capable of display

 hw/xfree86/dri2/dri2.c   |  35 +--
 hw/xfree86/drivers/modesetting/dri2.c|  33 ++-
 hw/xfree86/drivers/modesetting/driver.c  | 265 ---
 hw/xfree86/drivers/modesetting/driver.h  |   9 +-
 hw/xfree86/drivers/modesetting/drmmode_display.c | 128 ++-
 hw/xfree86/drivers/modesetting/drmmode_display.h |   3 +
 hw/xfree86/drivers/modesetting/dumb_bo.c |  11 +
 hw/xfree86/drivers/modesetting/dumb_bo.h |   1 +
 hw/xfree86/drivers/modesetting/pageflip.c|   4 +-
 hw/xfree86/drivers/modesetting/present.c |   6 +
 10 files changed, 385 insertions(+), 110 deletions(-)


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver] glamor: glamor_egl_get_display: Return NULL if eglGetPlatformDisplayEXT returns NULL

[Hans de Goede]

If the libEGL we are using has eglGetPlatformDisplayEXT, yet it still
returns NULL, then this very likely means that it does not support
the type (e.g. EGL_PLATFORM_GBM_MESA) passed in, and then returning NULL
is the right thing to do.

This avoids falling back to an eglGetDisplay() implementation which does
not understands the passed in gbm handle, treats it as a pointer to
something else completely, followed by a crash sooner or later.

Specifically this fixes using the nvidia binary driver, with nvidia's
libEGL + the modesetting driver on a secondary GPU crashing inside
glamor_egl_init() sometimes.

Cc: Adam Jackson <a...@redhat.com>
Cc: Eric Anholt <e...@anholt.net>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 glamor/glamor_egl.c | 4 
 glamor/glamor_egl.h | 4 +---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c
index 9cc0f8d..4bde637 100644
--- a/glamor/glamor_egl.c
+++ b/glamor/glamor_egl.c
@@ -769,6 +769,10 @@ glamor_egl_init(ScrnInfoPtr scrn, int fd)
 
 glamor_egl->display = glamor_egl_get_display(EGL_PLATFORM_GBM_MESA,
  glamor_egl->gbm);
+if (!glamor_egl->display) {
+xf86DrvMsg(scrn->scrnIndex, X_ERROR, "eglGetDisplay() failed\n");
+goto error;
+}
 #else
 glamor_egl->display = eglGetDisplay((EGLNativeDisplayType) (intptr_t) fd);
 #endif
diff --git a/glamor/glamor_egl.h b/glamor/glamor_egl.h
index 6b05f57..2c6d307 100644
--- a/glamor/glamor_egl.h
+++ b/glamor/glamor_egl.h
@@ -67,9 +67,7 @@ glamor_egl_get_display(EGLint type, void *native)
 PFNEGLGETPLATFORMDISPLAYEXTPROC getPlatformDisplayEXT =
 (void *) eglGetProcAddress("eglGetPlatformDisplayEXT");
 if (getPlatformDisplayEXT)
-dpy = getPlatformDisplayEXT(type, native, NULL);
-if (dpy)
-return dpy;
+return getPlatformDisplayEXT(type, native, NULL);
 }
 
 /* Welp, everything is awful. */
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xf86-input-libinput] Ignore LED updates for disabled devices

[Hans de Goede]

Hi,

On 20-12-16 11:09, Peter Hutterer wrote:

If an XKB AccessX timeout is set and a VT switch is triggered, the
AccessXTimeoutExpire function may be called after the device has already been
disabled. This can cause a null-pointer dereference as our shared libinput
device may have been released by then.

In the legacy drivers this would've simply caused a write to an invalid fd
(-1), not a crash. Here we need to be more careful.

https://bugs.freedesktop.org/show_bug.cgi?id=98464

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>


Patch LGTM:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans




---
 src/xf86libinput.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/xf86libinput.c b/src/xf86libinput.c
index b130a77..fd38c3b 100644
--- a/src/xf86libinput.c
+++ b/src/xf86libinput.c
@@ -785,6 +785,9 @@ xf86libinput_kbd_ctrl(DeviceIntPtr device, KeybdCtrl *ctrl)
 struct xf86libinput *driver_data = pInfo->private;
 struct libinput_device *ldevice = driver_data->shared_device->device;

+if (!device->enabled)
+   return;
+
 while (bits[i].xbit) {
if (ctrl->leds & bits[i].xbit)
leds |= bits[i].code;


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: Xorg glx module: GLVND, EGL, or ... ?

[Hans de Goede]

Hi,

On 15-12-16 17:08, Emil Velikov wrote:

On 15 December 2016 at 08:15, Michel Dänzer  wrote:


Hi Adam, Andy, Kyle,


even with GLVND in place and used by Mesa and other GL implementations,
one remaining issue preventing peaceful coexistence of Mesa based and
other GLX implementations is that other GLX implementations tend to ship
their own, mutually incompatible versions of the Xorg glx module. I'm
not sure about all the reasons for this, but an important one is that
the glx module in the xserver tree has been using the DRI driver
interface directly, which can only work with Mesa.


The "xfree86: Extend OutputClass config sections" series from Hans just landed.
With it one can correctly attribute/select the correct libglx.so,
which should tackle the issue ;-)


Not if you want to run some apps one GPU and other apps on the other
GPU ...

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [qxl] Xspice: Replace malloc/strdup use with xnfalloc/xnfstrdup

[Hans de Goede]

Hi,

On 15-12-16 10:57, Christophe Fergeau wrote:

spiceqxl_*.c files are Xspice-only code. They contain a few uses of
malloc/strdup, and none of these are checked for failure. It's better to
replace these with xfnalloc/xnfstrdup which are provided by the X server
and cannot fail (aborts on failure).

Signed-off-by: Christophe Fergeau <cferg...@redhat.com>


LGTM:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 src/spiceqxl_audio.c|  2 +-
 src/spiceqxl_main_loop.c|  4 ++--
 src/spiceqxl_spice_server.c | 12 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/spiceqxl_audio.c b/src/spiceqxl_audio.c
index 52a45f0..ec9260d 100644
--- a/src/spiceqxl_audio.c
+++ b/src/spiceqxl_audio.c
@@ -405,7 +405,7 @@ static void handle_one_change(qxl_screen_t *qxl, struct 
inotify_event *e)
 return;
 }

-fname = malloc(strlen(e->name) + strlen(qxl->playback_fifo_dir) + 1 + 
1);
+fname = xnfalloc(strlen(e->name) + strlen(qxl->playback_fifo_dir) + 1 
+ 1);
 strcpy(fname, qxl->playback_fifo_dir);
 strcat(fname, "/");
 strcat(fname, e->name);
diff --git a/src/spiceqxl_main_loop.c b/src/spiceqxl_main_loop.c
index c9894bb..669e116 100644
--- a/src/spiceqxl_main_loop.c
+++ b/src/spiceqxl_main_loop.c
@@ -209,7 +209,7 @@ int watch_count = 0;

 static SpiceWatch *watch_add(int fd, int event_mask, SpiceWatchFunc func, void 
*opaque)
 {
-SpiceWatch *watch = malloc(sizeof(SpiceWatch));
+SpiceWatch *watch = xnfalloc(sizeof(SpiceWatch));

 DPRINTF(0, "adding %p, fd=%d at %d", watch,
 fd, watch_count);
@@ -376,7 +376,7 @@ static int watch_update_mask_internal(SpiceWatch *watch, 
int event_mask)

 static SpiceWatch *watch_add(int fd, int event_mask, SpiceWatchFunc func, void 
*opaque)
 {
-SpiceWatch *watch = malloc(sizeof(SpiceWatch));
+SpiceWatch *watch = xnfalloc(sizeof(SpiceWatch));

 DPRINTF(0, "adding %p, fd=%d", watch, fd);

diff --git a/src/spiceqxl_spice_server.c b/src/spiceqxl_spice_server.c
index 6e8cf50..38257d0 100644
--- a/src/spiceqxl_spice_server.c
+++ b/src/spiceqxl_spice_server.c
@@ -200,23 +200,23 @@ void xspice_set_spice_server_options(OptionInfoPtr 
options)
 len = strlen(x509_dir) + 32;

 if (x509_key_file_base) {
-x509_key_file = strdup(x509_key_file_base);
+x509_key_file = xnfstrdup(x509_key_file_base);
 } else {
-x509_key_file = malloc(len);
+x509_key_file = xnfalloc(len);
 snprintf(x509_key_file, len, "%s/%s", x509_dir, 
X509_SERVER_KEY_FILE);
 }

 if (x509_cert_file_base) {
-x509_cert_file = strdup(x509_cert_file_base);
+x509_cert_file = xnfstrdup(x509_cert_file_base);
 } else {
-x509_cert_file = malloc(len);
+x509_cert_file = xnfalloc(len);
 snprintf(x509_cert_file, len, "%s/%s", x509_dir, 
X509_SERVER_CERT_FILE);
 }

 if (x509_cacert_file_base) {
-x509_cacert_file = strdup(x509_cert_file_base);
+x509_cacert_file = xnfstrdup(x509_cert_file_base);
 } else {
-x509_cacert_file = malloc(len);
+x509_cacert_file = xnfalloc(len);
 snprintf(x509_cacert_file, len, "%s/%s", x509_dir, 
X509_CA_CERT_FILE);
 }
 }


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [qxl] xspice: Adjust to X.org 1.19 changes

[Hans de Goede]

Hi,

On 14-12-16 11:51, Christophe Fergeau wrote:

In newer X.org versions, it's no longer supported to modify the set of
FDs passed to a BlockHandler method to get notified when the FD has data
to be read. This was limited anyway as we could only get read events
this way, and had to do our own polling to get notified about socket
writeability.

Starting from xserver 1.19, the supported way of doing this is to use
the SetNotifyFd/RemoveNotifyFd API, which is actually a much better way
as it matches very well the 'watch' API spice-server expects Xspice to
implement.

This commit switches to that new API, which removes the need for
RegisterBlockAndWakeupHandlers().


Thank you for doing this, one small comment inline, otherwise looks
good:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

(with the comment fixed).


---
I've lightly (xeyes/rxvt) tested this on f25, and tested this still builds on
f24.
This should fix the issues Hans mentioned in
https://lists.freedesktop.org/archives/spice-devel/2016-October/032501.html


 configure.ac |  5 ---
 src/spiceqxl_main_loop.c | 90 
 2 files changed, 84 insertions(+), 11 deletions(-)

diff --git a/configure.ac b/configure.ac
index e865e75..b737d0d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -68,8 +68,6 @@ PKG_CHECK_EXISTS(xfont2,

 # Obtain compiler/linker options for the driver dependencies
 PKG_CHECK_MODULES(XORG, [xorg-server >= 1.0.99.901] xproto fontsproto 
$xfont_pc $REQUIRED_MODULES)
-# Check for xorg 1.19 as XSpice is currently not working with it
-PKG_CHECK_EXISTS([xorg-server >= 1.18.99], [has_xorg119=yes])

 save_CFLAGS="$CFLAGS"
 CFLAGS="$XORG_CFLAGS"
@@ -141,9 +139,6 @@ if test "x$enable_xspice" = "xyes"; then
 AC_SUBST(SPICE_LIBS)
 ],
 )
-if test x"${enable_xspice}" = "xyes" && test x"${has_xorg119}" = "xyes"; 
then
-AC_MSG_ERROR("XSpice cannot currently work against X.Org 1.19")
-fi
 else
 enable_xspice=no
 fi
diff --git a/src/spiceqxl_main_loop.c b/src/spiceqxl_main_loop.c
index 49b715a..81bc418 100644
--- a/src/spiceqxl_main_loop.c
+++ b/src/spiceqxl_main_loop.c
@@ -193,6 +193,7 @@ static void timer_remove(SpiceTimer *timer)
 free(timer);
 }

+#if GET_ABI_MAJOR(ABI_VIDEODRV_VERSION) < 23
 struct SpiceWatch {
 RingItem link;
 int fd;
@@ -236,11 +237,6 @@ static void watch_remove(SpiceWatch *watch)
 watch_count--;
 }

-static void channel_event(int event, SpiceChannelEventInfo *info)
-{
-NOT_IMPLEMENTED
-}
-
 static int set_watch_fds(fd_set *rfds, fd_set *wfds)
 {
 SpiceWatch *watch;
@@ -276,7 +272,7 @@ static void xspice_block_handler(pointer data, OSTimePtr 
timeout, pointer readma
 /*
  * xserver only calls wakeup_handler with the read fd_set, so we
  * must either patch it or do a polling select ourselves, this is the
- * later approach. Since we are already doing a polling select, we
+ * latter approach. Since we are already doing a polling select, we
  * already select on all (we could avoid selecting on the read since
  * that *is* actually taken care of by the wakeup handler).
  */
@@ -338,9 +334,88 @@ static void xspice_wakeup_handler(pointer data, int nfds, 
pointer readmask)
 select_and_check_watches();
 }

+#else /* GET_ABI_MAJOR(ABI_VIDEODRV_VERSION) < 23 */
+
+struct SpiceWatch {
+int fd;
+int event_mask;
+SpiceWatchFunc func;
+void *opaque;
+};
+
+static void watch_fd_notified(int fd, int xevents, void *data)
+{
+SpiceWatch *watch = (SpiceWatch *)data;
+
+if ((watch->event_mask & SPICE_WATCH_EVENT_READ) && (xevents & 
X_NOTIFY_READ)) {
+watch->func(watch->fd, SPICE_WATCH_EVENT_READ, watch->opaque);
+}
+
+if ((watch->event_mask & SPICE_WATCH_EVENT_WRITE) && (xevents & 
X_NOTIFY_WRITE)) {
+watch->func(watch->fd, SPICE_WATCH_EVENT_WRITE, watch->opaque);
+}
+}
+
+static int watch_update_mask2(SpiceWatch *watch, int event_mask)
+{
+SetNotifyFd(watch->fd, NULL, X_NOTIFY_NONE, NULL);
+watch->event_mask = 0;
+
+if (event_mask & SPICE_WATCH_EVENT_READ) {
+SetNotifyFd(watch->fd, watch_fd_notified, X_NOTIFY_READ, watch);
+} else if (event_mask & SPICE_WATCH_EVENT_READ) {
+SetNotifyFd(watch->fd, watch_fd_notified, X_NOTIFY_WRITE, watch);
+} else {
+DPRINTF(0, "Unexpected watch event_mask: %i", event_mask);
+return -1;
+}
+watch->event_mask = event_mask;
+
+return 0;
+}
+
+static SpiceWatch *watch_add(int fd, int event_mask, SpiceWatchFunc func, void 
*opaque)
+{
+SpiceWatch *watch = malloc(sizeof(SpiceWatch));


This malloc may fail, please replace malloc with
xnfalloc which stands for X no fail alloc, it works like
the glib malloc functions.


+
+DPRINTF(0, "adding %p, fd=%d",

[PATCH xserver 5/6] xfree86: Allow overriding primary GPU detection from an OutputClass section

[Hans de Goede]

Allow using:

Option "PrimaryGPU" "yes"

In an OutputClass section to override the default primary GPU device
selection which selects the GPU used as output by the firmware.

If multiple output devices match an OutputClass section with
the PrimaryGPU option set, the first one enumerated becomes the
primary GPU.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 hw/xfree86/common/xf86platformBus.c | 19 +++
 hw/xfree86/man/xorg.conf.man| 12 +++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/hw/xfree86/common/xf86platformBus.c 
b/hw/xfree86/common/xf86platformBus.c
index fb4b6ea..665bb74 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -287,6 +287,7 @@ xf86platformProbe(void)
 {
 int i;
 Bool pci = TRUE;
+XF86ConfOutputClassPtr cl;
 
 config_odev_probe(xf86PlatformDeviceProbe);
 
@@ -302,6 +303,24 @@ xf86platformProbe(void)
 }
 }
 
+/* First see if there is an OutputClass match marking a device as primary 
*/
+for (i = 0; i < xf86_num_platform_devices; i++) {
+struct xf86_platform_device *dev = _platform_devices[i];
+for (cl = xf86configptr->conf_outputclass_lst; cl; cl = cl->list.next) 
{
+if (!OutputClassMatches(cl, dev))
+continue;
+
+if (xf86CheckBoolOption(cl->option_lst, "PrimaryGPU", FALSE)) {
+xf86Msg(X_CONFIG, "OutputClass \"%s\" setting %s as 
PrimaryGPU\n",
+cl->identifier, dev->attribs->path);
+primaryBus.type = BUS_PLATFORM;
+primaryBus.id.plat = dev;
+return 0;
+}
+}
+}
+
+/* Then check for pci_device_is_boot_vga() */
 for (i = 0; i < xf86_num_platform_devices; i++) {
 struct xf86_platform_device *dev = _platform_devices[i];
 
diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
index 990a3be..4668957 100644
--- a/hw/xfree86/man/xorg.conf.man
+++ b/hw/xfree86/man/xorg.conf.man
@@ -1291,11 +1291,21 @@ When an output device has been matched to the
 .B OutputClass
 section, any
 .B Option
-entries are applied to the device. See the
+entries are applied to the device. One
+.B OutputClass
+specific
+.B Option
+is recognized. See the
 .B Device
 section below for a description of the remaining
 .B Option
 entries.
+.TP 7
+.BI "Option \*qPrimaryGPU\*q \*q" boolean \*q
+This option specifies that the matched device should be treated as the
+primary GPU, replacing the selection of the GPU used as output by the
+firmware. If multiple output devices match an OutputClass section with
+the PrimaryGPU option set, the first one enumerated becomes the primary GPU.
 .SH "DEVICE SECTION"
 The config file may have multiple
 .B Device
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 6/6] xfree86: Add ModulePath support for OutputClass config Sections

[Hans de Goede]

Allow OutputClass config snippets to modify the module-path.

Note that any specified ModulePaths will be pre-pended to the normal
ModulePath. The idea behind this is that any output hardware specific
modules should have preference over the normal modules.

One use-case for this is the nvidia binary driver, this allows a
config snippet like this:

Section "OutputClass"
MatchDriver "nvidia"
Modulepath "/usr/lib64/nvidia/modules"
EndSection

To get the nvidia glx specific glx module loaded, but only when the
nvidia kernel driver is loaded.

Together with the glvnd work done recently, this allows the nouveau
+ mesa and nvidia-binary userspace stacks to co-exist on the same
system without any ldconfig / xorg.conf tweaking and the xserver will
automatically do the right thing depending on which kernel driver
(nouveau or nvidia) is loaded.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 hw/xfree86/common/xf86platformBus.c | 23 +++
 hw/xfree86/loader/loadmod.c |  1 +
 hw/xfree86/man/xorg.conf.man| 16 
 hw/xfree86/parser/OutputClass.c | 15 +++
 hw/xfree86/parser/xf86Parser.h  |  1 +
 5 files changed, 56 insertions(+)

diff --git a/hw/xfree86/common/xf86platformBus.c 
b/hw/xfree86/common/xf86platformBus.c
index 665bb74..61bd75b 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -40,6 +40,7 @@
 #include "hotplug.h"
 #include "systemd-logind.h"
 
+#include "loaderProcs.h"
 #include "xf86.h"
 #include "xf86_OSproc.h"
 #include "xf86Priv.h"
@@ -288,6 +289,7 @@ xf86platformProbe(void)
 int i;
 Bool pci = TRUE;
 XF86ConfOutputClassPtr cl;
+char *old_path, *path = NULL;
 
 config_odev_probe(xf86PlatformDeviceProbe);
 
@@ -301,8 +303,29 @@ xf86platformProbe(void)
 if (pci && (strncmp(busid, "pci:", 4) == 0)) {
 platform_find_pci_info(_platform_devices[i], busid);
 }
+
+/*
+ * Deal with OutputClass ModulePath directives, these must be
+ * processed before we do any module loading.
+ */
+for (cl = xf86configptr->conf_outputclass_lst; cl; cl = cl->list.next) 
{
+if (!OutputClassMatches(cl, _platform_devices[i]))
+continue;
+
+if (cl->modulepath && xf86ModPathFrom != X_CMDLINE) {
+old_path = path;
+XNFasprintf(, "%s,%s", cl->modulepath,
+path ? path : xf86ModulePath);
+free(old_path);
+xf86Msg(X_CONFIG, "OutputClass \"%s\" ModulePath extended to 
\"%s\"\n",
+cl->identifier, path);
+LoaderSetPath(path);
+}
+}
 }
 
+free(path);
+
 /* First see if there is an OutputClass match marking a device as primary 
*/
 for (i = 0; i < xf86_num_platform_devices; i++) {
 struct xf86_platform_device *dev = _platform_devices[i];
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 8bf6836..940f5fc 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -184,6 +184,7 @@ LoaderSetPath(const char *path)
 if (!path)
 return;
 
+FreeStringList(defaultPathList);
 defaultPathList = InitPathList(path);
 }
 
diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
index 4668957..5a68f24 100644
--- a/hw/xfree86/man/xorg.conf.man
+++ b/hw/xfree86/man/xorg.conf.man
@@ -1306,6 +1306,22 @@ This option specifies that the matched device should be 
treated as the
 primary GPU, replacing the selection of the GPU used as output by the
 firmware. If multiple output devices match an OutputClass section with
 the PrimaryGPU option set, the first one enumerated becomes the primary GPU.
+.PP
+A
+.B OutputClass
+Section may contain
+.B ModulePath
+entries. When an output device matches an
+.B OutputClass
+section, any
+.B ModulePath
+entries in that
+.B OutputClass
+are pre-pended to the search path for loadable Xorg server modules. See
+.B ModulePath
+in the
+.B Files
+section for more info.
 .SH "DEVICE SECTION"
 The config file may have multiple
 .B Device
diff --git a/hw/xfree86/parser/OutputClass.c b/hw/xfree86/parser/OutputClass.c
index f813ee6..01b348f 100644
--- a/hw/xfree86/parser/OutputClass.c
+++ b/hw/xfree86/parser/OutputClass.c
@@ -36,6 +36,7 @@ static const xf86ConfigSymTabRec OutputClassTab[] = {
 {ENDSECTION, "endsection"},
 {IDENTIFIER, "identifier"},
 {DRIVER, "driver"},
+{MODULEPATH, "modulepath"},
 {OPTION, "option"},
 {MATCH_DRIVER, "matchdriver"},
 {-1, ""},
@@ -53,6 +54,7 @@ xf86freeOutputClassList(XF86ConfOutputClassPtr ptr)

[PATCH xserver 3/6] xfree86: Add options support for OutputClass Options

[Hans de Goede]

Add support for setting options in OutputClass Sections and having these
applied to any matching output devices.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 hw/xfree86/common/xf86Option.c  |  5 -
 hw/xfree86/common/xf86platformBus.c | 42 +
 hw/xfree86/common/xf86platformBus.h |  2 ++
 hw/xfree86/man/xorg.conf.man| 10 +
 hw/xfree86/parser/OutputClass.c |  6 ++
 hw/xfree86/parser/xf86Parser.h  |  1 +
 6 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/hw/xfree86/common/xf86Option.c b/hw/xfree86/common/xf86Option.c
index 0e8bc1f..929724d 100644
--- a/hw/xfree86/common/xf86Option.c
+++ b/hw/xfree86/common/xf86Option.c
@@ -44,6 +44,7 @@
 #include "xf86Xinput.h"
 #include "xf86Optrec.h"
 #include "xf86Parser.h"
+#include "xf86platformBus.h" /* For OutputClass functions */
 #include "optionstr.h"
 
 static Bool ParseOptionValue(int scrnIndex, XF86OptionPtr options,
@@ -64,7 +65,7 @@ static Bool ParseOptionValue(int scrnIndex, XF86OptionPtr 
options,
  *
  * The order of precedence for options is:
  *
- *   extraOpts, display, confScreen, monitor, device
+ *   extraOpts, display, confScreen, monitor, device, outputClassOptions
  */
 
 void
@@ -79,6 +80,8 @@ xf86CollectOptions(ScrnInfoPtr pScrn, XF86OptionPtr extraOpts)
 pScrn->options = NULL;
 
 for (i = pScrn->numEntities - 1; i >= 0; i--) {
+xf86MergeOutputClassOptions(pScrn->entityList[i], >options);
+
 device = xf86GetDevFromEntity(pScrn->entityList[i],
   pScrn->entityInstanceList[i]);
 if (device && device->options) {
diff --git a/hw/xfree86/common/xf86platformBus.c 
b/hw/xfree86/common/xf86platformBus.c
index d1c2d1a..96816c5 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -311,6 +311,48 @@ xf86platformProbe(void)
 return 0;
 }
 
+void
+xf86MergeOutputClassOptions(int entityIndex, void **options)
+{
+const EntityPtr entity = xf86Entities[entityIndex];
+struct xf86_platform_device *dev = NULL;
+XF86ConfOutputClassPtr cl;
+XF86OptionPtr classopts;
+int i = 0;
+
+switch (entity->bus.type) {
+case BUS_PLATFORM:
+dev = entity->bus.id.plat;
+break;
+case BUS_PCI:
+for (i = 0; i < xf86_num_platform_devices; i++) {
+if (MATCH_PCI_DEVICES(xf86_platform_devices[i].pdev,
+  entity->bus.id.pci)) {
+dev = _platform_devices[i];
+break;
+}
+}
+break;
+default:
+xf86Msg(X_DEBUG, "xf86MergeOutputClassOptions unsupported bus type 
%d\n",
+entity->bus.type);
+}
+
+if (!dev)
+return;
+
+for (cl = xf86configptr->conf_outputclass_lst; cl; cl = cl->list.next) {
+if (!OutputClassMatches(cl, dev) || !cl->option_lst)
+continue;
+
+xf86Msg(X_INFO, "Applying OutputClass \"%s\" options to %s\n",
+cl->identifier, dev->attribs->path);
+
+classopts = xf86optionListDup(cl->option_lst);
+*options = xf86optionListMerge(*options, classopts);
+}
+}
+
 static int
 xf86ClaimPlatformSlot(struct xf86_platform_device * d, DriverPtr drvp,
   int chipset, GDevPtr dev, Bool active)
diff --git a/hw/xfree86/common/xf86platformBus.h 
b/hw/xfree86/common/xf86platformBus.h
index 0f5c0ef..70d9ec8 100644
--- a/hw/xfree86/common/xf86platformBus.h
+++ b/hw/xfree86/common/xf86platformBus.h
@@ -42,6 +42,7 @@ struct xf86_platform_device {
 int xf86platformProbe(void);
 int xf86platformProbeDev(DriverPtr drvp);
 int xf86platformAddGPUDevices(DriverPtr drvp);
+void xf86MergeOutputClassOptions(int entityIndex, void **options);
 
 extern int xf86_num_platform_devices;
 extern struct xf86_platform_device *xf86_platform_devices;
@@ -161,6 +162,7 @@ extern void xf86platformPrimary(void);
 #else
 
 static inline int xf86platformAddGPUDevices(DriverPtr drvp) { return FALSE; }
+static inline void xf86MergeOutputClassOptions(int index, void **options) {}
 
 #endif
 
diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
index 3e596e4..990a3be 100644
--- a/hw/xfree86/man/xorg.conf.man
+++ b/hw/xfree86/man/xorg.conf.man
@@ -1286,6 +1286,16 @@ For example:
 Check the case-sensitive string
 .RI \*q matchdriver \*q
 against the kernel driver of the device.
+.PP
+When an output device has been matched to the
+.B OutputClass
+section, any
+.B Option
+entries are applied to the device. See the
+.B Device
+section below for a description of the remaining
+.B Option
+entries.
 .SH "DEVICE SECTION"
 The config file may have multiple
 .B Device
diff --git a/hw/xfree86/parser/OutputClass.c b/hw/xfree86/parser/OutputClass.c
index 8064e0c..f813ee6 100644

[PATCH xserver 0/6] xfree86: Extend OutputClass config sections

[Hans de Goede]

Hi All,

This patch series main goal (*) is to allow the nvidia binary driver
to install a xorg.conf snippet with the following contents:

Section "OutputClass"
Identifier "nvidia"
MatchDriver "nvidia-drm"
Driver "nvidia"
Option "AllowEmptyInitialConfiguration"
Option "PrimaryGPU" "yes"
ModulePath "/usr/lib64/nvidia/xorg"
EndSection

This snippet has 2 purposes:

1) Make the xserver autoconfig code automatically set things up the
right way for the nvidia driver on optimus enabled laptops. This way
the nvidia driver will just work on such setups without the user needing
to write / use a laptop model specific xorg.conf, this is the main goal
of this series

2) Make the xserver add "/usr/lib64/nvidia/xorg" to the front of the
module search-path when the nvidia driver is in use. Before this patch-set
the nvidia driver would drop a config snippet which unconditionally
does this, causing the xserver to always load the nvidia glx module
instead of its own glx module.

Making the adding of "/usr/lib64/nvidia/xorg" conditional, fixes the
server using the wrong glx module when the nvidia kernel module did not
get loaded for some reason. This fixes e.g. gdm not working due to
broken glx support, requiring the user to reboot into text-only or
single user mode to fix things. Now we will atleast still give the
user a gui to e.g. go on a forum to ask for help.

Together with glvnd support for mesa this actually means that the
nouveau/modesetting + mesa user-space can be fully parallel installed with
the nvidia driver user-space and the xserver will just do the right thing
(depending on the loaded kernel module) and everything will just work. This
will also allow for easier switching between nouveau and the nvidia driver.

Regards,

Hans


*) I can imagine some other uses for it too, I've tried to keep this as
generic as possible.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 4/6] xfree86: xf86platformProbe: split finding pci-info and setting primary GPU

[Hans de Goede]

This is a preparation patch for allowing an OutputClass section to
override the default primary GPU device selection.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 hw/xfree86/common/xf86platformBus.c | 23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/hw/xfree86/common/xf86platformBus.c 
b/hw/xfree86/common/xf86platformBus.c
index 96816c5..fb4b6ea 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -146,16 +146,9 @@ platform_find_pci_info(struct xf86_platform_device *pd, 
char *busid)
 
 iter = pci_slot_match_iterator_create();
 info = pci_device_next(iter);
-if (info) {
+if (info)
 pd->pdev = info;
-pci_device_probe(info);
-if (pci_device_is_boot_vga(info)) {
-primaryBus.type = BUS_PLATFORM;
-primaryBus.id.plat = pd;
-}
-}
 pci_iterator_destroy(iter);
-
 }
 
 static Bool
@@ -308,6 +301,20 @@ xf86platformProbe(void)
 platform_find_pci_info(_platform_devices[i], busid);
 }
 }
+
+for (i = 0; i < xf86_num_platform_devices; i++) {
+struct xf86_platform_device *dev = _platform_devices[i];
+
+if (!dev->pdev)
+continue;
+
+pci_device_probe(dev->pdev);
+if (pci_device_is_boot_vga(dev->pdev)) {
+primaryBus.type = BUS_PLATFORM;
+primaryBus.id.plat = dev;
+}
+}
+
 return 0;
 }
 
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 2/6] xfree86: Make OutputClassMatches take a xf86_platform_device

[Hans de Goede]

Make OutputClassMatches directly take a xf86_platform_device as argument,
rather then an index into xf86_platform_devices. This makes things
easier for callers which already have a xf86_platform_device pointer.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 hw/xfree86/common/xf86platformBus.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/xfree86/common/xf86platformBus.c 
b/hw/xfree86/common/xf86platformBus.c
index c8378a5..d1c2d1a 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -215,9 +215,10 @@ MatchToken(const char *value, struct xorg_list *patterns,
 }
 
 static Bool
-OutputClassMatches(const XF86ConfOutputClassPtr oclass, int index)
+OutputClassMatches(const XF86ConfOutputClassPtr oclass,
+   struct xf86_platform_device *dev)
 {
-char *driver = xf86_platform_odev_attributes(index)->driver;
+char *driver = dev->attribs->driver;
 
 if (!MatchToken(driver, >match_driver, strcmp))
 return FALSE;
@@ -235,7 +236,7 @@ xf86OutputClassDriverList(int index, char *matches[], int 
nmatches)
 return 0;
 
 for (cl = xf86configptr->conf_outputclass_lst; cl; cl = cl->list.next) {
-if (OutputClassMatches(cl, index)) {
+if (OutputClassMatches(cl, _platform_devices[index])) {
 char *path = xf86_platform_odev_attributes(index)->path;
 
 xf86Msg(X_INFO, "Applying OutputClass \"%s\" to %s\n",
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 1/6] xfree86: Free devlist returned by xf86MatchDevice

[Hans de Goede]

xf86MatchDevice returns a dynamically allocated list of GDevPtr-s,
free this when we're done with it.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 hw/xfree86/common/xf86platformBus.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/xfree86/common/xf86platformBus.c 
b/hw/xfree86/common/xf86platformBus.c
index 42789ca..c8378a5 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -480,6 +480,8 @@ xf86platformProbeDev(DriverPtr drvp)
 isGPUDevice(devList[i]) ? 
PLATFORM_PROBE_GPU_SCREEN : 0);
 }
 
+free(devList);
+
 return foundScreen;
 }
 
@@ -506,6 +508,8 @@ xf86platformAddGPUDevices(DriverPtr drvp)
 }
 }
 
+free(devList);
+
 return foundScreen;
 }
 
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [xserver PULL master] randr fixes + primary GPU fallback

[Hans de Goede]

Hi,

On 06-12-16 19:06, Adam Jackson wrote:

On Tue, 2016-12-06 at 15:22 +0100, Hans de Goede wrote:

Hi,

On Mon Dec 5 16:54:51 UTC 2016, Adam Jackson wrote:
 > Not a fan of the autobind patch in terms of upstreaming, I feel like we
 > really should be able to do better.

We've had years to do better


And? This is seat configuration. It's something the system should tell
us, not something we should guess at. That solves it both for all DEs
and for all display servers.


No this is not seat configuration, when auto-configuring output devices
we already take the seat into account AFAIK, so we only bring up GPUs which
are part of the seat the xserver is running for, this is about actually
tying these GPUs together, so that they actually form a coherent seat,
rather then being claimed by the server (since they belong to the current
seat) but not being usable by the end-user unless the end-user first
types some black-magic xrandr commands.

I find the this is seat configuration pov quite interesting btw, that
does nicely solve the policy problem of which devices to bring up in
a DE agnostic manner, which means that the one thing which has stopped
this patch from going upstream actually has been solved long ago.

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver v3] autobind GPUs to the screen

[Hans de Goede]

From: Dave Airlie <airl...@redhat.com>

This is a modified version of a patch we've been carry-ing in Fedora and
RHEL for years now. This patch automatically adds secondary GPUs to the
master as output sink / offload source making e.g. the use of
slave-outputs just work, with requiring the user to manually run
"xrandr --setprovideroutputsource" before he can hookup an external
monitor to his hybrid graphics laptop.

There is one problem with this patch, which is why it was not upstreamed
before. What to do when a secondary GPU gets detected really is a policy
decission (e.g. one may want to autobind PCI GPUs but not USB ones) and
as such should be under control of the Desktop Environment.

Unconditionally adding autobinding support to the xserver will result
in races between the DE dealing with the hotplug of a secondary GPU
and the server itself dealing with it.

However we've waited for years for any Desktop Environments to actually
start doing some sort of autoconfiguration of secondary GPUs and there
is still not a single DE dealing with this, so I believe that it is
time to upstream this now.

To avoid potential future problems if any DEs get support for doing
secondary GPU configuration themselves, the new autobind functionality
is made optional. Since no DEs currently support doing this themselves it
is enabled by default. When DEs grow support for doing this themselves
they can disable the servers autobinding through the servers cmdline or a
xorg.conf snippet.

Signed-off-by: Dave Airlie <airl...@gmail.com>
[hdego...@redhat.com: Make configurable, fix with nvidia, submit upstream]
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2:
-Make the default enabled instead of installing a xorg.conf
 snippet which enables it unconditionally
Changes in v3:
-Handle GPUScreen autoconfig in randr/rrprovider.c, looking at
 rrScrPriv->provider, rather then in hw/xfree86/modes/xf86Crtc.c
 looking at xf86CrtcConfig->provider. This fixes the autoconfig not
 working with the nvidia binary driver
---
 hw/xfree86/common/xf86Config.c  | 19 +++
 hw/xfree86/common/xf86Globals.c |  2 ++
 hw/xfree86/common/xf86Init.c| 20 
 hw/xfree86/common/xf86Priv.h|  1 +
 hw/xfree86/common/xf86Privstr.h |  1 +
 hw/xfree86/common/xf86platformBus.c |  4 
 hw/xfree86/man/Xorg.man |  7 +++
 hw/xfree86/man/xorg.conf.man|  6 ++
 randr/randrstr.h|  3 +++
 randr/rrprovider.c  | 22 ++
 10 files changed, 85 insertions(+)

diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
index 21daf1a..df3ca50 100644
--- a/hw/xfree86/common/xf86Config.c
+++ b/hw/xfree86/common/xf86Config.c
@@ -719,6 +719,7 @@ typedef enum {
 FLAG_DRI2,
 FLAG_USE_SIGIO,
 FLAG_AUTO_ADD_GPU,
+FLAG_AUTO_BIND_GPU,
 FLAG_MAX_CLIENTS,
 FLAG_IGLX,
 } FlagValues;
@@ -778,6 +779,8 @@ static OptionInfoRec FlagOptions[] = {
  {0}, FALSE},
 {FLAG_AUTO_ADD_GPU, "AutoAddGPU", OPTV_BOOLEAN,
  {0}, FALSE},
+{FLAG_AUTO_BIND_GPU, "AutoBindGPU", OPTV_BOOLEAN,
+ {0}, FALSE},
 {FLAG_MAX_CLIENTS, "MaxClients", OPTV_INTEGER,
  {0}, FALSE },
 {FLAG_IGLX, "IndirectGLX", OPTV_BOOLEAN,
@@ -857,6 +860,22 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, 
XF86OptionPtr layoutopts)
 }
 xf86Msg(from, "%sutomatically adding GPU devices\n",
 xf86Info.autoAddGPU ? "A" : "Not a");
+
+if (xf86AutoBindGPUDisabled) {
+xf86Info.autoBindGPU = FALSE;
+from = X_CMDLINE;
+}
+else if (xf86IsOptionSet(FlagOptions, FLAG_AUTO_BIND_GPU)) {
+xf86GetOptValBool(FlagOptions, FLAG_AUTO_BIND_GPU,
+  );
+from = X_CONFIG;
+}
+else {
+from = X_DEFAULT;
+}
+xf86Msg(from, "%sutomatically binding GPU devices\n",
+xf86Info.autoBindGPU ? "A" : "Not a");
+
 /*
  * Set things up based on the config file information.  Some of these
  * settings may be overridden later when the command line options are
diff --git a/hw/xfree86/common/xf86Globals.c b/hw/xfree86/common/xf86Globals.c
index e962b75..0d1e31b 100644
--- a/hw/xfree86/common/xf86Globals.c
+++ b/hw/xfree86/common/xf86Globals.c
@@ -136,6 +136,7 @@ xf86InfoRec xf86Info = {
 #else
 .autoAddGPU = FALSE,
 #endif
+.autoBindGPU = TRUE,
 };
 
 const char *xf86ConfigFile = NULL;
@@ -197,6 +198,7 @@ Bool xf86FlipPixels = FALSE;
 Gamma xf86Gamma = { 0.0, 0.0, 0.0 };
 
 Bool xf86AllowMouseOpenFail = FALSE;
+Bool xf86AutoBindGPUDisabled = FALSE;
 
 #ifdef XF86VIDMODE
 Bool xf86VidModeDisabled = FALSE;
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index a544b65..b0cba3d 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@

Re: [xserver PULL master] randr fixes + primary GPU fallback

[Hans de Goede]

Hi,

On Mon Dec 5 16:54:51 UTC 2016, Adam Jackson wrote:
> Not a fan of the autobind patch in terms of upstreaming, I feel like we
> really should be able to do better.

We've had years to do better and carried this patch
in both Fedora and RHEL for years and nothing better
materialized, I think we're really doing (other)
downstream users a dis-service by keeping this patch
out of upstream. I can only hope that other distros
are including it themselves, otherwise a lot of
the effort spend on making prime setups work smoothly
is wasted, unless users can find the right magic
xrandr commands to make things work in google ...

Regards,

Hans


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[xserver PULL master] randr fixes + primary GPU fallback

[Hans de Goede]

Hi Adam, Keith,

Here is a pull-req consisting of 2 different patch-sets:

1) 2 randr fixes for sometimes the screen pixmap being
too small when using slave outputs, these have been
Reviewed-by Dave Airlie

2) 3 xfree86 patches which together make the xserver
choice the first available device, rather then exiting
with a "No screens" error, when output devices are found,
but non is recognized as being the primary / boot display
adapter. This is necessary at least for the simpledrm
kms driver, which due to its nature does not get recognized
as being the boot display. These 3 patches are *unreviewed*,
but have been part of the Fedora xserver pkg since F25 beta
and have shown no issues, as such I believe these are ready
for merging.

The following changes since commit e1d30075c923f96a375895d74ea12a3c92a640c6:

  configure: Enable glamor when building just Xwayland (2016-11-30 09:47:43 
+0100)

are available in the git repository at:

  git://people.freedesktop.org/~jwrdegoede/xserver for-master

for you to fetch changes up to 55a4ce24fd778627bd1ca65f4735c4568a2332cd:

  xfree86: Try harder to find atleast 1 non GPU Screen (2016-12-05 11:21:49 
+0100)

--------
Hans de Goede (5):
  randr: rrCheckPixmapBounding: Do not substract crtc non 0 x,y from screen 
size
  randr: rrCheckPixmapBounding: do not shrink the screen_pixmap
  xfree86: Remove redundant ServerIsNotSeat0 check from xf86CallDriverProbe
  xfree86: Make adding unclaimed devices as GPU devices a separate step
  xfree86: Try harder to find atleast 1 non GPU Screen

 hw/xfree86/common/xf86.h|  1 +
 hw/xfree86/common/xf86Bus.c | 32 +++-
 hw/xfree86/common/xf86Globals.c |  1 +
 hw/xfree86/common/xf86pciBus.c  |  4 
 hw/xfree86/common/xf86platformBus.c | 19 +++
 hw/xfree86/common/xf86platformBus.h |  6 ++
 randr/rrcrtc.c  | 10 --
 7 files changed, 66 insertions(+), 7 deletions(-)

Regards,

Hans


p.s.

Talking about unreviewed patched it would be really good to get
a review of / some discussion on:

https://patchwork.freedesktop.org/patch/112579/

Which is the last magic sauce in the Fedora xserver pkgs and
is really important for a smooth ootb experience on laptops
with hybrid gfx.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/3] xfree86: Immediately handle failure to set HW cursor, v5

[Hans de Goede]

Hi,

On 05-12-16 10:25, Michael Thayer wrote:

Hello Hans,

Polite ping on this one.


I've already given this series my:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

There is not much else I can do, from here on it
is up to the xserver maintainers to pick up the series.

Regards,

Hans





Regards and thanks
Michael

01.10.2016 12:01, Hans de Goede wrote:

Hi,

On 30-09-16 17:55, Michael Thayer wrote:

Based on v4 by Alexandre Courbot <acour...@nvidia.com>

There is currently no reliable way to report failure to set a HW
cursor. Still such failures can happen if e.g. the MODE_CURSOR DRM
ioctl fails (which currently happens at least with modesetting on Tegra
for format incompatibility reasons).

As failures are currently handled by setting the HW cursor size to
(0,0), the fallback to SW cursor will not happen until the next time the
cursor changes and xf86CursorSetCursor() is called again. In the
meantime, the cursor will be invisible to the user.

This patch addresses that by adding _xf86CrtcFuncs::set_cursor_check and
_xf86CursorInfoRec::ShowCursorCheck hook variants that return booleans.
This allows to propagate errors up to xf86CursorSetCursor(), which can
then fall back to using the SW cursor immediately.

v5: Updated the patch to apply to current git HEAD, split up into two
patches (server and modesetting driver) and adjusted the code slightly
to match surrounding code.  I also removed the new exported function
ShowCursorCheck(), as instead just changing ShowCursor() to return Bool
should not affect its current callers.

Signed-off-by: Michael Thayer <michael.tha...@oracle.com>


Series looks good to me and is:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

As discussed this will need to wait till we start the 1.20
cycle before it can be merged (it contains a video driver ABI change)

Regards,

Hans




---
 hw/xfree86/modes/xf86Crtc.h|  4 +++-
 hw/xfree86/modes/xf86Cursors.c | 40
++--
 hw/xfree86/ramdac/xf86Cursor.h | 16 
 hw/xfree86/ramdac/xf86HWCurs.c |  8 
 4 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
index 14ba9d7..215eb2a 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -195,6 +195,8 @@ typedef struct _xf86CrtcFuncs {
  */
 void
  (*show_cursor) (xf86CrtcPtr crtc);
+Bool
+ (*show_cursor_check) (xf86CrtcPtr crtc);

 /**
  * Hide cursor
@@ -993,7 +995,7 @@ static _X_INLINE _X_DEPRECATED void
xf86_reload_cursors(ScreenPtr screen) {}
 /**
  * Called from EnterVT to turn the cursors back on
  */
-extern _X_EXPORT void
+extern _X_EXPORT Bool
  xf86_show_cursors(ScrnInfoPtr scrn);

 /**
diff --git a/hw/xfree86/modes/xf86Cursors.c
b/hw/xfree86/modes/xf86Cursors.c
index 1bc2b27..9543eed 100644
--- a/hw/xfree86/modes/xf86Cursors.c
+++ b/hw/xfree86/modes/xf86Cursors.c
@@ -210,9 +210,15 @@ set_bit(CARD8 *image, xf86CursorInfoPtr
cursor_info, int x, int y, Bool mask)

 /*
  * Wrappers to deal with API compatibility with drivers that don't
expose
- * load_cursor_*_check
+ * *_cursor_*_check
  */
 static inline Bool
+xf86_driver_has_show_cursor(xf86CrtcPtr crtc)
+{
+return crtc->funcs->show_cursor_check || crtc->funcs->show_cursor;
+}
+
+static inline Bool
 xf86_driver_has_load_cursor_image(xf86CrtcPtr crtc)
 {
 return crtc->funcs->load_cursor_image_check ||
crtc->funcs->load_cursor_image;
@@ -225,6 +231,15 @@ xf86_driver_has_load_cursor_argb(xf86CrtcPtr crtc)
 }

 static inline Bool
+xf86_driver_show_cursor(xf86CrtcPtr crtc)
+{
+if (crtc->funcs->show_cursor_check)
+return crtc->funcs->show_cursor_check(crtc);
+crtc->funcs->show_cursor(crtc);
+return TRUE;
+}
+
+static inline Bool
 xf86_driver_load_cursor_image(xf86CrtcPtr crtc, CARD8 *cursor_image)
 {
 if (crtc->funcs->load_cursor_image_check)
@@ -333,16 +348,19 @@ xf86_hide_cursors(ScrnInfoPtr scrn)
 }
 }

-static void
+static Bool
 xf86_crtc_show_cursor(xf86CrtcPtr crtc)
 {
-if (!crtc->cursor_shown && crtc->cursor_in_range) {
-crtc->funcs->show_cursor(crtc);
-crtc->cursor_shown = TRUE;
-}
+if (!crtc->cursor_in_range)
+return TRUE;
+
+if (!crtc->cursor_shown)
+crtc->cursor_shown = xf86_driver_show_cursor(crtc);
+
+return crtc->cursor_shown;
 }

-void
+Bool
 xf86_show_cursors(ScrnInfoPtr scrn)
 {
 xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -352,9 +370,11 @@ xf86_show_cursors(ScrnInfoPtr scrn)
 for (c = 0; c < xf86_config->num_crtc; c++) {
 xf86CrtcPtr crtc = xf86_config->crtc[c];

-if (crtc->enabled)
-xf86_crtc_show_cursor(crtc);
+if (crtc->enabled && !xf86_crtc_show_cursor(crtc))
+return FALSE;
 }
+
+return TRUE;
 }

 static void
@@ 

Re: [PATCH xf86-input-libinput] Fix default scroll button number

[Hans de Goede]

Hi,

On 05-12-16 05:35, Peter Hutterer wrote:

Was exposing the evdev code rather than the xorg code.

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>



Patch LGTM:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans





---
 src/xf86libinput.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/xf86libinput.c b/src/xf86libinput.c
index 747e84b..b130a77 100644
--- a/src/xf86libinput.c
+++ b/src/xf86libinput.c
@@ -4386,6 +4386,7 @@ LibinputInitScrollMethodsProperty(DeviceIntPtr dev,
return;

scroll_button = 
libinput_device_config_scroll_get_default_button(device);
+   scroll_button = btn_linux2xorg(scroll_button);
prop_scroll_button_default = LibinputMakeProperty(dev,
  
LIBINPUT_PROP_SCROLL_BUTTON_DEFAULT,
  XA_CARDINAL, 
32,


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xf86-input-libinput 1/3] Add a comment regarding scroll dist default values

[Hans de Goede]

Hi,

On 29-11-16 00:25, Peter Hutterer wrote:

Changed this during development because I forgot that the value actually
matters (for touchpads anyway).

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>


Series looks good to me:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 src/xf86libinput.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/xf86libinput.c b/src/xf86libinput.c
index 747e84b..324bfc8 100644
--- a/src/xf86libinput.c
+++ b/src/xf86libinput.c
@@ -2928,13 +2928,19 @@ xf86libinput_pre_init(InputDriverPtr drv,

pInfo->private = driver_data;
driver_data->pInfo = pInfo;
-   driver_data->scroll.vdist = 15;
-   driver_data->scroll.hdist = 15;
driver_data->path = path;
driver_data->shared_device = shared_device;
xorg_list_append(_data->shared_device_link,
 _device->device_list);

+   /* Scroll dist value matters for source finger/continuous. For those
+* devices libinput provides pixel-like data, changing this will
+* affect touchpad scroll speed. For wheels it doesn't matter as
+* we're using the discrete value only.
+*/
+   driver_data->scroll.vdist = 15;
+   driver_data->scroll.hdist = 15;
+
if (!is_subdevice) {
if (libinput_device_has_capability(device, 
LIBINPUT_DEVICE_CAP_POINTER))
driver_data->capabilities |= CAP_POINTER;


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/2] randr: rrCheckPixmapBounding: Do not substract crtc non 0 x, y from screen size

[Hans de Goede]

Hi,

On 22-11-16 20:41, Dave Airlie wrote:

On 23 November 2016 at 00:28, Hans de Goede <hdego...@redhat.com> wrote:

The purpose of rrCheckPixmapBounding is to make sure that the
screen_pixmap is large enough for the slave-output which crtc is
being configured.

This should include crtc->x and crtc->y, otherwise the crtc might
still end up scanning out an area outside of the screen-pixmap.

For example: Take a laptop with an external monitor on a slave-output at
1920x1080+0+0 and its internal-screen at 3840x2160+1920+0 and in
gnome-settings-daemon move the external monitor to be on the ri ght of
the internal screen rather then on the left. First g-s-d will do a
RRSetScreenSize to 5760*2160 (which is a nop), then it calls RRSetCrtc
to move the slave output to 1920x1080+3840+0, since this is a slave
output, rrCheckPixmapBounding gets called, since the 2 crtcs now overlap
the code before this commit would shrinks the screen_pixmap to 3180*2160.
Then g-s-d calls RRSetCrtc to move the internal screen to 3180*2160+0+0.

And we end up with the slave-output configured to scan-out an area
which completely falls outside of the screen-pixmap (and end up with
a black display on the external monitor).

This commit fixes this by not substracting the x1 and y1 coordinates
of the union-ed region when determining the new screen_pixmap size.

Cc: Nikhil Mahale <nmah...@nvidia.com>
Cc: Dave Airlie <airl...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>


Thanks Hans, these seems to make sense to me.

Reviewed-by: Dave Airlie <airl...@redhat.com>


Thank you, "these" means the reviewed-by is for the series, right ?

Ajax, Keith, the exact push policy for the server is a bit unclear to
me now, I've seen several people push their own patches after a favorable
review. So can I just push these 2 with Dave's reviewed-by, or ... ?

Regards,

Hans







---
 randr/rrcrtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index 5d404e8..ac853ea 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -686,8 +686,8 @@ rrCheckPixmapBounding(ScreenPtr pScreen,
 }

 newsize = RegionExtents(_region);
-new_width = newsize->x2 - newsize->x1;
-new_height = newsize->y2 - newsize->y1;
+new_width = newsize->x2;
+new_height = newsize->y2;

 if (new_width == screen_pixmap->drawable.width &&
 new_height == screen_pixmap->drawable.height) {
--
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 2/2] randr: rrCheckPixmapBounding: do not shrink the screen_pixmap

[Hans de Goede]

Hi,

On 22-11-16 15:28, Hans de Goede wrote:

The purpose of rrCheckPixmapBounding is to make sure that the
screen_pixmap is *large* enough for the slave-output which crtc is
being configured.

However until now rrCheckPixmapBounding would also shrink the
screen_pixmap in certain scenarios leading to various problems.

For example: Take a laptop with its internalscreen on a slave-output and
currently disabled and an external monitor at 1920x1080+0+0.
Now lets say that we want to drive the external monitor at its native
resolution of 2560x1440 and have the internal screen mirror the top left
part of the external monitor, so we run:

  $ xrandr --output eDP --mode 1920x1080 --pos 0x0 --output HDMI \
  --mode 2560x1440 --pos 0x0

Here xrandr utility first calls RRSetScreenSize to 2560x1440, then it
calls RRSetCrtc 1920x1080+0+0 on the eDP, since this is a slave output,
rrCheckPixmapBounding gets called and resizes the screen_pixmap to
1920x1080, undoing the RRSetScreenSize. Then RRSetCrtc 2560x1440+0+0
gets called on the HDMI, depending on crtc->transforms this will
either result in a BadValue error from ProcRRSetCrtcConfig; or
it will succeed, but the monitor ends up running at 2560x1440
while showing a 1920x1080 screen_pixmap + black borders on the right
and bottom. Neither of which is what we want.

This commit removes the troublesome shrinking behavior, fixing this.

Note:

1) One could argue that this will leave us with a too large screen_pixmap
in some cases, but rrCheckPixmapBounding only gets called for slave
outputs, so xrandr clients already must manually shrink the screen_pixmap
after disabling crtcs in normal setups.

2) An alternative approach would be to also call rrCheckPixmapBounding
on RRSetCrtc on normal (non-slave) outputs, but that would result in
2 unnecessary resizes of the screen_pixmap in the above example, which
seems undesirable.

Cc: Nikhil Mahale <nmah...@nvidia.com>
Cc: Dave Airlie <airl...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>


p.s.

Nikhil, this should actually fix the issue you were trying to fix with this
patch: https://patchwork.freedesktop.org/patch/88159/

Also I'm pretty sure your patch did not actually fix this, as it
disabled the BadValue check for slave-outputs (GPU-screens), but the
BadValue error will actually trigger when setting up the non-slave
output, after calling RRSetCrtc on the slave-output and
rrCheckPixmapBounding has shrunk the screen_pixmap.

Also just disabling the check is no good, the shrinking is the problem,
not the check.

Regards,

Hans




---
 randr/rrcrtc.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index ac853ea..d1a51f0 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -689,6 +689,12 @@ rrCheckPixmapBounding(ScreenPtr pScreen,
 new_width = newsize->x2;
 new_height = newsize->y2;

+if (new_width < screen_pixmap->drawable.width)
+new_width = screen_pixmap->drawable.width;
+
+if (new_height < screen_pixmap->drawable.height)
+new_height = screen_pixmap->drawable.height;
+
 if (new_width == screen_pixmap->drawable.width &&
 new_height == screen_pixmap->drawable.height) {
 } else {


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 1/2] randr: rrCheckPixmapBounding: Do not substract crtc non 0 x, y from screen size

[Hans de Goede]

The purpose of rrCheckPixmapBounding is to make sure that the
screen_pixmap is large enough for the slave-output which crtc is
being configured.

This should include crtc->x and crtc->y, otherwise the crtc might
still end up scanning out an area outside of the screen-pixmap.

For example: Take a laptop with an external monitor on a slave-output at
1920x1080+0+0 and its internal-screen at 3840x2160+1920+0 and in
gnome-settings-daemon move the external monitor to be on the ri ght of
the internal screen rather then on the left. First g-s-d will do a
RRSetScreenSize to 5760*2160 (which is a nop), then it calls RRSetCrtc
to move the slave output to 1920x1080+3840+0, since this is a slave
output, rrCheckPixmapBounding gets called, since the 2 crtcs now overlap
the code before this commit would shrinks the screen_pixmap to 3180*2160.
Then g-s-d calls RRSetCrtc to move the internal screen to 3180*2160+0+0.

And we end up with the slave-output configured to scan-out an area
which completely falls outside of the screen-pixmap (and end up with
a black display on the external monitor).

This commit fixes this by not substracting the x1 and y1 coordinates
of the union-ed region when determining the new screen_pixmap size.

Cc: Nikhil Mahale <nmah...@nvidia.com>
Cc: Dave Airlie <airl...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 randr/rrcrtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index 5d404e8..ac853ea 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -686,8 +686,8 @@ rrCheckPixmapBounding(ScreenPtr pScreen,
 }
 
 newsize = RegionExtents(_region);
-new_width = newsize->x2 - newsize->x1;
-new_height = newsize->y2 - newsize->y1;
+new_width = newsize->x2;
+new_height = newsize->y2;
 
 if (new_width == screen_pixmap->drawable.width &&
 new_height == screen_pixmap->drawable.height) {
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 2/2] randr: rrCheckPixmapBounding: do not shrink the screen_pixmap

[Hans de Goede]

The purpose of rrCheckPixmapBounding is to make sure that the
screen_pixmap is *large* enough for the slave-output which crtc is
being configured.

However until now rrCheckPixmapBounding would also shrink the
screen_pixmap in certain scenarios leading to various problems.

For example: Take a laptop with its internalscreen on a slave-output and
currently disabled and an external monitor at 1920x1080+0+0.
Now lets say that we want to drive the external monitor at its native
resolution of 2560x1440 and have the internal screen mirror the top left
part of the external monitor, so we run:

  $ xrandr --output eDP --mode 1920x1080 --pos 0x0 --output HDMI \
  --mode 2560x1440 --pos 0x0

Here xrandr utility first calls RRSetScreenSize to 2560x1440, then it
calls RRSetCrtc 1920x1080+0+0 on the eDP, since this is a slave output,
rrCheckPixmapBounding gets called and resizes the screen_pixmap to
1920x1080, undoing the RRSetScreenSize. Then RRSetCrtc 2560x1440+0+0
gets called on the HDMI, depending on crtc->transforms this will
either result in a BadValue error from ProcRRSetCrtcConfig; or
it will succeed, but the monitor ends up running at 2560x1440
while showing a 1920x1080 screen_pixmap + black borders on the right
and bottom. Neither of which is what we want.

This commit removes the troublesome shrinking behavior, fixing this.

Note:

1) One could argue that this will leave us with a too large screen_pixmap
in some cases, but rrCheckPixmapBounding only gets called for slave
outputs, so xrandr clients already must manually shrink the screen_pixmap
after disabling crtcs in normal setups.

2) An alternative approach would be to also call rrCheckPixmapBounding
on RRSetCrtc on normal (non-slave) outputs, but that would result in
2 unnecessary resizes of the screen_pixmap in the above example, which
seems undesirable.

Cc: Nikhil Mahale <nmah...@nvidia.com>
Cc: Dave Airlie <airl...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 randr/rrcrtc.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index ac853ea..d1a51f0 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -689,6 +689,12 @@ rrCheckPixmapBounding(ScreenPtr pScreen,
 new_width = newsize->x2;
 new_height = newsize->y2;
 
+if (new_width < screen_pixmap->drawable.width)
+new_width = screen_pixmap->drawable.width;
+
+if (new_height < screen_pixmap->drawable.height)
+new_height = screen_pixmap->drawable.height;
+
 if (new_width == screen_pixmap->drawable.width &&
 new_height == screen_pixmap->drawable.height) {
 } else {
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [Spice-devel] [PATCH xf86-video-qxl] Adjust xspice_wakeup_handler() prototype for building xspice with server 1.19

[Hans de Goede]

Hi,

On 18-11-16 14:04, Timo Aaltonen wrote:

On 04.10.2016 14:41, Hans de Goede wrote:

Hi,

On 03-10-16 12:04, Christophe Fergeau wrote:



On Thu, Sep 29, 2016 at 01:03:01PM +0200, Hans de Goede wrote:

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 src/spiceqxl_main_loop.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/spiceqxl_main_loop.c b/src/spiceqxl_main_loop.c
index db89b6d..0ac1f3e 100644
--- a/src/spiceqxl_main_loop.c
+++ b/src/spiceqxl_main_loop.c
@@ -330,7 +330,11 @@ static int no_write_watches(Ring *w)
 return 1;
 }

+#if ABI_VIDEODRV_VERSION >= SET_ABI_VERSION(23, 0)


We have an occurrence of
#if GET_ABI_MAJOR(ABI_VIDEODRV_VERSION) < 6
I'd use this here too to stay consistent (I assume they are equivalent
here).

Acked-by: Christophe Fergeau <cferg...@redhat.com>


Sorry, but this patch turns out to be incomplete, self NACK.

I just saw that spiceqxl_main_loop also uses a BlockHandler
(xspice_block_handler) and expects to be able to add
fds to watch for read activity through the xserver mainloop
by treating the 3th argument as a FD_SET.

This is no longer supported as of xserver 1.19, instead
the new NotifyFD functionality should be used. The advantage
of this is that it can also properly watch fds for them
becoming ready for writing.

For an example patch of how to use the new NotifyFD
functionality see the recent tigervnc patch to make
tigervnc work with 1.19:

https://lists.x.org/archives/xorg-devel/2016-October/051482.html


Any update here?


Nope, sorry.


Looks like you still have the original patch in Fedora ;)


Yes, because it fixes the build. Xspice is not widely used,
so no-one has noticed it is broken yet...

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xf86-input-libinput] If the parent libinput_device is unavailable, create a new one

[Hans de Goede]

Hi,

On 18-11-16 07:47, Peter Hutterer wrote:

On Tue, Nov 15, 2016 at 10:35:31AM +0100, Hans de Goede wrote:

Hi,

On 15-11-16 05:37, Peter Hutterer wrote:

The parent device ref's the libinput device during pre_init and unref's it
during DEVICE_INIT, so the copy is lost. During DEVICE_ON, the libinput device
is re-added and ref'd, this one stays around now. But the takeaway is: unless
the device is enabled, no libinput device reference is available.

If a device is a mixed pointer + keyboard device, a subdevice is created
during a WorkProc. The subdevice relied on the parent's libinput_device being
available and didn't even check for it. This WorkProc usually runs after
the parent's DEVICE_ON, so in most cases all is well.

But when running without logind and the server is vt-switched away, the parent
device only runs PreInit and DEVICE_INIT but never DEVICE_ON, causing the
subdevice to burn, crash, and generally fail horribly when it dereferences the
parent's libinput device.

Fix this because we have global warming already and don't need to burn more
things and also because it's considered bad user experience to have the
server crash. The simple fix is to check the parent device first and if it is
unavailable, create a new one because it will end up disabled as well anyway,
so the ref goes away as well. The use-case where the parent somehow gets
disabled but the subdevice doesn't is a bit too niche to worry about.

This doesn't happen with logind because in that case we don't get a usable fd
while VT-switched away, so we can't even run PreInit and never get this far
(see the paused fd handling in the xfree86 code for that). It can be
reproduced by setting AutoEnableDevices off, but why would you do that,
seriously.

https://bugs.freedesktop.org/show_bug.cgi?id=97117

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>


Hmm, so what happens if the parent device later does get DEVICE_ON, after
the subdevice has created its own libinputdevice ?


we don't differ between parent and subdevices during DEVICE_ON, whichever
one is the first adds the device and the rest just refcounts it. so we
should be good here.


Ok, in that case, the original patch looks good to me and is:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xf86-input-libinput] If the parent libinput_device is unavailable, create a new one

[Hans de Goede]

Hi,

On 15-11-16 05:37, Peter Hutterer wrote:

The parent device ref's the libinput device during pre_init and unref's it
during DEVICE_INIT, so the copy is lost. During DEVICE_ON, the libinput device
is re-added and ref'd, this one stays around now. But the takeaway is: unless
the device is enabled, no libinput device reference is available.

If a device is a mixed pointer + keyboard device, a subdevice is created
during a WorkProc. The subdevice relied on the parent's libinput_device being
available and didn't even check for it. This WorkProc usually runs after
the parent's DEVICE_ON, so in most cases all is well.

But when running without logind and the server is vt-switched away, the parent
device only runs PreInit and DEVICE_INIT but never DEVICE_ON, causing the
subdevice to burn, crash, and generally fail horribly when it dereferences the
parent's libinput device.

Fix this because we have global warming already and don't need to burn more
things and also because it's considered bad user experience to have the
server crash. The simple fix is to check the parent device first and if it is
unavailable, create a new one because it will end up disabled as well anyway,
so the ref goes away as well. The use-case where the parent somehow gets
disabled but the subdevice doesn't is a bit too niche to worry about.

This doesn't happen with logind because in that case we don't get a usable fd
while VT-switched away, so we can't even run PreInit and never get this far
(see the paused fd handling in the xfree86 code for that). It can be
reproduced by setting AutoEnableDevices off, but why would you do that,
seriously.

https://bugs.freedesktop.org/show_bug.cgi?id=97117

Signed-off-by: Peter Hutterer 


Hmm, so what happens if the parent device later does get DEVICE_ON, after
the subdevice has created its own libinputdevice ?

Regards,

Hans




---
 src/xf86libinput.c | 42 +++---
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/xf86libinput.c b/src/xf86libinput.c
index 6792d1c..747e84b 100644
--- a/src/xf86libinput.c
+++ b/src/xf86libinput.c
@@ -2850,7 +2850,7 @@ xf86libinput_pre_init(InputDriverPtr drv,
struct xf86libinput *driver_data = NULL;
struct xf86libinput_device *shared_device = NULL;
struct libinput *libinput = NULL;
-   struct libinput_device *device;
+   struct libinput_device *device = NULL;
char *path = NULL;
bool is_subdevice;

@@ -2885,7 +2885,28 @@ xf86libinput_pre_init(InputDriverPtr drv,
}

is_subdevice = xf86libinput_is_subdevice(pInfo);
-   if (!is_subdevice) {
+   if (is_subdevice) {
+   InputInfoPtr parent;
+   struct xf86libinput *parent_driver_data;
+
+   parent = xf86libinput_get_parent(pInfo);
+   if (!parent) {
+   xf86IDrvMsg(pInfo, X_ERROR, "Failed to find parent 
device\n");
+   goto fail;
+   }
+
+   parent_driver_data = parent->private;
+   if (!parent_driver_data) /* parent already removed again */
+   goto fail;
+
+   xf86IDrvMsg(pInfo, X_INFO, "is a virtual subdevice\n");
+   shared_device = 
xf86libinput_shared_ref(parent_driver_data->shared_device);
+   device = shared_device->device;
+   if (!device)
+   xf86IDrvMsg(pInfo, X_ERROR, "Parent device not 
available\n");
+   }
+
+   if (!device) {
device = libinput_path_add_device(libinput, path);
if (!device) {
xf86IDrvMsg(pInfo, X_ERROR, "Failed to create a device for 
%s\n", path);
@@ -2903,23 +2924,6 @@ xf86libinput_pre_init(InputDriverPtr drv,
libinput_device_unref(device);
goto fail;
}
-   } else {
-   InputInfoPtr parent;
-   struct xf86libinput *parent_driver_data;
-
-   parent = xf86libinput_get_parent(pInfo);
-   if (!parent) {
-   xf86IDrvMsg(pInfo, X_ERROR, "Failed to find parent 
device\n");
-   goto fail;
-   }
-
-   parent_driver_data = parent->private;
-   if (!parent_driver_data) /* parent already removed again */
-   goto fail;
-
-   xf86IDrvMsg(pInfo, X_INFO, "is a virtual subdevice\n");
-   shared_device = 
xf86libinput_shared_ref(parent_driver_data->shared_device);
-   device = shared_device->device;
}

pInfo->private = driver_data;


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xf86-input-libinput] Link the left-handed property between the tools

[Hans de Goede]

Hi,

On 11-11-16 04:36, Peter Hutterer wrote:

The property is tablet-wide, not just per tool. So when one tool is updated,
run through all other devices that share the same underlying device.

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>


LGTM:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 src/xf86libinput.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/xf86libinput.c b/src/xf86libinput.c
index 5f7a551..9231ba6 100644
--- a/src/xf86libinput.c
+++ b/src/xf86libinput.c
@@ -3485,7 +3485,28 @@ LibinputSetPropertyLeftHanded(DeviceIntPtr dev,
if (!supported && left_handed)
return BadValue;
} else {
+   struct xf86libinput *other;
+
driver_data->options.left_handed = *data;
+
+   xorg_list_for_each_entry(other,
+
_data->shared_device->device_list,
+shared_device_link) {
+   DeviceIntPtr other_device = other->pInfo->dev;
+
+   if (other->options.left_handed == *data)
+   continue;
+
+   XIChangeDeviceProperty(other_device,
+  atom,
+  val->type,
+  val->format,
+  PropModeReplace,
+  val->size,
+  val->data,
+  TRUE);
+   }
+
}

return Success;


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xf86-input-libinput] Use __attribute__((cleanup)) for most of the xf86SetStrOption handling

[Hans de Goede]

Hi,

On 11-11-16 02:08, Peter Hutterer wrote:

Code was already clean, but this removes error path handling.

Signed-off-by: Peter Hutterer 
---
Somewhat in two minds about it because it doesn't gain us that much in this
codebase. I want to start using this more because it is quite useful and
this is a low-key project to start with :)


I'm not really a fan of using these gcc specific __attribute__ thingies.

Regards,

Hanse



 src/xf86libinput.c | 51 +--
 1 file changed, 21 insertions(+), 30 deletions(-)

diff --git a/src/xf86libinput.c b/src/xf86libinput.c
index a6481bc..5f7a551 100644
--- a/src/xf86libinput.c
+++ b/src/xf86libinput.c
@@ -84,6 +84,10 @@
 #define CAP_TABLET_TOOL0x10
 #define CAP_TABLET_PAD 0x20

+static inline void freep(void *p) { free(*(void**)p); }
+#define _cleanup_(x_) __attribute__((cleanup(x_)))
+#define _autofree_ __attribute__((cleanup(freep)))
+
 struct xf86libinput_driver {
struct libinput *libinput;
int device_enabled_count;
@@ -236,12 +240,11 @@ btn_xorg2linux(unsigned int b)
 static BOOL
 xf86libinput_is_subdevice(InputInfoPtr pInfo)
 {
-   char *source;
+   _autofree_ char *source = NULL;
BOOL is_subdevice;

source = xf86SetStrOption(pInfo->options, "_source", "");
is_subdevice = strcmp(source, "_driver/libinput") == 0;
-   free(source);

return is_subdevice;
 }
@@ -1134,12 +1137,11 @@ xf86libinput_init(DeviceIntPtr dev)
 static bool
 is_libinput_device(InputInfoPtr pInfo)
 {
-   char *driver;
+   _autofree_ char *driver = NULL;
BOOL rc;

driver = xf86CheckStrOption(pInfo->options, "driver", "");
rc = strcmp(driver, "libinput") == 0;
-   free(driver);

return rc;
 }
@@ -2012,13 +2014,12 @@ open_restricted(const char *path, int flags, void *data)
int fd = -1;

nt_list_for_each_entry(pInfo, xf86FirstLocalDevice(), next) {
-   char *device = xf86CheckStrOption(pInfo->options, "Device", 
NULL);
+   _autofree_ char *device = NULL;

-   if (device != NULL && strcmp(path, device) == 0) {
-   free(device);
+   device = xf86CheckStrOption(pInfo->options, "Device", NULL);
+
+   if (device != NULL && strcmp(path, device) == 0)
break;
-   }
-   free(device);
}

if (pInfo == NULL) {
@@ -2176,7 +2177,7 @@ xf86libinput_parse_tap_buttonmap_option(InputInfoPtr 
pInfo,
struct libinput_device *device)
 {
enum libinput_config_tap_button_map map;
-   char *str;
+   _autofree_ char *str = NULL;

if (libinput_device_config_tap_get_finger_count(device) == 0)
return FALSE;
@@ -2194,7 +2195,6 @@ xf86libinput_parse_tap_buttonmap_option(InputInfoPtr 
pInfo,
xf86IDrvMsg(pInfo, X_ERROR,
"Invalid TapButtonMap: %s\n",
str);
-   free(str);
}

if (libinput_device_config_tap_set_button_map(device, map) !=
@@ -2236,7 +2236,7 @@ xf86libinput_parse_accel_profile_option(InputInfoPtr 
pInfo,
struct libinput_device *device)
 {
enum libinput_config_accel_profile profile;
-   char *str;
+   _autofree_ char *str = NULL;

if (libinput_device_config_accel_get_profiles(device) ==
LIBINPUT_CONFIG_ACCEL_PROFILE_NONE)
@@ -2256,8 +2256,6 @@ xf86libinput_parse_accel_profile_option(InputInfoPtr 
pInfo,
profile = libinput_device_config_accel_get_profile(device);
}

-   free(str);
-
return profile;
 }

@@ -2290,7 +2288,7 @@ static inline enum libinput_config_send_events_mode
 xf86libinput_parse_sendevents_option(InputInfoPtr pInfo,
 struct libinput_device *device)
 {
-   char *modestr;
+   _autofree_ char *modestr = NULL;
enum libinput_config_send_events_mode mode;

if (libinput_device_config_send_events_get_modes(device) == 
LIBINPUT_CONFIG_SEND_EVENTS_ENABLED)
@@ -2311,7 +2309,6 @@ xf86libinput_parse_sendevents_option(InputInfoPtr pInfo,
xf86IDrvMsg(pInfo, X_ERROR,
"Invalid SendeventsMode: %s\n",
modestr);
-   free(modestr);
}

if (libinput_device_config_send_events_set_mode(device, mode) !=
@@ -2329,7 +2326,7 @@ xf86libinput_parse_calibration_option(InputInfoPtr pInfo,
  struct libinput_device *device,
  float matrix_out[9])
 {
-   char *str;
+   _autofree_ char *str = NULL;
float matrix[9] = { 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0};
int num_calibration;

@@ -2363,7 +2360,6 @@ 

Re: [PATCH xserver] glamor: restore vfunc handlers on init failure

[Hans de Goede]

Hi,

On 03-11-16 09:59, Olivier Fourdan wrote:

In glamor_init(), if the minimum requirements are not met, glamor may
fail after setting up its own CloseScreen() and DestroyPixmap()
routines, leading to a crash when either of the two routines is called
if glamor failed to complete its initialization, e.g:

  (EE) Backtrace:
  (EE) 0:  Xwayland (OsSigHandler+0x29)
  (EE) 1: /lib64/libpthread.so.0 (__restore_rt+0x0)
  (EE) 2: Xwayland (glamor_sync_close+0x2a)
  (EE) 3: Xwayland (glamor_close_screen+0x52)
  (EE) 4: Xwayland (CursorCloseScreen+0x88)
  (EE) 5: Xwayland (AnimCurCloseScreen+0xa4)
  (EE) 6: Xwayland (present_close_screen+0x42)
  (EE) 7: Xwayland (dix_main+0x4f9)
  (EE) 8: /lib64/libc.so.6 (__libc_start_main+0xf1)
  (EE) 9:  Xwayland (_start+0x2a)

Restore the previous CloseScreen() and DestroyPixmap() vfunc handlers in
case of failure when checking for the minimum requirements, so that if
any of the requirement is not met we don't leave the CloseScreen() and
DestroyPixmap() from glamor handlers in place.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1390018

Signed-off-by: Olivier Fourdan <ofour...@redhat.com>


Patch looks good to me:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans


---
 glamor/glamor.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index b771832..c54cf3b 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -470,7 +470,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 LogMessage(X_WARNING,
"glamor%d: Failed to allocate screen private\n",
screen->myNum);
-goto fail;
+goto free_glamor_private;
 }

 glamor_set_screen_private(screen, glamor_priv);
@@ -480,7 +480,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 LogMessage(X_WARNING,
"glamor%d: Failed to allocate pixmap private\n",
screen->myNum);
-goto fail;
+goto free_glamor_private;
 }

 if (!dixRegisterPrivateKey(_gc_private_key, PRIVATE_GC,
@@ -488,7 +488,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 LogMessage(X_WARNING,
"glamor%d: Failed to allocate gc private\n",
screen->myNum);
-goto fail;
+goto free_glamor_private;
 }

 glamor_priv->saved_procs.close_screen = screen->CloseScreen;
@@ -731,6 +731,11 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 return TRUE;

  fail:
+/* Restore default CloseScreen and DestroyPixmap handlers */
+screen->CloseScreen = glamor_priv->saved_procs.close_screen;
+screen->DestroyPixmap = glamor_priv->saved_procs.destroy_pixmap;
+
+ free_glamor_private:
 free(glamor_priv);
 glamor_set_screen_private(screen, NULL);
 return FALSE;


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] dix: Make sure client is not in output_pending chain after closed (RH 1382444)

[Hans de Goede]

Hi,

On 02-11-16 21:39, Keith Packard wrote:

I think it is possible that output could get queued to a client during
CloseDownClient. After it is removed from the pending queue, active
grabs are released, the client is awoken if sleeping and any work
queue entries related to the client are processed.

To fix this, move the call removing it from the output_pending chain
until after clientGone has been set and then check clientGone in
output_pending_mark.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1382444
Signed-off-by: Keith Packard <kei...@keithp.com>


Patch looks good to me:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 dix/dispatch.c  | 2 +-
 include/dixstruct.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/dix/dispatch.c b/dix/dispatch.c
index e111377..3d0fe26 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -3406,7 +3406,6 @@ CloseDownClient(ClientPtr client)
 UngrabServer(client);
 }
 mark_client_not_ready(client);
-xorg_list_del(>output_pending);
 BITCLEAR(grabWaiters, client->index);
 DeleteClientFromAnySelections(client);
 ReleaseActiveGrabs(client);
@@ -3435,6 +3434,7 @@ CloseDownClient(ClientPtr client)
 if (ClientIsAsleep(client))
 ClientSignal(client);
 ProcessWorkQueueZombies();
+output_pending_clear(client);
 CloseDownConnection(client);

 /* If the client made it to the Running stage, nClients has
diff --git a/include/dixstruct.h b/include/dixstruct.h
index 3b578f8..d71b0ac 100644
--- a/include/dixstruct.h
+++ b/include/dixstruct.h
@@ -159,7 +159,7 @@ extern struct xorg_list output_pending_clients;
 static inline void
 output_pending_mark(ClientPtr client)
 {
-if (xorg_list_is_empty(>output_pending))
+if (!client->clientGone && xorg_list_is_empty(>output_pending))
 xorg_list_append(>output_pending, _pending_clients);
 }



___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [ANNOUNCE] xorg-server 1.18.99.902

[Hans de Goede]

Hi,

On 02-11-16 16:50, Keith Packard wrote:

Michel Dänzer  writes:


Somebody still needs to come up with a fix for the FlushAllOutput
crashes, right?


That would sure be nice; can anyone reproduce them at all?


Judging from the amount of people filing bugs in Fedora's
bugzilla about this, quite a few people are hitting this, but
so far no one from the RH graphics team has been able to
reproduce :|

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] dix: Use 'output_pending_clear' instead of open-coding it in CloseDownClient

[Hans de Goede]

Hi,

On 28-10-16 18:25, Keith Packard wrote:

Signed-off-by: Keith Packard <kei...@keithp.com>


Patch LGTM:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 dix/dispatch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dix/dispatch.c b/dix/dispatch.c
index e111377..f16a84e 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -3406,7 +3406,7 @@ CloseDownClient(ClientPtr client)
 UngrabServer(client);
 }
 mark_client_not_ready(client);
-xorg_list_del(>output_pending);
+output_pending_clear(client);
 BITCLEAR(grabWaiters, client->index);
 DeleteClientFromAnySelections(client);
 ReleaseActiveGrabs(client);


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] os: Recompute whether any clients are ready after ProcessWorkQueue() (bug 98030)

[Hans de Goede]

Hi,

On 28-10-16 17:04, Keith Packard wrote:

If a work proc wakes up a sleeping client and it is ready to execute,
we need to re-compute the local 'are_ready' value before deciding
what timeout value to use in WaitForSomething.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98030
Signed-off-by: Keith Packard <kei...@keithp.com>


Patch LGTM:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 os/WaitFor.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/os/WaitFor.c b/os/WaitFor.c
index 7d5aa32..ff1c85e 100644
--- a/os/WaitFor.c
+++ b/os/WaitFor.c
@@ -204,8 +204,10 @@ WaitForSomething(Bool are_ready)
crashed connections and the screen saver timeout */
 while (1) {
 /* deal with any blocked jobs */
-if (workQueue)
+if (workQueue) {
 ProcessWorkQueue();
+are_ready = clients_are_ready();
+}

 if (are_ready)
 timeout = 0;


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [xserver PULL for 1.19 ] Various small fixes for 1.19

[Hans de Goede]

Hi,

On 26-10-16 12:26, Hans de Goede wrote:

Hi Adam, Keith,

Here is a pull-req with various small fixes
(all with at least 1 Reviewed-by) which I've collected
for merging into 1.19:


I just realized I forgot to add this one:

https://patchwork.freedesktop.org/patch/117894/

It would be good to merge that patch too IMO.

Regards,

Hans






The following changes since commit d13cb974426f7f1110b0bdb08c4ebb46ff8975f7:

  ddx: add new call to purge input devices that weren't added (2016-10-26 
15:35:07 +1000)

are available in the git repository at:

  git://people.freedesktop.org/~jwrdegoede/xserver for-server-1.19

for you to fetch changes up to 7b135f5e7d79622c0b922de8ee827a2556504d8f:

  xwayland: Transform pointer enter event coordinates (2016-10-26 11:51:38 
+0200)


Hans de Goede (1):
  xfree86: Xorg.wrap: Do not require root rights for cards with 0 outputs

Michel Dänzer (1):
  DRI2: Sync radeonsi_pci_ids.h from Mesa

Mihail Konev (2):
  xwin: make glx optional again
  modesetting: fix glamor ifdef

Nikhil Mahale (1):
  modesetting: unifdef MODESETTING_OUTPUT_SLAVE_SUPPORT

Rui Matos (1):
  xwayland: Transform pointer enter event coordinates

 configure.ac |  4 ++--
 hw/xfree86/dri2/pci_ids/radeonsi_pci_ids.h   | 12 
 hw/xfree86/drivers/modesetting/driver.c  |  2 ++
 hw/xfree86/drivers/modesetting/drmmode_display.c |  2 --
 hw/xfree86/xorg-wrapper.c|  2 +-
 hw/xwayland/xwayland-input.c |  5 -
 6 files changed, 21 insertions(+), 6 deletions(-)

Regards,

Hans

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[xserver PULL for 1.19 ] Various small fixes for 1.19

[Hans de Goede]

Hi Adam, Keith,

Here is a pull-req with various small fixes
(all with at least 1 Reviewed-by) which I've collected
for merging into 1.19:

The following changes since commit d13cb974426f7f1110b0bdb08c4ebb46ff8975f7:

  ddx: add new call to purge input devices that weren't added (2016-10-26 
15:35:07 +1000)

are available in the git repository at:

  git://people.freedesktop.org/~jwrdegoede/xserver for-server-1.19

for you to fetch changes up to 7b135f5e7d79622c0b922de8ee827a2556504d8f:

  xwayland: Transform pointer enter event coordinates (2016-10-26 11:51:38 
+0200)


Hans de Goede (1):
  xfree86: Xorg.wrap: Do not require root rights for cards with 0 outputs

Michel Dänzer (1):
  DRI2: Sync radeonsi_pci_ids.h from Mesa

Mihail Konev (2):
  xwin: make glx optional again
  modesetting: fix glamor ifdef

Nikhil Mahale (1):
  modesetting: unifdef MODESETTING_OUTPUT_SLAVE_SUPPORT

Rui Matos (1):
  xwayland: Transform pointer enter event coordinates

 configure.ac |  4 ++--
 hw/xfree86/dri2/pci_ids/radeonsi_pci_ids.h   | 12 
 hw/xfree86/drivers/modesetting/driver.c  |  2 ++
 hw/xfree86/drivers/modesetting/drmmode_display.c |  2 --
 hw/xfree86/xorg-wrapper.c|  2 +-
 hw/xwayland/xwayland-input.c |  5 -
 6 files changed, 21 insertions(+), 6 deletions(-)

Regards,

Hans
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver v2] inputthread: On Linux leave the main thread's name as-is

[Hans de Goede]

From: Peter Hutterer <peter.hutte...@who-t.net>

On Linux, setting the main thread's name changes the program name
(/proc/self/comm). Setting it to MainThread breaks scripts that rely on
the command name, e.g. ps -C Xorg.

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2 (hdegoede):
-Only leave the main thread as-is in Linux, naming it is not an issue on
 other platforms
---
 os/inputthread.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/os/inputthread.c b/os/inputthread.c
index ddafa7f..8e7f2ed 100644
--- a/os/inputthread.c
+++ b/os/inputthread.c
@@ -431,11 +431,13 @@ InputThreadPreInit(void)
 }
 hotplugPipeWrite = hotplugPipe[1];
 
+#ifndef __linux__ /* Linux does not deal well with renaming the main thread */
 #if defined(HAVE_PTHREAD_SETNAME_NP_WITH_TID)
 pthread_setname_np (pthread_self(), "MainThread");
 #elif defined(HAVE_PTHREAD_SETNAME_NP_WITHOUT_TID)
 pthread_setname_np ("MainThread");
 #endif
+#endif
 
 }
 
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] ramdac: Check sPriv != NULL in xf86CheckHWCursor()

[Hans de Goede]

Hi,

On 25-10-16 00:25, Alex Goins wrote:

xf86CheckHWCursor() would dereference sPriv without NULL checking it. If Option
"SWCursor" is specified, sPriv == NULL. In this case we should assume that HW
cursors are not supported.

Signed-off-by: Alex Goins <ago...@nvidia.com>
Reviewed-by: Andy Ritger <arit...@nvidia.com>


Patch looks good to me:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 hw/xfree86/ramdac/xf86HWCurs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
index da2b181..5e99526 100644
--- a/hw/xfree86/ramdac/xf86HWCurs.c
+++ b/hw/xfree86/ramdac/xf86HWCurs.c
@@ -148,7 +148,8 @@ xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, 
xf86CursorInfoPtr infoPtr
 continue;

 sPriv = dixLookupPrivate(>devPrivates, xf86CursorScreenKey);
-if (!xf86ScreenCheckHWCursor(pSlave, cursor, sPriv->CursorInfoPtr))
+if (!sPriv ||
+!xf86ScreenCheckHWCursor(pSlave, cursor, sPriv->CursorInfoPtr))
 return FALSE;
 }
 return TRUE;


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] modesetting: unifdef MODESETTING_OUTPUT_SLAVE_SUPPORT

[Hans de Goede]

Hi,

On 22-10-16 14:10, Nikhil Mahale wrote:

Commit c7e8d4a6ee9542f56cd241cf7a960fb8223a6b22 had already
unifdef MODESETTING_OUTPUT_SLAVE_SUPPORT but commit
9257b1252da9092ddc676fec9aabe2b33dfad272 didn't
notice that.

Signed-off-by: Nikhil Mahale <nmah...@nvidia.com>


Looks good to me:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 hw/xfree86/drivers/modesetting/drmmode_display.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 61a0e27..6e755e9 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -1701,10 +1701,8 @@ drmmode_create_name(ScrnInfoPtr pScrn, 
drmModeConnectorPtr koutput, char *name,
  fallback:
 if (koutput->connector_type >= MS_ARRAY_SIZE(output_names))
 snprintf(name, 32, "Unknown%d-%d", koutput->connector_type, 
koutput->connector_type_id);
-#ifdef MODESETTING_OUTPUT_SLAVE_SUPPORT
 else if (pScrn->is_gpu)
 snprintf(name, 32, "%s-%d-%d", output_names[koutput->connector_type], 
pScrn->scrnIndex - GPU_SCREEN_OFFSET + 1, koutput->connector_type_id);
-#endif
 else
 snprintf(name, 32, "%s-%d", output_names[koutput->connector_type], 
koutput->connector_type_id);
 }


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] xwayland: Activate and enable touch devices

[Hans de Goede]

HI,

On 21-10-16 10:11, Olivier Fourdan wrote:

On some random condition, a touch event may trigger a crash in Xwayland
in GetTouchEvents().

The (simplified) backtrace goes as follow:

 (gdb) bt
 #0  GetTouchEvents() at getevents.c:1892
 #1  QueueTouchEvents() at getevents.c:1866
 #2  xwl_touch_send_event() at xwayland-input.c:652
 #5  wl_closure_invoke() from libwayland-client.so.0
 #6  dispatch_event() from libwayland-client.so.0
 #7  wl_display_dispatch_queue_pending() from libwayland-client.so.0
 #8  xwl_read_events() at xwayland.c:483
 #9  ospoll_wait() at ospoll.c:412
 #10 WaitForSomething() at WaitFor.c:222
 #11 Dispatch() at dispatch.c:412
 #12 dix_main() at main.c:287
 #13 __libc_start_main() at libc-start.c:289
 #14 _start ()

The crash occurs when trying to access the sprite associated with the
touch device, which appears to be NULL. Reason being the device itself
is more a keyboard device than a touch device.

Moreover, it appears the device is neither enabled nor activated
(inited=0, enabled=0) which doesn't seem right, but matches the code in
init_touch() from xwayland-input.c which would enable the device if it
was previously existing and otherwise would create the device but not
activate it.

Make sure we do activate and enable touch devices just like we do for
other input devices such as keyboard and pointer.

Signed-off-by: Olivier Fourdan <ofour...@redhat.com>


Patch LGTM:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans





---
 hw/xwayland/xwayland-input.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
index 4d447a5..9a7123a 100644
--- a/hw/xwayland/xwayland-input.c
+++ b/hw/xwayland/xwayland-input.c
@@ -1056,12 +1056,13 @@ init_touch(struct xwl_seat *xwl_seat)
 wl_touch_add_listener(xwl_seat->wl_touch,
   _listener, xwl_seat);

-if (xwl_seat->touch)
-EnableDevice(xwl_seat->touch, TRUE);
-else {
+if (xwl_seat->touch == NULL) {
 xwl_seat->touch =
 add_device(xwl_seat, "xwayland-touch", xwl_touch_proc);
+ActivateDevice(xwl_seat->touch, TRUE);
 }
+EnableDevice(xwl_seat->touch, TRUE);
+
 }

 static void


___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH evdev] Fix off-by-one error counting axes

[Hans de Goede]

Hi,

On 21-10-16 01:55, Peter Hutterer wrote:

We stopped counting one too early, but still initialized that axis later,
leading to a bug macro to trigger.

https://bugs.freedesktop.org/show_bug.cgi?id=97956

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>


LGTM:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
 src/evdev.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/evdev.c b/src/evdev.c
index 5ace238..96fd97d 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -1238,7 +1238,7 @@ EvdevCountMTAxes(EvdevPtr pEvdev, int *num_mt_axes_total,
 return;

 /* Absolute multitouch axes: adjust mapping and axes counts. */
-for (axis = ABS_MT_SLOT; axis < ABS_MAX; axis++)
+for (axis = ABS_MT_SLOT; axis <= ABS_MAX; axis++)
 {
 int j;
 Bool skip = FALSE;
@@ -1288,7 +1288,7 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device, int 
num_scroll_axes)
 goto out;

 /* Find number of absolute axis, including MT ones, will decrease later. */
-for (i = 0; i < ABS_MAX; i++)
+for (i = 0; i <= ABS_MAX; i++)
 if (libevdev_has_event_code(pEvdev->dev, EV_ABS, i))
 num_axes++;

@@ -1456,7 +1456,7 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device, int 
num_scroll_axes)
 }

 for (i = 0; i < num_touches; i++) {
-for (axis = ABS_MT_TOUCH_MAJOR; axis < ABS_MAX; axis++) {
+for (axis = ABS_MT_TOUCH_MAJOR; axis <= ABS_MAX; axis++) {
 if (pEvdev->abs_axis_map[axis] >= 0) {
 int val = pEvdev->mtdev ? 0 : 
libevdev_get_current_slot(pEvdev->dev);
 /* XXX: read initial values from mtdev when it adds support
@@ -1669,7 +1669,7 @@ EvdevAddRelValuatorClass(DeviceIntPtr device, int 
num_scroll_axes)
 if (!libevdev_has_event_type(pEvdev->dev, EV_REL))
 goto out;

-for (i = 0; i < REL_MAX; i++) {
+for (i = 0; i <= REL_MAX; i++) {
 if (i == REL_WHEEL || i == REL_HWHEEL || i == REL_DIAL)
 continue;



___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 3/3] ddx: add new call to purge input devices that weren't added

[Hans de Goede]

Hi,

On 20-10-16 23:50, Peter Hutterer wrote:

Special case for the systemd-logind case in xfree86: when we're vt-switched
away and a device is plugged in, we get a paused fd from logind. Since we
can't probe the device or do anything with it, we store that device in the
xfree86 and handle it later when we vt-switch back. The device is not added to
inputInfo.devices until that time.

When the device is removed while still vt-switched away, the the config system
never notifies the DDX. It only runs through inputInfo.devices and our device
was never added to that.

When a device is plugged in, removed, and plugged in again while vt-switched
away, we have two entries in the xfree86-specific list that refer to the same
device node, both pending for addition later. On VT switch back, the first one
(the already removed one) will be added successfully, the second one (the
still plugged-in one) fails. Since the fd is correct, the device works until
it is removed again. The removed devices' config_info (i.e. the syspath)
doesn't match the actual device we addded tough (the input number increases
with each plug), it doesn't get removed, the fd remains open and we lose track
of the fd count. Plugging the device in again leads to a dead device.

Fix this by adding a call to notify the DDX to purge any remainders of devices
with the given config_info, that's the only identifiable bit we have at this
point.

https://bugs.freedesktop.org/show_bug.cgi?id=97928

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
 Xi/stubs.c | 14 ++
 config/config.c|  2 ++
 hw/dmx/dmxinput.c  |  5 +
 hw/kdrive/src/kinput.c |  5 +
 hw/xfree86/common/xf86Xinput.c | 19 +++
 hw/xquartz/darwinXinput.c  | 15 +++
 include/input.h|  1 +
 7 files changed, 61 insertions(+)

diff --git a/Xi/stubs.c b/Xi/stubs.c
index 39bee7c..27848a2 100644
--- a/Xi/stubs.c
+++ b/Xi/stubs.c
@@ -143,3 +143,17 @@ DeleteInputDeviceRequest(DeviceIntPtr dev)
 {
 RemoveDevice(dev, TRUE);
 }
+
+/
+ *
+ * Caller: configRemoveDevice (and others)
+ *
+ * Remove any traces of the input device specified in config_info.
+ * This is only necessary if the ddx keeps information around beyond
+ * the NewInputDeviceRequest/DeleteInputDeviceRequest
+ *
+ */
+void
+RemoveInputDeviceTraces(const char *config_info)
+{
+}
diff --git a/config/config.c b/config/config.c
index 1fb368c..fb60295 100644
--- a/config/config.c
+++ b/config/config.c
@@ -107,6 +107,8 @@ remove_devices(const char *backend, const char *config_info)
 if (dev->config_info && strcmp(dev->config_info, config_info) == 0)
 remove_device(backend, dev);
 }
+
+RemoveInputDeviceTraces(config_info);
 }

 BOOL
diff --git a/hw/dmx/dmxinput.c b/hw/dmx/dmxinput.c
index 4ccb439..d201034 100644
--- a/hw/dmx/dmxinput.c
+++ b/hw/dmx/dmxinput.c
@@ -123,3 +123,8 @@ void
 DeleteInputDeviceRequest(DeviceIntPtr pDev)
 {
 }
+
+void
+RemoveInputDeviceTraces(const char *config_info)
+{
+}
diff --git a/hw/kdrive/src/kinput.c b/hw/kdrive/src/kinput.c
index 2c39624..8b08747 100644
--- a/hw/kdrive/src/kinput.c
+++ b/hw/kdrive/src/kinput.c
@@ -2302,3 +2302,8 @@ DeleteInputDeviceRequest(DeviceIntPtr pDev)
 {
 RemoveDevice(pDev, TRUE);
 }
+
+void
+RemoveInputDeviceTraces(const char *config_info)
+{
+}
diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
index 0095272..d673711 100644
--- a/hw/xfree86/common/xf86Xinput.c
+++ b/hw/xfree86/common/xf86Xinput.c
@@ -1119,6 +1119,25 @@ DeleteInputDeviceRequest(DeviceIntPtr pDev)
 input_unlock();
 }

+void
+RemoveInputDeviceTraces(const char *config_info)
+{
+PausedInputDevicePtr d, tmp;
+
+if (new_input_devices_list.next == NULL &&
+new_input_devices_list.prev == NULL)
+xorg_list_init(_input_devices_list);


This bit is unnecessary, as you already initialize
new_input_devices_list when you declare it at the top of the file.

With these 3 lines dropped the entire series looks good to me and is:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans




+
+xorg_list_for_each_entry_safe(d, tmp, _input_devices_list, node) {
+const char *ci = xf86findOptionValue(d->pInfo->options, "config_info");
+if (!ci || strcmp(ci, config_info) != 0)
+continue;
+
+xorg_list_del(>node);
+free(d);
+}
+}
+
 /*
  * convenient functions to post events
  */
diff --git a/hw/xquartz/darwinXinput.c b/hw/xquartz/darwinXinput.c
index 3efaa2c..fea7e92 100644
--- a/hw/xquartz/darwinXinput.c
+++ b/hw/xquartz/darwinXinput.c
@@ -147,3 +147,18 @@ DeleteInputDeviceRequest(DeviceIntPtr dev)
 {
 DEBUG_LOG("DeleteInputDeviceRequest(%p)\n", dev);
 }
+
+/

[PATCH xf86-video-amdgpu] amdgpu_probe: Do not close server managed drm fds

[Hans de Goede]

This fixes the xserver only seeing AMD/ATI devices supported by the amdgpu
driver, as by the time xf86-video-ati gets a chance to probe them, the
fd has been closed.

This fixes e.g. Xorg not seeing the dGPU on a Lenovo Thinkpad E465 laptop
with a CARRIZO iGPU and a HAINAN dGPU.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 src/amdgpu_probe.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 213d245..5d17fe5 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -142,6 +142,15 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn, char 
*busid,
return fd;
 }
 
+static void amdgpu_kernel_close_fd(AMDGPUEntPtr pAMDGPUEnt)
+{
+#ifdef XF86_PDEV_SERVER_FD
+   if (!(pAMDGPUEnt->platform_dev &&
+ pAMDGPUEnt->platform_dev->flags & XF86_PDEV_SERVER_FD))
+#endif
+   drmClose(pAMDGPUEnt->fd);
+}
+
 static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn, AMDGPUEntPtr pAMDGPUEnt,
   char *busid)
 {
@@ -164,7 +173,7 @@ static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn, 
AMDGPUEntPtr pAMDGPUEnt,
if (err != 0) {
xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
   "[drm] failed to set drm interface version.\n");
-   drmClose(pAMDGPUEnt->fd);
+   amdgpu_kernel_close_fd(pAMDGPUEnt);
return FALSE;
}
 
@@ -252,7 +261,7 @@ static Bool amdgpu_get_scrninfo(int entity_num, struct 
pci_device *pci_dev)
return TRUE;
 
 error_amdgpu:
-   drmClose(pAMDGPUEnt->fd);
+   amdgpu_kernel_close_fd(pAMDGPUEnt);
 error_fd:
free(pPriv->ptr);
 error:
@@ -347,6 +356,7 @@ amdgpu_platform_probe(DriverPtr pDriver,
 
pPriv->ptr = xnfcalloc(sizeof(AMDGPUEntRec), 1);
pAMDGPUEnt = pPriv->ptr;
+   pAMDGPUEnt->platform_dev = dev;
pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, busid, dev);
if (pAMDGPUEnt->fd < 0)
goto error_fd;
@@ -365,7 +375,6 @@ amdgpu_platform_probe(DriverPtr pDriver,
pAMDGPUEnt = pPriv->ptr;
pAMDGPUEnt->fd_ref++;
}
-   pAMDGPUEnt->platform_dev = dev;
 
xf86SetEntityInstanceForScreen(pScrn, pEnt->index,
   xf86GetNumEntityInstances(pEnt->
@@ -377,7 +386,7 @@ amdgpu_platform_probe(DriverPtr pDriver,
return TRUE;
 
 error_amdgpu:
-   drmClose(pAMDGPUEnt->fd);
+   amdgpu_kernel_close_fd(pAMDGPUEnt);
 error_fd:
free(pPriv->ptr);
 error:
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] xfree86: Xorg.wrap: Do not require root rights for cards with 0 outputs

[Hans de Goede]

Prior to this commit the Xorg.wrap code to detect if root rights
are necessary checked for DRM_IOCTL_MODE_GETRESOURCES succeeding *and*
reporting more then 0 output connectors.

DRM_IOCTL_MODE_GETRESOURCES succeeding alone is enough to differentiate
between old drm only cards (which need ums and thus root) and kms capable
cards.

Some hybrid gfx laptops have 0 output connectors on one of their 2 GPUs,
resulting in Xorg needlessly running as root. This commits removes the
res.count_connectors > 0 check, fixing this.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 hw/xfree86/xorg-wrapper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/xfree86/xorg-wrapper.c b/hw/xfree86/xorg-wrapper.c
index d930962..a25e6ff 100644
--- a/hw/xfree86/xorg-wrapper.c
+++ b/hw/xfree86/xorg-wrapper.c
@@ -240,7 +240,7 @@ int main(int argc, char *argv[])
 
 memset(, 0, sizeof(struct drm_mode_card_res));
 r = ioctl(fd, DRM_IOCTL_MODE_GETRESOURCES, );
-if (r == 0 && res.count_connectors > 0)
+if (r == 0)
 kms_cards++;
 
 close(fd);
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel