> -----Original Message----- > From: Chris Wilson [mailto:chris at chris-wilson.co.uk] > Sent: Monday, November 14, 2011 5:07 PM > To: Zhigang Gong; intel-gfx at lists.freedesktop.org > Subject: RE: [PATCH 2/3] glamor: turn on glamor. > > On Mon, 14 Nov 2011 13:01:36 +0800, "Zhigang Gong" > <zhigang.gong at linux.intel.com> wrote: > > > > > > > -----Original Message----- > > > From: Chris Wilson [mailto:chris at chris-wilson.co.uk] > > > Sent: Friday, November 11, 2011 9:13 PM > > > To: Zhigang Gong; intel-gfx at lists.freedesktop.org > > > Subject: RE: [PATCH 2/3] glamor: turn on glamor. > > > > > > On Fri, 11 Nov 2011 18:52:11 +0800, "Zhigang Gong" > > > <zhigang.gong at linux.intel.com> wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Chris Wilson [mailto:chris at chris-wilson.co.uk] > > > > > Sent: Friday, November 11, 2011 5:12 PM > > > > > To: Zhigang Gong; intel-gfx at lists.freedesktop.org > > > > > Subject: Re: [PATCH 2/3] glamor: turn on glamor. > > > > > > > > > > On Fri, 11 Nov 2011 16:31:20 +0800, Zhigang Gong > > > > > <zhigang.gong at linux.intel.com> wrote: > > > > > > @@ -965,6 +969,9 @@ void > > > > > intel_uxa_block_handler(intel_screen_private *intel) > > > > > > * framebuffer until significantly later. > > > > > > */ > > > > > > intel_flush_rendering(intel); > > > > > > +#ifdef GLAMOR > > > > > > + intel_glamor_block_handler(intel); > > > > > > +#endif > > > > > > } > > > > > > > > > > I suspect this is the wrong way around as we are not flushing > > > > > the render cache of glamor's rendering to the scanout until the > > > > > next block > > > handler. > > > > I don't understand here. Would you please explain more detail? > Thanks. > > > > > > Whenever we render, the data ends up in the Render Cache and > needs > > > to be flushed out to memory before it is coherent with the CPU or in > > > this case the Display Engine (i.e. scanout). > > > > > > intel_flush_rendering() does two tasks. The first is to submit any > > > pending batch, and the second is to flush the Render Cache so that > > > the modifications land on the scanout in a timely manner. It is > > > probably best > > if > > > those two tasks were separated so that we do: > > > > > > intel_uxa_block_handler(intel); // flush the UXA batch > > > intel_glamor_block_handler(intel); // flush the GL batch > > > intel_flush_rendering(intel); // flush the RenderCache to scanout > > > > > > However, you can simply rearrange the code and achieve it with the > > > existing functions: > > > > > > intel_glamor_block_handler(intel); // mark the front bo as dirty > > > as needbe > > > intel_flush_rendering(intel); // flush UXA batch along with > > > RenderCache > > Thanks for the explanation here. But I still don't think the original > > code is wrong regard to this cache flushing issue. Here is my > > analysis: > > intel_glamor_block_handler calls to glFlush(), and glFlush is similar > > with the intel_flush_rendering, it calls intel_flush to flush the > > batch buffers and then call intel_flush_frontbuffer to flush the > > frontbuffer which flushes the scan out buffer. So when the screen > > pixmap is accessed by glamor, and after we call > > intel_glamor_block_handler, the Display Engine should see the correct > > data > > > > Right? > > No. > > glFlush() does call intel_flush_front(). However that in turn calls > dri2->flushFrontBuffer which is implemented for EGL with > > static void > dri2_flush_front_buffer(__DRIdrawable * driDrawable, void > *loaderPrivate) { > /* FIXME: Does EGL support front buffer rendering at all? */ } > > Neither does it perform the intended action via GLX (except that flushing > the scanout is handled by the DDX as a normal part of its operation). You are right. EGL layer will not do a really front buffer flushing. We have to let it be done in DDX layer. In my version 2 patch set, I already rearrange the code sequence as you suggested please review it again. The remaining work for this issue is that I need to add new code to set the needs_flush according to the access type of glamor. Will do that soon. Thanks. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
