Re: [PATCH xserver] glamor: Cannot use copies when accessing outside of composite source

2016-05-27 Thread Keith Packard 
Michel Dänzer  writes:

> You know the details better than I do, but my understanding is that the
> region can only be clipped to the destination in general, because
> accessing source pictures outside of their boundaries is defined such
> that it can change the contents of the destination.

Yeah, I didn't remember that the composite operation is supposed to do
source clipping as there may be a transform involved. However, we need
to make sure the rendered region is inside the source composite clip so
that the simple copy will work correctly.

> Anyway, the fact is that this patch fixes a fair number of rendercheck
> tests.

Here's a patch which uses region operations to make sure we don't use
copy when the source doesn't contain all of the necessary bits. Note
that this just means we'll fall back to the existing (broken) code which
doesn't bother to clip against the source either, but at least that's
only one bug instead of two?

|| (op == PictOpOver
&& source->format == dest->format
&& !PICT_FORMAT_A(source->format {
int dx, dy;

x_source += source->pDrawable->x;
y_source += source->pDrawable->y;
x_dest += dest->pDrawable->x;
y_dest += dest->pDrawable->y;
dx = x_source - x_dest;
dy = y_source - y_dest;

/* Align region with source */
pixman_region_translate(region, dx, dy);
box = RegionRects(region);
nbox = RegionNumRects(region);

for (i = 0; i < nbox; i++)
if (pixman_region_contains_rectangle(source->pCompositeClip, 
[i]) != PIXMAN_REGION_IN)
break;
pixman_region_translate(region, -dx, -dy);
if (i == nbox)
{
/* Re-align region with dest */
glamor_copy(source->pDrawable, dest->pDrawable, NULL,
RegionRects(region), RegionNumRects(region),
dx, dy, FALSE, FALSE, 0, NULL);
ok = TRUE;
goto out;
}


-- 
-keith


signature.asc
Description: PGP signature
___
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: Cannot use copies when accessing outside of composite source

2016-05-27 Thread Michel Dänzer 
On 28.05.2016 05:29, Keith Packard wrote:
> Michel Dänzer  writes:
>> From: Michel Dänzer 
>>
>> Commit b64108fa ("glamor: Check for composite operations which are
>> equivalent to copies") failed to copy conditions from exaComposite which
>> ensure that the composite operation doesn't access outside of the source
>> picture.
> 
> I'm confused -- the region to be copied should already have been clipped
> to the source, so there shouldn't be any access outside of the source
> picture.

You know the details better than I do, but my understanding is that the
region can only be clipped to the destination in general, because
accessing source pictures outside of their boundaries is defined such
that it can change the contents of the destination.

Anyway, the fact is that this patch fixes a fair number of rendercheck
tests.


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



signature.asc
Description: OpenPGP digital signature
___
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: Cannot use copies when accessing outside of composite source

2016-05-27 Thread Michel Dänzer 
On 26.05.2016 21:55, Hans de Goede wrote:
> On 26-05-16 12:04, Michel Dänzer wrote:
>> From: Michel Dänzer 
>>
>> Commit b64108fa ("glamor: Check for composite operations which are
>> equivalent to copies") failed to copy conditions from exaComposite which
>> ensure that the composite operation doesn't access outside of the source
>> picture.
>>
>> This fixes rendercheck regressions from the commit above.
>>
>> Signed-off-by: Michel Dänzer 
> 
> I was hoping that this fixed the thunderbird regressions I was seeing
> after commit b64108fa, but unfortunately it does not.
> 
>> Apologies for jumping the gun on the previous patch by submitting it
>> without appropriate testing with rendercheck.
>>
>> There is one regression left with this patch (blend test on a8), that
>> one is fixed by https://patchwork.freedesktop.org/patch/87222/ .
> 
> I've added that one to my local master clone too, still not fixed.
> 
> The problem is that thunderbird and any thunderbird dialogs simply
> show up as completely gray windows without anything in them.
> 
> Reproducing is as simple as starting thunderbird.
> 
> I'm seeing this on a skylake system using the igpu with modesetting +
> glamor.
> 
> For now I'm running xserver-master with b64108fa reverted, let me know
> if you need to me test any potential fixes for this.

Does Keith's commit 8b9b4387 ("glamor: Adjust for drawable x/y in
composite's copy optimization") fix this (possibly combined with this
patch)?


-- 
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 17/23] os: Add ospoll interface

2016-05-27 Thread Keith Packard 
Emil Velikov  writes:

> Hi Keith,
>
> Style question: do you/how many others refer having separate functions for
> of ifdeffed code each vs a single with all the ifdeffs. Or in other words
> having func_foo_poll and func_foo_epoll as opposed to having it all in
> func_foo.

I started having separate versions of each function (with the same
name), but that seemed bad, so I merged the contents of the separate
functions into the single functions you see now. I found it easier to
compare the semantics of the different implementations when they were
right next to each other.

> +#ifndef _OSPOLL_H_
>> +#define _OSPOLL_H_
>> +
>> +#include 
>> +
>
> This include should be in the C file with proper ifdef guard, right?

I'm not sure I understand -- we need  on every system as ospoll
uses the POLLIN/POLLOUT defines for its arguments. We could use X server
specific values, but I think the posix ones are common across all of our
supported systems.

-- 
-keith


signature.asc
Description: PGP signature
___
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] xfree86: Remove event reading code from xf86Wakeup

2016-05-27 Thread Keith Packard 
Oops. This didn't get removed when xfree86 was converted over to use
the input thread.

Signed-off-by: Keith Packard 
---
 hw/xfree86/common/xf86Events.c | 28 
 1 file changed, 28 deletions(-)

diff --git a/hw/xfree86/common/xf86Events.c b/hw/xfree86/common/xf86Events.c
index 7191980..5db0dfb 100644
--- a/hw/xfree86/common/xf86Events.c
+++ b/hw/xfree86/common/xf86Events.c
@@ -247,34 +247,6 @@ xf86ProcessActionEvent(ActionEvent action, void *arg)
 void
 xf86Wakeup(void *blockData, int err, void *pReadmask)
 {
-fd_set *LastSelectMask = (fd_set *) pReadmask;
-fd_set devicesWithInput;
-InputInfoPtr pInfo;
-
-if (err >= 0) {
-
-XFD_ANDSET(, LastSelectMask, );
-if (XFD_ANYSET()) {
-pInfo = xf86InputDevs;
-while (pInfo) {
-if (pInfo->read_input && pInfo->fd >= 0 &&
-(FD_ISSET(pInfo->fd, ) != 0)) {
-input_lock();
-
-/*
- * Remove the descriptior from the set because more than 
one
- * device may share the same file descriptor.
- */
-FD_CLR(pInfo->fd, );
-
-pInfo->read_input(pInfo);
-input_unlock();
-}
-pInfo = pInfo->next;
-}
-}
-}
-
 if (err >= 0) { /* we don't want the handlers called if 
select() */
 IHPtr ih, ih_tmp;   /* returned with an error condition, do we?
  */
 
-- 
2.8.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 xserver] dix: Don't update current time in the middle of input event processing

2016-05-27 Thread Keith Packard 
In patch 137ac094e7ab8c871f3b36e40ad826ac797f0e26, Adam moved an
expensive call to UpdateCurrentTime out of the main dispatch
loop. That's a good change as the original fix from Chase was a bit
expensive. However, it breaks grab processing and so a couple of the
calls to UpdateCurrenTime need to be removed.

Input event processing can generate a stream of events; a button press
that activates a grab will send a press followed by a sequence of
enter/leave events. All of these should have the same time stamp on
the wire as they occur at the 'same' time.

More importantly, the grab time recorded in the device is pulled from
currentTime after all of the events are delivered, so if currentTime
doesn't match the time in the device event, then future grab
modifications will fail as the time marked in the device will be
'later' than the grab time known to the client (which is defined as
the timestamp from the activating input event).

A bit of history here -- it used to be that currentTime was driven
*entirely* by input events; those timestamps didn't even have to be
related to the system time in any way. Then we started doing ICCCM
stuff and people got confused when PropertyNotify events would have
the same timestamp even when delivered minutes apart because no input
events were delivered.

We added code in the server to go update the time, but only if no
input events were pending (so that the clock "wouldn't" go
backwards). The only places where this is necessary is in request
processing which may generate an event with a timestamp, and there
only at the very top of the request processing code so that the whole
request would be processed at the 'same time', just like events.

cc: Chase Douglas 
cc: Peter Hutterer 
cc: Adam Jackson 
Signed-off-by: Keith Packard 
---
 dix/enterleave.c | 1 -
 dix/events.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/dix/enterleave.c b/dix/enterleave.c
index 0c6c616..1b341f2 100644
--- a/dix/enterleave.c
+++ b/dix/enterleave.c
@@ -782,7 +782,6 @@ DeviceFocusEvent(DeviceIntPtr dev, int type, int mode, int 
detail,
 DeviceIntPtr mouse;
 int btlen, len, i;
 
-UpdateCurrentTimeIf();
 mouse = IsFloating(dev) ? dev : GetMaster(dev, MASTER_POINTER);
 
 /* XI 2 event */
diff --git a/dix/events.c b/dix/events.c
index 0404eba..8efdf18 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -2242,7 +2242,6 @@ DeliverEventsToWindow(DeviceIntPtr pDev, WindowPtr pWin, 
xEvent
this mask is the mask of the grab. */
 int type = pEvents->u.u.type;
 
-UpdateCurrentTimeIf();
 /* Deliver to window owner */
 if ((filter == CantBeFiltered) || core_get_type(pEvents) != 0) {
 enum EventDeliveryState rc;
-- 
2.8.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 xserver 17/23] os: Add ospoll interface

2016-05-27 Thread Emil Velikov 
Hi Keith,

Style question: do you/how many others refer having separate functions for
of ifdeffed code each vs a single with all the ifdeffs. Or in other words
having func_foo_poll and func_foo_epoll as opposed to having it all in
func_foo.

On Friday, 27 May 2016, Keith Packard  wrote:

+#ifndef _OSPOLL_H_
> +#define _OSPOLL_H_
> +
> +#include 
> +

This include should be in the C file with proper ifdef guard, right?

Emil
___
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: Adjust for drawable x/y in composite's copy optimization

2016-05-27 Thread Keith Packard 
Alex Deucher  writes:

>> Signed-off-by: Keith Packard 
>
> Reviewed-by: Alex Deucher 

Merged.

   0d16a0c..8b9b438  master -> master

-- 
-keith


signature.asc
Description: PGP signature
___
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: Cannot use copies when accessing outside of composite source

2016-05-27 Thread Keith Packard 
Michel Dänzer  writes:

> From: Michel Dänzer 
>
> Commit b64108fa ("glamor: Check for composite operations which are
> equivalent to copies") failed to copy conditions from exaComposite which
> ensure that the composite operation doesn't access outside of the source
> picture.

I'm confused -- the region to be copied should already have been clipped
to the source, so there shouldn't be any access outside of the source
picture.

-- 
-keith


signature.asc
Description: PGP signature
___
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: Adjust for drawable x/y in composite's copy optimization

2016-05-27 Thread Alex Deucher 
On Fri, May 27, 2016 at 2:05 PM, Keith Packard  wrote:
> Patch b64108fa305e956e4edaae9d53071ff0abee268e added a short cut that
> identifies composite operations that can be performed with a simple
> copy instead.
>
> glamor_copy works in absolute coordinates, so the dx and dy values
> passed in need to be converted from drawable-relative to absolute by
> adding the drawable x/y values.
>
> Signed-off-by: Keith Packard 

Reviewed-by: Alex Deucher 

> ---
>  glamor/glamor_render.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
> index 9c5cca6..165bced 100644
> --- a/glamor/glamor_render.c
> +++ b/glamor/glamor_render.c
> @@ -1436,6 +1436,10 @@ glamor_composite_clipped_region(CARD8 op,
>   || (op == PictOpOver
>   && source->format == dest->format
>   && !PICT_FORMAT_A(source->format)) {
> +x_source += source->pDrawable->x;
> +y_source += source->pDrawable->y;
> +x_dest += dest->pDrawable->x;
> +y_dest += dest->pDrawable->y;
>  glamor_copy(source->pDrawable, dest->pDrawable, NULL,
>  box, nbox, x_source - x_dest,
>  y_source - y_dest, FALSE, FALSE, 0, NULL);
> --
> 2.8.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

Re: [PATCH v5 00/13] PRIME Synchronization

2016-05-27 Thread Alex Goins 
Got it.

Thanks!
Alex

On Fri, 27 May 2016, Dave Airlie wrote:

> On 20 May 2016 at 08:36, Alex Goins  wrote:
> > Hi Dave,
> >
> > Any update on this? Anything I can do to help?
> 
> Hey,
> 
> can you take a look at
> 
> prime: clean up slave bo properly. (v3)
> 
> I think with this rebased onto that, we should be pretty much good to merge.
> 
> Dave.
> 
___
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] xfree86/modes: Don't set xf86_config->cursor if loading the cursor fails

2016-05-27 Thread Keith Packard 
Michel Dänzer  writes:

> From: Michel Dänzer 
>
> If xf86_load_cursor_argb returns FALSE, the caller is expected to fall
> back to SW cursor, so xf86_config->cursor shouldn't be updated.

It doesn't matter -- xf86_config->cursor is supposed to be a temporary
field until drivers are converted over to xf86CurrentCursor, which
doesn't care whether the cursor is HW or SW.

Nacked-by: Keith Packard 

-- 
-keith


signature.asc
Description: PGP signature
___
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] prime: clean up slave bo properly. (v3)

2016-05-27 Thread Alex Goins 
Reviewed-by: Alex Goins 

Thanks,
Alex

On Fri, 6 May 2016, Dave Airlie wrote:

> This is an ABI break, in that we now pass NULL to a function
> that hasn't accepted it before.
> 
> Alex Goins had a different patch for this but it wasn't symmetrical,
> it freed something in a very different place than it allocated it,
> this attempts to retain symmetry in the releasing of the backing bo.
> 
> v2: use a new toplevel API, though it still passes NULL to something
> that wasn't expecting it.
> v3: pass -1 instead of 0.
> 
> Signed-off-by: Dave Airlie 
> ---
>  dix/pixmap.c | 7 +++
>  hw/xfree86/dri2/dri2.c   | 1 +
>  hw/xfree86/drivers/modesetting/driver.c  | 4 
>  hw/xfree86/drivers/modesetting/drmmode_display.c | 6 ++
>  include/pixmap.h | 3 +++
>  randr/rrcrtc.c   | 2 ++
>  6 files changed, 23 insertions(+)
> 
> diff --git a/dix/pixmap.c b/dix/pixmap.c
> index 11d83fe..49267a1 100644
> --- a/dix/pixmap.c
> +++ b/dix/pixmap.c
> @@ -132,6 +132,13 @@ FreePixmap(PixmapPtr pPixmap)
>  free(pPixmap);
>  }
>  
> +void PixmapUnshareSlavePixmap(PixmapPtr slave_pixmap)
> +{
> + int ihandle = -1;
> + ScreenPtr pScreen = slave_pixmap->drawable.pScreen;
> + pScreen->SetSharedPixmapBacking(slave_pixmap, ((void *)(long)ihandle));
> +}
> +
>  PixmapPtr PixmapShareToSlave(PixmapPtr pixmap, ScreenPtr slave)
>  {
>  PixmapPtr spix;
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index d55be19..f80599f 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -859,6 +859,7 @@ DrawablePtr DRI2UpdatePrime(DrawablePtr pDraw, 
> DRI2BufferPtr pDest)
>  if (pPriv->prime_slave_pixmap->master_pixmap == mpix)
>  return >prime_slave_pixmap->drawable;
>  else {
> +PixmapUnshareSlavePixmap(pPriv->prime_slave_pixmap);
>  
> (*pPriv->prime_slave_pixmap->master_pixmap->drawable.pScreen->DestroyPixmap)(pPriv->prime_slave_pixmap->master_pixmap);
>  (*slave->DestroyPixmap)(pPriv->prime_slave_pixmap);
>  pPriv->prime_slave_pixmap = NULL;
> diff --git a/hw/xfree86/drivers/modesetting/driver.c 
> b/hw/xfree86/drivers/modesetting/driver.c
> index cd59c06..1604044 100644
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -1074,6 +1074,10 @@ msSetSharedPixmapBacking(PixmapPtr ppix, void 
> *fd_handle)
>  Bool ret;
>  int ihandle = (int) (long) fd_handle;
>  
> +if (ihandle == -1)
> +if (!ms->drmmode.reverse_prime_offload_mode)
> +   return drmmode_SetSlaveBO(ppix, >drmmode, ihandle, 0, 0);
> +
>  if (ms->drmmode.reverse_prime_offload_mode) {
>  ret = glamor_back_pixmap_from_fd(ppix, ihandle,
>   ppix->drawable.width,
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
> b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index 2cda523..5e104b8 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -214,6 +214,12 @@ drmmode_SetSlaveBO(PixmapPtr ppix,
>  {
>  msPixmapPrivPtr ppriv = msGetPixmapPriv(drmmode, ppix);
>  
> +if (fd_handle == -1) {
> +dumb_bo_destroy(drmmode->fd, ppriv->backing_bo);
> +ppriv->backing_bo = NULL;
> +return TRUE;
> +}
> +
>  ppriv->backing_bo =
>  dumb_get_bo_from_fd(drmmode->fd, fd_handle, pitch, size);
>  if (!ppriv->backing_bo)
> diff --git a/include/pixmap.h b/include/pixmap.h
> index c6a7736..86b513d 100644
> --- a/include/pixmap.h
> +++ b/include/pixmap.h
> @@ -115,6 +115,9 @@ extern _X_EXPORT void FreePixmap(PixmapPtr /*pPixmap */ );
>  extern _X_EXPORT PixmapPtr
>  PixmapShareToSlave(PixmapPtr pixmap, ScreenPtr slave);
>  
> +extern _X_EXPORT void
> +PixmapUnshareSlavePixmap(PixmapPtr slave_pixmap);
> +
>  #define HAS_DIRTYTRACKING_ROTATION 1
>  extern _X_EXPORT Bool
>  PixmapStartDirtyTracking(PixmapPtr src,
> diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
> index 566a3db..5447133 100644
> --- a/randr/rrcrtc.c
> +++ b/randr/rrcrtc.c
> @@ -373,6 +373,8 @@ rrDestroySharedPixmap(RRCrtcPtr crtc, PixmapPtr pPixmap) {
>   * Unref the pixmap twice: once for the original reference, and once
>   * for the reference implicitly added by PixmapShareToSlave.
>   */
> +PixmapUnshareSlavePixmap(pPixmap);
> +
>  master->DestroyPixmap(pPixmap->master_pixmap);
>  master->DestroyPixmap(pPixmap->master_pixmap);
>  }
> -- 
> 2.5.5
> 
> ___
> 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:

Re: [PATCH xserver 1/2] xfree86/modes: Assign xf86_config->cursor in xf86_load_cursor_image

2016-05-27 Thread Keith Packard 
Michel Dänzer  writes:

> From: Michel Dänzer 
>
> Fixes a crash on startup in the radeon driver's drmmode_show_cursor()
> due to xf86_config->cursor == NULL, because no CRTC was enabled yet, so
> xf86_crtc_load_cursor_image was never called.

Good catch!

> @@ -507,6 +506,9 @@ xf86_load_cursor_image(ScrnInfoPtr scrn, unsigned char 
> *src)
>  return FALSE;
>  }
>  }
> +
> +xf86_config->cursor = xf86CurrentCursor(scrn->pScreen);
> +

This should probably be at the top of xf86_load_cursor_image instead of
the bottom. Otherwise, this patch is

Reviewed-by: Keith Packard 

-- 
-keith


signature.asc
Description: PGP signature
___
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: Adjust for drawable x/y in composite's copy optimization

2016-05-27 Thread Keith Packard 
Patch b64108fa305e956e4edaae9d53071ff0abee268e added a short cut that
identifies composite operations that can be performed with a simple
copy instead.

glamor_copy works in absolute coordinates, so the dx and dy values
passed in need to be converted from drawable-relative to absolute by
adding the drawable x/y values.

Signed-off-by: Keith Packard 
---
 glamor/glamor_render.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
index 9c5cca6..165bced 100644
--- a/glamor/glamor_render.c
+++ b/glamor/glamor_render.c
@@ -1436,6 +1436,10 @@ glamor_composite_clipped_region(CARD8 op,
  || (op == PictOpOver
  && source->format == dest->format
  && !PICT_FORMAT_A(source->format)) {
+x_source += source->pDrawable->x;
+y_source += source->pDrawable->y;
+x_dest += dest->pDrawable->x;
+y_dest += dest->pDrawable->y;
 glamor_copy(source->pDrawable, dest->pDrawable, NULL,
 box, nbox, x_source - x_dest,
 y_source - y_dest, FALSE, FALSE, 0, NULL);
-- 
2.8.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 xserver] os: Increase default client buffer to 16kB

2016-05-27 Thread Keith Packard 
Hans de Goede  writes:

>> Signed-off-by: Keith Packard 
>
> Ack.
>
> Reviewed-by: Hans de Goede 

Pushed.
   7147361..0d16a0c  master -> master

-- 
-keith


signature.asc
Description: PGP signature
___
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/9] xfree86: Set xf86CrtcConfigRec cursor pointer to NULL in HideCursor

2016-05-27 Thread Keith Packard 
Michel Dänzer  writes:

> [ Unknown signature status ]
>
> The patch is
>
> Reviewed-by: Michel Dänzer 

Thanks. Pushed.

   e69061e..7147361  master -> master

-- 
-keith


signature.asc
Description: PGP signature
___
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] xrandrprovider: Do not use separate lists for unbound / source / offload slaves

2016-05-27 Thread Hans de Goede 
A single provider can be both a offload and source slave at the same time,
the use of seperate lists breaks in this case e.g. :

xrandr --listproviders
Providers: number : 2
Provider 0: id: 0x7b cap: 0xf, Source Output, Sink Output, Source Offload, Sink 
Offload crtcs: 3 outputs: 2 associated providers: 0 name:modesetting
Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink 
Offload crtcs: 2 outputs: 5 associated providers: 0 name:modesetting

xrandr --setprovideroutputsource 1 0x7b
xrandr --listproviders
Providers: number : 2
Provider 0: id: 0x7b cap: 0xf, Source Output, Sink Output, Source Offload, Sink 
Offload crtcs: 3 outputs: 2 associated providers: 1 name:modesetting
Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink 
Offload crtcs: 2 outputs: 5 associated providers: 1 name:modesetting

xrandr --setprovideroffloadsink 1 0x7b
xrandr --listproviders
Providers: number : 3
Provider 0: id: 0x7b cap: 0xf, Source Output, Sink Output, Source Offload, Sink 
Offload crtcs: 3 outputs: 2 associated providers: 2 name:modesetting
Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink 
Offload crtcs: 2 outputs: 5 associated providers: 2 name:modesetting
Provider 2: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink 
Offload crtcs: 2 outputs: 5 associated providers: 2 name:modesetting

Not good. The problem is that the provider with id 0x46 now is on both
the output_slave_list and the offload_slave_list of the master screen.

This commit fixes this by unifying all 3 lists into a single slaves list.

Note that this does change the struct _Screen definition, so this is an ABI
break. I do not expect any of the drivers to actually use the removed / changed
fields so a recompile should suffice.

Signed-off-by: Hans de Goede 
---
Change sin v2:
-Do not call AttachUnboundGPU on SetOffloadSink/SetOutputSource(NULL)
-Fix "associated providers" count when source_provider and sink_provider
 are the same
-Fix assert(new->isGPU) triggering on server exit caused by calling
 DetachUnboundGPU on the master
---
 dix/dispatch.c | 30 ++---
 hw/xfree86/dri2/dri2.c |  7 --
 hw/xfree86/modes/xf86RandR12.c | 11 ++
 include/scrnintstr.h   | 15 ++---
 present/present.c  |  2 +-
 randr/randr.c  | 42 ++-
 randr/rrcrtc.c | 16 +++---
 randr/rrmonitor.c  | 17 +++---
 randr/rroutput.c   | 10 -
 randr/rrprovider.c | 50 ++
 randr/rrscreen.c   | 12 +++---
 11 files changed, 104 insertions(+), 108 deletions(-)

diff --git a/dix/dispatch.c b/dix/dispatch.c
index 26122c1..6b893b4 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -3790,9 +3790,7 @@ static int init_screen(ScreenPtr pScreen, int i, Bool gpu)
 pScreen->CreateScreenResources = 0;
 
 xorg_list_init(>pixmap_dirty_list);
-xorg_list_init(>unattached_list);
-xorg_list_init(>output_slave_list);
-xorg_list_init(>offload_slave_list);
+xorg_list_init(>slave_list);
 
 /*
  * This loop gets run once for every Screen that gets added,
@@ -3951,7 +3949,7 @@ AttachUnboundGPU(ScreenPtr pScreen, ScreenPtr new)
 {
 assert(new->isGPU);
 assert(!new->current_master);
-xorg_list_add(>unattached_head, >unattached_list);
+xorg_list_add(>slave_head, >slave_list);
 new->current_master = pScreen;
 }
 
@@ -3959,7 +3957,9 @@ void
 DetachUnboundGPU(ScreenPtr slave)
 {
 assert(slave->isGPU);
-xorg_list_del(>unattached_head);
+assert(!slave->is_output_slave);
+assert(!slave->is_offload_slave);
+xorg_list_del(>slave_head);
 slave->current_master = NULL;
 }
 
@@ -3967,31 +3967,35 @@ void
 AttachOutputGPU(ScreenPtr pScreen, ScreenPtr new)
 {
 assert(new->isGPU);
-xorg_list_add(>output_head, >output_slave_list);
-new->current_master = pScreen;
+assert(!new->is_output_slave);
+assert(new->current_master == pScreen);
+new->is_output_slave = TRUE;
+new->current_master->output_slaves++;
 }
 
 void
 DetachOutputGPU(ScreenPtr slave)
 {
 assert(slave->isGPU);
-xorg_list_del(>output_head);
-slave->current_master = NULL;
+assert(slave->is_output_slave);
+slave->current_master->output_slaves--;
+slave->is_output_slave = FALSE;
 }
 
 void
 AttachOffloadGPU(ScreenPtr pScreen, ScreenPtr new)
 {
 assert(new->isGPU);
-xorg_list_add(>offload_head, >offload_slave_list);
-new->current_master = pScreen;
+assert(!new->is_offload_slave);
+assert(new->current_master == pScreen);
+new->is_offload_slave = TRUE;
 }
 
 void
 DetachOffloadGPU(ScreenPtr slave)
 {
 assert(slave->isGPU);
-xorg_list_del(>offload_head);
-slave->current_master = NULL;
+assert(slave->is_offload_slave);
+slave->is_offload_slave

Re: xserver: Branch 'master' - 10 commits

2016-05-27 Thread Keith Packard 
Michel Dänzer  writes:

> On 27.05.2016 08:08, Keith Packard wrote:
>> 
>> commit f84703b50cc908a127f4ad923ebbf56f8f244c0d
>> Author: Keith Packard 
>> Date:   Tue Dec 8 14:20:21 2015 -0800
>> 
>> dix: Reallocate touchpoint buffer at input event time [v2]
>> 
>> Now that input is threaded, malloc can be used at event time to resize
>> the touchpoint buffer as needed.x
>> 
>> v2: Remove "Need to grow the queue means dropping events."
>> from comment as it no longer applies. (Peter Hutterer)
>> 
>> Signed-off-by: Keith Packard 
>> Reviewed-by: Peter Hutterer 
>
> This change broke make check for me, specifically test/touch:
>
> touch: ../../test/touch.c:62: touch_grow_queue: Assertion
> `TouchBeginDDXTouch(, 1234) == ((void *)0)' failed.

yeah, that test actually checks for the old (broken) X server behaviour
of dropping touch events that overfill the buffer. The X server doesn't
drop them anymore as it can call malloc while reading events now.

From 85fef2e05775beec41eaecf9ee517bd6ffeef858 Mon Sep 17 00:00:00 2001
From: Keith Packard 
Date: Fri, 27 May 2016 01:56:39 -0700
Subject: [PATCH xserver] test: Make touch test reflect new ability to realloc
 touch array

Threaded input allows the input code to call malloc while processing
events. In this case, that's in the middle of processing touch events
and needing to resize the touch buffer.

This test was expecting the old behaviour where touch points would get
dropped if the buffer was full. The fix is to check for the new
behaviour instead.

Signed-off-by: Keith Packard 
---
 test/touch.c | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/test/touch.c b/test/touch.c
index 981c694..e0c1bcc 100644
--- a/test/touch.c
+++ b/test/touch.c
@@ -58,9 +58,8 @@ touch_grow_queue(void)
 dev.last.touches[i].client_id = i * 2;
 }
 
-/* no more space, should've scheduled a workproc */
-assert(TouchBeginDDXTouch(, 1234) == NULL);
-ProcessWorkQueue();
+/* no more space, should've reallocated and succeeded */
+assert(TouchBeginDDXTouch(, 1234) != NULL);
 
 new_size = size + size / 2 + 1;
 assert(dev.last.num_touches == new_size);
@@ -74,8 +73,12 @@ touch_grow_queue(void)
 assert(t->client_id == i * 2);
 }
 
+assert(dev.last.touches[size].active == TRUE);
+assert(dev.last.touches[size].ddx_id == 1234);
+assert(dev.last.touches[size].client_id == 1);
+
 /* make sure those are zero-initialized */
-for (i = size; i < new_size; i++) {
+for (i = size + 1; i < new_size; i++) {
 DDXTouchPointInfoPtr t = [i];
 
 assert(t->active == FALSE);
@@ -136,22 +139,20 @@ touch_find_ddxid(void)
 for (i = 0; i < size; i++)
 dev.last.touches[i].active = TRUE;
 
-/* Try to create more, fail */
+/* Try to create more, succeed */
 ti = TouchFindByDDXID(, 30, TRUE);
-assert(ti == NULL);
+assert(ti != NULL);
 ti = TouchFindByDDXID(, 30, TRUE);
-assert(ti == NULL);
-/* make sure we haven't resized, we're in the signal handler */
-assert(dev.last.num_touches == size);
+assert(ti != NULL);
+/* make sure we have resized */
+assert(dev.last.num_touches == size + 3);
 
 /* stop one touchpoint, try to create, succeed */
 dev.last.touches[2].active = FALSE;
-ti = TouchFindByDDXID(, 30, TRUE);
+ti = TouchFindByDDXID(, 35, TRUE);
 assert(ti == [2]);
-/* but still grow anyway */
-ProcessWorkQueue();
 ti = TouchFindByDDXID(, 40, TRUE);
-assert(ti == [size]);
+assert(ti == [size+1]);
 
 free(dev.name);
 }
-- 
2.8.1


-- 
-keith


signature.asc
Description: PGP signature
___
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: Branch 'master' - 10 commits

2016-05-27 Thread Michel Dänzer 
On 27.05.2016 08:08, Keith Packard wrote:
> 
> commit f84703b50cc908a127f4ad923ebbf56f8f244c0d
> Author: Keith Packard 
> Date:   Tue Dec 8 14:20:21 2015 -0800
> 
> dix: Reallocate touchpoint buffer at input event time [v2]
> 
> Now that input is threaded, malloc can be used at event time to resize
> the touchpoint buffer as needed.x
> 
> v2: Remove "Need to grow the queue means dropping events."
> from comment as it no longer applies. (Peter Hutterer)
> 
> Signed-off-by: Keith Packard 
> Reviewed-by: Peter Hutterer 

This change broke make check for me, specifically test/touch:

touch: ../../test/touch.c:62: touch_grow_queue: Assertion 
`TouchBeginDDXTouch(, 1234) == ((void *)0)' failed.

Program received signal SIGABRT, Aborted.
0x75aff458 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:55
55  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt full
#0  0x75aff458 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:55
resultvar = 0
pid = 22575
selftid = 22575
#1  0x75b008da in __GI_abort () at abort.c:89
save_stage = 2
act = {__sigaction_handler = {sa_handler = 0x4, sa_sigaction = 0x4}, 
sa_mask = {__val = {244813135909, 1, 140737488347424, 8589934684, 0, 0, 0, 
21474836480, 140737316590703, 140737488347576, 140737488347376, 
140737316604560, 140737316624864, 7, 
  140737354088448, 5811584}}, sa_flags = 62, sa_restorer = 0x58ae70 
<__PRETTY_FUNCTION__.12825>}
sigs = {__val = {32, 0 }}
#2  0x75af8387 in __assert_fail_base (fmt=, 
assertion=assertion@entry=0x58ad80 "TouchBeginDDXTouch(, 1234) == ((void 
*)0)", file=file@entry=0x58ab90 "../../test/touch.c", line=line@entry=62, 
function=function@entry=0x58ae70 <__PRETTY_FUNCTION__.12825> 
"touch_grow_queue") at assert.c:92
str = 0x815a90 ""
total = 4096
#3  0x75af8432 in __GI___assert_fail 
(assertion=assertion@entry=0x58ad80 "TouchBeginDDXTouch(, 1234) == ((void 
*)0)", file=file@entry=0x58ab90 "../../test/touch.c", line=line@entry=62, 
function=function@entry=0x58ae70 <__PRETTY_FUNCTION__.12825> 
"touch_grow_queue") at assert.c:101
No locals.
#4  0x0040cadd in touch_grow_queue () at ../../test/touch.c:62
dev = {public = {devicePrivate = 0x0, processInputProc = 0x0, 
realInputProc = 0x0, enqueueInputProc = 0x0, on = 0}, next = 0x0, startup = 0, 
deviceProc = 0x0, inited = 0, enabled = 0, coreEvents = 0, deviceGrab = 
{grabTime = {months = 0, 
  milliseconds = 0}, fromPassiveGrab = 0, implicitGrab = 0, unused 
= 0x0, grab = 0x0, activatingKey = 0 '\000', ActivateGrab = 0x0, DeactivateGrab 
= 0x0, sync = {frozen = 0, state = 0, other = 0x0, event = 0x0}}, type = 0, 
xinput_type = 0, 
  name = 0x815100 "test device", id = 2, key = 0x0, valuator = 
0x7fffe290, touch = 0x7fffe270, button = 0x0, focus = 0x0, proximity = 
0x0, kbdfeed = 0x0, ptrfeed = 0x0, intfeed = 0x0, stringfeed = 0x0, bell = 0x0, 
leds = 0x0, 
  xkb_interest = 0x0, config_info = 0x0, unused_classes = 0x0, 
saved_master_id = 0, devPrivates = 0x0, unwrapProc = 0x0, spriteInfo = 0x0, 
master = 0x0, lastSlave = 0x0, last = {valuators = {0 }, 
numValuators = 0, slave = 0x0, 
scroll = 0x0, num_touches = 8, touches = 0x815120}, properties = 
{properties = 0x0, handlers = 0x0}, relative_transform = {m = {{0, 0, 0}, {0, 
0, 0}, {0, 0, 0}}}, scale_and_transform = {m = {{0, 0, 0}, {0, 0, 0}, {0, 0, 
0}}}, xtest_master_id = 0, 
  idle_counter = 0x0}
val = {sourceid = -7388, numMotionEvents = 32767, first_motion = 
-136429167, last_motion = 32767, motion = 0xd, motionHintWindow = 
0x72550df1, axes = 0x7254eab8, numAxes = 5, axisVal = 0xa4f92301, 
accelScheme = {number = 43246732, 
AccelSchemeProc = 0x1, accelData = 0x7fffe400, AccelInitProc = 
0x75e72f10, AccelCleanupProc = 0x75e70f18}, h_scroll_axis = -7388, 
v_scroll_axis = 32767}
touch = {sourceid = 158, touches = 0x75e72f10, num_touches = 32768, 
max_touches = 63481, mode = 255 '\377', buttonsDown = 127 '\177', state = 0, 
motionMask = 4294959912}
new_size = 
i = 
__PRETTY_FUNCTION__ = "touch_grow_queue"
#5  0x0040c6f9 in main (argc=, argv=) at 
../../test/touch.c:282
No locals.


-- 
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 1/9] xfree86: Set xf86CrtcConfigRec cursor pointer to NULL in HideCursor

2016-05-27 Thread Michel Dänzer 
On 12.05.2016 05:54, Keith Packard wrote:
> This makes the cursor pointer held by xf86Cursors.c get reset to NULL
> whenever the cursor isn't displayed, and means that the reference
> count held in xf86Cursor.c is sufficient to cover the reference in
> xf86Cursors.c.
> 
> As HideCursor may be called in the cursor loading path after
> UseHWCursor or UseHWCursorARGB when HARDWARE_CURSOR_UPDATE_UNHIDDEN
> isn't set in the Flags field, the setting of the cursor pointer had to
> be moved to the LoadCursor paths.
> 
> LoadCursorARGBCheck gets the cursor pointer, but LoadCursorImageCheck
> does not. For LoadCursorImageCheck, I added a new function,
> xf86CurrentCursor, which returns the current cursor. With this new
> function, we can eliminate the cursor pointer from the
> xf86CrtcConfigRec, once drivers are converted over to use it.

You could start by converting the modesetting driver. :)


-- 
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