Re: [PATCH] drm/amd/display: Make underlay rules less strict

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-05-07 10:37 a.m., Rodrigo Siqueira wrote:
Currently, we reject all conditions where the underlay plane goes
outside the overlay plane limits, which is not entirely correct since we
reject some valid cases like the ones illustrated below:

   +--------------------+  +--------------------+
   |   Overlay plane    |  |   Overlay plane    |
   |                    |  |        +-----------|--+
   | +--------------+   |  |        |           |  |
   | |              |   |  |        |           |  |
   +--------------------+  +--------------------+  |
     | Primary plane|               +--------------+
     |  (underlay)  |
     +--------------+
   +-+--------------+---+  +--------------------+
   |    Overlay plane   |  |    Overlay plane   |
+-|------------+       |  |       +--------------+
| |            |       |  |       |            | |
| |            |       |  |       |            | |
| |            |       |  |       |            | |
+-|------------+       |  |       +--------------+
   +--------------------+  +--------------------+

This patch fixes this issue by only rejecting commit requests where the
underlay is entirely outside the overlay limits. After applying this
patch, a set of subtests related to kms_plane, kms_plane_alpha_blend,
and kms_plane_scaling will pass.

Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx>

What's the size of the overlay plane in your examples? If the overlay plane does not cover the entire screen then this patch is incorrect.

We don't want to be enabling the cursor on multiple pipes and the checks in DC to allow disabling cursor on bottom pipes only work if the underlay is entirely contained within the overlay.

In the case where the primary (underlay) plane extends beyond the screen boundaries it should be preclipped by userspace or earlier in the DM code before this check.

Feel free to follow up with clarification, but for now this patch is a NAK from me.

Regards,
Nicholas Kazlauskas

---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index cc048c348a92..15006aafc630 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10098,10 +10098,10 @@ static int validate_overlay(struct drm_atomic_state *state)
  		return 0;
/* Perform the bounds check to ensure the overlay plane covers the primary */
-	if (primary_state->crtc_x < overlay_state->crtc_x ||
-	    primary_state->crtc_y < overlay_state->crtc_y ||
-	    primary_state->crtc_x + primary_state->crtc_w > overlay_state->crtc_x + overlay_state->crtc_w ||
-	    primary_state->crtc_y + primary_state->crtc_h > overlay_state->crtc_y + overlay_state->crtc_h) {
+	if (primary_state->crtc_x + primary_state->crtc_w < overlay_state->crtc_x ||
+	    primary_state->crtc_x > overlay_state->crtc_x + overlay_state->crtc_w ||
+	    primary_state->crtc_y > overlay_state->crtc_y + overlay_state->crtc_h ||
+	    primary_state->crtc_y + primary_state->crtc_h < overlay_state->crtc_y) {
  		DRM_DEBUG_ATOMIC("Overlay plane is enabled with hardware cursor but does not fully cover primary plane\n");
  		return -EINVAL;
  	}


_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux