[RFC,0/5] drivers: Add boot constraints core

Message ID cover.1498642745.git.viresh.kumar@linaro.org
Headers show
Series
  • drivers: Add boot constraints core
Related show

Message

Viresh Kumar June 28, 2017, 10:26 a.m.
Hi,

I am sending this RFC to get early comments before I put too much
effort in the solution proposed here. The solution isn't fully complete
yet.


Problem statement:

Some devices are powered ON by the bootloaders before the bootloader
handovers control to Linux. It maybe important for those devices to keep
working until the time a Linux device driver probes the device and
reconfigure its resources.

A typical example of that can be the LCD controller, which is used by
the bootloaders to show image(s) while the machine is booting into
Linux. The LCD controller can be using some resources, like clk,
regulators, etc, that are shared between several devices. These shared
resources should be programmed so that all the users of them are
satisfied. If a user (X) driver gets probed before the LCD controller
driver in this case, then it may end up reconfiguring these resources to
ranges satisfying the current users (only user X) and that can make the
LCD screen unstable.

Of course we can have more complex cases where the same resource is
getting used by two devices while the kernel boots and the order in
which devices get probed wouldn't matter as the other device will surely
break then.


Proposed solution:

This patchset introduces the concept of boot-constraints, which are set
by the different parts of the kernel (on behalf of the bootloaders) and
the kernel will satisfy them until the time driver for such a device is
probed (successfully or unsuccessfully). Once the driver's probe()
routine is called, the driver core removes the constraints set for the
particular device. Only the power-supply constraint type is supported
for now.

This can be used across platforms working with DT, ACPI, etc and has no
dependency on those.


What's left ?

There are a couple of problems which aren't solved yet:

o EPROBE_DEFER: Deferred probing is a major problem that needs to be
  solved. Because we want to add the constraint ASAP (i.e. before most
  of the drivers are registered), we would end up doing that right after
  the device is created. That is very early in the kernel boot cycle and
  its very much possible that the resource we need to get (in order to
  set the constraint) isn't available yet. Like regulator in case of
  supply constraint.
  
  Now how do we control that the constraint is set right after the
  resource is available, and before any other user of the resource comes
  up ?

  One way out is to set "driver_deferred_probe_enable" to 'true'
  (drivers/base/dd.c), after we got EPROBE_DEFER for any of the
  constraints. That would make sure that all the deferred drivers get a
  chance to get probed again, after addition of every new device.
  Without this change, we probe all the deferred devices only after
  late_init.

  But that may not work as it depends on a new workqueue thread for
  doing this work, and no one is stopping another driver to get
  registered by that time.

  I also thought about using the "functional dependency" stuff [1] that
  Rafael introduced earlier, but that too has its own challenges.
  Specifically, we may not have the device structures available for all
  the consumers/suppliers to start with.

o DT support will be added at first and I am planning to add the
  constraints right from drivers/of/platform.c after the AMBA and
  platform devices are created automatically from DT. Some sort of
  bindings (per constraint type) are required to be defined for that of
  course. We can think about other interfaces (like ACPI) as well, if we
  have any users right now.


But yeah, the first thing is to get some sort of feedback for the
proposed solution. :)

FWIW, I started discussing this with Mark Brown earlier [2] and this
series is a follow-up to that discussion.

--
viresh

[1] commit 9ed9895370ae ("driver core: Functional dependencies tracking support")
[2] https://marc.info/?l=linux-kernel&m=149423887617635

Viresh Kumar (5):
  drivers: Add boot constraints core
  drivers: boot_constraint: Add support for supply constraints
  drivers: boot_constraint: Add boot_constraints_disable kernel
    parameter
  drivers: boot_constraint: Add debugfs support
  drivers: Code to test boot constraints

 Documentation/admin-guide/kernel-parameters.txt |   2 +
 drivers/base/Kconfig                            |  11 +
 drivers/base/Makefile                           |   1 +
 drivers/base/boot_constraint.c                  | 402 ++++++++++++++++++++++++
 drivers/base/dd.c                               |  20 +-
 drivers/base/test_plat_boot_constraint.c        |  51 +++
 include/linux/boot_constraint.h                 |  35 +++
 7 files changed, 515 insertions(+), 7 deletions(-)
 create mode 100644 drivers/base/boot_constraint.c
 create mode 100644 drivers/base/test_plat_boot_constraint.c
 create mode 100644 include/linux/boot_constraint.h

-- 
2.13.0.71.gd7076ec9c9cb

Comments

Enrico Weigelt, metux IT consult June 29, 2017, 12:40 p.m. | #1
On 28.06.2017 10:26, Viresh Kumar wrote:

Hi,

> Some devices are powered ON by the bootloaders before the bootloader

> handovers control to Linux. It maybe important for those devices to keep

> working until the time a Linux device driver probes the device and

> reconfigure its resources.


Just curious: aren't the devices (at least w/ DT) only initialized after
dependencies (eg. regulators) are already up ?

Let's imagine a LCD panel driven by a regulator behind SPI. The panel
driver would ask the regulator framework to switch on, which would
call the regulator driver. This one now would talk to SPI framework,
which finally calls the SPI driver. If SPI isn't up yet, it would all
be deferred, leaving the panel driver uninitialized (tried again later).

Am I wrong here ?

If the bootloader already switched on the panel (therefore already
enabled SPI), why does it matter that the panel driver isn't up yet ?

Is there anything that accidentially switches it off again (eg. by
resetting the regulator) ? If so, shouldn't the corresponding drivers
make sure that all depencies are met before doing anyhing w/ the
device, not even attempting a reset ?


--mtx
Russell King - ARM Linux admin June 29, 2017, 12:49 p.m. | #2
On Wed, Jun 28, 2017 at 03:56:33PM +0530, Viresh Kumar wrote:
> A typical example of that can be the LCD controller, which is used by

> the bootloaders to show image(s) while the machine is booting into

> Linux. The LCD controller can be using some resources, like clk,

> regulators, etc, that are shared between several devices. These shared

> resources should be programmed so that all the users of them are

> satisfied. If a user (X) driver gets probed before the LCD controller

> driver in this case, then it may end up reconfiguring these resources to

> ranges satisfying the current users (only user X) and that can make the

> LCD screen unstable.


The thing that concerns me most about this is that typically the LCD
controller will be performing DMA to system RAM.

The location of the frame buffer is unknown to the decompressor - and
as the decompressor self-relocates itself (using purely assembly code),
it could relocate itself on top of the frame buffer, causing the "nice"
image to become very colourful.

The decompressor doesn't have the information from DT at that point to
know what are safe locations, so it's up to the boot loader to place
the frame buffer somewhere out of the way.  (If people want to write
a DT parser in position independent ARM assembly code that may change.)

