| Message ID | 1346939019-5866-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2012-09-06 06:43, Daniel Vetter wrote: > Otherwise the new&shiny 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.vetter@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)
On Sun, Sep 9, 2012 at 3:59 AM, Ben Widawsky <ben@bwidawsk.net> wrote: > On 2012-09-06 06:43, Daniel Vetter wrote: >> >> Otherwise the new&shiny 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.vetter@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
On 2012-09-09 02:50, Daniel Vetter wrote: > On Sun, Sep 9, 2012 at 3:59 AM, Ben Widawsky <ben@bwidawsk.net> > wrote: >> On 2012-09-06 06:43, Daniel Vetter wrote: >>> >>> Otherwise the new&shiny 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.vetter@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
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)
Otherwise the new&shiny 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.vetter@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(-)