[PATCH v6 06/18] video: tegra20: dc: add reset support

[Svyatoslav Ryhel]

пт, 19 квіт. 2024 р. о 19:46 Thierry Reding <thierry.reding at gmail.com> пише:
>
> On Fri Apr 19, 2024 at 6:37 PM CEST, Svyatoslav Ryhel wrote:
> > пт, 19 квіт. 2024 р. о 19:05 Thierry Reding <thierry.reding at gmail.com> пише:
> > >
> > > On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> > > > Implement reset use to discard any changes which could have been
> > > > applied to DC before and can interfere with current configuration.
> > > >
> > > > Tested-by: Agneli <poczt at protonmail.ch> # Toshiba AC100 T20
> > > > Tested-by: Robert Eckelmann <longnoserob at gmail.com> # ASUS TF101
> > > > Tested-by: Andreas Westman Dorcsak <hedmoo at yahoo.com> # ASUS Grouper E1565
> > > > Tested-by: Ion Agorria <ion at agorria.com> # HTC One X
> > > > Tested-by: Svyatoslav Ryhel <clamor95 at gmail.com> # Nvidia Tegratab T114
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
> > > > ---
> > > >  drivers/video/tegra20/tegra-dc.c | 17 +++++++++++++++++
> > > >  1 file changed, 17 insertions(+)
> > > >
> > > > diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> > > > index 56a23b3c97..35abb6fe46 100644
> > > > --- a/drivers/video/tegra20/tegra-dc.c
> > > > +++ b/drivers/video/tegra20/tegra-dc.c
> > > > @@ -10,7 +10,9 @@
> > > >  #include <panel.h>
> > > >  #include <part.h>
> > > >  #include <pwm.h>
> > > > +#include <reset.h>
> > > >  #include <video.h>
> > > > +#include <linux/delay.h>
> > > >  #include <asm/cache.h>
> > > >  #include <asm/global_data.h>
> > > >  #include <asm/system.h>
> > > > @@ -342,6 +344,7 @@ static int tegra_lcd_probe(struct udevice *dev)
> > > >       struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> > > >       struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> > > >       struct tegra_lcd_priv *priv = dev_get_priv(dev);
> > > > +     struct reset_ctl reset_ctl;
> > > >       int ret;
> > > >
> > > >       /* Initialize the Tegra display controller */
> > > > @@ -349,6 +352,20 @@ static int tegra_lcd_probe(struct udevice *dev)
> > > >       funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
> > > >  #endif
> > > >
> > > > +     ret = reset_get_by_name(dev, "dc", &reset_ctl);
> > > > +     if (ret) {
> > > > +             log_err("reset_get_by_name() failed: %d\n", ret);
> > > > +             return ret;
> > > > +     }
> > > > +
> > > > +     clock_disable(priv->dc_clk[0]);
> > > > +
> > > > +     /* Reset everything set before */
> > > > +     reset_assert(&reset_ctl);
> > > > +     mdelay(4);
> > > > +     reset_deassert(&reset_ctl);
> > > > +     mdelay(4);
> > >
> > > Are you sure this works as intended? It's been a long time since I
> > > worked on this, but I seem to recall that most of these resets are
> > > actually synchronous, so in order for them to do what they're supposed
> > > to the clock needs to be kept running.
> > >
> > > The Linux driver certainly does this differently.
> >
> > You have point, but I have tried to mostly adapt Linux tegra dc driver,
> > which has same logic in probe. Maybe I have not understood it properly.
> > Testing on T20, T30 and T114 passed without issues so far.
>
> Maybe look again. What it does is (basically):
>
>         clock_enable();
>         mdelay(4);
>         reset_assert();
>         mdelay(4);
>         clock_disable();
>
> That should ensure that it's completely reset at that point. Now before
> any subsequent register accesses happen it will do this:
>
>         clock_enable();
>         reset_deassert();
>
> to take it out of reset again. Perhaps that's something you want to keep
> doing in probe() in U-Boot. In that case maybe you want something like
> this instead:
>
>         clock_enable();
>         mdelay(4);
>         reset_assert();
>         mdelay(4);
>         reset_deassert();
>

You are correct. I assume This patch may be dropped entirely. Thanks.

> Thierry