[Mesa-dev] [PATCH V2 15/22] i965/gen9: Use _mesa_meta_pbo_TexSubImage to write to YF/YS surfaces

[Pohjolainen, Topi]

On Fri, Apr 24, 2015 at 05:12:34PM -0700, Anuj Phogat wrote:
> On Fri, Apr 24, 2015 at 12:22 PM, Pohjolainen, Topi
> <topi.pohjolainen at intel.com> wrote:
> > On Fri, Apr 17, 2015 at 04:51:36PM -0700, Anuj Phogat wrote:
> >> No other path currently supports uploading data to these surfaces.
> >>
> >> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> >> ---
> >>  src/mesa/drivers/dri/i965/intel_tex_image.c    | 24 ++++++++++++++++++++++--
> >>  src/mesa/drivers/dri/i965/intel_tex_subimage.c | 23 +++++++++++++++++++++--
> >>  2 files changed, 43 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c b/src/mesa/drivers/dri/i965/intel_tex_image.c
> >> index 31cbabe..03db100 100644
> >> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> >> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> >> @@ -93,8 +93,10 @@ intelTexImage(struct gl_context * ctx,
> >>                const struct gl_pixelstore_attrib *unpack)
> >>  {
> >>     struct intel_texture_image *intelImage = intel_texture_image(texImage);
> >> +   struct brw_context *brw = brw_context(ctx);
> >>     bool ok;
> >> -
> >> +   bool create_pbo = false;
> >> +   uint32_t tr_mode = INTEL_MIPTREE_TRMODE_NONE;
> >>     bool tex_busy = intelImage->mt && drm_intel_bo_busy(intelImage->mt->bo);
> >>
> >>     DBG("%s mesa_format %s target %s format %s type %s level %d %dx%dx%d\n",
> >> @@ -111,15 +113,33 @@ intelTexImage(struct gl_context * ctx,
> >>
> >>     assert(intelImage->mt);
> >>
> >> +   if (brw->gen >= 9) {
> >> +      tr_mode = intelImage->mt->tr_mode;
> >> +
> >> +      /* Set create_pbo = true for surfaces with INTEL_MIPTREE_TRMODE_{YF/YS}.
> >> +       * _mesa_meta_pbo_TexSubImage() is the only working path to upload data
> >> +       * to such surfaces.
> >> +       */
> >> +      create_pbo = tex_busy || (intelImage->mt &&
> >> +                   intelImage->mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE);
> >> +   } else {
> >> +      create_pbo = tex_busy;
> >> +   }
> >> +
> >
> > What would you think about a helper?
> Yes, we can use a helper here.
> >
> > static bool need_to_create_pbo(const struct intel_texture_image *img)
> > {
> >    if (!img->mt)
> >       return false;
> >
> >    const bool tex_busy = drm_intel_bo_busy(img->mt->bo);
> >    if (tex_busy || brw->gen < 9)
> >       return tex_busy;
> >
> >    /* Set create_pbo = true for surfaces with INTEL_MIPTREE_TRMODE_{YF/YS}.
> >     * _mesa_meta_pbo_TexSubImage() is the only working path to upload data
> >     * to such surfaces.
> >     */
> >    return img->mt->tr_mode != INTEL_MIPTREE_TRMODE_NONE;
> > }
> >
> >>     ok = _mesa_meta_pbo_TexSubImage(ctx, dims, texImage, 0, 0, 0,
> >>                                     texImage->Width, texImage->Height,
> >>                                     texImage->Depth,
> >>                                     format, type, pixels,
> >>                                     false /*allocate_storage*/,
> >> -                                   tex_busy, unpack);
> >> +                                   create_pbo, unpack);
> >>     if (ok)
> >>        return;
> >>
> >> +   /* Currently there are no fallback paths to upload data to surfaces with
> >> +    * tr_mode != INTEL_MIPTREE_TRMODE_NONE.
> >> +    */
> >> +   assert(tr_mode == INTEL_MIPTREE_TRMODE_NONE);
> >
> > And I would put this assertion into _mesa_meta_pbo_TexSubImage() instead
> > of duplicating it for both callers. What do you think?
> >
> Meta functions are not supposed to use any Intel specific code. I'm anyway
> getting rid of these assertions in my later patches. So, I'll just keep them
> here.

Ah, of course, sorry for the noise.