Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
Dne ponedeljek, 25. junij 2018 ob 10:14:52 CEST je Chen-Yu Tsai napisal(a): > On Mon, Jun 25, 2018 at 3:58 PM, Jernej Škrabec > > wrote: > > Dne ponedeljek, 25. junij 2018 ob 05:51:41 CEST je Chen-Yu Tsai napisal(a): > >> On Mon, Jun 25, 2018 at 3:52 AM, Jernej Škrabec > >> > >> wrote: > >> > Dne četrtek, 21. junij 2018 ob 17:35:45 CEST je Jernej Škrabec napisal(a): > >> >> Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): > >> >> > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec > >> >> > > >> >> > >> >> wrote: > >> >> > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai > > > > napisal(a): > >> >> > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec > >> >> > >> > >> >> > > > >> >> > > wrote: > >> >> > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai > >> > > >> > napisal(a): > >> >> > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > >> >> > >> >> > >> >> > >> >> 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 > >> >> > >> >> >> > --- > >> >> > >> >> >> > > >> >> > >> >> >> > 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 > >> >>
Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
Dne ponedeljek, 25. junij 2018 ob 05:51:41 CEST je Chen-Yu Tsai napisal(a): > On Mon, Jun 25, 2018 at 3:52 AM, Jernej Škrabec > > wrote: > > Dne četrtek, 21. junij 2018 ob 17:35:45 CEST je Jernej Škrabec napisal(a): > >> Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): > >> > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec > >> > > >> > >> wrote: > >> > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): > >> > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec > >> > >> > >> > > > >> > > wrote: > >> > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai > > > > napisal(a): > >> > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > >> > >> >> > >> > >> >> 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 > >> > >> >> >> > --- > >> > >> >> >> > > >> > >> >> >> > 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. Alternativel
Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
On Mon, Jun 25, 2018 at 3:58 PM, Jernej Škrabec wrote: > Dne ponedeljek, 25. junij 2018 ob 05:51:41 CEST je Chen-Yu Tsai napisal(a): >> On Mon, Jun 25, 2018 at 3:52 AM, Jernej Škrabec >> >> wrote: >> > Dne četrtek, 21. junij 2018 ob 17:35:45 CEST je Jernej Škrabec napisal(a): >> >> Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): >> >> > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec >> >> > >> >> >> >> wrote: >> >> > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai > napisal(a): >> >> > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec >> >> > >> >> >> > > >> >> > > wrote: >> >> > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai >> > >> > napisal(a): >> >> > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec >> >> > >> >> >> >> > >> >> 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 >> >> > >> >> >> > --- >> >> > >> >> >> > >> >> > >> >> >> > 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 th
Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
Dne četrtek, 21. junij 2018 ob 17:35:45 CEST je Jernej Škrabec napisal(a): > Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): > > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec > > wrote: > > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): > > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec > > >> > > > > > > wrote: > > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): > > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > > >> >> > > >> >> 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 > > >> >> >> > --- > > >> >> >> > > > >> >> >> > 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 rout
Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
On Mon, Jun 25, 2018 at 3:52 AM, Jernej Škrabec wrote: > Dne četrtek, 21. junij 2018 ob 17:35:45 CEST je Jernej Škrabec napisal(a): >> Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): >> > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec >> >> wrote: >> > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): >> > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec >> > >> >> > > >> > > wrote: >> > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai > napisal(a): >> > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec >> > >> >> >> > >> >> 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 >> > >> >> >> > --- >> > >> >> >> > >> > >> >> >> > 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 >> > >> >> a
Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec wrote: > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec > > > > wrote: > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > >> >> > >> >> 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 > >> >> >> > --- > >> >> >> > > >> >> >> > 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. > >> > > >> > Can you be more specific? I not sure what you mean here. > >> > >> For clock related properties in the device tree: > >> > >> &tcon_top { > >> > >> clocks = <&ccu CLK_BUS_TCON_TOP>, > >> > >> <&ccu CLK_TCON_TV0>, > >> <&tve0>, > >> <&ccu CLK_TCON_TV1>, > >> <&tve1>; > >> > >> clock-names = "bus", "tcon-tv0",
Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): > On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec wrote: > > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): > >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > >> > >> 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 > >> >> > --- > >> >> > > >> >> > 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. > > > > Can you be more specific? I not sure what you mean here. > > For clock related properties in the device tree: > > &tcon_top { > clocks = <&ccu CLK_BUS_TCON_TOP>, > <&ccu CLK_TCON_TV0>, > <&tve0>, > <&ccu CLK_TCON_TV1>, > <&tve1>; > clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; > clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; > }; > > &tcon_tv0 { > clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' > clock-names = "ahb", "tcon-ch1"; > }; > > A diagram would look like: >| This part is TCON TOP | > >v v > CCU CLK_TCON_TV0 --|\ | > >| mux gate |-- TCON_TV0 > > TVE0 --|/ | > > And the same goes for TCON_TV1 and TVE1. > > The user manual is a bit lacking on how TVE outputs a clock though. I didn'
Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec wrote: > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec > wrote: >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec >> >> >> >> 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 >> >> >> > --- >> >> >> > >> >> >> > 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. >> > >> > Can you be more specific? I not sure what you mean here. >> >> For clock related properties in the device tree: >> >> &tcon_top { >> clocks = <&ccu CLK_BUS_TCON_TOP>, >> <&ccu CLK_TCON_TV0>, >> <&tve0>, >> <&ccu CLK_TCON_TV1>, >> <&tve1>; >> clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; >> clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; >> }; >> >> &tcon_tv0 { >> clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' >> clock-names = "ahb", "tcon-ch1"; >> }; >> >> A diagram would look like: >>| This part is TCON TOP | >> >>v v >> CCU CLK_TCON_TV0 --|\ | >> >>| mux gate |-- TCON_TV0 >> >> TVE0
Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
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 > > --- > > > > 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. 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. Best regards, Jernej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): > On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > > 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 > >> > --- > >> > > >> > 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. Can you be more specific? I not sure what you mean here. Best regards, Jernej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec wrote: > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec >> >> 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 >> >> > --- >> >> > >> >> > 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. > > Can you be more specific? I not sure what you mean here. For clock related properties in the device tree: &tcon_top { clocks = <&ccu CLK_BUS_TCON_TOP>, <&ccu CLK_TCON_TV0>, <&tve0>, <&ccu CLK_TCON_TV1>, <&tve1>; clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; }; &tcon_tv0 { clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' clock-names = "ahb", "tcon-ch1"; }; A diagram would look like: | This part is TCON TOP | v v CCU CLK_TCON_TV0 --|\ | | mux gate |-- TCON_TV0 TVE0 --|/ | And the same goes for TCON_TV1 and TVE1. The user manual is a bit lacking on how TVE outputs a clock though. ChenYu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec 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 >> > --- >> > >> > 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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
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 > --- > 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. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
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 --- 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); + } + clk_prepare_enable(tcon->clk); if (tcon->quirks->has_channel_0) { @@ -712,6 +722,7 @@ static int sun4i_tcon_init_clocks(struct device *dev, static void sun4i_tcon_free_clocks(struct sun4i_tcon *tcon) { clk_disable_unprepare(tcon->clk); + clk_disable_unprepare(tcon->top_clk); } static int sun4i_tcon_init_irq(struct device *dev, diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index f6a071cd5a6f..652d5c37d7b4 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -224,6 +224,7 @@ struct sun4i_tcon_quirks { boolneeds_de_be_mux; /* sun6i needs mux to select backend */ boolneeds_edp_reset; /* a80 edp reset needed for tcon0 access */ boolsupports_lvds; /* Does the TCON support an LVDS output? */ + boolhas_tcon_top_gate; /* TCON TOP holds additional gate to enable */ /* callback to handle tcon muxing options */ int (*set_mux)(struct sun4i_tcon *, const struct drm_encoder *); @@ -249,6 +250,9 @@ struct sun4i_tcon { u8 dclk_max_div; u8 dclk_min_div; + /* TCON TOP clock */ + struct clk *top_clk; + /* Reset control */ struct reset_control*lcd_rst; struct reset_control*lvds_rst; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel