Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1687052

Re: [PATCH 07/18] drm/sun4i: tcon: Don't rely on encoders to set the TCON mode

From Chen-Yu Tsai <wens@csie.org>
Newsgroups linux.kernel
Subject Re: [PATCH 07/18] drm/sun4i: tcon: Don't rely on encoders to set the TCON mode
Date 2017-07-14 06:00 +0200
Message-ID <u2ZVL-69C-3@gated-at.bofh.it> (permalink)
References <u2N8d-6Ff-3@gated-at.bofh.it> <u2N8f-6Ff-19@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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.

>  {
>         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?)

> +{
> +       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.

> +}
> +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.

Regards
ChenYu

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/18] drm/sun4i: Allwinner MIPI-DSI support Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
  [PATCH 06/18] drm/sun4i: tcon: Don't rely on encoders to enable the TCON Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 06/18] drm/sun4i: tcon: Don't rely on encoders to enable  the TCON Chen-Yu Tsai <wens@csie.org> - 2017-07-14 05:50 +0200
  [PATCH 04/18] drm/sun4i: Remove useless atomic_check Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 04/18] drm/sun4i: Remove useless atomic_check Chen-Yu Tsai <wens@csie.org> - 2017-07-14 05:20 +0200
      Re: [PATCH 04/18] drm/sun4i: Remove useless atomic_check Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-17 10:50 +0200
  [PATCH 15/18] drm/panel: Add Huarui LHR050H41 panel driver Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 15/18] drm/panel: Add Huarui LHR050H41 panel driver Andrzej Hajda <a.hajda@samsung.com> - 2017-07-14 11:30 +0200
  [PATCH 08/18] drm/sun4i: tcon: Add TRI finish interrupt for vblank Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 08/18] drm/sun4i: tcon: Add TRI finish interrupt for vblank Chen-Yu Tsai <wens@csie.org> - 2017-07-14 06:00 +0200
  [PATCH 07/18] drm/sun4i: tcon: Don't rely on encoders to set the TCON mode Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 07/18] drm/sun4i: tcon: Don't rely on encoders to set the  TCON mode Chen-Yu Tsai <wens@csie.org> - 2017-07-14 06:00 +0200
  [PATCH 10/18] drm/sun4i: tcon: Move out the tcon0 common setup Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 10/18] drm/sun4i: tcon: Move out the tcon0 common setup kbuild test robot <lkp@intel.com> - 2017-07-14 12:00 +0200
    Re: [PATCH 10/18] drm/sun4i: tcon: Move out the tcon0 common setup Chen-Yu Tsai <wens@csie.org> - 2017-07-18 05:50 +0200
  [PATCH 16/18] arm: dts: sun8i: a33: Add the DSI-related nodes Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
  [PATCH 09/18] drm/sun4i: tcon: Adjust dotclock dividers range Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 09/18] drm/sun4i: tcon: Adjust dotclock dividers range Chen-Yu Tsai <wens@csie.org> - 2017-07-14 06:20 +0200
  [PATCH 17/18] arm: dts: sun8i: Add BananaPI M2-Magic DTS Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 17/18] arm: dts: sun8i: Add BananaPI M2-Magic DTS Chen-Yu Tsai <wens@csie.org> - 2017-07-14 06:50 +0200
      Re: [PATCH 17/18] arm: dts: sun8i: Add BananaPI M2-Magic DTS Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-17 11:10 +0200
  [PATCH 13/18] dt-bindings: vendor: Add Huarui Lighting Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 13/18] dt-bindings: vendor: Add Huarui Lighting Chen-Yu Tsai <wens@csie.org> - 2017-07-14 10:00 +0200
  [PATCH 01/18] regmap: mmio: Add function to attach a clock Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 01/18] regmap: mmio: Add function to attach a clock Mark Brown <broonie@kernel.org> - 2017-07-13 18:10 +0200
      Re: [PATCH 01/18] regmap: mmio: Add function to attach a clock Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-17 11:10 +0200
  [PATCH 02/18] drm/sun4i: Add if statement instead of depends on Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 02/18] drm/sun4i: Add if statement instead of depends on Chen-Yu Tsai <wens@csie.org> - 2017-07-14 05:10 +0200
      Re: [PATCH 02/18] drm/sun4i: Add if statement instead of depends on Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-17 10:50 +0200
  [PATCH 18/18] [DO NOT MERGE] arm: dts: sun8i: bpi-m2m: Add DSI display Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
  [PATCH 03/18] drm/sun4i: Realign Makefile padding and reorder it Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-07-13 16:20 +0200
    Re: [PATCH 03/18] drm/sun4i: Realign Makefile padding and reorder it Chen-Yu Tsai <wens@csie.org> - 2017-07-14 05:20 +0200

csiph-web