Re: [PATCH 1/3] drm/fbdev: Return -EBUSY when oopsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 28, 2015 at 7:18 AM, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
> Trying to do anything with kms drivers when oopsing has become a
> failing proposition. But since we can end up in the fbdev code simply
> due to the console unblanking that's done unconditionally just
> removing our panic handler isn't enough. We need to block all fbdev
> callbacks when oopsing.
>
> There was already one in the blank handler, but it failed silently.
> That makes it impossible for drivers (like i915) who subclass these
> functions to figure this out.
>
> Instead consistently return -EBUSY so that everyone knows that we
> really don't want to be bothered right now. This also allows us to
> remove a pile of FIXMEs from the i915 fbdev code (since due to the
> failure code they now won't attempt to grab dangerous locks any more).

I guess drivers that were keeping fbdev buffer pinned just for opps'
can stop doing that now..

anyways, lgtm.. for the series:

Reviewed-by: Rob Clark <robdclark@xxxxxxxxx>

> Cc: Dave Airlie <airlied@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_fb_helper.c    | 24 ++++++++++++------------
>  drivers/gpu/drm/i915/intel_fbdev.c | 21 ---------------------
>  2 files changed, 12 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 73f90f7e2f74..77f570a470da 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -491,14 +491,6 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
>         int i, j;
>
>         /*
> -        * fbdev->blank can be called from irq context in case of a panic.
> -        * Since we already have our own special panic handler which will
> -        * restore the fbdev console mode completely, just bail out early.
> -        */
> -       if (oops_in_progress)
> -               return;
> -
> -       /*
>          * For each CRTC in this fb, turn the connectors on/off.
>          */
>         drm_modeset_lock_all(dev);
> @@ -531,6 +523,9 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
>   */
>  int drm_fb_helper_blank(int blank, struct fb_info *info)
>  {
> +       if (oops_in_progress)
> +               return -EBUSY;
> +
>         switch (blank) {
>         /* Display: On; HSync: On, VSync: On */
>         case FB_BLANK_UNBLANK:
> @@ -755,9 +750,10 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>         int i, j, rc = 0;
>         int start;
>
> -       if (__drm_modeset_lock_all(dev, !!oops_in_progress)) {
> +       if (oops_in_progress)
>                 return -EBUSY;
> -       }
> +
> +       drm_modeset_lock_all(dev);
>         if (!drm_fb_helper_is_bound(fb_helper)) {
>                 drm_modeset_unlock_all(dev);
>                 return -EBUSY;
> @@ -906,6 +902,9 @@ int drm_fb_helper_set_par(struct fb_info *info)
>         struct drm_fb_helper *fb_helper = info->par;
>         struct fb_var_screeninfo *var = &info->var;
>
> +       if (oops_in_progress)
> +               return -EBUSY;
> +
>         if (var->pixclock != 0) {
>                 DRM_ERROR("PIXEL CLOCK SET\n");
>                 return -EINVAL;
> @@ -931,9 +930,10 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
>         int ret = 0;
>         int i;
>
> -       if (__drm_modeset_lock_all(dev, !!oops_in_progress)) {
> +       if (oops_in_progress)
>                 return -EBUSY;
> -       }
> +
> +       drm_modeset_lock_all(dev);
>         if (!drm_fb_helper_is_bound(fb_helper)) {
>                 drm_modeset_unlock_all(dev);
>                 return -EBUSY;
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index eef54feb7545..b763f24e20ef 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -55,13 +55,6 @@ static int intel_fbdev_set_par(struct fb_info *info)
>         ret = drm_fb_helper_set_par(info);
>
>         if (ret == 0) {
> -               /*
> -                * FIXME: fbdev presumes that all callbacks also work from
> -                * atomic contexts and relies on that for emergency oops
> -                * printing. KMS totally doesn't do that and the locking here is
> -                * by far not the only place this goes wrong.  Ignore this for
> -                * now until we solve this for real.
> -                */
>                 mutex_lock(&fb_helper->dev->struct_mutex);
>                 intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT);
>                 mutex_unlock(&fb_helper->dev->struct_mutex);
> @@ -80,13 +73,6 @@ static int intel_fbdev_blank(int blank, struct fb_info *info)
>         ret = drm_fb_helper_blank(blank, info);
>
>         if (ret == 0) {
> -               /*
> -                * FIXME: fbdev presumes that all callbacks also work from
> -                * atomic contexts and relies on that for emergency oops
> -                * printing. KMS totally doesn't do that and the locking here is
> -                * by far not the only place this goes wrong.  Ignore this for
> -                * now until we solve this for real.
> -                */
>                 mutex_lock(&fb_helper->dev->struct_mutex);
>                 intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT);
>                 mutex_unlock(&fb_helper->dev->struct_mutex);
> @@ -106,13 +92,6 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
>         ret = drm_fb_helper_pan_display(var, info);
>
>         if (ret == 0) {
> -               /*
> -                * FIXME: fbdev presumes that all callbacks also work from
> -                * atomic contexts and relies on that for emergency oops
> -                * printing. KMS totally doesn't do that and the locking here is
> -                * by far not the only place this goes wrong.  Ignore this for
> -                * now until we solve this for real.
> -                */
>                 mutex_lock(&fb_helper->dev->struct_mutex);
>                 intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT);
>                 mutex_unlock(&fb_helper->dev->struct_mutex);
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux