[Freedreno] [PATCH 1/3] drm/msm/mdp5: Configure PP_SYNC_HEIGHT to double the vtotal

[AngeloGioacchino Del Regno]

Il gio 8 apr 2021, 21:05 Rob Clark <robdclark at gmail.com> ha scritto:

> On Wed, Apr 7, 2021 at 12:11 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno at somainline.org> wrote:
> >
> > Il 07/04/21 20:19, abhinavk at codeaurora.org ha scritto:
> > > Hi Marijn
> > >
> > > On 2021-04-06 14:47, Marijn Suijten wrote:
> > >> Leaving this at a close-to-maximum register value 0xFFF0 means it
> takes
> > >> very long for the MDSS to generate a software vsync interrupt when the
> > >> hardware TE interrupt doesn't arrive.  Configuring this to double the
> > >> vtotal (like some downstream kernels) leads to a frame to take at most
> > >> twice before the vsync signal, until hardware TE comes up.
> > >>
> > >> In this case the hardware interrupt responsible for providing this
> > >> signal - "disp-te" gpio - is not hooked up to the mdp5 vsync/pp logic
> at
> > >> all.  This solves severe panel update issues observed on at least the
> > >> Xperia Loire and Tone series, until said gpio is properly hooked up to
> > >> an irq.
> > >
> > > The reason the CONFIG_HEIGHT was at such a high value is to make sure
> that
> > > we always get the TE only from the panel vsync and not false positives
> > > coming
> > > from the tear check logic itself.
> > >
> > > When you say that disp-te gpio is not hooked up, is it something
> > > incorrect with
> > > the schematic OR panel is not generating the TE correctly?
> > >
> >
> > Sometimes, some panels aren't getting correctly configured by the
> > OEM/ODM in the first place: especially when porting devices from
> > downstream to upstream, developers often get in a situation in which
> > their TE line is either misconfigured or the DriverIC is not configured
> > to raise V-Sync interrupts.
> > Please remember: some DDICs need a "commands sequence" to enable
> > generating the TE interrupts, sometimes this is not standard, and
> > sometimes OEMs/ODMs are not even doing that in their downstream code
> > (but instead they work around it in creative ways "for reasons", even
> > though their DDIC supports indeed sending TE events).
> >
> > This mostly happens when bringing up devices that have autorefresh
> > enabled from the bootloader (when the bootloader sets up the splash
> > screen) by using simple-panel as a (hopefully) temporary solution to get
> > through the initial stages of porting.
> >
> > We are not trying to cover cases related to incorrect schematics or
> > hardware mistakes here, as the fix for that - as you know - is to just
> > fix your hardware.
> > What we're trying to do here is to stop freezes and, in some cases,
> > lockups, other than false positives making the developer go offroad when
> > the platform shows that something is wrong during early porting.
> >
> > Also, sometimes, some DDICs will not generate TE interrupts when
> > expected... in these cases we get a PP timeout and a MDP5 recovery: this
> > is totally avoidable if we rely on the 2*vtotal, as we wouldn't get
> > through the very time consuming task of recovering the entire MDP.
> >
> > Of course, if something is wrong in the MDP and the block really needs
> > recovery, this "trick" won't save anyone and the recovery will anyway be
> > triggered, as the PP-done will anyway timeout.
>
> So, is this (mostly) a workaround due to TE not wired up?  In which
> case I think it is ok, but maybe should have a comment about the
> interaction with TE?
>

Mostly, yes.


> Currently I have this patch in msm-next-staging but I guess we need to
> decide in the next day or so whether to drop it or smash in a comment?
>
> BR,
> -R
>

Marijn, can you please urgently throw a comment in, reminding that these
timers are interacting with TE and send a fast V2?

Cheers,
 - Angelo


> > >>
> > >> Suggested-by: AngeloGioacchino Del Regno
> > >> <angelogioacchino.delregno at somainline.org>
> > >> Signed-off-by: Marijn Suijten <marijn.suijten at somainline.org>
> > >> Reviewed-by: AngeloGioacchino Del Regno
> > >> <angelogioacchino.delregno at somainline.org>
> > >> ---
> > >>  drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c | 2 +-
> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
> > >> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
> > >> index ff2c1d583c79..2d5ac03dbc17 100644
> > >> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
> > >> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
> > >> @@ -51,7 +51,7 @@ static int pingpong_tearcheck_setup(struct
> > >> drm_encoder *encoder,
> > >>
> > >>      mdp5_write(mdp5_kms, REG_MDP5_PP_SYNC_CONFIG_VSYNC(pp_id), cfg);
> > >>      mdp5_write(mdp5_kms,
> > >> -        REG_MDP5_PP_SYNC_CONFIG_HEIGHT(pp_id), 0xfff0);
> > >> +        REG_MDP5_PP_SYNC_CONFIG_HEIGHT(pp_id), (2 * mode->vtotal));
> > >>      mdp5_write(mdp5_kms,
> > >>          REG_MDP5_PP_VSYNC_INIT_VAL(pp_id), mode->vdisplay);
> > >>      mdp5_write(mdp5_kms, REG_MDP5_PP_RD_PTR_IRQ(pp_id),
> > >> mode->vdisplay + 1);
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210409/ea1428f7/attachment-0001.htm>