[PATCH] drm/i915: Shuffle modeset reset handling around

[PATCH] drm/i915: Shuffle modeset reset handling around

[Daniel Vetter]

Currently we're doing the reset handling a bit late, and we're doing
it both in the driver load code and on resume. This makes it unusable
for e.g. resetting the panel power sequence state like Paulo wants to.

Instead of adding yet another single-use callback shuffle things
around:
- Output handling code is responsible to reset/init all state on its
  own at driver load time.
- We call the reset functions much earlier, before we start using any
  of the modeset code.

Compared to Paulo's new ->resume callback the only difference in
placement is that ->reset is still called without dev->struct_mutex
held. Which is imo a feature.

Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c      | 1 +
 drivers/gpu/drm/i915/intel_crt.c     | 2 ++
 drivers/gpu/drm/i915/intel_display.c | 2 --
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0c7c966f7c1e..b91489ebd3a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -641,6 +641,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 	/* KMS EnterVT equivalent */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		intel_init_pch_refclk(dev);
+		drm_mode_config_reset(dev);
 
 		mutex_lock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index e2e39e65f109..5b444a4b625c 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -857,4 +857,6 @@ void intel_crt_init(struct drm_device *dev)
 
 		dev_priv->fdi_rx_config = I915_READ(_FDI_RXA_CTL) & fdi_config;
 	}
+
+	intel_crt_reset(connector);
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2ef6793c6a0c..f75f28e69c55 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11394,8 +11394,6 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 	}
 
 	intel_modeset_check_state(dev);
-
-	drm_mode_config_reset(dev);
 }
 
 void intel_modeset_gem_init(struct drm_device *dev)
-- 
1.8.5.2

Re: [PATCH] drm/i915: Shuffle modeset reset handling around

[Daniel Vetter]

On Thu, Jan 16, 2014 at 4:24 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Currently we're doing the reset handling a bit late, and we're doing
> it both in the driver load code and on resume. This makes it unusable
> for e.g. resetting the panel power sequence state like Paulo wants to.
>
> Instead of adding yet another single-use callback shuffle things
> around:
> - Output handling code is responsible to reset/init all state on its
>   own at driver load time.
> - We call the reset functions much earlier, before we start using any
>   of the modeset code.
>
> Compared to Paulo's new ->resume callback the only difference in
> placement is that ->reset is still called without dev->struct_mutex
> held. Which is imo a feature.
>
> Cc: Paulo Zanoni <przanoni@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I've forgotten to mention: The LVDS restore code also wants these
semantics when we move the register restoring into intel_lvds.c.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Shuffle modeset reset handling around

[Paulo Zanoni]

2014/1/16 Daniel Vetter <daniel.vetter@ffwll.ch>:
> On Thu, Jan 16, 2014 at 4:24 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> Currently we're doing the reset handling a bit late, and we're doing
>> it both in the driver load code and on resume. This makes it unusable
>> for e.g. resetting the panel power sequence state like Paulo wants to.
>>
>> Instead of adding yet another single-use callback shuffle things
>> around:
>> - Output handling code is responsible to reset/init all state on its
>>   own at driver load time.
>> - We call the reset functions much earlier, before we start using any
>>   of the modeset code.
>>
>> Compared to Paulo's new ->resume callback the only difference in
>> placement is that ->reset is still called without dev->struct_mutex
>> held. Which is imo a feature.
>>
>> Cc: Paulo Zanoni <przanoni@gmail.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> I've forgotten to mention: The LVDS restore code also wants these
> semantics when we move the register restoring into intel_lvds.c.

That doesn't apply on -nightly because of "drm/i915: Do not clobber
config status after a forced restore of hw state", which is on -fixes.
If I try to review this patch as-is, thinking that at some point we'll
merge -fixes back into -next-queued, I keep wondering what your
conflict resolution will look like. So could you please provide a
patch on top of -nightly, so I can try to rewrite the enable_pipe
patches on top of everything?

Thanks,
Paulo

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



-- 
Paulo Zanoni

[PATCH] drm/i915: Shuffle modeset reset handling around

[Daniel Vetter]

Currently we're doing the reset handling a bit late, and we're doing
it both in the driver load code and on resume. This makes it unusable
for e.g. resetting the panel power sequence state like Paulo wants to.

Instead of adding yet another single-use callback shuffle things
around:
- Output handling code is responsible to reset/init all state on its
  own at driver load time.
- We call the reset functions much earlier, before we start using any
  of the modeset code.

Compared to Paulo's new ->resume callback the only difference in
placement is that ->reset is still called without dev->struct_mutex
held. Which is imo a feature.