As long as people realise this, then it's not a problem, but given the
number of problems that we've already encountered with boot loaders and
memory space layout, I don't trust them to get this right.

Right now, the ARM kernel booting document requires:

- Quiesce all DMA capable devices so that memory does not get
  corrupted by bogus network packets or disk data. This will save
  you many hours of debug.

so we would need to modify that to make an exception for LCD controllers.
However, we definitely can't have devices left enabled which are capable
of writing to system memory, or which changing system memory is likely
to cause bad effects (eg, packet ring buffers, USB buffers etc, which is
really what the above requirement is about.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
Enrico Weigelt, metux IT consult June 29, 2017, 1:05 p.m. | #3
On 29.06.2017 12:49, Russell King - ARM Linux wrote:

> The location of the frame buffer is unknown to the decompressor - and

> as the decompressor self-relocates itself (using purely assembly code),

> it could relocate itself on top of the frame buffer, causing the "nice"

> image to become very colourful.


Could the bootloader pass safe (or blocked) memory regions to the
decompressor ?

--mtx
Viresh Kumar June 29, 2017, 2:47 p.m. | #4
On 29-06-17, 12:40, Enrico Weigelt, metux IT consult wrote:
> Just curious: aren't the devices (at least w/ DT) only initialized after

> dependencies (eg. regulators) are already up ?


No. Drivers are registered to the kernel (randomly, though we can know
their order) and devices are registered separately (platform/amba
devices get registered automatically with DT, hint:
drivers/of/platform.c). The device core checks while registering
devices/drivers if their drivers/devices are available or not. If
yes, then the devices are probed using the drivers. Now the drivers
must make sure all the dependencies are met at this point, else they
can return -EPROBE_DEFER and the kernel will try probing them again.

> Let's imagine a LCD panel driven by a regulator behind SPI. The panel

> driver would ask the regulator framework to switch on, which would

> call the regulator driver. This one now would talk to SPI framework,

> which finally calls the SPI driver. If SPI isn't up yet, it would all

> be deferred, leaving the panel driver uninitialized (tried again later).


This should happen in probe, otherwise we are screwed.

> If the bootloader already switched on the panel (therefore already

> enabled SPI), why does it matter that the panel driver isn't up yet ?


But the kernel doesn't know how it is configured, there can be so many
configurable parameters. The kernel needs to do it again by itself.

> Is there anything that accidentially switches it off again (eg. by

> resetting the regulator) ?


It is not just about switching it off, but the configuration here.

Let me try with an example. A regulator is shared between LCD and DMA
controller.

Operable ranges of the regulator: 1.8 - 3.0 V
Range required by LCD: 2.0 - 3.0 V
Range required by DMA: 1.8 - 2.5 V

Of course the right range for both of them to work here is 2.0 - 2.5
V.

Now the LCD is already enabled by the bootloader and kernel doesn't
know the ranges. DMA requests for the regulator and we set its voltage
to 1.8 V. LCD is screwed while we are still booting.

This will go worse if we add more devices to this example that share
the same regulator.

-- 
viresh
Viresh Kumar June 29, 2017, 2:58 p.m. | #5
On 29-06-17, 13:49, Russell King - ARM Linux wrote:
> The thing that concerns me most about this is that typically the LCD

> controller will be performing DMA to system RAM.

> 

> The location of the frame buffer is unknown to the decompressor - and

> as the decompressor self-relocates itself (using purely assembly code),

> it could relocate itself on top of the frame buffer, causing the "nice"

> image to become very colourful.

> 

> The decompressor doesn't have the information from DT at that point to

> know what are safe locations, so it's up to the boot loader to place

> the frame buffer somewhere out of the way.  (If people want to write

> a DT parser in position independent ARM assembly code that may change.)

> 

> As long as people realise this, then it's not a problem, but given the

> number of problems that we've already encountered with boot loaders and

> memory space layout, I don't trust them to get this right.

> 

> Right now, the ARM kernel booting document requires:

> 

> - Quiesce all DMA capable devices so that memory does not get

>   corrupted by bogus network packets or disk data. This will save

>   you many hours of debug.

> 

> so we would need to modify that to make an exception for LCD controllers.

> However, we definitely can't have devices left enabled which are capable

> of writing to system memory, or which changing system memory is likely

> to cause bad effects (eg, packet ring buffers, USB buffers etc, which is

> really what the above requirement is about.)


Well, LCD was just an example here. But yeah, it is one of the most
probable case we have.

So, this thing is already working for sure, of course with some out of
tree hacks. Every smart phone shows their company's logo (some kind of
flash) while the phone boots. How do they get around such issues?

@Stephen: Any idea how Qcom does it ? :)

Must be fixing some area in RAM for this purpose, isn't it ?

-- 
viresh
Enrico Weigelt, metux IT consult June 29, 2017, 3:06 p.m. | #6
On 29.06.2017 14:47, Viresh Kumar wrote:

> No. Drivers are registered to the kernel (randomly, though we can know

> their order) and devices are registered separately (platform/amba

> devices get registered automatically with DT, hint:

> drivers/of/platform.c). The device core checks while registering

> devices/drivers if their drivers/devices are available or not. If

> yes, then the devices are probed using the drivers. Now the drivers

> must make sure all the dependencies are met at this point, else they

> can return -EPROBE_DEFER and the kernel will try probing them again.


Could we somehow introduce an strict ordering ?
Maybe by letting the device core know of the dependencies, before
individual probe()'s explicitly ask for them ?

>> Let's imagine a LCD panel driven by a regulator behind SPI. The panel

>> driver would ask the regulator framework to switch on, which would

>> call the regulator driver. This one now would talk to SPI framework,

>> which finally calls the SPI driver. If SPI isn't up yet, it would all

>> be deferred, leaving the panel driver uninitialized (tried again later).

>

> This should happen in probe, otherwise we are screwed.


Yes, but the probe result may be deferred, so it's tried again in the
next round. Correct ?

>> If the bootloader already switched on the panel (therefore already

>> enabled SPI), why does it matter that the panel driver isn't up yet ?

>

> But the kernel doesn't know how it is configured, there can be so many

> configurable parameters. The kernel needs to do it again by itself.


Could it read back the config ?

By the way: I've got a similar problem w/ gpmc right now: uboot already
sets it up, but the kernel only knows about one CS (for the nand) and
screwes up the others (eg. fpga), so it cant access the fpga . Until
I've sorted out all the parameters for DT (unfortunately, only have the
raw register values), I'll have to rely on an userland test program
to set it all up ...

> Let me try with an example. A regulator is shared between LCD and DMA

> controller.

>

> Operable ranges of the regulator: 1.8 - 3.0 V

