Re: [PATCH] SWDEV-476969 - dm/amdgpu: Fail dm_atomic_check if cursor overlay is required at MAX_SURFACES

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

 






On 19/11/2024 14:04, Mohamed, Zaeem wrote:
[AMD Official Use Only - AMD Internal Distribution Only]

Hi Melissa,

Could you share your configuration that experienced this pagefault?
Hi Zaeem,

Page fault was experienced by one of the reporter here:
https://gitlab.freedesktop.org/drm/amd/-/issues/3693#note_2659173
^ You can find all details (dtn_log and drm_info) in this issue from that comment.

As I mentioned, I don't have the proper hardware and environment to reproduce their issues.

Thanks,

Melissa

Thanks,
Zaeem

-----Original Message-----
From: Melissa Wen <mwen@xxxxxxxxxx>
Sent: Monday, November 18, 2024 7:52 AM
To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Mohamed, Zaeem <Zaeem.Mohamed@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH] SWDEV-476969 - dm/amdgpu: Fail dm_atomic_check if cursor overlay is required at MAX_SURFACES




On 14/11/2024 16:04, Mario Limonciello wrote:
Although it's really useful information for AMD people, the Jira
shouldn't be in the "title" of the commit message.

"If" we want to get into the habit of including this information for
display code we should come up with a prescriptive field that goes
into the commit message during promotion and it should be part of all
patches in the promotion that have it.

Something like this:

AMD-Jira: SWDEV-476969

Probably need to align that with other stakeholders though before
starting that way.

On 11/14/2024 08:37, Zaeem Mohamed wrote:
[why]
Prevent index-out-of-bounds due to requiring cursor overlay when
plane_count is MAX_SURFACES.

[how]
Bounds check on plane_count when requiring overlay cursor.

Any link to failing bugs or anything like that you can include?
Hi Mario,

About this question, these are the related issues:
- Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3693
- Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3594

You can find more details in previous iterations related to this bug:
-
https://lore.kernel.org/amd-gfx/575d66c7-e77d-42ea-acbf-412d6e508a0b@xxxxxxxxxx/
- https://lore.kernel.org/amd-gfx/20240925154324.348774-1-mwen@xxxxxxxxxx/
Co-developed-by: Melissa Wen <mwen@xxxxxxxxxx>
Signed-off-by: Zaeem Mohamed <zaeem.mohamed@xxxxxxx>
You're missing Melisaa's SoB for a co-developed patch.
IIRC this should fail checkpatch.
I already mentioned before, I don't think I actually co-dev this code, so Zaeem can remove it in the next iteration.

---

BTW, about the implementation:

As I don't have the proper environment, I asked reporters to check this patch up and it doesn't help prevent interface freeze.
It seems to prevent the out of bounds bug but is causing a page fault now:

kernel: BUG: unable to handle page fault for address: 000000000174e354

  From their feedback, I found curious that MAX_SURFACES -> 4 prevents the freeze but not completely solve the problem.
MAX_SURFACES -> 6 solves it, what let me consider that the MAX_SURFACES vs MAX_SURFACE_NUM vs MAX_PLANE mismatch might be related.
I'm going to analyzing the logs but you can find more details here:
https://gitlab.freedesktop.org/drm/amd/-/issues/3693#note_2658994

BR,

Melissa Wen
---
   amdgpu_dm/amdgpu_dm.c | 10 +++++++++-
   1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/amdgpu_dm/amdgpu_dm.c b/amdgpu_dm/amdgpu_dm.c index
97e0a1bbba..964497c613 100644
--- a/amdgpu_dm/amdgpu_dm.c
+++ b/amdgpu_dm/amdgpu_dm.c
@@ -11821,8 +11821,16 @@ static int amdgpu_dm_atomic_check(struct
drm_device *dev,
             /* Overlay cusor not subject to native cursor
restrictions */
           dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-        if (dm_new_crtc_state->cursor_mode ==
DM_CURSOR_OVERLAY_MODE)
+        if (dm_new_crtc_state->cursor_mode ==
+DM_CURSOR_OVERLAY_MODE) {
+            if (dc->current_state->stream_status->plane_count >
MAX_SURFACES) {
+                drm_dbg_driver(crtc->dev,
+                       "Can't enable cursor plane with %d planes\n",
MAX_SURFACES);
+                ret = -EINVAL;
+                goto fail;
+            }
+
               continue;
+        }
             /* Check if rotation or scaling is enabled on DCN401 */
           if ((drm_plane_mask(crtc->cursor) &
new_crtc_state->plane_mask) &&




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

  Powered by Linux