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

[Ander Conselvan de Oliveira <conselvan2@gmail.com>]

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?

Thanks,
Ander