Re: [PATCH v1 5/5] coresight: etm4x: save/restore state across CPU low power states

From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: Al Grant <Al.Grant@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Andrew Murray <andrew.murray@arm.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Mike Leach <mike.leach@linaro.org>
Subject: Re: [PATCH v1 5/5] coresight: etm4x: save/restore state across CPU low power states
Date: Thu, 20 Jun 2019 10:47:38 -0600	[thread overview]
Message-ID: <CANLsYkymTnxRX61StUGvKGeiQV6P6YbCg81PSYeBpXLsX5tpiw@mail.gmail.com> (raw)
In-Reply-To: <20190620163426.GC25273@e107155-lin>

On Thu, 20 Jun 2019 at 10:34, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Thu, Jun 20, 2019 at 10:14:04AM -0600, Mathieu Poirier wrote:
> > On Thu, 20 Jun 2019 at 09:41, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
>
> [...]
>
> > > Sorry but even then I prefer it not to be default and force extra work
> > > to the people who add support and constantly be reminded that it's
> > > broken and they are deviating from default behaviour in the kernel
> > > which may come and latency penalty.
> > >
> > > Making it default may hide the problem if Linux is used for some validation.
> > >
> > > Also we hardly have 3-4 platforms in upstream that support coresight,
> > > and many are broken except Juno. But that doesn't imply all others
> > > are broken and we just can't derive that unless we have more information.
> >
> > For now we have a clear trend.  To me it is not a matter of broken vs.
> > non-broken but more about what people want to do or can
> > (realistically) do.
> >
>
> No disagreement there.
>
> > The coresight specification is broad and very permissive in terms of
> > implementation defined choices.  It is not because the TRCPDCR.PU it
> > not taken into account by a platform that it is automatically broken.
> > This could be a design choice or a trade off.  We already have two
> > ways of putting a CPU to sleep (architected or OS driven), we simply
> > do the same here for coresight.
> >
>
> Sure, if the term "broken" is inappropriate I am fine if anything else
> is used. The point is we are adding an idle notifier that adds latency
> and must be done if and only if necessary.
>
> How you identify that and implement doesn't bother me much, making
> that default just based on the fact that more platforms need it
> compared to others definitely does. So I am fine if this needs to be
> advertised *not broken* but *by design*, sure go for it.
>

Then all we have to do is make the ACPI/DT property that indicate the
method used to deal with tracer idling mandatory.  That way people are
conscious of the choice they are making.  To be backward compatible
with current systems we default to the TRCPDCR.PU method but print a
warning message, just like we do for obsolete DT bindings.

> My main concern was additional latency that this introduces for platforms
> that don't require.
>
> --
> Regards,
> Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel