From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ander Conselvan de Oliveira Subject: Re: [Intel-gfx] [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects Date: Fri, 19 Dec 2014 14:09:32 +0200 Message-ID: <5494157C.1080005@gmail.com> References: <1418199431-5293-1-git-send-email-chris@chris-wilson.co.uk> <20141210102344.GM27182@phenom.ffwll.local> <20141210111328.GG14956@nuc-i3427.alporthouse.com> <20141210135301.GY27182@phenom.ffwll.local> <20141210145356.GC10649@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20141210145356.GC10649@intel.com> Sender: stable-owner@vger.kernel.org To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On 12/10/2014 04:53 PM, Ville Syrj=C3=A4l=C3=A4 wrote: > On Wed, Dec 10, 2014 at 02:53:01PM +0100, Daniel Vetter wrote: >> On Wed, Dec 10, 2014 at 11:13:28AM +0000, Chris Wilson wrote: >>> On Wed, Dec 10, 2014 at 11:23:44AM +0100, Daniel Vetter wrote: >>>> On Wed, Dec 10, 2014 at 08:17:11AM +0000, Chris Wilson wrote: >>>>> This added as a BUG_ON as it considered that no one would ever re= quest >>>>> an unaligned object. However, it turns out that some BIOSes will >>>>> allocate a scanout that is offset from 0 and not aligned to a pag= e >>>>> boundary, and we were passing this through and hitting the BUG_ON= during >>>>> boot. >>>>> >>>>> Quietly reject such a request to reserve the unaligned stolen obj= ect and >>>>> let the boot continue, restoring previous behaviour (i.e. no BIOS >>>>> framebuffer preservation). >>>>> >>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3D86883 >>>>> Signed-off-by: Chris Wilson >>>>> Cc: stable@vger.kernel.org >>>>> --- >>>>> drivers/gpu/drm/i915/i915_gem_stolen.c | 10 ++++++---- >>>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu= /drm/i915/i915_gem_stolen.c >>>>> index 5c616ec2c5c8..a3bc0fa07c6c 100644 >>>>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c >>>>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c >>>>> @@ -646,13 +646,15 @@ i915_gem_object_create_stolen_for_prealloca= ted(struct drm_device *dev, >>>>> DRM_DEBUG_KMS("creating preallocated stolen object: stolen_off= set=3D%x, gtt_offset=3D%x, size=3D%x\n", >>>>> stolen_offset, gtt_offset, size); >>>>> >>>>> - /* KISS and expect everything to be page-aligned */ >>>>> - BUG_ON(stolen_offset & 4095); >>>>> - BUG_ON(size & 4095); >>>>> - >>>>> if (WARN_ON(size =3D=3D 0)) >>>>> return NULL; >>>>> >>>>> + /* KISS and expect everything to be GTT page-aligned */ >>>>> + if ((stolen_offset | size) & 4095) { >>>> >>>> Imo we should stil WARN_ON and fixup up the takeover code to align= things >>>> properly ... >>> >>> You shot down my idea for storing deltas into objects in the past..= =2E >>> >>> The BIOS scanout is properly aligned to the rules of the display en= gine, >>> just not according to our mm restrictions. The bigger question is >>> whether our 1:1 offset-to-stolen mapping is correct. It could well = be >>> that that the framebuffer is at stolen address 0, but just has a GT= T >>> offset. >>> >>> So the only question is whether we reject the object reservation at= the >>> stolen layer or at the plane config layer. I decided that stolen wa= s >>> better, because it is failing to meet our mm restrictions not >>> hardware restrictions. >> >> The framebuffer layer can very much cope with offsets, so no need to >> reject it. We just need to patch up the framebuffer we create a bit. >> Offsets are in pixels but that should align well. > > Or someone can dig out my old fb->offsets[] handling patch (and doubl= e check > that it's sane, fixing if not). http://lists.freedesktop.org/archives/intel-gfx/2012-May/017584.html Is it that one? Thanks, Ander