[Mesa-dev] [PATCH 24/32] i965/miptree: Allocate mt earlier in update winsys

[Pohjolainen, Topi]

On Wed, Jan 04, 2017 at 06:17:31PM -0800, Ben Widawsky wrote:
> On 17-01-04 10:57:40, Topi Pohjolainen Topi Pohjolainen wrote:
> > On Wed, Jan 04, 2017 at 10:26:50AM +0200, Pohjolainen, Topi wrote:
> > > On Mon, Jan 02, 2017 at 06:37:15PM -0800, Ben Widawsky wrote:
> > > > Allows us to continue utilizing common miptree creation using __DRIimage
> > > > without creating a new DRIimage (for the intel_process_dri2_buffer()
> > > > case).
> > > >
> > > > This is a bit ugly, but I think it's the best one can do.
> > > >
> > > > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > > > Acked-by: Daniel Stone <daniels at collabora.com>
> > > > ---
> > > >  src/mesa/drivers/dri/i965/brw_context.c       | 31 +++++++++++++++++++++++----
> > > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 17 ++-------------
> > > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  2 +-
> > > >  3 files changed, 30 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> > > > index 7e350c4e47..ce01605826 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_context.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_context.c
> > > > @@ -1652,10 +1652,24 @@ intel_process_dri2_buffer(struct brw_context *brw,
> > > >        return;
> > > >     }
> > > >
> > > > -   if (!intel_update_winsys_renderbuffer_miptree(brw, rb, bo,
> > > > +   struct intel_mipmap_tree *mt = intel_miptree_create_for_bo(brw,
> > > > +                                                              bo,
> > > > +                                                              intel_rb_format(rb),
> > > > +                                                              0,
> > > > +                                                              drawable->w,
> > > > +                                                              drawable->h,
> > > > +                                                              1,
> > > > +                                                              buffer->pitch,
> > > > +                                                              MIPTREE_LAYOUT_FOR_SCANOUT);
> > > 
> > > I chose this patch to discuss the window system integration which I know very
> > > little about. So here goes:
> > > 
> > > After this patch MIPTREE_LAYOUT_FOR_SCANOUT gets set from two places:
> > > here and from intel_miptree_create_for_renderbuffer().
> > > 
> > > This flag results into mt->is_scanout getting set and corresponding dri images
> > > passed to external consumption fast color cleared but compressed.
> > > 
> > > Effectively intel_mipmap_tree::is_scanout now means that the external party
> > > is compression aware meaning gpu or display controller. Could we add some
> > > explanation to intel_mipmap_tree.h? Current description for is_scanout is:
> > > 
> > >  /**
> > >   * Tells if the underlying buffer is to be also consumed by entities other
> > >   * than the driver. This allows logic to turn off features such as lossless
> > >   * compression which is not currently understood by client applications.
> > >   */
> > > 
> > > Big question for me is that how do DRI2 clients tell compressed buffers are
> > > understood?
> > 
> > All these scanout buffers originate from outside, and if they come with
> > auxiliary, it is clear that compression is supported. Is it that simple in the
> > end?
> 
> I think you get it, but let me make sure. Essentially it's required that when
> the buffer is created, the client specifically requests compression be added.
> For GBM this is through the create_surface_with_modifiers, and
> create_bo_with_modifiers. With EGL it would be done at dma-buf import time. I'm
> not really certain how it should be done with DRI3, or what needs to be done for
> Wayland to support this, but other people understand it. Did I answer your
> question?
> 

Yeah, pretty much. Thanks!