diff mbox

drm/i915: Fallback to using unmappable memory for scanout

Message ID 1426716481-19869-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson March 18, 2015, 10:08 p.m. UTC
The existing ABI says that scanouts are pinned into the mappable region
so that legacy clients (e.g. old Xorg or plymouthd) can write directly
into the scanout through a GTT mapping. However if the surface does not
fit into the mappable region, we are better off just trying to fit it
anywhere and hoping for the best. (Any userspace that is cappable of
using ginormous scanouts is also likely not to rely on pure GTT
updates.) In the future, there may even be a kernel mediated method for
the legacy clients.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
Cc: Deepak S <deepak.s@linux.intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

deepak.s@linux.intel.com March 19, 2015, 10:53 a.m. UTC | #1
On Thursday 19 March 2015 03:38 AM, Chris Wilson wrote:
> The existing ABI says that scanouts are pinned into the mappable region
> so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> into the scanout through a GTT mapping. However if the surface does not
> fit into the mappable region, we are better off just trying to fit it
> anywhere and hoping for the best. (Any userspace that is cappable of
> using ginormous scanouts is also likely not to rely on pure GTT
> updates.) In the future, there may even be a kernel mediated method for
> the legacy clients.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> Cc: Deepak S <deepak.s@linux.intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9e498e0bbf22..9a1de848e450 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>   
>   	/* As the user may map the buffer once pinned in the display plane
>   	 * (e.g. libkms for the bootup splash), we have to ensure that we
> -	 * always use map_and_fenceable for all scanout buffers.
> +	 * always use map_and_fenceable for all scanout buffers. However,
> +	 * it may simply be too big to fit into mappable, in which case
> +	 * put it anyway and hope that userspace can cope (but always first
> +	 * try to preserve the existing ABI).
>   	 */
>   	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
>   	if (ret)
> +		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> +	if (ret)
>   		goto err_unpin_display;
>   

Hi Chris,

if we map the object into unmappable region. I think we should skip fence create ?

Thanks
Deepak

>   	i915_gem_object_flush_cpu_write_domain(obj);
Chris Wilson March 19, 2015, 11:27 a.m. UTC | #2
On Thu, Mar 19, 2015 at 04:23:32PM +0530, Deepak S wrote:
> Hi Chris,
> 
> if we map the object into unmappable region. I think we should skip fence create ?

We should just ignore the failure. Whether or not we want the fence
pinned is a matter for the FBC code, which is a different level.

But true, it should be fixed in this patch as well.
-Chris
Shuang He March 19, 2015, 3:34 p.m. UTC | #3
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6000
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              272/272              271/272
ILK                                  301/301              301/301
SNB                                  303/303              303/303
IVB                 -1              342/342              341/342
BYT                                  287/287              287/287
HSW                                  362/362              362/362
BDW                                  308/308              308/308
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gem_userptr_blits_minor-sync-interruptible      PASS(2)      DMESG_WARN(1)PASS(1)
*IVB  igt_gem_storedw_batches_loop_secure-dispatch      PASS(2)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9e498e0bbf22..9a1de848e450 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4034,10 +4034,15 @@  i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 
 	/* As the user may map the buffer once pinned in the display plane
 	 * (e.g. libkms for the bootup splash), we have to ensure that we
-	 * always use map_and_fenceable for all scanout buffers.
+	 * always use map_and_fenceable for all scanout buffers. However,
+	 * it may simply be too big to fit into mappable, in which case
+	 * put it anyway and hope that userspace can cope (but always first
+	 * try to preserve the existing ABI).
 	 */
 	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
 	if (ret)
+		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
+	if (ret)
 		goto err_unpin_display;
 
 	i915_gem_object_flush_cpu_write_domain(obj);