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

[Michel Dänzer]

On 16/09/16 05:37 PM, Hans de Goede wrote:
> On 16-09-16 08:26, Michel Dänzer wrote:
>> 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.

Assuming GPU screens don't need to be flushed for damage events, our
drivers could use the damage Flush hook instead of an EventCallback
scanning for damage events though.


>>> @@ -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?
> 
> With render offloading I do think we want to flush,

Do you have a specific scenario in mind where a GPU screen needs to be
flushed before damage events are sent to a client?

I've actually almost convinced myself that it's not necessary, because
damage events can't refer to any pixmaps directly rendered to by GPU
screens?


> for the slave output case we should never get any
> Damage marked on the slave GPU pixmaps to begin with
> (and we do want the flush on master GPU pixmaps before
>  they get passed to the slave).

This patch only affects client damage records, not PRIME slave output.


> I assume that there is a check somewhere in the call chain
> to not do the flush if there is no damage on the pixmap ?

I don't know of such a thing FWIW.


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