> Range required by LCD: 2.0 - 3.0 V

> Range required by DMA: 1.8 - 2.5 V


Would a config readback help here ?

The regulator core then should know that we're already in proper
range for DMA and no need to touch the regulator.


--mtx
Russell King - ARM Linux admin June 29, 2017, 3:43 p.m. | #7
On Thu, Jun 29, 2017 at 08:28:08PM +0530, Viresh Kumar wrote:
> On 29-06-17, 13:49, Russell King - ARM Linux wrote:

> > The thing that concerns me most about this is that typically the LCD

> > controller will be performing DMA to system RAM.

> > 

> > The location of the frame buffer is unknown to the decompressor - and

> > as the decompressor self-relocates itself (using purely assembly code),

> > it could relocate itself on top of the frame buffer, causing the "nice"

> > image to become very colourful.

> > 

> > The decompressor doesn't have the information from DT at that point to

> > know what are safe locations, so it's up to the boot loader to place

> > the frame buffer somewhere out of the way.  (If people want to write

> > a DT parser in position independent ARM assembly code that may change.)

> > 

> > As long as people realise this, then it's not a problem, but given the

> > number of problems that we've already encountered with boot loaders and

> > memory space layout, I don't trust them to get this right.

> > 

> > Right now, the ARM kernel booting document requires:

> > 

> > - Quiesce all DMA capable devices so that memory does not get

> >   corrupted by bogus network packets or disk data. This will save

> >   you many hours of debug.

> > 

> > so we would need to modify that to make an exception for LCD controllers.

> > However, we definitely can't have devices left enabled which are capable

> > of writing to system memory, or which changing system memory is likely

> > to cause bad effects (eg, packet ring buffers, USB buffers etc, which is

> > really what the above requirement is about.)

> 

> Well, LCD was just an example here. But yeah, it is one of the most

> probable case we have.

> 

> So, this thing is already working for sure, of course with some out of

> tree hacks. Every smart phone shows their company's logo (some kind of

> flash) while the phone boots. How do they get around such issues?


As far as the memory being used goes, they probably locate the frame
buffer well away from the kernel or any area that the kernel is likely
to use during decompression.

It's probably also marked as a reserved memory region in DT to avoid
the kernel touching it during boot, or _maybe_ they just locate it
somewhere in memory that they've tested that the kernel doesn't touch
until after their kernel has initialised the LCD controller (thereby
avoiding the memory being permanently consumed.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
Stephen Boyd June 29, 2017, 9 p.m. | #8
On 06/29, Russell King - ARM Linux wrote:
> On Thu, Jun 29, 2017 at 08:28:08PM +0530, Viresh Kumar wrote:

> > On 29-06-17, 13:49, Russell King - ARM Linux wrote:

> > > The thing that concerns me most about this is that typically the LCD

> > > controller will be performing DMA to system RAM.

> > > 

> > > The location of the frame buffer is unknown to the decompressor - and

> > > as the decompressor self-relocates itself (using purely assembly code),

> > > it could relocate itself on top of the frame buffer, causing the "nice"

> > > image to become very colourful.

> > > 

> > > The decompressor doesn't have the information from DT at that point to

> > > know what are safe locations, so it's up to the boot loader to place

> > > the frame buffer somewhere out of the way.  (If people want to write

> > > a DT parser in position independent ARM assembly code that may change.)

> > > 

> > > As long as people realise this, then it's not a problem, but given the

> > > number of problems that we've already encountered with boot loaders and

> > > memory space layout, I don't trust them to get this right.

> > > 

> > > Right now, the ARM kernel booting document requires:

> > > 

> > > - Quiesce all DMA capable devices so that memory does not get

> > >   corrupted by bogus network packets or disk data. This will save

> > >   you many hours of debug.

> > > 

> > > so we would need to modify that to make an exception for LCD controllers.

> > > However, we definitely can't have devices left enabled which are capable

> > > of writing to system memory, or which changing system memory is likely

> > > to cause bad effects (eg, packet ring buffers, USB buffers etc, which is

> > > really what the above requirement is about.)

> > 

> > Well, LCD was just an example here. But yeah, it is one of the most

> > probable case we have.

> > 

> > So, this thing is already working for sure, of course with some out of

> > tree hacks. Every smart phone shows their company's logo (some kind of

> > flash) while the phone boots. How do they get around such issues?

> 

> As far as the memory being used goes, they probably locate the frame

> buffer well away from the kernel or any area that the kernel is likely

> to use during decompression.

> 

> It's probably also marked as a reserved memory region in DT to avoid

> the kernel touching it during boot, or _maybe_ they just locate it

> somewhere in memory that they've tested that the kernel doesn't touch

> until after their kernel has initialised the LCD controller (thereby

> avoiding the memory being permanently consumed.)

> 


Yes. The display controller is typically pointed to a memory
carveout that we treat as reserved in the kernel. I'm fairly
certain that we avoid the "permanently consumed" problem by
making it a carveout for the display controller, so that when the
display controller probes it can take ownership of the memory
from the bootloader.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Viresh Kumar June 30, 2017, 3:16 a.m. | #9
On 29-06-17, 15:06, Enrico Weigelt, metux IT consult wrote:
> On 29.06.2017 14:47, Viresh Kumar wrote:

> 

> >No. Drivers are registered to the kernel (randomly, though we can know

> >their order) and devices are registered separately (platform/amba

> >devices get registered automatically with DT, hint:

> >drivers/of/platform.c). The device core checks while registering

> >devices/drivers if their drivers/devices are available or not. If

> >yes, then the devices are probed using the drivers. Now the drivers

> >must make sure all the dependencies are met at this point, else they

> >can return -EPROBE_DEFER and the kernel will try probing them again.

> 

> Could we somehow introduce an strict ordering ?


The problem I am trying to solve isn't really related to ordering.

Consider this for example:

A supply shared between LCD and I2C controller (Not sure if such
configurations are there in any of the hardware we have), where the
same I2C controller is used to access the LCD controller's registers.
Both are enabled at boot and the supply is configured to satisfy both.
If the voltage requirements of the I2C controller are below that of
LCD, then we can't decide on which one to probe first. We can't probe
LCD first as its bus isn't active yt and if we probe I2C first, then
it may take the supply down to a level that isn't acceptable for the
LCD (which was on from boot).

> Maybe by letting the device core know of the dependencies, before

> individual probe()'s explicitly ask for them ?


That's what we are sorting out in probe() and I am not sure if we need
any more intelligence on that. Though, you may want to look at the
"functional dependency" stuff, which can be of some help in such
cases. Its mentioned in cover-letter as well.

> >This should happen in probe, otherwise we are screwed.

> 

