> -----Original Message----- > From: Chris Wilson [mailto:chris at chris-wilson.co.uk] > Sent: Wednesday, December 14, 2011 7:12 PM > To: Zhigang Gong > Cc: intel-gfx at lists.freedesktop.org; zhigang.gong at gmail.com > Subject: RE: [PATCH] uxa/glamor: Enable the rest glamor rendering > functions. > > On Wed, 14 Dec 2011 12:08:30 +0800, "Zhigang Gong" > <zhigang.gong at linux.intel.com> wrote: > > > -----Original Message----- > > > From: Chris Wilson [mailto:chris at chris-wilson.co.uk] > > > Sent: Wednesday, December 14, 2011 2:45 AM > > > To: zhigang.gong at linux.intel.com > > > Cc: intel-gfx at lists.freedesktop.org; zhigang.gong at gmail.com; > > > zhigang.gong at linux.intel.com > > > Subject: Re: [PATCH] uxa/glamor: Enable the rest glamor rendering > > > functions. > > > > > > On Tue, 13 Dec 2011 22:31:41 +0800, zhigang.gong at linux.intel.com > > > wrote: > > > > From: Zhigang Gong <zhigang.gong at linux.intel.com> > > > > > > > > This commit enable all the rest glamor rendering functions. > > > > Tested with latest glamor master branch, can pass rendercheck. > > > > > > Hmm, it exposes an issue with keeping a bo cache independent of > mesa > > > and trying to feed it our own handles: > > > > > > Region for name 6 already exists but is not compatible > > > > > > The w/a for this would be: > > > > > > diff --git a/src/intel_glamor.c b/src/intel_glamor.c index > > 0cf8ed7..2757fd6 > > > 100644 > > > --- a/src/intel_glamor.c > > > +++ b/src/intel_glamor.c > > > @@ -91,6 +91,7 @@ > intel_glamor_create_textured_pixmap(PixmapPtr > > > pixmap) > > > priv = intel_get_pixmap_private(pixmap); > > > if (glamor_egl_create_textured_pixmap(pixmap, > > > priv->bo->handle, > > > priv->stride)) { > > > + drm_intel_bo_disable_reuse(priv->bo); > > > priv->pinned = 1; > > > return TRUE; > > > } else > > > > > > but that gives up all pretense of maintaining a bo cache. > > > > Yes, I think this impacts the performance. Actually, I noticed this > > problem and I spent some time to track the root cause. If everything > > is ok, this error should not be triggered. Although the same BO maybe > > reused to create a new pixmap, the previous pixmap which own this BO > > should be already destroyed. And the previous image created with the > > previous pixmap should be destroyed either. > > Certainly it looks like glamor is taking all necessary steps to decouple the > bo from the textures and renderbuffer upon pixmap finish. There is one > other potential race here in that the ddx will mark the bo as purgeable as > soon as it releases it back to the cache, but it may not yet have been > submitted by mesa in its execbuffer. The kernel may choose to free the > memory associated with the bo before that happens, and may rightfully > complain the userspace is doing something silly. Right, we do have this race if the kernel free the BO's memory prior to The mesa submit its execbuffer. Hmm. But I think that may not be a real problem, as once we call intel_set_pixmap_bo(pixmap, NULL) to unlink the bo from the pixmap, the BO will not be released at DRM layer immediately, instead, it will be put on a in_flight list. And intel_batch_submit will empty the list, considering after switching to glamor, each pixmap's batch buffer should be empty, then the driver will only call intel_batch_submit at intel_flush_rendering which is called from intel_uxa_block_handler and is after the intel_glamor_flush. At intel_glamor_flush, it will do a glFlush, my understanding is glFlush should make sure the execbuffer was submitted to GPU. But I'm not very sure. Can you confirm that? Thanks. > > > And then, when we create a new pixmap/image with this BO, MESA > should > > not find any exist image/region for this BO. But it does happen. I > > tracked further into mesa internal and found that the previous image > > was not destroyed when we call eglDestroyImageKHR, as its reference > > count is decreased to zero. It's weird for me. Further tracking shows > > that the root cause is when I use the texture(bind to the image) as a > > shader's source texture, and call glDrawArrays to perform the > > rendering, the texture's reference count will be increased by 1 before > > return from glDrawArrays. And I failed to find any API to decrease it. > > Then this texture can't be freed when destroy that texture and thus > > the image's reference count will also remain 1 and can't be freed > > either. > > I'm looking at update_texture_state() which appears to hold onto a > reference to the texobj until its slot is reused. If the object is simply > destroyed and the unit disabled, the slot is skipped and the old reference > remains. _mesa_update_state (which calls into > update_texture_state) would seem to be only invoked upon a draw > primitives. Binding a dummy texture/fb and doing a dummy render might > be a little extreme as a workaround! Thanks for providing this workaround, I did try that way with my simple test case, it works. I just wonder whether this is a bug of MESA. I will implement it in glamor before we get a better solution. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
