On Wed, Jun 07, 2017 at 12:44:47PM +0100, Daniel Stone wrote:
> Hi Vidya,
>
> On 7 June 2017 at 12:40, Vidya Srinivas <vidya.srinivas@xxxxxxxxx> wrote:
> > +static const struct drm_format_info ccs_formats[] = {
> > + { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> > + { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> > + { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> > + { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> > +};
>
> I notice that here hsub/vsub are declared as 16x8. In Ville's tree
> which I pulled my submission from, this is 8x16, which aligns with
> this comment (missing from your submission):
> /*
> * 1 byte of CCS actually corresponds to 16x8 pixels on the main
> * surface, and the memory layout for the CCS tile us 64x64 bytes.
> * But since we're pretending the CCS tile is 128 bytes wide we
> * adjust hsub/vsub here accordingly to 8x16 so that the
> * bytes<->x/y conversions come out correct.
> */
>
> If the hsub/vsub is inverted to match the comment, trying to add a
> 3200x1800 (for example) framebuffer will fail, because (1800 % 16) !=
> 0. This is true even if the allocation is correct (i.e. the buffer
> contains an even number of tiles for both width and height). Generic
> userspace cannot know that it should try to create a larger
> framebuffer (in this case 3200x1808) and only show a smaller region of
> that framebuffer.
>
> This is the reason for the halign/valign patch mentioned earlier, as
> well as the additional part of the comment which was also present in
> my submission:
> /*
> * We don't require any
> * CCS block size alignment of the fb under the assumption that the
> * hardware will handle things correctly of only a single pixel
> * gets touched. The compression should be lossless so any garbage
> * pixels as part of the same block shouldn't cause visual artifacts.
> */
The alignment requirement is gone in upstream, hence my latest CCS
stuff doesn't have the valign/halign stuff anymore.
Anyways, I'll have to revisit the the offsets[] thing because people
didn't like my original linear offset idea, and it doesn't match what
userspace already does.
And I still need to convince myself that the ccs hash mode won't be
an issue, which I think I'm close to doing since I managed to trick
igt rendercopy to do ccs.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