> Yes, but the probe result may be deferred, so it's tried again in the

> next round. Correct ?


Right.

> >But the kernel doesn't know how it is configured, there can be so many

> >configurable parameters. The kernel needs to do it again by itself.

> 

> Could it read back the config ?


First, it may not always be possible to do that. And even if the
kernel reads it all well, then it wouldn't know why things are
configured the way they are. And trying to read the config in drivers
is going to be so so hacky, that we wouldn't want to do it anyway. We
need a clean way of doing this, so that the kernel knows of what's
going on and that's what this series is targeting here.

> By the way: I've got a similar problem w/ gpmc right now: uboot already

> sets it up, but the kernel only knows about one CS (for the nand) and

> screwes up the others (eg. fpga), so it cant access the fpga . Until

> I've sorted out all the parameters for DT (unfortunately, only have the

> raw register values), I'll have to rely on an userland test program

> to set it all up ...

> 

> >Let me try with an example. A regulator is shared between LCD and DMA

> >controller.

> >

> >Operable ranges of the regulator: 1.8 - 3.0 V

> >Range required by LCD: 2.0 - 3.0 V

> >Range required by DMA: 1.8 - 2.5 V

> 

> Would a config readback help here ?

> 

> The regulator core then should know that we're already in proper

> range for DMA and no need to touch the regulator.


No body is going to allow that kind of hacky code to get merged :)

-- 
viresh
Chen-Yu Tsai June 30, 2017, 3:33 a.m. | #10
On Fri, Jun 30, 2017 at 11:16 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 29-06-17, 15:06, Enrico Weigelt, metux IT consult wrote:

>> On 29.06.2017 14:47, Viresh Kumar wrote:

>>

>> >No. Drivers are registered to the kernel (randomly, though we can know

>> >their order) and devices are registered separately (platform/amba

>> >devices get registered automatically with DT, hint:

>> >drivers/of/platform.c). The device core checks while registering

>> >devices/drivers if their drivers/devices are available or not. If

>> >yes, then the devices are probed using the drivers. Now the drivers

>> >must make sure all the dependencies are met at this point, else they

>> >can return -EPROBE_DEFER and the kernel will try probing them again.

>>

>> Could we somehow introduce an strict ordering ?

>

> The problem I am trying to solve isn't really related to ordering.

>

> Consider this for example:

>

> A supply shared between LCD and I2C controller (Not sure if such

> configurations are there in any of the hardware we have), where the

> same I2C controller is used to access the LCD controller's registers.

> Both are enabled at boot and the supply is configured to satisfy both.

> If the voltage requirements of the I2C controller are below that of

> LCD, then we can't decide on which one to probe first. We can't probe

> LCD first as its bus isn't active yt and if we probe I2C first, then

> it may take the supply down to a level that isn't acceptable for the

> LCD (which was on from boot).


AFAIK regulator constraints are supposed to satisfy all users of it.

>> Maybe by letting the device core know of the dependencies, before

>> individual probe()'s explicitly ask for them ?

>

> That's what we are sorting out in probe() and I am not sure if we need

> any more intelligence on that. Though, you may want to look at the

> "functional dependency" stuff, which can be of some help in such

> cases. Its mentioned in cover-letter as well.

>

>> >This should happen in probe, otherwise we are screwed.

>>

>> Yes, but the probe result may be deferred, so it's tried again in the

>> next round. Correct ?

>

> Right.

>

>> >But the kernel doesn't know how it is configured, there can be so many

>> >configurable parameters. The kernel needs to do it again by itself.

>>

>> Could it read back the config ?

>

> First, it may not always be possible to do that. And even if the

> kernel reads it all well, then it wouldn't know why things are

> configured the way they are. And trying to read the config in drivers

> is going to be so so hacky, that we wouldn't want to do it anyway. We

> need a clean way of doing this, so that the kernel knows of what's

> going on and that's what this series is targeting here.

>

>> By the way: I've got a similar problem w/ gpmc right now: uboot already

>> sets it up, but the kernel only knows about one CS (for the nand) and

>> screwes up the others (eg. fpga), so it cant access the fpga . Until

>> I've sorted out all the parameters for DT (unfortunately, only have the

>> raw register values), I'll have to rely on an userland test program

>> to set it all up ...

>>

>> >Let me try with an example. A regulator is shared between LCD and DMA

>> >controller.

>> >

>> >Operable ranges of the regulator: 1.8 - 3.0 V

>> >Range required by LCD: 2.0 - 3.0 V

>> >Range required by DMA: 1.8 - 2.5 V


So for the example here, the regulator constraint should be 2.5 - 3.0 V,
or the intersection of all voltage requirements.

ChenYu

>>

>> Would a config readback help here ?

>>

>> The regulator core then should know that we're already in proper

>> range for DMA and no need to touch the regulator.

>

> No body is going to allow that kind of hacky code to get merged :)

>

> --

> viresh

>

> _______________________________________________

> linux-arm-kernel mailing list

> linux-arm-kernel@lists.infradead.org

> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Viresh Kumar June 30, 2017, 3:55 a.m. | #11
On 30-06-17, 11:33, Chen-Yu Tsai wrote:
> AFAIK regulator constraints are supposed to satisfy all users of it.


Right.

> >> >Let me try with an example. A regulator is shared between LCD and DMA

> >> >controller.

> >> >

> >> >Operable ranges of the regulator: 1.8 - 3.0 V

> >> >Range required by LCD: 2.0 - 3.0 V

> >> >Range required by DMA: 1.8 - 2.5 V

> 

> So for the example here, the regulator constraint should be 2.5 - 3.0 V,

> or the intersection of all voltage requirements.


Had a look at regulator_check_consumers() and the range selected by it
is the *highest* min_uV and *lowest* max_uV, to find that intersection
point.

For LCD: min_uV = 2.0 V, max_uV = 3.0 V
For DMA: min_uV = 1.8 V, max_uV = 2.5 V

Highest min_uV = 2.0 V
Lowest max_uV = 2.5 V

And so I mentioned the regulator's final range (that satisfies all
consumers) is 2 - 2.5 V.

Why do you say it should be 2.5 - 3.0 V ?

-- 
viresh
Chen-Yu Tsai June 30, 2017, 4:05 a.m. | #12
On Fri, Jun 30, 2017 at 11:55 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 30-06-17, 11:33, Chen-Yu Tsai wrote:

>> AFAIK regulator constraints are supposed to satisfy all users of it.

>

> Right.

>

>> >> >Let me try with an example. A regulator is shared between LCD and DMA

>> >> >controller.

>> >> >

>> >> >Operable ranges of the regulator: 1.8 - 3.0 V

>> >> >Range required by LCD: 2.0 - 3.0 V

