Re: [PATCH xserver] modesetting: always set a hardware cursor when requested to load one.

2016-09-27 Thread Michel Dänzer 
On 27/09/16 10:54 PM, Michael Thayer wrote:
> On 27.09.2016 12:06, Hans de Goede wrote:
>> On 23-09-16 10:20, Hans de Goede wrote:
>>> On 09/16/2016 06:52 PM, Michael Thayer wrote:
 When the X server asks us to load a hardware cursor, that
 request is always followed up by a request to show it if we
 report success, or to hide it if we report failure.  Therefore
 it makes no sense to suppress the request if the cursor is not
 currently visible.
>>> 
>>> Ok, I've just spend the last half hour tracing (using grep) all 
>>> callers of load_cursor_argb_check and you're right, either the
>>> cursor is already visible (XRecolorCursor does this), or it will
>>> get shown directly after the drmmode_load_cursor_argb_check()
>>> call.
>> 
>> And despite double checking I still missed a trouble-some scenario
>> here.
>> 
>> If we've 2 monitors active, then as the cursor moves from one to 
>> the other show()/hide() gets called on them.
>> 
>> If the cursor then changes load_cursor_argb_check() gets called on 
>> all active crtc-s causing the cursor to now be shown on both
>> monitors at the same time, not good.
> 
> Indeed.  Looking at other ways of handling this, but the whole thing
> is somewhat tricky due to the differences in how the X server and
> the kernel handle cursor setting.  I think that the most correct way
> to fix this would be to allow show and hide cursor operations to
> return failure too, but that would be a very invasive change.  A less
> invasive but also less clean change would be to extend the first time
> hack to also trigger if a load happens when the cursor is hidden
> globally (or perhaps better, if it is hidden on all crtc-s at the
> time of the load).  For VirtualBox, if a cursor load succeeds on one
> crtc we know it will succeed on any, but I wonder whether that would
> apply to other users.  I also wonder which other potential users
> there are.  So far most other people use their own DDX instead of
> modesetting.  Alexandre, did you say this was an issue for Tegra?

IIRC the issue with Tegra is that it can't use the HW cursor at all for
some reason, not that it sometimes can and sometimes can't.


> And it is potentially interesting for Qemu for the same reasons that 
> it is for us (relative input device support).

TBH, I feel like the issues you're having with VirtualBox would be
better dealt with at the virtual GPU hardware level, by making the HW
cursor work reliably for guests and making it the hypervisor's
responsibility to draw the cursor on the host side when the HW cursor
can't be used there.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
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] glamor: Fall back to software for CopyPlane if we need to

2016-09-27 Thread Michel Dänzer 
On 28/09/16 06:15 AM, Adam Jackson wrote:
> glUniform4ui is available starting in GL{,ES} 3.0. Technically it's
> also in EXT_gpu_shader4, but that's not worth supporting. There was also
> a MESA_shading_language_130 spec proposed at one point; if that ever
> gets finished, we can update epoxy to know about it and fix up the
> feature check.
> 
> Signed-off-by: Adam Jackson 

[...]

> diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c
> index 5b5f0e6..8a329d2 100644
> --- a/glamor/glamor_copy.c
> +++ b/glamor/glamor_copy.c
> @@ -351,6 +351,9 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src,
>  if (!glamor_set_alu(screen, gc ? gc->alu : GXcopy))
>  goto bail_ctx;
>  
> +if (bitplane && !glamor_priv->can_copyplane)
> +goto bail_ctx;
> +
>  if (bitplane) {
>  prog = _priv->copy_plane_prog;
>  copy_facet = _facet_copyplane;

I'd add the if (!glamor_priv->can_copyplane) inside the existing if
(bitplane). Either way though,

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
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: Fall back to software for CopyPlane if we need to

2016-09-27 Thread Adam Jackson 
glUniform4ui is available starting in GL{,ES} 3.0. Technically it's
also in EXT_gpu_shader4, but that's not worth supporting. There was also
a MESA_shading_language_130 spec proposed at one point; if that ever
gets finished, we can update epoxy to know about it and fix up the
feature check.

Signed-off-by: Adam Jackson 
---
 glamor/glamor.c  | 2 ++
 glamor/glamor_copy.c | 3 +++
 glamor/glamor_priv.h | 1 +
 3 files changed, 6 insertions(+)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index 45cc095..7b39536 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -616,6 +616,8 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 glamor_priv->is_core_profile =
 gl_version >= 31 && !epoxy_has_gl_extension("GL_ARB_compatibility");
 
+glamor_priv->can_copyplane = (gl_version >= 30);
+
 glamor_setup_debug_output(screen);
 
 glamor_priv->use_quads = (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) &&
diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c
index 5b5f0e6..8a329d2 100644
--- a/glamor/glamor_copy.c
+++ b/glamor/glamor_copy.c
@@ -351,6 +351,9 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src,
 if (!glamor_set_alu(screen, gc ? gc->alu : GXcopy))
 goto bail_ctx;
 
+if (bitplane && !glamor_priv->can_copyplane)
+goto bail_ctx;
+
 if (bitplane) {
 prog = _priv->copy_plane_prog;
 copy_facet = _facet_copyplane;
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 2e491a6..27f9552 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -201,6 +201,7 @@ typedef struct glamor_screen_private {
 Bool has_dual_blend;
 Bool has_texture_swizzle;
 Bool is_core_profile;
+Bool can_copyplane;
 int max_fbo_size;
 
 GLuint one_channel_format;
-- 
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/2] xwayland: Apply "last pointer window" check only to the pointer device

2016-09-27 Thread Carlos Garnacho 
The checks in xwayland's XYToWindow handler pretty much assumes that the
sprite is managed by the wl_pointer, which is not entirely right, given
1) The Virtual Core Pointer may be controlled from other interfaces, and
2) there may be other SpriteRecs than the VCP's.

This makes XYToWindow calls return a sprite trace with just the root
window if any of those two assumptions are broken, eg. on touch events.

So turn the check upside down, first assume that the default XYToWindow
proc behavior is right, and later cut down the spriteTrace if the
current device happens to be the pointer. We work our way to the
device's lastSlave here so as not to break assumption #1 above.

Also, simplify the actual nested call to XYToWindow, the method pointer
in ScreenPtr was reinstaurated before calling, and moved back to
xwl_screen domain afterwards. This seems kind of pointless, as we have
the function pointer anyway.

Signed-off-by: Carlos Garnacho 
---
 hw/xwayland/xwayland-input.c | 64 ++--
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
index 32cfb35..9b385dc 100644
--- a/hw/xwayland/xwayland-input.c
+++ b/hw/xwayland/xwayland-input.c
@@ -945,43 +945,55 @@ DDXRingBell(int volume, int pitch, int duration)
 {
 }
 
-static WindowPtr
-xwl_xy_to_window(ScreenPtr screen, SpritePtr sprite, int x, int y)
+static DeviceIntPtr
+sprite_get_last_active_device(SpritePtr sprite, struct xwl_seat **xwl_seat)
 {
-struct xwl_seat *xwl_seat = NULL;
 DeviceIntPtr device;
-WindowPtr ret;
 
 for (device = inputInfo.devices; device; device = device->next) {
-if (device->deviceProc == xwl_pointer_proc &&
-device->spriteInfo->sprite == sprite) {
-xwl_seat = device->public.devicePrivate;
+/* Ignore non-wayland devices */
+if (device->deviceProc != xwl_pointer_proc &&
+device->deviceProc != xwl_touch_proc &&
+device->deviceProc != xwl_keyboard_proc)
+continue;
+
+if (device->spriteInfo->sprite == sprite)
 break;
-}
 }
 
-if (xwl_seat == NULL) {
-sprite->spriteTraceGood = 1;
-return sprite->spriteTrace[0];
-}
+if (!device || IsFloating(device))
+return NULL;
 
-screen->XYToWindow = xwl_seat->xwl_screen->XYToWindow;
-ret = screen->XYToWindow(screen, sprite, x, y);
-xwl_seat->xwl_screen->XYToWindow = screen->XYToWindow;
-screen->XYToWindow = xwl_xy_to_window;
+*xwl_seat = device->public.devicePrivate;
 
-/* If the pointer has left the Wayland surface but the DIX still
- * finds the pointer within the previous X11 window, it means that
- * the pointer has crossed to another native Wayland window, in this
- * case, pretend we entered the root window so that a LeaveNotify
- * event is emitted.
+/* We do want the last active slave, we only check on slave xwayland 
devices
+ * so we can find out the xwl_seat, but those don't actually own their
+ * sprite, so the match doesn't mean a lot.
  */
-if (xwl_seat->focus_window == NULL && xwl_seat->last_xwindow == ret) {
-sprite->spriteTraceGood = 1;
-return sprite->spriteTrace[0];
-}
+return device->master->lastSlave;
+}
 
