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@xxxxxxxxxxxxxxx wrote: > > >>>> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > >>>> > > >>>> 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@xxxxxxxxxxxxxxx> > > >>> > > >>> 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@xxxxxxxx> > > >> > > >> 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 ;-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