>> >> >Range required by DMA: 1.8 - 2.5 V

>>

>> So for the example here, the regulator constraint should be 2.5 - 3.0 V,

>> or the intersection of all voltage requirements.

>

> Had a look at regulator_check_consumers() and the range selected by it

> is the *highest* min_uV and *lowest* max_uV, to find that intersection

> point.

>

> For LCD: min_uV = 2.0 V, max_uV = 3.0 V

> For DMA: min_uV = 1.8 V, max_uV = 2.5 V

>

> Highest min_uV = 2.0 V

> Lowest max_uV = 2.5 V

>

> And so I mentioned the regulator's final range (that satisfies all

> consumers) is 2 - 2.5 V.

>

> Why do you say it should be 2.5 - 3.0 V ?


You are right. It should be 2.0 - 2.5 V. Haven't had my coffee this
morning. :(

I also want to mention that for DT based platforms, this constraint
should already be set in the device tree for the regulator, so the
scenario where DMA comes up and sets a voltage level that LCD cannot
use should not even be possible.

ChenYu
Viresh Kumar June 30, 2017, 4:12 a.m. | #13
On 30-06-17, 12:05, Chen-Yu Tsai wrote:
> On Fri, Jun 30, 2017 at 11:55 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:

> > On 30-06-17, 11:33, Chen-Yu Tsai wrote:

> >> AFAIK regulator constraints are supposed to satisfy all users of it.

> >

> > Right.

> >

> >> >> >Let me try with an example. A regulator is shared between LCD and DMA

> >> >> >controller.

> >> >> >

> >> >> >Operable ranges of the regulator: 1.8 - 3.0 V

> >> >> >Range required by LCD: 2.0 - 3.0 V

> >> >> >Range required by DMA: 1.8 - 2.5 V

> >>

> >> So for the example here, the regulator constraint should be 2.5 - 3.0 V,

> >> or the intersection of all voltage requirements.

> >

> > Had a look at regulator_check_consumers() and the range selected by it

> > is the *highest* min_uV and *lowest* max_uV, to find that intersection

> > point.

> >

> > For LCD: min_uV = 2.0 V, max_uV = 3.0 V

> > For DMA: min_uV = 1.8 V, max_uV = 2.5 V

> >

> > Highest min_uV = 2.0 V

> > Lowest max_uV = 2.5 V

> >

> > And so I mentioned the regulator's final range (that satisfies all

> > consumers) is 2 - 2.5 V.

> >

> > Why do you say it should be 2.5 - 3.0 V ?

> 

> You are right. It should be 2.0 - 2.5 V. Haven't had my coffee this

> morning. :(


And I was worrying if I had something else in my coffee :)

> I also want to mention that for DT based platforms, this constraint

> should already be set in the device tree for the regulator, so the

> scenario where DMA comes up and sets a voltage level that LCD cannot

> use should not even be possible.


Yes, such constraints are already present. But the problem (this
series is trying to solve) is that the kernel doesn't know if the LCD
is already powered ON. And so when DMA gets probed first, the kernel
thinks that DMA is the only user of the regulator and the voltage is
set to 1.8-2.5 V. And so this series is somehow trying to make the
kernel aware about the constraints of the LCD controller which was
enabled in the bootloader.

-- 
viresh
Chen-Yu Tsai June 30, 2017, 4:22 a.m. | #14
On Fri, Jun 30, 2017 at 12:12 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 30-06-17, 12:05, Chen-Yu Tsai wrote:

>> On Fri, Jun 30, 2017 at 11:55 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:

>> > On 30-06-17, 11:33, Chen-Yu Tsai wrote:

>> >> AFAIK regulator constraints are supposed to satisfy all users of it.

>> >

>> > Right.

>> >

>> >> >> >Let me try with an example. A regulator is shared between LCD and DMA

>> >> >> >controller.

>> >> >> >

>> >> >> >Operable ranges of the regulator: 1.8 - 3.0 V

>> >> >> >Range required by LCD: 2.0 - 3.0 V

>> >> >> >Range required by DMA: 1.8 - 2.5 V

>> >>

>> >> So for the example here, the regulator constraint should be 2.5 - 3.0 V,

>> >> or the intersection of all voltage requirements.

>> >

>> > Had a look at regulator_check_consumers() and the range selected by it

>> > is the *highest* min_uV and *lowest* max_uV, to find that intersection

>> > point.

>> >

>> > For LCD: min_uV = 2.0 V, max_uV = 3.0 V

>> > For DMA: min_uV = 1.8 V, max_uV = 2.5 V

>> >

>> > Highest min_uV = 2.0 V

>> > Lowest max_uV = 2.5 V

>> >

>> > And so I mentioned the regulator's final range (that satisfies all

>> > consumers) is 2 - 2.5 V.

>> >

>> > Why do you say it should be 2.5 - 3.0 V ?

>>

>> You are right. It should be 2.0 - 2.5 V. Haven't had my coffee this

>> morning. :(

>

> And I was worrying if I had something else in my coffee :)

>

>> I also want to mention that for DT based platforms, this constraint

>> should already be set in the device tree for the regulator, so the

>> scenario where DMA comes up and sets a voltage level that LCD cannot

>> use should not even be possible.

>

> Yes, such constraints are already present. But the problem (this

> series is trying to solve) is that the kernel doesn't know if the LCD

> is already powered ON. And so when DMA gets probed first, the kernel

> thinks that DMA is the only user of the regulator and the voltage is

> set to 1.8-2.5 V. And so this series is somehow trying to make the

> kernel aware about the constraints of the LCD controller which was

> enabled in the bootloader.


What I'm saying is for the DT case, the constraints are already limited
to the intersection of all users, regardless of whether they are turned
on or not. At least this is what I believe makes sense. You really don't
want to set a regulator such that it over voltages for a subset of its
consumers. Consumers might not have proper power isolation for this.

I think what you mean is that the DT constraints are the union of all
consumer constraints (1.8 - 3.0 V in this case), then each consumer
comes in and adds its own constraints. And for such a design, the kernel
needs to know which and what constraints to apply.

Either way regulators already support constraints, so they are easier
to deal with. Clocks on the other hand, while the core does support
clock rate constraints, AFAIK no one really uses or supports them.

ChenYu
Viresh Kumar June 30, 2017, 5:12 a.m. | #15
On 30-06-17, 12:22, Chen-Yu Tsai wrote:
> On Fri, Jun 30, 2017 at 12:12 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:

> > On 30-06-17, 12:05, Chen-Yu Tsai wrote:


> >> I also want to mention that for DT based platforms, this constraint

> >> should already be set in the device tree for the regulator, so the

> >> scenario where DMA comes up and sets a voltage level that LCD cannot

