[Mesa-dev] [PATCH v13 09/36] st/dri: refactor multi-planar YUV import path

[Emil Velikov]

Hi Dan,

On 19 May 2017 at 10:37, Daniel Stone <daniels at collabora.com> wrote:
> 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>
> Signed-off-by: Daniel Stone <daniels 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 ed6004f836..0c5783cbd3 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[] = {

Any reason why we don't move this to a common place - say dri_util.c
and reuse it in all the drivers?
Be that including nouveau_vieux and friends or not.

AFAICT this is a constant data - let's annotate it as such.


> +static struct image_format *
> +image_format_from_fourcc(int fourcc)
>  {

> +   struct image_format *f = NULL;
> +
Nit: drop the temporary variable and return directly? Same idea
applies from_dri2_format below.


>  static enum pipe_format dri2_format_to_pipe_format (int format)
> @@ -791,13 +831,13 @@ dri2_create_image_from_winsys(__DRIscreen *_screen,
>     __DRIimage *img;
>     struct pipe_resource templ;
>     unsigned tex_usage;
> -   enum pipe_format pf;
> +   struct image_format *f = NULL;
Nit: Initialisation is not needed...  which applies for the rest of the patch.


> @@ -1077,7 +1086,19 @@ dri2_query_image(__DRIimage *image, int attrib, int *value)
>        *value = image->dri_components;
>        return GL_TRUE;
>     case __DRI_IMAGE_ATTRIB_FOURCC:
> -      *value = convert_to_fourcc(image->dri_format);
> +      switch (image->dri_components) {
> +      case __DRI_IMAGE_COMPONENTS_Y_U_V:
> +         nplanes = 3;
> +         break;
> +      case __DRI_IMAGE_COMPONENTS_Y_UV:
> +      case __DRI_IMAGE_COMPONENTS_Y_XUXV:
> +         nplanes = 2;
> +         break;
> +      default:
> +         nplanes = 1;
> +      }
> +      f = image_format_from_dri2_format(image->dri_format, nplanes);
> +      *value = f->fourcc;
Strictly speaking these queries may fail, although we don't check that
in the loader :-\
Might as pick the loose ends at a later stage.

-Emil