Hi,
On 16-09-16 10:59, Michel Dänzer wrote:
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?
If a client is somehow tracking a pixmap which gets rendered
by a secondary GPU, but I did not think about the copy that
involves, so thinking about this again in that scenario we
will have:
1) Rendering on secondary GPU (aka a GPU screen)
2) We need a flush here, but that should already be taken care of
3) Copy to buffer/pixmap on primary GPU
4) The copy will do damage to the pixmap, but then we're already
on the primary GPU
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?
See above I think you're right. Following the same logic though
I do not think the code this patch adds will ever get called on
a GPU screen, so no need to check for that ?
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.
Ah rights, it sits in damageext not in mi/damageext.
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.
If it is not there yet, it really should be, I do not think
we want to do flushes to make sure an undamaged pixmap
is completely rendered, as it already should be completely
rendered.
Regards,
Hans
_______________________________________________
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
