<div dir="auto"><div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Il gio 8 apr 2021, 21:05 Rob Clark <<a href="mailto:robdclark@gmail.com">robdclark@gmail.com</a>> ha scritto:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wed, Apr 7, 2021 at 12:11 PM AngeloGioacchino Del Regno<br>
<<a href="mailto:angelogioacchino.delregno@somainline.org" target="_blank" rel="noreferrer">angelogioacchino.delregno@somainline.org</a>> wrote:<br>
><br>
> Il 07/04/21 20:19, <a href="mailto:abhinavk@codeaurora.org" target="_blank" rel="noreferrer">abhinavk@codeaurora.org</a> ha scritto:<br>
> > Hi Marijn<br>
> ><br>
> > On 2021-04-06 14:47, Marijn Suijten wrote:<br>
> >> Leaving this at a close-to-maximum register value 0xFFF0 means it takes<br>
> >> very long for the MDSS to generate a software vsync interrupt when the<br>
> >> hardware TE interrupt doesn't arrive.  Configuring this to double the<br>
> >> vtotal (like some downstream kernels) leads to a frame to take at most<br>
> >> twice before the vsync signal, until hardware TE comes up.<br>
> >><br>
> >> In this case the hardware interrupt responsible for providing this<br>
> >> signal - "disp-te" gpio - is not hooked up to the mdp5 vsync/pp logic at<br>
> >> all.  This solves severe panel update issues observed on at least the<br>
> >> Xperia Loire and Tone series, until said gpio is properly hooked up to<br>
> >> an irq.<br>
> ><br>
> > The reason the CONFIG_HEIGHT was at such a high value is to make sure that<br>
> > we always get the TE only from the panel vsync and not false positives<br>
> > coming<br>
> > from the tear check logic itself.<br>
> ><br>
> > When you say that disp-te gpio is not hooked up, is it something<br>
> > incorrect with<br>
> > the schematic OR panel is not generating the TE correctly?<br>
> ><br>
><br>
> Sometimes, some panels aren't getting correctly configured by the<br>
> OEM/ODM in the first place: especially when porting devices from<br>
> downstream to upstream, developers often get in a situation in which<br>
> their TE line is either misconfigured or the DriverIC is not configured<br>
> to raise V-Sync interrupts.<br>
> Please remember: some DDICs need a "commands sequence" to enable<br>
> generating the TE interrupts, sometimes this is not standard, and<br>
> sometimes OEMs/ODMs are not even doing that in their downstream code<br>
> (but instead they work around it in creative ways "for reasons", even<br>
> though their DDIC supports indeed sending TE events).<br>
><br>
> This mostly happens when bringing up devices that have autorefresh<br>
> enabled from the bootloader (when the bootloader sets up the splash<br>
> screen) by using simple-panel as a (hopefully) temporary solution to get<br>
> through the initial stages of porting.<br>
><br>
> We are not trying to cover cases related to incorrect schematics or<br>
> hardware mistakes here, as the fix for that - as you know - is to just<br>
> fix your hardware.<br>
> What we're trying to do here is to stop freezes and, in some cases,<br>
> lockups, other than false positives making the developer go offroad when<br>
> the platform shows that something is wrong during early porting.<br>
><br>
> Also, sometimes, some DDICs will not generate TE interrupts when<br>
> expected... in these cases we get a PP timeout and a MDP5 recovery: this<br>
> is totally avoidable if we rely on the 2*vtotal, as we wouldn't get<br>
> through the very time consuming task of recovering the entire MDP.<br>
><br>
> Of course, if something is wrong in the MDP and the block really needs<br>
> recovery, this "trick" won't save anyone and the recovery will anyway be<br>
> triggered, as the PP-done will anyway timeout.<br>
<br>
So, is this (mostly) a workaround due to TE not wired up?  In which<br>
case I think it is ok, but maybe should have a comment about the<br>
interaction with TE?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"></blockquote></div></div><div dir="auto">Mostly, yes. </div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
Currently I have this patch in msm-next-staging but I guess we need to<br>
decide in the next day or so whether to drop it or smash in a comment?<br>
<br>
BR,<br>
-R<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"></blockquote></div></div><div dir="auto">Marijn, can you please urgently throw a comment in, reminding that these timers are interacting with TE and send a fast V2? </div><div dir="auto"><br></div><div dir="auto">Cheers, </div><div dir="auto"> - Angelo</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> >><br>
> >> Suggested-by: AngeloGioacchino Del Regno<br>
> >> <<a href="mailto:angelogioacchino.delregno@somainline.org" target="_blank" rel="noreferrer">angelogioacchino.delregno@somainline.org</a>><br>
> >> Signed-off-by: Marijn Suijten <<a href="mailto:marijn.suijten@somainline.org" target="_blank" rel="noreferrer">marijn.suijten@somainline.org</a>><br>
> >> Reviewed-by: AngeloGioacchino Del Regno<br>
> >> <<a href="mailto:angelogioacchino.delregno@somainline.org" target="_blank" rel="noreferrer">angelogioacchino.delregno@somainline.org</a>><br>
> >> ---<br>
> >>  drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c | 2 +-<br>
> >>  1 file changed, 1 insertion(+), 1 deletion(-)<br>
> >><br>
> >> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c<br>
> >> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c<br>
> >> index ff2c1d583c79..2d5ac03dbc17 100644<br>
> >> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c<br>
> >> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c<br>
> >> @@ -51,7 +51,7 @@ static int pingpong_tearcheck_setup(struct<br>
> >> drm_encoder *encoder,<br>
> >><br>
> >>      mdp5_write(mdp5_kms, REG_MDP5_PP_SYNC_CONFIG_VSYNC(pp_id), cfg);<br>
> >>      mdp5_write(mdp5_kms,<br>
> >> -        REG_MDP5_PP_SYNC_CONFIG_HEIGHT(pp_id), 0xfff0);<br>
> >> +        REG_MDP5_PP_SYNC_CONFIG_HEIGHT(pp_id), (2 * mode->vtotal));<br>
> >>      mdp5_write(mdp5_kms,<br>
> >>          REG_MDP5_PP_VSYNC_INIT_VAL(pp_id), mode->vdisplay);<br>
> >>      mdp5_write(mdp5_kms, REG_MDP5_PP_RD_PTR_IRQ(pp_id),<br>
> >> mode->vdisplay + 1);<br>
</blockquote></div></div></div>