[Bug 194559] amdgpu problems loading 2 firmwares on multi-smp system
https://bugzilla.kernel.org/show_bug.cgi?id=194559 --- Comment #4 from Janpieter Sollie (janpieter.sol...@dommel.be) --- Created attachment 254741 --> https://bugzilla.kernel.org/attachment.cgi?id=254741&action=edit config of working drm-next kernel -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 194559] amdgpu problems loading 2 firmwares on multi-smp system
https://bugzilla.kernel.org/show_bug.cgi?id=194559 Janpieter Sollie (janpieter.sol...@dommel.be) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |PATCH_ALREADY_AVAILABLE -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 194559] amdgpu problems loading 2 firmwares on multi-smp system
https://bugzilla.kernel.org/show_bug.cgi?id=194559 --- Comment #5 from fin4...@hotmail.com --- (In reply to Janpieter Sollie from comment #3) > it works! attached my config file of your drm-next kernel > I don't know what needs to be done for you developers to integrate drm-next > into the mainline kernel, but thank you!!! Amd should warn not use stock kernels and tell how to use use ~agd5f wip kernel and latest mesa git. Here is the page for you, dear Amd: http://support.amd.com/en-us/download/linux This and many other amdgpu bug reports prove my point. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: [PATCH 2/8] ARM: sun4i: Add display blocks for the sun4i dtsi.
Hi,
On Mon, Feb 13, 2017 at 06:46:45PM +0200, Priit Laes wrote:
> On Mon, 2017-02-13 at 17:20 +0800, Chen-Yu Tsai wrote:
> > On Mon, Feb 13, 2017 at 5:16 PM, Maxime Ripard
> > wrote:
> > > Hi,
> > >
> > > On Sat, Feb 11, 2017 at 07:43:59PM +0200, Priit Laes wrote:
> > > > Added basic display pipeline consisting of tcon, display backend
> > > > and
> > > > frontend blocks.
> > > >
> > > > Signed-off-by: Priit Laes
> > > > ---
> > > > arch/arm/boot/dts/sun4i-a10.dtsi | 104
> > > > +++
> > > > 1 file changed, 104 insertions(+)
> > > >
> > > > diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi
> > > > b/arch/arm/boot/dts/sun4i-a10.dtsi
> > > > index ba20b48..70991c9 100644
> > > > --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> > > > +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> > > > @@ -779,6 +779,45 @@
> > > > #size-cells = <0>;
> > > > };
> > > >
> > > > + tcon0: lcd-controller@1c0c000 {
> > > > + compatible = "allwinner,sun5i-a13-tcon";
> > >
> > > There's a few bits here and there that need to be setup differently
> > > in
> > > A10, so you cannot reuse that compatible (same thing for the
> > > other).
> > >
> > > Also, I'd really like to have all the blocks listed here, and not
> > > only
> > > the first pipeline. Ideally, on the A10, the two pipelines should
> > > be
> > > enabled too.
> >
> > The display pipeline driver has to be fixed before that can happen
> > though. And I haven't started to work on what I proposed yet. Though
> > if someone wants to take over I can forward any design plans I have.
>
> Well, my plan was to get at least minimum bits mainlined and then build
> additional features on top.
I understand that, but now that we have the DT stability to maintain,
I want to make sure that everything is correctly represented from day
one, with code that works.
Otherwise, we might or might not be able to fix things properly later
down the road (or at the expense of ugly hacks).
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 194559] amdgpu problems loading 2 firmwares on multi-smp system
https://bugzilla.kernel.org/show_bug.cgi?id=194559 --- Comment #6 from Michel Dänzer (mic...@daenzer.net) --- (In reply to fin4478 from comment #5) > This and many other amdgpu bug reports prove my point. Your bug report comments like this one rather indicate that you don't understand how the kernel development process works. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 118701] x86_64 GMA500 reports /dev/dri/card0: failed to set DRM interface version 1.4: Inappropriate ioctl for device
https://bugzilla.kernel.org/show_bug.cgi?id=118701 Stuart Foster (smf.li...@ntlworld.com) changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |CODE_FIX --- Comment #8 from Stuart Foster (smf.li...@ntlworld.com) --- Hi Patrik, Yes everything is ok now. Thanks. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] drm/ttm, amdgpu: fix use-after-free in recent deadlock fix
Hi all, based on my current theory on how a deadlock could happen in the buffer allocation code, these two patches should fix the deadlock without having a use-after-free. I'm still working on a way to clean up the ttm_bo_init sequence overall, but I'm separating these two out for a hopefully quick fix, since in my book use-after-frees are worse than deadlocks. Samuel, I'd very much appreciate if you could check that the deadlocks in Hitman remain fixed with these two patches. Thanks, Nicolai ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] Revert "drm/amdgpu: fix a potential deadlock in amdgpu_bo_create_restricted()"
From: Nicolai Hähnle
This reverts commit 38fc4856ad98f230bc91da0421dec69e4aee40f8, which
introduces a use-after-free.
The underlying bug should be properly fixed with "drm/ttm: never add BO
that failed to validate to the LRU list".
Cc: Samuel Pitoiset
Cc: zhoucm1
Signed-off-by: Nicolai Hähnle
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 556236a..d1ef1d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -403,11 +403,8 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
&bo->placement, page_align, !kernel, NULL,
acc_size, sg, resv ? resv : &bo->tbo.ttm_resv,
&amdgpu_ttm_bo_destroy);
- if (unlikely(r != 0)) {
- if (!resv)
- ww_mutex_unlock(&bo->tbo.resv->lock);
+ if (unlikely(r != 0))
return r;
- }
bo->tbo.priority = ilog2(bo->tbo.num_pages);
if (kernel)
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/ttm: never add BO that failed to validate to the LRU list
From: Nicolai Hähnle
Fixes a potential race condition in amdgpu that looks as follows:
Task 1: attempt ttm_bo_init, but ttm_bo_validate fails
Task 1: add BO to global list anyway
Task 2: grabs hold of the BO, waits on its reservation lock
Task 1: releases its reference of the BO; never gives up the
reservation lock
The patch "drm/amdgpu: fix a potential deadlock in
amdgpu_bo_create_restricted()" attempts to fix that by releasing
the reservation lock in amdgpu code; unfortunately, it introduces
a use-after-free when this race _doesn't_ happen.
This patch should fix the race properly by never adding the BO
to the global list in the first place.
Cc: Samuel Pitoiset
Cc: zhoucm1
Signed-off-by: Nicolai Hähnle
---
drivers/gpu/drm/ttm/ttm_bo.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 239a957..76bee42 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1215,18 +1215,20 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
if (likely(!ret))
ret = ttm_bo_validate(bo, placement, interruptible, false);
- if (!resv) {
+ if (!resv)
ttm_bo_unreserve(bo);
- } else if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
+ if (unlikely(ret)) {
+ ttm_bo_unref(&bo);
+ return ret;
+ }
+
+ if (resv && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
spin_lock(&bo->glob->lru_lock);
ttm_bo_add_to_lru(bo);
spin_unlock(&bo->glob->lru_lock);
}
- if (unlikely(ret))
- ttm_bo_unref(&bo);
-
return ret;
}
EXPORT_SYMBOL(ttm_bo_init);
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 0/4] libdrm: add new GPU sensor interface for AMDGPU
Am 14.02.2017 um 02:44 schrieb Emil Velikov: On 14 February 2017 at 00:08, Samuel Pitoiset wrote: Hi, This series implements a new GPU sensor interface which will allow to query the current temperature and the shader/memory clocks via the GALLIUM_HUD. This adds the same functionality as the Radeon driver. The first patch of the series syncs amdgpu_drm.h with kernel headers. Branch is here: https://cgit.freedesktop.org/~hakzsam/drm/log/?h=si_clocks_temp Please review. Thanks! Samuel Pitoiset (4): amdgpu: sync amdgpu_drm.h with the kernel Samuel, Please read through drm/include/drm/README in particular "When and how to update these files". JFYI multiple people are trying to get various UABI in, so do keep an eye open for fireworks. amdgpu: add new GPU sensor related interface amdgpu: allow to query GPU sensor related information You'll need to add the new interface(s) to the symbols test. `make check' must pass. Bump version for 2.4.76 release Skim through RELEASING for details on how to roll one. Pardon, if I'm flocking up a dead horse here. Please keep an eye open on this Emil. Additionally to what said before you didn't used upstream, but rather the branch including the DC code (with the freesync IOCTL). We are probably not getting this upstream like that, so pushing it into libdrm is clearly not a good idea. Regards, Christian. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/amd/dc: resource: fix semicolon.cocci warnings (fwd)
Am 14.02.2017 um 07:21 schrieb Julia Lawall:
Remove unneeded semicolon.
Generated by: scripts/coccinelle/misc/semicolon.cocci
CC: Harry Wentland
Signed-off-by: Julia Lawall
Signed-off-by: Fengguang Wu
Acked-by: Christian König for this one as
well as the other similar patches.
Thanks for letting us know about those typos.
Regards,
Christian.
---
v2: make subject line unique
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.9
head: 79d2de1bcb650296adff1cb08bfbf1501a6e6e14
commit: bad4c165a6986a131cdd1455507ba3857baaa561 [201/657] drm/amd/dc: Add
dc display driver
dc_resource.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c
@@ -135,7 +135,7 @@ static void update_num_audio(
break;
default:
DC_ERR("DC: unexpected audio fuse!\n");
- };
+ }
}
bool resource_construct(
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 97879] [amdgpu] Rocket League: long hangs (several seconds) when loading assets (models/textures/shaders?)
https://bugs.freedesktop.org/show_bug.cgi?id=97879 --- Comment #91 from Andreas Hartmetz --- Steam has now shipped the game side fix as well. It feels almost like a new game, much much better. -- You are receiving this mail because: You are the assignee for the bug. You are on the CC list for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 194579] AMDGPU: Possible size overflow detected by PaX in ttm_bo_handle_move_mem (drivers/gpu/drm/ttm/ttm_bo.c:388)
https://bugzilla.kernel.org/show_bug.cgi?id=194579 Christian König (deathsim...@vodafone.de) changed: What|Removed |Added CC||deathsim...@vodafone.de --- Comment #3 from Christian König (deathsim...@vodafone.de) --- Please ignore the incorrect comment by fin4...@hotmail.com. The upstream kernel the official base amdgpu driver code. Only a few not parts which are not upstream yet are in private repositories waiting for cleanup. Regards, Christian (co-maintainer of the amdgpu module). -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 194579] AMDGPU: Possible size overflow detected by PaX in ttm_bo_handle_move_mem (drivers/gpu/drm/ttm/ttm_bo.c:388)
https://bugzilla.kernel.org/show_bug.cgi?id=194579 --- Comment #4 from Christian König (deathsim...@vodafone.de) --- Now back to topic, what code base is this? Could you post the branch you are using somewhere? In the upstream kernel the line at ttm_bo.c:388 is just an assignment and can't cause any overflow. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm/ttm, amdgpu: fix use-after-free in recent deadlock fix
On 02/14/2017 10:18 AM, Nicolai Hähnle wrote: Hi all, based on my current theory on how a deadlock could happen in the buffer allocation code, these two patches should fix the deadlock without having a use-after-free. I'm still working on a way to clean up the ttm_bo_init sequence overall, but I'm separating these two out for a hopefully quick fix, since in my book use-after-frees are worse than deadlocks. Samuel, I'd very much appreciate if you could check that the deadlocks in Hitman remain fixed with these two patches. Sure, I will check. Thanks, Nicolai ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99552] Make Advanced Simulation Library (ASL) OpenCL GPU support work on Clover and RadeonSI
https://bugs.freedesktop.org/show_bug.cgi?id=99552 Vedran Miletić changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #3 from Vedran Miletić --- Fixed https://reviews.llvm.org/D27283 https://reviews.llvm.org/rL294786 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99553] Tracker bug for runnning OpenCL applications on Clover
https://bugs.freedesktop.org/show_bug.cgi?id=99553 Bug 99553 depends on bug 99552, which changed state. Bug 99552 Summary: Make Advanced Simulation Library (ASL) OpenCL GPU support work on Clover and RadeonSI https://bugs.freedesktop.org/show_bug.cgi?id=99552 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/i915: Use helper for setting plane->state->fb
We are told not to set plane_state->fb directly, but use drm_atomic_set_fb_for_plane() instead. Using the helper, means one less piece of code that needs fixing should the interface change... Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_display.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 04ca4e7bbbe0..7f7f6de011a4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2666,11 +2666,7 @@ update_state_fb(struct drm_plane *plane) if (plane->fb == plane->state->fb) return; - if (plane->state->fb) - drm_framebuffer_unreference(plane->state->fb); - plane->state->fb = plane->fb; - if (plane->state->fb) - drm_framebuffer_reference(plane->state->fb); + drm_atomic_set_fb_for_plane(plane->state, plane->fb); } static void -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 0/4] libdrm: add new GPU sensor interface for AMDGPU
On 02/14/2017 10:43 AM, Christian König wrote: Am 14.02.2017 um 02:44 schrieb Emil Velikov: On 14 February 2017 at 00:08, Samuel Pitoiset wrote: Hi, This series implements a new GPU sensor interface which will allow to query the current temperature and the shader/memory clocks via the GALLIUM_HUD. This adds the same functionality as the Radeon driver. The first patch of the series syncs amdgpu_drm.h with kernel headers. Branch is here: https://cgit.freedesktop.org/~hakzsam/drm/log/?h=si_clocks_temp Please review. Thanks! Samuel Pitoiset (4): amdgpu: sync amdgpu_drm.h with the kernel Samuel, Please read through drm/include/drm/README in particular "When and how to update these files". JFYI multiple people are trying to get various UABI in, so do keep an eye open for fireworks. amdgpu: add new GPU sensor related interface amdgpu: allow to query GPU sensor related information You'll need to add the new interface(s) to the symbols test. `make check' must pass. Bump version for 2.4.76 release Skim through RELEASING for details on how to roll one. Pardon, if I'm flocking up a dead horse here. Please keep an eye open on this Emil. Additionally to what said before you didn't used upstream, but rather the branch including the DC code (with the freesync IOCTL). We are probably not getting this upstream like that, so pushing it into libdrm is clearly not a good idea. Thanks for your feedbacks. I will first wait for the kernel interface to be merged before making a new series. Regards, Christian. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm-intel:drm-intel-next-queued 11/59] drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: unknown field 'mock' specified in initializer
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued head: d892e9398ecf6defc7972a62227b77dad6be20bd commit: 953c7f82eb890085c60dbe22578e883d6837c674 [11/59] drm/i915: Provide a hook for selftests config: x86_64-randconfig-s2-02141638 (attached as .config) compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7 reproduce: git checkout 953c7f82eb890085c60dbe22578e883d6837c674 # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): In file included from drivers/gpu/drm/i915/selftests/i915_selftest.c:68: >> drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: unknown >> field 'mock' specified in initializer cc1: warnings being treated as errors >> drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: missing >> braces around initializer drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: (near initialization for 'mock_selftests[0].') In file included from drivers/gpu/drm/i915/selftests/i915_selftest.c:74: >> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: unknown >> field 'live' specified in initializer >> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: missing >> braces around initializer drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: (near initialization for 'live_selftests[0].') >> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: >> initialization from incompatible pointer type vim +/mock +11 drivers/gpu/drm/i915/selftests/i915_mock_selftests.h 5 * 6 * The function should be of type int function(void). It may be conditionally 7 * compiled using #if IS_ENABLED(DRM_I915_SELFTEST). 8 * 9 * Tests are executed in order by igt/drv_selftest 10 */ > 11 selftest(sanitycheck, i915_mock_sanitycheck) /* keep first (igt selfcheck) */ --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/amdgpu: fix lock cleanup during buffer creation
From: Nicolai Hähnle
Open-code the initial ttm_bo_validate call, so that we can properly
unlock the reservation lock when it fails. Also, properly destruct
the reservation object when the first part of TTM BO initialization
fails.
Actual deadlocks caused by the missing unlock should have been fixed
by "drm/ttm: never add BO that failed to validate to the LRU list",
superseding the flawed fix in commit 38fc4856ad98 ("drm/amdgpu: fix
a potential deadlock in amdgpu_bo_create_restricted()").
This change fixes remaining recursive locking errors that can be seen
with lock debugging enabled, and avoids the error of freeing a locked
mutex.
Fixes: 12a852219583 ("drm/amdgpu: improve AMDGPU_GEM_CREATE_VRAM_CLEARED
handling (v2)")
Signed-off-by: Nicolai Hähnle
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 32 +-
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index d1ef1d0..bea845f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -399,12 +399,34 @@ int amdgpu_bo_create_restricted(struct amdgpu_device
*adev,
locked = ww_mutex_trylock(&bo->tbo.ttm_resv.lock);
WARN_ON(!locked);
}
- r = ttm_bo_init(&adev->mman.bdev, &bo->tbo, size, type,
- &bo->placement, page_align, !kernel, NULL,
- acc_size, sg, resv ? resv : &bo->tbo.ttm_resv,
- &amdgpu_ttm_bo_destroy);
- if (unlikely(r != 0))
+ r = ttm_bo_init_top(&adev->mman.bdev, &bo->tbo, size, type,
+ page_align, NULL,
+ acc_size, sg, resv ? resv : &bo->tbo.ttm_resv,
+ &amdgpu_ttm_bo_destroy);
+ if (unlikely(r != 0)) {
+ if (!resv) {
+ ww_mutex_unlock(&bo->tbo.ttm_resv.lock);
+ reservation_object_fini(&bo->tbo.ttm_resv);
+ }
+ amdgpu_ttm_bo_destroy(&bo->tbo);
+ return r;
+ }
+
+ r = ttm_bo_validate(&bo->tbo, &bo->placement, !kernel, false);
+ if (unlikely(r != 0)) {
+ struct ttm_buffer_object *tbo = &bo->tbo;
+
+ if (!resv)
+ ww_mutex_unlock(&bo->tbo.ttm_resv.lock);
+ ttm_bo_unref(&tbo);
return r;
+ }
+
+ if (!(bo->tbo.mem.placement & TTM_PL_FLAG_NO_EVICT)) {
+ spin_lock(&bo->tbo.glob->lru_lock);
+ ttm_bo_add_to_lru(&bo->tbo);
+ spin_unlock(&bo->tbo.glob->lru_lock);
+ }
bo->tbo.priority = ilog2(bo->tbo.num_pages);
if (kernel)
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/ttm: fix the documentation of ttm_bo_init
From: Nicolai Hähnle As the comment says: callers of ttm_bo_init cannot rely on having the only reference to the BO when the function returns successfully. Signed-off-by: Nicolai Hähnle --- include/drm/ttm/ttm_bo_api.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index d44b8e4..7b2807f 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -521,7 +521,11 @@ extern int ttm_bo_init_top(struct ttm_bo_device *bdev, * As this object may be part of a larger structure, this function, * together with the @destroy function, * enables driver-specific objects derived from a ttm_buffer_object. - * On successful return, the object kref and list_kref are set to 1. + * + * On successful return, the caller owns an object kref to @bo. The kref and + * list_kref are usually set to 1, but note that in some situations, other + * tasks may already be holding references to @bo as well. + * * If a failure occurs, the function will call the @destroy function, or * kfree() if @destroy is NULL. Thus, after a failure, dereferencing @bo is * illegal and will likely cause memory corruption. -- 2.9.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/ttm: split BO structure initialization into a separate function
From: Nicolai Hähnle
Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.
Signed-off-by: Nicolai Hähnle
---
drivers/gpu/drm/ttm/ttm_bo.c | 62 +---
include/drm/ttm/ttm_bo_api.h | 45
2 files changed, 86 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
}
EXPORT_SYMBOL(ttm_bo_validate);
-int ttm_bo_init(struct ttm_bo_device *bdev,
- struct ttm_buffer_object *bo,
- unsigned long size,
- enum ttm_bo_type type,
- struct ttm_placement *placement,
- uint32_t page_alignment,
- bool interruptible,
- struct file *persistent_swap_storage,
- size_t acc_size,
- struct sg_table *sg,
- struct reservation_object *resv,
- void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+ struct ttm_buffer_object *bo,
+ unsigned long size,
+ enum ttm_bo_type type,
+ uint32_t page_alignment,
+ struct file *persistent_swap_storage,
+ size_t acc_size,
+ struct sg_table *sg,
+ struct reservation_object *resv,
+ void (*destroy) (struct ttm_buffer_object *))
{
int ret = 0;
unsigned long num_pages;
struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
- bool locked;
ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
if (ret) {
pr_err("Out of kernel memory\n");
- if (destroy)
- (*destroy)(bo);
- else
- kfree(bo);
return -ENOMEM;
}
num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
if (num_pages == 0) {
pr_err("Illegal buffer object size\n");
- if (destroy)
- (*destroy)(bo);
- else
- kfree(bo);
ttm_mem_global_free(mem_glob, acc_size);
return -EINVAL;
}
@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
ret = drm_vma_offset_add(&bdev->vma_manager, &bo->vma_node,
bo->mem.num_pages);
+ return ret;
+}
+EXPORT_SYMBOL(ttm_bo_init_top);
+
+int ttm_bo_init(struct ttm_bo_device *bdev,
+ struct ttm_buffer_object *bo,
+ unsigned long size,
+ enum ttm_bo_type type,
+ struct ttm_placement *placement,
+ uint32_t page_alignment,
+ bool interruptible,
+ struct file *persistent_swap_storage,
+ size_t acc_size,
+ struct sg_table *sg,
+ struct reservation_object *resv,
+ void (*destroy) (struct ttm_buffer_object *))
+{
+ bool locked;
+ int ret;
+
+ ret = ttm_bo_init_top(bdev, bo, size, type, page_alignment,
+ persistent_swap_storage, acc_size, sg, resv,
+ destroy);
+ if (ret) {
+ if (destroy)
+ (*destroy)(bo);
+ else
+ kfree(bo);
+ return ret;
+ }
+
/* passed reservation objects should already be locked,
* since otherwise lockdep will be angered in radeon.
*/
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index f195899..d44b8e4 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -453,6 +453,51 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev,
unsigned struct_size);
/**
+ * ttm_bo_init_top
+ *
+ * @bdev: Pointer to a ttm_bo_device struct.
+ * @bo: Pointer to a ttm_buffer_object to be initialized.
+ * @size: Requested size of buffer object.
+ * @type: Requested type of buffer object.
+ * @flags: Initial placement flags.
+ * @page_alignment: Data alignment in pages.
+ * @persistent_swap_storage: Usually the swap storage is deleted for buffers
+ * pinned in physical memory. If this behaviour is not desired, this member
+ * holds a pointer to a persistent shmem object. Typically, this would
+ * point to the shmem object backing a GEM object if TTM is used to back a
+ * GEM user interface.
+ * @acc_size: Accounted size for this object.
+ * @resv: Pointer to a reservation_object, or NULL to let ttm allocate one.
+ * @destroy:
Re: [PATCH 0/2] drm/ttm, amdgpu: fix use-after-free in recent deadlock fix
On 02/14/2017 10:18 AM, Nicolai Hähnle wrote: Hi all, based on my current theory on how a deadlock could happen in the buffer allocation code, these two patches should fix the deadlock without having a use-after-free. I'm still working on a way to clean up the ttm_bo_init sequence overall, but I'm separating these two out for a hopefully quick fix, since in my book use-after-frees are worse than deadlocks. Samuel, I'd very much appreciate if you could check that the deadlocks in Hitman remain fixed with these two patches. Looks good. Thanks! Thanks, Nicolai ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/i915: Use helper for setting plane->state->fb
On Tuesday, 2017-02-14 10:18:31 +, Chris Wilson wrote: > We are told not to set plane_state->fb directly, but use > drm_atomic_set_fb_for_plane() instead. Using the helper, means one less > piece of code that needs fixing should the interface change... > > Signed-off-by: Chris Wilson Reviewed-by: Eric Engestrom > --- > drivers/gpu/drm/i915/intel_display.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 04ca4e7bbbe0..7f7f6de011a4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2666,11 +2666,7 @@ update_state_fb(struct drm_plane *plane) > if (plane->fb == plane->state->fb) > return; > > - if (plane->state->fb) > - drm_framebuffer_unreference(plane->state->fb); > - plane->state->fb = plane->fb; > - if (plane->state->fb) > - drm_framebuffer_reference(plane->state->fb); > + drm_atomic_set_fb_for_plane(plane->state, plane->fb); > } > > static void > -- > 2.11.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98869] Electronic Super Joy graphic artefacts (regression,bisected)
https://bugs.freedesktop.org/show_bug.cgi?id=98869 --- Comment #23 from Marek Olšák --- You can take the default case statement in the unbyteswap function on LE. That should safely disable the whole thing on LE. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98869] Electronic Super Joy graphic artefacts (regression,bisected)
https://bugs.freedesktop.org/show_bug.cgi?id=98869 --- Comment #24 from Marek Olšák --- Ah thats what the patch does. I'll reply on mesa-dev. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/ttm: never add BO that failed to validate to the LRU list
Am 14.02.2017 um 10:18 schrieb Nicolai Hähnle:
From: Nicolai Hähnle
Fixes a potential race condition in amdgpu that looks as follows:
Task 1: attempt ttm_bo_init, but ttm_bo_validate fails
Task 1: add BO to global list anyway
Task 2: grabs hold of the BO, waits on its reservation lock
Task 1: releases its reference of the BO; never gives up the
reservation lock
The patch "drm/amdgpu: fix a potential deadlock in
amdgpu_bo_create_restricted()" attempts to fix that by releasing
the reservation lock in amdgpu code; unfortunately, it introduces
a use-after-free when this race _doesn't_ happen.
This patch should fix the race properly by never adding the BO
to the global list in the first place.
Cc: Samuel Pitoiset
Cc: zhoucm1
Signed-off-by: Nicolai Hähnle
NAK, that is actually not correct either.
The previous implementation doesn't have an use after free, but actually
a memory leak.
I completely agree that adding the BO to the LRU in case of an error is
incorrect, but ttm_bo_add_to_lru() will add some references to the BO.
So the following ttm_bo_unref() will just drop the initial reference.
Let's clean up this mess by moving the freeing of the BO in case of an
error to the caller.
Regards,
Christian.
---
drivers/gpu/drm/ttm/ttm_bo.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 239a957..76bee42 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1215,18 +1215,20 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
if (likely(!ret))
ret = ttm_bo_validate(bo, placement, interruptible, false);
- if (!resv) {
+ if (!resv)
ttm_bo_unreserve(bo);
- } else if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
+ if (unlikely(ret)) {
+ ttm_bo_unref(&bo);
+ return ret;
+ }
+
+ if (resv && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
spin_lock(&bo->glob->lru_lock);
ttm_bo_add_to_lru(bo);
spin_unlock(&bo->glob->lru_lock);
}
- if (unlikely(ret))
- ttm_bo_unref(&bo);
-
return ret;
}
EXPORT_SYMBOL(ttm_bo_init);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/ttm: never add BO that failed to validate to the LRU list
On 14.02.2017 11:38, Christian König wrote:
Am 14.02.2017 um 10:18 schrieb Nicolai Hähnle:
From: Nicolai Hähnle
Fixes a potential race condition in amdgpu that looks as follows:
Task 1: attempt ttm_bo_init, but ttm_bo_validate fails
Task 1: add BO to global list anyway
Task 2: grabs hold of the BO, waits on its reservation lock
Task 1: releases its reference of the BO; never gives up the
reservation lock
The patch "drm/amdgpu: fix a potential deadlock in
amdgpu_bo_create_restricted()" attempts to fix that by releasing
the reservation lock in amdgpu code; unfortunately, it introduces
a use-after-free when this race _doesn't_ happen.
This patch should fix the race properly by never adding the BO
to the global list in the first place.
Cc: Samuel Pitoiset
Cc: zhoucm1
Signed-off-by: Nicolai Hähnle
NAK, that is actually not correct either.
The previous implementation doesn't have an use after free, but actually
a memory leak.
I completely agree that adding the BO to the LRU in case of an error is
incorrect, but ttm_bo_add_to_lru() will add some references to the BO.
Yes, but those are bo->list_krefs, not bo->krefs. ttm_bo_unref will put
the bo->kref, which should lead to ttm_bo_release. This will then add
the buffer to the ddestroy list and eventually put all the list_krefs.
(Or possibly, if the ttm_bo_wait in ttm_bo_cleanup_refs_or_queue fails,
it will explicitly delete the buffer from the LRU, which should amount
to the same, I think).
Or perhaps I'm still missing something, I'm not too familiar with the
whole BO reference counting.
So the following ttm_bo_unref() will just drop the initial reference.
Let's clean up this mess by moving the freeing of the BO in case of an
error to the caller.
Yes, see my other patches.
Thanks,
Nicolai
Regards,
Christian.
---
drivers/gpu/drm/ttm/ttm_bo.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 239a957..76bee42 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1215,18 +1215,20 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
if (likely(!ret))
ret = ttm_bo_validate(bo, placement, interruptible, false);
-if (!resv) {
+if (!resv)
ttm_bo_unreserve(bo);
-} else if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
+if (unlikely(ret)) {
+ttm_bo_unref(&bo);
+return ret;
+}
+
+if (resv && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
spin_lock(&bo->glob->lru_lock);
ttm_bo_add_to_lru(bo);
spin_unlock(&bo->glob->lru_lock);
}
-if (unlikely(ret))
-ttm_bo_unref(&bo);
-
return ret;
}
EXPORT_SYMBOL(ttm_bo_init);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function
Am 14.02.2017 um 11:37 schrieb Nicolai Hähnle:
From: Nicolai Hähnle
Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.
Signed-off-by: Nicolai Hähnle
Please squash that into your other patch. It fixes another bug, but I
don't think fixing one bug just to run into another is really a good idea.
Additional to that one comment below.
---
drivers/gpu/drm/ttm/ttm_bo.c | 62 +---
include/drm/ttm/ttm_bo_api.h | 45
2 files changed, 86 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
}
EXPORT_SYMBOL(ttm_bo_validate);
-int ttm_bo_init(struct ttm_bo_device *bdev,
- struct ttm_buffer_object *bo,
- unsigned long size,
- enum ttm_bo_type type,
- struct ttm_placement *placement,
- uint32_t page_alignment,
- bool interruptible,
- struct file *persistent_swap_storage,
- size_t acc_size,
- struct sg_table *sg,
- struct reservation_object *resv,
- void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+ struct ttm_buffer_object *bo,
+ unsigned long size,
+ enum ttm_bo_type type,
+ uint32_t page_alignment,
+ struct file *persistent_swap_storage,
+ size_t acc_size,
+ struct sg_table *sg,
+ struct reservation_object *resv,
+ void (*destroy) (struct ttm_buffer_object *))
{
int ret = 0;
unsigned long num_pages;
struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
- bool locked;
ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
if (ret) {
pr_err("Out of kernel memory\n");
- if (destroy)
- (*destroy)(bo);
- else
- kfree(bo);
return -ENOMEM;
}
num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
if (num_pages == 0) {
pr_err("Illegal buffer object size\n");
- if (destroy)
- (*destroy)(bo);
- else
- kfree(bo);
ttm_mem_global_free(mem_glob, acc_size);
return -EINVAL;
}
I would move those checks after all the field initializations. This way
the structure has at least a valid content and we can safely use
ttm_bo_unref on it.
Regards,
Christian.
@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
ret = drm_vma_offset_add(&bdev->vma_manager, &bo->vma_node,
bo->mem.num_pages);
+ return ret;
+}
+EXPORT_SYMBOL(ttm_bo_init_top);
+
+int ttm_bo_init(struct ttm_bo_device *bdev,
+ struct ttm_buffer_object *bo,
+ unsigned long size,
+ enum ttm_bo_type type,
+ struct ttm_placement *placement,
+ uint32_t page_alignment,
+ bool interruptible,
+ struct file *persistent_swap_storage,
+ size_t acc_size,
+ struct sg_table *sg,
+ struct reservation_object *resv,
+ void (*destroy) (struct ttm_buffer_object *))
+{
+ bool locked;
+ int ret;
+
+ ret = ttm_bo_init_top(bdev, bo, size, type, page_alignment,
+ persistent_swap_storage, acc_size, sg, resv,
+ destroy);
+ if (ret) {
+ if (destroy)
+ (*destroy)(bo);
+ else
+ kfree(bo);
+ return ret;
+ }
+
/* passed reservation objects should already be locked,
* since otherwise lockdep will be angered in radeon.
*/
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index f195899..d44b8e4 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -453,6 +453,51 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev,
unsigned struct_size);
/**
+ * ttm_bo_init_top
+ *
+ * @bdev: Pointer to a ttm_bo_device struct.
+ * @bo: Pointer to a ttm_buffer_object to be initialized.
+ * @size: Requested size of buffer object.
+ * @type: Requested type of buffer object.
+ * @flags: Initial placement flags.
+ * @page_alignment: Data alignment in pages.
+ * @persistent_swap_storage: Usually th
Re: [drm-intel:drm-intel-next-queued 11/59] drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: unknown field 'mock' specified in initializer
On Tue, Feb 14, 2017 at 06:32:17PM +0800, kbuild test robot wrote: > tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued > head: d892e9398ecf6defc7972a62227b77dad6be20bd > commit: 953c7f82eb890085c60dbe22578e883d6837c674 [11/59] drm/i915: Provide a > hook for selftests > config: x86_64-randconfig-s2-02141638 (attached as .config) > compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7 > reproduce: > git checkout 953c7f82eb890085c60dbe22578e883d6837c674 > # save the attached .config to linux build tree > make ARCH=x86_64 > > All errors (new ones prefixed by >>): > >In file included from drivers/gpu/drm/i915/selftests/i915_selftest.c:68: > >> drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: unknown > >> field 'mock' specified in initializer >cc1: warnings being treated as errors > >> drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: missing > >> braces around initializer >drivers/gpu/drm/i915/selftests/i915_mock_selftests.h:11: error: (near > initialization for 'mock_selftests[0].') >In file included from drivers/gpu/drm/i915/selftests/i915_selftest.c:74: > >> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: unknown > >> field 'live' specified in initializer > >> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: missing > >> braces around initializer >drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: (near > initialization for 'live_selftests[0].') > >> drivers/gpu/drm/i915/selftests/i915_live_selftests.h:11: error: > >> initialization from incompatible pointer type > > vim +/mock +11 drivers/gpu/drm/i915/selftests/i915_mock_selftests.h > > 5 * > 6 * The function should be of type int function(void). It may be > conditionally > 7 * compiled using #if IS_ENABLED(DRM_I915_SELFTEST). > 8 * > 9 * Tests are executed in order by igt/drv_selftest > 10 */ > > 11selftest(sanitycheck, i915_mock_sanitycheck) /* keep first (igt > selfcheck) */ This is a named initializer for an anonymous union. It wasn't supported in gcc until 4.6 :| -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99484] Crusader Kings 2 - Loading bars, siege bars, morale bars, etc. do not render correctly
https://bugs.freedesktop.org/show_bug.cgi?id=99484 --- Comment #5 from Timothy Arceri --- Running Mesa master with llvm 3.8 (instead of 5.0 from git) resolves the problem. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460
https://bugs.freedesktop.org/show_bug.cgi?id=99275 --- Comment #28 from alvarex --- Created attachment 129582 --> https://bugs.freedesktop.org/attachment.cgi?id=129582&action=edit artifacts on radeon rx460 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460
https://bugs.freedesktop.org/show_bug.cgi?id=99275 --- Comment #29 from alvarex --- Hi I think I ve run into the same issue. I'm not quite sure. Firefox and other elements sometimes present artifacts, and as Reimar suggests with kernel 4.8 it doesn't happen. RX460 on Opensuse 42.2. It seams like some sort of memory corruption, my worst fear is that maybe this is a defective hardware. What brand is your rx460? I have a rx460 Gigabyte Windforce OC edition. I attach a screenshot. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460
https://bugs.freedesktop.org/show_bug.cgi?id=99275 --- Comment #30 from alvarex --- it happens really fast for a fraction of a second and then it draws correctly, I had to record the desktop and play the video in slow motion to take the screenshot. And it's random sometimes it won't happen for a long period and sometimes clicking every drop down menu will trigger it. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65968] Massive memory corruption in Planetary Annihilation Alpha
https://bugs.freedesktop.org/show_bug.cgi?id=65968 --- Comment #11 from Timothy Arceri --- The game runs (mostly fine on) i965, and a trace from i965 seem to run without issue on radeonsi. However running the radeonsi trace on the nvidia blob results in the same corruptions. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99484] Crusader Kings 2 - Loading bars, siege bars, morale bars, etc. do not render correctly
https://bugs.freedesktop.org/show_bug.cgi?id=99484 --- Comment #6 from Andreas Boll --- (In reply to Timothy Arceri from comment #5) > Running Mesa master with llvm 3.8 (instead of 5.0 from git) resolves the > problem. Be aware that llvm 3.8 degrades OpenGL support for radeonsi. Just check your glxinfo output. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/bridge/tfp410: Make symbol tfp410_platform_driver static
On 2/9/2017 8:55 PM, Wei Yongjun wrote:
From: Wei Yongjun
Fixes the following sparse warning:
drivers/gpu/drm/bridge/ti-tfp410.c:223:24: warning:
symbol 'tfp410_platform_driver' was not declared. Should it be static?
This was queued to drm-misc-next
Thanks,
Archit
Signed-off-by: Wei Yongjun
---
drivers/gpu/drm/bridge/ti-tfp410.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c
b/drivers/gpu/drm/bridge/ti-tfp410.c
index b054ea3..b379d04 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -220,7 +220,7 @@ static const struct of_device_id tfp410_match[] = {
};
MODULE_DEVICE_TABLE(of, tfp410_match);
-struct platform_driver tfp410_platform_driver = {
+static struct platform_driver tfp410_platform_driver = {
.probe = tfp410_probe,
.remove = tfp410_remove,
.driver = {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: make TTM_MAX_BO_PRIORITY unsigned
From: Nicolai Hähnle
Fix a warning about different types in min() macro in amdgpu:
In file included from ./include/linux/list.h:8:0,
from drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:32:
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function
‘amdgpu_bo_create_restricted’:
./include/linux/kernel.h:739:16: warning: comparison of distinct pointer types
lacks a cast
(void) (&min1 == &min2); \
^
./include/linux/kernel.h:742:2: note: in expansion of macro ‘__min’
__min(typeof(x), typeof(y), \
^
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:434:21: note: in expansion of macro
‘min’
bo->tbo.priority = min(bo->tbo.priority, TTM_MAX_BO_PRIORITY - 1);
^~~
Signed-off-by: Nicolai Hähnle
---
include/drm/ttm/ttm_bo_driver.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 4395db1..c3d74be 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -42,7 +42,7 @@
#include
#include
-#define TTM_MAX_BO_PRIORITY16
+#define TTM_MAX_BO_PRIORITY16U
struct ttm_backend_func {
/**
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm-intel:drm-intel-next-queued 23/59] drivers/gpu/drm/i915/selftests/i915_gem_request.c:280: error: 'request' may be used uninitialized in this function
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
head: d892e9398ecf6defc7972a62227b77dad6be20bd
commit: b348090d6758cc391dc91f8a8233b18f0acc8458 [23/59] drm/i915: Simple
selftest to exercise live requests
config: x86_64-randconfig-s2-02141638 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
git checkout b348090d6758cc391dc91f8a8233b18f0acc8458
# save the attached .config to linux build tree
make ARCH=x86_64
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All errors (new ones prefixed by >>):
cc1: warnings being treated as errors
In file included from drivers/gpu/drm/i915/i915_gem_request.c:1204:
drivers/gpu/drm/i915/selftests/i915_gem_request.c: In function
'live_nop_request':
>> drivers/gpu/drm/i915/selftests/i915_gem_request.c:280: error: 'request' may
>> be used uninitialized in this function
vim +/request +280 drivers/gpu/drm/i915/selftests/i915_gem_request.c
274 */
275
276 mutex_lock(&i915->drm.struct_mutex);
277
278 for_each_engine(engine, i915, id) {
279 IGT_TIMEOUT(end_time);
> 280 struct drm_i915_gem_request *request;
281 unsigned long n, prime;
282 ktime_t times[2] = {};
283
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dma-buf/fence: Avoid use of uninitialised timestamp
[ 236.821534] WARNING: kmemcheck: Caught 64-bit read from uninitialized memory
(8802538683d0)
[ 236.828642] 42001e7f0008
[ 236.839543] i i i i u u u u i i i i i i i i u u u u u u u u u u u u u u u u
[ 236.850420] ^
[ 236.854123] RIP: 0010:[] []
fence_signal+0x17/0xd0
[ 236.861313] RSP: 0018:88024acd7ba0 EFLAGS: 00010282
[ 236.865027] RAX: 812f6a90 RBX: 8802527ca800 RCX: 880252cb30e0
[ 236.868801] RDX: 88024ac5d918 RSI: 880252f780e0 RDI: 880253868380
[ 236.872579] RBP: 88024acd7bc0 R08: 88024acd7be0 R09:
[ 236.876407] R10: R11: R12: 880253868380
[ 236.880185] R13: 8802538684d0 R14: 880253868380 R15: 88024cd48e00
[ 236.883983] FS: 7f1646d1a740() GS:88025d00()
knlGS:
[ 236.890959] CS: 0010 DS: ES: CR0: 80050033
[ 236.894702] CR2: 880251360318 CR3: 00024ad21000 CR4: 001406f0
[ 236.898481] [] i915_gem_request_retire+0x1cd/0x230
[ 236.902439] [] i915_gem_request_alloc+0xa3/0x2f0
[ 236.906435] [] i915_gem_do_execbuffer.isra.41+0xb6d/0x18b0
[ 236.910434] [] i915_gem_execbuffer2+0x95/0x1e0
[ 236.914390] [] drm_ioctl+0x1e5/0x460
[ 236.918275] [] do_vfs_ioctl+0x8f/0x5c0
[ 236.922168] [] SyS_ioctl+0x3c/0x70
[ 236.926090] [] entry_SYSCALL_64_fastpath+0x17/0x93
[ 236.930045] [] 0x
We only set the timestamp before we mark the fence as signaled. It is
done before to avoid observers having a window in which they may see the
fence as complete but no timestamp. Having it does incur a potential for
the timestamp to be written twice, and even for it to be corrupted if
the u64 write is not atomic. Instead use a new bit to record the
presence of the timestamp, and teach the readers to wait until it is set
if the fence is complete. There still remains a race where the timestamp
for the signaled fence may be shown before the fence is reported as
signaled, but that's a pre-existing error.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Gustavo Padovan
Cc: Daniel Vetter
---
drivers/dma-buf/dma-fence.c | 17 ++---
drivers/dma-buf/sync_debug.c | 2 +-
drivers/dma-buf/sync_file.c | 8 +++-
include/linux/dma-fence.h| 2 ++
4 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index d1f1f456f5c4..dd2d7b6d2831 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -74,11 +74,6 @@ int dma_fence_signal_locked(struct dma_fence *fence)
if (WARN_ON(!fence))
return -EINVAL;
- if (!ktime_to_ns(fence->timestamp)) {
- fence->timestamp = ktime_get();
- smp_mb__before_atomic();
- }
-
if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
ret = -EINVAL;
@@ -86,8 +81,11 @@ int dma_fence_signal_locked(struct dma_fence *fence)
* we might have raced with the unlocked dma_fence_signal,
* still run through all callbacks
*/
- } else
+ } else {
+ fence->timestamp = ktime_get();
+ set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
trace_dma_fence_signaled(fence);
+ }
list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) {
list_del_init(&cur->node);
@@ -114,14 +112,11 @@ int dma_fence_signal(struct dma_fence *fence)
if (!fence)
return -EINVAL;
- if (!ktime_to_ns(fence->timestamp)) {
- fence->timestamp = ktime_get();
- smp_mb__before_atomic();
- }
-
if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return -EINVAL;
+ fence->timestamp = ktime_get();
+ set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
trace_dma_fence_signaled(fence);
if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)) {
diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
index c769dc653b34..bfead12390f2 100644
--- a/drivers/dma-buf/sync_debug.c
+++ b/drivers/dma-buf/sync_debug.c
@@ -84,7 +84,7 @@ static void sync_print_fence(struct seq_file *s,
show ? "_" : "",
sync_status_str(status));
- if (status) {
+ if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags)) {
struct timespec64 ts64 =
ktime_to_timespec64(fence->timestamp);
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 2321035f6204..95f259b719fc 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -375,7 +375,13 @@ static void sync_fill_fence_info(struct dma_fence *fence,
sizeof(info->driver_name));
info->status = dm
[PATCH RESEND] drm/dp/mst: fix kernel oops when turning off secondary monitor
From: Pierre-Louis Bossart
100% reproducible issue found on SKL SkullCanyon NUC with two external
DP daisy-chained monitors in DP/MST mode. When turning off or changing
the input of the second monitor the machine stops with a kernel
oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
This issue is traced to an inconsistent control flow in
drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the
same time as 'req_payload.num_slots' is set to zero, but the pointer is
dereferenced even when req_payload.num_slot is zero.
The problematic dereference was introduced in commit dfda0df34
("drm/mst: rework payload table allocation to conform better") and may
impact all versions since v3.18
The fix suggested by Chris Wilson removes the kernel oops and was found to
work well after 10mn of monkey-testing with the second monitor power and
input buttons
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
Fixes: dfda0df34264 ("drm/mst: rework payload table allocation to conform
better.")
Cc: Dave Airlie
Cc: Chris Wilson
Cc: Nathan D Ciobanu
Cc: Dhinakaran Pandiyan
Cc: Sean Paul
Cc: # v3.18+
Tested-by: Nathan D Ciobanu
Reviewed-by: Dhinakaran Pandiyan
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 122a1b04bebc..f2cc375907d0 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
mgr->payloads[i].vcpi = req_payload.vcpi;
} else if (mgr->payloads[i].num_slots) {
mgr->payloads[i].num_slots = 0;
- drm_dp_destroy_payload_step1(mgr, port,
port->vcpi.vcpi, &mgr->payloads[i]);
+ drm_dp_destroy_payload_step1(mgr, port,
mgr->payloads[i].vcpi, &mgr->payloads[i]);
req_payload.payload_state =
mgr->payloads[i].payload_state;
mgr->payloads[i].start_slot = 0;
}
--
2.1.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function
Am 14.02.2017 um 13:00 schrieb Nicolai Hähnle:
On 14.02.2017 11:49, Christian König wrote:
Am 14.02.2017 um 11:37 schrieb Nicolai Hähnle:
From: Nicolai Hähnle
Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.
Signed-off-by: Nicolai Hähnle
Please squash that into your other patch. It fixes another bug, but I
don't think fixing one bug just to run into another is really a good
idea.
I don't understand. I'm not aware that this patch fixes anything, it
just enables the subsequent fix in amdgpu in patch #2. I don't think
squashing those together is a good idea (one is in ttm, the other in
amdgpu).
Ok, forget it I've messed up the different reference count.
With at least initializing bo->kref and bo->destroy before returning the
first error the patch is Reviewed-by: Christian König
.
Regards,
Christian.
Additional to that one comment below.
---
drivers/gpu/drm/ttm/ttm_bo.c | 62
+---
include/drm/ttm/ttm_bo_api.h | 45
2 files changed, 86 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
b/drivers/gpu/drm/ttm/ttm_bo.c
index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object
*bo,
}
EXPORT_SYMBOL(ttm_bo_validate);
-int ttm_bo_init(struct ttm_bo_device *bdev,
-struct ttm_buffer_object *bo,
-unsigned long size,
-enum ttm_bo_type type,
-struct ttm_placement *placement,
-uint32_t page_alignment,
-bool interruptible,
-struct file *persistent_swap_storage,
-size_t acc_size,
-struct sg_table *sg,
-struct reservation_object *resv,
-void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+uint32_t page_alignment,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
{
int ret = 0;
unsigned long num_pages;
struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
-bool locked;
ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
if (ret) {
pr_err("Out of kernel memory\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
return -ENOMEM;
}
num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
if (num_pages == 0) {
pr_err("Illegal buffer object size\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
ttm_mem_global_free(mem_glob, acc_size);
return -EINVAL;
}
I would move those checks after all the field initializations. This way
the structure has at least a valid content and we can safely use
ttm_bo_unref on it.
That feels odd to me, since the return value indicates that the buffer
wasn't properly initialized, but I don't feel strongly about it.
Cheers,
Nicolai
Regards,
Christian.
@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
ret = drm_vma_offset_add(&bdev->vma_manager, &bo->vma_node,
bo->mem.num_pages);
+return ret;
+}
+EXPORT_SYMBOL(ttm_bo_init_top);
+
+int ttm_bo_init(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+struct ttm_placement *placement,
+uint32_t page_alignment,
+bool interruptible,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
+{
+bool locked;
+int ret;
+
+ret = ttm_bo_init_top(bdev, bo, size, type, page_alignment,
+ persistent_swap_storage, acc_size, sg, resv,
+ destroy);
+if (ret) {
+if (destroy)
+(*destroy)(bo);
+else
+kfree(bo);
+return ret;
+}
+
/* passed reservation objects should already be locked,
* since otherwise lockdep will be angered in radeon.
*/
diff --git a/include/drm/ttm/ttm_bo_api.h
b/include/drm/ttm/ttm_bo_api.h
index f195899..d44b8e4 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -453,6 +453,51 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device
*bdev,
unsigned struct_size);
/**
+ * ttm_bo_init_top
+ *
+ * @bdev: Pointer to a ttm_bo_device struct.
+ * @bo: Pointer to a ttm_buffe
Re: [PATCH 1/2] drm/ttm: never add BO that failed to validate to the LRU list
Am 14.02.2017 um 12:37 schrieb Nicolai Hähnle:
On 14.02.2017 11:38, Christian König wrote:
Am 14.02.2017 um 10:18 schrieb Nicolai Hähnle:
From: Nicolai Hähnle
Fixes a potential race condition in amdgpu that looks as follows:
Task 1: attempt ttm_bo_init, but ttm_bo_validate fails
Task 1: add BO to global list anyway
Task 2: grabs hold of the BO, waits on its reservation lock
Task 1: releases its reference of the BO; never gives up the
reservation lock
The patch "drm/amdgpu: fix a potential deadlock in
amdgpu_bo_create_restricted()" attempts to fix that by releasing
the reservation lock in amdgpu code; unfortunately, it introduces
a use-after-free when this race _doesn't_ happen.
This patch should fix the race properly by never adding the BO
to the global list in the first place.
Cc: Samuel Pitoiset
Cc: zhoucm1
Signed-off-by: Nicolai Hähnle
NAK, that is actually not correct either.
The previous implementation doesn't have an use after free, but actually
a memory leak.
I completely agree that adding the BO to the LRU in case of an error is
incorrect, but ttm_bo_add_to_lru() will add some references to the BO.
Yes, but those are bo->list_krefs, not bo->krefs. ttm_bo_unref will
put the bo->kref, which should lead to ttm_bo_release. This will then
add the buffer to the ddestroy list and eventually put all the
list_krefs. (Or possibly, if the ttm_bo_wait in
ttm_bo_cleanup_refs_or_queue fails, it will explicitly delete the
buffer from the LRU, which should amount to the same, I think).
Or perhaps I'm still missing something, I'm not too familiar with the
whole BO reference counting.
Ah, yes of course, never mind.
So the following ttm_bo_unref() will just drop the initial reference.
Let's clean up this mess by moving the freeing of the BO in case of an
error to the caller.
Yes, see my other patches.
In this case the set is Reviewed-by: Christian König
.
Regards,
Christian.
Thanks,
Nicolai
Regards,
Christian.
---
drivers/gpu/drm/ttm/ttm_bo.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
b/drivers/gpu/drm/ttm/ttm_bo.c
index 239a957..76bee42 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1215,18 +1215,20 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
if (likely(!ret))
ret = ttm_bo_validate(bo, placement, interruptible, false);
-if (!resv) {
+if (!resv)
ttm_bo_unreserve(bo);
-} else if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
+if (unlikely(ret)) {
+ttm_bo_unref(&bo);
+return ret;
+}
+
+if (resv && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
spin_lock(&bo->glob->lru_lock);
ttm_bo_add_to_lru(bo);
spin_unlock(&bo->glob->lru_lock);
}
-if (unlikely(ret))
-ttm_bo_unref(&bo);
-
return ret;
}
EXPORT_SYMBOL(ttm_bo_init);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf/fence: Avoid use of uninitialised timestamp
Hi Chris,
2017-02-14 Chris Wilson :
> [ 236.821534] WARNING: kmemcheck: Caught 64-bit read from uninitialized
> memory (8802538683d0)
> [ 236.828642]
> 42001e7f0008
> [ 236.839543] i i i i u u u u i i i i i i i i u u u u u u u u u u u u u u u
> u
> [ 236.850420] ^
> [ 236.854123] RIP: 0010:[] []
> fence_signal+0x17/0xd0
> [ 236.861313] RSP: 0018:88024acd7ba0 EFLAGS: 00010282
> [ 236.865027] RAX: 812f6a90 RBX: 8802527ca800 RCX:
> 880252cb30e0
> [ 236.868801] RDX: 88024ac5d918 RSI: 880252f780e0 RDI:
> 880253868380
> [ 236.872579] RBP: 88024acd7bc0 R08: 88024acd7be0 R09:
>
> [ 236.876407] R10: R11: R12:
> 880253868380
> [ 236.880185] R13: 8802538684d0 R14: 880253868380 R15:
> 88024cd48e00
> [ 236.883983] FS: 7f1646d1a740() GS:88025d00()
> knlGS:
> [ 236.890959] CS: 0010 DS: ES: CR0: 80050033
> [ 236.894702] CR2: 880251360318 CR3: 00024ad21000 CR4:
> 001406f0
> [ 236.898481] [] i915_gem_request_retire+0x1cd/0x230
> [ 236.902439] [] i915_gem_request_alloc+0xa3/0x2f0
> [ 236.906435] []
> i915_gem_do_execbuffer.isra.41+0xb6d/0x18b0
> [ 236.910434] [] i915_gem_execbuffer2+0x95/0x1e0
> [ 236.914390] [] drm_ioctl+0x1e5/0x460
> [ 236.918275] [] do_vfs_ioctl+0x8f/0x5c0
> [ 236.922168] [] SyS_ioctl+0x3c/0x70
> [ 236.926090] [] entry_SYSCALL_64_fastpath+0x17/0x93
> [ 236.930045] [] 0x
>
> We only set the timestamp before we mark the fence as signaled. It is
> done before to avoid observers having a window in which they may see the
> fence as complete but no timestamp. Having it does incur a potential for
> the timestamp to be written twice, and even for it to be corrupted if
> the u64 write is not atomic. Instead use a new bit to record the
> presence of the timestamp, and teach the readers to wait until it is set
> if the fence is complete. There still remains a race where the timestamp
> for the signaled fence may be shown before the fence is reported as
> signaled, but that's a pre-existing error.
>
> Signed-off-by: Chris Wilson
> Cc: Sumit Semwal
> Cc: Gustavo Padovan
> Cc: Daniel Vetter
> ---
> drivers/dma-buf/dma-fence.c | 17 ++---
> drivers/dma-buf/sync_debug.c | 2 +-
> drivers/dma-buf/sync_file.c | 8 +++-
> include/linux/dma-fence.h| 2 ++
> 4 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index d1f1f456f5c4..dd2d7b6d2831 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -74,11 +74,6 @@ int dma_fence_signal_locked(struct dma_fence *fence)
> if (WARN_ON(!fence))
> return -EINVAL;
>
> - if (!ktime_to_ns(fence->timestamp)) {
> - fence->timestamp = ktime_get();
> - smp_mb__before_atomic();
> - }
> -
> if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> ret = -EINVAL;
>
> @@ -86,8 +81,11 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>* we might have raced with the unlocked dma_fence_signal,
>* still run through all callbacks
>*/
> - } else
> + } else {
> + fence->timestamp = ktime_get();
> + set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
> trace_dma_fence_signaled(fence);
> + }
>
> list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) {
> list_del_init(&cur->node);
> @@ -114,14 +112,11 @@ int dma_fence_signal(struct dma_fence *fence)
> if (!fence)
> return -EINVAL;
>
> - if (!ktime_to_ns(fence->timestamp)) {
> - fence->timestamp = ktime_get();
> - smp_mb__before_atomic();
> - }
> -
> if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> return -EINVAL;
>
> + fence->timestamp = ktime_get();
> + set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
> trace_dma_fence_signaled(fence);
>
> if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)) {
> diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
> index c769dc653b34..bfead12390f2 100644
> --- a/drivers/dma-buf/sync_debug.c
> +++ b/drivers/dma-buf/sync_debug.c
> @@ -84,7 +84,7 @@ static void sync_print_fence(struct seq_file *s,
> show ? "_" : "",
> sync_status_str(status));
>
> - if (status) {
> + if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags)) {
> struct timespec64 ts64 =
> ktime_to_timespec64(fence->timestamp);
How about add this test_bit() to dma_fence_is_signaled_locked() so
we test both for DMA_FENCE_FLAG_SIGNALED_BIT and
DMA_FENCE_F
Re: [PATCH] dma-buf/fence: Avoid use of uninitialised timestamp
On Tue, Feb 14, 2017 at 11:40:38AM -0200, Gustavo Padovan wrote:
> Hi Chris,
>
> 2017-02-14 Chris Wilson :
> > diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
> > index c769dc653b34..bfead12390f2 100644
> > --- a/drivers/dma-buf/sync_debug.c
> > +++ b/drivers/dma-buf/sync_debug.c
> > @@ -84,7 +84,7 @@ static void sync_print_fence(struct seq_file *s,
> >show ? "_" : "",
> >sync_status_str(status));
> >
> > - if (status) {
> > + if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags)) {
> > struct timespec64 ts64 =
> > ktime_to_timespec64(fence->timestamp);
>
> How about add this test_bit() to dma_fence_is_signaled_locked() so
> we test both for DMA_FENCE_FLAG_SIGNALED_BIT and
> DMA_FENCE_FLAG_TIMESTAMP_BIT there at the same time?
I was thinking of only using it as communication with the timestamp
user. That avoids getting into the situation as to which bit truly means
is-signaled and we still only synchronize on SIGNALED_BIT.
It would be possible, but I don't think it makes anything simpler.
One thing that occurs to me is whether we should be setting the
timestamp when we set an error. The above (sync_debug though) implies
that it expects the error to have the timestamp. sync_fence_info could
go either way.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460
https://bugs.freedesktop.org/show_bug.cgi?id=99275 --- Comment #31 from alvarex --- I will try to git bisect but last time I tried bisecting the kernel I failed miserably and it failed compiling several times. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: make TTM_MAX_BO_PRIORITY unsigned
Am 14.02.2017 um 13:25 schrieb Nicolai Hähnle:
From: Nicolai Hähnle
Fix a warning about different types in min() macro in amdgpu:
In file included from ./include/linux/list.h:8:0,
from drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:32:
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function
‘amdgpu_bo_create_restricted’:
./include/linux/kernel.h:739:16: warning: comparison of distinct pointer types
lacks a cast
(void) (&min1 == &min2); \
^
./include/linux/kernel.h:742:2: note: in expansion of macro ‘__min’
__min(typeof(x), typeof(y), \
^
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:434:21: note: in expansion of macro
‘min’
bo->tbo.priority = min(bo->tbo.priority, TTM_MAX_BO_PRIORITY - 1);
^~~
Signed-off-by: Nicolai Hähnle
Reviewed-by: Christian König
---
include/drm/ttm/ttm_bo_driver.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 4395db1..c3d74be 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -42,7 +42,7 @@
#include
#include
-#define TTM_MAX_BO_PRIORITY 16
+#define TTM_MAX_BO_PRIORITY16U
struct ttm_backend_func {
/**
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm-intel:drm-intel-next-queued 31/59] drivers/gpu/drm/i915/selftests/i915_gem_coherency.c:274: error: 'err' may be used uninitialized in this function
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued head: d892e9398ecf6defc7972a62227b77dad6be20bd commit: 170594502cf591fd0789d7e5239937b1a87af4c6 [31/59] drm/i915: Test coherency of and barriers between cache domains config: x86_64-randconfig-s2-02141638 (attached as .config) compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7 reproduce: git checkout 170594502cf591fd0789d7e5239937b1a87af4c6 # save the attached .config to linux build tree make ARCH=x86_64 Note: it may well be a FALSE warning. FWIW you are at least aware of it now. http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings All errors (new ones prefixed by >>): cc1: warnings being treated as errors In file included from drivers/gpu/drm/i915/i915_gem.c:5029: drivers/gpu/drm/i915/selftests/i915_gem_coherency.c: In function 'igt_gem_coherency': >> drivers/gpu/drm/i915/selftests/i915_gem_coherency.c:274: error: 'err' may be >> used uninitialized in this function drivers/gpu/drm/i915/i915_gem.c: In function 'i915_gem_object_map': drivers/gpu/drm/i915/i915_gem.c:2467: error: 'pgprot.pgprot' may be used uninitialized in this function vim +/err +274 drivers/gpu/drm/i915/selftests/i915_gem_coherency.c 268 I915_RND_STATE(prng); 269 struct drm_i915_private *i915 = arg; 270 const struct igt_coherency_mode *read, *write, *over; 271 struct drm_i915_gem_object *obj; 272 unsigned long count, n; 273 u32 *offsets, *values; > 274 int err; 275 276 /* We repeatedly write, overwrite and read from a sequence of 277 * cachelines in order to try and detect incoherency (unflushed writes --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf/fence: Avoid use of uninitialised timestamp
2017-02-14 Chris Wilson :
> On Tue, Feb 14, 2017 at 11:40:38AM -0200, Gustavo Padovan wrote:
> > Hi Chris,
> >
> > 2017-02-14 Chris Wilson :
> > > diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c
> > > index c769dc653b34..bfead12390f2 100644
> > > --- a/drivers/dma-buf/sync_debug.c
> > > +++ b/drivers/dma-buf/sync_debug.c
> > > @@ -84,7 +84,7 @@ static void sync_print_fence(struct seq_file *s,
> > > show ? "_" : "",
> > > sync_status_str(status));
> > >
> > > - if (status) {
> > > + if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags)) {
> > > struct timespec64 ts64 =
> > > ktime_to_timespec64(fence->timestamp);
> >
> > How about add this test_bit() to dma_fence_is_signaled_locked() so
> > we test both for DMA_FENCE_FLAG_SIGNALED_BIT and
> > DMA_FENCE_FLAG_TIMESTAMP_BIT there at the same time?
>
> I was thinking of only using it as communication with the timestamp
> user. That avoids getting into the situation as to which bit truly means
> is-signaled and we still only synchronize on SIGNALED_BIT.
>
> It would be possible, but I don't think it makes anything simpler.
Yes, it doesn't make anything better. We should keep it that way for
users that doesn't need timestamp.
>
> One thing that occurs to me is whether we should be setting the
> timestamp when we set an error. The above (sync_debug though) implies
> that it expects the error to have the timestamp. sync_fence_info could
> go either way.
We could do it. I don't see any reason against it.
Gustavo
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf/fence: Avoid use of uninitialised timestamp
On Tue, Feb 14, 2017 at 12:22:02PM -0200, Gustavo Padovan wrote: > 2017-02-14 Chris Wilson : > > > On Tue, Feb 14, 2017 at 11:40:38AM -0200, Gustavo Padovan wrote: > > > Hi Chris, > > > > > > 2017-02-14 Chris Wilson : > > One thing that occurs to me is whether we should be setting the > > timestamp when we set an error. The above (sync_debug though) implies > > that it expects the error to have the timestamp. sync_fence_info could > > go either way. > > We could do it. I don't see any reason against it. It is just uncertain as to what timestamp means, and what the user wants. In i915 we may end up flagging an error on a fence long (many 10s) before the fence is signaled. It still feels more appropriate to set the timestamp on when the fence is complete - certainly for the case where we replay the request (and we flag fence->error that we did so as there may have been side-effects that we should inform the user about). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 194559] amdgpu problems loading 2 firmwares on multi-smp system
https://bugzilla.kernel.org/show_bug.cgi?id=194559 --- Comment #7 from fin4...@hotmail.com --- (In reply to Michel Dänzer from comment #6) > (In reply to fin4478 from comment #5) > > This and many other amdgpu bug reports prove my point. > > Your bug report comments like this one rather indicate that you don't > understand how the kernel development process works. You do not see how agd5f wip kernel solved this and many other problems. Amd should warn not use stock kernels and tell how to use use ~agd5f wip kernel and latest mesa git. Here is the page for you, dear Amd: http://support.amd.com/en-us/download/linux You clearly want bad reputation for Amd gpus so I stop giving this info. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 194579] AMDGPU: Possible size overflow detected by PaX in ttm_bo_handle_move_mem (drivers/gpu/drm/ttm/ttm_bo.c:388)
https://bugzilla.kernel.org/show_bug.cgi?id=194579 --- Comment #5 from fin4...@hotmail.com --- (In reply to Christian König from comment #3) > Please ignore the incorrect comment by fin4...@hotmail.com. > > The upstream kernel the official base amdgpu driver code. Only a few not > parts which are not upstream yet are in private repositories waiting for > cleanup. > > Regards, > Christian (co-maintainer of the amdgpu module). Amd developers want bad bad reputation for amd gpus, so I stop giving info. Here is an example how the wip kernel solved the problem: https://bugzilla.kernel.org/show_bug.cgi?id=194559 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 194559] amdgpu problems loading 2 firmwares on multi-smp system
https://bugzilla.kernel.org/show_bug.cgi?id=194559 Christian König (deathsim...@vodafone.de) changed: What|Removed |Added CC||deathsim...@vodafone.de --- Comment #8 from Christian König (deathsim...@vodafone.de) --- (In reply to fin4478 from comment #7) > You clearly want bad reputation for Amd gpus so I stop giving this info. Well as an AMD employee I can only advise you to stop giving incorrect informations. Alex branches only contain additional features not upstream yet, so they are way more unstable than the upstream kernel driver. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460
https://bugs.freedesktop.org/show_bug.cgi?id=99275 --- Comment #32 from alvarex --- I'm sorry but I can't find a consistent way of reproducing the bug. I presumed that with 4.9 the bug will still be there but right now, no matter how hard I try I cannot reproduce with 4.9.4, 4.9.9 and 4.9rc1, I think that in my case is a hardware problem and possibly unrelated to Reimar bug. It seems to me that the artifacting varies under load, or under temperature, just that maybe some kernel versions mitigate the problem and it goes unnoticed and some a aggravate the problem (IMHO). I really don't know what the cause is in my case. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/7] dt-bindings: display/panel: Add common rotation property
On Sat, Feb 11, 2017 at 12:48 PM, Noralf Trønnes wrote: > Display panels can be oriented many ways, especially in the embedded > world. The rotation property is a way to describe this orientation. > The counter clockwise direction is chosen because that's what fbdev > and drm use. > > Signed-off-by: Noralf Trønnes > Acked-by: Thierry Reding > --- > > Changes since version 3: > - Changed display/display.txt -> display/panel/panel.txt > > Documentation/devicetree/bindings/display/panel/panel.txt | 4 > 1 file changed, 4 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/panel/panel.txt Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC simple allocator v2 0/2] Simple allocator
On Mon, Feb 13, 2017 at 11:01:14AM -0800, Laura Abbott wrote: > On 02/13/2017 10:18 AM, Mark Brown wrote: > > The software defined networking people seemed to think they had a use > > case for this as well. They're not entirely upstream of course but > > still... > This is the first I've heard of anything like this. Do you have any more > details/reading? No, unfortunately it was in a meeting and I was asking for more details on what specifically the hardware was doing myself. My understanding is that it's very similar to the GPU/video needs. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99815] Power management problems & kernel hangs with Cap Verde
https://bugs.freedesktop.org/show_bug.cgi?id=99815 Bug ID: 99815 Summary: Power management problems & kernel hangs with Cap Verde Product: DRI Version: unspecified Hardware: Other OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: klaus.kus...@computerix.info I recently switched from the radeon kernel driver to the amdgpu kernel driver. As a consequence, battery lifetime of my laptop was more than halved (before I could give *two* 90 minutes lectures in a row running on battery, now the battery is empty after around 70 minutes), i.e. with amdgpu, the graphic card consumes a multiple of the power it needed with radeon. GPU is AMDGPU(0): Chipset: "VERDE" (ChipID = 0x6825) Laptop is Dell Precision M6700 OS is linux 4.9, userland is running latest official releases (Gentoo) Typical load is two screens (internal + VGA-Beamer, both 1024x768 mirrored) with very light load (no 3D & no video at all, just static pdf slides). Power settings in battery mode with the old radeon driver were * no dpm (always was very unstable and consumed by far more power than "classic" pm) * "classic" pm with power_method = profile and power_profile = low Analysis of the problems with amdgpu showed: 1.) There is no "classic" power management: If dpm is turned off with amdgpu, there is no power management at all, and the card permanently runs with high power. Why was "classic" power management dropped completely? Basically, I neither want nor need any dynamic adjustment: When on battery, the gpu should permanently run on minimal power, independent of its load. 2.) The default dpm mode is "balanced" and "auto". "balanced" does not change to "battery" automatically when AC is removed, and it is impossible to set the mode to "battery" manually: Any attempt to set it via sysfs either kills the process doing so or hangs it in a kill-9-immune state (which prevents system shutdown). I then code-changed the default initialization in si_dpm.c from "balanced" to "battery". This hangs the whole kernel hard during early boot: No display, no reaction to Alt-Sysrq, ... only hard power off helps. 3.) It is possible to set the dpm level from "auto" to "low", but that does not seem to result in any measurable effect or power savings? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99801] Rx480 doesn't output properly onto z27q at 5120x2880
https://bugs.freedesktop.org/show_bug.cgi?id=99801 --- Comment #12 from Harry Wentland --- We're currently debugging an issue that looks very similar, if not the same. It's also with a 5k display with two DP input but on a different platform. The display pipe driving one half of the display is underflowing. Hope we'll find a fix soon. Not sure about the issue in 4k mode but there's a good chance the display pipe is underflowing for the same reason as the problem in 5k mode. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/amd/dc: resource: fix semicolon.cocci warnings (fwd)
Thanks for these fixes. I'll merge them.
Reviewed-by: Harry Wentland
Harry
On 2017-02-14 04:47 AM, Christian König wrote:
Am 14.02.2017 um 07:21 schrieb Julia Lawall:
Remove unneeded semicolon.
Generated by: scripts/coccinelle/misc/semicolon.cocci
CC: Harry Wentland
Signed-off-by: Julia Lawall
Signed-off-by: Fengguang Wu
Acked-by: Christian König for this one as
well as the other similar patches.
Thanks for letting us know about those typos.
Regards,
Christian.
---
v2: make subject line unique
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.9
head: 79d2de1bcb650296adff1cb08bfbf1501a6e6e14
commit: bad4c165a6986a131cdd1455507ba3857baaa561 [201/657]
drm/amd/dc: Add
dc display driver
dc_resource.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c
@@ -135,7 +135,7 @@ static void update_num_audio(
break;
default:
DC_ERR("DC: unexpected audio fuse!\n");
-};
+}
}
bool resource_construct(
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/dc: fix semicolon.cocci warnings
Reviewed-by: Harry Wentland
Harry
On 2017-02-14 01:14 AM, Julia Lawall wrote:
Remove unneeded semicolon.
Generated by: scripts/coccinelle/misc/semicolon.cocci
CC: Harry Wentland
Signed-off-by: Julia Lawall
Signed-off-by: Fengguang Wu
---
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.9
head: 79d2de1bcb650296adff1cb08bfbf1501a6e6e14
commit: bad4c165a6986a131cdd1455507ba3857baaa561 [201/657] drm/amd/dc: Add
dc display driver
dc_resource.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c
@@ -135,7 +135,7 @@ static void update_num_audio(
break;
default:
DC_ERR("DC: unexpected audio fuse!\n");
- };
+ }
}
bool resource_construct(
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/dc: fix semicolon.cocci warnings
Reviewed-by: Harry Wentland Harry On 2017-02-14 01:13 AM, Julia Lawall wrote: Remove unneeded semicolons. Generated by: scripts/coccinelle/misc/semicolon.cocci CC: Harry Wentland Signed-off-by: Julia Lawall Signed-off-by: Fengguang Wu --- tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.9 head: 79d2de1bcb650296adff1cb08bfbf1501a6e6e14 commit: bad4c165a6986a131cdd1455507ba3857baaa561 [201/657] drm/amd/dc: Add dc display driver amdgpu_dm_types.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) --- a/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_types.c +++ b/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_types.c @@ -890,11 +890,11 @@ static void copy_crtc_timing_for_drm_dis dst_mode->crtc_hsync_end = src_mode->crtc_hsync_end; dst_mode->crtc_htotal = src_mode->crtc_htotal; dst_mode->crtc_hskew = src_mode->crtc_hskew; - dst_mode->crtc_vblank_start = src_mode->crtc_vblank_start;; - dst_mode->crtc_vblank_end = src_mode->crtc_vblank_end;; - dst_mode->crtc_vsync_start = src_mode->crtc_vsync_start;; - dst_mode->crtc_vsync_end = src_mode->crtc_vsync_end;; - dst_mode->crtc_vtotal = src_mode->crtc_vtotal;; + dst_mode->crtc_vblank_start = src_mode->crtc_vblank_start; + dst_mode->crtc_vblank_end = src_mode->crtc_vblank_end; + dst_mode->crtc_vsync_start = src_mode->crtc_vsync_start; + dst_mode->crtc_vsync_end = src_mode->crtc_vsync_end; + dst_mode->crtc_vtotal = src_mode->crtc_vtotal; } static void decide_crtc_timing_for_drm_display_mode( ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98869] Electronic Super Joy graphic artefacts (regression,bisected)
https://bugs.freedesktop.org/show_bug.cgi?id=98869 --- Comment #25 from cosiek...@o2.pl --- I can do more testing if needed. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/dc: hw_sequencer: fix semicolon.cocci warnings
Reviewed-by: Harry Wentland
Harry
On 2017-02-14 01:19 AM, Julia Lawall wrote:
Remove unneeded semicolon.
Generated by: scripts/coccinelle/misc/semicolon.cocci
CC: Harry Wentland
Signed-off-by: Julia Lawall
Signed-off-by: Fengguang Wu
---
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-4.9
head: 79d2de1bcb650296adff1cb08bfbf1501a6e6e14
commit: bad4c165a6986a131cdd1455507ba3857baaa561 [201/657] drm/amd/dc: Add
dc display driver
dce110_hw_sequencer.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_hw_sequencer.c
@@ -1643,7 +1643,7 @@ static void init_hw(struct core_dc *dc)
true);
}
- dce_clock_gating_power_up(dc->hwseq, false);;
+ dce_clock_gating_power_up(dc->hwseq, false);
/***/
for (i = 0; i < dc->link_count; i++) {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65968] Massive memory corruption in Planetary Annihilation Alpha
https://bugs.freedesktop.org/show_bug.cgi?id=65968 Andreas Ringlstetter changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |INVALID --- Comment #12 from Andreas Ringlstetter --- It's a bug in PA itself, not in Mesa. The root cause is a race condition on the shared buffer which is used to transfer the rendered HTML UI from the Coherent host process back to PA. There is a missing mutex inside PA when the buffer gets reallocated as a result of a window resize event. Effectively, this results in a use-after-free by the render thread of the PA process. The faster the realloc, the lower the chance of this bug occurring. It's also subject to possibly missing protections against use after free conditions on previously shared buffers. And also to the memory allocation strategy, as a reuse of the same memory region without a clear leads to the most visible effect. Unfortunately, various Mesa drivers so not wipe the video memory after a buffer was returned to the global pool! -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98784] Talos Principle rendering flickering garbage on start instead of its logo and loading squares
https://bugs.freedesktop.org/show_bug.cgi?id=98784 --- Comment #16 from Torbjörn Andersson --- I can't reproduce the glitch any more. Perhaps today's update of the game fixed it? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC][PATCH 2/2] drm: kirin: Restrict modes to known good mode clocks
Currently the hikey dsi logic cannot generate accurate byte
clocks values for all pixel clock values. Thus if a mode clock
is selected that cannot match the calculated byte clock, the
device will boot with a blank screen.
This patch uses the new mode_valid callback to enforces known
good mode clocks for well known resolutions, which should allow
the display to work from given EDID options, and ensures for a
given resolution & refresh, the right mode clock is selected.
Cc: Daniel Vetter
Cc: Jani Nikula
Cc: Sean Paul
Cc: David Airlie
Cc: Rob Clark
Cc: Xinliang Liu
Cc: Xinliang Liu
Cc: Rongrong Zou
Cc: Xinwei Kong
Cc: Chen Feng
Cc: Archit Taneja
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz
---
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 38 +
1 file changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
index afc2b5d..9161633 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
@@ -504,6 +504,43 @@ static void ade_crtc_disable(struct drm_crtc *crtc)
acrtc->enable = false;
}
+static enum drm_mode_status ade_crtc_mode_valid(struct drm_crtc *crtc,
+ struct drm_display_mode *mode)
+{
+ /*
+* kirin_ade cannot generate all modes, so use the whitelist below
+*/
+ DRM_DEBUG("Checking mode %ix%i@%i clock: %i...",
+ mode->hdisplay, mode->vdisplay, drm_mode_vrefresh(mode),
mode->clock);
+ if ((mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock ==
148500) ||
+ (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock ==
80192) ||
+ (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock ==
74250) ||
+ (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock ==
61855) ||
+ (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock ==
147116) ||
+ (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock ==
146250) ||
+ (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock ==
144589) ||
+ (mode->hdisplay == 1600 && mode->vdisplay == 1200 && mode->clock ==
160961) ||
+ (mode->hdisplay == 1600 && mode->vdisplay == 900 && mode->clock ==
118963) ||
+ (mode->hdisplay == 1440 && mode->vdisplay == 900 && mode->clock ==
126991) ||
+ (mode->hdisplay == 1280 && mode->vdisplay == 1024 && mode->clock ==
128946) ||
+ (mode->hdisplay == 1280 && mode->vdisplay == 1024 && mode->clock ==
98619) ||
+ (mode->hdisplay == 1280 && mode->vdisplay == 960 && mode->clock ==
102081) ||
+ (mode->hdisplay == 1280 && mode->vdisplay == 800 && mode->clock ==
83496) ||
+ (mode->hdisplay == 1280 && mode->vdisplay == 720 && mode->clock ==
74440) ||
+ (mode->hdisplay == 1280 && mode->vdisplay == 720 && mode->clock ==
74250) ||
+ (mode->hdisplay == 1024 && mode->vdisplay == 768 && mode->clock ==
78800) ||
+ (mode->hdisplay == 1024 && mode->vdisplay == 768 && mode->clock ==
75000) ||
+ (mode->hdisplay == 1024 && mode->vdisplay == 768 && mode->clock ==
81833) ||
+ (mode->hdisplay == 800 && mode->vdisplay == 600 && mode->clock ==
48907) ||
+ (mode->hdisplay == 800 && mode->vdisplay == 600 && mode->clock ==
4)) {
+ mode->type |= DRM_MODE_TYPE_PREFERRED;
+ DRM_DEBUG("OK\n");
+ return MODE_OK;
+ }
+ DRM_DEBUG("BAD\n");
+ return MODE_BAD;
+}
+
static void ade_crtc_mode_set_nofb(struct drm_crtc *crtc)
{
struct ade_crtc *acrtc = to_ade_crtc(crtc);
@@ -557,6 +594,7 @@ static void ade_crtc_atomic_flush(struct drm_crtc *crtc,
static const struct drm_crtc_helper_funcs ade_crtc_helper_funcs = {
.enable = ade_crtc_enable,
.disable= ade_crtc_disable,
+ .mode_valid = ade_crtc_mode_valid,
.mode_set_nofb = ade_crtc_mode_set_nofb,
.atomic_begin = ade_crtc_atomic_begin,
.atomic_flush = ade_crtc_atomic_flush,
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
Currently, on the hikey board, we have the adv7511 bridge wired
up to the kirin ade drm driver. Unfortunately, the kirin ade
core cannot generate accurate byteclocks for all pixel clock
values.
Thus if a mode clock is selected that we cannot calculate a
matching byteclock, the device will boot with a blank screen.
Unfortunately, currently the only place we can properly check
potential modes for this issue in the connector mode_valid
helper. Again, hikey uses the adv7511 bridge, which is shared
between a number of different devices, so its improper to put
restrictions caused by the kirin drm driver in the adv7511
logic.
So this patch tries to correct for that, by adding some
infrastructure so that the drm_crtc_helper_funcs can optionally
implement a mode_valid check, so that the probe helpers can
check to make sure there are not any restrictions at the crtc
level as well.
Cc: Daniel Vetter
Cc: Jani Nikula
Cc: Sean Paul
Cc: David Airlie
Cc: Rob Clark
Cc: Xinliang Liu
Cc: Xinliang Liu
Cc: Rongrong Zou
Cc: Xinwei Kong
Cc: Chen Feng
Cc: Archit Taneja
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz
---
drivers/gpu/drm/drm_probe_helper.c | 24
include/drm/drm_modeset_helper_vtables.h | 26 ++
2 files changed, 50 insertions(+)
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index cf8f012..a808348 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -170,6 +170,28 @@ drm_connector_detect(struct drm_connector *connector, bool
force)
connector_status_connected;
}
+static enum drm_mode_status
+drm_connector_check_crtc_modes(struct drm_connector *connector,
+ struct drm_display_mode *mode)
+{
+ struct drm_device *dev = connector->dev;
+ const struct drm_crtc_helper_funcs *crtc_funcs;
+ struct drm_crtc *c;
+
+ if (mode->status != MODE_OK)
+ return mode->status;
+
+ /* Check all the crtcs on a connector to make sure the mode is valid */
+ drm_for_each_crtc(c, dev) {
+ crtc_funcs = c->helper_private;
+ if (crtc_funcs && crtc_funcs->mode_valid)
+ mode->status = crtc_funcs->mode_valid(c, mode);
+ if (mode->status != MODE_OK)
+ break;
+ }
+ return mode->status;
+}
+
/**
* drm_helper_probe_single_connector_modes - get complete set of display modes
* @connector: connector to probe
@@ -338,6 +360,8 @@ int drm_helper_probe_single_connector_modes(struct
drm_connector *connector,
if (mode->status == MODE_OK && connector_funcs->mode_valid)
mode->status = connector_funcs->mode_valid(connector,
mode);
+
+ mode->status = drm_connector_check_crtc_modes(connector, mode);
}
prune:
diff --git a/include/drm/drm_modeset_helper_vtables.h
b/include/drm/drm_modeset_helper_vtables.h
index 69c3974..53ca0e4 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -105,6 +105,32 @@ struct drm_crtc_helper_funcs {
void (*commit)(struct drm_crtc *crtc);
/**
+* @mode_valid:
+*
+* Callback to validate a mode for a crtc, irrespective of the
+* specific display configuration.
+*
+* This callback is used by the probe helpers to filter the mode list
+* (which is usually derived from the EDID data block from the sink).
+* See e.g. drm_helper_probe_single_connector_modes().
+*
+* NOTE:
+*
+* This only filters the mode list supplied to userspace in the
+* GETCONNECOTR IOCTL. Userspace is free to create modes of its own and
+* ask the kernel to use them. It this case the atomic helpers or legacy
+* CRTC helpers will not call this function. Drivers therefore must
+* still fully validate any mode passed in in a modeset request.
+*
+* RETURNS:
+*
+* Either MODE_OK or one of the failure reasons in enum
+* &drm_mode_status.
+*/
+ enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
+ struct drm_display_mode *mode);
+
+ /**
* @mode_fixup:
*
* This callback is used to validate a mode. The parameter mode is the
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC][PATCH 0/2] Add mode_valid drm_crtc_helper_funcs for HiKey
Currently, on the hikey board, we have the adv7511 bridge wired up to the kirin ade drm driver. Unfortunately, the kirin ade core cannot generate accurate byteclocks for all pixel clock values. Thus if a mode clock is selected that we cannot calculate a matching byteclock, the device will boot with a blank screen. Unfortunately, currently the only place we can properly check potential modes for this issue in the connector mode_valid helper. Again, hikey uses the adv7511 bridge, which is shared between a number of different devices, so its improper to put restrictions caused by the kirin drm driver in the adv7511 logic. So this patch set tries to fix this by adding some infrastructure and logic to the probe helpers so that drm_crtcs can filter the probed modes. And then adds a whitelist of valid modes for the kirin drm driver used on HiKey. This is a first pass attempt here, implementing a suggestion from Rob Clark on irc, so I'd really welcome any feedback or ideas for how to best do this. Thanks so much! -john Cc: Daniel Vetter Cc: Jani Nikula Cc: Sean Paul Cc: David Airlie Cc: Rob Clark Cc: Xinliang Liu Cc: Xinliang Liu Cc: Rongrong Zou Cc: Xinwei Kong Cc: Chen Feng Cc: Archit Taneja Cc: dri-devel@lists.freedesktop.org John Stultz (2): drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs drm: kirin: Restrict modes to known good mode clocks drivers/gpu/drm/drm_probe_helper.c | 24 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 38 + include/drm/drm_modeset_helper_vtables.h| 26 + 3 files changed, 88 insertions(+) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC simple allocator v2 1/2] Create Simple Allocator module
Hi Benjamin,
Thank you for the patch. I've CC'ed the linux-api mailing list.
On Monday 13 Feb 2017 15:45:05 Benjamin Gaignard wrote:
> This is the core of simple allocator module.
> It aim to offert one common ioctl to allocate specific memory.
>
> version 2:
> - rebased on 4.10-rc7
>
> Signed-off-by: Benjamin Gaignard
> ---
> Documentation/simple-allocator.txt | 81 +++
> drivers/Kconfig | 2 +
> drivers/Makefile| 1 +
> drivers/simpleallocator/Kconfig | 10 ++
> drivers/simpleallocator/Makefile| 1 +
> drivers/simpleallocator/simple-allocator-priv.h | 33 +
> drivers/simpleallocator/simple-allocator.c | 180 +
> include/uapi/linux/simple-allocator.h | 35 +
> 8 files changed, 343 insertions(+)
> create mode 100644 Documentation/simple-allocator.txt
> create mode 100644 drivers/simpleallocator/Kconfig
> create mode 100644 drivers/simpleallocator/Makefile
> create mode 100644 drivers/simpleallocator/simple-allocator-priv.h
> create mode 100644 drivers/simpleallocator/simple-allocator.c
> create mode 100644 include/uapi/linux/simple-allocator.h
>
> diff --git a/Documentation/simple-allocator.txt
> b/Documentation/simple-allocator.txt new file mode 100644
> index 000..89ba883
> --- /dev/null
> +++ b/Documentation/simple-allocator.txt
> @@ -0,0 +1,81 @@
> +Simple Allocator Framework
There's nothing simple in buffer allocation, otherwise we would have solved
the problem a long time ago. Let's not use a misleading name.
> +Simple Allocator offer a single ioctl SA_IOC_ALLOC to allocate buffers
> +on dedicated memory regions and export them as a dmabuf file descriptor.
> +Using dmabuf file descriptor allow to share this memory between processes
> +and/or import it into other frameworks like v4l2 or drm/kms (prime).
> +When userland wants to free the memory only a call to close() in needed
> +so it could done even without knowing that buffer has been allocated by
> +simple allocator ioctl.
> +
> +Each memory regions will be seen as a filein /dev/.
> +For example CMA regions will exposed has /dev/cmaX.
Why do you need multiple devices ? Furthermore, given how central this API
will become, I believe it needs very strict review, and would be a candidate
for a syscall.
Without diving into details yet, there are a few particular points I'm
concerned about.
- What is the scope of this API ? What problems do you want to solve, plan to
solve in the future, and consider as out of scope ?
- How should we handle permissions and resource limits ? Is file-based
permission on a device node a good model ? How do we prevent or at least
mitigate memory-related DoS ?
- How should such a central allocator API interact with containers and
virtualization in general ?
- What model do we want to expose to applications to select a memory type ?
You still haven't convinced me that we should expose memory pools explicitly
to application (à la ION), and I believe a usage/constraint model would be
better.
> +Implementing a simple allocator
> +---
> +
> +Simple Allocator provide helpers functions to register/unregister an
> +allocator:
> +- simple_allocator_register(struct sa_device *sadev)
> + Register simple_allocator_device using sa_device structure where name,
> + owner and allocate fields must be set.
> +
> +- simple_allocator_unregister(struct sa_device *sadev)
> + Unregister a simple allocator device.
> +
> +Using Simple Allocator /dev interface example
> +-
> +
> +This example of code allocate a buffer on the first CMA region (/dev/cma0)
> +before mmap and close it.
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "simple-allocator.h"
> +
> +#define LENGTH 1024*16
> +
> +void main (void)
> +{
> + struct simple_allocate_data data;
> + int fd = open("/dev/cma0", O_RDWR, 0);
> + int ret;
> + void *mem;
> +
> + if (fd < 0) {
> + printf("Can't open /dev/cma0\n");
> + return;
> + }
> +
> + memset(&data, 0, sizeof(data));
> +
> + data.length = LENGTH;
> + data.flags = O_RDWR | O_CLOEXEC;
> +
> + ret = ioctl(fd, SA_IOC_ALLOC, &data);
> + if (ret) {
> + printf("Buffer allocation failed\n");
> + goto end;
> + }
> +
> + mem = mmap(0, LENGTH, PROT_READ | PROT_WRITE, MAP_SHARED, data.fd, 0);
> + if (mem == MAP_FAILED) {
> + printf("mmap failed\n");
> + }
> +
> + memset(mem, 0xFF, LENGTH);
> + munmap(mem, LENGTH);
> +
> + printf("test simple allocator CMA OK\n");
> +end:
> + close(fd);
> +}
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index e1e2066..a6d8828 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -202,4 +202,6 @@
[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460
https://bugs.freedesktop.org/show_bug.cgi?id=99275 --- Comment #33 from Reimar Imhof --- (In reply to Michel Dänzer from comment #27) > (In reply to Reimar Imhof from comment #26) > > Together with comment #24 there is a render bug in kernel 4.8 that shows up > > at 100% cpu load. > > With kernel 4.9 this same bug shows up at 0% / idle cpu load. > > > > With > > af79ad2b1f337a00aa150b993635b10bc68dc842 > > Merge branch 'sched-core-for-linus' of > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip > > it changed from "bug shows at 100% load" to "bug shows at 0% load". And the > > change is something about the scheduler. > > To me this seems likely. > > Not really. That commit is a pure merge commit, which makes it unlikely that > it behaves any differently from either of its parent commits. So git bisect > should have identified one of its ancestor commits instead. The fact that it > identified a pure merge commit indicates that the result is incorrect, most > likely because at least one commit along the way was incorrectly classified > as good (or bad). I forgot to mention: I had a look at the first merge commits. I did _not_ do a "git bisect" but for example a "git reset --hard 7af8a0f8088831428051976cb06cc1e450f8bab5" followed by a "make rpm" to compile "Merge tag 'arm64-upstream' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux". "e606d81d2d9596ab2b4fd0dc052eea0485b7e8c2 Merge branch 'ras-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip" was a good commit - no problems at idle cpu as described in Comment #23. That one was followed by "af79ad2b1f337a00aa150b993635b10bc68dc842 Merge branch 'sched-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip" which turned out to be the first bad commit (glitches at 0 cpu load). So all tested commits were pure merge commits. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC simple allocator v2 1/2] Create Simple Allocator module
On Mon, Feb 13, 2017 at 3:45 PM, Benjamin Gaignard
wrote:
> This is the core of simple allocator module.
> It aim to offert one common ioctl to allocate specific memory.
>
> version 2:
> - rebased on 4.10-rc7
>
> Signed-off-by: Benjamin Gaignard
Why not ION? It's a bit a broken record question, but if there is a
clear answer it should be in the patch&docs ...
-Daniel
> ---
> Documentation/simple-allocator.txt | 81 +++
> drivers/Kconfig | 2 +
> drivers/Makefile| 1 +
> drivers/simpleallocator/Kconfig | 10 ++
> drivers/simpleallocator/Makefile| 1 +
> drivers/simpleallocator/simple-allocator-priv.h | 33 +
> drivers/simpleallocator/simple-allocator.c | 180
>
> include/uapi/linux/simple-allocator.h | 35 +
> 8 files changed, 343 insertions(+)
> create mode 100644 Documentation/simple-allocator.txt
> create mode 100644 drivers/simpleallocator/Kconfig
> create mode 100644 drivers/simpleallocator/Makefile
> create mode 100644 drivers/simpleallocator/simple-allocator-priv.h
> create mode 100644 drivers/simpleallocator/simple-allocator.c
> create mode 100644 include/uapi/linux/simple-allocator.h
>
> diff --git a/Documentation/simple-allocator.txt
> b/Documentation/simple-allocator.txt
> new file mode 100644
> index 000..89ba883
> --- /dev/null
> +++ b/Documentation/simple-allocator.txt
> @@ -0,0 +1,81 @@
> +Simple Allocator Framework
> +
> +Simple Allocator offer a single ioctl SA_IOC_ALLOC to allocate buffers
> +on dedicated memory regions and export them as a dmabuf file descriptor.
> +Using dmabuf file descriptor allow to share this memory between processes
> +and/or import it into other frameworks like v4l2 or drm/kms (prime).
> +When userland wants to free the memory only a call to close() in needed
> +so it could done even without knowing that buffer has been allocated by
> +simple allocator ioctl.
> +
> +Each memory regions will be seen as a filein /dev/.
> +For example CMA regions will exposed has /dev/cmaX.
> +
> +Implementing a simple allocator
> +---
> +
> +Simple Allocator provide helpers functions to register/unregister an
> +allocator:
> +- simple_allocator_register(struct sa_device *sadev)
> + Register simple_allocator_device using sa_device structure where name,
> + owner and allocate fields must be set.
> +
> +- simple_allocator_unregister(struct sa_device *sadev)
> + Unregister a simple allocator device.
> +
> +Using Simple Allocator /dev interface example
> +-
> +
> +This example of code allocate a buffer on the first CMA region (/dev/cma0)
> +before mmap and close it.
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "simple-allocator.h"
> +
> +#define LENGTH 1024*16
> +
> +void main (void)
> +{
> + struct simple_allocate_data data;
> + int fd = open("/dev/cma0", O_RDWR, 0);
> + int ret;
> + void *mem;
> +
> + if (fd < 0) {
> + printf("Can't open /dev/cma0\n");
> + return;
> + }
> +
> + memset(&data, 0, sizeof(data));
> +
> + data.length = LENGTH;
> + data.flags = O_RDWR | O_CLOEXEC;
> +
> + ret = ioctl(fd, SA_IOC_ALLOC, &data);
> + if (ret) {
> + printf("Buffer allocation failed\n");
> + goto end;
> + }
> +
> + mem = mmap(0, LENGTH, PROT_READ | PROT_WRITE, MAP_SHARED, data.fd, 0);
> + if (mem == MAP_FAILED) {
> + printf("mmap failed\n");
> + }
> +
> + memset(mem, 0xFF, LENGTH);
> + munmap(mem, LENGTH);
> +
> + printf("test simple allocator CMA OK\n");
> +end:
> + close(fd);
> +}
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index e1e2066..a6d8828 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -202,4 +202,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
>
> source "drivers/fpga/Kconfig"
>
> +source "drivers/simpleallocator/Kconfig"
> +
> endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 060026a..5081eb8 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -173,3 +173,4 @@ obj-$(CONFIG_STM) += hwtracing/stm/
> obj-$(CONFIG_ANDROID) += android/
> obj-$(CONFIG_NVMEM)+= nvmem/
> obj-$(CONFIG_FPGA) += fpga/
> +obj-$(CONFIG_SIMPLE_ALLOCATOR) += simpleallocator/
> diff --git a/drivers/simpleallocator/Kconfig b/drivers/simpleallocator/Kconfig
> new file mode 100644
> index 000..c6fc2e3
> --- /dev/null
> +++ b/drivers/simpleallocator/Kconfig
> @@ -0,0 +1,10 @@
> +menu "Simple Allocator"
> +
> +config SIMPLE_ALLOCATOR
> + tristate "Simple Alllocator Framework"
> + select DMA_SHARED_BUFFER
> + ---help---
> + The Simple Allocator Fra
[PATCH] drm/nouveau/core: recognise GP107 chipset
From: Chris Chiu
This new graphics card was failing to initialize with nouveau due to
an "unknown chipset" error.
Copy the GP106 configuration and rename for GP107/NV137. We don't
know for certain that this is fully correct, but brief desktop testing
suggests this is working fine.
Signed-off-by: Chris Chiu
Signed-off-by: Daniel Drake
---
drivers/gpu/drm/nouveau/nvkm/engine/device/base.c | 29 +++
1 file changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
index fea30d6..d242431 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
@@ -2237,6 +2237,34 @@ nv136_chipset = {
.fifo = gp100_fifo_new,
};
+static const struct nvkm_device_chip
+nv137_chipset = {
+ .name = "GP107",
+ .bar = gf100_bar_new,
+ .bios = nvkm_bios_new,
+ .bus = gf100_bus_new,
+ .devinit = gm200_devinit_new,
+ .fb = gp104_fb_new,
+ .fuse = gm107_fuse_new,
+ .gpio = gk104_gpio_new,
+ .i2c = gm200_i2c_new,
+ .ibus = gm200_ibus_new,
+ .imem = nv50_instmem_new,
+ .ltc = gp100_ltc_new,
+ .mc = gp100_mc_new,
+ .mmu = gf100_mmu_new,
+ .pci = gp100_pci_new,
+ .timer = gk20a_timer_new,
+ .top = gk104_top_new,
+ .ce[0] = gp104_ce_new,
+ .ce[1] = gp104_ce_new,
+ .ce[2] = gp104_ce_new,
+ .ce[3] = gp104_ce_new,
+ .disp = gp104_disp_new,
+ .dma = gf119_dma_new,
+ .fifo = gp100_fifo_new,
+};
+
static int
nvkm_device_event_ctor(struct nvkm_object *object, void *data, u32 size,
struct nvkm_notify *notify)
@@ -2673,6 +2701,7 @@ nvkm_device_ctor(const struct nvkm_device_func *func,
case 0x130: device->chip = &nv130_chipset; break;
case 0x134: device->chip = &nv134_chipset; break;
case 0x136: device->chip = &nv136_chipset; break;
+ case 0x137: device->chip = &nv137_chipset; break;
default:
nvdev_error(device, "unknown chipset (%08x)\n", boot0);
goto done;
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
On Tue, Feb 14, 2017 at 8:25 PM, John Stultz wrote:
> Currently, on the hikey board, we have the adv7511 bridge wired
> up to the kirin ade drm driver. Unfortunately, the kirin ade
> core cannot generate accurate byteclocks for all pixel clock
> values.
>
> Thus if a mode clock is selected that we cannot calculate a
> matching byteclock, the device will boot with a blank screen.
>
> Unfortunately, currently the only place we can properly check
> potential modes for this issue in the connector mode_valid
> helper. Again, hikey uses the adv7511 bridge, which is shared
> between a number of different devices, so its improper to put
> restrictions caused by the kirin drm driver in the adv7511
> logic.
>
> So this patch tries to correct for that, by adding some
> infrastructure so that the drm_crtc_helper_funcs can optionally
> implement a mode_valid check, so that the probe helpers can
> check to make sure there are not any restrictions at the crtc
> level as well.
>
> Cc: Daniel Vetter
> Cc: Jani Nikula
> Cc: Sean Paul
> Cc: David Airlie
> Cc: Rob Clark
> Cc: Xinliang Liu
> Cc: Xinliang Liu
> Cc: Rongrong Zou
> Cc: Xinwei Kong
> Cc: Chen Feng
> Cc: Archit Taneja
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz
So I'm going to be super-annoying here and ask for a complete
solution. This here is defacto what ever driver already does (or has
too), but it doesn't really solve the overall issue of having entirely
separate validation paths for probe and atomic_check paths. I think if
we wan to solve this, we need to solve this properly, with a generic
solution. That would mean:
- still in helpers, to make it all opt-int
- covers crtc and encoders and bridges
- allows you to implement the current mode_valid in terms of the new
stuff (maybe as a default hook)
- allows you to implement the current assortment of mode_fixup and/or
atomic_check in terms of the new stuff, or at least to not have to
duplicate logic in there
Docs for all this, especially updating all the warnings on how to use
the existing hooks correctly.
I think just pushing this specific case into the helpers, without
rethinking the overall big picture, isn't gaining us all that much.
For just this I'd say just put it into drivers, until we have some
good clue about how the helpers should look like (maybe this time is
the time? ...).
Cheers, Daniel
> ---
> drivers/gpu/drm/drm_probe_helper.c | 24
> include/drm/drm_modeset_helper_vtables.h | 26 ++
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index cf8f012..a808348 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -170,6 +170,28 @@ drm_connector_detect(struct drm_connector *connector,
> bool force)
> connector_status_connected;
> }
>
> +static enum drm_mode_status
> +drm_connector_check_crtc_modes(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> +{
> + struct drm_device *dev = connector->dev;
> + const struct drm_crtc_helper_funcs *crtc_funcs;
> + struct drm_crtc *c;
> +
> + if (mode->status != MODE_OK)
> + return mode->status;
> +
> + /* Check all the crtcs on a connector to make sure the mode is valid
> */
> + drm_for_each_crtc(c, dev) {
> + crtc_funcs = c->helper_private;
> + if (crtc_funcs && crtc_funcs->mode_valid)
> + mode->status = crtc_funcs->mode_valid(c, mode);
> + if (mode->status != MODE_OK)
> + break;
> + }
> + return mode->status;
> +}
> +
> /**
> * drm_helper_probe_single_connector_modes - get complete set of display
> modes
> * @connector: connector to probe
> @@ -338,6 +360,8 @@ int drm_helper_probe_single_connector_modes(struct
> drm_connector *connector,
> if (mode->status == MODE_OK && connector_funcs->mode_valid)
> mode->status = connector_funcs->mode_valid(connector,
>mode);
> +
> + mode->status = drm_connector_check_crtc_modes(connector,
> mode);
> }
>
> prune:
> diff --git a/include/drm/drm_modeset_helper_vtables.h
> b/include/drm/drm_modeset_helper_vtables.h
> index 69c3974..53ca0e4 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -105,6 +105,32 @@ struct drm_crtc_helper_funcs {
> void (*commit)(struct drm_crtc *crtc);
>
> /**
> +* @mode_valid:
> +*
> +* Callback to validate a mode for a crtc, irrespective of the
> +* specific display configuration.
> +*
> +* This callback is used by the probe helpers to filter the mode list
> +* (which is usually derived from the EDID data block
Re: [RFC simple allocator v2 1/2] Create Simple Allocator module
Hi Daniel,
On Tuesday 14 Feb 2017 20:33:58 Daniel Vetter wrote:
> On Mon, Feb 13, 2017 at 3:45 PM, Benjamin Gaignard wrote:
> > This is the core of simple allocator module.
> > It aim to offert one common ioctl to allocate specific memory.
> >
> > version 2:
> > - rebased on 4.10-rc7
> >
> > Signed-off-by: Benjamin Gaignard
>
> Why not ION? It's a bit a broken record question, but if there is a
> clear answer it should be in the patch&docs ...
There's a bit of love & hate relationship between Linux developers and ION.
The API has shortcomings, and attempts to fix the issues went nowhere. As
Laura explained, starting from a blank slate (obviously keeping in mind the
lessons learnt so far with ION and other similar APIs) and then adding a
wrapper to expose ION on Android systems (at least as an interim measure) was
thought to be a better option. I still believe it is, but we seem to lack
traction. The problem has been around for so long that I feel everybody has
lost hope.
I don't think this is unsolvable, but we need to regain motivation. In my
opinion the first step would be to define the precise extent of the problem we
want to solve.
> > ---
> >
> > Documentation/simple-allocator.txt | 81 +++
> > drivers/Kconfig | 2 +
> > drivers/Makefile| 1 +
> > drivers/simpleallocator/Kconfig | 10 ++
> > drivers/simpleallocator/Makefile| 1 +
> > drivers/simpleallocator/simple-allocator-priv.h | 33 +
> > drivers/simpleallocator/simple-allocator.c | 180 +++
> > include/uapi/linux/simple-allocator.h | 35 +
> > 8 files changed, 343 insertions(+)
> > create mode 100644 Documentation/simple-allocator.txt
> > create mode 100644 drivers/simpleallocator/Kconfig
> > create mode 100644 drivers/simpleallocator/Makefile
> > create mode 100644 drivers/simpleallocator/simple-allocator-priv.h
> > create mode 100644 drivers/simpleallocator/simple-allocator.c
> > create mode 100644 include/uapi/linux/simple-allocator.h
> >
> > diff --git a/Documentation/simple-allocator.txt
> > b/Documentation/simple-allocator.txt new file mode 100644
> > index 000..89ba883
> > --- /dev/null
> > +++ b/Documentation/simple-allocator.txt
> > @@ -0,0 +1,81 @@
> > +Simple Allocator Framework
> > +
> > +Simple Allocator offer a single ioctl SA_IOC_ALLOC to allocate buffers
> > +on dedicated memory regions and export them as a dmabuf file descriptor.
> > +Using dmabuf file descriptor allow to share this memory between processes
> > +and/or import it into other frameworks like v4l2 or drm/kms (prime).
> > +When userland wants to free the memory only a call to close() in needed
> > +so it could done even without knowing that buffer has been allocated by
> > +simple allocator ioctl.
> > +
> > +Each memory regions will be seen as a filein /dev/.
> > +For example CMA regions will exposed has /dev/cmaX.
> > +
> > +Implementing a simple allocator
> > +---
> > +
> > +Simple Allocator provide helpers functions to register/unregister an
> > +allocator:
> > +- simple_allocator_register(struct sa_device *sadev)
> > + Register simple_allocator_device using sa_device structure where name,
> > + owner and allocate fields must be set.
> > +
> > +- simple_allocator_unregister(struct sa_device *sadev)
> > + Unregister a simple allocator device.
> > +
> > +Using Simple Allocator /dev interface example
> > +-
> > +
> > +This example of code allocate a buffer on the first CMA region
> > (/dev/cma0)
> > +before mmap and close it.
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "simple-allocator.h"
> > +
> > +#define LENGTH 1024*16
> > +
> > +void main (void)
> > +{
> > + struct simple_allocate_data data;
> > + int fd = open("/dev/cma0", O_RDWR, 0);
> > + int ret;
> > + void *mem;
> > +
> > + if (fd < 0) {
> > + printf("Can't open /dev/cma0\n");
> > + return;
> > + }
> > +
> > + memset(&data, 0, sizeof(data));
> > +
> > + data.length = LENGTH;
> > + data.flags = O_RDWR | O_CLOEXEC;
> > +
> > + ret = ioctl(fd, SA_IOC_ALLOC, &data);
> > + if (ret) {
> > + printf("Buffer allocation failed\n");
> > + goto end;
> > + }
> > +
> > + mem = mmap(0, LENGTH, PROT_READ | PROT_WRITE, MAP_SHARED, data.fd,
> > 0); + if (mem == MAP_FAILED) {
> > + printf("mmap failed\n");
> > + }
> > +
> > + memset(mem, 0xFF, LENGTH);
> > + munmap(mem, LENGTH);
> > +
> > + printf("test simple allocator CMA OK\n");
> > +end:
> > + close(fd);
> > +}
> > diff --git a/drivers/Kconfig b/drivers/Kconfig
> > index e1e2066..a6d8828 100644
>
Re: [RFC simple allocator v2 1/2] Create Simple Allocator module
On Tue, Feb 14, 2017 at 8:39 PM, Laurent Pinchart wrote: > Hi Daniel, > > On Tuesday 14 Feb 2017 20:33:58 Daniel Vetter wrote: >> On Mon, Feb 13, 2017 at 3:45 PM, Benjamin Gaignard wrote: >> > This is the core of simple allocator module. >> > It aim to offert one common ioctl to allocate specific memory. >> > >> > version 2: >> > - rebased on 4.10-rc7 >> > >> > Signed-off-by: Benjamin Gaignard >> >> Why not ION? It's a bit a broken record question, but if there is a >> clear answer it should be in the patch&docs ... > > There's a bit of love & hate relationship between Linux developers and ION. > The API has shortcomings, and attempts to fix the issues went nowhere. As > Laura explained, starting from a blank slate (obviously keeping in mind the > lessons learnt so far with ION and other similar APIs) and then adding a > wrapper to expose ION on Android systems (at least as an interim measure) was > thought to be a better option. I still believe it is, but we seem to lack > traction. The problem has been around for so long that I feel everybody has > lost hope. > > I don't think this is unsolvable, but we need to regain motivation. In my > opinion the first step would be to define the precise extent of the problem we > want to solve. I'm not sure anyone really tried hard enough (in the same way no one tried hard enough to destage android syncpts, until last year). And anything new should at least very clearly explain why ION (even with the various todo items we collected at a few conferences) won't work, and how exactly the new allocator is different from ION. I don't think we need a full design doc (like you say, buffer allocation is hard, we'll get it wrong anyway), but at least a proper comparison with the existing thing. Plus explanation why we can't reuse the uabi. Because ime when you rewrite something, you generally get one thing right (the one thing that pissed you off about the old solution), plus lots and lots of things that the old solution got right, wrong (because it's all lost in the history). ADF was probably the best example in this. KMS also took a while until all the fbdev wheels have been properly reinvented (some are still the same old squeaky onces as fbdev had, e.g. fbcon). And I don't think destaging ION is going to be hard, just a bit of work (could be a nice gsoc or whatever). -Daniel > >> > --- >> > >> > Documentation/simple-allocator.txt | 81 +++ >> > drivers/Kconfig | 2 + >> > drivers/Makefile| 1 + >> > drivers/simpleallocator/Kconfig | 10 ++ >> > drivers/simpleallocator/Makefile| 1 + >> > drivers/simpleallocator/simple-allocator-priv.h | 33 + >> > drivers/simpleallocator/simple-allocator.c | 180 +++ >> > include/uapi/linux/simple-allocator.h | 35 + >> > 8 files changed, 343 insertions(+) >> > create mode 100644 Documentation/simple-allocator.txt >> > create mode 100644 drivers/simpleallocator/Kconfig >> > create mode 100644 drivers/simpleallocator/Makefile >> > create mode 100644 drivers/simpleallocator/simple-allocator-priv.h >> > create mode 100644 drivers/simpleallocator/simple-allocator.c >> > create mode 100644 include/uapi/linux/simple-allocator.h >> > >> > diff --git a/Documentation/simple-allocator.txt >> > b/Documentation/simple-allocator.txt new file mode 100644 >> > index 000..89ba883 >> > --- /dev/null >> > +++ b/Documentation/simple-allocator.txt >> > @@ -0,0 +1,81 @@ >> > +Simple Allocator Framework >> > + >> > +Simple Allocator offer a single ioctl SA_IOC_ALLOC to allocate buffers >> > +on dedicated memory regions and export them as a dmabuf file descriptor. >> > +Using dmabuf file descriptor allow to share this memory between processes >> > +and/or import it into other frameworks like v4l2 or drm/kms (prime). >> > +When userland wants to free the memory only a call to close() in needed >> > +so it could done even without knowing that buffer has been allocated by >> > +simple allocator ioctl. >> > + >> > +Each memory regions will be seen as a filein /dev/. >> > +For example CMA regions will exposed has /dev/cmaX. >> > + >> > +Implementing a simple allocator >> > +--- >> > + >> > +Simple Allocator provide helpers functions to register/unregister an >> > +allocator: >> > +- simple_allocator_register(struct sa_device *sadev) >> > + Register simple_allocator_device using sa_device structure where name, >> > + owner and allocate fields must be set. >> > + >> > +- simple_allocator_unregister(struct sa_device *sadev) >> > + Unregister a simple allocator device. >> > + >> > +Using Simple Allocator /dev interface example >> > +- >> > + >> > +This example of code allocate a buffer on the first CMA region >> > (/dev/cma0) >> > +before mmap and close it. >> > + >> > +#include >> > +#include >> > +#include >> >
Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
On Tue, Feb 14, 2017 at 11:38 AM, Daniel Vetter wrote: > On Tue, Feb 14, 2017 at 8:25 PM, John Stultz wrote: >> Currently, on the hikey board, we have the adv7511 bridge wired >> up to the kirin ade drm driver. Unfortunately, the kirin ade >> core cannot generate accurate byteclocks for all pixel clock >> values. >> >> Thus if a mode clock is selected that we cannot calculate a >> matching byteclock, the device will boot with a blank screen. >> >> Unfortunately, currently the only place we can properly check >> potential modes for this issue in the connector mode_valid >> helper. Again, hikey uses the adv7511 bridge, which is shared >> between a number of different devices, so its improper to put >> restrictions caused by the kirin drm driver in the adv7511 >> logic. >> >> So this patch tries to correct for that, by adding some >> infrastructure so that the drm_crtc_helper_funcs can optionally >> implement a mode_valid check, so that the probe helpers can >> check to make sure there are not any restrictions at the crtc >> level as well. >> >> Cc: Daniel Vetter >> Cc: Jani Nikula >> Cc: Sean Paul >> Cc: David Airlie >> Cc: Rob Clark >> Cc: Xinliang Liu >> Cc: Xinliang Liu >> Cc: Rongrong Zou >> Cc: Xinwei Kong >> Cc: Chen Feng >> Cc: Archit Taneja >> Cc: dri-devel@lists.freedesktop.org >> Signed-off-by: John Stultz > > So I'm going to be super-annoying here and ask for a complete > solution. This here is defacto what ever driver already does (or has > too), but it doesn't really solve the overall issue of having entirely > separate validation paths for probe and atomic_check paths. I think if > we wan to solve this, we need to solve this properly, with a generic > solution. That would mean: > - still in helpers, to make it all opt-int Just to be clear, I believe what I proposed is opt-in, but I assume you want that in addition to the following, right? > - covers crtc and encoders and bridges So you'd like similar mode_valid() calls in the crtc/encoder/bridge helpers? right? > - allows you to implement the current mode_valid in terms of the new > stuff (maybe as a default hook) This bit I'm not sure I'm following. The current drm_connector's mode_valid in terms of a new mode_valid call that also looks at crtc/encoder/bridges? Or do you mean something else? > - allows you to implement the current assortment of mode_fixup and/or > atomic_check in terms of the new stuff, or at least to not have to > duplicate logic in there This is over my head, so I'll have to research to better understand. > Docs for all this, especially updating all the warnings on how to use > the existing hooks correctly. That's fair. > I think just pushing this specific case into the helpers, without > rethinking the overall big picture, isn't gaining us all that much. > For just this I'd say just put it into drivers, until we have some Not following here. What do you mean by "put it into drivers"? Where? thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v3 7/8] drm: Connector helper function to release resources
On Mon, Feb 13, 2017 at 10:26 PM, Pandiyan, Dhinakaran
wrote:
> On Mon, 2017-02-13 at 09:05 +, Lankhorst, Maarten wrote:
>> Pandiyan, Dhinakaran schreef op do 09-02-2017 om 18:55 [+]:
>> > On Thu, 2017-02-09 at 09:01 +, Lankhorst, Maarten wrote:
>> > >
>> > > Dhinakaran Pandiyan schreef op wo 08-02-2017 om 22:38 [-0800]:
>> > > >
>> > > > Having a ->atomic_release callback is useful to release shared
>> > > > resources
>> > > > that get allocated in compute_config(). This function is expected
>> > > > to
>> > > > be
>> > > > called in the atomic_check() phase before new resources are
>> > > > acquired.
>> > > >
>> > > > v2: Moved the caller hunk to this patch (Daniel)
>> > > >
>> > > > Suggested-by: Daniel Vetter
>> > > > Signed-off-by: Dhinakaran Pandiyan > > > > >
>> > > > ---
>> > > > drivers/gpu/drm/drm_atomic_helper.c | 19
>> > > > +++
>> > > > include/drm/drm_modeset_helper_vtables.h | 13 +
>> > > > 2 files changed, 32 insertions(+)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> > > > b/drivers/gpu/drm/drm_atomic_helper.c
>> > > > index 8795088..92bd741 100644
>> > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
>> > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> > > > @@ -576,6 +576,25 @@ drm_atomic_helper_check_modeset(struct
>> > > > drm_device *dev,
>> > > > }
>> > > > }
>> > > >
>> > > > + for_each_connector_in_state(state, connector,
>> > > > connector_state, i) {
>> > > > + const struct drm_connector_helper_funcs
>> > > > *conn_funcs;
>> > > > + struct drm_crtc_state *crtc_state;
>> > > > +
>> > > > + conn_funcs = connector->helper_private;
>> > > > + if (!conn_funcs->atomic_release)
>> > > > + continue;
>> > > > +
>> > > > + if (!connector->state->crtc)
>> > > > + continue;
>> > > > +
>> > > > + crtc_state =
>> > > > drm_atomic_get_existing_crtc_state(state, connector->state-
>> > > > >crtc);
>> > > > +
>> > > > + if (crtc_state->connectors_changed ||
>> > > > + crtc_state->mode_changed ||
>> > > > + (crtc_state->active_changed && !crtc_state-
>> > > > >
>> > > > > active))
>> > > > + conn_funcs->atomic_release(connector,
>> > > > connector_state);
>> > > > + }
>> > >
>> > > Could we deal with the VCPI state separately in
>> > > intel_modeset_checks,
>> > > like we do with dpll?
>> >
>> > We'd want to release the VCPI slots before they are acquired in
>> > ->compute_config(). intel_modeset_checks() will be too late to
>> > release
>> > them. Are you suggesting both acquiring and releasing slots should be
>> > done in intel_modeset_checks()?
>>
>> That makes things a bit more nasty. Maybe add a
>> conn_funcs->atomic_check that always gets called, something like I did
>> below?
>>
>> I'd love to use it for some atomic connector properties too.
>
>
> Adding and unconditionally calling conn_funcs->atomic_check() should be
> doable. It also follows the pattern we have for encoders and CRTCs. But
> I'll have to move the connector->state->crtc state checks inside the
> function.
Adding ->atomic_check that's unconditionally called sounds troubling,
because all the other ->atomic_check functions are _only_ called when
enabling stuff. ->atomic_release sounds much better to me, and from a
helper pov DK's patch above is the right place.
If that place doesn't work for i915.ko, then we need our own callback
(like we already have with e.g. ->compute_config, we could do a
->release_config). But if it's just cosmetics, then I don't see the
reason why we need to change this. On that issue: How exactly does our
compute_config work if we haven't updated the routing (using the above
helper) yet? That sounds like a pretty big misdesign on our side ...
-Daniel
>
> -DK
>> > >
>> > >
>> > > Maybe implementing the relevant VCPI state could be done as an
>> > > atomic
>> > > helper function too, so other atomic drivers can just plug it in.
>> > >
>> > The idea was to reduce boilerplate in the drivers and use the
>> > private_obj state for different object types.
>> >
>> >
>> > >
>> > > Not sure how doable this is, but if it's not too hard, then it's
>> > > probably cleaner :)
>> > > ___
>> > > Intel-gfx mailing list
>> > > intel-...@lists.freedesktop.org
>> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> ___
>> Intel-gfx mailing list
>> intel-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
On Tue, Feb 14, 2017 at 08:38:40PM +0100, Daniel Vetter wrote: > On Tue, Feb 14, 2017 at 8:25 PM, John Stultz wrote: > > Currently, on the hikey board, we have the adv7511 bridge wired > > up to the kirin ade drm driver. Unfortunately, the kirin ade > > core cannot generate accurate byteclocks for all pixel clock > > values. > > > > Thus if a mode clock is selected that we cannot calculate a > > matching byteclock, the device will boot with a blank screen. > > > > Unfortunately, currently the only place we can properly check > > potential modes for this issue in the connector mode_valid > > helper. Again, hikey uses the adv7511 bridge, which is shared > > between a number of different devices, so its improper to put > > restrictions caused by the kirin drm driver in the adv7511 > > logic. > > > > So this patch tries to correct for that, by adding some > > infrastructure so that the drm_crtc_helper_funcs can optionally > > implement a mode_valid check, so that the probe helpers can > > check to make sure there are not any restrictions at the crtc > > level as well. > > > > Cc: Daniel Vetter > > Cc: Jani Nikula > > Cc: Sean Paul > > Cc: David Airlie > > Cc: Rob Clark > > Cc: Xinliang Liu > > Cc: Xinliang Liu > > Cc: Rongrong Zou > > Cc: Xinwei Kong > > Cc: Chen Feng > > Cc: Archit Taneja > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: John Stultz > > So I'm going to be super-annoying here and ask for a complete > solution. This here is defacto what ever driver already does (or has > too), but it doesn't really solve the overall issue of having entirely > separate validation paths for probe and atomic_check paths. I think if > we wan to solve this, we need to solve this properly, with a generic > solution. That would mean: > - still in helpers, to make it all opt-int > - covers crtc and encoders and bridges > - allows you to implement the current mode_valid in terms of the new > stuff (maybe as a default hook) > - allows you to implement the current assortment of mode_fixup and/or > atomic_check in terms of the new stuff, or at least to not have to > duplicate logic in there Long ago I quickly looked at doing this for i915. IIRC the main complication was the normal vs. the crtc_ timings stored in the mode. I suppose one option would be to populate the crtc_ timings in .mode_valid() as well and otherwise just ignore the normal timings. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 1/7] drm: Add DRM support for tiny LCD displays
On Sun, Feb 12, 2017 at 2:57 PM, Noralf Trønnes wrote: > tinydrm will be merged the way it is now, unless someone points to > something that is broken. But I collect your comments for a later > cleanup patchset. It takes a lot of effort for me as an amateur to > keeps this code up-to-date out-of-tree for months. It's not even > sure that I've hit the mark with this, so there will most likely be > changes when I start converting fbtft drivers, and I'll implement the > remaining bits and pieces as I make changes. The core of tinydrm won't > change because of unforseen fbtft quirks, but other parts might. +1, pls send pull request to Dave Airlie. DT-acks are all there, and Thierry confirmed on irc that his reply on the last thread was indeed meant as a full ack for going ahead and polishing later. I also chatted with Dave directly, and he's perfectly happy to take this in as-is. If you do this soon, should still easily get into 4.11 (since the 4.10 release is delayed). Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC simple allocator v2 1/2] Create Simple Allocator module
Hi Daniel, On Tuesday 14 Feb 2017 20:44:44 Daniel Vetter wrote: > On Tue, Feb 14, 2017 at 8:39 PM, Laurent Pinchart wrote: > > On Tuesday 14 Feb 2017 20:33:58 Daniel Vetter wrote: > >> On Mon, Feb 13, 2017 at 3:45 PM, Benjamin Gaignard wrote: > >>> This is the core of simple allocator module. > >>> It aim to offert one common ioctl to allocate specific memory. > >>> > >>> version 2: > >>> - rebased on 4.10-rc7 > >>> > >>> Signed-off-by: Benjamin Gaignard > >> > >> Why not ION? It's a bit a broken record question, but if there is a > >> clear answer it should be in the patch&docs ... > > > > There's a bit of love & hate relationship between Linux developers and > > ION. The API has shortcomings, and attempts to fix the issues went > > nowhere. As Laura explained, starting from a blank slate (obviously > > keeping in mind the lessons learnt so far with ION and other similar APIs) > > and then adding a wrapper to expose ION on Android systems (at least as an > > interim measure) was thought to be a better option. I still believe it is, > > but we seem to lack traction. The problem has been around for so long that > > I feel everybody has lost hope. > > > > I don't think this is unsolvable, but we need to regain motivation. In my > > opinion the first step would be to define the precise extent of the > > problem we want to solve. > > I'm not sure anyone really tried hard enough (in the same way no one > tried hard enough to destage android syncpts, until last year). And > anything new should at least very clearly explain why ION (even with > the various todo items we collected at a few conferences) won't work, > and how exactly the new allocator is different from ION. I don't think > we need a full design doc (like you say, buffer allocation is hard, > we'll get it wrong anyway), but at least a proper comparison with the > existing thing. Plus explanation why we can't reuse the uabi. I've explained several of my concerns (including open questions that need answers) in another reply to this patch, let's discuss them there to avoid splitting the discussion. > Because ime when you rewrite something, you generally get one thing > right (the one thing that pissed you off about the old solution), plus > lots and lots of things that the old solution got right, wrong > (because it's all lost in the history). History, repeating mistakes, all that. History never repeats itself though. We might make similar or identical mistakes, but there's no fatality, unless we decide not to try before even starting :-) > ADF was probably the best example in this. KMS also took a while until all > the fbdev wheels have been properly reinvented (some are still the same old > squeaky onces as fbdev had, e.g. fbcon). > > And I don't think destaging ION is going to be hard, just a bit of > work (could be a nice gsoc or whatever). Oh, technically speaking, it would be pretty simple. The main issue is to decide whether we want to commit to the existing ION API. I don't :-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/7] drm/atomic: Use new atomic iterator macros.
On Tue, Jan 17, 2017 at 01:55:50AM +0200, Laurent Pinchart wrote:
> Hi Maarten,
>
> Thank you for the patch.
>
> On Monday 16 Jan 2017 10:37:40 Maarten Lankhorst wrote:
>
> Could we please get a description ? Apart from that,
Typed something small and merged the first 3 patches from this series.
Thanks, Daniel
>
> Reviewed-by: Laurent Pinchart
>
> > Signed-off-by: Maarten Lankhorst
> > ---
> > drivers/gpu/drm/drm_atomic.c | 16
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 18cdf2c956c6..7b61e0da9ace 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1562,7 +1562,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > *state)
> >
> > DRM_DEBUG_ATOMIC("checking %p\n", state);
> >
> > - for_each_plane_in_state(state, plane, plane_state, i) {
> > + for_each_new_plane_in_state(state, plane, plane_state, i) {
> > ret = drm_atomic_plane_check(plane, plane_state);
> > if (ret) {
> > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check
> failed\n",
> > @@ -1571,7 +1571,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > *state) }
> > }
> >
> > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > ret = drm_atomic_crtc_check(crtc, crtc_state);
> > if (ret) {
> > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check
> failed\n",
> > @@ -1584,7 +1584,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > *state) ret = config->funcs->atomic_check(state->dev, state);
> >
> > if (!state->allow_modeset) {
> > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full
> modeset\n",
> > crtc->base.id, crtc->name);
> > @@ -1668,13 +1668,13 @@ static void drm_atomic_print_state(const struct
> > drm_atomic_state *state)
> >
> > DRM_DEBUG_ATOMIC("checking %p\n", state);
> >
> > - for_each_plane_in_state(state, plane, plane_state, i)
> > + for_each_new_plane_in_state(state, plane, plane_state, i)
> > drm_atomic_plane_print_state(&p, plane_state);
> >
> > - for_each_crtc_in_state(state, crtc, crtc_state, i)
> > + for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> > drm_atomic_crtc_print_state(&p, crtc_state);
> >
> > - for_each_connector_in_state(state, connector, connector_state, i)
> > + for_each_new_connector_in_state(state, connector, connector_state, i)
> > drm_atomic_connector_print_state(&p, connector_state);
> > }
> >
> > @@ -1961,7 +1961,7 @@ static int prepare_crtc_signaling(struct drm_device
> > *dev, if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
> > return 0;
> >
> > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > u64 __user *fence_ptr;
> >
> > fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
> > @@ -2041,7 +2041,7 @@ static void complete_crtc_signaling(struct drm_device
> > *dev, return;
> > }
> >
> > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > /*
> > * TEST_ONLY and PAGE_FLIP_EVENT are mutually
> > * exclusive, if they weren't, this code should be
>
> --
> Regards,
>
> Laurent Pinchart
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: change connector disconnected debug message to an error
On Fri, Feb 10, 2017 at 09:29:07AM -0700, Shuah Khan wrote: > On 02/03/2017 01:06 AM, Daniel Vetter wrote: > > On Thu, Feb 02, 2017 at 10:25:44AM -0700, Shuah Khan wrote: > >> On 02/02/2017 01:32 AM, Jani Nikula wrote: > >>> On Thu, 02 Feb 2017, Shuah Khan wrote: > Change drm_helper_probe_single_connector_modes() to print an error to > report connector disconnected status instead of a debug message. > > When this condition occurs, application doesn't know the real error and > reports it as driver lacking support for mode setting. Change it to an > error to make it easier to debug. > >>> > >>> Please explain what makes this condition an error. Connectors get > >>> connected and disconnected, business as usual, why should this be an > >>> error? > >>> > >>> BR, > >>> Jani. > >> > >> Disconnecting connector itself isn't an error. When user-space tries > >> to access it, it would be useful to report the status that the connector > >> is disconnected. > >> > >> I use embedded system(s) that don't like it when HDMI is hot added or > >> removed. Also, because of return power, it is safer to disconnect HDMI > >> and then apply power to the board. It chased a few libdrm and user-space > >> dead ends before I enabled drm debug and was able to fix the real issue, > >> which is a disconnected cable. > >> > >> User-space prints rather confusing messages as it doesn't really know > >> the disconnected status as it isn't returned to it. > >> > >> I figured it might be a good idea to at least print a message and this can > >> be a notice or info instead of an error. I do think its is worth while in > >> some cases. > > > > This sounds like a very specific use-case you have here, and it can easily > > be supported by a small deamon in userspace (only on debug builds ofc) > > that tell you that someone unplugged the screen when it shouldn't have > > been. > > drm_helper_probe_single_connector_modes() finds the condition and doesn't > have a means to return it to the user-space. Erhm, we send out the uevent for this, and userspace can react. If that's not working, then we need to fix this bug, not add more uapi interfaces on top ... -Daniel > > Instead of error or debug message, would it be useful to add a trace event > to report status of connector to drm_helper_probe_single_connector_modes() > Trace could be triggered as needed and turned off. > > Please let me know what you think of this idea? If it sounds useful, I can > add it. > > > > > Because upstream runs also on non-embedded systems, where unplugging is > > normal, and we definitely don't want to spam dmesg. > > -Daniel > > > > thanks, > -- Shuah > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/7] drm/atomic: Use new atomic iterator macros.
Hi Daniel,
On Tuesday 14 Feb 2017 21:03:07 Daniel Vetter wrote:
> On Tue, Jan 17, 2017 at 01:55:50AM +0200, Laurent Pinchart wrote:
> > On Monday 16 Jan 2017 10:37:40 Maarten Lankhorst wrote:
> >
> > Could we please get a description ? Apart from that,
>
> Typed something small and merged the first 3 patches from this series.
Thanks. 4 more patches to go. Maarten ? :-)
> > Reviewed-by: Laurent Pinchart
> >
> > > Signed-off-by: Maarten Lankhorst
> > > ---
> > >
> > > drivers/gpu/drm/drm_atomic.c | 16
> > > 1 file changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 18cdf2c956c6..7b61e0da9ace 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -1562,7 +1562,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > > *state)
> > >
> > > DRM_DEBUG_ATOMIC("checking %p\n", state);
> > >
> > > - for_each_plane_in_state(state, plane, plane_state, i) {
> > > + for_each_new_plane_in_state(state, plane, plane_state, i) {
> > >
> > > ret = drm_atomic_plane_check(plane, plane_state);
> > > if (ret) {
> > >
> > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check
> >
> > failed\n",
> >
> > > @@ -1571,7 +1571,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > > *state) }
> > >
> > > }
> > >
> > > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > >
> > > ret = drm_atomic_crtc_check(crtc, crtc_state);
> > > if (ret) {
> > >
> > > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check
> >
> > failed\n",
> >
> > > @@ -1584,7 +1584,7 @@ int drm_atomic_check_only(struct drm_atomic_state
> > > *state) ret = config->funcs->atomic_check(state->dev, state);
> > >
> > > if (!state->allow_modeset) {
> > >
> > > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > >
> > > if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> > >
> > > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full
> >
> > modeset\n",
> >
> > >crtc->base.id, crtc->name);
> > >
> > > @@ -1668,13 +1668,13 @@ static void drm_atomic_print_state(const struct
> > > drm_atomic_state *state)
> > >
> > > DRM_DEBUG_ATOMIC("checking %p\n", state);
> > >
> > > - for_each_plane_in_state(state, plane, plane_state, i)
> > > + for_each_new_plane_in_state(state, plane, plane_state, i)
> > >
> > > drm_atomic_plane_print_state(&p, plane_state);
> > >
> > > - for_each_crtc_in_state(state, crtc, crtc_state, i)
> > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> > >
> > > drm_atomic_crtc_print_state(&p, crtc_state);
> > >
> > > - for_each_connector_in_state(state, connector, connector_state, i)
> > > + for_each_new_connector_in_state(state, connector, connector_state, i)
> > >
> > > drm_atomic_connector_print_state(&p, connector_state);
> > >
> > > }
> > >
> > > @@ -1961,7 +1961,7 @@ static int prepare_crtc_signaling(struct
> > > drm_device
> > > *dev, if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
> > >
> > > return 0;
> > >
> > > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > >
> > > u64 __user *fence_ptr;
> > >
> > > fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
> > >
> > > @@ -2041,7 +2041,7 @@ static void complete_crtc_signaling(struct
> > > drm_device
> > > *dev, return;
> > >
> > > }
> > >
> > > - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > >
> > > /*
> > >
> > >* TEST_ONLY and PAGE_FLIP_EVENT are mutually
> > >* exclusive, if they weren't, this code should be
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] drm/cma-helper: Add multi buffer support for cma fbdev
On Mon, Feb 13, 2017 at 11:20:51AM +, Daniel Stone wrote: > Hi Maxime, > > On 13 February 2017 at 10:54, Maxime Ripard > wrote: > > On Sun, Feb 12, 2017 at 02:28:11PM +0200, Laurent Pinchart wrote: > >> On Thursday 02 Feb 2017 11:31:56 Maxime Ripard wrote: > >> > This patch add a config to support to create multi buffer for cma fbdev. > >> > Such as double buffer and triple buffer. > >> > > >> > Cma fbdev is convient to add a legency fbdev. And still many Android > >> > devices use fbdev now and at least double buffer is needed for these > >> > Android devices, so that a buffer flip can be operated. It will need > >> > some time for Android device vendors to abondon legency fbdev. So multi > >> > buffer for fbdev is needed. > >> > >> How exactly do we expect Android to move away from fbdev if we add > >> features to > >> the fbdev compat layer ? I'd much rather make it clear to them that fbdev > >> is a > >> thing from the past and that they'd better migrate now. > > > > If your point is that merging this patch will slow down the Android > > move away from fbdev, I disagree with that (obviously). > > > > I don't care at all about Android on my platform of choice, but don't > > see how that merging this patch will change anything. > > > > Let's be honest, Android trees typically have thousands of patches on > > top of mainline. Do you think a simple, 15 LoC, patch will make any > > difference to vendors? If they want to stay on fbdev and have that > > feature, they'll just merge this patch, done. > > So, in that case, why not just let them do that? They'd already have > to add patches to use this, surely; we don't have anything in mainline > kernels which allows people to actually use this larger allocation. > Apart from software mmap() and using panning to do flips, but I'm > taking it as a given that people shipping Android on their devices > aren't using software rendering. I think we need to make a distinction between fbdev the subsystem in the kernel, and fbdev the uabi: - fbdev the subsystem is completely dead in upstream. I think we have full agreement on that. - fbdev the uabi isn't, and if we can get more users from fbdev based drivers to kms/atomic drivers by adding fairly simple stuff like this, I'm all for it. Which means: Yes, I fully plan to merge this, it makes sense. It even _helps_ by making fbdev-the-subsystem even deader. Making live hard for out-of-tree folks or folks with shit userspace doesn't make sense, at least if the only benefit for us is that we'll feel pure about our intentions :-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}()
On Thu, Feb 09, 2017 at 09:55:22PM +, Emil Velikov wrote: > On 9 February 2017 at 20:41, Thierry Reding wrote: > > On Thu, Feb 09, 2017 at 06:08:10PM +0100, Daniel Vetter wrote: > >> On Wed, Feb 08, 2017 at 02:28:14PM -0500, Sean Paul wrote: > >> > On Wed, Feb 08, 2017 at 07:24:04PM +0100, Thierry Reding wrote: > >> > > From: Thierry Reding > >> > > > >> > > For consistency with other reference counting APIs in the kernel, add > >> > > drm_mode_object_get() and drm_mode_object_put() to reference count DRM > >> > > mode objects. > >> > > > >> > > Compatibility aliases are added to keep existing code working. To help > >> > > speed up the transition, all the instances of the old functions in the > >> > > DRM core are already replaced in this commit. > >> > > > >> > > >> > drm code looks good and is > >> > > >> > Reviewed-by: Sean Paul > >> > > >> > > A semantic patch is provided that can be used to convert all drivers to > >> > > the new helpers. > >> > > >> > I'm not convinced we need to commit the cocci patch. I think including > >> > it in > >> > your cover letter and then following up with a follow on series to > >> > actually make > >> > the change is sufficient (See: ickle's s/fence/dma_fence/ series). > >> > >> Yeah, if you do a large-scale refactor anyway, I think it's best to just > >> store the cocci in the commit message. I think storing the cocci is ok if > >> you have thousands of hits among lots of subsystems, and it's clear it's > >> going to take at least a few release cycles or maybe even years to clean > >> it all up. drm is luckily not yet that big :-) > >> > >> I'll drop this while applying if no one minds ... > > > > I thought it was actually quite nice that this was part of the series. > > That way it doesn't get lost and it is really easy to rerun. Also it can > > trivially be removed once we've converted everyone to the new functions > > and removed the old ones. > > > Hidden bonus: > Some of the people who run those on semi-regular basis can update > some/all the drivers ;-) I agree that this is a nice bonus, but thus far we just mass-converted everyone. That seems to be the plan here too, which is why I don't see much value in recording the cocci patch (outside of the commit message). But drm is growing, and in the future I guess we'll get more refactorings that can't be done in one go, and then adding the spatch makes perfect sense. Anyway, no strong opinions from my side at all, whatever makes sense, just wanted to explain my reasoning quickly. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/msm/dsi: fix error return code in msm_dsi_host_init()
On Thu, Feb 09, 2017 at 03:19:07PM +, Wei Yongjun wrote:
> From: Wei Yongjun
>
> Fix to return error code -ENOMEM from the malloc error handling
> case instead of 0, as done elsewhere in this function.
>
> Signed-off-by: Wei Yongjun
Applied for 4.12 to drm-misc-next, thanks.
-Daniel
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 1fc07ce..4f79b10 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1740,6 +1740,7 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
>
> msm_host->rx_buf = devm_kzalloc(&pdev->dev, SZ_4K, GFP_KERNEL);
> if (!msm_host->rx_buf) {
> + ret = -ENOMEM;
> pr_err("%s: alloc rx temp buf failed\n", __func__);
> goto fail;
> }
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99815] Power management problems & kernel hangs with Cap Verde
https://bugs.freedesktop.org/show_bug.cgi?id=99815 --- Comment #1 from Klaus Kusche --- I just checked: It seems that the problem does not depend on two active outputs. The gpu consumes roughly the same amount of power with just the laptop display. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: ensure atomic messages consistently include the name of the component
On Mon, Feb 13, 2017 at 04:47:01PM +, Russell King - ARM Linux wrote: > On Mon, Feb 13, 2017 at 02:37:31PM +0100, Maarten Lankhorst wrote: > > All for it, looks sane. The last hunk fails to apply because it's based on > > an older version of page_flip, but easy enough to fix. > > Yes, it's based on v4.10-rc7. Resolved conflict and merged for 4.12 into drm-misc-next. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
On Tue, Feb 14, 2017 at 11:45:54AM -0800, John Stultz wrote: > On Tue, Feb 14, 2017 at 11:38 AM, Daniel Vetter wrote: > > On Tue, Feb 14, 2017 at 8:25 PM, John Stultz wrote: > >> Currently, on the hikey board, we have the adv7511 bridge wired > >> up to the kirin ade drm driver. Unfortunately, the kirin ade > >> core cannot generate accurate byteclocks for all pixel clock > >> values. > >> > >> Thus if a mode clock is selected that we cannot calculate a > >> matching byteclock, the device will boot with a blank screen. > >> > >> Unfortunately, currently the only place we can properly check > >> potential modes for this issue in the connector mode_valid > >> helper. Again, hikey uses the adv7511 bridge, which is shared > >> between a number of different devices, so its improper to put > >> restrictions caused by the kirin drm driver in the adv7511 > >> logic. > >> > >> So this patch tries to correct for that, by adding some > >> infrastructure so that the drm_crtc_helper_funcs can optionally > >> implement a mode_valid check, so that the probe helpers can > >> check to make sure there are not any restrictions at the crtc > >> level as well. > >> > >> Cc: Daniel Vetter > >> Cc: Jani Nikula > >> Cc: Sean Paul > >> Cc: David Airlie > >> Cc: Rob Clark > >> Cc: Xinliang Liu > >> Cc: Xinliang Liu > >> Cc: Rongrong Zou > >> Cc: Xinwei Kong > >> Cc: Chen Feng > >> Cc: Archit Taneja > >> Cc: dri-devel@lists.freedesktop.org > >> Signed-off-by: John Stultz > > > > So I'm going to be super-annoying here and ask for a complete > > solution. This here is defacto what ever driver already does (or has > > too), but it doesn't really solve the overall issue of having entirely > > separate validation paths for probe and atomic_check paths. I think if > > we wan to solve this, we need to solve this properly, with a generic > > solution. That would mean: > > - still in helpers, to make it all opt-int > > Just to be clear, I believe what I proposed is opt-in, but I assume > you want that in addition to the following, right? > > > - covers crtc and encoders and bridges > > So you'd like similar mode_valid() calls in the crtc/encoder/bridge > helpers? right? > > > - allows you to implement the current mode_valid in terms of the new > > stuff (maybe as a default hook) > > This bit I'm not sure I'm following. The current drm_connector's > mode_valid in terms of a new mode_valid call that also looks at > crtc/encoder/bridges? Or do you mean something else? > > > - allows you to implement the current assortment of mode_fixup and/or > > atomic_check in terms of the new stuff, or at least to not have to > > duplicate logic in there > > This is over my head, so I'll have to research to better understand. > > > Docs for all this, especially updating all the warnings on how to use > > the existing hooks correctly. > > That's fair. > > > I think just pushing this specific case into the helpers, without > > rethinking the overall big picture, isn't gaining us all that much. > > For just this I'd say just put it into drivers, until we have some > > Not following here. What do you mean by "put it into drivers"? Where? In your driver callback for ->mode_valid, call into a shared function to validate CRTC limits. Which you also call from the crtc's ->mode_fixup function. In short my complain here is that this is only a partial solution of the larger problem, specific for your driver. That kind of code is better put into drivers, until we have a clear understanding to type up something complete in the helpers. Shared code is imo overrated :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
Hi John,
On 14 February 2017 at 19:25, John Stultz wrote:
> +static enum drm_mode_status
> +drm_connector_check_crtc_modes(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> +{
> + struct drm_device *dev = connector->dev;
> + const struct drm_crtc_helper_funcs *crtc_funcs;
> + struct drm_crtc *c;
> +
> + if (mode->status != MODE_OK)
> + return mode->status;
> +
> + /* Check all the crtcs on a connector to make sure the mode is valid
> */
> + drm_for_each_crtc(c, dev) {
> + crtc_funcs = c->helper_private;
> + if (crtc_funcs && crtc_funcs->mode_valid)
> + mode->status = crtc_funcs->mode_valid(c, mode);
> + if (mode->status != MODE_OK)
> + break;
> + }
> + return mode->status;
> +}
Hm, that's unfortunate: it limits the mode list for every connector,
to those which are supported by every single CRTC. So if you have one
CRTC serving low-res LVDS, and another serving higher-res HDMI,
suddenly you can't get bigger modes on HDMI. The idea seems sound
enough, but a little more nuance might be good ...
Cheers,
Daniel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/nouveau/core: recognise GP107 chipset
I believe what's missing at this point is a mmiotrace of the NVIDIA
driver to make sure that there's nothing different about this GPU. If
you can make one (see https://wiki.ubuntu.com/X/MMIOTracing for a
guide - should end up ~100MB uncompressed), please send a compressed
one to mmio.du...@gmail.com or make available some other way.
On Tue, Feb 14, 2017 at 2:34 PM, Daniel Drake wrote:
> From: Chris Chiu
>
> This new graphics card was failing to initialize with nouveau due to
> an "unknown chipset" error.
>
> Copy the GP106 configuration and rename for GP107/NV137. We don't
> know for certain that this is fully correct, but brief desktop testing
> suggests this is working fine.
>
> Signed-off-by: Chris Chiu
> Signed-off-by: Daniel Drake
> ---
> drivers/gpu/drm/nouveau/nvkm/engine/device/base.c | 29
> +++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
> index fea30d6..d242431 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
> @@ -2237,6 +2237,34 @@ nv136_chipset = {
> .fifo = gp100_fifo_new,
> };
>
> +static const struct nvkm_device_chip
> +nv137_chipset = {
> + .name = "GP107",
> + .bar = gf100_bar_new,
> + .bios = nvkm_bios_new,
> + .bus = gf100_bus_new,
> + .devinit = gm200_devinit_new,
> + .fb = gp104_fb_new,
> + .fuse = gm107_fuse_new,
> + .gpio = gk104_gpio_new,
> + .i2c = gm200_i2c_new,
> + .ibus = gm200_ibus_new,
> + .imem = nv50_instmem_new,
> + .ltc = gp100_ltc_new,
> + .mc = gp100_mc_new,
> + .mmu = gf100_mmu_new,
> + .pci = gp100_pci_new,
> + .timer = gk20a_timer_new,
> + .top = gk104_top_new,
> + .ce[0] = gp104_ce_new,
> + .ce[1] = gp104_ce_new,
> + .ce[2] = gp104_ce_new,
> + .ce[3] = gp104_ce_new,
> + .disp = gp104_disp_new,
> + .dma = gf119_dma_new,
> + .fifo = gp100_fifo_new,
> +};
> +
> static int
> nvkm_device_event_ctor(struct nvkm_object *object, void *data, u32 size,
>struct nvkm_notify *notify)
> @@ -2673,6 +2701,7 @@ nvkm_device_ctor(const struct nvkm_device_func *func,
> case 0x130: device->chip = &nv130_chipset; break;
> case 0x134: device->chip = &nv134_chipset; break;
> case 0x136: device->chip = &nv136_chipset; break;
> + case 0x137: device->chip = &nv137_chipset; break;
> default:
> nvdev_error(device, "unknown chipset (%08x)\n",
> boot0);
> goto done;
> --
> 2.9.3
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
On Tue, Feb 14, 2017 at 12:22 PM, Daniel Vetter wrote: > On Tue, Feb 14, 2017 at 11:45:54AM -0800, John Stultz wrote: >> >> Not following here. What do you mean by "put it into drivers"? Where? > > In your driver callback for ->mode_valid, call into a shared function to > validate CRTC limits. Which you also call from the crtc's ->mode_fixup > function. So bascially have the adv7511 driver's mode_valid() have a special callback to the kirin (and freedreno, and whatever other) drm driver to check the modes? Or I guess the drm driver that uses that bridge should register something w/ the adv7511 code? Part of my confusion here is that the main issue is that its not just one driver I'm dealing with, and they're all really just tied together via device tree, so I'm not sure how to special case it in just one of the drivers. > In short my complain here is that this is only a partial solution of the > larger problem, specific for your driver. That kind of code is better put > into drivers, until we have a clear understanding to type up something > complete in the helpers. Shared code is imo overrated :-) Yea, apologies for my not seeing the larger problem at first (its still a bit hazy, really), part of this submission is just trying to get something to throw darts at and figure out the right thing. But I'll try to figure out another approach here. thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
On Tue, Feb 14, 2017 at 12:32 PM, Daniel Stone wrote:
> Hi John,
>
> On 14 February 2017 at 19:25, John Stultz wrote:
>> +static enum drm_mode_status
>> +drm_connector_check_crtc_modes(struct drm_connector *connector,
>> + struct drm_display_mode *mode)
>> +{
>> + struct drm_device *dev = connector->dev;
>> + const struct drm_crtc_helper_funcs *crtc_funcs;
>> + struct drm_crtc *c;
>> +
>> + if (mode->status != MODE_OK)
>> + return mode->status;
>> +
>> + /* Check all the crtcs on a connector to make sure the mode is valid
>> */
>> + drm_for_each_crtc(c, dev) {
>> + crtc_funcs = c->helper_private;
>> + if (crtc_funcs && crtc_funcs->mode_valid)
>> + mode->status = crtc_funcs->mode_valid(c, mode);
>> + if (mode->status != MODE_OK)
>> + break;
>> + }
>> + return mode->status;
>> +}
>
> Hm, that's unfortunate: it limits the mode list for every connector,
> to those which are supported by every single CRTC. So if you have one
> CRTC serving low-res LVDS, and another serving higher-res HDMI,
> suddenly you can't get bigger modes on HDMI. The idea seems sound
> enough, but a little more nuance might be good ...
Yea. That is not my intent at all I'm just trying to get the drm_crtc
attached to the connector that we're getting the EDID mode lines from.
I had tried going connector->encoder->crtc, but at the time this is
called, the encoder is null. So Rob suggested the for_each_crtc(), and
I guess I mistook that for being each crtc on the connector.
Thanks for pointing out this issue. From Daniel's feedback it looks
like I need to start over from scratch though, so little worry this
implementation will go much further.
thanks
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] drm/cma-helper: Add multi buffer support for cma fbdev
Hi Daniel, On Tuesday 14 Feb 2017 21:09:51 Daniel Vetter wrote: > On Mon, Feb 13, 2017 at 11:20:51AM +, Daniel Stone wrote: > > On 13 February 2017 at 10:54, Maxime Ripard wrote: > >> On Sun, Feb 12, 2017 at 02:28:11PM +0200, Laurent Pinchart wrote: > >>> On Thursday 02 Feb 2017 11:31:56 Maxime Ripard wrote: > This patch add a config to support to create multi buffer for cma > fbdev. Such as double buffer and triple buffer. > > Cma fbdev is convient to add a legency fbdev. And still many Android > devices use fbdev now and at least double buffer is needed for these > Android devices, so that a buffer flip can be operated. It will need > some time for Android device vendors to abondon legency fbdev. So > multi buffer for fbdev is needed. > >>> > >>> How exactly do we expect Android to move away from fbdev if we add > >>> features to the fbdev compat layer ? I'd much rather make it clear to > >>> them that fbdev is a thing from the past and that they'd better > >>> migrate now. > >> > >> If your point is that merging this patch will slow down the Android > >> move away from fbdev, I disagree with that (obviously). > >> > >> I don't care at all about Android on my platform of choice, but don't > >> see how that merging this patch will change anything. > >> > >> Let's be honest, Android trees typically have thousands of patches on > >> top of mainline. Do you think a simple, 15 LoC, patch will make any > >> difference to vendors? If they want to stay on fbdev and have that > >> feature, they'll just merge this patch, done. > > > > So, in that case, why not just let them do that? They'd already have > > to add patches to use this, surely; we don't have anything in mainline > > kernels which allows people to actually use this larger allocation. > > Apart from software mmap() and using panning to do flips, but I'm > > taking it as a given that people shipping Android on their devices > > aren't using software rendering. > > I think we need to make a distinction between fbdev the subsystem in the > kernel, and fbdev the uabi: > > - fbdev the subsystem is completely dead in upstream. I think we have full > agreement on that. > - fbdev the uabi isn't, and if we can get more users from fbdev based > drivers to kms/atomic drivers by adding fairly simple stuff like this, > I'm all for it. The real question, in my opinion, is how to get more users for the DRM/KMS userspace API, to help killing the fbdev API. What's the incentive for userspace to migrate if we tell them that we're going to support the fbdev API forever, and will even go through the trouble of extending the supported feature set ? I have a customer who wouldn't have migrated their userspace to DRM/KMS if these two patches had been merged a few years ago. I'd rather *reduce* the supported feature set over time until we can finally switch fbdev off. > Which means: Yes, I fully plan to merge this, it makes sense. It even > _helps_ by making fbdev-the-subsystem even deader. Making live hard for > out-of-tree folks or folks with shit userspace doesn't make sense, at > least if the only benefit for us is that we'll feel pure about our > intentions :-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/8] Refactor DC atomic commit and gamma
On Fri, Feb 10, 2017 at 11:26:22AM -0500, Harry Wentland wrote: > Resending with CC to dri-devel as per Alex's suggestions. This > might be of interest to a wider audience. > > These patches are first steps of addressing some of the problems > in DC's atomic implementation. Please take a look and provide > feedback if possible. Our hope is that we can start setting a > direction on fixing up DC to do atomic correctly and lay the > groundwork for moving past the midlayer. > > THe biggest patch here is Andrey's work to bring atomic_commit > in line with the atomic helpers instead of rolling our own. We > got atomic_commmit_tail now and things appear to work correctly > with this change. It allowed us to clean up some of the commit > code, but there's still a lot left. > > The second important patch is fixing up our gamma implementation > and correct the use of crtc_set_property and atomic_set_properties. > > Beyond that there's some minor cleanup and support patches for > the above change. > > The whole DC tree with these patches and rebased on drm-next a couple > days ago can be found at > > https://cgit.freedesktop.org/~hwentland/linux/log/?h=dc-drm-next-atomic > > Known issue: > - corruption on one display in two-display setup Props to amd for starting to submit core stuff and critical driver bits for review, but since these are incremental patches a bit hard to review&comment ... Not sure what best to do, since I can't really justify to my boss that I constantly look at the entire amdgpu-dal branch either. Probably best if you folks ping me and others on irc with questions directly, and then I try to sometimes take a look at the end result. Probably best to wait until you've worked down the todo list for an area though. -Daniel > > Cheers, > Harry > > Andrey Grodzovsky (3): > drm/amdgpu: Add a few members to support DAL atomic refactor. > drm/amd/display: Refactor atomic commit implementation. > drm/amd/display: Refactor headless to use atomic commit. > > Harry Wentland (5): > drm/amdgpu: Expose mode_config functions for DM > drm/amd/display: Use amdgpu mode funcs statically > drm/amd/display: Use atomic helpers for gamma > drm/amd/display: Remove unused define from amdgpu_dm_types > Revert "drm/amdgpu: Refactor flip into prepare submit and submit. > (v3)" > > drivers/gpu/drm/amd/amdgpu/amdgpu_display.c| 140 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_display.h| 33 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 19 +- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 70 ++- > .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 548 > + > .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.h| 12 +- > 6 files changed, 341 insertions(+), 481 deletions(-) > create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_display.h > > -- > 2.9.3 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/rockchip: add extcon dependency for DP
The newly added DP driver links against the extcon core, which fails when
extcon is a module and this driver is not:
drivers/gpu/drm/rockchip/cdn-dp-core.o: In function `cdn_dp_get_port_lanes':
cdn-dp-core.c:(.text.cdn_dp_get_port_lanes+0x24): undefined reference to
`extcon_get_state'
cdn-dp-core.c:(.text.cdn_dp_get_port_lanes+0x44): undefined reference to
`extcon_get_property'
Let's make Kconfig enforce correct behavior with a dependency.
Fixes: 1a0f7ed3abe2 ("drm/rockchip: cdn-dp: add cdn DP support for rk3399")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/rockchip/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index ad31b3eb408f..0e4eb845cbb0 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -24,6 +24,7 @@ config ROCKCHIP_ANALOGIX_DP
config ROCKCHIP_CDN_DP
tristate "Rockchip cdn DP"
depends on DRM_ROCKCHIP
+ depends on EXTCON
select SND_SOC_HDMI_CODEC if SND_SOC
help
This selects support for Rockchip SoC specific extensions
--
2.9.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/atmel-hlcdc: Simplify the HLCDC layer logic
On Fri, Feb 10, 2017 at 07:07:45PM +0100, Boris Brezillon wrote:
> An HLCDC layers in Atmel's nomenclature is either a DRM plane or a 'Post
> Processing Layer' which can be used to output the results of the HLCDC
> composition in a memory buffer.
>
> atmel_hlcdc_layer.c was designed to be generic enough to be re-usable in
> both cases, but we're not exposing the post-processing layer yet, and
> even if we were, I'm not sure the code would provide the necessary tools
> to manipulate this kind of layer.
>
> Moreover, the code in atmel_hlcdc_{plane,layer}.c was designed before the
> atomic modesetting API, and was trying solve the
> check-setting/commit-if-ok/rollback-otherwise problem, which is now
> entirely solved by the existing core infrastructure.
>
> And finally, the code in atmel_hlcdc_layer.c in over-complicated compared
> to what we really need. This rework is a good excuse to simplify it. Note
> that this rework solves an existing resource leak (leading to a -EBUSY
> error) which I failed to clearly identify.
>
> Signed-off-by: Boris Brezillon
> ---
> Hi Daniel,
>
> I intentionally dropped your ack, since inheriting from atmel_hlcdc_layer
> is implying a lot of changes.
Well I acked the idea, that still kinda holds. But if you want to
kickstart the drm-misc driver ack economy, Eric has 1-2 vc4 patches that
still need an ack, you could trade r-bs :-)
Cheers, Daniel
>
> Regards,
>
> Boris
>
> Changes in v2:
> - make atmel_hlcdc_plane inherit from atmel_hlcdc_layer
> - provide read/write_reg/cfg() helpers to access layer regs
> - move all layer related definitions into atmel_hlcdc_dc.h and remove
> atmel_hlcdc_layer.h
> - fix a bug in atmel_hlcdc_plane_atomic_duplicate_state()
> ---
> drivers/gpu/drm/atmel-hlcdc/Makefile| 1 -
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 39 +-
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 82 +--
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h| 364 +++--
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c | 666
>
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.h | 399 --
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 637 +++---
> 7 files changed, 695 insertions(+), 1493 deletions(-)
> delete mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c
> delete mode 100644 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.h
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/Makefile
> b/drivers/gpu/drm/atmel-hlcdc/Makefile
> index 10ae426e60bd..bb5f8507a8ce 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/Makefile
> +++ b/drivers/gpu/drm/atmel-hlcdc/Makefile
> @@ -1,6 +1,5 @@
> atmel-hlcdc-dc-y := atmel_hlcdc_crtc.o \
> atmel_hlcdc_dc.o \
> - atmel_hlcdc_layer.o \
> atmel_hlcdc_output.o \
> atmel_hlcdc_plane.o
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 9b17a66cf0e1..2fcec0a72567 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -445,8 +445,8 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs
> = {
>
> int atmel_hlcdc_crtc_create(struct drm_device *dev)
> {
> + struct atmel_hlcdc_plane *primary = NULL, *cursor = NULL;
> struct atmel_hlcdc_dc *dc = dev->dev_private;
> - struct atmel_hlcdc_planes *planes = dc->planes;
> struct atmel_hlcdc_crtc *crtc;
> int ret;
> int i;
> @@ -457,20 +457,41 @@ int atmel_hlcdc_crtc_create(struct drm_device *dev)
>
> crtc->dc = dc;
>
> - ret = drm_crtc_init_with_planes(dev, &crtc->base,
> - &planes->primary->base,
> - planes->cursor ? &planes->cursor->base : NULL,
> - &atmel_hlcdc_crtc_funcs, NULL);
> + for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) {
> + if (!dc->layers[i])
> + continue;
> +
> + switch (dc->layers[i]->desc->type) {
> + case ATMEL_HLCDC_BASE_LAYER:
> + primary = atmel_hlcdc_layer_to_plane(dc->layers[i]);
> + break;
> +
> + case ATMEL_HLCDC_CURSOR_LAYER:
> + cursor = atmel_hlcdc_layer_to_plane(dc->layers[i]);
> + break;
> +
> + default:
> + break;
> + }
> + }
> +
> + ret = drm_crtc_init_with_planes(dev, &crtc->base, &primary->base,
> + &cursor->base, &atmel_hlcdc_crtc_funcs,
> + NULL);
> if (ret < 0)
> goto fail;
>
> crtc->id = drm_crtc_index(&crtc->base);
>
> - if (planes->cursor)
> - planes->cursor->base.possible_crtcs = 1 << crtc->id;
> + for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) {
> + struct atmel_hlcdc_plane *overlay;
>
> - for (i
Re: [Intel-gfx] [PATCH] drm: Reduce EDID warnings from DRM_ERROR to DRM_NOTE
On Mon, Feb 13, 2017 at 12:17:27PM -0500, Sean Paul wrote: > On Mon, Feb 13, 2017 at 3:59 AM, Chris Wilson > wrote: > > On Mon, Feb 13, 2017 at 08:41:10AM +0100, Thierry Reding wrote: > >> On Fri, Feb 10, 2017 at 07:59:13PM +, Chris Wilson wrote: > >> > The warnings from parsing the EDID are not driver errors, but the > >> > "normal but significant" conditions from the external device. As such, > >> > they do not need the ferocity of an *ERROR*, but can use the less harsh > >> > DRM_NOTE instead. > >> > > >> > Signed-off-by: Chris Wilson > >> > --- > >> > drivers/gpu/drm/drm_edid.c | 15 --- > >> > 1 file changed, 8 insertions(+), 7 deletions(-) > >> > >> The below are all conditions that happen when the EDID is bad. I'm not > >> sure that really qualifies as "normal". > > > > Often it is - a bad EDID on the monitor will always be bad. The > > challenge is distinguishing that from silent data corruption during the > > read - a reported read failure are trivial. > > > >> From a quick look through the code we don't always trigger an error from > >> the below failure paths at higher levels, so decreasing the level here > >> has the potential to let this kind of exceptional condition go > >> unnoticed. > > > > The messages are not gone, they are higher than the default loglevel, > > but now below the level at which they are printed to a terminal. The > > bad EDID is either expected or recoverable, and definitely not fatal > > so I don't think an *ERROR* is justified. > > I tend to agree. > > The description for the KERN_NOTICE level is "normal but significant > condition". I might argue that the presence of these EDID messages > represents a normal *or* significant condition (depending on why the > EDID is bad), but I don't think it's unreasonable to expect people to > check their logs if the display/mode is not working properly. So for cases where we know that there is shit hw out there (specifically kvm switches that mangle the cea block without adjusting the edid) we already tune down the error to debug level. So in principle totally agree with tuning down anything that happens because it's outside of our control to info or debug, but do we still need this patch after the cea one has landed? Our CI at least seems happy ... Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH RESEND] drm/dp/mst: fix kernel oops when turning off secondary monitor
On Tue, Feb 14, 2017 at 02:49:21PM +0200, Jani Nikula wrote:
> From: Pierre-Louis Bossart
>
> 100% reproducible issue found on SKL SkullCanyon NUC with two external
> DP daisy-chained monitors in DP/MST mode. When turning off or changing
> the input of the second monitor the machine stops with a kernel
> oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
>
> This issue is traced to an inconsistent control flow in
> drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the
> same time as 'req_payload.num_slots' is set to zero, but the pointer is
> dereferenced even when req_payload.num_slot is zero.
>
> The problematic dereference was introduced in commit dfda0df34
> ("drm/mst: rework payload table allocation to conform better") and may
> impact all versions since v3.18
>
> The fix suggested by Chris Wilson removes the kernel oops and was found to
> work well after 10mn of monkey-testing with the second monitor power and
> input buttons
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
> Fixes: dfda0df34264 ("drm/mst: rework payload table allocation to conform
> better.")
> Cc: Dave Airlie
> Cc: Chris Wilson
> Cc: Nathan D Ciobanu
> Cc: Dhinakaran Pandiyan
> Cc: Sean Paul
> Cc: # v3.18+
> Tested-by: Nathan D Ciobanu
> Reviewed-by: Dhinakaran Pandiyan
> Signed-off-by: Pierre-Louis Bossart
> Signed-off-by: Jani Nikula
You haz drm-misc commit rights, pls use them :-)
Since it doesn't have deps, probably simplest to smash into drm-misc-fixes
and then send a pull req to Dave right away. If you want, you can roll
-fixes forward to -rc8 while at it.
-Daniel
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 122a1b04bebc..f2cc375907d0 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct
> drm_dp_mst_topology_mgr *mgr)
> mgr->payloads[i].vcpi = req_payload.vcpi;
> } else if (mgr->payloads[i].num_slots) {
> mgr->payloads[i].num_slots = 0;
> - drm_dp_destroy_payload_step1(mgr, port,
> port->vcpi.vcpi, &mgr->payloads[i]);
> + drm_dp_destroy_payload_step1(mgr, port,
> mgr->payloads[i].vcpi, &mgr->payloads[i]);
> req_payload.payload_state =
> mgr->payloads[i].payload_state;
> mgr->payloads[i].start_slot = 0;
> }
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
