[Bug 98513] [REGRESSION][drm-next-4.10-wip][AMDGPU][CIK] Unable to suspend, hangs on hibernation bad commit 86f8c599b09c916f9aad30563271440dbd79213a
https://bugs.freedesktop.org/show_bug.cgi?id=98513 Shawn Starr changed: What|Removed |Added Summary|[REGRESSION][4.9-rc3 w/ |[REGRESSION][drm-next-4.10- |drm-next-4.10-wip][AMDGPU][ |wip][AMDGPU][CIK] Unable to |CIK] Unable to suspend, |suspend, hangs on |hangs on hibernation|hibernation bad commit ||86f8c599b09c916f9aad3056327 ||1440dbd79213a -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/873fa136/attachment.html>
[Bug 98513] [REGRESSION][4.9-rc3 w/ drm-next-4.10-wip][AMDGPU][CIK] Unable to suspend, hangs on hibernation
https://bugs.freedesktop.org/show_bug.cgi?id=98513 --- Comment #2 from Shawn Starr --- Bisected results confirmed: [spstarr at segfault linux]$ git bisect bad 86f8c599b09c916f9aad30563271440dbd79213a is the first bad commit commit 86f8c599b09c916f9aad30563271440dbd79213a Author: Rex Zhu Date: Mon Oct 3 20:46:36 2016 +0800 drm/amdgpu: when suspend, set boot state instand of disable dpm. fix pm-hibernate bug, when suspend/resume, dpm start failed. Signed-off-by: Rex Zhu Acked-by: Christian König Reviewed-by: Alex Deucher Signed-off-by: Alex Deucher :04 04 5537c5937b203abdb5c48c5a0dc8dc5f6a3d00f2 7f048011c861d8c35b922fd94f089cd07d670f60 M drivers [spstarr at segfault linux]$ git bisect log git bisect start # good: [1001354ca34179f3db924eb66672442a173147dc] Linux 4.9-rc1 git bisect good 1001354ca34179f3db924eb66672442a173147dc # bad: [2f5945e707b5dffbf12444ad8bea079626ad30ec] drm/amdgpu: add the interface of waiting multiple fences (v4) git bisect bad 2f5945e707b5dffbf12444ad8bea079626ad30ec # good: [61d0a04d6f5b2122f88aacbc4b1716e571961660] Merge tag 'topic/drm-misc-2016-10-24' of git://anongit.freedesktop.org/drm-intel into drm-next git bisect good 61d0a04d6f5b2122f88aacbc4b1716e571961660 # good: [e4ab73a13291fc844c9e24d5c347bd95818544d2] drm/i915: Respect alternate_ddc_pin for all DDI ports git bisect good e4ab73a13291fc844c9e24d5c347bd95818544d2 # bad: [415282b15e15c2d7fb18e29c5b554cc7f4ff5c52] drm/amdgpu: disable dpm before turn off clock when vce idle. git bisect bad 415282b15e15c2d7fb18e29c5b554cc7f4ff5c52 # good: [392f0c775c80de0eae4c07227cc220015df70abc] drm/amdgpu/gfx8: cache rb config values git bisect good 392f0c775c80de0eae4c07227cc220015df70abc # good: [472259f02657ef99cba2a64832ccadad8e3baabe] drm/amd/amdgpu: re-factor debugfs wave reader git bisect good 472259f02657ef99cba2a64832ccadad8e3baabe # good: [f1e68a7cf582b41d6da1dd15b9f0bfb9057c1164] drm/amdgpu/atom: remove a bunch of unused functions git bisect good f1e68a7cf582b41d6da1dd15b9f0bfb9057c1164 # good: [585ffd65441a4aea7e762d17f7a248d07cd1c9ac] drm/ttm: fix coding style in ttm_bo_driver.h git bisect good 585ffd65441a4aea7e762d17f7a248d07cd1c9ac # good: [8ed8147abc7cf1f689245deb316aabfe2f657ade] drm/amdgpu: use failed label to handle context init failure git bisect good 8ed8147abc7cf1f689245deb316aabfe2f657ade # bad: [3f767e3d076dd2a24a614917c8f0b05d8d82b90b] drm/amdgpu: just not load smc firmware if smu is already running git bisect bad 3f767e3d076dd2a24a614917c8f0b05d8d82b90b # bad: [86f8c599b09c916f9aad30563271440dbd79213a] drm/amdgpu: when suspend, set boot state instand of disable dpm. git bisect bad 86f8c599b09c916f9aad30563271440dbd79213a # first bad commit: [86f8c599b09c916f9aad30563271440dbd79213a] drm/amdgpu: when suspend, set boot state instand of disable dpm. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/d15beebc/attachment.html>
[drm-intel:topic/drm-misc 1/14] warning: (KMEMCHECK && ..) selects STACKTRACE which has unmet direct dependencies (STACKTRACE_SUPPORT)
tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc head: 93ce75fa3dba8781c5c042bd7a61d438662ed73c commit: 5705670d0463423337c82d00877989e7df8b169d [1/14] drm: Track drm_mm allocators and show leaks on shutdown config: alpha-allmodconfig (attached as .config) compiler: alpha-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 5705670d0463423337c82d00877989e7df8b169d # save the attached .config to linux build tree make.cross ARCH=alpha All warnings (new ones prefixed by >>): warning: (KMEMCHECK && STACK_TRACER && BLK_DEV_IO_TRACE && STACKDEPOT) selects STACKTRACE which has unmet direct dependencies (STACKTRACE_SUPPORT) --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- next part -- A non-text attachment was scrubbed... Name: .config.gz Type: application/gzip Size: 47637 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/2a5ff0b1/attachment-0001.gz>
BUG: 'list_empty(&vgdev->free_vbufs)' is true!
On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
> Hi,
>
> I can relatively easily reproduce this bug:
> BUG: 'list_empty(&vgdev->free_vbufs)' is true!
> [ cut here ]
> kernel BUG at /home/latest/linux/drivers/gpu/drm/virtio/virtgpu_vq.c:130!
> invalid opcode: [#1] PREEMPT SMP KASAN
> Modules linked in:
> CPU: 1 PID: 355 Comm: kworker/1:2 Not tainted 4.9.0-rc2-next-20161028+ #32
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
> Workqueue: events drm_fb_helper_dirty_work
> task: 88007b124980 task.stack: 88007b8a
> RIP: 0010:virtio_gpu_get_vbuf+0x32e/0x630
> RSP: 0018:88007b8a78c0 EFLAGS: 00010286
> RAX: 002e RBX: 11000f714f1d RCX:
> RDX: 002e RSI: 0001 RDI: ed000f714f0e
> RBP: 88007b8a7970 R08: 0001 R09:
> R10: 0002 R11: 0001 R12: 0030
> R13: 88007caeaba8 R14: 0018 R15: 88007cae
> FS: () GS:88007dc8() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: 00601028 CR3: 7740d000 CR4: 06e0
> Call Trace:
> Code: df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 bb 01 00 00 4c 89 69 e8
> eb 9e 48 c7 c6 e0 d2 d1 83 48 c7 c7 20 d3 d1 83 e8 6c fb 04 ff <0f> 0b
> 48 c7 c7 a0 fb b0 85 e8 09 95 86 ff 48 c7 c6 c0 d3 d1 83
> RIP: virtio_gpu_get_vbuf+0x32e/0x630 RSP: 88007b8a78c0
>
>
> There is no stacktrace, as the kernel starts panicing all over the place
> during its generation. Any ideas?
>
> thanks,
CC maintainers.
The following might be helpful for debugging - if kernel still will
not stop panicing, we are looking at some kind
of memory corruption.
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a0f8a7..d5e1e72 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -127,7 +127,11 @@ virtio_gpu_get_vbuf(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf;
spin_lock(&vgdev->free_vbufs_lock);
- BUG_ON(list_empty(&vgdev->free_vbufs));
+ WARN_ON(list_empty(&vgdev->free_vbufs));
+ if (list_empty(&vgdev->free_vbufs)) {
+ spin_unlock(&vgdev->free_vbufs_lock);
+ return ERR_PTR(-EINVAL);
+ }
vbuf = list_first_entry(&vgdev->free_vbufs,
struct virtio_gpu_vbuffer, list);
list_del(&vbuf->list);
[PATCH] drm/sun4i: Fix error handling
etely wrong.
> What I would have expected would be something like: (un-tested, just to give
> an idea)
>
>
> ==8<
>
> @@ -133,9 +133,9 @@ struct sun4i_layer **sun4i_layers_init(struct drm_device
> *drm)
> struct sun4i_layer **layers;
> int i;
>
> layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> - sizeof(**layers), GFP_KERNEL);
> + sizeof(*layers), GFP_KERNEL);
> if (!layers)
> return ERR_PTR(-ENOMEM);
>
> /*
> @@ -160,16 +160,17 @@ struct sun4i_layer **sun4i_layers_init(struct
> drm_device *drm)
> * layers.
> */
> for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> -struct sun4i_layer *layer = layers[i];
> +struct sun4i_layer *layer;
>
> layer = sun4i_layer_init_one(drm, plane);
> if (IS_ERR(layer)) {
> dev_err(drm->dev, "Couldn't initialize %s plane\n",
> i ? "overlay" : "primary");
> return ERR_CAST(layer);
> };
> +layers[i] = layer;
>
> DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> i ? "overlay" : "primary", plane->pipe);
> regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
You're totally right. Can you send this as a formal patch?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/2ed78f33/attachment-0001.sig>
[PATCH v2 0/5] ARM: da850: new drivers for better LCDC support
+ Arnd, Olof On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote: > This series adds two new drivers in order to better support the LCDC > rev1 present on the da850 boards. > > The first patch adds a new memory driver which allows to write to the > DDR2/mDDR memory controller present on the da8xx SoCs. > > The second patch adds a new bus driver which allows to interact with > the MSTPRI registers of the SYSCFG0 module I think patches 1/5 and 2/5 are ready to be merged. If there are no objections I would like to send a pull request for them to be merged through ARM-SoC tree for v4.10 kernel. Thanks, Sekhar > > As is mentioned in the comments: we don't want to commit to supporting > stable interfaces (DT bindings or sysfs attributes) so we hardcode the > settings required by some boards (for now only da850-lcdk) with the > hope that linux gets an appropriate framework for performance knobs > in the future. > > Potential extensions of these drivers should be straightforward in the > future. > > Subsequent patches add DT nodes for the new drivers: disabled nodes > in da850.dtsi and enabled in da850-lcdk.dts. > > The last patch adds a workaround for current lack of support for drm > bridges in tilcdc. > > Tested on a da850-lcdk with a display connected over VGA and two > additional patches for tilcdc (sent to linux-drm): ran simple modetest > for supported resolutions, used X.org and fluxbox as graphical > environment, played video with mplayer. > > v1 -> v2: > - used regular readl()/writel() instead of __raw_** versions > - used resource_size() instead of calculating the size by hand > - used ioremap instead of syscon in patch [2/5] > - added the DT nodes in patches [3/5]-[5/5] > > Bartosz Golaszewski (5): > ARM: memory: da8xx-ddrctl: new driver > ARM: bus: da8xx-mstpri: new driver > ARM: dts: da850: add the mstpri and ddrctl nodes > ARM: dts: da850-lcdk: enable mstpri and ddrctl nodes > ARM: dts: da850-lcdk: add tilcdc panel node > > .../devicetree/bindings/bus/ti,da850-mstpri.txt| 20 ++ > .../memory-controllers/ti-da8xx-ddrctl.txt | 20 ++ > arch/arm/boot/dts/da850-lcdk.dts | 71 ++ > arch/arm/boot/dts/da850.dtsi | 11 + > drivers/bus/Kconfig| 9 + > drivers/bus/Makefile | 2 + > drivers/bus/da8xx-mstpri.c | 269 > + > drivers/memory/Kconfig | 8 + > drivers/memory/Makefile| 1 + > drivers/memory/da8xx-ddrctl.c | 175 ++ > 10 files changed, 586 insertions(+) > create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt > create mode 100644 > Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt > create mode 100644 drivers/bus/da8xx-mstpri.c > create mode 100644 drivers/memory/da8xx-ddrctl.c >
[PATCH 4/4] drm/tegra: Set sgt pointer in BO pin
Fix tegra_bo_pin to set the parameter sgt pointer.
Host1x job pinning requires the sgt to determine
physical memory addresses of gathers.
Signed-off-by: Mikko Perttunen
---
drivers/gpu/drm/tegra/gem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 2508372..c08e527 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -36,6 +36,8 @@ static dma_addr_t tegra_bo_pin(struct host1x_bo *bo, struct
sg_table **sgt)
{
struct tegra_bo *obj = host1x_to_tegra_bo(bo);
+ *sgt = obj->sgt;
+
return obj->paddr;
}
--
2.9.3
[PATCH 3/4] drm/tegra: Support kernel mappings with IOMMU
From: Arto Merilainen
Host1x command buffer patching requires that the buffer object can be
mapped into kernel address space, however, the recent addition of
IOMMU did not account to this requirement. Therefore Host1x engines
cannot be used if IOMMU is enabled.
This patch implements kmap, kunmap, mmap and munmap functions to
host1x bo objects.
Signed-off-by: Arto Merilainen
Signed-off-by: Mikko Perttunen
---
drivers/gpu/drm/tegra/gem.c | 34 +++---
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 19bf9cd..2508372 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -2,7 +2,7 @@
* NVIDIA Tegra DRM GEM helper functions
*
* Copyright (C) 2012 Sascha Hauer, Pengutronix
- * Copyright (C) 2013 NVIDIA CORPORATION, All rights reserved.
+ * Copyright (C) 2013-2015 NVIDIA CORPORATION, All rights reserved.
*
* Based on the GEM/CMA helpers
*
@@ -47,23 +47,51 @@ static void *tegra_bo_mmap(struct host1x_bo *bo)
{
struct tegra_bo *obj = host1x_to_tegra_bo(bo);
- return obj->vaddr;
+ if (obj->vaddr)
+ return obj->vaddr;
+ else if (obj->gem.import_attach)
+ return dma_buf_vmap(obj->gem.import_attach->dmabuf);
+ else
+ return vmap(obj->pages, obj->num_pages, VM_MAP,
+ pgprot_writecombine(PAGE_KERNEL));
}
static void tegra_bo_munmap(struct host1x_bo *bo, void *addr)
{
+ struct tegra_bo *obj = host1x_to_tegra_bo(bo);
+
+ if (obj->vaddr)
+ return;
+ else if (obj->gem.import_attach)
+ dma_buf_vunmap(obj->gem.import_attach->dmabuf, addr);
+ else
+ vunmap(addr);
}
static void *tegra_bo_kmap(struct host1x_bo *bo, unsigned int page)
{
struct tegra_bo *obj = host1x_to_tegra_bo(bo);
- return obj->vaddr + page * PAGE_SIZE;
+ if (obj->vaddr)
+ return obj->vaddr + page * PAGE_SIZE;
+ else if (obj->gem.import_attach)
+ return dma_buf_kmap(obj->gem.import_attach->dmabuf, page);
+ else
+ return vmap(obj->pages + page, 1, VM_MAP,
+ pgprot_writecombine(PAGE_KERNEL));
}
static void tegra_bo_kunmap(struct host1x_bo *bo, unsigned int page,
void *addr)
{
+ struct tegra_bo *obj = host1x_to_tegra_bo(bo);
+
+ if (obj->vaddr)
+ return;
+ else if (obj->gem.import_attach)
+ dma_buf_kunmap(obj->gem.import_attach->dmabuf, page, addr);
+ else
+ vunmap(addr);
}
static struct host1x_bo *tegra_bo_get(struct host1x_bo *bo)
--
2.9.3
[PATCH 2/4] gpu: host1x: Add locking to syncpt
From: Arto Merilainen
Currently syncpoints are not locked by mutex and this causes races
if we are aggressively freeing and allocating syncpoints.
This patch adds missing mutex protection to syncpoint structures.
Signed-off-by: Arto Merilainen
Reviewed-by: Shridhar Rasal
Signed-off-by: Mikko Perttunen
---
drivers/gpu/host1x/dev.h| 3 ++-
drivers/gpu/host1x/syncpt.c | 25 +
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
index 5220510..06dd4f8 100644
--- a/drivers/gpu/host1x/dev.h
+++ b/drivers/gpu/host1x/dev.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012-2013, NVIDIA Corporation.
+ * Copyright (c) 2012-2015, NVIDIA Corporation.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -120,6 +120,7 @@ struct host1x {
struct host1x_syncpt *nop_sp;
+ struct mutex syncpt_mutex;
struct mutex chlist_mutex;
struct host1x_channel chlist;
unsigned long allocated_channels;
diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c
index 9558932..f3b04ed 100644
--- a/drivers/gpu/host1x/syncpt.c
+++ b/drivers/gpu/host1x/syncpt.c
@@ -1,7 +1,7 @@
/*
* Tegra host1x Syncpoints
*
- * Copyright (c) 2010-2013, NVIDIA Corporation.
+ * Copyright (c) 2010-2015, NVIDIA Corporation.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -61,22 +61,24 @@ static struct host1x_syncpt *host1x_syncpt_alloc(struct
host1x *host,
struct host1x_syncpt *sp = host->syncpt;
char *name;
+ mutex_lock(&host->syncpt_mutex);
+
for (i = 0; i < host->info->nb_pts && sp->name; i++, sp++)
;
if (i >= host->info->nb_pts)
- return NULL;
+ goto err_alloc_syncpt;
if (flags & HOST1X_SYNCPT_HAS_BASE) {
sp->base = host1x_syncpt_base_request(host);
if (!sp->base)
- return NULL;
+ goto err_alloc_base;
}
name = kasprintf(GFP_KERNEL, "%02u-%s", sp->id,
dev ? dev_name(dev) : NULL);
if (!name)
- return NULL;
+ goto err_alloc_name;
sp->dev = dev;
sp->name = name;
@@ -86,7 +88,17 @@ static struct host1x_syncpt *host1x_syncpt_alloc(struct
host1x *host,
else
sp->client_managed = false;
+ mutex_unlock(&host->syncpt_mutex);
return sp;
+
+err_alloc_name:
+ host1x_syncpt_base_free(sp->base);
+ sp->base = NULL;
+err_alloc_base:
+ sp = NULL;
+err_alloc_syncpt:
+ mutex_unlock(&host->syncpt_mutex);
+ return NULL;
}
u32 host1x_syncpt_id(struct host1x_syncpt *sp)
@@ -378,6 +390,7 @@ int host1x_syncpt_init(struct host1x *host)
for (i = 0; i < host->info->nb_bases; i++)
bases[i].id = i;
+ mutex_init(&host->syncpt_mutex);
host->syncpt = syncpt;
host->bases = bases;
@@ -405,12 +418,16 @@ void host1x_syncpt_free(struct host1x_syncpt *sp)
if (!sp)
return;
+ mutex_lock(&sp->host->syncpt_mutex);
+
host1x_syncpt_base_free(sp->base);
kfree(sp->name);
sp->base = NULL;
sp->dev = NULL;
sp->name = NULL;
sp->client_managed = false;
+
+ mutex_unlock(&sp->host->syncpt_mutex);
}
EXPORT_SYMBOL(host1x_syncpt_free);
--
2.9.3
[PATCH 1/4] gpu: host1x: Store device address to all bufs
From: Arto Merilainen
Currently job pinning is optimized to handle only the first buffer
using a certain host1x_bo object and all subsequent buffers using
the same host1x_bo are considered done.
In most cases this is correct, however, in case the same host1x_bo
is used in multiple gathers inside the same job, we skip also
storing the device address (physical or iova) to this buffer.
This patch reworks the host1x_job_pin() to store the device address
to all gathers.
Signed-off-by: Andrew Chew
Signed-off-by: Arto Merilainen
Signed-off-by: Mikko Perttunen
---
drivers/gpu/host1x/job.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index a91b7c4..92c3df9 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -1,7 +1,7 @@
/*
* Tegra host1x Job
*
- * Copyright (c) 2010-2013, NVIDIA Corporation.
+ * Copyright (c) 2010-2015, NVIDIA Corporation.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -539,9 +539,12 @@ int host1x_job_pin(struct host1x_job *job, struct device
*dev)
g->base = job->gather_addr_phys[i];
- for (j = i + 1; j < job->num_gathers; j++)
- if (job->gathers[j].bo == g->bo)
+ for (j = i + 1; j < job->num_gathers; j++) {
+ if (job->gathers[j].bo == g->bo) {
job->gathers[j].handled = true;
+ job->gathers[j].base = g->base;
+ }
+ }
err = do_relocs(job, g->bo);
if (err)
--
2.9.3
[PATCH 1/2] drm/imx: disable planes before DC
Am Dienstag, den 08.11.2016, 17:04 +0100 schrieb Lucas Stach: > If the DC clock is disabled before the attached IDMACs are properly > stopped the IDMACs may hang the IPU or even the whole system. > > Make sure the IDMACs are in safe state by disabling the planes before > removal of the DC clock. This patch not only moves disable_planes into the right place, it also changes the atomic parameter to false to avoid calling atomic_begin. Since ipu_crtc_atomic_begin only calls drm_crtc_vblank_on and possibly drm_crtc_arm_vblank_event, but we disable the IDMAC channels that could be the source for the vblank interrupts immediately afterwards, I think this is the right thing to do. I'll amend the commit message as follows: "Also set the atomic parameter to false to stop calling the atomic_begin hook, which does nothing useful as we immediately afterwards turn off vblank interrupts and possibly send the pending vblank event." > Fixes: 33f14235302f (drm/imx: atomic phase 1: Use transitional atomic > CRTC and plane helpers) > Signed-off-by: Lucas Stach > --- > drivers/gpu/drm/imx/ipuv3-crtc.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c > b/drivers/gpu/drm/imx/ipuv3-crtc.c > index 4e1ae3fc462d..6be515a9fb69 100644 > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c > @@ -68,6 +68,12 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc, > > ipu_dc_disable_channel(ipu_crtc->dc); > ipu_di_disable(ipu_crtc->di); > + /* > + * Planes must be disabled before DC clock is removed, as otherwise the > + * attached IDMACs will be left in undefined state, possibly hanging > + * the IPU or even system. > + */ > + drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, false); > ipu_dc_disable(ipu); > > spin_lock_irq(&crtc->dev->event_lock); > @@ -77,9 +83,6 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc, > } > spin_unlock_irq(&crtc->dev->event_lock); > > - /* always disable planes on the CRTC */ > - drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, true); > - > drm_crtc_vblank_off(crtc); > } > regards Philipp
[drm-intel:topic/drm-misc 1/13] ERROR: "depot_save_stack" [drivers/gpu/drm/drm.ko] undefined!
tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc head: 3835b46e5535d9ad534776bc93670db097682556 commit: 5705670d0463423337c82d00877989e7df8b169d [1/13] drm: Track drm_mm allocators and show leaks on shutdown config: i386-randconfig-i1-201645 (attached as .config) compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4 reproduce: git checkout 5705670d0463423337c82d00877989e7df8b169d # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): >> ERROR: "depot_save_stack" [drivers/gpu/drm/drm.ko] undefined! >> ERROR: "depot_fetch_stack" [drivers/gpu/drm/drm.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- next part -- A non-text attachment was scrubbed... Name: .config.gz Type: application/gzip Size: 33705 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/953192f4/attachment-0001.gz>
[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info
On 8 November 2016 at 18:36, Mauro Santos wrote:
> On 08-11-2016 18:08, Mauro Santos wrote:
>> On 08-11-2016 17:13, Emil Velikov wrote:
>>> On 8 November 2016 at 16:57, Mauro Santos
>>> wrote:
On 08-11-2016 15:57, Emil Velikov wrote:
> On 8 November 2016 at 15:27, Mauro Santos
> wrote:
>> On 08-11-2016 15:00, Emil Velikov wrote:
>>> On 8 November 2016 at 13:38, Mauro Santos >> gmail.com> wrote:
On 08-11-2016 11:06, Emil Velikov wrote:
> On 1 November 2016 at 18:47, Mauro Santos gmail.com> wrote:
>> On 01-11-2016 18:13, Emil Velikov wrote:
>>> From: Emil Velikov
>>>
>>> Parsing config sysfs file wakes up the device. The latter of which
>>> may
>>> be slow and isn't required to begin with.
>>>
>>> Reading through config is/was required since the revision is not
>>> available by other means, although with a kernel patch in the way
>>> we can
>>> 'cheat' temporarily.
>>>
>>> That should be fine, since no open-source project has ever used the
>>> value.
>>>
>>> Cc: Michel Dänzer
>>> Cc: Mauro Santos
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
>>> Signed-off-by: Emil Velikov
>>> ---
>>> Mauro can you apply this against libdrm and rebuild it. You do _not_
>>> need to rebuild mesa afterwords.
>>>
>>> Thanks
>>> ---
>>> xf86drm.c | 50 +++---
>>> 1 file changed, 35 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/xf86drm.c b/xf86drm.c
>>> index 52add5e..5a5100c 100644
>>> --- a/xf86drm.c
>>> +++ b/xf86drm.c
>>> @@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char
>>> *d_name,
>>> drmPciDeviceInfoPtr device)
>>> {
>>> #ifdef __linux__
>>> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
>>> +static const char *attrs[] = {
>>> + "revision", /* XXX: make sure it's always first, see note
>>> below */
>>> + "vendor",
>>> + "device",
>>> + "subsystem_vendor",
>>> + "subsystem_device",
>>> +};
>>> char path[PATH_MAX + 1];
>>> -unsigned char config[64];
>>> -int fd, ret;
>>> +unsigned int data[ARRAY_SIZE(attrs)];
>>> +FILE *fp;
>>> +int ret;
>>>
>>> -snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config",
>>> d_name);
>>> -fd = open(path, O_RDONLY);
>>> -if (fd < 0)
>>> -return -errno;
>>> +for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) {
>>> +snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s",
>>> + d_name, attrs[i]);
>>> +fp = fopen(path, "r");
>>> +if (!fp) {
>>> +/* Note: First we check the revision, since older
>>> kernels
>>> + * may not have it. Default to zero in such cases. */
>>> +if (i == 0) {
>>> +data[i] = 0;
>>> +continue;
>>> +}
>>> +return -errno;
>>> +}
>>>
>>> -ret = read(fd, config, sizeof(config));
>>> -close(fd);
>>> -if (ret < 0)
>>> -return -errno;
>>> +ret = fscanf(fp, "%x", &data[i]);
>>> +fclose(fp);
>>> +if (ret != 1)
>>> +return -errno;
>>> +
>>> +}
>>>
>>> -device->vendor_id = config[0] | (config[1] << 8);
>>> -device->device_id = config[2] | (config[3] << 8);
>>> -device->revision_id = config[8];
>>> -device->subvendor_id = config[44] | (config[45] << 8);
>>> -device->subdevice_id = config[46] | (config[47] << 8);
>>> +device->revision_id = data[0] & 0xff;
>>> +device->vendor_id = data[1] & 0x;
>>> +device->device_id = data[2] & 0x;
>>> +device->subvendor_id = data[3] & 0x;
>>> +device->subdevice_id = data[4] & 0x;
>>>
>>> return 0;
>>> #else
>>>
>>
>> I have applied this against libdrm 2.4.71 and I don't see any delays
>> when starting firefox/chromium/thunderbird/glxgears.
>>
>> There is also no indication in dmesg that the dGPU is being
>> reinitialized when starting the programs where I've detected the
>> problem.
>>
> Thanks Mauro. Can you give this a try alongside the kernel fix [1] ?
> I'd love to get the
[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info
On 08-11-2016 18:08, Mauro Santos wrote:
> On 08-11-2016 17:13, Emil Velikov wrote:
>> On 8 November 2016 at 16:57, Mauro Santos
>> wrote:
>>> On 08-11-2016 15:57, Emil Velikov wrote:
On 8 November 2016 at 15:27, Mauro Santos
wrote:
> On 08-11-2016 15:00, Emil Velikov wrote:
>> On 8 November 2016 at 13:38, Mauro Santos > gmail.com> wrote:
>>> On 08-11-2016 11:06, Emil Velikov wrote:
On 1 November 2016 at 18:47, Mauro Santos >>> gmail.com> wrote:
> On 01-11-2016 18:13, Emil Velikov wrote:
>> From: Emil Velikov
>>
>> Parsing config sysfs file wakes up the device. The latter of which
>> may
>> be slow and isn't required to begin with.
>>
>> Reading through config is/was required since the revision is not
>> available by other means, although with a kernel patch in the way we
>> can
>> 'cheat' temporarily.
>>
>> That should be fine, since no open-source project has ever used the
>> value.
>>
>> Cc: Michel Dänzer
>> Cc: Mauro Santos
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
>> Signed-off-by: Emil Velikov
>> ---
>> Mauro can you apply this against libdrm and rebuild it. You do _not_
>> need to rebuild mesa afterwords.
>>
>> Thanks
>> ---
>> xf86drm.c | 50 +++---
>> 1 file changed, 35 insertions(+), 15 deletions(-)
>>
>> diff --git a/xf86drm.c b/xf86drm.c
>> index 52add5e..5a5100c 100644
>> --- a/xf86drm.c
>> +++ b/xf86drm.c
>> @@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char
>> *d_name,
>> drmPciDeviceInfoPtr device)
>> {
>> #ifdef __linux__
>> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
>> +static const char *attrs[] = {
>> + "revision", /* XXX: make sure it's always first, see note
>> below */
>> + "vendor",
>> + "device",
>> + "subsystem_vendor",
>> + "subsystem_device",
>> +};
>> char path[PATH_MAX + 1];
>> -unsigned char config[64];
>> -int fd, ret;
>> +unsigned int data[ARRAY_SIZE(attrs)];
>> +FILE *fp;
>> +int ret;
>>
>> -snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config",
>> d_name);
>> -fd = open(path, O_RDONLY);
>> -if (fd < 0)
>> -return -errno;
>> +for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) {
>> +snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s",
>> + d_name, attrs[i]);
>> +fp = fopen(path, "r");
>> +if (!fp) {
>> +/* Note: First we check the revision, since older
>> kernels
>> + * may not have it. Default to zero in such cases. */
>> +if (i == 0) {
>> +data[i] = 0;
>> +continue;
>> +}
>> +return -errno;
>> +}
>>
>> -ret = read(fd, config, sizeof(config));
>> -close(fd);
>> -if (ret < 0)
>> -return -errno;
>> +ret = fscanf(fp, "%x", &data[i]);
>> +fclose(fp);
>> +if (ret != 1)
>> +return -errno;
>> +
>> +}
>>
>> -device->vendor_id = config[0] | (config[1] << 8);
>> -device->device_id = config[2] | (config[3] << 8);
>> -device->revision_id = config[8];
>> -device->subvendor_id = config[44] | (config[45] << 8);
>> -device->subdevice_id = config[46] | (config[47] << 8);
>> +device->revision_id = data[0] & 0xff;
>> +device->vendor_id = data[1] & 0x;
>> +device->device_id = data[2] & 0x;
>> +device->subvendor_id = data[3] & 0x;
>> +device->subdevice_id = data[4] & 0x;
>>
>> return 0;
>> #else
>>
>
> I have applied this against libdrm 2.4.71 and I don't see any delays
> when starting firefox/chromium/thunderbird/glxgears.
>
> There is also no indication in dmesg that the dGPU is being
> reinitialized when starting the programs where I've detected the
> problem.
>
Thanks Mauro. Can you give this a try alongside the kernel fix [1] ?
I'd love to get the latter merged soon(ish).
Independent of the kernel side, I might need to go another way for
libdrm/mesa so I'll CC you on future patches.
>>
[PATCH RFC 00/12] tda998x updates
On Tue, Nov 08, 2016 at 05:20:36PM +, Jon Medhurst (Tixy) wrote: > On Tue, 2016-11-08 at 13:34 +, Russell King - ARM Linux wrote: > > On Tue, Nov 08, 2016 at 01:32:15PM +, Russell King - ARM Linux wrote: > > > Unfortunately, my drm-tda998x-devel branch is slightly out of date with > > > these patches it's the original set of 10 patches. I've not pushed > > > these ones out to that branch yet, as I've three additional patches on > > > top of these which aren't "ready" for pushing out. > > > > Here's the delta between the branch and what I just posted: > > > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c > > b/drivers/gpu/drm/i2c/tda998x_drv.c > [...] > > I have a working setup for HDMI audio on Juno an would like to test this > series but am struggling to work out which patches to apply in what > order to what branch, can you be specific? (I've tried various > combinations of patches series from the list, drm-tda998x-devel, and the > diff you posted) Hmm, I guess this is going to be annoyingly rather difficult then. The structure of my git tree is: v4.8 mali patch -- merge --- these patches v4.7 -- tda998x audio patches (up to df0bd1e8f3c5) --^ which makes it rather difficult to send out a series that people can apply as patches without first replicating that merge. I guess the answer is... use the _patches_ for review, and I'll push out the changes into drm-tda998x-devel... should be there soon. Look for commit hash d61fa2e50f2a. (Bah, slow 'net connections.) -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[PATCH] drm/tegra: add tiling FB modifiers
On 11/08/2016 06:07 PM, Erik Faye-Lund wrote:
> On Tue, Nov 8, 2016 at 8:50 AM, Alexandre Courbot
> wrote:
>> Add FB modifiers to allow user-space to specify that a surface is in one
>> of the two tiling formats supported by Tegra chips, and add support in
>> the tegradrm driver to handle them properly. This is necessary for the
>> display controller to directly display buffers generated by the GPU.
>>
>> This feature is intended to replace the dedicated IOCTL enabled
>> by TEGRA_STAGING and to provide a non-staging alternative to that
>> solution.
>>
>> Signed-off-by: Alexandre Courbot
>> ---
>> drivers/gpu/drm/tegra/drm.c | 2 ++
>> drivers/gpu/drm/tegra/fb.c| 23 +++---
>> include/uapi/drm/drm_fourcc.h | 45
>> +++
>> 3 files changed, 67 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
>> index a9630c2d6cb3..36b4b30a5164 100644
>> --- a/drivers/gpu/drm/tegra/drm.c
>> +++ b/drivers/gpu/drm/tegra/drm.c
>> @@ -161,6 +161,8 @@ static int tegra_drm_load(struct drm_device *drm,
>> unsigned long flags)
>> drm->mode_config.max_width = 4096;
>> drm->mode_config.max_height = 4096;
>>
>> + drm->mode_config.allow_fb_modifiers = true;
>> +
>> drm->mode_config.funcs = &tegra_drm_mode_funcs;
>>
>> err = tegra_drm_fb_prepare(drm);
>> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
>> index e6d71fa4028e..2fded58b2ca5 100644
>> --- a/drivers/gpu/drm/tegra/fb.c
>> +++ b/drivers/gpu/drm/tegra/fb.c
>> @@ -52,9 +52,26 @@ int tegra_fb_get_tiling(struct drm_framebuffer
>> *framebuffer,
>> struct tegra_bo_tiling *tiling)
>> {
>> struct tegra_fb *fb = to_tegra_fb(framebuffer);
>> -
>> - /* TODO: handle YUV formats? */
>> - *tiling = fb->planes[0]->tiling;
>> + uint64_t modifier = fb->base.modifier[0];
>> +
>> + switch (fourcc_mod_tegra_mod(modifier)) {
>> + case NV_FORMAT_MOD_TEGRA_TILED:
>> + tiling->mode = TEGRA_BO_TILING_MODE_TILED;
>> + tiling->value = 0;
>> + break;
>> +
>> + case NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(0):
>> + tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
>> + tiling->value = fourcc_mod_tegra_param(modifier);
>> + if (tiling->value > 5)
>> + return -EINVAL;
>
> Shouldn't this contain some hardware-check for the support? AFAIK, not
> all Tegras support all block-heights (if even this mode at all?)...
tegra_dc_setup_window does that check later (check the test on
dc->soc->supports_block_linear). At the moment no error message is
displayed though (and it seems like we are writing a stale value in
DC_WIN_BUFFER_ADDR_MODE if the SoC doesn't support block linear and the
tiling mode is TEGRA_BO_TILING_MODE_BLOCK?)
[PATCH 04/20] drm/ast: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
On Wed, Oct 26, 2016 at 09:15:15PM +0200, Daniel Vetter wrote:
> On Wed, Oct 26, 2016 at 8:47 PM, Stefan Lengfeld
> wrote:
> >
> > On Tue, Oct 25, 2016 at 04:57:37PM +0200, Daniel Vetter wrote:
> >> On Thu, Sep 29, 2016 at 10:48:40PM +0200, Stefan Christ wrote:
> >> > Cc: Dave Airlie
> >> > Signed-off-by: Stefan Christ
> >> > ---
> >> > drivers/gpu/drm/ast/ast_fb.c | 6 +-
> >> > 1 file changed, 1 insertion(+), 5 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
> >> > index c017a93..b604fdd 100644
> >> > --- a/drivers/gpu/drm/ast/ast_fb.c
> >> > +++ b/drivers/gpu/drm/ast/ast_fb.c
> >> > @@ -150,14 +150,10 @@ static void ast_imageblit(struct fb_info *info,
> >> >
> >> > static struct fb_ops astfb_ops = {
> >> > .owner = THIS_MODULE,
> >> > - .fb_check_var = drm_fb_helper_check_var,
> >> > - .fb_set_par = drm_fb_helper_set_par,
> >> > + DRM_FB_HELPER_DEFAULT_OPS,
> >> > .fb_fillrect = ast_fillrect,
> >> > .fb_copyarea = ast_copyarea,
> >> > .fb_imageblit = ast_imageblit,
> >>
> >> Ah, here's the likely reason for not sharing these, ast/cirrus have their
> >> special dirtying stuff for fbdev emulation. And because the fbdev mmap
> >> must have a stable pointer it can't use the ttm bo mmap magic of
> >> automatically picking the right place for the mmap.
> >>
> >> I'd say just leave these 2 drivers out as special cases.
> >> -Daniel
> >
> > Hmm. There are even more drivers using special implementations like the
> > mgag200 using function mga_fillrect(), which is a wrapper around
> > drm_fb_helper_sys_fillrect().
>
> Hm, mga_fillrect is like ast/cirrus.
>
> > Even if the drivers ast/cirrus/.. are left out, there are still two
> > different fb_fillrect, fb_copyarea and fb_imageblit implementations:
> >1/ drm_fb_helper_sys_*() and
> >2/ drm_fb_helper_cfb_*()
> > used by different drivers. I don't know which one should be preferred.
>
> Hm, every day I learn something new about fbdev. Totally missed that
> there's 2 different kinds of helpers, and I think we do indeed need
> both.
>
> > Including fb_debug_enter and fb_debug_leave in DRM_FB_HELPER_DEFAULT_OPS
> > is not a problem since there is only a single implementation yet.
> >
> > Should I resend this series (without the first patch), but with
> > additional memebers fb_debug_enter and fb_debug_leave?
>
> Yeah I think that'd be reasonable. For the sys/cfb stuff, what about
> adding new #defines for those 2, e.g. DRM_FB_HELPER_SYS_OPS and
> DRM_FB_HELPER_CFB_OPS? Maybe as a follow-up series of course, if you
> have time. When resending please pick up the acks/reviews from this
> series (but annoted with a v1 or so.
Are you still working on a v2, or should I just pick up v1 for now?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Intel-gfx] linux-next: manual merge of the tip tree with the drm-intel tree
On Tue, Nov 08, 2016 at 06:04:15PM +0100, Peter Zijlstra wrote: > On Tue, Nov 08, 2016 at 05:09:16PM +0100, Daniel Vetter wrote: > > > Now, I know you're working on getting rid of this entirely for i915, but > > > what about that MSM driver? Will we continue to need it there, is > > > anybody actually maintaining that thing? > > > > Rob Clark is, and since he's a one-man gpu driver team with other > > responsibilities it might take even longer than for i915 :( > > Fair enough. For my information, how much a of copy/paste job from i915 > was that? Could he, in principle, copy/paste your changes to get rid of > this back into MSM without too much effort, or have things diverged > greatly since the initial copy/paste? Probably diverged too much already, and on top the big part is the command submission, and that's entirely driver/hw specific. But etnaviv is a plain gem driver which uses per-bo locking, and there's all the ttm drivers with similar designs, so there's plenty of templates. But it's not just copypasta for sure. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm/nouveau: fix LEDS_CLASS=m configuration
On 08/11/16 15:56, Arnd Bergmann wrote:
> The newly introduced LED handling for nouveau fails to link when the
> driver is built-in but the LED subsystem is a loadable module:
>
> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_suspend':
> tvnv17.c:(.text.nouveau_do_suspend+0x10): undefined reference to
> `nouveau_led_suspend'
> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_resume':
> tvnv17.c:(.text.nouveau_do_resume+0xf0): undefined reference to
> `nouveau_led_resume'
> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_unload':
> tvnv17.c:(.text.nouveau_drm_unload+0x34): undefined reference to
> `nouveau_led_fini'
> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_load':
> tvnv17.c:(.text.nouveau_drm_load+0x7d0): undefined reference to
> `nouveau_led_init'
>
> This adds a separate Kconfig symbol for the LED support that
> correctly tracks the dependencies.
>
> Fixes: 8d021d71b324 ("drm/nouveau/drm/nouveau: add a LED driver for the
> NVIDIA logo")
> Signed-off-by: Arnd Bergmann
> ---
> drivers/gpu/drm/nouveau/Kbuild| 2 +-
> drivers/gpu/drm/nouveau/Kconfig | 8
> drivers/gpu/drm/nouveau/nouveau_led.h | 2 +-
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/Kbuild b/drivers/gpu/drm/nouveau/Kbuild
> index fde6e3656636..5e00e911daa6 100644
> --- a/drivers/gpu/drm/nouveau/Kbuild
> +++ b/drivers/gpu/drm/nouveau/Kbuild
> @@ -22,7 +22,7 @@ nouveau-$(CONFIG_DEBUG_FS) += nouveau_debugfs.o
> nouveau-y += nouveau_drm.o
> nouveau-y += nouveau_hwmon.o
> nouveau-$(CONFIG_COMPAT) += nouveau_ioc32.o
> -nouveau-$(CONFIG_LEDS_CLASS) += nouveau_led.o
> +nouveau-$(CONFIG_DRM_NOUVEAU_LED) += nouveau_led.o
> nouveau-y += nouveau_nvif.o
> nouveau-$(CONFIG_NOUVEAU_PLATFORM_DRIVER) += nouveau_platform.o
> nouveau-y += nouveau_usif.o # userspace <-> nvif
> diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
> index 78631fb61adf..715cd6f4dc31 100644
> --- a/drivers/gpu/drm/nouveau/Kconfig
> +++ b/drivers/gpu/drm/nouveau/Kconfig
> @@ -46,6 +46,14 @@ config NOUVEAU_DEBUG
> The paranoia and spam levels will add a lot of extra checks which
> may potentially slow down driver operation.
>
> +config DRM_NOUVEAU_LED
> + bool "Support for logo LED"
> + depends on DRM_NOUVEAU && LEDS_CLASS
> + depends on !(DRM_NOUVEAU=y && LEDS_CLASS=m)
> + help
> + Say Y here to enabling controlling the brightness of the
> + LED behind NVIDIA logo on certain Titan cards.
Titan is a little too specific and inaccurate. How about high-end cards?
As for the patch itself... I was going for the auto-magic approach but
apparently failed at it :s Sorry... I guess what I wanted to do was to
only enable the LED support if LEDS_CLASS=y.
Anyway, your approach is cleaner because it makes it easy for the user
to say if he/she wants to enable.
I will have a closer look at it later. Thanks anyway for reporting the
issue!
Martin
[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info
On 08-11-2016 17:13, Emil Velikov wrote:
> On 8 November 2016 at 16:57, Mauro Santos
> wrote:
>> On 08-11-2016 15:57, Emil Velikov wrote:
>>> On 8 November 2016 at 15:27, Mauro Santos
>>> wrote:
On 08-11-2016 15:00, Emil Velikov wrote:
> On 8 November 2016 at 13:38, Mauro Santos
> wrote:
>> On 08-11-2016 11:06, Emil Velikov wrote:
>>> On 1 November 2016 at 18:47, Mauro Santos >> gmail.com> wrote:
On 01-11-2016 18:13, Emil Velikov wrote:
> From: Emil Velikov
>
> Parsing config sysfs file wakes up the device. The latter of which may
> be slow and isn't required to begin with.
>
> Reading through config is/was required since the revision is not
> available by other means, although with a kernel patch in the way we
> can
> 'cheat' temporarily.
>
> That should be fine, since no open-source project has ever used the
> value.
>
> Cc: Michel Dänzer
> Cc: Mauro Santos
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
> Signed-off-by: Emil Velikov
> ---
> Mauro can you apply this against libdrm and rebuild it. You do _not_
> need to rebuild mesa afterwords.
>
> Thanks
> ---
> xf86drm.c | 50 +++---
> 1 file changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 52add5e..5a5100c 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char
> *d_name,
> drmPciDeviceInfoPtr device)
> {
> #ifdef __linux__
> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
> +static const char *attrs[] = {
> + "revision", /* XXX: make sure it's always first, see note
> below */
> + "vendor",
> + "device",
> + "subsystem_vendor",
> + "subsystem_device",
> +};
> char path[PATH_MAX + 1];
> -unsigned char config[64];
> -int fd, ret;
> +unsigned int data[ARRAY_SIZE(attrs)];
> +FILE *fp;
> +int ret;
>
> -snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config",
> d_name);
> -fd = open(path, O_RDONLY);
> -if (fd < 0)
> -return -errno;
> +for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) {
> +snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s",
> + d_name, attrs[i]);
> +fp = fopen(path, "r");
> +if (!fp) {
> +/* Note: First we check the revision, since older kernels
> + * may not have it. Default to zero in such cases. */
> +if (i == 0) {
> +data[i] = 0;
> +continue;
> +}
> +return -errno;
> +}
>
> -ret = read(fd, config, sizeof(config));
> -close(fd);
> -if (ret < 0)
> -return -errno;
> +ret = fscanf(fp, "%x", &data[i]);
> +fclose(fp);
> +if (ret != 1)
> +return -errno;
> +
> +}
>
> -device->vendor_id = config[0] | (config[1] << 8);
> -device->device_id = config[2] | (config[3] << 8);
> -device->revision_id = config[8];
> -device->subvendor_id = config[44] | (config[45] << 8);
> -device->subdevice_id = config[46] | (config[47] << 8);
> +device->revision_id = data[0] & 0xff;
> +device->vendor_id = data[1] & 0x;
> +device->device_id = data[2] & 0x;
> +device->subvendor_id = data[3] & 0x;
> +device->subdevice_id = data[4] & 0x;
>
> return 0;
> #else
>
I have applied this against libdrm 2.4.71 and I don't see any delays
when starting firefox/chromium/thunderbird/glxgears.
There is also no indication in dmesg that the dGPU is being
reinitialized when starting the programs where I've detected the
problem.
>>> Thanks Mauro. Can you give this a try alongside the kernel fix [1] ?
>>> I'd love to get the latter merged soon(ish).
>>> Independent of the kernel side, I might need to go another way for
>>> libdrm/mesa so I'll CC you on future patches.
>>>
>>> Your help is greatly appreciated !
>>>
>>> Thanks
>>> Emil
>>>
>>> [1] http://patchwork.ozlabs.org/patch/689975/
>>>
>>
>> I have applied the patch on top of ker
[Intel-gfx] linux-next: manual merge of the tip tree with the drm-intel tree
On Tue, Nov 08, 2016 at 05:09:16PM +0100, Daniel Vetter wrote: > > Now, I know you're working on getting rid of this entirely for i915, but > > what about that MSM driver? Will we continue to need it there, is > > anybody actually maintaining that thing? > > Rob Clark is, and since he's a one-man gpu driver team with other > responsibilities it might take even longer than for i915 :( Fair enough. For my information, how much a of copy/paste job from i915 was that? Could he, in principle, copy/paste your changes to get rid of this back into MSM without too much effort, or have things diverged greatly since the initial copy/paste?
[Intel-gfx] linux-next: manual merge of the tip tree with the drm-intel tree
On Tue, Nov 08, 2016 at 05:09:16PM +0100, Daniel Vetter wrote: > > You're talking about: > > > > lkml.kernel.org/r/20161007154351.GL3117 at twins.programming.kicks-ass.net > > > > ? I got no feedback from you DRM guys on that so I kinda forgot about > > that in the hope we'd not have to do this at all. > > Yes. Chris/Joonas, pls give this is a spin and review. OK, I'll respin that thing with Linus' feedback etc. Might not be until tomorrow though, so any additional feedback would be good. Thanks!
[PATCH v9 09/11] drm/i915: add dev.i915.oa_max_sample_rate sysctl
On Tue, 2016-11-08 at 03:47 -0800, Robert Bragg wrote:
>
>
> On Tue, Nov 8, 2016 at 6:19 AM, sourab gupta
> wrote:
> On Mon, 2016-11-07 at 11:49 -0800, Robert Bragg wrote:
> > The maximum OA sampling frequency is now configurable via a
> > dev.i915.oa_max_sample_rate sysctl parameter.
> >
> > Following the precedent set by perf's similar
> > kernel.perf_event_max_sample_rate the default maximum rate
> is 10Hz
> >
> > Signed-off-by: Robert Bragg
> > ---
> > drivers/gpu/drm/i915/i915_perf.c | 61
>
> > 1 file changed, 50 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_perf.c
> b/drivers/gpu/drm/i915/i915_perf.c
> > index e51c1d8..1a87fe9 100644
> > --- a/drivers/gpu/drm/i915/i915_perf.c
> > +++ b/drivers/gpu/drm/i915/i915_perf.c
> > @@ -82,6 +82,21 @@ static u32 i915_perf_stream_paranoid =
> true;
> > #define INVALID_CTX_ID 0x
> >
> >
> > +/* For sysctl proc_dointvec_minmax of
> i915_oa_max_sample_rate
> > + *
> > + * 160ns is the smallest sampling period we can
> theoretically program the OA
> > + * unit with on Haswell, corresponding to 6.25MHz.
> > + */
> > +static int oa_sample_rate_hard_limit = 625;
> There's no check for 'oa_sample_rate_hard_limit' anywhere
> below.
>
>
> It's in the struct ctl_table oa_table[] declaration of the
> "oa_max_sample_rate" paramater, assigned to .extra2 which is
> referenced by the proc_dointvec_minmax validation handler for the
> parameter.
>
Ok. Seems fine then.
>
>
> > +
> > +/* Theoretically we can program the OA unit to sample every
> 160ns but don't
> > + * allow that by default unless root...
> > + *
> > + * The default threshold of 10Hz is based on perf's
> similar
> > + * kernel.perf_event_max_sample_rate sysctl parameter.
> > + */
> > +static u32 i915_oa_max_sample_rate = 10;
> > +
> > /* XXX: beware if future OA HW adds new report formats that
> the current
> > * code assumes all reports have a power-of-two size and
> ~(size - 1) can
> > * be used as a mask to align the OA tail pointer.
> > @@ -1314,6 +1329,7 @@ static int
> read_properties_unlocked(struct drm_i915_private *dev_priv,
> > }
> >
> > for (i = 0; i < n_props; i++) {
> > + u64 oa_period, oa_freq_hz;
> > u64 id, value;
> > int ret;
> >
> > @@ -1359,21 +1375,35 @@ static int
> read_properties_unlocked(struct drm_i915_private *dev_priv,
> > return -EINVAL;
> > }
> >
> > - /* NB: The exponent represents a
> period as follows:
> > - *
> > - * 80ns * 2^(period_exponent + 1)
> > - *
> > - * Theoretically we can program the OA
> unit to sample
> > + /* Theoretically we can program the OA
> unit to sample
> >* every 160ns but don't allow that by
> default unless
> >* root.
> >*
> > - * Referring to perf's
> > - * kernel.perf_event_max_sample_rate
> for a precedent
> > - * (10 by default); with an OA
> exponent of 6 we get
> > - * a period of 10.240 microseconds
> -just under 10Hz
> > + * On Haswell the period is derived
> from the exponent
> > + * as:
> > + *
> > + * period = 80ns * 2^(exponent + 1)
> > + */
> > + BUILD_BUG_ON(sizeof(oa_period) != 8);
> > + oa_period = 80ull * (2ull << value);
>
> I assume now that there'll be a platform specific check for
> 80ull, while
> programming oa_period, for subquent Gen8+ platforms, which
> should be
> fine.
>
>
> Yeah, this code will need adapting for gen9+. I guess we'll change it
> to work in terms of ((2<
Seems reasonable.
>
>
> > +
> > + /* This check is primarily to ensure
> that oa_period <=
> > + * UINT32_MAX (before passing to
> do_div which only
[GIT PULL] drm/fsl-dcu: fixes for v4.9-rc5
Hi Dave, Yet antoher small batch of fixes. Two of the patches I had prepared since quite some time, but they did not seem to affect operation in a visible manner so far. Until recently, when I discovered the third issue (disable planes before disabling CRTC), which made the two previous fixes necessary. -- Stefan The following changes since commit 020a0bbc0d89c15693e69ed2063584ef7ec2d811: Merge branch 'msm-fixes-4.9' of git://people.freedesktop.org/~robclark/linux into drm-fixes (2016-11-07 09:41:10 +1000) are available in the git repository at: http://git.agner.ch/git/linux-drm-fsl-dcu.git fixes-for-v4.9-rc5 for you to fetch changes up to 3d6f37102bd6e4b55a7f336d44974c0bd1c22a15: drm/fsl-dcu: disable planes before disabling CRTC (2016-11-08 17:14:08 -0800) Stefan Agner (3): drm/fsl-dcu: do not update when modifying irq registers drm/fsl-dcu: update all registers on flush drm/fsl-dcu: disable planes before disabling CRTC drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 13 +++-- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 4 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 5 - 3 files changed, 11 insertions(+), 11 deletions(-)
[Intel-gfx] [PATCH] drm/i915: avoid harmless empty-body warning
On Tue, Nov 08, 2016 at 02:49:05PM +, Chris Wilson wrote:
> On Tue, Nov 08, 2016 at 02:58:17PM +0100, Arnd Bergmann wrote:
> > The newly added assert_kernel_context_is_current introduces a warning
> > when built with W=1:
> >
> > drivers/gpu/drm/i915/i915_gem.c: In function
> > âassert_kernel_context_is_currentâ:
> > drivers/gpu/drm/i915/i915_gem.c:4417:63: error: suggest braces around empty
> > body in an âelseâ statement [-Werror=empty-body]
> >
> > Changing the GEM_BUG_ON() macro from an empty definition to "do { } while
> > (0)"
> > makes the macro more robust to use and avoids the warning.
> >
> > Fixes: 3033acab07f9 ("drm/i915: Queue the idling context switch after all
> > other timelines")
> > Signed-off-by: Arnd Bergmann
> Reviewed-by: Chris Wilson
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm/vmwgfx : Fix NULL pointer comparison
From: Ravikant B Sharma
Replace direct comparisons to NULL i.e.
'x == NULL' with '!x'. As per coding standard.
Signed-off-by: Ravikant B Sharma
---
drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf_res.c|4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_context.c |2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c |2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |6 +++---
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c |4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 10 +-
drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c |2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_mob.c |6 +++---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c |6 +++---
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c |6 +++---
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_shader.c|6 +++---
12 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf_res.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf_res.c
index 13db8a2..133c8bd 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf_res.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf_res.c
@@ -205,7 +205,7 @@ int vmw_cmdbuf_res_add(struct vmw_cmdbuf_res_manager *man,
int ret;
cres = kzalloc(sizeof(*cres), GFP_KERNEL);
- if (unlikely(cres == NULL))
+ if (unlikely(!cres))
return -ENOMEM;
cres->hash.key = user_key | (res_type << 24);
@@ -291,7 +291,7 @@ struct vmw_cmdbuf_res_manager *
int ret;
man = kzalloc(sizeof(*man), GFP_KERNEL);
- if (man == NULL)
+ if (!man)
return ERR_PTR(-ENOMEM);
man->dev_priv = dev_priv;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c
index 443d1ed..1a46b18 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c
@@ -776,7 +776,7 @@ static int vmw_context_define(struct drm_device *dev, void
*data,
}
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
- if (unlikely(ctx == NULL)) {
+ if (unlikely(!ctx)) {
ttm_mem_global_free(vmw_mem_glob(dev_priv),
vmw_user_context_size);
ret = -ENOMEM;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
index 265c81e..8751805 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
@@ -583,7 +583,7 @@ struct vmw_resource *vmw_cotable_alloc(struct vmw_private
*dev_priv,
return ERR_PTR(ret);
vcotbl = kzalloc(sizeof(*vcotbl), GFP_KERNEL);
- if (unlikely(vcotbl == NULL)) {
+ if (unlikely(!vcotbl)) {
ret = -ENOMEM;
goto out_no_alloc;
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 18061a4..8a269db 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -625,7 +625,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned
long chipset)
char host_log[100] = {0};
dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
- if (unlikely(dev_priv == NULL)) {
+ if (unlikely(!dev_priv)) {
DRM_ERROR("Failed allocating a device private struct.\n");
return -ENOMEM;
}
@@ -1029,7 +1029,7 @@ static int vmw_driver_open(struct drm_device *dev, struct
drm_file *file_priv)
int ret = -ENOMEM;
vmw_fp = kzalloc(sizeof(*vmw_fp), GFP_KERNEL);
- if (unlikely(vmw_fp == NULL))
+ if (unlikely(!vmw_fp))
return ret;
vmw_fp->tfile = ttm_object_file_init(dev_priv->tdev, 10);
@@ -1186,7 +1186,7 @@ static int vmw_master_create(struct drm_device *dev,
struct vmw_master *vmaster;
vmaster = kzalloc(sizeof(*vmaster), GFP_KERNEL);
- if (unlikely(vmaster == NULL))
+ if (unlikely(!vmaster))
return -ENOMEM;
vmw_master_init(vmaster);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index c7b53d9..2154257 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -264,7 +264,7 @@ static int vmw_resource_val_add(struct vmw_sw_context
*sw_context,
}
node = kzalloc(sizeof(*node), GFP_KERNEL);
- if (unlikely(node == NULL)) {
+ if (unlikely(!node)) {
DRM_ERROR("Failed to allocate a resource validation "
"entry.\n");
return -ENOMEM;
@@ -452,7 +452,7 @@ static int vmw_resource_relocation_add(struct list_head
*list,
struct vmw_resource_relocation *rel;
rel = kmalloc(sizeof(*rel), GFP_KERNEL);
- if (unlikely(rel == NULL)) {
+ if (unlikely(!rel)) {
DRM_ERROR("Failed to allocate a resource relocation.\n");
return -ENOMEM;
[Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration
2016-11-08 17:12 GMT+01:00 Arnd Bergmann :
> On Tuesday, November 8, 2016 5:07:01 PM CET Lukas Wunner wrote:
>> On Tue, Nov 08, 2016 at 04:52:49PM +0100, Arnd Bergmann wrote:
>> > The underlying problem is that we already have a number of other
>> > symbols that either have "depends on LEDS_CLASS" or
>> > "select LEDS_CLASS". To clean that up properly, we should either
>> > make the symbol itself hidden and only select it from other drivers,
>> > or use "depends on LEDS_CLASS" everywhere.
>> >
>> > Another option is to use the IS_REACHABLE() macro instead of IS_ENABLED()
>> > in the header file, to stub out the calls into the new file, but
>> > that can be a bit confusing.
>>
>> Why don't you just add empty inline stubs for nouveau_led_init / _fini /
>> _suspend / _resume?
>>
>
> That's what I was suggesting:
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_led.h
> b/drivers/gpu/drm/nouveau/nouveau_led.h
> index 9c9bb6ac938e..bc5f47cb516b 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_led.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_led.h
> @@ -35,21 +35,21 @@ struct nouveau_led {
> struct led_classdev led;
> };
>
> static inline struct nouveau_led *
> nouveau_led(struct drm_device *dev)
> {
> return nouveau_drm(dev)->led;
> }
>
> /* nouveau_led.c */
> -#if IS_ENABLED(CONFIG_LEDS_CLASS)
> +#if IS_REACHABLE(CONFIG_LEDS_CLASS)
> int nouveau_led_init(struct drm_device *dev);
> void nouveau_led_suspend(struct drm_device *dev);
> void nouveau_led_resume(struct drm_device *dev);
> void nouveau_led_fini(struct drm_device *dev);
> #else
> static inline int nouveau_led_init(struct drm_device *dev) { return 0; };
> static inline void nouveau_led_suspend(struct drm_device *dev) { };
> static inline void nouveau_led_resume(struct drm_device *dev) { };
> static inline void nouveau_led_fini(struct drm_device *dev) { };
> #endif
>
> The downside is that now the nouveau_led_init() just won't be called
> if CONFIG_LEDS_CLASS=m and CONFIG_DRM_NOUVEAU=y, which can be
> surprising to users.
yeah, it will. I guess it is fine to force LEDS to y if nouveau is set
to y. The thinks I absolutely dislike is:
1. auto hiding of features I _want_ to have, because I would have to
enable the dependencies first, which is like super annoying if there
are somewhere else
2. preventing me from enabling something, cause the dependency is missing.
We should clarify first if we actually want to enable those features
optionally, because there isn't much of a reason not to enable the
dependencies, except embedded systems. We have a lot more stuff where
we could add things like that: hwmon, debugfs, acpi, compat and maybe
there are even more things
>
> Arnd
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
[PATCH RFC 00/12] tda998x updates
On Tue, 2016-11-08 at 13:34 +, Russell King - ARM Linux wrote: > On Tue, Nov 08, 2016 at 01:32:15PM +, Russell King - ARM Linux wrote: > > Unfortunately, my drm-tda998x-devel branch is slightly out of date with > > these patches it's the original set of 10 patches. I've not pushed > > these ones out to that branch yet, as I've three additional patches on > > top of these which aren't "ready" for pushing out. > > Here's the delta between the branch and what I just posted: > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c > b/drivers/gpu/drm/i2c/tda998x_drv.c [...] I have a working setup for HDMI audio on Juno an would like to test this series but am struggling to work out which patches to apply in what order to what branch, can you be specific? (I've tried various combinations of patches series from the list, drm-tda998x-devel, and the diff you posted) Thanks -- Tixy
[PATCH] drm: mali-dp: Add support for setting plane's rotation property from userspace.
On Tue, Nov 08, 2016 at 01:43:25PM +, Liviu Dudau wrote:
> In order to support DRM_IOCTL_MODE_OBJ_SETPROPERTY for the rotation property
> we need to have a ->set_property hook defined for the planes. Set the
> plane's ->set_property hook to drm_atomic_helper_plane_set_property()
>
> Signed-off-by: Liviu Dudau
> ---
> drivers/gpu/drm/arm/malidp_planes.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c
> b/drivers/gpu/drm/arm/malidp_planes.c
> index bb1c528..8eef9a8 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -89,6 +89,7 @@ void malidp_destroy_plane_state(struct drm_plane *plane,
> static const struct drm_plane_funcs malidp_de_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> + .set_property = drm_atomic_helper_plane_set_property,
We discussed this a bit, I think it'd be nice to simply make this the
default for all atomic drivers (and demote all the set_prop helpers from
exported helpers to internal static functions). But can be done as a
cleanup later on, ack for this patch meanwhile.
-Daniel
> .destroy = malidp_de_plane_destroy,
> .reset = drm_atomic_helper_plane_reset,
> .atomic_duplicate_state = malidp_duplicate_plane_state,
> --
> 2.10.0
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration
2016-11-08 16:46 GMT+01:00 Ilia Mirkin :
> On Tue, Nov 8, 2016 at 8:56 AM, Arnd Bergmann wrote:
>> The newly introduced LED handling for nouveau fails to link when the
>> driver is built-in but the LED subsystem is a loadable module:
>>
>> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_suspend':
>> tvnv17.c:(.text.nouveau_do_suspend+0x10): undefined reference to
>> `nouveau_led_suspend'
>> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_resume':
>> tvnv17.c:(.text.nouveau_do_resume+0xf0): undefined reference to
>> `nouveau_led_resume'
>> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_unload':
>> tvnv17.c:(.text.nouveau_drm_unload+0x34): undefined reference to
>> `nouveau_led_fini'
>> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_load':
>> tvnv17.c:(.text.nouveau_drm_load+0x7d0): undefined reference to
>> `nouveau_led_init'
>>
>> This adds a separate Kconfig symbol for the LED support that
>> correctly tracks the dependencies.
>>
>> Fixes: 8d021d71b324 ("drm/nouveau/drm/nouveau: add a LED driver for the
>> NVIDIA logo")
>> Signed-off-by: Arnd Bergmann
>> ---
>> drivers/gpu/drm/nouveau/Kbuild| 2 +-
>> drivers/gpu/drm/nouveau/Kconfig | 8
>> drivers/gpu/drm/nouveau/nouveau_led.h | 2 +-
>> 3 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/Kbuild b/drivers/gpu/drm/nouveau/Kbuild
>> index fde6e3656636..5e00e911daa6 100644
>> --- a/drivers/gpu/drm/nouveau/Kbuild
>> +++ b/drivers/gpu/drm/nouveau/Kbuild
>> @@ -22,7 +22,7 @@ nouveau-$(CONFIG_DEBUG_FS) += nouveau_debugfs.o
>> nouveau-y += nouveau_drm.o
>> nouveau-y += nouveau_hwmon.o
>> nouveau-$(CONFIG_COMPAT) += nouveau_ioc32.o
>> -nouveau-$(CONFIG_LEDS_CLASS) += nouveau_led.o
>> +nouveau-$(CONFIG_DRM_NOUVEAU_LED) += nouveau_led.o
>> nouveau-y += nouveau_nvif.o
>> nouveau-$(CONFIG_NOUVEAU_PLATFORM_DRIVER) += nouveau_platform.o
>> nouveau-y += nouveau_usif.o # userspace <-> nvif
>> diff --git a/drivers/gpu/drm/nouveau/Kconfig
>> b/drivers/gpu/drm/nouveau/Kconfig
>> index 78631fb61adf..715cd6f4dc31 100644
>> --- a/drivers/gpu/drm/nouveau/Kconfig
>> +++ b/drivers/gpu/drm/nouveau/Kconfig
>> @@ -46,6 +46,14 @@ config NOUVEAU_DEBUG
>> The paranoia and spam levels will add a lot of extra checks which
>> may potentially slow down driver operation.
>>
>> +config DRM_NOUVEAU_LED
>> + bool "Support for logo LED"
>> + depends on DRM_NOUVEAU && LEDS_CLASS
>> + depends on !(DRM_NOUVEAU=y && LEDS_CLASS=m)
>> + help
>> + Say Y here to enabling controlling the brightness of the
>> + LED behind NVIDIA logo on certain Titan cards.
>
> This is a very odd restriction... could this be written as
>
> depends on DRM_NOUVEAU
> select LEDS_CLASS
>
> Or will that not flip the LEDS_CLASS from m to y in case
> DRM_NOUVEAU=y? If not, is there a way to cause that to happen?
>
> Separately, perhaps we should just drop this LEDS_CLASS select into
> DRM_NOUVEAU? We've tended to avoid adding tons of options.
>
well, that would mean that you always need the LEDS_CLASS and maybe on
a tegra system you don't want to, so the led stuff should stay
completely optional. Don't know though.
Alex, maybe you want to clarify which dependencies should stay
optional? If nobody on your side care, then we won't care as well and
only add switches if users actually request it.
> Cheers,
>
> -ilia
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
[GIT PULL][drm-next] drm/mali-dp fixes and updates
On Tue, Nov 08, 2016 at 01:41:08PM +, Liviu Dudau wrote: > On Tue, Nov 08, 2016 at 01:49:55PM +0100, Daniel Vetter wrote: > > On Tue, Nov 08, 2016 at 12:26:19PM +, Liviu Dudau wrote: > > > Hi Dave, > > > > > > Here is the list of fixes that I have for drm/mali-dp. They've been on > > > the mailing > > > lists for a while and merged into linux-next for a few weeks, but due to > > > holiday and > > > travel to Linux Plumbers I did not send the pull request earlier. I don't > > > know if > > > these patches can be pulled into v4.9 still (they will conflict with > > > Ville Syrjälä's > > > cleanup of DRM_ROTATE series that is already in drm-next), but if you do > > > that would > > > be great. > > > > > > The following changes since commit > > > fb422950c6cd726fd36eb72a7cf84583440a18a2: > > > > > > Merge branch 'linux-4.9' of git://github.com/skeggsb/linux into > > > drm-next (2016-10-28 14:24:56 +1000) > > > > > > are available in the git repository at: > > > > > > git://linux-arm.org/linux-ld.git for-upstream/mali-dp > > > > > > for you to fetch changes up to e64053f05eb924db45f90a1556a200d1acb4b01e: > > > > > > drm: mali-dp: Clear CVAL when leaving config mode (2016-11-08 11:40:02 > > > +) > > > > > > > > > Baoyou Xie (1): > > > drm/arm: mark symbols static where possible > > > > > > Brian Starkey (8): > > > drm: mali-dp: Add pitch alignment check function > > > drm: mali-dp: Add pitch alignment check for planes > > > arm: mali-dp: Extract mode_config cleanup into malidp_fini > > > drm: mali-dp: Refactor plane initialisation > > > drm: mali-dp: Enable alpha blending > > > drm: mali-dp: Store internal format and n_planes in plane state > > > drm: mali-dp: Don't set DRM_PLANE_COMMIT_ACTIVE_ONLY > > > drm: mali-dp: Clear CVAL when leaving config mode > > > > > > Liviu Dudau (3): > > > drm: mali-dp: Clear the config_valid flag before using it in > > > wait_event. > > > drm: mali-dp: Set the drm->irq_enabled flag to match driver's state. > > > drm: mali-dp: Add support for setting plane's rotation property > > > from userspace. > > > > Quick check says these patches haven't shown on up a mailing list. Please > > submit first, do review in public and then send the pull request. There's > > generally a lot of cross-driver learning possible with this kind of stuff. > > Imo splitting drivers into their own mailing list and community only makes > > sense once there's just too much traffic, like with the big ones (intel, > > amd and nouveau because it's developed in userspace entirely). > > Actually, all except the last patch from me and last from Brian have been on > the > dri-devel mailing list, I believe you have even commented on one of them > (drm->irq_enabled). Hm indeed, my search-fu was seriously failing me. > For my patches, first two start here [1]. For Brian's patches, his series > starts here [2] (also contains my patches) and one before last is here [3] > > [1] https://lists.freedesktop.org/archives/dri-devel/2016-July/114302.html > [2] https://lists.freedesktop.org/archives/dri-devel/2016-October/120486.html > [3] https://lists.freedesktop.org/archives/dri-devel/2016-October/121849.html > > I appologise for missing to send the last two patches, they were in the > public tree > but did not sent them for some reason to the public list. Will do that now. Thanks, and apologies anyway. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info
On 8 November 2016 at 16:57, Mauro Santos wrote:
> On 08-11-2016 15:57, Emil Velikov wrote:
>> On 8 November 2016 at 15:27, Mauro Santos
>> wrote:
>>> On 08-11-2016 15:00, Emil Velikov wrote:
On 8 November 2016 at 13:38, Mauro Santos
wrote:
> On 08-11-2016 11:06, Emil Velikov wrote:
>> On 1 November 2016 at 18:47, Mauro Santos > gmail.com> wrote:
>>> On 01-11-2016 18:13, Emil Velikov wrote:
From: Emil Velikov
Parsing config sysfs file wakes up the device. The latter of which may
be slow and isn't required to begin with.
Reading through config is/was required since the revision is not
available by other means, although with a kernel patch in the way we
can
'cheat' temporarily.
That should be fine, since no open-source project has ever used the
value.
Cc: Michel Dänzer
Cc: Mauro Santos
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
Signed-off-by: Emil Velikov
---
Mauro can you apply this against libdrm and rebuild it. You do _not_
need to rebuild mesa afterwords.
Thanks
---
xf86drm.c | 50 +++---
1 file changed, 35 insertions(+), 15 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index 52add5e..5a5100c 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char
*d_name,
drmPciDeviceInfoPtr device)
{
#ifdef __linux__
+#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
+static const char *attrs[] = {
+ "revision", /* XXX: make sure it's always first, see note below
*/
+ "vendor",
+ "device",
+ "subsystem_vendor",
+ "subsystem_device",
+};
char path[PATH_MAX + 1];
-unsigned char config[64];
-int fd, ret;
+unsigned int data[ARRAY_SIZE(attrs)];
+FILE *fp;
+int ret;
-snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config",
d_name);
-fd = open(path, O_RDONLY);
-if (fd < 0)
-return -errno;
+for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) {
+snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s",
+ d_name, attrs[i]);
+fp = fopen(path, "r");
+if (!fp) {
+/* Note: First we check the revision, since older kernels
+ * may not have it. Default to zero in such cases. */
+if (i == 0) {
+data[i] = 0;
+continue;
+}
+return -errno;
+}
-ret = read(fd, config, sizeof(config));
-close(fd);
-if (ret < 0)
-return -errno;
+ret = fscanf(fp, "%x", &data[i]);
+fclose(fp);
+if (ret != 1)
+return -errno;
+
+}
-device->vendor_id = config[0] | (config[1] << 8);
-device->device_id = config[2] | (config[3] << 8);
-device->revision_id = config[8];
-device->subvendor_id = config[44] | (config[45] << 8);
-device->subdevice_id = config[46] | (config[47] << 8);
+device->revision_id = data[0] & 0xff;
+device->vendor_id = data[1] & 0x;
+device->device_id = data[2] & 0x;
+device->subvendor_id = data[3] & 0x;
+device->subdevice_id = data[4] & 0x;
return 0;
#else
>>>
>>> I have applied this against libdrm 2.4.71 and I don't see any delays
>>> when starting firefox/chromium/thunderbird/glxgears.
>>>
>>> There is also no indication in dmesg that the dGPU is being
>>> reinitialized when starting the programs where I've detected the
>>> problem.
>>>
>> Thanks Mauro. Can you give this a try alongside the kernel fix [1] ?
>> I'd love to get the latter merged soon(ish).
>> Independent of the kernel side, I might need to go another way for
>> libdrm/mesa so I'll CC you on future patches.
>>
>> Your help is greatly appreciated !
>>
>> Thanks
>> Emil
>>
>> [1] http://patchwork.ozlabs.org/patch/689975/
>>
>
> I have applied the patch on top of kernel 4.8.6 and I'm using the libdrm
> with the patch you sent me previously.
>
> With this patch things still seem work fine, I don't see any of the
> problem
[Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration
On Tuesday, November 8, 2016 5:07:01 PM CET Lukas Wunner wrote:
> On Tue, Nov 08, 2016 at 04:52:49PM +0100, Arnd Bergmann wrote:
> > The underlying problem is that we already have a number of other
> > symbols that either have "depends on LEDS_CLASS" or
> > "select LEDS_CLASS". To clean that up properly, we should either
> > make the symbol itself hidden and only select it from other drivers,
> > or use "depends on LEDS_CLASS" everywhere.
> >
> > Another option is to use the IS_REACHABLE() macro instead of IS_ENABLED()
> > in the header file, to stub out the calls into the new file, but
> > that can be a bit confusing.
>
> Why don't you just add empty inline stubs for nouveau_led_init / _fini /
> _suspend / _resume?
>
That's what I was suggesting:
diff --git a/drivers/gpu/drm/nouveau/nouveau_led.h
b/drivers/gpu/drm/nouveau/nouveau_led.h
index 9c9bb6ac938e..bc5f47cb516b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_led.h
+++ b/drivers/gpu/drm/nouveau/nouveau_led.h
@@ -35,21 +35,21 @@ struct nouveau_led {
struct led_classdev led;
};
static inline struct nouveau_led *
nouveau_led(struct drm_device *dev)
{
return nouveau_drm(dev)->led;
}
/* nouveau_led.c */
-#if IS_ENABLED(CONFIG_LEDS_CLASS)
+#if IS_REACHABLE(CONFIG_LEDS_CLASS)
int nouveau_led_init(struct drm_device *dev);
void nouveau_led_suspend(struct drm_device *dev);
void nouveau_led_resume(struct drm_device *dev);
void nouveau_led_fini(struct drm_device *dev);
#else
static inline int nouveau_led_init(struct drm_device *dev) { return 0; };
static inline void nouveau_led_suspend(struct drm_device *dev) { };
static inline void nouveau_led_resume(struct drm_device *dev) { };
static inline void nouveau_led_fini(struct drm_device *dev) { };
#endif
The downside is that now the nouveau_led_init() just won't be called
if CONFIG_LEDS_CLASS=m and CONFIG_DRM_NOUVEAU=y, which can be
surprising to users.
Arnd
[PATCH v7 1/3] drm/fence: add in-fences support
On Tue, Nov 08, 2016 at 03:27:14PM +, Brian Starkey wrote:
> Hi Gustavo,
>
> On Tue, Nov 08, 2016 at 03:54:48PM +0900, Gustavo Padovan wrote:
> > From: Gustavo Padovan
> >
> > There is now a new property called IN_FENCE_FD attached to every plane
> > state that receives sync_file fds from userspace via the atomic commit
> > IOCTL.
> >
> > The fd is then translated to a fence (that may be a fence_array
> > subclass or just a normal fence) and then used by DRM to fence_wait() for
> > all fences in the sync_file to signal. So it only commits when all
> > framebuffers are ready to scanout.
> >
> > v2: Comments by Daniel Vetter:
> > - remove set state->fence = NULL in destroy phase
> > - accept fence -1 as valid and just return 0
> > - do not call fence_get() - sync_file_fences_get() already calls it
> > - fence_put() if state->fence is already set, in case userspace
> > set the property more than once.
> >
> > v3: WARN_ON if fence is set but state has no FB
> >
> > v4: Comment from Maarten Lankhorst
> > - allow set fence with no related fb
> >
> > v5: rename FENCE_FD to IN_FENCE_FD
> >
> > v6: Comments by Daniel Vetter:
> > - rename plane_state->in_fence back to "fence"
> > - re-introduce WARN_ON if fence set but no fb
> >
> > - rebase after fence -> dma_fence rename
> >
> > Signed-off-by: Gustavo Padovan
> > ---
> > drivers/gpu/drm/Kconfig | 1 +
> > drivers/gpu/drm/drm_atomic.c| 14 ++
> > drivers/gpu/drm/drm_atomic_helper.c | 3 +++
> > drivers/gpu/drm/drm_crtc.c | 6 ++
> > drivers/gpu/drm/drm_plane.c | 1 +
> > include/drm/drm_crtc.h | 5 +
> > 6 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 483059a..43cb33d 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -12,6 +12,7 @@ menuconfig DRM
> > select I2C
> > select I2C_ALGOBIT
> > select DMA_SHARED_BUFFER
> > + select SYNC_FILE
> > help
> > Kernel-level support for the Direct Rendering Infrastructure (DRI)
> > introduced in XFree86 4.0. If you say Y here, you need to select
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 5e73954..d1ae463 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -30,6 +30,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include "drm_crtc_internal.h"
> >
> > @@ -686,6 +687,17 @@ int drm_atomic_plane_set_property(struct drm_plane
> > *plane,
> > drm_atomic_set_fb_for_plane(state, fb);
> > if (fb)
> > drm_framebuffer_unreference(fb);
> > + } else if (property == config->prop_in_fence_fd) {
> > + if (U642I64(val) == -1)
> > + return 0;
> > +
>
> Sorry, I'm a bit late to bring this up but what's the expected
> behaviour of a commit which sets OUT_FENCE twice (first with a valid
> fence, then with -1 later in the list)? Maybe you need to actually
> clear state->fence for -1.
Good point, we should probaly reject that. Since ->fence should autoclear
(I totally missed that one) we should check for an existing fence and fail
if it's set already.
> > + if (state->fence)
> > + dma_fence_put(state->fence);
> > +
> > + state->fence = sync_file_get_fence(val);
> > + if (!state->fence)
> > + return -EINVAL;
> > +
> > } else if (property == config->prop_crtc_id) {
> > struct drm_crtc *crtc = drm_crtc_find(dev, val);
> > return drm_atomic_set_crtc_for_plane(state, crtc);
> > @@ -747,6 +759,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> >
> > if (property == config->prop_fb_id) {
> > *val = (state->fb) ? state->fb->base.id : 0;
> > + } else if (property == config->prop_in_fence_fd) {
> > + *val = -1;
> > } else if (property == config->prop_crtc_id) {
> > *val = (state->crtc) ? state->crtc->base.id : 0;
> > } else if (property == config->prop_crtc_x) {
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 75ad01d..26a067f 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3114,6 +3114,9 @@ void __drm_atomic_helper_plane_destroy_state(struct
> > drm_plane_state *state)
> > {
> > if (state->fb)
> > drm_framebuffer_unreference(state->fb);
> > +
> > + if (state->fence)
> > + dma_fence_put(state->fence);
> > }
> > EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
> >
>
> It looks like you need to add something in
> __drm_atomic_helper_plane_duplicate_state() to either get a reference
> on the fence or set state->fence = NULL, because the memcpy() there
> will copy the pointer.
Clearing to NULL would be best, since that matches the mea
[Intel-gfx] linux-next: manual merge of the tip tree with the drm-intel tree
On Tue, Nov 08, 2016 at 02:24:48PM +0100, Peter Zijlstra wrote:
> On Tue, Nov 08, 2016 at 11:44:03AM +0100, Daniel Vetter wrote:
> > On Tue, Nov 08, 2016 at 03:25:41PM +1100, Stephen Rothwell wrote:
> > > Hi all,
> > >
> > > FIXME: Add owner of second tree to To:
> > >Add author(s)/SOB of conflicting commits.
> > >
> > > Today's linux-next merge of the tip tree got a conflict in:
> > >
> > > drivers/gpu/drm/i915/i915_gem_shrinker.c
> > >
> > > between commits:
> > >
> > > 1233e2db199d ("drm/i915: Move object backing storage manipulation to
> > > its own locking")
> > >
> > > from the drm-intel tree and commit:
> > >
> > > 3ab7c086d5ec ("locking/drm: Kill mutex trickery")
> > > c7faee2109f9 ("locking/drm: Fix i915_gem_shrinker_lock() locking")
> >
> > Hm, this seems to be the older versions that nuke the recursive locking
> > trickery entirely, I thought we had version in-flight that kept that? I
> > know that the i915 (and msm locking fwiw) is horrible since essentially
> > it's a recursive BKL, and we're working (slowly, after all getting rid of
> > the BKL wasn't simple either) to fix this. But meanwhile I'm assuming that
> > we'll still need this to be able to get out of low memory situations in
> > i915. Has that part simply not yet landed?
>
> You're talking about:
>
> lkml.kernel.org/r/20161007154351.GL3117 at twins.programming.kicks-ass.net
>
> ? I got no feedback from you DRM guys on that so I kinda forgot about
> that in the hope we'd not have to do this at all.
Yes. Chris/Joonas, pls give this is a spin and review.
>
> I can try and resurrect, that I suppose.
>
> Now, I know you're working on getting rid of this entirely for i915, but
> what about that MSM driver? Will we continue to need it there, is
> anybody actually maintaining that thing?
Rob Clark is, and since he's a one-man gpu driver team with other
responsibilities it might take even longer than for i915 :(
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
imx-drm hang issue with etnaviv (GC3000)
Am Freitag, den 28.10.2016, 10:18 +0200 schrieb Lucas Stach: > Am Donnerstag, den 27.10.2016, 19:26 +0200 schrieb Wladimir J. van der > Laan: > > Hello, > > > > After running kmscube (or another KMS executable) on a i.MX6 QuadPlus > > (etnaviv, > > GC3000) a few times on I get the below crash in the drm kernel driver. > > This is on a device with LVDS panel. It is always reproducible, although the > > number of invocations needed differs. > > > > The only way to get rendering to work again after the crash is to reboot. > > Repeated tries only get the "flip_done timed out". > > > > This always happens while the program is exiting. > > > > Versions: > > > > - mesa: https://github.com/etnaviv/mesa 9a09984 > > > > - libdrm: https://cgit.freedesktop.org/mesa/drm/ fe4579e > > > > - Kernel: 4.8.0 or 4.8.4 + Pengutronix patches (20161007). > > > > Does anyone have an idea what could be the problem? > > > I think I've seen this problem a few times already. I'll have a look at > this today. And it had to wait a bit, but I was finally able to track down what's wrong here. You want patch "[PATCH 1/2] drm/imx: disable planes before DC" to get rid of this issue. Regards, Lucas
[Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration
On Tue, Nov 08, 2016 at 04:52:49PM +0100, Arnd Bergmann wrote: > The underlying problem is that we already have a number of other > symbols that either have "depends on LEDS_CLASS" or > "select LEDS_CLASS". To clean that up properly, we should either > make the symbol itself hidden and only select it from other drivers, > or use "depends on LEDS_CLASS" everywhere. > > Another option is to use the IS_REACHABLE() macro instead of IS_ENABLED() > in the header file, to stub out the calls into the new file, but > that can be a bit confusing. Why don't you just add empty inline stubs for nouveau_led_init / _fini / _suspend / _resume? Thanks, Lukas
[PATCH 2/2] gpu: ipu-v3: remove IRQ dance on DC channel disable
This has never worked properly, as the IRQ got retriggered immediately
on unmask. Remove the IRQ wait dance, as it is apparently safe to disable
the DC channel at any point in time.
Signed-off-by: Lucas Stach
---
drivers/gpu/ipu-v3/ipu-dc.c | 61 +++--
1 file changed, 4 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-dc.c b/drivers/gpu/ipu-v3/ipu-dc.c
index 659475c1e44a..7a4b8362dda8 100644
--- a/drivers/gpu/ipu-v3/ipu-dc.c
+++ b/drivers/gpu/ipu-v3/ipu-dc.c
@@ -112,8 +112,6 @@ struct ipu_dc_priv {
struct ipu_dc channels[IPU_DC_NUM_CHANNELS];
struct mutexmutex;
struct completion comp;
- int dc_irq;
- int dp_irq;
int use_count;
};
@@ -262,47 +260,13 @@ void ipu_dc_enable_channel(struct ipu_dc *dc)
}
EXPORT_SYMBOL_GPL(ipu_dc_enable_channel);
-static irqreturn_t dc_irq_handler(int irq, void *dev_id)
-{
- struct ipu_dc *dc = dev_id;
- u32 reg;
-
- reg = readl(dc->base + DC_WR_CH_CONF);
- reg &= ~DC_WR_CH_CONF_PROG_TYPE_MASK;
- writel(reg, dc->base + DC_WR_CH_CONF);
-
- /* The Freescale BSP kernel clears DIx_COUNTER_RELEASE here */
-
- complete(&dc->priv->comp);
- return IRQ_HANDLED;
-}
-
void ipu_dc_disable_channel(struct ipu_dc *dc)
{
- struct ipu_dc_priv *priv = dc->priv;
- int irq;
- unsigned long ret;
u32 val;
- /* TODO: Handle MEM_FG_SYNC differently from MEM_BG_SYNC */
- if (dc->chno == 1)
- irq = priv->dc_irq;
- else if (dc->chno == 5)
- irq = priv->dp_irq;
- else
- return;
-
- init_completion(&priv->comp);
- enable_irq(irq);
- ret = wait_for_completion_timeout(&priv->comp, msecs_to_jiffies(50));
- disable_irq(irq);
- if (ret == 0) {
- dev_warn(priv->dev, "DC stop timeout after 50 ms\n");
-
- val = readl(dc->base + DC_WR_CH_CONF);
- val &= ~DC_WR_CH_CONF_PROG_TYPE_MASK;
- writel(val, dc->base + DC_WR_CH_CONF);
- }
+ val = readl(dc->base + DC_WR_CH_CONF);
+ val &= ~DC_WR_CH_CONF_PROG_TYPE_MASK;
+ writel(val, dc->base + DC_WR_CH_CONF);
}
EXPORT_SYMBOL_GPL(ipu_dc_disable_channel);
@@ -389,7 +353,7 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
struct ipu_dc_priv *priv;
static int channel_offsets[] = { 0, 0x1c, 0x38, 0x54, 0x58, 0x5c,
0x78, 0, 0x94, 0xb4};
- int i, ret;
+ int i;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -410,23 +374,6 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
priv->channels[i].base = priv->dc_reg + channel_offsets[i];
}
- priv->dc_irq = ipu_map_irq(ipu, IPU_IRQ_DC_FC_1);
- if (!priv->dc_irq)
- return -EINVAL;
- ret = devm_request_irq(dev, priv->dc_irq, dc_irq_handler, 0, NULL,
- &priv->channels[1]);
- if (ret < 0)
- return ret;
- disable_irq(priv->dc_irq);
- priv->dp_irq = ipu_map_irq(ipu, IPU_IRQ_DP_SF_END);
- if (!priv->dp_irq)
- return -EINVAL;
- ret = devm_request_irq(dev, priv->dp_irq, dc_irq_handler, 0, NULL,
- &priv->channels[5]);
- if (ret < 0)
- return ret;
- disable_irq(priv->dp_irq);
-
writel(DC_WR_CH_CONF_WORD_SIZE_24 | DC_WR_CH_CONF_DISP_ID_PARALLEL(1) |
DC_WR_CH_CONF_PROG_DI_ID,
priv->channels[1].base + DC_WR_CH_CONF);
--
2.10.1
[PATCH 1/2] drm/imx: disable planes before DC
If the DC clock is disabled before the attached IDMACs are properly stopped the IDMACs may hang the IPU or even the whole system. Make sure the IDMACs are in safe state by disabling the planes before removal of the DC clock. Fixes: 33f14235302f (drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers) Signed-off-by: Lucas Stach --- drivers/gpu/drm/imx/ipuv3-crtc.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c index 4e1ae3fc462d..6be515a9fb69 100644 --- a/drivers/gpu/drm/imx/ipuv3-crtc.c +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c @@ -68,6 +68,12 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc, ipu_dc_disable_channel(ipu_crtc->dc); ipu_di_disable(ipu_crtc->di); + /* +* Planes must be disabled before DC clock is removed, as otherwise the +* attached IDMACs will be left in undefined state, possibly hanging +* the IPU or even system. +*/ + drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, false); ipu_dc_disable(ipu); spin_lock_irq(&crtc->dev->event_lock); @@ -77,9 +83,6 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc, } spin_unlock_irq(&crtc->dev->event_lock); - /* always disable planes on the CRTC */ - drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, true); - drm_crtc_vblank_off(crtc); } -- 2.10.1
[PATCH] gpu: ipu-di: silence videomode logspam
Adapting the videomode to the hardware constraints is something that can and must happen during normal operation and isn't something that the user can avoid. So printing a warning each time it happens isn't helpful. Demote this message to the debug level. Signed-off-by: Lucas Stach --- drivers/gpu/ipu-v3/ipu-di.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c index 92183ab19569..13cef8fa358f 100644 --- a/drivers/gpu/ipu-v3/ipu-di.c +++ b/drivers/gpu/ipu-v3/ipu-di.c @@ -535,7 +535,7 @@ int ipu_di_adjust_videomode(struct ipu_di *di, struct videomode *mode) return -EINVAL; } - dev_warn(di->ipu->dev, "videomode adapted for IPU restrictions\n"); + dev_dbg(di->ipu->dev, "videomode adapted for IPU restrictions\n"); return 0; } EXPORT_SYMBOL_GPL(ipu_di_adjust_videomode); -- 2.10.1
[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info
On 08-11-2016 15:57, Emil Velikov wrote:
> On 8 November 2016 at 15:27, Mauro Santos
> wrote:
>> On 08-11-2016 15:00, Emil Velikov wrote:
>>> On 8 November 2016 at 13:38, Mauro Santos
>>> wrote:
On 08-11-2016 11:06, Emil Velikov wrote:
> On 1 November 2016 at 18:47, Mauro Santos
> wrote:
>> On 01-11-2016 18:13, Emil Velikov wrote:
>>> From: Emil Velikov
>>>
>>> Parsing config sysfs file wakes up the device. The latter of which may
>>> be slow and isn't required to begin with.
>>>
>>> Reading through config is/was required since the revision is not
>>> available by other means, although with a kernel patch in the way we can
>>> 'cheat' temporarily.
>>>
>>> That should be fine, since no open-source project has ever used the
>>> value.
>>>
>>> Cc: Michel Dänzer
>>> Cc: Mauro Santos
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
>>> Signed-off-by: Emil Velikov
>>> ---
>>> Mauro can you apply this against libdrm and rebuild it. You do _not_
>>> need to rebuild mesa afterwords.
>>>
>>> Thanks
>>> ---
>>> xf86drm.c | 50 +++---
>>> 1 file changed, 35 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/xf86drm.c b/xf86drm.c
>>> index 52add5e..5a5100c 100644
>>> --- a/xf86drm.c
>>> +++ b/xf86drm.c
>>> @@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char
>>> *d_name,
>>> drmPciDeviceInfoPtr device)
>>> {
>>> #ifdef __linux__
>>> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
>>> +static const char *attrs[] = {
>>> + "revision", /* XXX: make sure it's always first, see note below
>>> */
>>> + "vendor",
>>> + "device",
>>> + "subsystem_vendor",
>>> + "subsystem_device",
>>> +};
>>> char path[PATH_MAX + 1];
>>> -unsigned char config[64];
>>> -int fd, ret;
>>> +unsigned int data[ARRAY_SIZE(attrs)];
>>> +FILE *fp;
>>> +int ret;
>>>
>>> -snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config",
>>> d_name);
>>> -fd = open(path, O_RDONLY);
>>> -if (fd < 0)
>>> -return -errno;
>>> +for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) {
>>> +snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s",
>>> + d_name, attrs[i]);
>>> +fp = fopen(path, "r");
>>> +if (!fp) {
>>> +/* Note: First we check the revision, since older kernels
>>> + * may not have it. Default to zero in such cases. */
>>> +if (i == 0) {
>>> +data[i] = 0;
>>> +continue;
>>> +}
>>> +return -errno;
>>> +}
>>>
>>> -ret = read(fd, config, sizeof(config));
>>> -close(fd);
>>> -if (ret < 0)
>>> -return -errno;
>>> +ret = fscanf(fp, "%x", &data[i]);
>>> +fclose(fp);
>>> +if (ret != 1)
>>> +return -errno;
>>> +
>>> +}
>>>
>>> -device->vendor_id = config[0] | (config[1] << 8);
>>> -device->device_id = config[2] | (config[3] << 8);
>>> -device->revision_id = config[8];
>>> -device->subvendor_id = config[44] | (config[45] << 8);
>>> -device->subdevice_id = config[46] | (config[47] << 8);
>>> +device->revision_id = data[0] & 0xff;
>>> +device->vendor_id = data[1] & 0x;
>>> +device->device_id = data[2] & 0x;
>>> +device->subvendor_id = data[3] & 0x;
>>> +device->subdevice_id = data[4] & 0x;
>>>
>>> return 0;
>>> #else
>>>
>>
>> I have applied this against libdrm 2.4.71 and I don't see any delays
>> when starting firefox/chromium/thunderbird/glxgears.
>>
>> There is also no indication in dmesg that the dGPU is being
>> reinitialized when starting the programs where I've detected the problem.
>>
> Thanks Mauro. Can you give this a try alongside the kernel fix [1] ?
> I'd love to get the latter merged soon(ish).
> Independent of the kernel side, I might need to go another way for
> libdrm/mesa so I'll CC you on future patches.
>
> Your help is greatly appreciated !
>
> Thanks
> Emil
>
> [1] http://patchwork.ozlabs.org/patch/689975/
>
I have applied the patch on top of kernel 4.8.6 and I'm using the libdrm
with the patch you sent me previously.
With this patch things still seem work fine, I don't see any of the
problems I've seen before, but I don't know how to confirm that the
value from sysfs is now being used by libdrm instead of defaulting to zero.
>>> Grr my bad. $libdrm_builddir/tests/drmdevice should do i
[GIT PULL] bridge/dw-hdmi: I2C master controller support
Hi Dave, On 2016-09-19 00:16, Philipp Zabel wrote: > Hi Dave, > > this tag contains support for the I2C master controller contained in the > HDMI TX IP core, for those boards that don't allow to mux their DDC pins > to SoC I2C controllers. This will make the dw-hdmi driver register its > internal I2C master if the ddc-i2c-bus property is not set on the device > tree node. This did not get merged so far, any specific reason? We have some device tree changes building on-top of that, so it would nice if it makes it into 4.10... -- Stefan
[PATCH] drm/nouveau: fix LEDS_CLASS=m configuration
On Tuesday, November 8, 2016 10:46:07 AM CET Ilia Mirkin wrote: > > diff --git a/drivers/gpu/drm/nouveau/Kconfig > > b/drivers/gpu/drm/nouveau/Kconfig > > index 78631fb61adf..715cd6f4dc31 100644 > > --- a/drivers/gpu/drm/nouveau/Kconfig > > +++ b/drivers/gpu/drm/nouveau/Kconfig > > @@ -46,6 +46,14 @@ config NOUVEAU_DEBUG > > The paranoia and spam levels will add a lot of extra checks which > > may potentially slow down driver operation. > > > > +config DRM_NOUVEAU_LED > > + bool "Support for logo LED" > > + depends on DRM_NOUVEAU && LEDS_CLASS > > + depends on !(DRM_NOUVEAU=y && LEDS_CLASS=m) > > + help > > + Say Y here to enabling controlling the brightness of the > > + LED behind NVIDIA logo on certain Titan cards. > > This is a very odd restriction... could this be written as > > depends on DRM_NOUVEAU > select LEDS_CLASS > > Or will that not flip the LEDS_CLASS from m to y in case > DRM_NOUVEAU=y? If not, is there a way to cause that to happen? Using 'select' on user-selectable symbols risks introducing circular dependencies. > Separately, perhaps we should just drop this LEDS_CLASS select into > DRM_NOUVEAU? We've tended to avoid adding tons of options. My first attempt was to add "depends on LEDS_CLASS || !LEDS_CLASS" to DRM_NOUVEAU, which led to one long circle: drivers/usb/Kconfig:39:error: recursive dependency detected! For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/usb/Kconfig:39: symbol USB is selected by MOUSE_APPLETOUCH For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/input/mouse/Kconfig:187:symbol MOUSE_APPLETOUCH depends on INPUT For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/input/Kconfig:8:symbol INPUT is selected by VT For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/tty/Kconfig:12: symbol VT is selected by FB_STI For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:669:symbol FB_STI depends on FB For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:42: symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/Kconfig:36: symbol DRM_KMS_HELPER is selected by DRM_NOUVEAU For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/gpu/drm/nouveau/Kconfig:1: symbol DRM_NOUVEAU depends on LEDS_CLASS For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/leds/Kconfig:16:symbol LEDS_CLASS is selected by OMAP_DEBUG_LEDS For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" arch/arm/plat-omap/Kconfig:19: symbol OMAP_DEBUG_LEDS depends on NEW_LEDS For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/leds/Kconfig:8: symbol NEW_LEDS is selected by ATH9K_HTC For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/net/wireless/ath/ath9k/Kconfig:158: symbol ATH9K_HTC depends on USB Tue, 08 Nov 2016 11:49:44 +0100 build/0x3053A542_defconfig warnings drivers/usb/Kconfig:39:error: recursive dependency detected! For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/usb/Kconfig:39: symbol USB is selected by MOUSE_APPLETOUCH For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/input/mouse/Kconfig:187:symbol MOUSE_APPLETOUCH depends on INPUT For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/input/Kconfig:8:symbol INPUT is selected by VT For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/tty/Kconfig:12: symbol VT is selected by FB_STI For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" drivers/video/fbdev/Kconfig:669
[PATCH] drm/tegra: add tiling FB modifiers
Add FB modifiers to allow user-space to specify that a surface is in one
of the two tiling formats supported by Tegra chips, and add support in
the tegradrm driver to handle them properly. This is necessary for the
display controller to directly display buffers generated by the GPU.
This feature is intended to replace the dedicated IOCTL enabled
by TEGRA_STAGING and to provide a non-staging alternative to that
solution.
Signed-off-by: Alexandre Courbot
---
drivers/gpu/drm/tegra/drm.c | 2 ++
drivers/gpu/drm/tegra/fb.c| 23 +++---
include/uapi/drm/drm_fourcc.h | 45 +++
3 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index a9630c2d6cb3..36b4b30a5164 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -161,6 +161,8 @@ static int tegra_drm_load(struct drm_device *drm, unsigned
long flags)
drm->mode_config.max_width = 4096;
drm->mode_config.max_height = 4096;
+ drm->mode_config.allow_fb_modifiers = true;
+
drm->mode_config.funcs = &tegra_drm_mode_funcs;
err = tegra_drm_fb_prepare(drm);
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index e6d71fa4028e..2fded58b2ca5 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -52,9 +52,26 @@ int tegra_fb_get_tiling(struct drm_framebuffer *framebuffer,
struct tegra_bo_tiling *tiling)
{
struct tegra_fb *fb = to_tegra_fb(framebuffer);
-
- /* TODO: handle YUV formats? */
- *tiling = fb->planes[0]->tiling;
+ uint64_t modifier = fb->base.modifier[0];
+
+ switch (fourcc_mod_tegra_mod(modifier)) {
+ case NV_FORMAT_MOD_TEGRA_TILED:
+ tiling->mode = TEGRA_BO_TILING_MODE_TILED;
+ tiling->value = 0;
+ break;
+
+ case NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(0):
+ tiling->mode = TEGRA_BO_TILING_MODE_BLOCK;
+ tiling->value = fourcc_mod_tegra_param(modifier);
+ if (tiling->value > 5)
+ return -EINVAL;
+ break;
+
+ default:
+ /* TODO: handle YUV formats? */
+ *tiling = fb->planes[0]->tiling;
+ break;
+ }
return 0;
}
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index a5890bf44c0a..967dfab16881 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -233,6 +233,51 @@ extern "C" {
*/
#define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE fourcc_mod_code(SAMSUNG, 1)
+
+/* NVIDIA Tegra frame buffer modifiers */
+
+/*
+ * Some modifiers take parameters, for example the number of vertical GOBs in
+ * a block. Reserve the lower 32 bits for parameters
+ */
+#define __fourcc_mod_tegra_mode_shift 32
+#define fourcc_mod_tegra_code(val, params) \
+ fourcc_mod_code(NV, __u64)val) << __fourcc_mod_tegra_mode_shift) |
params))
+#define fourcc_mod_tegra_mod(m) \
+ (m & ~((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
+#define fourcc_mod_tegra_param(m) \
+ (m & ((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
+
+/*
+ * Tegra Tiled Layout, used by Tegra 2, 3 and 4.
+ *
+ * Pixels are arranged in simple tiles of 16 x 16 bytes.
+ */
+#define NV_FORMAT_MOD_TEGRA_TILED fourcc_mod_tegra_code(1, 0)
+
+/*
+ * Tegra 16Bx2 Block Linear layout, used by TK1/TX1
+ *
+ * Pixels are arranged in 64x8 Groups Of Bytes (GOBs). GOBs are then stacked
+ * vertically by a power of 2 (1 to 32 GOBs) to form a block.
+ *
+ * Within a GOB, data is ordered as 16B x 2 lines sectors laid in Z-shape.
+ *
+ * Parameter 'v' is the log2 encoding of the number of GOBs stacked vertically.
+ * Valid values are:
+ *
+ * 0 == ONE_GOB
+ * 1 == TWO_GOBS
+ * 2 == FOUR_GOBS
+ * 3 == EIGHT_GOBS
+ * 4 == SIXTEEN_GOBS
+ * 5 == THIRTYTWO_GOBS
+ *
+ * Chapter 20 "Pixel Memory Formats" of the Tegra X1 TRM describes this format
+ * in full detail.
+ */
+#define NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(v) fourcc_mod_tegra_code(2, v)
+
#if defined(__cplusplus)
}
#endif
--
2.10.2
[PATCH v2] drm/udl: Ensure channel is selected before using the device.
On 8 November 2016 at 16:40, Dave Airlie wrote: >> [drm:udl_driver_load [udl]] *ERROR* Selecting channel failed >> udl 1-2:1.0: fb1: udldrmfb frame buffer device >> [drm] Initialized udl on minor 1 >> usbcore: registered new interface driver udl >> >> >> Is this expected WARN? > > I've just posted a patch in theory to fix it, please test if oyu have time. Actually I posted a v2 of it, because the first one didn't work either. Dave.
[PATCH] drm/udl: make control msg static const. (v2)
From: Dave Airlie
Thou shall not send control msg from the stack,
does that mean I can send it from the RO memory area?
and it looks like the answer is no, so here's
v2 which kmemdups.
Reported-by: poma
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/udl/udl_main.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 29f0207..873f010 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -98,17 +98,23 @@ static int udl_parse_vendor_descriptor(struct drm_device
*dev,
static int udl_select_std_channel(struct udl_device *udl)
{
int ret;
- u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
- 0x1C, 0x88, 0x5E, 0x15,
- 0x60, 0xFE, 0xC6, 0x97,
- 0x16, 0x3D, 0x47, 0xF2};
+ static const u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
+0x1C, 0x88, 0x5E, 0x15,
+0x60, 0xFE, 0xC6, 0x97,
+0x16, 0x3D, 0x47, 0xF2};
+ void *sendbuf;
+
+ sendbuf = kmemdup(set_def_chn, sizeof(set_def_chn), GFP_KERNEL);
+ if (!sendbuf)
+ return -ENOMEM;
ret = usb_control_msg(udl->udev,
usb_sndctrlpipe(udl->udev, 0),
NR_USB_REQUEST_CHANNEL,
(USB_DIR_OUT | USB_TYPE_VENDOR), 0, 0,
- set_def_chn, sizeof(set_def_chn),
+ sendbuf, sizeof(set_def_chn),
USB_CTRL_SET_TIMEOUT);
+ kfree(sendbuf);
return ret < 0 ? ret : 0;
}
--
2.5.5
[PATCH] drm: mali-dp: Add support for setting plane's rotation property from userspace.
On Tue, Nov 08, 2016 at 05:16:41PM +0100, Daniel Vetter wrote:
> On Tue, Nov 08, 2016 at 01:43:25PM +, Liviu Dudau wrote:
> > In order to support DRM_IOCTL_MODE_OBJ_SETPROPERTY for the rotation property
> > we need to have a ->set_property hook defined for the planes. Set the
> > plane's ->set_property hook to drm_atomic_helper_plane_set_property()
> >
> > Signed-off-by: Liviu Dudau
> > ---
> > drivers/gpu/drm/arm/malidp_planes.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/arm/malidp_planes.c
> > b/drivers/gpu/drm/arm/malidp_planes.c
> > index bb1c528..8eef9a8 100644
> > --- a/drivers/gpu/drm/arm/malidp_planes.c
> > +++ b/drivers/gpu/drm/arm/malidp_planes.c
> > @@ -89,6 +89,7 @@ void malidp_destroy_plane_state(struct drm_plane *plane,
> > static const struct drm_plane_funcs malidp_de_plane_funcs = {
> > .update_plane = drm_atomic_helper_update_plane,
> > .disable_plane = drm_atomic_helper_disable_plane,
> > + .set_property = drm_atomic_helper_plane_set_property,
>
> We discussed this a bit, I think it'd be nice to simply make this the
> default for all atomic drivers (and demote all the set_prop helpers from
> exported helpers to internal static functions). But can be done as a
> cleanup later on, ack for this patch meanwhile.
Thanks for the ACK! Yes, I will work on the cleanup and fix all other affected
drivers as well, this is my attempt to get the patches into v4.9 ;)
Best regards,
Liviu
> -Daniel
>
> > .destroy = malidp_de_plane_destroy,
> > .reset = drm_atomic_helper_plane_reset,
> > .atomic_duplicate_state = malidp_duplicate_plane_state,
> > --
> > 2.10.0
> >
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ã)_/¯
[PATCH v2] drm/udl: Ensure channel is selected before using the device.
> [drm:udl_driver_load [udl]] *ERROR* Selecting channel failed > udl 1-2:1.0: fb1: udldrmfb frame buffer device > [drm] Initialized udl on minor 1 > usbcore: registered new interface driver udl > > > Is this expected WARN? I've just posted a patch in theory to fix it, please test if oyu have time. Dave.
[PATCH 0/7] drm: add atomic state logging and debugfs
On Sat, Nov 5, 2016 at 11:08 AM, Rob Clark wrote: > I realized that I had not re-sent this after updating from review > comments, and adding kerneldoc. > > The drm/msm bits I can include in my msm-next pull-req for 4.10. Just > including them here to show example usage. > > There will be a minor conflict to resolve around drm_get_format_name(), > depending on what the final solution there is. But that should be > trivial. If needed I can rebase after that lands. But would be nice > if this ended up in drm-next for 4.10 so that I can land the drm/msm > bits (and some later patches that use drm_printer to dump SMP state > in debugfs and on error irqs) > > Rob Clark (7): > drm: helper macros to print composite types > drm: add helper for printing to log or seq_file > drm: add helpers to go from plane state to drm_rect > drm/atomic: add new drm_debug bit to dump atomic state > drm/atomic: add debugfs file to dump out atomic state > drm/msm/mdp5: add atomic_print_state support > drm/msm: module param to dump state on error irq > Applied the series to drm-misc, thanks Sean > Documentation/gpu/drm-internals.rst | 17 > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/drm_atomic.c | 156 > ++ > drivers/gpu/drm/drm_debugfs.c | 9 ++ > drivers/gpu/drm/drm_modes.c | 8 +- > drivers/gpu/drm/drm_plane_helper.c| 11 +-- > drivers/gpu/drm/drm_print.c | 54 +++ > drivers/gpu/drm/drm_rect.c| 11 +-- > drivers/gpu/drm/i915/intel_display.c | 10 +- > drivers/gpu/drm/i915/intel_sprite.c | 11 +-- > drivers/gpu/drm/msm/mdp/mdp4/mdp4_irq.c | 10 ++ > drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c | 11 +++ > drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 12 +++ > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 18 +++- > drivers/gpu/drm/msm/msm_drv.c | 4 + > include/drm/drmP.h| 22 + > include/drm/drm_atomic.h | 7 ++ > include/drm/drm_connector.h | 13 +++ > include/drm/drm_crtc.h| 13 +++ > include/drm/drm_plane.h | 36 +++ > include/drm/drm_print.h | 117 ++ > 21 files changed, 508 insertions(+), 44 deletions(-) > create mode 100644 drivers/gpu/drm/drm_print.c > create mode 100644 include/drm/drm_print.h > > -- > 2.7.4 >
[PATCH] drm/udl: make control msg static const.
From: Dave Airlie
Thou shall not send control msg from the stack,
does that mean I can send it from the RO memory area?
Reported-by: poma
Signed-off-by: Dave Airlie
---
drivers/gpu/drm/udl/udl_main.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index 29f0207..0798bcc 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -98,10 +98,10 @@ static int udl_parse_vendor_descriptor(struct drm_device
*dev,
static int udl_select_std_channel(struct udl_device *udl)
{
int ret;
- u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
- 0x1C, 0x88, 0x5E, 0x15,
- 0x60, 0xFE, 0xC6, 0x97,
- 0x16, 0x3D, 0x47, 0xF2};
+ static const u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
+0x1C, 0x88, 0x5E, 0x15,
+0x60, 0xFE, 0xC6, 0x97,
+0x16, 0x3D, 0x47, 0xF2};
ret = usb_control_msg(udl->udev,
usb_sndctrlpipe(udl->udev, 0),
--
2.5.5
[Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration
On 8 November 2016 at 16:21, Karol Herbst wrote:
> 2016-11-08 17:12 GMT+01:00 Arnd Bergmann :
>> On Tuesday, November 8, 2016 5:07:01 PM CET Lukas Wunner wrote:
>>> On Tue, Nov 08, 2016 at 04:52:49PM +0100, Arnd Bergmann wrote:
>>> > The underlying problem is that we already have a number of other
>>> > symbols that either have "depends on LEDS_CLASS" or
>>> > "select LEDS_CLASS". To clean that up properly, we should either
>>> > make the symbol itself hidden and only select it from other drivers,
>>> > or use "depends on LEDS_CLASS" everywhere.
>>> >
>>> > Another option is to use the IS_REACHABLE() macro instead of IS_ENABLED()
>>> > in the header file, to stub out the calls into the new file, but
>>> > that can be a bit confusing.
>>>
>>> Why don't you just add empty inline stubs for nouveau_led_init / _fini /
>>> _suspend / _resume?
>>>
>>
>> That's what I was suggesting:
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_led.h
>> b/drivers/gpu/drm/nouveau/nouveau_led.h
>> index 9c9bb6ac938e..bc5f47cb516b 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_led.h
>> +++ b/drivers/gpu/drm/nouveau/nouveau_led.h
>> @@ -35,21 +35,21 @@ struct nouveau_led {
>> struct led_classdev led;
>> };
>>
>> static inline struct nouveau_led *
>> nouveau_led(struct drm_device *dev)
>> {
>> return nouveau_drm(dev)->led;
>> }
>>
>> /* nouveau_led.c */
>> -#if IS_ENABLED(CONFIG_LEDS_CLASS)
>> +#if IS_REACHABLE(CONFIG_LEDS_CLASS)
>> int nouveau_led_init(struct drm_device *dev);
>> void nouveau_led_suspend(struct drm_device *dev);
>> void nouveau_led_resume(struct drm_device *dev);
>> void nouveau_led_fini(struct drm_device *dev);
>> #else
>> static inline int nouveau_led_init(struct drm_device *dev) { return 0; };
>> static inline void nouveau_led_suspend(struct drm_device *dev) { };
>> static inline void nouveau_led_resume(struct drm_device *dev) { };
>> static inline void nouveau_led_fini(struct drm_device *dev) { };
>> #endif
>>
>> The downside is that now the nouveau_led_init() just won't be called
>> if CONFIG_LEDS_CLASS=m and CONFIG_DRM_NOUVEAU=y, which can be
>> surprising to users.
>
> yeah, it will. I guess it is fine to force LEDS to y if nouveau is set
> to y. The thinks I absolutely dislike is:
> 1. auto hiding of features I _want_ to have, because I would have to
> enable the dependencies first, which is like super annoying if there
> are somewhere else
> 2. preventing me from enabling something, cause the dependency is missing.
>
> We should clarify first if we actually want to enable those features
> optionally, because there isn't much of a reason not to enable the
> dependencies, except embedded systems. We have a lot more stuff where
> we could add things like that: hwmon, debugfs, acpi, compat and maybe
> there are even more things
>
Sounds like people may have missed the core part:
This/earlier patch are required due to select "abuse" elsewhere in the
kernel. The IS_REACHABLE/DRM_NOUVEAU_LED is patch to workaround things
on nouveau side, with a proper one to remove/untangle the "select
LEDS_CLASS". The latter will likely be slow/pain, since devs love to
use select because it's convenient (and indeed it is).
Thus [temporary] workaround on nouveau side will be good in the short term.
Afaict, "forcing LEDS to y if nouveau is set to y" is going the
opposite direction of what one should be going ;-)
Regards,
Emil
Enable AMDGPU for CIK by default
On 07/11/16 11:25 PM, Bridgman, John wrote: >> From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On Behalf >> Of Michel Dänzer >> Sent: Monday, November 07, 2016 2:24 AM >> On 07/11/16 03:56 AM, Sandeep wrote: >>> >>> I was wondering when DRM_AMDGPU_CIK would be turned on by default >> in >>> the upstream kernel (or is this upto individual distros?) >>> >>> Is there any work left to be done/bugs to be fixed before it can be >>> enabled by default? >> >> There are still some functional regressions, notably amdgpu doesn't support >> HDMI/DP audio yet. >> >> Also, simply enabling DRM_AMDGPU_CIK by default isn't a good solution, >> since the radeon driver also still supports the CIK devices, and there's no >> good mechanism to choose which driver gets to drive a particular GPU at >> runtime. > > Right... we would need corresponding logic to disable CIK support in radeon. > > Would we need two flags, one for each driver, or could we define a flag at > drivers/gpu/drm level which would choose between radeon and amdgpu for > CIK hardware ? Technically, there's probably nothing preventing the radeon code from using CONFIG_DRM_AMDGPU_CIK/SI as well. > Even as I type that I don't like it... so two flags I guess. It might actually be useful to have support in drivers and be able to choose one or the other at runtime. Somebody "just" needs to come up with a suitable mechanism for that choice. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info
On 8 November 2016 at 15:27, Mauro Santos wrote:
> On 08-11-2016 15:00, Emil Velikov wrote:
>> On 8 November 2016 at 13:38, Mauro Santos
>> wrote:
>>> On 08-11-2016 11:06, Emil Velikov wrote:
On 1 November 2016 at 18:47, Mauro Santos
wrote:
> On 01-11-2016 18:13, Emil Velikov wrote:
>> From: Emil Velikov
>>
>> Parsing config sysfs file wakes up the device. The latter of which may
>> be slow and isn't required to begin with.
>>
>> Reading through config is/was required since the revision is not
>> available by other means, although with a kernel patch in the way we can
>> 'cheat' temporarily.
>>
>> That should be fine, since no open-source project has ever used the
>> value.
>>
>> Cc: Michel Dänzer
>> Cc: Mauro Santos
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
>> Signed-off-by: Emil Velikov
>> ---
>> Mauro can you apply this against libdrm and rebuild it. You do _not_
>> need to rebuild mesa afterwords.
>>
>> Thanks
>> ---
>> xf86drm.c | 50 +++---
>> 1 file changed, 35 insertions(+), 15 deletions(-)
>>
>> diff --git a/xf86drm.c b/xf86drm.c
>> index 52add5e..5a5100c 100644
>> --- a/xf86drm.c
>> +++ b/xf86drm.c
>> @@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char
>> *d_name,
>> drmPciDeviceInfoPtr device)
>> {
>> #ifdef __linux__
>> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
>> +static const char *attrs[] = {
>> + "revision", /* XXX: make sure it's always first, see note below */
>> + "vendor",
>> + "device",
>> + "subsystem_vendor",
>> + "subsystem_device",
>> +};
>> char path[PATH_MAX + 1];
>> -unsigned char config[64];
>> -int fd, ret;
>> +unsigned int data[ARRAY_SIZE(attrs)];
>> +FILE *fp;
>> +int ret;
>>
>> -snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config", d_name);
>> -fd = open(path, O_RDONLY);
>> -if (fd < 0)
>> -return -errno;
>> +for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) {
>> +snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s",
>> + d_name, attrs[i]);
>> +fp = fopen(path, "r");
>> +if (!fp) {
>> +/* Note: First we check the revision, since older kernels
>> + * may not have it. Default to zero in such cases. */
>> +if (i == 0) {
>> +data[i] = 0;
>> +continue;
>> +}
>> +return -errno;
>> +}
>>
>> -ret = read(fd, config, sizeof(config));
>> -close(fd);
>> -if (ret < 0)
>> -return -errno;
>> +ret = fscanf(fp, "%x", &data[i]);
>> +fclose(fp);
>> +if (ret != 1)
>> +return -errno;
>> +
>> +}
>>
>> -device->vendor_id = config[0] | (config[1] << 8);
>> -device->device_id = config[2] | (config[3] << 8);
>> -device->revision_id = config[8];
>> -device->subvendor_id = config[44] | (config[45] << 8);
>> -device->subdevice_id = config[46] | (config[47] << 8);
>> +device->revision_id = data[0] & 0xff;
>> +device->vendor_id = data[1] & 0x;
>> +device->device_id = data[2] & 0x;
>> +device->subvendor_id = data[3] & 0x;
>> +device->subdevice_id = data[4] & 0x;
>>
>> return 0;
>> #else
>>
>
> I have applied this against libdrm 2.4.71 and I don't see any delays
> when starting firefox/chromium/thunderbird/glxgears.
>
> There is also no indication in dmesg that the dGPU is being
> reinitialized when starting the programs where I've detected the problem.
>
Thanks Mauro. Can you give this a try alongside the kernel fix [1] ?
I'd love to get the latter merged soon(ish).
Independent of the kernel side, I might need to go another way for
libdrm/mesa so I'll CC you on future patches.
Your help is greatly appreciated !
Thanks
Emil
[1] http://patchwork.ozlabs.org/patch/689975/
>>>
>>> I have applied the patch on top of kernel 4.8.6 and I'm using the libdrm
>>> with the patch you sent me previously.
>>>
>>> With this patch things still seem work fine, I don't see any of the
>>> problems I've seen before, but I don't know how to confirm that the
>>> value from sysfs is now being used by libdrm instead of defaulting to zero.
>>>
>> Grr my bad. $libdrm_builddir/tests/drmdevice should do it. You might
>> need to explicitly build it (cd tests && make drmdevice)
>>
>
> When running drmdevice as my user it still wakes up the dGPU. The
> correct device revisions are being reported
[PATCH v7 3/3] drm/fence: add out-fences support
From: Gustavo Padovan
Support DRM out-fences by creating a sync_file with a fence for each CRTC
that sets the OUT_FENCE_PTR property.
We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
the sync_file fd back to userspace.
The sync_file and fd are allocated/created before commit, but the
fd_install operation only happens after we know that commit succeed.
v2: Comment by Rob Clark:
- Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
Comment by Daniel Vetter:
- Add clean up code for out_fences
v3: Comments by Daniel Vetter:
- create DRM_MODE_ATOMIC_EVENT_MASK
- userspace should fill out_fences_ptr with the crtc_ids for which
it wants fences back.
v4: Create OUT_FENCE_PTR properties and remove old approach.
v5: Comments by Brian Starkey:
- Remove extra fence_get() in atomic_ioctl()
- Check ret before iterating on the crtc_state
- check ret before fd_install
- set fence_state to NULL at the beginning
- check fence_state->out_fence_ptr before put_user()
- change order of fput() and put_unused_fd() on failure
- Add access_ok() check to the out_fence_ptr received
- Rebase after fence -> dma_fence rename
- Store out_fence_ptr in the drm_atomic_state
- Split crtc_setup_out_fence()
- return -1 as out_fence with TEST_ONLY flag
v6: Comments by Daniel Vetter
- Add prepare/unprepare_crtc_signaling()
- move struct drm_out_fence_state to drm_atomic.c
- mark get_crtc_fence() as static
Comments by Brian Starkey
- proper set fence_ptr fence_state array
- isolate fence_idx increment
- improve error handling
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/drm_atomic.c | 233 +++
drivers/gpu/drm/drm_crtc.c | 8 ++
include/drm/drm_atomic.h | 1 +
include/drm/drm_crtc.h | 7 +-
4 files changed, 206 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index d1ae463..9a7d84c 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -289,6 +289,25 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
}
EXPORT_SYMBOL(drm_atomic_get_crtc_state);
+static void
+drm_atomic_set_out_fence_for_crtc(struct drm_atomic_state *state,
+ struct drm_crtc *crtc, u64 __user *fence_ptr)
+{
+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
+}
+
+static u64 __user *
+drm_atomic_get_out_fence_for_crtc(struct drm_atomic_state *state,
+struct drm_crtc *crtc)
+{
+ u64 __user *fence_ptr;
+
+ fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
+
+ return fence_ptr;
+}
+
/**
* drm_atomic_set_mode_for_crtc - set mode for CRTC
* @state: the CRTC whose incoming state to update
@@ -490,6 +509,12 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
+ } else if (property == config->prop_out_fence_ptr) {
+ uint64_t __user *fence_ptr = u64_to_user_ptr(val);
+ if (!access_ok(VERIFY_WRITE, fence_ptr, sizeof(*fence_ptr)))
+ return -EFAULT;
+
+ drm_atomic_set_out_fence_for_crtc(state->state, crtc,
fence_ptr);
} else if (crtc->funcs->atomic_set_property)
return crtc->funcs->atomic_set_property(crtc, state, property,
val);
else
@@ -532,6 +557,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = (state->ctm) ? state->ctm->base.id : 0;
else if (property == config->gamma_lut_property)
*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
+ else if (property == config->prop_out_fence_ptr)
+ *val = 0;
else if (crtc->funcs->atomic_get_property)
return crtc->funcs->atomic_get_property(crtc, state, property,
val);
else
@@ -1506,11 +1533,9 @@ EXPORT_SYMBOL(drm_atomic_nonblocking_commit);
*/
static struct drm_pending_vblank_event *create_vblank_event(
- struct drm_device *dev, struct drm_file *file_priv,
- struct dma_fence *fence, uint64_t user_data)
+ struct drm_device *dev, uint64_t user_data)
{
struct drm_pending_vblank_event *e = NULL;
- int ret;
e = kzalloc(sizeof *e, GFP_KERNEL);
if (!e)
@@ -1520,17 +1545,6 @@ static struct drm_pending_vblank_event
*create_vblank_event(
e->event.base.length = sizeof(e->event);
e->event.user_data = user_data;
- if (file_priv) {
- ret = drm_event_reserve_init(dev, file_priv, &e->base,
-&e->event.base);
-
[PATCH v7 2/3] drm/fence: add fence timeline to drm_crtc
From: Gustavo Padovan
Create one timeline context for each CRTC to be able to handle out-fences
and signal them. It adds a few members to struct drm_crtc: fence_context,
where we store the context we get from fence_context_alloc(), the
fence seqno and the fence lock, that we pass in fence_init() to be
used by the fence.
v2: Comment by Daniel Stone:
- add BUG_ON() to fence_to_crtc() macro
v3: Comment by Ville Syrjälä
- Use more meaningful name as crtc timeline name
v4: Comments by Brian Starkey
- Use even more meaninful name for the crtc timeline
- add doc for timeline_name
Comment by Daniel Vetter
- use in-line style for comments
- rebase after fence -> dma_fence rename
v5: Comment by Daniel Vetter
- Add doc for drm_crtc_fence_ops
Signed-off-by: Gustavo Padovan
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/drm_crtc.c | 31 +++
include/drm/drm_crtc.h | 45 +
2 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 7878bfd..e2a06c8 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -165,6 +165,32 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
#endif
}
+static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->dev->driver->name;
+}
+
+static const char *drm_crtc_fence_get_timeline_name(struct dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->timeline_name;
+}
+
+static bool drm_crtc_fence_enable_signaling(struct dma_fence *fence)
+{
+ return true;
+}
+
+const struct dma_fence_ops drm_crtc_fence_ops = {
+ .get_driver_name = drm_crtc_fence_get_driver_name,
+ .get_timeline_name = drm_crtc_fence_get_timeline_name,
+ .enable_signaling = drm_crtc_fence_enable_signaling,
+ .wait = dma_fence_default_wait,
+};
+
/**
* drm_crtc_init_with_planes - Initialise a new CRTC object with
*specified primary and cursor planes.
@@ -222,6 +248,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev,
struct drm_crtc *crtc,
return -ENOMEM;
}
+ crtc->fence_context = dma_fence_context_alloc(1);
+ spin_lock_init(&crtc->fence_lock);
+ snprintf(crtc->timeline_name, sizeof(crtc->timeline_name),
+"CRTC:%d-%s", crtc->base.id, crtc->name);
+
crtc->base.properties = &crtc->properties;
list_add_tail(&crtc->head, &config->crtc_list);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 719b6a8..30f2401 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -32,6 +32,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -726,9 +728,52 @@ struct drm_crtc {
*/
struct drm_crtc_crc crc;
#endif
+
+ /**
+* @fence_context:
+*
+* timeline context used for fence operations.
+*/
+ unsigned int fence_context;
+
+ /**
+* @fence_lock:
+*
+* spinlock to protect the fences in the fence_context.
+*/
+
+ spinlock_t fence_lock;
+ /**
+* @fence_seqno:
+*
+* Seqno variable used as monotonic counter for the fences
+* created on the CRTC's timeline.
+*/
+ unsigned long fence_seqno;
+
+ /**
+* @timeline_name:
+*
+* The name of the CRTC's fence timeline.
+*/
+ char timeline_name[32];
};
/**
+ * dma_crtc_fence_ops - fence ops for the drm_crtc timeline
+ *
+ * It contains the dma_fence_ops that should be called by the dma_fence
+ * code. CRTC core should use this ops when initializing fences.
+ */
+extern const struct dma_fence_ops drm_crtc_fence_ops;
+
+static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
+{
+ BUG_ON(fence->ops != &drm_crtc_fence_ops);
+ return container_of(fence->lock, struct drm_crtc, fence_lock);
+}
+
+/**
* struct drm_mode_set - new values for a CRTC config change
* @fb: framebuffer to use for new config
* @crtc: CRTC whose configuration we're about to change
--
2.5.5
[PATCH v7 1/3] drm/fence: add in-fences support
From: Gustavo Padovan
There is now a new property called IN_FENCE_FD attached to every plane
state that receives sync_file fds from userspace via the atomic commit
IOCTL.
The fd is then translated to a fence (that may be a fence_array
subclass or just a normal fence) and then used by DRM to fence_wait() for
all fences in the sync_file to signal. So it only commits when all
framebuffers are ready to scanout.
v2: Comments by Daniel Vetter:
- remove set state->fence = NULL in destroy phase
- accept fence -1 as valid and just return 0
- do not call fence_get() - sync_file_fences_get() already calls it
- fence_put() if state->fence is already set, in case userspace
set the property more than once.
v3: WARN_ON if fence is set but state has no FB
v4: Comment from Maarten Lankhorst
- allow set fence with no related fb
v5: rename FENCE_FD to IN_FENCE_FD
v6: Comments by Daniel Vetter:
- rename plane_state->in_fence back to "fence"
- re-introduce WARN_ON if fence set but no fb
- rebase after fence -> dma_fence rename
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/drm_atomic.c| 14 ++
drivers/gpu/drm/drm_atomic_helper.c | 3 +++
drivers/gpu/drm/drm_crtc.c | 6 ++
drivers/gpu/drm/drm_plane.c | 1 +
include/drm/drm_crtc.h | 5 +
6 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 483059a..43cb33d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -12,6 +12,7 @@ menuconfig DRM
select I2C
select I2C_ALGOBIT
select DMA_SHARED_BUFFER
+ select SYNC_FILE
help
Kernel-level support for the Direct Rendering Infrastructure (DRI)
introduced in XFree86 4.0. If you say Y here, you need to select
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5e73954..d1ae463 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include "drm_crtc_internal.h"
@@ -686,6 +687,17 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
drm_atomic_set_fb_for_plane(state, fb);
if (fb)
drm_framebuffer_unreference(fb);
+ } else if (property == config->prop_in_fence_fd) {
+ if (U642I64(val) == -1)
+ return 0;
+
+ if (state->fence)
+ dma_fence_put(state->fence);
+
+ state->fence = sync_file_get_fence(val);
+ if (!state->fence)
+ return -EINVAL;
+
} else if (property == config->prop_crtc_id) {
struct drm_crtc *crtc = drm_crtc_find(dev, val);
return drm_atomic_set_crtc_for_plane(state, crtc);
@@ -747,6 +759,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
if (property == config->prop_fb_id) {
*val = (state->fb) ? state->fb->base.id : 0;
+ } else if (property == config->prop_in_fence_fd) {
+ *val = -1;
} else if (property == config->prop_crtc_id) {
*val = (state->crtc) ? state->crtc->base.id : 0;
} else if (property == config->prop_crtc_x) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 75ad01d..26a067f 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3114,6 +3114,9 @@ void __drm_atomic_helper_plane_destroy_state(struct
drm_plane_state *state)
{
if (state->fb)
drm_framebuffer_unreference(state->fb);
+
+ if (state->fence)
+ dma_fence_put(state->fence);
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 13441e2..7878bfd 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -397,6 +397,12 @@ static int drm_mode_create_standard_properties(struct
drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_fb_id = prop;
+ prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC,
+ "IN_FENCE_FD", -1, INT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.prop_in_fence_fd = prop;
+
prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
"CRTC_ID", DRM_MODE_OBJECT_CRTC);
if (!prop)
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 249c0ae..3957ef8 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -137,6 +137,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct
drm_plane *plane,
if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
drm_object_attach_property(&plane->ba
[PATCH v7 0/3] drm: add explict fencing
From: Gustavo Padovan Hi, This is yet another version of the DRM fences patches. Please refer to the cover letter[1] in a previous version to check for more details. In v7 we now have split most of the out_fences code into prepare_crtc_signaling() and unprepare_crtc_signaling() with improved error handling. More details on the v7 changes are embedded in each commit's message. Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and added support to fences. Current patches can be seen here: https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1 He managed to run AOSP on top of padovan/fences kernel branch with full fence support on qemu/virgl and msm db410c. That means we already have a working open source userspace using the explicit fencing implementation. Also i-g-t testing are available at: https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/ Please review! Gustavo [1] https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1253822.html --- Gustavo Padovan (3): drm/fence: add in-fences support drm/fence: add fence timeline to drm_crtc drm/fence: add out-fences support drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_atomic.c| 247 ++-- drivers/gpu/drm/drm_atomic_helper.c | 3 + drivers/gpu/drm/drm_crtc.c | 45 +++ drivers/gpu/drm/drm_plane.c | 1 + include/drm/drm_atomic.h| 1 + include/drm/drm_crtc.h | 55 7 files changed, 311 insertions(+), 42 deletions(-) -- 2.5.5
[PATCH RFC 00/12] tda998x updates
On Tue, Nov 08, 2016 at 03:25:45PM +, Robin Murphy wrote: > On 08/11/16 13:34, Russell King - ARM Linux wrote: > > On Tue, Nov 08, 2016 at 01:32:15PM +, Russell King - ARM Linux wrote: > >> Unfortunately, my drm-tda998x-devel branch is slightly out of date with > >> these patches it's the original set of 10 patches. I've not pushed > >> these ones out to that branch yet, as I've three additional patches on > >> top of these which aren't "ready" for pushing out. > > > > Here's the delta between the branch and what I just posted: > > Great, thanks. I committed that on top and my framebuffer console over > DVI is still working. I'm dubious that it's thorough enough to have > value (sadly I have neither the time nor the motivation to fight with > HDMI and I2S audio), but feel free to consider that a tested-by if you > wish. Well, "nothing obviously broken" is good enough, thanks. I don't have (or use) I2S video on my TDA998x, and that's something which is in Jyri's interest area. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
On Mon, 7 Nov 2016 23:37:41 +0100
Maxime Ripard wrote:
> Hi,
>
> On Fri, Oct 28, 2016 at 07:34:20PM +0200, Jean-Francois Moine wrote:
> > On Fri, 28 Oct 2016 00:03:16 +0200
> > Maxime Ripard wrote:
[snip]
> > > > > We've been calling them bus and mod.
> > > >
> > > > I can understand "bus" (which is better than "apb"), but why "mod"?
> > >
> > > Allwinner has been calling the clocks that are supposed to generate
> > > the external signals (depending on where you were looking) module or
> > > mod clocks (which is also why we have mod in the clock
> > > compatibles). The module 1 clocks being used for the audio and the
> > > module 0 for the rest (SPI, MMC, NAND, display, etc.)
> >
> > I did not find any 'module' in the H3 documentation.
> > So, is it really a good name?
>
> It's true that they use it less nowadays, but they still do,
> ie.
> https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/clk/sunxi/clk-sun8iw7.c#L513
There is a 'mod' suffix, but it is used for the bus gates only, not for
the main clocks.
> And we have to remain consistent anyway.
I don't see any consistency in the H3 DT:
- the bus gates are named "ahb" and apb"
- the (main) clocks are named "mmc", "usb0_phy" and "ir"
There is no "bus" nor "mod".
> > > > > > +
> > > > > > +- resets: phandle to the reset of the device
> > > > > > +
> > > > > > +- ports: phandle's to the LCD ports
> > > > >
> > > > > Please use the OF graph.
> > > >
> > > > These ports are references to the graph of nodes. See
> > > > http://www.kernelhub.org/?msg=911825&p=2
> > >
> > > In an OF-graph, your phandle to the LCD controller would be replaced
> > > by an output endpoint.
> >
> > This is the DE controller. There is no endpoint link at this level.
>
> The display engine definitely has an endpoint: the TCON.
Not at all. The video chain is simply
CRTC (TCON) -> connector (HDMI/LCD/DAC/..)
The DE is an ancillary device which handles the planes.
> > The Device Engine just handles the planes of the LCDs, but, indeed,
> > the LCDs must know about the DE and the DE must know about the LCDs.
> > There are 2 ways to realize this knowledge in the DT:
> > 1) either the DE has one or two phandle's to the LCDs,
> > 2) or the LCDs have a phandle to the DE.
> >
> > I chose the 1st way, the DE ports pointing to the endpoint of the LCDs
> > which is part of the video link (OF-graph LCD <-> connector).
> > It would be possible to have phandles to the LCDs themselves, but this
> > asks for more code.
> >
> > The second way is also possible, but it also complexifies a bit the
> > exchanges DE <-> LCD.
>
> I'm still not sure how it would complexify anything, and why you can't
> use the display graph to model the relation between the display engine
> and the TCON (and why you want to use a generic property that refers
> to the of-graph while it really isn't).
Complexification:
1- my solution:
At startup time, the DE device is the DRM device. It has to know the
devices entering in the video chains.
The CRTCs (LCD/TCON) are found by
ports[i] -> parent
The connectors are found by
ports[i] -> endpoint -> remote_endpoint -> parent
2- with ports pointing to the LCDs:
The CRTCs (LCD/TCON) are simply
ports[i]
The connectors are found by
loop on all ports of ports[i]
ports[i][j] -> endpoint -> remote_endpoint -> parent
3- with a phandle to the DE in the LCDs:
The DE cannot be the DRM device because there is no information about
the video devices in the DT. Then, the DRM devices are the LCDs.
These LCDs must give their indices to the DE. So, the DE must implement
some callback function to accept a LCD definition, and there must be
a list of DEs in the driver to make the association DE <-> LCD[i]
Some more problem may be raised if a user wants to have the same frame
buffer on the 2 LCDs of a DE.
Anyway, my solution is already used in the IMX Soc.
See 'display-subsystem' in arch/arm/boot/dts/imx6q.dtsi for an example.
> > > > > > +void de2_disable_vblank(struct drm_device *drm, unsigned crtc)
> > > > > > +{
> > > > > > + struct priv *priv = drm->dev_private;
> > > > > > + struct lcd *lcd = priv->lcds[crtc];
> > > > > > +
> > > > > > + tcon_write(lcd->mmio, gint0,
> > > > > > +tcon_read(lcd->mmio, gint0) &
> > > > > > + ~TCON_GINT0_TCON1_Vb_Int_En);
> > > > > > +}
> > > > > > +
> > > > > > +/* panel functions */
> > > > >
> > > > > Panel functions? In the CRTC driver?
> > > >
> > > > Yes, dumb panel.
> > >
> > > What do you mean by that? Using a Parallel/RGB interface?
> >
> > Sorry, I though this was a well-known name. The 'dump panel' was used
> > in the documentation of my previous ARM machine as the video frame sent
> > to the HDMI controller. 'video_frame' is OK for you?
>
> If it's the frame sent to the encoder, then it would be the CRTC by
> DRM's nomenclature.
The CRTC is a software enti
[PATCH v7 3/3] drm/fence: add out-fences support
On Tue, Nov 08, 2016 at 03:54:50PM +0900, Gustavo Padovan wrote:
>From: Gustavo Padovan
>
>+static struct dma_fence *get_crtc_fence(struct drm_crtc *crtc,
>+ struct drm_crtc_state *crtc_state)
>+{
>+ struct dma_fence *fence;
>+
>+ fence = kzalloc(sizeof(*fence), GFP_KERNEL);
>+ if (!fence)
>+ return NULL;
>+
>+ dma_fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
>+ crtc->fence_context, ++crtc->fence_seqno);
>+
>+ return fence;
>+}
>+
crtc_state is unused in this function.
-Brian
[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info
On 08-11-2016 15:00, Emil Velikov wrote:
> On 8 November 2016 at 13:38, Mauro Santos
> wrote:
>> On 08-11-2016 11:06, Emil Velikov wrote:
>>> On 1 November 2016 at 18:47, Mauro Santos
>>> wrote:
On 01-11-2016 18:13, Emil Velikov wrote:
> From: Emil Velikov
>
> Parsing config sysfs file wakes up the device. The latter of which may
> be slow and isn't required to begin with.
>
> Reading through config is/was required since the revision is not
> available by other means, although with a kernel patch in the way we can
> 'cheat' temporarily.
>
> That should be fine, since no open-source project has ever used the
> value.
>
> Cc: Michel Dänzer
> Cc: Mauro Santos
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
> Signed-off-by: Emil Velikov
> ---
> Mauro can you apply this against libdrm and rebuild it. You do _not_
> need to rebuild mesa afterwords.
>
> Thanks
> ---
> xf86drm.c | 50 +++---
> 1 file changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 52add5e..5a5100c 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char
> *d_name,
> drmPciDeviceInfoPtr device)
> {
> #ifdef __linux__
> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
> +static const char *attrs[] = {
> + "revision", /* XXX: make sure it's always first, see note below */
> + "vendor",
> + "device",
> + "subsystem_vendor",
> + "subsystem_device",
> +};
> char path[PATH_MAX + 1];
> -unsigned char config[64];
> -int fd, ret;
> +unsigned int data[ARRAY_SIZE(attrs)];
> +FILE *fp;
> +int ret;
>
> -snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config", d_name);
> -fd = open(path, O_RDONLY);
> -if (fd < 0)
> -return -errno;
> +for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) {
> +snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s",
> + d_name, attrs[i]);
> +fp = fopen(path, "r");
> +if (!fp) {
> +/* Note: First we check the revision, since older kernels
> + * may not have it. Default to zero in such cases. */
> +if (i == 0) {
> +data[i] = 0;
> +continue;
> +}
> +return -errno;
> +}
>
> -ret = read(fd, config, sizeof(config));
> -close(fd);
> -if (ret < 0)
> -return -errno;
> +ret = fscanf(fp, "%x", &data[i]);
> +fclose(fp);
> +if (ret != 1)
> +return -errno;
> +
> +}
>
> -device->vendor_id = config[0] | (config[1] << 8);
> -device->device_id = config[2] | (config[3] << 8);
> -device->revision_id = config[8];
> -device->subvendor_id = config[44] | (config[45] << 8);
> -device->subdevice_id = config[46] | (config[47] << 8);
> +device->revision_id = data[0] & 0xff;
> +device->vendor_id = data[1] & 0x;
> +device->device_id = data[2] & 0x;
> +device->subvendor_id = data[3] & 0x;
> +device->subdevice_id = data[4] & 0x;
>
> return 0;
> #else
>
I have applied this against libdrm 2.4.71 and I don't see any delays
when starting firefox/chromium/thunderbird/glxgears.
There is also no indication in dmesg that the dGPU is being
reinitialized when starting the programs where I've detected the problem.
>>> Thanks Mauro. Can you give this a try alongside the kernel fix [1] ?
>>> I'd love to get the latter merged soon(ish).
>>> Independent of the kernel side, I might need to go another way for
>>> libdrm/mesa so I'll CC you on future patches.
>>>
>>> Your help is greatly appreciated !
>>>
>>> Thanks
>>> Emil
>>>
>>> [1] http://patchwork.ozlabs.org/patch/689975/
>>>
>>
>> I have applied the patch on top of kernel 4.8.6 and I'm using the libdrm
>> with the patch you sent me previously.
>>
>> With this patch things still seem work fine, I don't see any of the
>> problems I've seen before, but I don't know how to confirm that the
>> value from sysfs is now being used by libdrm instead of defaulting to zero.
>>
> Grr my bad. $libdrm_builddir/tests/drmdevice should do it. You might
> need to explicitly build it (cd tests && make drmdevice)
>
When running drmdevice as my user it still wakes up the dGPU. The
correct device revisions are being reported by I suppose that is not
being read from sysfs.
sudo dmesg -C && dmesg -w & ./drmdevice
device[0]
available_nodes 0007
nodes
nodes[0] /dev/dri/card1
[PATCH v7 1/3] drm/fence: add in-fences support
Hi Gustavo,
On Tue, Nov 08, 2016 at 03:54:48PM +0900, Gustavo Padovan wrote:
>From: Gustavo Padovan
>
>There is now a new property called IN_FENCE_FD attached to every plane
>state that receives sync_file fds from userspace via the atomic commit
>IOCTL.
>
>The fd is then translated to a fence (that may be a fence_array
>subclass or just a normal fence) and then used by DRM to fence_wait() for
>all fences in the sync_file to signal. So it only commits when all
>framebuffers are ready to scanout.
>
>v2: Comments by Daniel Vetter:
> - remove set state->fence = NULL in destroy phase
> - accept fence -1 as valid and just return 0
> - do not call fence_get() - sync_file_fences_get() already calls it
> - fence_put() if state->fence is already set, in case userspace
> set the property more than once.
>
>v3: WARN_ON if fence is set but state has no FB
>
>v4: Comment from Maarten Lankhorst
> - allow set fence with no related fb
>
>v5: rename FENCE_FD to IN_FENCE_FD
>
>v6: Comments by Daniel Vetter:
> - rename plane_state->in_fence back to "fence"
> - re-introduce WARN_ON if fence set but no fb
>
> - rebase after fence -> dma_fence rename
>
>Signed-off-by: Gustavo Padovan
>---
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/drm_atomic.c| 14 ++
> drivers/gpu/drm/drm_atomic_helper.c | 3 +++
> drivers/gpu/drm/drm_crtc.c | 6 ++
> drivers/gpu/drm/drm_plane.c | 1 +
> include/drm/drm_crtc.h | 5 +
> 6 files changed, 30 insertions(+)
>
>diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>index 483059a..43cb33d 100644
>--- a/drivers/gpu/drm/Kconfig
>+++ b/drivers/gpu/drm/Kconfig
>@@ -12,6 +12,7 @@ menuconfig DRM
> select I2C
> select I2C_ALGOBIT
> select DMA_SHARED_BUFFER
>+ select SYNC_FILE
> help
> Kernel-level support for the Direct Rendering Infrastructure (DRI)
> introduced in XFree86 4.0. If you say Y here, you need to select
>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>index 5e73954..d1ae463 100644
>--- a/drivers/gpu/drm/drm_atomic.c
>+++ b/drivers/gpu/drm/drm_atomic.c
>@@ -30,6 +30,7 @@
> #include
> #include
> #include
>+#include
>
> #include "drm_crtc_internal.h"
>
>@@ -686,6 +687,17 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
> drm_atomic_set_fb_for_plane(state, fb);
> if (fb)
> drm_framebuffer_unreference(fb);
>+ } else if (property == config->prop_in_fence_fd) {
>+ if (U642I64(val) == -1)
>+ return 0;
>+
Sorry, I'm a bit late to bring this up but what's the expected
behaviour of a commit which sets OUT_FENCE twice (first with a valid
fence, then with -1 later in the list)? Maybe you need to actually
clear state->fence for -1.
>+ if (state->fence)
>+ dma_fence_put(state->fence);
>+
>+ state->fence = sync_file_get_fence(val);
>+ if (!state->fence)
>+ return -EINVAL;
>+
> } else if (property == config->prop_crtc_id) {
> struct drm_crtc *crtc = drm_crtc_find(dev, val);
> return drm_atomic_set_crtc_for_plane(state, crtc);
>@@ -747,6 +759,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>
> if (property == config->prop_fb_id) {
> *val = (state->fb) ? state->fb->base.id : 0;
>+ } else if (property == config->prop_in_fence_fd) {
>+ *val = -1;
> } else if (property == config->prop_crtc_id) {
> *val = (state->crtc) ? state->crtc->base.id : 0;
> } else if (property == config->prop_crtc_x) {
>diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>b/drivers/gpu/drm/drm_atomic_helper.c
>index 75ad01d..26a067f 100644
>--- a/drivers/gpu/drm/drm_atomic_helper.c
>+++ b/drivers/gpu/drm/drm_atomic_helper.c
>@@ -3114,6 +3114,9 @@ void __drm_atomic_helper_plane_destroy_state(struct
>drm_plane_state *state)
> {
> if (state->fb)
> drm_framebuffer_unreference(state->fb);
>+
>+ if (state->fence)
>+ dma_fence_put(state->fence);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
It looks like you need to add something in
__drm_atomic_helper_plane_duplicate_state() to either get a reference
on the fence or set state->fence = NULL, because the memcpy() there
will copy the pointer.
Cheers,
-Brian
>diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>index 13441e2..7878bfd 100644
>--- a/drivers/gpu/drm/drm_crtc.c
>+++ b/drivers/gpu/drm/drm_crtc.c
>@@ -397,6 +397,12 @@ static int drm_mode_create_standard_properties(struct
>drm_device *dev)
> return -ENOMEM;
> dev->mode_config.prop_fb_id = prop;
>
>+ prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC,
>+ "IN_FENCE_FD", -1, INT_MAX);
>+ if (!prop)
>+
[PATCH RFC 00/12] tda998x updates
On 08/11/16 13:34, Russell King - ARM Linux wrote:
> On Tue, Nov 08, 2016 at 01:32:15PM +, Russell King - ARM Linux wrote:
>> Unfortunately, my drm-tda998x-devel branch is slightly out of date with
>> these patches it's the original set of 10 patches. I've not pushed
>> these ones out to that branch yet, as I've three additional patches on
>> top of these which aren't "ready" for pushing out.
>
> Here's the delta between the branch and what I just posted:
Great, thanks. I committed that on top and my framebuffer console over
DVI is still working. I'm dubious that it's thorough enough to have
value (sadly I have neither the time nor the motivation to fight with
HDMI and I2S audio), but feel free to consider that a tested-by if you wish.
Robin.
>
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
> b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 1b262a89b7e1..3a5e5c466972 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -43,12 +43,12 @@ struct tda998x_priv {
> u16 rev;
> u8 current_page;
> bool is_on;
> - bool is_hdmi_sink;
> - bool is_hdmi_config;
> + bool supports_infoframes;
> + bool sink_has_audio;
> u8 vip_cntrl_0;
> u8 vip_cntrl_1;
> u8 vip_cntrl_2;
> - unsigned long tdms_clock;
> + unsigned long tmds_clock;
> struct tda998x_audio_params audio_params;
>
> struct platform_device *audio_pdev;
> @@ -774,7 +774,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
>* assume 100MHz requires larger divider.
>*/
> adiv = AUDIO_DIV_SERCLK_8;
> - if (priv->tdms_clock > 10)
> + if (priv->tmds_clock > 10)
> adiv++; /* AUDIO_DIV_SERCLK_16 */
>
> /* S/PDIF asks for a larger divider */
> @@ -869,8 +869,7 @@ static int tda998x_audio_hw_params(struct device *dev,
> void *data,
> }
>
> mutex_lock(&priv->audio_mutex);
> - /* We must not program the TDA998x for audio if the sink is DVI. */
> - if (priv->is_hdmi_config)
> + if (priv->supports_infoframes && priv->sink_has_audio)
> ret = tda998x_configure_audio(priv, &audio);
> else
> ret = 0;
> @@ -962,6 +961,27 @@ static int tda998x_connector_dpms(struct drm_connector
> *connector, int mode)
> return drm_helper_connector_dpms(connector, mode);
> }
>
> +static int tda998x_connector_fill_modes(struct drm_connector *connector,
> + uint32_t maxX, uint32_t maxY)
> +{
> + struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
> + int ret;
> +
> + mutex_lock(&priv->audio_mutex);
> + ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
> +
> + if (connector->edid_blob_ptr) {
> + struct edid *edid = (void *)connector->edid_blob_ptr->data;
> +
> + priv->sink_has_audio = drm_detect_monitor_audio(edid);
> + } else {
> + priv->sink_has_audio = false;
> + }
> + mutex_unlock(&priv->audio_mutex);
> +
> + return ret;
> +}
> +
> static enum drm_connector_status
> tda998x_connector_detect(struct drm_connector *connector, bool force)
> {
> @@ -980,7 +1000,7 @@ static void tda998x_connector_destroy(struct
> drm_connector *connector)
> static const struct drm_connector_funcs tda998x_connector_funcs = {
> .dpms = tda998x_connector_dpms,
> .reset = drm_atomic_helper_connector_reset,
> - .fill_modes = drm_helper_probe_single_connector_modes,
> + .fill_modes = tda998x_connector_fill_modes,
> .detect = tda998x_connector_detect,
> .destroy = tda998x_connector_destroy,
> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> @@ -1072,11 +1092,7 @@ static int tda998x_connector_get_modes(struct
> drm_connector *connector)
>
> drm_mode_connector_update_edid_property(connector, edid);
> n = drm_add_edid_modes(connector, edid);
> - priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
> -
> - mutex_lock(&priv->audio_mutex);
> drm_edid_to_eld(connector, edid);
> - mutex_unlock(&priv->audio_mutex);
>
> kfree(edid);
>
> @@ -1350,8 +1366,22 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
> /* must be last register set: */
> reg_write(priv, REG_TBG_CNTRL_0, 0);
>
> - /* Only setup the info frames if the sink is HDMI */
> - if (priv->is_hdmi_sink) {
> + priv->tmds_clock = adjusted_mode->clock;
> +
> + /* CEA-861B section 6 says that:
> + * CEA version 1 (CEA-861) has no support for infoframes.
> + * CEA version 2 (CEA-861A) supports version 1 AVI infoframes,
> + * and optional basic audio.
> + * CEA version 3 (CEA-861B) supports version 1 and 2 AVI infoframes,
> + * and optional digital audio, with audio infoframes.
> + *
> + * Since we only support generation of version 2 AVI infoframes,
> + * ignore CEA version 2 and below (i
linux-next: manual merge of the tip tree with the drm-intel tree
Hi all,
FIXME: Add owner of second tree to To:
Add author(s)/SOB of conflicting commits.
Today's linux-next merge of the tip tree got a conflict in:
drivers/gpu/drm/i915/i915_gem_shrinker.c
between commits:
1233e2db199d ("drm/i915: Move object backing storage manipulation to its own
locking")
from the drm-intel tree and commit:
3ab7c086d5ec ("locking/drm: Kill mutex trickery")
c7faee2109f9 ("locking/drm: Fix i915_gem_shrinker_lock() locking")
from the tip tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/gpu/drm/i915/i915_gem_shrinker.c
index a6fc1bdc48af,e9bd2a81d03a..
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@@ -35,33 -35,6 +35,15 @@@
#include "i915_drv.h"
#include "i915_trace.h"
- static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct *task)
- {
- if (!mutex_is_locked(mutex))
- return false;
-
- #if defined(CONFIG_DEBUG_MUTEXES) || defined(CONFIG_MUTEX_SPIN_ON_OWNER)
- return mutex->owner == task;
- #else
- /* Since UP may be pre-empted, we cannot assume that we own the lock */
- return false;
- #endif
- }
-
+static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
+{
- if (!mutex_trylock(&dev->struct_mutex)) {
- if (!mutex_is_locked_by(&dev->struct_mutex, current))
- return false;
-
- *unlock = false;
- } else {
- *unlock = true;
- }
++ if (!mutex_trylock(&dev->struct_mutex))
++ return false;
+
++ *unlock = true;
+ return true;
+}
+
static bool any_vma_pinned(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
[PATCH 2/2] drm/edid: Consider alternate cea timings to be the same VIC
On Tue, Nov 08, 2016 at 12:43:55PM +0100, Andrzej Hajda wrote:
> On 03.11.2016 13:53, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > CEA-861 specifies that the vertical front porch may vary by one or two
> > lines for specific VICs. Up to now we've only considered a mode to match
> > the VIC if it matched the shortest possible vertical front porch length
> > (as that is the variant we store in cea_modes[]). Let's allow our VIC
> > matching to work with the other timings variants as well so that that
> > we'll send out the correct VIC if the variant actually used isn't the
> > one with the shortest vertical front porch.
> >
> > Cc: Shashank Sharma
> > Cc: Andrzej Hajda
> > Cc: Adam Jackson
> > Signed-off-by: Ville Syrjälä
> I have checked against specification and it looks OK.
> I have few nitpicks below regarding implementation, but this could be
> matter of taste, so:
>
> Reviewed-by: Andrzej Hajda
>
>
> > ---
> > drivers/gpu/drm/drm_edid.c | 66
> > +-
> > 1 file changed, 54 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 7eec18925b70..728990fee4ef 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -2613,6 +2613,41 @@ cea_mode_alternate_clock(const struct
> > drm_display_mode *cea_mode)
> > return clock;
> > }
> >
> > +static bool
> > +cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode)
> > +{
> > + /*
> > +* For certain VICs the spec allows the vertical
> > +* front porch to vary by one or two lines.
> > +*
> > +* cea_modes[] stores the variant with the shortest
> > +* vertical front porch. We can adjust the mode to
> > +* get the other variants by simply increasing the
> > +* vertical front porch length.
> > +*/
> > + BUILD_BUG_ON(edid_cea_modes[8].vtotal != 262 ||
> > +edid_cea_modes[9].vtotal != 262 ||
> > +edid_cea_modes[12].vtotal != 262 ||
> > +edid_cea_modes[13].vtotal != 262 ||
> > +edid_cea_modes[23].vtotal != 312 ||
> > +edid_cea_modes[24].vtotal != 312 ||
> > +edid_cea_modes[27].vtotal != 312 ||
> > +edid_cea_modes[28].vtotal != 312);
>
> I am not sure if this compile check is really necessary,
> I would rather put comment before edid_cea_modes array
> which mode should be put into array in multi-mode case.
Comments can't guarantee this wouldn't break. Doesn't mean
we can't have a comment in addition though.
>
> > +
> > + if (((vic == 8 || vic == 9 ||
> > + vic == 12 || vic == 13) && mode->vtotal < 263) ||
> > + ((vic == 23 || vic == 24 ||
> > + vic == 27 || vic == 28) && mode->vtotal < 314)) {
>
> I wonder if it wouldn't be better to mark somehow these modes
> in the array as alternating ones. This way all info about cea modes
> will be in one place. For example by (ab)using private_flags:
> /* 8 - 720(1440)x240 at 60Hz */
> { DRM_MODE("720x240", DRM_MODE_TYPE_DRIVER, 13500, 720, 739,
>801, 858, 0, 240, 244, 247, 262, 0,
>DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
> DRM_MODE_FLAG_DBLCLK),
> .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
> .private_flags = CEA_MODE_FLAG_VFP2},
>
> This is of course just an idea, I am not sure if not overkill :)
Sounds potentially sensible. Should do the same for the alternate clock
thing as well then I suppose.
I was pondering whether I should just convert the cea mode array into
some kind of other data structure that could store multiple variants for
each mode. But then I figured it's maybe a bit too much work for just
these few excepttions.
Another thought that occurred to me recently is that the cea mode list
is getting rather long, so I was wondering if could speed up the lookups
somehow. Currently we just do a linear search through the whole thing.
A simple bsearch() might not work out since the modes aren't necessarily
sorted in any useful order, so we might need some kind of a hash or
something, But then I figured that we shouldn't do these lookups too
often so it might not be worth the effort to optimize it.
>
>
> > + mode->vsync_start++;
> > + mode->vsync_end++;
> > + mode->vtotal++;
> > +
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode
> > *to_match,
> > unsigned int clock_tolerance)
> > {
> > @@ -2622,19 +2657,21 @@ static u8 drm_match_cea_mode_clock_tolerance(const
> > struct drm_display_mode *to_m
> > return 0;
> >
> > for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> > - const struct drm_display_mode *cea_mode = &edid_cea_modes[vic];
> > + struct drm_d
[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info
On 8 November 2016 at 13:38, Mauro Santos wrote:
> On 08-11-2016 11:06, Emil Velikov wrote:
>> On 1 November 2016 at 18:47, Mauro Santos
>> wrote:
>>> On 01-11-2016 18:13, Emil Velikov wrote:
From: Emil Velikov
Parsing config sysfs file wakes up the device. The latter of which may
be slow and isn't required to begin with.
Reading through config is/was required since the revision is not
available by other means, although with a kernel patch in the way we can
'cheat' temporarily.
That should be fine, since no open-source project has ever used the
value.
Cc: Michel Dänzer
Cc: Mauro Santos
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
Signed-off-by: Emil Velikov
---
Mauro can you apply this against libdrm and rebuild it. You do _not_
need to rebuild mesa afterwords.
Thanks
---
xf86drm.c | 50 +++---
1 file changed, 35 insertions(+), 15 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index 52add5e..5a5100c 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char
*d_name,
drmPciDeviceInfoPtr device)
{
#ifdef __linux__
+#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
+static const char *attrs[] = {
+ "revision", /* XXX: make sure it's always first, see note below */
+ "vendor",
+ "device",
+ "subsystem_vendor",
+ "subsystem_device",
+};
char path[PATH_MAX + 1];
-unsigned char config[64];
-int fd, ret;
+unsigned int data[ARRAY_SIZE(attrs)];
+FILE *fp;
+int ret;
-snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config", d_name);
-fd = open(path, O_RDONLY);
-if (fd < 0)
-return -errno;
+for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) {
+snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s",
+ d_name, attrs[i]);
+fp = fopen(path, "r");
+if (!fp) {
+/* Note: First we check the revision, since older kernels
+ * may not have it. Default to zero in such cases. */
+if (i == 0) {
+data[i] = 0;
+continue;
+}
+return -errno;
+}
-ret = read(fd, config, sizeof(config));
-close(fd);
-if (ret < 0)
-return -errno;
+ret = fscanf(fp, "%x", &data[i]);
+fclose(fp);
+if (ret != 1)
+return -errno;
+
+}
-device->vendor_id = config[0] | (config[1] << 8);
-device->device_id = config[2] | (config[3] << 8);
-device->revision_id = config[8];
-device->subvendor_id = config[44] | (config[45] << 8);
-device->subdevice_id = config[46] | (config[47] << 8);
+device->revision_id = data[0] & 0xff;
+device->vendor_id = data[1] & 0x;
+device->device_id = data[2] & 0x;
+device->subvendor_id = data[3] & 0x;
+device->subdevice_id = data[4] & 0x;
return 0;
#else
>>>
>>> I have applied this against libdrm 2.4.71 and I don't see any delays
>>> when starting firefox/chromium/thunderbird/glxgears.
>>>
>>> There is also no indication in dmesg that the dGPU is being
>>> reinitialized when starting the programs where I've detected the problem.
>>>
>> Thanks Mauro. Can you give this a try alongside the kernel fix [1] ?
>> I'd love to get the latter merged soon(ish).
>> Independent of the kernel side, I might need to go another way for
>> libdrm/mesa so I'll CC you on future patches.
>>
>> Your help is greatly appreciated !
>>
>> Thanks
>> Emil
>>
>> [1] http://patchwork.ozlabs.org/patch/689975/
>>
>
> I have applied the patch on top of kernel 4.8.6 and I'm using the libdrm
> with the patch you sent me previously.
>
> With this patch things still seem work fine, I don't see any of the
> problems I've seen before, but I don't know how to confirm that the
> value from sysfs is now being used by libdrm instead of defaulting to zero.
>
Grr my bad. $libdrm_builddir/tests/drmdevice should do it. You might
need to explicitly build it (cd tests && make drmdevice)
> The values in /sys/class/drm/card{0,1}/device/revision match what is
> reported by lspci for the iGPU and dGPU.
> I don't know if it's related but the revision file in
> /sys/class/pci_bus/\:03/device/ does show 0xf1 for both GPUs, which
> is different from what is shown in /sys/class/drm/card{0,1}/device/revision.
>
That's perfect, thanks.
:03 is (in all likelihood) is the PCI bridge. You can check with
the device uevent/PCI_SLOT_NAME and cross referenc
[PATCH] drm/i915: avoid harmless empty-body warning
The newly added assert_kernel_context_is_current introduces a warning
when built with W=1:
drivers/gpu/drm/i915/i915_gem.c: In function
âassert_kernel_context_is_currentâ:
drivers/gpu/drm/i915/i915_gem.c:4417:63: error: suggest braces around empty
body in an âelseâ statement [-Werror=empty-body]
Changing the GEM_BUG_ON() macro from an empty definition to "do { } while (0)"
makes the macro more robust to use and avoids the warning.
Fixes: 3033acab07f9 ("drm/i915: Queue the idling context switch after all other
timelines")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/i915/i915_gem.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 735580d72eb1..51ec793f2e20 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -28,7 +28,7 @@
#ifdef CONFIG_DRM_I915_DEBUG_GEM
#define GEM_BUG_ON(expr) BUG_ON(expr)
#else
-#define GEM_BUG_ON(expr)
+#define GEM_BUG_ON(expr) do { } while (0)
#endif
#define I915_NUM_ENGINES 5
--
2.9.0
[PATCH] drm/nouveau: fix LEDS_CLASS=m configuration
The newly introduced LED handling for nouveau fails to link when the
driver is built-in but the LED subsystem is a loadable module:
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_suspend':
tvnv17.c:(.text.nouveau_do_suspend+0x10): undefined reference to
`nouveau_led_suspend'
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_resume':
tvnv17.c:(.text.nouveau_do_resume+0xf0): undefined reference to
`nouveau_led_resume'
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_unload':
tvnv17.c:(.text.nouveau_drm_unload+0x34): undefined reference to
`nouveau_led_fini'
drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_load':
tvnv17.c:(.text.nouveau_drm_load+0x7d0): undefined reference to
`nouveau_led_init'
This adds a separate Kconfig symbol for the LED support that
correctly tracks the dependencies.
Fixes: 8d021d71b324 ("drm/nouveau/drm/nouveau: add a LED driver for the NVIDIA
logo")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/nouveau/Kbuild| 2 +-
drivers/gpu/drm/nouveau/Kconfig | 8
drivers/gpu/drm/nouveau/nouveau_led.h | 2 +-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/Kbuild b/drivers/gpu/drm/nouveau/Kbuild
index fde6e3656636..5e00e911daa6 100644
--- a/drivers/gpu/drm/nouveau/Kbuild
+++ b/drivers/gpu/drm/nouveau/Kbuild
@@ -22,7 +22,7 @@ nouveau-$(CONFIG_DEBUG_FS) += nouveau_debugfs.o
nouveau-y += nouveau_drm.o
nouveau-y += nouveau_hwmon.o
nouveau-$(CONFIG_COMPAT) += nouveau_ioc32.o
-nouveau-$(CONFIG_LEDS_CLASS) += nouveau_led.o
+nouveau-$(CONFIG_DRM_NOUVEAU_LED) += nouveau_led.o
nouveau-y += nouveau_nvif.o
nouveau-$(CONFIG_NOUVEAU_PLATFORM_DRIVER) += nouveau_platform.o
nouveau-y += nouveau_usif.o # userspace <-> nvif
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 78631fb61adf..715cd6f4dc31 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -46,6 +46,14 @@ config NOUVEAU_DEBUG
The paranoia and spam levels will add a lot of extra checks which
may potentially slow down driver operation.
+config DRM_NOUVEAU_LED
+ bool "Support for logo LED"
+ depends on DRM_NOUVEAU && LEDS_CLASS
+ depends on !(DRM_NOUVEAU=y && LEDS_CLASS=m)
+ help
+ Say Y here to enabling controlling the brightness of the
+ LED behind NVIDIA logo on certain Titan cards.
+
config NOUVEAU_DEBUG_DEFAULT
int "Default debug level"
depends on DRM_NOUVEAU
diff --git a/drivers/gpu/drm/nouveau/nouveau_led.h
b/drivers/gpu/drm/nouveau/nouveau_led.h
index 187ecdb82002..9c9bb6ac938e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_led.h
+++ b/drivers/gpu/drm/nouveau/nouveau_led.h
@@ -42,7 +42,7 @@ nouveau_led(struct drm_device *dev)
}
/* nouveau_led.c */
-#if IS_ENABLED(CONFIG_LEDS_CLASS)
+#if IS_ENABLED(CONFIG_DRM_NOUVEAU_LED)
int nouveau_led_init(struct drm_device *dev);
void nouveau_led_suspend(struct drm_device *dev);
void nouveau_led_resume(struct drm_device *dev);
--
2.9.0
[PATCH] drm/amdgpu/powerplay/smu7: fix unintialized data usage
A recent bugfix replaced an out-of-bounds access with direct
use of unintialized data:
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c: In function
'smu7_patch_limits_vddc':
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c:2033:6: error: 'vddc' may be
used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c:2146:11: note: 'vddc' was
declared here
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c:2033:6: error: 'vddci' may be
used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c:2146:17: note: 'vddci' was
declared here
uint32_t vddc, vddci;
This initializes the data as before using the correct type.
Fixes: 77f7f71f5be1 ("drm/amdgpu/powerplay/smu7: fix static checker warning")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 9e49f2777143..b2d61d043d52 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -2147,9 +2147,11 @@ static int smu7_patch_limits_vddc(struct pp_hwmgr *hwmgr,
struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
if (tab) {
+ vddc = tab->vddc;
smu7_patch_ppt_v0_with_vdd_leakage(hwmgr, &vddc,
&data->vddc_leakage);
tab->vddc = vddc;
+ vddci = tab->vddci;
smu7_patch_ppt_v0_with_vdd_leakage(hwmgr, &vddci,
&data->vddci_leakage);
tab->vddci = vddci;
--
2.9.0
[PATCH] drm/i915: avoid harmless empty-body warning
On Tue, Nov 08, 2016 at 02:58:17PM +0100, Arnd Bergmann wrote:
> The newly added assert_kernel_context_is_current introduces a warning
> when built with W=1:
>
> drivers/gpu/drm/i915/i915_gem.c: In function
> âassert_kernel_context_is_currentâ:
> drivers/gpu/drm/i915/i915_gem.c:4417:63: error: suggest braces around empty
> body in an âelseâ statement [-Werror=empty-body]
>
> Changing the GEM_BUG_ON() macro from an empty definition to "do { } while (0)"
> makes the macro more robust to use and avoids the warning.
>
> Fixes: 3033acab07f9 ("drm/i915: Queue the idling context switch after all
> other timelines")
> Signed-off-by: Arnd Bergmann
Reviewed-by: Chris Wilson
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
linux-next: manual merge of the tip tree with the drm-intel tree
On Tue, Nov 08, 2016 at 11:44:03AM +0100, Daniel Vetter wrote:
> On Tue, Nov 08, 2016 at 03:25:41PM +1100, Stephen Rothwell wrote:
> > Hi all,
> >
> > FIXME: Add owner of second tree to To:
> >Add author(s)/SOB of conflicting commits.
> >
> > Today's linux-next merge of the tip tree got a conflict in:
> >
> > drivers/gpu/drm/i915/i915_gem_shrinker.c
> >
> > between commits:
> >
> > 1233e2db199d ("drm/i915: Move object backing storage manipulation to its
> > own locking")
> >
> > from the drm-intel tree and commit:
> >
> > 3ab7c086d5ec ("locking/drm: Kill mutex trickery")
> > c7faee2109f9 ("locking/drm: Fix i915_gem_shrinker_lock() locking")
>
> Hm, this seems to be the older versions that nuke the recursive locking
> trickery entirely, I thought we had version in-flight that kept that? I
> know that the i915 (and msm locking fwiw) is horrible since essentially
> it's a recursive BKL, and we're working (slowly, after all getting rid of
> the BKL wasn't simple either) to fix this. But meanwhile I'm assuming that
> we'll still need this to be able to get out of low memory situations in
> i915. Has that part simply not yet landed?
You're talking about:
lkml.kernel.org/r/20161007154351.GL3117 at twins.programming.kicks-ass.net
? I got no feedback from you DRM guys on that so I kinda forgot about
that in the hope we'd not have to do this at all.
I can try and resurrect, that I suppose.
Now, I know you're working on getting rid of this entirely for i915, but
what about that MSM driver? Will we continue to need it there, is
anybody actually maintaining that thing?
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > From: Gustavo Padovan > > Hi, > > This is yet another version of the DRM fences patches. Please refer > to the cover letter[1] in a previous version to check for more details. > > In v7 we now have split most of the out_fences code into > prepare_crtc_signaling() and unprepare_crtc_signaling() with improved error > handling. More details on the v7 changes are embedded in each commit's > message. > > Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and > added support to fences. Current patches can be seen here: > > https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1 > > He managed to run AOSP on top of padovan/fences kernel branch with full fence > support on qemu/virgl and msm db410c. That means we already have a working > open source userspace using the explicit fencing implementation. > > Also i-g-t testing are available at: > > https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/ > > Please review! I think we're getting there. Found some issues with patch 3 still. Besides that I think we need: - r-b/ack from Sean Paul, as an ack that he's happy with what this means for drm_hwcomposer (and that the hwc patches look ok, too). - acks/t-b from everyone who's run this, the more the better. This is a big uabi extension, making the effort by everyone explicit is important. - ack from Brian that he can use the out-fence stuff for his writeback support. Probably need to add a for_each_connector loop to prepare/complete_signalling (and drop the crtc_ in there, but Brian can do that). Cheers, Daniel > > Gustavo > > [1] https://www.mail-archive.com/linux-kernel at > vger.kernel.org/msg1253822.html > > --- > Gustavo Padovan (3): > drm/fence: add in-fences support > drm/fence: add fence timeline to drm_crtc > drm/fence: add out-fences support > > drivers/gpu/drm/Kconfig | 1 + > drivers/gpu/drm/drm_atomic.c| 247 > ++-- > drivers/gpu/drm/drm_atomic_helper.c | 3 + > drivers/gpu/drm/drm_crtc.c | 45 +++ > drivers/gpu/drm/drm_plane.c | 1 + > include/drm/drm_atomic.h| 1 + > include/drm/drm_crtc.h | 55 > 7 files changed, 311 insertions(+), 42 deletions(-) > > -- > 2.5.5 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH v7 3/3] drm/fence: add out-fences support
On Tue, Nov 08, 2016 at 03:54:50PM +0900, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Support DRM out-fences by creating a sync_file with a fence for each CRTC
> that sets the OUT_FENCE_PTR property.
>
> We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
> the sync_file fd back to userspace.
>
> The sync_file and fd are allocated/created before commit, but the
> fd_install operation only happens after we know that commit succeed.
>
> v2: Comment by Rob Clark:
> - Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
>
> Comment by Daniel Vetter:
> - Add clean up code for out_fences
>
> v3: Comments by Daniel Vetter:
> - create DRM_MODE_ATOMIC_EVENT_MASK
> - userspace should fill out_fences_ptr with the crtc_ids for which
> it wants fences back.
>
> v4: Create OUT_FENCE_PTR properties and remove old approach.
>
> v5: Comments by Brian Starkey:
> - Remove extra fence_get() in atomic_ioctl()
> - Check ret before iterating on the crtc_state
> - check ret before fd_install
> - set fence_state to NULL at the beginning
> - check fence_state->out_fence_ptr before put_user()
> - change order of fput() and put_unused_fd() on failure
>
> - Add access_ok() check to the out_fence_ptr received
> - Rebase after fence -> dma_fence rename
> - Store out_fence_ptr in the drm_atomic_state
> - Split crtc_setup_out_fence()
> - return -1 as out_fence with TEST_ONLY flag
>
> v6: Comments by Daniel Vetter
> - Add prepare/unprepare_crtc_signaling()
> - move struct drm_out_fence_state to drm_atomic.c
> - mark get_crtc_fence() as static
>
> Comments by Brian Starkey
> - proper set fence_ptr fence_state array
> - isolate fence_idx increment
>
> - improve error handling
>
> Signed-off-by: Gustavo Padovan
Found a bunch more nitpicks, and wishlist items for igt coverage.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 233
> +++
> drivers/gpu/drm/drm_crtc.c | 8 ++
> include/drm/drm_atomic.h | 1 +
> include/drm/drm_crtc.h | 7 +-
> 4 files changed, 206 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index d1ae463..9a7d84c 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -289,6 +289,25 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> }
> EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>
> +static void
> +drm_atomic_set_out_fence_for_crtc(struct drm_atomic_state *state,
> + struct drm_crtc *crtc, u64 __user *fence_ptr)
> +{
> + state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
> +}
> +
> +static u64 __user *
> +drm_atomic_get_out_fence_for_crtc(struct drm_atomic_state *state,
> + struct drm_crtc *crtc)
Since these two are static I'd drop the long prefix to make the code
easier to read.
> +{
> + u64 __user *fence_ptr;
> +
> + fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
> + state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
> +
> + return fence_ptr;
> +}
> +
> /**
> * drm_atomic_set_mode_for_crtc - set mode for CRTC
> * @state: the CRTC whose incoming state to update
> @@ -490,6 +509,12 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> &replaced);
> state->color_mgmt_changed |= replaced;
> return ret;
> + } else if (property == config->prop_out_fence_ptr) {
> + uint64_t __user *fence_ptr = u64_to_user_ptr(val);
Don't we need to filter out NULL/0 here like we filter out -1 for the
in-fences? Just in case some userspace samples/restores all properties.
Sounds like an igt is missing for this ...
Maybe we even need a special igt which samples _all_ current atomic
properties, and then does an atomic commit which changes none of them
(without setting the ALLOW_MODESET flag ofc). That /should/ work, and
would catch such bugs in the future.
> + if (!access_ok(VERIFY_WRITE, fence_ptr, sizeof(*fence_ptr)))
> + return -EFAULT;
Same comment about igt coverage I made for patch 1, but with
s/in-fence/out-fence/, and s/~0ULL/8/. I picked 8 as an invalid address !=
NULL.
And the testcase need to cover all possible combinations of output event
generation, i.e. out-fence, event and out-fence+event. So 3x3=9 testcases
for this I think.
> +
> + drm_atomic_set_out_fence_for_crtc(state->state, crtc,
> fence_ptr);
> } else if (crtc->funcs->atomic_set_property)
> return crtc->funcs->atomic_set_property(crtc, state, property,
> val);
> else
> @@ -532,6 +557,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> *val = (state->ctm) ? state->ctm->base.id : 0;
> else if (property == config->gamma_lut_property)
[Bug 98523] Can't run without "nomodeset" on MacPro6,1 with two AMD R9 280X Tahiti video cards
https://bugs.freedesktop.org/show_bug.cgi?id=98523 Alex Deucher changed: What|Removed |Added Resolution|--- |NOTOURBUG Status|NEW |RESOLVED --- Comment #2 from Alex Deucher --- The problem is Macs expose the vbios for some cards in a proprietary way. IIRC, the vbios is only available via EFI prior to the OS loading. The bootloader needs to snag the vbios and then make it available to the OS when it loads. I'm not really a Mac expert so I don't remember all the details. There's no way to work around it in the driver. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/650e3675/attachment.html>
[PATCH v3 08/11] drm/edid: Remove drm_select_eld
The only user was i915, which is now gone.
Signed-off-by: Maarten Lankhorst
Reviewed-by: Ville Syrjälä
Acked-by: Dave Airlie #irc
Cc: dri-devel at lists.freedesktop.org
---
drivers/gpu/drm/drm_edid.c | 26 --
include/drm/drm_edid.h | 1 -
2 files changed, 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9506933b41cd..1801e9c0e41b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3611,32 +3611,6 @@ int drm_av_sync_delay(struct drm_connector *connector,
EXPORT_SYMBOL(drm_av_sync_delay);
/**
- * drm_select_eld - select one ELD from multiple HDMI/DP sinks
- * @encoder: the encoder just changed display mode
- *
- * It's possible for one encoder to be associated with multiple HDMI/DP sinks.
- * The policy is now hard coded to simply use the first HDMI/DP sink's ELD.
- *
- * Return: The connector associated with the first HDMI/DP sink that has ELD
- * attached to it.
- */
-struct drm_connector *drm_select_eld(struct drm_encoder *encoder)
-{
- struct drm_connector *connector;
- struct drm_device *dev = encoder->dev;
-
- WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
- WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-
- drm_for_each_connector(connector, dev)
- if (connector->encoder == encoder && connector->eld[0])
- return connector;
-
- return NULL;
-}
-EXPORT_SYMBOL(drm_select_eld);
-
-/**
* drm_detect_hdmi_monitor - detect whether monitor is HDMI
* @edid: monitor EDID information
*
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index c3a7d440bc11..38eabf65f19d 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -330,7 +330,6 @@ int drm_edid_to_sad(struct edid *edid, struct cea_sad
**sads);
int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb);
int drm_av_sync_delay(struct drm_connector *connector,
const struct drm_display_mode *mode);
-struct drm_connector *drm_select_eld(struct drm_encoder *encoder);
#ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
int drm_load_edid_firmware(struct drm_connector *connector);
--
2.7.4
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 01:44:34PM +0100, Christian König wrote: > Am 08.11.2016 um 12:45 schrieb Chris Wilson: > > On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote: > > > On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: > > > > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > > > > > From: Gustavo Padovan > > > > > > > > > > Hi, > > > > > > > > > > This is yet another version of the DRM fences patches. Please refer > > > > > to the cover letter[1] in a previous version to check for more > > > > > details. > > > > Explicit fencing is not a superset of the implicit fences. The driver > > > > may be using implicit fences (on a reservation object) to serialise > > > > asynchronous operations wrt to each other (such as dispatching threads > > > > to flush cpu caches to memory, manipulating page tables and the like > > > > before the flip). Since the user doesn't know about these operations, > > > > they are not included in the explicit fence they provide, at which point > > > > we can't trust their fence to the exclusion of the implicit fences... > > > My thoughts are that in atomic_check drivers just fill in the fence from > > > the reservation_object (i.e. the uapi implicit fencing part). If there's > > > any additional work that's queued up in ->prepare_fb then I guess the > > > driver needs to track that internally, but _only_ for kernel-internally > > > queued work. > > That's not a trivial task to work out which of the fence contexts within > > the reservation object are required and which are to be replaced by the > > explicit fence, esp. when you have to consider external fences. > > > The reason for that is that with explicit fencing we want to allow > > > userspace to overwrite any existing implicit fences that might hang > > > around. > > I'm just suggesting the danger of that when userspace doesn't know > > everything and the current interfaces do not allow for userspace to know, > > we only tell userspace about its own action (more or less). > > It's even worse than that. See the kernel can for example swap out objects > any time it wants. > > Userspace doesn't know about such operations and so can't provide them as > explicit fence. > > Same is true for example in situations where one userspace process doesn't > know about operations another process does. E.g. for backward compatibility > with DRI2/3 for example. > > So we will always have a mixture of implicit fences and explicit fences. > > The approach we used for amdgpu is that we implicit wait for all fences > which the initiator of an operation can't know about (e.g. from another > process or kernel internally) and explicitly wait for all additional fences > provided by the initiator or an operation. On android userspace is also supposed to know about explicit fences from other users, i.e. shared buffers. So there you shouldn't wait for implicit fences from other processes, only for kernel-internal stuff. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[GIT PULL][drm-next] drm/mali-dp fixes and updates
On Tue, Nov 08, 2016 at 12:26:19PM +, Liviu Dudau wrote: > Hi Dave, > > Here is the list of fixes that I have for drm/mali-dp. They've been on the > mailing > lists for a while and merged into linux-next for a few weeks, but due to > holiday and > travel to Linux Plumbers I did not send the pull request earlier. I don't > know if > these patches can be pulled into v4.9 still (they will conflict with Ville > Syrjälä's > cleanup of DRM_ROTATE series that is already in drm-next), but if you do that > would > be great. > > The following changes since commit fb422950c6cd726fd36eb72a7cf84583440a18a2: > > Merge branch 'linux-4.9' of git://github.com/skeggsb/linux into drm-next > (2016-10-28 14:24:56 +1000) > > are available in the git repository at: > > git://linux-arm.org/linux-ld.git for-upstream/mali-dp > > for you to fetch changes up to e64053f05eb924db45f90a1556a200d1acb4b01e: > > drm: mali-dp: Clear CVAL when leaving config mode (2016-11-08 11:40:02 > +) > > > Baoyou Xie (1): > drm/arm: mark symbols static where possible > > Brian Starkey (8): > drm: mali-dp: Add pitch alignment check function > drm: mali-dp: Add pitch alignment check for planes > arm: mali-dp: Extract mode_config cleanup into malidp_fini > drm: mali-dp: Refactor plane initialisation > drm: mali-dp: Enable alpha blending > drm: mali-dp: Store internal format and n_planes in plane state > drm: mali-dp: Don't set DRM_PLANE_COMMIT_ACTIVE_ONLY > drm: mali-dp: Clear CVAL when leaving config mode > > Liviu Dudau (3): > drm: mali-dp: Clear the config_valid flag before using it in wait_event. > drm: mali-dp: Set the drm->irq_enabled flag to match driver's state. > drm: mali-dp: Add support for setting plane's rotation property from > userspace. Quick check says these patches haven't shown on up a mailing list. Please submit first, do review in public and then send the pull request. There's generally a lot of cross-driver learning possible with this kind of stuff. Imo splitting drivers into their own mailing list and community only makes sense once there's just too much traffic, like with the big ones (intel, amd and nouveau because it's developed in userspace entirely). Thanks, Daniel > > drivers/gpu/drm/arm/malidp_drv.c| 19 +--- > drivers/gpu/drm/arm/malidp_drv.h| 3 ++ > drivers/gpu/drm/arm/malidp_hw.c | 5 ++ > drivers/gpu/drm/arm/malidp_hw.h | 9 > drivers/gpu/drm/arm/malidp_planes.c | 96 > - > 5 files changed, 92 insertions(+), 40 deletions(-) > > > Many thanks, > Liviu > > -- > > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --- > ¯\_(ã)_/¯ > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm: Add stackdepot include for DRM_DEBUG_MM
On Tue, Nov 08, 2016 at 11:56:01AM +, Chris Wilson wrote:
> 0day found that stackdepot.h doesn't get automatically included on all
> architectures, so remember to add our #include.
>
> Reported-by: kbuild test robot
> Fixes: 5705670d0463 ("drm: Track drm_mm allocators and show leaks on
> shutdown")
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
Thx for the fast fixup, applied to drm-misc.
-Daniel
> ---
> drivers/gpu/drm/drm_mm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 89b891a4847f..632473beb40c 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -105,6 +105,8 @@ static struct drm_mm_node
> *drm_mm_search_free_in_range_generic(const struct drm_
> enum drm_mm_search_flags flags);
>
> #ifdef CONFIG_DRM_DEBUG_MM
> +#include
> +
> #define STACKDEPTH 32
> #define BUFSZ 4096
>
> --
> 2.10.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH] drm: Add stackdepot include for DRM_DEBUG_MM
Am 08.11.2016 um 12:56 schrieb Chris Wilson:
> 0day found that stackdepot.h doesn't get automatically included on all
> architectures, so remember to add our #include.
>
> Reported-by: kbuild test robot
> Fixes: 5705670d0463 ("drm: Track drm_mm allocators and show leaks on
> shutdown")
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
Reviewed-by: Christian König .
> ---
> drivers/gpu/drm/drm_mm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 89b891a4847f..632473beb40c 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -105,6 +105,8 @@ static struct drm_mm_node
> *drm_mm_search_free_in_range_generic(const struct drm_
> enum drm_mm_search_flags flags);
>
> #ifdef CONFIG_DRM_DEBUG_MM
> +#include
> +
> #define STACKDEPTH 32
> #define BUFSZ 4096
>
[PATCH] drm: mali-dp: Clear CVAL when leaving config mode
It's possible for CVAL to get set whilst we are in config mode. If this
happens, afer we leave config mode the HW will latch whatever
configuration is in the registers at the next vsync. Most likely this
will be a partial configuration, as we'll be racing against the ongoing
atomic_commit.
To avoid this, clear CVAL before leaving config mode.
Signed-off-by: Brian Starkey
---
drivers/gpu/drm/arm/malidp_hw.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 7f4a0bd..65e667b 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -125,6 +125,7 @@ static void malidp500_leave_config_mode(struct
malidp_hw_device *hwdev)
{
u32 status, count = 100;
+ malidp_hw_clearbits(hwdev, MALIDP_CFG_VALID, MALIDP500_CONFIG_VALID);
malidp_hw_clearbits(hwdev, MALIDP500_DC_CONFIG_REQ,
MALIDP500_DC_CONTROL);
while (count) {
status = malidp_hw_read(hwdev, hwdev->map.dc_base +
MALIDP_REG_STATUS);
@@ -271,6 +272,7 @@ static void malidp550_leave_config_mode(struct
malidp_hw_device *hwdev)
{
u32 status, count = 100;
+ malidp_hw_clearbits(hwdev, MALIDP_CFG_VALID, MALIDP550_CONFIG_VALID);
malidp_hw_clearbits(hwdev, MALIDP550_DC_CONFIG_REQ,
MALIDP550_DC_CONTROL);
while (count) {
status = malidp_hw_read(hwdev, hwdev->map.dc_base +
MALIDP_REG_STATUS);
--
1.7.9.5
[PATCH v7 0/3] drm: add explict fencing
Am 08.11.2016 um 12:45 schrieb Chris Wilson: > On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote: >> On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: >>> On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: From: Gustavo Padovan Hi, This is yet another version of the DRM fences patches. Please refer to the cover letter[1] in a previous version to check for more details. >>> Explicit fencing is not a superset of the implicit fences. The driver >>> may be using implicit fences (on a reservation object) to serialise >>> asynchronous operations wrt to each other (such as dispatching threads >>> to flush cpu caches to memory, manipulating page tables and the like >>> before the flip). Since the user doesn't know about these operations, >>> they are not included in the explicit fence they provide, at which point >>> we can't trust their fence to the exclusion of the implicit fences... >> My thoughts are that in atomic_check drivers just fill in the fence from >> the reservation_object (i.e. the uapi implicit fencing part). If there's >> any additional work that's queued up in ->prepare_fb then I guess the >> driver needs to track that internally, but _only_ for kernel-internally >> queued work. > That's not a trivial task to work out which of the fence contexts within > the reservation object are required and which are to be replaced by the > explicit fence, esp. when you have to consider external fences. > >> The reason for that is that with explicit fencing we want to allow >> userspace to overwrite any existing implicit fences that might hang >> around. > I'm just suggesting the danger of that when userspace doesn't know > everything and the current interfaces do not allow for userspace to know, > we only tell userspace about its own action (more or less). It's even worse than that. See the kernel can for example swap out objects any time it wants. Userspace doesn't know about such operations and so can't provide them as explicit fence. Same is true for example in situations where one userspace process doesn't know about operations another process does. E.g. for backward compatibility with DRI2/3 for example. So we will always have a mixture of implicit fences and explicit fences. The approach we used for amdgpu is that we implicit wait for all fences which the initiator of an operation can't know about (e.g. from another process or kernel internally) and explicitly wait for all additional fences provided by the initiator or an operation. Regards, Christian. > -Chris >
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 11:45:51AM +, Chris Wilson wrote: > On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote: > > On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: > > > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > > > > From: Gustavo Padovan > > > > > > > > Hi, > > > > > > > > This is yet another version of the DRM fences patches. Please refer > > > > to the cover letter[1] in a previous version to check for more details. > > > > > > Explicit fencing is not a superset of the implicit fences. The driver > > > may be using implicit fences (on a reservation object) to serialise > > > asynchronous operations wrt to each other (such as dispatching threads > > > to flush cpu caches to memory, manipulating page tables and the like > > > before the flip). Since the user doesn't know about these operations, > > > they are not included in the explicit fence they provide, at which point > > > we can't trust their fence to the exclusion of the implicit fences... > > > > My thoughts are that in atomic_check drivers just fill in the fence from > > the reservation_object (i.e. the uapi implicit fencing part). If there's > > any additional work that's queued up in ->prepare_fb then I guess the > > driver needs to track that internally, but _only_ for kernel-internally > > queued work. > > That's not a trivial task to work out which of the fence contexts within > the reservation object are required and which are to be replaced by the > explicit fence, esp. when you have to consider external fences. Hm, what kind of async kernel tasks are you thinking off? Atm I don't know of anyone who does e.g. clflush through the gpu. And ttm bo placement moves for display should be explicit enough that drivers will deal with them correctly. At least that seems to have been the conclusion from the long amdgpu thread. > > The reason for that is that with explicit fencing we want to allow > > userspace to overwrite any existing implicit fences that might hang > > around. > > I'm just suggesting the danger of that when userspace doesn't know > everything and the current interfaces do not allow for userspace to know, > we only tell userspace about its own action (more or less). tools for fools, but yes userspace is expected to get this 100% right (for any userspace-issued cs at least), and eat the fallout if it doesn't. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm: mali-dp: Add support for setting plane's rotation property from userspace.
In order to support DRM_IOCTL_MODE_OBJ_SETPROPERTY for the rotation property
we need to have a ->set_property hook defined for the planes. Set the
plane's ->set_property hook to drm_atomic_helper_plane_set_property()
Signed-off-by: Liviu Dudau
---
drivers/gpu/drm/arm/malidp_planes.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c
b/drivers/gpu/drm/arm/malidp_planes.c
index bb1c528..8eef9a8 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -89,6 +89,7 @@ void malidp_destroy_plane_state(struct drm_plane *plane,
static const struct drm_plane_funcs malidp_de_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
+ .set_property = drm_atomic_helper_plane_set_property,
.destroy = malidp_de_plane_destroy,
.reset = drm_atomic_helper_plane_reset,
.atomic_duplicate_state = malidp_duplicate_plane_state,
--
2.10.0
[GIT PULL][drm-next] drm/mali-dp fixes and updates
On Tue, Nov 08, 2016 at 01:49:55PM +0100, Daniel Vetter wrote: > On Tue, Nov 08, 2016 at 12:26:19PM +, Liviu Dudau wrote: > > Hi Dave, > > > > Here is the list of fixes that I have for drm/mali-dp. They've been on the > > mailing > > lists for a while and merged into linux-next for a few weeks, but due to > > holiday and > > travel to Linux Plumbers I did not send the pull request earlier. I don't > > know if > > these patches can be pulled into v4.9 still (they will conflict with Ville > > Syrjälä's > > cleanup of DRM_ROTATE series that is already in drm-next), but if you do > > that would > > be great. > > > > The following changes since commit fb422950c6cd726fd36eb72a7cf84583440a18a2: > > > > Merge branch 'linux-4.9' of git://github.com/skeggsb/linux into drm-next > > (2016-10-28 14:24:56 +1000) > > > > are available in the git repository at: > > > > git://linux-arm.org/linux-ld.git for-upstream/mali-dp > > > > for you to fetch changes up to e64053f05eb924db45f90a1556a200d1acb4b01e: > > > > drm: mali-dp: Clear CVAL when leaving config mode (2016-11-08 11:40:02 > > +) > > > > > > Baoyou Xie (1): > > drm/arm: mark symbols static where possible > > > > Brian Starkey (8): > > drm: mali-dp: Add pitch alignment check function > > drm: mali-dp: Add pitch alignment check for planes > > arm: mali-dp: Extract mode_config cleanup into malidp_fini > > drm: mali-dp: Refactor plane initialisation > > drm: mali-dp: Enable alpha blending > > drm: mali-dp: Store internal format and n_planes in plane state > > drm: mali-dp: Don't set DRM_PLANE_COMMIT_ACTIVE_ONLY > > drm: mali-dp: Clear CVAL when leaving config mode > > > > Liviu Dudau (3): > > drm: mali-dp: Clear the config_valid flag before using it in > > wait_event. > > drm: mali-dp: Set the drm->irq_enabled flag to match driver's state. > > drm: mali-dp: Add support for setting plane's rotation property from > > userspace. > > Quick check says these patches haven't shown on up a mailing list. Please > submit first, do review in public and then send the pull request. There's > generally a lot of cross-driver learning possible with this kind of stuff. > Imo splitting drivers into their own mailing list and community only makes > sense once there's just too much traffic, like with the big ones (intel, > amd and nouveau because it's developed in userspace entirely). Actually, all except the last patch from me and last from Brian have been on the dri-devel mailing list, I believe you have even commented on one of them (drm->irq_enabled). For my patches, first two start here [1]. For Brian's patches, his series starts here [2] (also contains my patches) and one before last is here [3] [1] https://lists.freedesktop.org/archives/dri-devel/2016-July/114302.html [2] https://lists.freedesktop.org/archives/dri-devel/2016-October/120486.html [3] https://lists.freedesktop.org/archives/dri-devel/2016-October/121849.html I appologise for missing to send the last two patches, they were in the public tree but did not sent them for some reason to the public list. Will do that now. Best regards, Liviu > > Thanks, Daniel > > > > drivers/gpu/drm/arm/malidp_drv.c| 19 +--- > > drivers/gpu/drm/arm/malidp_drv.h| 3 ++ > > drivers/gpu/drm/arm/malidp_hw.c | 5 ++ > > drivers/gpu/drm/arm/malidp_hw.h | 9 > > drivers/gpu/drm/arm/malidp_planes.c | 96 > > - > > 5 files changed, 92 insertions(+), 40 deletions(-) > > > > > > Many thanks, > > Liviu > > > > -- > > > > | I would like to | > > | fix the world, | > > | but they're not | > > | giving me the | > > \ source code! / > > --- > > ¯\_(ã)_/¯ > > ___ > > dri-devel mailing list > > dri-devel at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ã)_/¯
[PATCH libdrm] xf86drm: Parse the separate files to retrieve the vendor/device info
On 08-11-2016 11:06, Emil Velikov wrote:
> On 1 November 2016 at 18:47, Mauro Santos
> wrote:
>> On 01-11-2016 18:13, Emil Velikov wrote:
>>> From: Emil Velikov
>>>
>>> Parsing config sysfs file wakes up the device. The latter of which may
>>> be slow and isn't required to begin with.
>>>
>>> Reading through config is/was required since the revision is not
>>> available by other means, although with a kernel patch in the way we can
>>> 'cheat' temporarily.
>>>
>>> That should be fine, since no open-source project has ever used the
>>> value.
>>>
>>> Cc: Michel Dänzer
>>> Cc: Mauro Santos
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98502
>>> Signed-off-by: Emil Velikov
>>> ---
>>> Mauro can you apply this against libdrm and rebuild it. You do _not_
>>> need to rebuild mesa afterwords.
>>>
>>> Thanks
>>> ---
>>> xf86drm.c | 50 +++---
>>> 1 file changed, 35 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/xf86drm.c b/xf86drm.c
>>> index 52add5e..5a5100c 100644
>>> --- a/xf86drm.c
>>> +++ b/xf86drm.c
>>> @@ -2950,25 +2950,45 @@ static int drmParsePciDeviceInfo(const char *d_name,
>>> drmPciDeviceInfoPtr device)
>>> {
>>> #ifdef __linux__
>>> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
>>> +static const char *attrs[] = {
>>> + "revision", /* XXX: make sure it's always first, see note below */
>>> + "vendor",
>>> + "device",
>>> + "subsystem_vendor",
>>> + "subsystem_device",
>>> +};
>>> char path[PATH_MAX + 1];
>>> -unsigned char config[64];
>>> -int fd, ret;
>>> +unsigned int data[ARRAY_SIZE(attrs)];
>>> +FILE *fp;
>>> +int ret;
>>>
>>> -snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/config", d_name);
>>> -fd = open(path, O_RDONLY);
>>> -if (fd < 0)
>>> -return -errno;
>>> +for (unsigned i = 0; i < ARRAY_SIZE(attrs); i++) {
>>> +snprintf(path, PATH_MAX, "/sys/class/drm/%s/device/%s",
>>> + d_name, attrs[i]);
>>> +fp = fopen(path, "r");
>>> +if (!fp) {
>>> +/* Note: First we check the revision, since older kernels
>>> + * may not have it. Default to zero in such cases. */
>>> +if (i == 0) {
>>> +data[i] = 0;
>>> +continue;
>>> +}
>>> +return -errno;
>>> +}
>>>
>>> -ret = read(fd, config, sizeof(config));
>>> -close(fd);
>>> -if (ret < 0)
>>> -return -errno;
>>> +ret = fscanf(fp, "%x", &data[i]);
>>> +fclose(fp);
>>> +if (ret != 1)
>>> +return -errno;
>>> +
>>> +}
>>>
>>> -device->vendor_id = config[0] | (config[1] << 8);
>>> -device->device_id = config[2] | (config[3] << 8);
>>> -device->revision_id = config[8];
>>> -device->subvendor_id = config[44] | (config[45] << 8);
>>> -device->subdevice_id = config[46] | (config[47] << 8);
>>> +device->revision_id = data[0] & 0xff;
>>> +device->vendor_id = data[1] & 0x;
>>> +device->device_id = data[2] & 0x;
>>> +device->subvendor_id = data[3] & 0x;
>>> +device->subdevice_id = data[4] & 0x;
>>>
>>> return 0;
>>> #else
>>>
>>
>> I have applied this against libdrm 2.4.71 and I don't see any delays
>> when starting firefox/chromium/thunderbird/glxgears.
>>
>> There is also no indication in dmesg that the dGPU is being
>> reinitialized when starting the programs where I've detected the problem.
>>
> Thanks Mauro. Can you give this a try alongside the kernel fix [1] ?
> I'd love to get the latter merged soon(ish).
> Independent of the kernel side, I might need to go another way for
> libdrm/mesa so I'll CC you on future patches.
>
> Your help is greatly appreciated !
>
> Thanks
> Emil
>
> [1] http://patchwork.ozlabs.org/patch/689975/
>
I have applied the patch on top of kernel 4.8.6 and I'm using the libdrm
with the patch you sent me previously.
With this patch things still seem work fine, I don't see any of the
problems I've seen before, but I don't know how to confirm that the
value from sysfs is now being used by libdrm instead of defaulting to zero.
The values in /sys/class/drm/card{0,1}/device/revision match what is
reported by lspci for the iGPU and dGPU.
I don't know if it's related but the revision file in
/sys/class/pci_bus/\:03/device/ does show 0xf1 for both GPUs, which
is different from what is shown in /sys/class/drm/card{0,1}/device/revision.
--
Mauro Santos
[Bug 95306] Random Blank(black) screens on "Carrizo"
https://bugs.freedesktop.org/show_bug.cgi?id=95306 --- Comment #33 from Patrick Laurin --- Issue == screen turning black after a while. On Nov 8, 2016 08:35, "Patrick Laurin" wrote: > I confirm still present on rc4. Guess I only got lucky on rc3 for 45 > minutes. > > On Nov 8, 2016 03:10, wrote: > >> *Comment # 31 <https://bugs.freedesktop.org/show_bug.cgi?id=95306#c31> on >> bug 95306 <https://bugs.freedesktop.org/show_bug.cgi?id=95306> from Tom >> * >> >> The suspend issue is still present, it just doesn't happen so often. >> I also noticed another thing. It seems to be impossible to cold boot with >> 4.9rc4, I have to boot some other kernel or Windows and then reboot to >> successfully boot with 4.9rc4. >> So far no random blanking, but it already happened before, that it ran for >> hours without blanking. >> >> -- >> You are receiving this mail because: >> >>- You are on the CC list for the bug. >> >> -- You are receiving this mail because: You are the assignee for the bug. ------ next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/b5efdd79/attachment-0001.html>
[Bug 95306] Random Blank(black) screens on "Carrizo"
https://bugs.freedesktop.org/show_bug.cgi?id=95306 --- Comment #32 from Patrick Laurin --- I confirm still present on rc4. Guess I only got lucky on rc3 for 45 minutes. On Nov 8, 2016 03:10, wrote: > *Comment # 31 <https://bugs.freedesktop.org/show_bug.cgi?id=95306#c31> on > bug 95306 <https://bugs.freedesktop.org/show_bug.cgi?id=95306> from Tom > * > > The suspend issue is still present, it just doesn't happen so often. > I also noticed another thing. It seems to be impossible to cold boot with > 4.9rc4, I have to boot some other kernel or Windows and then reboot to > successfully boot with 4.9rc4. > So far no random blanking, but it already happened before, that it ran for > hours without blanking. > > -- > You are receiving this mail because: > >- You are on the CC list for the bug. > > -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/337c2123/attachment.html>
[PATCH RFC 00/12] tda998x updates
On Tue, Nov 08, 2016 at 01:32:15PM +, Russell King - ARM Linux wrote:
> Unfortunately, my drm-tda998x-devel branch is slightly out of date with
> these patches it's the original set of 10 patches. I've not pushed
> these ones out to that branch yet, as I've three additional patches on
> top of these which aren't "ready" for pushing out.
Here's the delta between the branch and what I just posted:
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 1b262a89b7e1..3a5e5c466972 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -43,12 +43,12 @@ struct tda998x_priv {
u16 rev;
u8 current_page;
bool is_on;
- bool is_hdmi_sink;
- bool is_hdmi_config;
+ bool supports_infoframes;
+ bool sink_has_audio;
u8 vip_cntrl_0;
u8 vip_cntrl_1;
u8 vip_cntrl_2;
- unsigned long tdms_clock;
+ unsigned long tmds_clock;
struct tda998x_audio_params audio_params;
struct platform_device *audio_pdev;
@@ -774,7 +774,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
* assume 100MHz requires larger divider.
*/
adiv = AUDIO_DIV_SERCLK_8;
- if (priv->tdms_clock > 10)
+ if (priv->tmds_clock > 10)
adiv++; /* AUDIO_DIV_SERCLK_16 */
/* S/PDIF asks for a larger divider */
@@ -869,8 +869,7 @@ static int tda998x_audio_hw_params(struct device *dev, void
*data,
}
mutex_lock(&priv->audio_mutex);
- /* We must not program the TDA998x for audio if the sink is DVI. */
- if (priv->is_hdmi_config)
+ if (priv->supports_infoframes && priv->sink_has_audio)
ret = tda998x_configure_audio(priv, &audio);
else
ret = 0;
@@ -962,6 +961,27 @@ static int tda998x_connector_dpms(struct drm_connector
*connector, int mode)
return drm_helper_connector_dpms(connector, mode);
}
+static int tda998x_connector_fill_modes(struct drm_connector *connector,
+ uint32_t maxX, uint32_t maxY)
+{
+ struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+ int ret;
+
+ mutex_lock(&priv->audio_mutex);
+ ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
+
+ if (connector->edid_blob_ptr) {
+ struct edid *edid = (void *)connector->edid_blob_ptr->data;
+
+ priv->sink_has_audio = drm_detect_monitor_audio(edid);
+ } else {
+ priv->sink_has_audio = false;
+ }
+ mutex_unlock(&priv->audio_mutex);
+
+ return ret;
+}
+
static enum drm_connector_status
tda998x_connector_detect(struct drm_connector *connector, bool force)
{
@@ -980,7 +1000,7 @@ static void tda998x_connector_destroy(struct drm_connector
*connector)
static const struct drm_connector_funcs tda998x_connector_funcs = {
.dpms = tda998x_connector_dpms,
.reset = drm_atomic_helper_connector_reset,
- .fill_modes = drm_helper_probe_single_connector_modes,
+ .fill_modes = tda998x_connector_fill_modes,
.detect = tda998x_connector_detect,
.destroy = tda998x_connector_destroy,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
@@ -1072,11 +1092,7 @@ static int tda998x_connector_get_modes(struct
drm_connector *connector)
drm_mode_connector_update_edid_property(connector, edid);
n = drm_add_edid_modes(connector, edid);
- priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
-
- mutex_lock(&priv->audio_mutex);
drm_edid_to_eld(connector, edid);
- mutex_unlock(&priv->audio_mutex);
kfree(edid);
@@ -1350,8 +1366,22 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
/* must be last register set: */
reg_write(priv, REG_TBG_CNTRL_0, 0);
- /* Only setup the info frames if the sink is HDMI */
- if (priv->is_hdmi_sink) {
+ priv->tmds_clock = adjusted_mode->clock;
+
+ /* CEA-861B section 6 says that:
+* CEA version 1 (CEA-861) has no support for infoframes.
+* CEA version 2 (CEA-861A) supports version 1 AVI infoframes,
+* and optional basic audio.
+* CEA version 3 (CEA-861B) supports version 1 and 2 AVI infoframes,
+* and optional digital audio, with audio infoframes.
+*
+* Since we only support generation of version 2 AVI infoframes,
+* ignore CEA version 2 and below (iow, behave as if we're a
+* CEA-861 source.)
+*/
+ priv->supports_infoframes = priv->connector.display_info.cea_rev >= 3;
+
+ if (priv->supports_infoframes) {
/* We need to turn HDMI HDCP stuff on to get audio through */
reg &= ~TBG_CNTRL_1_DWIN_DIS;
reg_write(priv, REG_TBG_CNTRL_1, reg);
@@ -1360,13 +1390,11 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
[PATCH RFC 00/12] tda998x updates
On Tue, Nov 08, 2016 at 01:19:51PM +, Robin Murphy wrote: > Hi Russell, > > On 08/11/16 12:24, Russell King - ARM Linux wrote: > > As no one responded to the previous round, I'm not spending soo much > > time writing up a description of these changes again. It's also been > > quite a long time, so I've forgotten all the details of the changes, > > so I'll do my best. > > > > Changes from the previous series include: > > - reorder the initial three patches > > - change the (now third patch)... I think to increase the size of the > > locked region. > > - fix edid parsing for infoframe generation - as was pointed out for > > dw-hdmi, parsing the EDID in get_modes() is incorrect, as that method > > will not be called when an override-edid is in effect. We need to > > parse the override-edid. Moreover, infoframe generation should not > > be keyed to whether the monitor is HDMI or not, CEA-861B allows non- > > HDMI to send infoframes. > > - only send audio if audio and infoframes are supported. > > > > Otherwise, these are very much like the previous posting of the series, > > except rebased upon the mali/hdlcd/tda998x change to remove the > > drm_connector_register() call. > > > > https://www.spinics.net/lists/dri-devel/msg121495.html > > > > It'd be nice to have other tda998x users ack and test these patches, > > I've just merged your drm-tda998x-devel branch and booted it on my Juno, > and I can at least confirm that DVI still seems to still work as before. > Anything in particular I should be looking out for? (Unfortunately the > only handy HDMI monitor is one of those slightly-out-of-spec ones which > has never really worked with the pixel clocks Juno provides) Unfortunately, my drm-tda998x-devel branch is slightly out of date with these patches it's the original set of 10 patches. I've not pushed these ones out to that branch yet, as I've three additional patches on top of these which aren't "ready" for pushing out. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[PATCH RFC 00/12] tda998x updates
Hi Russell, On 08/11/16 12:24, Russell King - ARM Linux wrote: > As no one responded to the previous round, I'm not spending soo much > time writing up a description of these changes again. It's also been > quite a long time, so I've forgotten all the details of the changes, > so I'll do my best. > > Changes from the previous series include: > - reorder the initial three patches > - change the (now third patch)... I think to increase the size of the > locked region. > - fix edid parsing for infoframe generation - as was pointed out for > dw-hdmi, parsing the EDID in get_modes() is incorrect, as that method > will not be called when an override-edid is in effect. We need to > parse the override-edid. Moreover, infoframe generation should not > be keyed to whether the monitor is HDMI or not, CEA-861B allows non- > HDMI to send infoframes. > - only send audio if audio and infoframes are supported. > > Otherwise, these are very much like the previous posting of the series, > except rebased upon the mali/hdlcd/tda998x change to remove the > drm_connector_register() call. > > https://www.spinics.net/lists/dri-devel/msg121495.html > > It'd be nice to have other tda998x users ack and test these patches, I've just merged your drm-tda998x-devel branch and booted it on my Juno, and I can at least confirm that DVI still seems to still work as before. Anything in particular I should be looking out for? (Unfortunately the only handy HDMI monitor is one of those slightly-out-of-spec ones which has never really worked with the pixel clocks Juno provides) Robin. > I've tried to test on Juno, but the Juno situation seems to be a huge > fail. (HBI0282B completely fails with latest firmware - (a) FPGA image > incompatibilities io_b118 causes all FPGA AMBA devices to vanish, (b) > seems no way to get SCPI support on it - adding the BL0 executable > start address in the SCC registers seems to be incompatible with the > devchip, causing the PLLs to fail. In discussion with Sudeep over > these issues, but no idea where things are with it at the moment, other > than Sudeep needs to investigate. All Linaro firmware releases are > broken on HBI0282B.) > > drivers/gpu/drm/i2c/tda998x_drv.c | 826 > -- > 1 file changed, 429 insertions(+), 397 deletions(-) >
[PATCH 2/2] drm: Restrict stackdepot usage to builtin drm.ko
I misread the kbuild result thinking that we had missed the include
(which we had for completeness anyway), what kbuild was actually warning
me about was that depot_save_stack was not exported.
Temporarily fix this by only selecting STACKDEPOT iff drm.ko is builtin
Reported-by: kbuild test robot
Fixes: 5705670d0463 ("drm: Track drm_mm allocators and show leaks on shutdown")
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 25e8e3793d29..d6ee0592b625 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -36,7 +36,7 @@ config DRM_DP_AUX_CHARDEV
config DRM_DEBUG_MM
bool "Insert extra checks and debug info into the DRM range managers"
default n
- depends on DRM
+ depends on DRM=y
select STACKDEPOT
help
Enable allocation tracking of memory manager and leak detection on
--
2.10.2
[PATCH 1/2] drm/i915: Restrict DRM_DEBUG_MM automatic selection
DRM_DEBUG_MM is only valid if the DRM.ko is builtin as currently
depot_save_stack is not exported.
Fixes: 5c7fcf2db027 ("drm/i915: Enable drm_mm debug when enabling
DRM_I915_DEBUG")
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/Kconfig.debug | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/Kconfig.debug
b/drivers/gpu/drm/i915/Kconfig.debug
index 69edf196ed03..51ba630a134b 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -21,7 +21,7 @@ config DRM_I915_DEBUG
select PREEMPT_COUNT
select X86_MSR # used by igt/pm_rpm
select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks)
-select DRM_DEBUG_MM
+select DRM_DEBUG_MM if DRM=y
default n
help
Choose this option to turn on extra driver debugging that may affect
--
2.10.2
[PATCH] drm: Add stackdepot include for DRM_DEBUG_MM
On Tue, Nov 08, 2016 at 01:46:15PM +0100, Daniel Vetter wrote:
> On Tue, Nov 08, 2016 at 11:56:01AM +, Chris Wilson wrote:
> > 0day found that stackdepot.h doesn't get automatically included on all
> > architectures, so remember to add our #include.
> >
> > Reported-by: kbuild test robot
> > Fixes: 5705670d0463 ("drm: Track drm_mm allocators and show leaks on
> > shutdown")
> > Signed-off-by: Chris Wilson
> > Cc: Daniel Vetter
>
> Thx for the fast fixup, applied to drm-misc.
And I misread the kbuild result. Apparently we all build drm.ko as a
builtin and kbuild doesn't.
depot_save_stack is not exported :(
Is
- depends on DRM
+ depends on DRM=y
a good temporary? Will have to munge i915 similarly. :(
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[v2] drm/rockchip: Use an enum to identify Rockchip VOPs
On Thu, Nov 03, 2016 at 12:46:48PM -0700, Kristian Høgsberg wrote:
> We used to call drm_of_encoder_active_endpoint_id() from
> rockchip_dp_drm_encoder_atomic_check() to determine the endpoint for the
> active encoder. However, the encoder isn't necessarily active at this
> point or it may be connected to different crtc than what we're switching
> to. Instead, look at the crtc from the drm_crtc_state. This fixes wrong
> colors when driving the eDP with the big VOP. Further, we can identify
> the type of VOP we're dealing with by just putting a VOP id enum in the
> vop_data.
>
> On way to test this is to use the modetest tool from libdrm:
>
> $ modetest -M rockchip -s 32 at 28:2400x1600
>
> which displays dark or black colors because we fail to look up the
> endpoint and use default 0 (which is ROCKCHIP_OUT_MODE_P888) for big
> VOP instead of RGB10 as required.
>
> For reference,
>
> $ modetest -M rockchip -s 32 at 25:2400x1600
>
> drives the eDP from little VOP and displays correctly.
>
> Signed-off-by: Kristian H. Kristensen
> ---
> v2: Stripped chromeos annotations, fix compile errors for drivers I didn't
> compile when I first wrote the patch.
>
[ ... ]
>
> +enum vop_id vop_get_crtc_vop_id(struct drm_crtc *crtc)
> +{
> + struct vop *vop = to_vop(crtc);
> +
> + return vop->data->id;
> +}
Missing EXPORT() causes build errors if calling code is built as module.
Example arm64:allmodconfig:
ERROR: "vop_get_crtc_vop_id" [drivers/gpu/drm/rockchip/dw-mipi-dsi.ko]
undefined!
ERROR: "vop_get_crtc_vop_id" [drivers/gpu/drm/rockchip/cdn-dp.ko] undefined!
ERROR: "vop_get_crtc_vop_id" [drivers/gpu/drm/rockchip/analogix_dp-rockchip.ko]
undefined!
Guenter
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 01:43:40PM +0100, Daniel Vetter wrote: > On Tue, Nov 08, 2016 at 11:45:51AM +, Chris Wilson wrote: > > On Tue, Nov 08, 2016 at 12:32:56PM +0100, Daniel Vetter wrote: > > > On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: > > > > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > > > > > From: Gustavo Padovan > > > > > > > > > > Hi, > > > > > > > > > > This is yet another version of the DRM fences patches. Please refer > > > > > to the cover letter[1] in a previous version to check for more > > > > > details. > > > > > > > > Explicit fencing is not a superset of the implicit fences. The driver > > > > may be using implicit fences (on a reservation object) to serialise > > > > asynchronous operations wrt to each other (such as dispatching threads > > > > to flush cpu caches to memory, manipulating page tables and the like > > > > before the flip). Since the user doesn't know about these operations, > > > > they are not included in the explicit fence they provide, at which point > > > > we can't trust their fence to the exclusion of the implicit fences... > > > > > > My thoughts are that in atomic_check drivers just fill in the fence from > > > the reservation_object (i.e. the uapi implicit fencing part). If there's > > > any additional work that's queued up in ->prepare_fb then I guess the > > > driver needs to track that internally, but _only_ for kernel-internally > > > queued work. > > > > That's not a trivial task to work out which of the fence contexts within > > the reservation object are required and which are to be replaced by the > > explicit fence, esp. when you have to consider external fences. > > Hm, what kind of async kernel tasks are you thinking off? Atm I don't know > of anyone who does e.g. clflush through the gpu. And ttm bo placement > moves for display should be explicit enough that drivers will deal with > them correctly. At least that seems to have been the conclusion from the > long amdgpu thread. Now that we (i915) serialise on an reservation_object (obj->resv), we have floated ideas to use that to serialise async tasks (such as offloading the 100ms clflush to a (cpu) worker, a gpu task would pose a similar problem with a fence inserted that is not exposed to userspace). Also tempted to look at using async tasks + fences to do GTT updates but that is not a common pain point at the moment, and cases where it is the GTT thrashing itself is the issue. So how does i915 deal with ttm bo fences? -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH v2] drm/i915: don't whitelist oacontrol in cmd parser
This v2 patch bumps the command parser version so it can be referenced in
corresponding i-g-t gem_exec_parse changes.
--- >8 ---
Being able to program OACONTROL from a non-privileged batch buffer is
not sufficient to be able to configure the OA unit. This was originally
allowed to help enable Mesa to expose OA counters via the
INTEL_performance_query extension, but the current implementation based
on programming OACONTROL via a batch buffer isn't able to report useable
data without a more complete OA unit configuration. Mesa handles the
possibility that writes to OACONTROL may not be allowed and so only
advertises the extension after explicitly testing that a write to
OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
should be ok for userspace.
Removing this simplifies adding a new kernel api for configuring the OA
unit without needing to consider the possibility that userspace might
trample on OACONTROL state which we'd like to start managing within
the kernel instead. In particular running any Mesa based GL application
currently results in clearing OACONTROL when initializing which would
disable the capturing of metrics.
v2:
This bumps the command parser version from 8 to 9, as the change is
visible to userspace.
Signed-off-by: Robert Bragg
Reviewed-by: Matthew Auld
Reviewed-by: Sourab Gupta
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 42 --
1 file changed, 5 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index c9d2ecd..f5762cd 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -450,7 +450,6 @@ static const struct drm_i915_reg_descriptor
gen7_render_regs[] = {
REG64(PS_INVOCATION_COUNT),
REG64(PS_DEPTH_COUNT),
REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
- REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below. */
REG64(MI_PREDICATE_SRC0),
REG64(MI_PREDICATE_SRC1),
REG32(GEN7_3DPRIM_END_OFFSET),
@@ -1060,8 +1059,7 @@ bool intel_engine_needs_cmd_parser(struct intel_engine_cs
*engine)
static bool check_cmd(const struct intel_engine_cs *engine,
const struct drm_i915_cmd_descriptor *desc,
const u32 *cmd, u32 length,
- const bool is_master,
- bool *oacontrol_set)
+ const bool is_master)
{
if (desc->flags & CMD_DESC_SKIP)
return true;
@@ -1099,31 +1097,6 @@ static bool check_cmd(const struct intel_engine_cs
*engine,
}
/*
-* OACONTROL requires some special handling for
-* writes. We want to make sure that any batch which
-* enables OA also disables it before the end of the
-* batch. The goal is to prevent one process from
-* snooping on the perf data from another process. To do
-* that, we need to check the value that will be written
-* to the register. Hence, limit OACONTROL writes to
-* only MI_LOAD_REGISTER_IMM commands.
-*/
- if (reg_addr == i915_mmio_reg_offset(GEN7_OACONTROL)) {
- if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
- DRM_DEBUG_DRIVER("CMD: Rejected LRM to
OACONTROL\n");
- return false;
- }
-
- if (desc->cmd.value == MI_LOAD_REGISTER_REG) {
- DRM_DEBUG_DRIVER("CMD: Rejected LRR to
OACONTROL\n");
- return false;
- }
-
- if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1))
- *oacontrol_set = (cmd[offset + 1] != 0);
- }
-
- /*
* Check the value written to the register against the
* allowed mask/value pair given in the whitelist entry.
*/
@@ -1214,7 +1187,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
u32 *cmd, *batch_end;
struct drm_i915_cmd_descriptor default_desc = noop_desc;
const struct drm_i915_cmd_descriptor *desc = &default_desc;
- bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd() */
bool needs_clflush_after = false;
int ret = 0;
@@ -1270,8 +1242,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
break;
}
- if (!check_cmd(engine, desc, cmd, length, is_master,
- &oacontrol_set)) {
+
[PATCH 2/2] drm/edid: Consider alternate cea timings to be the same VIC
On 03.11.2016 13:53, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä
>
> CEA-861 specifies that the vertical front porch may vary by one or two
> lines for specific VICs. Up to now we've only considered a mode to match
> the VIC if it matched the shortest possible vertical front porch length
> (as that is the variant we store in cea_modes[]). Let's allow our VIC
> matching to work with the other timings variants as well so that that
> we'll send out the correct VIC if the variant actually used isn't the
> one with the shortest vertical front porch.
>
> Cc: Shashank Sharma
> Cc: Andrzej Hajda
> Cc: Adam Jackson
> Signed-off-by: Ville Syrjälä
I have checked against specification and it looks OK.
I have few nitpicks below regarding implementation, but this could be
matter of taste, so:
Reviewed-by: Andrzej Hajda
> ---
> drivers/gpu/drm/drm_edid.c | 66
> +-
> 1 file changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7eec18925b70..728990fee4ef 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2613,6 +2613,41 @@ cea_mode_alternate_clock(const struct drm_display_mode
> *cea_mode)
> return clock;
> }
>
> +static bool
> +cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode)
> +{
> + /*
> + * For certain VICs the spec allows the vertical
> + * front porch to vary by one or two lines.
> + *
> + * cea_modes[] stores the variant with the shortest
> + * vertical front porch. We can adjust the mode to
> + * get the other variants by simply increasing the
> + * vertical front porch length.
> + */
> + BUILD_BUG_ON(edid_cea_modes[8].vtotal != 262 ||
> + edid_cea_modes[9].vtotal != 262 ||
> + edid_cea_modes[12].vtotal != 262 ||
> + edid_cea_modes[13].vtotal != 262 ||
> + edid_cea_modes[23].vtotal != 312 ||
> + edid_cea_modes[24].vtotal != 312 ||
> + edid_cea_modes[27].vtotal != 312 ||
> + edid_cea_modes[28].vtotal != 312);
I am not sure if this compile check is really necessary,
I would rather put comment before edid_cea_modes array
which mode should be put into array in multi-mode case.
> +
> + if (((vic == 8 || vic == 9 ||
> + vic == 12 || vic == 13) && mode->vtotal < 263) ||
> + ((vic == 23 || vic == 24 ||
> + vic == 27 || vic == 28) && mode->vtotal < 314)) {
I wonder if it wouldn't be better to mark somehow these modes
in the array as alternating ones. This way all info about cea modes
will be in one place. For example by (ab)using private_flags:
/* 8 - 720(1440)x240 at 60Hz */
{ DRM_MODE("720x240", DRM_MODE_TYPE_DRIVER, 13500, 720, 739,
801, 858, 0, 240, 244, 247, 262, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
DRM_MODE_FLAG_DBLCLK),
.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
.private_flags = CEA_MODE_FLAG_VFP2},
This is of course just an idea, I am not sure if not overkill :)
> + mode->vsync_start++;
> + mode->vsync_end++;
> + mode->vtotal++;
> +
> + return true;
> + }
> +
> + return false;
> +}
> +
> static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode
> *to_match,
>unsigned int clock_tolerance)
> {
> @@ -2622,19 +2657,21 @@ static u8 drm_match_cea_mode_clock_tolerance(const
> struct drm_display_mode *to_m
> return 0;
>
> for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> - const struct drm_display_mode *cea_mode = &edid_cea_modes[vic];
> + struct drm_display_mode cea_mode = edid_cea_modes[vic];
> unsigned int clock1, clock2;
>
> /* Check both 60Hz and 59.94Hz */
> - clock1 = cea_mode->clock;
> - clock2 = cea_mode_alternate_clock(cea_mode);
> + clock1 = cea_mode.clock;
> + clock2 = cea_mode_alternate_clock(&cea_mode);
>
> if (abs(to_match->clock - clock1) > clock_tolerance &&
> abs(to_match->clock - clock2) > clock_tolerance)
> continue;
>
> - if (drm_mode_equal_no_clocks(to_match, cea_mode))
> - return vic;
> + do {
> + if (drm_mode_equal_no_clocks_no_stereo(to_match,
> &cea_mode))
> + return vic;
> + } while (cea_mode_alternate_timings(vic, &cea_mode));
> }
>
> return 0;
> @@ -2655,18 +2692,23 @@ u8 drm_match_cea_mode(const struct drm_display_mode
> *to_match)
> return 0;
>
> for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> - const struct drm_display_mode *cea_mode
[PATCH v7 0/3] drm: add explict fencing
On Tue, Nov 08, 2016 at 10:35:08AM +, Chris Wilson wrote: > On Tue, Nov 08, 2016 at 03:54:47PM +0900, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > Hi, > > > > This is yet another version of the DRM fences patches. Please refer > > to the cover letter[1] in a previous version to check for more details. > > Explicit fencing is not a superset of the implicit fences. The driver > may be using implicit fences (on a reservation object) to serialise > asynchronous operations wrt to each other (such as dispatching threads > to flush cpu caches to memory, manipulating page tables and the like > before the flip). Since the user doesn't know about these operations, > they are not included in the explicit fence they provide, at which point > we can't trust their fence to the exclusion of the implicit fences... My thoughts are that in atomic_check drivers just fill in the fence from the reservation_object (i.e. the uapi implicit fencing part). If there's any additional work that's queued up in ->prepare_fb then I guess the driver needs to track that internally, but _only_ for kernel-internally queued work. The reason for that is that with explicit fencing we want to allow userspace to overwrite any existing implicit fences that might hang around. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] drm: panel: simple-panel: get the enable gpio as-is
On Mon, Nov 07, 2016 at 03:26:56PM +0100, Philipp Zabel wrote: > Am Montag, den 07.11.2016, 14:17 +0100 schrieb Thierry Reding: > > On Mon, Nov 07, 2016 at 06:12:43PM +0800, Chen-Yu Tsai wrote: > > > On Sun, Nov 6, 2016 at 7:09 PM, Icenowy Zheng wrote: > > > > The enable gpio of simple-panel may be used by a simplefb or other > > > > driver on the panel's display before the KMS driver get load. > > > > > > > > Get the GPIO as-is, so the panel won't be disabled, and the simplefb > > > > can work. > > > > > > > > Signed-off-by: Icenowy Zheng > > > > --- > > > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > > > b/drivers/gpu/drm/panel/panel-simple.c > > > > index 113db3c..ccee4c1 100644 > > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > > @@ -312,7 +312,7 @@ static int panel_simple_probe(struct device *dev, > > > > const struct panel_desc *desc) > > > > return PTR_ERR(panel->supply); > > > > > > > > panel->enable_gpio = devm_gpiod_get_optional(dev, "enable", > > > > -GPIOD_OUT_LOW); > > > > +GPIOD_ASIS); > > > > > > The GPIO requested as-is might be in input mode. You should change the > > > gpiod_set_value calls to gpiod_direction_output calls. The later also > > > allows you to give an initial value. Not sure if it checks for cansleep > > > like the set_value calls though. > > > > I'd prefer not to add gpiod_direction_output() calls outside of > > ->probe(). Instead, could we make this patch be smart about taking over > > from an earlier user? Could it read the current direction and value of > > the GPIO and not disable it if it had previously been enabled? > > Seconded, the PWM backlight driver in drivers/video/backlight/pwm_bl.c > already does a similar thing. > > > And even if we go this extra mile there's a possibility that the GPIO > > was just left dangling by earlier software (or hardware) and leaving it > > on would actually be worse than turning the panel off. > > Is this something the encoder driver should communicate to the panel? > That one will know whether its atomic_reset state is enabled or > disabled. At that point it's already too late. Configuration of the GPIO here is within the panel's ->probe() implementation, which is before it can ever even get attached to an encoder/connector. I can't think of a proper solution for this, but perhaps the best heuristic would be to request with GPIOD_ASIS and then query the direction. If it's configured as output we could assume that somebody's configured it explicitly and it has the correct value. That could still break existing use-cases, but it would at least allow basic hand-over. Also, if leaving the GPIO configured as-is causes glitches or other issues, then these are observable with the current code as well, until the panel driver takes over and disables everything. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161108/e0f3dae7/attachment.sig>
[PATCH] drm: tda998x: mali-dp: hdlcd: refactor connector registration
On Tue, Nov 08, 2016 at 10:59:43AM +, Russell King - ARM Linux wrote: > On Tue, Nov 08, 2016 at 10:25:52AM +0100, Daniel Vetter wrote: > > Hm, I entirely missed that part of the troubles. Anyway, if you all agree > > on a patch I certainly won't block it, feel free to merge through suitable > > trees (or I can smash it into drm-misc if that's wanted). > > I think those who are interested in seeing the drm_connector_register() > call disappear from tda998x only care about that happening, but not how > it happens. > > We have agreement between myself, Brian and Liviu on this approach, and > I think everyone else is waiting for me to push out the commit so it can > be used as the basis for their work. I think everyone else is waiting > for me to push something out which gets us past this log-jam. > > I don't understand the connectivity between drm-misc and David's drm > tree - so I'm going to let you make the decision on whether to merge > this into drm-misc. I normally send my pull requests for Armada and > TDA998x changes to David, which means when I send my other TDA998x > changes, the mali/tda998x commit will be included in that pull > request too. So I'm wondering whether it would make more sense for > me to send it to David instead, or whether I need to send my other > changes through drm-misc instead. I find the whole drm vs drm-misc > thing rather confusing. > > I think we should get this accepted into drm trees before anyone bases > their work on this commit (which is why I've been holding off during > the last week, waiting for DRM folk to get back from Santa Fe and > readjust to the higher atmospheric pressure!) > > Anyway, here is my pull request for the mali/hdlcd/tda998x commit which > I'd normally send to David - I don't mind which tree it goes into as > long as things work out nicely. drm-misc is just the collector for when it doesn't make sense to have a driver or topic/feature pull request (or there isn't really a permanent driver tree). Pull to Dave directly makes sense. -Daniel > 8<=== > > David, > > Please incorporate the latest TDA998x I2C driver (drm-tda998x-mali > branch), which can be found at: > > git://git.armlinux.org.uk/~rmk/linux-arm.git drm-tda998x-mali > > with SHA1 90731c24d2db7ec04df43ddbcee9605183d05187. > > This change removes the call to drm_connector_register() which has been > blocking the proper de-midlayer conversion of other DRM drivers. > Unfortunately, hdlcd and mali have intimate dependencies on this change, > which is why these drivers need to be fixed up in the same commit - they > can't be separate commits without these drivers breaking. All other > DRM drivers which make use of tda998x (to my knowledge - armada, tilcdc) > cope with this change. > > This will update the following files: > > drivers/gpu/drm/arm/hdlcd_drv.c | 19 +++ > drivers/gpu/drm/arm/malidp_drv.c | 18 +++--- > drivers/gpu/drm/i2c/tda998x_drv.c | 8 > 3 files changed, 22 insertions(+), 23 deletions(-) > > through these changes: > > Brian Starkey (1): > drm/i2c: tda998x: mali-dp: hdlcd: refactor connector registration > > Many thanks. > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