> >> use should not even be possible.

> 

> What I'm saying is for the DT case, the constraints are already limited

> to the intersection of all users, regardless of whether they are turned

> on or not.


Right, but someone needs to get the regulator first to have that
considered by the regulator core while deciding the final range.

Both DMA and LCD driver do regulator_get() for their devices but if
only DMA driver is probed until now, then the regulator core wouldn't
consider LCD as regulator_get() is never called for LCD.

> I think what you mean is that the DT constraints are the union of all

> consumer constraints (1.8 - 3.0 V in this case), then each consumer

> comes in and adds its own constraints. And for such a design, the kernel

> needs to know which and what constraints to apply.


Sorry, I am confused with what you just said and not sure if I
understand it completely.

Each consumer DT node will have its own set of constraints for the
regulator device. The kernel will do regulator_get() for them one by
one, based on when their drivers get probed. And an intersection of
those constraints (which already did regulator_get()) will be used by
the regulator core.

Now this series is saying that even if the driver didn't come up (for
LCD) and haven't done its regulator_get() yet, consider that device's
constraint while calculating the target voltage for the regulator.

> Either way regulators already support constraints, so they are easier

> to deal with. Clocks on the other hand, while the core does support

> clock rate constraints, AFAIK no one really uses or supports them.


Yeah, so I started with just regulators and that's when Mark suggested
to do something generic which can be reused by other resource types.
We may end up covering clk for sure I believe. Not sure yet about
other resource types though.

-- 
viresh
Chen-Yu Tsai June 30, 2017, 6:36 a.m. | #16
= On Fri, Jun 30, 2017 at 1:12 PM, Viresh Kumar
<viresh.kumar@linaro.org> wrote:
> On 30-06-17, 12:22, Chen-Yu Tsai wrote:

>> On Fri, Jun 30, 2017 at 12:12 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:

>> > On 30-06-17, 12:05, Chen-Yu Tsai wrote:

>

>> >> I also want to mention that for DT based platforms, this constraint

>> >> should already be set in the device tree for the regulator, so the

>> >> scenario where DMA comes up and sets a voltage level that LCD cannot

>> >> use should not even be possible.

>>

>> What I'm saying is for the DT case, the constraints are already limited

>> to the intersection of all users, regardless of whether they are turned

>> on or not.

>

> Right, but someone needs to get the regulator first to have that

> considered by the regulator core while deciding the final range.


AFAIK the regulator core automatically corrects any voltages outside
its constraints when the regulator is first registered. This is
independent of any consumer constraints.

> Both DMA and LCD driver do regulator_get() for their devices but if

> only DMA driver is probed until now, then the regulator core wouldn't

> consider LCD as regulator_get() is never called for LCD.

>

>> I think what you mean is that the DT constraints are the union of all

>> consumer constraints (1.8 - 3.0 V in this case), then each consumer

>> comes in and adds its own constraints. And for such a design, the kernel

>> needs to know which and what constraints to apply.

>

> Sorry, I am confused with what you just said and not sure if I

> understand it completely.

>

> Each consumer DT node will have its own set of constraints for the

> regulator device. The kernel will do regulator_get() for them one by

> one, based on when their drivers get probed. And an intersection of

> those constraints (which already did regulator_get()) will be used by

> the regulator core.


No. In the device tree, the only constraints (per the current state
of the bindings) is for the regulator supply. Any consumer constraints
are programmed purely by the driver, by using regulator_set_voltage().
All of them are considered by the core before setting the real voltage.

> Now this series is saying that even if the driver didn't come up (for

> LCD) and haven't done its regulator_get() yet, consider that device's

> constraint while calculating the target voltage for the regulator.


What I'm saying is that, for the constraints in the regulator supply node,
you would have already considered all consumer constraints. If one of its
consumers can't take power above 2.5 V, surely you don't want the regulator
sending power above that, so you would have

    regulator-max-microvolt = <2500000>;

for that regulator node. You would do something similar for the lower
limit of the voltage range.

You don't even need any actual consumers. Using regulator-min-microvolt,
regulator-max-microvolt, and regulator-always-on, you can have a regulator
provide power at a suitable voltage. We do this for various power rails
on Allwinner SoCs that don't really have proper consumers, like power
for internal logic, PLL, and others. I'm not saying this is a good solution,
because you lose runtime control of the regulator. It's just something
we came up with in lieu of any proper consumers.

>> Either way regulators already support constraints, so they are easier

>> to deal with. Clocks on the other hand, while the core does support

>> clock rate constraints, AFAIK no one really uses or supports them.

>

> Yeah, so I started with just regulators and that's when Mark suggested

> to do something generic which can be reused by other resource types.

> We may end up covering clk for sure I believe. Not sure yet about

> other resource types though.


This might be unrelated, but I think it is a similar problem. When a
clk rate change is propagated up the clk tree, any affected sibling
clks aren't automatically readjusted, i.e. try to keep roughly the
same output clk rate by adjusting its own dividers. This might be
one side of the problem you are trying to solve.

ChenYu
Viresh Kumar June 30, 2017, 8:43 a.m. | #17
On 30-06-17, 14:36, Chen-Yu Tsai wrote:
>  = On Fri, Jun 30, 2017 at 1:12 PM, Viresh Kumar

> <viresh.kumar@linaro.org> wrote:


> > Right, but someone needs to get the regulator first to have that

> > considered by the regulator core while deciding the final range.

> 

> AFAIK the regulator core automatically corrects any voltages outside

> its constraints when the regulator is first registered. This is

> independent of any consumer constraints.


Right, so the kernel checks if the current voltage value set for the
supply is within valid range as per the constraints present in
regulator node.

> > Both DMA and LCD driver do regulator_get() for their devices but if

> > only DMA driver is probed until now, then the regulator core wouldn't

> > consider LCD as regulator_get() is never called for LCD.

> >

> >> I think what you mean is that the DT constraints are the union of all

> >> consumer constraints (1.8 - 3.0 V in this case), then each consumer

> >> comes in and adds its own constraints. And for such a design, the kernel

> >> needs to know which and what constraints to apply.

> >

> > Sorry, I am confused with what you just said and not sure if I

> > understand it completely.

> >

> > Each consumer DT node will have its own set of constraints for the

> > regulator device. The kernel will do regulator_get() for them one by

> > one, based on when their drivers get probed. And an intersection of

> > those constraints (which already did regulator_get()) will be used by

> > the regulator core.

> 

> No. In the device tree, the only constraints (per the current state

> of the bindings) is for the regulator supply. Any consumer constraints

> are programmed purely by the driver, by using regulator_set_voltage().

> All of them are considered by the core before setting the real voltage.


