Re: [Intel-gfx] [PATCH 8/9] drm/i915: treat no fb - fb as simple flip instead of full mode set

2013-03-27 Thread Daniel Vetter 
On Wed, Mar 27, 2013 at 3:09 AM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 If for example the BIOS fb is too small for the dual pipe config we detect,
 we may have valid timings and such, but no fb. The pfit case also hits this
 path (though currently only fastboots if you hack your mode clock to match).

But even when the BIOS fb is to small and the BIOS config uses the
pfit, we /should/ have an fb around. Looking a bit closer I think the
confusion stems from you reading out the adjusted mode, but treating
it like the requested mode. I think it'd be much more solid if we
store the read-out mode in the adjusted_mode (won't work otherwise
with hw state cross-checking anyway), and then do two comparisons
here:

- Does the request mode (plus everything else) match? If so, just do
an fb_set_base call.
- Does the adjusted mode match (plus the entire output routing ofc)?
That means there's either the vga plane or a pfit in-between which we
don't like. If possible we can then play some tricks to avoid the full
modeset.

The reason that I'm a bit freaked about about your change here is that
in a lot of places we treat crtc-fb == NULL as no mode set. So I
fear we're breaking a few too many assumptions here with that hack,
and it simply needs more thought about what we should actually check.
-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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: treat no fb - fb as simple flip instead of full mode set

2013-03-27 Thread Chris Wilson 
On Wed, Mar 27, 2013 at 01:49:47PM +0100, Daniel Vetter wrote:
 On Wed, Mar 27, 2013 at 3:09 AM, Jesse Barnes jbar...@virtuousgeek.org 
 wrote:
  If for example the BIOS fb is too small for the dual pipe config we detect,
  we may have valid timings and such, but no fb. The pfit case also hits this
  path (though currently only fastboots if you hack your mode clock to match).
 
 But even when the BIOS fb is to small and the BIOS config uses the
 pfit, we /should/ have an fb around. Looking a bit closer I think the
 confusion stems from you reading out the adjusted mode, but treating
 it like the requested mode. I think it'd be much more solid if we
 store the read-out mode in the adjusted_mode (won't work otherwise
 with hw state cross-checking anyway), and then do two comparisons
 here:
 
 - Does the request mode (plus everything else) match? If so, just do
 an fb_set_base call.
 - Does the adjusted mode match (plus the entire output routing ofc)?
 That means there's either the vga plane or a pfit in-between which we
 don't like. If possible we can then play some tricks to avoid the full
 modeset.
 
 The reason that I'm a bit freaked about about your change here is that
 in a lot of places we treat crtc-fb == NULL as no mode set. So I
 fear we're breaking a few too many assumptions here with that hack,
 and it simply needs more thought about what we should actually check.

The check here is why we were so enthusiastic about moving to a
pipe_config framework that could match up a configured pipe with an
invalid fb and then just exchange the 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 8/9] drm/i915: treat no fb - fb as simple flip instead of full mode set

2013-03-27 Thread Jesse Barnes 
On Wed, 27 Mar 2013 13:49:47 +0100
Daniel Vetter dan...@ffwll.ch wrote:

 On Wed, Mar 27, 2013 at 3:09 AM, Jesse Barnes jbar...@virtuousgeek.org 
 wrote:
  If for example the BIOS fb is too small for the dual pipe config we detect,
  we may have valid timings and such, but no fb. The pfit case also hits this
  path (though currently only fastboots if you hack your mode clock to match).
 
 But even when the BIOS fb is to small and the BIOS config uses the
 pfit, we /should/ have an fb around. Looking a bit closer I think the
 confusion stems from you reading out the adjusted mode, but treating
 it like the requested mode. I think it'd be much more solid if we
 store the read-out mode in the adjusted_mode (won't work otherwise
 with hw state cross-checking anyway), and then do two comparisons
 here:

Yeah, there's an fb, but it may not be what we want.  So we don't
allocate it and thus crtc-fb is empty.

 - Does the request mode (plus everything else) match? If so, just do
 an fb_set_base call.
 - Does the adjusted mode match (plus the entire output routing ofc)?
 That means there's either the vga plane or a pfit in-between which we
 don't like. If possible we can then play some tricks to avoid the full
 modeset.

Right, I'm trying to play tricks here for sure.

 The reason that I'm a bit freaked about about your change here is that
 in a lot of places we treat crtc-fb == NULL as no mode set. So I
 fear we're breaking a few too many assumptions here with that hack,
 and it simply needs more thought about what we should actually check.

Yeah I was worried about that too.  Maybe we should use crtc-active
for that everywhere?

With the modeset rework and pipe config stuff, we should be able to
drop this in favor of something that looks at the pipe_config and
figures out the minimal mode set sequence required.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 8/9] drm/i915: treat no fb - fb as simple flip instead of full mode set

2013-03-26 Thread Jesse Barnes 
In case we don't get an fb from the BIOS, we may still be able to re-use
existing state and flip a new buffer.

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/intel_display.c |4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 0f3c036..f24da1a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8109,10 +8109,8 @@ intel_set_config_compute_mode_changes(struct 
drm_mode_set *set,
/* We should be able to check here if the fb has the same properties
 * and then just flip_or_move it */
if (set-crtc-fb != set-fb) {
-   /* If we have no fb then treat it as a full mode set */
if (set-crtc-fb == NULL) {
-   DRM_DEBUG_KMS(crtc has no fb, full mode set\n);
-   config-mode_changed = true;
+   config-fb_changed = true;
} else if (set-fb == NULL) {
config-mode_changed = true;
} else if (set-fb-depth != set-crtc-fb-depth) {
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: treat no fb - fb as simple flip instead of full mode set

2013-03-26 Thread Daniel Vetter 
On Tue, Mar 26, 2013 at 04:33:11PM -0700, Jesse Barnes wrote:
 In case we don't get an fb from the BIOS, we may still be able to re-use
 existing state and flip a new buffer.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org

This hack here smells extremely fishy. Where do we come up with no fb, but
a real mode and want to actually fastboot?
-Daniel

 ---
  drivers/gpu/drm/i915/intel_display.c |4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 0f3c036..f24da1a 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -8109,10 +8109,8 @@ intel_set_config_compute_mode_changes(struct 
 drm_mode_set *set,
   /* We should be able to check here if the fb has the same properties
* and then just flip_or_move it */
   if (set-crtc-fb != set-fb) {
 - /* If we have no fb then treat it as a full mode set */
   if (set-crtc-fb == NULL) {
 - DRM_DEBUG_KMS(crtc has no fb, full mode set\n);
 - config-mode_changed = true;
 + config-fb_changed = true;
   } else if (set-fb == NULL) {
   config-mode_changed = true;
   } else if (set-fb-depth != set-crtc-fb-depth) {
 -- 
 1.7.9.5
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: treat no fb - fb as simple flip instead of full mode set

2013-03-26 Thread Jesse Barnes 
If for example the BIOS fb is too small for the dual pipe config we detect, we 
may have valid timings and such, but no fb. The pfit case also hits this path 
(though currently only fastboots if you hack your mode clock to match). 

--
Jesse Barnes, Intel Open Source Technology Center

 Original message 
From: Daniel Vetter dan...@ffwll.ch 
Date: 03/26/2013  5:03 PM  (GMT-08:00) 
To: Jesse Barnes jbar...@virtuousgeek.org 
Cc: intel-gfx@lists.freedesktop.org 
Subject: Re: [Intel-gfx] [PATCH 8/9] drm/i915: treat no fb - fb as simple
  flip instead of full mode set 
 
On Tue, Mar 26, 2013 at 04:33:11PM -0700, Jesse Barnes wrote:
 In case we don't get an fb from the BIOS, we may still be able to re-use
 existing state and flip a new buffer.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org

This hack here smells extremely fishy. Where do we come up with no fb, but
a real mode and want to actually fastboot?
-Daniel

 ---
  drivers/gpu/drm/i915/intel_display.c |    4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 0f3c036..f24da1a 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -8109,10 +8109,8 @@ intel_set_config_compute_mode_changes(struct 
 drm_mode_set *set,
  /* We should be able to check here if the fb has the same properties
  * and then just flip_or_move it */
  if (set-crtc-fb != set-fb) {
 -  /* If we have no fb then treat it as a full mode set */
  if (set-crtc-fb == NULL) {
 -  DRM_DEBUG_KMS(crtc has no fb, full mode set\n);
 -  config-mode_changed = true;
 +  config-fb_changed = true;
  } else if (set-fb == NULL) {
  config-mode_changed = true;
  } else if (set-fb-depth != set-crtc-fb-depth) {
 -- 
 1.7.9.5
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx