Re: [Intel-gfx] [PATCH 1/6] drm/i915: unconditionally copy mode into crtc at boot time

2013-12-17 Thread Daniel Vetter
On Mon, Dec 16, 2013 at 04:35:40PM -0800, Jesse Barnes wrote:
 On Thu, 12 Dec 2013 14:44:33 -0800
 Jesse Barnes jbar...@virtuousgeek.org wrote:
 
  On Thu, 12 Dec 2013 23:39:39 +0100
  Daniel Vetter dan...@ffwll.ch wrote:
  
   On Thu, Dec 12, 2013 at 01:29:39PM -0800, Jesse Barnes wrote:
On Thu, 12 Dec 2013 22:21:29 +0100
Daniel Vetter dan...@ffwll.ch wrote:

 On Thu, Dec 12, 2013 at 12:41:52PM -0800, Jesse Barnes wrote:
  The BIOS code will need this to properly inherit the mode.
  
  Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
  ---
   drivers/gpu/drm/i915/intel_display.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index af3717a..1ae3d44 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -11175,7 +11175,7 @@ void intel_modeset_setup_hw_state(struct 
  drm_device *dev,
   */
  list_for_each_entry(crtc, dev-mode_config.crtc_list,
  base.head) {
  -   if (crtc-active  i915_fastboot) {
  +   if (crtc-active) {
 
 That's just enabling half of the fastboot hack, so nacked.

This part isn't the hack, but is required for fastboot.  Without this,
we'll end up with a bogus mode when we try to inherit from the BIOS.
So if you want to nack this I'll have to put the other BIOS bits under
fastboot as well.
   
   crtc-config.pipe_src_w/h not good enough? I've thought that's what you've
   used in the last iteration, hence my suprise why we suddenly need to
   resurrect this hack here. All the information this hack exposes to
   crtc-mode is available in the pipe config, so I really don't think you
   neeed it.
   
   Count me confused for now, please explain.
  
  Yes, we read out all the data into the pipe config.  But if we don't
  put that data into the CRTC proper, any subsequent code that uses the
  CRTC mode will fail and think there's nothing there (resulting in fail
  at fbcon DPMS time for example).
 
 The actual warning w/o this change is:
 
 [   17.088591] [drm:intel_pipe_config_compare] *ERROR* mismatch in pipe_src_w 
 (expected 0, found 4096)
 [   17.088592] [ cut here ]
 [   17.088619] WARNING: CPU: 1 PID: 534 at 
 drivers/gpu/drm/i915/intel_display.c:9588 check_crtc_state+0x6bf/0xc90 
 [i915]()
 [   17.088619] pipe state doesn't match!
 ...
 [   17.088710]  [a02f2caf] check_crtc_state+0x6bf/0xc90 [i915]
 [   17.088729]  [a03007ab] intel_modeset_check_state+0x2bb/0x7b0 
 [i915]
 [   17.088745]  [a0300d35] intel_set_mode+0x25/0x30 [i915]
 [   17.088762]  [a03015db] intel_crtc_set_config+0x7ab/0x9a0 [i915]
 [   17.088777]  [a0133a4d] drm_mode_set_config_internal+0x5d/0xe0 
 [drm]
 [   17.088783]  [a0184ea1] drm_fb_helper_set_par+0x71/0xf0 
 [drm_kms_helper]
 [   17.088787]  [8135b301] fb_set_var+0x191/0x430
 
 So the first mode set from the fb layer is half baked, because the core
 DRM structures didn't have the right mode to pass down, and so we fall
 over.
 
 I can fix this either by always copying the mode info into the core
 structs, or by putting the call to fbdev_init_bios under i915_fastboot
 as well.

Hm, I think I need to take a closer look here again since I really thought
it'd would Just Work. Hiding more behind the fastboot hack isn't really
what I want either, especially since we want to move from checking the
input mode to checking the pipe config for fastbooting. So having a single
piece which relies on that reconstructed mode would be rather ugly.

Can you please attach the full debug log for this?

Also, could this just be a side effect of the more fancy -initial_config
logic, i.e. what happens if we disable that one?

Cheers, 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 1/6] drm/i915: unconditionally copy mode into crtc at boot time

2013-12-16 Thread Jesse Barnes
On Thu, 12 Dec 2013 14:44:33 -0800
Jesse Barnes jbar...@virtuousgeek.org wrote:

 On Thu, 12 Dec 2013 23:39:39 +0100
 Daniel Vetter dan...@ffwll.ch wrote:
 
  On Thu, Dec 12, 2013 at 01:29:39PM -0800, Jesse Barnes wrote:
   On Thu, 12 Dec 2013 22:21:29 +0100
   Daniel Vetter dan...@ffwll.ch wrote:
   
On Thu, Dec 12, 2013 at 12:41:52PM -0800, Jesse Barnes wrote:
 The BIOS code will need this to properly inherit the mode.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_display.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index af3717a..1ae3d44 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -11175,7 +11175,7 @@ void intel_modeset_setup_hw_state(struct 
 drm_device *dev,
*/
   list_for_each_entry(crtc, dev-mode_config.crtc_list,
   base.head) {
 - if (crtc-active  i915_fastboot) {
 + if (crtc-active) {

That's just enabling half of the fastboot hack, so nacked.
   
   This part isn't the hack, but is required for fastboot.  Without this,
   we'll end up with a bogus mode when we try to inherit from the BIOS.
   So if you want to nack this I'll have to put the other BIOS bits under
   fastboot as well.
  
  crtc-config.pipe_src_w/h not good enough? I've thought that's what you've
  used in the last iteration, hence my suprise why we suddenly need to
  resurrect this hack here. All the information this hack exposes to
  crtc-mode is available in the pipe config, so I really don't think you
  neeed it.
  
  Count me confused for now, please explain.
 
 Yes, we read out all the data into the pipe config.  But if we don't
 put that data into the CRTC proper, any subsequent code that uses the
 CRTC mode will fail and think there's nothing there (resulting in fail
 at fbcon DPMS time for example).

The actual warning w/o this change is:

[   17.088591] [drm:intel_pipe_config_compare] *ERROR* mismatch in pipe_src_w 
(expected 0, found 4096)
[   17.088592] [ cut here ]
[   17.088619] WARNING: CPU: 1 PID: 534 at 
drivers/gpu/drm/i915/intel_display.c:9588 check_crtc_state+0x6bf/0xc90 [i915]()
[   17.088619] pipe state doesn't match!
...
[   17.088710]  [a02f2caf] check_crtc_state+0x6bf/0xc90 [i915]
[   17.088729]  [a03007ab] intel_modeset_check_state+0x2bb/0x7b0 
[i915]
[   17.088745]  [a0300d35] intel_set_mode+0x25/0x30 [i915]
[   17.088762]  [a03015db] intel_crtc_set_config+0x7ab/0x9a0 [i915]
[   17.088777]  [a0133a4d] drm_mode_set_config_internal+0x5d/0xe0 
[drm]
[   17.088783]  [a0184ea1] drm_fb_helper_set_par+0x71/0xf0 
[drm_kms_helper]
[   17.088787]  [8135b301] fb_set_var+0x191/0x430

So the first mode set from the fb layer is half baked, because the core
DRM structures didn't have the right mode to pass down, and so we fall
over.

I can fix this either by always copying the mode info into the core
structs, or by putting the call to fbdev_init_bios under i915_fastboot
as well.

-- 
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 1/6] drm/i915: unconditionally copy mode into crtc at boot time

2013-12-12 Thread Jesse Barnes
The BIOS code will need this to properly inherit the mode.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index af3717a..1ae3d44 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11175,7 +11175,7 @@ void intel_modeset_setup_hw_state(struct drm_device 
*dev,
 */
list_for_each_entry(crtc, dev-mode_config.crtc_list,
base.head) {
-   if (crtc-active  i915_fastboot) {
+   if (crtc-active) {
intel_crtc_mode_from_pipe_config(crtc, crtc-config);
 
DRM_DEBUG_KMS([CRTC:%d] found active mode: ,
-- 
1.8.4.2

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


Re: [Intel-gfx] [PATCH 1/6] drm/i915: unconditionally copy mode into crtc at boot time

2013-12-12 Thread Daniel Vetter
On Thu, Dec 12, 2013 at 12:41:52PM -0800, Jesse Barnes wrote:
 The BIOS code will need this to properly inherit the mode.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_display.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index af3717a..1ae3d44 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -11175,7 +11175,7 @@ void intel_modeset_setup_hw_state(struct drm_device 
 *dev,
*/
   list_for_each_entry(crtc, dev-mode_config.crtc_list,
   base.head) {
 - if (crtc-active  i915_fastboot) {
 + if (crtc-active) {

That's just enabling half of the fastboot hack, so nacked.
-Daniel

   intel_crtc_mode_from_pipe_config(crtc, crtc-config);
  
   DRM_DEBUG_KMS([CRTC:%d] found active mode: ,
 -- 
 1.8.4.2
 
 ___
 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 1/6] drm/i915: unconditionally copy mode into crtc at boot time

2013-12-12 Thread Jesse Barnes
On Thu, 12 Dec 2013 22:21:29 +0100
Daniel Vetter dan...@ffwll.ch wrote:

 On Thu, Dec 12, 2013 at 12:41:52PM -0800, Jesse Barnes wrote:
  The BIOS code will need this to properly inherit the mode.
  
  Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
  ---
   drivers/gpu/drm/i915/intel_display.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index af3717a..1ae3d44 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -11175,7 +11175,7 @@ void intel_modeset_setup_hw_state(struct drm_device 
  *dev,
   */
  list_for_each_entry(crtc, dev-mode_config.crtc_list,
  base.head) {
  -   if (crtc-active  i915_fastboot) {
  +   if (crtc-active) {
 
 That's just enabling half of the fastboot hack, so nacked.

This part isn't the hack, but is required for fastboot.  Without this,
we'll end up with a bogus mode when we try to inherit from the BIOS.
So if you want to nack this I'll have to put the other BIOS bits under
fastboot as well.

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


Re: [Intel-gfx] [PATCH 1/6] drm/i915: unconditionally copy mode into crtc at boot time

2013-12-12 Thread Daniel Vetter
On Thu, Dec 12, 2013 at 01:29:39PM -0800, Jesse Barnes wrote:
 On Thu, 12 Dec 2013 22:21:29 +0100
 Daniel Vetter dan...@ffwll.ch wrote:
 
  On Thu, Dec 12, 2013 at 12:41:52PM -0800, Jesse Barnes wrote:
   The BIOS code will need this to properly inherit the mode.
   
   Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
   ---
drivers/gpu/drm/i915/intel_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
   
   diff --git a/drivers/gpu/drm/i915/intel_display.c 
   b/drivers/gpu/drm/i915/intel_display.c
   index af3717a..1ae3d44 100644
   --- a/drivers/gpu/drm/i915/intel_display.c
   +++ b/drivers/gpu/drm/i915/intel_display.c
   @@ -11175,7 +11175,7 @@ void intel_modeset_setup_hw_state(struct 
   drm_device *dev,
  */
 list_for_each_entry(crtc, dev-mode_config.crtc_list,
 base.head) {
   - if (crtc-active  i915_fastboot) {
   + if (crtc-active) {
  
  That's just enabling half of the fastboot hack, so nacked.
 
 This part isn't the hack, but is required for fastboot.  Without this,
 we'll end up with a bogus mode when we try to inherit from the BIOS.
 So if you want to nack this I'll have to put the other BIOS bits under
 fastboot as well.

crtc-config.pipe_src_w/h not good enough? I've thought that's what you've
used in the last iteration, hence my suprise why we suddenly need to
resurrect this hack here. All the information this hack exposes to
crtc-mode is available in the pipe config, so I really don't think you
neeed it.

Count me confused for now, please explain.
-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 1/6] drm/i915: unconditionally copy mode into crtc at boot time

2013-12-12 Thread Jesse Barnes
On Thu, 12 Dec 2013 23:39:39 +0100
Daniel Vetter dan...@ffwll.ch wrote:

 On Thu, Dec 12, 2013 at 01:29:39PM -0800, Jesse Barnes wrote:
  On Thu, 12 Dec 2013 22:21:29 +0100
  Daniel Vetter dan...@ffwll.ch wrote:
  
   On Thu, Dec 12, 2013 at 12:41:52PM -0800, Jesse Barnes wrote:
The BIOS code will need this to properly inherit the mode.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index af3717a..1ae3d44 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11175,7 +11175,7 @@ void intel_modeset_setup_hw_state(struct 
drm_device *dev,
 */
list_for_each_entry(crtc, dev-mode_config.crtc_list,
base.head) {
-   if (crtc-active  i915_fastboot) {
+   if (crtc-active) {
   
   That's just enabling half of the fastboot hack, so nacked.
  
  This part isn't the hack, but is required for fastboot.  Without this,
  we'll end up with a bogus mode when we try to inherit from the BIOS.
  So if you want to nack this I'll have to put the other BIOS bits under
  fastboot as well.
 
 crtc-config.pipe_src_w/h not good enough? I've thought that's what you've
 used in the last iteration, hence my suprise why we suddenly need to
 resurrect this hack here. All the information this hack exposes to
 crtc-mode is available in the pipe config, so I really don't think you
 neeed it.
 
 Count me confused for now, please explain.

Yes, we read out all the data into the pipe config.  But if we don't
put that data into the CRTC proper, any subsequent code that uses the
CRTC mode will fail and think there's nothing there (resulting in fail
at fbcon DPMS time for example).

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


Re: [Intel-gfx] [PATCH 1/6] drm/i915: unconditionally copy mode into crtc at boot time

2013-12-12 Thread Daniel Vetter
On Thu, Dec 12, 2013 at 02:44:33PM -0800, Jesse Barnes wrote:
 On Thu, 12 Dec 2013 23:39:39 +0100
 Daniel Vetter dan...@ffwll.ch wrote:
 
  On Thu, Dec 12, 2013 at 01:29:39PM -0800, Jesse Barnes wrote:
   On Thu, 12 Dec 2013 22:21:29 +0100
   Daniel Vetter dan...@ffwll.ch wrote:
   
On Thu, Dec 12, 2013 at 12:41:52PM -0800, Jesse Barnes wrote:
 The BIOS code will need this to properly inherit the mode.
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
 ---
  drivers/gpu/drm/i915/intel_display.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index af3717a..1ae3d44 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -11175,7 +11175,7 @@ void intel_modeset_setup_hw_state(struct 
 drm_device *dev,
*/
   list_for_each_entry(crtc, dev-mode_config.crtc_list,
   base.head) {
 - if (crtc-active  i915_fastboot) {
 + if (crtc-active) {

That's just enabling half of the fastboot hack, so nacked.
   
   This part isn't the hack, but is required for fastboot.  Without this,
   we'll end up with a bogus mode when we try to inherit from the BIOS.
   So if you want to nack this I'll have to put the other BIOS bits under
   fastboot as well.
  
  crtc-config.pipe_src_w/h not good enough? I've thought that's what you've
  used in the last iteration, hence my suprise why we suddenly need to
  resurrect this hack here. All the information this hack exposes to
  crtc-mode is available in the pipe config, so I really don't think you
  neeed it.
  
  Count me confused for now, please explain.
 
 Yes, we read out all the data into the pipe config.  But if we don't
 put that data into the CRTC proper, any subsequent code that uses the
 CRTC mode will fail and think there's nothing there (resulting in fail
 at fbcon DPMS time for example).

If fbcon dpms is fumbling around in crtc-mode that's a bug (or at least a
pretty blantant layering violation). The one I know of is the crtc-hwmode
fumbling the vblank code does. But iirc Ville has patches for that
somewhere, and it's not really the worse offense in drm_irq.c.

We obviously need crtc-fb for our code to be happy (since we haven't
split off the primary planes yet). But anything beyond that shouldn't be
required. So I'm still confused ...
-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