All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/display: Check source height is > 0
@ 2022-12-27  5:53 ` Drew Davenport
  0 siblings, 0 replies; 24+ messages in thread
From: Drew Davenport @ 2022-12-27  5:53 UTC (permalink / raw)
  To: intel-gfx
  Cc: Drew Davenport, Daniel Vetter, David Airlie, Imre Deak,
	Jani Nikula, Joonas Lahtinen, José Roberto de Souza,
	Juha-Pekka Heikkilä,
	Matt Roper, Rodrigo Vivi, Thomas Zimmermann, Tvrtko Ursulin,
	Ville Syrjälä,
	dri-devel, linux-kernel

The error message suggests that the height of the src rect must be at
least 1. Reject source with height of 0.

Signed-off-by: Drew Davenport <ddavenport@chromium.org>

---
I was investigating some divide-by-zero crash reports on ChromeOS which
pointed to the intel_adjusted_rate function. Further prodding showed
that I could reproduce this in a simple test program if I made src_h
some value less than 1 but greater than 0.

This seemed to be a sensible place to check that the source height is at
least 1. I tried to repro this issue on an amd device I had on hand, and
the configuration was rejected.

Would it make sense to add a check that source dimensions are at least 1
somewhere in core, like in drm_atomic_plane_check? Or is that a valid
use case on some devices, and thus any such check should be done on a
per-driver basis?

Thanks.

 drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 4b79c2d2d6177..9b172a1e90deb 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
 	u32 offset;
 	int ret;
 
-	if (w > max_width || w < min_width || h > max_height) {
+	if (w > max_width || w < min_width || h > max_height || h < 1) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
 			    w, h, min_width, max_width, max_height);
-- 
2.39.0.314.g84b9a713c41-goog


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

* [PATCH] drm/i915/display: Check source height is > 0
@ 2022-12-27  5:53 ` Drew Davenport
  0 siblings, 0 replies; 24+ messages in thread
From: Drew Davenport @ 2022-12-27  5:53 UTC (permalink / raw)
  To: intel-gfx
  Cc: dri-devel, Tvrtko Ursulin, Thomas Zimmermann, linux-kernel,
	José Roberto de Souza, Rodrigo Vivi, Drew Davenport,
	Juha-Pekka Heikkilä

The error message suggests that the height of the src rect must be at
least 1. Reject source with height of 0.

Signed-off-by: Drew Davenport <ddavenport@chromium.org>

---
I was investigating some divide-by-zero crash reports on ChromeOS which
pointed to the intel_adjusted_rate function. Further prodding showed
that I could reproduce this in a simple test program if I made src_h
some value less than 1 but greater than 0.

This seemed to be a sensible place to check that the source height is at
least 1. I tried to repro this issue on an amd device I had on hand, and
the configuration was rejected.

Would it make sense to add a check that source dimensions are at least 1
somewhere in core, like in drm_atomic_plane_check? Or is that a valid
use case on some devices, and thus any such check should be done on a
per-driver basis?

Thanks.

 drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 4b79c2d2d6177..9b172a1e90deb 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
 	u32 offset;
 	int ret;
 
-	if (w > max_width || w < min_width || h > max_height) {
+	if (w > max_width || w < min_width || h > max_height || h < 1) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
 			    w, h, min_width, max_width, max_height);
-- 
2.39.0.314.g84b9a713c41-goog


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

