Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <2948115.KggauuSURZ@jernej-laptop>
References: <20180612200036.21483-1-jernej.skrabec@siol.net>
 <20180612200036.21483-12-jernej.skrabec@siol.net> <20180615083110.laruycabvyjmesmo@flea>
 <2948115.KggauuSURZ@jernej-laptop>
From:   Chen-Yu Tsai <wens@csie.org>
Date:   Sat, 16 Jun 2018 01:13:17 +0800
Message-ID: <CAGb2v66UFRKRF4_zbm8nQJYoOHb-iLbiU_0iYWep=WNiQDftMQ@mail.gmail.com>
Subject: Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support
 for tcon-top gate
To:     Jernej Skrabec <jernej.skrabec@siol.net>
Cc:     Maxime Ripard <maxime.ripard@bootlin.com>,
        Rob Herring <robh+dt@kernel.org>,
        David Airlie <airlied@linux.ie>,
        Gustavo Padovan <gustavo@padovan.org>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Sean Paul <seanpaul@chromium.org>,
        Mark Rutland <mark.rutland@arm.com>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        devicetree <devicetree@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-clk <linux-clk@vger.kernel.org>,
        linux-sunxi <linux-sunxi@googlegroups.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec
<jernej.skrabec@siol.net> wrote:
> Hi,
>
> Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard napisal(a):
>> Hi,
>>
>> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote:
>> > TV TCONs connected to TCON TOP have to enable additional gate in order
>> > to work.
>> >
>> > Add support for such TCONs.
>> >
>> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>> > ---
>> >
>> >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++
>> >  drivers/gpu/drm/sun4i/sun4i_tcon.h |  4 ++++
>> >  2 files changed, 15 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..0afb5a94a414
>> > 100644
>> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev,
>> >
>> >             dev_err(dev, "Couldn't get the TCON bus clock\n");
>> >             return PTR_ERR(tcon->clk);
>> >
>> >     }
>> >
>> > +
>> > +   if (tcon->quirks->has_tcon_top_gate) {
>> > +           tcon->top_clk = devm_clk_get(dev, "tcon-top");
>> > +           if (IS_ERR(tcon->top_clk)) {
>> > +                   dev_err(dev, "Couldn't get the TCON TOP bus clock\n");
>> > +                   return PTR_ERR(tcon->top_clk);
>> > +           }
>> > +           clk_prepare_enable(tcon->top_clk);
>> > +   }
>> > +
>>
>> Is it required for the TCON itself to operate, or does the TCON
>> requires the TCON TOP, which in turn requires that clock to be
>> functional?
>>
>> I find it quite odd to have a clock that isn't meant for a particular
>> device to actually be wired to another device. I'm not saying this
>> isn't the case, but it would be a first.
>
> Documentation doesn't say much about that gate. I did few tests and TCON
> registers can be read and written even if TCON TOP TV TCON gate is disabled.
> However, there is no image, as expected.

The R40 manual does include it in the diagram, on page 504. There's also a
mux to select whether the clock comes directly from the CCU or the TV encoder
(a feedback mode?). I assume this is the gate you are referring to here, in
which case it is not a bus clock, but rather the TCON module or channel clock,
strangely routed.

> More interestingly, I enabled test pattern directly in TCON to eliminate
> influence of the mixer. As soon as I disabled that gate, test pattern on HDMI
> screen was gone, which suggest that this gate influences something inside
> TCON.
>
> Another test I did was that I moved enable/disable gate code to
> sun4i_tcon_channel_set_status() and it worked just as well.
>
> I'll ask AW engineer what that gate actually does, but from what I saw, I
> would say that most appropriate location to enable/disable TCON TOP TV TCON
> gate is TCON driver. Alternatively, TCON TOP driver could check if any TV TCON
> is in use and enable appropriate gate. However, that doesn't sound right to me
> for some reason.

If what I said above it true, then yes, the appropriate location to enable it
is the TCON driver, but moreover, the representation of the clock tree should
be fixed such that the TCON takes the clock from the TCON TOP as its channel/
module clock instead. That way you don't need this patch, but you'd add another
for all the clock routing.

ChenYu