* [PATCH 1/2] drm/i915/fbdev: ORIGIN_GTT is only suitable for tracked writes
@ 2016-08-24 7:39 Chris Wilson
2016-08-24 7:39 ` [PATCH 2/2] drm/i915/fbdev: Setup for using FBC tracking Chris Wilson
2016-08-24 10:20 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/fbdev: ORIGIN_GTT is only suitable for tracked writes Patchwork
0 siblings, 2 replies; 6+ messages in thread
From: Chris Wilson @ 2016-08-24 7:39 UTC (permalink / raw)
To: intel-gfx
We can only use ORIGIN_GTT if the writes through the GTT can be tracked
by HW, i.e. if the region is fenced. If we do not fence the object,
declare the writes as originating from the CPU and do a full
invalidation.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_fbdev.c | 47 +++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index b7098f98bb67..8d6bfa565b50 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -45,6 +45,19 @@
#include <drm/i915_drm.h>
#include "i915_drv.h"
+static void intel_fbdev_invalidate(struct intel_fbdev *ifbdev)
+{
+ struct drm_device *dev = ifbdev->helper.dev;
+ struct drm_i915_gem_object *obj = ifbdev->fb->obj;
+ unsigned int origin;
+
+ origin = i915_gem_object_is_tiled(obj) ? ORIGIN_GTT : ORIGIN_CPU;
+
+ mutex_lock(&dev->struct_mutex);
+ intel_fb_obj_invalidate(obj, origin);
+ mutex_unlock(&dev->struct_mutex);
+}
+
static int intel_fbdev_set_par(struct fb_info *info)
{
struct drm_fb_helper *fb_helper = info->par;
@@ -53,12 +66,8 @@ static int intel_fbdev_set_par(struct fb_info *info)
int ret;
ret = drm_fb_helper_set_par(info);
-
- if (ret == 0) {
- mutex_lock(&fb_helper->dev->struct_mutex);
- intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT);
- mutex_unlock(&fb_helper->dev->struct_mutex);
- }
+ if (ret == 0)
+ intel_fbdev_invalidate(ifbdev);
return ret;
}
@@ -71,12 +80,8 @@ static int intel_fbdev_blank(int blank, struct fb_info *info)
int ret;
ret = drm_fb_helper_blank(blank, info);
-
- if (ret == 0) {
- mutex_lock(&fb_helper->dev->struct_mutex);
- intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT);
- mutex_unlock(&fb_helper->dev->struct_mutex);
- }
+ if (ret == 0)
+ intel_fbdev_invalidate(ifbdev);
return ret;
}
@@ -89,13 +94,10 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
container_of(fb_helper, struct intel_fbdev, helper);
int ret;
- ret = drm_fb_helper_pan_display(var, info);
- if (ret == 0) {
- mutex_lock(&fb_helper->dev->struct_mutex);
- intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT);
- mutex_unlock(&fb_helper->dev->struct_mutex);
- }
+ ret = drm_fb_helper_pan_display(var, info);
+ if (ret == 0)
+ intel_fbdev_invalidate(ifbdev);
return ret;
}
@@ -842,11 +844,8 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
if (!ifbdev->fb)
return;
- if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper)) {
+ if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper))
DRM_DEBUG("failed to restore crtc mode\n");
- } else {
- mutex_lock(&dev->struct_mutex);
- intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT);
- mutex_unlock(&dev->struct_mutex);
- }
+ else
+ intel_fbdev_invalidate(ifbdev);
}
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] drm/i915/fbdev: Setup for using FBC tracking
2016-08-24 7:39 [PATCH 1/2] drm/i915/fbdev: ORIGIN_GTT is only suitable for tracked writes Chris Wilson
@ 2016-08-24 7:39 ` Chris Wilson
2016-08-24 11:05 ` Daniel Vetter
2016-08-24 10:20 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/fbdev: ORIGIN_GTT is only suitable for tracked writes Patchwork
1 sibling, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2016-08-24 7:39 UTC (permalink / raw)
To: intel-gfx
When FBC is enabled, access through the fbdev is tracked using
ORIGIN_GTT, i.e. native hw tracking by FBC. This requires the
framebuffer to be fenced, which requires us to allocate the object
as X-tiled.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_fbdev.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 8d6bfa565b50..ad3995f87485 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -138,14 +138,17 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
mode_cmd.pitches[0] = ALIGN(mode_cmd.width *
DIV_ROUND_UP(sizes->surface_bpp, 8), 64);
+ if (i915.enable_fbc)
+ mode_cmd.pitches[0] = ALIGN(mode_cmd.pitches[0], 512);
+
mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
sizes->surface_depth);
- mutex_lock(&dev->struct_mutex);
-
size = mode_cmd.pitches[0] * mode_cmd.height;
size = PAGE_ALIGN(size);
+ mutex_lock(&dev->struct_mutex);
+
/* If the FB is too big, just don't use it since fbdev is not very
* important and we should probably use that space with FBC or other
* features. */
@@ -159,6 +162,12 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
goto out;
}
+ if (i915.enable_fbc) {
+ obj->tiling_and_stride = mode_cmd.pitches[0] | I915_TILING_X;
+ mode_cmd.modifier[0] = I915_FORMAT_MOD_X_TILED;
+ mode_cmd.flags |= DRM_MODE_FB_MODIFIERS;
+ }
+
fb = __intel_framebuffer_create(dev, &mode_cmd, obj);
if (IS_ERR(fb)) {
i915_gem_object_put(obj);
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/fbdev: ORIGIN_GTT is only suitable for tracked writes
2016-08-24 7:39 [PATCH 1/2] drm/i915/fbdev: ORIGIN_GTT is only suitable for tracked writes Chris Wilson
2016-08-24 7:39 ` [PATCH 2/2] drm/i915/fbdev: Setup for using FBC tracking Chris Wilson
@ 2016-08-24 10:20 ` Patchwork
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-08-24 10:20 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915/fbdev: ORIGIN_GTT is only suitable for tracked writes
URL : https://patchwork.freedesktop.org/series/11509/
State : failure
== Summary ==
Series 11509v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/11509/revisions/1/mbox/
Test gem_exec_gttfill:
Subgroup basic:
skip -> PASS (fi-snb-2600)
Test kms_cursor_legacy:
Subgroup basic-cursor-vs-flip-legacy:
pass -> FAIL (fi-bsw-n3050)
Subgroup basic-cursor-vs-flip-varying-size:
pass -> FAIL (fi-bsw-n3050)
Subgroup basic-flip-vs-cursor-legacy:
pass -> FAIL (fi-skl-6700k)
fail -> PASS (fi-bsw-n3050)
Subgroup basic-flip-vs-cursor-varying-size:
fail -> PASS (fi-bsw-n3050)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
incomplete -> DMESG-WARN (fi-hsw-4770k)
Test prime_busy:
Subgroup basic-after-default:
pass -> FAIL (fi-bsw-n3050)
Subgroup basic-before-default:
pass -> FAIL (fi-bsw-n3050)
Subgroup basic-wait-after-default:
pass -> FAIL (fi-bsw-n3050)
Subgroup basic-wait-before-default:
pass -> FAIL (fi-bsw-n3050)
Test prime_vgem:
Subgroup basic-busy-default:
pass -> FAIL (fi-bsw-n3050)
Subgroup basic-gtt:
pass -> FAIL (fi-bsw-n3050)
Subgroup basic-sync-default:
pass -> FAIL (fi-bsw-n3050)
Subgroup basic-wait-default:
pass -> FAIL (fi-bsw-n3050)
fi-bsw-n3050 total:249 pass:194 dwarn:0 dfail:0 fail:11 skip:44
fi-byt-n2820 total:249 pass:203 dwarn:0 dfail:0 fail:5 skip:41
fi-hsw-4770k total:249 pass:219 dwarn:6 dfail:1 fail:1 skip:22
fi-ivb-3520m total:249 pass:217 dwarn:0 dfail:0 fail:1 skip:31
fi-skl-6260u total:249 pass:233 dwarn:0 dfail:0 fail:2 skip:14
fi-skl-6700k total:249 pass:213 dwarn:4 dfail:0 fail:4 skip:28
fi-snb-2520m total:249 pass:201 dwarn:4 dfail:0 fail:2 skip:42
fi-snb-2600 total:249 pass:201 dwarn:4 dfail:0 fail:2 skip:42
Results at /archive/results/CI_IGT_test/Patchwork_2419/
9eafbdb3b053e7cc5f425665f51adf6be1cc0aa3 drm-intel-nightly: 2016y-08m-24d-07h-49m-28s UTC integration manifest
a6cfe68 drm/i915/fbdev: Setup for using FBC tracking
24da7a3 drm/i915/fbdev: ORIGIN_GTT is only suitable for tracked writes
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/fbdev: Setup for using FBC tracking
2016-08-24 7:39 ` [PATCH 2/2] drm/i915/fbdev: Setup for using FBC tracking Chris Wilson
@ 2016-08-24 11:05 ` Daniel Vetter
2016-08-24 11:16 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2016-08-24 11:05 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, Aug 24, 2016 at 08:39:52AM +0100, Chris Wilson wrote:
> When FBC is enabled, access through the fbdev is tracked using
> ORIGIN_GTT, i.e. native hw tracking by FBC. This requires the
> framebuffer to be fenced, which requires us to allocate the object
> as X-tiled.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 8d6bfa565b50..ad3995f87485 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -138,14 +138,17 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
>
> mode_cmd.pitches[0] = ALIGN(mode_cmd.width *
> DIV_ROUND_UP(sizes->surface_bpp, 8), 64);
> + if (i915.enable_fbc)
> + mode_cmd.pitches[0] = ALIGN(mode_cmd.pitches[0], 512);
> +
> mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> sizes->surface_depth);
>
> - mutex_lock(&dev->struct_mutex);
> -
> size = mode_cmd.pitches[0] * mode_cmd.height;
> size = PAGE_ALIGN(size);
>
> + mutex_lock(&dev->struct_mutex);
> +
> /* If the FB is too big, just don't use it since fbdev is not very
> * important and we should probably use that space with FBC or other
> * features. */
> @@ -159,6 +162,12 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> goto out;
> }
>
> + if (i915.enable_fbc) {
> + obj->tiling_and_stride = mode_cmd.pitches[0] | I915_TILING_X;
> + mode_cmd.modifier[0] = I915_FORMAT_MOD_X_TILED;
> + mode_cmd.flags |= DRM_MODE_FB_MODIFIERS;
> + }
Even more magic set-tiling calls, now also depending upon module options.
This sounds like a rather bad idea to me.
-Daniel
> +
> fb = __intel_framebuffer_create(dev, &mode_cmd, obj);
> if (IS_ERR(fb)) {
> i915_gem_object_put(obj);
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/fbdev: Setup for using FBC tracking
2016-08-24 11:05 ` Daniel Vetter
@ 2016-08-24 11:16 ` Chris Wilson
2016-08-24 11:19 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2016-08-24 11:16 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Wed, Aug 24, 2016 at 01:05:18PM +0200, Daniel Vetter wrote:
> On Wed, Aug 24, 2016 at 08:39:52AM +0100, Chris Wilson wrote:
> > When FBC is enabled, access through the fbdev is tracked using
> > ORIGIN_GTT, i.e. native hw tracking by FBC. This requires the
> > framebuffer to be fenced, which requires us to allocate the object
> > as X-tiled.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/intel_fbdev.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > index 8d6bfa565b50..ad3995f87485 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -138,14 +138,17 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> >
> > mode_cmd.pitches[0] = ALIGN(mode_cmd.width *
> > DIV_ROUND_UP(sizes->surface_bpp, 8), 64);
> > + if (i915.enable_fbc)
> > + mode_cmd.pitches[0] = ALIGN(mode_cmd.pitches[0], 512);
> > +
> > mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> > sizes->surface_depth);
> >
> > - mutex_lock(&dev->struct_mutex);
> > -
> > size = mode_cmd.pitches[0] * mode_cmd.height;
> > size = PAGE_ALIGN(size);
> >
> > + mutex_lock(&dev->struct_mutex);
> > +
> > /* If the FB is too big, just don't use it since fbdev is not very
> > * important and we should probably use that space with FBC or other
> > * features. */
> > @@ -159,6 +162,12 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> > goto out;
> > }
> >
> > + if (i915.enable_fbc) {
> > + obj->tiling_and_stride = mode_cmd.pitches[0] | I915_TILING_X;
> > + mode_cmd.modifier[0] = I915_FORMAT_MOD_X_TILED;
> > + mode_cmd.flags |= DRM_MODE_FB_MODIFIERS;
> > + }
>
> Even more magic set-tiling calls, now also depending upon module options.
> This sounds like a rather bad idea to me.
FB_MODIFIERS are pretty magic, I know!
This was more of if you want to use fbdev + fbc, this is what we should
do. Note that this is not so much a module option, but fbc state. At
this point in fbdev's lifecycle, we should be able to use HAS_FBC()
which is equivalent to i915.enable_fbc once we've sanitized the options.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/fbdev: Setup for using FBC tracking
2016-08-24 11:16 ` Chris Wilson
@ 2016-08-24 11:19 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2016-08-24 11:19 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, intel-gfx
On Wed, Aug 24, 2016 at 1:16 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > + if (i915.enable_fbc) {
>> > + obj->tiling_and_stride = mode_cmd.pitches[0] | I915_TILING_X;
>> > + mode_cmd.modifier[0] = I915_FORMAT_MOD_X_TILED;
>> > + mode_cmd.flags |= DRM_MODE_FB_MODIFIERS;
>> > + }
>>
>> Even more magic set-tiling calls, now also depending upon module options.
>> This sounds like a rather bad idea to me.
>
> FB_MODIFIERS are pretty magic, I know!
>
> This was more of if you want to use fbdev + fbc, this is what we should
> do. Note that this is not so much a module option, but fbc state. At
> this point in fbdev's lifecycle, we should be able to use HAS_FBC()
> which is equivalent to i915.enable_fbc once we've sanitized the options.
tbh I think fbdev is not something we should care about at all, beyond
making sure that it doesn't fail. For anyone who really hates X and
wayland enough to not even run them as terminal emulators there's
kmscon. Doing something "because fbdev" is imo negative justification
;-)
-Daniel
--
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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-24 11:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-24 7:39 [PATCH 1/2] drm/i915/fbdev: ORIGIN_GTT is only suitable for tracked writes Chris Wilson
2016-08-24 7:39 ` [PATCH 2/2] drm/i915/fbdev: Setup for using FBC tracking Chris Wilson
2016-08-24 11:05 ` Daniel Vetter
2016-08-24 11:16 ` Chris Wilson
2016-08-24 11:19 ` Daniel Vetter
2016-08-24 10:20 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/fbdev: ORIGIN_GTT is only suitable for tracked writes Patchwork
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.