[Mesa-dev] [PATCH 3/7] mesa/st: support lowering multi-planar YUV

[Marek Olšák]

On Mon, Sep 12, 2016 at 4:04 PM, Rob Clark <robdclark at gmail.com> wrote:
> On Mon, Sep 12, 2016 at 9:01 AM, Marek Olšák <maraeo at gmail.com> wrote:
>> On Thu, Sep 8, 2016 at 10:30 PM, Rob Clark <robdclark at gmail.com> wrote:
>>> Support multi-planar YUV for external EGLImage's (currently just in the
>>> dma-buf import path) by lowering to multiple texture fetch's for each
>>> plane and CSC in shader.
>>>
>>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>> ---
>>>  src/gallium/auxiliary/util/u_inlines.h      |   4 +-
>>>  src/gallium/include/pipe/p_state.h          |   9 +++
>>>  src/gallium/include/state_tracker/st_api.h  |   3 +
>>>  src/gallium/state_trackers/dri/dri2.c       | 119 +++++++++++++++++++++++-----
>>>  src/gallium/state_trackers/dri/dri_screen.c |  11 +++
>>>  src/mesa/main/mtypes.h                      |  16 ++++
>>>  src/mesa/program/ir_to_mesa.cpp             |   1 +
>>>  src/mesa/state_tracker/st_atom_sampler.c    |  41 +++++++++-
>>>  src/mesa/state_tracker/st_atom_shader.c     |   3 +
>>>  src/mesa/state_tracker/st_atom_texture.c    |  58 ++++++++++++++
>>>  src/mesa/state_tracker/st_cb_eglimage.c     |  18 +++++
>>>  src/mesa/state_tracker/st_context.c         |   7 +-
>>>  src/mesa/state_tracker/st_glsl_to_nir.cpp   |   1 +
>>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp  |   4 +
>>>  src/mesa/state_tracker/st_manager.c         |   1 +
>>>  src/mesa/state_tracker/st_program.c         |  35 ++++++++
>>>  src/mesa/state_tracker/st_program.h         |  37 +++++++++
>>>  src/mesa/state_tracker/st_texture.h         |  21 +++++
>>>  18 files changed, 362 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/src/gallium/auxiliary/util/u_inlines.h b/src/gallium/auxiliary/util/u_inlines.h
>>> index c2a0b08..b7b8313 100644
>>> --- a/src/gallium/auxiliary/util/u_inlines.h
>>> +++ b/src/gallium/auxiliary/util/u_inlines.h
>>> @@ -136,8 +136,10 @@ pipe_resource_reference(struct pipe_resource **ptr, struct pipe_resource *tex)
>>>     struct pipe_resource *old_tex = *ptr;
>>>
>>>     if (pipe_reference_described(&(*ptr)->reference, &tex->reference,
>>> -                                (debug_reference_descriptor)debug_describe_resource))
>>> +                                (debug_reference_descriptor)debug_describe_resource)) {
>>> +      pipe_resource_reference(&old_tex->next, NULL);
>>>        old_tex->screen->resource_destroy(old_tex->screen, old_tex);
>>> +   }
>>>     *ptr = tex;
>>>  }
>>>
>>> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
>>> index ebd0337..4a88da6 100644
>>> --- a/src/gallium/include/pipe/p_state.h
>>> +++ b/src/gallium/include/pipe/p_state.h
>>> @@ -498,6 +498,15 @@ struct pipe_resource
>>>
>>>     unsigned bind;            /**< bitmask of PIPE_BIND_x */
>>>     unsigned flags;           /**< bitmask of PIPE_RESOURCE_FLAG_x */
>>> +
>>> +   /**
>>> +    * For planar images, ie. YUV EGLImage external, etc, pointer to the
>>> +    * next plane.
>>> +    *
>>> +    * TODO might be useful for dealing w/ z32s8 too, since at least a
>>> +    * couple drivers split these out into separate buffers internally.
>>> +    */
>>
>> Please remove this TODO. Radeon can't do separate depth & stencil.
>> Yes, radeon allocates them separately, but they share the tiling mode
>> and HyperZ metadata, which is interleaved.
>>
>>> +   struct pipe_resource *next;
>>
>> How about this:
>> - pipe_resource::next will be removed.
>> - all planes will be backed by the same pipe_resource
>> - resource_from_handle will receive information about all planes.
>> (whatever you need)
>> - PIPE_FORMAT_XX_XX_XX_PLANAR will be added; replace XX with formats
>> of planes (define as many formats as you need)
>> - pipe_sampler_view::u::tex::first_layer will be used to select the plane.
>> - the driver will look at first_layer and the format and will set up
>> texture sampling from that plane
>>
>> What do you think?
>
>
> Hmm, this approach does end up needing some per-driver work, which I
> was hoping to avoid.. and I guess in the end support for most of the
> drivers will be needed: virtio/vc4/freedreno (linaro/aosp) and
> amdgpu/radeon/nouveau (android-x86).  It does sound like maybe less
> churn in mesa/st, but otoh more churn in all the drivers.  Plus having
> to re-work 'struct winsys_handle' to take an array of
> handle/stride/offset (and eventually modifier for tiled yuv, similar
> to how addfb2 works)..
>
> So it probably ends up being more churn than just making
> st_texture_object::pt an array (which I was going to try, time
> permitting, this week).
>
> On the plus side, maybe it would be easier for video state-trackers,
> although I think pipe_resource::next approach has the same advantage.

Well, it's up to you. I don't have a strong opinion either way.

Marek