Re: [Freedreno] [PATCH 1/3] drm/msm/mdp5: Configure PP_SYNC_HEIGHT to double the vtotal
From: AngeloGioacchino Del Regno
Date: Wed Apr 07 2021 - 15:12:03 EST
Il 07/04/21 20:19, abhinavk@xxxxxxxxxxxxxx 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.
Suggested-by: AngeloGioacchino Del Regno
<angelogioacchino.delregno@xxxxxxxxxxxxxx>
Signed-off-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
Reviewed-by: AngeloGioacchino Del Regno
<angelogioacchino.delregno@xxxxxxxxxxxxxx>
---
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);