[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMty3ZAD8qGV7tsgZiO9cQrZaiui1-E7Xzfq5o95BUoDoBEOkw@mail.gmail.com>
Date: Thu, 7 Mar 2019 00:32:09 +0530
From: Jagan Teki <jagan@...rulasolutions.com>
To: Maxime Ripard <maxime.ripard@...tlin.com>
Cc: David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
Chen-Yu Tsai <wens@...e.org>,
dri-devel <dri-devel@...ts.freedesktop.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-amarula@...rulasolutions.com,
Michael Trimarchi <michael@...rulasolutions.com>,
linux-sunxi <linux-sunxi@...glegroups.com>
Subject: Re: [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync_porch overflow
On Mon, Mar 4, 2019 at 9:24 PM Maxime Ripard <maxime.ripard@...tlin.com> wrote:
>
> 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?
Yes, ie true. will move it.
Powered by blists - more mailing lists