[Mesa-dev] [PATCH v3 01/15] st/dri: refactor multi-planar YUV import path

Tue May 23 15:40:36 UTC 2017

Hi Varad,

Am Dienstag, den 23.05.2017, 14:40 +0530 schrieb Varad Gautam:
> Hi Lucas,
> 
> On Mon, May 22, 2017 at 11:16 PM, Lucas Stach <l.stach at pengutronix.de> wrote:
> > Am Mittwoch, den 10.05.2017, 23:15 +0530 schrieb Varad Gautam:
> >> From: Varad Gautam <varad.gautam at collabora.com>
> >>
> >> we currently ignore the plane count when converting from
> >> __DRI_IMAGE_FORMAT* tokens to __DRI_IMAGE_FOURCC* for multiplanar
> >> images, and only return the first plane's simplified fourcc.
> >>
> >> this adds a fourcc to __DRI_IMAGE_FORMAT_* mapping to dri, allowing
> >> us to return the correct fourcc format from DRIimage queries, and
> >> simplifies the multiplane import logic.
> >>
> >> Signed-off-by: Varad Gautam <varad.gautam at collabora.com>
> >> ---
> >>  src/gallium/state_trackers/dri/dri2.c       | 288 +++++++++++++++-------------
> >>  src/gallium/state_trackers/dri/dri_screen.h |  13 ++
> >>  2 files changed, 168 insertions(+), 133 deletions(-)
> >>
> >> diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c
> >> index ed6004f..0c5783c 100644
> >> --- a/src/gallium/state_trackers/dri/dri2.c
> >> +++ b/src/gallium/state_trackers/dri/dri2.c
> >> @@ -52,93 +52,133 @@
> >>  #include "dri_query_renderer.h"
> >>  #include "dri2_buffer.h"
> >>
> >> -static int convert_fourcc(int format, int *dri_components_p)
> >> +/* format list taken from intel_screen.c */
> >> +static struct image_format image_formats[] = {
> >> +   { __DRI_IMAGE_FOURCC_ARGB8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_ARGB8888, 4 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_ABGR8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_ABGR8888, 4 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_SARGB8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_SARGB8, 4 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_XRGB8888, __DRI_IMAGE_COMPONENTS_RGB, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_XRGB8888, 4 }, } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_XBGR8888, __DRI_IMAGE_COMPONENTS_RGB, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_XBGR8888, 4 }, } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_ARGB1555, __DRI_IMAGE_COMPONENTS_RGBA, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_ARGB1555, 2 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_RGB565, __DRI_IMAGE_COMPONENTS_RGB, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_RGB565, 2 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_R8, __DRI_IMAGE_COMPONENTS_R, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 }, } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_R16, __DRI_IMAGE_COMPONENTS_R, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R16, 1 }, } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_GR88, __DRI_IMAGE_COMPONENTS_RG, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_GR88, 2 }, } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_GR1616, __DRI_IMAGE_COMPONENTS_RG, 1,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_GR1616, 2 }, } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YUV410, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 2, 2, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 2, 2, 2, __DRI_IMAGE_FORMAT_R8, 1 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YUV411, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 2, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 2, 2, 0, __DRI_IMAGE_FORMAT_R8, 1 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YUV420, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 1, 1, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 2, 1, 1, __DRI_IMAGE_FORMAT_R8, 1 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YUV422, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 1, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 2, 1, 0, __DRI_IMAGE_FORMAT_R8, 1 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YUV444, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 2, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YVU410, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 2, 2, 2, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 2, 2, __DRI_IMAGE_FORMAT_R8, 1 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YVU411, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 2, 2, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 2, 0, __DRI_IMAGE_FORMAT_R8, 1 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YVU420, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 2, 1, 1, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 1, 1, __DRI_IMAGE_FORMAT_R8, 1 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YVU422, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 2, 1, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 1, 0, __DRI_IMAGE_FORMAT_R8, 1 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YVU444, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 2, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_NV12, __DRI_IMAGE_COMPONENTS_Y_UV, 2,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 1, 1, __DRI_IMAGE_FORMAT_GR88, 2 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_NV16, __DRI_IMAGE_COMPONENTS_Y_UV, 2,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
> >> +       { 1, 1, 0, __DRI_IMAGE_FORMAT_GR88, 2 } } },
> >> +
> >> +   { __DRI_IMAGE_FOURCC_YUYV, __DRI_IMAGE_COMPONENTS_Y_XUXV, 2,
> >> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_GR88, 2 },
> >> +       { 0, 1, 0, __DRI_IMAGE_FORMAT_ARGB8888, 4 } } }
> >> +};
> >
> > I'm not sure how this is supposed to work. This is transparently
> > converting YUV images to RGB pipe formats, with no "behind the scenes"
> > shader conversion, relying on the application to provide the correct
> > conversion shader. This seems really inconsistent.
> >
> > Also there is no way to pass trough YUV images to the driver (Vivante
> > GPUs support sampling from YUYV, so we would really like to see a single
> > plane with PIPE_FORMAT_YUYV trickling down into the driver.
> >
> 
> The current way of dealing with YUV images is to feed a YUV image to
> the driver with each plane as a distinct pipe_resource, with the
> pipe_format used to determine the size of a plane's components and
> setup samplers (eg. an NV12 image translates to R8 and RG88 for the Y
> and UV planes respectively). This does not affect the colorspace of
> the image data here - the YUV->RGB conversion pass is emitted when the
> sampling happens via an external sampler [1].

I don't see where we are keeping the information that the image is
actually YUV, to trigger emission of the shader conversion code.

Related to that we are loosing the correct image FOURCC on a round trip.
If we import a YUV image te "dri_format" will contain the format of one
of the planes. Then when querying the image FOURCC we use dri_format to
reconstruct the FOURCC.

This isn't more broken than it was before, but I don't think this is
what we should implement.

Regards,
Lucas