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