[PATCH] drm/mcde: Fix off by 10^3 in calculation
Stephan Gerhold
stephan at gerhold.net
Wed Jun 9 10:04:52 UTC 2021
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 at linux.intel.com>
> Reported-by: Stephan Gerhold <stephan at gerhold.net>
> Fixes: 920dd1b1425b ("drm/mcde: Use mode->clock instead of reverse calculating it from the vrefresh")
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
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 at gerhold.net>
Reviewed-by: Stephan Gerhold <stephan at gerhold.net>
Thanks!
Stephan
More information about the dri-devel
mailing list