[Mesa-dev] [PATCH v2 12/34] i965/miptree: Add a helper for getting the aux isl_surf from a miptree

Pohjolainen, Topi topi.pohjolainen at intel.com
Mon Jun 27 15:52:27 UTC 2016


On Mon, Jun 27, 2016 at 08:40:41AM -0700, Jason Ekstrand wrote:
>    On Mon, Jun 27, 2016 at 8:34 AM, Pohjolainen, Topi
>    <[1]topi.pohjolainen at intel.com> wrote:
> 
>    On Thu, Jun 23, 2016 at 02:00:11PM -0700, Jason Ekstrand wrote:
>    > ---
>    >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 89
>    +++++++++++++++++++++++++++
>    >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  5 ++
>    >  2 files changed, 94 insertions(+)
>    >
>    > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>    b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>    > index 8a746ec..0f17411 100644
>    > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>    > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>    > @@ -3167,6 +3167,95 @@ intel_miptree_get_isl_surf(struct brw_context
>    *brw,
>    >     surf->usage = 0; /* TODO */
>    >  }
>    >
>    > +/* WARNING: THE SURFACE CREATED BY THIS FUNCTION IS NOT COMPLETE AND
>    CANNOT BE
>    > + * USED FOR ANY REAL CALCULATIONS.  THE ONLY VALID USE OF SUCH A
>    SURFACE IS TO
>    > + * PASS IT INTO isl_surf_fill_state.
>    > + */
>    > +void
>    > +intel_miptree_get_aux_isl_surf(struct brw_context *brw,
>    > +                               const struct intel_mipmap_tree *mt,
>    > +                               struct isl_surf *surf,
>    > +                               enum isl_aux_layout *layout)
>    > +{
>    > +   /* Much is the same as the regular surface */
>    > +   intel_miptree_get_isl_surf(brw, mt->mcs_mt, surf);
>    > +
>    > +   /* Figure out the layout */
>    > +   if (mt->num_samples > 1) {
>    > +      if (mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS)
>    > +         *layout = ISL_AUX_LAYOUT_MCS;
>    > +      else
>    > +         *layout = ISL_AUX_LAYOUT_NONE;
>    > +   } else if (intel_miptree_is_lossless_compressed(brw, mt)) {
>    > +      assert(brw->gen >= 9);
>    > +      *layout = ISL_AUX_LAYOUT_CCS_E;
>    > +   } else if (mt->fast_clear_state != INTEL_FAST_CLEAR_STATE_NO_MCS)
>    {
>    > +      *layout = ISL_AUX_LAYOUT_CCS_D;
>    > +   } else {
>    > +      *layout = ISL_AUX_LAYOUT_NONE;
>    > +   }
> 
>      Logic below doesn't use the resolved value of "*layout". Would it be
>      cleaner
>      to have this if-else-ladder as its own small helper function?
> 
>    One version of these patches did exactly that.  Then I realized that
>    get_aux_isl_surf and get_aux_layout were always called together and
>    thought it was more clear to actually have them together.  I can split
>    it back out if you'd like.

Ok, that is what I sort of expected - I haven't gone through the rest of the
series yet. I'm fine leaving it as is - it is easy to split later on when
another user emerges.

>    --Jason
> 
>    > +
>    > +   /* Figure out the format of the auxiliary surface */
>    > +   switch (mt->num_samples) {
>    > +   case 0:
>    > +   case 1:
>    > +      /*
>    > +       * From the BDW PRM, Volume 2d, page 260
>    (RENDER_SURFACE_STATE):
>    > +       * "When MCS is enabled for non-MSRT, HALIGN_16 must be used"
>    > +       *
>    > +       * From the hardware spec for GEN9:
>    > +       * "When Auxiliary Surface Mode is set to AUX_CCS_D or
>    AUX_CCS_E, HALIGN
>    > +       *  16 must be used."
>    > +       */
>    > +      if (brw->gen >= 9 || mt->num_samples == 1)
>    > +         assert(mt->halign == 16);
>    > +
>    > +      if (brw->gen >= 9) {
>    > +         assert(mt->tiling == I915_TILING_Y);
>    > +         switch (_mesa_get_format_bytes(mt->format)) {
>    > +         case 4:  surf->format = ISL_FORMAT_GEN9_CCS_32BPP;   break;
>    > +         case 8:  surf->format = ISL_FORMAT_GEN9_CCS_64BPP;   break;
>    > +         case 16: surf->format = ISL_FORMAT_GEN9_CCS_128BPP;  break;
>    > +         default:
>    > +            unreachable("Invalid format size for color
>    compression");
>    > +         }
>    > +      } else if (mt->tiling == I915_TILING_Y) {
>    > +         switch (_mesa_get_format_bytes(mt->format)) {
>    > +         case 4:  surf->format = ISL_FORMAT_GEN7_CCS_32BPP_Y;
>    break;
>    > +         case 8:  surf->format = ISL_FORMAT_GEN7_CCS_64BPP_Y;
>    break;
>    > +         case 16: surf->format = ISL_FORMAT_GEN7_CCS_128BPP_X;
>    break;
>    > +         default:
>    > +            unreachable("Invalid format size for color
>    compression");
>    > +         }
>    > +      } else {
>    > +         assert(mt->tiling == I915_TILING_X);
>    > +         switch (_mesa_get_format_bytes(mt->format)) {
>    > +         case 4:  surf->format = ISL_FORMAT_GEN7_CCS_32BPP_X;
>    break;
>    > +         case 8:  surf->format = ISL_FORMAT_GEN7_CCS_64BPP_X;
>    break;
>    > +         case 16: surf->format = ISL_FORMAT_GEN7_CCS_128BPP_X;
>    break;
>    > +         default:
>    > +            unreachable("Invalid format size for color
>    compression");
>    > +         }
>    > +      }
>    > +      break;
>    > +
>    > +   case 2:
>    > +      surf->format = ISL_FORMAT_MCS_2X;
>    > +      break;
>    > +   case 4:
>    > +      surf->format = ISL_FORMAT_MCS_4X;
>    > +      break;
>    > +   case 8:
>    > +      surf->format = ISL_FORMAT_MCS_8X;
>    > +      break;
>    > +   case 16:
>    > +      surf->format = ISL_FORMAT_MCS_16X;
>    > +      break;
>    > +   default:
>    > +      unreachable("Invalid number of samples");
>    > +   }
>    > +}
>    > +
>    >  union isl_color_value
>    >  intel_miptree_get_isl_clear_color(struct brw_context *brw,
>    >                                    const struct intel_mipmap_tree
>    *mt)
>    > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>    b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>    > index a50f181..6422e42 100644
>    > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>    > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>    > @@ -801,6 +801,11 @@ void
>    >  intel_miptree_get_isl_surf(struct brw_context *brw,
>    >                             const struct intel_mipmap_tree *mt,
>    >                             struct isl_surf *surf);
>    > +void
>    > +intel_miptree_get_aux_isl_surf(struct brw_context *brw,
>    > +                               const struct intel_mipmap_tree *mt,
>    > +                               struct isl_surf *surf,
>    > +                               enum isl_aux_layout *layout);
>    >
>    >  union isl_color_value
>    >  intel_miptree_get_isl_clear_color(struct brw_context *brw,
>    > --
>    > 2.5.0.400.gff86faf
>    >
> 
>      > _______________________________________________
>      > mesa-dev mailing list
>      > [2]mesa-dev at lists.freedesktop.org
>      > [3]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> References
> 
>    1. mailto:topi.pohjolainen at intel.com
>    2. mailto:mesa-dev at lists.freedesktop.org
>    3. https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list