From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhigang Gong" Subject: Re: [PATCH 2/3] glamor: turn on glamor. Date: Mon, 14 Nov 2011 20:02:49 +0800 Message-ID: <068b01cca2c5$550dda50$ff298ef0$@linux.intel.com> References: <1321000281-5097-1-git-send-email-zhigang.gong@linux.intel.com> <1321000281-5097-2-git-send-email-zhigang.gong@linux.intel.com> <056c01cca05f$f852b950$e8f82bf0$@linux.intel.com> <065401cca28a$7db719c0$79254d40$@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id ABF519EF3C for ; Mon, 14 Nov 2011 04:02:51 -0800 (PST) In-Reply-To: Content-Language: zh-cn List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: 'Chris Wilson' , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > Sent: Monday, November 14, 2011 5:07 PM > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > Subject: RE: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor. > > On Mon, 14 Nov 2011 13:01:36 +0800, "Zhigang Gong" > wrote: > > > > > > > -----Original Message----- > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > > > Sent: Friday, November 11, 2011 9:13 PM > > > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > > > Subject: RE: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor. > > > > > > On Fri, 11 Nov 2011 18:52:11 +0800, "Zhigang Gong" > > > wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > > > > > Sent: Friday, November 11, 2011 5:12 PM > > > > > To: Zhigang Gong; intel-gfx@lists.freedesktop.org > > > > > Subject: Re: [Intel-gfx] [PATCH 2/3] glamor: turn on glamor. > > > > > > > > > > On Fri, 11 Nov 2011 16:31:20 +0800, Zhigang Gong > > > > > 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