* [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
@ 2022-12-27  5:53 ` Drew Davenport
  0 siblings, 0 replies; 24+ messages in thread
From: Drew Davenport @ 2022-12-27  5:53 UTC (permalink / raw)
  To: intel-gfx
  Cc: dri-devel, Thomas Zimmermann, linux-kernel, Daniel Vetter,
	Rodrigo Vivi, Drew Davenport, David Airlie,
	Juha-Pekka Heikkilä

The error message suggests that the height of the src rect must be at
least 1. Reject source with height of 0.

Signed-off-by: Drew Davenport <ddavenport@chromium.org>

---
I was investigating some divide-by-zero crash reports on ChromeOS which
pointed to the intel_adjusted_rate function. Further prodding showed
that I could reproduce this in a simple test program if I made src_h
some value less than 1 but greater than 0.

This seemed to be a sensible place to check that the source height is at
least 1. I tried to repro this issue on an amd device I had on hand, and
the configuration was rejected.

Would it make sense to add a check that source dimensions are at least 1
somewhere in core, like in drm_atomic_plane_check? Or is that a valid
use case on some devices, and thus any such check should be done on a
per-driver basis?

Thanks.

 drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 4b79c2d2d6177..9b172a1e90deb 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
 	u32 offset;
 	int ret;
 
-	if (w > max_width || w < min_width || h > max_height) {
+	if (w > max_width || w < min_width || h > max_height || h < 1) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
 			    w, h, min_width, max_width, max_height);
-- 
2.39.0.314.g84b9a713c41-goog


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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display: Check source height is > 0
  2022-12-27  5:53 ` Drew Davenport
  (?)
  (?)
@ 2022-12-27 15:42 ` Patchwork
  -1 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2022-12-27 15:42 UTC (permalink / raw)
  To: Drew Davenport; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 6118 bytes --]

== Series Details ==

Series: drm/i915/display: Check source height is > 0
URL   : https://patchwork.freedesktop.org/series/112237/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12526 -> Patchwork_112237v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/index.html

Participating hosts (45 -> 44)
------------------------------

  Additional (1): fi-kbl-soraka 
  Missing    (2): fi-bdw-gvtdvm bat-dg2-oem1 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_gttfill@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][1] ([fdo#109271]) +7 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/fi-kbl-soraka/igt@gem_exec_gttfill@basic.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@dmabuf:
    - fi-bsw-nick:        [PASS][4] -> [DMESG-FAIL][5] ([i915#7562])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/fi-bsw-nick/igt@i915_selftest@live@dmabuf.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/fi-bsw-nick/igt@i915_selftest@live@dmabuf.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][6] ([i915#1886])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@late_gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [INCOMPLETE][7] ([i915#7640])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/fi-kbl-soraka/igt@i915_selftest@live@late_gt_pm.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-rkl-guc:         NOTRUN -> [SKIP][8] ([fdo#111827])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/fi-rkl-guc/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][9] ([fdo#109271] / [fdo#111827]) +7 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/fi-kbl-soraka/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@hangcheck:
    - fi-rkl-guc:         [INCOMPLETE][10] ([i915#4983]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/fi-rkl-guc/igt@i915_selftest@live@hangcheck.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/fi-rkl-guc/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@migrate:
    - {bat-adlp-9}:       [DMESG-FAIL][12] ([i915#7699]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/bat-adlp-9/igt@i915_selftest@live@migrate.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/bat-adlp-9/igt@i915_selftest@live@migrate.html
    - {bat-atsm-1}:       [DMESG-FAIL][14] ([i915#7699]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/bat-atsm-1/igt@i915_selftest@live@migrate.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/bat-atsm-1/igt@i915_selftest@live@migrate.html

  * igt@i915_selftest@live@mman:
    - {bat-rpls-1}:       [TIMEOUT][16] ([i915#6794]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/bat-rpls-1/igt@i915_selftest@live@mman.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/bat-rpls-1/igt@i915_selftest@live@mman.html

  * igt@i915_selftest@live@requests:
    - {bat-rpls-2}:       [INCOMPLETE][18] ([i915#4983] / [i915#6257]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/bat-rpls-2/igt@i915_selftest@live@requests.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/bat-rpls-2/igt@i915_selftest@live@requests.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6257]: https://gitlab.freedesktop.org/drm/intel/issues/6257
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
  [i915#7562]: https://gitlab.freedesktop.org/drm/intel/issues/7562
  [i915#7640]: https://gitlab.freedesktop.org/drm/intel/issues/7640
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699


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

  * Linux: CI_DRM_12526 -> Patchwork_112237v1

  CI-20190529: 20190529
  CI_DRM_12526: 34f2552b43d664ce4b76c6e356a57b7835f59c81 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7102: bacfdc84a9c02556c5441deb21e3a3f18a07347d @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_112237v1: 34f2552b43d664ce4b76c6e356a57b7835f59c81 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

1ea1e79f0dab drm/i915/display: Check source height is > 0

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/index.html

[-- Attachment #2: Type: text/html, Size: 7183 bytes --]

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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
  2022-12-27  5:53 ` Drew Davenport
  (?)
@ 2022-12-27 17:55   ` Teres Alexis, Alan Previn
  -1 siblings, 0 replies; 24+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-12-27 17:55 UTC (permalink / raw)
  To: ddavenport, intel-gfx
  Cc: dri-devel, daniel, Vivi, Rodrigo, linux-kernel, airlied,
	tzimmermann, Heikkila, Juha-pekka

Is there a better place for this check higher up the intel specific atomic-check? (so the check won't be skl specific - i notice that intel_adjusted_rate is also called by
ilk_foo as well and non-backend-specific functions). Else, perhaps intel_adjusted_rate should add a check + WARN? (if we are trying to propagate this slowly across HW).


...alan 

On Mon, 2022-12-26 at 22:53 -0700, Drew Davenport wrote:
> The error message suggests that the height of the src rect must be at
> least 1. Reject source with height of 0.
> 
> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> 
> ---
> I was investigating some divide-by-zero crash reports on ChromeOS which
> pointed to the intel_adjusted_rate function. Further prodding showed
> that I could reproduce this in a simple test program if I made src_h
> some value less than 1 but greater than 0.
> 
> This seemed to be a sensible place to check that the source height is at
> least 1. I tried to repro this issue on an amd device I had on hand, and
> the configuration was rejected.
> 
> Would it make sense to add a check that source dimensions are at least 1
> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> use case on some devices, and thus any such check should be done on a
> per-driver basis?
> 
> Thanks.
> 
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 4b79c2d2d6177..9b172a1e90deb 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>  	u32 offset;
>  	int ret;
>  
> -	if (w > max_width || w < min_width || h > max_height) {
> +	if (w > max_width || w < min_width || h > max_height || h < 1) {
>  		drm_dbg_kms(&dev_priv->drm,
>  			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>  			    w, h, min_width, max_width, max_height);
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 


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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
@ 2022-12-27 17:55   ` Teres Alexis, Alan Previn
  0 siblings, 0 replies; 24+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-12-27 17:55 UTC (permalink / raw)
  To: ddavenport, intel-gfx
  Cc: linux-kernel, dri-devel, Heikkila, Juha-pekka, tzimmermann, Vivi,
	Rodrigo

Is there a better place for this check higher up the intel specific atomic-check? (so the check won't be skl specific - i notice that intel_adjusted_rate is also called by
ilk_foo as well and non-backend-specific functions). Else, perhaps intel_adjusted_rate should add a check + WARN? (if we are trying to propagate this slowly across HW).


...alan 

On Mon, 2022-12-26 at 22:53 -0700, Drew Davenport wrote:
> The error message suggests that the height of the src rect must be at
> least 1. Reject source with height of 0.
> 
> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> 
> ---
> I was investigating some divide-by-zero crash reports on ChromeOS which
> pointed to the intel_adjusted_rate function. Further prodding showed
> that I could reproduce this in a simple test program if I made src_h
> some value less than 1 but greater than 0.
> 
> This seemed to be a sensible place to check that the source height is at
> least 1. I tried to repro this issue on an amd device I had on hand, and
> the configuration was rejected.
> 
> Would it make sense to add a check that source dimensions are at least 1
> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> use case on some devices, and thus any such check should be done on a
> per-driver basis?
> 
> Thanks.
> 
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 4b79c2d2d6177..9b172a1e90deb 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>  	u32 offset;
>  	int ret;
>  
> -	if (w > max_width || w < min_width || h > max_height) {
> +	if (w > max_width || w < min_width || h > max_height || h < 1) {
>  		drm_dbg_kms(&dev_priv->drm,
>  			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>  			    w, h, min_width, max_width, max_height);
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 


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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
@ 2022-12-27 17:55   ` Teres Alexis, Alan Previn
  0 siblings, 0 replies; 24+ messages in thread
From: Teres Alexis, Alan Previn @ 2022-12-27 17:55 UTC (permalink / raw)
  To: ddavenport, intel-gfx
  Cc: daniel, linux-kernel, dri-devel, Heikkila, Juha-pekka,
	tzimmermann, Vivi, Rodrigo, airlied

Is there a better place for this check higher up the intel specific atomic-check? (so the check won't be skl specific - i notice that intel_adjusted_rate is also called by
ilk_foo as well and non-backend-specific functions). Else, perhaps intel_adjusted_rate should add a check + WARN? (if we are trying to propagate this slowly across HW).


...alan 

On Mon, 2022-12-26 at 22:53 -0700, Drew Davenport wrote:
> The error message suggests that the height of the src rect must be at
> least 1. Reject source with height of 0.
> 
> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> 
> ---
> I was investigating some divide-by-zero crash reports on ChromeOS which
> pointed to the intel_adjusted_rate function. Further prodding showed
> that I could reproduce this in a simple test program if I made src_h
> some value less than 1 but greater than 0.
> 
> This seemed to be a sensible place to check that the source height is at
> least 1. I tried to repro this issue on an amd device I had on hand, and
> the configuration was rejected.
> 
> Would it make sense to add a check that source dimensions are at least 1
> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> use case on some devices, and thus any such check should be done on a
> per-driver basis?
> 
> Thanks.
> 
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 4b79c2d2d6177..9b172a1e90deb 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>  	u32 offset;
>  	int ret;
>  
> -	if (w > max_width || w < min_width || h > max_height) {
> +	if (w > max_width || w < min_width || h > max_height || h < 1) {
>  		drm_dbg_kms(&dev_priv->drm,
>  			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>  			    w, h, min_width, max_width, max_height);
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 


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

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/display: Check source height is > 0
  2022-12-27  5:53 ` Drew Davenport
                   ` (3 preceding siblings ...)
  (?)
@ 2022-12-27 19:34 ` Patchwork
  -1 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2022-12-27 19:34 UTC (permalink / raw)
  To: Drew Davenport; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 23647 bytes --]

== Series Details ==

Series: drm/i915/display: Check source height is > 0
URL   : https://patchwork.freedesktop.org/series/112237/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12526_full -> Patchwork_112237v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/index.html

Participating hosts (14 -> 10)
------------------------------

  Missing    (4): shard-rkl0 pig-kbl-iris pig-glk-j5005 pig-skl-6260u 

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

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

### IGT changes ###

#### Suppressed ####

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

  * igt@gem_lmem_swapping@basic@lmem0:
    - {shard-dg1}:        [PASS][1] -> [DMESG-WARN][2] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-dg1-18/igt@gem_lmem_swapping@basic@lmem0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-dg1-12/igt@gem_lmem_swapping@basic@lmem0.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-deadline:
    - shard-glk:          [PASS][3] -> [FAIL][4] ([i915#2846])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-glk1/igt@gem_exec_fair@basic-deadline.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk1/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none@vecs0:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([i915#2842])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-glk5/igt@gem_exec_fair@basic-none@vecs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk9/igt@gem_exec_fair@basic-none@vecs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-glk:          NOTRUN -> [FAIL][7] ([i915#2842])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk2/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_lmem_swapping@random:
    - shard-glk:          NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#4613]) +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk8/igt@gem_lmem_swapping@random.html

  * igt@gem_pwrite@basic-exhaustion:
    - shard-glk:          NOTRUN -> [WARN][9] ([i915#2658])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk2/igt@gem_pwrite@basic-exhaustion.html

  * igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc:
    - shard-glk:          NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#3886]) +5 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk2/igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_chamelium@dp-hpd-after-suspend:
    - shard-glk:          NOTRUN -> [SKIP][11] ([fdo#109271] / [fdo#111827]) +2 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk7/igt@kms_chamelium@dp-hpd-after-suspend.html

  * igt@kms_frontbuffer_tracking@psr-1p-pri-indfb-multidraw:
    - shard-glk:          NOTRUN -> [SKIP][12] ([fdo#109271]) +80 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk8/igt@kms_frontbuffer_tracking@psr-1p-pri-indfb-multidraw.html

  * igt@kms_plane_alpha_blend@alpha-basic@pipe-c-hdmi-a-1:
    - shard-glk:          NOTRUN -> [FAIL][13] ([i915#4573]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk8/igt@kms_plane_alpha_blend@alpha-basic@pipe-c-hdmi-a-1.html

  * igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb:
    - shard-glk:          NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#658]) +1 similar issue
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk2/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html

  
#### Possible fixes ####

  * igt@api_intel_bb@object-reloc-keep-cache:
    - {shard-rkl}:        [SKIP][15] ([i915#3281]) -> [PASS][16] +4 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-3/igt@api_intel_bb@object-reloc-keep-cache.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-5/igt@api_intel_bb@object-reloc-keep-cache.html

  * igt@gem_ctx_persistence@hang:
    - {shard-rkl}:        [SKIP][17] ([i915#6252]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-5/igt@gem_ctx_persistence@hang.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-1/igt@gem_ctx_persistence@hang.html

  * igt@gem_eio@reset-stress:
    - {shard-dg1}:        [FAIL][19] ([i915#5784]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-dg1-19/igt@gem_eio@reset-stress.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-dg1-19/igt@gem_eio@reset-stress.html

  * igt@gem_exec_balancer@fairslice:
    - {shard-rkl}:        [SKIP][21] ([i915#6259]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-5/igt@gem_exec_balancer@fairslice.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-3/igt@gem_exec_balancer@fairslice.html

  * igt@gem_mmap_gtt@hang-busy:
    - shard-glk:          [TIMEOUT][23] -> [PASS][24] +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-glk6/igt@gem_mmap_gtt@hang-busy.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk2/igt@gem_mmap_gtt@hang-busy.html

  * igt@gem_userptr_blits@forbidden-operations:
    - {shard-rkl}:        [SKIP][25] ([i915#3282]) -> [PASS][26] +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-2/igt@gem_userptr_blits@forbidden-operations.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-5/igt@gem_userptr_blits@forbidden-operations.html

  * igt@gem_workarounds@suspend-resume-context:
    - {shard-rkl}:        [DMESG-WARN][27] ([i915#5122]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-3/igt@gem_workarounds@suspend-resume-context.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-6/igt@gem_workarounds@suspend-resume-context.html

  * igt@gen9_exec_parse@batch-without-end:
    - {shard-rkl}:        [SKIP][29] ([i915#2527]) -> [PASS][30] +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-2/igt@gen9_exec_parse@batch-without-end.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-5/igt@gen9_exec_parse@batch-without-end.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a:
    - {shard-tglu}:       [FAIL][31] ([i915#3825]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-tglu-2/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-tglu-1/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html

  * igt@i915_pm_rc6_residency@rc6-idle@vecs0:
    - {shard-dg1}:        [FAIL][33] ([i915#3591]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-dg1-19/igt@i915_pm_rc6_residency@rc6-idle@vecs0.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-dg1-17/igt@i915_pm_rc6_residency@rc6-idle@vecs0.html

  * igt@i915_pm_rpm@cursor-dpms:
    - {shard-rkl}:        [SKIP][35] ([i915#1849]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-5/igt@i915_pm_rpm@cursor-dpms.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-6/igt@i915_pm_rpm@cursor-dpms.html

  * igt@i915_pm_rpm@dpms-lpsp:
    - {shard-rkl}:        [SKIP][37] ([i915#1397]) -> [PASS][38] +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-5/igt@i915_pm_rpm@dpms-lpsp.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-6/igt@i915_pm_rpm@dpms-lpsp.html

  * igt@i915_pm_rpm@dpms-mode-unset-lpsp:
    - {shard-dg1}:        [SKIP][39] ([i915#1397]) -> [PASS][40] +1 similar issue
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-dg1-15/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-dg1-14/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-glk:          [DMESG-FAIL][41] ([i915#5334]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-glk6/igt@i915_selftest@live@gt_heartbeat.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk6/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_big_fb@linear-32bpp-rotate-0:
    - shard-glk:          [FAIL][43] ([i915#5138]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-glk8/igt@kms_big_fb@linear-32bpp-rotate-0.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk9/igt@kms_big_fb@linear-32bpp-rotate-0.html

  * igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size:
    - shard-glk:          [FAIL][45] ([i915#2346]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-glk6/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk6/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size.html

  * igt@kms_flip@2x-plain-flip-ts-check-interruptible@bc-hdmi-a1-hdmi-a2:
    - shard-glk:          [FAIL][47] ([i915#2122]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-glk2/igt@kms_flip@2x-plain-flip-ts-check-interruptible@bc-hdmi-a1-hdmi-a2.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk3/igt@kms_flip@2x-plain-flip-ts-check-interruptible@bc-hdmi-a1-hdmi-a2.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite:
    - {shard-rkl}:        [SKIP][49] ([i915#1849] / [i915#4098]) -> [PASS][50] +8 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html

  * igt@kms_plane@plane-panning-bottom-right@pipe-a-planes:
    - {shard-rkl}:        [SKIP][51] ([i915#1849] / [i915#3558]) -> [PASS][52] +1 similar issue
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-5/igt@kms_plane@plane-panning-bottom-right@pipe-a-planes.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-6/igt@kms_plane@plane-panning-bottom-right@pipe-a-planes.html

  * igt@kms_plane@plane-position-covered@pipe-a-planes:
    - {shard-rkl}:        [SKIP][53] ([i915#3558]) -> [PASS][54] +1 similar issue
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-3/igt@kms_plane@plane-position-covered@pipe-a-planes.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-6/igt@kms_plane@plane-position-covered@pipe-a-planes.html

  * igt@kms_psr@sprite_mmap_gtt:
    - {shard-rkl}:        [SKIP][55] ([i915#1072]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-5/igt@kms_psr@sprite_mmap_gtt.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-6/igt@kms_psr@sprite_mmap_gtt.html

  * igt@kms_vblank@pipe-b-ts-continuation-idle:
    - {shard-rkl}:        [SKIP][57] ([i915#1845] / [i915#4098]) -> [PASS][58] +19 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-5/igt@kms_vblank@pipe-b-ts-continuation-idle.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-6/igt@kms_vblank@pipe-b-ts-continuation-idle.html

  * igt@perf@gen8-unprivileged-single-ctx-counters:
    - {shard-rkl}:        [SKIP][59] ([i915#2436]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-rkl-2/igt@perf@gen8-unprivileged-single-ctx-counters.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-rkl-5/igt@perf@gen8-unprivileged-single-ctx-counters.html

  * igt@perf@stress-open-close:
    - shard-glk:          [INCOMPLETE][61] ([i915#5213]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-glk9/igt@perf@stress-open-close.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk7/igt@perf@stress-open-close.html

  * igt@syncobj_wait@wait-all-delayed-signal:
    - {shard-dg1}:        [DMESG-WARN][63] ([i915#1982]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-dg1-14/igt@syncobj_wait@wait-all-delayed-signal.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-dg1-15/igt@syncobj_wait@wait-all-delayed-signal.html

  
#### Warnings ####

  * igt@kms_psr@cursor_render:
    - shard-glk:          [TIMEOUT][65] -> [SKIP][66] ([fdo#109271])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12526/shard-glk6/igt@kms_psr@cursor_render.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112237v1/shard-glk2/igt@kms_psr@cursor_render.html

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

  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109302]: https://bugs.freedesktop.org/show_bug.cgi?id=109302
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109312]: https://bugs.freedesktop.org/show_bug.cgi?id=109312
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111644]: https://bugs.freedesktop.org/show_bug.cgi?id=111644
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1257]: https://gitlab.freedesktop.org/drm/intel/issues/1257
  [i915#132]: https://gitlab.freedesktop.org/drm/intel/issues/132
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#1902]: https://gitlab.freedesktop.org/drm/intel/issues/1902
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2232]: https://gitlab.freedesktop.org/drm/intel/issues/2232
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2435]: https://gitlab.freedesktop.org/drm/intel/issues/2435
  [i915#2436]: https://gitlab.freedesktop.org/drm/intel/issues/2436
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
  [i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
  [i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3323]: https://gitlab.freedesktop.org/drm/intel/issues/3323
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3528]: https://gitlab.freedesktop.org/drm/intel/issues/3528
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3558]: https://gitlab.freedesktop.org/drm/intel/issues/3558
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3825]: https://gitlab.freedesktop.org/drm/intel/issues/3825
  [i915#3826]: https://gitlab.freedesktop.org/drm/intel/issues/3826
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
  [i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4573]: https://gitlab.freedesktop.org/drm/intel/issues/4573
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4767]: https://gitlab.freedesktop.org/drm/intel/issues/4767
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4877]: https://gitlab.freedesktop.org/drm/intel/issues/4877
  [i915#4991]: https://gitlab.freedesktop.org/drm/intel/issues/4991
  [i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
  [i915#5138]: https://gitlab.freedesktop.org/drm/intel/issues/5138
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5213]: https://gitlab.freedesktop.org/drm/intel/issues/5213
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#5327]: https://gitlab.freedesktop.org/drm/intel/issues/5327
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
  [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6117]: https://gitlab.freedesktop.org/drm/intel/issues/6117
  [i915#6247]: https://gitlab.freedesktop.org/drm/intel/issues/6247
  [i915#6248]: https://gitlab.freedesktop.org/drm/intel/issues/6248
  [i915#6252]: https://gitlab.freedesktop.org/drm/intel/issues/6252
  [i915#6259]: https://gitlab.freedesktop.org/drm/intel/issues/6259
  [i915#6335]: https://gitlab.freedesktop.org/drm/intel/issues/6335
  [i915#6355]: https://gitlab.freedesktop.org/drm/intel/issues/6355
  [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
  [i915#6497]: https://gitlab.freedesktop.org/drm/intel/issues/6497
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944
  [i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7582]: https://gitlab.freedesktop.org/drm/intel/issues/7582
  [i915#7651]: https://gitlab.freedesktop.org/drm/intel/issues/7651
  [i915#7679]: https://gitlab.freedesktop.org/drm/intel/issues/7679
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
  [i915#7707]: https://gitlab.freedesktop.org/drm/intel/issues/7707
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711


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

  * Linux: CI_DRM_12526 -> Patchwork_112237v1
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_12526: 34f2552b43d664ce4b76c6e356a57b7835f59c81 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7102: bacfdc84a9c02556c5441deb21e3a3f18a07347d @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_112237v1: 34f2552b43d664ce4b76c6e356a57b7835f59c81 @ 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_112237v1/index.html

[-- Attachment #2: Type: text/html, Size: 18924 bytes --]

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

* Re: [PATCH] drm/i915/display: Check source height is > 0
  2022-12-27  5:53 ` Drew Davenport
  (?)
@ 2023-01-03 10:42   ` Juha-Pekka Heikkila
  -1 siblings, 0 replies; 24+ messages in thread
From: Juha-Pekka Heikkila @ 2023-01-03 10:42 UTC (permalink / raw)
  To: Drew Davenport, intel-gfx
  Cc: dri-devel, Tvrtko Ursulin, Thomas Zimmermann, linux-kernel,
	José Roberto de Souza, Rodrigo Vivi,
	Juha-Pekka Heikkilä

Hi Drew,

this is good find. I went looking where the problem is in and saw what 
you probably also saw earlier.

I was wondering if diff below would be better fix? I assume this would 
end up with einval or erange in your case but code flow otherwise would 
stay as is while fixing all future callers for same issue:

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 10e1fc9d0698..a9948e8d3543 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct 
drm_rect *src,
                                  const struct drm_rect *dst,
                                  unsigned int rate)
  {
-       unsigned int src_w, src_h, dst_w, dst_h;
+       unsigned int src_w, src_h, dst_w, dst_h, dst_wh;

         src_w = drm_rect_width(src) >> 16;
         src_h = drm_rect_height(src) >> 16;
@@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct 
drm_rect *src,
         dst_w = min(src_w, dst_w);
         dst_h = min(src_h, dst_h);

-       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
-                               dst_w * dst_h);
+       /* in case src contained only fractional part */
+       dst_wh = max(dst_w * dst_h, (unsigned) 1);
+
+       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
  }

  unsigned int intel_plane_pixel_rate(const struct intel_crtc_state 
*crtc_state,


What do you think? I'll in any case come up with some test for this in igt.

/Juha-Pekka

On 27.12.2022 7.53, Drew Davenport wrote:
> The error message suggests that the height of the src rect must be at
> least 1. Reject source with height of 0.
> 
> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> 
> ---
> I was investigating some divide-by-zero crash reports on ChromeOS which
> pointed to the intel_adjusted_rate function. Further prodding showed
> that I could reproduce this in a simple test program if I made src_h
> some value less than 1 but greater than 0.
> 
> This seemed to be a sensible place to check that the source height is at
> least 1. I tried to repro this issue on an amd device I had on hand, and
> the configuration was rejected.
> 
> Would it make sense to add a check that source dimensions are at least 1
> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> use case on some devices, and thus any such check should be done on a
> per-driver basis?
> 
> Thanks.
> 
>   drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 4b79c2d2d6177..9b172a1e90deb 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>   	u32 offset;
>   	int ret;
>   
> -	if (w > max_width || w < min_width || h > max_height) {
> +	if (w > max_width || w < min_width || h > max_height || h < 1) {
>   		drm_dbg_kms(&dev_priv->drm,
>   			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>   			    w, h, min_width, max_width, max_height);


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

* Re: [PATCH] drm/i915/display: Check source height is > 0
@ 2023-01-03 10:42   ` Juha-Pekka Heikkila
  0 siblings, 0 replies; 24+ messages in thread
From: Juha-Pekka Heikkila @ 2023-01-03 10:42 UTC (permalink / raw)
  To: Drew Davenport, intel-gfx
  Cc: Tvrtko Ursulin, linux-kernel, José Roberto de Souza,
	dri-devel, Thomas Zimmermann, Rodrigo Vivi,
	Juha-Pekka Heikkilä

Hi Drew,

this is good find. I went looking where the problem is in and saw what 
you probably also saw earlier.

I was wondering if diff below would be better fix? I assume this would 
end up with einval or erange in your case but code flow otherwise would 
stay as is while fixing all future callers for same issue:

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 10e1fc9d0698..a9948e8d3543 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct 
drm_rect *src,
                                  const struct drm_rect *dst,
                                  unsigned int rate)
  {
-       unsigned int src_w, src_h, dst_w, dst_h;
+       unsigned int src_w, src_h, dst_w, dst_h, dst_wh;

         src_w = drm_rect_width(src) >> 16;
         src_h = drm_rect_height(src) >> 16;
@@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct 
drm_rect *src,
         dst_w = min(src_w, dst_w);
         dst_h = min(src_h, dst_h);

-       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
-                               dst_w * dst_h);
+       /* in case src contained only fractional part */
+       dst_wh = max(dst_w * dst_h, (unsigned) 1);
+
+       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
  }

  unsigned int intel_plane_pixel_rate(const struct intel_crtc_state 
*crtc_state,


What do you think? I'll in any case come up with some test for this in igt.

/Juha-Pekka

On 27.12.2022 7.53, Drew Davenport wrote:
> The error message suggests that the height of the src rect must be at
> least 1. Reject source with height of 0.
> 
> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> 
> ---
> I was investigating some divide-by-zero crash reports on ChromeOS which
> pointed to the intel_adjusted_rate function. Further prodding showed
> that I could reproduce this in a simple test program if I made src_h
> some value less than 1 but greater than 0.
> 
> This seemed to be a sensible place to check that the source height is at
> least 1. I tried to repro this issue on an amd device I had on hand, and
> the configuration was rejected.
> 
> Would it make sense to add a check that source dimensions are at least 1
> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> use case on some devices, and thus any such check should be done on a
> per-driver basis?
> 
> Thanks.
> 
>   drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 4b79c2d2d6177..9b172a1e90deb 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>   	u32 offset;
>   	int ret;
>   
> -	if (w > max_width || w < min_width || h > max_height) {
> +	if (w > max_width || w < min_width || h > max_height || h < 1) {
>   		drm_dbg_kms(&dev_priv->drm,
>   			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>   			    w, h, min_width, max_width, max_height);


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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
@ 2023-01-03 10:42   ` Juha-Pekka Heikkila
  0 siblings, 0 replies; 24+ messages in thread
From: Juha-Pekka Heikkila @ 2023-01-03 10:42 UTC (permalink / raw)
  To: Drew Davenport, intel-gfx
  Cc: linux-kernel, dri-devel, Thomas Zimmermann, Rodrigo Vivi,
	Juha-Pekka Heikkilä

Hi Drew,

this is good find. I went looking where the problem is in and saw what 
you probably also saw earlier.

I was wondering if diff below would be better fix? I assume this would 
end up with einval or erange in your case but code flow otherwise would 
stay as is while fixing all future callers for same issue:

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 10e1fc9d0698..a9948e8d3543 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct 
drm_rect *src,
                                  const struct drm_rect *dst,
                                  unsigned int rate)
  {
-       unsigned int src_w, src_h, dst_w, dst_h;
+       unsigned int src_w, src_h, dst_w, dst_h, dst_wh;

         src_w = drm_rect_width(src) >> 16;
         src_h = drm_rect_height(src) >> 16;
@@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct 
drm_rect *src,
         dst_w = min(src_w, dst_w);
         dst_h = min(src_h, dst_h);

-       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
-                               dst_w * dst_h);
+       /* in case src contained only fractional part */
+       dst_wh = max(dst_w * dst_h, (unsigned) 1);
+
+       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
  }

  unsigned int intel_plane_pixel_rate(const struct intel_crtc_state 
*crtc_state,


What do you think? I'll in any case come up with some test for this in igt.

/Juha-Pekka

On 27.12.2022 7.53, Drew Davenport wrote:
> The error message suggests that the height of the src rect must be at
> least 1. Reject source with height of 0.
> 
> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> 
> ---
> I was investigating some divide-by-zero crash reports on ChromeOS which
> pointed to the intel_adjusted_rate function. Further prodding showed
> that I could reproduce this in a simple test program if I made src_h
> some value less than 1 but greater than 0.
> 
> This seemed to be a sensible place to check that the source height is at
> least 1. I tried to repro this issue on an amd device I had on hand, and
> the configuration was rejected.
> 
> Would it make sense to add a check that source dimensions are at least 1
> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> use case on some devices, and thus any such check should be done on a
> per-driver basis?
> 
> Thanks.
> 
>   drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 4b79c2d2d6177..9b172a1e90deb 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>   	u32 offset;
>   	int ret;
>   
> -	if (w > max_width || w < min_width || h > max_height) {
> +	if (w > max_width || w < min_width || h > max_height || h < 1) {
>   		drm_dbg_kms(&dev_priv->drm,
>   			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>   			    w, h, min_width, max_width, max_height);


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

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/display: Check source height is > 0 (rev2)
  2022-12-27  5:53 ` Drew Davenport
                   ` (5 preceding siblings ...)
  (?)
@ 2023-01-03 10:56 ` Patchwork
  -1 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2023-01-03 10:56 UTC (permalink / raw)
  To: Juha-Pekka Heikkila; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/display: Check source height is > 0 (rev2)
URL   : https://patchwork.freedesktop.org/series/112237/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/112237/revisions/2/mbox/ not applied
Applying: drm/i915/display: Check source height is > 0
error: git diff header lacks filename information when removing 1 leading pathname component (line 2)
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915/display: Check source height is > 0
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".



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

* Re: [PATCH] drm/i915/display: Check source height is > 0
  2023-01-03 10:42   ` Juha-Pekka Heikkila
  (?)
@ 2023-01-10 20:30     ` Drew Davenport
  -1 siblings, 0 replies; 24+ messages in thread
From: Drew Davenport @ 2023-01-10 20:30 UTC (permalink / raw)
  To: Juha-Pekka Heikkila
  Cc: Tvrtko Ursulin, intel-gfx, linux-kernel,
	José Roberto de Souza, dri-devel, Thomas Zimmermann,
	Rodrigo Vivi, Juha-Pekka Heikkilä

On Tue, Jan 03, 2023 at 12:42:43PM +0200, Juha-Pekka Heikkila wrote:
> Hi Drew,

Hi Juha-Pekka, sorry for the late response since I was on vacation.

> 
> this is good find. I went looking where the problem is in and saw what you
> probably also saw earlier.
> 
> I was wondering if diff below would be better fix? I assume this would end
> up with einval or erange in your case but code flow otherwise would stay as
> is while fixing all future callers for same issue:

Yes, the function you identify below is where I encountered
divide-by-zero errors. If width/height less than 1 is a legitimate use
case, then your proposed solution looks like it would be better. It
should have no risk of regression in userspace either, which is nice.

I tested your patch, and as expected I did not hit the divide-by-zero
error, though the test commit was rejected due to a check further along
inside skl_update_scaler. Perhaps there is some other configuration
which would pass the test commit with a width/height less than 1, but I
didn't dig much further.

> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 10e1fc9d0698..a9948e8d3543 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> *src,
>                                  const struct drm_rect *dst,
>                                  unsigned int rate)
>  {
> -       unsigned int src_w, src_h, dst_w, dst_h;
> +       unsigned int src_w, src_h, dst_w, dst_h, dst_wh;
> 
>         src_w = drm_rect_width(src) >> 16;
>         src_h = drm_rect_height(src) >> 16;
> @@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> *src,
>         dst_w = min(src_w, dst_w);
>         dst_h = min(src_h, dst_h);
> 
> -       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
> -                               dst_w * dst_h);
> +       /* in case src contained only fractional part */
> +       dst_wh = max(dst_w * dst_h, (unsigned) 1);
> +
> +       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
>  }
> 
>  unsigned int intel_plane_pixel_rate(const struct intel_crtc_state
> *crtc_state,
> 
> 
> What do you think? I'll in any case come up with some test for this in igt.

I see that you've posted your fix to the list already. Adding a
test to cover this in IGT also sounds great. Thanks!

Breadcrumbs to Juha-Pekka's patch for anyone following this
thread: https://patchwork.freedesktop.org/series/112396/

> 
> /Juha-Pekka
> 
> On 27.12.2022 7.53, Drew Davenport wrote:
> > The error message suggests that the height of the src rect must be at
> > least 1. Reject source with height of 0.
> > 
> > Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> > 
> > ---
> > I was investigating some divide-by-zero crash reports on ChromeOS which
> > pointed to the intel_adjusted_rate function. Further prodding showed
> > that I could reproduce this in a simple test program if I made src_h
> > some value less than 1 but greater than 0.
> > 
> > This seemed to be a sensible place to check that the source height is at
> > least 1. I tried to repro this issue on an amd device I had on hand, and
> > the configuration was rejected.
> > 
> > Would it make sense to add a check that source dimensions are at least 1
> > somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> > use case on some devices, and thus any such check should be done on a
> > per-driver basis?
> > 
> > Thanks.
> > 
> >   drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 4b79c2d2d6177..9b172a1e90deb 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> >   	u32 offset;
> >   	int ret;
> > -	if (w > max_width || w < min_width || h > max_height) {
> > +	if (w > max_width || w < min_width || h > max_height || h < 1) {
> >   		drm_dbg_kms(&dev_priv->drm,
> >   			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
> >   			    w, h, min_width, max_width, max_height);
> 

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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
@ 2023-01-10 20:30     ` Drew Davenport
  0 siblings, 0 replies; 24+ messages in thread
From: Drew Davenport @ 2023-01-10 20:30 UTC (permalink / raw)
  To: Juha-Pekka Heikkila
  Cc: intel-gfx, linux-kernel, dri-devel, Thomas Zimmermann,
	Rodrigo Vivi, Juha-Pekka Heikkilä

On Tue, Jan 03, 2023 at 12:42:43PM +0200, Juha-Pekka Heikkila wrote:
> Hi Drew,

Hi Juha-Pekka, sorry for the late response since I was on vacation.

> 
> this is good find. I went looking where the problem is in and saw what you
> probably also saw earlier.
> 
> I was wondering if diff below would be better fix? I assume this would end
> up with einval or erange in your case but code flow otherwise would stay as
> is while fixing all future callers for same issue:

Yes, the function you identify below is where I encountered
divide-by-zero errors. If width/height less than 1 is a legitimate use
case, then your proposed solution looks like it would be better. It
should have no risk of regression in userspace either, which is nice.

I tested your patch, and as expected I did not hit the divide-by-zero
error, though the test commit was rejected due to a check further along
inside skl_update_scaler. Perhaps there is some other configuration
which would pass the test commit with a width/height less than 1, but I
didn't dig much further.

> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 10e1fc9d0698..a9948e8d3543 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> *src,
>                                  const struct drm_rect *dst,
>                                  unsigned int rate)
>  {
> -       unsigned int src_w, src_h, dst_w, dst_h;
> +       unsigned int src_w, src_h, dst_w, dst_h, dst_wh;
> 
>         src_w = drm_rect_width(src) >> 16;
>         src_h = drm_rect_height(src) >> 16;
> @@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> *src,
>         dst_w = min(src_w, dst_w);
>         dst_h = min(src_h, dst_h);
> 
> -       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
> -                               dst_w * dst_h);
> +       /* in case src contained only fractional part */
> +       dst_wh = max(dst_w * dst_h, (unsigned) 1);
> +
> +       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
>  }
> 
>  unsigned int intel_plane_pixel_rate(const struct intel_crtc_state
> *crtc_state,
> 
> 
> What do you think? I'll in any case come up with some test for this in igt.

I see that you've posted your fix to the list already. Adding a
test to cover this in IGT also sounds great. Thanks!

Breadcrumbs to Juha-Pekka's patch for anyone following this
thread: https://patchwork.freedesktop.org/series/112396/

> 
> /Juha-Pekka
> 
> On 27.12.2022 7.53, Drew Davenport wrote:
> > The error message suggests that the height of the src rect must be at
> > least 1. Reject source with height of 0.
> > 
> > Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> > 
> > ---
> > I was investigating some divide-by-zero crash reports on ChromeOS which
> > pointed to the intel_adjusted_rate function. Further prodding showed
> > that I could reproduce this in a simple test program if I made src_h
> > some value less than 1 but greater than 0.
> > 
> > This seemed to be a sensible place to check that the source height is at
> > least 1. I tried to repro this issue on an amd device I had on hand, and
> > the configuration was rejected.
> > 
> > Would it make sense to add a check that source dimensions are at least 1
> > somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> > use case on some devices, and thus any such check should be done on a
> > per-driver basis?
> > 
> > Thanks.
> > 
> >   drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 4b79c2d2d6177..9b172a1e90deb 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> >   	u32 offset;
> >   	int ret;
> > -	if (w > max_width || w < min_width || h > max_height) {
> > +	if (w > max_width || w < min_width || h > max_height || h < 1) {
> >   		drm_dbg_kms(&dev_priv->drm,
> >   			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
> >   			    w, h, min_width, max_width, max_height);
> 

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

* Re: [PATCH] drm/i915/display: Check source height is > 0
@ 2023-01-10 20:30     ` Drew Davenport
  0 siblings, 0 replies; 24+ messages in thread
From: Drew Davenport @ 2023-01-10 20:30 UTC (permalink / raw)
  To: Juha-Pekka Heikkila
  Cc: intel-gfx, dri-devel, Tvrtko Ursulin, Thomas Zimmermann,
	linux-kernel, José Roberto de Souza, Rodrigo Vivi,
	Juha-Pekka Heikkilä

On Tue, Jan 03, 2023 at 12:42:43PM +0200, Juha-Pekka Heikkila wrote:
> Hi Drew,

Hi Juha-Pekka, sorry for the late response since I was on vacation.

> 
> this is good find. I went looking where the problem is in and saw what you
> probably also saw earlier.
> 
> I was wondering if diff below would be better fix? I assume this would end
> up with einval or erange in your case but code flow otherwise would stay as
> is while fixing all future callers for same issue:

Yes, the function you identify below is where I encountered
divide-by-zero errors. If width/height less than 1 is a legitimate use
case, then your proposed solution looks like it would be better. It
should have no risk of regression in userspace either, which is nice.

I tested your patch, and as expected I did not hit the divide-by-zero
error, though the test commit was rejected due to a check further along
inside skl_update_scaler. Perhaps there is some other configuration
which would pass the test commit with a width/height less than 1, but I
didn't dig much further.

> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 10e1fc9d0698..a9948e8d3543 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> *src,
>                                  const struct drm_rect *dst,
>                                  unsigned int rate)
>  {
> -       unsigned int src_w, src_h, dst_w, dst_h;
> +       unsigned int src_w, src_h, dst_w, dst_h, dst_wh;
> 
>         src_w = drm_rect_width(src) >> 16;
>         src_h = drm_rect_height(src) >> 16;
> @@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> *src,
>         dst_w = min(src_w, dst_w);
>         dst_h = min(src_h, dst_h);
> 
> -       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
> -                               dst_w * dst_h);
> +       /* in case src contained only fractional part */
> +       dst_wh = max(dst_w * dst_h, (unsigned) 1);
> +
> +       return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
>  }
> 
>  unsigned int intel_plane_pixel_rate(const struct intel_crtc_state
> *crtc_state,
> 
> 
> What do you think? I'll in any case come up with some test for this in igt.

I see that you've posted your fix to the list already. Adding a
test to cover this in IGT also sounds great. Thanks!

Breadcrumbs to Juha-Pekka's patch for anyone following this
thread: https://patchwork.freedesktop.org/series/112396/

> 
> /Juha-Pekka
> 
> On 27.12.2022 7.53, Drew Davenport wrote:
> > The error message suggests that the height of the src rect must be at
> > least 1. Reject source with height of 0.
> > 
> > Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> > 
> > ---
> > I was investigating some divide-by-zero crash reports on ChromeOS which
> > pointed to the intel_adjusted_rate function. Further prodding showed
> > that I could reproduce this in a simple test program if I made src_h
> > some value less than 1 but greater than 0.
> > 
> > This seemed to be a sensible place to check that the source height is at
> > least 1. I tried to repro this issue on an amd device I had on hand, and
> > the configuration was rejected.
> > 
> > Would it make sense to add a check that source dimensions are at least 1
> > somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> > use case on some devices, and thus any such check should be done on a
> > per-driver basis?
> > 
> > Thanks.
> > 
> >   drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 4b79c2d2d6177..9b172a1e90deb 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> >   	u32 offset;
> >   	int ret;
> > -	if (w > max_width || w < min_width || h > max_height) {
> > +	if (w > max_width || w < min_width || h > max_height || h < 1) {
> >   		drm_dbg_kms(&dev_priv->drm,
> >   			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
> >   			    w, h, min_width, max_width, max_height);
> 

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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
  2022-12-27 17:55   ` Teres Alexis, Alan Previn
  (?)
@ 2023-01-11 18:47     ` Drew Davenport
  -1 siblings, 0 replies; 24+ messages in thread
From: Drew Davenport @ 2023-01-11 18:47 UTC (permalink / raw)
  To: Teres Alexis, Alan Previn
  Cc: intel-gfx, dri-devel, daniel, Vivi, Rodrigo, linux-kernel,
	airlied, tzimmermann, Heikkila, Juha-pekka

On Tue, Dec 27, 2022 at 05:55:17PM +0000, Teres Alexis, Alan Previn wrote:
> Is there a better place for this check higher up the intel specific atomic-check? (so the check won't be skl specific - i notice that intel_adjusted_rate is also called by
> ilk_foo as well and non-backend-specific functions). Else, perhaps intel_adjusted_rate should add a check + WARN? (if we are trying to propagate this slowly across HW).

Would intel_plane_atomic_check_with_state be a more appropriate
place to check that the src width and height are at least 1? This is
where skl_plane_check and other HW's foo_plane_check functions are called
from.

I don't think that the potential divide-by-zero will be hit in the case
where intel_adjusted_rate is called from ilk_pipe_pixel_rate because the
src rect will not have a fractional part to it in this case. I'm assuming
that something earlier on would catch and reject a src with zero
width/height.

Drew

> 
> 
> ...alan 
> 
> On Mon, 2022-12-26 at 22:53 -0700, Drew Davenport wrote:
> > The error message suggests that the height of the src rect must be at
> > least 1. Reject source with height of 0.
> > 
> > Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> > 
> > ---
> > I was investigating some divide-by-zero crash reports on ChromeOS which
> > pointed to the intel_adjusted_rate function. Further prodding showed
> > that I could reproduce this in a simple test program if I made src_h
> > some value less than 1 but greater than 0.
> > 
> > This seemed to be a sensible place to check that the source height is at
> > least 1. I tried to repro this issue on an amd device I had on hand, and
> > the configuration was rejected.
> > 
> > Would it make sense to add a check that source dimensions are at least 1
> > somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> > use case on some devices, and thus any such check should be done on a
> > per-driver basis?
> > 
> > Thanks.
> > 
> >  drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 4b79c2d2d6177..9b172a1e90deb 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> >  	u32 offset;
> >  	int ret;
> >  
> > -	if (w > max_width || w < min_width || h > max_height) {
> > +	if (w > max_width || w < min_width || h > max_height || h < 1) {
> >  		drm_dbg_kms(&dev_priv->drm,
> >  			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
> >  			    w, h, min_width, max_width, max_height);
> > -- 
> > 2.39.0.314.g84b9a713c41-goog
> > 
> 

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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
@ 2023-01-11 18:47     ` Drew Davenport
  0 siblings, 0 replies; 24+ messages in thread
From: Drew Davenport @ 2023-01-11 18:47 UTC (permalink / raw)
  To: Teres Alexis, Alan Previn
  Cc: tzimmermann, intel-gfx, linux-kernel, dri-devel, Heikkila,
	Juha-pekka, Vivi, Rodrigo

On Tue, Dec 27, 2022 at 05:55:17PM +0000, Teres Alexis, Alan Previn wrote:
> Is there a better place for this check higher up the intel specific atomic-check? (so the check won't be skl specific - i notice that intel_adjusted_rate is also called by
> ilk_foo as well and non-backend-specific functions). Else, perhaps intel_adjusted_rate should add a check + WARN? (if we are trying to propagate this slowly across HW).

Would intel_plane_atomic_check_with_state be a more appropriate
place to check that the src width and height are at least 1? This is
where skl_plane_check and other HW's foo_plane_check functions are called
from.

I don't think that the potential divide-by-zero will be hit in the case
where intel_adjusted_rate is called from ilk_pipe_pixel_rate because the
src rect will not have a fractional part to it in this case. I'm assuming
that something earlier on would catch and reject a src with zero
width/height.

Drew

> 
> 
> ...alan 
> 
> On Mon, 2022-12-26 at 22:53 -0700, Drew Davenport wrote:
> > The error message suggests that the height of the src rect must be at
> > least 1. Reject source with height of 0.
> > 
> > Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> > 
> > ---
> > I was investigating some divide-by-zero crash reports on ChromeOS which
> > pointed to the intel_adjusted_rate function. Further prodding showed
> > that I could reproduce this in a simple test program if I made src_h
> > some value less than 1 but greater than 0.
> > 
> > This seemed to be a sensible place to check that the source height is at
> > least 1. I tried to repro this issue on an amd device I had on hand, and
> > the configuration was rejected.
> > 
> > Would it make sense to add a check that source dimensions are at least 1
> > somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> > use case on some devices, and thus any such check should be done on a
> > per-driver basis?
> > 
> > Thanks.
> > 
> >  drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 4b79c2d2d6177..9b172a1e90deb 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> >  	u32 offset;
> >  	int ret;
> >  
> > -	if (w > max_width || w < min_width || h > max_height) {
> > +	if (w > max_width || w < min_width || h > max_height || h < 1) {
> >  		drm_dbg_kms(&dev_priv->drm,
> >  			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
> >  			    w, h, min_width, max_width, max_height);
> > -- 
> > 2.39.0.314.g84b9a713c41-goog
> > 
> 

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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
@ 2023-01-11 18:47     ` Drew Davenport
  0 siblings, 0 replies; 24+ messages in thread
From: Drew Davenport @ 2023-01-11 18:47 UTC (permalink / raw)
  To: Teres Alexis, Alan Previn
  Cc: tzimmermann, intel-gfx, linux-kernel, dri-devel, Heikkila,
	Juha-pekka, daniel, Vivi, Rodrigo, airlied

On Tue, Dec 27, 2022 at 05:55:17PM +0000, Teres Alexis, Alan Previn wrote:
> Is there a better place for this check higher up the intel specific atomic-check? (so the check won't be skl specific - i notice that intel_adjusted_rate is also called by
> ilk_foo as well and non-backend-specific functions). Else, perhaps intel_adjusted_rate should add a check + WARN? (if we are trying to propagate this slowly across HW).

Would intel_plane_atomic_check_with_state be a more appropriate
place to check that the src width and height are at least 1? This is
where skl_plane_check and other HW's foo_plane_check functions are called
from.

I don't think that the potential divide-by-zero will be hit in the case
where intel_adjusted_rate is called from ilk_pipe_pixel_rate because the
src rect will not have a fractional part to it in this case. I'm assuming
that something earlier on would catch and reject a src with zero
width/height.

Drew

> 
> 
> ...alan 
> 
> On Mon, 2022-12-26 at 22:53 -0700, Drew Davenport wrote:
> > The error message suggests that the height of the src rect must be at
> > least 1. Reject source with height of 0.
> > 
> > Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> > 
> > ---
> > I was investigating some divide-by-zero crash reports on ChromeOS which
> > pointed to the intel_adjusted_rate function. Further prodding showed
> > that I could reproduce this in a simple test program if I made src_h
> > some value less than 1 but greater than 0.
> > 
> > This seemed to be a sensible place to check that the source height is at
> > least 1. I tried to repro this issue on an amd device I had on hand, and
> > the configuration was rejected.
> > 
> > Would it make sense to add a check that source dimensions are at least 1
> > somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> > use case on some devices, and thus any such check should be done on a
> > per-driver basis?
> > 
> > Thanks.
> > 
> >  drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 4b79c2d2d6177..9b172a1e90deb 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> >  	u32 offset;
> >  	int ret;
> >  
> > -	if (w > max_width || w < min_width || h > max_height) {
> > +	if (w > max_width || w < min_width || h > max_height || h < 1) {
> >  		drm_dbg_kms(&dev_priv->drm,
> >  			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
> >  			    w, h, min_width, max_width, max_height);
> > -- 
> > 2.39.0.314.g84b9a713c41-goog
> > 
> 

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

* Re: [PATCH] drm/i915/display: Check source height is > 0
  2022-12-27  5:53 ` Drew Davenport
  (?)
@ 2023-01-12 18:28   ` Ville Syrjälä
  -1 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2023-01-12 18:28 UTC (permalink / raw)
  To: Drew Davenport
  Cc: dri-devel, Tvrtko Ursulin, Thomas Zimmermann, intel-gfx,
	linux-kernel, José Roberto de Souza, Rodrigo Vivi,
	Juha-Pekka Heikkilä

On Mon, Dec 26, 2022 at 10:53:24PM -0700, Drew Davenport wrote:
> The error message suggests that the height of the src rect must be at
> least 1. Reject source with height of 0.
> 
> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> 
> ---
> I was investigating some divide-by-zero crash reports on ChromeOS which
> pointed to the intel_adjusted_rate function. Further prodding showed
> that I could reproduce this in a simple test program if I made src_h
> some value less than 1 but greater than 0.
> 
> This seemed to be a sensible place to check that the source height is at
> least 1. I tried to repro this issue on an amd device I had on hand, and
> the configuration was rejected.
> 
> Would it make sense to add a check that source dimensions are at least 1
> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> use case on some devices, and thus any such check should be done on a
> per-driver basis?
> 
> Thanks.
> 
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 4b79c2d2d6177..9b172a1e90deb 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>  	u32 offset;
>  	int ret;
>  
> -	if (w > max_width || w < min_width || h > max_height) {
> +	if (w > max_width || w < min_width || h > max_height || h < 1) {

I liked this one best so pushed to drm-intel-next with cc:stable. Thanks.

In the future we might want to move some of these checks to an earlier
spot to make sure we don't hit any other weird issues in some other
code, but for the moment I think this will do.

>  		drm_dbg_kms(&dev_priv->drm,
>  			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>  			    w, h, min_width, max_width, max_height);
> -- 
> 2.39.0.314.g84b9a713c41-goog

-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
@ 2023-01-12 18:28   ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2023-01-12 18:28 UTC (permalink / raw)
  To: Drew Davenport
  Cc: dri-devel, Thomas Zimmermann, intel-gfx, linux-kernel,
	Daniel Vetter, Rodrigo Vivi, David Airlie,
	Juha-Pekka Heikkilä

On Mon, Dec 26, 2022 at 10:53:24PM -0700, Drew Davenport wrote:
> The error message suggests that the height of the src rect must be at
> least 1. Reject source with height of 0.
> 
> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> 
> ---
> I was investigating some divide-by-zero crash reports on ChromeOS which
> pointed to the intel_adjusted_rate function. Further prodding showed
> that I could reproduce this in a simple test program if I made src_h
> some value less than 1 but greater than 0.
> 
> This seemed to be a sensible place to check that the source height is at
> least 1. I tried to repro this issue on an amd device I had on hand, and
> the configuration was rejected.
> 
> Would it make sense to add a check that source dimensions are at least 1
> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> use case on some devices, and thus any such check should be done on a
> per-driver basis?
> 
> Thanks.
> 
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 4b79c2d2d6177..9b172a1e90deb 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>  	u32 offset;
>  	int ret;
>  
> -	if (w > max_width || w < min_width || h > max_height) {
> +	if (w > max_width || w < min_width || h > max_height || h < 1) {

I liked this one best so pushed to drm-intel-next with cc:stable. Thanks.

In the future we might want to move some of these checks to an earlier
spot to make sure we don't hit any other weird issues in some other
code, but for the moment I think this will do.

>  		drm_dbg_kms(&dev_priv->drm,
>  			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>  			    w, h, min_width, max_width, max_height);
> -- 
> 2.39.0.314.g84b9a713c41-goog

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH] drm/i915/display: Check source height is > 0
@ 2023-01-12 18:28   ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2023-01-12 18:28 UTC (permalink / raw)
  To: Drew Davenport
  Cc: intel-gfx, Daniel Vetter, David Airlie, Imre Deak, Jani Nikula,
	Joonas Lahtinen, José Roberto de Souza,
	Juha-Pekka Heikkilä,
	Matt Roper, Rodrigo Vivi, Thomas Zimmermann, Tvrtko Ursulin,
	dri-devel, linux-kernel

On Mon, Dec 26, 2022 at 10:53:24PM -0700, Drew Davenport wrote:
> The error message suggests that the height of the src rect must be at
> least 1. Reject source with height of 0.
> 
> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
> 
> ---
> I was investigating some divide-by-zero crash reports on ChromeOS which
> pointed to the intel_adjusted_rate function. Further prodding showed
> that I could reproduce this in a simple test program if I made src_h
> some value less than 1 but greater than 0.
> 
> This seemed to be a sensible place to check that the source height is at
> least 1. I tried to repro this issue on an amd device I had on hand, and
> the configuration was rejected.
> 
> Would it make sense to add a check that source dimensions are at least 1
> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
> use case on some devices, and thus any such check should be done on a
> per-driver basis?
> 
> Thanks.
> 
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 4b79c2d2d6177..9b172a1e90deb 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>  	u32 offset;
>  	int ret;
>  
> -	if (w > max_width || w < min_width || h > max_height) {
> +	if (w > max_width || w < min_width || h > max_height || h < 1) {

I liked this one best so pushed to drm-intel-next with cc:stable. Thanks.

In the future we might want to move some of these checks to an earlier
spot to make sure we don't hit any other weird issues in some other
code, but for the moment I think this will do.

>  		drm_dbg_kms(&dev_priv->drm,
>  			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>  			    w, h, min_width, max_width, max_height);
> -- 
> 2.39.0.314.g84b9a713c41-goog

-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
  2023-01-12 18:28   ` [Intel-gfx] " Ville Syrjälä
  (?)
@ 2023-01-13 11:06     ` Juha-Pekka Heikkila
  -1 siblings, 0 replies; 24+ messages in thread
From: Juha-Pekka Heikkila @ 2023-01-13 11:06 UTC (permalink / raw)
  To: Ville Syrjälä, Drew Davenport
  Cc: Thomas Zimmermann, intel-gfx, linux-kernel, dri-devel,
	Juha-Pekka Heikkilä,
	Rodrigo Vivi

On 12.1.2023 20.28, Ville Syrjälä wrote:
> On Mon, Dec 26, 2022 at 10:53:24PM -0700, Drew Davenport wrote:
>> The error message suggests that the height of the src rect must be at
>> least 1. Reject source with height of 0.
>>
>> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
>>
>> ---
>> I was investigating some divide-by-zero crash reports on ChromeOS which
>> pointed to the intel_adjusted_rate function. Further prodding showed
>> that I could reproduce this in a simple test program if I made src_h
>> some value less than 1 but greater than 0.
>>
>> This seemed to be a sensible place to check that the source height is at
>> least 1. I tried to repro this issue on an amd device I had on hand, and
>> the configuration was rejected.
>>
>> Would it make sense to add a check that source dimensions are at least 1
>> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
>> use case on some devices, and thus any such check should be done on a
>> per-driver basis?
>>
>> Thanks.
>>
>>   drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> index 4b79c2d2d6177..9b172a1e90deb 100644
>> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>>   	u32 offset;
>>   	int ret;
>>   
>> -	if (w > max_width || w < min_width || h > max_height) {
>> +	if (w > max_width || w < min_width || h > max_height || h < 1) {
> 
> I liked this one best so pushed to drm-intel-next with cc:stable. Thanks.
> 
> In the future we might want to move some of these checks to an earlier
> spot to make sure we don't hit any other weird issues in some other
> code, but for the moment I think this will do.
> 

Look ok to me. Tests which I had written to try different ways to cause 
this issue are now returning einval as expected. I'll polish my igt test 
for this issue and send it out bit later.

/Juha-pekka

>>   		drm_dbg_kms(&dev_priv->drm,
>>   			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>>   			    w, h, min_width, max_width, max_height);
>> -- 
>> 2.39.0.314.g84b9a713c41-goog
> 


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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
@ 2023-01-13 11:06     ` Juha-Pekka Heikkila
  0 siblings, 0 replies; 24+ messages in thread
From: Juha-Pekka Heikkila @ 2023-01-13 11:06 UTC (permalink / raw)
  To: Ville Syrjälä, Drew Davenport
  Cc: Thomas Zimmermann, intel-gfx, linux-kernel, dri-devel,
	Juha-Pekka Heikkilä,
	Daniel Vetter, Rodrigo Vivi, David Airlie

On 12.1.2023 20.28, Ville Syrjälä wrote:
> On Mon, Dec 26, 2022 at 10:53:24PM -0700, Drew Davenport wrote:
>> The error message suggests that the height of the src rect must be at
>> least 1. Reject source with height of 0.
>>
>> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
>>
>> ---
>> I was investigating some divide-by-zero crash reports on ChromeOS which
>> pointed to the intel_adjusted_rate function. Further prodding showed
>> that I could reproduce this in a simple test program if I made src_h
>> some value less than 1 but greater than 0.
>>
>> This seemed to be a sensible place to check that the source height is at
>> least 1. I tried to repro this issue on an amd device I had on hand, and
>> the configuration was rejected.
>>
>> Would it make sense to add a check that source dimensions are at least 1
>> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
>> use case on some devices, and thus any such check should be done on a
>> per-driver basis?
>>
>> Thanks.
>>
>>   drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> index 4b79c2d2d6177..9b172a1e90deb 100644
>> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>>   	u32 offset;
>>   	int ret;
>>   
>> -	if (w > max_width || w < min_width || h > max_height) {
>> +	if (w > max_width || w < min_width || h > max_height || h < 1) {
> 
> I liked this one best so pushed to drm-intel-next with cc:stable. Thanks.
> 
> In the future we might want to move some of these checks to an earlier
> spot to make sure we don't hit any other weird issues in some other
> code, but for the moment I think this will do.
> 

Look ok to me. Tests which I had written to try different ways to cause 
this issue are now returning einval as expected. I'll polish my igt test 
for this issue and send it out bit later.

/Juha-pekka

>>   		drm_dbg_kms(&dev_priv->drm,
>>   			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>>   			    w, h, min_width, max_width, max_height);
>> -- 
>> 2.39.0.314.g84b9a713c41-goog
> 


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

* Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0
@ 2023-01-13 11:06     ` Juha-Pekka Heikkila
  0 siblings, 0 replies; 24+ messages in thread
From: Juha-Pekka Heikkila @ 2023-01-13 11:06 UTC (permalink / raw)
  To: Ville Syrjälä, Drew Davenport
  Cc: dri-devel, Thomas Zimmermann, intel-gfx, linux-kernel,
	Daniel Vetter, Rodrigo Vivi, David Airlie,
	Juha-Pekka Heikkilä

On 12.1.2023 20.28, Ville Syrjälä wrote:
> On Mon, Dec 26, 2022 at 10:53:24PM -0700, Drew Davenport wrote:
>> The error message suggests that the height of the src rect must be at
>> least 1. Reject source with height of 0.
>>
>> Signed-off-by: Drew Davenport <ddavenport@chromium.org>
>>
>> ---
>> I was investigating some divide-by-zero crash reports on ChromeOS which
>> pointed to the intel_adjusted_rate function. Further prodding showed
>> that I could reproduce this in a simple test program if I made src_h
>> some value less than 1 but greater than 0.
>>
>> This seemed to be a sensible place to check that the source height is at
>> least 1. I tried to repro this issue on an amd device I had on hand, and
>> the configuration was rejected.
>>
>> Would it make sense to add a check that source dimensions are at least 1
>> somewhere in core, like in drm_atomic_plane_check? Or is that a valid
>> use case on some devices, and thus any such check should be done on a
>> per-driver basis?
>>
>> Thanks.
>>
>>   drivers/gpu/drm/i915/display/skl_universal_plane.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> index 4b79c2d2d6177..9b172a1e90deb 100644
>> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> @@ -1627,7 +1627,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>>   	u32 offset;
>>   	int ret;
>>   
>> -	if (w > max_width || w < min_width || h > max_height) {
>> +	if (w > max_width || w < min_width || h > max_height || h < 1) {
> 
> I liked this one best so pushed to drm-intel-next with cc:stable. Thanks.
> 
> In the future we might want to move some of these checks to an earlier
> spot to make sure we don't hit any other weird issues in some other
> code, but for the moment I think this will do.
> 

Look ok to me. Tests which I had written to try different ways to cause 
this issue are now returning einval as expected. I'll polish my igt test 
for this issue and send it out bit later.

/Juha-pekka

>>   		drm_dbg_kms(&dev_priv->drm,
>>   			    "requested Y/RGB source size %dx%d outside limits (min: %dx1 max: %dx%d)\n",
>>   			    w, h, min_width, max_width, max_height);
>> -- 
>> 2.39.0.314.g84b9a713c41-goog
> 


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

end of thread, other threads:[~2023-01-13 11:13 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-27  5:53 [PATCH] drm/i915/display: Check source height is > 0 Drew Davenport
2022-12-27  5:53 ` [Intel-gfx] " Drew Davenport
2022-12-27  5:53 ` Drew Davenport
2022-12-27 15:42 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2022-12-27 17:55 ` [Intel-gfx] [PATCH] " Teres Alexis, Alan Previn
2022-12-27 17:55   ` Teres Alexis, Alan Previn
2022-12-27 17:55   ` Teres Alexis, Alan Previn
2023-01-11 18:47   ` Drew Davenport
2023-01-11 18:47     ` Drew Davenport
2023-01-11 18:47     ` Drew Davenport
2022-12-27 19:34 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork
2023-01-03 10:42 ` [PATCH] " Juha-Pekka Heikkila
2023-01-03 10:42   ` [Intel-gfx] " Juha-Pekka Heikkila
2023-01-03 10:42   ` Juha-Pekka Heikkila
2023-01-10 20:30   ` Drew Davenport
2023-01-10 20:30     ` Drew Davenport
2023-01-10 20:30     ` [Intel-gfx] " Drew Davenport
2023-01-03 10:56 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/display: Check source height is > 0 (rev2) Patchwork
2023-01-12 18:28 ` [PATCH] drm/i915/display: Check source height is > 0 Ville Syrjälä
2023-01-12 18:28   ` Ville Syrjälä
2023-01-12 18:28   ` [Intel-gfx] " Ville Syrjälä
2023-01-13 11:06   ` Juha-Pekka Heikkila
2023-01-13 11:06     ` Juha-Pekka Heikkila
2023-01-13 11:06     ` Juha-Pekka Heikkila

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.