v2: Rebase on top of the now merge dinq.

Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c      | 2 +-
 drivers/gpu/drm/i915/intel_crt.c     | 2 ++
 drivers/gpu/drm/i915/intel_display.c | 1 -
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 95dcf9de5c80..ce55440b0526 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -643,6 +643,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 	/* KMS EnterVT equivalent */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		intel_init_pch_refclk(dev);
+		drm_mode_config_reset(dev);
 
 		mutex_lock(&dev->struct_mutex);
 
@@ -655,7 +656,6 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 		intel_modeset_init_hw(dev);
 
 		drm_modeset_lock_all(dev);
-		drm_mode_config_reset(dev);
 		intel_modeset_setup_hw_state(dev, true);
 		drm_modeset_unlock_all(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index e2e39e65f109..5b444a4b625c 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -857,4 +857,6 @@ void intel_crt_init(struct drm_device *dev)
 
 		dev_priv->fdi_rx_config = I915_READ(_FDI_RXA_CTL) & fdi_config;
 	}
+
+	intel_crt_reset(connector);
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dde9802033ec..0c9e249e52e6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11412,7 +11412,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
 	intel_setup_overlay(dev);
 
 	mutex_lock(&dev->mode_config.mutex);
-	drm_mode_config_reset(dev);
 	intel_modeset_setup_hw_state(dev, false);
 	mutex_unlock(&dev->mode_config.mutex);
 }
-- 
1.8.5.2

Re: [PATCH] drm/i915: Shuffle modeset reset handling around

[Paulo Zanoni]

2014/1/16 Daniel Vetter <daniel.vetter@ffwll.ch>:
> Currently we're doing the reset handling a bit late, and we're doing
> it both in the driver load code and on resume. This makes it unusable
> for e.g. resetting the panel power sequence state like Paulo wants to.
>
> Instead of adding yet another single-use callback shuffle things
> around:
> - Output handling code is responsible to reset/init all state on its
>   own at driver load time.
> - We call the reset functions much earlier, before we start using any
>   of the modeset code.
>
> Compared to Paulo's new ->resume callback the only difference in
> placement is that ->reset is still called without dev->struct_mutex
> held. Which is imo a feature.
>
> v2: Rebase on top of the now merge dinq.
>
> Cc: Paulo Zanoni <przanoni@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Yeah, now I can use it on the eDP patch.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c      | 2 +-
>  drivers/gpu/drm/i915/intel_crt.c     | 2 ++
>  drivers/gpu/drm/i915/intel_display.c | 1 -
>  3 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 95dcf9de5c80..ce55440b0526 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -643,6 +643,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>         /* KMS EnterVT equivalent */
>         if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>                 intel_init_pch_refclk(dev);
> +               drm_mode_config_reset(dev);
>
>                 mutex_lock(&dev->struct_mutex);
>
> @@ -655,7 +656,6 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>                 intel_modeset_init_hw(dev);
>
>                 drm_modeset_lock_all(dev);
> -               drm_mode_config_reset(dev);
>                 intel_modeset_setup_hw_state(dev, true);
>                 drm_modeset_unlock_all(dev);
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index e2e39e65f109..5b444a4b625c 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -857,4 +857,6 @@ void intel_crt_init(struct drm_device *dev)
>
>                 dev_priv->fdi_rx_config = I915_READ(_FDI_RXA_CTL) & fdi_config;
>         }
> +
> +       intel_crt_reset(connector);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dde9802033ec..0c9e249e52e6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11412,7 +11412,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
>         intel_setup_overlay(dev);
>
>         mutex_lock(&dev->mode_config.mutex);
> -       drm_mode_config_reset(dev);
>         intel_modeset_setup_hw_state(dev, false);
>         mutex_unlock(&dev->mode_config.mutex);
>  }
> --
> 1.8.5.2
>



-- 
Paulo Zanoni

Re: [PATCH] drm/i915: Shuffle modeset reset handling around

[Daniel Vetter]

On Fri, Jan 17, 2014 at 06:14:50PM -0200, Paulo Zanoni wrote:
> 2014/1/16 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > Currently we're doing the reset handling a bit late, and we're doing
> > it both in the driver load code and on resume. This makes it unusable
> > for e.g. resetting the panel power sequence state like Paulo wants to.
> >
> > Instead of adding yet another single-use callback shuffle things
> > around:
> > - Output handling code is responsible to reset/init all state on its
> >   own at driver load time.
> > - We call the reset functions much earlier, before we start using any
> >   of the modeset code.
> >
> > Compared to Paulo's new ->resume callback the only difference in
> > placement is that ->reset is still called without dev->struct_mutex
> > held. Which is imo a feature.
> >
> > v2: Rebase on top of the now merge dinq.
> >
> > Cc: Paulo Zanoni <przanoni@gmail.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Yeah, now I can use it on the eDP patch.
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

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