On Tue, Jun 08, 2021 at 11:33:18PM +0200, Linus Walleij wrote: > The calclulation of how many bytes we stuff into the > DSI pipeline for video mode panels is off by three > orders of magnitude because we did not account for the > fact that the DRM mode clock is in kilohertz rather > than hertz. > > This used to be: > drm_mode_vrefresh(mode) * mode->htotal * mode->vtotal > which would become for example for s6e63m0: > 60 x 514 x 831 = 25628040 Hz, but mode->clock is > 25628 as it is in kHz. > > This affects only the Samsung GT-I8190 "Golden" phone > right now since it is the only MCDE device with a video > mode display. > > Curiously some specimen work with this code and wild > settings in the EOL and empty packets at the end of the > display, but I have noticed an eeire flicker until now. > Others were not so lucky and got black screens. > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Reported-by: Stephan Gerhold <stephan@xxxxxxxxxxx> > Fixes: 920dd1b1425b ("drm/mcde: Use mode->clock instead of reverse calculating it from the vrefresh") > Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx> Can confirm this makes things much better, thanks :) There is some garbage on the screen for a short moment, but overall it works really well now. > --- > drivers/gpu/drm/mcde/mcde_dsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c > index b3fd3501c412..5275b2723293 100644 > --- a/drivers/gpu/drm/mcde/mcde_dsi.c > +++ b/drivers/gpu/drm/mcde/mcde_dsi.c > @@ -577,7 +577,7 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d, > * porches and sync. > */ > /* (ps/s) / (pixels/s) = ps/pixels */ > - pclk = DIV_ROUND_UP_ULL(1000000000000, mode->clock); > + pclk = DIV_ROUND_UP_ULL(1000000000000, (mode->clock * 1000)); Removing three 0 in the dividend might be slightly more efficient, i.e. pclk = DIV_ROUND_UP(1000000000, mode->clock); since then we don't need 64-bit division (_ULL) anymore (1000000000 < 4294967296 = 2^32). but that's more nitpick level. I tested both, so for both options: Tested-by: Stephan Gerhold <stephan@xxxxxxxxxxx> Reviewed-by: Stephan Gerhold <stephan@xxxxxxxxxxx> Thanks! Stephan