linux-kernel - Re: [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190304155444.rwslksn57uggpazw@flea>
Date:   Mon, 4 Mar 2019 16:54:44 +0100
From:   Maxime Ripard <maxime.ripard@...tlin.com>
To:     Jagan Teki <jagan@...rulasolutions.com>
Cc:     David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
        Chen-Yu Tsai <wens@...e.org>, dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-amarula@...rulasolutions.com,
        Michael Trimarchi <michael@...rulasolutions.com>,
        linux-sunxi@...glegroups.com
Subject: Re: [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync_porch
 overflow

On Sun, Mar 03, 2019 at 11:05:23PM +0530, Jagan Teki wrote:
> Loop N1 instruction delay for burst mode devices are computed
> based on horizontal sync and porch timing values.
> 
> The current driver is using u16 type for computing this hsync_porch
> value, which would failed to fit within the u16 type for large sync
> and porch timings devices. This would result in hsync_porch overflow
> and eventually computed wrong instruction delay value.
> 
> Example, timings, where it produces the overflow
> {
> 	.hdisplay       = 1080,
> 	.hsync_start    = 1080 + 408,
>         .hsync_end      = 1080 + 408 + 4,
>         .htotal         = 1080 + 408 + 4 + 38,
> }
> 
> It reproduces the desired delay value 65487 but the correct working
> value should be 7.
> 
> So, Fix it by computing hsync_porch value separately with u32 type.
> 
> Fixes: 1c1a7aa3663c ("drm/sun4i: dsi: Add burst support")
> Signed-off-by: Jagan Teki <jagan@...rulasolutions.com>
> Tested-by: Merlijn Wajer <merlijn@...zup.org>
> ---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 6ff585055a07..465e7fc57899 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -457,8 +457,9 @@ static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi *dsi,
>  	u16 delay = 50 - 1;
>  
>  	if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> -		delay = (mode->htotal - mode->hdisplay) * 150;
> -		delay /= (mode->clock / 1000) * 8;
> +		u32 hsync_porch = (mode->htotal - mode->hdisplay);
> +
> +		delay = ((hsync_porch * 150) / ((mode->clock / 1000) * 8));

shouldn't we keep the multiplication by 150 in the u32 assignation?
Otherwise, we could keep a u16 for the delay

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)