Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Tue, Jul 07, 2015 at 08:23:46PM +, Vivi, Rodrigo wrote:
On Tue, 2015-07-07 at 13:24 +0200, Daniel Vetter wrote:
On Mon, Jul 06, 2015 at 11:19:03PM +, Vivi, Rodrigo wrote:
On Mon, 2015-07-06 at 19:56 +0200, Daniel Vetter wrote:
On Mon, Jul 06, 2015 at 02:11:55PM -0300, Paulo Zanoni wrote:
2015-07-06 13:43 GMT-03:00 Vivi, Rodrigo rodrigo.v...@intel.com:
On Fri, 2015-07-03 at 09:10 +0200, Daniel Vetter wrote:
On Thu, Jul 02, 2015 at 04:41:32PM +, Vivi, Rodrigo wrote:
On Thu, 2015-07-02 at 13:03 -0300, Paulo Zanoni wrote:
2015-06-30 20:42 GMT-03:00 Rodrigo Vivi
rodrigo.v...@intel.com:
Let's do a frontbuffer invalidation on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a
splash
screen involved like plymouth.
Plymoth will do a modeset over ioctl that flushes frontbuffer
tracking and PSR gets back to work while it cannot track the
screen updates and exit properly. However plymouth also uses
a dirtyfb ioctl whenever updating the screen. So let's use it
to invalidate PSR back again.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since
dirty
callback is just called after few screen updates and not
on
everyone as pointed by Daniel.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..b55b1b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14393,9 +14393,27 @@ static int
intel_user_framebuffer_create_handle(struct drm_framebuffer
*fb,
return drm_gem_handle_create(file, obj-base,
handle);
}
+static int intel_user_framebuffer_dirty(struct
drm_framebuffer *fb,
+ struct
drm_file *file,
+ unsigned
flags, unsigned color,
+ struct
drm_clip_rect *clips,
+ unsigned
num_clips)
You don't need the white space on the lines above, just the
tabs.
+{
+ struct drm_device *dev = fb-dev;
+ struct intel_framebuffer *intel_fb =
to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+
+ mutex_lock(dev-struct_mutex);
+ intel_fb_obj_invalidate(obj, ORIGIN_GTT);
As far as I understood from my brief investigation, the dirty
IOCTL
just says hey, I'm done writing on the memory, you can flush
things
now, and if the user writes on the buffer again later, it
will need
to call the dirty IOCTL again. So shouldn't we call
intel_fb_obj_flush(obj, false) here? It seems this was also
pointed by
Daniel on the first review. It would be better because it
would allow
us to actually keep PSR/FBC enabled.
The flush caused by the dumb modeset ioctl is exactly what I
want to
revert here.
Well, I just tested to double check here and flush makes me to
miss
screen updates. (triple checked with retired = true as well just
in
case)
fbdev comes to place and invalidated frontbuffer, then plymouth
does a
flush that makes psr start working when it should continue
disabled.
Continue flushing it doesn't solve the problem, just ratify it.
But beside the issue that it is solving I don't believe we want
is a
flush anyway. There is something writing directly to frontbuffer
with no
invalidation. The dirty call is supposed to be a damage call that
actually tells something on the screen got written and needs to
be
updated if it hasn't still. In our cause this is exactly the
frontbuffer
invalidate, not the flush.
The flush place would be after it really stopped doing
something, but
since I don't trust it I prefer to let it invalidated until next
flip.
See my review on the first round of this patch: dirtyfb _is_ a
flush. If
it's not enough then there's some other problems
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Tue, 2015-07-07 at 13:24 +0200, Daniel Vetter wrote:
On Mon, Jul 06, 2015 at 11:19:03PM +, Vivi, Rodrigo wrote:
On Mon, 2015-07-06 at 19:56 +0200, Daniel Vetter wrote:
On Mon, Jul 06, 2015 at 02:11:55PM -0300, Paulo Zanoni wrote:
2015-07-06 13:43 GMT-03:00 Vivi, Rodrigo rodrigo.v...@intel.com:
On Fri, 2015-07-03 at 09:10 +0200, Daniel Vetter wrote:
On Thu, Jul 02, 2015 at 04:41:32PM +, Vivi, Rodrigo wrote:
On Thu, 2015-07-02 at 13:03 -0300, Paulo Zanoni wrote:
2015-06-30 20:42 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
Let's do a frontbuffer invalidation on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a
splash
screen involved like plymouth.
Plymoth will do a modeset over ioctl that flushes frontbuffer
tracking and PSR gets back to work while it cannot track the
screen updates and exit properly. However plymouth also uses
a dirtyfb ioctl whenever updating the screen. So let's use it
to invalidate PSR back again.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since
dirty
callback is just called after few screen updates and not on
everyone as pointed by Daniel.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..b55b1b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14393,9 +14393,27 @@ static int
intel_user_framebuffer_create_handle(struct drm_framebuffer
*fb,
return drm_gem_handle_create(file, obj-base, handle);
}
+static int intel_user_framebuffer_dirty(struct
drm_framebuffer *fb,
+ struct drm_file
*file,
+ unsigned flags,
unsigned color,
+ struct
drm_clip_rect *clips,
+ unsigned
num_clips)
You don't need the white space on the lines above, just the tabs.
+{
+ struct drm_device *dev = fb-dev;
+ struct intel_framebuffer *intel_fb =
to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+
+ mutex_lock(dev-struct_mutex);
+ intel_fb_obj_invalidate(obj, ORIGIN_GTT);
As far as I understood from my brief investigation, the dirty
IOCTL
just says hey, I'm done writing on the memory, you can flush
things
now, and if the user writes on the buffer again later, it will
need
to call the dirty IOCTL again. So shouldn't we call
intel_fb_obj_flush(obj, false) here? It seems this was also
pointed by
Daniel on the first review. It would be better because it would
allow
us to actually keep PSR/FBC enabled.
The flush caused by the dumb modeset ioctl is exactly what I want
to
revert here.
Well, I just tested to double check here and flush makes me to miss
screen updates. (triple checked with retired = true as well just in
case)
fbdev comes to place and invalidated frontbuffer, then plymouth
does a
flush that makes psr start working when it should continue
disabled.
Continue flushing it doesn't solve the problem, just ratify it.
But beside the issue that it is solving I don't believe we want is
a
flush anyway. There is something writing directly to frontbuffer
with no
invalidation. The dirty call is supposed to be a damage call that
actually tells something on the screen got written and needs to be
updated if it hasn't still. In our cause this is exactly the
frontbuffer
invalidate, not the flush.
The flush place would be after it really stopped doing something,
but
since I don't trust it I prefer to let it invalidated until next
flip.
See my review on the first round of this patch: dirtyfb _is_ a
flush. If
it's not enough then there's some other problems still around.
Well, the flush itself in the way it is defined is to flush
frontbuffer
bits and put power saving features back to work. PSR flush for
instance
doesn't exit on every flush. Only in the cases
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Mon, Jul 06, 2015 at 11:19:03PM +, Vivi, Rodrigo wrote:
On Mon, 2015-07-06 at 19:56 +0200, Daniel Vetter wrote:
On Mon, Jul 06, 2015 at 02:11:55PM -0300, Paulo Zanoni wrote:
2015-07-06 13:43 GMT-03:00 Vivi, Rodrigo rodrigo.v...@intel.com:
On Fri, 2015-07-03 at 09:10 +0200, Daniel Vetter wrote:
On Thu, Jul 02, 2015 at 04:41:32PM +, Vivi, Rodrigo wrote:
On Thu, 2015-07-02 at 13:03 -0300, Paulo Zanoni wrote:
2015-06-30 20:42 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
Let's do a frontbuffer invalidation on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a splash
screen involved like plymouth.
Plymoth will do a modeset over ioctl that flushes frontbuffer
tracking and PSR gets back to work while it cannot track the
screen updates and exit properly. However plymouth also uses
a dirtyfb ioctl whenever updating the screen. So let's use it
to invalidate PSR back again.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty
callback is just called after few screen updates and not on
everyone as pointed by Daniel.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..b55b1b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14393,9 +14393,27 @@ static int
intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
return drm_gem_handle_create(file, obj-base, handle);
}
+static int intel_user_framebuffer_dirty(struct drm_framebuffer
*fb,
+ struct drm_file
*file,
+ unsigned flags,
unsigned color,
+ struct
drm_clip_rect *clips,
+ unsigned
num_clips)
You don't need the white space on the lines above, just the tabs.
+{
+ struct drm_device *dev = fb-dev;
+ struct intel_framebuffer *intel_fb =
to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+
+ mutex_lock(dev-struct_mutex);
+ intel_fb_obj_invalidate(obj, ORIGIN_GTT);
As far as I understood from my brief investigation, the dirty IOCTL
just says hey, I'm done writing on the memory, you can flush
things
now, and if the user writes on the buffer again later, it will
need
to call the dirty IOCTL again. So shouldn't we call
intel_fb_obj_flush(obj, false) here? It seems this was also
pointed by
Daniel on the first review. It would be better because it would
allow
us to actually keep PSR/FBC enabled.
The flush caused by the dumb modeset ioctl is exactly what I want to
revert here.
Well, I just tested to double check here and flush makes me to miss
screen updates. (triple checked with retired = true as well just in
case)
fbdev comes to place and invalidated frontbuffer, then plymouth does
a
flush that makes psr start working when it should continue disabled.
Continue flushing it doesn't solve the problem, just ratify it.
But beside the issue that it is solving I don't believe we want is a
flush anyway. There is something writing directly to frontbuffer
with no
invalidation. The dirty call is supposed to be a damage call that
actually tells something on the screen got written and needs to be
updated if it hasn't still. In our cause this is exactly the
frontbuffer
invalidate, not the flush.
The flush place would be after it really stopped doing something, but
since I don't trust it I prefer to let it invalidated until next
flip.
See my review on the first round of this patch: dirtyfb _is_ a flush.
If
it's not enough then there's some other problems still around.
Well, the flush itself in the way it is defined is to flush frontbuffer
bits and put power saving features back to work. PSR flush for instance
doesn't exit on every flush. Only in the cases that we know HW tracking
doesn't work.
One possibility would be change PSR to respect that all flush always is
invalidate (exit) + flush fb_bits. But I'm against this since PSR on
core platforms doesn't have SW
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Mon, Jul 06, 2015 at 02:11:55PM -0300, Paulo Zanoni wrote:
2015-07-06 13:43 GMT-03:00 Vivi, Rodrigo rodrigo.v...@intel.com:
On Fri, 2015-07-03 at 09:10 +0200, Daniel Vetter wrote:
On Thu, Jul 02, 2015 at 04:41:32PM +, Vivi, Rodrigo wrote:
On Thu, 2015-07-02 at 13:03 -0300, Paulo Zanoni wrote:
2015-06-30 20:42 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
Let's do a frontbuffer invalidation on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a splash
screen involved like plymouth.
Plymoth will do a modeset over ioctl that flushes frontbuffer
tracking and PSR gets back to work while it cannot track the
screen updates and exit properly. However plymouth also uses
a dirtyfb ioctl whenever updating the screen. So let's use it
to invalidate PSR back again.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty
callback is just called after few screen updates and not on
everyone as pointed by Daniel.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..b55b1b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14393,9 +14393,27 @@ static int
intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
return drm_gem_handle_create(file, obj-base, handle);
}
+static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
+ struct drm_file *file,
+ unsigned flags,
unsigned color,
+ struct drm_clip_rect
*clips,
+ unsigned num_clips)
You don't need the white space on the lines above, just the tabs.
+{
+ struct drm_device *dev = fb-dev;
+ struct intel_framebuffer *intel_fb =
to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+
+ mutex_lock(dev-struct_mutex);
+ intel_fb_obj_invalidate(obj, ORIGIN_GTT);
As far as I understood from my brief investigation, the dirty IOCTL
just says hey, I'm done writing on the memory, you can flush things
now, and if the user writes on the buffer again later, it will need
to call the dirty IOCTL again. So shouldn't we call
intel_fb_obj_flush(obj, false) here? It seems this was also pointed by
Daniel on the first review. It would be better because it would allow
us to actually keep PSR/FBC enabled.
The flush caused by the dumb modeset ioctl is exactly what I want to
revert here.
Well, I just tested to double check here and flush makes me to miss
screen updates. (triple checked with retired = true as well just in
case)
fbdev comes to place and invalidated frontbuffer, then plymouth does a
flush that makes psr start working when it should continue disabled.
Continue flushing it doesn't solve the problem, just ratify it.
But beside the issue that it is solving I don't believe we want is a
flush anyway. There is something writing directly to frontbuffer with no
invalidation. The dirty call is supposed to be a damage call that
actually tells something on the screen got written and needs to be
updated if it hasn't still. In our cause this is exactly the frontbuffer
invalidate, not the flush.
The flush place would be after it really stopped doing something, but
since I don't trust it I prefer to let it invalidated until next flip.
See my review on the first round of this patch: dirtyfb _is_ a flush. If
it's not enough then there's some other problems still around.
Well, the flush itself in the way it is defined is to flush frontbuffer
bits and put power saving features back to work. PSR flush for instance
doesn't exit on every flush. Only in the cases that we know HW tracking
doesn't work.
One possibility would be change PSR to respect that all flush always is
invalidate (exit) + flush fb_bits. But I'm against this since PSR on
core platforms doesn't have SW trackking ext and it wasn't implemented
to be fully enabled/disabled every time.
Another idea on this line is to make flushs know origins or at least a
boolean to separate cases where flush must be handled as flush bits +
continue working or invalidate + flush bits + start working again
Please let me know what you think.
But putting flushs on this dirty callback is currently useless
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
2015-07-06 13:43 GMT-03:00 Vivi, Rodrigo rodrigo.v...@intel.com:
On Fri, 2015-07-03 at 09:10 +0200, Daniel Vetter wrote:
On Thu, Jul 02, 2015 at 04:41:32PM +, Vivi, Rodrigo wrote:
On Thu, 2015-07-02 at 13:03 -0300, Paulo Zanoni wrote:
2015-06-30 20:42 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
Let's do a frontbuffer invalidation on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a splash
screen involved like plymouth.
Plymoth will do a modeset over ioctl that flushes frontbuffer
tracking and PSR gets back to work while it cannot track the
screen updates and exit properly. However plymouth also uses
a dirtyfb ioctl whenever updating the screen. So let's use it
to invalidate PSR back again.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty
callback is just called after few screen updates and not on
everyone as pointed by Daniel.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..b55b1b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14393,9 +14393,27 @@ static int
intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
return drm_gem_handle_create(file, obj-base, handle);
}
+static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
+ struct drm_file *file,
+ unsigned flags,
unsigned color,
+ struct drm_clip_rect
*clips,
+ unsigned num_clips)
You don't need the white space on the lines above, just the tabs.
+{
+ struct drm_device *dev = fb-dev;
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+
+ mutex_lock(dev-struct_mutex);
+ intel_fb_obj_invalidate(obj, ORIGIN_GTT);
As far as I understood from my brief investigation, the dirty IOCTL
just says hey, I'm done writing on the memory, you can flush things
now, and if the user writes on the buffer again later, it will need
to call the dirty IOCTL again. So shouldn't we call
intel_fb_obj_flush(obj, false) here? It seems this was also pointed by
Daniel on the first review. It would be better because it would allow
us to actually keep PSR/FBC enabled.
The flush caused by the dumb modeset ioctl is exactly what I want to
revert here.
Well, I just tested to double check here and flush makes me to miss
screen updates. (triple checked with retired = true as well just in
case)
fbdev comes to place and invalidated frontbuffer, then plymouth does a
flush that makes psr start working when it should continue disabled.
Continue flushing it doesn't solve the problem, just ratify it.
But beside the issue that it is solving I don't believe we want is a
flush anyway. There is something writing directly to frontbuffer with no
invalidation. The dirty call is supposed to be a damage call that
actually tells something on the screen got written and needs to be
updated if it hasn't still. In our cause this is exactly the frontbuffer
invalidate, not the flush.
The flush place would be after it really stopped doing something, but
since I don't trust it I prefer to let it invalidated until next flip.
See my review on the first round of this patch: dirtyfb _is_ a flush. If
it's not enough then there's some other problems still around.
Well, the flush itself in the way it is defined is to flush frontbuffer
bits and put power saving features back to work. PSR flush for instance
doesn't exit on every flush. Only in the cases that we know HW tracking
doesn't work.
One possibility would be change PSR to respect that all flush always is
invalidate (exit) + flush fb_bits. But I'm against this since PSR on
core platforms doesn't have SW trackking ext and it wasn't implemented
to be fully enabled/disabled every time.
Another idea on this line is to make flushs know origins or at least a
boolean to separate cases where flush must be handled as flush bits +
continue working or invalidate + flush bits + start working again
Please let me know what you think.
But putting flushs on this dirty callback is currently useless because
flush just tells psr to continue working without getting screen updates.
Well, this is the most important discussion for now, but also even we
change flush
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Fri, 2015-07-03 at 09:10 +0200, Daniel Vetter wrote:
On Thu, Jul 02, 2015 at 04:41:32PM +, Vivi, Rodrigo wrote:
On Thu, 2015-07-02 at 13:03 -0300, Paulo Zanoni wrote:
2015-06-30 20:42 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
Let's do a frontbuffer invalidation on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a splash
screen involved like plymouth.
Plymoth will do a modeset over ioctl that flushes frontbuffer
tracking and PSR gets back to work while it cannot track the
screen updates and exit properly. However plymouth also uses
a dirtyfb ioctl whenever updating the screen. So let's use it
to invalidate PSR back again.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty
callback is just called after few screen updates and not on
everyone as pointed by Daniel.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..b55b1b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14393,9 +14393,27 @@ static int
intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
return drm_gem_handle_create(file, obj-base, handle);
}
+static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
+ struct drm_file *file,
+ unsigned flags, unsigned
color,
+ struct drm_clip_rect
*clips,
+ unsigned num_clips)
You don't need the white space on the lines above, just the tabs.
+{
+ struct drm_device *dev = fb-dev;
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+
+ mutex_lock(dev-struct_mutex);
+ intel_fb_obj_invalidate(obj, ORIGIN_GTT);
As far as I understood from my brief investigation, the dirty IOCTL
just says hey, I'm done writing on the memory, you can flush things
now, and if the user writes on the buffer again later, it will need
to call the dirty IOCTL again. So shouldn't we call
intel_fb_obj_flush(obj, false) here? It seems this was also pointed by
Daniel on the first review. It would be better because it would allow
us to actually keep PSR/FBC enabled.
The flush caused by the dumb modeset ioctl is exactly what I want to
revert here.
Well, I just tested to double check here and flush makes me to miss
screen updates. (triple checked with retired = true as well just in
case)
fbdev comes to place and invalidated frontbuffer, then plymouth does a
flush that makes psr start working when it should continue disabled.
Continue flushing it doesn't solve the problem, just ratify it.
But beside the issue that it is solving I don't believe we want is a
flush anyway. There is something writing directly to frontbuffer with no
invalidation. The dirty call is supposed to be a damage call that
actually tells something on the screen got written and needs to be
updated if it hasn't still. In our cause this is exactly the frontbuffer
invalidate, not the flush.
The flush place would be after it really stopped doing something, but
since I don't trust it I prefer to let it invalidated until next flip.
See my review on the first round of this patch: dirtyfb _is_ a flush. If
it's not enough then there's some other problems still around.
Well, the flush itself in the way it is defined is to flush frontbuffer
bits and put power saving features back to work. PSR flush for instance
doesn't exit on every flush. Only in the cases that we know HW tracking
doesn't work.
One possibility would be change PSR to respect that all flush always is
invalidate (exit) + flush fb_bits. But I'm against this since PSR on
core platforms doesn't have SW trackking ext and it wasn't implemented
to be fully enabled/disabled every time.
Another idea on this line is to make flushs know origins or at least a
boolean to separate cases where flush must be handled as flush bits +
continue working or invalidate + flush bits + start working again
Please let me know what you think.
But putting flushs on this dirty callback is currently useless because
flush just tells psr to continue working without getting screen updates.
Well, this is the most important discussion for now, but also even we
change flush interface or only in PSR I'm not sure this will work.
This dirty case
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Mon, 2015-07-06 at 19:56 +0200, Daniel Vetter wrote:
On Mon, Jul 06, 2015 at 02:11:55PM -0300, Paulo Zanoni wrote:
2015-07-06 13:43 GMT-03:00 Vivi, Rodrigo rodrigo.v...@intel.com:
On Fri, 2015-07-03 at 09:10 +0200, Daniel Vetter wrote:
On Thu, Jul 02, 2015 at 04:41:32PM +, Vivi, Rodrigo wrote:
On Thu, 2015-07-02 at 13:03 -0300, Paulo Zanoni wrote:
2015-06-30 20:42 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
Let's do a frontbuffer invalidation on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a splash
screen involved like plymouth.
Plymoth will do a modeset over ioctl that flushes frontbuffer
tracking and PSR gets back to work while it cannot track the
screen updates and exit properly. However plymouth also uses
a dirtyfb ioctl whenever updating the screen. So let's use it
to invalidate PSR back again.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty
callback is just called after few screen updates and not on
everyone as pointed by Daniel.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..b55b1b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14393,9 +14393,27 @@ static int
intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
return drm_gem_handle_create(file, obj-base, handle);
}
+static int intel_user_framebuffer_dirty(struct drm_framebuffer
*fb,
+ struct drm_file
*file,
+ unsigned flags,
unsigned color,
+ struct
drm_clip_rect *clips,
+ unsigned num_clips)
You don't need the white space on the lines above, just the tabs.
+{
+ struct drm_device *dev = fb-dev;
+ struct intel_framebuffer *intel_fb =
to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+
+ mutex_lock(dev-struct_mutex);
+ intel_fb_obj_invalidate(obj, ORIGIN_GTT);
As far as I understood from my brief investigation, the dirty IOCTL
just says hey, I'm done writing on the memory, you can flush things
now, and if the user writes on the buffer again later, it will need
to call the dirty IOCTL again. So shouldn't we call
intel_fb_obj_flush(obj, false) here? It seems this was also pointed
by
Daniel on the first review. It would be better because it would allow
us to actually keep PSR/FBC enabled.
The flush caused by the dumb modeset ioctl is exactly what I want to
revert here.
Well, I just tested to double check here and flush makes me to miss
screen updates. (triple checked with retired = true as well just in
case)
fbdev comes to place and invalidated frontbuffer, then plymouth does a
flush that makes psr start working when it should continue disabled.
Continue flushing it doesn't solve the problem, just ratify it.
But beside the issue that it is solving I don't believe we want is a
flush anyway. There is something writing directly to frontbuffer with
no
invalidation. The dirty call is supposed to be a damage call that
actually tells something on the screen got written and needs to be
updated if it hasn't still. In our cause this is exactly the
frontbuffer
invalidate, not the flush.
The flush place would be after it really stopped doing something, but
since I don't trust it I prefer to let it invalidated until next flip.
See my review on the first round of this patch: dirtyfb _is_ a flush. If
it's not enough then there's some other problems still around.
Well, the flush itself in the way it is defined is to flush frontbuffer
bits and put power saving features back to work. PSR flush for instance
doesn't exit on every flush. Only in the cases that we know HW tracking
doesn't work.
One possibility would be change PSR to respect that all flush always is
invalidate (exit) + flush fb_bits. But I'm against this since PSR on
core platforms doesn't have SW trackking ext and it wasn't implemented
to be fully enabled/disabled every time.
Another idea on this line is to make flushs know origins or at least a
boolean to separate cases where flush must be handled
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Thu, Jul 02, 2015 at 04:41:32PM +, Vivi, Rodrigo wrote:
On Thu, 2015-07-02 at 13:03 -0300, Paulo Zanoni wrote:
2015-06-30 20:42 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
Let's do a frontbuffer invalidation on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a splash
screen involved like plymouth.
Plymoth will do a modeset over ioctl that flushes frontbuffer
tracking and PSR gets back to work while it cannot track the
screen updates and exit properly. However plymouth also uses
a dirtyfb ioctl whenever updating the screen. So let's use it
to invalidate PSR back again.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty
callback is just called after few screen updates and not on
everyone as pointed by Daniel.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..b55b1b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14393,9 +14393,27 @@ static int
intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
return drm_gem_handle_create(file, obj-base, handle);
}
+static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
+ struct drm_file *file,
+ unsigned flags, unsigned
color,
+ struct drm_clip_rect
*clips,
+ unsigned num_clips)
You don't need the white space on the lines above, just the tabs.
+{
+ struct drm_device *dev = fb-dev;
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+
+ mutex_lock(dev-struct_mutex);
+ intel_fb_obj_invalidate(obj, ORIGIN_GTT);
As far as I understood from my brief investigation, the dirty IOCTL
just says hey, I'm done writing on the memory, you can flush things
now, and if the user writes on the buffer again later, it will need
to call the dirty IOCTL again. So shouldn't we call
intel_fb_obj_flush(obj, false) here? It seems this was also pointed by
Daniel on the first review. It would be better because it would allow
us to actually keep PSR/FBC enabled.
The flush caused by the dumb modeset ioctl is exactly what I want to
revert here.
Well, I just tested to double check here and flush makes me to miss
screen updates. (triple checked with retired = true as well just in
case)
fbdev comes to place and invalidated frontbuffer, then plymouth does a
flush that makes psr start working when it should continue disabled.
Continue flushing it doesn't solve the problem, just ratify it.
But beside the issue that it is solving I don't believe we want is a
flush anyway. There is something writing directly to frontbuffer with no
invalidation. The dirty call is supposed to be a damage call that
actually tells something on the screen got written and needs to be
updated if it hasn't still. In our cause this is exactly the frontbuffer
invalidate, not the flush.
The flush place would be after it really stopped doing something, but
since I don't trust it I prefer to let it invalidated until next flip.
See my review on the first round of this patch: dirtyfb _is_ a flush. If
it's not enough then there's some other problems still around.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Thu, 2015-07-02 at 13:03 -0300, Paulo Zanoni wrote:
2015-06-30 20:42 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
Let's do a frontbuffer invalidation on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a splash
screen involved like plymouth.
Plymoth will do a modeset over ioctl that flushes frontbuffer
tracking and PSR gets back to work while it cannot track the
screen updates and exit properly. However plymouth also uses
a dirtyfb ioctl whenever updating the screen. So let's use it
to invalidate PSR back again.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty
callback is just called after few screen updates and not on
everyone as pointed by Daniel.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..b55b1b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14393,9 +14393,27 @@ static int
intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
return drm_gem_handle_create(file, obj-base, handle);
}
+static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
+ struct drm_file *file,
+ unsigned flags, unsigned
color,
+ struct drm_clip_rect *clips,
+ unsigned num_clips)
You don't need the white space on the lines above, just the tabs.
+{
+ struct drm_device *dev = fb-dev;
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+
+ mutex_lock(dev-struct_mutex);
+ intel_fb_obj_invalidate(obj, ORIGIN_GTT);
As far as I understood from my brief investigation, the dirty IOCTL
just says hey, I'm done writing on the memory, you can flush things
now, and if the user writes on the buffer again later, it will need
to call the dirty IOCTL again. So shouldn't we call
intel_fb_obj_flush(obj, false) here? It seems this was also pointed by
Daniel on the first review. It would be better because it would allow
us to actually keep PSR/FBC enabled.
The flush caused by the dumb modeset ioctl is exactly what I want to
revert here.
Well, I just tested to double check here and flush makes me to miss
screen updates. (triple checked with retired = true as well just in
case)
fbdev comes to place and invalidated frontbuffer, then plymouth does a
flush that makes psr start working when it should continue disabled.
Continue flushing it doesn't solve the problem, just ratify it.
But beside the issue that it is solving I don't believe we want is a
flush anyway. There is something writing directly to frontbuffer with no
invalidation. The dirty call is supposed to be a damage call that
actually tells something on the screen got written and needs to be
updated if it hasn't still. In our cause this is exactly the frontbuffer
invalidate, not the flush.
The flush place would be after it really stopped doing something, but
since I don't trust it I prefer to let it invalidated until next flip.
Also, don't forget that switching to intel_fb_obj_flush() will allow
you to kill the struct_mutex lock.
BTW, I'm also looking forward to see the IGT test suggested by Daniel :)
+ mutex_unlock(dev-struct_mutex);
+
+ return 0;
+}
+
static const struct drm_framebuffer_funcs intel_fb_funcs = {
.destroy = intel_user_framebuffer_destroy,
.create_handle = intel_user_framebuffer_create_handle,
+ .dirty = intel_user_framebuffer_dirty,
};
static
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
2015-06-30 20:42 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com:
Let's do a frontbuffer invalidation on dirty fb.
To be used for DIRTYFB drm ioctl.
This patch solves the biggest PSR known issue, that is
missed screen updates during boot, mainly when there is a splash
screen involved like plymouth.
Plymoth will do a modeset over ioctl that flushes frontbuffer
tracking and PSR gets back to work while it cannot track the
screen updates and exit properly. However plymouth also uses
a dirtyfb ioctl whenever updating the screen. So let's use it
to invalidate PSR back again.
v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty
callback is just called after few screen updates and not on
everyone as pointed by Daniel.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 724b0e3..b55b1b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14393,9 +14393,27 @@ static int
intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
return drm_gem_handle_create(file, obj-base, handle);
}
+static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
+ struct drm_file *file,
+ unsigned flags, unsigned color,
+ struct drm_clip_rect *clips,
+ unsigned num_clips)
You don't need the white space on the lines above, just the tabs.
+{
+ struct drm_device *dev = fb-dev;
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+
+ mutex_lock(dev-struct_mutex);
+ intel_fb_obj_invalidate(obj, ORIGIN_GTT);
As far as I understood from my brief investigation, the dirty IOCTL
just says hey, I'm done writing on the memory, you can flush things
now, and if the user writes on the buffer again later, it will need
to call the dirty IOCTL again. So shouldn't we call
intel_fb_obj_flush(obj, false) here? It seems this was also pointed by
Daniel on the first review. It would be better because it would allow
us to actually keep PSR/FBC enabled.
Also, don't forget that switching to intel_fb_obj_flush() will allow
you to kill the struct_mutex lock.
BTW, I'm also looking forward to see the IGT test suggested by Daniel :)
+ mutex_unlock(dev-struct_mutex);
+
+ return 0;
+}
+
static const struct drm_framebuffer_funcs intel_fb_funcs = {
.destroy = intel_user_framebuffer_destroy,
.create_handle = intel_user_framebuffer_create_handle,
+ .dirty = intel_user_framebuffer_dirty,
};
static
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Wed, Jul 01, 2015 at 09:04:08AM +0100, Chris Wilson wrote: On Tue, Jun 30, 2015 at 04:42:00PM -0700, Rodrigo Vivi wrote: Let's do a frontbuffer invalidation on dirty fb. To be used for DIRTYFB drm ioctl. This patch solves the biggest PSR known issue, that is missed screen updates during boot, mainly when there is a splash screen involved like plymouth. Plymoth will do a modeset over ioctl that flushes frontbuffer tracking and PSR gets back to work while it cannot track the screen updates and exit properly. However plymouth also uses a dirtyfb ioctl whenever updating the screen. So let's use it to invalidate PSR back again. v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty callback is just called after few screen updates and not on everyone as pointed by Daniel. Cc: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Will it ever grow the ability to handle clip rects? I can detect the presence of the syscall and call it appropriately, but I don't want to have to start tracking frontbuffer damage unless there's a significant advantage in doing so (to offset the cost of the tracking). For now this is just for generic userspace using the dumb mmap ioctls, which does already dirty everything. For gem/i915 userspace the existing frontbuffer tracking rules will still apply. I think the damage rect will only be useful for psr2 single-shot uploads, but we need to do some serious hacks using debug rects to pull this off - the hw wants to be way to helpful and gets in the way. And I think for that usecase it only makes sense to supply the overall per-crtc damage rect in an atomic flip. Tracking damage for frontbuffer rendering probably won't be worth it anytime soon at all. In short: Nothing to worry about yet for a long time ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Wed, Jul 01, 2015 at 04:09:19PM +0200, Daniel Vetter wrote: On Wed, Jul 01, 2015 at 02:21:40PM +0100, Chris Wilson wrote: On Wed, Jul 01, 2015 at 03:19:31PM +0200, Daniel Vetter wrote: On Wed, Jul 01, 2015 at 09:04:08AM +0100, Chris Wilson wrote: On Tue, Jun 30, 2015 at 04:42:00PM -0700, Rodrigo Vivi wrote: Let's do a frontbuffer invalidation on dirty fb. To be used for DIRTYFB drm ioctl. This patch solves the biggest PSR known issue, that is missed screen updates during boot, mainly when there is a splash screen involved like plymouth. Plymoth will do a modeset over ioctl that flushes frontbuffer tracking and PSR gets back to work while it cannot track the screen updates and exit properly. However plymouth also uses a dirtyfb ioctl whenever updating the screen. So let's use it to invalidate PSR back again. v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty callback is just called after few screen updates and not on everyone as pointed by Daniel. Cc: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Will it ever grow the ability to handle clip rects? I can detect the presence of the syscall and call it appropriately, but I don't want to have to start tracking frontbuffer damage unless there's a significant advantage in doing so (to offset the cost of the tracking). For now this is just for generic userspace using the dumb mmap ioctls, which does already dirty everything. For gem/i915 userspace the existing frontbuffer tracking rules will still apply. But they are inadequate for the map/set-domain scanout once and write through the GTT for umpteen seconds, which can happen quite frequenctly. In that situation, we behave exactly like fbdev/dumb fb. Yeah you can use it to flush gtt of course too. And there I'd just defensively flush the entire fb until we've grown more clueful in the kernel. But for forntbuffer flushing I don't expect that to ever happen for i915. It makes more sense ofc for udl/qxl and others where uploads are really expensive. Ha, I thought that MIPI was going off in this direction precisely because high pixel count displays only transferring the dirty regions is a big power saving. Likewise I expect at some point, there will be a chipset mode to only portions of the framebuffer out of the chip local cache across the bus. I want to make sure we are not going to shoot ourselves in the foot and can forward-proof the design so that we can easily detect if we want to use cliprects. An easy way would be to return the number of rectangles pushed by dirtyfb i.e. if dirtyfb(fb, .num_rects=1)== 0 I know that it just pushes the whole framebuffer everytime and need not track damage. The ABI would be negative error return on failure, 0 or postive value on success, where the positive value is the number of rects pushed (which should be identical to the user request on success). Dumb users then don't need to care as they can either always request full fb flushes or always push rects. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Wed, Jul 01, 2015 at 03:19:31PM +0200, Daniel Vetter wrote: On Wed, Jul 01, 2015 at 09:04:08AM +0100, Chris Wilson wrote: On Tue, Jun 30, 2015 at 04:42:00PM -0700, Rodrigo Vivi wrote: Let's do a frontbuffer invalidation on dirty fb. To be used for DIRTYFB drm ioctl. This patch solves the biggest PSR known issue, that is missed screen updates during boot, mainly when there is a splash screen involved like plymouth. Plymoth will do a modeset over ioctl that flushes frontbuffer tracking and PSR gets back to work while it cannot track the screen updates and exit properly. However plymouth also uses a dirtyfb ioctl whenever updating the screen. So let's use it to invalidate PSR back again. v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty callback is just called after few screen updates and not on everyone as pointed by Daniel. Cc: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Will it ever grow the ability to handle clip rects? I can detect the presence of the syscall and call it appropriately, but I don't want to have to start tracking frontbuffer damage unless there's a significant advantage in doing so (to offset the cost of the tracking). For now this is just for generic userspace using the dumb mmap ioctls, which does already dirty everything. For gem/i915 userspace the existing frontbuffer tracking rules will still apply. But they are inadequate for the map/set-domain scanout once and write through the GTT for umpteen seconds, which can happen quite frequenctly. In that situation, we behave exactly like fbdev/dumb fb. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Wed, Jul 01, 2015 at 02:21:40PM +0100, Chris Wilson wrote: On Wed, Jul 01, 2015 at 03:19:31PM +0200, Daniel Vetter wrote: On Wed, Jul 01, 2015 at 09:04:08AM +0100, Chris Wilson wrote: On Tue, Jun 30, 2015 at 04:42:00PM -0700, Rodrigo Vivi wrote: Let's do a frontbuffer invalidation on dirty fb. To be used for DIRTYFB drm ioctl. This patch solves the biggest PSR known issue, that is missed screen updates during boot, mainly when there is a splash screen involved like plymouth. Plymoth will do a modeset over ioctl that flushes frontbuffer tracking and PSR gets back to work while it cannot track the screen updates and exit properly. However plymouth also uses a dirtyfb ioctl whenever updating the screen. So let's use it to invalidate PSR back again. v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty callback is just called after few screen updates and not on everyone as pointed by Daniel. Cc: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Will it ever grow the ability to handle clip rects? I can detect the presence of the syscall and call it appropriately, but I don't want to have to start tracking frontbuffer damage unless there's a significant advantage in doing so (to offset the cost of the tracking). For now this is just for generic userspace using the dumb mmap ioctls, which does already dirty everything. For gem/i915 userspace the existing frontbuffer tracking rules will still apply. But they are inadequate for the map/set-domain scanout once and write through the GTT for umpteen seconds, which can happen quite frequenctly. In that situation, we behave exactly like fbdev/dumb fb. Yeah you can use it to flush gtt of course too. And there I'd just defensively flush the entire fb until we've grown more clueful in the kernel. But for forntbuffer flushing I don't expect that to ever happen for i915. It makes more sense ofc for udl/qxl and others where uploads are really expensive. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make fb user dirty operation to invalidate frontbuffer
On Tue, Jun 30, 2015 at 04:42:00PM -0700, Rodrigo Vivi wrote: Let's do a frontbuffer invalidation on dirty fb. To be used for DIRTYFB drm ioctl. This patch solves the biggest PSR known issue, that is missed screen updates during boot, mainly when there is a splash screen involved like plymouth. Plymoth will do a modeset over ioctl that flushes frontbuffer tracking and PSR gets back to work while it cannot track the screen updates and exit properly. However plymouth also uses a dirtyfb ioctl whenever updating the screen. So let's use it to invalidate PSR back again. v2: Remove ORIGIN_FB_DIRTY and use ORIGIN_GTT instead since dirty callback is just called after few screen updates and not on everyone as pointed by Daniel. Cc: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Will it ever grow the ability to handle clip rects? I can detect the presence of the syscall and call it appropriately, but I don't want to have to start tracking frontbuffer damage unless there's a significant advantage in doing so (to offset the cost of the tracking). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
