Re: [PATCH 2/2] glamor: Address GLAMOR/UXA flushing problem.

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

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Tuesday, November 15, 2011 8:25 PM
> To: Zhigang Gong; intel-gfx@lists.freedesktop.org
> Cc: zhigang.gong@linux.intel.com
> Subject: Re: [PATCH 2/2] glamor: Address GLAMOR/UXA flushing problem.
> 
> On Tue, 15 Nov 2011 19:36:14 +0800, Zhigang Gong
> <zhigang.gong@linux.intel.com> wrote:
> > This commit introduces a new function in UXA layer need_flush which is
> > used to let the UXA layer to notify the lower layer that some pixmap
> > get modified by GLAMOR. And then the intel driver could know it need
> > to flush front buffer latter.
> >
> > This commit also adds some necessary flushing pointis for UXA layer
> > and glamor layer. Basicly, there are three scenarios:
> >
> > 1. Before calling into glamor layer, it needs to flush all the
> > corresponding UXA batch comand buffer.
> >
> > 2. After calling the glamor rendering functions, it needs to flush the
> > pending GL operations.
> >
> > 3. Before we map a pixmap's BO, we also need to flush all the pending
> > GL operations.
> >
> > The scenario 2 could be eliminated when we fully change to glamor
> > path.
> 
> I much prefer the one-sided uxa_prepare_access() you have implemented
> here for the glamor side. For completeness, you still probably want the
> uxa_finish_access() though. 

I was thinking about that, but current uxa_finish_access doesn't have a
parameter
to indicate the access type. I prefer to change the uxa_finish_access's
prototype
to the same as uxa_prepare_access(). And then will do glamor flush there if
the
access type is GLAMOR_WRITE. I'm just not very sure whether it is the best
way.

Will work out a new patch to implement that soon.

BTW, May I have your review tag in the commits which get reviewed by you.
Thanks.

> Alternatively you need to mark up all
> operations with uxa_prepare_access(GLAMOR | NATIVE | SW, READ |
> WRITE) which then perform the implicit uxa_finish_access() when the
> mode changes, and would also allow you to only flush the glamor
> operations as required.
This looks much complicate than the first solution. I prefer the first
solution.

-- Zhigang
> 
> Aside from that, this looks like the right approach to handling the
flushing.
> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre