[Intel-gfx] [PATCH 00/12] drm/i915: Fix up the CCS code

[Ville Syrjälä]

On Mon, Aug 28, 2017 at 02:35:54PM +0100, Daniel Stone wrote:
> Hi Daniel,
> 
> On 25 August 2017 at 18:17, Daniel Vetter <daniel at ffwll.ch> wrote:
> > Which of these do we need to cherry-pick over to -next-fixes? There's no
> > annotations about that. If the answer is "most" I'm leaning towards
> > disabling CCS for 4.14, minimal set would be ideal (and first in the patch
> > series).
> 
> My opinion below; tl;dr is that I don't think most of them are
> super-critical. Ville obviously has a far stronger opinion than me on
> the shape of the code, so I'm fine with this series, which seems to
> mostly be a merge back of the delta between whatever Ville's latest
> branch was, and whatever the last patchset Ben sent out was.
> 
> >> Ville Syrjälä (12):
> >>   drm/i915: Treat fb->offsets[] as a raw byte offset instead of a linear
> >>     offset
> 
> This should land into -fixes. I trust Ville that it has no UABI
> impact, but seems like something to be very consistent on.

It does change the uabi. That's the whole point. What was merged doesn't
agree with what userspace wants. So this we want in definitely so that
we don't end up exposing the wrong uabi in any released kernel.

> 
> >>   drm/i915: Skip fence alignemnt check for the CCS plane
> 
> Not sure if this is -fixes material really, just a cleanup?

It makes the kernel less likely to reject the fb entirely. So
without this userspace has to be rather careful where it places
the aux surface. I would include this as well.

> 
> >>   drm/i915: Switch over to the LLC/eLLC hotspot avoidance hash mode for
> >>     CCS
> 
> Not -fixes, performance optimisation.

We hope. It does change the layout of the compressed data though so if
our testcases try to generate compressed data with the CPU it'll not go
well if the test assumes the wrong hash mode. I would include this as
well so that we don't end up in any kind of a mess later when we try to
change it.

So the patches were more or less sorted in priority order, and we want
at least 01,02 and maybe 03.

> 
> >>   drm/i915: Add a comment exlaining CCS hsub/vsub
> 
> Seems harmless to land to -fixes.
> 
> >>   drm/i915: Nuke a pointless unreachable()
> 
> Ditto.
> 
> >>   drm/i915: Add the missing Y/Yf modifiers for SKL+ sprites
> 
> Per my previous reply, NAK to landing at all, since DDB/WM allocation
> seems too broken for it to work.
> 
> >>   drm/i915: Clean up the sprite modifier checks
> 
> Fine with this, but doesn't seem like -fixes material.
> 
> >>   drm/i915: Add CCS capability for sprites
> 
> NAK, same reason as Y/Yf.
> 
> >>   drm/i915: Allow up to 32KB stride on SKL+ "sprites"
> 
> Again doesn't seem like -fixes necessarily?
> 
> >>   drm: Fix modifiers_property kernel doc
> 
> Good for -fixes.
> 
> >>   drm: Check that the plane supports the request format+modifier combo
> 
> Good for core (not Intel) -fixes.
> 
> >>   drm/i915: Remove the pipe/plane ID checks from
> >>     skl_check_ccs_aux_surface()
> 
> Seems fine but probably not -fixes material; land in Intel after a merge?
> 
> Cheers,
> Daniel

-- 
Ville Syrjälä
Intel OTC