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

[Chris Wilson <chris@chris-wilson.co.uk>]

On Mon, 14 Nov 2011 13:01:36 +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 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?

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