All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap
@ 2019-02-05  9:15 Chris Wilson
  2019-02-05  9:21 ` Chris Wilson
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Chris Wilson @ 2019-02-05  9:15 UTC (permalink / raw)
  To: intel-gfx

clear_intel_crtc_state() uses the stack for saving a temporary copy of
certain bits of the inherited crtc_state before clearing the unwanted
bits. This pushes it over the stack limit for my little 32b Pineview,
so move the temporary allocation to the heap instead.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 636b703e02cb..d3263650eb62 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11468,44 +11468,47 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 	return ret;
 }
 
-static void
+static int
 clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(crtc_state->base.crtc->dev);
-	struct intel_crtc_scaler_state scaler_state;
-	struct intel_dpll_hw_state dpll_hw_state;
-	struct intel_shared_dpll *shared_dpll;
-	struct intel_crtc_wm_state wm_state;
-	bool force_thru, ips_force_disable;
+	struct intel_crtc_state *saved_state;
+
+	saved_state = kzalloc(sizeof(*saved_state), GFP_KERNEL);
+	if (!saved_state)
+		return -ENOMEM;
 
 	/* FIXME: before the switch to atomic started, a new pipe_config was
 	 * kzalloc'd. Code that depends on any field being zero should be
 	 * fixed, so that the crtc_state can be safely duplicated. For now,
 	 * only fields that are know to not cause problems are preserved. */
 
-	scaler_state = crtc_state->scaler_state;
-	shared_dpll = crtc_state->shared_dpll;
-	dpll_hw_state = crtc_state->dpll_hw_state;
-	force_thru = crtc_state->pch_pfit.force_thru;
-	ips_force_disable = crtc_state->ips_force_disable;
+	saved_state->scaler_state = crtc_state->scaler_state;
+	saved_state->shared_dpll = crtc_state->shared_dpll;
+	saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
+	saved_state->pch_pfit.force_thru = crtc_state->pch_pfit.force_thru;
+	saved_state->ips_force_disable = crtc_state->ips_force_disable;
 	if (IS_G4X(dev_priv) ||
 	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		wm_state = crtc_state->wm;
+		saved_state->wm = crtc_state->wm;
 
 	/* Keep base drm_crtc_state intact, only clear our extended struct */
 	BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
 	memset(&crtc_state->base + 1, 0,
 	       sizeof(*crtc_state) - sizeof(crtc_state->base));
 
-	crtc_state->scaler_state = scaler_state;
-	crtc_state->shared_dpll = shared_dpll;
-	crtc_state->dpll_hw_state = dpll_hw_state;
-	crtc_state->pch_pfit.force_thru = force_thru;
-	crtc_state->ips_force_disable = ips_force_disable;
+	crtc_state->scaler_state = saved_state->scaler_state;
+	crtc_state->shared_dpll = saved_state->shared_dpll;
+	crtc_state->dpll_hw_state = saved_state->dpll_hw_state;
+	crtc_state->pch_pfit.force_thru = saved_state->pch_pfit.force_thru;
+	crtc_state->ips_force_disable = saved_state->ips_force_disable;
 	if (IS_G4X(dev_priv) ||
 	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		crtc_state->wm = wm_state;
+		crtc_state->wm = saved_state->wm;
+
+	kfree(saved_state);
+	return 0;
 }
 
 static int
@@ -11520,7 +11523,9 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 	int i;
 	bool retry = true;
 
-	clear_intel_crtc_state(pipe_config);
+	ret = clear_intel_crtc_state(pipe_config);
+	if (ret)
+		return ret;
 
 	pipe_config->cpu_transcoder =
 		(enum transcoder) to_intel_crtc(crtc)->pipe;
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap
  2019-02-05  9:15 [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap Chris Wilson
@ 2019-02-05  9:21 ` Chris Wilson
  2019-02-05  9:27 ` Chris Wilson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2019-02-05  9:21 UTC (permalink / raw)
  To: intel-gfx

Quoting Chris Wilson (2019-02-05 09:15:52)
> clear_intel_crtc_state() uses the stack for saving a temporary copy of
> certain bits of the inherited crtc_state before clearing the unwanted
> bits. This pushes it over the stack limit for my little 32b Pineview,
> so move the temporary allocation to the heap instead.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++------------
>  1 file changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 636b703e02cb..d3263650eb62 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11468,44 +11468,47 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>         return ret;
>  }
>  
> -static void
> +static int
>  clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
>  {
>         struct drm_i915_private *dev_priv =
>                 to_i915(crtc_state->base.crtc->dev);
> -       struct intel_crtc_scaler_state scaler_state;
> -       struct intel_dpll_hw_state dpll_hw_state;
> -       struct intel_shared_dpll *shared_dpll;
> -       struct intel_crtc_wm_state wm_state;
> -       bool force_thru, ips_force_disable;
> +       struct intel_crtc_state *saved_state;
> +
> +       saved_state = kzalloc(sizeof(*saved_state), GFP_KERNEL);
> +       if (!saved_state)
> +               return -ENOMEM;
>  
>         /* FIXME: before the switch to atomic started, a new pipe_config was
>          * kzalloc'd. Code that depends on any field being zero should be
>          * fixed, so that the crtc_state can be safely duplicated. For now,
>          * only fields that are know to not cause problems are preserved. */
>  
> -       scaler_state = crtc_state->scaler_state;
> -       shared_dpll = crtc_state->shared_dpll;
> -       dpll_hw_state = crtc_state->dpll_hw_state;
> -       force_thru = crtc_state->pch_pfit.force_thru;
> -       ips_force_disable = crtc_state->ips_force_disable;
> +       saved_state->scaler_state = crtc_state->scaler_state;
> +       saved_state->shared_dpll = crtc_state->shared_dpll;
> +       saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
> +       saved_state->pch_pfit.force_thru = crtc_state->pch_pfit.force_thru;
> +       saved_state->ips_force_disable = crtc_state->ips_force_disable;
>         if (IS_G4X(dev_priv) ||
>             IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> -               wm_state = crtc_state->wm;
> +               saved_state->wm = crtc_state->wm;
>  
>         /* Keep base drm_crtc_state intact, only clear our extended struct */
>         BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
>         memset(&crtc_state->base + 1, 0,
>                sizeof(*crtc_state) - sizeof(crtc_state->base));

Now actually, we could reduce this clear to a memcpy...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap
  2019-02-05  9:15 [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap Chris Wilson
  2019-02-05  9:21 ` Chris Wilson
