[PATCH 23/27] drm: Replace fb_helper->atomic with mode_config->atomic_commit

[Daniel Vetter]

On Sun, Jun 12, 2016 at 05:01:27PM +0800, Ying Liu wrote:
> On Wed, Jun 8, 2016 at 8:19 PM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> > Drivers transitioning to atomic might not yet want to enable full
> > DRIVER_ATOMIC support when it's not entirely working. But using atomic
> > internally makes a lot more sense earlier.
> >
> > Instead of spreading such flags to more places I figured it's simpler
> > to just check for mode_config->funcs->atomic_commit, and use atomic
> > paths if that is set. For the only driver currently transitioning
> > (i915) this does the right thing.
> 
> Well, imx-drm is transitioning.
> Unfortunately, after applying this patch, legacy fbdev cannot enable
> displays when the imx-drm transitional patch set reaches phase 3 step 1[1].
> 
> For those transitioning drivers with fine-grained steps, this patch
> is likely to expose the atomic function __drm_atomic_helper_set_config
> to legacy fbdev support too early. They could still be using
> drm_crtc_helper_set_config when adding ->atomic_commit.
> 
> BTW, this patch and those for generic nonblocking commit are making
> the imx-drm transition a bit harder.  Not good timing probably.
> But, I'll go on with the task anyway.

Hm, making transition harder wasn't really the intention. What exactly is
blowing up for you? At least how I planned the transition the first thing
you should be able to do is basic modesets (once you fill out
->atomic_commit), so I hoped that this wouldn't cause trouble.

Wrt nonblocking commit helpers breaking transitioning drivers: The most
likely cause is your driver isn't handling crtc_state->event yet. Before
you start using ->atomic_commit or one of the helpers to map legacy ioctl
to atomic, you need to fill out some code to handle that in ->atomic_begin
or ->atomic_flush. Then the transition should still work as before.

I'll be happy to help you out debug this, and then update my blog post
with the transition plan with the latest findings.

Thanks, Daniel

> 
> [1] https://patchwork.kernel.org/patch/9144035/
> 
> Regards,
> Liu Ying
> 
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c    |  6 ++----
> >  drivers/gpu/drm/i915/intel_fbdev.c |  2 --
> >  include/drm/drm_fb_helper.h        | 11 -----------
> >  3 files changed, 2 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index c0e0a2e78d75..ba2fcb2a68ad 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -385,7 +385,7 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> >
> >         drm_warn_on_modeset_not_all_locked(dev);
> >
> > -       if (fb_helper->atomic)
> > +       if (dev->mode_config.funcs->atomic_commit)
> >                 return restore_fbdev_mode_atomic(fb_helper);
> >
> >         drm_for_each_plane(plane, dev) {
> > @@ -716,8 +716,6 @@ int drm_fb_helper_init(struct drm_device *dev,
> >                 i++;
> >         }
> >
> > -       fb_helper->atomic = !!drm_core_check_feature(dev, DRIVER_ATOMIC);
> > -
> >         return 0;
> >  out_free:
> >         drm_fb_helper_crtc_free(fb_helper);
> > @@ -1344,7 +1342,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
> >                 return -EBUSY;
> >         }
> >
> > -       if (fb_helper->atomic) {
> > +       if (dev->mode_config.funcs->atomic_commit) {
> >                 ret = pan_display_atomic(var, info);
> >                 goto unlock;
> >         }
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > index ef8e67690f3d..4c725ad6fb54 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -724,8 +724,6 @@ int intel_fbdev_init(struct drm_device *dev)
> >                 return ret;
> >         }
> >
> > -       ifbdev->helper.atomic = true;
> > -
> >         dev_priv->fbdev = ifbdev;
> >         INIT_WORK(&dev_priv->fbdev_suspend_work, intel_fbdev_suspend_worker);
> >
> > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> > index 5b4aa35026a3..db8d4780eaa2 100644
> > --- a/include/drm/drm_fb_helper.h
> > +++ b/include/drm/drm_fb_helper.h
> > @@ -212,17 +212,6 @@ struct drm_fb_helper {
> >          * needs to be reprobe when fbdev is in control again.
> >          */
> >         bool delayed_hotplug;
> > -
> > -       /**
> > -        * @atomic:
> > -        *
> > -        * Use atomic updates for restore_fbdev_mode(), etc.  This defaults to
> > -        * true if driver has DRIVER_ATOMIC feature flag, but drivers can
> > -        * override it to true after drm_fb_helper_init() if they support atomic
> > -        * modeset but do not yet advertise DRIVER_ATOMIC (note that fb-helper
> > -        * does not require ASYNC commits).
> > -        */
> > -       bool atomic;
> >  };
> >
> >  #ifdef CONFIG_DRM_FBDEV_EMULATION
> > --
> > 2.8.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch