<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Dec 22, 2017 at 11:22 AM, Ville Syrjala <span dir="ltr"><<a href="mailto:ville.syrjala@linux.intel.com" target="_blank">ville.syrjala@linux.intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Ville Syrjälä <<a href="mailto:ville.syrjala@linux.intel.com">ville.syrjala@linux.intel.com</a><wbr>><br>
<br>
Let's document why we claim hsub==8,vsub==16 for CCS even though the<br>
memory layout would suggest that we use 16x8 instead.<br>
<br>
Cc: Daniel Vetter <<a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a>><br>
Cc: Ben Widawsky <<a href="mailto:ben@bwidawsk.net">ben@bwidawsk.net</a>><br>
Cc: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br>
Cc: Daniel Stone <<a href="mailto:daniels@collabora.com">daniels@collabora.com</a>><br>
Signed-off-by: Ville Syrjälä <<a href="mailto:ville.syrjala@linux.intel.com">ville.syrjala@linux.intel.com</a><wbr>><br>
---<br>
 drivers/gpu/drm/i915/intel_<wbr>display.c | 7 +++++++<br>
 1 file changed, 7 insertions(+)<br>
<br>
diff --git a/drivers/gpu/drm/i915/intel_<wbr>display.c b/drivers/gpu/drm/i915/intel_<wbr>display.c<br>
index 0cd355978ab4..83aec68537b4 100644<br>
--- a/drivers/gpu/drm/i915/intel_<wbr>display.c<br>
+++ b/drivers/gpu/drm/i915/intel_<wbr>display.c<br>
@@ -2387,6 +2387,13 @@ static unsigned int intel_fb_modifier_to_tiling(<wbr>uint64_t fb_modifier)<br>
        }<br>
 }<br>
<br>
+/*<br>
+ * 1 byte of CCS actually corresponds to 16x8 pixels on the main<br>
+ * surface, and the memory layout for the CCS tile is 64x64 bytes.<br>
+ * But since we're pretending the CCS tile is 128 bytes wide we<br>
+ * adjust hsub/vsub here accordingly to 8x16 so that the<br>
+ * bytes<->x/y conversions come out correct.<br></blockquote><div><br></div><div>I'm not particularly happy with this comment as I think it pushes the mental model for these calculations in the wrong direction.  The PRM says:</div><div><br></div><div>The Color Control Surface (CCS) contains the compression status of the cache-line pairs. The<br>compression state of the cache-line pair is specified by 2 bits in the CCS. Each CCS cache-line represents<br>an area on the main surface of 16 x16 sets of 128 byte Y-tiled cache-line-pairs. CCS is always Y tiled.</div><div><br></div><div>If you understand that a "cache line pair" in the main surface is a horizontally adjacent cache line pair (cl1_addr = cl0_addr + 512) and you just accept the statement about Y-tiling, this is the correct calculation.  Calculating these things in terms of pixels is occasionally useful but is the wrong mental model.  The cache line statement above both accurately describes the layout of the CCS (at the cache line granularity) and scales to other pixel formats which are not 32-bit.</div><div><br></div><div>I know that Ville and I have disagreed on this in the past but I don't think adding comments about how we're "pretending the CCS tile is 128 bytes wide" is making anything more clear.</div><div><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ */<br>
 static const struct drm_format_info ccs_formats[] = {<br>
        { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 8, .vsub = 16, },<br>
        { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 8, .vsub = 16, },<br>
<span class="gmail-HOEnZb"><font color="#888888">--<br>
2.13.6<br>
<br>
</font></span></blockquote></div><br></div></div>