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.pohjolai...@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@lists.freedesktop.org
> > [3]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> References
>
> 1. mailto:topi.pohjolai...@intel.com
> 2. mailto:mesa-dev@lists.freedesktop.org
> 3. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
