[Mesa-dev] [PATCH 2/2] st/dri: replace format conversion functions with single mapping table

[Lucas Stach]

Hi Marek,

thanks for the review.

Am Samstag, den 26.05.2018, 00:36 -0400 schrieb Marek Olšák:
[...]
> 
> > @@ -983,7 +760,7 @@ dri2_update_tex_buffer(struct dri_drawable *drawable,
> > 
> >  static __DRIimage *
> >  dri2_create_image_from_winsys(__DRIscreen *_screen,
> > -                              int width, int height, int format,
> > +                              int width, int height, enum pipe_format pf,
> >                                int num_handles, struct winsys_handle *whandle,
> >                                void *loaderPrivate)
> >  {
> > @@ -992,13 +769,8 @@ dri2_create_image_from_winsys(__DRIscreen *_screen,
> >     __DRIimage *img;
> >     struct pipe_resource templ;
> >     unsigned tex_usage = 0;
> > -   enum pipe_format pf;
> >     int i;
> > 
> > -   pf = dri2_format_to_pipe_format (format);
> > -   if (pf == PIPE_FORMAT_NONE)
> > -      return NULL;
> > -
> >     if (pscreen->is_format_supported(pscreen, pf, screen->target, 0,
> >                                      PIPE_BIND_RENDER_TARGET))
> >        tex_usage |= PIPE_BIND_RENDER_TARGET;
> > @@ -1006,6 +778,20 @@ dri2_create_image_from_winsys(__DRIscreen *_screen,
> >                                      PIPE_BIND_SAMPLER_VIEW))
> >        tex_usage |= PIPE_BIND_SAMPLER_VIEW;
> > 
> > +   if (!tex_usage && util_format_is_yuv(pf)) {
> > +      /* YUV format sampling can be emulated by the Mesa state tracker by
> > +       * using multiple R8/RG88 samplers. So try to rewrite the pipe format.
> > +       */
> > +      pf = PIPE_FORMAT_R8_UNORM;
> > +
> > +      if (pscreen->is_format_supported(pscreen, pf, screen->target, 0,
> > +                                       PIPE_BIND_SAMPLER_VIEW))
> > +         tex_usage |= PIPE_BIND_SAMPLER_VIEW;
> > +   }
> 
> This doesn't seem to belong in this commit.

It does. The removed conversion functions would end up with
PIPE_FORMAT_R8_UNORM for I420 or NV12 input FOURCCs, with the implicit
assumption that all drivers need the emulation by the Mesa state-
tracker.

The new format table maps those to the proper PIPE_FORMAT_IYUV and
PIPE_FORMAT_NV12, so in order to not break drivers which can't handle
those formats natively and need the Mesa side emulation we need this
hunk of code.

> Other than that, the series is:
> 
> > Reviewed-by: Marek Olšák <marek.olsak at amd.com>

With the above explanation, can I keep your review for the whole patch?

Regards,
Lucas