* [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of drm_dp_add_payload_part2()
@ 2022-10-04 20:24 Lyude Paul
2022-10-04 20:46 ` Rodrigo Siqueira Jordao
0 siblings, 1 reply; 5+ messages in thread
From: Lyude Paul @ 2022-10-04 20:24 UTC (permalink / raw)
To: dri-devel, amd-gfx
Cc: Ian Chen, Leo Li, Pan, Xinhui, Rodrigo Siqueira, open list,
Jani Nikula, Fangzhi Zuo, Claudio Suarez, Hamza Mahfooz,
Wayne Lin, Alex Deucher, Mikita Lipski, Christian König,
Colin Ian King
Yikes, it appears somehow I totally made a mistake here. We're currently
checking to see if drm_dp_add_payload_part2() returns a non-zero value to
indicate success. That's totally wrong though, as this function only
returns a zero value on success - not the other way around.
So, fix that.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the atomic state")
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 b8077fcd4651..00598def5b39 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
@@ -297,7 +297,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
clr_flag = MST_ALLOCATE_NEW_PAYLOAD;
}
- if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload)) {
+ if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload) == 0) {
amdgpu_dm_set_mst_status(&aconnector->mst_status,
set_flag, false);
} else {
--
2.37.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of drm_dp_add_payload_part2()
2022-10-04 20:24 [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of drm_dp_add_payload_part2() Lyude Paul
@ 2022-10-04 20:46 ` Rodrigo Siqueira Jordao
2022-10-05 19:37 ` Lyude Paul
0 siblings, 1 reply; 5+ messages in thread
From: Rodrigo Siqueira Jordao @ 2022-10-04 20:46 UTC (permalink / raw)
To: Lyude Paul, dri-devel, amd-gfx
Cc: Ian Chen, Leo Li, Pan, Xinhui, open list, Jani Nikula,
Fangzhi Zuo, Claudio Suarez, Hamza Mahfooz, Wayne Lin,
Alex Deucher, Mikita Lipski, Christian König,
Colin Ian King
On 2022-10-04 16:24, Lyude Paul wrote:
> Yikes, it appears somehow I totally made a mistake here. We're currently
> checking to see if drm_dp_add_payload_part2() returns a non-zero value to
> indicate success. That's totally wrong though, as this function only
> returns a zero value on success - not the other way around.
>
> So, fix that.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
> Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the atomic state")
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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 b8077fcd4651..00598def5b39 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
> @@ -297,7 +297,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
> clr_flag = MST_ALLOCATE_NEW_PAYLOAD;
> }
>
> - if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload)) {
> + if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload) == 0) {
> amdgpu_dm_set_mst_status(&aconnector->mst_status,
> set_flag, false);
> } else {
Hi Lyude,
Maybe I'm missing something, but I can't find the
drm_dp_add_payload_part2() function on amd-staging-drm-next. Which repo
are you using?
Thanks
Siqueira
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of drm_dp_add_payload_part2()
2022-10-04 20:46 ` Rodrigo Siqueira Jordao
@ 2022-10-05 19:37 ` Lyude Paul
2022-10-17 3:09 ` Lin, Wayne
0 siblings, 1 reply; 5+ messages in thread
From: Lyude Paul @ 2022-10-05 19:37 UTC (permalink / raw)
To: Rodrigo Siqueira Jordao, dri-devel, amd-gfx
Cc: Ian Chen, Leo Li, Pan, Xinhui, open list, Jani Nikula,
Fangzhi Zuo, Claudio Suarez, Hamza Mahfooz, Wayne Lin,
Alex Deucher, Mikita Lipski, Christian König,
Colin Ian King
On Tue, 2022-10-04 at 16:46 -0400, Rodrigo Siqueira Jordao wrote:
>
> On 2022-10-04 16:24, Lyude Paul wrote:
> > Yikes, it appears somehow I totally made a mistake here. We're currently
> > checking to see if drm_dp_add_payload_part2() returns a non-zero value to
> > indicate success. That's totally wrong though, as this function only
> > returns a zero value on success - not the other way around.
> >
> > So, fix that.
> >
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > Issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
> > Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the atomic state")
> > ---
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > 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 b8077fcd4651..00598def5b39 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
> > @@ -297,7 +297,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
> > clr_flag = MST_ALLOCATE_NEW_PAYLOAD;
> > }
> >
> > - if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload)) {
> > + if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload) == 0) {
> > amdgpu_dm_set_mst_status(&aconnector->mst_status,
> > set_flag, false);
> > } else {
>
> Hi Lyude,
>
> Maybe I'm missing something, but I can't find the
> drm_dp_add_payload_part2() function on amd-staging-drm-next. Which repo
> are you using?
If it's not on amd-staging-drm-next then it likely hasn't gotten backported to
amd's branch yet and is in drm-misc-next
>
> Thanks
> Siqueira
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of drm_dp_add_payload_part2()
2022-10-05 19:37 ` Lyude Paul
@ 2022-10-17 3:09 ` Lin, Wayne
2022-10-19 21:32 ` Lyude Paul
0 siblings, 1 reply; 5+ messages in thread
From: Lin, Wayne @ 2022-10-17 3:09 UTC (permalink / raw)
To: Lyude Paul, Siqueira, Rodrigo, dri-devel, amd-gfx
Cc: Chen, Ian, Li, Sun peng (Leo),
Pan, Xinhui, open list, Jani Nikula, Zuo, Jerry, Claudio Suarez,
Mahfooz, Hamza, Deucher, Alexander, Mikita Lipski, Koenig,
Christian, Colin Ian King
[Public]
> -----Original Message-----
> From: Lyude Paul <lyude@redhat.com>
> Sent: Thursday, October 6, 2022 3:37 AM
> To: Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; dri-
> devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: Wentland, Harry <Harry.Wentland@amd.com>; Li, Sun peng (Leo)
> <Sunpeng.Li@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; David
> Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Zuo, Jerry
> <Jerry.Zuo@amd.com>; Lin, Wayne <Wayne.Lin@amd.com>; Chen, Ian
> <Ian.Chen@amd.com>; Mikita Lipski <mikita.lipski@amd.com>; Mahfooz,
> Hamza <Hamza.Mahfooz@amd.com>; Claudio Suarez <cssk@net-c.es>; Colin
> Ian King <colin.i.king@gmail.com>; Jani Nikula <jani.nikula@intel.com>; open
> list <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of
> drm_dp_add_payload_part2()
>
> On Tue, 2022-10-04 at 16:46 -0400, Rodrigo Siqueira Jordao wrote:
> >
> > On 2022-10-04 16:24, Lyude Paul wrote:
> > > Yikes, it appears somehow I totally made a mistake here. We're
> > > currently checking to see if drm_dp_add_payload_part2() returns a
> > > non-zero value to indicate success. That's totally wrong though, as
> > > this function only returns a zero value on success - not the other way
> around.
> > >
> > > So, fix that.
> > >
> > > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > > Issue:
> > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > tlab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F2171&data=05%7C0
> > >
> 1%7Cwayne.lin%40amd.com%7Ccd5a63120e064f4bb6aa08daa7090baf%7C3d
> d8961
> > >
> fe4884e608e11a82d994e183d%7C0%7C0%7C638005954559719396%7CUnkno
> wn%7CT
> > >
> WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLC
> JXV
> > >
> CI6Mn0%3D%7C3000%7C%7C%7C&sdata=nMIGnUKS6EDrdKJ0rR%2BAh
> FRa4ST0%2
> > > BYr9bILmXv40yv0%3D&reserved=0
> > > Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into
> > > the atomic state")
> > > ---
> > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2
> +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > 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 b8077fcd4651..00598def5b39 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
> > > @@ -297,7 +297,7 @@ bool
> dm_helpers_dp_mst_send_payload_allocation(
> > > clr_flag = MST_ALLOCATE_NEW_PAYLOAD;
> > > }
> > >
> > > - if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state-
> >base.state, payload)) {
> > > + if (enable && drm_dp_add_payload_part2(mst_mgr,
> > > +mst_state->base.state, payload) == 0) {
Hi Lyude,
This line changes the original logic a bit. The 'if' case was trying to catch failure
while sending ALLOCATE_PAYLOAD. If the msg fails, set the set_flag to false.
If succeed, set the set_flag to true and clear the clr_flag.
Sorry if the code wording misleading. Thanks!
> > > amdgpu_dm_set_mst_status(&aconnector->mst_status,
> > > set_flag, false);
> > > } else {
> >
> > Hi Lyude,
> >
> > Maybe I'm missing something, but I can't find the
> > drm_dp_add_payload_part2() function on amd-staging-drm-next. Which
> > repo are you using?
>
> If it's not on amd-staging-drm-next then it likely hasn't gotten backported to
> amd's branch yet and is in drm-misc-next
>
> >
> > Thanks
> > Siqueira
> >
>
> --
> Cheers,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
--
Regards,
Wayne Lin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of drm_dp_add_payload_part2()
2022-10-17 3:09 ` Lin, Wayne
@ 2022-10-19 21:32 ` Lyude Paul
0 siblings, 0 replies; 5+ messages in thread
From: Lyude Paul @ 2022-10-19 21:32 UTC (permalink / raw)
To: Lin, Wayne, Siqueira, Rodrigo, dri-devel, amd-gfx
Cc: Chen, Ian, Li, Sun peng (Leo),
Pan, Xinhui, open list, Jani Nikula, Zuo, Jerry, Claudio Suarez,
Mahfooz, Hamza, Deucher, Alexander, Mikita Lipski, Koenig,
Christian, Colin Ian King
Gotcha, I'll take another look at this tomorrow
On Mon, 2022-10-17 at 03:09 +0000, Lin, Wayne wrote:
> [Public]
>
>
>
> > -----Original Message-----
> > From: Lyude Paul <lyude@redhat.com>
> > Sent: Thursday, October 6, 2022 3:37 AM
> > To: Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; dri-
> > devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> > Cc: Wentland, Harry <Harry.Wentland@amd.com>; Li, Sun peng (Leo)
> > <Sunpeng.Li@amd.com>; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; Koenig, Christian
> > <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; David
> > Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Zuo, Jerry
> > <Jerry.Zuo@amd.com>; Lin, Wayne <Wayne.Lin@amd.com>; Chen, Ian
> > <Ian.Chen@amd.com>; Mikita Lipski <mikita.lipski@amd.com>; Mahfooz,
> > Hamza <Hamza.Mahfooz@amd.com>; Claudio Suarez <cssk@net-c.es>; Colin
> > Ian King <colin.i.king@gmail.com>; Jani Nikula <jani.nikula@intel.com>; open
> > list <linux-kernel@vger.kernel.org>
> > Subject: Re: [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of
> > drm_dp_add_payload_part2()
> >
> > On Tue, 2022-10-04 at 16:46 -0400, Rodrigo Siqueira Jordao wrote:
> > >
> > > On 2022-10-04 16:24, Lyude Paul wrote:
> > > > Yikes, it appears somehow I totally made a mistake here. We're
> > > > currently checking to see if drm_dp_add_payload_part2() returns a
> > > > non-zero value to indicate success. That's totally wrong though, as
> > > > this function only returns a zero value on success - not the other way
> > around.
> > > >
> > > > So, fix that.
> > > >
> > > > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > > > Issue:
> > > >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > > tlab.freedesktop.org%2Fdrm%2Famd%2F-
> > %2Fissues%2F2171&data=05%7C0
> > > >
> > 1%7Cwayne.lin%40amd.com%7Ccd5a63120e064f4bb6aa08daa7090baf%7C3d
> > d8961
> > > >
> > fe4884e608e11a82d994e183d%7C0%7C0%7C638005954559719396%7CUnkno
> > wn%7CT
> > > >
> > WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLC
> > JXV
> > > >
> > CI6Mn0%3D%7C3000%7C%7C%7C&sdata=nMIGnUKS6EDrdKJ0rR%2BAh
> > FRa4ST0%2
> > > > BYr9bILmXv40yv0%3D&reserved=0
> > > > Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into
> > > > the atomic state")
> > > > ---
> > > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2
> > +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > 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 b8077fcd4651..00598def5b39 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
> > > > @@ -297,7 +297,7 @@ bool
> > dm_helpers_dp_mst_send_payload_allocation(
> > > > clr_flag = MST_ALLOCATE_NEW_PAYLOAD;
> > > > }
> > > >
> > > > - if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state-
> > > base.state, payload)) {
> > > > + if (enable && drm_dp_add_payload_part2(mst_mgr,
> > > > +mst_state->base.state, payload) == 0) {
>
> Hi Lyude,
>
> This line changes the original logic a bit. The 'if' case was trying to catch failure
> while sending ALLOCATE_PAYLOAD. If the msg fails, set the set_flag to false.
> If succeed, set the set_flag to true and clear the clr_flag.
>
> Sorry if the code wording misleading. Thanks!
>
> > > > amdgpu_dm_set_mst_status(&aconnector->mst_status,
> > > > set_flag, false);
> > > > } else {
> > >
> > > Hi Lyude,
> > >
> > > Maybe I'm missing something, but I can't find the
> > > drm_dp_add_payload_part2() function on amd-staging-drm-next. Which
> > > repo are you using?
> >
> > If it's not on amd-staging-drm-next then it likely hasn't gotten backported to
> > amd's branch yet and is in drm-misc-next
> >
> > >
> > > Thanks
> > > Siqueira
> > >
> >
> > --
> > Cheers,
> > Lyude Paul (she/her)
> > Software Engineer at Red Hat
> --
> Regards,
> Wayne Lin
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-19 21:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-04 20:24 [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of drm_dp_add_payload_part2() Lyude Paul
2022-10-04 20:46 ` Rodrigo Siqueira Jordao
2022-10-05 19:37 ` Lyude Paul
2022-10-17 3:09 ` Lin, Wayne
2022-10-19 21:32 ` Lyude Paul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).