-xwl_seat->last_xwindow = ret;
+static WindowPtr
+xwl_xy_to_window(ScreenPtr screen, SpritePtr sprite, int x, int y)
+{
+struct xwl_seat *xwl_seat = NULL;
+struct xwl_screen *xwl_screen;
+DeviceIntPtr device;
+WindowPtr ret;
+
+xwl_screen = xwl_screen_get(screen);
+ret = xwl_screen->XYToWindow(screen, sprite, x, y);
+
+device = sprite_get_last_active_device(sprite, _seat);
+
+if (device && xwl_seat && xwl_seat->pointer == device) {
+if (xwl_seat->focus_window == NULL && xwl_seat->last_xwindow == ret) {
+sprite->spriteTraceGood = 1;
+return sprite->spriteTrace[0];
+}
+
+xwl_seat->last_xwindow = ret;
+}
 
 return ret;
 }
-- 
2.10.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

[PATCH xserver 0/2] xwayland touch handling fixes

2016-09-27 Thread Carlos Garnacho 
Hi!

I'm sending a couple of fixes to fix touch handling in wayland. The
first patch makes the XYToWindow proc handler friendlier to touch-driven
pointer emulation (and possibly other pointer-driving devices in the
future with standalone foci in wayland, like tablets).

The second patch makes the coordinate space consistent in xwayland, so
it's not affected by changes in output configuration. It makes touch
events work through output geometry/mode changes.

Cheers,
  Carlos

___
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] xwayland: Apply touch abs axes transformation before posting events

2016-09-27 Thread Carlos Garnacho 
The way we map the touch absolute device to screen coordinates can't
work across wl_output mode and geometry events. Instead, set up
a fixed coordinate space, and transform touch events according to
the screen coordinate space as they happen.

Signed-off-by: Carlos Garnacho 
---
 hw/xwayland/xwayland-input.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
index 9b385dc..678cc7e 100644
--- a/hw/xwayland/xwayland-input.c
+++ b/hw/xwayland/xwayland-input.c
@@ -167,7 +167,6 @@ xwl_touch_proc(DeviceIntPtr device, int what)
 #define NTOUCHPOINTS 20
 #define NBUTTONS 1
 #define NAXES 2
-struct xwl_seat *xwl_seat = device->public.devicePrivate;
 Atom btn_labels[NBUTTONS] = { 0 };
 Atom axes_labels[NAXES] = { 0 };
 BYTE map[NBUTTONS + 1] = { 0 };
@@ -191,13 +190,10 @@ xwl_touch_proc(DeviceIntPtr device, int what)
 return BadValue;
 
 /* Valuators */
-/* FIXME: devices might be mapped to a single wl_output */
 InitValuatorAxisStruct(device, 0, axes_labels[0],
-   0, xwl_seat->xwl_screen->width,
-   1, 0, 1, Absolute);
+   0, 0x, 1, 0, 1, Absolute);
 InitValuatorAxisStruct(device, 1, axes_labels[1],
-   0, xwl_seat->xwl_screen->height,
-   1, 0, 1, Absolute);
+   0, 0x, 1, 0, 1, Absolute);
 return Success;
 
 case DEVICE_ON:
@@ -640,15 +636,18 @@ static void
 xwl_touch_send_event(struct xwl_touch *xwl_touch,
  struct xwl_seat *xwl_seat, int type)
 {
-int32_t dx, dy;
+double dx, dy, x, y;
 ValuatorMask mask;
 
 dx = xwl_touch->window->window->drawable.x;
 dy = xwl_touch->window->window->drawable.y;
 
+x = (dx + xwl_touch->x) * 0x / xwl_seat->xwl_screen->width;
+y = (dy + xwl_touch->y) * 0x / xwl_seat->xwl_screen->height;
+
 valuator_mask_zero();
-valuator_mask_set(, 0, dx + xwl_touch->x);
-valuator_mask_set(, 1, dy + xwl_touch->y);
+valuator_mask_set_double(, 0, x);
+valuator_mask_set_double(, 1, y);
 QueueTouchEvents(xwl_seat->touch, type, xwl_touch->id, 0, );
 }
 
-- 
2.10.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: [PATCH xserver] modesetting: always set a hardware cursor when requested to load one.

2016-09-27 Thread Michael Thayer 

Hello,

On 27.09.2016 12:06, Hans de Goede wrote:

Hi,

On 23-09-16 10:20, Hans de Goede wrote:

Hi All,

On 09/16/2016 06:52 PM, Michael Thayer wrote:

When the X server asks us to load a hardware cursor, that request is
always
followed up by a request to show it if we report success, or to hide
it if
we report failure.  Therefore it makes no sense to suppress the
request if
the cursor is not currently visible.


Ok, I've just spend the last half hour tracing (using grep) all
callers of
load_cursor_argb_check and you're right, either the cursor is already
visible (XRecolorCursor does this), or it will get shown directly after
the drmmode_load_cursor_argb_check() call.


And despite double checking I still missed a trouble-some scenario here.

If we've 2 monitors active, then as the cursor moves from one to
the other show()/hide() gets called on them.

If the cursor then changes load_cursor_argb_check() gets called on
all active crtc-s causing the cursor to now be shown on both monitors
at the same time, not good.


Indeed.  Looking at other ways of handling this, but the whole thing is 
somewhat tricky due to the differences in how the X server and the 
kernel handle cursor setting.  I think that the most correct way to fix 
this would be to allow show and hide cursor operations to return failure 
too, but that would be a very invasive change.  A less invasive but also 
less clean change would be to extend the first time hack to also trigger 
if a load happens when the cursor is hidden globally (or perhaps better, 
if it is hidden on all crtc-s at the time of the load).  For VirtualBox, 
if a cursor load succeeds on one crtc we know it will succeed on any, 
but I wonder whether that would apply to other users.  I also wonder 
which other potential users there are.  So far most other people use 
their own DDX instead of modesetting.  Alexandre, did you say this was 
an issue for Tegra?  And it is potentially interesting for Qemu for the 
same reasons that it is for us (relative input device support).


Hans, any thoughts?  Probably better not to try to force anything into 1.19.

Regards and thanks,

Michael



So this patch is:

Reviewed-by: Hans de Goede 


So I hereby withdraw my Reviewed-by, and have to NACK this in its
current form as it is causing regressions in multi-monitor
setups.

I'll send a v2 my pull-req with fixes for 1.19, dropping this one
and related follow-up patches.

Regards,

Hans




Signed-off-by: Michael Thayer 
---
Tested that both hardware and software cursors still work after
applying the
patch.

 hw/xfree86/drivers/modesetting/drmmode_display.c | 11 +--
 hw/xfree86/drivers/modesetting/drmmode_display.h |  2 --
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 41810d9..7d171a3 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -813,14 +813,7 @@ drmmode_load_cursor_argb_check(xf86CrtcPtr crtc,
CARD32 *image)
 for (i = 0; i < ms->cursor_width * ms->cursor_height; i++)
 ptr[i] = image[i];  // cpu_to_le32(image[i]);

-if (drmmode_crtc->cursor_up ||
!drmmode_crtc->first_cursor_load_done) {
-Bool ret = drmmode_set_cursor(crtc);
-if (!drmmode_crtc->cursor_up)
-drmmode_hide_cursor(crtc);
-drmmode_crtc->first_cursor_load_done = TRUE;
-return ret;
-}
-return TRUE;
+return drmmode_set_cursor(crtc);
 }

 static void
@@ -830,7 +823,6 @@ drmmode_hide_cursor(xf86CrtcPtr crtc)
 drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
 drmmode_ptr drmmode = drmmode_crtc->drmmode;

-drmmode_crtc->cursor_up = FALSE;
 drmModeSetCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, 0,
  ms->cursor_width, ms->cursor_height);
 }
@@ -839,7 +831,6 @@ static void
 drmmode_show_cursor(xf86CrtcPtr crtc)
 {
 drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
-drmmode_crtc->cursor_up = TRUE;
 drmmode_set_cursor(crtc);
 }

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h
b/hw/xfree86/drivers/modesetting/drmmode_display.h
index 50976b8..bd82968 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.h
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
@@ -91,9 +91,7 @@ typedef struct {
 uint32_t vblank_pipe;
 int dpms_mode;
 struct dumb_bo *cursor_bo;
-Bool cursor_up;
 Bool set_cursor2_failed;
-Bool first_cursor_load_done;
 uint16_t lut_r[256], lut_g[256], lut_b[256];

 drmmode_bo rotate_bo;



--
Michael Thayer | VirtualBox engineer
ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstraße 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.

[xserver PULL for 1.19 v2] Various modesetting and other fixes

2016-09-27 Thread Hans de Goede 

Hi Adam, Keith,

Here is an updated pull-req dropping the troublesome
modesetting cursor patches, as well as the
"Simplify, statement is always true." patch which
has also seen some comments on the list, so needs a v2.

Here is the cover blurb of my original pull-req:

"
Here is a pull-req consisting of a bunch of fixes written by
me reviewed by others as well as a bunch of fixes from patchwork /
the list which I've picked up and which are reviewed by me.

Note there is one unreviewed patch in here v2 of:
"glx: Always enable EXT_texture_from_pixmap for DRI swrast glx"

But I believe that Adam and I are in agreement wrt v2, so that
one should be good to go too.
"

And here is the git reqeuest-pull output for the updated
pull-req:

The following changes since commit ba199cb90157cefab01183f2e2c909895df73321:

  glamor: Make glamor_sync_init work with --disable-xshmfence (2016-09-25 
11:00:24 -0700)

are available in the git repository at:

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

for you to fetch changes up to c95c38c3677c85697f1dd5ccd34412b57cb5afa0:

  XF86VidMode: Fix free() on walked pointer (2016-09-27 12:25:03 +0200)


Adam Jackson (1):
  xephyr: Don't crash if the server advertises zero xv adaptors

Daniel Martin (1):
  modesetting: Consume all available udev events at once

David CARLIER (1):
  xfree86: small memory leaks fixes

Hans de Goede (7):
  modesetting: Fix reverse prime partial update issues on secondary GPU 
outputs
  modesetting: Fix reverse prime update lagging on secondary GPU outputs
  xf86RandR12: Move calculating of shift inside init_one_component
  xf86RandR12: Fix XF86VidModeSetGamma triggering a BadImplementation error
  glx: Always enable EXT_texture_from_pixmap for DRI swrast glx
  Xext: Fix a memory leak
  XF86VidMode: Fix free() on walked pointer

Kyle Guinn (1):
  xfree86: Fix null pointer dereference

Laszlo Ersek (1):
  xfree86: recognize primary BUS_PCI device in xf86IsPrimaryPlatform()

Michael Thayer (1):
  modesetting: only fall back to drmModeSetCursor() on -EINVAL

Qiang Yu (1):
  config: fix GPUDevice fail when AutoAddGPU off + BusID

 Xext/shm.c   |  4 +--
 Xext/vidmode.c   |  2 +-
 glx/glxdriswrast.c   |  3 +-
 hw/kdrive/ephyr/ephyrvideo.c |  2 +-
 hw/xfree86/common/xf86platformBus.c  | 28 ---
 hw/xfree86/ddc/ddc.c |  2 +-
 hw/xfree86/drivers/modesetting/driver.c  | 29 +++
 hw/xfree86/drivers/modesetting/drmmode_display.c | 22 +++-
 hw/xfree86/i2c/xf86i2c.c |  8 ++---
 hw/xfree86/modes/xf86RandR12.c   | 45 
 hw/xfree86/utils/gtf/gtf.c   |  2 ++
 11 files changed, 110 insertions(+), 37 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

Re: [PATCH] remove dead code in dummy driver

2016-09-27 Thread Antoine Martin 
On 24/09/16 00:20, Aaron Plattner wrote:
> On 09/22/2016 04:30 PM, Bob Terek wrote:
>> On 09/21/2016 10:22 AM, Aaron Plattner wrote:
>>> On 09/20/2016 02:07 AM, Eric Engestrom wrote:
 On Tue, Sep 20, 2016 at 01:34:40PM +0700, Antoine Martin wrote:
> Signed-off-by: Antoine Martin 

 Reviewed-by: Eric Engestrom 
>>>
>>> Looks good to me too (although I'm cheating since this chunk is
>>> identical to part of
>>> https://patchwork.freedesktop.org/patch/41058/)
>>
>> Shouldn't the first 5 of Aaron's patches be applied, since they are all
>> cleanup items?
>>
>> https://lists.x.org/archives/xorg-devel/2015-January/045395.html
> 
> I never pushed them because they were never reviewed. Would it help if I
> resent them?
Yes, please re-send and I'll make sure to test and review this week.

>> Patch 6 supposedly caused a server crash, but the first 5 should be ok?
> 
> Patch 6 was kind of controversial so I don't know if we want it anyway.
IIRC, I was the one who reported a crash when I tested it - I didn't
investigate it further.

Sounds like Bob Terek's approach is much more complete anyway.

Cheers
Antoine



> 
>> -- 
>> Bob Terek
> ___
> 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: [xserver, 2/2] xf86RandR12: Fix XF86VidModeSetGamma triggering a BadImplementation error

2016-09-27 Thread Hans de Goede 

Hi,

On 25-09-16 09:51, Michel Dänzer wrote:

On 23/09/16 07:48 PM, Hans de Goede wrote:

On 09/23/2016 11:13 AM, Michel Dänzer wrote:

On 23/09/16 03:46 PM, Hans de Goede wrote:


Since this is a fallback which ideally should never get used (all
drivers should call xf86HandleColormaps) I believe it is not worth
the time to implement your (admittedly better) fallback suggestion.


Fine with me, but please at least change the comment to be more accurate
and clearer.


OK, the comment now reads:
/*
 * Compatibility pScrn->ChangeGamma provider for ddx drivers which do
not call
 * xf86HandleColormaps(). Note such drivers really should be fixed to call
 * xf86HandleColormaps() as this clobbers any per-crtc setup.
 */


As I mentioned before, "clobbers any per-CRTC setup" isn't accurate.
Only the gamma ramp of the CRTC assigned to the RandR compatibility
output is clobbered.


Ah I missed you had provided an improved comment and rolled my own.

I've fixed this in my personal tree now, and the fixed version
will be part of the updated pull-req for 1.19 I'm preparing.

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] remove dead code in dummy driver

2016-09-27 Thread Antoine Martin 
On 25/09/16 18:52, Bob Terek wrote:
> 
> On 09/23/2016 07:20 AM, Aaron Plattner wrote:
> 
>> On 09/22/2016 04:30 PM, Bob Terek wrote:
> 
>>> Shouldn't the first 5 of Aaron's patches be applied, since they are all
>>> cleanup items?
>>>
>>> https://lists.x.org/archives/xorg-devel/2015-January/045395.html
>>
>> I never pushed them because they were never reviewed. Would it help if I
>> resent them?
> 
> Why don't you hold off for a bit. . .
> 
> 
>>> Patch 6 supposedly caused a server crash, but the first 5 should be ok?
>>
>> Patch 6 was kind of controversial so I don't know if we want it anyway.
> 
> Welp, on that topic, your patch 6 was an alternative approach to support
> arbitrary pixel dimensions for the framebuffer. The more conventional
> approach had been posted by Boichat,
> https://lists.x.org/archives/xorg-devel/2014-November/044580.html . I'd
> like to suggest that Boichat's approach be used, but extended even further:
> 
> In 2014 I had also modified the dummy driver while working with Teradici
> Corporation to support not only arbitrary pixel dimensions, but also
> multiple monitors. The latter feature might not make sense to some
> folks, but if you have a server-side Xserver mapped to a hardware thin
> client sitting across a network, which has multiple physical monitors
> attached, you want the Xserver to be configured in the exact same manner
> as the tin client. Even though there is just a virtual framebuffer in
> main memory, X applications need to know the number/size/location of
> monitors so that toolbars are placed properly, windows are fullscreen'ed
> properly, etc. And when the user moves from one hardware thin client to
> another, which may have a different monitor configuration, the Xserver
> session needs to change to that configuration.
> 
> At Teradici we made dummy be RandR 1.2 compliant in a manner similar to
> Boichat, but added xorg.conf parameters to specify how many CRTC/Outputs
> should be created at startup. Thus the configuration could be
> manipulated via the standard RandR API (either programatically or
> through the xrandr command). This might happen when the hardware thin
> client connects to an existing X session, and server-side session
> management software will issue what is needed to reconfigure the
> Xserver. The end result is that the hardware thin client could be
> supported by multiple CRTC/Output's, with typical VESA modes, but a
> software thin client could use arbitrary pixel dimensions on a single
> CRTC/Output to map exactly the size of the window it was running in.
> 
> Teradici and I would like to submit these patches to dummy to support
> the functionality that many want. The dummy driver is so simple that I
> don't see this as overly complex. Yes, it is a bit more code to add, but
> it is very straightforward, nothing at all surprising. And it is
> backwards compatible, the multiple CRTC/Output support does not have to
> be utilized, the default is 1. Our modified driver behaves just like the
> current one, except that arbitrary modes may be added and switched to
> via the RandR protocol.
> 
> I can post these patches, and include Aaron's cleanups, if folks will
> support this approach. It has been tested via Teradici for over a year
> and seems to be stable. I am in the process of updating the code to the
> current version and so far so good.
> 
> Thoughts?
That sounds great, I'd be happy to test and review those changes.
It think it probably makes sense to apply the cleanup patches first, to
remove code before adding some more?
These RandR bits probably warrant bumping the version number too, 0.4.0?

I was going to add comments to Aaron's cleanup patches, but patchwork
won't let me add comments.

Cheers
Antoine


> 
> Thanks,
> 
> Bob Terek
> ___
> 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] modesetting: always set a hardware cursor when requested to load one.

2016-09-27 Thread Hans de Goede 

Hi,

On 23-09-16 10:20, Hans de Goede wrote:

Hi All,

On 09/16/2016 06:52 PM, Michael Thayer wrote:

When the X server asks us to load a hardware cursor, that request is always
followed up by a request to show it if we report success, or to hide it if
we report failure.  Therefore it makes no sense to suppress the request if
the cursor is not currently visible.


Ok, I've just spend the last half hour tracing (using grep) all callers of
load_cursor_argb_check and you're right, either the cursor is already
visible (XRecolorCursor does this), or it will get shown directly after
the drmmode_load_cursor_argb_check() call.


And despite double checking I still missed a trouble-some scenario here.

If we've 2 monitors active, then as the cursor moves from one to
the other show()/hide() gets called on them.

If the cursor then changes load_cursor_argb_check() gets called on
all active crtc-s causing the cursor to now be shown on both monitors
at the same time, not good.



So this patch is:

Reviewed-by: Hans de Goede 


So I hereby withdraw my Reviewed-by, and have to NACK this in its
current form as it is causing regressions in multi-monitor
setups.

I'll send a v2 my pull-req with fixes for 1.19, dropping this one
and related follow-up patches.

Regards,

Hans




Signed-off-by: Michael Thayer 
---
Tested that both hardware and software cursors still work after applying the
patch.

 hw/xfree86/drivers/modesetting/drmmode_display.c | 11 +--
 hw/xfree86/drivers/modesetting/drmmode_display.h |  2 --
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 41810d9..7d171a3 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -813,14 +813,7 @@ drmmode_load_cursor_argb_check(xf86CrtcPtr crtc, CARD32 
*image)
 for (i = 0; i < ms->cursor_width * ms->cursor_height; i++)
 ptr[i] = image[i];  // cpu_to_le32(image[i]);

-if (drmmode_crtc->cursor_up || !drmmode_crtc->first_cursor_load_done) {
-Bool ret = drmmode_set_cursor(crtc);
-if (!drmmode_crtc->cursor_up)
-drmmode_hide_cursor(crtc);
-drmmode_crtc->first_cursor_load_done = TRUE;
-return ret;
-}
-return TRUE;
+return drmmode_set_cursor(crtc);
 }

 static void
@@ -830,7 +823,6 @@ drmmode_hide_cursor(xf86CrtcPtr crtc)
 drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
 drmmode_ptr drmmode = drmmode_crtc->drmmode;

-drmmode_crtc->cursor_up = FALSE;
 drmModeSetCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, 0,
  ms->cursor_width, ms->cursor_height);
 }
@@ -839,7 +831,6 @@ static void
 drmmode_show_cursor(xf86CrtcPtr crtc)
 {
 drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
-drmmode_crtc->cursor_up = TRUE;
 drmmode_set_cursor(crtc);
 }

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h 
b/hw/xfree86/drivers/modesetting/drmmode_display.h
index 50976b8..bd82968 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.h
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
@@ -91,9 +91,7 @@ typedef struct {
 uint32_t vblank_pipe;
 int dpms_mode;
 struct dumb_bo *cursor_bo;
-Bool cursor_up;
 Bool set_cursor2_failed;
-Bool first_cursor_load_done;
 uint16_t lut_r[256], lut_g[256], lut_b[256];

 drmmode_bo rotate_bo;


___
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 modesetting and other fixes

2016-09-27 Thread Hans de Goede 

Hi,

On 23-09-16 15:09, Hans de Goede wrote:

Hi Adam, Keith,

Here is a pull-req consisting of a bunch of fixes written by
me reviewed by others as well as a bunch of fixes from patchwork /
the list which I've picked up and which are reviewed by me.

Note there is one unreviewed patch in here v2 of:
"glx: Always enable EXT_texture_from_pixmap for DRI swrast glx"

But I believe that Adam and I are in agreement wrt v2, so that
one should be good to go too.


Self NAK, one of the modesetting driver cursor patches causes
issues on multi-monitor setups I'll send out a revised pull-req later today.

Regards,

Hans





The following changes since commit d0c5d205a919fc1d2eb599356090b58b1bf0176d:

  dix: Make InitCoreDevices() failures more verbose. (2016-09-21 21:11:40 +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 01189c04a073447dc4eb474ec4cee7c59be5576e:

  Simplify, statement is always true. (2016-09-23 15:55:22 +0300)


Adam Jackson (1):
  xephyr: Don't crash if the server advertises zero xv adaptors

Daniel Martin (1):
  modesetting: Consume all available udev events at once

David CARLIER (1):
  xfree86: small memory leaks fixes

Hans de Goede (8):
  modesetting: Fix reverse prime partial update issues on secondary GPU 
outputs
  modesetting: Fix reverse prime update lagging on secondary GPU outputs
  xf86RandR12: Move calculating of shift inside init_one_component
  xf86RandR12: Fix XF86VidModeSetGamma triggering a BadImplementation error
  glx: Always enable EXT_texture_from_pixmap for DRI swrast glx
  modesetting: Do not call drmModeSetCursor2() twice on every cursor change
  Xext: Fix a memory leak
  XF86VidMode: Fix free() on walked pointer

Keith Packard (2):
  os: Ready clients with pending output aren't flushed, so set 
NewOutputPending
  os: Clear saved poll events in listen so that edge triggering works

Kyle Guinn (1):
  xfree86: Fix null pointer dereference

Laszlo Ersek (1):
  xfree86: recognize primary BUS_PCI device in xf86IsPrimaryPlatform()

Maya Rashish (1):
  Simplify, statement is always true.

Michael Thayer (3):
  modesetting: only fall back to drmModeSetCursor() on -EINVAL
  modesetting: always set a hardware cursor when requested to load one.
  modesetting: allow switching from software to hardware cursors (v4).

Qiang Yu (1):
  config: fix GPUDevice fail when AutoAddGPU off + BusID

 Xext/shm.c   |  4 +-
 Xext/vidmode.c   |  2 +-
 glx/glxdriswrast.c   |  3 +-
 hw/kdrive/ephyr/ephyrvideo.c |  2 +-
 hw/xfree86/common/xf86pciBus.c   |  6 +--
 hw/xfree86/common/xf86platformBus.c  | 28 +--
 hw/xfree86/ddc/ddc.c |  2 +-
 hw/xfree86/drivers/modesetting/driver.c  | 29 +---
 hw/xfree86/drivers/modesetting/drmmode_display.c | 60 +++-
 hw/xfree86/drivers/modesetting/drmmode_display.h |  2 -
 hw/xfree86/i2c/xf86i2c.c |  8 ++--
 hw/xfree86/modes/xf86RandR12.c   | 44 ++---
 hw/xfree86/utils/gtf/gtf.c   |  2 +
 os/io.c  |  3 +-
 os/ospoll.c  |  8 +++-
 15 files changed, 132 insertions(+), 71 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

Re: [PATCH xserver 02/10 v2] glamor: Require that 16bpp pixmap depths match for Render copies.

2016-09-27 Thread Michel Dänzer 
On 26/09/16 03:53 AM, Eric Anholt wrote:
> Michel Dänzer  writes:
>> On 23/09/16 09:51 PM, Eric Anholt wrote:
>>>
>>> We do need some concerted effort on actually fixing our rendering bugs
>>> and reenabling the skipped tests.  I've spent a while trying to come up
>>> with why the remaining rendercheck test fails and come up with nothing
>>> yet.
>>
>> How can others help with this?

[...]

> and rendercheck/cacomposite/all/a8:

FWIW, this test passes with radeonsi. The other tests you mentioned also
have failures with radeonsi, but different ones.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

___
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