[Intel-gfx] [PATCH 00/22] drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic

[Daniel Vetter]

On Wed, Oct 14, 2015 at 06:59:38PM +0200, Daniel Vetter wrote:
> On Wed, Oct 14, 2015 at 07:28:52PM +0300, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > So while reviewing the NV12 stuff it became clear to me no one
> > had really given fb->offsets[] handling any serious thought.
> > So this patch series aims to fix that. We now treat fb->offsets[]
> > as a linear offset always. One clear benefit over treating it as
> > a linear offset as opposed to a raw byte offset is that we don't
> > have to think about the layout of bytes within the tile at all.
> > 
> > The series also generalizes the page rotation to be format agnostic,
> > the caller just specifies the desired geometry in pages for each
> > plane, and the rotation code builds up the sg. The intel_rotation_info
> > then just contains the minimal amount of information needed to
> > do the page rotation.
> > 
> > SKL+ also gets changed to use the compute_page_offset stuff so that
> > the plane SURF register will contain the closes (properly aligned)
> > page boundary, and the x/y offsets deal with whatever is left over.
> > The plane code for the other platforms also gets simpler in the end
> > I think. Also the 90/270 rotation handling becomes rather trivial
> > for the plane code.
> > 
> > I should still write some decent tests to exercise fb->offsets[].
> > 
> > Series available here:
> > git://github.com/vsyrjala/linux.git fb_offsets
> 
> As mentioned on irc patches 13&14 need to me unified with my 3 patch
> series, but otherwise lgtm overall.

Ok done a review pass and I think I unconfused myself about the bytes vs.
pixels fun. Imo that really needs to be a lot more explicit. My
suggestion:

- tile_width, tile_height, pixel_stride: always pixels
- tile_pitch, pitch: always bytes

And instead of silently changing units just use new variables. gcc will
clean it up. I'm totally ok with doing that as a follow-up.

I didn't review the 2nd last patch in detail since -ETOOBIG.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch