From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 12 Jan 2015 17:43:34 +0000 From: Luis Henriques To: Daniel Vetter Cc: Ben Hutchings , Chris Wilson , Greg Kroah-Hartman , Linux Kernel Mailing List , stable , kernel-team@lists.ubuntu.com, "Nikula, Jani" , intel-gfx Subject: Re: [PATCH 3.16.y-ckt 030/168] drm/i915: Handle failure to kick out a conflicting fb driver Message-ID: <20150112174334.GI2455@charon> References: <1418653622-21105-1-git-send-email-luis.henriques@canonical.com> <1418653622-21105-31-git-send-email-luis.henriques@canonical.com> <1421012945.19708.27.camel@decadent.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: On Mon, Jan 12, 2015 at 06:20:22PM +0100, Daniel Vetter wrote: > On Sun, Jan 11, 2015 at 10:49 PM, Ben Hutchings wrote: > > On Mon, 2014-12-15 at 14:24 +0000, Luis Henriques wrote: > >> 3.16.7-ckt3 -stable review patch. If anyone has any objections, please let me know. > >> > >> ------------------ > >> > >> From: Chris Wilson > >> > >> commit f96de58fc7e7d3d717c7c63975c3b896c906b5e3 upstream. > >> > >> Signed-off-by: Chris Wilson > >> Reviewed-by: Jani Nikula > >> Signed-off-by: Daniel Vetter > >> Signed-off-by: Luis Henriques > > > > Should this also be applied to any older stable branches? > > > > i915_kick_out_firmware_fb() was introduced in 3.6 and it has always been > > possible for the alloc_apertures() call to fail. > > > > remove_conflicting_framebuffers() has returned an error code since 3.14 > > (but could silently fail before then!) so this should be applicable to > > the 3.14 stable branch too. > > tbh I don't know why this patch ended up in a stable kernel, at least > I didn't find anything where we (drm/i915 maintainers) marked it as > such. And there's no bugzilla references added either. Imo the patch > doesn't qualify for stable (it's not a real-world bug afaik). You're right, this patch was not tagged for stable. While applying 0485c9dc24ec ("drm/i915: Kick fbdev before vgacon") to the 3.16 kernel, I found that cherry-picking f96de58fc7e7 would make it apply cleanly and it didn't shock me to have it in a stable kernel. That's why I applied it for release 3.16.7-ckt3. I guess I should have added a note in the commit text justifying its presence. Cheers, -- Lu�s > -Daniel > > > > > Ben. > > > >> --- > >> drivers/gpu/drm/i915/i915_dma.c | 18 +++++++++++++----- > >> 1 file changed, 13 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > >> index d44344140627..0663fb4fa606 100644 > >> --- a/drivers/gpu/drm/i915/i915_dma.c > >> +++ b/drivers/gpu/drm/i915/i915_dma.c > >> @@ -1425,15 +1425,16 @@ void i915_master_destroy(struct drm_device *dev, struct drm_master *master) > >> } > >> > >> #if IS_ENABLED(CONFIG_FB) > >> -static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv) > >> +static int i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv) > >> { > >> struct apertures_struct *ap; > >> struct pci_dev *pdev = dev_priv->dev->pdev; > >> bool primary; > >> + int ret; > >> > >> ap = alloc_apertures(1); > >> if (!ap) > >> - return; > >> + return -ENOMEM; > >> > >> ap->ranges[0].base = dev_priv->gtt.mappable_base; > >> ap->ranges[0].size = dev_priv->gtt.mappable_end; > >> @@ -1441,13 +1442,16 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv) > >> primary = > >> pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW; > >> > >> - remove_conflicting_framebuffers(ap, "inteldrmfb", primary); > >> + ret = remove_conflicting_framebuffers(ap, "inteldrmfb", primary); > >> > >> kfree(ap); > >> + > >> + return ret; > >> } > >> #else > >> -static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv) > >> +static int i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv) > >> { > >> + return 0; > >> } > >> #endif > >> > >> @@ -1664,7 +1668,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > >> goto out_gtt; > >> } > >> > >> - i915_kick_out_firmware_fb(dev_priv); > >> + ret = i915_kick_out_firmware_fb(dev_priv); > >> + if (ret) { > >> + DRM_ERROR("failed to remove conflicting framebuffer drivers\n"); > >> + goto out_gtt; > >> + } > >> } > >> > >> pci_set_master(dev->pdev); > > > > -- > > Ben Hutchings > > One of the nice things about standards is that there are so many of them. > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch