Re: [Intel-gfx] [PATCH] drm/i915: reorder setup sequence to have irqs for output setup

2012-09-09 Thread Daniel Vetter 
On Sun, Sep 9, 2012 at 3:59 AM, Ben Widawsky b...@bwidawsk.net wrote:
 On 2012-09-06 06:43, Daniel Vetter wrote:

 Otherwise the newshiny irq-driven gmbus and dp aux code won't work that
 well. Noticed since the dp aux code doesn't have an automatic fallback
 with a timeout (since the hw provides for that already).

 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_dma.c  |  5 ++---
  drivers/gpu/drm/i915/i915_drv.h  |  2 +-
  drivers/gpu/drm/i915/intel_display.c | 15 +--
  3 files changed, 16 insertions(+), 6 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_dma.c
 b/drivers/gpu/drm/i915/i915_dma.c
 index 2cba7b4..58320e0 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -1325,9 +1325,8 @@ static int i915_load_modeset_init(struct
 drm_device *dev)
 if (ret)
 goto cleanup_gem_stolen;

 -   intel_modeset_gem_init(dev);
 -
 -   ret = drm_irq_install(dev);
 +   /* Note: This also enables irqs. */
 +   ret = intel_modeset_gem_init(dev);
 if (ret)
 goto cleanup_gem;


 I'd much rather you just drm_irq_install() before intel_modeset_gem_init(),
 unless there is some reason that won't work?

I've simple been paranoid and wanted to enable the irq as late as
possible. After rechecking the code I think just moving the irq
enabling up a bit should be safe, too. I'll do that.

Thanks, Daniel



 diff --git a/drivers/gpu/drm/i915/i915_drv.h
 b/drivers/gpu/drm/i915/i915_drv.h
 index 9fce782..5c8d5e3 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -1536,7 +1536,7 @@ static inline void
 intel_unregister_dsm_handler(void) { return; }
  /* modesetting */
  extern void intel_modeset_init_hw(struct drm_device *dev);
  extern void intel_modeset_init(struct drm_device *dev);
 -extern void intel_modeset_gem_init(struct drm_device *dev);
 +extern int intel_modeset_gem_init(struct drm_device *dev);
  extern void intel_modeset_cleanup(struct drm_device *dev);
  extern int intel_modeset_vga_set_state(struct drm_device *dev, bool
 state);
  extern void intel_modeset_setup_hw_state(struct drm_device *dev);
 diff --git a/drivers/gpu/drm/i915/intel_display.c
 b/drivers/gpu/drm/i915/intel_display.c
 index 6658367..01289eb 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -7948,7 +7948,6 @@ void intel_modeset_init(struct drm_device *dev)

 /* Just disable it once at startup */
 i915_disable_vga(dev);
 -   intel_setup_outputs(dev);
  }

  static void
 @@ -8206,13 +8205,25 @@ void intel_modeset_setup_hw_state(struct
 drm_device *dev)
 drm_mode_config_reset(dev);
  }

 -void intel_modeset_gem_init(struct drm_device *dev)
 +int intel_modeset_gem_init(struct drm_device *dev)
  {
 +   int ret;
 +
 intel_modeset_init_hw(dev);

 intel_setup_overlay(dev);

 +   ret = drm_irq_install(dev);
 +   if (ret)
 +   return ret;
 +
 +   /* Importanat: Output setup needs working interrupts to use
 +* interrupt-driven gmbus and dp aux. */
 +   intel_setup_outputs(dev);
 +
 intel_modeset_setup_hw_state(dev);
 +
 +   return 0;
  }

  void intel_modeset_cleanup(struct drm_device *dev)


 --
 Ben Widawsky, Intel Open Source Technology Center



-- 
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] drm/i915: reorder setup sequence to have irqs for output setup

2012-09-09 Thread Ben Widawsky 

On 2012-09-09 02:50, Daniel Vetter wrote:
On Sun, Sep 9, 2012 at 3:59 AM, Ben Widawsky b...@bwidawsk.net 
wrote:

On 2012-09-06 06:43, Daniel Vetter wrote:


Otherwise the newshiny irq-driven gmbus and dp aux code won't work 
that
well. Noticed since the dp aux code doesn't have an automatic 
fallback

with a timeout (since the hw provides for that already).

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_dma.c  |  5 ++---
 drivers/gpu/drm/i915/i915_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 15 +--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index 2cba7b4..58320e0 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1325,9 +1325,8 @@ static int i915_load_modeset_init(struct
drm_device *dev)
if (ret)
goto cleanup_gem_stolen;

-   intel_modeset_gem_init(dev);
-
-   ret = drm_irq_install(dev);
+   /* Note: This also enables irqs. */
+   ret = intel_modeset_gem_init(dev);
if (ret)
goto cleanup_gem;



I'd much rather you just drm_irq_install() before 
intel_modeset_gem_init(),

unless there is some reason that won't work?


I've simple been paranoid and wanted to enable the irq as late as
possible. After rechecking the code I think just moving the irq
enabling up a bit should be safe, too. I'll do that.

Thanks, Daniel



mm.interruptible = false; //paranoia
drm_irq_install()
intel_modeset_gem_init()

^^ seemed okay to me





diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 9fce782..5c8d5e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1536,7 +1536,7 @@ static inline void
intel_unregister_dsm_handler(void) { return; }
 /* modesetting */
 extern void intel_modeset_init_hw(struct drm_device *dev);
 extern void intel_modeset_init(struct drm_device *dev);
-extern void intel_modeset_gem_init(struct drm_device *dev);
+extern int intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, 
bool

state);
 extern void intel_modeset_setup_hw_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6658367..01289eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7948,7 +7948,6 @@ void intel_modeset_init(struct drm_device 
*dev)


/* Just disable it once at startup */
i915_disable_vga(dev);
-   intel_setup_outputs(dev);
 }

 static void
@@ -8206,13 +8205,25 @@ void intel_modeset_setup_hw_state(struct
drm_device *dev)
drm_mode_config_reset(dev);
 }

-void intel_modeset_gem_init(struct drm_device *dev)
+int intel_modeset_gem_init(struct drm_device *dev)
 {
+   int ret;
+
intel_modeset_init_hw(dev);

intel_setup_overlay(dev);

+   ret = drm_irq_install(dev);
+   if (ret)
+   return ret;
+
+   /* Importanat: Output setup needs working interrupts to use
+* interrupt-driven gmbus and dp aux. */
+   intel_setup_outputs(dev);
+
intel_modeset_setup_hw_state(dev);
+
+   return 0;
 }

 void intel_modeset_cleanup(struct drm_device *dev)



--
Ben Widawsky, Intel Open Source Technology Center


--
Ben Widawsky, 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] drm/i915: reorder setup sequence to have irqs for output setup

2012-09-08 Thread Ben Widawsky 

On 2012-09-06 06:43, Daniel Vetter wrote:
Otherwise the newshiny irq-driven gmbus and dp aux code won't work 
that
well. Noticed since the dp aux code doesn't have an automatic 
fallback

with a timeout (since the hw provides for that already).

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_dma.c  |  5 ++---
 drivers/gpu/drm/i915/i915_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 15 +--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index 2cba7b4..58320e0 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1325,9 +1325,8 @@ static int i915_load_modeset_init(struct
drm_device *dev)
if (ret)
goto cleanup_gem_stolen;

-   intel_modeset_gem_init(dev);
-
-   ret = drm_irq_install(dev);
+   /* Note: This also enables irqs. */
+   ret = intel_modeset_gem_init(dev);
if (ret)
goto cleanup_gem;



I'd much rather you just drm_irq_install() before 
intel_modeset_gem_init(), unless there is some reason that won't work?



diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 9fce782..5c8d5e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1536,7 +1536,7 @@ static inline void
intel_unregister_dsm_handler(void) { return; }
 /* modesetting */
 extern void intel_modeset_init_hw(struct drm_device *dev);
 extern void intel_modeset_init(struct drm_device *dev);
-extern void intel_modeset_gem_init(struct drm_device *dev);
+extern int intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool 
state);

 extern void intel_modeset_setup_hw_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6658367..01289eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7948,7 +7948,6 @@ void intel_modeset_init(struct drm_device *dev)

/* Just disable it once at startup */
i915_disable_vga(dev);
-   intel_setup_outputs(dev);
 }

 static void
@@ -8206,13 +8205,25 @@ void intel_modeset_setup_hw_state(struct
drm_device *dev)
drm_mode_config_reset(dev);
 }

-void intel_modeset_gem_init(struct drm_device *dev)
+int intel_modeset_gem_init(struct drm_device *dev)
 {
+   int ret;
+
intel_modeset_init_hw(dev);

intel_setup_overlay(dev);

+   ret = drm_irq_install(dev);
+   if (ret)
+   return ret;
+
+   /* Importanat: Output setup needs working interrupts to use
+* interrupt-driven gmbus and dp aux. */
+   intel_setup_outputs(dev);
+
intel_modeset_setup_hw_state(dev);
+
+   return 0;
 }

 void intel_modeset_cleanup(struct drm_device *dev)


--
Ben Widawsky, 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] drm/i915: reorder setup sequence to have irqs for output setup

2012-09-06 Thread Daniel Vetter 
Otherwise the newshiny irq-driven gmbus and dp aux code won't work that
well. Noticed since the dp aux code doesn't have an automatic fallback
with a timeout (since the hw provides for that already).

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_dma.c  |  5 ++---
 drivers/gpu/drm/i915/i915_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 15 +--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2cba7b4..58320e0 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1325,9 +1325,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_gem_stolen;
 
-   intel_modeset_gem_init(dev);
-
-   ret = drm_irq_install(dev);
+   /* Note: This also enables irqs. */
+   ret = intel_modeset_gem_init(dev);
if (ret)
goto cleanup_gem;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9fce782..5c8d5e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1536,7 +1536,7 @@ static inline void intel_unregister_dsm_handler(void) { 
return; }
 /* modesetting */
 extern void intel_modeset_init_hw(struct drm_device *dev);
 extern void intel_modeset_init(struct drm_device *dev);
-extern void intel_modeset_gem_init(struct drm_device *dev);
+extern int intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
 extern void intel_modeset_setup_hw_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6658367..01289eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7948,7 +7948,6 @@ void intel_modeset_init(struct drm_device *dev)
 
/* Just disable it once at startup */
i915_disable_vga(dev);
-   intel_setup_outputs(dev);
 }
 
 static void
@@ -8206,13 +8205,25 @@ void intel_modeset_setup_hw_state(struct drm_device 
*dev)
drm_mode_config_reset(dev);
 }
 
-void intel_modeset_gem_init(struct drm_device *dev)
+int intel_modeset_gem_init(struct drm_device *dev)
 {
+   int ret;
+
intel_modeset_init_hw(dev);
 
intel_setup_overlay(dev);
 
+   ret = drm_irq_install(dev);
+   if (ret)
+   return ret;
+
+   /* Importanat: Output setup needs working interrupts to use
+* interrupt-driven gmbus and dp aux. */
+   intel_setup_outputs(dev);
+
intel_modeset_setup_hw_state(dev);
+
+   return 0;
 }
 
 void intel_modeset_cleanup(struct drm_device *dev)
-- 
1.7.11.2

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