[Mesa-dev] [PATCH 4/5] nouveau: Add framebuffer modifier support

[Emil Velikov]

Hi Thierry,

On 21 February 2018 at 15:30, Thierry Reding <thierry.reding at gmail.com> wrote:

>
>  struct pipe_resource *
>  nouveau_buffer_create(struct pipe_screen *pscreen,
> -                      const struct pipe_resource *templ);
> +                      const struct pipe_resource *templ,
> +                      const uint64_t *modifiers, unsigned int count);
>
The extra arguments seem unused. I guess it's a left-over from earlier
iteration?
Or perhaps you have extra patches that depend on this?


>  struct pipe_resource *
>  nouveau_user_buffer_create(struct pipe_screen *screen, void *ptr,
> diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c b/src/gallium/drivers/nouveau/nouveau_screen.c
> index c144b39b2dd2..d651cc7f4b8c 100644
> --- a/src/gallium/drivers/nouveau/nouveau_screen.c
> +++ b/src/gallium/drivers/nouveau/nouveau_screen.c
> @@ -1,3 +1,5 @@
> +#include <drm_fourcc.h>
> +
>  #include "pipe/p_defines.h"
>  #include "pipe/p_screen.h"
>  #include "pipe/p_state.h"
> @@ -23,6 +25,8 @@
>  #include "nouveau_mm.h"
>  #include "nouveau_buffer.h"
>
> +#include "nvc0/nvc0_resource.h"
> +
> +static uint64_t nouveau_bo_get_modifier(struct nouveau_bo *bo)
> +{
> +   struct nouveau_device *dev = bo->device;
> +
> +   if (dev->chipset >= 0xc0)
> +      return nvc0_bo_get_modifier(bo);
> +
> +   return DRM_FORMAT_MOD_INVALID;
> +}
>
Normally the backends (include and) call into the core nouveau code.
Calling In the opposite direction is achieved via vfuncs, IIRC.


>     switch (templ->target) {
>     case PIPE_BUFFER:
> -      return nouveau_buffer_create(screen, templ);
> +      return nouveau_buffer_create(screen, templ, &modifier, 1);
>     default:
>        return nv50_miptree_create(screen, templ);
>     }
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c b/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c
> index 27674f72a7c0..627d6b7346c3 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c
> @@ -20,6 +20,8 @@
>   * OTHER DEALINGS IN THE SOFTWARE.
>   */
>
> +#include <drm_fourcc.h>
> +
>  #include "pipe/p_state.h"
>  #include "pipe/p_defines.h"
>  #include "util/u_inlines.h"
> @@ -244,7 +246,8 @@ const struct u_resource_vtbl nvc0_miptree_vtbl =
>
>  struct pipe_resource *
>  nvc0_miptree_create(struct pipe_screen *pscreen,
> -                    const struct pipe_resource *templ)
> +                    const struct pipe_resource *templ,
> +                    const uint64_t *modifiers, unsigned int count)
>  {
>     struct nouveau_device *dev = nouveau_screen(pscreen)->device;
>     struct nouveau_drm *drm = nouveau_screen(pscreen)->drm;
> @@ -277,6 +280,9 @@ nvc0_miptree_create(struct pipe_screen *pscreen,
>        }
>     }
>
> +   if (count == 1 && modifiers[0] == DRM_FORMAT_MOD_LINEAR)
> +      pt->flags |= NOUVEAU_RESOURCE_FLAG_LINEAR;
> +
Through the patch count 1 + DRM_FORMAT_MOD_INVALID is used, yet
DRM_FORMAT_MOD_LINEAR is checked above.
Am I having a silly moment and those should be the same or ?

> +static void
> +nvc0_query_dmabuf_modifiers(struct pipe_screen *screen,
> +                            enum pipe_format format, int max,
> +                            uint64_t *modifiers, unsigned int *external_only,
> +                            int *count)
> +{
> +}
Add a TODO/WIP/EMPTY note with some brief info why it's empty?


-Emil