[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 05:20:02PM +0200, Daniel Vetter wrote:
> On Wed, Oct 21, 2015 at 05:22:40PM +0300, Ville Syrjälä wrote:
> > 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.
> 
> I agree, my only complaint is that it's hard to judge the low-level polish
> without the pretty clothes on top ;-)

Quality tailoring requires decent measurements, or we might end up with
a clown costume instead of a nice suit. At least I haven't really thought
through what we really want here.

-- 
Ville Syrjälä
Intel OTC