That's right. I had a bit of misunderstanding here. By default min/max
for the consumers is set to zero and they are ignored in
regulator_check_consumers().

> > Now this series is saying that even if the driver didn't come up (for

> > LCD) and haven't done its regulator_get() yet, consider that device's

> > constraint while calculating the target voltage for the regulator.

> 

> What I'm saying is that, for the constraints in the regulator supply node,

> you would have already considered all consumer constraints.


Yes, so whatever voltage the bootloader has programmed for LCD should
fit within constraints present in supply's node.

> If one of its

> consumers can't take power above 2.5 V, surely you don't want the regulator

> sending power above that, so you would have

> 

>     regulator-max-microvolt = <2500000>;

>

> for that regulator node. You would do something similar for the lower

> limit of the voltage range.

> 


I am no regulators expert, but AFAIU that's not true. What's wrong
with the following scenario:

Operable ranges of the regulator: 1.8 - 3.0 V
Range required by LCD: 2.0 - 3.0 V
Range required by DMA: 1.8 - 2.5 V

Here DMA can't work with regulator voltages > 2.5 V, but regulator can
go max to 3.0 V. Of course if the DMA driver has done
regulator_set_voltage(), then we will be within 2.5 V range. But that
doesn't force us to have regulator-max-microvolt set to 2.5 V.

And so the DT node shall have this:

        regulator-min-microvolt = <1800000>;
        regulator-max-microvolt = <3000000>;

Isn't it ?

> This might be unrelated, but I think it is a similar problem. When a

> clk rate change is propagated up the clk tree, any affected sibling

> clks aren't automatically readjusted, i.e. try to keep roughly the

> same output clk rate by adjusting its own dividers. This might be

> one side of the problem you are trying to solve.


