| Message ID | 1467128521-6037-1-git-send-email-bob.j.paauwe@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 28, 2016 at 08:42:01AM -0700, Bob Paauwe wrote: > When fbdev emulation is disabled it is not a good idea to call into > the core fb_set_suspend() function as this will cause suspend and > resume to fail. Also ifbdev->fb is null so the defeference farther > down will oops. Nothing in that path is needed in this case so bail > early when ifbdev->fb is null. This function does not exist when fbdev emulation is disabled. I suspect you mean something else, so please give the evidence of the problem this patch is addressing. -Chris
On Tue, 28 Jun 2016 16:47:28 +0100 Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Tue, Jun 28, 2016 at 08:42:01AM -0700, Bob Paauwe wrote: > > When fbdev emulation is disabled it is not a good idea to call into > > the core fb_set_suspend() function as this will cause suspend and > > resume to fail. Also ifbdev->fb is null so the defeference farther > > down will oops. Nothing in that path is needed in this case so bail > > early when ifbdev->fb is null. > > This function does not exist when fbdev emulation is disabled. I suspect > you mean something else, so please give the evidence of the problem this > patch is addressing. > -Chris > Sorry, I probably should have been more explicit. This happens when you set "drm_kms_helper.fbdev_emulation=0" on the command line. In that case the function is called and will crash during a S3 suspend and probably during hibernation as well. You're right, it isn't applicable when the kernel config CONFIG_DRM_FBDEV_EMULATION is off. Bob -- Bob Paauwe Bob.J.Paauwe@intel.com IOTG / PED Software Organization Intel Corp. Folsom, CA (916) 356-6193
On Tue, Jun 28, 2016 at 09:51:34AM -0700, Bob Paauwe wrote: > On Tue, 28 Jun 2016 16:47:28 +0100 > Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > On Tue, Jun 28, 2016 at 08:42:01AM -0700, Bob Paauwe wrote: > > > When fbdev emulation is disabled it is not a good idea to call into > > > the core fb_set_suspend() function as this will cause suspend and > > > resume to fail. Also ifbdev->fb is null so the defeference farther > > > down will oops. Nothing in that path is needed in this case so bail > > > early when ifbdev->fb is null. > > > > This function does not exist when fbdev emulation is disabled. I suspect > > you mean something else, so please give the evidence of the problem this > > patch is addressing. > > -Chris > > > > Sorry, I probably should have been more explicit. This happens when you > set "drm_kms_helper.fbdev_emulation=0" on the command line. In that > case the function is called and will crash during a S3 suspend and > probably during hibernation as well. > > You're right, it isn't applicable when the kernel config > CONFIG_DRM_FBDEV_EMULATION is off. Simply put, the bug is in drm_fb_helper_init(). Lots of drivers to review for their handling of errors from fbdev. -Chris
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index b75484a..9b14c13 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -767,7 +767,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous struct intel_fbdev *ifbdev = dev_priv->fbdev; struct fb_info *info; - if (!ifbdev) + if (!ifbdev || !ifbdev->fb) return; info = ifbdev->helper.fbdev;
When fbdev emulation is disabled it is not a good idea to call into the core fb_set_suspend() function as this will cause suspend and resume to fail. Also ifbdev->fb is null so the defeference farther down will oops. Nothing in that path is needed in this case so bail early when ifbdev->fb is null. Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> --- drivers/gpu/drm/i915/intel_fbdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)