From: Maxime Ripard <maxime.ripard@free-electrons.com>
To: Chen-Yu Tsai <wens@csie.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
devicetree <devicetree@vger.kernel.org>,
Boris Brezillon <boris.brezillon@free-electrons.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Rob Herring <robh+dt@kernel.org>, Mark Brown <broonie@kernel.org>,
Thierry Reding <thierry.reding@gmail.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Daniel Vetter <daniel.vetter@intel.com>,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 07/18] drm/sun4i: tcon: Don't rely on encoders to set the TCON mode
Date: Thu, 20 Jul 2017 15:34:48 +0200 [thread overview]
Message-ID: <20170720133448.uywpaps4xcbgk7rp@flea> (raw)
In-Reply-To: <CAGb2v66d500j5YWFbTYn_BZcF_q=sM+hr2q+yJ88OmL_j4DVPg@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 5564 bytes --]
On Fri, Jul 14, 2017 at 11:56:18AM +0800, Chen-Yu Tsai wrote:
> On Thu, Jul 13, 2017 at 10:13 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Just like we did for the TCON enable and disable, for historical reasons we
> > used to rely on the encoders calling the TCON mode_set function, while the
> > CRTC has a callback for that.
> >
> > Let's implement it in order to reduce the boilerplate code.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> > drivers/gpu/drm/sun4i/sun4i_crtc.c | 11 ++++-
> > drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c | 1 +-
> > drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 7 +---
> > drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 1 +-
> > drivers/gpu/drm/sun4i/sun4i_rgb.c | 15 +------
> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 56 ++++++++++------------
> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 10 +----
> > drivers/gpu/drm/sun4i/sun4i_tv.c | 6 +--
> > 8 files changed, 40 insertions(+), 67 deletions(-)
> >
>
> [...]
>
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > index dc70bc2a42a5..c4407910dfaf 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -106,29 +106,6 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable)
> > }
> > EXPORT_SYMBOL(sun4i_tcon_enable_vblank);
> >
> > -void sun4i_tcon_set_mux(struct sun4i_tcon *tcon, int channel,
> > - struct drm_encoder *encoder)
> > -{
> > - u32 val;
> > -
> > - if (!tcon->quirks->has_unknown_mux)
> > - return;
> > -
> > - if (channel != 1)
> > - return;
> > -
> > - if (encoder->encoder_type == DRM_MODE_ENCODER_TVDAC)
> > - val = 1;
> > - else
> > - val = 0;
> > -
> > - /*
> > - * FIXME: Undocumented bits
> > - */
> > - regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, val);
> > -}
> > -EXPORT_SYMBOL(sun4i_tcon_set_mux);
> > -
> > static int sun4i_tcon_get_clk_delay(struct drm_display_mode *mode,
> > int channel)
> > {
> > @@ -147,8 +124,8 @@ static int sun4i_tcon_get_clk_delay(struct drm_display_mode *mode,
> > return delay;
> > }
> >
> > -void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon,
> > - struct drm_display_mode *mode)
> > +static void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon,
> > + struct drm_display_mode *mode)
>
> Nit on the side: maybe we could mark mode as constant?
> Since the function doesn't change it. Same applies to the
> other mode_set functions. But this could be left to another
> patch.
We totally should. I'll do it.
> > {
> > unsigned int bp, hsync, vsync;
> > u8 clk_delay;
> > @@ -221,10 +198,9 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon,
> > /* Enable the output on the pins */
> > regmap_write(tcon->regs, SUN4I_TCON0_IO_TRI_REG, 0);
> > }
> > -EXPORT_SYMBOL(sun4i_tcon0_mode_set);
> >
> > -void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
> > - struct drm_display_mode *mode)
> > +static void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
> > + struct drm_display_mode *mode)
> > {
> > unsigned int bp, hsync, vsync, vtotal;
> > u8 clk_delay;
> > @@ -312,7 +288,29 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
> > SUN4I_TCON_GCTL_IOMAP_MASK,
> > SUN4I_TCON_GCTL_IOMAP_TCON1);
> > }
> > -EXPORT_SYMBOL(sun4i_tcon1_mode_set);
> > +
> > +void sun4i_tcon_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder,
> > + struct drm_display_mode *mode)
>
> (also mark encoder as const?)
Yep.
> > +{
> > + switch (encoder->encoder_type) {
> > + case DRM_MODE_ENCODER_NONE:
> > + sun4i_tcon0_mode_set(tcon, mode);
> > + break;
> > + case DRM_MODE_ENCODER_TVDAC:
> > + /*
> > + * FIXME: Undocumented bits
> > + */
> > + if (tcon->quirks->has_unknown_mux)
> > + regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, 1);
> > + /* Fallthrough */
> > + case DRM_MODE_ENCODER_TMDS:
> > + sun4i_tcon1_mode_set(tcon, mode);
>
> IIRC you need to clear the mux bit here. So ...
>
> > + break;
> > + default:
> > + DRM_DEBUG_DRIVER("Unknown encoder type, doing nothing...\n");
> > + }
>
> I think keeping the muxing in a separate function would be cleaner.
> The above is already slightly messy if you add the bit clearing part.
> With all the other muxing possibilities in the other SoC this is
> going to get really messy.
Ok.
> > +}
> > +EXPORT_SYMBOL(sun4i_tcon_mode_set);
> >
> > static void sun4i_tcon_finish_page_flip(struct drm_device *dev,
> > struct sun4i_crtc *scrtc)
>
> [...]
>
> Thanks for working on this. Now we've decoupled the TCON/CRTC code
> from all the encoders.
Yeah, I still have mixed feelings about this, but it was the sensible
thing I guess.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2017-07-20 13:34 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-13 14:12 [PATCH 00/18] drm/sun4i: Allwinner MIPI-DSI support Maxime Ripard
2017-07-13 14:12 ` [PATCH 01/18] regmap: mmio: Add function to attach a clock Maxime Ripard
2017-07-13 16:01 ` Mark Brown
2017-07-17 9:01 ` Maxime Ripard
2017-07-18 13:21 ` Mark Brown
2017-07-20 14:44 ` Maxime Ripard
2017-07-13 14:12 ` [PATCH 02/18] drm/sun4i: Add if statement instead of depends on Maxime Ripard
2017-07-14 3:09 ` Chen-Yu Tsai
[not found] ` <CAGb2v64KXCm1SzCz1hSkk1WOA03rju5_v_Q9XdF=cSs26Wz+Tw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-17 8:45 ` Maxime Ripard
2017-07-13 14:12 ` [PATCH 03/18] drm/sun4i: Realign Makefile padding and reorder it Maxime Ripard
[not found] ` <fd70853df9e7ee66a20df9d42921f1f6608a2bc9.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-14 3:13 ` Chen-Yu Tsai
[not found] ` <CAGb2v67HmUpSaoJWS6jKg3JvsPmqMtv30EGdWE50YEPm2cyjcg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-20 13:08 ` Maxime Ripard
2017-07-13 14:12 ` [PATCH 04/18] drm/sun4i: Remove useless atomic_check Maxime Ripard
2017-07-14 3:15 ` Chen-Yu Tsai
[not found] ` <CAGb2v67=x+6tdKsKEkY2VS75H561zUfh55h2sNUcrCERr+z72A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-17 8:47 ` Maxime Ripard
2017-07-13 14:13 ` [PATCH 05/18] drm/sun4i: tcon: remove unused function Maxime Ripard
[not found] ` <7d8e9b8d77c9bef3258e97a06aab42314355acf4.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-14 3:16 ` Chen-Yu Tsai
2017-07-13 14:13 ` [PATCH 06/18] drm/sun4i: tcon: Don't rely on encoders to enable the TCON Maxime Ripard
[not found] ` <e8e204b4b366f609db5b7d848f8cfb88345cd2d7.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-14 3:40 ` Chen-Yu Tsai
[not found] ` <CAGb2v67LNwLWWJEkw=FKE-kPkVHPK7XZC-OY02A1a94w6E5e1g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-20 13:20 ` Maxime Ripard
2017-07-13 14:13 ` [PATCH 07/18] drm/sun4i: tcon: Don't rely on encoders to set the TCON mode Maxime Ripard
[not found] ` <31e3fd8540d85c8f4637af1083c5c2a436403cd2.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-14 3:56 ` Chen-Yu Tsai
2017-07-20 13:34 ` Maxime Ripard [this message]
2017-07-13 14:13 ` [PATCH 08/18] drm/sun4i: tcon: Add TRI finish interrupt for vblank Maxime Ripard
2017-07-14 3:57 ` Chen-Yu Tsai
2017-07-13 14:13 ` [PATCH 09/18] drm/sun4i: tcon: Adjust dotclock dividers range Maxime Ripard
[not found] ` <74db696da66cffafe1e729fe2df73b437c8fd483.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-14 4:14 ` Chen-Yu Tsai
[not found] ` <CAGb2v67WtqukOHPNkyzA+s8GAUR6fnomvT6Sqp8547euHH3G-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-20 14:55 ` Maxime Ripard
2017-07-20 15:16 ` Chen-Yu Tsai
2017-07-13 14:13 ` [PATCH 10/18] drm/sun4i: tcon: Move out the tcon0 common setup Maxime Ripard
2017-07-14 9:50 ` kbuild test robot
2017-07-18 3:41 ` Chen-Yu Tsai
[not found] ` <CAGb2v65TfUKyWg=vxWP-YhM3Be1i=N1OmNTO16aFk2VTCg4VXg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-20 13:55 ` Maxime Ripard
2017-07-13 14:13 ` [PATCH 11/18] dt-bindings: display: Add Allwinner MIPI-DSI bindings Maxime Ripard
[not found] ` <41dca303c02faf350e765b81c36ddcddbba6f950.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-17 18:41 ` Rob Herring
2017-07-18 10:18 ` Laurent Pinchart
2017-07-20 14:21 ` Maxime Ripard
2017-07-20 14:19 ` Maxime Ripard
2017-07-13 14:13 ` [PATCH 12/18] drm/sun4i: Add Allwinner A31 MIPI-DSI controller support Maxime Ripard
[not found] ` <55729048aa9c57b46ed3f8d284eec2fbf5f1ae6a.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-14 10:15 ` kbuild test robot
2017-07-13 14:13 ` [PATCH 13/18] dt-bindings: vendor: Add Huarui Lighting Maxime Ripard
2017-07-14 7:53 ` Chen-Yu Tsai
2017-07-20 13:37 ` Maxime Ripard
2017-07-13 14:13 ` [PATCH 14/18] dt-bindings: panel: Add Huarui LHR050H41 panel documentation Maxime Ripard
[not found] ` <5c1df0e5171fd2fb0e4536e88363eeb8c8382b80.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-17 18:44 ` Rob Herring
[not found] ` <cover.a5e7407c191c66e264164d017f64582039c56acb.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-13 14:13 ` [PATCH 15/18] drm/panel: Add Huarui LHR050H41 panel driver Maxime Ripard
[not found] ` <78212e5adad59f09db9ff87b4430b97494f4d874.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-14 9:22 ` Andrzej Hajda
2017-07-13 14:13 ` [PATCH 16/18] arm: dts: sun8i: a33: Add the DSI-related nodes Maxime Ripard
2017-07-13 14:13 ` [PATCH 17/18] arm: dts: sun8i: Add BananaPI M2-Magic DTS Maxime Ripard
[not found] ` <fd6f80ac08ce9cbe4ccba884750dae128c538a2f.1499955058.git-series.maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-07-14 4:40 ` Chen-Yu Tsai
[not found] ` <CAGb2v678uy7YBHyO01q84ypQu+zfp2Mp+pnORz7s5m91svx0vA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-17 9:03 ` Maxime Ripard
2017-07-13 14:13 ` [PATCH 18/18] [DO NOT MERGE] arm: dts: sun8i: bpi-m2m: Add DSI display Maxime Ripard
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=20170720133448.uywpaps4xcbgk7rp@flea \
--to=maxime.ripard@free-electrons.com \
--cc=boris.brezillon@free-electrons.com \
--cc=broonie@kernel.org \
--cc=daniel.vetter@intel.com \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=robh+dt@kernel.org \
--cc=thierry.reding@gmail.com \
--cc=thomas.petazzoni@free-electrons.com \
--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).