[Intel-gfx] [PATCH 2/6] drm/atomic: Unconditionally call prepare_fb.

[Daniel Vetter]

On Tue, Dec 13, 2016 at 03:13:54PM +0100, Maarten Lankhorst wrote:
> Op 09-12-16 om 09:25 schreef Daniel Vetter:
> > On Fri, Dec 09, 2016 at 12:42:19AM +0200, Laurent Pinchart wrote:
> >> Hi Daniel,
> >>
> >> On Thursday 08 Dec 2016 16:41:04 Daniel Vetter wrote:
> >>> On Thu, Dec 08, 2016 at 02:45:25PM +0100, Maarten Lankhorst wrote:
> >>>> Atomic drivers may set properties like rotation on the same fb, which
> >>>> may require a call to prepare_fb even when framebuffer stays identical.
> >>>>
> >>>> Instead of handling all the special cases in the core, let the driver
> >>>> decide when prepare_fb and cleanup_fb are noops.
> >>>>
> >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> >>> I think this makes sense, but would be really good to get a pile of acks
> >>> from driver maintainers on this one. Rob, Eric, Laurent, others?
> >> This is all very nice, but it will introduce at least a performance 
> >> regression, and possibly worse, until drivers get updated. There are 7 drivers 
> >> implementing the .prepare_fb() callback (plus a bunch of drivers that probably 
> >> should use drm_fb_cma_prepare_fb() but don't at the moment). I can't ack this 
> >> patch before they get fixed.
> > Maarten's commit message is insufficient, since this is defacto a revert
> > of
> >
> > commit fcc60b413d14dd06ddbd79ec50e83c4fb2a097ba
> > Author: Keith Packard <keithp at keithp.com>
> > Date:   Sat Jun 4 01:16:22 2016 -0700
> >
> >     drm: Don't prepare or cleanup unchanging frame buffers [v3]
> >
> > because that breaks stuff. We're simply going back to where we've been a
> > few months ago. Since this is a regression fix, back to original
> > behaviour, can you ack (assuming Maarten updates the commit message to
> > reflect the nature of the commit here)?
> 
> Waiting on a reply, but what about this commit message for this patch?
> ---
> Atomic drivers may set properties like rotation on the same fb, which
> may require a call to prepare_fb even when framebuffer stays identical.
> 
> Instead of handling all the special cases in the core, let the driver
> decide when prepare_fb and cleanup_fb are noops.
> 
> This is a revert of:
> 
> commit fcc60b413d14dd06ddbd79ec50e83c4fb2a097ba
> Author: Keith Packard <keithp at keithp.com>
> Date:   Sat Jun 4 01:16:22 2016 -0700
> 
>     drm: Don't prepare or cleanup unchanging frame buffers [v3]
> 
> The original commit mentions that this prevents waiting in i915 on all
> previous rendering during cursor updates, but there are better ways to
> fix this.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>

Yeah sounds good to me. Since we don't want to backport all the i915
cursor patches no cc: stable on this. Also, this is only an issue for
drivers which both have a cursor plane, and implement that cursor using
universal planes (i.e. settting drm_crtc->cursor). Afaik the only two are
vc4 and i915, and after this series both will have appropriate hacks (for
now) to keep existing userspace happy.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch