Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] fbdev: radeon: Remove 'struct timeval' usage
From: Dave Airlie @ 2015-06-05  4:40 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Tina Ruchandani, Arnd Bergmann, y2038, Benjamin Herrenschmidt,
	Jean-Christophe Plagniol-Villard, Linux Fbdev development list,
	LKML
In-Reply-To: <556EEC3D.4010608@ti.com>

On 3 June 2015 at 21:59, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>
> On 25/05/15 07:07, Tina Ruchandani wrote:
>> 'struct timeval' uses a 32-bit representation for the
>> seconds field which will overflow in the year 2038 and beyond.
>> This patch replaces the usage of 'struct timeval' with
>> ktime_t which uses a 64-bit time representation and does not
>> suffer from the y2038 problem. This patch is part of a larger
>> effort to remove all instances of 'struct timeval', 'struct
>> timespec', time_t and other 32-bit timekeeping variables
>> from the kernel.
>> The patch also replaces the use of real time (do_gettimeofday)
>> with monotonic time (ktime_get).
>>
>> Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
>> ---
>>  drivers/video/fbdev/aty/radeon_base.c | 29 ++++++++++++++---------------
>>  1 file changed, 14 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
>> index 01237c8..9747e9e 100644
>> --- a/drivers/video/fbdev/aty/radeon_base.c
>> +++ b/drivers/video/fbdev/aty/radeon_base.c
>> @@ -64,6 +64,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/delay.h>
>>  #include <linux/time.h>
>> +#include <linux/ktime.h>
>>  #include <linux/fb.h>
>>  #include <linux/ioport.h>
>>  #include <linux/init.h>
>> @@ -461,8 +462,8 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>>       int hTotal, vTotal, num, denom, m, n;
>>       unsigned long long hz, vclk;
>>       long xtal;
>> -     struct timeval start_tv, stop_tv;
>> -     long total_secs, total_usecs;
>> +     ktime_t start, stop;
>> +     s64 delta;
>>       int i;
>>
>>       /* Ugh, we cut interrupts, bad bad bad, but we want some precision
>> @@ -478,7 +479,7 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>>               if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) = 0)
>>                       break;
>>
>> -     do_gettimeofday(&start_tv);
>> +     start = ktime_get();
>>
>>       for(i=0; i<1000000; i++)
>>               if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) != 0)
>> @@ -487,20 +488,18 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>>       for(i=0; i<1000000; i++)
>>               if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) = 0)
>>                       break;
>> -
>> -     do_gettimeofday(&stop_tv);
>> -
>> +
>> +     stop = ktime_get();
>> +
>>       local_irq_enable();
>>
>> -     total_secs = stop_tv.tv_sec - start_tv.tv_sec;
>> -     if (total_secs > 10)
>> +     delta = ktime_us_delta(stop, start);
>> +
>> +     /* Return -1 if more than 10 seconds have elapsed */
>> +     if (delta > (10*1000000))
>>               return -1;
>> -     total_usecs = stop_tv.tv_usec - start_tv.tv_usec;
>> -     total_usecs += total_secs * 1000000;
>> -     if (total_usecs < 0)
>> -             total_usecs = -total_usecs;
>> -     hz = 1000000/total_usecs;
>> -
>> +     hz = 1000000/delta;

This needs to be on of the do_div family.

Dave.

^ permalink raw reply

* Re: linux-next: Tree for Jun 4 (radeonfb && xfs)
From: Dave Chinner @ 2015-06-04 23:00 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Stephen Rothwell, Linux Fbdev development list,
	Benjamin Herrenschmidt, linux-kernel, xfs, linux-next
In-Reply-To: <5570A3C6.6050509@infradead.org>

On Thu, Jun 04, 2015 at 12:15:18PM -0700, Randy Dunlap wrote:
> On 06/04/15 06:15, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Changes since 20150603:
> > 
> 
> 
> on i386:
> 
> fs/built-in.o: In function `xfs_ifree_cluster':
> xfs_inode.c:(.text+0x1c491a): undefined reference to `__umoddi3'
> 
> drivers/built-in.o: In function `radeonfb_pci_register':
> radeon_base.c:(.text+0x72fe0): undefined reference to `__divdi3'

Fix should be in today's linux-next tree.

-Dave
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply

* Re: [PATCH 00/21] On-demand device registration
From: Alexander Holler @ 2015-06-04 21:03 UTC (permalink / raw)
  To: Rob Clark, Tomeu Vizoso
  Cc: linux-arm-kernel@lists.infradead.org, Mark Rutland,
	Linux Fbdev development list, linux-usb, linux-pwm,
	dri-devel@lists.freedesktop.org, Thierry Reding, linux-i2c,
	linux-clk, moderated list:ARM/S5P EXYNOS AR..., Grant Likely,
	devicetree@vger.kernel.org, linux-pm, linux-gpio, Rob Herring,
	linux-tegra@vger.kernel.org, Dan Williams,
	Stéphane Marchesin, Dmitry Torokhov
In-Reply-To: <CAF6AEGuOP-3sxDQcNOjVmfP4ue6LmY7KFrj9W75aHKuvZpTFMw@mail.gmail.com>

Am 03.06.2015 um 23:12 schrieb Rob Clark:
> On Mon, May 25, 2015 at 10:53 AM, Tomeu Vizoso
> <tomeu.vizoso@collabora.com> wrote:
>> Hello,
>>
>> I have a problem with the panel on my Tegra Chromebook taking longer than
>> expected to be ready during boot (Stéphane Marchesin reported what is
>> basically the same issue in [0]), and have looked into ordered probing as a
>> better way of solving this than moving nodes around in the DT or playing with
>> initcall levels.
>>
>> While reading the thread [1] that Alexander Holler started with his series to
>> make probing order deterministic, it occurred to me that it should be possible
>> to achieve the same by registering devices as they are referenced by other
>> devices.
>>
>> This basically reuses the information that is already implicit in the probe()
>> implementations, saving us from refactoring existing drivers or adding
>> information to DTBs.
>>
>> Something I'm not completely happy with is that I have had to move the call to
>> of_platform_populate after all platform drivers have been registered.
>> Otherwise I don't see how I could register drivers on demand as we don't have
>> yet each driver's compatible strings.
>>
>> For machs that don't move of_platform_populate() to a later point, these
>> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
>> all the deferred probes as some drivers may not be registered yet.
>>
>> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
>> patches were enough to eliminate all the deferred probes.
>>
>> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.
> 
> So, complete drive-by comment (and I won't claim to be a DT expert,
> etc, etc, so take this with a few grains of salt), but why not push
> the problem to the DT compiler (or a pre-process step that could be
> run on existing DT blobs), which generates an optional DT node that is
> the recommended probe order?  That seems like it avoids adding
> complexity into the early boot code (which seems like a good thing)..

I've played with that approach too (as my patches for dtc do contain the
same code I've put into the kernel, but decided that it doesn't make
much sense. The sort algorithm is really small (some dozen lines), very
fast (around 3-5ms on a omap) and might be later used to sort necessary
module loading too. So there would be no advantage to put a sorted list
into the DT. And having the sort algorithm in the kernel, would make it
possible to use it for acpi or something else too, if they manage it to
provide the necessary dependencies.

Regards,

Alexander Holler


^ permalink raw reply

* Re: [PATCH 00/21] On-demand device registration
From: Alexander Holler @ 2015-06-04 20:39 UTC (permalink / raw)
  To: Grygorii.Strashko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	Tomeu Vizoso
  Cc: Rob Herring,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Stéphane Marchesin, Thierry Reding, Dmitry Torokhov,
	Grant Likely, Rob Herring, Mark Rutland, Dan Williams,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, dri-devel,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <556F5C24.1030101-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Am 03.06.2015 um 21:57 schrieb Grygorii.Strashko@linaro.org:

...

> So few comments from above:
> - registering devices later during the System boot may improve boot time.
>   But resolving of all deferred probes may NOT improve boot time ;) 
>   Have you seen smth like this?

If someone is out for boot time reduction, I think one of the best ways
would by making driver initialization parallel. Keep in mind that all
linked in drivers currently are initialized in series.

As it seems to have been forgotten or overread, I've mentioned in my
series of patches last year that, with a few changes, it's possible to
let the algorithm I've used (dfs) to spit out all drivers which can be
initialized in parallel.

But as I'm not paid for the work I've done and just did it out of
curiosity, interest or how ever you want name it, I haven't spend any
more time into that topic, especially as I'm missing the necessary
connections to get patches into the kernel. ;)

But, as said, it's easy (at least if aren't getting panic when it comes
to a bit of algorithm theory) to get a list drivers you can start in
parallel if you have such a complete list of dependencies as DT already
offers. Just look at the pictures generate by dtc (using my patches),
you will see, they already show which drivers can be initialized in
parallel.

So it would be easy to use e.g. all cores already very early at boot to
initialize drivers, not just after init got started. Besides that the
würgaround of defered init (which, btw. leads devs to supress error
messages, which is especially bad if you are searching a problem) isn't
needed anymore if you have a list of dependecies (however you get it,
I've used DT because the dependencies already are all there).

Regards,

Alexander Holler

^ permalink raw reply

* Re: linux-next: Tree for Jun 4 (radeonfb && xfs)
From: Randy Dunlap @ 2015-06-04 19:15 UTC (permalink / raw)
  To: Stephen Rothwell, linux-next
  Cc: linux-kernel, Linux Fbdev development list,
	Benjamin Herrenschmidt, xfs
In-Reply-To: <20150604231545.506332b5@canb.auug.org.au>

On 06/04/15 06:15, Stephen Rothwell wrote:
> Hi all,
> 
> Changes since 20150603:
> 


on i386:

fs/built-in.o: In function `xfs_ifree_cluster':
xfs_inode.c:(.text+0x1c491a): undefined reference to `__umoddi3'

drivers/built-in.o: In function `radeonfb_pci_register':
radeon_base.c:(.text+0x72fe0): undefined reference to `__divdi3'



-- 
~Randy

^ permalink raw reply

* Re: [PATCH 00/21] On-demand device registration
From: Grygorii.Strashko@linaro.org @ 2015-06-04 16:51 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: Mark Rutland, linux-fbdev@vger.kernel.org, Linux USB List,
	Linux PWM List, dri-devel, Thierry Reding,
	linux-i2c@vger.kernel.org, Alexander Holler, linux-clk,
	linux-samsung-soc@vger.kernel.org, Grant Likely,
	devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-gpio@vger.kernel.org, Rob Herring,
	linux-tegra@vger.kernel.org, Dan Williams,
	"linux-arm-kernel@lists.infradead.org" <linux-arm-kern>
In-Reply-To: <CAAObsKAFQrm-maqzadCELi2JVt5jd+yoqCbDpu5OP7gkSk8gcA@mail.gmail.com>

On 06/04/2015 11:39 AM, Tomeu Vizoso wrote:
> On 3 June 2015 at 21:57, Grygorii.Strashko@linaro.org
> <grygorii.strashko@linaro.org> wrote:
>> On 05/28/2015 07:33 AM, Rob Herring wrote:
>>> On Mon, May 25, 2015 at 9:53 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>>> I have a problem with the panel on my Tegra Chromebook taking longer than
>>>> expected to be ready during boot (Stéphane Marchesin reported what is
>>>> basically the same issue in [0]), and have looked into ordered probing as a
>>>> better way of solving this than moving nodes around in the DT or playing with
>>>> initcall levels.
>>>>
>>>> While reading the thread [1] that Alexander Holler started with his series to
>>>> make probing order deterministic, it occurred to me that it should be possible
>>>> to achieve the same by registering devices as they are referenced by other
>>>> devices.
>>>
>>> I like the concept and novel approach.
>>>
>>>> This basically reuses the information that is already implicit in the probe()
>>>> implementations, saving us from refactoring existing drivers or adding
>>>> information to DTBs.
>>>>
>>>> Something I'm not completely happy with is that I have had to move the call to
>>>> of_platform_populate after all platform drivers have been registered.
>>>> Otherwise I don't see how I could register drivers on demand as we don't have
>>>> yet each driver's compatible strings.
>>>
>>> Yeah, this is the opposite of what we'd really like. Ideally, we would
>>> have a solution that works for modules too. However, we're no worse
>>> off. We pretty much build-in dependencies to avoid module ordering
>>> problems.
>>>
>>> Perhaps we need to make the probing on-demand rather than simply on
>>> device<->driver match occurring.
>>>
>>>> For machs that don't move of_platform_populate() to a later point, these
>>>> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
>>>> all the deferred probes as some drivers may not be registered yet.
>>>
>>> Ideally, of_platform_populate is not explicitly called by each
>>> platform. So I think we need to make this work for the default case.
>>>
>>>> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
>>>> patches were enough to eliminate all the deferred probes.
>>>>
>>>> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.
>>>
>>> That's certainly compelling.
>>
>> I've found your idea about moving device registration later during System boot
>> very interesting so I've decided to try it on dra7-evem (TI) :).
>> It's good to know time during Kernel boot when we can assume that all drivers are
>> ready for probing, so there are more ways to control probing order.
> 
> Thanks, though right now I'm following Rob's suggestion and only delay
> probing, not registration. The patch is really simple (applies on
> linux-next, with async probing):
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 8da8e07..7e6b1e1 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -407,6 +407,11 @@ int driver_probe_device(struct device_driver
> *drv, struct device *dev)
>          if (!device_is_registered(dev))
>                  return -ENODEV;
> 
> +       if (!driver_deferred_probe_enable) {
> +               driver_deferred_probe_add(dev);
> +               return 0;
> +       }
> +
>          pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
>                   drv->bus->name, __func__, dev_name(dev), drv->name);
> 
> @@ -585,7 +590,7 @@ EXPORT_SYMBOL_GPL(device_attach);
> 
>   void device_initial_probe(struct device *dev)
>   {
> -       __device_attach(dev, true);
> +       __device_attach(dev, driver_deferred_probe_enable);
>   }
> 
>   static int __driver_attach(struct device *dev, void *data)

Can't boot my 3.14 kernel with this change :( Most probably,
the problem is related to platform_driver_probe() usage :(
Have no time to play with it now :(, but recommend you to check also
earlyprintk, last log message I can see:
[    1.435522] bootconsole [earlycon0] disabled

But, nice try ;) Seems -EPROBE_DEFER is reality of life which
has to be accepted as is.

> 
>> Pls, Note here that TI OMAP2+ mach is not pure DT mach - it's combination of
>> DT and not DT devices/drivers.
>>
>> Ok. So What was done...
>>
>> LKML Linux 4.1-rc3 (a simple case)
>> 1) use your patches 3/4 as reference (only these two patches :)
>> 2) move of_platform_populate later at device_initcall_sync time
>> Boot time reduction ~0.4 sec
> 
> I'm a bit surprised at such a big improvement. May I ask how you are
> measuring it?

Ah. My measurements are not precise. I've just tracking time of message
"[    4.110756] Freeing unused kernel memory: 344K (c0994000 - c09ea000)"

> 
>> TI Android Kernel 3.14 (NOT a simple case)
>> 1) use your patches 3/4 as reference (only these two patches :)
>> 2) move of_platform_populate later at device_initcall_sync time
>> 3) make it to boot (not sure I've fixed all issues, but those which
>>     break the System boot):
>>   - split non-DT and DT devices registration in platform code;
>>   - keep non-DT devices registration from .init_machine() [arch_initcall]
>>   - move DT-devices registration at device_initcall_sync time
>>   - fix drivers which use platform_driver_probe().
>>     Note. Now there are at about ~190 occurrences of this macro in Kernel.
>>   - re-order few devices in DT (4 devices)
>>   - fix one driver which uses of_find_device_by_node() wrongly
>>     Note. This API is used some times with assumption that
>>     requested dev has been probed already.
>> Boot time reduction ~0.3 sec. Probing of some devices are still deferred.
> 
> I got no deferred probes on a pandaboard with just these changes:
> 
> https://git.collabora.com/cgit/user/tomeu/linux.git/commit/?id\x1586c6f50b3d3c65dc219a3cdc3327d798cabca6

As I've mentioned I tried TI Android Kernel 3.14 where all DRA7 SoC features are enabled.
It is very close to production SW. LKML, by default, enables
mostly nothing and not all features are supported. 
In my prev exercise I was able to boot till Android GUI and it has worked :)

Also, cool statistic - really_probe() was called 136 times for me
(successful execution of dev->bus->probe() and drv->probe()).

> 
>> TI Android Kernel 3.14 + of_platform_device_ensure
>> 1) backport of_platform_device_ensure() (also need patches
>>     "of: Introduce device tree node flag helpers" and
>>     "of: Keep track of populated platform devices")
>> 2) back-port all your patches which uses of_platform_device_ensure()
>> 3) make it to boot:
>>     - drop patch dma: of: Probe DMA controllers on demand
>>     - fix deadlock in regulator core:
>>     regulator_dev_lookup() called from regulator_register() in K3.14
>> 4) get rid of deferred probes - add of_platform_device_ensure() calls in:
>>     - drivers/video/fbdev/omap2/dss/output.c
>>     - drivers/extcon/extcon-class.c
>>
>>   Boot time reduction: NONE !?
>>
>> So few comments from above:
>> - registering devices later during the System boot may improve boot time.
>>    But resolving of all deferred probes may NOT improve boot time ;)
>>    Have you seen smth like this?
> 
> Yeah, I don't really expect for on-demand probing to improve boot time
> much in the general case, as drivers tend to first acquire resources
> (and defer probe if needed) and only then access hardware and wait for
> stuff to happen.
> 
> The main advantage of ordered/ondemand probing is that it is much
> easier to see the order in which devices will be bound. In my case
> this is useful because one could get one device (eg. the drm panel) to
> probe very early by just moving that node to the beginning of the DT.
> With deferred probe, one would have to figure out all the dependencies
> and shuffle them around in the DT.
> 
> Another advantage (but not the one why I'm doing this work) is that in
> general a driver's probe will be called only once per device, and if
> it fails then we can be almost certain that something is wrong. This
> should aid in debugging as right now one has to take into account the
> several reasons why a device might defer its probe.
> 
> Depending on what work your drivers do in your platform, enabling
> async probing for all of them may have a noticeable impact though.
> 
>> - usage of of_platform_device_ensure() will require continuous fixing of Kernel :(
> 
> You mean adding those calls to more subsystems?

Exactly. Each time new framework will be introduced or reworked (or even for some drivers),
because each of them implement its own of_get_xxx() API. 

> 
>> - late_initcall is not (as for me not safe) a good time to register devices. A lot
>>    of platforms/subsystems/frameworks perform their final initialization or clean-up
>>    steps, with assumption that System mostly ready to work. For example, CPUIdle/CPUFreq
>>    are allowed and other PM staff. CPUIdle and driver's probing are not friends.
> 
> Yeah, I was expecting to find more problems due to this, but the
> platforms I tested on didn't show any. Do you have pointers to
> concrete issues?

No. This observation comes from my working experience with OMAP4 devices where
lowest possible CPUIdle state was mostly equal to Device-OFF state. 
Fast search has allowed me to find possible source of issues in code - I'm pretty sure
smth. similar can be found in other ARM maches:
- arch/arm/mach-omap2/omap_device.c -> omap_device_late_init().

> 
>> What would be nice to have for now in my opinion:
>> - some useful place to move device population code.
>>    May be machine_desc->init_device_sync() callback.
> 
> I'm looking at leave device registration where it currently is and
> just add devices to the deferred list until we get to late_initcall,
> where we would start to actually probe them. Seems promising for now.
> 
>> - some optional feature in DTC which will allow to re-arrange DT nodes for levels 0(root)
>>    and 1(simple-bus) using standard or widely used bindings (gpio, regulators, clocks, dma,
>>    pinctrl, irqs).
> 
> Could you extend on this, please?

This is actually mostly the same idea as was mentioned by Rob Clark 
http://www.spinics.net/lists/arm-kernel/msg423485.html


For example, mmc1 probe will be deferred always for below DT,
but if we put evm_3v3_sw before ocp and exchange mmc1 and i2c1
- mmc1 will be probed without deferring.
Potentially, It can be done in DTC where we do not have so strict limits
as for Kernel code. 

Soc file:
/ { /* root - level 0 */
	ocp { /* simple-bus - level 1 */
		compatible =  "simple-bus";

		mmc1: mmc@4809c000 {
			compatible = "mmc";
		...
		}

		i2c1: i2c@48070000 {
			compatible = "i2c";
		}
	}
}

board file:
#include "SoC.dtsi"

/ {
	evm_3v3_sw: fixedregulator-evm_3v3_sw {
		compatible = "regulator-fixed";
		regulator-name = "evm_3v3_sw";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
	};
};

&i2c1 {
	pcf_gpio_21: gpio@21 {
		compatible = "ti,pcf8575";
		reg = <0x21>;
		lines-initial-states = <0x1408>;
		gpio-controller;
		#gpio-cells = <2>;
	};
}

&mmc1 {
	status = "okay";
	vmmc-supply = <&evm_3v3_sw>;
	bus-width = <4>;
	cd-gpios = <&pcf_gpio_21 5 0>;
};
	


-- 
regards,
-grygorii

^ permalink raw reply

* [PATCH v4 3/3] video: fbdev: vesafb: use arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-06-04 16:44 UTC (permalink / raw)
  To: tomi.valkeinen
  Cc: plagnioj, bp, hpa, linux-fbdev, luto, toshi.kani, sbsiddha, mingo,
	tglx, jgross, daniel.vetter, airlied, adaplas, robdclark, jg1.han,
	wsa, linux-kernel, Luis R. Rodriguez
In-Reply-To: <1433436288-16792-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This driver uses the same area for MTRR as for the ioremap_wc(), if
anything it just uses a smaller size in case MTRR reservation fails.
ioremap_wc() API is already used to take advantage of architecture
write-combining when available.

Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available.

There are a few motivations for this:

a) Take advantage of PAT when available

b) Help bury MTRR code away, MTRR is architecture specific and on
   x86 its replaced by PAT

c) Help with the goal of eventually using _PAGE_CACHE_UC over
   _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
   de33c442e titled "x86 PAT: fix performance drop for glx,
   use UC minus for ioremap(), ioremap_nocache() and
   pci_mmap_page_range()")

The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.

@ mtrr_found @
expression index, base, size;
@@

-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);

@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@

-mtrr_del(index, base, size);
+arch_phys_wc_del(index);

@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@

-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);

@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@

-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);

@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);

@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);

Generated-by: Coccinelle SmPL
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/vesafb.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index 3db3908..528fe91 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -19,10 +19,9 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/screen_info.h>
+#include <linux/io.h>
 
 #include <video/vga.h>
-#include <asm/io.h>
-#include <asm/mtrr.h>
 
 #define dac_reg	(0x3c8)
 #define dac_val	(0x3c9)
@@ -180,16 +179,10 @@ static int vesafb_setcolreg(unsigned regno, unsigned red, unsigned green,
 
 static void vesafb_destroy(struct fb_info *info)
 {
-#ifdef CONFIG_MTRR
 	struct vesafb_par *par = info->par;
-#endif
 
 	fb_dealloc_cmap(&info->cmap);
-
-#ifdef CONFIG_MTRR
-	if (par->wc_cookie >= 0)
-		mtrr_del(par->wc_cookie, 0, 0);
-#endif
+	arch_phys_wc_del(par->wc_cookie);
 	if (info->screen_base)
 		iounmap(info->screen_base);
 	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
@@ -420,7 +413,6 @@ static int vesafb_probe(struct platform_device *dev)
 	request_region(0x3c0, 32, "vesafb");
 
 	if (mtrr = 3) {
-#ifdef CONFIG_MTRR
 		unsigned int temp_size = size_total;
 
 		/* Find the largest power-of-two */
@@ -428,18 +420,16 @@ static int vesafb_probe(struct platform_device *dev)
 
 		/* Try and find a power of two to add */
 		do {
-			par->wc_cookie = mtrr_add(vesafb_fix.smem_start,
-						  temp_size,
-						  MTRR_TYPE_WRCOMB, 1);
+			par->wc_cookie +				arch_phys_wc_add(vesafb_fix.smem_start,
+						 temp_size);
 			temp_size >>= 1;
-		} while (temp_size >= PAGE_SIZE && par->wc_cookie = -EINVAL);
-#endif
+		} while (temp_size >= PAGE_SIZE && par->wc_cookie < 0);
+
 		info->screen_base = ioremap_wc(vesafb_fix.smem_start, vesafb_fix.smem_len);
 	} else {
-#ifdef CONFIG_MTRR
 		if (mtrr && mtrr != 3)
 			WARN_ONCE(1, "Only MTRR_TYPE_WRCOMB (3) make sense\n");
-#endif
 		info->screen_base = ioremap(vesafb_fix.smem_start, vesafb_fix.smem_len);
 	}
 
@@ -477,10 +467,7 @@ static int vesafb_probe(struct platform_device *dev)
 	fb_info(info, "%s frame buffer device\n", info->fix.id);
 	return 0;
 err:
-#ifdef CONFIG_MTRR
-	if (par->wc_cookie >= 0)
-		mtrr_del(par->wc_cookie, 0, 0);
-#endif
+	arch_phys_wc_del(par->wc_cookie);
 	if (info->screen_base)
 		iounmap(info->screen_base);
 	framebuffer_release(info);
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v4 2/3] video: fbdev: vesafb: add missing mtrr_del() for added MTRR
From: Luis R. Rodriguez @ 2015-06-04 16:44 UTC (permalink / raw)
  To: tomi.valkeinen
  Cc: plagnioj, bp, hpa, linux-fbdev, luto, toshi.kani, sbsiddha, mingo,
	tglx, jgross, daniel.vetter, airlied, adaplas, robdclark, jg1.han,
	wsa, linux-kernel, Luis R. Rodriguez
In-Reply-To: <1433436288-16792-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

The MTRR added was never being deleted, in order to store the
MTRR cookie we need to make use of the private info->par so we
create a struct for this. This driver was already using the extra
space typically used for info->par for the info->pseudo_palette
which typically used stuffed on driver's own private structs
(the respective info->par), so we just move the pseudo_palette
into the private struct as well.

Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/vesafb.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index 191156b..3db3908 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -29,6 +29,11 @@
 
 /* --------------------------------------------------------------------- */
 
+struct vesafb_par {
+	u32 pseudo_palette[256];
+	int wc_cookie;
+};
+
 static struct fb_var_screeninfo vesafb_defined = {
 	.activate	= FB_ACTIVATE_NOW,
 	.height		= -1,
@@ -175,7 +180,16 @@ static int vesafb_setcolreg(unsigned regno, unsigned red, unsigned green,
 
 static void vesafb_destroy(struct fb_info *info)
 {
+#ifdef CONFIG_MTRR
+	struct vesafb_par *par = info->par;
+#endif
+
 	fb_dealloc_cmap(&info->cmap);
+
+#ifdef CONFIG_MTRR
+	if (par->wc_cookie >= 0)
+		mtrr_del(par->wc_cookie, 0, 0);
+#endif
 	if (info->screen_base)
 		iounmap(info->screen_base);
 	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
@@ -228,6 +242,7 @@ static int vesafb_setup(char *options)
 static int vesafb_probe(struct platform_device *dev)
 {
 	struct fb_info *info;
+	struct vesafb_par *par;
 	int i, err;
 	unsigned int size_vmode;
 	unsigned int size_remap;
@@ -291,14 +306,14 @@ static int vesafb_probe(struct platform_device *dev)
 		   spaces our resource handlers simply don't know about */
 	}
 
-	info = framebuffer_alloc(sizeof(u32) * 256, &dev->dev);
+	info = framebuffer_alloc(sizeof(struct vesafb_par), &dev->dev);
 	if (!info) {
 		release_mem_region(vesafb_fix.smem_start, size_total);
 		return -ENOMEM;
 	}
 	platform_set_drvdata(dev, info);
-	info->pseudo_palette = info->par;
-	info->par = NULL;
+	par = info->par;
+	info->pseudo_palette = par->pseudo_palette;
 
 	/* set vesafb aperture size for generic probing */
 	info->apertures = alloc_apertures(1);
@@ -407,17 +422,17 @@ static int vesafb_probe(struct platform_device *dev)
 	if (mtrr = 3) {
 #ifdef CONFIG_MTRR
 		unsigned int temp_size = size_total;
-		int rc;
 
 		/* Find the largest power-of-two */
 		temp_size = roundup_pow_of_two(temp_size);
 
 		/* Try and find a power of two to add */
 		do {
-			rc = mtrr_add(vesafb_fix.smem_start, temp_size,
-				      MTRR_TYPE_WRCOMB, 1);
+			par->wc_cookie = mtrr_add(vesafb_fix.smem_start,
+						  temp_size,
+						  MTRR_TYPE_WRCOMB, 1);
 			temp_size >>= 1;
-		} while (temp_size >= PAGE_SIZE && rc = -EINVAL);
+		} while (temp_size >= PAGE_SIZE && par->wc_cookie = -EINVAL);
 #endif
 		info->screen_base = ioremap_wc(vesafb_fix.smem_start, vesafb_fix.smem_len);
 	} else {
@@ -462,6 +477,10 @@ static int vesafb_probe(struct platform_device *dev)
 	fb_info(info, "%s frame buffer device\n", info->fix.id);
 	return 0;
 err:
+#ifdef CONFIG_MTRR
+	if (par->wc_cookie >= 0)
+		mtrr_del(par->wc_cookie, 0, 0);
+#endif
 	if (info->screen_base)
 		iounmap(info->screen_base);
 	framebuffer_release(info);
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v4 1/3] video: fbdev: vesafb: only support MTRR_TYPE_WRCOMB
From: Luis R. Rodriguez @ 2015-06-04 16:44 UTC (permalink / raw)
  To: tomi.valkeinen
  Cc: plagnioj, bp, hpa, linux-fbdev, luto, toshi.kani, sbsiddha, mingo,
	tglx, jgross, daniel.vetter, airlied, adaplas, robdclark, jg1.han,
	wsa, linux-kernel, Luis R. Rodriguez, Linus Torvalds
In-Reply-To: <1433436288-16792-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

No other video driver uses MTRR types except for MTRR_TYPE_WRCOMB,
the other MTRR types were implemented and supported here but with
no real good reason. The ioremap() APIs are architecture agnostic and
at least on x86 PAT is a new design that extends MTRRs and
can replace it in a much cleaner way, where so long as the
proper ioremap_wc() or variant API is used the right thing will
be done behind the scenes. This is the only driver left using the
other MTRR types -- and since there is no good reason for it now
rip them out.

Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/vesafb.c | 62 ++++++++++++--------------------------------
 1 file changed, 16 insertions(+), 46 deletions(-)

diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index d79a0ac..191156b 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -404,60 +404,30 @@ static int vesafb_probe(struct platform_device *dev)
 	 * region already (FIXME) */
 	request_region(0x3c0, 32, "vesafb");
 
+	if (mtrr = 3) {
 #ifdef CONFIG_MTRR
-	if (mtrr) {
 		unsigned int temp_size = size_total;
-		unsigned int type = 0;
+		int rc;
 
-		switch (mtrr) {
-		case 1:
-			type = MTRR_TYPE_UNCACHABLE;
-			break;
-		case 2:
-			type = MTRR_TYPE_WRBACK;
-			break;
-		case 3:
-			type = MTRR_TYPE_WRCOMB;
-			break;
-		case 4:
-			type = MTRR_TYPE_WRTHROUGH;
-			break;
-		default:
-			type = 0;
-			break;
-		}
-
-		if (type) {
-			int rc;
-
-			/* Find the largest power-of-two */
-			temp_size = roundup_pow_of_two(temp_size);
+		/* Find the largest power-of-two */
+		temp_size = roundup_pow_of_two(temp_size);
 
-			/* Try and find a power of two to add */
-			do {
-				rc = mtrr_add(vesafb_fix.smem_start, temp_size,
-					      type, 1);
-				temp_size >>= 1;
-			} while (temp_size >= PAGE_SIZE && rc = -EINVAL);
-		}
-	}
+		/* Try and find a power of two to add */
+		do {
+			rc = mtrr_add(vesafb_fix.smem_start, temp_size,
+				      MTRR_TYPE_WRCOMB, 1);
+			temp_size >>= 1;
+		} while (temp_size >= PAGE_SIZE && rc = -EINVAL);
 #endif
-	
-	switch (mtrr) {
-	case 1: /* uncachable */
-		info->screen_base = ioremap_nocache(vesafb_fix.smem_start, vesafb_fix.smem_len);
-		break;
-	case 2: /* write-back */
-		info->screen_base = ioremap_cache(vesafb_fix.smem_start, vesafb_fix.smem_len);
-		break;
-	case 3: /* write-combining */
 		info->screen_base = ioremap_wc(vesafb_fix.smem_start, vesafb_fix.smem_len);
-		break;
-	case 4: /* write-through */
-	default:
+	} else {
+#ifdef CONFIG_MTRR
+		if (mtrr && mtrr != 3)
+			WARN_ONCE(1, "Only MTRR_TYPE_WRCOMB (3) make sense\n");
+#endif
 		info->screen_base = ioremap(vesafb_fix.smem_start, vesafb_fix.smem_len);
-		break;
 	}
+
 	if (!info->screen_base) {
 		printk(KERN_ERR
 		       "vesafb: abort, cannot ioremap video memory 0x%x @ 0x%lx\n",
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v4 0/3] vesafb: remove theoretical MTRR uses
From: Luis R. Rodriguez @ 2015-06-04 16:44 UTC (permalink / raw)
  To: tomi.valkeinen
  Cc: plagnioj, bp, hpa, linux-fbdev, luto, toshi.kani, sbsiddha, mingo,
	tglx, jgross, daniel.vetter, airlied, adaplas, robdclark, jg1.han,
	wsa, linux-kernel, Luis R. Rodriguez

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Tomi,

This v4 fixes the issue of the use of the old use of
info->par for the pseudo_palette and our need to extend
it for the MTRR cookie by extending our proposed new
struct vesafb_par with the pseudo_palette and later
pointing info->pseudo_palette to it. This also means
we can simplify the allocation of the info when calling
framebuffer_alloc() by now just passing the size of our
struct vesafb_par.

Patch 1 is unmodified from v3 series, patch 2 does the
fixes mentioned above, and patch 3 might have small
changes to account for contextual changes.

Luis R. Rodriguez (3):
  video: fbdev: vesafb: only support MTRR_TYPE_WRCOMB
  video: fbdev: vesafb: add missing mtrr_del() for added MTRR
  video: fbdev: vesafb: use arch_phys_wc_add()

 drivers/video/fbdev/vesafb.c | 80 ++++++++++++++++----------------------------
 1 file changed, 28 insertions(+), 52 deletions(-)

-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply

* Re: [PATCH v3 2/3] video: fbdev: vesafb: add missing mtrr_del() for added MTRR
From: Luis R. Rodriguez @ 2015-06-04 16:24 UTC (permalink / raw)
  To: cocci
In-Reply-To: <556FE863.9070000@ti.com>

On Wed, Jun 3, 2015 at 10:55 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>
> On 20/05/15 22:46, Luis R. Rodriguez wrote:
>> On Wed, May 20, 2015 at 12:52:14PM +0300, Tomi Valkeinen wrote:
>
>>> Hmm, this looks a bit odd... You're removing the pseudo_palette, and
>>> using its memory for mtrr cookie?
>>
>> You are totally right, sorry, will spin a v3.
>
> Did you ever send a new version (v4 actually), or have I somehow missed it?

Crap no, sorry about that, will send it out right away.

 Luis

^ permalink raw reply

* Re: [PATCH 00/21] On-demand device registration
From: Tomeu Vizoso @ 2015-06-04  8:39 UTC (permalink / raw)
  To: Grygorii.Strashko@linaro.org
  Cc: Mark Rutland, linux-fbdev@vger.kernel.org, Dmitry Torokhov,
	devicetree@vger.kernel.org, dri-devel, linux-i2c@vger.kernel.org,
	Alexander Holler, linux-clk, linux-samsung-soc@vger.kernel.org,
	Grant Likely, Linux PWM List, linux-pm@vger.kernel.org,
	linux-gpio@vger.kernel.org, Rob Herring,
	linux-tegra@vger.kernel.org, Dan Williams,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <556F5C24.1030101@linaro.org>

On 3 June 2015 at 21:57, Grygorii.Strashko@linaro.org
<grygorii.strashko@linaro.org> wrote:
> Hi Tomeu,
>
> On 05/28/2015 07:33 AM, Rob Herring wrote:
>> On Mon, May 25, 2015 at 9:53 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>> I have a problem with the panel on my Tegra Chromebook taking longer than
>>> expected to be ready during boot (Stéphane Marchesin reported what is
>>> basically the same issue in [0]), and have looked into ordered probing as a
>>> better way of solving this than moving nodes around in the DT or playing with
>>> initcall levels.
>>>
>>> While reading the thread [1] that Alexander Holler started with his series to
>>> make probing order deterministic, it occurred to me that it should be possible
>>> to achieve the same by registering devices as they are referenced by other
>>> devices.
>>
>> I like the concept and novel approach.
>>
>>> This basically reuses the information that is already implicit in the probe()
>>> implementations, saving us from refactoring existing drivers or adding
>>> information to DTBs.
>>>
>>> Something I'm not completely happy with is that I have had to move the call to
>>> of_platform_populate after all platform drivers have been registered.
>>> Otherwise I don't see how I could register drivers on demand as we don't have
>>> yet each driver's compatible strings.
>>
>> Yeah, this is the opposite of what we'd really like. Ideally, we would
>> have a solution that works for modules too. However, we're no worse
>> off. We pretty much build-in dependencies to avoid module ordering
>> problems.
>>
>> Perhaps we need to make the probing on-demand rather than simply on
>> device<->driver match occurring.
>>
>>> For machs that don't move of_platform_populate() to a later point, these
>>> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
>>> all the deferred probes as some drivers may not be registered yet.
>>
>> Ideally, of_platform_populate is not explicitly called by each
>> platform. So I think we need to make this work for the default case.
>>
>>> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
>>> patches were enough to eliminate all the deferred probes.
>>>
>>> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.
>>
>> That's certainly compelling.
>
> I've found your idea about moving device registration later during System boot
> very interesting so I've decided to try it on dra7-evem (TI) :).
> It's good to know time during Kernel boot when we can assume that all drivers are
> ready for probing, so there are more ways to control probing order.

Thanks, though right now I'm following Rob's suggestion and only delay
probing, not registration. The patch is really simple (applies on
linux-next, with async probing):

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 8da8e07..7e6b1e1 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -407,6 +407,11 @@ int driver_probe_device(struct device_driver
*drv, struct device *dev)
        if (!device_is_registered(dev))
                return -ENODEV;

+       if (!driver_deferred_probe_enable) {
+               driver_deferred_probe_add(dev);
+               return 0;
+       }
+
        pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
                 drv->bus->name, __func__, dev_name(dev), drv->name);

@@ -585,7 +590,7 @@ EXPORT_SYMBOL_GPL(device_attach);

 void device_initial_probe(struct device *dev)
 {
-       __device_attach(dev, true);
+       __device_attach(dev, driver_deferred_probe_enable);
 }

 static int __driver_attach(struct device *dev, void *data)

> Pls, Note here that TI OMAP2+ mach is not pure DT mach - it's combination of
> DT and not DT devices/drivers.
>
> Ok. So What was done...
>
> LKML Linux 4.1-rc3 (a simple case)
> 1) use your patches 3/4 as reference (only these two patches :)
> 2) move of_platform_populate later at device_initcall_sync time
> Boot time reduction ~0.4 sec

I'm a bit surprised at such a big improvement. May I ask how you are
measuring it?

> TI Android Kernel 3.14 (NOT a simple case)
> 1) use your patches 3/4 as reference (only these two patches :)
> 2) move of_platform_populate later at device_initcall_sync time
> 3) make it to boot (not sure I've fixed all issues, but those which
>    break the System boot):
>  - split non-DT and DT devices registration in platform code;
>  - keep non-DT devices registration from .init_machine() [arch_initcall]
>  - move DT-devices registration at device_initcall_sync time
>  - fix drivers which use platform_driver_probe().
>    Note. Now there are at about ~190 occurrences of this macro in Kernel.
>  - re-order few devices in DT (4 devices)
>  - fix one driver which uses of_find_device_by_node() wrongly
>    Note. This API is used some times with assumption that
>    requested dev has been probed already.
> Boot time reduction ~0.3 sec. Probing of some devices are still deferred.

I got no deferred probes on a pandaboard with just these changes:

https://git.collabora.com/cgit/user/tomeu/linux.git/commit/?id\x1586c6f50b3d3c65dc219a3cdc3327d798cabca6

> TI Android Kernel 3.14 + of_platform_device_ensure
> 1) backport of_platform_device_ensure() (also need patches
>    "of: Introduce device tree node flag helpers" and
>    "of: Keep track of populated platform devices")
> 2) back-port all your patches which uses of_platform_device_ensure()
> 3) make it to boot:
>    - drop patch dma: of: Probe DMA controllers on demand
>    - fix deadlock in regulator core:
>    regulator_dev_lookup() called from regulator_register() in K3.14
> 4) get rid of deferred probes - add of_platform_device_ensure() calls in:
>    - drivers/video/fbdev/omap2/dss/output.c
>    - drivers/extcon/extcon-class.c
>
>  Boot time reduction: NONE !?
>
> So few comments from above:
> - registering devices later during the System boot may improve boot time.
>   But resolving of all deferred probes may NOT improve boot time ;)
>   Have you seen smth like this?

Yeah, I don't really expect for on-demand probing to improve boot time
much in the general case, as drivers tend to first acquire resources
(and defer probe if needed) and only then access hardware and wait for
stuff to happen.

The main advantage of ordered/ondemand probing is that it is much
easier to see the order in which devices will be bound. In my case
this is useful because one could get one device (eg. the drm panel) to
probe very early by just moving that node to the beginning of the DT.
With deferred probe, one would have to figure out all the dependencies
and shuffle them around in the DT.

Another advantage (but not the one why I'm doing this work) is that in
general a driver's probe will be called only once per device, and if
it fails then we can be almost certain that something is wrong. This
should aid in debugging as right now one has to take into account the
several reasons why a device might defer its probe.

Depending on what work your drivers do in your platform, enabling
async probing for all of them may have a noticeable impact though.

> - usage of of_platform_device_ensure() will require continuous fixing of Kernel :(

You mean adding those calls to more subsystems?

> - late_initcall is not (as for me not safe) a good time to register devices. A lot
>   of platforms/subsystems/frameworks perform their final initialization or clean-up
>   steps, with assumption that System mostly ready to work. For example, CPUIdle/CPUFreq
>   are allowed and other PM staff. CPUIdle and driver's probing are not friends.

Yeah, I was expecting to find more problems due to this, but the
platforms I tested on didn't show any. Do you have pointers to
concrete issues?

> What would be nice to have for now in my opinion:
> - some useful place to move device population code.
>   May be machine_desc->init_device_sync() callback.

I'm looking at leave device registration where it currently is and
just add devices to the deferred list until we get to late_initcall,
where we would start to actually probe them. Seems promising for now.

> - some optional feature in DTC which will allow to re-arrange DT nodes for levels 0(root)
>   and 1(simple-bus) using standard or widely used bindings (gpio, regulators, clocks, dma,
>   pinctrl, irqs).

Could you extend on this, please?

> Additional note:
> - Changing device's registration order will change devices's order in
>  dpm_list (drivers/base/power/main.c) and devices_kset list
>  (drivers/base/core.c). This might potentially affect on suspend [1] and shutdown.

Yeah, I don't plan to change registration order, but probing certainly will.

> [1] https://lkml.org/lkml/2014/12/12/324

Thanks,

Tomeu

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

^ permalink raw reply related

* Re: [PATCH v3 2/3] video: fbdev: vesafb: add missing mtrr_del() for added MTRR
From: Tomi Valkeinen @ 2015-06-04  5:55 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Luis R. Rodriguez, plagnioj, hpa, linux-fbdev, luto, cocci,
	Toshi Kani, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
	Juergen Gross, Daniel Vetter, Dave Airlie, Antonino Daplas,
	Rob Clark, Jingoo Han, Wolfram Sang, linux-kernel
In-Reply-To: <20150520194632.GY23057@wotan.suse.de>

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]



On 20/05/15 22:46, Luis R. Rodriguez wrote:
> On Wed, May 20, 2015 at 12:52:14PM +0300, Tomi Valkeinen wrote:

>> Hmm, this looks a bit odd... You're removing the pseudo_palette, and
>> using its memory for mtrr cookie?
> 
> You are totally right, sorry, will spin a v3.

Did you ever send a new version (v4 actually), or have I somehow missed it?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 00/21] On-demand device registration
From: Rob Clark @ 2015-06-03 21:12 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: Mark Rutland, Linux Fbdev development list, Dmitry Torokhov,
	devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-i2c, Alexander Holler, linux-clk,
	moderated list:ARM/S5P EXYNOS AR..., Grant Likely, linux-pwm,
	linux-pm, linux-gpio, Rob Herring, linux-tegra@vger.kernel.org,
	Dan Williams, linux-arm-kernel@lists.infradead.org, linux-usb,
	Linux Kernel Mailing List, dmaengine
In-Reply-To: <1432565608-26036-1-git-send-email-tomeu.vizoso@collabora.com>

On Mon, May 25, 2015 at 10:53 AM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:
> Hello,
>
> I have a problem with the panel on my Tegra Chromebook taking longer than
> expected to be ready during boot (Stéphane Marchesin reported what is
> basically the same issue in [0]), and have looked into ordered probing as a
> better way of solving this than moving nodes around in the DT or playing with
> initcall levels.
>
> While reading the thread [1] that Alexander Holler started with his series to
> make probing order deterministic, it occurred to me that it should be possible
> to achieve the same by registering devices as they are referenced by other
> devices.
>
> This basically reuses the information that is already implicit in the probe()
> implementations, saving us from refactoring existing drivers or adding
> information to DTBs.
>
> Something I'm not completely happy with is that I have had to move the call to
> of_platform_populate after all platform drivers have been registered.
> Otherwise I don't see how I could register drivers on demand as we don't have
> yet each driver's compatible strings.
>
> For machs that don't move of_platform_populate() to a later point, these
> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
> all the deferred probes as some drivers may not be registered yet.
>
> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
> patches were enough to eliminate all the deferred probes.
>
> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.

So, complete drive-by comment (and I won't claim to be a DT expert,
etc, etc, so take this with a few grains of salt), but why not push
the problem to the DT compiler (or a pre-process step that could be
run on existing DT blobs), which generates an optional DT node that is
the recommended probe order?  That seems like it avoids adding
complexity into the early boot code (which seems like a good thing)..

As a bonus, a bootarg (or something like that) which runs through the
recommended probe order in reverse (to continue our current state of
ensuring that EPROBE_DEFER error paths are well tested)

At any rate, for a device like a drm driver that has multiple
sub-components, and depends on various other clk/gpio/regulator/etc
drivers, the current EPROBE_DEFER situation is pretty comical, so any
solution that improves on things is very much welcome :-)

BR,
-R

> Regards,
>
> Tomeu
>
> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>
> [1] https://lkml.org/lkml/2014/5/12/452
>
> Tomeu Vizoso (21):
>   regulator: core: Reduce critical area in _regulator_get
>   ARM: tegra: Add gpio-ranges property
>   ARM: tegra: Register drivers before devices
>   ARM: EXYNOS: Register drivers before devices
>   ARM i.MX6q: Register drivers before devices
>   of/platform: Add of_platform_device_ensure()
>   of/platform: Ensure device registration on lookup
>   gpio: Probe GPIO drivers on demand
>   gpio: Probe pinctrl devices on demand
>   regulator: core: Probe regulators on demand
>   drm: Probe panels on demand
>   drm/tegra: Probe dpaux devices on demand
>   i2c: core: Probe i2c master devices on demand
>   pwm: Probe PWM chip devices on demand
>   backlight: Probe backlight devices on demand
>   usb: phy: Probe phy devices on demand
>   clk: Probe clk providers on demand
>   pinctrl: Probe pinctrl devices on demand
>   phy: core: Probe phy providers on demand
>   dma: of: Probe DMA controllers on demand
>   power-supply: Probe power supplies on demand
>
>  arch/arm/boot/dts/tegra124.dtsi     |  1 +
>  arch/arm/mach-exynos/exynos.c       |  4 +--
>  arch/arm/mach-imx/mach-imx6q.c      | 12 ++++-----
>  arch/arm/mach-tegra/tegra.c         | 21 ++++++---------
>  drivers/clk/clk.c                   |  3 +++
>  drivers/dma/of-dma.c                |  3 +++
>  drivers/gpio/gpiolib-of.c           |  5 ++++
>  drivers/gpu/drm/drm_panel.c         |  3 +++
>  drivers/gpu/drm/tegra/dpaux.c       |  3 +++
>  drivers/i2c/i2c-core.c              |  3 +++
>  drivers/of/platform.c               | 53 +++++++++++++++++++++++++++++++++++++
>  drivers/phy/phy-core.c              |  3 +++
>  drivers/pinctrl/devicetree.c        |  2 ++
>  drivers/power/power_supply_core.c   |  3 +++
>  drivers/pwm/core.c                  |  3 +++
>  drivers/regulator/core.c            | 45 +++++++++++++++----------------
>  drivers/usb/phy/phy.c               |  3 +++
>  drivers/video/backlight/backlight.c |  3 +++
>  include/linux/of_platform.h         |  2 ++
>  19 files changed, 130 insertions(+), 45 deletions(-)
>
> --
> 2.4.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 00/21] On-demand device registration
From: Grygorii.Strashko@linaro.org @ 2015-06-03 19:57 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: Rob Herring,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Stéphane Marchesin, Thierry Reding, Dmitry Torokhov,
	Alexander Holler, Grant Likely, Rob Herring, Mark Rutland,
	Dan Williams, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, dri-devel,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_Jsq+EWLEJhRudTGAwYsOg4tX2-pGhygeQGHae9RL8rBpMiA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Tomeu,

On 05/28/2015 07:33 AM, Rob Herring wrote:
> On Mon, May 25, 2015 at 9:53 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> I have a problem with the panel on my Tegra Chromebook taking longer than
>> expected to be ready during boot (Stéphane Marchesin reported what is
>> basically the same issue in [0]), and have looked into ordered probing as a
>> better way of solving this than moving nodes around in the DT or playing with
>> initcall levels.
>>
>> While reading the thread [1] that Alexander Holler started with his series to
>> make probing order deterministic, it occurred to me that it should be possible
>> to achieve the same by registering devices as they are referenced by other
>> devices.
> 
> I like the concept and novel approach.
> 
>> This basically reuses the information that is already implicit in the probe()
>> implementations, saving us from refactoring existing drivers or adding
>> information to DTBs.
>>
>> Something I'm not completely happy with is that I have had to move the call to
>> of_platform_populate after all platform drivers have been registered.
>> Otherwise I don't see how I could register drivers on demand as we don't have
>> yet each driver's compatible strings.
> 
> Yeah, this is the opposite of what we'd really like. Ideally, we would
> have a solution that works for modules too. However, we're no worse
> off. We pretty much build-in dependencies to avoid module ordering
> problems.
> 
> Perhaps we need to make the probing on-demand rather than simply on
> device<->driver match occurring.
> 
>> For machs that don't move of_platform_populate() to a later point, these
>> patches shouldn't cause any problems but it's not guaranteed that we'll avoid
>> all the deferred probes as some drivers may not be registered yet.
> 
> Ideally, of_platform_populate is not explicitly called by each
> platform. So I think we need to make this work for the default case.
> 
>> I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these
>> patches were enough to eliminate all the deferred probes.
>>
>> With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s.
> 
> That's certainly compelling.

I've found your idea about moving device registration later during System boot
very interesting so I've decided to try it on dra7-evem (TI) :).
It's good to know time during Kernel boot when we can assume that all drivers are
ready for probing, so there are more ways to control probing order.

Pls, Note here that TI OMAP2+ mach is not pure DT mach - it's combination of
DT and not DT devices/drivers.

Ok. So What was done...

LKML Linux 4.1-rc3 (a simple case)
1) use your patches 3/4 as reference (only these two patches :)
2) move of_platform_populate later at device_initcall_sync time
Boot time reduction ~0.4 sec


TI Android Kernel 3.14 (NOT a simple case)
1) use your patches 3/4 as reference (only these two patches :)
2) move of_platform_populate later at device_initcall_sync time
3) make it to boot (not sure I've fixed all issues, but those which
   break the System boot):
 - split non-DT and DT devices registration in platform code;
 - keep non-DT devices registration from .init_machine() [arch_initcall]
 - move DT-devices registration at device_initcall_sync time
 - fix drivers which use platform_driver_probe().
   Note. Now there are at about ~190 occurrences of this macro in Kernel.
 - re-order few devices in DT (4 devices)
 - fix one driver which uses of_find_device_by_node() wrongly
   Note. This API is used some times with assumption that
   requested dev has been probed already.
Boot time reduction ~0.3 sec. Probing of some devices are still deferred.

TI Android Kernel 3.14 + of_platform_device_ensure
1) backport of_platform_device_ensure() (also need patches 
   "of: Introduce device tree node flag helpers" and 
   "of: Keep track of populated platform devices")
2) back-port all your patches which uses of_platform_device_ensure()
3) make it to boot:
   - drop patch dma: of: Probe DMA controllers on demand
   - fix deadlock in regulator core:
   regulator_dev_lookup() called from regulator_register() in K3.14
4) get rid of deferred probes - add of_platform_device_ensure() calls in:
   - drivers/video/fbdev/omap2/dss/output.c
   - drivers/extcon/extcon-class.c

 Boot time reduction: NONE !?


So few comments from above:
- registering devices later during the System boot may improve boot time.
  But resolving of all deferred probes may NOT improve boot time ;) 
  Have you seen smth like this?

- usage of of_platform_device_ensure() will require continuous fixing of Kernel :(

- late_initcall is not (as for me not safe) a good time to register devices. A lot
  of platforms/subsystems/frameworks perform their final initialization or clean-up
  steps, with assumption that System mostly ready to work. For example, CPUIdle/CPUFreq
  are allowed and other PM staff. CPUIdle and driver's probing are not friends.

What would be nice to have for now in my opinion:
- some useful place to move device population code.
  May be machine_desc->init_device_sync() callback.
- some optional feature in DTC which will allow to re-arrange DT nodes for levels 0(root)
  and 1(simple-bus) using standard or widely used bindings (gpio, regulators, clocks, dma,
  pinctrl, irqs).

Additional note:
- Changing device's registration order will change devices's order in
 dpm_list (drivers/base/power/main.c) and devices_kset list
 (drivers/base/core.c). This might potentially affect on suspend [1] and shutdown. 
[1] https://lkml.org/lkml/2014/12/12/324


-- 
regards,
-grygorii

^ permalink raw reply

* Re: [PATCH 2/4] video: fbdev: imxfb: Constify platform_device_id
From: Lee Jones @ 2015-06-03 13:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <556EEABD.6020902@ti.com>

On Wed, 03 Jun 2015, Tomi Valkeinen wrote:
> On 03/06/15 14:50, Lee Jones wrote:
> > On Wed, 03 Jun 2015, Tomi Valkeinen wrote:
> > 
> >> Hi Lee,
> >>
> >> On 05/05/15 11:28, Lee Jones wrote:
> >>> On Sat, 02 May 2015, Krzysztof Kozlowski wrote:
> >>>
> >>>> The platform_device_id is not modified by the driver and core uses it as
> >>>> const.
> >>>>
> >>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
> >>>> ---
> >>>>  drivers/video/fbdev/imxfb.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> Applied, thanks.
> >>
> >> You applied the fbdev changes in this series also, not only the
> >> backlight changes?
> > 
> > Ah, yes I did.  I assumed they were all due for Backlight.
> > 
> > Would you like me to un-apply the non-Backlight patches?
> 
> If you can easily remove them, yes.
> 
> But chances for conflict are quite minimal, so if you already have a
> pile of commits on top of these, it's fine if they go via backlight tree.

I can easily remove them, no problem.

Consider them unapplied.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH] fbdev: radeon: Remove 'struct timeval' usage
From: Tomi Valkeinen @ 2015-06-03 11:59 UTC (permalink / raw)
  To: Tina Ruchandani, Arnd Bergmann
  Cc: y2038, Benjamin Herrenschmidt, Jean-Christophe Plagniol-Villard,
	linux-fbdev, linux-kernel
In-Reply-To: <20150525040716.GA4448@tinar>

[-- Attachment #1: Type: text/plain, Size: 3189 bytes --]



On 25/05/15 07:07, Tina Ruchandani wrote:
> 'struct timeval' uses a 32-bit representation for the
> seconds field which will overflow in the year 2038 and beyond.
> This patch replaces the usage of 'struct timeval' with
> ktime_t which uses a 64-bit time representation and does not
> suffer from the y2038 problem. This patch is part of a larger
> effort to remove all instances of 'struct timeval', 'struct
> timespec', time_t and other 32-bit timekeeping variables
> from the kernel.
> The patch also replaces the use of real time (do_gettimeofday)
> with monotonic time (ktime_get).
> 
> Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
> ---
>  drivers/video/fbdev/aty/radeon_base.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
> index 01237c8..9747e9e 100644
> --- a/drivers/video/fbdev/aty/radeon_base.c
> +++ b/drivers/video/fbdev/aty/radeon_base.c
> @@ -64,6 +64,7 @@
>  #include <linux/slab.h>
>  #include <linux/delay.h>
>  #include <linux/time.h>
> +#include <linux/ktime.h>
>  #include <linux/fb.h>
>  #include <linux/ioport.h>
>  #include <linux/init.h>
> @@ -461,8 +462,8 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>  	int hTotal, vTotal, num, denom, m, n;
>  	unsigned long long hz, vclk;
>  	long xtal;
> -	struct timeval start_tv, stop_tv;
> -	long total_secs, total_usecs;
> +	ktime_t start, stop;
> +	s64 delta;
>  	int i;
>  
>  	/* Ugh, we cut interrupts, bad bad bad, but we want some precision
> @@ -478,7 +479,7 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>  		if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) == 0)
>  			break;
>  
> -	do_gettimeofday(&start_tv);
> +	start = ktime_get();
>  
>  	for(i=0; i<1000000; i++)
>  		if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) != 0)
> @@ -487,20 +488,18 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>  	for(i=0; i<1000000; i++)
>  		if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) == 0)
>  			break;
> -	
> -	do_gettimeofday(&stop_tv);
> -	
> +
> +	stop = ktime_get();
> +
>  	local_irq_enable();
>  
> -	total_secs = stop_tv.tv_sec - start_tv.tv_sec;
> -	if (total_secs > 10)
> +	delta = ktime_us_delta(stop, start);
> +
> +	/* Return -1 if more than 10 seconds have elapsed */
> +	if (delta > (10*1000000))
>  		return -1;
> -	total_usecs = stop_tv.tv_usec - start_tv.tv_usec;
> -	total_usecs += total_secs * 1000000;
> -	if (total_usecs < 0)
> -		total_usecs = -total_usecs;
> -	hz = 1000000/total_usecs;
> - 
> +	hz = 1000000/delta;
> +
>  	hTotal = ((INREG(CRTC_H_TOTAL_DISP) & 0x1ff) + 1) * 8;
>  	vTotal = ((INREG(CRTC_V_TOTAL_DISP) & 0x3ff) + 1);
>  	vclk = (long long)hTotal * (long long)vTotal * hz;
> @@ -548,7 +547,7 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>  		denom *= 3;
>  		break;
>  	case 6:
> -		denom *= 6;   
> +		denom *= 6;
>  		break;
>  	case 7:
>  		denom *= 12;

The above change was extra. I removed it, and queued this for 4.2.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 2/4] video: fbdev: imxfb: Constify platform_device_id
From: Tomi Valkeinen @ 2015-06-03 11:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20150603115050.GB12304@x1>

[-- Attachment #1: Type: text/plain, Size: 924 bytes --]



On 03/06/15 14:50, Lee Jones wrote:
> On Wed, 03 Jun 2015, Tomi Valkeinen wrote:
> 
>> Hi Lee,
>>
>> On 05/05/15 11:28, Lee Jones wrote:
>>> On Sat, 02 May 2015, Krzysztof Kozlowski wrote:
>>>
>>>> The platform_device_id is not modified by the driver and core uses it as
>>>> const.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
>>>> ---
>>>>  drivers/video/fbdev/imxfb.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> Applied, thanks.
>>
>> You applied the fbdev changes in this series also, not only the
>> backlight changes?
> 
> Ah, yes I did.  I assumed they were all due for Backlight.
> 
> Would you like me to un-apply the non-Backlight patches?

If you can easily remove them, yes.

But chances for conflict are quite minimal, so if you already have a
pile of commits on top of these, it's fine if they go via backlight tree.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 2/4] video: fbdev: imxfb: Constify platform_device_id
From: Lee Jones @ 2015-06-03 11:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <556EE743.5030405@ti.com>

On Wed, 03 Jun 2015, Tomi Valkeinen wrote:

> Hi Lee,
> 
> On 05/05/15 11:28, Lee Jones wrote:
> > On Sat, 02 May 2015, Krzysztof Kozlowski wrote:
> > 
> >> The platform_device_id is not modified by the driver and core uses it as
> >> const.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
> >> ---
> >>  drivers/video/fbdev/imxfb.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Applied, thanks.
> 
> You applied the fbdev changes in this series also, not only the
> backlight changes?

Ah, yes I did.  I assumed they were all due for Backlight.

Would you like me to un-apply the non-Backlight patches?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH 2/4] video: fbdev: imxfb: Constify platform_device_id
From: Tomi Valkeinen @ 2015-06-03 11:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20150505082807.GV4047@x1>

[-- Attachment #1: Type: text/plain, Size: 481 bytes --]

Hi Lee,

On 05/05/15 11:28, Lee Jones wrote:
> On Sat, 02 May 2015, Krzysztof Kozlowski wrote:
> 
>> The platform_device_id is not modified by the driver and core uses it as
>> const.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
>> ---
>>  drivers/video/fbdev/imxfb.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Applied, thanks.

You applied the fbdev changes in this series also, not only the
backlight changes?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 0/5] OMAPDSS: HDMI audio compliance fixes
From: Tomi Valkeinen @ 2015-06-03 11:35 UTC (permalink / raw)
  To: Jyri Sarha, alsa-devel, linux-fbdev, linux-omap
  Cc: peter.ujfalusi, broonie, liam.r.girdwood, misael.lopez
In-Reply-To: <cover.1429705991.git.jsarha@ti.com>

[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]

Hi,

On 22/04/15 16:22, Jyri Sarha wrote:
> I have rebased these patches from ti-linux-3.14.y on top of Linux
> 4.0.0. I tested them briefly on OMAP4 and OMAP5 in my environment, but
> I could not test any channel count beyond stereo. However, there were
> no conflicts in the rebase and each fix makes sense when looking at
> the chip TRM and the relevant HDMI specs.
> 
> All the changes are really minor, but most of them apply to OMAPDSS side,
> so it is probably best to merge them trough there.
> 
> Best regards,
> Jyri
> 
> Misael Lopez Cruz (5):
>   OMAPDSS: HDMI4: Set correct CC for 8-channels layout
>   OMAPDSS: HDMI5: Set valid sample order
>   OMAPDSS: HDMI5: Fix AUDICONF3 bitfield offsets
>   ASoC: omap-hdmi-audio: Force channel allocation only for OMAP4
>   ASoC: omap-hdmi-audio: Fix invalid combination of DM_INH and CA
> 
>  drivers/video/fbdev/omap2/dss/hdmi4_core.c | 12 +++++++++++-
>  drivers/video/fbdev/omap2/dss/hdmi5_core.c |  5 ++++-
>  sound/soc/omap/omap-hdmi-audio.c           | 12 +++++++-----
>  3 files changed, 22 insertions(+), 7 deletions(-)

I've queued these for 4.2.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH v5 0/17] framebuffer: simple conversions to arch_phys_wc_add()
From: Tomi Valkeinen @ 2015-06-03  9:42 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-kernel, linux-fbdev, Luis R. Rodriguez
In-Reply-To: <1432937236-9327-1-git-send-email-mcgrof@do-not-panic.com>

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]



On 30/05/15 01:07, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> Tomi,
> 
> this v5 series goes with the patch 3/17 titled "video: fbdev: gbefb:
> use arch_phys_wc_add() and devm_ioremap_wc()" with a change from
> dma_free_coherent() to dma_free_writecombine() for the area now allocated
> with dma_alloc_writecombine(). The remaining patches remain the same.
> 
> As for the pull request all I did was reset my tree to yours, and
> then reset it to the same old commit 36520841a443d5ee966f9632c417fcc8a25e07e3,
> then applied the new series. The only delta from the v4 seires is the
> patch 3/17. The new branch changes from simple-arch_phys-20150528 to
> simple-arch_phys-20150529. Pull request below for your convenience.

Thanks, I've picked up the v5 of the patch 3.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 00/21] On-demand device registration
From: Tomeu Vizoso @ 2015-06-02 10:14 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Grant Likely, Mark Rutland, devicetree@vger.kernel.org,
	linux-fbdev@vger.kernel.org, linux-samsung-soc,
	linux-tegra@vger.kernel.org, linux-gpio@vger.kernel.org,
	linux-pm@vger.kernel.org, Dmitry Torokhov,
	linux-kernel@vger.kernel.org, Rob Herring,
	linux-pwm@vger.kernel.org, open list:DRM PANEL DRIVERS, dmaengine,
	Alexander Holler, Dan Williams, linux-usb@vger.kernel.org,
	linux-clk
In-Reply-To: <CACRpkdasLwMfEHGMRd1X3K_DU7NMPreE8TMwqqDTg8k9wj=wJQ@mail.gmail.com>

On 2 June 2015 at 10:48, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, May 25, 2015 at 4:53 PM, Tomeu Vizoso
> <tomeu.vizoso@collabora.com> wrote:
>
>> have looked into ordered probing as a
>> better way of solving this than moving nodes around in the DT or playing with
>> initcall levels.
>>
>> While reading the thread [1] that Alexander Holler started with his series to
>> make probing order deterministic, it occurred to me that it should be possible
>> to achieve the same by registering devices as they are referenced by other
>> devices.
>
> This is pretty cool, but a too local solution to a global problem.
>
> Deferred probe and initcall reordering, silly as they may seem,
> does not require you to use device tree.
>
> The real solution, which I think I pointed out already when we
> added deferred probe, is to put dependency graphs in the drivers

By this you mean something like what Thierry suggested here?

http://article.gmane.org/gmane.linux.kernel/1774623

> and have the kernel device driver core percolate dependecies by
> walking the graph on probing driver, removing driver (usually the
> inverse use case), [runtime] suspend and [runtime] resumeing
> a driver. Possibly the dependencies will even be different
> depending on use case.
>
> This is what systemd is doing in userspace for starting services:
> ask for your dependencies and wait for them if they are not
> there. So drivers ask for resources and wait for them. It also
> needs to be abstract, so for example we need to be able to
> hang on regulator_get() until the driver is up and providing that
> regulator, and as long as everything is in slowpath it should
> be OK. (And vice versa mutatis mutandis for clk, gpio, pin
> control, interrupts (!) and DMA channels for example.)

I understood above that you propose probing devices in order, but now
you mention that resource getters would block until the dependency is
fulfilled which confuses me because if we are probing in order then
all dependencies would be fulfilled before the device in question gets
probed.

> So if this should be solved it should be solved in an abstract way
> in the device driver core available for all, then have calls calling
> out to DT, ACPI, possibly even PCI or USB (as these
> enumerate devices themselves) to obtain a certain
> dependency.

Yeah, I was planning looking into this now that I got it working with
async probing.

Thanks,

Tomeu

> Yours,
> Linus Walleij
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH 00/21] On-demand device registration
From: Linus Walleij @ 2015-06-02  8:48 UTC (permalink / raw)
  To: Tomeu Vizoso, Grant Likely
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	linux-fbdev@vger.kernel.org, linux-samsung-soc,
	linux-tegra@vger.kernel.org, linux-gpio@vger.kernel.org,
	linux-pm@vger.kernel.org, Dmitry Torokhov,
	linux-kernel@vger.kernel.org, Rob Herring,
	linux-pwm@vger.kernel.org, open list:DRM PANEL DRIVERS, dmaengine,
	Alexander Holler, Dan Williams, linux-usb@vger.kernel.org,
	linux-clk, linux-arm-kernel
In-Reply-To: <1432565608-26036-1-git-send-email-tomeu.vizoso@collabora.com>

On Mon, May 25, 2015 at 4:53 PM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:

> have looked into ordered probing as a
> better way of solving this than moving nodes around in the DT or playing with
> initcall levels.
>
> While reading the thread [1] that Alexander Holler started with his series to
> make probing order deterministic, it occurred to me that it should be possible
> to achieve the same by registering devices as they are referenced by other
> devices.

This is pretty cool, but a too local solution to a global problem.

Deferred probe and initcall reordering, silly as they may seem,
does not require you to use device tree.

The real solution, which I think I pointed out already when we
added deferred probe, is to put dependency graphs in the drivers
and have the kernel device driver core percolate dependecies by
walking the graph on probing driver, removing driver (usually the
inverse use case), [runtime] suspend and [runtime] resumeing
a driver. Possibly the dependencies will even be different
depending on use case.

This is what systemd is doing in userspace for starting services:
ask for your dependencies and wait for them if they are not
there. So drivers ask for resources and wait for them. It also
needs to be abstract, so for example we need to be able to
hang on regulator_get() until the driver is up and providing that
regulator, and as long as everything is in slowpath it should
be OK. (And vice versa mutatis mutandis for clk, gpio, pin
control, interrupts (!) and DMA channels for example.)


So if this should be solved it should be solved in an abstract way
in the device driver core available for all, then have calls calling
out to DT, ACPI, possibly even PCI or USB (as these
enumerate devices themselves) to obtain a certain
dependency.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 12/12] Staging: sm750fb: Insert spaces after commas in three files.
From: Sudip Mukherjee @ 2015-06-02  8:37 UTC (permalink / raw)
  To: Isaac Assegai; +Cc: gregkh, teddy.wang, linux-fbdev, devel, linux-kernel
In-Reply-To: <20150602075522.GA10942@ubuntu-desktop>

On Tue, Jun 02, 2015 at 12:55:22AM -0700, Isaac Assegai wrote:
> On Tue, Jun 02, 2015 at 12:18:06PM +0530, Sudip Mukherjee wrote:
> > On Sun, May 31, 2015 at 10:01:13PM -0700, Isaac Assegai wrote:
<snip>
> > > +					  FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING, ENABLE)|
> > > +					  FIELD_SET(0, CRT_DISPLAY_CTRL, PLANE, ENABLE);
> > I am getting confused now.
> > This part you have modified in patch 10/12 and i just replied that
> > you have missed two modifications.
> > 
> Thanks for your feedback Sudip, I have a few questions:
> 1. After applying the first 11 patches I ran checkpatch again and noticed that
> I missed three warnings so I ran a 12th patch fixing them. That's why you
> see the 12th patch modifying a file that another patch already touched.
> What should be done in this type of situation?
you have to start from the beginning again. :(
I have faced such similar situation also, but starting for the beginnig
is the only way.
> 
> 2. On Patch 3 you pointed out two trailing whitespace errors.
> I ran checkpatch on all the patches before I sent them to
> make sure I didn't introduce any new errors,
but it looks like they were introduced in your patch.
> -     int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> -                                             u32,u32,u32,u32,u32,u32,u32,u32,u32);
> +     int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32, u32,
the original code was having three u32, i think when you moved one more
u32 from the following line you forgot to remove the space after that.

>however
> there are *many* warnings and errors showing up from
> the poor styling already present and I missed these
> whitespace errors in the mess.
> Can I make checkpatch suppress any errors that might have already
> been present in the code and only show me those issues introduced
> by the patch itself? If not, how did you identify it?
i dont think checkpatch can work like that way.
identification was easy. i saw checkpatch giving error and noticed you
have modified the code. So i checked the original code if trailing
whitespace was there or not.

regards
sudip

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox