[Bug 89909] [radeonsi][bisected] Cities: Skylines black rooftops
https://bugs.freedesktop.org/show_bug.cgi?id=89909 John Brooks changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #5 from John Brooks --- No longer seems to be happening as of 11.0.3 (914966befcd57764941405707d8f57d3e7e7f768) Thank you to the development team. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/1ce54374/attachment.html>
[Bug 92526] latest llvm 3.8 git + mesa git crashes kernel 4.3.0 rc6
ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Kernel driver in use: ahci 00:1f.3 SMBus: Intel Corporation 8 Series SMBus Controller (rev 04) Subsystem: Dell Device 0609 Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- TAbort- SERR- Kernel driver in use: r8169 02:00.0 Network controller: Qualcomm Atheros QCA9565 / AR9565 Wireless Network Adapter (rev 01) Subsystem: Dell Device 020c Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Kernel driver in use: ath9k 03:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Venus PRO [Radeon HD 8850M / R9 M265X] (rev ff) (prog-if ff) !!! Unknown header type 7f Kernel driver in use: radeon -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/853649dc/attachment-0001.html>
[Bug 92526] latest llvm 3.8 git + mesa git crashes kernel 4.3.0 rc6
https://bugs.freedesktop.org/show_bug.cgi?id=92526 Paulo Dias changed: What|Removed |Added Priority|medium |high CC||paulo.miguel.dias at gmail.com Hardware|Other |x86-64 (AMD64) Version|unspecified |git Severity|normal |critical -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/bb4ab4c8/attachment.html>
[Bug 77835] X crash when using xrandr with R9 290X and 7870 for triple head
https://bugs.freedesktop.org/show_bug.cgi?id=77835 --- Comment #5 from Michel Dänzer --- Does this still happen with current versions of xf86-video-ati and xserver? If so, can you get a gdb backtrace of the crash? See http://wiki.x.org/wiki/Development/Documentation/ServerDebugging/ . -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/321755bf/attachment.html>
[PATCH] drm/sti: Remove select of CONFIG_FW_LOADER_USER_HELPER_FALLBACK
The commit [4fdbc678fe4d: drm: sti: add HQVDP plane] added the select
of CONFIG_FW_LOADER_USER_HELPER_FALLBACK by some unwritten reason.
But this config is known to be harmful, and is present only for
compatibility reason for an old exotic system that mandates udev
interaction which isn't supposed to be selected by a driver.
Let's remove it.
Fixes: 4fdbc678fe4d ('drm: sti: add HQVDP plane')
Cc:
Signed-off-by: Takashi Iwai
---
drivers/gpu/drm/sti/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
index fbccc105819b..a18159074b76 100644
--- a/drivers/gpu/drm/sti/Kconfig
+++ b/drivers/gpu/drm/sti/Kconfig
@@ -6,7 +6,6 @@ config DRM_STI
select DRM_GEM_CMA_HELPER
select DRM_KMS_CMA_HELPER
select DRM_PANEL
- select FW_LOADER_USER_HELPER_FALLBACK
help
Choose this option to enable DRM on STM stiH41x chipset
--
2.6.1
[PATCH] drm/virtio: use %llu format string form atomic64_t
On Wed, Oct 7, 2015 at 1:23 PM, Arnd Bergmann wrote:
> On Wednesday 07 October 2015 13:04:06 Arnd Bergmann wrote:
>> On Wednesday 07 October 2015 11:45:02 Russell King - ARM Linux wrote:
>> > On Wed, Oct 07, 2015 at 12:41:21PM +0200, Arnd Bergmann wrote:
>> > > The virtgpu driver prints the last_seq variable using the %ld or
>> > > %lu format string, which does not work correctly on all architectures
>> > > and causes this compiler warning on ARM:
>> > >
>> > > drivers/gpu/drm/virtio/virtgpu_fence.c: In function
>> > > 'virtio_timeline_value_str':
>> > > drivers/gpu/drm/virtio/virtgpu_fence.c:64:22: warning: format '%lu'
>> > > expects argument of type 'long unsigned int', but argument 4 has type
>> > > 'long long int' [-Wformat=]
>> > > snprintf(str, size, "%lu", atomic64_read(&fence->drv->last_seq));
>> > > ^
>> > > drivers/gpu/drm/virtio/virtgpu_debugfs.c: In function
>> > > 'virtio_gpu_debugfs_irq_info':
>> > > drivers/gpu/drm/virtio/virtgpu_debugfs.c:37:16: warning: format '%ld'
>> > > expects argument of type 'long int', but argument 3 has type 'long long
>> > > int' [-Wformat=]
>> > > seq_printf(m, "fence %ld %lld\n",
>> > > ^
>> > >
>> > > In order to avoid the warnings, this changes the format strings to %llu
>> > > and adds a cast to u64, which makes it work the same way everywhere.
>> >
>> > You have to wonder why atomic64_* functions do not use u64 types.
>> > If they're not reliant on manipulating 64-bit quantities, then what's
>> > the point of calling them atomic _64_.
>>
>> I haven't checked all architectures, but I assume what happens is that
>> 64-bit ones just #define atomic64_t atomic_long_t, so they don't have
>> to provide three sets of functions.
>
> scratch that, I just looked at all the architectures and found that it's
> just completely arbitrary, even within one architecture you get a mix
> of 'long' and 'long long', plus this gem from MIPS:
>
> static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)
>
> which truncates the result to 32 bit.
Woops.
See also my unanswered question in "atomic64 on 32-bit vs 64-bit (was:
Re: Add virtio gpu driver.)", which is still valid:
https://lkml.org/lkml/2015/6/28/18
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at
linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH] drm: fix mutex leak in drm_dp_get_mst_branch_device
On Fri, 16 Oct 2015, Daniel Vetter wrote:
> On Fri, Oct 16, 2015 at 03:33:02AM -0700, Adam Richter wrote:
>> In Linux 4.3-rc5, there is an error case in drm_dp_get_branch_device
>> that returns without releasing mgr->lock, resulting a spew of kernel
>> messages about a kernel work function possibly having leaked a mutex
>> and presumably more serious adverse consequences later. This patch
>> changes the error to "goto out" to unlock the mutex before returning.
>>
>> Signed-off-by: Adam J. Richter
>
> Patch was whitespace mangled (don't past them into your mailer but instead
> send them with git send-email directly, that's safer), so I had to
> recreate it.
>
> Applied to drm-misc, thanks.
It's missing
Reviewed-by: Jani Nikula
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92480
Fixes: 9eb1e57f564d ("drm/dp/mst: take lock around looking up the branch device
on hpd irq")
Cc: stable at vger.kernel.org # v4.2+
and arguably was for drm-fixes.
BR,
Jani.
> -Daniel
>
>> ---
>>
>> bugzilla.freedesktop.org ticket:
>> https://bugs.freedesktop.org/show_bug.cgi?id=92480
>>
>> This patch is against the latest
>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git . I
>> apologize in advance in I should be submitting patches against some
>> other reference. Please feel free to let me know if I should use some
>> other reference for submitting future patches.
>>
>> My request to subscribe to dri-devel at lists.freedesktop.org is pending,
>> so I apologize if it takes a while for this message to be posted on
>> the mailing list.
>>
>> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
>> b/drivers/gpu/drm/drm_dp_mst_topology.c
>> index bf27a07..018ad7f 100644
>> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
>> @@ -1194,17 +1194,18 @@ static struct drm_dp_mst_branch
>> *drm_dp_get_mst_branch_device(struct drm_dp_mst_
>>
>> list_for_each_entry(port, &mstb->ports, next) {
>> if (port->port_num == port_num) {
>> - if (!port->mstb) {
>> + mstb = port->mstb;
>> + if (!mstb) {
>> DRM_ERROR("failed to lookup
>> MSTB with lct %d, rad %02x\n", lct, rad[0]);
>> - return NULL;
>> + goto out;
>> }
>>
>> - mstb = port->mstb;
>> break;
>> }
>> }
>> }
>> kref_get(&mstb->kref);
>> + out:
>> mutex_unlock(&mgr->lock);
>> return mstb;
>> }
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Jani Nikula, Intel Open Source Technology Center
[Bug 86320] Monitor on DisplayPort doesn't wake up
https://bugs.freedesktop.org/show_bug.cgi?id=86320 --- Comment #14 from Morten Leikvoll --- I should warn that some displays does not seem to follow the displayport standard properly. I would recommend trying a different one, and if its related to some models, discuss what to do. I dont have an answer for that. I am working on a embedded displayport source here and find a Dell U2913 2560x1080 monitor (It has a STDP9320-BB "Athena" displayport sink, with unknown firmware) does not report a correct SINK_STATUS (DPCD register 0x00205, bit 0) wich the specification says it should do. Also its seem to lock the LINK_STATUS_UPDATED bit to 1 (in DPCD register 0x00204) But even if this is wrong, it still shows an image. Googling my symptom made me find this thread. Wether to skip this test is a good idea is up for discussion. For an home/office environment, its probably ok, but for an industrial environment it may not be ok. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/cdee5153/attachment.html>
[Bug 91896] AMDGPU Fiji: only getting 1080i on DP
https://bugs.freedesktop.org/show_bug.cgi?id=91896 Christian König changed: What|Removed |Added Status|RESOLVED|CLOSED --- Comment #21 from Christian König --- And the patch should even have a stable tag on it so it goes into 4.2 as well. Anyway code is on it's way and I think we can close this ticket. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/97a2b8b9/attachment-0001.html>
[Bug 89887] AMD 3650 Mobility locks up on overdraw
https://bugs.freedesktop.org/show_bug.cgi?id=89887 --- Comment #9 from Christian König --- (In reply to Marek Olšák from comment #8) > I will close this as NOTABUG. Please try again with a lower instance count > and make sure it finishes in 10 seconds. Alternatively use the kernel parameter radeon.lockup_timeout=2 to increase the timeout to 20 seconds. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/866ac432/attachment.html>
[Intel-gfx] [PATCH] drm: Explicitly compute the last cacheline for clflush on range
On Sun, Oct 18, 2015 at 05:07:06PM +0100, Chris Wilson wrote:
> On Sun, Oct 18, 2015 at 02:07:13PM +0100, Chris Wilson wrote:
> > > I couldn't spot the difference either. I am beginning to suspect it is
> > > gcc as
> > >
> > > diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
> > > index 6743ff7..c9097b5 100644
> > > --- a/drivers/gpu/drm/drm_cache.c
> > > +++ b/drivers/gpu/drm/drm_cache.c
> > > @@ -130,11 +130,11 @@ drm_clflush_virt_range(void *addr, unsigned long
> > > length)
> > > {
> > > #if defined(CONFIG_X86)
> > > if (cpu_has_clflush) {
> > > const int size = boot_cpu_data.x86_clflush_size;
> > > - void *end = addr + length;
> > > + void *end = addr + length - 1;
> > > addr = (void *)(((unsigned long)addr) & -size);
> > > mb();
> > > - for (; addr < end; addr += size)
> > > + for (; addr <= end; addr += size)
> > > clflushopt(addr);
> > > mb();
> > > return;
> >
> > s/clflushopt/clflush/ works just as well.
> >
> > Plot thickens. Current guess is that gcc doesn't see the constraints
> > underneath the alternative()?
>
> Adding a barrier() after clflushopt() in the loop is sufficient as well.
> Almost certain that alternative() is confusing gcc.
So adding that barrier() to clflushopt with a massive comment that gcc
gets confused?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[RFC v4 02/11] drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.
On Fri, Oct 16, 2015 at 10:12:04PM +0200, Philipp Zabel wrote: > From: CK Hu > > This patch adds an initial DRM driver for the Mediatek MT8173 DISP > subsystem. It currently supports two fixed output streams from the > OVL0/OVL1 sources to the DSI0/DPI0 sinks, respectively. > > Signed-off-by: CK Hu > Signed-off-by: YT Shen > Signed-off-by: Philipp Zabel Bunch of drive-by comments below to point out deprecated functions and more common approaches used by other drivers. Don't consider this a full review ;-) Cheers, Daniel > --- > Changes since v3: > - Removed crtc enabling/disabling on bind/unbind, the enable/disable >callbacks should suffice. > - Find sibling components in the device tree via compatible value btw for DT components stuff there's piles of RFCs floating around to extract this into helper libraries. Would be great we could push one of them forward. > - Split component type and instance id enums > - Determine component type from compatible, component instance id from >type and of alias > - Added remaining DSI, RDMA and WDMA ids > - Moved register mapping and mutex node search into probe function > - Bind drm driver to mediatek,mt8173-mmsys compatible node > - Do not try to call dma_free_attrs if dma_alloc_attrs failed. > --- > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile| 1 + > drivers/gpu/drm/mediatek/Kconfig| 16 + > drivers/gpu/drm/mediatek/Makefile | 10 + > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 524 ++ > drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 84 + > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 218 +++ > drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 39 ++ > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 409 > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 86 + > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 556 > > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 61 +++ > drivers/gpu/drm/mediatek/mtk_drm_fb.c | 151 > drivers/gpu/drm/mediatek/mtk_drm_fb.h | 29 ++ > drivers/gpu/drm/mediatek/mtk_drm_gem.c | 208 +++ > drivers/gpu/drm/mediatek/mtk_drm_gem.h | 56 +++ > drivers/gpu/drm/mediatek/mtk_drm_plane.c| 175 + > drivers/gpu/drm/mediatek/mtk_drm_plane.h| 38 ++ > 18 files changed, 2663 insertions(+) > create mode 100644 drivers/gpu/drm/mediatek/Kconfig > create mode 100644 drivers/gpu/drm/mediatek/Makefile > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_crtc.c > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_crtc.h > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_ddp.c > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_ddp.h > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_drv.c > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_drv.h > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fb.c > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_fb.h > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_gem.c > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_gem.h > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_plane.c > create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_plane.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 1a0a8df..9e9987b 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -264,3 +264,5 @@ source "drivers/gpu/drm/sti/Kconfig" > source "drivers/gpu/drm/amd/amdkfd/Kconfig" > > source "drivers/gpu/drm/imx/Kconfig" > + > +source "drivers/gpu/drm/mediatek/Kconfig" > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 45e7719..af6b592 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -67,6 +67,7 @@ obj-$(CONFIG_DRM_MSM) += msm/ > obj-$(CONFIG_DRM_TEGRA) += tegra/ > obj-$(CONFIG_DRM_STI) += sti/ > obj-$(CONFIG_DRM_IMX) += imx/ > +obj-$(CONFIG_DRM_MEDIATEK) += mediatek/ > obj-y+= i2c/ > obj-y+= panel/ > obj-y+= bridge/ > diff --git a/drivers/gpu/drm/mediatek/Kconfig > b/drivers/gpu/drm/mediatek/Kconfig > new file mode 100644 > index 000..5343cf1 > --- /dev/null > +++ b/drivers/gpu/drm/mediatek/Kconfig > @@ -0,0 +1,16 @@ > +config DRM_MEDIATEK > + tristate "DRM Support for Mediatek SoCs" > + depends on DRM > + depends on ARCH_MEDIATEK || (ARM && COMPILE_TEST) > + select MTK_SMI > + select DRM_PANEL > + select DRM_MIPI_DSI > + select DRM_PANEL_SIMPLE > + select DRM_KMS_HELPER > + select IOMMU_DMA > + help > + Choose this option if you have a Mediatek SoCs. > + The module will be called mediatek-drm > + This driver provides kernel mode setting and > + buffer management to us
[PATCH] drm/virtio: use %llu format string form atomic64_t
On Wed, Oct 07, 2015 at 01:23:07PM +0200, Arnd Bergmann wrote: > > I haven't checked all architectures, but I assume what happens is that > > 64-bit ones just #define atomic64_t atomic_long_t, so they don't have > > to provide three sets of functions. > > scratch that, I just looked at all the architectures and found that it's > just completely arbitrary, even within one architecture you get a mix > of 'long' and 'long long', plus this gem from MIPS: > > static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u) > > which truncates the result to 32 bit. Eh... The result is 0/1 so nothing is truncated. Alpha, MIPS, PARISC and PowerPC are using the same prototype and x86 only differs in the use of inline instead __inline__. And anyway, that function on MIPS is only built for CONFIG_64BIT. What's wrong on MIPS is the comment describing the function's return value which was changed by f24219b4e90cf70ec4a211b17fbabc725a0ddf3c (atomic: move atomic_add_unless to generic code) and I've queued up a patch to fix that since a few days. I guess that was a cut and paste error from __atomic_add_unless which indeed does return the old value. Ralf
[PATCH] x86: Add an explicit barrier() to clflushopt()
During testing we observed that the last cacheline was not being flushed from a mb() for (addr = addr & -clflush_size; addr < end; addr += clflush_size) clflushopt(); mb() loop (where the initial addr and end were not cacheline aligned). Changing the loop from addr < end to addr <= end, or replacing the clflushopt() with clflush() both fixed the testcase. Hinting that GCC was miscompling the assembly within the loop and specifically the alternative within clflushopt() was confusing the loop optimizer. Adding a barrier() into clflushopt() is enough for GCC to dtrt, but solving why GCC is not seeing the constraints from the alternative_io() would be smarter... Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92501 Testcase: gem_tiled_partial_pwrite_pread/read Signed-off-by: Chris Wilson Cc: Ross Zwisler Cc: H. Peter Anvin Cc: Imre Deak Cc: Daniel Vetter Cc: dri-devel at lists.freedesktop.org --- arch/x86/include/asm/special_insns.h | 5 + 1 file changed, 5 insertions(+) diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h index 2270e41b32fd..0c7aedbf8930 100644 --- a/arch/x86/include/asm/special_insns.h +++ b/arch/x86/include/asm/special_insns.h @@ -199,6 +199,11 @@ static inline void clflushopt(volatile void *__p) ".byte 0x66; clflush %P0", X86_FEATURE_CLFLUSHOPT, "+m" (*(volatile char __force *)__p)); + /* GCC (4.9.1 and 5.2.1 at least) appears to be very confused when +* meeting this alternative() and demonstrably miscompiles loops +* iterating over clflushopts. +*/ + barrier(); } static inline void clwb(volatile void *__p) -- 2.6.1
[PATCH] drm/virtio: use %llu format string form atomic64_t
On Monday 19 October 2015 11:37:00 Ralf Baechle wrote: > On Wed, Oct 07, 2015 at 01:23:07PM +0200, Arnd Bergmann wrote: > > > > I haven't checked all architectures, but I assume what happens is that > > > 64-bit ones just #define atomic64_t atomic_long_t, so they don't have > > > to provide three sets of functions. > > > > scratch that, I just looked at all the architectures and found that it's > > just completely arbitrary, even within one architecture you get a mix > > of 'long' and 'long long', plus this gem from MIPS: > > > > static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u) > > > > which truncates the result to 32 bit. > > Eh... The result is 0/1 so nothing is truncated. Alpha, MIPS, > PARISC and PowerPC are using the same prototype and x86 only differs > in the use of inline instead __inline__. And anyway, that function on > MIPS is only built for CONFIG_64BIT. Ah, got it. Sorry about that. > What's wrong on MIPS is the comment describing the function's return value > which was changed by f24219b4e90cf70ec4a211b17fbabc725a0ddf3c (atomic: move > atomic_add_unless to generic code) and I've queued up a patch to fix that > since a few days. I guess that was a cut and paste error from > __atomic_add_unless which indeed does return the old value. Thanks! Arnd
[PATCH] x86: Add an explicit barrier() to clflushopt()
On Mon, Oct 19, 2015 at 10:58:55AM +0100, Chris Wilson wrote: > During testing we observed that the last cacheline was not being flushed > from a > > mb() > for (addr = addr & -clflush_size; addr < end; addr += clflush_size) > clflushopt(); > mb() > > loop (where the initial addr and end were not cacheline aligned). > > Changing the loop from addr < end to addr <= end, or replacing the > clflushopt() with clflush() both fixed the testcase. Hinting that GCC > was miscompling the assembly within the loop and specifically the > alternative within clflushopt() was confusing the loop optimizer. > > Adding a barrier() into clflushopt() is enough for GCC to dtrt, but > solving why GCC is not seeing the constraints from the alternative_io() > would be smarter... Hmm, would something like adding the memory clobber to the alternative_io() definition work? --- diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 7bfc85bbb8ff..d923e5dacdb1 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -207,7 +207,7 @@ static inline int alternatives_text_reserved(void *start, void *end) /* Like alternative_input, but with a single output argument */ #define alternative_io(oldinstr, newinstr, feature, output, input...) \ asm volatile (ALTERNATIVE(oldinstr, newinstr, feature) \ - : output : "i" (0), ## input) + : output : "i" (0), ## input : "memory") /* Like alternative_io, but for replacing a direct call with another one. */ #define alternative_call(oldfunc, newfunc, feature, output, input...) \ -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply.
[PATCH] drm/virtio: use %llu format string form atomic64_t
On Monday 19 October 2015 09:34:15 Geert Uytterhoeven wrote: > On Wed, Oct 7, 2015 at 1:23 PM, Arnd Bergmann wrote: > > static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u) > > > > which truncates the result to 32 bit. > > Woops. > > See also my unanswered question in "atomic64 on 32-bit vs 64-bit (was: > Re: Add virtio gpu driver.)", which is still valid: > https://lkml.org/lkml/2015/6/28/18 > Regarding your question of > Instead of sprinkling casts, is there any good reason why atomic64_read() > and atomic64_t aren't "long long" everywhere, cfr. u64? I assume the answer is that some (all?) 64-bit architectures intentionally return 'long' here, in order for atomic_long_read() to return 'long' on all architectures, given the definitions from include/asm-generic/atomic-long.h We would have to either change those, or we have to pick between atomic_long_* or atomic64_* to have a consistent return type. Arnd
v4.3-rc4: i915: ThinkPad Yoga 12: *ERROR* The master control interrupt lied (SDE)! [regression]
could please upload the log before this error. thanks On Oct 14, 2015 3:14 PM, "Jani Nikula" wrote: > > On Wed, 14 Oct 2015, "Miramontes Caton, Jairo Daniel" < jairo.daniel.miramontes.caton at intel.com> wrote: > > Created bug in fdo bugzilla to keep track of this regression: > > https://bugs.freedesktop.org/show_bug.cgi?id=92454 > > Please disregard that, there's already a bug report at > https://bugs.freedesktop.org/show_bug.cgi?id=92084. > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Technology Center > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/a6fe7bdb/attachment.html>
[GIT PULL] On-demand device probing
On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote: > On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote: > > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote: > > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote: > > > > > I can't see adding calls like this all over the tree just to solve a > > > > bus-specific problem, you are adding of_* calls where they aren't > > > > needed, or wanted, at all. > > > > This isn't bus specific, I'm not sure what makes you say that? > > > You are making it bus-specific by putting these calls all over the tree > > in different bus subsystems semi-randomly for all I can determine. > > Do you mean firmware rather than bus here? I think that's the confusion > I have... Certainly, if it literally is adding of_* calls then that would seem to be gratuitously firmware-specific. Nothing should be using those these days; any new code should be using the generic device property APIs (except in special cases). -- dwmw2 -- next part -- A non-text attachment was scrubbed... Name: smime.p7s Type: application/x-pkcs7-signature Size: 5691 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/9cdace2e/attachment-0001.bin>
[GIT PULL] On-demand device probing
On Mon, Oct 19, 2015 at 10:44:41AM +0100, David Woodhouse wrote: > On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote: > > On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote: > > > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote: > > > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote: > > > > > > > I can't see adding calls like this all over the tree just to solve a > > > > > bus-specific problem, you are adding of_* calls where they aren't > > > > > needed, or wanted, at all. > > > > > > This isn't bus specific, I'm not sure what makes you say that? > > > > > You are making it bus-specific by putting these calls all over the tree > > > in different bus subsystems semi-randomly for all I can determine. > > > > Do you mean firmware rather than bus here? I think that's the confusion > > I have... > > Certainly, if it literally is adding of_* calls then that would seem to > be gratuitously firmware-specific. Nothing should be using those these > days; any new code should be using the generic device property APIs > (except in special cases). I asked Linus Walleij about that with the fwnode_get_named_gpiod() stuff, and Linus didn't seem to know how this should be used. It doesn't help that dev->fwnode is not initialised, but dev->of_node is. Are we supposed to grope around in dev->of_node for the embedded fwnode instead of using dev->fwnode? At the moment, at least to me, fwnode looks like some kind of experimental half-baked thing rather than a real usable solution. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[PATCH] drm/virtio: use %llu format string form atomic64_t
On Mon, Oct 19, 2015 at 12:11 PM, Arnd Bergmann wrote:
> On Monday 19 October 2015 09:34:15 Geert Uytterhoeven wrote:
>> On Wed, Oct 7, 2015 at 1:23 PM, Arnd Bergmann wrote:
>> > static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)
>> >
>> > which truncates the result to 32 bit.
>>
>> Woops.
>>
>> See also my unanswered question in "atomic64 on 32-bit vs 64-bit (was:
>> Re: Add virtio gpu driver.)", which is still valid:
>> https://lkml.org/lkml/2015/6/28/18
>>
>
> Regarding your question of
>
>> Instead of sprinkling casts, is there any good reason why atomic64_read()
>> and atomic64_t aren't "long long" everywhere, cfr. u64?
>
>
> I assume the answer is that some (all?) 64-bit architectures intentionally
> return 'long' here, in order for atomic_long_read() to return 'long' on
> all architectures, given the definitions from
> include/asm-generic/atomic-long.h
>
> We would have to either change those, or we have to pick between
> atomic_long_* or atomic64_* to have a consistent return type.
I guess the main reason is this comment in include/asm-generic/atomic-long.h,
which I hadn't noticed before:
* Casts for parameters are avoided for existing atomic functions in order to
* avoid issues with cast-as-lval under gcc 4.x and other limitations that the
* macros of a platform may have.
Still, it's a pity, as printing atomic_64 is one more place where casts are
needed in callers.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at
linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH] x86: Add an explicit barrier() to clflushopt()
On Mon, Oct 19, 2015 at 12:16:12PM +0200, Borislav Petkov wrote: > On Mon, Oct 19, 2015 at 10:58:55AM +0100, Chris Wilson wrote: > > Adding a barrier() into clflushopt() is enough for GCC to dtrt, but > > solving why GCC is not seeing the constraints from the alternative_io() > > would be smarter... > > Hmm, would something like adding the memory clobber to the > alternative_io() definition work? > > --- > diff --git a/arch/x86/include/asm/alternative.h > b/arch/x86/include/asm/alternative.h > index 7bfc85bbb8ff..d923e5dacdb1 100644 > --- a/arch/x86/include/asm/alternative.h > +++ b/arch/x86/include/asm/alternative.h > @@ -207,7 +207,7 @@ static inline int alternatives_text_reserved(void *start, > void *end) > /* Like alternative_input, but with a single output argument */ > #define alternative_io(oldinstr, newinstr, feature, output, input...) > \ > asm volatile (ALTERNATIVE(oldinstr, newinstr, feature) \ > - : output : "i" (0), ## input) > + : output : "i" (0), ## input : "memory") > > /* Like alternative_io, but for replacing a direct call with another one. */ > #define alternative_call(oldfunc, newfunc, feature, output, input...) > \ In order to add the clobbers, I had to adjust the macro slightly: +#define alternative_output(oldinstr, newinstr, feature, output)\ + asm volatile (ALTERNATIVE(oldinstr, newinstr, feature) \ + : output : "i" (0) : "memory") and that also works. The concern I have here is that the asm does use "+m" as its output already, and the clflush() asm doesn't require the manual clobber. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[Bug 91880] Radeonsi on Grenada cards (r9 390) exceptionally unstable and poorly performing
https://bugs.freedesktop.org/show_bug.cgi?id=91880 --- Comment #16 from John Frei --- In my case just a hard reset is the only option. The computer doesn't restore itself. But the screen still get a signal from the graphics card, which is just black with a red dotted noise on the left. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/7164ec0f/attachment.html>
[PATCH] x86: Add an explicit barrier() to clflushopt()
On Mon, Oct 19, 2015 at 12:05:29PM +0100, Chris Wilson wrote: > In order to add the clobbers, I had to adjust the macro slightly: > > +#define alternative_output(oldinstr, newinstr, feature, output)\ > + asm volatile (ALTERNATIVE(oldinstr, newinstr, feature) \ > + : output : "i" (0) : "memory") > > and that also works. The concern I have here is that the asm does use > "+m" as its output already, and the clflush() asm doesn't require the > manual clobber. Yeah, you could make the above #define alternative_output(oldinstr, newinstr, feature, output, clobber...) and use it to define clflushopt(). hpa might have a better idea, though... -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply.
[Bug 91620] Tonga warnings after mem sleep with drm-next-4.3-wip
https://bugs.freedesktop.org/show_bug.cgi?id=91620 Andy Furniss changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |WORKSFORME --- Comment #1 from Andy Furniss --- Haven't seen this for some time now. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/1ce4062c/attachment.html>
[Bug 91620] Tonga warnings after mem sleep with drm-next-4.3-wip
https://bugs.freedesktop.org/show_bug.cgi?id=91620 Andy Furniss changed: What|Removed |Added Status|RESOLVED|CLOSED -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/be0f5fa9/attachment-0001.html>
[PATCH 00/10] drm/exynos: add pm_runtime support
Hi Gustavo, Please ping~ and re-base on top of exynos-drm-next. Thanks, Inki Dae 2015ë 09ì 05ì¼ 05:15ì Gustavo Padovan ì´(ê°) ì´ ê¸: > From: Gustavo Padovan > > Hi, > > This series adds proper runtime PM suport to CRTCs and Encoders, so > now instead of relying on 'suspended' or 'enabled' flags to track when > the CRTC or Encoder is enabled we let the pm_runtime subsystem do it for us > and remove all the flags. This is a important step to the atomic > suspend/resume > support that will land in drm anytime soon. > > Please review! > > Gustavo > > Gustavo Padovan (10): >drm/exynos: do not start enabling DP at bind() phase >drm/exynos: add pm_runtime to DP >drm/exynos: add pm_runtime to HDMI >drm/exynos: add pm_runtime to Mixer >drm/exynos: remove exynos_crtc commit() callback >drm/exynos: Remove exynos_crtc commit() callback >drm/exynos: add pm_runtime to FIMD >drm/exynos: Enable DP clock directly from FIMD >drm/exynos: add pm_runtime to DECON 5433 >drm/exynos: add pm_runtime to DECON 7 > > drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 90 -- > drivers/gpu/drm/exynos/exynos7_drm_decon.c| 126 > +++-- > drivers/gpu/drm/exynos/exynos_dp_core.c | 57 ++-- > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 10 -- > drivers/gpu/drm/exynos/exynos_drm_drv.h | 7 -- > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 129 > -- > drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++ > drivers/gpu/drm/exynos/exynos_mixer.c | 125 > - > 8 files changed, 275 insertions(+), 325 deletions(-) >
[PATCH] drm/imx: hdmi: fix HDMI setup to allow modes larger than FullHD
Dave,
could you please pick this patch up as a fix for 4.3? It fixes a
regression introduced in this cycle by the stricter parameter validation
in the DW HDMI driver. Philipp is on holiday this week, so I guess he
won't be able to pick it up.
Thanks,
Lucas
Am Donnerstag, den 15.10.2015, 15:42 +0200 schrieb Lucas Stach:
> This worked before the dw-hdmi bridge code was changed to validate
> the setup data more strictly. Add back support for modes with a
> pixel clock up to 216MHz. Even higher clocks should work, but we
> are missing the required setup data for now.
>
> Also change the mode validate callbacks to disallow modes with
> higher pixelclocks, so we don't end up failing the modeset later
> on.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/imx/dw_hdmi-imx.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> index 4aad6bce6241..13596e822391 100644
> --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> @@ -48,11 +48,17 @@ static const struct dw_hdmi_mpll_config imx_mpll_cfg[] = {
> { 0x40a2, 0x000a },
> },
> }, {
> - ~0UL, {
> + 21600, {
> { 0x00a0, 0x000a },
> { 0x2001, 0x000f },
> { 0x4002, 0x000f },
> },
> + }, {
> + ~0UL, {
> + { 0x, 0x },
> + { 0x, 0x },
> + { 0x, 0x },
> + },
> }
> };
>
> @@ -82,7 +88,7 @@ static const struct dw_hdmi_curr_ctrl imx_cur_ctr[] = {
> */
> static const struct dw_hdmi_phy_config imx_phy_config[] = {
> /*pixelclk symbol term vlev */
> - { 14850, 0x800d, 0x0005, 0x01ad},
> + { 21600, 0x800d, 0x0005, 0x01ad},
> { ~0UL, 0x, 0x, 0x}
> };
>
> @@ -148,7 +154,8 @@ static enum drm_mode_status imx6q_hdmi_mode_valid(struct
> drm_connector *con,
> {
> if (mode->clock < 13500)
> return MODE_CLOCK_LOW;
> - if (mode->clock > 266000)
> + /* FIXME: Hardware is capable of 266MHz, but setup data is missing. */
> + if (mode->clock > 216000)
> return MODE_CLOCK_HIGH;
>
> return MODE_OK;
> @@ -159,7 +166,8 @@ static enum drm_mode_status imx6dl_hdmi_mode_valid(struct
> drm_connector *con,
> {
> if (mode->clock < 13500)
> return MODE_CLOCK_LOW;
> - if (mode->clock > 27)
> + /* FIXME: Hardware is capable of 270MHz, but setup data is missing. */
> + if (mode->clock > 216000)
> return MODE_CLOCK_HIGH;
>
> return MODE_OK;
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[PATCH 6/9] drm/exynos: switch to new buffer allocation
Hi,
How about combining patch 5 and 6?
Patch 5 just introduces new internal API but these API aren't used
anywhere in patch 5.
Thanks,
Inki Dae
2015ë
10ì 13ì¼ 16:00ì Joonyoung Shim ì´(ê°) ì´ ê¸:
> The buffer allocation using DMA mapping API can't support non-continuous
> buffer on non-iommu and cachable buffer, so switch to new buffer
> allocation using drm_gem_get/put_pages() and doesn't use DMA mapping API
> for mmap except allocation of physically continuous buffer on non-iommu.
>
> Signed-off-by: Joonyoung Shim
> ---
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 90
> +++--
> 1 file changed, 29 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index d982d46b04da..163d113df1ab 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -77,10 +77,7 @@ static int exynos_drm_alloc_dma(struct exynos_drm_gem
> *exynos_gem)
>
> init_dma_attrs(&exynos_gem->dma_attrs);
>
> - if (exynos_gem->flags & EXYNOS_BO_WC ||
> - !(exynos_gem->flags & EXYNOS_BO_CACHABLE))
> - dma_set_attr(DMA_ATTR_WRITE_COMBINE, &exynos_gem->dma_attrs);
> -
> + dma_set_attr(DMA_ATTR_WRITE_COMBINE, &exynos_gem->dma_attrs);
> dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &exynos_gem->dma_attrs);
>
> nr_pages = exynos_gem->size >> PAGE_SHIFT;
> @@ -128,51 +125,21 @@ static void exynos_drm_free_dma(struct exynos_drm_gem
> *exynos_gem)
> static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
> {
> struct drm_device *dev = exynos_gem->base.dev;
> - enum dma_attr attr;
> - unsigned int nr_pages;
> + int ret;
>
> if (exynos_gem->dma_addr) {
> DRM_DEBUG_KMS("already allocated.\n");
> return 0;
> }
>
> - if (!is_drm_iommu_supported(dev))
> - return exynos_drm_alloc_dma(exynos_gem);
> -
> - init_dma_attrs(&exynos_gem->dma_attrs);
> -
> - /*
> - * if EXYNOS_BO_CONTIG, fully physically contiguous memory
> - * region will be allocated else physically contiguous
> - * as possible.
> - */
> - if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
> - dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &exynos_gem->dma_attrs);
> -
> - /* if EXYNOS_BO_WC or EXYNOS_BO_NONCACHABLE, writecombine mapping */
> - if (exynos_gem->flags & EXYNOS_BO_WC ||
> - !(exynos_gem->flags & EXYNOS_BO_CACHABLE))
> - attr = DMA_ATTR_WRITE_COMBINE;
> -
> - dma_set_attr(attr, &exynos_gem->dma_attrs);
> - dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &exynos_gem->dma_attrs);
> -
> - nr_pages = exynos_gem->size >> PAGE_SHIFT;
> -
> - exynos_gem->cookie = dma_alloc_attrs(dev->dev, exynos_gem->size,
> - &exynos_gem->dma_addr, GFP_KERNEL,
> - &exynos_gem->dma_attrs);
> - if (!exynos_gem->cookie) {
> - DRM_ERROR("failed to allocate buffer.\n");
> - if (exynos_gem->pages)
> - drm_free_large(exynos_gem->pages);
> - return -ENOMEM;
> + if (!is_drm_iommu_supported(dev)) {
> + if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
> + return exynos_drm_alloc_dma(exynos_gem);
> }
>
> - exynos_gem->pages = exynos_gem->cookie;
> -
> - DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
> - (unsigned long)exynos_gem->dma_addr, exynos_gem->size);
> + ret = exynos_drm_get_pages(exynos_gem);
> + if (ret < 0)
> + return ret;
>
> return 0;
> }
> @@ -186,15 +153,12 @@ static void exynos_drm_free_buf(struct exynos_drm_gem
> *exynos_gem)
> return;
> }
>
> - if (!is_drm_iommu_supported(dev))
> - return exynos_drm_free_dma(exynos_gem);
> -
> - DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
> - (unsigned long)exynos_gem->dma_addr, exynos_gem->size);
> + if (!is_drm_iommu_supported(dev)) {
> + if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
> + return exynos_drm_free_dma(exynos_gem);
> + }
>
> - dma_free_attrs(dev->dev, exynos_gem->size, exynos_gem->cookie,
> - (dma_addr_t)exynos_gem->dma_addr,
> - &exynos_gem->dma_attrs);
> + exynos_drm_put_pages(exynos_gem);
> }
>
> static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
> @@ -400,8 +364,8 @@ void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
> drm_gem_object_unreference_unlocked(obj);
> }
>
> -static int exynos_drm_gem_mmap_buffer(struct exynos_drm_gem *exynos_gem,
> - struct vm_area_struct *vma)
> +static int exynos_drm_gem_mmap_dma(struct exynos_drm_gem *exynos_gem,
> +struct vm_area_struct
[RFC PATCH v2 4/4] drm/armada: Convert the probe function to the generic drm_of_component_probe()
The armada DRM driver keeps some old platform data compatibility in the
probe function that makes moving to the generic drm_of_component_probe()
a bit more complicated that it should. Refactor the probe function to do
the platform_data processing after the generic probe (and only if that
fails). This way future cleanup can further remove support for it.
Signed-off-by: Liviu Dudau
Cc: Russell King
---
drivers/gpu/drm/armada/armada_drv.c | 73 +++--
1 file changed, 22 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_drv.c
b/drivers/gpu/drm/armada/armada_drv.c
index 63d909e..25c42f3 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include "armada_crtc.h"
#include "armada_drm.h"
#include "armada_gem.h"
@@ -265,78 +266,48 @@ static void armada_add_endpoints(struct device *dev,
}
}
-static int armada_drm_find_components(struct device *dev,
- struct component_match **match)
-{
- struct device_node *port;
- int i;
-
- if (dev->of_node) {
- struct device_node *np = dev->of_node;
-
- for (i = 0; ; i++) {
- port = of_parse_phandle(np, "ports", i);
- if (!port)
- break;
-
- component_match_add(dev, match, compare_of, port);
- of_node_put(port);
- }
+static const struct component_master_ops armada_master_ops = {
+ .bind = armada_drm_bind,
+ .unbind = armada_drm_unbind,
+};
- if (i == 0) {
- dev_err(dev, "missing 'ports' property\n");
- return -ENODEV;
- }
+static int armada_drm_probe(struct platform_device *pdev)
+{
+ struct component_match *match = NULL;
+ int ret;
- for (i = 0; ; i++) {
- port = of_parse_phandle(np, "ports", i);
- if (!port)
- break;
+ ret = drm_of_component_probe(&pdev->dev, compare_dev_name,
+&armada_master_ops);
+ if (ret != -EINVAL)
+ return ret;
- armada_add_endpoints(dev, match, port);
- of_node_put(port);
- }
- } else if (dev->platform_data) {
- char **devices = dev->platform_data;
+ if (pdev->dev.platform_data) {
+ char **devices = pdev->dev.platform_data;
+ struct device_node *port;
struct device *d;
+ int i;
for (i = 0; devices[i]; i++)
- component_match_add(dev, match, compare_dev_name,
+ component_match_add(&pdev->dev, match, compare_dev_name,
devices[i]);
if (i == 0) {
- dev_err(dev, "missing 'ports' property\n");
+ dev_err(&pdev->dev, "missing 'ports' property\n");
return -ENODEV;
}
for (i = 0; devices[i]; i++) {
d = bus_find_device_by_name(&platform_bus_type, NULL,
- devices[i]);
+ devices[i]);
if (d && d->of_node) {
for_each_child_of_node(d->of_node, port)
- armada_add_endpoints(dev, match, port);
+ armada_add_endpoints(&pdev->dev, match,
+port);
}
put_device(d);
}
}
- return 0;
-}
-
-static const struct component_master_ops armada_master_ops = {
- .bind = armada_drm_bind,
- .unbind = armada_drm_unbind,
-};
-
-static int armada_drm_probe(struct platform_device *pdev)
-{
- struct component_match *match = NULL;
- int ret;
-
- ret = armada_drm_find_components(&pdev->dev, &match);
- if (ret < 0)
- return ret;
-
return component_master_add_with_match(&pdev->dev, &armada_master_ops,
match);
}
--
2.6.0
[RFC PATCH v2 0/4] drm: Cleanup probe function for component based masters.
Changelog: v2: Rebased the patchset on top of drm-next rather than Linus' latest -rc A few drivers in drivers/gpu/drm are component-enabled and use quite similar code sequences to probe for their encoder slaves at the remote end of the ports. Move the code into a "generic" function and remove it from the drivers. The end results is that drivers get a reference count fix (imx), more thorough error checking (imx again), setting the DMA coherent mask (armada) plus a decrease in the overall count of LoC. I'm looking for comments and testing of the patchset (only compile tested from my end as I don't have access to all the devices touched by the changes). My main interest is in finding out if -EINVAL is the correct code to return if dev->of_node == NULL (handy now, as it is different from the other possible error codes and used in armada to trigger old platform_data support. Also looking for thoughts on the correctness of the patch and if it possible to co-opt more drivers into using the function. Best regards, Liviu Liviu Dudau (4): drm: Introduce generic probe function for component based masters. drm/imx: Convert the probe function to the generic drm_of_component_probe() drm/rockchip: Convert the probe function to the generic drm_of_component_probe() drm/armada: Convert the probe function to the generic drm_of_component_probe() drivers/gpu/drm/armada/armada_drv.c | 73 +++ drivers/gpu/drm/drm_of.c| 92 + drivers/gpu/drm/imx/imx-drm-core.c | 54 + drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 86 ++- include/drm/drm_of.h| 12 5 files changed, 133 insertions(+), 184 deletions(-) -- 2.6.0
[RFC PATCH v2 2/4] drm/imx: Convert the probe function to the generic drm_of_component_probe()
The generic function is functionally equivalent to the driver's
imx_drm_platform_probe(). Use the generic function and reduce the
overall code size.
Signed-off-by: Liviu Dudau
---
drivers/gpu/drm/imx/imx-drm-core.c | 54 +-
1 file changed, 1 insertion(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c
b/drivers/gpu/drm/imx/imx-drm-core.c
index de00a6c..384d819 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -531,59 +531,7 @@ static const struct component_master_ops imx_drm_ops = {
static int imx_drm_platform_probe(struct platform_device *pdev)
{
- struct device_node *ep, *port, *remote;
- struct component_match *match = NULL;
- int ret;
- int i;
-
- /*
-* Bind the IPU display interface ports first, so that
-* imx_drm_encoder_parse_of called from encoder .bind callbacks
-* works as expected.
-*/
- for (i = 0; ; i++) {
- port = of_parse_phandle(pdev->dev.of_node, "ports", i);
- if (!port)
- break;
-
- component_match_add(&pdev->dev, &match, compare_of, port);
- }
-
- if (i == 0) {
- dev_err(&pdev->dev, "missing 'ports' property\n");
- return -ENODEV;
- }
-
- /* Then bind all encoders */
- for (i = 0; ; i++) {
- port = of_parse_phandle(pdev->dev.of_node, "ports", i);
- if (!port)
- break;
-
- for_each_child_of_node(port, ep) {
- remote = of_graph_get_remote_port_parent(ep);
- if (!remote || !of_device_is_available(remote)) {
- of_node_put(remote);
- continue;
- } else if (!of_device_is_available(remote->parent)) {
- dev_warn(&pdev->dev, "parent device of %s is
not available\n",
-remote->full_name);
- of_node_put(remote);
- continue;
- }
-
- component_match_add(&pdev->dev, &match, compare_of,
- remote);
- of_node_put(remote);
- }
- of_node_put(port);
- }
-
- ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
- if (ret)
- return ret;
-
- return component_master_add_with_match(&pdev->dev, &imx_drm_ops, match);
+ return drm_of_component_probe(&pdev->dev, compare_of, &imx_drm_ops);
}
static int imx_drm_platform_remove(struct platform_device *pdev)
--
2.6.0
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
A lot of component based DRM drivers use a variant of the same code
as the probe function. They bind the crtc ports in the first iteration
and then scan through the child nodes and bind the encoders attached
to the remote endpoints. Factor the common code into a separate
function called drm_of_component_probe() in order to increase code
reuse.
Cc: David Airlie
Signed-off-by: Liviu Dudau
---
drivers/gpu/drm/drm_of.c | 92
include/drm/drm_of.h | 12 +++
2 files changed, 104 insertions(+)
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index be38840..cf16240 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -1,3 +1,4 @@
+#include
#include
#include
#include
@@ -61,3 +62,94 @@ uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
return possible_crtcs;
}
EXPORT_SYMBOL(drm_of_find_possible_crtcs);
+
+/**
+ * drm_of_component_probe - Generic probe function for a component based master
+ * @dev: master device containing the OF node
+ * @compare_of: compare function used for matching components
+ * @master_ops: component master ops to be used
+ *
+ * Parse the platform device OF node and bind all the components associated
+ * with the master. Interface ports are added before the encoders in order to
+ * satisfy their .bind requirements
+ * See Documentation/devicetree/bindings/graph.txt for the bindings.
+ *
+ * Returns zero if successful, or one of the standard error codes if it fails.
+ */
+int drm_of_component_probe(struct device *dev,
+ int (*compare_of)(struct device *, void *),
+ const struct component_master_ops *master_ops)
+{
+ struct device_node *ep, *port, *remote;
+ struct component_match *match = NULL;
+ int i, ret;
+
+ if (!dev->of_node)
+ return -EINVAL;
+
+ /*
+* Bind the crtc's ports first, so that drm_of_find_possible_crtcs()
+* called from encoder's .bind callbacks works as expected
+*/
+ for (i = 0; ; i++) {
+ port = of_parse_phandle(dev->of_node, "ports", i);
+ if (!port)
+ break;
+
+ if (!of_device_is_available(port->parent)) {
+ of_node_put(port);
+ continue;
+ }
+
+ component_match_add(dev, &match, compare_of, port);
+ of_node_put(port);
+ }
+
+ if (i == 0) {
+ dev_err(dev, "missing 'ports' property\n");
+ return -ENODEV;
+ }
+
+ if (!match) {
+ dev_err(dev, "no available port\n");
+ return -ENODEV;
+ }
+
+ /*
+* For bound crtcs, bind the encoders attached to their remote endpoint
+*/
+ for (i = 0; ; i++) {
+ port = of_parse_phandle(dev->of_node, "ports", i);
+ if (!port)
+ break;
+
+ if (!of_device_is_available(port->parent)) {
+ of_node_put(port);
+ continue;
+ }
+
+ for_each_child_of_node(port, ep) {
+ remote = of_graph_get_remote_port_parent(ep);
+ if (!remote || !of_device_is_available(remote)) {
+ of_node_put(remote);
+ continue;
+ } else if (!of_device_is_available(remote->parent)) {
+ dev_warn(dev, "parent device of %s is not
available\n",
+remote->full_name);
+ of_node_put(remote);
+ continue;
+ }
+
+ component_match_add(dev, &match, compare_of, remote);
+ of_node_put(remote);
+ }
+ of_node_put(port);
+ }
+
+ ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+
+ return component_master_add_with_match(dev, master_ops, match);
+}
+EXPORT_SYMBOL(drm_of_component_probe);
diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
index 2441f71..7c0c05b 100644
--- a/include/drm/drm_of.h
+++ b/include/drm/drm_of.h
@@ -1,18 +1,30 @@
#ifndef __DRM_OF_H__
#define __DRM_OF_H__
+struct component_master_ops;
+struct device;
struct drm_device;
struct device_node;
#ifdef CONFIG_OF
extern uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
struct device_node *port);
+extern int drm_of_component_probe(struct device *dev,
+ int (*compare_of)(struct device *, void *),
+ const struct component_master_ops
*master_ops);
#else
static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
[RFC PATCH v2 3/4] drm/rockchip: Convert the probe function to the generic drm_of_component_probe()
Use the generic drm_of_component_probe() function to probe for components.
Signed-off-by: Liviu Dudau
---
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 86 ++---
1 file changed, 6 insertions(+), 80 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index f22e1e1..a32b596 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -45,11 +46,6 @@ int rockchip_drm_dma_attach_device(struct drm_device
*drm_dev,
struct device *dev)
{
struct dma_iommu_mapping *mapping = drm_dev->dev->archdata.mapping;
- int ret;
-
- ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
- if (ret)
- return ret;
dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
@@ -418,29 +414,6 @@ static int compare_of(struct device *dev, void *data)
return dev->of_node == np;
}
-static void rockchip_add_endpoints(struct device *dev,
- struct component_match **match,
- struct device_node *port)
-{
- struct device_node *ep, *remote;
-
- for_each_child_of_node(port, ep) {
- remote = of_graph_get_remote_port_parent(ep);
- if (!remote || !of_device_is_available(remote)) {
- of_node_put(remote);
- continue;
- } else if (!of_device_is_available(remote->parent)) {
- dev_warn(dev, "parent device of %s is not available\n",
-remote->full_name);
- of_node_put(remote);
- continue;
- }
-
- component_match_add(dev, match, compare_of, remote);
- of_node_put(remote);
- }
-}
-
static int rockchip_drm_bind(struct device *dev)
{
struct drm_device *drm;
@@ -483,61 +456,14 @@ static const struct component_master_ops rockchip_drm_ops
= {
static int rockchip_drm_platform_probe(struct platform_device *pdev)
{
- struct device *dev = &pdev->dev;
- struct component_match *match = NULL;
- struct device_node *np = dev->of_node;
- struct device_node *port;
- int i;
+ int ret = drm_of_component_probe(&pdev->dev, compare_of,
+&rockchip_drm_ops);
- if (!np)
+ /* keep compatibility with old code that was returning -ENODEV */
+ if (ret == -EINVAL)
return -ENODEV;
- /*
-* Bind the crtc ports first, so that
-* drm_of_find_possible_crtcs called from encoder .bind callbacks
-* works as expected.
-*/
- for (i = 0;; i++) {
- port = of_parse_phandle(np, "ports", i);
- if (!port)
- break;
-
- if (!of_device_is_available(port->parent)) {
- of_node_put(port);
- continue;
- }
- component_match_add(dev, &match, compare_of, port->parent);
- of_node_put(port);
- }
-
- if (i == 0) {
- dev_err(dev, "missing 'ports' property\n");
- return -ENODEV;
- }
-
- if (!match) {
- dev_err(dev, "No available vop found for display-subsystem.\n");
- return -ENODEV;
- }
- /*
-* For each bound crtc, bind the encoders attached to its
-* remote endpoint.
-*/
- for (i = 0;; i++) {
- port = of_parse_phandle(np, "ports", i);
- if (!port)
- break;
-
- if (!of_device_is_available(port->parent)) {
- of_node_put(port);
- continue;
- }
-
- rockchip_add_endpoints(dev, &match, port);
- of_node_put(port);
- }
-
- return component_master_add_with_match(dev, &rockchip_drm_ops, match);
+ return ret;
}
static int rockchip_drm_platform_remove(struct platform_device *pdev)
--
2.6.0
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 01:21:50PM +0100, Liviu Dudau wrote:
> A lot of component based DRM drivers use a variant of the same code
> as the probe function. They bind the crtc ports in the first iteration
> and then scan through the child nodes and bind the encoders attached
> to the remote endpoints. Factor the common code into a separate
> function called drm_of_component_probe() in order to increase code
> reuse.
>
> Cc: David Airlie
> Signed-off-by: Liviu Dudau
> ---
> drivers/gpu/drm/drm_of.c | 92
>
> include/drm/drm_of.h | 12 +++
> 2 files changed, 104 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index be38840..cf16240 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -1,3 +1,4 @@
> +#include
> #include
> #include
> #include
> @@ -61,3 +62,94 @@ uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> return possible_crtcs;
> }
> EXPORT_SYMBOL(drm_of_find_possible_crtcs);
> +
> +/**
> + * drm_of_component_probe - Generic probe function for a component based
> master
> + * @dev: master device containing the OF node
> + * @compare_of: compare function used for matching components
> + * @master_ops: component master ops to be used
> + *
> + * Parse the platform device OF node and bind all the components associated
> + * with the master. Interface ports are added before the encoders in order to
> + * satisfy their .bind requirements
> + * See Documentation/devicetree/bindings/graph.txt for the bindings.
> + *
> + * Returns zero if successful, or one of the standard error codes if it
> fails.
> + */
> +int drm_of_component_probe(struct device *dev,
> +int (*compare_of)(struct device *, void *),
> +const struct component_master_ops *master_ops)
> +{
...
> + ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
Please don't move this into here, it's completely inappropriate. Just
because something makes use of this does not mean they only support
32-bit DMA. Besides, this has nothing to do with whether or not it's
OF-based or not.
> +
> + return component_master_add_with_match(dev, master_ops, match);
> +}
> +EXPORT_SYMBOL(drm_of_component_probe);
> diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
> index 2441f71..7c0c05b 100644
> --- a/include/drm/drm_of.h
> +++ b/include/drm/drm_of.h
> @@ -1,18 +1,30 @@
> #ifndef __DRM_OF_H__
> #define __DRM_OF_H__
>
> +struct component_master_ops;
> +struct device;
> struct drm_device;
> struct device_node;
>
> #ifdef CONFIG_OF
> extern uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> struct device_node *port);
> +extern int drm_of_component_probe(struct device *dev,
> + int (*compare_of)(struct device *, void *),
> + const struct component_master_ops
> *master_ops);
> #else
> static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> struct device_node *port)
> {
> return 0;
> }
> +
> +static inline int drm_of_component_probe(struct device *dev,
> + int (*compare_of)(struct device *, void
> *),
> + const struct component_master_ops
> *master_ops)
Some of these lines will fail checkpatch. It would be nice to avoid
lines longer than 80 columns.
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
HDMI codec, way forward?
On 10/16/15 16:51, Arnaud Pouliquen wrote: >> After reading the ELCE Audio mini conf minutes [1] I gather that HDMI >> audio was not discussed after all. >> >> My conclusion from the Lars-Peter's latest mail [2] related to the >> subject is that the wind is currently blowing slightly in favour of my >> version of the hdmi codec [3], or at least not in favour of Arnaud's >> version [4]. > Based on previous discussions and lack of feedback from DRM community, > This make sens for me. > >> >> So how should we proceed? Are we still waiting for some feedback from >> DRM/video side? >> >> There was not too many clear suggestions to my latest patch series >> [3], so I do not see too much point in sending yet another version of >> that series. > For me, some points need to be clarified: > - Do we need the abort callback. Is there a uses cases that makes it > mandatory (some timeout mechanism already exists to stop audio...) > I am not currently using the abort_cb my self, so it can be dropped as well. Is should not be needed for codec DAI implementations, as long as the CPU-DAI is the i2s master. > - Does trigger is needed when CPU-DAI is master on bus? > Otherwise how to inform HDMI that I2S bus is no more clocked? I'm not > sure that mute is sufficient for all hardwares... For instance for my > hardware CTS parameter is hardware computed based on I2S L/R clock. > The most of the codec drivers do not implement the trigger callback as the stream start and stop timing is usually handled at the CPU-DAI. Codec is just prepared (quite often even without prepare callback) and the stream start/stop is triggered at CPU DAI end. However, I should probably still implement the trigger callback and let the video side driver decide if it needs it or not. > - IEC controls to support compress mode. This should be handled by codec > driver in I2S mode, but not in SPDIF mode. > >> >> Arnaud, if I'd try to make a patch series that would implement sti >> HDMI audio with my HDMI codec, would you be willing try that out? > hmm...more simple that i do the stuff for sti platform as some pieces of > code are missing today in kernel (Device tree part). > I will try to find time next week to make a prototype for test and give > you a feedback. > That would be even better. Just let me know I can help you in any way. > Additional point: We should also define some new helper functions: > > - For N and CTS HDMI parameters: In my RFC i have proposed helper > function, don't know if that matches with other platforms need... > Absolutely, but I don't think these helpers should have any direct association to ASoC side. So they are in a way orthogonal to the ASoC side HDMI codec implementation. But very relevant to the video side driver registering the HDMI codec. > - For IEC standard controls (could be added in core/pcm_iec958.c) > Oh, I did not even realize that there are predefined special kcontrols for handling the status bits. Adding those sounds like right thing to do. Cheers, Jyr
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 01:25:37PM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 19, 2015 at 01:21:50PM +0100, Liviu Dudau wrote:
> > A lot of component based DRM drivers use a variant of the same code
> > as the probe function. They bind the crtc ports in the first iteration
> > and then scan through the child nodes and bind the encoders attached
> > to the remote endpoints. Factor the common code into a separate
> > function called drm_of_component_probe() in order to increase code
> > reuse.
> >
> > Cc: David Airlie
> > Signed-off-by: Liviu Dudau
> > ---
> > drivers/gpu/drm/drm_of.c | 92
> >
> > include/drm/drm_of.h | 12 +++
> > 2 files changed, 104 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> > index be38840..cf16240 100644
> > --- a/drivers/gpu/drm/drm_of.c
> > +++ b/drivers/gpu/drm/drm_of.c
> > @@ -1,3 +1,4 @@
> > +#include
> > #include
> > #include
> > #include
> > @@ -61,3 +62,94 @@ uint32_t drm_of_find_possible_crtcs(struct drm_device
> > *dev,
> > return possible_crtcs;
> > }
> > EXPORT_SYMBOL(drm_of_find_possible_crtcs);
> > +
> > +/**
> > + * drm_of_component_probe - Generic probe function for a component based
> > master
> > + * @dev: master device containing the OF node
> > + * @compare_of: compare function used for matching components
> > + * @master_ops: component master ops to be used
> > + *
> > + * Parse the platform device OF node and bind all the components associated
> > + * with the master. Interface ports are added before the encoders in order
> > to
> > + * satisfy their .bind requirements
> > + * See Documentation/devicetree/bindings/graph.txt for the bindings.
> > + *
> > + * Returns zero if successful, or one of the standard error codes if it
> > fails.
> > + */
> > +int drm_of_component_probe(struct device *dev,
> > + int (*compare_of)(struct device *, void *),
> > + const struct component_master_ops *master_ops)
> > +{
> ...
> > + ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> > + if (ret)
> > + return ret;
>
> Please don't move this into here, it's completely inappropriate. Just
> because something makes use of this does not mean they only support
> 32-bit DMA. Besides, this has nothing to do with whether or not it's
> OF-based or not.
Understood. My thinking process was that component-based drivers are all
OF-enabled (how else do you make use of the framework?) and 32-bit DMA is
present in 2 out of 3 drivers that are converted, so it looks to be common
enough that adding it to armada would not hurt. It was all done in the name of
collecting common code in a single function.
> > +
> > + return component_master_add_with_match(dev, master_ops, match);
> > +}
> > +EXPORT_SYMBOL(drm_of_component_probe);
> > diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
> > index 2441f71..7c0c05b 100644
> > --- a/include/drm/drm_of.h
> > +++ b/include/drm/drm_of.h
> > @@ -1,18 +1,30 @@
> > #ifndef __DRM_OF_H__
> > #define __DRM_OF_H__
> >
> > +struct component_master_ops;
> > +struct device;
> > struct drm_device;
> > struct device_node;
> >
> > #ifdef CONFIG_OF
> > extern uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> >struct device_node *port);
> > +extern int drm_of_component_probe(struct device *dev,
> > + int (*compare_of)(struct device *, void *),
> > + const struct component_master_ops
> > *master_ops);
> > #else
> > static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> > struct device_node *port)
> > {
> > return 0;
> > }
> > +
> > +static inline int drm_of_component_probe(struct device *dev,
> > + int (*compare_of)(struct device *, void
> > *),
> > +const struct component_master_ops
> > *master_ops)
>
> Some of these lines will fail checkpatch. It would be nice to avoid
> lines longer than 80 columns.
I'm torn between following the 80 columns rule, aligning parameters to the
opening brace, keeping the return value on the same line as the function name
and having this high res monitor that can make code more readable by going to
86 columns. :)
I will update the patch to trim down the line width, thanks for reviewing it!
Best regards,
Liviu
>
> --
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ã)_/¯
[PATCH] drm/core: Fix error condition in __setplane_internal.
Just like the other checks the crtc coordinates need to use goto out.
This fixes a framebuffer leak introduced by commit 3968be946a057baa.
"drm: Make integer overflow checking cover universal cursor updates (v2)"
Cc: Matt Roper
Fixes: 3968be946a057baa
Signed-off-by: Maarten Lankhorst
---
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 7ec0247cf8db..7db9b3ed5415 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2342,7 +2342,8 @@ static int __setplane_internal(struct drm_plane *plane,
crtc_y > INT_MAX - (int32_t) crtc_h) {
DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
crtc_w, crtc_h, crtc_x, crtc_y);
- return -ERANGE;
+ ret = -ERANGE;
+ goto out;
}
[PULL] drm-intel-next
Hi Dave, drm-intel-next-2015-10-10: - dmc fixes from Animesh (not yet all) for deeper sleep states - piles of prep patches from Ville to make mmio functions type-safe - more fbc work from Paulo all over - w/a shuffling from Arun Siluvery - first part of atomic watermark updates from Matt and Ville (later parts had to be dropped again unfortunately) - lots of patches to prepare bxt dsi support ( Shashank Sharma) - userptr fixes from Chris - audio rate interface between i915/snd_hda plus kerneldoc (Libin Yang) - shrinker improvements and fixes (Chris Wilson) - lots and lots of small patches all over Final feature pull for 4.4. As usual I'll follow up with a -next-fixes pull and then hand over to Jani. Note that there's a minor conflict in g4x_get_vblank_counter between drm-next and dinq, the prototype of the function after the merge should be static u32 g4x_get_vblank_counter(struct drm_device *dev, unsigned int pipe) Otherwise nothing nefarious conflict-wise afaics. Cheers, Daniel The following changes since commit 44cc6c08da0b6c8321c6740bbb6a0c6feb45b2c2: Merge remote-tracking branch 'airlied/drm-next' into drm-intel-next (2015-09-30 08:47:41 +0200) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-next-2015-10-10 for you to fetch changes up to 80bea1897d7bc35e2b201847e12029a9d677cf12: drm/i915: Update DRIVER_DATE to 20151010 (2015-10-10 13:35:42 +0200) - dmc fixes from Animesh (not yet all) for deeper sleep states - piles of prep patches from Ville to make mmio functions type-safe - more fbc work from Paulo all over - w/a shuffling from Arun Siluvery - first part of atomic watermark updates from Matt and Ville (later parts had to be dropped again unfortunately) - lots of patches to prepare bxt dsi support ( Shashank Sharma) - userptr fixes from Chris - audio rate interface between i915/snd_hda plus kerneldoc (Libin Yang) - shrinker improvements and fixes (Chris Wilson) - lots and lots of small patches all over Akash Goel (1): drm/i915/bxt: Set time interval unit to 0.833us Alex Dai (3): drm/i915/guc: Fix a bug in GuC status check drm/i915/guc: Media domain bit needed when notify GuC rc6 state drm/i915/guc: Add host2guc notification for suspend and resume Ander Conselvan de Oliveira (1): drm/i915: Rename DP link training functions Animesh Manna (4): drm/i915/skl: Added a check for the hardware status of csr fw before loading. drm/i915/skl Remove the call for csr uninitialization from suspend path drm/i915/skl: Do not disable cdclk PLL if csr firmware is present drm/i915/skl: Block disable call for pw1 if dmc firmware is present. Arun Siluvery (15): drm/i915/gen9: Handle error returned by gen9_init_workarounds drm/i915/gen9: Merge two WA as they part of same register drm/i915/bxt: Add WaStoreMultiplePTEenable name drm/i915/skl: Remove WaDisableSDEUnitClockGating drm/i915/skl: Remove WaSetGAPSunitClckGateDisable drm/i915/skl: Remove WaDisableVFUnitClockGating drm/i915/gen8: Add gen8_init_workarounds for common WA drm/i915/gen8: Move INSTPM WA to common function drm/i915/gen8: Move WaDisableAsyncFlipPerfMode to common init fn drm/i915/gen8: Move WaDisablePartialInstShootdown to common init fn drm/i915/gen8: Move HiZ RAW stall optimization disable WA to common init fn drm/i915/gen8: Move Wa4x4STCOptimizationDisable to common init fn drm/i915/gen8: Move GEN7_GT_MODE WA to common init fn drm/i915/gen8: Move WaForceEnableNonCoherent to common init fn drm/i915/gen8: Move WaHdcDisableFetchWhenMasked to common init fn Chris Wilson (9): drm/i915: Only update the current userptr worker drm/i915: Fix userptr deadlock with aliased GTT mmappings drm/i915: Use a task to cancel the userptr on invalidate_range drm/i915: shrinker_control->nr_to_scan is now unsigned long drm/i915: Add a tracepoint for the shrinker drm/i915: During shrink_all we only need to idle the GPU drm/i915: Remove dead i915_gem_evict_everything() drm/i915: Avoid GPU stalls from kswapd drm/i915: Kill DRI1 cliprects Dan Carpenter (1): drm/i915: unlock on error in i915_ppgtt_info() Daniel Vetter (6): drm/i915: Fix kerneldoc for i915_gem_shrink_all Merge remote-tracking branch 'takashi/topic/drm-sync-audio-rate' into drm-intel-next-queued drm/i915: Resurrect golden context on gen6/7 Revert "drm/i915: Add hot_plug hook for hdmi encoder" Revert "drm/i915: Call encoder hotplug for init and resume cases" drm/i915: Update DRIVER_DATE to 20151010 Francisco Jerez (2): drm/i915: Don't warn if the workaround list is empty. drm/i915: Hook up ring workaround writes at context creation time on Gen6-7. Imre Deak (
[alsa-devel] HDMI codec, way forward?
On Sun, 18 Oct 2015 19:16:42 +0200, Russell King - ARM Linux wrote: > > On Sun, Oct 18, 2015 at 09:43:29PM +0530, Vinod Koul wrote: > > Right but can I ask why you didn't try making video as component and then > > CEC, audio and others receive the notification over this. > > Okay, I think I see what you're getting at. No, I don't want to tie > this stuff into the component helpers because that's the wrong approach. > > The component helper is purely about combining several struct devices > into one larger component for a subsystem which deals with card-level > components. It's not about cross-subsystem stuff. > > The problem with using it for cross-subsystem stuff is that it becomes > too tightly bound together: why should the graphics side get torn down > if you unload the audio or CEC driver? > > Audio and CEC are rather optional for HDMI - HDMI can work without audio > and CEC being present. However, audio can't be conveyed across the link > without the video side being configured. So, it makes sense to allow > the CEC and audio parts to be loaded separately (possibly as modules) > while having the video parts built-in to the kernel - especially if you > want to use the HDMI output as the console. > > Binding CEC and audio into the component helper alongside the video part > will mean that nothing will come up until all the components are present, > and everything will be torn down when any one of those components are > removed. Clearly, that's undesirable. Currently i915/audio component works as you described. The audio is optional and HDMI graphics works without audio, while HDMI HD-audio mandates i915 graphics. Takashi
[PATCH 0/2] Smartly allocate memory on the two BARs
Currently a single type of surface is allocated in a specific BAR. This also changes from userspace driver to the kernel one. This way it could happen that allocation are failing even if there are plenty of space in the other BAR. For instance this can happen trying to change resolution as the old and the new virtual screen is supposed to be contained in a single BAR. The change allows allocation to occur in the BAR not being the default for a surface type. The patches prove to be really stable. I tested setting quite small BARs (one or the oher) or changing default allocation BAR and continued working. Setting large resolution is working fine while without these patches fails for not so big BAR sizes. Changes from previous version: - remove RFC; - added signed-off-by; - added prefix in commit title; - rebased. Frediano Ziglio (2): drm/qxl: change the way slot is detected drm/qxl: allocate objects in both video rams drivers/gpu/drm/qxl/qxl_cmd.c| 2 +- drivers/gpu/drm/qxl/qxl_drv.h| 9 - drivers/gpu/drm/qxl/qxl_object.c | 11 +++ 3 files changed, 16 insertions(+), 6 deletions(-) -- 2.4.3
[PATCH 1/2] drm/qxl: change the way slot is detected
Instead of relaying on surface type use the actual placement.
This allow to have different placement for a single type of
surface.
Signed-off-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
drivers/gpu/drm/qxl/qxl_drv.h | 9 -
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index fdc1833..3a1b055 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -511,7 +511,7 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev,
cmd->u.surface_create.height = surf->surf.height;
cmd->u.surface_create.stride = surf->surf.stride;
if (new_mem) {
- int slot_id = surf->type == QXL_GEM_DOMAIN_VRAM ?
qdev->main_mem_slot : qdev->surfaces_mem_slot;
+ int slot_id = qxl_bo_get_slot_id(qdev, surf);
struct qxl_memslot *slot = &(qdev->mem_slots[slot_id]);
/* TODO - need to hold one of the locks to read tbo.offset */
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 01a8694..60f0062 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -365,11 +365,18 @@ qxl_fb_virtual_address(struct qxl_device *qdev, unsigned
long physical)
return 0;
}
+static inline int
+qxl_bo_get_slot_id(struct qxl_device *qdev, struct qxl_bo *bo)
+{
+ return ((bo->tbo.cur_placement & TTM_PL_MASK_MEM) == TTM_PL_FLAG_VRAM) ?
+ qdev->main_mem_slot : qdev->surfaces_mem_slot;
+}
+
static inline uint64_t
qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo,
unsigned long offset)
{
- int slot_id = bo->type == QXL_GEM_DOMAIN_VRAM ? qdev->main_mem_slot :
qdev->surfaces_mem_slot;
+ int slot_id = qxl_bo_get_slot_id(qdev, bo);
struct qxl_memslot *slot = &(qdev->mem_slots[slot_id]);
/* TODO - need to hold one of the locks to read tbo.offset */
--
2.4.3
[PATCH 2/2] drm/qxl: allocate objects in both video rams
If memory is not enough in the default BAR for a type try other BAR
this allow better memory usage and avoid memory allocation failure
if a BAR is quite small and other is quite unused.
Signed-off-by: Frediano Ziglio
---
drivers/gpu/drm/qxl/qxl_object.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index b28370e..b99395e 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -58,14 +58,17 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32
domain, bool pinned)
qbo->placement.placement = qbo->placements;
qbo->placement.busy_placement = qbo->placements;
- if (domain == QXL_GEM_DOMAIN_VRAM)
+ if (domain == QXL_GEM_DOMAIN_VRAM) {
qbo->placements[c++].flags = TTM_PL_FLAG_CACHED |
TTM_PL_FLAG_VRAM | pflag;
- if (domain == QXL_GEM_DOMAIN_SURFACE)
qbo->placements[c++].flags = TTM_PL_FLAG_CACHED |
TTM_PL_FLAG_PRIV0 | pflag;
- if (domain == QXL_GEM_DOMAIN_CPU)
+ } else if (domain == QXL_GEM_DOMAIN_SURFACE) {
+ qbo->placements[c++].flags = TTM_PL_FLAG_CACHED |
TTM_PL_FLAG_PRIV0 | pflag;
+ qbo->placements[c++].flags = TTM_PL_FLAG_CACHED |
TTM_PL_FLAG_VRAM | pflag;
+ } else if (domain == QXL_GEM_DOMAIN_CPU) {
qbo->placements[c++].flags = TTM_PL_MASK_CACHING |
TTM_PL_FLAG_SYSTEM | pflag;
- if (!c)
+ } else {
qbo->placements[c++].flags = TTM_PL_MASK_CACHING |
TTM_PL_FLAG_SYSTEM;
+ }
qbo->placement.num_placement = c;
qbo->placement.num_busy_placement = c;
for (i = 0; i < c; ++i) {
--
2.4.3
[PULL] topic/drm-misc
Hi Dave,
More drm-misc for 4.4.
- fb refcount fix in atomic fbdev
- various locking reworks to reduce drm_global_mutex and dev->struct_mutex
- rename docbook to gpu.tmpl and include vga_switcheroo stuff, plus more
vga_switcheroo (Lukas Wunner)
- viewport check fixes for atomic drivers from Ville
- DRM_DEBUG_VBL from Ville
- non-contentious header fixes from Mikko Rapeli
- small things all over
Cheers, Daniel
The following changes since commit b44f84081b8db1b5830cbd30280ba1109cc1a084:
drm: Stop using drm_vblank_count() as the hw frame counter (2015-10-07
16:13:52 +0200)
are available in the git repository at:
git://anongit.freedesktop.org/drm-intel tags/topic/drm-misc-2015-10-19
for you to fetch changes up to a0fb6ad7ae28a4dce34c010028dc070eeacae1d9:
drm/fb-helper: Fix fb refcounting in pan_display_atomic (2015-10-19 11:00:48
+0200)
Adam Richter (1):
drm: fix mutex leak in drm_dp_get_mst_branch_device
Daniel Vetter (11):
drm/i915: Mark getparam ioctl as DRM_UNLOCKED
drm: Enforce unlocked ioctl operation for kms driver ioctls
drm/: Drop DRM_UNLOCKED from modeset drivers
drm/doc: Rename docbook to gpu.tmpl
drm/gem: Drop struct_mutex requirement from drm_gem_mmap_obj
drm/gem: Check locking in drm_gem_object_unreference
drm/gem: Use container_of in drm_gem_object_free
drm/vgem: Drop vgem_drm_gem_mmap
drm/gem: Use kref_get_unless_zero for the weak mmap references
drm/fb-helper: Set plane rotation directly
drm/fb-helper: Fix fb refcounting in pan_display_atomic
Lukas Wunner (8):
vga_switcheroo: Use enum vga_switcheroo_state instead of int
vga_switcheroo: Use VGA_SWITCHEROO_UNKNOWN_ID instead of -1
vga_switcheroo: Use enum vga_switcheroo_client_id instead of int
gpu/doc: Fix up remaining occurrences of old document title
gpu/doc: Add vga_switcheroo documentation
gpu/doc: Convert to markdown harder
drm: Fix return value of drm_framebuffer_init()
ALSA: hda - Spell vga_switcheroo consistently
Mikko Rapeli (3):
savage_drm.h: include
r128_drm.h: include drm/drm.h
drm/i810_drm.h: include drm/drm.h
Rob Clark (1):
drm: misc cleanup
Ville Syrjälä (7):
drm: Don't use '\' for string literal concatenation
drm: Add DRM_DEBUG_VBL()
drm: Don't leak fb when plane crtc coodinates are bad
drm: Swap w/h when converting the mode to src coordidates for a rotated
primary plane
drm: Refactor plane src coordinate checks
drm: Check crtc viewport correctly with rotated primary plane on atomic
drivers
drm: Check plane src coordinates correctly during page flip for atomic
drivers
Documentation/DocBook/Makefile | 2 +-
Documentation/DocBook/{drm.tmpl => gpu.tmpl} | 94
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 24 +++
drivers/gpu/drm/armada/armada_drv.c | 9 +--
drivers/gpu/drm/drm_atomic_helper.c | 9 ++-
drivers/gpu/drm/drm_crtc.c | 81 +---
drivers/gpu/drm/drm_dp_mst_topology.c| 7 ++-
drivers/gpu/drm/drm_drv.c| 4 +-
drivers/gpu/drm/drm_fb_helper.c | 35 ---
drivers/gpu/drm/drm_gem.c| 47 +-
drivers/gpu/drm/drm_gem_cma_helper.c | 2 -
drivers/gpu/drm/drm_ioctl.c | 10 ++-
drivers/gpu/drm/drm_irq.c| 26
drivers/gpu/drm/drm_vma_manager.c| 40
drivers/gpu/drm/exynos/exynos_drm_drv.c | 20 +++---
drivers/gpu/drm/i915/i915_dma.c | 70 ++---
drivers/gpu/drm/msm/msm_drv.c| 14 ++---
drivers/gpu/drm/msm/msm_fbdev.c | 5 --
drivers/gpu/drm/msm/msm_gem_prime.c | 2 -
drivers/gpu/drm/nouveau/nouveau_drm.c| 24 +++
drivers/gpu/drm/omapdrm/omap_crtc.c | 3 -
drivers/gpu/drm/omapdrm/omap_drv.c | 12 ++--
drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c| 3 -
drivers/gpu/drm/qxl/qxl_ioctl.c | 14 ++---
drivers/gpu/drm/radeon/radeon_kms.c | 30 -
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 3 -
drivers/gpu/drm/tegra/drm.c | 28 -
drivers/gpu/drm/vgem/vgem_drv.c | 55 +---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 54
drivers/gpu/vga/vga_switcheroo.c | 36 ++-
include/drm/drmP.h | 11 +++-
include/drm/drm_crtc.h | 5 --
include/drm/drm_gem.h| 5 +-
include/drm/drm_vma_manager.h| 24 +++
include/linux/vga_switcheroo.h | 14 +++--
include/uapi/drm/i810_drm.h | 2 +
include/uapi/drm/r128_drm.h | 2 +
include/uapi/drm/savage_drm.h
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 02:02:58PM +0100, Liviu Dudau wrote: > On Mon, Oct 19, 2015 at 01:25:37PM +0100, Russell King - ARM Linux wrote: > > Please don't move this into here, it's completely inappropriate. Just > > because something makes use of this does not mean they only support > > 32-bit DMA. Besides, this has nothing to do with whether or not it's > > OF-based or not. > > Understood. My thinking process was that component-based drivers are all > OF-enabled (how else do you make use of the framework?) and 32-bit DMA is > present in 2 out of 3 drivers that are converted, so it looks to be common > enough that adding it to armada would not hurt. It was all done in the name of > collecting common code in a single function. Which is an utterly crap reason. It's also not appropriate. I'm really not sure why you think that moving this here would in any way be appropriate - from my point of view, the mere proposal is utterly insane. The "container" device does not do any DMA, so it's inappropriate for it to have DMA masks set or negotiated on it. So, actually, no one should be setting the DMA mask for their container device. It's wrong. What if we have a 64-bit OF based platform wanting to use the component helper, and they want to call this function? You prevent them doing so by moving this into here, because they're then forced down to 32-bit DMA. Please, get rid of it, and leave this crappiness in the respective drivers. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
HDMI codec, way forward?
On Mon, Oct 19, 2015 at 04:10:32PM +0300, Jyri Sarha wrote: > On 10/16/15 16:51, Arnaud Pouliquen wrote: > >- For IEC standard controls (could be added in core/pcm_iec958.c) > > > > Oh, I did not even realize that there are predefined special kcontrols for > handling the status bits. Adding those sounds like right thing to do. And this is an example of stuff done properly: it's a library that drivers can opt into using if they so desire. It can be used with ALSA drivers, and it can be used with ASoC drivers too. Or, if a driver wants to do something more complex, it can opt not to use it at all, but make use of everything else. It can pick-and-choose. :) -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[PATCH] drm/core: Fix error condition in __setplane_internal.
On Mon, Oct 19, 2015 at 03:14:52PM +0200, Maarten Lankhorst wrote:
> Just like the other checks the crtc coordinates need to use goto out.
>
> This fixes a framebuffer leak introduced by commit 3968be946a057baa.
> "drm: Make integer overflow checking cover universal cursor updates (v2)"
>
> Cc: Matt Roper
> Fixes: 3968be946a057baa
> Signed-off-by: Maarten Lankhorst
I already merged a patch from Ville to fix this. At least git didn't
manage to apply yours on top ;-)
-Daniel
> ---
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 7ec0247cf8db..7db9b3ed5415 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2342,7 +2342,8 @@ static int __setplane_internal(struct drm_plane *plane,
> crtc_y > INT_MAX - (int32_t) crtc_h) {
> DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n",
> crtc_w, crtc_h, crtc_x, crtc_y);
> - return -ERANGE;
> + ret = -ERANGE;
> + goto out;
> }
>
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 02:26:38PM +0100, Russell King - ARM Linux wrote: > On Mon, Oct 19, 2015 at 02:02:58PM +0100, Liviu Dudau wrote: > > On Mon, Oct 19, 2015 at 01:25:37PM +0100, Russell King - ARM Linux wrote: > > > Please don't move this into here, it's completely inappropriate. Just > > > because something makes use of this does not mean they only support > > > 32-bit DMA. Besides, this has nothing to do with whether or not it's > > > OF-based or not. > > > > Understood. My thinking process was that component-based drivers are all > > OF-enabled (how else do you make use of the framework?) and 32-bit DMA is > > present in 2 out of 3 drivers that are converted, so it looks to be common > > enough that adding it to armada would not hurt. It was all done in the name > > of > > collecting common code in a single function. > > Which is an utterly crap reason. > > It's also not appropriate. I'm really not sure why you think that moving > this here would in any way be appropriate - from my point of view, the > mere proposal is utterly insane. The proposal is to collect similar code present in DRM drivers that act as component masters in one place in order to reduce code duplication. I want to add another DRM driver that will make use of the same code and did not see any reason to copy paste one of the slightly similar versions that are now peppered in the drivers/drm drivers. Sorry for making you believe this is more than just code cleanup, but the cover letter clearly stated in the title the intent. > > The "container" device does not do any DMA, so it's inappropriate for > it to have DMA masks set or negotiated on it. So, actually, no one > should be setting the DMA mask for their container device. It's wrong. > > What if we have a 64-bit OF based platform wanting to use the component > helper, and they want to call this function? You prevent them doing so > by moving this into here, because they're then forced down to 32-bit DMA. > Please, get rid of it, and leave this crappiness in the respective > drivers. OK, will do. Best regards, Liviu > > -- > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ã)_/¯
[Bug 92524] system hang on "radeon_hwmon_get_pwm1_enable"
https://bugs.freedesktop.org/show_bug.cgi?id=92524 --- Comment #3 from Alex Deucher --- Created attachment 118978 --> https://bugs.freedesktop.org/attachment.cgi?id=118978&action=edit possible fix The attached patch should fix it. pwm control is only available with dpm, and you've disabled dpm via the kernel command line. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/75eb6ada/attachment-0001.html>
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 02:32:55PM +0100, Liviu Dudau wrote: > On Mon, Oct 19, 2015 at 02:26:38PM +0100, Russell King - ARM Linux wrote: > > On Mon, Oct 19, 2015 at 02:02:58PM +0100, Liviu Dudau wrote: > > > On Mon, Oct 19, 2015 at 01:25:37PM +0100, Russell King - ARM Linux wrote: > > > > Please don't move this into here, it's completely inappropriate. Just > > > > because something makes use of this does not mean they only support > > > > 32-bit DMA. Besides, this has nothing to do with whether or not it's > > > > OF-based or not. > > > > > > Understood. My thinking process was that component-based drivers are all > > > OF-enabled (how else do you make use of the framework?) and 32-bit DMA is > > > present in 2 out of 3 drivers that are converted, so it looks to be common > > > enough that adding it to armada would not hurt. It was all done in the > > > name of > > > collecting common code in a single function. > > > > Which is an utterly crap reason. > > > > It's also not appropriate. I'm really not sure why you think that moving > > this here would in any way be appropriate - from my point of view, the > > mere proposal is utterly insane. > > The proposal is to collect similar code present in DRM drivers that act as > component masters in one place in order to reduce code duplication. I want to > add another DRM driver that will make use of the same code and did not see > any reason to copy paste one of the slightly similar versions that are now > peppered in the drivers/drm drivers. I know why people want to set the DMA mask on the "software" component, and that's drivers/gpu/drm/drm_gem_cma_helper.c, which wants to use drm->dev as the struct device to allocate memory against. That's not really appropriate, and probably ought to get fixed. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 02:26:38PM +0100, Russell King - ARM Linux wrote: > On Mon, Oct 19, 2015 at 02:02:58PM +0100, Liviu Dudau wrote: > > On Mon, Oct 19, 2015 at 01:25:37PM +0100, Russell King - ARM Linux wrote: > > > Please don't move this into here, it's completely inappropriate. Just > > > because something makes use of this does not mean they only support > > > 32-bit DMA. Besides, this has nothing to do with whether or not it's > > > OF-based or not. > > > > Understood. My thinking process was that component-based drivers are all > > OF-enabled (how else do you make use of the framework?) and 32-bit DMA is > > present in 2 out of 3 drivers that are converted, so it looks to be common > > enough that adding it to armada would not hurt. It was all done in the name > > of > > collecting common code in a single function. > > Which is an utterly crap reason. > > It's also not appropriate. I'm really not sure why you think that moving > this here would in any way be appropriate - from my point of view, the > mere proposal is utterly insane. > > The "container" device does not do any DMA, so it's inappropriate for > it to have DMA masks set or negotiated on it. So, actually, no one > should be setting the DMA mask for their container device. It's wrong. I think (and my opinion doesn't carry as much wheight here on dri-devel than intel-gfx) the above is over the top bashing of a new contributor to drm who really seems trying to do right. I think that's unecessary, especially since you follow up with the reasonable reply below. Thanks, Daniel > What if we have a 64-bit OF based platform wanting to use the component > helper, and they want to call this function? You prevent them doing so > by moving this into here, because they're then forced down to 32-bit DMA. > Please, get rid of it, and leave this crappiness in the respective > drivers. > > -- > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 04:42:25PM +0200, Daniel Vetter wrote: > On Mon, Oct 19, 2015 at 02:26:38PM +0100, Russell King - ARM Linux wrote: > > On Mon, Oct 19, 2015 at 02:02:58PM +0100, Liviu Dudau wrote: > > > On Mon, Oct 19, 2015 at 01:25:37PM +0100, Russell King - ARM Linux wrote: > > > > Please don't move this into here, it's completely inappropriate. Just > > > > because something makes use of this does not mean they only support > > > > 32-bit DMA. Besides, this has nothing to do with whether or not it's > > > > OF-based or not. > > > > > > Understood. My thinking process was that component-based drivers are all > > > OF-enabled (how else do you make use of the framework?) and 32-bit DMA is > > > present in 2 out of 3 drivers that are converted, so it looks to be common > > > enough that adding it to armada would not hurt. It was all done in the > > > name of > > > collecting common code in a single function. > > > > Which is an utterly crap reason. > > > > It's also not appropriate. I'm really not sure why you think that moving > > this here would in any way be appropriate - from my point of view, the > > mere proposal is utterly insane. > > > > The "container" device does not do any DMA, so it's inappropriate for > > it to have DMA masks set or negotiated on it. So, actually, no one > > should be setting the DMA mask for their container device. It's wrong. > > I think (and my opinion doesn't carry as much wheight here on dri-devel > than intel-gfx) the above is over the top bashing of a new contributor to > drm who really seems trying to do right. I think that's unecessary, > especially since you follow up with the reasonable reply below. It's justified because it took _two_ messages to get the point across. The first one asking nicely didn't make the necessary impact. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On 19 October 2015 at 15:50, Russell King - ARM Linux wrote: > On Mon, Oct 19, 2015 at 04:42:25PM +0200, Daniel Vetter wrote: >> On Mon, Oct 19, 2015 at 02:26:38PM +0100, Russell King - ARM Linux wrote: >> > On Mon, Oct 19, 2015 at 02:02:58PM +0100, Liviu Dudau wrote: >> > > On Mon, Oct 19, 2015 at 01:25:37PM +0100, Russell King - ARM Linux wrote: >> > > > Please don't move this into here, it's completely inappropriate. Just >> > > > because something makes use of this does not mean they only support >> > > > 32-bit DMA. Besides, this has nothing to do with whether or not it's >> > > > OF-based or not. >> > > >> > > Understood. My thinking process was that component-based drivers are all >> > > OF-enabled (how else do you make use of the framework?) and 32-bit DMA is >> > > present in 2 out of 3 drivers that are converted, so it looks to be >> > > common >> > > enough that adding it to armada would not hurt. It was all done in the >> > > name of >> > > collecting common code in a single function. >> > >> > Which is an utterly crap reason. >> > >> > It's also not appropriate. I'm really not sure why you think that moving >> > this here would in any way be appropriate - from my point of view, the >> > mere proposal is utterly insane. >> > >> > The "container" device does not do any DMA, so it's inappropriate for >> > it to have DMA masks set or negotiated on it. So, actually, no one >> > should be setting the DMA mask for their container device. It's wrong. >> >> I think (and my opinion doesn't carry as much wheight here on dri-devel >> than intel-gfx) the above is over the top bashing of a new contributor to >> drm who really seems trying to do right. I think that's unecessary, >> especially since you follow up with the reasonable reply below. > > It's justified because it took _two_ messages to get the point across. > The first one asking nicely didn't make the necessary impact. > Unlike Daniel, I carry little to no weight here, yet bashing on people if they don't get things correct the first time is inconsiderable. We are people - we can misread/misinterpret things, have a headache and/or just a bad day. If that makes you angry/annoyed, please don't reply straight away, but give it a few hours/day for the emotions to settle. Thanks Emil
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 03:50:27PM +0100, Russell King - ARM Linux wrote: > On Mon, Oct 19, 2015 at 04:42:25PM +0200, Daniel Vetter wrote: > > On Mon, Oct 19, 2015 at 02:26:38PM +0100, Russell King - ARM Linux wrote: > > > On Mon, Oct 19, 2015 at 02:02:58PM +0100, Liviu Dudau wrote: > > > > On Mon, Oct 19, 2015 at 01:25:37PM +0100, Russell King - ARM Linux > > > > wrote: > > > > > Please don't move this into here, it's completely inappropriate. Just > > > > > because something makes use of this does not mean they only support > > > > > 32-bit DMA. Besides, this has nothing to do with whether or not it's > > > > > OF-based or not. > > > > > > > > Understood. My thinking process was that component-based drivers are all > > > > OF-enabled (how else do you make use of the framework?) and 32-bit DMA > > > > is > > > > present in 2 out of 3 drivers that are converted, so it looks to be > > > > common > > > > enough that adding it to armada would not hurt. It was all done in the > > > > name of > > > > collecting common code in a single function. > > > > > > Which is an utterly crap reason. > > > > > > It's also not appropriate. I'm really not sure why you think that moving > > > this here would in any way be appropriate - from my point of view, the > > > mere proposal is utterly insane. > > > > > > The "container" device does not do any DMA, so it's inappropriate for > > > it to have DMA masks set or negotiated on it. So, actually, no one > > > should be setting the DMA mask for their container device. It's wrong. > > > > I think (and my opinion doesn't carry as much wheight here on dri-devel > > than intel-gfx) the above is over the top bashing of a new contributor to > > drm who really seems trying to do right. I think that's unecessary, > > especially since you follow up with the reasonable reply below. > > It's justified because it took _two_ messages to get the point across. > The first one asking nicely didn't make the necessary impact. Russell, I've got your point and I have accepted it after first message. Go back to my initial reply and re-read it. It starts with "Understood. " I'm not looking for a flame war. I am more interested in knowing if you think that the armada code re-org makes sense or the use of -EINVAL to mean that dev->of_node is missing. Coming to armada changes, I would also like to know if pdev->dev.platform_data is in use at all. If so, are there any examples where that is being setup? Because I think the "missing 'ports' property" message is missleading. Best regards, Liviu > > -- > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ã)_/¯
[RFC PATCH v3 4/4] drm/armada: Convert the probe function to the generic drm_of_component_probe()
The armada DRM driver keeps some old platform data compatibility in the
probe function that makes moving to the generic drm_of_component_probe()
a bit more complicated that it should. Refactor the probe function to do
the platform_data processing after the generic probe (and only if that
fails). This way future cleanup can further remove support for it.
Signed-off-by: Liviu Dudau
Cc: Russell King
---
drivers/gpu/drm/armada/armada_drv.c | 73 +++--
1 file changed, 22 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_drv.c
b/drivers/gpu/drm/armada/armada_drv.c
index 63d909e..25c42f3 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include "armada_crtc.h"
#include "armada_drm.h"
#include "armada_gem.h"
@@ -265,78 +266,48 @@ static void armada_add_endpoints(struct device *dev,
}
}
-static int armada_drm_find_components(struct device *dev,
- struct component_match **match)
-{
- struct device_node *port;
- int i;
-
- if (dev->of_node) {
- struct device_node *np = dev->of_node;
-
- for (i = 0; ; i++) {
- port = of_parse_phandle(np, "ports", i);
- if (!port)
- break;
-
- component_match_add(dev, match, compare_of, port);
- of_node_put(port);
- }
+static const struct component_master_ops armada_master_ops = {
+ .bind = armada_drm_bind,
+ .unbind = armada_drm_unbind,
+};
- if (i == 0) {
- dev_err(dev, "missing 'ports' property\n");
- return -ENODEV;
- }
+static int armada_drm_probe(struct platform_device *pdev)
+{
+ struct component_match *match = NULL;
+ int ret;
- for (i = 0; ; i++) {
- port = of_parse_phandle(np, "ports", i);
- if (!port)
- break;
+ ret = drm_of_component_probe(&pdev->dev, compare_dev_name,
+&armada_master_ops);
+ if (ret != -EINVAL)
+ return ret;
- armada_add_endpoints(dev, match, port);
- of_node_put(port);
- }
- } else if (dev->platform_data) {
- char **devices = dev->platform_data;
+ if (pdev->dev.platform_data) {
+ char **devices = pdev->dev.platform_data;
+ struct device_node *port;
struct device *d;
+ int i;
for (i = 0; devices[i]; i++)
- component_match_add(dev, match, compare_dev_name,
+ component_match_add(&pdev->dev, match, compare_dev_name,
devices[i]);
if (i == 0) {
- dev_err(dev, "missing 'ports' property\n");
+ dev_err(&pdev->dev, "missing 'ports' property\n");
return -ENODEV;
}
for (i = 0; devices[i]; i++) {
d = bus_find_device_by_name(&platform_bus_type, NULL,
- devices[i]);
+ devices[i]);
if (d && d->of_node) {
for_each_child_of_node(d->of_node, port)
- armada_add_endpoints(dev, match, port);
+ armada_add_endpoints(&pdev->dev, match,
+port);
}
put_device(d);
}
}
- return 0;
-}
-
-static const struct component_master_ops armada_master_ops = {
- .bind = armada_drm_bind,
- .unbind = armada_drm_unbind,
-};
-
-static int armada_drm_probe(struct platform_device *pdev)
-{
- struct component_match *match = NULL;
- int ret;
-
- ret = armada_drm_find_components(&pdev->dev, &match);
- if (ret < 0)
- return ret;
-
return component_master_add_with_match(&pdev->dev, &armada_master_ops,
match);
}
--
2.6.0
[RFC PATCH v3 3/4] drm/rockchip: Convert the probe function to the generic drm_of_component_probe()
Use the generic drm_of_component_probe() function to probe for components.
Signed-off-by: Liviu Dudau
---
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 81 +++--
1 file changed, 6 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index f22e1e1..d26e0cc 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -418,29 +419,6 @@ static int compare_of(struct device *dev, void *data)
return dev->of_node == np;
}
-static void rockchip_add_endpoints(struct device *dev,
- struct component_match **match,
- struct device_node *port)
-{
- struct device_node *ep, *remote;
-
- for_each_child_of_node(port, ep) {
- remote = of_graph_get_remote_port_parent(ep);
- if (!remote || !of_device_is_available(remote)) {
- of_node_put(remote);
- continue;
- } else if (!of_device_is_available(remote->parent)) {
- dev_warn(dev, "parent device of %s is not available\n",
-remote->full_name);
- of_node_put(remote);
- continue;
- }
-
- component_match_add(dev, match, compare_of, remote);
- of_node_put(remote);
- }
-}
-
static int rockchip_drm_bind(struct device *dev)
{
struct drm_device *drm;
@@ -483,61 +461,14 @@ static const struct component_master_ops rockchip_drm_ops
= {
static int rockchip_drm_platform_probe(struct platform_device *pdev)
{
- struct device *dev = &pdev->dev;
- struct component_match *match = NULL;
- struct device_node *np = dev->of_node;
- struct device_node *port;
- int i;
-
- if (!np)
- return -ENODEV;
- /*
-* Bind the crtc ports first, so that
-* drm_of_find_possible_crtcs called from encoder .bind callbacks
-* works as expected.
-*/
- for (i = 0;; i++) {
- port = of_parse_phandle(np, "ports", i);
- if (!port)
- break;
-
- if (!of_device_is_available(port->parent)) {
- of_node_put(port);
- continue;
- }
-
- component_match_add(dev, &match, compare_of, port->parent);
- of_node_put(port);
- }
+ int ret = drm_of_component_probe(&pdev->dev, compare_of,
+&rockchip_drm_ops);
- if (i == 0) {
- dev_err(dev, "missing 'ports' property\n");
+ /* keep compatibility with old code that was returning -ENODEV */
+ if (ret == -EINVAL)
return -ENODEV;
- }
- if (!match) {
- dev_err(dev, "No available vop found for display-subsystem.\n");
- return -ENODEV;
- }
- /*
-* For each bound crtc, bind the encoders attached to its
-* remote endpoint.
-*/
- for (i = 0;; i++) {
- port = of_parse_phandle(np, "ports", i);
- if (!port)
- break;
-
- if (!of_device_is_available(port->parent)) {
- of_node_put(port);
- continue;
- }
-
- rockchip_add_endpoints(dev, &match, port);
- of_node_put(port);
- }
-
- return component_master_add_with_match(dev, &rockchip_drm_ops, match);
+ return ret;
}
static int rockchip_drm_platform_remove(struct platform_device *pdev)
--
2.6.0
[RFC PATCH v3 1/4] drm: Introduce generic probe function for component based masters.
A lot of component based DRM drivers use a variant of the same code
as the probe function. They bind the crtc ports in the first iteration
and then scan through the child nodes and bind the encoders attached
to the remote endpoints. Factor the common code into a separate
function called drm_of_component_probe() in order to increase code
reuse.
Cc: David Airlie
Signed-off-by: Liviu Dudau
---
drivers/gpu/drm/drm_of.c | 88
include/drm/drm_of.h | 13 +++
2 files changed, 101 insertions(+)
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index be38840..493c05c 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -1,3 +1,4 @@
+#include
#include
#include
#include
@@ -61,3 +62,90 @@ uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
return possible_crtcs;
}
EXPORT_SYMBOL(drm_of_find_possible_crtcs);
+
+/**
+ * drm_of_component_probe - Generic probe function for a component based master
+ * @dev: master device containing the OF node
+ * @compare_of: compare function used for matching components
+ * @master_ops: component master ops to be used
+ *
+ * Parse the platform device OF node and bind all the components associated
+ * with the master. Interface ports are added before the encoders in order to
+ * satisfy their .bind requirements
+ * See Documentation/devicetree/bindings/graph.txt for the bindings.
+ *
+ * Returns zero if successful, or one of the standard error codes if it fails.
+ */
+int drm_of_component_probe(struct device *dev,
+ int (*compare_of)(struct device *, void *),
+ const struct component_master_ops *m_ops)
+{
+ struct device_node *ep, *port, *remote;
+ struct component_match *match = NULL;
+ int i;
+
+ if (!dev->of_node)
+ return -EINVAL;
+
+ /*
+* Bind the crtc's ports first, so that drm_of_find_possible_crtcs()
+* called from encoder's .bind callbacks works as expected
+*/
+ for (i = 0; ; i++) {
+ port = of_parse_phandle(dev->of_node, "ports", i);
+ if (!port)
+ break;
+
+ if (!of_device_is_available(port->parent)) {
+ of_node_put(port);
+ continue;
+ }
+
+ component_match_add(dev, &match, compare_of, port);
+ of_node_put(port);
+ }
+
+ if (i == 0) {
+ dev_err(dev, "missing 'ports' property\n");
+ return -ENODEV;
+ }
+
+ if (!match) {
+ dev_err(dev, "no available port\n");
+ return -ENODEV;
+ }
+
+ /*
+* For bound crtcs, bind the encoders attached to their remote endpoint
+*/
+ for (i = 0; ; i++) {
+ port = of_parse_phandle(dev->of_node, "ports", i);
+ if (!port)
+ break;
+
+ if (!of_device_is_available(port->parent)) {
+ of_node_put(port);
+ continue;
+ }
+
+ for_each_child_of_node(port, ep) {
+ remote = of_graph_get_remote_port_parent(ep);
+ if (!remote || !of_device_is_available(remote)) {
+ of_node_put(remote);
+ continue;
+ } else if (!of_device_is_available(remote->parent)) {
+ dev_warn(dev, "parent device of %s is not
available\n",
+remote->full_name);
+ of_node_put(remote);
+ continue;
+ }
+
+ component_match_add(dev, &match, compare_of, remote);
+ of_node_put(remote);
+ }
+ of_node_put(port);
+ }
+
+ return component_master_add_with_match(dev, m_ops, match);
+}
+EXPORT_SYMBOL(drm_of_component_probe);
diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
index 2441f71..8544665 100644
--- a/include/drm/drm_of.h
+++ b/include/drm/drm_of.h
@@ -1,18 +1,31 @@
#ifndef __DRM_OF_H__
#define __DRM_OF_H__
+struct component_master_ops;
+struct device;
struct drm_device;
struct device_node;
#ifdef CONFIG_OF
extern uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
struct device_node *port);
+extern int drm_of_component_probe(struct device *dev,
+ int (*compare_of)(struct device *, void *),
+ const struct component_master_ops *m_ops);
#else
static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
struct device_node *port)
{
return 0;
}
+
+static inline int
+drm_of_component_probe(struct device *dev,
+
[RFC PATCH v3 0/4] drm: Cleanup probe function for component based masters.
Changelog: v3: Removed the call to dma_set_coherent_mask() from the generic drm_of_component_probe(). Also changes to shorten lines over 80 chars long. v2: Rebased the patchset on top of drm-next rather than Linus' latest -rc A few drivers in drivers/gpu/drm are component-enabled and use quite similar code sequences to probe for their encoder slaves at the remote end of the ports. Move the code into a "generic" function and remove it from the drivers. The end results is that drivers get a reference count fix (imx), more thorough error checking (imx again) plus a decrease in the overall count of LoC. I'm looking for comments and testing of the patchset (only compile tested from my end as I don't have access to all the devices touched by the changes). My main interest is in finding out if -EINVAL is the correct code to return if dev->of_node == NULL (handy now, as it is different from the other possible error codes and used in armada to trigger old platform_data support. Also looking for thoughts on the correctness of the patch and if it possible to co-opt more drivers into using the function. Best regards, Liviu Liviu Dudau (4): drm: Introduce generic probe function for component based masters. drm/imx: Convert the probe function to the generic drm_of_component_probe() drm/rockchip: Convert the probe function to the generic drm_of_component_probe() drm/armada: Convert the probe function to the generic drm_of_component_probe() drivers/gpu/drm/armada/armada_drv.c | 73 drivers/gpu/drm/drm_of.c| 88 + drivers/gpu/drm/imx/imx-drm-core.c | 55 ++ drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 81 ++ include/drm/drm_of.h| 13 + 5 files changed, 133 insertions(+), 177 deletions(-) -- 2.6.0
[RFC PATCH v3 2/4] drm/imx: Convert the probe function to the generic drm_of_component_probe()
The generic function is functionally equivalent to the driver's
imx_drm_platform_probe(). Use the generic function and reduce the
overall code size.
Signed-off-by: Liviu Dudau
---
drivers/gpu/drm/imx/imx-drm-core.c | 55 +++---
1 file changed, 4 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c
b/drivers/gpu/drm/imx/imx-drm-core.c
index de00a6c..64f16ea 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -531,59 +531,12 @@ static const struct component_master_ops imx_drm_ops = {
static int imx_drm_platform_probe(struct platform_device *pdev)
{
- struct device_node *ep, *port, *remote;
- struct component_match *match = NULL;
- int ret;
- int i;
-
- /*
-* Bind the IPU display interface ports first, so that
-* imx_drm_encoder_parse_of called from encoder .bind callbacks
-* works as expected.
-*/
- for (i = 0; ; i++) {
- port = of_parse_phandle(pdev->dev.of_node, "ports", i);
- if (!port)
- break;
-
- component_match_add(&pdev->dev, &match, compare_of, port);
- }
+ int ret = drm_of_component_probe(&pdev->dev, compare_of, &imx_drm_ops);
- if (i == 0) {
- dev_err(&pdev->dev, "missing 'ports' property\n");
- return -ENODEV;
- }
+ if (!ret)
+ ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
- /* Then bind all encoders */
- for (i = 0; ; i++) {
- port = of_parse_phandle(pdev->dev.of_node, "ports", i);
- if (!port)
- break;
-
- for_each_child_of_node(port, ep) {
- remote = of_graph_get_remote_port_parent(ep);
- if (!remote || !of_device_is_available(remote)) {
- of_node_put(remote);
- continue;
- } else if (!of_device_is_available(remote->parent)) {
- dev_warn(&pdev->dev, "parent device of %s is
not available\n",
-remote->full_name);
- of_node_put(remote);
- continue;
- }
-
- component_match_add(&pdev->dev, &match, compare_of,
- remote);
- of_node_put(remote);
- }
- of_node_put(port);
- }
-
- ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
- if (ret)
- return ret;
-
- return component_master_add_with_match(&pdev->dev, &imx_drm_ops, match);
+ return ret;
}
static int imx_drm_platform_remove(struct platform_device *pdev)
--
2.6.0
[PATCH 0/8] Add ASoC support for AMD APUs [v4]
On Thu, Oct 8, 2015 at 12:12 PM, Alex Deucher wrote: > This patch set implements support for i2s audio and new AMD GPUs. > The i2s codec is fed by a DMA engine on the GPU. To handle this > we create mfd cells which we hang the i2s codec and DMA engine on. > Because of this, this patch set covers two subsystems: drm and alsa. > The drm patches add support for the ACP hw block which provides the > DMA engine for the i2s codec. The alsa patches add the ASoC driver > for the i2s codec. Since the alsa changes depend on the drm changes > in this patch set as well as some other drm changes queued for 4.3, > I'd like to take the alsa patches in via the drm tree. > Ping? Anyone had a chance to look over this latest revision? I think we've addressed all the outstanding comments. Thanks, Alex > V2 changes: > - Use the MFD subsystem rather than adding our own bus > - Squash all sub-feature patches together > - fix comments mentioned in previous review > > V3 changes: > - Update the designware driver to handle slave mode, amd specific > features > - Use the designware driver directly for i2s > - Move the DMA handling from the GPU driver into the AMD ASoC > driver > - Change the license on the ASoC driver to GPL > > v4 changes: > - patch "ASoC : dwc : support dw i2s in slave mode" accepted > - Add a _dai_fmt() operation that checks to make sure that the mode > we're setting corresponds to what we read back from the hardware. > - Split specific quirks into separate patches > - Set the specific quirks that apply to AMD chips in the acp driver > > Patch 7 adds the register headers for the ACP block which is a > pretty big patch so I've excluded it from email. The entire patch > set can be viewed here: > http://cgit.freedesktop.org/~agd5f/linux/log/?h=acp-upstream5 > > Thanks, > > Alex > > Maruthi Bayyavarapu (1): > drm/amd: add ACP driver support > > Maruthi Srinivas Bayyavarapu (7): > ASoC : dwc : add check for master/slave format > ASoC : dwc : add different playback/capture base > ASoC : dwc : add quirk for dwc controller instances > ASoC : dwc : add quirk for different register offset > ASoC : dwc : reconfigure dwc in 'resume' from 'suspend' > ASoC : AMD : add ACP 2.2 register headers > ASoC: AMD: add AMD ASoC ACP-I2S driver > > drivers/gpu/drm/Kconfig |2 + > drivers/gpu/drm/amd/acp/Kconfig | 10 + > drivers/gpu/drm/amd/acp/Makefile |9 + > drivers/gpu/drm/amd/acp/acp_hw.c | 127 ++ > drivers/gpu/drm/amd/acp/include/acp_gfx_if.h | 49 + > drivers/gpu/drm/amd/amdgpu/Makefile | 13 +- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 12 + > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 275 +++ > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.h | 41 + > drivers/gpu/drm/amd/amdgpu/vi.c | 12 + > drivers/gpu/drm/amd/include/amd_shared.h |1 + > include/linux/mfd/amd_acp.h | 43 + > include/sound/designware_i2s.h |6 + > sound/soc/Kconfig|1 + > sound/soc/Makefile |1 + > sound/soc/amd/Kconfig|4 + > sound/soc/amd/Makefile |3 + > sound/soc/amd/acp-pcm-dma.c | 518 ++ > sound/soc/amd/acp.c | 736 + > sound/soc/amd/acp.h | 147 ++ > sound/soc/amd/include/acp_2_2_d.h| 609 +++ > sound/soc/amd/include/acp_2_2_enum.h | 1068 > sound/soc/amd/include/acp_2_2_sh_mask.h | 2292 > ++ > sound/soc/dwc/designware_i2s.c | 206 ++- > 24 files changed, 6121 insertions(+), 64 deletions(-) > create mode 100644 drivers/gpu/drm/amd/acp/Kconfig > create mode 100644 drivers/gpu/drm/amd/acp/Makefile > create mode 100644 drivers/gpu/drm/amd/acp/acp_hw.c > create mode 100644 drivers/gpu/drm/amd/acp/include/acp_gfx_if.h > create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.h > create mode 100644 include/linux/mfd/amd_acp.h > create mode 100644 sound/soc/amd/Kconfig > create mode 100644 sound/soc/amd/Makefile > create mode 100644 sound/soc/amd/acp-pcm-dma.c > create mode 100644 sound/soc/amd/acp.c > create mode 100644 sound/soc/amd/acp.h > create mode 100644 sound/soc/amd/include/acp_2_2_d.h > create mode 100644 sound/soc/amd/include/acp_2_2_enum.h > create mode 100644 sound/soc/amd/include/acp_2_2_sh_mask.h > > -- > 1.8.3.1 >
[RFC PATCH v3 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 04:07:47PM +0100, Liviu Dudau wrote: > A lot of component based DRM drivers use a variant of the same code > as the probe function. They bind the crtc ports in the first iteration > and then scan through the child nodes and bind the encoders attached > to the remote endpoints. Factor the common code into a separate > function called drm_of_component_probe() in order to increase code > reuse. Thanks, this now looks perfect. Acked-by: Russell King -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[RFC PATCH v3 2/4] drm/imx: Convert the probe function to the generic drm_of_component_probe()
On Mon, Oct 19, 2015 at 04:07:48PM +0100, Liviu Dudau wrote: > The generic function is functionally equivalent to the driver's > imx_drm_platform_probe(). Use the generic function and reduce the > overall code size. Looks fine, thanks. Acked-by: Russell King -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[RFC PATCH v3 4/4] drm/armada: Convert the probe function to the generic drm_of_component_probe()
On Mon, Oct 19, 2015 at 04:07:50PM +0100, Liviu Dudau wrote: > The armada DRM driver keeps some old platform data compatibility in the > probe function that makes moving to the generic drm_of_component_probe() > a bit more complicated that it should. Refactor the probe function to do > the platform_data processing after the generic probe (and only if that > fails). This way future cleanup can further remove support for it. I'm mainly using the old code, even with DT based platforms, because it needs a block of reserved memory which I haven't got around to converting to DT stuff (each time I've looked into doing that, bits for dealing with reserved memory nodes were missing.) Acked-by: Russell King -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.
On Mon, Oct 19, 2015 at 04:04:27PM +0100, Emil Velikov wrote: > Unlike Daniel, I carry little to no weight here, yet bashing on people > if they don't get things correct the first time is inconsiderable. > We are people - we can misread/misinterpret things, have a headache > and/or just a bad day. If that makes you angry/annoyed, please don't > reply straight away, but give it a few hours/day for the emotions to > settle. Sorry, I can't do that. I get far too much email. If I leave something, it gets buried, and I'll never reply. Then people get annoyed that I don't reply to emails. There's no way to win here, sorry. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[RFC PATCH v3 4/4] drm/armada: Convert the probe function to the generic drm_of_component_probe()
On Mon, Oct 19, 2015 at 04:17:14PM +0100, Russell King - ARM Linux wrote: > On Mon, Oct 19, 2015 at 04:07:50PM +0100, Liviu Dudau wrote: > > The armada DRM driver keeps some old platform data compatibility in the > > probe function that makes moving to the generic drm_of_component_probe() > > a bit more complicated that it should. Refactor the probe function to do > > the platform_data processing after the generic probe (and only if that > > fails). This way future cleanup can further remove support for it. > > I'm mainly using the old code, even with DT based platforms, because it > needs a block of reserved memory which I haven't got around to converting > to DT stuff (each time I've looked into doing that, bits for dealing with > reserved memory nodes were missing.) Thanks for explaining this! > > Acked-by: Russell King And thanks for all the ACKs! Best regards, Liviu > > -- > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ã)_/¯
[Bug 92524] system hang on "radeon_hwmon_get_pwm1_enable"
https://bugs.freedesktop.org/show_bug.cgi?id=92524 --- Comment #4 from Thomas DEBESSE --- > The attached patch should fix it. pwm control is only available with dpm, and you've disabled dpm via the kernel command line. Oh yes, the last grub update (fortuitously made the morning before I change my GPU for the one described above) introduced a "radeon.dpm=0" default kernel command line option, probably to workaround that other bug https://bugzilla.kernel.org/show_bug.cgi?id=103271 Thank you for your fast answer, I will try both (the patch and the kernel command line option). On which source tree this patch must be applied? -- Thomas DEBESSE -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/2ff14614/attachment.html>
[Bug 92524] system hang on "radeon_hwmon_get_pwm1_enable"
https://bugs.freedesktop.org/show_bug.cgi?id=92524 --- Comment #5 from Alex Deucher --- (In reply to Thomas DEBESSE from comment #4) > > On which source tree this patch must be applied? It was against my drm-next tree, but should apply to any recent kernel. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/9e75e6b5/attachment.html>
[GIT PULL] On-demand device probing
On 19 October 2015 at 16:30, Russell King - ARM Linux wrote: > On Mon, Oct 19, 2015 at 04:10:56PM +0200, Tomeu Vizoso wrote: >> On 19 October 2015 at 15:18, Russell King - ARM Linux >> wrote: >> > On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote: >> >> ... If a device is available and has >> >> a compatible driver, but it cannot be probed because a dependency >> >> isn't going to be available, that's an error and is going to cause >> >> real-world problems unless the device is redundant. Currently we say >> >> nothing because with deferred probe the probe callbacks are also part >> >> of the mechanism that determines the dependency order. >> > >> > So what if device X depends on device Y, and we have a driver for >> > device Y built-in to the kernel, but the driver for device X is a >> > module? >> > >> > I don't see this being solvable in the way you describe above - it's >> > going to identify X as being unable to be satisfied, and report it as >> > an error - but it's not an error at all. >> >> It's going to probe Y at late_initcall, then probe X when its driver >> is registered. No deferred probes nor messages about it. >> >> But if you meant to write the opposite case (X built-in and Y in a >> module), then I have to ask you in what situation that would make >> sense. > > I did mean the opposite way around. It may not make sense if you're > targetting a single platform, but it may make sense in a single zImage > kernel. > > Consider something like a single zImage kernel that is built with > everything built-in to be able to boot and mount rootfs without > initramfs support on both platform A and platform B. Both platforms > share some hardware (eg, an I2C GPIO expander) which is built as a > module. It is a resource provider. Platform B contains a driver > which is required to boot on platform A, but not platform B (so the > kernel has to have that driver built-in.) On platform B, there is > a dependency to the I2C GPIO expander device. I see, in this situation the person trying to find out why some device hadn't probed would enable debug logging of failed probes and would see one spurious message if there was a deferred probe because of the module. >> >> Having a specific switch for enabling deferred probe logging sounds >> >> good, but there's going to be hundreds of spurious messages about >> >> deferred probes that were just deferrals and only one of them is going >> >> to be the actual error in which a device failed to find a dependency. >> > >> > Why would there be? Sounds like something's very wrong there. >> >> Sorry about that, I have checked that only now and I "only" get 39 >> deferred probe messages on exynos5250-snow. > > I typically see one or two, maybe five maximum on the platforms I have > here, but normally zero. Hmm, I have given a look at our lava farm and have seen 2 dozens as common (with multi_v7). >> > So, really, after boot and all appropriate modules have been loaded, >> > you should end up with no deferred probes. Are you saying that you >> > still have "hundreds" at that point? If you do, that sounds like >> > there's something very wrong. >> >> I was talking about messages if we log each -EPROBE_DEFER, not devices >> that remain to be probed. The point being that right now we don't have >> a way to know if we are deferring because the dependency will be >> around later, or if we have a problem and the dependency isn't going >> to be there at all. > > What's the difference between a dependency which isn't around because > the driver is not built into the kernel but is available as a module, > and a dependency that isn't around because the module hasn't been > loaded yet? > > How do you distinguish between those two scenarios? In the former > scenario, the device will eventually come up when udev loads the > module. In the latter case, it's a persistent failing case. Agreed, but it's something that doesn't happen often and that's why such messages would be at the debug level instead of being warns or errors. >> Agreed, with the note from above on why it would be better to only >> print such a message only when the -EPROBE_DEFER is likely to be a >> problem. > > ... and my argument is that there's _no way_ to know for certain which > deferred probes will be a problem, and which won't. The only way to > definitely know that is if you disable kernel modules, and require > all drivers to be built into the kernel. > > What you can do is print those devices which have failed to probe at > late_initcall() time - possibly augmenting that with reports from > subsystems showing what resources are not available, but that's only > a guide, because of the "it might or might not be in a kernel module" > problem. Well, adding those reports would give you a changelog similar to the one in this series... Thanks, Tomeu > -- > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. > -- > To unsubscribe from this li
[PATCH] drm/mgag200: don't pass NULL file_priv to drm_gem_object_lookup()
Commit bf89209a6d ("drm/mga200g: Hold a proper reference for
cursor_set") clearly didn't take the call site in
drm_fb_helper.c:restore_fbdev_mode() into account, which passes NULL
for file_priv and hence causes drm_gem_object_lookup() to fault. Move
the lookup back to before "obj" is actually needed, adjusting error
paths suitably once again.
Signed-off-by: Jan Beulich
Cc: Daniel Vetter
Cc: Thierry Reding
---
drivers/gpu/drm/mgag200/mgag200_cursor.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
--- 4.3-rc6/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ 4.3-rc6-mgag200-cursor-set/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -40,7 +40,7 @@ int mga_crtc_cursor_set(struct drm_crtc
struct mgag200_bo *pixels_2 = mdev->cursor.pixels_2;
struct mgag200_bo *pixels_current = mdev->cursor.pixels_current;
struct mgag200_bo *pixels_prev = mdev->cursor.pixels_prev;
- struct drm_gem_object *obj;
+ struct drm_gem_object *obj = NULL;
struct mgag200_bo *bo = NULL;
int ret = 0;
unsigned int i, row, col;
@@ -70,15 +70,11 @@ int mga_crtc_cursor_set(struct drm_crtc
BUG_ON(pixels_2 != pixels_current && pixels_2 != pixels_prev);
BUG_ON(pixels_current == pixels_prev);
- obj = drm_gem_object_lookup(dev, file_priv, handle);
- if (!obj)
- return -ENOENT;
-
ret = mgag200_bo_reserve(pixels_1, true);
if (ret) {
WREG8(MGA_CURPOSXL, 0);
WREG8(MGA_CURPOSXH, 0);
- goto out_unref;
+ return ret;
}
ret = mgag200_bo_reserve(pixels_2, true);
if (ret) {
@@ -110,6 +106,12 @@ int mga_crtc_cursor_set(struct drm_crtc
}
}
+ obj = drm_gem_object_lookup(dev, file_priv, handle);
+ if (!obj) {
+ ret = -ENOENT;
+ goto out1;
+ }
+
bo = gem_to_mga_bo(obj);
ret = mgag200_bo_reserve(bo, true);
if (ret) {
@@ -243,13 +245,13 @@ int mga_crtc_cursor_set(struct drm_crtc
out2:
mgag200_bo_unreserve(bo);
out1:
+ if (obj)
+ drm_gem_object_unreference_unlocked(obj);
if (ret)
mga_hide_cursor(mdev);
mgag200_bo_unreserve(pixels_1);
out_unreserve1:
mgag200_bo_unreserve(pixels_2);
-out_unref:
- drm_gem_object_unreference_unlocked(obj);
return ret;
}
drm/mgag200: don't use uninitialized variables in mga_g200se_set_plls()
I can only guess that instead of testm/testn (which are either
uninitialized or have pre-determined values at the end of the preceding
loops) n and m were meant to be used by commit e829d7ef9f
("drm/mgag200: Add support for a new rev of G200e"). In any event the
compiler is right in warning that testm/testn are possibly uninitalized
at this point, i.e. some change is needed no matter what.
Signed-off-by: Jan Beulich
Cc: Mathieu Larouche
---
drivers/gpu/drm/mgag200/mgag200_mode.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- 4.3-rc6/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ 4.3-rc6-mgag200-uninit/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -194,7 +194,7 @@ static int mga_g200se_set_plls(struct mg
}
}
- fvv = pllreffreq * testn / testm;
+ fvv = pllreffreq * n / m;
fvv = (fvv - 80) / 5;
if (fvv > 15)
[GIT PULL] On-demand device probing
On 18 October 2015 at 21:53, Mark Brown wrote: > On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote: >> On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote: >> > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote: > >> > > I can't see adding calls like this all over the tree just to solve a >> > > bus-specific problem, you are adding of_* calls where they aren't >> > > needed, or wanted, at all. > >> > This isn't bus specific, I'm not sure what makes you say that? > >> You are making it bus-specific by putting these calls all over the tree >> in different bus subsystems semi-randomly for all I can determine. > > Do you mean firmware rather than bus here? I think that's the confusion > I have... Hi all, hope you don't mind I summarize the points taken instead of replying to the individual emails. I tried to address all the concerns that have been raised again in the cover letter, but I guess I did a bad job at explaining myself, so here's another (more in-depth) go at it. 1) About the sprinkling of calls, everybody agreed it's a bad smell from the start, but the intention is to modify the behaviour of the already-DT-specific part of each subsystem without duplicating code. A way to avoid the sprinkling would be to move the storage and lookup of resources to the core (using classes and their list of devices to replace the likes of __of_usb_find_phy). I also like Mark's idea of calling of_device_probe from of_parse_phandle, which would be much less invasive but I'm not sure if it would be right to call that function in all the current cases in which of_parse_phandle is called. 2) About the goal of the series, what matters to my employer is that once a device defers its probe it's only going to be reprobed in late_initcall, after all the devices have been tentatively probed once. In the practice this means that devices get probed in a dependency order in which first go devices without dependencies then go up the tree until the leave devices (which tend to be the ones with effects visible to the user). This series changes to another dependency order in which when a leaf node gets probed, it recursively "pulls" its dependencies. This way we stop massively delaying the probing of the display devices and vendors can stop carrying sizeable hacks in their trees which just further reduce the incentive to upstream. The above is what funds this work, but in my personal opinion the biggest advantage of this work is that it makes development on embedded platforms more efficient because right now we don't have a way of telling if a device deferred its probe because of an ordering issue, or because there's a problem. If a device is available and has a compatible driver, but it cannot be probed because a dependency isn't going to be available, that's an error and is going to cause real-world problems unless the device is redundant. Currently we say nothing because with deferred probe the probe callbacks are also part of the mechanism that determines the dependency order. I have wasted countless hours hunting for the reason why a device didn't probe and I have heard the same several times from others. Having a specific switch for enabling deferred probe logging sounds good, but there's going to be hundreds of spurious messages about deferred probes that were just deferrals and only one of them is going to be the actual error in which a device failed to find a dependency. 3) Regarding total boot time, I don't expect this series to make much of a difference because though we would save a lot of matching and querying for resources, that's little time compared with how long we wait for hardware to react during probing. Async probing is more likely to help with drivers that take a long time to probe. 4) About the breakage we have seen, that's not caused so far by probing devices on-demand but by delaying probes until all built-in drivers have been registered. The latter isn't strictly needed for on-demand probing but without it most of the benefits are lost because probes of dependencies are going to be deferred because the drivers aren't there yet. We could avoid that by registering drivers also on-demand but we would need to make the matching information available beforehand, which is a massive change in itself. This should speed up boot some, and also cause leaf devices to be up earlier. One more thing about the breakage we have seen so far is that it's generally caused by implicit dependencies and hunting those is probably the second biggest timesink of the linux embedded developer after failed probes. We depend on hacks such as link order, node order in the DT, initcall gerrymandering and a blind hope in things that started some time ago to have finished by now. And those implicit dependencies are often left completely undocumented. This is really fragile and breaks often when changing something unrelated such as when adding another revision of a board or soc and a dependen
[GIT PULL] On-demand device probing
On Mon, Oct 19, 2015 at 4:44 AM, David Woodhouse wrote: > On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote: >> On Sun, Oct 18, 2015 at 12:37:57PM -0700, Greg Kroah-Hartman wrote: >> > On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote: >> > > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote: >> >> > > > I can't see adding calls like this all over the tree just to solve a >> > > > bus-specific problem, you are adding of_* calls where they aren't >> > > > needed, or wanted, at all. >> >> > > This isn't bus specific, I'm not sure what makes you say that? >> >> > You are making it bus-specific by putting these calls all over the tree >> > in different bus subsystems semi-randomly for all I can determine. >> >> Do you mean firmware rather than bus here? I think that's the confusion >> I have... > > Certainly, if it literally is adding of_* calls then that would seem to > be gratuitously firmware-specific. Nothing should be using those these > days; any new code should be using the generic device property APIs > (except in special cases). See version 2 of the series[1] which did that. It became obvious that was pointless because the call paths ended up looking like this: Generic subsystem code -> DT look-up code -> fwnode_probe_device -> of_probe_device Rob [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/361137.html
[GIT PULL] On-demand device probing
On Mon, 2015-10-19 at 07:35 -0500, Rob Herring wrote: > > > Certainly, if it literally is adding of_* calls then that would seem to > > be gratuitously firmware-specific. Nothing should be using those these > > days; any new code should be using the generic device property APIs > > (except in special cases). > > See version 2 of the series[1] which did that. It became obvious that > was pointless because the call paths ended up looking like this: > > Generic subsystem code -> DT look-up code -> fwnode_probe_device -> > of_probe_device You link to a thread which says that "AT LEAST CURRENTLY, the calling locations [the 'DT look-up code' you mention above] are DT specific functions anyway. But the point I'm making is that we are working towards *fixing* that, and *not* using DT-specific code in places where we should be using the generic APIs. Sure, Russell is probably right that there are some places where the generic APIs need fixing because they don't quite cover all use cases yet. And Mark is (unfortunately) right that some people are inventing new bindings *purely* for ACPI which are different to the DT bindings for the same device. But still, in those cases you'll theoretically be able to see the *same* device represented under ACPI with *either* its new ACPI HID and the ACPI-specific bindings, *or* as a PRP0001 with the DT bindings. And this was always possible even with just DT â you could have two incompatible bindings for the *same* hardware, with different drivers. It was just a bad thing. And still is when one is ACPI and one is DT, in my opinion. None of that really negates that fact that we are *working* on cleaning these code paths up to be firmware-agnostic, and the fact that we haven't got to this one *yet* isn't necessarily a good reason to make it *worse* by adding new firmware-specificity to it. -- dwmw2 -- next part -- A non-text attachment was scrubbed... Name: smime.p7s Type: application/x-pkcs7-signature Size: 5691 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/98eff440/attachment-0001.bin>
[GIT PULL] On-demand device probing
On 19 October 2015 at 15:18, Russell King - ARM Linux wrote: > On Mon, Oct 19, 2015 at 02:34:22PM +0200, Tomeu Vizoso wrote: >> ... If a device is available and has >> a compatible driver, but it cannot be probed because a dependency >> isn't going to be available, that's an error and is going to cause >> real-world problems unless the device is redundant. Currently we say >> nothing because with deferred probe the probe callbacks are also part >> of the mechanism that determines the dependency order. > > So what if device X depends on device Y, and we have a driver for > device Y built-in to the kernel, but the driver for device X is a > module? > > I don't see this being solvable in the way you describe above - it's > going to identify X as being unable to be satisfied, and report it as > an error - but it's not an error at all. It's going to probe Y at late_initcall, then probe X when its driver is registered. No deferred probes nor messages about it. But if you meant to write the opposite case (X built-in and Y in a module), then I have to ask you in what situation that would make sense. >> Having a specific switch for enabling deferred probe logging sounds >> good, but there's going to be hundreds of spurious messages about >> deferred probes that were just deferrals and only one of them is going >> to be the actual error in which a device failed to find a dependency. > > Why would there be? Sounds like something's very wrong there. Sorry about that, I have checked that only now and I "only" get 39 deferred probe messages on exynos5250-snow. > You should only get deferred probes for devices which are declared to > be present, but their resources have not yet been satisfied. It > doesn't change anything if you have a kernel with lots of device drivers > or just the device drivers you need - the device drivers you don't need > do not contribute to the deferred probing in any way. I don't think that the number of registered drivers affects the number of probes that get deferred (but I'm not sure why you mention that). > So, really, after boot and all appropriate modules have been loaded, > you should end up with no deferred probes. Are you saying that you > still have "hundreds" at that point? If you do, that sounds like > there's something very wrong. I was talking about messages if we log each -EPROBE_DEFER, not devices that remain to be probed. The point being that right now we don't have a way to know if we are deferring because the dependency will be around later, or if we have a problem and the dependency isn't going to be there at all. If we had a way to enable printing the cause of each -EPROBE_DEFER, right now that would print 39 messages of this board that are only due to ordering. The actual issue would be printed in exactly the same way somewhere in the middle. >> 3) Regarding total boot time, I don't expect this series to make much >> of a difference because though we would save a lot of matching and >> querying for resources, that's little time compared with how long we >> wait for hardware to react during probing. Async probing is more >> likely to help with drivers that take a long time to probe. > > For me, on my fastest ARM board, though running serial console: > > [2.293468] VFS: Mounted root (ext4 filesystem) on device 179:1. > > There's a couple of delays in there, but they're not down to deferred > probing. The biggest one is serial console startup (due to the time > it takes to write out the initial kernel messages): > > [0.289962] f1012000.serial: ttyS0 at MMIO 0xf1012000 (irq = 23, base_baud > = 15625000) is a 16550A > [0.944124] console [ttyS0] enabled > > and DSA switch initialisation: > > [1.530655] libphy: dsa slave smi: probed > [2.034426] dsa dsa at 0 lan6 (uninitialized): attached PHY at address 0 > [Generic PHY] > > I'm not sure what causes that, but at a guess it's having to talk to the > DSA switch over the MDIO bus via several layers of indirect accesses. > Of course, serial console adds to the boot time significantly anyway, > especially at the "standard" kernel logging level. Yes, I don't think it makes any sense to measure boot times with the serial console on, because it's not comparable to production and because printing an additional line during boot affects significantly the times. To be clear, I was saying that this series should NOT affect total boot times much. >> One more thing about the breakage we have seen so far is that it's >> generally caused by implicit dependencies and hunting those is >> probably the second biggest timesink of the linux embedded developer >> after failed probes. > > ... which is generally caused by the crappy code which the average > embedded Linux developer creates, particularly with the crappy error > messages they like creating. For the most part, they _might_ as well > just print "Error!\n" and be done with it, for all the use they are. > When creating an error print, your average embedd
[GIT PULL] On-demand device probing
On Mon, Oct 19, 2015 at 01:47:50PM +0100, David Woodhouse wrote: > On Mon, 2015-10-19 at 07:35 -0500, Rob Herring wrote: > > See version 2 of the series[1] which did that. It became obvious that > > was pointless because the call paths ended up looking like this: > > Generic subsystem code -> DT look-up code -> fwnode_probe_device -> > > of_probe_device > You link to a thread which says that "AT LEAST CURRENTLY, the calling > locations [the 'DT look-up code' you mention above] are DT specific > functions anyway. > But the point I'm making is that we are working towards *fixing* that, > and *not* using DT-specific code in places where we should be using the > generic APIs. What is the plan for fixing things here? It's not obvious (at least to me) that we don't want to have the subsystems having knowledge of how they are bound to a specific firmware which is what you seem to imply here. That seems like it's going to fall down since the different firmware interfaces do have quite different ideas about how things fit together at a system level and different compatibility needs which do suggest that just trying to do a direct mapping from DT into ACPI may well not make people happy but it sounds like that's the intention. When it gets to drivers the situation is much more clear since it's normally just simple properties, it's generally a bit more worrying if drivers are needing to directly interact with cross-device linkage. This is all subsystem level code though. > None of that really negates that fact that we are *working* on cleaning > these code paths up to be firmware-agnostic, and the fact that we > haven't got to this one *yet* isn't necessarily a good reason to make > it *worse* by adding new firmware-specificity to it. It seems like we're going to have to refactor these bits of code when they get generalised anyway so I'm not sure that the additional cost here is that big. -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/b4bad635/attachment-0001.sig>
[PATCH v6 0/17] Add Analogix Core Display Port Driver
Hello Yakir, On 10/10/2015 05:35 PM, Yakir Yang wrote: > > Hi all, > >The Samsung Exynos eDP controller and Rockchip RK3288 eDP controller > share the same IP, so a lot of parts can be re-used. I split the common > code into bridge directory, then rk3288 and exynos only need to keep > some platform code. Cause I can't find the exact IP name of exynos dp > controller, so I decide to name dp core driver with "analogix" which I > find in rk3288 eDP TRM :) > > But there are still three light registers setting differents bewteen > exynos and rk3288. > 1. RK3288 have five special pll resigters which not indicata in exynos >dp controller. > 2. The address of DP_PHY_PD(dp phy power manager register) are different >between rk3288 and exynos. > 3. Rk3288 and exynos have different setting with AUX_HW_RETRY_CTL(dp debug >register). > > This series have been well tested on Rockchip platform with eDP panel > on Jerry Chromebook and Display Port Monitor on RK3288 board and thanks > to Javier at Samsung help me to find a way to install mainline kernel to > Samsung Exynos Chromebooks, so this series also have been tested on Samsung > Snow and Peach Pit Chromebooks which borrowed from my friends. > > Besides, This version was build on linux-next branch (tag next-20150918), and > the above test experiments also base on that tag. But I know the latest tag is > next-20151009, so i do rebase this series again on next-20151009, there were > little conflicts(exynos_dp removed the suspend/resume). > > But after I retest this series on next-20151009, I saw kernel crashed in mmc > driver(dw_mci_probe failed to get regulator). So i have to disabled the MMC > module(after all I boot with USB device), and I can see eDP light up normally > in startup stage, but kernel keep crashed when it try to mount the filesystem. > I thought this isn't related to dp driver directly, so i choice not to debug > more depth. > > That's to say if someone want to test this series, I suggest you applied this > series on tag-20150918, just need to fix some light conflicts with the 01 & 02 > patches (or just email me, I can send you directly). > > Thanks, Do you have a branch that I can use to test this series? Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America
[GIT PULL] On-demand device probing
On Mon, Oct 19, 2015 at 10:44:41AM +0100, David Woodhouse wrote: > On Sun, 2015-10-18 at 20:53 +0100, Mark Brown wrote: > > Do you mean firmware rather than bus here? I think that's the confusion > > I have... > Certainly, if it literally is adding of_* calls then that would seem to > be gratuitously firmware-specific. Nothing should be using those these > days; any new code should be using the generic device property APIs > (except in special cases). It's not entirely clear to me that we should be moving to fwnode_ wholesale yet - the last advice was to hold off for a little while which makes sense given that the ACPI community still doesn't seem to have worked out what it wants to do here and how. The x86 embedded people are all gung ho but it's less clear that anyone else wants to use _DSD in quite the same way (I know of some efforts to use _DSD separately to the DT compatibility stuff) and there are some vendors who definitely do have completely different binding schemes for ACPI and DT and therefore specifically care which is in use. It would really help if ACPI could get their binding review process in place, and if we do want to actually start converting everything to fwnode_ we need to start communicating that actively since otherwise people can't really be expected to know. -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/f1d7d9d1/attachment-0001.sig>
[GIT PULL] On-demand device probing
On Mon, 2015-10-19 at 15:50 +0100, Mark Brown wrote: > > But the point I'm making is that we are working towards *fixing* that, > > and *not* using DT-specific code in places where we should be using the > > generic APIs. > > What is the plan for fixing things here? It's not obvious (at least to > me) that we don't want to have the subsystems having knowledge of how > they are bound to a specific firmware which is what you seem to imply > here. I don't know that there *is* a coherent plan here to address it all. Certainly, we *will* need subsystems to have firmware-specific knowledge in some cases. Take GPIO as an example; ACPI *has* a way to describe GPIO, and properties which reference GPIO pins are intended to work through that â while in DT, properties which reference GPIO pins will have different contents. They'll be compatible at the driver level, in the sense that there's a call to get a given GPIO given the property name, but the subsystems *will* be doing different things behind the scenes. My plan, such as it is, is to go through the leaf-node drivers which almost definitely *should* be firmware-agnostic, and convert those. And then take stock of what we have left, and work out what, if anything, still needs to be done. > It seems like we're going to have to refactor these bits of code when > they get generalised anyway so I'm not sure that the additional cost > here is that big. That's an acceptable answer â "we're adding legacy code here but we know it's going to be refactored anyway". If that's true, all it takes is a note in the commit comment to that effect. That's different from having not thought about it :) -- dwmw2 -- next part -- A non-text attachment was scrubbed... Name: smime.p7s Type: application/x-pkcs7-signature Size: 5691 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/f1e07573/attachment.bin>
[PATCH 1/9] drm: fix trivial typos
On Sun, Oct 18, 2015 at 11:29 AM, Geliang Tang wrote: > s/regsiter/register/ > > Signed-off-by: Geliang Tang Applied. thanks! Alex > --- > drivers/gpu/drm/amd/include/atombios.h | 2 +- > drivers/gpu/drm/radeon/cayman_blit_shaders.c| 2 +- > drivers/gpu/drm/radeon/evergreen_blit_shaders.c | 2 +- > drivers/gpu/drm/radeon/r600_blit_shaders.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/include/atombios.h > b/drivers/gpu/drm/amd/include/atombios.h > index 44c5d4a..5526226 100644 > --- a/drivers/gpu/drm/amd/include/atombios.h > +++ b/drivers/gpu/drm/amd/include/atombios.h > @@ -6784,7 +6784,7 @@ typedef struct _ATOM_MC_INIT_PARAM_TABLE_V2_1 >ULONG ulMCUcodeRomStartAddr; >ULONG ulMCUcodeLength; >USHORTusMcRegInitTableOffset; // offset of > ATOM_REG_INIT_SETTING array for MC core register settings. > - USHORTusReserved; // offset of > ATOM_INIT_REG_BLOCK for MC SEQ/PHY regsiter setting > + USHORTusReserved; // offset of > ATOM_INIT_REG_BLOCK for MC SEQ/PHY register setting > }ATOM_MC_INIT_PARAM_TABLE_V2_1; > > > diff --git a/drivers/gpu/drm/radeon/cayman_blit_shaders.c > b/drivers/gpu/drm/radeon/cayman_blit_shaders.c > index 98d009e..9fec4d0 100644 > --- a/drivers/gpu/drm/radeon/cayman_blit_shaders.c > +++ b/drivers/gpu/drm/radeon/cayman_blit_shaders.c > @@ -32,7 +32,7 @@ > * evergreen cards need to use the 3D engine to blit data which requires > * quite a bit of hw state setup. Rather than pull the whole 3D driver > * (which normally generates the 3D state) into the DRM, we opt to use > - * statically generated state tables. The regsiter state and shaders > + * statically generated state tables. The register state and shaders > * were hand generated to support blitting functionality. See the 3D > * driver or documentation for descriptions of the registers and > * shader instructions. > diff --git a/drivers/gpu/drm/radeon/evergreen_blit_shaders.c > b/drivers/gpu/drm/radeon/evergreen_blit_shaders.c > index d433834..1a96ddb 100644 > --- a/drivers/gpu/drm/radeon/evergreen_blit_shaders.c > +++ b/drivers/gpu/drm/radeon/evergreen_blit_shaders.c > @@ -32,7 +32,7 @@ > * evergreen cards need to use the 3D engine to blit data which requires > * quite a bit of hw state setup. Rather than pull the whole 3D driver > * (which normally generates the 3D state) into the DRM, we opt to use > - * statically generated state tables. The regsiter state and shaders > + * statically generated state tables. The register state and shaders > * were hand generated to support blitting functionality. See the 3D > * driver or documentation for descriptions of the registers and > * shader instructions. > diff --git a/drivers/gpu/drm/radeon/r600_blit_shaders.c > b/drivers/gpu/drm/radeon/r600_blit_shaders.c > index 34c8b23..443cbe5 100644 > --- a/drivers/gpu/drm/radeon/r600_blit_shaders.c > +++ b/drivers/gpu/drm/radeon/r600_blit_shaders.c > @@ -32,7 +32,7 @@ > * R6xx+ cards need to use the 3D engine to blit data which requires > * quite a bit of hw state setup. Rather than pull the whole 3D driver > * (which normally generates the 3D state) into the DRM, we opt to use > - * statically generated state tables. The regsiter state and shaders > + * statically generated state tables. The register state and shaders > * were hand generated to support blitting functionality. See the 3D > * driver or documentation for descriptions of the registers and > * shader instructions. > -- > 2.5.0 > > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/mgag200: don't pass NULL file_priv to drm_gem_object_lookup()
On Mon, Oct 19, 2015 at 04:27:18AM -0600, Jan Beulich wrote:
> Commit bf89209a6d ("drm/mga200g: Hold a proper reference for
> cursor_set") clearly didn't take the call site in
> drm_fb_helper.c:restore_fbdev_mode() into account, which passes NULL
> for file_priv and hence causes drm_gem_object_lookup() to fault. Move
> the lookup back to before "obj" is actually needed, adjusting error
> paths suitably once again.
>
> Signed-off-by: Jan Beulich
> Cc: Daniel Vetter
> Cc: Thierry Reding
Oops, totally forgot that handle=0 is used to disable the cursor. Thanks
for the fix.
Reviewed-by: Daniel Vetter
Dave, since this is in 4.3 can you pls pick this up for drm-fixes?
Thanks, Daniel
> ---
> drivers/gpu/drm/mgag200/mgag200_cursor.c | 18 ++
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> --- 4.3-rc6/drivers/gpu/drm/mgag200/mgag200_cursor.c
> +++ 4.3-rc6-mgag200-cursor-set/drivers/gpu/drm/mgag200/mgag200_cursor.c
> @@ -40,7 +40,7 @@ int mga_crtc_cursor_set(struct drm_crtc
> struct mgag200_bo *pixels_2 = mdev->cursor.pixels_2;
> struct mgag200_bo *pixels_current = mdev->cursor.pixels_current;
> struct mgag200_bo *pixels_prev = mdev->cursor.pixels_prev;
> - struct drm_gem_object *obj;
> + struct drm_gem_object *obj = NULL;
> struct mgag200_bo *bo = NULL;
> int ret = 0;
> unsigned int i, row, col;
> @@ -70,15 +70,11 @@ int mga_crtc_cursor_set(struct drm_crtc
> BUG_ON(pixels_2 != pixels_current && pixels_2 != pixels_prev);
> BUG_ON(pixels_current == pixels_prev);
>
> - obj = drm_gem_object_lookup(dev, file_priv, handle);
> - if (!obj)
> - return -ENOENT;
> -
> ret = mgag200_bo_reserve(pixels_1, true);
> if (ret) {
> WREG8(MGA_CURPOSXL, 0);
> WREG8(MGA_CURPOSXH, 0);
> - goto out_unref;
> + return ret;
> }
> ret = mgag200_bo_reserve(pixels_2, true);
> if (ret) {
> @@ -110,6 +106,12 @@ int mga_crtc_cursor_set(struct drm_crtc
> }
> }
>
> + obj = drm_gem_object_lookup(dev, file_priv, handle);
> + if (!obj) {
> + ret = -ENOENT;
> + goto out1;
> + }
> +
> bo = gem_to_mga_bo(obj);
> ret = mgag200_bo_reserve(bo, true);
> if (ret) {
> @@ -243,13 +245,13 @@ int mga_crtc_cursor_set(struct drm_crtc
> out2:
> mgag200_bo_unreserve(bo);
> out1:
> + if (obj)
> + drm_gem_object_unreference_unlocked(obj);
> if (ret)
> mga_hide_cursor(mdev);
> mgag200_bo_unreserve(pixels_1);
> out_unreserve1:
> mgag200_bo_unreserve(pixels_2);
> -out_unref:
> - drm_gem_object_unreference_unlocked(obj);
>
> return ret;
> }
>
>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] [PATCH 1/3] drm/edid: Fix up clock for CEA/HDMI modes specified via detailed timings
On Fri, Oct 09, 2015 at 01:54:58PM +0300, Jani Nikula wrote: > On Thu, 08 Oct 2015, Daniel Vetter wrote: > > On Thu, Oct 08, 2015 at 12:22:31PM -0400, Adam Jackson wrote: > >> On Thu, 2015-10-08 at 11:43 +0300, ville.syrjala at linux.intel.com wrote: > >> > From: Ville Syrjälä > >> > > >> > EDID detailed timings have a resolution of 10kHz for the pixel clock, so > >> > they can't represent certain CEA/HDMI modes accurately. If we see a mode > >> > coming in via detailed timings which otherwise matches one of the > >> > CEA/HDMI modes except the clock is just a bit off, let's assume that the > >> > intention was for that mode to be one of the CEA/HDMI modes and go ahead > >> > and fix up the clock to match the CEA/HDMI spec exactly (well, as close > >> > as we can get with the 1 kHz resolution we use). > >> > > >> > This should help code that's looking for an exact clock match (eg. i915 > >> > audio N/CTS setup). > >> > >> Looks like a sane set of changes. Series is: > >> > >> Reviewed-by: Adam Jackson > > > > Merged the first two patches to drm-misc (the later one has conflicts with > > the lack of drm-intel-next, so can pull it in only after a rebase). > > This is needed in v4.3. Ok, after first dropping them I now reapplied them again for 4.4. Hopefully we're converging on this here now wrt maintainer fumbles ;-) Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[Bug 106271] New: Switch between AMD hybrid graphics (HD 8650G / HD 8970M) makes hardware reset.
https://bugzilla.kernel.org/show_bug.cgi?id=106271 Bug ID: 106271 Summary: Switch between AMD hybrid graphics (HD 8650G / HD 8970M) makes hardware reset. Product: Drivers Version: 2.5 Kernel Version: 4.2.3 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri at kernel-bugs.osdl.org Reporter: aneroid at inbox.ru Regression: No Created attachment 190531 --> https://bugzilla.kernel.org/attachment.cgi?id=190531&action=edit lspci -vvv Hello, Hardware: MSI GX60 with two graphic cards, integrated inside A10 GPU and discrete AMD R9 M290X. Integrated card is the main card by default: sudo cat /sys/kernel/debug/vgaswitcheroo/switch 0:IGD:+:Pwr::00:01.0 1:DIS: :DynOff::01:00.0 If I try to swich to discrete card: sudo echo DIS > /sys/kernel/debug/vgaswitcheroo/switch Hardware reboot happend and BIOS starting. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 106271] Switch between AMD hybrid graphics (HD 8650G / HD 8970M) makes hardware reset.
https://bugzilla.kernel.org/show_bug.cgi?id=106271 --- Comment #1 from Aneroid --- Created attachment 190541 --> https://bugzilla.kernel.org/attachment.cgi?id=190541&action=edit glxinfo -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 106271] Switch between AMD hybrid graphics (HD 8650G / HD 8970M) makes hardware reset.
https://bugzilla.kernel.org/show_bug.cgi?id=106271 --- Comment #2 from Aneroid --- Created attachment 190551 --> https://bugzilla.kernel.org/attachment.cgi?id=190551&action=edit dmesg -- You are receiving this mail because: You are watching the assignee of the bug.
[Intel-gfx] [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction
On Mon, Oct 19, 2015 at 06:08:52PM +, Smith, Gary K wrote:
> FYI - this shouldn't block the commits, but should be optimized later (fairly
> soon).
>
> I believe the current implementation ends up executing
> while (count < CHV_DEGAMMA_MAX_VALS) {
> // Do lots of caclulation
> I915_WRITE(cgm_degamma_reg, word);
> Every frame after you set the property, whether you change the contents of
> the blob or not. Clearly this is really inefficient when there are 100s of
> gamma values to write.
> Same with the Gamma and CTM. CTM is less of an issue with only 9 entries.
>
> My suggestion here is to set a boolean when the property has been set as
> part of a flip and only apply it on the atomic commit after the update
> was received.
Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g.
for changed planes tracked in the crtc_state) in the state where we need
to commit the update. That /should/ be there really, didn't ever realize
it's not done. Note that that for legacy cursors we update them without
waiting for vblanks and legacy userspace does that a _lot_ (*cough* X
server *cough*), if the overhead is severe this might be a problem and
needs to be fixed before merging.
-Daniel
>
> Thanks
> Gary
>
> -Original Message-
> From: Sharma, Shashank
> Sent: Friday, October 16, 2015 3:29 PM
> To: dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org;
> emil.l.velikov at gmail.com; Roper, Matthew D; Bradford, Robert; Bish, Jim
> Cc: Matheson, Annie J; kausalmalladi at gmail.com; Kumar, Kiran S; Smith,
> Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, Avinash Reddy; Sharma,
> Shashank
> Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction
>
> CHV/BSW supports Degamma color correction, which linearizes all the
> non-linear color values. This will be applied before Color Transformation.
>
> This patch does the following:
> 1. Attach deGamma property to CRTC
> 2. Add the core function to program DeGamma correction values for
>CHV/BSW platform
> 2. Add DeGamma correction macros/defines
>
> Signed-off-by: Shashank Sharma
> Signed-off-by: Kausal Malladi
> ---
> drivers/gpu/drm/i915/i915_reg.h| 6 ++
> drivers/gpu/drm/i915/intel_color_manager.c | 92
> ++ drivers/gpu/drm/i915/intel_color_manager.h |
> 5 ++
> 3 files changed, 103 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define
> _PIPE_GAMMA_BASE(pipe) \
> (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))
>
> +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000)
> +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000)
> +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000)
> +#define _PIPE_DEGAMMA_BASE(pipe) \
> + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA,
> +PIPEC_CGM_DEGAMMA))
> +
> #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index acb9647..1bbad79 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,92 @@
>
> #include "intel_color_manager.h"
>
> +static int chv_set_degamma(struct drm_device *dev,
> + struct drm_property_blob *blob, struct drm_crtc *crtc) {
> + u16 red_fract, green_fract, blue_fract;
> + u32 red, green, blue;
> + u32 num_samples;
> + u32 word = 0;
> + u32 count, cgm_control_reg, cgm_degamma_reg;
> + enum pipe pipe;
> + struct drm_palette *degamma_data;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_r32g32b32 *correction_values = NULL;
> + struct drm_crtc_state *state = crtc->state;
> +
> + if (WARN_ON(!blob))
> + return -EINVAL;
> +
> + degamma_data = (struct drm_palette *)blob->data;
> + pipe = to_intel_crtc(crtc)->pipe;
> + num_samples = blob->length / sizeof(struct drm_r32g32b32);
> +
> + switch (num_samples) {
> + case GAMMA_DISABLE_VALS:
> + /* Disable DeGamma functionality on Pipe - CGM Block */
> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
> + cgm_control_reg &= ~CGM_DEGAMMA_EN;
> + state->palette_before_ctm_blob = NULL;
> +
> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
> + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n",
> + pipe_name(pipe));
> + break;
> +
> + case CHV_DEGAMMA_MAX_VALS:
> + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe);
> + count = 0;
> + correction_values = (struct drm_r32g32
[Bug 92544] Tonga fails second resume from mem sleep.
https://bugs.freedesktop.org/show_bug.cgi?id=92544 Bug ID: 92544 Summary: Tonga fails second resume from mem sleep. Product: DRI Version: DRI git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel at lists.freedesktop.org Reporter: adf.lists at gmail.com R9285 resumes OK after doing echo mem >/sys/power/state, but will not sleep/resume a second time. Tested with a few kernels 4.4 wip 4.3 and 4.2. Nothing is logged in dmesg for 4.4 and 4.3 X bails after some time with - [ 29371.315] (EE) Backtrace: [ 29371.326] (EE) 0: /usr/bin/X (xorg_backtrace+0x49) [0x584559] [ 29371.326] (EE) 1: /usr/bin/X (0x40+0x188459) [0x588459] [ 29371.326] (EE) 2: /lib/libpthread.so.0 (0x7f74cfb8a000+0xf2a0) [0x7f74cfb992a0] [ 29371.326] (EE) 3: /lib/libc.so.6 (0x7f74ce1cb000+0x8a7e7) [0x7f74ce2557e7] [ 29371.327] (EE) 4: /usr/lib/xorg/modules/libglamoregl.so (0x7f74cdd8+0x196ab) [0x7f74cdd996ab] [ 29371.327] (EE) 5: /usr/bin/X (0x40+0x111a9f) [0x511a9f] [ 29371.327] (EE) 6: /usr/bin/X (miPaintWindow+0x20c) [0x56802c] [ 29371.327] (EE) 7: /usr/bin/X (miHandleValidateExposures+0x47) [0x57c767] [ 29371.327] (EE) 8: /usr/bin/X (SetRootClip+0x2b5) [0x465905] [ 29371.327] (EE) 9: /usr/bin/X (0x40+0xb81d2) [0x4b81d2] [ 29371.327] (EE) 10: /usr/bin/X (xf86VTEnter+0x12b) [0x47420b] [ 29371.327] (EE) 11: /usr/bin/X (WakeupHandler+0x6b) [0x43b59b] [ 29371.327] (EE) 12: /usr/bin/X (WaitForSomething+0x1ba) [0x58173a] [ 29371.327] (EE) 13: /usr/bin/X (0x40+0x3699e) [0x43699e] [ 29371.328] (EE) 14: /usr/bin/X (0x40+0x3aacb) [0x43aacb] [ 29371.328] (EE) 15: /lib/libc.so.6 (__libc_start_main+0xed) [0x7f74ce1ec36d] [ 29371.328] (EE) 16: /usr/bin/X (0x40+0x262a1) [0x4262a1] [ 29371.328] (EE) [ 29371.328] (EE) Bus error at address 0x7f74c7003320 [ 29371.328] (EE) Fatal server error: [ 29371.328] (EE) Caught signal 7 (Bus error). Server aborting fbcon is still functional at this point but trying again to startx will hang, last lines of xorg log = [ 10618.330] (II) Loading sub module "dri2" [ 10618.330] (II) LoadModule: "dri2" [ 10618.330] (II) Module "dri2" already built-in On 4.2 I see logged at this point lots of - GPU lockup (current fence id 0x0147 last fence id 0x0148 on ring 0) ring 0 stalled for more than 61125msec Waiting with 4.3 or 4.4 does not produce a hung task backtrace. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/7abddf13/attachment-0001.html>
[Bug 92524] system hang on "radeon_hwmon_get_pwm1_enable"
https://bugs.freedesktop.org/show_bug.cgi?id=92524 --- Comment #6 from Thomas DEBESSE --- So, some news! I've just tested with radeon.dpm=1, but less than 10 minute after the boot the graphic display completely hangs with graphic glitches (the system was available from ssh for example, but was not able to reboot), the command "cat /sys/class/hwmon/hwmon0/pwm1_enable" works (I did it *after* the graphical hang, so the graphical hang was not a consequence of this command). So, the glitch is another bug, but I now verified that "cat /sys/class/hwmon/hwmon0/pwm1_enable" is not a crash cause when radeon.dpm=1. Also, I tried to load amdgpu instead of radeon module, blacklinsting radeon, then I verified that with amdgpu.dpm=0 there is no "/sys/class/hwmon/hwmon0/pwm1_enable" file and with amdgpu.dpm=1, there is a "/sys/class/hwmon/hwmon0/pwm1_enable" file and I can "cat" it. So, the bug was not reproduced in amdgpu module, which is a good news. I've not yet tried to recompile the kernel to test your patch. I'm now running Ubuntu 15.10, of course it changes nothing about this bug, but they ship by default a 4.2 kernel so I can use my distro tools to recompile the kernel and do it cleanly, applying the patch on the official Ubuntu kernel tree. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20151019/a55419fa/attachment.html>
[RFCv0 0/8] gallium: add support for NIR as alternate IR
From: Rob Clark Still quite rough around the edges, but now it's at the point of sorta- kinda-working-sometimes. So I guess time to get wider feedback. The initial goal is to figure out things that glsl_to_nir does differently from tgsi_to_nir, so we can fix them up and have better likelyhood of drivers being able to consume NIR from multiple sources. Eventually I'd like to be able to re-use spriv->nir for compute/ clover support, for example. And at some point I'd like to be able to bypass the conversion from glsl->tgsi for mesa st. But for now, being able to track down and fix differences between glsl_to_nir vs tgsi_to_nir is a good start. With what I've added to pipe_shader_state (which is, I suppose, missing some doc updates), the intention is that drivers can ask for an IR other than TGSI. And what they actually get will be either TGSI or their preferred IR. Drivers will have to always handle the TGSI case, so the intention isn't to port all the different state trackers (or internally generated shaders, etc). I suppose pipe_compute_state needs similar treatment, but I've skipped that for now. Currently only the direct GLSL->NIR path is implemented for vertex shaders, which results in a huge hack at the end of the series. It isn't ever intended for drivers to mix/match glsl->nir vs glsl->tgsi-> nir across shader stages. But this is sufficient for now for me to debug other issues. Shader variant handling will be completely broken at the moment, without having a nir_clone()[1]. (Ian, depending on how far that is from the top of your TODO list, I can take a stab at that, since I might be the first consumer.) Also, there is some trivial shader variant handling in mesa st which would have to be ported to NIR. Or, perhaps, just somehow expose the shader key to the driver. (Currently most drivers are doing much more variant handling within the driver, and having two levels of variants does seem a bit silly.) [1] https://bugs.freedesktop.org/show_bug.cgi?id=89436 (First patch isn't actually RFC.. just an issue that I noticed in the process.) Rob Clark (8): nir: add nir_var_all enum gallium: refactor pipe_shader_state to support multiple IR's gallium: add NIR as a possible IR mesa/st: add support for NIR as possible driver IR freedreno/ir3: drop tokens arg freedreno/ir3: add support for NIR as preferred IR freedreno/ir3: fix const_index handling for uniforms HACK: freedreno/ir3: hack for mismatch on varying slots for glsl->tgsi->nir vs glsl->nir src/gallium/auxiliary/hud/hud_context.c| 14 +- src/gallium/auxiliary/postprocess/pp_run.c | 4 +- src/gallium/auxiliary/tgsi/tgsi_ureg.c | 6 +- src/gallium/auxiliary/util/u_simple_shaders.c | 42 +++- src/gallium/auxiliary/util/u_tests.c | 7 +- src/gallium/drivers/freedreno/freedreno_screen.c | 5 +- src/gallium/drivers/freedreno/freedreno_util.h | 1 + .../drivers/freedreno/ir3/ir3_compiler_nir.c | 38 +++- src/gallium/drivers/freedreno/ir3/ir3_shader.c | 30 ++- src/gallium/drivers/freedreno/ir3/ir3_shader.h | 3 + src/gallium/include/pipe/p_defines.h | 13 +- src/gallium/include/pipe/p_state.h | 21 +- src/glsl/nir/nir.c | 4 + src/glsl/nir/nir.h | 1 + src/glsl/nir/nir_lower_io.c| 2 +- src/mesa/drivers/dri/i965/brw_nir.c| 3 +- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 223 - src/mesa/state_tracker/st_program.c| 43 +++- 18 files changed, 422 insertions(+), 38 deletions(-) -- 2.5.0
[PATCH 1/8] nir: add nir_var_all enum
Otherwise, passing -1 gets you:
error: invalid conversion from 'int' to 'nir_variable_mode' [-fpermissive]
Signed-off-by: Rob Clark
---
src/glsl/nir/nir.c | 4
src/glsl/nir/nir.h | 1 +
src/glsl/nir/nir_lower_io.c | 2 +-
src/mesa/drivers/dri/i965/brw_nir.c | 3 ++-
4 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index 793bdaf..caa9ffc 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -107,6 +107,10 @@ void
nir_shader_add_variable(nir_shader *shader, nir_variable *var)
{
switch (var->data.mode) {
+ case nir_var_all:
+ assert(!"invalid mode");
+ break;
+
case nir_var_local:
assert(!"nir_shader_add_variable cannot be used for local variables");
break;
diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index c867e6d..2a84cb0 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -82,6 +82,7 @@ typedef struct {
} nir_state_slot;
typedef enum {
+ nir_var_all = -1,
nir_var_shader_in,
nir_var_shader_out,
nir_var_global,
diff --git a/src/glsl/nir/nir_lower_io.c b/src/glsl/nir/nir_lower_io.c
index 688b48f..b4ed857 100644
--- a/src/glsl/nir/nir_lower_io.c
+++ b/src/glsl/nir/nir_lower_io.c
@@ -176,7 +176,7 @@ nir_lower_io_block(nir_block *block, void *void_state)
nir_variable_mode mode = intrin->variables[0]->var->data.mode;
- if (state->mode != -1 && state->mode != mode)
+ if (state->mode != nir_var_all && state->mode != mode)
continue;
switch (intrin->intrinsic) {
diff --git a/src/mesa/drivers/dri/i965/brw_nir.c
b/src/mesa/drivers/dri/i965/brw_nir.c
index af9d041..e22f5fa 100644
--- a/src/mesa/drivers/dri/i965/brw_nir.c
+++ b/src/mesa/drivers/dri/i965/brw_nir.c
@@ -219,7 +219,8 @@ brw_create_nir(struct brw_context *brw,
nir_assign_var_locations(&nir->uniforms,
&nir->num_uniforms,
is_scalar ? type_size_scalar : type_size_vec4);
- nir_lower_io(nir, -1, is_scalar ? type_size_scalar : type_size_vec4);
+ nir_lower_io(nir, nir_var_all,
+is_scalar ? type_size_scalar : type_size_vec4);
nir_validate_shader(nir);
nir_remove_dead_variables(nir);
--
2.5.0
[RFCv0 2/8] gallium: refactor pipe_shader_state to support multiple IR's
The goal is to allow the pipe driver to request something other than
TGSI, but detect whether what is getting is TGSI vs what it requested.
The pipe drivers will always have to support TGSI (and convert that into
whatever it is that they prefer), but in some cases we should be able to
skip the TGSI intermediate step (such as glsl->nir vs glsl->tgsi->nir).
I think pipe_compute_state should get similar treatment. Currently,
afaict, it has one user and one consumer, which has allowed it to be
sloppy wrt. supporting alternative IR's.
---
src/gallium/auxiliary/hud/hud_context.c | 14 +++--
src/gallium/auxiliary/postprocess/pp_run.c| 4 ++-
src/gallium/auxiliary/tgsi/tgsi_ureg.c| 6 ++--
src/gallium/auxiliary/util/u_simple_shaders.c | 42 +++
src/gallium/auxiliary/util/u_tests.c | 7 -
src/gallium/include/pipe/p_defines.h | 12 ++--
src/gallium/include/pipe/p_state.h| 20 +++--
7 files changed, 89 insertions(+), 16 deletions(-)
diff --git a/src/gallium/auxiliary/hud/hud_context.c
b/src/gallium/auxiliary/hud/hud_context.c
index 95eed26..9f39abd 100644
--- a/src/gallium/auxiliary/hud/hud_context.c
+++ b/src/gallium/auxiliary/hud/hud_context.c
@@ -1179,7 +1179,12 @@ hud_create(struct pipe_context *pipe, struct cso_context
*cso)
};
struct tgsi_token tokens[1000];
- struct pipe_shader_state state = {tokens};
+ struct pipe_shader_state state;
+
+ memset(&state, 0, sizeof(state));
+
+ state.ir = PIPE_SHADER_IR_TGSI;
+ state.tokens = tokens;
if (!tgsi_text_translate(fragment_shader_text, tokens,
Elements(tokens))) {
assert(0);
@@ -1226,7 +1231,12 @@ hud_create(struct pipe_context *pipe, struct cso_context
*cso)
};
struct tgsi_token tokens[1000];
- struct pipe_shader_state state = {tokens};
+ struct pipe_shader_state state;
+
+ memset(&state, 0, sizeof(state));
+
+ state.ir = PIPE_SHADER_IR_TGSI;
+ state.tokens = tokens;
if (!tgsi_text_translate(vertex_shader_text, tokens, Elements(tokens))) {
assert(0);
diff --git a/src/gallium/auxiliary/postprocess/pp_run.c
b/src/gallium/auxiliary/postprocess/pp_run.c
index caa2062..6cd2b70 100644
--- a/src/gallium/auxiliary/postprocess/pp_run.c
+++ b/src/gallium/auxiliary/postprocess/pp_run.c
@@ -272,8 +272,10 @@ pp_tgsi_to_state(struct pipe_context *pipe, const char
*text, bool isvs,
return NULL;
}
+ memset(&state, 0, sizeof(state));
+
+ state.ir = PIPE_SHADER_IR_TGSI;
state.tokens = tokens;
- memset(&state.stream_output, 0, sizeof(state.stream_output));
if (isvs) {
ret_state = pipe->create_vs_state(pipe, &state);
diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
index 3d21319..96a51c4 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
@@ -1777,14 +1777,16 @@ void *ureg_create_shader( struct ureg_program *ureg,
{
struct pipe_shader_state state;
+ memset(&state, 0, sizeof(state));
+
+ state.ir = PIPE_SHADER_IR_TGSI;
+
state.tokens = ureg_finalize(ureg);
if(!state.tokens)
return NULL;
if (so)
state.stream_output = *so;
- else
- memset(&state.stream_output, 0, sizeof(state.stream_output));
switch (ureg->processor) {
case TGSI_PROCESSOR_VERTEX:
diff --git a/src/gallium/auxiliary/util/u_simple_shaders.c
b/src/gallium/auxiliary/util/u_simple_shaders.c
index 6eed337..8be0da9 100644
--- a/src/gallium/auxiliary/util/u_simple_shaders.c
+++ b/src/gallium/auxiliary/util/u_simple_shaders.c
@@ -121,7 +121,12 @@ void *util_make_layered_clear_vertex_shader(struct
pipe_context *pipe)
"MOV OUT[2], SV[0]\n"
"END\n";
struct tgsi_token tokens[1000];
- struct pipe_shader_state state = {tokens};
+ struct pipe_shader_state state;
+
+ memset(&state, 0, sizeof(state));
+
+ state.ir = PIPE_SHADER_IR_TGSI;
+ state.tokens = tokens;
if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
assert(0);
@@ -149,7 +154,12 @@ void *util_make_layered_clear_helper_vertex_shader(struct
pipe_context *pipe)
"MOV OUT[2].x, SV[0].\n"
"END\n";
struct tgsi_token tokens[1000];
- struct pipe_shader_state state = {tokens};
+ struct pipe_shader_state state;
+
+ memset(&state, 0, sizeof(state));
+
+ state.ir = PIPE_SHADER_IR_TGSI;
+ state.tokens = tokens;
if (!tgsi_text_translate(text, tokens, Elements(tokens))) {
assert(0);
@@ -192,7 +202,12 @@ void *util_make_layered_clear_geometry_shader(struct
pipe_context *pipe)
"EMIT IMM[0].\n"
"END\n";
struct tgsi_token tokens[1000];
- struct pipe_shader_state state = {tokens};
+ struct pipe_shader_state state;
+
+ memset(&state, 0, sizeof(state));
+
+ state.ir = PIPE_SHADER_IR_TGSI;
+ state.tokens = tokens;
if (!tgsi_text_translate(text, tokens, Elements(tokens)))
[RFCv0 3/8] gallium: add NIR as a possible IR
--- src/gallium/include/pipe/p_defines.h | 1 + src/gallium/include/pipe/p_state.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h index 29b0bfb..0dbc54c 100644 --- a/src/gallium/include/pipe/p_defines.h +++ b/src/gallium/include/pipe/p_defines.h @@ -715,6 +715,7 @@ enum pipe_shader_ir PIPE_SHADER_IR_TGSI = 0, PIPE_SHADER_IR_LLVM, PIPE_SHADER_IR_NATIVE, + PIPE_SHADER_IR_NIR, }; /** diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index 50bfc4a..54ca31f 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -230,6 +230,7 @@ struct pipe_shader_state const struct tgsi_token *tokens; void *llvm; void *native; + void *nir; }; struct pipe_stream_output_info stream_output; }; -- 2.5.0
[RFCv0 6/8] freedreno/ir3: add support for NIR as preferred IR
For now under debug flag, since only suitable for debugging/testing.
---
src/gallium/drivers/freedreno/freedreno_screen.c | 5 +++-
src/gallium/drivers/freedreno/freedreno_util.h | 1 +
.../drivers/freedreno/ir3/ir3_compiler_nir.c | 15 ++-
src/gallium/drivers/freedreno/ir3/ir3_shader.c | 30 +++---
src/gallium/drivers/freedreno/ir3/ir3_shader.h | 3 +++
5 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c
b/src/gallium/drivers/freedreno/freedreno_screen.c
index b64f78c..30def8e 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -71,6 +71,7 @@ static const struct debug_named_value debug_options[] = {
{"glsl120", FD_DBG_GLSL120,"Temporary flag to force GLSL 1.20
(rather than 1.30) on a3xx+"},
{"shaderdb", FD_DBG_SHADERDB, "Enable shaderdb output"},
{"flush", FD_DBG_FLUSH, "Force flush after every draw"},
+ {"nir", FD_DBG_NIR,"Prefer NIR as native IR"},
DEBUG_NAMED_VALUE_END
};
@@ -398,7 +399,7 @@ fd_screen_get_shader_param(struct pipe_screen *pscreen,
unsigned shader,
case PIPE_SHADER_CAP_TGSI_DROUND_SUPPORTED:
case PIPE_SHADER_CAP_TGSI_DFRACEXP_DLDEXP_SUPPORTED:
case PIPE_SHADER_CAP_TGSI_FMA_SUPPORTED:
-case PIPE_SHADER_CAP_TGSI_ANY_INOUT_DECL_RANGE:
+ case PIPE_SHADER_CAP_TGSI_ANY_INOUT_DECL_RANGE:
return 0;
case PIPE_SHADER_CAP_TGSI_SQRT_SUPPORTED:
return 1;
@@ -410,6 +411,8 @@ fd_screen_get_shader_param(struct pipe_screen *pscreen,
unsigned shader,
case PIPE_SHADER_CAP_MAX_SAMPLER_VIEWS:
return 16;
case PIPE_SHADER_CAP_PREFERRED_IR:
+ if ((fd_mesa_debug & FD_DBG_NIR) && is_ir3(screen))
+ return PIPE_SHADER_IR_NIR;
return PIPE_SHADER_IR_TGSI;
}
debug_printf("unknown shader param %d\n", param);
diff --git a/src/gallium/drivers/freedreno/freedreno_util.h
b/src/gallium/drivers/freedreno/freedreno_util.h
index 0d2418e..56d3235 100644
--- a/src/gallium/drivers/freedreno/freedreno_util.h
+++ b/src/gallium/drivers/freedreno/freedreno_util.h
@@ -73,6 +73,7 @@ enum adreno_stencil_op fd_stencil_op(unsigned op);
#define FD_DBG_GLSL120 0x0400
#define FD_DBG_SHADERDB 0x0800
#define FD_DBG_FLUSH0x1000
+#define FD_DBG_NIR 0x2000
extern int fd_mesa_debug;
extern bool fd_binning_enabled;
diff --git a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
index b4e5483..5a53e32 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
@@ -164,7 +164,14 @@ to_nir(struct ir3_compile *ctx, struct ir3_shader_variant
*so)
tex_options.lower_txp = (1 << GLSL_SAMPLER_DIM_3D);
}
- struct nir_shader *s = tgsi_to_nir(so->shader->tokens, &options);
+ struct nir_shader *s;
+
+ if (so->shader->nir) {
+ // XXX need nir_clone() here..
+ s = so->shader->nir;
+ } else {
+ s = tgsi_to_nir(so->shader->tokens, &options);
+ }
if (fd_mesa_debug & FD_DBG_OPTMSGS) {
debug_printf("--\n");
@@ -292,6 +299,12 @@ compile_error(struct ir3_compile *ctx, const char *format,
...)
static void
compile_free(struct ir3_compile *ctx)
{
+ /* TODO .. probably need a ralloc_free(ctx->s) here, at least
+* for the tgsi_to_nir case.. right now in the case we get
+* NIR directly, we want to skip this since the nir_shader isn't
+* getting cloned.. and/or if it was we might be wanting to
+* free it in ir3_shader_destroy()??
+*/
ralloc_free(ctx);
}
diff --git a/src/gallium/drivers/freedreno/ir3/ir3_shader.c
b/src/gallium/drivers/freedreno/ir3/ir3_shader.c
index 7b56533..0cbf8fe 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_shader.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_shader.c
@@ -39,6 +39,7 @@
#include "ir3_shader.h"
#include "ir3_compiler.h"
+#include "ir3_nir.h"
static void
@@ -188,9 +189,15 @@ create_variant(struct ir3_shader *shader, struct
ir3_shader_key key)
v->type = shader->type;
if (fd_mesa_debug & FD_DBG_DISASM) {
- DBG("dump tgsi: type=%d, k={bp=%u,cts=%u,hp=%u}", shader->type,
- key.binning_pass, key.color_two_side,
key.half_precision);
- tgsi_dump(shader->tokens, 0);
+ if (shader->nir) {
+ DBG("dump nir: type=%d, k={bp=%u,cts=%u,hp=%u}",
shader->type,
+ key.binning_pass, key.color_two_side,
key.half_precision);
+ nir_print_shader(shader->nir, stderr);
+ } else {
+
[RFCv0 7/8] freedreno/ir3: fix const_index handling for uniforms
When coming directly from glsl_to_nir (rather than via TGSI where
information about, for example, mat4's is lost), both const_index
fields will be used (vs. tgsi_to_nir where the 2nd is always zero).
For example:
decl_var uniform INTERP_QUALIFIER_NONE mat4 ModelViewProjectionMatrix (0, 0)
decl_var uniform INTERP_QUALIFIER_NONE mat4 NormalMatrix (4, 4)
...
vec4 ssa_54 = intrinsic load_uniform () () (4, 0) /* NormalMatrix
*/
vec4 ssa_56 = intrinsic load_uniform () () (4, 1) /* NormalMatrix
*/
vs:
decl_var uniform INTERP_QUALIFIER_NONE vec4[8] uniform_0 (0, 0)
...
vec4 ssa_54 = intrinsic load_uniform () () (4, 0)
vec4 ssa_56 = intrinsic load_uniform () () (5, 0)
---
src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
index 5a53e32..2b9a841 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
@@ -1377,7 +1377,10 @@ emit_intrinisic(struct ir3_compile *ctx,
nir_intrinsic_instr *intr)
const nir_intrinsic_info *info = &nir_intrinsic_infos[intr->intrinsic];
struct ir3_instruction **dst, **src;
struct ir3_block *b = ctx->block;
- unsigned idx = intr->const_index[0];
+ unsigned idx = 0;
+
+ for (unsigned i = 0; i <
nir_intrinsic_infos[intr->intrinsic].num_indices; i++)
+ idx += intr->const_index[i];
if (info->has_dest) {
dst = get_dst(ctx, &intr->dest, intr->num_components);
--
2.5.0
[RFCv0 8/8] freedreno/ir3: hack for mismatch on varying slots for glsl->tgsi->nir vs glsl->nir
Complete hack just for debugging. It isn't intended that mixing and
matching glsl->tgsi->nir vs glsl->nir between shader stages should
actually work.
---
src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
index 2b9a841..317ed79 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
@@ -2232,6 +2232,12 @@ setup_output(struct ir3_compile *ctx, nir_variable *out)
unsigned slot = out->data.location;
unsigned comp = 0;
+ // XXX temporary hack for NIR vs and TGSI fs.. since there is
+ // some disagreement about varying slots..
+ if (ctx->so->shader->nir)
+ if (slot >= VARYING_SLOT_VAR0)
+ slot += 9;
+
DBG("; out: slot=%u, len=%ux%u, drvloc=%u",
slot, array_len, ncomp, n);
--
2.5.0
[RFCv0 4/8] mesa/st: add support for NIR as possible driver IR
---
src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 223 -
src/mesa/state_tracker/st_program.c| 43 +-
2 files changed, 260 insertions(+), 6 deletions(-)
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 06f510d..49f496f 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -35,6 +35,9 @@
#include "glsl_parser_extras.h"
#include "ir_optimization.h"
+#include "nir.h"
+#include "glsl_to_nir.h"
+
#include "main/errors.h"
#include "main/shaderobj.h"
#include "main/uniforms.h"
@@ -5486,9 +5489,9 @@ out:
* generating Mesa IR.
*/
static struct gl_program *
-get_mesa_program(struct gl_context *ctx,
- struct gl_shader_program *shader_program,
- struct gl_shader *shader)
+get_mesa_program_tgsi(struct gl_context *ctx,
+ struct gl_shader_program *shader_program,
+ struct gl_shader *shader)
{
glsl_to_tgsi_visitor* v;
struct gl_program *prog;
@@ -5680,6 +5683,220 @@ get_mesa_program(struct gl_context *ctx,
return prog;
}
+/* TODO dup'd from brw_vec4_vistor.cpp.. what should we do? */
+static int
+type_size_vec4(const struct glsl_type *type)
+{
+ unsigned int i;
+ int size;
+
+ switch (type->base_type) {
+ case GLSL_TYPE_UINT:
+ case GLSL_TYPE_INT:
+ case GLSL_TYPE_FLOAT:
+ case GLSL_TYPE_BOOL:
+ if (type->is_matrix()) {
+return type->matrix_columns;
+ } else {
+/* Regardless of size of vector, it gets a vec4. This is bad
+ * packing for things like floats, but otherwise arrays become a
+ * mess. Hopefully a later pass over the code can pack scalars
+ * down if appropriate.
+ */
+return 1;
+ }
+ case GLSL_TYPE_ARRAY:
+ assert(type->length > 0);
+ return type_size_vec4(type->fields.array) * type->length;
+ case GLSL_TYPE_STRUCT:
+ size = 0;
+ for (i = 0; i < type->length; i++) {
+size += type_size_vec4(type->fields.structure[i].type);
+ }
+ return size;
+ case GLSL_TYPE_SUBROUTINE:
+ return 1;
+
+ case GLSL_TYPE_SAMPLER:
+ /* Samplers take up no register space, since they're baked in at
+ * link time.
+ */
+ return 0;
+ case GLSL_TYPE_ATOMIC_UINT:
+ return 0;
+ case GLSL_TYPE_IMAGE:
+// return DIV_ROUND_UP(BRW_IMAGE_PARAM_SIZE, 4);
+ case GLSL_TYPE_VOID:
+ case GLSL_TYPE_DOUBLE:
+ case GLSL_TYPE_ERROR:
+ case GLSL_TYPE_INTERFACE:
+ unreachable("not reached");
+ }
+
+ return 0;
+}
+
+static struct gl_program *
+get_mesa_program_nir(struct gl_context *ctx,
+ struct gl_shader_program *shader_program,
+ struct gl_shader *shader)
+{
+ struct gl_program *prog;
+ GLenum target = _mesa_shader_stage_to_program(shader->Stage);
+ struct gl_shader_compiler_options *options =
+
&ctx->Const.ShaderCompilerOptions[_mesa_shader_enum_to_shader_stage(shader->Type)];
+ struct pipe_screen *pscreen = ctx->st->pipe->screen;
+ unsigned ptarget = st_shader_stage_to_ptarget(shader->Stage);
+ nir_shader_compiler_options nir_options = {0};
+ nir_shader *nir;
+
+ /* TODO maybe options marked with 'XXX' should be PIPE_SHADER_CAP_x's */
+ nir_options.lower_ffma =
+ !pscreen->get_shader_param(pscreen, ptarget,
PIPE_SHADER_CAP_TGSI_FMA_SUPPORTED);
+ nir_options.lower_flrp = true; // XXX
+ nir_options.lower_fpow = options->EmitNoPow;
+ nir_options.lower_fsat = options->EmitNoSat;
+ nir_options.lower_fsqrt =
+ !pscreen->get_shader_param(pscreen, ptarget,
PIPE_SHADER_CAP_TGSI_SQRT_SUPPORTED);
+ nir_options.lower_negate = false; // XXX
+ nir_options.lower_sub = false; // XXX
+ nir_options.lower_scmp = true; // XXX
+ nir_options.fdot_replicates = false;// XXX
+ nir_options.lower_ffract = true;// XXX
+ nir_options.native_integers = ctx->Const.NativeIntegers;
+
+ validate_ir_tree(shader->ir);
+
+ prog = ctx->Driver.NewProgram(ctx, target, shader_program->Name);
+ if (!prog)
+ return NULL;
+ prog->Parameters = _mesa_new_parameter_list();
+
+ _mesa_copy_linked_program_data(shader->Stage, shader_program, prog);
+ _mesa_generate_parameters_list_for_uniforms(shader_program, shader,
+ prog->Parameters);
+
+// /* Remove reads from output registers. */
+// lower_output_reads(shader->Stage, shader->ir);
+
+ /* Make a pass over the IR to add state references for any built-in
+* uniforms that are used. This has to be done now (during linking).
+* Code generation doesn't happen until the first time this shader is
+* used for rendering. Waiting until then to generate the parameters is
+* too late. At that point, the values for the built-in uniforms won't
+* get sent to the shader.
+*/
+ foreach_in_list(ir_instruction, node, s
[RFCv0 5/8] freedreno/ir3: drop tokens arg
We can get the tokens from variant->shader so drop the extra arg. This
will make things easier to support either getting TGSI or NIR as input.
---
src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
index 8c9234b..b4e5483 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
@@ -126,8 +126,8 @@ struct ir3_compile {
static struct ir3_instruction * create_immed(struct ir3_block *block, uint32_t
val);
static struct ir3_block * get_block(struct ir3_compile *ctx, nir_block
*nblock);
-static struct nir_shader *to_nir(struct ir3_compile *ctx,
- const struct tgsi_token *tokens, struct ir3_shader_variant *so)
+static struct nir_shader *
+to_nir(struct ir3_compile *ctx, struct ir3_shader_variant *so)
{
static const nir_shader_compiler_options options = {
.lower_fpow = true,
@@ -164,7 +164,7 @@ static struct nir_shader *to_nir(struct ir3_compile *ctx,
tex_options.lower_txp = (1 << GLSL_SAMPLER_DIM_3D);
}
- struct nir_shader *s = tgsi_to_nir(tokens, &options);
+ struct nir_shader *s = tgsi_to_nir(so->shader->tokens, &options);
if (fd_mesa_debug & FD_DBG_OPTMSGS) {
debug_printf("--\n");
@@ -214,9 +214,7 @@ static struct nir_shader *to_nir(struct ir3_compile *ctx,
}
static struct ir3_compile *
-compile_init(struct ir3_compiler *compiler,
- struct ir3_shader_variant *so,
- const struct tgsi_token *tokens)
+compile_init(struct ir3_compiler *compiler, struct ir3_shader_variant *so)
{
struct ir3_compile *ctx = rzalloc(NULL, struct ir3_compile);
@@ -245,7 +243,7 @@ compile_init(struct ir3_compiler *compiler,
ctx->block_ht = _mesa_hash_table_create(ctx,
_mesa_hash_pointer, _mesa_key_pointer_equal);
- ctx->s = to_nir(ctx, tokens, so);
+ ctx->s = to_nir(ctx, so);
so->first_driver_param = so->first_immediate = ctx->s->num_uniforms;
@@ -2432,7 +2430,7 @@ ir3_compile_shader_nir(struct ir3_compiler *compiler,
assert(!so->ir);
- ctx = compile_init(compiler, so, so->shader->tokens);
+ ctx = compile_init(compiler, so);
if (!ctx) {
DBG("INIT failed!");
ret = -1;
--
2.5.0
[RFCv0 0/8] gallium: add support for NIR as alternate IR
On Mon, Oct 19, 2015 at 3:47 PM, Rob Clark wrote: > Also, there is some trivial shader variant handling in mesa st which > would have to be ported to NIR. Or, perhaps, just somehow expose the > shader key to the driver. (Currently most drivers are doing much more > variant handling within the driver, and having two levels of variants > does seem a bit silly.) With Marek's latest series, the are no more variants in st/mesa (well, except for glDrawPixels/glBitmap-related shaders) provided that your driver exposes the right caps. -ilia
[Bug 106291] New: amdgpu fails GPU reset when resuming from suspend
https://bugzilla.kernel.org/show_bug.cgi?id=106291 Bug ID: 106291 Summary: amdgpu fails GPU reset when resuming from suspend Product: Drivers Version: 2.5 Kernel Version: 4.2.3 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri at kernel-bugs.osdl.org Reporter: universaledge97 at gmail.com Regression: No Created attachment 190561 --> https://bugzilla.kernel.org/attachment.cgi?id=190561&action=edit output of "dmesg | grep amdgpu" Rarely, when resuming from suspend, amdgpu will fail to reset the GPU, causing X.org to crash. This does have any particular cause I can identify. Any variety of programs could be open when this error occurs. Attached is a log of the output of "dmesg | grep amdgpu", although I have the full ouput from dmesg if needed. Running XFX R9 285 Tonga -- You are receiving this mail because: You are watching the assignee of the bug.
HDMI codec, way forward?
On 10/18/15 18:02, Vinod Koul wrote: > Jyri's approach to add generic IEC code makes sense and drives reuse. I kind > of didn't like adding rates and formats for DAIs, if they are placeholders > then it is okay but otherwise we should read and set them from ELD. Also I The idea is to provide all possible formats and rates to the ASoC core at the DAI registration phase. Then at the stream startup the currently valid ELD is scanned and and ALSA constraints are set according to it (with snd_pcm_hw_constraint_eld()). > have reservations about hdmi_codec_ops and this being set by drm driver, why > not use component interface for these things and let data be shared! Russell already listed why the component interface is not good for registering optional subsystems. Any particular reason why passing function pointers over pdata should be considered bad? Can you suggest an alternative? Best regards, Jyri
[regression] [git pull] drm for 4.3
On Wed, Sep 30, 2015 at 08:56:26AM +0200, Daniel Vetter wrote:
> > The warning on boot seems to be gone as of rc3, but I can now trigger this
> > pretty easily..
>
> http://patchwork.freedesktop.org/patch/60618/
Back from several weeks of travel.. I tried again with rc6, and
I'm still seeing the same traces.
Dave
> > WARNING: CPU: 2 PID: 28911 at drivers/gpu/drm/drm_atomic.c:889
> > drm_atomic_get_property+0x244/0x2d0()
> > CPU: 2 PID: 28911 Comm: trinity-c313 Not tainted 4.3.0-rc3-think+ #14
> > 0379 8801a1377c88 8e35d5ec
> > 8801a1377cc0 8e07a862 880500b392b8 880500a13008
> > 880500b39290 8804fe3806d8 88003fa45668 8801a1377cd0
> > Call Trace:
> > [] dump_stack+0x4e/0x82
> > [] warn_slowpath_common+0x82/0xc0
> > [] warn_slowpath_null+0x1a/0x20
> > [] drm_atomic_get_property+0x244/0x2d0
> > [] drm_object_property_get_value+0x6c/0x70
> > [] dpms_show+0x2f/0x70
> > [] dev_attr_show+0x20/0x50
> > [] ? sysfs_file_ops+0x41/0x60
> > [] sysfs_kf_seq_show+0xb7/0x110
> > [] kernfs_seq_show+0x26/0x30
> > [] seq_read+0xe6/0x430
> > [] kernfs_fop_read+0x127/0x170
> > [] ? mutex_lock_nested+0x26b/0x3f0
> > [] __vfs_read+0x28/0xe0
> > [] ? mutex_lock_nested+0x287/0x3f0
> > [] ? __fdget_pos+0x49/0x50
> > [] ? __fdget_pos+0x49/0x50
> > [] vfs_read+0x86/0x130
> > [] SyS_read+0x49/0xb0
> > [] entry_SYSCALL_64_fastpath+0x12/0x6f
> > ---[ end trace e053063c697a1355 ]---
> >
> > 887 case DRM_MODE_OBJECT_CONNECTOR: {
> > 888 struct drm_connector *connector =
> > obj_to_connector(obj);
> > 889
> > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> > 890 ret = drm_atomic_connector_get_property(connector,
> > 891 connector->state, property, val);
> > 892 break;
> > 893 }
> >
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
---end quoted text---
