Re: [Intel-gfx] [PATCH] drm/i915: Move the unclaimed mmio detection into the powerwell for KMS

2017-06-07 Thread Rodrigo Vivi
It seems appropriate for me.

Although it will now run only if (intel_state->modeset) instead in all
commit_tail it is
apparently in sync with original motivation. And also be able to catch
all unclaimed reg access anyways.

So,
Reviewed-by: Rodrigo Vivi 

On Wed, Jun 7, 2017 at 1:04 PM, Chris Wilson  wrote:
> Quoting Chris Wilson (2017-05-04 12:55:08)
>> Replace the large comment about requiring the powerwell for
>> intel_uncore_arm_unclaimed_mmio_detection() by moving the arming of the
>> mmio error detection into the powerwell held for modesetting. Thereby
>> also accomplishing the goal of only arming the mmio detection after a
>> full modeset.
>>
>> Signed-off-by: Chris Wilson 
>> Cc: Mika Kuoppala 
>> Cc: Daniel Vetter 
>> Cc: Ville Syrjälä 
>
> Ping?
>
>>  drivers/gpu/drm/i915/intel_display.c | 23 +--
>>  1 file changed, 9 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 85b9e2f521a0..14e12e46eda5 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12912,8 +12912,16 @@ static void intel_atomic_commit_tail(struct 
>> drm_atomic_state *state)
>>
>> drm_atomic_helper_commit_hw_done(state);
>>
>> -   if (intel_state->modeset)
>> +   if (intel_state->modeset) {
>> +   /* As one of the primary mmio accessors, KMS has a high
>> +* likelihood of triggering bugs in unclaimed access. After 
>> we
>> +* finish modesetting, see if an error has been flagged, and 
>> if
>> +* so enable debugging for the next modeset - and hope we 
>> catch
>> +* the culprit.
>> +*/
>> +   intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
>> intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
>> +   }
>>
>> mutex_lock(&dev->struct_mutex);
>> drm_atomic_helper_cleanup_planes(dev, state);
>> @@ -12923,19 +12931,6 @@ static void intel_atomic_commit_tail(struct 
>> drm_atomic_state *state)
>>
>> drm_atomic_state_put(state);
>>
>> -   /* As one of the primary mmio accessors, KMS has a high likelihood
>> -* of triggering bugs in unclaimed access. After we finish
>> -* modesetting, see if an error has been flagged, and if so
>> -* enable debugging for the next modeset - and hope we catch
>> -* the culprit.
>> -*
>> -* XXX note that we assume display power is on at this point.
>> -* This might hold true now but we need to add pm helper to check
>> -* unclaimed only when the hardware is on, as atomic commits
>> -* can happen also when the device is completely off.
>> -*/
>> -   intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
>> -
>> intel_atomic_helper_free_state(dev_priv);
>>  }
>>
>> --
>> 2.11.0
>>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Move the unclaimed mmio detection into the powerwell for KMS

2017-06-07 Thread Chris Wilson
Quoting Chris Wilson (2017-05-04 12:55:08)
> Replace the large comment about requiring the powerwell for
> intel_uncore_arm_unclaimed_mmio_detection() by moving the arming of the
> mmio error detection into the powerwell held for modesetting. Thereby
> also accomplishing the goal of only arming the mmio detection after a
> full modeset.
> 
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Daniel Vetter 
> Cc: Ville Syrjälä 

Ping?

>  drivers/gpu/drm/i915/intel_display.c | 23 +--
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 85b9e2f521a0..14e12e46eda5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12912,8 +12912,16 @@ static void intel_atomic_commit_tail(struct 
> drm_atomic_state *state)
>  
> drm_atomic_helper_commit_hw_done(state);
>  
> -   if (intel_state->modeset)
> +   if (intel_state->modeset) {
> +   /* As one of the primary mmio accessors, KMS has a high
> +* likelihood of triggering bugs in unclaimed access. After we
> +* finish modesetting, see if an error has been flagged, and 
> if
> +* so enable debugging for the next modeset - and hope we 
> catch
> +* the culprit.
> +*/
> +   intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
> intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
> +   }
>  
> mutex_lock(&dev->struct_mutex);
> drm_atomic_helper_cleanup_planes(dev, state);
> @@ -12923,19 +12931,6 @@ static void intel_atomic_commit_tail(struct 
> drm_atomic_state *state)
>  
> drm_atomic_state_put(state);
>  
> -   /* As one of the primary mmio accessors, KMS has a high likelihood
> -* of triggering bugs in unclaimed access. After we finish
> -* modesetting, see if an error has been flagged, and if so
> -* enable debugging for the next modeset - and hope we catch
> -* the culprit.
> -*
> -* XXX note that we assume display power is on at this point.
> -* This might hold true now but we need to add pm helper to check
> -* unclaimed only when the hardware is on, as atomic commits
> -* can happen also when the device is completely off.
> -*/
> -   intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
> -
> intel_atomic_helper_free_state(dev_priv);
>  }
>  
> -- 
> 2.11.0
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Move the unclaimed mmio detection into the powerwell for KMS

2017-05-04 Thread Chris Wilson
Replace the large comment about requiring the powerwell for
intel_uncore_arm_unclaimed_mmio_detection() by moving the arming of the
mmio error detection into the powerwell held for modesetting. Thereby
also accomplishing the goal of only arming the mmio detection after a
full modeset.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Daniel Vetter 
Cc: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_display.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 85b9e2f521a0..14e12e46eda5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12912,8 +12912,16 @@ static void intel_atomic_commit_tail(struct 
drm_atomic_state *state)
 
drm_atomic_helper_commit_hw_done(state);
 
-   if (intel_state->modeset)
+   if (intel_state->modeset) {
+   /* As one of the primary mmio accessors, KMS has a high
+* likelihood of triggering bugs in unclaimed access. After we
+* finish modesetting, see if an error has been flagged, and if
+* so enable debugging for the next modeset - and hope we catch
+* the culprit.
+*/
+   intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
+   }
 
mutex_lock(&dev->struct_mutex);
drm_atomic_helper_cleanup_planes(dev, state);
@@ -12923,19 +12931,6 @@ static void intel_atomic_commit_tail(struct 
drm_atomic_state *state)
 
drm_atomic_state_put(state);
 
-   /* As one of the primary mmio accessors, KMS has a high likelihood
-* of triggering bugs in unclaimed access. After we finish
-* modesetting, see if an error has been flagged, and if so
-* enable debugging for the next modeset - and hope we catch
-* the culprit.
-*
-* XXX note that we assume display power is on at this point.
-* This might hold true now but we need to add pm helper to check
-* unclaimed only when the hardware is on, as atomic commits
-* can happen also when the device is completely off.
-*/
-   intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
-
intel_atomic_helper_free_state(dev_priv);
 }
 
-- 
2.11.0

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