@ 2019-02-05  9:27 ` Chris Wilson
  2019-02-05 19:10   ` Ville Syrjälä
  2019-02-05 10:04 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2019-02-05  9:27 UTC (permalink / raw)
  To: intel-gfx

clear_intel_crtc_state() uses the stack for saving a temporary copy of
certain bits of the inherited crtc_state before clearing the unwanted
bits. This pushes it over the stack limit for my little 32b Pineview,
so move the temporary allocation to the heap instead. As we now use a
zeroed struct, we can copy the whole extended state back to both
preserve what bits need to be preserved and zero the rest.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++---------------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 636b703e02cb..6ee0cd27e875 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11468,44 +11468,38 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 	return ret;
 }
 
-static void
+static int
 clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(crtc_state->base.crtc->dev);
-	struct intel_crtc_scaler_state scaler_state;
-	struct intel_dpll_hw_state dpll_hw_state;
-	struct intel_shared_dpll *shared_dpll;
-	struct intel_crtc_wm_state wm_state;
-	bool force_thru, ips_force_disable;
+	struct intel_crtc_state *saved_state;
+
+	saved_state = kzalloc(sizeof(*saved_state), GFP_KERNEL);
+	if (!saved_state)
+		return -ENOMEM;
 
 	/* FIXME: before the switch to atomic started, a new pipe_config was
 	 * kzalloc'd. Code that depends on any field being zero should be
 	 * fixed, so that the crtc_state can be safely duplicated. For now,
 	 * only fields that are know to not cause problems are preserved. */
 
