[SRU][OEM-5.14/J][PATCH 3/4] drm/amd/display: Fix new dmub notification enabling in DM

[Stefan Bader]

On 01.08.22 15:21, Mario Limonciello wrote:
> On 8/1/22 07:48, Stefan Bader wrote:
>> On 29.07.22 20:57, Mario Limonciello wrote:
>>> From: Stylon Wang <stylon.wang at amd.com>
>>>
>>> BugLink: https://bugs.launchpad.net/bugs/1983143
>>>
>>> [Why]
>>> Changes from "Fix for dmub outbox notification enable" need to land
>>> in DM or DMUB outbox notification would be disabled.
>>>
>>> [How]
>>> Enable outbox notification only after interrupt are enabled and IRQ
>>> handlers registered. Any pending notification will be sent by DMUB
>>> once outbox notification is enabled.
>>>
>>> Fixes: ed7208706448 ("drm/amd/display: Fix for dmub outbox notification enable")
>>> Reviewed-by: Nicholas Kazlauskas <Nicholas.Kazlauskas at amd.com>
>>> Acked-by: Solomon Chiu <solomon.chiu at amd.com>
>>> Signed-off-by: Stylon Wang <stylon.wang at amd.com>
>>> Acked-by: Harry Wentland <harry.wentland at amd.com>
>>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>>> Cc: stable at vger.kernel.org
>>> (cherry picked from commit 2d4bd81fea1ad6ebba543bd6da3ef5179d130e6a; minor 
>>> fixups for missing 32685b32d825c and 6d63fcc2a334f)
>>
>> Looking closer into that I believe this would only be a backport xxx
>> [minor context adjustments]
>>
>> if it were complete. But it seems to have dropped hunk #3 of the original 
>> patch (details below)
> 
> Right - so I guess you can potentially have 
> 6eff272dbee7ad444c491c9a96d49e78e91e2161 in the series too, but since this went 
> out of order due to the stable commit that might not really make sense.
> 
> LMK what you would like done (if anything).

Right now it is hard to tell whether dropping that hunk is intentional or not. 
It was added in that patch together with the hunk about re-enabling things which 
you do drop. So compared to the upstream patch the backport appears unbalanced.

> 
> I would do a fixup to @stable for you to just pickup, but this use case for USB4 
> DP tunneling only matters for Jammy-5.15 not 5.15.y since Jammy-5.15 has all the 
> USB4 commits for DPIA handling and 5.15.y doesn't.

Hm, that is probably what you are saying here. That together with all the other 
backports we have that hunk should remain where it is and just the other 
addition needs to be moved?

-Stefan
> 
>>
>>> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
>>> ---
>>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 +++++++++++++++----
>>>   1 file changed, 19 insertions(+), 5 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 785db0708c95..25357128122f 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -1443,7 +1443,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>>>   #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
>>>       adev->dm.crc_rd_wrk = amdgpu_dm_crtc_secure_display_create_work();
>>>   #endif
>>> -    if (dc_enable_dmub_notifications(adev->dm.dc)) {
>>> +    if (dc_is_dmub_outbox_supported(adev->dm.dc)) {
>>>           init_completion(&adev->dm.dmub_aux_transfer_done);
>>>           adev->dm.dmub_notify = kzalloc(sizeof(struct dmub_notification), 
>>> GFP_KERNEL);
>>>           if (!adev->dm.dmub_notify) {
>>> @@ -1481,6 +1481,13 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>>>           goto error;
>>>       }
>>> +    /* Enable outbox notification only after IRQ handlers are registered and 
>>> DMUB is alive.
>>> +     * It is expected that DMUB will resend any pending notifications at 
>>> this point, for
>>> +     * example HPD from DPIA.
>>> +     */
>>> +    if (dc_is_dmub_outbox_supported(adev->dm.dc))
>>> +        dc_enable_dmub_outbox(adev->dm.dc);
>>> +
>>>       /* create fake encoders for MST */
>>>       dm_dp_create_fake_mst_encoders(adev);
>>>
>>
>> Original patch has:
>> @ -2678,9 +2685,6 @@ static int dm_resume(void *handle)
>>                   */
>>                  link_enc_cfg_copy(adev->dm.dc->current_state, dc_state);
>>
>> -               if (dc_enable_dmub_notifications(adev->dm.dc))
>> -                       amdgpu_dm_outbox_init(adev);
>> -
>>                  r = dm_dmub_hw_init(adev);
>>                  if (r)
>>                          DRM_ERROR("DMUB interface failed to initialize: 
>> status=%d\n", r);
>>
>> and at least in Jammy there is:
>>
>>          if (amdgpu_in_reset(adev)) {
>>                    dc_state = dm->cached_dc_state;
>>
>>                    if (dc_enable_dmub_notifications(adev->dm.dc))
>>                            amdgpu_dm_outbox_init(adev);
>>
>>                    r = dm_dmub_hw_init(adev);
>>                    if (r)
>>                            DRM_ERROR("DMUB interface failed to initialize: 
>> status=%d\n", r);
>>
>> which got there via stable:
>>
>> "drm/amd/display: Fix DPIA outbox timeout after S3/S4/reset"
>> commit af6902ec415655236adea91826bd96ed0ab16f42 upstream.
>>
>> -Stefan
>>
>>> @@ -2455,6 +2462,11 @@ static int dm_resume(void *handle)
>>>           WARN_ON(dc_validate_global_state(dm->dc, dc_state, false) != DC_OK);
>>>   #endif
>>> +        if (dc_is_dmub_outbox_supported(adev->dm.dc)) {
>>> +            amdgpu_dm_outbox_init(adev);
>>> +            dc_enable_dmub_outbox(adev->dm.dc);
>>> +        }
>>> +
>>>           WARN_ON(!dc_commit_state(dm->dc, dc_state));
>>>           dm_gpureset_commit_state(dm->cached_dc_state, dm);
>>> @@ -2476,13 +2488,15 @@ static int dm_resume(void *handle)
>>>       /* TODO: Remove dc_state->dccg, use dc->dccg directly. */
>>>       dc_resource_state_construct(dm->dc, dm_state->context);
>>> -    /* Re-enable outbox interrupts for DPIA. */
>>> -    if (dc_enable_dmub_notifications(adev->dm.dc))
>>> -        amdgpu_dm_outbox_init(adev);
>>> -
>>>       /* Before powering on DC we need to re-initialize DMUB. */
>>>       dm_dmub_hw_resume(adev);
>>> +    /* Re-enable outbox interrupts for DPIA. */
>>> +    if (dc_is_dmub_outbox_supported(adev->dm.dc)) {
>>> +        amdgpu_dm_outbox_init(adev);
>>> +        dc_enable_dmub_outbox(adev->dm.dc);
>>> +    }
>>> +
>>>       /* power on hardware */
>>>       dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
>>
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20220801/e09c79ca/attachment.sig>