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

[Ben Widawsky]

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?