On Wed, Sep 13, 2023 at 4:46 PM Hamza Mahfooz <hamza.mahfooz@xxxxxxx> wrote: > > On 9/13/23 16:03, Alex Deucher wrote: > > On Wed, Sep 13, 2023 at 3:57 PM Hamza Mahfooz <hamza.mahfooz@xxxxxxx> wrote: > >> > >> > >> On 9/13/23 15:54, Alex Deucher wrote: > >>> On Wed, Sep 13, 2023 at 12:17 PM Hamza Mahfooz <hamza.mahfooz@xxxxxxx> wrote: > >>>> > >>>> > >>>> On 9/13/23 12:10, Nathan Chancellor wrote: > >>>>> When building with clang, there is a warning (or error when > >>>>> CONFIG_WERROR is set): > >>>>> > >>>>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:368:21: error: variable 'old_payload' is uninitialized when used here [-Werror,-Wuninitialized] > >>>>> 368 | new_payload, old_payload); > >>>>> | ^~~~~~~~~~~ > >>>>> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:344:61: note: initialize the variable 'old_payload' to silence this warning > >>>>> 344 | struct drm_dp_mst_atomic_payload *new_payload, *old_payload; > >>>>> | ^ > >>>>> | = NULL > >>>>> 1 error generated. > >>>>> > >>>>> This variable is not required outside of this function so allocate > >>>>> old_payload on the stack and pass it by reference to > >>>>> dm_helpers_construct_old_payload(), resolving the warning. > >>>>> > >>>>> Closes: https://github.com/ClangBuiltLinux/linux/issues/1931 > >>>>> Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement") > >>>>> Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx> > >>>> > >>>> Reviewed-by: Hamza Mahfooz <hamza.mahfooz@xxxxxxx> > >>>> > >>>> Hm, seems like this was pushed through drm-misc-next and as such our CI > >>>> didn't get a chance to test it. > >>> > >>> Can you push this to drm-misc? That is where Wayne's patches landed. > >>> If not, I can push it. > >> > >> No need, I cherry-picked Wayne's patches to amd-staging-drm-next and > >> applied Nathan's fix. > > > > Yes, but we can only have patches go upstream via one tree. Wayne's > > patches are in drm-misc, so that is where the fix should be. It's > > fine to have them in amd-staging-drm-next for testing purposes, but I > > won't be upstreaming them via the amdgpu -next tree since they are > > already in drm-misc. > > In that case can you push it to drm-misc? Pushed. Thanks! Alex Alex > > > > > Alex > > > >> > >>> > >>> Alex > >>> > >>> > >>>> > >>>> > >>>>> --- > >>>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 +++--- > >>>>> 1 file changed, 3 insertions(+), 3 deletions(-) > >>>>> > >>>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > >>>>> index 9ad509279b0a..c4c35f6844f4 100644 > >>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > >>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > >>>>> @@ -341,7 +341,7 @@ bool dm_helpers_dp_mst_send_payload_allocation( > >>>>> struct amdgpu_dm_connector *aconnector; > >>>>> struct drm_dp_mst_topology_state *mst_state; > >>>>> struct drm_dp_mst_topology_mgr *mst_mgr; > >>>>> - struct drm_dp_mst_atomic_payload *new_payload, *old_payload; > >>>>> + struct drm_dp_mst_atomic_payload *new_payload, old_payload; > >>>>> enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD; > >>>>> enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD; > >>>>> int ret = 0; > >>>>> @@ -365,8 +365,8 @@ bool dm_helpers_dp_mst_send_payload_allocation( > >>>>> ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, new_payload); > >>>>> } else { > >>>>> dm_helpers_construct_old_payload(stream->link, mst_state->pbn_div, > >>>>> - new_payload, old_payload); > >>>>> - drm_dp_remove_payload_part2(mst_mgr, mst_state, old_payload, new_payload); > >>>>> + new_payload, &old_payload); > >>>>> + drm_dp_remove_payload_part2(mst_mgr, mst_state, &old_payload, new_payload); > >>>>> } > >>>>> > >>>>> if (ret) { > >>>>> > >>>>> --- > >>>>> base-commit: 8569c31545385195bdb0c021124e68336e91c693 > >>>>> change-id: 20230913-fix-wuninitialized-dm_helpers_dp_mst_send_payload_allocation-c37b33aaad18 > >>>>> > >>>>> Best regards, > >>>> -- > >>>> Hamza > >>>> > >> -- > >> Hamza > >> > -- > Hamza >
