| Message ID | 20230913-fix-wuninitialized-dm_helpers_dp_mst_send_payload_allocation-v1-1-2d1b0a3ef16c@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/display: Fix -Wuninitialized in dm_helpers_dp_mst_send_payload_allocation() | expand |
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@kernel.org> Reviewed-by: Hamza Mahfooz <hamza.mahfooz@amd.com> Hm, seems like this was pushed through drm-misc-next and as such our CI didn't get a chance to test it. > --- > 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,
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@kernel.org> Applied, thanks! > --- > 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,
On Wed, Sep 13, 2023 at 12:17 PM Hamza Mahfooz <hamza.mahfooz@amd.com> 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@kernel.org> > > Reviewed-by: Hamza Mahfooz <hamza.mahfooz@amd.com> > > 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. 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 >
On 9/13/23 15:54, Alex Deucher wrote: > On Wed, Sep 13, 2023 at 12:17 PM Hamza Mahfooz <hamza.mahfooz@amd.com> 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@kernel.org> >> >> Reviewed-by: Hamza Mahfooz <hamza.mahfooz@amd.com> >> >> 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. > > 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 >>
On Wed, Sep 13, 2023 at 3:57 PM Hamza Mahfooz <hamza.mahfooz@amd.com> wrote: > > > On 9/13/23 15:54, Alex Deucher wrote: > > On Wed, Sep 13, 2023 at 12:17 PM Hamza Mahfooz <hamza.mahfooz@amd.com> 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@kernel.org> > >> > >> Reviewed-by: Hamza Mahfooz <hamza.mahfooz@amd.com> > >> > >> 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. 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 >
On 9/13/23 16:03, Alex Deucher wrote: > On Wed, Sep 13, 2023 at 3:57 PM Hamza Mahfooz <hamza.mahfooz@amd.com> wrote: >> >> >> On 9/13/23 15:54, Alex Deucher wrote: >>> On Wed, Sep 13, 2023 at 12:17 PM Hamza Mahfooz <hamza.mahfooz@amd.com> 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@kernel.org> >>>> >>>> Reviewed-by: Hamza Mahfooz <hamza.mahfooz@amd.com> >>>> >>>> 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? > > 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 >>
On Wed, Sep 13, 2023 at 4:46 PM Hamza Mahfooz <hamza.mahfooz@amd.com> wrote: > > On 9/13/23 16:03, Alex Deucher wrote: > > On Wed, Sep 13, 2023 at 3:57 PM Hamza Mahfooz <hamza.mahfooz@amd.com> wrote: > >> > >> > >> On 9/13/23 15:54, Alex Deucher wrote: > >>> On Wed, Sep 13, 2023 at 12:17 PM Hamza Mahfooz <hamza.mahfooz@amd.com> 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@kernel.org> > >>>> > >>>> Reviewed-by: Hamza Mahfooz <hamza.mahfooz@amd.com> > >>>> > >>>> 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 >
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) {
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@kernel.org> --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) --- base-commit: 8569c31545385195bdb0c021124e68336e91c693 change-id: 20230913-fix-wuninitialized-dm_helpers_dp_mst_send_payload_allocation-c37b33aaad18 Best regards,