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

2015-02-10 Thread Jani Nikula
On Thu, 22 Jan 2015, Chris Wilson ch...@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 jar...@molb.org


 -Chris

 -- 
 Chris Wilson, Intel Open Source Technology Centre

-- 
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-01-21 Thread Daniel Vetter
On Wed, Jan 21, 2015 at 07:45:01PM +0200, Jani Nikula wrote:
 On Wed, 10 Dec 2014, Daniel Vetter dan...@ffwll.ch wrote:
  On Wed, Dec 10, 2014 at 08:17:11AM +, 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 ch...@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,
   

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

2015-01-21 Thread Jani Nikula
On Wed, 10 Dec 2014, Daniel Vetter dan...@ffwll.ch wrote:
 On Wed, Dec 10, 2014 at 08:17:11AM +, 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 ch...@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-...@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-...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-01-21 Thread Chris Wilson
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
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-12-19 Thread Ander Conselvan de Oliveira

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 +, 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 +, 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 ch...@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

--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-12-19 Thread Ville Syrjälä
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 +, 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 +, 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 ch...@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
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-12-10 Thread Daniel Vetter
On Wed, Dec 10, 2014 at 08:17:11AM +, 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 ch...@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-...@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
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-12-10 Thread Chris Wilson
On Wed, Dec 10, 2014 at 11:23:44AM +0100, Daniel Vetter wrote:
 On Wed, Dec 10, 2014 at 08:17:11AM +, 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 ch...@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
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-12-10 Thread Daniel Vetter
On Wed, Dec 10, 2014 at 11:13:28AM +, 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 +, 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 ch...@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
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-12-10 Thread Ville Syrjälä
On Wed, Dec 10, 2014 at 02:53:01PM +0100, Daniel Vetter wrote:
 On Wed, Dec 10, 2014 at 11:13:28AM +, 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 +, 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 ch...@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
--
To unsubscribe from this list: send the line unsubscribe stable in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html