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.
> > ---
> >
> > drivers/gpu/drm/drm_atomic_helper.c | 9 ---------
> > 1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c index 23767df72615..d19563651e07
> > 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1664,9 +1664,6 @@ int drm_atomic_helper_prepare_planes(struct
> > drm_device *dev,>
> > funcs = plane->helper_private;
> >
> > - if (!drm_atomic_helper_framebuffer_changed(dev, state,
> > plane_state->crtc)) - continue;
> > -
> >
> > if (funcs->prepare_fb) {
> >
> > ret = funcs->prepare_fb(plane, plane_state);
> > if (ret)
> >
> > @@ -1683,9 +1680,6 @@ int drm_atomic_helper_prepare_planes(struct
> > drm_device *dev,>
> > if (j >= i)
> >
> > continue;
> >
> > - if (!drm_atomic_helper_framebuffer_changed(dev, state,
> > plane_state->crtc)) - continue;
> > -
> >
> > funcs = plane->helper_private;
> >
> > if (funcs->cleanup_fb)
> >
> > @@ -1952,9 +1946,6 @@ void drm_atomic_helper_cleanup_planes(struct
> > drm_device *dev,>
> > for_each_plane_in_state(old_state, plane, plane_state, i) {
> >
> > const struct drm_plane_helper_funcs *funcs;
> >
> > - if (!drm_atomic_helper_framebuffer_changed(dev, old_state,
> > plane_state->crtc)) - continue;
> > -
> >
> > funcs = plane->helper_private;
> >
> > if (funcs->cleanup_fb)
--
Regards,
Laurent Pinchart
