* [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
@ 2014-12-10 8:17 Chris Wilson
2014-12-10 10:23 ` Daniel Vetter
2014-12-10 15:04 ` shuang.he
0 siblings, 2 replies; 12+ messages in thread
From: Chris Wilson @ 2014-12-10 8:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson, stable
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) {
+ DRM_DEBUG_KMS("request for unaligned stolen object, denied\n");
+ return NULL;
+ }
+
stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
if (!stolen)
return NULL;
--
2.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2014-12-10 8:17 [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects Chris Wilson
@ 2014-12-10 10:23 ` Daniel Vetter
2014-12-10 11:13 ` [Intel-gfx] " Chris Wilson
2015-01-21 17:45 ` Jani Nikula
2014-12-10 15:04 ` shuang.he
1 sibling, 2 replies; 12+ messages in thread
From: Daniel Vetter @ 2014-12-10 10:23 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, stable
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 ...
-Daniel
> + DRM_DEBUG_KMS("request for unaligned stolen object, denied\n");
> + return NULL;
> + }
> +
> stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
> if (!stolen)
> return NULL;
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2014-12-10 10:23 ` Daniel Vetter
@ 2014-12-10 11:13 ` Chris Wilson
2014-12-10 13:53 ` Daniel Vetter
2015-01-21 17:45 ` Jani Nikula
1 sibling, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2014-12-10 11:13 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, stable
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.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2014-12-10 11:13 ` [Intel-gfx] " Chris Wilson
@ 2014-12-10 13:53 ` Daniel Vetter
2014-12-10 14:53 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2014-12-10 13:53 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, intel-gfx, stable
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.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2014-12-10 13:53 ` Daniel Vetter
@ 2014-12-10 14:53 ` Ville Syrjälä
2014-12-19 12:09 ` [Intel-gfx] " Ander Conselvan de Oliveira
0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2014-12-10 14:53 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, stable
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).
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2014-12-10 8:17 [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects Chris Wilson
2014-12-10 10:23 ` Daniel Vetter
@ 2014-12-10 15:04 ` shuang.he
1 sibling, 0 replies; 12+ messages in thread
From: shuang.he @ 2014-12-10 15:04 UTC (permalink / raw)
To: shuang.he, intel-gfx, chris
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 364/364 364/364
ILK +1 364/366 365/366
SNB 448/450 448/450
IVB 497/498 497/498
BYT 289/289 289/289
HSW 563/564 563/564
BDW 417/417 417/417
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
ILK igt_kms_flip_wf_vblank-ts-check DMESG_WARN(2, M26)PASS(15, M26M37) PASS(1, M37)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2014-12-10 14:53 ` Ville Syrjälä
@ 2014-12-19 12:09 ` Ander Conselvan de Oliveira
2014-12-19 14:18 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Ander Conselvan de Oliveira @ 2014-12-19 12:09 UTC (permalink / raw)
To: Ville Syrjälä, Daniel Vetter; +Cc: intel-gfx, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2014-12-19 12:09 ` [Intel-gfx] " Ander Conselvan de Oliveira
@ 2014-12-19 14:18 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2014-12-19 14:18 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: Daniel Vetter, intel-gfx, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2014-12-10 10:23 ` Daniel Vetter
2014-12-10 11:13 ` [Intel-gfx] " Chris Wilson
@ 2015-01-21 17:45 ` Jani Nikula
2015-01-21 18:13 ` Daniel Vetter
1 sibling, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2015-01-21 17:45 UTC (permalink / raw)
To: Daniel Vetter, Chris Wilson; +Cc: intel-gfx, stable
On Wed, 10 Dec 2014, Daniel Vetter <daniel@ffwll.ch> 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 ...
Can we fix the regression with Chris' patch and do proper stuff when we
figure out what we want to do?
BR,
Jani.
> -daniel
>
>> + DRM_DEBUG_KMS("request for unaligned stolen object, denied\n");
>> + return NULL;
>> + }
>> +
>> stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
>> if (!stolen)
>> return NULL;
>> --
>> 2.1.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2015-01-21 17:45 ` Jani Nikula
@ 2015-01-21 18:13 ` Daniel Vetter
2015-01-21 22:22 ` [Intel-gfx] " Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2015-01-21 18:13 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, stable
On Wed, Jan 21, 2015 at 07:45:01PM +0200, Jani Nikula wrote:
> On Wed, 10 Dec 2014, Daniel Vetter <daniel@ffwll.ch> 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 ...
>
> Can we fix the regression with Chris' patch and do proper stuff when we
> figure out what we want to do?
Something like the below would imo be the proper bandaid. Untested since I
don't have an affected system. Then we can still (later on, if bored)
recover the offsets properly if ever needed.
-Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index a2045848bd1a..f75bf292285d 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -485,10 +485,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
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))
+ if (WARN_ON(size == 0 || stolen_offset & 4095 || size & 4095))
return NULL;
stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 86831aec5c0d..733c99d5b671 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2370,13 +2370,17 @@ intel_alloc_plane_obj(struct intel_crtc *crtc,
struct drm_device *dev = crtc->base.dev;
struct drm_i915_gem_object *obj = NULL;
struct drm_mode_fb_cmd2 mode_cmd = { 0 };
- u32 base = plane_config->base;
+ u32 base_aligned = round_down(plane_config->base, PAGE_SIZE);
+ u32 size_aligned = round_up(plane_config->base + plane_config->size,
+ PAGE_SIZE);
if (plane_config->size == 0)
return false;
- obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base,
- plane_config->size);
+ obj = i915_gem_object_create_stolen_for_preallocated(dev,
+ base_aligned,
+ base_aligned,
+ size_aligned);
if (!obj)
return false;
@@ -6626,7 +6630,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
aligned_height = intel_fb_align_height(dev, fb->height,
plane_config->tiling);
- plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height);
+ plane_config->size = fb->pitches[0] * aligned_height;
DRM_DEBUG_KMS("pipe/plane %c/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
pipe_name(pipe), plane, fb->width, fb->height,
@@ -7663,7 +7667,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
aligned_height = intel_fb_align_height(dev, fb->height,
plane_config->tiling);
- plane_config->size = ALIGN(fb->pitches[0] * aligned_height, PAGE_SIZE);
+ plane_config->size = fb->pitches[0] * aligned_height, PAGE_SIZE;
DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
pipe_name(pipe), fb->width, fb->height,
@@ -7754,7 +7758,7 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
aligned_height = intel_fb_align_height(dev, fb->height,
plane_config->tiling);
- plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height);
+ plane_config->size = fb->pitches[0] * aligned_height;
DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
pipe_name(pipe), fb->width, fb->height,
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2015-01-21 18:13 ` Daniel Vetter
@ 2015-01-21 22:22 ` Chris Wilson
2015-02-10 12:34 ` Jani Nikula
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2015-01-21 22:22 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Jani Nikula, intel-gfx, stable
On Wed, Jan 21, 2015 at 07:13:04PM +0100, Daniel Vetter wrote:
> Something like the below would imo be the proper bandaid. Untested since I
> don't have an affected system. Then we can still (later on, if bored)
> recover the offsets properly if ever needed.
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 86831aec5c0d..733c99d5b671 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2370,13 +2370,17 @@ intel_alloc_plane_obj(struct intel_crtc *crtc,
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_gem_object *obj = NULL;
> struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> - u32 base = plane_config->base;
> + u32 base_aligned = round_down(plane_config->base, PAGE_SIZE);
> + u32 size_aligned = round_up(plane_config->base + plane_config->size,
> + PAGE_SIZE);
You forgot size_aligned -= base_aligned;
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects
2015-01-21 22:22 ` [Intel-gfx] " Chris Wilson
@ 2015-02-10 12:34 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2015-02-10 12:34 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter; +Cc: intel-gfx, stable
On Thu, 22 Jan 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Wed, Jan 21, 2015 at 07:13:04PM +0100, Daniel Vetter wrote:
>> Something like the below would imo be the proper bandaid. Untested since I
>> don't have an affected system. Then we can still (later on, if bored)
>> recover the offsets properly if ever needed.
>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 86831aec5c0d..733c99d5b671 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2370,13 +2370,17 @@ intel_alloc_plane_obj(struct intel_crtc *crtc,
>> struct drm_device *dev = crtc->base.dev;
>> struct drm_i915_gem_object *obj = NULL;
>> struct drm_mode_fb_cmd2 mode_cmd = { 0 };
>> - u32 base = plane_config->base;
>> + u32 base_aligned = round_down(plane_config->base, PAGE_SIZE);
>> + u32 size_aligned = round_up(plane_config->base + plane_config->size,
>> + PAGE_SIZE);
>
> You forgot size_aligned -= base_aligned;
Stalled again it seems.
>From the bug, the original patch gets
Tested-by: Johannes W <jargon@molb.org>
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-02-10 12:34 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-10 8:17 [PATCH] drm/i915: Quietly reject attempts to create non-pagealigned stolen objects Chris Wilson
2014-12-10 10:23 ` Daniel Vetter
2014-12-10 11:13 ` [Intel-gfx] " Chris Wilson
2014-12-10 13:53 ` Daniel Vetter
2014-12-10 14:53 ` Ville Syrjälä
2014-12-19 12:09 ` [Intel-gfx] " Ander Conselvan de Oliveira
2014-12-19 14:18 ` Ville Syrjälä
2015-01-21 17:45 ` Jani Nikula
2015-01-21 18:13 ` Daniel Vetter
2015-01-21 22:22 ` [Intel-gfx] " Chris Wilson
2015-02-10 12:34 ` Jani Nikula
2014-12-10 15:04 ` shuang.he
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.