Message ID | 1401469930-32750-9-git-send-email-robdclark@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Fri, May 30, 2014 at 01:12:10PM -0400, Rob Clark wrote: > All drm_fb_helper_restore_fbdev_mode() call sites, save one, do the same > locking. Simplify this into drm_fb_helper_restore_fbdev_mode_unlocked(). > > Signed-off-by: Rob Clark <robdclark@gmail.com> Presuming I'm not blind the last such user is in the fb helper itself for the panic handler. Should we drop the EXPORT_SYMBOL for the helper? If we do that we might as well drop the _unlocked prefix and just remove locking from drivers ... -Daniel > --- > drivers/gpu/drm/armada/armada_fbdev.c | 4 +-- > drivers/gpu/drm/drm_fb_cma_helper.c | 9 ++---- > drivers/gpu/drm/drm_fb_helper.c | 51 +++++++++++++++++++++++-------- > drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 4 +-- > drivers/gpu/drm/gma500/psb_drv.c | 4 +-- > drivers/gpu/drm/i915/intel_fbdev.c | 6 +--- > drivers/gpu/drm/msm/msm_drv.c | 7 ++--- > drivers/gpu/drm/omapdrm/omap_drv.c | 4 +-- > drivers/gpu/drm/tegra/fb.c | 7 ++--- > include/drm/drm_fb_helper.h | 1 + > 10 files changed, 50 insertions(+), 47 deletions(-) > > diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c > index 948cb14..fd166f5 100644 > --- a/drivers/gpu/drm/armada/armada_fbdev.c > +++ b/drivers/gpu/drm/armada/armada_fbdev.c > @@ -181,10 +181,8 @@ void armada_fbdev_lastclose(struct drm_device *dev) > { > struct armada_private *priv = dev->dev_private; > > - drm_modeset_lock_all(dev); > if (priv->fbdev) > - drm_fb_helper_restore_fbdev_mode(priv->fbdev); > - drm_modeset_unlock_all(dev); > + drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); > } > > void armada_fbdev_fini(struct drm_device *dev) > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c > index 61b5a47..f27c883 100644 > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -429,13 +429,8 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini); > */ > void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma) > { > - if (fbdev_cma) { > - struct drm_device *dev = fbdev_cma->fb_helper.dev; > - > - drm_modeset_lock_all(dev); > - drm_fb_helper_restore_fbdev_mode(&fbdev_cma->fb_helper); > - drm_modeset_unlock_all(dev); > - } > + if (fbdev_cma) > + drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev_cma->fb_helper); > } > EXPORT_SYMBOL_GPL(drm_fbdev_cma_restore_mode); > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index b891f4c..dc861a1 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -273,15 +273,7 @@ int drm_fb_helper_debug_leave(struct fb_info *info) > } > EXPORT_SYMBOL(drm_fb_helper_debug_leave); > > -/** > - * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration > - * @fb_helper: fbcon to restore > - * > - * This should be called from driver's drm ->lastclose callback > - * when implementing an fbcon on top of kms using this helper. This ensures that > - * the user isn't greeted with a black screen when e.g. X dies. > - */ > -bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) > +static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) > { > struct drm_device *dev = fb_helper->dev; > struct drm_plane *plane; > @@ -311,8 +303,44 @@ bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) > } > return error; > } > +/** > + * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration > + * @fb_helper: fbcon to restore > + * @lockless: true in drm_fb_helper_force_kernel_mode() path, to avoid > + * blocking in panic case, everywhere else should use false > + * > + * This should be called from driver's drm ->lastclose callback > + * when implementing an fbcon on top of kms using this helper. This ensures that > + * the user isn't greeted with a black screen when e.g. X dies. > + * > + * Use this variant if you need to bypass locking (panic), or already > + * hold all modeset locks. Otherwise use drm_fb_helper_restore_fbdev_mode_unlocked() > + */ > +bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) > +{ > + return restore_fbdev_mode(fb_helper); > +} > EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode); > > +/** > + * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration > + * @fb_helper: fbcon to restore > + * > + * This should be called from driver's drm ->lastclose callback > + * when implementing an fbcon on top of kms using this helper. This ensures that > + * the user isn't greeted with a black screen when e.g. X dies. > + */ > +bool drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper) > +{ > + struct drm_device *dev = fb_helper->dev; > + bool ret; > + drm_modeset_lock_all(dev); > + ret = restore_fbdev_mode(fb_helper); > + drm_modeset_unlock_all(dev); > + return ret; > +} > +EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked); > + > /* > * restore fbcon display for all kms driver's using this helper, used for sysrq > * and panic handling. > @@ -824,7 +852,6 @@ EXPORT_SYMBOL(drm_fb_helper_check_var); > int drm_fb_helper_set_par(struct fb_info *info) > { > struct drm_fb_helper *fb_helper = info->par; > - struct drm_device *dev = fb_helper->dev; > struct fb_var_screeninfo *var = &info->var; > > if (var->pixclock != 0) { > @@ -832,9 +859,7 @@ int drm_fb_helper_set_par(struct fb_info *info) > return -EINVAL; > } > > - drm_modeset_lock_all(dev); > - drm_fb_helper_restore_fbdev_mode(fb_helper); > - drm_modeset_unlock_all(dev); > + drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper); > > if (fb_helper->delayed_hotplug) { > fb_helper->delayed_hotplug = false; > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > index addbf75..93e7ba6 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > @@ -375,7 +375,5 @@ void exynos_drm_fbdev_restore_mode(struct drm_device *dev) > if (!private || !private->fb_helper) > return; > > - drm_modeset_lock_all(dev); > - drm_fb_helper_restore_fbdev_mode(private->fb_helper); > - drm_modeset_unlock_all(dev); > + drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper); > } > diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c > index 0a3101a..59ea45e 100644 > --- a/drivers/gpu/drm/gma500/psb_drv.c > +++ b/drivers/gpu/drm/gma500/psb_drv.c > @@ -112,11 +112,9 @@ static void psb_driver_lastclose(struct drm_device *dev) > struct drm_psb_private *dev_priv = dev->dev_private; > struct psb_fbdev *fbdev = dev_priv->fbdev; > > - drm_modeset_lock_all(dev); > - ret = drm_fb_helper_restore_fbdev_mode(&fbdev->psb_fb_helper); > + ret = drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->psb_fb_helper); > if (ret) > DRM_DEBUG("failed to restore crtc mode\n"); > - drm_modeset_unlock_all(dev); > > return; > } > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index fce4a0d..f8258ab 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -683,11 +683,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev) > if (!dev_priv->fbdev) > return; > > - drm_modeset_lock_all(dev); > - > - ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper); > + ret = drm_fb_helper_restore_fbdev_mode_unlocked(&dev_priv->fbdev->helper); > if (ret) > DRM_DEBUG("failed to restore crtc mode\n"); > - > - drm_modeset_unlock_all(dev); > } > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index 50ec1be..afedd8b 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -382,11 +382,8 @@ static void msm_preclose(struct drm_device *dev, struct drm_file *file) > static void msm_lastclose(struct drm_device *dev) > { > struct msm_drm_private *priv = dev->dev_private; > - if (priv->fbdev) { > - drm_modeset_lock_all(dev); > - drm_fb_helper_restore_fbdev_mode(priv->fbdev); > - drm_modeset_unlock_all(dev); > - } > + if (priv->fbdev) > + drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); > } > > static irqreturn_t msm_irq(int irq, void *arg) > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c > index c8270e4..002b972 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -588,9 +588,7 @@ static void dev_lastclose(struct drm_device *dev) > } > } > > - drm_modeset_lock_all(dev); > - ret = drm_fb_helper_restore_fbdev_mode(priv->fbdev); > - drm_modeset_unlock_all(dev); > + ret = drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); > if (ret) > DBG("failed to restore crtc mode"); > } > diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c > index f7fca09..9798a70 100644 > --- a/drivers/gpu/drm/tegra/fb.c > +++ b/drivers/gpu/drm/tegra/fb.c > @@ -346,11 +346,8 @@ static void tegra_fbdev_free(struct tegra_fbdev *fbdev) > > void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev) > { > - if (fbdev) { > - drm_modeset_lock_all(fbdev->base.dev); > - drm_fb_helper_restore_fbdev_mode(&fbdev->base); > - drm_modeset_unlock_all(fbdev->base.dev); > - } > + if (fbdev) > + drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->base); > } > > static void tegra_fb_output_poll_changed(struct drm_device *drm) > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > index 6e622f7..0630b51 100644 > --- a/include/drm/drm_fb_helper.h > +++ b/include/drm/drm_fb_helper.h > @@ -109,6 +109,7 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > struct fb_info *info); > > bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper); > +bool drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper); > void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helper, > uint32_t fb_width, uint32_t fb_height); > void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch, > -- > 1.9.3 >
On Tue, Jun 3, 2014 at 9:33 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, May 30, 2014 at 01:12:10PM -0400, Rob Clark wrote: >> All drm_fb_helper_restore_fbdev_mode() call sites, save one, do the same >> locking. Simplify this into drm_fb_helper_restore_fbdev_mode_unlocked(). >> >> Signed-off-by: Rob Clark <robdclark@gmail.com> > > Presuming I'm not blind the last such user is in the fb helper itself for > the panic handler. Should we drop the EXPORT_SYMBOL for the helper? only reason I could think of for leaving (and for keeping the _unlocked suffix) was to catch merge issues.. leaving same name but with locking removed, it would compile fine and fail at runtime ;-) I can remove the prototype and EXPORT_SYMBOL I suppose just to be extra sure we catch everything (because once we start doing things atomically, it won't work if drivers still wrap it with their own _lock_all()) BR, -R > If we do that we might as well drop the _unlocked prefix and just remove > locking from drivers ... > -Daniel > >> --- >> drivers/gpu/drm/armada/armada_fbdev.c | 4 +-- >> drivers/gpu/drm/drm_fb_cma_helper.c | 9 ++---- >> drivers/gpu/drm/drm_fb_helper.c | 51 +++++++++++++++++++++++-------- >> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 4 +-- >> drivers/gpu/drm/gma500/psb_drv.c | 4 +-- >> drivers/gpu/drm/i915/intel_fbdev.c | 6 +--- >> drivers/gpu/drm/msm/msm_drv.c | 7 ++--- >> drivers/gpu/drm/omapdrm/omap_drv.c | 4 +-- >> drivers/gpu/drm/tegra/fb.c | 7 ++--- >> include/drm/drm_fb_helper.h | 1 + >> 10 files changed, 50 insertions(+), 47 deletions(-) >> >> diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c >> index 948cb14..fd166f5 100644 >> --- a/drivers/gpu/drm/armada/armada_fbdev.c >> +++ b/drivers/gpu/drm/armada/armada_fbdev.c >> @@ -181,10 +181,8 @@ void armada_fbdev_lastclose(struct drm_device *dev) >> { >> struct armada_private *priv = dev->dev_private; >> >> - drm_modeset_lock_all(dev); >> if (priv->fbdev) >> - drm_fb_helper_restore_fbdev_mode(priv->fbdev); >> - drm_modeset_unlock_all(dev); >> + drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); >> } >> >> void armada_fbdev_fini(struct drm_device *dev) >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c >> index 61b5a47..f27c883 100644 >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -429,13 +429,8 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini); >> */ >> void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma) >> { >> - if (fbdev_cma) { >> - struct drm_device *dev = fbdev_cma->fb_helper.dev; >> - >> - drm_modeset_lock_all(dev); >> - drm_fb_helper_restore_fbdev_mode(&fbdev_cma->fb_helper); >> - drm_modeset_unlock_all(dev); >> - } >> + if (fbdev_cma) >> + drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev_cma->fb_helper); >> } >> EXPORT_SYMBOL_GPL(drm_fbdev_cma_restore_mode); >> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >> index b891f4c..dc861a1 100644 >> --- a/drivers/gpu/drm/drm_fb_helper.c >> +++ b/drivers/gpu/drm/drm_fb_helper.c >> @@ -273,15 +273,7 @@ int drm_fb_helper_debug_leave(struct fb_info *info) >> } >> EXPORT_SYMBOL(drm_fb_helper_debug_leave); >> >> -/** >> - * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration >> - * @fb_helper: fbcon to restore >> - * >> - * This should be called from driver's drm ->lastclose callback >> - * when implementing an fbcon on top of kms using this helper. This ensures that >> - * the user isn't greeted with a black screen when e.g. X dies. >> - */ >> -bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) >> +static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) >> { >> struct drm_device *dev = fb_helper->dev; >> struct drm_plane *plane; >> @@ -311,8 +303,44 @@ bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) >> } >> return error; >> } >> +/** >> + * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration >> + * @fb_helper: fbcon to restore >> + * @lockless: true in drm_fb_helper_force_kernel_mode() path, to avoid >> + * blocking in panic case, everywhere else should use false >> + * >> + * This should be called from driver's drm ->lastclose callback >> + * when implementing an fbcon on top of kms using this helper. This ensures that >> + * the user isn't greeted with a black screen when e.g. X dies. >> + * >> + * Use this variant if you need to bypass locking (panic), or already >> + * hold all modeset locks. Otherwise use drm_fb_helper_restore_fbdev_mode_unlocked() >> + */ >> +bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) >> +{ >> + return restore_fbdev_mode(fb_helper); >> +} >> EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode); >> >> +/** >> + * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration >> + * @fb_helper: fbcon to restore >> + * >> + * This should be called from driver's drm ->lastclose callback >> + * when implementing an fbcon on top of kms using this helper. This ensures that >> + * the user isn't greeted with a black screen when e.g. X dies. >> + */ >> +bool drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper) >> +{ >> + struct drm_device *dev = fb_helper->dev; >> + bool ret; >> + drm_modeset_lock_all(dev); >> + ret = restore_fbdev_mode(fb_helper); >> + drm_modeset_unlock_all(dev); >> + return ret; >> +} >> +EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked); >> + >> /* >> * restore fbcon display for all kms driver's using this helper, used for sysrq >> * and panic handling. >> @@ -824,7 +852,6 @@ EXPORT_SYMBOL(drm_fb_helper_check_var); >> int drm_fb_helper_set_par(struct fb_info *info) >> { >> struct drm_fb_helper *fb_helper = info->par; >> - struct drm_device *dev = fb_helper->dev; >> struct fb_var_screeninfo *var = &info->var; >> >> if (var->pixclock != 0) { >> @@ -832,9 +859,7 @@ int drm_fb_helper_set_par(struct fb_info *info) >> return -EINVAL; >> } >> >> - drm_modeset_lock_all(dev); >> - drm_fb_helper_restore_fbdev_mode(fb_helper); >> - drm_modeset_unlock_all(dev); >> + drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper); >> >> if (fb_helper->delayed_hotplug) { >> fb_helper->delayed_hotplug = false; >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c >> index addbf75..93e7ba6 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c >> @@ -375,7 +375,5 @@ void exynos_drm_fbdev_restore_mode(struct drm_device *dev) >> if (!private || !private->fb_helper) >> return; >> >> - drm_modeset_lock_all(dev); >> - drm_fb_helper_restore_fbdev_mode(private->fb_helper); >> - drm_modeset_unlock_all(dev); >> + drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper); >> } >> diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c >> index 0a3101a..59ea45e 100644 >> --- a/drivers/gpu/drm/gma500/psb_drv.c >> +++ b/drivers/gpu/drm/gma500/psb_drv.c >> @@ -112,11 +112,9 @@ static void psb_driver_lastclose(struct drm_device *dev) >> struct drm_psb_private *dev_priv = dev->dev_private; >> struct psb_fbdev *fbdev = dev_priv->fbdev; >> >> - drm_modeset_lock_all(dev); >> - ret = drm_fb_helper_restore_fbdev_mode(&fbdev->psb_fb_helper); >> + ret = drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->psb_fb_helper); >> if (ret) >> DRM_DEBUG("failed to restore crtc mode\n"); >> - drm_modeset_unlock_all(dev); >> >> return; >> } >> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c >> index fce4a0d..f8258ab 100644 >> --- a/drivers/gpu/drm/i915/intel_fbdev.c >> +++ b/drivers/gpu/drm/i915/intel_fbdev.c >> @@ -683,11 +683,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev) >> if (!dev_priv->fbdev) >> return; >> >> - drm_modeset_lock_all(dev); >> - >> - ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper); >> + ret = drm_fb_helper_restore_fbdev_mode_unlocked(&dev_priv->fbdev->helper); >> if (ret) >> DRM_DEBUG("failed to restore crtc mode\n"); >> - >> - drm_modeset_unlock_all(dev); >> } >> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c >> index 50ec1be..afedd8b 100644 >> --- a/drivers/gpu/drm/msm/msm_drv.c >> +++ b/drivers/gpu/drm/msm/msm_drv.c >> @@ -382,11 +382,8 @@ static void msm_preclose(struct drm_device *dev, struct drm_file *file) >> static void msm_lastclose(struct drm_device *dev) >> { >> struct msm_drm_private *priv = dev->dev_private; >> - if (priv->fbdev) { >> - drm_modeset_lock_all(dev); >> - drm_fb_helper_restore_fbdev_mode(priv->fbdev); >> - drm_modeset_unlock_all(dev); >> - } >> + if (priv->fbdev) >> + drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); >> } >> >> static irqreturn_t msm_irq(int irq, void *arg) >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c >> index c8270e4..002b972 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >> @@ -588,9 +588,7 @@ static void dev_lastclose(struct drm_device *dev) >> } >> } >> >> - drm_modeset_lock_all(dev); >> - ret = drm_fb_helper_restore_fbdev_mode(priv->fbdev); >> - drm_modeset_unlock_all(dev); >> + ret = drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); >> if (ret) >> DBG("failed to restore crtc mode"); >> } >> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c >> index f7fca09..9798a70 100644 >> --- a/drivers/gpu/drm/tegra/fb.c >> +++ b/drivers/gpu/drm/tegra/fb.c >> @@ -346,11 +346,8 @@ static void tegra_fbdev_free(struct tegra_fbdev *fbdev) >> >> void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev) >> { >> - if (fbdev) { >> - drm_modeset_lock_all(fbdev->base.dev); >> - drm_fb_helper_restore_fbdev_mode(&fbdev->base); >> - drm_modeset_unlock_all(fbdev->base.dev); >> - } >> + if (fbdev) >> + drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->base); >> } >> >> static void tegra_fb_output_poll_changed(struct drm_device *drm) >> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h >> index 6e622f7..0630b51 100644 >> --- a/include/drm/drm_fb_helper.h >> +++ b/include/drm/drm_fb_helper.h >> @@ -109,6 +109,7 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, >> struct fb_info *info); >> >> bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper); >> +bool drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper); >> void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helper, >> uint32_t fb_width, uint32_t fb_height); >> void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch, >> -- >> 1.9.3 >> > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c index 948cb14..fd166f5 100644 --- a/drivers/gpu/drm/armada/armada_fbdev.c +++ b/drivers/gpu/drm/armada/armada_fbdev.c @@ -181,10 +181,8 @@ void armada_fbdev_lastclose(struct drm_device *dev) { struct armada_private *priv = dev->dev_private; - drm_modeset_lock_all(dev); if (priv->fbdev) - drm_fb_helper_restore_fbdev_mode(priv->fbdev); - drm_modeset_unlock_all(dev); + drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); } void armada_fbdev_fini(struct drm_device *dev) diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 61b5a47..f27c883 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -429,13 +429,8 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini); */ void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma) { - if (fbdev_cma) { - struct drm_device *dev = fbdev_cma->fb_helper.dev; - - drm_modeset_lock_all(dev); - drm_fb_helper_restore_fbdev_mode(&fbdev_cma->fb_helper); - drm_modeset_unlock_all(dev); - } + if (fbdev_cma) + drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev_cma->fb_helper); } EXPORT_SYMBOL_GPL(drm_fbdev_cma_restore_mode); diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index b891f4c..dc861a1 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -273,15 +273,7 @@ int drm_fb_helper_debug_leave(struct fb_info *info) } EXPORT_SYMBOL(drm_fb_helper_debug_leave); -/** - * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration - * @fb_helper: fbcon to restore - * - * This should be called from driver's drm ->lastclose callback - * when implementing an fbcon on top of kms using this helper. This ensures that - * the user isn't greeted with a black screen when e.g. X dies. - */ -bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) +static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) { struct drm_device *dev = fb_helper->dev; struct drm_plane *plane; @@ -311,8 +303,44 @@ bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) } return error; } +/** + * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration + * @fb_helper: fbcon to restore + * @lockless: true in drm_fb_helper_force_kernel_mode() path, to avoid + * blocking in panic case, everywhere else should use false + * + * This should be called from driver's drm ->lastclose callback + * when implementing an fbcon on top of kms using this helper. This ensures that + * the user isn't greeted with a black screen when e.g. X dies. + * + * Use this variant if you need to bypass locking (panic), or already + * hold all modeset locks. Otherwise use drm_fb_helper_restore_fbdev_mode_unlocked() + */ +bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) +{ + return restore_fbdev_mode(fb_helper); +} EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode); +/** + * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration + * @fb_helper: fbcon to restore + * + * This should be called from driver's drm ->lastclose callback + * when implementing an fbcon on top of kms using this helper. This ensures that + * the user isn't greeted with a black screen when e.g. X dies. + */ +bool drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper) +{ + struct drm_device *dev = fb_helper->dev; + bool ret; + drm_modeset_lock_all(dev); + ret = restore_fbdev_mode(fb_helper); + drm_modeset_unlock_all(dev); + return ret; +} +EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked); + /* * restore fbcon display for all kms driver's using this helper, used for sysrq * and panic handling. @@ -824,7 +852,6 @@ EXPORT_SYMBOL(drm_fb_helper_check_var); int drm_fb_helper_set_par(struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; - struct drm_device *dev = fb_helper->dev; struct fb_var_screeninfo *var = &info->var; if (var->pixclock != 0) { @@ -832,9 +859,7 @@ int drm_fb_helper_set_par(struct fb_info *info) return -EINVAL; } - drm_modeset_lock_all(dev); - drm_fb_helper_restore_fbdev_mode(fb_helper); - drm_modeset_unlock_all(dev); + drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper); if (fb_helper->delayed_hotplug) { fb_helper->delayed_hotplug = false; diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index addbf75..93e7ba6 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -375,7 +375,5 @@ void exynos_drm_fbdev_restore_mode(struct drm_device *dev) if (!private || !private->fb_helper) return; - drm_modeset_lock_all(dev); - drm_fb_helper_restore_fbdev_mode(private->fb_helper); - drm_modeset_unlock_all(dev); + drm_fb_helper_restore_fbdev_mode_unlocked(private->fb_helper); } diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 0a3101a..59ea45e 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -112,11 +112,9 @@ static void psb_driver_lastclose(struct drm_device *dev) struct drm_psb_private *dev_priv = dev->dev_private; struct psb_fbdev *fbdev = dev_priv->fbdev; - drm_modeset_lock_all(dev); - ret = drm_fb_helper_restore_fbdev_mode(&fbdev->psb_fb_helper); + ret = drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->psb_fb_helper); if (ret) DRM_DEBUG("failed to restore crtc mode\n"); - drm_modeset_unlock_all(dev); return; } diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index fce4a0d..f8258ab 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -683,11 +683,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev) if (!dev_priv->fbdev) return; - drm_modeset_lock_all(dev); - - ret = drm_fb_helper_restore_fbdev_mode(&dev_priv->fbdev->helper); + ret = drm_fb_helper_restore_fbdev_mode_unlocked(&dev_priv->fbdev->helper); if (ret) DRM_DEBUG("failed to restore crtc mode\n"); - - drm_modeset_unlock_all(dev); } diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 50ec1be..afedd8b 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -382,11 +382,8 @@ static void msm_preclose(struct drm_device *dev, struct drm_file *file) static void msm_lastclose(struct drm_device *dev) { struct msm_drm_private *priv = dev->dev_private; - if (priv->fbdev) { - drm_modeset_lock_all(dev); - drm_fb_helper_restore_fbdev_mode(priv->fbdev); - drm_modeset_unlock_all(dev); - } + if (priv->fbdev) + drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); } static irqreturn_t msm_irq(int irq, void *arg) diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index c8270e4..002b972 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -588,9 +588,7 @@ static void dev_lastclose(struct drm_device *dev) } } - drm_modeset_lock_all(dev); - ret = drm_fb_helper_restore_fbdev_mode(priv->fbdev); - drm_modeset_unlock_all(dev); + ret = drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev); if (ret) DBG("failed to restore crtc mode"); } diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c index f7fca09..9798a70 100644 --- a/drivers/gpu/drm/tegra/fb.c +++ b/drivers/gpu/drm/tegra/fb.c @@ -346,11 +346,8 @@ static void tegra_fbdev_free(struct tegra_fbdev *fbdev) void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev) { - if (fbdev) { - drm_modeset_lock_all(fbdev->base.dev); - drm_fb_helper_restore_fbdev_mode(&fbdev->base); - drm_modeset_unlock_all(fbdev->base.dev); - } + if (fbdev) + drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev->base); } static void tegra_fb_output_poll_changed(struct drm_device *drm) diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 6e622f7..0630b51 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -109,6 +109,7 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, struct fb_info *info); bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper); +bool drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper); void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helper, uint32_t fb_width, uint32_t fb_height); void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
All drm_fb_helper_restore_fbdev_mode() call sites, save one, do the same locking. Simplify this into drm_fb_helper_restore_fbdev_mode_unlocked(). Signed-off-by: Rob Clark <robdclark@gmail.com> --- drivers/gpu/drm/armada/armada_fbdev.c | 4 +-- drivers/gpu/drm/drm_fb_cma_helper.c | 9 ++---- drivers/gpu/drm/drm_fb_helper.c | 51 +++++++++++++++++++++++-------- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 4 +-- drivers/gpu/drm/gma500/psb_drv.c | 4 +-- drivers/gpu/drm/i915/intel_fbdev.c | 6 +--- drivers/gpu/drm/msm/msm_drv.c | 7 ++--- drivers/gpu/drm/omapdrm/omap_drv.c | 4 +-- drivers/gpu/drm/tegra/fb.c | 7 ++--- include/drm/drm_fb_helper.h | 1 + 10 files changed, 50 insertions(+), 47 deletions(-)