Re: [Intel-gfx] [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Dec 19, 2014 at 02:09:32PM +0200, Ander Conselvan de Oliveira wrote:
> On 12/10/2014 04:53 PM, Ville Syrjälä 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 request
> >>>>> an unaligned object. However, it turns out that some BIOSes will
> >>>>> allocate a scanout that is offset from 0 and not aligned to a page
> >>>>> boundary, and we were passing this through and hitting the BUG_ON during
> >>>>> boot.
> >>>>>
> >>>>> Quietly reject such a request to reserve the unaligned stolen object and
> >>>>> let the boot continue, restoring previous behaviour (i.e. no BIOS
> >>>>> framebuffer preservation).
> >>>>>
> >>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86883
> >>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>>> 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_preallocated(struct drm_device *dev,
> >>>>>   	DRM_DEBUG_KMS("creating preallocated stolen object: stolen_offset=%x, gtt_offset=%x, size=%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 == 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...
> >>>
> >>> The BIOS scanout is properly aligned to the rules of the display engine,
> >>> 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 GTT
> >>> 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 was
> >>> 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 double check
> > that it's sane, fixing if not).
> 
> http://lists.freedesktop.org/archives/intel-gfx/2012-May/017584.html
> 
> Is it that one?

Looks like it.

-- 
Ville Syrjälä
Intel OTC