Re: [PATCH 39/43] drm/amd/display: Use udelay to avoid context switch
On 01/11, Christian König wrote: > Am 10.01.20 um 22:30 schrieb Harry Wentland: > > On 2020-01-10 4:16 p.m., Harry Wentland wrote: > > > On 2020-01-10 1:47 p.m., Liu, Zhan wrote: > > > > > > > > > -Original Message- > > > > > From: amd-gfx On Behalf Of > > > > > Christian König > > > > > Sent: 2020/January/10, Friday 10:02 AM > > > > > To: Siqueira, Rodrigo ; amd- > > > > > g...@lists.freedesktop.org > > > > > Cc: Li, Sun peng (Leo) ; Cheng, Tony > > > > > ; Tsai, Martin ; Lakha, > > > > > Bhawanpreet ; Wentland, Harry > > > > > > > > > > Subject: Re: [PATCH 39/43] drm/amd/display: Use udelay to avoid > > > > > context > > > > > switch > > > > > > > > > > Am 10.01.20 um 15:46 schrieb Rodrigo Siqueira: > > > > > > From: Martin Tsai > > > > > > > > > > > > [why] > > > > > > The rapid msleep operation causes the white line garbage when DAL > > > > > > check flip pending status in SetVidPnSourceVisibility. > > > > > > To execute this msleep will induce context switch, and longer delay > > > > > > could cause worse garbage situation. > > > > > > > > > > > > [how] > > > > > > To replace msleep with udelay. > > > > > > > > > > > > Signed-off-by: Martin Tsai > > > > > > Reviewed-by: Tony Cheng > > > > > > Acked-by: Harry Wentland > > > > > > Acked-by: Rodrigo Siqueira > > > > > > --- > > > > > >drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c | 4 ++-- > > > > > >1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c > > > > > > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c > > > > > > index 89920924a154..0dc652e76848 100644 > > > > > > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c > > > > > > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c > > > > > > @@ -1642,9 +1642,9 @@ void dcn20_program_front_end_for_ctx( > > > > > > struct hubp *hubp = pipe->plane_res.hubp; > > > > > > int j = 0; > > > > > > > > > > > > - for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS > > > > > > + for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS*1000 > > > > > > && hubp->funcs- > > > > > > hubp_is_flip_pending(hubp); j++) > > > > > > - msleep(1); > > > > > > + udelay(1); > > > > > Why not using mdelay() here? > > > > As far as I know, mdelay() is only defined on Linux side. > > > > > > > > This piece of code is shared by both Linux and Windows, so we have to > > > > use a function that's available on both platforms. > > > > > > > It was used here before so we definitely have it defined for Windows as > > > well. > > > > > Whoops, I misread that. > > > > mdelay is indeed not defined on our other platforms but we could go > > ahead and define it if needed. > > Yes, indeed. And to repeat other platforms are irrelevant for up streaming. > > When changing the code in a certain way for Linux makes sense then we have > do to this and need to adjust other platforms accordingly or maintain the > delta somehow. Hi, I already made a patch to define mdelay on other OS platforms. For this patch, I just replace udelay by mdelay. Best Regards > Christian. > > > > > Harry > > > > > From the commit description it sounds like msleep wasn't tight enough > > > and longer delays lead to issues here, at least on Windows. > > > > > > Martin, Tony, do you have more details about this? > > > > > > Thanks, > > > Harry > > > > > > > Zhan > > > > > > > > > Christian. > > > > > > > > > > > } > > > > > > } > > > > > > > > > > > ___ > > > > > amd-gfx mailing list > > > > > amd-gfx@lists.freedesktop.org > > ___ > > amd-gfx mailing list > > amd-gfx@lists.freedesktop.org > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CRodrigo.Siqueira%40amd.com%7Cc13de5453b1b45fe04a108d796b41f24%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637143572686205847sdata=VgCSlghDomSX5T6v2JtujBb%2F0qwYUNvX0UCDUU0ehB0%3Dreserved=0 > -- Rodrigo Siqueira Software Engineer, Advanced Micro Devices (AMD) https://siqueira.tech signature.asc Description: PGP signature ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 39/43] drm/amd/display: Use udelay to avoid context switch
Am 10.01.20 um 22:30 schrieb Harry Wentland: On 2020-01-10 4:16 p.m., Harry Wentland wrote: On 2020-01-10 1:47 p.m., Liu, Zhan wrote: -Original Message- From: amd-gfx On Behalf Of Christian König Sent: 2020/January/10, Friday 10:02 AM To: Siqueira, Rodrigo ; amd- g...@lists.freedesktop.org Cc: Li, Sun peng (Leo) ; Cheng, Tony ; Tsai, Martin ; Lakha, Bhawanpreet ; Wentland, Harry Subject: Re: [PATCH 39/43] drm/amd/display: Use udelay to avoid context switch Am 10.01.20 um 15:46 schrieb Rodrigo Siqueira: From: Martin Tsai [why] The rapid msleep operation causes the white line garbage when DAL check flip pending status in SetVidPnSourceVisibility. To execute this msleep will induce context switch, and longer delay could cause worse garbage situation. [how] To replace msleep with udelay. Signed-off-by: Martin Tsai Reviewed-by: Tony Cheng Acked-by: Harry Wentland Acked-by: Rodrigo Siqueira --- drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c index 89920924a154..0dc652e76848 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c @@ -1642,9 +1642,9 @@ void dcn20_program_front_end_for_ctx( struct hubp *hubp = pipe->plane_res.hubp; int j = 0; - for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS + for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS*1000 && hubp->funcs- hubp_is_flip_pending(hubp); j++) - msleep(1); + udelay(1); Why not using mdelay() here? As far as I know, mdelay() is only defined on Linux side. This piece of code is shared by both Linux and Windows, so we have to use a function that's available on both platforms. It was used here before so we definitely have it defined for Windows as well. Whoops, I misread that. mdelay is indeed not defined on our other platforms but we could go ahead and define it if needed. Yes, indeed. And to repeat other platforms are irrelevant for up streaming. When changing the code in a certain way for Linux makes sense then we have do to this and need to adjust other platforms accordingly or maintain the delta somehow. Christian. Harry From the commit description it sounds like msleep wasn't tight enough and longer delays lead to issues here, at least on Windows. Martin, Tony, do you have more details about this? Thanks, Harry Zhan Christian. } } ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 39/43] drm/amd/display: Use udelay to avoid context switch
On 2020-01-10 4:16 p.m., Harry Wentland wrote: > On 2020-01-10 1:47 p.m., Liu, Zhan wrote: >> >> >>> -Original Message- >>> From: amd-gfx On Behalf Of >>> Christian König >>> Sent: 2020/January/10, Friday 10:02 AM >>> To: Siqueira, Rodrigo ; amd- >>> g...@lists.freedesktop.org >>> Cc: Li, Sun peng (Leo) ; Cheng, Tony >>> ; Tsai, Martin ; Lakha, >>> Bhawanpreet ; Wentland, Harry >>> >>> Subject: Re: [PATCH 39/43] drm/amd/display: Use udelay to avoid context >>> switch >>> >>> Am 10.01.20 um 15:46 schrieb Rodrigo Siqueira: >>>> From: Martin Tsai >>>> >>>> [why] >>>> The rapid msleep operation causes the white line garbage when DAL >>>> check flip pending status in SetVidPnSourceVisibility. >>>> To execute this msleep will induce context switch, and longer delay >>>> could cause worse garbage situation. >>>> >>>> [how] >>>> To replace msleep with udelay. >>>> >>>> Signed-off-by: Martin Tsai >>>> Reviewed-by: Tony Cheng >>>> Acked-by: Harry Wentland >>>> Acked-by: Rodrigo Siqueira >>>> --- >>>> drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c >>>> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c >>>> index 89920924a154..0dc652e76848 100644 >>>> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c >>>> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c >>>> @@ -1642,9 +1642,9 @@ void dcn20_program_front_end_for_ctx( >>>>struct hubp *hubp = pipe->plane_res.hubp; >>>>int j = 0; >>>> >>>> - for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS >>>> + for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS*1000 >>>>&& hubp->funcs- >>>> hubp_is_flip_pending(hubp); j++) >>>> - msleep(1); >>>> + udelay(1); >>> >>> Why not using mdelay() here? >> >> As far as I know, mdelay() is only defined on Linux side. >> >> This piece of code is shared by both Linux and Windows, so we have to use a >> function that's available on both platforms. >> > > It was used here before so we definitely have it defined for Windows as > well. > Whoops, I misread that. mdelay is indeed not defined on our other platforms but we could go ahead and define it if needed. Harry > From the commit description it sounds like msleep wasn't tight enough > and longer delays lead to issues here, at least on Windows. > > Martin, Tony, do you have more details about this? > > Thanks, > Harry > >> Zhan >> >>> >>> Christian. >>> >>>>} >>>>} >>>> >>> >>> ___ >>> amd-gfx mailing list >>> amd-gfx@lists.freedesktop.org ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 39/43] drm/amd/display: Use udelay to avoid context switch
On 2020-01-10 1:47 p.m., Liu, Zhan wrote: > > >> -Original Message- >> From: amd-gfx On Behalf Of >> Christian König >> Sent: 2020/January/10, Friday 10:02 AM >> To: Siqueira, Rodrigo ; amd- >> g...@lists.freedesktop.org >> Cc: Li, Sun peng (Leo) ; Cheng, Tony >> ; Tsai, Martin ; Lakha, >> Bhawanpreet ; Wentland, Harry >> >> Subject: Re: [PATCH 39/43] drm/amd/display: Use udelay to avoid context >> switch >> >> Am 10.01.20 um 15:46 schrieb Rodrigo Siqueira: >>> From: Martin Tsai >>> >>> [why] >>> The rapid msleep operation causes the white line garbage when DAL >>> check flip pending status in SetVidPnSourceVisibility. >>> To execute this msleep will induce context switch, and longer delay >>> could cause worse garbage situation. >>> >>> [how] >>> To replace msleep with udelay. >>> >>> Signed-off-by: Martin Tsai >>> Reviewed-by: Tony Cheng >>> Acked-by: Harry Wentland >>> Acked-by: Rodrigo Siqueira >>> --- >>> drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c >>> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c >>> index 89920924a154..0dc652e76848 100644 >>> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c >>> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c >>> @@ -1642,9 +1642,9 @@ void dcn20_program_front_end_for_ctx( >>> struct hubp *hubp = pipe->plane_res.hubp; >>> int j = 0; >>> >>> - for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS >>> + for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS*1000 >>> && hubp->funcs- >>> hubp_is_flip_pending(hubp); j++) >>> - msleep(1); >>> + udelay(1); >> >> Why not using mdelay() here? > > As far as I know, mdelay() is only defined on Linux side. > > This piece of code is shared by both Linux and Windows, so we have to use a > function that's available on both platforms. > It was used here before so we definitely have it defined for Windows as well. From the commit description it sounds like msleep wasn't tight enough and longer delays lead to issues here, at least on Windows. Martin, Tony, do you have more details about this? Thanks, Harry > Zhan > >> >> Christian. >> >>> } >>> } >>> >> >> ___ >> amd-gfx mailing list >> amd-gfx@lists.freedesktop.org ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 39/43] drm/amd/display: Use udelay to avoid context switch
> -Original Message- > From: amd-gfx On Behalf Of > Christian König > Sent: 2020/January/10, Friday 10:02 AM > To: Siqueira, Rodrigo ; amd- > g...@lists.freedesktop.org > Cc: Li, Sun peng (Leo) ; Cheng, Tony > ; Tsai, Martin ; Lakha, > Bhawanpreet ; Wentland, Harry > > Subject: Re: [PATCH 39/43] drm/amd/display: Use udelay to avoid context > switch > > Am 10.01.20 um 15:46 schrieb Rodrigo Siqueira: > > From: Martin Tsai > > > > [why] > > The rapid msleep operation causes the white line garbage when DAL > > check flip pending status in SetVidPnSourceVisibility. > > To execute this msleep will induce context switch, and longer delay > > could cause worse garbage situation. > > > > [how] > > To replace msleep with udelay. > > > > Signed-off-by: Martin Tsai > > Reviewed-by: Tony Cheng > > Acked-by: Harry Wentland > > Acked-by: Rodrigo Siqueira > > --- > > drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c > > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c > > index 89920924a154..0dc652e76848 100644 > > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c > > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c > > @@ -1642,9 +1642,9 @@ void dcn20_program_front_end_for_ctx( > > struct hubp *hubp = pipe->plane_res.hubp; > > int j = 0; > > > > - for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS > > + for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS*1000 > > && hubp->funcs- > >hubp_is_flip_pending(hubp); j++) > > - msleep(1); > > + udelay(1); > > Why not using mdelay() here? As far as I know, mdelay() is only defined on Linux side. This piece of code is shared by both Linux and Windows, so we have to use a function that's available on both platforms. Zhan > > Christian. > > > } > > } > > > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 39/43] drm/amd/display: Use udelay to avoid context switch
Am 10.01.20 um 15:46 schrieb Rodrigo Siqueira: From: Martin Tsai [why] The rapid msleep operation causes the white line garbage when DAL check flip pending status in SetVidPnSourceVisibility. To execute this msleep will induce context switch, and longer delay could cause worse garbage situation. [how] To replace msleep with udelay. Signed-off-by: Martin Tsai Reviewed-by: Tony Cheng Acked-by: Harry Wentland Acked-by: Rodrigo Siqueira --- drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c index 89920924a154..0dc652e76848 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c @@ -1642,9 +1642,9 @@ void dcn20_program_front_end_for_ctx( struct hubp *hubp = pipe->plane_res.hubp; int j = 0; - for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS + for (j = 0; j < TIMEOUT_FOR_PIPE_ENABLE_MS*1000 && hubp->funcs->hubp_is_flip_pending(hubp); j++) - msleep(1); + udelay(1); Why not using mdelay() here? Christian. } } ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
