[PATCH xserver] damage: Add screen func called before damage event delivery

[Michel Dänzer]

On 16/09/16 01:37 PM, Keith Packard wrote:
> This lets the video driver flush rendering to the kernel before the
> client receives a damage event to a pixmap which the client has direct
> rendering access to.

I'm afraid I'm not sure this is going in a good direction.

At the very least, the damage and glamor parts should be split into
separate patches.


> diff --git a/damageext/damageext.c b/damageext/damageext.c
> index 86b54ee..547f048 100644
> --- a/damageext/damageext.c
> +++ b/damageext/damageext.c
> @@ -98,6 +98,7 @@ DamageExtNotify(DamageExtPtr pDamageExt, BoxPtr pBoxes, int nBoxes)
>      damageGetGeometry(pDrawable, &x, &y, &w, &h);
>  
>      UpdateCurrentTimeIf();
> +    DamageFlushDrawable(pDrawable);
>      ev = (xDamageNotifyEvent) {
>          .type = DamageEventBase + XDamageNotify,
>          .level = pDamageExt->level,

Does FlushClient get called after every DamageExtNotify call? Otherwise,
some of the GPU flushes performed by DamageFlushDrawable will be wasted,
hurting performance.


> @@ -362,6 +379,9 @@ glamor_create_screen_resources(ScreenPtr screen)
>          ret = screen->CreateScreenResources(screen);
>      screen->CreateScreenResources = glamor_create_screen_resources;
>  
> +    damage_funcs = DamageGetScreenFuncs(screen);
> +    if (damage_funcs)
> +        damage_funcs->Flush = glamor_damage_flush_drawable;
>      return ret;
>  }

I don't like glamor hooking into this automatically, because it means
the Xorg driver can't know whether or not glamor needs to be flushed.
The Xorg driver needs to flush glamor for other reasons as well, e.g.
for DRI2/3.


> @@ -1943,3 +1948,13 @@ DamageReportDamage(DamagePtr pDamage, RegionPtr pDamageRegion)
>          break;
>      }
>  }
> +
> +void
> +DamageFlushDrawable(DrawablePtr pDrawable)
> +{
> +    ScreenPtr pScreen = pDrawable->pScreen;
> +    damageScrPriv(pScreen);
> +
> +    if (pScrPriv->funcs.Flush)
> +        (*pScrPriv->funcs.Flush)(pDrawable);
> +}

FWIW, this will do nothing for GPU screens. I'm not sure whether or not
GPU screens need to be flushed for damage events, what are others'
thoughts on that?


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