On Wed, Nov 18, 2015 at 10:28:27AM -0800, Chad Versace wrote: > On Tue 17 Nov 2015, Ben Widawsky wrote: > > Background: Prior to Skylake and since Ivybridge Intel hardware has had the > > ability to use a MCS (Multisample Control Surface) as auxiliary data in > > "compression" operations on the surface. This reduces memory bandwidth. > > This > > hardware was either used for MSAA compression, and fast clear operations. > > On > > Gen8, a similar mechanism exists to allow the hiz buffer to be sampled > > from, and > > therefore this feature is sometimes referred to more generally as "AUX > > buffers". > > > > Skylake adds the ability to have the display engine directly source > > compressed > > surfaces on top of the ability to sample from them. Inference dictates that > > enabling this display features adding a restriction to the formats which > > could > > actually be compressed. The current set of surfaces seems to be a subset as > > compared to previous gens (see the next patch). Also, if I had to guess I > > would > > guess that future gens add support for more surface formats. To make > > handling > > this a bit easier to read, and more future proof, the support for this is > > moved > > into the surface formats table. > > > > Along with the modifications to the table, a helper function is also > > provided to > > determine if a surface is CCS compatible. Because fast clears are currently > ----------------------------^^^ > Should say "CCS_E".
I don't understand why you are insisting on calling everything CCS_E. Admittedly, the docs aren't crystal clear here, but here is how I understand it. For single sample render compression - the thing we're not using yet, you use CCS_E. For single sample fast clears, you use CCS_D (with the restriction that such surfaces must be supported for render compression when using the sampler). For multisample compression, I don't understand the difference. More confusingly, the bit we're actually setting in the surface state is CCS_D, so the last change you asked me to make (renaming ccs to ccs_e in the table) didn't make sense to me - but for the sake of making progress, I just changed it to make you happy. However, since you're now asking for another version without leaving an R-B, I really must know what is going on in your head. Can you prove to me from the docs why it's not accurate to say CCS, or if you must CCS_*, and why it is MORE accurate to say CCS_E? I will happily change it for you if you give me a convincing answer. > > > disabled on SKL, we can plumb the helper all the way through here, and not > > actually have anything break. > > > > The logic in the table works a bit differently than the other columns in the > > table and therefore deserves a small mention. For most other features, the > > GEN > > which began implementing it is set, and it is assumed future gens also > > support > > this. For this feature, GEN9 actually eliminates support for certain > > formats. We > > could use this column to determine support for the similar feature on older > > generation hardware. Aside from that being an error prone task which is > > unrelated to enabling this on GEN9, it becomes somewhat tricky to implement > > because of the fact that surface format support diminishes. You'd probably > > want > > another column to cleanly implement it. > > Does the above paragraph still apply to the table's ccs_e column? > I understand your patch series, the ccs_e column behaves identically to > all other columns: > > feature_is_supported == (10 * gen >= table[format].feature) > > The patch's diff looks good to me. My only remaining questions/issues > with the patch are the ones stated in this message. -- Ben Widawsky, Intel Open Source Technology Center _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
