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. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
