Re: [Glamor] [PATCH v2] uxa/glamor: Create glamor pixmap by default.

["Zhigang Gong" <zhigang.gong@linux.intel.com>]

> -----Original Message-----
> From:
> glamor-bounces+zhigang.gong=linux.intel.com@lists.freedesktop.org
> [mailto:glamor-bounces+zhigang.gong=linux.intel.com@lists.freedesktop.o
> rg] On Behalf Of Chris Wilson
> Sent: Wednesday, January 11, 2012 2:01 AM
> To: zhigang.gong@gmail.com
> Cc: intel-gfx@lists.freedesktop.org; zhigang.gong@linux.intel.com;
> zhigang.gong@gmail.com; glamor@lists.freedesktop.org
> Subject: Re: [Glamor] [PATCH v2] uxa/glamor: Create glamor pixmap by
> default.
> 
> On Wed, 11 Jan 2012 10:01:09 +0800, zhigang.gong@gmail.com wrote:
> > From: Zhigang Gong <zhigang.gong@linux.intel.com>
> >
> > A minor fix, after convert the old pixmap to the textured-drm pixmap,
> > we need to modify the old pixmap's header to make sure the
> > width/height and stride are the same as the new textured-drm BO.
> 
> We can not just call FatalError there, but should just propagate the error
> so that the client at least sees BadMatch and X doesn't simply die... How
Agree. X server should not die for a unsupported client application. Maybe I
can just change it to put a warning indicator there.

As to propagate the error to client, I have no idea how to pass a BadMatch
error To client. 
Just return the old texture_only pixmap, and the pixmap_flink will get a 0
name, and then
the client will get a 0 buffer name. If the client check the buffer name
then it can
find something bad happened. If it doesn't, then client may trigger a
segfault which
is what mutter does currently.  Any hint here?

> does this resolve the issue of mesa replacing a bo for a glamor pixmap
> even though we've exported it? Or is that simply an orthogonal problem to
> be tackled later?

My understanding is for the texture which is bound to KHR Image by using
EGLlImageTargetTexture2DOES, mesa will not replacing its BO to a new one. 
For those pure glamor pixmap, as we never export a pure glamor pixmap's BO,
it
will not be a problem. The fixup_glamor will convert a texture only
pixmap to a textured_drm pixmap eventually, then after we export its BO,
mesa will not change it.  Right?

> 
> > ---
> > As create glamor pixmap by default will get much better performance by
> > using the textured-drm pixmap, this commit is to make that the default
> > behaviour when configure to use glamor.
> >
> > A side effect is that for those glamor pixmaps, they don't have a
> > valid BO attached to it and thus it fails to get a DRI drawable. This
> > commit also fixes that problem by copy the fixup_shadow mechanism. I
> > tested this with mutter, and it seems work fine.
> >
> > The performance gain to apply this patch is about 20% to 40% with
> > different workload.
> 
> Waiting to see if I get any results to support that claim... ;-) You can
also
> mention that by using glamor to allocate the pixmaps, we reduce the risk
> of encountering the "incompatible region exists for this name"
> and the associated render corruption. Until that is resolved, every time
we
> export a DRI pixmap and create a BO we still may trigger that bug.
Yeah, will add that at v3 patch.

> However, since we now never intentionally allocate a reusable pixmap we
> could just make all (intel_glamor) allocations non-reusable without
> incurring too great an overhead.
Agree, will disable reusable when create textured_drm pixmap at v3 patch.

> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Glamor mailing list
> Glamor@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/glamor