From: Jagan Teki <jagan@amarulasolutions.com>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>,
Rob Herring <robh+dt@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
Icenowy Zheng <icenowy@aosc.io>,
Jernej Skrabec <jernej.skrabec@siol.net>,
Vasily Khoruzhick <anarsoul@gmail.com>,
Thierry Reding <thierry.reding@gmail.com>,
Mark Rutland <mark.rutland@arm.com>,
dri-devel <dri-devel@lists.freedesktop.org>,
devicetree <devicetree@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
Michael Trimarchi <michael@amarulasolutions.com>,
TL Lim <tllim@pine64.org>,
linux-sunxi@googlegroups.com, linux-amarula@amarulasolutions.com
Subject: Re: [PATCH v2 04/12] drm/sun4i: sun6i_mipi_dsi: Simplify drq set to support all modes
Date: Mon, 19 Nov 2018 16:52:17 +0530 [thread overview]
Message-ID: <CAMty3ZAKL2fZqwLwd_WiQpzz+cOETMR7ES7HQsBo3PsAvjM+6A@mail.gmail.com> (raw)
In-Reply-To: <20181119083243.4njj2p2sy2xf2zyf@flea>
On Mon, Nov 19, 2018 at 2:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Fri, Nov 16, 2018 at 10:09:08PM +0530, Jagan Teki wrote:
> > Allwinner MIPI DSI DRQ set value can be varied with respective
> > video modes.
> > - burst mode the set value is always 0
> > - video modes whose front porch greater than 20, the set value
> > can be computed based front porch and bpp.
> > - video modes whose front porch is not greater than 20, the set value
> > is simply 0
> >
> > This patch simplifies existing drq set value code by grouping
> > into sun6i_dsi_get_drq and support all video modes.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 38 ++++++++++++++++----------
> > 1 file changed, 23 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index efd08bfd0cb8..d60955880c43 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -363,6 +363,26 @@ static void sun6i_dsi_inst_init(struct sun6i_dsi *dsi,
> > SUN6I_DSI_INST_JUMP_CFG_NUM(1));
> > };
> >
> > +static int sun6i_dsi_get_drq(struct sun6i_dsi *dsi,
> > + struct drm_display_mode *mode)
> > +{
> > + struct mipi_dsi_device *device = dsi->device;
> > + int drq = 0;
>
> So, here, you declaring a variable called drq, set to 0.
>
> > + if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST)
> > + return drq;
>
> That you return here. You could just return 0, to be clearer.
>
> > + if ((mode->hsync_start - mode->hdisplay) > 20) {
> > + /* Maaaaaagic */
> > + u16 drq = (mode->hsync_start - mode->hdisplay) - 20;
>
> You re-declare a variable with the same name here, but a different
> type....
>
> > + drq *= mipi_dsi_pixel_format_to_bpp(device->format);
> > + drq /= 32;
> > + }
> > +
> > + return drq;
>
> And then return the first one? How is that even working?
Will fix this.
>
> > +
> > static u16 sun6i_dsi_get_timings_vblk(struct sun6i_dsi *dsi,
> > struct drm_display_mode *mode, u16 hblk)
> > {
> > @@ -478,21 +498,9 @@ static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi,
> > static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
> > struct drm_display_mode *mode)
> > {
> > - struct mipi_dsi_device *device = dsi->device;
> > - u32 val = 0;
> > -
> > - if ((mode->hsync_start - mode->hdisplay) > 20) {
> > - /* Maaaaaagic */
> > - u16 drq = (mode->hsync_start - mode->hdisplay) - 20;
> > -
> > - drq *= mipi_dsi_pixel_format_to_bpp(device->format);
> > - drq /= 32;
> > -
> > - val = (SUN6I_DSI_TCON_DRQ_ENABLE_MODE |
> > - SUN6I_DSI_TCON_DRQ_SET(drq));
> > - }
> > -
> > - regmap_write(dsi->regs, SUN6I_DSI_TCON_DRQ_REG, val);
> > + regmap_write(dsi->regs, SUN6I_DSI_TCON_DRQ_REG,
> > + SUN6I_DSI_TCON_DRQ_ENABLE_MODE |
> > + SUN6I_DSI_TCON_DRQ_SET(sun6i_dsi_get_drq(dsi, mode)));
>
> On top of that, you now enable the DRQ stuff all the time, while it
Earlier the val value is ENABLE_MODE ORed with drq value. for the
condition drq is computed in if block otherwise the val is 0.
So as I explained in commit message the drq value is 0
- for video modes whose front porch is not greater than 20 and
- for burst mode the val
ie reason I mode it common.
next prev parent reply other threads:[~2018-11-19 11:22 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-16 16:39 [PATCH v2 00/12] drm/sun4i: Allwinner MIPI-DSI Burst mode support Jagan Teki
2018-11-16 16:39 ` [PATCH v2 01/12] drm/sun4i: sun6i_mipi_dsi: Compute burst mode loop N1 instruction delay Jagan Teki
2018-11-19 8:27 ` Maxime Ripard
2018-11-19 10:58 ` Jagan Teki
2018-11-20 13:23 ` Maxime Ripard
2018-11-20 13:36 ` Jagan Teki
2018-11-20 15:58 ` Maxime Ripard
2018-11-20 16:01 ` Vasily Khoruzhick
2018-11-20 16:19 ` Jagan Teki
2018-11-16 16:39 ` [PATCH v2 02/12] drm/sun4i: sun6i_mipi_dsi: Support instruction loop selection Jagan Teki
2018-11-16 16:39 ` [PATCH v2 03/12] drm/sun4i: sun6i_mipi_dsi: Setup burst mode timings Jagan Teki
2018-11-19 8:30 ` Maxime Ripard
2018-11-19 11:00 ` Jagan Teki
2018-11-20 15:45 ` Maxime Ripard
2018-11-20 16:22 ` Jagan Teki
2018-11-27 10:07 ` Maxime Ripard
2018-11-16 16:39 ` [PATCH v2 04/12] drm/sun4i: sun6i_mipi_dsi: Simplify drq set to support all modes Jagan Teki
2018-11-19 8:32 ` Maxime Ripard
2018-11-19 11:22 ` Jagan Teki [this message]
2018-11-20 14:32 ` Maxime Ripard
2018-11-20 14:48 ` Jagan Teki
2018-11-16 16:39 ` [PATCH v2 05/12] drm/sun4i: tcon: Export get tcon0 routine Jagan Teki
2018-11-19 8:34 ` Maxime Ripard
2018-11-16 16:39 ` [PATCH v2 06/12] drm/sun4i: sun6i_mipi_dsi: Probe tcon0 during dsi_bind Jagan Teki
2018-11-19 8:38 ` Maxime Ripard
2018-11-19 11:36 ` Jagan Teki
2018-11-20 15:44 ` Maxime Ripard
2018-11-16 16:39 ` [PATCH v2 07/12] drm/sun4i: sun6i_mipi_dsi: Setup burst mode Jagan Teki
2018-11-16 16:39 ` [PATCH v2 08/12] drm/sun4i: sun6i_mipi_dsi: Enable 2byte trail for 4-lane " Jagan Teki
2018-11-16 16:39 ` [PATCH v2 09/12] drm/sun4i: sun6i_mipi_dsi: Enable burst mode HBP, HSA_HSE Jagan Teki
2018-11-16 16:39 ` [PATCH v2 10/12] dt-bindings: panel: Add Feiyang FY07024DI26A30-D MIPI-DSI LCD panel Jagan Teki
2018-11-16 16:39 ` [PATCH v2 11/12] drm/panel: " Jagan Teki
2018-12-10 16:12 ` Jagan Teki
2018-12-13 15:07 ` Sean Paul
2018-12-13 19:26 ` Jagan Teki
2018-12-13 19:55 ` Sean Paul
2018-12-14 11:05 ` Jagan Teki
2018-12-14 14:15 ` Sean Paul
2018-11-16 16:39 ` [PATCH v2 12/12][DO NOT MERGE] arm64: allwinner: a64: pine64-lts: Enable Feiyang FY07024DI26A30-D DSI panel Jagan Teki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMty3ZAKL2fZqwLwd_WiQpzz+cOETMR7ES7HQsBo3PsAvjM+6A@mail.gmail.com \
--to=jagan@amarulasolutions.com \
--cc=airlied@linux.ie \
--cc=anarsoul@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=icenowy@aosc.io \
--cc=jernej.skrabec@siol.net \
--cc=linux-amarula@amarulasolutions.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sunxi@googlegroups.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mark.rutland@arm.com \
--cc=maxime.ripard@bootlin.com \
--cc=michael@amarulasolutions.com \
--cc=robh+dt@kernel.org \
--cc=sean@poorly.run \
--cc=thierry.reding@gmail.com \
--cc=tllim@pine64.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).