-	scaler_state = crtc_state->scaler_state;
-	shared_dpll = crtc_state->shared_dpll;
-	dpll_hw_state = crtc_state->dpll_hw_state;
-	force_thru = crtc_state->pch_pfit.force_thru;
-	ips_force_disable = crtc_state->ips_force_disable;
+	saved_state->scaler_state = crtc_state->scaler_state;
+	saved_state->shared_dpll = crtc_state->shared_dpll;
+	saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
+	saved_state->pch_pfit.force_thru = crtc_state->pch_pfit.force_thru;
+	saved_state->ips_force_disable = crtc_state->ips_force_disable;
 	if (IS_G4X(dev_priv) ||
 	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		wm_state = crtc_state->wm;
+		saved_state->wm = crtc_state->wm;
 
 	/* Keep base drm_crtc_state intact, only clear our extended struct */
 	BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
-	memset(&crtc_state->base + 1, 0,
+	memcpy(&crtc_state->base + 1, &saved_state->base + 1,
 	       sizeof(*crtc_state) - sizeof(crtc_state->base));
 
-	crtc_state->scaler_state = scaler_state;
-	crtc_state->shared_dpll = shared_dpll;
-	crtc_state->dpll_hw_state = dpll_hw_state;
-	crtc_state->pch_pfit.force_thru = force_thru;
-	crtc_state->ips_force_disable = ips_force_disable;
-	if (IS_G4X(dev_priv) ||
-	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		crtc_state->wm = wm_state;
+	kfree(saved_state);
+	return 0;
 }
 
 static int
@@ -11520,7 +11514,9 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 	int i;
 	bool retry = true;
 
-	clear_intel_crtc_state(pipe_config);
+	ret = clear_intel_crtc_state(pipe_config);
+	if (ret)
+		return ret;
 
 	pipe_config->cpu_transcoder =
 		(enum transcoder) to_intel_crtc(crtc)->pipe;
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Push clear_intel_crtc_state() onto the heap
  2019-02-05  9:15 [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap Chris Wilson
  2019-02-05  9:21 ` Chris Wilson
  2019-02-05  9:27 ` Chris Wilson
@ 2019-02-05 10:04 ` Patchwork
  2019-02-05 10:39 ` ✓ Fi.CI.BAT: success for drm/i915: Push clear_intel_crtc_state() onto the heap (rev2) Patchwork
  2019-02-05 12:16 ` ✓ Fi.CI.IGT: " Patchwork
  4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2019-02-05 10:04 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Push clear_intel_crtc_state() onto the heap
URL   : https://patchwork.freedesktop.org/series/56217/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5538 -> Patchwork_12134
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56217/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12134 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  
#### Possible fixes ####

  * igt@debugfs_test@read_all_entries:
    - fi-ilk-650:         DMESG-WARN [fdo#106387] -> PASS

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS +1

  * igt@pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       {SKIP} [fdo#109271] -> PASS

  * igt@pm_rpm@basic-rte:
    - fi-bsw-kefka:       FAIL [fdo#108800] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (49 -> 44)
------------------------------

  Missing    (5): fi-kbl-soraka fi-ilk-m540 fi-bsw-cyan fi-icl-y fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5538 -> Patchwork_12134

  CI_DRM_5538: 79ba474280bd19dec91157a47d09436ca38a1747 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4807: b2920f54dc410d5fde705713c7d7eb76ae23dc1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12134: 5d209eba2aed5aad4ac3eecf4598c0a9ca84e9cf @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5d209eba2aed drm/i915: Push clear_intel_crtc_state() onto the heap

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12134/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Push clear_intel_crtc_state() onto the heap (rev2)
  2019-02-05  9:15 [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap Chris Wilson
                   ` (2 preceding siblings ...)
  2019-02-05 10:04 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2019-02-05 10:39 ` Patchwork
  2019-02-05 12:16 ` ✓ Fi.CI.IGT: " Patchwork
  4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2019-02-05 10:39 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Push clear_intel_crtc_state() onto the heap (rev2)
URL   : https://patchwork.freedesktop.org/series/56217/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5539 -> Patchwork_12135
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56217/revisions/2/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12135:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live_objects:
    - {fi-icl-y}:         PASS -> INCOMPLETE

  
Known issues
------------

  Here are the changes found in Patchwork_12135 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927]

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       PASS -> INCOMPLETE [fdo#108602] / [fdo#108744]

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       PASS -> FAIL [fdo#109485]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485


Participating hosts (50 -> 44)
------------------------------

  Missing    (6): fi-ilk-m540 fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-pnv-d510 fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5539 -> Patchwork_12135

  CI_DRM_5539: 1148925c6f5a3e777d4578c840e4ed2e1dbf95be @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4807: b2920f54dc410d5fde705713c7d7eb76ae23dc1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12135: ef6d044e99cb1801882c32de198a52becf9efe4a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ef6d044e99cb drm/i915: Push clear_intel_crtc_state() onto the heap

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12135/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Push clear_intel_crtc_state() onto the heap (rev2)
  2019-02-05  9:15 [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap Chris Wilson
                   ` (3 preceding siblings ...)
  2019-02-05 10:39 ` ✓ Fi.CI.BAT: success for drm/i915: Push clear_intel_crtc_state() onto the heap (rev2) Patchwork
@ 2019-02-05 12:16 ` Patchwork
  4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2019-02-05 12:16 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Push clear_intel_crtc_state() onto the heap (rev2)
URL   : https://patchwork.freedesktop.org/series/56217/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5539_full -> Patchwork_12135_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12135_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@reset-stress:
    - shard-snb:          PASS -> INCOMPLETE [fdo#105411]

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          PASS -> FAIL [fdo#106641]

  * igt@kms_color@pipe-c-ctm-max:
    - shard-apl:          PASS -> FAIL [fdo#108147]

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +3

  * igt@kms_cursor_crc@cursor-64x64-sliding:
    - shard-glk:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-apl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-glk:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max:
    - shard-apl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-none:
    - shard-glk:          PASS -> FAIL [fdo#103166]
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  
#### Possible fixes ####

  * igt@kms_cursor_crc@cursor-128x128-dpms:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-apl:          FAIL [fdo#109350] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-apl:          FAIL [fdo#103167] -> PASS +1
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          {SKIP} [fdo#109271] -> PASS

  
#### Warnings ####

  * igt@i915_suspend@shrink:
    - shard-snb:          INCOMPLETE [fdo#105411] / [fdo#106886] -> DMESG-WARN [fdo#109244]

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-glk:          FAIL [fdo#103232] -> INCOMPLETE [fdo#103359] / [k.org#198133]

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * Linux: CI_DRM_5539 -> Patchwork_12135

  CI_DRM_5539: 1148925c6f5a3e777d4578c840e4ed2e1dbf95be @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4807: b2920f54dc410d5fde705713c7d7eb76ae23dc1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12135: ef6d044e99cb1801882c32de198a52becf9efe4a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12135/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap
  2019-02-05  9:27 ` Chris Wilson
@ 2019-02-05 19:10   ` Ville Syrjälä
  2019-02-05 19:28     ` Chris Wilson
  0 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjälä @ 2019-02-05 19:10 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Tue, Feb 05, 2019 at 09:27:59AM +0000, Chris Wilson wrote:
> clear_intel_crtc_state() uses the stack for saving a temporary copy of
> certain bits of the inherited crtc_state before clearing the unwanted
> bits. This pushes it over the stack limit for my little 32b Pineview,
> so move the temporary allocation to the heap instead. As we now use a
> zeroed struct, we can copy the whole extended state back to both
> preserve what bits need to be preserved and zero the rest.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++---------------
>  1 file changed, 18 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 636b703e02cb..6ee0cd27e875 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11468,44 +11468,38 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>  	return ret;
>  }
>  
> -static void
> +static int
>  clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv =
>  		to_i915(crtc_state->base.crtc->dev);
> -	struct intel_crtc_scaler_state scaler_state;
> -	struct intel_dpll_hw_state dpll_hw_state;
> -	struct intel_shared_dpll *shared_dpll;
> -	struct intel_crtc_wm_state wm_state;
> -	bool force_thru, ips_force_disable;
> +	struct intel_crtc_state *saved_state;
> +
> +	saved_state = kzalloc(sizeof(*saved_state), GFP_KERNEL);
> +	if (!saved_state)
> +		return -ENOMEM;
>  
>  	/* FIXME: before the switch to atomic started, a new pipe_config was
>  	 * kzalloc'd. Code that depends on any field being zero should be
>  	 * fixed, so that the crtc_state can be safely duplicated. For now,
>  	 * only fields that are know to not cause problems are preserved. */
>  
> -	scaler_state = crtc_state->scaler_state;
> -	shared_dpll = crtc_state->shared_dpll;
> -	dpll_hw_state = crtc_state->dpll_hw_state;
> -	force_thru = crtc_state->pch_pfit.force_thru;
> -	ips_force_disable = crtc_state->ips_force_disable;
> +	saved_state->scaler_state = crtc_state->scaler_state;
> +	saved_state->shared_dpll = crtc_state->shared_dpll;
> +	saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
> +	saved_state->pch_pfit.force_thru = crtc_state->pch_pfit.force_thru;
> +	saved_state->ips_force_disable = crtc_state->ips_force_disable;
>  	if (IS_G4X(dev_priv) ||
>  	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> -		wm_state = crtc_state->wm;
> +		saved_state->wm = crtc_state->wm;
>  
>  	/* Keep base drm_crtc_state intact, only clear our extended struct */
>  	BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
> -	memset(&crtc_state->base + 1, 0,
> +	memcpy(&crtc_state->base + 1, &saved_state->base + 1,
>  	       sizeof(*crtc_state) - sizeof(crtc_state->base));
>  
> -	crtc_state->scaler_state = scaler_state;
> -	crtc_state->shared_dpll = shared_dpll;
> -	crtc_state->dpll_hw_state = dpll_hw_state;
> -	crtc_state->pch_pfit.force_thru = force_thru;
> -	crtc_state->ips_force_disable = ips_force_disable;
> -	if (IS_G4X(dev_priv) ||
> -	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> -		crtc_state->wm = wm_state;

Nice. Less risk of someone messing up the save vs. restore.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	kfree(saved_state);
> +	return 0;
>  }
>  
>  static int
> @@ -11520,7 +11514,9 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  	int i;
>  	bool retry = true;
>  
> -	clear_intel_crtc_state(pipe_config);
> +	ret = clear_intel_crtc_state(pipe_config);
> +	if (ret)
> +		return ret;
>  
>  	pipe_config->cpu_transcoder =
>  		(enum transcoder) to_intel_crtc(crtc)->pipe;
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap
  2019-02-05 19:10   ` Ville Syrjälä
@ 2019-02-05 19:28     ` Chris Wilson
  0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2019-02-05 19:28 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Quoting Ville Syrjälä (2019-02-05 19:10:39)
> On Tue, Feb 05, 2019 at 09:27:59AM +0000, Chris Wilson wrote:
> > clear_intel_crtc_state() uses the stack for saving a temporary copy of
> > certain bits of the inherited crtc_state before clearing the unwanted
> > bits. This pushes it over the stack limit for my little 32b Pineview,
> > so move the temporary allocation to the heap instead. As we now use a
> > zeroed struct, we can copy the whole extended state back to both
> > preserve what bits need to be preserved and zero the rest.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++---------------
> >  1 file changed, 18 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 636b703e02cb..6ee0cd27e875 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11468,44 +11468,38 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> >       return ret;
> >  }
> >  
> > -static void
> > +static int
> >  clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> >  {
> >       struct drm_i915_private *dev_priv =
> >               to_i915(crtc_state->base.crtc->dev);
> > -     struct intel_crtc_scaler_state scaler_state;
> > -     struct intel_dpll_hw_state dpll_hw_state;
> > -     struct intel_shared_dpll *shared_dpll;
> > -     struct intel_crtc_wm_state wm_state;
> > -     bool force_thru, ips_force_disable;
> > +     struct intel_crtc_state *saved_state;
> > +
> > +     saved_state = kzalloc(sizeof(*saved_state), GFP_KERNEL);
> > +     if (!saved_state)
> > +             return -ENOMEM;
> >  
> >       /* FIXME: before the switch to atomic started, a new pipe_config was
> >        * kzalloc'd. Code that depends on any field being zero should be
> >        * fixed, so that the crtc_state can be safely duplicated. For now,
> >        * only fields that are know to not cause problems are preserved. */
> >  
> > -     scaler_state = crtc_state->scaler_state;
> > -     shared_dpll = crtc_state->shared_dpll;
> > -     dpll_hw_state = crtc_state->dpll_hw_state;
> > -     force_thru = crtc_state->pch_pfit.force_thru;
> > -     ips_force_disable = crtc_state->ips_force_disable;
> > +     saved_state->scaler_state = crtc_state->scaler_state;
> > +     saved_state->shared_dpll = crtc_state->shared_dpll;
> > +     saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
> > +     saved_state->pch_pfit.force_thru = crtc_state->pch_pfit.force_thru;
> > +     saved_state->ips_force_disable = crtc_state->ips_force_disable;
> >       if (IS_G4X(dev_priv) ||
> >           IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > -             wm_state = crtc_state->wm;
> > +             saved_state->wm = crtc_state->wm;
> >  
> >       /* Keep base drm_crtc_state intact, only clear our extended struct */
> >       BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
> > -     memset(&crtc_state->base + 1, 0,
> > +     memcpy(&crtc_state->base + 1, &saved_state->base + 1,
> >              sizeof(*crtc_state) - sizeof(crtc_state->base));
> >  
> > -     crtc_state->scaler_state = scaler_state;
> > -     crtc_state->shared_dpll = shared_dpll;
> > -     crtc_state->dpll_hw_state = dpll_hw_state;
> > -     crtc_state->pch_pfit.force_thru = force_thru;
> > -     crtc_state->ips_force_disable = ips_force_disable;
> > -     if (IS_G4X(dev_priv) ||
> > -         IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > -             crtc_state->wm = wm_state;
> 
> Nice. Less risk of someone messing up the save vs. restore.

Yup, using the heap is a small price to pay for less typing ;)
 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ta, pushed.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-02-05 19:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-05  9:15 [PATCH] drm/i915: Push clear_intel_crtc_state() onto the heap Chris Wilson
2019-02-05  9:21 ` Chris Wilson
2019-02-05  9:27 ` Chris Wilson
2019-02-05 19:10   ` Ville Syrjälä
2019-02-05 19:28     ` Chris Wilson
2019-02-05 10:04 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-02-05 10:39 ` ✓ Fi.CI.BAT: success for drm/i915: Push clear_intel_crtc_state() onto the heap (rev2) Patchwork
2019-02-05 12:16 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.