Re: [PATCH v5 0/2] Fix TLB invalidate issues with Broadwell [preempt-rt regression]
[[PATCH v5 0/2] Fix TLB invalidate issues with Broadwell] On 12/07/2022 (Tue
16:21) Mauro Carvalho Chehab wrote:
> i915 selftest hangcheck is causing the i915 driver timeouts, as reported
> by Intel CI bot:
>
> http://gfx-ci.fi.intel.com/cibuglog-ng/issuefilterassoc/24297?query_key=42a999f48fa6ecce068bc8126c069be7c31153b4
[...]
> After that, the machine just silently hangs.
>
> Bisecting the issue, the patch that introduced the regression is:
>
> 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")
>
> Reverting it fix the issues, but introduce other problems, as TLB
> won't be invalidated anymore. So, instead, let's fix the root cause.
>
> It turns that the TLB flush logic ends conflicting with i915 reset,
> which is called during selftest hangcheck. So, the TLB cache should
> be serialized together with i915 reset.
>
> Tested on an Intel NUC5i7RYB with an i7-5557U Broadwell CPU.
It turns out that this breaks PM-suspend operations on preempt-rt, on
multiple versions, due to all the linux-stable backports. This happens
because the uncore->lock is now used in atomic contexts.
As the uncore->lock is widely used, conversion to a raw lock seems
inappropriate at 1st glance, and hence some alternate solution will
likely be required.
Below is an example of the regression on v5.15-rt, with backport:
commit 0ee5874dad61d2b154a9e3db196fc33e8208ce1b
Author: Chris Wilson
Date: Tue Jul 12 16:21:32 2022 +0100
drm/i915/gt: Serialize GRDOM access between multiple engine resets
[ Upstream commit b24dcf1dc507f69ed3b5c66c2b6a0209ae80d4d4 ]
Reverting the engine reset serialization change avoids the PM-suspend
regression and is a temporary workaround for -rt users, but of course
leaves this original TLB issue exposed.
BUG: sleeping function called from invalid context at
kernel/locking/spinlock_rt.c:46
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 45092, name:
kworker/u8:4
preempt_count: 1, expected: 0
RCU nest depth: 0, expected: 0
INFO: lockdep is turned off.
Preemption disabled at:
[] __intel_gt_reset+0x92/0x100 [i915]
CPU: 3 PID: 45092 Comm: kworker/u8:4 Tainted: GW O
5.15.59-rt48-preempt-rt #1
Hardware name: Intel(R) Client Systems NUC7i5DNKE/NUC7i5DNB, BIOS
DNKBLi5v.86A.0064.2019.0523.1933 05/23/2019
Workqueue: events_unbound async_run_entry_fn
Call Trace:
show_stack+0x52/0x5c
dump_stack_lvl+0x5b/0x86
dump_stack+0x10/0x16
__might_resched.cold+0xf7/0x12f
? __gen6_reset_engines.constprop.0+0x80/0x80 [i915]
rt_spin_lock+0x4e/0xf0
? gen8_reset_engines+0x2e/0x1e0 [i915]
gen8_reset_engines+0x2e/0x1e0 [i915]
? __gen6_reset_engines.constprop.0+0x80/0x80 [i915]
__intel_gt_reset+0x9d/0x100 [i915]
gt_sanitize+0x16c/0x190 [i915]
intel_gt_suspend_late+0x3d/0xc0 [i915]
i915_gem_suspend_late+0x57/0x130 [i915]
i915_drm_suspend_late+0x38/0x110 [i915]
i915_pm_suspend_late+0x1d/0x30 [i915]
pm_generic_suspend_late+0x28/0x40
pci_pm_suspend_late+0x37/0x50
? pci_pm_poweroff_late+0x50/0x50
dpm_run_callback.cold+0x3c/0xa8
__device_suspend_late+0xa4/0x1e0
async_suspend_late+0x20/0xa0
async_run_entry_fn+0x28/0xc0
process_one_work+0x239/0x6c0
worker_thread+0x58/0x3e0
kthread+0x1a9/0x1d0
? process_one_work+0x6c0/0x6c0
? set_kthread_struct+0x50/0x50
ret_from_fork+0x1f/0x30
PM: late suspend of devices complete after 26.497 msecs
Paul.
--
>
> v5:
> - Added a missing SoB on patch 2.
> - No other changes.
>
> v4:
> - No functional changes. All changes are at the patch descriptions:
> - collected acked-by/reviewed-by;
> - use the same e-mail on Author and SoB on patch 1.
>
> v3:
> - Removed the logic that would check if the engine is awake before doing
> TLB flush invalidation as backporting PM logic up to Kernel 4.x could be
> too painful. After getting this one merged, I'll submit a separate patch
> with the PM awake logic.
>
> v2:
>
> - Reduced to bare minimum fixes, as this shoud be backported deeply
> into stable.
>
> Chris Wilson (2):
> drm/i915/gt: Serialize GRDOM access between multiple engine resets
> drm/i915/gt: Serialize TLB invalidates with GT resets
>
> drivers/gpu/drm/i915/gt/intel_gt.c| 15 ++-
> drivers/gpu/drm/i915/gt/intel_reset.c | 37 ---
> 2 files changed, 42 insertions(+), 10 deletions(-)
>
> --
> 2.36.1
>
>
bisected gma500 kernel splat on portwell nano-8044
[Re: bisected gma500 kernel splat on portwell nano-8044] On 29/10/2016 (Sat
23:05) Patrik Jakobsson wrote:
> On Sat, Oct 29, 2016 at 9:13 PM, Paul Gortmaker
> wrote:
> > Dusted off an old Portwell nano-8044 that hadn't been booted for years
> > and found that it wouldn't boot mainline - it puts the D-sub VGA into no
> > signal mode. Hooking up a serial cable and I see a boot splat, which I
> > bisected down to
> >
> > # first bad commit: [01934c2a691882185b3021d437df13bcba07711d]
> > # drm/fb-helper: Propagate errors from initial config failure
> >
> > ..introduced 1st into the 4.0 kernel. To be fair, the 3.19 kernel also
> > blanked the screen once gma500 was loaded but it came on again once X11
> > started up and was splat free.
> >
> > Looking at the above commit, it is clear we were returning an error
> > before that we were simply ignoring, and I confirmed that by making this
> > simple change on the latest mainline from Linus, and X11 started again
> > just like 3.19 before the commit was merged.
> >
> >
> > diff --git a/drivers/gpu/drm/gma500/framebuffer.c
> > b/drivers/gpu/drm/gma500/framebuffer.c
> > index 3a44e705db53..96503a329a3b 100644
> > --- a/drivers/gpu/drm/gma500/framebuffer.c
> > +++ b/drivers/gpu/drm/gma500/framebuffer.c
> > @@ -586,7 +586,7 @@ int psb_fbdev_init(struct drm_device *dev)
> >
> > ret = drm_fb_helper_initial_config(&fbdev->psb_fb_helper, 32);
> > if (ret)
> > - goto fini;
> > + printk("gma500: initial config = %d\n", ret);
> >
> > return 0;
> >
> >
> > The return value it prints is:
> >
> > paul at nano:~$ dmesg|grep gma
> > [4.239282] gma500 :00:02.0: trying to get vblank count for disabled
> > pipe 1
> > [4.240006] gma500 :00:02.0: trying to get vblank count for disabled
> > pipe 1
> > [4.567972] gma500: initial config = -12
> > [4.576715] gma500 :00:02.0: I2C transfer error
> > [4.581628] [drm] Initialized gma500 1.0.0 20140314 for :00:02.0 on
> > minor 0
> > [ 24.143524] gma500 :00:02.0: I2C transfer error
> > paul at nano:~$
>
> My best bet is that we are failing to allocate backing for the
> framebuffer in psbfb_gtt_alloc_range(). We allocate contiguous memory
> from a special memory pool for the framebuffer layer which is quite
> limited in size. Possibly when I2C fails we fall back on bogus VBT
> data for LVDS or something. The driver assumes that the board (BIOS /
> VBT / fuses) is configured properly and currently don't fail
> gracefully.
Sounds reasonable. I forgot to mention -12 == -ENOMEM for the context
of the discussion. This was advertised as the 1st nano-ATX Atom based
board back in the day, so I'm sure the BIOS isn't populating things with
100% correct info. (If it was, text mode presumably wouldn't get trashed
by the loading of gma500_gfx).
>
> Can you give me the dmesg with drm.debug=0xfe?
Sure ; it looks quite verbose, so I'll send it off list. I'm still
booting recent mainline (8e819101ce6fcc58801c9) with my one line patch,
so I can log in and run dmesg ; if you'd rather I run w/o the patch and
capture the output from the serial console, let me know.
> > I assume the disabled pipe msgs are for the internal (currently unused) LVDS
> > connector.
> >
> > The splats varied a bit ; I've included a couple samples below.
> > Sometimes it would stumble on a bit further but most times a subsequent
> > fault triggered by the initial one would kill it dead.
> >
> > I'd checked Portwell's site to see if there was a newer BIOS on
> > drivers.portwell.com since the kernel also complains about a Firmware
> > bug relating to initial brightness, but couldn't find any.
>
> Sounds like the board is configured to have an LVDS panel connected.
I reloaded the CMOS defaults - it didn't help. It does have an LVDS
panel selection choice dialog in the BIOS, but there isn't a choice for
"None" for some odd reason.
>
> >
> > Probably not worth losing a lot of sleep over, since the board is old
> > enough to be tossed into the e-waste now anyways, but since I bisected
> > it I figured I might as well report what I'd found.
> >
> > https://www.portwell.com.tw/products/NANO-8044.html
>
> Thanks for the report, who needs sleep anyway :)
Well, if you are sufficiently bored (or interested) to keep diagnosing
it by remote, I'll run whatever tests/patches you want. :)
Paul.
--
> -Patrik
>
> > Paul.
>
bisected gma500 kernel splat on portwell nano-8044
Dusted off an old Portwell nano-8044 that hadn't been booted for years
and found that it wouldn't boot mainline - it puts the D-sub VGA into no
signal mode. Hooking up a serial cable and I see a boot splat, which I
bisected down to
# first bad commit: [01934c2a691882185b3021d437df13bcba07711d]
# drm/fb-helper: Propagate errors from initial config failure
..introduced 1st into the 4.0 kernel. To be fair, the 3.19 kernel also
blanked the screen once gma500 was loaded but it came on again once X11
started up and was splat free.
Looking at the above commit, it is clear we were returning an error
before that we were simply ignoring, and I confirmed that by making this
simple change on the latest mainline from Linus, and X11 started again
just like 3.19 before the commit was merged.
diff --git a/drivers/gpu/drm/gma500/framebuffer.c
b/drivers/gpu/drm/gma500/framebuffer.c
index 3a44e705db53..96503a329a3b 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -586,7 +586,7 @@ int psb_fbdev_init(struct drm_device *dev)
ret = drm_fb_helper_initial_config(&fbdev->psb_fb_helper, 32);
if (ret)
- goto fini;
+ printk("gma500: initial config = %d\n", ret);
return 0;
The return value it prints is:
paul at nano:~$ dmesg|grep gma
[4.239282] gma500 :00:02.0: trying to get vblank count for disabled
pipe 1
[4.240006] gma500 :00:02.0: trying to get vblank count for disabled
pipe 1
[4.567972] gma500: initial config = -12
[4.576715] gma500 :00:02.0: I2C transfer error
[4.581628] [drm] Initialized gma500 1.0.0 20140314 for :00:02.0 on
minor 0
[ 24.143524] gma500 :00:02.0: I2C transfer error
paul at nano:~$
I assume the disabled pipe msgs are for the internal (currently unused) LVDS
connector.
The splats varied a bit ; I've included a couple samples below.
Sometimes it would stumble on a bit further but most times a subsequent
fault triggered by the initial one would kill it dead.
I'd checked Portwell's site to see if there was a newer BIOS on
drivers.portwell.com since the kernel also complains about a Firmware
bug relating to initial brightness, but couldn't find any.
Probably not worth losing a lot of sleep over, since the board is old
enough to be tossed into the e-waste now anyways, but since I bisected
it I figured I might as well report what I'd found.
https://www.portwell.com.tw/products/NANO-8044.html
Paul.
--
[ 25.647208] BUG: unable to handle kernel paging request at 0001022d
[ 25.648016] IP: [] mutex_lock+0xd/0x30
[ 25.648016] *pde =
[ 25.648016] Oops: 0002 [#1] SMP
[ 25.648016] Modules linked in:
[ 25.648016] CPU: 0 PID: 1387 Comm: gpu-manager Not tainted 3.19.0-rc3+ #18
[ 25.648016] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To
be filled by O.E.M., BIOS 080015 07/24/2009
[ 25.648016] task: f35363e0 ti: f529c000 task.ti: f529c000
[ 25.648016] EIP: 0060:[] EFLAGS: 00010246 CPU: 0
[ 25.648016] EIP is at mutex_lock+0xd/0x30
[ 25.648016] EAX: 0001022d EBX: 0001022d ECX: EDX: 8000
[ 25.648016] ESI: EDI: 00010101 EBP: f529dee4 ESP: f529dee0
[ 25.648016] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[ 25.648016] CR0: 80050033 CR2: 0001022d CR3: 35194000 CR4: 07c0
[ 25.648016] Stack:
[ 25.648016] f34b3260 f529def8 c136da41 f614df00 00010101 f6289000 f529df00
c136daea
[ 25.648016] f529df14 c1351e12 f6289000 f6289000 f6289000 f529df1c c140c611
f529df2c
[ 25.648016] c13555c2 f6289000 f529df5c c135595f f529df8c f6289034
f6289000
[ 25.648016] Call Trace:
[ 25.648016] [] __drm_modeset_lock_all+0x71/0x110
[ 25.648016] [] drm_modeset_lock_all+0xa/0x30
[ 25.648016] [] drm_fb_helper_restore_fbdev_mode_unlocked+0x12/0x50
[ 25.648016] [] psb_driver_lastclose+0x11/0x40
[ 25.648016] [] drm_lastclose+0x22/0x130
[ 25.648016] [] drm_release+0x28f/0x450
[ 25.648016] [] __fput+0xb5/0x1c0
[ 25.648016] [] fput+0x8/0x10
[ 25.648016] [] task_work_run+0xa1/0xd0
[ 25.648016] [] do_notify_resume+0x65/0x70
[ 25.648016] [] work_notifysig+0x24/0x29
[ 25.648016] Code: 01 75 13 64 a1 38 77 b0 c1 5d 89 42 10 b8 01 00 00 00 c3
8d 76 00 31 c0 5d c3 8d 74 26 00 55 89 e5 53 89 c3 e8 15 f0 ff ff 89 d8 ff
08 79 05 e8 79 06 00 00 64 a1 38 77 b0 c1 89 43 10 5bd
[ 25.648016] EIP: [] mutex_lock+0xd/0x30 SS:ESP 0068:f529dee0
[ 25.648016] CR2: 0001022d
[ 25.837795] ---[ end trace 6df62ac22aa38097 ]---
[ 14.559091] BUG: unable to handle kernel paging request at 74746d9e
[ 14.560006] IP: [] mutex_lock+0xd/0x30
[ 14.560006] *pde =
[ 14.560006] Oops: 0002 [#1] SMP
[ 14.560006] Modules linked in:
[ 14.560006] CPU: 0 PID: 483 Comm: kworker/0:1 Not tainted 3.19.0-rc3+ #17
[ 14.560006] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To
be filled by O.E.M., BIOS 080015 07/24/2009
[ 14.560006] Workq
linux-next: Tree for Oct 25
On Mon, Oct 24, 2016 at 11:31 PM, Stephen Rothwell wrote: > Hi all, > > There will probably be no linux-next releases next week while I attend > the Kernel Summit. > > Changes since 20161024: > > The pm tree gained a conflict against the imx-mxs tree. > > The mali-dp tree gained a conflict against the drm-misc tree. > > The sunxi tree gained a build failure so I used the version from > next-20161024. > > The akpm-current tree still had its build failures for which I applied > 2 patches. > > Non-merge commits (relative to Linus' tree): 2174 > 2622 files changed, 187829 insertions(+), 38953 deletions(-) The i386 allmodconfig fails with bad 32/64 math for about the last 5 days. ERROR: "__umoddi3" [drivers/gpu/drm/i915/i915.ko] undefined! http://kisskb.ellerman.id.au/kisskb/buildresult/12840554/ git bisect says: first bad commit: [4d60c5fd3f8751ea751d6dc6cfe0c1620420ccf8] drm/i915/gvt: vGPU PCI configuration space virtualization Authors/maintainers added to Cc/To list. Guessing folks weren't aware of the regression. Thanks, Paul.
[PATCH v2 2/3] drm: simpledrm: add fbdev fallback support
On Fri, Aug 5, 2016 at 11:44 AM, Noralf Trønnes wrote:
> Create a simple fbdev device during SimpleDRM setup so legacy user-space
> and fbcon can use it.
>
> Original work by David Herrmann.
>
> Cc: dh.herrmann at gmail.com
> Signed-off-by: Noralf Trønnes
> ---
>
> Changes from version 1:
> No changes
>
> Changes from previous version:
> - Remove the DRM_SIMPLEDRM_FBDEV kconfig option and use DRM_FBDEV_EMULATION
> - Suspend fbcon/fbdev when the pipeline is enabled, resume in lastclose
> - Add FBINFO_CAN_FORCE_OUTPUT flag so we get oops'es on the console
>
> drivers/gpu/drm/simpledrm/Kconfig | 3 +
> drivers/gpu/drm/simpledrm/Makefile | 1 +
> drivers/gpu/drm/simpledrm/simpledrm.h | 24 +
> drivers/gpu/drm/simpledrm/simpledrm_drv.c | 4 +
> drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 160
>
> drivers/gpu/drm/simpledrm/simpledrm_kms.c | 12 +++
> 6 files changed, 204 insertions(+)
> create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
>
> diff --git a/drivers/gpu/drm/simpledrm/Kconfig
> b/drivers/gpu/drm/simpledrm/Kconfig
> index f45b25d..9454536 100644
> --- a/drivers/gpu/drm/simpledrm/Kconfig
> +++ b/drivers/gpu/drm/simpledrm/Kconfig
> @@ -13,6 +13,9 @@ config DRM_SIMPLEDRM
> SimpleDRM supports "simple-framebuffer" DeviceTree objects and
> compatible platform framebuffers.
>
> + If fbdev support is enabled, this driver will also provide an fbdev
> + compatibility layer.
> +
> If unsure, say Y.
>
> To compile this driver as a module, choose M here: the
> diff --git a/drivers/gpu/drm/simpledrm/Makefile
> b/drivers/gpu/drm/simpledrm/Makefile
> index f6a62dc..7087245 100644
> --- a/drivers/gpu/drm/simpledrm/Makefile
> +++ b/drivers/gpu/drm/simpledrm/Makefile
> @@ -1,4 +1,5 @@
> simpledrm-y := simpledrm_drv.o simpledrm_kms.o simpledrm_gem.o \
> simpledrm_damage.o
> +simpledrm-$(CONFIG_DRM_FBDEV_EMULATION) += simpledrm_fbdev.o
>
> obj-$(CONFIG_DRM_SIMPLEDRM) := simpledrm.o
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h
> b/drivers/gpu/drm/simpledrm/simpledrm.h
> index f9f082c..eb18d59 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm.h
> +++ b/drivers/gpu/drm/simpledrm/simpledrm.h
> @@ -30,6 +30,7 @@ struct sdrm_device {
> struct drm_device *ddev;
> struct drm_simple_display_pipe pipe;
> struct drm_connector conn;
> + struct fb_info *fbdev;
>
> /* framebuffer information */
> const struct simplefb_format *fb_sformat;
> @@ -52,6 +53,7 @@ struct sdrm_device {
> #endif
> };
>
> +void sdrm_lastclose(struct drm_device *ddev);
> int sdrm_drm_modeset_init(struct sdrm_device *sdrm);
> int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma);
>
> @@ -93,4 +95,26 @@ struct sdrm_framebuffer {
>
> #define to_sdrm_fb(x) container_of(x, struct sdrm_framebuffer, base)
>
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
> +
> +void sdrm_fbdev_init(struct sdrm_device *sdrm);
> +void sdrm_fbdev_cleanup(struct sdrm_device *sdrm);
> +void sdrm_fbdev_set_suspend(struct sdrm_device *sdrm, int state);
> +
> +#else
> +
> +static inline void sdrm_fbdev_init(struct sdrm_device *sdrm)
> +{
> +}
> +
> +static inline void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
> +{
> +}
> +
> +static inline void sdrm_fbdev_set_suspend(struct sdrm_device *sdrm, int
> state)
> +{
> +}
> +
> +#endif
> +
> #endif /* SDRM_DRV_H */
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> index 35296d2..88ad717c 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> @@ -41,6 +41,7 @@ static struct drm_driver sdrm_drm_driver = {
> .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
>DRIVER_ATOMIC,
> .fops = &sdrm_drm_fops,
> + .lastclose = sdrm_lastclose,
>
> .gem_free_object = sdrm_gem_free_object,
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> @@ -447,6 +448,8 @@ static int sdrm_simplefb_probe(struct platform_device
> *pdev)
> if (ret)
> goto err_regulators;
>
> + sdrm_fbdev_init(ddev->dev_private);
> +
> DRM_INFO("Initialized %s on minor %d\n", ddev->driver->name,
> ddev->primary->index);
>
> @@ -472,6 +475,7 @@ static int sdrm_simplefb_remove(struct platform_device
> *pdev)
> struct drm_device *ddev = platform_get_drvdata(pdev);
> struct sdrm_device *sdrm = ddev->dev_private;
>
> + sdrm_fbdev_cleanup(sdrm);
> drm_dev_unregister(ddev);
> drm_mode_config_cleanup(ddev);
>
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> new file mode 100644
> index 000..b83646b
> --- /dev/null
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> @@ -0,0 +1,160 @@
> +/*
> + * SimpleDRM firmware framebuffer driver
>
[PATCH v3 5/7] PM / devfreq: event: support rockchip dfi controller
On Fri, Jul 22, 2016 at 5:07 AM, Lin Huang wrote: > on rk3399 platform, there is dfi conroller can monitor > ddr load, base on this result, we can do ddr freqency > scaling. > > Signed-off-by: Lin Huang > Acked-by: Chanwoo Choi > --- > Changes in v3: > - None > > Changes in v2: > - use clk_disable_unprepare and clk_enable_prepare > - remove clk_enable_prepare in probe > - remove rockchip_dfi_remove function > > Changes in v1: > - None > > drivers/devfreq/event/Kconfig| 7 + > drivers/devfreq/event/Makefile | 1 + > drivers/devfreq/event/rockchip-dfi.c | 253 > +++ > 3 files changed, 261 insertions(+) > create mode 100644 drivers/devfreq/event/rockchip-dfi.c > > diff --git a/drivers/devfreq/event/Kconfig b/drivers/devfreq/event/Kconfig > index eb6f74a..0d32d68 100644 > --- a/drivers/devfreq/event/Kconfig > +++ b/drivers/devfreq/event/Kconfig > @@ -30,4 +30,11 @@ config DEVFREQ_EVENT_EXYNOS_PPMU > (Platform Performance Monitoring Unit) counters to estimate the > utilization of each module. > > +config DEVFREQ_EVENT_ROCKCHIP_DFI > + bool "ROCKCHIP DFI DEVFREQ event Driver" > + depends on ARCH_ROCKCHIP > + help > + This add the devfreq-event driver for Rockchip SoC. It provides DFI > + (DDR Monitor Module) driver to count ddr load. > + > endif # PM_DEVFREQ_EVENT > diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile > index 3d6afd3..dda7090 100644 > --- a/drivers/devfreq/event/Makefile > +++ b/drivers/devfreq/event/Makefile > @@ -2,3 +2,4 @@ > > obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_NOCP) += exynos-nocp.o > obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o > +obj-$(CONFIG_DEVFREQ_EVENT_ROCKCHIP_DFI) += rockchip-dfi.o > diff --git a/drivers/devfreq/event/rockchip-dfi.c > b/drivers/devfreq/event/rockchip-dfi.c > new file mode 100644 > index 000..96a0307 > --- /dev/null > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -0,0 +1,253 @@ > +/* > + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd > + * Author: Lin Huang > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Same comment applies here. Also might want to check any instances of "IS_ENABLED" being used on bool items, as that is overkill too. Thanks, Paul.
[PATCH v3 6/7] PM / devfreq: rockchip: add devfreq driver for rk3399 dmc
On Fri, Jul 22, 2016 at 5:07 AM, Lin Huang wrote: > base on dfi result, we do ddr frequency scaling, register > dmc driver to devfreq framework, and use simple-ondemand > policy. > > Signed-off-by: Lin Huang > --- > Changes in v3: > - operate dram setting through sip call > - imporve set rate flow > > Changes in v2: > - None > > Changes in v1: > - move dfi controller to event > - fix set voltage sequence when set rate fail > - change Kconfig type from tristate to bool > - move unuse EXPORT_SYMBOL_GPL() > > drivers/devfreq/Kconfig | 1 + > drivers/devfreq/Makefile| 1 + > drivers/devfreq/rockchip/Kconfig| 15 + > drivers/devfreq/rockchip/Makefile | 2 + > drivers/devfreq/rockchip/rk3399_dmc.c | 482 > > drivers/devfreq/rockchip/rockchip_dmc.c | 143 ++ > include/soc/rockchip/rockchip_dmc.h | 45 +++ > 7 files changed, 689 insertions(+) > create mode 100644 drivers/devfreq/rockchip/Kconfig > create mode 100644 drivers/devfreq/rockchip/Makefile > create mode 100644 drivers/devfreq/rockchip/rk3399_dmc.c > create mode 100644 drivers/devfreq/rockchip/rockchip_dmc.c > create mode 100644 include/soc/rockchip/rockchip_dmc.h > > diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig > index a5be56e..cb67246 100644 > --- a/drivers/devfreq/Kconfig > +++ b/drivers/devfreq/Kconfig > @@ -101,5 +101,6 @@ config ARM_TEGRA_DEVFREQ > operating frequencies and voltages with OPP support. > > source "drivers/devfreq/event/Kconfig" > +source "drivers/devfreq/rockchip/Kconfig" > > endif # PM_DEVFREQ > diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile > index 09f11d9..48e2ae6 100644 > --- a/drivers/devfreq/Makefile > +++ b/drivers/devfreq/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o > # DEVFREQ Drivers > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > obj-$(CONFIG_ARM_TEGRA_DEVFREQ)+= tegra-devfreq.o > +obj-$(CONFIG_ARCH_ROCKCHIP)+= rockchip/ > > # DEVFREQ Event Drivers > obj-$(CONFIG_PM_DEVFREQ_EVENT) += event/ > diff --git a/drivers/devfreq/rockchip/Kconfig > b/drivers/devfreq/rockchip/Kconfig > new file mode 100644 > index 000..7fb1cff > --- /dev/null > +++ b/drivers/devfreq/rockchip/Kconfig > @@ -0,0 +1,15 @@ > +config ARM_ROCKCHIP_DMC_DEVFREQ > + bool "ARM ROCKCHIP DMC DEVFREQ Driver" > + depends on ARCH_ROCKCHIP > + help > + This adds the DEVFREQ driver framework for the rockchip dmc. > + > +config ARM_RK3399_DMC_DEVFREQ > + bool "ARM RK3399 DMC DEVFREQ Driver" Since you are using bool Kconfigs for your driver, please do not use module.h or MODULE_ tags in your driver, and use the builtin register function. Alternatively if there really is a use case for it to be a modular driver then use a tristate Kconfig. THanks, Paul. -- > + depends on ARM_ROCKCHIP_DMC_DEVFREQ > + select PM_OPP > + select DEVFREQ_GOV_SIMPLE_ONDEMAND > + help > + This adds the DEVFREQ driver for the RK3399 dmc. It sets the > frequency > + for the memory controller and reads the usage counts from hardware. > +
[PATCH 01/16] gpu: ipu-v3: Add Video Deinterlacer unit
On Thu, Jul 7, 2016 at 7:03 PM, Steve Longerbeam
wrote:
> Adds the Video Deinterlacer (VDIC) unit.
>
> Signed-off-by: Steve Longerbeam
> ---
> drivers/gpu/ipu-v3/Makefile | 2 +-
> drivers/gpu/ipu-v3/ipu-common.c | 11 ++
> drivers/gpu/ipu-v3/ipu-prv.h| 6 +
> drivers/gpu/ipu-v3/ipu-vdi.c| 266
>
> include/video/imx-ipu-v3.h | 27
> 5 files changed, 311 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/ipu-v3/ipu-vdi.c
>
> diff --git a/drivers/gpu/ipu-v3/Makefile b/drivers/gpu/ipu-v3/Makefile
> index 107ec23..aeba9dc 100644
> --- a/drivers/gpu/ipu-v3/Makefile
> +++ b/drivers/gpu/ipu-v3/Makefile
> @@ -1,4 +1,4 @@
> obj-$(CONFIG_IMX_IPUV3_CORE) += imx-ipu-v3.o
>
> imx-ipu-v3-objs := ipu-common.o ipu-cpmem.o ipu-csi.o ipu-dc.o ipu-di.o \
> - ipu-dp.o ipu-dmfc.o ipu-ic.o ipu-smfc.o
> + ipu-dp.o ipu-dmfc.o ipu-ic.o ipu-smfc.o ipu-vdi.o
> diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
> index 99dcacf..30dc115 100644
> --- a/drivers/gpu/ipu-v3/ipu-common.c
> +++ b/drivers/gpu/ipu-v3/ipu-common.c
> @@ -833,6 +833,14 @@ static int ipu_submodules_init(struct ipu_soc *ipu,
> goto err_ic;
> }
>
> + ret = ipu_vdi_init(ipu, dev, ipu_base + devtype->vdi_ofs,
> + IPU_CONF_VDI_EN | IPU_CONF_ISP_EN |
> + IPU_CONF_IC_INPUT);
> + if (ret) {
> + unit = "vdi";
> + goto err_vdi;
> + }
> +
> ret = ipu_di_init(ipu, dev, 0, ipu_base + devtype->disp0_ofs,
> IPU_CONF_DI0_EN, ipu_clk);
> if (ret) {
> @@ -887,6 +895,8 @@ err_dc:
> err_di_1:
> ipu_di_exit(ipu, 0);
> err_di_0:
> + ipu_vdi_exit(ipu);
> +err_vdi:
> ipu_ic_exit(ipu);
> err_ic:
> ipu_csi_exit(ipu, 1);
> @@ -971,6 +981,7 @@ static void ipu_submodules_exit(struct ipu_soc *ipu)
> ipu_dc_exit(ipu);
> ipu_di_exit(ipu, 1);
> ipu_di_exit(ipu, 0);
> + ipu_vdi_exit(ipu);
> ipu_ic_exit(ipu);
> ipu_csi_exit(ipu, 1);
> ipu_csi_exit(ipu, 0);
> diff --git a/drivers/gpu/ipu-v3/ipu-prv.h b/drivers/gpu/ipu-v3/ipu-prv.h
> index bfb1e8a..845f64c 100644
> --- a/drivers/gpu/ipu-v3/ipu-prv.h
> +++ b/drivers/gpu/ipu-v3/ipu-prv.h
> @@ -138,6 +138,7 @@ struct ipu_dc_priv;
> struct ipu_dmfc_priv;
> struct ipu_di;
> struct ipu_ic_priv;
> +struct ipu_vdi;
> struct ipu_smfc_priv;
>
> struct ipu_devtype;
> @@ -169,6 +170,7 @@ struct ipu_soc {
> struct ipu_di *di_priv[2];
> struct ipu_csi *csi_priv[2];
> struct ipu_ic_priv *ic_priv;
> + struct ipu_vdi *vdi_priv;
> struct ipu_smfc_priv*smfc_priv;
> };
>
> @@ -199,6 +201,10 @@ int ipu_ic_init(struct ipu_soc *ipu, struct device *dev,
> unsigned long base, unsigned long tpmem_base);
> void ipu_ic_exit(struct ipu_soc *ipu);
>
> +int ipu_vdi_init(struct ipu_soc *ipu, struct device *dev,
> +unsigned long base, u32 module);
> +void ipu_vdi_exit(struct ipu_soc *ipu);
> +
> int ipu_di_init(struct ipu_soc *ipu, struct device *dev, int id,
> unsigned long base, u32 module, struct clk *ipu_clk);
> void ipu_di_exit(struct ipu_soc *ipu, int id);
> diff --git a/drivers/gpu/ipu-v3/ipu-vdi.c b/drivers/gpu/ipu-v3/ipu-vdi.c
> new file mode 100644
> index 000..1303bcc
> --- /dev/null
> +++ b/drivers/gpu/ipu-v3/ipu-vdi.c
> @@ -0,0 +1,266 @@
> +/*
> + * Copyright (C) 2012 Mentor Graphics Inc.
> + * Copyright (C) 2005-2009 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +#include
> +#include
You have a u32 field in a struct called "modules" but aside from that, I do not
see anything in this code requiring module.h -- did I miss something?
You might want export.h for EXPORT_SYMBOL though.
Paul.
--
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "ipu-prv.h"
> +
[...]
[Intel-gfx] [v3 0/7] Crystalcove (CRC) PMIC based panel and pwm control
[Re: [Intel-gfx] [v3 0/7] Crystalcove (CRC) PMIC based panel and pwm control] On 26/06/2015 (Fri 20:47) Ville Syrjälä wrote: > On Fri, Jun 26, 2015 at 06:31:37PM +0200, Daniel Vetter wrote: > > On Fri, Jun 26, 2015 at 02:32:03PM +0530, Shobhit Kumar wrote: > > > Hi, > > > Next update of the series reviewed at > > > https://lkml.org/lkml/2015/6/22/155 > > > > > > Major changes are few review comments from Varka and Ville being > > > addressed. Also except > > > for intel-gfx patches, all patches reviesion history is moved out of > > > commit message. > > > > > > Hope this series finally finds its mark. > > > > > > Regards > > > Shobhit > > > > > > Shobhit Kumar (7): > > > gpiolib: Add support for removing registered consumer lookup table > > > mfd: intel_soc_pmic_core: Add lookup table for Panel Control as GPIO > > > signal > > > mfd: intel_soc_pmic_crc: Add PWM cell device for Crystalcove PMIC > > > mfd: intel_soc_pmic_core: ADD PWM lookup table for CRC PMIC based PWM > > > pwm: crc: Add Crystalcove (CRC) PWM driver > > > drm/i915: Use the CRC gpio for panel enable/disable > > > drm/i915: Backlight control using CRC PMIC based PWM driver > > > > I think we have r-b/acks on all the patches now. Ok if I pull this in > > through drm-intel.git for 4.3? Or should I make a topic branch with tag > > and then send out pull requests to everyone? Or will each maintainer merge > > on their own since it's all only coupled at runtime anyway? Any of these > > would suit me. > > I forgot to mention that I had a build failure due to > builtin_platform_driver() when I tried this (just changed it to > module_platform_driver() to get past it). So I'm not sure if this > now depends on some tree which isn't included in -nightly... builtin_platform_register does not yet exist in mainline; as Paul (the other one) said earlier. So you can either open-code what it does for now, or use module_platform_register. If you do the latter, then ensure you (temorarily) also include module.h or you risk additional breakage in the future. Paul. -- > > -- > Ville Syrjälä > Intel OTC
[Intel-gfx] [PATCH 6/8] drivers/pwm: Add Crystalcove (CRC) PWM driver
[Re: [Intel-gfx] [PATCH 6/8] drivers/pwm: Add Crystalcove (CRC) PWM driver] On 20/06/2015 (Sat 13:23) Paul Bolle wrote: > [Added Paul Gortmaker.] > > Hi Shobhit, > > On Fri, 2015-06-19 at 12:16 +0530, Shobhit Kumar wrote: > > So what is the exact big problem with this ? > > The main problem I have is that it's hard to read a submitter's mind. > And, I think, in cases like this we need to know if the submitter just > made some silly mistake or that the mismatch (between Kconfig type and > code) was intentional. So each time such a mismatch is spotted the > submitter ought to be asked about it. > > (I'd guess that one or two new drivers are submitted _each_ day. And > these mismatches are quite common. I'd say I receive answers like: > - "Oops, yes bool should have been tristate"; or > - "Oops, forgot to clean up after noticing tristate didn't work"; or > - "I just copy-and-pasted a similar driver, the module stuff isn't > actually needed" > at least once a week. Not sure, I don't keep track of this stuff.) > > Furthermore, it appears that Paul Gortmaker is on a mission to, badly > summarized, untangle the module and init code. See for instance > https://lkml.org/lkml/2015/5/28/809 and > https://lkml.org/lkml/2015/5/31/205 . > > Now, I don't know whether (other) Paul is bothered by these MODULE_* > macros. But Paul did submit a series that adds Yes, I agree that it would be nice to not see these mismatches, regardless of whether we can get away with it or not. > builtin_platform_driver(), see https://lkml.org/lkml/2015/5/10/131 . > That new macro ensures built-in only code doesn't have to use > module_platform_driver(), which your patch also uses. So perhaps Paul > can explain some of the non-obvious issues caused by built-in only code > using module specific constructs. In https://lkml.org/lkml/2015/5/10/125 I'd written: There are several downsides to this: 1) The code can appear modular to a reader of the code, and they won't know if the code really is modular without checking the Makefile and Kconfig to see if compilation is governed by a bool or tristate. 2) Coders of drivers may be tempted to code up an __exit function that is never used, just in order to satisfy the required three args of the modular registration function. 3) Non-modular code ends up including the which increases CPP overhead that they don't need. 4) It hinders us from performing better separation of the module init code and the generic init code. The nature of linux means that thousands of developers are reading the code every day -- so I think that there is a genuine value in having the code convey a clear message on how it was designed to be used. Only using module related headers/macros for genuinely modular code helps us (albeit in a small way) towards achieving that. Looking at this thread, I see that one of the reasons given for this code's ambiguous module vs. built-in identity was the observation of a similar identity crisis of the related INTEL_SOC_PMIC code. Does that not back up the point above about the value in having the code speak for itself? So IMHO we probably should clarify the PMIC code vs. adding another example that looks just like it. Paul. -- > > > I can anyway shove out these macros to end the discussion. > > I'd rather convince you than annoy you into doing as I suggested. > > > BTW whether you buy the argument or not, please do treat yourself > > with ice cream for being able to make such a comment. > > Will do. > > Thanks, > > > Paul Bolle >
[PATCH 04/11] drivers/gpu: include for modular rockchip code
These files are built off of a tristate Kconfig option and also contain modular function calls so they should explicitly include module.h to avoid compile breakage during header shuffles done in the future. Cc: David Airlie Cc: Mark Yao Cc: dri-devel at lists.freedesktop.org Signed-off-by: Paul Gortmaker --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 + drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 3962176ee713..01b558fe3695 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index ccb0ce073ef2..38155215efcd 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -19,6 +19,7 @@ #include #include +#include #include #include #include -- 2.2.1
[PATCH 71/73] drivers/gpu: delete non-required instances of
None of these files are actually using any __init type directives and hence don't need to include . Most are just a left over from __devinit and __cpuinit removal, or simply due to code getting copied from one driver to the next. Cc: David Airlie Cc: Daniel Vetter Cc: Jani Nikula Cc: dri-devel at lists.freedesktop.org Cc: intel-gfx at lists.freedesktop.org Signed-off-by: Paul Gortmaker --- drivers/gpu/drm/ast/ast_fb.c | 1 - drivers/gpu/drm/drm_dp_helper.c | 1 - drivers/gpu/drm/gma500/accel_2d.c| 1 - drivers/gpu/drm/gma500/framebuffer.c | 1 - drivers/gpu/drm/i915/intel_fbdev.c | 1 - drivers/gpu/drm/nouveau/nouveau_fbcon.c | 1 - drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 1 - 7 files changed, 7 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c index 7b33e14..e4a35c1 100644 --- a/drivers/gpu/drm/ast/ast_fb.c +++ b/drivers/gpu/drm/ast/ast_fb.c @@ -34,7 +34,6 @@ #include #include #include -#include #include diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 9e978aa..d96e445 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include diff --git a/drivers/gpu/drm/gma500/accel_2d.c b/drivers/gpu/drm/gma500/accel_2d.c index de6f62a..426e8dd 100644 --- a/drivers/gpu/drm/gma500/accel_2d.c +++ b/drivers/gpu/drm/gma500/accel_2d.c @@ -29,7 +29,6 @@ #include #include #include -#include #include #include diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index 94b3fec..6a3aa1b 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 39eac99..de80c29 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -33,7 +33,6 @@ #include #include #include -#include #include #include diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 7903e0e..3da3999 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -33,7 +33,6 @@ #include #include #include -#include #include #include #include diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c index 701c4c1..d7f1c85 100644 --- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c +++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c @@ -15,7 +15,6 @@ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. */ -#include #include #include /* platform_device() */ #include -- 1.8.4.1
