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). -Chris -- Chris Wilson, Intel Open Source Technology Centre