[Intel-gfx] [PATCH 11/22] drm/i915: Don't pass plane+plane_state to intel_pin_and_fence_fb_obj()

[Ville Syrjälä]

On Wed, Oct 21, 2015 at 02:22:10PM +0100, Tvrtko Ursulin wrote:
> 
> On 21/10/15 14:09, Ville Syrjälä wrote:
> > On Wed, Oct 21, 2015 at 01:17:10PM +0100, Tvrtko Ursulin wrote:
> >>
> >> On 21/10/15 12:28, Daniel Vetter wrote:
> >>> On Wed, Oct 14, 2015 at 07:29:03PM +0300, ville.syrjala at linux.intel.com wrote:
> >>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >>>>
> >>>> intel_pin_and_fence_fb_obj() only needs the framebuffer, and the desird
> >>>> rotation (to find the right GTT view for it), so no need to pass all
> >>>> kinds of plane stuff.
> >>>>
> >>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >>>
> >>> Feels indeed a bit like a bikeshed and just churn without resolving the
> >>> "pass vma in plane_state around" idea for real. But meh, it's imo not
> >>> worse than the existing code, and looks correct. Less churn would make me
> >>> a happier reviewer thought (there's a pile of whitespace in here too).
> >>>
> >>> Grumpily-Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >>
> >> It is worse, and is definitely not better. :(
> >
> > I might accept that if it wasn't for the ugly 'if (!plane_state)' mess.
> 
> Ok you have a point there. Maybe there should be a fake default state 
> available, if somehow possible? Would all zero be a valid state?

Let's not start adding fake stuff, that's going to be even more
confusing. IMO the correct approach is to have a low level function that
doesn't depend on the state (which is what we have after my patch more or
less), and then we can just wrap it up in nicer clothing for most normal
users.

-- 
Ville Syrjälä
Intel OTC