<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <p style="font-family:Arial;font-size:10pt;color:#0000FF;margin:5pt;" align="Left"> [AMD Official Use Only - General]<br> </p> <br> <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof"> Thank you very much Harry and Hamza.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof ContentPasted0 ContentPasted1 ContentPasted2 ContentPasted3 ContentPasted4"> You are right, adev->mode_info.num_crtc has already been updated, the variables that really need updating are</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof ContentPasted0 ContentPasted1 ContentPasted2 ContentPasted3 ContentPasted4"> adev->mode_info.num_crtc, adev->vline0_irq.num_types, adev->vupdate_irq.num_types and adev->pageflip_irq.num_types.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof ContentPasted0 ContentPasted1 ContentPasted2 ContentPasted3 ContentPasted4"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof ContentPasted0 ContentPasted1 ContentPasted2 ContentPasted3 ContentPasted4 ContentPasted5"> I have made a new patch with title "drm/amd/display: fix dm irq error message in gpu recover", please help to review.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof ContentPasted0 ContentPasted1 ContentPasted2 ContentPasted3 ContentPasted4 ContentPasted5"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof ContentPasted0 ContentPasted1 ContentPasted2 ContentPasted3 ContentPasted4 ContentPasted5"> Regards,</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof ContentPasted0 ContentPasted1 ContentPasted2 ContentPasted3 ContentPasted4 ContentPasted5"> Rico</div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Wentland, Harry <Harry.Wentland@amd.com><br> <b>Sent:</b> Wednesday, February 8, 2023 0:19<br> <b>To:</b> Mahfooz, Hamza <Hamza.Mahfooz@amd.com>; Yin, Tianci (Rico) <Tianci.Yin@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br> <b>Cc:</b> Wang, Yu (Charlie) <Yu.Wang4@amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; Pillai, Aurabindo <Aurabindo.Pillai@amd.com><br> <b>Subject:</b> Re: [PATCH] drm/amd/display: Align num_crtc to max_streams</font> <div> </div> </div> <div class="BodyFragment"><font size="2"><span style="font-size:11pt;"> <div class="PlainText"><br> <br> On 2/7/23 09:41, Hamza Mahfooz wrote:<br> > <br> > On 2/7/23 09:31, Harry Wentland wrote:<br> >><br> >><br> >> On 2/7/23 08:00, Hamza Mahfooz wrote:<br> >>><br> >>> On 2/6/23 23:05, Tianci Yin wrote:<br> >>>> From: tiancyin <tianci.yin@amd.com><br> >>>><br> >>>> [Why]<br> >>>> Display pipe might be harvested on some SKUs, that cause the<br> >>>> adev->mode_info.num_crtc mismatch with the usable crtc number,<br> >>>> then below error dmesgs observed after GPU recover.<br> >>>><br> >>>> *ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3<br> >>>> *ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3<br> >>>> *ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3<br> >>>> *ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3<br> >>>> *ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3<br> >>>> *ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3<br> >>>> *ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3<br> >>>> *ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3<br> >>>> *ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3<br> >>>> *ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3<br> >>>><br> >>>> [How]<br> >>>> The max_streams is limited number after pipe fuse, align num_crtc<br> >>>> to max_streams to eliminate the error logs.<br> >>>><br> >>>> Signed-off-by: tiancyin <tianci.yin@amd.com><br> >>>> ---<br> >>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++<br> >>>> 1 file changed, 3 insertions(+)<br> >>>><br> >>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br> >>>> index b31cfda30ff9..87ec2574cc09 100644<br> >>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br> >>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c<br> >>>> @@ -4285,6 +4285,9 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)<br> >>>> break;<br> >>>> }<br> >>>> + /* Adjust the crtc number according to the DCN pipe fuse. */<br> >>>> + adev->mode_info.num_crtc = dm->dc->caps.max_streams;<br> >>><br> >>> This would introduce array-out-bounds issues, since there are arrays of<br> >>> size AMDGPU_MAX_CRTCS that use num_crtc as a bounds check.<br> >><br> >> From what I can tell max_streams is always <= AMDGPU_MAX_CRTCS (6),<br> >> though we're not checking. Maybe it'd be good to check and print a<br> >> DRM_ERROR here if that's ever not the case (e.g., if we ever add<br> >> any new ASIC that has more streams).<br> > <br> > I have had UBSAN warns before commit d633b7a25fbe ("drm/amd/display: fix<br> > possible buffer overflow relating to secure display") was applied, so it<br> > seems to already be the case, maybe due to virtual streams.<br> > <br> <br> Interesting.<br> <br> On closer look I'm not sure why this patch is needed. We already<br> do exactly what this patch does at the beginning of<br> amdgpu_dm_initialize_drm_device:<br> <br> > dm->display_indexes_num = dm->dc->caps.max_streams;<br> > /* Update the actual used number of crtc */<br> > adev->mode_info.num_crtc = adev->dm.display_indexes_num;<br> <br> Harry<br> <br> >><br> >> Harry<br> >><br> >>><br> >>>> +<br> >>>> for (i = 0; i < dm->dc->caps.max_streams; i++)<br> >>>> if (amdgpu_dm_crtc_init(dm, mode_info->planes[i], i)) {<br> >>>> DRM_ERROR("KMS: Failed to initialize crtc\n");<br> >>><br> >><br> > <br> <br> </div> </span></font></div> </div> </body> </html>