Re: [PATCH 2/3] glamor: turn on glamor.

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

> -----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"
> <zhigang.gong@linux.intel.com> 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
> > > <zhigang.gong@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?

- Zhigang

> 
> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre