From: Icenowy Zheng <icenowy@aosc.io>
To: linux-arm-kernel@lists.infradead.org,
Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: devicetree@vger.kernel.org,
"Jernej Škrabec" <jernej.skrabec@siol.net>,
linux-sunxi@googlegroups.com, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, "Chen-Yu Tsai" <wens@csie.org>,
"Rob Herring" <robh+dt@kernel.org>,
linux-clk@vger.kernel.org
Subject: Re: [PATCH v2 07/11] drm: sun4i: add support for the TV encoder in H3 SoC
Date: Tue, 13 Jun 2017 17:51:42 +0800 [thread overview]
Message-ID: <5A2149A9-CC7B-4946-9219-A4FE3228B402@aosc.io> (raw)
In-Reply-To: <20170613074432.btwgc4e7pndtsvbf@flea.lan>
于 2017年6月13日 GMT+08:00 下午3:44:32, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Sun, Jun 11, 2017 at 02:43:42PM +0800, icenowy@aosc.io wrote:
>> 在 2017-06-07 17:38,Maxime Ripard 写道:
>> > On Mon, Jun 05, 2017 at 12:01:45AM +0800, Icenowy Zheng wrote:
>> > > Allwinner H3 features a TV encoder similar to the one in earlier
>SoCs,
>> > > but has a internal fixed clock divider that divides the TCON1
>clock
>> > > (called TVE clock in datasheet) by 11.
>> > >
>> > > Add support for it.
>> > >
>> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> > > ---
>> > > Changes in v2:
>> > > - Quirk part rewritten.
>> > >
>> > > drivers/gpu/drm/sun4i/sun4i_tv.c | 35
>> > > ++++++++++++++++++++++++++++++++++-
>> > > 1 file changed, 34 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c
>> > > b/drivers/gpu/drm/sun4i/sun4i_tv.c
>> > > index 338b9e5bb2a3..b9ff6d5ea67a 100644
>> > > --- a/drivers/gpu/drm/sun4i/sun4i_tv.c
>> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
>> > > @@ -13,6 +13,7 @@
>> > > #include <linux/clk.h>
>> > > #include <linux/component.h>
>> > > #include <linux/of_address.h>
>> > > +#include <linux/of_device.h>
>> > > #include <linux/regmap.h>
>> > > #include <linux/reset.h>
>> > >
>> > > @@ -169,14 +170,21 @@ struct tv_mode {
>> > > const struct resync_parameters *resync_params;
>> > > };
>> > >
>> > > +struct sun4i_tv_quirks {
>> > > + int fixed_divider;
>> > > +};
>> > > +
>> > > struct sun4i_tv {
>> > > struct drm_connector connector;
>> > > struct drm_encoder encoder;
>> > >
>> > > struct clk *clk;
>> > > + struct clk *mod_clk;
>> > > struct regmap *regs;
>> > > struct reset_control *reset;
>> > >
>> > > + const struct sun4i_tv_quirks *quirks;
>> > > +
>> > > struct sun4i_drv *drv;
>> > > };
>> > >
>> > > @@ -391,6 +399,12 @@ static void sun4i_tv_mode_set(struct
>> > > drm_encoder *encoder,
>> > > struct sun4i_tcon *tcon = crtc->tcon;
>> > > const struct tv_mode *tv_mode = sun4i_tv_find_tv_by_mode(mode);
>> > >
>> > > + if (tv->quirks->fixed_divider) {
>> > > + DRM_DEBUG_DRIVER("Applying fixed divider %d on TVE clock\n",
>> > > + tv->quirks->fixed_divider);
>> > > + mode->crtc_clock *= tv->quirks->fixed_divider;
>> > > + }
>> > > +
>> >
>> > You're not allowed to change the mode in mode_set, and you
>shouldn't
>> > even change it. The output pixel clock is still 27MHz.
>> >
>> > You should implement that using the states, as we discussed
>already.
>>
>> After reading the comments at
>include/drm/drm_modeset_helper_vtables.h,
>> I think the atomic_check function is allowed to directly change
>> the adjust_mode of crtc_state.
>>
>> And according to other comments at include/drm/drm_modes.h, the
>> crtc_clock in adjust_mode should be the clock to be really feed
>> to the hardware.
>>
>> So I think I can just change this in atomic_check.
>>
>> However, the mode_set function of sun4i_tv seems to be not
>> regarding adjusted_mode and still use original mode.
>>
>> So my current design is:
>> - Multiply adjusted_mode in atomic_check.
>> - Feed adjust_mode to TCON code in mode_set function.
>>
>> Is this proper?
>>
>> (From my own understanding to the comments I think so.)
>
>No, it's not. The pixel clock in the mode is the pixel clock output
>physically by the connector. You want to use it for an intermediate
>clock in your pipeline.
>
>This is not the same clock, and not the same frequency.
So should a multiplier parameter be passed via
tcon1_mode_set function?
>
>Maxime
next prev parent reply other threads:[~2017-06-13 9:53 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-04 16:01 [PATCH v2 00/11] Support for H3 Composite Output support Icenowy Zheng
2017-06-04 16:01 ` [PATCH v2 01/11] dt-bindings: update the binding for Allwinner H3 TVE support Icenowy Zheng
2017-06-07 8:45 ` Maxime Ripard
2017-06-07 8:48 ` Icenowy Zheng
2017-06-09 16:49 ` Maxime Ripard
2017-06-09 16:51 ` [linux-sunxi] " Icenowy Zheng
2017-06-09 21:24 ` Jernej Škrabec
2017-06-10 15:26 ` icenowy
2017-06-13 7:41 ` Maxime Ripard
2017-06-04 16:01 ` [PATCH v2 02/11] drm: sun4i: add support for H3 mixers Icenowy Zheng
2017-06-04 16:01 ` [PATCH v2 03/11] drm: sun4i: ignore swapped mixer<->tcon connection for DE2 Icenowy Zheng
2017-06-07 9:35 ` Maxime Ripard
2017-06-07 9:44 ` Icenowy Zheng
2017-06-07 10:01 ` Icenowy Zheng
2017-06-07 14:38 ` Maxime Ripard
2017-06-07 18:15 ` Jernej Škrabec
2017-06-09 14:45 ` Maxime Ripard
2017-06-08 5:01 ` icenowy
2017-06-09 14:46 ` Maxime Ripard
2017-06-10 14:57 ` icenowy
2017-06-10 15:16 ` icenowy
2017-06-13 9:46 ` Maxime Ripard
2017-06-13 9:43 ` Maxime Ripard
2017-06-13 10:05 ` Chen-Yu Tsai
2017-06-23 7:31 ` Maxime Ripard
2017-06-04 16:01 ` [PATCH v2 04/11] drm: sun4i: add support for H3's TCON0/1 Icenowy Zheng
2017-06-04 18:46 ` Jernej Škrabec
2017-06-04 19:03 ` Icenowy Zheng
2017-06-07 9:43 ` Maxime Ripard
2017-06-07 9:44 ` Icenowy Zheng
2017-06-07 14:19 ` Maxime Ripard
2017-06-07 14:21 ` Icenowy Zheng
2017-06-09 14:48 ` Maxime Ripard
2017-06-07 10:00 ` Icenowy Zheng
2017-06-04 22:43 ` kbuild test robot
2017-06-04 16:01 ` [PATCH v2 05/11] drm: sun4i: add compatible for H3 display engine Icenowy Zheng
2017-06-04 16:01 ` [PATCH v2 06/11] drm: sun4i: add color space correction support for DE2 mixer Icenowy Zheng
2017-06-04 16:01 ` [PATCH v2 07/11] drm: sun4i: add support for the TV encoder in H3 SoC Icenowy Zheng
2017-06-07 9:38 ` Maxime Ripard
2017-06-11 6:43 ` icenowy
2017-06-13 7:44 ` Maxime Ripard
2017-06-13 9:51 ` Icenowy Zheng [this message]
2017-06-23 14:44 ` Maxime Ripard
2017-06-04 16:01 ` [PATCH v2 08/11] clk: sunxi-ng: allow CLK_DE to set CLK_PLL_DE for H3 Icenowy Zheng
2017-06-04 16:01 ` [PATCH v2 09/11] clk: sunxi-ng: export " Icenowy Zheng
2017-06-04 16:01 ` [PATCH v2 10/11] ARM: sun8i: h3: add display engine pipeline for TVE Icenowy Zheng
2017-06-07 9:42 ` Maxime Ripard
2017-06-11 6:58 ` [linux-sunxi] " icenowy
2017-06-13 8:02 ` Maxime Ripard
2017-06-04 16:01 ` [PATCH v2 11/11] [DO NOT MERGE] ARM: sun8i: h3: enable TV output on Orange Pi PC Icenowy Zheng
2017-06-07 0:26 ` [PATCH v2 00/11] Support for H3 Composite Output support icenowy
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=5A2149A9-CC7B-4946-9219-A4FE3228B402@aosc.io \
--to=icenowy@aosc.io \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=jernej.skrabec@siol.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sunxi@googlegroups.com \
--cc=maxime.ripard@free-electrons.com \
--cc=robh+dt@kernel.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).