I am not sure how this problem can be solved with what this set is
proposing. :(

-- 
viresh
Mark Brown June 30, 2017, 12:10 p.m. | #18
On Fri, Jun 30, 2017 at 02:13:30PM +0530, Viresh Kumar wrote:
> On 30-06-17, 14:36, Chen-Yu Tsai wrote:


> Operable ranges of the regulator: 1.8 - 3.0 V

> Range required by LCD: 2.0 - 3.0 V

> Range required by DMA: 1.8 - 2.5 V


> Here DMA can't work with regulator voltages > 2.5 V, but regulator can

> go max to 3.0 V. Of course if the DMA driver has done

> regulator_set_voltage(), then we will be within 2.5 V range. But that

> doesn't force us to have regulator-max-microvolt set to 2.5 V.


> And so the DT node shall have this:


>         regulator-min-microvolt = <1800000>;

>         regulator-max-microvolt = <3000000>;


> Isn't it ?


If the DMA can't tolerate more than 2.5V then why would the constraints
allow the voltage to float that far?  Similarly on the low end?

Please remember that devices shouldn't be managing their voltages unless
they are actively changing them at runtime, simply setting them at
startup is the job of the constraints.  I would be very surprised to see
a DMA controller doing anything like DVFS.

> 

> > This might be unrelated, but I think it is a similar problem. When a

> > clk rate change is propagated up the clk tree, any affected sibling

> > clks aren't automatically readjusted, i.e. try to keep roughly the

> > same output clk rate by adjusting its own dividers. This might be

> > one side of the problem you are trying to solve.

> 

> I am not sure how this problem can be solved with what this set is

> proposing. :(

> 

> -- 

> viresh
Mark Brown June 30, 2017, 12:12 p.m. | #19
On Fri, Jun 30, 2017 at 12:22:13PM +0800, Chen-Yu Tsai wrote:

> What I'm saying is for the DT case, the constraints are already limited

> to the intersection of all users, regardless of whether they are turned

> on or not. At least this is what I believe makes sense. You really don't

> want to set a regulator such that it over voltages for a subset of its

> consumers. Consumers might not have proper power isolation for this.


Right, and we also shouldn't have voltage setting code in consumers that
don't need to vary their voltage at runtime.  This keeps complexity out
of drivers and avoids the need to handle things like variants that have
different power requirements.

> I think what you mean is that the DT constraints are the union of all

> consumer constraints (1.8 - 3.0 V in this case), then each consumer

> comes in and adds its own constraints. And for such a design, the kernel

> needs to know which and what constraints to apply.


That's the broad idea.
Viresh Kumar July 3, 2017, 6:15 a.m. | #20
On 30-06-17, 13:10, Mark Brown wrote:
> On Fri, Jun 30, 2017 at 02:13:30PM +0530, Viresh Kumar wrote:

> > On 30-06-17, 14:36, Chen-Yu Tsai wrote:

> 

> > Operable ranges of the regulator: 1.8 - 3.0 V

> > Range required by LCD: 2.0 - 3.0 V

> > Range required by DMA: 1.8 - 2.5 V

> 

> > Here DMA can't work with regulator voltages > 2.5 V, but regulator can

> > go max to 3.0 V. Of course if the DMA driver has done

> > regulator_set_voltage(), then we will be within 2.5 V range. But that

> > doesn't force us to have regulator-max-microvolt set to 2.5 V.

> 

> > And so the DT node shall have this:

> 

> >         regulator-min-microvolt = <1800000>;

> >         regulator-max-microvolt = <3000000>;

> 

> > Isn't it ?

> 

> If the DMA can't tolerate more than 2.5V then why would the constraints

> allow the voltage to float that far?  Similarly on the low end?


The above regulator-min/max-microvolt values I mentioned were for the regulator
device and not what the consumers would request. Yes, DMA will request something
between 1.8 to 2.5 V, but in the above example LCD can request from 2.0 to 3.0 V
and so I had those limits for the regulator device (in DT).

> Please remember that devices shouldn't be managing their voltages unless

> they are actively changing them at runtime, simply setting them at

> startup is the job of the constraints.  I would be very surprised to see

> a DMA controller doing anything like DVFS.


Sure, DMA would most likely set a constraint from probe. Maybe I could have used
MMC in the above example, which may actually do DVFS at runtime.

-- 
viresh
Mark Brown July 3, 2017, 3:07 p.m. | #21
On Mon, Jul 03, 2017 at 11:45:52AM +0530, Viresh Kumar wrote:
> On 30-06-17, 13:10, Mark Brown wrote:

> > On Fri, Jun 30, 2017 at 02:13:30PM +0530, Viresh Kumar wrote:

> > > On 30-06-17, 14:36, Chen-Yu Tsai wrote:


> > > And so the DT node shall have this:


> > >         regulator-min-microvolt = <1800000>;

> > >         regulator-max-microvolt = <3000000>;


> > > Isn't it ?


> > If the DMA can't tolerate more than 2.5V then why would the constraints

> > allow the voltage to float that far?  Similarly on the low end?


> The above regulator-min/max-microvolt values I mentioned were for the regulator

> device and not what the consumers would request. Yes, DMA will request something


If you're putting the maximum possible range that the physical regulator
can supply into machine constraints then you really haven't understood
what machine constraints are at all.

> > Please remember that devices shouldn't be managing their voltages unless

> > they are actively changing them at runtime, simply setting them at

> > startup is the job of the constraints.  I would be very surprised to see

> > a DMA controller doing anything like DVFS.


> Sure, DMA would most likely set a constraint from probe. Maybe I could have used


No, it really shouldn't.  Please read what I wrote.

> MMC in the above example, which may actually do DVFS at runtime.


Yes, MMC does vary voltage.
Viresh Kumar July 4, 2017, 6:45 a.m. | #22
On 03-07-17, 16:07, Mark Brown wrote:
> On Mon, Jul 03, 2017 at 11:45:52AM +0530, Viresh Kumar wrote:

> > The above regulator-min/max-microvolt values I mentioned were for the regulator

> > device and not what the consumers would request. Yes, DMA will request something

> 

> If you're putting the maximum possible range that the physical regulator

> can supply into machine constraints then you really haven't understood

> what machine constraints are at all.


I wasn't referring to the limits of the physical regulators but the min/max that
the consumers can set on a particular platform.

> No, it really shouldn't.  Please read what I wrote.


Sorry about that. Understood it now.

-- 
viresh
Rob Clark July 5, 2017, 10:07 p.m. | #23
On Thu, Jun 29, 2017 at 5:00 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 06/29, Russell King - ARM Linux wrote:

>> On Thu, Jun 29, 2017 at 08:28:08PM +0530, Viresh Kumar wrote:

>> > On 29-06-17, 13:49, Russell King - ARM Linux wrote:

>> > > The thing that concerns me most about this is that typically the LCD

>> > > controller will be performing DMA to system RAM.

>> > >

>> > > The location of the frame buffer is unknown to the decompressor - and

>> > > as the decompressor self-relocates itself (using purely assembly code),

>> > > it could relocate itself on top of the frame buffer, causing the "nice"

>> > > image to become very colourful.

>> > >

>> > > The decompressor doesn't have the information from DT at that point to

>> > > know what are safe locations, so it's up to the boot loader to place

>> > > the frame buffer somewhere out of the way.  (If people want to write

>> > > a DT parser in position independent ARM assembly code that may change.)

>> > >

>> > > As long as people realise this, then it's not a problem, but given the

>> > > number of problems that we've already encountered with boot loaders and

>> > > memory space layout, I don't trust them to get this right.

>> > >

>> > > Right now, the ARM kernel booting document requires:

>> > >

>> > > - Quiesce all DMA capable devices so that memory does not get

>> > >   corrupted by bogus network packets or disk data. This will save

>> > >   you many hours of debug.

>> > >

>> > > so we would need to modify that to make an exception for LCD controllers.

>> > > However, we definitely can't have devices left enabled which are capable

>> > > of writing to system memory, or which changing system memory is likely

>> > > to cause bad effects (eg, packet ring buffers, USB buffers etc, which is

>> > > really what the above requirement is about.)

>> >

>> > Well, LCD was just an example here. But yeah, it is one of the most

>> > probable case we have.

>> >

>> > So, this thing is already working for sure, of course with some out of

>> > tree hacks. Every smart phone shows their company's logo (some kind of

>> > flash) while the phone boots. How do they get around such issues?

>>

>> As far as the memory being used goes, they probably locate the frame

>> buffer well away from the kernel or any area that the kernel is likely

>> to use during decompression.

>>

>> It's probably also marked as a reserved memory region in DT to avoid

>> the kernel touching it during boot, or _maybe_ they just locate it

>> somewhere in memory that they've tested that the kernel doesn't touch

>> until after their kernel has initialised the LCD controller (thereby

>> avoiding the memory being permanently consumed.)

>>

>

> Yes. The display controller is typically pointed to a memory

> carveout that we treat as reserved in the kernel. I'm fairly

> certain that we avoid the "permanently consumed" problem by

> making it a carveout for the display controller, so that when the

> display controller probes it can take ownership of the memory

> from the bootloader.

>


For aarch64/arm64 booting with EFI, the bootloader passes info about
memory layout to the kernel (including the fact that the framebuffer
is carved out) so kernel doesn't clobber the scanout buffer.  The
non-EFI case, the bootloader should (not that lk does) patch up the
fdt reserved memory node w/ correct address/size.  I think
lk+downstream just relies on luck.

(I have a patch that makes lk insert a framebuffer-simple node, which
I think should also solve it in the non-bootefi case)

BR,
-R
Stephen Boyd July 7, 2017, 10:39 p.m. | #24
On 07/05, Rob Clark wrote:
> On Thu, Jun 29, 2017 at 5:00 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:

> > On 06/29, Russell King - ARM Linux wrote:

> >>

> >> As far as the memory being used goes, they probably locate the frame

> >> buffer well away from the kernel or any area that the kernel is likely

> >> to use during decompression.

> >>

> >> It's probably also marked as a reserved memory region in DT to avoid

> >> the kernel touching it during boot, or _maybe_ they just locate it

> >> somewhere in memory that they've tested that the kernel doesn't touch

> >> until after their kernel has initialised the LCD controller (thereby

> >> avoiding the memory being permanently consumed.)

> >>

> >

> > Yes. The display controller is typically pointed to a memory

> > carveout that we treat as reserved in the kernel. I'm fairly

> > certain that we avoid the "permanently consumed" problem by

> > making it a carveout for the display controller, so that when the

> > display controller probes it can take ownership of the memory

> > from the bootloader.

> >

> 

> For aarch64/arm64 booting with EFI, the bootloader passes info about

> memory layout to the kernel (including the fact that the framebuffer

> is carved out) so kernel doesn't clobber the scanout buffer.  The

> non-EFI case, the bootloader should (not that lk does) patch up the

> fdt reserved memory node w/ correct address/size.  I think

> lk+downstream just relies on luck.

> 


Downstream+lk seems to be doing a carveout with reserved-memory
in DT[1]. The bootloader isn't updating the DT to indicate this,
instead we rely on an agreement between bootloader and kernel's
DT file to have the carveout. Once the display driver configures
things enough to allocate another display buffer it actually
frees the memory back to the system[2]. Most of the code for this
splash screen stuff is in that mdss_mdp_splash_logo.c file.

[1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8996.dtsi?h=rel/msm-4.4#n231
[2] https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/drivers/video/fbdev/msm/mdss_mdp_splash_logo.c?h=rel/msm-4.4#n252

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project