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

Michel Dänzer michel at daenzer.net
Tue Sep 20 09:31:27 UTC 2016


On 17/09/16 12:51 AM, Keith Packard wrote:
> Michel Dänzer <michel at daenzer.net> writes:
> 
>> Does FlushClient get called after every DamageExtNotify call? Otherwise,
>> some of the GPU flushes performed by DamageFlushDrawable will be wasted,
>> hurting performance.
> 
> The driver gets told which drawable is being affected, so it could limit
> flushes if there were no rendering operations affecting that pixmap.

When would damage be posted to a drawable without any corresponding
rendering operations? ;)

What I might want to do in our drivers is to use the new hook instead of
an EventCallback to determine when we're sending damage events to a
client. But for that purpose, a) the hook is missing a ClientPtr
parameter and b) its name is a bit misleading.


> I didn't add that code as it wasn't necessary to fix the bug.

FWIW, as is it might result in the modesetting driver getting lower
scores in some 2D benchmarks than our drivers.


>> 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.
> 
> The lower level driver could wrap the function if it likes; glamor
> certainly needs to flush at this point, so if the lower level driver
> also has work to do at that point, it could.

Unless the hook gets at least a ClientPtr parameter, I'm afraid it's
useless for our drivers, and they'll just have to set the hook to NULL.
Seems a bit pointless. Let the modesetting driver and any other drivers
which want to use glamor_damage_flush_drawable hook it up?


>> 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?
> 
> The immediate need here is for applications which use DRI to access
> window pixmaps. As GPU screen pixmaps can also be directly accessed
> through DRI, I suspect they would also need these hooks, but I guess I
> don't understand why that wouldn't already be the case here?

In the sub-thread with Hans, we came to the conclusion that GPU screens
don't need to be flushed for damage events, because there's no way for
clients to directly use GPU screen pixmaps. In which case, there's
actually the reverse problem that this patch would introduce superfluous
flushes on GPU screens.


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



More information about the xorg-devel mailing list